From 933d2a6250cb6872828e99af7d8642bf4af6af4a Mon Sep 17 00:00:00 2001 From: bigboateng Date: Tue, 13 Oct 2026 15:22:15 +0100 Subject: [PATCH] Fix native Windows dev launch and bound output cleanup --- README.md | 9 ++ cmd/pitot/dev_command.go | 136 ++++++++++++++++++ cmd/pitot/dev_command_test.go | 198 +++++++++++++++++++++++++++ cmd/pitot/testdata/dev-agent/main.go | 77 +++++++++++ cmd/pitot/testdata/npm-node.cmd | 17 +++ cmd/pitot/workbench.go | 20 ++- cmd/pitot/workbench_dev_test.go | 169 ++++++++++++++++------- 7 files changed, 569 insertions(+), 57 deletions(-) create mode 100644 cmd/pitot/dev_command.go create mode 100644 cmd/pitot/dev_command_test.go create mode 100644 cmd/pitot/testdata/dev-agent/main.go create mode 100644 cmd/pitot/testdata/npm-node.cmd diff --git a/README.md b/README.md index 1248004..416858b 100644 --- a/README.md +++ b/README.md @@ -311,6 +311,15 @@ The runtime descriptor lives in a per-invocation temporary path and is removed on exit, so concurrent `pitot dev` sessions never collide. +On Windows, `pitot dev` recognizes standard npm Node launchers for Codex and +Qwen, preserving their package entrypoint and arguments without invoking a +command shell. Custom `.cmd` wrappers are rejected with guidance to use +`-- node ABSOLUTE_ENTRYPOINT ARGS`. Prefer `-- COMMAND ARGS` for paths or +arguments containing spaces; `--exec` splits its value on whitespace. +When output is captured, an agent descendant that keeps an output pipe open +can cause an incomplete-output error after a two-second drain limit. This +releases the dev runtime; it does not terminate arbitrary descendant processes. + **3. Swap the agent.** The same project — the same Controller and the same fragment — works with any other supported host whose hook is wired. Change only `--host` and the agent command: diff --git a/cmd/pitot/dev_command.go b/cmd/pitot/dev_command.go new file mode 100644 index 0000000..d05341d --- /dev/null +++ b/cmd/pitot/dev_command.go @@ -0,0 +1,136 @@ +package main + +import ( + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "time" +) + +const devWaitDelay = 2 * time.Second + +func devCommand(host, program string, args []string) (string, []string, error) { + if runtime.GOOS != "windows" || (host != "codex" && host != "qwen") { + return program, args, nil + } + resolved, err := exec.LookPath(program) + if err != nil { + return "", nil, err + } + if !strings.EqualFold(filepath.Ext(resolved), ".cmd") { + return resolved, args, nil + } + node, entry, err := npmDevCommand(host, resolved) + if err != nil { + return "", nil, fmt.Errorf("unsupported %s launcher %q: %w; use -- node ABSOLUTE_ENTRYPOINT ARGS for an explicit Node invocation", host, resolved, err) + } + return node, append([]string{entry}, args...), nil +} + +// npmDevCommand recognizes the complete plain-Node wrapper emitted by npm's +// cmd-shim. Unknown wrappers may have extra setup or flags, so never bypass them. +func npmDevCommand(host, shim string) (string, string, error) { + packages := map[string]string{"codex": "@openai/codex", "qwen": "@qwen-code/qwen-code"} + pkgName, ok := packages[host] + if !ok || !strings.EqualFold(filepath.Base(shim), host+".cmd") { + return "", "", fmt.Errorf("unrecognized command name") + } + shim, err := filepath.Abs(shim) + if err != nil { + return "", "", err + } + wrapper, err := readRegularFile(shim) + if err != nil { + return "", "", err + } + dir := filepath.Dir(shim) + packageRoot := filepath.Join(dir, "node_modules", filepath.FromSlash(pkgName)) + if strings.EqualFold(filepath.Base(dir), ".bin") && strings.EqualFold(filepath.Base(filepath.Dir(dir)), "node_modules") { + packageRoot = filepath.Join(filepath.Dir(dir), filepath.FromSlash(pkgName)) + } + data, err := readRegularFile(filepath.Join(packageRoot, "package.json")) + if err != nil { + return "", "", err + } + var manifest struct { + Name string `json:"name"` + Bin map[string]string `json:"bin"` + } + if err := json.Unmarshal(data, &manifest); err != nil { + return "", "", fmt.Errorf("invalid package manifest: %w", err) + } + target := manifest.Bin[host] + expectedTarget := map[string]string{"codex": "bin/codex.js", "qwen": "cli-entry.js"}[host] + if manifest.Name != pkgName || target != expectedTarget || !filepath.IsLocal(target) { + return "", "", fmt.Errorf("package name or bin target does not match %s", pkgName) + } + entry := filepath.Join(packageRoot, target) + switch strings.ToLower(filepath.Ext(entry)) { + case ".js", ".mjs", ".cjs": + default: + return "", "", fmt.Errorf("bin target must be a JavaScript file") + } + realRoot, err := filepath.EvalSymlinks(packageRoot) + if err != nil { + return "", "", err + } + realEntry, err := filepath.EvalSymlinks(entry) + if err != nil { + return "", "", err + } + inside, err := filepath.Rel(realRoot, realEntry) + if err != nil || !filepath.IsLocal(inside) { + return "", "", fmt.Errorf("bin target escapes package") + } + if err := requireRegularFile(entry); err != nil { + return "", "", err + } + relative, err := filepath.Rel(dir, entry) + if err != nil { + return "", "", err + } + relative = strings.ReplaceAll(relative, "/", `\`) + if strings.ReplaceAll(string(wrapper), "\r\n", "\n") != npmNodeShim(relative) { + return "", "", fmt.Errorf("wrapper is not a supported standard npm Node shim") + } + node := filepath.Join(dir, "node.exe") + if info, err := os.Stat(node); err == nil { + if !info.Mode().IsRegular() { + return "", "", fmt.Errorf("sibling node.exe is not a regular file") + } + } else if os.IsNotExist(err) { + node, err = exec.LookPath("node.exe") + if err != nil { + return "", "", err + } + } else { + return "", "", err + } + return node, entry, nil +} + +func requireRegularFile(path string) error { + info, err := os.Stat(path) + if err != nil { + return err + } + if !info.Mode().IsRegular() { + return fmt.Errorf("%s is not a regular file", path) + } + return nil +} + +func readRegularFile(path string) ([]byte, error) { + if err := requireRegularFile(path); err != nil { + return nil, err + } + return os.ReadFile(path) +} + +func npmNodeShim(target string) string { + return "@ECHO off\nGOTO start\n:find_dp0\nSET dp0=%~dp0\nEXIT /b\n:start\nSETLOCAL\nCALL :find_dp0\n\nIF EXIST \"%dp0%\\node.exe\" (\n SET \"_prog=%dp0%\\node.exe\"\n) ELSE (\n SET \"_prog=node\"\n SET PATHEXT=%PATHEXT:;.JS;=;%\n)\n\nendLocal & goto #_undefined_# 2>NUL || title %COMSPEC% & \"%_prog%\" \"%dp0%\\" + target + "\" %*\n" +} diff --git a/cmd/pitot/dev_command_test.go b/cmd/pitot/dev_command_test.go new file mode 100644 index 0000000..1882337 --- /dev/null +++ b/cmd/pitot/dev_command_test.go @@ -0,0 +1,198 @@ +package main + +import ( + "context" + "encoding/json" + "os" + "os/exec" + "path/filepath" + "reflect" + "runtime" + "strings" + "testing" +) + +// This fixture was generated by npm cmd-shim 8.0.0 from a plain Node shebang. +// Keep it independent of the production template matcher. +func writeNpmDevFixture(t *testing.T, host string, local bool) (shim, entry string) { + t.Helper() + base := filepath.Join(t.TempDir(), "tools with spaces 你好") + name, target := "@qwen-code/qwen-code", "cli-entry.js" + if host == "codex" { + name, target = "@openai/codex", "bin/codex.js" + } + dir := base + if local { + dir = filepath.Join(base, "node_modules", ".bin") + } + entry = filepath.Join(base, "node_modules", filepath.FromSlash(name), filepath.FromSlash(target)) + mustWriteDevFile(t, entry, []byte("#!/usr/bin/env node\n")) + manifest, _ := json.Marshal(map[string]any{"name": name, "bin": map[string]string{host: target}}) + mustWriteDevFile(t, filepath.Join(base, "node_modules", filepath.FromSlash(name), "package.json"), manifest) + template, err := os.ReadFile("testdata/npm-node.cmd") + if err != nil { + t.Fatal(err) + } + rel, err := filepath.Rel(dir, entry) + if err != nil { + t.Fatal(err) + } + wrapper := strings.ReplaceAll(string(template), `node_modules\@qwen-code\qwen-code\cli-entry.js`, strings.ReplaceAll(rel, "/", `\`)) + shim = filepath.Join(dir, host+".cmd") + mustWriteDevFile(t, shim, []byte(wrapper)) + mustWriteDevFile(t, filepath.Join(dir, "node.exe"), []byte("owned Node fixture")) + return shim, entry +} +func mustWriteDevFile(t *testing.T, path string, data []byte) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, data, 0700); err != nil { + t.Fatal(err) + } +} + +func TestNpmDevCommand(t *testing.T) { + for _, host := range []string{"codex", "qwen"} { + for _, local := range []bool{false, true} { + t.Run(host+map[bool]string{false: "-global", true: "-local"}[local], func(t *testing.T) { + shim, entry := writeNpmDevFixture(t, host, local) + node, got, err := npmDevCommand(host, shim) + if err != nil || got != entry || node != filepath.Join(filepath.Dir(shim), "node.exe") { + t.Fatalf("resolve: %q %q %v", node, got, err) + } + wrapper := readFileString(t, shim) + mustWriteDevFile(t, shim, []byte(strings.ReplaceAll(wrapper, "\r\n", "\n"))) + if _, _, err := npmDevCommand(host, shim); err != nil { + t.Fatalf("LF: %v", err) + } + }) + } + } +} + +func TestNpmDevRejectsUnknownLaunchers(t *testing.T) { + for _, mode := range []string{"changed-bin", "custom-prelude", "custom-flags", "wrong-target", "bad-json", "wrong-name", "missing-bin", "escape-bin", "missing-entry", "entry-directory", "sibling-directory", "missing-node", "unknown-name"} { + t.Run(mode, func(t *testing.T) { + shim, entry := writeNpmDevFixture(t, "qwen", false) + manifest := filepath.Join(filepath.Dir(entry), "package.json") + switch mode { + case "changed-bin": + mustWriteDevFile(t, manifest, []byte(`{"name":"@qwen-code/qwen-code","bin":{"qwen":"%TARGET%.js"}}`)) + mustWriteDevFile(t, filepath.Join(filepath.Dir(entry), "%TARGET%.js"), []byte("#!/usr/bin/env node\n")) + mustWriteDevFile(t, shim, []byte(strings.ReplaceAll(readFileString(t, shim), "cli-entry.js", "%TARGET%.js"))) + case "custom-prelude": + mustWriteDevFile(t, shim, []byte("SET CUSTOM=1\r\n"+readFileString(t, shim))) + case "custom-flags": + mustWriteDevFile(t, shim, []byte(strings.ReplaceAll(readFileString(t, shim), `"%_prog%" `, `"%_prog%" --trace-warnings `))) + case "wrong-target": + mustWriteDevFile(t, shim, []byte(strings.ReplaceAll(readFileString(t, shim), "cli-entry.js", "other.js"))) + case "bad-json": + mustWriteDevFile(t, manifest, []byte("{")) + case "wrong-name": + mustWriteDevFile(t, manifest, []byte(`{"name":"other","bin":{"qwen":"cli-entry.js"}}`)) + case "missing-bin": + mustWriteDevFile(t, manifest, []byte(`{"name":"@qwen-code/qwen-code"}`)) + case "escape-bin": + mustWriteDevFile(t, manifest, []byte(`{"name":"@qwen-code/qwen-code","bin":{"qwen":"../outside.js"}}`)) + case "missing-entry": + if err := os.Remove(entry); err != nil { + t.Fatal(err) + } + case "entry-directory": + if err := os.Remove(entry); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(entry, 0700); err != nil { + t.Fatal(err) + } + case "sibling-directory": + node := filepath.Join(filepath.Dir(shim), "node.exe") + if err := os.Remove(node); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(node, 0700); err != nil { + t.Fatal(err) + } + case "missing-node": + if err := os.Remove(filepath.Join(filepath.Dir(shim), "node.exe")); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", t.TempDir()) + case "unknown-name": + other := filepath.Join(filepath.Dir(shim), "custom.cmd") + mustWriteDevFile(t, other, []byte(readFileString(t, shim))) + shim = other + } + if _, _, err := npmDevCommand("qwen", shim); err == nil { + t.Fatal("accepted unsupported launcher") + } + }) + } +} + +func TestWindowsDevNpmArgv(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("native Windows executable resolution") + } + helper := buildDevAgent(t) + binary, err := os.ReadFile(helper) + if err != nil { + t.Fatal(err) + } + for _, host := range []string{"codex", "qwen"} { + for _, local := range []bool{false, true} { + for _, sibling := range []bool{false, true} { + name := host + map[bool]string{true: "-local", false: "-global"}[local] + map[bool]string{true: "-sibling", false: "-path"}[sibling] + t.Run(name, func(t *testing.T) { + shim, entry := writeNpmDevFixture(t, host, local) + nodePath := filepath.Join(filepath.Dir(shim), "node.exe") + pathDir := t.TempDir() + pathNode := filepath.Join(pathDir, "node.exe") + if sibling { + mustWriteDevFile(t, nodePath, binary) + // A broken PATH node must not replace the shim's sibling interpreter. + mustWriteDevFile(t, pathNode, []byte("not an executable")) + } else { + if err := os.Remove(nodePath); err != nil { + t.Fatal(err) + } + mustWriteDevFile(t, pathNode, binary) + } + t.Setenv("PATH", pathDir) + t.Setenv("PITOT_TEST_NODE", "1") + t.Setenv("PITOT_TEST_MODE", "record") + seen := t.TempDir() + t.Setenv("PITOT_TEST_SEEN", seen) + args := []string{"", "a b", `say "hi"`, `C:\path with space\`, "你好", "&|<>^%!()"} + executable, argv, err := devCommand(host, shim, args) + if err != nil { + t.Fatal(err) + } + expectedNode := pathNode + if sibling { + expectedNode = nodePath + } + if executable != expectedNode || argv[0] != entry { + t.Fatalf("wrong invocation: %q %#v", executable, argv) + } + if out, err := exec.CommandContext(context.Background(), executable, argv...).CombinedOutput(); err != nil { + t.Fatalf("launch: %v %s", err, out) + } + var got []string + if err := json.Unmarshal([]byte(readFileString(t, filepath.Join(seen, "args.json"))), &got); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(got, args) { + t.Fatalf("argv: %#v, want %#v", got, args) + } + direct, directArgs, err := devCommand(host, expectedNode, args) + if err != nil || direct != expectedNode || !reflect.DeepEqual(directArgs, args) { + t.Fatalf("direct executable changed: %q %#v %v", direct, directArgs, err) + } + }) + } + } + } +} diff --git a/cmd/pitot/testdata/dev-agent/main.go b/cmd/pitot/testdata/dev-agent/main.go new file mode 100644 index 0000000..9b0bad4 --- /dev/null +++ b/cmd/pitot/testdata/dev-agent/main.go @@ -0,0 +1,77 @@ +// A native, deterministic stand-in used by the dev lifecycle tests. +package main + +import ( + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "time" +) + +func main() { + if err := run(); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } +} +func run() error { + mode := os.Getenv("PITOT_TEST_MODE") + if len(os.Args) > 1 && os.Args[1] == "--hold-output" { + time.Sleep(4 * time.Second) + return nil + } + seen := os.Getenv("PITOT_TEST_SEEN") + args := os.Args[1:] + // When impersonating node.exe, the first argument is its JS entrypoint. + if os.Getenv("PITOT_TEST_NODE") == "1" { + args = args[1:] + } + data, err := json.Marshal(args) + if err != nil { + return err + } + for name, value := range map[string]string{"runtime.txt": os.Getenv("PITOT_RUNTIME"), "argc.txt": fmt.Sprint(len(args)), "args.json": string(data), "ready": "yes"} { + if err := os.WriteFile(filepath.Join(seen, name), []byte(value), 0600); err != nil { + return err + } + } + switch mode { + case "fail": + return fmt.Errorf("agent rejected request") + case "wait": + time.Sleep(15 * time.Second) + return nil + case "pipe": + self, err := os.Executable() + if err != nil { + return err + } + child := exec.Command(self, "--hold-output") + child.Stdout = os.Stdout + child.Stderr = os.Stderr + if err := child.Start(); err != nil { + return err + } + return child.Process.Release() + case "record": + return nil + } + for _, command := range []string{"echo ok", "PITOT_DENY_ME=1 echo no"} { + payload := fmt.Sprintf(`{"hook_event_name":"PreToolUse","session_id":"dev","tool_name":"Bash","tool_input":{"command":%q}}`, command) + cmd := exec.Command(os.Getenv("PITOT_BIN"), "hook", "qwen", "--runtime", os.Getenv("PITOT_RUNTIME")) + cmd.Stdin = strings.NewReader(payload) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + err := cmd.Run() + if command == "echo ok" && err != nil { + return err + } + if command != "echo ok" && err == nil { + return fmt.Errorf("denied action unexpectedly succeeded") + } + } + return nil +} diff --git a/cmd/pitot/testdata/npm-node.cmd b/cmd/pitot/testdata/npm-node.cmd new file mode 100644 index 0000000..d560f1a --- /dev/null +++ b/cmd/pitot/testdata/npm-node.cmd @@ -0,0 +1,17 @@ +@ECHO off +GOTO start +:find_dp0 +SET dp0=%~dp0 +EXIT /b +:start +SETLOCAL +CALL :find_dp0 + +IF EXIST "%dp0%\node.exe" ( + SET "_prog=%dp0%\node.exe" +) ELSE ( + SET "_prog=node" + SET PATHEXT=%PATHEXT:;.JS;=;% +) + +endLocal & goto #_undefined_# 2>NUL || title %COMSPEC% & "%_prog%" "%dp0%\node_modules\@qwen-code\qwen-code\cli-entry.js" %* diff --git a/cmd/pitot/workbench.go b/cmd/pitot/workbench.go index 9ee3854..f31c325 100644 --- a/cmd/pitot/workbench.go +++ b/cmd/pitot/workbench.go @@ -938,12 +938,20 @@ func runDev(ctx context.Context, args []string, stdout, stderr io.Writer) error devinOptions.Runtime = runtimePath runErr = runDevinACP(devCtx, *devinOptions) } else { - cmd := exec.CommandContext(devCtx, program, programArgs...) - cmd.Env = append(os.Environ(), "PITOT_RUNTIME="+runtimePath) - cmd.Stdout = stdout - cmd.Stderr = stderr - cmd.Stdin = os.Stdin - runErr = cmd.Run() + var executable string + var argv []string + executable, argv, runErr = devCommand(host, program, programArgs) + if runErr == nil { + cmd := exec.CommandContext(devCtx, executable, argv...) + cmd.Env = append(os.Environ(), "PITOT_RUNTIME="+runtimePath) + cmd.Stdout = stdout + cmd.Stderr = stderr + cmd.Stdin = os.Stdin + // A descendant can retain captured output after the agent exits. + // Bound that wait and report incomplete output as an execution error. + cmd.WaitDelay = devWaitDelay + runErr = cmd.Run() + } } // Stop the runtime and reap its goroutine before returning. diff --git a/cmd/pitot/workbench_dev_test.go b/cmd/pitot/workbench_dev_test.go index ad5ca2f..fd6dc1b 100644 --- a/cmd/pitot/workbench_dev_test.go +++ b/cmd/pitot/workbench_dev_test.go @@ -2,14 +2,17 @@ package main import ( "context" - "fmt" + "encoding/json" + "errors" "os" "os/exec" "path/filepath" + "reflect" "regexp" goruntime "runtime" "strings" "testing" + "time" ) // decisionLine models one entry from `pitot dev`'s decision timeline. @@ -62,57 +65,45 @@ func writeFragment(t *testing.T, root, name, body string) { } } -// devAgentScript writes a POSIX agent stand-in that records the runtime it was -// handed and its own argv, then drives one allow and one deny decision through -// the running runtime via `pitot hook kimi`. It stands in for a real coding -// agent so the dev harness can be exercised deterministically. -func devAgentScript(t *testing.T, seenDir string) string { +// buildDevAgent compiles a shell-free helper before the test changes directory. +func buildDevAgent(t *testing.T) string { t.Helper() - allowPayload := `{"hook_event_name":"PreToolUse","session_id":"dev","tool_name":"Bash","tool_input":{"command":"echo ok"}}` - denyPayload := `{"hook_event_name":"PreToolUse","session_id":"dev","tool_name":"Bash","tool_input":{"command":"PITOT_DENY_ME=1 echo no"}}` - script := fmt.Sprintf(`#!/bin/sh -printf '%%s' "$PITOT_RUNTIME" > %q -printf '%%s' "$#" > %q -printf '%%s' "$*" > %q -printf '%s' | "$PITOT_BIN" hook kimi --runtime "$PITOT_RUNTIME" -printf '%s' | "$PITOT_BIN" hook kimi --runtime "$PITOT_RUNTIME" || true -`, - filepath.Join(seenDir, "runtime.txt"), - filepath.Join(seenDir, "argc.txt"), - filepath.Join(seenDir, "args.txt"), - allowPayload, denyPayload) - path := filepath.Join(seenDir, "agent.sh") - if err := os.WriteFile(path, []byte(script), 0o755); err != nil { - t.Fatal(err) + name := "dev-agent" + if goruntime.GOOS == "windows" { + name += ".exe" + } + path := filepath.Join(t.TempDir(), name) + if out, err := exec.Command("go", "build", "-o", path, "./testdata/dev-agent").CombinedOutput(); err != nil { + t.Fatalf("build dev agent: %v\n%s", err, out) } return path } // TestDevRunsAgentBehindShellController is the end-to-end proof of the dev // harness: it starts the runtime + the compiled shell-policy controller, launches -// a scripted agent, and verifies the decision timeline, PITOT_RUNTIME +// a native agent stand-in, and verifies the decision timeline, PITOT_RUNTIME // propagation, argv preservation, and runtime-dir teardown. func TestDevRunsAgentBehindShellController(t *testing.T) { - if goruntime.GOOS == "windows" { - t.Skip("dev agent stand-in is a POSIX shell script") - } // Build inputs before we chdir into the project directory. pitotBin := buildPitotBinary(t) + agent := buildDevAgent(t) _, configBody := buildGeneratedShellPolicy(t) proj := t.TempDir() writeFragment(t, proj, "shell-policy", configBody) seenDir := t.TempDir() - agent := devAgentScript(t, seenDir) + t.Setenv("PITOT_TEST_SEEN", seenDir) + t.Setenv("PATH", filepath.Dir(agent)+string(os.PathListSeparator)+os.Getenv("PATH")) t.Setenv("PITOT_BIN", pitotBin) - t.Setenv("PITOT_RUNTIME", "") + t.Setenv("PITOT_RUNTIME", "stale-descriptor") t.Chdir(proj) + wantedArgs := []string{"", "alpha beta", `say "hi"`, `C:\a\`, "你好", "&|<>^%!()"} var stdout lockedBuffer var stderr lockedBuffer - // `-- sh AGENT a b` exercises the explicit argv form (args after --). - if err := runDev(context.Background(), []string{"--host", "kimi", "--", "sh", agent, "alpha", "beta"}, &stdout, &stderr); err != nil { + // The explicit argv form must preserve arguments without shell interpolation. + if err := runDev(context.Background(), append([]string{"--host", "qwen", "--", agent}, wantedArgs...), &stdout, &stderr); err != nil { t.Fatalf("pitot dev: %v\nstdout:\n%s\nstderr:\n%s", err, stdout.String(), stderr.String()) } @@ -143,12 +134,12 @@ func TestDevRunsAgentBehindShellController(t *testing.T) { if !strings.Contains(runtimeSeen, "pitot-dev-") { t.Errorf("agent PITOT_RUNTIME = %q, want a per-invocation dev runtime path", runtimeSeen) } - // argv after `--` is preserved verbatim: two positional args, joined "alpha beta". - if argc := readFileString(t, filepath.Join(seenDir, "argc.txt")); argc != "2" { - t.Errorf("agent argc = %q, want 2", argc) + var gotArgs []string + if err := json.Unmarshal([]byte(readFileString(t, filepath.Join(seenDir, "args.json"))), &gotArgs); err != nil { + t.Fatal(err) } - if args := readFileString(t, filepath.Join(seenDir, "args.txt")); args != "alpha beta" { - t.Errorf("agent args = %q, want \"alpha beta\"", args) + if !reflect.DeepEqual(gotArgs, wantedArgs) { + t.Errorf("argv = %#v, want %#v", gotArgs, wantedArgs) } // The per-invocation runtime directory must be removed once dev exits. if _, err := os.Stat(filepath.Dir(runtimeSeen)); !os.IsNotExist(err) { @@ -159,24 +150,23 @@ func TestDevRunsAgentBehindShellController(t *testing.T) { // TestDevRuntimePathsAreUniquePerRun confirms concurrent-safe isolation: two dev // runs of the same project hand the agent two different runtime descriptors. func TestDevRuntimePathsAreUniquePerRun(t *testing.T) { - if goruntime.GOOS == "windows" { - t.Skip("dev agent stand-in is a POSIX shell script") - } pitotBin := buildPitotBinary(t) + agent := buildDevAgent(t) _, configBody := buildGeneratedShellPolicy(t) proj := t.TempDir() writeFragment(t, proj, "shell-policy", configBody) + t.Setenv("PATH", filepath.Dir(agent)+string(os.PathListSeparator)+os.Getenv("PATH")) t.Setenv("PITOT_BIN", pitotBin) - t.Setenv("PITOT_RUNTIME", "") + t.Setenv("PITOT_RUNTIME", "stale-descriptor") t.Chdir(proj) run := func() string { seenDir := t.TempDir() - agent := devAgentScript(t, seenDir) + t.Setenv("PITOT_TEST_SEEN", seenDir) var stdout lockedBuffer var stderr lockedBuffer - if err := runDev(context.Background(), []string{"--host", "kimi", "--", "sh", agent}, &stdout, &stderr); err != nil { + if err := runDev(context.Background(), []string{"--host", "qwen", "--", agent}, &stdout, &stderr); err != nil { t.Fatalf("pitot dev: %v\n%s", err, stderr.String()) } return readFileString(t, filepath.Join(seenDir, "runtime.txt")) @@ -190,25 +180,24 @@ func TestDevRuntimePathsAreUniquePerRun(t *testing.T) { // TestDevExecSplitsWhereArgvDoesNot pins the distinct semantics of the two agent // forms: --exec field-splits its string, while `-- CMD ARGS` preserves argv. func TestDevExecSplitsWhereArgvDoesNot(t *testing.T) { - if goruntime.GOOS == "windows" { - t.Skip("dev agent stand-in is a POSIX shell script") - } pitotBin := buildPitotBinary(t) + agent := buildDevAgent(t) _, configBody := buildGeneratedShellPolicy(t) proj := t.TempDir() writeFragment(t, proj, "shell-policy", configBody) + t.Setenv("PATH", filepath.Dir(agent)+string(os.PathListSeparator)+os.Getenv("PATH")) t.Setenv("PITOT_BIN", pitotBin) - t.Setenv("PITOT_RUNTIME", "") + t.Setenv("PITOT_RUNTIME", "stale-descriptor") t.Chdir(proj) argc := func(devArgs []string) string { seenDir := t.TempDir() - agent := devAgentScript(t, seenDir) + t.Setenv("PITOT_TEST_SEEN", seenDir) // Substitute the agent path into the caller's arg template. filled := make([]string, len(devArgs)) for i, a := range devArgs { - filled[i] = strings.ReplaceAll(a, "{AGENT}", agent) + filled[i] = strings.ReplaceAll(a, "{AGENT}", filepath.Base(agent)) } var stdout lockedBuffer var stderr lockedBuffer @@ -218,12 +207,12 @@ func TestDevExecSplitsWhereArgvDoesNot(t *testing.T) { return readFileString(t, filepath.Join(seenDir, "argc.txt")) } - // --exec "sh AGENT a b" splits on whitespace -> sh sees 2 positional args. - if got := argc([]string{"--host", "kimi", "--exec", "sh {AGENT} a b"}); got != "2" { + // --exec splits on whitespace. + if got := argc([]string{"--host", "qwen", "--exec", "{AGENT} a b"}); got != "2" { t.Errorf("--exec field-split argc = %q, want 2", got) } - // -- sh AGENT "a b" preserves argv -> sh sees 1 positional arg. - if got := argc([]string{"--host", "kimi", "--", "sh", "{AGENT}", "a b"}); got != "1" { + // The explicit form preserves one argument containing a space. + if got := argc([]string{"--host", "qwen", "--", "{AGENT}", "a b"}); got != "1" { t.Errorf("-- argv-preserving argc = %q, want 1", got) } } @@ -236,3 +225,81 @@ func readFileString(t *testing.T, path string) string { } return string(data) } + +func TestDevFailureCleanup(t *testing.T) { + agent := buildDevAgent(t) + _, configBody := buildGeneratedShellPolicy(t) + proj := t.TempDir() + writeFragment(t, proj, "shell-policy", configBody) + t.Chdir(proj) + for _, mode := range []string{"missing", "fail", "cancel", "pipe", "record"} { + t.Run(mode, func(t *testing.T) { + seen := t.TempDir() + tempRoot := t.TempDir() + for _, name := range []string{"TMPDIR", "TMP", "TEMP"} { + t.Setenv(name, tempRoot) + } + t.Setenv("PITOT_TEST_SEEN", seen) + helperMode := mode + if mode == "cancel" { + helperMode = "wait" + } + t.Setenv("PITOT_TEST_MODE", helperMode) + program := agent + if mode == "missing" { + program = filepath.Join(seen, "missing-agent") + } + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + canceled := make(chan struct{}) + if mode == "cancel" { + go func() { + defer close(canceled) + ticker := time.NewTicker(10 * time.Millisecond) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + if _, err := os.Stat(filepath.Join(seen, "ready")); err == nil { + cancel() + return + } + } + } + }() + } + var stdout, stderr lockedBuffer + started := time.Now() + err := runDev(ctx, []string{"--host", "qwen", "--", program}, &stdout, &stderr) + elapsed := time.Since(started) + if mode == "cancel" { + <-canceled + } + if mode == "record" { + if err != nil { + t.Fatalf("normal exit: %v", err) + } + } else { + if err == nil || strings.Contains(stdout.String(), "Agent finished.") { + t.Fatalf("false success: %v\n%s", err, stdout.String()) + } + } + if mode == "pipe" { + if !errors.Is(err, exec.ErrWaitDelay) { + t.Fatalf("expected incomplete output: %v", err) + } + if elapsed >= 4*time.Second { + t.Errorf("output drain was not bounded: %s", elapsed) + } + // The helper descendant self-exits; WaitDelay does not claim to kill it. + time.Sleep(4*time.Second - elapsed + 100*time.Millisecond) + } + entries, readErr := os.ReadDir(tempRoot) + if readErr != nil || len(entries) != 0 { + t.Errorf("runtime cleanup: entries=%v err=%v", entries, readErr) + } + }) + } +}