agent-orchestrator/backend/internal/session_manager/manager_test.go

package sessionmanager

import (
	"bytes"
	"context"
	"errors"
	"fmt"
	"log/slog"
	"os"
	"path/filepath"
	"strings"
	"testing"
	"time"

	"github.com/aoagents/agent-orchestrator/backend/internal/domain"
	"github.com/aoagents/agent-orchestrator/backend/internal/ports"
)

var ctx = context.Background()

type fakeStore struct {
	sessions  map[domain.SessionID]domain.SessionRecord
	pr        map[domain.SessionID]domain.PRFacts
	projects  map[string]domain.ProjectRecord
	num       int
	deleteErr error
}

func newFakeStore() *fakeStore {
	return &fakeStore{sessions: map[domain.SessionID]domain.SessionRecord{}, pr: map[domain.SessionID]domain.PRFacts{}, projects: map[string]domain.ProjectRecord{}}
}
func (f *fakeStore) GetProject(_ context.Context, id string) (domain.ProjectRecord, bool, error) {
	r, ok := f.projects[id]
	return r, ok, nil
}
func (f *fakeStore) CreateSession(_ context.Context, rec domain.SessionRecord) (domain.SessionRecord, error) {
	f.num++
	rec.ID = domain.SessionID(fmt.Sprintf("%s-%d", rec.ProjectID, f.num))
	f.sessions[rec.ID] = rec
	return rec, nil
}
func (f *fakeStore) UpdateSession(_ context.Context, rec domain.SessionRecord) error {
	f.sessions[rec.ID] = rec
	return nil
}
func (f *fakeStore) GetSession(_ context.Context, id domain.SessionID) (domain.SessionRecord, bool, error) {
	r, ok := f.sessions[id]
	return r, ok, nil
}
func (f *fakeStore) ListSessions(_ context.Context, p domain.ProjectID) ([]domain.SessionRecord, error) {
	var out []domain.SessionRecord
	for _, r := range f.sessions {
		if r.ProjectID == p {
			out = append(out, r)
		}
	}
	return out, nil
}
func (f *fakeStore) ListAllSessions(context.Context) ([]domain.SessionRecord, error) {
	var out []domain.SessionRecord
	for _, r := range f.sessions {
		out = append(out, r)
	}
	return out, nil
}
func (f *fakeStore) DeleteSession(_ context.Context, id domain.SessionID) (bool, error) {
	if f.deleteErr != nil {
		return false, f.deleteErr
	}
	rec, ok := f.sessions[id]
	if !ok {
		return false, nil
	}
	// Mirror the sqlite gate: only delete rows still in seed state.
	if rec.IsTerminated || rec.Metadata.WorkspacePath != "" || rec.Metadata.RuntimeHandleID != "" || rec.Metadata.AgentSessionID != "" || rec.Metadata.Prompt != "" {
		return false, nil
	}
	delete(f.sessions, id)
	return true, nil
}
func (f *fakeStore) GetDisplayPRFactsForSession(_ context.Context, id domain.SessionID) (domain.PRFacts, bool, error) {
	if pr := f.pr[id]; pr.URL != "" {
		return pr, true, nil
	}
	return domain.PRFacts{}, false, nil
}

type fakeLCM struct {
	store     *fakeStore
	completed int
}

func (l *fakeLCM) MarkSpawned(_ context.Context, id domain.SessionID, metadata domain.SessionMetadata) error {
	l.completed++
	rec := l.store.sessions[id]
	rec.IsTerminated = false
	rec.Activity = domain.Activity{State: domain.ActivityIdle, LastActivityAt: time.Now()}
	rec.Metadata = metadata
	l.store.sessions[id] = rec
	return nil
}
func (l *fakeLCM) MarkTerminated(_ context.Context, id domain.SessionID) error {
	rec := l.store.sessions[id]
	rec.IsTerminated = true
	rec.Activity = domain.Activity{State: domain.ActivityExited, LastActivityAt: time.Now()}
	l.store.sessions[id] = rec
	return nil
}

type fakeRuntime struct {
	createErr          error
	created, destroyed int
	lastCfg            ports.RuntimeConfig
}

func (r *fakeRuntime) Create(_ context.Context, cfg ports.RuntimeConfig) (ports.RuntimeHandle, error) {
	if r.createErr != nil {
		return ports.RuntimeHandle{}, r.createErr
	}
	r.lastCfg = cfg
	r.created++
	return ports.RuntimeHandle{ID: "h1"}, nil
}
func (r *fakeRuntime) Destroy(context.Context, ports.RuntimeHandle) error { r.destroyed++; return nil }

type fakeAgent struct{}

func (fakeAgent) GetConfigSpec(context.Context) (ports.ConfigSpec, error) {
	return ports.ConfigSpec{}, nil
}
func (fakeAgent) GetLaunchCommand(context.Context, ports.LaunchConfig) ([]string, error) {
	return []string{"launch"}, nil
}
func (fakeAgent) GetPromptDeliveryStrategy(context.Context, ports.LaunchConfig) (ports.PromptDeliveryStrategy, error) {
	return ports.PromptDeliveryInCommand, nil
}
func (fakeAgent) GetAgentHooks(context.Context, ports.WorkspaceHookConfig) error { return nil }
func (fakeAgent) GetRestoreCommand(_ context.Context, cfg ports.RestoreConfig) ([]string, bool, error) {
	if id := cfg.Session.Metadata[ports.MetadataKeyAgentSessionID]; id != "" {
		return []string{"resume", id}, true, nil
	}
	return nil, false, nil
}
func (fakeAgent) SessionInfo(context.Context, ports.SessionRef) (ports.SessionInfo, bool, error) {
	return ports.SessionInfo{}, false, nil
}

// fakeAgents resolves every harness to the same fakeAgent.
type fakeAgents struct{}

func (fakeAgents) Agent(domain.AgentHarness) (ports.Agent, bool) { return fakeAgent{}, true }

// recordingAgent captures the LaunchConfig it is handed so a test can assert the
// session manager resolved and forwarded a project's agent config.
type recordingAgent struct {
	fakeAgent
	lastConfig ports.AgentConfig
	lastLaunch ports.LaunchConfig
}

func (a *recordingAgent) GetLaunchCommand(_ context.Context, cfg ports.LaunchConfig) ([]string, error) {
	a.lastConfig = cfg.Config
	a.lastLaunch = cfg
	return []string{"launch"}, nil
}

func (a *recordingAgent) GetRestoreCommand(_ context.Context, cfg ports.RestoreConfig) ([]string, bool, error) {
	a.lastConfig = cfg.Config
	return []string{"resume"}, true, nil
}

type singleAgent struct{ agent ports.Agent }

func (s singleAgent) Agent(domain.AgentHarness) (ports.Agent, bool) { return s.agent, true }

type fakeWorkspace struct {
	createErr  error
	destroyErr error
	destroyed  int
	lastCfg    ports.WorkspaceConfig
	// path, when set, is returned as the workspace path so provisioning tests
	// can point at a real temp directory.
	path string
}

func (w *fakeWorkspace) Create(_ context.Context, cfg ports.WorkspaceConfig) (ports.WorkspaceInfo, error) {
	if w.createErr != nil {
		return ports.WorkspaceInfo{}, w.createErr
	}
	w.lastCfg = cfg
	path := w.path
	if path == "" {
		path = "/ws/" + string(cfg.SessionID)
	}
	return ports.WorkspaceInfo{Path: path, Branch: cfg.Branch, SessionID: cfg.SessionID, ProjectID: cfg.ProjectID}, nil
}
func (w *fakeWorkspace) Destroy(context.Context, ports.WorkspaceInfo) error {
	w.destroyed++
	return w.destroyErr
}
func (w *fakeWorkspace) Restore(ctx context.Context, cfg ports.WorkspaceConfig) (ports.WorkspaceInfo, error) {
	return w.Create(ctx, cfg)
}

type fakeMessenger struct{ msgs []string }

func (m *fakeMessenger) Send(_ context.Context, _ domain.SessionID, msg string) error {
	m.msgs = append(m.msgs, msg)
	return nil
}

func newManager() (*Manager, *fakeStore, *fakeRuntime, *fakeWorkspace) {
	st := newFakeStore()
	rt := &fakeRuntime{}
	ws := &fakeWorkspace{}
	// Stub lookPath so the pre-launch agent-binary check passes; the fakeAgent
	// returns argv ["launch"] which is not a real binary on PATH.
	lookPath := func(string) (string, error) { return "/bin/true", nil }
	m := New(Deps{Runtime: rt, Agents: fakeAgents{}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath})
	return m, st, rt, ws
}
func seedTerminal(st *fakeStore, id domain.SessionID, meta domain.SessionMetadata) {
	st.sessions[id] = domain.SessionRecord{ID: id, ProjectID: "mer", Metadata: meta, IsTerminated: true, Activity: domain.Activity{State: domain.ActivityExited}}
}
func mkLive(id domain.SessionID) domain.SessionRecord {
	return domain.SessionRecord{ID: id, ProjectID: "mer", Metadata: domain.SessionMetadata{WorkspacePath: "/ws/" + string(id), RuntimeHandleID: "h1"}, Activity: domain.Activity{State: domain.ActivityActive}}
}

func TestSpawn_ResolvesProjectConfig(t *testing.T) {
	st := newFakeStore()
	st.projects["mer"] = domain.ProjectRecord{ID: "mer", Config: domain.ProjectConfig{
		DefaultBranch: "develop",
		Env:           map[string]string{"FOO": "bar"},
		AgentConfig:   domain.AgentConfig{Model: "base-model"},
		// A worker role override wins over the base agent config for workers.
		Worker: domain.RoleOverride{Harness: domain.HarnessCodex, AgentConfig: domain.AgentConfig{Model: "worker-model"}},
	}}
	agent := &recordingAgent{}
	rt := &fakeRuntime{}
	ws := &fakeWorkspace{}
	lookPath := func(string) (string, error) { return "/bin/true", nil }
	m := New(Deps{Runtime: rt, Agents: singleAgent{agent: agent}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath})

	rec, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker})
	if err != nil {
		t.Fatal(err)
	}
	if agent.lastConfig.Model != "worker-model" {
		t.Fatalf("launch model = %q, want role override worker-model", agent.lastConfig.Model)
	}
	if rec.Harness != domain.HarnessCodex {
		t.Fatalf("harness = %q, want codex from role override", rec.Harness)
	}
	if ws.lastCfg.BaseBranch != "develop" {
		t.Fatalf("workspace base branch = %q, want develop", ws.lastCfg.BaseBranch)
	}
	if rt.lastCfg.Env["FOO"] != "bar" {
		t.Fatalf("runtime env FOO = %q, want bar", rt.lastCfg.Env["FOO"])
	}
	if rt.lastCfg.Env[EnvSessionID] == "" {
		t.Fatal("runtime env missing AO_SESSION_ID")
	}

	// A project with no stored config yields a zero AgentConfig (adapter defaults).
	st.projects["bare"] = domain.ProjectRecord{ID: "bare"}
	agent.lastConfig = ports.AgentConfig{Model: "stale"}
	if _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "bare", Kind: domain.KindWorker}); err != nil {
		t.Fatal(err)
	}
	if !agent.lastConfig.IsZero() {
		t.Fatalf("launch config = %#v, want zero for project without config", agent.lastConfig)
	}
}

func TestSpawn_AssignsIDAndGoesIdle(t *testing.T) {
	m, st, rt, _ := newManager()
	s, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker, Prompt: "do it"})
	if err != nil {
		t.Fatal(err)
	}
	if s.ID != "mer-1" {
		t.Fatalf("got %q", s.ID)
	}
	if s.Activity.State != domain.ActivityIdle {
		t.Fatalf("fresh session records idle, got %q", s.Activity.State)
	}
	if rt.created != 1 {
		t.Fatal("runtime not created")
	}
	if st.sessions["mer-1"].Metadata.RuntimeHandleID != "h1" {
		t.Fatal("handle not folded")
	}
}
func TestSpawn_RollsBackOnRuntimeFailure(t *testing.T) {
	m, st, _, ws := newManager()
	m.runtime = &fakeRuntime{createErr: errors.New("boom")}
	if _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer"}); err == nil {
		t.Fatal("expected failure")
	}
	if ws.destroyed != 1 {
		t.Fatal("workspace should roll back")
	}
	if !st.sessions["mer-1"].IsTerminated {
		t.Fatal("orphaned spawn should be terminated")
	}
}

// TestSpawn_DeletesSeedRowOnWorkspaceFailure covers the failed-spawn cleanup:
// when workspace materialization fails (e.g. gitworktree refuses a branch
// checked out elsewhere), nothing observable was built, so the seed row is
// deleted outright rather than parked as a terminated orphan that clutters
// session lists.
func TestSpawn_DeletesSeedRowOnWorkspaceFailure(t *testing.T) {
	m, st, rt, ws := newManager()
	ws.createErr = ports.ErrWorkspaceBranchCheckedOutElsewhere
	_, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker})
	if !errors.Is(err, ports.ErrWorkspaceBranchCheckedOutElsewhere) {
		t.Fatalf("err = %v, want ports.ErrWorkspaceBranchCheckedOutElsewhere", err)
	}
	if rec, present := st.sessions["mer-1"]; present {
		t.Fatalf("seed row must be deleted, got %+v", rec)
	}
	if rt.created != 0 {
		t.Fatal("runtime.Create must not run when workspace materialization fails")
	}
}

// TestSpawn_ParksRowTerminatedWhenSeedDeleteFails asserts the fallback: if the
// seed-row delete itself fails, the failed spawn still parks the row as
// terminated so it never looks live.
func TestSpawn_ParksRowTerminatedWhenSeedDeleteFails(t *testing.T) {
	m, st, _, ws := newManager()
	ws.createErr = ports.ErrWorkspaceBranchNotFetched
	st.deleteErr = errors.New("db locked")
	if _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker}); !errors.Is(err, ports.ErrWorkspaceBranchNotFetched) {
		t.Fatalf("err = %v, want ports.ErrWorkspaceBranchNotFetched", err)
	}
	if !st.sessions["mer-1"].IsTerminated {
		t.Fatal("row must fall back to terminated when the seed delete fails")
	}
}
func TestKill_TearsDownRuntimeAndWorkspace(t *testing.T) {
	m, st, rt, ws := newManager()
	st.sessions["mer-1"] = mkLive("mer-1")
	freed, err := m.Kill(ctx, "mer-1")
	if err != nil || !freed {
		t.Fatalf("freed=%v err=%v", freed, err)
	}
	if rt.destroyed != 1 || ws.destroyed != 1 {
		t.Fatal("kill should destroy runtime and workspace")
	}
}
func TestKill_RefusesIncompleteHandle(t *testing.T) {
	m, st, _, _ := newManager()
	st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", Activity: domain.Activity{State: domain.ActivityActive}}
	if _, err := m.Kill(ctx, "mer-1"); !errors.Is(err, ErrIncompleteHandle) {
		t.Fatalf("want ErrIncompleteHandle, got %v", err)
	}
}

// TestKill_DirtyWorkspaceTerminatesAndPreserves: a workspace teardown refused
// because of uncommitted work must NOT fail the kill — the session terminates,
// the runtime is gone, and freed=false reports the preserved worktree. This is
// the normal path for any session with in-progress changes, so an error here
// would turn every such kill into a 500.
func TestKill_DirtyWorkspaceTerminatesAndPreserves(t *testing.T) {
	m, st, rt, ws := newManager()
	st.sessions["mer-1"] = mkLive("mer-1")
	ws.destroyErr = fmt.Errorf("gitworktree: refusing to remove: %w", ports.ErrWorkspaceDirty)
	freed, err := m.Kill(ctx, "mer-1")
	if err != nil {
		t.Fatalf("kill dirty workspace err = %v, want nil", err)
	}
	if freed {
		t.Fatal("freed = true, want false for preserved workspace")
	}
	if rt.destroyed != 1 {
		t.Fatal("runtime should be destroyed")
	}
	if !st.sessions["mer-1"].IsTerminated {
		t.Fatal("session should be terminated")
	}
}

// TestKill_OtherWorkspaceErrorStillFails: only the typed dirty refusal is a
// success-with-preserved-workspace; any other teardown failure keeps erroring.
func TestKill_OtherWorkspaceErrorStillFails(t *testing.T) {
	m, st, _, ws := newManager()
	st.sessions["mer-1"] = mkLive("mer-1")
	ws.destroyErr = errors.New("disk on fire")
	if _, err := m.Kill(ctx, "mer-1"); err == nil || !strings.Contains(err.Error(), "disk on fire") {
		t.Fatalf("kill err = %v, want workspace error surfaced", err)
	}
}
func TestRestore_ReopensTerminal(t *testing.T) {
	m, st, rt, _ := newManager()
	seedTerminal(st, "mer-1", domain.SessionMetadata{WorkspacePath: "/ws/mer-1", Branch: "b", AgentSessionID: "agent-x"})
	s, err := m.Restore(ctx, "mer-1")
	if err != nil {
		t.Fatal(err)
	}
	if s.Activity.State != domain.ActivityIdle {
		t.Fatalf("restored records idle, got %q", s.Activity.State)
	}
	if rt.created != 1 {
		t.Fatal("restore should relaunch")
	}
}
func TestRestore_AppliesProjectAgentConfig(t *testing.T) {
	st := newFakeStore()
	st.projects["mer"] = domain.ProjectRecord{ID: "mer", Config: domain.ProjectConfig{AgentConfig: domain.AgentConfig{Model: "restore-model"}}}
	seedTerminal(st, "mer-1", domain.SessionMetadata{WorkspacePath: "/ws/mer-1", Branch: "b", AgentSessionID: "agent-x"})
	agent := &recordingAgent{}
	lookPath := func(string) (string, error) { return "/bin/true", nil }
	m := New(Deps{Runtime: &fakeRuntime{}, Agents: singleAgent{agent: agent}, Workspace: &fakeWorkspace{}, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath})

	if _, err := m.Restore(ctx, "mer-1"); err != nil {
		t.Fatal(err)
	}
	if agent.lastConfig.Model != "restore-model" {
		t.Fatalf("restore config model = %q, want restore-model (config must carry across restore)", agent.lastConfig.Model)
	}
}

func TestRestore_RefusesLiveSession(t *testing.T) {
	m, st, _, _ := newManager()
	st.sessions["mer-1"] = mkLive("mer-1")
	if _, err := m.Restore(ctx, "mer-1"); !errors.Is(err, ErrNotRestorable) {
		t.Fatalf("want ErrNotRestorable, got %v", err)
	}
}
func TestCleanup_ReclaimsTerminalWorkspaces(t *testing.T) {
	m, st, _, ws := newManager()
	seedTerminal(st, "mer-1", domain.SessionMetadata{WorkspacePath: "/ws/mer-1"})
	st.sessions["mer-2"] = mkLive("mer-2")
	res, err := m.Cleanup(ctx, "mer")
	if err != nil {
		t.Fatal(err)
	}
	if len(res.Cleaned) != 1 || res.Cleaned[0] != "mer-1" {
		t.Fatalf("got %v", res.Cleaned)
	}
	if len(res.Skipped) != 0 {
		t.Fatalf("skipped = %v, want none", res.Skipped)
	}
	if ws.destroyed != 1 {
		t.Fatal("live workspace must not be destroyed")
	}
}

// TestCleanup_ReportsSkippedWorkspaces: a refused teardown must be visible in
// the result with a reason — a silent skip leaves users staring at
// "Would clean N … 0 sessions cleaned" with no explanation.
func TestCleanup_ReportsSkippedWorkspaces(t *testing.T) {
	m, st, _, ws := newManager()
	seedTerminal(st, "mer-1", domain.SessionMetadata{WorkspacePath: "/ws/mer-1"})
	ws.destroyErr = fmt.Errorf("gitworktree: refusing to remove: %w", ports.ErrWorkspaceDirty)
	res, err := m.Cleanup(ctx, "mer")
	if err != nil {
		t.Fatal(err)
	}
	if len(res.Cleaned) != 0 {
		t.Fatalf("cleaned = %v, want none", res.Cleaned)
	}
	if len(res.Skipped) != 1 || res.Skipped[0].SessionID != "mer-1" {
		t.Fatalf("skipped = %v, want mer-1", res.Skipped)
	}
	if res.Skipped[0].Reason != "workspace has uncommitted changes" {
		t.Fatalf("reason = %q", res.Skipped[0].Reason)
	}

	// A non-dirty teardown failure is reported too — but with a fixed public
	// reason: the raw cause carries internal filesystem paths and belongs in
	// the server log, not the API response.
	ws.destroyErr = errors.New("disk on fire")
	res, err = m.Cleanup(ctx, "mer")
	if err != nil {
		t.Fatal(err)
	}
	if len(res.Skipped) != 1 || res.Skipped[0].Reason != "workspace teardown failed" {
		t.Fatalf("skipped = %v, want fixed teardown-failed reason", res.Skipped)
	}
	if strings.Contains(res.Skipped[0].Reason, "disk on fire") {
		t.Fatalf("raw internal error leaked into public reason: %q", res.Skipped[0].Reason)
	}
}

func TestSpawn_DefaultsBranchFromSessionID(t *testing.T) {
	m, st, _, _ := newManager()
	s, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker})
	if err != nil {
		t.Fatal(err)
	}
	// An empty SpawnConfig.Branch defaults to a unique per-session branch.
	if got := st.sessions[s.ID].Metadata.Branch; got != "ao/mer-1" {
		t.Fatalf("default branch = %q, want ao/mer-1", got)
	}
}

func TestSpawnWorker_AppendsActiveOrchestratorContact(t *testing.T) {
	st := newFakeStore()
	st.num = 1
	st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", Kind: domain.KindOrchestrator}
	agent := &recordingAgent{}
	rt := &fakeRuntime{}
	ws := &fakeWorkspace{}
	lookPath := func(string) (string, error) { return "/bin/true", nil }
	m := New(Deps{Runtime: rt, Agents: singleAgent{agent: agent}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath})

	s, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker, Prompt: "do it"})
	if err != nil {
		t.Fatal(err)
	}

	// The user prompt must be preserved and stored in metadata as-is.
	if got := st.sessions[s.ID].Metadata.Prompt; got != "do it" {
		t.Fatalf("metadata prompt = %q, want %q", got, "do it")
	}

	// Coordination instructions must be in the system prompt, not the user prompt.
	systemPrompt := agent.lastLaunch.SystemPrompt
	for _, want := range []string{
		"## Orchestrator coordination",
		`ao send --session mer-1 --message "<your message>"`,
		"Only ping the orchestrator for true blockers, cross-session coordination",
	} {
		if !strings.Contains(systemPrompt, want) {
			t.Fatalf("system prompt missing %q:\n%s", want, systemPrompt)
		}
	}
	if strings.Contains(agent.lastLaunch.Prompt, "## Orchestrator coordination") {
		t.Fatalf("orchestrator coordination must not be in the user prompt:\n%s", agent.lastLaunch.Prompt)
	}
}

func TestSpawnWorker_SkipsTerminatedOrchestratorContact(t *testing.T) {
	st := newFakeStore()
	st.num = 1
	st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", Kind: domain.KindOrchestrator, IsTerminated: true}
	agent := &recordingAgent{}
	rt := &fakeRuntime{}
	ws := &fakeWorkspace{}
	lookPath := func(string) (string, error) { return "/bin/true", nil }
	m := New(Deps{Runtime: rt, Agents: singleAgent{agent: agent}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath})

	_, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker, Prompt: "do it"})
	if err != nil {
		t.Fatal(err)
	}
	systemPrompt := agent.lastLaunch.SystemPrompt
	if strings.Contains(systemPrompt, "## Orchestrator coordination") || strings.Contains(systemPrompt, "ao send --session mer-1") {
		t.Fatalf("terminated orchestrator should not be added to system prompt:\n%s", systemPrompt)
	}
}

func TestSpawnOrchestrator_UsesCoordinatorPrompt(t *testing.T) {
	st := newFakeStore()
	agent := &recordingAgent{}
	rt := &fakeRuntime{}
	ws := &fakeWorkspace{}
	lookPath := func(string) (string, error) { return "/bin/true", nil }
	m := New(Deps{Runtime: rt, Agents: singleAgent{agent: agent}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath})

	_, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindOrchestrator})
	if err != nil {
		t.Fatal(err)
	}

	// Coordinator instructions must be in the system prompt, not the user prompt.
	systemPrompt := agent.lastLaunch.SystemPrompt
	for _, want := range []string{
		"You are the human-facing coordinator for project mer",
		`ao spawn --project mer --prompt "<clear worker task>"`,
		"`ao send`",
		"avoid doing implementation yourself unless it is necessary",
	} {
		if !strings.Contains(systemPrompt, want) {
			t.Fatalf("system prompt missing %q:\n%s", want, systemPrompt)
		}
	}
	if strings.Contains(agent.lastLaunch.Prompt, "You are the human-facing coordinator") {
		t.Fatalf("coordinator role must not be in the user prompt:\n%s", agent.lastLaunch.Prompt)
	}
}

// TestRestore_RefusesIncompleteHandle covers Bug 2: a terminated row whose
// spawn failed before the workspace landed (no WorkspacePath, no Branch) must
// fail Restore with ErrIncompleteHandle — the same typed sentinel Kill returns
// for the same shape — so the HTTP layer surfaces a typed 409 instead of an
// opaque 500.
func TestRestore_RefusesIncompleteHandle(t *testing.T) {
	m, st, _, _ := newManager()
	// Seed a terminated row with no workspace and no branch (the post-failure
	// shape of a Spawn that died before workspace.Create succeeded).
	st.sessions["mer-1"] = domain.SessionRecord{
		ID:           "mer-1",
		ProjectID:    "mer",
		IsTerminated: true,
		Metadata:     domain.SessionMetadata{Prompt: "do it"},
	}
	if _, err := m.Restore(ctx, "mer-1"); !errors.Is(err, ErrIncompleteHandle) {
		t.Fatalf("want ErrIncompleteHandle, got %v", err)
	}
}

// TestRollbackSpawn_DeletesSeedRow covers Bug 4: a session row in seed state
// (no workspace, no runtime, no agent session id, not terminated) is deleted
// outright by RollbackSpawn so the user never sees an orphan terminated row.
func TestRollbackSpawn_DeletesSeedRow(t *testing.T) {
	m, st, _, _ := newManager()
	// Seed row matches what CreateSession produces — no Metadata at all.
	st.sessions["mer-1"] = domain.SessionRecord{
		ID:        "mer-1",
		ProjectID: "mer",
		Activity:  domain.Activity{State: domain.ActivityIdle},
	}
	deleted, killed, err := m.RollbackSpawn(ctx, "mer-1")
	if err != nil {
		t.Fatalf("rollback err = %v", err)
	}
	if !deleted || killed {
		t.Fatalf("deleted=%v killed=%v, want deleted=true killed=false", deleted, killed)
	}
	if _, present := st.sessions["mer-1"]; present {
		t.Fatal("seed row must be removed from the store, not left as terminated")
	}
}

// TestRollbackSpawn_FallsBackToKillForLiveRow asserts the no-resurrection
// guarantee from Bug 4's RCA: once a row has observable spawn output (workspace
// + runtime handle), DeleteSession is a no-op and rollback falls back to Kill
// so the runtime + workspace are torn down rather than abandoned.
func TestRollbackSpawn_FallsBackToKillForLiveRow(t *testing.T) {
	m, st, rt, ws := newManager()
	st.sessions["mer-1"] = mkLive("mer-1")
	deleted, killed, err := m.RollbackSpawn(ctx, "mer-1")
	if err != nil {
		t.Fatalf("rollback err = %v", err)
	}
	if deleted || !killed {
		t.Fatalf("deleted=%v killed=%v, want deleted=false killed=true", deleted, killed)
	}
	if rt.destroyed != 1 || ws.destroyed != 1 {
		t.Fatalf("kill teardown not invoked: rt=%d ws=%d", rt.destroyed, ws.destroyed)
	}
	if !st.sessions["mer-1"].IsTerminated {
		t.Fatal("live row should be marked terminated after kill-fallback")
	}
}

// TestSpawn_RejectsMissingAgentBinary covers Bug 6: when the agent adapter
// returns an argv whose binary is not on PATH, Manager.Spawn must abort BEFORE
// runtime.Create rather than launching into an empty zellij pane that the
// reaper later mistakes for a live session.
func TestSpawn_RejectsMissingAgentBinary(t *testing.T) {
	st := newFakeStore()
	rt := &fakeRuntime{}
	ws := &fakeWorkspace{}
	notFound := func(name string) (string, error) {
		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})

	_, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker})
	if !errors.Is(err, ports.ErrAgentBinaryNotFound) {
		t.Fatalf("err = %v, want ports.ErrAgentBinaryNotFound", err)
	}
	if rt.created != 0 {
		t.Fatal("runtime.Create must NOT run when the agent binary is missing")
	}
	if ws.destroyed != 1 {
		t.Fatal("workspace must be torn down when the pre-launch binary check fails")
	}
	if !st.sessions["mer-1"].IsTerminated {
		t.Fatal("the orphan row should be marked terminated after the failed spawn")
	}
}

// pathPinManager builds a manager whose Executable dep is stubbed, plus a
// buffer capturing its log output, for the hook PATH pin tests.
func pathPinManager(executable func() (string, error)) (*Manager, *fakeStore, *fakeRuntime, *bytes.Buffer) {
	st := newFakeStore()
	rt := &fakeRuntime{}
	logBuf := &bytes.Buffer{}
	lookPath := func(string) (string, error) { return "/bin/true", nil }
	m := New(Deps{
		Runtime: rt, Agents: fakeAgents{}, Workspace: &fakeWorkspace{}, Store: st,
		Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st},
		LookPath: lookPath, Executable: executable,
		Logger: slog.New(slog.NewTextHandler(logBuf, nil)),
	})
	return m, st, rt, logBuf
}

// TestSpawnAndRestore_PinHookPATHToDaemonBinary covers the activity-tracking
// fix: the spawned session's PATH must put the daemon executable's directory
// first, so the bare `ao` in the workspace hook commands resolves to the
// daemon that installed them, not a foreign `ao` earlier on the user's PATH
// (e.g. the legacy TypeScript CLI, which has no `hooks` command and silently
// kills activity tracking).
func TestSpawnAndRestore_PinHookPATHToDaemonBinary(t *testing.T) {
	daemonExe := filepath.Join(t.TempDir(), "ao")
	want := filepath.Dir(daemonExe) + string(os.PathListSeparator) + "/usr/bin"
	executable := func() (string, error) { return daemonExe, nil }

	cases := []struct {
		name   string
		launch func(m *Manager, st *fakeStore) error
	}{
		{
			name: "spawn",
			launch: func(m *Manager, _ *fakeStore) error {
				_, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker})
				return err
			},
		},
		{
			name: "restore",
			launch: func(m *Manager, st *fakeStore) error {
				seedTerminal(st, "mer-1", domain.SessionMetadata{WorkspacePath: "/ws/mer-1", Branch: "b", AgentSessionID: "agent-x"})
				_, err := m.Restore(ctx, "mer-1")
				return err
			},
		},
	}
	for _, tc := range cases {
		t.Run(tc.name, func(t *testing.T) {
			t.Setenv("PATH", "/usr/bin")
			m, st, rt, _ := pathPinManager(executable)
			if err := tc.launch(m, st); err != nil {
				t.Fatal(err)
			}
			if got := rt.lastCfg.Env["PATH"]; got != want {
				t.Fatalf("runtime env PATH = %q, want %q", got, want)
			}
		})
	}
}

// TestSpawn_HookPATHPinUnavailable asserts the degraded path is loud, not
// silent: when the daemon executable cannot anchor `ao` resolution, PATH is
// left to the runtime's inherited default and a warning is logged.
func TestSpawn_HookPATHPinUnavailable(t *testing.T) {
	cases := []struct {
		name       string
		executable func() (string, error)
	}{
		{"executable unresolvable", func() (string, error) { return "", errors.New("no exe") }},
		{"executable not named ao", func() (string, error) { return "/opt/aod/ao-daemon", nil }},
	}
	for _, tc := range cases {
		t.Run(tc.name, func(t *testing.T) {
			m, _, rt, logBuf := pathPinManager(tc.executable)
			if _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker}); err != nil {
				t.Fatal(err)
			}
			if got, ok := rt.lastCfg.Env["PATH"]; ok {
				t.Fatalf("runtime env PATH = %q, want unset when the pin cannot be applied", got)
			}
			if !strings.Contains(logBuf.String(), "not pinned") {
				t.Fatalf("expected a 'not pinned' warning in the log, got %q", logBuf.String())
			}
		})
	}
}

// TestSpawn_ProjectPATHIsPinBase asserts a project's PATH override survives the
// pin as its base rather than being clobbered or clobbering: the daemon dir
// still comes first.
func TestSpawn_ProjectPATHIsPinBase(t *testing.T) {
	daemonExe := filepath.Join(t.TempDir(), "ao")
	m, st, rt, _ := pathPinManager(func() (string, error) { return daemonExe, nil })
	st.projects["mer"] = domain.ProjectRecord{ID: "mer", Config: domain.ProjectConfig{
		Env: map[string]string{"PATH": "/proj/bin"},
	}}
	if _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker}); err != nil {
		t.Fatal(err)
	}
	want := filepath.Dir(daemonExe) + string(os.PathListSeparator) + "/proj/bin"
	if got := rt.lastCfg.Env["PATH"]; got != want {
		t.Fatalf("runtime env PATH = %q, want %q", got, want)
	}
}

func TestSpawn_KeepsExplicitBranch(t *testing.T) {
	m, st, _, _ := newManager()
	s, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker, Branch: "feature/x"})
	if err != nil {
		t.Fatal(err)
	}
	if got := st.sessions[s.ID].Metadata.Branch; got != "feature/x" {
		t.Fatalf("explicit branch = %q, want feature/x", got)
	}
}