mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-11 16:36:32 +02:00
main
377 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
6bce42b84a |
MUL-5983 fix(workspace): stop workspace delete from hanging on advisory lock 4246 (#6710)
* fix(workspace): stop workspace delete from hanging on advisory lock 4246 (MUL-5983) Deleting a workspace from the latest client did nothing: the confirm dialog stayed on "Deleting…" forever and the workspace survived. The teardown transaction takes global advisory lock 4246 (added in #6230) so the task_usage_hourly rollup cannot write aggregates for a workspace being torn down, and it waited on that lock with no bound. Nothing above the handler has a deadline either — the API client's fetch has no timeout — so any holder of 4246 turns the delete into an infinite spinner. Two fixes: - Migration 272 makes the rollup's lock transaction-scoped. Migration 102 used the session-scoped pg_try_advisory_lock and released it from an `EXCEPTION WHEN OTHERS` handler, but plpgsql's OTHERS does not match query_canceled and a session-level advisory lock survives the rollback that follows. A cancelled tick therefore handed its pooled connection back to pgxpool still holding 4246 — permanently blocking every later tick (usage aggregation silently stops) and every workspace delete. - DeleteWorkspace sets SET LOCAL lock_timeout on its transaction and maps SQLSTATE 55P03 to 503. Lock 4246 is also held for whole runs by the backfill commands and for up to 25 min by a rollup tick, so contention has to surface as a retryable error instead of a request that never returns. The cap applies to lock waits only; the teardown work itself stays unbounded. Both paths are pinned by tests that fail without the fix: the scheduler test proves a cancelled tick leaks 4246 against the old function, and the handler test proves DeleteWorkspace blocks on a held 4246 without the lock timeout. Co-authored-by: multica-agent <github@multica.ai> * test(workspace): serialise the new 4246 tests with the rollup guard (MUL-5983) Review found that the two new handler tests take advisory lock 4246 by hand without joining the cross-binary guard the rollup family uses (42463980), so `go test` running internal/handler and internal/scheduler in parallel against one database made the scheduler's lock tests fail. Reproduced locally, then fixed by calling lockRollupSingleton in both. Stressing the combined run surfaced a second, older interference: every workspace teardown takes 4246 in production code (#6230), so any handler delete test can own the lock for a moment and leave the scheduler's rollup tests seeing "no work" — TestPgCronConcurrentNoDoubleWrite fails with winners=0, and this predates the guard fix above. Both rollup tests now treat a round lost to an outside holder as a retry rather than a verdict; the real invariants (no leaked lock, no double write) are unchanged. Also from review: correct the sqlc source comment that still described the rollup's lock as session-scoped (regenerated), and drop migration 272's claim that the TTL prune is bounded and only delays the next tick — it has no row cap and now also delays workspace deletes waiting on 4246. Co-authored-by: multica-agent <github@multica.ai> * test(workspace): make the advisory-lock holder release idempotent (MUL-5983) The 15 s deadline branch releases the 4246 holder early so the blocked handler can finish, and the defer then released it again. pgxpool's Release is idempotent but Exec on a returned connection dereferences a nil resource, so the second call panicked and buried the assertion failure it was supposed to report. sync.Once makes the cleanup safe from both paths. Verified by forcing the deadline branch (lock_timeout temporarily removed from DeleteWorkspace): the test now reports the assertion cleanly where it previously ended in a nil-pointer panic. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
|
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 |
||
|
|
984a2c2bff |
MUL-5954: feat: saved issue views V1 (MUL-4796) (#6516)
* feat(issues): server-backed saved issue views API (MUL-4796) issue_view table (migrations 262-264: table + two CONCURRENTLY partial indexes) stores named filter definitions: query jsonb is the shared identity, display jsonb only seeds a user's first open. Scope model is workspace / my (with scope_variant, forced private) / project (validated scope_id, deleted with the project in the same app transaction). issue_view_preference (migration 265) holds each user's view-bar layout (hidden + order doc) per surface container, last-write-wins. CRUD + GET/PUT preference endpoints are gated on the saved_issue_views_v1 feature flag (404 while off, key published to the frontend config). Authorization: private views 404 for everyone but the owner; shared views editable by owner or workspace admin; updates use expected_revision -> 409. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(core): saved issue views client core (MUL-4796) - zod schemas + parseWithFallback for views and preferences (definition blobs stay loose records interpreted per definition_version), with malformed-response tests per the API-compat rules - issue-views module: list/create/update/delete + preference queries with wsId-scoped keys; preference upsert is optimistic (toggle-grade rollback) - active-view store (zustand client state, URL is the durable carrier) and useActiveIssueView with missing-view detection - baseline.ts: enum-sanitized FilterSnapshot of a view's query for value-level locking, delta chips, and baseline-aware resets - view-store: clearFilterDimension + resetFiltersTo actions; surface store seeding routes server blobs through mergeViewStatePersisted Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(issues): three-bar priority icons with a distinct urgent badge low/medium/high share one three-bar frame (fill level = severity, ghost bars at 35%); urgent breaks out of the scale as a filled badge with an exclamation cutout — it is an interrupt, not a fourth bar. All glyphs share the same 2-14 optical frame. Board card: PickerWrapper's bare block div gave its inline-flex trigger a line box (line-height 24px > 20px button), floating icons above the text midline — the wrappers are flex containers now, and the priority trigger gets a constant size-5 box so the row's rhythm doesn't depend on the glyph. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(views): saved views UI — view bar, save/edit dialog, locked view mode (MUL-4796) View bar replaces the built-in tab row on Issues and My Issues: built-in tabs and saved views render as one flat, per-user ordered row that wraps instead of overflowing, with a menu for new-view / manage. The manage dialog does drag reorder (translate-only transform, vertical axis lock), show/hide (anchor built-in unhidable), and edit/delete for owned views. Open-view semantics: the view's query is the baseline — its values render checked-and-disabled in the filter menu, never as chips; chips show only the user's additions and every reset path (chip x, clear, menu reset, filtered-empty CTA, board/swimlane hide-column) returns to the baseline, not to empty. Display seeds once per user (view:<id> surface key) and is free afterwards. Built-in tab click exits; deleted/revoked views fall back with a toast; ?view= deep links sync on web via a platform hook. Save dialog doubles as the edit dialog (PATCH with expected_revision -> 409 toast); manager edits seed from the view definition, in-view edits from the live panel. Filter chips bar, extracted IssueFilterMenu (virtual frozen anchor), and the reworked display panel round out the header. Also: filter menu consistency fixes (closeOnClick, calendar apply, TableColumnPicker checkbox items), SelectGroup padding, dropdown anchor passthrough, four-locale i18n. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(views): neutralize workspace tab filter while a saved view is open The Members/Agents tabs are a coarse assignee-type filter carried in the surface scope, outside the filter-menu dimensions a view can express. It kept applying underneath an open view, so the same shared view returned different rows depending on which tab the user stood on when opening it — with nothing on screen to explain the difference (tabs dimmed, no chip). While a view is active the workspace scope now resolves to actorKind "all": a view's results are defined by the view alone, matching what the dimmed tabs already imply. Deliberately NOT captured into saved queries — a view condition must be visible and lockable in the filter menu, and assignee-type has no menu representation (add the menu dimension first if that need ever materializes; the loose schema takes the field additively). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(issue-views): workspace scope_variant captures and applies the assignee-type tab A workspace view now saves which built-in tab it was created from (members/agents, NULL = all) as scope_variant, alongside the existing my-scope variants. The save dialog shows a scope selector so the variant can be switched later; scope_type stays immutable. While a view is open, its own variant drives the workspace scope's assignee-type axis instead of whichever tab the user stood on. - migration 266 widens the scope_variant CHECKs (members/agents on workspace, NULL = all; my keeps its four variants; project stays NULL) - UpdateIssueView persists scope_variant; handler validates the variant against the view's scope_type on create and update - issue-surface applies activeView.scope_variant to effectiveScope, replacing the blanket tab neutralization - issues-header seeds the dialog scope from the live tab Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(issue-views): project pages get a real assignee-type axis and clearer variant copy The Members/Agents tabs on project pages were decorative: they wrote the global tab store but the project query never read it. The project scope now carries actorKind like the workspace scope (assignee_types composes with project_id server-side), so the tabs filter for real, and project views join the same optional scope_variant vocabulary — saved from the tab, switchable in the dialog, applied while the view is open. - migration 266 (unmerged) widened in place: project allows members/agents, NULL = all - handler treats workspace and project variants identically; new TestProjectIssueViewVariant covers persist / normalize / reject - project scope key gains a per-tab segment (members/agents) so each tab keeps its own display state; the unrestricted tab keeps the old key - save dialog shows the variant selector for project views; the surface now provides the dialog's default variant directly (header-side injection removed) - variant labels spell out the meaning (Assigned to members/agents, Any assignee, Any relation) and project hints are variant-aware, x4 locales Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(issue-table): project table scope honors assignee_types The table/rows channel dropped the Members/Agents narrowing on project pages: the client's project scope never sent assignee_types and the server's project branch never compiled it, so switching tabs only changed the highlight while table rows stayed put (the board/list channel already filtered). Both sides now share the workspace scope's optional assignee-type narrowing, with a compile test covering the predicate and the invalid-type rejection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(issues): single assignee-type mapping + per-page tab state One core helper pair (assigneeTypesForActorKind / actorKindForViewVariant) replaces the four inlined tab->assignee_types literal mappings across query-plan, the table spec compiler, and the view-variant application. The assignee-type tab store becomes page-keyed (issues / project:<id>, persisted with a v0 migration carrying the old global tab to the Issues page): switching tabs inside a project no longer drags the Issues page or other projects along. The save-view dialog's default variant now comes from the header's own page tab, restoring the /issues default that the surface-side injection had dropped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(issues): converge all issue surfaces onto the table query channel The legacy list/grouped query layer duplicated the table channel's scope semantics behind enable-conditions that were provably always false: list and status-board ride server status branches, every non-status grouping and swimlane ride server group branches, and the assignee-board / per-status load-more / flat-list / flat-export / client-side my:any 3-request merge fallbacks could never execute. Delete the whole layer: - queries: myIssueListOptions, myIssueAssigneeGroupsOptions, issueAssigneeGroupsOptions, issueFlatListOptions/ExportOptions, fetchAllFlatPages / fetchAllMyFlatIssues / fetchAllMyFirstPages / fetchAllMyAssigneeGroups, compareIssuesForSort (server owns ordering) - mutations: useLoadMoreByStatus, useLoadMoreByAssigneeGroup - board/swimlane: the never-taken client-paginated columns, property pool loader, hidden-column client counts, legacy footer rows - surface data hook / controller: dead queries and passthrough fields - gantt keeps its scheduled-only endpoint and now compiles the assignee-type tab into it, restoring the change alongside the cleanup query-plan shrinks to the scope's non-Table residue: scopeKey, the gantt filter, and create defaults. issueKeys and cache invalidation prefixes are untouched; mobile's own data layer is unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(issues): review fixes — active view's variant wins in save dialog, stable dialog scope, dead mocks Post-review pass on the convergence refactor: - The save dialog's default variant now prefers the OPEN view's scope_variant over the page tab: saving a copy while an agents-scoped view is open must describe the rows on screen, not the tab underneath. - The dialog scope prop is memoized on primitive projections — a fresh object per header render re-armed the dialog's draft-reset effect, so any background refetch wiped a half-typed name. - Tests: persist migration coverage for the page-keyed tab store; gantt assignee_types reaches the request and forks the cache key. - Dead legacy load-more mocks removed from three view test files; stale hidden-columns doc comment; unknown hidden-column totals render no count instead of a blank; my-relation plan branch is return-exhaustive; leftover blank lines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(issues): keep the grabbing cursor for the whole row drag in manage views The handle's cursor classes only apply while the pointer hovers the handle — the vertical-locked drag loses it immediately, and buttons the pointer crosses flip it to pointer. Mirror the sidebar-resize cursor contract: an html[data-dnd-dragging] rule forces cursor: grabbing (and no text selection) document-wide for the drag's duration, toggled by the DndContext lifecycle with an unmount safety net. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(issues): my views apply their relation at the query layer, not by switching the tab Opening a my-view used to overwrite the user's relation tab with the view's variant (VARIANT_TO_TAB), so when an open view vanished (deleted / access revoked) the fallback landed on the view's tab instead of where the user actually was. Views are now applied the same way on all three surfaces: while a view is open, effectiveScope substitutes the view's variant — assignee-type for workspace/project, relation for my — and the user's own tab state is never touched. myRelationForViewVariant joins its sibling mapping in scope.ts with unit coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(issues): ease the save-view dialog's expand and validation transitions The display-defaults section snapped open/closed and the name-required hint popped in. The panel now runs the Base UI height transition (--collapsible-panel-height + data-starting/ending-style — the tw-animate collapsible keyframes only know Radix's variable, so the accordion's animate-* classes cannot be reused here), and the hint eases in with the house animate-in vocabulary. Scoped to this dialog: the shared Collapsible primitive stays untouched because chat/transcript panels grow while open and a pinned panel height would clip them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(issues): filter chips ease in Chips popped into the bar and the save-view dialog's embedded list with no transition. Each chip now enters with the house fade+zoom vocabulary, keyed per dimension so value edits inside an existing chip don't re-animate. No exit animation by design. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(issue-views): drag-reorder the view bar, tab context menu, pin views to sidebar The bar's tabs (built-ins and saved views alike) now reorder by drag, writing the same per-user preference document the manage dialog edits — one list, two editors. Saved-view tabs gain a context menu: - Edit: visible to everyone, disabled without manage permission (the greyed row is the signal); Delete: rendered only for the owner or a workspace admin on shared views — mirroring the server's canManageIssueView — and routed through the shared confirm dialog now extracted from the manage dialog. The manage dialog's own affordances follow the same widened rule (admins included). - Pin to sidebar: pinned_item grows a 'view' type (migration 267; create validates read access so foreign private views 404). Sidebar pin rows resolve the view's name via the new GET-by-id options, render the view-bar icon, activate the view in the store and navigate to its owning surface on click. Flag-off clients keep view pins dormant instead of auto-unpinning; deleted views follow the existing 404-auto-unpin pattern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(issue-views): review fixes — guard preference wipes, never auto-unpin views Two data-loss paths from review, plus hardening: - A drag landing before the views list loaded pruned the preference document against an empty list, silently erasing every saved-view entry from order and hidden. savePrefs now refuses to write until the list has actually loaded (viewsReady threads down from useActiveIssueView). - View pins are exempt from 404-auto-unpin: the detail endpoint also 404s when the feature flag is switched off server-side, and a client that booted with the flag on cannot tell the difference — a flag flip would have deleted every view pin permanently. Deleted views' rows hide instead. - POST /api/pins with item_type=view now sits behind the same feature gate as every other view endpoint. - SortableBarTab: drop dnd-kit's aria attributes (they nested a second focusable role=button around the tab), move the wasDragged ref into an effect, and clear it after a cancelled drag so the next real click isn't swallowed. - Sidebar view pins carry ?view= in their href so a web reload stays on the view; scope resolution collapsed to one branch driving both the path and the container key. - Malformed GET /api/issue-views/{id} responses pinned by test as null (hide the row), never an error (which alone may unpin). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(issue-views): single-row view bar with overflow popover The bar no longer wraps: tabs that don't fit collapse into a trailing popover, Linear-style, with two differences — built-ins are first-class orderable entries, and the whole list (hidden rows dimmed) lives in the pulled-down panel with drag reorder, per-row actions (edit/delete under the same permission rule, pin, hide/show) and a new-view entry. - useSingleRowFit (views/common): hidden mirror row measures natural tab widths; ResizeObserver + change-guarded per-commit remeasure computes the fitting prefix with trigger reservation. First reusable collapse-into-menu primitive in the repo. - The open view is always visible: when it overflows it takes the trigger slot, showing its name + chevron (wider reservation; the two-pass promotion settle is monotonic, no oscillation). - Every tab now shares one max width (max-w-40) and truncates with the full name in tooltips/rows. - ManageViewsDialog retired: the popover carries all of its duties (reorder, hide/show incl. built-ins, edit/delete); hiding stays impossible for the anchor built-in and still exits a hidden open view. DeleteViewConfirm and ViewBarItem move to view-bar-popover.tsx; orphaned i18n keys dropped, row action keys added x4 locales. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(issue-views): restore [layers] menu + manage dialog; 'more' appears only on overflow; fix zero-width bar The single-row rework overreached: it replaced the existing new/manage entries and dialog with an all-in-one popover, and the bar's container collapsed to zero width (content-sized parent + measurement feedback), rendering an empty header. Back to the intended shape: - The [layers] menu (new view / manage views) and the manage dialog are restored exactly as before; DeleteViewConfirm and ViewBarItem stay shared from view-bar-popover. - The 'more' trigger exists ONLY while tabs overflow, showing just the overflowed entries (click to open; per-row edit/delete/pin/hide with the same permission rules — those tabs have no rendered surface to right-click). The open view still takes the trigger slot when it overflows. - Header view-bar slots gain flex-1 so the fit measures a real width; reserve tiers (menu / +more / +promoted) settle monotonically. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(issue-views): list panel shows ALL tabs with drag reorder; trigger always visible The pulled-down panel now lists every bar entry (built-ins included) in bar order with vertical drag reorder — since the bar renders the longest fitting prefix, reordering in the panel directly decides what sits on the bar versus tucks away, with a separator marking the current fold. The trigger no longer appears/disappears with overflow: it is a fixed part of the bar's right side (still replaced by the open view's own tab when that view overflows). Row actions (edit/delete/pin/hide) and the [layers] new/manage entries are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(issue-views): kill the post-drop bounce with a synchronous order mirror The preference mutation is optimistic but its onMutate is async: on drop, dnd-kit clears the drag transform in the same frame while the reordered list lands a beat later, so the dropped tab flashed back to its old slot and then jumped — the manage dialog never bounced because it mirrors the new order synchronously in the drag handler. Both bar drag and panel drag now route through applyMove: set a local order mirror in the same tick, persist, and clear the mirror once the preference data catches up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(issue-views): panel row menu is always visible, not hover-revealed Hover-reveal made the three-dot row menu unreachable on touch devices; it is a primary affordance in the list panel, so it renders always. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(issue-views): long view names truncate/wrap everywhere The manage dialog's row strip is a grid item — min-width:auto let an 80-char name blow past the dialog frame, carrying the row controls with it. Pin the wrapper (min-w-0 + overflow-hidden) and the rows (min-w-0) so labels truncate as designed. The delete confirmation and the save dialog's scope hint interpolate names/titles into prose — break-words so an unbroken name wraps instead of overflowing. Audited every other name surface (bar tabs, promoted trigger, panel rows, sidebar pins, context menus): all already bounded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(issue-views): remove the saved_issue_views_v1 feature flag The feature ships unconditionally. The flag never reached any released client, so — unlike the retired flags that live on as always-on compat keys — the key is removed outright on both sides: - server: the issueViewsEnabled 404 gate drops from every view endpoint, the preference endpoints and the view pin type; the key leaves the registry and the frontend-public list; gate tests and the per-test enable helper go with it - frontend: the === true checks, the pre-view-bar fallback tab rows in both headers, and useActiveIssueView's enabled parameter are deleted; the view bar is now the only rendering - compat holds by construction: a NEW client against an OLD backend gets 404s on the list/preference queries → empty views, viewsReady stays false (no preference writes), sidebar view pins hide without auto-unpinning (comment updated to name this as the reason); an OLD client against the NEW backend reads an absent flag as false and keeps the feature hidden Full-suite run also surfaced that the workspace deletion manifest never classified issue_view / issue_view_preference: both now fall with the other issue roots in DeleteWorkspaceIssueRoots (no-FK policy: explicit teardown), and the manifest records the decision. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(issue-views): fit reserve applies even when every tab fits The all-fit fast path skipped the reserve — a leftover from when the overflow trigger only rendered on overflow. With the list trigger and the [layers] menu now permanent, a row whose tabs exactly filled the container declared everything fitting and overflow-hidden clipped the trailing chrome. The reserve is now subtracted unconditionally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(issue-views): a freshly created view opens itself Creating a view now activates it on its surface immediately — the view you just saved is the one you meant to be looking at. Skipped only when the create response fails to parse (the list refetch still shows it). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(issue-views): creating a view no longer bounces the surface to the default tab Activation raced the list invalidation: the fresh view's id was set active before the refetched list contained it, the stale list read as 'view deleted', and the missing-view fallback kicked the surface back to the anchor tab (with an 'unavailable' toast). Two-sided fix: the create mutation seeds the created view into its scope's list cache before invalidating, and the missing verdict now requires the list to not be mid-refetch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(issue-views): release-gate fixes — rollback safety, limits, old-client pin compat, revoke sweep Addresses the pre-ship review blockers: - CI styles: text-muted-foreground/70 -> text-faint-foreground (solid tone rule) in the header hint and the dialog my-note; the chip emoji stack's text-[9px] -> text-micro (type-scale rule). apps/web text-contrast + type-scale suites pass. - migration 269 down normalizes workspace/project members/agents variants to NULL before restoring the old constraints — verified on a scratch DB with real variant data (up -> write -> down). - view pins are withheld from the legacy /api/pins contract unless the client opts in (?include=view): old Desktop builds classified any non-issue pin as a project pin and permanently auto-unpinned it on 404. New clients opt in; regression test pins one and asserts the legacy list never leaks it. - write hardening: MaxBytesReader (128KB) on view create/update and preference PUT; isJSONObject rejects JSON null (was a DB-CHECK 500); per-owner view quota (100/workspace); list query LIMIT 200. Boundary tests for null blob, oversized body, and quota. - member revoke now sweeps the departed member's PRIVATE views and their view-bar preferences in the same transaction (shared views stay, same rule as private quick actions) — the migration comments promised this; test covers private-gone/shared-stays/prefs-gone. - sidebar comment no longer claims project ?view= reload support. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(issue-views): deleting a view sweeps its sidebar pins on every path View pins never auto-unpin client-side (old-backend safety), so a pin whose view is gone was invisible in the UI and unremovable forever — and could leave an empty-looking Pinned group. All three deletion paths now sweep matching pinned_item rows atomically in the same statement via CTEs: direct view delete, project deletion (its scoped views), and member revoke (their private views). Regression tests cover each path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
cdf93e4914 |
MUL-5892 fix(chat): cancel queued tasks when a session is archived, and stop telling non-Slack sessions they have no channel (#6595)
* fix(chat): cancel queued tasks when a session is archived, and stop telling non-Slack sessions they have no channel Two channel-agnostic fixes found while auditing WeCom; both help Lark and DingTalk equally. **Archiving left queued tasks claimable.** SetChatSessionArchived deletes the channel binding but never cancelled the session's in-flight tasks, and ClaimAgentTask does not read chat_session.status. So the daemon still ran the turn — and with the binding row already gone, the runtime brief described a private web chat while the channel adapter still held the chat id and posted the answer into the group. The person who archived the conversation never saw it happen. The cancel now runs first, in the same transaction. DeleteChatSession has always done this, for the reason its own query comment gives — "so the daemon doesn't keep running work whose result has nowhere to land" — and archiving means the same thing. **The empty-history note lied to every non-Slack session.** The reader is Slack-only, so WeCom, Lark and DingTalk all land on "this conversation is not connected to a chat channel", which is false. The reader is an agent deciding who can see its answer: told it is in a web-only conversation it reasons differently than told it is in a group whose backlog it cannot read. The note now names the platform, and the response carries channel_type. Written channel-agnostically — a per-platform lookup here would go blind the next time a channel is added, which is how this came to be wrong. Two tests against a real database. Both drive the real paths, not the helpers: task status after archive = "queued", want cancelled — the daemon will still run this turn, and with the channel binding already gone its answer goes to the room while the brief says private a WeCom-bound session was told it is not on a channel: "This conversation is not connected to a chat channel, so there is no channel history to read." * fix(chat): broadcast the archive cancellation instead of only writing it Review follow-up on the archive-cancel path. SetChatSessionArchived dropped the rows CancelAgentTasksByChatSession returns, so the cancellation reached the database and stopped there. None of the post-commit lifecycle ran: no captureTaskCancelled (the tasks' 24h mat_ tokens stayed live), no ReconcileAgentStatus (an agent whose only task was cancelled kept showing 'working'), no task:cancelled (other clients kept the turn on screen until something else forced a refresh), and no notifyTasksFinished (the runtime waited for the daemon's next poll before claiming the next task). The query is not limited to never-started work — it also matches dispatched, running, waiting_local_directory and deferred — so this is most visible when a running turn is cancelled. Keep the rows and hand them to TaskService.BroadcastCancelledTasks after the commit, which is what DeleteChatSession has always done twenty lines below. The empty-history response also read the binding twice, once for channel_type and once inside noHistoryNote. An archive landing between the two reads could answer channel_type "wecom" next to a note saying the conversation is not connected to a chat channel. Resolve the channel type once and derive both from it; noHistoryNote becomes a pure function of that value, so the two cannot disagree. TestArchivingAChatSessionCancelsItsQueuedTasks now queues a running turn alongside the queued one, marks the agent 'working', mints a task token, and asserts the post-commit side as well as the status column: agent back to 'idle', task:cancelled for both rows, token revoked. Without the broadcast it fails on all three. The history test drives the real response path on both legs and pins channel_type against the note. * fix(chat): scope the archive cancel to sessions that were on a channel Review follow-up. SetChatSessionArchived cancelled on every archive, with no check for whether the session had ever been bound to a channel. That is wider than the bug: the failure this fixes is a channel-bound session answering into a group room after its binding was dropped, and a plain web chat has no room, no adapter and no late answer. Cancelling there was a new product behaviour riding along with the fix, and it contradicts what the UI says archive is. A session row offers Stop or Archive, never both (chat-thread-list.tsx:364-377). Stop is `danger` behind a confirmation row; Archive fires immediately with none. Archive is the reversible step: unarchive exists, and hard delete is offered only once a chat is archived (chat-session-header.tsx:169-186). The two also cost the user different things. Stop goes through CancelTaskByUser, which hands the typed prompt back as cancelled_chat_message.restore_to_input or a durable chat_draft_restore row; CancelAgentTasksByChatSession is a bare UPDATE with neither, and BroadcastCancelledTasks does capture, reconciliation, the event and the runtime wakeup but no draft restore. So an unconditional cancel let one unconfirmed click destroy a prompt that unarchiving cannot bring back. The cancel now runs only when GetChannelChatSessionBindingBySessionAny finds a binding, read before DeleteChannelChatSessionBindingBySession erases it. A read error that is not ErrNoRows fails the archive rather than silently skipping the cancel — "not bound" and "cannot tell" must not take the same branch. This also settles the header's Archive item having no isRunning gate: archiving a running web chat from the dropdown now cancels nothing, so it no longer bypasses the confirmation the Stop button requires. Gating that item is a frontend question and stays out of this PR. The tests are one fixture archived twice. The bound leg is the previous test with the binding it was always describing — its failure message named an answer reaching the room, but it never created a room. It keeps every post-commit assertion (queued + running cancelled, agent back to idle, task:cancelled for both, token revoked) and adds that the binding is still deleted. The web-only leg asserts the opposite: both turns still in flight, agent still working, no task:cancelled, token still live, session still archived. Dropping the cancel fails the first; making it unconditional again fails the second; reading the binding after the delete fails the first. * fix(chat): refuse a debounced chat enqueue onto an archived session Review follow-up. The archive cancel only reaches tasks that already exist, and the channel run trigger is debounced by DefaultChatRunBatchWindow: the message row is persisted on arrival, the task row is created a window later when flushChatRun reloads the session and calls EnqueueChatTask. Archive inside that window and the cancel finds nothing, drops the binding and commits — then the flush enqueues onto a conversation the user closed. ClaimAgentTask does not read chat_session.status either, so the daemon runs it: runtime and quota spent on a closed conversation, assistant messages written into an archived chat, the agent flipped back to 'working', and with the binding already deleted the brief describes the run as a private web chat. EnqueueChatTask was the one enqueue path without the guard the direct-send path and OpenMikaOnboardingChat already take: lock the chat_session, re-read it, refuse a status that is not 'active' with ErrChatSessionArchived. It now takes it as its first statement, so the chat_session -> agent_task_queue order is unchanged. The lock is LockChatSessionForEnqueue (FOR NO KEY UPDATE), not the FOR UPDATE the send, delete and draft paths take, and the difference is deliberate. Those paths want to block INSERTs that reference the row: FOR UPDATE conflicts with the FOR KEY SHARE an FK insert takes on its parent, which is how LockChatSessionForDelete keeps a send from slipping a task in between its cancel and its delete. Here that conflict is the cost, not the point — every inbound channel message is an INSERT into chat_message FK'd to this row, so FOR UPDATE would make a room's next message wait for the previous message's enqueue. Measured on pg17 while the row is held: an append blocks under FOR UPDATE and does not under FOR NO KEY UPDATE, while SetChatSessionArchived's UPDATE (status is not a key column) blocks against FOR NO KEY UPDATE in both directions. Serialisation against the archive is kept, ingestion is not serialised behind it. flushChatRun needs no change: it clears the typing indicator before the error switch, and this error is not one of the two the switch names, so it falls to the default log branch and posts nothing into a room the archive just unbound. Three tests. The handler one is the case the existing archive tests cannot reach — message persisted, debounce not yet flushed, archive, then flush — asserting no task row and no task:queued; removing the status check fails it naming the run. The service one holds the lock mode from inside the enqueue transaction: a concurrent archive must block, a concurrent append must not. The router one pins that an archived-session flush clears typing and says nothing. Also carried over from review: sessionChannelType returned "" for every binding-lookup failure, so a transient database error answered 200 with the "not connected to a chat channel" note this PR removes — telling a WeCom or Lark session it is web-only because a query failed. It returns (string, error) now, with only pgx.ErrNoRows mapped to empty, and the caller reports the rest. And the bug's own description was wider than the bug: the outbound senders resolve their destination through the binding row the archive deletes, so a late answer has no route left and does not reach the room. The handler comment, the test file header and the failing assertion now say what actually happens — a wasted, mis-briefed run against a closed conversation. |
||
|
|
f3a7fce8d8 |
fix(chat): a predecessor's reply must not seal the next message out of its own batch (#6611)
On a self-hosted tenant a user sent a WeCom message at 18:31:17.7 while the
previous agent turn was finishing. The engine enqueued a task for it at
18:31:20.7 and cancelled the task 27ms later:
ERR chat claim: task-owned direct task has no user input; cancelling
task_id=bdc0947e chat_session_id=2fccd3c9 chat_input_task_id=bdc0947e
The database afterwards:
chat_message "挺好的 正常了" (18:31:17.726997) -> task_id = NULL
every other user message in that session -> task_id set
select count(*) from chat_message
where task_id='bdc0947e' and role='user' -> 0
The user's message was accepted, a task appeared and instantly vanished, and
no answer ever came. On WeCom the typing bubble also spun forever, because the
indicator only unsubscribed on task:failed, not task:cancelled.
Mechanism
Channel messages (Slack / Lark / WeCom) are written durably and unowned on the
ACK path; the 3s silence debouncer later fires one flush per session, and
EnqueueChatTask creates the task, stamps chat_input_task_id = id, and seals the
waiting messages into it with LinkUnownedChannelChatMessagesToTask. That seal
skipped any unowned user row that had a non-user row after it. The rule was
carried over from the pre-ownership trailing-message selector, and it does not
survive the debounce window: a predecessor completing inside that window writes
its reply AFTER the waiting message, so the reply to an already-sealed batch
became the boundary for a message it never saw.
18:31:04.6 user "测试下" unowned
18:31:07.6 task 6a4ae7f8 created, seals it
18:31:17.7 user "挺好的 正常了" unowned, 3s window arms
18:31:20.2 6a4ae7f8 completes, writes its reply <- newer than the message
18:31:20.7 window expires, task bdc0947e created, seal matches ZERO rows
18:31:20.7 claim finds no input, cancels bdc0947e
Every timestamp above is from the tenant's own tables. The claim guard at
internal/handler/daemon.go:2347 is right to fail closed on an empty input
batch; the batch should never have been empty. Its comment blamed a send path
that does not commit message and task together — SendDirectChatMessage does,
and the channel path does too. Neither was the problem. The seal predicate was.
Fix
The boundary is "already answered", not "something newer exists". A reply can
only be a reply TO a message if the turn that wrote it started AFTER the
message arrived. A turn already running when the message landed sealed its own
input before the message existed, so its reply — however late it lands —
answers an earlier batch and must not move the boundary. The seal now qualifies
each candidate boundary row by its turn's creation time, resolved through
COALESCE(chat_input_task_id, id) so an auto-retry clone is judged by the root
batch it inherited rather than by its own fresh id.
The check is kept, not dropped, and deliberately stays conservative: a non-user
row whose task is NULL or gone still counts as a boundary, so pre-ownership
transcripts are never swept into a new batch, and a message already stranded by
this bug is not resurrected into a turn minutes later — a following turn that
ran past it holds the boundary.
Tests
internal/handler/chat_channel_debounce_boundary_test.go drives the whole
failure: message in, claim, run, second message while it runs, predecessor
completes, window expires, claim. Before the fix:
--- FAIL: TestChannelChat_ReplyLandingInsideDebounceWindowStillAnswersTheNextMessage
chat_channel_debounce_boundary_test.go:134: claim = 500 {"error":"chat task has no user input"}
(task <nil>); task 2a67492b-cd61-45e6-afd6-156079971152 owns 0 user messages and is now "cancelled". The user's message was accepted and never answered.
internal/service/task_channel_seal_race_test.go pins the same boundary at the
seal, twice — the sequential ordering the tenant hit, and the tighter window
where the reply commits on another connection after the enqueue transaction
has begun (same raceInjectTxStarter the media-deferral race test uses), so
the seal statement's READ COMMITTED snapshot is the first to see it. Before
the fix:
--- FAIL: TestEnqueueChatTaskSealsMessageStrandedByPredecessorReply
task_channel_seal_race_test.go:78: waiting user message task_id = "", want the new turn "34b62bad-955c-46f0-99e9-2194999d2c10" — a predecessor reply that landed after the message must not seal it out
--- FAIL: TestEnqueueChatTaskSealsMessageWhenReplyCommitsDuringEnqueue
task_channel_seal_race_test.go:142: waiting user message task_id = "", want the new turn "989a83e4-e945-4b36-a5b7-69720449e08c"
The third, TestEnqueueChatTaskLeavesAlreadyPassedMessagesUnowned, passes both
before and after by design: it is the guard against over-correcting, and only
a fix that swept too much could break it.
The two transactions cannot interleave more finely than this: CompleteTask
takes chat_session FOR UPDATE and the task INSERT takes the FK's FOR KEY SHARE
on the same row. "Predecessor commits, then the flush runs" is the reachable
interleaving, and it is the one the tenant hit.
Claude-Session: https://claude.ai/code/session_01MKFf7wYM9Q2iaPtoFhG8MV
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
|
||
|
|
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. |
||
|
|
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 <xichangzhao@outlook.com> |
||
|
|
47f6e970f6 |
feat(onboarding): write Mika's opening on the server instead of running an agent (MUL-5827) (#6520)
* refactor(onboarding): drop the cross-workspace returning-member branch (MUL-5827) Mika's onboarding kickoff carried a `returning` flag, set by the frontend whenever the member was creating an additional workspace. It told the model "this member has onboarded elsewhere; keep the introduction to one line". Every workspace onboards from scratch. A member's second workspace may carry entirely different work, different collaborators, and a different reason for existing — compressing its opening on the strength of an unrelated workspace only makes this one worse. The flag was also the single place where one workspace's state reached into another's first conversation. Removed end to end: the request field, the prompt note, the API client type, the bootstrap input, and the `returning: isNewWorkspace` call site. Older desktop builds that still send `returning` are unaffected — encoding/json ignores the unknown field, which is exactly the new behavior. The built-in skill never branched on this, so SKILL.md needs no change. Co-authored-by: multica-agent <github@multica.ai> * feat(onboarding): write Mika's opening on the server instead of running an agent (MUL-5827) The first message a new member ever read cost a runtime cold start plus two model round trips: the opening was produced by a full chat task, so they watched a spinner for tens of seconds, and a run that failed introduced Mika as a red error bubble. Nothing in that reply needed an agent. The multica-onboarding skill already pinned it to four beats under a length budget, it reads no workspace state and calls no tool, and every input it personalizes on — language, workspace name, agent display name — is already in the request. It also never generated quick actions (starter cards replace them), so the run bought nothing. The endpoint now writes two rows in one transaction and enqueues nothing: - the opening the member reads, final on arrival, kind onboarding_opening so the starter cards still render under it; - the hidden kickoff, deliberately WITHOUT a task. The member's first real message adopts that kickoff into its input batch. That is what carries the onboarding skill instruction and the profile block into the run that does the first real work, and — because the kickoff quotes the opening verbatim — what stops Mika introducing herself a second time. She has no memory of an opening the server wrote. The cold start does not disappear; it moves to the member's first real request, where they are waiting on something they asked for with content and three one-click starter cards already in front of them. Three shared invariants that a two-row input batch newly exposes: - Both rows are written in one transaction, so DEFAULT now() gives them an identical timestamp. The session-list LATERAL picks the last message with no tiebreaker and ids are random UUIDs, so a tie could select the kickoff — whose kind makes buildChatLastMessage return nil, reporting no last message and bringing back the "Start with Mika" card after a perfect onboarding. The opening is written one microsecond after the kickoff. - DeleteUserChatMessageByTask deleted every user row of a cancelled turn, so cancelling the first message would destroy the only copy of the onboarding context and could hand the member the product's internal prompt as their restored draft. It now excludes the kickoff, and callers release it back to unowned so the next send re-adopts it. - Both reanchor queries would move the kickoff to dispatch time, putting the product's context AFTER the member's message inside one batch. Both now exclude it. The regression test fails without that guard. Also drops the completion-path rule that stamped a kickoff-input turn's reply as the opening: with the kickoff riding into the first real turn, it would render the starter cards a second time under a reply that is not an opening. Co-authored-by: multica-agent <github@multica.ai> * fix(onboarding): keep stamping the opening for a pre-deploy kickoff turn (MUL-5827) The server writes the opening now, so no new task produces one — but a kickoff task enqueued by the previous server can still be claimed by this one during a rolling deploy, and its reply IS that member's opening. Dropping the stamp outright left it a plain 'message', and message_kind is persisted with nothing to recompute it: that member's onboarding session would render without the starter cards forever. Gated on "the input batch is a kickoff and nothing else", which is exactly the old shape. The new kickoff always rides in alongside a real member message, so the member's first answer still cannot be mistaken for an opening. Deletable once no pre-deploy kickoff task can still be in flight. Co-authored-by: multica-agent <github@multica.ai> * fix(onboarding): release the kickoff when the first turn fails terminally (MUL-5827) Caught in a live environment, not by a test. The member's first turn died in preparation, the adopted kickoff stayed bound to that dead task, and their next message reached Mika with no onboarding context and no record that she had already greeted them — so she introduced herself a second time, in a conversation she had already opened. That is the exact failure this design exists to prevent. The cancel path already released the kickoff. The failure path did not, and a first-turn failure is entirely ordinary: runtime offline, agent CLI missing, timeout, prepare error. Released in FailTask under the existing retried == nil branch, which is the terminal-failure condition. Deliberately not on the retry path: a retry child inherits the root's chat_input_task_id and the kickoff stays bound to that root, so the retry still reads it — releasing there would strip the context off a turn that is about to run. Co-authored-by: multica-agent <github@multica.ai> * fix(onboarding): hand the kickoff to a queued successor, not to nobody (MUL-5827) Review catch. Adoption happens inside a send's transaction, so a message the member queues WHILE the kickoff's turn is still running finds nothing to adopt and never gets another chance. Releasing the kickoff to NULL when that turn died therefore left the already-sealed successor to execute with no onboarding skill, no profile block, and no record that Mika had already greeted them — the double introduction this design exists to prevent. Worse, a message sent later could adopt the orphan instead, delivering the context to a turn that had nothing to do with the opening. ReleaseOnboardingKickoffFromTask now re-targets the session's next un-started turn, falling back to NULL only when there is none. Three restrictions on the target, each load-bearing: - status = 'queued' only. A dispatched/running successor has already built its prompt from its input batch, so joining it now would consume the kickoff without ever delivering it; it waits unowned for the next send. - chat_input_task_id = id selects roots that own their input batch. A retry child names its root, and the kickoff is already reachable through that root, so retries must not be re-targeted. - regenerate_quick_actions_for IS NULL skips background suggestion passes, which carry no user input. Ordering matches the shared visible-head selector, so the kickoff lands on the turn the member will actually see run next. Covers both directions: A-owns/B-queued/A-fails hands off in the right order, and an already-dispatched successor is skipped. The first test fails against the previous NULL-only release. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Lambda <lambda@multica.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
|
318d51377f |
MUL-5870: fix(chat): hide channel issue commands from Chat
Hide shared channel /issue commands from public Chat projections while preserving issue creation, ordinary conversations, and cleanup paths.\n\nCloses #6571. |
||
|
|
d09019cd95 |
MUL-5839: preserve media in /issue descriptions (#6536)
* fix(channel): preserve images in issue descriptions * fix(views): hide channel-media provenance in board card previews descriptionPreview strips image Markdown but not HTML comments, so a `/issue` description whose media was materialized rendered its provenance marker as the card's visible preview text. cardProperties.description defaults to true, so this showed on the default board view for every issue created from a channel message carrying an image. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: J <j@multica.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
|
4900a4a85b |
MUL-5832: separate directory waits from working signals (#6553)
* fix(daemon): separate directory waits from working status Co-authored-by: multica-agent <github@multica.ai> * fix(status): address directory wait review feedback Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai> |
||
|
|
7a809ddf80 |
feat(channel): materialize inline media references (MUL-5835) (#6529)
Replace adapter-generated inline media placeholders in channel message bodies with authenticated attachment references, preserving original image positions. Closes #6528 |
||
|
|
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 <10457833+seacen@users.noreply.github.com> Follow-up: #6547 |
||
|
|
2d993a6a80 |
MUL-5823 fix(dashboard): count cancelled runs in usage run time (#6518)
* fix(dashboard): count cancelled runs in usage run time (MUL-5823)
CancelAgentTask accepts a task in 'running', so a cancelled row can carry
both started_at and completed_at — real agent occupancy. The run-time
rollups filtered on `status IN ('completed','failed')`, so every run the
user stopped mid-flight contributed 0 seconds and 0 to the task count.
The cost side has no status filter at all (UpsertTaskUsage and the hourly
rollup ignore status), so Cost/Tokens counted those runs while Time/Tasks
did not — two different task populations on the same dashboard, diverging
further the more runs get stopped.
Widen both run-time queries to include 'cancelled' and report it as a
third outcome alongside failed. The existing `started_at IS NOT NULL`
guard keeps a run cancelled while still queued out: it never occupied an
agent. The failure rollups keep the two-status filter on purpose — a
manual stop is not a failure and must not dilute the error rate.
Migrations 261/262 swap the supporting partial index to a predicate that
covers the third status; without that the widened filter can no longer
use it and the rollups fall back to a full table scan.
Co-authored-by: multica-agent <github@multica.ai>
* fix(migrate): guard the 261/262 index swap against an interrupted build
An interrupted CREATE INDEX CONCURRENTLY leaves an INVALID index behind.
`IF NOT EXISTS` then reports success on retry without rebuilding it, the
runner records 261 as applied, and 262 drops the still-valid v1 — leaving
every dashboard rollup on a full table scan.
Register cleanupInvalidConcurrentIndexHook for 261, the same guard
migration 257 already uses for the same hazard.
The down path needs different handling: hooks only run in the `up`
direction, so 262.down drops IF NOT EXISTS and fails closed instead —
matching what 258.down does for the 257/258 pair.
Adds a regression test covering both. Unlike 257's unique index this one
cannot be failed with a duplicate row, so the build is interrupted the way
a real one is: a concurrent open transaction blocks the wait phase until
statement_timeout cancels it. The test asserts the bare retry is a silent
no-op, that the hook repairs it, and that 262 only drops v1 once v2 is
valid.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
|
||
|
|
3c9b861de3 |
MUL-5765 feat(chat): starter cards under Mika's onboarding opening (#6505)
* MUL-5765 feat(chat): starter cards under Mika's onboarding opening Replace the LLM quick-action chips on Mika's onboarding opening with three product-fixed starter cards (board / delegate / digest), so a new member never starts from a blank canvas. Clicking a card sends its fixed prompt as a visible member message through the existing quick-action path; cards follow the chips' disabled rule while a task runs and stay clickable in history. Older clients keep the LLM chips untouched. The multica-onboarding skill gains matching starter plays (one-question budget each) and a digest-only exception to the no-autopilot rule. Co-authored-by: multica-agent <github@multica.ai> (cherry picked from commit |
||
|
|
c3577cb04b |
feat(dingtalk): add DingTalk bot integration (MUL-3958) (#4829)
Adds a DingTalk (钉钉) bot integration on the bring-your-own-app model: a workspace admin creates their own Stream-mode robot and pastes its AppKey / AppSecret, so no public webhook or OAuth redirect is required. Each agent gets its own bot identity, so several agents can be distinct, separately @-mentionable contacts in one DingTalk organization. Supports DMs, @-mentions in groups, inbound images, /issue quick-create, and /new. Built on the shared channel engine (ForceFresh/BareFresh, MediaResolver / MediaRef and the intent ledger) rather than a private implementation. Off unless MULTICA_DINGTALK_SECRET_KEY is set. Docs in en/zh/ja/ko. Closes #4791. Community-maintained: @yyclaw is the code owner for server/internal/integrations/dingtalk/. |
||
|
|
5a618e0925 |
fix(agents): stop resuming a session that can't resolve its provider auth (MUL-5803) (#6482)
* fix(agents): stop resuming a session that can't resolve its provider auth A Hermes agent on a self-hosted install can get permanently stuck on a single issue while every other issue on the same agent keeps working. The task terminates with: hermes provider error: "Could not resolve authentication method. Expected either api_key or auth_token to be set. Or for one of the X-Api-Key or Authorization headers to be explicitly omitted" and no amount of retry / Rerun recovers it. Root cause (resume-pointer poisoning): every daemon version classifies this text as agent_error.unknown, which is resume-safe. So GetLastTaskSession / GetLastChatTaskSession keep returning the same failed session to every retry and Rerun on that (issue, agent) pair, deterministically reproducing the auth error forever. Other issues use fresh sessions, so they're unaffected — the codebase's repeated "(agent, issue) permanently stuck" pattern (GH #6066 / #5760 / #6360, MUL-5722), with this error falling through every prior defense. The fix rests entirely on text guards; the classifier is deliberately left untouched. Reclassifying this under missing_config would flip freshSessionMayHelp to false and silently disable the in-turn fresh-session retry on the five ResumeRejectionUndetectable backends — contradicting the (correct) diagnosis that a fresh session cures it: - service/task.go ResumeUnsafeFailure: text guard so manual Rerun and the fallback claim path start fresh rather than replaying the dead session. - GetLastTaskSession / GetLastChatTaskSession SQL: ILIKE exclusion so already-wedged issues recover on their next trigger without a daemon upgrade. Tests pin both halves: a freshSessionMayHelp regression (must stay true for this error), the Go ResumeUnsafeFailure cases, and SQL exclusion + narrowness regressions for both query families. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: address PR review nits Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: qiushihao279-cloud <301943329+qiushihao279-cloud@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai> |
||
|
|
b99b04bb86 |
feat(onboarding): Mika issue-first onboarding (#6378)
* feat(onboarding): Mika issue-first onboarding Replaces the starter-agent welcome with one conversation: onboarding creates Mika, the workspace's built-in Chief of Staff, and opens a real chat whose first turn is a product-authored kickoff hidden from the transcript. Every workspace — first and subsequent — is created through this flow. Mika is a system agent, not an agent-template instance. Her product prompt is //go:embed-ed and composed at claim time, so a release updates it without touching any workspace's row; the row holds only the workspace's own notes. Creation is server-owned and idempotent under a per-workspace advisory lock, and archiving a system agent is rejected. This is the pre-merge half of the branch, squashed while rebasing onto main. Replaying its fifteen commits individually meant re-deriving each one against a main they were never written for; the net change reconciles against today's main in four files, so it is reconciled once, here. Three of those four are main moving under the branch: MUL-5573 took quick-actions generation server-side and dropped QuickActionsDisabled / RegenerateQuickActionsFor from the task payload and the SendDirectChatMessage signature, so this takes main's shape and keeps only the onboarding entry point. The fourth keeps main's OnboardingLogoutButton wrapper around the flow's new mode/onCancel props. Co-authored-by: multica-agent <github@multica.ai> * feat(onboarding): let quick-action chips carry the opening's examples Chat now renders agent-suggested follow-up actions as buttons under a reply (MUL-5149), and the onboarding kickoff qualifies for that suggestion pass with no extra wiring — it is a direct chat turn on a web session with a non-empty reply. That made the opening's fourth beat redundant: Mika wrote a three-to-five line menu of example tasks, then three chips appeared underneath offering the same thing. The prose menu is the worse half — a member has to retype a line they read, but can send a button — so the beat is gone and the opening budget drops with it. Measured on a real local run: the first reply went from 307 to 208 characters, and the chips still arrived 21s after the kickoff. The questionnaire profile stays in the kickoff. The suggestion pass resumes the same provider session, so the profile now steers the chips as well as the reply; only the sentence naming its purpose changes. Co-authored-by: multica-agent <github@multica.ai> * refactor(onboarding): cut the step rails down to what the screen can't already show The right rail carried more than twice the words of the column members were actually working in — 42 vs 36 on the workspace step, 91 vs 34 on the runtime step — and it got heavier the further in the flow went, which is backwards. Step 3's rail was the clearest case. Its "Good to know" section promised the runtime was swappable and that more could be added later; the step's own lede already said both in nine words rather than forty-two, on the same screen. Its 60-word definition of "agent runtime" sat beside a list of named, online runtimes under a headline reading "This computer is connected" — by then it answers a question the member has stopped asking. What survives is the one thing the screen does not show: what that background process is. Step 2's rail keeps the workspace preview card, which does show something not otherwise visible, and drops the bullet lists — promises the product is about to keep on its own. The freed words did not move to the rail; one moved into the main column. On the create path "Mika" was never introduced before Step 3 used the name twice, once on the primary button, so the lede now names the role in an appositive right above that button. Mika stays ungendered, as everywhere else in the product. Net across the three regions: 167 words to 89. Co-authored-by: multica-agent <github@multica.ai> * refactor(onboarding): drop the right rail from the workspace and runtime steps Every remaining rail item was either something the screen already showed or something the product was about to do anyway, so the column was costing a member's attention without answering a question they had. Removing it leaves each step a single full-width column — the shape the questionnaire step has always had, and the only step nobody has complained reads as sparse. Gone with it: RuntimeAsidePanel, the workspace preview card and its entity rows, and 27 copy keys per locale. The two runtime paths (desktop runtime-connect, web platform-fork) shared that panel, so both lose it in one move and stay identical. The welcome step keeps its column — it holds an illustration, not prose. Co-authored-by: multica-agent <github@multica.ai> * fix(onboarding): put every step's header and content on one measured axis Removing the right rail left the four steps geometrically inconsistent in four ways, all of which the member sees as things moving while they advance. The header carried the horizontal padding itself, so it ran flush to the window edge while the content column stayed centred. A 480px rail had been absorbing that difference; without it the two were 267px apart on a 1283px window, and since StepHeader is justify-between the step indicator floated off at the far right, ~270px from anything it labelled. The rest compounded it: the measure changed between steps (920px on the questionnaire, 620px after), the header bar and content block used different vertical padding per step, and padding living inside a max-w box made the reading width jump from 508px to 620px at the lg breakpoint. All four now come from step-shell.tsx. Padding belongs to the gutter, never to a measured box, so the reading width is constant from ~700px up. The header measures on STEP_FRAME on every step, so the one element that survives each transition never moves; content picks STEP_FRAME or STEP_COLUMN by what it holds, and both centre, so a step that needs the width still sits on the header's centreline. Vertical rhythm is one value. The header was near-identical in four files, which is how it drifted in the first place, so it is now one component. Its test pins the invariant that broke — padding out of the measured box, header measured on the frame — and fails if either is put back. Co-authored-by: multica-agent <github@multica.ai> * fix(onboarding): give the runtime step the frame measure and cut its copy Nine runtimes in a 620px column meant truncated names, five rows, and scrolling to reach the last four, with ~330px of dead space on each side. This is the case step-shell's wider measure exists for, so the step takes STEP_FRAME — which also puts it on exactly the header's measure — and the card grid gains a third column at lg. Nine runtimes now land in three rows. Copy went with it. The headline was two sentences over two lines; the first one, "This computer is connected", is already said louder by the "9 agent runtimes · all online" row directly beneath it, so only the instruction remains. The lede was 44 words and five lines — I had grown it myself adding Mika's introduction — and is 20 now, still naming the role. The remote-computer note drops from 25 words to 15. Prose stays capped at 620px inside the wider frame; a 920px measure is for the card grid, not for reading. The found-phase test keyed on the headline copy, so a copy edit read as a behaviour regression. It now asserts the runtime count row, which is the signal the test is actually about. Co-authored-by: multica-agent <github@multica.ai> * refactor(onboarding): put the workspace step on the shared frame It was the only step still measuring at STEP_COLUMN, so its eyebrow, headline and footer CTA started and ended ~150px inside where every other step's did — the page margins visibly moved when you advanced from step 1 to step 2. The step now sits on STEP_FRAME like the other two, with a new STEP_MEASURE capping the prose and the form inside it. Matching the frame is not the same as widening the field: a workspace name does not want an 800px input, so the form keeps its reading measure and left-aligns to the frame instead. The footer row spans the frame, which is what puts the CTA in the same place on all three screens. STEP_MEASURE deliberately does not centre — centring would pull the content off the frame's left edge, undoing the alignment. Its test pins that. Co-authored-by: multica-agent <github@multica.ai> * fix(onboarding): close the gap above the step CTA and put Log out back on one row Two defects that read as one thing on screen: a large empty block sitting directly above the primary button. The card list was capped at STEP_MEASURE along with the form. That cap exists so a workspace name does not get an 800px input — a good reason for a text field and a bad one for selection cards, which are a list like the runtime grid. Capped, they stopped 299px short of the CTA, leaving a void above the button. The cards take the frame now; only the form keeps the reading measure. Log out came in from main as `fixed right-8 top-8`, pinned to the window corner. Its own comment says the fixed position exists to survive the flow's full-bleed layouts — which is what the measured frame replaced, so it landed outside the measure and above Back / Step N of N as a second header row. It now rides the header row on the frame. StepShellHeader takes it as a `trailing` slot rather than rendering it: calling useLogout inside the shared header forced a QueryClient into five step test files just to render a header bar. The flow injects it, matching how runtimeInstructions is already threaded, and the header stays presentational. Co-authored-by: multica-agent <github@multica.ai> * feat(onboarding): make step 3 about Mika, with the runtime as a sub-decision The step was titled after its dependency — "Pick an agent runtime" — while the thing actually being created was named only in the grey lede. A member reached a button reading "Start with Mika" without having been told who that is, and said so: being confused there is not a failure to read carefully, it is the page putting the lead role in a footnote. So the subject changes rather than the step count. The headline names the outcome, a card carries the introduction with a mark on it, and the runtime list drops to a labelled sub-section under "Where should Mika run?". Reading order becomes: who you are getting, she needs a machine, pick one, start. MikaIntro sits above the phase switch so the subject holds still while the runtime block below cycles through scanning / found / empty, and each phase's own heading drops from h1 to h2 now that the page has a real h1. Mika does not exist yet at this point — she is created on commit — so the card cannot render her stored avatar. It reuses the mark the Runtimes page already uses for "Start with Mika", so the two entry points read as the same thing. No fourth screen: the introduction and the only decision on this screen are one beat, and splitting them would add a step between finishing setup and the payoff. The defect was never a missing screen, it was an invisible introduction. Co-authored-by: multica-agent <github@multica.ai> * feat(onboarding): pick the runtime and the model from dropdowns Nine runtimes as cards took three rows and left nowhere to put a second decision. Two dropdowns fit both on one screen, and the model is a real choice the card grid had no room for. Both controls already existed on the agents surface — RuntimePicker and ModelDropdown, the same pair create-agent-dialog uses — so this reuses them rather than growing a parallel picker. ModelDropdown owns its own discovery, grouping and unsupported-runtime states, and selecting a different runtime clears the model because models are per-runtime. The model now reaches the agent: POST /api/agents/mika takes an optional model, CreateSystemUserAgent writes it to the column the agent table already had, and empty still means "whatever the runtime defaults to" — which is what every deployment without per-agent model support gets anyway. Also drops the "No local runtime, or prefer a remote computer?" note. It said in twenty-five words what the Skip button next to it says by existing, and it appeared on all three phases. currentUserId comes in as a prop rather than from the auth store: reading the store inside the step broke six tests that render it without one, the same coupling the header slot avoided. Co-authored-by: multica-agent <github@multica.ai> * fix(onboarding): stop gendering Mika in the intro card Mika is ungendered everywhere else in the product — the agent instructions, the onboarding skill and every other locale string avoid a pronoun. The intro card I added last round reintroduced one in two languages ("she turns it into an issue" / "她会把它变成一个 issue"), which the Chinese screenshot made obvious. All four locales now read around it. Co-authored-by: multica-agent <github@multica.ai> * feat(ui): port the ReUI stepper primitive Groundwork for rebuilding onboarding on the @reui/onboarding-3 interaction model: a persistent vertical stepper with named steps and click-to-return navigation, which our horizontal "Step N of 3" dots cannot express. Routed to components/ui rather than the vendor's components/reui namespace — it is our code now — and rewritten to the role-named type scale (text-xs -> text-caption, text-sm -> text-label / text-caption, dropping the leading-none the token already supplies). No other convention fixes were needed: shadcn had already rewritten the imports, "use client" survived because main's components.json now sets rsc: true, and it pulls no npm dependency we do not already have (@base-ui/react is declared). The block's other twelve registry dependencies are primitives we already own, so only this one was installed. Co-authored-by: multica-agent <github@multica.ai> * feat(onboarding): rebuild the step shell as a named progress rail Onboarding's chrome was a row of dots and a "Step 2 of 3" counter. It told a member how much was left but never what was coming, so every step arrived unannounced -- the same reason the runtime step read as a surprise even after it was retitled "Meet Mika". Naming all three steps up front makes onboarding legible as a whole before the first field is filled in. StepShellHeader becomes StepShell, and it owns the window rather than a strip at the top: rail on the left, the step's own content scrolling on the right. That lets the four steps drop an identical wrapper/DragStrip/header/<main> preamble, including the scroll-fade wiring that was duplicated verbatim in all four and is now set up once. The rail is built on the ported ReUI stepper, but deliberately not as a tablist: that component defaults to role=tablist with each trigger owning aria-controls on a panel id, which is right for a stepper that renders its own panels and wrong here, where the panel is a routed step and those ids would dangle. It uses the presentational slots and marks position with aria-current instead. Only completed steps are clickable -- moving forward has to run the current step's validation and submit, so the rail would skip it. New-workspace mode gets no rail navigation at all: it enters at the workspace step and, once that workspace exists, every step behind it is gone. Same invariant runtimeStepBack already enforced for the Back button. step_header.step_of is replaced by step_nav across all four locales; Mika stays ungendered in each. Co-authored-by: multica-agent <github@multica.ai> * feat(onboarding): standardise the steps on the shared UI primitives Follow-up to the rail, fixing what the rail exposed. The workspace form was three hand-rolled `flex flex-col gap-1.5` stacks with their own label sizing and a bare <p> for the slug error -- exactly what Field/FieldLabel/FieldError/FieldDescription standardise. The manual version had already drifted: its labels were caption-sized and muted while every other form in the product labels at body weight. The platform fork wrapped on STEP_COLUMN while the steps before it wrapped on STEP_FRAME. Both measures centre, so 620px and 920px put their left edges ~150px apart and the headline visibly jumped right on arrival. It now sits on the frame and caps its own content, which is the pattern step-shell already documents. The About you eyebrow read "About you" -- now the rail's label for that very step, so the page said its own name twice, once in grey caps and once in the headline under it. Dropped, along with the locale key. The other steps keep theirs because they say something the rail doesn't ("Connect a computer", "Workspace creation is disabled"). Log out is `inline` on every step, and inline now means "on the rail", which is an inverted surface -- the muted/destructive pair it used unqualified is mixed from the light palette, so it was rendering as near-invisible grey on black. e2e: the smoke spec asserted "Step 1 of 3", text the rail replaced. It now asserts the rail's named steps and which one is aria-current, and carries on into the runtime step so all three get captured. Two pre-existing bugs in that spec surfaced while fixing it: the zh-Hans case pinned its locale cookie to a hardcoded port, and it advanced with getByRole("button").first(), which is the pinned Log out button -- so that case had been signing the user out and asserting against a login redirect. Co-authored-by: multica-agent <github@multica.ai> * feat(onboarding): rebuild the rail as the ReUI inset panel The first pass kept the block's idea -- a named vertical rail -- and dropped most of its structure. Side by side with onboarding-3 the gap was obvious: no brand lockup, no inset panel, no texture, steps jammed against the top edge, numbered chips instead of the ring/check/dot progression, stub separators instead of one continuous track, and a bare Log out where the block has a footer row. This ports the structure properly: - Inset panel -- the aside carries the padding and the panel is rounded with a hairline ring, instead of a dark rectangle bleeding to the window edge. - Brand lockup top-left (the existing MulticaIcon plus a wordmark), Back demoted to an icon button top-right, which is where the block puts it. - Step list centred in the remaining height rather than stacked under Back. - Indicators follow the block: filled + check when done, ring + dot when current, faint ring when upcoming. Numbers move to sr-only text, since the ring already encodes position and the digit was redundant next to a label. - One continuous hairline behind each row instead of a stub between rows. - DotSphere ported to packages/ui as the panel's texture. It is decorative, reads no product state, and already honours prefers-reduced-motion. Two fixes fell out of doing it properly: `.dark` is a plain class selector in our token sheet, so scoping it to the panel redefines the custom properties for that subtree and `bg-background` / `text-muted-foreground` mean the right thing inside it. That replaces the hand-mixed `text-background/60` shades of the first pass -- which is what had made Log out near-invisible -- and it is why the button is back on ordinary tokens here. StepperSeparator hardcodes a 3rem height for vertical navs, so an absolutely positioned track overshot its row and drew the line straight through the next indicator. Overridden with the same variant rather than !important, so tailwind-merge drops theirs. DotSphere cycles three constant arrays by `index % length`. Under noUncheckedIndexedAccess that is `T | undefined`, so they are typed as non-empty tuples and index 0 is the fallback -- no non-null assertions. Co-authored-by: multica-agent <github@multica.ai> * feat(onboarding): put the content pane on the block's type and layout The rail matched onboarding-3 and the pane it sat next to did not, so the two halves read as different products: serif display headlines and a grey uppercase eyebrow on the right, the block's sans hierarchy on the left. Typography now maps onto the block exactly, and it lands on our scale without rounding -- ReUI's `text-xl/7` heading is our `text-title-lg` (20/28) and its `text-sm/5` supporting line is our `text-body` (14/20). StepHeading owns both, so no step hand-rolls a headline again. The eyebrows are gone. The block has no such slot, and with the rail naming every step, a grey uppercase label above a headline saying the same thing was the third name for one screen. The workspace step's disabled-state wording was the only eyebrow carrying information the headline lacked, and that variant already exists as its own headline copy. Geometry collapses from three competing measures -- a 920px frame, a 620px column, an in-frame cap -- to one 28rem column. Three measures is what let the platform fork sit ~150px right of every other step. STEP_MEASURE stays for capping a single control inside the column. Actions move into StepFooter: full-width, stacked, pinned to the bottom of the column. In a 28rem column the old right-aligned inline bar left the primary action floating mid-screen instead of where the eye finishes the form. The column is `min-h-full` rather than centred by the pane, because `items-center` on a scroll container clips the top of anything taller than the viewport and these steps do overflow on short windows. Questionnaire options become the block's wrapping chips. They were full-width cards in a 4-column grid; inside a 28rem column that grid had nowhere to go, and stacking all 18 as rows turned one screen into a long scroll. Chips are the block's own answer for a many-option question. This also reaches the workspace source-backfill prompt, which shares the component -- intentionally, it is the same question in the same style. MikaIntro moves onto StepHeading + Item for the same reason. Co-authored-by: multica-agent <github@multica.ai> * fix(onboarding): drop the rail clear of the macOS traffic lights The rail is a dark inset panel and it started at the window's top-left corner, which is exactly where macOS draws the traffic lights -- so the close/minimise/zoom buttons sat on top of the dark surface. The underlying mistake was structural. The desktop shell rule asks for a single DragStrip as the first flex child of a full-window view; this had two hand-rolled strips instead, one inside each pane, and neither was first. The sidebar's was 28px of internal padding trying to duck under the traffic lights from inside a panel that had already begun above them, which cannot work -- the panel's own background was the thing being overlapped. One DragStrip now spans the window above both panes. The panel starts below it (48px strip + the aside's inset, measured at 64px on a wide window against traffic lights that end around 32px), the whole band stays draggable, and the two ad-hoc strips are gone. Co-authored-by: multica-agent <github@multica.ai> * fix(onboarding): make every block in the column share one width Reported on the workspace step: the name field ended short of the description above it. Measured rather than eyeballed -- the heading, description and footer all render at 448px, the form at 384px, because the FieldGroup still carried STEP_MEASURE. STEP_MEASURE made sense against the old 920px frame, where a full-width input would have been absurd. Against a 28rem column it does nothing but misalign, so both remaining uses are gone -- the workspace form and the runtime picker -- and the constant with them. A single measure that no step can locally narrow is the whole point of the column; leaving the knob exported invites the same drift back. Two stale measures went with it. The runtime phase views still capped their ledes at max-w-[620px], inert inside a 448px column, and sized them text-body-lg against StepHeading's text-body; their h2 was text-title-lg, the same size as the h1 above it. Both now match the shared scale. Guarded in e2e rather than a unit test. The shell test renders a stub child, so asserting "no narrow cap inside the column" there would pass whatever the real steps do. The new spec walks all three steps and compares rendered geometry -- it fails on the reported bug and passes after the fix, which is what makes it worth keeping. Co-authored-by: multica-agent <github@multica.ai> * fix(onboarding): stop the whole window re-fading on every step change Every step rendered its own <StepShell>. Because each step is a different component type, React tore the shell down and built a new one on each transition: the "persistent" rail remounted, its canvas restarted, and the shell replayed `animate-onboarding-enter` -- a 0.4s fade from opacity 0 across the entire window. That full-window re-fade is the flash. Measured before changing anything: tagging the live <aside> and switching steps showed the attribute gone, and the shell root still reporting animation-name `onboarding-enter` after the switch. The upstream block has no step-change motion at all -- no animate-*, no transitions, no framer-motion, no AnimatePresence. Its sidebar and section live in one component and only the step body swaps. This does the same: the flow owns a single StepShell and the steps render content. The shell's entrance fade now runs once, on entering onboarding, which is what it was for. `backDisabled` was the one thing blocking the hoist -- the shell needs it but only the workspace step knows the create request is in flight. It reports upward through `onBusyChange`, with an unmount cleanup: a successful create advances immediately, so without clearing the flag the next step would open with Back and the rail dead. Guarded in e2e by tagging the rail and content nodes and asserting they survive a step change, plus a single DotSphere canvas rather than one per visited step. Co-authored-by: multica-agent <github@multica.ai> * chore: drop a stray commit-message file Co-authored-by: multica-agent <github@multica.ai> * i18n(zh): retranslate onboarding and fix cross-file inconsistencies Reported as "生硬" on the onboarding Chinese. Reviewed all 217 onboarding strings against the Chinese voice guide in conventions.mdx, then swept the other 24 locale files for the same classes of problem. Three were mistranslations, not stiffness. The worst: an agent runtime was described as the AI coding tool "我们接管的" — take over — where the English says "we connect to". Also a "推荐" that exists in no source string, and "12 calls" (user interviews) rendered as "12 通电话". Punctuation, decided from the repo's own zh docs rather than guessed: 「」 is forbidden by the guide and appeared 16 times; 破折号 spacing was split 26 spaced vs the rest unspaced, and the docs run 152 unspaced to 35, so unspaced wins. Four strings had a stray space inside Chinese text ("正在跳转到 工作区"). 67 English strings had two or more Chinese translations. Most are legitimate — weekday pickers use single characters where labels spell them out, 飞书/Lark are deliberate regional variants, and "Name" is 姓名 for a person and 名称 for an object. 42 were arbitrary and are now unified. ja/ko got the same consistency pass on the clear-cut cases only, kept parallel with the zh choices. Their punctuation was deliberately left alone: 「」 is standard Japanese quoting, so the zh rule does not transfer. Edits are applied to the raw file text rather than through a JSON round-trip, which reformatted compact one-line objects and turned ~20 real edits into a 97-line diff. Co-authored-by: multica-agent <github@multica.ai> * chore: drop a stray commit-message file Co-authored-by: multica-agent <github@multica.ai> * feat(mika): use the unicorn emoji as Mika's placeholder avatar Mika shipped with a hand-rolled data-URI SVG — a sparkle glyph on a dark rounded square — which only that one constant knew how to produce. Agents already have an emoji avatar convention (`emoji:` marker in avatar_url, owned by agentEmojiAvatarPrefix on the server and parseAvatarEmoji on the client), so this reuses it instead: ActorAvatar renders the emoji as text and no surface needs to special-case her. The two cards that stand in for Mika before the agent row exists move with it — the onboarding intro card and the Runtimes "Start with Mika" card. Both were drawing the same sparkle mark, and leaving them would mean a member sees one face during onboarding and a different one the moment Mika is created. They now share MIKA_PLACEHOLDER_EMOJI, which carries a pointer to the server constant so the two cannot drift apart silently. The dark square went with the sparkle: it was built to frame a white line-art glyph, and an emoji on it reads badly. These use bg-muted, matching how ActorAvatar already frames an emoji avatar. Placeholder until Mika has real artwork. Co-authored-by: multica-agent <github@multica.ai> * i18n: stop the skip path promising things it does not do Two claims in the runtime-skip copy that the code does not back, both found while walking the flow end to end. "Enter your workspace in read-only mode" — there is no workspace read-only concept on the server. The only READ ONLY in handlers is Postgres transaction isolation on the issue-table and search queries. What actually exists is admission.go's ReasonAgentRuntimeRequired, which blocks dispatch when no runtime is connected. So the sentence's second half was already true and enforced; the first half promised a restriction that does not exist — a member who skips can create issues, comment, edit fields and invite people exactly as normal. Same over-claim in cloud_waitlist.intro_warning. "We've added one task" — the skip path creates an *issue* (it lands on the Issues board as NOR-1), and task/issue are distinct entities in this product. Now says issue. All four locales. Co-authored-by: multica-agent <github@multica.ai> * i18n: reframe Mika as your first agent teammate Requested copy change on the step 3 headline. The rail's description for that step moves with it. It is the step's own subtitle and sits on the same screen as the headline, so leaving it saying "Your Chief of Staff" would have put two different framings of Mika side by side on one page. Three "Chief of Staff" references are deliberately untouched, because dropping the title everywhere is a positioning call rather than a copy fix: the role chip on the intro card (name + title reads fine under the new headline), and the web fork's lede, which is a different screen. Chinese follows the glossary (Agent -> 智能体) rather than the mixed-language phrasing in the request. Co-authored-by: multica-agent <github@multica.ai> * fix(runtimes): let the member pick the runtime before Mika is created "Start with Mika" provisioned immediately on `runtimes.find(online) ?? runtimes[0]`. One machine commonly registers every agent CLI installed on it — nine on the box this was reported from — so "the first online one" is an arbitrary pick, and Mika could end up bound to a CLI the member never intended to run their Chief of Staff on. Rebinding after the fact is more work than choosing up front. The action now opens a dialog with the same two controls onboarding already uses for this decision, RuntimePicker and ModelDropdown, so the same choice reached from a different entry point is asked the same way. The old heuristic survives only as the dialog's initial selection, which makes it visible and changeable instead of silent. Model resets when the runtime changes, because models are per-runtime and a value picked for the previous one may not exist on the next. Co-authored-by: multica-agent <github@multica.ai> * refactor(runtimes): one component for "which runtime should Mika use" Three surfaces asked this same question and had drifted apart: desktop onboarding and the Runtimes page offered a runtime plus a model, while the web CLI dialog offered only a runtime. `step-platform-fork` called `onNext(picker.selected)` with no second argument, so connecting through the web terminal path silently created Mika on whatever model the runtime defaulted to, while the desktop step let you choose. Two of the three also re-implemented "changing the runtime clears the model"; the third simply lacked it. MikaRuntimeChoice now owns the pair and that reset rule, so no caller can forget it and the three entry points cannot drift again. The web CLI path gains model selection, which is the behaviour change here. `layout` is a prop rather than a single unified presentation because the difference is real: the CLI dialog lists machines because that is the moment they appear one at a time after `multica setup`, and a collapsed dropdown hides exactly the feedback that dialog exists to give. Everything below the list is identical. compact-runtime-row moves from onboarding/ to runtimes/ so imports only flow onboarding -> runtimes rather than both ways. Creation is deliberately left alone. A and B still funnel through `handleRuntimeNext`, which also runs saveQuestionnaire, completeOnboarding and onComplete; the Runtimes page must not do any of that, since that member is already onboarded. Merging those would leak onboarding completion into a non-onboarding surface. The platform-fork test now needs a QueryClientProvider, because the dialog renders a model dropdown that queries the runtime's model list, and its onNext assertion moves to the two-argument signature. Co-authored-by: multica-agent <github@multica.ai> * fix(onboarding): stop the download card claiming a result it cannot know Clicking "Use this computer" set `downloaded` unconditionally, which flipped the card to "Opening the download page..." / "Opened in a new tab." Neither is knowable: `window.open` is called with `noopener`, and per spec that returns null whether the tab opened or a popup blocker ate it, so a blocked click still produced a card asserting a tab had opened. The same flag was also write-once, so the transient "Opening..." became a terminal state — come back to the tab later and it still says the page is opening. And because the swapped title wraps to two lines, the card grew 10px and pushed the two cards under it down, which is what made the click read as a page refresh in the first report. The card now states its intent up front — "Opens in a new tab — pick your platform there" — which is true before the click, after it, and when the popup never appears. The state, both `_after` strings and `hint_downloaded` are gone from all four locales. Its test asserted the flip, so it now asserts the opposite: the mocked window.open returns null (the blocked case) and the card must be unchanged. Measured after the change: primary card 330px before and after, and the cards below it do not move. Co-authored-by: multica-agent <github@multica.ai> * fix(desktop): stop one worktree in a thousand booting into a blank window (#6436) Worktree renderer ports are `5174 + cksum(path) % 1000`, a 5174-6173 window that contains exactly one port Chromium refuses to navigate to: 6000, the X11 port on its restricted list. A worktree whose path hashes to offset 826 gets a healthy Vite server on 6000 and an Electron window that fails the load with ERR_UNSAFE_PORT -- so it reads as a renderer bug, not a port one, and the only way out was setting DESKTOP_RENDERER_PORT by hand. Restricted ports in the window are now remapped into the block immediately above it (6000 -> 6174). Sending them past the end rather than shifting them by one keeps the offset -> port mapping injective, so two worktrees still cannot land on the same port and race for it. Co-authored-by: Lambda <lambda@multica.ai> Co-authored-by: multica-agent <github@multica.ai> * fix(onboarding): close the Mika multi-member and failure-recovery gaps From Emacs's review of #6378. All seven findings reproduced. The two blocking server bugs were the same shape: Mika is one agent per workspace but sessions and ownership are per member. - StartMikaOnboarding required the agent owner, so the member who lost the CreateMikaAgent race — the race that handler's advisory lock exists to survive — got a valid Mika, opened a valid session, and then a 403. Mika is created workspace-visible and workspace-invocable; the session gate and canInvokeAgent were already the checks that matter. - The onboarding session was resolved client-side by listing sessions and creating one on a miss, matched on the localized title. LockWorkspaceForChatSessionCreate is FOR KEY SHARE precisely so concurrent creators do not block, so two tabs each opened their own conversation with its own kickoff, and switching language between a failed attempt and its retry opened another. It is now get-or-create server-side under a per-(workspace, member) advisory lock, keyed on (workspace, creator, agent), returned alongside the agent. Also: - The skipped-runtime welcome dismissed itself silently when provisioning the guide issue failed. The signal is not persisted and onboarding is already complete, so a blip was terminal. It now offers a retry. - The Runtimes recovery card gated on `agents.length === 0`, so creating any ordinary agent hid the only surface that can mint a Mika — the generic endpoint accepts no system_key. Gated on Mika's absence. - CompactRuntimeRow ignored `disabled`; the CLI dialog was already passing it, so the runtime could change mid-submit. It is a real <button> now, which also gets focus and Enter/Space for free. - The rail never went below 15rem while the content pane kept its gutter, leaving ~87px of form at 375px. It is hidden under md, where a compact bar carries the step name and the Back button instead. - Dropped a stray __pycache__ artifact I had committed by accident. Each new test was checked against the bug it covers: reinstating the owner gate, the title-keyed lookup, or the dropped disabled prop makes the corresponding test fail. Co-authored-by: multica-agent <github@multica.ai> * fix(onboarding): make the Mika entrypoint survive a partial bootstrap From Emacs's second review of #6378. Bootstrapping Mika is three server steps — provision the agent, open the member's session, enqueue the opening turn — and the last two can fail after the agent commits. Two things then conspired: the server reported success with the session omitted, and the Runtimes card gated on the agent existing. The agent's own `agent:created` broadcast invalidates the agent list, so the card (and its open dialog) was torn down the instant step one succeeded, and never came back on reload — the agent is durable, the rest was not. The member was left holding a Mika they could not start. - The endpoint now fails when the session cannot be resolved. Every step is idempotent, so a retry converges; handing back a half-built flow the caller cannot distinguish from a finished one does not. - The entrypoint is gated on the member's own state — does this member have a Mika conversation that was actually kicked off — rather than on the workspace having an agent. That is the question the card answers, and it is true again for every partial state above. Also restores the Log out escape hatch below `md`. Hiding the rail last round took its footer with it, which stranded every step but Welcome with no way out on a narrow screen; the compact bar now renders the same slot, so `sidebarFooter` is `chromeFooter`. Each new test was checked against its bug: the old agent-only gate fails three of the memberNeedsMikaSetup cases, and dropping the footer prop fails the chrome test. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Lambda <lambda@multica.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
|
24d209794e | fix(channel): attach /issue media to created issues (#6388) | ||
|
|
ba129b1963 |
feat(issues): show per-run token usage on the execution log (MUL-5762) (#6440)
* feat(issues): show per-run token usage on the execution log (MUL-5762)
The execution log already lists every agent run on an issue; it just
never said what any of them cost. task_usage has held per-task token
counts since migration 032 — nothing surfaced them per run.
Three placements, one data source:
- Execution-log header carries the issue total ("2.1M · $4.92") and
opens the breakdown.
- Each row carries its own token figure. This takes the slot the
relative timestamp held: the sidebar is 288px and a third column
would come out of the trigger text, which is what people scan. The
list is sorted newest-first, so ordinal recency is already free; the
timestamp moves into the row tooltip alongside duration and model,
neither of which was surfaced there before.
- The transcript dialog gets the same figure in its header, with the
input/output/cache split in the run-info popover.
Backend: ListIssueTaskUsage returns per-(task, provider, model) rows in
one query, joined onto the existing task-runs response. The model
dimension stays on the wire because cost is priced client-side per
model — a row that collapsed two models cannot be priced at all.
Cost reuses estimateCost from the runtime usage page, so the issue and
the workspace never disagree; the new summarizeTaskUsage helpers live
next to it rather than starting a second cost formula.
No usage recorded stays distinguishable from zero end to end — omitted
on the wire, undefined in the schema, null from the summarizer, an em
dash in the UI. A run from before usage reporting was not free.
Removes the standalone "Token usage" sidebar section: it showed the
same issue totals minus the cost and minus any way to attribute them,
and every field it had is in the dialog. The /api/issues/:id/usage
endpoint it read stays — the CLI's `issue usage` still uses it.
Co-authored-by: multica-agent <github@multica.ai>
* fix(issues): address review on per-run token usage (MUL-5762)
Three findings from @Emacs, all confirmed against the code:
1. Drop the token figure from active rows. The daemon reports usage
once, after `runner.run` returns (internal/daemon/daemon.go), and
ReportTaskUsage publishes no realtime event — so a running task has
no usage to show and would not learn of it mid-run if it did. The
branch was only ever exercised by a hand-written fixture, which is a
test asserting a scenario production cannot produce. The row keeps
its timer; restore the figure in the same change that adds
incremental reporting + cache invalidation.
2. Subscribe the usage surfaces to the custom-pricing store. estimateCost
reads custom rates imperatively via getCustomPricing(), so nothing
re-rendered these after a saved rate change — the header total, the
dialog's totals and per-run costs, the cost-by-agent split, and the
"unmapped model" notice all kept quoting the old price until the task
list happened to refetch. Same subscription the runtime usage page
already carries, plus the snapshot in every memo that prices usage.
Regression test pinned: it fails without the subscription.
3. Give the dialog's status glyph an sr-only label. TaskStatusIcon is
aria-hidden, so a screen reader could not tell a failed run from a
completed one — the execution log rows already pair the two.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Lambda <lambda@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
|
||
|
|
c896f677d5 |
fix(agent): recover from a codex resume that overflows the reader (MUL-5722) (#6420)
Layer 1 (#6383) raised the shared scanner cap to 32 MiB, which moved the cliff without removing it — a codex rollout is append-only, so a thread that outgrows any fixed cap still fails its resume forever. This adds the recovery path. Layer 2: an oversized thread/resume response is reported as Result.ResumeRejected rather than a crash, which is the positive evidence shouldRetryWithFreshSession needs and reconnects the recovery #5715 had unintentionally cut off. Reaching it required one real fix — the reader wrapped the scanner error with %v, so bufio.ErrTooLong never survived to a caller. Layer 3: a new codex_resume_oversized reason marks the session resume-unsafe, and both resume lookups block by TIME rather than by matching the failed row. That shape is required, not stylistic: the failure happens before the turn starts, so the row lands with session_id NULL and is dropped by latest_per_session before any error-text filter runs. The block expires once a thread terminates after the overflow, so an issue recovers instead of starting cold forever. Also splits the MUL-4424 continuity notice by what each surface can still read — issue comments and Slack channel history can be re-read, web chat and Feishu cannot — so only the last group tells the user a loss happened. The backend no longer holds any of that wording; it receives ExecOptions.ResumeContinuityNotice from the caller and stays silent when the prompt already carries it, which makes the duplicate injection on the retry path structurally impossible. Known gap, tracked not claimed: for chat, the claim handler reads chat_session.session_id before the fallback query, so a daemon predating this PR leaves that pointer naming the oversized thread. Clearing it needs the attempted session recorded at claim time, which is a schema change. |
||
|
|
521a63671c |
fix(chat): preserve visible queue-head ordering (MUL-5751) (#6431)
* fix(chat): keep visible queue heads in transcript order Co-authored-by: multica-agent <github@multica.ai> * test(chat): cover cancelled follow-up ordering Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai> |
||
|
|
b3fa17f0e0 |
fix(chat): preserve follow-up transcript order (#6419)
Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai> |
||
|
|
73283556d7 |
MUL-5750: fix(chat): keep idle sends out of follow-up queue (#6418)
* fix(chat): keep idle sends out of follow-up queue Co-authored-by: multica-agent <github@multica.ai> * fix(chat): preserve the positional queue head Co-authored-by: multica-agent <github@multica.ai> * fix(chat): polish deferred queue states Co-authored-by: multica-agent <github@multica.ai> * fix(migrations): resolve pending index prefix collision Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai> |
||
|
|
c1be369f6d |
[MUL-5736] fix(channel): make issue commands terminal outcomes (#6398)
* fix(channel): make issue commands terminal outcomes * fix(channel): stop handled command media from gating later chat tasks A `/issue` turn is now answered synchronously and excluded from every later input batch, but it still persisted a media deadline, and both session-scoped media gates counted it. The next, unrelated chat message was therefore deferred until the command's attachments bound — or for the full fallback budget on the create-failure path, where no binder ever runs to clear the marker. Give GetChannelMediaPendingUntil and PromoteChannelChatTasksIfMediaReady the same population as the batch seal, so only a turn that can join a batch can gate one. DeferChatTaskForSealedPendingMedia already scopes by task id and needs no change. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
|
a4dc54f2ed |
MUL-5570 / MUL-5493: feat(chat): add a managed follow-up queue (#6211)
* feat(chat): queue follow-up messages during active runs * fix(chat): preserve queued message compatibility * fix(chat): harden queued task consistency * fix(chat): protect legacy and mobile queue state * fix(mobile): refresh stop handler on task promotion * fix(mobile): preserve queued chat state * fix(chat): hide queued prompts from legacy clients * test(chat): read queued channel input from paged transcript * fix(chat): protect legacy and mobile queue state * fix(mobile): refresh stop handler on task promotion * fix(mobile): preserve queued chat state * fix(chat): hide queued prompts from legacy clients * test(chat): read queued channel input from paged transcript * fix(chat): hide queued inputs from paged transcripts * test: prove queued messages do not consume cursor pages --------- Co-authored-by: TeAmo <liu.junhui3@iwhalecloud.com> |
||
|
|
9fb46adc43 |
fix(agent-builder): serialise draft autosave against session delete (MUL-5642) (#6376)
SaveAgentBuilderDraft read the chat session, then upserted agent_builder_draft as a separate statement, with no lock between them. DeleteChatSession takes LockChatSessionForDelete for its whole transaction, so a save could pass its checks, block on nothing, and land its INSERT after the delete committed. agent_builder_draft carries no chat_session FK (repo rule), so nothing rejected that write. The surviving row held the configuration the user had just confirmed discarding. It is invisible to the UI — the drafts list joins through chat_session — and no prune can reach it: DeleteAgentBuilderDraft and the runtime teardown both key off a session that no longer exists, leaving only the workspace teardown. The client autosaves on an 800ms debounce and the conversation is addressable by URL, so a second tab can autosave at any moment while this one discards. Add LockChatSessionForDraftWrite, the same row and lock mode the delete and runtime-bind paths take, and run the upsert in a transaction that acquires it first and re-reads the session under it. Existence and status are the only two things a concurrent writer can change, and both are now decided inside the lock; workspace, creator and carrier are immutable for a session and stay on the cheap unlocked read. Either ordering is now correct: the save commits first and the delete prunes it, or the delete commits first and the save returns 404. The same lock closes the archive variant, where the last autosave after "create agent" could write a draft onto an already read-only session. Both regression tests drive the interleaving deterministically — hold the session row, prove the save blocks, then commit — and fail on the pre-fix handler with a 204 that writes the orphan. Co-authored-by: multica-agent <github@multica.ai> |
||
|
|
eea687d461 |
MUL-5686: fix(chat): resume the cancelled turn's session instead of starting cold (#6352)
* fix(chat): resume the cancelled turn's session instead of starting cold
Stopping a chat turn the agent had already begun answering, then sending
the next message, produced a reply with no memory of the conversation.
The cancelled turn's provider session is real and recorded — the daemon
pins it onto the task row mid-flight and cancellation keeps it there —
but nothing could hand it back:
- GetLastChatTaskSession / GetLastTaskSession only considered
'completed' and 'failed' rows, so a cancelled row's session was
invisible to resume resolution;
- chat_session.session_id is written only by CompleteTask / FailTask,
and a cancelled task reaches neither: the daemon discards its result
and sends a cancel-ack. On a chat whose first turn was cancelled the
pointer therefore stayed NULL and the next turn started cold, since
buildChatPrompt injects only the current user message.
Let cancelled rows into both resume lookups, and advance the chat-level
pointer at cancel time so a provider that mints a new session id per
resume does not rewind past the cancelled exchange. The mid-flight pin
may now also fill an EMPTY session slot on a just-cancelled row, which
is what a Codex pin waiting on its rollout needs when the cancel wins
the race; occupied slots and completed/failed rows stay untouchable.
Retired sessions, poisoned-failure filters and the rollout-present guard
are unchanged. A transcript killed mid-tool-call that the provider later
refuses is still caught by taskfailure.UnresumableHistory, which retires
the session and starts the next turn fresh.
Fixes #6340
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): close the two cancel/pin races on the chat resume pointer
Review found the pointer advance had the same shape of hole it was meant
to fix, on both sides of the cancel.
1. The status flip and the pointer advance were separate statements. In
the gap the row is already `cancelled` while the pointer still names
the PREVIOUS turn, so a follow-up the user had queued could be claimed
there and resume the older session — and a failed pointer write only
logged, still reporting the cancel as successful. Both now commit in
one transaction and the error propagates.
2. The mid-flight pin may land AFTER the cancel (for Codex it waits for
the rollout), so the cancel transaction sees no session to publish and
the pin only fills the task row. On a chat that already had history
the stale pointer kept shadowing it, which is precisely the case the
pin change was added for.
Both paths now run one guarded statement,
AdvanceCancelledChatSessionPointer. It reads the task row itself rather
than trusting an in-memory copy, ignores anything that is not a cancelled
chat task, and refuses to move the pointer when a NEWER task on the chat
already recorded a session — so a straggler pin cannot drag the
conversation back onto the interrupted turn.
Regression tests, both failing before this commit: a cancel whose pointer
write is blocked must not expose `cancelled` to another connection, and a
late pin on an already-cancelled row must reach the next real claim. The
newer-turn guard is covered too.
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): make the pin atomic and restore the chat->task lock order
Second review round found the remaining half of the same class of bug.
1. PinTaskSession still committed the session onto the task row and
advanced the chat pointer as two statements, and the pointer failure
only logged while the endpoint still answered 204. A follow-up claimed
in that window resumes the previous turn — the exact case the pin
change was added for. Both writes now share one transaction and every
failure is reported.
2. Adding the pointer write gave the cancel transaction two rows to hold,
and it took them in the wrong order: agent_task_queue first, then
chat_session. DeleteChatSession takes them the other way round, so the
two could deadlock (40P01, and runInTx has no deadlock retry). The
repo's documented order is chat_session -> agent_task_queue; both the
cancel and the pin now open with LockChatSessionForTask, the same
helper FinalizeDeferredCancelledChat uses. ErrNoRows there means a
non-chat task or an already-deleted session — nothing to lock and
nothing to advance.
Regression tests, all three failing before this commit (the concurrency
one with a real `deadlock detected`): the pin must not expose a session
on the task row while its pointer write is blocked; a cancel waiting on
the chat session must hold no lock on the task row (FOR UPDATE NOWAIT
probe); and cancel racing a chat delete must never come back with 40P01.
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): put every chat-task terminal write behind the same lock order
The cancel and pin paths took chat_session before agent_task_queue; the
terminal reports still took them the other way round, so the two could
deadlock (40P01) — reproduced deterministically as cancel-vs-complete.
Choosing per-path was never going to work: either every writer that holds
both rows agrees on an order or none of them are safe.
CompleteTask, FailTask and the cancelled-chat finalize now open with the
same LockChatSessionForTask the cancel, pin, DeleteChatSession and
FinalizeDeferredCancelledChat paths use, via a shared helper that
documents the invariant in one place.
Also de-flakes the concurrency tests this PR added. They held a row lock
and then read other rows on a FRESH pooled connection; the suite shares
one database, so a sibling package's DDL could queue an ACCESS EXCLUSIVE
request in between, park our later ACCESS SHARE request behind it, and
wedge the package until the 10-minute timeout (observed in a parallel
`go test ./internal/...` run). Those transactions now take their table
locks up front and read on the connection that already holds them, and
every racing call is bounded so a stall fails loudly instead of hanging.
Regression tests, all failing before this commit: complete and fail must
hold no lock on the task row while waiting for the chat session (FOR
UPDATE NOWAIT probe), and cancel-vs-complete plus pin-vs-fail must never
come back with 40P01.
Co-authored-by: multica-agent <github@multica.ai>
* test(chat): fail the race tests on any unexpected error, bound every call
Review nits on the concurrency tests.
Matching only *pgconn.PgError(40P01) let the pin side through: a pin that
loses a deadlock is reported by the HTTP handler as a plain 500 with no
PgError to unwrap, so the check skipped exactly the failure the test
exists for. Both racers now fail on anything except the one benign
outcome — whoever finalized the task first leaves the loser matching no
row (pgx.ErrNoRows).
The remaining racing calls still used an unbounded context.Background(),
which this PR had already claimed were bounded. They now share the same
raceTimeout as the rest.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: multica-agent <github@multica.ai>
|
||
|
|
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 <noreply@anthropic.com>
|
||
|
|
a5c1d44701 |
MUL-5642: fix(agents): stop the creation studio polling, and stop it losing work (#6307)
* feat(agents): make AI agent creation resumable (#6246) Leaving the Agent Creation Studio destroyed the conversation. The unmount cleanup called deleteChatSession, so a sidebar click, a tab close or a route change deleted the builder session and every message in it — the bug external users reported. Archiving instead (PR #6247) would have stopped the deletion without giving anyone a way back in: builder sessions hang off a hidden `kind = 'system'` carrier agent, which the `kind = 'user'` filter keeps out of every chat list, so an archived one is unreachable rather than recoverable. A creation conversation is now a durable object with its own address. Server: - GET /api/agent-builder/sessions lists the caller's unfinished creations. Creator-scoped like every other chat read. It reports the CARRIER's runtime, not chat_session.runtime_id — the latter is the daemon's resume pointer and is deliberately left stale after a switch, so resuming from it would put the picker on a runtime that executes nothing (MUL-5163). - PUT /api/agent-builder/sessions/{id}/draft stores the configuration, including the edits the user typed but never sent. Migration 251 adds agent_builder_draft (no FK per repo rule; pruned explicitly by DeleteChatSession, the runtime teardown and the workspace teardown, and registered in the workspace-deletion manifest). - The payload is opaque to the server: its shape is the studio's AgentDraft, validated client-side. Teaching Postgres and the handler about it would create a second definition to keep in sync for no gain. Client: - The session id lives in `?session=`, so a refresh, a back/forward and a reopened tab land back in the same conversation. - Leaving no longer deletes anything. The only destructive path is an explicit "discard", confirmed in a dialog, next to the create button. - Creating the agent archives the conversation instead of deleting it: it is the record of how that agent was designed, and an idle carrier costs nothing since usage is booked per task. - The configuration autosaves (debounced) and restores on arrival, with the applied-assistant-message marker stored alongside it so a restore cannot re-apply the last reply over edits made after it. - The 1.5s polling of messages and pending-task is gone. The global realtime sync already invalidates both per session id, exactly as it does for the main chat window, which has never polled. - The `<agent_draft>` block collapses to one "configuration updated" line. The regex now also swallows an unterminated block, which is what streaming produces — the raw payload used to scroll past on every turn. The 2185-line agent-creation-studio.tsx is split into three routes (`/agents/new`, `/agents/new/manual`, `/agents/new/ai`), its pure logic moves to packages/core/agents/ with its tests, and the unreachable template flow — `setMode("templates")` had no caller — is removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(agents): let the builder panes resize The conversation / configuration split was not draggable. Two structural reasons, both fixed by giving the group the same shape the chat page uses: - The panels reached the group through BuilderWorkspace's fragment, so they were not children the group could measure. - The group's children alternated between one panel (runtime setup) and two (conversation), under one persisted layout id. The group now lives inside BuilderWorkspace with its two panels as its only children, and the setup screen renders no group at all — it has nothing to split. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ui): give the resize handle a cursor on hover The separator had no cursor of its own, so the only signal that a split was draggable arrived after the drag started — the library writes a global `cursor: ... !important` while dragging, and nothing before it. Fixed on the shared handle rather than at one call site: every split surface (chat, inbox, issue detail, project detail, the agent builder) was missing the same affordance. The library's drag-time rule still outranks this one, so the cursor keeps narrowing to `e-resize` / `w-resize` once a panel hits its bound. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(agents): render the builder's draft block as an inspectable row Every builder reply ends in an <agent_draft> block that rewrites the form on the right. Flattening it to a line of prose said that something changed but not what, and the payload — the only record of what the builder actually claimed — was unreachable. A settled reply now carries a full-width row saying the configuration was updated, which opens the exact payload. A streaming one keeps a text line instead: the block is still being written, so there is nothing complete to open, and without the line the half-finished JSON scrolls past. ChatMessageList gains an optional `renderAssistantAddon`. It is opt-in per surface and undefined everywhere but this one, because no other chat speaks this protocol — the alternative was to keep pushing an embedded protocol through `transformContent`, which can only ever produce prose. `extractBuilderDraftBlock` returns an unparseable payload verbatim rather than withholding it: a malformed block is exactly when someone wants to read it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(agents): address review blockers on the resumable builder 1. Migration prefix collision. `251_agent_runtime_unbind` landed on main after this branch cut, so the backend's prefix-uniqueness guard failed. Renumbered to 252. 2. #6287 — the manual form still lost everything. The route split moved where you land, not what survives: the draft was `useState`, so a tab switch (the desktop shell mounts only the active tab) remounted it empty, and the beforeunload guard covered a hard reload and nothing else. It now persists through the repo's draft-store factory, scoped by what is being created — a blank agent and a copy of agent X are different work, and a copy of X is not a copy of Y — cleared once the agent is committed, and registered for logout / workspace-delete cleanup. 3. A saved draft with no messages was unreachable. The configuration form is editable from the moment a builder session exists and autosaves, so someone could open it, type a name and leave before the first turn; the list keyed "is this a draft" on messages alone, so that row existed and nothing could reach it. A session now qualifies on a message OR a stored draft, and sorts by whichever it has. 4. The debounce dropped the last edits. Its timer died with the component, so navigating away inside the 800ms window lost exactly the keystrokes the user had just made. The pending payload is now flushed on unmount. `useUnsavedDraftWarning` is gone with its last caller: both routes persist, so the browser prompt would have been warning about work that is already saved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ui): stop the resize cursor flipping mid-drag The library narrows its cursor the moment a panel hits a bound — col-resize while both directions are open, a one-way arrow once only one is. Truthful, but it reads as a glitch: the icon changes under your hand halfway through a drag you never stopped making. `disableCursor` turns that global rule off; the handle's own `cursor-col-resize` is now the only source. A drag captures the pointer and walks it across the panels, away from the 8px handle, so the group carries the same cursor for as long as a separator is active — otherwise it would fall back to a text caret the instant the pointer left the handle. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(agents): key manual drafts by owner, drop the draft-block rendering Two changes. One slot destroyed the other flow's work. The manual draft was stored under a single key: opening a blank form, or a copy of a different agent, refused to adopt the stored draft and then immediately wrote its own empty form over it — so a half-finished copy of agent A died the moment the user opened anything else, before typing a character. Drafts are now keyed by what is being created, the same shape the chat composer uses for its per-session drafts, and a slot is dropped when its content is gone rather than parked blank (which also stops the map growing a dead key per agent ever opened for duplication). Committing an agent clears that flow's slot only. The `<agent_draft>` block goes back to being hidden outright. Labelling it and opening its payload dressed up machinery as content: the block drives the configuration form, and the form is where its effect is already visible. `renderAssistantAddon` goes with it — ChatMessageList is back to what it was, since no surface needs the slot. The two-pattern strip stays: an unterminated block is what streaming produces, and without matching it the raw JSON scrolled past the reader on every turn. Also removed six barrel exports nothing imported through, and unexported five types only their own file used. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(agents): keep a manual draft whose only edit is a picker The "is this worth storing" predicate listed six fields by name, and the draft serializes eleven. A form whose only change was the model, the thinking level, the service tier, the access scope or a team grant read as untouched, so the next save deleted its slot — picking a model before typing a name and switching tabs lost the model. Enumerating was the mistake, not the specific omissions: the predicate stops covering every field added after it is written, and the failure is invisible because each field saves correctly as long as some *other* field is also set. It now compares the whole draft against a fresh one. The runtime stays outside that comparison, on the entry rather than in the draft, because the form seeds it on every visit and counting it would store a draft for a form nobody touched. Covered field by field, one edit at a time, so a future field cannot quietly fall out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
4fe94a6d40 |
revert: "MUL-5637 fix(mcp): agent mcp_config is an authoritative allowlist (#6292)" (#6314)
This reverts commit
|
||
|
|
aa349fed02 |
MUL-5637 fix(mcp): agent mcp_config is an authoritative allowlist (#6292)
* fix(mcp): treat agent mcp_config as an authoritative allowlist
An agent's saved mcp_config was silently widened with the runtime host's
own user-level MCP servers, so an explicitly empty `{"mcpServers":{}}`
resolved to the COMPLETE host set instead of no servers at all — the
opposite of what the operator configured (GitHub #6283).
`--strict-mcp-config` was being passed correctly; the merge happened
before it, in the daemon, so strict mode constrained an already-widened
set. Introduced by #5277 and present in v0.4.16 through main.
Restore the three-state contract in resolveEffectiveMcpConfig:
null / unset -> inherit the provider's native MCP configuration
{"mcpServers":{}} -> strict empty, no host servers
non-empty object -> strict allowlist, exactly those servers
Two explicit inherit paths keep the additive behaviour reachable without
weakening the default:
- runtime_config.mcp.inherit_runtime = true opts an agent back in.
- The claim response now carries mcp_config_overlay_only so the daemon
can tell an agent-authored config from a per-task Composio overlay.
Without it, enabling an integration on an agent that never configured
MCP would have stripped the host servers it was already inheriting.
Both decode paths fail closed: malformed runtime_config never enables
inheritance, and a failed runtime merge falls back to the agent's own
config.
The web MCP tab and the `agent create/update --mcp-config` help text
described the old additive behaviour, which is how a tightened config
could look correct while exposing every host server; both now state
which mode is in effect.
Note for rollout: the fix lives in the daemon, so self-hosted users must
upgrade the daemon — a server/UI upgrade alone does not apply it.
Co-authored-by: multica-agent <github@multica.ai>
* fix(mcp): close review gaps in the authoritative mcp_config change
Addresses the four must-fix findings from review of #6292.
1. Deleting the last managed server no longer widens access.
removeManagedMcpServer cleared the config to null, which now means
"inherit the host's MCP servers" — so a delete took the agent from one
allowed server to every server on the host. It now leaves an explicit
`{"mcpServers":{}}`. Restoring inheritance moved to a separate
clearManagedMcpConfig action behind its own confirmation that states the
widening. The delete dialog no longer claims "Runtime servers are not
affected", which was the opposite of the truth.
2. The UI no longer promises a boundary an old daemon does not enforce.
The strict semantics live in the daemon, so a config saved against an
older daemon is not yet in effect. Adds the authoritative-mcp-v1 daemon
capability:
- The daemon advertises it and reports authoritative_mcp on the
runtime-capabilities response.
- The claim path fails closed: a managed, non-inheriting mcp_config
claimed by a daemon without the capability cancels the task and
returns 412 with an actionable message, instead of letting that daemon
merge the host's servers in. runtime_config.mcp.inherit_runtime is the
documented escape hatch, and it is honest — it declares that the
operator accepts the host's servers.
- The MCP tab shows "needs upgrade" rather than "Not exposed" while the
bound runtime lacks the capability.
3. Saving OpenClaw settings no longer drops the inherit opt-in.
parseOpenclawRuntimeConfig discarded unknown keys and the tab persisted
the result as the whole runtime_config, so one unrelated routing save
silently deleted mcp.inherit_runtime. Unknown keys now round-trip
through OpenclawRuntimeConfig.passthrough, excluded from the dirty check
so they cannot make the form look edited.
4. Documents the new semantics in the built-in creating-agents skill and
its source map: the three states, the persisted
runtime_config.mcp.inherit_runtime field, and the claim-time capability
gate.
Also corrects the PR's rollout claim: there is no database migration, but
this does add a persisted JSON field and change the meaning of an existing
one.
Co-authored-by: multica-agent <github@multica.ai>
* fix(mcp): stop the authoritative-daemon gate from blocking valid claims
CI's backend job failed three handler claim tests with the new 412. Two
distinct problems, both real:
1. The gate fired for a non-object mcp_config. 66 handler fixtures seed
`[]`, which is not a valid MCP config and cannot carry `mcpServers`, so
it expresses no boundary to protect. An old daemon does not widen it
either: mergeRuntimeAndAgentMcpConfig fails to unmarshal a non-object
and falls back to the agent config alone (verified directly). Gating
these blocked tasks with no security benefit, so the gate now requires a
JSON object.
2. The shared daemon test-request helper advertised no capabilities, so
every claim test was accidentally simulating a pre-#6283 daemon. It now
defaults authoritative-mcp-v1 on, matching what every current daemon
sends. Only that capability — skill-bundles / coalesced-comments / rpc
are feature negotiations whose absence tests real legacy behaviour, so
they stay opt-in per test.
Adds claim-level coverage for the gate itself, which is what the unit tests
alone could not catch: an outdated daemon gets 412 with an actionable
message and the task is cancelled; a capability-advertising daemon gets
200; the inherit_runtime opt-in lets an outdated daemon through; and an
unmanaged or non-object config is never gated.
Verified against a real migrated schema this time (throwaway Postgres),
which is how the three failures were reproduced locally and confirmed
fixed: `go test ./internal/handler ./internal/daemon` both ok.
Co-authored-by: multica-agent <github@multica.ai>
* fix(mcp): surface the daemon-upgrade refusal and stop gating safe providers
Addresses the second review round on #6292.
1. The refusal is now visible wherever the operator looks. The default
claim path is the machine-level BATCH endpoint, which skips build
failures and still answers 200 {"tasks":[]}, so the previous bare
CancelTask showed a task that vanished with no stated reason — turning an
explicit upgrade requirement into an unexplained failure. The claim path
now fails the task with a new classified reason,
mcp_config_daemon_outdated, plus the actionable message. That reaches the
user on all three claim paths and on any daemon version, which a new
response field could not: the audience is by definition a daemon too old
to read one. The per-runtime path keeps its 412.
The reason is deliberately not auto-retryable — the same outdated daemon
would claim the retry and fail it again.
2. The gate no longer cancels safe tasks. It applied to every provider, but
only claude / codebuddy / codex / cursor / opencode / openclaw were ever
merged with host MCP by an old daemon (loadRuntimeMcpServerConfigs).
Qwen was never merged and already had strict semantics, so its tasks were
being failed for a risk that does not exist. Scoped via
providersOldDaemonsMergedRuntimeMcp; an unknown provider does not gate,
because the gate should only fire where the old behaviour is concrete.
3. The new authoritative_mcp flag now goes through the API schema layer.
Both local-skills responses were returning raw network JSON, so the flag
that decides whether the UI may assert an MCP boundary rested on an
unchecked type assertion. Adds RuntimeLocalSkillListRequestSchema with
authoritative_mcp and mcp_supported defaulting to FALSE — the fail-closed
direction — and a MALFORMED_ fallback that cannot express a guarantee.
Claim-level tests now cover all three paths, which is what the previous
helper-only tests missed: per-runtime 412, batch recording the refusal on
the task while still delivering the healthy tasks in the same batch, WS RPC
refusing and accepting, the qwen negative case, the inherit_runtime escape
hatch, and unmanaged / non-object configs.
Verified against a real migrated schema (throwaway Postgres):
go test ./internal/handler ./internal/daemon ./pkg/agent ./pkg/taskfailure
./internal/service all ok.
Co-authored-by: multica-agent <github@multica.ai>
* fix(mcp): register the new failure reason and wire its copy into the UI
Addresses the third review round on #6292.
1. mcp_config_daemon_outdated was declared but never registered in
taskfailure.allReasons, so metrics.NormalizeFailureReason missed the
known-value map and fell through to free-text Classify() — relabelling a
platform-side refusal as `agent_error.unknown` (verified directly) and
leaving the Prometheus series un-pre-warmed. Registered it, canonical
count 22 → 23 (platform 8 → 9), with the wire value and IsAgentError split
pinned. New test pins the WHOLE canonical set through
NormalizeFailureReason so forgetting the next reason fails a test instead
of quietly mislabelling a metric; NormalizeFailureReason had no coverage
at all before.
2. The upgrade copy was dead. The locale strings landed last round but
neither consumer mapped the reason: chatFailureCopy fell back to generic
failure text with the actionable detail buried in the collapsed raw
error, and task-failure.ts rendered the bare wire value
`mcp_config_daemon_outdated` in the agent activity list and issue
execution log. Both are mapped now, with regression tests, plus the
runtime class pinned in failure-class.test.ts. This directly contradicted
the claim in the claim-path comment that every path reaches the user, so
that is now actually true.
3. providersOldDaemonsMergedRuntimeMcp is documented as what it is: a FROZEN
record of what pre-capability daemons merged, not a mirror of the daemon's
current provider switch. The old "keep the two lists in lockstep" note was
actively harmful advice — runtime MCP discovery for a new provider can only
ship in a daemon that already advertises the capability (never gated), so
adding it here would fail tasks on old daemons that never merged for it,
re-creating the qwen false-positive. Pinned with a test.
Also corrects a stale count in task-failure.ts (7 → 9 platform reasons).
Verified against a real migrated schema (throwaway Postgres): full backend
suite green apart from the pre-existing environmental cmd/multica guard; all
9 TestMcpGate_* integration tests pass.
Co-authored-by: multica-agent <github@multica.ai>
* docs(taskfailure): correct taxonomy counts and finish the reason registration
Non-blocking nits from the fourth review round on #6292.
- Taxonomy counts now say 23 reasons / 9 platform-side. Registering
mcp_config_daemon_outdated last round updated the assertions but not the
prose. Swept the whole repo rather than only the flagged lines, which
turned up four more that were already stale at 21 and drifted further:
handler/dashboard.go, daemon/poisoned.go, core/types/agent.ts, and the
db/queries/task_usage.sql comment sqlc copies into the generated file.
The generated file's comment was updated by hand to match its source.
Running `sqlc generate` churned 58 lines across 47 unrelated files — the
local sqlc version differs from the one that produced the checked-in
output — so that churn was reverted and only the one intended line kept.
- failure_test.go's `required` list now includes
ReasonMcpConfigDaemonOutdated. Length and label assertions already covered
the reason, but the list is documented as the complete canonical set, so
the omission contradicted its own comment.
- Restored the line break in chat-message-list.test.tsx that a previous edit
of mine collapsed.
Comment, test-fixture and formatting only; no behaviour change.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
|
||
|
|
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 <github@multica.ai> * fix: make runtime unbinding recoverable Co-authored-by: multica-agent <github@multica.ai> * fix: address runtime unbind review nits Co-authored-by: multica-agent <github@multica.ai> * fix: resolve runtime unbind review blockers Co-authored-by: multica-agent <github@multica.ai> * fix(migrations): renumber runtime unbind after main merge Co-authored-by: multica-agent <github@multica.ai> * test(daemon): avoid late-request lease flake Co-authored-by: multica-agent <github@multica.ai> * test(autopilots): bind validation fixture runtime Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai> |
||
|
|
28b6105edc |
fix(subscribers): notify the human an agent files sub-issues for (MUL-5483) (#6209)
When an agent created a sub-issue while working on a human's behalf, that human received no notifications for it at all. issue_subscriber modelled ACTOR identity, so an agent-created, agent-assigned issue had a full subscriber list and zero members to deliver to. The platform already knew who the work was for (agent_task_queue.originator_user_id, MUL-4302); notification never asked. - attribution.DelegatedSubscriber: one shared rule over the same origin waterfall ClassifyDirect uses. agent_create subscribes the originator as 'delegated'; quick_create keeps the direct 'creator' tier; autopilot and degraded attribution subscribe nobody. - Delegated is a reduced delivery tier: in_review/done/cancelled/blocked plus failures and mentions. Routine churn is suppressed, and the parent bubble cannot re-deliver what the tier dropped. - Unsubscribe becomes stateful: an unsubscribed_at tombstone survives later rule passes, and opt_out_scope distinguishes "this issue" from "this subtree" so a narrow opt-out no longer silently suppresses future children. - Subtree unsubscribe is its own endpoint. A body flag cannot fail loudly against an older backend (Go drops unknown fields); an unknown route 404s, which the UI now surfaces with a distinct message. - Eligibility and the write share one statement under a (workspace, user) advisory lock that subtree unsubscribe and member revoke also take, closing the check-then-insert races. Revoke additionally clears the departing member's subscriptions in the same tx. - UI explains a delegated subscription and offers both unsubscribe scopes. Migrations 249/250 add the delegated reason, the opt-out tombstone, and the opt-out scope, using NOT VALID + VALIDATE CONSTRAINT so the widened CHECK does not scan issue_subscriber under an exclusive lock. Reviewed across eight rounds; an earlier write-time subtree roll-up was built and then removed in full once it proved unfixable without serializing every topology mutation. The parent's own status transition already carries that signal. Closes MUL-5483. |
||
|
|
f48bd655bc |
MUL-5562: optimize application-owned workspace deletion (#6230)
* fix(workspace): optimize application-owned deletion Co-authored-by: multica-agent <github@multica.ai> * fix(workspace): address deletion review findings Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai> |
||
|
|
f13969b996 |
refactor(chat): generate quick actions server-side via the LLM layer (MUL-5573) (#6214)
* refactor(chat): generate quick actions server-side via the LLM layer (MUL-5573)
Follow-up suggestions were produced by a second, full provider CLI invocation
per chat turn: the daemon resumed the just-finished session and ran a
suggestion-only pass. That pass inherited the main turn's exec options, so its
20s budget had to cover process spawn, every MCP handshake, session replay, and
model reasoning at the agent's own thinking level — typically 8-15s of visible
skeleton, and every turn paid two provider cold starts.
Generate them here instead, through the same pkg/llm layer that backs chat
auto-titling. Suggestions need no tools, workdir, or agent identity — only the
tail of the conversation — so a bounded 8s call on the deployment's small model
replaces the whole resumed turn.
Quality changes that came with the move:
- The prompt now states the frame explicitly ("you write FOR THE USER"). The
old pass ran inside the agent's session and inherited the runtime brief's
identity, which drifted suggestions toward agent-operations actions.
- Previously-offered labels are replayed as ALREADY SUGGESTED. The old
architecture had the opposite effect: on providers that append on resume,
each pass saw its predecessor's JSON and anchored on it.
- A failed generation broadcasts failed=true. Before, a timeout delivered an
empty array — indistinguishable from "nothing worth suggesting", so every
slow pass read as a quality problem.
- The in-band footer is still stripped from replies but its actions are now
discarded, so a pre-upgrade session is not pinned to the retired
suggestions with the replacing pass suppressed.
The refresh path no longer enqueues an agent task: it validates the target and
calls the same generator, which also drops the not-resumable refusal — a session
whose runtime was rebound can now be refreshed. Client contract is unchanged
(chat:done pending flag, chat:quick_actions supplement); the only frontend
change is the pending window, resized from 30s to 12s to match the new budget.
Also removes the daemon's TMPDIR-after-cleanup hazard by construction: the old
pass started after runTask's defers had already deleted the temp dir it was
still pointed at.
Co-authored-by: multica-agent <github@multica.ai>
* refactor(chat): drop the quick-actions opt-out setting (MUL-5573)
Suggestions are always on. The Settings → Chat toggle is removed along with
the whole per-turn opt-out path it fed: the persisted client preference, the
quick_actions_enabled send field, the quick_actions_disabled task stamp, and
the eligibility gate that read it.
The toggle predates server-side generation, when it could only hide chips a
provider pass had already paid for. Now that generation is a bounded call the
server decides on, an off switch buys nothing a user would miss, and it was
the last piece of UI implying the feature might be unavailable.
agent_task_queue.quick_actions_disabled is no longer written (dropped from
CreateChatTask's INSERT; the column keeps its false default). Left in place
alongside regenerate_quick_actions_for for a later drop migration — removing
columns an already-running binary still inserts would break mid-deploy.
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): address quick-actions review findings (MUL-5573)
Four defects from review of the server-side generation change.
1. Automatic failures were reported as refresh failures. The generator
broadcast failed=true on any LLM error, but the client turns every
failed=true into a "couldn't refresh" toast — so an automatic timeout
popped a toast for an action the user never took. This also contradicted
ChatQuickActionsPayload.Failed, which documents false for the automatic
pass. The caller now passes its origin; only an explicit refresh reports.
2. Generation context was not bound to the target turn. The pass re-read the
session's newest messages while always writing to the task it was handed,
so a turn landing between the completion callback and the detached read
supplied the context for a reply it did not belong to. Worse, a user
typing a follow-up in the second after a reply left the window ending on
a user row, which the old code treated as "nothing to build on" — that
turn silently never got pills. The window is now anchored on the target
assistant message and queried strictly before it.
3. No concurrency or idempotency bound on generation. Refresh stopped
creating a task, so the busy check could not see a pass already running:
two refreshes both returned 202, spent two upstream calls, and raced to
write one row. Nothing bounded generation process-wide either. Adds a
per-session in-flight guard (refresh now 409s on a duplicate) and a
process-wide ceiling; a shed pass still resolves the client placeholder
so no skeleton hangs on work that never started.
4. A new daemon could not safely talk to an older server. The refresh task
discriminator was deleted, so a regenerate task from such a server fell
through to the ordinary chat path: no user message, but the agent would
answer anyway and the server would persist it as a real reply. The field
is restored as a refusal marker only — the task completes empty, which is
the shape the retired pass produced and which that server writes no row
for. Not a restored execution path.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Lambda <lambda@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
|
||
|
|
c3f5df8bf4 |
MUL-5492: fix timeline cap dropping newest entries + stop double-broadcasting descriptions (#6175)
* fix(timeline): cap the issue timeline at the newest end and report the clamp The per-issue timeline cap was applied with ORDER BY created_at ASC LIMIT 2000, so once an issue accumulated more than 2000 comments or activities the cap discarded the NEWEST rows. The timeline appeared to stop at some point in the past and every later event was invisible, with nothing in the response indicating anything was missing. Activity is machine-paced — description autosave, every agent run, status and assignee changes all write rows — so this was reachable in normal use, not only on pathological issues. Take the window with the keyset ordering (created_at DESC, id DESC) in a subquery and re-sort ascending in the outer query. This keeps the chronological contract for every existing caller, including the comment list endpoint that shares ListCommentsForIssue, and is served as an index-only scan by the idx_*_keyset indexes already added in migration 068 — no new migration, no call-site changes. Two things beyond the ordering flip: - Clamp both lists to a shared window floor. The two caps are applied independently, so each list has its own floor. Merging windows with different floors produces a timeline that looks continuous but, below the higher floor, contains only one of the two kinds — e.g. comments with no interleaved activity. That is worse than a timeline that visibly stops, because nothing about it looks wrong. Both lists are now clamped to the newest floor, so the result is a contiguous, correctly interleaved slice. - Stop truncating silently. The unpaginated response is a bare JSON array with nowhere to put a flag, so the clamp is reported via X-Timeline-Truncated and X-Timeline-Window-From, added to ExposedHeaders because a custom response header is otherwise unreadable from browser JS. The legacy wrapped shape's has_more_before is now truthful instead of hardcoded false. Queries read one row past the cap so "hit the cap" is distinguishable from "holds exactly 2000 rows", which would otherwise report a complete timeline as truncated and drag the other list's window down with it. Regression tests cover all four properties and were confirmed to fail against both the original query and a floor-less DESC flip. Co-authored-by: multica-agent <github@multica.ai> * perf(realtime): stop broadcasting two full descriptions on issue:updated issue:updated carried prev_description alongside the new description in the issue object, and the WS forwarder reuses the producer's payload map verbatim. Every debounced description autosave therefore pushed two full copies of the description to every connection in the workspace, including users who did not have the issue open. The DB write is O(1); the fanout was O(connections x description size), and it repeats on every pause in an editing session. prev_description and prev_title exist only for in-process listeners — subscriber_listeners adds newly @mentioned users, notification_listeners builds mention notifications, activity_listeners records the title change. No client reads them: IssueUpdatedPayload in packages/core/types/events.ts does not declare either field. Project the payload on the way out. The bus dispatches bus.Subscribe handlers before the SubscribeAll forwarder, so the in-process consumers are unaffected, and projecting at the forwarder covers both the single- node Hub and the Redis relays since that is where the frame is serialized. The producer's map is copied rather than mutated. The removed keys are listed in a table rather than an if on one event type. The bug was structural, not a typo: the next large field added to a published payload inherits the same cost silently, and a declarative list puts the internal/external payload boundary in one reviewable place. issue.description itself is deliberately kept — clients apply it to their cache, so stripping it would trade fanout bytes for N refetches. Cutting the remaining fanout needs the per-issue scope routing already scaffolded server-side for MUL-1138, which is blocked on the client sending subscribe frames. Tests assert both halves: the keys are absent from the serialized frame, and the in-process listener still receives them. Co-authored-by: multica-agent <github@multica.ai> * fix(timeline): keep comment threads whole under the newest-N cap Review found that the newest-N window can orphan a reply, and an orphaned reply is invisible rather than merely mis-nested: the timeline builds its top level from "activities + comments with no parent_id" and renders replies by looking them up under their parent, so an orphan sits in the map with no card to render it. MUL-1847 / #2263 was exactly this shape — 1 root + 29 replies, root dropped, all 29 vanished from the UI while the API returned them. Root cause of the regression: capping with the OLDEST n could never orphan anything, because a reply is always newer than its parent, so a prefix of the timeline is closed under "parent of". A newest-n window is a suffix and has no such property. Flipping which end the cap bites silently invalidated a structural property the comment tree relies on. Two changes. Drop the cross-kind clamp. The previous revision trimmed both lists to a shared floor so the window was provably contiguous. That was the wrong trade and it was also the dominant source of orphans. Comments are human-paced (p99 ~30, max ever observed ~1.1k) and essentially never reach the cap, while activity is machine-paced and reaches it routinely — so the shared floor was almost always the activity floor deleting comments that had been fetched successfully and would have rendered fine. On an issue with thirty comments it was pure loss. Each list now reports its own truncation and X-Timeline-Truncated names which kinds were affected. Not clamping costs only activity density in the older part of the range, which is metadata rather than content, and it is reported rather than hidden. Complete parent chains for the case that remains — comments themselves exceeding the cap. ListMissingAncestorComments walks parent_id upward via a recursive CTE and returns the ancestors not already held; the handler merges them and restores the ascending order. This only ever ADDS rows, so unlike clamping it cannot hide anything the caller would have seen, and it is bounded by the number of distinct missing ancestors. Whole- thread windowing was considered and rejected: a single thread can exceed any row budget, so its degradation is not definable. Applied to the shared query's default list path too, not just the timeline. foldResolvedThreads documents a COMPLETE-thread set as its precondition and comment.go asserts the default list mode satisfies it; a half thread made that assertion false and a resolved thread whose root was cut stopped folding correctly. Also drops X-Timeline-Window-From. It was second-precision RFC3339 while the real ordering key is (created_at, id) at full precision, so it could not resume a read without skipping or repeating rows inside a shared second. A resumable cursor should be opaque and carry both halves; worth designing when there is a consumer rather than shipping as a lossy approximation. Tests: the reviewer's exact scenario, plus a no-orphaned-replies invariant on both endpoints, the fold-still-works case, and a guard that activity truncation does not delete comments. Each was confirmed to fail with the fix disabled. TestListTimeline_JointWindowHasNoOneSidedRegion was rewritten rather than deleted — it pinned the clamp behaviour being abandoned here, so leaving it would lock in the wrong contract and deleting it would drop the coverage. Co-authored-by: multica-agent <github@multica.ai> * fix(comments): bound parent-chain completion and stop folding partial threads Second review round on MUL-5492. Four must-fixes, all stemming from one conflation: parent-chain closure, a newest-N window, and a complete thread are three different things. Closure makes a reply renderable; it does not license thread-level derivations. Do not fold a truncated read. fetchCommentsForList closes parent chains, but older siblings and descendants of a retained reply stay outside the window, so the set holds partial threads. Folding them produced wrong answers rather than incomplete ones: a resolution reply outside the window made a resolved thread look unresolved, and folded_count reported a total derived only from retained replies. The previous revision claimed closure restored foldResolvedThreads' COMPLETE-thread precondition; it did not, and that claim is removed. --recent and untailed --thread still return whole threads and still fold. Bound the walk. The recursive CTE climbed to the root with no depth limit, so a deep chain could drag its entire ancestry back and defeat the row cap it was meant to preserve. Depth is genuinely unbounded in stored data: the general write path stores the exact comment being replied to (only the agent path collapses to the thread root), so chains can run far deeper than the two levels the UI renders. Replaced with a layered walk under explicit budgets — 2000 extra rows, 64 levels — making a response provably bounded by 4000 comments, or 6000 timeline entries with activities. Scope every level to the tenant. The CTE's recursive branch matched on parent_id alone. parent_id carries a foreign key to comment(id) but not to a matching issue, so a stray cross-issue parent reference is representable, and the walk would have followed it into another issue's comments. The replacement filters issue_id and workspace_id on every level. A negative test confirms the leak: with the filter removed it reports "a comment from another issue leaked into this issue's response". Degrade by pruning, not by orphaning. When a budget is exhausted, a parent row is missing, or a parent is out of scope, keepRootConnected drops the affected comments instead of returning replies the UI cannot render. Dropping a node also drops its descendants, since their chains run through it. Returning fewer new replies is conservative and already signalled as a truncated read; leaking another tenant's data, returning an unbounded response, or emitting invisible orphans are all worse. Also: probe read on the comment list so exactly-2000 is not misreported as truncated, which would needlessly suppress the fold; CommentsTruncated is carried on fetchCommentsResult rather than inferred from the result length, which is meaningless once completion adds rows. The new query returns db.Comment directly instead of a hand-copied row, which is how quick_action_id came to be dropped after the rebase — a backfilled quick-action root would have rendered as a raw prompt. Corrected three inaccurate comments: the "index-only scan" claim (the index avoids the sort but does not cover SELECT *), the "write path collapses replies to root" claim, and a test header still describing the abandoned contiguous-window behaviour. Tests cover exactly-at-cap still folding, truncated reads not folding (both reply-resolved and root-resolved), depth beyond budget pruned not orphaned, shared ancestors fetched once, cross-issue parents never crossing the boundary, and quick_action_id surviving backfill. Each was confirmed to fail with its specific fix disabled; the reply-resolved fold test was reshaped after the first version passed for the wrong reason. Co-authored-by: multica-agent <github@multica.ai> * fix(timeline): preserve complete threads under comment cap Co-authored-by: multica-agent <github@multica.ai> * fix(comments): preserve newest bounded views Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai> |
||
|
|
e6610c0831 |
fix(usage): close the per-agent rollup windows so the leaderboard cannot exceed the totals (MUL-5551) (#6194)
The Usage page showed a single agent with 1021.0M tokens under a workspace Tokens KPI of 805.9M for the same 1D window. Both halves read the same rows and disagreed only on the window. parseSinceParamInTZ deliberately returns N+1 calendar days of headroom, and the date-bucketed series (usage/daily, runtime/daily) get trimmed back to -(days-1) client-side before the KPIs and the chart are computed. The two per-agent rollups behind the leaderboard carry no date column, so nothing trimmed them and they kept the full N+1 span: at days=1 that is today PLUS yesterday. One busy agent's two-day total then trivially exceeded the workspace's one-day total. Same defect and same fix already applied to failures/by-agent: switch usage/by-agent and agent-runtime to parseExactSinceParamInTZ. This also realigns the Run time / Tasks KPI tiles, which are sourced from agent-runtime and were therefore a day wider than the Cost / Tokens tiles beside them. Co-authored-by: Eve <eve@multica-ai.local> |
||
|
|
0fdc38704e |
MUL-5149: add agent-generated Chat quick actions (#5766)
* feat(chat): add agent-generated quick actions
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): preserve mid-response quick-action fences
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): drop quick actions on empty reply to keep no_response fallback
An actions-only completion — a quick-actions footer with no visible text —
wrote an empty-content assistant message (message_kind=message). Older
Desktop/mobile clients ignore the quick_actions field and render that as an
empty bubble, breaking the MUL-4351 contract that an empty turn always gives
old clients a visible no_response fallback.
Drop the quick actions when the visible body is empty so an actions-only turn
falls through to the visible no_response outcome, and revert the completion
switch to gate the message row on visible text only. Update the completion
test to pin the corrected behavior.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
* feat(chat): generate quick actions via daemon suggestion pass
Replace the in-band runtime-brief instruction with a dedicated post-completion
provider turn: after a direct chat reply finishes, the daemon resumes the same
session with a JSON-only suggest prompt and forwards the raw output on the
complete callback. The server parses it leniently and reuses the existing
sanitize/redact/store/broadcast pipeline; the stripped in-band footer stays as
a fallback for older daemons and pre-upgrade sessions. The footer strip now
covers every chat completion, fixing the intro-turn protocol leak. Adds a
Settings → Chat toggle (client-persisted, default on) that hides the chips.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(chat): deliver quick actions async with skeleton placeholders
Decouple suggestion generation from the turn: the daemon reports completion
immediately (chat:done carries quick_actions_pending as a per-turn capability
signal) and runs the suggestion pass in the background, delivering results
through a new supplement endpoint + chat:quick_actions broadcast. A new turn
on the same session cancels the stale pass. Clients render pill skeletons
under the finished reply until the supplement resolves them (entrance
animation on arrival, 30s safety timeout); older daemons never raise the flag
so no skeleton dangles. Suggest usage re-reports merged totals because
task_usage upserts replace per (task, provider, model). Prompt now asks for
exactly 3 actions.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(chat): make the quick-actions toggle stop generation, not hide pills
The Settings → Chat toggle previously only hid rendered pills while the
daemon kept burning a suggestion call every turn. It now travels with each
send (quick_actions_enabled, absent = enabled for older clients), is stamped
on the chat task (migration 213), forwarded on the claim, and gates the
daemon's suggestion pass at the source — no call, no pending flag, no
skeleton. Existing suggestions stay visible; settings copy now says
'generate' instead of 'show'.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(migrations): renumber quick-action migrations onto current main
Merging current origin/main brought the vcs migrations to their canonical
216-221 prefixes, which collided with the quick-action migrations that were
sitting at 219/220 (backend CI red in
TestMigrationNumericPrefixesStayUniqueAfterLegacySet). Renumber them to the
next unused prefixes:
- 219_chat_message_quick_actions -> 222_chat_message_quick_actions
- 220_agent_task_quick_actions_disabled -> 223_agent_task_quick_actions_disabled
Contents are unchanged; sqlc regeneration produces no drift since the added
columns are independent of the vcs tables.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
* fix(mobile): render async chat quick actions via chat:quick_actions
The daemon generates quick actions in a background pass after the turn
finishes, delivering them on a separate chat:quick_actions event. Mobile
only handled chat:done (which invalidates + refetches an actions-less
message list) and keeps the messages query at staleTime: Infinity, so an
active mobile session never rendered async-generated quick actions until a
manual pull-to-refresh or refocus.
Add applyChatQuickActionsToCache — mirroring web's patcher — which patches
the supplement onto the targeted assistant message in the flat messages
cache, and subscribe to chat:quick_actions in use-chat-session-realtime.
Patch-only (no invalidate), matching web and mobile's cellular
patch-over-invalidate rule; an empty supplement is a terminal no-op. Covered
by chat-ws-updaters.test.ts.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): cancel in-flight messages refetch before quick-actions patch
The chat:done invalidate can leave a messages refetch in flight that read the
assistant row before the daemon persisted the quick actions. If that refetch
resolves after the chat:quick_actions setQueryData patch, it overwrites the
freshly-patched actions with an actions-less row. Both message caches are
staleTime: Infinity, so the overwrite never self-heals and the actions vanish
permanently (MUL-5149, Howard review).
applyChatQuickActionsToCache now awaits cancelQueries for the affected caches
(web: flat messages + messagesPage, mobile: flat messages) before patching, so
a stale in-flight refetch is cancelled and cannot land after the patch. Cancel
must precede setQueryData because cancelQueries reverts to the pre-fetch state.
WS handlers call it via `void` (fire-and-forget).
Adds an active-query race regression test on both web and mobile that holds a
refetch open across the supplement and asserts the patched actions survive;
verified to fail without the cancel.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
* feat(chat): quick-actions refresh/regenerate + review hardening (MUL-5149)
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): address quick-actions re-review (MUL-5149)
- Ack alignment: refresh request carries the target message_id; server
atomically confirms it is still the session's latest turn (409 stale
otherwise), so the client marker always matches the resolving
chat:quick_actions — no response reconciliation. Adds a regression test.
- Converge the pending marker on every terminal path: HandleFailedTasks
(sweeper/orphan) now resolves it, and the daemon reports a failed supplement
so FailTask resolves it instead of leaving a completed-but-unresolved task.
- Timeout fallback now clears the real query state (useQuickActionsPendingTimeout)
instead of a component-local flag that only masked the UI; drop the skeleton's
and pill row's local timers.
- frontend-test type-scale: text-xs -> text-caption. Strip EOF blank line.
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): close quick-actions refresh races and failure feedback (MUL-5149)
Third-round review of the refresh button surfaced three issues; all three
are addressed here.
§1/§2 Session-busy race + concurrent-refresh double-spend: a newer reply
that is queued/running but whose assistant row hasn't landed leaves the old
turn as latest-persisted, so the stale check passes and the regen resumes
the newer provider state — attaching suggestions to the wrong turn. And two
concurrent refreshes each enqueue a quota-spending pass. Add
HasActiveChatTaskForSession and refuse a refresh (ErrChatQuickActionsBusy →
409) whenever the session has any task in flight, checked under the same
session lock as the enqueue so no sibling insert slips past.
§3a Timeout re-arm on surface switch: the pending marker now carries an
absolute expires_at deadline instead of a per-mount timer, so switching
between the floating window and the chat tab resumes the same deadline
rather than restarting a fresh 30s window each remount.
§3b Generation failure masked as success: runChatSuggestPass now returns ok
so an explicit refresh distinguishes a failed pass (didn't start / didn't
complete / timed out) from a completed-but-empty one. On failure the regen
task reports failure, resolveFailedRegenerateQuickActions broadcasts a
FAILED chat:quick_actions, and the client resolves the spinner AND toasts
"couldn't refresh" instead of silently stopping on unchanged pills.
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): count deferred tasks in refresh busy check; solid refresh icon tone (MUL-5149)
Two re-review blockers on
|
||
|
|
5e3b7a8c37 |
feat(issues): Issue Quick Actions — preset agent + prompt, one-click from the sidebar (MUL-5465) (#6132)
* feat(issues): Issue Quick Actions — preset agent + prompt, one-click from the sidebar (MUL-5465)
Preset "who to call and what to say" once in Settings, then trigger it from
any issue's sidebar with a single click.
Running one is NOT a new dispatch path. The server renders the prompt, posts
a `quick_action` comment carrying the target's mention markup, and hands off
to the existing comment -> mention -> task trigger. Permission
(canInvokeAgent), attribution, squad-leader routing, the execution log, and
pending-task coalescing are inherited rather than reimplemented — the
MUL-3375 lesson about four drifting copies of one trigger decision.
Three things the UI has to be honest about, because the backend already
decided them:
- One pending task per (issue, agent) is a DB invariant
(idx_one_pending_task_per_issue_agent). A second click against a busy agent
starts no new run; the comment merges into the pending task. The toast says
"Added to Lambda's current run", not "Lambda started working".
- An offline target defers rather than fails; the run reuses the existing
dispatch.ReasonCode vocabulary instead of inventing one.
- Private agents are deny-by-default with no admin bypass. The sidebar filters
by the caller's own invoke verdict, so a dead button is never rendered, and
a direct API call still 403s with `invocation_not_allowed`.
Visibility is DERIVED from the bound agent's permission_mode on every request,
never stored — so it cannot drift after someone flips an agent between private
and public_to. Binding a workspace action to a private agent is allowed (the
alternative pressures people into making agents public just to satisfy a
config constraint) but the settings form says so at bind time, and the
catalog badges it. The target's name is withheld from callers who cannot see
it, so the response never discloses a private agent's existence.
Prompt templating is flat substitution over a closed whitelist. No
conditionals, loops, or filters — the agent already reads the whole issue, so
natural language is the control flow. One optional runtime input ({{input}})
keeps a single action from splitting into five near-identical variants; both
directions of the input/{{input}} agreement are rejected at write time so a
typo can never land silently.
Surfaces: sidebar (top 5, rest behind More), the `/` menu in the comment
composer (inserts the server-rendered body to edit before sending), and
Alt-click for the same hand-off from the sidebar.
Migrations 234-236: quick_action table, its listing index (CONCURRENTLY, own
file), and comment.type + comment.quick_action_id.
Co-authored-by: multica-agent <github@multica.ai>
* refactor(issues): simplify quick action permissions to a stored public/private intent (MUL-5465)
Replaces the derived four-value visibility model with a two-value choice made
at creation, and collapses permission handling to a single check.
The old model computed visibility per request from the bound agent's
permission_mode and used it to filter the sidebar. That filtering was the
problem: two people on one issue saw different sidebars with nothing to
explain the difference, which is harder to debug than a button that tells you
why it refused. It also required the list endpoint to run an invocation-target
query per action per request.
Now:
- `visibility` is stored INTENT — 'public' or 'private' — chosen up front.
- A public action must bind a target every workspace member can invoke
(public_to carrying a workspace target), enforced at write time. So a
public action is runnable by construction and dead buttons are eliminated
at the source rather than filtered out later.
- A private action allows any target and is returned only to its creator.
That scoping is what the field MEANS, not a permission check.
- Permission is checked in exactly one place: RunQuickAction. A refusal is a
structured 403 the client renders as one dialog. The dialog does not
distinguish "no permission" from "the binding drifted" — the person
reading it takes the same next step either way, and the person who can fix
it looks at settings.
Removed: can_run, position + manual ordering (settings sorted by usage while
the sidebar sorted by position — one list, two orders), the derived
visibility_broken flag, the runnable_only projection and its second cache
entry, target_name redaction, the alt-click composer hand-off (the `/` menu
covers insert-then-edit and is discoverable), and the sidebar_limit response
field (now a shared constant).
Ordering is use_count DESC everywhere. Settings shows the target's current
reachability as plain metadata ("Nova · private"), so a public action pointing
at a now-private agent reads as visibly wrong without a bespoke error state.
The tradeoff — no active signal when that drift happens — was accepted
deliberately: drift is rare and the failure is loud at click time.
Migration 234 is edited in place rather than layered, since the PR is
unmerged and the table has never been deployed.
Co-authored-by: multica-agent <github@multica.ai>
* refactor(issues): drop quick action variables and runtime input (MUL-5465)
V1 ships a preset prompt sent verbatim, triggered from the sidebar or the `/`
slash command. Two features are removed and one guard is kept.
Runtime input goes because `/` already covers it. Typing `/code review` drops
the rendered body into the composer, where any part of it can be edited before
sending — strictly more flexible than one fixed field, and the field was
specified before `/` was in V1. Two UIs for one need.
Variables go because none of them passed their own test. The rule was that a
variable earns its place only if it changes what the agent ATTENDS TO, not what
it KNOWS. Checked one by one — {{issue.title}}, {{issue.identifier}},
{{issue.url}}, {{user.name}}, {{date}} — the agent already has every one from
the issue context and from the fact that the comment is authored by the person
who triggered it. They were inherited from autopilot's title template rather
than justified.
The REJECTION survives the feature: any `{{...}}` is refused at write time,
naming the offending token. Someone carrying the habit over would otherwise
have `{{issue.title}}` rendered literally into an agent's instructions and
never notice — the exact silent-typo failure the whitelist existed to prevent.
The check is a fraction of the interpolation engine it replaces and keeps the
door open to enabling variables later without touching stored data.
Removed: 4 columns (input_enabled/label/placeholder/required),
renderQuickActionPrompt + the variable whitelist + quickActionIssueURL, the
two-way {{input}} agreement logic, the run/render `input` parameter, the
variable insert chips, the entire "Ask for input on click" block, and the
sidebar's Popover branch — every row is now a plain button. The settings
dialog drops from six field groups to four.
Migration 234 is edited in place rather than layered, since the PR is unmerged
and the table has never been deployed.
Co-authored-by: multica-agent <github@multica.ai>
* refactor(settings): align Quick Actions with the Labels/Properties list, then fix what the UI review found (MUL-5465)
The tab used a bespoke card list while its two siblings — Labels and
Properties — share one table layout. These three are the workspace's catalog
of small named things and should read as one surface, so Quick Actions now
uses the same structure: search + primary action row, bordered card, responsive
column grid that collapses to stacked rows under `md`, and an overflow menu
instead of a row of icon buttons. Columns are Name / Runs as / Who / Used /
Updated. The tab joins the max-w-5xl group for the same reason.
A UI review pass over the result found five things, four of which are fixed
here:
- The visibility chooser communicated selection through border and background
only, so a screen reader announced both options identically. Added
aria-pressed.
- The editor dialog was max-w-xl while both siblings use sm:max-w-lg, and the
unprefixed cap applied at every breakpoint.
- The empty-state hint diverged from the Properties tab it was copied from
(text-sm and no max width vs mx-auto max-w-sm text-xs).
- Two hardcoded `text-amber-600 dark:text-amber-400` usages replaced with the
`text-warning` semantic token, per the repo's design-token rule.
Also fixed a signal-quality bug the review surfaced: the usage column
highlighted anything with use_count 0, so an action was flagged the instant it
was created. Staleness now means "has had time to be used and wasn't" — 90
days since last use, or 90 days since creation for one never used.
Not fixed here: the overflow trigger is size-7 (28px), under the 44px touch
floor. Labels and Properties use the identical size, so changing only this tab
would break the consistency this commit exists to create; it needs one pass
across all three.
Co-authored-by: multica-agent <github@multica.ai>
* fix(issues): drop the quick_action comment type, widen the mention guard, harden the slash race (MUL-5465)
Second review round on PR #6132. All four remaining findings.
**Comment type removed entirely (#2 blocker + #3).** Adding a `quick_action`
type meant dropping and re-adding comment_type_check, and re-adding a CHECK
holds ACCESS EXCLUSIVE on `comment` for a full table scan — a read/write stall
on one of the hottest tables in the product, every deploy. It was also
forgeable: `type` is client-supplied on POST /comments, so any member could
post type='quick_action' and have an ordinary comment render as an action
audit record with its body collapsed out of view.
Both go away by not having the type. A quick action now posts an ORDINARY
comment marked with `quick_action_id`, and the collapsed card keys off that id.
There is no request field for it, so the marker cannot be forged, and the
migration is a bare nullable ADD COLUMN — metadata-only and instant. Verified
against a fresh database: comment_type_check is untouched.
The generic comment endpoint now also validates `type` instead of letting the
DB CHECK reject it. An unknown type surfaced as a 500 on a constraint
violation, which reads as a server fault for plainly bad input; it is a 400
now. `status_change` and `system` are excluded from what a client may author —
claiming those would be forging system narration.
**Member mentions rejected too (#1).** The first pass allowed
`mention://member/...` in prompts on the reasoning that it "only renders a
link". That was wrong: notification_listeners.go adds member mentions to the
recipient set and creates an inbox item, so a saved prompt pinged that person
on every single click. Only `mention://issue/...` reaches nobody and stays
allowed.
**Slash race, properly this time (#4).** The previous fix checked only that the
range still started with "/". Rewriting `/review` into `/fix` while the request
was open passed that check, and the stale response overwrote the new command.
The exact original text is now captured and compared; if the command was
edited, moved, or removed, the pick is abandoned rather than inserted
somewhere wrong. Adds the three regression tests the review asked for:
delayed resolve, rejection, and edit-during-flight.
Co-authored-by: multica-agent <github@multica.ai>
* fix(issues): stop the quick action card repeating its own prompt, and insert the `/` body as markdown (MUL-5465)
Two fixes, one reported and one found while verifying it.
**The card printed the prompt twice.** The collapsed header previewed the
prompt's first line, and expanding showed the mention line plus that same
prompt again. The header now identifies WHICH action ran — "Code Review via
Lambda" — which is both non-redundant and something the body never told you:
the prompt text alone does not say which action produced it. This is what the
original design called for; previewing the prompt was the implementation
drifting from it.
When the action cannot be resolved — deleted, or another member's private one
and so absent from this viewer's catalog — the header falls back to the
prompt's opening line, which is the previous behaviour.
**The `/` menu inserted its body as literal text.** insertContentAt was called
with a plain string, so Tiptap treated the server-rendered markdown as text
rather than parsing it. The mention never became a node; it serialised back out
with escaped brackets (`\[@Lambda\](mention://agent/…)`) and rendered as raw
markup in the thread. Passing `contentType: "markdown"` — the same option the
description editor already uses — parses it properly. Found by reading the
comment rows while checking the first fix: one had escaped brackets and no
quick_action_id, which is what a slash-inserted comment looked like.
The existing async test now asserts the contentType, so the option cannot be
dropped again without failing.
Co-authored-by: multica-agent <github@multica.ai>
* docs(issues): correct the stale quick actions sidebar comment (MUL-5465)
The comment still claimed the section renders nothing when no action is
runnable by the member. Permission filtering was removed several rounds
ago -- the list is deliberately unfiltered and a refusal is explained at
run time -- so the comment described behavior that no longer exists.
Co-authored-by: multica-agent <github@multica.ai>
* refactor(settings): cut the quick action dialog's helper copy in half (MUL-5465)
The dialog had five blocks of explanatory prose around four fields, and
three of them wrapped to two lines, so the form read as a paragraph with
inputs in it.
Each helper now earns its line or loses it:
- The header explained the implementation ("keeps the same history,
permissions, and execution log as an @mention") -- an architecture note
the person creating an action does not need. Reduced to the one fact
they do: it posts a comment.
- "Who can use it" is a question, so the hints answer it as noun phrases
("Everyone in the workspace" / "Only you") instead of restating the
verb. Both now fit one line, which also makes the two cards the same
height -- the shorter one used to sit in dead space.
- The target and prompt hints front-load the constraint rather than
burying it mid-sentence.
70 words to 32 across the dialog, with no fact dropped. Field spacing
goes 4 -> 5 so the gap between groups beats the gap inside one.
Co-authored-by: multica-agent <github@multica.ai>
* refactor(issues): render a quick action comment as an ordinary comment (MUL-5465)
The card had a collapsed one-line header that expanded to reveal the
prompt, on the theory that repeated runs of the same action would bury
the discussion. That was solving a problem the feature does not have:
prompts are a sentence or two, the header restated what the body already
said, and the disclosure only put a click between the reader and the
text.
A quick action posts a real comment through the real mention path, so
the honest rendering is the one every other comment gets. Drops
QuickActionCommentBody, its query for the action catalog, and the
now-orphaned quick_action_ran_via string in all four locales.
quick_action_id stays on the comment: it is provenance, and it was never
the reason the card looked different -- keying the special rendering off
it is what is going away, not the record itself.
Co-authored-by: multica-agent <github@multica.ai>
* fix(settings): use the faint tone token for the empty-state icon (MUL-5465)
main added apps/web/app/text-contrast.test.ts, a guard that rejects
transparency standing in for a text tone. The empty-state Zap used
text-muted-foreground/60, which is exactly the pattern it forbids: an
alpha-dimmed tone lands at a different contrast on every surface it is
composited over, so it cannot be reasoned about the way a token can.
text-faint-foreground is the token the guard names for icons and glyphs.
The rule arrived on main after this branch's last merge, so local runs
never saw it -- CI tests the merge commit, which is why only CI caught
it. Merged main first so the branch is checked against the same rules.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Lambda <lambda@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
|
||
|
|
f0110da555 |
feat(inbox): mark a notification unread from the row context menu (MUL-5496) (#6137)
The inbox auto-marks a notification read the moment it is selected, so
"opened" and "handled" were the same signal — a row you glanced at and
meant to come back to was gone from the unread count with no way back.
Right-click any inbox row for a shared context menu: Mark as read /
Mark as unread, plus Archive (Unarchive in the archived view).
- POST /api/inbox/{id}/unread + MarkInboxUnread query, publishing
inbox:unread. Item-scoped, mirroring mark-read: the list renders one
row per issue carrying that group's newest item, so flipping the whole
group would resurrect siblings the user already dealt with.
- useMarkInboxUnread patches both lists optimistically and re-pulls the
cross-workspace unread summary on settle.
- One shared menu per list rather than a Base UI root per row (the same
shape IssueContextMenuProvider uses): only one is ever open, and a
per-row root would unmount with its menu when the row scrolls out of
the virtualized viewport.
- The read toggle is main-view only — archived rows deliberately render
as read and the unread count excludes them, so a toggle there would
report success and change nothing on screen.
- Parking the row that is currently open holds the auto-read effect off
that one item while it stays selected; re-opening it later marks it
read again.
- Mobile subscribes to inbox:unread so the unread dots agree across
clients.
Co-authored-by: Lambda <lambda@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
|
||
|
|
9072cef12c |
Revert "MUL-5493: feat(chat): add a visible follow-up queue (#6133)" (#6171)
This reverts commit
|
||
|
|
b13657be71 |
MUL-5493: feat(chat): add a visible follow-up queue (#6133)
* feat(chat): add a visible follow-up queue Add a visible, manageable FIFO follow-up queue for Web and Desktop chat while preserving the existing per-session scheduler and backward-compatible pending-task response. * fix(chat): preserve queue after deferred cancellation --------- Co-authored-by: TeAmo <liu.junhui3@iwhalecloud.com> |
||
|
|
30318b79bc |
MUL-5426: fix(daemon): retire sessions whose history the provider refuses to replay (#6083)
* fix(daemon): retire sessions whose history the provider refuses to replay A run killed mid-reply (machine shutdown, force-quit, SIGKILL) can leave an empty assistant message in the agent CLI's transcript. Every later resume replays it, the provider rejects the request, and the (agent, issue) pair is bricked with no self-healing and no user-facing recovery. Multica already has the mechanism for this — poisoned-session classification — but its detector paired "400" with "invalid_request_error", which is the Anthropic wire shape. The same defect reported by any other provider carried neither token, so it classified as agent_error.unknown: resume-safe by omission. GetLastTaskSession kept handing back the dead session on every follow-up, manual Rerun resolved it through the same predicate, and the in-turn fresh-session retry never fired because ResumeRejected is false here (nothing rejected the resume — the transcript loaded and the provider refused to replay it). Add taskfailure.UnresumableHistory, which recognises the defect by what the provider says is wrong — some content is empty, and here is which message in the history — rather than by status code or provider name. Both signals are required, so a tool reporting "field must not be empty" does not match. Wire it into the four places that decide whether a session survives: - classifyPoisonedError, so the task is written as api_invalid_request - shouldRetryWithFreshSession, so the turn recovers on all 17 backends instead of the subset whose adapter learned to detect it; the tools == 0 gate is unchanged, so a run that already used a tool is never re-run - ResumeUnsafeFailure, covering the manual-Rerun path - both resume queries, as defense-in-depth for hosts whose daemon predates this (self-host daemons upgrade on their own cadence) Fixes #6066. Also covers the daemon half of #5760. Co-authored-by: multica-agent <github@multica.ai> * fix(session): close the Chat and fresh-retry paths that resurrect a poisoned session Review found the previous commit stopped short in two places, both of which put the dead transcript back in play. Chat never consulted the guarded query. The claim handler reads chat_session.session_id first and only falls back to GetLastChatTaskSession when it is empty, so a poisoned pointer there bypasses every filter that query applies. The fail path merely declined to OVERWRITE the pointer, leaving it in place. It now clears it in the same transaction, matched on session and runtime so a concurrent turn's newer pointer survives. The promote guard moves to ResumeUnsafeFailure as well — the reason-only check passed an un-upgraded daemon's agent_error.unknown row and re-pinned what the clear had just removed. GetLastChatTaskSession also kept the row-level filter the issue query dropped in GH #5975: it discarded the newest poisoned row and fell back to an older completed row carrying the same dead session. It now judges each session by its latest terminal state, matching GetLastTaskSession. A recovered turn could not retire anything. A terminal report carried one session_id, and an empty one meant both "nothing to report" and "forget the old session", so a fresh-session retry that SUCCEEDED left the id it retried away from selectable — through an older completed row on the issue, or through the chat pointer. agent_task_queue.retired_session_id records the abandonment itself, reported on every terminal path including completed, and both resume lookups exclude it. This is the contract gap the previous PR deferred; the fresh-retry path now runs on all backends, so deferring it is not safe. Also narrows what the cross-backend test claims: it pins the shared decision, not that all 17 adapters surface the error into Result.Error (#5760 is the counter-example), and says so. Co-authored-by: multica-agent <github@multica.ai> * test(session): require pgx.ErrNoRows in the resume-exclusion assertions The `if err == nil && prior.SessionID.Valid` form these tests shared is false-green: any real fault — undefined column, syntax error, dead connection — makes err non-nil, so the condition is false and the test passes. Run against a database missing this branch's new column, the exclusion tests reported PASS on a SQLSTATE 42703, meaning they could not have caught a broken query. requireSessionExcluded demands pgx.ErrNoRows specifically and fails loudly on anything else, so a green run now means the filter worked rather than the query never ran. Applied to all nine sites, not just the four this branch added: the other five guard the same GetLastTaskSession exclusion behaviour that this branch changes, so leaving them false-green would leave the change under-tested. All nine pass on a correctly migrated database. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
|
d4dac0e77c |
perf(agents): index-backed latest-terminal lookup in task snapshot (MUL-5436) (#6085)
ListWorkspaceAgentTaskSnapshot took each agent's latest completed/failed
task with a workspace-wide DISTINCT ON, so every presence load read and
sorted the workspace's whole terminal history. Neither existing index
matches that shape: (agent_id, status) has no completed_at, and migration
231's (completed_at) partial index is completed_at-first for the Usage
rollups.
Replace the outcome half with a per-agent JOIN LATERAL Top-1 and add a
partial index on (agent_id, completed_at DESC NULLS LAST, created_at DESC,
id DESC) WHERE status IN ('completed','failed'). On a 40-agent workspace
with 200k terminal rows this goes from 6631 shared buffers / 48.3 ms to
162 buffers / 0.1 ms, with an identical row set.
The (created_at, id) tie-break also makes the pick deterministic when
completed_at ties or is NULL — completed_at DESC alone left the winner up
to the plan.
Report #6075 asked to delete the outcome half as dead code, but PR #2608
made the Squad hover card (AgentLivePeekCard) read those rows for its
"last activity" line, so removing them would be a product regression for
shipped desktop builds. The response contract is unchanged here; splitting
the outcome into a lazy endpoint stays follow-up work.
Also tighten pickLatestTerminal to completed/failed only, matching the
snapshot's filter — it accepted cancelled, which the endpoint never
returns and which would have masked an agent's last real outcome.
Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
|
||
|
|
2e9a3d0119 |
fix(dashboard): stop leaking private agents from the per-agent rollups (MUL-5409) (#6051)
* fix(dashboard): stop leaking private agents from the per-agent rollups (MUL-5409) Three per-agent dashboard endpoints authorized on workspace membership alone and returned a bare agent_id for every agent in the workspace: GET /api/dashboard/usage/by-agent GET /api/dashboard/agent-runtime GET /api/dashboard/failures/by-agent That told a plain member which private agents exist, how much they spend, how long they run and what they fail on. The client already collapsed those rows, but client-side filtering is decoration — one curl bypasses it. Server: rows for agents the caller may not view are now folded onto a `__restricted_agents__` sentinel before serialization, via one shared helper. Folded, not dropped: each of these responses is the per-agent half of a pair whose other half (usage/daily, runtime/daily, failures/daily) is workspace- scoped and unfiltered, so dropping rows would make the per-agent breakdown stop adding up to the KPIs rendered beside it. The bucket keeps its provider/model and failure_reason dimensions — both are derivable by subtraction from the workspace-level series anyway, and the client needs them to price the bucket and compute its failure rate. Owner/admin and agent actors short-circuit before any extra query, so the governance view is unchanged. Hard-deleted agents are deliberately excluded from the fold — they have no visibility left to protect and keep their own bucket. Client: fixes the mislabelling that shipped with this. A live private agent was folded into a row labelled "Deleted agents" with a bin icon, and counted into the card's "· N deleted" caption — telling the user N agents were deleted when they are alive and still running. The restricted bucket is now its own row with neutral copy, keeps its real Time / Tasks values, and counts as neither an agent nor a deletion in the caption. Tests: handler regression coverage proving a plain member's response contains no private agent UUID while every aggregate still sums to the privileged view's total, plus view coverage for the label and caption. Co-authored-by: multica-agent <github@multica.ai> * fix(dashboard): fold hidden system agent carriers into the restricted bucket (MUL-5409) Review follow-up. The first pass built the restricted set from ListAllAgents, which filters `kind = 'user'` — so it missed the hidden `kind = 'system'` execution carriers behind agent-builder sessions. Those carriers run real tasks and book real usage, and all three rollups aggregate over agent_task_queue / task_usage with no kind filter of their own. No list endpoint returns them either (ListAgents / ListAllAgents both filter on kind), so no client can resolve one to a name. Net effect: the exact two bugs this PR exists to fix, still live — a bare UUID exposing one member's builder session (with its spend and failure profile) to every other member, and, once the agent list loads, a running agent folded into the client's "Deleted agents" row and counted as a deletion. restrictedAgentIDs now reads a new ListAllAgentsAnyKind and restricts every non-user-kind agent for EVERYONE, workspace owner included — nobody can name one, so a bare UUID row is wrong for every viewer, not just plain members. User agents keep the per-viewer visibility rule. The invocation-target lookup is skipped for actors that rule can never restrict (agent actors, owner/admin), so the added cost is one indexed list query. Because the bucket now also carries carriers that are nobody's "restricted" agents, its copy drops to the neutral "Other agents" — the same wording the Errors card already uses for its equivalent row, in all four locales. Adds a regression test seeding a kind=system private carrier with tasks and usage: no endpoint may return its UUID to either the plain member OR the workspace owner who owns it, a bucket must be present to carry its rows, and every metric delta (tokens, seconds, tasks, failures, runs) must equal its exact contribution. Verified to fail on all three endpoints for both viewers with the kind-filtered query restored. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
|
2274f521dc |
feat(issues): agents-working chip on the sub-issues header (#5825) (#5834)
* feat(issues): aggregate agents-working chip on the sub-issues header (#5825) Add a live "N agents working" chip next to the sub-issues progress ring in issue detail. The per-row IssueAgentActivityIndicator shows which sub-issue is being worked; this chip shows how many agents are on the parent's children at a glance — and keeps that signal visible while the list is collapsed. Derives from the shared workspace agent-task snapshot narrowed by a new selectIssuesTasks select (structural sharing keeps unrelated snapshot churn from re-rendering the header). Counts unique agents to match the workspace chip, whose chip_agents_working / hover_header_queued strings it reuses — already translated in every locale. Hover opens the shared AgentActivityHoverContent task list. Fixes #5825 Co-authored-by: multica-agent <github@multica.ai> * refactor(issues): read the sub-issues chip from the working-agents projection (#5825) The chip landed deriving its own count from the workspace agent-task snapshot, which put a second definition of "an agent is working" in the client. It showed up immediately: the number came from the running tasks only while the hover body listed running plus queued, so a parent with 2 running and 3 queued agents read "2 agents working" over a five-row card. A header count is a claim about a scope, so let the server own both the scope and the arithmetic, exactly as the Issues list header already does. ListWorkspaceWorkingAgents grows an optional parent_issue_id narrowing and the chip reads /api/working-agents?type=issue&parent=<id>. The number, the avatars and the hover body are now one list rather than three derivations, so they cannot disagree. Row indicators keep reading the snapshot. One shared query sliced per row is the right shape for a per-row cue and a stale row decoration costs nothing; a header number is the opposite, it has to be authoritative. The new parameter is additive: omitted, the query and the response are byte-for-byte what they were, so an installed client that never sends it keeps the workspace-wide behaviour. A regression test pins that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Lambda <lambda@multica.ai> Co-authored-by: multica-agent <github@multica.ai> Co-authored-by: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
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. |
||
|
|
4d0475ce89 |
feat(usage): error/failure charts on the Usage page (MUL-5352) (#5991)
* feat(usage): add error/failure visibility to the Usage dashboard The Usage page could only answer "how much did we spend"; nothing on it showed how often agents fail, what kind of failure it was, or which agent is responsible. Operators had to open failed tasks one at a time to spot a pattern. `agent_task_queue.failure_reason` already carries the refined 21-value taxonomy from server/pkg/taskfailure, so this is a read path over data that already exists. Backend — two rollups, both scoped by workspace/project/window like the existing dashboard endpoints: GET /api/dashboard/failures/daily per-(date, failure_reason) GET /api/dashboard/failures/by-agent per-(agent, failure_reason) They return every terminal task, not just failures: the `failure_reason: ""` row carries the succeeded count. That is what makes the error rate's denominator share filters with its numerator. The run-time rollups can't serve as that denominator — they require `started_at IS NOT NULL`, so a task that expired in the queue (the signature of a runtime outage) contributes nothing to their failed_count. A failed row with an empty reason column lands in an `unclassified` bucket rather than being mistaken for a success. Frontend: - "Errors" joins the trend toggle, daily and weekly, stacked by failure class with the bucket's error rate in the tooltip. - An Errors card breaks the window down by class and by agent, with the raw failure_reason strings behind a disclosure (unlocalised — an operator pastes them into a log search). Each agent row links to its Work tab, which lists the actual failed runs. - The 21 backend reasons fold into 7 display classes in @multica/core/dashboard. Unknown reasons — including ones from a backend newer than the client — land in "other" instead of being dropped, so the class totals always reconcile with the failure count. The Tasks KPI tile is deliberately left alone: its value counts started tasks only, so quoting the failure rollup's larger count there would put two denominators in one tile. The Errors card states its rate with the denominator spelled out instead. Migration 225 adds a partial index on agent_task_queue(completed_at) for terminal statuses. The table had no completed_at index at all, so the two pre-existing run-time rollups were already scanning it; these two new queries would have doubled that. Closes #4429 (MUL-5352) Co-authored-by: multica-agent <github@multica.ai> * fix(usage): correct the Errors drill-down, window and agent exposure Review findings on PR #5991. 1. The drill-down pointed at the wrong page. `?view=work` renders ActorIssuesPanel — the issues assigned to the agent — while its runs live in the Overview pane's ActivityTab. Link to Overview. That page also could not show why a run failed: `failureReasonLabel` was a `Record<TaskFailureReason, string>` indexed with a cast to the old 6-value coarse enum, so every refined reason the backend has written since MUL-1949 resolved to `undefined`. It is now a function over the full 21-value taxonomy plus the legacy coarse values, falling back to the raw wire string for anything newer than the client. Fixes the issue execution log too, which had the same cast. 2. The Errors card covered one more calendar day than the chart above it. `parseSinceParamInTZ` returns N+1 days of headroom on purpose and the dashboard trims the surplus client-side — but only a series carrying a date can be trimmed that way. Totals / classes / reasons now derive from the date-bucketed rollup after that trim, and the per-agent rollup (which has no date to trim on) closes its window server-side via a new `parseExactSinceParamInTZ`. At days=1 the card previously reported yesterday's failures beside a chart showing none. 3. The top-offenders list leaked agents the viewer cannot see. The failure rollups are workspace-scoped and deliberately skip per-agent visibility, but the agent list they are joined against does not — members only see a private agent when they own it or are owner/admin. `name ?? row.agentId` therefore rendered a bare UUID along with that agent's failure count, rate and dominant error class. Unresolvable agents now fold into one anonymous row, and the renderer never falls back to an id. Stricter than `bucketUnknownAgentRows` while the agent list loads: a transient flash of UUIDs is the leak, not a cosmetic glitch. Also from the review: the Errors tooltip echoed the raw Recharts dataKey ("rate_limit") instead of the translated label the legend already carries. Not changed — the schema's `failure_reason` default stays `""`. Defaulting a missing field to a failure bucket guards against a deflated rate, but the realistic drift is `omitempty` on the Go struct tag, which would strip the field from exactly the SUCCESS rows and read as a 100% error rate. Added TestDashboardFailureWireContractKeepsEmptyReason to pin that the server always emits the field, which is the assumption the default rests on. Co-authored-by: multica-agent <github@multica.ai> * fix(usage): renumber migration and fix the anonymous bucket's failure class Review findings on PR #5991, round 2. 1. Migration prefix 225 collided with `225_chat_message_channel_media_pending`, which landed on main while this branch was open — backend CI failed on TestMigrationNumericPrefixesStayUniqueAfterLegacySet. Merged main and renumbered to 231; main now carries 225 through 230, so 226 is taken too. 2. The anonymous "Other agents" bucket could announce the wrong failure class. It merged rows that had ALREADY collapsed to one dominant class per agent, then credited each agent's entire failure count to that class. An agent failing auth 6 / timeout 5 contributed 11 to auth and 0 to timeout, so a bucket whose real composition was timeout 15 / auth 6 rendered as Auth. Fixed by anonymizing the raw per-(agent, reason) rows instead: the sentinel becomes just another agent_id and `aggregateAgentFailures` computes its classes from real counts. That also deletes the parallel bucketing pass — one identity rewrite replaces it. `knownAgentIds` moves up to where both consumers can see it. Also from the review: - The wire-contract test decoded both payloads into one map. json.Unmarshal merges into a non-nil map rather than resetting it, so a residual failure_reason from the first case could have masked an omitempty regression in the second — exactly what the test is meant to catch. Now table-driven with a fresh map per case. - A test comment still described the drill-down as pointing at the Work tab. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
|
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> |