mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-04 17:18:35 +02:00
main
1230 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
niudakok
|
6b5d26b235 |
feat: add QwenPaw ACP backend support (MUL-5355) (#5986)
* feat: add QwenPaw ACP backend support Add qwenpaw as a supported agent backend. QwenPaw runs via `qwenpaw acp` over stdio using the ACP (Agent Client Protocol) JSON-RPC 2.0, reusing the hermesClient transport layer. Changes: - New backend: pkg/agent/qwenpaw.go (ACP stdio via hermesClient) - Register in agent.SupportedTypes, New(), launchHeaders - Daemon config: probe qwenpaw binary, QWENPAW_ARGS env var - Display name 'QwenPaw', inline system prompt support - Runtime config: AGENTS.md injection, .qwenpaw/skills/ discovery - User-level skills via QWENPAW_HOME env var - DB migration 224: add qwenpaw to protocol_family whitelist * fix(ci): add qwenpaw to CLI guard names and rename migration 231 - Add 'qwenpaw' to scripts/agent-cli-command-names.txt (TestAgentCLIGuardCoversDefaultCommands) - Rename migration to 231 to avoid collision with existing migrations (TestMigrationNumericPrefixesStayUniqueAfterLegacySet) * fix: address Bohan-J review comments on qwenpaw backend (PR #5986) Blocking fixes: 1. config.go: Add qwenpaw CLI probe so daemon discovers the binary 2. models.go: Add qwenpaw case returning empty list (same as qwen) 3. qwenpaw.go: Fix resume path — use session/load (QwenPaw implements load_session, not session/resume), use resolveResumedSessionID, only set resumeRejected on session-not-found errors, fail the run on set_model failure instead of silently continuing 4. daemon.go: Add qwenpaw→"QwenPaw" to runtimeDisplayNameOverrides 5. agent.go: Update error message to list qwenpaw Non-blocking fixes: 6. qwenpaw_test.go: Add 10 tests covering session/new, session/load, session/load not-found, set_model failure, ListModels, blocked args, protocol verification (session/load not session/resume), timeout, usage tracking, and backend construction 7. sidecar_manifest_test.go: Add qwenpaw to allFileBasedProviders * fix: address Bohan-J second-round review on qwenpaw backend (PR #5986) Blocking fixes: 1. qwenpaw.go: Send qwenpaw.coding_project_dir inside _meta, not as top-level parameter — ACP Pydantic model ignores extra root-level keys (model_config has no extra='allow'), only merges field_meta into kwargs 2. qwenpaw.go: Handle set_model returning result:null (not RPC error) — real QwenPaw server swallows exceptions and returns None, which serialises as JSON null; json.RawMessage('null') is non-nil so bytes.Equal check is needed 3. context.go: Workspace skills are at <workDir>/skills, not <workDir>/.qwenpaw/skills (confirmed against QwenPaw v2.0.1) 4. local_skills.go: QwenPaw resolves global root from QWENPAW_WORKING_DIR -> COPAW_WORKING_DIR -> ~/.copaw -> ~/.qwenpaw, not from QWENPAW_HOME Test additions: 5. TestQwenpawSetModelReturnsNull — regression test for result:null 6. TestQwenpawSessionLoadTransientError — transient error does not set ResumeRejected=true 7. TestQwenpawSessionNewSendsCodingProjectDir — verifies _meta format 8. TestQwenpawSessionLoadSendsCodingProjectDir — same for session/load 9. TestQwenpawTimeout — deterministic sync via signal file All 15 Qwenpaw tests pass. Verified against real qwenpaw v2.0.1 binary (end-to-end: prompt, session ID, resume, system prompt). * fix: rebase against upstream/main and add per-task qwenpaw workspace/agent isolation - Rebase add-qwenpaw-backend branch on upstream/main (88 commits ahead) - Resolve conflict in config.go: keep refactored probe() with shell resolution fallback, which already includes qwenpaw probe - Add --workspace and --agent CLI args to qwenpaw acp for per-task skill isolation and agent identity isolation - Mark --workspace and --agent as blocked in qwenpawBlockedArgs so user custom_args cannot override them - Add deriveQwenpawAgentID() to produce deterministic agent IDs from task's issue ID and agent ID - Wire QwenpawWorkspace and QwenpawAgentID through ExecOptions - Add prepareQwenpawWorkspace() in execenv to materialize bound skills into a per-task workspace directory * fix: address Bohan-J third-round review on qwenpaw backend (PR #5986) Three blocking issues resolved: 1. Agent ID registration — remove session/set_model and --agent entirely. The simpler route: QwenPaw model override is declared unsupported, eliminating the need for agent profile registration in QwenPaw config. 2. Skill revocation — prepareQwenpawWorkspace now does os.RemoveAll on the skill_pool dir and manifest before rebuilding, making it idempotent. A->empty (revoke all), A->B (replace), and A->A (repeated reuse) all work correctly. Added 3 new unit tests. 3. Skill root path — changed 'skills' to 'skill_pool' in both prepareQwenpawWorkspace and skillsDirPath to match QwenPaw's store.py get_workspace_skills_dir. Also cleaned up: removed deriveQwenpawAgentID function and its test, removed QwenpawAgentID from ExecOptions, removed --agent from qwenpawBlockedArgs, removed session/set_model test cases. * fix: add missing qwenpaw probe() call in agents_probe.go The TestDefaultAgentCommandNamesCoversAllProbes test found only 17 probe() calls in agents_probe.go but defaultAgentCommandNames has 18 entries. The probe() call for qwenpaw was missing, causing the backend CI test failure. Adding the probe ensures GUI-launched daemons can resolve qwenpaw via the login shell fallback, matching all other providers. * fix: sync models.go with upstream/main (remove qwenpaw from ListModels) * fix: CI failures — migrate prefix 235→236 + update test - migration prefix 235 was reused by upstream 235_chat_message_quick_actions; renamed our qwenpaw migration from 235 to 236 - TestQwenpawListModels called len() on Catalog struct (compile error); fixed to expect error for unknown provider type * fix: bump qwenpaw migration prefix 236->241 (upstream took 236) * fix: qwenpaw local skill root — 'skills' → 'skill_pool' (MUL-5355) Bohan-J third-round review blocker #3: local_skills.go still scanned <QWENPAW_HOME>/skills but the QwenPaw shared skill pool is <QWENPAW_HOME>/skill_pool (store.py get_workspace_skills_dir). Verified no other qwenpaw paths in the codebase assume the wrong layout. * fix: bump qwenpaw migration prefix 241->242 (upstream took 241) lint test fails with: migration prefix 241 is reused by [241_comment_parent_lookup_index 241_runtime_profile_add_qwenpaw]. Upstream added 241_comment_parent_lookup_index; bump our migration. * feat: add QwenPaw integration test and version declaration (MUL-5355) - New: server/pkg/agent/qwenpaw_integration_test.go with three agentintegration build-tagged tests: TestQwenpawRealACPSmoke — full end-to-end ACP smoke test with session/new → session/prompt and session/load resume validation. TestQwenpawRealWorkspaceSmoke — validates skill_pool workspace flag handling and per-task skill isolation. validateQwenpawVersion — attempt version detection via qwenpaw --version, pip show qwenpaw, and python import. - Document QwenPaw v2.0.1 as the supported baseline version in qwenpaw.go package comment, noting the contract details: _meta qwenpaw.coding_project_dir for Coding Mode, session/set_model NOT supported, skill_pool workspace layout. - This test suite is gated by MULTICA_RUN_REAL_AGENT_SMOKE=1 and requires qwenpaw on PATH, matching the pattern used by grok, cursor, and traeecli integration tests. * fix: bump qwenpaw migration prefix 242->243 (upstream took 242 for qoderclicn) Upstream added 242_runtime_profile_add_qoderclicn in the same rebase window, colliding with our 242_runtime_profile_add_qwenpaw. The migration lint test TestMigrationNumericPrefixesStayUniqueAfterLegacySet catches duplicate prefixes after the legacy range. Also add 'qoderclicn' to our migration's CHECK constraint so it doesn't regress the whitelist added by upstream's 242. * temp: stub out integration test to isolate CI failure * fix: restore upstream probeAgentCLIs() call in config.go (rebase regression) Rebase conflict resolution accidentally reverted upstream's MUL-5439 refactor (extracting probe logic to agents_probe.go) back to the old inline probe block. This also dropped qoderclicn detection and duplicated the probe logic already in agents_probe.go. Restore the single 'agents := probeAgentCLIs()' call — qwenpaw is already probed in agents_probe.go. * fix: bump qwenpaw migration prefix 243->251 (upstream took 243-250) Upstream added migrations 243-250 since our last rebase. Bump to 251, the next available prefix. * feat: add QwenPaw integration test (agentintegration build tag) TestQwenpawRealACPSmoke drives the real qwenpaw acp binary end-to-end: - session/new + session/prompt produces 'pong' - session/load resume with ResumeSessionID works - --workspace flag is forwarded correctly Gated by MULTICA_RUN_REAL_AGENT_SMOKE=1, matching grok/cursor pattern. Validated against QwenPaw v2.0.1. * fix: workspace skills dir 'skills' not 'skill_pool' + add skill loading integration test Two fixes: 1. qwenpaw_workspace.go: write skills to <workspace>/skills/ instead of <workspace>/skill_pool/. QwenPaw's get_workspace_skills_dir() looks for workspace skills at <workspace>/skills/ (store.py:65-67), not skill_pool (which is the shared pool at WORKING_DIR/skill_pool). Verified against real qwenpaw acp — skills in skill_pool/ were never discovered. 2. Add TestQwenpawRealWorkspaceSkill integration test that proves a bound skill is actually loaded and effective: writes a skill that overrides the agent's response, sends an unrelated prompt, and asserts the skill's marker text appears in the output. This addresses R3 review feedback: 'please also add a test that exercises an actually-bound skill'. All three integration tests pass against QwenPaw v2.0.1: - TestQwenpawRealACPSmoke (session/new + prompt + session/load resume) - TestQwenpawRealWorkspaceSkill (skill discovery + effectiveness) * feat: add ACP model discovery for qwenpaw via session/new models field QwenPaw v2.0.1+ now includes a 'models' field (SessionModelState) in the session/new response, added by agentscope-ai/QwenPaw#6531. This lets ACP clients discover available models without session/set_model. - ListModels for qwenpaw now uses discoverACPModels (same pattern as traecli/grok/kiro) to spin up 'qwenpaw acp', call session/new, and parse the models catalog from the response. - discoverQwenpawModels mirrors discoverTraecliModels — ACP-native, no auth selection needed. - Model override via session/set_model remains unsupported: it persists to agent.json at the agent scope (not session-scoped), so calling it would mutate the user's shared agent config. The model picker shows available models for display/selection, but the daemon does not send set_model. - Updated TestQwenpawListModels to verify qwenpaw is a recognized type (not 'unknown agent type' error). * fix: address Bohan-J Review 5 — ModelSelectionSupported=false, version bump to v2.1.0-beta.1, remove debug files - ModelSelectionSupported('qwenpaw') now returns false with rationale (session/set_model persists to agent scope, not session scope) - Add TestQwenpawModelSelectionUnsupported regression test - Update version references from v2.0.1 to v2.1.0-beta.1 (includes agentscope-ai/QwenPaw#6531 — models field in session/new response) - Remove check_ci.py, jobs.json, runs.json debug artifacts * fix: bump qwenpaw migration prefix 251->253 (upstream took 251) Upstream added 251_agent_runtime_unbind. Bump to 253, the next available prefix after 252_agent_builder_draft. * fix: address Bohan-J Review 6 — drop unused discovery, always attribute to unknown - ListModels for qwenpaw returns empty catalog without spawning ACP subprocess (model selection is unsupported, so no consumer exists) - Usage attribution always uses 'unknown' instead of opts.Model (the backend never sends opts.Model to QwenPaw) - Add TestQwenpawUsageModelIgnored regression test - Fix stale v2.0.1 comment in TestQwenpawListModels * chore(agent): clean up qwenpaw model-discovery leftovers Follow-up nits from review 7 on PR #5986: - Drop discoverQwenpawModels: it lost its only caller when ListModels started returning an empty catalog for qwenpaw. - Correct the version contract in qwenpaw.go. The execution path needs only the ACP surface present in v2.0.1 (current stable); the models field on session/new landed in v2.1.0-beta.1 but has no consumer now that model selection is unsupported. - Make TestQwenpawListModels actually guard the no-subprocess promise. It pointed at a nonexistent path, which the old discovery helper also answered with an empty catalog, so it passed either way. It now uses an executable fake that records invocation; verified it fails if a discovery path is reintroduced. - gofmt agent.go (ExecOptions alignment broke when QwenpawWorkspace was added) and restore the trailing newline in qwenpaw_test.go. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: niudakok <niudakok@users.noreply.github.com> Co-authored-by: Bohan-J <bohan.optimism@gmail.com> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Bohan Jiang
|
79c5832e1d |
MUL-5708 fix(taskfailure): classify response-side context-window overflow (#6366)
* fix(taskfailure): classify response-side context-window overflow (MUL-5708)
Claude Code 2.1.x reports an exhausted context window on the response,
not as a 400 on the request: the turn ends with stop_reason
"model_context_window_exceeded" and the CLI prints
API Error: The model has reached its context window limit.
That string carries none of the phrases rule 1 matched and no "token",
so it classified as agent_error.unknown. Unknown is absent from the
resume blacklists (resumeUnsafeFailureReason, GetLastTaskSession,
GetLastChatTaskSession), so the over-full session stayed pinned as the
resume pointer for the (agent, issue) pair and every later comment on
the issue resumed the same transcript and overflowed again.
Match the CLI copy and the raw stop reason so the failure lands in
agent_error.context_overflow, which those blacklists already exclude —
the next comment then starts from a fresh session instead of replaying
the overflow.
Co-authored-by: multica-agent <github@multica.ai>
* fix(taskfailure): upgrade an old daemon's catchall on context overflow (MUL-5708)
Installed daemons update on their own cadence, and FailTask only
re-classifies when the caller supplied no reason. A daemon whose rule 1
predates the response-side wordings reports agent_error.unknown, which
is on no resume blacklist — so until every host updates, the over-full
session stays pinned as the (agent, issue) resume pointer and every
later comment replays the same overflow. One un-upgraded host means a
permanently stuck issue, not just a missing label.
Recognise the two witnesses server-side in NormalizeDaemonReason, next
to the MUL-5370 skill-bundle rule it mirrors, so the retirement lands
the moment the server deploys. The accepted legacy set is narrower than
that rule's: only the catchall and the pre-MUL-1949 coarse agent_error.
A refined reason means the old daemon matched an earlier rule on the
same text, which says more about what ended the run than a witness
appearing somewhere in the blob does.
The witnesses move into one shared var so Classify and the normalizer
cannot drift apart.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
|
||
|
Bohan Jiang
|
1e661bb04b |
refactor(daemon): stage-1 judgment rewrite — mode router and small guardrail sections (MUL-5442) (#6365)
* refactor(daemon): compress the mode router and Reply-block preamble to their facts (MUL-5442) Stage 1 batch A of the judgment rewrite. The mode-router paragraph spent ~800 bytes explaining WHY the file cannot know the turn mode; the fact an agent needs is only WHERE the mode is named and that exactly one mode block applies. Compressed to a single Turn-mode lead carrying every routing fact: both mode lines, the shared-steps note, the one-block rule, and the no-mode-line default. The Reply block's first bullet drops its derivations (session history, confusion warnings) and keeps the id-source rule; the reply-warranted bullet keeps both pinned anchors and every behavioural outcome, dropping only the worked examples. Re-pins: the 'Mode router' heading anchor becomes '**Turn mode.**'. Co-authored-by: multica-agent <github@multica.ai> * refactor(daemon): trim the mechanism prose from the small guardrail sections (MUL-5442) Stage 1 batch B. Comment Formatting (both variants), Issue Body Formatting, and Always Use the CLI keep every ban, every incident id, and every pinned phrase verbatim; what goes is the mechanism narration (what the shell rewrites, why the CLI rejects outside paths, the resource-type enumeration). Zero test changes — every pin survives as-is. Co-authored-by: multica-agent <github@multica.ai> * fix(daemon): restore two platform boundaries and pin the routing rules (MUL-5442) Stage-1 review catches by Elon on #6365: 1. Two behavioural boundaries had been deleted as explanation. The issue-body H1 rule narrowed 'body or description' to 'body' — but 'description' is the CLI/API field name, so the alias is a cross-surface mapping. And step 2 lost 'CLI failures are normal' — platform failure semantics (a failed metadata read must not block the task), not tool mechanics. The second deletion also went undisclosed in the PR description. Both restored; both now pinned. 2. The compressed Turn-mode lead was guarded only by its markers: every routing rule (shared steps, apply-exactly-one, status difference, no-mode-line fallback, no status change) could have been dropped with green tests. All five now pinned as semantic anchors, and both Reply outcomes (work-produced arm, silent-exit arm) pinned individually alongside the existing heading anchors. Co-authored-by: multica-agent <github@multica.ai> * fix(daemon): pin the full no-mode-line mapping and gofmt the touched tests (MUL-5442) Final-review catches on #6365: runtime_config_kind_test.go:99 was not gofmt-clean (a scripted insertion), and the no-mode-line fallback was pinned as two separable halves — 'No mode line' plus a 'Reply mode' that other file content satisfies — so the text could mutate to route the fallback at Ownership mode with green tests. The anchor is now the exact mapping clause 'No mode line → Reply mode'; the status anchor stays. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Multica Eve
|
d60bc63f9a |
Revert "MUL-5516: fix(issues): re-trigger blocked issues when resumed (#6155)" (#6377)
This reverts commit
|
||
|
Naiyuan Qing
|
9fb46adc43 |
fix(agent-builder): serialise draft autosave against session delete (MUL-5642) (#6376)
SaveAgentBuilderDraft read the chat session, then upserted agent_builder_draft as a separate statement, with no lock between them. DeleteChatSession takes LockChatSessionForDelete for its whole transaction, so a save could pass its checks, block on nothing, and land its INSERT after the delete committed. agent_builder_draft carries no chat_session FK (repo rule), so nothing rejected that write. The surviving row held the configuration the user had just confirmed discarding. It is invisible to the UI — the drafts list joins through chat_session — and no prune can reach it: DeleteAgentBuilderDraft and the runtime teardown both key off a session that no longer exists, leaving only the workspace teardown. The client autosaves on an 800ms debounce and the conversation is addressable by URL, so a second tab can autosave at any moment while this one discards. Add LockChatSessionForDraftWrite, the same row and lock mode the delete and runtime-bind paths take, and run the upsert in a transaction that acquires it first and re-reads the session under it. Existence and status are the only two things a concurrent writer can change, and both are now decided inside the lock; workspace, creator and carrier are immutable for a session and stay on the cheap unlocked read. Either ordering is now correct: the save commits first and the delete prunes it, or the delete commits first and the save returns 404. The same lock closes the archive variant, where the last autosave after "create agent" could write a draft onto an already read-only session. Both regression tests drive the interleaving deterministically — hold the session row, prove the save blocks, then commit — and fail on the pre-fix handler with a 204 that writes the orphan. Co-authored-by: multica-agent <github@multica.ai> |
||
|
Jiayuan Zhang
|
c1ec1f4646 |
docs: cut docs/ to design + product-overview, English by default (MUL-5698) (#6364)
* docs: delete completed plan docs and fix stale repo documentation (MUL-5698) Ten repo docs were verifiably out of date against the code. Completed plans are deleted outright rather than archived — they are dead weight in every agent's context and their decisions already live in the code. Deleted (all describe work that has shipped): - docs/agent-quick-create-plan.md — marked "未动工", but agenttmpl templates (Phase 1) and the AI-create-agent page (Phase 3) are live - docs/docs-outline.md — tracker planning "Chinese only, 25 pages"; the doc site is 39 pages x 4 languages - docs/docs-rewrite-plan.md — plans 55 mdx, lists webhook autopilot triggers as unrouted; both superseded by what shipped - docs/docs-onboarding-optimization-plan.md — work log for #5714, which landed the four languages and screenshots it lists as pending - docs/onboarding-refactor-plan.md — v3 shipped (welcome-store, welcome-after-onboarding, onboarding_shim) - apps/mobile/docs/project-v1-{plan,gap-audit}.md — marked "pre-implementation"; the picker files they scope no longer exist - docs/plans/* + docs/ideation/* — implementation plans for agent access-scope (agent.visibility) and issue-table server query (/api/issues/grouped), both shipped Fixed: - docs/codex-sandbox-troubleshooting.md — the decision matrix claimed non-darwin gets workspace-write. Linux is danger-full-access (MUL-5578) and Windows is danger-full-access (MUL-4957); Windows had no row at all - apps/mobile/docs/rnr-migration.md — Phase 1 is complete, not "not started"; every checklist item is in the tree - AGENTS.md — architecture list was missing apps/mobile, apps/docs, packages/eslint-config - docs/ui-consistency-audit.md — §3.2 status column is a mid-PR snapshot; #5263 and #5258 have merged - server/internal/agenttmpl/loader.go — drop reference to a deleted doc Co-authored-by: multica-agent <github@multica.ai> * docs: reduce docs/ to design + product-overview, English by default (MUL-5698) docs/ now holds two documents, each with an English default and a .zh.md translation. Everything else was engineering scratch that agents load as context on every run without ever being read by a human. Deleted: - docs/analytics.md, docs/feature-flags.md, docs/timezone-architecture-rfc.md, docs/codex-sandbox-troubleshooting.md, docs/codex-usage-cache-backfill.md, docs/custom-runtimes.md, docs/ui-consistency-audit.md Language convention — English is the default filename, translations carry a language suffix: - docs/design.md (new English) + docs/design.zh.md (was design.md) - docs/product-overview.md (new English) + docs/product-overview.zh.md (was product-overview.md) While translating product-overview, three facts were corrected against the code rather than carried over from the 2026-04-21 survey: the provider list now matches README, onboarding is the shipped three-step about_you/workspace/runtime sequence with Helper creation moved after exit (packages/core/onboarding/step-order.ts), and the stale "28 tables" total was dropped. Reference cleanup so no comment points at a deleted file — .env.example, server/internal/analytics/{client,events}.go, packages/core/analytics/index.ts, server/cmd/server/main.go, server/pkg/featureflag/doc.go, server/cmd/backfill_task_usage_hourly/main.go, and the doc pointers in migrations 100/101/103/104. Migration edits are comment-only; the runner tracks applied versions by filename, not by checksum. docs/assets/ is kept — README.md and README.zh-CN.md embed those images. Co-authored-by: multica-agent <github@multica.ai> * docs: drop product-overview, fix rnr body and CLAUDE/AGENTS drift (MUL-5698) Addresses all four blockers from review. 1+2. Delete docs/product-overview.md and docs/product-overview.zh.md. The review found the doc carried facts that would make an agent do the wrong thing — skill injection claimed a .agent_context/skills fallback for providers that now have native paths in execenv/context.go, and it documented `multica skill create --title` when the CLI only registers --name. Rather than chase those, the document goes: it is derived from code and can be regenerated from code when it is actually wanted. That also removes the zh-as-historical-snapshot problem, since neither language survives. docs/ is now design.md + design.zh.md + assets/. design.zh.md is a faithful translation of the English, not a snapshot, so blocker 1 does not apply to it. 3. apps/mobile/docs/rnr-migration.md: the body contradicted its own status line. §1 asserted in present tense that there is no theming infrastructure, hardcoded tailwind hex, and a three-line global.css; §5.2 said the same. Both are now marked as the pre-Phase-1 baseline with the shipped state noted, §6's Phase 0/1 checklists are backfilled as complete, and Phase 2 is labelled not started. 4. CLAUDE.md gains apps/docs/ and packages/eslint-config/, so the authoritative Project Shape list matches the pointer list in AGENTS.md. The two lists are now identical. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Lambda <lambda@multica.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Bohan Jiang
|
eea687d461 |
MUL-5686: fix(chat): resume the cancelled turn's session instead of starting cold (#6352)
* fix(chat): resume the cancelled turn's session instead of starting cold
Stopping a chat turn the agent had already begun answering, then sending
the next message, produced a reply with no memory of the conversation.
The cancelled turn's provider session is real and recorded — the daemon
pins it onto the task row mid-flight and cancellation keeps it there —
but nothing could hand it back:
- GetLastChatTaskSession / GetLastTaskSession only considered
'completed' and 'failed' rows, so a cancelled row's session was
invisible to resume resolution;
- chat_session.session_id is written only by CompleteTask / FailTask,
and a cancelled task reaches neither: the daemon discards its result
and sends a cancel-ack. On a chat whose first turn was cancelled the
pointer therefore stayed NULL and the next turn started cold, since
buildChatPrompt injects only the current user message.
Let cancelled rows into both resume lookups, and advance the chat-level
pointer at cancel time so a provider that mints a new session id per
resume does not rewind past the cancelled exchange. The mid-flight pin
may now also fill an EMPTY session slot on a just-cancelled row, which
is what a Codex pin waiting on its rollout needs when the cancel wins
the race; occupied slots and completed/failed rows stay untouchable.
Retired sessions, poisoned-failure filters and the rollout-present guard
are unchanged. A transcript killed mid-tool-call that the provider later
refuses is still caught by taskfailure.UnresumableHistory, which retires
the session and starts the next turn fresh.
Fixes #6340
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): close the two cancel/pin races on the chat resume pointer
Review found the pointer advance had the same shape of hole it was meant
to fix, on both sides of the cancel.
1. The status flip and the pointer advance were separate statements. In
the gap the row is already `cancelled` while the pointer still names
the PREVIOUS turn, so a follow-up the user had queued could be claimed
there and resume the older session — and a failed pointer write only
logged, still reporting the cancel as successful. Both now commit in
one transaction and the error propagates.
2. The mid-flight pin may land AFTER the cancel (for Codex it waits for
the rollout), so the cancel transaction sees no session to publish and
the pin only fills the task row. On a chat that already had history
the stale pointer kept shadowing it, which is precisely the case the
pin change was added for.
Both paths now run one guarded statement,
AdvanceCancelledChatSessionPointer. It reads the task row itself rather
than trusting an in-memory copy, ignores anything that is not a cancelled
chat task, and refuses to move the pointer when a NEWER task on the chat
already recorded a session — so a straggler pin cannot drag the
conversation back onto the interrupted turn.
Regression tests, both failing before this commit: a cancel whose pointer
write is blocked must not expose `cancelled` to another connection, and a
late pin on an already-cancelled row must reach the next real claim. The
newer-turn guard is covered too.
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): make the pin atomic and restore the chat->task lock order
Second review round found the remaining half of the same class of bug.
1. PinTaskSession still committed the session onto the task row and
advanced the chat pointer as two statements, and the pointer failure
only logged while the endpoint still answered 204. A follow-up claimed
in that window resumes the previous turn — the exact case the pin
change was added for. Both writes now share one transaction and every
failure is reported.
2. Adding the pointer write gave the cancel transaction two rows to hold,
and it took them in the wrong order: agent_task_queue first, then
chat_session. DeleteChatSession takes them the other way round, so the
two could deadlock (40P01, and runInTx has no deadlock retry). The
repo's documented order is chat_session -> agent_task_queue; both the
cancel and the pin now open with LockChatSessionForTask, the same
helper FinalizeDeferredCancelledChat uses. ErrNoRows there means a
non-chat task or an already-deleted session — nothing to lock and
nothing to advance.
Regression tests, all three failing before this commit (the concurrency
one with a real `deadlock detected`): the pin must not expose a session
on the task row while its pointer write is blocked; a cancel waiting on
the chat session must hold no lock on the task row (FOR UPDATE NOWAIT
probe); and cancel racing a chat delete must never come back with 40P01.
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): put every chat-task terminal write behind the same lock order
The cancel and pin paths took chat_session before agent_task_queue; the
terminal reports still took them the other way round, so the two could
deadlock (40P01) — reproduced deterministically as cancel-vs-complete.
Choosing per-path was never going to work: either every writer that holds
both rows agrees on an order or none of them are safe.
CompleteTask, FailTask and the cancelled-chat finalize now open with the
same LockChatSessionForTask the cancel, pin, DeleteChatSession and
FinalizeDeferredCancelledChat paths use, via a shared helper that
documents the invariant in one place.
Also de-flakes the concurrency tests this PR added. They held a row lock
and then read other rows on a FRESH pooled connection; the suite shares
one database, so a sibling package's DDL could queue an ACCESS EXCLUSIVE
request in between, park our later ACCESS SHARE request behind it, and
wedge the package until the 10-minute timeout (observed in a parallel
`go test ./internal/...` run). Those transactions now take their table
locks up front and read on the connection that already holds them, and
every racing call is bounded so a stall fails loudly instead of hanging.
Regression tests, all failing before this commit: complete and fail must
hold no lock on the task row while waiting for the chat session (FOR
UPDATE NOWAIT probe), and cancel-vs-complete plus pin-vs-fail must never
come back with 40P01.
Co-authored-by: multica-agent <github@multica.ai>
* test(chat): fail the race tests on any unexpected error, bound every call
Review nits on the concurrency tests.
Matching only *pgconn.PgError(40P01) let the pin side through: a pin that
loses a deadlock is reported by the HTTP handler as a plain 500 with no
PgError to unwrap, so the check skipped exactly the failure the test
exists for. Both racers now fail on anything except the one benign
outcome — whoever finalized the task first leaves the loser matching no
row (pgx.ErrNoRows).
The remaining racing calls still used an unbounded context.Background(),
which this PR had already claimed were bounded. They now share the same
raceTimeout as the rest.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: multica-agent <github@multica.ai>
|
||
|
Bohan Jiang
|
84fe84cd9e |
fix(daemon): resolve six injected-instruction contradictions (MUL-5696 conflict audit) (#6354)
* fix(daemon): resolve six injected-instruction contradictions found by the MUL-5696 conflict audit Cross-checking the four injection surfaces (runtime brief, per-turn prompts, built-in skills, CLI help) against each other: - brief taught --due-date <RFC3339> while the CLI help, the projects skill, and util.ParseCalendarDate all pin YYYY-MM-DD (RFC3339 passes only at exact UTC midnight); 3 occurrences aligned to <YYYY-MM-DD> - autopilot per-turn prompt banned 'multica issue get' unconditionally while the brief's autopilot workflow allows issue commands when the autopilot instructions direct issue work; prompt now mirrors the brief - quick-create per-turn prompt said 'do NOT pass --attachment' while the quick-create ## Output section (test-pinned) names --attachment on the create call as the surface's only file channel; ban is now scoped to URLs from user input - assignment prompt claimed the workflow file documents pagination, but MUL-5442 (#6347) retreated those semantics to --help; pointer updated - 'attachment download' help example recommended -o /tmp/images, the exact machine-shared path the --content-file/--attachment workdir guards reject (MUL-4252) and the brief's Attachments section assumes away; example moved inside the workdir - multica-squads skill still listed an unbounded 'issue comment list' pull, contradicting the never-one-bulk-pull doctrine (same class as MUL-5372 / #6347); replaced with the bounded roots scan - multica-working-on-issues pinned a different metadata write bar ('explicit task requirement') than the brief's two-condition bar; aligned to the brief MUL-5696 Co-authored-by: multica-agent <github@multica.ai> * fix(cli): create the download output directory; pin MUL-5696 alignments with tests Review follow-ups for PR #6354 (MUL-5696): - 'attachment download -o' now creates the output directory, so the help example ('-o ./attachments') works from a clean workdir; regression test added and verified red without the fix - the run-only autopilot issue-command boundary is now a single shared constant (execenv.AutopilotIssueCommandsGuard) emitted by both the brief and the per-turn prompt, so the two copies cannot drift apart again - regression pins for the audit alignments: the brief --due-date synopsis stays calendar-day (<RFC3339> banned), the quick-create prompt keeps the URL-scoped --attachment boundary (blanket ban banned), the squads skill carries no unbounded comment read, and working-on-issues keeps the brief's metadata write bar Co-authored-by: multica-agent <github@multica.ai> * test(cli): let attachment download tests run inside agent workdirs Second-round review follow-up for PR #6354 (MUL-5696): the download tests set only setCLITestServerEnv's non-mat_ token, so inside an agent workdir (daemon task marker present) newAPIClient rejected them before the download logic ran — including the new directory-creation test, which made it a net-new in-tree failure. All three download tests now set a task-scoped mat_ test token, the same pattern the upload tests already use; in a standard agent workdir the full-suite failure set drops from 94 (base) to 92, with the two pre-existing download failures fixed and none added. Also bans the old 'explicit task requirement' metadata phrase in the working-on-issues skill test (review nit). Co-authored-by: multica-agent <github@multica.ai> * refactor(daemon): single-source the autopilot guard in the brief; trim per-turn prompt additions Follow-up to keep PR #6354 byte-neutral-or-better per surface: - the per-turn autopilot prompt no longer restates the issue-command boundary; the brief's autopilot workflow section is its single emission point (a second hand-maintained copy is exactly how the two drifted before), pinned on both sides - the assignment prompt's read-surface pointer and the quick-create attachment bullet are tightened Measured against origin/main with identical contexts: per-turn prompts assignment -26 B, autopilot -68 B, quick-create -7 B; briefs +6/+6/+3 B (YYYY-MM-DD is 3 bytes longer than RFC3339). Net per run: assignment -20 B, autopilot -62 B, quick-create -4 B, comment-reply +6 B. Rebase note: the audit's metadata-bar finding is superseded by #6351, which landed the opposite canonical direction (brief defers to the skill); the rebase keeps main's owner-ruled skill text and drops the now-wrong test pins. --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Bohan Jiang
|
dc60429366 |
refactor(daemon): slim Issue Metadata to its judgment core, defer discipline to the skill (MUL-5442) (#6351)
* refactor(daemon): fold the metadata ban list into the write rule (MUL-5442) The Issue Metadata section spent 1,024 bytes teaching a KV bag. The What-NOT-to-pin bullet was a separate heading restating the write rule's negative space; it folds into Write on exit as one sentence with every ban kept explicitly (secrets/tokens/API keys, logs or comment summaries, runtime bookkeeping, single-run details). The parenthetical examples and connective prose go; every rule stays. Pins updated in kind: the merged ban list is pinned as one sentence plus the result-comment redirect, so the next compression pass cannot drop a ban without failing CI. -202 bytes (16,099 -> 15,897 on the standard fixture). Co-authored-by: multica-agent <github@multica.ai> * fix(daemon): restore the runtime-bookkeeping examples that define the ban (MUL-5442) Review catch by Elon on #6351: 'runtime bookkeeping' has no other definition anywhere in the brief or skills, so its examples (attempts, run timestamps, agent IDs) are the category's boundary, not connective prose — and all three are high-probability miswrites that can look like re-readable diagnostics or durable facts. Restored inside the merged bullet. Also adopts the review's pin advice, which is our own anchor-pin doctrine applied properly: the whole-sentence pin from the previous commit is replaced with separate semantic anchors (each ban plus the bookkeeping examples plus the result-comment redirect), so the sentence can be reworded later without CI churn while dropping any single ban still fails. +52 bytes; the section lands at 874 (from 1,024), the PR nets -150. Co-authored-by: multica-agent <github@multica.ai> * refactor(daemon): defer the metadata write discipline to the working-on-issues skill (MUL-5442) Owner decision on MUL-5442: metadata is deliberately free-form custom key-value state — the recommended-keys block never matched the feature's intent and is removed outright, not relocated. The full ban list defers to the multica-working-on-issues skill, which already carried a near-complete copy; the skill gains the two bans it lacked (secrets/tokens/API keys, agent ids) so nothing loses its home. The brief keeps only what the interface cannot express: the read-as-hints stance, the will-a-future-run-re-read-it bar, and the two write-time boundaries (never secrets, never long content). Section: 874 -> 505 bytes (1,024 at the round's start). Pins move with the content: the brief side slims to the surviving semantics plus the skill pointer; the skill-side contract test gains the relocated ban anchors so the pointer cannot dangle. Co-authored-by: multica-agent <github@multica.ai> * fix(daemon): remove the curated key list from the skill and restore the ban categories (MUL-5442) Round-3 review catches by Elon on #6351, both accepted: 1. The recommended-keys concept survived in the skill — which loads exactly when an agent is about to write metadata, so it still steered free-form KV into a platform vocabulary. The owner's ruling was that the concept should not exist, not that it should move. The key list, the 'high-signal keys only' heading, and the pr_url-specific example are gone; the section now describes free-form durable custom state and a generic set example. A mustNotContain guard keeps the curation from creeping back. 2. The relocated ban list had dropped its two defining categories (runtime bookkeeping, other single-run details), leaving unlisted values looking writable. Restored with the reviewer's structure: each category names its examples, and the test pins every category AND every example as separate line-safe anchors — no item can be silently dropped again. Co-authored-by: multica-agent <github@multica.ai> * fix(daemon): remove the last key-vocabulary residue from the property-vs-metadata note (MUL-5442) Round-4 review catch by Elon on #6351: the property-vs-metadata bullet still read 'free-form scratchpad for run state (pr_url, waiting_on, ...)' — recommending the ruled-out fields AND calling metadata a home for run state, in direct conflict with the runtime-bookkeeping ban restored two sections above. Now reads 'free-form bag for durable custom issue state', consistent with both the owner ruling and the ban list. Test side per the review: the broad pr_url anchor narrows to the full stale-warning phrase (the one sanctioned pr_url reference — a negative compatibility note, not a write recommendation), and the curation guard gains the removed residue phrases so neither the vocabulary nor the run-state framing can creep back. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Bohan Jiang
|
766c7a7fa2 |
refactor(daemon): retreat deep read semantics to --help, trim BTS prose (MUL-5442) (#6347)
* feat(cli): carry the comment-read contract in comment list --help (MUL-5442) The --recent flag help gains the MUL-5372 saturation semantics (N caps threads, not comments; every thread returns uncapped; small issues return the whole history) and a pointer at the bounded alternative. The --before help names the stderr cursor labels alongside the response header. This is the relocation target for the brief's deep read semantics: the contract follows the flag, and TestIssueCommentListHelpCarriesReadContract pins it here so the brief-side pointer cannot dangle. Co-authored-by: multica-agent <github@multica.ai> * refactor(daemon): retreat deep read semantics to --help and trim BTS prose (MUL-5442) Unblocked by #6309: the platform no longer recommends --recent anywhere, so the flag's deep semantics no longer need to ride in every brief. - Available Commands comment-list entry: keep the signature, the two bounded read shapes, and the one-clause saturation warning; per-thread cap detail, folding rules, and cursor labels retreat to --help (pinned there by TestIssueCommentListHelpCarriesReadContract). - Workflow step 3: drop the worked examples and the redundant trailing pointer; the mandatory two-step read and both motivation pins stay. - Background Task Safety: tighten connective prose only — every pinned phrase and every behavioural constraint is untouched. - multica-squads SKILL.md quick-start: replace the last remaining --recent recommendation on the platform with the bounded scan — same class of fix as #6309, found while relocating the warning. -676 bytes on the standard fixture (16,766 -> 16,090). Co-authored-by: multica-agent <github@multica.ai> * fix(cli,daemon): repair the help rendering, restore the BTS orphan scope, complete the squads read (MUL-5442) Three review catches by Elon on #6347: 1. The --before usage string wrapped the cursor labels in backticks, and pflag's UnquoteUsage hijacked the first pair as the flag's value placeholder — rendered help showed '--before Next thread cursor' instead of '--before string' (same regression class as TestLoginTokenHelpOutputRendersCleanly). Labels now use double quotes, and the --recent help no longer suggests composing mutually exclusive flags ('--roots-only + --thread --tail' -> an explicit two-step read). TestIssueCommentListHelpCarriesReadContract now asserts on the RENDERED FlagUsages output, pinning '--before string' and the two-step order. 2. The compressed BTS opening said 'anything still running is orphaned', which swept externally-owned work (GitHub Actions) into the orphan rule the same section later scopes out. Restored: 'any run-owned work still active is orphaned', pinned. 3. The squads quick-start's roots-only scan never returns reply bodies, where mention triggers and failure reasons usually live. Added the bounded drill-down step and the sequence rationale; both reads pinned in TestSquadsSkillCoversLeaderRoutingContract, which also guards against a regression back to --recent. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Bohan Jiang
|
757b09c801 |
fix(server): remove the last language seeds from the LLM prompts (MUL-5689) (#6348)
* fix(server): drop the Chinese seed from the chat title prompt (MUL-5689) chatTitleSystemPrompt carried literal Chinese in two places: a rule that spelled out "Chinese input → Chinese title", and a formatting example listing "标题：" as a prefix not to use. It is the same defect as the quick-actions label rule — a prompt that names a language, even only as a formatting example, reads as permission to answer in it. Chat titles are generated from the user's opening message alone, so they have neither the ALREADY SUGGESTED feedback loop nor an agent reply to be pulled by. Removing the seed is the whole fix; the language rule stays, now stated without naming a language. Nothing is lost by dropping the "标题：" example: chatTitleLabelPrefixes already strips 标题/题目/主题 (and the English forms) from the model's output, which is where that guarantee actually lives. Co-authored-by: multica-agent <github@multica.ai> * fix(server): correct two inaccuracies in the quick-actions language rule Both from review on #6345, non-blocking there and deferred to keep that merge unblocked. "these instructions" was self-referential: the LANGUAGE RULE is itself an instruction, so a model reading "ignore these instructions when choosing the language" could read the rule as disowning itself. It means the system prompt, so it now says so. The system prompt claimed the user message "ends with" the LANGUAGE RULE. It does not — the task line follows it. Reworded to "contains a LANGUAGE RULE line near the end", which is what the renderer actually produces. The rule's position is unchanged: still after the conversation and the replayed labels, which is the part that matters. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Bao Ming
|
738217c275 |
MUL-5516: fix(issues): re-trigger blocked issues when resumed (#6155)
* fix(issues): re-trigger blocked issues when resumed * fix(issues): enforce agent permission on resume --------- Co-authored-by: baoming.lv <baoming.lv@ly.com> |
||
|
Bohan Jiang
|
2f431cd74a |
fix(server): follow the user's language in chat quick actions (MUL-5689) (#6345)
Quick actions never pinned their output language: the system prompt named Chinese in a rule about button width, and ALREADY SUGGESTED replayed the previous turn's labels, so one bad pass seeded the next. Neither prompt names a language now, and the user message closes with an explicit rule anchored on the most recent [user] turn, disowning the agent's reply, older turns, the instructions, and the replayed labels. MUL-5689 |
||
|
YYClaw
|
fe62be266d | fix(channel): defer bare fresh commands (#6338) | ||
|
Xichang(Seacen) Zhao
|
671a1a0c21 |
fix(server): read the chat's channel from its binding instead of guessing (#6330)
A claim reported chat_channel_type by trying the binding lookup once per
channel it knew the name of. The list was Slack alone, then {Slack, Feishu}
after MUL-4899. Any channel added later — WeCom is the first — fell off the
end and claimed as a web chat, so the runtime brief told the agent to deliver
files with `multica attachment upload` into a conversation that cannot carry
an attachment. That is the same failure MUL-4899 fixed, reintroduced by the
shape of the fix.
Every channel writes the same channel_chat_session_binding row and differs
only in channel_type, and UNIQUE (chat_session_id) allows at most one, so the
row already holds the answer. Read it by session id and take channel_type off
what comes back. chat_in_thread stays Slack-only, now keyed on the row's own
channel_type: the two commands it picks between are hardwired to the Slack
history reader, and no other channel has one.
The channel_type-scoped query stays for the outbound senders, which are
per-platform by construction and must not deliver into a foreign channel.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
feig
|
e422b169a8 |
MUL-5415: fix(issues): keep running-agent transcript open
Closes #6053 |
||
|
Bohan Jiang
|
e92d828f0c |
refactor(daemon): compress brief prose and demote the sub-issue playbook (MUL-5442) (#6310)
* refactor(daemon): compress brief prose and demote the sub-issue playbook (MUL-5442) Six static edits to the issue brief, -1,249 bytes on the standard fixture (17,977 -> 16,728). All edits are identical for every issue run; prompt-cache byte stability (#6008) is untouched. - BTS persistent-service bullet: keep the handoff contract (deliverable-only, lifecycle detached, readiness verified, best-effort survival), drop the operational walkthrough. Pins re-anchored to concepts (durable logs, recorded PID, verify readiness). - BTS CI-ban bullet: keep the command blacklist and the complete-hand-off rule, drop the auto-merge/snapshot elaboration. - Ownership mode: state the identity-forbids clause once on the header instead of once per status bullet. - Delivery invariant: merge the three sub-bullets into the lead paragraph; also fixes the stale '(below)' — the per-surface delivery line renders above the invariant, not below it. - Sub-issue Creation: demote the todo/backlog/stage playbook to the multica-working-on-issues skill; the brief keeps a one-line flag map plus the skill pointer. Skill-side anchors added to TestWorkingOnIssuesSkillCoversIssueLoopContracts so the pointer cannot dangle. - Attachments: collapse the two-sentence CLI-fetch intro into one line. Every pinned behavioral phrase is either carried verbatim or re-pinned to an equivalent semantic anchor in the same assertion; no assertion is deleted without a replacement. Co-authored-by: multica-agent <github@multica.ai> * fix(daemon): restore the full persistent-service handoff contract (MUL-5442) Review catch by Elon on #6310: the compressed bullet weakened the contract two ways — the reply requirement dropped 'logs' from the URL/logs/stop triple (durable logs alone are unobservable if the user is never told where they are), and the general ownership/cleanup handle narrowed to a bare PID (a supervisor/profile-managed service has no single stable PID). Both test pin sets had been updated to the weakened phrases, which would have made the regression look legitimate. Keeps the compressed sentence shape; restores both halves of the contract and pins them ("cleanup handle such as PID/profile", "URL, logs, and stop instructions") so they cannot be compressed away again. +38 bytes; the PR still nets -1,211 against main. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Bohan Jiang
|
3c6176cbcd |
refactor(daemon): de-duplicate cross-section rules in the runtime brief (MUL-5442) (#6302)
* refactor(daemon): fold four restatements of the end-of-turn rule into one (MUL-5442) Background Task Safety opened with four bullets that were four views of the same rule -- do not end the turn with run-owned work outstanding: the general ban, the "tool says it will notify you" case, the unobservable-result case, and the "standing by" sign-off. Separating them cost bytes without adding a distinct behaviour. Fold them into the leading bullet. Every phrase the behaviour tests pin is carried over verbatim, including "Do NOT end your turn while background tasks", "Never background-and-yield", "wait for a future notification/reminder", "running in the background so you can keep working", "run the work synchronously instead" and "standing by". MUL-5442 Co-authored-by: multica-agent <github@multica.ai> * refactor(daemon): demote workflow-step restatements of delivery, mention and metadata policy to pointers (MUL-5442) The six workflow steps and the Reply mode block restated policy that already has a dedicated section. Each restatement is a second place to edit when the policy changes, which is how they drifted apart in the first place. Give each rule one canonical home and leave a pointer at the call site: - delivery ("only a comment reaches the user") -> ## Output; steps 5 and the Reply block point at it. - mention discipline -> ## Mentions. The reply-time phrasing the loop-hardening test pins moves into that section rather than being duplicated in the Reply block, so the anti-loop signal is unchanged. - metadata read/write bar -> ## Issue Metadata; steps 2 and 6 point at it instead of paraphrasing the bar. Tests: the metadata scope test pinned the old pointer wording, and the mention test's comment claimed the sign-off rule lived in the workflow steps. Both are updated to the new placement; every behavioural phrase they guard is still asserted, file-wide. MUL-5442 Co-authored-by: multica-agent <github@multica.ai> * refactor(daemon): give the comment-read surface and the file-safety rules one home each (MUL-5442) Three cross-section duplications, each resolved toward the section that owns the rule: - comment reads: the workflow step and the Available Commands entry both explained what --roots-only / --summary / --thread --tail do. The step keeps the two reads it mandates and its anti-stale motive; flag semantics stay in Available Commands, which is the single discovery point. The saturation trap ("caps THREADS, not comments") and the pagination cursor labels stay put -- they are load-bearing after MUL-5372 and remain asserted. - --content-file: the comment add entry restated the guardrail Comment Formatting owns; it now names the rule and points there for the rationale. - workdir path rule (MUL-4252): issue create carried its own copy of the stale-file rationale; it keeps the rule and defers the why. - inbound attachments: trimmed to the pinned rule plus a pointer. MUL-5442 Co-authored-by: multica-agent <github@multica.ai> * refactor(daemon): merge the Agent Identity action list, gate squad maintenance, trim the attachment restatement (MUL-5442) Three follow-ups from review, each re-examined against what the text guards today rather than against the fact that a test pinned it. 1. Agent Identity enumeration. Instruction Precedence and workflow step 4 were added in the same commit (#3802) and each carried its own list of actions Agent Identity can forbid -- and the lists disagreed: one named status changes, the other named issue create/update and delegation, neither contained the other. Merge them into Instruction Precedence, which owns the rule. Step 4 keeps only what that section cannot express: a delegation-only role stops once its delegation is delivered. 2. Squad maintenance. `multica squad member set-role` shipped to every run, including every agent that leads no squad and therefore has no squad whose roles it could change. Gate it on IsSquadLeader -- agent configuration, not per-run state, so the brief stays byte-stable across runs of one session (MUL-5377), the same predicate the workflow already branches on. 3. Inbound attachments. The section restated Output's no-clickable-local-path rule verbatim. Keep the framing Output cannot express -- a downloaded attachment feels shared but landed in a private workdir -- and point at Output for the rule. The delivery test now also asserts the pointed-at rule is present, so the pointer cannot dangle. Brief size, plain issue task: 19,231 -> 18,009 bytes for an ordinary agent (-6.4%), 20,718 -> 19,759 for a squad leader (-4.6%). MUL-5442 Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Bohan Jiang
|
459dd90f82 |
fix(daemon): stop the coalesced-comment fallback from pulling the whole issue history (MUL-5442) (#6309)
* fix(daemon): stop the coalesced-comment fallback from pulling the whole issue history (MUL-5442) When a run covers comments that arrived before it started and the server sent only their ids (no bodies), the prompt told the agent to find them by running `multica issue comment list <issue> --recent 30`. `--recent N` caps THREADS, not comments, and every returned thread carries all of its descendants. On an issue with fewer than 30 root threads that is the entire comment history — measured on a live 3-thread issue: 88,301 bytes (~22k tokens) to locate two ids. It also contradicted the runtime brief's own catch-up step, which tells the agent to read in two bounded steps and never make one bulk pull (MUL-5372): the platform was recommending the exact shape it forbids elsewhere. Those comments all arrived between the agent's previous run and this one, so when the server supplied that anchor `--since` returns precisely them in one bounded read — 17,376 bytes on the same issue. When there is no anchor (no prior run on this issue, so the server sends none) the fallback now uses the same scan-then-expand pair the brief teaches instead of a bulk pull. Also drops the "--full if a thread is folded" note: per `comment list --help`, `--since`, `--tail` and `--roots-only` reads are never folded, so it was advice that could not apply to either replacement read. This was the last place the platform steered agents onto `--recent`. The flag and its saturation warning stay documented in Available Commands, since an agent can still choose it. MUL-5442 Co-authored-by: multica-agent <github@multica.ai> * fix(daemon): make the coalesced-id lookup deterministic, not window-dependent (MUL-5442) Review found the previous version could still drop a user instruction. `--since` is not a reliable lower bound for these ids. A retry inherits the previous attempt's coalesced_comment_ids verbatim (queries/agent.sql RetryTask) while the anchor is recomputed from the last STARTED task's started_at (GetLastTaskStartedAtForIssueAndAgent), so an inherited id can predate the anchor. If any unrelated comment lands after the anchor, NewCommentsSince is populated, the prompt sent the agent at the window, and the inherited id was simply not in the result. It was not a precise fetch in the other direction either -- the window also carries the trigger comment and unrelated comments. Replace the window-or-heuristic pair with one deterministic contract: `--thread` accepts ANY comment id, reply or root, and the server resolves it to the containing thread. So every id is fetchable directly and bounded, with `--before`/`--before-id` paging when it is older than the tail window. That pass now runs unconditionally; `--since` is demoted to an optional prefetch, described as a candidate window rather than an exact fetch. This also removes the anchorless branch's "expand the threads whose last_activity_at is recent" heuristic. Completeness of user instructions is not a good place for the agent to guess. Tests assert the contract rather than the command spelling: the per-id lookup, the reply-id capability, cursor paging and the "account for every id" rule are required in both shapes; the anchored shape must not overpromise the window; the anchorless shape must carry no --since and no recency heuristic. MUL-5442 Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Naiyuan Qing
|
a5c1d44701 |
MUL-5642: fix(agents): stop the creation studio polling, and stop it losing work (#6307)
* feat(agents): make AI agent creation resumable (#6246) Leaving the Agent Creation Studio destroyed the conversation. The unmount cleanup called deleteChatSession, so a sidebar click, a tab close or a route change deleted the builder session and every message in it — the bug external users reported. Archiving instead (PR #6247) would have stopped the deletion without giving anyone a way back in: builder sessions hang off a hidden `kind = 'system'` carrier agent, which the `kind = 'user'` filter keeps out of every chat list, so an archived one is unreachable rather than recoverable. A creation conversation is now a durable object with its own address. Server: - GET /api/agent-builder/sessions lists the caller's unfinished creations. Creator-scoped like every other chat read. It reports the CARRIER's runtime, not chat_session.runtime_id — the latter is the daemon's resume pointer and is deliberately left stale after a switch, so resuming from it would put the picker on a runtime that executes nothing (MUL-5163). - PUT /api/agent-builder/sessions/{id}/draft stores the configuration, including the edits the user typed but never sent. Migration 251 adds agent_builder_draft (no FK per repo rule; pruned explicitly by DeleteChatSession, the runtime teardown and the workspace teardown, and registered in the workspace-deletion manifest). - The payload is opaque to the server: its shape is the studio's AgentDraft, validated client-side. Teaching Postgres and the handler about it would create a second definition to keep in sync for no gain. Client: - The session id lives in `?session=`, so a refresh, a back/forward and a reopened tab land back in the same conversation. - Leaving no longer deletes anything. The only destructive path is an explicit "discard", confirmed in a dialog, next to the create button. - Creating the agent archives the conversation instead of deleting it: it is the record of how that agent was designed, and an idle carrier costs nothing since usage is booked per task. - The configuration autosaves (debounced) and restores on arrival, with the applied-assistant-message marker stored alongside it so a restore cannot re-apply the last reply over edits made after it. - The 1.5s polling of messages and pending-task is gone. The global realtime sync already invalidates both per session id, exactly as it does for the main chat window, which has never polled. - The `<agent_draft>` block collapses to one "configuration updated" line. The regex now also swallows an unterminated block, which is what streaming produces — the raw payload used to scroll past on every turn. The 2185-line agent-creation-studio.tsx is split into three routes (`/agents/new`, `/agents/new/manual`, `/agents/new/ai`), its pure logic moves to packages/core/agents/ with its tests, and the unreachable template flow — `setMode("templates")` had no caller — is removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(agents): let the builder panes resize The conversation / configuration split was not draggable. Two structural reasons, both fixed by giving the group the same shape the chat page uses: - The panels reached the group through BuilderWorkspace's fragment, so they were not children the group could measure. - The group's children alternated between one panel (runtime setup) and two (conversation), under one persisted layout id. The group now lives inside BuilderWorkspace with its two panels as its only children, and the setup screen renders no group at all — it has nothing to split. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ui): give the resize handle a cursor on hover The separator had no cursor of its own, so the only signal that a split was draggable arrived after the drag started — the library writes a global `cursor: ... !important` while dragging, and nothing before it. Fixed on the shared handle rather than at one call site: every split surface (chat, inbox, issue detail, project detail, the agent builder) was missing the same affordance. The library's drag-time rule still outranks this one, so the cursor keeps narrowing to `e-resize` / `w-resize` once a panel hits its bound. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(agents): render the builder's draft block as an inspectable row Every builder reply ends in an <agent_draft> block that rewrites the form on the right. Flattening it to a line of prose said that something changed but not what, and the payload — the only record of what the builder actually claimed — was unreachable. A settled reply now carries a full-width row saying the configuration was updated, which opens the exact payload. A streaming one keeps a text line instead: the block is still being written, so there is nothing complete to open, and without the line the half-finished JSON scrolls past. ChatMessageList gains an optional `renderAssistantAddon`. It is opt-in per surface and undefined everywhere but this one, because no other chat speaks this protocol — the alternative was to keep pushing an embedded protocol through `transformContent`, which can only ever produce prose. `extractBuilderDraftBlock` returns an unparseable payload verbatim rather than withholding it: a malformed block is exactly when someone wants to read it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(agents): address review blockers on the resumable builder 1. Migration prefix collision. `251_agent_runtime_unbind` landed on main after this branch cut, so the backend's prefix-uniqueness guard failed. Renumbered to 252. 2. #6287 — the manual form still lost everything. The route split moved where you land, not what survives: the draft was `useState`, so a tab switch (the desktop shell mounts only the active tab) remounted it empty, and the beforeunload guard covered a hard reload and nothing else. It now persists through the repo's draft-store factory, scoped by what is being created — a blank agent and a copy of agent X are different work, and a copy of X is not a copy of Y — cleared once the agent is committed, and registered for logout / workspace-delete cleanup. 3. A saved draft with no messages was unreachable. The configuration form is editable from the moment a builder session exists and autosaves, so someone could open it, type a name and leave before the first turn; the list keyed "is this a draft" on messages alone, so that row existed and nothing could reach it. A session now qualifies on a message OR a stored draft, and sorts by whichever it has. 4. The debounce dropped the last edits. Its timer died with the component, so navigating away inside the 800ms window lost exactly the keystrokes the user had just made. The pending payload is now flushed on unmount. `useUnsavedDraftWarning` is gone with its last caller: both routes persist, so the browser prompt would have been warning about work that is already saved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ui): stop the resize cursor flipping mid-drag The library narrows its cursor the moment a panel hits a bound — col-resize while both directions are open, a one-way arrow once only one is. Truthful, but it reads as a glitch: the icon changes under your hand halfway through a drag you never stopped making. `disableCursor` turns that global rule off; the handle's own `cursor-col-resize` is now the only source. A drag captures the pointer and walks it across the panels, away from the 8px handle, so the group carries the same cursor for as long as a separator is active — otherwise it would fall back to a text caret the instant the pointer left the handle. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(agents): key manual drafts by owner, drop the draft-block rendering Two changes. One slot destroyed the other flow's work. The manual draft was stored under a single key: opening a blank form, or a copy of a different agent, refused to adopt the stored draft and then immediately wrote its own empty form over it — so a half-finished copy of agent A died the moment the user opened anything else, before typing a character. Drafts are now keyed by what is being created, the same shape the chat composer uses for its per-session drafts, and a slot is dropped when its content is gone rather than parked blank (which also stops the map growing a dead key per agent ever opened for duplication). Committing an agent clears that flow's slot only. The `<agent_draft>` block goes back to being hidden outright. Labelling it and opening its payload dressed up machinery as content: the block drives the configuration form, and the form is where its effect is already visible. `renderAssistantAddon` goes with it — ChatMessageList is back to what it was, since no surface needs the slot. The two-pattern strip stays: an unterminated block is what streaming produces, and without matching it the raw JSON scrolled past the reader on every turn. Also removed six barrel exports nothing imported through, and unexported five types only their own file used. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(agents): keep a manual draft whose only edit is a picker The "is this worth storing" predicate listed six fields by name, and the draft serializes eleven. A form whose only change was the model, the thinking level, the service tier, the access scope or a team grant read as untouched, so the next save deleted its slot — picking a model before typing a name and switching tabs lost the model. Enumerating was the mistake, not the specific omissions: the predicate stops covering every field added after it is written, and the failure is invisible because each field saves correctly as long as some *other* field is also set. It now compares the whole draft against a fresh one. The runtime stays outside that comparison, on the entry rather than in the draft, because the form seeds it on every visit and counting it would store a draft for a form nobody touched. Covered field by field, one edit at a time, so a future field cannot quietly fall out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Multica Eve
|
4fe94a6d40 |
revert: "MUL-5637 fix(mcp): agent mcp_config is an authoritative allowlist (#6292)" (#6314)
This reverts commit
|
||
|
Multica Eve
|
a54662f30e |
fix(daemon): provision config-referenced Codex instruction files into task home (#6291)
* fix(daemon): provision config-referenced Codex instruction files into task home A per-task CODEX_HOME copies the user's config.toml verbatim, so a model_instructions_file reference survived the move while the file it names did not. Codex resolves the relative value against CODEX_HOME — now the task home — and failed loading its configuration before the task prompt was ever delivered (#6271). Generalize the existing model_catalog_json provisioning into a keyed table of path-valued config keys and add model_instructions_file plus its deprecated experimental_instructions_file alias. Semantics are unchanged per key: absolute/~ values are left for Codex to read directly, relative values must stay inside the task home, the copy refreshes on reuse, and a missing source fails during environment preparation with a diagnostic naming the key instead of an opaque os error 2 at thread/start. Co-authored-by: multica-agent <github@multica.ai> * fix(daemon): root-scope task-home writes for config-referenced Codex files Review must-fix: filepath.IsLocal only proves the config string has no lexical '..'. A task home is reused, and the task that ran in it can replace an intermediate directory of the copy destination with a symlink, which made the daemon's next MkdirAll/Remove/copy follow that link and delete or overwrite a file outside the task home. Do the mkdir, stale-copy removal, and write through os.OpenRoot(codexHome) so links leaving the task home are rejected (links staying inside it are harmless), and refuse outright when codexHome itself is a symlink, since OpenRoot would resolve it before confining anything below. The pre-existing model_catalog_json path is covered by the same helper. Also stop reporting every source stat failure as a missing file — a permission or IO error now says so — and document the source-side symlink policy plus the stale-copy contract. Co-authored-by: multica-agent <github@multica.ai> * test(daemon): pin stale-copy contract when a referenced Codex file is repointed Co-authored-by: multica-agent <github@multica.ai> * fix(daemon): bind codex-home root check to the opened handle Review must-fix: checking the path with os.Lstat and then opening it with os.OpenRoot leaves a window where the directory is swapped for a link to somewhere else. OpenRoot resolves the path it is given, so the resulting root is confined to the wrong tree and every root-scoped write lands outside the task home without ever escaping its root. Task homes are reused and Windows cannot confirm descendant cleanup, so a leftover process that knows its old CODEX_HOME can create that window. Open first, then prove identity against the handle: compare root.Stat(".") with a no-follow os.Lstat of codexHome via os.SameFile, and reject a symlink outright. A swap before the open now fails the check; a swap after it cannot matter because all writes go through the verified handle. verifyCodexHomeRoot is split out so the swap is tested deterministically instead of raced, plus an end-to-end test for a task home that is already a link to an outside directory. Co-authored-by: multica-agent <github@multica.ai> * docs(daemon): scope the codex-home symlink test name and root-handle claim Review nit: the end-to-end test asserted only that the referenced-file copy refuses a symlinked task home, but its name read as though the whole prepare were safe. Rename it accordingly and state in both the test and openVerifiedCodexHomeRoot that the earlier path-addressed steps of prepareCodexHomeWithOpts are out of scope, tracked in MUL-5647. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Multica Eve
|
aa349fed02 |
MUL-5637 fix(mcp): agent mcp_config is an authoritative allowlist (#6292)
* fix(mcp): treat agent mcp_config as an authoritative allowlist
An agent's saved mcp_config was silently widened with the runtime host's
own user-level MCP servers, so an explicitly empty `{"mcpServers":{}}`
resolved to the COMPLETE host set instead of no servers at all — the
opposite of what the operator configured (GitHub #6283).
`--strict-mcp-config` was being passed correctly; the merge happened
before it, in the daemon, so strict mode constrained an already-widened
set. Introduced by #5277 and present in v0.4.16 through main.
Restore the three-state contract in resolveEffectiveMcpConfig:
null / unset -> inherit the provider's native MCP configuration
{"mcpServers":{}} -> strict empty, no host servers
non-empty object -> strict allowlist, exactly those servers
Two explicit inherit paths keep the additive behaviour reachable without
weakening the default:
- runtime_config.mcp.inherit_runtime = true opts an agent back in.
- The claim response now carries mcp_config_overlay_only so the daemon
can tell an agent-authored config from a per-task Composio overlay.
Without it, enabling an integration on an agent that never configured
MCP would have stripped the host servers it was already inheriting.
Both decode paths fail closed: malformed runtime_config never enables
inheritance, and a failed runtime merge falls back to the agent's own
config.
The web MCP tab and the `agent create/update --mcp-config` help text
described the old additive behaviour, which is how a tightened config
could look correct while exposing every host server; both now state
which mode is in effect.
Note for rollout: the fix lives in the daemon, so self-hosted users must
upgrade the daemon — a server/UI upgrade alone does not apply it.
Co-authored-by: multica-agent <github@multica.ai>
* fix(mcp): close review gaps in the authoritative mcp_config change
Addresses the four must-fix findings from review of #6292.
1. Deleting the last managed server no longer widens access.
removeManagedMcpServer cleared the config to null, which now means
"inherit the host's MCP servers" — so a delete took the agent from one
allowed server to every server on the host. It now leaves an explicit
`{"mcpServers":{}}`. Restoring inheritance moved to a separate
clearManagedMcpConfig action behind its own confirmation that states the
widening. The delete dialog no longer claims "Runtime servers are not
affected", which was the opposite of the truth.
2. The UI no longer promises a boundary an old daemon does not enforce.
The strict semantics live in the daemon, so a config saved against an
older daemon is not yet in effect. Adds the authoritative-mcp-v1 daemon
capability:
- The daemon advertises it and reports authoritative_mcp on the
runtime-capabilities response.
- The claim path fails closed: a managed, non-inheriting mcp_config
claimed by a daemon without the capability cancels the task and
returns 412 with an actionable message, instead of letting that daemon
merge the host's servers in. runtime_config.mcp.inherit_runtime is the
documented escape hatch, and it is honest — it declares that the
operator accepts the host's servers.
- The MCP tab shows "needs upgrade" rather than "Not exposed" while the
bound runtime lacks the capability.
3. Saving OpenClaw settings no longer drops the inherit opt-in.
parseOpenclawRuntimeConfig discarded unknown keys and the tab persisted
the result as the whole runtime_config, so one unrelated routing save
silently deleted mcp.inherit_runtime. Unknown keys now round-trip
through OpenclawRuntimeConfig.passthrough, excluded from the dirty check
so they cannot make the form look edited.
4. Documents the new semantics in the built-in creating-agents skill and
its source map: the three states, the persisted
runtime_config.mcp.inherit_runtime field, and the claim-time capability
gate.
Also corrects the PR's rollout claim: there is no database migration, but
this does add a persisted JSON field and change the meaning of an existing
one.
Co-authored-by: multica-agent <github@multica.ai>
* fix(mcp): stop the authoritative-daemon gate from blocking valid claims
CI's backend job failed three handler claim tests with the new 412. Two
distinct problems, both real:
1. The gate fired for a non-object mcp_config. 66 handler fixtures seed
`[]`, which is not a valid MCP config and cannot carry `mcpServers`, so
it expresses no boundary to protect. An old daemon does not widen it
either: mergeRuntimeAndAgentMcpConfig fails to unmarshal a non-object
and falls back to the agent config alone (verified directly). Gating
these blocked tasks with no security benefit, so the gate now requires a
JSON object.
2. The shared daemon test-request helper advertised no capabilities, so
every claim test was accidentally simulating a pre-#6283 daemon. It now
defaults authoritative-mcp-v1 on, matching what every current daemon
sends. Only that capability — skill-bundles / coalesced-comments / rpc
are feature negotiations whose absence tests real legacy behaviour, so
they stay opt-in per test.
Adds claim-level coverage for the gate itself, which is what the unit tests
alone could not catch: an outdated daemon gets 412 with an actionable
message and the task is cancelled; a capability-advertising daemon gets
200; the inherit_runtime opt-in lets an outdated daemon through; and an
unmanaged or non-object config is never gated.
Verified against a real migrated schema this time (throwaway Postgres),
which is how the three failures were reproduced locally and confirmed
fixed: `go test ./internal/handler ./internal/daemon` both ok.
Co-authored-by: multica-agent <github@multica.ai>
* fix(mcp): surface the daemon-upgrade refusal and stop gating safe providers
Addresses the second review round on #6292.
1. The refusal is now visible wherever the operator looks. The default
claim path is the machine-level BATCH endpoint, which skips build
failures and still answers 200 {"tasks":[]}, so the previous bare
CancelTask showed a task that vanished with no stated reason — turning an
explicit upgrade requirement into an unexplained failure. The claim path
now fails the task with a new classified reason,
mcp_config_daemon_outdated, plus the actionable message. That reaches the
user on all three claim paths and on any daemon version, which a new
response field could not: the audience is by definition a daemon too old
to read one. The per-runtime path keeps its 412.
The reason is deliberately not auto-retryable — the same outdated daemon
would claim the retry and fail it again.
2. The gate no longer cancels safe tasks. It applied to every provider, but
only claude / codebuddy / codex / cursor / opencode / openclaw were ever
merged with host MCP by an old daemon (loadRuntimeMcpServerConfigs).
Qwen was never merged and already had strict semantics, so its tasks were
being failed for a risk that does not exist. Scoped via
providersOldDaemonsMergedRuntimeMcp; an unknown provider does not gate,
because the gate should only fire where the old behaviour is concrete.
3. The new authoritative_mcp flag now goes through the API schema layer.
Both local-skills responses were returning raw network JSON, so the flag
that decides whether the UI may assert an MCP boundary rested on an
unchecked type assertion. Adds RuntimeLocalSkillListRequestSchema with
authoritative_mcp and mcp_supported defaulting to FALSE — the fail-closed
direction — and a MALFORMED_ fallback that cannot express a guarantee.
Claim-level tests now cover all three paths, which is what the previous
helper-only tests missed: per-runtime 412, batch recording the refusal on
the task while still delivering the healthy tasks in the same batch, WS RPC
refusing and accepting, the qwen negative case, the inherit_runtime escape
hatch, and unmanaged / non-object configs.
Verified against a real migrated schema (throwaway Postgres):
go test ./internal/handler ./internal/daemon ./pkg/agent ./pkg/taskfailure
./internal/service all ok.
Co-authored-by: multica-agent <github@multica.ai>
* fix(mcp): register the new failure reason and wire its copy into the UI
Addresses the third review round on #6292.
1. mcp_config_daemon_outdated was declared but never registered in
taskfailure.allReasons, so metrics.NormalizeFailureReason missed the
known-value map and fell through to free-text Classify() — relabelling a
platform-side refusal as `agent_error.unknown` (verified directly) and
leaving the Prometheus series un-pre-warmed. Registered it, canonical
count 22 → 23 (platform 8 → 9), with the wire value and IsAgentError split
pinned. New test pins the WHOLE canonical set through
NormalizeFailureReason so forgetting the next reason fails a test instead
of quietly mislabelling a metric; NormalizeFailureReason had no coverage
at all before.
2. The upgrade copy was dead. The locale strings landed last round but
neither consumer mapped the reason: chatFailureCopy fell back to generic
failure text with the actionable detail buried in the collapsed raw
error, and task-failure.ts rendered the bare wire value
`mcp_config_daemon_outdated` in the agent activity list and issue
execution log. Both are mapped now, with regression tests, plus the
runtime class pinned in failure-class.test.ts. This directly contradicted
the claim in the claim-path comment that every path reaches the user, so
that is now actually true.
3. providersOldDaemonsMergedRuntimeMcp is documented as what it is: a FROZEN
record of what pre-capability daemons merged, not a mirror of the daemon's
current provider switch. The old "keep the two lists in lockstep" note was
actively harmful advice — runtime MCP discovery for a new provider can only
ship in a daemon that already advertises the capability (never gated), so
adding it here would fail tasks on old daemons that never merged for it,
re-creating the qwen false-positive. Pinned with a test.
Also corrects a stale count in task-failure.ts (7 → 9 platform reasons).
Verified against a real migrated schema (throwaway Postgres): full backend
suite green apart from the pre-existing environmental cmd/multica guard; all
9 TestMcpGate_* integration tests pass.
Co-authored-by: multica-agent <github@multica.ai>
* docs(taskfailure): correct taxonomy counts and finish the reason registration
Non-blocking nits from the fourth review round on #6292.
- Taxonomy counts now say 23 reasons / 9 platform-side. Registering
mcp_config_daemon_outdated last round updated the assertions but not the
prose. Swept the whole repo rather than only the flagged lines, which
turned up four more that were already stale at 21 and drifted further:
handler/dashboard.go, daemon/poisoned.go, core/types/agent.ts, and the
db/queries/task_usage.sql comment sqlc copies into the generated file.
The generated file's comment was updated by hand to match its source.
Running `sqlc generate` churned 58 lines across 47 unrelated files — the
local sqlc version differs from the one that produced the checked-in
output — so that churn was reverted and only the one intended line kept.
- failure_test.go's `required` list now includes
ReasonMcpConfigDaemonOutdated. Length and label assertions already covered
the reason, but the list is documented as the complete canonical set, so
the omission contradicted its own comment.
- Restored the line break in chat-message-list.test.tsx that a previous edit
of mine collapsed.
Comment, test-fixture and formatting only; no behaviour change.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
|
||
|
Naiyuan Qing
|
76fbd48849 |
fix(skills): measure draft dirtiness against a seeded baseline (MUL-5645) (#6294)
* fix(skills): measure draft dirtiness against a seeded baseline (MUL-5645) The detail page inferred "the user edited something" from "the draft differs from the latest server skill". That difference has two independent causes — the user typed, or the server moved — and the page could not tell them apart, so it read both as a local edit. Two user-visible failures came out of it: - A description ending in whitespace (what `description: |` frontmatter yields, so every imported skill) compared unequal to itself, because the dirty check trimmed the draft side and not the server side. The page opened permanently dirty and Discard reseeded the same value, so the save bar could not be dismissed at all — only Save cleared it, by rewriting the stored description. - Any remote update to an open skill was taken for a local edit, so the page raised the conflict banner and refused to reseed. The editor stayed frozen on pre-update text with no way to see what had changed, and saving from there pushed the stale draft back over the newer version. Record the seeded snapshot in `baselineRef` and compare against that instead. With a baseline the two causes separate: `draft !== baseline` is a local edit, and a new `updated_at` with no local edits is just a remote update, which now reseeds silently. The conflict banner is left for the case it was written for — a remote update landing on real unsaved work. `toDraft` also trims name and description at the single seam where server data becomes a draft, matching what Save persists, so later comparisons are plain equality rather than a trim both sides have to remember. Content and file bodies are not normalized: whitespace in a SKILL.md body is content. File sets are compared through a path-sorted signature, since GET sorts files by path while PUT echoes request order and that difference is not a content change. Four of the five regression tests fail against the previous implementation; the fifth covers the true-conflict path, which was already correct and must stay that way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai> * fix(skills): trim frontmatter name and description at the parse seam (MUL-5645) Both fields are single-line labels everywhere they are consumed, but YAML clip chomping gives `description: |` and `description: >` a trailing newline, so imports stored a description that differed from its own trimmed form. The detail page no longer depends on this — it normalizes when it seeds a draft — but leaving it means every new import keeps writing the padded value, and any future consumer that compares a stored description to a trimmed one inherits the same trap. Trimming here covers all four import paths (GitHub, skills.sh, archive, runtime-local) in one place rather than asking each to remember. Callers that need the raw SKILL.md still have it: `content` is stored untouched. Defensive only. Reverting this commit alone does not reintroduce the bug. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai> * fix(skills): release the conflict when the user reverts their own edits (MUL-5645) Review catch on the previous commit. Once a conflict was raised, the seed effect only reconsidered when a new server version arrived, so a user who resolved the conflict by hand — retyping the field back to what it was — got stuck: the draft was clean again, and because the save bar renders only while dirty, its Discard button unmounted. That left the banner sitting above stale text with no control left to dismiss it, and no way forward short of a reload. This state is a regression from measuring dirtiness locally. Previously the draft was compared against the moved server value, so reverting still counted as dirty and Discard stayed on screen. Re-run the decision on draft changes too. When the local edits go away there is nothing left to protect, so the page adopts the server version and clears the banner — the same outcome as the never-edited case, reached a moment later. The true-conflict path is unchanged, and its regression test still passes, which is what keeps this from over-correcting into "always release". Reseeding also grew a third caller, so the four pieces that have to move together — draft, baseline, seeded key, conflict flag — are now assigned in exactly one place, `adoptServerVersion`, used by first load, silent refresh, Save and Discard alike. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Wangjue Yao
|
69d3208586 |
fix(channels): handle /new in shared router (#6251)
* fix(channels): handle /new in shared router * test(channels): cover /new provider reset end to end * fix(channels): preserve command source across rewrites --------- Co-authored-by: 无瑕 <yaowangjue.ywj@antgroup.com> |
||
|
Bohan Jiang
|
20d2dd8cef |
MUL-5620: fix(daemon): close the repo-eviction liveness window and stop GC walking internal caches (#6299)
* fix(daemon): close the repo-eviction liveness window and stop GC walking internal caches Two review follow-ups from #6297. 1. The live-repo set was snapshotted once at the start of the .repos walk, then consulted per repo after running git and filesystem work on each one in turn. A workspace re-attaching a repo inside that window could see the cache evicted anyway. Replace the snapshot with a per-path query and ask twice: once as a cheap early-out so an attached repo never pays for the git work, and again immediately before RemoveAll. Re-reading in-memory state costs one mutex and no network, and shrinks the window from the whole walk to a few adjacent statements. This narrows the race rather than eliminating it — attachment updates workspaceState without taking the repo lock, so a sufficiently unlucky interleaving is still possible. It stays benign: a freshly attached repo has no last-used stamp and takes the backfill-and-skip path, and a wrong eviction costs one re-clone via ensureRepoReady. 2. runGC skipped only .repos, so it walked .skill-cache as if it were a workspace. Its "v1" directory then looked like a task dir with no .gc_meta.json and the orphan path deleted the entire bundle cache once its mtime went GCOrphanTTL without a new bundle — a few hundred KB reclaimed in exchange for a full re-download. Skip every dot-directory, matching what ScanDiskUsage already does; workspace directories are always UUIDs, so a dot-prefixed entry is one of our own caches. The skill cache has no lifecycle of its own, but it is measured in hundreds of KB, so leaving it unmanaged is clearly better than deleting it wholesale on an unrelated TTL. Giving it a real lifecycle is separate work if it ever grows. Steve's third observation — repoCacheSize measuring any second-level directory while eviction only handles isBareRepo ones — is left as is on purpose: measuring more widely than we delete is the right asymmetry for a visibility feature, so a corrupted leftover stays visible instead of silently uncounted. Co-authored-by: multica-agent <github@multica.ai> * fix(daemon): measure repo size before the final liveness check, not after Review follow-up: the second liveness check was not actually adjacent to the delete. dirSize walks every file in the bare repo and ran between the check and RemoveAll, so on a multi-GiB cache — the size that motivated this work in the first place — a workspace re-attaching during that walk would still lose its cache. Measure first, then check, then delete. That leaves only the window between two adjacent statements, which is what the comment claimed all along, and it keeps the slow filesystem walk outside the section the check is meant to protect. No behaviour change beyond the ordering: bytes_reclaimed still reports the size measured just before deletion, which TestEvictRepoCache_RemovesIdleDetachedRepo already asserts is non-zero. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Bohan Jiang
|
a735e4808d |
feat(daemon): account for and evict the bare repo cache (#6297)
The .repos bare-clone cache was excluded from disk-usage and never reclaimed, so it grew monotonically and was invisible while doing it. On the machine in #6265 it was 3.78 GB — 29% of the workspaces root, and exactly the difference between what disk-usage reported (9.1 GiB) and what the user's file manager showed (12.88 GiB). Accounting: .repos is now measured and reported on its own line rather than skipped. It stays out of the task totals on purpose — every task in a workspace checks out from this one shared cache, so folding it into per-task numbers would attribute it to directories that do not contain it. Other daemon-internal dot-directories (.skill-cache) are no longer counted as workspaces, which is what produced bogus rows like '.skillca'. Eviction: a bare repo is removed only when all four hold — GCRepoTTL > 0, no watched workspace still claims it, no worktrees remain, and no task has created a worktree from it within the TTL (default 30d). Two decisions worth calling out: - The workspace check is a RETAIN predicate, not a delete predicate. Sync re-clones every listed repo that is missing whenever a workspace registers, which happens on every daemon start, so evicting a repo the workspace still claims just buys a full re-clone on the next restart — that moves disk cost, it does not reclaim it. Because the set only prevents deletion, a stale or empty one cannot widen what we delete. - Idleness is an explicit stamp written by CreateWorktree, not directory mtime. Restarts re-fetch every cached repo, refreshing the mtime of repos no task has checked out in months; atime is unavailable in practice (noatime on Linux, off by default on Windows). A cache with no stamp reports unknown and gets its clock started, never treated as ancient — otherwise the first cycle after an upgrade would wipe every cache on the machine. Evicting wrongly costs a re-clone, not a failure: the next task that needs the repo takes the cache-miss path in ensureRepoReady. Verified on a live runtime: both table views now show the .repos line (242.8 MiB across 3 repos) and no longer list .skill-cache as a workspace. Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
it235
|
eb14e73a20 |
MUL-5617: fix(storage): disable request checksum in buffered S3 Upload (#6261)
* fix(storage): disable request checksum in buffered S3 Upload
* test(storage): make the checksum-trailer regression test actually fail without the fix
The test built its client with s3.New(s3.Options{...}), which leaves
RequestChecksumCalculation unset. An unset value emits no checksum at all,
so the assertions passed with or without the fix in Upload(). Production
builds the client via config.LoadDefaultConfig, which resolves the field to
WhenSupported.
Pin that production default in the test client and serve over TLS, since the
SDK only switches to a trailing checksum when the request is HTTPS. Removing
the option from Upload() now fails buffered_upload on the exact
STREAMING-UNSIGNED-PAYLOAD-TRAILER value Aliyun OSS rejects, while
streaming_upload still passes.
Also gofmt s3.go: the added option callback used spaces instead of tabs.
Co-authored-by: multica-agent <github@multica.ai>
* fix(storage): scope the checksum workaround to non-AWS endpoints
Downgrading RequestChecksumCalculation to WhenRequired is what makes Aliyun
OSS and Tencent COS accept the buffered upload, but it drops the client-side
checksum entirely rather than moving it out of the trailer: on TLS the request
goes out as UNSIGNED-PAYLOAD with no x-amz-checksum-* at all. Applying that to
every backend would change requests that work today, and AWS buckets with a
default Object Lock retention require a checksum to be present.
Gate the option on the endpoint instead. Real AWS S3 — no AWS_ENDPOINT_URL, or
an explicit amazonaws.com host — keeps whatever the SDK resolved, including any
operator-set AWS_REQUEST_CHECKSUM_CALCULATION. Only S3-compatible endpoints get
WhenRequired.
Adds TestS3StorageAWSUploadKeepsChecksumTrailer so the AWS path is pinned from
the other side: making the workaround unconditional now fails that test, while
removing it entirely still fails buffered_upload.
Also drops the incorrect claim that the seekable body stays covered by a real
SigV4 payload hash.
Co-authored-by: multica-agent <github@multica.ai>
* fix(storage): match the AWS endpoint on the host label boundary
usesAWSEndpoint substring-matched "amazonaws.com" anywhere in the configured
endpoint, which got the decision wrong in both directions: an uppercase
"https://S3.US-EAST-1.AMAZONAWS.COM" was treated as third-party and lost its
checksum, while "https://notamazonaws.com", "https://s3.amazonaws.com.evil.net"
and any URL merely carrying the string in its path or query were treated as AWS
and never got the fix they needed.
Parse the endpoint and compare the hostname on the label boundary instead,
covering the China partition. Scheme-less values are retried as https, since
url.Parse otherwise reads the whole value as a path.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: renjianjun <renjianjun@angelalign.com>
Co-authored-by: Bohan-J <bohan.optimism@gmail.com>
Co-authored-by: multica-agent <github@multica.ai>
|
||
|
Xichang(Seacen) Zhao
|
cb04226fd4 |
MUL-5632: fix(engine): broadcast the full issue payload for chat-created issues (#6278)
* fix(engine): broadcast the full issue payload for chat-created issues
An issue opened with /issue from a chat channel broadcast
{"issue_id": <uuid>} — the minimal fallback IssueService.Create emits
whenever a caller leaves IssueCreateOpts.BroadcastPayload nil, which the
engine did. Every issue:created consumer reads payload["issue"], so the
missing key meant extractIssueFields (cmd/server/subscriber_listeners.go)
failed its map assertion and returned false before it ever reached the
id / creator_id check. The listener returned early and no auto-subscribe
rule ran, so the person who typed /issue was never subscribed to the
issue they had just filed and got no notifications for it. Both channels
that register with the engine — Feishu/Lark and Slack — were affected.
The HTTP handler already supplies a BroadcastPayload and autopilot
publishes its own event through issueToMap; only the engine path was
short. Export issueToMap as IssueToMap and have the engine use it, so a
single builder is the one source of truth for that shape instead of
three descriptions drifting apart. The workspace issue prefix comes from
the GetWorkspace call the /issue path already made for the chat reply's
identifier, hoisted so it is read once and used for both.
No regression — the payload only gains keys, and consumers read named
fields. The activity and notification listeners type-assert
payload["issue"] to handler.IssueResponse and still skip a map, exactly
as they already do for autopilot-created issues. The subscriber listener
accepts either shape. The workspace WS fanout marshals the payload
as-is, so open clients now render a chat-created issue live instead of
waiting for a refetch.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(service): complete the issue:created payload contract
IssueToMap omitted project_id, stage, metadata and properties, all of
which handler.IssueResponse — the other rendering of the same event —
always emits. Clients type both as a complete Issue and insert the
object straight into the list cache without runtime validation, so an
issue created by autopilot, quick-create or the chat /issue command
appeared in open clients missing its project and custom properties
until the next refetch. Broadcasting the full issue from the chat path
would otherwise have spread that existing defect to a third entry
point.
Fill in the missing keys and pin the contract with a test that fails if
the two renderings ever drift apart again, so adding a field to
IssueResponse without adding it here is caught in CI rather than in the
UI. metadata and properties render as {} when unset, matching the
"always present" part of the contract.
Also route both identifier renderings through service.IssueIdentifier,
so a workspace lookup that degrades the prefix cannot show "#42" in the
chat reply while the realtime list shows "-42".
Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
* docs(service): name the actual IssueToMap call sites
The doc comment and the shape test listed quick-create among the
non-HTTP publishers of the issue payload. It is not one: the three call
sites are autopilot and the channel engine on issue:created, and the
background stuck-issue status reset on issue:updated.
Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
|
||
|
Bohan Jiang
|
c037c134ad |
MUL-5620: fix(daemon): make disk-usage report what is actually on disk (#6270)
* fix(daemon): make disk-usage report what is actually on disk Two defects made `multica daemon disk-usage` misreport, both of which matter when a user is trying to work out why their workspaces root is large (#6265). 1. The STATUS column was dead code. TaskDiskUsage.ParentStatus was declared and rendered but never assigned anywhere, so the column always printed "-" and parent_status was always "" in JSON. The GC is unaffected — it resolves status independently via gcDecisionIssueResult — but the one column that answers "which of these issues are still open?" was blank. ScanDiskUsage stays network-free; ResolveParentStatuses is an opt-in second pass wired into the CLI, backed by the same batch gc-check endpoint the GC loop uses, so the column reports exactly the status the GC would act on. The daemon already authenticates with the profile's CLI token, so this needs no new API surface. Best-effort: offline or logged out, the column stays blank and the command still works. Only issue-kind dirs are resolved — they dominate, and they are the only kind with a batch endpoint. Chat / autopilot-run / quick-create keep an empty status rather than costing one request each. 2. taskSize skipped .git entirely, so size_bytes under-reported every task dir holding a real git checkout. That disagreed with both the user's file manager and the GC itself: a full gcActionClean removes .git with the rest of the dir, and dirSize (which reports bytes_reclaimed there) counts it. Now counted wholesale into totalBytes, still never descended into, so artifact accounting stays aligned with cleanTaskArtifacts. Verified against a live runtime: STATUS resolves (in_review on the largest dirs, i.e. not done — which is exactly the question that prompted this), and top dirs went from 104.9 MiB to 132.6 MiB reported, closing most of the gap against du. Co-authored-by: multica-agent <github@multica.ai> * fix(daemon): skip disk-usage status resolution when nothing renders it Review follow-up on #6270. The --by-workspace table has no STATUS column, but the CLI resolved statuses unconditionally. With credentials on disk and an unreachable server that cost a full timeout before printing a purely local report — and --all-profiles paid it once per root, serially — then warned about a column the output does not contain. Gate resolution on whether task rows are actually rendered; JSON keeps resolving because it always carries the task array. Also correct two contract statements that the .git change had made false: the command's Long help still claimed 'The walk skips .git', and ScanDiskUsage's doc comment still claimed it never enters .git. Both now state the real rule (counted toward the total, never toward the artifact subset, symlinks still never followed), and the help now says STATUS needs the network and is left blank when it cannot be reached. Adds the CLI-level regression tests the wiring was missing: the by-workspace table makes no request, the per-task table resolves and renders the status, a failing server still exits 0 with valid JSON on stdout, and --all-profiles resolves each root with its own profile's token. Verified the no-request test fails when the gate is removed. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Bohan Jiang
|
d38da27ed4 |
MUL-5619: fix(cli): surface the server's 409 message instead of the generic conflict template (#6267)
* fix(cli): show the server's conflict message instead of the generic 409 template
Every 409 this API returns is a deterministic refusal that names its own fix
("a skill with this name already exists", "set parent_id (--parent) to <id>").
The CLI replaced all of them with a template that says the opposite — that the
state changed underneath you and you should re-fetch and retry. Agents took the
retry hint literally: GH #6264 reports 15+ identical retries over 10 minutes
followed by hours spent chasing an optimistic-concurrency theory that never
existed, and GH #5948 is a second user misdiagnosing the same way. MUL-4417 had
already written the useful message server-side; it just never reached anyone.
Route 409 through the same server-message extraction 400/422 already uses, so
roughly forty hand-written conflict messages across skills, agents, runtimes,
labels, projects and comments become visible by default. A body we cannot
recognize still falls back to the template, so this never dumps a raw response.
extractServerMessage now prefers prose over a bare identifier, because a few
endpoints put a stable code in "error" and the sentence in "message".
MUL-5619
Co-authored-by: multica-agent <github@multica.ai>
* fix(comments): stop telling a wrong --parent that it posted a top-level comment
The reply guard returns one message for two different mistakes. A resumed
session that carries a previous turn's --parent forward (GH #6264) did not ask
for a top-level comment, but is told it did — which sends it looking for a
new-thread opt-in (GH #5383) instead of correcting the parent it already passed.
Split the copy: name the rejected parent when one was supplied, and keep the
existing top-level wording for the parentless case. Both still point at the
trigger comment to use.
MUL-5619
Co-authored-by: multica-agent <github@multica.ai>
* fix(runtime): return 500, not a 409 echo, when the update store fails
InitiateUpdate answered every UpdateStore.Create failure with a 409 carrying
err.Error(). The in-memory store only ever returns errUpdateInProgress, so this
looked safe — but the Redis store also wraps infrastructure failures as
"reserve active update: <dial error>" and "persist update request: <error>".
Surfacing 409 bodies in the CLI turns that into a user-visible leak of internal
addresses, and labels an outage as a conflict the caller could fix by retrying.
Classify instead: errUpdateInProgress keeps its 409 and its actionable message,
everything else is logged and answered with a 500 and fixed copy.
Also pins the prose-over-machine-code preference for validation bodies, which
the shared extractor applies to 400/422 as well as 409. Only the issue-table
endpoints are shaped that way and none is reachable from the CLI today, but the
change is intentional and should fail loudly if reverted.
MUL-5619
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
|
||
|
Multica Eve
|
b06af2ae17 |
feat(runtime): unbind agents on runtime delete instead of destroying them (#6220)
* feat(runtime): unbind agents on runtime delete instead of destroying them Deleting a runtime archived its agents and then hard-deleted the rows, so the agents and every conversation with them disappeared — while the confirmation dialog said "archive", which a user reasonably reads as recoverable. Retiring a laptop is an ordinary action; losing the agents configured on it is not an ordinary consequence. An agent is now a persistent business object and a runtime is replaceable execution capacity: deleting a runtime unbinds its agents. `runtime_id IS NULL` means unbound — orthogonal to archived — and the agent keeps its instructions, skills, chats, labels, channel installations, autopilots and task history. service.AgentReadiness already refused an agent with no runtime, so the scheduling safety gate needed no change. Two columns become nullable, not one. Without `agent_task_queue.runtime_id`, deleting the runtime still cascades the task history away (and task_message / task_usage / task_token with it), so the agents would survive with no record of anything they did — the same class of loss. A NOT VALID CHECK keeps NULL confined to history: an active task must always have a runtime, so claim / dispatch / delivery-CAS paths can never observe one without. It is written against completed_at rather than a status list so a future non-terminal status fails closed instead of slipping through. Two prerequisites this depends on: - 'deferred' (migration 128) was missing from CancelAgentTasksByRuntimeOrAgent. It went unnoticed because the delete used to cascade those rows away; with the new CHECK it would abort the delete and make the runtime undeletable. - The channel-installation / label / chat-pin / invocation-target / draft-restore cleanups were scoped to "archived agents on this runtime". Archived user agents now survive, so that scope is narrowed to kind='system' — otherwise the fix would produce a subtler loss: agent alive, configuration wiped. Also removes the squad guard that refused (409) when an active squad's leader was an archived agent on the runtime, plus the archived-squad delete that existed only to get past squad.leader_id's RESTRICT FK. The leader is no longer deleted, so nothing needs to be given up to retire a machine. Autopilots are no longer paused either: their assignee survives, and a rebind restores them without the owner having to remember to re-enable. Reason codes: an unbound agent reports agent_runtime_required, not runtime_offline. The copy for runtime_offline tells users to reconnect a machine; an unbound agent has no machine to reconnect, and the fix is to bind a runtime. Chat's bare 409 string gains the same code so the composer can offer that action. API: agents gain runtime_bound. runtime_id stays a string (empty when unbound) so installed clients keep parsing and no gated two-release rollout is needed. The confirmed-delete endpoint is /unbind-agents-and-delete; /archive-agents-and-delete still routes to it, and the compared expected_active_agent_ids set is unchanged — widening it would 409 every older client forever. Co-authored-by: multica-agent <github@multica.ai> * fix: make runtime unbinding recoverable Co-authored-by: multica-agent <github@multica.ai> * fix: address runtime unbind review nits Co-authored-by: multica-agent <github@multica.ai> * fix: resolve runtime unbind review blockers Co-authored-by: multica-agent <github@multica.ai> * fix(migrations): renumber runtime unbind after main merge Co-authored-by: multica-agent <github@multica.ai> * test(daemon): avoid late-request lease flake Co-authored-by: multica-agent <github@multica.ai> * test(autopilots): bind validation fixture runtime Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Xichang(Seacen) Zhao
|
829a4e5af9 |
fix(daemon): honour HTTP(S)_PROXY when dialing the wakeup WebSocket (#6279)
runTaskWakeupConnection built its dialer by hand:
dialer := websocket.Dialer{HandshakeTimeout: 10 * time.Second}
A zero-value Proxy field means "dial direct and ignore the
environment". websocket.DefaultDialer sets Proxy to
http.ProxyFromEnvironment; a dialer built this way gets nothing, and
gorilla skips the CONNECT wrapper entirely.
This is the daemon's control connection to the Multica server, so in
the SaaS deployment it dials out to the public internet. On a machine
whose only egress is a corporate proxy the handshake can never succeed.
Nothing points at the cause either: the loop logs "task wakeup
websocket unavailable; polling fallback remains active" at debug level
and never mentions a proxy. The daemon then runs permanently degraded —
task pickup waits for the HTTP poll (PollInterval, 30s by default)
instead of a server push, heartbeats stay on HTTP, and the WS-first
batch claim path (MUL-4257) falls back to HTTP on every task.
Set Proxy: http.ProxyFromEnvironment. gorilla rewrites wss:// to
https:// on the parsed URL before it calls Proxy, so HTTPS_PROXY — and
NO_PROXY — apply to this dial the same way they apply to every other
HTTPS client in the process. The lark connector fixed the same defect
the same way in #4165; this was the last bare dialer left in non-test
code.
No regression where no proxy is configured: ProxyFromEnvironment
returns a nil URL, gorilla leaves netDial untouched, and the dial is
byte-for-byte the direct dial it was before. Everything downstream of
the handshake — headers, heartbeat writer, RPC attach, teardown — is
unchanged.
The regression test drives the dial from a child process. net/http
resolves the proxy environment once per process and caches the result
(envProxyOnce), so by the time a test in this package runs, an earlier
test has already primed that cache with "no proxy" and t.Setenv can no
longer reach it. The child starts with a clean environment pointing at
a stub CONNECT proxy, and the parent asserts the CONNECT for the wss
target arrived. Without the fix the stub proxy sees nothing.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Multica Eve
|
33a2743f93 |
fix(llm): make quick actions GPT-5.6 compatible (MUL-5573) (#6243)
* fix(llm): use max completion token limits Co-authored-by: multica-agent <github@multica.ai> * fix(llm): harden GPT-5.6 JSON generation Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Jiayuan Zhang
|
28b6105edc |
fix(subscribers): notify the human an agent files sub-issues for (MUL-5483) (#6209)
When an agent created a sub-issue while working on a human's behalf, that human received no notifications for it at all. issue_subscriber modelled ACTOR identity, so an agent-created, agent-assigned issue had a full subscriber list and zero members to deliver to. The platform already knew who the work was for (agent_task_queue.originator_user_id, MUL-4302); notification never asked. - attribution.DelegatedSubscriber: one shared rule over the same origin waterfall ClassifyDirect uses. agent_create subscribes the originator as 'delegated'; quick_create keeps the direct 'creator' tier; autopilot and degraded attribution subscribe nobody. - Delegated is a reduced delivery tier: in_review/done/cancelled/blocked plus failures and mentions. Routine churn is suppressed, and the parent bubble cannot re-deliver what the tier dropped. - Unsubscribe becomes stateful: an unsubscribed_at tombstone survives later rule passes, and opt_out_scope distinguishes "this issue" from "this subtree" so a narrow opt-out no longer silently suppresses future children. - Subtree unsubscribe is its own endpoint. A body flag cannot fail loudly against an older backend (Go drops unknown fields); an unknown route 404s, which the UI now surfaces with a distinct message. - Eligibility and the write share one statement under a (workspace, user) advisory lock that subtree unsubscribe and member revoke also take, closing the check-then-insert races. Revoke additionally clears the departing member's subscriptions in the same tx. - UI explains a delegated subscription and offers both unsubscribe scopes. Migrations 249/250 add the delegated reason, the opt-out tombstone, and the opt-out scope, using NOT VALID + VALIDATE CONSTRAINT so the widened CHECK does not scan issue_subscriber under an exclusive lock. Reviewed across eight rounds; an earlier write-time subtree roll-up was built and then removed in full once it proved unfixable without serializing every topology mutation. The parent's own status transition already carries that signal. Closes MUL-5483. |
||
|
Jiayuan Zhang
|
d4ae220cc1 |
feat(rich-content): render bare in-app project/issue URLs as chips (MUL-5499) (#6141)
* feat(rich-content): render bare in-app project/issue URLs as chips (MUL-5499) A project has no `MUL-123`-style identifier — only a UUID and a free-text title — so there is nothing for the bare-identifier autolink preprocessor to detect, and the link copied out of the app is how people actually reference one. It rendered as a raw URL. RichLink now unfurls a bare in-app entity URL into the same chip the `mention://project/<uuid>` form already produces (issue URLs go through the same path for symmetry). Render-only: stored markdown is untouched, and the editable Tiptap path is deliberately unaffected. Three guards, each load-bearing: the link must be bare (an authored label is never discarded), same-workspace (a chip resolves its title in the current workspace only), and address exactly one entity page by UUID with no query or fragment. Also: - mobile: tapping a `mention://project/` link navigated nowhere despite the `project/[id]` route existing — it now pushes the project detail. - agents had no documented way to emit a clickable project reference: add the link form to the runtime brief's Mentions section and to the projects skill, and record in the mentioning skill why `project` sits outside `MentionRe` (render-only, enqueues nothing). Co-authored-by: multica-agent <github@multica.ai> * fix(rich-content): unfurl issue URLs in identifier form The unfurl required a UUID id, on the stated grounds that "every link the app itself produces carries a UUID". That holds for a project but not for an issue: `copyLink` and `openInNewTab` both build `paths.issueDetail(issueIdentifier || issueId)`, and the issue route rewrites a UUID URL back to the identifier — so `MUL-123` is the shape a user actually copies, out of the app or out of the address bar. The issue half of the feature could not fire on the links people paste, while bare `MUL-123` prose did become a chip: the fuller reference lost to the shorter one. `parseWorkspaceEntityLink` now accepts an issue identifier as well as a UUID. A project still requires a UUID — it has no shorthand, so an identifier-shaped id under /projects/ addresses nothing. An identifier needs a lookup, which means it can miss, and the miss has to differ by entry point. `AutolinkedIssueMentionLink` degraded to plain text, which is right for autolinked prose and wrong for a URL: the author wrote a link, and an issue this workspace cannot see must not cost them the only pointer to it. The fallback is now a prop — plain text for the autolink path, the original anchor for a URL. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(editor): stop drawing link chrome over mention chips A mention chip already carries its own affordance — border, icon, hover background — so the generic `.rich-text-editor a` color and underline draw a second, competing one straight through the card. `.issue-mention` reset it; `.project-mention` never did, so project chips shipped with a brand-coloured underline through them. The rule belongs to the chip shape rather than to one entity, so both selectors now share it and a future chip is one line. The hover card had the same gap: it skipped `.issue-mention` only, so hovering a project chip opened a URL card offering to copy `/{slug}/projects/{uuid}` — an in-app path, not the shareable link that wording implies. Both are pre-existing, but a bare project URL now renders as a chip, so what used to surface on hand-written mentions alone shows up on ordinary pasted links. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(rich-content): decide in-app by resolving the URL, not by its prefix `href.startsWith("/")` was standing in for "this deployment". It is not: a browser reads `//other.example/x` and `/\other.example/x` as another host and goes there, and both start with a slash. The parser skipped the origin check for exactly the hrefs that most needed it. Nothing shipped from this: an unfurled chip links to `paths.projectDetail(uuid)`, so the href it was parsed from is discarded and a misparse could not send anyone anywhere. The prefix test was still the wrong instrument. Adding `&& !startsWith("//")` would have looked like a fix while leaving the backslash spelling through — the gap is the technique, not the case, so this resolves the href against the app origin with `URL` and compares `origin`, which is one comparison for every spelling and for the schemes (`javascript:`, `data:`) whose opaque origin can never match. Relative and absolute now take the same path, so the slugless legacy form parses identically whether or not it carries the origin — previously the absolute spelling was rejected by a reserved-slug test meant for workspace slugs, and the two disagreed. `openLink` still tests the prefix, and its result IS navigated. That is a live issue, older than this feature and wider than it; it needs its own change rather than a quiet ride here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(skills): state what mobile actually does with a project mention The projects skill told agents a `mention://project/<uuid>` link "renders as a navigable project chip on web, desktop, and mobile", and that a pasted project URL is unfurled into that same chip by "the reader's client". Neither holds on mobile: `apps/mobile/lib/markdown/markdown.tsx` renders the default enriched link and only routes the tap, and a bare URL still goes to `Linking.openURL`, which leaves the app. These files enter agent context and read as product contract, so an agent choosing between a mention link and a pasted URL was choosing on false information — and the URL is the option that strands a mobile reader in a browser. Both skills and both source maps now say chip on web/desktop, ordinary link that opens the project on tap on mobile, and unfurling as web/desktop only. The projects skill also now states the preference outright rather than presenting the two forms as equivalent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(views): give a project mention the component an issue mention has `IssueMentionCard` owns "chip inside a link" for issues; the project equivalent lived inline in the readonly renderer, so nothing named the pairing and nothing held the rules that come with being a link. That cost was not hypothetical. Both gaps fixed a commit ago landed on project mentions alone: `.project-mention` never got the CSS rule cancelling generic link chrome, and the hover card never learned to skip it. Each was written for `.issue-mention` at the component that owns it, and project had no such place for the second half to be written. `ProjectMentionCard` is that place. No behaviour change: same anchor, same href, same hover affordance, same accessibility contract that project-mention-a11y.test.tsx pins. The "open in new tab" preference stays out — it is scoped to issue links, and inheriting it by symmetry would be inventing product. Also drops `not-prose` from both cards. It has no definition anywhere in the repo — Tailwind's typography plugin is not installed, and the class does not appear in built CSS — so it read as protection that was not there. The editor's `MentionView` keeps its hand-rolled anchors: it needs a modifier-click intent hook `AppLink` does not expose, and it does the same for issues, so the two stay symmetric there too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Lambda <lambda@multica.ai> Co-authored-by: multica-agent <github@multica.ai> Co-authored-by: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Multica Eve
|
f48bd655bc |
MUL-5562: optimize application-owned workspace deletion (#6230)
* fix(workspace): optimize application-owned deletion Co-authored-by: multica-agent <github@multica.ai> * fix(workspace): address deletion review findings Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Multica Eve
|
e4b6f7a31b |
MUL-5581: add Qoder CN CLI runtime (#6232)
* feat(agent): add Qoder CN CLI runtime Co-authored-by: multica-agent <github@multica.ai> * fix(agent): address Qoder CN review nits Co-authored-by: multica-agent <github@multica.ai> * fix(agent): defer Qoder CN version gate Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Jiayuan Zhang
|
f13969b996 |
refactor(chat): generate quick actions server-side via the LLM layer (MUL-5573) (#6214)
* refactor(chat): generate quick actions server-side via the LLM layer (MUL-5573)
Follow-up suggestions were produced by a second, full provider CLI invocation
per chat turn: the daemon resumed the just-finished session and ran a
suggestion-only pass. That pass inherited the main turn's exec options, so its
20s budget had to cover process spawn, every MCP handshake, session replay, and
model reasoning at the agent's own thinking level — typically 8-15s of visible
skeleton, and every turn paid two provider cold starts.
Generate them here instead, through the same pkg/llm layer that backs chat
auto-titling. Suggestions need no tools, workdir, or agent identity — only the
tail of the conversation — so a bounded 8s call on the deployment's small model
replaces the whole resumed turn.
Quality changes that came with the move:
- The prompt now states the frame explicitly ("you write FOR THE USER"). The
old pass ran inside the agent's session and inherited the runtime brief's
identity, which drifted suggestions toward agent-operations actions.
- Previously-offered labels are replayed as ALREADY SUGGESTED. The old
architecture had the opposite effect: on providers that append on resume,
each pass saw its predecessor's JSON and anchored on it.
- A failed generation broadcasts failed=true. Before, a timeout delivered an
empty array — indistinguishable from "nothing worth suggesting", so every
slow pass read as a quality problem.
- The in-band footer is still stripped from replies but its actions are now
discarded, so a pre-upgrade session is not pinned to the retired
suggestions with the replacing pass suppressed.
The refresh path no longer enqueues an agent task: it validates the target and
calls the same generator, which also drops the not-resumable refusal — a session
whose runtime was rebound can now be refreshed. Client contract is unchanged
(chat:done pending flag, chat:quick_actions supplement); the only frontend
change is the pending window, resized from 30s to 12s to match the new budget.
Also removes the daemon's TMPDIR-after-cleanup hazard by construction: the old
pass started after runTask's defers had already deleted the temp dir it was
still pointed at.
Co-authored-by: multica-agent <github@multica.ai>
* refactor(chat): drop the quick-actions opt-out setting (MUL-5573)
Suggestions are always on. The Settings → Chat toggle is removed along with
the whole per-turn opt-out path it fed: the persisted client preference, the
quick_actions_enabled send field, the quick_actions_disabled task stamp, and
the eligibility gate that read it.
The toggle predates server-side generation, when it could only hide chips a
provider pass had already paid for. Now that generation is a bounded call the
server decides on, an off switch buys nothing a user would miss, and it was
the last piece of UI implying the feature might be unavailable.
agent_task_queue.quick_actions_disabled is no longer written (dropped from
CreateChatTask's INSERT; the column keeps its false default). Left in place
alongside regenerate_quick_actions_for for a later drop migration — removing
columns an already-running binary still inserts would break mid-deploy.
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): address quick-actions review findings (MUL-5573)
Four defects from review of the server-side generation change.
1. Automatic failures were reported as refresh failures. The generator
broadcast failed=true on any LLM error, but the client turns every
failed=true into a "couldn't refresh" toast — so an automatic timeout
popped a toast for an action the user never took. This also contradicted
ChatQuickActionsPayload.Failed, which documents false for the automatic
pass. The caller now passes its origin; only an explicit refresh reports.
2. Generation context was not bound to the target turn. The pass re-read the
session's newest messages while always writing to the task it was handed,
so a turn landing between the completion callback and the detached read
supplied the context for a reply it did not belong to. Worse, a user
typing a follow-up in the second after a reply left the window ending on
a user row, which the old code treated as "nothing to build on" — that
turn silently never got pills. The window is now anchored on the target
assistant message and queried strictly before it.
3. No concurrency or idempotency bound on generation. Refresh stopped
creating a task, so the busy check could not see a pass already running:
two refreshes both returned 202, spent two upstream calls, and raced to
write one row. Nothing bounded generation process-wide either. Adds a
per-session in-flight guard (refresh now 409s on a duplicate) and a
process-wide ceiling; a shed pass still resolves the client placeholder
so no skeleton hangs on work that never started.
4. A new daemon could not safely talk to an older server. The refresh task
discriminator was deleted, so a regenerate task from such a server fell
through to the ordinary chat path: no user message, but the agent would
answer anyway and the server would persist it as a real reply. The field
is restored as a refusal marker only — the task completes empty, which is
the shape the retired pass produced and which that server writes no row
for. Not a restored execution path.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Lambda <lambda@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
|
||
|
Bohan Jiang
|
98766cc06a |
feat(daemon): remove the Linux Codex per-task HOME; default Linux to danger-full-access (MUL-5578) (#6233)
* feat(daemon): default Linux Codex to danger-full-access on the real HOME (MUL-5578) Linux Codex tasks ran under the `workspace-write` Landlock sandbox with a generated per-task HOME: the daemon rewrote HOME/XDG_*/npm_config_cache into `<envRoot>/home`, symlinked a hand-maintained allowlist of seven credential paths back into it, and granted that directory as a `writable_roots` entry. That mechanism could not converge. Any host CLI outside the allowlist (aws, kubectl, gcloud, glab, rclone, cargo, …) started every task as if unconfigured even though it works in the daemon user's shell, and three open issues asked to extend it in three incompatible directions (#5636, #5573, #3867). The containment it bought was also narrower than it looked: workspace-write restricts writes only — reads and network were already unrestricted, and `.ssh` was seeded into the task home — so credentials under the real HOME were readable and exfiltratable regardless. Linux now runs `danger-full-access` on the daemon user's real HOME and inherited XDG environment, matching macOS and Windows. The task filesystem boundary is the boundary the daemon itself runs inside (VM, container, or dedicated Unix user), which is what the other providers already assumed — Claude Code runs with `--permission-mode bypassPermissions` today. This also removes the split-brain the mechanism could enter: the task-HOME decision read the platform default only, so a `-c sandbox_mode=danger-full-access` override (which passes arg filtering and wins over config.toml) left a task running unsandboxed while the daemon still redirected HOME and emitted writable_roots for a sandbox that was not in effect. With no HOME rewrite there is only one environment contract left to disagree about. Removed: prepareTaskHome, prepareCodexSandboxHome, TaskHomeEnv, Env.TaskHome, both seed allowlists, and the now-unfed WritableRoots plumbing through codexSandboxPolicy / CodexHomeOptions. Env roots created by older daemons keep working; their leftover `home/` directory is simply ignored and reclaimed with the env root. Task-scoped CODEX_HOME is untouched — it is managed Codex state, not the Unix HOME. macOS, Windows, and non-Codex providers are unchanged. Co-authored-by: multica-agent <github@multica.ai> * docs: document the agent execution security model (MUL-5578) The docs had no page describing what a task can reach on the machine that runs it — the sandbox posture was only discoverable from source. Adds one, stating plainly that tasks run with the full permissions of the daemon user and that isolation must come from a dedicated Unix user, container, or VM. Also separates what Multica genuinely isolates (per-task workdir, task-scoped CODEX_HOME, agent+task-bound API tokens) from what is not a boundary (the coding tool's own sandbox and approval settings), and links it from step 5 of the self-host quickstart, where the daemon is first installed. Co-authored-by: multica-agent <github@multica.ai> * fix(daemon): address review on the Linux full-access default (MUL-5578) Docs, both must-fix: - The Chinese page rendered the product concept as `Agent`; the repo glossary in developers/conventions.zh.mdx mandates 智能体. Fixed all nine occurrences. ja/ko already used エージェント / 에이전트 and needed no change. - The security page asserted without qualification that every task runs with the daemon user's full permissions and that the Codex filesystem sandbox is always off. codexSandboxPolicyForWindows still keeps workspace-write when a user explicitly opts into a native windows.sandbox, so an authoritative security page contradicted the code. It now states that Multica makes no filesystem-sandbox guarantee, names the Windows opt-in as the one current exception, and says which combinations sandbox anything is a compatibility detail that moves with tool versions. All four locales updated, plus the historical callout which now says Linux matches the macOS/Windows *default*. Both stale comments from the non-blocking notes: - prepareCodexHome no longer claims to assume workspace-write + network_access; it pins GOOS=linux, which now resolves to danger-full-access. - ensureCodexSandboxConfig's warn-level logging is no longer described as a macOS-only fallback; it fires for every danger-full-access resolution. Adds TestCodexTaskShellEnvInheritsRealHome at the daemon env-assembly layer: HOME and the XDG base dirs must reach a Codex task's shell tools from the inherited daemon environment. Verified it fails when that pass-through breaks. It guards the pass-through, not runTask's decision not to inject a HOME of its own — that decision is inline in runTask and has no unit seam. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Multica Eve
|
51f44873cc |
fix(labels): always enable resource labels (MUL-5563) (#6225)
* fix(labels): always enable resource labels (MUL-5563) Co-authored-by: multica-agent <github@multica.ai> * docs(labels): clarify resource label rollback safety (MUL-5563) Co-authored-by: multica-agent <github@multica.ai> * docs(labels): correct compat client range (MUL-5563) Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Multica Eve
|
c3f5df8bf4 |
MUL-5492: fix timeline cap dropping newest entries + stop double-broadcasting descriptions (#6175)
* fix(timeline): cap the issue timeline at the newest end and report the clamp The per-issue timeline cap was applied with ORDER BY created_at ASC LIMIT 2000, so once an issue accumulated more than 2000 comments or activities the cap discarded the NEWEST rows. The timeline appeared to stop at some point in the past and every later event was invisible, with nothing in the response indicating anything was missing. Activity is machine-paced — description autosave, every agent run, status and assignee changes all write rows — so this was reachable in normal use, not only on pathological issues. Take the window with the keyset ordering (created_at DESC, id DESC) in a subquery and re-sort ascending in the outer query. This keeps the chronological contract for every existing caller, including the comment list endpoint that shares ListCommentsForIssue, and is served as an index-only scan by the idx_*_keyset indexes already added in migration 068 — no new migration, no call-site changes. Two things beyond the ordering flip: - Clamp both lists to a shared window floor. The two caps are applied independently, so each list has its own floor. Merging windows with different floors produces a timeline that looks continuous but, below the higher floor, contains only one of the two kinds — e.g. comments with no interleaved activity. That is worse than a timeline that visibly stops, because nothing about it looks wrong. Both lists are now clamped to the newest floor, so the result is a contiguous, correctly interleaved slice. - Stop truncating silently. The unpaginated response is a bare JSON array with nowhere to put a flag, so the clamp is reported via X-Timeline-Truncated and X-Timeline-Window-From, added to ExposedHeaders because a custom response header is otherwise unreadable from browser JS. The legacy wrapped shape's has_more_before is now truthful instead of hardcoded false. Queries read one row past the cap so "hit the cap" is distinguishable from "holds exactly 2000 rows", which would otherwise report a complete timeline as truncated and drag the other list's window down with it. Regression tests cover all four properties and were confirmed to fail against both the original query and a floor-less DESC flip. Co-authored-by: multica-agent <github@multica.ai> * perf(realtime): stop broadcasting two full descriptions on issue:updated issue:updated carried prev_description alongside the new description in the issue object, and the WS forwarder reuses the producer's payload map verbatim. Every debounced description autosave therefore pushed two full copies of the description to every connection in the workspace, including users who did not have the issue open. The DB write is O(1); the fanout was O(connections x description size), and it repeats on every pause in an editing session. prev_description and prev_title exist only for in-process listeners — subscriber_listeners adds newly @mentioned users, notification_listeners builds mention notifications, activity_listeners records the title change. No client reads them: IssueUpdatedPayload in packages/core/types/events.ts does not declare either field. Project the payload on the way out. The bus dispatches bus.Subscribe handlers before the SubscribeAll forwarder, so the in-process consumers are unaffected, and projecting at the forwarder covers both the single- node Hub and the Redis relays since that is where the frame is serialized. The producer's map is copied rather than mutated. The removed keys are listed in a table rather than an if on one event type. The bug was structural, not a typo: the next large field added to a published payload inherits the same cost silently, and a declarative list puts the internal/external payload boundary in one reviewable place. issue.description itself is deliberately kept — clients apply it to their cache, so stripping it would trade fanout bytes for N refetches. Cutting the remaining fanout needs the per-issue scope routing already scaffolded server-side for MUL-1138, which is blocked on the client sending subscribe frames. Tests assert both halves: the keys are absent from the serialized frame, and the in-process listener still receives them. Co-authored-by: multica-agent <github@multica.ai> * fix(timeline): keep comment threads whole under the newest-N cap Review found that the newest-N window can orphan a reply, and an orphaned reply is invisible rather than merely mis-nested: the timeline builds its top level from "activities + comments with no parent_id" and renders replies by looking them up under their parent, so an orphan sits in the map with no card to render it. MUL-1847 / #2263 was exactly this shape — 1 root + 29 replies, root dropped, all 29 vanished from the UI while the API returned them. Root cause of the regression: capping with the OLDEST n could never orphan anything, because a reply is always newer than its parent, so a prefix of the timeline is closed under "parent of". A newest-n window is a suffix and has no such property. Flipping which end the cap bites silently invalidated a structural property the comment tree relies on. Two changes. Drop the cross-kind clamp. The previous revision trimmed both lists to a shared floor so the window was provably contiguous. That was the wrong trade and it was also the dominant source of orphans. Comments are human-paced (p99 ~30, max ever observed ~1.1k) and essentially never reach the cap, while activity is machine-paced and reaches it routinely — so the shared floor was almost always the activity floor deleting comments that had been fetched successfully and would have rendered fine. On an issue with thirty comments it was pure loss. Each list now reports its own truncation and X-Timeline-Truncated names which kinds were affected. Not clamping costs only activity density in the older part of the range, which is metadata rather than content, and it is reported rather than hidden. Complete parent chains for the case that remains — comments themselves exceeding the cap. ListMissingAncestorComments walks parent_id upward via a recursive CTE and returns the ancestors not already held; the handler merges them and restores the ascending order. This only ever ADDS rows, so unlike clamping it cannot hide anything the caller would have seen, and it is bounded by the number of distinct missing ancestors. Whole- thread windowing was considered and rejected: a single thread can exceed any row budget, so its degradation is not definable. Applied to the shared query's default list path too, not just the timeline. foldResolvedThreads documents a COMPLETE-thread set as its precondition and comment.go asserts the default list mode satisfies it; a half thread made that assertion false and a resolved thread whose root was cut stopped folding correctly. Also drops X-Timeline-Window-From. It was second-precision RFC3339 while the real ordering key is (created_at, id) at full precision, so it could not resume a read without skipping or repeating rows inside a shared second. A resumable cursor should be opaque and carry both halves; worth designing when there is a consumer rather than shipping as a lossy approximation. Tests: the reviewer's exact scenario, plus a no-orphaned-replies invariant on both endpoints, the fold-still-works case, and a guard that activity truncation does not delete comments. Each was confirmed to fail with the fix disabled. TestListTimeline_JointWindowHasNoOneSidedRegion was rewritten rather than deleted — it pinned the clamp behaviour being abandoned here, so leaving it would lock in the wrong contract and deleting it would drop the coverage. Co-authored-by: multica-agent <github@multica.ai> * fix(comments): bound parent-chain completion and stop folding partial threads Second review round on MUL-5492. Four must-fixes, all stemming from one conflation: parent-chain closure, a newest-N window, and a complete thread are three different things. Closure makes a reply renderable; it does not license thread-level derivations. Do not fold a truncated read. fetchCommentsForList closes parent chains, but older siblings and descendants of a retained reply stay outside the window, so the set holds partial threads. Folding them produced wrong answers rather than incomplete ones: a resolution reply outside the window made a resolved thread look unresolved, and folded_count reported a total derived only from retained replies. The previous revision claimed closure restored foldResolvedThreads' COMPLETE-thread precondition; it did not, and that claim is removed. --recent and untailed --thread still return whole threads and still fold. Bound the walk. The recursive CTE climbed to the root with no depth limit, so a deep chain could drag its entire ancestry back and defeat the row cap it was meant to preserve. Depth is genuinely unbounded in stored data: the general write path stores the exact comment being replied to (only the agent path collapses to the thread root), so chains can run far deeper than the two levels the UI renders. Replaced with a layered walk under explicit budgets — 2000 extra rows, 64 levels — making a response provably bounded by 4000 comments, or 6000 timeline entries with activities. Scope every level to the tenant. The CTE's recursive branch matched on parent_id alone. parent_id carries a foreign key to comment(id) but not to a matching issue, so a stray cross-issue parent reference is representable, and the walk would have followed it into another issue's comments. The replacement filters issue_id and workspace_id on every level. A negative test confirms the leak: with the filter removed it reports "a comment from another issue leaked into this issue's response". Degrade by pruning, not by orphaning. When a budget is exhausted, a parent row is missing, or a parent is out of scope, keepRootConnected drops the affected comments instead of returning replies the UI cannot render. Dropping a node also drops its descendants, since their chains run through it. Returning fewer new replies is conservative and already signalled as a truncated read; leaking another tenant's data, returning an unbounded response, or emitting invisible orphans are all worse. Also: probe read on the comment list so exactly-2000 is not misreported as truncated, which would needlessly suppress the fold; CommentsTruncated is carried on fetchCommentsResult rather than inferred from the result length, which is meaningless once completion adds rows. The new query returns db.Comment directly instead of a hand-copied row, which is how quick_action_id came to be dropped after the rebase — a backfilled quick-action root would have rendered as a raw prompt. Corrected three inaccurate comments: the "index-only scan" claim (the index avoids the sort but does not cover SELECT *), the "write path collapses replies to root" claim, and a test header still describing the abandoned contiguous-window behaviour. Tests cover exactly-at-cap still folding, truncated reads not folding (both reply-resolved and root-resolved), depth beyond budget pruned not orphaned, shared ancestors fetched once, cross-issue parents never crossing the boundary, and quick_action_id surviving backfill. Each was confirmed to fail with its specific fix disabled; the reply-resolved fold test was reshaped after the first version passed for the wrong reason. Co-authored-by: multica-agent <github@multica.ai> * fix(timeline): preserve complete threads under comment cap Co-authored-by: multica-agent <github@multica.ai> * fix(comments): preserve newest bounded views Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Bohan Jiang
|
f4bf8e2c36 |
fix(realtime): bound inbound WebSocket message size (MUL-5569) (#6222)
The client-facing realtime hub upgraded a connection and read from it without ever calling SetReadLimit, so gorilla buffered a whole inbound message in memory before any application-level check ran. A fragmented message with interleaved pong frames keeps refreshing the read deadline, so a single connection could grow that buffer without bound and OOM the process, taking every workspace on the instance down with it. Set a 64 KiB limit — matching the daemon hub, three orders of magnitude above the largest legitimate frame — immediately after the upgrade rather than in readPump: the token auth path reads its first frame before the caller is authenticated, so a limit installed any later leaves that read unbounded. Over-limit closes get their own counter on both paths so the breach stays visible instead of blending into ordinary churn. Closes #6210 Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Bohan Jiang
|
abdfd3e28c |
refactor(skills): make the brief's skill list a names-only index (MUL-5529) (#6207)
* refactor(skills): make the brief's skill list a names-only index (MUL-5529)
Step 3 of MUL-5529. Every runtime CLI discovers the SKILL.md files the daemon
writes and builds its own listing from their frontmatter — verified against 11
locally installed CLIs plus official docs for 5 more. The brief's copy of those
descriptions was therefore the same routing signal paid for twice: measured on
a real task, `## Skills` was 13,295 chars, 40% of the entire brief, against a
16,304-char CLI listing of the same 28 skills.
Now 850 chars for that same set — roughly 3,100 tokens back per brief.
The index itself stays. It is the one skill listing Multica controls; each
CLI's own listing is theirs, and its format — or its existence — can change
with any release.
Three changes:
- Descriptions dropped from the `## Skills` entries.
- The per-provider branch is gone. Its fallback told providers outside a
hardcoded list to read `.agent_context/skills/`, but the only providers
that ever reached it were grok and traecli, whose files are written to
`.grok/skills` and `.traecli/skills` and which discover natively. The
pointer was wrong for everyone it addressed, so removing the branch
deletes the bug rather than relocating it. This closes MUL-5537.
- issue_context.md and its quick-create / autopilot variants no longer render
`## Agent Skills`. That copy duplicated the brief once both were
names-only, and nothing ever read it: no prompt references the path, and
grepping the server finds only the writer. `.agent_context/skills/` had the
same fate for hermes (issue #5242). Quick-create, previously skipped in the
brief and served only by that unread copy, now gets the brief section like
every other kind — one index, one place.
Not included: skills carrying `disable-model-invocation` are still written to
disk for every provider. The plan assumed that key needed provider-specific
handling for everything except claude; probing the installed CLIs shows 9 of 11
honor it, and only opencode and hermes do not. The remaining question is
narrow and a genuine product tradeoff — withholding the file honors the
author's intent but also removes explicit invocation — so it is left to a
separate decision rather than folded in here.
Co-authored-by: multica-agent <github@multica.ai>
* docs(skills): align stale comments with the names-only brief contract (MUL-5529)
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
Co-authored-by: Steve Jobs (Multica Agent) <agent-steve-jobs@multica.ai>
|
||
|
Naiyuan Qing
|
13b06f038e |
fix(issues): count agents working in the surface, not the workspace (MUL-5525) (#6191)
* fix(issues): count agents working in the surface, not the workspace (MUL-5525) The "N agents working" chip ran its own workspace-wide `/api/working-agents` read while the list it filters came from the surface's own compiled query. Two definitions of the same question, so on a project page the chip could advertise agents working nowhere near that project and then open an empty list. Every other narrowing the list knows about — status, priority, assignee, creator, label, custom property, date, sub-issue display, the /issues Members/Agents tabs — was invisible to the count for the same reason. Only /my-issues (relation) and the issue-detail sub-issue chip (parent) were narrowed, because those were the two cases the endpoint had grown parameters for. Rather than add a `project_id` parameter and leave the next dimension to be discovered the same way, the count now comes from a `working_agents` facet on the existing issue-table facets endpoint: same scope, same filters, same compiled WHERE clause the rows come from, joined to running issue tasks and grouped by agent. Correct-by-construction instead of correct-by-keeping-two-lists-in-sync. - Facet is disjunctive like every other one: it drops `working_issue_ids` / `working_only`, so the answer is identical whether the filter is on or off and the number does not move when you click the chip. - Facet keys are agent ids, so they pass the same visibility gate as the other workspace-wide agent aggregations — a private or non-allow-listed agent is not disclosed by id, count, or presence. - Gantt keeps a client-side count: its canvas projection (scheduled + dated + showCompleted) cannot be expressed in the Table query spec, so it counts the agents holding canvas rows instead. - The chip is now presentational; `undefined` renders the existing indeterminate label rather than a zero it cannot stand behind. - Removes the MUL-4884 `workingScopeIssues` plumbing, dead since the count moved to the endpoint in MUL-5200, keeping only the Gantt branch that still has a real consumer. Also fixes the empty state that bug dropped you into: a filtered-empty surface claimed "No issues linked — create one" while 41 issues sat behind the filter. Shared filtered-empty state now precedes each surface's own copy and offers to clear exactly the filters it blames. Verified: pnpm typecheck, pnpm test (469 files), pnpm lint (0 errors), go test ./internal/handler (new facet tests cover project scope, status and sub-issue narrowing, filter-independence, and the access gate). Co-authored-by: multica-agent <github@multica.ai> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(issues): keep the working-agents unknown state unknown (MUL-5525) The chip correctly refused to print a number for an unresolved projection, then handed the hover card `agents ?? []` — so hovering an indeterminate chip read "No agents working right now". That is the same unearned claim this issue is about, made by the one surface with room to spell it out: the label said "—" while the body next to it asserted zero. - `WorkingAgentsHoverContent` takes `readonly WorkingAgentSummary[] | undefined` and distinguishes all three states: `undefined` renders new `agent_activity.unknown_hover` copy, `[]` keeps the empty sentence, a non-empty list keeps the roster. The chip passes its projection through untouched. - The colour tier had the same collapse: unknown wore the neutral tier WITH muted text, which is exactly the "nothing is happening here" tier a known zero wears. `chipAppearance` now takes a `ChipActivity` ("unknown" | "none" | "some") instead of a boolean, so the three cases cannot be written as two, and unknown stays neutral but undimmed. - The sub-issues chip is unaffected: it passes a resolved array and renders nothing at zero, so it never claimed anything either way. Regression tests cover the hover path specifically — reverting either downgrade fails "does not let the hover body downgrade an unresolved projection to zero", "does not dim the chip while the projection is unresolved", and the chipAppearance unknown case (verified by reverting). `WorkingAgentsHoverContent` also gets direct unknown / empty / roster tests, and `chipActivity` one for the three-way split. Verified: pnpm typecheck, pnpm test (469 files), pnpm lint (0 errors). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Multica Eve
|
e6610c0831 |
fix(usage): close the per-agent rollup windows so the leaderboard cannot exceed the totals (MUL-5551) (#6194)
The Usage page showed a single agent with 1021.0M tokens under a workspace Tokens KPI of 805.9M for the same 1D window. Both halves read the same rows and disagreed only on the window. parseSinceParamInTZ deliberately returns N+1 calendar days of headroom, and the date-bucketed series (usage/daily, runtime/daily) get trimmed back to -(days-1) client-side before the KPIs and the chart are computed. The two per-agent rollups behind the leaderboard carry no date column, so nothing trimmed them and they kept the full N+1 span: at days=1 that is today PLUS yesterday. One busy agent's two-day total then trivially exceeded the workspace's one-day total. Same defect and same fix already applied to failures/by-agent: switch usage/by-agent and agent-runtime to parseExactSinceParamInTZ. This also realigns the Run time / Tasks KPI tiles, which are sourced from agent-runtime and were therefore a day wider than the Cost / Tokens tiles beside them. Co-authored-by: Eve <eve@multica-ai.local> |
||
|
yushen
|
0a54485ab6 | chore(llm): use gpt-5.6-luna by default | ||
|
Jiayuan Zhang
|
2e0c599edd |
fix(agent): avoid H1 headings in issue bodies (#6199)
Co-authored-by: Lambda <lambda@multica.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Jiayuan Zhang
|
0fdc38704e |
MUL-5149: add agent-generated Chat quick actions (#5766)
* feat(chat): add agent-generated quick actions
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): preserve mid-response quick-action fences
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): drop quick actions on empty reply to keep no_response fallback
An actions-only completion — a quick-actions footer with no visible text —
wrote an empty-content assistant message (message_kind=message). Older
Desktop/mobile clients ignore the quick_actions field and render that as an
empty bubble, breaking the MUL-4351 contract that an empty turn always gives
old clients a visible no_response fallback.
Drop the quick actions when the visible body is empty so an actions-only turn
falls through to the visible no_response outcome, and revert the completion
switch to gate the message row on visible text only. Update the completion
test to pin the corrected behavior.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
* feat(chat): generate quick actions via daemon suggestion pass
Replace the in-band runtime-brief instruction with a dedicated post-completion
provider turn: after a direct chat reply finishes, the daemon resumes the same
session with a JSON-only suggest prompt and forwards the raw output on the
complete callback. The server parses it leniently and reuses the existing
sanitize/redact/store/broadcast pipeline; the stripped in-band footer stays as
a fallback for older daemons and pre-upgrade sessions. The footer strip now
covers every chat completion, fixing the intro-turn protocol leak. Adds a
Settings → Chat toggle (client-persisted, default on) that hides the chips.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(chat): deliver quick actions async with skeleton placeholders
Decouple suggestion generation from the turn: the daemon reports completion
immediately (chat:done carries quick_actions_pending as a per-turn capability
signal) and runs the suggestion pass in the background, delivering results
through a new supplement endpoint + chat:quick_actions broadcast. A new turn
on the same session cancels the stale pass. Clients render pill skeletons
under the finished reply until the supplement resolves them (entrance
animation on arrival, 30s safety timeout); older daemons never raise the flag
so no skeleton dangles. Suggest usage re-reports merged totals because
task_usage upserts replace per (task, provider, model). Prompt now asks for
exactly 3 actions.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(chat): make the quick-actions toggle stop generation, not hide pills
The Settings → Chat toggle previously only hid rendered pills while the
daemon kept burning a suggestion call every turn. It now travels with each
send (quick_actions_enabled, absent = enabled for older clients), is stamped
on the chat task (migration 213), forwarded on the claim, and gates the
daemon's suggestion pass at the source — no call, no pending flag, no
skeleton. Existing suggestions stay visible; settings copy now says
'generate' instead of 'show'.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(migrations): renumber quick-action migrations onto current main
Merging current origin/main brought the vcs migrations to their canonical
216-221 prefixes, which collided with the quick-action migrations that were
sitting at 219/220 (backend CI red in
TestMigrationNumericPrefixesStayUniqueAfterLegacySet). Renumber them to the
next unused prefixes:
- 219_chat_message_quick_actions -> 222_chat_message_quick_actions
- 220_agent_task_quick_actions_disabled -> 223_agent_task_quick_actions_disabled
Contents are unchanged; sqlc regeneration produces no drift since the added
columns are independent of the vcs tables.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
* fix(mobile): render async chat quick actions via chat:quick_actions
The daemon generates quick actions in a background pass after the turn
finishes, delivering them on a separate chat:quick_actions event. Mobile
only handled chat:done (which invalidates + refetches an actions-less
message list) and keeps the messages query at staleTime: Infinity, so an
active mobile session never rendered async-generated quick actions until a
manual pull-to-refresh or refocus.
Add applyChatQuickActionsToCache — mirroring web's patcher — which patches
the supplement onto the targeted assistant message in the flat messages
cache, and subscribe to chat:quick_actions in use-chat-session-realtime.
Patch-only (no invalidate), matching web and mobile's cellular
patch-over-invalidate rule; an empty supplement is a terminal no-op. Covered
by chat-ws-updaters.test.ts.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): cancel in-flight messages refetch before quick-actions patch
The chat:done invalidate can leave a messages refetch in flight that read the
assistant row before the daemon persisted the quick actions. If that refetch
resolves after the chat:quick_actions setQueryData patch, it overwrites the
freshly-patched actions with an actions-less row. Both message caches are
staleTime: Infinity, so the overwrite never self-heals and the actions vanish
permanently (MUL-5149, Howard review).
applyChatQuickActionsToCache now awaits cancelQueries for the affected caches
(web: flat messages + messagesPage, mobile: flat messages) before patching, so
a stale in-flight refetch is cancelled and cannot land after the patch. Cancel
must precede setQueryData because cancelQueries reverts to the pre-fetch state.
WS handlers call it via `void` (fire-and-forget).
Adds an active-query race regression test on both web and mobile that holds a
refetch open across the supplement and asserts the patched actions survive;
verified to fail without the cancel.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
* feat(chat): quick-actions refresh/regenerate + review hardening (MUL-5149)
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): address quick-actions re-review (MUL-5149)
- Ack alignment: refresh request carries the target message_id; server
atomically confirms it is still the session's latest turn (409 stale
otherwise), so the client marker always matches the resolving
chat:quick_actions — no response reconciliation. Adds a regression test.
- Converge the pending marker on every terminal path: HandleFailedTasks
(sweeper/orphan) now resolves it, and the daemon reports a failed supplement
so FailTask resolves it instead of leaving a completed-but-unresolved task.
- Timeout fallback now clears the real query state (useQuickActionsPendingTimeout)
instead of a component-local flag that only masked the UI; drop the skeleton's
and pill row's local timers.
- frontend-test type-scale: text-xs -> text-caption. Strip EOF blank line.
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): close quick-actions refresh races and failure feedback (MUL-5149)
Third-round review of the refresh button surfaced three issues; all three
are addressed here.
§1/§2 Session-busy race + concurrent-refresh double-spend: a newer reply
that is queued/running but whose assistant row hasn't landed leaves the old
turn as latest-persisted, so the stale check passes and the regen resumes
the newer provider state — attaching suggestions to the wrong turn. And two
concurrent refreshes each enqueue a quota-spending pass. Add
HasActiveChatTaskForSession and refuse a refresh (ErrChatQuickActionsBusy →
409) whenever the session has any task in flight, checked under the same
session lock as the enqueue so no sibling insert slips past.
§3a Timeout re-arm on surface switch: the pending marker now carries an
absolute expires_at deadline instead of a per-mount timer, so switching
between the floating window and the chat tab resumes the same deadline
rather than restarting a fresh 30s window each remount.
§3b Generation failure masked as success: runChatSuggestPass now returns ok
so an explicit refresh distinguishes a failed pass (didn't start / didn't
complete / timed out) from a completed-but-empty one. On failure the regen
task reports failure, resolveFailedRegenerateQuickActions broadcasts a
FAILED chat:quick_actions, and the client resolves the spinner AND toasts
"couldn't refresh" instead of silently stopping on unchanged pills.
Co-authored-by: multica-agent <github@multica.ai>
* fix(chat): count deferred tasks in refresh busy check; solid refresh icon tone (MUL-5149)
Two re-review blockers on
|
||
|
Bohan Jiang
|
5199278780 |
fix(skills): give every skill one name across brief, directory, and frontmatter (MUL-5529) (#6189)
* fix(skills): give every skill one name across brief, directory, and frontmatter (MUL-5529)
A skill could answer to three different names at once. The runtime brief listed
`AgentSkillData.Name` verbatim — a workspace skill's human display name ("PR
review") — while the invocable identity on disk is the sanitized slug
(`pr-review`). Separately, ensureSkillFrontmatter returned valid upstream
frontmatter untouched, so a SKILL.md could declare `name: multica-dev-workflow`
inside a directory called `multica-git-workflow`.
That last divergence is the sharp one: runtimes disagree on which field
identifies a skill. Claude routes on the directory name, OpenCode on the
frontmatter `name`. So the same skill is invocable under different names
depending on where it runs, and the brief's instruction to use "only names from
the listing" pointed at names that resolve nowhere.
The slug is authoritative: it is what lands on disk, it derives from the name
users see in the product, and it is the only value with a uniqueness guarantee
(allocateCollisionFreeSkillDir). A frontmatter `name` is author-supplied and two
imported skills may both claim the same one.
- modelVisibleSkills normalizes Name to the slug. All four model-visible
listings (runtime brief + the three issue_context renderers) already route
through it, so they cannot drift apart.
- ensureSkillFrontmatter rewrites `name` to the allocated slug and keeps every
other key byte-identical, so deliberately shaped upstream frontmatter still
survives. The rewrite follows the collision fallback slug too.
- Name matching is now top-level only. An indented `name:` belongs to a nested
mapping; treating it as the skill's identity both missed that the block had no
top-level name and would have spliced a top-level key into the nested one.
Known gap, tracked separately: the listings render the natural slug, so a
collision fallback to `<slug>-multica` still leaves the brief naming the user's
skill. Closing it needs the allocated slug threaded back from Prepare, which the
renderers cannot reach without giving up the byte-identical-brief guarantee.
Co-authored-by: multica-agent <github@multica.ai>
* fix(skills): cover multi-line name values and in-batch slug collisions (MUL-5529)
Two holes in the name-unification change, both found in review, both
reproduced before fixing.
1. setFrontmatterName replaced only the `name:` line, not the rest of the YAML
value. A value may continue onto indented lines — `name: >-\n upstream`,
multi-line plain scalars, wrapped quoted scalars — so the continuation
survived and YAML folded it into the new value: the block parsed as
"my-slug upstream-name", not "my-slug". The directory == frontmatter-name
invariant this change set exists to establish was still broken, just less
visibly. frontmatterNameSpan now covers the whole value.
As a side effect this also fixes `name:` with the value entirely on the
following line, which previously read as "no name" and got a second
top-level `name` injected above it — two `name` keys, which strict loaders
reject outright.
2. The listings derived slugs with sanitizeSkillName alone, which is not
injective: "A B" and "A-B" both reduce to "a-b". writeSkillFiles resolved
that at write time, so the second skill landed in `a-b-multica` while both
were listed as `a-b` — the second skill had no invocable name and the model
was pointed at the first. This needed no user-installed skill and no
local_directory; two such skills bound to one agent reproduce it in a clean
workdir. resolveSkillSlugs now deduplicates the batch up front and both the
listings and the writer derive from it, with skillSlugCandidate shared so
the in-memory and filesystem allocators cannot disagree on the suffix
sequence.
Slugs are allocated over the unfiltered batch: hidden
(disable-model-invocation) skills are still written to disk and still
consume a slug, so filtering first would shift every later suffix.
Filesystem-dependent collisions against user-installed directories remain out
of scope and are still tracked in MUL-5550.
Regression tests assert the parsed YAML value rather than the output text —
the first line looked correct in every one of these cases — and set-equality
between listed names and the directories actually written. Both were confirmed
to fail against the previous implementation.
Co-authored-by: multica-agent <github@multica.ai>
* fix(skills): bound the frontmatter name value with the YAML parser (MUL-5529)
Review round two found a valid multi-line name the indentation rule still
mangled. A quoted scalar may wrap onto a line at the *same* indentation as its
key:
name: "upstream
continued"
The rule stopped at the key's line, stranding `continued"` and producing
invalid YAML. Probing the parser showed the same holds for flow collections
(`name: [a,\nb]`, `name: {a: 1,\nb: 2}`), so this was not a quoting special
case: indentation simply does not bound a YAML value, and no amount of
patching the heuristic would have made it one.
Value extent now comes from yaml.v3's own line numbers. frontmatterNameValueSpan
locates the `name` key node and ends the span where the next top-level key
begins, stepping back over blank lines and unindented comments so they survive
the rewrite. Unindented is the operative word: block scalar content is always
indented, so an unindented `#` can only be a comment, while ` # text` inside a
block scalar is value and stays in the span.
Detection stays lexical, in lexicalFrontmatterNameSpan. It runs on malformed
blocks too, where there is no parse to consult, and only decides which branch
to take.
setFrontmatterName now re-parses its own output and returns verified=false
unless `name` really is the slug; ensureSkillFrontmatter then routes to the
existing re-synthesis path rather than emitting a block it cannot vouch for.
This adds no new fallback — it feeds an already-present one. The invariant is
the whole point of the change, so an unprovable rewrite is worth less than a
reformatted block: with the span deliberately broken, output stays valid YAML
carrying the right name and loses only upstream formatting.
Tests extend the table to same-indent single/double quoted scalars, flow
sequences and mappings, and name-as-last-key, and now assert the *input* parses
so a case cannot pass by silently taking the re-synthesis path. Two more cover
comment survival and a `#` line inside a block scalar name. All four
same-indent cases fail against the previous implementation.
Co-authored-by: multica-agent <github@multica.ai>
* fix(skills): preserve policy keys when the surgical name rewrite fails (MUL-5529)
Review round three: the post-condition check added last round was routing valid
YAML into bare re-synthesis, and re-synthesis emits only name and description.
An anchor on the name value is one way to get there. Replacing the line drops
`&skill_name`, so `description: *skill_name` no longer resolves, the check
correctly rejects the rewrite — and the block was then rebuilt as just:
name: my-slug
`disable-model-invocation: true` went with it. That key is the author's
instruction that a runtime must not surface the skill on its own, and the
SKILL.md we write is what native discovery reads, so losing it advertises a
skill that was deliberately hidden. Confirmed on the written output:
skillDisablesModelInvocation went from true to false. That is a semantic
regression, not the formatting loss the fallback was justified by.
The two failure modes are now separate:
- invalid YAML → re-synthesize, unchanged; there is nothing to preserve.
- valid YAML, rewrite unprovable → renameFrontmatterNameViaNode rebuilds the
block from the parsed node with `name` set to the slug, keeping every other
key. Formatting normalizes; semantics survive.
The anchor is deliberately kept on the name node. Dropping it is what
invalidates a document that aliases it; keeping it means the alias resolves to
the slug — that value changes, but the document still loads and every policy
key is intact. The rebuilt block is re-parsed and checked like the surgical
path, so an unprovable result still falls through to re-synthesis.
Regression test asserts name, disable-model-invocation, and a custom key all
survive, and that the written file still reads as hidden to
skillDisablesModelInvocation. It fails without the new path.
Co-authored-by: multica-agent <github@multica.ai>
* fix(skills): materialize aliases before renaming an anchored name (MUL-5529)
Round three kept the anchor on the name node so aliases would not dangle. That
avoided an invalid document but created a worse one: every alias resolves
through the anchor, so renaming the anchored value silently rewrote whatever
those fields meant.
name: &shared "true"
disable-model-invocation: *shared
skillDisablesModelInvocation went true -> false across the rewrite. Same
skill-exposing regression as round three, reached by keeping the key instead of
dropping it — which is the lesson: preserving a key is not the invariant,
preserving each key's resolved value is.
Aliases pointing at the name node are now materialized to the value they
resolved to *before* the rename, and the anchor is dropped afterwards as
unreferenced. Ordering matters: the clones are taken first, so they capture the
original value rather than the slug. Copies are per-alias, since sharing one
node would make the encoder re-emit an anchor/alias pair.
Nested aliases are covered by walking the whole document, not just the top
mapping.
Tests: three shapes (alias carrying the policy value, alias nested in another
mapping, two aliases of one anchor) each assert the fixture starts hidden and
stays hidden, that no anchor or alias survives, and that name is the slug. The
round-three test now also pins its aliased `description` to the pre-rename
value instead of merely tolerating the slug. All four fail without the change.
Co-authored-by: multica-agent <github@multica.ai>
* fix(skills): let the parser decide whether a name key exists (MUL-5529)
The branch that chooses between "rewrite the name" and "inject a name" was
still gated on a lexical scan, which only recognizes a bare `name:` carrying a
value on the same line. Two valid spellings therefore read as nameless:
"name": upstream # quoting is syntax, not identity
name: # a key with no value is still the key
Both got a second `name` injected above the existing one, and a duplicate
mapping key is rejected outright — `mapping key "name" already defined`. The
skill does not end up misnamed, it fails to load. Single-quoted keys have the
same problem.
For valid YAML the parsed top-level mapping now answers the question, so any
spelling of the key routes to the rewrite. The lexical scan is confined to the
invalid-YAML branch, where there is no parse to consult and it is only choosing
between re-synthesis and injection. Nesting still reads as absent: a `name`
under another mapping is not the skill's name, so one is added.
Once past the gate the existing paths handle both shapes unchanged, since
frontmatterNameValueSpan already bounds the entry by node line numbers rather
than by how the key is written.
Also tightens the alias tests per review: the nested and two-alias cases now
assert `meta.inner` and `other` still resolve to the anchor's original value,
not merely that visibility survived. The invariant is every key's resolved
value, so every alias should be pinned, not just the one that gates hiding.
Co-authored-by: multica-agent <github@multica.ai>
* fix(skills): detect quoted and valueless name keys in malformed frontmatter (MUL-5529)
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
Co-authored-by: Steve Jobs (Multica Agent) <agent-steve-jobs@multica.ai>
|