test(tools): assert the stop intent killed the session

The retry loop added to survive a slow PowerShell teardown also made the
test unfalsifiable. The helper sleeps 5s, well inside the 30s deadline, so
a terminate that did nothing at all would let the sleep end on its own and
every existing assertion would still hold: the session reports exited, it
reports an exit code, and the interrupted flag echoes the request rather
than the outcome. Neutering KillProcessTree confirmed it, the test passed.

Assert instead that the helper's completion line never appears, which
distinguishes a killed session from a finished one without depending on
how long the loop took. Output is accumulated across every poll rather
than read off the last result, because each continuation returns only what
the session produced since the previous collection.

internal/tools/bash_tool_test.go

@@ -625,7 +625,14 @@ func TestBashToolRequireEscalatedMsysGuard(t *testing.T) {
 
 	t.Run("approved require_escalated still blocks an unrelated syntax issue", func(t *testing.T) {
 		result := registry.RunWithOptions(context.Background(), "bash", map[string]any{
-			"command":             `cd /d/tmp/zero-pr-158 && dir`,
+			// No `&&` here on purpose. The POSIX-style path is what this case is
+			// about, and on Windows PowerShell 5.1 a chained command trips the
+			// separate windows_powershell_version preflight first, which shadows
+			// the block being asserted. That only shows up on 5.1: the runners
+			// have PowerShell 7, where && is valid and the version check does not
+			// fire, so the substitution kept this green in CI while failing on the
+			// shell a stock Windows box actually gets.
+			"command":             `cd /d/tmp/zero-pr-158`,
 			"sandbox_permissions": string(SandboxPermissionsRequireEscalated),
 		}, RunOptions{
 			PermissionGranted: true,

internal/tools/exec_command_test.go

@@ -18,7 +18,7 @@ import (
 )
 
 func TestIndependentExecCommandConstructorsShareDefaultManager(t *testing.T) {
-	root := t.TempDir()
+	root := execTestRoot(t)
 	execTool := NewScopedExecCommandTool(root, nil, nil)
 	writeTool := NewWriteStdinTool(nil)
 
@@ -90,7 +90,7 @@ func TestExecCommandToolDescribesHostShellSyntax(t *testing.T) {
 }
 
 func TestExecCommandReturnsSessionAndWriteStdinPollsCompletion(t *testing.T) {
-	root := t.TempDir()
+	root := execTestRoot(t)
 	manager := newExecSessionManager()
 	execTool := NewScopedExecCommandTool(root, nil, manager)
 	writeTool := NewWriteStdinTool(manager)
@@ -138,7 +138,7 @@ func TestExecCommandReturnsSessionAndWriteStdinPollsCompletion(t *testing.T) {
 }
 
 func TestExecCommandRequireEscalatedBypassesNativeSandboxAfterApproval(t *testing.T) {
-	root := t.TempDir()
+	root := execTestRoot(t)
 	manager := newExecSessionManager()
 	registry := NewRegistry()
 	registry.Register(NewScopedExecCommandTool(root, nil, manager))
@@ -187,7 +187,7 @@ func TestExecCommandRequireEscalatedBypassesMsysGuardAfterApproval(t *testing.T)
 	if runtime.GOOS != "windows" {
 		t.Skip("windows-only MSYS sandbox guard")
 	}
-	root := t.TempDir()
+	root := execTestRoot(t)
 	manager := newExecSessionManager()
 	registry := NewRegistry()
 	registry.Register(NewScopedExecCommandTool(root, nil, manager))
@@ -224,7 +224,7 @@ func TestExecCommandRequireEscalatedBypassesMsysGuardAfterApproval(t *testing.T)
 }
 
 func TestExecCommandReturnsExitCodeWhenCommandCompletesDuringInitialYield(t *testing.T) {
-	root := t.TempDir()
+	root := execTestRoot(t)
 	manager := newExecSessionManager()
 	execTool := NewScopedExecCommandTool(root, nil, manager)
 
@@ -263,7 +263,7 @@ func TestExecCommandReportsWorkspaceChanges(t *testing.T) {
 	if runtime.GOOS == "windows" {
 		t.Skip("uses POSIX shell syntax")
 	}
-	root := t.TempDir()
+	root := execTestRoot(t)
 	result := NewScopedExecCommandTool(root, nil, newExecSessionManager()).Run(context.Background(), map[string]any{
 		"cmd":           "mkdir -p src node_modules/pkg && printf 'export {}' > src/main.ts && printf generated > node_modules/pkg/index.js",
 		"yield_time_ms": 30000,
@@ -357,7 +357,7 @@ func executionRequestHasCapability(request execution.Request, kind execution.Cap
 }
 
 func TestExecCommandForegroundServerReturnsSessionAndServesHTTP(t *testing.T) {
-	root := t.TempDir()
+	root := execTestRoot(t)
 	manager := newExecSessionManager()
 	execTool := NewScopedExecCommandTool(root, nil, manager)
 	writeTool := NewWriteStdinTool(manager)
@@ -411,7 +411,7 @@ func parseListeningAddress(output string) string {
 }
 
 func TestExecCommandReapsFinishedUnpolledSession(t *testing.T) {
-	root := t.TempDir()
+	root := execTestRoot(t)
 	manager := execution.NewProcessManager(execution.ProcessManagerOptions{CompletedRetention: 10 * time.Millisecond})
 	execTool := NewScopedExecCommandTool(root, nil, manager)
 
@@ -537,7 +537,7 @@ func TestWriteStdinInterruptTerminatesSession(t *testing.T) {
 }
 
 func TestWriteStdinRejectsInputForNonTTYSession(t *testing.T) {
-	root := t.TempDir()
+	root := execTestRoot(t)
 	manager := newExecSessionManager()
 	execTool := NewScopedExecCommandTool(root, nil, manager)
 	writeTool := NewWriteStdinTool(manager)
@@ -568,9 +568,43 @@ func TestWriteStdinRejectsInputForNonTTYSession(t *testing.T) {
 	manager.Stop(sessionID)
 }
 
+// execTestRoot is t.TempDir with a cleanup that tolerates Windows still holding
+// the directory.
+//
+// A terminated process does not release its handles the instant the session
+// reports it exited, and Windows refuses to remove a directory with an open
+// handle where POSIX does not. t.TempDir's own cleanup removes once and fails
+// the test on a sharing violation, which turns ordinary teardown timing into a
+// test failure with nothing actually wrong underneath it.
+func execTestRoot(t *testing.T) string {
+	t.Helper()
+
+	root, err := os.MkdirTemp("", "zero-exec-test-")
+	if err != nil {
+		t.Fatalf("create test root: %v", err)
+	}
+	t.Cleanup(func() {
+		deadline := time.Now().Add(10 * time.Second)
+		for {
+			if err := os.RemoveAll(root); err == nil {
+				return
+			}
+			if time.Now().After(deadline) {
+				// Deliberately not a failure. The directory is under the OS temp
+				// root and the process holding it has already been terminated, so
+				// the worst case is a stale directory, not a broken test.
+				t.Logf("test root %s still held after the cleanup deadline; leaving it", root)
+				return
+			}
+			time.Sleep(50 * time.Millisecond)
+		}
+	})
+	return root
+}
+
 func TestWriteStdinStopIntentTerminatesNonTTYSession(t *testing.T) {
 	for _, chars := range []string{`\u0003`, "exit\n"} {
-		root := t.TempDir()
+		root := execTestRoot(t)
 		manager := newExecSessionManager()
 		execTool := NewScopedExecCommandTool(root, nil, manager)
 		writeTool := NewWriteStdinTool(manager)
@@ -587,16 +621,46 @@ func TestWriteStdinStopIntentTerminatesNonTTYSession(t *testing.T) {
 			t.Fatalf("session_id is not numeric: %v", err)
 		}
 
-		result := writeTool.Run(context.Background(), map[string]any{
-			"session_id":    sessionID,
-			"chars":         chars,
-			"yield_time_ms": 1000,
-		})
-		if result.Status != StatusOK {
-			t.Fatalf("stop input %q status = %s: %s", chars, result.Status, result.Output)
-		}
-		if result.Meta["session_id"] != "" {
-			t.Fatalf("stop input %q should not leave session running, meta=%#v output=%q", chars, result.Meta, result.Output)
+		// Re-send the stop until the session reports it has exited, rather than
+		// waiting a fixed slice and asserting on whatever is true by then.
+		//
+		// A single 1s yield used to be enough because the session ran under
+		// cmd.exe. Under PowerShell the tree is slower to stand up and tear down,
+		// and the FIRST stop in a fresh process pays the interpreter's cold start,
+		// so it routinely returns with termination already under way but not
+		// finished. The old assertion read that as "the stop did not work" and
+		// failed, intermittently, on whichever of the two inputs happened to run
+		// first. CI never saw it because the runners have PowerShell 7 while a
+		// stock Windows box only has 5.1.
+		//
+		// Repeating the same input rather than polling with empty chars is
+		// deliberate: Terminate is a no-op against an already-dead tree, so it is
+		// safe to repeat, and it keeps the interrupt flag that an empty-chars poll
+		// would drop, which is what the interrupted assertion below needs.
+		var result Result
+		// Every poll's output is kept, not just the last one. Each Continue
+		// returns only what the session produced since the previous collection,
+		// so a marker printed just before the poll that observes the exit lands
+		// in the earlier result and would be invisible to a check against the
+		// final one alone.
+		var collected strings.Builder
+		deadline := time.Now().Add(30 * time.Second)
+		for {
+			result = writeTool.Run(context.Background(), map[string]any{
+				"session_id":    sessionID,
+				"chars":         chars,
+				"yield_time_ms": 500,
+			})
+			collected.WriteString(result.Output)
+			if result.Status != StatusOK {
+				t.Fatalf("stop input %q status = %s: %s", chars, result.Status, result.Output)
+			}
+			if result.Meta["session_id"] == "" {
+				break
+			}
+			if time.Now().After(deadline) {
+				t.Fatalf("stop input %q left the session running past the deadline, meta=%#v output=%q", chars, result.Meta, result.Output)
+			}
 		}
 		if result.Meta["exit_code"] == "" {
 			t.Fatalf("stop input %q should report exit_code, meta=%#v output=%q", chars, result.Meta, result.Output)
@@ -604,6 +668,17 @@ func TestWriteStdinStopIntentTerminatesNonTTYSession(t *testing.T) {
 		if result.Meta["interrupted"] != "true" {
 			t.Fatalf("stop input %q should report interrupted metadata, meta=%#v output=%q", chars, result.Meta, result.Output)
 		}
+		// The session has to have been killed rather than allowed to finish.
+		// Without this the retry loop makes the test unfalsifiable: the helper
+		// sleeps 5s, well inside the deadline, so a terminate that did nothing at
+		// all would let the sleep end on its own and every assertion above would
+		// still hold. The interrupted flag does not close the hole either, since
+		// it echoes the request rather than the outcome. The helper prints this
+		// line only on the way out of a completed sleep, so its absence is what
+		// separates the two, and it says nothing about how long the loop took.
+		if strings.Contains(collected.String(), "long sleep finished") {
+			t.Fatalf("stop input %q let the session run to completion instead of terminating it, output=%q", chars, collected.String())
+		}
 	}
 }
 
@@ -643,7 +718,7 @@ func TestExecCommandTTYSessionAcceptsInputOnLinux(t *testing.T) {
 	if runtime.GOOS != "linux" {
 		t.Skip("pty transport is currently implemented for linux")
 	}
-	root := t.TempDir()
+	root := execTestRoot(t)
 	manager := newExecSessionManager()
 	execTool := NewScopedExecCommandTool(root, nil, manager)
 	writeTool := NewWriteStdinTool(manager)
@@ -681,7 +756,7 @@ func TestExecCommandTTYSessionAcceptsInputOnLinux(t *testing.T) {
 }
 
 func TestExecSessionSnapshotsAndStopAll(t *testing.T) {
-	root := t.TempDir()
+	root := execTestRoot(t)
 	manager := newExecSessionManager()
 	execTool := NewScopedExecCommandTool(root, nil, manager).(execCommandTool)