fix(agent): cancel a persistent-session command by resetting the session

[Daily-AC]

Fixes #46. Option (b): a cancelled session is destroyed, and the next command on that target builds a fresh one. ADR: docs/adr/0011-session-cancel-resets-session.md.

Round 3. Round 1 implemented option (a) and was blocked; round 2 rebuilt it on option (b) and was blocked on how the cancellation is scoped. This round fixes those six findings. Every one has a regression test ported from the reviewer's probes, and each was confirmed to fail against the previous behaviour.

What the container does

	container	cancel
Unix	SysProcAttr.Setpgid — the shell leads a new group whose id is its own pid	SIGKILL to -pgid
Windows	created suspended, assigned to a job object with no breakaway rights, then resumed	TerminateJobObject

Creating suspended closes the window in which a wrapper shell could fork a worker before the assignment landed: a shell that cannot be contained never runs an instruction.

Round-2 findings

P1 — the gate had no request identity. A watcher could wake after its own request disarmed and after the next one armed; a shared "is anything armed" flag reads true in that state, measured at 50 misfires in 100. Every arming now takes a generation, the watcher carries the one it was armed with, and firing does nothing unless that generation is still live.

P1 — cancellation waited on the shell's descendants. Their stdout is an inherited OS pipe, so a process that left the container (set -m puts a background job in its own group; setsid() leaves outright) still holds the write end after the kill, and the copier never sees EOF. The command never returned, Closed() never answered, and because the agent asks Closed() while holding the lock that guards every session on the device, one escaped process stalled all of them.

Three changes: the cancellation closes the read half itself, Closed() is now atomic and takes no lock, and the reaper carries WaitDelay so its goroutine and the pipe's descriptors are released on a bound rather than never. Cutting the output is also what makes a failed kill reach the caller while the command it could not stop is still running, instead of whenever that command happens to end.

P2 — double Kill(-pgid). A process-group id is only a number and is reusable once the group is gone. The Unix container now signals at most once per lifetime and never after the shell is reaped; until then the leader is at worst a zombie and the kernel will not reuse its pid, so the number is still unambiguous. reap runs the moment cmd.Wait returns, under the lock the kill takes.

P2 — a request holding a cancelled session. It came back session closed for a command that never reached the device. The session now distinguishes that case (ErrSessionUnusable), and the agent answers it by acquiring a fresh session and running once. Nothing else is retried: every other error leaves open the possibility that the command did run.

P2 — Windows suspended create. Above.

P2 — post-Start cleanup. Every failure after Start now goes through one path that kills, reaps and closes both ends of the output pipe. Three injected failures previously left three copier goroutines blocked.

Regressions, and that they discriminate

Each was run against the pre-fix code:

test	pre-fix result
TestLateWatcherCannotFireForTheRequestThatFollowedIt	a watcher belonging to a finished request killed the session 1 times
TestStaleCancelCallbackCannotKillTheNextCommand	an uncancelled command was stopped by an earlier request's cancellation
TestEscapedDescendantCannotHoldACancelledSession	the cancel waited 2.001123s for a process that had left the container
TestFailedKillIsReportedWhileTheCommandStillRuns	the command never returned
TestRequestHoldingACancelledSessionGetsAFreshShell	reported session closed before the command was submitted

The last two bound on sessionWaitDelay/2 on purpose. The reaper eventually closes the pipes anyway, so a looser bound passed without the cancellation cutting the output at all — the 2.001 s above is exactly that, and is why the bound was tightened.

Also: TestContainerKillsOnceAndNeverAfterReap, TestShellThatCannotBeContainedIsCleanedUp (goroutine count across three failed starts), plus the round-2 set — compound statement after the cancel is not executed, pre-cancelled request runs nothing, set -e, and the chain-level acceptance.

Verification

gofmt -l . && go build ./... && go vet ./... && go test ./...   # clean, rebased on main
go test -race ./internal/server/ ./internal/agent/              # clean
GOOS=windows GOARCH=amd64 go build ./...                        # clean

Chain level: the device-side sleep 600 ended 52 ms after the controller stream closed, the next exec works, and it sees a fresh session — not the cancelled one's cwd, and $WANCTL_MARK empty.

The cost, stated where callers read it

Cancelling loses the session's working directory and environment. The exec catalog entry and the regenerated docs/contract.md say so and point at --oneshot.

Not verified here

• Windows was not run. The job-object and suspended-resume paths cross-compile and vet; nothing executed against a real powershell.exe.
• GOOS=windows go vet ./... fails on syscall.Kill in internal/client/exec_cancel_test.go and internal/agent/delegation_test.go. Both are pre-existing on main and outside this diff. This branch's own test files are build-tagged !windows.
• What still escapes: on Unix, anything that leaves the process group — setsid(), a double-fork, and more easily setpgid() including the set -m that job control uses. What such a process can no longer do is hold the session open. On Windows nothing the contained shell creates escapes; a process it asks another service to create (Win32_Process.Create over WMI) is created by that service and is not in the job.

🤖 Generated with Claude Code