Files
multica/server/pkg/agent/traecli.go
T
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]>
2026-08-17 15:11:36 +08:00

445 lines
14 KiB
Go

package agent
import (
"context"
"fmt"
"io"
"os/exec"
"strings"
"sync"
"sync/atomic"
"time"
)
// traecliBlockedArgs are flags hardcoded by the daemon that must not be
// overridden by user-configured custom_args. `acp` and `serve` are the
// protocol subcommand/action; `-y`/`--yolo` is daemon-owned so headless ACP
// always runs in bypass-permissions mode (the official traecli gates non-read
// tools behind a permission prompt otherwise — see
// https://docs.trae.cn/cli_permission-mode). `--print`/`-p` and
// `--output-format` would switch the binary out of ACP into print mode and
// break the daemon↔traecli transport, and `--permission-mode` is owned by the
// daemon via --yolo.
var traecliBlockedArgs = map[string]blockedArgMode{
"acp": blockedStandalone,
"serve": blockedStandalone,
"-y": blockedStandalone,
"--yolo": blockedStandalone,
"-p": blockedStandalone,
"--print": blockedStandalone,
"--output-format": blockedWithValue,
"--permission-mode": blockedWithValue,
}
// traecliBackend implements Backend by spawning `traecli acp serve --yolo` and
// communicating via the standard ACP (Agent Client Protocol) JSON-RPC 2.0
// transport over stdin/stdout.
//
// This targets ByteDance's official TRAE CLI (the `traecli` binary documented
// at https://docs.trae.cn/cli — NOT the open-source bytedance/trae-agent
// `trae-cli`, which has no ACP transport). traecli is ACP-native: it advertises
// `loadSession: true` and `mcpCapabilities: {http, sse}` from `initialize`,
// returns its model catalog from `session/new`, and supports `session/load`
// (resume), `session/set_model`, and `session/prompt`. That lets the existing
// Hermes/Kimi/Kiro/Qoder ACP client (hermesClient) drive it with only
// provider-specific launch args and tool-name normalization.
//
// The `initialize` capabilities above were captured from the real traecli
// v0.120.42 binary; the streaming gate mirrors the other ACP backends so
// history replay flushed during session setup does not corrupt the streamed
// output.
type traecliBackend struct {
cfg Config
}
var traecliReaderDrainGrace = 2 * time.Second
// traecliMessageStream serializes sends and the final close so a late stdout
// reader (traecli may keep the process and its pipes open briefly after
// session/prompt returns) cannot send on a closed channel. Mirrors qoder.
type traecliMessageStream struct {
ch chan Message
mu sync.Mutex
closed bool
}
func newTraecliMessageStream(size int) *traecliMessageStream {
return &traecliMessageStream{ch: make(chan Message, size)}
}
func (s *traecliMessageStream) send(msg Message) {
s.mu.Lock()
defer s.mu.Unlock()
if s.closed {
return
}
trySend(s.ch, msg)
}
func (s *traecliMessageStream) close() {
s.mu.Lock()
defer s.mu.Unlock()
if s.closed {
return
}
s.closed = true
close(s.ch)
}
func (b *traecliBackend) Execute(ctx context.Context, prompt string, opts ExecOptions) (*Session, error) {
execPath := b.cfg.ExecutablePath
if execPath == "" {
execPath = "traecli"
}
if _, err := exec.LookPath(execPath); err != nil {
return nil, fmt.Errorf("traecli executable not found at %q: %w", execPath, err)
}
// Translate the agent's mcp_config (Claude-style object of objects) into
// the array shape ACP session/new and session/load expect. Fail closed on
// malformed JSON so the launch surfaces the real error instead of silently
// dropping every MCP server.
mcpServers, err := buildACPMcpServers(opts.McpConfig, b.cfg.Logger)
if err != nil {
return nil, fmt.Errorf("traecli: invalid mcp_config: %w", err)
}
timeout := opts.Timeout
runCtx, cancel := runContext(ctx, timeout)
traecliArgs := append(
[]string{"acp", "serve", "--yolo"},
filterCustomArgs(opts.CustomArgs, traecliBlockedArgs, b.cfg.Logger)...,
)
cmd := b.cfg.commandAt(execPath).exec(runCtx, traecliArgs...)
hideAgentWindow(cmd)
b.cfg.Logger.Info("agent command", "exec", execPath, "args", traecliArgs)
if opts.Cwd != "" {
cmd.Dir = opts.Cwd
}
cmd.Env = buildEnv(b.cfg.Env)
stdout, err := cmd.StdoutPipe()
if err != nil {
cancel()
return nil, fmt.Errorf("traecli stdout pipe: %w", err)
}
stdin, err := cmd.StdinPipe()
if err != nil {
cancel()
return nil, fmt.Errorf("traecli stdin pipe: %w", err)
}
// StderrPipe + an explicit copier give us a join point (`stderrDone`) that
// fires before the failure-promotion decision; see the matching comment in
// hermes.go for why the io.MultiWriter form races with stopReason=end_turn
// under load.
providerErr := newACPProviderErrorSniffer("traecli")
stderr, err := cmd.StderrPipe()
if err != nil {
cancel()
return nil, fmt.Errorf("traecli stderr pipe: %w", err)
}
if err := cmd.Start(); err != nil {
cancel()
return nil, fmt.Errorf("start traecli: %w", err)
}
stderrSink := io.MultiWriter(newLogWriter(b.cfg.Logger, "[traecli:stderr] "), providerErr)
stderrDone := make(chan struct{})
go func() {
defer close(stderrDone)
_, _ = io.Copy(stderrSink, stderr)
}()
b.cfg.Logger.Info("traecli acp started", "pid", cmd.Process.Pid, "cwd", opts.Cwd)
msgStream := newTraecliMessageStream(256)
resCh := make(chan Result, 1)
// Traecli streams interim narration and the final answer as the same
// agent_message_chunk type; the tracker keeps only the post-tool-call block
// for Result.Output while retaining the full text for error detection.
var deliverable acpDeliverableTracker
var streamingCurrentTurn atomic.Bool
promptDone := make(chan hermesPromptResult, 1)
activity := make(chan struct{}, 1)
c := &hermesClient{
cfg: b.cfg,
stdin: stdin,
pending: make(map[int]*pendingRPC),
pendingTools: make(map[string]*pendingToolCall),
acceptNotification: func(string) bool {
return streamingCurrentTurn.Load()
},
onActivity: func() {
select {
case activity <- struct{}{}:
default:
}
},
onMessage: func(msg Message) {
if !streamingCurrentTurn.Load() {
return
}
if msg.Type == MessageToolUse {
msg.Tool = kimiToolNameFromTitle(msg.Tool)
}
deliverable.observe(msg)
msgStream.send(msg)
},
onPromptDone: func(result hermesPromptResult) {
if !streamingCurrentTurn.Load() {
return
}
select {
case promptDone <- result:
default:
}
},
}
readerDone := make(chan struct{})
go func() {
defer close(readerDone)
scanner := newAgentStreamScanner(stdout)
for scanner.Scan() {
line := strings.TrimSpace(scanner.Text())
if line == "" {
continue
}
c.handleLine(line)
}
c.closeAllPending(fmt.Errorf("traecli process exited"))
}()
go func() {
defer cancel()
defer msgStream.close()
defer close(resCh)
defer func() {
stdin.Close()
_ = cmd.Wait()
}()
startTime := time.Now()
finalStatus := "completed"
var finalError string
var sessionID string
// Set when the ACP runtime refuses the session we asked to
// resume. Only that is curable by starting a fresh session, so
// handshake/network failures below must leave it false.
var resumeRejected bool
effectiveModel := strings.TrimSpace(opts.Model)
initResult, err := c.request(runCtx, "initialize", map[string]any{
"protocolVersion": 1,
"clientInfo": map[string]any{
"name": "multica-agent-sdk",
"version": "0.2.0",
},
"clientCapabilities": map[string]any{},
})
if err != nil {
finalStatus = "failed"
finalError = fmt.Sprintf("traecli initialize failed: %v", err)
resCh <- Result{Status: finalStatus, Error: finalError, DurationMs: time.Since(startTime).Milliseconds()}
return
}
// Drop MCP entries whose remote transport the runtime didn't advertise
// (traecli advertises mcpCapabilities {http, sse}). See hermes.go for
// why sending an unsupported transport tanks the whole session/new.
mcpServers = filterACPMcpServersByCapability(mcpServers, extractACPMcpCapabilities(initResult), "traecli", b.cfg)
cwd := opts.Cwd
if cwd == "" {
cwd = "."
}
if opts.ResumeSessionID != "" {
// traecli advertises loadSession:true, so resume goes through the
// standard ACP session/load (same as Kiro).
result, err := c.request(runCtx, "session/load", map[string]any{
"cwd": cwd,
"sessionId": opts.ResumeSessionID,
"mcpServers": mcpServers,
})
if err != nil {
finalStatus = "failed"
finalError = fmt.Sprintf("traecli session/load failed: %v", err)
resCh <- Result{Status: finalStatus, Error: finalError, DurationMs: time.Since(startTime).Milliseconds()}
return
}
var changed bool
sessionID, changed = resolveResumedSessionID(opts.ResumeSessionID, result)
if changed {
b.cfg.Logger.Warn("agent returned a different session id on resume — original was likely lost; continuing with the new id",
"backend", "traecli",
"requested", opts.ResumeSessionID,
"actual", sessionID,
)
}
if effectiveModel == "" {
effectiveModel = extractACPCurrentModelID(result)
}
} else {
result, err := c.request(runCtx, "session/new", map[string]any{
"cwd": cwd,
"mcpServers": mcpServers,
})
if err != nil {
finalStatus = "failed"
finalError = fmt.Sprintf("traecli session/new failed: %v", err)
resCh <- Result{Status: finalStatus, Error: finalError, DurationMs: time.Since(startTime).Milliseconds()}
return
}
sessionID = extractACPSessionID(result)
if sessionID == "" {
finalStatus = "failed"
finalError = "traecli session/new returned no session ID"
resCh <- Result{Status: finalStatus, Error: finalError, DurationMs: time.Since(startTime).Milliseconds()}
return
}
if effectiveModel == "" {
effectiveModel = extractACPCurrentModelID(result)
}
}
c.sessionID = sessionID
b.cfg.Logger.Info("traecli session created", "session_id", sessionID)
if opts.Model != "" {
if _, err := c.request(runCtx, "session/set_model", map[string]any{
"sessionId": sessionID,
"modelId": opts.Model,
}); err != nil {
b.cfg.Logger.Warn("traecli set_session_model failed", "error", err, "requested_model", opts.Model)
finalStatus = "failed"
finalError = fmt.Sprintf("traecli could not switch to model %q: %v", opts.Model, err)
if opts.ResumeSessionID != "" && isACPSessionNotFound(err) {
b.cfg.Logger.Warn("resumed session not found at set_model time; clearing session id so the daemon retries fresh",
"backend", "traecli",
"session_id", sessionID,
)
sessionID = ""
resumeRejected = true
}
resCh <- Result{
Status: finalStatus,
Error: finalError,
DurationMs: time.Since(startTime).Milliseconds(),
SessionID: sessionID,
ResumeRejected: resumeRejected,
}
return
}
b.cfg.Logger.Info("traecli session model set", "model", opts.Model)
}
userText := prompt
if opts.SystemPrompt != "" {
userText = opts.SystemPrompt + "\n\n---\n\n" + prompt
}
streamingCurrentTurn.Store(true)
_, err = c.request(runCtx, "session/prompt", map[string]any{
"sessionId": sessionID,
"prompt": []map[string]any{
{"type": "text", "text": userText},
},
})
if err != nil {
if runCtx.Err() == context.DeadlineExceeded {
finalStatus = "timeout"
finalError = fmt.Sprintf("traecli timed out after %s", timeout)
} else if runCtx.Err() == context.Canceled {
finalStatus = "aborted"
finalError = "execution cancelled"
} else {
finalStatus = "failed"
finalError = fmt.Sprintf("traecli session/prompt failed: %v", err)
if opts.ResumeSessionID != "" && isACPSessionNotFound(err) {
b.cfg.Logger.Warn("resumed session not found at prompt time; clearing session id so the daemon retries fresh",
"backend", "traecli",
"session_id", sessionID,
)
sessionID = ""
resumeRejected = true
}
}
} else {
select {
case pr := <-promptDone:
if pr.stopReason == "cancelled" {
finalStatus = "aborted"
finalError = "traecli cancelled the prompt"
}
c.mergeUsage(pr.usage)
default:
}
waitForACPNotificationQuiescence(runCtx, activity, readerDone, acpNotificationQuietTime, traecliReaderDrainGrace)
}
duration := time.Since(startTime)
b.cfg.Logger.Info("traecli finished", "pid", cmd.Process.Pid, "status", finalStatus, "duration", duration.Round(time.Millisecond).String())
stdin.Close()
cancel()
// traecli ACP may keep the process — and the stdout/stderr pipes — open
// briefly after session/prompt returns. The prompt response is already
// terminal, so bound the drain: wait for the stdout reader and the
// stderr copier, but no longer than the grace window (CommandContext
// cancellation tears the process down). Draining stderr is what makes
// the provider-error promotion below see a terminal marker.
drainCtx, drainCancel := context.WithTimeout(context.Background(), traecliReaderDrainGrace)
select {
case <-readerDone:
case <-drainCtx.Done():
}
select {
case <-stderrDone:
case <-drainCtx.Done():
}
drainCancel()
// Flip the gate before the defer closes msgStream; a late reader that
// already passed the gate is serialized by traecliMessageStream so the
// late send is dropped instead of panicking.
streamingCurrentTurn.Store(false)
finalOutput, providerErrorOutput := deliverable.result()
// Promote completed→failed when stderr or the agent text stream show a
// terminal upstream-LLM failure (HTTP 4xx / rate-limit / expired token).
// Mirrors hermes/kimi/kiro/qoder, and reads the full text stream so a
// give-up turn that lands before a tool call stays visible.
finalStatus, finalError = promoteACPResultOnProviderError(finalStatus, finalError, providerErrorOutput, providerErr)
u := c.accumulatedUsage()
var usageMap map[string]TokenUsage
if acpUsagePresent(u) {
model := effectiveModel
if model == "" {
model = "unknown"
}
usageMap = map[string]TokenUsage{model: u}
}
resCh <- Result{
Status: finalStatus,
Output: finalOutput,
Error: finalError,
DurationMs: duration.Milliseconds(),
SessionID: sessionID,
ResumeRejected: resumeRejected,
Usage: usageMap,
}
}()
return &Session{Messages: msgStream.ch, Result: resCh}, nil
}