From d00c83ee97cc1e46100a37c88d6ee77c6d81227a Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 23 Oct 2026 00:15:44 -0400 Subject: [PATCH 1/3] fix: run native Windows commands in Git Bash On native Windows codeaf's bash resolved to System32\bash.exe, the WSL launcher, which parsed every line a second time inside Linux: a PATH holding Program Files (x86) broke every command and $vars came back empty. The shell now resolves CODEAF_BASH, then Git for Windows, then other standard roots, then a non-WSL bash on PATH, and every door that starts a shell uses it. Background job and service logs open writable at their end, because Git Bash exits silently on an append-only handle. With no usable bash the shell tool is absent and the manual says to install Git for Windows. This ports the Windows fixes of #1882, which landed on app/v3, to dev. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/changes/unreleased/0-windows-git-bash.md | 12 + internal/automation/store_windows_test.go | 38 ++ internal/exec/bare/shell_windows_test.go | 45 +++ internal/exec/bare/streaming.go | 16 +- internal/exec/bare/tools.go | 43 ++- internal/exec/jobs.go | 26 +- internal/exec/jobs_test.go | 14 +- internal/exec/process_signal_unix_test.go | 9 + internal/exec/process_signal_windows_test.go | 31 ++ internal/exec/services.go | 9 +- internal/exec/services_test.go | 2 + internal/exec/shell_windows_test.go | 175 +++++++++ internal/exec/tools.go | 27 +- .../chat/running-commands-on-windows.md | 52 +++ internal/manual/chat_test.go | 23 ++ internal/plandb/persist.go | 4 +- internal/plandb/preview.go | 6 +- internal/plandb/uri.go | 35 ++ internal/plandb/uri_test.go | 94 +++++ internal/rtk/rtk.go | 4 +- internal/session/automation_run.go | 5 +- internal/session/bashbelt.go | 8 +- internal/session/bashbelt_jobs_test.go | 2 +- internal/session/beltfacts.go | 6 +- internal/session/chatpage.go | 3 + internal/session/jobs.go | 24 +- internal/session/plan_uri_test.go | 60 ++++ internal/session/plandb_plan.go | 16 +- internal/session/plandb_tasks.go | 6 +- internal/session/principal_audit.go | 7 +- internal/session/process_signal_unix_test.go | 9 + .../session/process_signal_windows_test.go | 31 ++ internal/session/program_model_commit_test.go | 2 + internal/session/prompt.go | 15 +- internal/session/shell_unix_test.go | 20 ++ internal/session/shell_windows_test.go | 213 +++++++++++ internal/session/sqlite_windows_test.go | 11 + internal/session/task_checks.go | 18 +- internal/session/task_job_park_test.go | 3 +- internal/session/tools_watch.go | 5 +- internal/session/userbash.go | 4 + internal/shell/cache_windows_test.go | 60 ++++ internal/shell/doors_test.go | 62 ++++ internal/shell/git_shim_test.go | 75 ++++ internal/shell/listpath_test.go | 51 +++ internal/shell/listpath_windows_test.go | 42 +++ internal/shell/log.go | 36 ++ internal/shell/log_test.go | 41 +++ internal/shell/native_windows_test.go | 90 +++++ internal/shell/shell.go | 336 ++++++++++++++++++ internal/shell/shell_test.go | 226 ++++++++++++ internal/store/store_windows_test.go | 36 ++ internal/verify/run.go | 5 +- internal/verify/run_windows_test.go | 34 ++ internal/workspace/store_windows_test.go | 38 ++ 55 files changed, 2188 insertions(+), 77 deletions(-) create mode 100644 docs/changes/unreleased/0-windows-git-bash.md create mode 100644 internal/automation/store_windows_test.go create mode 100644 internal/exec/bare/shell_windows_test.go create mode 100644 internal/exec/process_signal_unix_test.go create mode 100644 internal/exec/process_signal_windows_test.go create mode 100644 internal/exec/shell_windows_test.go create mode 100644 internal/manual/chat/running-commands-on-windows.md create mode 100644 internal/plandb/uri.go create mode 100644 internal/plandb/uri_test.go create mode 100644 internal/session/plan_uri_test.go create mode 100644 internal/session/process_signal_unix_test.go create mode 100644 internal/session/process_signal_windows_test.go create mode 100644 internal/session/shell_unix_test.go create mode 100644 internal/session/shell_windows_test.go create mode 100644 internal/session/sqlite_windows_test.go create mode 100644 internal/shell/cache_windows_test.go create mode 100644 internal/shell/doors_test.go create mode 100644 internal/shell/git_shim_test.go create mode 100644 internal/shell/listpath_test.go create mode 100644 internal/shell/listpath_windows_test.go create mode 100644 internal/shell/log.go create mode 100644 internal/shell/log_test.go create mode 100644 internal/shell/native_windows_test.go create mode 100644 internal/shell/shell.go create mode 100644 internal/shell/shell_test.go create mode 100644 internal/store/store_windows_test.go create mode 100644 internal/verify/run_windows_test.go create mode 100644 internal/workspace/store_windows_test.go diff --git a/docs/changes/unreleased/0-windows-git-bash.md b/docs/changes/unreleased/0-windows-git-bash.md new file mode 100644 index 0000000000..49859959b1 --- /dev/null +++ b/docs/changes/unreleased/0-windows-git-bash.md @@ -0,0 +1,12 @@ +--- +kind: fixed +title: Run native Windows commands in Git Bash +pr: 0 +surface: [chat, engine] +invalidates: + - "Native Windows commands previously selected the WSL launcher and parsed variables a second time in Linux. Native commands now select Git Bash, preserve shell variables and Windows file access, and omit the shell tool when no usable bash is installed." + - "Git Bash background jobs and detached services inherited append-only stdout handles and could exit silently. Windows logs now open writable at their end; Unix append flags and command arguments stay unchanged." + - "The Windows SQLite URI fix already covered graph, workspace and automation stores, but plan writers and spending readers still built raw file URIs. They now share the existing formatter on Windows while Unix plan readers retain the writer's raw filename bytes." +--- + +Install Git for Windows to run commands with the native Windows build, or choose an MSYS2 bash with CODEAF_BASH. Running the Linux build inside WSL continues to use Linux bash. The manual and project facts explain the shell requirement and Windows drive paths. diff --git a/internal/automation/store_windows_test.go b/internal/automation/store_windows_test.go new file mode 100644 index 0000000000..f0f7efefd5 --- /dev/null +++ b/internal/automation/store_windows_test.go @@ -0,0 +1,38 @@ +//go:build windows + +package automation + +import ( + "path/filepath" + "testing" + "time" +) + +// N2 writes and reopens an automation in a native Windows folder whose spaces, +// Unicode, fragment and percent characters must remain part of the filename. +func TestWindowsAutomationStoreNativePath(t *testing.T) { + root := filepath.Join(t.TempDir(), "dir with space é", "automations #%25") + s, err := Open(root) + if err != nil { + t.Fatal(err) + } + defer func() { + if s != nil { + _ = s.Close() + } + }() + item, err := s.Create(Automation{Title: "héllo", Workspace: t.TempDir(), Schedule: Schedule{At: time.Now().Add(time.Hour)}, Action: Action{Say: "hello"}}) + if err != nil { + t.Fatal(err) + } + if err := s.Close(); err != nil { + t.Fatal(err) + } + s, err = Open(root) + if err != nil { + t.Fatal(err) + } + if got, err := s.Get(item.ID); err != nil || got.Title != "héllo" { + t.Fatalf("automation = %+v, %v", got, err) + } +} diff --git a/internal/exec/bare/shell_windows_test.go b/internal/exec/bare/shell_windows_test.go new file mode 100644 index 0000000000..0a9673b2b8 --- /dev/null +++ b/internal/exec/bare/shell_windows_test.go @@ -0,0 +1,45 @@ +//go:build windows + +package bare + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Agent-Field/codeaf/internal/env" + "github.com/Agent-Field/codeaf/internal/shell" +) + +// N1 makes a shell with no implementation absent from both public inventories, +// and direct calls explain the setup instead of creating a process. +func TestWindowsMissingBashIsAbsent(t *testing.T) { + dir := t.TempDir() + t.Setenv("CODEAF_HOME", dir) + for _, name := range []string{"ProgramW6432", "ProgramFiles", "ProgramFiles(x86)", "LOCALAPPDATA", "USERPROFILE", env.Legacy("CODEAF_BASH")} { + t.Setenv(name, "") + } + t.Setenv("PATH", filepath.Join(os.Getenv("SystemRoot"), "System32")) + for _, explicit := range []string{filepath.Join(dir, "missing.exe"), ""} { + t.Setenv("CODEAF_BASH", explicit) + for _, tools := range [][]Tool{AllTools(dir), Tools(dir)} { + for _, tool := range tools { + if tool.Name == "bash" { + t.Fatal("unusable bash is advertised") + } + } + } + output, failed, err := newBashTool(dir, DefaultCaps()).Execute(context.Background(), []byte(`{"command":"echo WRONG > marker"}`)) + if err != nil || !failed || !strings.Contains(output, "CODEAF_BASH") { + t.Fatalf("direct call: %q %v %v", output, failed, err) + } + if explicit == "" && output != shell.NoBash { + t.Fatalf("missing discovery remedy: %q, want %q", output, shell.NoBash) + } + if _, err := os.Stat(filepath.Join(dir, "marker")); !os.IsNotExist(err) { + t.Fatalf("command ran: %v", err) + } + } +} diff --git a/internal/exec/bare/streaming.go b/internal/exec/bare/streaming.go index ce3c7e0952..b6b8833dcc 100644 --- a/internal/exec/bare/streaming.go +++ b/internal/exec/bare/streaming.go @@ -55,6 +55,7 @@ package bare import ( "os" "os/exec" + "runtime" "strings" "sync" @@ -92,13 +93,16 @@ func TailForResultAt(text string, caps Caps) string { return truncateTail(text, // reads while it runs — the foreground bash tool here, and the background job // registry in internal/session, which used to keep a hand-copied three-line // version of the choice and now asks this instead. -func StreamingShell(command string) (string, []string) { - shell, shellArgs := getShellConfig() +func StreamingShell(command string) (string, []string, error) { + shell, shellArgs, err := getShellConfig() + if err != nil { + return "", nil, err + } full := append(append([]string{}, shellArgs...), command) if prefix := stdbufPrefix(); len(prefix) > 0 { - return prefix[0], append(append(append([]string{}, prefix[1:]...), shell), full...) + return prefix[0], append(append(append([]string{}, prefix[1:]...), shell), full...), nil } - return shell, full + return shell, full, nil } // StreamingEnv is the environment such a command runs in: the person's own, @@ -139,6 +143,10 @@ func StreamingEnv() []string { // run once against `true`, and it is taken only if it exits clean and says // nothing at all. var stdbufPrefix = sync.OnceValue(func() []string { + // A native Windows child must be launched directly, without an MSYS wrapper. + if runtime.GOOS == "windows" { + return nil + } path, err := exec.LookPath("stdbuf") if err != nil { return nil diff --git a/internal/exec/bare/tools.go b/internal/exec/bare/tools.go index 30ff7841d0..0a7f5782ab 100644 --- a/internal/exec/bare/tools.go +++ b/internal/exec/bare/tools.go @@ -20,6 +20,7 @@ import ( "github.com/Agent-Field/codeaf/internal/guard" "github.com/Agent-Field/codeaf/internal/processgroup" + "github.com/Agent-Field/codeaf/internal/shell" ) // Tool is the shape every tool in this package wears, and the shape a @@ -53,7 +54,7 @@ func AllTools(cwd string) []Tool { return AllToolsCapped(cwd, DefaultCaps()) } // from the pair they are actually applying. func AllToolsCapped(cwd string, caps Caps) []Tool { caps = caps.resolve() - return []Tool{ + return availableTools([]Tool{ newReadTool(cwd, caps), newBashTool(cwd, caps), newEditTool(cwd), @@ -61,7 +62,7 @@ func AllToolsCapped(cwd string, caps Caps) []Tool { newGrepTool(cwd, caps), newFindTool(cwd, caps), newLsTool(cwd, caps), - } + }) } // Tools returns the four default active pi tools in registry order: read, @@ -74,12 +75,12 @@ func Tools(cwd string) []Tool { return ToolsCapped(cwd, DefaultCaps()) } // ToolsCapped is [Tools] with the belt's own result caps. See [AllToolsCapped]. func ToolsCapped(cwd string, caps Caps) []Tool { caps = caps.resolve() - return []Tool{ + return availableTools([]Tool{ newReadTool(cwd, caps), newBashTool(cwd, caps), newEditTool(cwd), newWriteTool(cwd), - } + }) } // ── wire schemas (verbatim from pi) ──────────────────────────────────────── @@ -461,7 +462,10 @@ func newBashTool(cwd string, caps Caps) Tool { // held in a 4KB buffer until it exits is a command that looks dead to // anybody reading its log, and a long one gets killed for it // (streaming.go states the whole case). - shell, shellArgs := StreamingShell(p.Command) + shell, shellArgs, shellErr := StreamingShell(p.Command) + if shellErr != nil { + return shellErr.Error(), true, nil + } // THE COMMAND IS NOT BOUND TO THE CONTEXT, and the watcher below // does the binding by hand. exec.CommandContext's own watcher kills @@ -707,16 +711,10 @@ func exitCodeFromWait(err error) int { } // getShellConfig mirrors pi's shell.js:getShellConfig on Unix: /bin/bash if -// it exists, then bash on PATH, then fallback to sh. Returns the shell path -// and args (["-c"]). -func getShellConfig() (string, []string) { - if _, err := os.Stat("/bin/bash"); err == nil { - return "/bin/bash", []string{"-c"} - } - if bash, err := exec.LookPath("bash"); err == nil { - return bash, []string{"-c"} - } - return "sh", []string{"-c"} +// it exists, then bash on PATH, then fallback to sh. Windows instead uses the +// engine's native bash, and refuses before a command can reach a WSL launcher. +func getShellConfig() (string, []string, error) { + return shell.Preferred() } // killProcessGroup sends SIGKILL to the process group, mirroring pi's @@ -1273,3 +1271,18 @@ func commandCutWords(began time.Time, text string) string { } return head + "\n" + text } + +// availableTools omits a shell with no native implementation, so the model +// cannot select a command that will fail on every call. Unix is unchanged. +func availableTools(tools []Tool) []Tool { + if _, err := shell.Bash(); err == nil { + return tools + } + out := tools[:0] + for _, tool := range tools { + if tool.Name != "bash" { + out = append(out, tool) + } + } + return out +} diff --git a/internal/exec/jobs.go b/internal/exec/jobs.go index eec6063ea0..2a621bb122 100644 --- a/internal/exec/jobs.go +++ b/internal/exec/jobs.go @@ -9,6 +9,7 @@ import ( "os/exec" "path/filepath" "regexp" + "runtime" "sort" "strings" "sync" @@ -17,6 +18,8 @@ import ( "github.com/Agent-Field/codeaf/internal/env" "github.com/Agent-Field/codeaf/internal/guard" "github.com/Agent-Field/codeaf/internal/processgroup" + "github.com/Agent-Field/codeaf/internal/rtk" + commandshell "github.com/Agent-Field/codeaf/internal/shell" "github.com/Agent-Field/codeaf/internal/store" ) @@ -132,6 +135,10 @@ func newJobRegistry(workspace *Workspace, leaf string, results int) *jobRegistry // startBackground starts a shell with file-backed output. Because no parent // pipe is involved, a verbose build cannot block on a reader in the executor. func (t *Toolbox) startBackground(ctx context.Context, command string, args map[string]any) Result { + bash, shellErr := commandshell.Bash() + if shellErr != nil { + return errorf("%s", shellErr) + } duration, err := backgroundDuration(ctx, args) if err != nil { return errorf("could not start background job: %v", err) @@ -143,7 +150,20 @@ func (t *Toolbox) startBackground(ctx context.Context, command string, args map[ environment = os.Environ() environment = replaceEnv(environment, "CODEAF_SKILLS_BIN", bin) environment = replaceEnv(environment, env.Legacy("CODEAF_SKILLS_BIN"), bin) - command = "export PATH=\"${CODEAF_SKILLS_BIN:?}:$PATH\"\n" + command + command = commandshell.PathExport("CODEAF_SKILLS_BIN", bin) + command + } + } + // Windows jobs need the native rtk shelf on their own shell PATH too. + // No rewriting is applied: a background job keeps its ordinary output. + if runtime.GOOS == "windows" { + if tool, ok := rtk.Available(); ok { + if environment == nil { + environment = os.Environ() + } + bin := filepath.Dir(tool.Path) + environment = replaceEnv(environment, "CODEAF_RTK_BIN", bin) + environment = replaceEnv(environment, env.Legacy("CODEAF_RTK_BIN"), bin) + command = commandshell.PathExport("CODEAF_RTK_BIN", bin) + command } } // A BACKGROUND JOB IS STILL A MODEL'S COMMAND, and its shell must not reach @@ -167,13 +187,13 @@ func (t *Toolbox) startBackground(ctx context.Context, command string, args map[ if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { return errorf("could not create background log directory: %v", err) } - logFile, err := os.OpenFile(full, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o644) + logFile, err := commandshell.OpenLog(full, 0o644) if err != nil { return errorf("could not create background log %s: %v", relative, err) } jobCtx, cancel := context.WithCancel(context.Background()) - cmd := exec.CommandContext(jobCtx, "bash", "-lc", command) + cmd := exec.CommandContext(jobCtx, bash, "-lc", command) configureDetachedCommand(cmd, r.workspace.Root(), logFile) cmd.Env = environment cmd.Cancel = func() error { diff --git a/internal/exec/jobs_test.go b/internal/exec/jobs_test.go index 4d169e8788..12ddea0421 100644 --- a/internal/exec/jobs_test.go +++ b/internal/exec/jobs_test.go @@ -8,7 +8,6 @@ import ( "strconv" "strings" "sync/atomic" - "syscall" "testing" "time" @@ -99,6 +98,7 @@ func TestBackgroundStartReturnsImmediatelyAndCreatesDurableLog(t *testing.T) { } } +// N2 and N3 keep service ownership and cleanup through native process probes. func TestKeepTransfersOwnershipAndLeafTeardownCountsOnlyKilledJobs(t *testing.T) { tools, _ := backgroundToolbox(t) for index := 0; index < 2; index++ { @@ -118,11 +118,11 @@ func TestKeepTransfersOwnershipAndLeafTeardownCountsOnlyKilledJobs(t *testing.T) if killed := tools.Close(); killed != 1 { t.Fatalf("leaf teardown killed %d jobs, want only the unpromoted job", killed) } - if err := syscall.Kill(requests[0].PID, 0); err != nil { + if err := testProcessSignal(requests[0].PID, 0); err != nil { t.Fatalf("requested service did not survive leaf teardown: %v", err) } requests[0].Stop() - if err := syscall.Kill(requests[0].PID, 0); err == nil { + if err := testProcessSignal(requests[0].PID, 0); err == nil { t.Fatal("declined service process still alive") } } @@ -253,10 +253,10 @@ func TestJobKillTerminatesTheWholeProcessGroup(t *testing.T) { t.Fatalf("kill result = %+v", result) } deadline := time.Now().Add(slack(3 * time.Second)) - for time.Now().Before(deadline) && syscall.Kill(childPID, 0) == nil { + for time.Now().Before(deadline) && testProcessSignal(childPID, 0) == nil { time.Sleep(20 * time.Millisecond) } - if err := syscall.Kill(childPID, 0); err == nil { + if err := testProcessSignal(childPID, 0); err == nil { t.Fatalf("grandchild %d survived process-group kill", childPID) } } @@ -273,7 +273,7 @@ func TestExitedBackgroundShellDoesNotLeaveDetachedChildren(t *testing.T) { t.Fatalf("child pid %q: %v", pidText, err) } waitForJobDone(t, tools, 1, 5*time.Second) - if err := syscall.Kill(childPID, 0); err == nil { + if err := testProcessSignal(childPID, 0); err == nil { t.Fatalf("detached child %d survived its background shell", childPID) } } @@ -482,7 +482,7 @@ func TestSchedulerAbandonmentTearsDownLeafJobs(t *testing.T) { t.Fatalf("abandoned process did not start: %v; log=%q", err, logBody) } pid, _ := strconv.Atoi(strings.TrimSpace(string(pidBody))) - if err := syscall.Kill(pid, 0); err == nil { + if err := testProcessSignal(pid, 0); err == nil { t.Fatalf("process %d survived scheduler abandonment", pid) } } diff --git a/internal/exec/process_signal_unix_test.go b/internal/exec/process_signal_unix_test.go new file mode 100644 index 0000000000..a017d64fa9 --- /dev/null +++ b/internal/exec/process_signal_unix_test.go @@ -0,0 +1,9 @@ +//go:build !windows + +package exec + +import "syscall" + +// C10 keeps the existing Unix signal probes unchanged while allowing the native +// Windows contract tests to compile in the same test package. +func testProcessSignal(pid int, signal syscall.Signal) error { return syscall.Kill(pid, signal) } diff --git a/internal/exec/process_signal_windows_test.go b/internal/exec/process_signal_windows_test.go new file mode 100644 index 0000000000..b224d7ba61 --- /dev/null +++ b/internal/exec/process_signal_windows_test.go @@ -0,0 +1,31 @@ +//go:build windows + +package exec + +import ( + "syscall" + + "github.com/Agent-Field/codeaf/internal/processgroup" +) + +// C10 uses the platform's own process probes; Windows has no kill syscall. +func testProcessSignal(pid int, signal syscall.Signal) error { + group := pid < 0 + if group { + pid = -pid + } + if signal == 0 { + alive := processgroup.ProcessAlive(pid) + if group { + alive = processgroup.Alive(pid) + } + if alive { + return nil + } + return syscall.ESRCH + } + if signal == syscall.SIGKILL { + return processgroup.Kill(pid) + } + return processgroup.Terminate(pid) +} diff --git a/internal/exec/services.go b/internal/exec/services.go index c4bcd6473d..2c4ab50bb3 100644 --- a/internal/exec/services.go +++ b/internal/exec/services.go @@ -10,6 +10,7 @@ import ( "time" "github.com/Agent-Field/codeaf/internal/processgroup" + commandshell "github.com/Agent-Field/codeaf/internal/shell" ) const ( @@ -60,11 +61,15 @@ func ProcessIdentityMatches(pid int, startedAt time.Time) (bool, error) { // StartDetachedService uses the same shell, log, session/process-group shape // as background jobs. No pipe or resident goroutine is created. func StartDetachedService(command, dir, logPath string) (int, time.Time, error) { - logFile, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o644) + bash, err := commandshell.Bash() if err != nil { return 0, time.Time{}, err } - cmd := exec.Command("bash", "-lc", command) + logFile, err := commandshell.OpenLog(logPath, 0o644) + if err != nil { + return 0, time.Time{}, err + } + cmd := exec.Command(bash, "-lc", command) configureDetachedCommand(cmd, dir, logFile) if err := cmd.Start(); err != nil { _ = logFile.Close() diff --git a/internal/exec/services_test.go b/internal/exec/services_test.go index 051b429d15..797f01b757 100644 --- a/internal/exec/services_test.go +++ b/internal/exec/services_test.go @@ -1,3 +1,5 @@ +//go:build !windows + package exec import ( diff --git a/internal/exec/shell_windows_test.go b/internal/exec/shell_windows_test.go new file mode 100644 index 0000000000..5394801037 --- /dev/null +++ b/internal/exec/shell_windows_test.go @@ -0,0 +1,175 @@ +//go:build windows + +package exec + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/processgroup" + "github.com/Agent-Field/codeaf/internal/rtk" + commandshell "github.com/Agent-Field/codeaf/internal/shell" + "github.com/Agent-Field/codeaf/internal/store" +) + +// N2 proves the detached-service door gives Git Bash a writable output handle, +// appending stdout and stderr after earlier bytes instead of dying silently. +func TestWindowsDetachedServiceWritesLog(t *testing.T) { + requireWindowsBash(t) + dir := t.TempDir() + path := filepath.Join(dir, "service.log") + if err := os.WriteFile(path, []byte("earlier\n"), 0o644); err != nil { + t.Fatal(err) + } + pid, _, err := StartDetachedService("printf 'stdout\\n'; printf 'stderr\\n' >&2", dir, path) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = processgroup.Kill(pid) }) + deadline := time.After(20 * time.Second) + tick := time.NewTicker(20 * time.Millisecond) + defer tick.Stop() + for { + body, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(body) == "earlier\nstdout\nstderr\n" { + return + } + select { + case <-deadline: + t.Fatalf("service log = %q", body) + case <-tick.C: + } + } +} + +func requireWindowsBash(t *testing.T) { + t.Helper() + if _, err := commandshell.Bash(); err != nil { + t.Skipf("Git Bash is required for native command-door tests: %v", err) + } + t.Setenv("CODEAF_HOME", t.TempDir()) + t.Setenv("HOME", t.TempDir()) +} + +func windowsBackgroundOutput(t *testing.T, box *Toolbox, command string) string { + t.Helper() + result := box.startBackground(context.Background(), command, map[string]any{}) + if result.IsError { + t.Fatalf("job refused: %s", result.Content) + } + box.jobs.mutex.Lock() + var job *backgroundJob + for _, one := range box.jobs.jobs { + if job == nil || one.id > job.id { + job = one + } + } + box.jobs.mutex.Unlock() + if job == nil { + t.Fatal("job was not registered") + } + select { + case <-job.done: + case <-time.After(20 * time.Second): + t.Fatal("Windows job did not finish") + } + body, err := os.ReadFile(job.fullPath) + if err != nil || job.exitCode != 0 { + t.Fatalf("job output %q, exit %d, error %v", body, job.exitCode, err) + } + return string(body) +} + +// N2 proves both real Toolbox shell doors parse once and retain multiline text, +// quotes, backslashes, percent signs, substitutions and non-ASCII bytes. +func TestWindowsToolboxSingleParse(t *testing.T) { + requireWindowsBash(t) + box, _ := backgroundToolbox(t) + literal := "\" ' \\ %PATH% $(echo WRONG) héllo ✓" + command := "export PATH='/c/Program Files (x86)/test':$PATH\nx=inner; echo \"x=[$x]\"\nprintf 'line two\\n'\ncat <<'CODEAF_LITERAL'\n" + literal + "\nCODEAF_LITERAL\n" + want := "x=[inner]\nline two\n" + literal + "\n" + run := box.runShell(context.Background(), command, 20, "") + if run.err != nil || run.body != want { + t.Fatalf("foreground: %q %v", run.body, run.err) + } + if got := windowsBackgroundOutput(t, box, command); got != want { + t.Fatalf("background: %q, want %q", got, want) + } +} + +// N2 proves skills and rtk shelf programs run by bare name through foreground +// and background doors while their exported variables retain native paths. +func TestWindowsToolboxShelves(t *testing.T) { + requireWindowsBash(t) + history, err := store.Open(filepath.Join(t.TempDir(), "history.db")) + if err != nil { + t.Fatal(err) + } + defer history.Close() + box := NewToolboxWithStore(workspace(t), "windows", nil, history) + t.Cleanup(func() { box.Close() }) + skills, err := store.SkillsBinDir() + if err != nil { + t.Fatal(err) + } + rtkDir := filepath.Join(t.TempDir(), "rtk shelf (héllo)") + for _, shelf := range []struct{ dir, name, body string }{ + {skills, "windows-skill", "printf 'SKILL=%s\\n' \"$CODEAF_SKILLS_BIN\""}, + {rtkDir, "windows-rtk", "printf 'RTK=%s\\n' \"$CODEAF_RTK_BIN\""}, + {rtkDir, "rtk.exe", "unused stub for shelf discovery"}, + } { + if err := os.MkdirAll(shelf.dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(shelf.dir, shelf.name), []byte("#!/bin/sh\n"+shelf.body+"\n"), 0o755); err != nil { + t.Fatal(err) + } + } + rtkBin := filepath.Join(rtkDir, "rtk.exe") + t.Setenv(rtk.EnvBinary, rtkBin) + command := "windows-skill; windows-rtk" + want := "SKILL=" + skills + "\nRTK=" + rtkDir + "\n" + run := box.runShell(context.Background(), command, 20, rtkBin) + if run.err != nil || run.body != want { + t.Fatalf("foreground shelves: %q %v; want %q", run.body, run.err, want) + } + if got := windowsBackgroundOutput(t, box, command); got != want { + t.Fatalf("background shelves: %q; want %q", got, want) + } +} + +// N2 proves refusal happens before either real door starts a process or creates +// a job. A marker would expose a command run despite the explicit bad choice. +func TestWindowsToolboxRefusesMissingBash(t *testing.T) { + t.Setenv("CODEAF_HOME", t.TempDir()) + t.Setenv("HOME", t.TempDir()) + box, space := backgroundToolbox(t) + missing := filepath.Join(t.TempDir(), "missing bash.exe") + t.Setenv("CODEAF_BASH", missing) + for _, definition := range box.Definitions() { + if definition.Function.Name == "sh" { + t.Fatal("unusable shell is advertised") + } + } + command := "echo WRONG > should-not-exist" + run := box.runShell(context.Background(), command, 20, "").result(20) + job := box.startBackground(context.Background(), command, map[string]any{}) + for _, result := range []Result{run, job} { + if !result.IsError || !strings.Contains(result.Content, "CODEAF_BASH") || !strings.Contains(result.Content, missing) { + t.Fatalf("refusal: %+v", result) + } + } + if len(box.jobs.jobs) != 0 { + t.Fatal("refused job was registered") + } + if _, err := os.Stat(filepath.Join(space.Root(), "should-not-exist")); !os.IsNotExist(err) { + t.Fatalf("command ran: %v", err) + } +} diff --git a/internal/exec/tools.go b/internal/exec/tools.go index e822d50ba2..b7b4e055fd 100644 --- a/internal/exec/tools.go +++ b/internal/exec/tools.go @@ -25,6 +25,7 @@ import ( "github.com/Agent-Field/codeaf/internal/guard" "github.com/Agent-Field/codeaf/internal/processgroup" "github.com/Agent-Field/codeaf/internal/rtk" + commandshell "github.com/Agent-Field/codeaf/internal/shell" "github.com/Agent-Field/codeaf/internal/store" ) @@ -808,6 +809,11 @@ func (t *Toolbox) Definitions() []ai.ToolDefinition { "n": prop("integer", "max search results, default 6"), }), } + // An unavailable shell has no schema; direct callers still get the shared + // installation remedy before a process or background job can be created. + if _, err := commandshell.Bash(); err != nil { + definitions = definitions[1:] + } // Stratum 2: the door to the optional families. Its presence follows the // machine's wiring and nothing else, so it is in the same slot on every turn // of every leaf of a run, or on none of them. @@ -1459,6 +1465,9 @@ func (t *Toolbox) sh(ctx context.Context, args map[string]any) Result { if command == "" { return errorf("sh needs cmd") } + if _, err := commandshell.Bash(); err != nil { + return errorf("%s", err) + } if boolArg(args, "bg") { // A background job is deliberately never compressed. Its output is // watched rather than read — readiness loops grep the log, the job tool @@ -1510,6 +1519,9 @@ type shellRun struct { // bounded says the body was cut as it streamed and already carries its own // truncation notice naming the file the whole output went to. bounded bool + // refused keeps discovery errors in the shared remedy's exact words, before + // any process has started or produced an exit status. + refused bool } // trustworthy asks whether a wrapped run may stand as the answer. @@ -1537,6 +1549,9 @@ func (r shellRun) trustworthy(class rtk.Class) bool { // comes off the front. bounded rides along so the turn boundary does not cut an // already-cut body a second time. func (r shellRun) result(seconds int) Result { + if r.refused { + return Result{Content: r.err.Error(), IsError: true, shape: shapeCommand} + } if r.timedOut { out := errorf("command timed out after %ds. Partial output:\n%s", seconds, r.body) out.shape, out.bounded, out.timedOut = shapeCommand, r.bounded, true @@ -1565,6 +1580,10 @@ func (r shellRun) result(seconds int) Result { // resolved rtk when this is a wrapped run and empty otherwise; a rewritten line // calls rtk by bare name, so its shelf goes on PATH here. func (t *Toolbox) runShell(ctx context.Context, command string, seconds int, rtkBin string) shellRun { + bash, err := commandshell.Bash() + if err != nil { + return shellRun{err: err, exitCode: 127, refused: true} + } runCtx, cancel := context.WithTimeout(ctx, time.Duration(seconds)*time.Second) defer cancel() var environment []string @@ -1576,7 +1595,7 @@ func (t *Toolbox) runShell(ctx context.Context, command string, seconds int, rtk environment = os.Environ() environment = replaceEnv(environment, "CODEAF_SKILLS_BIN", bin) environment = replaceEnv(environment, env.Legacy("CODEAF_SKILLS_BIN"), bin) - command = "export PATH=\"${CODEAF_SKILLS_BIN:?}:$PATH\"\n" + command + command = commandshell.PathExport("CODEAF_SKILLS_BIN", bin) + command } } if rtkBin != "" { @@ -1585,7 +1604,7 @@ func (t *Toolbox) runShell(ctx context.Context, command string, seconds int, rtk } environment = replaceEnv(environment, "CODEAF_RTK_BIN", filepath.Dir(rtkBin)) environment = replaceEnv(environment, env.Legacy("CODEAF_RTK_BIN"), filepath.Dir(rtkBin)) - command = "export PATH=\"${CODEAF_RTK_BIN:?}:$PATH\"\n" + command + command = commandshell.PathExport("CODEAF_RTK_BIN", filepath.Dir(rtkBin)) + command } // A COMMAND THE MODEL RUNS MUST NOT REACH THE TMUX SERVER HOSTING CODEAF, // and here the bare path used to hand the child its parent's whole @@ -1593,7 +1612,7 @@ func (t *Toolbox) runShell(ctx context.Context, command string, seconds int, rtk // names a private socket directory even in that nil case (tools.go). environment = JobShellEnv(environment) - cmd := exec.CommandContext(runCtx, "bash", "-lc", command) + cmd := exec.CommandContext(runCtx, bash, "-lc", command) cmd.Dir = t.workspace.Root() cmd.Env = environment // A command that leaves a background child sharing its stdout used to hang @@ -1629,7 +1648,7 @@ func (t *Toolbox) runShell(ctx context.Context, command string, seconds int, rtk // the notice that ends up in context names a command that actually works. collected := newCappedOutput(strip, t.budgets.spill, t.budgets.preview, t.openSpill) cmd.Stdout, cmd.Stderr = collected, collected - err := cmd.Run() + err = cmd.Run() run := shellRun{body: collected.String(), bounded: collected.truncated(), err: err, exitCode: exitCode(cmd, err)} run.timedOut = runCtx.Err() == context.DeadlineExceeded diff --git a/internal/manual/chat/running-commands-on-windows.md b/internal/manual/chat/running-commands-on-windows.md new file mode 100644 index 0000000000..aea2c33946 --- /dev/null +++ b/internal/manual/chat/running-commands-on-windows.md @@ -0,0 +1,52 @@ +# Running commands on Windows + +## Which bash does codeaf use on Windows? (Git Bash) + +On Windows, codeaf runs commands in Git for Windows' bash (or another MSYS2-family bash). It looks in this order: + +1. `CODEAF_BASH`, an explicit path to a bash.exe. A missing file or a WSL launcher stops the command and names the variable and path. +2. Git found through `git.exe` on PATH, in `cmd`, `bin`, or `mingw64\bin`. If it is a package-manager shim elsewhere, codeaf asks `git --exec-path` (with a three-second timeout) and looks up at most four parent folders for Git Bash. A failed lookup falls through to the next step. +3. Git under `%ProgramW6432%`, `%ProgramFiles%`, `%ProgramFiles(x86)%`, then `%LOCALAPPDATA%\Programs\Git` and `%USERPROFILE%\scoop\apps\git\current`. +4. `bash.exe` on PATH, unless it is a WSL launcher. + +For each Git installation, `bin\bash.exe` wins over `usr\bin\bash.exe`. Set `CODEAF_BASH` to choose a Git for Windows or MSYS2 bash.exe yourself; a relative choice is resolved from the current working folder. Unset it to restore this search. + +## Can codeaf use WSL to run commands? + +codeaf running natively on Windows never uses WSL's bash.exe in Windows system folders or WindowsApps. WSL would run commands in a different system from the files codeaf reads and writes. A command and a file tool could then see different files. Tools installed only inside WSL are not on the native Windows command path; install the Windows version or a tool that works in Git Bash. + +Windows commands use Git Bash or another MSYS2-family bash. Setting `CODEAF_BASH` to WSL's launcher stops the command and explains how to choose a usable bash.exe. + +## Why do my commands fail on Windows when no bash was found? + +codeaf needs Git for Windows' bash or another MSYS2-family bash to run commands on Windows. If none is found, the `bash` shell tool is absent from the model’s tools, no command runs, and the project facts explain: + +"codeaf runs commands in Git for Windows' bash, and none was found: install Git for Windows, or set CODEAF_BASH to a bash.exe; WSL's bash.exe is not used." + +Install Git for Windows and restart codeaf, or set `CODEAF_BASH` to its bash.exe before starting codeaf. WSL's bash.exe runs in a different system from codeaf's file tools and is never used. + +## CODEAF_BASH does not exist or names WSL's launcher + +On Windows, `CODEAF_BASH` chooses an explicit bash.exe. An unusable choice stops the command; codeaf does not silently search for another. With `` replaced by the path you set, the missing-file answer is: + +`"CODEAF_BASH names , which does not exist. Set it to a Git for Windows or MSYS2 bash.exe, or unset it and codeaf will look for Git Bash."` + +The WSL-launcher answer is: + +`"CODEAF_BASH names , which is WSL's launcher and runs commands in a different system. Set it to a Git for Windows or MSYS2 bash.exe, or unset it and codeaf will look for Git Bash."` + +## How do I write Windows drive paths in Git Bash? + +Git Bash uses `/c/…`, `/d/…` for Windows drives, for example `/c/Users/me/project`. codeaf converts paths it places in shell PATH lists and command targets to this form. Relative Windows paths such as `C:skills\bin`, `\tools\bin` and `skills\bin` are first made absolute using Windows' current drive and folder. Already POSIX-shaped paths beginning with `/` stay as written. Values read by native Windows programs keep their Windows form. The worker's project facts name its selected shell and the drive path form. Cygwin's `/cygdrive/` prefix is not supported. + +When Git Bash starts a Windows program, it normally converts arguments beginning with `/` into Windows paths. That is how `/c/Users/me` reaches a native Windows tool as a drive path. + +## Why does gh api say not found on Windows? + +Git Bash normally rewrites arguments beginning with `/` into Windows paths when it starts a Windows program. Thus `gh api /repos/owner/project` can send a Windows path instead of an API route. This is Git Bash's default behavior, also used by commands in codeaf on Windows. + +Write the API route without its leading slash: `gh api repos/owner/project`. Or disable conversion for that command: `MSYS_NO_PATHCONV=1 gh api /repos/owner/project`. Keep the default conversion for commands that need Git Bash's `/c/…` drive paths translated for a Windows program. + +## Can I run codeaf inside WSL on Windows? + +Yes. Run the Linux build of codeaf inside WSL and its commands use Linux bash, beside the Linux files its file tools read and write. The Git for Windows requirement applies to the native Windows build. Native Windows codeaf never selects the WSL launcher as its shell. diff --git a/internal/manual/chat_test.go b/internal/manual/chat_test.go index ccb145b219..fcc856b7a9 100644 --- a/internal/manual/chat_test.go +++ b/internal/manual/chat_test.go @@ -1,6 +1,7 @@ package manual import ( + "fmt" "go/ast" "go/parser" "go/token" @@ -10,6 +11,8 @@ import ( "strconv" "strings" "testing" + + "github.com/Agent-Field/codeaf/internal/shell" ) // RETRIEVAL IS THE FEATURE, NOT THE PAGES. @@ -30,6 +33,13 @@ func TestTheChatManualAnswersTheQuestionsPeopleAsk(t *testing.T) { question string page string }{ + // N4 reaches the Windows requirements in the asker's words. + {"which bash does codeaf use on Windows", "running-commands-on-windows"}, + {"my commands fail on Windows when no bash was found", "running-commands-on-windows"}, + {"CODEAF_BASH does not exist", "running-commands-on-windows"}, + {"why does gh api say not found on Windows", "running-commands-on-windows"}, + {"can I run codeaf inside WSL on Windows", "running-commands-on-windows"}, + // Installer refusals must reach the page that explains both API attempts. {"installer says github api rate limit is used up", "running-from-the-terminal"}, {"install script says GitHub rejected GITHUB_TOKEN bad credentials", "running-from-the-terminal"}, @@ -4177,3 +4187,16 @@ func TestTheChatManualExplainsStoppedResultRouting(t *testing.T) { } } } + +// N4 pins all shared refusals quoted by the Windows manual sections. +func TestWindowsManualQuotesTheShellRefusal(t *testing.T) { + page, ok := Chat().Page("running-commands-on-windows") + if !ok { + t.Fatal("the Windows shell page is missing") + } + for _, refusal := range []string{shell.NoBash, fmt.Sprintf(shell.MissingBash, ""), fmt.Sprintf(shell.WSLBash, "")} { + if !strings.Contains(page, "\""+refusal+"\"") { + t.Errorf("Windows manual must quote the shared refusal exactly: %q", refusal) + } + } +} diff --git a/internal/plandb/persist.go b/internal/plandb/persist.go index 569468ba4e..d810755ec6 100644 --- a/internal/plandb/persist.go +++ b/internal/plandb/persist.go @@ -218,7 +218,7 @@ func openDatabase(path string) (*sql.DB, error) { return nil, fmt.Errorf("create plan store directory: %w", err) } } - dsn := "file:" + filepath.ToSlash(path) + "?_txlock=immediate&_pragma=busy_timeout(5000)" + dsn := StoreURI(path, "_txlock=immediate&_pragma=busy_timeout(5000)") db, err := sql.Open("sqlite", dsn) if err != nil { return nil, err @@ -247,7 +247,7 @@ func openDatabase(path string) (*sql.DB, error) { // for it. The write handle is always opened first, so the schema this one // reads already exists. func openReadDatabase(path string) (*sql.DB, error) { - db, err := sql.Open("sqlite", "file:"+filepath.ToSlash(path)+"?_pragma=busy_timeout(5000)") + db, err := sql.Open("sqlite", StoreURI(path, "_pragma=busy_timeout(5000)")) if err != nil { return nil, err } diff --git a/internal/plandb/preview.go b/internal/plandb/preview.go index 7197b5add3..a4caf8f221 100644 --- a/internal/plandb/preview.go +++ b/internal/plandb/preview.go @@ -4,8 +4,6 @@ import ( "database/sql" "path/filepath" "time" - - "github.com/Agent-Field/codeaf/internal/sqliteuri" ) // RootPreview is the original request behind a saved run. Listing a conversation @@ -24,9 +22,7 @@ func ReadRootPreview(path string) (RootPreview, error) { if err != nil { return RootPreview{}, err } - uri := sqliteuri.File(absolute) - uri.RawQuery = "mode=ro" - db, err := sql.Open("sqlite", uri.String()) + db, err := sql.Open("sqlite", StoreURI(absolute, "mode=ro")) if err != nil { return RootPreview{}, err } diff --git a/internal/plandb/uri.go b/internal/plandb/uri.go new file mode 100644 index 0000000000..79d448d20e --- /dev/null +++ b/internal/plandb/uri.go @@ -0,0 +1,35 @@ +package plandb + +import ( + "path/filepath" + "runtime" + + "github.com/Agent-Field/codeaf/internal/sqliteuri" +) + +// StoreURI keeps every plan reader at the writer's existing address. SQLite +// interprets escapes and fragments in this raw spelling, so escaping a reader's +// filename would send it to a different file for paths containing % or #. +// THE WRITER'S URI BYTES ARE THE COMPATIBILITY CONTRACT, including its query. +// +// WINDOWS IS THE ONE PLACE THE CONTRACT DOES NOT BIND. There the raw spelling +// could never open a store whose path held a percent escape or a #: SQLite +// decoded or cut the name to a folder that did not exist, and the open failed +// with "unable to open database file" (measured natively on 2026-10-19). No +// store was ever written at such an address, so Windows uses the escaped URI +// every other store uses, and for every ordinary path that names the same file +// the raw spelling did. +func StoreURI(path, query string) string { return storeURI(runtime.GOOS, path, query) } + +func storeURI(goos, path, query string) string { + if goos == "windows" { + uri := sqliteuri.File(path) + uri.RawQuery = query + return uri.String() + } + uri := "file:" + filepath.ToSlash(path) + if query != "" { + uri += "?" + query + } + return uri +} diff --git a/internal/plandb/uri_test.go b/internal/plandb/uri_test.go new file mode 100644 index 0000000000..70dbaaa986 --- /dev/null +++ b/internal/plandb/uri_test.go @@ -0,0 +1,94 @@ +package plandb + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// N2 and N3 pins both writer handles' original Unix bytes and the four session +// readers' pre-branch Unix bytes, including characters SQLite interprets in a +// URI, and that Windows takes the escaped spelling instead. +func TestStoreURIKeepsWriterAndUnixReaderBytes(t *testing.T) { + for _, path := range []string{ + "relative plan.db", "/tmp/dir with space é %20 #/plan %20 #.db", + `C:\dir with space é %20 #\plan %20 #.db`, `\\server\share\plan.db`, + } { + for _, query := range []string{ + "", "_txlock=immediate&_pragma=busy_timeout(5000)", + "_pragma=busy_timeout(5000)", "mode=ro", "mode=ro&_pragma=busy_timeout(2000)", + } { + want := "file:" + filepath.ToSlash(path) + if query != "" { + want += "?" + query + } + if got := storeURI("linux", path, query); got != want { + t.Fatalf("writer bytes: %q, want %q", got, want) + } + + } + if runtime.GOOS != "windows" { + if got, want := StoreURI(path, "mode=ro"), "file:"+path+"?mode=ro"; got != want { + t.Fatalf("Unix session reader bytes: %q, want %q", got, want) + } + } + } +} + +// N2 and N3 prove both public readers reach the real writer for directories and file +// names containing spaces, Unicode, percent escapes and fragment characters. +func TestPlanReadersUseWriterURI(t *testing.T) { + testPlanReadersUseWriterURI(t) +} + +func testPlanReadersUseWriterURI(t *testing.T) { + t.Helper() + // Keep metacharacters out of the subtest name: TempDir includes that name + // before its unique suffix, which a raw URI fragment would otherwise cut off. + for _, tc := range []struct{ name, directory string }{ + {"fragment in directory", "dir with space é %20 #"}, + {"fragment in filename", "dir with space é %20"}, + } { + t.Run(tc.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), tc.directory, "plan with space %20 #.db") + // The raw writer decodes %20. Prepare that existing-store parent too, + // so the second case reaches the filename's # rather than stopping + // at the directory's #. Both spellings stay under this test's temp dir. + if err := os.MkdirAll(strings.ReplaceAll(filepath.Dir(path), "%20", " "), 0o700); err != nil { + t.Fatal(err) + } + s, err := Open(path, "uri", "root", "héllo", "read the committed plan") + if err != nil { + t.Fatal(err) + } + defer s.Close() + if _, err := s.AddMany([]TaskSpec{{ID: "work", ParentID: "root", Title: "do the work"}}); err != nil { + t.Fatal(err) + } + if err := s.AddSpend("work", "uri-model", RoleWork, 0.25, 10, 20); err != nil { + t.Fatal(err) + } + preview, err := ReadRootPreview(path) + if err != nil || preview.Title != "héllo" || preview.Description != "read the committed plan" || preview.CreatedAt.IsZero() { + t.Fatalf("preview = %+v, %v", preview, err) + } + }) + } +} + +// N2 uses the existing shared Windows URI door while keeping plan query bytes. +func TestWindowsPlanURI(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("the shared formatter follows the host filesystem") + } + for _, tc := range []struct{ path, want string }{ + {`C:\plans\state %20 #.db`, "file:///C:/plans/state%20%2520%20%23.db?mode=ro"}, + {`\\server\share\state.db`, "file:////server/share/state.db?mode=ro"}, + } { + if got := StoreURI(tc.path, "mode=ro"); got != tc.want { + t.Fatalf("URI = %q, want %q", got, tc.want) + } + } +} diff --git a/internal/rtk/rtk.go b/internal/rtk/rtk.go index 0ee4d4e186..c4721da856 100644 --- a/internal/rtk/rtk.go +++ b/internal/rtk/rtk.go @@ -31,6 +31,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "strings" "sync" @@ -230,7 +231,8 @@ func locate() (string, bool) { func executable(path string) bool { info, err := os.Stat(path) - return err == nil && !info.IsDir() && info.Mode()&0o111 != 0 + // Windows has no Unix execute bits; its native binaries are regular files. + return err == nil && !info.IsDir() && (runtime.GOOS == "windows" || info.Mode()&0o111 != 0) } // BinDir is where a bootstrapped rtk lives. It is codeaf's own shelf, not the diff --git a/internal/session/automation_run.go b/internal/session/automation_run.go index 8b3a4985e6..7b25463766 100644 --- a/internal/session/automation_run.go +++ b/internal/session/automation_run.go @@ -168,7 +168,10 @@ func (r *automationRunner) Look(ctx context.Context, item automation.Automation) // it replaces did not do. A failing command is evidence, not an error: "is CI // red" is often answered by an exit status. func automationLookCommand(ctx context.Context, workspace, command string) (automation.Sight, error) { - shell, args := jobShell() + shell, args, shellErr := jobShell() + if shellErr != nil { + return automation.Sight{}, shellErr + } process := exec.CommandContext(ctx, shell, append(args, command)...) process.Dir = workspace process.Env = bare.StreamingEnv() diff --git a/internal/session/bashbelt.go b/internal/session/bashbelt.go index 4c9422f43b..38cdeeadfd 100644 --- a/internal/session/bashbelt.go +++ b/internal/session/bashbelt.go @@ -59,7 +59,10 @@ func bashBeltSourceCaps() bare.Caps { // in the order [Agent.belt] composes today's node belt so a worker sees the // same list minus what came off. func (a *Agent) bashBelt() []bare.Tool { - tools := []bare.Tool{a.branchBash()} + var tools []bare.Tool + if hand := a.branchBash(); hand.Name != "" { + tools = append(tools, hand) + } tools = append(tools, a.documentTool(), a.jobsTool(), a.manualTool()) // ASK IS OFF THIS BELT. A bash-belt worker coordinates through the plan CLI // and reports through `plandb done`; there is no consent gate to reach the @@ -120,6 +123,9 @@ func (a *Agent) branchBash() bare.Tool { break } } + if hand.Name == "" { + return hand + } // THE DESCRIPTION IS THE BRANCH'S OWN, because the cut it names is: the // model reads a description that promises the last N lines and plans // around a tool that does something else. The schema is bare's verbatim — diff --git a/internal/session/bashbelt_jobs_test.go b/internal/session/bashbelt_jobs_test.go index fa623e140a..92dc19154f 100644 --- a/internal/session/bashbelt_jobs_test.go +++ b/internal/session/bashbelt_jobs_test.go @@ -105,7 +105,7 @@ func TestBashBeltEnvelopeStillRefusesANameTheBeltDoesNotCarry(t *testing.T) { func processGroupGone(group int) bool { deadline := time.Now().Add(5 * time.Second) for { - if err := syscall.Kill(-group, 0); errors.Is(err, syscall.ESRCH) { + if err := testProcessSignal(-group, 0); errors.Is(err, syscall.ESRCH) { return true } if time.Now().After(deadline) { diff --git a/internal/session/beltfacts.go b/internal/session/beltfacts.go index 58884a41b7..5c74d454f9 100644 --- a/internal/session/beltfacts.go +++ b/internal/session/beltfacts.go @@ -4,6 +4,7 @@ import ( "strings" "github.com/Agent-Field/codeaf/internal/exec" + "github.com/Agent-Field/codeaf/internal/shell" ) // THE PROMPT NAMES EXACTLY THE TOOLS THE CALL CARRIES. @@ -77,7 +78,10 @@ func (c Config) shelvesCapabilities() bool { return !c.InTask } // mayWatch says whether `watch` belongs on this belt (tools.go). It comes off // inside a task because its whole delivery mechanism is a note arriving in a // conversation, and a node has none. -func (c Config) mayWatch() bool { return !c.InTask } +func (c Config) mayWatch() bool { + _, err := shell.Bash() + return !c.InTask && err == nil +} // mayAsk says whether `ask` belongs on this belt. It is unconditional on the // conversation's belt — only the shelf changes whether the schema is carried now diff --git a/internal/session/chatpage.go b/internal/session/chatpage.go index 99bb12ac09..305d9583d1 100644 --- a/internal/session/chatpage.go +++ b/internal/session/chatpage.go @@ -6,6 +6,8 @@ import ( "runtime" "strings" "time" + + "github.com/Agent-Field/codeaf/internal/shell" ) // ── THE PAGE A MODEL WITH NO TOOLS READS ──────────────────────────────────── @@ -43,6 +45,7 @@ func chatPage(config Config, now time.Time) string { out.WriteString("\n\n# Project\n") fmt.Fprintf(&out, "- Workstation: %s/%s\n", runtime.GOOS, runtime.GOARCH) fmt.Fprintf(&out, "- Working directory: %s\n", config.Workspace) + out.WriteString(shell.PromptFact()) out.WriteString(nowLine(now)) return out.String() } diff --git a/internal/session/jobs.go b/internal/session/jobs.go index 99e5364527..8688acdc09 100644 --- a/internal/session/jobs.go +++ b/internal/session/jobs.go @@ -65,6 +65,7 @@ import ( "github.com/Agent-Field/codeaf/internal/exec/bare" "github.com/Agent-Field/codeaf/internal/processgroup" + "github.com/Agent-Field/codeaf/internal/shell" ) const ( @@ -808,6 +809,10 @@ func (r *jobRegistry) settled(one *job, code int) bool { // answering the person. The job's lifetime is the session's, and Close is the // only thing that ends it early. func (r *jobRegistry) start(command string) (*job, error) { + program, shellArgs, shellErr := bare.StreamingShell(command) + if shellErr != nil { + return nil, shellErr + } started, err := r.newJob(command, jobKindBash) if err != nil { return nil, err @@ -819,8 +824,7 @@ func (r *jobRegistry) start(command string) (*job, error) { // stays empty until exit is a job that looks dead for as long as it runs — // and this used to be a hand-copied three-line shell choice with no // buffering fix in it at all. - shell, shellArgs := bare.StreamingShell(command) - process := exec.Command(shell, shellArgs...) + process := exec.Command(program, shellArgs...) process.Dir = started.dir process.Env = bare.StreamingEnv() // Setsid puts the job and everything it spawns in one process group, so a @@ -1738,24 +1742,16 @@ func (s *jobSink) lastNonEmptyLine() string { // ── process plumbing ──────────────────────────────────────────────────────── -// jobShell mirrors bare's shell preference (/bin/bash, bash on PATH, sh). It is -// a copy rather than a call because bare's is unexported and this slice wraps -// that package rather than editing it; the order is three lines and it is the -// same three lines. +// jobShell shares bare's shell preference through internal/shell: Unix keeps +// /bin/bash, bash on PATH, sh, and Windows uses the engine's native bash. // // IT IS NOT FOR ANYTHING WHOSE OUTPUT SOMEBODY READS WHILE IT RUNS. A bash job // goes through [bare.StreamingShell] instead, which is this choice plus the // line-buffering that keeps a long command's log from being empty until it // exits. What is left on this one is a watch's tick and an automation's look // — commands that are short by construction and read only after they end. -func jobShell() (string, []string) { - if _, err := os.Stat("/bin/bash"); err == nil { - return "/bin/bash", []string{"-c"} - } - if bash, err := exec.LookPath("bash"); err == nil { - return bash, []string{"-c"} - } - return "sh", []string{"-c"} +func jobShell() (string, []string, error) { + return shell.Preferred() } // waitExitCode extracts an exit code from cmd.Wait's error, -1 when the process diff --git a/internal/session/plan_uri_test.go b/internal/session/plan_uri_test.go new file mode 100644 index 0000000000..252a493d92 --- /dev/null +++ b/internal/session/plan_uri_test.go @@ -0,0 +1,60 @@ +package session + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/plandb" +) + +// N2 and N3 prove every plan reader opens the store the real writer wrote, including +// the four session rollups that otherwise silently return no rows. +func TestSessionPlanReadersUseWriterURI(t *testing.T) { + testSessionPlanReadersUseWriterURI(t) +} + +func testSessionPlanReadersUseWriterURI(t *testing.T) { + t.Helper() + // Keep metacharacters out of the subtest name: TempDir includes that name + // before its unique suffix, which a raw URI fragment would otherwise cut off. + for _, tc := range []struct{ name, directory string }{ + {"fragment in directory", "dir with space é %20 #"}, + {"fragment in filename", "dir with space é %20"}, + } { + t.Run(tc.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), tc.directory, "plan with space %20 #.db") + // The raw writer decodes %20. Prepare that existing-store parent too, + // so the second case reaches the filename's # rather than stopping + // at the directory's #. Both spellings stay under this test's temp dir. + if err := os.MkdirAll(strings.ReplaceAll(filepath.Dir(path), "%20", " "), 0o700); err != nil { + t.Fatal(err) + } + s, err := plandb.Open(path, "uri", "root", "héllo", "read spending", "chat") + if err != nil { + t.Fatal(err) + } + defer s.Close() + if _, err := s.AddMany([]plandb.TaskSpec{{ID: "work", ParentID: "root", Title: "do the work"}}); err != nil { + t.Fatal(err) + } + if err := s.AddSpend("work", "uri-model", plandb.RoleWork, 0.25, 10, 20); err != nil { + t.Fatal(err) + } + if got, err := plandb.ReadRootPreview(path); err != nil || got.Title != "héllo" || got.Description != "read spending" || got.CreatedAt.IsZero() { + t.Fatalf("preview = %+v, %v", got, err) + } + if got := planSpendBySeat(path, "chat", time.Time{}); len(got) != 1 || got[0].Seat != plandb.RoleWork || got[0].Model != "uri-model" || got[0].USD != 0.25 || got[0].Calls != 1 { + t.Fatalf("seat spend = %+v", got) + } + if got := planSpendByTask(path); len(got) != 1 || got["work"] != 0.25 { + t.Fatalf("task spend = %+v", got) + } + if got := planUsageByTask(path); len(got) != 1 || got["work"].model != "uri-model" || got["work"].tokens != 30 { + t.Fatalf("task usage = %+v", got) + } + }) + } +} diff --git a/internal/session/plandb_plan.go b/internal/session/plandb_plan.go index 8f9fb60a22..6e4649b87b 100644 --- a/internal/session/plandb_plan.go +++ b/internal/session/plandb_plan.go @@ -24,12 +24,14 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "strings" "sync" "time" "github.com/Agent-Field/codeaf/internal/env" "github.com/Agent-Field/codeaf/internal/plandb" + "github.com/Agent-Field/codeaf/internal/shell" ) // planState is one run's plan: where the store lives, and whether the bin @@ -721,7 +723,11 @@ func (p *planState) armShim() error { return err } words := make([]string, 0, len(argv)+1) - for _, word := range argv { + for i, word := range argv { + // Only the exec target is a shell path; native CLI arguments keep their bytes. + if i == 0 { + word = shell.ListPath(word) + } words = append(words, quoteShWord(word)) } if err := writeShim(filepath.Join(bin, planShimFilename), "#!/bin/sh\nexec "+strings.Join(words, " ")+" \"$@\"\n"); err != nil { @@ -795,7 +801,7 @@ func (g *TaskGraph) planBashPrefix() string { if bin == "" { return "" } - prefix := "export PATH=" + quoteShWord(bin) + ":$PATH PLANDB_DB=" + quoteShWord(plan.path) + prefix := "export PATH=" + quoteShWord(shell.ListPath(bin)) + ":$PATH PLANDB_DB=" + quoteShWord(plan.path) // AND THE RUN IS BOUND, NOT ONLY THE PATH. A path says where the run's store // was when the run opened it; a later request can set that store aside and // seed another at the same path, and a worker bound by the path alone then @@ -839,7 +845,7 @@ func codeafShimScript() string { if runningCLI == "" { return "#!/bin/sh\necho " + quoteShWord(codeafShimRefusal) + " >&2\nexit 127\n" } - return "#!/bin/sh\nexec " + quoteShWord(runningCLI) + " \"$@\"\n" + return "#!/bin/sh\nexec " + quoteShWord(shell.ListPath(runningCLI)) + " \"$@\"\n" } // planCLIBinEnv is the resolver's one override: it names a binary that @@ -930,7 +936,9 @@ func fileExecutable(path string) bool { // line. A path with a space in it is ordinary on this machine and the shim // is read by /bin/sh, which splits on spaces without this. func quoteShWord(word string) string { - if !strings.ContainsAny(word, " \t\"'$`\\") { + // Drive conversion removes backslashes that used to force quoting. Windows + // shim words stay quoted even when their only punctuation is a parenthesis. + if runtime.GOOS != "windows" && !strings.ContainsAny(word, " \t\"'$`\\") { return word } return "'" + strings.ReplaceAll(word, "'", "'\\''") + "'" diff --git a/internal/session/plandb_tasks.go b/internal/session/plandb_tasks.go index 801f56d18d..f6a8a6cd88 100644 --- a/internal/session/plandb_tasks.go +++ b/internal/session/plandb_tasks.go @@ -615,7 +615,7 @@ func (a *Agent) PlanSpend(since time.Time) []PlanSpendLine { // digits vary, so a text comparison against a bound would misorder a whole // second against its own fraction. func planSpendBySeat(path, chat string, since time.Time) []PlanSpendLine { - db, err := sql.Open("sqlite", "file:"+path+"?mode=ro") + db, err := sql.Open("sqlite", plandb.StoreURI(path, "mode=ro")) if err != nil { return nil } @@ -924,7 +924,7 @@ func planTrajectory(dir, id string) []PlanStep { // beside a writer, and a store that will not open as a reader answers no // figures rather than failing the read. func planSpendByTask(path string) map[string]float64 { - db, err := sql.Open("sqlite", "file:"+path+"?mode=ro") + db, err := sql.Open("sqlite", plandb.StoreURI(path, "mode=ro")) if err != nil { return nil } @@ -958,7 +958,7 @@ type planTaskUsage struct { // its own read-only connection for the same reason. A row that names no model // names none, and a task whose rows carry no tokens has none known. func planUsageByTask(path string) map[string]planTaskUsage { - db, err := sql.Open("sqlite", "file:"+path+"?mode=ro") + db, err := sql.Open("sqlite", plandb.StoreURI(path, "mode=ro")) if err != nil { return nil } diff --git a/internal/session/principal_audit.go b/internal/session/principal_audit.go index 7285210c81..9f05619396 100644 --- a/internal/session/principal_audit.go +++ b/internal/session/principal_audit.go @@ -56,6 +56,7 @@ import ( "time" "github.com/Agent-Field/codeaf/internal/guard" + "github.com/Agent-Field/codeaf/internal/shell" "github.com/Agent-Field/codeaf/internal/verify" ) @@ -248,7 +249,11 @@ func runOneCheck(ctx context.Context, tree, check string, windows ...time.Durati } ctx, done := context.WithTimeout(ctx, window) defer done() - command := exec.CommandContext(ctx, "bash", "-c", check) + bash, err := shell.Bash() + if err != nil { + return CheckRun{Command: check, Tail: err.Error()} + } + command := exec.CommandContext(ctx, bash, "-c", check) command.Dir = tree output, err := command.CombinedOutput() // A COMMAND THAT RAN AND A COMMAND THAT COULD NOT BE RUN ARE DIFFERENT NEWS. diff --git a/internal/session/process_signal_unix_test.go b/internal/session/process_signal_unix_test.go new file mode 100644 index 0000000000..8f46de6786 --- /dev/null +++ b/internal/session/process_signal_unix_test.go @@ -0,0 +1,9 @@ +//go:build !windows + +package session + +import "syscall" + +// C10 keeps the existing Unix signal probes unchanged while allowing the native +// Windows contract tests to compile in the same test package. +func testProcessSignal(pid int, signal syscall.Signal) error { return syscall.Kill(pid, signal) } diff --git a/internal/session/process_signal_windows_test.go b/internal/session/process_signal_windows_test.go new file mode 100644 index 0000000000..3c95308bad --- /dev/null +++ b/internal/session/process_signal_windows_test.go @@ -0,0 +1,31 @@ +//go:build windows + +package session + +import ( + "syscall" + + "github.com/Agent-Field/codeaf/internal/processgroup" +) + +// C10 uses the platform's own process probes; Windows has no kill syscall. +func testProcessSignal(pid int, signal syscall.Signal) error { + group := pid < 0 + if group { + pid = -pid + } + if signal == 0 { + alive := processgroup.ProcessAlive(pid) + if group { + alive = processgroup.Alive(pid) + } + if alive { + return nil + } + return syscall.ESRCH + } + if signal == syscall.SIGKILL { + return processgroup.Kill(pid) + } + return processgroup.Terminate(pid) +} diff --git a/internal/session/program_model_commit_test.go b/internal/session/program_model_commit_test.go index cd5ed6791f..d37ba7301b 100644 --- a/internal/session/program_model_commit_test.go +++ b/internal/session/program_model_commit_test.go @@ -1,3 +1,5 @@ +//go:build !windows + package session import ( diff --git a/internal/session/prompt.go b/internal/session/prompt.go index a329a23ece..8c5f4e96d4 100644 --- a/internal/session/prompt.go +++ b/internal/session/prompt.go @@ -10,6 +10,7 @@ import ( "time" "github.com/Agent-Field/codeaf/internal/filememo" + "github.com/Agent-Field/codeaf/internal/shell" ) // systemPromptSource is omp's normal-chat system prompt, adapted (Decision 2). @@ -305,7 +306,13 @@ func renderSystemAt(config Config, now time.Time) string { // ([renderBeltFacts]) or the shell, or the pages are the wrong shape's. if config.mayBashBelt() { var out strings.Builder - out.WriteString(bashWorkerPage()) + if _, err := shell.Bash(); err != nil { + // A worker without its command tool must explain the missing + // requirement rather than follow a doctrine that promises it. + out.WriteString("You are codeaf. Shell commands are unavailable here. Explain the shell requirement from Project, and do not claim to run commands. The manual tool can explain Windows setup.\n") + } else { + out.WriteString(bashWorkerPage()) + } out.WriteString(workerFooter(config, now)) return out.String() } @@ -319,6 +326,11 @@ func renderSystemAt(config Config, now time.Time) string { // the shape; this conditions the sentences INSIDE one, which is where five // families of tools were being promised to workers that do not carry them. page := strings.TrimRight(promptWithBeltFacts(config), "\n") + if _, err := shell.Bash(); err != nil { + // Unix keeps its exact prompt bytes. On Windows an absent command + // tool cannot inherit instructions that promise shell execution. + page = "Shell commands are unavailable here. Instructions below about bash apply only when that tool is present; explain the shell requirement from Project instead of promising commands.\n\n" + page + } // AND THE PROFILE'S OWN CUT, WHICH IS THE ONE DOOR INTO IT. A lean prefix // drops the sections [leanPageSections] names, by their `# ` heading, and // gains the one line a shelved verb owes (promptprofile.go). A full prefix @@ -442,6 +454,7 @@ func workerFooter(config Config, now time.Time) string { if config.inOwnSpace() { out.WriteString("- There is no project here: this is the conversation's own space, and it holds only what this conversation has put there.\n") } + out.WriteString(shell.PromptFact()) out.WriteString(nowLine(now)) // THE PROJECT'S OWN RULES, UNDER THIS PROFILE'S BOUND. Both numbers here are diff --git a/internal/session/shell_unix_test.go b/internal/session/shell_unix_test.go new file mode 100644 index 0000000000..ab30b5290c --- /dev/null +++ b/internal/session/shell_unix_test.go @@ -0,0 +1,20 @@ +//go:build !windows + +package session + +import ( + "fmt" + "runtime" + "testing" + "time" +) + +// N3 pins the complete Unix project footer to its pre-Windows-shell bytes. +func TestUnixWorkerFooterBytes(t *testing.T) { + config := Config{Workspace: t.TempDir()} + now := time.Date(2026, 10, 19, 12, 0, 0, 0, time.UTC) + want := fmt.Sprintf("\n\n# Project\n- Workstation: %s/%s\n- Working directory: %s\n", runtime.GOOS, runtime.GOARCH, config.Workspace) + nowLine(now) + if got := workerFooter(config, now); got != want { + t.Fatalf("Unix worker footer changed:\ngot %q\nwant %q", got, want) + } +} diff --git a/internal/session/shell_windows_test.go b/internal/session/shell_windows_test.go new file mode 100644 index 0000000000..066a473b58 --- /dev/null +++ b/internal/session/shell_windows_test.go @@ -0,0 +1,213 @@ +//go:build windows + +package session + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/plandb" + commandshell "github.com/Agent-Field/codeaf/internal/shell" +) + +// The helper is a native CLI, so C6 observes the environment Windows programs +// really receive after Git Bash has executed the run's own shim. +func init() { + if os.Getenv("GO_WANT_WINDOWS_PLAN_CLI") == "1" && len(os.Args) > 1 && (os.Args[1] == "plandb" || os.Args[1] == "marker") { + fmt.Printf("RIGHT %s\nDB=%s\nRUN=%s\n", strings.Join(os.Args[1:], " "), os.Getenv("PLANDB_DB"), os.Getenv("PLANDB_RUN")) + os.Exit(0) + } +} + +// N2 prove the armed worker's real bash belt reaches its own native +// plandb and codeaf from another cwd, ahead of stale programs on PATH. +func TestWindowsWorkerShims(t *testing.T) { + if _, err := commandshell.Bash(); err != nil { + t.Skipf("Git Bash is required for native worker-shim tests: %v", err) + } + t.Setenv("CODEAF_HOME", t.TempDir()) + t.Setenv("HOME", t.TempDir()) + t.Setenv("CODEAF_TASK_BELT", "bash") + t.Setenv(planCLIBinEnv, "") + t.Setenv("GO_WANT_WINDOWS_PLAN_CLI", "1") + self, err := os.Executable() + if err != nil { + t.Fatal(err) + } + body, err := os.ReadFile(self) + if err != nil { + t.Fatal(err) + } + registered := filepath.Join(t.TempDir(), "running codeaf's (héllo).exe") + if err := os.WriteFile(registered, body, 0o755); err != nil { + t.Fatal(err) + } + previous := runningCLI + SetRunningCLI(registered) + t.Cleanup(func() { runningCLI = previous }) + stale := t.TempDir() + for _, name := range []string{"codeaf", "plandb"} { + writeScript(t, stale, name, "echo WRONG; exit 3") + } + t.Setenv("PATH", stale+string(os.PathListSeparator)+os.Getenv("PATH")) + planDir := filepath.Join(t.TempDir(), "plan's state (héllo)") + if err := os.MkdirAll(planDir, 0o755); err != nil { + t.Fatal(err) + } + store, err := plandb.Open(filepath.Join(planDir, planStoreFilename), "the work", planRootID, "", "") + if err != nil { + t.Fatal(err) + } + defer store.Close() + workspace := t.TempDir() + writeScript(t, workspace, "check.sh", "printf 'SCRIPT RIGHT\\n'") + agent, err := NewBeltWorker(Config{Workspace: workspace, Model: "test/model"}, &scriptedCompleter{}, store.Task(planRootID), store.Path(), store.RootID()) + if err != nil { + t.Fatal(err) + } + defer agent.Close() + plan := agent.graph().planIfArmed() + if plan == nil { + t.Fatal("worker has no armed plan") + } + destination := t.TempDir() + command := "cd " + quoteShWord(commandshell.ListPath(destination)) + "; x=inner; echo \"x=[$x]\"; plandb status; codeaf marker; " + checkCommand(workspace, "bash check.sh") + args, err := json.Marshal(map[string]string{"command": command}) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + text, failed, err := beltBash(t, agent).Execute(ctx, args) + if err != nil || failed { + t.Fatalf("worker bash: %q %v %v", text, failed, err) + } + for _, want := range []string{"x=[inner]", "RIGHT plandb status", "RIGHT marker", "SCRIPT RIGHT", "DB=" + plan.path, "RUN=" + plan.root} { + if !strings.Contains(text, want) { + t.Fatalf("missing %q in %q", want, text) + } + } + if strings.Contains(text, "WRONG") { + t.Fatalf("a stale CLI ran: %q", text) + } +} + +// N4 proves both rendered project-fact footers name the chosen Windows shell +// and drive spelling, rather than only testing a detached formatting helper. +func TestWindowsPromptFooter(t *testing.T) { + bash, err := commandshell.Bash() + if err != nil { + t.Skipf("Git Bash is required for shell facts: %v", err) + } + config := Config{Workspace: t.TempDir()} + now := time.Date(2026, 10, 19, 12, 0, 0, 0, time.UTC) + for _, page := range []string{workerFooter(config, now), chatPage(config, now)} { + // R4 carries the route conversion remedies to both model-facing pages. + if !strings.Contains(page, "- Shell: Git Bash ("+bash+")") || !strings.Contains(page, "/c/…, /d/…") || !strings.Contains(page, commandshell.PathConversionFact) { + t.Fatalf("missing shell facts: %q", page) + } + } +} + +// N1 omits bash from ordinary and task belts and stops before registering a job. +func TestWindowsMissingShellBeltAndJobs(t *testing.T) { + t.Setenv("CODEAF_HOME", t.TempDir()) + t.Setenv("CODEAF_BASH", filepath.Join(t.TempDir(), "missing.exe")) + for _, task := range []bool{false, true} { + agent, err := newAgent(Config{Workspace: t.TempDir(), Model: "test/model", InTask: task, bashBelt: task}, &scriptedCompleter{}) + if err != nil { + t.Fatal(err) + } + for _, tool := range agent.belt() { + if tool.Name == "bash" || tool.Name == "" || tool.Name == "watch" { + t.Fatalf("unusable tool %q", tool.Name) + } + } + if !task { + if _, err := agent.SubmitBash(context.Background(), "! echo WRONG > marker"); err == nil || !strings.Contains(err.Error(), "CODEAF_BASH") { + t.Fatalf("human shell did not explain missing bash: %v", err) + } + } + if _, err := agent.jobs.start("echo WRONG > marker"); err == nil { + t.Fatal("missing shell started a job") + } + page := renderSystemAt(agent.config, time.Date(2026, 10, 22, 12, 0, 0, 0, time.UTC)) + if !strings.Contains(page, "Shell commands are unavailable") || !strings.Contains(page, "CODEAF_BASH") { + t.Fatalf("missing setup facts: %q", page) + } + agent.Close() + } +} + +// N1 and N2 run session jobs, automations, watches and task checks through the +// same native shell, preserving variables, cwd, exit status and the job log. +func TestWindowsSessionCommandDoors(t *testing.T) { + if _, err := commandshell.Bash(); err != nil { + t.Fatal(err) + } + t.Setenv("CODEAF_HOME", t.TempDir()) + // This fixture owns its spending ledger as well as its agent; Windows + // cannot remove the temporary home while the ledger writer holds it open. + t.Cleanup(func() { + if !StopUsageWriter(UsageLedgerPath()) { + t.Error("fixture ledger did not close") + } + }) + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "marker"), []byte("right"), 0600); err != nil { + t.Fatal(err) + } + agent, err := newAgent(Config{Workspace: dir, Model: "test/model"}, &scriptedCompleter{}) + if err != nil { + t.Fatal(err) + } + defer agent.Close() + command := `export PATH='/c/Program Files (x86)/fixture':$PATH; x=inner; printf '%s:' "$x"; cat marker` + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + sight, err := automationLookCommand(ctx, dir, command+"; exit 23") + if err != nil || sight.Text != "inner:right\n(exit status 23)" { + t.Fatalf("automation: %+v %v", sight, err) + } + output, failure := agent.jobs.runTick(ctx, watchSpec{command: command, every: 20 * time.Second}) + if output != "inner:right" || failure != "" { + t.Fatalf("watch: %q %q", output, failure) + } + reading := runOneCheck(ctx, dir, command) + if !reading.Ran || !reading.Passed || reading.Tail != "inner:right" { + t.Fatalf("task check: %+v", reading) + } + stream, err := agent.SubmitBash(ctx, "!"+command+"; exit 23") + if err != nil { + t.Fatal(err) + } + found := false + for _, event := range collect(t, stream) { + if event.Kind == EventToolFailed && strings.Contains(event.Output, "inner:right") && strings.Contains(event.Output, "23") { + found = true + } + } + if !found { + t.Fatal("the person's command lost its cwd, variables or exit status") + } + + job, err := agent.jobs.start(command) + if err != nil { + t.Fatal(err) + } + select { + case <-job.done: + case <-ctx.Done(): + t.Fatal("job did not finish") + } + body, err := os.ReadFile(job.logPath) + if err != nil || !strings.Contains(string(body), "inner:right") { + t.Fatalf("job log: %q %v", body, err) + } +} diff --git a/internal/session/sqlite_windows_test.go b/internal/session/sqlite_windows_test.go new file mode 100644 index 0000000000..9563322996 --- /dev/null +++ b/internal/session/sqlite_windows_test.go @@ -0,0 +1,11 @@ +//go:build windows + +package session + +import "testing" + +// N2 and N3 reach all six readers on Windows without constructing an agent: +// each must see the writer's rows at a native path containing %20 and #. +func TestWindowsSessionPlanReadersNativePath(t *testing.T) { + testSessionPlanReadersUseWriterURI(t) +} diff --git a/internal/session/task_checks.go b/internal/session/task_checks.go index 91aeb8ae76..09b7278107 100644 --- a/internal/session/task_checks.go +++ b/internal/session/task_checks.go @@ -148,6 +148,7 @@ import ( "time" "github.com/Agent-Field/codeaf/internal/approval" + "github.com/Agent-Field/codeaf/internal/shell" ) const ( @@ -713,7 +714,7 @@ func checkCommand(ground, check string) string { // The second word is the one [fileChecksIn] resolved, so the file it // found is the file this command is about — and a wildcard the work // wrote takes its first match, for the reason stated just below. - return fields[0] + " " + shellQuoted(files[0].path) + return fields[0] + " " + shellQuoted(checkFileArgument(fields[0], files[0].path)) } // A WILDCARD THE WORK WROTE RESOLVES TO WHATEVER IT MATCHES, and the // first match that can be started is the check. The alternative — @@ -722,9 +723,9 @@ func checkCommand(ground, check string) string { for _, file := range files { switch { case file.runnable: - return shellQuoted(file.path) + return shellQuoted(shell.ListPath(file.path)) case file.interpreter != "": - return file.interpreter + " " + shellQuoted(file.path) + return file.interpreter + " " + shellQuoted(checkFileArgument(file.interpreter, file.path)) } } return "" @@ -735,6 +736,17 @@ func checkCommand(ground, check string) string { return "" } +// checkFileArgument converts filenames read by a shell interpreter, while a +// native interpreter keeps its own Windows filename rather than an MSYS path. +func checkFileArgument(interpreter, filename string) string { + switch strings.ToLower(filepath.Base(interpreter)) { + case "bash", "sh", "bash.exe", "sh.exe": + return shell.ListPath(filename) + default: + return filename + } +} + // shellQuoted wraps one path so that a shell reads it as ONE WORD, whatever is // in it. // diff --git a/internal/session/task_job_park_test.go b/internal/session/task_job_park_test.go index aeb876ec05..fd33154a8f 100644 --- a/internal/session/task_job_park_test.go +++ b/internal/session/task_job_park_test.go @@ -458,6 +458,7 @@ func TestATaskParksOnItsPromotedForegroundCommandUntilItExits(t *testing.T) { // CAN HAVE. A `sleep 30` stood here once and was the same test with a ceiling on // it: nothing about this is supposed to depend on the stop arriving within half // a minute. +// N2 and N3 preserve command-stop behaviour through the platform process probe. func TestAStoppedCommandWakesTheWaitWithItsOwnEnding(t *testing.T) { record := &askLog{} long := holdACommand(t, "the long one", "never") @@ -510,7 +511,7 @@ func TestAStoppedCommandWakesTheWaitWithItsOwnEnding(t *testing.T) { t.Fatalf("job %d is still running after the person stopped it", promoted.id) } waitFor(t, "the stopped process to be gone", func() bool { - return syscall.Kill(pid, syscall.Signal(0)) != nil + return testProcessSignal(pid, syscall.Signal(0)) != nil }) // AND NOBODY IS STILL PARKED. A wait released by a road nothing closes is a diff --git a/internal/session/tools_watch.go b/internal/session/tools_watch.go index 1dd43a431d..7a29064a3c 100644 --- a/internal/session/tools_watch.go +++ b/internal/session/tools_watch.go @@ -664,7 +664,10 @@ func (r *jobRegistry) runTick(ctx context.Context, spec watchSpec) (string, stri tickCtx, cancel := context.WithTimeout(ctx, timeout) defer cancel() - shell, shellArgs := jobShell() + shell, shellArgs, shellErr := jobShell() + if shellErr != nil { + return "", shellErr.Error() + } process := exec.CommandContext(tickCtx, shell, append(shellArgs, spec.command)...) process.Dir = r.workspace process.Env = os.Environ() diff --git a/internal/session/userbash.go b/internal/session/userbash.go index a9cb1fe06a..a525cafe65 100644 --- a/internal/session/userbash.go +++ b/internal/session/userbash.go @@ -12,6 +12,7 @@ import ( "github.com/Agent-Field/agentfield/sdk/go/ai" "github.com/Agent-Field/codeaf/internal/exec/bare" + "github.com/Agent-Field/codeaf/internal/shell" ) // These refusals are shared with the composer so a late engine refusal gives @@ -43,6 +44,9 @@ func (a *Agent) SubmitBash(ctx context.Context, text string) (<-chan Event, erro if a.config.InTask { return nil, errors.New("run ! commands from the parent conversation") } + if _, err := shell.Bash(); err != nil { + return nil, err + } user := userText(text) user.bash = command return a.submitUser(ctx, user) diff --git a/internal/shell/cache_windows_test.go b/internal/shell/cache_windows_test.go new file mode 100644 index 0000000000..de6e6afaf9 --- /dev/null +++ b/internal/shell/cache_windows_test.go @@ -0,0 +1,60 @@ +//go:build windows + +package shell + +import ( + "fmt" + "os" + "path/filepath" + "testing" +) + +// N1 binds a relative override to its current directory, never the cached +// choice from a former directory. No executable needs to run for this proof. +func TestWindowsRelativeOverrideCacheFollowsDirectory(t *testing.T) { + a, b := t.TempDir(), t.TempDir() + previous, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := os.Chdir(previous); err != nil { + t.Error(err) + } + }) + changeDir := func(dir string) { + t.Helper() + if err := os.Chdir(dir); err != nil { + t.Fatal(err) + } + } + candidate := filepath.Join(a, "bash.exe") + if err := os.WriteFile(candidate, []byte("resolver fixture"), 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("CODEAF_BASH", "bash.exe") + changeDir(a) + if got, err := Bash(); err != nil || got != candidate { + t.Fatalf("relative bash in A: %q, %v", got, err) + } + changeDir(b) + if got, err := Bash(); got != "" || err == nil || err.Error() != fmt.Sprintf(MissingBash, "bash.exe") { + t.Fatalf("relative bash in B: %q, %v", got, err) + } + changeDir(a) + if got, err := Bash(); err != nil || got != candidate { + t.Fatalf("relative bash back in A: %q, %v", got, err) + } + // Absolute choices retain their cached resolution across directory changes. + t.Setenv("CODEAF_BASH", candidate) + if got, err := Bash(); err != nil || got != candidate { + t.Fatalf("absolute bash: %q, %v", got, err) + } + if err := os.Remove(candidate); err != nil { + t.Fatal(err) + } + changeDir(b) + if got, err := Bash(); err != nil || got != candidate { + t.Fatalf("absolute cache depended on cwd: %q, %v", got, err) + } +} diff --git a/internal/shell/doors_test.go b/internal/shell/doors_test.go new file mode 100644 index 0000000000..ae851d7149 --- /dev/null +++ b/internal/shell/doors_test.go @@ -0,0 +1,62 @@ +package shell + +import ( + "bytes" + "go/ast" + "go/format" + "go/parser" + "go/token" + "path/filepath" + "testing" +) + +// N3 pins the argv expressions at every Unix engine door. Together with the +// injected Unix preference tests, this does not require an installed /bin/bash. +func TestUnixDoorArgv(t *testing.T) { + for _, tc := range []struct{ file, function, call string }{ + {"exec/tools.go", "runShell", `exec.CommandContext(runCtx, bash, "-lc", command)`}, + {"exec/jobs.go", "startBackground", `exec.CommandContext(jobCtx, bash, "-lc", command)`}, + {"exec/services.go", "StartDetachedService", `exec.Command(bash, "-lc", command)`}, + {"exec/bare/tools.go", "getShellConfig", `shell.Preferred()`}, + {"exec/bare/tools.go", "newBashTool", `exec.Command(shell, shellArgs...)`}, + {"exec/bare/streaming.go", "StreamingShell", `append(append([]string{}, shellArgs...), command)`}, + {"session/jobs.go", "jobShell", `shell.Preferred()`}, + {"session/jobs.go", "start", `exec.Command(program, shellArgs...)`}, + {"session/tools_watch.go", "runTick", `exec.CommandContext(tickCtx, shell, append(shellArgs, spec.command)...)`}, + {"session/automation_run.go", "automationLookCommand", `exec.CommandContext(ctx, shell, append(args, command)...)`}, + {"session/principal_audit.go", "runOneCheck", `exec.CommandContext(ctx, bash, "-c", check)`}, + {"verify/run.go", "RunReading", `exec.CommandContext(ctx, bash, "-c", strictPreamble+strategy.Command)`}, + } { + t.Run(tc.file+"/"+tc.function, func(t *testing.T) { + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, filepath.Join("..", tc.file), nil, 0) + if err != nil { + t.Fatal(err) + } + found := false + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Name.Name != tc.function { + continue + } + ast.Inspect(fn.Body, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + var out bytes.Buffer + if err := format.Node(&out, fset, call); err != nil { + t.Fatal(err) + } + if out.String() == tc.call { + found = true + } + return true + }) + } + if !found { + t.Fatalf("Unix argv changed: %s must retain %s", tc.function, tc.call) + } + }) + } +} diff --git a/internal/shell/git_shim_test.go b/internal/shell/git_shim_test.go new file mode 100644 index 0000000000..bdf68cd8fb --- /dev/null +++ b/internal/shell/git_shim_test.go @@ -0,0 +1,75 @@ +package shell + +import ( + "errors" + "testing" +) + +// N1 follows a shim's real Git installation, preserves bin-before-usr/bin and +// WSL rejection, bounds the ancestor walk, and falls through on probe failure. +func TestWindowsGitShimResolution(t *testing.T) { + const shim = `D:\tools\shims\git.exe` + const root = `E:\nonstandard Git` + const bin = root + `\bin\bash.exe` + const usr = root + `\usr\bin\bash.exe` + const fallback = `F:\msys64\usr\bin\bash.exe` + for _, tc := range []struct { + name, execPath, want string + files []string + err error + }{ + {"shim root", "E:/nonstandard Git/mingw64/libexec/git-core\r\n", bin, []string{bin, usr}, nil}, + {"usr fallback", root + `\mingw64\libexec\git-core`, usr, []string{usr}, nil}, + {"first ancestor wins", root + `\mingw64\libexec\git-core`, root + `\mingw64\bin\bash.exe`, []string{root + `\mingw64\bin\bash.exe`, bin}, nil}, + {"four parents", root + `\a\b\c\d`, bin, []string{bin}, nil}, + {"five parents is too far", root + `\a\b\c\d\e`, fallback, []string{bin, fallback}, nil}, + {"nowhere useful", `E:\missing\libexec\git-core`, fallback, []string{fallback}, nil}, + {"probe failed or timed out", "", fallback, []string{bin, fallback}, errors.New("probe failed")}, + {"empty probe", " \r\n", fallback, []string{fallback}, nil}, + {"WSL alias root rejected", `C:\WindowsApps\Git\mingw64\libexec\git-core`, fallback, []string{`C:\WindowsApps\Git\bin\bash.exe`, fallback}, nil}, + } { + t.Run(tc.name, func(t *testing.T) { + lookup := fakeLookup(nil, map[string]string{"git.exe": shim, "bash.exe": fallback}, tc.files...) + calls := 0 + lookup.GitExecPath = func(git string) (string, error) { + calls++ + if git != shim { + t.Fatalf("probed %q, want the discovered shim %q", git, shim) + } + return tc.execPath, tc.err + } + if got, err := ResolveWindows(lookup); err != nil || got != tc.want || calls != 1 { + t.Fatalf("got %q, %v, calls %d; want %q once", got, err, calls, tc.want) + } + }) + } + // A failed shim probe still gives the ordered install roots their turn. + lookup := fakeLookup(map[string]string{"ProgramFiles": `E:`}, map[string]string{"git.exe": shim}, `E:\Git\bin\bash.exe`) + lookup.GitExecPath = func(string) (string, error) { return "", errors.New("timeout") } + if got, err := ResolveWindows(lookup); err != nil || got != `E:\Git\bin\bash.exe` { + t.Fatalf("standard root after failure: %q, %v", got, err) + } +} + +// N1 keeps known Git layouts free of process probes, even when their bash is +// absent and the resolver must continue through the ordinary fallback rules. +func TestWindowsKnownGitLayoutsNeverProbe(t *testing.T) { + for _, suffix := range []string{`cmd\git.exe`, `bin\git.exe`, `mingw64\bin\git.exe`} { + for _, installed := range []bool{false, true} { + const bash = `E:\Git\bin\bash.exe` + const fallback = `F:\msys64\usr\bin\bash.exe` + files, want := []string{fallback}, fallback + if installed { + files, want = append(files, bash), bash + } + lookup := fakeLookup(nil, map[string]string{"git.exe": `E:\Git\` + suffix, "bash.exe": fallback}, files...) + lookup.GitExecPath = func(string) (string, error) { + t.Fatal("known layout spawned a Git probe") + return "", nil + } + if got, err := ResolveWindows(lookup); err != nil || got != want { + t.Fatalf("%s installed=%v: %q, %v; want %q", suffix, installed, got, err, want) + } + } + } +} diff --git a/internal/shell/listpath_test.go b/internal/shell/listpath_test.go new file mode 100644 index 0000000000..81ef2924d5 --- /dev/null +++ b/internal/shell/listpath_test.go @@ -0,0 +1,51 @@ +package shell + +import "testing" + +// N2 and N3 ask Windows to bind relative names before converting them, preserving +// drive-relative, root-relative and ordinary relative path semantics. +func TestWindowsListPathBindsRelativeNames(t *testing.T) { + for _, tc := range []struct{ native, absolute, want string }{ + {`C:`, `C:\drive cwd`, `/c/drive cwd`}, + {`C:rel`, `C:\drive cwd\rel`, `/c/drive cwd/rel`}, + {`\x`, `D:\x`, `/d/x`}, + {`rel\x`, `D:\current\rel\x`, `/d/current/rel/x`}, + } { + calls := 0 + abs := func(name string) (string, error) { + calls++ + if name != tc.native { + t.Fatalf("absolutiser saw %q, want original %q", name, tc.native) + } + return tc.absolute, nil + } + if got := listPath("windows", tc.native, abs); got != tc.want || calls != 1 { + t.Fatalf("%q => %q, calls %d; want %q once", tc.native, got, calls, tc.want) + } + } +} + +// N2 and N3 leave absolute Windows, UNC, POSIX, empty and every Unix path independent +// of the current directory, so they never invoke the injected absolutiser. +func TestListPathOnlyBindsRelativeWindowsNames(t *testing.T) { + abs := func(name string) (string, error) { + t.Fatalf("unexpected absolutiser call for %q", name) + return "", nil + } + for _, tc := range []struct{ native, want string }{ + {`C:\tools\bin`, `/c/tools/bin`}, {`D:/tools/bin`, `/d/tools/bin`}, + {`\\server\share\bin`, `//server/share/bin`}, {`//server/share/bin`, `//server/share/bin`}, + {`/tools/bin`, `/tools/bin`}, {"", ""}, + } { + if got := listPath("windows", tc.native, abs); got != tc.want { + t.Fatalf("%q => %q, want %q", tc.native, got, tc.want) + } + } + for _, goos := range []string{"linux", "darwin"} { + for _, name := range []string{`C:`, `C:rel`, `\x`, `rel\x`, `C:\tools\bin`, `/usr/bin`, ""} { + if got := listPath(goos, name, abs); got != name { + t.Fatalf("%s changed %q to %q", goos, name, got) + } + } + } +} diff --git a/internal/shell/listpath_windows_test.go b/internal/shell/listpath_windows_test.go new file mode 100644 index 0000000000..b65dc6d524 --- /dev/null +++ b/internal/shell/listpath_windows_test.go @@ -0,0 +1,42 @@ +//go:build windows + +package shell + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// N2 and N3 reach the real ListPath with Windows' own absolutiser, including the +// current drive's separate directory for C:rel and its root for \tools\bin. +func TestWindowsListPathUsesNativeAbsoluteRules(t *testing.T) { + cwd := t.TempDir() + previous, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := os.Chdir(previous); err != nil { + t.Error(err) + } + }) + if err := os.Chdir(cwd); err != nil { + t.Fatal(err) + } + drive := filepath.VolumeName(cwd) + for _, native := range []string{drive, drive + `skills\bin`, `\tools\bin`, `rel\x`} { + absolute, err := filepath.Abs(native) + if err != nil { + t.Fatal(err) + } + if !filepath.IsAbs(absolute) || len(absolute) < 3 || absolute[1] != ':' { + t.Fatalf("test requires a drive-based temporary directory: %q", absolute) + } + want := "/" + strings.ToLower(absolute[:1]) + filepath.ToSlash(absolute[2:]) + if got := ListPath(native); got != want { + t.Errorf("%q => %q, want %q from native absolute %q", native, got, want, absolute) + } + } +} diff --git a/internal/shell/log.go b/internal/shell/log.go new file mode 100644 index 0000000000..477d8db1c8 --- /dev/null +++ b/internal/shell/log.go @@ -0,0 +1,36 @@ +package shell + +import ( + "io" + "os" + "runtime" +) + +// OpenLog opens a shell child's output at the end of its existing log. On the +// native Windows host on 2026-10-19, Git Bash exited 1 without writing a byte +// when stdout or stderr was Go's append-only Windows handle. A writable handle +// sought to the end worked; Unix keeps its original append flags and permissions. +// +// ONE CHILD OWNS THIS OFFSET ON WINDOWS. A separate appender cannot advance +// this handle's position, so callers must write their headers before Start and +// their trailers after Wait, or carry all writes through one synchronized sink. +func OpenLog(path string, mode os.FileMode) (*os.File, error) { + file, err := os.OpenFile(path, logFlags(runtime.GOOS), mode) + if err != nil { + return nil, err + } + if runtime.GOOS == "windows" { + if _, err := file.Seek(0, io.SeekEnd); err != nil { + _ = file.Close() + return nil, err + } + } + return file, nil +} + +func logFlags(goos string) int { + if goos == "windows" { + return os.O_CREATE | os.O_WRONLY + } + return os.O_CREATE | os.O_WRONLY | os.O_APPEND +} diff --git a/internal/shell/log_test.go b/internal/shell/log_test.go new file mode 100644 index 0000000000..545ce6b04a --- /dev/null +++ b/internal/shell/log_test.go @@ -0,0 +1,41 @@ +package shell + +import ( + "os" + "path/filepath" + "testing" +) + +// N3 pins the existing Unix open flags independently of the host's platform. +func TestUnixLogFlags(t *testing.T) { + for _, goos := range []string{"linux", "darwin", "freebsd"} { + if got, want := logFlags(goos), os.O_CREATE|os.O_WRONLY|os.O_APPEND; got != want { + t.Fatalf("%s flags = %d, want %d", goos, got, want) + } + } + if got, want := logFlags("windows"), os.O_CREATE|os.O_WRONLY; got != want { + t.Fatalf("Windows flags = %d, want %d", got, want) + } +} + +// N2 keeps earlier output when a command reopens a log on either platform. +func TestOpenLogKeepsExistingOutput(t *testing.T) { + path := filepath.Join(t.TempDir(), "command.log") + if err := os.WriteFile(path, []byte("earlier\n"), 0o600); err != nil { + t.Fatal(err) + } + file, err := OpenLog(path, 0o600) + if err != nil { + t.Fatal(err) + } + if _, err := file.WriteString("later\n"); err != nil { + _ = file.Close() + t.Fatal(err) + } + if err := file.Close(); err != nil { + t.Fatal(err) + } + if got, err := os.ReadFile(path); err != nil || string(got) != "earlier\nlater\n" { + t.Fatalf("log = %q, %v", got, err) + } +} diff --git a/internal/shell/native_windows_test.go b/internal/shell/native_windows_test.go new file mode 100644 index 0000000000..8ca32c5096 --- /dev/null +++ b/internal/shell/native_windows_test.go @@ -0,0 +1,90 @@ +//go:build windows + +package shell + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/env" +) + +// N2 proves a real Windows process hands Git Bash one parse, including multiline +// text and every character that the WSL launcher used to expand prematurely. +func TestWindowsSingleParse(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + bash, err := Bash() + if err != nil { + t.Skipf("Git Bash is required for native shell tests: %v", err) + } + literal := "double=\" single=' backslash=\\ percent=%PATH% substitution=$(echo WRONG) héllo ✓" + command := "export PATH='/c/Program Files (x86)/test':$PATH\nx=inner; echo \"x=[$x]\"\nprintf 'second line\\n'\ncat <<'CODEAF_LITERAL'\n" + literal + "\nCODEAF_LITERAL\n" + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, bash, "-lc", command) + cmd.Dir = t.TempDir() + got, err := cmd.CombinedOutput() + if err != nil || string(got) != "x=[inner]\nsecond line\n"+literal+"\n" { + t.Fatalf("got %q, %v", got, err) + } +} + +// N1 proves an explicit bad choice refuses and changing the override +// invalidates the cached answer, rather than silently falling through to PATH. +func TestWindowsOverrideCacheAndRefusal(t *testing.T) { + bash, err := Bash() + if err != nil { + t.Skipf("Git Bash is required for cache reset proof: %v", err) + } + t.Setenv(env.Legacy("CODEAF_BASH"), bash) + t.Setenv("CODEAF_BASH", "") + if got, err := Bash(); got != bash || err != nil { + t.Fatalf("owned-variable fallback: %q %v", got, err) + } + t.Setenv("CODEAF_BASH", bash) + if got, err := Bash(); err != nil || got != bash { + t.Fatalf("explicit bash: %q %v", got, err) + } + missing := filepath.Join(t.TempDir(), "missing bash.exe") + t.Setenv("CODEAF_BASH", missing) + if got, err := Bash(); got != "" || err == nil || !strings.Contains(err.Error(), missing) || !strings.Contains(err.Error(), "CODEAF_BASH") { + t.Fatalf("missing override: %q %v", got, err) + } + t.Setenv("CODEAF_BASH", filepath.Join(os.Getenv("SystemRoot"), "System32", "bash.exe")) + if got, err := Bash(); got != "" || err == nil || !strings.Contains(err.Error(), "CODEAF_BASH") { + t.Fatalf("WSL override: %q %v", got, err) + } + t.Setenv("CODEAF_BASH", bash) + if got, err := Bash(); err != nil || got != bash { + t.Fatalf("restored bash: %q %v", got, err) + } + // R4 keeps both route remedies in the Windows-only shell fact. + if fact := PromptFact(); !strings.Contains(fact, bash) || !strings.Contains(fact, "/c/…") || !strings.HasSuffix(fact, PathConversionFact) { + t.Fatalf("C8 shell fact: %q", fact) + } +} + +// N2 keeps cwd, long output and exit status through the real native bash door. +func TestWindowsCwdLongOutputAndExit(t *testing.T) { + bash, err := Bash() + if err != nil { + t.Fatal(err) + } + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "marker"), []byte("cwd is right"), 0600); err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, bash, "-c", "cat marker; printf '\\n'; for ((n=0;n<12000;n++)); do printf 'long output %05d\\n' \"$n\"; done; exit 23") + cmd.Dir = dir + body, err := cmd.CombinedOutput() + if cmd.ProcessState == nil || cmd.ProcessState.ExitCode() != 23 || err == nil || !strings.HasPrefix(string(body), "cwd is right\n") || !strings.HasSuffix(string(body), "long output 11999\n") || strings.Count(string(body), "long output ") != 12000 { + t.Fatalf("exit/cwd/output: bytes=%d, state=%v, err=%v", len(body), cmd.ProcessState, err) + } +} diff --git a/internal/shell/shell.go b/internal/shell/shell.go new file mode 100644 index 0000000000..49df2cb31d --- /dev/null +++ b/internal/shell/shell.go @@ -0,0 +1,336 @@ +// Package shell chooses a bash that shares the engine's filesystem and OS. +// +// On 2026-10-19 the native Windows desktop resolved bash to +// C:\Windows\System32\bash.exe, WSL's launcher. It parsed the command line a +// second time in Linux: $PATH in the plandb prefix expanded to WSL's PATH, +// whose Program Files (x86) entry made every task command fail with "syntax +// error near unexpected token '('". A chat's for n in …; do gh pr view $n; done +// also reached gh with an empty $n. Choosing a real bash fixes both failures +// and keeps commands beside the files the engine reads and writes. +// +// NEVER THE WSL LAUNCHER. A PATH THAT BASH READS IS IN BASH'S FORM, A VALUE A +// WINDOWS PROGRAM READS KEEPS ITS OWN. UNIX ARGV IS UNTOUCHED. +package shell + +import ( + "context" + "errors" + "fmt" + "os" + "os/exec" + "path" + "path/filepath" + "runtime" + "strings" + "sync" + "time" + + "github.com/Agent-Field/codeaf/internal/env" +) + +// NoBash is shared by every Windows command door so both remedies stay visible. +const NoBash = "codeaf runs commands in Git for Windows' bash, and none was found: install Git for Windows, or set CODEAF_BASH to a bash.exe; WSL's bash.exe is not used." + +// MissingBash and WSLBash distinguish an explicit choice that cannot work from +// discovery finding nothing, so the remedy never contradicts the person's choice. +const MissingBash = "CODEAF_BASH names %s, which does not exist. Set it to a Git for Windows or MSYS2 bash.exe, or unset it and codeaf will look for Git Bash." +const WSLBash = "CODEAF_BASH names %s, which is WSL's launcher and runs commands in a different system. Set it to a Git for Windows or MSYS2 bash.exe, or unset it and codeaf will look for Git Bash." + +// PathConversionFact keeps the command advice beside the selected shell fact; +// native programs receive Git Bash's usual argument conversion unless told otherwise. +const PathConversionFact = "- Git Bash rewrites arguments starting with / into Windows paths for Windows programs: write API routes without the leading slash (gh api repos/…), or prefix that command with MSYS_NO_PATHCONV=1.\n" + +// Lookup makes resolution independent of the host so Windows rules can be proved +// without starting a process or depending on this machine's installed programs. +type Lookup struct { + Env func(string) string + Exists func(string) bool + LookPath func(string) (string, error) + // GitExecPath follows a package-manager shim only when its location says + // nothing about the installation. Failures leave the remaining rules usable. + GitExecPath func(git string) (string, error) + // Abs binds an explicit relative file to this process's filesystem instead of + // letting exec search PATH again. Pure fixtures may omit it for absolute paths. + Abs func(string) (string, error) +} + +// windowsClean uses Windows separators even when the resolver is tested on Unix. +// Cleaning with slash semantics first also removes dot components on either OS. +func windowsClean(name string) string { + slash := strings.ReplaceAll(name, `\`, "/") + volume := "" + switch { + case len(slash) >= 2 && slash[1] == ':': + volume, slash = slash[:2], slash[2:] + case strings.HasPrefix(slash, "//"): + // A UNC server and share are a volume: parent components cannot escape it. + if serverEnd := strings.IndexByte(slash[2:], '/'); serverEnd >= 0 { + shareStart := serverEnd + 3 + shareEnd := len(slash) + if end := strings.IndexByte(slash[shareStart:], '/'); end >= 0 { + shareEnd = shareStart + end + } + volume, slash = slash[:shareEnd], slash[shareEnd:] + if slash == "" { + return strings.ReplaceAll(volume, "/", `\`) + } + } + } + return strings.ReplaceAll(volume+path.Clean(slash), "/", `\`) +} + +// IsWSL rejects the system launchers and application aliases: neither executes +// commands in the same OS as the engine's native file tools. +func IsWSL(name, systemRoot string) bool { + name = strings.ToLower(windowsClean(name)) + if strings.Contains(`\`+name, `\windowsapps\`) { + return true + } + root := strings.ToLower(windowsClean(systemRoot)) + if systemRoot == "" { + // A stripped environment cannot turn the ordinary Windows launchers + // into native shells merely by omitting SystemRoot. + for _, dir := range []string{"system32", "syswow64", "sysnative"} { + if strings.HasSuffix(name, `\windows\`+dir+`\bash.exe`) { + return true + } + } + return false + } + for _, dir := range []string{"system32", "syswow64", "sysnative"} { + if name == root+`\`+dir+`\bash.exe` { + return true + } + } + return false +} + +// ResolveWindows follows the explicit choice, Git installation, standard roots, +// then PATH order. An unusable explicit choice is a refusal, never a suggestion. +func ResolveWindows(l Lookup) (string, error) { + root := l.Env("SystemRoot") + usable := func(name string) bool { return !IsWSL(name, root) && l.Exists(name) } + if explicit := l.Env("CODEAF_BASH"); explicit != "" { + candidate := explicit + var err error + if l.Abs != nil { + candidate, err = l.Abs(explicit) + } + if IsWSL(candidate, root) { + return "", fmt.Errorf(WSLBash, explicit) + } + if err != nil || !l.Exists(candidate) { + return "", fmt.Errorf(MissingBash, explicit) + } + return candidate, nil + } + gitBash := func(root string) string { + for _, sub := range []string{`bin\bash.exe`, `usr\bin\bash.exe`} { + candidate := windowsClean(root + `\` + sub) + if usable(candidate) { + return candidate + } + } + return "" + } + if git, err := l.LookPath("git.exe"); err == nil { + git = windowsClean(git) + lower := strings.ToLower(git) + knownLayout := false + for _, suffix := range []string{`\mingw64\bin\git.exe`, `\cmd\git.exe`, `\bin\git.exe`} { + if strings.HasSuffix(lower, suffix) { + knownLayout = true + if bash := gitBash(git[:len(git)-len(suffix)]); bash != "" { + return bash, nil + } + break + } + } + if !knownLayout && l.GitExecPath != nil { + if execPath, err := l.GitExecPath(git); err == nil && strings.TrimSpace(execPath) != "" { + root := windowsClean(strings.TrimSpace(execPath)) + // Git's usual mingw64/libexec/git-core is three parents below + // its root. Bound the search so an unrelated shim stays cheap. + for level := 0; level <= 4; level++ { + if bash := gitBash(root); bash != "" { + return bash, nil + } + parent := windowsClean(root + `\..`) + if parent == root { + break + } + root = parent + } + } + } + } + for _, install := range []struct{ env, sub string }{ + {"ProgramW6432", "Git"}, {"ProgramFiles", "Git"}, {"ProgramFiles(x86)", "Git"}, + {"LOCALAPPDATA", `Programs\Git`}, {"USERPROFILE", `scoop\apps\git\current`}, + } { + if base := l.Env(install.env); base != "" { + if bash := gitBash(base + `\` + install.sub); bash != "" { + return bash, nil + } + } + } + if bash, err := l.LookPath("bash.exe"); err == nil && usable(bash) { + return bash, nil + } + // A WSL launcher early on PATH must not hide a native bash later on it. + // Split with Windows semantics even in pure Unix-host resolver tests. + for _, dir := range strings.Split(l.Env("PATH"), ";") { + dir = strings.Trim(dir, "\"") + if dir == "" || dir == "." { + continue + } + candidate := windowsClean(dir + `\bash.exe`) + if l.Abs != nil { + absolute, err := l.Abs(candidate) + if err != nil { + continue + } + candidate = absolute + } + if usable(candidate) { + return candidate, nil + } + } + return "", errors.New(NoBash) +} + +var bashCache struct { + sync.Mutex + key string + ready bool + bash string + err error +} + +// Bash preserves the literal Unix argv[0]. Windows discovery is cached by its +// inputs, so changing an override in a test resets the answer without repeated +// filesystem probes on every command in a running session. +func Bash() (string, error) { + if runtime.GOOS != "windows" { + return "bash", nil + } + names := []string{"CODEAF_BASH", "PATH", "PATHEXT", "SystemRoot", "ProgramW6432", "ProgramFiles", "ProgramFiles(x86)", "LOCALAPPDATA", "USERPROFILE"} + values := make(map[string]string, len(names)) + var key strings.Builder + for _, name := range names { + values[name] = env.Value(name) + key.WriteString(values[name]) + key.WriteByte(0) + } + // A relative override belongs to this directory. An absolute choice never + // depends on cwd and must not repeat discovery when the engine changes it. + if explicit := values["CODEAF_BASH"]; explicit != "" && !filepath.IsAbs(explicit) { + cwd, err := os.Getwd() + key.WriteString(cwd) + if err != nil { + key.WriteString(err.Error()) + } + key.WriteByte(0) + } + bashCache.Lock() + defer bashCache.Unlock() + if !bashCache.ready || bashCache.key != key.String() { + bashCache.bash, bashCache.err = ResolveWindows(Lookup{ + Env: func(name string) string { return values[name] }, + Exists: func(name string) bool { info, err := os.Stat(name); return err == nil && info.Mode().IsRegular() }, + LookPath: exec.LookPath, + Abs: filepath.Abs, + GitExecPath: func(git string) (string, error) { + // The result lives in the resolution cache, not in the command + // path. A broken shim cannot hold up the first command forever. + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, git, "--exec-path") + cmd.WaitDelay = 100 * time.Millisecond + output, err := cmd.Output() + return strings.TrimSpace(string(output)), err + }, + }) + bashCache.key, bashCache.ready = key.String(), true + } + return bashCache.bash, bashCache.err +} + +// Preferred preserves the Unix /bin/bash, PATH bash, sh preference used by the +// streaming and short-job doors, while Windows always uses the shared resolver. +func Preferred() (string, []string, error) { + if runtime.GOOS == "windows" { + bash, err := Bash() + return bash, []string{"-c"}, err + } + return preferred(Lookup{ + Exists: func(name string) bool { _, err := os.Stat(name); return err == nil }, + LookPath: exec.LookPath, + }) +} + +func preferred(l Lookup) (string, []string, error) { + if l.Exists("/bin/bash") { + return "/bin/bash", []string{"-c"}, nil + } + if bash, err := l.LookPath("bash"); err == nil { + return bash, []string{"-c"}, nil + } + return "sh", []string{"-c"}, nil +} + +// Reading preserves verify's Unix PATH lookup, including its absence semantics. +func Reading() (string, error) { + if runtime.GOOS == "windows" { + return Bash() + } + return exec.LookPath("bash") +} + +// ListPath converts only paths bash interprets itself. Values passed to native +// programs, such as PLANDB_DB, must keep their native Windows spelling. +func ListPath(native string) string { return listPath(runtime.GOOS, native, filepath.Abs) } + +func listPath(goos, native string, abs func(string) (string, error)) string { + if goos != "windows" { + return native + } + if native == "" || strings.HasPrefix(native, "/") { + return native + } + slash := strings.ReplaceAll(native, `\`, "/") + // Drive-relative and root-relative names need Windows' own drive/cwd rules + // before MSYS conversion; replacing separators alone sends bash elsewhere. + if !strings.HasPrefix(slash, "//") && !(len(slash) >= 3 && slash[1] == ':' && slash[2] == '/') { + absolute, err := abs(native) + if err != nil { + return native + } + slash = strings.ReplaceAll(absolute, `\`, "/") + } + if len(slash) >= 3 && slash[1] == ':' && slash[2] == '/' && ((slash[0] >= 'A' && slash[0] <= 'Z') || (slash[0] >= 'a' && slash[0] <= 'z')) { + return "/" + strings.ToLower(slash[:1]) + slash[2:] + } + return slash +} + +// PathExport leaves Unix prefix bytes intact and puts a converted literal in a +// Windows PATH list. The native shelf variable remains available to programs. +func PathExport(variable, native string) string { + if runtime.GOOS != "windows" { + return "export PATH=\"${" + variable + ":?}:$PATH\"\n" + } + return "export PATH='" + strings.ReplaceAll(ListPath(native), "'", "'\\''") + "':$PATH\n" +} + +// PromptFact is absent on Unix so existing prompt bytes stay unchanged. +func PromptFact() string { + if runtime.GOOS != "windows" { + return "" + } + bash, err := Bash() + if err != nil { + return "- Shell commands are unavailable; there is no shell tool in this call. " + err.Error() + "\n" + } + return "- Shell: Git Bash (" + bash + "); Windows drives are /c/…, /d/…\n" + PathConversionFact +} diff --git a/internal/shell/shell_test.go b/internal/shell/shell_test.go new file mode 100644 index 0000000000..d47810ccd1 --- /dev/null +++ b/internal/shell/shell_test.go @@ -0,0 +1,226 @@ +package shell + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "reflect" + "runtime" + "strings" + "testing" +) + +// N1 distinguishes the two explicit-choice refusals without claiming discovery +// failed, and preserves the person's path exactly in either answer. +func TestWindowsOverrideRefusalSentences(t *testing.T) { + for _, tc := range []struct{ path, format string }{ + {`C:\missing é\bash.exe`, MissingBash}, + {`C:\Windows\System32\bash.exe`, WSLBash}, + } { + lookup := fakeLookup(map[string]string{"CODEAF_BASH": tc.path, "SystemRoot": `C:\Windows`}, nil) + if got, err := ResolveWindows(lookup); got != "" || err == nil || err.Error() != fmt.Sprintf(tc.format, tc.path) { + t.Fatalf("override %q: %q, %v", tc.path, got, err) + } + } +} + +func fakeLookup(environment, programs map[string]string, files ...string) Lookup { + return Lookup{ + Env: func(name string) string { return environment[name] }, + Exists: func(name string) bool { + for _, file := range files { + if strings.EqualFold(windowsClean(name), windowsClean(file)) { + return true + } + } + return false + }, + LookPath: func(name string) (string, error) { + if found := programs[name]; found != "" { + return found, nil + } + return "", errors.New("not found") + }, + } +} + +// N1 proves every resolution tier without depending on a Windows host. +func TestWindowsResolutionRules(t *testing.T) { + const git = `C:\Program Files\Git` + const bin = git + `\bin\bash.exe` + const usr = git + `\usr\bin\bash.exe` + const other = `D:\msys64\usr\bin\bash.exe` + const wsl = `C:\Windows\System32\bash.exe` + tests := []struct { + name string + env, programs map[string]string + files []string + want, refusal string + }{ + {name: "explicit wins", env: map[string]string{"CODEAF_BASH": other, "ProgramFiles": `C:\Program Files`}, programs: map[string]string{"git.exe": git + `\cmd\git.exe`, "bash.exe": wsl}, files: []string{other, bin, wsl}, want: other}, + {name: "missing explicit refuses", env: map[string]string{"CODEAF_BASH": other}, programs: map[string]string{"bash.exe": bin}, files: []string{bin}, refusal: other}, + {name: "WSL explicit refuses", env: map[string]string{"CODEAF_BASH": wsl, "SystemRoot": `C:\Windows`}, programs: map[string]string{"git.exe": git + `\cmd\git.exe`}, files: []string{bin, wsl}, refusal: wsl}, + {name: "git cmd", programs: map[string]string{"git.exe": git + `\cmd\git.exe`, "bash.exe": other}, files: []string{bin, usr, other}, want: bin}, + {name: "git bin", programs: map[string]string{"git.exe": git + `\bin\git.exe`}, files: []string{bin}, want: bin}, + {name: "git mingw64", programs: map[string]string{"git.exe": git + `\mingw64\bin\git.exe`}, files: []string{bin}, want: bin}, + {name: "git mixed slash case", programs: map[string]string{"git.exe": `C:/Program Files/Git/MINGW64/bin/git.exe`}, files: []string{bin}, want: bin}, + {name: "git usr fallback", programs: map[string]string{"git.exe": git + `\cmd\git.exe`}, files: []string{usr}, want: usr}, + {name: "git beats roots", env: map[string]string{"ProgramW6432": `D:\Programs`}, programs: map[string]string{"git.exe": git + `\cmd\git.exe`}, files: []string{bin, `D:\Programs\Git\bin\bash.exe`}, want: bin}, + {name: "missing git bash falls through", env: map[string]string{"ProgramFiles": `C:\Program Files`}, programs: map[string]string{"git.exe": `D:\Git\cmd\git.exe`}, files: []string{bin}, want: bin}, + {name: "unrelated git path is not a root", programs: map[string]string{"git.exe": `D:\tools\git.exe`, "bash.exe": other}, files: []string{`D:\bin\bash.exe`, other}, want: other}, + {name: "root beats PATH WSL", env: map[string]string{"SystemRoot": `C:\Windows`, "ProgramFiles": `C:\Program Files`}, programs: map[string]string{"bash.exe": wsl}, files: []string{bin, wsl}, want: bin}, + {name: "PATH MSYS", programs: map[string]string{"bash.exe": other}, files: []string{other}, want: other}, + {name: "PATH WSL refuses", env: map[string]string{"SystemRoot": `C:\Windows`}, programs: map[string]string{"bash.exe": wsl}, files: []string{wsl}, refusal: NoBash}, + {name: "PATH alias refuses", programs: map[string]string{"bash.exe": `C:\Users\x\AppData\Local\WindowsApps\bash.exe`}, files: []string{`C:\Users\x\AppData\Local\WindowsApps\bash.exe`}, refusal: NoBash}, + {name: "missing PATH file", programs: map[string]string{"bash.exe": other}, refusal: NoBash}, + {name: "native PATH bash after WSL", env: map[string]string{"SystemRoot": `C:\Windows`, "PATH": `C:\Windows\System32;D:\msys64\usr\bin`}, programs: map[string]string{"bash.exe": wsl}, files: []string{wsl, other}, want: other}, + {name: "quoted native PATH after WSL", env: map[string]string{"SystemRoot": `C:\Windows`, "PATH": `C:\Windows\System32;"C:\Program Files\Git\bin"`}, programs: map[string]string{"bash.exe": wsl}, files: []string{wsl, bin}, want: bin}, + {name: "PATH WSL without SystemRoot refuses", programs: map[string]string{"bash.exe": wsl}, files: []string{wsl}, refusal: NoBash}, + {name: "explicit WSL without SystemRoot refuses", env: map[string]string{"CODEAF_BASH": wsl}, files: []string{wsl}, refusal: wsl}, + {name: "none", refusal: NoBash}, + } + // C3 also proves the complete standard-root ordering, including usr fallback. + roots := []struct{ env, base, sub string }{ + {"ProgramW6432", `C:\W6432`, "Git"}, {"ProgramFiles", `C:\PF`, "Git"}, {"ProgramFiles(x86)", `C:\PF86`, "Git"}, + {"LOCALAPPDATA", `C:\Local`, `Programs\Git`}, {"USERPROFILE", `C:\Users\me`, `scoop\apps\git\current`}, + } + for i, root := range roots { + environment := map[string]string{} + files := []string{other} + for _, r := range roots[i:] { + environment[r.env] = r.base + files = append(files, r.base+`\`+r.sub+`\usr\bin\bash.exe`, r.base+`\`+r.sub+`\bin\bash.exe`) + } + tests = append(tests, struct { + name string + env, programs map[string]string + files []string + want, refusal string + }{name: root.env, env: environment, programs: map[string]string{"bash.exe": other}, files: files, want: root.base + `\` + root.sub + `\bin\bash.exe`}) + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := ResolveWindows(fakeLookup(tc.env, tc.programs, tc.files...)) + if tc.refusal != "" { + if got != "" || err == nil || !strings.Contains(err.Error(), tc.refusal) { + t.Fatalf("got %q, %v; want refusal %q", got, err, tc.refusal) + } + if tc.refusal == NoBash && err.Error() != NoBash { + t.Fatalf("unshared refusal: %v", err) + } + if tc.env["CODEAF_BASH"] != "" && !strings.Contains(err.Error(), "CODEAF_BASH") { + t.Fatal(err) + } + } else if err != nil || !strings.EqualFold(got, tc.want) { + t.Fatalf("got %q, %v; want %q", got, err, tc.want) + } + }) + } +} + +// N1 covers cleaned, case-insensitive system launcher and WindowsApps paths. +func TestWindowsWSLPredicate(t *testing.T) { + for _, name := range []string{ + `C:\Windows\System32\bash.exe`, `C:\WINDOWS\SysWOW64\BASH.EXE`, `c:/windows/Sysnative/bash.exe/`, + `C:/Windows/temp/../System32/./bash.exe`, `C:/../Windows/System32/bash.exe`, `\\server\WindowsApps\..\bash.exe`, `C:\Users\x\WindowsApps\bash.exe`, `D:/WINDOWSAPPS/alias/bash.exe`, `WindowsApps/bash.exe`, + } { + if !IsWSL(name, `c:\Windows\`) { + t.Errorf("accepted WSL %q", name) + } + } + for _, name := range []string{`C:\Git\bin\bash.exe`, `C:\Windows\System32\Git\bash.exe`, `D:\Windows\System32\bash.exe`, `C:\WindowsAppsExtra\bash.exe`, `/usr/bin/bash`} { + if IsWSL(name, `C:\Windows`) { + t.Errorf("rejected bash %q", name) + } + } +} + +// N2 and N3 pin drive, UNC and POSIX forms without spawning a converter. +func TestWindowsListPath(t *testing.T) { + for _, tc := range []struct{ native, want string }{ + {`C:\a\b`, `/c/a/b`}, {`D:/a/b`, `/d/a/b`}, {`z:\Program Files (x86)\héllo ✓`, `/z/Program Files (x86)/héllo ✓`}, + {`\\server\share\x`, `//server/share/x`}, {`/c/a/b`, `/c/a/b`}, {`//server/share/x`, `//server/share/x`}, {`/usr/bin`, `/usr/bin`}, {"", ""}, + } { + if got := listPath("windows", tc.native, filepath.Abs); got != tc.want { + t.Errorf("%q => %q, want %q", tc.native, got, tc.want) + } + for _, goos := range []string{"linux", "darwin"} { + if got := listPath(goos, tc.native, filepath.Abs); got != tc.native { + t.Errorf("%s changed %q to %q", goos, tc.native, got) + } + } + } +} + +// N3 pins all Unix preferences, including missing /bin/bash, with fake lookups. +func TestUnixShellArgv(t *testing.T) { + for _, tc := range []struct { + absolute, onPath bool + want string + }{ + {true, true, "/bin/bash"}, {true, false, "/bin/bash"}, {false, true, "/fake/bin/bash"}, {false, false, "sh"}, + } { + programs := map[string]string{} + if tc.onPath { + programs["bash"] = "/fake/bin/bash" + } + var files []string + if tc.absolute { + files = []string{"/bin/bash"} + } + got, args, err := preferred(fakeLookup(nil, programs, files...)) + if got != tc.want || !reflect.DeepEqual(args, []string{"-c"}) || err != nil { + t.Fatalf("got %q %q %v", got, args, err) + } + } + if runtime.GOOS == "windows" { + return + } + if got, err := Bash(); got != "bash" || err != nil { + t.Fatalf("literal Unix bash changed: %q %v", got, err) + } + if fact := PromptFact(); fact != "" { + t.Fatalf("Unix footer changed: %q", fact) + } + if got := PathExport("CODEAF_SKILLS_BIN", `C:\shelf`); got != "export PATH=\"${CODEAF_SKILLS_BIN:?}:$PATH\"\n" { + t.Fatalf("Unix shelf changed: %q", got) + } +} + +// N3 proves verify still resolves bash from PATH, rather than /bin/bash or sh. +func TestUnixReadingUsesPathLookup(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Unix lookup contract") + } + dir := t.TempDir() + bash := filepath.Join(dir, "bash") + if err := os.WriteFile(bash, []byte("unused"), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", dir) + if got, err := Reading(); got != bash || err != nil { + t.Fatalf("reading shell: %q %v", got, err) + } + if err := os.Remove(bash); err != nil { + t.Fatal(err) + } + if got, err := Reading(); got != "" || err == nil { + t.Fatalf("missing bash fell through: %q %v", got, err) + } +} + +// N1 proves an explicit relative file cannot be re-searched on PATH and reach +// WSL, and that an explicit relative path into a system folder is also refused. +func TestWindowsExplicitRelativePath(t *testing.T) { + const candidate = `C:\work\bash.exe` + lookup := fakeLookup(map[string]string{"CODEAF_BASH": "bash.exe", "SystemRoot": `C:\Windows`}, map[string]string{"bash.exe": `C:\Windows\System32\bash.exe`}, candidate, `C:\Windows\System32\bash.exe`) + lookup.Abs = func(name string) (string, error) { return candidate, nil } + if got, err := ResolveWindows(lookup); got != candidate || err != nil { + t.Fatalf("explicit relative file: %q %v", got, err) + } + lookup.Abs = func(name string) (string, error) { return `C:\Windows\System32\bash.exe`, nil } + if got, err := ResolveWindows(lookup); got != "" || err == nil || err.Error() != fmt.Sprintf(WSLBash, "bash.exe") { + t.Fatalf("relative WSL: %q %v", got, err) + } +} diff --git a/internal/store/store_windows_test.go b/internal/store/store_windows_test.go new file mode 100644 index 0000000000..ac0c5adf80 --- /dev/null +++ b/internal/store/store_windows_test.go @@ -0,0 +1,36 @@ +//go:build windows + +package store + +import ( + "path/filepath" + "testing" +) + +// N2 opens the real history store at a native path with spaces and non-ASCII, +// writes a graph row, and reads it after reopening the same database. +func TestWindowsStoreNativePath(t *testing.T) { + path := filepath.Join(t.TempDir(), "dir with space é", "graph #%25.db") + s, err := Open(path) + if err != nil { + t.Fatal(err) + } + defer func() { + if s != nil { + _ = s.Close() + } + }() + if err := s.Splice(RootID, Subtree{Nodes: []NodeSpec{{ID: "windows", Brief: "héllo"}}}, Provenance{Origin: OriginUser, Intent: "héllo"}); err != nil { + t.Fatal(err) + } + if err := s.Close(); err != nil { + t.Fatal(err) + } + s, err = Open(path) + if err != nil { + t.Fatal(err) + } + if node, ok, err := s.Node("windows"); err != nil || !ok || node.Brief != "héllo" { + t.Fatalf("node = %+v, %v, %v", node, ok, err) + } +} diff --git a/internal/verify/run.go b/internal/verify/run.go index 326f8870a9..b4f30f3372 100644 --- a/internal/verify/run.go +++ b/internal/verify/run.go @@ -18,6 +18,7 @@ import ( "time" "github.com/Agent-Field/codeaf/internal/processgroup" + "github.com/Agent-Field/codeaf/internal/shell" ) // strictPreamble is prefixed to every command this package runs. A pipeline @@ -134,7 +135,7 @@ func RunReading( if strategy.Empty() { return Result{}, false } - shell, err := exec.LookPath(readingShell) + bash, err := shell.Reading() if err != nil { return Result{}, false } @@ -142,7 +143,7 @@ func RunReading( ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() - command := exec.CommandContext(ctx, shell, "-c", strictPreamble+strategy.Command) + command := exec.CommandContext(ctx, bash, "-c", strictPreamble+strategy.Command) command.Dir = filepath.Join(workspace, strategy.Workdir) // A suite spawns children — a test server, a browser, a compiler — and // killing only the shell leaves them holding the pipe this reading is being diff --git a/internal/verify/run_windows_test.go b/internal/verify/run_windows_test.go new file mode 100644 index 0000000000..371e93e703 --- /dev/null +++ b/internal/verify/run_windows_test.go @@ -0,0 +1,34 @@ +//go:build windows + +package verify + +import ( + "context" + "os" + "path/filepath" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/shell" +) + +// N1 and N2 take a real native reading with shell variables, the selected cwd, +// and the command's own failing exit status, without requiring make on Windows. +func TestWindowsNativeReading(t *testing.T) { + if _, err := shell.Bash(); err != nil { + t.Fatal(err) + } + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "marker"), []byte("right"), 0600); err != nil { + t.Fatal(err) + } + strategy := Strategy{Command: `export PATH='/c/Program Files (x86)/fixture':$PATH; x=inner; test "$x" = inner; test "$(cat marker)" = right; printf '%s\n' '--- FAIL: TestWindowsNative (0s)'; exit 23`} + result, ok := RunReading(context.Background(), dir, strategy, 20*time.Second) + if !ok || result.Exit != 23 || result.TimedOut || len(result.Failing) != 1 || result.Failing[0] != "TestWindowsNative" { + t.Fatalf("native reading: %+v, present=%v", result, ok) + } + t.Setenv("CODEAF_BASH", filepath.Join(dir, "missing.exe")) + if _, ok := RunReading(context.Background(), dir, strategy, 20*time.Second); ok { + t.Fatal("a reading with no usable shell was present") + } +} diff --git a/internal/workspace/store_windows_test.go b/internal/workspace/store_windows_test.go new file mode 100644 index 0000000000..6688b88419 --- /dev/null +++ b/internal/workspace/store_windows_test.go @@ -0,0 +1,38 @@ +//go:build windows + +package workspace + +import ( + "context" + "path/filepath" + "testing" +) + +// N2 opens the collections store through its real door, keeping the native +// filename's space, Unicode, fragment and percent characters after reopening. +func TestWindowsWorkspaceStoreNativePath(t *testing.T) { + path := filepath.Join(t.TempDir(), "dir with space é", "collections #%25.db") + s, err := Open(path) + if err != nil { + t.Fatal(err) + } + defer func() { + if s != nil { + _ = s.Close() + } + }() + _, err = s.Create(context.Background(), "héllo") + if err != nil { + t.Fatal(err) + } + if err := s.Close(); err != nil { + t.Fatal(err) + } + s, err = Open(path) + if err != nil { + t.Fatal(err) + } + if got, err := s.Collections(context.Background()); err != nil || len(got) != 1 || got[0].Name != "héllo" { + t.Fatalf("collection = %+v, %v", got, err) + } +} From 046196595b02ae2638784fa0853eb03a9f07fb8d Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 23 Oct 2026 00:17:00 -0400 Subject: [PATCH 2/3] changelog: number the Windows Git Bash entry for #1909 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../{0-windows-git-bash.md => 1909-windows-git-bash.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename docs/changes/unreleased/{0-windows-git-bash.md => 1909-windows-git-bash.md} (99%) diff --git a/docs/changes/unreleased/0-windows-git-bash.md b/docs/changes/unreleased/1909-windows-git-bash.md similarity index 99% rename from docs/changes/unreleased/0-windows-git-bash.md rename to docs/changes/unreleased/1909-windows-git-bash.md index 49859959b1..75765ba0e8 100644 --- a/docs/changes/unreleased/0-windows-git-bash.md +++ b/docs/changes/unreleased/1909-windows-git-bash.md @@ -1,7 +1,7 @@ --- kind: fixed title: Run native Windows commands in Git Bash -pr: 0 +pr: 1909 surface: [chat, engine] invalidates: - "Native Windows commands previously selected the WSL launcher and parsed variables a second time in Linux. Native commands now select Git Bash, preserve shell variables and Windows file access, and omit the shell tool when no usable bash is installed." From c112da52ddfae4f2f3cf815171bded1f9dc142b7 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 23 Oct 2026 06:06:25 -0400 Subject: [PATCH 3/3] config: CODEAF_BASH is operator plumbing The environment registry law found it unregistered; a path to a program is plumbing, like CODEAF_FURROW. Co-Authored-By: Claude Opus 5.5 (1M context) --- internal/config/settings.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/config/settings.go b/internal/config/settings.go index c5e54b9abb..21fcd0e0b0 100644 --- a/internal/config/settings.go +++ b/internal/config/settings.go @@ -1041,6 +1041,10 @@ var OperatorEnvPins = []string{ // CODEAF_FURROW names a furrow to use instead of the one codeaf carries // (internal/furrow). A path to a program is plumbing. "CODEAF_FURROW", + // CODEAF_BASH names the bash native Windows runs commands in when Git for + // Windows is not where codeaf looks (internal/shell). A path to a program + // is plumbing for the same reason CODEAF_FURROW is. + "CODEAF_BASH", // The three site-attribution pins — a URL, an app name, a category list — // used to sit here, and they are gone rather than moved: the OpenRouter app // this binary reports as is decided in internal/provider from the binary's