40 Commits

Include renames

Author	SHA1	Message	Date
3 people	e720fe2219	MUL-8135: feat(wakeups): wake on linked PR merge conflicts (--until-pr conflicts) (#9406) ... * MUL-8135: feat(wakeups): wake on linked PR merge conflicts (--until-pr conflicts) Co-authored-by: Cursor <[email protected]> Co-authored-by: multica-agent <[email protected]> * MUL-8135: fix(wakeups): conflict wakes per PR identity, rotate the watch sweep - A linked-PR condition only fires for a PR key the rule has not seen; a PR leaving the satisfying set no longer re-wakes the agent for the others. - Conflict keys use the PR id, so same-numbered fork PRs on one head stay distinct. - The conflict-watch sweep rotates an address cursor so failing fetches cannot starve the rest. Co-authored-by: Cursor <[email protected]> Co-authored-by: multica-agent <[email protected]> * MUL-8135: fix(wakeups): count repeated linked-PR keys when deduping Checks and merged keys carry only the PR number, so same-numbered PRs from two repositories share a key. Compare occurrence counts so the second one satisfying still wakes the agent. Co-authored-by: Cursor <[email protected]> Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Cursor <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-10-22 01:23:23 +08:00
3 people	fc4a615700	MUL-7726: let the workspace choose the status merged PRs move an issue to (#8862) ... * feat(pr): move issues to a workspace-chosen status when their PRs merge (MUL-7726) The merge automation completed an issue only when a PR said "Closes MUL-1", so the workspace switch mostly did nothing: most PRs are linked by title or branch and never completed anything. The workspace now picks what a merge does, in settings.pr_merge_status: "none", or the key of a started or done status (custom ones included, Blocked excluded). Absent means Done. When every linked PR is merged, the issue moves to that status. A closing keyword only links. - Decision: terminal, triage, "none", the per-issue switch and a new at_target state (the issue already has the target) come before the PR states, so the issue page only speaks when a merge would move the issue. The response adds target_status. - MoveIssueFromPullRequests replaces CompleteIssueFromPullRequests and writes the target status. The target is resolved through the delivery's shared catalog read (Resolver.WritableCategory). An archived or unknown choice, or a failed read, fails closed to no write. - Close intent is no longer synced or read. The column stays. - Migration 551 pins "none" on existing workspaces whose history shows a merge never moved every issue: auto-complete switched off, or a linked PR whose merges were not all keyword merges. Workspaces with no links, or with only keyword merges, keep the Done default. Replays are harmless. - CLI: `multica issue pr-automation <id> on|off` lets an agent keep one issue where it is. The multica-platform skill describes the new rule. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * feat(views): choose the status merged PRs move an issue to on the GitHub page (MUL-7726) - Settings → Integrations → GitHub → Features: "After PRs merge, move the issue to" picks Don't change, or a started or done status, custom ones included and Blocked excluded. It replaces the read-only auto-complete row. The self-hosted Git page shows the same setting. The statuses page drops its switch, and a badge on the target status now links to the setting. - Issue sidebar: the line under the PR list names the target status ("Moves to In Review when #19 merges"). It says nothing when the workspace leaves status alone, when the issue already has the target, or on an older backend's no_close_intent. The per-issue switch reads "Keep status when PRs merge" and is hidden when it would change nothing. The menu opens the GitHub setting. The link hint only shows while auto-link is on. - core: derivePRMergeStatus replaces derivePRAutoCompleteEnabled; the auto_complete schema adds target_status and the at_target state. - Copy in 5 languages. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * docs: describe the workspace-chosen status for merged PRs (MUL-7726) GitHub integration, self-hosted Git, issues and environment variable pages (4-5 languages) now say that the workspace picks what a merge does: Done by default, another started or done status, or no change. They also say that a closing keyword only links, and how to keep one issue's status. The troubleshooting entries explain what a missing line under the PR list means. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * refactor(pr): keep the merge automation out of agents' instructions (MUL-7726) Agents don't act on what a merge does to an issue, so they don't need a command for it or a rule to learn. Drop `multica issue pr-automation`, and cut the multica-platform skill down to how PRs get linked: no merge rule, no auto_complete field guide, no "the server moves it to done" note. People still keep one issue's status from its Pull requests menu, and the docs now point there. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(pr): keep the retired auto-complete switch working for old desktop clients (MUL-7726) Review of #8862: an admin on a desktop client from before this change can still flip "Complete issues when their PRs merge". The client saved pr_auto_complete_enabled = false, and the save looked successful, but the server only read pr_merge_status, so merges kept moving issues to Done. - Settings writes: a flip of the retired switch becomes a choice (off → none, on → Done). An old client that echoes a chosen target without flipping the switch leaves the target alone. Every write mirrors the switch from the effective choice (false for none, absent otherwise), so old clients show the right state. - Reads: without pr_merge_status, the retired switch decides (off → none, else Done), on the server and in derivePRMergeStatus. This also covers settings written by a pod that predates the change during rollout. - Migration 551 sets the switch off on the workspaces it pins to none. 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]>	2026-09-27 02:26:24 +08:00
Bohan Jiangandmultica-agent	0da0b46bbf	MUL-7672: only a closing keyword completes an issue on PR merge (#8794) ... * feat(pr): only a closing keyword completes an issue on merge (MUL-7672) #8758 made every linked PR count toward auto-complete, so a title-only "MUL-123: ..." PR moved its issue to Done on merge. Restore the earlier contract: a PR completes its issue only when its title or body puts a closing keyword (Closes/Fixes/Resolves) right before the identifier. - Body closing keywords link again; a bare body mention still links nothing. - close_intent is recorded on the link and follows the PR text until the merge/close event, then freezes; a link first made after merge carries none. - The decision requires every linked PR merged and at least one with close intent; a new no_close_intent state explains why a merge won't complete. - Everything else from #8758 stays: manual link/remove, per-issue and workspace switches, edge-triggered evaluation, the result line. - Docs (4 languages), UI copy (5 languages) and the multica-platform skill describe the keyword rule again. Co-authored-by: multica-agent <[email protected]> * fix(pr): sync close intent on every link of the PR (MUL-7672) Review of #8794 found two ways a withdrawn closing keyword still completed the issue: - close_intent was refreshed only for identifiers the PR still claimed, so a manual link, or an automatic link whose merge event (carrying the final text) arrived before the edit event, kept a stale true. - the refresh sat behind the auto-link toggle, so turning auto-link off froze whatever intent was recorded. Replace the per-link update with one PR-wide sync: until the merge/close event, every link of the PR (automatic or manual) gets close_intent exactly when the current title/body closes its issue. It runs whenever GitHub features are on, independent of auto-link, which now only decides which links are created. Co-authored-by: multica-agent <[email protected]> * fix(pr): decide close intent apart from auto-link ownership (MUL-7672) Re-review of #8794: the PR-wide close intent sync reused the auto-link verdict, and resolvePRLinkPolicy skips workspaces with auto-link off. With an installation bound to several workspaces, turning auto-link off in one of them therefore cleared a closing keyword only that workspace resolves, and the merge no longer completed the issue. The policy now reads every bound workspace with GitHub on, and records the identifier's sole resolver next to the auto-linking owner. Links still follow the owner (auto-link off workspaces are not competing claimants); close intent is permitted for the owner or the sole resolver, so an identifier that resolves in more than one workspace stays withheld. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-agent <[email protected]>	2026-09-24 14:56:59 +08:00
3 people	9df0cae78a	MUL-7429: complete issues when every linked PR merges (#8758) ... * feat(pr): complete issues when every linked PR merges (MUL-7429) Title/branch identifiers link a PR; keywords and the body no longer matter. Completion is evaluated only on a PR event for the issue (merge, link, unlink), so settings changes and reopening never complete an issue. Adds manual link/unlink with remembered exclusions and a per-issue auto-complete switch. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * feat(pr): issue-page PR automation and single auto-complete setting (MUL-7429) The issue's Pull requests section says what the merge rule will do next, links a PR by URL, removes one per row, and turns auto-complete off for that issue. The workspace switch lives under Issue statuses; the GitHub page reports it. Agent skill and docs describe the title/branch rule. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(pr): accept PR links pasted without a scheme (MUL-7429) Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * test(skills): pin the PR auto-complete contract in the platform skill (MUL-7429) Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(pr): re-check linked PRs at the completion write; skip stale GitHub events (MUL-7429) The status write now requires every linked PR to still be merged when it runs, so a PR linked between the decision and the write keeps the issue open. GitHub PR upserts ignore events older than the stored row, like the self-hosted path, so a late delivery cannot roll a merged PR back to open and turn a redelivered merge into a new one. 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]>	2026-09-24 02:58:19 +08:00
3 people	4469bac632	fix(issues): duplicate mark follow-ups from #8741 review (MUL-7349) (#8752) ... * perf(issues): resolve a page's duplicate originals in one read (MUL-7349) Filling duplicate_of read each distinct original with its own primary-key lookup, so a page of duplicates of different issues cost one round trip per row. Pages now pass the originals their rows point at to newStatusCategoryFiller, which loads them with one ListIssueRefsInWorkspace read, the same way labelsByIssue loads a page's labels. List, open issues, grouped, search and table rows do this; a response built on its own still resolves its original, so a caller that passes nothing is slower, never wrong. A pointer left on a reopened issue is not looked up at all, and an original that no longer exists is looked up once and left off. Co-authored-by: multica-agent <[email protected]> * fix(issues): stop logging stale duplicate pointers as unmarks (MUL-7349) Servers from before the duplicate column reopened duplicates and deleted originals without clearing the pointer. After an upgrade, the next write to such an issue clears it, and the activity log compared raw pointers, so it logged "removed duplicate mark" for a mark that no longer counted and dropped the real status row in its place. Event publishers now send only live marks: a pointer counts while the issue is cancelled, the same rule reads apply. Separately, the status row is dropped only when a duplicate row was actually written to stand in for it; when the original is gone there is no duplicate row, and the reopen used to leave no activity at all. Co-authored-by: multica-agent <[email protected]> * fix(issues): keep duplicate marks in background issue snapshots (MUL-7349) IssueToMap hardcoded duplicate_of to null on the assumption that no background publisher renders a marked issue. PublishAttachmentsChanged does: a channel /issue with media re-reads the issue after a download of up to 45 seconds, and a mark made in that window was broadcast as null. Clients patch their cache with that snapshot, so the list and board marker disappeared until the next refetch. IssueToMapResolved now resolves duplicate_of the way the HTTP rendering does; only a cancelled issue with a pointer costs a read. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-09-23 17:41:27 +08:00
Bohan Jiang	1539c54ec5	feat(issues): make duplicate marks traceable and one click from the original (MUL-7349) (#8741) ... * feat(issues): make duplicate marks traceable and one click from the original (MUL-7349) - Log duplicate_marked / duplicate_unmarked on the duplicate and duplicate_added / duplicate_removed on its original, with the other issue linked in the activity feed; the delete and GitHub merge paths now carry both ends of the mark so those rows are written too. - Expose duplicate_of_issue_id on issue responses (only while cancelled) so list rows, board cards and table cells can show an arrow to the original that opens it directly. - Banner links as a whole to the original and shows its status; the undo action becomes the secondary "Not a duplicate" with a toast. - Sidebar duplicates show reporters and a count; the status picker reads "Change original" on an issue that already is a duplicate; the mark picker opens with same-title and recently viewed candidates. * fix(issues): keep the duplicate pointer in step with CI shape tests (MUL-7349) - Add duplicate_of_issue_id to the CLI --fields whitelist, which the list-endpoint shape test checks against a real response. - Project the column in the search parity test's legacy query copy and scan it, so both queries read through the one scanner. - Mount the row marker's query and navigation hooks only when the issue is a duplicate, so surfaces rendered without a QueryClient (the table's inline title) and thousands of ordinary rows subscribe to nothing. * fix(issues): resolve a duplicate's original on the server and log one row per mark change (MUL-7349) - Issue responses carry duplicate_of {id, identifier, title, status}, filled beside the status category only while the issue is cancelled and the original still exists; the bare pointer is no longer exposed. Rows read it directly, so the marker makes no request and is a real link where the row is not one. - A mark change suppresses the generic status_changed row; the unmarked row records where the status went. Covered through the real UpdateIssue path, with the test server wiring activity listeners like main.go. - The picker applies one selectability predicate to search results and suggestions, so an existing duplicate can no longer be chosen.	2026-09-23 16:55:50 +08:00
Bohan Jiangandmultica-agent	bd70632999	fix(wakeup): end closed-issue wakeups in the application, not a trigger (MUL-7557) (#8642) ... Migration 518/520 (#8467) registered stop_issue_wakeups, an AFTER UPDATE OF status trigger on issue that disabled issue_wakeup rows and cancelled queued/deferred wakeup runs in agent_task_queue. That is a database-level cascading update, which the repository's migration rules forbid. Drop the trigger and function (migration 533) and apply the same close rule in the transaction that writes the status: single and batch issue updates and the merged-PR auto-close. Cancelled runs are now broadcast after commit. Co-authored-by: multica-agent <[email protected]>	2026-09-21 18:04:26 +08:00
3 people	f0c9146c4b	MUL-7437: fix(api): classify disabled capabilities as non-retryable (#8487) ... * fix(plugin): return 403 when API is disabled (MUL-7437) Co-authored-by: multica-agent <[email protected]> * fix(api): classify disabled capabilities as non-retryable (MUL-7437) Co-authored-by: multica-agent <[email protected]> * fix(api): address disabled capability review feedback (MUL-7437) Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Sol-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-09-16 17:27:05 +08:00
3 people	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]>	2026-09-15 15:34:21 +08:00
3 people	8908fcfbc4	feat(server): reserve the triage status key (MUL-7212) (#8298) ... Make `triage` a platform-reserved issue status: it has no catalog row, is not a category a custom status can declare, and no ordinary write can set it or move an issue out of it. - issuestatus: Triage constant; IsBuiltIn covers it, so ValidateKey, DeriveKey and firstFreeKey refuse or skip the key; Resolve refuses it with ErrReservedStatus; Effective and Category resolve it without a catalog read - migrations: a barrier file adds CHECK (key <> 'triage') NOT VALID on its own, so it commits before the repair file renames a pre-existing custom `triage` key per workspace (firstFreeKey rule) across the catalog, issues and saved views and VALIDATEs; issue_effective_status() gains 'triage' on its fast path - writes: status_reserved_for_triage on create/update/batch; issue_in_triage when status, project_id or parent_issue_id is written on an issue in Triage; merged PRs no longer auto-advance a Triage issue - status sort ranks triage first; duplicate guard ignores triage issues - notification label, CLI help, TriageV1 feature flag key - cmd/migrate forwards notices marked 'migration report: ' to its log Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-09-14 15:20:19 +08:00
Bohan Jiang	116761e505	MUL-7072 fix(github): retire reference-only PR links (#8245) ... A bare body mention of an issue key claimed nothing, yet auto-link wrote a link row for it and flagged it reference_only — filtered out of the PR list, the close aggregate and the review dedup head SHA. The row had no reader, and once the PR went terminal the preserve gate froze the flag, so adding `Closes MUL-1` to a merged PR's body could never surface the PR on its issue. With no unlink surface anywhere, the one repair available from GitHub was the one that could not work. Write no row for a passing mention, delete the historical hidden rows (migration 458), and drop the column's reads and writes; the column itself goes in a follow-up release that re-runs the delete as a mop-up. The visible set of links is unchanged, except that a post-merge `Closes` now links the PR (without retroactively completing the issue). An open PR whose claim is downgraded in place loses its link, which is what hiding used to accomplish. Docs, the four locales, and the agent-facing platform skill are updated in step.	2026-09-10 17:16:50 +08:00
importcpp	f5f0b604f6	perf(webhooks): reuse workspace status catalogs for PR close checks (#8191) ... Resolve custom issue statuses once per workspace mirror pass using the existing request-scoped Resolver. Preserve built-in fast paths and existing close-intent decisions, with provider and query-count regressions.	2026-09-09 14:36:42 +08:00
3 people	18a85b8c7f	fix(github): support single-character issue prefixes (#7385) ... Co-authored-by: Rogee <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-08-21 15:36:33 +08:00
3 people	14c2e4e831	MUL-6243 feat(issue-status): board fetches by category; archive retires without migration (#7084) ... * feat(issue-status): board fetches by category; archive retires without migration (MUL-6243) Product decisions from the issue thread: 1. Board fetches by CATEGORY, not per status. PAGINATED_STATUSES becomes PAGINATED_CATEGORIES and the cache buckets by category, so the board's fan-out stays fixed at 7 parallel requests however many custom statuses a workspace defines — instead of one more request per status added. Adds status_category / status_categories filters to the issue list endpoints, backed by the existing issue_effective_status() SQL function. 3. Archiving retires a status from FUTURE use only. It no longer refuses while issues still carry it: those issues keep their status and keep resolving to their category's behavior (Effective ignores archived_at), while Resolve still rejects the status so nothing new can be assigned to it. The exclusive catalog lock stays — it is what makes "no NEW assignment after archive" exact rather than approximate. Also lands the frontend catalog foundation: packages/core/issue-statuses with useIssueStatuses(wsId) (React Query, keyed by wsId per the state rules) and a pure buildIssueStatusCatalog for non-React callers, plus IssueStatusCategory as a distinct type from the status key and status_category on issue payloads. NOT included, and deliberately: widening IssueStatus to string. That change forces every Record<IssueStatus, …> presentation lookup in packages/views to resolve a key to its category first, and it is the first task of the follow-up PR. Everything here is the groundwork that makes it mechanical. Co-authored-by: multica-agent <[email protected]> * fix(issue-status): restore the status index, make category authoritative, keep archived resolvable Review round 1 on #7084 (MUL-6243): 1. Category filtering wrapped issue.status in issue_effective_status(), which makes the (workspace_id, status) index unusable — and the board switched to it unconditionally, so every workspace paid for it immediately. Categories are now expanded to concrete status keys (issuestatus.ExpandCategories) and filtered with an indexed status = ANY(...). Measured on a 100k-row table with the production index: seq scan cost 27013 -> index-only scan cost 6.07. A workspace with no custom statuses expands each category to exactly its own key, reproducing the pre-feature query. Archived keys are included, since issues left on them still belong in their category's column. 2. status_category was only filled for built-ins, so a custom-status issue arriving over the websocket was silently dropped by addIssueToBuckets and a cross-category update was skipped by the cache coordinator, drifting off-window totals permanently. List, grouped, get, create, update and batch responses now carry an authoritative category via a per-request Resolver (archived included), and the client invalidates instead of no-oping when a category is unresolvable. 3. The catalog was fetched without archived entries, so after archiving a status the issues still on it lost their name and colour and had their category guessed as todo — contradicting the decision that archiving leaves existing issues untouched. Resolution now includes archived entries; activeStatuses / inCategory expose only assignable ones to pickers. Co-authored-by: multica-agent <[email protected]> * fix(issue-status): category on every payload exit, one resolver per request, no same-category duplication Review round 2 on #7084 (MUL-6243): 1. Category fill was incomplete and N+1. The create WEBSOCKET payload was unfilled — only the creator's HTTP response was, which is too late for the other tabs the event exists to serve. GetIssue, the open_only list, search, the duplicate-conflict response, the table rows, the GitHub PR update and the onboarding shim were also unfilled. Grouped and batch called the single-response helper per row, so each custom-status row hit the catalog separately. Every exit now goes through newStatusCategoryFiller, a request-scoped closure over ONE Resolver: built-in statuses still cost no query, and a page of custom-status rows costs one catalog read rather than one per row. 2. A status KEY change within one category duplicated the card and inflated the total. patchIssueInBuckets compared patch.status (a key) against loc.status (a bucket CATEGORY), so in_review -> a custom human_review took the cross-bucket path, then deleted from and re-inserted into the same bucket from a pre-delete snapshot. It now resolves the destination category first and compares category to category; same-category changes replace in place and leave the total alone. The destination category is resolved from the patch alone — the merged issue inherits the previous status_category, which would have kept a genuinely moved card in its old column. patchNeedsInvalidation lets the loaded branch of the cache coordinator invalidate instead of silently no-oping on a status it cannot resolve. Also drops the unrelated share_link.sql.go churn from an sqlc version difference, per the review nit. The four cache cases were verified to fail against the previous logic (card duplicated, total +1) before the fix was restored. Co-authored-by: multica-agent <[email protected]> * fix(issue-status): normalize status_category on every cache write; one filler per create Review round 3 on #7084 (MUL-6243): Must-fix — a cached entity kept the STALE status_category after a status change. The server now sends a category on every issue, so a cached entity is {status: "todo", status_category: "todo"}; an optimistic patch carries only {status: "done"}, and a bare {...issue, ...patch} moved the card to the done bucket while the entity still claimed todo. A single update self-heals from the server response, but the batch API returns only {updated: n} and does not refetch bucketed lists, so the inconsistency was permanent — and the next off-window count then decremented the wrong bucket, recreating total drift. normalizeStatusPatch now rewrites status_category to match status before any cache write, applied at applyIssueChange's entry (covering the flat, table and detail spreads), in patchIssueInBuckets, and in the children mutation. An unresolvable custom key drops the inherited category rather than keeping it; patchNeedsInvalidation still routes it to a refetch. Test fixtures now carry status_category for built-ins, matching what the server actually sends — without that the regression was invisible. Non-blocking items also addressed: - CreateIssue builds one filler and shares it between the broadcast payload and the HTTP response, so a custom-status create reads the catalog once. - Background events via IssueToMap now resolve an authoritative category (IssueToMapWithCategory) for the task, issue and autopilot publishers. The channel router keeps plain IssueToMap and says why: it always creates with the built-in "todo", where key IS category. The "one resolver serves a whole page" assertion was REMOVED rather than kept: pg_stat counters lag, so a delta around one request passes even with a per-row filler. Verified that directly before dropping it. The comment now states what is and is not pinned instead of implying coverage that does not exist. Co-authored-by: multica-agent <[email protected]> * fix(issue-status): address review nits - Normalize the single-update children optimistic patch too, matching the batch path. It self-healed on onSettled, but it broke the entity/bucket invariant for the duration of the request. - Rename TestListEndpointsCarryStatusCategory: the old name claimed it covered "without per row queries", which the test explicitly does not assert. - Drop the unrelated gofmt churn in empty_claim_cache.go. - Add a background-event assertion: IssueToMap leaves a custom status uncategorized, IssueToMapWithCategory resolves it — the reason the publishers use the latter. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-08-18 03:00:21 +08:00
3 people	446080bd57	MUL-6243 feat(issue-status): per-workspace custom issue statuses over 7 canonical categories (#7065) ... * feat(issue-status): per-workspace custom issue statuses over 7 canonical categories (MUL-6243) Co-authored-by: multica-agent <[email protected]> * fix(issue-status): address review — normalize all status consumers, close archive race, gate rollout, split PK index Review round 1 (MUL-6243 / PR #7065): 1. Behavior normalization was incomplete. Adds issue_effective_status() as a SQL mirror of issuestatus.Effective so the six SQL consumers the Go resolver cannot reach (duplicate guard, open-issue list, sub-issue progress, project stats, completed-inbox archive, open_only) resolve categories too. Wires the resolver into the task-failure sweeper and the stage-barrier helpers. 2. Archive TOCTOU. Archive now runs its in-use census and the archive itself in one transaction under an exclusive catalog lock; writes that target a custom status take the shared side. Built-in writes stay lock-free — a built-in can never be archived. 3. Rollout safety. Creating a custom status is gated behind the default-off CustomIssueStatuses flag, so a value old pods cannot interpret cannot exist until the fleet is updated. Readers ship ungated and are safe. 4. Primary key index. Follows the repo convention (327-329): table without a PK, unique index built CONCURRENTLY in its own migration, attached via PRIMARY KEY USING INDEX, and registered for invalid-index cleanup. 5. Status normalization vs stored value. Write paths now persist the canonical key from Resolve, so 'HUMAN_REVIEW' no longer validates and then fails the column format constraint as a 500. Also fixes the two CLI tests failing in CI: they asserted the pre-MUL-6243 contract that an unknown-but-well-formed status is rejected locally. Co-authored-by: multica-agent <[email protected]> * fix(issue-status): close the archive race in both orderings; resolve categories in batch barrier and CLI progress Review round 2 (MUL-6243 / PR #7065): 1. The archive race was only half closed. Handlers resolved the status BEFORE opening a transaction, so an archive could commit in that window and the write would still land on an archived key. assertIssueStatusStillActive now takes the shared catalog lock and RE-RESOLVES inside the caller's transaction, which is what actually makes the status provably active at write time. Applied to create, update, batch, and both description-merge branches, which previously bypassed the guard entirely. A losing writer gets 409 rather than a silent partial update. 2. The batch stage-barrier prefilter still compared literals, so a batch that moved the last child onto a custom done status left childDoneCompleted empty and skipped the parent notification. It now resolves categories first. 3. multica issue children --output json counted only literal done/cancelled. ListChildIssues now emits status_category and the CLI reads it, falling back to status for older backends. TestArchiveCommitsInsideTheWriteRaceWindow interleaves the two operations so the writer validates against an active status and the archive commits before the write. Verified it fails without the re-resolve (issue stranded on the archived status) and passes with it. Co-authored-by: multica-agent <[email protected]> * refactor(issue-status): address review nits — honest race-test naming, one catalog read per list Review round 3 nits (MUL-6243 / PR #7065): 1. TestArchiveRaceBothOrderings claimed more than it proved. Its 'archive wins' cases archived first and only then sent the request, which the pre-flight check alone rejects. Split into TestWritesRejectAnArchivedStatus (the sequential case, named for what it is) and TestArchiveRefusesAfterACommitted Write, and extended TestArchiveCommitsInsideTheWriteRaceWindow — the only test that truly interleaves archive between pre-flight and write — from create alone to all five write endpoints. Re-verified by deleting the in-transaction re-resolve: 4 of 5 fail with issues stranded on the archived status (create is guarded in the service layer and checked separately). 2. ListChildIssues / ListChildrenByParents issued one category lookup per custom-status child. issuestatus.Resolver now amortizes that to a single catalog read per request, lazily on the first custom key, so built-in statuses still cost zero queries however long the list is. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-08-17 18:32:02 +08:00
3 people	b4692a192d	MUL-6048 fix(github): withhold close intent when a PR identifier is ambiguous across workspaces (#6808) ... * fix(github): withhold close intent when a PR identifier is ambiguous across workspaces Issue prefixes are deliberately not globally unique (#2797) and issue numbers restart at 1 in every workspace, so once #5183 fanned PR webhooks out to every workspace bound to an installation, "Closes ABC-100" could resolve to a genuine — but different — issue in each of them. Each workspace then acted on it independently, silently advancing an issue in a workspace that had nothing to do with the PR (#6804). Resolve the collision before any workspace links: count how many bound workspaces resolve each closing identifier, and where more than one does, write the link rows but withhold close_intent, which keeps the auto-advance gate shut. Links stay visible so a human can see and unlink the wrong one; only the irreversible half is suppressed. The workspace that legitimately owns the PR also loses auto-complete for that identifier — deliberate, since nothing in the event says which one that is. A warn log names the colliding identifier, because an issue moving to done for no visible reason is otherwise untraceable back to GitHub. The pre-pass only runs for closing identifiers on installations with more than one binding, so the single-workspace case does no extra work. Co-authored-by: multica-agent <[email protected]> * fix(github): make the close-intent verdict authoritative and fail closed Review found the ambiguity scan could fail open, and that its verdict was not the same state the write path acted on. Fail open: the scan reused helpers that fold read failures into permissive defaults — workspaceAutoLinkPRsEnabled returns true when the settings blob cannot be read or parsed, getIssuePrefix returns "" on a failed load, and lookupIssueByIdentifier reports any query error as "no such issue". One transient failure could count two real resolvers as one, and both workspaces would then advance to done on merge — the exact outcome the change exists to prevent. Not authoritative: the scan produced a denylist, then mirrorPullRequest- ForWorkspace independently re-read the toggle, the prefix and the issue and decided close intent from those reads. Anything changing in between (an admin enabling auto-link, a prefix edit, a same-numbered issue created) reopened the hole. Replace the denylist with closeIntentPolicy, an allowlist recording which single workspace was proven to own each closing identifier. Absence denies, so the zero value returned by every indeterminate read permits nothing, and the mirror pass can only narrow the verdict, never grant what the scan withheld. Recording the owner rather than a bare "allowed" also closes the scan-to-write window: a workspace that grows a same-numbered issue in between is not the recorded owner and still cannot act. Surface the reads the scan depends on instead of swallowing them: autoLinkPRsEnabledForWorkspace now returns a parse error, and the scan distinguishes pgx.ErrNoRows (a proven miss) from an error it cannot interpret. Existing callers keep the permissive default. Extract issuePrefixForWorkspace and issueNumberForPrefix so the scan derives both from the row it already holds, which also drops a redundant workspace read per binding. Tests: a table test pinning that the policy denies by default, an integration test that an unreadable bound workspace withholds close intent (fails with the old fail-open behavior), and one that a proven unique resolver among several bindings still auto-completes, so the fix cannot over-suppress. Co-authored-by: multica-agent <[email protected]> * docs(handler): split the merged getIssuePrefix / issuePrefixForWorkspace doc comments Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-08-12 15:24:59 +08:00
3 people	aa110aac9c	MUL-5088: import repositories from GitHub App (#5896) ... * feat(github): import repositories from app installations Co-authored-by: multica-agent <[email protected]> * fix(github): address repository import review nits Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Eve <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-07-28 16:19:35 +08:00
3 people	ecce589867	MUL-5265: GitHub API-snapshot PR cards — CI status + mergeability (#5889) ... * feat(github): API-snapshot PR cards — CI status + mergeability (MUL-5265) Fetch each linked PR's CI checks and mergeability from the GitHub GraphQL API as the single source of truth (Plan C). Webhooks, page visits and a bounded TTL sweep are refresh triggers only; nothing is inferred from webhook payloads anymore. Backend (server/internal/integrations/ghsnapshot): - installation-token cache + GraphQL client (private key / tokens never logged) - one paginated pullRequest query -> normalized per-check snapshot - outbound queue: (installation,repo,PR) dedup + single in-flight per PR, bounded worker pool, Retry-After / rate-limit backoff, jitter - head-SHA-guarded atomic batch replace (a slow response for an old head can never overwrite a newer head's snapshot) - bounded chase window (30s->5m, stops on terminal/closed) + page-visit + TTL refresh; clean degradation when no App private key is configured Removes the old suite-level webhook aggregation display path (query + handlers + tests). check_suite / check_run / status are now pure triggers. Frontend: PR card shows two independent tri-state elements (CI status + mergeability). "Ready to merge" only when merge state is clean; no-checks and unknown-mergeable never assert a positive verdict; progress strip removed; four locales; stale marker. Docs: github-integration + environment-variables (four languages) — now required App private key, read-only Checks/Commit-statuses permissions, new event subscriptions, capability boundaries and troubleshooting. Co-authored-by: multica-agent <[email protected]> * fix(github): address PR snapshot review blockers Co-authored-by: multica-agent <[email protected]> * fix(github): bound snapshot refresh scheduling Co-authored-by: multica-agent <[email protected]> * fix(github): concurrent check-run index migration + singleflight token mint Address Elon's third-round review on the MUL-5265 PR snapshot pipeline. Must-fix — migration built a non-concurrent index. The github_pull_request_check_run table declared PRIMARY KEY (pr_id, ordinal) inside CREATE TABLE, which builds a unique index synchronously and violates the repo rule that every migration-created index (including on a new table) use CREATE UNIQUE INDEX CONCURRENTLY in its own single-statement file. Split: 222 now creates the table without a primary key; new 223 adds the (pr_id, ordinal) unique index CONCURRENTLY. The atomic delete-all/insert write path already guarantees ordinal uniqueness, so a plain unique index is sufficient; the index also serves the pr_id-prefix list aggregation and the workspace/PR cleanup deletes. Nit — token mint now singleflights per installation. installationToken released the lock before minting, so the N workers of one installation could mint N tokens on a cold cache or a simultaneous renew. Concurrent callers for the same installation are now collapsed via singleflight into one HTTP mint; added a -race concurrent-mint test asserting a single mint under 16 callers. Verified: fresh DB migrates through 223 (table has no PK, concurrent unique index present); ghsnapshot suite + new test pass under -race; migration lint and handler github/workspace-delete tests pass; sqlc produced no diff; go build / vet / gofmt / git diff --check clean. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-07-24 18:30:20 +08:00
dixonl90andBohan	581d9527ba	feat(vcs): self-hosted Git providers (Forgejo, Gitea, GitLab) alongside GitHub (MUL-3772) (#5006) ... Adds self-hosted Git provider support (Forgejo, Gitea, GitLab) alongside GitHub: per-workspace token connection, a provider-dispatched webhook, PR/MR and CI mirroring, and the shared issue auto-link / auto-close machinery. Off until MULTICA_VCS_SECRET_KEY is set, so existing deployments are unaffected. Co-authored-by: Bohan <[email protected]>	2026-07-24 15:01:27 +08:00
3 people	8d18d3a9ec	Revert "MUL-5180: fix(github): surface CI status on PR cards (#5811)" (#5855) ... This reverts commit 139cc89200. Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-07-24 02:52:22 +08:00
3 people	139cc89200	MUL-5180: fix(github): surface CI status on PR cards (#5811) ... * fix(github): surface CI status on PR cards (MUL-5180) The CI mirroring pipeline (MUL-2228, MUL-2392) has never received a single event in production. The GitHub App setup docs only ever asked operators to grant `pull_requests: read` and subscribe to `pull_request`, so GitHub never delivered `check_suite` — `handleCheckSuiteEvent` sat dead behind a subscription nobody was told to enable. Every linked PR reports checks_passed/failed/pending = 0 and the sidebar row falls through to "Checks haven't reported yet" forever. Docs (the root cause), all four locales: - add `Checks: Read-only` permission + `Check suite` event to the App setup table - drop the stale "CI check states are not modeled" claim, which predates MUL-2228 and is what let the setup table stay incomplete - add a "PR rows show no CI status" troubleshooting entry with the public `/apps/<slug>` probe to confirm what an App is actually subscribed to, and a warning that existing installations must accept the new permission before any `check_suite` is delivered UI: - give the actionable status kinds (checks failed/pending/passed, conflicts, ready) their own icon + color. CI outcome previously rendered as plain muted 11px text, visually identical to the diff stats beside it — a failing build read the same as "+437 −6 · 6 files". Terminal and unknown kinds stay muted; the row's state icon already carries that meaning. Co-authored-by: multica-agent <[email protected]> * fix(github): unbreak docs build, stop overclaiming CI completeness (MUL-5180) Both must-fixes from review. 1. docs production build failed. `<your App>` in prose was parsed as a JSX tag, so `pnpm --filter @multica/docs build` died with `Expected a closing tag for <your>`. Dropped the angle brackets. Repo CI never caught this because no workflow runs the docs production build — only Vercel does, which is why the PR's GitHub checks were green while the deployment errored. 2. `Checks: Read-only` cannot support the pending status the docs promised. GitHub's webhook contract delivers `check_suite.requested` / `.rerequested` only to Apps holding Checks *write*; read-level access receives `completed` only. Verified against GitHub's published docs. Direction chosen: keep read-only, degrade honestly to final-results-only. Checks *write* is a repo-write capability (create/update check runs), not a wider read — escalating every installation to it just to render an in-flight spinner is not a trade to make on the operator's behalf, and it contradicts the integration's read-only posture. The concrete bug this leaves is premature green: with two reporting apps, the first to complete makes total=1/passed=1 and the row claimed "All checks passed" while the second was still running and might fail. Copy is now "Checks passed" in all four locales — it reports what reported and never asserts completeness. `derivePullRequestStatusKind` documents why. Docs gain a "what CI status can and cannot tell you" section (all four locales) with the read-vs-write delivery table, both consequences stated plainly, and the opt-in path for teams that do want in-flight status: set Checks to Read and write on their own App and the existing pending code lights up with no code change. The pending promise is removed from the read-only setup path. Co-authored-by: multica-agent <[email protected]> * fix(github): ignore non-completed check_suite actions (MUL-5180) Review was right: the `Read and write` opt-in the previous commit documented does not produce reliable pending, and following it would break the card. `check_suite.requested` / `.rerequested` are not observations that some CI provider started. GitHub sends them only to Apps holding Checks write, and per the CI-checks App docs they mean "GitHub has created a check suite for YOUR app on this commit; now add your check runs to it". Multica observes other apps' results and never creates check runs. Recording such a suite parks a `queued` row nothing can ever complete, and since `checks_pending` outranks `checks_passed` in derivePullRequestStatusKind, one stuck row freezes every PR on that installation at "checks running" and hides the real pass/fail result. Any self-hoster who already grants Checks write hits this on every push, so the gate is on the action, not the permission. - handleCheckSuiteEvent drops every action except `completed`, with the reasoning and the "don't resurrect requested as a running signal" warning recorded at the gate. - TestWebhook_CheckSuite_QueuedCountsAsPending encoded the wrong delivery semantics (two external apps sending `requested`, which GitHub never does). Replaced by TestWebhook_CheckSuite_NonCompletedActionsIgnored, which pins the drop and checks a later `completed` suite still lands. - The two out-of-order stash tests used `requested` payloads to exercise paths that are really about completed suites; both now use `completed` and assert the same guarantees. - Docs (four locales): the write opt-in is gone. In-flight CI is documented as unsupported at any permission level, with the actual reason and the note that real running status needs polling or a check_run model instead. Co-authored-by: multica-agent <[email protected]> * fix(github): make legacy non-completed check suites inert (MUL-5180) Review was right again: the previous commit gated the webhook entry point but left the pre-upgrade state — and the people it was meant to protect (self- hosters who already granted Checks write) are exactly the ones holding it. Two leftovers, both now closed: 1. Rows already in github_pull_request_check_suite. The old handler stored GitHub's `requested` suites as `queued`; nothing will ever complete them. ListPullRequestsByIssue still counted them, so `checks_pending` kept outranking `checks_passed` and the PR stayed pinned to "checks running" for as long as its head SHA stood. The aggregation now selects only `completed` suites. Filtering beats deleting here: recovery is automatic on deploy, needs no migration over a table that can be large, and holds for any writer that misses a gate — not just for today's legacy rows. DISTINCT ON runs after the filter, so an app whose newest suite is a stuck `queued` still reports its most recent completed verdict instead of disappearing. 2. Rows already in github_pending_check_suite. replayPendingCheckSuitesForPR is a second write path into the live table that never passes through handleCheckSuiteEvent, so the next `pull_request` event would re-inject a permanently-queued suite after the fix shipped. It now skips non-completed rows; the drain is DELETE ... RETURNING, so skipping discards them. Both are covered by regression tests that seed the legacy row directly — the fixed handler can no longer produce one — and both were confirmed to fail with their respective fix reverted. The stash test additionally asserts its fixture landed under the repo address the drain keys on; the first draft used the wrong owner and passed vacuously. Also corrects the aggregateChecksConclusion doc comment, which still described "pending" as a not-yet-completed suite. It is now reachable only for a completed suite carrying a null conclusion, and is explicitly not a "CI is running" signal. Co-authored-by: multica-agent <[email protected]> * test(github): assert the legacy stash row is consumed by the drain (MUL-5180) Review nits. The stash test proved its fixture existed before the webhook but never that the drain consumed it, so a future change to firePullRequestWebhookWithHead's repo address would make the assertions pass for the wrong reason again — the same way the first draft of this test did. Asserting the stash is empty afterwards closes that gap from the other side. Also fixes two comment typos: `an "CI is running"` -> `a`, and drops the "merged-but-open PR" state, which cannot exist. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-07-23 21:23:25 +08:00
3 people	53f05cca5e	feat(github): fan out PR/check_suite webhooks to all bound workspaces (MUL-4343) (#5218) ... * feat(github): fan out PR/check_suite webhooks to all bound workspaces (MUL-4343) One GitHub App installation can be bound to several workspaces (#4855), but pull_request and check_suite webhooks were still routed to a single workspace via resolveWorkspaceForRepo (workspace.repos registry + oldest-binding fallback). Every workspace but one silently received nothing for a shared repo, with no way to opt in. Deliver each repo event to every workspace bound to the installation. Repo scope is whatever GitHub authorized the installation for; we no longer gate on the workspace.repos registry (that list means "code the agent clones", not a webhook subscription). Each workspace independently mirrors the PR, auto-links against its own issue prefix + github toggles, records check suites against its own PR mirror, and gets its own realtime broadcast. - Extract mirrorPullRequestForWorkspace / recordCheckSuiteForWorkspace and loop over all installation bindings instead of resolving one workspace. - Remove the now-dead resolveWorkspaceForRepo / repoIdentityFromURL routing. - Replace the registry-routing tests with PR + check_suite fan-out tests. Co-authored-by: multica-agent <[email protected]> * test(github): out-of-order fan-out test + drop dead query + document multi-workspace delivery (MUL-4343) Addresses review feedback on the webhook fan-out change: - Add TestWebhook_CheckSuite_OutOfOrderFansOutToBoundWorkspaces: a check_suite that arrives before the PR must stash a pending row per bound workspace, and each workspace must drain its own row when the PR fans out. - Remove the now-unused ListWorkspacesWithRepos query (its only caller was the deleted resolveWorkspaceForRepo) and regenerate sqlc; fix the stale "picks the target workspace via the repos registry" comment on ListGitHubInstallationsByInstallationID. - Document multi-workspace event delivery in the GitHub integration docs (en + zh), including an explicit self-host upgrade note: delivery is now keyed on the GitHub connection, so a workspace that relied on the code-repository list alone (without connecting GitHub) must connect the installation to keep receiving events. This is an intentional, documented behavior change — the PR description's earlier "single-binding behavior is unchanged" claim was inaccurate and has been corrected. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-07-10 20:10:25 +08:00
3 people	7116691c07	fix(github): hide reference-only PR links from the issue PR list (#4611) ... * fix(github): hide reference-only PR links from the issue PR list A PR that merely mentions an issue key in passing in its description (e.g. "Related to MUL-3739") was auto-linked and shown in that issue's right-side PR list as if it were a working PR for the issue. Add a reference_only flag to issue_pull_request. The webhook keeps linking generously (so close_intent stays trackable across edits) but flags a link as reference_only unless the key is a genuine target: a title prefix, a branch reference, or a body closing keyword (Closes/Fixes/Resolves). ListPullRequestsByIssue filters reference_only rows, so passing body mentions are hidden from the CLI and the UI PR list while real targets remain. reference_only follows the same terminal preserve gate as close_intent; the auto-advance gate is unchanged. Closes MUL-3739 Co-authored-by: multica-agent <[email protected]> * fix(github): exclude reference_only links from the close aggregate A reference_only link is hidden from the issue PR list, but GetIssuePullRequestCloseAggregate still counted it toward open_count. An open body-only mention ("Related to MUL-X") could therefore block the issue from auto-advancing to `done` after a real closing PR merged, while being invisible in the right-side PR list. Filter `AND NOT reference_only` in the aggregate too (reference_only rows never carry close_intent, so merged_with_close_intent_count is unchanged). Add TestWebhook_HiddenBodyMentionDoesNotBlockAutoAdvance. Addresses code review on PR #4611. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Lambda <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-07-05 13:30:18 +08:00
3 people	5901997bf6	fix(squad): wake private-leader squad parent leader on child-done (MUL-4063) (#4934) ... * fix(squad): wake private-leader squad parent leader on child-done The child-done parent wake routed squad leaders through canEnqueueSquadLeader/canInvokeAgent, while the agent-parent path (triggerChildDoneAgent) has never gated. Agents default to private visibility, so a default squad leader is private; when a child is closed by an agent/system actor (the normal process-squad pipeline) there is no resolvable human originator, the gate fails closed, and the leader is never woken -- stranding every multi-stage squad pipeline after its first stage. Assigning the parent directly to the leader agent worked only because that path is ungated. Remove the child-done leader-invocation gate so agent and squad child-done follow one path. The parent was already permission-checked at squad-assign time (validateAssigneePair); waking its own leader to advance the next stage is a coordination handoff, not a fresh invocation, and grants no new privilege -- the actor can only wake the leader on the specific parent that leader already owns. If invocation permission is ever reintroduced it must be added to both paths together. Also drops the now-dead actor plumbing threaded solely for the gate, flips the plain-member child-done test to assert the leader is woken, adds an agent-actor regression, and updates the squad / mentioning skill docs. MUL-4063 Co-authored-by: multica-agent <[email protected]> * docs(squad): refresh Private Leader Access source map to canInvokeAgent The squad + mentioning source maps still described the old canAccessPrivateAgent model (visibility!=private, agent short-circuit, system->agent remap). The trigger gate is canInvokeAgent (MUL-3963); update both to match and note the child-done wake is now ungated (MUL-4063). Review nit follow-up, docs only. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-07-05 02:35:19 +08:00
3 people	e4994cd431	fix(github): allow one installation to bind multiple workspaces (#4855) ... Connecting the same GitHub App installation in a second workspace silently overwrote the first workspace's binding: github_installation was UNIQUE(installation_id) and CreateGitHubInstallation's upsert overwrote workspace_id on conflict (#4823). Widen the uniqueness key to (workspace_id, installation_id) so each workspace keeps its own binding row, and teach the webhook/lifecycle paths to handle N bindings per installation_id: - CreateGitHubInstallation upserts per (workspace_id, installation_id). - Webhook lookup lists all bindings; PR/check_suite routing uses the oldest binding as the deterministic fallback and still routes per-repo via the existing workspace.repos registry. - installation.deleted/suspend drops every workspace binding and broadcasts to each affected workspace. - installation.created/unsuspend refreshes account metadata across all bindings. - Add a standalone index on installation_id (the dropped unique constraint was the only index behind the webhook lookup). MUL-3950 Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-07-02 17:21:50 +08:00
Ivan Fokeev	ca43c83abc	MUL-3523: fix(github): route PR/check_suite webhooks by repo ... Fix GitHub pull_request and check_suite webhook routing so events are attributed to the workspace that registered the repository, with fallback to the installation workspace. Includes host-qualified repo matching, account-gated registry routing, deterministic matching, and regression coverage.	2026-06-22 23:44:46 +08:00
3 people	41586f1499	fix(github): surface in-flight CI on PR cards (MUL-2392) (#2887) ... * fix(github): surface in-flight CI on PRs and recover out-of-order check_suite events Two bugs caused PR cards to render "checks not reported yet" while CI was actually running (MUL-2392): 1. handleCheckSuiteEvent dropped every action except `completed`, so `requested`/`rerequested` events (status queued/in_progress) never landed in the suite table. Aggregated `checks_pending` stayed at 0 until the first suite finished, and the frontend fell through to the unknown bucket. Persist all actions; the ListPullRequestsByIssue aggregation already counts status<>completed as pending. 2. A check_suite for an unmirrored PR was logged and dropped, with no replay path. Add a `github_pending_check_suite` stash keyed by (workspace, repo, pr_number, suite_id); the pull_request webhook drains it after the PR upsert and replays each entry through the normal check_suite upsert. One-shot drain via DELETE … RETURNING keeps it idempotent and free of retry storms. Follow-ups for fork PRs (empty `pull_requests[]`) and a more specific frontend placeholder ship in separate issues. Co-authored-by: multica-agent <[email protected]> * fix(github): guard pending check_suite stash against out-of-order events UpsertPendingCheckSuite previously overwrote unconditionally on conflict, so an older `requested/in_progress` event arriving after a newer `completed/success` for the same suite would roll the stash back to pending. The subsequent PR upsert then drained the stale state and the PR card stuck on "pending" until the next suite. Mirror the suite_updated_at guard from UpsertPullRequestCheckSuite and add a regression test covering the PR-missing path. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Lambda <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-06-17 09:24:15 +02:00
3 people	64ce459e30	fix(github): preserve early installation webhook metadata (#4193) ... Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-06-17 12:26:25 +08:00
3 people	905ebbdde1	fix(github): populate connected account name on install [MUL-3078] (#3811) ... * fix(github): populate connected account name on install [MUL-3078] The Settings → GitHub connection card was rendering 'Connected to unknown' because: 1. fetchInstallationAccount in the setup callback hit GitHub's /app/installations/{id} endpoint unauthenticated. That endpoint requires App JWT auth; the call returned 401, and the function fell through to the 'unknown' placeholder which was persisted as account_login. 2. The installation webhook handler did upsert the row with the real login when GitHub later delivered installation.created, but it never published a github_installation:created event. The frontend query stayed stale, so the UI kept showing 'unknown' even after the row had been refreshed. Fix: - Add optional GITHUB_APP_ID + GITHUB_APP_PRIVATE_KEY env vars. When set, signGitHubAppJWT mints a short-lived RS256 JWT (back-dated 60s for clock skew, capped at 9m to stay inside GitHub's 10m max) and fetchInstallationAccount uses it as a Bearer token. The setup callback now writes the real org/user name on install. - When the new env vars are not configured, the call still falls through to 'unknown' as before — but the webhook handler now publishes EventGitHubInstallationCreated after the upsert, so the realtime listener invalidates the installations query and the UI converges to the real value within seconds, no manual refresh. Tests cover JWT signing (claims, signature, malformed PEM, partial config), fetchInstallationAccount with a JWT-gated httptest mock, and the webhook refresh + broadcast on a seeded 'unknown' row. Docs updated for .env.example and github-integration / environment-variables in en, zh, ja, ko. Co-authored-by: multica-agent <[email protected]> * test(github): defuse JWT clock-bomb by injecting parser time [MUL-3078] PR review caught that TestSignGitHubAppJWT_ClaimsAndSignature signed the token with a fixed 'now' (2026-06-05T12:00:00Z) but parsed it with a default jwt.Parse, which uses real time.Now() for exp validation. Once real wall clock crossed the token's exp (now + 9m = 12:09:00Z), the test would have flipped to a deterministic failure on every CI run. Inject the same fixed 'now' into the parser via jwt.WithTimeFunc so both signing and validation share one clock. Verified independently that without the fix the parser rejects the token as 'expired', and with the fix it accepts. Also clarified the fetchInstallationAccount comment to be unambiguous about what 'do not block' actually means: the HTTP call IS synchronous (no independent timeout, pre-existing wart), but a failure here just falls back to the unknown placeholder rather than aborting the callback. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Eve <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-06-05 15:21:22 +08:00
3 people	8c98940b79	Lark Bot integration MVP: migration + service boundary (MUL-2671) (#3277) ... * feat(db): add Lark integration migration (MUL-2671) Introduces seven tables for the 飞书 Bot integration MVP — per-agent PersonalAgent installations, user/chat bindings, inbound dedup + non-content drop audit, outbound card mapping, and short-lived single-use member binding tokens. Schema notes: - chat_session schema unchanged; Lark routes through a separate binding table rather than adding a metadata JSONB column. - Outbound card mapping is task/message scoped so multiple runs on the same session can't stomp each other's cards. - lark_inbound_audit stores routing / identity / drop_reason ONLY, never message body — the audit channel for unbound users and group messages that don't address the Bot. - app_secret stores ciphertext (encryption helper lands in a follow-up commit on this branch); DB never sees plaintext. Co-authored-by: multica-agent <[email protected]> * feat(util): add secretbox AES-256-GCM helper for at-rest secrets First consumer is lark_installation.app_secret (MUL-2671 §4.4), but the helper is intentionally generic — future per-tenant secrets that must not appear in a DB dump can reuse it. Construction: AES-256-GCM with a per-message random nonce, providing authenticated encryption. Tampered ciphertext fails Open instead of silently decrypting to garbage. Master key loaded from a base64 env var via LoadKey; key rotation is not in scope yet. Co-authored-by: multica-agent <[email protected]> * refactor(issues): extract IssueService.Create as single create entry (MUL-2671) Establishes the service-layer boundary mandated by Elon's 二审 of MUL-2671 §4.8: issue creation no longer lives inside the HTTP handler. Both the HTTP POST /issues handler and the future Lark /issue command call into service.IssueService.Create, so duplicate guard, issue numbering, attachment linking, broadcast, analytics, and agent/squad enqueue stay aligned. Handler responsibilities shrink to parsing the HTTP request, doing actor resolution / validation (transport-specific), and converting service results into the IssueResponse + 201. The transaction-wrapped core, attachment link, event publish, analytics capture, and agent/squad enqueue all move into service.IssueService.Create. A BroadcastPayload callback on the service keeps the WS broadcast shape (the full IssueResponse) without forcing the service to depend on handler-layer response types. Co-authored-by: multica-agent <[email protected]> * feat(integrations): add Lark package skeleton (MUL-2671) Establishes the architectural boundaries Elon's 二审 mandated as first-PR blockers without dragging in OAuth, WebSocket, or card-patching code (those land in follow-up PRs): - ChatSessionService interface — channel-aware chat-session entry point for Lark, deliberately separate from the HTTP SendChatMessage handler. The HTTP handler's single-creator guard (creator_id == request user_id) is correct for the browser client but rejects group chat_sessions by construction; Lark needs its own service. - AuditLogger interface — the only path for recording dropped events. Its signature deliberately omits message body, enforcing the drop-audit policy (MUL-2671 §4.7) at the type level: unbound users and non-addressed group messages can't accidentally end up in chat_session. - Typed IDs (OpenID, ChatID) prevent UUIDs from being conflated with Lark-side identifiers at compile time. - DropReason constants align dashboard/audit queries across callers. Co-authored-by: multica-agent <[email protected]> * refactor(issues): move parent/project workspace check into IssueService (MUL-2671) Parent existence and project workspace membership now live inside IssueService.Create, inside the same transaction as the duplicate guard and counter increment. The HTTP handler stops re-implementing the lookup; every future create entry (Lark /issue, MCP, API keys) inherits the same boundary without copy-pasting the SQL. Adds two error sentinels (ErrParentIssueNotFound, ErrProjectNotFound) so transports can translate to their own error shapes. Handler-level cross-workspace tests guard the boundary against future regressions. Co-authored-by: multica-agent <[email protected]> * fix(db): harden Lark migration safety底座 — TTL cap + workspace FK (MUL-2671) Two storage-layer hardenings that move the must-fix line off "the app layer enforces it" and onto the schema itself, so future write paths or hand-inserted rows cannot regress the invariants. 1) lark_binding_token TTL cap. The DB CHECK was 1 hour as defense-in-depth while the app constant was 15 minutes; the CHECK now matches the product cap (15 minutes). Application constant docstring updated to reflect that storage enforces the same bound. 2) lark_user_binding workspace membership. The table previously only FK'd to workspace / user / installation independently, so a binding could exist for a user no longer in the workspace, or claim a workspace different from its installation's. Two composite FKs close the gap structurally: * (installation_id, workspace_id) → lark_installation(id, workspace_id) — guarantees a binding's workspace_id always matches its installation's workspace_id. A new UNIQUE (id, workspace_id) on lark_installation is added as the FK target. * (workspace_id, multica_user_id) → member(workspace_id, user_id) with ON DELETE CASCADE — when a user is removed from the workspace, the binding cascades away in the same transaction. There is no longer a path where lark_user_binding outlives workspace membership. These two FKs are the schema-level proof for §4.3's "unbound or non-workspace members cannot leak content into chat_session" invariant. Co-authored-by: multica-agent <[email protected]> * feat(integrations/lark): inbound services + /issue dispatcher (MUL-2671) Lands the inbound service layer for the Lark Bot MVP, sitting on top of the migration + service-boundary scaffold from the previous commits. What ships: - sqlc queries for all seven lark_* tables (idempotent dedup insert, CAS WS-lease, single-use binding-token consume, etc.) plus GetMostRecentUserChatMessage for the /issue fallback. - AuditLogger backed by lark_inbound_audit; signature deliberately body-free so callers cannot leak content into the drop log. - ChatSessionService: find-or-create chat_session via the binding table (winner-takes-all on the UNIQUE race), append-with-dedup, /issue parser, "previous user message" fallback for bare `/issue` invocation. - Dispatcher orchestrates the inbound pipeline in one place: installation routing → group-mention filter → identity check → ensure session → append+dedup → /issue → enqueue chat task. Group sessions use the installer as creator (stable workspace identity); p2p uses the sender. Agent-offline path falls through with OutcomeAgentOffline so the WS adapter can reply with the offline notice from §4.6. - BindingTokenService: random URL-safe token, SHA-256 stored hash, 15-min TTL pinned at the application AND the DB CHECK; Redeem returns the same opaque error for all rejection cases (no timing oracle on replay). - Unit tests for the parser (13 cases), dispatcher (8 cases via fake Queries/Chat/Audit/IssueCreator/Enqueuer), and binding-token hash/entropy. Real-DB integration tests for OAuth + token redeem land alongside the HTTP handlers in the next commit. Out of scope for this commit (next ones on the same feature branch): OAuth callback, HTTP routes, WebSocket hub, outbound card patcher, frontend. Co-authored-by: multica-agent <[email protected]> * feat(integrations/lark): installation HTTP surface + secretbox-gated wiring (MUL-2671) Lands the HTTP boundary on top of the inbound services from the previous commit. What ships: - InstallationService.Upsert: the only path that writes lark_installation. Encrypts app_secret with the secretbox passed in at construction time; refuses to fall back to plaintext storage (returns an error from the constructor if no Box is supplied), so a misconfigured dev environment cannot accidentally land a row with cleartext credentials. Revoke flips status without DELETE so audit trail survives. - HTTP handlers under /api/workspaces/{id}/lark/: * GET /installations — member-visible (Integrations tab renders for non-admins). Soft 200 with empty list + configured:false when MULTICA_LARK_SECRET_KEY is unset, so the tab does not error on self-host that has not opted in. * POST /installations — admin-only; 503 when not configured. Re-validates agent_id ∈ workspace before accepting credentials so a cross-workspace agent UUID is rejected. * DELETE /installations/{id} — admin-only; workspace-scoped lookup so one workspace cannot revoke another's installation by UUID guess. - POST /api/lark/binding/redeem (user-scoped, no workspace context): the only path that mints a lark_user_binding row from user action. Redeemer identity comes from the session, not the token, so a stolen link cannot bind an open_id to an attacker's Multica user. The composite FK on lark_user_binding cascades the binding away if the user is not (or no longer) a workspace member, so a non-member who steals the link gets 403 at the DB layer. - Two new event-bus types in protocol.events: EventLarkInstallationCreated, EventLarkInstallationRevoked. - Router wiring: MULTICA_LARK_SECRET_KEY drives a conditional initialization of h.LarkInstallations + h.LarkBindingTokens. When unset, the integration disables itself with an INFO log and the rest of the server boots normally. - Handler tests cover all four not-configured short-circuits. Happy-path integration tests (real DB, full create→list→revoke cycle and token mint→redeem) ship alongside the WS hub PR. Co-authored-by: multica-agent <[email protected]> * fix(integrations/lark): close binding-token rebind & typed task errors (MUL-2671) Two must-fixes from PR review on HEAD 87ad15e1: 1. Binding-token redeem could be used to grab an already-bound Lark open_id. Two changes harden the path: - lark.sql `CreateLarkUserBinding` now gates ON CONFLICT DO UPDATE on `multica_user_id = EXCLUDED.multica_user_id`, so a cross-user rebind via a second valid token returns zero rows instead of silently switching ownership. - `BindingTokenService.RedeemAndBind` consumes the token and writes the binding row inside one transaction. A failed bind no longer burns the token; a successful bind never leaves a consumed-but- unused token. Distinct typed errors: ErrBindingTokenInvalid (410), ErrBindingAlreadyAssigned (409), ErrBindingNotWorkspaceMember (403). The handler maps each to its own status code. 2. Dispatcher collapsed every `EnqueueChatTask` error to `OutcomeAgentOffline`, hiding infra failure and misusing the "offline" label for cases (e.g. archived agent) where it doesn't fit. Now: - `service.EnqueueChatTask` returns `ErrChatTaskAgentNoRuntime` and `ErrChatTaskAgentArchived` as sentinel errors; DB / load / insert failures stay wrapped as ordinary errors. - Dispatcher uses `errors.Is` to map only the productizable cases (`OutcomeAgentOffline`, new `OutcomeAgentArchived`); any other error is returned to the WS adapter so it can retry or page instead of disguising the outage as an offline card. A daemon that's merely disconnected is still NOT an error — as long as `agent.runtime_id` is set the chat task enqueues and waits for the daemon to claim it on next online (returns `OutcomeIngested`). Co-authored-by: multica-agent <[email protected]> * ci: re-trigger workflow on lark MVP must-fix HEAD Co-authored-by: multica-agent <[email protected]> * ci: re-trigger workflow on lark MVP must-fix HEAD (retry) Co-authored-by: multica-agent <[email protected]> * test(integrations/lark): guard binding-token sentinel contract (MUL-2671) Two unit tests that document and protect the must-fix invariants without requiring a DB: 1. TestRedeemAndBindRequiresTxStarter — if a future refactor wires up BindingTokenService without a TxStarter, RedeemAndBind must fail fast with a clear error rather than nil-panic on Begin. The atomicity contract (consume + bind commit together) depends on that transaction existing. 2. TestBindingErrorSentinelsAreDistinct — the HTTP handler maps ErrBindingTokenInvalid → 410, ErrBindingAlreadyAssigned → 409, ErrBindingNotWorkspaceMember → 403. Accidentally aliasing them (e.g. var ErrBindingAlreadyAssigned = ErrBindingTokenInvalid) would silently regress the response codes without any other test catching it. Co-authored-by: multica-agent <[email protected]> * feat(integrations/lark): WS hub orchestrator + outbound card patcher (MUL-2671) The hub owns one supervisor goroutine per active installation. Each supervisor acquires the WS lease via the existing CAS query, runs an EventConnector (interface — real Lark wire protocol lands in a follow-up behind it), renews the lease on a tighter cadence than the TTL, and backs off (with jitter) on connector failure. Lease loss tears the connector down cleanly; revocation is reaped on the next sweep. Per- process node id satisfies §4.4 multi-replica safety: at most one Hub globally holds the lease for any installation. The patcher subscribes to task / chat-done events on the existing events.Bus and keeps the per-task Lark interactive card in sync (thinking → streaming → final | error). Card binding is per-task as required by §4.5; throttled patches via an in-memory last-patched map; final / error transitions bypass the throttle so the user always sees the terminal state. The Renderer is plug-replaceable so the product card template can evolve without touching transport. The APIClient interface centralizes the Lark Open Platform surface this package needs (send card, patch card, send binding prompt, exchange OAuth code). The default stubAPIClient returns ErrAPIClientNotConfigured for every transport call so a misconfigured deployment fails loudly instead of dropping cards silently. Real implementation lands in a follow-up; OAuth callback + frontend entries land in the next commits on this branch. Co-authored-by: multica-agent <[email protected]> * feat(integrations/lark): OAuth install start / callback (MUL-2671) OAuthService builds a signed-state Lark authorization URL the frontend can render as a QR (or open directly), then on callback verifies the HMAC-protected state, exchanges the OAuth code for installation credentials via APIClient.ExchangeOAuthCode, and persists the row via InstallationService.Upsert (which keeps app_secret encryption inside a single chokepoint). State token format: workspaceID.agentID.initiatorID.expiresUnix.nonce.sig — HMAC-SHA256 over the first five fields with a deployment-level secret. TTL defaults to 10 minutes (covered by tests). Three failure modes (invalid state / expired state / missing code) map to typed errors so the HTTP handler can emit a single lark_error= query param the frontend uses to pick copy. Both endpoints degrade cleanly: the at-rest key gate (already in place) returns 503 from /install/start when the InstallationService is nil, and the OAuth gate (MULTICA_LARK_OAUTH_APP_ID / _SECRET / _REDIRECT_URI / _STATE_SECRET) returns configured:false from /install/start so the frontend can render "configure manually instead" without an error banner. /install/callback always finishes with a redirect to /settings?tab=lark carrying either lark_installed=1 or lark_error=<code>. Tests cover signed-URL shape, missing-config rejection, tampered state, expired state, propagated exchange error, and the no-config redirect path on the HTTP handler. Co-authored-by: multica-agent <[email protected]> * feat(views/lark): settings tab + agent bind button + /lark/bind redemption page (MUL-2671) Adds the user-facing Lark surface across the shared packages: - packages/core/types/lark.ts — wire shapes that mirror server/internal/ handler/lark.go. Optional fields default to undefined so older desktop builds keep parsing if the server adds new keys (CLAUDE.md → API Response Compatibility). - packages/core/lark/{queries,index}.ts — Tanstack Query options keyed by workspace id; realtime sync invalidates `installations(wsId)` on `lark_installation:*` events. - packages/core/api/client.ts — listLarkInstallations, getLarkInstallURL, deleteLarkInstallation, redeemLarkBindingToken. - packages/views/settings/components/lark-tab.tsx — Settings → Lark panel. Listing is member-visible (matches backend); disconnect is admin-only. Empty state points users at the per-Agent bind entry, matching the (workspace_id, agent_id) UNIQUE: there is no "pick an agent" UI here because the bind URL is per-agent. - LarkAgentBindButton (same file) is the per-Agent CTA the Agent detail page imports. Opens the OAuth URL in a new tab; the callback bounces back to /settings?tab=lark with a query param the panel reads for inline confirmation copy. - packages/views/lark/bind-page.tsx — the Bot's "you need to bind" destination. Requires session before redeeming, distinguishes the 410/409/403 backend responses into distinct copy. - apps/web/app/lark/bind/page.tsx — Next.js route wrapping the shared bind page in a Suspense boundary (Next 15 useSearchParams rule). i18n: all user-facing strings land in en/zh-Hans, settings tab nav includes a Sparkles-iconed Lark entry, bind-page copy lives under common.lark_bind so it works pre-workspace-context too. typecheck + lint clean. Co-authored-by: multica-agent <[email protected]> * chore(integrations/lark): wire outbound Patcher into server bootstrap (MUL-2671) Constructs the Patcher next to the existing Installation/BindingToken wiring in router.go and Register()s it on the event bus. With the stub APIClient any actual transport call surfaces ErrAPIClientNotConfigured; once the real Lark client lands, swap NewStubAPIClient for the real implementation here without touching the Patcher's subscription logic. doc.go updated to reflect everything the package now contains (Hub, Patcher, OAuthService, APIClient interface). The Hub itself is NOT booted here yet — it needs an EventConnector implementation for the Lark long-connection wire protocol, which lands in a follow-up; the orchestrator code and its unit tests are in place so that follow-up can focus on the WS protocol rather than lifecycle plumbing. Co-authored-by: multica-agent <[email protected]> * fix(integrations/lark): address Elon 二审 5 must-fix items (MUL-2671) - Hub: renewer cancels run ctx on lease loss so the connector exits even if its wire I/O is blocked, keeping the §4.4 ownership invariant intact under lease theft. - Hub: EventEmitter returns (DispatchResult, error) so the real connector can post the matching Lark-side card (needs_binding, agent_offline, agent_archived) and react to infra failures instead of silently logging at the seam. - Dispatcher: top-level message_id dedup runs before group filter and identity check, so a reconnect storm cannot re-fire binding prompts or re-spam not_addressed_in_group audit rows; the in- AppendUserMessage dedup is removed since the table-level UNIQUE is the ultimate backstop. - OAuth: HandleCallback auto-binds the installer via the new InstallerBinder seam (BindingTokenService implements it), so the §2.1 "scan to bind, you're done" promise holds end-to-end. validateExchangeResult now requires installer open_id; new error reason codes wired through the callback redirect. - Frontend / handler: install_supported listing field + StartLark- Install short-circuit on stub APIClient hide install entry points (Settings tab + per-agent button) while no real Lark HTTP client is wired, so users do not land in an OAuth flow that fails at exchange. Includes tests for each fix (lease-loss cancel, emit error propagation, dedup ordering, OAuth installer-bind contract, stub- client install gate) and i18n strings for the new preview state. Co-authored-by: multica-agent <[email protected]> * fix(integrations/lark): two-phase dedup so infra failures do not swallow messages (MUL-2671) The pre-fix top-level dedup wrote the lark_inbound_message_dedup row before EnsureChatSession / AppendUserMessage. An infra error in either step left the row in place and a WS-adapter retry was mis-classified as a duplicate, so the user's Lark message was permanently lost without ever landing in chat_session. Make dedup two-phase: - ClaimLarkInboundDedup acquires an in-flight claim (processed_at NULL). Stale claims older than 60 s are re-takeable so a process crash does not strand the message_id. - MarkLarkInboundDedupProcessed flips processed_at on durable success (audit row OR chat_message + session touch). - ReleaseLarkInboundDedup deletes the in-flight row on infra failure before any durable side effect, so the retry can re-claim immediately. Dispatcher.Handle now finalizes the claim exactly once based on whether the inner pipeline reached a durable outcome — chat_message commit being the transition point (errors past it Mark, errors before it Release). Regression tests cover the two failure variants Elon flagged plus the inverse invariants (durable-error Marks, drops Mark, in-flight replays drop, stale claims re-claim). Co-authored-by: multica-agent <[email protected]> * fix(integrations/lark): owner-fence dedup claim to close the double-write windows (MUL-2671) The two-phase Claim/Mark/Release fix from the previous commit closed the "infra error swallows a replay" gap but left two windows that could still write a chat_message twice for the same Lark message_id: 1. Stale-reclaim race. Worker A claims at t=0, runs slowly past the 60 s staleness TTL but is still alive. Worker B sees the row as stale and re-takes the claim. A reaches AppendUserMessage and commits a second chat_message. 2. Mark window. Worker A commits chat_message but the post-pipeline MarkLarkInboundDedupProcessed fails (DB hiccup) or the process crashes before it runs. 60 s later a retry treats the in-flight row as stale, re-claims it, and writes a second chat_message. Close both with owner fencing + same-tx Mark: - lark_inbound_message_dedup now carries a `claim_token` UUID; ClaimLarkInboundDedup mints a fresh one on insert and on stale re-take, so a reclaim ROTATES the token. - MarkLarkInboundDedupProcessed and ReleaseLarkInboundDedup are fenced on (message_id, claim_token, processed_at IS NULL) and return rowsAffected. Zero means our token is no longer live, and the caller treats it as a no-op (not an error). - AppendUserMessage invokes MarkLarkInboundDedupProcessed INSIDE its chat_message+session tx (qtx). If the token has been rotated by a concurrent reclaim, the Mark matches zero rows and the method returns ErrClaimLost; the deferred Rollback unwinds the chat_message insert, so the other holder is the sole writer. The durable write and the Mark therefore commit (or roll back) atomically — there is no "committed but not yet Marked" window for a crash or retry to exploit. Dispatcher.processClaimed now returns a tri-state dedupFinalize directive (none / mark / release): finalizeNone for the in-tx Mark path (and ErrClaimLost), finalizeMark for audit-drop branches and the defensive post-Append-success fallback, finalizeRelease for pre-durable infra errors. ErrClaimLost is translated into OutcomeDropped + DropReason- Duplicate at the Handle boundary, matching what the WS adapter expects for a "another worker is the writer" outcome. Regression tests: - TestDispatcher_StaleReclaimRaceDoesNotDoubleWrite injects worker B's reclaim via a beforeAppend hook so the claim_token rotates between Claim and AppendUserMessage. Asserts worker A's AppendUserMessage returns ErrClaimLost (no chat_message committed), the dispatcher surfaces a duplicate drop, the token rotated to a value distinct from A's original, and a follow-up replay still duplicate-drops. - TestDispatcher_InTxMarkPreventsPostCommitReclaim verifies the "Mark window" case is unreachable: a successful in-tx Mark produces exactly one Mark call (no post-finalize duplicate), the row is terminal, and a retry with dedupReclaim=true still duplicate-drops without re-rotating the token. - TestDispatcher_InTxMarkSucceedsAndSkipsPostFinalize pins the positive contract: DedupMarked=true must make applyFinalize a no-op (no extra Mark, no Release). fakeQueries gains a fakeDedupRow model carrying (processed, token, rotations) so the test seam matches production's UPDATE-with-WHERE semantics; fakeChat gains a beforeAppend hook to inject race timing. go test ./... and go vet ./... pass. Co-authored-by: multica-agent <[email protected]> * feat(integrations/lark): real Lark HTTP APIClient for IM v1 send/patch (MUL-2671) Lands the production Lark Open Platform HTTP APIClient that replaces the stub for outbound transport. The patcher's "thinking → streaming → final | error" card lifecycle and the dispatcher's binding-prompt card both now reach Lark for real once MULTICA_LARK_HTTP_ENABLED=true. Scope of this stage: - tenant_access_token retrieval via /open-apis/auth/v3/ tenant_access_token/internal, cached in-process per app_id with a 60s safety margin against Lark's `expire` value. Sub-2-minute expires are clamped to 120s so we never cache an entry that's already past its safe window. - SendInteractiveCard: POST /open-apis/im/v1/messages?receive_id_type=chat_id returning the Lark message_id the Patcher persists in lark_outbound_card_message for later patches. - PatchInteractiveCard: PATCH /open-apis/im/v1/messages/:id with the full re-rendered card body (Lark's update endpoint replaces, not deep-merges). - SendBindingPromptCard: open_id-targeted interactive card with a primary "去绑定" CTA pointing at the redemption URL. Template is co-located with the transport so the dispatcher never has to know about Lark's card schema. - Token-error invalidation: Lark codes 99991663 (expired) / 99991664 (invalid) drop the cached token so the next call refreshes from /tenant_access_token/internal instead of looping on a stale entry. Out of scope (deferred to follow-up stages): - ExchangeOAuthCode stays unimplemented behind ErrAPIClientNotConfigured. The PersonalAgent install handshake's response shape (returning per-installation app credentials in a single call) is not yet verified against the production endpoint, and a silent mis-fill of OAuthExchangeResult would corrupt lark_installation rows past validateExchangeResult. Operators continue to use the manual-paste InstallationService path until the OAuth stage lands. - Inbound WS EventConnector — Hub's ConnectorFactory still needs a real wire-protocol implementation. Wiring: - MULTICA_LARK_HTTP_ENABLED=true switches router.go from the stub to the real client. MULTICA_LARK_HTTP_BASE_URL overrides the default open.feishu.cn host (set to open.larksuite.com for the Lark international tenant, or to an httptest URL for integration tests). - The OAuth handler now also receives the real client (its ExchangeOAuthCode still surfaces ErrAPIClientNotConfigured, so callback behavior is unchanged until that stage lands). Tests (19 new cases against an httptest.Server fake): - happy path send/patch/binding-prompt round trips, asserting URL query params, body shape, Authorization header - token cache: 3 sends share one /tenant_access_token/internal hit - token refresh after clock-driven expiry - sub-margin expire clamping (10s expire → cached for >= safety margin of wall-clock) - Lark error code surfacing (230001 send, 230002 patch, 10003 auth) - token-expired (99991663) invalidates the cache; caller's retry re-fetches and succeeds - non-2xx HTTP status surfaces "http 500: …" - input validation: missing chat_id short-circuits BEFORE auth round-trip, missing card json / open_id / bind url all fail pre-flight without hitting Lark - ExchangeOAuthCode still returns ErrAPIClientNotConfigured - binding-prompt template carries the BindURL and the localized "去绑定" CTA in valid JSON go build ./..., go vet ./..., and go test ./internal/integrations/lark/... pass. Pre-existing handler/router integration tests that require a real Postgres connection are unaffected by this change. Co-authored-by: multica-agent <[email protected]> * fix(integrations/lark): split outbound vs OAuth-install capability + card update_multi (MUL-2671) Address Elon's two must-fix items from the HEAD a09993b1 review: 1. HTTP outbound and OAuth-install are now distinct APIClient capabilities. The new SupportsOAuthInstall() reports whether the install flow can succeed end-to-end (i.e. ExchangeOAuthCode is implemented); the real httpAPIClient still returns IsConfigured() = true (send / patch / binding prompt work) but SupportsOAuthInstall() = false until the PersonalAgent install-time response shape is pinned. Handler-side `install_supported` and StartLarkInstall now gate on SupportsOAuthInstall, so a half-wired client never reveals the scan-to-bind UI. larkOAuthErrorReason also maps ErrAPIClientNotConfigured to a dedicated `oauth_exchange_unimplemented` reason so a raw callback hit no longer masquerades as `internal_error`. 2. defaultRenderer now emits config.update_multi=true on every Kind. Lark refuses to apply PatchInteractiveCard to a card whose initial config doesn't declare it shared/updatable, so the absent flag would make every patch after the first send silently no-op on the wire while the local outbound status row still flipped to streaming/final. Tests cover both halves of each fix: - TestHTTPClient_SupportsOAuthInstall_FalseUntilExchangeLands + TestHTTPClient_StubReportsBothCapabilitiesFalse pin the new capability surface. - TestStartLarkInstall_TransportOnlyClientReportsNotConfigured + TestListLarkInstallations_TransportOnlyClientReportsInstallNotSupported pin the handler gate at exactly the half-wired state. - TestLarkOAuthErrorReason_APIClientNotConfigured pins the mapping for both the bare sentinel and the fmt.Errorf-wrapped form HandleCallback produces. - TestDefaultRendererConfigCarriesUpdateMulti covers every CardKind. - TestHTTPClient_(Send|Patch)InteractiveCard_DefaultRendererBodyHasUpdateMulti verify the wire body Lark actually receives carries update_multi through both send and patch transport paths. Co-authored-by: multica-agent <[email protected]> * feat(integrations/lark): real OAuth code exchange + agent-detail bind entry (MUL-2671) Stages the install side of the MVP critical path on top of the real HTTP outbound work: - httpAPIClient.ExchangeOAuthCode runs the production Lark v2 OAuth flow: POST /authen/v2/oauth/token to swap the authorization code for the installer's open_id, then GET /bot/v3/info under the parent app's tenant_access_token to fetch bot_open_id. Result feeds InstallationParams unchanged so OAuthService.HandleCallback's auto-bind step lights up automatically. - HTTPClientConfig gains OAuthAppID/OAuthAppSecret, read from the same MULTICA_LARK_OAUTH_APP_ID/_APP_SECRET env vars the OAuthConfig consumes. SupportsOAuthInstall now mirrors that pair so the install capability gate is honest: outbound transport without OAuth creds reports configured-but-not-install-supported, exactly like before. - Agent detail inspector wires the LarkAgentBindButton in a new Integrations section, viewer-hidden by canEdit. The button still self-hides when SupportsOAuthInstall is false, so a deployment without OAuth creds renders the section empty rather than CTA-broken. - Capability wording cleaned across handler / router / lark-tab to say "OAuth-install capability" instead of "real APIClient wired", and the misleading TransportOnly... test was renamed/refocused on the early-return branch it actually exercises (Elon non-blocking note). Co-authored-by: multica-agent <[email protected]> * fix(integrations/lark): identity-only OAuth + atomic bind (MUL-2671) Addresses Elon's round-4 must-fix items on PR #3277: 1. OAuth v2 token → user_info chain now matches Lark's official user-OAuth shape. `httpAPIClient.ExchangeOAuthCode` POSTs /open-apis/authen/v2/oauth/token (RFC 6749: top-level access_token, NO open_id), then GETs /open-apis/authen/v1/user_info with the user_access_token as Bearer to obtain the installer's open_id / union_id. The test fixture now reflects the real wire shape (separate user_info handler; no synthetic open_id in the token response). 2. `OAuthExchangeResult` is identity-only — drops the synthesized shared-parent AppID / AppSecret / BotOpenID return that broke the UNIQUE(app_id) constraint and the dispatcher's per-app_id routing. `OAuthService.HandleCallback` no longer Upserts an installation row: it looks up the lark_installation already provisioned via the manual-paste POST /lark/installations route and binds the installer onto it. Two new typed errors — ErrInstallationNotProvisioned and ErrInstallationRevoked — map to `installation_not_provisioned` / `installation_revoked` reasons at the HTTP boundary so the UI can guide the admin. The PersonalAgent install API (which would deliver per-installation bot credentials at scan time) remains a follow-up; until it lands the OAuth flow is identity-binding only and the agent-detail bind button stays hidden on deployments without OAuth env (capability gate unchanged). 3. The installation lookup + installer bind run inside a single DB transaction so a concurrent revoke / re-provision between the read and the binding insert cannot leak a half-applied state. `InstallerBinder.BindInstaller` is renamed to `BindInstallerTx` and accepts the OAuth-service-owned transaction's qtx; the binding_token redemption path is unchanged. 4. `validateExchangeResult` is simplified to require only the installer's open_id; the obsolete ErrExchangeMissingAppID / AppSecret / BotOpenID sentinels are removed (no caller can trip them now). The oauth_test suite is rewritten to use a stub failTxStarter so tests covering state-token verification and exchange-error propagation remain DB-free, while a new TestOAuthCallbackOpensTxAfterValidExchange pins the post-must-fix order (state ok + exchange ok ⇒ Begin runs before any lookup or bind, and a Begin failure aborts cleanly with no bind). Verified locally: - go build ./... / go vet ./... clean - go test ./internal/integrations/lark/... ✓ - go test ./internal/handler -run 'Lark|Binding|OAuth' ✓ - go test ./internal/util/secretbox/... ./internal/service/... ✓ Co-authored-by: multica-agent <[email protected]> * feat(integrations/lark): device-flow scan-to-install (MUL-2671) Replaces the manual paste-credentials install path + identity-only OAuth callback (rejected in product review: too many steps before a user sees value) with a true single-step scan-to-install built on Lark's RFC 8628 device-flow registration endpoint (POST accounts.feishu.cn/oauth/v1/app/registration) — the same protocol the official larksuite/oapi-sdk-go/scene/registration package and zarazhangrui/feishu-claude-code-bridge use. User journey: admin clicks "Bind to Lark" on the Agent detail page → QR dialog opens → admin scans in the Lark app on their phone → authorizes the new PersonalAgent → dialog auto-closes with the new installation visible. No app_id / app_secret to copy, no Lark developer console visit, no Multica-side OAuth env to configure. Backend (server/internal/integrations/lark): - registration.go — inline ~280-line RFC 8628 client. Begin posts archetype=PersonalAgent / auth_method=client_secret / request_user_info=open_id; Poll follows the upstream SDK's state machine including the tenant-brand mid-stream domain swap to accounts.larksuite.com when a Lark-international account authorizes. SDK is NOT vendored — one endpoint isn't worth dragging the full oapi-sdk-go + transitive deps. - registration_service.go — owns the in-process session store + background polling goroutine. On success calls APIClient.GetBotInfo (the new IM-side endpoint added below) and writes lark_installation + the installer's lark_user_binding inside one DB transaction so a half-applied install can never land. Stable error_reason codes (expired / access_denied / lark_protocol_error / bot_info_failed / installation_conflict / installer_bind_failed / internal_error) drive the UI copy without parsing prose. - client.go / http_client.go — drops ExchangeOAuthCode and SupportsOAuthInstall (no longer applicable: device-flow returns identity alongside credentials in one response); adds GetBotInfo which mints a tenant_access_token from the freshly-minted client_id / client_secret and calls /open-apis/bot/v3/info for the bot_open_id. install_supported now gates on IsConfigured() (real HTTP client wired) instead of a separate OAuth capability. - binding_token.go — absorbs InstallerBindParams / InstallerBinder (previously in oauth.go), retargets the doc-comment from the OAuth caller to the device-flow caller. - Deletes oauth.go + oauth_test.go entirely. Handler & router (server/internal/handler, server/cmd/server): - POST /api/workspaces/{id}/lark/install/begin — opens a new registration session, returns {session_id, qr_code_url, expires_in_seconds, poll_interval_seconds}. Admin-only. - GET /api/workspaces/{id}/lark/install/{sessionId}/status — polling endpoint, returns {status, installation_id?, error_reason?, error_message?}. Workspace-scoped lookup so a stolen session_id cannot be polled from another workspace. Admin-only. - Removes POST /lark/installations (paste form), GET /lark/install/start (OAuth-redirect entry), and GET /api/lark/install/callback (OAuth redirect target). - Removes MULTICA_LARK_OAUTH_APP_ID / _APP_SECRET / _REDIRECT_URI / _STATE_SECRET / _AUTHORIZE_URL / _SUCCESS_URL env vars. Self-host operators no longer need a parent Lark app at all. Frontend (packages/core, packages/views): - New types BeginLarkInstallResponse / LarkInstallStatusResponse + matching API methods (beginLarkInstall / getLarkInstallStatus); drops getLarkInstallURL. - LarkAgentBindButton opens LarkInstallDialog instead of a window.open() to Lark's authorize page. The dialog uses react-qr-code (catalog) to render the verification_uri_complete inline as SVG (no external CDN image), polls status at the server-supplied cadence, auto-closes on success, offers "scan again" on terminal failure. Per CLAUDE.md "Enum drift downgrades, not crashes", error_reason switch has a default fallback so an older desktop build on a newer server still renders the generic failure copy. - Adds the device-flow strings to en + zh-Hans settings.json; removes the obsolete OAuth-not-configured copy. Verified locally: - go build ./... / go vet ./... clean - go test ./internal/integrations/lark/... — all green (existing tests + 15 new registration / GetBotInfo tests) - go test ./internal/handler -run 'Lark|Binding' — all green - pnpm typecheck — all 6 packages clean - pnpm lint — 0 errors (15 pre-existing warnings, none in changed files) - pnpm --filter @multica/views test — 859/859 pass Pre-existing failures in server/internal/middleware (column "profile_description" missing from local test DB) reproduce against the parent commit and are unrelated to this change. Co-authored-by: multica-agent <[email protected]> * fix(integrations/lark): gate bind CTA to workspace admins, terminate QR polling on 4xx (MUL-2671) Two frontend must-fixes from the PR #3277 二审: 1. LarkAgentBindButton now self-hides for non-admin viewers in addition to the existing install_supported check. The agent-detail page mounts the button under `canEdit`, which canEditAgent lets agent owners through even when they are not workspace admins — but the backend gates POST /lark/install/begin and the status poll on owner/admin (router.go:478-487), so the previous behavior shipped a CTA that was guaranteed to 403. The new gate reads workspace role from the same member list the settings tab already uses. 2. The status polling loop now terminates on 404 (session gone — server restarted, multi-instance routing, or in-process GC swept it) and 403/401 (permission revoked mid-session). Previously every error path scheduled another setTimeout, which trapped the user on a stale QR forever. ApiError gives us the HTTP status verbatim; terminal responses set status=error with stable error_reason codes (session_lost, forbidden) that flow through the existing dialog switch + retry/close affordances. 5xx + network blips still retry. i18n: new install_error_session_lost / install_error_forbidden in en and zh-Hans, with default fallback preserved per the enum-drift rule. Coverage: 6 new vitest cases — admin/owner allow, member deny, unsupported-install deny, and the two terminal-error polling paths using fake timers to assert the loop stops scheduling. Also clears a handful of stale OAuth/manual-install doc comments flagged in the review (non-blocker cleanup): doc.go's §10 now points at RegistrationService, installation.go's input-shape doc loses the OAuth-callback half, and client.go's stubAPIClient comments no longer reference OAuth callbacks. Co-authored-by: multica-agent <[email protected]> * docs(integrations/lark): describe gate as device-flow install in agent-detail integrations comment (MUL-2671) The comment block above the agent-detail Integrations section still described the capability gate as 'server-side OAuth-install'. The OAuth path is gone — install is now device-flow per RFC 8628 — so the comment now reads 'server-side device-flow install capability gate'. Pure comment change; behavior is unchanged. Cleans up the nit Elon called out in PR #3277 二审 (MUL-2671). Co-authored-by: multica-agent <[email protected]> * feat(integrations/lark): wire inbound pipeline + WS Hub at boot (MUL-2671) Stage 3.a of MUL-2671. Hub class, Dispatcher, ChatSessionService and AuditLogger have all been implemented and tested in prior PRs but none of them was constructed at boot, so the in-process plumbing was never exercised end-to-end. This change wires them together behind the same `MULTICA_LARK_SECRET_KEY` gate that already gates InstallationService / RegistrationService, and starts the Hub under the existing `sweepCtx` so it winds down alongside the other long-running workers after HTTP drain. The real long-conn EventConnector is still pending; the factory hands every supervisor a shared NoopConnector that holds the lease and emits nothing. That lets staging exercise the lease / supervisor / shutdown lifecycle against real DB rows without committing to the Lark wire protocol implementation. Swapping in the real connector is a single line change in the same router block; the Dispatcher / ChatSessionService / Hub seams stay frozen. ## Why a noop placeholder, not a stub-or-skip The Hub's value is mostly its lifecycle: §4.4 ownership lease, LeaseRenewInterval / LeaseTTL, supervisor reap on revoke, clean release on shutdown. None of that runs unless the Hub is actually started. Holding off until the real connector lands means the next PR has to debut both pieces simultaneously; wiring the supervisor loop first lets the real connector PR be a focused, reviewable swap. ## Changes - `internal/integrations/lark/noop_connector.go` — `NoopConnector` implementing `EventConnector`: blocks on ctx until the Hub cancels (lease loss / shutdown / revoke), emits no events, logs on enter/exit so operators see exactly which installation the supervisor is holding the lease for. - `internal/integrations/lark/noop_connector_test.go` — verifies the connector blocks until ctx cancel, returns nil on clean exit, never invokes the emit callback, and the factory shares a single connector instance across installations. - `internal/handler/handler.go` — new `LarkHub *lark.Hub` field on `Handler`. Nil when the Lark integration is disabled. - `cmd/server/router.go` — inside the existing Lark wiring block, construct `AuditLogger`, `ChatSessionService` (with `*pgxpool.Pool` for the in-tx dedup Mark), `Dispatcher` (wiring `h.IssueService` and `h.TaskService` so `/issue`-created issues share counter / duplicate guard / project boundary / broadcast / analytics with the rest of the product), and the `Hub` with the `NoopConnectorFactory`. `NewRouterWithOptions` now returns `(chi.Router, *handler.Handler)` so main.go can drive Hub lifecycle; `NewRouter` discards the handler. - `cmd/server/main.go` — start the Hub under `sweepCtx` after the other background workers, and `Wait` on it after HTTP drain + sweep cancel so the lease renewer can issue a final release before exit. Skipped entirely when `h.LarkHub == nil`. ## Test plan - [x] `go build ./...` clean - [x] `go vet ./...` clean - [x] `go test ./internal/integrations/lark/...` (new noop tests + existing hub / dispatcher / chat_service / registration / binding_token / outbound / issue_command suites) — all pass - [x] `go test ./internal/handler -run 'TestLark|TestRedeemLarkBinding'` pass — handler-side Lark surfaces unchanged - [x] `go test ./internal/service/... ./internal/util/secretbox/...` pass - [x] `pnpm --filter @multica/views exec vitest run settings/components/lark-tab` pass (6/6) — frontend lark surfaces unchanged - [ ] Local broad `go test ./internal/handler/...` still blocked by the pre-existing test DB schema drift Elon flagged in the previous round (`column "metadata" does not exist`, unrelated to this change); CI is the authoritative check. - [ ] Manual end-to-end deferred until the real long-conn EventConnector lands (next stage). MUL-2671 Co-authored-by: multica-agent <[email protected]> * fix(integrations/lark): bound Hub lease release + shutdown wait (MUL-2671) Lease release used context.Background(); a stalled DB pool could pin shutdown indefinitely. Add LeaseReleaseTimeout (5s default) and ShutdownTimeout (15s default) to HubConfig, route releaseLease through a bounded context, and expose WaitWithTimeout for main.go so a wedged supervisor degrades to LeaseTTL expiry on the next replica instead of blocking process exit. Also correct the LarkHub field comment in handler.go: the Hub is wired whenever the at-rest secret key is set, independent of whether the outbound HTTP APIClient is configured. Co-authored-by: multica-agent <[email protected]> * feat(integrations/lark): real WS long-conn connector + ctx-cancel-breaks-read (MUL-2671) Replaces NoopConnectorFactory with a production EventConnector that opens Lark's event-subscription WebSocket. Gated behind MULTICA_LARK_WS_ENABLED so staging boots stay on the noop path until operators opt in, and falls back to noop with a warning when the WS flag is set without MULTICA_LARK_HTTP_ENABLED (the real connector needs the cached tenant_access_token). Why this connector exists separately from the Hub: gorilla/websocket ReadMessage blocks on the underlying TCP socket and does not observe context. The watchdog goroutine inside WSLongConnConnector.Run closes the conn the moment ctx fires, so lease loss / shutdown breaks the blocking read in bounded time — exactly the invariant Hub renewLeaseUntil's runCancel depends on for the "at most one active WS per installation across replicas" guarantee. Tests cover this explicitly (TestWSConnectorRunReturnsOnCtxCancelEvenWhenReadIsBlocked). The Lark wire surface is split into three swappable seams so the transport layer stays tested in isolation: - EndpointFetcher (POST /event-subscription/v1/connection_token) resolves a one-shot wss URL per Run. No caching — replaying a one-shot token would look like a Lark outage. - FrameDecoder turns one raw JSON envelope into an InboundMessage or a "control / heartbeat / drop" verdict. Decoder errors log + drop the frame; they do NOT tear down the connection. - CredentialsProvider wraps InstallationService.DecryptAppSecret so plaintext app_secret lives in memory only during a Run. Also fixes the handler.go LarkHub comment: it still said "joins on Wait during graceful shutdown" but main.go has used WaitWithTimeout (bounded wait) for several commits. Comment now matches. Co-authored-by: multica-agent <[email protected]> * feat(integrations/lark): align WS to official binary Frame protocol + DispatchResult outbound replies (MUL-2671) Two must-fix items from Elon's review of PR #3277: 1. WS protocol layer rewritten to match the official Lark Go SDK (`larksuite/oapi-sdk-go/v3/ws`): - Bootstrap is `POST /callback/ws/endpoint` with AppID/AppSecret in the body (no tenant_access_token bearer). Response carries wss URL + ClientConfig (PingInterval / ReconnectInterval / ReconnectNonce / ReconnectCount). - `service_id` is parsed from the wss URL query and used as Frame.Service on every outbound frame. - Wire envelope is the binary protobuf `pbbp2.Frame` (hand-rolled via protowire to avoid pulling the whole SDK in, byte-identical field tags). JSON payloads are nested inside Frame.Payload. - Inbound data frames are ACKed with a `Response{code:200,...}` JSON payload that reuses the inbound headers; infra failures produce code=500 so Lark retries. - Ping is the app-layer binary `NewPingFrame(serviceID)` at the server-supplied cadence; WebSocket protocol PING is removed (Lark ignores it). Server-initiated pings get a pong reply. - ctx-cancel-breaks-read invariant preserved via the watchdog goroutine that closes the conn on ctx.Done; the read loop and ping goroutine serialize their writes through a single mutex. 2. `DispatchResult` outbound replies wired via a new `OutcomeReplier`: - `OutcomeNeedsBinding` mints a one-shot binding token and sends the binding prompt card to the sender's open_id. - `OutcomeAgentOffline` / `OutcomeAgentArchived` push a notice card into the chat with the agent name + Chinese copy matching §4.6. - `OutcomeIngested` stays owned by the Patcher; `OutcomeDropped` is silent. - The replier is best-effort: outbound failures are logged and swallowed so a Lark outage cannot stall the inbound pipeline. - Hub installs the noop replier by default; router wires the production `LarkOutcomeReplier` when APIClient.IsConfigured(). PersonalAgent long-conn risk surfaced (open per Feishu docs: `长连接模式仅支持企业自建应用`). The implementation works for any app archetype; the open question is whether `/callback/ws/endpoint` accepts PersonalAgent credentials in practice. Surfacing the Lark code+msg verbatim from the bootstrap response so an operator running the smoke test sees the exact failure rather than a generic timeout. Co-authored-by: multica-agent <[email protected]> * fix(integrations/lark): byte-compat Frame marshal, chunk reassembly, ACK off reply critical path (MUL-2671) Three protocol blockers from Elon's review of 9540008a: 1. Frame.Marshal is now byte-identical to oapi-sdk-go/v3/ws/pbbp2.Frame: - SeqID/LogID/Service/Method (proto2 req) emit unconditionally even at zero - PayloadEncoding/PayloadType/LogIDNew emit unconditionally per gogo generated MarshalToSizedBuffer (no zero-guard) - Payload uses the SDK's `!= nil` guard (nil omits, []byte{} emits 0-length) - ACK payload JSON matches SDK's NewResponseByCode + json.Marshal output ({"code":N,"headers":null,"data":null}) Golden tests pin exact byte sequences for ping/pong/ACK/full/zero frames; verified against the real SDK pbbp2.pb.go MarshalToSizedBuffer producing identical bytes. 2. Multi-frame events (sum>1) are reassembled via the new chunkAssembler: - 5s sliding TTL (matches SDK combine() cache TTL) - Lazy GC on admit (no separate sweeper goroutine) - Out-of-order seq + duplicate seq idempotent - Partial chunks are NOT ACKed (SDK behaviour: only the final chunk's ACK confirms the whole event so Lark can retry on partial loss) - Connector wires assembler per-Run; state dies with the session 3. OutcomeReplier detached from ACK critical path: - HubConfig.ReplyTimeout default 2.5s, strictly under Lark's 3s ACK deadline - handleEvent dispatches synchronously (fast DB path), then spawns the replier under a fresh background ctx with WithTimeout(ReplyTimeout) - Hub.replyWg tracks in-flight replies; Hub.Wait / WaitWithTimeout drain them so shutdown is bounded - Noop replier short-circuits inline (no goroutine cost when outbound APIClient isn't configured) Proof tests: - TestHubScheduleReplyReturnsImmediately: scheduleReply with a 10s slow replier returns in <50ms - TestHubReplyTimeoutCancelsHungReplier: hung replier ctx fires at ReplyTimeout - TestHubWaitDrainsInFlightReplies: Wait blocks until replies finish - TestHubACKNotBlockedByOutboundReply: end-to-end through the connector — data-frame ACK lands within 500ms even when the replier hangs 5s PersonalAgent real-env smoke remains Bohan's decision; this PR closes the technical blockers Elon flagged. Co-authored-by: multica-agent <[email protected]> * docs(service/issue): narrow position concurrency claim to create-create (MUL-2671) Elon's review of the merge resolution flagged that the comment on the new NextTopPosition call promised more than the code guarantees: concurrent manual reorder via UpdateIssue(position) does NOT take the workspace row lock that IncrementIssueCounter holds, so a create racing a reorder can still land on the same position. Rewrite the comment to only claim create-create serialization, which is the behaviour the lock actually delivers. No code change. Co-authored-by: multica-agent <[email protected]> * fix(integrations/lark): keep device-flow polling on RFC 8628 HTTP 400 (MUL-2671) Lark's device-flow polling endpoint returns HTTP 400 with the JSON body `{"error":"authorization_pending"}` while the user hasn't scanned the QR yet — this is the RFC 8628 spec, and the upstream oapi-sdk-go implements the same handling. Our previous doForm treated ANY non-2xx as a terminal protocol error, so every install session was killed by the first poll (~5s after begin) and the install dialog appeared silently empty: the frontend received status=error + lark_protocol_error before the user could even read the description. Fix: doForm now decodes the JSON body first; if it parses, the caller (Begin / Poll) routes on the body's `error` field, where the existing switch correctly maps authorization_pending / slow_down to "keep polling" and access_denied / expired_token to terminal failure. Only unparseable bodies (5xx HTML proxy pages, gateway timeouts) still surface as a typed http_NNN RegistrationError. Three regression tests pin the new behaviour: - HTTP 400 + authorization_pending → res.Status="authorization_pending" - HTTP 400 + access_denied → res.Err.Code="access_denied" (terminal) - HTTP 502 + HTML body → http_502 RegistrationError Verified against the live local env: install/begin -> 200, status stays "pending" through the first poll cycle, no longer flips to "error" within seconds. Co-authored-by: multica-agent <[email protected]> * fix(views/lark): reset closedRef on every mount so StrictMode double-mount renders QR (MUL-2671) Empty QR dialog body in the dev env: Bohan opened the bind dialog and got an empty white area where the QR should have been — no QR, no "starting" placeholder, no error text. Backend was returning the QR URL correctly; the bug was on the frontend. Root cause: React 19 / Next.js dev StrictMode mounts every component twice (mount → cleanup → mount). The component instance is REUSED across the simulated remount, which means useRef objects are preserved. The dialog's `closedRef` lifecycle: 1. Mount #1: closedRef={current:false}, beginSession() kicked off (HTTP request still in flight) 2. Cleanup runs: closedRef.current=true 3. Mount #2: beginSession() kicked off again, BUT the ref still reads {current:true} from step 2 4. Both promises resolve. Both hit the post-await guard `if (closedRef.current) return;` and bail out before setSession(). 5. Result: session stays null forever. Every conditional in the dialog body (beginning/session-pending/success/error) is false → empty body. Fix: reset closedRef.current=false at the START of the effect, not just at component construction. The cleanup-then-mount pair now re-arms the guard so subsequent setSession calls actually land. Regression test wraps the dialog in <StrictMode> and asserts the QR appears within 2s with the correct value — fails closed if anyone removes the reset. Co-authored-by: multica-agent <[email protected]> * fix(integrations/lark): drop EventTaskCompleted subscription so the chat reply doesn't get overwritten by "Done." (MUL-2671) Bohan reproduced on the live dev env: agent replies show only a card saying "Done." in Lark, even though Multica's own chat panel has the real "Hello! I'm cc…" reply. Tasks succeed end-to-end, but the user loses the reply on the Lark side. Root cause: TaskService.CompleteTask publishes two events for every chat task IN ORDER: 1. broadcastChatDone(...) → ChatDonePayload{Content: "Hello!..."} 2. broadcastTaskEvent(Completed) → map[string]any{task_id, agent_id,...} (no `content` key) The Patcher subscribed to BOTH and routed each to finalize(). The first patch correctly rendered the reply text, the second patched the same card with an empty payload — chatDoneContent() returned "" and the renderer fell back to "Done." (default empty-body copy). The second patch wins because Lark stores whatever was last applied. Fix: stop subscribing to EventTaskCompleted in the Patcher and remove the corresponding switch arm. EventChatDone is the canonical "agent finished replying" signal for the Lark card path; EventTaskCompleted is still emitted to the bus for other listeners (web UI, analytics, task usage) where the lack of content doesn't matter. Regression test TestPatcherIgnoresEventTaskCompletedForChatTasks emits ChatDone followed by TaskCompleted on a streaming card and asserts: exactly one patch, body contains the agent reply, body does NOT contain "Done.". If anyone re-adds the EventTaskCompleted subscription, this fails immediately. Co-authored-by: multica-agent <[email protected]> * feat(integrations/lark): chat replies as plain text IM messages, not card chrome (MUL-2671) Bohan reported on the live dev env that even with the agent's reply shown correctly, every message is wrapped in an interactive card with the agent name as the header — it feels like a system notification, not a normal chat reply. He wants the reply to land as a regular Lark text bubble. Changes: - Add APIClient.SendTextMessage backed by Lark's /open-apis/im/v1/messages with msg_type=text. JSON-encodes the {"text": ...} envelope Lark requires so callers pass raw strings. - Patcher.Register no longer subscribes to EventTaskQueued / EventTaskRunning. There is no more thinking → running → final card lifecycle on the success path: it added card chrome without buying anything for free-form chat. - On EventChatDone, the new sendChatReply path posts the assistant message content as plain text. Empty content is silently dropped rather than rendered as "Done." (the prior fallback that confused Bohan). - Failure path keeps a one-shot error card on EventTaskFailed — the visual distinction from a normal reply is genuinely useful, and failures are rare enough that the chrome isn't noisy. - Throttle / lastPatched map / MinPatchInterval / shouldPatch / markPatched / loadCardOrSkip are all removed; nothing in the new flow patches. Tests: - TestPatcherSendsPlainTextOnChatDone pins the new contract: exactly one SendTextMessage call, no card sends or patches, content matches the ChatDonePayload. - TestPatcherDropsEmptyChatReply pins the "no more Done. fallback" decision — empty content drops, period. - TestPatcherFailEventSendsErrorCard pins the failure path still uses a card (one-shot, no patching). - TestPatcherIgnoresEventTaskCompletedForChatTasks rewritten for text path: ChatDone then TaskCompleted yields exactly one text send, no duplicate. - TestPatcherSkipsWhenNoChatSessionBinding and TestPatcherSwallowsInstallationLoadErrors rewritten to drive EventChatDone (the new entry point) instead of TaskQueued. - TestPatcherSendsThinkingCardOnTaskQueued deleted (no more thinking card). Co-authored-by: multica-agent <[email protected]> * feat(integrations/lark): pre-fill PersonalAgent bot name as "<agent> - Multica" (MUL-2823) (#3520) The device-flow install left the bot at Lark's auto-generated "{用户姓名}的智能助手". Lark's registration scene supports pre-filling the name via a `name` query param on the verification/QR URL (mirrors the upstream SDK's AppPreset.Name) — a user-editable default that rides on the QR URL, not the begin POST body (which has no name field). BeginInstall already loads the agent for its ownership check, so we keep it and thread `<agent.Name> - Multica` through Begin → decorateQRCodeURL. A blank name degrades to plain "Multica". There is no post-install rename API (bot/v3 is read-only; no bot/v3/update), so the install-time pre-fill is the only programmatic lever; the user can still edit the name on the creation form. Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(integrations/lark): restore /issue confirmation + pin SendTextMessage wire (MUL-2671) Two recovered/added contracts off Trump's review of HEAD fe381a07: 1) /issue confirmation in Lark was a casualty of the plain-text refactor. The pre-refactor `RenderInput.IssueNumber` field was declared but never actually rendered into the card body, so even in the original card-based flow the user never saw a "Created [MUL-42]" confirmation. Now the OutcomeReplier handles OutcomeIngested + IssueID.Valid by sending a plain text message: Created MUL-42 — fix login bug https://multica.example/issues/MUL-42 Composed from a new DispatchResult.IssueIdentifier + IssueTitle, populated by the Dispatcher from workspace.IssuePrefix + issue.Number / issue.Title. Workspace lookup is best-effort: a Postgres blip on workspace gets a "#42" fallback rather than silently dropping the confirmation. The agent's own chat reply (if any) continues to land separately via the Patcher on EventChatDone — these are two semantically distinct messages and the user benefits from seeing both. 2) SendTextMessage is the wire layer Trump flagged for missing coverage. Three new wire tests pin: - happy path: POST /open-apis/im/v1/messages?receive_id_type=chat_id, msg_type=text, Bearer <tenant_access_token>, double-JSON content envelope - special-character round trip: newlines, double quotes, backslashes, tabs, Chinese + emoji, JSON-lookalike strings. The inner {"text": ...} is encoded once at JSON.Marshal time and once again when the outer body serializes; losing either pass corrupts the message and the bug is invisible without a contract pin. - Lark error path: non-zero `code` surfaces as a wrapped error with the code embedded. Tests: - TestDispatcher_IssueCreationFromCommand asserts IssueIdentifier ("MUL-42") and IssueTitle propagate through DispatchResult. - TestDispatcher_IssueIdentifierFallsBackToNumberOnWorkspaceLookupErr pins the "#7" degrade-graceful fallback. - TestLarkOutcomeReplierIssueCreatedSendsConfirmation pins the text body (identifier + title + deep link) and asserts no card send on this path. - TestLarkOutcomeReplierOutcomeIngestedSilentWithoutIssue pins the silent-on-plain-chat default so we don't accidentally start emitting a confirmation for every message. - TestHTTPClient_SendTextMessage_* covers the wire contract. Frontend locale parity (en + zh-Hans, 53 tests) is currently green on this HEAD; no changes needed. Co-authored-by: multica-agent <[email protected]> * fix(views/locales): add missing ko keys for Lark MVP (MUL-2671) Trump flagged on PR #3277 review that the ko bundle was missing the Lark-MVP-only keys that en + zh-Hans both carry. The parity test caught it cleanly after main was merged in (Korean PR landed on main between the prior review and this one): common.lark_bind.* (13 keys) settings.page.tabs.lark (1 key) settings.lark.* (45 keys) agents.inspector.section_integrations (1 key) Korean translations are professional/concise — "Lark" stays as the brand name (matches how en keeps "Lark" + "(飞书)" parenthetically; ko/users searching for the product expect "Lark"), and product copy follows the zh-Hans tone where Multica nouns ("에이전트", "워크스페이스") are romanized loan words consistent with the rest of the ko bundle. Slot ordering preserved against EN: - page.tabs.lark sits between github and integrations - inspector.section_integrations sits right after section_skills Verified: pnpm exec vitest run locales/parity → 105/105 pass. Co-authored-by: multica-agent <[email protected]> * fix(integrations/lark): /issue origin_type CHECK + Hub restart on credentials rotation (MUL-2671) Two live-env bugs Bohan reproduced: 1) /issue command crashed the WS connector. Dispatcher writes origin_type='lark_chat' on issues born from `/issue`, but the issue_origin_type_check CHECK constraint was last extended in migration 060 for quick_create — it doesn't list lark_chat, so every Lark /issue tripped SQLSTATE 23514 and bubbled up as an infra error. The infra error tore down the WS connector, Lark retried the same message, the new connector tripped the same constraint and crashed again. Repro in the live env: three crashes from the same /issue event over ~40s, each leaving the user with no confirmation in Lark. Migration 111 extends the CHECK list: CHECK (origin_type IN ('autopilot', 'quick_create', 'lark_chat')) 2) Re-scanning an already-bound agent silenced the bot. The device flow re-registers with Lark, which mints a brand-new bot (fresh app_id + app_secret); RegistrationService.finishSuccess upserts into lark_installation by agent_id, so the row's credentials rotate in place. But the running supervisor held the OLD inst struct by value and kept a WS open against the OLD bot's app_id — so all events to the NEW bot went nowhere. Bohan's "claude code 现在不能在飞书里回复了" symptom maps exactly to this: log timeline: 16:29:57 cc connector connected with app_id=cli_aa9398dd... (OLD) 16:34:07 lark registration: install complete (rotation) → row.app_id is now cli_aa93f36f... (NEW) → old WS still subscribed to OLD app_id; new app_id receives nothing Fix: Hub.sweep now compares each installation row's credentials fingerprint (app_id + bot_open_id + sha256(app_secret_encrypted)) against the snapshot the running supervisor was started with. On diff, cancel the old supervisor and start a fresh one inline. A monotonic gen counter on the supervisor entry disambiguates the old goroutine's deferred cleanup from the new entry the rotation path already swapped in. Tests: - TestHubRestartsSupervisorOnCredentialsRotation pins the new path: starts hub on app_one, rotates the row to app_two, asserts the connector factory is called again with the fresh AppID. - TestHubDoesNotRestartSupervisorOnUnchangedRow pins the negative case so an unchanged row doesn't degenerate into a per-sweep busy-loop. - Existing hub tests (lease, supervise, shutdown, ACK timing, noop replier) all green. Verification: - go test ./internal/integrations/lark/... -race -count=1 ok - go build ./... clean - migration applied locally; \d+ issue confirms lark_chat in CHECK Co-authored-by: multica-agent <[email protected]> * fix(integrations/lark): per-supervisor lease token to fence rotation handoff (MUL-2671) Elon flagged a race in HEAD be8d4cef's rotation path: both the old and the new supervisors of the same Hub used the hub-wide nodeID as their WS lease token, so an old supervisor's post-cancel releaseLease(nodeID) would CAS-match the lease row the successor had just acquired with the SAME token and DELETE it. Symptom would be a silently empty lease row a few hundred ms after every device-flow re-scan — no replica owning the install, no events delivered, the "bot goes quiet" pattern Bohan hit the first time but now from the fencing side rather than the credentials side. Fix: leaseToken(nodeID, gen) composes "<nodeID>-g<gen>", where gen is the monotonic counter already attached to each supervisorEntry. The nodeID prefix keeps cross-replica observability (an operator inspecting lark_installation.ws_lease_token can still map back to a process) while the -g suffix makes the OLD supervisor's release target the OLD row state. Once the rotation path swaps in the new supervisor, the row's CurrentToken is the new -g(N+1) token, so the old -gN release's WHERE clause no-ops instead of clobbering. acquireLease / renewLeaseUntil / releaseLease now take an explicit token argument; supervise threads its leaseToken through. The plumbing isn't pretty, but having an explicit argument at every call site is the only way the rotation invariant survives subsequent refactors — without it, a future caller could quietly reintroduce "just use h.nodeID" and the race is back. Two regression tests: - TestHubRotationStaleReleaseDoesNotClearSuccessorLease drives the fake lease state machine directly: 1. old acquires(tokenA) 2. rotation lands; new acquires(tokenB) 3. old's stale release(tokenA) fires Asserts owner ends up still tokenB. Hub-wide-nodeID code would fail step 3 by clearing the entry. - TestHubRotationEndToEndKeepsSuccessorLeased runs the same scenario through the live supervise loop: starts hub, rotates the row, waits for sup2 to take over with a distinct token, sleeps past sup1's unwind, asserts the row is still held by a non-sup1 token. Catches the bug even when the goroutine timing is non-deterministic. Verification: go test ./internal/integrations/lark/... -race -count=1 ok go build ./... clean go vet ./... clean Co-authored-by: multica-agent <[email protected]> * fix(integrations/lark): route group @-mentions via union_id, not open_id (MUL-2671) In a Lark group with multiple Multica bots installed, the bot whose WS received the event sometimes failed to recognize that it was the @-target while the OTHER bot's supervisor falsely fired. Bohan's controlled three- message test (only @A, only @B, @both) hit this: @A and @B alone went unanswered, @both got picked up by A only. Root cause: the `mentions[].id.open_id` field Lark puts on the WS event is structurally INVERSE to `/bot/v3/info`'s `bot.open_id` across the two WSes. From A's WS perspective, the wire-form open_id for "A was @-ed" is NOT equal to A's API-side open_id, but IS equal to what B's WS sees on its side, and vice versa. The decoder's `mention.open_id == inst.BotOpenID` match therefore fires on the wrong bot in multi-bot groups. Only `union_id` (the Lark-tenant-scoped stable identifier) is consistent across both WSes. Changes: - migration 112 adds nullable `lark_installation.bot_union_id` - sqlc query exposes UpsertLarkInstallation/CreateLarkInstallation with bot_union_id, plus a focused SetLarkInstallationBotUnionID for the backfill path - httpAPIClient.GetBotInfo now follows /bot/v3/info with /contact/v3/ users/{open_id}?user_id_type=open_id and returns both identifiers on BotInfo. Soft-fails on contact-scope denial: install still succeeds with an empty UnionID, and the decoder falls back to the legacy open_id match for single-bot deployments. - RegistrationService.finishSuccess persists union_id alongside open_id during the device-flow finalize. - ws_frame_decoder.containsMention prefers union_id and only walks open_id when the installation row has not been backfilled yet. - BackfillBotUnionIDs runs once at server boot for installations created before migration 112; bounded per-row 10s timeout and a pure soft-fail policy so a slow Lark round-trip cannot block startup. - regression tests cover the three decoder paths: union_id match wins over open_id mismatch, union_id mismatch overrides open_id match, and open_id fallback when union_id is unknown. Co-authored-by: multica-agent <[email protected]> * chore: drop trailing blank lines at EOF on four files (MUL-2671) git diff --check origin/main..origin/pr-3277 flagged these as new blank lines at EOF; clearing so the diff stays clean for review. Co-authored-by: multica-agent <[email protected]> * fix(views/locales): add missing ja keys for Lark MVP + section_integrations (MUL-2671) CI frontend job tripped on the ja locale parity check: ja is missing the lark_bind block in common.json, the lark block + page.tabs.lark in settings.json, and inspector.section_integrations in agents.json. The ko fix earlier covered Korean; ja was added separately on main and the merge surfaced these gaps. Translations mirror the en source and follow the same voice as the existing ja bundle. Co-authored-by: multica-agent <[email protected]> * fix(integrations/lark): rewrite @_user_N placeholders into clean body (MUL-2671) When Lark dispatches a group `im.message.receive_v1`, the message text contains opaque `@_user_1`, `@_user_2`, … placeholders and the real identity is in `mentions[]`. We were forwarding the raw text to the agent, so a Bohan-typed "@Bot ping test" arrived as "@_user_1 ping test" — neither human-readable nor useful as LLM context, and the agent was paying tokens to figure out which `@_user_N` was even itself. The new resolveMentions pass: * strips the bot's own mention entirely (the dispatcher already routes the event on AddressedToBot; re-emitting @<self> in front of every message adds zero signal and pollutes context), * substitutes other participants with `@<displayName>` so a follow-up "@Alice" reads naturally, * collapses horizontal whitespace introduced by the strip while preserving original newlines. Bot identity check uses the same union_id-preferred + open_id fallback as containsMention, so the rewrite stays consistent with the routing path. Tests cover the four shapes: bot self-mention, mixed bot + other-user mention, multi-line body with stripped mention, and a no-mention body that should be left untouched. Co-authored-by: multica-agent <[email protected]> * fix(integrations/lark): union_id-first self mention strip + token-aware scan + local whitespace cleanup (MUL-2671) Three review blockers on the mention rewrite from PR review: 1. isBotMention now mirrors containsMention's union_id-first policy. When the installation row knows our union_id, we trust it exclusively (open_id is structurally inverted in multi-bot groups — matching on it would re-introduce the routing bug we fixed two commits ago). open_id fallback fires only when union_id is absent. New tests: @-ing both bots in one message correctly strips only self and renders the sibling as @<name>; open_id-matches-but-union_id-differs does NOT strip. 2. resolveMentions no longer collapses or trims whitespace globally. Indentation, tabs, code blocks, tables — all preserved verbatim. When the self mention is removed we eat exactly one adjacent horizontal space (the one after the placeholder, or, when the mention sits at end-of-input, a single space already emitted right before it). New test exercises a multi-line indented + tabbed body and asserts the whole shape survives. 3. Prefix-collision-safe replacement. A chat with 11+ participants exposes both `@_user_1` and `@_user_10`; naive ReplaceAll for `@_user_1` would mangle the substring of `@_user_10`. The resolver now does a single-pass token scan with the mention list sorted longest-key-first, so the longer placeholder always wins at any scan position. New test covers the @_user_1 / @_user_10 case explicitly. Also drops the temporary INFO-level diag logging the previous commit added — root cause was confirmed (union_id swap in the manual backfill; not a decoder bug). Co-authored-by: multica-agent <[email protected]> * fix(integrations/lark): scope inbound dedup per (installation_id, message_id) (MUL-2671) Root cause of the residual "@Cc gets dropped as not_addressed_in_group" even after the union_id swap landed: lark_inbound_message_dedup was keyed on `message_id` alone. In a Lark group chat where the workspace has multiple Multica bots installed, Lark delivers the SAME message_id to every bot's WS supervisor. Whichever WS claimed first then ran its own AddressedToBot check; the bot that was actually @-ed lost the dedup race, found the row already terminal (`processed_at IS NOT NULL`), and was dropped as `duplicate` BEFORE it could evaluate its own mention. Net: every @ silently disappeared if Lark happened to route the OTHER bot's WS first. The dedup gate's original purpose (idempotency against WS reconnect replay) is per-installation by definition, so the right key is composite (installation_id, message_id). Changes: - migration 113 drops + recreates lark_inbound_message_dedup with installation_id NOT NULL REFERENCES lark_installation(id) ON DELETE CASCADE and PRIMARY KEY (installation_id, message_id). The table is a 24h transient cache, so dropping existing rows is safe. - sqlc queries: ClaimLarkInboundDedup / MarkLarkInboundDedupProcessed / ReleaseLarkInboundDedup all now take installation_id. - AppendUserMessageParams carries InstallationID through to the in-tx Mark call so the chat_message+dedup atomicity stays intact. - Dispatcher passes inst.ID to claim + applyFinalize + AppendUserMessage. - Test fakes key dedup state on (installation_id, message_id) via a composite map key; all existing pre-seeded rows use a seedDedupKey helper bound to the default activeInstallation fixture so the prior staleness / token-rotation / in-tx mark tests still exercise the same regression they did before. - New regression TestDispatcher_DedupIsScopedPerInstallation pins the multi-bot invariant: a row pre-seeded for installation A does NOT block installation B's first delivery of the same message_id; B runs through its own group-filter / identity / ingest pipeline. Co-authored-by: multica-agent <[email protected]> * feat(integrations/lark): render markdown chat replies via schema-2.0 card (MUL-2671) The agent's chat replies were going out as msg_type=text, so every `**bold**`, fenced code block, list, table, and link in the body showed up as literal markdown characters in Lark — the user saw raw asterisks, hashes, pipes instead of formatted text. Bohan reported this and pointed at zarazhangrui/lark-coding-agent-bridge as the shape to emulate. The bridge repo uses Lark interactive cards with the schema-2.0 envelope and a `tag: "markdown"` body element; Lark's client renders that to formatted text (GFM-ish: bold/italic, headings, lists, links, fenced code blocks, tables, blockquotes). They expose multiple reply modes (card / markdown-as-post / text) gated by user config; we go a step simpler — auto-detect markdown syntax in the agent's body and route accordingly: - containsMarkdown(): cheap substring + regex pass for fenced code blocks, headings, list markers, bold/italic, tables, links, blockquotes, horizontal rules, inline code. Biases toward false- positive — wrapping prose in a card still renders fine, but missing a real markdown block leaves raw characters visible. - APIClient gains SendMarkdownCard / SendMarkdownCardParams. Implementation marshals the schema-2.0 envelope verbatim: {schema:"2.0", body:{elements:[{tag:"markdown", content: md}]}}. Stub returns ErrAPIClientNotConfigured. - Patcher.sendChatReply now branches on containsMarkdown: markdown → SendMarkdownCard, plain prose → SendTextMessage. A one-liner "sure, on it" stays as a normal IM bubble (no card chrome); anything with markdown gets the rendered card. Tests: TestContainsMarkdown pins the heuristic across plain prose and ten markdown shapes; TestPatcherRoutesMarkdownReplyToCard and TestPatcherRoutesPlainReplyToText cover the router; new HTTP wire test TestHTTPClient_SendMarkdownCard_HappyPath contract-pins the card envelope (msg_type=interactive, schema 2.0, markdown tag, verbatim body). Full lark suite passes. Co-authored-by: multica-agent <[email protected]> * fix(service/issue): route analytics.IssueCreated through obsmetrics.RecordEvent (MUL-2671) CI's TestNoNakedAnalyticsCaptureInHandlersOrServices guard caught the post-merge analytics call in IssueService.captureCreatedAnalytics that still used s.Analytics.Capture(...) directly. Main added that lint to prevent the Prometheus and PostHog sides from drifting — any new analytics.* event must go through obsmetrics.RecordEvent so the business-metrics collector and the PostHog client fire from the same call site. Fix mirrors how TaskService handles it: IssueService gains a Metrics *obsmetrics.BusinessMetrics field (router wires it via h.IssueService.Metrics = opts.BusinessMetrics next to the existing TaskService line), and the in-service Capture call becomes obsmetrics.RecordEvent(s.Analytics, s.Metrics, ...). nil-safe by construction — RecordEvent treats a nil Metrics as PostHog-only. Co-authored-by: multica-agent <[email protected]> * feat(views/lark): swap Bind CTA for Connected+Manage link when agent already has an installation (MUL-2671) Bohan reported the agent-detail Bind button keeps inviting the user to re-scan the QR even when the agent already has an active Lark PersonalAgent connected — and re-scanning silently upserts the installation row, leaving the previously-created Lark bot dangling as a zombie. Frustrating UX and an actual product footgun. Anti-zombie guard at the only entry point: LarkAgentBindButton now checks the cached installations listing for an active row pinned to this agent_id. When one exists, the install CTA is gone — replaced by a small Connected pill + an "Manage in Lark" link that opens the Bot's app page in Lark's developer console (open.feishu.cn/app/<app_id>) in a new tab. That's where scopes, display name, and additional permission requests actually live; re-scanning never was the right answer for managing an existing bot. Scoping is per-agent: an active installation on a DIFFERENT agent in the same workspace doesn't affect this agent's button, and a revoked installation falls back to the bind CTA so the user can re-create. Tests cover all four states (own-active / own-revoked / other-agent-active / no-installation) and pin the Manage link's href + target=_blank + noopener. i18n: three new keys in settings.json (en / zh-Hans / ja / ko): agent_bot_connected_label, agent_bot_manage_link, agent_bot_manage_tooltip. Locale parity test still 157/157. The dev console host is hardcoded to open.feishu.cn — operators on the Lark international tenant currently get the wrong host; future-proof fix wants the backend to surface a per-installation dev_console_url on the listings response, called out in a code comment. Co-authored-by: multica-agent <[email protected]> * feat(views/settings): collapse Lark into Integrations + render agent identity (MUL-2671) Lark was its own top-level workspace settings tab while Integrations sat empty next to it. As more integrations land, the sidebar would balloon with one tab per provider. Move the Lark surface into Integrations as the first hosted integration; the old ?tab=lark URL redirects through LEGACY_WORKSPACE_TAB_REDIRECTS so bookmarks still resolve. The Connected bots list was leaking the raw Lark app_id (cli_…) as the row title with bot_open_id (ou_…) underneath — meaningless to product users. Since the binding is 1:1 with a Multica Agent, join on agent_id and render the agent's avatar + name via the workspace-standard ActorAvatar + useActorName.getAgentName. Deleted agents fall back to "Unknown Agent" so the row is still actionable for cleanup. Tests: stub useActorName + ActorAvatar in lark-tab.test.tsx and add LarkTab connected-bot tests covering the agent identity render and the deleted-agent fallback. Drop the now-dead integrations.* + page.tabs.lark + lark.bot_open_id_label keys across all four locales — parity still 157/157, views suite 1141/1141. Co-authored-by: multica-agent <[email protected]> * feat(views/settings): wrap Lark in a named section inside Integrations (MUL-2671) Integrations is meant to host multiple providers (Slack, Linear etc. as they land), so the Lark content should sit under a Lark heading rather than fill the tab directly — otherwise the first additional integration would feel like it broke the IA. Add a "Lark" / "飞书" section heading above LarkTab using the same h2 chrome the other settings tabs use, and pin lark.section_title across all four locales (parity 169/169). Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-agent <[email protected]> Co-authored-by: J <[email protected]>	2026-06-03 19:12:14 +08:00
3 people	de900b2ba6	feat(server): funnel/community/commercial business metrics + PostHog pairing (MUL-2949) (#3698) ... * feat(server): funnel/community/commercial business metrics + PostHog pairing (MUL-2949) PR3 of the Grafana board metrics split (parent MUL-2328). Adds 23 new Prometheus counter/histogram families to the PR2 BusinessMetrics collector covering the activation/community/commercial funnels, and binds every PostHog event emission to a matching metric increment so the two sides cannot drift. Funnel: signup, workspace_created, team_invite_sent/accepted, onboarding_*, cloud_waitlist_joined. Content: issue_created, chat_message_sent, agent_created, squad_created, autopilot_created, issue_executed. Runtime: runtime_registered/ready/failed/offline + ready_seconds histogram, daemon_ws_message_received_total. Autopilot: autopilot_run_started/terminal/skipped. Webhook/GitHub: webhook_delivery_total, github_event_received_total, github_pr_review_total, github_pr_merge_seconds histogram. CloudRuntime: cloudruntime_request_total + duration histogram, wired through a small RequestRecorder interface so the cloudruntime package stays decoupled from metrics. Commercial: feedback_submitted, contact_sales_submitted. The pairing helper metrics.RecordEvent(client, m, ev) emits the PostHog event AND increments the matching counter via IncForEvent dispatch, reading labels from the analytics event Properties. Every existing h.Analytics.Capture(analytics.X(...)) call site has been migrated to the helper across handler/, service/, and cmd/server/runtime_sweeper.go. Lint enforcement (server/internal/metrics/business_pairing_test.go): - TestEveryAnalyticsEventHasPrometheusCounter: every Event* constant in analytics/events.go either dispatches via IncForEvent or is in the taskMetricEvents allow-list (PR2 typed RecordTask* methods). - TestNoNakedAnalyticsCaptureInHandlersOrServices: AST-walks handler/ service/cmd-server for direct Analytics.Capture(...) calls — only service/task.go's captureTaskEvent helper is allow-listed. - TestEveryAnalyticsRecordEventTakesAnalyticsHelper: validates the third arg of every metrics.RecordEvent call is built from analytics.*. Cardinality protection: all new label values pass through fixed allow-lists in labels_pr3.go; unknown values collapse to 'other'/'unknown'/'error'. Refs: - Spec MUL-2328 / MUL-2949. - Builds on PR2 (MUL-2948) — collectors registered through the same BusinessMetrics struct, no separate Registry. - Uses PR1's taskfailure.Reason (MUL-2946) for runtime_failed's failure_reason label via NormalizeFailureReason. Out of scope: Sampler-class metrics (PR4 / MUL-2947), pr_review_total emission point (no review event handler exists yet — counter is defined, TODO to wire up when /api/webhooks/github grows pull_request_review handling). Co-authored-by: multica-agent <[email protected]> * fix(server): tighten PR3 review items — signup_source bucket, fill platform/kind/form_source enums, onboarding_started server emission, lint scope (MUL-2949) Addresses 张大彪's review on #3698: 1. signup_source: NormalizeSignupSource added to labels_pr3.go with a fixed allow-list bucket (direct/google/twitter/linkedin/.../other). Parses JSON cookie payload for utm_source/source/referrer fields, strips URL schemes, maps well-known hostnames to channel buckets. PostHog event still ships the raw cookie value for analytics; only the Prometheus label is bucketed. 2. Filled the unknown/other label gaps: - analytics.IssueCreated and analytics.ChatMessageSent now take a platform parameter sourced from middleware.ClientMetadataFromContext (X-Client-Platform header) at the handler. Autopilot-originated issues stamp PlatformServer. - analytics.FeedbackSubmitted now takes a kind parameter; CreateFeedback reads req.Kind (default "general") so the picker selection lights up the metric's kind label instead of long-term "other". - analytics.ContactSalesSubmitted now takes a formSource (page / onboarding / agents_page); CreateContactSales reads req.Source. The metric reads ev.Properties["form_source"] so the analytics CoreProperties.Source ("marketing_contact_sales") stays backward-compat for PostHog dashboards. 3. analytics.OnboardingStarted helper added; server-side emission lives in PatchOnboarding, fired exactly once per user on the first PATCH that carries a non-empty questionnaire payload (firstTouch logic compares prior bytes against {} / null). Frontend onboarding_started keeps firing on page open; the server emission is what guarantees the Prometheus counter exists so Grafana can be cross-checked against the PostHog funnel without depending on the SDK roundtrip. 4. business_pairing_test.go tightened: - TestNoNakedAnalyticsCaptureInHandlersOrServices now allow-lists at function granularity (just captureTaskEvent in service/task.go), not whole-file. Any future naked Capture in the same file fails CI. - TestEveryAnalyticsRecordEventTakesAnalyticsHelper now does def-use tracking inside the enclosing FuncDecl: when RecordEvent's third arg is an *ast.Ident, the test walks the function body for the assignment that defined it and confirms the RHS is an analytics.<Helper>(...) call. Bare local idents that didn't originate from analytics are now caught. 5. gofmt -w applied across the touched files; gofmt -l clean. Tests: go test ./internal/metrics/... ./internal/analytics/... pass. Pre-existing TestClaimTask_/TestWebhook_MergedPR/TestDeleteIssueByIdentifier failures on origin/main are DB-environment-dependent and not regressions from this change. Co-authored-by: multica-agent <[email protected]> * fix(server): normalise onboarding_started platform label + regression test (MUL-2949) Addresses 张大彪's last review nit: - IncForEvent's EventOnboardingStarted case now wraps the platform property with NormalizePlatform, matching every other platform-bearing metric. A misbehaving frontend can no longer leak a raw X-Client-Platform header value into the multica_onboarding_started_total{platform=...} series. - New labels_pr3_test.go covers every PR3 normalizer with both a happy-path value and an unknown value, asserting the unknown collapses to the documented fallback bucket. Includes a focused regression for onboarding_started: emits one event with an attacker-shaped platform string and asserts the metric only exposes web + unknown label values (no raw header bleed). - testutil.go gains a small GatherForTest helper so the regression test can pull the typed MetricFamily map without re-implementing the registry-walk dance. Co-authored-by: multica-agent <[email protected]> * fix(server): NormalizeTaskSource on workspace_created + document lint limitations (MUL-2949) Final review touch-ups before merge: - IncForEvent's EventWorkspaceCreated case wraps source through NormalizeTaskSource, matching the other source-bearing dispatches (issue_created, agent_created, issue_executed). Closes the last raw property leak in the dispatcher table. - business_pairing_test.go inline docstrings now spell out the two known limitations of the lint gate that 张大彪 / Eve flagged: analyticsBackedIdents matches by ident NAME (not SSA def-use, so a nested-scope shadow could pass) and isMetricsRecordEvent hard-codes the import alias set. PR description carries a Follow-ups section with the same two items so the work is visible after merge. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: 魏和尚 <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-06-03 16:39:06 +08:00
LinYushenandmultica-agent	2348301d2b	fix: gate private squad leader bypass (MUL-2860) (#3648) ... * fix: gate private squad leader from being triggered by unauthorized members Add canEnqueueSquadLeader helper that checks canAccessPrivateAgent before allowing a squad leader to be enqueued. Gate all EnqueueTaskForSquadLeader call sites: 1. enqueueSquadLeaderTask (comment trigger, assign trigger, backlog→todo) 2. triggerChildDoneSquad (child-done → parent squad leader) 3. autopilot.go (defensive comment; actor is always agent → always passes) Also fix validateAssigneePair's squad branch to run canAccessPrivateAgent on the squad leader, returning 403 'cannot assign to squad with private leader' when the actor lacks access. Thread actorType/actorID through notifyParentOfChildDone → dispatchParentAssigneeTrigger → triggerChildDoneSquad so the child-done path can enforce the private-leader gate. Regression tests: - Plain member blocked from create-issue to private-leader squad (403) - Plain member blocked from update-issue to private-leader squad (403) - Owner allowed to assign private-leader squad - Plain member comment on squad-assigned issue doesn't trigger private leader - Child-done by plain member doesn't trigger parent's private leader - Agent actor can still trigger private leader via comment Closes MUL-2860 Co-authored-by: multica-agent <[email protected]> * fix: add private-leader gate to autopilot save + dispatch paths - validateAutopilotAssignee squad branch: call canAccessPrivateAgent on the leader, returning 403 for unauthorized members at save time. - service/autopilot.go: add canCreatorAccessPrivateLeader helper that mirrors the handler-level canAccessPrivateAgent logic (agent creators pass; member creators must be owner/admin or agent owner). - Gate both dispatch paths (dispatchCreateIssue and dispatchRunOnly) with fail-closed check: if leader is private and creator lacks access, the run is skipped instead of triggering the private leader. Regression tests: - Plain member create autopilot to private-leader squad → 403 - Plain member update autopilot to private-leader squad → 403 - Owner create autopilot to private-leader squad → 201 - Owner-created autopilot dispatch → issue_created (positive) - Legacy plain-member-created autopilot dispatch → skipped (fail-closed) Co-authored-by: multica-agent <[email protected]> * test: add run_only legacy private-leader squad dispatch regression test Covers the dispatchRunOnly path explicitly, complementing the existing create_issue dispatch test. Both dispatch branches now have direct test coverage for the private-leader fail-closed gate. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-agent <[email protected]>	2026-06-02 15:47:57 +08:00
3 people	ae11f290b4	fix(server): gate GitHub auto-close on closing keywords (MUL-2680) (#3281) ... * fix(server): gate GitHub auto-close on closing keywords (MUL-2680) Closes multica-ai/multica#3264. The PR webhook previously treated any mention of an issue identifier in a PR title/body/branch as a close intent, so a body of "Closes MUL-1. Follow up in MUL-2. Unblocks MUL-3." would advance all three issues to done on merge. The auto-link layer stays generous (mentions still link the PR), but advancing to done now requires an explicit "Closes/Fixes/Resolves MUL-X" keyword adjacent to the identifier in the title or body — bare title prefixes (`MUL-1: ...`) and branch-name references no longer auto-complete. MUL-2680 Co-authored-by: multica-agent <[email protected]> * fix(server): persist close_intent on issue↔PR link rows (MUL-2680) The first take of MUL-2680 gated auto-advance on `closingIdents[id]` from the current webhook event. That broke the multi-PR sibling case: a PR declaring `Closes MUL-X` could merge first while a link-only sibling stayed open, leaving the issue in_progress; when the sibling closed later, its webhook carried no closing keyword and the handler skipped re-evaluation, so the issue stayed stuck forever. Move close intent from per-event state to per-link state: - New `close_intent` column on `issue_pull_request` (migration 109), set monotonically — `LinkIssueToPullRequest` ORs the existing flag with the incoming one so a subsequent webhook re-fire without the keyword cannot clear it. - New `GetIssuePullRequestCloseAggregate` query returns open-count and merged-with-close-intent-count for an issue. The auto-advance gate now reads from this persisted aggregate, which is event-agnostic: any terminal linked-PR event re-evaluates and the verdict only depends on accumulated DB state. - Webhook handler links all mentioned identifiers first (writing close_intent for the ones declared with a keyword), then iterates the affected issues in a separate pass to re-evaluate. The 'only fires for keyword-declared identifiers in this event' gate is gone — replaced by `merged_with_close_intent_count > 0` against the link rows. Regression test `TestWebhook_LinkOnlySiblingMergeAfterCloseKeywordPR` walks the full open→merge→open→merge sequence Elon described and asserts the issue advances on the link-only sibling's merge. MUL-2680 Co-authored-by: multica-agent <[email protected]> * Fix GitHub close intent updates Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-agent <[email protected]> Co-authored-by: Eve <[email protected]>	2026-05-26 16:45:46 +08:00
Bohan Jiangandmultica-agent	c967ae0e0e	feat(issues): platform-owned parent notify on child done (MUL-2538) (#3055) ... * feat(issues): platform-owned parent notify on child done (MUL-2538) When a child issue transitions from a non-done status into `done` and has an open parent, the server now posts a top-level platform-generated comment on the parent itself. Replaces the agent-prompt rule shipped in PR #2918, which produced self-mention loops, planner ping-pong, and accidental `MUL-` prefix hardcoding because the agent did not always know the workspace prefix. - Migration 107 widens `comment.author_type` to allow `system`; the zero UUID is used as the sentinel `author_id` (the column stays NOT NULL, callers branch on `author_type === 'system'`). - `Handler.notifyParentOfChildDone` fires from both `UpdateIssue` and `BatchUpdateIssues`. Guards: prev status != done, new status == done, parent set, parent not in `done`/`cancelled`. Bypasses the CreateComment HTTP path so the assignee on_comment trigger and the mention-trigger paths do not fire — the comment content carries only the safe issue mention for the child, no `mention://agent/...` / `mention://member/...` / `mention://squad/...` links. - `runtime_config.go` downgrades the Parent/Sub-issue Protocol rule 1 to an explicit "do NOT post one yourself" guardrail; rule 2 (sub-issue creation `--status todo` vs `backlog`) is unchanged. - New handler test exercises the happy path, idempotency, reopen+done, parent done/cancelled guards, and the no-parent case. Runtime-config tests reassert the new wording and the banned strings from the prior revision. Co-authored-by: multica-agent <[email protected]> * fix(issues): isolate system comments + wire GH merge path (MUL-2538) Addresses the two must-fix items from the PR #3055 second review: 1. The platform-generated `comment:created` event (author_type='system') was running through the generic comment listeners, which (a) tried to subscribe the zero-UUID author and (b) parsed @mentions from the body for inbox notifications. Both subscriber_listeners and notification_listeners now early-return on author_type='system' so the event becomes a pure WS broadcast for the timeline — no inbox rows, no transcluded-mention attack surface. 2. advanceIssueToDone (the GitHub merge auto-done path) only published issue:updated and skipped notifyParentOfChildDone, so a child closed via merged PR — the dominant completion path — left the parent silent. The helper is now invoked on the same prev/updated pair, with the existing guards (transition + parent state) protecting double-fire. Tests: - New cmd/server/notification_listeners_test: TestNotification_SystemCommentSkipsInboxAndMentions (parent subscribers and smuggled @mention targets stay quiet), TestSubscriberSystemCommentDoesNotSubscribe (zero-UUID never reaches AddIssueSubscriber). - New internal/handler/github_test: TestWebhook_MergedPR_ChildWithParent_NotifiesParent fires a real pull_request closed-merged webhook against a child and asserts the parent receives exactly one safe system comment with the workspace's real identifier (no `mention://agent|member|squad` links). Co-authored-by: multica-agent <[email protected]> * fix(runtime): drop parent-notification guidance from agent brief (MUL-2538) Per Bohan's product call on PR #3055: the platform now owns the child-done parent notification, so the runtime brief should not mention the parent-comment path at all — not as an instruction, not as a "do not do it" guardrail. The previous revision kept rule 1 of the Parent / Sub-issue Protocol as a "Do NOT post your own parent-notification comment." sentence; that still puts the concept in front of the agent every run, which is exactly what we are trying to avoid. What changes: - Delete the "Parent / Sub-issue Protocol" preamble and rule 1 from buildMetaSkillContent. The remaining content — the `--status todo` vs `--status backlog` rule for creating sub-issues — now lives in a dedicated `## Sub-issue Creation` section, since the parent/child framing it previously sat under is gone. - The system comment on the parent stays exactly as in 366f6e2: the agent simply does not need to know about it. Tests: - runtime_config_test.go is rewritten around the new section name and the wider "no parent-notification guidance" canary; the banned list now covers both the original PR #2918 wording and the intermediate "do NOT post one" wording. System comment UI: the frontend already renders `author_type === "system"` with author name "Multica" (`useActorName`) and the MulticaIcon avatar (`ActorAvatar` via `isSystem`), matching Bohan's "looks like a normal comment, author is multica + multica logo" requirement — no frontend changes needed. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-agent <[email protected]>	2026-05-22 14:51:43 +08:00
3 people	1c91c2a3b2	security(db): scope DELETE/UpdateIssueStatus by workspace_id (defense-in-depth) (#3027) ... * fix(security): scope DELETE/UpdateIssueStatus by workspace_id Add workspace_id to the WHERE clause of DeleteIssue, DeleteComment, DeleteProject, DeleteSkill, DeleteChatSession, and UpdateIssueStatus as SQL-layer defense-in-depth. Handler loaders (loadIssueForUser / loadSkillForUser / etc.) already enforce workspace membership today, so this is not patching a known live vuln. But the tenant invariant is currently a handler-layer guarantee — a future loader bypass or a new caller skipping the loader would be silently catastrophic. Making workspace_id part of the SQL identity collapses the trust surface to the schema itself: forging a sibling-workspace UUID becomes ErrNoRows instead of a cross-tenant write. Reference: incident #1661 (util.ParseUUID silent zero UUID returning 204 on a DELETE that matched zero rows) — same class of failure, prevented at a different layer. Scope: - 5 DELETE queries: issue, comment, project, skill, chat_session - 1 simple UPDATE: UpdateIssueStatus (2 narg, no SET ordering risk) - All callers updated (handlers, service, runtime sweeper fallback) Multi-narg UPDATE queries (UpdateIssue, UpdateProject, UpdateSkill, UpdateComment, UpdateChatSession*) are deferred to a follow-up to keep this change reviewable: each needs its narg pinning shifted and per-caller verification. sqlc was regenerated by hand (no local sqlc toolchain); CI's backend job is the authoritative compile check. * test(security): add workspace_scope_guard regression test Locks in the SQL-layer tenant guard added in this PR. For each of the 6 scoped queries (DeleteIssue, DeleteComment, DeleteProject, DeleteSkill, DeleteChatSession, UpdateIssueStatus), creates the resource in workspace A, invokes the query with a foreign workspace UUID, and asserts the row is untouched (0 rows affected with no error for :exec; pgx.ErrNoRows for :one). A future refactor that drops the workspace_id arg from any of these queries will now fail loudly instead of silently regressing. Includes a sanity sub-test that the in-workspace path still mutates, so a buggy guard that returns no-op for every call would not pass. Co-Authored-By: Claude Opus 4 <[email protected]> --------- Co-authored-by: Tom Qiao <[email protected]> Co-authored-by: Claude Opus 4 <[email protected]>	2026-05-22 14:39:47 +08:00
3 people	e48f6a84d6	feat(github): expose read-only installation list to workspace members (MUL-2413) (#2886) ... * feat(github): expose read-only installation list to workspace members (MUL-2413) Relax `GET /api/workspaces/{id}/github/installations` from owner/admin-only to any workspace member so the Settings → Integrations tab no longer renders blank for non-admins (the original symptom of MUL-2413). The handler now reads the caller's role from the workspace middleware: - owner / admin keep the full row including the numeric `installation_id` (the connect / disconnect handle) and receive `can_manage: true`. - every other role (member / guest) receives rows with `installation_id` omitted and `can_manage: false`, giving them visibility into "is GitHub wired up?" without the management handle. `GET /github/connect` and `DELETE /github/installations/{id}` stay under the admin/owner middleware group — this PR only relaxes the read path. Tests: `TestListGitHubInstallations_RoleGating` exercises admin, owner, member, and guest paths against the real DB-backed handler fixture and asserts the field stripping + `can_manage` contract. Refs: MUL-2413 Co-authored-by: multica-agent <[email protected]> * fix(github): redact installation_id from realtime broadcasts (MUL-2413) GET /github/installations strips the numeric installation_id for non-admin members, but the github_installation:created / uninstall / suspend WS events were still publishing it, so the same handle was reachable from any workspace client subscribed to the workspace scope. Broadcast both payload variants without it — the frontend uses these events only to invalidate the installations query, so admins re-query the list endpoint to recover the management handle. Also adds a router-level test that mounts the production middleware split (member-visible list vs. owner/admin connect+delete) so a future routing change can't silently widen the write surface. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Lambda <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-05-20 04:17:45 +02:00
Jiayuan Zhang	cd37b4e3d6	feat(settings): consolidate GitHub options under a dedicated Settings tab (MUL-2414)	2026-05-19 17:23:30 +02:00
Jiayuan Zhangandmultica-agent	668cab6022	feat(github): mirror PR CI checks and merge conflict status (MUL-2228) (#2632) ... * feat(github): mirror PR CI checks and merge conflict status (MUL-2228) Surface "checks passed/failed" and "conflicts/no conflicts" badges under each linked PR on the issue page so users can judge readiness without flipping over to GitHub. CI state is fed by check_suite webhooks (GitHub Actions + apps using the Checks API; legacy status events are out of scope for MVP); conflicts are read from pull_request.mergeable_state. Data model: * github_pull_request: add head_sha + mergeable_state * github_pull_request_check_suite: per-suite rows keyed by (pr_id, suite_id) * Aggregation done at query time, filtering by current head_sha so late-arriving suites for a stale head can't contaminate the new head's pending view; per-app latest suite chosen first so a single app firing multiple suites isn't counted N times. Webhook hardening: * synchronize/opened/reopened/edited(base) explicitly clear mergeable_state * single-row ordering protection on the check_suite upsert prevents a late-delivered older event from overwriting a newer one * check_suite.pull_requests is iterated; unknown PRs are logged and dropped UI: * PR row shows Checks + Conflicts badges; opaque mergeable values (blocked/behind/unstable/...) render as no badge, not as conflicts. * Terminal PR states (merged/closed) suppress the status row entirely. Tests: * Pure unit coverage for derivePRMergeableState + aggregateChecksConclusion * Webhook integration tests: multi-app aggregation, old-head ignore, late-older-event ignore, synchronize clears mergeable_state * Vitest coverage for pull-request-list badge rendering across CI/conflict combinations and the legacy (null) fallback. Co-authored-by: multica-agent <[email protected]> * fix(github): scope check_suite PR lookup; preserve mergeable on metadata Addresses code review on PR #2632. 1. check_suite handler now resolves the PR through the workspace-scoped GetGitHubPullRequest query instead of GetGitHubPullRequestByRepoNumber. The (workspace_id, repo_owner, repo_name, pr_number) tuple is the real uniqueness key, so a bare (owner, repo, number) lookup could return a stale row from another workspace and either land the suite on the wrong PR or skip the right one when the installation ids drifted. The old unscoped query is removed. 2. derivePRMergeableState now returns (value, clear) and the upsert SQL distinguishes three cases: state-changing actions clear the column to NULL, non-empty payloads write the value, and metadata events with an empty payload preserve the existing column. Previously every empty payload became NULL, so a labeled/assigned event silently wiped a known clean/dirty verdict in violation of the RFC's "metadata empty payload preserves" rule. 3. ListPullRequestsByIssue narrows to the issue's PR ids before running the per-app check_suite aggregation, avoiding a full-table scan over github_pull_request_check_suite when only a handful of rows belong to the requested issue. New helper test covers labeled+empty preserves; new integration test verifies a metadata event after a known mergeable_state keeps the value. Co-authored-by: multica-agent <[email protected]> * feat(github): PR card layout v3 increment — stats + segmented progress bar Replaces the row + badge layout under "Pull requests" on the issue detail sidebar with a card that mirrors the GitHub PR summary look: title, author/avatar, +N −M · K files diff stats, segmented progress bar (failed → pending → passed, failure leftmost), and a one-line status caption following an explicit priority pass-through. Backend - Migration 092: github_pull_request adds additions / deletions / changed_files (INT NOT NULL DEFAULT 0). Zero defaults are what the new frontend treats as "legacy backend — hide the stats row" so old PR rows that pre-date this migration don't render "+0 −0 · 0 files". - pull_request webhook handler reads stats off the top-level payload. - ListPullRequestsByIssue now surfaces per-suite counts (checks_passed / failed / pending) alongside the existing aggregate conclusion, so the segmented bar reuses the already-computed counts with no new aggregation. Frontend (packages) - core/github/pull-request-status.{ts,test.ts}: pure-function module for the status-kind priority table and the segment derivation; 15 cases covered, includes the "all-zero → hide stats" guard. - views/issues/components/pull-request-list.tsx: PullRequestCard plus a compact-row fallback used when count > 4 (first 3 as cards, the remainder collapsed behind a Show more toggle). - i18n: new `pull_request_card_*` keys in en + zh-Hans. Tests - 12 component tests covering each rule of the priority table, the legacy-zero stats fallback, and the collapse threshold. - Reuse of the v3 webhook handler tests confirmed. Verification - pnpm typecheck + pnpm test green (60 test files, 536 tests). - go build ./... + go vet ./... clean. - 6 demo issues (DEV-2..DEV-7) screenshotted via Playwright; see the PR comments for the visual check matrix. Co-authored-by: multica-agent <[email protected]> * fix(views): collapse PR cards at N>=4, not N>4 The card-vs-collapse threshold used `>` so 4 PRs slipped past it and all rendered as full cards, contrary to RFC v3 (N >= 4 collapses to 3 cards + compact tail). Switch to `>=` and update the threshold- boundary test to expect "Show 1 more". Co-authored-by: multica-agent <[email protected]> * fix(views): align PR sidebar rows with existing list style Co-authored-by: multica-agent <[email protected]> * fix(views): hide terminal PR status badges Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-agent <[email protected]>	2026-05-16 21:26:30 +02:00
Bohan Jiangandmultica-agent	a02e58b488	fix(github): only auto-close issue after all linked PRs resolve (#2470) ... * fix(github): only auto-close issue when all linked PRs have resolved Previously, the webhook handler unconditionally moved an issue to `done` as soon as a single linked PR was merged. If a second PR was also linked to the same issue and still open / draft, the issue would close before the work was actually finished. Add `CountOpenSiblingPullRequestsForIssue` and gate the auto-status transition on it: a merged PR advances its linked issues only when no sibling PR linked to the same issue is still in flight. Issues stay put while siblings are open or draft, and the merge that resolves the last in-flight PR is the one that closes the issue. Adds an integration test that opens two PRs against the same issue, merges the first, asserts the issue stays in_progress, then merges the second and asserts the issue advances to done. Co-authored-by: multica-agent <[email protected]> * fix(github): re-evaluate auto-close on closed-without-merge events too GPT-Boy review on #2470: gating only the `state == "merged"` branch left one ordering hole. PR-A merges first → issue stays in_progress because PR-B is open; PR-B later closes WITHOUT merging → no event ever re-runs the auto-close check, so the issue is stuck in_progress. Generalise the trigger to every terminal PR event (`merged` or `closed`) and advance the issue only when: - the issue is not already terminal (done / cancelled); - no sibling PR is still in flight (open / draft); - at least one linked PR — current or sibling — actually merged. Rule (3) preserves "user closed every PR without merging → leave the issue alone": if no work was delivered, the user decides what to do. Replace `CountOpenSiblingPullRequestsForIssue` with `GetSiblingPullRequestStateCountsForIssue`, which returns both the in-flight count and the merged count in a single roundtrip. Adds `TestWebhook_ClosedSiblingAfterMerge` (the regression GPT-Boy flagged) and `TestWebhook_AllClosedWithoutMerge` (the negative case guarding rule 3). Refactors the multi-PR webhook helper out of the existing two-merge test so all three multi-PR scenarios share it. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-agent <[email protected]>	2026-05-12 15:39:55 +08:00
Bohan Jiangandmultica-agent	caeb146bac	feat(github): GitHub App integration for PR ↔ issue linking (#1817) ... * feat(github): GitHub App backend for PR ↔ issue linking - New tables: github_installation (workspace ↔ App install), github_pull_request (mirrored PR state), issue_pull_request (M:N link). - Webhook handler verifies HMAC-SHA256, upserts PR rows, parses issue identifiers from PR title/body/branch and auto-links them. Merging a linked PR moves the issue to done. - Connect/setup endpoints power the zero-config "Connect GitHub" install flow; state token is HMAC-signed so the setup callback can recover the workspace. - Workspace-scoped admin routes for listing/disconnecting installations, plus a per-issue `pull-requests` list endpoint. Co-authored-by: multica-agent <[email protected]> * feat(github): UI for connecting GitHub and viewing linked PRs - Settings → Integrations: new tab with Connect GitHub / installations list / disconnect, gated on the deployment having the App configured. - Issue detail sidebar: Pull requests section showing linked PR title, repo, state (open/draft/merged/closed), and author, with deep link to GitHub. - Real-time refresh: github_installation:* and pull_request:* events invalidate the matching TanStack Query caches. Co-authored-by: multica-agent <[email protected]> * fix(github): address review — null actor, role gating, configured guard, scoped uninstall broadcast - listeners: use optionalUUID(e.ActorID) so the system actor on the github-driven issue:updated event no longer panics activity / notification listeners; merged-PR → issue done now produces a status_changed activity and inbox entry. - IntegrationsTab: gate the admin-only installations query on canManage so members no longer hit /github/installations 403; the configured/not-configured copy is also scoped to admins. - backend: introduce isGitHubConfigured() requiring both GITHUB_APP_SLUG and GITHUB_WEBHOOK_SECRET, and surface that single flag from list-installations + connect endpoints so the frontend Connect button stays disabled until both are set. - DeleteGitHubInstallationByInstallationID now RETURNs workspace_id; webhook handler publishes github_installation:deleted scoped to the right workspace so already-open Settings tabs invalidate in real time. ErrNoRows on a re-fired delete short-circuits cleanly. - tests: focused webhook integration coverage (auto-link + merge → done, cancelled preservation, uninstall returns workspace). Co-authored-by: multica-agent <[email protected]> * fix(github): i18n the new GitHub UI strings to satisfy lint CI flagged every literal string in the Integrations tab, the Pull requests sidebar section, and the per-PR row label. Move them through useT() and add the matching `integrations.*` block to settings.json (en / zh-Hans) plus `detail.section_pull_requests` / `detail.pull_request_state_*` / loading + empty copy under `issues.json`. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-agent <[email protected]>	2026-05-12 13:49:03 +08:00