fix(spawn): fail fast when tmux is missing (#2259)

Co-authored-by: Swyam Sharma <cruzer@Swyams-MacBook-Pro.local>
This commit is contained in:
swyam sharma 2026-06-30 00:14:46 +05:30 committed by GitHub
parent a31cf1b582
commit 2c4bf55e95
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 82 additions and 15 deletions

View File

@ -307,7 +307,7 @@ func (c *commandContext) checkTerminalRuntime(ctx context.Context) doctorCheck {
func (c *commandContext) checkTmux(ctx context.Context) doctorCheck { func (c *commandContext) checkTmux(ctx context.Context) doctorCheck {
path, err := c.deps.LookPath("tmux") path, err := c.deps.LookPath("tmux")
if err != nil || path == "" { if err != nil || path == "" {
return doctorCheck{Level: doctorWarn, Section: doctorSectionTools, Name: "tmux", Message: "not found in PATH"} return doctorCheck{Level: doctorWarn, Section: doctorSectionTools, Name: "tmux", Message: "not found in PATH; required on macOS/Linux to start sessions"}
} }
reqCtx, cancel := context.WithTimeout(ctx, probeTimeout) reqCtx, cancel := context.WithTimeout(ctx, probeTimeout)
defer cancel() defer cancel()

View File

@ -247,12 +247,18 @@ func TestDoctorJSONOutputIsDecodable(t *testing.T) {
clearDoctorGitHubEnv(t) clearDoctorGitHubEnv(t)
out, errOut, err := executeCLI(t, Deps{ out, errOut, err := executeCLI(t, Deps{
LookPath: func(name string) (string, error) { LookPath: func(name string) (string, error) {
if name == "git" { switch name {
case "git":
return "/bin/git", nil return "/bin/git", nil
case "tmux":
return "/bin/tmux", nil
} }
return "", errors.New("missing") return "", errors.New("missing")
}, },
CommandOutput: func(context.Context, string, ...string) ([]byte, error) { CommandOutput: func(_ context.Context, name string, _ ...string) ([]byte, error) {
if name == "/bin/tmux" {
return []byte("tmux 3.3a\n"), nil
}
return []byte("git version 2.43.0\n"), nil return []byte("git version 2.43.0\n"), nil
}, },
ProcessAlive: func(int) bool { return false }, ProcessAlive: func(int) bool { return false },
@ -277,12 +283,18 @@ func TestDoctorTextOutputIsGrouped(t *testing.T) {
clearDoctorGitHubEnv(t) clearDoctorGitHubEnv(t)
out, errOut, err := executeCLI(t, Deps{ out, errOut, err := executeCLI(t, Deps{
LookPath: func(name string) (string, error) { LookPath: func(name string) (string, error) {
if name == "git" { switch name {
case "git":
return "/bin/git", nil return "/bin/git", nil
case "tmux":
return "/bin/tmux", nil
} }
return "", errors.New("missing") return "", errors.New("missing")
}, },
CommandOutput: func(context.Context, string, ...string) ([]byte, error) { CommandOutput: func(_ context.Context, name string, _ ...string) ([]byte, error) {
if name == "/bin/tmux" {
return []byte("tmux 3.3a\n"), nil
}
return []byte("git version 2.43.0\n"), nil return []byte("git version 2.43.0\n"), nil
}, },
ProcessAlive: func(int) bool { return false }, ProcessAlive: func(int) bool { return false },

View File

@ -164,6 +164,9 @@ var (
// conflict markers for manual resolution. Adapters wrap this sentinel via // conflict markers for manual resolution. Adapters wrap this sentinel via
// fmt.Errorf so callers can match it with errors.Is. // fmt.Errorf so callers can match it with errors.Is.
ErrPreservedConflict = errors.New("workspace: preserved apply produced conflicts") ErrPreservedConflict = errors.New("workspace: preserved apply produced conflicts")
// ErrRuntimePrerequisite reports a missing host prerequisite for the selected
// runtime before a session can be created.
ErrRuntimePrerequisite = errors.New("runtime: prerequisite missing")
) )
// WorkspaceConfig is the spec for creating or restoring a session's workspace. // WorkspaceConfig is the spec for creating or restoring a session's workspace.

View File

@ -481,6 +481,8 @@ func toAPIError(err error) error {
return apierr.Invalid("INVALID_BRANCH", err.Error(), nil) return apierr.Invalid("INVALID_BRANCH", err.Error(), nil)
case errors.Is(err, ports.ErrAgentBinaryNotFound): case errors.Is(err, ports.ErrAgentBinaryNotFound):
return apierr.Invalid("AGENT_BINARY_NOT_FOUND", err.Error(), nil) return apierr.Invalid("AGENT_BINARY_NOT_FOUND", err.Error(), nil)
case errors.Is(err, ports.ErrRuntimePrerequisite):
return apierr.Invalid("RUNTIME_PREREQUISITE_MISSING", err.Error(), nil)
default: default:
return err return err
} }

View File

@ -534,6 +534,7 @@ func TestToAPIErrorMapsWorkspaceBranchSentinels(t *testing.T) {
{"not fetched", fmt.Errorf("spawn mer-1: workspace: %w: \"x\" has no local head", ports.ErrWorkspaceBranchNotFetched), apierr.KindInvalid, "BRANCH_NOT_FETCHED"}, {"not fetched", fmt.Errorf("spawn mer-1: workspace: %w: \"x\" has no local head", ports.ErrWorkspaceBranchNotFetched), apierr.KindInvalid, "BRANCH_NOT_FETCHED"},
{"invalid branch", fmt.Errorf("spawn mer-1: workspace: %w: \"bad!!\" (exit 1)", ports.ErrWorkspaceBranchInvalid), apierr.KindInvalid, "INVALID_BRANCH"}, {"invalid branch", fmt.Errorf("spawn mer-1: workspace: %w: \"bad!!\" (exit 1)", ports.ErrWorkspaceBranchInvalid), apierr.KindInvalid, "INVALID_BRANCH"},
{"agent binary not found", fmt.Errorf("spawn mer-1: %w", ports.ErrAgentBinaryNotFound), apierr.KindInvalid, "AGENT_BINARY_NOT_FOUND"}, {"agent binary not found", fmt.Errorf("spawn mer-1: %w", ports.ErrAgentBinaryNotFound), apierr.KindInvalid, "AGENT_BINARY_NOT_FOUND"},
{"runtime prerequisite missing", fmt.Errorf("spawn: %w: tmux required on macOS/Linux but not in PATH", ports.ErrRuntimePrerequisite), apierr.KindInvalid, "RUNTIME_PREREQUISITE_MISSING"},
{"unknown harness", fmt.Errorf("spawn: %w: %q", sessionmanager.ErrUnknownHarness, "bogus"), apierr.KindInvalid, "UNKNOWN_HARNESS"}, {"unknown harness", fmt.Errorf("spawn: %w: %q", sessionmanager.ErrUnknownHarness, "bogus"), apierr.KindInvalid, "UNKNOWN_HARNESS"},
{"missing harness", fmt.Errorf("spawn: %w: configure project worker.agent or pass --harness", sessionmanager.ErrMissingHarness), apierr.KindInvalid, "AGENT_REQUIRED"}, {"missing harness", fmt.Errorf("spawn: %w: configure project worker.agent or pass --harness", sessionmanager.ErrMissingHarness), apierr.KindInvalid, "AGENT_REQUIRED"},
} }

View File

@ -202,6 +202,10 @@ func (m *Manager) Spawn(ctx context.Context, cfg ports.SpawnConfig) (domain.Sess
return domain.SessionRecord{}, fmt.Errorf("spawn: %w: %q", ErrUnknownHarness, cfg.Harness) return domain.SessionRecord{}, fmt.Errorf("spawn: %w: %q", ErrUnknownHarness, cfg.Harness)
} }
if err := m.validateRuntimePrerequisites(); err != nil {
return domain.SessionRecord{}, fmt.Errorf("spawn: %w", err)
}
prompt, systemPrompt, err := m.buildSpawnTexts(ctx, cfg) prompt, systemPrompt, err := m.buildSpawnTexts(ctx, cfg)
if err != nil { if err != nil {
return domain.SessionRecord{}, fmt.Errorf("spawn: prompt: %w", err) return domain.SessionRecord{}, fmt.Errorf("spawn: prompt: %w", err)
@ -237,19 +241,19 @@ func (m *Manager) Spawn(ctx context.Context, cfg ports.SpawnConfig) (domain.Sess
// post-create commands (e.g. `pnpm install`) before the agent launches. // post-create commands (e.g. `pnpm install`) before the agent launches.
if err := m.provisionWorkspace(ctx, project, ws.Path); err != nil { if err := m.provisionWorkspace(ctx, project, ws.Path); err != nil {
_ = m.workspace.Destroy(ctx, ws) _ = m.workspace.Destroy(ctx, ws)
m.markSpawnFailedTerminated(ctx, id) m.rollbackSpawnSeedRow(ctx, id)
return domain.SessionRecord{}, fmt.Errorf("spawn %s: provision: %w", id, err) return domain.SessionRecord{}, fmt.Errorf("spawn %s: provision: %w", id, err)
} }
agent, ok := m.agents.Agent(cfg.Harness) agent, ok := m.agents.Agent(cfg.Harness)
if !ok { if !ok {
_ = m.workspace.Destroy(ctx, ws) _ = m.workspace.Destroy(ctx, ws)
m.markSpawnFailedTerminated(ctx, id) m.rollbackSpawnSeedRow(ctx, id)
return domain.SessionRecord{}, fmt.Errorf("spawn %s: no agent adapter for harness %q", id, cfg.Harness) return domain.SessionRecord{}, fmt.Errorf("spawn %s: no agent adapter for harness %q", id, cfg.Harness)
} }
if err := m.prepareWorkspace(ctx, agent, id, ws.Path); err != nil { if err := m.prepareWorkspace(ctx, agent, id, ws.Path); err != nil {
_ = m.workspace.Destroy(ctx, ws) _ = m.workspace.Destroy(ctx, ws)
m.markSpawnFailedTerminated(ctx, id) m.rollbackSpawnSeedRow(ctx, id)
return domain.SessionRecord{}, fmt.Errorf("spawn %s: %w", id, err) return domain.SessionRecord{}, fmt.Errorf("spawn %s: %w", id, err)
} }
agentConfig := effectiveAgentConfig(cfg.Kind, project.Config) agentConfig := effectiveAgentConfig(cfg.Kind, project.Config)
@ -264,7 +268,7 @@ func (m *Manager) Spawn(ctx context.Context, cfg ports.SpawnConfig) (domain.Sess
}) })
if err != nil { if err != nil {
_ = m.workspace.Destroy(ctx, ws) _ = m.workspace.Destroy(ctx, ws)
m.markSpawnFailedTerminated(ctx, id) m.rollbackSpawnSeedRow(ctx, id)
return domain.SessionRecord{}, fmt.Errorf("spawn %s: launch command: %w", id, err) return domain.SessionRecord{}, fmt.Errorf("spawn %s: launch command: %w", id, err)
} }
// Pre-flight: confirm argv[0] actually exists on PATH (or as an absolute // Pre-flight: confirm argv[0] actually exists on PATH (or as an absolute
@ -273,7 +277,7 @@ func (m *Manager) Spawn(ctx context.Context, cfg ports.SpawnConfig) (domain.Sess
// unresolved binary would leak through as a "live" session that never ran. // unresolved binary would leak through as a "live" session that never ran.
if err := m.validateAgentBinary(argv); err != nil { if err := m.validateAgentBinary(argv); err != nil {
_ = m.workspace.Destroy(ctx, ws) _ = m.workspace.Destroy(ctx, ws)
m.markSpawnFailedTerminated(ctx, id) m.rollbackSpawnSeedRow(ctx, id)
return domain.SessionRecord{}, fmt.Errorf("spawn %s: %w", id, err) return domain.SessionRecord{}, fmt.Errorf("spawn %s: %w", id, err)
} }
handle, err := m.runtime.Create(ctx, ports.RuntimeConfig{ handle, err := m.runtime.Create(ctx, ports.RuntimeConfig{
@ -284,7 +288,7 @@ func (m *Manager) Spawn(ctx context.Context, cfg ports.SpawnConfig) (domain.Sess
}) })
if err != nil { if err != nil {
_ = m.workspace.Destroy(ctx, ws) _ = m.workspace.Destroy(ctx, ws)
m.markSpawnFailedTerminated(ctx, id) m.rollbackSpawnSeedRow(ctx, id)
return domain.SessionRecord{}, fmt.Errorf("spawn %s: runtime: %w", id, err) return domain.SessionRecord{}, fmt.Errorf("spawn %s: runtime: %w", id, err)
} }
@ -1281,6 +1285,16 @@ func (m *Manager) validateAgentBinary(argv []string) error {
return nil return nil
} }
func (m *Manager) validateRuntimePrerequisites() error {
if runtime.GOOS == "windows" {
return nil
}
if path, err := m.lookPath("tmux"); err != nil || path == "" {
return fmt.Errorf("%w: tmux required on macOS/Linux but not in PATH", ports.ErrRuntimePrerequisite)
}
return nil
}
func runtimeHandle(meta domain.SessionMetadata) ports.RuntimeHandle { func runtimeHandle(meta domain.SessionMetadata) ports.RuntimeHandle {
return ports.RuntimeHandle{ID: meta.RuntimeHandleID} return ports.RuntimeHandle{ID: meta.RuntimeHandleID}
} }

View File

@ -8,6 +8,7 @@ import (
"log/slog" "log/slog"
"os" "os"
"path/filepath" "path/filepath"
"runtime"
"strings" "strings"
"testing" "testing"
"time" "time"
@ -465,8 +466,8 @@ func TestSpawn_RollsBackOnRuntimeFailure(t *testing.T) {
if ws.destroyed != 1 { if ws.destroyed != 1 {
t.Fatal("workspace should roll back") t.Fatal("workspace should roll back")
} }
if !st.sessions["mer-1"].IsTerminated { if rec, present := st.sessions["mer-1"]; present {
t.Fatal("orphaned spawn should be terminated") t.Fatalf("seed row must be deleted before a runtime handle is live, got %+v", rec)
} }
} }
@ -1098,6 +1099,9 @@ func TestSpawn_RejectsMissingAgentBinary(t *testing.T) {
rt := &fakeRuntime{} rt := &fakeRuntime{}
ws := &fakeWorkspace{} ws := &fakeWorkspace{}
notFound := func(name string) (string, error) { notFound := func(name string) (string, error) {
if name == "tmux" {
return "/bin/tmux", nil
}
return "", fmt.Errorf("exec: %q: not found", name) return "", fmt.Errorf("exec: %q: not found", name)
} }
m := New(Deps{Runtime: rt, Agents: fakeAgents{}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: notFound}) m := New(Deps{Runtime: rt, Agents: fakeAgents{}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: notFound})
@ -1112,8 +1116,39 @@ func TestSpawn_RejectsMissingAgentBinary(t *testing.T) {
if ws.destroyed != 1 { if ws.destroyed != 1 {
t.Fatal("workspace must be torn down when the pre-launch binary check fails") t.Fatal("workspace must be torn down when the pre-launch binary check fails")
} }
if !st.sessions["mer-1"].IsTerminated { if rec, present := st.sessions["mer-1"]; present {
t.Fatal("the orphan row should be marked terminated after the failed spawn") t.Fatalf("seed row must be deleted before a runtime handle is live, got %+v", rec)
}
}
func TestSpawn_RejectsMissingTmuxBeforeSessionRow(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Windows uses ConPTY, not tmux")
}
st := newFakeStore()
st.projects["mer"] = domain.ProjectRecord{ID: "mer", Config: testRoleAgents()}
rt := &fakeRuntime{}
ws := &fakeWorkspace{}
lookPath := func(name string) (string, error) {
if name == "tmux" {
return "", fmt.Errorf("exec: %q: not found", name)
}
return "/bin/true", nil
}
m := New(Deps{Runtime: rt, Agents: fakeAgents{}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath})
_, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker})
if !errors.Is(err, ports.ErrRuntimePrerequisite) || !strings.Contains(err.Error(), "tmux required") {
t.Fatalf("err = %v, want missing tmux prerequisite", err)
}
if len(st.sessions) != 0 {
t.Fatalf("no session row should be created before runtime prerequisites pass, got %d", len(st.sessions))
}
if ws.lastCfg.SessionID != "" || ws.destroyed != 0 {
t.Fatal("workspace must not be created when tmux is missing")
}
if rt.created != 0 {
t.Fatal("runtime must not be created when tmux is missing")
} }
} }