mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-10 10:26:46 +08:00
admin-workspace-create2
84
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Jiayuan Zhangandmultica-agent
|
231dedb552 |
MUL-8034: chore: clean historical comments across the codebase (#9274)
* chore: clean historical comments (MUL-8034) Co-authored-by: multica-agent <[email protected]> * docs: preserve pending Composio expiry contract Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-agent <[email protected]> |
||
|
|
8a2f518d54 |
fix(agent): MUL-7781 / MUL-7793 interrupt Codex turns after repeated Guardian 429s (#8934)
* fix(agent): stop Codex turns after repeated Guardian 429s * fix(agent): recognize Codex Guardian rate-limit events Use the app-server wire method and preserve semantic activity. Distinguish reviewer failures from policy denials, deduplicate recent reviews, and explain manual recovery after interruption. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Yin <[email protected]> Co-authored-by: Sol-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
mrlonely
|
b2561aad61 | fix(codex): exclude historical turns from resume responses (#9020) | ||
|
|
93ae552ba9 |
feat: add messages to active Claude Code and Codex runs (MUL-7565) (#8682)
* feat: add live run supplements (MUL-7565) Co-authored-by: multica-agent <[email protected]> * fix: address MUL-7565 review feedback Co-authored-by: multica-agent <[email protected]> * fix: resolve MUL-7565 supplement panel and receipt review Co-authored-by: multica-agent <[email protected]> * refactor: remove MUL-7565 supplement ordinals Co-authored-by: multica-agent <[email protected]> * fix: close supplement composer after run remount * fix: exclude task supplements from completion reconciliation * fix: build a persistent binary for local daemon launches * feat: support Claude task supplements through SDK hooks * refactor: simplify task supplement delivery * test: remove redundant task supplement coverage * fix: remove redundant supplement delivery notices * refactor: extract local daemon launcher fix to its own PR * fix: preserve ordinary comment size compatibility * fix: accept empty task start acknowledgements * fix: bound task start response decoding and retries --------- Co-authored-by: Forge-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> Co-authored-by: yushen <[email protected]> |
||
|
Multica Eveandmultica-agent
|
8c9f865e3e |
MUL-7480: Revert comment steering feature (#8648)
* Revert "MUL-7480: Link delivered steers to final replies (#8623)" This reverts commit |
||
|
|
8ce795b007 |
MUL-7480: Steer active agent turns from comments (#8605)
* feat: steer active agent turns from comments (MUL-7480) Co-authored-by: multica-agent <[email protected]> * fix: make comment steering hint driven Co-authored-by: multica-agent <[email protected]> * fix: harden active comment steering races Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Forge-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
83c04ceae0 |
test(codex): pin first-delta visibility contract (MUL-7465) (#8539)
Co-authored-by: Sol-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
043821ab77 |
MUL-7465 fix(chat): stream Codex deltas end to end (#8536)
* fix(chat): stream first agent text sooner (MUL-7465) Co-authored-by: multica-agent <[email protected]> * fix(codex): make delta streaming lossless (MUL-7465) Co-authored-by: multica-agent <[email protected]> * fix(codex): retain mismatched pending deltas (MUL-7465) Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Sol-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
importcpp
|
a843b44a48 |
MUL-7382 docs(agent): clarify token usage normalization contract (#8423)
Follow-up to the review notes on #8416. Comments only — token arithmetic, fields, JSON tags and test assertions are unchanged. State the actual rule on the shared TokenUsage contract: only a breakdown already included in a total must not be added to that total again, rather than banning provider breakdowns as a category. Keep the four-bucket mutual-exclusivity sentence normative. Explain why codexRawTokenUsage retains ReasoningOutputTokens — it mirrors the provider payload and keeps resume-baseline subtraction symmetric — and record that no downstream consumer reads that breakdown. Restore the normative sentence on the usage contract test and scope its coverage claim to the Codex and Claude cases it actually exercises. |
||
|
|
a00ebbb952 |
fix(codex): MUL-7381 synchronize codex thread ID access (#8428)
The task goroutine published c.threadID after thread/start or thread/resume returned, while the stdout goroutine was already reading the same field in isNotificationFromOtherThread. That filter runs ahead of the current-turn gate, so the gate's state could not serialize the read, and the app-server routinely emits a notification immediately after the thread setup response. Guard the field with its own RWMutex and route the write plus all three reads through setThreadID/getThreadID, mirroring how turnID is already handled. Behaviour is unchanged: the filter still compares a single snapshot and keeps the existing no-current-thread, missing-threadId, subthread and resume-history cases. Fixes #8422 Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
importcppandimportcpp
|
cf52ba33cc |
fix(codex): MUL-7374 avoid double-counting reasoning output tokens (#8416)
Codex reports reasoning tokens as a breakdown inside `output_tokens`, but both the app-server event path and the rollout JSONL fallback added them on top of the output total, inflating recorded output usage and the usage-based cost estimate. Consolidate both readers onto one shared conversion that keeps the provider's inclusive output total, preserving cache-bucket exclusivity, resume baselines and duplicate-snapshot handling. Add cross-provider contract tests covering both Codex sources and both Claude result formats. Verified against a live codex app-server session (codex-cli 0.144.6): the provider's own totalTokens equals input + output with reasoning not added, and the recorded buckets now sum to that total exactly. Previously persisted rows are not rewritten. Co-authored-by: importcpp <[email protected]> |
||
|
|
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]> |
||
|
mrlonelyandL
|
074f5ab237 |
MUL-6834: fix(codex): preserve token usage on cancelled turns (#7753)
* fix(codex): preserve usage on cancelled turns * fix(codex): bound cancelled turn cleanup * test(codex): measure real turn interrupt latency * fix(codex): make cancelled usage accounting idempotent * fix(codex): separate cache-write usage --------- Co-authored-by: L <[email protected]> |
||
|
|
ca47495fcc |
MUL-7053 fix(daemon): explain the codex retired-compaction 404 (#8032)
* MUL-7053 fix(daemon): explain the codex retired-compaction 404
Codex fails a turn with:
Error running remote compact task: unexpected status 404 Not Found:
{"detail":"Not Found"}, url: .../codex/responses/compact, cf-ray: ...
and stops there. The line that chose that URL — `remote_compaction_v2 =
false` under `[features]` in ~/.codex/config.toml — appears nowhere in the
text, so the error names no file, no key and no remedy. Codex before 26.901
ignored that value, so a line pasted long ago as a workaround for an
unrelated defect starts failing on upgrade without the user touching
anything. Nor does it resolve on its own: the conversation stays above the
auto-compact threshold, so every following turn retries the same retired
call. The reporter in GH #8000 had to open an issue to learn the fix was
deleting one line.
Recognise the failure and append the remedy to the error text, the same way
the Hermes "no LLM provider configured" hint works — a constant, kept clear
of every phrase taskfailure.Classify and the resume guards match, since it
is persisted in agent_task_queue.error and re-scanned there indefinitely.
The raw error still reaches the user unchanged; the hint rides behind it.
The compaction marker alone does not fire it: a transient 5xx or a rate
limit on the same call would send users to edit a file that was never the
problem. It needs the 404 status, or the legacy `responses/compact` path,
which is evidence of the stale flag whatever status came back.
Text only, deliberately. This does NOT retire the session: unlike the
resume-unsafe failures classified alongside it, the thread is recoverable
and recovers by itself once the flag is gone, so dropping the session
pointer would discard the conversation the one-line fix brings back.
Not doing what PR #8019 proposed — forcing `--enable remote_compaction_v2`
in daemon-managed sessions. That override carries no Multica semantics
(unlike fast_mode, which honours a tier the user paid for), upstream has
stated it will fix the flag, and it would then be permanent dead code that
blocks anyone with a legitimate reason to disable it.
Upstream: openai/codex#42468
Co-authored-by: multica-agent <[email protected]>
* MUL-7053 docs(codex): state when the compaction hint can be deleted
The hint is scoped to a defect upstream intends to fix, so the question it
has to answer for a future reader is what happens then. Under either
upstream remedy — Stage::Removed or a 404 fallback — the predicate stops
matching and nothing has to be unwound: it overrides no config and blocks
nobody from disabling the flag deliberately. Record that, and the contrast
with the override declined in #8019, next to the code rather than in the PR
thread.
Co-authored-by: multica-agent <[email protected]>
* MUL-7053 fix(codex): key the compaction hint on the route, not the status
Two ways the hint could fire on a case its remedy does not fix, both found
in review of #8032.
Requiring only the compaction marker plus a 404 was wrong about which route
failed. Codex wraps v2 compaction failures in the same "Error running remote
compact task" prefix (compact_remote_v2.rs), and a v2 stream can answer 404
too — so a user whose setting was already correct would be told to go turn
it off. Require the retired /responses/compact path instead: it cannot be
produced by the v2 route, and a compaction request reaching it means the
legacy route was selected whatever status came back. The status then stops
mattering, so a non-404 on that path matches too.
That gives up the case where Codex reports the failure without the url tail.
Nothing else in that text identifies the route and the two errors are
otherwise indistinguishable, so the choice was between missing some cases
and misdirecting the users whose config was never the problem. A missed hint
costs what today already costs; a wrong one spends someone's afternoon.
The hint also asserted the shared config file was the source. It is the
common case and the only one the upstream reports mention, but the launch
arguments reach the same state: codexBlockedArgs blocks only --listen, and
the sole feature-conflict stripping here is scoped to fast_mode under an
explicit service tier, so `--disable remote_compaction_v2` and
`-c features.remote_compaction_v2=false` survive from custom args, daemon
extra args, and a custom runtime profile's prefix alike. Anyone in that case
would have followed the hint to edit a line that is not there. The text now
names both sources — it stays a constant, which is load-bearing for the
resume guards, so it cannot narrow itself to the source that actually
applied. TestCodexLaunchArgsCanSelectTheRetiredCompactionRoute pins that the
argument half describes something real, and fails if a later change starts
stripping those spellings.
Also drops the "before 26.901" / "years ago" framing, which mixed the
Desktop build numbers with CLI versions and overstated how long the
workaround had been circulating.
Co-authored-by: multica-agent <[email protected]>
* MUL-7053 fix(daemon): stop the compaction hint contradicting itself
Adding the launch arguments as a second source left the closing line behind.
The hint sent people to check custom_args, a daemon's extra args, or a
profile's fixed args, and then asserted nothing on the Multica side needed
changing — stranding exactly the users that second source was added for,
which is the same dead end the addition was meant to close.
Scope the exclusion to the one target that is genuinely wrong to edit: the
per-task copy, which is rebuilt from the shared config every run, so an edit
there is discarded. Both real sources are simply re-read on the next run, so
that is what the text now says.
Also covers the third argv region the hint names. The prior test exercised
ExtraArgs and CustomArgs, but a custom runtime profile's fixed args reach the
child through the launch prefix, which has its own filters —
FilterLaunchPrefix, the managed-mcp_config override strip, and the priority
tier's fast_mode strip. The setting survives all three; asserting it is what
makes the hint's third source a checked claim rather than a comment.
Co-authored-by: multica-agent <[email protected]>
---------
Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
|
||
|
mrlonelyandL
|
108407088c |
MUL-6836: fix(codex): allow explicit standard speed (#7756)
* fix(codex): allow explicit standard speed * fix(codex): negotiate explicit standard support --------- Co-authored-by: L <[email protected]> |
||
|
mrlonely
|
2ebe0ae327 |
MUL-6862: fix(codex): widen thread setup handshake budget (#7790)
* fix(codex): widen thread setup handshake budget * fix(codex): preserve handshake timeout overrides |
||
|
VvVandweiweiwei
|
7d128b5a45 |
fix(daemon): own the openclaw CLI process tree so its deadline is enforceable (MUL-5467, MUL-5630, MUL-6736) (#6275)
* 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]> |
||
|
Roman Yufimov
|
620f99d8ce | fix(codex): wait for authoritative turn completion (#7554) | ||
|
|
46b5d9e6d1 |
MUL-6658 fix(agent): own the runtime process tree on every backend (#7522) (#7531)
* fix(agent): own the runtime process tree on every backend Cancelling a task on Windows left the agent running. The daemon marked the task cancelled, revoked its mat_ token, and killed what it thought was the CLI — but on Windows the direct child is usually a cmd.exe shim and the real CLI is already a grandchild, so it survived. One reporter's orphan kept working for 40 minutes, read the daemon owner's profile PAT after its own token started returning 401, and posted comments as the member (GH #7522). Whole-tree ownership already existed on both platforms. It was opt-in, and opt-in rotted: 8 of the 27 places this package starts a process asked for a Unix process group, and 3 of the 23 backends took Windows Job Object ownership. Adding claude to that list would have left 19 backends with the same hole, so the fix is structural instead. - newRuntimeCmd, at the single point where a runtime process is built, now puts every command in its own process group and replaces os/exec's leader-only Cancel with a group-wide kill. A backend that wants a graceful shutdown still assigns its own cmd.Cancel and wins. This also covers the --version and model-discovery probes, which cmd.Output() runs without ever reaching a start helper. - startOwnedProcessTree is now the only way this package starts a process, so Windows Job Object ownership is not something a backend can forget. Each start is paired with releaseProcessGroup after the reap, which drops the job handle and kills whatever outlived it. - Two structural tests enforce both halves, the way TestOnlyLaunchGoSpawnsRuntimeProcesses already enforces the argv boundary: a new backend calling cmd.Start fails, and one that takes ownership without releasing it fails. The Unix half is #5918 for the 20 backends the original fix did not reach: their CLI was never in its own group at all, so no group-wide signal could have reached it. Co-authored-by: multica-agent <[email protected]> * fix(agent): own the process tree for synchronous probes too Review found the ownership boundary had a hole the PR claimed to cover. os/exec's Run, Output and CombinedOutput call Start themselves, so a probe written with cmd.Output() never reaches startOwnedProcessTree and owns nothing on Windows — where the direct child of a `--version` probe is typically the npm shim and the real CLI is already a grandchild. That is the same escape #7522 was reported for, in 15 call sites nobody was looking at; detectCLIVersion's own comment already describes a broken CLI leaving grandchildren that hold the stdout pipe. - runOwned / outputOwned / combinedOutputOwned give a synchronous probe the same start, reap and release a task launch gets. They keep the stdlib contract: stdout returned, stderr attached to the ExitError, combined output through a single writer so os/exec gives it one pipe. The stderr sample is bounded, as os/exec bounds its own. - All 15 probe sites route through them, and the structural guard now rejects Run/Output/CombinedOutput alongside Start. The release guard did not hold its invariant either: it only asked whether a file contained any releaseProcessGroup, so a second unreleased start in a file that already had a pair would pass — and models.go is exactly that shape. It now works per function: one owned start each, paired with at least one release. Equal counts would be the wrong rule, since dsh's Execute legitimately releases on two exit paths. Both guards are now pure functions over an AST, with tests that feed them synthetic sources — including the second-unreleased-start case that the file-level version missed — so the guards fail closed without needing a real regression committed to find out. Adds the Windows probe regression (descendant must be in the job and die with a cancelled probe) plus unix equivalents, and names the consequence in the degraded-ownership warning, which is the only signal an operator gets that cancellation on that host is back to killing the leader alone. Co-authored-by: multica-agent <[email protected]> * fix(agent): bound a finished probe's wait on pipes its descendants hold Review caught that runOwned could wait for its own cleanup and never return, and it reproduces: a descendant that inherited the output pipes holds them open after the leader exits, cmd.Wait blocks until the copy goroutines see EOF, and what would close those pipes is the release that runs after Wait. Cancellation breaks the cycle, which is why every test in the last round missed this — they all cancelled first, and they all set WaitDelay by hand. Normal exit has nothing to break it, and checkOpenclawVersion runs on the caller's context before runContext establishes any task timeout, so a version wrapper that exits cleanly while leaving a pipe-holding descendant would hang the task until a human cancelled it. runOwned now fills in a bounded WaitDelay when the caller left it zero — Go defines WaitDelay for precisely "a child process that exits but leaves its I/O pipes unclosed" — and signals the group before dropping ownership, so the descendant is reaped on Unix too, where releasing a process group is a no-op. A caller with its own bound keeps it; detectCLIVersion has set one by hand since MUL-3812 for this same shape. Adds the normal-exit regression on both platforms: the leader exits holding nothing, the descendant keeps the pipes, and outputOwned has to return in bounded time with the descendant gone. The Windows descendant spawner gains a spawn-and-exit mode and now hands its stdio to the descendant, which is what makes it hold the pipes at all. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
3c3e77b00c |
MUL-6362: redact secrets from provider command logs (#7206)
* fix(agent): redact provider command values Co-authored-by: multica-agent <[email protected]> * fix(agent): preserve safe command diagnostics Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Sol-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
3b08423684 |
MUL-6260 fix(daemon): treat custom runtime fixed_args as the argv prefix (#7052)
* fix(daemon): treat custom runtime fixed_args as the argv prefix (MUL-6260) A custom runtime profile configured as `ccms start q36` failed every task with `unknown option '-p'`. The profile's fixed_args were merged into ExecOptions.ExtraArgs, which each backend appends *after* the protocol flags it hardcodes, so the wrapper saw `-p` before its own `start` subcommand had been selected and rejected it (GH #7046). Ordering was only half the defect. ExtraArgs is opt-in — six of the twenty-one backends read it — so on the other fifteen families, including every ACP one, fixed_args were silently discarded rather than misordered. Model discovery and `--version` probing carried the profile's path but not its arguments, so even a fixed launch would have left the model picker and the reported version describing the wrapper instead of the CLI behind it. fixed_args now travel as agent.Config.LaunchPrefix and are spliced in directly after the executable, ahead of everything a backend builds. The prefix is applied by a new Command boundary in launch.go through which every runtime process in the package is constructed — launches, version probes and discovery alike — so a backend cannot forget it. A guard test fails the build if any file outside launch.go calls os/exec directly; distributed opt-in is what let ExtraArgs rot, and repeating it would rot the same way. Flag-style commands (`agent --model composer-2.5`) are unaffected: a CLI takes its own flags in any position, which is why prefix-first can be the single order rather than a per-runtime setting. One deliberate behaviour change. The prefix used to sit last and win every last-wins parse, so a profile pinning `--model` silently overrode the model chosen on the agent — the UI showed one model and ran another. Prefix-first inverts that: the more specific per-agent choice is the later token and wins. Overlaps are logged. Protocol-critical flags are filtered out of the prefix once in New(), which knows the family; positional tokens always pass through. Codex additionally strips `-c` overrides aimed at daemon-managed config namespaces, since those beat config.toml from any argv position. The order was never documented, and every shipped fixed_args example is flag-style, so this fills in a contract rather than breaking one. Co-authored-by: multica-agent <[email protected]> * fix(daemon): close the launch-prefix gaps found in review (MUL-6260) Four defects from the first round of review on the fixed_args-as-argv-prefix change. Hermes could escape its own task overlay. The profile selection was resolved from custom_args alone and stripped from custom_args alone, so a profile configured as `hermes -p research` kept that flag in the launch prefix and re-pointed HERMES_HOME past the overlay the daemon had just built. Resolution now parses the argv hermes actually sees — launch prefix then custom args, in process order — and the strip covers both regions. It also removes every selection rather than the first: taking out the first promotes the second, and two separately-configured argv regions make "two selections" an ordinary configuration rather than a user mistake. Positional tokens were not passing through as documented. filterLaunchPrefix delegated to filterCustomArgs, whose blocklists deliberately carry positional tokens — hermes and qwenpaw block `acp`, traecli blocks `acp`/`serve`, grok blocks `agent`/`stdio`/`serve` — because in custom_args those are someone re-issuing the backend's own subcommand. In a launch prefix the same token is the command's identity, so `hermes-wrapper acp tenant` was being rewritten to `tenant`. Only a leading `-` now makes a token eligible for the blocklist; a blocked flag still consumes its separate value so no stray value survives to be read as a subcommand. Codex fast mode lost to the prefix. `--disable fast_mode` beats `--enable fast_mode` regardless of argv order and `-c features.fast_mode=false` beats config.toml from any position, so prefix-first does not protect an explicitly selected priority tier. The removal half of enforceCodexFastMode is now a separate helper applied to the prefix too; the managed enable is still appended once, on the final args. Windows CI could not compile the daemon tests: the Command migration left an unused `path := runtimeCmd.Path` in a hook whose body never referenced it. Verified with GOOS=windows and GOOS=linux vet, which is what should have caught it the first time. Also softens the docs and the Command doc comment: flag-style prefixes usually parse the same in either position, but a CLI that separates global from subcommand flags can care, so the runtime docs now name the position change and tell operators with existing fixed_args to re-run one task after upgrading instead of promising the move is invisible. Co-authored-by: multica-agent <[email protected]> * fix(daemon): resolve and strip the Hermes profile from the real launch argv (MUL-6260) Second-round review found the Hermes profile chain still reasoning about a hand-assembled approximation of the command line instead of the command line itself. The backend launches `<prefix> acp <filtered custom args>`, but the daemon assembled `<prefix> <custom args>` — no `acp`, no filtering — and that token participates in Hermes' own argv scan. With fixed_args `--model` and custom_args `-p research`, the approximation reads `-p` as `--model`'s value and finds no selection, so the overlay is seeded from the default home; the real `--model acp -p research` skips `--model acp` and selects `research`. The selector was then stripped, so the task stayed inside the overlay — but an overlay built from the wrong profile, silently. Stripping had the matching defect, and this one was still an escape rather than a mismatch. A launch prefix ending in a bare `-p` captures the backend's `acp` as its profile value, so neither region holds a complete selection and per-region stripping removes nothing: `-p acp research --yolo` reaches Hermes intact and re-points HERMES_HOME straight out of the overlay. Both halves now go through one assembly. agent.HermesLaunchArgv builds exactly what hermesBackend.Execute builds — the backend calls the same helper, so they cannot drift — and the daemon resolves from it. StripHermesProfileSelectors strips against that same argv and hands surviving tokens back to the region they came from, iterating to a fixed point because removing one selection promotes the next. The `acp` token is never removed; the backend re-adds it, and dropping the flag that captured it is what breaks the selection. StripAllHermesProfileArgs, hermesLaunchArgs and hermesEffectiveArgs are gone: each was only ever correct on a complete argv, and keeping them would leave the same footgun for the next caller. Tests cover the case the review named (prefix ending in a value-taking flag, custom args opening with a selector), the straddling-selection escape, multiple occurrences across both regions, and an invalid value left untouched because Hermes discards it anyway. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
4d495056e8 |
[MUL-6139] Expand Plugin skills and hosted MCP connections (#6920)
* feat(plugins): add remote MCP runtime Co-authored-by: multica-agent <[email protected]> * chore(plugins): add private Exa example Co-authored-by: multica-agent <[email protected]> * docs(plugins): remove private plugin guide Co-authored-by: multica-agent <[email protected]> * fix(plugins): stabilize remote MCP tool digests Co-authored-by: multica-agent <[email protected]> * fix(plugins): preserve remote MCP notification status Co-authored-by: multica-agent <[email protected]> * fix(plugins): make Exa example runnable end to end Co-authored-by: multica-agent <[email protected]> * fix(codex): record MCP tool calls in agent log Co-authored-by: multica-agent <[email protected]> * chore(plugins): add community skills example Co-authored-by: multica-agent <[email protected]> * feat(plugins): support multi-file skill bundles Co-authored-by: multica-agent <[email protected]> * feat(plugins): add Mobbin MCP example Co-authored-by: multica-agent <[email protected]> * chore(plugins): bundle full Matt Pocock skill set Co-authored-by: multica-agent <[email protected]> * feat(plugins): add remote MCP OAuth connections Co-authored-by: multica-agent <[email protected]> * fix(plugins): gate enable on remote MCP readiness Co-authored-by: multica-agent <[email protected]> * fix(plugin): authenticate remote mcp credential broker Co-authored-by: multica-agent <[email protected]> * fix(plugins): harden remote mcp oauth flow Co-authored-by: multica-agent <[email protected]> * refactor(plugins): use a single feature flag Co-authored-by: multica-agent <[email protected]> * fix(plugins): clean up remote MCP OAuth state Co-authored-by: multica-agent <[email protected]> * chore(plugins): remove Matt Pocock example Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Lambda <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
Danila Katalshov
|
3025c38027 |
MUL-6019: fix(codex): add MULTICA_CODEX_FIRST_TURN_TIMEOUT to raise the first-turn ceiling (#6753)
* fix(codex): add MULTICA_CODEX_FIRST_TURN_TIMEOUT to raise the first-turn ceiling The first-turn no-progress watchdog caps at the 60s default that the configured semantic-inactivity timeout can only shrink, never raise. An app-server that is legitimately slow to its first event (heavy MCP boot, cold model catalog) is killed at 60s with no way to extend it. Add a dedicated MULTICA_CODEX_FIRST_TURN_TIMEOUT knob (default 0 = unset) threaded through ExecOptions to codexFirstTurnNoProgressTimeout, which now honors an explicit value upward and is otherwise byte-identical. Default behaviour and the clamp contract are preserved. Supersedes #5959. Refs #3262. * fix(codex): pin and document the first-turn/semantic timer interaction The MULTICA_CODEX_FIRST_TURN_TIMEOUT override only raises the first-turn watchdog, not the semantic-inactivity timer that the same first status:running arms concurrently. An override above the semantic timeout is therefore truncated to it, and the resulting failure is reclassified as semantic inactivity — which also skips the #3291 model-catalog startup retry. The prior docs, comment, and test wording implied the override "wins outright / unbounded above", hiding this real upper bound and the configuration-dependent loss of automatic recovery. - Warn at config load when MULTICA_CODEX_FIRST_TURN_TIMEOUT exceeds the semantic timeout, naming both the truncation and the disabled startup retry. - State the effective bound (semantic timeout + overall execution timeout) in all four environment-variables*.mdx pages, and that both must be raised together when the desired first-turn wait exceeds the semantic limit. - Narrow the "wins outright / unbounded above" resolver comment and test name to describe only the first-turn watchdog ceiling. - Add a runtime regression test that drives the real run loop with an override above the semantic timeout and asserts the semantic timer wins the race and the startup retry is skipped; add a LoadConfig empty/explicit/zero test (this knob is environment-only, no Overrides parity). Addresses review on #6753. Refs #3262 / #5959 / #3291. * fix(codex): warn at equal first-turn/semantic timeouts, require strictly greater multica-eve's re-review on #6753: the warning fired only when the first-turn timeout was strictly greater than the semantic timeout, and told operators to raise the semantic timeout "to match". But the semantic timer is armed first (codex.go resets it before creating the first-turn timer), so at equal durations the semantic deadline is reached first and Go's select does not deterministically favour the first-turn branch — the failure can still be classified as semantic inactivity and skip the #3291 startup retry. An operator following the old guidance to equal values got no warning and silently lost the retry. - Warn when first-turn >= semantic (was >), and tell operators to set the semantic timeout strictly above the first-turn timeout (with margin), not "to match". - Update all four environment-variables*.mdx locales accordingly. - Add TestLoadConfig_CodexFirstTurnTimeoutEqualToSemanticWarns pinning the equal-duration warning and the strictly-greater no-warning boundary. Addresses review on #6753. Refs #3262 / #3291. |
||
|
|
e7e01638ad |
MUL-6118: fix(agent): own Codex process trees with a Job Object on Windows (#6897)
* fix(agent): own Codex process trees with a Job Object on Windows Windows has no process groups, so configureProcessGroup was a no-op, waitProcessGroupGone always reported false, and signalProcessGroup killed only the direct child. That child is usually a cmd.exe wrapping a .cmd shim, so the real tree — the Node wrapper, the native app-server, and the sandbox helpers under them — was left running as orphans. Assign the started process to a Job Object with KILL_ON_JOB_CLOSE, which every process it later creates inherits. Termination now targets the job, emptiness is read from job accounting, and ownership is dropped only once the tree has been reaped. The process handle is held for the lifetime of the entry so Windows cannot recycle the pid out from under the key. Assignment failure is not fatal: cleanup degrades to the previous kill-the-child behaviour, and the cleanup_confirmed gate keeps suppressing initialize retries on any host where ownership could not be taken. Refs #6883 Co-authored-by: multica-agent <[email protected]> * fix(agent): close the pre-attach window and stop keying trees by pid Two defects in the Job Object ownership added by the previous commit. Windows grants job membership only to processes created after the assignment, never retroactively. Assigning after cmd.Start left a window in which the direct child — usually a cmd.exe wrapping a .cmd shim — had already spawned the real agent outside the job. That is worse than owning nothing: the job then holds a wrapper that exits immediately, so accounting reports the tree empty while the escaped app-server is still running and cleanup is reported as confirmed. startOwnedProcessTree now creates the child suspended, assigns it, and only then resumes it, so it cannot create anything before it belongs to the job. A child is never left suspended: ownership that cannot be taken is logged and the child runs unowned, and a child that cannot be resumed is killed and reported. Ownership was also keyed by pid. A pid is unique only while its process is alive, so a launch that outlived its process could look up — and terminate — an unrelated task that inherited the number. Key by the *exec.Cmd of one launch instead; signalProcessGroup and waitProcessGroupGone take the command on both platforms. The tests assert job membership through the job's own accounting rather than through timing, so an escaped descendant fails deterministically. Refs #6883 Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
Bohan Jiang
|
c896f677d5 |
fix(agent): recover from a codex resume that overflows the reader (MUL-5722) (#6420)
Layer 1 (#6383) raised the shared scanner cap to 32 MiB, which moved the cliff without removing it — a codex rollout is append-only, so a thread that outgrows any fixed cap still fails its resume forever. This adds the recovery path. Layer 2: an oversized thread/resume response is reported as Result.ResumeRejected rather than a crash, which is the positive evidence shouldRetryWithFreshSession needs and reconnects the recovery #5715 had unintentionally cut off. Reaching it required one real fix — the reader wrapped the scanner error with %v, so bufio.ErrTooLong never survived to a caller. Layer 3: a new codex_resume_oversized reason marks the session resume-unsafe, and both resume lookups block by TIME rather than by matching the failed row. That shape is required, not stylistic: the failure happens before the turn starts, so the row lands with session_id NULL and is dropped by latest_per_session before any error-text filter runs. The block expires once a thread terminates after the overflow, so an issue recovers instead of starting cold forever. Also splits the MUL-4424 continuity notice by what each surface can still read — issue comments and Slack channel history can be re-read, web chat and Feishu cannot — so only the last group tells the user a loss happened. The backend no longer holds any of that wording; it receives ExecOptions.ResumeContinuityNotice from the caller and stays silent when the prompt already carries it, which makes the duplicate injection on the retry path structurally impossible. Known gap, tracked not claimed: for chat, the claim handler reads chat_session.session_id before the fallback query, so a daemon predating this PR leaves that pointer naming the oversized thread. Clearing it needs the attempted session recorded at claim time, which is a schema change. |
||
|
|
a23b936b7b |
fix(agent): raise the agent stream scanner cap to 32 MiB and share it (MUL-5722) (#6383)
* fix(agent): raise the agent stream scanner cap to 32 MiB and share it (MUL-5722) Codex serializes an entire thread into the single `thread/resume` response line, so a long thread crossed the per-backend 10 MiB bufio.Scanner cap and failed the resume with "bufio.Scanner: token too long". Because that classifies as agent_error.process_failure, the session pointer survives and every later turn resumes the same oversized thread — the task is stuck for good. This is GH#4520 recurring: #4563 bounded the hang and the orphan process, but the cap follow-up it promised was never done. Raise the bound to 32 MiB and route every line-delimited agent transport through one newAgentStreamScanner constructor. The cap was copy-pasted into 17 sites and had already drifted (pi at 32 MiB, everything else at 10 MiB), which is how #4563's fix reached only one backend. The three tests that deliberately overflow the scanner now size their payloads from the constant, so raising the cap again cannot silently downgrade them into plain oversized-line passes. This is the containment layer only. A thread can still outgrow any fixed cap, and the recovery path is still missing: overflow is reported as a transport error, which blocks the thread/start fallback, and codex never sets ResumeRejected so the daemon's fresh-session retry does not fire either. Those remain open on MUL-5722. Co-authored-by: multica-agent <[email protected]> * fix(agent): route the QwenPaw ACP stream through the shared scanner (MUL-5722) QwenPaw (#5986) landed on main after this branch was cut, carrying its own bufio.Scanner with the old 10 MiB cap. Merging as-is would have left one backend still on the cliff this PR exists to remove — the same way #4563's fix originally reached only one backend. Rebased onto main and routed it through newAgentStreamScanner. No stray per-backend cap remains: the only bufio.NewScanner calls left in the package are the model-listing and codex session-file parsers, which are deliberately out of scope. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
55dc302d0e |
MUL-5610: add Codex first-item wait telemetry (#6312)
* fix(codex): add first-item wait telemetry Co-authored-by: multica-agent <[email protected]> * test(codex): stabilize first-item telemetry race coverage Co-authored-by: multica-agent <[email protected]> * test(codex): harden first-item wait lifecycle fixture Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Eve <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
99d8f29bde |
fix(codex): raise the first-turn no-progress ceiling to 60s (MUL-5542) (#6192)
The first-turn watchdog killed healthy turns. Two independent field reports measured the first progress event landing just past 30s on gpt-5.5, and ~39s for a WSL app-server, all inside the window the watchdog treats as "stuck". Raise the ceiling to 60s. The window's only job is to fail fast instead of waiting out the 10 minute semantic inactivity backstop, so 60s keeps that value while clearing the observed evidence with margin. Add a regression test for the clamp, which had none: the configured semantic inactivity timeout can only shrink this window, never raise it. Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
999e9f93c7 |
fix(codex): record file edit payload for patch_apply events (#6158)
* fix(redact): scrub secrets nested inside tool input maps and slices
InputMap only passed top-level string values through Text and documented
non-string values as "preserved as-is". Any secret one level down reached
the database and the WebSocket broadcast untouched:
flat -> [REDACTED ...] (scrubbed)
nested -> [map[diff:token=ghp_... path:a.go]] (leaked verbatim)
This is a prerequisite for recording structured file-edit payloads. Codex
reports an edit as changes[]{path, diff, content}, and the legacy protocol
reports a deletion as the whole outgoing file — so without this, deleting a
.env would persist its full contents in cleartext.
redactValue now walks the composite shapes json.Unmarshal produces, plus
[]string and map[string]string for argv-style inputs. Composites are copied
rather than scrubbed in place, because the caller keeps using the map it
passed in.
Nesting depth comes from daemon-supplied JSON, so the walk is bounded at 32
levels; a pathologically nested payload would otherwise recurse until the
stack blows. Hitting the bound yields a placeholder rather than the raw
value, keeping the fail-safe direction.
Verified: the five new tests each fail against the previous top-level-only
implementation and pass now; full ./pkg/redact suite green.
Co-authored-by: multica-agent <[email protected]>
* fix(codex): record file edit payload for patch_apply events
Both Codex protocol paths recorded a file edit as a bare call ID and set no
payload, so a run that edited six files left six blank, unexpandable rows in
the transcript. The same task on Claude or Grok showed a readable diff, and
the branch Codex pushed was the only surviving record of what it changed
(GH #6157).
The omission was specific to this one tool, not to the adapter: the
exec_command handlers directly above already captured command and output.
Both paths are fixed, since the protocol is sniffed at runtime. Their wire
shapes differ more than they appear, and the normalizer reconciles that:
- legacy patch_apply_begin/end carry map[path]FileChange, internally tagged
on `type`, where add/delete hold whole-file `content` and only update
holds a `unified_diff` plus `move_path`. There is no diff for every case,
so the normalized form keeps diff and content as alternatives.
- v2 fileChange items carry an ordered array of {path, kind, diff} where
`kind` is an object, not a string — reading it as a string silently
yields "" and loses the add/delete/update distinction.
- status spellings differ too: legacy is snake_case, v2 is camelCase and
adds inProgress. Both normalize onto one vocabulary, and a legacy event
predating `status` falls back to its `success` bool.
Legacy map iteration is sorted by path so a replayed event does not reshuffle
the file list.
Completion events now also produce a non-empty output (status, file count,
and any apply_patch stdout/stderr), because an empty output renders as an
unexpandable blank row just like a missing input.
Anything unrecognised — absent, wrongly typed, or malformed changes — returns
no payload, preserving exactly the previous degradation rather than risking
the transcript.
Total diff/content bytes are bounded at 64 KiB with UTF-8-safe truncation,
recording `truncated` and `original_bytes`; paths and kinds always survive,
since they are what a reviewer needs when the body is gone. The bound is
deliberately scoped to this new payload: other providers stream tool inputs
through unbounded, and clamping them here would silently truncate
transcripts that render correctly today. Unifying the limit at the
persistence boundary is left as a follow-up.
Verified: the new tests reproduce the reported symptom (Input:map[],
Output:"") against the previous call sites and pass now; ./pkg/agent and
./pkg/redact green, go vet and gofmt clean.
Co-authored-by: multica-agent <[email protected]>
* feat(transcript): render Codex multi-file patch payloads as diffs
The presenter identified an edit by input shape — a top-level file_path plus
old_string/new_string or content — which is Claude's and Grok's shape. Codex
records one patch_apply covering several files as changes[], so even with the
payload now populated it fell through to pretty JSON instead of a diff.
A new `patch` detail kind carries one entry per file, since collapsing them
into a single body would lose which change belongs where. Each file reuses the
existing single-file surfaces, so all bodies behave alike inside the
virtualized list.
Codex hands over a ready-made unified diff, so parseUnifiedDiff maps it onto
diff rows rather than recomputing one — there is no before/after pair to
compare, and reconstructing both sides from the diff just to diff them again
would be circular. Hunk headers become `gap` rows, which is what they denote:
skipped unchanged content.
A deletion renders as all-removals rather than as a whole-file write, because
the legacy protocol reports it as the outgoing file's content and a green
"+N" gutter would state the opposite of what happened.
The collapsed row needed its own fix: with no single path field, the summary
fell through the preference chain and came back empty. It now reads as the
first path plus "+N more".
Anything that is not this shape still falls back to pretty JSON, so a payload
this presenter does not understand stays readable.
Verified: 17 new tests (43 in the presenter suite) pass; repo typecheck and
lint clean. The one failing views test, layout/sidebar-resize, fails
identically on an untouched checkout.
Co-authored-by: multica-agent <[email protected]>
* fix(codex): route v2 add/delete payloads as content, not diff
Addresses review on #6158.
Upstream's format_file_change_diff only produces a unified diff for `update`.
For `add` and `delete` it returns the whole file's contents under the same
`diff` field, and for a moved `update` it appends a trailing
"\n\nMoved to: <path>" line:
FileChange::Add { content } => content.clone(),
FileChange::Delete { content } => content.clone(),
FileChange::Update { unified_diff, move_path } => ...
(codex-rs/app-server-protocol/src/protocol/item_builders.rs, rust-v0.145.0)
Recording that as a diff mislabels every line of an added or deleted file as
context, and actively inverts any line whose content begins with '+' or '-' —
so an added file containing "-minus lead" rendered as a deletion. The payload
is now routed by `kind` rather than by field name, and the "Moved to:"
sentence is stripped since move_path already carries the destination.
The previous v2 tests hid this by using a fixture the real protocol never
emits (an `add` carrying "@@ ... +package main"). They now use upstream's
shape, plus cases for delete, an empty add, and an add whose contents look
like diff headers.
Empty bodies are also kept on both paths: presence of the field, not its
non-emptiness, decides whether a body was reported, so an empty added file
renders as an empty body instead of "no content reported".
Verified: the new assertions fail against the previous normalizer — where an
`add` came through as {"diff": "package main\n"} — and pass now.
Co-authored-by: multica-agent <[email protected]>
* fix(transcript): stop treating header-like content lines as file headers
Addresses review on #6158.
parseUnifiedDiff matched "---" / "+++" / "diff --git" / "index " at any
position, so a changed line whose *content* starts with a dash or plus was
silently discarded:
parseUnifiedDiff("@@ -1 +1 @@\n--- old markdown\n+++ new markdown\n")
// before: [{ kind: "gap", ... }] — both changed lines gone
A removal of "-- old markdown" is spelled "--- old markdown" on the wire, so
this hit Markdown rules, embedded patches, and comment banners.
File headers only exist ahead of the first hunk, so they are only recognised
there; once inside a hunk every line is parsed strictly by its first
character.
Also localizes the multi-file summary count, which was hardcoded English and
so leaked into the zh-Hans / ja / ko transcript rows. The presenter owns no
React and no i18n by design, so the phrasing is injected by the caller rather
than imported here, keeping the module unit-testable in isolation; the English
form remains the fallback. The three Chinese/Japanese/Korean truncation
strings now use "..." to match the English source they translate.
Verified: both new parser assertions fail against the previous
strip-anywhere behaviour and pass now; 47 presenter tests green, repo
typecheck and lint clean.
Co-authored-by: multica-agent <[email protected]>
* fix(daemon): redact nested tool input before it leaves the daemon
Addresses review on #6158.
Recursive redaction ran only in the server's ingest handler. The daemon built
the new nested edit payload and sent msg.Input verbatim, so a daemon that
self-updated ahead of the server — or one talking to a server mid-rollout —
would ship whole-file edit contents to a peer that does not scrub nested
values yet. The legacy protocol reports a deletion as the whole outgoing file,
so that window covered a deleted .env in cleartext.
Ordering three commits inside one PR is not a deployment barrier, and daemon
and server upgrade independently. Deployment order is not a control we have,
so the sending side is now safe on its own; the server keeps redacting on
ingest as the second line of defence.
Scoped to Input, which is the field this PR newly fills with file contents.
Content and Output are plain strings already redacted server-side, and
changing their daemon-side handling would be unrelated to this fix.
Verified: the new daemon test asserts the nested token is masked in the
reported batch while the change metadata survives. It fails without this
change, reporting the full GITHUB_TOKEN= line on the wire, and passes with
it; ./internal/daemon green.
Co-authored-by: multica-agent <[email protected]>
* fix(transcript): correct the Chinese multi-file patch count semantics
Addresses review on #6158.
The summary is handed the number of files *beyond* the named one, but the
Chinese phrasing stated a total: "a.go 等 2 个文件" reads as two files including
a.go, so a three-file patch under-reported by one. English hides the
distinction ("+2 more"), which is why it survived the first pass.
Rewords zh-Hans to "另有 N 个文件". Japanese (他) and Korean (외) already read
as "besides", so their wording is unchanged.
Also renames the interpolation variable from `count` to `extra`, for two
reasons. i18next treats `count` as the plural selector — this very namespace
relies on that for events_one/events_other — so a plain number had no business
borrowing it. And the name is what a translator reads: `extra` cannot be
mistaken for a total the way `count` was.
Guards the whole bug class rather than just this string: a locale test asserts
every locale interpolates {{path}} and {{extra}} and never the reserved
{{count}}, and a presenter test pins that the injected number is the count of
additional files, not the total.
Verified: both new locale assertions fail against the reverted string and pass
now; rendering the real locale strings for a three-file patch yields "+2 more",
"另有 2 个文件", "他 2 件", "외 2개". 53 target tests pass, repo typecheck clean,
views lint back to its pre-existing 16 warnings and 0 errors.
Co-authored-by: multica-agent <[email protected]>
* fix(transcript): put the patch surface on the type scale
Addresses review on #6158.
The patch surface wrote text-[10px] / text-[11px] / text-[10px], copied from
the sibling transcript surfaces as they looked when this branch started. Since
then MUL-5451 (#6136) introduced a role-named type scale and migrated those
same siblings to text-micro, so these three call sites were the only remaining
arbitrary sizes — and the type-scale guard reports them precisely.
All three become text-micro. That matches the analogues they were copied from
now that those have moved: the FileWriteSurface line-count row, the
DiffDetailSurface header row, and the ToolDetailSurface body. It is also the
only correct target, since micro (11px) is the smallest step the scale defines
— there is nothing at 10px to map to.
Merges origin/main so the guard runs here rather than only in CI.
Verified: apps/web app/type-scale.test.ts 13/13 (it listed exactly these three
lines before), no `text-[` left in the file, repo typecheck clean, views lint
unchanged at 16 pre-existing warnings and 0 errors.
Co-authored-by: multica-agent <[email protected]>
* fix(codex): redact patch bodies before applying the size budget
Addresses review on #6158.
The adapter sized and truncated the normalized changes, and redaction only ran
later — in the daemon before sending, then again in the server on ingest. That
order loses secrets that straddle the budget.
The PEM rule needs both markers to match:
-----BEGIN[A-Z\s]*PRIVATE KEY-----.*?-----END[A-Z\s]*PRIVATE KEY-----
So a 70 KB private key whose BEGIN sits inside the first 64 KiB and whose END
falls past the cut stops matching once truncated. Neither later pass can
recognise what truncation already broke, so the marker and 64 KiB of key
material reach the database and the WebSocket broadcast. Measured on the
previous code:
stored bytes 65536 | BEGIN marker present | key body present | placeholder absent
Redaction now runs first, and the budget measures the redacted bodies — which
is also the honest measurement, since those are what actually gets stored and
redaction usually shrinks them (that key collapses to 23 bytes, so no trimming
is needed at all). `original_bytes` still reports the pre-redaction size so the
reader sees how large the real patch was. The daemon and server passes stay as
defence in depth; redaction is idempotent, so running three times is safe and
that is now asserted.
Note for callers: codexPatchInput no longer trims its argument in place, because
redaction copies first. Two existing tests were asserting on the caller's
original slice and had silently become vacuous; they now read the returned
payload, and one pins the no-mutation contract. The delete fixture in the
diff-vs-content routing test was also a credential-shaped string, which now
redacts — it is plain text so that test keeps testing routing.
Verified: the new boundary test fails on the previous order, reporting the
surviving BEGIN marker and key material, and passes now. go test ./pkg/agent
./pkg/redact ./internal/daemon green; execenv ByteIdentical green; go vet and
gofmt clean.
Co-authored-by: multica-agent <[email protected]>
---------
Co-authored-by: Eve <[email protected]>
Co-authored-by: multica-agent <[email protected]>
|
||
|
|
0066ab259e |
refactor(agent): drop unreachable inline system-prompt branches (MUL-5392) (#6050)
* refactor(agent): drop unreachable inline system-prompt branches (MUL-5392) The daemon only populates ExecOptions.SystemPrompt for openclaw, kimi and traecli (providerNeedsInlineSystemPrompt); every other backend receives the runtime brief as a per-task context file in the workdir. The inline branches in claude, codex, opencode and pi were therefore dead, and read as if they were the live delivery path. Probed each backend over its real launch path with a canary in the context file and no inline delivery — claude 2.1.220 (CLAUDE.md), codex 0.144.6 via the app-server, opencode 1.17.7, pi 0.67.2, hermes 0.18.2 via ACP — and all of them picked the brief up from disk, so the branches were removable with no behaviour change. An empty workdir returned no canary, confirming the probe could fail. opencode's branch was worse than dead: `opencode run` has no --prompt flag, so enabling inline delivery there would have made every opencode task exit 1 with a usage dump. The DevEco backend, forked from opencode, already documents this constraint; opencode itself never got the fix. Regression tests pin all three arg builders against re-adding the flag, and providerNeedsInlineSystemPrompt now documents what was verified and what is still unprobed (grok, qoder, codebuddy). Hermes and kiro are untouched: their exclusion is deliberate and already tested. Co-authored-by: multica-agent <[email protected]> * test(agent): pin codex developerInstructions contract, drop stale pi flag doc Review follow-up on MUL-5392. buildPiArgs' doc comment still advertised --append-system-prompt after the branch that emitted it was removed — exactly the stale-signal this PR set out to delete. The two codex sites fixed to a literal nil had no regression test, so restoring nilIfEmpty(opts.SystemPrompt) would still have gone green. Both thread/start and thread/resume now run with a canary SystemPrompt and assert developerInstructions comes through as an explicit null. Mutation-checked: reverting either site fails its test. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
ba108978ac |
fix(channel): deliver the final answer only to Slack and Lark (MUL-5378) (#6016)
* fix(channel): deliver the final answer only to Slack and Lark (MUL-5378) Channel replies could carry the agent's interim narration alongside its answer (GH #6006). Two independent causes, both fixed at the layer that owns the deliverable. Prompt regression. #4776 told every channel-backed chat to reply with the answer only and not narrate. The MUL-4899 split (#5557) moved that rule into the Slack branch along with the `chat history` / `chat thread` commands its wording happened to mention, so Feishu/Lark silently lost it on 2026-07-17. The rule is a third axis — it keys off "is there a channel at all", like the attachment-upload axis — so it now sits outside the Slack gate, generalized from "these history reads" to any progress note. The old two-layer matrix could not catch the regression because it only asserted the rule on the Slack case; the new test pins all three states. Runtime contract. Result.Output is documented as "final user-facing output selected by the backend" (agent.go), but Codex concatenated every agent_message and Copilot joined every assistant turn with "\n\n", so a tool-using run handed the daemon narration + answer as one string. Codex now takes the message its app-server labels phase="final_answer", falling back to the most recent agent message on the legacy protocol; Copilot keeps the latest complete turn, with the streaming deltas retained as the process-died-mid-turn fallback. This narrows delivery only — every message still streams as MessageText, so the Multica transcript is unchanged. Claude Code, CodeBuddy and qwen already selected a terminal result and are untouched; opencode/deveco/openclaw share the accumulating shape and want the same audit (pi was already fixed this way in #4894). Verified: go build ./..., go vet, full ./pkg/agent and ./internal/daemon/... suites pass locally. Co-authored-by: multica-agent <[email protected]> * fix(channel): scope the no-narration rule to process, not results Review follow-ups on the channel delivery rule and the Copilot turn boundary. The prompt said a reply "must not say what you are about to do or just did", which literally forbids the deliverable itself: asked to create an issue, the correct reply IS "created issue X". Rewritten to ban planned and in-progress narration while explicitly protecting the completion confirmation. The test now pins both halves — a future edit that drops the carve-out, or restores the blanket past-tense ban, fails. The example also referenced "check the code", which is not a thing an agent does inside a Slack or Lark conversation. Replaced with a generic "let me look into that first". Copilot cleared pendingDelta only when the authoritative assistant.message carried content. A tool-only turn reports content:"" with the requests as the whole turn, so its streamed deltas stayed buffered and were stitched onto the next turn's partial text if the process then died mid-stream — verified: the new test yields "Checking the logs now.The retry loop" before the fix. The reset now happens on every assistant.message, since that event is the turn boundary regardless of whether it carries text. Verified: go build ./..., go vet, full ./pkg/agent and ./internal/daemon/... suites pass locally. Co-authored-by: multica-agent <[email protected]> * fix(channel): tighten the no-narration rule to one sentence Same contract, fewer tokens: the four-line rationale comment collapses to one, and the delivery rule drops the restatement, the second example and the completion examples. What survives is exactly the semantic boundary the tests pin — no planned/in-progress narration, completed actions still count as the outcome — plus the one narration example actually observed in the report. Verified: go build ./..., go vet, ./internal/daemon/... suite pass locally. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
6992c58de3 |
MUL-5185: add Codex Fast mode (#5821)
* feat(agents): add Codex fast mode (MUL-5185) Co-authored-by: multica-agent <[email protected]> * fix(agents): make Codex Fast override authoritative Co-authored-by: multica-agent <[email protected]> * fix(agents): remove Codex Fast config conflicts Co-authored-by: multica-agent <[email protected]> * chore: refresh checks after conflict resolution Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Eve <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
milymarkovicandmultica-agent
|
09dce598df |
MUL-5155: KAP-1051: diagnose Codex thread/start timeouts fail-closed (#5759)
* fix(agent/codex): diagnose thread start timeouts (KAP-1051) Co-authored-by: multica-agent <[email protected]> * fix(agent/codex): validate thread ID before success lifecycle Co-authored-by: multica-agent <[email protected]> * fix(agent/codex): confirm process tree cleanup Co-authored-by: multica-agent <[email protected]> * fix(agent/codex): bound Windows pipe cleanup Co-authored-by: multica-agent <[email protected]> * ci: run bounded Codex cleanup test on Windows Co-authored-by: multica-agent <[email protected]> * fix(agent): reap initialize timeout process groups * fix(agent): redact initialize timeout stderr * fix(agent): redact initialize context failures --------- Co-authored-by: multica-agent <[email protected]> |
||
|
Bohan Jiang
|
6ced54b44a |
fix(agent/codex): retry once when the model catalog refresh blocks the first turn (MUL-5110) (#5734)
Retries once when Codex's model catalog refresh blocks the first turn, with a narrow safety gate: first-turn no-progress timeout, zero semantic progress, catalog-refresh evidence in stderr, and a confirmed-reaped process tree. Clears ResumeSessionID on retry so a stalled thread is never resumed, and buffers the leading session pin so a discarded attempt cannot pin the resume pointer. |
||
|
|
fbe00ca164 |
fix(daemon): run Codex unsandboxed on Windows to stop reject-by-policy (MUL-4957) (#5672)
* fix(daemon): run Codex unsandboxed on Windows to stop reject-by-policy (MUL-4957) Windows has no Landlock/Seatbelt-equivalent filesystem sandbox that the daemon configures, so the per-task `sandbox_mode = "workspace-write"` it wrote was unenforceable. Worse than having no sandbox, it pushed Codex into rejecting non-safe mutation commands "by policy": `multica issue create` fails with "was rejected by policy" because Codex can neither sandbox the command nor (under approval_policy = "never") escalate it to the daemon's auto-approver, so the request never reaches the approver. Mirror the existing macOS fallback and give Windows danger-full-access so those commands run. Also generalize the danger-full-access warn log so it no longer hardcodes "on macOS" and only surfaces the macOS-specific upgrade hint on macOS (new codexSandboxPolicy.Hint field). Co-authored-by: multica-agent <[email protected]> * fix(daemon): correct Windows sandbox rationale and respect user windows.sandbox (MUL-4957) Addresses two review must-fixes on #5672: 1. Correct a false security fact. The comments and log Reason claimed Windows has no filesystem sandbox backend. Codex 0.144.5 does ship a native Windows sandbox (windows.sandbox = "unelevated"/"elevated"); it is experimental with open upstream reliability bugs, so the daemon defaults to danger-full-access as a deliberate compatibility choice. Enabling the native sandbox is tracked as separate follow-up work. 2. Stop silently downgrading users who opted into isolation. The fallback was unconditional. Add codexSandboxPolicyForConfig: on Windows an explicit windows.sandbox = unelevated|elevated keeps workspace-write so Codex enforces task isolation with the user's chosen backend; danger-full-access applies only when windows.sandbox is absent, disabled, or unparseable. This is also the branch point for a future native-sandbox rollout (flip the default; callers unchanged). Adds fixture tests locking the priority (user opt-in kept vs. unconfigured fallback) plus predicate coverage for codexSandboxPolicyForConfig. Co-authored-by: multica-agent <[email protected]> * fix(daemon): fail closed on undecidable Windows sandbox config, honor -c windows.sandbox (MUL-4957) Second review round on #5672. Two must-fixes. 1. Undecidable config no longer fails open. The old bool detector collapsed "unparseable / invalid value / failed copy" into "unconfigured" and then loosened to danger-full-access. Replaced with a tri-state (absent/native/undecidable): only exact-lowercase unelevated|elevated (the sole values Codex accepts — verified: any other value makes Codex refuse to load the config) counts as native; any other present value, unparseable TOML, a read error, or a missing per-task config when a shared ~/.codex/config.toml exists (i.e. the copy failed) is undecidable and fails closed to workspace-write — it never loosens — logged at error level. 2. windows.sandbox set via `-c`/`--config` custom args is now honored. Such args never land in config.toml, so config-only detection silently downgraded those users' isolation. The effective Codex args (daemon defaults + profile-fixed + per-agent custom_args) are threaded through PrepareParams/ReuseParams/CodexHomeOptions into the sandbox decision and scanned for a windows.sandbox override (inline, two-token, quoted, spaced; last-wins). Also drops issue-status-bound source comments (openai/codex#24098 has since closed). Adds unit coverage for config/args classification, the fold precedence (undecidable > native > absent), and the copy-failed fail-closed path. Co-authored-by: multica-agent <[email protected]> * fix(daemon): fail closed on config-sync errors and honor shell-quoted -c windows.sandbox (MUL-4957) Round-3 review must-fixes: 1. resolveWindowsSandboxState now takes the config.toml sync error and a tri-state shared-config presence instead of re-stat-ing inside. A failed sync (stale/absent per-task copy) or an un-stat-able shared source is undecidable and keeps workspace-write, closing the fail-open where a failed sync was read as "unconfigured". Splits IO from the decision so the paths are unit-testable without faulting the filesystem. 2. The Windows sandbox decision consumes agent.NormalizeCodexLaunchArgs (the shared helper buildCodexArgs now uses) so a shell-quoted -c windows.sandbox opt-in is normalized identically to launch, instead of being missed by a raw-token scan and silently downgraded. Co-authored-by: multica-agent <[email protected]> * fix(daemon): abort when the Codex sandbox block cannot be written (MUL-4957) Round-4 review must-fix: ensureCodexSandboxConfig failures were warn-and-continue, so a computed fail-closed workspace-write policy could stay only in memory while config.toml kept a stale danger-full-access from a prior run — the decision failed closed but the effective config failed open. prepareCodexHomeWithOpts now returns the error, which blocks startup on both paths: fresh Prepare fails the task, and Reuse leaves env.CodexHome unset, which configureCodexTaskShellEnvironment already refuses to start. Regression covers the full reuse scenario (stale danger-full-access + failed config sync + failed managed-block write); it fails with "got nil" without the fix. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]> Co-authored-by: J <[email protected]> |
||
|
|
f51828f568 |
MUL-4936: Fix empty replies in resumed Codex and Hermes chats (#5675)
* fix(agent): isolate resumed Codex turns (MUL-4936) Co-authored-by: multica-agent <[email protected]> * fix(agent): drain late Hermes replies (MUL-4936) Co-authored-by: multica-agent <[email protected]> * fix(agent): harden resumed chat cleanup (MUL-4936) Co-authored-by: multica-agent <[email protected]> * fix(agent): filter Codex subagents before turn gate (MUL-4936) Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Eve <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
adc7636674 |
MUL-4921: fix(codex): report resumed usage as task delta (#5569)
* fix(codex): report resumed usage as task delta Codex session logs expose cumulative total_token_usage values. Subtract the pre-task baseline so issue usage does not count earlier turns again. Refs #5568 * fix(codex): harden resumed usage accounting * fix(codex): scope fallback usage to current thread Co-authored-by: multica-agent <[email protected]> * fix(codex): harden rollout usage edge cases Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Eve <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
ZIce
|
910671b185 |
Fix Codex task MULTICA_TOKEN passthrough (ZIC-82)
Merge approved PR after independent final review; all required CI checks passed. |
||
|
|
ea8ccf3123 |
MUL-4903 fix(agent): harden Cursor terminal failures (#5559)
* fix(agent): harden Cursor terminal failures Co-authored-by: multica-agent <[email protected]> * fix(agent): preserve UTF-8 stderr tails Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Eve <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
milymarkovic
|
983545b34d |
KAP-867: retry safe Codex initialize timeouts
Merge reviewed changes after fresh approval and green CI. |
||
|
|
6cc553e5a3 |
fix(daemon): isolate Codex sessions per task to unblock initialize (MUL-4424) (#5360)
* fix(daemon): isolate Codex sessions per task to unblock initialize (MUL-4424) Codex 0.143+ backfills a per-home session-state DB by enumerating every rollout visible under sessions/ during `initialize`. The per-task CODEX_HOME symlinked the shared ~/.codex/sessions in, so a machine with a large accumulated history (one reporter: ~2000 rollouts / ~22 GiB) stalled `initialize` for tens of seconds — the app-server started but the task produced no output before it was cancelled (github #5273). Give each task its own local sessions/ directory instead: - Fresh task: create an empty local sessions/ so backfill is trivial. - Reused task with a real sessions/ dir: it is authoritative — leave it. - Reused task still holding a legacy symlink (older build): migrate in place. Replace the symlink with a real dir; when resuming, symlink only the single rollout being resumed (never copy — a rollout can be GiB and this is on initialize's critical path); and drop the stale, rebuildable session-state DB (state_*.sqlite*, session_index.jsonl) so Codex re-indexes the task-local sessions. Unrelated per-task DBs (goals_*, logs_*, memories_*) are left intact. Also point the token-usage fallback scan at the backend's per-task CODEX_HOME instead of the daemon-global home, so usage isn't lost now that sessions are isolated there. Complements the #5319 handshake watchdog (which turns a silent stall into a loud, phased timeout); this removes the underlying cause. Co-authored-by: multica-agent <[email protected]> * fix(daemon): address Codex session isolation review (MUL-4424) Resolves the three blockers from Elon's review of #5360: 1. local_directory context loss. local_directory tasks get a fresh codex-home per task ID (the daemon never reuses their workdir), so task-local isolation stranded every follow-up run with an empty sessions dir and silently restarted the conversation. Their only stable, GC-safe cross-task store is the user's own ~/.codex/sessions (a persistent store under WorkspacesRoot would be orphan-GC'd), so keep the shared-sessions symlink for them (IsLocalDirectory). Managed tasks stay isolated. 2. Migration resume robustness. - Rollout lookup now covers the flat layout and background-compressed .jsonl.zst rollouts, not just nested YYYY/MM/DD *.jsonl — both are legitimate Codex 0.144 history that were previously judged "not found", silently dropping resume. - Exposure hard-links first, then symlinks, never copies — hard links need no privilege and work on Windows within a volume, so the zero-copy path is exercised identically on CI. - The daemon now verifies the rollout is actually present in the task CODEX_HOME (execenv.CodexResumeRolloutPresent) before the brief is generated; if absent it clears the resume from both the backend and the brief instead of telling the agent it is continuing a lost thread. 3. session_index.jsonl is no longer deleted during migration — Codex uses it as the authoritative thread-id -> name store (not rebuildable from rollouts). Only the rebuildable state_*.sqlite* is reset. Tests: 2-round local_directory resume across task IDs; compressed/flat lookup; hard-link zero-copy (os.SameFile); session_index preserved; CodexResumeRolloutPresent + the daemon gate helper (present keeps / absent drops / non-codex + empty no-op). Co-authored-by: multica-agent <[email protected]> * fix(daemon): scope Codex sessions to a per-issue store; disclose lost resumes (MUL-4424) Addresses the three blockers from Elon's second review of #5360. 1. local_directory still enumerated the whole machine history. The prior fix re-linked the entire ~/.codex/sessions into every fresh local_directory codex-home, so Codex still backfilled from thousands of unrelated rollouts on `initialize` (measured ~8.3s with 3450 rollouts; the reporter's 22 GiB could exceed the 30s watchdog). Point sessions/ at a persistent, per-(agent, issue) store under the shared Codex home (multica-sessions/<agent>/<issue>) that holds only this issue's rollouts. It is keyed stably across task IDs and lives outside the task-scoped envRoot the GC reclaims, so follow-up runs resume it while `initialize` only ever sees this issue's history. 2. Windows cross-volume resume was lost. Exposing a single rollout by hard-link (same-volume only) then file symlink (needs Windows privilege) can't cross a volume boundary. The store now lives on the shared Codex volume, so the resume rollout is hard-linked there zero-copy, and sessions/ is exposed to the task home via a directory link — a symlink on Unix, a junction on Windows — which crosses volumes without privilege and never copies a (possibly GiB) rollout on initialize's critical path. There is no remaining per-file cross-volume link. 3. An unavailable resume was a silent downgrade. Both resume gates (gateResumeToReusedWorkdir, gateCodexResumeToRolloutPresence) now set PriorSessionResumeUnavailable, and the runtime brief renders a Session Continuity Notice telling the agent to disclose to the user, up front in its reply, that the previous conversation context could not be restored and this run starts fresh — turning a silent restart into a user-visible one. The task is not failed: it can still do useful work without the prior context. Managed fresh / reused-real-dir tasks keep their task-local, GC-collected sessions dir unchanged; only the legacy-symlink migration with a resume routes through the store (cross-volume-safe), and a home already linked to the store is treated as authoritative on reuse. Tests: local_directory per-issue store (only this issue's history, no whole- machine leak); no-key fallback to an empty dir; two-round resume across task IDs through the store; legacy migration routed through the store with a zero-copy hard link; reused store link stays authoritative; both gates set the resume-unavailable flag; brief renders the continuity notice only when a resume was lost. execenv + daemon + pkg/agent packages, go vet, and gofmt all pass. Co-authored-by: multica-agent <[email protected]> * fix(daemon): disclose live resume-RPC loss; bound Codex session store lifecycle (MUL-4424) Addresses the two blockers from Elon's third review of #5360. 1. A real thread/resume failure was still a silent new session. The brief's Session Continuity Notice only covers losses the daemon detects before launch (workdir not reused, rollout absent). But when the rollout is present yet Codex rejects the live thread/resume (corrupt/incompatible rollout, server-side thread GC, schema drift), startOrResumeThread falls back to thread/start and the run succeeds on a fresh thread with no user-facing signal. Carry the original resume intent into the backend as ExecOptions.ResumeExpected (set from the post-gate PriorSessionID, so a pre-flight drop still routes through the brief and never double-notifies), and when a resume was expected but the backend landed on a fresh thread, prepend the same continuity notice to the first turn/start input. This also covers the daemon's transport-error fresh-session retry, which clears ResumeSessionID but not ResumeExpected. 2. The persistent per-issue store had no data lifecycle. multica-sessions stores live outside the task-scoped envRoot the GC reclaims (so resume survives across task IDs), which meant a done/abandoned issue's prompts and full rollouts (one reporter: a single 1.5 GiB rollout) accumulated forever and were never freed on issue/agent/workspace deletion. Add PruneCodexSessionStores: the daemon GC loop reclaims any store untouched for GCCodexSessionTTL (default 14 days, configurable via MULTICA_GC_CODEX_SESSION_TTL, 0 disables). A store's newest rollout mtime is its last activity, so an active or recently-resumed task keeps its store fresh and is never reclaimed, while a deleted issue's store ages out — an eventual reclamation guarantee without needing deletion events. Tests: codexTurnInput discloses on resume fallback and stays silent on success / fresh start (paired with the existing live-RPC fallback test); store pruning reclaims aged stores, keeps recent ones, isolates issues, cleans empty agent dirs, and is disable-able. execenv / daemon / pkg/agent, go vet, gofmt all pass. Co-authored-by: multica-agent <[email protected]> * fix(daemon): protect a reopened Codex session store from GC mid-mount (MUL-4424) Addresses Elon's fourth-review blocker: reopening an issue idle past GCCodexSessionTTL could lose context, because mounting its per-issue session store (MkdirAll + rollout lookup + task-home link) never refreshed the store's mtime, so a GC cycle firing before the resumed turn wrote its first rollout saw a >TTL-old store and reclaimed it — a stat->remove race with no in-use guard. Two complementary defenses: - Activity refresh: linkCodexSessionsToStore now os.Chtimes the store to now after linking, so codexStoreStat (which reads the newest mtime as last activity) sees a just-used store. This fixes the sequential repro — a mount immediately followed by a prune keeps the store. - In-process active-store guard: the daemon marks the per-issue store in-use (execenv.CodexSessionStorePath) from before Prepare/Reuse mounts it until the task ends, and PruneCodexSessionStores now takes an isActive predicate and skips any store a live task holds. Because prepare and prune run in the same process, this closes the remaining concurrent stat->remove window the mtime refresh alone cannot. Reference-counted, mirroring the env-root guard. Tests: a reopened >TTL store survives a GC cycle after remount and stays resumable; an idle-on-disk store marked active is skipped, then reclaimed once inactive; the existing idle-reclaim / isolation / disable / empty-agent-dir cases still pass. execenv + daemon, go vet, gofmt all pass. Co-authored-by: multica-agent <[email protected]> * fix(daemon): make Codex store delete atomic with mark-active (MUL-4424) Addresses Elon's fifth-review blocker: the active-store guard's check and delete were not one atomic step. PruneCodexSessionStores called isActive (which locked, read, and unlocked) and only then RemoveAll'd, leaving a window where a task could markActiveCodexStore between the check and the removal and still lose its store — the exact mark-then-delete interleaving Elon reproduced. Replace the point-in-time isActive predicate with a reserve-for-deletion protocol that shares one lock with mark-active: - reserveCodexStoreForDeletion(store) atomically refuses when a live task holds the store (or another delete already reserved it) and otherwise marks it reserved, all under one activeCodexStoresMu acquisition. PruneCodexSessionStores reserves before RemoveAll and commits after, so confirm-inactive and remove are effectively atomic against a concurrent mark. - markActiveCodexStore now waits (on a sync.Cond) while a store is reserved, so a task never mounts a store mid-removal; committing the removal wakes it and the store is recreated fresh by Prepare (with the continuity notice). So mark-before-reserve keeps the store (reserve refused); reserve-before-mark removes it and blocks the late mark until the removal commits. The genuinely idle case still reclaims. Tests (daemon, run under -race): mark-then-reserve is refused; reserve blocks a concurrent mark until commit then the store reads active; a second reserve is refused mid-flight. The execenv prune tests move to the reserve seam; the activity-refresh / reopen-then-prune / isolation / disable cases still pass. Co-authored-by: multica-agent <[email protected]> * fix(daemon): namespace Codex session stores per profile for cross-daemon safety (MUL-4424) Addresses Elon's sixth-review blocker: the in-process reservation guard cannot span processes, but Multica supports multiple profile daemons on one machine (e.g. production + staging) that share the same ~/.codex. Each daemon's GC scanned the whole multica-sessions root, so a staging daemon could reclaim a store a production task was actively resuming — its reservation lived only in the other process's memory. Isolate by profile instead of trying to lock across processes: - Store path is now <shared>/multica-sessions/<namespace>/<agent>/<issue>, where namespace is the daemon's profile (empty -> "default"). PrepareParams/ReuseParams carry Profile; codexSessionStoreKey and CodexSessionStorePath fold it in. - PruneCodexSessionStores takes the profile and scans ONLY that namespace, so a daemon never even sees another profile's stores, let alone deletes them. The per-profile trees are disjoint, so the in-process guard is sufficient within a namespace (profiles get separate daemon state, so no two daemons share one). Test: a "staging"-owned idle store is untouched by a default-profile prune and reclaimed only by staging's own prune. Existing prune/guard/reopen tests move under the namespace. execenv + daemon under -race, go vet, gofmt all pass. Co-authored-by: multica-agent <[email protected]> * fix(daemon): make the Codex store profile→namespace map injective (MUL-4424) Addresses Elon's seventh-review blocker: the per-profile namespace was derived by dropping unsafe characters, which is not injective. The CLI treats the empty (default) profile and a profile literally named "default" as separate daemons, yet both mapped to namespace "default"; likewise "staging.prod" and "stagingprod" both mapped to "stagingprod". Two distinct daemons then shared one store tree, so one could again reclaim the other's live session — the cross-process blocker reopened for those profile names. Make codexSessionStoreNamespace injective: the empty profile gets a reserved bare literal "default", and every named profile is hex-encoded (bijective, filesystem-safe) under a "p_" prefix a bare literal can never collide with. So "" -> "default" while "default" -> "p_64656661756c74", and "staging.prod" / "stagingprod" get distinct hex segments. sanitizeCodexPathSegment stays for the UUID agent/issue segments (injective for real UUIDs); only the user-controlled profile needed the encoding. Tests: codexSessionStoreNamespace is distinct for "" vs "default", punctuation variants, case variants, and an encoded-looking name; and end-to-end, pruning one profile never reclaims the other's store for the "" vs "default" and "staging.prod" vs "stagingprod" pairs. execenv + daemon under -race, go vet, gofmt all pass. Co-authored-by: multica-agent <[email protected]> * fix(daemon): fixed-length Codex store namespace so long profiles fit (MUL-4424) Addresses Elon's eighth-review blocker: hex-encoding the full profile doubled the namespace segment length. A profile can be as long as a filesystem segment allows (~255 bytes) and the CLI persists it as its own config dir, but the store namespace "p_" + hex(profile) reached 2 + 127*2 = 256 bytes at 127 chars, overflowing the 255-byte single-segment limit — the profile's own dir created fine, then the session store failed with "file name too long". Derive the namespace from a fixed-length hash instead: a named profile is now "p_" + hex(sha256(profile)) — a constant 64 hex chars (66 with the prefix), filesystem-safe and collision-resistant. The empty (default) profile keeps its reserved bare literal "default", which the "p_"-prefixed 66-char segment can never equal. Still injective across the CLI's distinct-daemon cases; just no longer length-expanding. Test: the namespace stays <=255 bytes and creatable for profiles up to the 255-byte segment limit (127- and 255-char cases that overflowed under hex); the prior injectivity and cross-profile prune-isolation tests still hold. execenv + daemon under -race, go vet, gofmt all pass. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
41b3045efa |
MUL-4424: bound Codex app-server startup RPCs (#5319)
* fix(codex): bound app-server startup RPCs Co-authored-by: multica-agent <[email protected]> * test(codex): de-flake bounded-handshake test The single 500ms handshake bound was shared by the successful preamble RPCs, so a slow fork/exec of the /bin/sh fake app-server could make initialize spuriously time out under parallel load. Raise the test bound to 3s (still below the 5s semantic timeout and 10s harness ceiling) and loosen the elapsed assertion to match. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Eve <[email protected]> Co-authored-by: multica-agent <[email protected]> Co-authored-by: J <[email protected]> |
||
|
Jiayuan Zhang
|
a14098288b | feat: redesign agent Skills and MCP capabilities (#5277) | ||
|
ZIceandmultica-agent
|
5dfb0bec06 |
Fix Codex MCP allowlist config rendering (#4949)
Co-authored-by: multica-agent <[email protected]> |
||
|
|
1ff99e5afc |
fix: honor completed codex turns during process eof races (#4899)
Co-authored-by: Eve <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
|
cb68669c73 |
feat(composio): gate MCP apps behind feature flag (#4876)
* feat(composio): server-side connect flow + connections REST (Notion MVP) (MUL-3720) (#4608)
* feat(composio): server-side connect flow + connections REST (Notion MVP) (MUL-3720)
Compose the merged server/pkg/composio SDK into a user-facing connection
manager: signed-state connect handshake, local user_composio_connection
mirror, idempotent disconnect, and a per-user MCP session helper (not yet
wired into task dispatch).
- migration 127_user_composio_connection (no FK/cascade, per DB rules)
- sqlc queries: upsert (idempotent on user_id+connected_account_id), list
active, owner-scoped get, mark revoked
- internal/integrations/composio: signed HMAC-SHA256 state, BeginConnect,
CompleteCallback (idempotent upsert), ListConnections, Disconnect
(upstream 404 = idempotent success), CreateMCPSession (no-op when empty,
pins connected_accounts per toolkit), CallbackRedirect
- REST handlers under /api/integrations/composio (user-scoped, 503 when
COMPOSIO_API_KEY unset): connect/init, callback (302), connections list,
delete
- router wiring gated by COMPOSIO_API_KEY; COMPOSIO_AUTH_CONFIGS_JSON maps
toolkit->auth_config (MVP: notion); state secret from COMPOSIO_STATE_SECRET
or derived from JWT_SECRET; callback base from COMPOSIO_CALLBACK_BASE_URL
or MULTICA_PUBLIC_URL
- tests: state (expire/tamper/wrong-secret), service (mapping, callback
idempotency, non-success, disconnect owner/404 idempotency, MCP pin),
handlers (httptest), redact regression for Bearer mcp_ tokens
MVP scope: Notion only; no task-dispatch overlay, sharing, or webhook
event handling (later stages).
Co-authored-by: multica-agent <[email protected]>
* fix(composio): bind callback account to user + idempotent revoked disconnect (MUL-3720)
Address PR 4608 review (CHANGES_REQUESTED):
- callback: verify connected_account_id with Composio before mirroring it.
The signed state only proved user/toolkit/exp, so a valid state paired with
a tampered connected_account_id would be written verbatim. CompleteCallback
now calls ListConnectedAccounts and fails closed (ErrAccountVerification)
unless the account belongs to the state's user (composio_user_id == multica
user id) and was created under the toolkit's auth config. No row is written
on mismatch / unknown account / upstream error.
- disconnect: short-circuit to a no-op when the local row is already revoked,
before touching upstream. Previously a second DELETE re-hit Composio and a
non-404 upstream error surfaced as a 502, breaking the 204-idempotent
contract.
- CreateMCPSession: document the v1 single-active-connection-per-(user,toolkit)
constraint and make duplicate selection deterministic (newest-wins, rows are
connected_at DESC) instead of order-dependent map overwrite. Stage 3 owns the
real single-account-enforcement vs multi-account-shape decision.
Tests: tampered/wrong-auth-config/unknown-account callback rejection, revoked-row
disconnect no-op (asserts upstream not re-hit). composio pkg 85% coverage; all
green.
Co-authored-by: multica-agent <[email protected]>
* feat(composio): list all toolkits + dynamic auth-config resolution (MUL-3720)
Yushen's follow-up to the Notion MVP: surface the full Composio toolkit
catalog, render it in Settings, and drop the static env mapping in favor of
dynamic auth-config discovery.
Config correctness (per Composio docs):
- Remove COMPOSIO_AUTH_CONFIGS_JSON entirely. The toolkit→auth_config mapping
is now resolved at request time from the project's /auth_configs (cached,
5-min TTL), so enabling a toolkit is a dashboard action, not a redeploy.
- Do NOT add COMPOSIO_PROJECT_ID. The project API key (x-api-key) authenticates
to exactly one project; the project is resolved from the key. Only org-level
endpoints use x-org-api-key, which this integration never calls.
Backend:
- SDK: server/pkg/composio/auth_configs.go — ListAuthConfigs (toolkit_slug,
is_composio_managed, show_disabled, limit, cursor).
- service: dynamic resolver (authConfigMap cache; betterAuthConfig prefers a
custom/white-label config over Composio-managed, newest wins); BeginConnect
and CompleteCallback resolve via it; ListToolkits fetches the full catalog
(paginated, capped) annotated with connectable = has an enabled auth config,
connectable-first ordering.
- handler + route: GET /api/integrations/composio/toolkits (user-scoped, 503
when COMPOSIO_API_KEY unset) returning slug/name/logo/category/connectable.
Frontend:
- core: ComposioToolkit/ComposioConnection types, api client methods, and
composio query options (@multica/core/composio).
- views: Settings → Integrations now has a Composio section rendering every
toolkit as a card with search. Connect is gated on `connectable`;
non-connectable toolkits show a muted "not configured" hint instead of a
dead button. Connected toolkits show a badge + Disconnect (with confirm).
- i18n: composio block added to en/zh-Hans/ja/ko settings.
Tests: SDK + service (dynamic resolution, custom-over-managed preference,
connectable flag, resolver-error soft-degrade) and handler toolkits endpoint;
composio pkg 85.7% coverage. go build/vet/gofmt clean; core+views typecheck,
core+views lint, and core tests (691) all green.
Co-authored-by: multica-agent <[email protected]>
* fix(composio): close cross-toolkit callback fail-open by signing auth_config_id into state (MUL-3720)
Re-review blocker: CompleteCallback resolved the toolkit's auth config at
callback time and ignored a resolve error/empty result, while
verifyAccountOwnership skipped the auth-config comparison when the expected
value was empty. A user could then pass another toolkit's connected_account_id
into this toolkit's callback — the owner check passed and it was written under
the wrong toolkit_slug/account binding.
Fix: the auth_config_id is already resolved in BeginConnect (before the state
is signed), so sign it into the state and compare it exactly at callback. No
re-resolve, no fail-open. verifyAccountOwnership now fails closed when the
expected auth config is empty (rejects instead of skipping) and requires an
exact match — closing the cross-toolkit binding gap.
Tests: state round-trips auth_config_id; BeginConnect signs it; callback
rejects wrong/cross-toolkit auth config and an empty (no-mapping) auth config
fails closed. composio pkg 85.2% coverage, all green.
Frontend (non-blocking): the Composio settings tab now surfaces an error when
the connections query fails instead of silently rendering everything as
unconnected.
Co-authored-by: multica-agent <[email protected]>
* fix(composio): hide Settings section entirely when integration unconfigured (MUL-3720)
Decision (option 2, hide-then-merge): don't show a card that leaks the internal
COMPOSIO_API_KEY env-var name to every end user. IntegrationsTab now gates the
whole Composio section (heading + body) on the toolkits query — a 503 means the
key is unset, so the section is withheld instead of rendering the not-configured
card. Admin-only setup guidance is a later, role-gated affordance.
Removed the notConfigured card (and now-unused ApiError import) from
ComposioTab; it only mounts when configured. views typecheck + lint clean.
Co-authored-by: multica-agent <[email protected]>
---------
Co-authored-by: multica-agent <[email protected]>
* feat(composio): Stage 2 frontend polish — callback toast, last_used & expired UI, e2e (MUL-3718) (#4688)
* feat(composio): callback toast + refresh, last_used & expired UI, e2e (MUL-3718)
Co-authored-by: multica-agent <[email protected]>
* fix(composio): real callback redirect route + StrictMode-safe toast dedup (MUL-3718 review)
Co-authored-by: multica-agent <[email protected]>
---------
Co-authored-by: multica-agent <[email protected]>
* fix(composio): callback endpoint should not require Multica auth (MUL-3843) (#4709)
* fix(composio): move OAuth callback out of the Auth group (MUL-3843)
Composio 302-redirects the browser to /api/integrations/composio/callback
at the end of the OAuth flow, but PR #4608 mounted it inside the cookie-auth
middleware group. When the session cookie is absent (expired session,
SameSite=Strict / Safari ITP, private window, self-hosted callback subdomain)
the Auth middleware returned a hard 401 and a JSON blob instead of the
settings redirect, breaking the flow.
Identity never came from the cookie anyway: it is carried by the HMAC-signed
state param that CompleteCallback verifies (signature, expiry, replay) and
cross-checked by verifyAccountOwnership; h.Composio == nil still 503s. So the
callback is registered alongside the other public OAuth/webhook routes; the
other four composio endpoints stay session-gated.
Refs MUL-3843, MUL-3715.
Co-authored-by: multica-agent <[email protected]>
* fix(composio): correct stale callback routing comments (MUL-3843)
The package header and ComposioCallback doc comments still described the
callback as sitting under the Auth middleware group. After the route was
moved out (this PR), update both to state it is a public route whose identity
comes from the signed state — addressing review nit from 张大彪.
Refs MUL-3843.
Co-authored-by: multica-agent <[email protected]>
---------
Co-authored-by: multica-agent <[email protected]>
* feat(composio): inject MCP overlay into agent runtime at task dispatch (MUL-3721) (#4704)
Stage 3 of the Composio epic. Wires the per-user Composio MCP session into
every agent task so the agent process sees the initiator's connected tools
without any prompt-time plumbing.
Server side
- Migration 128 adds agent_task_queue.runtime_mcp_overlay JSONB plus a
BEFORE-UPDATE trigger that wipes the column on any transition into a
terminal status (completed / failed / cancelled). A trigger is the single
source of truth — future queries that flip status cannot bypass it.
- composio.Service.BuildTaskOverlay(userID) reuses CreateMCPSession and
emits the Claude-style { mcpServers: { composio: { type: http, url,
headers } } } shape the daemon's existing sidecar generators consume.
Returns (nil, nil) on zero active connections so we never burn a
Composio session for a user with nothing to call.
- TaskService grows a Composio ComposioOverlayBuilder seam, wired in
router.go after composiointeg.NewService succeeds. Five enqueue paths
(issue / mention / quick-create / chat / auto-retry) attach the overlay
after CreateAgentTask returns and before the daemon is notified — so
every claim reads a settled row, with no second daemon hop. Best-effort:
a builder failure logs and proceeds with no overlay.
- resolveInitiatorFromTriggerComment derives the initiator user from the
trigger comment when it was authored by a member. Agent-authored
triggers are not treated as initiators (their connected-apps view is
empty by construction).
Daemon side
- handler/daemon.go claim path merges task.runtime_mcp_overlay onto
agent.mcp_config via mergeMCPOverlay before populating
TaskAgentData.McpConfig. Overlay wins on server-name collisions
because it carries the live user-scoped session URL. Errors fall back
to the agent config unchanged — a bad overlay must not surprise-disable
saved MCP tools. The existing execenv sidecar generators (cursor /
codex / openclaw / opencode / hermes-kiro) need no changes: they keep
consuming the merged result through TaskAgentData.McpConfig.
Tests
- 9 merge cases (mcp_overlay_test): both-nil short-circuit, agent-only
pass-through, overlay-only canonicalization, two-side merge, name
collision (overlay wins), top-level key preservation, malformed agent
fallback, malformed overlay fallback, non-object server rejection.
- 4 dispatch cases (composio): zero-connections returns nil without
CreateSession, happy-path emits the right shape with the right user
id, empty-URL defensive branch, SDK error surfacing.
- 4 TaskService helper cases: nil Composio is a no-op (Queries-safe),
invalid initiator does not call the builder, nil overlay skips the
UPDATE, builder error swallowed without panic.
- Migration 128 verified to roll up + down + up cleanly against the test
database.
Out of scope (deferred): assignment-triggered enqueue paths with no
trigger comment get no overlay attached today (no initiator UUID flows
through enqueueIssueTask in that case). Retry paths recompute the overlay
fresh from the parent's initiator_user_id instead of inheriting the bearer
from the parent row, so a stale token can never resurface on a retry.
Co-authored-by: Eve <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* feat(composio): per-agent allowlist + originator-scoped MCP overlay (MUL-3869) (#4736)
* feat(composio): per-agent allowlist + originator-scoped MCP overlay (MUL-3869)
Stage 3.1 of the Composio epic (MUL-3721 parent). PR #4704 wired in the
runtime_mcp_overlay column and a per-task dispatch hook; this change
inverts the default from "all-on" to opt-in and locks the overlay to the
agent owner's own connected apps:
- Agents carry composio_toolkit_allowlist TEXT[]. NULL or [] => no MCP.
Owner-only read/write; non-owner GET/PUT silently redacts/drops the
field (same shape as mcp_config).
- agent_task_queue carries originator_user_id UUID. Set from the
top-of-chain HUMAN at every enqueue path:
* issue/mention comment by member -> author_id
* issue/mention comment by agent -> inherit via comment.source_task_id
-> parent task originator_user_id
* quick-create -> requester_id
* chat -> initiator_user_id
* retry -> SQL-inherited from parent row
* autopilot -> NULL (system-driven)
- BuildTaskOverlay (composio dispatch) now takes (ctx, originatorUserID,
agent) and short-circuits on five gates: invalid originator,
originator != agent.owner_id, empty allowlist, empty intersection of
allowlist ∩ active connections, defensive empty session URL. Composio
CreateSession is called with BOTH `toolkits.slugs` (the intersection)
AND `connected_accounts` (the pinned account ids), narrowing the
tool-router twice.
- The originator-vs-owner gate closes the agent-fanout privacy hole: any
workspace member who can @-mention a public agent used to project the
owner's connected apps into their run. Now the overlay only mounts
when the human at the top of the chain IS the agent owner.
Tests:
- dispatch_test.go covers all 5 gates plus uppercase/whitespace slug
normalisation.
- task_runtime_mcp_overlay_test.go covers the no-op gates of the new
applyRuntimeMCPOverlay signature.
- agent_composio_allowlist_test.go (handler): owner roundtrip
(list/empty/null), workspace-admin silent-drop, owner-only GET
visibility, pure normaliseComposioToolkitAllowlist.
- resolve_originator_test.go (service, DB-backed): member-authored,
agent-authored inherits via comment.source_task_id, invalid id.
Migration 129 up/down/up verified against docker postgres.
Co-authored-by: multica-agent <[email protected]>
* chore(composio): gofmt + regenerate sqlc with v1.31.1 (MUL-3869 review nits)
Co-authored-by: multica-agent <[email protected]>
---------
Co-authored-by: Eve <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* fix(composio): accept nested connected account auth config
* feat(views): creator-only MCP tab for per-agent Composio allowlist (MUL-3870) (#4743)
Stage 3.2 frontend on top of the Stage 3.1 backend (MUL-3869,
|
||
|
|
aa4478af52 |
fix(codex): unhang cleanup after stdout scanner overflow (#4520) (#4563)
When codex emits a single stdout line larger than the daemon's 10 MB
bufio.Scanner cap (typical trigger: thread/resume on a long-history
session), the reader goroutine returns scanner.Err()="token too long",
markProcessExited fails the in-flight RPC, and the lifecycle goroutine
enters its failure path. That path calls drainAndWait() — stdin.Close()
+ cmd.Wait() — before sending the failed Result. But cmd.Wait() never
returns: codex is alive and blocked writing the rest of the oversized
line into a stdout pipe nobody is reading, so it never reaches its
stdin read syscall and never sees the EOF. The lifecycle goroutine
therefore never sends Result{failed} to its caller, the outer daemon
blocks on the result channel, and the existing PriorSessionID-with-
empty-SessionID fallback never fires — the task is permanently
stalled and codex (Node wrapper + native Rust app-server) leaks until
the OS reaps them.
The cancel() that would have unblocked things via cmd.WaitDelay's
SIGKILL was registered as a defer AFTER drainAndWait, so LIFO defer
order put cancel last — drainAndWait blocks first, cancel never runs.
Fix:
1. drainAndWait now runs the existing graceful-then-cancel pattern
itself, in two bounded phases. Phase 1 waits for readerDone (capped
by codexGracefulShutdownTimeout, so we still give codex its OTEL
flush window on clean exits); on timeout it cancels the runCtx so
cmd.Cancel kills the tree and the reader unblocks. Phase 2 bounds
cmd.Wait() the same way for the scanner-overflow case, where
readerDone closed early but the process is still alive on a full
stdout pipe. The success-path cleanup that previously duplicated the
graceful-cancel pattern around readerDone collapses to a single
drainAndWait() call.
2. cmd.Cancel is set to send SIGKILL to the whole codex process group
(Setpgid via configureProcessGroup, signalProcessGroup on cancel)
instead of just the leader. This addresses YOMXXX's
orphaned-Codex-child concern: the Node wrapper and the native
app-server it spawns now both die when cleanup forces the kill,
rather than the native binary leaking as an orphan reparented to
init. configureProcessGroup is a no-op on Windows.
3. codexGracefulShutdownTimeoutNanos atomic.Int64 mirrors
opencodeTerminateGraceNanos so the regression test can shrink the
grace window from 10 s to 500 ms. Production code is unchanged
(default 10 s).
Outer daemon (daemon.go) already retries with a fresh session when
result.Status == "failed" && PriorSessionID != "" && result.SessionID
== ""; the failed Result now actually reaches it, so the recovery
fires on its own without any daemon-side change.
Tests:
- New regression TestCodexExecuteCleansUpWhenScannerOverflowsOnResume
spawns a fake codex that emits an 11 MB single-line thread/resume
response (trips the scanner cap) and then sleeps without re-reading
stdin. With the original drainAndWait body it blocks at the 10 s
executeFakeCodex deadline ("timeout waiting for result") — verified
by temporarily reverting just the helper body — and with the fix it
completes in ~1.3 s with Result.Status="failed",
Result.SessionID="" so the outer fallback can fire.
- Full codex test suite, full agent package, daemon + execenv +
repocache packages, go build ./..., and go vet on agent/daemon all
pass.
Out of scope (deferred to follow-up per YOMXXX): bumping the 10 MB
bufio.Scanner cap on codex / claude / copilot / cursor / hermes /
kimi / kiro / codebuddy / antigravity / qoder / openclaw / opencode
(pi already sits at 32 MB), and the shared bounded JSON-RPC line
reader that would eliminate the single-line-overflow risk class
entirely. Buffer size alone is not the fix — recovery behaviour is.
Refs: GH#4520
Co-authored-by: Eve <[email protected]>
Co-authored-by: multica-agent <[email protected]>
|
||
|
|
b13e1808a4 |
refactor(codex): make permission approval auto-grant observable (#4390)
The daemon auto-grants Codex item/permissions/requestApproval requests by echoing back the network / fileSystem profile scoped to the current turn. Previously a malformed params payload and any permission key outside network / fileSystem were dropped silently, so a future app-server protocol that adds a new permission shape would be narrowed away with no trace in daemon logs. Log both cases (parse failure and dropped keys) without changing the granted response. Addresses review nits on #4346 / MUL-3451. Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |
||
|
YOMXXX
|
4bbaf5363c | fix(codex): handle app-server permission requests (#4346) | ||
|
|
114a1ffb8f |
Fix: fail fast when Codex app-server exits MUL-2840 (#4228)
* fix: fail fast when codex process exits Co-authored-by: multica-agent <[email protected]> * fix: fail active codex turns on process exit Co-authored-by: multica-agent <[email protected]> * fix: prefer codex context terminal states Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]> |