From cc75a519ab2244534e47d8f766cf5778486976c9 Mon Sep 17 00:00:00 2001 From: NIKHIL ACHALE <96230495+nikhilachale@users.noreply.github.com> Date: Sat, 4 Jul 2026 11:35:03 +0530 Subject: [PATCH] fix: replace orchestrators through safe canonical branch handoff (#2338) * fix: replace orchestrators through canonical handoff * fix(tests): correct logic in TestRetireForReplacementCapturesAndReleasesWorkspace * fix: enhance tests and add project config handling in service and workspace components * fix: update project summary to include orchestrator agent and adjust related tests * fix: block orchestrator replacement on runtime teardown failure * fix: enhance orchestrator handling in ProjectSettingsForm and SessionsBoard components * fix: rename parameter for clarity in projectConfigPtr function --- backend/internal/httpd/apispec/openapi.yaml | 2 + backend/internal/service/project/service.go | 24 ++-- .../internal/service/project/service_test.go | 26 ++++ backend/internal/service/project/types.go | 13 +- backend/internal/service/session/service.go | 112 +++++++++++++-- .../internal/service/session/service_test.go | 81 ++++++++++- backend/internal/session_manager/manager.go | 53 +++++++ .../internal/session_manager/manager_test.go | 130 +++++++++++++++++- frontend/src/api/schema.ts | 1 + .../OrchestratorReplacementDialog.tsx | 75 ++++++++++ .../components/ProjectSettingsForm.test.tsx | 115 +++++++++++++++- .../components/ProjectSettingsForm.tsx | 31 ++++- .../src/renderer/components/SessionsBoard.tsx | 52 +++++-- .../src/renderer/components/ShellTopbar.tsx | 11 +- frontend/src/renderer/components/Sidebar.tsx | 11 +- .../renderer/hooks/useWorkspaceQuery.test.tsx | 13 +- .../src/renderer/hooks/useWorkspaceQuery.ts | 1 + ...orchestrator-replacement-telemetry.test.ts | 26 ++++ .../lib/orchestrator-replacement-telemetry.ts | 10 ++ .../renderer/lib/restart-orchestrator.test.ts | 73 ++++++++++ .../src/renderer/lib/restart-orchestrator.ts | 52 +++++++ frontend/src/renderer/routes/_shell.tsx | 33 +++++ frontend/src/renderer/stores/ui-store.ts | 26 ++++ frontend/src/renderer/types/workspace.test.ts | 90 ++++++++++++ frontend/src/renderer/types/workspace.ts | 70 ++++++++-- 25 files changed, 1068 insertions(+), 63 deletions(-) create mode 100644 frontend/src/renderer/components/OrchestratorReplacementDialog.tsx create mode 100644 frontend/src/renderer/lib/orchestrator-replacement-telemetry.test.ts create mode 100644 frontend/src/renderer/lib/orchestrator-replacement-telemetry.ts create mode 100644 frontend/src/renderer/lib/restart-orchestrator.test.ts create mode 100644 frontend/src/renderer/lib/restart-orchestrator.ts diff --git a/backend/internal/httpd/apispec/openapi.yaml b/backend/internal/httpd/apispec/openapi.yaml index d3c628a46..2551c32ee 100644 --- a/backend/internal/httpd/apispec/openapi.yaml +++ b/backend/internal/httpd/apispec/openapi.yaml @@ -2069,6 +2069,8 @@ components: type: string name: type: string + orchestratorAgent: + type: string path: type: string resolveError: diff --git a/backend/internal/service/project/service.go b/backend/internal/service/project/service.go index c8d2e3d65..3f8da5cee 100644 --- a/backend/internal/service/project/service.go +++ b/backend/internal/service/project/service.go @@ -104,11 +104,12 @@ func (m *Service) List(ctx context.Context) ([]Summary, error) { out := make([]Summary, 0, len(projects)) for _, row := range projects { out = append(out, Summary{ - ID: domain.ProjectID(row.ID), - Name: displayName(row), - Path: row.Path, - Kind: row.Kind.WithDefault(), - SessionPrefix: resolveSessionPrefix(row), + ID: domain.ProjectID(row.ID), + Name: displayName(row), + Path: row.Path, + Kind: row.Kind.WithDefault(), + SessionPrefix: resolveSessionPrefix(row), + OrchestratorAgent: row.Config.Orchestrator.Harness, }) } return out, nil @@ -390,13 +391,18 @@ func (m *Service) projectFromRow(row domain.ProjectRecord) Project { DefaultBranch: row.Config.WithDefaults().DefaultBranch, Agent: string(m.defaultHarness), } - if !row.Config.IsZero() { - cfg := row.Config - p.Config = &cfg - } + p.Config = projectConfigPtr(row.Config) return p } +func projectConfigPtr(projectConfig domain.ProjectConfig) *domain.ProjectConfig { + if projectConfig.IsZero() { + return nil + } + cfg := projectConfig + return &cfg +} + func displayName(row domain.ProjectRecord) string { if strings.TrimSpace(row.DisplayName) != "" { return row.DisplayName diff --git a/backend/internal/service/project/service_test.go b/backend/internal/service/project/service_test.go index f10dba67e..48e66a653 100644 --- a/backend/internal/service/project/service_test.go +++ b/backend/internal/service/project/service_test.go @@ -436,6 +436,32 @@ func TestManager_SetConfig(t *testing.T) { wantCode(t, err, "PROJECT_NOT_FOUND") } +func TestManager_ListIncludesOnlySummarySafeProjectConfig(t *testing.T) { + ctx := context.Background() + m := newManager(t) + repo := gitRepo(t) + + cfg := domain.ProjectConfig{ + DefaultBranch: "develop", + Env: map[string]string{"GITHUB_TOKEN": "secret"}, + Orchestrator: domain.RoleOverride{Harness: domain.HarnessCodex}, + } + if _, err := m.Add(ctx, project.AddInput{Path: repo, ProjectID: ptr("ao"), Config: &cfg}); err != nil { + t.Fatalf("Add: %v", err) + } + + list, err := m.List(ctx) + if err != nil { + t.Fatalf("List: %v", err) + } + if len(list) != 1 { + t.Fatalf("List len = %d, want 1", len(list)) + } + if list[0].OrchestratorAgent != domain.HarnessCodex { + t.Fatalf("summary orchestrator agent = %q, want codex", list[0].OrchestratorAgent) + } +} + func TestManager_ReaddAfterRemove(t *testing.T) { ctx := context.Background() m := newManager(t) diff --git a/backend/internal/service/project/types.go b/backend/internal/service/project/types.go index f33b338d5..81e5da2be 100644 --- a/backend/internal/service/project/types.go +++ b/backend/internal/service/project/types.go @@ -4,12 +4,13 @@ import "github.com/aoagents/agent-orchestrator/backend/internal/domain" // Summary is the row shape returned by GET /api/v1/projects. type Summary struct { - ID domain.ProjectID `json:"id"` - Name string `json:"name"` - Path string `json:"path"` - Kind domain.ProjectKind `json:"kind"` - SessionPrefix string `json:"sessionPrefix"` - ResolveError string `json:"resolveError,omitempty"` + ID domain.ProjectID `json:"id"` + Name string `json:"name"` + Path string `json:"path"` + Kind domain.ProjectKind `json:"kind"` + SessionPrefix string `json:"sessionPrefix"` + OrchestratorAgent domain.AgentHarness `json:"orchestratorAgent,omitempty"` + ResolveError string `json:"resolveError,omitempty"` } // Project is the full read-model returned by GET /api/v1/projects/{id}. diff --git a/backend/internal/service/session/service.go b/backend/internal/service/session/service.go index 32d338897..0851f3e37 100644 --- a/backend/internal/service/session/service.go +++ b/backend/internal/service/session/service.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "strings" + "sync" "time" "github.com/aoagents/agent-orchestrator/backend/internal/domain" @@ -45,6 +46,7 @@ type commander interface { Spawn(ctx context.Context, cfg ports.SpawnConfig) (domain.SessionRecord, error) Restore(ctx context.Context, id domain.SessionID) (domain.SessionRecord, error) Kill(ctx context.Context, id domain.SessionID) (bool, error) + RetireForReplacement(ctx context.Context, id domain.SessionID) error Send(ctx context.Context, id domain.SessionID, message string) error Cleanup(ctx context.Context, project domain.ProjectID) (sessionmanager.CleanupResult, error) RollbackSpawn(ctx context.Context, id domain.SessionID) (deleted, killed bool, err error) @@ -81,12 +83,14 @@ type scmProvider interface { // session operations to the internal sessionmanager.Manager and owns read-model // assembly, including user-facing display status derivation. type Service struct { - manager commander - store Store - prClaimer ports.PRClaimer - scm scmProvider - clock func() time.Time - telemetry ports.EventSink + manager commander + store Store + prClaimer ports.PRClaimer + scm scmProvider + clock func() time.Time + telemetry ports.EventSink + orchestratorLocksMu sync.Mutex + orchestratorLocks map[domain.ProjectID]*sync.Mutex // signalCapable reports whether a harness has a hook pipeline that can // deliver activity signals at all. Only capable harnesses are eligible for // the no_signal downgrade: a hook-less harness staying silent forever is @@ -263,6 +267,13 @@ func (s *Service) emitSpawnFailed(cfg ports.SpawnConfig, err error, durationMs i // active orchestrator already exists it is returned as-is. A business rule that // belongs here, not in the HTTP controller. func (s *Service) SpawnOrchestrator(ctx context.Context, projectID domain.ProjectID, clean bool) (domain.Session, error) { + unlock := s.lockOrchestratorProject(projectID) + defer unlock() + + project, err := s.requireProject(ctx, projectID) + if err != nil { + return domain.Session{}, err + } active := true if clean { existing, err := s.List(ctx, ListFilter{ProjectID: projectID, Active: &active, OrchestratorOnly: true}) @@ -270,8 +281,9 @@ func (s *Service) SpawnOrchestrator(ctx context.Context, projectID domain.Projec return domain.Session{}, err } for _, orch := range existing { - if _, err := s.Kill(ctx, orch.ID); err != nil { - return domain.Session{}, err + _ = s.sendRetireNotice(ctx, orch.ID) + if err := s.manager.RetireForReplacement(ctx, orch.ID); err != nil { + return domain.Session{}, toAPIError(err) } } } else { @@ -281,10 +293,90 @@ func (s *Service) SpawnOrchestrator(ctx context.Context, projectID domain.Projec return domain.Session{}, err } if len(existing) > 0 { - return existing[0], nil + return newestSession(existing), nil } } - return s.Spawn(ctx, ports.SpawnConfig{ProjectID: projectID, Kind: domain.KindOrchestrator}) + sess, err := s.Spawn(ctx, ports.SpawnConfig{ProjectID: projectID, Kind: domain.KindOrchestrator}) + if err != nil { + return domain.Session{}, err + } + if err := verifyOrchestratorReplacement(project, sess); err != nil { + return domain.Session{}, err + } + return sess, nil +} + +const orchestratorRetireNotice = "AO is replacing this project orchestrator. Stop coordinating new work now; a fresh orchestrator will take over on the canonical branch." + +func (s *Service) sendRetireNotice(ctx context.Context, id domain.SessionID) error { + if err := s.manager.Send(ctx, id, orchestratorRetireNotice); err != nil { + return fmt.Errorf("send retire notice to %s: %w", id, err) + } + return nil +} + +func verifyOrchestratorReplacement(project domain.ProjectRecord, sess domain.Session) error { + if sess.IsTerminated { + return fmt.Errorf("orchestrator replacement verification failed: new session %s is terminated", sess.ID) + } + if sess.Kind != domain.KindOrchestrator { + return fmt.Errorf("orchestrator replacement verification failed: new session %s has kind %q", sess.ID, sess.Kind) + } + if expected := project.Config.Orchestrator.Harness; expected != "" && sess.Harness != expected { + return fmt.Errorf("orchestrator replacement verification failed: new session %s uses harness %q, want %q", sess.ID, sess.Harness, expected) + } + expectedBranch := "ao/" + serviceSessionPrefix(project) + "-orchestrator" + if sess.Metadata.Branch != "" && sess.Metadata.Branch != expectedBranch { + return fmt.Errorf("orchestrator replacement verification failed: new session %s uses branch %q, want %q", sess.ID, sess.Metadata.Branch, expectedBranch) + } + return nil +} + +func serviceSessionPrefix(project domain.ProjectRecord) string { + if p := strings.TrimSpace(project.Config.SessionPrefix); p != "" { + return p + } + id := project.ID + if len(id) <= 12 { + return id + } + return id[:12] +} + +func newestSession(sessions []domain.Session) domain.Session { + newest := sessions[0] + for _, sess := range sessions[1:] { + if sessionNewer(sess.SessionRecord, newest.SessionRecord) { + newest = sess + } + } + return newest +} + +func sessionNewer(a, b domain.SessionRecord) bool { + if !a.CreatedAt.Equal(b.CreatedAt) { + return a.CreatedAt.After(b.CreatedAt) + } + if !a.UpdatedAt.Equal(b.UpdatedAt) { + return a.UpdatedAt.After(b.UpdatedAt) + } + return string(a.ID) > string(b.ID) +} + +func (s *Service) lockOrchestratorProject(projectID domain.ProjectID) func() { + s.orchestratorLocksMu.Lock() + if s.orchestratorLocks == nil { + s.orchestratorLocks = make(map[domain.ProjectID]*sync.Mutex) + } + mu := s.orchestratorLocks[projectID] + if mu == nil { + mu = &sync.Mutex{} + s.orchestratorLocks[projectID] = mu + } + s.orchestratorLocksMu.Unlock() + + mu.Lock() + return mu.Unlock } // Restore relaunches a terminated session and returns the API-facing read model. diff --git a/backend/internal/service/session/service_test.go b/backend/internal/service/session/service_test.go index c5b1b1ca6..9dea20257 100644 --- a/backend/internal/service/session/service_test.go +++ b/backend/internal/service/session/service_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strings" "testing" "time" @@ -201,10 +202,15 @@ func TestSessionRenameMissingSessionReturnsNotFound(t *testing.T) { // clean-orchestrator ordering without wiring a real session engine. type fakeCommander struct { killed []domain.SessionID + retired []domain.SessionID + sent []domain.SessionID cleanupProjects []domain.ProjectID killErr error + retireErr error + sendErr error cleanupErr error spawnErr error + spawnRecord domain.SessionRecord spawned bool killsAtSpawn int } @@ -214,7 +220,10 @@ func (f *fakeCommander) Spawn(_ context.Context, cfg ports.SpawnConfig) (domain. return domain.SessionRecord{}, f.spawnErr } f.spawned = true - f.killsAtSpawn = len(f.killed) + f.killsAtSpawn = len(f.retired) + if f.spawnRecord.ID != "" { + return f.spawnRecord, nil + } return domain.SessionRecord{ID: "mer-9", ProjectID: cfg.ProjectID, Kind: cfg.Kind, Harness: cfg.Harness}, nil } func (f *fakeCommander) Restore(context.Context, domain.SessionID) (domain.SessionRecord, error) { @@ -227,7 +236,20 @@ func (f *fakeCommander) Kill(_ context.Context, id domain.SessionID) (bool, erro f.killed = append(f.killed, id) return true, nil } -func (f *fakeCommander) Send(context.Context, domain.SessionID, string) error { return nil } +func (f *fakeCommander) RetireForReplacement(_ context.Context, id domain.SessionID) error { + if f.retireErr != nil { + return f.retireErr + } + f.retired = append(f.retired, id) + return nil +} +func (f *fakeCommander) Send(_ context.Context, id domain.SessionID, _ string) error { + if f.sendErr != nil { + return f.sendErr + } + f.sent = append(f.sent, id) + return nil +} func (f *fakeCommander) Cleanup(_ context.Context, project domain.ProjectID) (sessionmanager.CleanupResult, error) { f.cleanupProjects = append(f.cleanupProjects, project) if f.cleanupErr != nil { @@ -293,7 +315,7 @@ func TestTeardownProjectStopsOnKillError(t *testing.T) { } } -func TestSpawnOrchestratorCleanKillsActiveOrchestratorsBeforeSpawn(t *testing.T) { +func TestSpawnOrchestratorCleanRetiresActiveOrchestratorsBeforeSpawn(t *testing.T) { st := newFakeStore() st.projects["mer"] = domain.ProjectRecord{ID: "mer"} // Two active orchestrators plus an unrelated worker and a terminated @@ -310,11 +332,35 @@ func TestSpawnOrchestratorCleanKillsActiveOrchestratorsBeforeSpawn(t *testing.T) t.Fatalf("SpawnOrchestrator: %v", err) } - if len(fc.killed) != 2 { - t.Fatalf("killed = %v, want the two active orchestrators", fc.killed) + if len(fc.retired) != 2 { + t.Fatalf("retired = %v, want the two active orchestrators", fc.retired) + } + if len(fc.sent) != 2 { + t.Fatalf("retire notices = %v, want the two active orchestrators", fc.sent) } if !fc.spawned || fc.killsAtSpawn != 2 { - t.Fatalf("spawn must run after both kills: spawned=%v killsAtSpawn=%d", fc.spawned, fc.killsAtSpawn) + t.Fatalf("spawn must run after both retirements: spawned=%v retirementsAtSpawn=%d", fc.spawned, fc.killsAtSpawn) + } + if len(fc.killed) != 0 { + t.Fatalf("interactive Kill must not be used for replacement: killed=%v", fc.killed) + } +} + +func TestSpawnOrchestratorCleanContinuesWhenRetireNoticeFails(t *testing.T) { + st := newFakeStore() + st.projects["mer"] = domain.ProjectRecord{ID: "mer"} + st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", Kind: domain.KindOrchestrator} + fc := &fakeCommander{sendErr: errors.New("pane closed")} + svc := &Service{manager: fc, store: st} + + if _, err := svc.SpawnOrchestrator(context.Background(), "mer", true); err != nil { + t.Fatalf("SpawnOrchestrator: %v", err) + } + if len(fc.retired) != 1 || fc.retired[0] != "mer-1" { + t.Fatalf("retired = %v, want mer-1 despite retire notice failure", fc.retired) + } + if !fc.spawned { + t.Fatal("replacement should still spawn when retire notice delivery fails") } } @@ -620,6 +666,29 @@ func TestSpawnOrchestratorNoCleanSpawnsWhenNoneExists(t *testing.T) { } } +func TestSpawnOrchestratorVerifiesReplacementHarness(t *testing.T) { + st := newFakeStore() + st.projects["mer"] = domain.ProjectRecord{ + ID: "mer", + Config: domain.ProjectConfig{Orchestrator: domain.RoleOverride{Harness: domain.HarnessCodex}}, + } + fc := &fakeCommander{ + spawnRecord: domain.SessionRecord{ + ID: "mer-9", + ProjectID: "mer", + Kind: domain.KindOrchestrator, + Harness: domain.HarnessClaudeCode, + Metadata: domain.SessionMetadata{Branch: "ao/mer-orchestrator"}, + }, + } + svc := &Service{manager: fc, store: st} + + _, err := svc.SpawnOrchestrator(context.Background(), "mer", false) + if err == nil || !strings.Contains(err.Error(), `uses harness "claude-code", want "codex"`) { + t.Fatalf("SpawnOrchestrator err = %v, want harness verification failure", err) + } +} + type fakePRClaimer struct { out errorFreeClaimOutcome err error diff --git a/backend/internal/session_manager/manager.go b/backend/internal/session_manager/manager.go index 9fc3af565..11da9edd8 100644 --- a/backend/internal/session_manager/manager.go +++ b/backend/internal/session_manager/manager.go @@ -476,6 +476,59 @@ func (m *Manager) Kill(ctx context.Context, id domain.SessionID) (bool, error) { return freed, nil } +// RetireForReplacement terminates a live orchestrator and releases its branch +// for a replacement session. Unlike Kill, this captures uncommitted work before +// force-removing the worktree, so a dirty canonical orchestrator worktree does +// not block the replacement from claiming the canonical branch. +// +// This deliberately does not write a session_worktrees row: those rows are +// boot-restore markers, and a replaced orchestrator must stay terminated. +func (m *Manager) RetireForReplacement(ctx context.Context, id domain.SessionID) error { + rec, ok, err := m.store.GetSession(ctx, id) + if err != nil { + return fmt.Errorf("retire replacement %s: %w", id, err) + } + if !ok || rec.IsTerminated { + return nil + } + if rec.Metadata.WorkspacePath == "" || rec.Metadata.Branch == "" { + if err := m.store.DeleteSessionWorktrees(ctx, rec.ID); err != nil { + return fmt.Errorf("retire replacement %s: clear restore markers: %w", id, err) + } + handle := runtimeHandle(rec.Metadata) + if handle.ID != "" { + if err := m.runtime.Destroy(ctx, handle); err != nil { + return fmt.Errorf("retire replacement %s: runtime: %w", id, err) + } + } + if err := m.lcm.MarkTerminated(ctx, id); err != nil { + return fmt.Errorf("retire replacement %s: mark terminated: %w", id, err) + } + return nil + } + + ws := workspaceInfo(rec) + if _, err := m.workspace.StashUncommitted(ctx, ws); err != nil { + return fmt.Errorf("retire replacement %s: stash: %w", id, err) + } + if err := m.store.DeleteSessionWorktrees(ctx, rec.ID); err != nil { + return fmt.Errorf("retire replacement %s: clear restore markers: %w", id, err) + } + handle := runtimeHandle(rec.Metadata) + if handle.ID != "" { + if err := m.runtime.Destroy(ctx, handle); err != nil { + return fmt.Errorf("retire replacement %s: runtime: %w", id, err) + } + } + if err := m.workspace.ForceDestroy(ctx, ws); err != nil { + return fmt.Errorf("retire replacement %s: force destroy: %w", id, err) + } + if err := m.lcm.MarkTerminated(ctx, rec.ID); err != nil { + return fmt.Errorf("retire replacement %s: mark terminated: %w", id, err) + } + return nil +} + // Restore relaunches a torn-down session in its workspace. The fallible I/O runs // before any durable session write, so a failure never resurrects the row or destroys // the worktree (it may hold the agent's prior work). diff --git a/backend/internal/session_manager/manager_test.go b/backend/internal/session_manager/manager_test.go index 679f7a6e5..4b290885d 100644 --- a/backend/internal/session_manager/manager_test.go +++ b/backend/internal/session_manager/manager_test.go @@ -114,6 +114,9 @@ func (f *fakeStore) ListSessionWorktrees(_ context.Context, id domain.SessionID) return f.worktrees[id], nil } func (f *fakeStore) DeleteSessionWorktrees(_ context.Context, id domain.SessionID) error { + if f.sharedLog != nil { + *f.sharedLog = append(*f.sharedLog, "DeleteSessionWorktrees:"+string(id)) + } delete(f.worktrees, id) return nil } @@ -148,6 +151,7 @@ func (l *fakeLCM) MarkTerminated(_ context.Context, id domain.SessionID) error { type fakeRuntime struct { createErr error + destroyErr error created, destroyed int lastCfg ports.RuntimeConfig // aliveByHandle maps a RuntimeHandle.ID to its liveness; missing = false. @@ -167,7 +171,7 @@ func (r *fakeRuntime) Create(_ context.Context, cfg ports.RuntimeConfig) (ports. func (r *fakeRuntime) Destroy(_ context.Context, handle ports.RuntimeHandle) error { r.destroyed++ r.destroyedIDs = append(r.destroyedIDs, handle.ID) - return nil + return r.destroyErr } func (r *fakeRuntime) IsAlive(_ context.Context, handle ports.RuntimeHandle) (bool, error) { if r.aliveErr != nil { @@ -1427,6 +1431,130 @@ func TestSaveAndTeardownAll_CaptureOrderAndMarker(t *testing.T) { } } +func TestRetireForReplacementCapturesAndReleasesWorkspace(t *testing.T) { + m, st, rt, ws := newLifecycleManager() + var sharedLog []string + st.sharedLog = &sharedLog + ws.sharedLog = &sharedLog + ws.stashRef = "refs/ao/preserved/mer-orch" + st.sessions["mer-orch"] = domain.SessionRecord{ + ID: "mer-orch", + ProjectID: "mer", + Kind: domain.KindOrchestrator, + Metadata: domain.SessionMetadata{WorkspacePath: "/ws/mer-orch", Branch: "ao/mer-orchestrator", RuntimeHandleID: "orch-handle"}, + Activity: domain.Activity{State: domain.ActivityActive}, + } + st.worktrees["mer-orch"] = []domain.SessionWorktreeRecord{{ + SessionID: "mer-orch", + RepoName: domain.RootWorkspaceRepoName, + Branch: "ao/mer-orchestrator", + WorktreePath: "/ws/mer-orch", + PreservedRef: "refs/ao/preserved/old", + }} + + if err := m.RetireForReplacement(ctx, "mer-orch"); err != nil { + t.Fatalf("RetireForReplacement err = %v", err) + } + + if rows := st.worktrees["mer-orch"]; len(rows) != 0 { + t.Fatalf("replacement retirement must not write restore markers, got %#v", rows) + } + if !st.sessions["mer-orch"].IsTerminated { + t.Fatal("retired orchestrator must be marked terminated") + } + if rt.destroyed != 1 || rt.destroyedIDs[0] != "orch-handle" { + t.Fatalf("runtime destroyed = %d ids=%v, want orch-handle", rt.destroyed, rt.destroyedIDs) + } + + stashIdx, deleteIdx, forceIdx := -1, -1, -1 + for i, c := range sharedLog { + switch c { + case "StashUncommitted:mer-orch": + stashIdx = i + case "DeleteSessionWorktrees:mer-orch": + deleteIdx = i + case "ForceDestroy:mer-orch": + forceIdx = i + } + } + if stashIdx == -1 || deleteIdx == -1 || forceIdx == -1 { + t.Fatalf("missing expected calls in shared log: %v", sharedLog) + } + if stashIdx >= deleteIdx || deleteIdx >= forceIdx { + t.Fatalf("replacement retire must capture, clear restore marker, then force release; log=%v", sharedLog) + } +} + +func TestRetireForReplacementForceDestroyFailureLeavesSessionActive(t *testing.T) { + m, st, rt, ws := newLifecycleManager() + ws.forceDestroyErr = errors.New("worktree still registered") + ws.stashRef = "refs/ao/preserved/mer-orch" + st.sessions["mer-orch"] = domain.SessionRecord{ + ID: "mer-orch", + ProjectID: "mer", + Kind: domain.KindOrchestrator, + Metadata: domain.SessionMetadata{WorkspacePath: "/ws/mer-orch", Branch: "ao/mer-orchestrator", RuntimeHandleID: "orch-handle"}, + Activity: domain.Activity{State: domain.ActivityActive}, + } + st.worktrees["mer-orch"] = []domain.SessionWorktreeRecord{{ + SessionID: "mer-orch", + RepoName: domain.RootWorkspaceRepoName, + Branch: "ao/mer-orchestrator", + WorktreePath: "/ws/mer-orch", + PreservedRef: "refs/ao/preserved/old", + }} + + err := m.RetireForReplacement(ctx, "mer-orch") + if err == nil || !strings.Contains(err.Error(), "force destroy") { + t.Fatalf("RetireForReplacement err = %v, want force destroy failure", err) + } + if st.sessions["mer-orch"].IsTerminated { + t.Fatal("session must remain active so retry can retire it again") + } + if rt.destroyed != 1 { + t.Fatalf("runtime destroyed = %d, want 1 before workspace release", rt.destroyed) + } + if ws.stashCalls != 1 { + t.Fatalf("stash calls = %d, want 1", ws.stashCalls) + } +} + +func TestRetireForReplacementRuntimeDestroyFailureBlocksWorkspaceRelease(t *testing.T) { + m, st, rt, ws := newLifecycleManager() + rt.destroyErr = errors.New("tmux transient") + ws.stashRef = "refs/ao/preserved/mer-orch" + st.sessions["mer-orch"] = domain.SessionRecord{ + ID: "mer-orch", + ProjectID: "mer", + Kind: domain.KindOrchestrator, + Metadata: domain.SessionMetadata{WorkspacePath: "/ws/mer-orch", Branch: "ao/mer-orchestrator", RuntimeHandleID: "orch-handle"}, + Activity: domain.Activity{State: domain.ActivityActive}, + } + st.worktrees["mer-orch"] = []domain.SessionWorktreeRecord{{ + SessionID: "mer-orch", + RepoName: domain.RootWorkspaceRepoName, + Branch: "ao/mer-orchestrator", + WorktreePath: "/ws/mer-orch", + PreservedRef: "refs/ao/preserved/old", + }} + + err := m.RetireForReplacement(ctx, "mer-orch") + if err == nil || !strings.Contains(err.Error(), "runtime") { + t.Fatalf("RetireForReplacement err = %v, want runtime failure", err) + } + if st.sessions["mer-orch"].IsTerminated { + t.Fatal("session must remain active when runtime destroy fails") + } + if rt.destroyed != 1 || rt.destroyedIDs[0] != "orch-handle" { + t.Fatalf("runtime destroyed = %d ids=%v, want one attempt for orch-handle", rt.destroyed, rt.destroyedIDs) + } + for _, call := range ws.calls { + if call == "ForceDestroy:mer-orch" { + t.Fatalf("ForceDestroy must not run after runtime destroy failure; calls=%v", ws.calls) + } + } +} + // TestSaveAndTeardownAll_CleanWorktreeWritesEmptyRef verifies that a clean // worktree (StashUncommitted returns "") still writes a worktree row (with // empty preserved_ref). The row's presence is the shutdown-saved marker. diff --git a/frontend/src/api/schema.ts b/frontend/src/api/schema.ts index fe960f8e0..294de0c00 100644 --- a/frontend/src/api/schema.ts +++ b/frontend/src/api/schema.ts @@ -746,6 +746,7 @@ export interface components { id: string; kind: string; name: string; + orchestratorAgent?: string; path: string; resolveError?: string; sessionPrefix: string; diff --git a/frontend/src/renderer/components/OrchestratorReplacementDialog.tsx b/frontend/src/renderer/components/OrchestratorReplacementDialog.tsx new file mode 100644 index 000000000..c9f3817e6 --- /dev/null +++ b/frontend/src/renderer/components/OrchestratorReplacementDialog.tsx @@ -0,0 +1,75 @@ +import * as Dialog from "@radix-ui/react-dialog"; +import { useNavigate } from "@tanstack/react-router"; +import { AlertTriangle, RotateCw, X } from "lucide-react"; +import { findProjectOrchestrator, type WorkspaceSummary } from "../types/workspace"; + +type OrchestratorReplacementDialogProps = { + projectId: string | null; + error?: string; + workspaces: WorkspaceSummary[]; + onOpenChange: (open: boolean) => void; + onRetry: (projectId: string) => void; +}; + +export function OrchestratorReplacementDialog({ + projectId, + error, + workspaces, + onOpenChange, + onRetry, +}: OrchestratorReplacementDialogProps) { + const navigate = useNavigate(); + const open = Boolean(projectId && error); + const orchestrator = projectId ? findProjectOrchestrator(workspaces, projectId) : undefined; + + const openCurrent = () => { + if (!projectId || !orchestrator) return; + onOpenChange(false); + void navigate({ + to: "/projects/$projectId/sessions/$sessionId", + params: { projectId, sessionId: orchestrator.id }, + }); + }; + + return ( + + + + +
+
+
+
+ Orchestrator replacement failed + + {error ?? "The project orchestrator could not be replaced."} + +
+ + + +
+
+ {orchestrator ? ( + + ) : null} + +
+ + + + ); +} diff --git a/frontend/src/renderer/components/ProjectSettingsForm.test.tsx b/frontend/src/renderer/components/ProjectSettingsForm.test.tsx index e1a58e340..8811eefb9 100644 --- a/frontend/src/renderer/components/ProjectSettingsForm.test.tsx +++ b/frontend/src/renderer/components/ProjectSettingsForm.test.tsx @@ -3,15 +3,17 @@ import { render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { beforeEach, describe, expect, it, vi } from "vitest"; -const { getMock, putMock } = vi.hoisted(() => ({ +const { getMock, putMock, postMock } = vi.hoisted(() => ({ getMock: vi.fn(), putMock: vi.fn(), + postMock: vi.fn(), })); vi.mock("../lib/api-client", () => ({ apiClient: { GET: getMock, PUT: putMock, + POST: postMock, }, apiErrorMessage: (error: unknown) => { if (error instanceof Error) return error.message; @@ -23,14 +25,19 @@ vi.mock("../lib/api-client", () => ({ })); import { ProjectSettingsForm } from "./ProjectSettingsForm"; +import { workspaceQueryKey } from "../hooks/useWorkspaceQuery"; +import type { WorkspaceSummary } from "../types/workspace"; -function renderSettings(projectId = "proj-1") { +function renderSettings(projectId = "proj-1", workspaces?: WorkspaceSummary[]) { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false }, mutations: { retry: false }, }, }); + if (workspaces) { + queryClient.setQueryData(workspaceQueryKey, workspaces); + } render( @@ -86,7 +93,9 @@ function mockProject(project: Record) { beforeEach(() => { getMock.mockReset(); putMock.mockReset(); + postMock.mockReset(); putMock.mockResolvedValue({ data: { project: {} }, error: undefined }); + postMock.mockResolvedValue({ data: { orchestrator: { id: "proj-1-orch-2" } }, error: undefined, response: { status: 200 } }); }); describe("ProjectSettingsForm", () => { @@ -168,6 +177,10 @@ describe("ProjectSettingsForm", () => { }, }, }); + await waitFor(() => expect(postMock).toHaveBeenCalledTimes(1)); + expect(postMock).toHaveBeenCalledWith("/api/v1/orchestrators", { + body: { projectId: "proj-1", clean: true }, + }); expect(await screen.findByText("Saved.")).toBeInTheDocument(); }, 20_000); @@ -195,6 +208,7 @@ describe("ProjectSettingsForm", () => { expect(await screen.findByText("invalid permissions")).toBeInTheDocument(); expect(screen.queryByText("Saved.")).not.toBeInTheDocument(); + expect(postMock).not.toHaveBeenCalled(); }); it("requires worker and orchestrator agents for existing projects missing role config", async () => { @@ -323,4 +337,101 @@ describe("ProjectSettingsForm", () => { expect(await screen.findAllByText("Enabling intake requires an assignee.")).toHaveLength(2); expect(putMock).not.toHaveBeenCalled(); }); + + it("restarts when the saved orchestrator agent already differs from the running orchestrator", async () => { + getMock.mockResolvedValue({ + data: { + status: "ok", + project: { + id: "proj-1", + name: "Project One", + kind: "single_repo", + path: "/repo/project-one", + repo: "", + defaultBranch: "main", + config: { + worker: { agent: "codex" }, + orchestrator: { agent: "goose" }, + }, + }, + }, + error: undefined, + }); + + renderSettings("proj-1", [ + { + id: "proj-1", + name: "Project One", + path: "/repo/project-one", + orchestratorAgent: "goose", + sessions: [ + { + id: "proj-1-orchestrator", + workspaceId: "proj-1", + workspaceName: "Project One", + title: "Orchestrator", + provider: "claude-code", + kind: "orchestrator", + branch: "ao/proj-1-orchestrator", + status: "working", + createdAt: "2026-07-03T00:00:00Z", + updatedAt: "2026-07-03T00:00:00Z", + prs: [], + }, + ], + }, + ]); + + const orchestratorAgent = await screen.findByRole("combobox", { name: "Default orchestrator agent" }); + expect(orchestratorAgent).toHaveTextContent("goose"); + + await userEvent.click(screen.getByRole("button", { name: "Save changes" })); + + await waitFor(() => expect(putMock).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(postMock).toHaveBeenCalledTimes(1)); + expect(postMock).toHaveBeenCalledWith("/api/v1/orchestrators", { + body: { projectId: "proj-1", clean: true }, + }); + }); + + it("keeps the config save successful when orchestrator replacement fails", async () => { + getMock.mockResolvedValue({ + data: { + status: "ok", + project: { + id: "proj-1", + name: "Project One", + kind: "single_repo", + path: "/repo/project-one", + repo: "", + defaultBranch: "main", + config: { + worker: { agent: "codex" }, + orchestrator: { agent: "claude-code" }, + }, + }, + }, + error: undefined, + }); + postMock.mockResolvedValue({ + data: undefined, + error: { message: "missing goose binary" }, + response: { status: 500 }, + }); + + const queryClient = renderSettings(); + const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries"); + + const orchestratorAgent = await screen.findByRole("combobox", { name: "Default orchestrator agent" }); + await chooseOption(orchestratorAgent, "goose"); + await userEvent.click(screen.getByRole("button", { name: "Save changes" })); + + await waitFor(() => expect(putMock).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(postMock).toHaveBeenCalledTimes(1)); + expect(await screen.findByText("Saved.")).toBeInTheDocument(); + expect(await screen.findByText("Orchestrator restart failed: missing goose binary")).toBeInTheDocument(); + expect(screen.queryByText("Save failed")).not.toBeInTheDocument(); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ["project", "proj-1"] }); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: workspaceQueryKey }); + }); }); diff --git a/frontend/src/renderer/components/ProjectSettingsForm.tsx b/frontend/src/renderer/components/ProjectSettingsForm.tsx index d50216dca..513567027 100644 --- a/frontend/src/renderer/components/ProjectSettingsForm.tsx +++ b/frontend/src/renderer/components/ProjectSettingsForm.tsx @@ -1,9 +1,11 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { useState } from "react"; import type { components } from "../../api/schema"; -import { apiClient, apiErrorMessage } from "../lib/api-client"; import { agentsQueryKey, agentsQueryOptions, refreshAgents } from "../hooks/useAgentsQuery"; -import { workspaceQueryKey } from "../hooks/useWorkspaceQuery"; +import { useWorkspaceQuery, workspaceQueryKey } from "../hooks/useWorkspaceQuery"; +import { apiClient, apiErrorMessage } from "../lib/api-client"; +import { spawnOrchestrator } from "../lib/spawn-orchestrator"; +import { newestActiveOrchestrator } from "../types/workspace"; import { RequiredAgentField } from "./CreateProjectAgentSheet"; import { DashboardSubhead } from "./DashboardSubhead"; import { buildIntake, deriveGitHubRepo, IntakeFields, type IntakeForm, intakeNeedsRule } from "./IntakeFields"; @@ -68,7 +70,10 @@ export function ProjectSettingsForm({ projectId }: { projectId: string }) { function SettingsBody({ project, projectId, onSaved }: { project: Project; projectId: string; onSaved: () => void }) { const queryClient = useQueryClient(); + const workspaceQuery = useWorkspaceQuery(); const config = project.config ?? {}; + const workspace = workspaceQuery.data?.find((item) => item.id === projectId); + const activeOrchestrator = newestActiveOrchestrator(workspace?.sessions ?? []); const intake: TrackerIntakeConfig = config.trackerIntake ?? {}; const [form, setForm] = useState({ defaultBranch: config.defaultBranch ?? project.defaultBranch ?? "", @@ -83,7 +88,9 @@ function SettingsBody({ project, projectId, onSaved }: { project: Project; proje intakeAssignee: intake.assignee ?? "", }); const [savedAt, setSavedAt] = useState(null); + const [replacementError, setReplacementError] = useState(null); const [validationError, setValidationError] = useState(null); + const initialOrchestratorAgent = config.orchestrator?.agent ?? ""; const missingRequiredAgent = form.workerAgent === "" || form.orchestratorAgent === ""; const agentsQuery = useQuery(agentsQueryOptions); const agentCatalog = agentsQuery.data; @@ -134,9 +141,23 @@ function SettingsBody({ project, projectId, onSaved }: { project: Project; proje body: { config: next }, }); if (error) throw new Error(apiErrorMessage(error)); + if ( + form.orchestratorAgent !== initialOrchestratorAgent || + (activeOrchestrator && activeOrchestrator.provider !== form.orchestratorAgent) + ) { + try { + await spawnOrchestrator(projectId, true); + } catch (error) { + return { + replacementError: error instanceof Error ? error.message : "Could not replace orchestrator", + }; + } + } + return { replacementError: null }; }, - onSuccess: () => { + onSuccess: (result) => { setSavedAt(Date.now()); + setReplacementError(result.replacementError); setValidationError(null); void queryClient.invalidateQueries({ queryKey: ["project", projectId] }); onSaved(); @@ -149,6 +170,7 @@ function SettingsBody({ project, projectId, onSaved }: { project: Project; proje onSubmit={(event) => { event.preventDefault(); setSavedAt(null); + setReplacementError(null); if (missingRequiredAgent) { setValidationError("Worker and orchestrator agents are required."); return; @@ -304,6 +326,9 @@ function SettingsBody({ project, projectId, onSaved }: { project: Project; proje {savedAt && !mutation.isPending && !mutation.isError && ( Saved. )} + {replacementError && !mutation.isPending && !mutation.isError && ( + Orchestrator restart failed: {replacementError} + )} ); diff --git a/frontend/src/renderer/components/SessionsBoard.tsx b/frontend/src/renderer/components/SessionsBoard.tsx index 6e85b89fa..30f4fde97 100644 --- a/frontend/src/renderer/components/SessionsBoard.tsx +++ b/frontend/src/renderer/components/SessionsBoard.tsx @@ -1,14 +1,15 @@ import { type KeyboardEvent, useState } from "react"; import { useQueryClient } from "@tanstack/react-query"; import { useNavigate } from "@tanstack/react-router"; - +import { AlertTriangle, Plus, RotateCw } from "lucide-react"; import { DashboardSubhead } from "./DashboardSubhead"; -import { Plus } from "lucide-react"; import { type AttentionZone, type WorkspaceSession, attentionZone, canonicalTrackerIssueId, + newestActiveOrchestrator, + orchestratorHealth, workerSessions, } from "../types/workspace"; import { useSessionScmSummary, type SessionPRSummary } from "../hooks/useSessionScmSummary"; @@ -16,9 +17,11 @@ import { useWorkspaceQuery, workspaceQueryKey } from "../hooks/useWorkspaceQuery import { OrchestratorIcon } from "./icons"; import { NewTaskDialog } from "./NewTaskDialog"; import { spawnOrchestrator } from "../lib/spawn-orchestrator"; +import { restartProjectOrchestrator } from "../lib/restart-orchestrator"; import { prDiffSummary, sessionPRDisplaySummaries } from "../lib/pr-display"; import { cn } from "../lib/utils"; import { PRAttentionPanel, PRStatusStrip } from "./PRSummaryDisplay"; +import { useUiStore } from "../stores/ui-store"; type SessionsBoardProps = { /** When set, the board shows only this project's sessions. */ @@ -77,12 +80,16 @@ export function SessionsBoard({ projectId }: SessionsBoardProps) { const workspaceQuery = useWorkspaceQuery(); const all = workspaceQuery.data ?? []; const workspaces = projectId ? all.filter((w) => w.id === projectId) : all; + const workspace = projectId ? workspaces[0] : undefined; const sessions = workspaces.flatMap((w) => workerSessions(w.sessions)); - const orchestrator = projectId - ? workspaces[0]?.sessions.find((session) => session.kind === "orchestrator" && session.status !== "terminated") - : undefined; + const orchestrator = projectId ? newestActiveOrchestrator(workspaces[0]?.sessions ?? []) : undefined; const [isNewTaskOpen, setIsNewTaskOpen] = useState(false); const [isSpawning, setIsSpawning] = useState(false); + const restartingProjectIds = useUiStore((state) => state.restartingProjectIds); + const setProjectRestarting = useUiStore((state) => state.setProjectRestarting); + const setOrchestratorReplacementError = useUiStore((state) => state.setOrchestratorReplacementError); + const isProjectRestarting = projectId ? restartingProjectIds.has(projectId) : false; + const health = workspace ? orchestratorHealth(workspace, isProjectRestarting) : { state: "ok" as const }; const byZone = new Map(); for (const session of sessions) { @@ -101,7 +108,7 @@ export function SessionsBoard({ projectId }: SessionsBoardProps) { }); const openOrchestrator = async () => { - if (!projectId) return; + if (!projectId || isProjectRestarting) return; if (orchestrator) { void navigate({ to: "/projects/$projectId/sessions/$sessionId", @@ -122,6 +129,17 @@ export function SessionsBoard({ projectId }: SessionsBoardProps) { } }; + const restartOrchestrator = async () => { + if (!projectId) return; + await restartProjectOrchestrator({ + projectId, + queryClient, + navigate, + setProjectRestarting, + setOrchestratorReplacementError, + }); + }; + const handleTaskCreated = async (sessionId: string) => { if (!projectId) return; await queryClient.invalidateQueries({ queryKey: workspaceQueryKey }); @@ -136,6 +154,7 @@ export function SessionsBoard({ projectId }: SessionsBoardProps) { ) : undefined; @@ -164,6 +183,23 @@ export function SessionsBoard({ projectId }: SessionsBoardProps) { />
+ {projectId && health.state !== "ok" ? ( +
+
+ ) : null} {workspaceQuery.isError ? (

Could not load sessions.

) : ( diff --git a/frontend/src/renderer/components/ShellTopbar.tsx b/frontend/src/renderer/components/ShellTopbar.tsx index c336b19cb..2929475c7 100644 --- a/frontend/src/renderer/components/ShellTopbar.tsx +++ b/frontend/src/renderer/components/ShellTopbar.tsx @@ -55,6 +55,7 @@ export function ShellTopbar() { const params = useParams({ strict: false }) as { projectId?: string; sessionId?: string }; const isInspectorOpen = useUiStore((state) => state.isInspectorOpen); const toggleInspector = useUiStore((state) => state.toggleInspector); + const restartingProjectIds = useUiStore((state) => state.restartingProjectIds); const [isSpawning, setIsSpawning] = useState(false); const [isNewTaskOpen, setIsNewTaskOpen] = useState(false); const all = useWorkspaceQuery().data ?? []; @@ -74,17 +75,18 @@ export function ShellTopbar() { const project = projectId ? all.find((workspace) => workspace.id === projectId) : undefined; const projectLabel = project?.name ?? session?.workspaceName ?? (projectId ? "" : "agent-orchestrator"); const orchestrator = projectId ? findProjectOrchestrator(all, projectId) : undefined; + const isProjectRestarting = projectId ? restartingProjectIds.has(projectId) : false; const openBoard = () => projectId ? void navigate({ to: "/projects/$projectId", params: { projectId } }) : void navigate({ to: "/" }); const openNewTask = () => { - if (!projectId) return; + if (!projectId || isProjectRestarting) return; setIsNewTaskOpen(true); }; const handleTaskCreated = async (sessionId: string) => { - if (!projectId) return; + if (!projectId || isProjectRestarting) return; await queryClient.invalidateQueries({ queryKey: workspaceQueryKey }); void navigate({ to: "/projects/$projectId/sessions/$sessionId", @@ -171,6 +173,7 @@ export function ShellTopbar() { )} {/* Inspector collapse (worker sessions only — orchestrators have no rail). */} diff --git a/frontend/src/renderer/components/Sidebar.tsx b/frontend/src/renderer/components/Sidebar.tsx index e1200349f..09923768c 100644 --- a/frontend/src/renderer/components/Sidebar.tsx +++ b/frontend/src/renderer/components/Sidebar.tsx @@ -16,7 +16,7 @@ import { import { useRef, useState, type ReactNode } from "react"; import { attentionZone, - isOrchestratorSession, + newestActiveOrchestrator, sessionIsActive, type WorkspaceSession, type WorkspaceSummary, @@ -426,16 +426,19 @@ function ProjectItem({ const [removeError, setRemoveError] = useState(null); const [isRemoving, setIsRemoving] = useState(false); const [isSpawning, setIsSpawning] = useState(false); + const restartingProjectIds = useUiStore((state) => state.restartingProjectIds); + const isProjectRestarting = restartingProjectIds.has(workspace.id); // Live workers only: merged/terminated sessions leave the sidebar and stay // reachable through the board's Done / Terminated bar (SessionsBoard). const sessions = workerSessions(workspace.sessions).filter(sessionIsActive); // The project's live orchestrator (if any) backs the hover Orchestrator // button: navigate to it when present, otherwise spawn one first. - const orchestrator = workspace.sessions.find((s) => isOrchestratorSession(s) && sessionIsActive(s)); + const orchestrator = newestActiveOrchestrator(workspace.sessions); // Mirrors ShellTopbar's launcher: attach to the running orchestrator, or // spawn one via the daemon and follow it once the workspace refetches. const openOrchestrator = async () => { + if (isProjectRestarting) return; if (orchestrator) { selection.goSession(workspace.id, orchestrator.id); return; @@ -546,7 +549,7 @@ function ProjectItem({ - {isSpawning ? "Spawning…" : orchestrator ? "Orchestrator" : "Spawn orchestrator"} + {isProjectRestarting ? "Restarting…" : isSpawning ? "Spawning…" : orchestrator ? "Orchestrator" : "Spawn orchestrator"} diff --git a/frontend/src/renderer/hooks/useWorkspaceQuery.test.tsx b/frontend/src/renderer/hooks/useWorkspaceQuery.test.tsx index 21b402872..60fc3c4e8 100644 --- a/frontend/src/renderer/hooks/useWorkspaceQuery.test.tsx +++ b/frontend/src/renderer/hooks/useWorkspaceQuery.test.tsx @@ -51,7 +51,16 @@ describe("useWorkspaceQuery", () => { it("maps projects and their sessions, applying provider/status/title fallbacks", async () => { respondWith({ projects: { - data: { projects: [{ id: "proj-1", name: "my-app", path: "/home/me/my-app" }] }, + data: { + projects: [ + { + id: "proj-1", + name: "my-app", + path: "/home/me/my-app", + orchestratorAgent: "codex", + }, + ], + }, error: undefined, }, sessions: { @@ -92,7 +101,7 @@ describe("useWorkspaceQuery", () => { await waitFor(() => expect(result.current.isSuccess).toBe(true)); const [workspace] = result.current.data ?? []; - expect(workspace).toMatchObject({ id: "proj-1", name: "my-app", path: "/home/me/my-app" }); + expect(workspace).toMatchObject({ id: "proj-1", name: "my-app", path: "/home/me/my-app", orchestratorAgent: "codex" }); expect(workspace.sessions).toHaveLength(2); expect(workspace.sessions[0]).toMatchObject({ id: "sess-1", diff --git a/frontend/src/renderer/hooks/useWorkspaceQuery.ts b/frontend/src/renderer/hooks/useWorkspaceQuery.ts index b46373aff..479323c78 100644 --- a/frontend/src/renderer/hooks/useWorkspaceQuery.ts +++ b/frontend/src/renderer/hooks/useWorkspaceQuery.ts @@ -44,6 +44,7 @@ async function fetchWorkspaces(): Promise { id: project.id, name: project.name, path: project.path, + orchestratorAgent: project.orchestratorAgent ? toAgentProvider(project.orchestratorAgent) : undefined, sessions: (sessionsData?.sessions ?? []) .filter((session) => session.projectId === project.id) .map((session) => ({ diff --git a/frontend/src/renderer/lib/orchestrator-replacement-telemetry.test.ts b/frontend/src/renderer/lib/orchestrator-replacement-telemetry.test.ts new file mode 100644 index 000000000..e56be81e1 --- /dev/null +++ b/frontend/src/renderer/lib/orchestrator-replacement-telemetry.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it, vi } from "vitest"; + +const { captureRendererExceptionMock } = vi.hoisted(() => ({ + captureRendererExceptionMock: vi.fn(), +})); + +vi.mock("./telemetry", () => ({ + captureRendererException: captureRendererExceptionMock, +})); + +import { captureOrchestratorReplacementFailure } from "./orchestrator-replacement-telemetry"; + +describe("captureOrchestratorReplacementFailure", () => { + it("records the shell restart-failure telemetry payload", () => { + const error = new Error("missing goose binary"); + + captureOrchestratorReplacementFailure(error, "proj-1"); + + expect(captureRendererExceptionMock).toHaveBeenCalledWith(error, { + source: "orchestrator-replace", + operation: "replace_orchestrator", + surface: "shell", + project_id: "proj-1", + }); + }); +}); diff --git a/frontend/src/renderer/lib/orchestrator-replacement-telemetry.ts b/frontend/src/renderer/lib/orchestrator-replacement-telemetry.ts new file mode 100644 index 000000000..64e28a826 --- /dev/null +++ b/frontend/src/renderer/lib/orchestrator-replacement-telemetry.ts @@ -0,0 +1,10 @@ +import { captureRendererException } from "./telemetry"; + +export function captureOrchestratorReplacementFailure(error: unknown, projectId: string) { + void captureRendererException(error, { + source: "orchestrator-replace", + operation: "replace_orchestrator", + surface: "shell", + project_id: projectId, + }); +} diff --git a/frontend/src/renderer/lib/restart-orchestrator.test.ts b/frontend/src/renderer/lib/restart-orchestrator.test.ts new file mode 100644 index 000000000..8b85b9bf8 --- /dev/null +++ b/frontend/src/renderer/lib/restart-orchestrator.test.ts @@ -0,0 +1,73 @@ +import { QueryClient } from "@tanstack/react-query"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { workspaceQueryKey } from "../hooks/useWorkspaceQuery"; + +const { spawnMock } = vi.hoisted(() => ({ + spawnMock: vi.fn(), +})); + +vi.mock("./spawn-orchestrator", () => ({ + spawnOrchestrator: spawnMock, +})); + +import { restartProjectOrchestrator } from "./restart-orchestrator"; + +describe("restartProjectOrchestrator", () => { + beforeEach(() => { + spawnMock.mockReset(); + }); + + it("invalidates workspace state and records an error when clean restart fails", async () => { + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries").mockResolvedValue(); + const navigate = vi.fn(); + const setProjectRestarting = vi.fn(); + const setOrchestratorReplacementError = vi.fn(); + const onError = vi.fn(); + const failure = new Error("missing goose binary"); + spawnMock.mockRejectedValue(failure); + + await restartProjectOrchestrator({ + projectId: "proj-1", + queryClient, + navigate, + setProjectRestarting, + setOrchestratorReplacementError, + onError, + }); + + expect(spawnMock).toHaveBeenCalledWith("proj-1", true); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: workspaceQueryKey }); + expect(setOrchestratorReplacementError).toHaveBeenNthCalledWith(1, "proj-1", null); + expect(setOrchestratorReplacementError).toHaveBeenNthCalledWith(2, "proj-1", "missing goose binary"); + expect(setProjectRestarting).toHaveBeenNthCalledWith(1, "proj-1", true); + expect(setProjectRestarting).toHaveBeenLastCalledWith("proj-1", false); + expect(onError).toHaveBeenCalledWith(failure); + expect(navigate).not.toHaveBeenCalled(); + }); + + it("still records the replacement error when workspace invalidation fails", async () => { + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + vi.spyOn(queryClient, "invalidateQueries").mockRejectedValue(new Error("refetch failed")); + const navigate = vi.fn(); + const setProjectRestarting = vi.fn(); + const setOrchestratorReplacementError = vi.fn(); + const onError = vi.fn(); + const failure = new Error("missing goose binary"); + spawnMock.mockRejectedValue(failure); + + await restartProjectOrchestrator({ + projectId: "proj-1", + queryClient, + navigate, + setProjectRestarting, + setOrchestratorReplacementError, + onError, + }); + + expect(setOrchestratorReplacementError).toHaveBeenLastCalledWith("proj-1", "missing goose binary"); + expect(setProjectRestarting).toHaveBeenLastCalledWith("proj-1", false); + expect(onError).toHaveBeenCalledWith(failure); + expect(navigate).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/renderer/lib/restart-orchestrator.ts b/frontend/src/renderer/lib/restart-orchestrator.ts new file mode 100644 index 000000000..0d41f7981 --- /dev/null +++ b/frontend/src/renderer/lib/restart-orchestrator.ts @@ -0,0 +1,52 @@ +import type { QueryClient } from "@tanstack/react-query"; +import { workspaceQueryKey } from "../hooks/useWorkspaceQuery"; +import { spawnOrchestrator } from "./spawn-orchestrator"; + +type NavigateToSession = (options: { + to: "/projects/$projectId/sessions/$sessionId"; + params: { projectId: string; sessionId: string }; +}) => unknown; + +type RestartProjectOrchestratorOptions = { + projectId: string; + queryClient: QueryClient; + navigate: NavigateToSession; + setProjectRestarting: (projectId: string, restarting: boolean) => void; + setOrchestratorReplacementError: (projectId: string, message: string | null) => void; + onError?: (error: unknown) => void; +}; + +async function refreshWorkspaceState(queryClient: QueryClient) { + try { + await queryClient.invalidateQueries({ queryKey: workspaceQueryKey }); + } catch { + // The restart outcome is more important than cache refresh bookkeeping: + // callers still need navigation/error state even if refetching fails. + } +} + +export async function restartProjectOrchestrator({ + projectId, + queryClient, + navigate, + setProjectRestarting, + setOrchestratorReplacementError, + onError, +}: RestartProjectOrchestratorOptions) { + setProjectRestarting(projectId, true); + setOrchestratorReplacementError(projectId, null); + try { + const sessionId = await spawnOrchestrator(projectId, true); + await refreshWorkspaceState(queryClient); + void navigate({ + to: "/projects/$projectId/sessions/$sessionId", + params: { projectId, sessionId }, + }); + } catch (error) { + await refreshWorkspaceState(queryClient); + setOrchestratorReplacementError(projectId, error instanceof Error ? error.message : "Could not replace orchestrator"); + onError?.(error); + } finally { + setProjectRestarting(projectId, false); + } +} diff --git a/frontend/src/renderer/routes/_shell.tsx b/frontend/src/renderer/routes/_shell.tsx index a6f300d71..8891662b2 100644 --- a/frontend/src/renderer/routes/_shell.tsx +++ b/frontend/src/renderer/routes/_shell.tsx @@ -2,6 +2,7 @@ import { createFileRoute, Outlet, useNavigate } from "@tanstack/react-router"; import { useQueryClient } from "@tanstack/react-query"; import { type CSSProperties, useCallback, useEffect, useRef } from "react"; import { ShellTopbar } from "../components/ShellTopbar"; +import { OrchestratorReplacementDialog } from "../components/OrchestratorReplacementDialog"; import { Sidebar } from "../components/Sidebar"; import { SidebarProvider } from "../components/ui/sidebar"; import { TitlebarNav } from "../components/TitlebarNav"; @@ -13,6 +14,8 @@ import { refreshDaemonStatus } from "../lib/daemon-status"; import { addRendererExceptionStep, captureRendererEvent, captureRendererException } from "../lib/telemetry"; import { ShellProvider } from "../lib/shell-context"; import { spawnOrchestrator } from "../lib/spawn-orchestrator"; +import { restartProjectOrchestrator } from "../lib/restart-orchestrator"; +import { captureOrchestratorReplacementFailure } from "../lib/orchestrator-replacement-telemetry"; import { readStoredTheme, type Theme, useUiStore } from "../stores/ui-store"; import type { WorkspaceSummary } from "../types/workspace"; import type { components } from "../../api/schema"; @@ -48,6 +51,10 @@ function ShellLayout() { const daemonStatus = useDaemonStatus(queryClient); const agentCatalogPortRef = useRef(undefined); const { theme, setTheme, isSidebarOpen, toggleSidebar } = useUiStore(); + const setProjectRestarting = useUiStore((state) => state.setProjectRestarting); + const orchestratorReplacementErrors = useUiStore((state) => state.orchestratorReplacementErrors); + const setOrchestratorReplacementError = useUiStore((state) => state.setOrchestratorReplacementError); + const replacementErrorProjectId = Object.keys(orchestratorReplacementErrors)[0] ?? null; const updateWorkspaces = useCallback( (updater: (workspaces: WorkspaceSummary[]) => WorkspaceSummary[]) => { @@ -99,6 +106,7 @@ function ShellLayout() { name: data.project.name, path: data.project.path, type: "main", + orchestratorAgent: input.orchestratorAgent as WorkspaceSummary["orchestratorAgent"], sessions: [], }; void captureRendererEvent("ao.renderer.project_add_succeeded", { project_id: workspace.id }); @@ -146,6 +154,22 @@ function ShellLayout() { [updateWorkspaces], ); + const restartOrchestrator = useCallback( + async (projectId: string) => { + await restartProjectOrchestrator({ + projectId, + queryClient, + navigate, + setProjectRestarting, + setOrchestratorReplacementError, + onError: (error) => { + captureOrchestratorReplacementFailure(error, projectId); + }, + }); + }, + [navigate, queryClient, setOrchestratorReplacementError, setProjectRestarting], + ); + useEffect(() => { document.documentElement.dataset.theme = theme; document.documentElement.style.colorScheme = theme; @@ -229,6 +253,15 @@ function ShellLayout() { by window-drag even though DOM hit-testing looks correct. */} + { + if (!open && replacementErrorProjectId) setOrchestratorReplacementError(replacementErrorProjectId, null); + }} + onRetry={(projectId) => void restartOrchestrator(projectId)} + projectId={replacementErrorProjectId} + workspaces={workspaces} + />
); diff --git a/frontend/src/renderer/stores/ui-store.ts b/frontend/src/renderer/stores/ui-store.ts index 237405512..7d50c8179 100644 --- a/frontend/src/renderer/stores/ui-store.ts +++ b/frontend/src/renderer/stores/ui-store.ts @@ -13,11 +13,15 @@ type UiState = { isSidebarOpen: boolean; isInspectorOpen: boolean; theme: Theme; + restartingProjectIds: ReadonlySet; + orchestratorReplacementErrors: Record; setWorkbenchTab: (tab: WorkbenchTab) => void; setTheme: (theme: Theme) => void; toggleTheme: () => void; toggleSidebar: () => void; toggleInspector: () => void; + setProjectRestarting: (projectId: string, restarting: boolean) => void; + setOrchestratorReplacementError: (projectId: string, message: string | null) => void; }; const sidebarStorageKey = "ao.sidebar.open"; @@ -58,6 +62,8 @@ export const useUiStore = create((set) => ({ isSidebarOpen: initialSidebarOpen(), isInspectorOpen: initialInspectorOpen(), theme: initialTheme(), + restartingProjectIds: new Set(), + orchestratorReplacementErrors: {}, setWorkbenchTab: (workbenchTab) => set({ workbenchTab }), setTheme: (theme) => { getLocalStorage()?.setItem(themeStorageKey, theme); @@ -81,4 +87,24 @@ export const useUiStore = create((set) => ({ getLocalStorage()?.setItem(inspectorStorageKey, String(isInspectorOpen)); return { isInspectorOpen }; }), + setProjectRestarting: (projectId, restarting) => + set((state) => { + const restartingProjectIds = new Set(state.restartingProjectIds); + if (restarting) { + restartingProjectIds.add(projectId); + } else { + restartingProjectIds.delete(projectId); + } + return { restartingProjectIds }; + }), + setOrchestratorReplacementError: (projectId, message) => + set((state) => { + const orchestratorReplacementErrors = { ...state.orchestratorReplacementErrors }; + if (message) { + orchestratorReplacementErrors[projectId] = message; + } else { + delete orchestratorReplacementErrors[projectId]; + } + return { orchestratorReplacementErrors }; + }), })); diff --git a/frontend/src/renderer/types/workspace.test.ts b/frontend/src/renderer/types/workspace.test.ts index 992db2685..1f9021da6 100644 --- a/frontend/src/renderer/types/workspace.test.ts +++ b/frontend/src/renderer/types/workspace.test.ts @@ -3,6 +3,8 @@ import { attentionZone, canonicalTrackerIssueId, findProjectOrchestrator, + newestActiveOrchestrator, + orchestratorHealth, sessionIsActive, sessionNeedsAttention, toAgentProvider, @@ -144,6 +146,50 @@ describe("findProjectOrchestrator", () => { const live = sessionWith({ id: "skills-5", kind: "orchestrator", status: "working" }); expect(findProjectOrchestrator([workspaceWith([live])], "other")).toBeUndefined(); }); + + it("selects the newest active orchestrator, not the first active one", () => { + const older = sessionWith({ + id: "skills-1", + kind: "orchestrator", + status: "working", + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + }); + const newer = sessionWith({ + id: "skills-2", + kind: "orchestrator", + status: "working", + createdAt: "2026-01-02T00:00:00Z", + updatedAt: "2026-01-02T00:00:00Z", + }); + expect(findProjectOrchestrator([workspaceWith([older, newer])], "skills")).toBe(newer); + }); + + it("uses updatedAt and id as newest orchestrator tie breakers", () => { + const oldUpdate = sessionWith({ + id: "skills-2", + kind: "orchestrator", + status: "working", + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + }); + const newUpdate = sessionWith({ + id: "skills-1", + kind: "orchestrator", + status: "working", + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-02T00:00:00Z", + }); + const sameTimesHigherID = sessionWith({ + id: "skills-3", + kind: "orchestrator", + status: "working", + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-02T00:00:00Z", + }); + expect(newestActiveOrchestrator([oldUpdate, newUpdate])).toBe(newUpdate); + expect(newestActiveOrchestrator([newUpdate, sameTimesHigherID])).toBe(sameTimesHigherID); + }); }); describe("sessionNeedsAttention", () => { @@ -164,6 +210,50 @@ describe("sessionNeedsAttention", () => { }); }); +describe("orchestratorHealth", () => { + it("reports restart_needed when the configured orchestrator agent differs from the newest active orchestrator", () => { + const older = sessionWith({ + id: "skills-1", + kind: "orchestrator", + provider: "codex", + status: "working", + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + }); + const newest = sessionWith({ + id: "skills-2", + kind: "orchestrator", + provider: "claude-code", + status: "working", + createdAt: "2026-01-02T00:00:00Z", + updatedAt: "2026-01-02T00:00:00Z", + }); + + expect( + orchestratorHealth({ + id: "skills", + name: "skills", + path: "/tmp/skills", + orchestratorAgent: "codex", + sessions: [older, newest], + }), + ).toEqual({ + state: "duplicates", + message: "Multiple orchestrators are active. The newest one is used; stale ones will be cleaned up on daemon reconcile.", + }); + + expect( + orchestratorHealth({ + id: "skills", + name: "skills", + path: "/tmp/skills", + orchestratorAgent: "codex", + sessions: [newest], + }).state, + ).toBe("restart_needed"); + }); +}); + describe("workerStatusPulses", () => { it("pulses only for working and needs_you", () => { expect(workerStatusPulses("working")).toBe(true); diff --git a/frontend/src/renderer/types/workspace.ts b/frontend/src/renderer/types/workspace.ts index d3e597fa4..9d0ad4587 100644 --- a/frontend/src/renderer/types/workspace.ts +++ b/frontend/src/renderer/types/workspace.ts @@ -241,14 +241,31 @@ export function findProjectOrchestrator( projectId: string, ): WorkspaceSession | undefined { const workspace = workspaces.find((w) => w.id === projectId); - if (!workspace) return undefined; - for (let i = workspace.sessions.length - 1; i >= 0; i -= 1) { - const session = workspace.sessions[i]; - if (isOrchestratorSession(session) && sessionIsActive(session)) { - return session; - } - } - return undefined; + return newestActiveOrchestrator(workspace?.sessions ?? []); +} + +export function newestActiveOrchestrator(sessions: WorkspaceSession[]): WorkspaceSession | undefined { + const active = sessions.filter((session) => isOrchestratorSession(session) && sessionIsActive(session)); + return active.reduce( + (newest, session) => (!newest || sessionNewer(session, newest) ? session : newest), + undefined, + ); +} + +function sessionNewer(a: WorkspaceSession, b: WorkspaceSession): boolean { + const aCreated = timestamp(a.createdAt); + const bCreated = timestamp(b.createdAt); + if (aCreated !== bCreated) return aCreated > bCreated; + const aUpdated = timestamp(a.updatedAt); + const bUpdated = timestamp(b.updatedAt); + if (aUpdated !== bUpdated) return aUpdated > bUpdated; + return a.id > b.id; +} + +function timestamp(value?: string): number { + if (!value) return 0; + const parsed = Date.parse(value); + return Number.isNaN(parsed) ? 0 : parsed; } export function workerSessions(sessions: WorkspaceSession[]): WorkspaceSession[] { @@ -340,6 +357,7 @@ export type WorkspaceSummary = { name: string; path: string; type?: "main" | "worktree"; + orchestratorAgent?: AgentProvider; accentColor?: string; diff?: { additions: number; @@ -348,6 +366,42 @@ export type WorkspaceSummary = { sessions: WorkspaceSession[]; }; +export function orchestratorNeedsRestart(workspace: WorkspaceSummary, orchestrator?: WorkspaceSession): boolean { + if (!orchestrator || !workspace.orchestratorAgent) return false; + return orchestrator.provider !== workspace.orchestratorAgent; +} + +export type OrchestratorHealth = + | { state: "ok" } + | { state: "restarting"; message: string } + | { state: "restart_needed"; message: string } + | { state: "missing"; message: string } + | { state: "duplicates"; message: string }; + +export function orchestratorHealth(workspace: WorkspaceSummary, restarting = false): OrchestratorHealth { + if (restarting) { + return { state: "restarting", message: "Restarting orchestrator. New tasks wait until the replacement is ready." }; + } + const active = workspace.sessions.filter((session) => isOrchestratorSession(session) && sessionIsActive(session)); + if (active.length > 1) { + return { + state: "duplicates", + message: "Multiple orchestrators are active. The newest one is used; stale ones will be cleaned up on daemon reconcile.", + }; + } + const orchestrator = newestActiveOrchestrator(workspace.sessions); + if (!orchestrator) { + return { state: "missing", message: "No orchestrator is running for this project." }; + } + if (orchestratorNeedsRestart(workspace, orchestrator)) { + return { + state: "restart_needed", + message: `Configured orchestrator agent is ${workspace.orchestratorAgent}; running agent is ${orchestrator.provider}.`, + }; + } + return { state: "ok" }; +} + export function toAgentProvider(provider?: string): AgentProvider { switch (provider) { case "claude-code":