mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-08 17:36:45 +08:00
main
14
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
74ff8aabd7 |
MUL-7983: fix(agent): handle CodeBuddy startup errors without aborting active runs (#9214)
* fix(agent): fail codebuddy runs that emit a top-level error event
CodeBuddy's startup-failure shape is one top-level {"type":"error"}
line followed by a process that stays alive and silent: with an
inherited SERVER__PORT (a user-scope config shared by several tools),
a second concurrent run of the same agent fails to listen, prints
{"type":"error","error":"listen EADDRINUSE: ..."} and never emits
anything again. The parser had no branch for that event type, so the
run sat at "waiting for agent response" until the idle watchdog
stopped it - 19 of 48 runs hung this way on the reporter's machine.
- type=error is now fatal when no terminal result has been seen: the
CLI's own message becomes the run error and the hung process is torn
down immediately instead of idling. A result already seen keeps
governing the outcome, so a late informational error line cannot
flip a finished run.
- buildCodebuddyEnv now also forces SERVER__PORT=0 (OS-assigned port
per run, so concurrent runs never share one) and
CODEBUDDY_BASH_AUTO_BACKGROUND_DISABLED=1 (CodeBuddy's PowerShell
tool moves timed-out commands to the background and only honors that
variable). Both are appended last like the existing background-task
disable so a caller-provided value cannot outvote them.
Fixes #9200
Generated-by: GLM-5.3-Flash (ZCode)
* fix(agent): keep CodeBuddy running after recoverable errors
MUL-7983
Co-authored-by: multica-agent <[email protected]>
---------
Co-authored-by: Yushen <[email protected]>
Co-authored-by: multica-agent <[email protected]>
|
||
|
|
4d4cae7753 |
MUL-7312 LARI-10: Desktop PATH fix + CodeBuddy Claude-parity stream-json guards (#8340)
* fix(desktop): do not prepend PATH fallbacks over login-shell Node Prepending /usr/local/bin after fix-path let a stale system Node 12 shadow nvm/fnm, breaking shebang CLIs (CodeBuddy/OpenClaw) on --version. Append only missing fallback dirs so recovered shell PATH keeps precedence. Co-authored-by: Cursor <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(agent): align CodeBuddy stream-json guards with Claude Pass stderr into resume rejection, fail on prompt_too_long terminal_reason, and force foreground when Bash requests run_in_background — the same failure modes Claude already handles for Multica-managed headless runs. Co-authored-by: Cursor <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(agent): align CodeBuddy guards with real CLI protocol Address review on #8340: force CODEBUDDY_CODE_DISABLE_BACKGROUND_TASKS, fail on system/task_* background events, and drop Claude-only prompt_too_long / async_launched handling that CodeBuddy 2.150.0 never emits. Co-authored-by: Cursor <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(agent): update CodeBuddy handleUser test after signature change Co-authored-by: Cursor <[email protected]> Co-authored-by: multica-agent <[email protected]> * fix(agent): force CodeBuddy background-disable env last for Windows Append CODEBUDDY_CODE_DISABLE_BACKGROUND_TASKS=1 so os/exec's case-insensitive last-wins dedup keeps it over lowercase custom_env. Also extract Desktop PATH fallback helper with unit tests. Co-authored-by: Cursor <[email protected]> Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Cursor <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
c26da76c42 |
fix(codebuddy): deduplicate fallback usage by response ID (#8395)
CodeBuddy is a Claude Code fork on the same stream-json protocol, and its adapter carried the same usage-accounting defect #8382 fixed for Claude: one response can emit several assistant events sharing a message ID and usage, and the adapter summed every one of them. A run that ends without usable final usage therefore reported inflated input and cache tokens. Count each identified main-loop response once within an execution. Keep forwarding every text and tool block, and keep the existing precedence of the result's modelUsage over result-level usage over the fallback. The fallback also stops summing the assistant output_tokens placeholder and excludes nested-agent events, matching claude.go and the documented crash recovery procedure. It recovers main-loop input/cache usage, not a full bill. Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
46b5d9e6d1 |
MUL-6658 fix(agent): own the runtime process tree on every backend (#7522) (#7531)
* fix(agent): own the runtime process tree on every backend Cancelling a task on Windows left the agent running. The daemon marked the task cancelled, revoked its mat_ token, and killed what it thought was the CLI — but on Windows the direct child is usually a cmd.exe shim and the real CLI is already a grandchild, so it survived. One reporter's orphan kept working for 40 minutes, read the daemon owner's profile PAT after its own token started returning 401, and posted comments as the member (GH #7522). Whole-tree ownership already existed on both platforms. It was opt-in, and opt-in rotted: 8 of the 27 places this package starts a process asked for a Unix process group, and 3 of the 23 backends took Windows Job Object ownership. Adding claude to that list would have left 19 backends with the same hole, so the fix is structural instead. - newRuntimeCmd, at the single point where a runtime process is built, now puts every command in its own process group and replaces os/exec's leader-only Cancel with a group-wide kill. A backend that wants a graceful shutdown still assigns its own cmd.Cancel and wins. This also covers the --version and model-discovery probes, which cmd.Output() runs without ever reaching a start helper. - startOwnedProcessTree is now the only way this package starts a process, so Windows Job Object ownership is not something a backend can forget. Each start is paired with releaseProcessGroup after the reap, which drops the job handle and kills whatever outlived it. - Two structural tests enforce both halves, the way TestOnlyLaunchGoSpawnsRuntimeProcesses already enforces the argv boundary: a new backend calling cmd.Start fails, and one that takes ownership without releasing it fails. The Unix half is #5918 for the 20 backends the original fix did not reach: their CLI was never in its own group at all, so no group-wide signal could have reached it. Co-authored-by: multica-agent <[email protected]> * fix(agent): own the process tree for synchronous probes too Review found the ownership boundary had a hole the PR claimed to cover. os/exec's Run, Output and CombinedOutput call Start themselves, so a probe written with cmd.Output() never reaches startOwnedProcessTree and owns nothing on Windows — where the direct child of a `--version` probe is typically the npm shim and the real CLI is already a grandchild. That is the same escape #7522 was reported for, in 15 call sites nobody was looking at; detectCLIVersion's own comment already describes a broken CLI leaving grandchildren that hold the stdout pipe. - runOwned / outputOwned / combinedOutputOwned give a synchronous probe the same start, reap and release a task launch gets. They keep the stdlib contract: stdout returned, stderr attached to the ExitError, combined output through a single writer so os/exec gives it one pipe. The stderr sample is bounded, as os/exec bounds its own. - All 15 probe sites route through them, and the structural guard now rejects Run/Output/CombinedOutput alongside Start. The release guard did not hold its invariant either: it only asked whether a file contained any releaseProcessGroup, so a second unreleased start in a file that already had a pair would pass — and models.go is exactly that shape. It now works per function: one owned start each, paired with at least one release. Equal counts would be the wrong rule, since dsh's Execute legitimately releases on two exit paths. Both guards are now pure functions over an AST, with tests that feed them synthetic sources — including the second-unreleased-start case that the file-level version missed — so the guards fail closed without needing a real regression committed to find out. Adds the Windows probe regression (descendant must be in the job and die with a cancelled probe) plus unix equivalents, and names the consequence in the degraded-ownership warning, which is the only signal an operator gets that cancellation on that host is back to killing the leader alone. Co-authored-by: multica-agent <[email protected]> * fix(agent): bound a finished probe's wait on pipes its descendants hold Review caught that runOwned could wait for its own cleanup and never return, and it reproduces: a descendant that inherited the output pipes holds them open after the leader exits, cmd.Wait blocks until the copy goroutines see EOF, and what would close those pipes is the release that runs after Wait. Cancellation breaks the cycle, which is why every test in the last round missed this — they all cancelled first, and they all set WaitDelay by hand. Normal exit has nothing to break it, and checkOpenclawVersion runs on the caller's context before runContext establishes any task timeout, so a version wrapper that exits cleanly while leaving a pipe-holding descendant would hang the task until a human cancelled it. runOwned now fills in a bounded WaitDelay when the caller left it zero — Go defines WaitDelay for precisely "a child process that exits but leaves its I/O pipes unclosed" — and signals the group before dropping ownership, so the descendant is reaped on Unix too, where releasing a process group is a no-op. A caller with its own bound keeps it; detectCLIVersion has set one by hand since MUL-3812 for this same shape. Adds the normal-exit regression on both platforms: the leader exits holding nothing, the descendant keeps the pipes, and outputOwned has to return in bounded time with the descendant gone. The Windows descendant spawner gains a spawn-and-exit mode and now hands its stdio to the descendant, which is what makes it hold the pipes at all. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
3c3e77b00c |
MUL-6362: redact secrets from provider command logs (#7206)
* fix(agent): redact provider command values Co-authored-by: multica-agent <[email protected]> * fix(agent): preserve safe command diagnostics Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Sol-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
3b08423684 |
MUL-6260 fix(daemon): treat custom runtime fixed_args as the argv prefix (#7052)
* fix(daemon): treat custom runtime fixed_args as the argv prefix (MUL-6260) A custom runtime profile configured as `ccms start q36` failed every task with `unknown option '-p'`. The profile's fixed_args were merged into ExecOptions.ExtraArgs, which each backend appends *after* the protocol flags it hardcodes, so the wrapper saw `-p` before its own `start` subcommand had been selected and rejected it (GH #7046). Ordering was only half the defect. ExtraArgs is opt-in — six of the twenty-one backends read it — so on the other fifteen families, including every ACP one, fixed_args were silently discarded rather than misordered. Model discovery and `--version` probing carried the profile's path but not its arguments, so even a fixed launch would have left the model picker and the reported version describing the wrapper instead of the CLI behind it. fixed_args now travel as agent.Config.LaunchPrefix and are spliced in directly after the executable, ahead of everything a backend builds. The prefix is applied by a new Command boundary in launch.go through which every runtime process in the package is constructed — launches, version probes and discovery alike — so a backend cannot forget it. A guard test fails the build if any file outside launch.go calls os/exec directly; distributed opt-in is what let ExtraArgs rot, and repeating it would rot the same way. Flag-style commands (`agent --model composer-2.5`) are unaffected: a CLI takes its own flags in any position, which is why prefix-first can be the single order rather than a per-runtime setting. One deliberate behaviour change. The prefix used to sit last and win every last-wins parse, so a profile pinning `--model` silently overrode the model chosen on the agent — the UI showed one model and ran another. Prefix-first inverts that: the more specific per-agent choice is the later token and wins. Overlaps are logged. Protocol-critical flags are filtered out of the prefix once in New(), which knows the family; positional tokens always pass through. Codex additionally strips `-c` overrides aimed at daemon-managed config namespaces, since those beat config.toml from any argv position. The order was never documented, and every shipped fixed_args example is flag-style, so this fills in a contract rather than breaking one. Co-authored-by: multica-agent <[email protected]> * fix(daemon): close the launch-prefix gaps found in review (MUL-6260) Four defects from the first round of review on the fixed_args-as-argv-prefix change. Hermes could escape its own task overlay. The profile selection was resolved from custom_args alone and stripped from custom_args alone, so a profile configured as `hermes -p research` kept that flag in the launch prefix and re-pointed HERMES_HOME past the overlay the daemon had just built. Resolution now parses the argv hermes actually sees — launch prefix then custom args, in process order — and the strip covers both regions. It also removes every selection rather than the first: taking out the first promotes the second, and two separately-configured argv regions make "two selections" an ordinary configuration rather than a user mistake. Positional tokens were not passing through as documented. filterLaunchPrefix delegated to filterCustomArgs, whose blocklists deliberately carry positional tokens — hermes and qwenpaw block `acp`, traecli blocks `acp`/`serve`, grok blocks `agent`/`stdio`/`serve` — because in custom_args those are someone re-issuing the backend's own subcommand. In a launch prefix the same token is the command's identity, so `hermes-wrapper acp tenant` was being rewritten to `tenant`. Only a leading `-` now makes a token eligible for the blocklist; a blocked flag still consumes its separate value so no stray value survives to be read as a subcommand. Codex fast mode lost to the prefix. `--disable fast_mode` beats `--enable fast_mode` regardless of argv order and `-c features.fast_mode=false` beats config.toml from any position, so prefix-first does not protect an explicitly selected priority tier. The removal half of enforceCodexFastMode is now a separate helper applied to the prefix too; the managed enable is still appended once, on the final args. Windows CI could not compile the daemon tests: the Command migration left an unused `path := runtimeCmd.Path` in a hook whose body never referenced it. Verified with GOOS=windows and GOOS=linux vet, which is what should have caught it the first time. Also softens the docs and the Command doc comment: flag-style prefixes usually parse the same in either position, but a CLI that separates global from subcommand flags can care, so the runtime docs now name the position change and tell operators with existing fixed_args to re-run one task after upgrading instead of promising the move is invisible. Co-authored-by: multica-agent <[email protected]> * fix(daemon): resolve and strip the Hermes profile from the real launch argv (MUL-6260) Second-round review found the Hermes profile chain still reasoning about a hand-assembled approximation of the command line instead of the command line itself. The backend launches `<prefix> acp <filtered custom args>`, but the daemon assembled `<prefix> <custom args>` — no `acp`, no filtering — and that token participates in Hermes' own argv scan. With fixed_args `--model` and custom_args `-p research`, the approximation reads `-p` as `--model`'s value and finds no selection, so the overlay is seeded from the default home; the real `--model acp -p research` skips `--model acp` and selects `research`. The selector was then stripped, so the task stayed inside the overlay — but an overlay built from the wrong profile, silently. Stripping had the matching defect, and this one was still an escape rather than a mismatch. A launch prefix ending in a bare `-p` captures the backend's `acp` as its profile value, so neither region holds a complete selection and per-region stripping removes nothing: `-p acp research --yolo` reaches Hermes intact and re-points HERMES_HOME straight out of the overlay. Both halves now go through one assembly. agent.HermesLaunchArgv builds exactly what hermesBackend.Execute builds — the backend calls the same helper, so they cannot drift — and the daemon resolves from it. StripHermesProfileSelectors strips against that same argv and hands surviving tokens back to the region they came from, iterating to a fixed point because removing one selection promotes the next. The `acp` token is never removed; the backend re-adds it, and dropping the flag that captured it is what breaks the selection. StripAllHermesProfileArgs, hermesLaunchArgs and hermesEffectiveArgs are gone: each was only ever correct on a complete argv, and keeping them would leave the same footgun for the next caller. Tests cover the case the review named (prefix ending in a value-taking flag, custom args opening with a selector), the straddling-selection escape, multiple occurrences across both regions, and an invalid value left untouched because Hermes discards it anyway. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
9914d51040 |
MUL-5846 fix(agent): stop launching CodeBuddy with MCP disabled (#6556)
* fix(agent): stop launching CodeBuddy with MCP disabled (MUL-5846) `--strict-mcp-config` means "only use MCP servers from --mcp-config". buildCodebuddyArgs passed it unconditionally, so every CodeBuddy agent without a Multica-managed mcp_config was launched in strict mode with no config at all — zero MCP servers, and the user's own ~/.codebuddy.json silently ignored. claude.go already gates the flag on hasManagedMcpConfig; do the same here, and gate the --mcp-config temp file on the same three-state check so a JSON `null` no longer counts as a managed set. Also split CodeBuddy off Claude in runtime_mcp.go. CodeBuddy is a Claude Code fork but keeps its own config file and plugin root (~/.codebuddy.json, ~/.workbuddy/), so reading ~/.claude.json leaked Claude's servers into CodeBuddy launches while dropping CodeBuddy's own. Add kimi to the runtime MCP inventory (<KIMI_CODE_HOME>/mcp.json), which previously reported this runtime as not supporting MCP. Inventory only: `kimi acp` already merges that file with the ephemeral `mcpServers` array the ACP backend sends in session/new, and a duplicate name spawns the server twice, so the task-local merge stays a passthrough for kimi. Verified against CodeBuddy 2.x and kimi-code 0.33.0 with a local stdio MCP probe server: before, the strict launch line answered NO_MCP_TOOL and never spawned the server; after, it returns the probe's payload. Co-authored-by: multica-agent <[email protected]> * fix(daemon): resolve CodeBuddy's real MCP config path (MUL-5846) Review follow-up. The previous commit pointed CodeBuddy at ~/.codebuddy.json, which is only the last fallback in its chain. Verified against the installed CodeBuddy CLI: - the config dir is $CODEBUDDY_CONFIG_DIR, defaulting to ~/.codebuddy - user-scope MCP is the FIRST existing of <configDir>/.mcp.json -> <configDir>/mcp.json -> ~/.codebuddy.json (a fallback chain, not a merge); `codebuddy mcp add --scope user` writes the first - those files are JSONC: comments and trailing commas parse fine - plugins live under <configDir>/plugins, not ~/.workbuddy This is not just a missing inventory row. When an agent has a managed mcp_config, codebuddy.go turns on --strict-mcp-config, so the merged document the daemon writes is the entire world CodeBuddy sees. Reading the wrong file meant "add one cloud MCP server" silently dropped every server the user had configured locally, which is exactly what mergeRuntimeAndAgentMcpConfig exists to prevent. stripJSONC rewrites comments to spaces and drops trailing commas without touching string literals, so a `//` or a comma inside a command argument survives. Also adds the session/new counterpart to TestKimiResumeIncludesMcpServers: Kimi takes MCP over ACP rather than as a CLI flag, so a bare `kimi acp` launch line is not evidence the servers were dropped. Regressions added: path precedence, CODEBUDDY_CONFIG_DIR, JSONC (including comment-like text inside strings), managed + local merge, same-name override, explicit empty set, JSON null passthrough. Co-authored-by: multica-agent <[email protected]> * fix(agent): let CodeBuddy merge its own MCP scopes natively (MUL-5846) Round-2 review follow-up. Pointing the daemon at CodeBuddy's user-scope file only fixed one of three scopes: --strict-mcp-config drops user, project AND local config, so an agent with a managed mcp_config still lost every server the user had configured locally. Measured against CodeBuddy 2.x with one real MCP server registered per scope, using process spawns as the oracle (the model's own tool list is not trustworthy here — it happily names servers that failed to connect): --mcp-config only ....... managed + user + local --mcp-config + strict ... managed only strict only ............. nothing at all So stop passing --strict-mcp-config entirely and let CodeBuddy build the union itself. A managed entry already wins a same-name collision natively (verified), which is exactly what mergeRuntimeAndAgentMcpConfig promises, so codebuddy also drops out of the daemon-side merge and becomes a passthrough like kimi. This removes the class of bugs the previous approach kept generating: the daemon no longer has to reproduce CodeBuddy's config-root resolution, scope precedence, or JSONC parsing on the launch path, and the review's second must-fix goes away with it — agent custom_env CODEBUDDY_CONFIG_DIR is now read by the CLI itself, so the daemon resolving a different directory can no longer desync the launch. Project-scope servers also keep their native approval gate instead of being rewritten into daemon-generated CLI config. (Measured: a project-scope server sits at "Needs approval" and never starts headless, with or without our flags.) codebuddyUserMcpConfigPath and the JSONC reader stay, now used only by the read-only runtime inventory the Agent > MCP tab renders. stripJSONC also tightened per the review: only the last comma before a closer is dropped, so `[1,,,]` stays invalid instead of being silently repaired, and output is now exactly the same length as input so parse-error offsets line up. Co-authored-by: multica-agent <[email protected]> * test(agent): pin kimi MCP delivery against the real CLI (MUL-5846) Users reported that MCP configured in Multica never reaches kimi, citing the bare `kimi acp` launch line as evidence. That line carries no MCP flags because the CLI has none — `kimi acp --help` lists only --login and --help — so kimi takes MCP over ACP session/new instead, and only an end-to-end run against the real binary can settle the claim. Drives this package's own kimi backend with a Multica-shaped agent.mcp_config. The oracle is the MCP server process itself: a stdio fixture that appends to a log when spawned and returns a sentinel from its one tool, so neither a hand-written ACP fixture nor the model's own account of its tools can fake a pass. Verified against kimi-code 0.33.0: server spawned, tools/list served, tools/call answered, sentinel in the task output. Behind the agentintegration tag and MULTICA_RUN_REAL_AGENT_SMOKE, so the default suites and CI are unaffected. Co-authored-by: multica-agent <[email protected]> * fix(daemon): reject unterminated JSONC block comments (MUL-5846) Review nits. stripJSONC blanked an unterminated `/*` all the way to EOF, so `{"mcpServers":{}} /* oops` became valid JSON plus whitespace, and the `/*/` near-miss let the opener's own `*` double as the closer's. Either could make the Agent > MCP tab list servers out of a file CodeBuddy itself rejects — verified: the CLI answers "No MCP servers configured" for both. It now consumes the opener before scanning and returns an error when no closer is found. Only the inventory is affected; the launch path stopped depending on this reader last commit. Also refreshes the launch-path comment, which still described the managed file as a "controlled set" gated by a strict-mode three-state check that buildCodebuddyArgs no longer performs. The flag is gone; --mcp-config now adds to CodeBuddy's own user/project/local scopes. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
Bohan Jiang
|
3eff25401d |
MUL-5785 fix(agent): return an empty output when a stream-json run has no final text (#6557)
A successful stream-json run with no deliverable text returned the sentence "The agent completed without a final response." as its output. That string is a signal, not a message to a user, and shipping it as content defeated every no-response decision the delivery layer already owns — all three branch on the output being EMPTY: an issue task skips the synthesized fallback comment, a direct chat writes a localized no_response outcome, and a Slack/Lark channel drops the turn instead of pushing anything out. Return the empty string instead, and classify each assistant event explicitly so the fallback answer only survives turns we positively understood: a tool-using turn clears it, an unparseable or unknown-shape event clears it (fail closed), and a genuinely silent thinking-only turn leaves it alone. The decision lives in one shared assistantTurn.resolveFallback across claude, codebuddy and qwen. Unreadable events are counted as unreadable_assistant_count so a CLI schema drift surfaces as a number rather than silently empty outputs. Status stays completed: an empty output is a legitimate terminal result for a turn that ended on a tool call, and re-running it would repeat side effects that already happened. Fixes #6462 |
||
|
|
a23b936b7b |
fix(agent): raise the agent stream scanner cap to 32 MiB and share it (MUL-5722) (#6383)
* fix(agent): raise the agent stream scanner cap to 32 MiB and share it (MUL-5722) Codex serializes an entire thread into the single `thread/resume` response line, so a long thread crossed the per-backend 10 MiB bufio.Scanner cap and failed the resume with "bufio.Scanner: token too long". Because that classifies as agent_error.process_failure, the session pointer survives and every later turn resumes the same oversized thread — the task is stuck for good. This is GH#4520 recurring: #4563 bounded the hang and the orphan process, but the cap follow-up it promised was never done. Raise the bound to 32 MiB and route every line-delimited agent transport through one newAgentStreamScanner constructor. The cap was copy-pasted into 17 sites and had already drifted (pi at 32 MiB, everything else at 10 MiB), which is how #4563's fix reached only one backend. The three tests that deliberately overflow the scanner now size their payloads from the constant, so raising the cap again cannot silently downgrade them into plain oversized-line passes. This is the containment layer only. A thread can still outgrow any fixed cap, and the recovery path is still missing: overflow is reported as a transport error, which blocks the thread/start fallback, and codex never sets ResumeRejected so the daemon's fresh-session retry does not fire either. Those remain open on MUL-5722. Co-authored-by: multica-agent <[email protected]> * fix(agent): route the QwenPaw ACP stream through the shared scanner (MUL-5722) QwenPaw (#5986) landed on main after this branch was cut, carrying its own bufio.Scanner with the old 10 MiB cap. Merging as-is would have left one backend still on the cliff this PR exists to remove — the same way #4563's fix originally reached only one backend. Rebased onto main and routed it through newAgentStreamScanner. No stray per-backend cap remains: the only bufio.NewScanner calls left in the package are the model-listing and codex session-file parsers, which are deliberately out of scope. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
280fa28e0d |
fix(agent): deny CodeBuddy's interactive plan-mode tools in headless runs (MUL-5383) (#6104)
CodeBuddy exempts AskUserQuestion and ExitPlanMode from permission-mode finalization, so `--permission-mode bypassPermissions` never auto-approves them. Once the model entered plan mode the session mode is Plan, not BypassPermissions, so ExitPlanMode also missed the daemon's bypass fast-path and went to the SDK permission bridge — which waits with no timeout for a confirmation the headless runtime cannot render. The task sat in-flight until the 2h tool watchdog, and users killed it by hand. Deny EnterPlanMode/ExitPlanMode alongside the AskUserQuestion we already deny. Each tool is passed as its own argv value because CodeBuddy matches disallowedTools entries exactly and does not split on commas. Also send `allowed: true` on control_response: CodeBuddy's SdkPermissionClient reads `allowed`, not Claude Code's `behavior`, so the daemon's "auto-approve" was being read as a denial. Fixes #6012 Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
a5a42846e6 |
fix(daemon): retry with a fresh session only when the resume was actually rejected (MUL-4966) (#5715)
* fix(daemon): gate fresh-session retry on tools executed, not session id (MUL-4966) Switching provider accounts leaves the stored session id pointing at a conversation the new account does not own. The daemon still passes it to --resume, the provider rejects it, and the task dies before doing any work. The existing fresh-session fallback was supposed to catch this but was gated on `result.SessionID == ""`, which is not a lifecycle fact: - Too narrow: a backend that echoes the requested id back when it rejects a resume keeps SessionID non-empty, so the fallback never fired — the reported bug. - Too broad: a provider 401 before the first stream message also leaves SessionID empty, so an unrecoverable auth failure burned a second full run. Gate on `tools == 0` instead. That states the property that actually makes a retry safe — the agent executed no tool, so it mutated nothing, so re-running cannot double-post a comment (comment creation has no idempotency key and a duplicate re-fires its @mention triggers), reopen a PR, or re-plan on top of its own half-finished work in the reused workdir. Auth failures are excluded, mirroring retryableReasons in service/task.go. The predicate is extracted to shouldRetryWithFreshSession so the tests exercise production logic; both existing fallback tests re-implemented the condition inline and would not have caught a regression in it. Co-authored-by: multica-agent <[email protected]> * fix(daemon): gate fresh-session retry on a positive resume-rejected signal (MUL-4966) Review of the previous commit was right: `tools == 0` plus "not an auth error" answers whether re-running is *safe*, not whether a new session can *fix* the failure. Those are orthogonal, and answering the second by exclusion inverts the burden of proof — the failures a fresh session cures are a small enumerable set, while the ones it cannot are open-ended. Concretely, the previous predicate fresh-retried on provider_network, 429/529, quota, 5xx and unclassified startup failures. provider_network is the sharpest conflict: internal/service/task.go marks it resume-safe (MUL-4910) specifically so the platform retry inherits the session and continues the truncated conversation. Resetting the session first made that contract unsatisfiable, silently discarding conversation context on a transient blip — and rate limits got an immediate no-backoff re-run. Replace the inference with positive evidence: agent.Result gains an explicit ResumeRejected field, set only when a backend has proof the resume itself was refused. claude/codebuddy/qwen derive it from resumeWasRejected, which promotes the predicate resolveSessionID was already computing and encoding as the side effect of blanking SessionID — using an empty string to carry that meaning is what made the original bug possible. SessionID keeps being dropped for a rejected resume (a dead pointer must not be persisted), but it is no longer the signal the daemon reads to decide *why* a run failed. The six ACP backends that recover from "session not found" set the flag at the same points they already clear the id, so their existing recovery is not caught by the narrower gate. codex needs nothing: thread/resume already falls back to thread/start in-process, and deliberately does not on transport errors. Matching now includes the account-switch guardrail reported in #5704 (Claude Code 2.1.207, zh-CN): "400 此 session 已绑定另外的ai账号，请执行 /new 开启新 session". The en-US wording of the same guardrail has not been captured yet, so those variants are marked inferred in the source; a miss degrades to a terminal failure carrying the provider's raw text rather than a mis-routed run. Tests: backend-level fixtures drive ResumeRejected from real stream-json for both the account-binding 400 and a network drop, and the predicate now covers network/rate-limit/quota/5xx/auth/unclassified as explicit non-retries. Co-authored-by: multica-agent <[email protected]> * fix(daemon): restore fresh-session recovery for backends with no rejection signal (MUL-4966) Final review caught qwen regressing: its verified rejection string ("No saved session found with ID ...", already captured in testdata/qwen-code-0.20.0-resume-not-found.stderr.txt) was not in the phrase list, and qwen reports no session id on that path, so the new inclusion gate turned a working auto-recovery into a terminal failure. Auditing the other 17 resume-capable backends showed qwen was not alone. antigravity, copilot, cursor, deveco and opencode all recovered from a refused resume purely by reporting an empty SessionID, and none of them has any rejection detection to convert into ResumeRejected — copilot's own comment documents the hole (session.error before session.start), and antigravity's helper returns "" when "the CLI exited before dispatching". Making ResumeRejected the sole gate silently removed recovery from all five. Fixing that by guessing rejection phrases for five more CLIs is the wrong trade: no real output has been captured for any of them, and a false positive discards a recoverable session pointer. So the gate is now two tiers. Positive evidence (ResumeRejected) decides on its own where a backend can produce it. Where none is available, an empty SessionID still gates the retry — it proves no session was established, which is exactly what the pre-change behaviour relied on — minus the classes a fresh session provably cannot cure (network, rate limit, quota, provider 5xx, auth). That keeps the resume-safe contract in internal/service/task.go intact while restoring what these five backends had. Also renames claudeResumeRejectedPhrases to resumeRejectedPhrases: it is matched by claude, codebuddy and qwen, so a qwen-only string living under a claude-prefixed name would be actively misleading. Tests: qwen's existing missing-resume fixture now asserts ResumeRejected (verified failing without the phrase), and the predicate covers the no-signal tiers — retry when nothing was established, no retry once a session exists or the failure classifies as uncurable. Co-authored-by: multica-agent <[email protected]> * fix(daemon): scope the no-signal fallback to backends that cannot detect rejections (MUL-4966) Final review caught the compatibility path applying to every backend, not just the five it was justified for. shouldRetryWithFreshSession only saw (Result, priorSessionID, tools), so a false ResumeRejected could not be told apart from a backend that has no way to answer — and claude/codebuddy/qwen/ACP startup failures with no session id still fell through to the exclusion branch. That contradicted both the stated intent and the function's own doc comment ("where a backend can produce it, it is the whole answer"). Make the capability explicit. agent.ResumeRejectionUndetectable names the five backends that scrape SessionID out of stream output and have no rejection detection at all; the daemon takes provider and consults it, so a capable backend reporting false is now taken at its word. Membership is opt-in, so a new backend fails closed instead of silently inheriting a guess-based retry. Also completes the exclusion set: missing config, unavailable model, missing executable, unsupported runtime version and (defensively) agent timeout all have defined non-session remedies and were reaching `default: true`. What is left through stays narrow — unknown, process failure, unparseable output, context overflow — because a real rejection from these five most likely surfaces as a non-zero exit or unparseable output, none of them reporting one explicitly. Tests: one identical result asserted across all five undetectable backends (retries), twelve capable ones (no retry), and an unregistered provider (fails closed), plus table cases for each newly excluded reason. Classifier inputs were verified to map to the intended reasons rather than passing by accident. Also updates the ResumeRejected doc comment, which still said the daemon gates on it alone. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
cf1fc64671 |
MUL-4824: fail closed on incomplete Claude-style result streams (#5522)
* fix(agent): fail closed on incomplete stream results Co-authored-by: multica-agent <[email protected]> * fix(agent): tighten empty result fallback Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Eve <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
083e045ec6 |
MUL-4103: harden Windows browser MCP config (#4976)
* fix: harden Windows browser MCP config Co-authored-by: multica-agent <[email protected]> * fix: address browser mcp review nits Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Eve <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
Liu Guanzhongandkrislliu
|
4594c776e1 |
feat(agent): add CodeBuddy as first-class CLI backend (#3186)
* feat(agent): add codebuddyBackend struct and buildCodebuddyArgs Introduces the codebuddy agent backend skeleton with args builder that mirrors claudeBackend's protocol flags (stream-json, bypass permissions, blocked args filtering) for the codebuddy CLI fork. * feat(agent): implement codebuddyBackend.Execute with stream-json parsing * feat(agent): wire codebuddy into New() factory and launchHeaders * feat(agent): add codebuddy dynamic model discovery from --help * feat(agent): add codebuddy thinking/effort discovery and providerThinkingEnums * feat(daemon): add codebuddy CLI probe, env vars, and args support * fix(agent): use len(models)==0 for default model instead of loop index * fix(agent): increase codebuddy --help timeout to 35s for slow CLI startup * fix(agent): address codebuddy PR review feedback - Wire codebuddy into execenv: reuse claude's CLAUDE.md, .claude/skills, and ~/.claude/skills paths since CodeBuddy is a Claude Code fork - Replace hardcoded 20-min timeout with runContext for zero-timeout = no-deadline semantics matching all other backends - Restore runContext regression tests lost in rebase merge - Mirror claude.go execution model: concurrent stdin write to prevent pipe deadlock, sync.Once for stdin closure, keep stdin open for control_request auto-approval mid-run - Add control_request handling with auto-approve behavior - Add RequestID/Request fields to codebuddySDKMessage - Add codebuddy to metrics knownRuntimeProviders - Add codebuddy to provider-logo.tsx (reuses ClaudeLogo) - Consolidate --help discovery: shared codebuddyHelpOutput cache eliminates duplicate cold-start invocations --------- Co-authored-by: krislliu <[email protected]> |