mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-11 02:46:45 +08:00
262dcb5fa7ee2986aaabf3a78ee698f4afb4f151
329
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Bohan Jiangandmultica-agent
|
43b0571f99 |
MUL-7821: feat(cli): mark duplicates with issue status/update --duplicate-of (#8979)
* feat(cli): mark duplicates from issue status and issue update (MUL-7821) Add --duplicate-of to `multica issue status` and `multica issue update`. It resolves an issue key or UUID and sends duplicate_of_issue_id, which the server already accepts (MUL-7349). The CLI rejects a status other than cancelled, and description or attachment changes, before any request or upload. It fails when the response has no duplicate_of field, so a server older than v0.5.2 cannot silently drop the mark. Closes #8974 Co-authored-by: multica-agent <[email protected]> * docs: teach agents to mark duplicates with --duplicate-of (MUL-7821) Add the command to the agent runtime brief, the multica-platform issues reference, CLI_AND_DAEMON.md and the CLI docs tables, so agents record the mark instead of cancelling a duplicate and explaining it in a comment. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-agent <[email protected]> |
||
|
|
5ed09342af |
MUL-7680: issue wakeup v2 — conditions, runaway protection, check-ins and visibility (#8807)
* feat(wakeups): add rule deadlines and expose the child-done system rule (MUL-7680)
Wakeups can now end: an absolute deadline (expires_at) or a relative wait
(expires_in_seconds) that restarts when the rule is re-enabled. When the
deadline passes first, the scheduler ends the rule; event rules with
on_timeout=wake run the target once with a wakeup.timeout fact. A timed-out
rule keeps disabled_at NULL so its timeout run stays claimable.
The implicit "wake the parent's assignee when a stage of sub-issues
finishes" behavior is now described by GET /api/issues/{id}/system-wakeups
and can be turned off or given a supplementary instruction per issue via
PUT /api/issues/{id}/system-wakeups/child_done. Rule reads fail open to the
previous behavior.
List responses add the creator (member or agent) and the new expiry fields.
The CLI gains --expires-in, --expires-at and --on-timeout.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* feat(issues): let members create wakeups and show the child-done system rule (MUL-7680)
The issue sidebar's Wakeups section is always available on open issues and
gains a New wakeup popover. Members pick a condition (a time, a recurring
check with an end date, someone's reply, an agent's run ending, or raw
events), the agent to wake, the instruction, how often it fires, how long
to wait and what happens on timeout. It posts the same configuration agents
create with the CLI.
The parent's child-done wake appears as a System rule with its stage,
remaining sub-issues and target, a per-issue toggle and a supplementary
instruction. Rows show when a rule ends, timed-out rules read as such, and
details name who created the rule. Rule titles wrap to two lines instead of
truncating.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* docs(wakeups): document deadlines, member-created rules and the system rule (MUL-7680)
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* feat(wakeups): platform conditions, runaway protection and check-ins (MUL-7680)
Conditions: a wakeup can now wait for a fact the platform checks itself
(a status, assignee, label or property value; sub-issues or one stage
finishing; a linked PR's checks finishing or merging; another issue's
status). Related events only prompt an early evaluation; the rule wakes
its agent when the predicate becomes true, and a repeating rule fires
again only after it turned false or its facts changed. Finished PR checks
from before registration are ignored.
Runaway protection: repeating event rules stop after max_fires runs
(default 20), a rule pauses when its causal chain passes through it a
third time without a person in between, or when it starts 12 runs in an
hour. The reason is stored and shown.
Also: silent check-ins for every/cron checks (the run then posts no
fallback comment), wake now, delete, a per-rule run history, paused-rule
lists, timeline entries for created/triggered/timed-out/paused/check-in,
and source, paused scope, 7-day runs and child-done system rows in the
workspace list. The CLI gains --until-* conditions, --max-fires and the
trigger/delete/checkin/runs commands; the brief and wakeup prompt state
the check-in exception.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* feat(issues): record child-done transitions in the status write (MUL-7680)
The child-done system rule ran after the status write committed and gave
up on any error, so a crash, deploy or transient failure lost the
parent's wake. A trigger now records each child's move into a closed
status in the writing transaction, for every writer. The request that
wrote it processes the rows right away and a scheduler sweep retries
anything left over; claims make the two paths process a transition
once.
The rule also follows a workspace default (settings key
system_wakeup_child_done) until an issue sets its own, the per-issue
update accepts partial bodies, and each trigger adds a timeline entry
that names its system comment.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* feat(views): wakeup conditions, history, timeline and workspace list (MUL-7680)
Issue sidebar: members can create the platform-checked conditions (a field
reaching a value, sub-issues or a stage finishing, a linked PR's CI or merge,
another issue's status) and cap how often a repeating wait fires. Rule
details show the fire count, why the platform paused a rule, the latest runs
with silent-check notes, and offer wake now and delete.
The issue header says what the issue is waiting for ("Emacs 在等 Jiayuan 回复
+2") and opens the Wakeups section. Board and list cards use the same short
sentence, or flag a paused rule. Wakeup runs read as "由唤醒触发 · <condition>"
in the execution log, transcript and on the comments they post.
The timeline shows created, triggered, timed-out, paused and check-in entries
with the rule they came from; consecutive check-ins merge, and the child-done
entry stands in for its system comment, which it can reveal.
Automation → Issue wakeups adds a source column and filter, 7-day runs, a
paused scope with a banner, the child-done system rule on each waiting parent,
and "New wakeup" with an issue picker. Settings → Issue statuses sets the
system rule's workspace default. Copy in all five languages.
The server records the watched issue's identifier in other-issue conditions
and the rule's creator in timeline snapshots, and returns board summaries with
their condition.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* docs(wakeups): document conditions, runaway protection and check-ins (MUL-7680)
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* fix(wakeups): list system rules without a revision and localize statuses (MUL-7680)
System rule rows returned revision 0, which clients reject as a rule
revision; they now return null, and a list row with an invalid revision no
longer fails the whole page. Built-in statuses in conditions and the status
picker read in the viewer's language. Adds a browser test for a condition
created from the form, the header summary, the scheduler waking the agent
and the workspace list.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* fix(views): say why a rule paused without repeating "paused" (MUL-7680)
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* fix(i18n): use the product's task terms in wakeup copy (MUL-7680)
ja, ko and fr wakeup strings said イシュー / 이슈 / ticket; the product term
for an issue in those languages is タスク / 태스크 / tâche (and sub-tasks
accordingly), per the conventions page. French strings are rewritten for
the feminine noun.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* test(views): pin the shortcut platform in the wakeup form test (MUL-7680)
The send shortcut is primary+Enter, which is Ctrl on Linux CI runners, so
pressing Cmd+Enter only submitted on macOS.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* fix(views): simplify the workspace wakeup list
Put the filters on one row: scope as a segmented control on the left,
source/trigger/agent and a search-as-you-type box on the right. "New
wakeup" moves to the page header, which drops the separate Search button.
Rows are single-line: the identifier sits before the title, frequency and
timezone move into the trigger tooltip, and selection, prompt edit and
the last run's transcript show on row hover or focus. The issue column
takes the remaining width and truncates, so the table fits the page.
Also fix creating from the list: the issue picker closes itself right
after reporting the selection, and that close cancelled the flow before
the form could open.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* refactor(server): run the child_done rule as a system issue wakeup
The parent-assignee wake on closed sub-issue stages was its own code path:
a system comment, a mention-style run, an override table and endpoints.
It is now an ordinary issue_wakeup row with system_rule set, sharing the
condition, receipts, runaway protection, timeline entries and run model of
people's rules.
- One children_done evaluator: a stage wakes the assignee when it closes
while a later stage waits; the wrap-up waits for every sub-issue,
unstaged ones included. People's sub-issue conditions read the same set.
- issue_child_event records closing, reopening, re-parenting and restaging
for every writer; the request processes it right after commit and the
sweep retries. User sub-issue rules become immediate too.
- The target is resolved when it fires: an agent run, a squad leader run,
an inbox notification for a member, or only the timeline entry.
- A parked (backlog) parent catches up when it leaves backlog; a waiting
run of the same agent is joined instead of queuing a second one; the
hourly limit pauses a runaway rule. No system comment is posted.
- The run gets the issue's instruction, else the workspace's, else the
built-in one, plus each stage's counts and the next stage.
- Workspace defaults move to GET/PUT /api/system-wakeups (owners and
admins) and apply to every rule nobody customized; turning a rule on
records what already holds so old facts do not fire.
- Existing open parents get their rule from a one-time backfill.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* feat(views): workspace wakeup settings and sub-issue notifications
- Settings → Wakeups holds the sub-issue rule's workspace default and
default instruction; the toggle leaves Issue statuses.
- The issue's system rule shows who it reaches (a member is notified),
a platform pause, and the instruction it will use.
- Timeline entries say whether the assignee was woken, notified or joined
a waiting run. The folding of the old system comment is gone.
- Inbox renders the new children_done notification on web, desktop and
mobile.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* fix(server): stop wakeups from repeating their agent's runs
Runs are serialized per issue and agent, so a wakeup never ran beside
another run of its agent, but it queued behind one and repeated it. Each
rule now checks two things before it starts a run:
- Acknowledged: every input came from the agent itself. Its own comments
and issue changes never wake it, and a condition its own unfinished run
on the issue satisfied does not wake it when the platform or the agent
set the rule up. This fixes #8849: a coordinator closing its own stage is
not woken again while that run is going. A person's condition rule still
runs afterwards, since the running agent lacks its instruction.
- Merged: when a run of the agent is already waiting to start on the issue
(assignment, comment, another rule), the rule's instruction and facts
join that run (context.wakeup_joined, sent to the daemon as
wakeup_joined and rendered for every prompt kind) instead of queuing
another. Turning the rule off or replacing it withdraws what it joined.
Sub-issue changes now record the run that made them, and hints and
condition.met inputs carry those runs so a satisfied condition knows
whether the agent caused it. A rule that names the agent itself as the
actor keeps waking it on its own changes.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* feat(views): show merged and acknowledged wakeups, hint assignee comment rules
- Timeline entries say when a wakeup joined the agent's waiting run or was
the agent's own doing, for people's rules and the sub-issue rule.
- The create form tells a member that a reply or comment rule for the
issue's agent assignee joins the run a comment already starts.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* fix(server): hand waiting wakeups to a run when it is claimed
A rule that fired while its agent had a run waiting used to write its
instruction into that run right away and consume its inputs. That let a
member's rule run under another member's identity, kept deleted rules'
instructions in the run, lost the inputs when the run was cancelled or
claimed by an older daemon, and skipped the fire cap and loop check.
Now the rule only waits for a run that runs as the same person its own
run would, keeping its inputs. When a daemon advertising
joined-wakeups-v1 claims that run, JoinWaitingWakeups rechecks each rule
(on, same agent and person, creator still allowed, not the agent's own
input, no loop), consumes its inputs, counts the fire toward max_fires,
adds its chain and records the merge. A re-claim drops entries of rules
turned off or changed since. Anything else leaves the rule its inputs to
start its own run.
A child_done rule created by the backfill or by processing a change now
treats sub-issues with unprocessed close events as still open, so the
backfill no longer swallows a close waiting for the sweep.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* fix(server): consume joined wakeup inputs only once the run starts
A claim used to consume the inputs of the rules that joined the run. If
that claim did not go through, the run went back to the queue carrying
inputs the rules no longer had: turning the sub-issue rule off on the
issue or for the workspace (which only changes enabled) did not remove
them from the next claim, and cancelling the run lost a once rule that
had already ended.
A claim now reserves the inputs for the run (receipt task_id, still
unprocessed). The rule's next dispatch settles them: a run that started
consumes them as a merged firing (once ends, max_fires counts and can
pause, timeline entry); a run that ended without starting gives them
back; one that has not started keeps them. Turning a rule off or
changing it discards its pending inputs, reserved ones included, and a
later claim of the same run rechecks every entry and drops the ones
without reserved inputs or no longer allowed to reach the run.
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]>
|
||
|
|
7dac88a6a8 |
MUL-7649: issue deliverables and dynamic blocks — sidebar section, overview grid, viewer info panel, html/mermaid block frame (#8779)
* feat(attachments): issue deliverables model and sequence block ids (MUL-7649) collectDeliverableFiles groups every comment upload into deliverables, merging same-name, same-type re-uploads into versions (newest version first). Description attachments are inputs and never enter the model. Sequence items now carry the id of the block they came from, so a viewer can say which comment (or the description) a file was posted in. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * feat(viewer): host details, overview and info panel slots (MUL-7649) PreviewSequenceProvider takes an optional describeItem (title accessory, info panel, locate action) and onOpenOverview. The viewer renders only the slots it is handed: a locate button, a grid button (G) and an info toggle (I) whose panel sits beside the stage. Letter keys stand down in fields, and any key pressed inside an open menu stays with the menu. A zoom canvas still at fit now follows a viewport resize (the info panel opening), while a zoom the reader chose is only clamped, as before. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * feat(issues): deliverables section, overview grid and info panel (MUL-7649) The sidebar's "Deliverables" section shows what the issue delivered as a whole: its pull requests and the files its comments uploaded, re-uploads merged into one entry marked v2. It lists the three newest images and four newest files, plus "View all N deliverables". The former Pull requests section becomes the code group, keeping its MUL-7429 actions (link a PR by URL, the auto-complete menu) beside the group label. While the workspace shows PRs, the section stays up even with nothing delivered, since that is where a PR gets linked by hand. The overview (from "view all" or G in the viewer) shows code first, then files grouped by posting comment, filterable by kind, each group one click from its comment. Its count always equals the sidebar's. In the viewer, deliverables get a version switcher, and every file gets an info panel (source excerpt, sibling files, uploader, time, size) and "Show in comments", which unfolds a resolved thread if needed and reuses the timeline's jump-and-flash highlight. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * feat(comments): standalone attachments as cards and an image row (MUL-7649) The files under a comment (and a chat message) no longer stack as one full-width row each. Other files become cards in a grid: type glyph, name, "TYPE · size", download / remove on hover, and the whole card opens the file. Several images sit in one row at a shared height; a lone image keeps its full size. HTML keeps its embedded preview (MUL-2330). On the issue page a re-uploaded file's card carries its version (v1, v2) from the deliverables model, through a small context. Standalone attachments are grouped images, then HTML, then files, and the preview sequence walks them in that same order, so paging follows the screen. The file-type glyph moves to editor/utils/file-icon so comment cards and the deliverables surfaces share one mapping. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * refactor(issues): pull requests keep their own section (MUL-7649) Since MUL-7429 the PR block is issue workflow — linking a PR by hand, the auto-complete switch, "won't auto-complete: missing Closes" — rather than output, so it goes back to main's own Pull requests section, unchanged, directly above Deliverables. Deliverables is now the delivered files only: hidden until a comment delivers one, and the sidebar number, "View all N" and the overview's "All N" all count files. The overview drops its code group. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(comments): lay several images out as justified rows (MUL-7649) Fixed-height tiles wrapped as soon as the column narrowed — three screenshots became two plus an orphan, with a ragged right edge. Rows are now justified from each image's real aspect ratio: every row shares one height and a full row runs edge to edge. A row takes as many images as fit above a minimum height, never grows past a maximum, and a last row that does not fill keeps the height of the row above it. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * feat(rich-content): dynamic block frame for html and mermaid fences (MUL-7649) A fenced ```html or ```mermaid block is part of the message body, so it now renders in one shared frame instead of two unrelated widgets: - Always-visible title bar: kind icon, the fence's title="..." (or the kind name), a kind chip, Preview | Source, fullscreen and copy. Previously the controls only appeared on hover and floated over the content. - The body takes its content's height (at least 120px) and collapses past 480px behind a fade and "Show all". An HTML block used to be a fixed 480px. - Loading keeps the height this block had earlier in the session; a script error or a Mermaid parse error is explained inside the frame with "View source" and "Copy error". - HTML gets the app's theme tokens (--foreground, --chart-1 ...); HTML that uses one also gets the app's color-scheme, so it follows dark mode. HTML that uses none keeps its own look. The sandboxed document reports its height and first uncaught error through a one-line postMessage bridge; the page checks the message source and clamps every value, and stops a document whose height follows the frame. rehype-raw drops the fence meta, so the title is read in the closed-fence parse. The Mermaid sandbox now also declares the app's color-scheme and font: in dark mode it was painted on an opaque white canvas, and its labels were drawn in a serif font that did not match the layout Mermaid measured. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * feat(attachments): HTML files show as cards, not inline previews (MUL-7649) Reverses the MUL-2330 pin. An uploaded HTML file is a deliverable to open, not part of the text: like every other non-image file it shows as a card (a row inline in the body, a grid card under it) that opens in the viewer, and its contents are no longer fetched to embed a 480px iframe. HTML meant to be read in place is written as a ```html block, which renders as a dynamic block. - Attachment drops its html branch; HtmlAttachmentPreview is deleted and HtmlPreviewBody keeps only the inline source the viewer uses. - orderStandaloneAttachments groups images, then files (HTML included), so AttachmentList and the viewer sequence still walk the same order. - The MUL-2330 regression pins now assert the card and that no text fetch happens. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * docs: charts go in html/mermaid blocks, files show as cards (MUL-7649) Agents that uploaded an HTML chart expecting it to render in the comment now get a file card instead. Tell them where each kind of content goes: - multica-platform skill (issues reference, routed from the table): a fenced ```html / ```mermaid block renders in place with a title bar and content height; an attached file, HTML included, is a card that opens the viewer. Covers title="...", the sandbox, the theme variables and their opt-in color scheme, and sizing to content rather than the viewport. - `multica issue comment add --attachment` help says the same in one line. - Comments docs (all five languages) describe both. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * feat(daemon): runtime brief says where charts and diagrams go (MUL-7649) The chart/file rule lived only in the multica-platform skill's issues reference, which an agent opens on demand, and the skill's description did not mention comment formatting, so an agent that just wanted a chart in its result had no reason to read it. Agents that uploaded report.html expecting an inline chart now get a card. - The brief's Output section carries one line, beside the file-delivery line, on the surfaces the web renders (issue comments and web/mobile chat): put charts and diagrams in the text as a fenced html or mermaid block, name it with title="...", and an attached file, HTML included, shows as a card. Theming and sizing stay in the reference. - Channel chats, autopilot run results and quick-create stdout do not render these blocks and do not get the line; the delivery tests pin both sides. - The skill description names "charts and files in comments" so the reference is picked for that task. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(issues): locate a file in a collapsed thread (MUL-7649) "Show in comments" for a file posted in a reply only unfolded resolved threads and gave up after 30 frames, so a thread the reader had collapsed stayed shut and the reply never mounted. A reply now goes through the quick-jump rail's jumpToReply, which already undoes every kind of folding and waits for the reply to land. A root comment gets the same treatment for its own thread (the reader's collapse, or a resolved bar), then the jump. Found in review by Emacs. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(viewer): the info panel's "Show in comments" closes the viewer (MUL-7649) The top bar's locate went through the viewer, which closes itself first; the same button in the info panel called the host's callback directly, so the page scrolled underneath a viewer that still covered it. Controls handed to describeItem gain close(), and the issue page builds one close-then-locate action for both entry points. Found in review by Emacs. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(issues): the deliverables overview is a real modal dialog (MUL-7649) The overview was a portaled layer with role="dialog" and nothing else: focus stayed on the opener, Tab walked into the page underneath, and closing left focus wherever it was. It now renders through the shared Dialog, restyled to cover the window, so the primitive moves focus in, keeps it inside, returns it to the opener, and handles Escape. `G` back to the viewer's file stays. The dialog is named by its title ("MUL-123 deliverables"). Found in review by Emacs. 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]> |
||
|
|
e909e9c89d |
MUL-7587 fix(daemon): show on-behalf-of identity in agent context (#8688)
* fix(daemon): expose original requester in agent context * fix(daemon): render run originator as on-behalf-of identity * docs(daemon): update stale Task Initiator comments to On Behalf Of Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
bojackli
|
df2e86cf3a |
fix(daemon): retry Windows task root record rename (#8786)
Refs #8785 |
||
|
pseudoyuandClaude Fable 5.1
|
62efe26aa0 |
MUL-7620 feat(telegram): send and receive photos, videos and files (#8727)
* feat(telegram): send and receive photos, videos and files Telegram was text-only in both directions: dispatch dropped every non-text update with an "unsupported" notice, the outbound subscriber only knew sendMessage/editMessageText, and the channel was never declared to DeclareChannelFileDelivery, so agents were told they cannot attach files. Inbound: photos, documents, video, video notes, animations, audio and voice notes become chat attachments through the engine's MediaResolver seam (getFile, file host download, object storage, intent-ledger row before the PUT, as Slack does). The caption is the message text and a placeholder marker leads the sender's own text; the engine swaps it for the attachment link once the bind commits. Quoted and recent group context still go in front of it. Stickers and other non-file kinds keep the unsupported notice. Outbound: files the agent bound to its reply are sent into the chat as their own messages once the text settles (sendPhoto for common images up to 10 MB, sendVideo/sendAudio for mp4 and common audio, sendDocument for the rest up to 50 MB). A photo Telegram refuses to process is resent as a document. Both halves hang off the same store != nil branch that declares file delivery, so the agent is promised the hop only where it exists. Compared with the WeCom reference this drops relay routing, the three-state delivery classification, per-reply metrics and the pending/admitted counters: Telegram outbound is stateless HTTP and the delivery lease already guarantees a single sender per turn. Co-Authored-By: Claude Fable 5.1 <[email protected]> * fix(telegram): address media review — deliver files once, admit before spawning, keep the notice without storage Review on #8727 raised five points; each lands here with a test. 1. A file-only reply could be delivered twice. closeTurn returned true whether this call ended the turn, a deeper attempt owned it, or it was settled already, and the empty-reply branch delivered files on all three. closeTurn now reports closedNow / closedAlready / closeHeld, and files go out only on closedNow. Regression tests: a replayed chat:done on another replica sends the files once; a completion an automatic retry has superseded sends nothing and the retry's answer still lands. 2. Admission sat behind the goroutine and the lookup. The attachment lookup now runs on the terminal worker, so a reply with nothing bound costs one indexed read and no goroutine; a delivery is spawned only for files known to exist, under a slot claimed first (non-blocking, four slots). Every slot busy sheds the reply with the notice instead of parking it. 3. Rebased on main over MUL-7585: recent group context, /new isolation and the privacy rule are kept; unaddressed group media is buffered and never a turn, as before; the media placeholder leads the sender's own text with quoted and recent context still in front. 4. Without object storage the member got no signal. The polling loop now knows whether the deployment stores media (ChannelDeps.AcceptsMedia, set from the same store != nil branch as the resolver) and keeps the old unsupported notice when it does not. A fetch that fails — over the 20 MB bot download limit, or a failed download — tells the sender so, instead of leaving the agent a placeholder nothing stands behind. 5. The failure notice claimed more than the code knew: a lost response may mean Telegram accepted the file. It now says delivery could not be confirmed and that the file stays attached in Multica, which holds either way. Co-Authored-By: Claude Fable 5.1 <[email protected]> * fix(telegram): offset the media placeholder past prepended context; report a failed attachment lookup Second review round on #8727. 1. With quoted or recent group context ahead of the sender's text, the MediaRef carried only the placeholder, so the engine replaced occurrence 0: a member who typed "[Image]" in the window received the sender's file and the sender's own marker stayed bare. inboundMedia now carries PlaceholderIndex — the enrichers only prepend, so it is the count of the marker ahead of the sender's segment, literals included (the DingTalk rule) — and the resolver sets it as InlineIndex. Regression tests cover the recent-context and the quoted-message paths. 2. A failed ListAttachmentsByChatMessage dropped the files silently while the text already on screen referred to them. The read has no side effects, so it is retried three times 250 ms apart; if it still fails the member gets a notice worded for what is known — whether the reply had files could not be checked — never the one that presumes a file existed. Co-Authored-By: Claude Fable 5.1 <[email protected]> --------- Co-authored-by: Claude Fable 5.1 <[email protected]> |
||
|
Bohan Jiangandmultica-agent
|
81f0fb418a |
test(execenv): stop the test repo template racing git auto maintenance (#8730)
The shared template repo is built once per test binary and handed to every test as an os.CopyFS copy, so nothing may write inside it after buildTestRepoTemplate returns. git commit breaks that: it forks a detached `git maintenance run --auto` and returns without waiting, leaving a background process that creates and removes lock files under .git/. CopyFS then lists .git/objects/maintenance.lock and fails to open it once maintenance has removed it, failing whichever test happened to call newTestRepo at that moment — most recently TestGitRootLockTimeoutDoesNotAdviseDeletingTheLock on an auth-only PR (#8706), which touches nothing in this package. Disable auto maintenance in the template's config. Copies inherit it, so the git commands tests run against their own copy cannot spawn a background process that races their assertions either. Co-authored-by: multica-agent <[email protected]> |
||
|
|
d2423aa35c |
MUL-7590: fix(execenv): support OpenClaw agents.entries per-agent workspaces (#8678)
* fix(execenv): pin per-agent workspaces under the OpenClaw agents.entries schema OpenClaw 2026.8 retired the agents.list config path and reports it as "Unknown config path: agents.list", moving the agents to a map keyed by agent id under agents.entries. The per-task wrapper only ever wrote agents.defaults.workspace, and an agents.entries.<id>.workspace entry outranks it verbatim: on a host where adding a second agent pinned the gateway agent to an absolute workspace, every task ran in that pinned directory instead of its prepared workdir, so the workdir skills/ never loaded and the agent edited the wrong tree. Read agents.entries when agents.list is gone, pin every entry workspace to the task workdir on write-back, and keep the registry fallback for the generations that have neither config path. The stdout-envelope branch now matches the path-aware wording, so a 2026.8 host that prints its envelope without exiting is read instead of failed closed, and the entries probe shares the agents.list deadline so the worst case stays three CLI deadlines. Co-authored-by: multica-agent <[email protected]> * test(execenv): cover the agents.entries probe, write-back and shared deadline Adds the regressions the review of the earlier attempt asked for: the current "Unknown config path" envelope with err == nil selects the entries probe and never the registry, both the plain and the managed-MCP flows pin every agents.entries.<id>.workspace to the task workdir, a valid-but-unset entries path falls back to the registry, and an envelope naming an unrelated path still fails closed. The worst-case budget test now drives the five-invocation chain and pins the config-schema pair to one shared deadline, so a fourth budget fails loudly. Co-authored-by: multica-agent <[email protected]> * fix(execenv): pin agents.entries workspaces without copying the entry Review feedback on the write-back: `config get` redacts the config before it returns the path it was asked for, so every sensitive per-agent field comes back as `__OPENCLAW_REDACTED__`. The wrapper is a sibling of the user's config in the $include merge and siblings win, so copying the resolved entry would write that sentinel over the user's real secret — memory search API key, web fetch headers, sandbox SSH credentials — for every task on the host, and nothing on OpenClaw's load path restores it. `agents.entries` is an object keyed by id and objects merge recursively, so `{"<id>": {"workspace": "<workdir>"}}` is enough to pin the workspace while every other field still arrives from the user's own config. The same reasoning drops the resolved payload from the row representation: an entries row now carries the id marker alone, so no redacted copy of the user's agent config is held in memory or written to the discovery cache file. Since only the source still decides the write-back, it is now carried explicitly (`openclawAgentsSource`: list / entries / registry) rather than as a boolean that effectively meant "not agents.list", and the source is recorded in the cache entry so a hit rebuilds the same wrapper. The cache shape version is bumped, so an entry written by an older daemon is a clean miss. Co-authored-by: multica-agent <[email protected]> * test(execenv): assert the entries write-back emits workspace alone Review feedback: the tests encoded the old contract (name / model carried into the wrapper verbatim). They now assert the wrapper carries exactly one key per entry — `workspace` — and that nothing else from the resolved payload reaches the file OpenClaw reads. The new regression drives the redaction shapes upstream's own redact-snapshot suite covers (memory search apiKey, web fetch headers, sandbox SSH credentials) through the entries probe of a real preparation, and fails if any of them ends up in the wrapper: with the copying write-back restored it reports the sentinel in `agents.entries.main`. The unit test for the write-back gate also covers a half-marked list, which a quiet per-row filter would have pinned anyway. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: tianshuai1 <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
Multica Eveandmultica-agent
|
5140b99d86 |
MUL-7577: remove no-start guidance from runtime and platform skill (#8680)
* fix(prompts): remove no-start guidance from runtime briefs Co-authored-by: multica-agent <[email protected]> * fix(skills): remove no-start guidance from platform instructions Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-agent <[email protected]> |
||
|
Multica Eveandmultica-agent
|
8c9f865e3e |
MUL-7480: Revert comment steering feature (#8648)
* Revert "MUL-7480: Link delivered steers to final replies (#8623)" This reverts commit |
||
|
Bohan Jiangandmultica-agent
|
624648cc14 |
fix(daemon): offer a Git Bash variant of the Windows reply cookbook (MUL-7554) (#8634)
The Windows brief only knew PowerShell: the per-turn reply cookbook and the Comment Formatting cleanup line assumed every Windows agent pastes into PowerShell. The daemon knows the host OS, not the shell behind the agent's command tool. Claude Code runs Git Bash for its Bash tool, and a user who turns off its PowerShell tool (CLAUDE_CODE_USE_POWERSHELL_TOOL=0, GitHub #8627) gets a cookbook whose 'if ($LASTEXITCODE ...)' line is a bash syntax error that fires AFTER the post succeeded, so the agent sees a failure and may re-post. Render one variant per shell on Windows (PowerShell with the $LASTEXITCODE gate, Git Bash with '&&') and tell the agent to pick the one its command tool runs. The shell-execution test now selects the block by shell and adds a windows/git-bash case that runs the bash variant under sh. Co-authored-by: multica-agent <[email protected]> |
||
|
VvVandweiweiwei
|
2b62b2b52e |
fix(cli): compare workdir paths in one namespace (MUL-4252, MUL-7193) (#8028)
* fix(cli): compare workdir paths in one namespace (MUL-4252)
The containment guard behind --content-file, --description-file and --attachment
canonicalizes the workdir but could not canonicalize a candidate path that does
not exist yet, and compared the two sides in different namespaces. That is wrong
in three ways, and only the first is merely a wrong answer.
## A path inside the workdir read as outside
filepath.EvalSymlinks fails outright when any component is missing. The fallback
resolved one level up (filepath.Dir) and otherwise called filepath.Clean, which
does not follow symlinks at all.
os.Getwd() returns the LOGICAL working directory whenever $PWD names the current
directory, and everything that writes $PWD carries the path as typed: a shell's
`cd`, and the PWD=<workdir> this repo exports to agent processes
(pkg/agent/opencode.go, deveco.go, codearts.go). So on a host whose workdir path
crosses a symlink, the base was /private/var/... while the candidate stayed
/var/..., and `--content-file reports/summary.md` was rejected as a guardrail
violation when the real problem was that `reports/` did not exist. The only
remedy the error offers is --allow-external-file, so the caller's next move is to
disable the guard and collect an unrelated not-found error on the retry.
## A path outside the workdir read as inside
With an already-canonical workdir the same fallback inverts. `escape/sub/x.md`,
where `escape` is a symlink pointing out of the workdir, read as INSIDE it: the
unresolvable tail fell back to a lexical clean that never sees the symlink.
## ".." was applied to the string, not to the followed path
filepath.Join and filepath.Clean collapse "escape/.." before any symlink is
followed, while the kernel follows `escape` out of the workdir first and then
goes up. So the guard judged a different path than os.ReadFile then opened:
guard says within=true ; os.ReadFile got "another run's file"
That is the MUL-4252 failure this guard exists to prevent — reading a file left
behind by another run — reachable with a file that exists and no race.
## A path on another volume read as an internal failure
Windows only: filepath.Rel cannot relate two absolute paths on different volumes
and returns an error, which surfaced as `Rel: can't make Z:\report.md relative to
C:\Users\...\workdir`. Two absolute paths Rel cannot relate are on different
volumes, which is as far outside the workdir as a path gets, so the guard now
answers that instead.
## The fix
util.ResolveSymlinksBestEffort canonicalizes as far as the filesystem allows: it
resolves the path as given first — the only step that can see ".." the way the
kernel does — then walks up to the deepest existing ancestor, resolves that, and
re-attaches the missing tail lexically. A mismatch inside the missing tail is
inert: a path whose intermediate directories do not exist cannot be opened at
all. Both sides of the comparison go through it.
internal/daemon/execenv already had this algorithm as resolvePathBestEffort,
unexported and documented as Python's Path.resolve(strict=False). It now
delegates, so there is one owner, and the ".." handling makes it more faithful to
that documented contract rather than less.
## Coverage
TestFileWithinWorkingDir runs its table under two working-directory shapes, and
both are load-bearing: under a symlinked workdir an unresolved candidate always
looks outside, so the symlink-escape rows can only fail under a canonical
workdir. Reverting one part of the mechanism at a time:
candidate unresolved (Clean) 5 tests / 15 subtests fail
one-level fallback 3 tests / 8 subtests fail
base side left unresolved 5 tests / 9 subtests fail
clean before resolve ("..") 2 tests / 5 subtests fail
helper walks only one level 4 tests / 9 subtests fail
The lint side is covered too: classifyLocalPathTarget uses the same predicate,
and there the same false negative flips direction — a workdir-local path with a
missing intermediate directory matched neither of its signals, so the link was
published instead of blocked.
cmd/multica has no Windows job, so the guard's Windows-only behaviour ran
nowhere: a candidate on another volume, and a directory junction, which
filepath.EvalSymlinks resolves to its own name rather than descending through it
(internal/daemon/config_windows_test.go asserts that refusal directly). A
junction inside the workdir pointing out of it therefore still reads as inside,
before this change and after it. Failing closed on a component that exists but
cannot be canonicalized would reject every path under a pnpm-style node_modules
junction, and on a host whose workdir is itself reached through a junction it
would reject every --content-file and --attachment on that host — a larger break
than the one it prevents. The windows-tagged test pins the gap with the
observations it rests on and fails with instructions if the platform starts
resolving junctions. Its CI step is scoped and runs with -v so a silent skip is
visible rather than passing as "ok".
* fix(cli): lead with the missing file when a rejected path does not exist (MUL-4252)
The workdir containment guard answers one question and reports it as if it
answered two. A path outside the workdir that does not exist got the same error as
a stale file sitting in /tmp:
--content-file path "/tmp/desc.md" resolves outside the current working
directory; write agent temp files inside the task workdir ... Pass
--allow-external-file to override.
The only remedy that error offers is the flag, so the caller's reasonable next
move is to disable the guard and retry — earning a second, unrelated "no such
file or directory". The diagnosis points away from the actual problem, and the
lesson it teaches an agent is to turn the guard off.
MUL-4252 was about reading a file that exists and belongs to another run. When
the path does not exist there is no such file to protect anyone from, so the
missing file leads, with the containment problem kept as the second clause since
the caller still needs to know the flag would not help.
pathExists treats any stat error other than "not exists" as "exists", so a
permission wall or a broken mount keeps the guard's own wording and stays
fail-closed rather than being reported as a missing file.
Both halves are pinned by mutation: dropping the branch fails
TestExternalFileErrorLeadsWithMissingFile on the missing-file assertion, and
making pathExists report everything as missing fails the same test on the
assertion that an existing external file must still be told about
--allow-external-file.
* fix(cli): preserve Windows path kinds and resolve junctions in the containment guard (MUL-4252)
Addresses the two blockers in the review on 3ec1781e:
1. absNoClean prefixed the working directory onto every non-IsAbs path,
but on Windows a root-relative path (\tmp\desc.md) resolves against
the volume ROOT and a drive-relative path (C:tmp\desc.md) against the
current directory on that drive. The guard inspected
C:\task\workdir\tmp\desc.md while os.ReadFile opened C:\tmp\desc.md,
so an in-workdir shadow could pass an outside read. absNoClean now
splits per platform (path_other.go / path_windows.go) and preserves
the kinds; a drive-relative path on an unobservable drive is handed
back uncanonicalized and fails closed through the Rel branch.
2. The junction case in workdir_guard_windows_test.go pinned a known
contract violation (EvalSymlinks refuses to descend through junctions
under the winsymlink semantics this module ships with). Resolution is
now component-wise on Windows (evalPath) and follows junctions via
os.Readlink — which answers under both winsymlink settings — with a
bounded follow budget; reparse points Readlink cannot speak for
(OneDrive placeholders) stay opaque. A junction out of the workdir is
rejected, one into it (the pnpm shape) is still admitted, and the
known-gap test is replaced by cases asserting both directions plus a
chain and a dead target.
The shared walk in ResolveSymlinksBestEffort is now component-wise on
both platforms: it applies ".." to the resolved prefix even when the
tail is missing (previously the second phase started from filepath.Abs,
which cleans first), and it never depends on what EvalSymlinks does at
a filesystem root. Unix behaviour is unchanged for every shape the
existing tests cover; the new dot-dot-in-the-walk case pins the
difference.
* fix(cli): classify link targets with the full Windows path grammar and resolve unknown reparse points by handle (MUL-4252)
Addresses the two blockers in the review on d3d370377:
1. Link targets were spliced onto the link's parent unless
filepath.IsAbs said otherwise, but the kernel applies the full path
grammar to targets: a root-relative target (\outside) resolves from
the root of the volume the link sits on, and a volume-GUID target
(\\?\Volume{GUID}\...) is absolute in the device namespace. The
resolver now classifies targets through classifyTarget, which reuses
the same grammar as absNoClean; device-namespace targets without a
Win32 spelling are kept verbatim so the containment comparison fails
closed across the namespace instead of splicing a mangled relative
path (the prefix rules mirror internal/daemon's #6883 handling).
2. An os.Readlink failure on a ModeIrregular reparse point no longer
means "does not redirect". Readlink decodes only the symlink and
mount-point tags; other name-surrogates (e.g. container-isolation
links) redirect through a filter while the lexical location stays
behind. Such components are now resolved through
GetFinalPathNameByHandle (zero desired access, so no hydration, and
FILE_FLAG_BACKUP_SEMANTICS for directories); if the handle names the
same path the component is opaque, otherwise the walk continues from
the final path. When even the handle cannot name the location the
resolver returns the new util.ErrUnresolvablePath instead of
guessing: fileWithinWorkingDir treats it as outside (fail closed)
and execenv's delegate falls back to the input, which its
containment comparison rejects.
ResolveSymlinksBestEffort now returns an error so the fail-closed case
is explicit rather than a look-alike path. Negative tests cover a
root-relative symlink target with an in-workdir shadow (the kernel's
target is verified by content before asserting rejection) and a
volume-GUID junction target with the prefix-stripped shadow.
* fix(cli): classify drive-absolute link targets before the volume check (MUL-4252)
classifyTarget asked VolumeName before IsAbs, but VolumeName is
non-empty for a drive-absolute path (C:\dir) too. A junction's absolute
target was therefore misread as drive-relative and resolved against the
working directory — a nonexistent spelling — which errored the
resolution and left the escape to be judged lexically inside the
workdir. The IsAbs branch is asked first, and the load-bearing ordering
is pinned by TestClassifyTarget, a fixture-free table over the target
grammar that would have caught this without a filesystem.
* ci: run the util link-target classification table on Windows
TestClassifyTarget pins the branch ordering classifyTarget's link-target
grammar depends on, but windows-tagged tests in internal/util had no
execution point: the windows-execenv job only ran the guard tests in
cmd/multica. Scoped to the table by name, matching the job's pattern.
* test(cli): correct classifyTarget expectations for root and base shapes
The walk base is the volume root and the working directory's components
live in the segments for a drive-relative target, and the plain-relative
branch returns the link's directory verbatim without a trailing
separator. The expectations said otherwise; the code was right.
---------
Co-authored-by: weiweiwei <[email protected]>
|
||
|
|
c375f83c2e |
MUL-7418: add issue event and time wakeups (#8467)
* feat: add issue event and time wakeups (MUL-7418) Co-authored-by: multica-agent <[email protected]> * feat(wakeups): capture issue collaboration event lifecycle Co-authored-by: multica-agent <[email protected]> * fix(wakeups): preserve reply source when reopening threads Co-authored-by: multica-agent <[email protected]> * feat(wakeups): summarize sidebar and issue activity cues Co-authored-by: multica-agent <[email protected]> * fix(wakeups): restore disabled configurations with a two-way toggle Co-authored-by: multica-agent <[email protected]> * feat(wakeups): manage workspace wakeups in Autopilot Co-authored-by: multica-agent <[email protected]> * chore(wakeups): reserve migrations after current main Co-authored-by: multica-agent <[email protected]> * test(wakeups): align inventory fixtures with shared contracts Co-authored-by: multica-agent <[email protected]> * fix(wakeups): distinguish trigger conditions and execution results Co-authored-by: multica-agent <[email protected]> * fix(wakeups): suppress registration feedback and drain bounded evidence Co-authored-by: multica-agent <[email protected]> * fix(wakeups): align sidebar actions with titles Co-authored-by: multica-agent <[email protected]> * fix(wakeups): bound database work and unify shared issue visibility Co-authored-by: multica-agent <[email protected]> * feat(wakeups): filter mutation events by member or agent actor Co-authored-by: multica-agent <[email protected]> * refactor(wakeups): merge structured evidence and narrow receipt locks Co-authored-by: multica-agent <[email protected]> * feat(wakeups): let people edit wakeup instructions Co-authored-by: multica-agent <[email protected]> * fix(wakeups): align inventory controls and compact prompt editing Co-authored-by: multica-agent <[email protected]> * fix(mobile): handle deferred agent run status Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Lambda <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
8ce795b007 |
MUL-7480: Steer active agent turns from comments (#8605)
* feat: steer active agent turns from comments (MUL-7480) Co-authored-by: multica-agent <[email protected]> * fix: make comment steering hint driven Co-authored-by: multica-agent <[email protected]> * fix: harden active comment steering races Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Forge-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
Bohan Jiangandmultica-agent
|
9018df3efb |
feat(projects): set a repository's checkout ref from the UI (MUL-7504) (#8592)
* feat(projects): set a repository's checkout ref from the UI (MUL-7504) A project that tracks one delivery line could already pin the branch its tasks start from — `github_repo.resource_ref.ref` has been stored, sent to the daemon and honored at checkout since #4467. The UI only ever displayed it. Anyone working through the web, desktop or mobile app had to drop to `multica project resource update --ref` to set something the interface already showed as a property of the resource, and until they did, every task silently started from the remote default branch. This adds the missing input, on the three paths a repository is configured: - Create-project modal: a "Branch, tag, or commit" field per selected repo, shown once a URL is entered so the common default-branch case still reads as one field. - Resource panel: the same field in the attach form, plus a dialog on each attached repo — the row previously offered only delete. - Mobile: the field in the attach sheet, and the ref rendered in the list, which never showed it at all. Three details the shape of the existing code forced: - The ref renders on its own line rather than appended to the repo label. A custom name took that slot, so naming a repo hid the one signal that its tasks do not start from the default branch — and mobile is the only client that collects a name. - Edits spread the stored ref before overwriting. The server replaces resource_ref wholesale, so a payload carrying only `ref` is rejected for a missing URL. - Pasting a `.../tree/<branch>` URL now splits into the two fields. Mobile's URL check accepted that whole string and stored it as the clone URL, so the gap was already producing targets that cannot be cloned. Server-side, `ref` was stored with no validation beyond a trim, so a typo survived to checkout and surfaced as a daemon 500 naming its local repo cache, minutes later and to whoever ran the task rather than whoever typed it. validateGitRef applies the shape rules from `git check-ref-format` plus a length cap. Existence on the remote is deliberately still not checked: the server cannot see the repository, and an offline daemon or a private repo must not block saving configuration. Free text rather than a branch picker, by design. Nothing in the product can list a repository's branches today, and a picker could not express a tag or a commit SHA anyway. A "list remote branches" endpoint remains the follow-up #8572 proposes; it should stay an input aid, never the only way. Co-authored-by: multica-agent <[email protected]> * feat(projects): make the pinned branch the delivery target too (MUL-7504) Pinning a repository's starting point only solved half the problem. Nothing in Multica creates pull requests — `gh pr create` is the agent's own call, and without `--base` it targets the repository's default branch. So a project pinned to `release/2026-09` had its agents start in the right place and then open pull requests into `main`, carrying every commit the release line had that `main` lacked. Product review asked for the two halves to move together, so the brief now states both. The Repositories section names each repo's starting point and, when anything is pinned, says to deliver back to it. The Project Context bullet stops implying `--ref` is how you reach the configured revision — the daemon already applied it, and `--ref` is the override. The delivery rule is stated conditionally, because a pin is not necessarily a branch. The field accepts anything git resolves, and neither the server nor the daemon can tell a branch from a tag or a commit without asking the remote — which the product deliberately does not do. A tag has nothing to merge back into, so the agent is told to confirm rather than guess a base. That same ambiguity shapes how the UI narrows to branches. The field now says "Starting branch" and explains that tasks both start and deliver there, but the promise cannot be enforced: `v1.2.3` is a legal branch name and `main` is a legal tag. Only a full-length object id is unambiguous, and only that is declined, pointing at `multica repo checkout --ref` where a one-off revision belongs. The stored grammar is unchanged, so the CLI and API still accept tags and commits — the daemon has always resolved all three, and this is a UI-level product choice, not a data restriction. Two fixes from the same review: - An unpinned repository now reads "Default branch" instead of showing nothing. Clearing a branch used to make the line vanish, which looks identical to the setting never having existed — there was no way to confirm from the panel that a repo was deliberately on its default, or that the row had a branch setting at all. - The edit dialog says the change applies to newly started work. The ref is read when a task is claimed, so work already underway keeps the branch it started on, and the copy now says so rather than leaving it to be discovered. Co-authored-by: multica-agent <[email protected]> * fix(projects): close three gaps in the starting-branch flow (MUL-7504) All three found in review of the two commits before this one. **A resumed task could be told to deliver somewhere new.** The starting point in the brief is the project's CURRENT setting, read when the run was claimed. A task that started from `release/a`, left uncommitted work and resumed after the project moved to `release/b` kept its old checkout — the work is still branched off `release/a` — while the brief now named `release/b` and told it to deliver there. Following that would push the old line's work into the new one, and would break what the edit dialog promises: work already underway keeps the branch it started on. The brief cannot settle this alone, because whether a checkout gets reused is only known once `repo checkout` runs. It defers to the checkout instead, which already reports `Kept` and names the branch it is on. **Create Project accepted a branch it had already rejected.** The submit gate only checked the add-a-repo field, so a branch edited on an already-selected repo reached the payload with nothing but an inline error to show for it — and the server accepts commit ids by design, so nothing downstream caught it. Both the button and handleSubmit now check every selected repo; the button alone was never enough, since TitleEditor's onSubmit calls handleSubmit directly. **A second pasted browse URL was stored whole.** Splitting `/tree/<branch>` was gated on the branch field being empty, so pasting another repository's browse URL after one was already filled in saved the entire `/tree/...` string as the clone URL — a target that cannot be cloned. Normalising the URL is now unconditional in all three entry points. Whether to overwrite the BRANCH is the separate question, and the pasted pair wins: the branch field only appears once a URL is present, so a value sitting in it came from the previous URL rather than from something typed ahead of time. One defect the review surfaced indirectly: GithubRefField took an optional `id` and the per-repo instance in the create-project modal passed none, which detached the label from its input — leaving the field unnamed for a screen reader, and unreachable by getByLabelText, which is why the gap had no test. The id is now generated when omitted. Co-authored-by: multica-agent <[email protected]> * fix(execenv): stop naming the checkout as the resumed delivery target (MUL-7504) The resume warning added in the previous commit told the agent to deliver kept work "to the branch it actually started from, which the checkout names". The second clause is wrong. A kept checkout reports the branch the worktree is ON — resolved by `git symbolic-ref`, so the task's own `agent/<name>/<task>` branch. That is the HEAD of a pull request, never its base, and nothing in WorktreeResult carries the ref it was cut from. An agent following that sentence would pass its own branch as `--base` and open a pull request whose base equals its head. The warning now says what the reported branch actually is, and points at the sources that do settle the question: the base of the work's existing pull request, or the target the task states, and ask when neither does. It also moves out of the `if pinned` branch. A project cleared back to its default branch renders no starting point at all, yet a task resumed after that change still holds a checkout cut from the old one — gating the warning on a pin dropped it exactly where the mismatch is invisible. The one sentence that only makes sense alongside a listed starting point ("do not retarget it to a starting point listed above") stays conditional, so the cleared variant does not point at a line that is not there. Regressions cover both resume shapes — pinned A to B, and A cleared to the default branch — plus the specific wrong phrasing, so it cannot come back. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-agent <[email protected]> |
||
|
|
6b69b3710c |
Revert "MUL-7517: clarify squad leader identity framing (#8597)" (#8600)
This reverts commit
|
||
|
|
a51fd83072 |
MUL-7517: clarify squad leader identity framing (#8597)
* fix(squads): clarify leader identity framing (MUL-7517) Co-authored-by: multica-agent <[email protected]> * fix(squads): address briefing review feedback (MUL-7517) Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Forge-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
Bohan Jiang
|
d7009df8cc |
Revert "MUL-7300 fix(issues): preserve Quick Create original input (#8519)" (#8546)
This reverts commit
|
||
|
|
a6472044c3 |
MUL-7300 fix(issues): preserve Quick Create original input (#8519)
* fix(issues): preserve quick-create original input Co-authored-by: multica-agent <[email protected]> * fix(issues): validate quick-create origins Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Sol-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
Bohan Jiangandmultica-agent
|
29987fdda5 |
MUL-7461 test(execenv): clear MULTICA_TASK_CONFIG_ROOT so the suite is environment-independent (#8512)
Four tests in this package failed for every agent running the suite from
inside a Multica task, and passed in CI:
TestHermesMemoryStorePathLayout
TestPruneHermesMemoryStores
TestHermesSessionStorePathLayout
TestPruneHermesSessionStores
They isolate themselves by pointing HOME at a t.TempDir() and then assert a
path under $HOME/.multica. But HermesMemoryStorePath / HermesSessionStorePath
resolve through cli.ProfileDir, and multicaConfigRoot consults
MULTICA_TASK_CONFIG_ROOT before it ever reaches HOME — returning that root
directly, without the .multica segment. The daemon sets that variable for
every task it runs (taskMulticaEnvironment), so overriding HOME had no effect
and the tests were asserting against a branch they never took. CI never sets
the variable, so it stayed green and the trap only ever hit agents.
Clearing the variable once in TestMain makes the package resolve profile dirs
the same way in both environments. Preferred over a per-test
t.Setenv(cli.TaskConfigRootEnv, "") for three reasons:
- It also covers TestPruneHermesMemoryStoresDisabled and
TestPruneHermesSessionStoresDisabled, which passed for the wrong reason:
their stores were never created under the root the pruner scans, and the
assertions held only because retention <= 0 disables the pruner outright.
- t.Setenv cannot be called from a parallel test, so the per-test fix would
force future tests that resolve a profile dir to choose between running in
parallel and running in the right environment.
- New tests in the package are correct by default rather than by remembering
to copy a line.
A test that wants the task-local branch still sets the variable itself.
Verified on the same commit, whole package:
go test ./internal/daemon/execenv/ -count=1 ok
env -u MULTICA_TASK_CONFIG_ROOT go test ./internal/daemon/execenv/ ok
go test ./internal/daemon/execenv/ -count=1 -race ok
go vet ./internal/daemon/execenv/ ok
Co-authored-by: multica-agent <[email protected]>
|
||
|
Bohan Jiangandmultica-agent
|
d54300abaa |
refactor(claim): compare only title and description (MUL-7344) (#8488)
Status leaves the compared set, so the snapshot is now just the two body fields. The comparison answers one question — must the agent re-read the issue BODY? — and status never helped answer it: its current value ships on every claim, so the agent learns it without a read. Worse, comparing it worked against the feature. The workflow has the agent set in_progress when it starts and in_review when it delivers, so the agent's OWN bookkeeping came back the next turn as "changed: status" and demanded a body re-read that nothing in the body justified. Replaying this issue's 21 follow-ups, that alone cut "unchanged" from 17 to 4. The earlier rationale for keeping it — a free signal that somebody intervened — was wrong on both counts: it is not free, and the agent can see an intervention by comparing the shipped status against the value it last set itself. issueSnapshotVersion stays 1, and this time rows in the old shape DO exist. That is still correct: removing a field is backward compatible, because every field the new code reads is present in an old row and still means the same thing, and the stale "status" key is ignored. The comment now states the actual bump rule rather than "the set changed" — bump when an old row can no longer answer the current question, which means adding a field or changing how a value is derived, not removing one. A test pins the legacy shape comparing correctly in both the unchanged and the changed direction, so that claim is checked rather than asserted. No migration: the column is JSONB and the schema is unchanged. TestClaimTaskByRuntime_AssigneeAndPriorityAreOutOfScope becomes NonBodyFieldsAreOutOfScope and now moves status alongside assignee and priority, asserting the current status still ships with its NEW value — without that, the fixture's UPDATE could silently not apply and the empty changed-list would prove nothing. Co-authored-by: multica-agent <[email protected]> |
||
|
|
f39debf8d1 |
MUL-7344: skip the issue read and the comment scan when the server already answered them (#8377)
* feat(daemon): read the comment delta with one --since, not a roots scan (MUL-7344) A resumed follow-up turn whose claim carried a server-computed issue-wide comment delta was still told to run the workflow's two-step scan: a `--roots-only --summary` pass to discover which threads moved, then a `--thread ... --since ...` expansion per thread. That rediscovers, in two or more calls, an answer the server already has. `--since <anchor>` without `--thread` IS that answer. ListCommentsSinceForIssue returns every comment on the issue created after the anchor, in every thread and at every nesting level, with no folding, on the same `created_at > since` predicate CountNewCommentsSince counted with. It is a strict superset of what the roots scan could surface: roots-only `last_activity_at` is MAX(created_at) over a thread, so any comment able to move a thread's activity is already in the `--since` result. So BuildNewCommentsHint now hands over exactly one wide read and says it answers step 2's scan — the same rule BuildResumedCommentsHint already applies to a server-computed EMPTY delta. The optional second command stays a THREAD read on `--tail 30` for the reply itself; it is never the wide read, and it never combines `--thread` with `--since`, which drops the thread root. Brief step 2 and the multica-platform skill's bounded-reads rule both gain the matching static sentence, so an agent is not left choosing between the hint and two surfaces that contradict it. Cold start, dropped resume and unknown-delta paths are untouched, byte for byte: only an affirmative server report may stand in for the scan. Co-authored-by: multica-agent <[email protected]> * feat(claim): compare the issue against the last run so a resumed turn can skip the read (MUL-7344) A follow-up turn that resumes its provider session already holds the issue it read minutes ago, and was still told, unconditionally, to run `multica issue get` before doing anything. On an issue that did not move, that read returns bytes the session already has. The claim now answers the question instead. Each claim records the comparable state of the issue it built its payload from — status, assignee, priority, and sha256 of title and description — in a new `agent_task_queue.issue_snapshot` JSONB column, and compares against the snapshot the SAME agent's previous run on the SAME issue recorded. The response carries `issue_state_delta_known`, `issue_changed_fields`, and the current `issue_status` / assignee. Why a per-claim snapshot and not a `content_revision` on `issue`: a revision column has to be bumped by every writer that touches one of those five fields, and the first one missed reports "unchanged" forever with no symptom. A snapshot taken at claim time has exactly one writer. It also rides the anchor the comment delta already uses (agent_id, issue_id, started_at DESC), so both deltas on a claim mean "since your last run" in the same sense. The daemon renders one of three things, gated on a REAL resume: - unchanged → reported as workflow step 1's answer; the read survives only as "if resumed memory is not enough". - changed → the changed field NAMES plus the read, so the agent knows what to look for instead of diffing blind. - otherwise → the pre-existing instruction, byte for byte. `issue_state_delta_known` carries the same contract as `new_comments_delta_known` (MUL-6984), and for the same reason: a cold start, a missing prior snapshot, a read error, a foreign shape version and an old server all produce an empty changed-field list, identical to a genuine "nothing changed". Only the flag separates them, and only the affirmative one may stand in for the read. The compared set is named in full in the prompt rather than summarised as "the issue": labels, parent, due date, stage, project and metadata are NOT compared, and an agent told "unchanged" has to be able to see that. The snapshot write is NOT gated on the task being comment-backed — an assignment run that recorded nothing would leave the next run with no baseline. It shares the receipt's CAS, so a stale handler cannot write over a newer reclaim or after execution starts, and a snapshot that fails to encode is simply not recorded: the next run reports "not compared" and reads the issue. Co-authored-by: multica-agent <[email protected]> * fix(claim): renumber the migration past main and fold in review nits (MUL-7344) CI caught a real collision: main landed 471_comment_deleted_at through 474 while this branch was open, so 471_agent_task_issue_snapshot shared a numeric prefix and TestMigrationNumericPrefixesAreUnique failed. Renumbered to 475 after merging main; no other migration references it. Review nits from the PR review, all non-blocking, all cheap to take now: 1. Brief step 2's closing sentence told the agent that a resumed run's scan surfaces moved threads via `last_activity_at`. That is now true only on the path where the delta is UNKNOWN — when the server hands over a `--since` delta there is no scan to read it from. Rewritten to attach the `last_activity_at` guidance to the scan itself, wherever the scan happens. Done in this PR rather than deferred because this PR already moves brief bytes, so it costs no extra prompt-cache invalidation. 2. The two per-claim anchor reads are now one. GetLastTaskStartedAtForIssueAndAgent and GetLastTaskIssueSnapshotForIssueAndAgent selected the same row with the same WHERE and ORDER, differing only in projection; they are replaced by GetLastRunAnchorForIssueAndAgent returning started_at and issue_snapshot together. A comment-triggered claim is back to ONE anchor read, as before this branch. The saved round trip is the smaller half. The real gain is that "since your last run" is now a single resolved fact on a claim instead of two independently-resolved ones that could disagree. The shared read also shares a failure mode, and both consumers already degrade the same safe way: comment scan required, issue comparison unknown. 3. The hint's `%d new comment(s)` excludes the injected trigger and the agent's own comments; the `--since` read it hands over has no such filter, so the row count is legitimately higher. Narrowing the read to match would mean a second count-shaped query for no gain, so the text now states the mismatch and its direction outright instead of leaving the agent to reconcile two numbers. New test pins the merge: one claim, one prior run, both deltas resolved from that single anchor row. Co-authored-by: multica-agent <[email protected]> * fix(migrations): renumber the issue-snapshot migration to 478 (MUL-7344) main landed 475_issue_status_key_not_reserved through 477 while the previous renumber was in flight, so 475_agent_task_issue_snapshot collided in turn. Merged main again and moved to 478. TestMigrationNumericPrefixesAreUnique passes, and the full ledger applies cleanly from scratch through 478. Co-authored-by: multica-agent <[email protected]> * fix(claim): measure both deltas from the run whose session is resumed (MUL-7344) Review found a real correctness bug, not a performance one: the claim could tell a resumed agent "the issue is unchanged" while the session it handed back predated the change. Both deltas were anchored on the most recently STARTED run for (agent, issue). The session the daemon resumes is chosen by a different rule, and the two are not the same row: - GetLastTaskSession skips poisoned and retired sessions, so a newer run that died on an unprocessable conversation is passed over for an OLDER healthy one. - A manual rerun resumes the exact source task the operator clicked. So: an old session remembers description A, the user edits it to B, a newer run records B and then dies unresumable. The next follow-up resumes the OLD session but compares B against B, reports "unchanged", and the agent keeps working from a context that still says A. PriorSessionResumeUnavailable does not catch this — it flags a missing Codex rollout, which is only one of the ways the resumed session can be older than the newest run. The fix binds both deltas to the run actually being resumed. GetLastTaskSession now returns that row's started_at and issue_snapshot alongside its session, the rerun path takes them from its source task, and the delta computation moves after resume resolution so it can only ever read the adopted row. Where no session is adopted — no prior session, a session on another runtime, a forced-fresh retry, a rerun whose source failure poisoned the conversation — no delta is reported and the daemon performs the reads it always has. That makes the standalone anchor query unnecessary: the anchor is now a projection on a row the claim already fetched, so a claim reads no extra row at all (GetLastRunAnchorForIssueAndAgent is removed). The precondition is now visible in the fixtures: a prior run has to be resumable for a delta to exist, so the comment-delta tests seed a session_id. The regression test covers both paths and was verified to fail against the previous anchor — both sub-cases report an empty changed-field list, which is exactly the wrong "unchanged". Also renumbers the migration 478 → 491; main landed 478_issue_status_category_expand through 490 while the branch was open. Co-authored-by: multica-agent <[email protected]> * fix(claim): a never-started anchor row may date neither delta (MUL-7344) Final review found the last corner of the resumed-run bug. Binding the anchor to the resumed SESSION was necessary but not sufficient: GetLastTaskSession returns that session's latest TERMINAL row, and that row is not always a run that ran. A retry child can inherit the session, be claimed — which writes its snapshot — and then fail before the agent ever starts: runtime offline during prepare, cancelled while dispatched. Its snapshot describes the issue as of ITS claim, while the session's memory stops at the last run that actually executed. If the description was edited in between, the current issue matches that snapshot and the claim reports "unchanged" to an agent whose context predates the edit. The started_at gate now covers both deltas rather than only the comment one. The comment delta was already immune because it read StartedAt directly; the issue delta read only the snapshot, so it had no such guard. Hoisting the check makes the row's requirement explicit — a row that never ran dates nothing — and the inner duplicate is dropped so no reader has to wonder what each guards. Regression test reproduces the exact fixture and was verified to fail without the gate: known=true with an empty changed-field list, i.e. the false "unchanged". Also from review: - Adopt the no-adopted-session table (missing session, session on another runtime, force-fresh retry, poisoned rerun source). Each seeds a usable snapshot AND an unread comment, so a regression has something to report and the assertion cannot pass on an empty fixture. - The resumed-run regression test asserted a comment anchor that never executed: both sub-cases created only the trigger comment, which the count excludes, so NewCommentsSince was empty and the branch was dead. A comment between the two runs now makes it run and pins known/count/since. - Rewrite the migration header, which still described the old "(agent_id, issue_id, started_at DESC)" anchor and an index rationale for a column that is never a lookup key. Co-authored-by: multica-agent <[email protected]> * refactor(claim): compare title, description and status only (MUL-7344) The snapshot compared five fields. The comparison answers exactly one question — must the agent run `issue get` again? — so a field earns a place only if changing it alters what the agent does AND its current value is not already in the per-turn message. Assignee fails the second test: the claim ships the current assignee outright, and "is this mine now" is answered by that value, not by whether it moved. Priority fails the first: it is reachable only by reading the issue, but it does not change what the agent does. Both are dropped. Status stays even though it is also shipped as a current value, so comparing it is strictly redundant for "what is it now". It earns its place as a signal rather than a value: "changed: status" is the clearest evidence that somebody intervened between the runs — a push back from in_review to todo means the delivery was rejected — and it costs nothing to carry. Narrowing the set means a narrower claim, so the prompt now says "compared title, description and status". An issue whose only change is assignee or priority is reported as unchanged, which is correct and now pinned by a test that also asserts the current assignee still ships — without that second assertion the fixture's UPDATE could silently not apply and the test would pass on nothing. Snapshot version goes to 2, because the rule is that the version tracks the compared SET. No deployment has written a v1 row, but the gate is what makes narrowing safe in general: a v1 row read by v2 code is "not compared", which degrades to the unconditional read. Test fixtures now build snapshots at issueSnapshotVersion rather than a literal 1, so a future bump cannot quietly turn them into not-compared tests while they keep passing. The foreign-version case stays explicit at +1. Also renumbers the migration 491 -> 495; main landed 491 through 494 while the branch was open. Fifth renumber. Co-authored-by: multica-agent <[email protected]> * refactor(claim): keep the snapshot at v1 (MUL-7344) The previous commit bumped issueSnapshotVersion to 2 when it narrowed the compared set. That was the version rule applied past its purpose. The rule — "bump when the compared SET changes" — exists to stop a stored snapshot from being compared against a differently-shaped one. It protects DEPLOYED rows. This column is introduced by this same unmerged PR: the migration exists only on this branch, main has neither the column nor the writer, and no environment has ever stored a row with the wider set. There is nothing for the bump to protect, and shipping v2 as the first version the column has ever held would leave a reader hunting for a v1 that never existed. Back to 1. The `v` field and its unknown-version gate stay exactly as they were — those are what make a real narrowing safe later. The comment now says which version this is and why the review-time narrowing did not earn a bump, so the rule still has a stated precedent without a burned number. No behaviour change: the compared set is still title, description and status, and fixtures build at issueSnapshotVersion rather than a literal, so they followed the constant back down. Co-authored-by: multica-agent <[email protected]> * fix(claim): only a completed run may date a delta (MUL-7344) Final review found a window one step later than the never-started case. A snapshot is written at CLAIM time. The daemon then writes started_at (TaskService.StartTask) and only afterwards launches the provider. So a run can carry both a snapshot and a started_at while its prompt never reached the session: the daemon exits during prepare, the runtime drops, orphan recovery marks the row failed. Anchoring there compares the current issue against a snapshot describing an edit that session never saw — reporting "unchanged" across a real change, and dating the comment delta late enough to hide every comment older than that row's start. Reproduced on the real CreateRetryTask -> claim -> StartTask -> RecoverOrphanedTasksForRuntime -> follow-up claim path. Nothing on the row separates "failed after the provider ran" from "failed before it ran". session_id is inherited by CreateRetryTask at insert, so it does not mean the session was fed; started_at precedes the launch, so it does not mean the turn began. The only terminal status that PROVES delivery is 'completed' — a run cannot finish a turn it never started. So failed and cancelled sources now date neither delta. Their SESSION is still resumed exactly as before, and GetLastTaskSession still accepts them on purpose; those runs simply perform the context reads they always did. That is the cheap side of the trade: a follow-up after a failure or a stop is already recovering from something, while the case the optimization exists for — an ordinary follow-up after a healthy run — is untouched. Both conditions now live in newResumedRunAnchor rather than at the use site, so "which run may date a delta" is answered once, and GetLastTaskSession carries the status needed to answer it. Regression test is the reviewer's repro, verified to fail without the rule: known=true with an empty changed list AND a computed zero comment count, i.e. both a false "unchanged" and a false "no new comments". Also: - Renumber the migration 495 -> 499; main landed 495 through 498. Sixth renumber. - The migration's content_revision rationale named the old five-field set while arguing about hypothetical issue-row writers; it now refers to "any compared field", which is what the argument actually depends on. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
Jiayuan Zhangandmultica-agent
|
080b118f07 |
fix(daemon): isolate cached worktree Git identities (MUL-7423) (#8470)
* fix(daemon): isolate Git identities in cached worktrees (MUL-7423) Co-authored-by: multica-agent <[email protected]> * fix(repocache): preserve config lock ownership on handoff Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-agent <[email protected]> |
||
|
Bohan Jiang
|
d665f0ac60 |
MUL-7379 docs: retranslate the issue status model and correct category-wide behavior claims (#8438)
Documentation half of MUL-7379, after #8435 fixed the code. The stale pages did not merely use outdated wording — they promised behavior the platform stopped providing, in the three languages nobody re-read. zh/ja/ko issues.mdx were left on the retired seven-category inheritance model when MUL-7240 rewrote the English page. Each still said a new status "inherits that category's behavior in full", with a seven-row table promising that an in_review-category status finishes the autopilot run and auto-archives past failure notices, an in_progress-category status rolls back to todo on failure, and a backlog-category status parks — plus the worked example naming `Code Review`. An admin following the Chinese page would create that status and their autopilot runs would silently never finish. All three now mirror the English source, including the installed-client wire-enum paragraph they lacked entirely; ja and ko also gained the agent status-writing paragraph only en and zh carried. Status keys and category names stay lowercase English per the i18n glossary, which already listed the four values correctly. assigning-issues (all four locales) said only the `backlog` CATEGORY parks and a custom status in it behaves identically. WillEnqueueRun parks on the literal key, so a reader following that page parks work on their own unstarted status and gets exactly the run the page promises they will not. The same sentence named a `cancelled` category, which is now `closed`. inbox (all four locales) attributed failure-notice archiving to an `in_review` category that does not exist; it now states the handoff rule #8435 implemented, including why blocked is excluded. The Korean settings subtitle said five lifecycle categories directly above the four groups. CLI_AND_DAEMON.md presented the seven built-ins as the complete enum while validateIssueStatus is deliberately shape-only. One behavioral change, in its own commit with its test: the agent brief rendered each lifecycle group as "- `unstarted`: `backlog`, `todo` (built-in)". That leading backticked token used to always be a valid status key, so sharing the formatting was harmless; `unstarted`, `started` and `closed` are now category-only names that Resolve refuses, so the one line teaching agents how to write status invited `multica issue status <id> started` — a 400. The category label is now plain text and only settable keys keep backticks. Review caught two overclaims, both corrected: filtering resolves on the concrete status like boards, lists and sorting do, not on the category; and `done` is both a lifecycle category and a built-in key, so it is not among the names Resolve refuses. |
||
|
|
b62cce56da |
MUL-7359 perf(runtime): avoid echoing final comment bodies (#8390)
* perf(runtime): avoid echoing final comment bodies * refactor(daemon): keep the file-first guardrail as the Comment Formatting lede The receipt-mode rule opened the section, pushing the file-first guardrail (MUL-2904 / #4182 / MUL-4252) out of the lede. The guardrail is the section's correctness red line — a shell-mangled body is a wrong comment — while the receipt mode only decides how much of an already-correct comment is echoed back. Move the rule after the guardrail in both OS variants; it is now a single const so the two branches cannot drift. Co-authored-by: multica-agent <[email protected]> * docs(skills): drop the duplicated final-comment-receipt section issues.md states its own charter as "Product contracts the runtime brief does not fully encode", and SKILL.md gives the brief ownership of "when to comment". The brief's ## Comment Formatting now carries this rule in full, so the copy here duplicates a brief-owned instruction against the single-owner convention (MUL-6984 #7971, MUL-5825 #6517). It was also unreachable by the documented routing: the section was added to the file's own table of contents, but SKILL.md's Routing row for issues.md was not updated, so no task would route here for this topic. Co-authored-by: multica-agent <[email protected]> * fix(daemon): gate final-comment cleanup on the post succeeding Agents hand the reply cookbook to one shell call. With cleanup as its own statement, a failed post is followed by a cleanup that SUCCEEDS, so the call exits 0 and ./reply.md is deleted. Under `--output table` a failed and a successful post both write nothing to stdout, which leaves the exit status as the only machine-checkable signal — masking it lets an agent end its turn believing an unposted final result was delivered, and drops the body it would need to retry with. The brief already told agents to check the posting command's exit status; the rendered snippet made that impossible. POSIX chains with `&&`. PowerShell 5.1 has no `&&` (it landed in 7), so the Windows variant propagates $LASTEXITCODE instead. Both keep ./reply.md on disk when the post fails. The guard EXECUTES the rendered snippet against a stub CLI: this is a shell-composition property, and the handler-level test can only prove the command returns an error, not that the pasted block preserves it. Verified to fail on the previous two-statement form. The pwsh case runs wherever PowerShell is installed, including Linux runners, and skips otherwise. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
d77a1c5ea7 |
fix(prompts): remove status wire details from operational guidance (#8401)
Co-authored-by: Lambda <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
aac85ab263 |
fix(daemon): preserve issue status catalog wire compatibility (#8378)
Co-authored-by: Lambda <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
686957e743 |
MUL-7271: test(server): cut slow Go test time further (follow-up to #8295) (#8318)
* test(server): reduce slow unit test waits Co-authored-by: multica-agent <[email protected]> * test(server): cut slow Go unit test time further Follow-up to the first pass on MUL-7271. Test-only changes plus a few package-private seams whose production values are unchanged. - Re-executed race-built test binaries no longer pay the race runtime's 1s atexit sleep (GORACE=atexit_sleep_ms=0 for child processes only). - pkg/agent: event-driven or shorter test-only bounds for handshake, drain and idle graces; redundant timeout variants merged. - daemon/execenv: git fixtures copied from a template repo built once; tests that re-ran the same worktree lifecycle folded into their supersets; ForkLock-guarded fake-CLI writes so parallel tests cannot hit ETXTBSY. - handler/service: cap and paging tests shrink their caps instead of seeding thousands of rows; lock waits probe pg_locks instead of sleeping; one shared pgxpool per service test binary; cheaper cron and RSA fixtures. - wecom: the phishing-link sweep reuses one parser and derives CRLF from LF with a sampled cross-check; fake sockets acknowledge instead of waiting out the ack timeout. - migrate: the migration corpus is parsed once; isolated DB tests run in parallel. Co-authored-by: multica-agent <[email protected]> * test(server): remove transport timeout unit tests Co-authored-by: multica-agent <[email protected]> * test(server): restore two non-timeout tests and make a bound load-bearing Follow-up to the review of #8318. All test-only; no production code changes. - daemon: restore the three read-liveness cases that the transport-timeout sweep took with it. They assert that a ping, a pong and an application frame each EXTEND the read deadline, so a healthy wakeup socket survives without traffic of our own — the deadline never fires on the happy path, which is why they are not transport-timeout tests. Verified they fail (all three) when the extension calls are removed from wakeup.go, and pass in 0.57s together. The pure "times out without peer traffic" case stays deleted. - pkg/agent: restore the parent-DEADLINE half of the opaque-env redaction pair. codex.go treats a parent deadline and a parent cancellation as one ending; only the Canceled arm was left covered. The fake now writes the secret to stderr before reading the handshake, so the check no longer races our own initialize write, and the budget is 2s against a measured 430-465ms launch. Verified it fails when the !contextEnded guard is removed. - pkg/agent: TestRunCollectQuietDoesNotWaitWhenTheAnswerIsIn had its bound widened to a literal 5s, which the shrunk 750ms drain grace can never reach — the test could no longer fail. It now raises the drain grace to 10s for its own duration and asserts under 3s: healthy runs take 0.19-0.44s, and a broken short-circuit takes 10.19s and fails. - wecom: gofmt the import block in relay_settle_budget_test.go. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Sol-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> Co-authored-by: J <[email protected]> |
||
|
|
7dafc0cd62 |
MUL-7240: Unify issue lifecycle into four stored categories (#8261)
* feat: simplify issue status categories Co-authored-by: multica-agent <[email protected]> * feat: adopt four stored issue lifecycle categories Co-authored-by: multica-agent <[email protected]> * fix: preserve visible work when upgrading saved category preferences Co-authored-by: multica-agent <[email protected]> * docs: align lifecycle comments with stored and wire contracts Co-authored-by: multica-agent <[email protected]> * test: align mobile grouping expectations with four categories Co-authored-by: multica-agent <[email protected]> * fix(issues): group user views by concrete status keys Co-authored-by: multica-agent <[email protected]> * fix(settings): simplify status editor copy and category selector Co-authored-by: multica-agent <[email protected]> * feat(statuses): allow ordering built-ins and unify row actions Co-authored-by: multica-agent <[email protected]> * feat(statuses): configure custom status icons independently of category Co-authored-by: multica-agent <[email protected]> * fix(statuses): dismiss built-in status notice Co-authored-by: multica-agent <[email protected]> * fix(issue-status): require moving issues before archive and polish controls Co-authored-by: multica-agent <[email protected]> * fix(ci): renumber issue status migrations after main updates Co-authored-by: multica-agent <[email protected]> * fix(statuses): isolate archive inspection and align catalog behavior Co-authored-by: multica-agent <[email protected]> * fix(statuses): respect injectable catalog for table grouping Co-authored-by: multica-agent <[email protected]> * refactor(statuses): simplify built-in skill and runtime guidance Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Lambda <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
2441638c05 |
MUL-7284: fix(daemon): keep an existing checkout's work when repo checkout runs again (#8309)
* fix(daemon): keep an existing checkout's work when repo checkout runs again Re-running `multica repo checkout` on a path that already held a checkout ran git reset --hard and git clean -fd and moved it to a new branch from the default branch; isolated checkouts also deleted earlier agent/* branches. Uncommitted changes, untracked files, and unpushed commits were lost with output that still read as a normal checkout. An existing checkout that holds work, or is already on this task's branch, is now kept exactly as it is and only its remote refs are fetched. The result reports that it was kept, with its branch and uncommitted/unpushed counts, and the CLI says so. --fresh restores the old start-over behavior. Isolated-mode pruning keeps agent/* branches with unpushed commits, and a linked worktree holding work is no longer removed to migrate it. MUL-7284 Co-authored-by: multica-agent <[email protected]> * fix(daemon): carry a migrated linked worktree's unpushed branch into the isolated checkout --fresh migration of a linked worktree to an isolated checkout removed the worktree and seeded a new clone whose copied branches are all deleted, so a branch holding unpushed commits stayed only in the shared cache: out of the agent's reach and dropped by the next GC. Carry that one branch into the new checkout (no other cache branch), imported before the task branch so a same-named task branch moves to a timestamped name. Align the agent-facing --fresh wording: it discards uncommitted and untracked files and starts a new branch; commits stay on the old branch, but push any still needed first, since a linked worktree's old branch lives in the shared cache that GC prunes. MUL-7284 Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
mrlonely
|
6194a8b3a7 |
MUL-7226 fix(daemon): decline Codex reuse when home preparation fails (#8236)
When the task-local Codex home cannot be prepared on the reuse path, Reuse now returns nil so the daemon falls back to a fresh Prepare, instead of returning an environment with an empty CodexHome that fails at launch with "task CODEX_HOME is missing". Declining reuse gives up the prior Codex session and the old workdir's contents; a fresh Prepare that fails the same way still stops the task. Co-authored-by: mameikagou <[email protected]> |
||
|
|
5f1f297d57 |
MUL-6966 chore(brief): stop teaching agents to use issue metadata (#8102)
* MUL-6966 chore(brief): stop teaching agents to use issue metadata
Phase 1 of retiring the per-issue metadata KV bag: remove every piece of
official guidance that recruits agents into writing it, while leaving the
UI, API, CLI and stored data fully working.
The concept was added when the platform had no custom properties and no
custom statuses. Both exist now, so metadata's stated purpose ("custom
key-value state a future run re-reads") no longer names anything the
product does not model better — what is left is an unstructured bag the
brief actively invites agents to fill, with no defined consumer and no
way for a user to notice it drifting out of sync with status, PR links
and comments.
Removed from the runtime brief:
- the `## Issue Metadata` section and its dispatch
- the three `multica issue metadata …` bullets in Available Commands
- the metadata clauses on workflow steps 1 and 5
Removed from the multica-platform skill:
- `## Metadata: durable custom state` in references/issues.md
- the metadata framing of custom properties, replaced by a
where-state-belongs rule (property / status / comment)
- the routing-table and legacy-redirect mentions
Kept deliberately: the one negative `pr_url` clause. `multica issue get`
still returns whatever an older run left in the bag, so the warning not
to infer PR state from it is still load-bearing.
Every presence assertion becomes an absence assertion — no test coverage
is dropped. TestInjectRuntimeConfigIssueMetadataSectionScope is inverted
into TestBriefCarriesNoMetadataGuidance, which checks all five task kinds.
Brief size: 15443 → 14264 bytes on a comment-triggered issue run (~315
tokens off every run's always-loaded context).
Phase 2 — removing the UI, CLI and API surface — waits until the
remaining consumers are known to have migrated.
Co-authored-by: multica-agent <[email protected]>
* MUL-6966 chore(skill): drop the pr_url name from the stale-PR-state guard
Review feedback: the surviving `pr_url` mention was not worth its cost.
The guard it sat in is "query the link table, do not infer PR state from
branch names, GitHub search, memory, or something stale" — and that rule
is complete without naming a key. Spelling the key out is itself a way of
teaching the vocabulary: an agent that reads "do not trust the `pr_url`
metadata key" has just learned there is one. Phase 1 is about not putting
that vocabulary in front of agents at all.
The stale-data case is unchanged and still covered: `issue get` returns
whatever an older run left on the issue, and the clause now warns about
that generically.
references/issues.md now contains neither "metadata" nor "pr_url", and
the test asserts exactly that — a blanket vocabulary ban replaces the
five string-by-string checks it had after the previous commit.
Co-authored-by: multica-agent <[email protected]>
* MUL-6966 chore(skill): tell the legacy redirect the truth about metadata
Review nit: the stub promised "Nothing was dropped in the move — the
contracts were reorganized, not shortened." The metadata guidance has now
been dropped, and the daemons that reach this stub are exactly the ones
whose frozen brief still sends them here for it. The one place the claim
is read is the one place it is false.
It now names the exception and states the rule that replaces it —
property for state a person filters by, status for the stage, the result
comment for everything else — so an old daemon's stale pointer lands on
an answer instead of a gap.
This is the one file where saying the word is free: the stub is served
only to pre-merge daemons, whose brief already teaches metadata in full.
Mirroring the note into references/issues.md would put the vocabulary in
front of every new agent forever to serve a window that closes.
Pinned by TestLegacyRedirectsFollowTheDaemonsBrief.
Co-authored-by: multica-agent <[email protected]>
---------
Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
|
||
|
|
2efe1b2a43 |
MUL-6984 fix(daemon): let workflow step 2 alone decide the comment scan (#7972)
* MUL-6984 fix(daemon): let workflow step 2 alone decide the comment scan
The brief's step 2 called the catch-up scan mandatory while the per-turn
hints in the same context made it conditional ("Need cross-thread
background?", "Only if you need context from the other threads", "If your
reply depends on thread context"). Step 2 now owns the modality; the hints
carry facts and this turn's exact commands and hand the scan over as the
wide read. Hint dispatch keys on PriorSessionID before the delta so a run
whose resume the daemon dropped takes the fresh-session path instead of the
resumed delta hint. The assignment pointer drops the parenthetical that
implied only assignment turns treat the read as mandatory.
Measured on 537 comment-triggered runs: 1 in 10 scans opened a thread the
prompt had not named; 0 of 36 non-scanning runs did; scan median 434 B.
Co-authored-by: multica-agent <[email protected]>
* fix(daemon): only waive the comment scan on a delta the server actually computed (MUL-6984)
Addresses review on #7972.
[P1] NewCommentCount == 0 was read as "nothing else was said on this issue",
and used to tell the agent its scan is already answered. The producer in
buildClaimedTaskResponse is best-effort: it writes the count fields only when
the anchor lookup and the count query both succeed AND the count is positive.
A failed anchor read, a failed count query, a cold start with no prior run, a
true zero, and an old server that never sends the fields therefore all arrive
as the same zero — and only the true zero looked at the issue. On a resumed
run a single failed count query was enough to delete the mandatory scan
exactly when the run could least afford it.
The claim now carries new_comments_delta_known, set on the success path
whatever the count. The count fields still stay suppressed at zero (there is
no delta hint to render from a zero), so the flag is the only thing that
separates a checked zero from an unchecked one. Absent on old servers, which
reads as "not authoritative" and keeps the scan — the safe direction in both
mixed-version pairings. The resumed path with no authoritative delta now
renders BuildResumedUnknownDeltaCommentsHint, which keeps the session facts
and hands the scan over as a command instead of claiming it is answered.
Workflow step 2 is tightened to match: only an explicit server report waives
the scan, and silence is not that report.
[P2] The warm path was selected on PriorSessionID != "" alone. MUL-5305 lets
the server withhold a more recent Codex session whose rollout is missing and
return an OLDER fallback session instead: PriorSessionResumeUnavailable is
true while PriorSessionID is non-empty. That older session may resume cleanly,
but it is not the turn this run continues from, so the run was taking the warm
path — and on an empty delta could skip both the full trigger-thread read and
the scan — on the strength of context it does not have. It now requires
!PriorSessionResumeUnavailable and reconstructs from the record instead, which
is what the continuity notice already tells the user happened.
Tests cover the states the existing suite could not see: an older fallback
session with every other warm-path precondition satisfied, resumed-with-delta-
unavailable versus resumed-with-authoritative-zero as siblings, and a
server-side computed zero carrying the flag. Each was verified against the
un-fixed code, where it fails. TestBuildPromptResumedNoDeltaDoesNotForceThread-
Read now sets the flag: its subject is the waiver branch, which is exactly the
branch that requires an authoritative answer.
Co-authored-by: multica-agent <[email protected]>
* docs(handler): correct the new-comment delta's scope on the claim wire (MUL-6984)
Review should-fix on #7972. NewCommentCount's doc comment still described it as
"trigger-thread comments since last run", while the query that produces it
(CountNewCommentsSince: "ACROSS THE WHOLE ISSUE, every thread, not just the
triggering one"), the daemon-side mirror field, and the NewCommentsDeltaKnown
comment directly beneath it all say issue-wide. One struct carrying two
mutually exclusive readings of the same field is worse than either.
Also records on NewCommentsSince that it is suppressed together with the count
at zero, so the next reader does not mistake a non-empty anchor for the signal
that the server looked — NewCommentsDeltaKnown is that signal.
Comment text only.
Co-authored-by: multica-agent <[email protected]>
* fix(daemon): stop calling an older-fallback resume a fresh session (MUL-6984)
Under MUL-5305 the server hands back an OLDER session with the continuity
gap flagged, and the daemon resumes it (ResumeSessionID is not gated on the
flag). The cold hint opened with "This run starts a fresh session on this
issue" and the readable continuity notices said "you are on a fresh one";
both were false in that state, and a test pinned the first.
The cold hint now states nothing about the session — none of the four runs
it serves (first run, dropped resume, explicit fresh, older fallback) makes
a different decision on it, and all four reconstruct from the issue record
with the same two reads. The three readable notices say the run "does not
continue" the lost session and that the memory gone is "from the turns
that did not come back", true in both the fresh and the fallback case.
TestBuildPromptOlderFallbackSessionRequiresReconstruction (renamed) asserts
the reconstruction commands and the notice, and bans "fresh session" and
"fresh one"; the resumed-path negative anchors now key on a phrase only
the cold hint carries.
Co-authored-by: multica-agent <[email protected]>
---------
Co-authored-by: Bohan-J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
|
||
|
|
0242f37154 |
MUL-6986 refactor(skills): merge built-in skills into one platform skill and drop source maps (#7968)
* MUL-6986 refactor(skills): merge built-in skills into one platform skill
Built-in skills are read on the user's machine, which has no checkout of
this repo. Half the shipped payload by bytes was a source-location index:
eight `references/*-source-map.md` files (103KB, 1084 lines) whose content
was `internal/handler/github.go:687` style citations, plus "verification
commands" like `go test ./internal/service` and
`grep -n 'func derivePRState' internal/handler/github.go`. Four SKILL.md
bodies also carried source paths and Go identifiers, and every one of them
pointed the reading agent at its source map ("When in doubt, read that
file"). One source map even leaked an internal branch name.
That was not an oversight: CLAUDE.md required updating a source map in the
same PR as any behavior change, and this test file asserted all seven
existed. The reasoning against it was already written at the top of this
file — anything inside a skill directory is shipped to agent machines, so
evals must stay in _test.go — it just was never applied to the maps.
The nine skills were also split along backend modules, not along how the
work arrives. An agent has one entry point ("I am doing something on the
Multica platform"), so nine descriptions competed in the always-loaded
listing with overlapping boundaries, a cross-domain task activated three
skills, and Quick start / Core model / CLI / Side effects were restated in
most of them.
- Delete all eight source maps.
- Merge the eight platform skills into `multica-platform`: an 81-line
routing SKILL.md (positioning, the eight shared invariants that were
duplicated across the old bodies, and a table naming one reference per
domain) plus `references/{issues,mentions,agents,squads,autopilots,
projects,runtimes,skill-import}.md`. This is the layout the skill spec
documents for `references/` — a large skill split by domain, loaded on
demand — rather than an evidence layer for source.
- Strip every source path, Go identifier, test name, migration ordinal and
tracker citation from the payload; the behavior contracts themselves are
unchanged, restated in user-observable terms.
- Scope `multica-onboarding` to Mika. It is one agent's procedure, and
every agent was paying for its description and its files.
- Add TestBuiltinSkillPayloadCarriesNoSourceReferences so this cannot
regress, and re-home the ~100 contract anchors onto the reference that
now owns each one. Anchors match with whitespace collapsed, so a
paragraph reflow no longer reads as a deleted contract.
- Point the brief's two skill references and CLAUDE.md at the new layout,
and add one recall line to the brief's Skills section: eight
descriptions became one, so an agent has one name to guess instead of
eight. It is emitted only when a listed skill resolves to exactly
`multica-platform`, so a workspace skill taking that slug silences the
pointer rather than aiming it at a stranger's skill.
Payload per ordinary agent goes from ~211KB across nine skills to ~100KB
in one; the always-loaded description budget from 1,616 chars to 229.
Co-authored-by: multica-agent <[email protected]>
* MUL-6986 fix(skills): resolve the brief's platform pointer from the built-in
Review found the brief's recall line claimed a fail-closed behavior it did not
have. The claim was that a workspace skill taking the `multica-platform` slug
would silence the pointer; in fact the workspace skill is listed first, wins the
bare slug, and the pointer named it — handing the agent a stranger's skill as
the source of Multica's platform contracts, while the built-in sat at
`multica-platform-multica` and became unreachable.
Documenting that would have left a recall hole, and recall is the one thing this
merge may not cost: eight descriptions became one, so the agent's path to a
platform contract has to stay intact.
Fixed at the source instead. Built-in skills now declare `Source` at load time
(the bundle builder already derived it for the refs path, but the older inline
claim path appended them raw), the daemon carries `Source` into the brief's view
of a skill instead of dropping it, and the pointer resolves the slug from the
built-in's own entry. A collision now points at the built-in wherever it landed.
No bundle hash changes: the bundle builder normalizes `Source` to this value
before hashing.
Also adds TestPlatformSkillDescriptionNamesEveryDomain, which fails if a
reference ships without its trigger word in the description — the merge's recall
guarantee as a test rather than an assertion.
Review nits:
- TOCs for projects.md and runtimes.md, both over the 100-line bar.
- Inlined the two reference-to-reference links in mentions.md, so the routing
file's "they do not depend on each other" is true.
- Example issue keys use MUL-123, not MUL-2759 — a real internal issue.
- Fixtures naming the deleted skills now name live ones.
- Recorded where the description boundary sits: task nouns are trigger wording,
body section headings are the inventory the cap rules out.
Co-authored-by: multica-agent <[email protected]>
* MUL-6986 fix(skills): keep the brief's skill pointers resolvable across daemon skew
The runtime brief is assembled by the DAEMON, from a binary the user installs on
their own schedule. Deploying a backend does not rewrite it, and backend
upgrades do not force a daemon upgrade, so renaming the built-ins left two brief
lines — the metadata write discipline and the sub-issue playbook — naming
`multica-working-on-issues`, a skill this server no longer ships. The window is
open-ended, and a pointer to a skill that is not installed is worse than no
pointer: the agent hunts for it and may skip the contract when it misses.
Both skew directions happen, and each needs the fix on the side that is still
deployable:
- Old daemon, new backend. That daemon's brief text is frozen, so the fix has to
come from the server: a daemon that does not advertise `platform-skill-v1`
additionally receives a redirect stub under the old name, carrying nothing but
the new location. A current daemon receives nothing extra, so the stub retires
itself per install as daemons update — no cleanup release to remember, and no
cost paid by daemons that never needed it. Gated on capability, never on a
version string, matching how every other daemon skew is handled here.
- New daemon, old backend. The server has no idea the merge happened, so this
side has to cope. The brief now resolves both pointers from the skills the
task actually received: the merged skill if present, else the pre-merge one,
else no pointer at all. The `--status todo` / `--stage <N>` / secrets rules
stay unconditional either way — only the "read more here" clause is
conditional.
Resolution matches on built-in source as well as slug, so a workspace skill that
sanitizes to the same name cannot be advertised as the source of Multica's
platform contracts.
The redirect stub is deliberately a signpost with no contracts of its own: a
stub that grew content would rot against references/issues.md precisely on the
installs that cannot be updated from here, and a test caps its size and forbids
supporting files. It also rides through the template, strict-frontmatter and
source-leak suites like any other shipped payload.
Co-authored-by: multica-agent <[email protected]>
* MUL-6986 fix(skills): route by domain, not by count, and test the redirect wiring
Review must-fix: the routing text turned "load references on demand" into "load
exactly one". `SKILL.md` said to open "the ONE reference that matches your
task", the brief said "open the one reference its routing table names", and a
test pinned the singular.
That is wrong for the tasks the merge was argued on. Creating a squad, assigning
it an issue, then writing a mention needs squads.md, issues.md AND mentions.md;
wording that implies one would have the agent act on contracts it never read —
exactly the recall loss this work is not allowed to cause. Routing is now by
domain: open what the task touches, usually one and sometimes a few, and never
all eight out of uncertainty. The tests pin the domain framing and reject the
singular forms.
Also adds the wiring test the review found missing. The service test passed the
decision in as a bool and the brief test hand-built a skill set, so both ends
were covered while the middle — X-Client-Capabilities on the claim, through the
handler, into the delivered payload — was not. A break there would have left
every existing test green while an old daemon silently lost its redirect.
Covered across both claim branches, since each assembles the built-in set
separately, and verified to fail when the wire is cut. The daemon's capability
header test now pins platform-skill-v1 alongside the worktree gate.
Non-blocking review cleanups:
- The invariant claimed every domain has `list` and `get`; there is no
`runtime get` and a mention is not a gettable entity. It now points at the
read-only commands the opened reference actually names.
- Two comments still said the `multica-` prefix means a built-in slug can never
collide with a workspace skill. This PR ships the fix for that exact
collision, so they now say what is true: the prefix namespaces, nothing
reserves it server-side, and the brief resolves pointers from built-in source
rather than trusting the bare name.
- Recorded the allowed-tools tradeoff: merging eight skills merged their tool
declarations, so the router declares Bash(git *) / Bash(gh *) for domains that
do not need them. Dropping them would regress the issue workflow that runs
`gh pr`; today the daemon's global permission mode means this grants nothing
the agent did not already have.
Co-authored-by: multica-agent <[email protected]>
* MUL-6986 docs: drop the CLAUDE.md payload rule that a test already enforces
CLAUDE.md is loaded into every task in this repo and asks to stay short:
rules there should be hard to infer from code or easy to get wrong. This one
was neither — TestBuiltinSkillPayloadCarriesNoSourceReferences fails CI on
every class it named, and its failure message already carries the rule and the
reason at the moment someone violates it, which is both cheaper and impossible
to drift from.
The line above it stays: nothing enforces "update the reference in the same
PR" when a CLI flag changes.
Co-authored-by: multica-agent <[email protected]>
* MUL-6986 refactor(skills): accept the skill-name collision instead of handling it
Product decision: a workspace skill whose name sanitizes to exactly
`multica-platform` is rare enough not to design around. Half-handling it was
costing more than the risk — the brief resolved collisions dynamically while the
redirect stub could not (the stub is server-authored text, and the final slug is
only assigned when the daemon writes it to disk), so every review round found
another place that "did not handle it". Either handle it everywhere or nowhere;
this takes nowhere, and states the assumption where the pointers are.
Removed:
- builtinSlug matches the exact name again, instead of built-in source plus a
slug prefix.
- SkillContextForEnv.Source, the line carrying it through convertSkillsForEnv,
and the Source stamp on loaded built-ins. All three existed only for the
collision check, and dropping the stamp also removes a wire-facing change:
the bundle builder derives "builtin" from an empty id/source exactly as it
did before.
- The three collision test cases.
- The "the prefix does not guarantee uniqueness, so resolve from source"
narrative, replaced by the assumption it stands in for: `multica-` is the
platform namespace, pointers name built-ins by bare name, and nothing
reserves the prefix server-side yet. When that becomes real the fix is to
reject the prefix at skill create/import, not to make every pointer
defensive.
Kept, because it is version skew rather than collision and its window has no
upper bound: the `platform-skill-v1` capability, the redirect stub for daemons
that predate the merge, the brief's new-then-legacy-then-none resolution, and
their tests.
Co-authored-by: multica-agent <[email protected]>
---------
Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
|
||
|
|
5f0e0f2293 |
fix(daemon): stop rejecting preparation fields from an older parent (MUL-7029) (#7994)
Every task on an upgraded host failed with reuse execution environment: execenv: preparation helper failed: exit status 1: decode preparation request: json: unknown field "HandoffNote" The preparation helper is spawned from the running daemon's executable path, so parent and helper are the same program but not necessarily the same build: every upgrade path replaces that file under a live daemon. Desktop swaps the bundled/managed CLI and defers the restart while the daemon is busy; brew and manual replacements are picked up by trySelfReload only on its next idle tick. Until the daemon re-execs, an older parent talks to a newer helper. decodePreparationRequest ran DisallowUnknownFields, which turned that ordinary window into a hard failure of every task on the host. Removing TaskContextForEnv.HandoffNote (#7626) was enough to trigger it: the field was untagged and had no omitempty, so installed daemons kept sending "HandoffNote": "" and the new helper refused the whole request. The request is now decoded on the same best-effort terms as the response the parent already reads back — a param the other side does not know about is dropped, not fatal. Strictness bought nothing here: parent and helper are both this package, and the shape is pinned by its own tests. Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
49098deb6f |
MUL-6984 refactor(prompt): give each duplicated instruction one owner (#7971)
* refactor(prompt): give each duplicated instruction one owner (MUL-6984) Four cases where the same data or rule was rendered two or three times into one context window, following the sibling-block removal in #7937. Each is resolved by picking the surface that owns it and pointing the others there — no behavior change beyond what an agent stops reading twice. The rule used to pick the owner is the one MUL-5377 already set for issue tasks: the runtime brief holds what is true of this KIND of task (stable, and it is the prompt-cache prefix), the per-turn user message holds what is true of this RUN, and the workdir sidecar holds nothing. Autopilot, quick-create and squad-leader paths had never been cleaned up against it. Autopilot run data (run id, autopilot id, title, source, trigger payload, instructions) was rendered in the brief, the per-turn message, and the sidecar. Now per-turn only; the brief keeps the run-only framing and the shared issue-command guard. This also settles a contract violation: those are per-run values, and the brief promises not to carry any. MUL-5696 already caught the two copies drifting on the guard sentence and fixed that one line with a shared constant. The payload is now capped at 8 KB with an explicit truncation notice — it is written by whatever system fired the trigger, so nothing here bounds it, and a clipped JSON object read as a whole one is worse than a short one. The squad leader no_action rule was written four times: the Squad Operating Protocol, the per-turn prompt, brief workflow step 4, and brief ## Output. The four had already drifted — only some carried the MUL-6622 escape hatch that keeps a FAILED `squad activity` call from ending the turn in silence. The protocol is now the single statement (it is the only document sent exclusively to leaders); the two brief sites and the per-turn reply imperative carry the EXCEPTION to their own imperative and point back at it. Quick-create's five run/output rules were in the brief's Workflow, the brief's ## Output, and the per-turn "Output format" block; the --description-file /tmp guidance was in the brief's Available Commands and per-turn. The brief owns both now — its own comment says these guardrails must hold "even if the user message is missing" — and per-turn renders only the field values the modal picked. .agent_context/issue_context.md is deleted outright. Nothing read it: the rendered CLAUDE.md never mentioned .agent_context, no per-turn message pointed at it, and no provider discovers it. Its writer's own comment said "the runtime brief already carries every fact this file would". With it gone the directory is created only when the default provider needs .agent_context/skills, so a local_directory task no longer leaves an empty managed directory in the user's checkout. Compatibility: prompt text only. No wire field, CLI flag, or DB column changes, so no server/daemon version coupling. Tests follow the text to its new owner rather than being deleted: the protocol's no_action rule is now pinned in internal/handler, the quick-create rules in the brief's own suite, and the payload cap has its own coverage including the rune-boundary cut. Co-authored-by: multica-agent <[email protected]> * fix(prompt): drop the autopilot payload cap, pin the moved invariant (MUL-6984) Addresses review on #7971. The 8 KB trigger-payload cap is removed. Its rationale was wrong twice over: webhook ingress already bounds a body at 256 KiB (handler.maxWebhookBodyBytes), so the payload was never unbounded from Multica's side; and because de-duplication made the per-turn message the payload's ONLY rendering, truncating there dropped task input with no way back — GET /api/autopilots/{id}/runs/{runId} returns the full payload but no CLI command exposes it, and `autopilot runs` lists runs without payloads. A legitimate 9 KB webhook would have silently lost fields the autopilot instructions might name, and the "JSON is incomplete" notice only prevents misreading, it does not recover anything. The cap belongs with an agent-facing read-back command and a threshold taken from real payload sizes, not as a side effect of removing duplicates. Rendering it once instead of twice is still a strict improvement over the pre-PR behavior. TestBriefByteIdenticalAcrossRunsForEveryKind could not see the invariant this PR actually moves: its autopilot row pins run-1 / ap-1 and every variant mutates only the generic resume / initiator / connected-app fields, so putting the title, source, payload or description back in the brief left it green. TestAutopilotBriefByteIdenticalAcrossRunScopedFields adds a second and third run that vary all of them — including a different autopilot id — asserts the brief is byte-identical, and asserts the brief contains none of those values. That second half matters: byte-identity alone also holds if the values reach no surface at all. Verified against a deliberately reintroduced field, where it fails as intended. TestBuildPromptAutopilotRunOnly gains the source and payload assertions so the per-turn message is pinned as their sole owner. Contract comments that still described the deleted sidecar are corrected: agent.MinHandoffCLIVersion (the gate always measured the opening prompt — no provider read the sidecar), the handoff_note field docs on both sides of the claim wire, the claim handler's note, TaskContextForEnv.HandoffNote, and errPathPreExists's namespace list. The Reuse path keeps its issue_context.md mentions, now labelled as cleanup of manifests written by older builds rather than of anything this code still writes. Co-authored-by: multica-agent <[email protected]> * refactor(squad): keep the protocol's ownership note out of the prompt (MUL-6984) Review nit on #7971. The Squad Operating Protocol ended with a paragraph saying it is the single statement of the no_action rule and that the rule changes here — true, and worth writing down, but addressed to whoever maintains the text. That const is sent to the model on every leader turn, so the leader paid ~40 tokens per turn for a sentence it cannot act on. Moved to the const's doc comment, next to the other constraints a maintainer reads before editing it. The skill side already records the same ownership in multica-squads/references/squad-source-map.md. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
d276991c60 |
MUL-6743 refactor(issues): remove the assignment handoff note (#7626)
* refactor(issues): remove the assignment handoff note (MUL-6743) Usage data puts the handoff note at 0.2% of assignments, so the box occupied the confirm dialog's centre for the other 99.8% and bought them nothing. Removing it takes the whole feature with it: the note input and its runtime version gate, the `handoff_note` write field on PATCH/batch issue updates, the task column write, and the daemon's opening-prompt + issue_context.md handoff sections. The confirm dialog itself stays — it still names who is about to run and still offers "Don't start yet" (suppress_run). The `*WithHandoff` enqueue methods only ever varied by that note, so they become `EnqueueTaskForIssueByActor` / `EnqueueTaskForSquadLeaderByActor` — the actor-carrying variants autopilot already called with an empty note. `preview-trigger` drops `handoff_supported`, which nothing read after the gate moved client-side. `agent_task_queue.handoff_note` is deliberately left in place: it holds historical rows and this is the expand half of an expand-contract pair. Nothing reads or writes it now, so a follow-up migration can drop it once no deployed binary selects it. Co-authored-by: multica-agent <[email protected]> * docs(issues): reshoot the assignment confirm dialog (MUL-6743) The asset still showed the handoff note box the surrounding prose no longer describes. Recaptured from the app at the same 600x450 frame; the confirm button now carries the send-chord keycaps it has shipped with since MUL-5694. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Lambda <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
879d0de916 |
refactor(agent tasks): drop the Active sibling runs prompt block (MUL-6984) (#7937)
Every issue turn carried a claim-time snapshot of the same agent's other in-flight tasks, scoped only by (agent, workspace) and capped at five. The scope had no relatedness filter, so the list answered "what else is this agent running right now", not "what else touches the code you are about to change". Measured over one day of runs on this workspace: the block was injected 78 times across 30 runs, listing 96 distinct (current issue, sibling) pairs, and not one pair shared a PR, a branch, or an issue family. `run-messages` was invoked zero times off it. Meanwhile the day's one genuine overlap — a PR review pushing to server/internal/metrics/pricing.go and a concurrent change to the model catalog next to it — was never paired, because overlapping in time is not the same as overlapping in code. The duplicate-run defect this shipped with (#6947) is fixed by shouldSuppressActiveSelfAssignment, which stops a second run on the same (issue, agent) at the server. Once that holds, the block can only ever list a DIFFERENT issue, so it was structurally unable to help with what it was added for. #7922 then gave the coordination question a correct answer — `issue runs --active` / `--siblings`, cross-agent, family-scoped, and read at the moment it is asked — which is the same information as a pull instead of a per-turn push. Removed end to end rather than hidden behind a flag: the prompt block, the claim payload field on both sides of the wire, and ListActiveSiblingIssueTasks. The field was `omitempty` and the daemon already rendered nothing for an empty list, so old and new server/daemon pairs stay compatible in both directions; nothing outside server/ ever read active_sibling_runs. Kept: the self-assignment suppression and its tests, `--no-start`, `issue run-messages`, and `issue runs --active/--siblings`. Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
603519d158 |
MUL-6977: test: remove implementation-only coverage (#7932)
* test: remove implementation-only coverage Co-authored-by: multica-agent <[email protected]> * test: remove remaining implementation-only coverage Co-authored-by: multica-agent <[email protected]> * test: restore compatibility and fragment navigation coverage Co-authored-by: multica-agent <[email protected]> * test: preserve behavioral cleanup contracts Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Sol-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
4dfa08917b |
fix(daemon): isolate managed OpenClaw MCP servers without reading user config (MUL-6684, MUL-6857) (#7279)
* fix(daemon): build the managed-MCP config from `config get mcp --json`
Preparing a task for an OpenClaw agent with a managed `mcp_config` reads the
user's resolved configuration so the wrapper can replace `mcp.servers` while
keeping the user's other `mcp.*` settings. That read asks for the resolved root,
`openclaw config get --json` with no key path, and current releases do not
answer it: measured against 2026.5.27 the CLI writes zero bytes to stdout and
never exits, and the same shape is reported against 2026.7.1-2. Every
managed-MCP task on such a host therefore fails preparation.
`config get mcp --json` is answered, and it is also the only part of the
configuration this code needs. Switching to it means the snapshot can no longer
be a flat copy of the user's resolved config, so it becomes an include chain:
1. `$include` the user's live config,
2. merge `{"mcp": null}`, which OpenClaw's include merge treats as a
wholesale replacement of the user's MCP object,
3. restore the resolved non-server MCP settings.
The wrapper then contributes the managed server set, which is the only server
definition the chain can yield. `sessionIdleTtlMs` and its siblings still
resolve; user-only servers do not.
Security posture moves in both directions, so both are worth stating:
- Better: the task directory no longer receives a copy of the user's resolved
configuration. That copy was a flat snapshot of everything the loader
produced, which the code it replaces acknowledged could carry API keys and
model-provider tokens, mitigated only by 0o600. The chain writes include paths
and MCP tuning, and nothing else.
- Worse: because the chain reaches out to the user's real config, the daemon has
to grant its directory through OPENCLAW_INCLUDE_ROOTS. The flat copy was
self-contained and needed no grant. The trade is a read grant on one directory
instead of duplicating that directory's secrets into per-task state, which
also means the include never goes stale relative to the user's config.
The MCP read uses the same deadline as every other call in the preparation
rather than a shorter budget of its own. It is one of the
openclawMaxCLICallsPerPreparation calls the budget arithmetic already accounts
for, it is the same class of invocation as the discovery calls — measured at
4.3s on the slow host in #7142 — and it has to honour
MULTICA_OPENCLAW_CLI_TIMEOUT for the same reason those do. A shorter private
budget would fail closed on exactly the machines that raise the override because
they need it.
`openclawResolvedFullConfig` and `stripUserMcpServers` lose their only caller.
The discovery cache's note about deliberately not caching the resolved config is
updated: the reason is no longer that the payload holds secrets, but that the
MCP subtree is per-agent state that has to reflect the user's configuration as
it is now.
Tests: the three strict-replace tests move to the MCP subtree and assert the
chain rather than a copy, including that the snapshot contains none of the
user's server bytes. TestPrepareOpenclawConfigIncludeRootGrants covers the grant
for all three include targets, because getting it wrong fails at OpenClaw's
include-confinement check with nothing pointing back here. The real-CLI test
behind the agentintegration tag asks the installed binary to resolve the
generated chain, since the correctness of the reset bridge lives in OpenClaw's
merge semantics rather than in the JSON this package writes.
* fix(ci): compile the agentintegration tests, and correct the snapshot doc
Two review follow-ups on the managed-MCP include chain.
The `openclawUserSnapshotFile` comment still described the flat sanitized
copy this PR replaced — "the user's config minus the `mcp` block". The file
now holds no user config bytes at all, only an include chain, which is also
why its 0o600 is about the paths it names rather than secrets it carries.
The name stays because the wrapper's include target is not worth churning
for a rename; the comment says why.
The design's load-bearing claim — that `{"mcp": null}` makes OpenClaw's
include merge replace the user's MCP object rather than merge into it —
is verified by openclaw_mcp_real_integration_test.go, and nothing compiled
that file. `agentintegration` guards nine files across the module and no CI
job referenced the tag, so a helper renamed in an ordinary test file, or two
branches each defining one in the same package, breaks the tagged build with
every untagged job still green. That is not hypothetical here: #6275 adds
openclaw_real_integration_test.go to this package with its own
`realOpenclawBin`, and dropping it in beside this one leaves `go build ./...`
passing while the new step reports
vet: openclaw_real_integration_test.go:18:6: realOpenclawBin redeclared
vet type-checks test files, so the gate needs no agent CLI and no runner
that can execute one. It sits next to Build, before the suite that would
otherwise be the first thing to notice.
* docs(execenv): scope the root-read claim to what was actually measured
Two comments asserted that "current releases" do not answer
`config get --json`, citing a report against 2026.7.1-2 as corroboration. The
measurement behind them is one host on 2026.5.27, and the citation does not
support the claim: the 2026.7.1-2 reports (@kaverjody in #6275, #7308) are both
about `openclaw config file` — cold-start timings and a descendant holding
stdout — not about the root read. #7308 in fact measures `config file` exiting
cleanly in 4.2s on that version.
Which command is which is the whole premise of this change: `config get mcp
--json` is answered while the root is not, so treating an observation of one as
evidence about the other undercuts the argument it was meant to support.
Scoped both comments to the measurement, and stated that the root read's
behaviour on 2026.7.1-2 is unmeasured. The decision does not need the broader
claim: the subtree is the only part of the configuration this code needs, and
the fail-open branch being removed is a silent-degradation bug on its own terms,
reachable from any cause that makes the read come back empty.
* fix(daemon): harden OpenClaw MCP isolation
* fix(execenv): correct the root-read diagnosis against measured channels
Measured 2026-08-26 on Windows 10.0.19045.6466 against the three npm channels.
Two claims in this branch were wrong, and the corrected version is a stronger
argument for the change rather than a weaker one.
**The root read does not hang; it is not a valid invocation.** `config get`
requires a path, and the pathless form is rejected outright:
2026.6.34 (extended-stable) exit 1 in 2.9s Missing required argument "path".
2026.7.1-2 (latest) exit 1 in 3.0s same message
2026.8.1-beta.3 (beta) exit 1 in 4.4s same message, JSON envelope
So `openclawResolvedFullConfig` did not degrade on some hosts — it failed
preparation for every managed-MCP task on all three channels shipping today.
2026.5.27 writing nothing and never exiting within 60s is one version differing
from every current one, and is now recorded as that rather than as the general
case. Switching to the subtree stops being a workaround for a hang and becomes
the request this code should always have made.
**The "exits 0 with empty stdout" shape is unobserved.** It needs exit 0; all
three channels exit 1. The fail-open branch removal still stands on its own —
it is a silent-degradation path and the new chain has no equivalent — but it is
no longer offered as the mechanism behind the reported `Unknown model`, which
remains a real symptom with an unestablished cause.
Also corrected from the same measurements:
- The missing-key transports are not a migration. 2026.6.34 and 2026.7.1-2 are
stderr-only with empty stdout; only 2026.7.2-beta.7 onward puts the envelope
on stdout. Both paths are load-bearing for channels shipping today, which the
detector doc now states with the version table.
- `config get agents.list --json` on 2026.6.34 and 2026.7.1-2 emits
"Config path not found: agents.list" and names the path, so requiring the
diagnostic to name the requested key does not regress the registry fallback on
current stable. `agents list --json` answers on all three.
- 2026.8.1-beta.3's `mcp` schema is exactly `servers` + `apps` with
additionalProperties:false; `sessionIdleTtlMs` is gone. Stage 3 of the chain
can only carry keys the same CLI just returned, which is what keeps it safe
against that constraint. Comments now use `mcp.apps` as the current example.
New regressions, all from verbatim measured output:
- the pathless usage error must not read as a missing key, in both the stderr
and the JSON-envelope form. This is the shape that actually occurs, and
misreading it would turn a broken invocation into a silent "user has no mcp
block";
- beta.3's invalid-config envelope (schema violation in a sibling `issues`
array) must not read as a missing key either.
The smoke workflow now fails when the test skips. A skipped Go test reports `ok`
at the package level, so the job would have gone green whenever the CLI was
absent or the -run pattern stopped matching — green exactly when nothing was
verified. #6897's Windows step documents the same hazard.
The real-CLI test's doc records the platform, the three resolved versions and
their PASS durations (36.9s / 40.6s / 34.2s), and the hand-run control: dropping
the reset stage leaks `user-only` back into the resolved server map on all three
channels, which is what makes those passes mean something.
* test(execenv): pin the reset-stage/wrapper-mcp pairing the loader depends on
The reset stage is only safe because the wrapper always contributes its
own mcp object under the same condition: OpenClaw merges sibling keys
over the include result, and a null that instead survives to the
resolved root fails schema validation (zod .strict().optional() rejects
null), taking the whole config load down. The pairing held by
construction but nothing named it; this assertion fails in the PR that
breaks it, instead of a week later in the scheduled smoke.
Co-Authored-By: Claude Fable 5 <[email protected]>
* docs(execenv): match the deadline-budget arithmetic to the constant
openclawMaxCLIDeadlinesPerPreparation dropped from 4 to 3 when the
managed-MCP read step was removed, but the worst-case explanation on
openclawCLIMaxTimeout still said 4 x 60s = 4m. Say 3 x 60s = 3m so the
documented safety arithmetic matches the constant and its test.
Co-Authored-By: Claude Fable 5 <[email protected]>
---------
Co-authored-by: weiweiwei <[email protected]>
Co-authored-by: Claude Fable 5 <[email protected]>
|
||
|
mrlonely
|
2e29745100 |
MUL-6870 fix(daemon): require proven task ownership before GC mutation (#7798)
Daemon GC treated every second-level directory under WorkspacesRoot as a task root and deleted it once it had no .gc_meta.json and went past the orphan TTL, without ever proving Multica had created it. Pointing workspaces_root at a directory holding real user content therefore made GC delete that content. Require valid Prepare-time .task_owner provenance, matching the exact workspace/task path, before every GC mutation including artifact-only cleanup. Revalidate after taking the env-root lock and again immediately before os.RemoveAll. Directories that cannot prove ownership are retained. |
||
|
|
61ea48fd2e |
MUL-6881 fix(daemon): keep one worktree branch per conversation in local worktree mode (#7815)
* fix(daemon): keep one worktree branch per conversation in local worktree mode Every comment on a local_directory issue started a new task, and worktree mode gave each one a branch of its own forked from the user's HEAD. Turn two therefore stood in a tree that did not contain turn one's work, with that work stranded on another branch and nothing saying so. Tasks that share a conversation — the turns of one issue, one chat session — now share a branch, agent/<agent>/<issue>, and a follow-up checks it out at its tip instead of forking from HEAD. Env roots stay per task, so sibling tasks still run concurrently; when one already holds the conversation's branch, the other forks from its tip onto agent/<agent>/<issue>-<task> rather than falling back to HEAD. Replaying the user's uncommitted tree onto a continued branch cannot use the turn-one path: that merge takes the user's HEAD as its base, so it conflicts exactly when the agent edited the lines the user had left uncommitted. Each turn now records the snapshot it saw at refs/multica/local-state/<branch> and the next one replays only the difference — a cherry-pick whose parent is that snapshot. A genuine conflict leaves the branch as it stands and logs which files did not come across, rather than stranding the conversation. Teardown follows the same rule everywhere: a branch this prepare did not create belongs to the turns before it, so Finalize's read-only cleanup, Discard and the prepare rollbacks only ever drop a branch they created themselves. Co-authored-by: multica-agent <[email protected]> * fix(daemon): prove branch ownership and hand replay conflicts to the agent Review of the per-conversation worktree branch found four ways it could put the wrong tree in front of the agent. All four came from the same two gaps: the user's snapshot did not describe their whole directory, and a branch was recognised by its name. The snapshot is now one commit covering tracked edits AND untracked files, built in a private index inside the task's env root. It answers the question a continued branch actually has to ask — what has the user changed since the state I already carry — for every file, so a once-untracked file the agent edited is no longer reverted to the user's older copy each turn, and their later deletion of one carries. Redirecting GIT_INDEX_FILE also removes the .git/index.lock race the previous capture had to retry around (#7434). A conflict between the user's newer edits and the branch's own work is now the agent's to resolve, per the product decision on the issue: the worktree is handed over mid-merge, the turn's prompt names the files and says which side is which, and Finalize refuses to deliver — preserving the worktree — while anything is still unmerged. The snapshot only advances once the branch really carries it, so an unresolved edit is offered again next turn instead of being recorded as synced. Before this, the conflict was reset away and the user's change was lost from every later turn. Continuing a branch is decided by an ownership record — workspace, agent and conversation ids, recorded in the snapshot commit at refs/multica/local-state/<branch> — never by the branch name. A same-named branch the user created is left untouched and the task takes agent/<agent>/<issue>-<fingerprint>, which its own follow-ups then continue. The same check keeps two workspaces that mint one issue identifier apart. Those refs are now GC'd: every prepare drops the record of a branch that no longer exists, and the docs say where they live and how to clear them. Co-authored-by: multica-agent <[email protected]> * fix(daemon): tie a task branch's record to the branch, and quote untrusted paths Ownership proved that Multica once wrote a branch of this name, which stayed true after the user deleted it and created their own under the same name. No prepare runs in that gap, so the orphan sweep never sees the branch missing and the next turn continued straight into the user's unrelated work. The record now also pins the branch tip it was written at, as the second parent of the record commit, and a branch is only continued while that commit is still an ancestor of its tip. Deleted-and-recreated, force-moved and rebased branches all fail that test; committing on top of a delivered branch — the normal case — passes it and picks the user's commit up. Finalize re-records afterwards, because a checkpoint written at prepare time sits on the branch's base, which is exactly where a recreated branch would sit. Conflicted paths reaching the turn's prompt are quoted rather than wrapped in a code span. They come from the user's repository and git allows newlines, backticks and quotes in a filename, so a raw one could close its list item and continue as an instruction line of its own; the same quoting is applied to the Finalize error. The list is bounded at 50 names with the remainder counted. Also drops a duplicated pair of bullets in the Chinese docs and corrects two file comments that still described the stash-and-copy replay. Co-authored-by: multica-agent <[email protected]> * fix(daemon): make the branch record part of delivery, and bound the conflict list by bytes The record could still authorise a branch that was not ours. Finalize wrote it after removing the worktree and treated a failure as non-fatal, so a failed update-ref left the ref pointing at the record Prepare had written — whose checkpoint is the branch's base, which is exactly where a branch the user deletes and recreates from HEAD sits. The ancestor check then passed on their work. Two changes close it. A checkpoint is only written when it is a commit the branch could not sit at without this conversation's work: Finalize records the tip it actually delivered, and Prepare records early only once the branch has moved off the user's HEAD — which is the baseline commit, or a continued branch's own tip. And the record is now part of delivering: Finalize writes it BEFORE removing the worktree and fails the task if it cannot, so the worktree is still there to preserve, exactly as for a commit that could not be made. The checkpoint is the commit the caller delivered rather than a re-read of the branch ref, so a branch moved in that window fails the next ancestor check instead of being recorded as current. A sibling racing the very first turn of a conversation on a clean tree now finds no record yet and starts its own line of work rather than forking from a branch it cannot prove is ours. That is the price of the rule and it is the safe side. The conflict list in the prompt is bounded by rendered bytes rather than by entries — a git path is as long as the filesystem allows, so 50 entries bounded nothing — with the remainder counted and a single oversized path unable to overrun the budget either. Co-authored-by: multica-agent <[email protected]> * fix(daemon): give every task branch a commit of its own, and verify what a run delivers The checkpoint rule — record only a commit the branch could not sit at without this conversation's work — held in Prepare and not in Finalize. Finalize took whatever the worktree's HEAD was, so a run that reset its worktree back to the user's own HEAD had that plain user commit recorded as the branch's checkpoint, and a branch the user later recreated there passed the ownership check. Two changes make the rule total. Every branch this mode creates now gets a baseline commit of its own, empty when the user's tree was clean: without it the branch sits exactly where the user's HEAD does and nothing tells it apart from a branch they create at the same place. It also makes `git diff <baseline>.. <branch>` the agent's work on every branch rather than only on the ones that started dirty, which is what the surrounding comments already claimed. And Finalize verifies the delivery point before recording it: the tip has to BE the task's branch — not a detached checkout, not a branch someone moved underneath the run — and it has to still contain the anchor, which is that baseline or the checkpoint the previous turn recorded and this one verified. A delivery that fails either test is not recorded at all: the task fails, the worktree is preserved, and the error names what is missing. A sibling racing the first turn of a conversation forks from it again, since every branch now carries a recordable checkpoint from the moment it is prepared. Co-authored-by: multica-agent <[email protected]> * fix(daemon): keep the anchor on the commit that carries this turn's user edits A follow-up turn commits the user's newest edits as its own baseline, but the anchor stayed on the checkpoint the previous turn had recorded. A run that then reset its worktree back to the previous tip still passed verification — the old checkpoint is an ancestor of it — so the turn was recorded as having delivered edits the branch no longer had. The turn after that computed its replay from a state the branch never took, and the user's edits were simply absent from the worktree with nothing to indicate it. The anchor now follows every baseline this task commits, so it always names the newest commit carrying the state about to be recorded. Resetting past it fails the turn and preserves the worktree, as for the first-turn case. The conflict path had the same hole from the other end: there the branch does not carry the snapshot until the agent resolves and something is committed. When a turn that started mid-merge ends with the branch exactly where it began, the record keeps the state the branch is known to carry instead of advancing, and the next turn offers those edits again. A resolution in favour of the branch's own side leaves the same empty trace as discarding the merge, so this trades a merge that may have to be concluded twice against recording edits as delivered that may have been dropped. Co-authored-by: multica-agent <[email protected]> * fix(daemon): measure a delivery against the commit its own turn started from A turn that starts mid-merge only carries the user's edits once the agent resolves and something is committed, and the guard for that compared the delivered tip against the checkpoint an earlier turn had recorded. Those are not the same commit: the user committing on a delivered branch is supported and tested, so the branch tip a later turn starts from can already be past that checkpoint. A resolution that committed nothing then left the tip equal to the turn's own starting point but not to the older checkpoint, the guard did not fire, and the edits were recorded as delivered — missing from the turn after with nothing to indicate it. Both questions now measure against LocalWorktree.BaseCommit, the commit this turn actually started from. It is at least as new as the earlier checkpoint in every case, so the separate anchor field is gone: a delivered tip that contains this turn's starting point contains everything an earlier turn recorded, and a tip that does not is a run that reset away work already delivered. Co-authored-by: multica-agent <[email protected]> * test(daemon): cover the claim-to-branch wiring with a two-turn Prepare test Every multi-turn test so far drove PrepareLocalWorktree with hand-filled params, which skips the step that turns a claim into them: which field becomes the branch name, and which become the identity the branch is recorded under. This one runs two turns of one issue through Prepare itself, and a third for another workspace carrying the same issue identifier. Both halves of that mapping are load-bearing — blanking the conversation key or fixing the workspace id makes it fail. Co-authored-by: multica-agent <[email protected]> * fix(daemon): carry the worktree's own state across the preparation helper Prepare runs in a short-lived helper process and the Environment it built comes back to the daemon as JSON. LocalWorktree keeps everything Finalize reasons about in unexported fields, and ordinary struct marshalling drops those without a word — so the daemon finalized a worktree whose owner, snapshot, tracksState and createdBranch were all zero. Nothing failed; the guarantees were simply absent in production. No delivery verification ran, Finalize recorded nothing (leaving the checkpoint at the baseline Prepare had written inside the helper), a conflicted turn could not hold its snapshot back, and a read-only turn kept its branch. Every in-process test passed throughout, because they call PrepareLocalWorktree directly. Custom Marshal/Unmarshal now carry that state. aborted stays out of the wire deliberately: the daemon sets it after Prepare returns, so it belongs to the parent. The regression tests run two turns through PrepareIsolated — the real boundary — and assert the state arrives, that Finalize's record names the tip it delivered rather than the baseline, and that a read-only turn still drops its branch. Found by running the whole stack: a real daemon, a real local_directory in worktree mode, and two real turns. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
f93855470f |
MUL-6874 fix(desktop): recover unexpectedly stopped daemon (#7808)
* MUL-6874 fix(desktop): recover unexpectedly stopped daemon Co-authored-by: multica-agent <[email protected]> * MUL-6874 fix(desktop): bound daemon recovery retries Co-authored-by: multica-agent <[email protected]> * MUL-6874 fix(desktop): queue one-shot daemon operations Co-authored-by: multica-agent <[email protected]> * MUL-6874 fix(desktop): reject nested daemon operations Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Sol-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
mrlonelyandL
|
6231962e98 |
MUL-6840 fix(pi): preserve sessions across workdir changes (#7760)
* fix(pi): resume session across workdir changes * fix(pi): serialize session transcript writers * fix(pi): keep busy sessions resumable --------- Co-authored-by: L <[email protected]> |
||
|
mikyan
|
fe59262188 |
feat(agent): add Huawei Cloud CodeArts support (#6985)
* feat(agent): add Huawei Cloud CodeArts support * fix(migrations): resolve CodeArts prefix collision * fix(agent): address CodeArts review feedback * fix(agent): tighten CodeArts integration scope * fix(agent): resolve CodeArts follow-up review blockers * fix(agent): redact CodeArts launch arguments * docs: sync supported runtime counts * fix(agent): own CodeArts model probe process tree |
||
|
feig
|
a92e8f6f7e |
MUL-6689: feat(agents): support MCP config for the Oh-My-Pi runtime (#7565)
Adds MCP configuration support to the Oh-My-Pi (omp) runtime. The MCP tab is
now visible for omp agents, and the managed config is materialised as
`{workDir}/.omp/mcp.json` — omp's highest-precedence project-level path —
before the runtime starts.
Materialisation lives in `execenv` and is recorded in `sidecarManifest`, so it
inherits refuse-to-clobber on a pre-existing file plus daemon-side teardown and
reuse cleanup. `Reuse` declines (returns nil, forcing a fresh Prepare) when the
config cannot be written, matching the Cursor path, so a leftover file from a
crashed run cannot silently carry stale MCP servers into the next task.
`.pi` and `.omp` are added to the repo-cache git excludes so runtime sidecars —
which for MCP routinely embed third-party tokens — cannot be committed by an
agent; `.pi/skills/` was already being written unexcluded before this change.
`listRuntimeLocalMcpServers` reports omp's user-scope inventory from
`~/.omp/agent/mcp.json`, with a comment recording that this is deliberately
narrower than omp's full discovery chain.
Pi itself is excluded by design. Upstream states the position in its README
("**No MCP.** Build CLI tools with READMEs, or build an extension that adds MCP
support."), confirmed against pi 0.67.2: no MCP code in the published bundle,
no MCP flag or subcommand, and a probe server placed in a project config is
never launched. There is no `.pi/mcp.json` convention to write into, so the
negative assertion pinning `pi` out of the allowlist is kept and now carries
that reasoning.
Closes #7558 for omp only; Pi would require authoring a Pi extension for the
daemon to inject, which is separate work.
|
||
|
VvVandweiweiwei
|
7d128b5a45 |
fix(daemon): own the openclaw CLI process tree so its deadline is enforceable (MUL-5467, MUL-5630, MUL-6736) (#6275)
* fix(daemon): own the openclaw CLI process tree so its deadline is enforceable (MUL-5467) openclawCLITimeout has carried a "Known gap (deliberately not fixed here)" note since #6084: the 5s deadline could not actually bound an `openclaw config ...` invocation. This closes it, and fixes a second OpenClaw misbehaviour found alongside it. ## Bug 1 — the deadline was decorative exec.CommandContext kills only the direct child. cmd.Output() blocks in Wait() until the output pipes os/exec manages reach EOF, and cancelling the context does not unblock that io.Copy. So when openclaw leaves a descendant holding stdout — its `openclaw-config` helper, or cmd.exe → node on Windows — the call runs for the *descendant's* lifetime. #6084 measured this on linux/dash (a shim whose backgrounded child slept 6s took 6.01s against a 150ms deadline), tried a cmd.WaitDelay backstop, and reverted it on review because it bounds the call by leaving the descendant running — trading a hang for a process leak that nothing on Unix reaps. The note left behind names the real fix: process-tree ownership. Three call sites had the shape: - execenv.execOpenclawCLI on the task's critical path (Prepare → prepareOpenclawConfig) - agent.detectCLIVersion runs for every provider inside the daemon's blocking registration preflight - agent.discoverOpenclawAgents four invocations per call ## Bug 2 — the answer was already on stdout Measured on an OpenClaw host (openclaw 2026.5.27): openclaw --version 258ms exits cleanly openclaw config file 60s correct path printed, then never exits openclaw agents list 60s correct list printed, then never exits Waiting for exit turns two working commands into a task-fatal error: agent_error.process_failure (prepare execution environment: execenv: prepare openclaw config: locate openclaw active config: openclaw config file: context deadline exceeded (process: signal: killed)) The contract of these commands is "print a value". Once the value has arrived and nothing more is coming, whether the process tidies itself up is not the caller's business, and it should not fail a chat task. ## The fix Two helpers in server/pkg/agent, both built on the process-group primitives codex/claude/opencode already use — no new platform-specific functions: - RunCollect: hands os/exec an *os.File instead of a buffer, so os/exec starts no copy goroutine and Wait returns the instant the direct child exits. Then signals the child's process group, which reaps the helper and releases the last write end, so our own readers see EOF. Unlike WaitDelay this reports the CLI's real exit status, which matters because a failed `--version` probe skips runtime registration entirely, and because openclawShimDiagnostic type-switches on *exec.ExitError. - RunCollectQuiet: returns when the process exits, or when stdout has produced output that then stayed idle for 400ms — whichever comes first. Reports quiet=true on the idle path so callers can log the CLI's failure to exit without failing on it. The idle shortcut deliberately cannot mask a silent CLI: with no output there is nothing to salvage, so the deadline still governs and the call still fails. A non-zero exit still fails too, with stderr intact. execOpenclawCLI keeps its shim diagnostics and its context-attribution order byte-for-byte; only the exec mechanism changes. discoverOpenclawAgents keeps its existing 30s function-level budget rather than gaining a per-attempt one. Windows: there is no process group to signal, so calls are bounded (by the drain grace) but a descendant can still outlive one. Owning descendants there needs a Job Object and is not attempted here. ## Verification - Both packages green with -race; 6 consecutive runs, no flakes. - All 51 existing Openclaw* tests in execenv pass, including TestExecOpenclawCLITimeoutIsNotMisdiagnosedAsMissingInterpreter and TestExecOpenclawCLICancellationIsWrapped — their shims produce no stdout, so the salvage path is not reachable and the errors.Is(err, context.DeadlineExceeded) contract is preserved. - 13 new tests. Both mechanisms are mutation-verified: * RunCollect reverted to buffers with no group reap → the guard test hangs and the dump is the production stack: cmd.Wait → awaitGoroutines blocked, writerDescriptor parked in io.Copy. * RunCollectQuiet's idle path disabled → both the unit test and the execenv end-to-end test fail after a full 60s. * fix(daemon): address review on the openclaw CLI collector (MUL-5467) Three blocking items from review of #6275. All three were reproduced before being fixed, and the first one was a genuine correctness hole rather than a style objection. ## 1. The deadline branch salvaged partial answers RunCollectQuiet returned success from the ctx.Done() branch whenever stdout was non-empty, without requiring the idle grace or any check that the answer was finished. A CLI still streaming when the deadline arrived therefore had its truncated output reported as a completed response. Reproduced with a stub emitting a JSON document forever against a 250ms deadline: 9 runs in 10 came back as success carrying an unparseable fragment. The early return now needs two independent conditions, neither sufficient alone: - an OutputComplete rule supplied by the caller accepts the buffer, and - the buffer has since been idle for idleGrace. Reaching the deadline is never success. The captured bytes are still returned alongside the error so a caller with its own rule can inspect them, but the runner no longer decides on the caller's behalf. A nil rule disables the early return entirely, which is the conservative default for a command whose output shape has no completeness rule. The rule cannot be inferred inside the runner, which is the point: only the caller knows whether `{"agents":[{"id":"a"},` is truncated or legitimate. - `--json` subcommands -> agent.JSONOutputComplete (whole buffer parses). - `openclaw config file` -> openclawConfigPathComplete (last non-empty line looks like a path). Deliberately stricter than openclawParseActiveConfigPath, which resolves a relative line through filepath.Abs and would therefore accept a Doctor warning border as an answer. That matters because OpenClaw prints Doctor UI *before* the path (MUL-3136), so an early return on idle output alone hands back the banner as the config path — and nothing downstream catches it. - `agents list` text fallback -> RunCollect (wait for exit). Decorated text has no completeness rule and a silently short agent catalog is worse than a slow one; this path still gains the reaping and the bounded return. ## 2. quietWriter published bytes before their timestamp The buffer write and the last-write timestamp were updated in separate critical sections, so a reader could observe new bytes carrying a stale timestamp and conclude the stream had gone quiet at the moment it was producing — truncating an answer mid-write. Both now move in one critical section (outputBuffer). ## 3. The drain-timeout path raced the readers, and could park a goroutine RunCollect read its buffers after the drain grace expired while io.Copy could still be appending to them; the deferred Close on the read ends ran only after the return values had been evaluated. Separately, RunCollectQuiet handed os/exec io.Writers, so its cmd.Wait goroutine could sit forever on a descendant holding the pipe — on Windows nothing released it, since there is no process group to signal. Both are now structural rather than patched. A single collector owns the pipes for both helpers (the *os.File ownership RunCollect already had), and finish() guarantees that when it returns, every goroutine startCollector spawned has been joined and nothing can still append to the buffers: it reaps the tree, waits bounded for Wait, and if the readers still have not seen EOF it closes the read ends outright — which makes the in-flight Read return — and only then joins them. ## Verification - Both packages pass with -race under CI's flags (-p 2 -parallel 2). - All pre-existing Openclaw* tests still pass, including the timeout attribution and shim diagnostic ones. - New regression tests, all three named in the review: continuous output until the deadline (must not be success), a prompt followed by a delayed body (must not return the prompt), and a late non-zero exit within the grace (must be reported as the failure it is). Plus the completeness rules per subcommand, the banner-then-path case end to end through execOpenclawCLI, goroutine collection on both helpers, and the two outputBuffer contracts. - Windows contract coverage added (run_collect_windows_test.go) with a scoped step in the existing windows-execenv CI job, since the close-the-read-ends path is unreachable on Unix where the group kill releases the pipe. - Mutation-verified: restoring the unconditional salvage fails both the unit and the execenv regression; dropping the completeness rule from execOpenclawCLI makes the Doctor banner come back as the config path. * test(daemon): make the openclaw config-path fixtures hermetic The two execenv fixtures printed the literal path the host had printed, `/root/.openclaw/openclaw.json`. openclawParseActiveConfigPath stats what it parses and tolerates only os.ErrNotExist, so that path resolves differently by environment: ENOENT on a developer machine, where /root does not exist, but EACCES on a Linux CI runner, where it exists and is unreadable. The second case is a hard error, so TestExecOpenclawCLIWaitsForThePathAfterDoctorBanner failed in CI for a reason unrelated to the boundary it covers. Both fixtures now print a real file inside t.TempDir(), which stats cleanly everywhere. The second test only inspects raw stdout today, but a hard-coded path there would be a trap for whoever next adds a parse to it. The literal path stays in the completeness-rule table, which calls the predicate directly and never stats, so it keeps documenting the shape the CLI actually emits. * fix(agent): own one-shot CLI process trees on Windows * fix(daemon): support current OpenClaw config discovery * Address review, and narrow this branch to the process-tree fix Blocking items from the review: 1. The deadline branch no longer salvages. It returns an error unconditionally, and callers supply a per-subcommand completeness rule (openclawOutputComplete) that decides whether the captured bytes are a finished answer. Regression coverage for the three shapes named in the review: output continuing right up to the deadline, a banner ahead of the real answer, and a late non-zero exit. finish() now reports, rather than being wired to a field that was never assigned. Two cases do not converge and both cost the caller something it would otherwise never learn: a process still outstanding in Wait after the tree kill, and a descendant still holding the pipe when the read ends had to be force-closed, which can truncate trailing output. collectorError joins that with the command's own error, so nothing is displaced. 2. outputBuffer publishes bytes and the write timestamp inside one critical section, so a poller cannot see new bytes with a stale timestamp and conclude the stream went quiet while it was in fact producing. 3. The drain-timeout path closes the read ends and waits for both absorb loops before any snapshot is taken, so no io.Copy can be appending to a buffer the caller is about to read. Windows contract coverage runs as its own CI job. The goroutine guarantee is stated at its real strength instead of "no goroutine outlives the call, on any platform". Convergence holds whenever the tree kill takes. When it does not, one goroutine can stay parked in cmd.Wait on a process nothing can terminate; that residue is not removable here, so it is reported instead of asserted away. On Windows the same case extends the call rather than leaking, because an anonymous pipe is not pollable and closing the read end waits for an in-flight Read to release it. Also from review: - A JSON error envelope is valid JSON, so JSONOutputComplete accepts one and a CLI that prints `{"error": ...}` and then lingers past the idle grace hands back err == nil with the envelope in stdout — where the exit-status path never sees it. All three JSON readers now check for that explicitly. It matters most for `config get agents.list --json`, where the envelope is how a 2026.6+ host says the key moved to the registry: decoding it as data turned a graceful fallback into a failed preparation. For the object-target read the failure is silent, since the envelope would be sanitized and written into the task's snapshot as if it were the user's config. - RunCollectCmd selects on ctx.Done(). It takes a command the caller built, so enforcement must not depend on that caller having used exec.CommandContext; the ordering matches RunCollectQuiet so the two entry points cannot report different error shapes for the same situation. startCollector drops its unused ctx parameter rather than implying it enforces one. - checkOpenclawVersion keeps stderr in the parse. It replaced CombinedOutput(), and a build that prints its version there would otherwise fail the gate on empty output, which skips runtime registration entirely. extractVersionLine is bypassed on purpose: parseOpenclawVersion already scans the whole text, while picking the first line containing a semver lets unrelated stderr noise answer for it. The probe is bounded because the caller's ctx is the task's, and ExplainExecError is applied here too so an ENOEXEC shim is still named (MUL-6164). - TestRunCollectQuietWithoutCompletenessRuleWaitsForExit synchronizes on a marker the CLI writes after its answer instead of a deadline. Its two assertions want opposite budgets, and the reviewer reproduced the resulting flake 3/3 under load. Waiting for the marker also proves more than a timeout could: that the runner was still blocked at a moment when the answer was already fully available. Scope: the MCP work that shared this branch is reverted to main's behaviour and will come back on its own, because it needs a discussion this one does not. It changed which command supplies the resolved MCP settings, restructured the include chain, and moved where the user's config bytes live — none of which the deadline fix depends on. Preparation for a managed-MCP agent behaves exactly as it does on main here. $OPENCLAW_HOME path expansion stays: openclawConfigPathComplete calls openclawHomeRest directly, so the completeness rule for `config file` cannot tell whether that shape is a finished answer without it. * fix(agent): converge the collector's tree kill and stop failing calls on cleanup (MUL-5467) Review follow-up on #6275. All three findings share one root cause: whether the caller got its answer depended on whether cleanup converged, and cleanup convergence depended on a single kill(-pgid) landing. - Retry the tree kill across collectReapWindow. One pass loses a descendant whose fork completes between the kill's enumeration of the process group and the signal's delivery — measured 3 misses in 10 runs of the forking stub, each leaving a `sleep 300` reparented to init and stalling the call until the drain budget expired. The window is deliberately short so it does not widen the pid-reuse window reapProcessTree already documents. - Replace the 5s EOF budget with a 400ms settle. Once the direct child has exited, at most one pipe buffer of its output can still be unread — a child that had written more would have blocked in write() and could not have exited — so a short cap cannot truncate a well-behaved CLI's answer. What it cuts short is a surviving descendant's trailing output, which is not the answer. - Stop reporting cleanup residue as a call error. finish() now logs and returns nothing: a probe whose output arrived and whose helper the kernel would not kill is exactly the case #6084's cmd.WaitDelay got wrong, and a failed version probe skips runtime registration entirely. This drops finishErr, collectorError and the drainForced/waitPending split, which the four call sites were interpreting two different ways by accident. - Fix the reaping tests. They asserted on `$$` captured inside `( )`, which POSIX keeps at the invoking shell's pid, so they named the direct child that cmd.Wait reaps regardless — all of them passed with reapProcessTree removed, while the run leaked 7 orphans. Record `$!` instead, and add TestRunCollectRetriesTheTreeKill, which injects a missed pass because the race lives inside one syscall and cannot be forced from a stub. Verified: collector group 0/10 failures (was 3/10) with 0 orphans, 0/5 under -race, full ./pkg/agent -race green, execenv Openclaw -race green, and both mutations — retry removed, reaping disabled — fail the tests that now cover them, on their own assertions rather than on a cleanup error. * refactor: move the `$OPENCLAW_HOME` expansion out of this branch It is not process-tree work. `openclaw config file` printing `$OPENCLAW_HOME\...` instead of the expanded path is the same class of bug as #6630 — a path the daemon cannot stat, reported as a fresh install, wrapper without the user's `$include` — and #6643 shipped the tilde half of it as its own change. This half belongs there too, not folded into a branch about owning the CLI's process tree. Removed here: openclawHomeRest, its branch in expandOpenclawPath, the unit test, and the completeness-rule row that depended on it. Neither PR now depends on the other. openclawConfigPathComplete therefore does not recognize the variable shape, which is correct in isolation: `main` cannot expand it either, so a rule that accepted the line would hand the caller a path the parser then fails on. Such a host keeps `main`'s behaviour — no rule match means RunCollectQuiet waits for exit — so it loses the hang tolerance and nothing else. The comment on that function says so and names the change that should close it, because a gap nobody wrote down is one a rebase re-opens silently. * fix(daemon): teach the config-path rule the `$OPENCLAW_HOME` shape (#7310 hand-off) #7310 landed the parser side of this, so the deferred half is now due — and it was recorded in the code precisely so a rebase could not drop it silently. While expandOpenclawPath could not resolve `$OPENCLAW_HOME\...`, this rule had to refuse the shape: accepting it would have reported a finished answer and then handed the caller a path the parser failed on. The cost was that a host with that variable set lost the early return entirely and went back to waiting for the CLI to exit — which is the hang this branch exists to survive. With the parser fixed, the refusal was the only thing left standing between those hosts and the tolerance. Resolved through openclawHomeFromEnv rather than read raw from the environment. The value may itself be a tilde path that upstream expands before printing, so `filepath.IsAbs` on the raw string answers false for `OPENCLAW_HOME=~/svc` and silently withholds the early return on exactly the configuration #7310's review was about. Mutation-checked: the raw-env version fails the new `tilde-valued OPENCLAW_HOME` row with `rule(...) = false, want true`. TestOpenclawOutputCompleteRules becomes subtests so a row can carry the environment it is about, and gains three: an absolute OPENCLAW_HOME, a tilde-valued one, and an unset one — the last staying false, so a line with nothing to resolve it against still waits for exit and lets the parser report the real error instead of this rule guessing. * docs(agent): site the collector against outputOwned, not cmd.Output() #7531 moved every one-shot probe in this package onto launch.go's runOwned / outputOwned / combinedOutputOwned, which makes three claims in this branch stale rather than wrong-in-detail: - RunCollect called itself "the safe replacement for cmd.Output()". There are no cmd.Output() call sites left in pkg/agent. The comparison a reader needs is against outputOwned, which owns the same process tree and differs only in who owns the pipes — so that is what the doc compares, including why the probeWaitDelay bound is the right default for the probes it serves and the wrong one here. - RunCollect's path-taking form advertised itself as the migration entry point for "the remaining cmd.Output() call sites (models.go's catalog probes, codex.go, deveco_models.go, thinking.go)". #7531 migrated all four to the owned helpers. It stays as the form for a caller holding only a path, and for a probe that turns out to need pipe ownership. - collector's doc now says outright which of the two properties launch.go cannot provide, since "os/exec owns the draining" now describes in-package code rather than a stdlib call nobody here makes. Also adds the `if: needs.changes.outputs.backend == 'true'` guard the new Windows CI step was missing. Every other step in that job has it; without it the step ran on frontend-only changes. * docs(agent): cite upstream for the hang tolerance, not just the timings The measured "prints its answer and then never exits" for `config file` and `agents list` came from one host on openclaw 2026.5.27, which invites the obvious objection: this is an upstream bug, so why compensate for it here rather than wait for a fix. Read at openclaw v2026.7.1, it is not a bug in a build — it is the exit model. `src/cli/one-shot-exit.ts` provides an explicit opt-in, `requestExitAfterOneShotOutput`, which `runCli` flushes after both streams drain. In the shipped 2026.7.1 tarball exactly three modules take it: `models list`, `models status`, and the hooks CLI. The commands this package invokes — `config file`, `config get`, `agents list` — do not: their success paths only print, and `runtime.exit` appears solely on their failure paths, so termination is whatever the event loop happens to do. A version that exits promptly is not promising the next one will. Also pins openclawHomeRest's citation forward. `runConfigFile` still prints shortenHomePath at v2026.7.1, and resolveHomeDisplayPrefix there still selects `$OPENCLAW_HOME` on a non-empty trimmed variable and `~` otherwise. Worth recording that the separator comes from the original path rather than being inserted by the prefix, since that is what the rest-slice in this function relies on. * fix(daemon): size the config-file idle grace to the real warning gap E2E on a real OpenClaw host (2026.7.1-2, plus 2026.6.8 and 2026.7.2-beta.7) falsified two claims this branch was making. Both are corrected here; one needed a code change. **The 400ms idle grace was sized against the wrong gap.** The claim was that a one-shot response arrives in back-to-back writes, so 400ms is "orders of magnitude longer than the gap between those writes". The first half is confirmed — 0.012-0.033ms between writes. The gap that matters is a different one: with a config that produces Doctor and plugin warnings, `openclaw config file` spread its stdout over 3289ms, of which a single 3285ms silence sat between the end of the warning block and the path. The default grace is eligible to fire inside that silence, on whatever the warnings left in the buffer. Why it lands on `config file` alone: upstream routes incidental console output to stderr when argv carries `--json` (withConsoleLogsRoutedToStderrForJson, src/cli/json-output-mode.ts at v2026.7.1), so every `--json` caller here gets a stdout that is the JSON document and nothing else, and a partial document cannot parse however long the writer pauses. `config file` has no `--json`, so the warnings share its stdout with the answer. openclawConfigPathComplete is still the guarantee — a box-drawing border is not an absolute path — and a grace can only be defence in depth, since the gap is a property of the host's plugin and Doctor work. But it should not be defence that loses by three seconds, so openclawQuietIdleGrace gives `config file` 5s and leaves `--json` on the default. The cost is paid only by a CLI that hangs: `config file` then returns in path + 5s rather than path + 400ms, against a 30s deadline. **RunCollect guarantee 2 overstated what a group kill can reach.** The helper was measured holding the same stdout pipe as its parent (both `pipe:[135202]`) while running with its own PGID and SID. `kill(-pgid)` never reaches that, and neither does #7531's ownership, which uses the same group. "Invoking a CLI on a timer cannot accumulate orphans" was therefore wrong on Unix, and the three comments that repeated it are corrected. What holds regardless is guarantee 1: it is pipe ownership, not the kill, that makes Wait return and the deadline enforceable. Windows is unaffected — a Job Object owns the tree irrespective of process groups. Tests: TestOpenclawQuietIdleGraceCoversTheWarningGap pins the per-command grace and asserts the `config file` value exceeds the measured gap; TestExecOpenclawCLISurvivesTheWarningGapBeforeThePath drives a stub with a warning block, a gap past the default grace, then the path, and requires the path back. * fix(agent): make pipe EOF the completion signal, and bound what is kept Second review round found three blockers on b9b39eaea. All three reproduce exactly as reported, and all three are cases where this branch was *worse* than the launch.go helpers it replaced — which is the bar a replacement has to clear, so the fix narrows the branch rather than patching around them. **1. Leader exit was treated as answer completion (worst of the three).** RunCollect*Cmd reaped the process group as soon as the direct child exited. A wrapper can exit 0 while the real CLI — its descendant, holding the inherited stdout — has not printed yet, which is the normal shape for an npm or PowerShell shim. Measured here: the wrapper exits 0, its child prints the version 500ms later, and this head returned an empty version with a *nil error* in 427ms where outputOwned returns the version in 517ms. Empty-and-nil is the worst available shape, and detectCLIVersion routes all 23 providers through it. Pipe EOF is the only signal that means "no more output is coming", so: - detectCLIVersion, checkOpenclawVersion and discoverOpenclawAgents go back to launch.go's outputOwned / combinedOutputOwned, i.e. back to os/exec's own EOF wait. What this branch still fixes there is the *reporting*: a lingering descendant makes Wait report exec.ErrWaitDelay over an answer that arrived, and a failed --version probe skips runtime registration entirely (#6084 reverted a WaitDelay backstop over exactly that). So salvageProbeAnswer drops the error when, and only when, the caller's own parse found the answer — never on a non-zero exit, never on a deadline. - The collector keeps its reason to exist (execenv's task-critical path) and gains awaitOutputAfterExit: after the direct child exits it waits for pipe EOF, bounded by collectDrainGrace (2s, matching probeWaitDelay), unless the caller's completeness rule says the answer is already in — so a CLI that prints and exits with a helper on the pipe still pays nothing. - RunCollect / RunCollectCmd are deleted. With the probes back on outputOwned they had no production caller, and an exported wait-for-exit collector is exactly the API that invited this bug. **2. A path-shaped warning could be returned as the config path.** openclawConfigPathComplete accepted any absolute-looking last line, and the previous commit's answer — a 5s grace — was the wrong dimension, as the review pointed out by quoting this branch's own comment that no fixed grace can bound the host's plugin work. Reproduced: a CLI printing an existing plugin-cache.json path, pausing 6s, then printing the real path returned the warning path after 5.51s. Fixed by not asking a question content cannot answer. `openclaw config validate --json` reports the path in a named `path` field, on every branch (valid / invalid / file-not-found / error), in every version from v2026.5.5 — which is minOpenclawVersion — through v2026.8.1-beta.3, and `--json` makes upstream route incidental output to stderr so stdout is the document alone. openclawActiveConfigPath asks that first and keeps `config file` as a fallback with no completeness rule at all: it waits for exit and fails closed on the deadline. Upstream runs the same readConfigFileSnapshot() for both commands, so preferring the JSON form costs no extra work; it costs one extra invocation only on a CLI too old for either, which is pinned in TestPrepareOpenclawConfigFallsBackWhenConfigFileUnsupported. openclawConfigPathComplete, openclawQuietIdleGrace and openclawConfigFileIdleGrace are gone. `--json` is now the only shape with a rule, and the doc says why that is the design and not a gap. **3. Retention was unbounded.** absorb appended without limit, where outputOwned keeps the last probeStderrSampleBytes. Measured: 13,107,400 bytes of stderr retained from a CLI writing continuously, ~400x the bound it replaced. stderr now keeps its tail at collectStderrTail (= 32 KiB, so moving a call site between mechanisms cannot change what a failed probe reports) and stdout is capped at collectStdoutLimit and reports overflow as an error — deliberately asymmetric, because a head-truncated answer is how a partial catalog becomes a confident empty one. Every new guard was mutation-verified by restoring the mechanism it covers: | Mutation | Test | Failure | |---|---|---| | collector + reap on leader exit | TestDetectCLIVersionWaitsForAWrapperDescendant | `version = ""` in 0.42s | | awaitOutputAfterExit returns immediately | TestRunCollectQuietWaitsForAWrapperDescendant | `stdout = ""` | | drop the rule short-circuit | TestRunCollectQuietDoesNotWaitWhenTheAnswerIsIn | pays the full 2s | | unbounded buffers | TestCollectedStderrKeepsOnlyItsTail | retained 13,107,417 bytes | | unbounded buffers | TestCollectedStdoutOverflowIsReportedNotTruncated | 524,296 bytes, nil error | | shape rule for `config file` | TestOpenclawActiveConfigPathIgnoresAPathShapedWarning | returns plugin-cache.json | The new fallback test also caught a bug in this commit before it shipped: openclawValidatedConfigPath accepted upstream's `CONFIG_PATH ?? "openclaw.json"` literal, because expandOpenclawPath turned that relative name into a confident path under the daemon's cwd. It now requires the reported value to be absolute before expansion, allowing only the `~` and `$OPENCLAW_HOME` forms. * fix(agent): repair the Windows-only collector test call sites The mechanical conversion of the deleted RunCollect call sites over-widened three tuples in run_collect_windows_test.go, which already used RunCollectQuiet's four results. Host `go build` and `go vet` never compiled them — the file is `//go:build windows` — so the break surfaced on the windows-execenv job as `assignment mismatch: 5 variables but RunCollectQuiet returns 4 values`. Verified this time the way it should have been the first time: `GOOS=windows go build ./...` and `GOOS=windows go vet ./...`, plus darwin and linux. That is the check to run whenever this package's exported signatures move, since a third of its process-lifecycle tests only compile on Windows. * test(execenv): pin the config-path payload shapes E2E measured E2E on OpenClaw 2026.7.1-2 confirmed `config validate --json` as the config-path source and corrected two things this branch had inferred from source. **The exit status is load-bearing in the wrong direction, and this was untested.** Two of the three real states exit *non-zero* while carrying the answer: a missing config file gives `{"valid":false,"path":"…","error":"file not found"}` and an invalid one `{"valid":false,"path":"…","issues":[…]}`, both exit 1 with stderr empty. A fresh install is the missing-file case, so reading a non-zero exit as "no answer" would break first run. openclawValidatedConfigPath already ignored the status, but nothing pinned it — writeOpenclawConfigStub could only exit 0, so the tests covered the rarest branch. It now takes an exit code, and TestOpenclawActiveConfigPathReadsThePathFromANonZeroExit drives all three measured payloads. **Two source-based claims corrected.** - "Preferring the JSON form costs nothing extra" understated it. Measured over three runs each on the same host and config: 1.65/1.66/1.65s for `config validate --json` against 6.36/4.16/4.06s for `config file`. The JSON form is ~2.5-4x faster, which tracks the output it does not render — 715 bytes of JSON against 2663 bytes and 28 lines of warning UI. - The reason a `--json` stdout is clean was attributed to upstream's console-log reroute (`withConsoleLogsRoutedToStderrForJson`). That function exists, but it is not what was observed doing the work: the Doctor and plugin warnings arrive as a `warnings` array *inside* the payload. The distinction matters for why JSONOutputComplete is a real signal — the stream is structurally one document, not merely quieter — so the comments now describe the measurement rather than the mechanism inferred from it. Unrelated and pre-existing: `./pkg/agent`'s ACP timing tests (1-6s wall-clock budgets) flake under load, and which ones fail varies per run. Control run of `origin/main`'s whole package on this machine, same conditions: fails TestCodexExecuteSemanticInactivityAllowsContinuousMessages and TestCodexExecuteLegacyFirstTurnMessageSatisfiesProgress. Not introduced here. * fix(agent): require a recognised version before salvaging a probe answer The ErrWaitDelay salvage in detectCLIVersion was gated on `version != ""`, and extractVersionLine falls back to the trimmed raw output when no line carries a semver token — so any non-empty text satisfied it, including whatever a wrapper printed before the real version existed. Review reproduced it with a stub that prints a banner, exits 0, and has a descendant print the version past the 2s WaitDelay: detectCLIVersion returned "initializing plugins" with a nil error, and logged "CLI answered but left its output pipes open" — the opposite of what happened. That value is then persisted as the runtime version, for every provider, since DetectVersion routes all 23 through here. extractVersionLine now reports whether the semver scan actually matched, and only that answers the salvage question. The trimmed-raw fallback still stands on the success path: a CLI that exits 0 with an unusual version format is reporting its version (#2516), and it is only unusable as evidence that a bounded, incomplete read already holds the answer. Mutation-checked: with the old gate, TestDetectCLIVersionDoesNotSalvageABannerAsTheVersion fails with exactly the reviewed shape — version="initializing plugins", nil error, misleading warn. * test(agent): derive the collected-stdout cap from the largest answer collectStdoutLimit argued for why a fail-closed cap is needed but not for its value, and where it falls is a product compatibility boundary rather than only a memory bound: a legal config past the cap stops a task from starting at all. The derivation is now recorded and executable. Measured answers on 2026.7.1-2 are 715 B (`config validate --json`) and 2663 B (`config file`); the largest answer any call site asks for is the resolved config from `config get --json`, which upstream puts no ceiling on. So the worst case is constructed — 1000 agents and 250 MCP servers with the fields OpenClaw carries, 547,291 bytes, already orders of magnitude past a real deployment — and 8 MiB leaves 15x on top of it while matching what the daemon already holds for a comparable per-task payload (maxLocalSkillBundleSize). Two tests pin it: the constructed high-water mark must round-trip through the collector byte-identically with the stated headroom, and the boundary itself is exact — at the limit the answer is returned whole, one byte past it fails closed with errCollectStdoutTooLarge. * fix(execenv): share one CLI deadline across both path-resolution attempts openclawActiveConfigPath asks `config validate --json` first and falls back to `config file`, and the fallback carried a fresh full deadline. That made the worst case five budgets against a ceiling derived from four: at the 60s override ceiling, 5m of CLI time alone, landing exactly on daemon.defaultTaskPrepareTimeout — so the specific, non-retryable ErrOpenclawCLITimeout collapses back into the generic retryable prepare timeout, which is the outcome openclawCLIMaxTimeout exists to prevent. Both invocations answer the same question, so they now share one context. The multiplier is renamed to openclawMaxCLIDeadlinesPerPreparation because that is what it always measured — the budget is per step, not per invocation. The consequence is deliberate and asserted: a validate that burns the whole budget leaves the fallback none, os/exec reports the expired context on Start, and the failure still carries ErrOpenclawCLITimeout. A CLI that cannot say where its config is within the entire deadline will not answer the same question on a fresh one. TestPrepareOpenclawConfigWorstCaseCLIBudgets now drives the real worst case (validate fails, config file answers, registry fallback, managed MCP) and counts distinct ctx deadlines rather than calls — the old test synthesized a validate success from the `config file` response, so the two never fired in the same run. Mutation-checked: with separate deadlines it reports 5 budgets against a constant of 4. --------- Co-authored-by: weiweiwei <[email protected]> |
||
|
|
cd169947b5 |
MUL-6686 feat(daemon): use readable workdir paths (#7316)
Create new task environments under sanitized, bounded readable segments: <workspace-slug>-<workspace-short>/<issue-key>-<task-short>/workdir. The readable name is the visible part; the substance is moving task identity off the directory name. A human-readable segment is mutable — workspace slugs and issue prefixes change — so workspace/task identity now lives in .task_owner, .gc_meta.json and a stable ID-keyed index under .task_roots that freezes one physical root per task for its lifetime. GC and disk-usage read that authoritative identity instead of inferring a workspace from a path string, which also fixes active and preparation-crashed roots being attributed to a nonexistent workspace. Existing workdirs are not renamed or migrated, legacy roots are adopted into the index, and both layouts render privacy-safe relative paths. Labels are lowercased ASCII with separators and traversal collapsed, segments are capped at 24 characters so the two together stay within the previous 49-character Windows path budget, and stable ID suffixes drawn from the UUIDv7 random tail keep roots distinct across renames and case-insensitive filesystems. Closes #6063 Co-authored-by: iiwish <[email protected]> Co-authored-by: J <[email protected]> |
||
|
YYClaw
|
8c563b49be |
MUL-6555 feat(issues): preserve source context for sub-issues (#7461)
* feat(issues): preserve source context for sub-issues * fix(issues): address source context review findings * fix(migrations): guard source context rollback preflight |
||
|
|
8a5a6adbf7 |
feat(agent): add ZeroClaw as a native ACP runtime (MUL-6511) (#7351)
* feat(agent): add ZeroClaw as a generic ACP agent runtime
Add ZeroClaw (a Rust-based, single-binary generic agent CLI) as a
first-party runtime, driven over the standard ACP JSON-RPC transport via
`zeroclaw acp`, mirroring the Dim/QwenPaw/Grok ACP backend shape. Only the
binary, launch args, and blocked-flags policy are ZeroClaw-specific; the
backend reuses the shared hermesClient ACP transport and deliverable
tracker.
No provider-specific quirks (permission-preset injection, minimum-version
gate, or a separate authenticate step) are assumed without evidence — those
were verified against real Dim/Grok binaries and would need the same
hands-on verification against a real ZeroClaw binary before being added
here.
Backend: SupportedTypes/New/launchHeaders wiring, model discovery via
session/new's advertised catalog, daemon probe with
MULTICA_ZEROCLAW_PATH/MODEL overrides, AGENTS.md context delivery,
default-command-name registration, metrics label, and a migration widening
the runtime_profile.protocol_family whitelist.
Frontend: provider union, MCP-config support flag, and a placeholder
provider-logo mark (no official ZeroClaw brand asset exists yet).
Tests mirror the existing ACP backend conventions: fresh-session,
resume-via-session/load, resume-not-found, transient-load-error, blocked
launch args, timeout, and the shared ACP deliverable-boundary regression
suite.
* fix(agent): renumber ZeroClaw's migration to 378 to avoid a same-day collision
Both this branch and a concurrently-implemented Junie runtime branch
independently picked migration 377 off the same origin/main tip.
Renumbering ZeroClaw to 378 up front avoids opening two PRs from the
same author that claim the same migration number.
* fix(agent): align the ZeroClaw ACP backend with the real 0.8.4 runtime
The backend was written against an assumed vanilla ACP handshake and never
checked against a ZeroClaw binary. Driving a real zeroclaw 0.8.4 over stdio
shows five of those assumptions are wrong, and the test fake encoded the same
assumptions, so the suite stayed green against a contract the runtime does not
have.
- Drop the session/set_model request. It is not in ZeroClaw's dispatch table
(-32601) and no ACP handler reads a model param at all, so it could never
succeed; because its failure was fatal, any agent with a model configured
failed before session/prompt was reached. The model comes from the ZeroClaw
agent profile, so opt zeroclaw out of ModelSelectionSupported and stop
reading MULTICA_ZEROCLAW_MODEL rather than leaving the UI offering an
override that is silently ignored.
- Label token usage from initialize's _meta.zeroclaw.defaultModel, the only
model identity the ACP surface exposes. session/new returns just
{sessionId, workspaceDir}, so the two extractACPCurrentModelID reads were
dead code and the model-catalog discovery they fed could only ever return an
empty fallback while paying an ACP subprocess per catalog read. Return an
empty catalog directly, as qwenpaw/mcode do.
- Resume through session/resume instead of session/load. Both restore the
transcript into the agent, but load also replays every retained message back
as session/update notifications, which fed the previous answer into this
turn's deliverable. Add the acceptNotification turn gate every other ACP
backend carries, so out-of-turn chunks are dropped even if a runtime flushes
them early (#1997).
- Send agentAlias on session/new when the operator names one. ZeroClaw
auto-selects an agent only when its config holds exactly one entry, so
multi-agent installs failed with -32602 and had no way out: `zeroclaw acp`
has no --agent flag, and passing one kills the process at argument parsing.
The alias is consumed from custom_args and travels as a param; it is omitted
when unset, because a hardcoded guess breaks the single-agent case that
works today. The refusal now names both remedies.
- Stop advertising MCP support. No ZeroClaw handler reads params.mcpServers;
acp_enable_mcp gates that agent's own mcp_bundles, not the client list, so
MCP is operator-side only. Hide the MCP tab and warn on a stale mcp_config
instead of failing the task.
Also teach isACPSessionNotFound about ZeroClaw's custom -32000
SESSION_NOT_FOUND, without which a stale session id was retried forever
instead of restarting fresh. The wording check still decides, so a transient
error under the same code stays non-rejecting.
Tests now encode the observed handshake — _meta default model, session/new
without a catalog, -32000 for an unknown session, -32601 for set_model — and
cover the replayed-history, alias and MCP paths.
* fix(agent): make ZeroClaw integration merge-safe
Co-authored-by: multica-agent <[email protected]>
* fix(agent): tighten ZeroClaw ACP fallbacks
Co-authored-by: multica-agent <[email protected]>
* fix(agent): preserve ZeroClaw retry semantics
Co-authored-by: multica-agent <[email protected]>
---------
Co-authored-by: Sol-Boy <[email protected]>
Co-authored-by: multica-agent <[email protected]>
|