Files
multica/server/pkg/agent/cursor_background_process_linux.go
8661e586f6 MUL-7063 fix(cursor): bound stalled runs after background shell launch (#8050)
* 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]>
2026-09-08 16:24:41 +08:00

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
}