From 89973c8deb14d91e3440b3c3e1d7571774b76de4 Mon Sep 17 00:00:00 2001 From: rohan-chaturvedi Date: Mon, 26 Oct 2026 21:23:08 +0530 Subject: [PATCH 01/11] fix(ai): route Claude Code shells on Windows with a port probe On Windows, env.sh guarded proxy routing with `kill -0 `, but Claude Code's shell there is Git Bash (MSYS2), whose kill cannot see native Windows PIDs. The guard never passed: shells got the decoys but never HTTPS_PROXY, PHASE_AI_ENABLED or the CA variables, so Phase-managed commands sent decoys straight to providers while `phase ai status` reported routing as active. On Windows the guard now checks that the proxy still accepts connections (`true `, a builtin with no subshell, about 3 ms); other platforms keep the PID check. Hooks still retire routing for a dead daemon through the Win32 liveness check in reconcileClaudeEnvironment. A new test sources the Windows rendering in bash (Git Bash on Windows) against a real listener, and the hook tests now assert the platform's guard instead of the literal `kill -0`. --- src/cmd/ai_claude_environment.go | 63 ++++++++++++-- src/cmd/ai_claude_environment_test.go | 115 ++++++++++++++++++++++++++ src/cmd/ai_claude_hook_test.go | 10 ++- 3 files changed, 179 insertions(+), 9 deletions(-) diff --git a/src/cmd/ai_claude_environment.go b/src/cmd/ai_claude_environment.go index f88c6111..03d37034 100644 --- a/src/cmd/ai_claude_environment.go +++ b/src/cmd/ai_claude_environment.go @@ -5,9 +5,13 @@ import ( "context" "errors" "fmt" + "net" + "net/url" "os" "path/filepath" + "runtime" "sort" + "strconv" "strings" "time" @@ -35,6 +39,10 @@ const ( claudeHookKickBudget = 1500 * time.Millisecond ) +// claudeEnvironmentGOOS selects the liveness guard renderClaudeEnvironment +// writes; tests override it to render another platform's file. +var claudeEnvironmentGOOS = runtime.GOOS + type claudeEnvironmentStatus struct { Schema int `json:"schema"` PID int `json:"pid"` @@ -70,15 +78,22 @@ type claudeRuntimeHealth struct { // renderClaudeEnvironment produces the POSIX shell a harness sources before each // command. Proxy routing, Phase runtime markers, and trust overrides live inside -// a liveness guard on the issuing proxy PID, so a crashed proxy can never strand -// a shell behind a dead port. Decoys, adapter endpoints, and explicit unsets -// stay outside the guard: while Phase AI is enabled, provider tools either reach -// the Phase proxy or fail on the decoy. They never fall through to an ambient -// credential by accident. +// a liveness guard on the issuing proxy (see claudeProxyLivenessGuard), so a +// crashed proxy can never strand a shell behind a dead port. Decoys, adapter +// endpoints, and explicit unsets stay outside the guard: while Phase AI is +// enabled, provider tools either reach the Phase proxy or fail on the decoy. +// They never fall through to an ambient credential by accident. func renderClaudeEnvironment(pid int, plan phaseproxy.ProcessEnvironmentPlan, proxyLive bool) (string, error) { if proxyLive && pid <= 0 { return "", errors.New("Phase AI proxy PID is required for a live environment") } + var guard string + if proxyLive { + var err error + if guard, err = claudeProxyLivenessGuard(pid, plan); err != nil { + return "", err + } + } keys := make([]string, 0, len(plan.Set)) for key, value := range plan.Set { if !phaseproxy.ValidShellEnvironmentKey(key) { @@ -115,7 +130,7 @@ func renderClaudeEnvironment(pid int, plan phaseproxy.ProcessEnvironmentPlan, pr } if proxyLive { out.WriteString(claudeProxyBlockBegin + "\n") - fmt.Fprintf(&out, "if kill -0 %d 2>/dev/null; then\n", pid) + out.WriteString("if " + guard + "; then\n") for _, key := range keys { if !phaseproxy.RuntimeControlEnvironmentKey(key) { continue @@ -133,6 +148,42 @@ func renderClaudeEnvironment(pid int, plan phaseproxy.ProcessEnvironmentPlan, pr return out.String(), nil } +// claudeProxyLivenessGuard returns the shell condition that keeps proxy routing +// on only while the issuing proxy is alive. POSIX shells signal its PID. On +// Windows the sourcing shell is Git Bash (MSYS2), whose kill cannot see native +// Windows PIDs, so the guard checks that the proxy still accepts connections +// instead; hooks retire routing for a dead daemon through the Win32 liveness +// check in reconcileClaudeEnvironment. +func claudeProxyLivenessGuard(pid int, plan phaseproxy.ProcessEnvironmentPlan) (string, error) { + if claudeEnvironmentGOOS != "windows" { + return fmt.Sprintf("kill -0 %d 2>/dev/null", pid), nil + } + raw := "" + for _, key := range []string{"HTTPS_PROXY", "HTTP_PROXY", "https_proxy", "http_proxy"} { + if raw = plan.Set[key]; raw != "" { + break + } + } + if raw == "" { + // Nothing in this plan routes through the proxy. + return "false", nil + } + proxyURL, err := url.Parse(raw) + if err != nil { + return "", errors.New("Phase AI proxy URL is invalid") + } + host, port := proxyURL.Hostname(), proxyURL.Port() + if address := net.ParseIP(host); address == nil || address.To4() == nil { + return "", fmt.Errorf("Phase AI proxy host %q is not an IPv4 address", host) + } + if number, err := strconv.Atoi(port); err != nil || number < 1 || number > 65535 { + return "", fmt.Errorf("Phase AI proxy port %q is invalid", port) + } + // `true` is a regular builtin: a refused connection only fails the test, it + // never ends a non-interactive shell, and the probe needs no subshell. + return fmt.Sprintf("true 2>/dev/null /dev/null /dev/null; then", os.Getpid())) || + guard, err := claudeProxyLivenessGuard(os.Getpid(), phaseproxy.ProcessEnvironmentPlan{Set: map[string]string{"HTTP_PROXY": managed.states[index].ProxyURL}}) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(environment, "if "+guard+"; then") || !strings.Contains(environment, "export HTTP_PROXY='"+managed.states[index].ProxyURL+"'") || !strings.Contains(environment, "export AWS_ACCESS_KEY_ID='AKIADECOY'") { t.Fatalf("cycle %d did not publish a live sourced environment:\n%s", index+1, environment) @@ -288,7 +292,7 @@ func TestClaudeDaemonRecyclesSameProcessAndGenerationOnOneEngine(t *testing.T) { if err != nil { t.Fatal(err) } - if strings.Contains(string(final), "kill -0") || strings.Contains(string(final), "HTTP_PROXY") || + if strings.Contains(string(final), claudeProxyBlockBegin) || strings.Contains(string(final), "HTTP_PROXY") || !strings.Contains(string(final), "export AWS_ACCESS_KEY_ID='AKIADECOY'") { t.Fatalf("daemon exit left proxy routing in the sourced environment:\n%s", final) } @@ -928,7 +932,7 @@ func TestClaudeHeartbeatReportsStatusAndRetiresStaleRouting(t *testing.T) { if err != nil { t.Fatal(err) } - if strings.Contains(string(environment), "kill -0") || strings.Contains(string(environment), "HTTPS_PROXY") || + if strings.Contains(string(environment), claudeProxyBlockBegin) || strings.Contains(string(environment), "HTTPS_PROXY") || !strings.Contains(string(environment), "export AWS_ACCESS_KEY_ID='AKIADECOY'") { t.Fatalf("heartbeat left routing for a dead proxy:\n%s", environment) } From 201937d4e998e37435b85414654e4cba8b730dd5 Mon Sep 17 00:00:00 2001 From: rohan-chaturvedi Date: Mon, 26 Oct 2026 21:23:08 +0530 Subject: [PATCH 02/11] fix(ai): install the OpenCode plugin where OpenCode loads it on Windows On Windows the plugin went to os.UserConfigDir() (%AppData%\opencode\plugins), but OpenCode resolves its global config directory with xdg-basedir, which is ~/.config/opencode on every OS. OpenCode never loaded the plugin, so its shells got no Phase environment while the skill, already installed under ~/.config/opencode, said the environment was automatic. The plugin now always goes to ~/.config/opencode/plugins, beside the skill. enable and disable remove a Phase-owned copy left at the old %AppData% path; foreign files are left alone. The OpenCode tests set USERPROFILE as well as HOME, since os.UserHomeDir reads USERPROFILE on Windows. --- src/cmd/ai_disable.go | 7 ++++ src/cmd/ai_enable.go | 5 +++ src/pkg/ai/opencode_plugin.go | 34 ++++++++++++----- src/pkg/ai/opencode_plugin_test.go | 60 ++++++++++++++++++++++++++++++ 4 files changed, 97 insertions(+), 9 deletions(-) diff --git a/src/cmd/ai_disable.go b/src/cmd/ai_disable.go index 8d01eba9..c98af853 100644 --- a/src/cmd/ai_disable.go +++ b/src/cmd/ai_disable.go @@ -77,6 +77,13 @@ func runAIDisable(cmd *cobra.Command, args []string) error { } else if statErr == nil { fmt.Printf(" Removed OpenCode Phase plugin: %s\n", pluginPath) } + removed, err := ai.RemoveLegacyOpenCodePlugins() + if err != nil { + cleanupErrors = append(cleanupErrors, fmt.Errorf("remove legacy OpenCode Phase plugin: %w", err)) + } + for _, path := range removed { + fmt.Printf(" Removed OpenCode Phase plugin: %s\n", path) + } } if configPath, err := ai.DefaultDevinConfigPath(); err == nil && cfg != nil && cfg.HarnessConfigured(ai.HarnessDevin) { if _, statErr := os.Stat(configPath); statErr == nil { diff --git a/src/cmd/ai_enable.go b/src/cmd/ai_enable.go index 8101a1d6..275f7821 100644 --- a/src/cmd/ai_enable.go +++ b/src/cmd/ai_enable.go @@ -459,6 +459,11 @@ func installConfiguredAIHarnessAdapters(ctx context.Context, harnesses []string, return nil, fmt.Errorf("install OpenCode Phase plugin: %w", err) } installed[harness] = "OpenCode Phase plugin installed to: " + installation.PluginPath + if removed, err := ai.RemoveLegacyOpenCodePlugins(); err != nil { + installed[harness] += fmt.Sprintf(" (could not remove an older copy OpenCode never loads: %v)", err) + } else if len(removed) > 0 { + installed[harness] += " (removed an older copy OpenCode never loads: " + strings.Join(removed, ", ") + ")" + } case ai.HarnessDevin: path, err := ai.DefaultDevinConfigPath() if err != nil { diff --git a/src/pkg/ai/opencode_plugin.go b/src/pkg/ai/opencode_plugin.go index 92db3a2a..1d250e75 100644 --- a/src/pkg/ai/opencode_plugin.go +++ b/src/pkg/ai/opencode_plugin.go @@ -22,23 +22,39 @@ type OpenCodePluginInstall struct { AdapterProtocol string } -// DefaultOpenCodePluginPath returns OpenCode's documented global local-plugin -// path. Local JavaScript plugins are loaded automatically on the next start. +// DefaultOpenCodePluginPath returns OpenCode's global local-plugin path. OpenCode +// resolves its global config directory with xdg-basedir, which is +// ~/.config/opencode on every OS, Windows included, so the plugin sits beside +// the skill OpenCode loads from the same directory. Local JavaScript plugins +// are loaded automatically on the next start. func DefaultOpenCodePluginPath() (string, error) { home, err := os.UserHomeDir() if err != nil { return "", fmt.Errorf("resolve home directory: %w", err) } - if runtime.GOOS == "windows" { - configDirectory, err := os.UserConfigDir() - if err != nil { - return "", fmt.Errorf("resolve user config directory: %w", err) - } - return filepath.Join(configDirectory, "opencode", "plugins", OpenCodePluginName+".js"), nil - } return filepath.Join(home, ".config", "opencode", "plugins", OpenCodePluginName+".js"), nil } +// RemoveLegacyOpenCodePlugins removes Phase-owned copies of the plugin that +// earlier builds installed where OpenCode never loads them (%AppData% on +// Windows). Missing and foreign files are left alone. It returns the paths it +// removed. +func RemoveLegacyOpenCodePlugins() ([]string, error) { + if runtime.GOOS != "windows" { + return nil, nil + } + configDirectory, err := os.UserConfigDir() + if err != nil { + return nil, nil + } + legacy := filepath.Join(configDirectory, "opencode", "plugins", OpenCodePluginName+".js") + removed, err := removeOwnedOpenCodePluginFile(legacy, false) + if err != nil || !removed { + return nil, err + } + return []string{legacy}, nil +} + // InstallOpenCodePlugin installs a dependency-free, provider-agnostic plugin. // It gives every shell OpenCode spawns the Phase AI environment published by // the hook-owned proxy daemon (environmentPath, a JSON twin of the file Claude diff --git a/src/pkg/ai/opencode_plugin_test.go b/src/pkg/ai/opencode_plugin_test.go index cd4535c7..77a585ad 100644 --- a/src/pkg/ai/opencode_plugin_test.go +++ b/src/pkg/ai/opencode_plugin_test.go @@ -14,6 +14,7 @@ import ( func TestInstallOpenCodePluginIsDependencyFreeAndIdempotent(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) pluginPath := filepath.Join(home, ".config", "opencode", "plugins", OpenCodePluginName+".js") first := filepath.Join(home, "first-phase") if err := os.WriteFile(first, []byte("first"), 0o700); err != nil { @@ -97,6 +98,7 @@ func TestInstalledOpenCodeOnlyDelegatesExplicitPhaseControlCalls(t *testing.T) { } home := t.TempDir() t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) pluginPath := filepath.Join(home, OpenCodePluginName+".js") helper := filepath.Join(home, "phase-hook-v1") helperSource := "#!/bin/sh\nprintf '%s\\n' '{\"schemaVersion\":1,\"managed\":true,\"command\":\"phase-rewritten\"}'\n" @@ -179,6 +181,7 @@ func TestInstalledOpenCodePluginAppliesPublishedEnvironmentOnlyWhileProxyIsAlive } home := t.TempDir() t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) pluginPath := filepath.Join(home, OpenCodePluginName+".js") helper := filepath.Join(home, "phase-hook-v1") kickLog := filepath.Join(home, "kicks.log") @@ -258,6 +261,7 @@ check(Object.keys(missing.env).length === 1 && missing.env.ONLY === "1", "missin func TestUninstallOpenCodePluginPreservesForeignFile(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) pluginPath := filepath.Join(home, ".config", "opencode", "plugins", OpenCodePluginName+".js") if err := os.MkdirAll(filepath.Dir(pluginPath), 0o700); err != nil { t.Fatal(err) @@ -276,6 +280,7 @@ func TestUninstallOpenCodePluginPreservesForeignFile(t *testing.T) { func TestUninstallOpenCodePluginRemovesOwnedFiles(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) pluginPath := filepath.Join(home, ".config", "opencode", "plugins", OpenCodePluginName+".js") executable := filepath.Join(home, "phase") if err := os.WriteFile(executable, []byte("phase"), 0o700); err != nil { @@ -306,6 +311,7 @@ func TestInstalledOpenCodePluginExportsOneDefaultDefinition(t *testing.T) { } home := t.TempDir() t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) pluginPath := filepath.Join(home, ".config", "opencode", "plugins", OpenCodePluginName+".js") if _, err := InstallOpenCodePlugin(pluginPath, filepath.Join(home, "phase-hook-v1"), filepath.Join(home, ".phase", "ai", "env.json")); err != nil { t.Fatal(err) @@ -323,3 +329,57 @@ func TestInstalledOpenCodePluginExportsOneDefaultDefinition(t *testing.T) { t.Fatalf("OpenCode plugin module shape: %v\n%s", err, output) } } + +func TestDefaultOpenCodePluginPathSitsBesideTheSkillOnEveryOS(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv("APPDATA", filepath.Join(home, "AppData", "Roaming")) + pluginPath, err := DefaultOpenCodePluginPath() + if err != nil { + t.Fatal(err) + } + // OpenCode reads ~/.config/opencode on Windows too; %AppData% is never loaded. + if want := filepath.Join(home, ".config", "opencode", "plugins", OpenCodePluginName+".js"); pluginPath != want { + t.Fatalf("plugin path = %s, want %s", pluginPath, want) + } + for _, target := range SkillTargets() { + if target.ID == HarnessOpenCode && filepath.Dir(filepath.Dir(filepath.Dir(target.Path))) != filepath.Dir(filepath.Dir(pluginPath)) { + t.Fatalf("OpenCode skill %s and plugin %s are in different config directories", target.Path, pluginPath) + } + } +} + +func TestRemoveLegacyOpenCodePluginsRemovesOnlyOwnedWindowsCopies(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("earlier builds used %AppData% only on Windows") + } + appData := t.TempDir() + t.Setenv("APPDATA", appData) + legacy := filepath.Join(appData, "opencode", "plugins", OpenCodePluginName+".js") + if removed, err := RemoveLegacyOpenCodePlugins(); err != nil || len(removed) != 0 { + t.Fatalf("missing legacy plugin: removed=%v err=%v", removed, err) + } + if err := os.MkdirAll(filepath.Dir(legacy), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(legacy, []byte("// a user's own plugin\n"), 0o600); err != nil { + t.Fatal(err) + } + if removed, err := RemoveLegacyOpenCodePlugins(); err != nil || len(removed) != 0 { + t.Fatalf("foreign legacy plugin: removed=%v err=%v", removed, err) + } + if _, err := os.Stat(legacy); err != nil { + t.Fatalf("foreign plugin was not preserved: %v", err) + } + if err := os.WriteFile(legacy, []byte(openCodeOwnershipMarker+"\nexport default {}\n"), 0o600); err != nil { + t.Fatal(err) + } + removed, err := RemoveLegacyOpenCodePlugins() + if err != nil || len(removed) != 1 || removed[0] != legacy { + t.Fatalf("owned legacy plugin: removed=%v err=%v", removed, err) + } + if _, err := os.Stat(legacy); !os.IsNotExist(err) { + t.Fatalf("owned legacy plugin still exists: %v", err) + } +} From 74980ccbc600091d5239af18bbc174b922d973ea Mon Sep 17 00:00:00 2001 From: rohan-chaturvedi Date: Mon, 26 Oct 2026 21:23:09 +0530 Subject: [PATCH 03/11] fix(proxy): keep inherited environment names that are not shell identifiers ApplyProcessEnvironment dropped every inherited variable whose name was not a POSIX shell identifier. Its result is an exec environment, not shell source, so this only removed real variables: on Windows every managed child lost ProgramFiles(x86) and CommonProgramFiles(x86), which vswhere-based Visual Studio discovery (node-gyp, setuptools, MSBuild) needs, and on Unix exported bash functions disappeared. Inherited names now pass through unless they are empty or contain '=' or NUL, so Windows' hidden per-drive "=C:" entries are still dropped. Keys Phase sets or unsets keep the shell-identifier check, since they also appear in sourced shell files. --- src/pkg/proxy/environment.go | 11 ++++++++++- src/pkg/proxy/store_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/pkg/proxy/environment.go b/src/pkg/proxy/environment.go index 790b4cfd..57e418bd 100644 --- a/src/pkg/proxy/environment.go +++ b/src/pkg/proxy/environment.go @@ -650,11 +650,13 @@ func CompileProcessEnvironment(options EnvironmentOptions) (ProcessEnvironmentPl // ApplyProcessEnvironment applies a compiled Phase-owned delta to an inherited // process environment without modifying the parent process or any files. +// Inherited variables keep their names: the result is an exec environment, not +// shell source, so only the keys Phase sets or unsets must be shell identifiers. func ApplyProcessEnvironment(base []string, plan ProcessEnvironmentPlan) ([]string, error) { values := make(map[string]string) for _, entry := range base { key, value, ok := strings.Cut(entry, "=") - if !ok || !ValidShellEnvironmentKey(key) || containsEnvironmentKey(plan.Unset, key) { + if !ok || !validInheritedEnvironmentKey(key) || containsEnvironmentKey(plan.Unset, key) { continue } if _, phaseOwned := planValue(plan.Set, key); phaseOwned { @@ -682,6 +684,13 @@ func ApplyProcessEnvironment(base []string, plan ProcessEnvironmentPlan) ([]stri return environment, nil } +// validInheritedEnvironmentKey accepts any name an OS environment block can +// carry, such as Windows' ProgramFiles(x86) or bash's exported BASH_FUNC_name%%. +// An empty name is what Windows' hidden per-drive "=C:" entries split into. +func validInheritedEnvironmentKey(key string) bool { + return key != "" && !strings.ContainsAny(key, "=\x00") +} + // ValidShellEnvironmentKey reports whether a key can be emitted as a POSIX // shell assignment without interpreting any part of it as shell syntax. func ValidShellEnvironmentKey(key string) bool { diff --git a/src/pkg/proxy/store_test.go b/src/pkg/proxy/store_test.go index e3bad272..b4d4df9c 100644 --- a/src/pkg/proxy/store_test.go +++ b/src/pkg/proxy/store_test.go @@ -414,6 +414,33 @@ func TestCompileProcessEnvironmentUsesExplicitCleanupAndSanitizesNoProxy(t *test } } +func TestApplyProcessEnvironmentKeepsInheritedNamesThatAreNotShellIdentifiers(t *testing.T) { + environment, err := ApplyProcessEnvironment([]string{ + `ProgramFiles(x86)=C:\Program Files (x86)`, + `CommonProgramFiles(x86)=C:\Program Files (x86)\Common Files`, + "BASH_FUNC_module%%=() { :\n}", + `=C:=C:\work`, + "AWS_PROFILE=ambient-admin", + "AWS_ACCESS_KEY_ID=ambient", + }, ProcessEnvironmentPlan{Set: map[string]string{"AWS_ACCESS_KEY_ID": "decoy"}, Unset: []string{"AWS_PROFILE"}}) + if err != nil { + t.Fatal(err) + } + // Windows toolchains find Visual Studio through ProgramFiles(x86); the + // child must see it exactly as the parent does. + joined := "\n" + strings.Join(environment, "\n") + "\n" + for _, entry := range []string{`ProgramFiles(x86)=C:\Program Files (x86)`, `CommonProgramFiles(x86)=C:\Program Files (x86)\Common Files`, "BASH_FUNC_module%%=() { :\n}", "AWS_ACCESS_KEY_ID=decoy"} { + if !strings.Contains(joined, "\n"+entry+"\n") { + t.Fatalf("missing %q in %q", entry, environment) + } + } + for _, entry := range environment { + if strings.HasPrefix(entry, "=") || strings.HasPrefix(entry, "AWS_PROFILE=") || entry == "AWS_ACCESS_KEY_ID=ambient" { + t.Fatalf("hidden drive entry, unset or overridden value survived: %q", environment) + } + } +} + func TestCompileProcessEnvironmentRejectsBindingAndAdapterCollisions(t *testing.T) { base := EnvironmentOptions{ ProxyURL: "http://phase:capability@127.0.0.1:9543", CABundlePath: "/bundle.pem", From 502163cb18ce086b768690a3eeeddff13eb11a29 Mon Sep 17 00:00:00 2001 From: rohan-chaturvedi Date: Mon, 26 Oct 2026 21:23:09 +0530 Subject: [PATCH 04/11] test(ai): isolate USERPROFILE in the Devin hook tests The Devin tests set only HOME, but os.UserHomeDir reads USERPROFILE on Windows, so running them there installed the shared hook into the real %USERPROFILE%\.phase\ai\bin, replacing phase-hook-v1.exe with a test file. --- src/pkg/ai/devin_hooks_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/pkg/ai/devin_hooks_test.go b/src/pkg/ai/devin_hooks_test.go index 60fad84a..315d403b 100644 --- a/src/pkg/ai/devin_hooks_test.go +++ b/src/pkg/ai/devin_hooks_test.go @@ -11,6 +11,7 @@ import ( func TestInstallDevinHooksPreservesConfigAndIsIdempotent(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) configPath := filepath.Join(home, ".config", "devin", "config.json") if err := os.MkdirAll(filepath.Dir(configPath), 0o700); err != nil { t.Fatal(err) @@ -59,6 +60,7 @@ func TestInstallDevinHooksPreservesConfigAndIsIdempotent(t *testing.T) { func TestUninstallDevinHooksPreservesUnrelatedHooks(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) configPath := filepath.Join(home, ".config", "devin", "config.json") executable := filepath.Join(home, "phase") if err := os.WriteFile(executable, []byte("phase"), 0o700); err != nil { From 51375b72a6f620bb86100d047c48dd6094be907d Mon Sep 17 00:00:00 2001 From: rohan-chaturvedi Date: Mon, 26 Oct 2026 22:11:38 +0530 Subject: [PATCH 05/11] fix(ai): let Codex, Cursor and Devin reach phase.invalid on Windows Phase control calls from the guarded harnesses never reached the proxy on Windows: - The guard rewrote a phase.invalid call to `"C:\...\phase-hook-v1.exe" "ai" "__harness-exec" ...`. PowerShell, which Codex (including the desktop app's bundled pwsh) and Cursor use on Windows, reads a leading quoted string as an expression, so the rewritten call failed to parse. The wrapper is now rendered as an unquoted forward-slash path followed by plain words, which PowerShell 5.1 and 7, cmd and Git Bash all run. A path with spaces uses its 8.3 short name, and failing that PowerShell's call operator. - The wrapper re-runs the call in the default shell, Windows PowerShell 5.1 when pwsh is not on PATH, where curl and wget are aliases of Invoke-WebRequest. That cmdlet ignores HTTP(S)_PROXY, so the request went to DNS and phase.invalid failed to resolve. The wrapper now removes those aliases first so the real executables run. - Invoke-WebRequest and Invoke-RestMethod (iwr, irm) calls to phase.invalid were not recognized at all and failed the same way. The Codex, Cursor, Devin and OpenCode guards now reject them with a message pointing the Agent at curl, which they route. Tests run the rendered wrapper form in cmd, PowerShell and Git Bash from a plain path and from a path with a space, check that curl is the real executable inside the wrapped PowerShell, and cover the new guard denial. --- src/cmd/ai_harness_command_unix.go | 10 +++ src/cmd/ai_harness_command_windows.go | 26 +++++++ src/cmd/ai_harness_command_windows_test.go | 89 ++++++++++++++++++++++ src/cmd/ai_harness_hook.go | 73 ++++++++++++++++-- src/cmd/ai_harness_hook_test.go | 74 ++++++++++++++++++ src/cmd/ai_shell_scope.go | 15 ++++ src/cmd/ai_shell_scope_test.go | 22 ++++++ 7 files changed, 302 insertions(+), 7 deletions(-) create mode 100644 src/cmd/ai_harness_command_unix.go create mode 100644 src/cmd/ai_harness_command_windows.go create mode 100644 src/cmd/ai_harness_command_windows_test.go diff --git a/src/cmd/ai_harness_command_unix.go b/src/cmd/ai_harness_command_unix.go new file mode 100644 index 00000000..482f832b --- /dev/null +++ b/src/cmd/ai_harness_command_unix.go @@ -0,0 +1,10 @@ +//go:build !windows + +package cmd + +import "errors" + +// windowsShortPathName exists only on Windows. +func windowsShortPathName(string) (string, error) { + return "", errors.New("short path names exist only on Windows") +} diff --git a/src/cmd/ai_harness_command_windows.go b/src/cmd/ai_harness_command_windows.go new file mode 100644 index 00000000..dd483d2c --- /dev/null +++ b/src/cmd/ai_harness_command_windows.go @@ -0,0 +1,26 @@ +//go:build windows + +package cmd + +import "syscall" + +// windowsShortPathName returns the 8.3 short form of an existing path. It has +// no spaces when the volume keeps short names, so harness wrapper commands can +// name the hook executable without quoting. +func windowsShortPathName(path string) (string, error) { + long, err := syscall.UTF16PtrFromString(path) + if err != nil { + return "", err + } + buffer := make([]uint16, syscall.MAX_PATH) + for { + length, err := syscall.GetShortPathName(long, &buffer[0], uint32(len(buffer))) + if err != nil { + return "", err + } + if length < uint32(len(buffer)) { + return syscall.UTF16ToString(buffer[:length]), nil + } + buffer = make([]uint16, length) + } +} diff --git a/src/cmd/ai_harness_command_windows_test.go b/src/cmd/ai_harness_command_windows_test.go new file mode 100644 index 00000000..c64fec33 --- /dev/null +++ b/src/cmd/ai_harness_command_windows_test.go @@ -0,0 +1,89 @@ +//go:build windows + +package cmd + +import ( + "bytes" + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "syscall" + "testing" + + "github.com/phasehq/cli/pkg/util" +) + +// windowsTestShells returns the shells a harness may run a wrapped command in +// on Windows, each running one exact command line. Missing shells are skipped. +func windowsTestShells() map[string]func(string) (string, error) { + shells := map[string]func(string) (string, error){ + "cmd": func(line string) (string, error) { + command := exec.Command(filepath.Join(os.Getenv("SystemRoot"), "System32", "cmd.exe")) + command.SysProcAttr = &syscall.SysProcAttr{CmdLine: `cmd.exe /d /s /c "` + line + `"`} + output, err := command.CombinedOutput() + return string(output), err + }, + } + for _, name := range []string{"powershell", "pwsh"} { + if path, err := exec.LookPath(name); err == nil { + shells[name] = func(line string) (string, error) { + output, err := exec.Command(path, "-NoLogo", "-NoProfile", "-NonInteractive", "-Command", line).CombinedOutput() + return string(output), err + } + } + } + if bash := filepath.Join(os.Getenv("ProgramFiles"), "Git", "bin", "bash.exe"); func() bool { _, err := os.Stat(bash); return err == nil }() { + shells["git-bash"] = func(line string) (string, error) { + output, err := exec.Command(bash, "-c", line).CombinedOutput() + return string(output), err + } + } + return shells +} + +// TestWindowsHarnessCommandRunsInEveryWindowsShell runs the rendered wrapper +// form, with curl.exe standing in for the hook executable, in each shell, from +// a plain path and from a path with a space. +func TestWindowsHarnessCommandRunsInEveryWindowsShell(t *testing.T) { + system := filepath.Join(os.Getenv("SystemRoot"), "System32", "curl.exe") + payload, err := os.ReadFile(system) + if err != nil { + t.Skip("curl.exe is not installed") + } + directory := filepath.Join(t.TempDir(), "with space") + if err := os.MkdirAll(directory, 0o700); err != nil { + t.Fatal(err) + } + spaced := filepath.Join(directory, "curl.exe") + if err := os.WriteFile(spaced, payload, 0o700); err != nil { + t.Fatal(err) + } + shells := windowsTestShells() + for _, executable := range []string{system, spaced} { + line := windowsHarnessCommand([]string{executable, "--version"}) + for name, run := range shells { + if strings.HasPrefix(line, "& ") && name != "powershell" && name != "pwsh" { + // No 8.3 name on this volume: only PowerShell runs the fallback form. + continue + } + output, err := run(line) + if err != nil || !strings.HasPrefix(strings.TrimSpace(output), "curl ") { + t.Errorf("%s could not run %q: %v\n%s", name, line, err, output) + } + } + } +} + +func TestExecuteHarnessShellRunsRealCurlUnderWindowsPowerShell(t *testing.T) { + shell := util.GetDefaultShell() + if len(shell) == 0 || (!strings.Contains(strings.ToLower(shell[0]), "powershell") && !strings.Contains(strings.ToLower(shell[0]), "pwsh")) { + t.Skip("the default shell is not PowerShell") + } + var output bytes.Buffer + err := executeHarnessShell(context.Background(), "(Get-Command curl).CommandType", os.Environ(), nil, &output, &output) + if err != nil || strings.TrimSpace(output.String()) != "Application" { + t.Fatalf("curl in the wrapped PowerShell = %q (err %v), want the real executable, not the Invoke-WebRequest alias", output.String(), err) + } +} diff --git a/src/cmd/ai_harness_hook.go b/src/cmd/ai_harness_hook.go index fb1ce9dd..8caa0a20 100644 --- a/src/cmd/ai_harness_hook.go +++ b/src/cmd/ai_harness_hook.go @@ -239,6 +239,9 @@ func runAICodexEvent(cmd *cobra.Command, args []string) error { if err != nil || strings.TrimSpace(original) == "" { return denyCodexShell(cmd) } + if shellCommandUsesPowerShellWeb(original) { + return denyCodexShellWithReason(cmd, harnessPowerShellWebMessage()) + } if !shellCommandRequiresPhase(original) { return nil } @@ -366,6 +369,9 @@ func runAICursorEvent(cmd *cobra.Command, args []string) error { strings.TrimSpace(original) == "" { return denyCursorShell(cmd) } + if shellCommandUsesPowerShellWeb(original) { + return denyCursorShellWithReason(cmd, harnessPowerShellWebMessage()) + } if !shellCommandRequiresPhase(original) { return nil } @@ -489,6 +495,9 @@ func runAIDevinEvent(cmd *cobra.Command, args []string) error { if err != nil || strings.TrimSpace(original) == "" { return denyDevin(cmd, harnessFailureMessage("Devin")) } + if shellCommandUsesPowerShellWeb(original) { + return denyDevin(cmd, harnessPowerShellWebMessage()) + } if !shellCommandRequiresPhase(original) { return nil } @@ -614,6 +623,10 @@ func harnessMixedCommandMessage() string { return "This shell call mixes a proxy-local Phase control request with ordinary work. Split it into separate tool calls so the Phase control request can be handled independently." } +func harnessPowerShellWebMessage() string { + return "PowerShell's Invoke-WebRequest and Invoke-RestMethod do not use the Phase AI proxy, so they cannot reach phase.invalid. Send the same Phase control request with curl in its own tool call instead, for example: curl.exe -sS http://phase.invalid/v1/status" +} + func harnessUnavailableContext(name string) string { return "Phase AI credentials are currently unavailable in this " + name + " session. Ordinary tools, including git and gh, remain available. Commands that need Phase-managed credentials stay blocked until the authenticated proxy recovers." } @@ -686,7 +699,7 @@ func buildHarnessWrapperCommand(harness, leaseID, original string) (string, erro encodedCommand := base64.RawURLEncoding.EncodeToString([]byte(original)) arguments := []string{executable, "ai", "__harness-exec", "--harness", harness, "--lease", encodedLease, "--command", encodedCommand} if runtime.GOOS == "windows" { - return quoteWindowsCommand(arguments), nil + return windowsHarnessCommand(arguments), nil } quoted := make([]string, len(arguments)) for index, argument := range arguments { @@ -695,12 +708,48 @@ func buildHarnessWrapperCommand(harness, leaseID, original string) (string, erro return strings.Join(quoted, " "), nil } -func quoteWindowsCommand(arguments []string) string { - quoted := make([]string, len(arguments)) - for index, argument := range arguments { - quoted[index] = `"` + strings.ReplaceAll(argument, `"`, `\"`) + `"` +// harnessShortPathName resolves a path's 8.3 short name; tests replace it. +var harnessShortPathName = windowsShortPathName + +// windowsHarnessCommand renders the wrapper call for whichever shell a harness +// runs it in on Windows: PowerShell (Codex and Cursor by default), cmd, or Git +// Bash. Only an unquoted path with forward slashes followed by plain words +// parses as a command in all of them: PowerShell reads a leading quoted string +// as an expression, while cmd and bash reject PowerShell's & call operator. A +// path with spaces or shell characters uses its 8.3 short name, and failing +// that the PowerShell call-operator form. +func windowsHarnessCommand(arguments []string) string { + words := append([]string{strings.ReplaceAll(arguments[0], `\`, "/")}, arguments[1:]...) + if !windowsShellPlainWord(words[0]) { + if short, err := harnessShortPathName(arguments[0]); err == nil && windowsShellPlainWord(strings.ReplaceAll(short, `\`, "/")) { + words[0] = strings.ReplaceAll(short, `\`, "/") + } + } + for _, word := range words { + if !windowsShellPlainWord(word) { + quoted := make([]string, len(arguments)) + for index, argument := range arguments { + quoted[index] = "'" + strings.ReplaceAll(argument, "'", "''") + "'" + } + return "& " + strings.Join(quoted, " ") + } } - return strings.Join(quoted, " ") + return strings.Join(words, " ") +} + +// windowsShellPlainWord reports a word that PowerShell, cmd, and POSIX shells +// all pass through unquoted and unexpanded. +func windowsShellPlainWord(word string) bool { + if word == "" || word[0] == '~' { + return false + } + for _, character := range word { + if (character < 'A' || character > 'Z') && (character < 'a' || character > 'z') && + (character < '0' || character > '9') && !strings.ContainsRune(":/._-~", character) { + return false + } + } + return true } type harnessRuntimeBinding struct { @@ -1037,6 +1086,11 @@ func startHarnessCommandLeaseRenewal(done <-chan struct{}, interval time.Duratio return renewal.Wait } +// windowsPowerShellPrelude makes curl and wget run the real executables. +// Windows PowerShell aliases both to Invoke-WebRequest, which ignores +// HTTP(S)_PROXY, so a wrapped phase.invalid call would miss the Phase proxy. +const windowsPowerShellPrelude = "Remove-Item -Path Alias:curl, Alias:wget -Force -ErrorAction SilentlyContinue; " + func executeHarnessShell(ctx context.Context, command string, environment []string, stdin io.Reader, stdout, stderr io.Writer) error { shell := util.GetDefaultShell() if len(shell) == 0 { @@ -1046,7 +1100,7 @@ func executeHarnessShell(ctx context.Context, command string, environment []stri if runtime.GOOS == "windows" { base := strings.ToLower(shell[0]) if strings.Contains(base, "powershell") || strings.Contains(base, "pwsh") { - arguments = []string{"-NoLogo", "-NoProfile", "-NonInteractive", "-Command", command} + arguments = []string{"-NoLogo", "-NoProfile", "-NonInteractive", "-Command", windowsPowerShellPrelude + command} } else { arguments = []string{"/D", "/S", "/C", command} } @@ -1082,6 +1136,11 @@ func runAIOpenCodeCommand(cmd *cobra.Command, _ []string) error { }) } original := string(commandBytes) + if shellCommandUsesPowerShellWeb(original) { + return emitOpenCodeCommandResponse(cmd, openCodeCommandResponse{ + SchemaVersion: 1, Managed: true, Error: harnessPowerShellWebMessage(), + }) + } if shellCommandMixesPhase(original) { return emitOpenCodeCommandResponse(cmd, openCodeCommandResponse{ SchemaVersion: 1, Managed: true, Error: harnessMixedCommandMessage(), diff --git a/src/cmd/ai_harness_hook_test.go b/src/cmd/ai_harness_hook_test.go index ac05758a..dcddf952 100644 --- a/src/cmd/ai_harness_hook_test.go +++ b/src/cmd/ai_harness_hook_test.go @@ -644,6 +644,80 @@ func TestHarnessWrapperCommandEnforcesCrossPlatformArgumentLimit(t *testing.T) { } } +func TestWindowsHarnessCommandParsesInEveryShellOrFallsBack(t *testing.T) { + originalShortPath := harnessShortPathName + t.Cleanup(func() { harnessShortPathName = originalShortPath }) + arguments := []string{`C:\Users\dev\.phase\ai\bin\phase-hook-v1.exe`, "ai", "__harness-exec", "--harness", "codex", "--lease", "Y29kZXg6c2Vzc2lvbg", "--command", "Y3VybA"} + harnessShortPathName = func(string) (string, error) { return "", errors.New("not needed") } + // PowerShell, cmd and Git Bash all run an unquoted forward-slash path. + if got, want := windowsHarnessCommand(arguments), "C:/Users/dev/.phase/ai/bin/phase-hook-v1.exe ai __harness-exec --harness codex --lease Y29kZXg6c2Vzc2lvbg --command Y3VybA"; got != want { + t.Fatalf("plain path command = %q, want %q", got, want) + } + spaced := append([]string{`C:\Users\First Last\.phase\ai\bin\phase-hook-v1.exe`}, arguments[1:]...) + harnessShortPathName = func(path string) (string, error) { + if path != spaced[0] { + t.Fatalf("short name requested for %q", path) + } + return `C:\Users\FIRSTL~1\.phase\ai\bin\PHASE-~1.EXE`, nil + } + if got := windowsHarnessCommand(spaced); !strings.HasPrefix(got, "C:/Users/FIRSTL~1/.phase/ai/bin/PHASE-~1.EXE ai __harness-exec ") { + t.Fatalf("spaced path did not use its short name: %q", got) + } + harnessShortPathName = func(string) (string, error) { return "", errors.New("8.3 names are disabled") } + quoted := append([]string{`C:\Users\O'Brien Smith\.phase\ai\bin\phase-hook-v1.exe`}, arguments[1:]...) + if got, want := windowsHarnessCommand(quoted), `& 'C:\Users\O''Brien Smith\.phase\ai\bin\phase-hook-v1.exe' 'ai' '__harness-exec' '--harness' 'codex' '--lease' 'Y29kZXg6c2Vzc2lvbg' '--command' 'Y3VybA'`; got != want { + t.Fatalf("fallback command = %q, want %q", got, want) + } +} + +func TestHarnessGuardsRejectPowerShellWebCallsToPhase(t *testing.T) { + const call = "Invoke-RestMethod http://phase.invalid/v1/status" + t.Setenv("PHASE_AI_ENABLED", "") + t.Run("codex", func(t *testing.T) { + withHarnessHookConfig(t, ai.HarnessCodex) + command, output := newHarnessHookCommand(`{"session_id":"codex-session","hook_event_name":"PreToolUse","tool_name":"Bash", + "tool_input":{"command":"` + call + `"},"tool_use_id":"tool-one"}`) + if err := runAICodexEvent(command, []string{"guard"}); err != nil { + t.Fatal(err) + } + var result codexToolOutput + if err := json.Unmarshal(output.Bytes(), &result); err != nil { + t.Fatal(err) + } + if result.HookSpecificOutput.PermissionDecision != "deny" || result.HookSpecificOutput.PermissionDecisionReason != harnessPowerShellWebMessage() { + t.Fatalf("Codex guard output = %#v", result) + } + }) + t.Run("cursor", func(t *testing.T) { + withHarnessHookConfig(t, ai.HarnessCursor) + command, output := newHarnessHookCommand(`{"hook_event_name":"preToolUse","tool_name":"Shell", + "tool_input":{"command":"` + call + `"},"tool_use_id":"tool-two"}`) + if err := runAICursorEvent(command, []string{"guard"}); err != nil { + t.Fatal(err) + } + if !strings.Contains(output.String(), "do not use the Phase AI proxy") || strings.Contains(output.String(), "__harness-exec") { + t.Fatalf("Cursor guard output = %s", output.String()) + } + }) + t.Run("devin", func(t *testing.T) { + withHarnessHookConfig(t, ai.HarnessDevin) + command, output := newHarnessHookCommand(`{"session_id":"devin-session","hook_event_name":"PreToolUse","tool_name":"exec", + "tool_input":{"command":"` + call + `"},"tool_use_id":"devin-tool"}`) + if err := runAIDevinEvent(command, []string{"guard"}); err != nil { + t.Fatal(err) + } + if !strings.Contains(output.String(), "do not use the Phase AI proxy") || strings.Contains(output.String(), "__harness-exec") { + t.Fatalf("Devin guard output = %s", output.String()) + } + }) + t.Run("opencode", func(t *testing.T) { + withHarnessHookConfig(t, ai.HarnessOpenCode) + if response := runOpenCodeCommandForTest(t, call); !response.Managed || response.Error != harnessPowerShellWebMessage() { + t.Fatalf("OpenCode command response = %#v", response) + } + }) +} + func TestCursorGuardUsesSessionEnvironmentAndCursorSchema(t *testing.T) { withHarnessHookConfig(t, ai.HarnessCursor) prepareHarnessHookRuntime(t) diff --git a/src/cmd/ai_shell_scope.go b/src/cmd/ai_shell_scope.go index 02bd01bf..e8c1961b 100644 --- a/src/cmd/ai_shell_scope.go +++ b/src/cmd/ai_shell_scope.go @@ -3,12 +3,27 @@ package cmd import ( "net/url" "path/filepath" + "regexp" "strings" "unicode" ) const phaseShellRedirect = "\x00redirect" +// powerShellWebCmdlet matches PowerShell's web cmdlets and their aliases as +// whole words, not inside names such as "confirm" or "$irm". +var powerShellWebCmdlet = regexp.MustCompile(`(?i)(^|[\s;|&({=])(invoke-webrequest|invoke-restmethod|iwr|irm)($|[\s;|)}])`) + +// shellCommandUsesPowerShellWeb reports a PowerShell web cmdlet in a command +// that names phase.invalid. Invoke-WebRequest and Invoke-RestMethod ignore +// HTTP(S)_PROXY in Windows PowerShell, so such a call can never reach the Phase +// proxy and only fails to resolve the host; hooks reject it and point the Agent +// at curl, which they route. The check spans the whole command so a URL held +// in a variable ($u = "http://phase.invalid/..."; irm $u) is caught too. +func shellCommandUsesPowerShellWeb(command string) bool { + return strings.Contains(strings.ToLower(command), "phase.invalid") && powerShellWebCmdlet.MatchString(command) +} + type phaseShellWord struct { value string control bool diff --git a/src/cmd/ai_shell_scope_test.go b/src/cmd/ai_shell_scope_test.go index 9dd17148..19bb0651 100644 --- a/src/cmd/ai_shell_scope_test.go +++ b/src/cmd/ai_shell_scope_test.go @@ -44,6 +44,28 @@ func TestShellCommandRequiresPhase(t *testing.T) { } } +func TestShellCommandUsesPowerShellWeb(t *testing.T) { + for _, test := range []struct { + command string + want bool + }{ + {command: "Invoke-RestMethod http://phase.invalid/v1/status", want: true}, + {command: `irm "http://phase.invalid/v1/requests" -Method Post -Body $body`, want: true}, + {command: "$r = Invoke-WebRequest -Uri http://phase.invalid/v1/status -UseBasicParsing", want: true}, + {command: "(iwr http://phase.invalid/v1/status).Content", want: true}, + {command: `$u = "http://phase.invalid/v1/status"; IRM $u`, want: true}, + {command: "curl.exe -sS http://phase.invalid/v1/status", want: false}, + {command: "curl --fail http://phase.invalid/v1/status", want: false}, + {command: "Invoke-RestMethod https://api.github.com/user", want: false}, + {command: "Write-Output confirm phase.invalid", want: false}, + {command: "Get-Content irm.txt; Get-Content phase.invalid.md", want: false}, + } { + if got := shellCommandUsesPowerShellWeb(test.command); got != test.want { + t.Errorf("shellCommandUsesPowerShellWeb(%q) = %v, want %v", test.command, got, test.want) + } + } +} + func TestShellCommandMixesPhase(t *testing.T) { for _, test := range []struct { command string From 33da4f495bacff55065c84e0240ab9f92b510db8 Mon Sep 17 00:00:00 2001 From: rohan-chaturvedi Date: Mon, 26 Oct 2026 22:30:02 +0530 Subject: [PATCH 06/11] fix(ai): run Phase hooks on Windows in any shell Codex invoked the Phase hooks on Windows, but none of them ran. Their commandWindows entries were written as `"C:\...\phase-hook-v1.exe" "ai" "__codex-event" "guard"`, and Codex runs them in PowerShell (the desktop app's bundled pwsh 7), which reads a leading quoted string as an expression and stops with a ParserError. Codex lets a tool call through when its hook fails, so SessionStart never started the proxy, the guard never routed anything, and curl to phase.invalid went to DNS and failed to resolve. Cursor and Devin hook commands used the same quoted form, which fails the same way wherever they run in PowerShell. The renderer the previous commit added for the guard's rewritten call now lives in pkg/ai as WindowsShellCommand, and every Windows hook command uses it: Codex commandWindows, the Cursor and Devin hooks, and the harness-exec wrapper. It writes an unquoted forward-slash path followed by plain words, which PowerShell 5.1 and 7, cmd and Git Bash all run, and falls back to the 8.3 short name and then PowerShell's call operator for paths with spaces. Devin install and uninstall still recognize hook entries written in the old quoted form. The hook definitions change, so Codex asks to review them again after `phase ai enable`. --- src/cmd/ai_harness_hook.go | 48 +------------ src/cmd/ai_harness_hook_test.go | 26 ------- src/cmd/ai_harness_shell_windows_test.go | 25 +++++++ src/pkg/ai/codex_plugin.go | 10 +-- src/pkg/ai/cursor_plugin.go | 10 +-- src/pkg/ai/devin_hooks.go | 13 ++-- src/pkg/ai/windows_command.go | 59 ++++++++++++++++ .../ai/windows_command_other.go} | 2 +- src/pkg/ai/windows_command_test.go | 70 +++++++++++++++++++ .../ai/windows_command_windows.go} | 6 +- .../ai/windows_command_windows_test.go} | 32 +++------ 11 files changed, 179 insertions(+), 122 deletions(-) create mode 100644 src/cmd/ai_harness_shell_windows_test.go create mode 100644 src/pkg/ai/windows_command.go rename src/{cmd/ai_harness_command_unix.go => pkg/ai/windows_command_other.go} (94%) create mode 100644 src/pkg/ai/windows_command_test.go rename src/{cmd/ai_harness_command_windows.go => pkg/ai/windows_command_windows.go} (80%) rename src/{cmd/ai_harness_command_windows_test.go => pkg/ai/windows_command_windows_test.go} (62%) diff --git a/src/cmd/ai_harness_hook.go b/src/cmd/ai_harness_hook.go index 8caa0a20..5de41e84 100644 --- a/src/cmd/ai_harness_hook.go +++ b/src/cmd/ai_harness_hook.go @@ -699,7 +699,9 @@ func buildHarnessWrapperCommand(harness, leaseID, original string) (string, erro encodedCommand := base64.RawURLEncoding.EncodeToString([]byte(original)) arguments := []string{executable, "ai", "__harness-exec", "--harness", harness, "--lease", encodedLease, "--command", encodedCommand} if runtime.GOOS == "windows" { - return windowsHarnessCommand(arguments), nil + // The harness runs this in PowerShell, cmd, or Git Bash; see + // ai.WindowsShellCommand for the one form all of them parse. + return ai.WindowsShellCommand(arguments), nil } quoted := make([]string, len(arguments)) for index, argument := range arguments { @@ -708,50 +710,6 @@ func buildHarnessWrapperCommand(harness, leaseID, original string) (string, erro return strings.Join(quoted, " "), nil } -// harnessShortPathName resolves a path's 8.3 short name; tests replace it. -var harnessShortPathName = windowsShortPathName - -// windowsHarnessCommand renders the wrapper call for whichever shell a harness -// runs it in on Windows: PowerShell (Codex and Cursor by default), cmd, or Git -// Bash. Only an unquoted path with forward slashes followed by plain words -// parses as a command in all of them: PowerShell reads a leading quoted string -// as an expression, while cmd and bash reject PowerShell's & call operator. A -// path with spaces or shell characters uses its 8.3 short name, and failing -// that the PowerShell call-operator form. -func windowsHarnessCommand(arguments []string) string { - words := append([]string{strings.ReplaceAll(arguments[0], `\`, "/")}, arguments[1:]...) - if !windowsShellPlainWord(words[0]) { - if short, err := harnessShortPathName(arguments[0]); err == nil && windowsShellPlainWord(strings.ReplaceAll(short, `\`, "/")) { - words[0] = strings.ReplaceAll(short, `\`, "/") - } - } - for _, word := range words { - if !windowsShellPlainWord(word) { - quoted := make([]string, len(arguments)) - for index, argument := range arguments { - quoted[index] = "'" + strings.ReplaceAll(argument, "'", "''") + "'" - } - return "& " + strings.Join(quoted, " ") - } - } - return strings.Join(words, " ") -} - -// windowsShellPlainWord reports a word that PowerShell, cmd, and POSIX shells -// all pass through unquoted and unexpanded. -func windowsShellPlainWord(word string) bool { - if word == "" || word[0] == '~' { - return false - } - for _, character := range word { - if (character < 'A' || character > 'Z') && (character < 'a' || character > 'z') && - (character < '0' || character > '9') && !strings.ContainsRune(":/._-~", character) { - return false - } - } - return true -} - type harnessRuntimeBinding struct { directory string leasePath string diff --git a/src/cmd/ai_harness_hook_test.go b/src/cmd/ai_harness_hook_test.go index dcddf952..83c7e76f 100644 --- a/src/cmd/ai_harness_hook_test.go +++ b/src/cmd/ai_harness_hook_test.go @@ -644,32 +644,6 @@ func TestHarnessWrapperCommandEnforcesCrossPlatformArgumentLimit(t *testing.T) { } } -func TestWindowsHarnessCommandParsesInEveryShellOrFallsBack(t *testing.T) { - originalShortPath := harnessShortPathName - t.Cleanup(func() { harnessShortPathName = originalShortPath }) - arguments := []string{`C:\Users\dev\.phase\ai\bin\phase-hook-v1.exe`, "ai", "__harness-exec", "--harness", "codex", "--lease", "Y29kZXg6c2Vzc2lvbg", "--command", "Y3VybA"} - harnessShortPathName = func(string) (string, error) { return "", errors.New("not needed") } - // PowerShell, cmd and Git Bash all run an unquoted forward-slash path. - if got, want := windowsHarnessCommand(arguments), "C:/Users/dev/.phase/ai/bin/phase-hook-v1.exe ai __harness-exec --harness codex --lease Y29kZXg6c2Vzc2lvbg --command Y3VybA"; got != want { - t.Fatalf("plain path command = %q, want %q", got, want) - } - spaced := append([]string{`C:\Users\First Last\.phase\ai\bin\phase-hook-v1.exe`}, arguments[1:]...) - harnessShortPathName = func(path string) (string, error) { - if path != spaced[0] { - t.Fatalf("short name requested for %q", path) - } - return `C:\Users\FIRSTL~1\.phase\ai\bin\PHASE-~1.EXE`, nil - } - if got := windowsHarnessCommand(spaced); !strings.HasPrefix(got, "C:/Users/FIRSTL~1/.phase/ai/bin/PHASE-~1.EXE ai __harness-exec ") { - t.Fatalf("spaced path did not use its short name: %q", got) - } - harnessShortPathName = func(string) (string, error) { return "", errors.New("8.3 names are disabled") } - quoted := append([]string{`C:\Users\O'Brien Smith\.phase\ai\bin\phase-hook-v1.exe`}, arguments[1:]...) - if got, want := windowsHarnessCommand(quoted), `& 'C:\Users\O''Brien Smith\.phase\ai\bin\phase-hook-v1.exe' 'ai' '__harness-exec' '--harness' 'codex' '--lease' 'Y29kZXg6c2Vzc2lvbg' '--command' 'Y3VybA'`; got != want { - t.Fatalf("fallback command = %q, want %q", got, want) - } -} - func TestHarnessGuardsRejectPowerShellWebCallsToPhase(t *testing.T) { const call = "Invoke-RestMethod http://phase.invalid/v1/status" t.Setenv("PHASE_AI_ENABLED", "") diff --git a/src/cmd/ai_harness_shell_windows_test.go b/src/cmd/ai_harness_shell_windows_test.go new file mode 100644 index 00000000..18631ac0 --- /dev/null +++ b/src/cmd/ai_harness_shell_windows_test.go @@ -0,0 +1,25 @@ +//go:build windows + +package cmd + +import ( + "bytes" + "context" + "os" + "strings" + "testing" + + "github.com/phasehq/cli/pkg/util" +) + +func TestExecuteHarnessShellRunsRealCurlUnderWindowsPowerShell(t *testing.T) { + shell := util.GetDefaultShell() + if len(shell) == 0 || (!strings.Contains(strings.ToLower(shell[0]), "powershell") && !strings.Contains(strings.ToLower(shell[0]), "pwsh")) { + t.Skip("the default shell is not PowerShell") + } + var output bytes.Buffer + err := executeHarnessShell(context.Background(), "(Get-Command curl).CommandType", os.Environ(), nil, &output, &output) + if err != nil || strings.TrimSpace(output.String()) != "Application" { + t.Fatalf("curl in the wrapped PowerShell = %q (err %v), want the real executable, not the Invoke-WebRequest alias", output.String(), err) + } +} diff --git a/src/pkg/ai/codex_plugin.go b/src/pkg/ai/codex_plugin.go index 732ee3e9..aad94220 100644 --- a/src/pkg/ai/codex_plugin.go +++ b/src/pkg/ai/codex_plugin.go @@ -363,11 +363,7 @@ func codexBootstrapHooks(hookExecutable string) codexHookFile { func codexPluginCommand(executable string, windows bool, arguments ...string) string { if windows { - parts := []string{quoteWindowsCommandArgument(executable)} - for _, argument := range arguments { - parts = append(parts, quoteWindowsCommandArgument(argument)) - } - return strings.Join(parts, " ") + return WindowsShellCommand(append([]string{executable}, arguments...)) } parts := []string{quotePOSIXCommandArgument(executable)} for _, argument := range arguments { @@ -380,10 +376,6 @@ func quotePOSIXCommandArgument(value string) string { return "'" + strings.ReplaceAll(value, "'", `'"'"'`) + "'" } -func quoteWindowsCommandArgument(value string) string { - return `"` + strings.ReplaceAll(value, `"`, `\"`) + `"` -} - func codexPluginSelector() string { return CodexBootstrapPluginName + "@" + CodexBootstrapMarketplaceName } diff --git a/src/pkg/ai/cursor_plugin.go b/src/pkg/ai/cursor_plugin.go index 5c91957e..e1e30e17 100644 --- a/src/pkg/ai/cursor_plugin.go +++ b/src/pkg/ai/cursor_plugin.go @@ -129,16 +129,12 @@ func cursorBootstrapExecutablePath(root string) string { } func cursorHookCommand(executable string, args ...string) string { - command := quotePOSIXCommandArgument(executable) if runtime.GOOS == "windows" { - command = quoteWindowsCommandArgument(executable) + return WindowsShellCommand(append([]string{executable}, args...)) } + command := quotePOSIXCommandArgument(executable) for _, arg := range args { - if runtime.GOOS == "windows" { - command += " " + quoteWindowsCommandArgument(arg) - } else { - command += " " + quotePOSIXCommandArgument(arg) - } + command += " " + quotePOSIXCommandArgument(arg) } return command } diff --git a/src/pkg/ai/devin_hooks.go b/src/pkg/ai/devin_hooks.go index a0bc0fd0..ee80d9d2 100644 --- a/src/pkg/ai/devin_hooks.go +++ b/src/pkg/ai/devin_hooks.go @@ -61,7 +61,7 @@ func InstallDevinHooks(configPath, phaseExecutable string) (DevinHooksInstall, e if err != nil { return installation, err } - removePhaseDevinGroups(hooks, helperPath) + removePhaseDevinGroups(hooks, windowsExecutableForms(helperPath)...) for event, groups := range phaseDevinHooks(helperPath) { for _, group := range groups { payload, err := json.Marshal(group) @@ -99,7 +99,7 @@ func UninstallDevinHooks(configPath string) error { if err != nil { return err } - removePhaseDevinGroups(hooks, helperPath) + removePhaseDevinGroups(hooks, windowsExecutableForms(helperPath)...) hooksPayload, err := json.Marshal(hooks) if err != nil { return err @@ -156,13 +156,12 @@ func phaseDevinHooks(helperPath string) map[string][]devinHookGroup { } func shellCommand(arguments []string) string { + if runtime.GOOS == "windows" { + return WindowsShellCommand(arguments) + } quoted := make([]string, len(arguments)) for index, argument := range arguments { - if runtime.GOOS == "windows" { - quoted[index] = quoteWindowsCommandArgument(argument) - } else { - quoted[index] = quotePOSIXCommandArgument(argument) - } + quoted[index] = quotePOSIXCommandArgument(argument) } return strings.Join(quoted, " ") } diff --git a/src/pkg/ai/windows_command.go b/src/pkg/ai/windows_command.go new file mode 100644 index 00000000..ad91f9f5 --- /dev/null +++ b/src/pkg/ai/windows_command.go @@ -0,0 +1,59 @@ +package ai + +import "strings" + +// shortPathName resolves a path's 8.3 short name; tests replace it. +var shortPathName = windowsShortPathName + +// WindowsShellCommand renders a command line that runs the same way in every +// shell a harness may use on Windows: PowerShell, cmd, Git Bash, or a direct +// CreateProcess. Only an unquoted path with forward slashes followed by plain +// words parses as a command in all of them: PowerShell reads a leading quoted +// string as an expression, and cmd and bash reject PowerShell's & call +// operator. A path with spaces or shell characters uses its 8.3 short name, +// and failing that the PowerShell call-operator form, PowerShell being the +// default shell of the Windows harnesses. +func WindowsShellCommand(arguments []string) string { + words := append([]string{strings.ReplaceAll(arguments[0], `\`, "/")}, arguments[1:]...) + if !windowsShellPlainWord(words[0]) { + if short, err := shortPathName(arguments[0]); err == nil && windowsShellPlainWord(strings.ReplaceAll(short, `\`, "/")) { + words[0] = strings.ReplaceAll(short, `\`, "/") + } + } + for _, word := range words { + if !windowsShellPlainWord(word) { + quoted := make([]string, len(arguments)) + for index, argument := range arguments { + quoted[index] = "'" + strings.ReplaceAll(argument, "'", "''") + "'" + } + return "& " + strings.Join(quoted, " ") + } + } + return strings.Join(words, " ") +} + +// windowsExecutableForms lists the spellings of executable that a rendered +// command may contain, including the backslash form earlier builds quoted, so +// hook entries written before WindowsShellCommand are still recognized. +func windowsExecutableForms(executable string) []string { + forms := []string{executable, strings.ReplaceAll(executable, `\`, "/")} + if short, err := shortPathName(executable); err == nil { + forms = append(forms, strings.ReplaceAll(short, `\`, "/")) + } + return forms +} + +// windowsShellPlainWord reports a word that PowerShell, cmd, and POSIX shells +// all pass through unquoted and unexpanded. +func windowsShellPlainWord(word string) bool { + if word == "" || word[0] == '~' { + return false + } + for _, character := range word { + if (character < 'A' || character > 'Z') && (character < 'a' || character > 'z') && + (character < '0' || character > '9') && !strings.ContainsRune(":/._-~", character) { + return false + } + } + return true +} diff --git a/src/cmd/ai_harness_command_unix.go b/src/pkg/ai/windows_command_other.go similarity index 94% rename from src/cmd/ai_harness_command_unix.go rename to src/pkg/ai/windows_command_other.go index 482f832b..dc4948a4 100644 --- a/src/cmd/ai_harness_command_unix.go +++ b/src/pkg/ai/windows_command_other.go @@ -1,6 +1,6 @@ //go:build !windows -package cmd +package ai import "errors" diff --git a/src/pkg/ai/windows_command_test.go b/src/pkg/ai/windows_command_test.go new file mode 100644 index 00000000..5566139d --- /dev/null +++ b/src/pkg/ai/windows_command_test.go @@ -0,0 +1,70 @@ +package ai + +import ( + "encoding/json" + "errors" + "strings" + "testing" +) + +func withShortPathName(t *testing.T, resolve func(string) (string, error)) { + t.Helper() + original := shortPathName + shortPathName = resolve + t.Cleanup(func() { shortPathName = original }) +} + +func TestWindowsShellCommandRunsEverywhereOrFallsBack(t *testing.T) { + arguments := []string{`C:\Users\dev\.phase\ai\bin\phase-hook-v1.exe`, "ai", "__codex-event", "guard"} + withShortPathName(t, func(string) (string, error) { return "", errors.New("not needed") }) + // PowerShell, cmd and Git Bash all run an unquoted forward-slash path. + if got, want := WindowsShellCommand(arguments), "C:/Users/dev/.phase/ai/bin/phase-hook-v1.exe ai __codex-event guard"; got != want { + t.Fatalf("plain path command = %q, want %q", got, want) + } + spaced := append([]string{`C:\Users\First Last\.phase\ai\bin\phase-hook-v1.exe`}, arguments[1:]...) + withShortPathName(t, func(path string) (string, error) { + if path != spaced[0] { + t.Fatalf("short name requested for %q", path) + } + return `C:\Users\FIRSTL~1\.phase\ai\bin\PHASE-~1.EXE`, nil + }) + if got, want := WindowsShellCommand(spaced), "C:/Users/FIRSTL~1/.phase/ai/bin/PHASE-~1.EXE ai __codex-event guard"; got != want { + t.Fatalf("spaced path command = %q, want %q", got, want) + } + withShortPathName(t, func(string) (string, error) { return "", errors.New("8.3 names are disabled") }) + quoted := append([]string{`C:\Users\O'Brien Smith\.phase\ai\bin\phase-hook-v1.exe`}, arguments[1:]...) + if got, want := WindowsShellCommand(quoted), `& 'C:\Users\O''Brien Smith\.phase\ai\bin\phase-hook-v1.exe' 'ai' '__codex-event' 'guard'`; got != want { + t.Fatalf("fallback command = %q, want %q", got, want) + } +} + +func TestCodexWindowsHookCommandsAvoidALeadingQuotedPath(t *testing.T) { + withShortPathName(t, func(string) (string, error) { return "", errors.New("not needed") }) + command := codexPluginCommand(`C:\Users\dev\.phase\ai\bin\phase-hook-v1.exe`, true, "ai", "__codex-event", "guard") + // Codex runs commandWindows in a Windows shell; PowerShell cannot run a + // command line that starts with a quoted path. + if strings.HasPrefix(command, `"`) || command != "C:/Users/dev/.phase/ai/bin/phase-hook-v1.exe ai __codex-event guard" { + t.Fatalf("Codex commandWindows = %q", command) + } +} + +func TestDevinRecognizesEveryWindowsSpellingOfTheHelper(t *testing.T) { + withShortPathName(t, func(string) (string, error) { return "", errors.New("8.3 names are disabled") }) + helper := `C:\Users\dev\.phase\ai\bin\phase-hook-v1.exe` + group := func(command string) json.RawMessage { + payload, err := json.Marshal(devinHookGroup{Matcher: "exec", Hooks: []devinHookHandler{{Type: "command", Command: command, Timeout: 45}}}) + if err != nil { + t.Fatal(err) + } + return payload + } + hooks := map[string][]json.RawMessage{"PreToolUse": { + group(`"C:\Users\dev\.phase\ai\bin\phase-hook-v1.exe" "ai" "__devin-event" "guard"`), + group("C:/Users/dev/.phase/ai/bin/phase-hook-v1.exe ai __devin-event guard"), + group("node C:/Users/dev/hooks/audit.js"), + }} + removePhaseDevinGroups(hooks, windowsExecutableForms(helper)...) + if len(hooks["PreToolUse"]) != 1 || !strings.Contains(string(hooks["PreToolUse"][0]), "audit.js") { + t.Fatalf("Phase groups from earlier and current builds must both be removed, and only them: %s", hooks["PreToolUse"]) + } +} diff --git a/src/cmd/ai_harness_command_windows.go b/src/pkg/ai/windows_command_windows.go similarity index 80% rename from src/cmd/ai_harness_command_windows.go rename to src/pkg/ai/windows_command_windows.go index dd483d2c..1b4894d7 100644 --- a/src/cmd/ai_harness_command_windows.go +++ b/src/pkg/ai/windows_command_windows.go @@ -1,12 +1,12 @@ //go:build windows -package cmd +package ai import "syscall" // windowsShortPathName returns the 8.3 short form of an existing path. It has -// no spaces when the volume keeps short names, so harness wrapper commands can -// name the hook executable without quoting. +// no spaces when the volume keeps short names, so commands can name the hook +// executable without quoting. func windowsShortPathName(path string) (string, error) { long, err := syscall.UTF16PtrFromString(path) if err != nil { diff --git a/src/cmd/ai_harness_command_windows_test.go b/src/pkg/ai/windows_command_windows_test.go similarity index 62% rename from src/cmd/ai_harness_command_windows_test.go rename to src/pkg/ai/windows_command_windows_test.go index c64fec33..f01e2b75 100644 --- a/src/cmd/ai_harness_command_windows_test.go +++ b/src/pkg/ai/windows_command_windows_test.go @@ -1,22 +1,18 @@ //go:build windows -package cmd +package ai import ( - "bytes" - "context" "os" "os/exec" "path/filepath" "strings" "syscall" "testing" - - "github.com/phasehq/cli/pkg/util" ) -// windowsTestShells returns the shells a harness may run a wrapped command in -// on Windows, each running one exact command line. Missing shells are skipped. +// windowsTestShells returns the shells a harness may run a command line in on +// Windows, each running one exact command line. Missing shells are skipped. func windowsTestShells() map[string]func(string) (string, error) { shells := map[string]func(string) (string, error){ "cmd": func(line string) (string, error) { @@ -43,10 +39,10 @@ func windowsTestShells() map[string]func(string) (string, error) { return shells } -// TestWindowsHarnessCommandRunsInEveryWindowsShell runs the rendered wrapper -// form, with curl.exe standing in for the hook executable, in each shell, from -// a plain path and from a path with a space. -func TestWindowsHarnessCommandRunsInEveryWindowsShell(t *testing.T) { +// TestWindowsShellCommandRunsInEveryWindowsShell runs a rendered command line, +// with curl.exe standing in for the hook executable, in each shell, from a +// plain path and from a path with a space. +func TestWindowsShellCommandRunsInEveryWindowsShell(t *testing.T) { system := filepath.Join(os.Getenv("SystemRoot"), "System32", "curl.exe") payload, err := os.ReadFile(system) if err != nil { @@ -62,7 +58,7 @@ func TestWindowsHarnessCommandRunsInEveryWindowsShell(t *testing.T) { } shells := windowsTestShells() for _, executable := range []string{system, spaced} { - line := windowsHarnessCommand([]string{executable, "--version"}) + line := WindowsShellCommand([]string{executable, "--version"}) for name, run := range shells { if strings.HasPrefix(line, "& ") && name != "powershell" && name != "pwsh" { // No 8.3 name on this volume: only PowerShell runs the fallback form. @@ -75,15 +71,3 @@ func TestWindowsHarnessCommandRunsInEveryWindowsShell(t *testing.T) { } } } - -func TestExecuteHarnessShellRunsRealCurlUnderWindowsPowerShell(t *testing.T) { - shell := util.GetDefaultShell() - if len(shell) == 0 || (!strings.Contains(strings.ToLower(shell[0]), "powershell") && !strings.Contains(strings.ToLower(shell[0]), "pwsh")) { - t.Skip("the default shell is not PowerShell") - } - var output bytes.Buffer - err := executeHarnessShell(context.Background(), "(Get-Command curl).CommandType", os.Environ(), nil, &output, &output) - if err != nil || strings.TrimSpace(output.String()) != "Application" { - t.Fatalf("curl in the wrapped PowerShell = %q (err %v), want the real executable, not the Invoke-WebRequest alias", output.String(), err) - } -} From ed8e51bdfc46a583e36307bb72477c015b0c2f23 Mon Sep 17 00:00:00 2001 From: rohan-chaturvedi Date: Tue, 27 Oct 2026 11:03:09 +0530 Subject: [PATCH 07/11] fix(ai): enable other harnesses when the Codex CLI is not on PATH phase ai enable refreshes the adapter of every configured harness, not only the selected one. Once Codex had been enabled, every later enable, for example `phase ai enable --harness cursor`, failed with "Codex CLI is required to refresh the configured Phase plugin" wherever codex was not on PATH, for example on a Windows machine with only the Codex desktop app installed. The failure came after the skills were installed and the runtime stopped, but before the configuration was saved, so the new harness was left half set up. When the Codex CLI is missing and Codex is not the selected harness, its adapter is now left as it was, still running the shared helper that the same pass refreshes, and the summary says Codex was not refreshed and how to refresh it. Enabling Codex itself still requires the CLI, and its error now says the CLI has to be on PATH. --- src/cmd/ai_enable.go | 57 ++++++++++++++++++++++++--------------- src/cmd/ai_enable_test.go | 49 ++++++++++++++++++++++++++++----- 2 files changed, 78 insertions(+), 28 deletions(-) diff --git a/src/cmd/ai_enable.go b/src/cmd/ai_enable.go index 275f7821..ccc69161 100644 --- a/src/cmd/ai_enable.go +++ b/src/cmd/ai_enable.go @@ -245,7 +245,7 @@ func runAIEnable(cmd *cobra.Command, args []string) error { fmt.Printf(" Earlier Phase skill migrated to: %s\n", migratedPath) } - installations, err := installConfiguredAIHarnessAdapters(cmd.Context(), cfg.ConfiguredHarnessIDs(), executable) + installations, skipped, err := installConfiguredAIHarnessAdapters(cmd.Context(), cfg.ConfiguredHarnessIDs(), selectedTarget.ID, executable) if err != nil { return err } @@ -263,7 +263,7 @@ func runAIEnable(cmd *cobra.Command, args []string) error { if user, err := config.GetDefaultUser(); err == nil { principal = user } - fmt.Print(formatAIEnableSummary(selectedTarget, cfg, principal, installations)) + fmt.Print(formatAIEnableSummary(selectedTarget, cfg, principal, installations, skipped)) return nil } @@ -280,7 +280,7 @@ func trustAttemptPossible(flagChanged, flagValue, interactive bool) bool { // line, then the steps the human must take before the harness can use Phase. // It is separated from the install log by blank lines so the next steps are // the last thing on screen. -func formatAIEnableSummary(target ai.SkillTarget, cfg *config.AIConfig, principal *config.UserConfig, installations map[string]string) string { +func formatAIEnableSummary(target ai.SkillTarget, cfg *config.AIConfig, principal *config.UserConfig, installations, skipped map[string]string) string { var out strings.Builder out.WriteString("\n") if target.Name != "" { @@ -324,6 +324,11 @@ func formatAIEnableSummary(target ai.SkillTarget, cfg *config.AIConfig, principa out.WriteString(util.BoldYellow("⚠️ Codex: its Phase hook was refreshed too.") + "\n") out.WriteString(fmt.Sprintf(" Open the Codex CLI, run /hooks, and trust every hook from %s@%s again.\n", ai.CodexBootstrapPluginName, ai.CodexBootstrapMarketplaceName)) } + if reason := skipped[ai.HarnessCodex]; reason != "" { + out.WriteString("\n") + out.WriteString(util.BoldYellow("⚠️ Codex: its Phase plugin was not refreshed, because "+reason+".") + "\n") + out.WriteString(" Rerun `phase ai enable --harness codex` with the Codex CLI on PATH to refresh it.\n") + } if target.ID != "" { out.WriteString("\n Add another harness with `phase ai enable --harness `; existing integrations stay installed.\n") } @@ -393,9 +398,13 @@ func claudeHookExecutablePath() (string, error) { // installConfiguredAIHarnessAdapters refreshes one major-versioned helper from // the current CLI, then points every thin harness adapter at it. Provider and -// discovery changes do not require reinstalling those adapters. -func installConfiguredAIHarnessAdapters(ctx context.Context, harnesses []string, executable string) (map[string]string, error) { +// discovery changes do not require reinstalling those adapters. A configured +// Codex adapter is left as it was when the Codex CLI is not on PATH, unless +// Codex is the selected harness, so adding another harness does not depend on +// Codex; the second result maps each skipped harness to the reason. +func installConfiguredAIHarnessAdapters(ctx context.Context, harnesses []string, selected, executable string) (map[string]string, map[string]string, error) { installed := make(map[string]string, len(harnesses)) + skipped := make(map[string]string) hookExecutable := executable for _, harness := range harnesses { if harness == ai.HarnessCopilot { @@ -403,7 +412,7 @@ func installConfiguredAIHarnessAdapters(ctx context.Context, harnesses []string, } shared, err := ai.InstallSharedHookExecutable(executable) if err != nil { - return nil, fmt.Errorf("install shared Phase Agent hook: %w", err) + return nil, nil, fmt.Errorf("install shared Phase Agent hook: %w", err) } hookExecutable = shared.ExecutablePath break @@ -413,50 +422,56 @@ func installConfiguredAIHarnessAdapters(ctx context.Context, harnesses []string, case ai.HarnessClaudeCode: path, err := ai.DefaultClaudeBootstrapPluginPath() if err != nil { - return nil, err + return nil, nil, err } if err := ai.InstallClaudeBootstrapPlugin(path, hookExecutable); err != nil { - return nil, fmt.Errorf("install Claude Phase discovery adapter: %w", err) + return nil, nil, fmt.Errorf("install Claude Phase discovery adapter: %w", err) } installed[harness] = "Claude Phase discovery adapter installed to: " + path case ai.HarnessCursor: path, err := ai.DefaultCursorBootstrapPluginPath() if err != nil { - return nil, err + return nil, nil, err } if err := ai.InstallCursorBootstrapPlugin(path, hookExecutable); err != nil { - return nil, fmt.Errorf("install Cursor Phase discovery adapter: %w", err) + return nil, nil, fmt.Errorf("install Cursor Phase discovery adapter: %w", err) } installed[harness] = "Cursor Phase discovery adapter installed to: " + path case ai.HarnessCodex: path, err := ai.DefaultCodexBootstrapMarketplacePath() if err != nil { - return nil, err + return nil, nil, err } codexExecutable, err := exec.LookPath("codex") + if err != nil && harness != selected { + // The adapter from an earlier enable still runs the shared + // helper refreshed above. + skipped[harness] = "the Codex CLI is not on PATH" + continue + } if err != nil { - return nil, errors.New("Codex CLI is required to refresh the configured Phase plugin; install Codex, then rerun `phase ai enable`") + return nil, nil, errors.New("Codex CLI is required to install the Phase plugin; install Codex or add its CLI to PATH, then rerun `phase ai enable --harness codex`") } installation, err := ai.InstallCodexBootstrapPlugin(path, hookExecutable) if err != nil { - return nil, fmt.Errorf("install Codex Phase discovery adapter: %w", err) + return nil, nil, fmt.Errorf("install Codex Phase discovery adapter: %w", err) } if err := ai.ActivateCodexBootstrapPlugin(ctx, codexExecutable, path); err != nil { - return nil, fmt.Errorf("activate Codex Phase discovery adapter: %w", err) + return nil, nil, fmt.Errorf("activate Codex Phase discovery adapter: %w", err) } installed[harness] = "Codex Phase discovery adapter installed: " + installation.Selector case ai.HarnessOpenCode: path, err := ai.DefaultOpenCodePluginPath() if err != nil { - return nil, err + return nil, nil, err } runtimeDirectory, err := claudeHookRuntimeDir() if err != nil { - return nil, err + return nil, nil, err } installation, err := ai.InstallOpenCodePlugin(path, hookExecutable, claudeEnvironmentJSONPath(runtimeDirectory)) if err != nil { - return nil, fmt.Errorf("install OpenCode Phase plugin: %w", err) + return nil, nil, fmt.Errorf("install OpenCode Phase plugin: %w", err) } installed[harness] = "OpenCode Phase plugin installed to: " + installation.PluginPath if removed, err := ai.RemoveLegacyOpenCodePlugins(); err != nil { @@ -467,20 +482,20 @@ func installConfiguredAIHarnessAdapters(ctx context.Context, harnesses []string, case ai.HarnessDevin: path, err := ai.DefaultDevinConfigPath() if err != nil { - return nil, err + return nil, nil, err } installation, err := ai.InstallDevinHooks(path, hookExecutable) if err != nil { - return nil, fmt.Errorf("install Devin Phase discovery adapter: %w", err) + return nil, nil, fmt.Errorf("install Devin Phase discovery adapter: %w", err) } installed[harness] = "Devin Phase discovery adapter installed in: " + installation.ConfigPath case ai.HarnessCopilot: // Copilot consumes only the Agent Skill; it has no automatic helper. default: - return nil, fmt.Errorf("unsupported configured Agent harness %q", harness) + return nil, nil, fmt.Errorf("unsupported configured Agent harness %q", harness) } } - return installed, nil + return installed, skipped, nil } func codexHookReviewRequired(installations map[string]string) bool { diff --git a/src/cmd/ai_enable_test.go b/src/cmd/ai_enable_test.go index 67119493..1e756820 100644 --- a/src/cmd/ai_enable_test.go +++ b/src/cmd/ai_enable_test.go @@ -79,9 +79,10 @@ func TestInstallConfiguredAIHarnessAdaptersRefreshesEveryHelper(t *testing.T) { if err != nil { t.Fatal(err) } - installed, err := installConfiguredAIHarnessAdapters( + installed, _, err := installConfiguredAIHarnessAdapters( context.Background(), []string{ai.HarnessClaudeCode, ai.HarnessCursor}, + ai.HarnessCursor, source, ) if err != nil { @@ -141,6 +142,29 @@ func TestCodexHookReviewRequiredWhenExistingAdapterIsRefreshed(t *testing.T) { } } +func TestInstallConfiguredAIHarnessAdaptersSkipsCodexWithoutItsCLIUnlessSelected(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv("PATH", t.TempDir()) + source, err := os.Executable() + if err != nil { + t.Fatal(err) + } + // Codex was enabled earlier, and Cursor is added where codex is not on PATH. + harnesses := []string{ai.HarnessCodex, ai.HarnessCursor} + installed, skipped, err := installConfiguredAIHarnessAdapters(context.Background(), harnesses, ai.HarnessCursor, source) + if err != nil { + t.Fatalf("adding Cursor failed because the Codex CLI is missing: %v", err) + } + if installed[ai.HarnessCursor] == "" || installed[ai.HarnessCodex] != "" || skipped[ai.HarnessCodex] == "" { + t.Fatalf("installed=%v skipped=%v", installed, skipped) + } + if _, _, err := installConfiguredAIHarnessAdapters(context.Background(), harnesses, ai.HarnessCodex, source); err == nil || !strings.Contains(err.Error(), "PATH") { + t.Fatalf("enabling Codex itself must still require its CLI, got %v", err) + } +} + func TestAIEnableHarnessRestartMessagesRepeatOriginalPrompt(t *testing.T) { tests := []struct { harness string @@ -174,7 +198,7 @@ func TestFormatAIEnableSummaryEndsWithNextSteps(t *testing.T) { codex, _ := ai.SkillTargetByID(ai.HarnessCodex) cfg := &config.AIConfig{Enabled: true, MaskSecretValues: true} agent := &config.UserConfig{PrincipalType: config.PrincipalAgent} - summary := formatAIEnableSummary(codex, cfg, agent, map[string]string{ai.HarnessCodex: "Codex Phase discovery adapter installed: phase-ai-bootstrap@phase-cli-local"}) + summary := formatAIEnableSummary(codex, cfg, agent, map[string]string{ai.HarnessCodex: "Codex Phase discovery adapter installed: phase-ai-bootstrap@phase-cli-local"}, nil) if !strings.HasPrefix(summary, "\n") { t.Fatalf("summary must be separated from the install log:\n%s", summary) } @@ -197,7 +221,7 @@ func TestFormatAIEnableSummaryEndsWithNextSteps(t *testing.T) { claude, _ := ai.SkillTargetByID(ai.HarnessClaudeCode) user := &config.UserConfig{PrincipalType: "user"} - summary = formatAIEnableSummary(claude, &config.AIConfig{Enabled: true}, user, map[string]string{ai.HarnessCodex: "refreshed"}) + summary = formatAIEnableSummary(claude, &config.AIConfig{Enabled: true}, user, map[string]string{ai.HarnessCodex: "refreshed"}, nil) for _, required := range []string{ "AI tools can read secret values", "No default Workflow yet", "👉 Before Claude Code can use Phase:", " " + aiEnableHarnessRestartMessage(ai.HarnessClaudeCode) + "\n", "automatically while the proxy runs", @@ -210,19 +234,30 @@ func TestFormatAIEnableSummaryEndsWithNextSteps(t *testing.T) { if strings.Contains(summary, "1. ") { t.Fatalf("a single step must not be numbered:\n%s", summary) } - summary = formatAIEnableSummary(claude, &config.AIConfig{Enabled: true, DefaultWorkflowID: "wf-123"}, nil, nil) + summary = formatAIEnableSummary(claude, &config.AIConfig{Enabled: true, DefaultWorkflowID: "wf-123"}, nil, nil, nil) if !strings.Contains(summary, "Default Workflow: wf-123") { t.Fatalf("default Workflow missing:\n%s", summary) } - custom := formatAIEnableSummary(ai.SkillTarget{}, cfg, nil, nil) + custom := formatAIEnableSummary(ai.SkillTarget{}, cfg, nil, nil, nil) if !strings.Contains(custom, "✅ Phase AI enabled\n") || !strings.Contains(custom, "phase ai run -- ") || strings.Contains(custom, "Add another harness") { t.Fatalf("custom-path summary:\n%s", custom) } copilot, _ := ai.SkillTargetByID(ai.HarnessCopilot) - if summary := formatAIEnableSummary(copilot, cfg, nil, nil); !strings.Contains(summary, "phase ai exec -- ") { + if summary := formatAIEnableSummary(copilot, cfg, nil, nil, nil); !strings.Contains(summary, "phase ai exec -- ") { t.Fatalf("Copilot summary must point at phase ai exec:\n%s", summary) } - t.Logf("codex summary:%s", formatAIEnableSummary(codex, cfg, agent, nil)) + t.Logf("codex summary:%s", formatAIEnableSummary(codex, cfg, agent, nil, nil)) +} + +func TestFormatAIEnableSummaryReportsASkippedCodexRefresh(t *testing.T) { + cursor, _ := ai.SkillTargetByID(ai.HarnessCursor) + summary := formatAIEnableSummary(cursor, &config.AIConfig{Enabled: true}, nil, + map[string]string{ai.HarnessCursor: "Cursor Phase discovery adapter installed"}, + map[string]string{ai.HarnessCodex: "the Codex CLI is not on PATH"}) + if !strings.Contains(summary, "⚠️ Codex: its Phase plugin was not refreshed, because the Codex CLI is not on PATH.") || + !strings.Contains(summary, "`phase ai enable --harness codex`") || strings.Contains(summary, "refreshed too") { + t.Fatalf("a skipped Codex refresh must be reported, without asking to re-trust hooks that did not change:\n%s", summary) + } } func TestTrustAttemptPossibleOnlyWhenADialogCanAppear(t *testing.T) { From 474b9e67e9fcec359c67b60448685cf901a39537 Mon Sep 17 00:00:00 2001 From: rohan-chaturvedi Date: Tue, 27 Oct 2026 11:20:18 +0530 Subject: [PATCH 08/11] fix(ai): give Devin and OpenCode a wrapper PowerShell can run on Windows harnessWrapperCommandLine gives Codex, and Cursor on a PowerShell profile, PowerShell's call-operator form. Every other harness on Windows (Devin, OpenCode, and Cursor on a Git Bash or Command Prompt profile) got the double-quoted form `"C:\...\phase-hook-v1.exe" "ai" ...`. cmd and Git Bash run it, but PowerShell reads a leading quoted string as an expression and rejects it, which is what kept Codex's hooks from running. Nothing guarantees that Devin and OpenCode use cmd or Git Bash on Windows. Those harnesses now get the unquoted forward-slash form that PowerShell, cmd and Git Bash all run (ai.WindowsPlainCommand, split out of WindowsShellCommand), and the double-quoted form only when the path cannot be written unquoted even with its 8.3 name. --- src/cmd/ai_harness_hook.go | 6 ++++++ src/cmd/ai_harness_hook_test.go | 9 +++++++- src/pkg/ai/windows_command.go | 34 +++++++++++++++++++----------- src/pkg/ai/windows_command_test.go | 3 +++ 4 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/cmd/ai_harness_hook.go b/src/cmd/ai_harness_hook.go index 3dc354d5..cfae49b9 100644 --- a/src/cmd/ai_harness_hook.go +++ b/src/cmd/ai_harness_hook.go @@ -705,11 +705,17 @@ func buildHarnessWrapperCommand(harness, leaseID, original string) (string, erro // harnessWrapperCommandLine quotes the wrapper for the shell the harness runs // it in. On Windows, Codex uses PowerShell, which needs the call operator to // run a quoted executable path; Cursor uses its default terminal profile. +// Other harnesses may run it in PowerShell too, so they get the unquoted form +// every Windows shell runs, and the double-quoted cmd form only when the path +// cannot be written unquoted. func harnessWrapperCommandLine(goos, harness string, arguments []string) string { if goos == "windows" { if harness == ai.HarnessCodex || (harness == ai.HarnessCursor && !cursorWindowsShellIsBashOrCmd(cursorWindowsSettings())) { return quotePowerShellCommand(arguments) } + if line, ok := ai.WindowsPlainCommand(arguments); ok { + return line + } return quoteWindowsCommand(arguments) } quoted := make([]string, len(arguments)) diff --git a/src/cmd/ai_harness_hook_test.go b/src/cmd/ai_harness_hook_test.go index 611a48c1..3b26931b 100644 --- a/src/cmd/ai_harness_hook_test.go +++ b/src/cmd/ai_harness_hook_test.go @@ -1340,7 +1340,14 @@ func TestHarnessWrapperCommandLineMatchesTheHarnessShell(t *testing.T) { t.Fatalf("typographic quote not doubled: %s", got) } if got := harnessWrapperCommandLine("windows", ai.HarnessDevin, arguments); !strings.HasPrefix(got, `"C:\Users`) { - t.Fatalf("Devin on Windows keeps the cmd form: %s", got) + t.Fatalf("Devin falls back to the cmd form for a path that needs quoting: %s", got) + } + plain := []string{`C:\Users\dev\.phase\ai\bin\phase-hook-v1.exe`, "ai", "__harness-exec", "--lease", "Y29kZXg6YQ"} + for _, harness := range []string{ai.HarnessDevin, ai.HarnessOpenCode} { + // Their Windows shell may be PowerShell, which cannot run the cmd form. + if got, want := harnessWrapperCommandLine("windows", harness, plain), "C:/Users/dev/.phase/ai/bin/phase-hook-v1.exe ai __harness-exec --lease Y29kZXg6YQ"; got != want { + t.Fatalf("%s on Windows:\n got %s\nwant %s", harness, got, want) + } } if got := harnessWrapperCommandLine("linux", ai.HarnessCodex, []string{"/home/o'brien/phase", "ai"}); got != `'/home/o'"'"'brien/phase' 'ai'` { t.Fatalf("POSIX form = %s", got) diff --git a/src/pkg/ai/windows_command.go b/src/pkg/ai/windows_command.go index ad91f9f5..4cc8020d 100644 --- a/src/pkg/ai/windows_command.go +++ b/src/pkg/ai/windows_command.go @@ -7,13 +7,27 @@ var shortPathName = windowsShortPathName // WindowsShellCommand renders a command line that runs the same way in every // shell a harness may use on Windows: PowerShell, cmd, Git Bash, or a direct -// CreateProcess. Only an unquoted path with forward slashes followed by plain -// words parses as a command in all of them: PowerShell reads a leading quoted -// string as an expression, and cmd and bash reject PowerShell's & call -// operator. A path with spaces or shell characters uses its 8.3 short name, -// and failing that the PowerShell call-operator form, PowerShell being the -// default shell of the Windows harnesses. +// CreateProcess. That is the WindowsPlainCommand form when there is one, and +// otherwise the PowerShell call-operator form, PowerShell being the default +// shell of the Windows harnesses. func WindowsShellCommand(arguments []string) string { + if line, ok := WindowsPlainCommand(arguments); ok { + return line + } + quoted := make([]string, len(arguments)) + for index, argument := range arguments { + quoted[index] = "'" + strings.ReplaceAll(argument, "'", "''") + "'" + } + return "& " + strings.Join(quoted, " ") +} + +// WindowsPlainCommand renders the only command line form that PowerShell, cmd +// and Git Bash all parse as a command: an unquoted path with forward slashes +// followed by plain words. PowerShell reads a leading quoted string as an +// expression, and cmd and bash reject PowerShell's & call operator. A path with +// spaces or shell characters uses its 8.3 short name; it reports false when a +// word still needs quoting. +func WindowsPlainCommand(arguments []string) (string, bool) { words := append([]string{strings.ReplaceAll(arguments[0], `\`, "/")}, arguments[1:]...) if !windowsShellPlainWord(words[0]) { if short, err := shortPathName(arguments[0]); err == nil && windowsShellPlainWord(strings.ReplaceAll(short, `\`, "/")) { @@ -22,14 +36,10 @@ func WindowsShellCommand(arguments []string) string { } for _, word := range words { if !windowsShellPlainWord(word) { - quoted := make([]string, len(arguments)) - for index, argument := range arguments { - quoted[index] = "'" + strings.ReplaceAll(argument, "'", "''") + "'" - } - return "& " + strings.Join(quoted, " ") + return "", false } } - return strings.Join(words, " ") + return strings.Join(words, " "), true } // windowsExecutableForms lists the spellings of executable that a rendered diff --git a/src/pkg/ai/windows_command_test.go b/src/pkg/ai/windows_command_test.go index 5566139d..5e6c70b1 100644 --- a/src/pkg/ai/windows_command_test.go +++ b/src/pkg/ai/windows_command_test.go @@ -36,6 +36,9 @@ func TestWindowsShellCommandRunsEverywhereOrFallsBack(t *testing.T) { if got, want := WindowsShellCommand(quoted), `& 'C:\Users\O''Brien Smith\.phase\ai\bin\phase-hook-v1.exe' 'ai' '__codex-event' 'guard'`; got != want { t.Fatalf("fallback command = %q, want %q", got, want) } + if line, ok := WindowsPlainCommand(quoted); ok { + t.Fatalf("a path that still needs quoting has no plain form, got %q", line) + } } func TestCodexWindowsHookCommandsAvoidALeadingQuotedPath(t *testing.T) { From b9fecd18f7361452991ebb2baa9381235ab1f6d0 Mon Sep 17 00:00:00 2001 From: rohan-chaturvedi Date: Tue, 27 Oct 2026 11:20:59 +0530 Subject: [PATCH 09/11] fix(ai): catch Invoke-WebRequest calls inside quoted PowerShell scripts The guard's check for PowerShell web cmdlets accepted a cmdlet only after whitespace or a shell operator, so `pwsh -Command "iwr http://phase.invalid/..."` slipped past it: the Agent got a failed host lookup instead of the message pointing it at curl. A quote now also counts as a word edge. --- src/cmd/ai_shell_scope.go | 5 +++-- src/cmd/ai_shell_scope_test.go | 3 +++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/cmd/ai_shell_scope.go b/src/cmd/ai_shell_scope.go index 42466d52..b527368f 100644 --- a/src/cmd/ai_shell_scope.go +++ b/src/cmd/ai_shell_scope.go @@ -11,8 +11,9 @@ import ( const phaseShellRedirect = "\x00redirect" // powerShellWebCmdlet matches PowerShell's web cmdlets and their aliases as -// whole words, not inside names such as "confirm" or "$irm". -var powerShellWebCmdlet = regexp.MustCompile(`(?i)(^|[\s;|&({=])(invoke-webrequest|invoke-restmethod|iwr|irm)($|[\s;|)}])`) +// whole words, including at the edge of a quoted script such as +// pwsh -Command "iwr ...", but not inside names such as "confirm" or "$irm". +var powerShellWebCmdlet = regexp.MustCompile(`(?i)(^|[\s;|&({="'])(invoke-webrequest|invoke-restmethod|iwr|irm)($|[\s;|)}"'])`) // shellCommandUsesPowerShellWeb reports a PowerShell web cmdlet in a command // that names phase.invalid. Invoke-WebRequest and Invoke-RestMethod ignore diff --git a/src/cmd/ai_shell_scope_test.go b/src/cmd/ai_shell_scope_test.go index 96d1d722..4b88c0d0 100644 --- a/src/cmd/ai_shell_scope_test.go +++ b/src/cmd/ai_shell_scope_test.go @@ -70,6 +70,9 @@ func TestShellCommandUsesPowerShellWeb(t *testing.T) { {command: "$r = Invoke-WebRequest -Uri http://phase.invalid/v1/status -UseBasicParsing", want: true}, {command: "(iwr http://phase.invalid/v1/status).Content", want: true}, {command: `$u = "http://phase.invalid/v1/status"; IRM $u`, want: true}, + {command: `pwsh -NoProfile -Command "iwr http://phase.invalid/v1/status"`, want: true}, + {command: `powershell -c 'Invoke-RestMethod http://phase.invalid/v1/status'`, want: true}, + {command: `Write-Output "irmx" phase.invalid`, want: false}, {command: "curl.exe -sS http://phase.invalid/v1/status", want: false}, {command: "curl --fail http://phase.invalid/v1/status", want: false}, {command: "Invoke-RestMethod https://api.github.com/user", want: false}, From 7269d9ff84bc399c8b466867e042212f44579dc9 Mon Sep 17 00:00:00 2001 From: rohan-chaturvedi Date: Tue, 27 Oct 2026 11:32:22 +0530 Subject: [PATCH 10/11] fix(ai): leave an up-to-date Codex adapter alone when enabling another harness phase ai enable refreshes every configured harness, so enabling Cursor or OpenCode on a machine where Codex was enabled earlier still went through Codex. With the Codex CLI on PATH it reactivated an unchanged plugin and asked to re-trust its hooks; without it, it warned that Codex was not refreshed. When another harness is selected, enable now compares the Codex plugin files with what this build would write (ai.CodexBootstrapPluginCurrent, which needs no Codex CLI) and leaves an up-to-date adapter alone, with no CLI call and no Codex notes in the summary. An outdated adapter is still refreshed when the CLI is available, and is otherwise reported as out of date. --- src/cmd/ai_enable.go | 18 +++++++----- src/cmd/ai_enable_test.go | 19 +++++++++++-- src/pkg/ai/claude_plugin.go | 12 ++++++-- src/pkg/ai/codex_plugin.go | 49 +++++++++++++++++++++++++-------- src/pkg/ai/codex_plugin_test.go | 13 +++++++++ 5 files changed, 89 insertions(+), 22 deletions(-) diff --git a/src/cmd/ai_enable.go b/src/cmd/ai_enable.go index ccc69161..c6cf9608 100644 --- a/src/cmd/ai_enable.go +++ b/src/cmd/ai_enable.go @@ -326,7 +326,7 @@ func formatAIEnableSummary(target ai.SkillTarget, cfg *config.AIConfig, principa } if reason := skipped[ai.HarnessCodex]; reason != "" { out.WriteString("\n") - out.WriteString(util.BoldYellow("⚠️ Codex: its Phase plugin was not refreshed, because "+reason+".") + "\n") + out.WriteString(util.BoldYellow("⚠️ Codex: its Phase plugin is out of date and was not refreshed, because "+reason+".") + "\n") out.WriteString(" Rerun `phase ai enable --harness codex` with the Codex CLI on PATH to refresh it.\n") } if target.ID != "" { @@ -398,10 +398,11 @@ func claudeHookExecutablePath() (string, error) { // installConfiguredAIHarnessAdapters refreshes one major-versioned helper from // the current CLI, then points every thin harness adapter at it. Provider and -// discovery changes do not require reinstalling those adapters. A configured -// Codex adapter is left as it was when the Codex CLI is not on PATH, unless -// Codex is the selected harness, so adding another harness does not depend on -// Codex; the second result maps each skipped harness to the reason. +// discovery changes do not require reinstalling those adapters. When another +// harness is selected, a Codex adapter that is already up to date is left +// alone, and an outdated one is left as it was if the Codex CLI is not on PATH, +// so adding another harness does not depend on Codex; the second result maps +// each harness left outdated to the reason. func installConfiguredAIHarnessAdapters(ctx context.Context, harnesses []string, selected, executable string) (map[string]string, map[string]string, error) { installed := make(map[string]string, len(harnesses)) skipped := make(map[string]string) @@ -442,10 +443,13 @@ func installConfiguredAIHarnessAdapters(ctx context.Context, harnesses []string, if err != nil { return nil, nil, err } + if harness != selected && ai.CodexBootstrapPluginCurrent(path, hookExecutable) { + // It already runs the shared helper refreshed above, and + // refreshing it would change nothing Codex runs. + continue + } codexExecutable, err := exec.LookPath("codex") if err != nil && harness != selected { - // The adapter from an earlier enable still runs the shared - // helper refreshed above. skipped[harness] = "the Codex CLI is not on PATH" continue } diff --git a/src/cmd/ai_enable_test.go b/src/cmd/ai_enable_test.go index 1e756820..de563d18 100644 --- a/src/cmd/ai_enable_test.go +++ b/src/cmd/ai_enable_test.go @@ -142,7 +142,7 @@ func TestCodexHookReviewRequiredWhenExistingAdapterIsRefreshed(t *testing.T) { } } -func TestInstallConfiguredAIHarnessAdaptersSkipsCodexWithoutItsCLIUnlessSelected(t *testing.T) { +func TestInstallConfiguredAIHarnessAdaptersDoNotNeedCodexForAnotherHarness(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) t.Setenv("USERPROFILE", home) @@ -163,6 +163,21 @@ func TestInstallConfiguredAIHarnessAdaptersSkipsCodexWithoutItsCLIUnlessSelected if _, _, err := installConfiguredAIHarnessAdapters(context.Background(), harnesses, ai.HarnessCodex, source); err == nil || !strings.Contains(err.Error(), "PATH") { t.Fatalf("enabling Codex itself must still require its CLI, got %v", err) } + // Once the Codex adapter is up to date, adding Cursor leaves it alone. + root, err := ai.DefaultCodexBootstrapMarketplacePath() + if err != nil { + t.Fatal(err) + } + helper, err := ai.DefaultSharedHookExecutablePath() + if err != nil { + t.Fatal(err) + } + if _, err := ai.InstallCodexBootstrapPlugin(root, helper); err != nil { + t.Fatal(err) + } + if installed, skipped, err := installConfiguredAIHarnessAdapters(context.Background(), harnesses, ai.HarnessCursor, source); err != nil || len(skipped) != 0 || installed[ai.HarnessCodex] != "" { + t.Fatalf("an up-to-date Codex adapter was touched or reported: installed=%v skipped=%v err=%v", installed, skipped, err) + } } func TestAIEnableHarnessRestartMessagesRepeatOriginalPrompt(t *testing.T) { @@ -254,7 +269,7 @@ func TestFormatAIEnableSummaryReportsASkippedCodexRefresh(t *testing.T) { summary := formatAIEnableSummary(cursor, &config.AIConfig{Enabled: true}, nil, map[string]string{ai.HarnessCursor: "Cursor Phase discovery adapter installed"}, map[string]string{ai.HarnessCodex: "the Codex CLI is not on PATH"}) - if !strings.Contains(summary, "⚠️ Codex: its Phase plugin was not refreshed, because the Codex CLI is not on PATH.") || + if !strings.Contains(summary, "⚠️ Codex: its Phase plugin is out of date and was not refreshed, because the Codex CLI is not on PATH.") || !strings.Contains(summary, "`phase ai enable --harness codex`") || strings.Contains(summary, "refreshed too") { t.Fatalf("a skipped Codex refresh must be reported, without asking to re-trust hooks that did not change:\n%s", summary) } diff --git a/src/pkg/ai/claude_plugin.go b/src/pkg/ai/claude_plugin.go index 532ab2d4..3cb2e898 100644 --- a/src/pkg/ai/claude_plugin.go +++ b/src/pkg/ai/claude_plugin.go @@ -186,12 +186,20 @@ func UninstallClaudeBootstrapPlugin(root string) error { return nil } -func writeJSONFile(path string, value any, mode os.FileMode) error { +// jsonFilePayload is exactly what writeJSONFile writes for value. +func jsonFilePayload(value any) ([]byte, error) { payload, err := json.MarshalIndent(value, "", " ") + if err != nil { + return nil, err + } + return append(payload, '\n'), nil +} + +func writeJSONFile(path string, value any, mode os.FileMode) error { + payload, err := jsonFilePayload(value) if err != nil { return err } - payload = append(payload, '\n') temporary, err := os.CreateTemp(filepath.Dir(path), ".phase-plugin-*") if err != nil { return err diff --git a/src/pkg/ai/codex_plugin.go b/src/pkg/ai/codex_plugin.go index aad94220..0a447907 100644 --- a/src/pkg/ai/codex_plugin.go +++ b/src/pkg/ai/codex_plugin.go @@ -175,9 +175,24 @@ func InstallCodexBootstrapPlugin(root, phaseExecutable string) (CodexBootstrapIn installation.PluginVersion = HarnessAdapterProtocolVersion hooks := codexBootstrapHooks(hookExecutable) - manifest := codexPluginManifest{ + manifest := codexBootstrapManifest() + if err := writeJSONFile(filepath.Join(manifestDirectory, "plugin.json"), manifest, 0o600); err != nil { + return installation, fmt.Errorf("write Codex bootstrap plugin manifest: %w", err) + } + if err := writeJSONFile(filepath.Join(hooksDirectory, "hooks.json"), hooks, 0o600); err != nil { + return installation, fmt.Errorf("write Codex bootstrap hooks: %w", err) + } + if err := upsertCodexMarketplace(root); err != nil { + return installation, err + } + return installation, nil +} + +// codexBootstrapManifest is the plugin.json InstallCodexBootstrapPlugin writes. +func codexBootstrapManifest() codexPluginManifest { + return codexPluginManifest{ Name: CodexBootstrapPluginName, - Version: installation.PluginVersion, + Version: HarnessAdapterProtocolVersion, Description: "Makes explicitly selected Phase-managed Connections available to Codex.", Author: codexPluginAuthor{Name: "Phase"}, Homepage: "https://phase.dev", @@ -197,16 +212,28 @@ func InstallCodexBootstrapPlugin(root, phaseExecutable string) (CodexBootstrapIn }, }, } - if err := writeJSONFile(filepath.Join(manifestDirectory, "plugin.json"), manifest, 0o600); err != nil { - return installation, fmt.Errorf("write Codex bootstrap plugin manifest: %w", err) - } - if err := writeJSONFile(filepath.Join(hooksDirectory, "hooks.json"), hooks, 0o600); err != nil { - return installation, fmt.Errorf("write Codex bootstrap hooks: %w", err) - } - if err := upsertCodexMarketplace(root); err != nil { - return installation, err +} + +// CodexBootstrapPluginCurrent reports whether the plugin under root already +// holds exactly the manifest and hooks InstallCodexBootstrapPlugin would write +// for phaseExecutable, so refreshing it would change nothing Codex runs. It +// needs no Codex CLI. +func CodexBootstrapPluginCurrent(root, phaseExecutable string) bool { + pluginRoot := filepath.Join(root, "plugins", CodexBootstrapPluginName) + for path, value := range map[string]any{ + filepath.Join(pluginRoot, ".codex-plugin", "plugin.json"): codexBootstrapManifest(), + filepath.Join(pluginRoot, "hooks", "hooks.json"): codexBootstrapHooks(phaseExecutable), + } { + want, err := jsonFilePayload(value) + if err != nil { + return false + } + got, err := os.ReadFile(path) + if err != nil || !bytes.Equal(got, want) { + return false + } } - return installation, nil + return true } // ActivateCodexBootstrapPlugin registers the Phase-owned marketplace and diff --git a/src/pkg/ai/codex_plugin_test.go b/src/pkg/ai/codex_plugin_test.go index 96ac7a90..06bed9da 100644 --- a/src/pkg/ai/codex_plugin_test.go +++ b/src/pkg/ai/codex_plugin_test.go @@ -368,6 +368,19 @@ func TestCodexBootstrapPluginRefusesUnownedPaths(t *testing.T) { } } +func TestCodexBootstrapPluginCurrentMatchesWhatInstallWrites(t *testing.T) { + root, executable := installCodexTestPlugin(t) + if !CodexBootstrapPluginCurrent(root, executable) { + t.Fatal("the plugin InstallCodexBootstrapPlugin just wrote is not current") + } + if CodexBootstrapPluginCurrent(root, executable+"-moved") { + t.Fatal("hooks that run another helper path are current") + } + if CodexBootstrapPluginCurrent(filepath.Join(root, "missing"), executable) { + t.Fatal("a missing plugin is current") + } +} + func installCodexTestPlugin(t *testing.T) (string, string) { t.Helper() parent := t.TempDir() From ccd687b8c5a0c3f039344e1432756fbc2580bf17 Mon Sep 17 00:00:00 2001 From: Nimish Date: Tue, 27 Oct 2026 13:28:47 +0530 Subject: [PATCH 11/11] refactor(ai): trim the Windows harness fixes - One PowerShell quoting helper (ai.PowerShellCallCommand) serves both the guard's wrapper and the hook fallback, so the fallback now doubles typographic single quotes too; Devin recognizes that spelling on reinstall. - Drop the %AppData% OpenCode plugin cleanup: no released build installed the plugin there. - CodexBootstrapPluginCurrent compares the plugin version and hooks without reshaping InstallCodexBootstrapPlugin. - Shorter comments. --- src/cmd/ai_claude_environment.go | 36 ++++++-------- src/cmd/ai_claude_environment_test.go | 8 +--- src/cmd/ai_disable.go | 7 --- src/cmd/ai_enable.go | 13 +---- src/cmd/ai_harness_hook.go | 30 ++---------- src/cmd/ai_harness_hook_test.go | 3 +- src/cmd/ai_shell_scope.go | 12 ++--- src/pkg/ai/claude_plugin.go | 12 +---- src/pkg/ai/codex_plugin.go | 55 +++++++++------------- src/pkg/ai/opencode_plugin.go | 21 --------- src/pkg/ai/opencode_plugin_test.go | 34 ------------- src/pkg/ai/windows_command.go | 52 +++++++++++--------- src/pkg/ai/windows_command_test.go | 13 +++-- src/pkg/ai/windows_command_windows.go | 4 +- src/pkg/ai/windows_command_windows_test.go | 8 +--- src/pkg/proxy/environment.go | 8 ++-- src/pkg/proxy/store_test.go | 2 - 17 files changed, 94 insertions(+), 224 deletions(-) diff --git a/src/cmd/ai_claude_environment.go b/src/cmd/ai_claude_environment.go index a9ec1866..1fb456b5 100644 --- a/src/cmd/ai_claude_environment.go +++ b/src/cmd/ai_claude_environment.go @@ -39,8 +39,7 @@ const ( claudeHookKickBudget = 1500 * time.Millisecond ) -// claudeEnvironmentGOOS selects the liveness guard renderClaudeEnvironment -// writes; tests override it to render another platform's file. +// claudeEnvironmentGOOS picks the liveness guard; tests override it. var claudeEnvironmentGOOS = runtime.GOOS type claudeEnvironmentStatus struct { @@ -78,11 +77,11 @@ type claudeRuntimeHealth struct { // renderClaudeEnvironment produces the POSIX shell a harness sources before each // command. Proxy routing, Phase runtime markers, and trust overrides live inside -// a liveness guard on the issuing proxy (see claudeProxyLivenessGuard), so a -// crashed proxy can never strand a shell behind a dead port. Decoys, adapter -// endpoints, and explicit unsets stay outside the guard: while Phase AI is -// enabled, provider tools either reach the Phase proxy or fail on the decoy. -// They never fall through to an ambient credential by accident. +// a liveness guard on the issuing proxy, so a crashed proxy can never strand +// a shell behind a dead port. Decoys, adapter endpoints, and explicit unsets +// stay outside the guard: while Phase AI is enabled, provider tools either reach +// the Phase proxy or fail on the decoy. They never fall through to an ambient +// credential by accident. func renderClaudeEnvironment(pid int, plan phaseproxy.ProcessEnvironmentPlan, proxyLive bool) (string, error) { if proxyLive && pid <= 0 { return "", errors.New("Phase AI proxy PID is required for a live environment") @@ -148,12 +147,8 @@ func renderClaudeEnvironment(pid int, plan phaseproxy.ProcessEnvironmentPlan, pr return out.String(), nil } -// claudeProxyLivenessGuard returns the shell condition that keeps proxy routing -// on only while the issuing proxy is alive. POSIX shells signal its PID. On -// Windows the sourcing shell is Git Bash (MSYS2), whose kill cannot see native -// Windows PIDs, so the guard checks that the proxy still accepts connections -// instead; hooks retire routing for a dead daemon through the Win32 liveness -// check in reconcileClaudeEnvironment. +// claudeProxyLivenessGuard keeps routing on only while the proxy is alive. Git +// Bash cannot signal native Windows PIDs, so on Windows it probes the port. func claudeProxyLivenessGuard(pid int, plan phaseproxy.ProcessEnvironmentPlan) (string, error) { if claudeEnvironmentGOOS != "windows" { return fmt.Sprintf("kill -0 %d 2>/dev/null", pid), nil @@ -165,22 +160,19 @@ func claudeProxyLivenessGuard(pid int, plan phaseproxy.ProcessEnvironmentPlan) ( } } if raw == "" { - // Nothing in this plan routes through the proxy. return "false", nil } proxyURL, err := url.Parse(raw) if err != nil { return "", errors.New("Phase AI proxy URL is invalid") } - host, port := proxyURL.Hostname(), proxyURL.Port() - if address := net.ParseIP(host); address == nil || address.To4() == nil { - return "", fmt.Errorf("Phase AI proxy host %q is not an IPv4 address", host) + host, port, err := net.SplitHostPort(proxyURL.Host) + number, portErr := strconv.Atoi(port) + if err != nil || portErr != nil || net.ParseIP(host).To4() == nil || number < 1 || number > 65535 { + return "", fmt.Errorf("Phase AI proxy address %q is not an IPv4 address and port", proxyURL.Host) } - if number, err := strconv.Atoi(port); err != nil || number < 1 || number > 65535 { - return "", fmt.Errorf("Phase AI proxy port %q is invalid", port) - } - // `true` is a regular builtin: a refused connection only fails the test, it - // never ends a non-interactive shell, and the probe needs no subshell. + // true is a regular builtin, so a refused connection fails the test without + // ending a non-interactive shell. return fmt.Sprintf("true 2>/dev/null /dev/null 0 { - installed[harness] += " (removed an older copy OpenCode never loads: " + strings.Join(removed, ", ") + ")" - } case ai.HarnessDevin: path, err := ai.DefaultDevinConfigPath() if err != nil { diff --git a/src/cmd/ai_harness_hook.go b/src/cmd/ai_harness_hook.go index cfae49b9..50d3b994 100644 --- a/src/cmd/ai_harness_hook.go +++ b/src/cmd/ai_harness_hook.go @@ -705,13 +705,11 @@ func buildHarnessWrapperCommand(harness, leaseID, original string) (string, erro // harnessWrapperCommandLine quotes the wrapper for the shell the harness runs // it in. On Windows, Codex uses PowerShell, which needs the call operator to // run a quoted executable path; Cursor uses its default terminal profile. -// Other harnesses may run it in PowerShell too, so they get the unquoted form -// every Windows shell runs, and the double-quoted cmd form only when the path -// cannot be written unquoted. +// Other harnesses get the plain form every Windows shell runs when there is one. func harnessWrapperCommandLine(goos, harness string, arguments []string) string { if goos == "windows" { if harness == ai.HarnessCodex || (harness == ai.HarnessCursor && !cursorWindowsShellIsBashOrCmd(cursorWindowsSettings())) { - return quotePowerShellCommand(arguments) + return ai.PowerShellCallCommand(arguments) } if line, ok := ai.WindowsPlainCommand(arguments); ok { return line @@ -888,25 +886,6 @@ func stripJSONTrailingCommas(source []byte) []byte { return out } -// quotePowerShellCommand renders `& 'exe' 'arg' ...`. PowerShell treats the -// typographic single quotes as quote characters too, so each is doubled. -func quotePowerShellCommand(arguments []string) string { - quoted := make([]string, len(arguments)) - for index, argument := range arguments { - var builder strings.Builder - builder.WriteByte('\'') - for _, character := range argument { - if character == '\'' || character == '\u2018' || character == '\u2019' || character == '\u201A' || character == '\u201B' { - builder.WriteRune(character) - } - builder.WriteRune(character) - } - builder.WriteByte('\'') - quoted[index] = builder.String() - } - return "& " + strings.Join(quoted, " ") -} - func quoteWindowsCommand(arguments []string) string { quoted := make([]string, len(arguments)) for index, argument := range arguments { @@ -1249,9 +1228,8 @@ func startHarnessCommandLeaseRenewal(done <-chan struct{}, interval time.Duratio return renewal.Wait } -// windowsPowerShellPrelude makes curl and wget run the real executables. -// Windows PowerShell aliases both to Invoke-WebRequest, which ignores -// HTTP(S)_PROXY, so a wrapped phase.invalid call would miss the Phase proxy. +// windowsPowerShellPrelude drops Windows PowerShell's curl and wget aliases of +// Invoke-WebRequest, which ignores HTTP(S)_PROXY. const windowsPowerShellPrelude = "Remove-Item -Path Alias:curl, Alias:wget -Force -ErrorAction SilentlyContinue; " func executeHarnessShell(ctx context.Context, command string, environment []string, stdin io.Reader, stdout, stderr io.Writer) error { diff --git a/src/cmd/ai_harness_hook_test.go b/src/cmd/ai_harness_hook_test.go index 3b26931b..dcf2d949 100644 --- a/src/cmd/ai_harness_hook_test.go +++ b/src/cmd/ai_harness_hook_test.go @@ -1336,7 +1336,7 @@ func TestHarnessWrapperCommandLineMatchesTheHarnessShell(t *testing.T) { t.Fatalf("%s on Windows:\n got %s\nwant %s", harness, got, want) } } - if got := quotePowerShellCommand([]string{"a\u2019b"}); got != "& 'a\u2019\u2019b'" { + if got := ai.PowerShellCallCommand([]string{"a\u2019b"}); got != "& 'a\u2019\u2019b'" { t.Fatalf("typographic quote not doubled: %s", got) } if got := harnessWrapperCommandLine("windows", ai.HarnessDevin, arguments); !strings.HasPrefix(got, `"C:\Users`) { @@ -1344,7 +1344,6 @@ func TestHarnessWrapperCommandLineMatchesTheHarnessShell(t *testing.T) { } plain := []string{`C:\Users\dev\.phase\ai\bin\phase-hook-v1.exe`, "ai", "__harness-exec", "--lease", "Y29kZXg6YQ"} for _, harness := range []string{ai.HarnessDevin, ai.HarnessOpenCode} { - // Their Windows shell may be PowerShell, which cannot run the cmd form. if got, want := harnessWrapperCommandLine("windows", harness, plain), "C:/Users/dev/.phase/ai/bin/phase-hook-v1.exe ai __harness-exec --lease Y29kZXg6YQ"; got != want { t.Fatalf("%s on Windows:\n got %s\nwant %s", harness, got, want) } diff --git a/src/cmd/ai_shell_scope.go b/src/cmd/ai_shell_scope.go index b527368f..1dd6ec04 100644 --- a/src/cmd/ai_shell_scope.go +++ b/src/cmd/ai_shell_scope.go @@ -10,17 +10,11 @@ import ( const phaseShellRedirect = "\x00redirect" -// powerShellWebCmdlet matches PowerShell's web cmdlets and their aliases as -// whole words, including at the edge of a quoted script such as -// pwsh -Command "iwr ...", but not inside names such as "confirm" or "$irm". var powerShellWebCmdlet = regexp.MustCompile(`(?i)(^|[\s;|&({="'])(invoke-webrequest|invoke-restmethod|iwr|irm)($|[\s;|)}"'])`) -// shellCommandUsesPowerShellWeb reports a PowerShell web cmdlet in a command -// that names phase.invalid. Invoke-WebRequest and Invoke-RestMethod ignore -// HTTP(S)_PROXY in Windows PowerShell, so such a call can never reach the Phase -// proxy and only fails to resolve the host; hooks reject it and point the Agent -// at curl, which they route. The check spans the whole command so a URL held -// in a variable ($u = "http://phase.invalid/..."; irm $u) is caught too. +// shellCommandUsesPowerShellWeb reports a PowerShell web cmdlet, as a whole +// word, in a command naming phase.invalid. Windows PowerShell's web cmdlets +// ignore HTTP(S)_PROXY, so hooks deny the call and point the Agent at curl. func shellCommandUsesPowerShellWeb(command string) bool { return strings.Contains(strings.ToLower(command), "phase.invalid") && powerShellWebCmdlet.MatchString(command) } diff --git a/src/pkg/ai/claude_plugin.go b/src/pkg/ai/claude_plugin.go index 3cb2e898..532ab2d4 100644 --- a/src/pkg/ai/claude_plugin.go +++ b/src/pkg/ai/claude_plugin.go @@ -186,20 +186,12 @@ func UninstallClaudeBootstrapPlugin(root string) error { return nil } -// jsonFilePayload is exactly what writeJSONFile writes for value. -func jsonFilePayload(value any) ([]byte, error) { - payload, err := json.MarshalIndent(value, "", " ") - if err != nil { - return nil, err - } - return append(payload, '\n'), nil -} - func writeJSONFile(path string, value any, mode os.FileMode) error { - payload, err := jsonFilePayload(value) + payload, err := json.MarshalIndent(value, "", " ") if err != nil { return err } + payload = append(payload, '\n') temporary, err := os.CreateTemp(filepath.Dir(path), ".phase-plugin-*") if err != nil { return err diff --git a/src/pkg/ai/codex_plugin.go b/src/pkg/ai/codex_plugin.go index 0a447907..75fcffea 100644 --- a/src/pkg/ai/codex_plugin.go +++ b/src/pkg/ai/codex_plugin.go @@ -175,24 +175,9 @@ func InstallCodexBootstrapPlugin(root, phaseExecutable string) (CodexBootstrapIn installation.PluginVersion = HarnessAdapterProtocolVersion hooks := codexBootstrapHooks(hookExecutable) - manifest := codexBootstrapManifest() - if err := writeJSONFile(filepath.Join(manifestDirectory, "plugin.json"), manifest, 0o600); err != nil { - return installation, fmt.Errorf("write Codex bootstrap plugin manifest: %w", err) - } - if err := writeJSONFile(filepath.Join(hooksDirectory, "hooks.json"), hooks, 0o600); err != nil { - return installation, fmt.Errorf("write Codex bootstrap hooks: %w", err) - } - if err := upsertCodexMarketplace(root); err != nil { - return installation, err - } - return installation, nil -} - -// codexBootstrapManifest is the plugin.json InstallCodexBootstrapPlugin writes. -func codexBootstrapManifest() codexPluginManifest { - return codexPluginManifest{ + manifest := codexPluginManifest{ Name: CodexBootstrapPluginName, - Version: HarnessAdapterProtocolVersion, + Version: installation.PluginVersion, Description: "Makes explicitly selected Phase-managed Connections available to Codex.", Author: codexPluginAuthor{Name: "Phase"}, Homepage: "https://phase.dev", @@ -212,28 +197,30 @@ func codexBootstrapManifest() codexPluginManifest { }, }, } + if err := writeJSONFile(filepath.Join(manifestDirectory, "plugin.json"), manifest, 0o600); err != nil { + return installation, fmt.Errorf("write Codex bootstrap plugin manifest: %w", err) + } + if err := writeJSONFile(filepath.Join(hooksDirectory, "hooks.json"), hooks, 0o600); err != nil { + return installation, fmt.Errorf("write Codex bootstrap hooks: %w", err) + } + if err := upsertCodexMarketplace(root); err != nil { + return installation, err + } + return installation, nil } -// CodexBootstrapPluginCurrent reports whether the plugin under root already -// holds exactly the manifest and hooks InstallCodexBootstrapPlugin would write -// for phaseExecutable, so refreshing it would change nothing Codex runs. It -// needs no Codex CLI. +// CodexBootstrapPluginCurrent reports, without the Codex CLI, whether root holds +// this build's plugin version and hooks for phaseExecutable. func CodexBootstrapPluginCurrent(root, phaseExecutable string) bool { pluginRoot := filepath.Join(root, "plugins", CodexBootstrapPluginName) - for path, value := range map[string]any{ - filepath.Join(pluginRoot, ".codex-plugin", "plugin.json"): codexBootstrapManifest(), - filepath.Join(pluginRoot, "hooks", "hooks.json"): codexBootstrapHooks(phaseExecutable), - } { - want, err := jsonFilePayload(value) - if err != nil { - return false - } - got, err := os.ReadFile(path) - if err != nil || !bytes.Equal(got, want) { - return false - } + var manifest codexPluginManifest + payload, err := os.ReadFile(filepath.Join(pluginRoot, ".codex-plugin", "plugin.json")) + if err != nil || json.Unmarshal(payload, &manifest) != nil || manifest.Version != HarnessAdapterProtocolVersion { + return false } - return true + want, err := json.MarshalIndent(codexBootstrapHooks(phaseExecutable), "", " ") + got, readErr := os.ReadFile(filepath.Join(pluginRoot, "hooks", "hooks.json")) + return err == nil && readErr == nil && bytes.Equal(bytes.TrimSpace(got), want) } // ActivateCodexBootstrapPlugin registers the Phase-owned marketplace and diff --git a/src/pkg/ai/opencode_plugin.go b/src/pkg/ai/opencode_plugin.go index f7eb2511..3a18cd0d 100644 --- a/src/pkg/ai/opencode_plugin.go +++ b/src/pkg/ai/opencode_plugin.go @@ -6,7 +6,6 @@ import ( "fmt" "os" "path/filepath" - "runtime" "strings" ) @@ -33,26 +32,6 @@ func DefaultOpenCodePluginPath() (string, error) { return filepath.Join(home, ".config", "opencode", "plugins", OpenCodePluginName+".js"), nil } -// RemoveLegacyOpenCodePlugins removes Phase-owned copies of the plugin that -// earlier builds installed where OpenCode never loads them (%AppData% on -// Windows). Missing and foreign files are left alone. It returns the paths it -// removed. -func RemoveLegacyOpenCodePlugins() ([]string, error) { - if runtime.GOOS != "windows" { - return nil, nil - } - configDirectory, err := os.UserConfigDir() - if err != nil { - return nil, nil - } - legacy := filepath.Join(configDirectory, "opencode", "plugins", OpenCodePluginName+".js") - removed, err := removeOwnedOpenCodePluginFile(legacy, false) - if err != nil || !removed { - return nil, err - } - return []string{legacy}, nil -} - // InstallOpenCodePlugin installs a dependency-free, provider-agnostic plugin. // It gives every shell OpenCode spawns the Phase AI environment published by // the hook-owned proxy daemon (environmentPath, a JSON twin of the file Claude diff --git a/src/pkg/ai/opencode_plugin_test.go b/src/pkg/ai/opencode_plugin_test.go index 172c15ec..cf3b96d7 100644 --- a/src/pkg/ai/opencode_plugin_test.go +++ b/src/pkg/ai/opencode_plugin_test.go @@ -343,37 +343,3 @@ func TestDefaultOpenCodePluginPathUsesDotConfigOnEveryPlatform(t *testing.T) { t.Fatalf("plugin path = %s, want %s", path, want) } } - -func TestRemoveLegacyOpenCodePluginsRemovesOnlyOwnedWindowsCopies(t *testing.T) { - if runtime.GOOS != "windows" { - t.Skip("earlier builds used %AppData% only on Windows") - } - appData := t.TempDir() - t.Setenv("APPDATA", appData) - legacy := filepath.Join(appData, "opencode", "plugins", OpenCodePluginName+".js") - if removed, err := RemoveLegacyOpenCodePlugins(); err != nil || len(removed) != 0 { - t.Fatalf("missing legacy plugin: removed=%v err=%v", removed, err) - } - if err := os.MkdirAll(filepath.Dir(legacy), 0o700); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(legacy, []byte("// a user's own plugin\n"), 0o600); err != nil { - t.Fatal(err) - } - if removed, err := RemoveLegacyOpenCodePlugins(); err != nil || len(removed) != 0 { - t.Fatalf("foreign legacy plugin: removed=%v err=%v", removed, err) - } - if _, err := os.Stat(legacy); err != nil { - t.Fatalf("foreign plugin was not preserved: %v", err) - } - if err := os.WriteFile(legacy, []byte(openCodeOwnershipMarker+"\nexport default {}\n"), 0o600); err != nil { - t.Fatal(err) - } - removed, err := RemoveLegacyOpenCodePlugins() - if err != nil || len(removed) != 1 || removed[0] != legacy { - t.Fatalf("owned legacy plugin: removed=%v err=%v", removed, err) - } - if _, err := os.Stat(legacy); !os.IsNotExist(err) { - t.Fatalf("owned legacy plugin still exists: %v", err) - } -} diff --git a/src/pkg/ai/windows_command.go b/src/pkg/ai/windows_command.go index 4cc8020d..1f7f06ae 100644 --- a/src/pkg/ai/windows_command.go +++ b/src/pkg/ai/windows_command.go @@ -5,28 +5,18 @@ import "strings" // shortPathName resolves a path's 8.3 short name; tests replace it. var shortPathName = windowsShortPathName -// WindowsShellCommand renders a command line that runs the same way in every -// shell a harness may use on Windows: PowerShell, cmd, Git Bash, or a direct -// CreateProcess. That is the WindowsPlainCommand form when there is one, and -// otherwise the PowerShell call-operator form, PowerShell being the default -// shell of the Windows harnesses. +// WindowsShellCommand renders the WindowsPlainCommand form, or PowerShell's +// call-operator form when there is none. func WindowsShellCommand(arguments []string) string { if line, ok := WindowsPlainCommand(arguments); ok { return line } - quoted := make([]string, len(arguments)) - for index, argument := range arguments { - quoted[index] = "'" + strings.ReplaceAll(argument, "'", "''") + "'" - } - return "& " + strings.Join(quoted, " ") + return PowerShellCallCommand(arguments) } -// WindowsPlainCommand renders the only command line form that PowerShell, cmd -// and Git Bash all parse as a command: an unquoted path with forward slashes -// followed by plain words. PowerShell reads a leading quoted string as an -// expression, and cmd and bash reject PowerShell's & call operator. A path with -// spaces or shell characters uses its 8.3 short name; it reports false when a -// word still needs quoting. +// WindowsPlainCommand renders the one form PowerShell, cmd and Git Bash all +// run: an unquoted forward-slash path, or its 8.3 name, and plain words. It +// reports false when a word still needs quoting. func WindowsPlainCommand(arguments []string) (string, bool) { words := append([]string{strings.ReplaceAll(arguments[0], `\`, "/")}, arguments[1:]...) if !windowsShellPlainWord(words[0]) { @@ -42,19 +32,37 @@ func WindowsPlainCommand(arguments []string) (string, bool) { return strings.Join(words, " "), true } -// windowsExecutableForms lists the spellings of executable that a rendered -// command may contain, including the backslash form earlier builds quoted, so -// hook entries written before WindowsShellCommand are still recognized. +// PowerShellCallCommand renders `& 'exe' 'arg' ...`. PowerShell treats the +// typographic single quotes as quote characters too, so each is doubled. +func PowerShellCallCommand(arguments []string) string { + quoted := make([]string, len(arguments)) + for index, argument := range arguments { + quoted[index] = "'" + powerShellQuoteEscape(argument) + "'" + } + return "& " + strings.Join(quoted, " ") +} + +func powerShellQuoteEscape(value string) string { + var builder strings.Builder + for _, character := range value { + if character == '\'' || character == '\u2018' || character == '\u2019' || character == '\u201A' || character == '\u201B' { + builder.WriteRune(character) + } + builder.WriteRune(character) + } + return builder.String() +} + +// windowsExecutableForms lists the spellings of executable in a rendered command. func windowsExecutableForms(executable string) []string { - forms := []string{executable, strings.ReplaceAll(executable, `\`, "/")} + forms := []string{executable, strings.ReplaceAll(executable, `\`, "/"), powerShellQuoteEscape(executable)} if short, err := shortPathName(executable); err == nil { forms = append(forms, strings.ReplaceAll(short, `\`, "/")) } return forms } -// windowsShellPlainWord reports a word that PowerShell, cmd, and POSIX shells -// all pass through unquoted and unexpanded. +// windowsShellPlainWord reports a word every shell passes through unchanged. func windowsShellPlainWord(word string) bool { if word == "" || word[0] == '~' { return false diff --git a/src/pkg/ai/windows_command_test.go b/src/pkg/ai/windows_command_test.go index 5e6c70b1..ce83012f 100644 --- a/src/pkg/ai/windows_command_test.go +++ b/src/pkg/ai/windows_command_test.go @@ -17,7 +17,6 @@ func withShortPathName(t *testing.T, resolve func(string) (string, error)) { func TestWindowsShellCommandRunsEverywhereOrFallsBack(t *testing.T) { arguments := []string{`C:\Users\dev\.phase\ai\bin\phase-hook-v1.exe`, "ai", "__codex-event", "guard"} withShortPathName(t, func(string) (string, error) { return "", errors.New("not needed") }) - // PowerShell, cmd and Git Bash all run an unquoted forward-slash path. if got, want := WindowsShellCommand(arguments), "C:/Users/dev/.phase/ai/bin/phase-hook-v1.exe ai __codex-event guard"; got != want { t.Fatalf("plain path command = %q, want %q", got, want) } @@ -39,13 +38,15 @@ func TestWindowsShellCommandRunsEverywhereOrFallsBack(t *testing.T) { if line, ok := WindowsPlainCommand(quoted); ok { t.Fatalf("a path that still needs quoting has no plain form, got %q", line) } + typographic := []string{"C:\\Users\\O\u2019Brien\\phase-hook-v1.exe", "ai"} + if got, want := WindowsShellCommand(typographic), "& 'C:\\Users\\O\u2019\u2019Brien\\phase-hook-v1.exe' 'ai'"; got != want { + t.Fatalf("typographic fallback = %q, want %q", got, want) + } } func TestCodexWindowsHookCommandsAvoidALeadingQuotedPath(t *testing.T) { withShortPathName(t, func(string) (string, error) { return "", errors.New("not needed") }) command := codexPluginCommand(`C:\Users\dev\.phase\ai\bin\phase-hook-v1.exe`, true, "ai", "__codex-event", "guard") - // Codex runs commandWindows in a Windows shell; PowerShell cannot run a - // command line that starts with a quoted path. if strings.HasPrefix(command, `"`) || command != "C:/Users/dev/.phase/ai/bin/phase-hook-v1.exe ai __codex-event guard" { t.Fatalf("Codex commandWindows = %q", command) } @@ -70,4 +71,10 @@ func TestDevinRecognizesEveryWindowsSpellingOfTheHelper(t *testing.T) { if len(hooks["PreToolUse"]) != 1 || !strings.Contains(string(hooks["PreToolUse"][0]), "audit.js") { t.Fatalf("Phase groups from earlier and current builds must both be removed, and only them: %s", hooks["PreToolUse"]) } + quotedHelper := `C:\Users\O'Brien Smith\.phase\ai\bin\phase-hook-v1.exe` + hooks = map[string][]json.RawMessage{"PreToolUse": {group(WindowsShellCommand([]string{quotedHelper, "ai", "__devin-event", "guard"}))}} + removePhaseDevinGroups(hooks, windowsExecutableForms(quotedHelper)...) + if len(hooks["PreToolUse"]) != 0 { + t.Fatalf("the PowerShell fallback spelling was not recognized: %s", hooks["PreToolUse"]) + } } diff --git a/src/pkg/ai/windows_command_windows.go b/src/pkg/ai/windows_command_windows.go index 1b4894d7..a39ff497 100644 --- a/src/pkg/ai/windows_command_windows.go +++ b/src/pkg/ai/windows_command_windows.go @@ -4,9 +4,7 @@ package ai import "syscall" -// windowsShortPathName returns the 8.3 short form of an existing path. It has -// no spaces when the volume keeps short names, so commands can name the hook -// executable without quoting. +// windowsShortPathName returns the 8.3 short form of an existing path. func windowsShortPathName(path string) (string, error) { long, err := syscall.UTF16PtrFromString(path) if err != nil { diff --git a/src/pkg/ai/windows_command_windows_test.go b/src/pkg/ai/windows_command_windows_test.go index f01e2b75..2468fbb7 100644 --- a/src/pkg/ai/windows_command_windows_test.go +++ b/src/pkg/ai/windows_command_windows_test.go @@ -11,8 +11,7 @@ import ( "testing" ) -// windowsTestShells returns the shells a harness may run a command line in on -// Windows, each running one exact command line. Missing shells are skipped. +// windowsTestShells runs one exact command line in each installed shell. func windowsTestShells() map[string]func(string) (string, error) { shells := map[string]func(string) (string, error){ "cmd": func(line string) (string, error) { @@ -39,9 +38,7 @@ func windowsTestShells() map[string]func(string) (string, error) { return shells } -// TestWindowsShellCommandRunsInEveryWindowsShell runs a rendered command line, -// with curl.exe standing in for the hook executable, in each shell, from a -// plain path and from a path with a space. +// curl.exe stands in for the hook executable, from a plain and a spaced path. func TestWindowsShellCommandRunsInEveryWindowsShell(t *testing.T) { system := filepath.Join(os.Getenv("SystemRoot"), "System32", "curl.exe") payload, err := os.ReadFile(system) @@ -61,7 +58,6 @@ func TestWindowsShellCommandRunsInEveryWindowsShell(t *testing.T) { line := WindowsShellCommand([]string{executable, "--version"}) for name, run := range shells { if strings.HasPrefix(line, "& ") && name != "powershell" && name != "pwsh" { - // No 8.3 name on this volume: only PowerShell runs the fallback form. continue } output, err := run(line) diff --git a/src/pkg/proxy/environment.go b/src/pkg/proxy/environment.go index 57e418bd..dc407ae3 100644 --- a/src/pkg/proxy/environment.go +++ b/src/pkg/proxy/environment.go @@ -650,8 +650,7 @@ func CompileProcessEnvironment(options EnvironmentOptions) (ProcessEnvironmentPl // ApplyProcessEnvironment applies a compiled Phase-owned delta to an inherited // process environment without modifying the parent process or any files. -// Inherited variables keep their names: the result is an exec environment, not -// shell source, so only the keys Phase sets or unsets must be shell identifiers. +// Only the keys Phase sets or unsets must be shell identifiers. func ApplyProcessEnvironment(base []string, plan ProcessEnvironmentPlan) ([]string, error) { values := make(map[string]string) for _, entry := range base { @@ -684,9 +683,8 @@ func ApplyProcessEnvironment(base []string, plan ProcessEnvironmentPlan) ([]stri return environment, nil } -// validInheritedEnvironmentKey accepts any name an OS environment block can -// carry, such as Windows' ProgramFiles(x86) or bash's exported BASH_FUNC_name%%. -// An empty name is what Windows' hidden per-drive "=C:" entries split into. +// validInheritedEnvironmentKey accepts names such as ProgramFiles(x86), but not +// the empty names Windows' hidden "=C:" entries split into. func validInheritedEnvironmentKey(key string) bool { return key != "" && !strings.ContainsAny(key, "=\x00") } diff --git a/src/pkg/proxy/store_test.go b/src/pkg/proxy/store_test.go index b4d4df9c..e001eccc 100644 --- a/src/pkg/proxy/store_test.go +++ b/src/pkg/proxy/store_test.go @@ -426,8 +426,6 @@ func TestApplyProcessEnvironmentKeepsInheritedNamesThatAreNotShellIdentifiers(t if err != nil { t.Fatal(err) } - // Windows toolchains find Visual Studio through ProgramFiles(x86); the - // child must see it exactly as the parent does. joined := "\n" + strings.Join(environment, "\n") + "\n" for _, entry := range []string{`ProgramFiles(x86)=C:\Program Files (x86)`, `CommonProgramFiles(x86)=C:\Program Files (x86)\Common Files`, "BASH_FUNC_module%%=() { :\n}", "AWS_ACCESS_KEY_ID=decoy"} { if !strings.Contains(joined, "\n"+entry+"\n") {