mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-02 18:13:27 +02:00
a0ef1a43f3
* fix(daemon): diagnose silent OpenClaw npm shim failures on Windows (MUL-5422)
#6061 reported every OpenClaw task failing in execenv prep on Windows with
a bare `exit status 1` and no stderr, leaving the user nothing to act on.
An npm-installed `openclaw.cmd` is a batch shim that re-execs OpenClaw's
`openclaw.mjs` entrypoint through `node`, resolved from PATH. The daemon pins
`openclaw` to an absolute path, so the shim always looks correct — but that
interpreter lookup is a second, invisible resolution step that can fail on its
own. The reporter had to run their own subprocess experiments to find it.
Enrich the error instead of guessing at a fix: when a `.cmd`/`.bat` shim exits
non-zero with no stderr, report whether the interpreter resolves. Both
directions are useful — missing names the likely cause with a next step,
present clears PATH of blame and points at the remaining hypotheses (PATH
drift between the runtime `--version` gate and task prep, or a broken install).
Deliberately NOT included: rebuilding or freezing a Windows PATH. The
version-probe gate (probeBuiltinRuntime skips a provider whose `--version`
fails) and the prep helper both inherit the same daemon environment, so a
daemon that could not resolve `node` would never have registered OpenClaw at
all. That contradiction is unresolved, and a boot-time PATH snapshot would also
fight the MUL-4486 self-heal design, which re-resolves per attempt on purpose.
This change collects the evidence needed to settle it.
- Error text only; no control flow change, and real stderr still wins.
- PATH summarised as an entry count, never dumped, so daemon logs and pasted
bug reports carry no environment detail.
- Tests: shim detection (case, spaces, Unicode), both diagnostic directions,
out-of-scope no-ops (timeout, missing binary, native exe), and end-to-end
through execOpenclawCLI. A windows-tagged file reproduces a real npm shim
with and without node on PATH, and pins TEMP/TMP as not load-bearing — the
originally reported root cause, since retracted upstream.
- New scoped step in the existing ci.yml windows-execenv job.
Co-authored-by: multica-agent <github@multica.ai>
* fix(daemon): address review on OpenClaw shim diagnostics (MUL-5422)
Four must-fix items from PR review, each verified against the real behaviour
rather than assumed.
1. Timeout was misdiagnosed as a missing interpreter. openclawCLITimeout kills
the child via CommandContext, and a killed process surfaces as
*exec.ExitError ("signal: killed") — the same type a genuine exit 1
produces. The errors.As gate accepted it and appended "install Node.js",
sending users to fix something that was never broken. execOpenclawCLI now
attributes ctx.Err() before consulting the diagnostic. Confirmed locally:
`signal: killed`, errors.As(*exec.ExitError)=true, ctx.Err() set. The old
test passed context.DeadlineExceeded directly and so never saw the real
shape; replaced with a genuine CommandContext timeout regression on both
Linux and Windows.
2. The interpreter lookup did not match npm's. npm's cmd-shim template emits
`IF EXIST "%dp0%\node.exe" (...) ELSE ( SET "_prog=node" )`, so a co-located
node.exe wins over PATH entirely. Checking only LookPath reported "node is
not resolvable" for installs that actually run fine — confidently wrong,
which is worse than silence. Now resolves co-located `node.exe`/`node`
first, then PATH, and reports which. Wording is also conditional now
("if <name> is an npm-generated shim"): a batch extension does not prove npm
authorship, since MULTICA_OPENCLAW_PATH can point at any batch file.
3. The message leaked local paths off-box. On prep failure this text is not
log-local — it travels reportTerminalTask → Client.FailTask and is persisted
server-side as the task error, so an absolute Windows shim path uploads the
account name and install layout. Now reports only the shim's base name,
whether the interpreter resolved and from where, and a PATH entry count.
Never an absolute path, never PATH contents.
4. Windows CI was green without exercising the new code. The job log showed
`cmd.exe stderr DID reach Go's pipe` with `'node' is not recognized`, so the
missing-node case takes the existing stderr branch and the diagnostic never
ran — masked by an "either branch passes" assertion. That disjunction is
gone: the missing-node test now asserts the observed stderr behaviour
(disproving #6061's premise), and a new test drives a genuinely silent shim
to prove the diagnostic branch itself works on Windows. Also added Windows
coverage for the co-located interpreter and the timeout case.
The windows-tagged shim is now npm's real generated template rather than a
hand-simplified `node ...` one-liner, so the co-located branch is reproduced
faithfully instead of hidden.
Co-authored-by: multica-agent <github@multica.ai>
* test(daemon): make the OpenClaw timeout regression PATH-independent
The new timeout test stripped PATH (so a stray interpreter lookup would report
"missing") while its hanging shim invoked `sleep` through a PATH lookup. macOS
`sh` quietly falls back to a default PATH so this passed locally; dash on Linux
does not, so CI failed with `exit status 127 (stderr: sleep: not found)` — the
shim died instantly instead of hanging, and the assertion never saw a timeout.
Resolve `sleep` before PATH is stripped and embed it by absolute path, so the
shim needs no PATH of its own. Verified the failure mode and the fix directly:
`env -i /bin/sh -c 'PATH=/nonexistent; sleep 0.05'` reproduces
"sleep: command not found", while the absolute path runs fine with the same
empty PATH.
Windows is skipped here and covered by TestWindowsOpenclawShimTimeoutIsNotMisdiagnosed,
which has a real cmd.exe host and a System32 PATH that can resolve its own helper.
Co-authored-by: multica-agent <github@multica.ai>
* fix(daemon): bound execOpenclawCLI so its 5s timeout is actually enforceable
The new timeout regression exposed a real bug in the code it was testing, not
just a flaky test: openclawCLITimeout could not bound the call at all.
CommandContext kills only the direct child, and cmd.Output() blocks in Wait()
until the stdout pipe closes. Any grandchild that inherited stdout keeps the
call parked for its own lifetime. Verified on linux/dash: a shim whose child
slept 5s ran the FULL 5.01s against a 150ms deadline. With a WaitDelay backstop
the same case returns in ~2.17s.
This is not a hypothetical shape — it is precisely an npm shim on Windows
(cmd.exe → node), so a wedged node could stall task prep far past the 5s cap
that comment claims. detectCLIVersion already carries this exact backstop for
the `--version` probe for the same reason; execOpenclawCLI now matches it.
Also corrected the test comment: an earlier revision claimed a trailing
`exit 0` was needed to force the grandchild. Docker showed otherwise — dash
hangs either way and macOS reproduces neither, which is why CI caught this and
local runs did not. The comment now records the measured behaviour.
Verified in a linux/dash container (the CI platform, not just macOS): the full
execenv package passes with -race, and the timeout case takes 2.17s.
Co-authored-by: multica-agent <github@multica.ai>
* fix(daemon): drop the WaitDelay change and wrap the context error (MUL-5422)
Round-2 review: take option 1 — keep this PR to diagnostics and split the
timeout/process-tree work out.
The reviewer is right that WaitDelay traded a hang for a process leak, and I
had the mechanism wrong. Measured on linux/dash by recording the grandchild PID
and reading /proc/<pid>/stat at the moment Output returns:
no WaitDelay: elapsed 6.01s (6s sleep, 150ms deadline), grandchild state Z
with WaitDelay: elapsed 2.17s (60s sleep, 150ms deadline), grandchild state S
So without WaitDelay the call is hostage to the descendant's lifetime but no
live process is left behind — it returned precisely because the descendant had
exited. With WaitDelay the call is bounded but a live descendant survives. My
earlier claim that the orphan pre-existed was an artifact of a sleep duration
that happened to equal the return time.
Go's WaitDelay contract covers killing the direct child and closing our pipe
ends; it does not reap orphans. Closing this properly needs process-tree
ownership (Unix process group, Windows Job Object) so the deadline can terminate
the whole tree — and on Unix nothing else will, since
preparationProcessController.finish() is a no-op there (isolation_unix.go).
That is its own change with its own risk surface, so it is tracked separately
and openclawCLITimeout now documents the gap with the measurements rather than
shipping half a fix.
Also fixes the round-2 nit: the context branch %w-wrapped the process error
while printing ctxErr with %v, so errors.Is(err, context.DeadlineExceeded) was
false despite the text containing it. The context error is now the wrapped
cause and the process error is attached for diagnosis:
openclaw config file: context deadline exceeded (process: signal: killed)
Tests: the timeout cases no longer depend on WaitDelay and no longer leave a
live process — short sleeps keep them about attribution, which is what they are
for. Added an explicit errors.Is assertion for both DeadlineExceeded and
Canceled. Verified in a linux/dash container (the CI platform): full execenv
package passes with -race and `ps` shows no leftover sleep processes.
Co-authored-by: multica-agent <github@multica.ai>
* docs(daemon): correct two stale comments on the OpenClaw CLI timeout (MUL-5422)
Both nits from the third review. Comment-only; no code change.
1. openclawCLITimeout's doc contradicted itself — it opened with "caps ...
without letting a hung CLI stall task dispatch indefinitely" and then
explained that the deadline cannot actually bound the call. Reworded to say
what it is (a 5s context deadline) and to point at the gap rather than assert
a guarantee it does not provide. Also names MUL-5467 instead of the vague
"tracked separately".
2. The two timeout tests claimed a long wait would "leave a live process
behind". That described the reverted WaitDelay behaviour, not the current
code. Without WaitDelay, cmd.Output() returns only once the descendant has
closed stdout — its exit is what produces the EOF — so a long wait makes the
test slow, it does not leak. Re-verified on linux/dash after the fix: the
case takes 1.01s for a 1s sleep and `ps` shows no leftover process, and the
earlier PID probe recorded the grandchild in state Z at the return point.
Rebased onto c25a82eee.
Co-authored-by: multica-agent <github@multica.ai>
* docs(daemon): tighten OpenClaw timeout wording and pipe-EOF claims (MUL-5422)
Round-4 review nits. Comment-only; verified no non-comment line changed.
1. OpenclawConfigPrep.Timeout still said it "caps each CLI invocation", which
contradicts the openclawCLITimeout doc corrected last round. It now says it
sets the context deadline and points at that note. Fixed the same word in the
struct's own doc comment, which had the identical claim.
2. The two timeout-test comments equated "descendant closes the pipe" with
"descendant has exited". That holds for these helpers but is not a general
property — a process can close its pipes and keep running — so the comments
now scope the claim to the helper and say so explicitly.
Also corrected "stdout" to the output pipes os/exec manages for both stdout
AND stderr. Verified rather than assumed: with cmd.Stderr set to an
in-memory writer (as execOpenclawCLI does), a grandchild holding EITHER
stream parks cmd.Output() for its full 3s lifetime, while one holding
neither returns in 0s. So Wait genuinely depends on both.
No rebase this round: the branch is 1 commit behind main, that commit does not
touch execenv, and GitHub already reports MERGEABLE — not worth another forced
CI rerun.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>