mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-09 18:06:45 +08:00
* fix(daemon): own the openclaw CLI process tree so its deadline is enforceable (MUL-5467) openclawCLITimeout has carried a "Known gap (deliberately not fixed here)" note since #6084: the 5s deadline could not actually bound an `openclaw config ...` invocation. This closes it, and fixes a second OpenClaw misbehaviour found alongside it. ## Bug 1 — the deadline was decorative exec.CommandContext kills only the direct child. cmd.Output() blocks in Wait() until the output pipes os/exec manages reach EOF, and cancelling the context does not unblock that io.Copy. So when openclaw leaves a descendant holding stdout — its `openclaw-config` helper, or cmd.exe → node on Windows — the call runs for the *descendant's* lifetime. #6084 measured this on linux/dash (a shim whose backgrounded child slept 6s took 6.01s against a 150ms deadline), tried a cmd.WaitDelay backstop, and reverted it on review because it bounds the call by leaving the descendant running — trading a hang for a process leak that nothing on Unix reaps. The note left behind names the real fix: process-tree ownership. Three call sites had the shape: - execenv.execOpenclawCLI on the task's critical path (Prepare → prepareOpenclawConfig) - agent.detectCLIVersion runs for every provider inside the daemon's blocking registration preflight - agent.discoverOpenclawAgents four invocations per call ## Bug 2 — the answer was already on stdout Measured on an OpenClaw host (openclaw 2026.5.27): openclaw --version 258ms exits cleanly openclaw config file 60s correct path printed, then never exits openclaw agents list 60s correct list printed, then never exits Waiting for exit turns two working commands into a task-fatal error: agent_error.process_failure (prepare execution environment: execenv: prepare openclaw config: locate openclaw active config: openclaw config file: context deadline exceeded (process: signal: killed)) The contract of these commands is "print a value". Once the value has arrived and nothing more is coming, whether the process tidies itself up is not the caller's business, and it should not fail a chat task. ## The fix Two helpers in server/pkg/agent, both built on the process-group primitives codex/claude/opencode already use — no new platform-specific functions: - RunCollect: hands os/exec an *os.File instead of a buffer, so os/exec starts no copy goroutine and Wait returns the instant the direct child exits. Then signals the child's process group, which reaps the helper and releases the last write end, so our own readers see EOF. Unlike WaitDelay this reports the CLI's real exit status, which matters because a failed `--version` probe skips runtime registration entirely, and because openclawShimDiagnostic type-switches on *exec.ExitError. - RunCollectQuiet: returns when the process exits, or when stdout has produced output that then stayed idle for 400ms — whichever comes first. Reports quiet=true on the idle path so callers can log the CLI's failure to exit without failing on it. The idle shortcut deliberately cannot mask a silent CLI: with no output there is nothing to salvage, so the deadline still governs and the call still fails. A non-zero exit still fails too, with stderr intact. execOpenclawCLI keeps its shim diagnostics and its context-attribution order byte-for-byte; only the exec mechanism changes. discoverOpenclawAgents keeps its existing 30s function-level budget rather than gaining a per-attempt one. Windows: there is no process group to signal, so calls are bounded (by the drain grace) but a descendant can still outlive one. Owning descendants there needs a Job Object and is not attempted here. ## Verification - Both packages green with -race; 6 consecutive runs, no flakes. - All 51 existing Openclaw* tests in execenv pass, including TestExecOpenclawCLITimeoutIsNotMisdiagnosedAsMissingInterpreter and TestExecOpenclawCLICancellationIsWrapped — their shims produce no stdout, so the salvage path is not reachable and the errors.Is(err, context.DeadlineExceeded) contract is preserved. - 13 new tests. Both mechanisms are mutation-verified: * RunCollect reverted to buffers with no group reap → the guard test hangs and the dump is the production stack: cmd.Wait → awaitGoroutines blocked, writerDescriptor parked in io.Copy. * RunCollectQuiet's idle path disabled → both the unit test and the execenv end-to-end test fail after a full 60s. * fix(daemon): address review on the openclaw CLI collector (MUL-5467) Three blocking items from review of #6275. All three were reproduced before being fixed, and the first one was a genuine correctness hole rather than a style objection. ## 1. The deadline branch salvaged partial answers RunCollectQuiet returned success from the ctx.Done() branch whenever stdout was non-empty, without requiring the idle grace or any check that the answer was finished. A CLI still streaming when the deadline arrived therefore had its truncated output reported as a completed response. Reproduced with a stub emitting a JSON document forever against a 250ms deadline: 9 runs in 10 came back as success carrying an unparseable fragment. The early return now needs two independent conditions, neither sufficient alone: - an OutputComplete rule supplied by the caller accepts the buffer, and - the buffer has since been idle for idleGrace. Reaching the deadline is never success. The captured bytes are still returned alongside the error so a caller with its own rule can inspect them, but the runner no longer decides on the caller's behalf. A nil rule disables the early return entirely, which is the conservative default for a command whose output shape has no completeness rule. The rule cannot be inferred inside the runner, which is the point: only the caller knows whether `{"agents":[{"id":"a"},` is truncated or legitimate. - `--json` subcommands -> agent.JSONOutputComplete (whole buffer parses). - `openclaw config file` -> openclawConfigPathComplete (last non-empty line looks like a path). Deliberately stricter than openclawParseActiveConfigPath, which resolves a relative line through filepath.Abs and would therefore accept a Doctor warning border as an answer. That matters because OpenClaw prints Doctor UI *before* the path (MUL-3136), so an early return on idle output alone hands back the banner as the config path — and nothing downstream catches it. - `agents list` text fallback -> RunCollect (wait for exit). Decorated text has no completeness rule and a silently short agent catalog is worse than a slow one; this path still gains the reaping and the bounded return. ## 2. quietWriter published bytes before their timestamp The buffer write and the last-write timestamp were updated in separate critical sections, so a reader could observe new bytes carrying a stale timestamp and conclude the stream had gone quiet at the moment it was producing — truncating an answer mid-write. Both now move in one critical section (outputBuffer). ## 3. The drain-timeout path raced the readers, and could park a goroutine RunCollect read its buffers after the drain grace expired while io.Copy could still be appending to them; the deferred Close on the read ends ran only after the return values had been evaluated. Separately, RunCollectQuiet handed os/exec io.Writers, so its cmd.Wait goroutine could sit forever on a descendant holding the pipe — on Windows nothing released it, since there is no process group to signal. Both are now structural rather than patched. A single collector owns the pipes for both helpers (the *os.File ownership RunCollect already had), and finish() guarantees that when it returns, every goroutine startCollector spawned has been joined and nothing can still append to the buffers: it reaps the tree, waits bounded for Wait, and if the readers still have not seen EOF it closes the read ends outright — which makes the in-flight Read return — and only then joins them. ## Verification - Both packages pass with -race under CI's flags (-p 2 -parallel 2). - All pre-existing Openclaw* tests still pass, including the timeout attribution and shim diagnostic ones. - New regression tests, all three named in the review: continuous output until the deadline (must not be success), a prompt followed by a delayed body (must not return the prompt), and a late non-zero exit within the grace (must be reported as the failure it is). Plus the completeness rules per subcommand, the banner-then-path case end to end through execOpenclawCLI, goroutine collection on both helpers, and the two outputBuffer contracts. - Windows contract coverage added (run_collect_windows_test.go) with a scoped step in the existing windows-execenv CI job, since the close-the-read-ends path is unreachable on Unix where the group kill releases the pipe. - Mutation-verified: restoring the unconditional salvage fails both the unit and the execenv regression; dropping the completeness rule from execOpenclawCLI makes the Doctor banner come back as the config path. * test(daemon): make the openclaw config-path fixtures hermetic The two execenv fixtures printed the literal path the host had printed, `/root/.openclaw/openclaw.json`. openclawParseActiveConfigPath stats what it parses and tolerates only os.ErrNotExist, so that path resolves differently by environment: ENOENT on a developer machine, where /root does not exist, but EACCES on a Linux CI runner, where it exists and is unreadable. The second case is a hard error, so TestExecOpenclawCLIWaitsForThePathAfterDoctorBanner failed in CI for a reason unrelated to the boundary it covers. Both fixtures now print a real file inside t.TempDir(), which stats cleanly everywhere. The second test only inspects raw stdout today, but a hard-coded path there would be a trap for whoever next adds a parse to it. The literal path stays in the completeness-rule table, which calls the predicate directly and never stats, so it keeps documenting the shape the CLI actually emits. * fix(agent): own one-shot CLI process trees on Windows * fix(daemon): support current OpenClaw config discovery * Address review, and narrow this branch to the process-tree fix Blocking items from the review: 1. The deadline branch no longer salvages. It returns an error unconditionally, and callers supply a per-subcommand completeness rule (openclawOutputComplete) that decides whether the captured bytes are a finished answer. Regression coverage for the three shapes named in the review: output continuing right up to the deadline, a banner ahead of the real answer, and a late non-zero exit. finish() now reports, rather than being wired to a field that was never assigned. Two cases do not converge and both cost the caller something it would otherwise never learn: a process still outstanding in Wait after the tree kill, and a descendant still holding the pipe when the read ends had to be force-closed, which can truncate trailing output. collectorError joins that with the command's own error, so nothing is displaced. 2. outputBuffer publishes bytes and the write timestamp inside one critical section, so a poller cannot see new bytes with a stale timestamp and conclude the stream went quiet while it was in fact producing. 3. The drain-timeout path closes the read ends and waits for both absorb loops before any snapshot is taken, so no io.Copy can be appending to a buffer the caller is about to read. Windows contract coverage runs as its own CI job. The goroutine guarantee is stated at its real strength instead of "no goroutine outlives the call, on any platform". Convergence holds whenever the tree kill takes. When it does not, one goroutine can stay parked in cmd.Wait on a process nothing can terminate; that residue is not removable here, so it is reported instead of asserted away. On Windows the same case extends the call rather than leaking, because an anonymous pipe is not pollable and closing the read end waits for an in-flight Read to release it. Also from review: - A JSON error envelope is valid JSON, so JSONOutputComplete accepts one and a CLI that prints `{"error": ...}` and then lingers past the idle grace hands back err == nil with the envelope in stdout — where the exit-status path never sees it. All three JSON readers now check for that explicitly. It matters most for `config get agents.list --json`, where the envelope is how a 2026.6+ host says the key moved to the registry: decoding it as data turned a graceful fallback into a failed preparation. For the object-target read the failure is silent, since the envelope would be sanitized and written into the task's snapshot as if it were the user's config. - RunCollectCmd selects on ctx.Done(). It takes a command the caller built, so enforcement must not depend on that caller having used exec.CommandContext; the ordering matches RunCollectQuiet so the two entry points cannot report different error shapes for the same situation. startCollector drops its unused ctx parameter rather than implying it enforces one. - checkOpenclawVersion keeps stderr in the parse. It replaced CombinedOutput(), and a build that prints its version there would otherwise fail the gate on empty output, which skips runtime registration entirely. extractVersionLine is bypassed on purpose: parseOpenclawVersion already scans the whole text, while picking the first line containing a semver lets unrelated stderr noise answer for it. The probe is bounded because the caller's ctx is the task's, and ExplainExecError is applied here too so an ENOEXEC shim is still named (MUL-6164). - TestRunCollectQuietWithoutCompletenessRuleWaitsForExit synchronizes on a marker the CLI writes after its answer instead of a deadline. Its two assertions want opposite budgets, and the reviewer reproduced the resulting flake 3/3 under load. Waiting for the marker also proves more than a timeout could: that the runner was still blocked at a moment when the answer was already fully available. Scope: the MCP work that shared this branch is reverted to main's behaviour and will come back on its own, because it needs a discussion this one does not. It changed which command supplies the resolved MCP settings, restructured the include chain, and moved where the user's config bytes live — none of which the deadline fix depends on. Preparation for a managed-MCP agent behaves exactly as it does on main here. $OPENCLAW_HOME path expansion stays: openclawConfigPathComplete calls openclawHomeRest directly, so the completeness rule for `config file` cannot tell whether that shape is a finished answer without it. * fix(agent): converge the collector's tree kill and stop failing calls on cleanup (MUL-5467) Review follow-up on #6275. All three findings share one root cause: whether the caller got its answer depended on whether cleanup converged, and cleanup convergence depended on a single kill(-pgid) landing. - Retry the tree kill across collectReapWindow. One pass loses a descendant whose fork completes between the kill's enumeration of the process group and the signal's delivery — measured 3 misses in 10 runs of the forking stub, each leaving a `sleep 300` reparented to init and stalling the call until the drain budget expired. The window is deliberately short so it does not widen the pid-reuse window reapProcessTree already documents. - Replace the 5s EOF budget with a 400ms settle. Once the direct child has exited, at most one pipe buffer of its output can still be unread — a child that had written more would have blocked in write() and could not have exited — so a short cap cannot truncate a well-behaved CLI's answer. What it cuts short is a surviving descendant's trailing output, which is not the answer. - Stop reporting cleanup residue as a call error. finish() now logs and returns nothing: a probe whose output arrived and whose helper the kernel would not kill is exactly the case #6084's cmd.WaitDelay got wrong, and a failed version probe skips runtime registration entirely. This drops finishErr, collectorError and the drainForced/waitPending split, which the four call sites were interpreting two different ways by accident. - Fix the reaping tests. They asserted on `$$` captured inside `( )`, which POSIX keeps at the invoking shell's pid, so they named the direct child that cmd.Wait reaps regardless — all of them passed with reapProcessTree removed, while the run leaked 7 orphans. Record `$!` instead, and add TestRunCollectRetriesTheTreeKill, which injects a missed pass because the race lives inside one syscall and cannot be forced from a stub. Verified: collector group 0/10 failures (was 3/10) with 0 orphans, 0/5 under -race, full ./pkg/agent -race green, execenv Openclaw -race green, and both mutations — retry removed, reaping disabled — fail the tests that now cover them, on their own assertions rather than on a cleanup error. * refactor: move the `$OPENCLAW_HOME` expansion out of this branch It is not process-tree work. `openclaw config file` printing `$OPENCLAW_HOME\...` instead of the expanded path is the same class of bug as #6630 — a path the daemon cannot stat, reported as a fresh install, wrapper without the user's `$include` — and #6643 shipped the tilde half of it as its own change. This half belongs there too, not folded into a branch about owning the CLI's process tree. Removed here: openclawHomeRest, its branch in expandOpenclawPath, the unit test, and the completeness-rule row that depended on it. Neither PR now depends on the other. openclawConfigPathComplete therefore does not recognize the variable shape, which is correct in isolation: `main` cannot expand it either, so a rule that accepted the line would hand the caller a path the parser then fails on. Such a host keeps `main`'s behaviour — no rule match means RunCollectQuiet waits for exit — so it loses the hang tolerance and nothing else. The comment on that function says so and names the change that should close it, because a gap nobody wrote down is one a rebase re-opens silently. * fix(daemon): teach the config-path rule the `$OPENCLAW_HOME` shape (#7310 hand-off) #7310 landed the parser side of this, so the deferred half is now due — and it was recorded in the code precisely so a rebase could not drop it silently. While expandOpenclawPath could not resolve `$OPENCLAW_HOME\...`, this rule had to refuse the shape: accepting it would have reported a finished answer and then handed the caller a path the parser failed on. The cost was that a host with that variable set lost the early return entirely and went back to waiting for the CLI to exit — which is the hang this branch exists to survive. With the parser fixed, the refusal was the only thing left standing between those hosts and the tolerance. Resolved through openclawHomeFromEnv rather than read raw from the environment. The value may itself be a tilde path that upstream expands before printing, so `filepath.IsAbs` on the raw string answers false for `OPENCLAW_HOME=~/svc` and silently withholds the early return on exactly the configuration #7310's review was about. Mutation-checked: the raw-env version fails the new `tilde-valued OPENCLAW_HOME` row with `rule(...) = false, want true`. TestOpenclawOutputCompleteRules becomes subtests so a row can carry the environment it is about, and gains three: an absolute OPENCLAW_HOME, a tilde-valued one, and an unset one — the last staying false, so a line with nothing to resolve it against still waits for exit and lets the parser report the real error instead of this rule guessing. * docs(agent): site the collector against outputOwned, not cmd.Output() #7531 moved every one-shot probe in this package onto launch.go's runOwned / outputOwned / combinedOutputOwned, which makes three claims in this branch stale rather than wrong-in-detail: - RunCollect called itself "the safe replacement for cmd.Output()". There are no cmd.Output() call sites left in pkg/agent. The comparison a reader needs is against outputOwned, which owns the same process tree and differs only in who owns the pipes — so that is what the doc compares, including why the probeWaitDelay bound is the right default for the probes it serves and the wrong one here. - RunCollect's path-taking form advertised itself as the migration entry point for "the remaining cmd.Output() call sites (models.go's catalog probes, codex.go, deveco_models.go, thinking.go)". #7531 migrated all four to the owned helpers. It stays as the form for a caller holding only a path, and for a probe that turns out to need pipe ownership. - collector's doc now says outright which of the two properties launch.go cannot provide, since "os/exec owns the draining" now describes in-package code rather than a stdlib call nobody here makes. Also adds the `if: needs.changes.outputs.backend == 'true'` guard the new Windows CI step was missing. Every other step in that job has it; without it the step ran on frontend-only changes. * docs(agent): cite upstream for the hang tolerance, not just the timings The measured "prints its answer and then never exits" for `config file` and `agents list` came from one host on openclaw 2026.5.27, which invites the obvious objection: this is an upstream bug, so why compensate for it here rather than wait for a fix. Read at openclaw v2026.7.1, it is not a bug in a build — it is the exit model. `src/cli/one-shot-exit.ts` provides an explicit opt-in, `requestExitAfterOneShotOutput`, which `runCli` flushes after both streams drain. In the shipped 2026.7.1 tarball exactly three modules take it: `models list`, `models status`, and the hooks CLI. The commands this package invokes — `config file`, `config get`, `agents list` — do not: their success paths only print, and `runtime.exit` appears solely on their failure paths, so termination is whatever the event loop happens to do. A version that exits promptly is not promising the next one will. Also pins openclawHomeRest's citation forward. `runConfigFile` still prints shortenHomePath at v2026.7.1, and resolveHomeDisplayPrefix there still selects `$OPENCLAW_HOME` on a non-empty trimmed variable and `~` otherwise. Worth recording that the separator comes from the original path rather than being inserted by the prefix, since that is what the rest-slice in this function relies on. * fix(daemon): size the config-file idle grace to the real warning gap E2E on a real OpenClaw host (2026.7.1-2, plus 2026.6.8 and 2026.7.2-beta.7) falsified two claims this branch was making. Both are corrected here; one needed a code change. **The 400ms idle grace was sized against the wrong gap.** The claim was that a one-shot response arrives in back-to-back writes, so 400ms is "orders of magnitude longer than the gap between those writes". The first half is confirmed — 0.012-0.033ms between writes. The gap that matters is a different one: with a config that produces Doctor and plugin warnings, `openclaw config file` spread its stdout over 3289ms, of which a single 3285ms silence sat between the end of the warning block and the path. The default grace is eligible to fire inside that silence, on whatever the warnings left in the buffer. Why it lands on `config file` alone: upstream routes incidental console output to stderr when argv carries `--json` (withConsoleLogsRoutedToStderrForJson, src/cli/json-output-mode.ts at v2026.7.1), so every `--json` caller here gets a stdout that is the JSON document and nothing else, and a partial document cannot parse however long the writer pauses. `config file` has no `--json`, so the warnings share its stdout with the answer. openclawConfigPathComplete is still the guarantee — a box-drawing border is not an absolute path — and a grace can only be defence in depth, since the gap is a property of the host's plugin and Doctor work. But it should not be defence that loses by three seconds, so openclawQuietIdleGrace gives `config file` 5s and leaves `--json` on the default. The cost is paid only by a CLI that hangs: `config file` then returns in path + 5s rather than path + 400ms, against a 30s deadline. **RunCollect guarantee 2 overstated what a group kill can reach.** The helper was measured holding the same stdout pipe as its parent (both `pipe:[135202]`) while running with its own PGID and SID. `kill(-pgid)` never reaches that, and neither does #7531's ownership, which uses the same group. "Invoking a CLI on a timer cannot accumulate orphans" was therefore wrong on Unix, and the three comments that repeated it are corrected. What holds regardless is guarantee 1: it is pipe ownership, not the kill, that makes Wait return and the deadline enforceable. Windows is unaffected — a Job Object owns the tree irrespective of process groups. Tests: TestOpenclawQuietIdleGraceCoversTheWarningGap pins the per-command grace and asserts the `config file` value exceeds the measured gap; TestExecOpenclawCLISurvivesTheWarningGapBeforeThePath drives a stub with a warning block, a gap past the default grace, then the path, and requires the path back. * fix(agent): make pipe EOF the completion signal, and bound what is kept Second review round found three blockers on b9b39eaea. All three reproduce exactly as reported, and all three are cases where this branch was *worse* than the launch.go helpers it replaced — which is the bar a replacement has to clear, so the fix narrows the branch rather than patching around them. **1. Leader exit was treated as answer completion (worst of the three).** RunCollect*Cmd reaped the process group as soon as the direct child exited. A wrapper can exit 0 while the real CLI — its descendant, holding the inherited stdout — has not printed yet, which is the normal shape for an npm or PowerShell shim. Measured here: the wrapper exits 0, its child prints the version 500ms later, and this head returned an empty version with a *nil error* in 427ms where outputOwned returns the version in 517ms. Empty-and-nil is the worst available shape, and detectCLIVersion routes all 23 providers through it. Pipe EOF is the only signal that means "no more output is coming", so: - detectCLIVersion, checkOpenclawVersion and discoverOpenclawAgents go back to launch.go's outputOwned / combinedOutputOwned, i.e. back to os/exec's own EOF wait. What this branch still fixes there is the *reporting*: a lingering descendant makes Wait report exec.ErrWaitDelay over an answer that arrived, and a failed --version probe skips runtime registration entirely (#6084 reverted a WaitDelay backstop over exactly that). So salvageProbeAnswer drops the error when, and only when, the caller's own parse found the answer — never on a non-zero exit, never on a deadline. - The collector keeps its reason to exist (execenv's task-critical path) and gains awaitOutputAfterExit: after the direct child exits it waits for pipe EOF, bounded by collectDrainGrace (2s, matching probeWaitDelay), unless the caller's completeness rule says the answer is already in — so a CLI that prints and exits with a helper on the pipe still pays nothing. - RunCollect / RunCollectCmd are deleted. With the probes back on outputOwned they had no production caller, and an exported wait-for-exit collector is exactly the API that invited this bug. **2. A path-shaped warning could be returned as the config path.** openclawConfigPathComplete accepted any absolute-looking last line, and the previous commit's answer — a 5s grace — was the wrong dimension, as the review pointed out by quoting this branch's own comment that no fixed grace can bound the host's plugin work. Reproduced: a CLI printing an existing plugin-cache.json path, pausing 6s, then printing the real path returned the warning path after 5.51s. Fixed by not asking a question content cannot answer. `openclaw config validate --json` reports the path in a named `path` field, on every branch (valid / invalid / file-not-found / error), in every version from v2026.5.5 — which is minOpenclawVersion — through v2026.8.1-beta.3, and `--json` makes upstream route incidental output to stderr so stdout is the document alone. openclawActiveConfigPath asks that first and keeps `config file` as a fallback with no completeness rule at all: it waits for exit and fails closed on the deadline. Upstream runs the same readConfigFileSnapshot() for both commands, so preferring the JSON form costs no extra work; it costs one extra invocation only on a CLI too old for either, which is pinned in TestPrepareOpenclawConfigFallsBackWhenConfigFileUnsupported. openclawConfigPathComplete, openclawQuietIdleGrace and openclawConfigFileIdleGrace are gone. `--json` is now the only shape with a rule, and the doc says why that is the design and not a gap. **3. Retention was unbounded.** absorb appended without limit, where outputOwned keeps the last probeStderrSampleBytes. Measured: 13,107,400 bytes of stderr retained from a CLI writing continuously, ~400x the bound it replaced. stderr now keeps its tail at collectStderrTail (= 32 KiB, so moving a call site between mechanisms cannot change what a failed probe reports) and stdout is capped at collectStdoutLimit and reports overflow as an error — deliberately asymmetric, because a head-truncated answer is how a partial catalog becomes a confident empty one. Every new guard was mutation-verified by restoring the mechanism it covers: | Mutation | Test | Failure | |---|---|---| | collector + reap on leader exit | TestDetectCLIVersionWaitsForAWrapperDescendant | `version = ""` in 0.42s | | awaitOutputAfterExit returns immediately | TestRunCollectQuietWaitsForAWrapperDescendant | `stdout = ""` | | drop the rule short-circuit | TestRunCollectQuietDoesNotWaitWhenTheAnswerIsIn | pays the full 2s | | unbounded buffers | TestCollectedStderrKeepsOnlyItsTail | retained 13,107,417 bytes | | unbounded buffers | TestCollectedStdoutOverflowIsReportedNotTruncated | 524,296 bytes, nil error | | shape rule for `config file` | TestOpenclawActiveConfigPathIgnoresAPathShapedWarning | returns plugin-cache.json | The new fallback test also caught a bug in this commit before it shipped: openclawValidatedConfigPath accepted upstream's `CONFIG_PATH ?? "openclaw.json"` literal, because expandOpenclawPath turned that relative name into a confident path under the daemon's cwd. It now requires the reported value to be absolute before expansion, allowing only the `~` and `$OPENCLAW_HOME` forms. * fix(agent): repair the Windows-only collector test call sites The mechanical conversion of the deleted RunCollect call sites over-widened three tuples in run_collect_windows_test.go, which already used RunCollectQuiet's four results. Host `go build` and `go vet` never compiled them — the file is `//go:build windows` — so the break surfaced on the windows-execenv job as `assignment mismatch: 5 variables but RunCollectQuiet returns 4 values`. Verified this time the way it should have been the first time: `GOOS=windows go build ./...` and `GOOS=windows go vet ./...`, plus darwin and linux. That is the check to run whenever this package's exported signatures move, since a third of its process-lifecycle tests only compile on Windows. * test(execenv): pin the config-path payload shapes E2E measured E2E on OpenClaw 2026.7.1-2 confirmed `config validate --json` as the config-path source and corrected two things this branch had inferred from source. **The exit status is load-bearing in the wrong direction, and this was untested.** Two of the three real states exit *non-zero* while carrying the answer: a missing config file gives `{"valid":false,"path":"…","error":"file not found"}` and an invalid one `{"valid":false,"path":"…","issues":[…]}`, both exit 1 with stderr empty. A fresh install is the missing-file case, so reading a non-zero exit as "no answer" would break first run. openclawValidatedConfigPath already ignored the status, but nothing pinned it — writeOpenclawConfigStub could only exit 0, so the tests covered the rarest branch. It now takes an exit code, and TestOpenclawActiveConfigPathReadsThePathFromANonZeroExit drives all three measured payloads. **Two source-based claims corrected.** - "Preferring the JSON form costs nothing extra" understated it. Measured over three runs each on the same host and config: 1.65/1.66/1.65s for `config validate --json` against 6.36/4.16/4.06s for `config file`. The JSON form is ~2.5-4x faster, which tracks the output it does not render — 715 bytes of JSON against 2663 bytes and 28 lines of warning UI. - The reason a `--json` stdout is clean was attributed to upstream's console-log reroute (`withConsoleLogsRoutedToStderrForJson`). That function exists, but it is not what was observed doing the work: the Doctor and plugin warnings arrive as a `warnings` array *inside* the payload. The distinction matters for why JSONOutputComplete is a real signal — the stream is structurally one document, not merely quieter — so the comments now describe the measurement rather than the mechanism inferred from it. Unrelated and pre-existing: `./pkg/agent`'s ACP timing tests (1-6s wall-clock budgets) flake under load, and which ones fail varies per run. Control run of `origin/main`'s whole package on this machine, same conditions: fails TestCodexExecuteSemanticInactivityAllowsContinuousMessages and TestCodexExecuteLegacyFirstTurnMessageSatisfiesProgress. Not introduced here. * fix(agent): require a recognised version before salvaging a probe answer The ErrWaitDelay salvage in detectCLIVersion was gated on `version != ""`, and extractVersionLine falls back to the trimmed raw output when no line carries a semver token — so any non-empty text satisfied it, including whatever a wrapper printed before the real version existed. Review reproduced it with a stub that prints a banner, exits 0, and has a descendant print the version past the 2s WaitDelay: detectCLIVersion returned "initializing plugins" with a nil error, and logged "CLI answered but left its output pipes open" — the opposite of what happened. That value is then persisted as the runtime version, for every provider, since DetectVersion routes all 23 through here. extractVersionLine now reports whether the semver scan actually matched, and only that answers the salvage question. The trimmed-raw fallback still stands on the success path: a CLI that exits 0 with an unusual version format is reporting its version (#2516), and it is only unusable as evidence that a bounded, incomplete read already holds the answer. Mutation-checked: with the old gate, TestDetectCLIVersionDoesNotSalvageABannerAsTheVersion fails with exactly the reviewed shape — version="initializing plugins", nil error, misleading warn. * test(agent): derive the collected-stdout cap from the largest answer collectStdoutLimit argued for why a fail-closed cap is needed but not for its value, and where it falls is a product compatibility boundary rather than only a memory bound: a legal config past the cap stops a task from starting at all. The derivation is now recorded and executable. Measured answers on 2026.7.1-2 are 715 B (`config validate --json`) and 2663 B (`config file`); the largest answer any call site asks for is the resolved config from `config get --json`, which upstream puts no ceiling on. So the worst case is constructed — 1000 agents and 250 MCP servers with the fields OpenClaw carries, 547,291 bytes, already orders of magnitude past a real deployment — and 8 MiB leaves 15x on top of it while matching what the daemon already holds for a comparable per-task payload (maxLocalSkillBundleSize). Two tests pin it: the constructed high-water mark must round-trip through the collector byte-identically with the stated headroom, and the boundary itself is exact — at the limit the answer is returned whole, one byte past it fails closed with errCollectStdoutTooLarge. * fix(execenv): share one CLI deadline across both path-resolution attempts openclawActiveConfigPath asks `config validate --json` first and falls back to `config file`, and the fallback carried a fresh full deadline. That made the worst case five budgets against a ceiling derived from four: at the 60s override ceiling, 5m of CLI time alone, landing exactly on daemon.defaultTaskPrepareTimeout — so the specific, non-retryable ErrOpenclawCLITimeout collapses back into the generic retryable prepare timeout, which is the outcome openclawCLIMaxTimeout exists to prevent. Both invocations answer the same question, so they now share one context. The multiplier is renamed to openclawMaxCLIDeadlinesPerPreparation because that is what it always measured — the budget is per step, not per invocation. The consequence is deliberate and asserted: a validate that burns the whole budget leaves the fallback none, os/exec reports the expired context on Start, and the failure still carries ErrOpenclawCLITimeout. A CLI that cannot say where its config is within the entire deadline will not answer the same question on a fresh one. TestPrepareOpenclawConfigWorstCaseCLIBudgets now drives the real worst case (validate fails, config file answers, registry fallback, managed MCP) and counts distinct ctx deadlines rather than calls — the old test synthesized a validate success from the `config file` response, so the two never fired in the same run. Mutation-checked: with separate deadlines it reports 5 budgets against a constant of 4. --------- Co-authored-by: weiweiwei <[email protected]>
798 lines
30 KiB
Go
798 lines
30 KiB
Go
package agent
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"encoding/json"
|
|
"errors"
|
|
"fmt"
|
|
"io"
|
|
"log/slog"
|
|
"os/exec"
|
|
"regexp"
|
|
"strconv"
|
|
"strings"
|
|
"time"
|
|
)
|
|
|
|
// openclawNoParseableOutput is the canonical error string surfaced when the
|
|
// adapter cannot extract any usable JSON from a run's stdout. The exact
|
|
// phrase is depended on by external log-grep / dashboard alerts; do not
|
|
// change it without also updating those consumers.
|
|
const openclawNoParseableOutput = "openclaw returned no parseable output"
|
|
|
|
// minOpenclawVersion is the lowest openclaw version that emits its
|
|
// --json result on stdout. PR #2101 swapped the adapter from reading
|
|
// stderr to stdout; older builds wrote JSON to stderr and now appear
|
|
// to silently produce no output. The check in Execute fails fast with
|
|
// a hardcoded upgrade hint so users see an actionable message instead
|
|
// of "openclaw returned no parseable output".
|
|
const minOpenclawVersion = "2026.5.5"
|
|
|
|
// openclawVersionPattern extracts a three-segment dotted version from
|
|
// arbitrary `openclaw --version` output (e.g. "openclaw 2026.5.5",
|
|
// "openclaw v2026.5.5 c37871e").
|
|
var openclawVersionPattern = regexp.MustCompile(`(\d+)\.(\d+)\.(\d+)`)
|
|
|
|
// openclawBlockedArgs are flags hardcoded by the daemon that must not be
|
|
// overridden by user-configured custom_args.
|
|
var openclawBlockedArgs = map[string]blockedArgMode{
|
|
"--local": blockedStandalone, // local mode for daemon execution
|
|
"--json": blockedStandalone, // JSON output for daemon communication
|
|
"--session-id": blockedWithValue, // managed by daemon for session resumption
|
|
"--message": blockedWithValue, // prompt is set by daemon
|
|
"--model": blockedWithValue, // openclaw agent does not accept --model; model is bound at registration via `openclaw agents add/update --model`
|
|
"--system-prompt": blockedWithValue, // openclaw agent does not accept --system-prompt; instructions are injected into --message
|
|
}
|
|
|
|
// openclawBackend implements Backend by spawning `openclaw agent --message <prompt>
|
|
// --output-format stream-json --yes` and reading streaming NDJSON events from
|
|
// stdout — similar to the opencode backend.
|
|
type openclawBackend struct {
|
|
cfg Config
|
|
}
|
|
|
|
func (b *openclawBackend) Execute(ctx context.Context, prompt string, opts ExecOptions) (*Session, error) {
|
|
execPath := b.cfg.ExecutablePath
|
|
if execPath == "" {
|
|
execPath = "openclaw"
|
|
}
|
|
if _, err := exec.LookPath(execPath); err != nil {
|
|
return nil, fmt.Errorf("openclaw executable not found at %q: %w", execPath, err)
|
|
}
|
|
|
|
if err := checkOpenclawVersion(ctx, b.cfg.commandAt(execPath)); err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
timeout := opts.Timeout
|
|
runCtx, cancel := runContext(ctx, timeout)
|
|
|
|
sessionID := opts.ResumeSessionID
|
|
if sessionID == "" {
|
|
sessionID = fmt.Sprintf("multica-%d", time.Now().UnixNano())
|
|
}
|
|
args := buildOpenclawArgs(prompt, sessionID, opts, b.cfg.Logger)
|
|
|
|
cmd := b.cfg.commandAt(execPath).exec(runCtx, args...)
|
|
hideAgentWindow(cmd)
|
|
b.cfg.logAgentCommand(cmd, newAgentCommandLogArgs(args, trustAgentCommandPositional(0, "agent")))
|
|
// 500ms, matching cursor-agent — the other backend whose CLI can deliver a
|
|
// terminal result while keeping a process alive.
|
|
//
|
|
// Note what WaitDelay actually bounds, because it is easy to get wrong: the
|
|
// timer starts when the context is done OR when Wait observes the child has
|
|
// exited, whichever comes first. So a *clean* exit reaches it too, whenever
|
|
// any descendant still holds one of the pipes os/exec manages — and this
|
|
// backend has such a pipe, since cmd.Stderr below is a plain io.Writer. In
|
|
// that case Wait returns exec.ErrWaitDelay even though the process exited 0,
|
|
// which is why the status switch has to special-case it: the result is
|
|
// already parsed and in hand, and only a tail of stderr logs is lost.
|
|
//
|
|
// Lowering the bound is still right. The delay is only reached when someone
|
|
// is holding a pipe open, and on the cut-short path we deliberately kill a
|
|
// process that is doing exactly that — a long delay there would add its full
|
|
// length to every reply.
|
|
cmd.WaitDelay = 500 * time.Millisecond
|
|
if opts.Cwd != "" {
|
|
cmd.Dir = opts.Cwd
|
|
}
|
|
cmd.Env = buildEnv(b.cfg.Env)
|
|
|
|
// openclaw writes its --json output to stdout. Stderr carries log
|
|
// overflow (security warnings, tool errors, etc.) — capture it via a
|
|
// log writer so it surfaces in daemon logs without being fed into the
|
|
// JSON parser.
|
|
stdout, err := cmd.StdoutPipe()
|
|
if err != nil {
|
|
cancel()
|
|
return nil, fmt.Errorf("openclaw stdout pipe: %w", err)
|
|
}
|
|
cmd.Stderr = newLogWriter(b.cfg.Logger, "[openclaw:stderr] ")
|
|
|
|
if err := startOwnedProcessTree(cmd, b.cfg.Logger); err != nil {
|
|
cancel()
|
|
return nil, fmt.Errorf("start openclaw: %w", err)
|
|
}
|
|
|
|
b.cfg.Logger.Info("openclaw started", "pid", cmd.Process.Pid, "cwd", opts.Cwd, "model", opts.Model)
|
|
|
|
msgCh := make(chan Message, 256)
|
|
resCh := make(chan Result, 1)
|
|
|
|
// Close stdout when the context is cancelled so the scanner unblocks.
|
|
go func() {
|
|
<-runCtx.Done()
|
|
_ = stdout.Close()
|
|
}()
|
|
|
|
go func() {
|
|
defer cancel()
|
|
defer close(msgCh)
|
|
defer close(resCh)
|
|
|
|
startTime := time.Now()
|
|
scanResult := b.processOutput(stdout, msgCh)
|
|
|
|
// openclaw delivered a complete result but would not exit. Cancel the
|
|
// run context so CommandContext kills it and cmd.Wait can return —
|
|
// otherwise this goroutine parks forever on an agent that has already
|
|
// finished, and the reply never reaches the user. Same protocol-boundary
|
|
// treatment cursor-agent gets on its terminal `result` event.
|
|
if scanResult.cutShort {
|
|
b.cfg.Logger.Warn("openclaw delivered its result but did not exit; "+
|
|
"treating the complete result as the protocol boundary",
|
|
"pid", cmd.Process.Pid)
|
|
cancel()
|
|
}
|
|
|
|
// Wait for process exit.
|
|
exitErr := cmd.Wait()
|
|
releaseProcessGroup(cmd)
|
|
duration := time.Since(startTime)
|
|
|
|
switch {
|
|
case scanResult.cutShort:
|
|
// A complete result is the protocol boundary. Ignore the
|
|
// cancellation and exit error caused by stopping an openclaw that
|
|
// lingers afterward — the run succeeded, and reporting it as
|
|
// "aborted" would throw away a reply we already hold.
|
|
case runCtx.Err() == context.DeadlineExceeded:
|
|
scanResult.status = "timeout"
|
|
scanResult.errMsg = fmt.Sprintf("openclaw timed out after %s", timeout)
|
|
case runCtx.Err() == context.Canceled:
|
|
scanResult.status = "aborted"
|
|
scanResult.errMsg = "execution cancelled"
|
|
case errors.Is(exitErr, exec.ErrWaitDelay) && scanResult.status == "completed":
|
|
// The process itself exited successfully — that is what
|
|
// ErrWaitDelay means by definition — and only a lingering
|
|
// descendant kept one of os/exec's pipes open past WaitDelay.
|
|
// stdout has already been read to EOF and the result parsed, so the
|
|
// only thing lost is a tail of stderr log lines. Reporting this as
|
|
// a failure would discard a deliverable reply, which is a worse
|
|
// outcome than the hang this whole change fixes.
|
|
//
|
|
// Not folded into the cutShort case above: that path cancels on
|
|
// purpose, and a Cancel call makes Wait report the kill instead of
|
|
// ErrWaitDelay. This case is specifically the clean-exit one.
|
|
//
|
|
// Reword with care: this warning is the only observable proof that
|
|
// this branch ran, so TestOpenclawExecuteToleratesLingeringStderrHolder
|
|
// asserts on the "held a pipe past WaitDelay" fragment. That fragment
|
|
// straddles the concatenation below, so grepping the source for it
|
|
// finds nothing — hence this note.
|
|
b.cfg.Logger.Warn("openclaw exited cleanly but a descendant held a "+
|
|
"pipe past WaitDelay; delivering the parsed result and dropping "+
|
|
"the stderr tail", "pid", cmd.Process.Pid)
|
|
case exitErr != nil && scanResult.status == "completed":
|
|
scanResult.status = "failed"
|
|
scanResult.errMsg = fmt.Sprintf("openclaw exited with error: %v", exitErr)
|
|
}
|
|
|
|
b.cfg.Logger.Info("openclaw finished", "pid", cmd.Process.Pid, "status", scanResult.status, "duration", duration.Round(time.Millisecond).String())
|
|
|
|
// Build usage map. Prefer the model openclaw reported in
|
|
// `meta.agentMeta.model` (the actual LLM, e.g. `deepseek-chat`).
|
|
// Fall back to opts.Model — which for openclaw is the agent name
|
|
// passed via `--agent`, not a real model identifier — only when
|
|
// the runtime didn't surface its own model. Last resort is the
|
|
// daemon's `unknown` placeholder.
|
|
var usage map[string]TokenUsage
|
|
u := scanResult.usage
|
|
if u.InputTokens > 0 || u.OutputTokens > 0 || u.CacheReadTokens > 0 || u.CacheWriteTokens > 0 {
|
|
model := scanResult.model
|
|
if model == "" {
|
|
model = opts.Model
|
|
}
|
|
if model == "" {
|
|
model = "unknown"
|
|
}
|
|
usage = map[string]TokenUsage{model: u}
|
|
}
|
|
|
|
resCh <- Result{
|
|
Status: scanResult.status,
|
|
Output: scanResult.output,
|
|
Error: scanResult.errMsg,
|
|
DurationMs: duration.Milliseconds(),
|
|
SessionID: scanResult.sessionID,
|
|
Usage: usage,
|
|
}
|
|
}()
|
|
|
|
return &Session{Messages: msgCh, Result: resCh}, nil
|
|
}
|
|
|
|
// buildOpenclawArgs assembles the argv for a one-shot `openclaw agent` invocation.
|
|
//
|
|
// The CLI only accepts --local, --json, --session-id, --timeout, --message (and
|
|
// flags like --agent / --channel that users pass through CustomArgs). Notably
|
|
// it does NOT accept --model or --system-prompt — model is bound at agent
|
|
// registration time via `openclaw agents add/update --model`, and instructions
|
|
// must be injected inline into --message because openclaw loads AGENTS.md from
|
|
// its own workspace directory, not from cwd.
|
|
//
|
|
// Routing (issue #3260): `openclaw agent` defaults to Gateway routing; --local
|
|
// is the embedded-mode opt-in. The daemon historically forced --local so every
|
|
// run executed in-process on the daemon host. When opts.OpenclawMode ==
|
|
// "gateway" the daemon drops --local so openclaw dials its configured Gateway
|
|
// instead — useful when the daemon host is a lightweight coordinator and the
|
|
// real agent work should land on a remote machine running the Gateway.
|
|
// --local stays in openclawBlockedArgs so users cannot smuggle it back in via
|
|
// custom_args under gateway mode (mode is the single source of truth).
|
|
func buildOpenclawArgs(prompt, sessionID string, opts ExecOptions, logger *slog.Logger) []string {
|
|
args := []string{"agent"}
|
|
if opts.OpenclawMode != "gateway" {
|
|
args = append(args, "--local")
|
|
}
|
|
args = append(args, "--json", "--session-id", sessionID)
|
|
if opts.Timeout > 0 {
|
|
args = append(args, "--timeout", fmt.Sprintf("%d", int(opts.Timeout.Seconds())))
|
|
}
|
|
// OpenClaw binds models to pre-registered agents at `openclaw agents
|
|
// add/update --model` time; the daemon selects one at runtime by
|
|
// passing --agent <id>. The model dropdown populates its list from
|
|
// `openclaw agents list`, so opts.Model here is an agent id (see
|
|
// openclawEntriesToModels — the agent's display name lives in the
|
|
// dropdown label, not in opts.Model). Only inject when the user
|
|
// hasn't already set --agent via custom_args — custom_args wins for
|
|
// backward compatibility with existing configs.
|
|
customArgs := filterCustomArgs(opts.CustomArgs, openclawBlockedArgs, logger)
|
|
if opts.Model != "" && !customArgsContains(customArgs, "--agent") {
|
|
args = append(args, "--agent", opts.Model)
|
|
}
|
|
args = append(args, customArgs...)
|
|
|
|
if opts.SystemPrompt != "" {
|
|
prompt = opts.SystemPrompt + "\n\n" + prompt
|
|
}
|
|
args = append(args, "--message", prompt)
|
|
return args
|
|
}
|
|
|
|
// customArgsContains reports whether args contains the given flag
|
|
// (either as a standalone token "--flag" or in "--flag=value" form).
|
|
func customArgsContains(args []string, flag string) bool {
|
|
prefix := flag + "="
|
|
for _, a := range args {
|
|
if a == flag || strings.HasPrefix(a, prefix) {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|
|
|
|
// checkOpenclawVersion runs `<execPath> --version` and returns a
|
|
// user-facing error when the installed openclaw is older than
|
|
// minOpenclawVersion. The returned error becomes the task's failure
|
|
// comment, so the message intentionally names the detected version
|
|
// and the upgrade command.
|
|
func checkOpenclawVersion(ctx context.Context, runtimeCmd Command) error {
|
|
// This runs synchronously before Execute creates the provider session, so a
|
|
// pipe-holding descendant here would otherwise leave a task marked running
|
|
// with no backend and no inactivity watchdog to stop it. ctx is the task's
|
|
// and can be hours long, so the probe carries the same bound every other
|
|
// provider's version probe gets.
|
|
ctx, cancel := context.WithTimeout(ctx, detectVersionTimeout)
|
|
defer cancel()
|
|
|
|
// combinedOutputOwned, for the reason recorded on detectCLIVersion: pipe EOF
|
|
// is the signal that no more output is coming, and the direct child's exit is
|
|
// not. openclaw is npm-installed, so on Windows the direct child is a shim and
|
|
// the real CLI is already a descendant.
|
|
//
|
|
// Both streams are parsed, and extractVersionLine is deliberately bypassed.
|
|
// CombinedOutput rather than separate buffers because a build that prints its
|
|
// banner on stderr must still pass the gate, and one shared writer is what
|
|
// makes os/exec give the two streams a single pipe and therefore one
|
|
// interleaving. parseOpenclawVersion already scans the whole text with
|
|
// openclaw's own version pattern, whereas picking "the first line containing a
|
|
// semver" first would let unrelated stderr noise answer for it — a node
|
|
// deprecation warning carries one.
|
|
cmd := runtimeCmd.exec(ctx, "--version")
|
|
hideAgentWindow(cmd)
|
|
raw, err := combinedOutputOwned(cmd, runtimeCmd.logger)
|
|
out := string(raw)
|
|
detected, parsed := parseOpenclawVersion(out)
|
|
if err != nil {
|
|
// The gate may proceed on a version that arrived before a lingering
|
|
// `openclaw-config` helper held the pipes past WaitDelay — failing here
|
|
// would fail the task over an answer we have. Anything else (non-zero
|
|
// exit, deadline) still fails; see salvageProbeAnswer.
|
|
if !salvageProbeAnswer(runtimeCmd, "--version", parsed, err) {
|
|
// ExplainExecError by hand: detectCLIVersion applies it on the daemon's
|
|
// probe path, and this is the other gate that has to name an ENOEXEC
|
|
// shim rather than report an opaque exec failure (MUL-6164).
|
|
return fmt.Errorf("openclaw --version failed: %w", ExplainExecError(err))
|
|
}
|
|
}
|
|
if !parsed {
|
|
return fmt.Errorf("could not parse openclaw version from output: %q", strings.TrimSpace(out))
|
|
}
|
|
if compareOpenclawVersion(detected, minOpenclawVersion) < 0 {
|
|
return fmt.Errorf("openclaw %s is below the minimum supported version %s. Run `openclaw update` to upgrade and try again.", detected, minOpenclawVersion)
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// parseOpenclawVersion extracts the first three-segment dotted version
|
|
// from arbitrary `openclaw --version` output. Returns ok=false when no
|
|
// match is found.
|
|
func parseOpenclawVersion(raw string) (string, bool) {
|
|
m := openclawVersionPattern.FindString(raw)
|
|
if m == "" {
|
|
return "", false
|
|
}
|
|
return m, true
|
|
}
|
|
|
|
// compareOpenclawVersion compares two three-segment dotted versions
|
|
// numerically. Returns -1, 0, or +1 like bytes.Compare. Inputs must be
|
|
// well-formed (matched by openclawVersionPattern); malformed segments
|
|
// compare as zero.
|
|
func compareOpenclawVersion(a, b string) int {
|
|
aParts := strings.SplitN(a, ".", 3)
|
|
bParts := strings.SplitN(b, ".", 3)
|
|
for i := 0; i < 3; i++ {
|
|
ai, _ := strconv.Atoi(aParts[i])
|
|
bi, _ := strconv.Atoi(bParts[i])
|
|
if ai < bi {
|
|
return -1
|
|
}
|
|
if ai > bi {
|
|
return 1
|
|
}
|
|
}
|
|
return 0
|
|
}
|
|
|
|
// ── Event handlers ──
|
|
|
|
// openclawEventResult holds accumulated state from processing the event stream.
|
|
type openclawEventResult struct {
|
|
status string
|
|
errMsg string
|
|
output string
|
|
sessionID string
|
|
usage TokenUsage
|
|
// model is the LLM identifier reported by openclaw in its result blob
|
|
// (`meta.agentMeta.model`). Empty when the run did not emit it (older
|
|
// openclaw versions, partial outputs). Distinct from `opts.Model`,
|
|
// which for the openclaw backend is the openclaw *agent* name passed
|
|
// via `--agent`, not the underlying model.
|
|
model string
|
|
// cutShort is true when the run ended because openclaw had delivered a
|
|
// complete result but would not exit, rather than because stdout reached
|
|
// EOF. The caller must cancel the run context before cmd.Wait() in that
|
|
// case — otherwise it waits on a process that never leaves — and must not
|
|
// report the resulting cancellation as an abort. Same treatment
|
|
// cursor-agent's `resultSeen` gets.
|
|
cutShort bool
|
|
}
|
|
|
|
// processOutput reads the JSON output from openclaw --json stdout and returns
|
|
// the parsed result. OpenClaw writes its JSON output to stdout; stderr carries
|
|
// log overflow and is captured separately by the caller. The stream may
|
|
// contain:
|
|
//
|
|
// - A final result JSON (with payloads + meta) — the format openclaw 2026.5.x
|
|
// emits today, typically pretty-printed across many lines
|
|
// - NDJSON streaming events (type: "text", "tool_use", "tool_result", "error",
|
|
// "step_start", "step_finish") — supported for forward compatibility and
|
|
// other backends sharing this code path; openclaw does not emit these today
|
|
//
|
|
// Implementation note (WOR-10 follow-up): we previously scanned line-by-line
|
|
// only, then tried a whole-buffer parse in a fallback path. Under load
|
|
// (daemon shutdown racing the scanner, partial chunked reads) the line
|
|
// scanner could see truncated input that never reassembled, surfacing the
|
|
// generic "openclaw returned no parseable output" error even though the
|
|
// agent's work succeeded. We now read the full buffer first and try a
|
|
// single whole-buffer parse against the final-result schema. Only if that
|
|
// fails do we fall through to the line-by-line NDJSON scanner. This makes
|
|
// the dominant happy path (one pretty-printed JSON blob) deterministic
|
|
// while keeping NDJSON event support intact.
|
|
func (b *openclawBackend) processOutput(r io.Reader, ch chan<- Message) openclawEventResult {
|
|
buf, cutShort, readErr := readOpenclawStdout(r, openclawResultIdleGrace)
|
|
if readErr != nil {
|
|
return openclawEventResult{status: "failed", errMsg: fmt.Sprintf("read stdout: %v", readErr)}
|
|
}
|
|
|
|
// Whole-buffer fast path: openclaw 2026.5.x emits a single pretty-printed
|
|
// JSON result blob. Try parsing the entire buffer (after trimming whitespace
|
|
// and any preceding non-JSON log lines) as the final-result schema. If it
|
|
// matches, we're done — no need to involve the line scanner at all.
|
|
if result, ok := parseWholeBufferOpenclawResult(buf); ok {
|
|
var output strings.Builder
|
|
res := b.buildOpenclawEventResult(result, ch, &output)
|
|
res.cutShort = cutShort
|
|
return res
|
|
}
|
|
|
|
// Fall-back path: NDJSON line scanner. Note that because we already
|
|
// drained the full buffer with io.ReadAll above, this path is no longer
|
|
// truly streaming — events accumulate until the subprocess closes
|
|
// stdout, then drain all at once. OpenClaw 2026.5.x does not emit
|
|
// streaming events, so this regression is invisible today; if a future
|
|
// backend on this code path emits real NDJSON streams and needs live
|
|
// progress updates, we'll need to split the fast path off a streaming
|
|
// reader instead of io.ReadAll.
|
|
scanner := newAgentStreamScanner(bytes.NewReader(buf))
|
|
|
|
var output strings.Builder
|
|
var sessionID string
|
|
var model string
|
|
var usage TokenUsage
|
|
finalStatus := "completed"
|
|
var finalError string
|
|
gotEvents := false // true if we parsed at least one streaming event or result
|
|
|
|
var rawLines []string
|
|
for scanner.Scan() {
|
|
line := strings.TrimSpace(scanner.Text())
|
|
if line == "" {
|
|
continue
|
|
}
|
|
|
|
// Try parsing as a streaming NDJSON event first.
|
|
if event, ok := tryParseOpenclawEvent(line); ok {
|
|
gotEvents = true
|
|
if event.SessionID != "" {
|
|
sessionID = event.SessionID
|
|
}
|
|
switch event.Type {
|
|
case "text":
|
|
if event.Text != "" {
|
|
output.WriteString(event.Text)
|
|
trySend(ch, Message{Type: MessageText, Content: event.Text})
|
|
}
|
|
case "tool_use":
|
|
var input map[string]any
|
|
if event.Input != nil {
|
|
_ = json.Unmarshal(event.Input, &input)
|
|
}
|
|
trySend(ch, Message{
|
|
Type: MessageToolUse,
|
|
Tool: event.Tool,
|
|
CallID: event.CallID,
|
|
Input: input,
|
|
})
|
|
case "tool_result":
|
|
trySend(ch, Message{
|
|
Type: MessageToolResult,
|
|
Tool: event.Tool,
|
|
CallID: event.CallID,
|
|
Output: event.Text,
|
|
})
|
|
case "error":
|
|
errMsg := event.errorMessage()
|
|
b.cfg.Logger.Warn("openclaw error event", "error", errMsg)
|
|
trySend(ch, Message{Type: MessageError, Content: errMsg})
|
|
finalStatus = "failed"
|
|
finalError = errMsg
|
|
case "lifecycle":
|
|
phase := event.Phase
|
|
if phase == "error" || phase == "failed" || phase == "cancelled" {
|
|
errMsg := event.errorMessage()
|
|
b.cfg.Logger.Warn("openclaw lifecycle failure", "phase", phase, "error", errMsg)
|
|
trySend(ch, Message{Type: MessageError, Content: errMsg})
|
|
finalStatus = "failed"
|
|
finalError = errMsg
|
|
}
|
|
case "step_start":
|
|
trySend(ch, Message{Type: MessageStatus, Status: "running"})
|
|
case "step_finish":
|
|
if event.Usage != nil {
|
|
u := parseOpenclawUsage(event.Usage)
|
|
usage.InputTokens += u.InputTokens
|
|
usage.OutputTokens += u.OutputTokens
|
|
usage.CacheReadTokens += u.CacheReadTokens
|
|
usage.CacheWriteTokens += u.CacheWriteTokens
|
|
}
|
|
}
|
|
continue
|
|
}
|
|
|
|
// Try parsing as a final result blob (legacy format).
|
|
if result, ok := tryParseOpenclawResult(line); ok {
|
|
gotEvents = true
|
|
res := b.buildOpenclawEventResult(result, ch, &output)
|
|
if res.sessionID != "" {
|
|
sessionID = res.sessionID
|
|
}
|
|
if res.model != "" {
|
|
model = res.model
|
|
}
|
|
// Prefer usage from the final result if no streaming events reported it.
|
|
u := res.usage
|
|
if u.InputTokens > 0 || u.OutputTokens > 0 || u.CacheReadTokens > 0 || u.CacheWriteTokens > 0 {
|
|
usage = u
|
|
}
|
|
continue
|
|
}
|
|
|
|
// Not JSON — treat as log line.
|
|
b.cfg.Logger.Debug("[openclaw:stdout] " + line)
|
|
rawLines = append(rawLines, line)
|
|
}
|
|
|
|
if err := scanner.Err(); err != nil {
|
|
return openclawEventResult{status: "failed", errMsg: fmt.Sprintf("read stdout: %v", err)}
|
|
}
|
|
|
|
// If we got no events at all, fall back to raw output. The whole-buffer
|
|
// fast path above already tried the structured-result parse — by the time
|
|
// we reach here the buffer truly is unstructured (just log lines, plain
|
|
// text, or empty). Surface the trimmed text as a completed run when we
|
|
// have any, otherwise the canonical no-parseable-output failure.
|
|
if !gotEvents {
|
|
trimmed := strings.TrimSpace(strings.Join(rawLines, "\n"))
|
|
if trimmed != "" {
|
|
return openclawEventResult{status: "completed", output: trimmed}
|
|
}
|
|
return openclawEventResult{
|
|
status: "failed",
|
|
errMsg: openclawNoParseableOutput,
|
|
}
|
|
}
|
|
|
|
return openclawEventResult{
|
|
status: finalStatus,
|
|
errMsg: finalError,
|
|
output: output.String(),
|
|
sessionID: sessionID,
|
|
usage: usage,
|
|
model: model,
|
|
}
|
|
}
|
|
|
|
// parseWholeBufferOpenclawResult attempts to parse the entire stdout buffer
|
|
// as a single openclaw final-result JSON blob (the format openclaw 2026.5.x
|
|
// emits today, almost always pretty-printed across multiple lines).
|
|
//
|
|
// It first tries the buffer as-is, then strips any leading non-JSON log
|
|
// lines (lines that don't start with '{' at column 0) so a daemon log
|
|
// preamble doesn't defeat the parse. It does NOT scan into the middle of
|
|
// log lines: only line starts that begin with '{' are considered candidate
|
|
// JSON entry points, mirroring the conservative behaviour of
|
|
// tryParseOpenclawResult.
|
|
func parseWholeBufferOpenclawResult(buf []byte) (openclawResult, bool) {
|
|
trimmed := strings.TrimSpace(string(buf))
|
|
if trimmed == "" {
|
|
return openclawResult{}, false
|
|
}
|
|
if result, ok := tryParseOpenclawResult(trimmed); ok {
|
|
return result, true
|
|
}
|
|
// Strip any leading log lines that precede the JSON blob.
|
|
lines := strings.Split(trimmed, "\n")
|
|
for i, line := range lines {
|
|
if len(line) > 0 && line[0] == '{' {
|
|
candidate := strings.TrimSpace(strings.Join(lines[i:], "\n"))
|
|
if result, ok := tryParseOpenclawResult(candidate); ok {
|
|
return result, true
|
|
}
|
|
return openclawResult{}, false
|
|
}
|
|
}
|
|
return openclawResult{}, false
|
|
}
|
|
|
|
// tryParseOpenclawEvent attempts to parse a line as a streaming NDJSON event.
|
|
// Returns the event and true if the line is a valid event with a known type.
|
|
func tryParseOpenclawEvent(line string) (openclawEvent, bool) {
|
|
if len(line) == 0 || line[0] != '{' {
|
|
return openclawEvent{}, false
|
|
}
|
|
var event openclawEvent
|
|
if err := json.Unmarshal([]byte(line), &event); err != nil {
|
|
return openclawEvent{}, false
|
|
}
|
|
if event.Type == "" {
|
|
return openclawEvent{}, false
|
|
}
|
|
return event, true
|
|
}
|
|
|
|
// tryParseOpenclawResult attempts to parse a line as a final result blob
|
|
// (the legacy format with payloads + meta). Lines must start with '{' to be
|
|
// considered — we no longer scan for braces at arbitrary positions, which
|
|
// avoids false matches on log lines containing JSON fragments.
|
|
func tryParseOpenclawResult(raw string) (openclawResult, bool) {
|
|
if len(raw) == 0 || raw[0] != '{' {
|
|
return openclawResult{}, false
|
|
}
|
|
var result openclawResult
|
|
if err := json.Unmarshal([]byte(raw), &result); err != nil {
|
|
return openclawResult{}, false
|
|
}
|
|
if result.Payloads == nil && result.Meta.DurationMs == 0 {
|
|
return openclawResult{}, false
|
|
}
|
|
return result, true
|
|
}
|
|
|
|
// buildOpenclawEventResult extracts text and metadata from a final result blob.
|
|
// Text payloads are appended to the shared output builder and emitted to ch.
|
|
func (b *openclawBackend) buildOpenclawEventResult(result openclawResult, ch chan<- Message, output *strings.Builder) openclawEventResult {
|
|
for _, p := range result.Payloads {
|
|
if p.Text != "" {
|
|
output.WriteString(p.Text)
|
|
trySend(ch, Message{Type: MessageText, Content: p.Text})
|
|
}
|
|
}
|
|
|
|
var sessionID string
|
|
var model string
|
|
var usage TokenUsage
|
|
if result.Meta.AgentMeta != nil {
|
|
if sid, ok := result.Meta.AgentMeta["sessionId"].(string); ok {
|
|
sessionID = sid
|
|
}
|
|
// `meta.agentMeta.model` is openclaw's true LLM identifier
|
|
// (e.g. "deepseek-chat", "claude-sonnet-4"). Take it as-is — the
|
|
// dashboard expects whatever string the runtime reports, mirroring
|
|
// claude/pi/codex which read model directly off their stream.
|
|
if m, ok := result.Meta.AgentMeta["model"].(string); ok {
|
|
model = strings.TrimSpace(m)
|
|
}
|
|
if u, ok := result.Meta.AgentMeta["usage"].(map[string]any); ok {
|
|
usage = parseOpenclawUsage(u)
|
|
}
|
|
}
|
|
|
|
return openclawEventResult{
|
|
status: "completed",
|
|
output: output.String(),
|
|
sessionID: sessionID,
|
|
usage: usage,
|
|
model: model,
|
|
}
|
|
}
|
|
|
|
// parseOpenclawUsage extracts token usage from a map, supporting multiple
|
|
// field name conventions used by different OpenClaw versions and PaperClip:
|
|
//
|
|
// input / inputTokens / input_tokens
|
|
// output / outputTokens / output_tokens
|
|
// cacheRead / cachedInputTokens / cached_input_tokens / cache_read
|
|
// cacheWrite / cacheCreationInputTokens / cache_creation_input_tokens / cache_write
|
|
func parseOpenclawUsage(data map[string]any) TokenUsage {
|
|
return TokenUsage{
|
|
InputTokens: openclawInt64FirstOf(data, "input", "inputTokens", "input_tokens"),
|
|
OutputTokens: openclawInt64FirstOf(data, "output", "outputTokens", "output_tokens"),
|
|
CacheReadTokens: openclawInt64FirstOf(data, "cacheRead", "cachedInputTokens", "cached_input_tokens", "cache_read", "cache_read_input_tokens"),
|
|
CacheWriteTokens: openclawInt64FirstOf(data, "cacheWrite", "cacheCreationInputTokens", "cache_creation_input_tokens", "cache_write"),
|
|
}
|
|
}
|
|
|
|
// openclawInt64FirstOf returns the first non-zero int64 value found under any
|
|
// of the given keys. This supports field name variants across protocol versions.
|
|
func openclawInt64FirstOf(data map[string]any, keys ...string) int64 {
|
|
for _, key := range keys {
|
|
if v := openclawInt64(data, key); v != 0 {
|
|
return v
|
|
}
|
|
}
|
|
return 0
|
|
}
|
|
|
|
// openclawInt64 safely extracts an int64 from a JSON-decoded map value (which
|
|
// may be float64 due to Go's JSON number handling).
|
|
func openclawInt64(data map[string]any, key string) int64 {
|
|
v, ok := data[key]
|
|
if !ok {
|
|
return 0
|
|
}
|
|
switch n := v.(type) {
|
|
case float64:
|
|
return int64(n)
|
|
case int64:
|
|
return n
|
|
default:
|
|
return 0
|
|
}
|
|
}
|
|
|
|
// ── JSON types for `openclaw agent --json` output ──
|
|
|
|
// openclawEvent represents a single streaming NDJSON event from openclaw --json.
|
|
//
|
|
// Event types:
|
|
// - "text" — text output (text field)
|
|
// - "tool_use" — tool invocation (tool, callId, input)
|
|
// - "tool_result" — tool output (tool, callId, text)
|
|
// - "error" — error (text, or structured error object)
|
|
// - "lifecycle" — phase changes (phase: "error"/"failed"/"cancelled")
|
|
// - "step_start" — agent step begins
|
|
// - "step_finish" — agent step ends (usage)
|
|
type openclawEvent struct {
|
|
Type string `json:"type"`
|
|
SessionID string `json:"sessionId,omitempty"`
|
|
Text string `json:"text,omitempty"`
|
|
Tool string `json:"tool,omitempty"`
|
|
CallID string `json:"callId,omitempty"`
|
|
Input json.RawMessage `json:"input,omitempty"`
|
|
Usage map[string]any `json:"usage,omitempty"`
|
|
Phase string `json:"phase,omitempty"` // lifecycle event phase
|
|
Error *openclawError `json:"error,omitempty"` // structured error object
|
|
Message string `json:"message,omitempty"` // alternative error message field
|
|
}
|
|
|
|
// errorMessage extracts a human-readable error message from the event,
|
|
// checking multiple fields: structured error object, text, message, or fallback.
|
|
func (e openclawEvent) errorMessage() string {
|
|
if e.Error != nil {
|
|
if msg := e.Error.message(); msg != "" {
|
|
return msg
|
|
}
|
|
}
|
|
if e.Text != "" {
|
|
return e.Text
|
|
}
|
|
if e.Message != "" {
|
|
return e.Message
|
|
}
|
|
return "unknown openclaw error"
|
|
}
|
|
|
|
// openclawError represents a structured error in an openclaw event,
|
|
// compatible with PaperClip's error format (name + data.message).
|
|
type openclawError struct {
|
|
Name string `json:"name,omitempty"`
|
|
Data *openclawErrorData `json:"data,omitempty"`
|
|
Message string `json:"message,omitempty"`
|
|
}
|
|
|
|
func (e *openclawError) message() string {
|
|
if e.Data != nil && e.Data.Message != "" {
|
|
return e.Data.Message
|
|
}
|
|
if e.Message != "" {
|
|
return e.Message
|
|
}
|
|
if e.Name != "" {
|
|
return e.Name
|
|
}
|
|
return ""
|
|
}
|
|
|
|
type openclawErrorData struct {
|
|
Message string `json:"message,omitempty"`
|
|
}
|
|
|
|
// openclawResult represents the final JSON output from `openclaw agent --json`
|
|
// (the legacy single-blob format with payloads + meta).
|
|
type openclawResult struct {
|
|
Payloads []openclawPayload `json:"payloads"`
|
|
Meta openclawMeta `json:"meta"`
|
|
}
|
|
|
|
type openclawPayload struct {
|
|
Text string `json:"text"`
|
|
}
|
|
|
|
type openclawMeta struct {
|
|
DurationMs int64 `json:"durationMs"`
|
|
AgentMeta map[string]any `json:"agentMeta"`
|
|
}
|