Files
multica/server/pkg/agent/cursor_background_process_windows_test.go
Bohan Jiangandmultica-agent f41fae6b08 fix(cursor): own Windows background shells by launch Job membership (MUL-7417) (#8638)
* fix(cursor): own Windows background shells by launch Job membership (MUL-7417)

captureCursorBackgroundProcess on Windows walked a machine-wide PID->PPID
snapshot to prove a reported shell descends from the launch and to find
its pre-existing children. Windows keeps a child's recorded PPID after
the parent exits and reuses the PID, so the walk could reach a process
owned by someone else; opening it failed with 'Access is denied' or
'not an owned descendant' and the whole capture aborted. The daemon then
fell back to the idle watchdog and a Cursor run that later went quiet
took the 10-minute watchdog path instead of completing. In CI this was
TestCursorBackgroundLifecycle/multiple failing about 2 runs in 43.

Take candidates from the launch's own Job Object instead
(QueryInformationJobObject / JobObjectBasicProcessIdList). Descendants
inherit the Job at creation and cannot leave it, so membership is the
ownership proof: the parent-chain walk goes away, the PPID snapshot is
used only to attach a member's pre-existing children, and a member that
exits or is reused between listing and open is skipped instead of
failing the capture. Creation ordering still tells a real child from
one that points at a reused parent PID.

Restore TestCaptureCursorBackgroundLateDescendant on Windows; it was
excluded for exactly this race.

ci.yml runs the Windows Cursor step with -count=20 on this PR only to
show the race is gone; it is reverted before merge.

Co-authored-by: multica-agent <[email protected]>

* ci: restore -count=1 on the Windows Cursor background step

The -count=20 stress run on this PR passed (run 35580440594, windows-execenv);
the per-merge step goes back to a single pass.

Co-authored-by: multica-agent <[email protected]>

* fix(cursor): complete the Job member list and refuse unopenable live members (MUL-7417)

Review follow-ups on #8638:

- cursorJobProcessIDs grew its buffer only on ERROR_MORE_DATA. The kernel
  can also return success with NumberOfProcessIdsInList short of
  NumberOfAssignedProcesses, which a Job over the initial capacity or one
  growing during the query would hit; the short list then passed for the
  membership and could reject the target or leave children out. Retry
  until the counts agree, fail explicitly if they never do, and make the
  initial capacity a package var so a three-process tree exercises the
  growth path in a test.

- The child walk skipped every open failure as 'exited or reused'. A live
  member whose DACL denies PROCESS_SET_QUOTA|PROCESS_TERMINATE also fails
  to open, and skipping it left the capture reporting success with that
  member outside the tool Job. Skip only PIDs proven to carry no member
  (handle outside the launch Job, or no longer listed after a re-query,
  confirmed by one more open); otherwise fail the capture so the caller
  takes its existing watchdog fallback. OpenProcess goes through a
  package var so a test can make a live member unopenable.

Windows-only tests cover both, plus the plain claim-and-terminate path
for a shell with a pre-existing child.

Co-authored-by: multica-agent <[email protected]>

* test(cursor): name the Job list growth test so the Windows CI filter runs it

The windows-execenv step selects 'TestCursorBackground|TestCaptureCursorBackground';
TestCursorJobProcessIDsGrowsPastInitialCapacity matched neither and never ran.

Co-authored-by: multica-agent <[email protected]>

---------

Co-authored-by: multica-agent <[email protected]>
2026-09-21 18:36:34 +08:00

123 lines
4.1 KiB
Go

//go:build windows
package agent
import (
"encoding/json"
"log/slog"
"os"
"os/exec"
"path/filepath"
"syscall"
"testing"
"time"
"golang.org/x/sys/windows"
)
// spawnCursorTestTree starts the fake cursor-agent in its default mode, which
// launches one background shell that launches one leaf and publishes
// [shell, leaf] to <dir>/0.json. All three live in the launch's Job.
func spawnCursorTestTree(t *testing.T) (root *exec.Cmd, shell, leaf int) {
t.Helper()
dir := t.TempDir()
root = exec.Command(os.Args[0])
root.Env = append(os.Environ(), cursorFakeModeEnv+"=natural", "CURSOR_FAKE_DIR="+dir, "CURSOR_FAKE_DURATION=30s")
hideAgentWindow(root)
if err := startOwnedProcessTree(root, slog.Default()); err != nil {
t.Fatal(err)
}
t.Cleanup(func() { signalProcessGroup(root, syscall.SIGKILL); _ = root.Wait(); releaseProcessGroup(root) })
deadline := time.Now().Add(5 * time.Second)
for time.Now().Before(deadline) {
var pids []int
data, _ := os.ReadFile(filepath.Join(dir, "0.json"))
if json.Unmarshal(data, &pids) == nil && len(pids) == 2 {
return root, pids[0], pids[1]
}
time.Sleep(5 * time.Millisecond)
}
t.Fatal("fixture did not publish shell and leaf PIDs")
return nil, 0, 0
}
func assertCursorTestProcessAlive(t *testing.T, pid int) {
t.Helper()
handle, err := windows.OpenProcess(windows.SYNCHRONIZE, false, uint32(pid))
if err != nil {
t.Fatalf("process %d is gone: %v", pid, err)
}
defer windows.CloseHandle(handle)
status, err := windows.WaitForSingleObject(handle, 0)
if err != nil || status != uint32(windows.WAIT_TIMEOUT) {
t.Fatalf("process %d exited: status=%d err=%v", pid, status, err)
}
}
// A short PID list is not the membership. With the first buffer sized below
// the tree, the query must grow until the two counts agree and return every
// member, whether the kernel signals the shortfall with ERROR_MORE_DATA or
// only through the counts.
func TestCaptureCursorBackgroundJobListGrowsPastInitialCapacity(t *testing.T) {
root, shell, leaf := spawnCursorTestTree(t)
tree, ok := lookupProcessTree(root)
if !ok {
t.Fatal("launch is not owned")
}
saved := cursorJobProcessIDListCapacity
t.Cleanup(func() { cursorJobProcessIDListCapacity = saved })
for _, capacity := range []int{1, 2, 3} {
cursorJobProcessIDListCapacity = capacity
members, err := cursorJobProcessIDs(tree.job)
if err != nil {
t.Fatalf("capacity %d: %v", capacity, err)
}
for _, pid := range []int{root.Process.Pid, shell, leaf} {
if _, ok := members[uint32(pid)]; !ok {
t.Fatalf("capacity %d: member %d missing from %v", capacity, pid, members)
}
}
}
}
// A launch member that is alive but cannot be opened must fail the capture,
// not be skipped: skipped, it would sit outside the tool Job and outlive the
// tool's completion. The failed capture must also leave the healthy shell and
// its child running, exactly as before the attempt.
func TestCaptureCursorBackgroundProcessFailsWhenLiveMemberCannotBeOpened(t *testing.T) {
root, shell, leaf := spawnCursorTestTree(t)
saved := cursorOpenProcess
t.Cleanup(func() { cursorOpenProcess = saved })
cursorOpenProcess = func(access uint32, inherit bool, pid uint32) (windows.Handle, error) {
if pid == uint32(leaf) {
return 0, windows.ERROR_ACCESS_DENIED
}
return saved(access, inherit, pid)
}
if p, err := captureCursorBackgroundProcess(root, shell); err == nil {
p.Close()
t.Fatal("capture succeeded while a live launch member could not be opened")
}
assertCursorTestProcessAlive(t, shell)
assertCursorTestProcessAlive(t, leaf)
}
// Sanity for the same tree with nothing in the way: the shell and its
// pre-existing child are both claimed and both die with the tool Job.
func TestCaptureCursorBackgroundProcessClaimsExistingChild(t *testing.T) {
root, shell, leaf := spawnCursorTestTree(t)
owned, err := captureCursorBackgroundProcess(root, shell)
if err != nil {
t.Fatal(err)
}
if err := owned.Terminate(); err != nil {
t.Fatalf("terminate: %v", err)
}
assertCursorTestProcessGone(t, shell)
assertCursorTestProcessGone(t, leaf)
if alive, err := owned.Alive(); alive || err != nil {
t.Fatalf("tool job still alive: alive=%v err=%v", alive, err)
}
owned.Close()
}