From 96b268af0362ae7997f86c9c8f5c447de0fad950 Mon Sep 17 00:00:00 2001 From: Alan Buscaglia Date: Sun, 22 Mar 2026 00:12:47 +0100 Subject: [PATCH] fix(setup): bake OpenCode plugin binary fallback --- internal/setup/setup.go | 68 +++- internal/setup/setup_test.go | 625 ++++++++++++++++++++++------------- 2 files changed, 446 insertions(+), 247 deletions(-) diff --git a/internal/setup/setup.go b/internal/setup/setup.go index a8a13b5..dbc98a8 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -1,9 +1,10 @@ // Package setup handles agent plugin installation. // // - OpenCode: copies embedded plugin file to ~/.config/opencode/plugins/ -// and injects MCP registration in opencode.json using the resolved binary -// path (absolute on Windows, bare "engram" on Unix) so the MCP subprocess -// never requires PATH resolution in headless/systemd environments. +// (patching ENGRAM_BIN to bake in the absolute binary path as a final +// fallback) and injects MCP registration in opencode.json using the +// resolved absolute binary path so child processes never require PATH +// resolution in headless/systemd environments. // - Claude Code: runs `claude plugin marketplace add` + `claude plugin install`, // then writes a durable MCP config to ~/.claude/mcp/engram.json using the // absolute binary path so the subprocess never needs PATH resolution. @@ -89,11 +90,12 @@ var claudeCodeMCPTools = []string{ // Command is always the bare "engram" name in this constant because // upsertCodexEngramBlock generates the actual content via codexEngramBlockStr() // which uses resolveEngramCommand() at runtime. This constant is kept for tests -// that verify idempotency against the already-written string. +// that verify idempotency against the already-written string when os.Executable +// returns "engram" (fallback path). const codexEngramBlock = "[mcp_servers.engram]\ncommand = \"engram\"\nargs = [\"mcp\", \"--tools=agent\"]" // codexEngramBlockStr returns the Codex TOML block for the engram MCP server, -// using the resolved command (absolute path on Windows, bare name on Unix). +// using the resolved absolute binary path from os.Executable(). func codexEngramBlockStr() string { cmd := resolveEngramCommand() return "[mcp_servers.engram]\ncommand = " + fmt.Sprintf("%q", cmd) + "\nargs = [\"mcp\", \"--tools=agent\"]" @@ -251,6 +253,42 @@ func Install(agentName string) (*Result, error) { // ─── OpenCode ──────────────────────────────────────────────────────────────── +// patchEngramBINLine rewrites the ENGRAM_BIN constant declaration in the +// plugin source so the installed copy contains an absolute fallback path. +// +// Original line in source: +// +// const ENGRAM_BIN = process.env.ENGRAM_BIN ?? "engram" +// +// Patched line in installed copy: +// +// const ENGRAM_BIN = process.env.ENGRAM_BIN ?? Bun.which("engram") ?? "/abs/path/engram" +// +// Priority (left to right, first truthy wins): +// 1. ENGRAM_BIN env var — explicit user override, always respected. +// 2. Bun.which("engram") — runtime PATH lookup; works in interactive shells. +// 3. Absolute baked-in path — works in headless/systemd where PATH is stripped. +// +// If absBin is already bare "engram" (os.Executable fallback) we don't add it +// as the third fallback because it would be redundant with Bun.which("engram"). +func patchEngramBINLine(src []byte, absBin string) []byte { + const marker = `const ENGRAM_BIN = process.env.ENGRAM_BIN ?? "engram"` + + var replacement string + if absBin == "engram" { + // os.Executable failed — add Bun.which but no baked-in absolute path + replacement = `const ENGRAM_BIN = process.env.ENGRAM_BIN ?? Bun.which("engram") ?? "engram"` + } else { + // Normal case: bake in the absolute path as final fallback + replacement = fmt.Sprintf( + `const ENGRAM_BIN = process.env.ENGRAM_BIN ?? Bun.which("engram") ?? %q`, + absBin, + ) + } + + return []byte(strings.Replace(string(src), marker, replacement, 1)) +} + func installOpenCode() (*Result, error) { dir := openCodePluginDir() if err := os.MkdirAll(dir, 0755); err != nil { @@ -262,6 +300,14 @@ func installOpenCode() (*Result, error) { return nil, fmt.Errorf("read embedded engram.ts: %w", err) } + // Patch ENGRAM_BIN in the installed copy so the plugin can find the binary + // in headless/systemd environments where PATH may not include user tool dirs. + // The installed file gets a baked-in absolute path while still honoring + // process.env.ENGRAM_BIN (explicit user override) and Bun.which("engram") + // (runtime PATH lookup when PATH is available). The source plugin file is + // not modified — it keeps the simple env-var form for development flexibility. + data = patchEngramBINLine(data, resolveEngramCommand()) + dest := filepath.Join(dir, "engram.ts") if err := openCodeWriteFileFn(dest, data, 0644); err != nil { return nil, fmt.Errorf("write %s: %w", dest, err) @@ -704,14 +750,12 @@ func injectGeminiMCP(configPath string) error { return nil } -// resolveEngramCommand returns the command string to put in agent MCP configs. -// On Windows, MCP subprocesses may not inherit PATH, so we use the absolute -// binary path from os.Executable(). On Unix, bare "engram" is sufficient -// because PATH is reliably inherited. +// resolveEngramCommand returns the absolute path to the engram binary. +// It uses os.Executable() so that headless/systemd environments (where PATH +// is not reliably inherited by child processes) still find the binary. +// EvalSymlinks makes the path stable across package-manager upgrades. +// Falls back to bare "engram" only if os.Executable() itself fails. func resolveEngramCommand() string { - if runtimeGOOS != "windows" { - return "engram" - } exe, err := osExecutable() if err != nil { return "engram" // fallback to PATH-based name diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 7aeb538..6ed45c4 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -132,8 +132,15 @@ func TestInstallGeminiCLIInjectsMCPConfig(t *testing.T) { t.Fatalf("expected mcpServers.engram object") } - if got := engram["command"]; got != "engram" { - t.Fatalf("expected command engram, got %#v", got) + // Since resolveEngramCommand() uses os.Executable() on all platforms, the + // command will be the real test binary path in integration tests (not bare + // "engram"). Verify it is a non-empty absolute path. + cmd, ok := engram["command"].(string) + if !ok || cmd == "" { + t.Fatalf("expected non-empty command string, got %#v", engram["command"]) + } + if cmd == "engram" { + t.Fatalf("expected absolute path from os.Executable(), got bare 'engram'") } args, ok := engram["args"].([]any) @@ -228,7 +235,9 @@ func TestInstallCodexInjectsTOMLAndIsIdempotent(t *testing.T) { if strings.Count(text, "[mcp_servers.engram]") != 1 { t.Fatalf("expected exactly one engram section, got:\n%s", text) } - if !strings.Contains(text, "command = \"engram\"") { + // resolveEngramCommand() uses os.Executable() on all platforms — command + // will be the real absolute path in tests, not bare "engram". + if !strings.Contains(text, "command = ") || !strings.Contains(text, "engram") { t.Fatalf("expected engram command in config, got:\n%s", text) } if !strings.Contains(text, `args = ["mcp", "--tools=agent"]`) { @@ -796,23 +805,33 @@ func TestWriteClaudeCodeUserMCP(t *testing.T) { } func TestResolveEngramCommand(t *testing.T) { - t.Run("unix returns bare name", func(t *testing.T) { + t.Run("unix returns absolute path from os.Executable", func(t *testing.T) { resetSetupSeams(t) runtimeGOOS = "linux" osExecutable = func() (string, error) { return "/usr/local/bin/engram", nil } - if got := resolveEngramCommand(); got != "engram" { - t.Fatalf("expected bare 'engram' on unix, got %q", got) + got := resolveEngramCommand() + // EvalSymlinks on a non-existent path returns an error, so the result + // is the raw os.Executable() value. + if got == "engram" { + t.Fatalf("expected absolute path on unix, got bare 'engram'") + } + if !strings.Contains(got, "engram") { + t.Fatalf("expected engram in path, got %q", got) } }) - t.Run("darwin returns bare name", func(t *testing.T) { + t.Run("darwin returns absolute path from os.Executable", func(t *testing.T) { resetSetupSeams(t) runtimeGOOS = "darwin" osExecutable = func() (string, error) { return "/opt/homebrew/bin/engram", nil } - if got := resolveEngramCommand(); got != "engram" { - t.Fatalf("expected bare 'engram' on darwin, got %q", got) + got := resolveEngramCommand() + if got == "engram" { + t.Fatalf("expected absolute path on darwin, got bare 'engram'") + } + if !strings.Contains(got, "engram") { + t.Fatalf("expected engram in path, got %q", got) } }) @@ -832,13 +851,17 @@ func TestResolveEngramCommand(t *testing.T) { } }) - t.Run("windows executable error falls back to bare name", func(t *testing.T) { - resetSetupSeams(t) - runtimeGOOS = "windows" - osExecutable = func() (string, error) { return "", errors.New("no executable") } + t.Run("executable error falls back to bare name on all platforms", func(t *testing.T) { + for _, goos := range []string{"linux", "darwin", "windows"} { + t.Run(goos, func(t *testing.T) { + resetSetupSeams(t) + runtimeGOOS = goos + osExecutable = func() (string, error) { return "", errors.New("no executable") } - if got := resolveEngramCommand(); got != "engram" { - t.Fatalf("expected fallback to bare 'engram', got %q", got) + if got := resolveEngramCommand(); got != "engram" { + t.Fatalf("expected fallback to bare 'engram', got %q", got) + } + }) } }) } @@ -858,39 +881,51 @@ func TestClaudeCodeMCPDirPaths(t *testing.T) { } } -func TestGeminiInjectUsesAbsolutePathOnWindows(t *testing.T) { - t.Run("windows uses absolute path", func(t *testing.T) { - resetSetupSeams(t) - runtimeGOOS = "windows" - osExecutable = func() (string, error) { return `C:\Users\user\bin\engram.exe`, nil } - - configPath := filepath.Join(t.TempDir(), "settings.json") - if err := injectGeminiMCP(configPath); err != nil { - t.Fatalf("injectGeminiMCP failed: %v", err) - } +// TestGeminiInjectUsesAbsolutePath verifies that injectGeminiMCP writes the +// absolute binary path from os.Executable() on all platforms (issue #113). +func TestGeminiInjectUsesAbsolutePath(t *testing.T) { + for _, tc := range []struct { + goos string + exe string + }{ + {"windows", `C:\Users\user\bin\engram.exe`}, + {"linux", "/usr/local/bin/engram"}, + {"darwin", "/opt/homebrew/bin/engram"}, + } { + t.Run(tc.goos+" uses absolute path", func(t *testing.T) { + resetSetupSeams(t) + runtimeGOOS = tc.goos + osExecutable = func() (string, error) { return tc.exe, nil } + + configPath := filepath.Join(t.TempDir(), "settings.json") + if err := injectGeminiMCP(configPath); err != nil { + t.Fatalf("injectGeminiMCP failed: %v", err) + } - raw, err := os.ReadFile(configPath) - if err != nil { - t.Fatalf("read config: %v", err) - } - var cfg map[string]any - if err := json.Unmarshal(raw, &cfg); err != nil { - t.Fatalf("parse config: %v", err) - } - mcpServers := cfg["mcpServers"].(map[string]any) - engram := mcpServers["engram"].(map[string]any) - cmd := engram["command"].(string) - if cmd == "engram" { - t.Fatalf("expected absolute path on windows, got bare 'engram'") - } - if !strings.Contains(cmd, "engram") { - t.Fatalf("expected engram in command path, got %q", cmd) - } - }) + raw, err := os.ReadFile(configPath) + if err != nil { + t.Fatalf("read config: %v", err) + } + var cfg map[string]any + if err := json.Unmarshal(raw, &cfg); err != nil { + t.Fatalf("parse config: %v", err) + } + mcpServers := cfg["mcpServers"].(map[string]any) + engram := mcpServers["engram"].(map[string]any) + cmd := engram["command"].(string) + if cmd == "engram" { + t.Fatalf("expected absolute path on %s, got bare 'engram'", tc.goos) + } + if !strings.Contains(cmd, "engram") { + t.Fatalf("expected engram in command path, got %q", cmd) + } + }) + } - t.Run("unix uses bare name", func(t *testing.T) { + t.Run("fallback to bare engram when os.Executable fails", func(t *testing.T) { resetSetupSeams(t) runtimeGOOS = "linux" + osExecutable = func() (string, error) { return "", errors.New("no executable") } configPath := filepath.Join(t.TempDir(), "settings.json") if err := injectGeminiMCP(configPath); err != nil { @@ -908,37 +943,49 @@ func TestGeminiInjectUsesAbsolutePathOnWindows(t *testing.T) { mcpServers := cfg["mcpServers"].(map[string]any) engram := mcpServers["engram"].(map[string]any) if got := engram["command"]; got != "engram" { - t.Fatalf("expected bare 'engram' on unix, got %#v", got) + t.Fatalf("expected bare 'engram' fallback, got %#v", got) } }) } -func TestCodexBlockUsesAbsolutePathOnWindows(t *testing.T) { - t.Run("windows uses absolute path in codex block", func(t *testing.T) { - resetSetupSeams(t) - runtimeGOOS = "windows" - osExecutable = func() (string, error) { return `C:\Users\user\bin\engram.exe`, nil } - - block := codexEngramBlockStr() - if strings.Contains(block, `"engram"`) && !strings.Contains(block, `C:\`) { - // The block should contain an absolute path, not just bare "engram" - t.Fatalf("expected absolute path in windows codex block, got:\n%s", block) - } - if !strings.Contains(block, "[mcp_servers.engram]") { - t.Fatalf("expected mcp_servers.engram header, got:\n%s", block) - } - if !strings.Contains(block, `args = ["mcp", "--tools=agent"]`) { - t.Fatalf("expected args in codex block, got:\n%s", block) - } - }) +// TestCodexBlockUsesAbsolutePath verifies codexEngramBlockStr() always bakes +// in the absolute binary path from os.Executable() (issue #113). +func TestCodexBlockUsesAbsolutePath(t *testing.T) { + for _, tc := range []struct { + goos string + exe string + want string + }{ + {"windows", `C:\Users\user\bin\engram.exe`, `C:\Users\user\bin\engram.exe`}, + {"linux", "/usr/local/bin/engram", "/usr/local/bin/engram"}, + {"darwin", "/opt/homebrew/bin/engram", "/opt/homebrew/bin/engram"}, + } { + t.Run(tc.goos+" uses absolute path in codex block", func(t *testing.T) { + resetSetupSeams(t) + runtimeGOOS = tc.goos + osExecutable = func() (string, error) { return tc.exe, nil } + + block := codexEngramBlockStr() + if !strings.Contains(block, "[mcp_servers.engram]") { + t.Fatalf("expected mcp_servers.engram header, got:\n%s", block) + } + if !strings.Contains(block, `args = ["mcp", "--tools=agent"]`) { + t.Fatalf("expected args in codex block, got:\n%s", block) + } + if block == codexEngramBlock { + t.Fatalf("expected absolute path, got bare-engram fallback block:\n%s", block) + } + }) + } - t.Run("unix uses bare name in codex block", func(t *testing.T) { + t.Run("falls back to bare engram when os.Executable fails", func(t *testing.T) { resetSetupSeams(t) runtimeGOOS = "linux" + osExecutable = func() (string, error) { return "", errors.New("no executable") } block := codexEngramBlockStr() if !strings.Contains(block, `command = "engram"`) { - t.Fatalf("expected bare engram in unix codex block, got:\n%s", block) + t.Fatalf("expected bare engram fallback in codex block, got:\n%s", block) } }) } @@ -1089,6 +1136,9 @@ func TestInstallCodexErrorPropagation(t *testing.T) { func TestGeminiAndCodexHelpersErrorPaths(t *testing.T) { t.Run("injectGeminiMCP creates file from missing config", func(t *testing.T) { + resetSetupSeams(t) + // Force a known absolute path so the test is deterministic. + osExecutable = func() (string, error) { return "/usr/local/bin/engram", nil } configPath := filepath.Join(t.TempDir(), "settings.json") if err := injectGeminiMCP(configPath); err != nil { @@ -1113,8 +1163,10 @@ func TestGeminiAndCodexHelpersErrorPaths(t *testing.T) { if !ok { t.Fatalf("expected engram server object") } - if engram["command"] != "engram" { - t.Fatalf("expected engram command, got %#v", engram["command"]) + // resolveEngramCommand() now returns absolute path on all platforms. + cmd, ok := engram["command"].(string) + if !ok || !strings.Contains(cmd, "engram") { + t.Fatalf("expected command containing 'engram', got %#v", engram["command"]) } }) @@ -1359,6 +1411,10 @@ func TestGeminiAndCodexHelpersErrorPaths(t *testing.T) { }) t.Run("upsertCodexEngramBlock from empty content", func(t *testing.T) { + resetSetupSeams(t) + // Force fallback path so output matches the constant. + osExecutable = func() (string, error) { return "", errors.New("no executable") } + output := upsertCodexEngramBlock("\n\n") if output != codexEngramBlock+"\n" { t.Fatalf("unexpected output for empty content:\n%s", output) @@ -2018,237 +2074,336 @@ func TestInjectOpenCodeMCPHandlesJSONC(t *testing.T) { // ─── Issue #112: OpenCode MCP absolute-path config ─────────────────────────── // TestInjectOpenCodeMCPUsesResolvedCommand verifies that injectOpenCodeMCP() -// writes the correct command based on the OS: -// - Windows: absolute path from os.Executable() so headless MCP subprocesses -// don't need PATH. -// - Unix: bare "engram" (PATH is reliably inherited by child processes). +// writes the absolute binary path from os.Executable() on all platforms +// (issue #113: headless environments where PATH may not include user tools). func TestInjectOpenCodeMCPUsesResolvedCommand(t *testing.T) { - t.Run("windows writes absolute path in command array", func(t *testing.T) { - resetSetupSeams(t) - home := useTestHome(t) - runtimeGOOS = "windows" - osExecutable = func() (string, error) { return `C:\Users\user\bin\engram.exe`, nil } - t.Setenv("XDG_CONFIG_HOME", "") + for _, tc := range []struct { + goos string + exe string + }{ + {"windows", `C:\Users\user\bin\engram.exe`}, + {"linux", "/usr/local/bin/engram"}, + {"darwin", "/opt/homebrew/bin/engram"}, + } { + t.Run(tc.goos+" writes absolute path in command array", func(t *testing.T) { + resetSetupSeams(t) + home := useTestHome(t) + runtimeGOOS = tc.goos + osExecutable = func() (string, error) { return tc.exe, nil } + t.Setenv("XDG_CONFIG_HOME", "") + + configDir := filepath.Join(home, ".config", "opencode") + if err := os.MkdirAll(configDir, 0755); err != nil { + t.Fatalf("mkdir config dir: %v", err) + } - configDir := filepath.Join(home, ".config", "opencode") - if err := os.MkdirAll(configDir, 0755); err != nil { - t.Fatalf("mkdir config dir: %v", err) - } + if err := injectOpenCodeMCP(); err != nil { + t.Fatalf("injectOpenCodeMCP failed: %v", err) + } - if err := injectOpenCodeMCP(); err != nil { - t.Fatalf("injectOpenCodeMCP failed: %v", err) - } + raw, err := os.ReadFile(filepath.Join(configDir, "opencode.json")) + if err != nil { + t.Fatalf("read config: %v", err) + } + var cfg map[string]any + if err := json.Unmarshal(raw, &cfg); err != nil { + t.Fatalf("parse config: %v", err) + } + mcp := cfg["mcp"].(map[string]any) + engram := mcp["engram"].(map[string]any) + cmd := engram["command"].([]any) + if len(cmd) == 0 { + t.Fatalf("expected non-empty command array") + } + first := cmd[0].(string) + if first == "engram" { + t.Fatalf("expected absolute path on %s, got bare 'engram'", tc.goos) + } + if !strings.Contains(first, "engram") { + t.Fatalf("expected engram in command path, got %q", first) + } + // Remaining args should be the MCP flags + if len(cmd) != 3 || cmd[1] != "mcp" || cmd[2] != "--tools=agent" { + t.Fatalf("expected args [ mcp --tools=agent], got %v", cmd) + } + }) + } - raw, err := os.ReadFile(filepath.Join(configDir, "opencode.json")) - if err != nil { - t.Fatalf("read config: %v", err) - } - var cfg map[string]any - if err := json.Unmarshal(raw, &cfg); err != nil { - t.Fatalf("parse config: %v", err) - } - mcp := cfg["mcp"].(map[string]any) - engram := mcp["engram"].(map[string]any) - cmd := engram["command"].([]any) - if len(cmd) == 0 { - t.Fatalf("expected non-empty command array") - } - first := cmd[0].(string) - if first == "engram" { - t.Fatalf("expected absolute path on windows, got bare 'engram'") - } - if !strings.Contains(first, "engram") { - t.Fatalf("expected engram in command path, got %q", first) + t.Run("executable error falls back to bare engram on all platforms", func(t *testing.T) { + for _, goos := range []string{"linux", "darwin", "windows"} { + t.Run(goos, func(t *testing.T) { + resetSetupSeams(t) + home := useTestHome(t) + runtimeGOOS = goos + osExecutable = func() (string, error) { return "", errors.New("no executable") } + t.Setenv("XDG_CONFIG_HOME", "") + + configDir := filepath.Join(home, ".config", "opencode") + if err := os.MkdirAll(configDir, 0755); err != nil { + t.Fatalf("mkdir config dir: %v", err) + } + + if err := injectOpenCodeMCP(); err != nil { + t.Fatalf("injectOpenCodeMCP failed: %v", err) + } + + raw, err := os.ReadFile(filepath.Join(configDir, "opencode.json")) + if err != nil { + t.Fatalf("read config: %v", err) + } + var cfg map[string]any + if err := json.Unmarshal(raw, &cfg); err != nil { + t.Fatalf("parse config: %v", err) + } + mcp := cfg["mcp"].(map[string]any) + engram := mcp["engram"].(map[string]any) + cmd := engram["command"].([]any) + if len(cmd) == 0 { + t.Fatalf("expected non-empty command array") + } + if got := cmd[0].(string); got != "engram" { + t.Fatalf("expected fallback to bare 'engram' when os.Executable fails, got %q", got) + } + }) } }) +} - t.Run("linux writes bare engram in command array", func(t *testing.T) { - resetSetupSeams(t) - home := useTestHome(t) - runtimeGOOS = "linux" - t.Setenv("XDG_CONFIG_HOME", "") +// TestInstallOpenCodeWarningUsesResolvedCommand verifies that when MCP injection +// fails, the warning message printed to stderr uses the resolved absolute command +// path so the user's manual config snippet contains the correct binary path even +// in headless/systemd environments (issue #113). +func TestInstallOpenCodeWarningUsesResolvedCommand(t *testing.T) { + for _, tc := range []struct { + goos string + exe string + }{ + {"windows", `C:\bin\engram.exe`}, + {"linux", "/nonexistent/bin/engram"}, // non-existent so EvalSymlinks is a no-op + {"darwin", "/nonexistent/bin/engram"}, // non-existent so EvalSymlinks is a no-op + } { + t.Run(tc.goos+" warning contains absolute path", func(t *testing.T) { + resetSetupSeams(t) + home := useTestHome(t) + runtimeGOOS = tc.goos + osExecutable = func() (string, error) { return tc.exe, nil } + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, "xdg")) + + // Force MCP injection to fail so the warning branch is exercised + injectOpenCodeMCPFn = func() error { + return errors.New("cannot write config") + } - configDir := filepath.Join(home, ".config", "opencode") - if err := os.MkdirAll(configDir, 0755); err != nil { - t.Fatalf("mkdir config dir: %v", err) - } + // Capture stderr + origStderr := os.Stderr + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("pipe: %v", err) + } + os.Stderr = w - if err := injectOpenCodeMCP(); err != nil { - t.Fatalf("injectOpenCodeMCP failed: %v", err) - } + _, installErr := installOpenCode() + w.Close() + os.Stderr = origStderr - raw, err := os.ReadFile(filepath.Join(configDir, "opencode.json")) - if err != nil { - t.Fatalf("read config: %v", err) + if installErr != nil { + t.Fatalf("installOpenCode should not fail when MCP injection is non-fatal: %v", installErr) + } + + buf := make([]byte, 4096) + n, _ := r.Read(buf) + stderr := string(buf[:n]) + + // Warning must reference the binary path — not just bare "engram" + if !strings.Contains(stderr, "engram") { + t.Fatalf("expected engram path in warning on %s, got:\n%s", tc.goos, stderr) + } + // Must NOT be the bare "engram" unquoted form (since we have an absolute path) + if strings.Contains(stderr, `["engram",`) { + t.Fatalf("expected absolute path (not bare engram) in warning message, got:\n%s", stderr) + } + }) + } +} + +// ─── Issue #113: OpenCode plugin ENGRAM_BIN bake-in ───────────────────────── + +// TestPatchEngramBINLine verifies that patchEngramBINLine() correctly rewrites +// the ENGRAM_BIN constant in the plugin source to include a Bun.which() runtime +// fallback and a baked-in absolute path as the final headless fallback. +func TestPatchEngramBINLine(t *testing.T) { + const original = `const ENGRAM_BIN = process.env.ENGRAM_BIN ?? "engram"` + + t.Run("bakes in absolute path with Bun.which intermediate fallback", func(t *testing.T) { + result := string(patchEngramBINLine([]byte(original), "/usr/local/bin/engram")) + + if strings.Contains(result, `?? "engram"`) { + t.Fatalf("original bare-engram fallback should be replaced, got:\n%s", result) } - var cfg map[string]any - if err := json.Unmarshal(raw, &cfg); err != nil { - t.Fatalf("parse config: %v", err) + if !strings.Contains(result, `process.env.ENGRAM_BIN`) { + t.Fatalf("must keep process.env.ENGRAM_BIN as first option, got:\n%s", result) } - mcp := cfg["mcp"].(map[string]any) - engram := mcp["engram"].(map[string]any) - cmd := engram["command"].([]any) - if len(cmd) == 0 { - t.Fatalf("expected non-empty command array") + if !strings.Contains(result, `Bun.which("engram")`) { + t.Fatalf("must include Bun.which fallback, got:\n%s", result) } - if got := cmd[0].(string); got != "engram" { - t.Fatalf("expected bare 'engram' on linux, got %q", got) + if !strings.Contains(result, `"/usr/local/bin/engram"`) { + t.Fatalf("must include baked-in absolute path, got:\n%s", result) } - // Remaining args should be the MCP flags - if len(cmd) != 3 || cmd[1] != "mcp" || cmd[2] != "--tools=agent" { - t.Fatalf("expected args [engram mcp --tools=agent], got %v", cmd) + // Verify precedence order: env var ?? Bun.which ?? absolute path + envIdx := strings.Index(result, `process.env.ENGRAM_BIN`) + whichIdx := strings.Index(result, `Bun.which`) + absIdx := strings.Index(result, `"/usr/local/bin/engram"`) + if !(envIdx < whichIdx && whichIdx < absIdx) { + t.Fatalf("wrong precedence order (env < which < abs), got:\n%s", result) } }) - t.Run("darwin writes bare engram in command array", func(t *testing.T) { - resetSetupSeams(t) - home := useTestHome(t) - runtimeGOOS = "darwin" - t.Setenv("XDG_CONFIG_HOME", "") + t.Run("Windows path with backslashes is JSON-quoted correctly", func(t *testing.T) { + result := string(patchEngramBINLine([]byte(original), `C:\Users\user\bin\engram.exe`)) - configDir := filepath.Join(home, ".config", "opencode") - if err := os.MkdirAll(configDir, 0755); err != nil { - t.Fatalf("mkdir config dir: %v", err) + // The path must appear as a properly JSON-escaped string + if !strings.Contains(result, `Bun.which("engram")`) { + t.Fatalf("must include Bun.which fallback, got:\n%s", result) } - - if err := injectOpenCodeMCP(); err != nil { - t.Fatalf("injectOpenCodeMCP failed: %v", err) + if !strings.Contains(result, `engram.exe`) { + t.Fatalf("must include Windows binary name, got:\n%s", result) } + }) - raw, err := os.ReadFile(filepath.Join(configDir, "opencode.json")) - if err != nil { - t.Fatalf("read config: %v", err) + t.Run("bare engram fallback when os.Executable failed", func(t *testing.T) { + result := string(patchEngramBINLine([]byte(original), "engram")) + + // When absBin=="engram", we still add Bun.which but don't repeat "engram" as absolute + if !strings.Contains(result, `process.env.ENGRAM_BIN`) { + t.Fatalf("must keep process.env.ENGRAM_BIN, got:\n%s", result) } - var cfg map[string]any - if err := json.Unmarshal(raw, &cfg); err != nil { - t.Fatalf("parse config: %v", err) + if !strings.Contains(result, `Bun.which("engram")`) { + t.Fatalf("must include Bun.which fallback, got:\n%s", result) } - mcp := cfg["mcp"].(map[string]any) - engram := mcp["engram"].(map[string]any) - cmd := engram["command"].([]any) - if len(cmd) == 0 { - t.Fatalf("expected non-empty command array") + }) + + t.Run("does not modify source if marker is absent", func(t *testing.T) { + src := []byte(`// already patched\nconst ENGRAM_BIN = process.env.ENGRAM_BIN ?? Bun.which("engram") ?? "/bin/engram"`) + result := patchEngramBINLine(src, "/new/bin/engram") + // Marker not found — returns original unchanged + if string(result) != string(src) { + t.Fatalf("expected no-op when marker absent, got:\n%s", string(result)) } - if got := cmd[0].(string); got != "engram" { - t.Fatalf("expected bare 'engram' on darwin, got %q", got) + }) + + t.Run("only replaces first occurrence", func(t *testing.T) { + doubled := original + "\n" + original + result := string(patchEngramBINLine([]byte(doubled), "/bin/engram")) + // One line should be replaced, the other should remain as-is + if strings.Count(result, `?? "engram"`) != 1 { + t.Fatalf("expected exactly one original line to remain, got:\n%s", result) } }) +} - t.Run("windows executable error falls back to bare engram", func(t *testing.T) { +// TestInstallOpenCodeBakesENGRAMBIN verifies that installOpenCode() writes a +// plugin file where ENGRAM_BIN includes the absolute binary path as a fallback, +// so the plugin works in headless/systemd environments (issue #113). +func TestInstallOpenCodeBakesENGRAMBIN(t *testing.T) { + t.Run("installed plugin contains absolute path fallback", func(t *testing.T) { resetSetupSeams(t) home := useTestHome(t) - runtimeGOOS = "windows" - osExecutable = func() (string, error) { return "", errors.New("no executable") } - t.Setenv("XDG_CONFIG_HOME", "") + runtimeGOOS = "linux" + osExecutable = func() (string, error) { return "/usr/local/bin/engram", nil } + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, "xdg")) - configDir := filepath.Join(home, ".config", "opencode") - if err := os.MkdirAll(configDir, 0755); err != nil { - t.Fatalf("mkdir config dir: %v", err) + result, err := installOpenCode() + if err != nil { + t.Fatalf("installOpenCode failed: %v", err) } - - if err := injectOpenCodeMCP(); err != nil { - t.Fatalf("injectOpenCodeMCP failed: %v", err) + if result.Agent != "opencode" { + t.Fatalf("unexpected agent: %q", result.Agent) } - raw, err := os.ReadFile(filepath.Join(configDir, "opencode.json")) + pluginPath := filepath.Join(home, "xdg", "opencode", "plugins", "engram.ts") + raw, err := os.ReadFile(pluginPath) if err != nil { - t.Fatalf("read config: %v", err) + t.Fatalf("read installed plugin: %v", err) } - var cfg map[string]any - if err := json.Unmarshal(raw, &cfg); err != nil { - t.Fatalf("parse config: %v", err) + content := string(raw) + + // Must have env var override as first priority + if !strings.Contains(content, `process.env.ENGRAM_BIN`) { + t.Fatalf("installed plugin must keep process.env.ENGRAM_BIN override") + } + // Must have Bun.which intermediate fallback + if !strings.Contains(content, `Bun.which("engram")`) { + t.Fatalf("installed plugin must include Bun.which fallback") } - mcp := cfg["mcp"].(map[string]any) - engram := mcp["engram"].(map[string]any) - cmd := engram["command"].([]any) - if len(cmd) == 0 { - t.Fatalf("expected non-empty command array") + // Must have the baked-in absolute path + if !strings.Contains(content, `"/usr/local/bin/engram"`) { + t.Fatalf("installed plugin must contain baked-in absolute path, got:\n%s", content) } - // Should fall back gracefully to bare "engram" - if got := cmd[0].(string); got != "engram" { - t.Fatalf("expected fallback to bare 'engram' when os.Executable fails, got %q", got) + // Source plugin file must remain unchanged (no patching of the template) + srcRaw, err := openCodeReadFile("plugins/opencode/engram.ts") + if err != nil { + t.Fatalf("read embedded plugin: %v", err) + } + if !strings.Contains(string(srcRaw), `?? "engram"`) { + t.Fatalf("source embedded plugin must remain unpatched") } }) -} -// TestInstallOpenCodeWarningUsesResolvedCommand verifies that when MCP injection -// fails, the warning message printed to stderr uses the resolved command rather -// than always printing bare "engram". On Windows this means the user's manual -// config snippet will contain the absolute path they actually need. -func TestInstallOpenCodeWarningUsesResolvedCommand(t *testing.T) { - t.Run("windows warning contains absolute path", func(t *testing.T) { + t.Run("ENGRAM_BIN env var still takes precedence at runtime", func(t *testing.T) { + // We verify by inspection: the installed plugin must use ?? so that a + // truthy process.env.ENGRAM_BIN short-circuits before Bun.which and the + // baked-in path. This is the JavaScript ?? semantics guarantee. resetSetupSeams(t) home := useTestHome(t) - runtimeGOOS = "windows" - osExecutable = func() (string, error) { return `C:\bin\engram.exe`, nil } + runtimeGOOS = "linux" + osExecutable = func() (string, error) { return "/usr/local/bin/engram", nil } t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, "xdg")) - // Force MCP injection to fail so the warning branch is exercised - injectOpenCodeMCPFn = func() error { - return errors.New("cannot write config") + if _, err := installOpenCode(); err != nil { + t.Fatalf("installOpenCode failed: %v", err) } - // Capture stderr - origStderr := os.Stderr - r, w, err := os.Pipe() + pluginPath := filepath.Join(home, "xdg", "opencode", "plugins", "engram.ts") + raw, err := os.ReadFile(pluginPath) if err != nil { - t.Fatalf("pipe: %v", err) - } - os.Stderr = w - - _, installErr := installOpenCode() - w.Close() - os.Stderr = origStderr - - if installErr != nil { - t.Fatalf("installOpenCode should not fail when MCP injection is non-fatal: %v", installErr) + t.Fatalf("read installed plugin: %v", err) } + content := string(raw) - buf := make([]byte, 4096) - n, _ := r.Read(buf) - stderr := string(buf[:n]) - - // The path is written via %q so backslashes are escaped in the output. - if !strings.Contains(stderr, `engram.exe`) { - t.Fatalf("expected absolute path in warning message, got:\n%s", stderr) + // The line must have the form: + // const ENGRAM_BIN = process.env.ENGRAM_BIN ?? Bun.which("engram") ?? "/abs/path" + // where process.env.ENGRAM_BIN is leftmost (wins if set). + envIdx := strings.Index(content, `process.env.ENGRAM_BIN`) + whichIdx := strings.Index(content, `Bun.which("engram")`) + absIdx := strings.Index(content, `"/usr/local/bin/engram"`) + if envIdx == -1 || whichIdx == -1 || absIdx == -1 { + t.Fatalf("missing expected tokens in installed plugin:\n%s", content) } - // Must NOT be the bare "engram" unquoted form - if strings.Contains(stderr, `["engram",`) { - t.Fatalf("expected absolute path (not bare engram) in warning message, got:\n%s", stderr) + if !(envIdx < whichIdx && whichIdx < absIdx) { + t.Fatalf("wrong operator precedence in ENGRAM_BIN line:\n%s", content) } }) - t.Run("linux warning contains bare engram", func(t *testing.T) { + t.Run("os.Executable fallback: Bun.which added but no double-engram", func(t *testing.T) { resetSetupSeams(t) home := useTestHome(t) runtimeGOOS = "linux" + osExecutable = func() (string, error) { return "", errors.New("no executable") } t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, "xdg")) - injectOpenCodeMCPFn = func() error { - return errors.New("cannot write config") + if _, err := installOpenCode(); err != nil { + t.Fatalf("installOpenCode failed: %v", err) } - origStderr := os.Stderr - r, w, err := os.Pipe() + pluginPath := filepath.Join(home, "xdg", "opencode", "plugins", "engram.ts") + raw, err := os.ReadFile(pluginPath) if err != nil { - t.Fatalf("pipe: %v", err) + t.Fatalf("read installed plugin: %v", err) } - os.Stderr = w - - _, installErr := installOpenCode() - w.Close() - os.Stderr = origStderr - - if installErr != nil { - t.Fatalf("installOpenCode should not fail when MCP injection is non-fatal: %v", installErr) - } - - buf := make([]byte, 4096) - n, _ := r.Read(buf) - stderr := string(buf[:n]) + content := string(raw) - if !strings.Contains(stderr, `"engram"`) { - t.Fatalf("expected bare 'engram' in warning message on linux, got:\n%s", stderr) + if !strings.Contains(content, `Bun.which("engram")`) { + t.Fatalf("must still add Bun.which even when os.Executable fails") } }) }