mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-10 02:16:45 +08:00
main
156
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
16f0d307dd |
fix(realtime): announce comments folded into a queued run (#9702)
A reply posted while the agent already has a queued run in the same thread is folded into that run on the server: the reply becomes the run's trigger and the earlier comment joins its covered set. The run's status does not change, so no task event fired and open issue pages kept the run under the earlier comment, leaving the new reply with no visible queue row. Publish task:coalesced after the merge (and after a delegated-failure merge). Web and desktop already refresh the issue's runs on any task: event, which moves the queued block under the newest comment it answers; mobile now listens for it too. Co-authored-by: Cursor <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
Multica Eveandmultica-agent
|
f2a6398d6e |
MUL-7257: refactor(server): remove unused compatibility queries and capabilities (#9671)
* refactor(comments): drop redundant RFC3339 fallback parse RFC3339Nano parsing already accepts timestamps without fractional seconds, so the second time.Parse(time.RFC3339) attempt can never succeed where the first failed. Keep the existing 400 responses and add a test pinning second-precision since and the invalid since/before cases. Co-authored-by: multica-agent <[email protected]> * refactor(daemon): stop advertising unread plugin capabilities execution-manifest-v1, agent-skill-v1 and remote-mcp-v1 have had no server-side reader since the Plugin V1 gates were removed in #7144. Drop the constants and the daemon advertisement. Co-authored-by: multica-agent <[email protected]> * refactor(autopilot): remove legacy no-reservation terminal queries UpdateAutopilotRunCompleted, UpdateAutopilotRunFailed and UpdateAutopilotRunSkippedWithResult have no production callers; terminal transitions go through UpdateAutopilotRunTerminalWithQuota. Their generated Params structs were still used as service argument types, so replace them with service-local structs. The reconciler test now seeds its crash window with an explicit fixture UPDATE. Co-authored-by: multica-agent <[email protected]> * refactor(tasks): remove unused StartAgentTask query Production start paths use StartAgentTaskWithSupplement. Move the seven test fixtures to the same query with both capability flags off, which performs the same running transition. The empty-body start handler branch is unchanged. Co-authored-by: multica-agent <[email protected]> * refactor(channels): remove unused pending_fresh queries MarkChannelChatSessionPendingFresh, LockChannelChatSessionPendingFresh and ClearChannelChatSessionPendingFresh have no callers. The context generation path and ClearChannelChatSessionPendingFreshForRevision remain. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-agent <[email protected]> |
||
|
|
bb3747186e |
feat: let agents steer running turns they could start (MUL-8285) (#9630)
* feat(server): let agents steer running turns they could start (MUL-8285) An agent comment can now be added to another agent's running turn, the way a member steers from the composer: exactly when the comment could start that agent (canInvokeAgent, judged on the human at the top of the chain), never its own turn. - steer_task_ids now work for agent authors; steer: true asks the server to add the comment to the current running turn of every recipient without a named turn. - Agent-authored bindings are capped per turn (10) and per authoring agent (3). The task row is locked in its own statement before the bind, so the cap count sees what an earlier lock holder committed. - An agent comment that steered a turn but was never read is replayed on completion like the mention it replaced. Member receipts keep today's behaviour (the member sees the failure and can retry or resend). - The supplement claim returns author_type and the agent's name. A daemon that does not send task-supplement-author-v1 gets the attribution in the content, so an agent message never reads as a human's. - can_supplement on task-runs is computed for an agent caller by the same rule. Co-authored-by: Tim Smart <[email protected]> Co-authored-by: WitMiao <[email protected]> Co-authored-by: multica-agent <[email protected]> * feat(daemon): present an agent's added message as an agent's (MUL-8285) Claims send the task-supplement-author-v1 capability and render agent messages as peer input that cannot replace the objective or override humans. Co-authored-by: Tim Smart <[email protected]> Co-authored-by: multica-agent <[email protected]> * feat(cli): add --steer and --steer-active to issue comment add (MUL-8285) Docs: CLI help, CLI_AND_DAEMON.md, the CLI docs in every locale, the squad leader briefing, and a steering section in the multica-platform mentions reference. Co-authored-by: Tim Smart <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix: close the review gaps in agent steering (MUL-8285) - Replay an unread agent steer even when it reached the turn through an implicit route (a worker's reply to its leader): a comment bound to the completing run counts as planned input for the replay filter. - Drop the receipt of an agent steer once completion has replayed it, so no client (old or new) offers to resend what the follow-up already handles. - Never hand an agent's message to a daemon without task-supplement-author-v1: it would frame it as a human's. The message stays pending, settles when the turn ends, and is replayed as a mention. - Tell a squad leader about --steer only when its daemon advertises cli-comment-steer-v1, since an older CLI rejects the flag; the skill reference says to drop the flag on 'unknown flag'. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Tim Smart <[email protected]> Co-authored-by: WitMiao <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
1eb4b5d1f8 |
MUL-8203: record permission-blocked mentions as system replies (#9624)
* fix(comments): leave system replies for denied mentions (MUL-8203) Co-authored-by: multica-agent <[email protected]> * fix(comments): address refusal notice review nits (MUL-8203) Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Yushen <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
Multica Eveandmultica-agent
|
72d9058cc6 |
feat(telemetry): add visiting and delegating member counts (MUL-8161) (#9435)
* feat(telemetry): report visiting and delegating member counts in self-host snapshot v2 (MUL-8161) Co-authored-by: multica-agent <[email protected]> * fix(telemetry): count delegating members from a human-action ledger, drop v1 fallback (MUL-8161) - member_delegation_day records (member, UTC day) at the human action itself, so retries, system wakeups and creator-fallback runs never count, and a comment folded into a queued run keeps both humans and their own days. - Activity read failures now fail collection and use the existing retry instead of silently sending a v1 snapshot. Co-authored-by: multica-agent <[email protected]> * fix(telemetry): record create-with-assignee, honor DO_NOT_TRACK, build ledger PK concurrently (MUL-8161) - A member creating an issue assigned to an agent or squad now records a delegation day, on both the immediate and deferred create paths. - TaskService.RecordMemberActivity gates the ledger and is set from the startup telemetry config, so DO_NOT_TRACK stops collection too. - member_delegation_day's primary key is built with CREATE UNIQUE INDEX CONCURRENTLY + PRIMARY KEY USING INDEX (589-591), with cleanup registered. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-agent <[email protected]> |
||
|
Jiayuan Zhangandmultica-agent
|
231dedb552 |
MUL-8034: chore: clean historical comments across the codebase (#9274)
* chore: clean historical comments (MUL-8034) Co-authored-by: multica-agent <[email protected]> * docs: preserve pending Composio expiry contract Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-agent <[email protected]> |
||
|
|
e7742b052b |
MUL-7776: fix(api): stop returning raw error text in 500 responses (#8905)
* fix(api): stop returning raw error text in 500 responses 39 handlers answered a 500 with the message followed by err.Error(), sending database and store errors (constraint names, SQL states, Redis errors) to API clients, while the other 720 500 responses in the handler package send a fixed message. These now send their fixed message too and log the error with the request's attributes, as the handlers that already logged it do. * fix(api): drop raw error text from the task callbacks and skill import 500s completeTask and failTask sent err.Error() as the whole 500 message, and the structured skill import put it in the result's Reason on its three 500s. They now send fixed messages and log the error. The task callbacks keep their 500, so the daemon still retries them: isTransientError reads the status code, never the body. * fix(api): close remaining raw error leaks Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Yushen <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
e2f4a22030 |
feat: steer a running agent from the normal composer, per recipient (MUL-7631) (#8760)
* feat(server): steer running turns from ordinary comments (MUL-7631) A comment can now name recipients whose running turn should receive it (`steer_agent_ids` on POST /issues/:id/comments). Each named recipient with a running, steer-capable turn gets the comment bound to that turn instead of a follow-up run; any other recipient keeps its normal queued / coalesced / deferred trigger, so losing a race never drops input. - One comment can steer several turns: receipts are keyed by (comment_id, task_id) and returned as `supplements` on comments and timeline entries. The single supplement_* fields mirror the first receipt for older clients. - Completion reconciliation skips a steered comment only for the agent it steered, so the same comment still reaches recipients that were addressed without steering. - Delivery claims match the (comment, run) pair. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * feat(issues): choose per recipient how a message reaches a running agent (MUL-7631) Steering moves from a separate "Add message" box on the run row into the normal reply and comment composers. Recipients keep the existing trigger rules; each recipient's current run on the issue decides what the message can do: - running (and steer-capable): add to the current run (default in that run's own thread), start after this run, stop and start over, or skip - queued: include when it starts, or skip - idle: start on send, or skip Receipts follow the message ("Waiting for Lambda to read it" → "Read by Lambda · after step N"), a failed delivery offers retry or sending it as a new run, and the run's step list marks where it read the message. A draft whose recipient finished meanwhile stays in place and says it will start a new run; a message with files is handled after the run. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(server): steer only the chosen turn, and make a retried steer idempotent (MUL-7631) - Steering names the exact running turns the author chose (`steer_task_ids`). A chosen turn that ended before the send arrives is never swapped for a later turn of the same agent; that recipient keeps its normal trigger. - A steering send carries `client_request_id`. A retry after a lost response returns the original comment (200) instead of creating a second comment and a second delivery; a concurrent twin binding the same request is treated as already steered. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(issues): guard steering sends against stale previews and broad resends (MUL-7631) - "Stop and start over" never stops a run while the trigger preview is still catching up with an edited @mention: the stop is dropped unless the preview answers the submitted text. - Steering sends the chosen turns' task ids and one request id per draft text, reused on retries of the same text. - "Send as a new run" on a failed receipt reaches only that receipt's agent: every other current recipient is suppressed. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(issues): keep the reply box placeholder neutral (MUL-7631) The empty reply box cannot know who a reply goes to: the author may @ someone else or only talk to a teammate. The recipient chip already says what Send will do once there is text. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * test(issues): give steering composer tests room for the preview debounce (MUL-7631) Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * feat(issues): choose the default reply to a running agent (MUL-7631) Preferences > Comments gains "When replying to a running agent": "Add to current run" (default) or "Start after this run". It only changes what a reply does without a per-message choice; the recipient chip still offers both, and replies that never steered by default (another thread, an idle or queued agent) are unaffected. Saved on this device next to the sticky comment bar. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(server): save one comment per send and honor "don't start" on replay (MUL-7631) Idempotency now lives on the comment, not the delivery receipt. A send's client_request_id is stored on the comment behind a partial unique index (migrations 550/551), so a retry, or a concurrent twin, returns the saved comment (200) whether the send steered a running turn or fell back to a normal trigger. Before, a twin could save a second comment, and a send whose turn had ended kept no receipt to find. The agents an author chose not to start are stored on the comment too, and completion replay skips them. "Send as a new run" for one missed recipient, and "Won't start this time", no longer wake the other agent once its current run completes. An edit re-records the choice. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(issues): repeat the original steering request on retry (MUL-7631) Until a send is settled, retrying the same text sends its first steering request unchanged, even after the chosen turn ended. The server then returns the comment the first attempt saved instead of posting it again as new input. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(migrations): fail fast on locks in 549 and 550 (MUL-7631) Both take an ACCESS EXCLUSIVE lock on a table the product touches continuously: comment for every read and write, task_supplement for every daemon claim. Bound lock acquisition and execution, as 483 does for issue, so a long transaction makes the migration fail and retry on the next run instead of queueing all traffic behind it. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(server): steer at most one turn per comment during rollout (MUL-7631) Steering is live, and servers from before per-run receipts claim every receipt of a comment at once. While they still serve daemons during a rollout, a comment bound to two turns could have the second copy marked in flight and never delivered. Until every server claims per (comment, run), a comment steers the first chosen turn only; later recipients keep their normal trigger. A follow-up lifts the limit after this ships. Also covers a steer_task_ids entry that names another issue's turn. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(server): finish a saved send on retry; keep an edit's choices with its text (MUL-7631) A send's comment was saved before its agents were started or steered, and deduplication treated the saved comment as the whole send. A dropped connection cancelled the request between the two, and the retry only returned the comment, so no agent ever received it. - Attempts of one logical send now run one at a time under a transaction advisory lock, held on its own connection: a twin waits for the attempt in progress (up to 30s, then 409) instead of racing it, and a process that dies releases it. - After the comment is saved, dispatch runs on a non-cancelable context, and comment.client_request_dispatched_at (migration 552) records that it finished. A retry that finds a saved but undispatched comment finishes the dispatch with the comment's recorded choices. An edit's "don't start" choices were written by a separate statement after the text, so interleaved edits could leave an older edit's choice. They are now part of UpdateComment, under the same revision check as the text. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(issues): offer one steered recipient per message during rollout (MUL-7631) The server steers at most one turn per comment until #8800, but the composer showed "Add to current run" for every running recipient and the server quietly queued the rest. Now only one recipient steers: the one the author picked, otherwise the first that would by default. The others show "Start after this run", and picking "Add to current run" on one of them moves the steer there. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(server): claim a send before dispatching it; drop the request lock (MUL-7631) The previous recovery marked a send dispatched only after reaching its agents, so a failure after dispatch made a retry reach them all again. It also held a pooled connection per send for an advisory lock while the send needed more from the same pool, which could wait on itself until timeout. A send's comment now holds a dispatch claim instead (comment.client_request_dispatched_at, one conditional update): only the attempt whose claim takes effect publishes the comment and reaches its agents. A failed claim writes nothing else and returns 500 for the client to retry; a retry that finds an unclaimed comment claims and dispatches it; a claimed one is only returned. Concurrent attempts no longer wait on each other, and post-save dispatch keeps a 30s bound of its own. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * feat(server): let one comment steer several running turns (MUL-7631) Lifts the rollout limit from #8760. Every server now claims receipts per (comment, run), so a comment can be bound to each running turn its author chose, and each recipient receives it in its own run. Merge only after #8760 is deployed everywhere. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * feat(issues): steer every running recipient a message addresses (MUL-7631) The client side of lifting the rollout limit: each running recipient can take the message in its current turn again, instead of only one per message. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * refactor(issues): drop send-retry idempotency and two display extras (MUL-7631) A steering send now behaves like any other comment when its response is lost: a retry posts it again. The machinery that tried to make that retry return the saved comment grew with every edge it met, and ordinary comments never had it: - Remove comment.client_request_id, its unique index, and the dispatch claim (migrations 551/552, and 550 reduced to suppressed_agent_ids). - Remove the replay/resume path in CreateComment, the post-save non-cancelable dispatch context, and the client's sticky request id. Steering sends only `steer_task_ids`; each binding uses the comment id as its receipt request id. Two display extras go too, since the UI already shows the same thing: - The "run ended while you were writing" notice: the recipient chip already switches from "Add to current run" to its new action. - "after step N" on a read receipt: the run trace already places the message at the step where it was read. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(issues): space agent names in Chinese copy; neutral recipients header (MUL-7631) - The steer badge joined names with Intl.ListFormat, which gives "Lambda和Orion" in Chinese. Latin names are now spaced from the Chinese joiner ("Lambda 和 Orion"), per the zh copy conventions; the enumeration comma stays unspaced. - The multi-recipient popover was still titled "Will start when sent" and its tooltip said "choose who runs", though each row now picks how that agent handles the message. Title it "Recipients" and reword the tooltip. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(issues): translate the recipient source labels in Chinese (MUL-7631) "assignee" and "@mention" were left in English in zh-Hans; the recipient menu header shows them. Use 负责人 (per the zh conventions) and @提及. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
d45aba1cd7 |
fix(tasks): settle supplements in application transactions (MUL-7630) (#8755)
Co-authored-by: Sol-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
93ae552ba9 |
feat: add messages to active Claude Code and Codex runs (MUL-7565) (#8682)
* feat: add live run supplements (MUL-7565) Co-authored-by: multica-agent <[email protected]> * fix: address MUL-7565 review feedback Co-authored-by: multica-agent <[email protected]> * fix: resolve MUL-7565 supplement panel and receipt review Co-authored-by: multica-agent <[email protected]> * refactor: remove MUL-7565 supplement ordinals Co-authored-by: multica-agent <[email protected]> * fix: close supplement composer after run remount * fix: exclude task supplements from completion reconciliation * fix: build a persistent binary for local daemon launches * feat: support Claude task supplements through SDK hooks * refactor: simplify task supplement delivery * test: remove redundant task supplement coverage * fix: remove redundant supplement delivery notices * refactor: extract local daemon launcher fix to its own PR * fix: preserve ordinary comment size compatibility * fix: accept empty task start acknowledgements * fix: bound task start response decoding and retries --------- Co-authored-by: Forge-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> Co-authored-by: yushen <[email protected]> |
||
|
YYClaw
|
42727f697e |
MUL-7611: fix(comments): read only thread routing owners (#8712)
Comment owner routing loaded every task row on an issue, including context/result payloads, before selecting the current thread owners. Filter by issue and root comment in SQL and return one agent/squad pair per owner. Closes #8711 |
||
|
|
af62622e50 |
MUL-7288: fix(server): wake guest squad leaders on worker replies (#8314)
* fix(server): wake guest squad leaders on worker replies * fix(server): ignore tombstoned guest delegations * fix(server): preserve assigned squad routing under tombstones * test(server): narrow guest squad routing coverage Co-authored-by: multica-agent <[email protected]> * test(server): cover member-assigned guest squads Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Sol-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
Multica Eveandmultica-agent
|
8c9f865e3e |
MUL-7480: Revert comment steering feature (#8648)
* Revert "MUL-7480: Link delivered steers to final replies (#8623)" This reverts commit |
||
|
|
3de6275eb3 |
MUL-7480: Link delivered steers to final replies (#8623)
* feat: link steer receipts to final replies Co-authored-by: multica-agent <[email protected]> * fix: keep steer final reply locators stable Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Forge-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
c375f83c2e |
MUL-7418: add issue event and time wakeups (#8467)
* feat: add issue event and time wakeups (MUL-7418) Co-authored-by: multica-agent <[email protected]> * feat(wakeups): capture issue collaboration event lifecycle Co-authored-by: multica-agent <[email protected]> * fix(wakeups): preserve reply source when reopening threads Co-authored-by: multica-agent <[email protected]> * feat(wakeups): summarize sidebar and issue activity cues Co-authored-by: multica-agent <[email protected]> * fix(wakeups): restore disabled configurations with a two-way toggle Co-authored-by: multica-agent <[email protected]> * feat(wakeups): manage workspace wakeups in Autopilot Co-authored-by: multica-agent <[email protected]> * chore(wakeups): reserve migrations after current main Co-authored-by: multica-agent <[email protected]> * test(wakeups): align inventory fixtures with shared contracts Co-authored-by: multica-agent <[email protected]> * fix(wakeups): distinguish trigger conditions and execution results Co-authored-by: multica-agent <[email protected]> * fix(wakeups): suppress registration feedback and drain bounded evidence Co-authored-by: multica-agent <[email protected]> * fix(wakeups): align sidebar actions with titles Co-authored-by: multica-agent <[email protected]> * fix(wakeups): bound database work and unify shared issue visibility Co-authored-by: multica-agent <[email protected]> * feat(wakeups): filter mutation events by member or agent actor Co-authored-by: multica-agent <[email protected]> * refactor(wakeups): merge structured evidence and narrow receipt locks Co-authored-by: multica-agent <[email protected]> * feat(wakeups): let people edit wakeup instructions Co-authored-by: multica-agent <[email protected]> * fix(wakeups): align inventory controls and compact prompt editing Co-authored-by: multica-agent <[email protected]> * fix(mobile): handle deferred agent run status Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Lambda <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
8ce795b007 |
MUL-7480: Steer active agent turns from comments (#8605)
* feat: steer active agent turns from comments (MUL-7480) Co-authored-by: multica-agent <[email protected]> * fix: make comment steering hint driven Co-authored-by: multica-agent <[email protected]> * fix: harden active comment steering races Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Forge-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
78a5f69ae1 |
MUL-6813: fix: surface private runtime owner mismatch as explicit failure (#7805)
* fix: fail private runtime owner mismatches (PUCK-89) Co-authored-by: multica-agent <[email protected]> * fix: keep runtime mismatch repair off idle claim path * test: cover mixed runtime claim outcomes (PUCK-89) Co-authored-by: multica-agent <[email protected]> * fix: settle runtime owner mismatches during claim * fix: reject ownerless agents on private runtime claims * chore: restore claim version sampling layout (PUCK-89) Co-authored-by: multica-agent <[email protected]> * fix: re-authorize delivery at the finalize boundary, dedicated runtime_access_denied client copy, fixture convention (PUCK-89) Blocker 1 — final delivery gate: FinalizeTaskClaim now runs a caller- supplied authorize closure inside its transaction. The singular and batch claim paths share finalizeClaimDelivery, which re-locks the runtime row (FOR UPDATE), re-reads the agent, and re-verifies the private-runtime owner fence against CURRENT ownership before the task token commits. A concurrent re-registration that would change owner_id blocks until the gate commits, closing the stale-snapshot TOCTOU. Mismatched tasks settle through the existing failClaimedTaskBeforeLaunch -> FailTask path; singular keeps 200 {"task":null}, batch skips the task and keeps returning valid ones. Regressions cover both paths. Blocker 2 — client copy: runtime_access_denied gets dedicated actionable copy (make runtime public / rebind-copy agent) on the issues blocked-trigger mapping, both chat send-failure toasts, the chat failure-reason map, the mobile dispatch-reason helper, and the autopilot run-now toast. Generic fallbacks unchanged; locale keys added for en/ko/ja/zh-Hans. Blocker 3 — fixture convention: runtime_access_denied_test.go now uses testutil.Call; daemon_runtime_access_test.go seeds its chat task via the dbfx.Task fixture. Test semantics unchanged. Co-authored-by: multica-agent <[email protected]> * fix: bind final task token to locked runtime owner Co-authored-by: multica-agent <[email protected]> * fix: keep delivered user context on current owner Co-authored-by: multica-agent <[email protected]> * test: use testutil.Call in settlement-failure claim regression Convert the last manual httptest.NewRecorder flow in TestFinalizeClaimDelivery_SettlementFailureIsUnsettled to the repository's required testutil.Call helper (hard convention from review) and drop the now-unused net/http/httptest import. No production code changes. Co-authored-by: multica-agent <[email protected]> * PUCK-132: settle queued private-runtime owner mismatches as runtime_access_denied Align persisted task failure semantics with admission semantics: queued private-runtime ownership mismatches now settle as runtime_access_denied, letting clients reach the dedicated recovery copy instead of the generic invalid_task_identity copy. Actual task/agent identity violations (agent rebound, agent deleted, response identity mismatch, error_agent_runtime_changed) keep invalid_task_identity. - add taskfailure.ReasonRuntimeAccessDenied (permanent, non-retryable ownership authorization failure; agent process never launched) - swap the reason in the two owner-mismatch settlement paths in handler/daemon.go (response-assembly recheck + delivery-gate authz-error default branch, including ownerless-runtime denial) - regression A (queued issue task), B (queued chat task + assistant failure message via existing FailTask path), C (agent rebind keeps invalid_task_identity) Co-authored-by: multica-agent <[email protected]> * PUCK-132: map persisted runtime access failures in clients * fix(i18n): add missing fr translations for runtime_access_denied keys Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-agent <[email protected]> Co-authored-by: worker-opencode <[email protected]> Co-authored-by: puck-181 <puck-181@local> |
||
|
|
7ebe0bf58d |
MUL-7213: feat(server): Triage has no executor (#8373)
* feat(server): start no run for an issue in triage (MUL-7213) An issue waiting in Triage produces no execution run, and the triage run itself leaves no trace an execution run can inherit. The rule cannot be enforced at the call sites. Five paths enqueue without looking at status at all (comment and mention dispatch, quick action, manual rerun, auto-retry), and dispatchIssueRun discards the enqueue error entirely, so a missed call site would fail silently. So the check sits at the queue door — immediately before every INSERT of an issue-linked agent_task_queue row — and the upstream short-circuits exist only so that previews and dispatch reasons stay honest about what the door will do. guardIssueNotInTriage covers all five insert sites: both enqueue funnels, FailTask's in-transaction retry, the orphan sweeper's retry, and the delegated-failure recovery dispatch. Two future paths are meant to be exempt and both are Triage's own — the triage run, and accept, which leaves Triage in the same transaction that enqueues the execution run. Upstream, WillEnqueueRun, maybeEnqueueOnAssign, the two shouldEnqueue*OnAssign gates and the comment trigger router all refuse Triage; rerun and quick action refuse with 403 issue_in_triage, a new dispatch reason code. WillEnqueueRun also treats leaving Triage as a creation rather than a status change: accept is when the issue becomes work, and the backlog-to-elsewhere rule would refuse it outright. Two narrower rules follow. retryEligible excludes triage runs for the reason it excludes autopilot runs: re-triaging reads the entry as it stands now, so replaying a failed attempt is never the right recovery. And GetLastTaskSession excludes them from all three CTEs, so the first execution run after accept does not resume the conversation in which the agent was still deciding whether the entry was worth keeping. Refusals are deliberately quiet where a caller cannot act on them. The FailTask guard never returns — that transaction also carries the parent's failed status, and losing it would leave the task stuck in 'running'. Verified: internal/handler, internal/service and cmd/server pass; every new test was confirmed to fail with the change neutralized. Co-authored-by: multica-agent <[email protected]> * fix(server): close two triage session bypasses around rerun (MUL-7213) Review found that session isolation had two holes the query-level exclusion could not reach, and that the two quietest queue-door paths had no direct coverage. A manual rerun resolves its session from the task the user named, via rerun_of_task_id, not from GetLastTaskSession — so after accept, naming the historical triage task reran the triager and resumed the triage conversation, walking straight around the exclusion. RerunIssue now refuses a triage source: redoing triage is re-triage, a different action on a different status. rerunSourceMatchesTaskScope refuses it too, which is what still holds for a rerun row an older server wrote mid-deploy. GetLatestTaskRolloutMissing reports only whether GetLastTaskSession fell back, so the two have to exclude the same rows. It did not, so a terminal triage run with session_rollout_missing told the first execution run after accept that it had lost the previous turn's context — about a turn it was never entitled to. Tests now cover the two doors that refuse silently: FailTask's in-transaction retry, which must leave the parent failed and add no child, and dispatchDelegatedFailureRecovery, which builds its coordinator task with a direct CreateAgentTask call. Both assert against a positive control so a zero cannot come from an inert fixture; the recovery test also shows the obligation is deferred rather than dropped — the same pending comment wakes the coordinator once the issue is accepted. Verified: internal/handler, internal/service and cmd/server pass; all five new tests were confirmed to fail with the fixes neutralized. Co-authored-by: multica-agent <[email protected]> * refactor(server): let a named agent talk on a Triage issue (MUL-7213) The rule shipped as "an issue in Triage starts no run", which is a mechanical restatement of the product red line rather than the line itself, and it was too wide: @-mentioning an agent to discuss a Triage entry was refused along with everything else. What Triage actually lacks is an EXECUTOR. Nothing may derive one from the issue — the assignee, a prefilled owner, a squad leader, an automatic retry — because that is exactly what the triager is still deciding, and because prefilling an owner is how a run starts today, the triager would otherwise dispatch the very owner it was only proposing. A person naming an agent by hand is a different act: no triager action produces it and the cost is one the person chose. That is the same shape as backlog, which parks assignment but lets comments through, one notch tighter because in Triage the assignee is only a proposal. So the queue door now judges the run's origin rather than the funnel it came through. OriginDerived is the zero value, so a caller that says nothing still gets the strict answer and a new enqueue path cannot leak in by omission; the two mixed wrappers take it as a parameter, which made the compiler enumerate every call site. Comment routing loses its blanket short-circuit and instead refuses the two routes that derive an executor — the assignee fallback and the squad leader fallback. Explicit mentions, replies to an agent, and thread continuation route as they do anywhere else. Rerun now follows its source: a discussion run may be repeated, naming no source means "run the assignee again" and is refused, and the triage run itself keeps its own reason. Assignment, quick action, auto-retry and all three session isolations are unchanged. The delegated-failure door is gone because main's #8406 gate subsumes it: Triage is its own lifecycle category, so it is neither unstarted nor started and both that gate and the pending-recovery query already exclude it. A test pins that. Verified: internal/handler, internal/service and cmd/server pass; both halves of the new rule were confirmed to fail when neutralized in place. Co-authored-by: multica-agent <[email protected]> * refactor(server): hold Triage in its own column, not in status (MUL-7213) `status` answers "how far along is this work". Triage answers "is this work at all". Holding the second question inside the first field makes every reader of `status` re-answer it, which is where the scattered `<> 'triage'` checks came from — main carried 9 of them and this branch had grown to 23. A column of its own asks it once. issue.triage_state is NULL for an ordinary issue and `pending` for one waiting in Triage. Adding a nullable column with no default is a catalog-only change, so no table rewrite and no backfill; the queue reads a partial index sized to itself rather than to the issue table. What follows from the move: - issuestatus goes back to 7 keys and 4 categories. Triage is no longer a category, a behavior, or something ExpandCategories can name, so CategoryForBehavior, BehaviorsForCategory, ParseCategory and ExpandCategories lose their special cases, and the dead board-order arm and status label go with them. - WillEnqueueRun loses `acceptedFromTriage`. Accept clears a column and then takes the ordinary create / assign path, so there is no "was in triage" transition left for the predicate to recognise. - The write lock disappears rather than narrowing. Status, project and parent on a Triage entry are the triager's proposal and accept is what confirms them, so ordinary writes reach them; the marker itself is not a field of the issue API, so there is nothing left to lock. - The duplicate guards carry `triage_state IS NULL` instead of a status key, and the delegated-failure recovery predicate gains the same, in both its SQL and its Go mirror. What does NOT follow, deliberately: the reserved NAME stays. Migration 475's CHECK is deployed and 476's renames are irreversible, so `triage` remains unavailable as a custom status key and Resolve keeps refusing it — one reserved word rather than five special cases. Read-path exclusion is a separate sub-issue and is not attempted here. Also from review: the quick-action refusal is now documented as a first-phase product choice rather than a consequence of the rule (its target is configured, not derived), and ErrIssueInTriage no longer claims that nothing runs. Verified: handler, service, issuestatus, cmd/migrate and cmd/server pass on a database migrated from 001; every column read was confirmed to turn its own tests red when neutralized. cmd/migrate's concurrent-index convention test caught a missing cleanup hook for the new index. Co-authored-by: multica-agent <[email protected]> * fix(server): hold the parent of a Triage entry (MUL-7213) Removing the write lock went one field too far. Status, project, assignee and the rest on a Triage entry are the triager's proposal and accept confirms them, so those writes belong. The parent is not a proposal: - the routing panel has no parent field. "Merge into an existing issue" links rather than re-parents, so a Triage entry hanging off a parent is a half-state the product never asks for; - a child in Triage carries a proposed status, so it is never terminal. stageBarrierClosed would wait on it forever — holding the whole sibling set, or one stage's frontier — and ChildIssueProgress would count it in the denominator. Both reach it through parent_issue_id, so refusing the write is what keeps them out of reach. That is behavior, not display, and it is why this one is worth a lock rather than a read-path filter. UpdateIssue and batch refuse a request that carries parent_issue_id for an issue in Triage, with or without a value: null clears, which is equally a parent decision the triager does not get to make. The batch asks one count rather than listing statuses, since "any" is all it needs. Verified: handler, service, issuestatus, issueguard, cmd/migrate and cmd/server pass; both new tests were confirmed to fail with the guard neutralized. Co-authored-by: multica-agent <[email protected]> * fix(server): drop the triage case from the handoff predicate (MUL-7213) MUL-7379 (#8435) landed on main while this branch was open and added issueStatusIsHandoff with a `triage` case, resting on Triage resolving to a category of its own without a catalog read. Once Triage stops being a status that resolution is gone, so the call errors on a value no issue can hold any more. The case is removed rather than repaired: a Triage entry carries whatever status the triager has proposed and is answered on that. Suppressing notifications for Triage entries is a different rule and belongs to MUL-7219 — the status model was giving it incidentally, and that was never the mechanism the design asks for. Verified on a database migrated from 001: cmd/server, internal/handler, internal/service, internal/issuestatus and cmd/migrate pass. Co-authored-by: multica-agent <[email protected]> * chore(db): renumber the triage migrations past the telemetry batch (MUL-7213) MUL-7236 (#8252) took 479-482 on main while this branch was open, so both triage migrations collided. 483 adds issue.triage_state, 484 builds its partial index; the concurrent-index cleanup hook moves with it. Verified on a database migrated from 001 with the new numbering. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
Rusty RavenandBohan Jiang
|
b8d03bc3f4 |
MUL-7326 fix(service): coalesce duplicate EnqueueTaskForIssue on the assignee path (#5914) (#6304)
* fix(service): coalesce duplicate EnqueueTaskForIssue on the assignee path (#5914) Follow-up to #5958, which mapped the duplicate-pending-task race to ErrDuplicatePendingTask on the mention path (enqueueMentionTaskWithCommentPlan) but left the issue-assignee path untouched. enqueueIssueTaskWithCommentPlan is the second write point of the idx_one_pending_task_per_issue_agent unique index, so a concurrent duplicate there still logged at ERROR and surfaced as a generic create-task error (HTTP 500 leaking the raw constraint name). - enqueueIssueTaskWithCommentPlan: reuse isDuplicatePendingTaskErr and return the existing ErrDuplicatePendingTask sentinel (slog.Debug, not Error) on the race — identical treatment to the mention path. - enqueueSingleCommentTrigger (issue-assignee case): switch the plain slog.Warn to the shared logCommentEnqueueFailure helper so the now-typed sentinel is downgraded to debug and resolveCommentTriggerEnqueue can coalesce, matching the sibling squad-leader and mention cases. - Add TestEnqueueTaskForIssueCoalescesDuplicatePendingTask alongside the mention-path regression: a second EnqueueTaskForIssue for the same (issue, agent) returns ErrDuplicatePendingTask, does not leak the constraint name/SQLSTATE, and leaves exactly one pending task. Fixes the remaining EnqueueTaskForIssue case called out in #5938. * test(handler): cover the issue-assignee source in the queued-winner race Bohan-J review on #6304, must-fix 1 and 2. Table-drive TestCommentEnqueueRaceQueuedWinnerFoldsLoser over the two trigger sources that can lose the duplicate-pending-task race on an issue whose agent is the assignee. The mention source has been covered since #5958; the issue-assignee source — the half #5914 left behind — had no handler-level coverage at all, so the comment.go hunk could be reverted with the whole handler suite still green. The new subtest pins both hunks: the status assertion guards the service change (blocked -> coalesced), the log assertion guards the handler change (benign race no longer surfaces as WARN). Also correct the pendingSlotTakenErr doc comment, which this PR made wrong: both paths behind RerunIssue now normalize the unique violation into ErrDuplicatePendingTask, so the raw-violation half is a defensive net rather than the issue-assignee path's shape. --------- Co-authored-by: Bohan Jiang <[email protected]> |
||
|
|
686957e743 |
MUL-7271: test(server): cut slow Go test time further (follow-up to #8295) (#8318)
* test(server): reduce slow unit test waits Co-authored-by: multica-agent <[email protected]> * test(server): cut slow Go unit test time further Follow-up to the first pass on MUL-7271. Test-only changes plus a few package-private seams whose production values are unchanged. - Re-executed race-built test binaries no longer pay the race runtime's 1s atexit sleep (GORACE=atexit_sleep_ms=0 for child processes only). - pkg/agent: event-driven or shorter test-only bounds for handshake, drain and idle graces; redundant timeout variants merged. - daemon/execenv: git fixtures copied from a template repo built once; tests that re-ran the same worktree lifecycle folded into their supersets; ForkLock-guarded fake-CLI writes so parallel tests cannot hit ETXTBSY. - handler/service: cap and paging tests shrink their caps instead of seeding thousands of rows; lock waits probe pg_locks instead of sleeping; one shared pgxpool per service test binary; cheaper cron and RSA fixtures. - wecom: the phishing-link sweep reuses one parser and derives CRLF from LF with a sampled cross-check; fake sockets acknowledge instead of waiting out the ack timeout. - migrate: the migration corpus is parsed once; isolated DB tests run in parallel. Co-authored-by: multica-agent <[email protected]> * test(server): remove transport timeout unit tests Co-authored-by: multica-agent <[email protected]> * test(server): restore two non-timeout tests and make a bound load-bearing Follow-up to the review of #8318. All test-only; no production code changes. - daemon: restore the three read-liveness cases that the transport-timeout sweep took with it. They assert that a ping, a pong and an application frame each EXTEND the read deadline, so a healthy wakeup socket survives without traffic of our own — the deadline never fires on the happy path, which is why they are not transport-timeout tests. Verified they fail (all three) when the extension calls are removed from wakeup.go, and pass in 0.57s together. The pure "times out without peer traffic" case stays deleted. - pkg/agent: restore the parent-DEADLINE half of the opaque-env redaction pair. codex.go treats a parent deadline and a parent cancellation as one ending; only the Canceled arm was left covered. The fake now writes the secret to stderr before reading the handshake, so the check no longer races our own initialize write, and the budget is 2s against a measured 430-465ms launch. Verified it fails when the !contextEnded guard is removed. - pkg/agent: TestRunCollectQuietDoesNotWaitWhenTheAnswerIsIn had its bound widened to a literal 5s, which the shrunk 750ms drain grace can never reach — the test could no longer fail. It now raises the drain grace to 10s for its own duration and asserts under 3s: healthy runs take 0.19-0.44s, and a broken short-circuit takes 10.19s and fails. - wecom: gofmt the import block in relay_settle_budget_test.go. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Sol-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> Co-authored-by: J <[email protected]> |
||
|
Bohan Jiang
|
a9e82c7973 |
MUL-7282: fix(comments): delete only the selected comment and keep its replies (#8323)
Deleting a comment removed every reply under it, through the parent_id ON DELETE CASCADE. Now delete removes only that comment: one with replies becomes a tombstone (deleted_at set, body/attachments/reactions/resolution cleared) so the replies keep their parent, and one without replies is removed along with any tombstone ancestor left empty.
Tombstones are read-only and are never agent input. Every write to a comment's children takes its owners first (issue -> comment -> child), and a comment commits together with the attachments it was posted with. Clients only promise that replies are kept when the server declares comment_delete_keep_replies_supported, and then delete through DELETE /api/comments/{id}/keep-replies, which older servers do not route.
|
||
|
Yushan
|
3551e72e76 |
MUL-7232 feat(daemon): add dsh desktop(self-hosted) support (#8237)
* feat(daemon): discover the DSH CLI bundled in DeepSeek Harness Desktop DeepSeek Harness Desktop ships its CLI inside the .app and never installs `dsh` onto PATH, so a GUI-launched daemon missed it on exec.LookPath — and the login-shell sweep could not rescue it either, because no rc file has a reason to know that path. Only a hand-made symlink made dsh visible at all. Add the app-bundle fallback the Codex provider has had since #5205. The candidate is a Node script rather than a native binary, so it counts only while it is executable: an entry discovered without the executable bit would be advertised as a healthy runtime and then fail on every spawn. The Multica runtime profile gate still applies — a bundled CLI without the profile is not a runtime. * fix(daemon): report and install the missing DSH runtime profile probeAgentCLIs is documented as pure discovery, but dsh's runtime-profile gate lived inside it — one layer above the verdict machinery that turns a drop into a /health skipped_agents reason and a daemon-log line. A dsh binary without the profile therefore vanished from the runtime list with nothing anywhere saying why, which is the shape the DSH reports in #7721 and #6936 run into. Move the gate down to probeBuiltinRuntime and give it its own verdict: dsh is now discovered like any other CLI, and failing to use it is a reported, actionable drop. The verdict is deterministic, so it demotes immediately rather than waiting out a confirmation window — it is a fact about the CLI's environment, not about bytes mid-write. The executable is checked first, because probeDshMulticaProfile cannot tell "the profile refused" from "there was nothing to run", and would otherwise blame a missing profile for a path that vanished under us. Reporting is only half of it: DSH is the one built-in whose CLI is not enough on its own, and no other provider needs anything installed into the agent's own home. MULTICA_DSH_PROFILE_BUNDLE now names what to install — an npm spec, a directory or a tarball, candidates tried in order. Unset stays the default and means "do not touch the user's DSH installation"; there is nothing to point it at yet either, because Multica's own bridge is unpublished (#6936). DSH Desktop ships the pnpm that `dsh plugin` forwards to inside its own runtime-commands directory and injects it only for the processes it spawns, so the install prepends that directory to PATH. Without it the command fails with "pnpm not found on PATH" on exactly the machines this is for. The install runs once per daemon, and the discovery loop is kicked when it finishes so dsh comes online immediately instead of at the next tick. * fix(daemon): keep a registered dsh runtime in step with its runtime profile Two states have to agree: the Multica runtime profile is installed, and a dsh runtime is registered. Neither disagreement corrects itself, and both leave the server routing work into a CLI that cannot start. When the profile is removed, dsh stays discovered and keeps its runtime, so nothing else in the discovery loop looks at it again. Observed in the field: a profile removed a minute after startup stayed live for three minutes and a chat task failed against it. When a profile is installed by hand, the runtime is missing but the converge backoff has already doubled to agentConvergeMaxBackoff (30m) over the rounds that could not register it, so the fix stays invisible for up to half an hour. Compare the two states on every tick and force a round on disagreement, in either direction. A round forced for a registered provider has nothing missing to register, so convergeRuntimeRegistrations now consumes the verdict map it used to discard — safe because demoteUnusableRuntimes owns the claim barrier and the seq-stamped hold, and stays the single owner of demotion. The profile's presence is a stat of the manifest DSH itself looks for, so the check costs one syscall per tick; the authoritative --probe stays inside the probe round the check forces. * docs(runtime): document the DSH runtime-profile variables The two variables this branch adds were undocumented, and the install guide still told readers to run `dsh plugin --profile multica add <bundle>` without ever saying what <bundle> is — the hole #6936 was filed against. Document MULTICA_DSH_PROFILE_BUNDLE and MULTICA_DSH_PLUGIN_PATH next to the other MULTICA_DSH_* variables, and name the bundle in the DSH setup step: what implements the protocol, where the bridge lives, and that the daemon reports the missing profile on /health rather than registering a runtime that cannot start. All four locale files, so the translations do not drift. * refactor(agent): export the DSH stdio protocol version The daemon's runtime-profile probe accepts or rejects a profile on this exact number: one answering `--probe` with a protocol this backend does not drive must not be registered. That decision was about to be made against a second copy of the constant in internal/daemon, and a second copy is one that can drift from the code actually speaking the protocol. Export it so the probe reads the number from its only honest source. No behaviour change: the value is still 1. * fix(redact): mask credentials carried in URL userinfo The connection-string rule covers postgres, mysql, mongodb, redis and amqp only, so `https://user:token@registry/...` passed through untouched. That shape is not hypothetical for agent output: npm and pnpm accept an authenticated registry URL as a package spec, and both the package manager and the registry echo the request URL back in their error text, which is how it reaches a log line without anyone logging the spec itself. Add a scheme-agnostic userinfo rule. The scheme and host survive — which registry refused the request is the part an operator reads, and it is not the secret — and userinfo requires both a colon and an `@` before the next path separator, so `https://host:8443/p`, `ssh://git@host/p` and a bare `a:b` in a path are all left alone. Tests cover both directions. * fix(daemon): decide the DSH runtime-profile verdict on evidence probeDshMulticaProfile returned a bare bool, so a timeout, a non-zero exit, unparseable output, a future protocol version and a genuinely absent manifest all collapsed into "the profile is not installed" — a verdict that demotes a live runtime and, with a bundle configured, installs over the user's DSH home. This rebuilds the verdict layer around what was actually observed. Classification. The probe now returns one of OK / missing / incompatible / unavailable. The frame decides, not the exit status: a profile that printed a valid answer has answered, whatever the process did on its way out — which also stops a process-tree lifecycle error from being read as a protocol failure. With no usable frame, a cancelled ROUND stays transient (the daemon is shutting down; that says nothing about the profile) and otherwise the manifest decides. A manifest that IS present keeps every failure transient, which is what protects a working install during an upgrade. Timeouts are not transient by themselves. Asked for a profile it does not have, DSH does not refuse `--probe` — it hangs, printing nothing, until something kills it. Measured: still running after 10s, zero bytes on both streams. So a timeout is the ordinary way a missing profile presents, and treating it as transient made the profile undetectable on exactly the host the automatic install exists for. The manifest is the same file DSH's own loadProfile consults before refusing to boot, so a confirmed-absent one is evidence. Order. The profile probe now runs only after version detection succeeds. exec.LookPath is far too weak a proxy for "this CLI works": on Windows the CLI is a .cmd shim it resolves happily and which exits 9009 — cmd.exe's "command not found" — when what it forwards to is missing. Probing the profile first reported such a machine as missing its runtime profile, a repair for a problem it did not have. Demotion. An incompatible profile is reported and never demoted: this daemon can be the stale side of a protocol skew, and tearing down a live runtime because THIS build is behind is a destructive answer to a version question. The confirmation window that gated the not-executable verdict now gates every condemnable verdict, keyed per verdict so a provider moving between states starts a fresh clock rather than inheriting one earned by another problem. Convergence. A registered dsh whose profile was removed forces a round on every tick, because condemning it takes two of them and nothing else schedules the second: with dsh still registered nothing is missing a runtime, so an unforced tick returns without probing at all. The mirror direction — a profile with no runtime — stays throttled to one forced round, since no round is guaranteed to resolve it and forcing would bypass agentConvergeMaxBackoff forever. Offline cause. A missing profile reaches the server as a structured `dsh_profile` code rather than a bare offline, and carries whether the daemon is installing the profile right now — the one case where queueing is right. That claim is withdrawn if the install gives up: the runtime row is written once, by the demotion that also removes the runtime from the index, so nothing later would correct it and the work would queue forever behind an install that already stopped. Provisioning. Runs under the daemon's lifetime through processtree, so the pnpm/node descendants cannot outlive a shutdown and keep writing into the DSH home. Success is the manifest on disk, not the package manager's exit status, and a candidate that exits 0 without producing a profile is a failed candidate so the next one still gets its turn. The bundle spec no longer reaches a log line — an npm URL can carry credentials and a path a username — and captured output is redacted and bounded on a rune boundary. The PATH merge matches the variable case-insensitively, because Windows spells it `Path` and an exact `PATH=` miss left the package manager with only the plugin directory under Go's case-insensitive de-duplication. Startup. A daemon that registered nothing continues when it has just begun installing the profile itself. On a host whose only provider is a dsh without one, failing here is a deadlock rather than a fail-fast: the probe starts the install, the error kills the process milliseconds later, the install's process tree dies with it, and the next start repeats all of it. Nothing else is forgiven. Because such a daemon tracks no workspace, a finished install picks the workspace up before converging. Repair. The reported repair names the missing package and carries no command: the install needs a bundle only the operator can choose, and Repair.Command is rendered to the user inside a shell fence as the line to run, so a placeholder there is a paste-and-fail instruction. Tests. The classification matrix, "manifest present + timeout / non-zero exit / malformed output / future protocol ⇒ no demotion, no provisioning", the 9009 shim, the forced-round sequence, the withdrawal, the startup exception, the empty install, and the case-insensitive PATH merge. Two existing tests read the developer's real ~/.dsh and one fake dsh could not answer `--version` at all — both now pinned to the test's own directory and to the shape the feature is actually about. * feat(daemon): discover the CLI shims DSH Desktop generates The macOS fallback pointed at one path inside the app bundle, and Windows had none at all — every candidate there was a /Applications path that cannot exist, so a Windows host running DSH Desktop reported no dsh whatsoever. Replace it with one layout that serves both platforms. The CLI is not in the install directory. DSH Desktop generates shims under its own per-user data directory — ~/Library/Application Support/DSH Desktop on macOS, %APPDATA%\DSH Desktop on Windows — and on Windows that is roaming application data even though the app installs to Program Files or %LOCALAPPDATA%\Programs. One lookup therefore covers both install modes, since a per-machine install still generates these shims per user. Payload directories are content hashes (and, under host-commands, a hash plus a generation uuid), so they are enumerated rather than composed. Only directories holding a bin/ are accepted, so a stray file or half-extracted download is skipped; modification time orders them so an upgrade's new payload wins over an abandoned one, and the name breaks ties because ReadDir order is not a promise and the daemon must not pick a different CLI on different rounds. host-commands outranks cli, and that is load-bearing: nothing here executes a candidate, so a broken-but-present shim is indistinguishable from a working one at this layer — and on the install that prompted this, the cli shim was exactly that, resolvable by exec.LookPath and exiting 9009. Ranking is the only lever. Generated shims also outrank the macOS app-bundle script, which is entered through `#!/usr/bin/env node` and so only works where node happens to be installed; a shim runs the Desktop app's own runtime by absolute path and needs nothing on PATH. That gap was latent on macOS too. executableCandidate had to learn the platform difference for any of this to work: Go synthesizes a regular file's mode from the read-only attribute and never sets an executable bit on Windows, so the Unix test rejected every candidate and the whole fallback would have been dead code there. Windows uses exec.LookPath, the same check the launcher makes, so a candidate accepted here is one that can actually start — which is also why the Node script has no Windows counterpart and .js never appears in that list. Only default locations are covered, and nothing reads an install receipt or the registry, so a relocated install still needs MULTICA_DSH_PATH. The path builders take goos/env/home as arguments rather than reading runtime.GOOS and os.Getenv, so the Windows layout is asserted from any host — the alternative leaves it testable only on Windows, which in this package has meant untested. * fix(server): refuse a missing DSH runtime profile under its own reason A runtime taken offline for a missing profile reached the server as a bare offline, so runtimeVerdict classified it as waitable/runtime_offline and assignments and @mentions bound to that agent queued forever — even though, with no automatic install configured, that wait never resolves on its own. That contradicts the semantics settled in MUL-6164: when a human has to intervene, refuse the trigger and say how to fix it. Blocked, but not as runtime_unusable. The two causes have opposite repairs: an unrunnable CLI is reinstalled, while a CLI missing its runtime profile is working perfectly and reinstalling it changes nothing. Sharing one code meant every client told DSH users to reinstall a CLI that was never the problem — the chip and toast said "reinstall it there", and the durable system comment left on the issue opened with "its CLI is installed but cannot be executed on that machine", blamed a blocked npm postinstall, and rendered `dsh plugin --profile multica add <bundle>` inside a bash fence as the line to run on that machine. A placeholder presented as a command is the shape of instruction people paste verbatim and then report as broken. So: a new runtime_profile_missing reason code, carried through readiness, dispatch and the localized copy in all four languages. The notice branches on the reason rather than on whether a repair command happened to be present, and the profile text names what is missing, says the CLI is fine, and points at the two ways to supply a real bundle without printing a command to copy. An install the daemon is running right now is the exception: that wait does end by itself, so the work queues instead of being refused. RuntimeBlockedNeedsNotice replaces three inline comparisons, because a code added to one admission path and not the others is a trigger that vanishes with no trace on exactly the surfaces that have no response for the user to read. * docs(runtime): document DSH profile provisioning and shim discovery Automatic provisioning is a new, persistent feature surface, and it had no product surface: an admin had no documented way to configure it, and the one paragraph that existed attached the install semantics to the wrong variable — the sentences describing what MULTICA_DSH_PROFILE_BUNDLE does followed the MULTICA_DSH_PLUGIN_PATH sentence, so "the variable is unset by default and the install it enables is bounded" read as being about a variable that enables no install, and PLUGIN_PATH was then described twice. Environment variables, in all four languages: the install semantics move onto the variable they belong to, and now state the bounds — at most once per daemon lifetime, only on a confirmed-absent profile, failures logged with the package manager's own output and retried only after a restart, a timeout or an unsupported protocol version reported and never installed over. Plus how to diagnose (search the daemon log for `DSH runtime profile`) and how to roll back (delete $DSH_HOME/profiles/multica). MULTICA_DSH_PLUGIN_PATH is described once, and Windows and Linux are named as the platforms with no bundled pnpm directory, where the daemon uses whatever is on PATH unless this points at one. Install guide, in all four languages: what to actually put in the variable. Multica's bridge is not on a public npm registry yet (multica#6936), so a self-hosted team builds it from the repository and points at the build directory, or at the tarball npm pack produces when the daemon host is not the build host; a package name becomes the simplest option once it is published. With a warning that belongs on this value specifically — it is installed into every daemon host's DSH home without further confirmation, so an unvetted third-party package is a supply-chain decision, not a convenience. Also documents where the daemon looks for a Desktop-managed CLI, including the detail that trips people on Windows: it is roaming application data, not the install directory. And the one exception to "the daemon must detect at least one built-in CLI before it will start" — a daemon that has just begun installing the profile itself starts with no runtimes, because otherwise a host whose only tool is a DSH needing that profile could never finish the install. * fix(daemon): find DSH Desktop's bundled pnpm on Windows too The automatic install shells out to `dsh plugin`, which forwards to pnpm, and the daemon inherits no path to the one DSH Desktop ships. The lookup composed a single macOS path and was commented as macOS-only, so on Windows the install failed with "pnpm not found on PATH" unless the operator set MULTICA_DSH_PLUGIN_PATH by hand — on exactly the hosts the automatic install exists for. Both platforms bundle it under runtime-commands in the app's own per-user data directory, but not in the same shape: macOS 2.0.5 keeps a flat runtime-commands/bin, while a Windows install keeps runtime-commands/generations/<id>/bin. Composing either path leaves the other broken, so the lookup now shares the enumerator with CLI-shim discovery and searches both shapes at both levels. A Desktop version that moves between them is then a version this daemon keeps working with rather than one it silently stops supporting. Sorting is pooled per tier rather than per directory. Doing it per directory made the answer depend on the order ReadDir returned DSH profile names, which is the same non-determinism the name tie-break exists to remove: the newest generation has to win even when it belongs to a profile that sorts later. An existing test caught this while the enumerator was being extracted. An explicit MULTICA_DSH_PLUGIN_PATH now replaces the search instead of merely being tried first. An operator who pinned a directory is bypassing the bundled pnpm on purpose, and quietly falling back to it would defeat that. Documented in all four languages, where the previous text stated the opposite — that Windows has no bundled directory. * fix(server): bound the in-flight DSH install claim instead of trusting it `installing` tells the server to queue triggers rather than refuse them, so a claim that outlives its install recreates exactly the failure the structured reason was added to prevent: work queued behind something that already stopped. The withdraw cannot be made to guarantee that. Three paths leave the claim on the row. The verdict reads dshInstallInFlight when it is built, but the wait is recorded later, when the demotion knows the runtime ids — an install that fails in between finds an empty list, and the `installing` written afterwards is never withdrawn. The corrective Deregister is best-effort, and during shutdown it runs on an already-cancelled lifecycle context. And a daemon restarted mid-install does not track the row at all. Bound the claim instead. SetAgentRuntimeOfflineWithReason stamps updated_at, and the install it describes is itself bounded by the daemon's provisioning timeout, so honouring `installing` only inside a window comfortably longer than that covers a real install while capping every way the claim can be left behind. A row with no usable timestamp cannot be bounded and is not honoured: refusing during a real install costs one retry the notice already asks for, while honouring a stale claim costs work that queues until it expires. The withdraw stays as the fast path — this is the floor under it. * refactor(daemon): scrub URL credentials in provisioning output, not globally The rule was added to pkg/redact's shared pattern list, where redact.Text runs over every agent's task messages, comments and error text — so a fix aimed at one package manager's output changed what every provider emits. The gap is real and still worth closing in that list, but widening a shared filter is its own change with its own blast radius, and it does not belong in a PR about DSH. Move it to dshProvisionOutput, which is the only place this PR needs it: a bundle spec can be an authenticated npm URL, and both the package manager and the registry echo the request URL back in their error text. The shared redactor still runs after it for the token shapes it already knows. Behaviour for the provisioning log line is unchanged; behaviour for every other provider returns to main's. Same assertions as before, moved next to the code they now cover, plus the false-positive cases — a port, an scp-style git remote, a bare colon in a path — and one asserting the host survives, since which registry refused the request is what an operator reads and is not the secret. * fix(daemon): let a converge round condemn dsh only convergeRuntimeRegistrations started consuming the verdict map it used to discard, so that a round forced by a DSH profile mismatch could reach demotion — a registered dsh whose profile was removed leaves nothing missing a runtime, so the round has no registration left to do and would otherwise return having done nothing. But it acted on every provider's verdict, not just dsh. On main, demotion is reached from refreshAgentVersions alone, on its own ten-minute cadence, and that is the schedule every provider's verdict was tuned against. Claude Code, Codex and the rest would have moved onto a different one as a side effect of a DSH fix — a behaviour change for providers this PR has no reason to touch. Narrow it to dsh. It is the only built-in with a precondition that can change without any version changing, which is why it is the only one that needs a round it can be forced into; every other provider keeps main's behaviour exactly. |
||
|
importcpp
|
1500abe357 |
MUL-7272: fix(comments): preserve deferred worker reply handoffs (#8288)
A worker's plain reply that arrived after the squad leader's run was claimed, but before it started, was never delivered. Creation deferred it to completion reconcile, and reconcile only replays explicit agent mentions, so the leader never received another run for it. Record such replies in the claimed run's planned inputs. On completion, replay the assigned-squad-leader route only when the reply was recorded there, after rechecking current routing and permissions. Timestamp-only agent replies, self-replies, notes and suppressed replies still earn no follow-up. RegisterPlannedCommentForActiveTask now also rechecks status on the UPDATE target, so a registration that waited on a completing row misses and enqueues a successor instead of attaching an obligation to a completed run. |
||
|
|
b5a7ee1e0e |
fix(comments): remove delayed assignee fallbacks (#8174)
* fix(comments): remove delayed assignee fallbacks * fix(comments): retire fallback retry descendants Co-authored-by: multica-agent <[email protected]> * fix(comments): preserve fallbacks with merged comments Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Lambda <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
Jiayuan Zhang
|
028f3b4437 |
feat(issues): show agent execution inline in comment threads (#8131)
* feat(issues): show agent runs inline with comment replies * fix(issues): unify run comment projection and preserve disclosure focus * refactor(issues): replace nested run cards with inline status rows * fix(issues): align thread roots with reply content * style(issues): remove thread toggle background * style(issues): group reaction picker with comment actions * feat(issues): show readable issue references in run summaries * style(issues): move completed run logs into comment headers * style(issues): balance comment block vertical spacing * style(issues): shimmer live agent progress text * style(issues): unify comment action hover feedback * style(issues): reveal comment actions on block hover * fix(issues): move published reply activity into the header * fix(issues): settle comment UI as soon as replies are published * fix(issues): reveal comment log buttons on hover * fix(issues): defer run placement until trigger comments arrive * fix(issues): isolate agent comment queues by thread * fix(issues): display pending runs in execution order * fix(ci): align integration tests with thread-scoped queues * fix(issues): fit run actions to narrow comment columns |
||
|
Bohan JiangandCatCatUncle
|
3ac53a68a6 |
MUL-7108 fix(autopilot): judge every autopilot write as the human it acts for (#8107)
Autopilot write endpoints judged the request's authenticated user, which under
a mat_ task token is the RUNTIME OWNER. Any member could therefore @ an agent
running on someone else's machine and spend that machine owner's autopilot
rights — pause a rule, re-point a webhook, rotate a token, or grant themselves
a collaborator row.
Every write on this surface now resolves the human it acts FOR (a member acts
for themselves, an agent for its run's originator), through one seam, and
stamps that same human into created_by / published_by / granted_by and the
rule versions. The gate and the row can no longer name two different people.
Two bypasses of that gate are closed with it:
- The webhook credential was broadcast to the whole workspace on
autopilot:updated, handing every member the token GetAutopilot had just
redacted for them. All five trigger fan-outs now carry a redacted copy; the
HTTP caller still gets the live value.
- Agent identity itself was forgeable: Auth stripped only X-Actor-Source, so a
member could replay an observable (agent_id, task_id) pair — both returned by
GET /api/issues/{id}/task-runs — and be resolved as that agent. Auth and
DaemonAuth now strip both headers, leaving the mat_ branch as the only writer
of agent identity.
That last part is MUL-3428, reported and fixed by @CatCatUncle in #4313; it is
carried here because this change cannot be correct on top of a forgeable actor.
Behavior tightening, no migration: the only path it breaks is a member changing
an autopilot they hold no grant on. A run with no originator can no longer
write here.
Co-authored-by: CatCatUncle <[email protected]>
|
||
|
|
cddef22f3b |
MUL-7006: fix(server): keep leader routing when a reply targets a leader comment (#7975)
* MUL-7006: fix(server): keep leader routing when a reply targets a leader comment Replying directly to a squad leader's comment demoted the follow-up to a generic direct-agent task: routeReplyToParentAuthor resolves the agent purely as the parent comment's author and never populates Squad, and the dispatch switch only upgraded the conversation-continuation source. The same reply posted against a member comment in the thread already restores the role (routeConversationOwnersForRoot), so the two reply paths disagreed. The demotion costs more than the role name. No squad briefing is injected at claim time and `multica squad activity` is rejected with a 400, so the leader cannot coordinate at all that turn. On a project with a local_directory resource it also binds the coordinator to the user's own directory, queues it behind the path mutex, and drops the provider session the server had just handed it a managed prior workdir for. Read the role from the replied-to comment's own authoring run (source_task_id) rather than the agent's latest task on the issue: an agent that is both leader and worker of one squad posts in both roles, and a reply continues the role of the comment it answers. Fails closed to today's routing when the comment records no authoring run, the run was not a leader run, the squad is gone, or leadership has moved. Co-authored-by: multica-agent <[email protected]> * MUL-7006: fix(server): role-scope comment coalescing and refuse archived squads Review found two ways the leader role could still be lost or wrongly revived. Coalescing ignored the role. A pending task for the same (issue, agent) short- circuits the enqueue through MergeCommentIntoPendingTask, which re-attributes a task but never re-roles one. A leader-role reply arriving while a direct-agent task sat queued was therefore folded into it and reported coalesced, so the leader enqueue never ran: no squad briefing, and on a local_directory project still bound to the user's own directory. Role-scope the merge the same way it is already head-scoped, and for the same reason — this statement cannot make a task correct for a comment it was not routed for. Re-roling the pending task in place would be worse: it would move work the direct task was meant to do in the user's directory into a managed one. A miss is not a drop — the queued task counts as active, so the caller defers and completion reconciliation recomputes the routing and enqueues the correctly-roled task once it finishes. Deleting a squad archives it rather than removing it, after transferring its issues to the leader agent, and GetSquadInWorkspace does not filter archived_at. A reply to a leader comment predating the deletion would then create a leader task for a squad the issue no longer has any assignment for. Fail closed on squad.ArchivedAt. Covers both with regression tests: the four role-pairings of a reply against a pending task (mismatches defer and leave the task untouched, matches still coalesce), and an archived-squad reply staying a direct-agent task. Co-authored-by: multica-agent <[email protected]> * MUL-7006: fix(server): prove reconcile coverage before deferring a refused merge Role scoping broke an invariant the AlreadyPending path rested on. That path deferred to "an active task" because the comment was taken to be newer than it, and so inside reconcile's `created_at > since` window. That held only while every pending task the (unscoped) check accepted was one the merge could fold into: a miss meant the task had been claimed, and a claimed task still predated the comment. A merge that can refuse on role removes the guarantee — a task inserted for the same (issue, agent) between a comment's INSERT and its trigger resolution is NEWER than the comment and is not carrying it in its planned ids, so nothing would replay it. The response still promised deferred. Ask reconcile's own question instead of assuming its answer: HasActiveTaskCoveringCommentForIssueAndAgent requires an active task created strictly before the comment. When none exists the enqueue is retried, and its duplicate-key path routes the comment into the same durable hand-off a lost race gets — converging to a truthful non-success rather than a promise. A queued wrong-role blocker has no safe park (a planned id on a pre-claim task is marked delivered at claim time, which is exactly how the wrong-role run would consume it), the same shape as the different-head queued blocker already documented there. Also narrow the merge refusal to the one harmful direction. A leader task is a strict superset of a direct-agent one, so a direct-role comment folds into it safely — as the mention resolver already assumes when a @squad upgrades a plain @agent trigger for the same leader. Refusing both directions would have stopped an @agent mention from coalescing into a queued leader task, splitting a run that correctly folds today, and widened the uncovered window for no gain. Regression: a comment older than an opposite-role queued task must not be reported deferred/coalesced/queued, and the blocker must be left untouched — not re-roled, not carrying the comment as a planned id. Co-authored-by: multica-agent <[email protected]> * MUL-7006: fix(server): require written proof after role merge misses Co-authored-by: multica-agent <[email protected]> * fix(server): narrow leader reply routing scope (MUL-7006) Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> Co-authored-by: J <[email protected]> |
||
|
|
765321f053 |
MUL-6951 fix(autopilot): carry the trigger creator's authorization on scheduled runs (#7918)
* fix(autopilot): carry the trigger owner's authorization on scheduled runs A schedule/webhook autopilot run left originator_user_id NULL, so canInvokeAgent saw no human and fail-closed against private agents. Arming a trigger is the same authorizing act as clicking "run now" — deferred, not absent — so withholding the originator did not remove that authority. It only forced every capability the automation legitimately needed to arrive as a separately-bounded borrow path: MUL-4857 for child creation, MUL-6691 for the run's own issues, #7902 for the accountable member's issues, with sub-issue dispatch queued up as the fourth. Each carries a hand-written scope that no operator can see in the UI. attribution.TriggerOwner / RuleOwner now populate UserID (the originator) instead of only the accountable side. finalizeAttribution mirrors it, so the originator == accountable invariant that migrations 190/197 enforce holds without a schema change or backfill. Both autopilot execution modes resolve through triggerOwnerAttribution, so run_only and create_issue are covered by the same two lines. This grants exactly the trigger owner's own rights — canInvokeAgent is untouched, only the principal handed to it changes — and originator_source still distinguishes a scheduled fire from a manual one in the audit trail. The borrowed-authority machinery in handler/agent_access.go is now dead for newly enqueued runs but is deliberately left in place: tasks enqueued by the previous backend are still running during a rolling deploy and carry originator NULL, and they must keep the path they were admitted under. Deleting it is a follow-up once this is deployed. Composio connected-app credentials also key on originator_user_id, so scheduled runs now receive the trigger owner's connected apps. Decoupling that from authorization was considered and explicitly deferred. Verification (real PostgreSQL 17): - internal/attribution, internal/service, internal/handler: PASS - the five attribution_stamp tests that pinned "originator stays NULL" were updated to assert the new principal, and fail against the old constructors - pre-existing, unrelated failures in internal/daemon/execenv (4 Hermes store-path tests) reproduce identically on a clean tree Co-authored-by: multica-agent <[email protected]> * refactor(autopilot): delete the borrowed-authority machinery With a scheduled run carrying its trigger owner as originator, every autopilot-shaped authority path in agent_access.go is unreachable: the invoke gate resolves a real human from the speaking task and judges it exactly as it judges that human acting directly. Removed: - autopilotTaskAssignAuthority + autopilotAssignAuthorityFromRequest (MUL-6691) - issueBoundToAutopilotTask, taskRunsAutopilotInWorkspace - autopilotDelegationAuthority and its FromRequest / FromComment resolvers (MUL-4857) - effectiveInvocationAuthorityFromRequest - assignAuthorityScope, assignScopeKind and the four scope constructors - commentTriggerComputeOptions.AutopilotDelegationAuthorityUserID + effectiveInvoker() validateAssigneePair and triggerTasksForComment lose their now-meaningless scope / delegation-authority parameters; every caller passes the run's originator instead. This also retires the fourth borrow scope #7902 proposed. CreateIssue still loads the parent issue when assigning, even though the gate no longer needs the row: it is what turns a cross-workspace or missing parent into 400 "parent issue not found" instead of a 403. Covered by a test. Two behaviour notes, both accepted: - Authority now propagates down a delegation chain, because the parent run finally has an originator to copy. Under the old design each hop had to re-borrow. This matches what a manual "run now" already did. - The old borrow path required task.Status == "running"; the originator path does not, and never did for any non-autopilot run. Task tokens are revoked at the terminal transition (TaskService, MUL-2600), so the primary guard remains; this only drops a second layer that existed solely for autopilot runs. Tests: the two suites that pinned the borrow bounds (autopilot-created-issue binding, authored-by-this-task binding) described a mechanism that no longer exists, so they collapse into one file asserting the rule that replaced them — rights / no rights / no human at all, across create-top-level, create-child, update and batch. The mention-authority suite keeps its shape; its fixture now gives the run a human, as a live run has. Verification (real PostgreSQL 17): internal/attribution, internal/service and internal/handler all PASS. Pre-existing unrelated failures in cmd/multica, internal/cli, internal/daemon and internal/daemon/execenv reproduce identically on a clean tree. Co-authored-by: multica-agent <[email protected]> * fix(autopilot): resolve one principal per dispatch and expire it with the run Addresses Elon's review of #7918. Three findings, all confirmed against the code before fixing. P1-1 — admission and the run's identity could be two different people. Automatic dispatch admitted the assignee agent as the AUTOPILOT CREATOR (canCreatorInvokeAgent) while the task took its originator from the trigger. With A owning both the autopilot and a private agent, and B owning the trigger, a dispatch was admitted as A and then executed as B — rights neither holds alone. The old suite could not see it because it called dispatchRunOnly directly, skipping admission. - autopilot_trigger gains an IMMUTABLE created_by (migration 449, best-effort backfill from published_by). published_by keeps its audit meaning and still transfers on a substantive edit; it is no longer an authorization input, because letting an editor move it silently re-authorizes the automation as that editor. Per Bohan's ruling the run always acts as the trigger CREATOR. - ResolveAutopilotTriggerPrincipal is now the single answer to "who does this run act as". Admission, the task stamp and every delegated run read it, so the fork is unrepresentable. It also binds the trigger to its autopilot (GetAutopilotTriggerForAutopilot — the old lookup was WHERE id = $1, which would let a foreign trigger select the principal) and re-checks workspace membership on every dispatch, so removing a member revokes their triggers. - canCreatorInvokeAgent is now unreachable and deleted. - RuleOwner reverts to accountable-only. It is a guess at who owns a rule; promoting it would hand a legacy trigger somebody's rights without that person arming anything. Such runs carry no originator and fail closed. P1-2 — a terminal task could still spend its human's rights. invokeOriginatorFromRequest now refuses completed / failed / cancelled tasks. Token revocation at the terminal transition stays the primary guard, but it is best-effort, explicitly non-fatal on failure, with a 24h expiry behind it. The check sits at the authorization boundary rather than the auth boundary, so a finishing run can still post its final comment but cannot start new work. P1-3 — the PR and code comments described Composio backwards. BuildTaskOverlay ignores the originator entirely and builds from the AGENT OWNER's connections and allow-list (MUL-3963). Naming the originator hands out no third-party credentials. Comments corrected; no decoupling was ever needed. Operational note: a trigger with neither created_by nor published_by (predating migration 186) resolves no principal, so its autopilot stops dispatching until the trigger is re-saved. That is the intended fail-closed behaviour. Tests: full dispatch through admission for both fork directions; principal fails closed on legacy / cross-autopilot / removed-member; terminal task refused on all three statuses; the edit-transfer tests now assert the principal STAYS with the creator while published_by moves. Verified the fork test fails when admission is reverted to canCreatorInvokeAgent. Verification (real PostgreSQL 17, pristine database): internal/attribution, internal/service, internal/handler and cmd/server all PASS. Migration 449 up/down round-trips. Pre-existing unrelated failures in cmd/multica, internal/cli, internal/daemon and internal/daemon/execenv are unchanged. Co-authored-by: multica-agent <[email protected]> * fix(autopilot): bind the trigger principal to its workspace, correct stale docs Second round of Elon's review on #7918. 1. The trigger lookup only enforced half of what its comment claimed. GetAutopilotTriggerForAutopilot matched on (trigger id, autopilot id); the workspace was used only for the creator's membership check afterwards. That check proves the resolved human belongs to the workspace PASSED IN, not that the autopilot does — a member of two workspaces satisfies it while the trigger came from the other tenant. The query now joins autopilot and filters on workspace_id, and the resolver passes it through. Regression test seeds two workspaces sharing one member and asserts the foreign workspace resolves nobody; verified it fails against the pre-fix query. 2. Several permission-boundary comments still described the pre-MUL-6951 model — that a run's authority and its accountable human transfer to whoever last edited the trigger. That is now the opposite of the code, and these are the comments a future reader would use to reason about the authorization model, so they are corrected at all four sites: the trigger create site, the SetAutopilotTriggerPublisher query, triggerOwnerAttribution, and the dispatchRunOnly attribution block. published_by is described as what it now is: a config-audit column with no bearing on a run. 3. The migration comment now states the backfill's imprecision plainly: for an already-edited trigger it freezes the last recoverable EDITOR as the immutable creator, not the historical creator, which the schema never recorded. It also records that a NULL/NULL trigger has no recovery path by decision (Bohan: leave them empty) — re-saving re-stamps published_by, not created_by — so its runs stay refused with a recorded failure_reason. Not done, per Bohan's ruling: no re-save takeover path for legacy triggers and no migration-period alerting. Verification (real PostgreSQL 17, pristine database): internal/attribution, internal/service, internal/handler and cmd/server PASS; full suite shows only the four pre-existing unrelated failures (cmd/multica, internal/cli, internal/daemon, internal/daemon/execenv). go vet ./internal/... and git diff --check clean. Co-authored-by: multica-agent <[email protected]> * docs(autopilot): carry the creator-principal semantics into every description Third round of Elon's review on #7918. Two must-fixes, no behaviour change. 1. The final semantics had not reached every surface that describes them. Migration 189 wrote COMMENT ON COLUMN text for autopilot_trigger.published_by_* saying it decides the accountable human of the runs a trigger fires. That is now false, and because sqlc generates column comments into models.go, the stalest description of the authorization model sat in generated code every reader passes through. 189 is released and untouched; migration 449 overrides both comments and documents created_by_* for the first time, and sqlc is regenerated. 449's down restores 189's original text verbatim so a rollback does not leave MUL-6951 wording describing a model the database no longer runs. The same stale semantics remained in prose at six code sites — the plain and manual dispatch entry points, the scheduled-plan path, the create_issue enqueue comment, the autopilot-level republish path, and the webhook trigger create site. All said a run's authority or accountable human follows the last editor, or that automation resolves rule_owner. These sit on the authorization path, so they are corrected rather than left to drift. published_by is now described as what it is: config audit, with no bearing on a run. Also corrects the backfill comment's migration reference: trigger publishers arrived in 189, not 186. 2. The test file this PR added open-coded INSERT ... RETURNING id with matching t.Cleanup(DELETE ...), which CLAUDE.md prohibits. Rewritten onto testutil.Fixture: a small principalFixture embeds it and every row — members, runtimes, private agents, autopilots, triggers, the second workspace — is built and cleaned up by dbfx. The only remaining DELETE is the mid-test member removal that IS the behaviour under test. Re-verified after the rewrite that both regressions still fail against the old code: reverting admission to the autopilot-creator check reddens the fork test, and dropping the workspace join reddens the cross-workspace test. Verification (real PostgreSQL 17, pristine database): full suite shows only the four pre-existing unrelated failures (cmd/multica, internal/cli, internal/daemon, internal/daemon/execenv). Migration 449 up/down round-trips and the down restores 189's comments. go vet ./internal/... and git diff --check clean. Co-authored-by: multica-agent <[email protected]> * docs(autopilot): state the backfill widening plainly, drop raw-SQL test seeding Fourth round of Elon's review on #7918. Both items are leftovers from the previous round; no behaviour change. 1. Migration 449's backfill comment carried two factual errors. It said published_by_* arrived in migration 186; it was 189. The previous round intended to fix this but the string replacement did not match the wrapped line and silently did nothing, so the wrong number shipped. It also claimed the backfill "is no wider than the authority those runs carry today", which is false: before MUL-6951 an automatic run had no originator at all, only narrowly-scoped borrow paths, so promoting a backfilled editor to the run's full originator IS a widening. The product decision to accept it stands; the comment now says so directly as a compatibility tradeoff instead of denying the widening. A migration comment that understates what it grants is worse than no comment. 2. The new test file still reached raw SQL transitively. It called seedAttributionFixture, which hand-writes INSERT and cleanup for its user / workspace / member / runtime / agent — so the file's own claim that "every row goes through testutil.Fixture" was untrue and the repository's fixture rule was still unmet. It now bootstraps the base identity with Fixture.User / Workspace / Member on an unbound fixture and adopts it, which also drops the agent and issue that helper builds and these tests never use. The comment explains why the helper is deliberately not reused. Re-verified after the fixture change that both regressions still fail against the old code: reverting admission to the autopilot-creator check reddens the fork test, and dropping the workspace join reddens the cross-workspace test. Verification (real PostgreSQL 17, pristine database): full suite shows only the four pre-existing unrelated failures (cmd/multica, internal/cli, internal/daemon, internal/daemon/execenv). go vet ./internal/..., gofmt and git diff --check clean. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
f1ce9d4542 |
MUL-6883 perf(server): stop rescanning settled delegated-failure recoveries (#7820)
* feat(server): retire settled delegated-failure recoveries from the outbox scan ListPendingDelegatedFailureRecoveries had no indexable notion of a finished recovery. Its partial index (343) selected every recovery comment ever written, so each sweeper tick re-proved through four joins and two NOT EXISTS subqueries that history was already handled. LIMIT bounds the rows RETURNED, never the rows CHECKED, so a tick that found nothing still grew more expensive as history accumulated. Add comment.recovery_settled_at and a partial index that holds only unsettled recovery comments, and write the marker inside the transaction that makes a task terminal. Every other exclusion the scan applies is reversible — an issue can leave 'done', a cancelled retry can be superseded — so none of them can be frozen into an index predicate. A delivery receipt on a terminal task cannot be taken back, which is what makes it safe to record as durable state. The marker is deliberately written only once the task is terminal. SetTaskDeliveredCommentIDs replaces rather than appends a receipt while a task is still dispatched, so a reclaim by a differently-capable daemon can drop an id it will not deliver; settling at receipt-write time would freeze that legitimate transient window into a permanently lost recovery. Exhaustion is the one terminal outcome whose receipt lands on a row that may still be running, so it retires the comment directly in the same transaction. An unmarked row is the safe direction: it is scanned exactly as it was before this column existed. That is what makes the rolling-deploy window harmless — an older replica that writes a receipt without the marker only leaves work for the backfill. Co-authored-by: multica-agent <[email protected]> * feat(server): backfill delegated-failure recoveries settled before the marker The marker only covers tasks that reach a terminal status after the writing code ships. History written before that keeps matching the unsettled index predicate and keeps being re-checked on every sweeper tick, which is the cost this change set exists to remove. The walk retires a candidate only when a terminal task already recorded it in delivered_comment_ids — the same monotonic condition the runtime path uses. Its keyset watermark advances over every candidate EXAMINED rather than every candidate marked: a recovery that is genuinely still pending must be left alone, and keying on updated rows would hand back the same watermark forever and never terminate. Separate from migrations and startup, in the shape the other backfill commands already use, so a large comment table cannot block a deploy. Running it is a performance recovery step, not a correctness step — until it finishes the scan behaves exactly as it does today. Co-authored-by: multica-agent <[email protected]> * fix(server): close the review gaps in the settled-recovery backfill and paths Three fixes from review. 1. The backfill reproduced the bug it was written to fix. Its keyset ordered by id while idx_comment_delegated_failure_unsettled is (created_at, id), so the candidate scan read every unsettled row and sorted it to return LIMIT of them; and the receipt test ran a per-candidate EXISTS over agent_task_queue, the largest table in the database. The keyset now matches the index, and the receipt lookup is inverted into one agent_task_queue read per BATCH that hash-joins delivered ids back against the candidates. Measured on 500k comments / 200k tasks / 50k unsettled recoveries: 17,393ms and 816,963 temp blocks before, 235ms and no temp spill after. The candidate scan goes from reading 50,000 rows to reading the 1,000 it needs. Total walk time is flat across batch size (50x228ms, 10x1016ms, 5x2199ms — all ~11s), so batch size only trades how long one statement holds row locks. 1000 stays, now for a measured reason rather than a guessed one. 2. Bulk cancellation paths could strand a delivered receipt permanently unsettled, which lets the new index regrow the history it exists to remove. Manual rerun's pending-slot clear and agent archive were the two named in review; auditing the rest found comment-edit cancellation, both chat-session cancellations, workspace revoke and runtime teardown in the same position. All now settle — inside the caller's transaction where one exists, and through CaptureCancelledTasks for bulk paths that finalize outside one. The invariant is stated on the exported helper so the next such path has somewhere to read it. 3. --max-batches could not actually resume. The watermark lived only in the process, so a bounded rerun restarted at the front of the index — which is precisely where the rows this walk never settles live, so repeated bounded runs would rescan the same prefix and never reach the settleable history behind it. Adds --checkpoint-file (atomic replace, read on start) and explicit --after-created-at/--after-id, and logs the resume cursor on every exit path including SIGTERM. Co-authored-by: multica-agent <[email protected]> * fix(server): make delegated-failure settlement terminal-checked and atomic Two fixes from review. 1. The settlement SQL trusted its caller. "A delivery receipt is only immutable once the task is terminal" is the safety premise of the whole design, but the exported helper enforced nothing: passing a dispatched or running task would settle a receipt a reclaiming daemon can still replace, and the marker is monotonic, so nothing could undo it. The predicate now lives in the SQL, so a non-terminal task updates no rows instead of destroying the obligation. 2. Bulk settlement was post-commit best-effort, and the comment claiming the sweeper would replay a failed one was simply wrong: the covering task is terminal and already holds the receipt, so ListPendingDelegatedFailureRecoveries excludes the comment. It would neither replay nor settle — it would sit in the partial index forever, restoring the unbounded scan this change set removes. Every terminal write now commits with its settlement. CancelTasksForIssue / ForAgent / ByTriggerComment and the rerun pending-slot clear run in a transaction; CancelQueuedChatTasks settles inside the one it already had; the sweeper and daemon-recovery bulk fails move behind TaskService wrappers that own the transaction; agent archive gets CancelTasksForArchivedAgent, which cancels and settles atomically while still skipping the per-task broadcast the archive event makes redundant. BroadcastCancelledTasks, CaptureCancelledTasks and HandleFailedTasks run after their caller committed, so they no longer settle at all — a failure there could be neither rolled back nor repaired. The best-effort helper is deleted rather than left available; every remaining call site passes a qtx. Tests: a dispatched/running task with a receipt is not settled (fails without the SQL predicate), a bulk cancel whose settlement fails rolls the cancel back, and the archive path settles through its atomic entry point. Co-authored-by: multica-agent <[email protected]> * docs(server): correct settlement invariant comment and drop leaked test function The SettleDeliveredDelegatedFailureRecoveries doc still described the pre-fix contract — settlement reachable through CaptureCancelledTasks or a post-commit best-effort call — which is exactly the shape the atomicity fix removed. Restate the rule as it now stands (same qtx as the terminal write, never from a post-commit helper) so the stale text cannot guide a future change back into stranding rows in the partial index. TestBulkCancelRollsBackWhenSettlementFails dropped its trigger but left reject_recovery_settlement() behind in the shared test database; drop the function too. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> Co-authored-by: Eve <[email protected]> |
||
|
|
eddb0dc2ad |
MUL-6884 feat(metrics): attribute agent_runtime lookups by product source (#7822)
* feat(metrics): attribute agent_runtime lookups by product source
MUL-6884 asked which product API is behind the hot GetAgentRuntime query.
It has no single answer: a dozen unrelated paths issue the same primary-key
read, so pg_stat_statements sums them into one row and can report that the
statement is busy without saying which behaviour is driving it. Deciding
between "raise the heartbeat interval" and "back off the 500ms browser polls"
needs that split, and today it is a guess.
Add multica_agent_runtime_lookup_total{source,result} and route every
production read through service.RuntimeLookup, which carries the source with
the connection and classifies pgx.ErrNoRows as not_found rather than as an
error — a deleted runtime is the daemon's self-heal signal, not an outage.
The labels are a closed enum (15 sources x 3 results, all prewarmed at zero)
because this fires on the hottest read in the system; an unclassified call
site degrades to "other" instead of minting a series. No query, no timing
histogram, and no extra SQL on /metrics — this PR only makes the existing
calls countable.
An AST lint test fails CI on any direct Queries.GetAgentRuntime call outside
the wrapper, since a bypassed call site would quietly undercount whichever
source it belonged to and send the follow-up work after the wrong caller.
Co-authored-by: multica-agent <[email protected]>
* refactor(metrics): drop the standalone handler lookup file
The two helpers it held are not an endpoint — no route, no HTTP handler, no
new file under cmd/ — but a new agent_runtime_lookup.go under internal/handler
reads like one, and a file name that has to be opened before it can be ruled
out is a bad file name.
Move both into runtime.go next to requireRuntimeReadAccess, where the rest of
the runtime read helpers already live, and name the metrics test after what it
tests. No behaviour change.
Co-authored-by: multica-agent <[email protected]>
---------
Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
|
||
|
|
3adc0061a9 |
MUL-6490 fix(server): keep the A2A originator chain across cross-issue agent comments (#7381)
* fix(server): keep the originator chain across cross-issue agent comments An agent comment only recorded its authoring run in source_task_id when that run's task was on the SAME issue as the comment. The ordinary "agent creates issue Y, then coordinates there" flow therefore persisted a NULL lineage: the run that comment woke resolved as unattributed, and every @mention, assign and sub-issue inside it hit invocation_not_allowed — while the identical delegation on the originating issue succeeded. Admins recovered by widening public_to to workspace, which dismantles the minimum-privilege setup the gate exists for. Stamp the lineage unconditionally. The header it comes from is server-trusted (the task token forces X-Task-ID and X-Agent-ID from its own row; resolveActor rejects the JWT path unless the task belongs to the agent), and propagation is monotonic: a run can only carry the human it already acts for, and every hop re-runs canInvokeAgent against that same human. Substituting a human — the owner fallback the report proposed, or adopting the target issue's originator — stays rejected. The same-issue requirement moves to the consumers that actually need it: the reply-parent / no_action guards, and autopilotDelegationAuthority, which already verified task.issue_id itself (MUL-4857). The edit path re-stamps the same way, so the admission gate and the persisted chain can no longer disagree — the split that let the gate admit a delegation whose run then started with no authority. Also name the root cause in the logs when the gate denies an agent whose chain carries no human. The wire reason stays the deliberately generic invocation_not_allowed, so the one denial a workspace can fix was previously diagnosable only by querying agent_task_queue directly. Fixes #7328 Co-authored-by: multica-agent <[email protected]> * docs(skills): document that a delegation chain keeps its human across issues Co-authored-by: multica-agent <[email protected]> * test(server): build the cross-issue chain suite on the shared fixtures Rewrite the MUL-6490 regression suite against internal/testutil instead of hand-rolled setup, per the repo rule that DB-backed handler tests go through dbfx and testutil.Call: dbfx.Agent/Issue/Task/InsertNoID for rows (with explicit teardown for the grants and handler-written rows the fixture cannot know about), and testutil.WithHeaders/WithURLParams/Call for requests. Drive the assign and sub-issue delegations through the real UpdateIssue and CreateIssue handlers rather than calling validateAssigneePair directly. This fix's risk is that one entry point resolves the originator chain differently from another, and only the wired path can hold that: the suite now runs {mention, assign, sub-issue} x {an allow-listed human's chain, another member's chain, no chain} from inside a run woken across an issue boundary. Issue numbers come from the workspace counter here, because CreateIssue allocates from it and dbfx's MAX(number)+1 default would collide with the sub-issue the test asks the handler to create. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
b4bfe083be |
MUL-6407: mint UUIDv7 primary keys for append-heavy tables (#7230)
* perf(db): mint UUIDv7 primary keys for the append-heavy tables
Random v4 primary keys scatter every insert across the whole primary-key
B-tree, so each row dirties a different page. On the tables that grow
continuously — the task queue, messages, comments, activity and audit rows —
that is pure write amplification. UUIDv7 puts a big-endian millisecond
timestamp in the leading 48 bits, so consecutive inserts share a page.
The 12 tables converted here are the append-only hot paths:
agent_task_queue, activity_log, inbox_item, comment, issue, chat_session,
chat_message, autopilot_run, webhook_delivery, task_token,
channel_inbound_audit, sys_cron_executions
Ids come from the new pkg/dbid, which returns a pgtype.UUID ready to assign to
a sqlc params struct. Upsert-dominated tables (task_usage, agent_runtime, ...)
are deliberately left alone: v7 only buys locality for rows that are actually
inserted. Lease and claim tokens keep gen_random_uuid() — they must stay
unpredictable and carry no timestamp.
No migration, and no backfill. Every converted INSERT binds its id as
COALESCE(sqlc.narg('id')::uuid, gen_random_uuid()), so the column default stays
reachable: a caller that passes no id inserts exactly as it does today. Tables
will hold a mix of v4 and v7 ids permanently, which the uuid type and every
parser in the repo already accept.
Verification: - pkg/dbid: version/variant, uniqueness, and a non-decreasing timestamp
prefix; a shape test asserting every converted query keeps the COALESCE
fallback; and a source scan asserting every production params literal mints
an id, so a call site added later cannot silently fall back to v4.
- internal/handler: round-trips issue, comment, activity_log, inbox_item,
chat_session, chat_message, agent_task_queue, task_token and
channel_inbound_audit against Postgres and asserts the stored id is v7,
plus one case proving an unset id still inserts (as v4).
- internal/scheduler: covers the one hand-written INSERT, asserting the
execution id is v7 while lease_token stays v4.
Co-authored-by: multica-agent <[email protected]>
* fix(db): address UUIDv7 review feedback
Co-authored-by: multica-agent <[email protected]>
* fix(db): cover Telegram audit UUIDv7
Co-authored-by: multica-agent <[email protected]>
---------
Co-authored-by: Sol-Boy <[email protected]>
Co-authored-by: multica-agent <[email protected]>
|
||
|
|
5d5fe64672 |
MUL-6343: Add semantic issue activity timestamps (#7193)
* feat: add semantic issue activity timestamps Co-authored-by: multica-agent <[email protected]> * fix(migrations): register issue activity index cleanup Co-authored-by: multica-agent <[email protected]> * fix(issues): address last activity review Co-authored-by: multica-agent <[email protected]> * fix(issues): close last activity review gaps Co-authored-by: multica-agent <[email protected]> * fix(issues): harden activity rollout safeguards Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Sol-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
YYClaw
|
4b8873df3a |
feat(issues): add revision-aware concurrency guards (#7038)
* feat(issues): add revision-aware concurrency guards * fix(concurrency): decouple text edits from aggregate revisions |
||
|
|
afd05ca221 |
MUL-6063: recover terminal delegated task failures (#6844)
* fix: recover terminal delegated task failures Co-authored-by: multica-agent <[email protected]> * fix(tasks): make delegated failure recovery durable Co-authored-by: multica-agent <[email protected]> * fix(tasks): bound delegated failure recovery Co-authored-by: multica-agent <[email protected]> * fix(tasks): address delegated recovery review nits Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Eve <[email protected]> Co-authored-by: multica-agent <[email protected]> Co-authored-by: Sol-Boy <[email protected]> |
||
|
|
b30a3ae55c |
MUL-6164 feat(runtime): take an unusable agent CLI offline and refuse its triggers with the fix (#6966)
* fix(daemon): take a runtime offline when its CLI cannot be executed (MUL-6164) A CLI that cannot exec was classified as a transient probe failure, so the runtime stayed online and every task it claimed died at fork/exec. The user in MUL-6164 hit the same error once per task with no signal that the runtime was the problem — an npm placeholder stub passes exec.LookPath, so nothing else the daemon checks can tell it apart from a working install. ENOEXEC (and its Windows spelling, ERROR_BAD_EXE_FORMAT) is a fact about the bytes on disk, not a hiccup: it will reproduce until someone reinstalls. Give it its own probe verdict and route it down the track below-minimum already uses — claim barrier, seq-stamped hold, deregistration — so the provider goes offline with its reason instead of collecting failed tasks. Recovery needs no new machinery: a demoted provider has no runtime, which puts it back in providersMissingRuntimes, and the first healthy probe clears the hold and lets converge register it again, with no restart. One thing the below-minimum track did not need: a confirmation window. The repair we tell users to run overwrites the bin entry in place, so a probe landing mid-copy sees a truncated file. Demotion therefore requires two sightings a window apart; a genuinely broken install pays one extra probe round, and a spurious verdict would in any case recover on its own. detectBuiltinRuntimes' second return value is now "providers condemned this round, with the evidence against them" rather than "below-minimum versions" — same shape, both kinds, and the value is the reason /health already shows. Co-authored-by: multica-agent <[email protected]> * feat(runtime): refuse triggers for an unusable agent CLI instead of queueing them silently (MUL-6164) Taking the runtime offline stopped the per-task failures, but it replaced them with silence: the next @mention or assignment queued as if the machine were merely asleep, and nothing ran for two hours until queued_expired. Waiting only makes sense when waiting is a plan — a sleeping laptop comes back on its own; a CLI that cannot be executed does not. dispatch/reason.go already draws exactly that line between runtime_offline ("bring the machine back, queued work waits") and agent_runtime_required ("nothing will ever claim this"), and warns that collapsing the two sends users to fix the wrong thing. This adds the third member of that family, runtime_unusable, and routes the new verdict to it end to end: - The daemon reports WHY on deregistration — a stable code plus the repair command read from the package's own manifest, since only that side knows the path. Stored on agent_runtime.metadata, which registration overwrites wholesale, so a repaired runtime drops the explanation structurally. - AgentReadiness now returns a verdict (available / waitable / blocked) instead of a bool plus prose, and the direct-agent trigger paths start consulting it — blocking ONLY on the blocked verdict, so an offline machine still queues. agentReadinessReasonCode, which re-derived the same code from agent state, is gone: one source, no drift. - Every path that refuses without a human watching the response leaves the reason on the issue: an assignment, or another agent's @mention. The interactive surfaces already carry the code back to the composer, which now localizes it in all four languages. Below-minimum keeps today's behaviour deliberately. It is equally futile to wait on, but changing when THAT refuses a trigger is a separate product decision. Co-authored-by: multica-agent <[email protected]> * fix(runtime): keep the unusable verdict through a late register, and repair Windows (MUL-6164 review) Two ways the refusal path failed to reach the user. The verdict could be erased by an older answer. A register whose request was still in flight across the demotion upserts the runtime row afterwards, and that upsert overwrites metadata wholesale — including the reason just stored. The local hold refuses the response correctly, but the cleanup that took the revived row offline again sent no reason, so the server ended up "offline, cause unknown". Every admission path reads that as "the machine will come back" and queues, which is exactly the behaviour this change exists to remove. The demotion record now keeps the structured cause and the revived-row cleanup re-attaches it per row. The regression test drives the real interleave — a register held inside the server handler across the verdict — and fails without the fix. The repair command was POSIX-only. Windows had just gained detection (ERROR_BAD_EXE_FORMAT), so a Windows user was correctly refused and then handed `cd 'C:\...' && node install.cjs`: cmd.exe treats the quotes literally and PowerShell 5.1 has no `&&`. The command is now rendered for the shell of the machine that reported it — Set-Location on two lines for PowerShell, with PowerShell's own quote escaping — and carries that shell so the notice fences it truthfully instead of labelling everything bash. Also corrected the notice's own docs: it is written for every author, not only unattended ones. The chip and toast carry the reason code but not the repair command, so a human needs the durable copy too. The behaviour was already this; the comment claimed otherwise. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
a937710fc7 |
MUL-5958: 回复人类评论时跳过 assignee 兜底 (#6676)
* fix(comments): skip assignee fallback for member replies Co-authored-by: multica-agent <[email protected]> * test(comments): align member reply routing e2e Co-authored-by: multica-agent <[email protected]> * docs: clarify reply assignee fallback Co-authored-by: multica-agent <[email protected]> * docs: clarify localized assignee wording Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Eve <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
YYClaw
|
24d209794e | fix(channel): attach /issue media to created issues (#6388) | ||
|
|
d38da27ed4 |
MUL-5619: fix(cli): surface the server's 409 message instead of the generic conflict template (#6267)
* fix(cli): show the server's conflict message instead of the generic 409 template
Every 409 this API returns is a deterministic refusal that names its own fix
("a skill with this name already exists", "set parent_id (--parent) to <id>").
The CLI replaced all of them with a template that says the opposite — that the
state changed underneath you and you should re-fetch and retry. Agents took the
retry hint literally: GH #6264 reports 15+ identical retries over 10 minutes
followed by hours spent chasing an optimistic-concurrency theory that never
existed, and GH #5948 is a second user misdiagnosing the same way. MUL-4417 had
already written the useful message server-side; it just never reached anyone.
Route 409 through the same server-message extraction 400/422 already uses, so
roughly forty hand-written conflict messages across skills, agents, runtimes,
labels, projects and comments become visible by default. A body we cannot
recognize still falls back to the template, so this never dumps a raw response.
extractServerMessage now prefers prose over a bare identifier, because a few
endpoints put a stable code in "error" and the sentence in "message".
MUL-5619
Co-authored-by: multica-agent <[email protected]>
* fix(comments): stop telling a wrong --parent that it posted a top-level comment
The reply guard returns one message for two different mistakes. A resumed
session that carries a previous turn's --parent forward (GH #6264) did not ask
for a top-level comment, but is told it did — which sends it looking for a
new-thread opt-in (GH #5383) instead of correcting the parent it already passed.
Split the copy: name the rejected parent when one was supplied, and keep the
existing top-level wording for the parentless case. Both still point at the
trigger comment to use.
MUL-5619
Co-authored-by: multica-agent <[email protected]>
* fix(runtime): return 500, not a 409 echo, when the update store fails
InitiateUpdate answered every UpdateStore.Create failure with a 409 carrying
err.Error(). The in-memory store only ever returns errUpdateInProgress, so this
looked safe — but the Redis store also wraps infrastructure failures as
"reserve active update: <dial error>" and "persist update request: <error>".
Surfacing 409 bodies in the CLI turns that into a user-visible leak of internal
addresses, and labels an outage as a conflict the caller could fix by retrying.
Classify instead: errUpdateInProgress keeps its 409 and its actionable message,
everything else is logged and answered with a 500 and fixed copy.
Also pins the prose-over-machine-code preference for validation bodies, which
the shared extractor applies to 400/422 as well as 409. Only the issue-table
endpoints are shaped that way and none is reachable from the CLI today, but the
change is intentional and should fail loudly if reverted.
MUL-5619
Co-authored-by: multica-agent <[email protected]>
---------
Co-authored-by: Bohan-J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
|
||
|
|
b06af2ae17 |
feat(runtime): unbind agents on runtime delete instead of destroying them (#6220)
* feat(runtime): unbind agents on runtime delete instead of destroying them Deleting a runtime archived its agents and then hard-deleted the rows, so the agents and every conversation with them disappeared — while the confirmation dialog said "archive", which a user reasonably reads as recoverable. Retiring a laptop is an ordinary action; losing the agents configured on it is not an ordinary consequence. An agent is now a persistent business object and a runtime is replaceable execution capacity: deleting a runtime unbinds its agents. `runtime_id IS NULL` means unbound — orthogonal to archived — and the agent keeps its instructions, skills, chats, labels, channel installations, autopilots and task history. service.AgentReadiness already refused an agent with no runtime, so the scheduling safety gate needed no change. Two columns become nullable, not one. Without `agent_task_queue.runtime_id`, deleting the runtime still cascades the task history away (and task_message / task_usage / task_token with it), so the agents would survive with no record of anything they did — the same class of loss. A NOT VALID CHECK keeps NULL confined to history: an active task must always have a runtime, so claim / dispatch / delivery-CAS paths can never observe one without. It is written against completed_at rather than a status list so a future non-terminal status fails closed instead of slipping through. Two prerequisites this depends on: - 'deferred' (migration 128) was missing from CancelAgentTasksByRuntimeOrAgent. It went unnoticed because the delete used to cascade those rows away; with the new CHECK it would abort the delete and make the runtime undeletable. - The channel-installation / label / chat-pin / invocation-target / draft-restore cleanups were scoped to "archived agents on this runtime". Archived user agents now survive, so that scope is narrowed to kind='system' — otherwise the fix would produce a subtler loss: agent alive, configuration wiped. Also removes the squad guard that refused (409) when an active squad's leader was an archived agent on the runtime, plus the archived-squad delete that existed only to get past squad.leader_id's RESTRICT FK. The leader is no longer deleted, so nothing needs to be given up to retire a machine. Autopilots are no longer paused either: their assignee survives, and a rebind restores them without the owner having to remember to re-enable. Reason codes: an unbound agent reports agent_runtime_required, not runtime_offline. The copy for runtime_offline tells users to reconnect a machine; an unbound agent has no machine to reconnect, and the fix is to bind a runtime. Chat's bare 409 string gains the same code so the composer can offer that action. API: agents gain runtime_bound. runtime_id stays a string (empty when unbound) so installed clients keep parsing and no gated two-release rollout is needed. The confirmed-delete endpoint is /unbind-agents-and-delete; /archive-agents-and-delete still routes to it, and the compared expected_active_agent_ids set is unchanged — widening it would 409 every older client forever. Co-authored-by: multica-agent <[email protected]> * fix: make runtime unbinding recoverable Co-authored-by: multica-agent <[email protected]> * fix: address runtime unbind review nits Co-authored-by: multica-agent <[email protected]> * fix: resolve runtime unbind review blockers Co-authored-by: multica-agent <[email protected]> * fix(migrations): renumber runtime unbind after main merge Co-authored-by: multica-agent <[email protected]> * test(daemon): avoid late-request lease flake Co-authored-by: multica-agent <[email protected]> * test(autopilots): bind validation fixture runtime Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Eve <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * fix(timeline): preserve complete threads under comment cap Co-authored-by: multica-agent <[email protected]> * fix(comments): preserve newest bounded views Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Eve <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
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 <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
---------
Co-authored-by: Lambda <[email protected]>
Co-authored-by: multica-agent <[email protected]>
|
||
|
|
f73250654f |
fix(issues): stop blaming permission for an unresolved mention target (MUL-5548) (#6190)
* fix(issues): stop blaming permission for an unresolved mention target (MUL-5548) A well-formed but wrong agent mention UUID comes back as `invocation_not_allowed`, and the UI rendered that as "You don't have permission to use this target". The server never claimed a permission cause: `invocation_not_allowed` is deliberately ambiguous so a blocked reason cannot confirm that a private agent in another workspace exists (dispatch/reason.go). The copy turned a typo into an access-control investigation — GH #6181 hit exactly this on a squad handoff. - Reword the blocked-trigger labels in all four locales to name both possibilities instead of asserting permission, and record in blocked-trigger-copy.ts why a label must not narrow the wire code. - Report a mention id that is not a valid UUID at all (`mention://agent/-`) as `target_unavailable`, matching the squad branch beside it and the autopilot admission path. A non-UUID names no entity in any workspace, so it conceals nothing and must not be blamed on invoke permission. The well-formed-but-unresolved case is unchanged and still shares `invocation_not_allowed` with a private agent — the enumeration boundary this issue asked us to move stays exactly where it is. Also refresh the multica-mentioning skill: the mention path gates on `canInvokeAgent`, not `canAccessPrivateAgent` (split in MUL-3963), and the skill now tells agents to check a mention UUID against the roster before touching any visibility setting. Co-authored-by: multica-agent <[email protected]> * docs(skills): correct the mentioning skill's "silent no-op" framing (MUL-5548) Review nit on #6190: the source map still titled its table "Guards that make a valid mention a silent no-op", but a parsed mention is never silently dropped — it is either blocked with a reason_code or folded into a running task. Several rows in the same table also pointed at comment.go:14xx line numbers that had drifted. - Retitle to "Guards and outcomes for a parsed mention" and split the outcome into its own column, so each row states the reason_code it produces. - Replace the drifted line numbers with stable search anchors, matching the convention the newer rows in this file already use. - Correct two rows that were wrong, not just stale: archived / no-runtime targets are blocked (target_unavailable, runtime_offline), and an already-pending target is a coalesce/defer fold, not a skip. - Apply the same correction to SKILL.md, where the frontmatter and the "What does NOT happen" section told agents an already-pending mention was dropped. It is folded into the running task and still gets read — worth being exact about, since believing otherwise invites a re-post. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
607209de7c |
fix(avatar): serve avatars through a signed endpoint on private buckets (MUL-5393) (#6088)
* fix(avatar): serve avatars through a signed endpoint on private buckets (MUL-5393) Avatar uploads persisted the raw storage object URL into `avatar_url`. On a deployment whose bucket is private and has no public CDN domain (S3 with Block Public Access, R2, MinIO) that URL is a guaranteed 403 in the browser: ATTACHMENT_DOWNLOAD_MODE only ever applied to the attachment download endpoint, so every user / agent / squad / workspace avatar rendered broken even though the upload itself succeeded. Resolve at read time instead of at upload time. What is persisted stays the durable object reference, so nothing with a TTL is ever written to the database and avatars saved by an older build are fixed without a backfill. What is served is `/api/avatars/<sig>/<key>`, a stable URL the server resolves per request through the deployment's existing storage download policy (presigned redirect, CloudFront-signed redirect, or proxied body). The endpoint is unauthenticated and the HMAC signature is the credential: the session cookie is SameSite=Strict, so an auth-gated URL cannot be a native <img src> from Desktop, a mobile webview, or a split-origin self-hosted web app. The signature covers the storage key and only image extensions resolve, so an avatar_url pointed at a private document cannot launder it into a publicly fetchable URL. Deployments that already work are untouched: a public CDN domain without per-request signing, and the local-disk backend whose /uploads/* route is public, both keep returning the raw URL. Fixes #6024 Co-authored-by: multica-agent <[email protected]> * fix(avatar): only publish avatar-class objects through the signed endpoint (MUL-5393) Review found that being able to name a storage object was treated as permission to publish it. `ownedStorageKey` proved only that a URL came from this deployment's storage, and every image-shaped key was then signed — while the avatar update endpoints accepted any raw storage URL. A caller who had seen a private image attachment's URL could submit it as their own avatar, and the unauthenticated endpoint would keep re-signing it indefinitely. A user avatar propagates to every workspace that user belongs to, so the leak crossed workspace boundaries. Add the missing authorization rule: an object is serveable as an avatar only when it is avatar-class — a standalone image upload not attached to an issue, comment, chat session, chat message, or task. The check resolves the backing attachment row from the id UploadFile embeds in the object filename, so it needs no lookup by URL and no new index. It is enforced on both sides. The write side rejects such a value with 403 before anything is stored; the read side re-checks per request, which is what makes the guarantee hold for rows written before this existed and revokes the URL if an object is later bound to a comment or chat. Scope is the `workspaces/` namespace — the only place that can hold content belonging to someone other than whoever is setting the avatar, covering both uploads and channel media ingest. Keys elsewhere (the per-user standalone namespace, or objects an operator placed in the bucket) stay usable, which keeps the documented "an explicit avatar_url is preserved" contract intact. Uploader identity is deliberately not part of the rule: duplicating an agent legitimately reuses the source agent's avatar object, which a different admin may have uploaded. Publishing someone else's unbound image would require knowing its URL, and unbound rows appear in no listing endpoint. Also clamp the 302's cache lifetime to half the signed URL's own TTL (0 -> no-store). ATTACHMENT_DOWNLOAD_URL_TTL takes any positive duration, so the fixed 60s could outlive the target it pointed at on a short-TTL deployment. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
45e24e3565 |
fix(comment): @all no longer swallows an explicit @agent mention (MUL-5411) (#6048)
* fix(comment): @all no longer swallows an explicit @agent mention (MUL-5411) computeCommentAgentTriggers short-circuited on `util.HasMentionAll` before it looked for explicit mentions, so a comment carrying both `@all` and an `@agent` / `@squad` mention enqueued nothing at all — the named target never ran. Evaluate the explicit-mention branch first; `@all` now only suppresses the implicit routes (assignee / thread parent / conversation), which was its intent. `all` is neither "agent" nor "squad", so it is still skipped inside resolveMentionedAgentCommentTriggers and never enqueues a run of its own. Tests: preview + create coverage for @all + @agent (mentioned agent only, no assignee fallback), @all + @squad (leader wakes), @all + @member (still suppressed), plus an end-to-end subtest in the @all suppression integration test. Refreshed the builtin multica-mentioning skill and its source map, which documented the old short-circuit and a function name that no longer exists. Co-authored-by: multica-agent <[email protected]> * fix(comment): malformed mention id no longer panics the trigger resolver Review finding on PR #6048. MentionRe accepts any `[0-9a-fA-F-]+` id, so `mention://agent/-` parses as a real mention, and resolveMentionedAgentCommentTriggers handed it to parseUUID (util.MustParseUUID) — a panic on attacker-controlled comment text. Preview returned a 500; on the create path the comment row was already committed before the panic fired. Parse both the agent and the squad mention id with the error-returning util.ParseUUID (the convention routeFirstExplicitRootMentionOwner already follows) and record a blocked outcome instead: agent → invocation_not_allowed, squad → target_unavailable. Those are the same enumeration-safe codes a well-formed id that owns no entity already produces, so a malformed id reveals nothing new and never enqueues. The panic predated the @all reordering on plain explicit mentions; the reorder made it reachable for @all + malformed id too, so it is closed here. Tests: table-driven preview + create regression for a bare `-` agent id, a short hex agent id, a malformed squad id, and both @all combinations — asserting no panic, no trigger, and the exact blocked outcome. Verified the tests fail with the panic before the fix. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Eve <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
Bohan Jiang
|
49fd6cd08a |
fix(handler,service): return 409/coalesced instead of 500 on duplicate-key violations (MUL-5285) (#5958)
Two unique-constraint violations surfaced as an HTTP 500 with the raw Postgres constraint name leaked to the caller (#5914). - UpdateAgent now mirrors CreateAgent: a 23505 / agent_workspace_name_unique violation returns a clean 409 instead of a 500 whose body leaked the constraint name. The (workspace_id, name) constraint does not exclude archived agents, so renaming into a name still held by an archived agent hit exactly this path. - The mention enqueue detects the idx_one_pending_task_per_issue_agent violation, returns a bare typed sentinel (no driver text), and logs the benign race at debug instead of error. Review then hardened the same duplicate-enqueue path so it never reports an outcome it cannot back: coalesced only after an atomic head-scoped merge, deferred only when an active task's reconcile will replay the comment, queued only after a fresh enqueue, and an honest internal_error otherwise. Head scoping (TEN-356) is preserved throughout, planned-id registration excludes queued tasks so re-attribution stays atomic (MUL-4302), and a blocked completion replay is handed on rather than discarded. One shape remains best-effort — a different-head queued blocker can neither cover the comment nor accept it without violating TEN-356 — and is logged at error; that drop predates this change. Tracked in #5985. No migration, foreign key, or cascade. MUL-5285 Fixes #5914 |
||
|
|
69744241e8 |
fix(comment): restore autopilot @mention delegation authority (MUL-4857) (#5512)
* fix(comment): restore autopilot @mention delegation authority (MUL-4857) A schedule/webhook autopilot run is unattributed by design (no top-of-chain human originator, MUL-4302). Since MUL-3963 the A2A invoke gate (canInvokeAgent) keys on that originator, so a mid-run @agent/@squad delegation on an autopilot-created issue fails closed for the DEFAULT private agent (and member-scoped public_to agents): the mention renders but no run is enqueued. The SAME autopilot's first dispatch is admitted via the autopilot creator (autopilotAdmitInvoke -> canCreatorInvokeAgent), so first-dispatch and mid-run delegation disagreed. Align them: when an unattributed agent/system-authored comment on an autopilot-origin issue reaches computeCommentAgentTriggers with no originator, fall back to the autopilot creator as the effective invoking user for the gate. The gate still runs (no unrestricted agent-to-agent bypass); it is authorization only -- the enqueued task's originator/attribution stays unattributed. Scoped to autopilot-origin issues so other unattributed chains stay fail-closed. Adds a DB-backed regression test covering: creator-owns-target admits, a non-autopilot unattributed run stays denied, and a creator without invoke rights stays denied. Co-authored-by: multica-agent <[email protected]> * fix(comment): bind autopilot @mention authority to verified task lineage (MUL-4857) Address the review's confused-deputy finding on the P0 fix. The first cut keyed the invoke-gate fallback on issue provenance + an empty originator alone (invokeAuthorityForAutopilotIssue took only the issue), so any unattributed run could borrow a stranger autopilot creator's rights merely by commenting on that autopilot's issue — and the fallback also leaked past explicit @mention into the plain-comment squad-leader path and system actors. Rework it so the autopilot-creator authority is granted ONLY when the SPEAKING task's lineage is verified against this issue: - resolve the authority separately (new AutopilotDelegationAuthorityUserID on commentTriggerComputeOptions), never by overwriting OriginatorUserID; the gate reads it through opts.effectiveInvoker() only when no human originator resolved, so attribution stays untouched; - resolve from a server-trusted speaking task — X-Task-ID on create/preview, comment.source_task_id on edit/reconcile — via autopilotDelegationAuthority, which admits only when author == task agent AND task.issue_id == this issue AND the issue is autopilot-origin, then keys on the member autopilot creator; - do NOT key on autopilot_run_id: in create_issue mode (the reported case) the leader task is enqueued through the ordinary issue-assignment path and has no autopilot_run_id — the task.issue_id == issue binding is what proves the run is part of this autopilot's work while rejecting foreign-issue runs. Tests: replace the provenance-only regression with lineage-bound coverage — verified-lineage-admits, creator-without-rights-denied, non-autopilot-denied, missing-source-task-denied, cross-issue-source-task-denied, author!=task-agent- denied — plus an end-to-end CreateComment path asserting the private worker is enqueued and the delegated run stays unattributed. Verified the fallback is load-bearing (positive + e2e fail with it disabled) and the full internal/handler package passes. Skill docs (multica-mentioning) updated to the lineage-bound contract and new helper names. Co-authored-by: multica-agent <[email protected]> * fix(comment): make autopilot @mention authority consistent across defer/edit (MUL-4857) Second review round (Elon) surfaced two must-fixes on top of the lineage binding. 1. Busy-target completion reconcile lost the authority. A delegation to a target that is already running is deferred to that target's completion reconcile (reconcileCommentsOnCompletion). That path recomputed triggers with only the (empty) originator, so an unattributed autopilot delegation's follow-up was gate-denied again and silently dropped. It now restores the delegation authority from comment.source_task_id, so the follow-up fires once the target frees up — still unattributed. 2. Edit could borrow the old authoring run's authority, and preview != save. The edit preview keyed authority on the current request task while save keyed it on the comment's original source_task_id, so an agent editing its old autopilot comment from a task on an UNRELATED issue would fail-closed in preview but reuse the old autopilot creator's authority on save (cross-issue confused-deputy, and a preview/side-effect divergence). Fix: treat source_task_id as the persisted per-action authority lineage and re-stamp it on edit to the CURRENT editing task, issue-scoped exactly like CreateComment. A cross-issue edit re-stamps it to NULL, so preview, save, AND the deferred reconcile all fail closed identically. UpdateComment query gains a source_task_id param (sqlc regen). Also locks the review-accepted behavior that effectiveInvoker() carries the autopilot-creator authority into the plain assigned-squad-leader wake (a worker's result comment on the autopilot issue can still wake the private leader). Tests: reconcile-restores-authority (owns -> one unattributed follow-up; no rights -> none); edit re-stamp (same-issue keeps authority and triggers; cross-issue clears source_task_id and fails closed); worker-result wakes private squad leader. Verified both fixes are load-bearing (each negative control reproduces the exact regression Elon described), full internal/handler + internal/service packages pass, gofmt/vet clean. Skill docs (multica-mentioning) updated. Co-authored-by: multica-agent <[email protected]> * fix(comment): clear stale task lineage on non-author comment edits (MUL-4857) An admin editing an autopilot Agent's comment previously preserved the comment's original source_task_id. The immediate save is judged on the admin's member identity and correctly fails closed, but the deferred completion-reconcile routes the comment under its original agent author and resolved the delegation authority from the stale source_task_id, resurrecting the autopilot creator's invoke authority once the busy target freed up — an admin (manage rights) could thereby trigger another owner's private agent (invoke rights). Now a content edit re-derives lineage from the edit action: only the agent author editing its own comment re-stamps source_task_id to the current editing task; every other editor (member/admin, or any non-author) clears it, so preview, save, and reconcile all fail closed. Adds a regression covering the admin-edit + busy-target path and syncs the multica-mentioning skill docs. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
4fac8d772f |
feat(attribution): Human Attribution Phase 1 (MUL-4302) (#5150)
* feat(attribution): Phase 1 foundation — provenance schema + resolver (MUL-4302)
Human Attribution, Phase 1 (地基) first increment. Every agent run must be
traceable to exactly one accountable human AND record at which waterfall level
that human was resolved, so a NULL originator can be told apart from a genuine
'no human in the chain'.
- migration 149: add originator_source (waterfall label) + delegation/retry/
rerun/rule-version lineage + kind-tagged trigger evidence to agent_task_queue.
No FK, no cascade, no CHECK on the source enum (MUL-4302 §7); nullable ADD
COLUMNs = fast metadata-only change on the hot queue table.
- internal/attribution: the accountable-human vocabulary (Source, EvidenceKind,
TriggerKind) + pure, unit-tested classification rules (ClassifyComment/
ClassifyDirect). No DB, no authorization — provenance labeling only.
- service: attributionFor{IssueTask,TriggerComment} gather facts and delegate to
the pure classifier; the legacy originator resolvers now delegate here so
there is one source of truth. originator_user_id's VALUE is unchanged, so the
Composio-overlay and canInvokeAgent A2A authorization boundaries are
byte-for-byte preserved (MUL-4302 §1.3).
- enqueueIssueTask / enqueueMentionTask stamp originator_source + evidence;
CreateRetryTask carries the parent attribution forward and records
retry_of_task_id so retry and manual rerun stay separable (MUL-4302 §5).
Verified: go build ./..., go vet, gofmt clean; new attribution unit tests +
enqueue stamping integration test green; existing resolve_originator tests
unchanged.
Co-authored-by: multica-agent <[email protected]>
* feat(attribution): split accountable_user_id from originator, close enqueue bypasses (MUL-4302)
Phase 1, per Bohan's decision on the MUL-4302 thread: audit and authorization
answer different questions and get different columns.
- Migration 151 adds agent_task_queue.accountable_user_id (no FK, no cascade).
Authorization keeps reading ONLY originator_user_id (canInvokeAgent A2A gate,
Composio overlay); audit/UI/usage read accountable_user_id + source + evidence.
- Invariant (finalizeAttribution, single chokepoint + §11 tests): originator
non-null ⟹ accountable equals it. The two diverge only when originator is null
(autopilot / degraded fallback), which is the deferred rule_owner/owner_fallback
increment; this lands the column + mirror-write so that split has a home.
- Close the NULL-source enqueue bypasses Elon flagged: chat, quick-create,
deferred-fallback and run_only-autopilot now stamp originator_source + evidence
(+ accountable where a human exists). Autopilot stays unattributed until the
rule-version snapshot table lands, but is no longer a silent NULL-source row.
Retry inherits accountable_user_id like the rest of the attribution lineage.
- Fix assign/promote attribution (§4): a member who assigns/promotes an existing
issue is now the accountable human (and, by the invariant, originator) ahead of
the issue creator. Threaded as an OPTIONAL actor override, so comment/rerun/
autopilot paths keep today's resolution and create-with-assignee (creator ==
actor) is unchanged. The squad leader gate already judged the same member.
Also merges origin/main: renumbers the attribution migration 149→150 (main took
149 for issue_origin_agent_create) and folds agent_create into ClassifyDirect's
origin inheritance.
go build/vet/gofmt clean; attribution unit tests + service stamp/actor tests +
handler suite pass on a fresh DB migrated through 151.
Co-authored-by: multica-agent <[email protected]>
* docs(attribution): fix accountable NULL semantics + close chat/quick-create evidence boundary (MUL-4302)
Addresses Elon's 2nd-round review on PR #5150 (pre-merge doc/evidence items):
- Migration 151 no longer overclaims NULL. accountable_user_id is NULL not only
on pre-migration rows but on NEW rows whose audit source resolved no human yet
(run_only autopilot writes originator_source='unattributed' with NULL
accountable until rule_owner lands). Header + COMMENT ON COLUMN reworded so a
schema reader does not misjudge the invariant.
- Chat now uses the UNIFORM evidence pair (kind=chat, ref=chat_session_id), like
autopilot_run/issue_assignment, instead of relying only on the dedicated
chat_session_id column — new EvidenceChat kind. Added a service test asserting
chat stamps direct_human + chat evidence.
- Quick-create is documented as the ONE intentional no-antecedent-row path: no
comment/issue/session/run exists at enqueue time (the run creates the issue), so
trigger_evidence_kind/ref stay NULL while the human rides originator/accountable
and source is direct_human — not a NULL-source bypass.
No authorization behavior change. attribution + service + handler suites pass on a
DB migrated through 151.
Co-authored-by: multica-agent <[email protected]>
* chore(attribution): renumber migrations 150/151 → 157/158 after merging main (MUL-4302)
main's #5162 ("unblock release migrations") renumbered the chat migrations and
took 150 (agent_task_coalesced_comments) and 151 (chat_read_cursor), colliding
with this branch's attribution migrations. Renumber them above main's new highest
(156) so TestMigrationNumericPrefixesStayUniqueAfterLegacySet passes:
- 150_agent_task_attribution → 157_agent_task_attribution
- 151_agent_task_accountable_user → 158_agent_task_accountable_user
Fixed the internal "migration 150" references in 158's header to 157. Migrations
apply cleanly through 158 on a fresh DB; migration lint green.
Co-authored-by: multica-agent <[email protected]>
* feat(attribution): autopilot rule_owner — accountable = rule version publisher (MUL-4302)
Implements rule_owner (MUL-4302 §3.4), the first attribution source where the
accountable human diverges from the (NULL) authorization originator.
- Migration 159 adds the append-only autopilot_rule_version snapshot table (no FK,
no cascade); migration 160 adds its CONCURRENTLY lookup index.
- Write-on-publish: CreateAutopilot appends v1 (publisher = creator); UpdateAutopilot
appends a new version when a SUBSTANTIVE autopilot-row field changes (assignee /
status / execution_mode) — cosmetic edits (title/description/template) write none.
Both run inside the existing handler tx (atomic with the autopilot write).
- Dispatch resolution: both autopilot execution modes now resolve the active rule
version and stamp originator_source='rule_owner', accountable_user_id=publisher,
rule_version_id=<snapshot>, with originator_user_id left NULL (authorization
unchanged). run_only stamps CreateAutopilotTask directly; create_issue resolves in
attributionForIssueTask so both modes attribute identically. A missing version /
non-member publisher degrades to unattributed — never fabricates a human.
- finalizeAttribution now enforces the invariant ONE-WAY: it mirrors originator onto
accountable only when originator is valid, leaving an explicitly-set accountable
(rule_owner / future owner_fallback) intact when originator is NULL. Added
rule_version_id to CreateAgentTask so the create_issue path persists it too.
Also merges origin/main and renumbers this branch's attribution migrations
150/151 → 157/158 (main's #5162 took 150/151); rule_version table is 159/160.
Tests: attribution unit RuleOwner + one-way invariant table; service integration
tests proving an autopilot-origin issue stamps rule_owner + rule_version_id (and
degrades to unattributed with no version). Full service/attribution/handler/
migration suites pass on a DB migrated through 160; build/vet/gofmt clean.
Deferred (same PR): trigger-table republish (cron/webhook/event_filters) and
system-pause/archive versioning; owner_fallback + fail-closed; manual rerun.
Co-authored-by: multica-agent <[email protected]>
* fix(attribution): manual autopilot trigger → direct_human to the triggering member (MUL-4302)
Elon's blocking finding: a member manually triggering an autopilot was attributed
rule_owner (accountable = rule publisher, originator NULL) like a schedule/webhook
run, so member B triggering member A's autopilot landed accountable=A and carried
no originator authorization context for the run. Per MUL-4302 §4 a manual "run now"
is a direct human action and must attribute direct_human to the triggering member.
- Thread the triggering member from TriggerAutopilot into dispatch: new
DispatchAutopilotManual carries actorUserID (resolved via resolveActor +
memberActorUserID, so only a member actor is a human; an A2A agent actor falls
back to rule_owner). DispatchAutopilot / DispatchAutopilotForPlan keep their
public signatures (pass an invalid actor); only the internal dispatchAutopilot /
dispatchCreateIssue / dispatchRunOnly gained the param, so the many existing
callers are untouched.
- run_only: dispatchRunOnly stamps direct_human (originator == accountable ==
actor, no rule_version) for a manual actor, else rule_owner. CreateAutopilotTask
gains an originator_user_id param for the manual case.
- create_issue: dispatchCreateIssue enqueues a manual trigger via the actor-carrying
*WithHandoff entry points; attributionForIssueTask's autopilot-origin rule_owner
branch is now guarded on !actorUserID.Valid, so a valid actor falls through to the
direct_human override. Both execution modes attribute identically.
- schedule / webhook keep rule_owner (no actor). Trigger-table + system-pause/archive
versioning remain the pre-merge follow-ups.
Tests: the run_only row assertion Elon asked for (schedule → rule_owner row on
CreateAutopilotTask), plus manual direct_human on BOTH modes (run_only and
create_issue), including a manual actor distinct from the rule publisher. Full
service/attribution/handler/migration/scheduler/cmd suites pass on a DB migrated
through 160; build/vet/gofmt clean.
Co-authored-by: multica-agent <[email protected]>
* feat(attribution): owner_fallback + fail-closed policy, and manual-rerun direct_human (MUL-4302)
Two of the three remaining Phase 1 items (trigger-table / system-pause versioning
is deferred — see PR description).
owner_fallback + fail-closed (§1/§3.5) — the never-null accountable guarantee:
- attribution.OwnerFallback degrades an UNATTRIBUTED result to owner_fallback:
accountable = agent owner, originator stays NULL (audit-only, authz untouched),
Source.Precise()==false. finalizeAttribution's one-way invariant already allows
accountable-set / originator-NULL divergence, so nothing else changes.
- Migration 161 adds workspace.attribution_fail_closed (default FALSE) + a lean
GetWorkspaceAttributionFailClosed read. (Also added the column to ListWorkspaces'
explicit column list so its row type stays db.Workspace.)
- applyAttributionFallback is applied at every enqueue boundary (issue, mention,
chat, quick-create, deferred-fallback, autopilot run_only): unattributed →
owner_fallback (agent owner) by default, or ErrAttributionFailClosed when the
workspace is fail-closed, which the caller surfaces to refuse the enqueue (the
run does not start). So no run is left without an accountable human, and a
compliance workspace can block unattributable runs instead.
manual rerun (§5) — a rerun is a NEW direct_human trigger to the rerunning member:
- RerunIssue threads the acting member (resolved in the handler via resolveActor)
down to enqueueRerunTask, and attributionForIssueTask is now actor-first so the
actor wins over an INHERITED trigger comment (a rerun keeps the comment for the
daemon's prompt context but must attribute to whoever clicked rerun, not the
original comment's human).
- rerun_of_task_id lineage is recorded via a targeted SetAgentTaskRerunOf update on
the rerun path only (keeping the shared CreateAgentTask insert untouched), so
system retry (retry_of_task_id) and human rerun stay separable in reporting.
Tests: OwnerFallback unit test; owner_fallback + fail-closed-refusal + manual-rerun
(direct_human + rerun_of_task_id) service tests; the prior "degrades to unattributed"
test updated to owner_fallback. Full service/attribution/handler/migration/scheduler/
cmd suites pass on a DB migrated through 161; build/vet/gofmt clean.
Co-authored-by: multica-agent <[email protected]>
* fix(attribution): close fail-open holes + move rerun_of_task_id into creation snapshot (MUL-4302)
Addresses Elon's two must-fixes on PR #5150.
1. accountable-never-null / fail-closed had fail-open holes. applyAttributionFallback
now, for an UNATTRIBUTED run, refuses the enqueue (ErrAttributionFailClosed) in
THREE cases instead of silently degrading to a runnable NULL-accountable task:
- workspace policy read fails (or no workspace) → fail closed; we cannot confirm
fallback is permitted, so we don't run an unattributable task on a DB hiccup.
(Only the rare unattributed path pays this; precise runs never read the policy.)
- workspace is fail-closed → refuse (unchanged).
- owner_fallback has no valid agent owner → refuse rather than enqueue a task with
a NULL accountable_user_id.
ErrAttributionFailClosed's doc now covers all three "cannot guarantee an
accountable human" refusals. Added missing-owner / policy-read-failure /
precise-passthrough tests.
2. manual rerun rerun_of_task_id was a post-notify UPDATE (race: the queued event /
daemon claim could see rerun_of_task_id = NULL, and a failed update degraded the
run to a plain direct_human). It now rides the CreateAgentTask insert — threaded
through enqueueIssueTask / enqueueMentionTask as a creation param (like
retry_of_task_id) so it is written in the same statement before the daemon is
notified. Removed the SetAgentTaskRerunOf follow-up query.
Also merges origin/main (unrelated CLI fix #5167, no conflict). Full service /
attribution / handler / migration / scheduler / cmd suites pass on a DB migrated
through 161; build / vet / gofmt clean.
Co-authored-by: multica-agent <[email protected]>
* feat(attribution): rule_owner versioning on trigger edits + system-pause/archive (MUL-4302)
The final remaining Phase 1 item: substantive publishes beyond the autopilot row now
republish the rule version, so a run's rule_owner accountable follows whoever last
changed what the rule does.
- Extracted the config-summary + insert into service.RecordAutopilotRuleVersion so
the handler and the (different-package) failure monitor share one writer; the
handler's recordAutopilotRuleVersion is now a thin wrapper.
- Trigger edits: UpdateAutopilotTrigger and DeleteAutopilotTrigger republish the rule
version with the acting member as publisher, ATOMICALLY (tx-wrapped mutation +
version write, mirroring CreateAutopilot/UpdateAutopilot). CreateAutopilotTrigger
republishes best-effort — the webhook path mints its token with a retry loop that
cannot share one tx, and a create is usually initial setup already covered by v1;
a failed write there is benign (active version stays the current publisher, the new
trigger fires under it, no immediate daemon claim rides it).
- Archive (DeleteAutopilot) republishes (member, status=archived), tx-wrapped.
- System auto-pause (failure monitor) republishes with a 'system' publisher,
best-effort — a background sweep to a non-dispatching state (a paused autopilot
never dispatches; a later member resume supersedes).
- RotateWebhookToken / SetSigningSecret deliberately do NOT version: they rotate
credentials, not the rule's behavior (not §3.4 substantive).
Semantics: a system-published (no-member) active version degrades dispatch to
unattributed → owner_fallback, never fabricating a human.
Tests: republish-reattributes (member A → member B supersedes → dispatch resolves to
B; system publisher → unattributed). Full service/attribution/handler/migration/
scheduler/cmd suites pass on a DB migrated through 161; build/vet/gofmt clean.
Also merges origin/main (unrelated frontend feature #5074).
Co-authored-by: multica-agent <[email protected]>
* fix(attribution): make trigger-create rule-version republish atomic (MUL-4302)
Addresses Elon's final Phase 1 blocking finding: CreateAutopilotTrigger recorded the
rule-version republish best-effort AFTER the trigger insert. If member B added a
schedule/webhook trigger to member A's autopilot and the version write failed, future
schedule/webhook dispatches would keep attributing to A — violating the rule_owner
invariant that the last member to substantively change the rule owns future runs
("no immediate daemon claim" doesn't save it, since the miss surfaces at the LATER
trigger firing).
Both create paths now write the version in the SAME tx as the trigger INSERT:
- schedule create: wrap CreateAutopilotTrigger + recordAutopilotRuleVersion in one tx.
- webhook create: each mint-with-retry attempt runs in its own tx (insert + version
commit together; a token collision rolls that attempt back and retries with a fresh
token; a version-write failure rolls the trigger back). Passes ap + the acting
member id into the helper.
- removed the best-effort recordTriggerRuleVersionBestEffort helper (and the now-unused
slog import).
Test: TestCreateTrigger_RepublishesRuleVersionAtomically drives both create paths
through the handler and asserts a rule version is published by the acting member.
Existing webhook/trigger/archive handler tests still pass. Also merges origin/main
(unrelated avatar feature #5074). Full service/attribution/handler/migration/
scheduler/cmd suites pass on a DB migrated through 161; build/vet/gofmt clean.
Co-authored-by: multica-agent <[email protected]>
* feat(attribution): Phase 2.1 — surface run attribution on the task API (MUL-4302 §9)
First Phase 2 (visibility) increment: the agent-task API now returns the resolved
accountable-human provenance so the UI can render an "on behalf of" badge.
- AgentTaskResponse gains an `attribution` object: source label (never blank —
pre-migration NULL renders "unattributed") + `precise` flag (false for the degraded
owner_fallback / backfill / unattributed sources), the initiator (accountable) and
originator (authorization) user refs, the evidence {kind, ref_id} pointer, and the
rule_version / delegated / retry / rerun lineage ids.
- The label + evidence + raw ids are built in the PURE taskToResponse (no DB), so
every task response carries them. Names are hydrated separately, only on the
user-facing surfaces (ListAgentTasks, ListWorkspaceAgentTaskSnapshot, RerunIssue,
CancelTaskByUser) — daemon-claim paths stay lean.
- Hydration resolves initiator/originator from the GLOBAL user table (departed-member
safe) via a new batch GetUsersByIDs query (no N+1); best-effort, so a lookup hiccup
leaves the raw ids intact.
Tests: pure taskAttributionBase (direct_human / rule_owner NULL-originator /
owner_fallback degraded / pre-migration→unattributed) + DB hydration (fills known
ref, leaves unknown id un-filled, skips nil). Full handler/service/attribution/
migration/scheduler/cmd suites pass on a DB migrated through 161; build/vet/gofmt
clean. The field is additive — the frontend's parseWithFallback ignores unknown keys,
so nothing breaks until the UI increment consumes it.
Also merges origin/main (unrelated editor feature #5090).
Remaining Phase 2 (next increments, same PR): frontend zod schema + "on behalf of"
badge + evidence-chain jump; append-only correction events (write + display).
Co-authored-by: multica-agent <[email protected]>
* feat(attribution): Phase 2.2 — on-behalf-of badge in the execution log (MUL-4302 §9)
Surface the accountable human on every agent run row:
- AttributionBadge composes Badge + ActorAvatar, shows "on behalf of <member>"
with the resolution source as a tooltip; degraded (non-precise) attribution
gets a warning tone, and an unresolved initiator renders an explicit
"no responsible member" chip.
- Wire the badge into both active and past rows of the execution log.
- Mirror the attribution shape into AgentTaskResponseSchema (defensive, .loose())
so the cancel-task path carries it through zod; add parse tests.
- Export TaskAttribution/AttributionUser/TaskEvidence from @multica/core/types
and add the attribution block to all four issues.json locales.
Co-authored-by: multica-agent <[email protected]>
* fix(attribution): hydrate initiator names on issue-facing task endpoints + bound the badge (MUL-4302 §9)
Address Elon's PR #5150 review:
- ListTasksByIssue (the execution-log data source), GetActiveTaskForIssue and the
issue-scoped CancelTask now call hydrateTaskAttributions, so the "on behalf of
<member>" badge shows the real member name on issue detail instead of falling
back to "someone". Mirrors the existing ListAgentTasks / snapshot behavior.
- AttributionBadge: cap width (max-w-40, min-w-0) and truncate the name span so a
long name / narrow right column can't squeeze out trigger/status/actions; keep
the avatar shrink-0.
- Add a handler test asserting the issue task list returns a hydrated
attribution.initiator.name.
Co-authored-by: multica-agent <[email protected]>
* fix(attribution): use semantic AvatarSize 'xs' for the badge avatar
main refactored ActorAvatar.size from a raw pixel number to the semantic
AvatarSize union (packages/ui/lib/avatar-size). Switch the on-behalf-of badge
avatar from size={14} to size="xs" (16px) after merging main.
Co-authored-by: multica-agent <[email protected]>
* fix(attribution): stage-cascade falls back to parent-issue provenance, not agent owner (MUL-4302)
When closing the last sub-issue in a Stage wakes the parent's assignee agent, the
run was enqueued via a system-authored child-done comment with no actor, which the
resolver classified as unattributed and then degraded to owner_fallback (the agent's
own owner). That is the wrong accountable human: the woken run should be accountable
to whoever caused the parent issue to exist.
attributionForIssueTask now detects a system-authored trigger comment and falls
through to the parent issue's own provenance — the same creator / agent_create-origin
/ autopilot-origin chain a direct enqueue resolves (so an agent-decomposed parent
attributes via delegation to the human who drove it; a member-created parent to that
member; an autopilot parent to the rule publisher). owner_fallback is now only the
last resort when the parent provenance itself has no human.
- Extract attributionFromComment so attributionForIssueTask can inspect author_type
without a second GetComment; authorization resolution stays byte-identical.
- Add a DB-backed test asserting a system child-done comment resolves to the parent
issue's origin human (delegation), not owner_fallback.
Co-authored-by: multica-agent <[email protected]>
* feat(attribution): autopilot runs attribute to the firing trigger's creator (MUL-4302)
Per Bohan: an autopilot schedule/webhook run should be accountable to the human
who created the SPECIFIC trigger that fired it, not the rule publisher. (Manual
triggers already attribute to the invoking member via direct_human — unchanged.)
- Migration 162: add autopilot_trigger.created_by_type/created_by_id (nullable, no
FK/cascade). Capture the creating member at both trigger-create sites (schedule +
webhook).
- New precise source trigger_owner: originator stays NULL (an autonomous fire
carries no human authorization — same authz-safe divergence as rule_owner),
accountable = the trigger's member creator.
- triggerOwnerAttribution resolves run.trigger_id → creator; wired into run_only
dispatch and the create_issue path (bridging issue → active run → trigger_id).
Legacy triggers with no recorded creator, and agent-created triggers, degrade to
rule_owner then owner_fallback — nothing regresses.
- Frontend: trigger_owner source label in all four locales + badge switch case.
- Tests: attribution TriggerOwner unit + Precise/invariant; DB-backed resolver
tests (member creator → trigger_owner; creatorless → rule_owner fallback).
Co-authored-by: multica-agent <[email protected]>
* chore(attribution): re-trigger CI (dropped synchronize event on
|
||
|
|
6caa3397ea |
fix(comment): sanitize NUL/invalid bytes in comment content and trigger preview (#5391)
Agent-written comment content can carry a byte PostgreSQL's TEXT column rejects -- reachably an embedded NUL (0x00, SQLSTATE 22021) that survives a JSON round trip. CreateComment/UpdateComment inserted content verbatim, so such a byte failed the INSERT with an opaque 500 the CLI renders as "server unavailable" and retries. This is the plausible cause of #5388 (a large --content-file body erroring while shell-re-encoded --content-stdin succeeded); the reporter's exact bytes are unconfirmed. - Apply the existing sanitizeNullBytes() helper (already used on skill import for this PG failure mode) to comment content before the empty check, in CreateComment and UpdateComment. - Apply the SAME normalization in PreviewCommentTriggers so the trigger preview and the real create/update side effect agree; otherwise a mention hidden behind a NUL reads as inert in preview but enqueues the agent on submit. - Tests: NUL content posts (201, byte stripped) instead of 500; the preview target set matches the enqueue set for a NUL-hidden mention on both the create and edit paths. Refs #5388 Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
9d453fac1e |
fix(comments): clarify 409 for top-level comment from a comment-triggered task (MUL-4417) (#5292)
* fix(comments): clarify 409 when a comment-triggered task posts a top-level comment (MUL-4417) A comment-triggered task that posted a parentless top-level comment on its own issue got a 409 whose message named the required parent id but never said top-level comments are disallowed. Agents misread it as the issue being locked and deleted good replies trying to reset. Keep the guard (agents must reply under their trigger comment), but make the error self-explanatory and document the constraint in the CLI --parent help. Add handler-level tests pinning the rejected top-level case and the allowed reply-under-trigger case. Refs GH #5266. Co-authored-by: multica-agent <[email protected]> * fix(comments): tighten 409 wording and assert the fix hint (MUL-4417) Review nits on #5292: drop the inaccurate "while it is active" phrasing and the redundancy from the 409 message so it matches the actual allow-set (trigger or coalesced comment); collapse the incident narration to one line; and assert the actionable parent_id (--parent) hint in the regression test so the guidance can't be dropped silently. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J (Multica agent) <[email protected]> Co-authored-by: multica-agent <[email protected]> |