mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-09 01:46:46 +08:00
admin-workspace-create2
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
686957e743 |
MUL-7271: test(server): cut slow Go test time further (follow-up to #8295) (#8318)
* test(server): reduce slow unit test waits Co-authored-by: multica-agent <[email protected]> * test(server): cut slow Go unit test time further Follow-up to the first pass on MUL-7271. Test-only changes plus a few package-private seams whose production values are unchanged. - Re-executed race-built test binaries no longer pay the race runtime's 1s atexit sleep (GORACE=atexit_sleep_ms=0 for child processes only). - pkg/agent: event-driven or shorter test-only bounds for handshake, drain and idle graces; redundant timeout variants merged. - daemon/execenv: git fixtures copied from a template repo built once; tests that re-ran the same worktree lifecycle folded into their supersets; ForkLock-guarded fake-CLI writes so parallel tests cannot hit ETXTBSY. - handler/service: cap and paging tests shrink their caps instead of seeding thousands of rows; lock waits probe pg_locks instead of sleeping; one shared pgxpool per service test binary; cheaper cron and RSA fixtures. - wecom: the phishing-link sweep reuses one parser and derives CRLF from LF with a sampled cross-check; fake sockets acknowledge instead of waiting out the ack timeout. - migrate: the migration corpus is parsed once; isolated DB tests run in parallel. Co-authored-by: multica-agent <[email protected]> * test(server): remove transport timeout unit tests Co-authored-by: multica-agent <[email protected]> * test(server): restore two non-timeout tests and make a bound load-bearing Follow-up to the review of #8318. All test-only; no production code changes. - daemon: restore the three read-liveness cases that the transport-timeout sweep took with it. They assert that a ping, a pong and an application frame each EXTEND the read deadline, so a healthy wakeup socket survives without traffic of our own — the deadline never fires on the happy path, which is why they are not transport-timeout tests. Verified they fail (all three) when the extension calls are removed from wakeup.go, and pass in 0.57s together. The pure "times out without peer traffic" case stays deleted. - pkg/agent: restore the parent-DEADLINE half of the opaque-env redaction pair. codex.go treats a parent deadline and a parent cancellation as one ending; only the Canceled arm was left covered. The fake now writes the secret to stderr before reading the handshake, so the check no longer races our own initialize write, and the budget is 2s against a measured 430-465ms launch. Verified it fails when the !contextEnded guard is removed. - pkg/agent: TestRunCollectQuietDoesNotWaitWhenTheAnswerIsIn had its bound widened to a literal 5s, which the shrunk 750ms drain grace can never reach — the test could no longer fail. It now raises the drain grace to 10s for its own duration and asserts under 3s: healthy runs take 0.19-0.44s, and a broken short-circuit takes 10.19s and fails. - wecom: gofmt the import block in relay_settle_budget_test.go. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Sol-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> Co-authored-by: J <[email protected]> |
||
|
|
3551e72e76 |
MUL-7232 feat(daemon): add dsh desktop(self-hosted) support (#8237)
* feat(daemon): discover the DSH CLI bundled in DeepSeek Harness Desktop DeepSeek Harness Desktop ships its CLI inside the .app and never installs `dsh` onto PATH, so a GUI-launched daemon missed it on exec.LookPath — and the login-shell sweep could not rescue it either, because no rc file has a reason to know that path. Only a hand-made symlink made dsh visible at all. Add the app-bundle fallback the Codex provider has had since #5205. The candidate is a Node script rather than a native binary, so it counts only while it is executable: an entry discovered without the executable bit would be advertised as a healthy runtime and then fail on every spawn. The Multica runtime profile gate still applies — a bundled CLI without the profile is not a runtime. * fix(daemon): report and install the missing DSH runtime profile probeAgentCLIs is documented as pure discovery, but dsh's runtime-profile gate lived inside it — one layer above the verdict machinery that turns a drop into a /health skipped_agents reason and a daemon-log line. A dsh binary without the profile therefore vanished from the runtime list with nothing anywhere saying why, which is the shape the DSH reports in #7721 and #6936 run into. Move the gate down to probeBuiltinRuntime and give it its own verdict: dsh is now discovered like any other CLI, and failing to use it is a reported, actionable drop. The verdict is deterministic, so it demotes immediately rather than waiting out a confirmation window — it is a fact about the CLI's environment, not about bytes mid-write. The executable is checked first, because probeDshMulticaProfile cannot tell "the profile refused" from "there was nothing to run", and would otherwise blame a missing profile for a path that vanished under us. Reporting is only half of it: DSH is the one built-in whose CLI is not enough on its own, and no other provider needs anything installed into the agent's own home. MULTICA_DSH_PROFILE_BUNDLE now names what to install — an npm spec, a directory or a tarball, candidates tried in order. Unset stays the default and means "do not touch the user's DSH installation"; there is nothing to point it at yet either, because Multica's own bridge is unpublished (#6936). DSH Desktop ships the pnpm that `dsh plugin` forwards to inside its own runtime-commands directory and injects it only for the processes it spawns, so the install prepends that directory to PATH. Without it the command fails with "pnpm not found on PATH" on exactly the machines this is for. The install runs once per daemon, and the discovery loop is kicked when it finishes so dsh comes online immediately instead of at the next tick. * fix(daemon): keep a registered dsh runtime in step with its runtime profile Two states have to agree: the Multica runtime profile is installed, and a dsh runtime is registered. Neither disagreement corrects itself, and both leave the server routing work into a CLI that cannot start. When the profile is removed, dsh stays discovered and keeps its runtime, so nothing else in the discovery loop looks at it again. Observed in the field: a profile removed a minute after startup stayed live for three minutes and a chat task failed against it. When a profile is installed by hand, the runtime is missing but the converge backoff has already doubled to agentConvergeMaxBackoff (30m) over the rounds that could not register it, so the fix stays invisible for up to half an hour. Compare the two states on every tick and force a round on disagreement, in either direction. A round forced for a registered provider has nothing missing to register, so convergeRuntimeRegistrations now consumes the verdict map it used to discard — safe because demoteUnusableRuntimes owns the claim barrier and the seq-stamped hold, and stays the single owner of demotion. The profile's presence is a stat of the manifest DSH itself looks for, so the check costs one syscall per tick; the authoritative --probe stays inside the probe round the check forces. * docs(runtime): document the DSH runtime-profile variables The two variables this branch adds were undocumented, and the install guide still told readers to run `dsh plugin --profile multica add <bundle>` without ever saying what <bundle> is — the hole #6936 was filed against. Document MULTICA_DSH_PROFILE_BUNDLE and MULTICA_DSH_PLUGIN_PATH next to the other MULTICA_DSH_* variables, and name the bundle in the DSH setup step: what implements the protocol, where the bridge lives, and that the daemon reports the missing profile on /health rather than registering a runtime that cannot start. All four locale files, so the translations do not drift. * refactor(agent): export the DSH stdio protocol version The daemon's runtime-profile probe accepts or rejects a profile on this exact number: one answering `--probe` with a protocol this backend does not drive must not be registered. That decision was about to be made against a second copy of the constant in internal/daemon, and a second copy is one that can drift from the code actually speaking the protocol. Export it so the probe reads the number from its only honest source. No behaviour change: the value is still 1. * fix(redact): mask credentials carried in URL userinfo The connection-string rule covers postgres, mysql, mongodb, redis and amqp only, so `https://user:token@registry/...` passed through untouched. That shape is not hypothetical for agent output: npm and pnpm accept an authenticated registry URL as a package spec, and both the package manager and the registry echo the request URL back in their error text, which is how it reaches a log line without anyone logging the spec itself. Add a scheme-agnostic userinfo rule. The scheme and host survive — which registry refused the request is the part an operator reads, and it is not the secret — and userinfo requires both a colon and an `@` before the next path separator, so `https://host:8443/p`, `ssh://git@host/p` and a bare `a:b` in a path are all left alone. Tests cover both directions. * fix(daemon): decide the DSH runtime-profile verdict on evidence probeDshMulticaProfile returned a bare bool, so a timeout, a non-zero exit, unparseable output, a future protocol version and a genuinely absent manifest all collapsed into "the profile is not installed" — a verdict that demotes a live runtime and, with a bundle configured, installs over the user's DSH home. This rebuilds the verdict layer around what was actually observed. Classification. The probe now returns one of OK / missing / incompatible / unavailable. The frame decides, not the exit status: a profile that printed a valid answer has answered, whatever the process did on its way out — which also stops a process-tree lifecycle error from being read as a protocol failure. With no usable frame, a cancelled ROUND stays transient (the daemon is shutting down; that says nothing about the profile) and otherwise the manifest decides. A manifest that IS present keeps every failure transient, which is what protects a working install during an upgrade. Timeouts are not transient by themselves. Asked for a profile it does not have, DSH does not refuse `--probe` — it hangs, printing nothing, until something kills it. Measured: still running after 10s, zero bytes on both streams. So a timeout is the ordinary way a missing profile presents, and treating it as transient made the profile undetectable on exactly the host the automatic install exists for. The manifest is the same file DSH's own loadProfile consults before refusing to boot, so a confirmed-absent one is evidence. Order. The profile probe now runs only after version detection succeeds. exec.LookPath is far too weak a proxy for "this CLI works": on Windows the CLI is a .cmd shim it resolves happily and which exits 9009 — cmd.exe's "command not found" — when what it forwards to is missing. Probing the profile first reported such a machine as missing its runtime profile, a repair for a problem it did not have. Demotion. An incompatible profile is reported and never demoted: this daemon can be the stale side of a protocol skew, and tearing down a live runtime because THIS build is behind is a destructive answer to a version question. The confirmation window that gated the not-executable verdict now gates every condemnable verdict, keyed per verdict so a provider moving between states starts a fresh clock rather than inheriting one earned by another problem. Convergence. A registered dsh whose profile was removed forces a round on every tick, because condemning it takes two of them and nothing else schedules the second: with dsh still registered nothing is missing a runtime, so an unforced tick returns without probing at all. The mirror direction — a profile with no runtime — stays throttled to one forced round, since no round is guaranteed to resolve it and forcing would bypass agentConvergeMaxBackoff forever. Offline cause. A missing profile reaches the server as a structured `dsh_profile` code rather than a bare offline, and carries whether the daemon is installing the profile right now — the one case where queueing is right. That claim is withdrawn if the install gives up: the runtime row is written once, by the demotion that also removes the runtime from the index, so nothing later would correct it and the work would queue forever behind an install that already stopped. Provisioning. Runs under the daemon's lifetime through processtree, so the pnpm/node descendants cannot outlive a shutdown and keep writing into the DSH home. Success is the manifest on disk, not the package manager's exit status, and a candidate that exits 0 without producing a profile is a failed candidate so the next one still gets its turn. The bundle spec no longer reaches a log line — an npm URL can carry credentials and a path a username — and captured output is redacted and bounded on a rune boundary. The PATH merge matches the variable case-insensitively, because Windows spells it `Path` and an exact `PATH=` miss left the package manager with only the plugin directory under Go's case-insensitive de-duplication. Startup. A daemon that registered nothing continues when it has just begun installing the profile itself. On a host whose only provider is a dsh without one, failing here is a deadlock rather than a fail-fast: the probe starts the install, the error kills the process milliseconds later, the install's process tree dies with it, and the next start repeats all of it. Nothing else is forgiven. Because such a daemon tracks no workspace, a finished install picks the workspace up before converging. Repair. The reported repair names the missing package and carries no command: the install needs a bundle only the operator can choose, and Repair.Command is rendered to the user inside a shell fence as the line to run, so a placeholder there is a paste-and-fail instruction. Tests. The classification matrix, "manifest present + timeout / non-zero exit / malformed output / future protocol ⇒ no demotion, no provisioning", the 9009 shim, the forced-round sequence, the withdrawal, the startup exception, the empty install, and the case-insensitive PATH merge. Two existing tests read the developer's real ~/.dsh and one fake dsh could not answer `--version` at all — both now pinned to the test's own directory and to the shape the feature is actually about. * feat(daemon): discover the CLI shims DSH Desktop generates The macOS fallback pointed at one path inside the app bundle, and Windows had none at all — every candidate there was a /Applications path that cannot exist, so a Windows host running DSH Desktop reported no dsh whatsoever. Replace it with one layout that serves both platforms. The CLI is not in the install directory. DSH Desktop generates shims under its own per-user data directory — ~/Library/Application Support/DSH Desktop on macOS, %APPDATA%\DSH Desktop on Windows — and on Windows that is roaming application data even though the app installs to Program Files or %LOCALAPPDATA%\Programs. One lookup therefore covers both install modes, since a per-machine install still generates these shims per user. Payload directories are content hashes (and, under host-commands, a hash plus a generation uuid), so they are enumerated rather than composed. Only directories holding a bin/ are accepted, so a stray file or half-extracted download is skipped; modification time orders them so an upgrade's new payload wins over an abandoned one, and the name breaks ties because ReadDir order is not a promise and the daemon must not pick a different CLI on different rounds. host-commands outranks cli, and that is load-bearing: nothing here executes a candidate, so a broken-but-present shim is indistinguishable from a working one at this layer — and on the install that prompted this, the cli shim was exactly that, resolvable by exec.LookPath and exiting 9009. Ranking is the only lever. Generated shims also outrank the macOS app-bundle script, which is entered through `#!/usr/bin/env node` and so only works where node happens to be installed; a shim runs the Desktop app's own runtime by absolute path and needs nothing on PATH. That gap was latent on macOS too. executableCandidate had to learn the platform difference for any of this to work: Go synthesizes a regular file's mode from the read-only attribute and never sets an executable bit on Windows, so the Unix test rejected every candidate and the whole fallback would have been dead code there. Windows uses exec.LookPath, the same check the launcher makes, so a candidate accepted here is one that can actually start — which is also why the Node script has no Windows counterpart and .js never appears in that list. Only default locations are covered, and nothing reads an install receipt or the registry, so a relocated install still needs MULTICA_DSH_PATH. The path builders take goos/env/home as arguments rather than reading runtime.GOOS and os.Getenv, so the Windows layout is asserted from any host — the alternative leaves it testable only on Windows, which in this package has meant untested. * fix(server): refuse a missing DSH runtime profile under its own reason A runtime taken offline for a missing profile reached the server as a bare offline, so runtimeVerdict classified it as waitable/runtime_offline and assignments and @mentions bound to that agent queued forever — even though, with no automatic install configured, that wait never resolves on its own. That contradicts the semantics settled in MUL-6164: when a human has to intervene, refuse the trigger and say how to fix it. Blocked, but not as runtime_unusable. The two causes have opposite repairs: an unrunnable CLI is reinstalled, while a CLI missing its runtime profile is working perfectly and reinstalling it changes nothing. Sharing one code meant every client told DSH users to reinstall a CLI that was never the problem — the chip and toast said "reinstall it there", and the durable system comment left on the issue opened with "its CLI is installed but cannot be executed on that machine", blamed a blocked npm postinstall, and rendered `dsh plugin --profile multica add <bundle>` inside a bash fence as the line to run on that machine. A placeholder presented as a command is the shape of instruction people paste verbatim and then report as broken. So: a new runtime_profile_missing reason code, carried through readiness, dispatch and the localized copy in all four languages. The notice branches on the reason rather than on whether a repair command happened to be present, and the profile text names what is missing, says the CLI is fine, and points at the two ways to supply a real bundle without printing a command to copy. An install the daemon is running right now is the exception: that wait does end by itself, so the work queues instead of being refused. RuntimeBlockedNeedsNotice replaces three inline comparisons, because a code added to one admission path and not the others is a trigger that vanishes with no trace on exactly the surfaces that have no response for the user to read. * docs(runtime): document DSH profile provisioning and shim discovery Automatic provisioning is a new, persistent feature surface, and it had no product surface: an admin had no documented way to configure it, and the one paragraph that existed attached the install semantics to the wrong variable — the sentences describing what MULTICA_DSH_PROFILE_BUNDLE does followed the MULTICA_DSH_PLUGIN_PATH sentence, so "the variable is unset by default and the install it enables is bounded" read as being about a variable that enables no install, and PLUGIN_PATH was then described twice. Environment variables, in all four languages: the install semantics move onto the variable they belong to, and now state the bounds — at most once per daemon lifetime, only on a confirmed-absent profile, failures logged with the package manager's own output and retried only after a restart, a timeout or an unsupported protocol version reported and never installed over. Plus how to diagnose (search the daemon log for `DSH runtime profile`) and how to roll back (delete $DSH_HOME/profiles/multica). MULTICA_DSH_PLUGIN_PATH is described once, and Windows and Linux are named as the platforms with no bundled pnpm directory, where the daemon uses whatever is on PATH unless this points at one. Install guide, in all four languages: what to actually put in the variable. Multica's bridge is not on a public npm registry yet (multica#6936), so a self-hosted team builds it from the repository and points at the build directory, or at the tarball npm pack produces when the daemon host is not the build host; a package name becomes the simplest option once it is published. With a warning that belongs on this value specifically — it is installed into every daemon host's DSH home without further confirmation, so an unvetted third-party package is a supply-chain decision, not a convenience. Also documents where the daemon looks for a Desktop-managed CLI, including the detail that trips people on Windows: it is roaming application data, not the install directory. And the one exception to "the daemon must detect at least one built-in CLI before it will start" — a daemon that has just begun installing the profile itself starts with no runtimes, because otherwise a host whose only tool is a DSH needing that profile could never finish the install. * fix(daemon): find DSH Desktop's bundled pnpm on Windows too The automatic install shells out to `dsh plugin`, which forwards to pnpm, and the daemon inherits no path to the one DSH Desktop ships. The lookup composed a single macOS path and was commented as macOS-only, so on Windows the install failed with "pnpm not found on PATH" unless the operator set MULTICA_DSH_PLUGIN_PATH by hand — on exactly the hosts the automatic install exists for. Both platforms bundle it under runtime-commands in the app's own per-user data directory, but not in the same shape: macOS 2.0.5 keeps a flat runtime-commands/bin, while a Windows install keeps runtime-commands/generations/<id>/bin. Composing either path leaves the other broken, so the lookup now shares the enumerator with CLI-shim discovery and searches both shapes at both levels. A Desktop version that moves between them is then a version this daemon keeps working with rather than one it silently stops supporting. Sorting is pooled per tier rather than per directory. Doing it per directory made the answer depend on the order ReadDir returned DSH profile names, which is the same non-determinism the name tie-break exists to remove: the newest generation has to win even when it belongs to a profile that sorts later. An existing test caught this while the enumerator was being extracted. An explicit MULTICA_DSH_PLUGIN_PATH now replaces the search instead of merely being tried first. An operator who pinned a directory is bypassing the bundled pnpm on purpose, and quietly falling back to it would defeat that. Documented in all four languages, where the previous text stated the opposite — that Windows has no bundled directory. * fix(server): bound the in-flight DSH install claim instead of trusting it `installing` tells the server to queue triggers rather than refuse them, so a claim that outlives its install recreates exactly the failure the structured reason was added to prevent: work queued behind something that already stopped. The withdraw cannot be made to guarantee that. Three paths leave the claim on the row. The verdict reads dshInstallInFlight when it is built, but the wait is recorded later, when the demotion knows the runtime ids — an install that fails in between finds an empty list, and the `installing` written afterwards is never withdrawn. The corrective Deregister is best-effort, and during shutdown it runs on an already-cancelled lifecycle context. And a daemon restarted mid-install does not track the row at all. Bound the claim instead. SetAgentRuntimeOfflineWithReason stamps updated_at, and the install it describes is itself bounded by the daemon's provisioning timeout, so honouring `installing` only inside a window comfortably longer than that covers a real install while capping every way the claim can be left behind. A row with no usable timestamp cannot be bounded and is not honoured: refusing during a real install costs one retry the notice already asks for, while honouring a stale claim costs work that queues until it expires. The withdraw stays as the fast path — this is the floor under it. * refactor(daemon): scrub URL credentials in provisioning output, not globally The rule was added to pkg/redact's shared pattern list, where redact.Text runs over every agent's task messages, comments and error text — so a fix aimed at one package manager's output changed what every provider emits. The gap is real and still worth closing in that list, but widening a shared filter is its own change with its own blast radius, and it does not belong in a PR about DSH. Move it to dshProvisionOutput, which is the only place this PR needs it: a bundle spec can be an authenticated npm URL, and both the package manager and the registry echo the request URL back in their error text. The shared redactor still runs after it for the token shapes it already knows. Behaviour for the provisioning log line is unchanged; behaviour for every other provider returns to main's. Same assertions as before, moved next to the code they now cover, plus the false-positive cases — a port, an scp-style git remote, a bare colon in a path — and one asserting the host survives, since which registry refused the request is what an operator reads and is not the secret. * fix(daemon): let a converge round condemn dsh only convergeRuntimeRegistrations started consuming the verdict map it used to discard, so that a round forced by a DSH profile mismatch could reach demotion — a registered dsh whose profile was removed leaves nothing missing a runtime, so the round has no registration left to do and would otherwise return having done nothing. But it acted on every provider's verdict, not just dsh. On main, demotion is reached from refreshAgentVersions alone, on its own ten-minute cadence, and that is the schedule every provider's verdict was tuned against. Claude Code, Codex and the rest would have moved onto a different one as a side effect of a DSH fix — a behaviour change for providers this PR has no reason to touch. Narrow it to dsh. It is the only built-in with a precondition that can change without any version changing, which is why it is the only one that needs a round it can be forced into; every other provider keeps main's behaviour exactly. |
||
|
|
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]> |
||
|
|
b30a3ae55c |
MUL-6164 feat(runtime): take an unusable agent CLI offline and refuse its triggers with the fix (#6966)
* fix(daemon): take a runtime offline when its CLI cannot be executed (MUL-6164) A CLI that cannot exec was classified as a transient probe failure, so the runtime stayed online and every task it claimed died at fork/exec. The user in MUL-6164 hit the same error once per task with no signal that the runtime was the problem — an npm placeholder stub passes exec.LookPath, so nothing else the daemon checks can tell it apart from a working install. ENOEXEC (and its Windows spelling, ERROR_BAD_EXE_FORMAT) is a fact about the bytes on disk, not a hiccup: it will reproduce until someone reinstalls. Give it its own probe verdict and route it down the track below-minimum already uses — claim barrier, seq-stamped hold, deregistration — so the provider goes offline with its reason instead of collecting failed tasks. Recovery needs no new machinery: a demoted provider has no runtime, which puts it back in providersMissingRuntimes, and the first healthy probe clears the hold and lets converge register it again, with no restart. One thing the below-minimum track did not need: a confirmation window. The repair we tell users to run overwrites the bin entry in place, so a probe landing mid-copy sees a truncated file. Demotion therefore requires two sightings a window apart; a genuinely broken install pays one extra probe round, and a spurious verdict would in any case recover on its own. detectBuiltinRuntimes' second return value is now "providers condemned this round, with the evidence against them" rather than "below-minimum versions" — same shape, both kinds, and the value is the reason /health already shows. Co-authored-by: multica-agent <[email protected]> * feat(runtime): refuse triggers for an unusable agent CLI instead of queueing them silently (MUL-6164) Taking the runtime offline stopped the per-task failures, but it replaced them with silence: the next @mention or assignment queued as if the machine were merely asleep, and nothing ran for two hours until queued_expired. Waiting only makes sense when waiting is a plan — a sleeping laptop comes back on its own; a CLI that cannot be executed does not. dispatch/reason.go already draws exactly that line between runtime_offline ("bring the machine back, queued work waits") and agent_runtime_required ("nothing will ever claim this"), and warns that collapsing the two sends users to fix the wrong thing. This adds the third member of that family, runtime_unusable, and routes the new verdict to it end to end: - The daemon reports WHY on deregistration — a stable code plus the repair command read from the package's own manifest, since only that side knows the path. Stored on agent_runtime.metadata, which registration overwrites wholesale, so a repaired runtime drops the explanation structurally. - AgentReadiness now returns a verdict (available / waitable / blocked) instead of a bool plus prose, and the direct-agent trigger paths start consulting it — blocking ONLY on the blocked verdict, so an offline machine still queues. agentReadinessReasonCode, which re-derived the same code from agent state, is gone: one source, no drift. - Every path that refuses without a human watching the response leaves the reason on the issue: an assignment, or another agent's @mention. The interactive surfaces already carry the code back to the composer, which now localizes it in all four languages. Below-minimum keeps today's behaviour deliberately. It is equally futile to wait on, but changing when THAT refuses a trigger is a separate product decision. Co-authored-by: multica-agent <[email protected]> * fix(runtime): keep the unusable verdict through a late register, and repair Windows (MUL-6164 review) Two ways the refusal path failed to reach the user. The verdict could be erased by an older answer. A register whose request was still in flight across the demotion upserts the runtime row afterwards, and that upsert overwrites metadata wholesale — including the reason just stored. The local hold refuses the response correctly, but the cleanup that took the revived row offline again sent no reason, so the server ended up "offline, cause unknown". Every admission path reads that as "the machine will come back" and queues, which is exactly the behaviour this change exists to remove. The demotion record now keeps the structured cause and the revived-row cleanup re-attaches it per row. The regression test drives the real interleave — a register held inside the server handler across the verdict — and fails without the fix. The repair command was POSIX-only. Windows had just gained detection (ERROR_BAD_EXE_FORMAT), so a Windows user was correctly refused and then handed `cd 'C:\...' && node install.cjs`: cmd.exe treats the quotes literally and PowerShell 5.1 has no `&&`. The command is now rendered for the shell of the machine that reported it — Set-Location on two lines for PowerShell, with PowerShell's own quote escaping — and carries that shell so the notice fences it truthfully instead of labelling everything bash. Also corrected the notice's own docs: it is written for every author, not only unattended ones. The chip and toast carry the reason code but not the repair command, so a human needs the durable copy too. The behaviour was already this; the comment claimed otherwise. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]> |