mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-08 01:16:45 +08:00
* fix(agent): supervise Cursor background shells with a liveness guard Cursor's stream-json reports a launched background shell as the `isBackground` boolean at the root of its completed tool-call result, and the adapter acted on none of it: a top-level execution that went quiet after launching one kept holding its runtime slot until the execution deadline (upstream #7833). Bound that shape instead of condemning it. The structural observation now arms a Cursor-private liveness guard: a rolling semantic-progress deadline (10 minutes in production, overridable per backend so tests do not have to wait a minute-scale timeout) while execution continues normally. Only a supervised run that stops progressing and never emits its terminal result is ended, and it fails closed with a dedicated, content-free reason rather than a generic cancellation. Precedence is decided when the cancellation happens, not guessed at finalization: fire() declines to claim a run the execution deadline or the caller had already ended, so `timeout` and `aborted` keep their classifications, while a cancellation the guard provoked is never rewritten into "execution cancelled". A valid terminal Cursor result outranks a latched background observation and still completes. Detection is structural only — no reading of the command string, no substring of captured output, no assistant text — and malformed or non-object result metadata leaves the call foreground, so it cannot arm the guard or fail the rest of the tool parse. Raw result bytes are forwarded unchanged. The guard owns one goroutine and one timer per execution, started on the first background observation and waited for before the reader hands over its last channel; the overwhelmingly common foreground run never arms it and pays nothing. Tests run cross-platform by re-executing the test binary as a fake cursor-agent, so the pathological stream stays alive and only the daemon can end it. Test A is RED on unpatched main, where the same run is bounded solely by the 6s execution deadline with status "timeout". Co-authored-by: multica-agent <[email protected]> * fix(agent): keep the Cursor liveness guard on its own evidence Cross-verification of 15605057e found three places where the guard acted on weaker evidence than the spec allows, plus one helper that did not do what it documented. - cursorSemanticProgressEvent believed every `tool_call` subtype. Execute handles `started` and `completed` and counts anything else as unhandled, so a stream of subtypes it drops could extend the deadline forever. Progress is now limited to the subtypes that are actually handled. - The lifecycle observation read `isBackground` off any completed tool result. The flag the spec names lives on the shell payload, and another tool reporting it is not evidence that Cursor detached a process, so the read is gated on the `shellToolCall` key. An unrecognised payload key now leaves the run unsupervised, which is the pre-existing behaviour, not a new failure. - Precedence between a native terminal result and the guard is now decided when the result is observed. A result that lands after the guard ended the run is late evidence: it no longer rewrites the failure to `completed`, its text is not exposed as output, and the run keeps the guard's own reason. A cause the guard declined to claim — a real deadline or a caller cancellation — still leaves the result authoritative. - cursorBackgroundGuard.Stop closes its channels through sync.Once, matching the idempotency its doc claims, and returns the guard's cause as of that moment, which is what makes the ordering above decidable at all: after fire() the guard's cancellation is otherwise indistinguishable from the caller's. Tests: the guard's own contract (fire-once, progress extension, yielding to real causes, Stop's report, concurrent Stop) is unit-tested directly; the observation rules are pinned by structural table cases and by two subprocess fakes — a stalled non-shell `isBackground` must still end on the execution timeout, not the guard. Co-authored-by: multica-agent <[email protected]> * chore(agent): clarify Cursor fake dispatch comment in TestMain Co-authored-by: multica-agent <[email protected]> * fix(cursor): recheck observed progress before liveness cancellation Record semantic progress under the guard mutex and recheck its deadline when handling timer or progress notifications, so a stale tick cannot cancel an active run. Keep the background-liveness failure content-free. Add a deterministic virtual-time regression for the expiry boundary and strengthen the noisy subprocess regression to check the exact error. Validation: focused regressions fail before the fix and pass after it; Cursor/ParseCursor tests pass with -race; go vet ./pkg/agent passes. Co-authored-by: multica-agent <[email protected]> * fix(cursor): supervise background tools with the daemon tool budget Co-authored-by: multica-agent <[email protected]> * fix(cursor): retain background ownership and refresh watchdog state Co-authored-by: multica-agent <[email protected]> * fix(cursor): validate unreaped Darwin group anchors Co-authored-by: multica-agent <[email protected]> * fix(cursor): distinguish empty Darwin groups from lost ownership Co-authored-by: multica-agent <[email protected]> * test(cursor): isolate reviewed baseline regression checks Co-authored-by: multica-agent <[email protected]> * test(cursor): stop platform checks at the first failure Co-authored-by: multica-agent <[email protected]> * fix(cursor): verify Darwin process identities during cleanup Co-authored-by: multica-agent <[email protected]> * test(cursor): use standard agent limits on macOS Co-authored-by: multica-agent <[email protected]> * test(cursor): keep lifecycle reproductions ahead of full suite Co-authored-by: multica-agent <[email protected]> * test(cursor): keep macOS CI focused on lifecycle Co-authored-by: multica-agent <[email protected]> * chore(ci): drop the PR-specific Cursor regression reproduction steps The two steps gated on `github.event.pull_request.number == 8050` proved the new regression tests were RED on earlier heads of this PR. That is useful while the PR is open and dead weight once it merges: the condition can never hold again on main, and both steps pin commit SHAs that only exist on a closed PR's branch. The permanent coverage stays: the lifecycle job still runs on ubuntu, macOS and Windows, and macOS still repeats the finalization test and checks the cgo-free ownership path. Co-authored-by: multica-agent <[email protected]> * docs(daemon): state what a zero tool budget means for a background shell Keeping a Cursor background shell in flight for the launched process's whole lifetime is what puts a legitimate long background job on the tool budget instead of the shorter idle one. It also changes what MULTICA_AGENT_TOOL_WATCHDOG=0 means in practice: such a run is now bounded only by MULTICA_AGENT_TIMEOUT, which is 0 by default. That follows from the invariant rather than contradicting it — a foreground tool that never returns already behaves this way — but an operator reading "never force-stop during a tool call" would not predict it, so say it where the knob is defined. Co-authored-by: multica-agent <[email protected]> * test(daemon): pin the captured-background zero tool budget outcome TestCursorBackgroundUncapturedDisabledToolWatchdog covers the case where nothing was claimed, and releases the tool so the idle budget still applies. Its mirror was untested: when ownership IS held the process is genuinely in flight, so a zero tool budget deliberately force-stops nothing. Without this test the two cases look like an inconsistency and invite a later "fix" that quietly changes what the knob promises. Co-authored-by: multica-agent <[email protected]> * fix(daemon): let an observed terminal result outrank the idle watchdog A Cursor run could still be reported as a hang after it had already succeeded. On reading its authoritative result the backend called background.Close() synchronously, before Session.Result reached the daemon. That call takes the tracker lock, so it can queue behind a tool-watchdog Interrupt already in progress — and that Interrupt is allowed to return false without moving native accounting or its timestamp. The watchdog's revalidation then saw an unchanged count, an unchanged timestamp and an empty queue, cancelled the run, and executeAndDrain re-tagged the completed result as idle_watchdog. The missing piece was a lifecycle boundary the daemon could see: cleanup succeeding is not the same fact as the outcome being decided, and only the latter should silence a liveness policy. Session.TerminalObserved carries it, the Cursor backend publishes it before any cleanup that can block or fail, and the watchdog checks it both before interrupting and immediately before force-stopping. The re-tag at Session.Result is gated on it too, so even a watchdog that wins the last instant cannot rewrite a decided outcome. Both regression tests drive the exact interleaving through channel handshakes rather than sleeps: the cleanup callback itself publishes the terminal observation, so the terminal result provably lands while cleanup is in progress. They fail on the parent commit with 'fired=true' and 'Status:idle_watchdog' respectively. Co-authored-by: multica-agent <[email protected]> * docs: sync the Cursor background watchdog contract to ja/ko/zh The English page gained three operator-facing contracts that the other three locales still described with the pre-change one-liner: that a Cursor background shell stays in flight until its owned processes exit, that normal finalization stops those processes, and what the per-platform fail-closed limits are. These decide whether an operator sets an overall timeout, sets a tool budget, and expects a background server to survive a run, so they cannot ship in one locale only — the same watchdog settings are already maintained in four languages. Also states explicitly, in all four, that a zero tool budget leaves claimed background work in flight for as long as it lives, bounding the run only by MULTICA_AGENT_TIMEOUT, which is itself 0 by default. Co-authored-by: multica-agent <[email protected]> * fix(daemon): keep a decided outcome through the drain-timeout classifier The terminal boundary was checked in the watchdog and on the Result arm, but not on the one path a losing race actually takes. The watchdog reads terminalObserved and only then writes fired and cancels, so Cursor can publish its terminal result in between. The cancellation that follows does not surface on the Result arm — the backend is still finalizing and cannot deliver Result yet — it surfaces on drainCtx.Done(), which classified purely on the fired flag and returned idle_watchdog with an empty output for a run that had already succeeded. Check the boundary there too and wait, bounded, for the real result. Falling through on expiry keeps the previous behaviour, so the backstop cannot hang a run; the backend bounds its own finalization, which is what makes the wait short in practice. The earlier regression test could not reach this: there the watchdog returned at its final gate, so fired was never set and neither the rewrite guard nor this classifier ran. The new test makes TerminalObserved publish on the read that the watchdog's final gate performs while still answering false to that reader, which is the losing race exactly, and asserts the watchdog really did cancel before checking the outcome. It fails on the parent commit with Status:idle_watchdog and an emptied output. Co-authored-by: multica-agent <[email protected]> * fix(agent): bound the whole of Cursor background cleanup at close Each process termination was bounded, but the number of background shells a run may launch is not, so finalization as a whole was not. That matters only after a terminal result — precisely when the daemon's watchdog has deliberately stopped supervising the run and MULTICA_AGENT_TIMEOUT is 0 by default, leaving nothing else to bound it. Give Close() one budget for the whole pass. Work still unconfirmed when it expires takes the existing unconfirmed-at-close path, so its launch result is still surfaced and its cleanup is still logged as unconfirmed rather than reported as successful. Reap and Interrupt keep their previous behaviour. The test drives 40 tools whose termination never confirms; it takes ~8s on the parent commit and stays inside the budget here. Co-authored-by: multica-agent <[email protected]> * docs(ja): match the repo's existing wording in the Cursor watchdog contract Uses 最終結果 as elsewhere in the docs, makes the two signalling clauses say シグナル送信 explicitly, and names プロセス捕捉時 and the background process rather than the launch. Co-authored-by: multica-agent <[email protected]> * fix(daemon): linearize the post-force-stop verdict on the result hand-off Checking terminalObserved before classifying only moved the window: the backend can publish between that read and the classifier that follows it. Reading a flag and then acting on it cannot be made safe by adding more read sites. Enter the hand-off unconditionally once the watchdog has fired, and decide from what arrives. The result's delivery is then the linearization point, and the backend contract — publish the observation before sending Result — is what makes the check after delivery reliable rather than lucky. The Result arm already read in that order; this brings the drain arm in line. An expired budget still linearizes to the liveness verdict, so the branch stays bounded whatever a backend does. The previous test could not reach this: its backend sent Result immediately on cancellation, so the outer select could satisfy itself on the Result arm and the drain classifier never ran. The new one withholds Result until the daemon has entered the hand-off, using the hand-off's own log record as both the gate and the evidence that the branch executed — otherwise a passing status proves nothing about which arm produced it. On the parent commit it fails with 'the post-force-stop hand-off never ran'. Co-authored-by: multica-agent <[email protected]> * fix(agent): bound every locked background-cleanup pass, not just the closing one Close() took its deadline before the lock, which is correct, but the deadline could not shorten the wait for it. A concurrent Interrupt() still scanned an unbounded number of tools with no deadline of its own, so an unbounded pass held by anyone else was an unbounded Close() one level down — and interrupt-holds-the-lock-while-the-terminal-result-arrives is the exact topology this boundary exists for. Bound every pass that holds the lock by the same budget. If waiting for the lock consumed it, the closing pass takes a fresh one rather than pushing every tool onto the unconfirmed path without trying; two budgets is the honest ceiling, and terminalResultHandoffBudget is derived from exactly that. The existing test had no concurrent holder, so it could not see this. The new one puts an Interrupt() pass on the lock first: it takes ~1.1s on the parent commit against a 100ms budget. Co-authored-by: multica-agent <[email protected]> * fix(daemon): scope the result hand-off to backends that publish a boundary Entering the hand-off on every force stop taxed the case the watchdog exists for. A wedged backend has no result that could outrank the stop, so waiting for one only delays freeing the runtime slot — the three existing idle-watchdog latency tests caught this immediately, each taking the full budget instead of one window. Nil TerminalObserved is the signal and is now kept distinguishable rather than collapsed into a false-returning shim at the point of use. Backends without a boundary keep exactly their previous behaviour. Cursor, which has one, always closes Result, so even a wedged Cursor run ends this wait through the closed channel rather than the budget; the budget remains as the bound for a backend that neither delivers nor closes. The new test pins the fast path so the hand-off cannot quietly become a tax on every force stop again. Co-authored-by: multica-agent <[email protected]> * test(agent): prove the interrupting pass holds the lock before Close asks The gate was closed just before calling Interrupt(), which only says the goroutine was about to try. Close() could win the lock, finish on its own, and the test would pass even against an unbounded Interrupt() — a regression test that can go green without exercising what it guards. Close the gate from inside the first process's terminate() instead: finish() holds mu for the whole pass, so reaching it is proof the pass owns the lock. The ordering is now a fact rather than a scheduling habit. It did not misbehave in practice — with the old gate the test still failed 110/110 against an unbounded Interrupt(), including at GOMAXPROCS=1 and 2, because close() does not yield the P. It is now deterministic by construction: 20/20 at GOMAXPROCS 1, 2 and 8. Also corrects the handoff budget derivation: closing the reaper's stop channel wakes it immediately, so its tick is not a term. What deserves the margin is an already-started termination overshooting its pass's deadline by its own bound. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: worker-opencode <[email protected]> Co-authored-by: multica-agent <[email protected]> Co-authored-by: J <[email protected]>
114 lines
3.1 KiB
Go
114 lines
3.1 KiB
Go
//go:build linux
|
|
|
|
package agent
|
|
|
|
import (
|
|
"fmt"
|
|
"os"
|
|
"path/filepath"
|
|
"strconv"
|
|
"strings"
|
|
"syscall"
|
|
|
|
"golang.org/x/sys/unix"
|
|
)
|
|
|
|
// Linux 6.9's UAPI flag is not yet exported by x/sys/unix. It targets the
|
|
// pidfd's retained struct pid as a group, including children after leader exit.
|
|
const cursorPidfdSignalProcessGroup = 1 << 2
|
|
|
|
type cursorUnixGroupHandle struct{ fd int }
|
|
|
|
func captureCursorUnixGroup(pgid int) (*cursorUnixGroupHandle, error) {
|
|
fd, err := unix.PidfdOpen(pgid, 0)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
g := &cursorUnixGroupHandle{fd: fd}
|
|
// Probe support without sending a signal. Older kernels fail closed;
|
|
// silently falling back to kill(-pgid) would lose durable ownership.
|
|
if err := g.signal(0); err != nil {
|
|
g.close()
|
|
return nil, err
|
|
}
|
|
return g, nil
|
|
}
|
|
|
|
func (g *cursorUnixGroupHandle) signal(sig syscall.Signal) error {
|
|
return unix.PidfdSendSignal(g.fd, unix.Signal(sig), nil, cursorPidfdSignalProcessGroup)
|
|
}
|
|
func (g *cursorUnixGroupHandle) close() { _ = unix.Close(g.fd) }
|
|
|
|
func readCursorUnixProcessInfo(pid int) (cursorUnixProcessInfo, error) {
|
|
data, err := os.ReadFile(filepath.Join("/proc", strconv.Itoa(pid), "stat"))
|
|
if err != nil {
|
|
return cursorUnixProcessInfo{}, err
|
|
}
|
|
raw := string(data)
|
|
openParen := strings.IndexByte(raw, '(')
|
|
closeParen := strings.LastIndex(raw, ") ")
|
|
if openParen <= 0 || closeParen < openParen {
|
|
return cursorUnixProcessInfo{}, fmt.Errorf("malformed /proc/%d/stat", pid)
|
|
}
|
|
parsedPID, err := strconv.Atoi(strings.TrimSpace(raw[:openParen]))
|
|
if err != nil || parsedPID != pid {
|
|
return cursorUnixProcessInfo{}, fmt.Errorf("unexpected pid in /proc/%d/stat", pid)
|
|
}
|
|
fields := strings.Fields(raw[closeParen+2:])
|
|
if len(fields) <= 19 {
|
|
return cursorUnixProcessInfo{}, fmt.Errorf("short /proc/%d/stat", pid)
|
|
}
|
|
ppid, err := strconv.Atoi(fields[1])
|
|
if err != nil {
|
|
return cursorUnixProcessInfo{}, err
|
|
}
|
|
pgid, err := strconv.Atoi(fields[2])
|
|
if err != nil {
|
|
return cursorUnixProcessInfo{}, err
|
|
}
|
|
start, err := strconv.ParseUint(fields[19], 10, 64)
|
|
if err != nil {
|
|
return cursorUnixProcessInfo{}, err
|
|
}
|
|
return cursorUnixProcessInfo{pid: pid, ppid: ppid, pgid: pgid, start: start, zombie: fields[0] == "Z"}, nil
|
|
}
|
|
|
|
func listCursorUnixProcessInfos() ([]cursorUnixProcessInfo, error) {
|
|
entries, err := os.ReadDir("/proc")
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
infos := make([]cursorUnixProcessInfo, 0, len(entries))
|
|
for _, entry := range entries {
|
|
pid, err := strconv.Atoi(entry.Name())
|
|
if err != nil {
|
|
continue
|
|
}
|
|
info, err := readCursorUnixProcessInfo(pid)
|
|
if err == nil && !info.zombie {
|
|
infos = append(infos, info)
|
|
}
|
|
}
|
|
return infos, nil
|
|
}
|
|
|
|
func cursorUnixIsDescendant(pid int, root cursorUnixProcessInfo) bool {
|
|
seen := make(map[int]struct{}, 8)
|
|
for pid > 1 {
|
|
if pid == root.pid {
|
|
info, err := readCursorUnixProcessInfo(pid)
|
|
return err == nil && info.pid == root.pid && info.start == root.start
|
|
}
|
|
if _, ok := seen[pid]; ok {
|
|
return false
|
|
}
|
|
seen[pid] = struct{}{}
|
|
info, err := readCursorUnixProcessInfo(pid)
|
|
if err != nil {
|
|
return false
|
|
}
|
|
pid = info.ppid
|
|
}
|
|
return false
|
|
}
|