From 5029ae56852eb182919585dcfa7fedd3889ab53f Mon Sep 17 00:00:00 2001 From: nikhil achale Date: Wed, 1 Jul 2026 23:48:38 +0530 Subject: [PATCH 1/9] fix: replace orchestrators through canonical handoff --- 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 | 57 +++++++++ .../OrchestratorReplacementDialog.tsx | 75 ++++++++++++ .../src/renderer/components/SessionsBoard.tsx | 59 ++++++++- .../src/renderer/components/ShellTopbar.tsx | 11 +- frontend/src/renderer/components/Sidebar.tsx | 7 +- frontend/src/renderer/routes/_shell.tsx | 42 +++++++ frontend/src/renderer/stores/ui-store.ts | 26 ++++ frontend/src/renderer/types/workspace.ts | 37 ++++++ 11 files changed, 533 insertions(+), 27 deletions(-) create mode 100644 frontend/src/renderer/components/OrchestratorReplacementDialog.tsx diff --git a/backend/internal/service/session/service.go b/backend/internal/service/session/service.go index 32d338897a..0851f3e377 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 + } + } + 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 s.Spawn(ctx, ports.SpawnConfig{ProjectID: projectID, Kind: domain.KindOrchestrator}) + 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 c5b1b1ca64..9dea20257d 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 4ae540c72c..52d079ed05 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) + } + if err := m.lcm.MarkTerminated(ctx, id); err != nil { + return fmt.Errorf("retire replacement %s: mark terminated: %w", id, err) + } + handle := runtimeHandle(rec.Metadata) + if handle.ID != "" { + if err := m.runtime.Destroy(ctx, handle); err != nil { + m.logger.Warn("retire replacement: runtime destroy failed", "sessionID", rec.ID, "error", 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) + } + if err := m.lcm.MarkTerminated(ctx, rec.ID); err != nil { + return fmt.Errorf("retire replacement %s: mark terminated: %w", id, err) + } + handle := runtimeHandle(rec.Metadata) + if handle.ID != "" { + if err := m.runtime.Destroy(ctx, handle); err != nil { + m.logger.Warn("retire replacement: runtime destroy failed", "sessionID", rec.ID, "error", err) + } + } + if err := m.workspace.ForceDestroy(ctx, ws); err != nil { + return fmt.Errorf("retire replacement %s: force destroy: %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 0ea59273d5..9e69063491 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 } @@ -1425,6 +1428,60 @@ 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) + } +} + // 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/renderer/components/OrchestratorReplacementDialog.tsx b/frontend/src/renderer/components/OrchestratorReplacementDialog.tsx new file mode 100644 index 0000000000..c9f3817e67 --- /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/SessionsBoard.tsx b/frontend/src/renderer/components/SessionsBoard.tsx index 7da1b8a303..5aed131520 100644 --- a/frontend/src/renderer/components/SessionsBoard.tsx +++ b/frontend/src/renderer/components/SessionsBoard.tsx @@ -1,8 +1,14 @@ import { useState, type KeyboardEvent } from "react"; import { useQueryClient } from "@tanstack/react-query"; import { useNavigate } from "@tanstack/react-router"; -import { Plus } from "lucide-react"; -import { type AttentionZone, type WorkspaceSession, attentionZone, workerSessions } from "../types/workspace"; +import { AlertTriangle, Plus, RotateCw } from "lucide-react"; +import { + type AttentionZone, + type WorkspaceSession, + attentionZone, + orchestratorHealth, + workerSessions, +} from "../types/workspace"; import { useSessionScmSummary, type SessionPRSummary } from "../hooks/useSessionScmSummary"; import { useWorkspaceQuery, workspaceQueryKey } from "../hooks/useWorkspaceQuery"; import { DashboardSubhead } from "./DashboardSubhead"; @@ -12,6 +18,7 @@ import { spawnOrchestrator } from "../lib/spawn-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. */ @@ -70,12 +77,18 @@ 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 [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) { @@ -94,7 +107,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", @@ -115,6 +128,24 @@ export function SessionsBoard({ projectId }: SessionsBoardProps) { } }; + const restartOrchestrator = async () => { + if (!projectId) return; + setProjectRestarting(projectId, true); + setOrchestratorReplacementError(projectId, null); + try { + const sessionId = await spawnOrchestrator(projectId, true); + await queryClient.invalidateQueries({ queryKey: workspaceQueryKey }); + void navigate({ + to: "/projects/$projectId/sessions/$sessionId", + params: { projectId, sessionId }, + }); + } catch (error) { + setOrchestratorReplacementError(projectId, error instanceof Error ? error.message : "Could not replace orchestrator"); + } finally { + setProjectRestarting(projectId, false); + } + }; + const handleTaskCreated = async (sessionId: string) => { if (!projectId) return; await queryClient.invalidateQueries({ queryKey: workspaceQueryKey }); @@ -129,6 +160,7 @@ export function SessionsBoard({ projectId }: SessionsBoardProps) { ) : undefined; @@ -157,6 +189,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 c336b19cb3..2929475c71 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 c9567db77d..d2cb7ae28b 100644 --- a/frontend/src/renderer/components/Sidebar.tsx +++ b/frontend/src/renderer/components/Sidebar.tsx @@ -405,6 +405,8 @@ 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); @@ -415,6 +417,7 @@ function ProjectItem({ // 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; @@ -525,7 +528,7 @@ function ProjectItem({ - {isSpawning ? "Spawning…" : orchestrator ? "Orchestrator" : "Spawn orchestrator"} + {isProjectRestarting ? "Restarting…" : isSpawning ? "Spawning…" : orchestrator ? "Orchestrator" : "Spawn orchestrator"} diff --git a/frontend/src/renderer/routes/_shell.tsx b/frontend/src/renderer/routes/_shell.tsx index 72c12a11e0..de0ed574e3 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 } 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"; @@ -45,6 +46,10 @@ function ShellLayout() { const workspaces = workspaceQuery.data ?? []; const daemonStatus = useDaemonStatus(queryClient); 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[]) => { @@ -86,6 +91,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 }); @@ -133,6 +139,33 @@ function ShellLayout() { [updateWorkspaces], ); + const restartOrchestrator = useCallback( + async (projectId: string) => { + setProjectRestarting(projectId, true); + setOrchestratorReplacementError(projectId, null); + try { + const sessionId = await spawnOrchestrator(projectId, true); + await queryClient.invalidateQueries({ queryKey: workspaceQueryKey }); + void navigate({ + to: "/projects/$projectId/sessions/$sessionId", + params: { projectId, sessionId }, + }); + } catch (error) { + const message = error instanceof Error ? error.message : "Could not replace orchestrator"; + setOrchestratorReplacementError(projectId, message); + void captureRendererException(error, { + source: "orchestrator-replace", + operation: "replace_orchestrator", + surface: "shell", + project_id: projectId, + }); + } finally { + setProjectRestarting(projectId, false); + } + }, + [navigate, queryClient, setOrchestratorReplacementError, setProjectRestarting], + ); + useEffect(() => { document.documentElement.dataset.theme = theme; document.documentElement.style.colorScheme = theme; @@ -206,6 +239,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 237405512a..7d50c81799 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.ts b/frontend/src/renderer/types/workspace.ts index afb69b780e..fbbda857e2 100644 --- a/frontend/src/renderer/types/workspace.ts +++ b/frontend/src/renderer/types/workspace.ts @@ -291,6 +291,7 @@ export type WorkspaceSummary = { name: string; path: string; type?: "main" | "worktree"; + orchestratorAgent?: AgentProvider; accentColor?: string; diff?: { additions: number; @@ -299,6 +300,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 = active[0]; + 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": From 82c32c64ac02a5f0a3e983601d6986df6177677a Mon Sep 17 00:00:00 2001 From: nikhil achale Date: Wed, 1 Jul 2026 23:54:29 +0530 Subject: [PATCH 2/9] fix(tests): correct logic in TestRetireForReplacementCapturesAndReleasesWorkspace --- backend/internal/session_manager/manager_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/internal/session_manager/manager_test.go b/backend/internal/session_manager/manager_test.go index 9e69063491..e3d5fe9a42 100644 --- a/backend/internal/session_manager/manager_test.go +++ b/backend/internal/session_manager/manager_test.go @@ -1477,7 +1477,7 @@ func TestRetireForReplacementCapturesAndReleasesWorkspace(t *testing.T) { if stashIdx == -1 || deleteIdx == -1 || forceIdx == -1 { t.Fatalf("missing expected calls in shared log: %v", sharedLog) } - if !(stashIdx < deleteIdx && deleteIdx < forceIdx) { + if stashIdx >= deleteIdx || deleteIdx >= forceIdx { t.Fatalf("replacement retire must capture, clear restore marker, then force release; log=%v", sharedLog) } } From 3983d37c44fb96235ee2efc3997bf189f4ff9025 Mon Sep 17 00:00:00 2001 From: nikhil achale Date: Thu, 2 Jul 2026 03:29:24 +0530 Subject: [PATCH 3/9] fix: enhance tests and add project config handling in service and workspace components --- backend/internal/cli/stop_test.go | 1 + backend/internal/httpd/apispec/openapi.yaml | 2 + backend/internal/httpd/server_test.go | 2 + backend/internal/observe/scm/observer_test.go | 1 + backend/internal/runfile/runfile_test.go | 4 + backend/internal/service/project/service.go | 14 ++- .../internal/service/project/service_test.go | 25 ++++++ backend/internal/service/project/types.go | 13 +-- backend/internal/session_manager/manager.go | 6 +- .../internal/session_manager/manager_test.go | 34 +++++++ frontend/src/api/schema.ts | 1 + .../src/renderer/components/SessionsBoard.tsx | 5 +- frontend/src/renderer/components/Sidebar.tsx | 4 +- .../renderer/hooks/useWorkspaceQuery.test.tsx | 13 ++- .../src/renderer/hooks/useWorkspaceQuery.ts | 1 + frontend/src/renderer/types/workspace.test.ts | 90 +++++++++++++++++++ frontend/src/renderer/types/workspace.ts | 28 +++++- 17 files changed, 222 insertions(+), 22 deletions(-) diff --git a/backend/internal/cli/stop_test.go b/backend/internal/cli/stop_test.go index 0a150d6330..9a298ecae4 100644 --- a/backend/internal/cli/stop_test.go +++ b/backend/internal/cli/stop_test.go @@ -43,6 +43,7 @@ func TestWaitForStoppedKeepsRunFileFromConcurrentStart(t *testing.T) { } if info == nil { t.Fatal("new daemon's run-file was deleted by stop of a different PID") + return } if info.PID != newPID { t.Fatalf("run-file PID = %d, want %d (new daemon)", info.PID, newPID) diff --git a/backend/internal/httpd/apispec/openapi.yaml b/backend/internal/httpd/apispec/openapi.yaml index c194ffa0bd..8d642702a0 100644 --- a/backend/internal/httpd/apispec/openapi.yaml +++ b/backend/internal/httpd/apispec/openapi.yaml @@ -1968,6 +1968,8 @@ components: type: object ProjectSummary: properties: + config: + $ref: '#/components/schemas/ProjectConfig' id: type: string kind: diff --git a/backend/internal/httpd/server_test.go b/backend/internal/httpd/server_test.go index 299d218f6c..a315c78a02 100644 --- a/backend/internal/httpd/server_test.go +++ b/backend/internal/httpd/server_test.go @@ -96,6 +96,7 @@ func TestServerLifecycle(t *testing.T) { } ctx, cancel := context.WithCancel(context.Background()) + defer cancel() runErr := make(chan error, 1) go func() { runErr <- srv.Run(ctx) }() @@ -109,6 +110,7 @@ func TestServerLifecycle(t *testing.T) { } if info == nil { t.Fatal("run-file not written while server running") + return } if info.Port == 0 { t.Error("run-file recorded port 0; want the actual bound port") diff --git a/backend/internal/observe/scm/observer_test.go b/backend/internal/observe/scm/observer_test.go index dec26750a5..6e02ca3507 100644 --- a/backend/internal/observe/scm/observer_test.go +++ b/backend/internal/observe/scm/observer_test.go @@ -565,6 +565,7 @@ func TestPoll_DiscoveredPRPersistedAsBaselineBeforeRefresh(t *testing.T) { } if baseline == nil { t.Fatalf("discovered PR #1 not persisted as a baseline row; writes=%#v", store.writes) + return } if baseline.Merged || baseline.Closed { t.Fatalf("baseline row must be open, got merged=%v closed=%v", baseline.Merged, baseline.Closed) diff --git a/backend/internal/runfile/runfile_test.go b/backend/internal/runfile/runfile_test.go index 87b62b320f..162421e401 100644 --- a/backend/internal/runfile/runfile_test.go +++ b/backend/internal/runfile/runfile_test.go @@ -20,6 +20,7 @@ func TestWriteReadRoundTrip(t *testing.T) { } if got == nil { t.Fatal("Read returned nil for an existing file") + return } if got.PID != want.PID || got.Port != want.Port || !got.StartedAt.Equal(want.StartedAt) { t.Errorf("round trip mismatch: got %+v, want %+v", *got, want) @@ -44,6 +45,7 @@ func TestWriteReadRoundTripOwner(t *testing.T) { } if got == nil { t.Fatal("Read returned nil for an existing file") + return } if got.Owner != "app" { t.Errorf("Owner round trip: got %q, want %q", got.Owner, "app") @@ -60,6 +62,7 @@ func TestWriteReadRoundTripOwner(t *testing.T) { } if got == nil { t.Fatal("Read returned nil for headless file") + return } if got.Owner != "" { t.Errorf("headless Owner round trip: got %q, want %q", got.Owner, "") @@ -164,6 +167,7 @@ func TestCheckStaleLivePID(t *testing.T) { } if live == nil { t.Fatal("CheckStale on live PID = nil, want the live Info") + return } if live.PID != os.Getpid() { t.Errorf("live.PID = %d, want %d", live.PID, os.Getpid()) diff --git a/backend/internal/service/project/service.go b/backend/internal/service/project/service.go index af8593737f..92a6abf30e 100644 --- a/backend/internal/service/project/service.go +++ b/backend/internal/service/project/service.go @@ -94,6 +94,7 @@ func (m *Service) List(ctx context.Context) ([]Summary, error) { Path: row.Path, Kind: row.Kind.WithDefault(), SessionPrefix: resolveSessionPrefix(row), + Config: projectConfigPtr(row.Config), }) } return out, nil @@ -374,13 +375,18 @@ func projectFromRow(row domain.ProjectRecord) Project { Repo: row.RepoOriginURL, DefaultBranch: row.Config.WithDefaults().DefaultBranch, } - if !row.Config.IsZero() { - cfg := row.Config - p.Config = &cfg - } + p.Config = projectConfigPtr(row.Config) return p } +func projectConfigPtr(config domain.ProjectConfig) *domain.ProjectConfig { + if config.IsZero() { + return nil + } + cfg := config + 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 4901b83bdc..8b22d4dea9 100644 --- a/backend/internal/service/project/service_test.go +++ b/backend/internal/service/project/service_test.go @@ -407,6 +407,31 @@ func TestManager_SetConfig(t *testing.T) { wantCode(t, err, "PROJECT_NOT_FOUND") } +func TestManager_ListIncludesProjectConfig(t *testing.T) { + ctx := context.Background() + m := newManager(t) + repo := gitRepo(t) + + cfg := domain.ProjectConfig{ + DefaultBranch: "develop", + 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].Config == nil || list[0].Config.Orchestrator.Harness != domain.HarnessCodex { + t.Fatalf("summary config = %#v, want orchestrator codex", list[0].Config) + } +} + 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 f33b338d58..4281ea5c7a 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"` + Config *domain.ProjectConfig `json:"config,omitempty"` + ResolveError string `json:"resolveError,omitempty"` } // Project is the full read-model returned by GET /api/v1/projects/{id}. diff --git a/backend/internal/session_manager/manager.go b/backend/internal/session_manager/manager.go index 52d079ed05..28546df5cf 100644 --- a/backend/internal/session_manager/manager.go +++ b/backend/internal/session_manager/manager.go @@ -514,9 +514,6 @@ func (m *Manager) RetireForReplacement(ctx context.Context, id domain.SessionID) if err := m.store.DeleteSessionWorktrees(ctx, rec.ID); err != nil { return fmt.Errorf("retire replacement %s: clear restore markers: %w", id, err) } - if err := m.lcm.MarkTerminated(ctx, rec.ID); err != nil { - return fmt.Errorf("retire replacement %s: mark terminated: %w", id, err) - } handle := runtimeHandle(rec.Metadata) if handle.ID != "" { if err := m.runtime.Destroy(ctx, handle); err != nil { @@ -526,6 +523,9 @@ func (m *Manager) RetireForReplacement(ctx context.Context, id domain.SessionID) 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 } diff --git a/backend/internal/session_manager/manager_test.go b/backend/internal/session_manager/manager_test.go index e3d5fe9a42..00245e5948 100644 --- a/backend/internal/session_manager/manager_test.go +++ b/backend/internal/session_manager/manager_test.go @@ -1482,6 +1482,40 @@ func TestRetireForReplacementCapturesAndReleasesWorkspace(t *testing.T) { } } +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) + } +} + // 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 9ee643dadf..901c5698d9 100644 --- a/frontend/src/api/schema.ts +++ b/frontend/src/api/schema.ts @@ -691,6 +691,7 @@ export interface components { project: components["schemas"]["Project"]; }; ProjectSummary: { + config?: components["schemas"]["ProjectConfig"]; id: string; kind: string; name: string; diff --git a/frontend/src/renderer/components/SessionsBoard.tsx b/frontend/src/renderer/components/SessionsBoard.tsx index 5aed131520..4897240d6c 100644 --- a/frontend/src/renderer/components/SessionsBoard.tsx +++ b/frontend/src/renderer/components/SessionsBoard.tsx @@ -6,6 +6,7 @@ import { type AttentionZone, type WorkspaceSession, attentionZone, + newestActiveOrchestrator, orchestratorHealth, workerSessions, } from "../types/workspace"; @@ -79,9 +80,7 @@ export function SessionsBoard({ projectId }: SessionsBoardProps) { 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); diff --git a/frontend/src/renderer/components/Sidebar.tsx b/frontend/src/renderer/components/Sidebar.tsx index d2cb7ae28b..fefdecc33f 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, @@ -412,7 +412,7 @@ function ProjectItem({ 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. diff --git a/frontend/src/renderer/hooks/useWorkspaceQuery.test.tsx b/frontend/src/renderer/hooks/useWorkspaceQuery.test.tsx index a81309c177..16599d36b6 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", + config: { orchestrator: { agent: "codex" } }, + }, + ], + }, error: undefined, }, sessions: { @@ -90,7 +99,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 0967d8d31c..bff939ee20 100644 --- a/frontend/src/renderer/hooks/useWorkspaceQuery.ts +++ b/frontend/src/renderer/hooks/useWorkspaceQuery.ts @@ -43,6 +43,7 @@ async function fetchWorkspaces(): Promise { id: project.id, name: project.name, path: project.path, + orchestratorAgent: project.config?.orchestrator?.agent ? toAgentProvider(project.config.orchestrator.agent) : undefined, sessions: (sessionsData?.sessions ?? []) .filter((session) => session.projectId === project.id) .map((session) => ({ diff --git a/frontend/src/renderer/types/workspace.test.ts b/frontend/src/renderer/types/workspace.test.ts index c0ffab1274..04c7132009 100644 --- a/frontend/src/renderer/types/workspace.test.ts +++ b/frontend/src/renderer/types/workspace.test.ts @@ -2,6 +2,8 @@ import { describe, expect, it } from "vitest"; import { attentionZone, findProjectOrchestrator, + newestActiveOrchestrator, + orchestratorHealth, sessionIsActive, sessionNeedsAttention, toAgentProvider, @@ -129,6 +131,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", () => { @@ -149,6 +195,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 fbbda857e2..1f195353f3 100644 --- a/frontend/src/renderer/types/workspace.ts +++ b/frontend/src/renderer/types/workspace.ts @@ -199,7 +199,31 @@ export function findProjectOrchestrator( projectId: string, ): WorkspaceSession | undefined { const workspace = workspaces.find((w) => w.id === projectId); - return workspace?.sessions.find((session) => isOrchestratorSession(session) && sessionIsActive(session)); + 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[] { @@ -323,7 +347,7 @@ export function orchestratorHealth(workspace: WorkspaceSummary, restarting = fal message: "Multiple orchestrators are active. The newest one is used; stale ones will be cleaned up on daemon reconcile.", }; } - const orchestrator = active[0]; + const orchestrator = newestActiveOrchestrator(workspace.sessions); if (!orchestrator) { return { state: "missing", message: "No orchestrator is running for this project." }; } From b300c6c2b7bd15203040e223c6b6ec517aa3c709 Mon Sep 17 00:00:00 2001 From: nikhil achale Date: Fri, 3 Jul 2026 01:51:59 +0530 Subject: [PATCH 4/9] fix: update project summary to include orchestrator agent and adjust related tests --- backend/internal/httpd/apispec/openapi.yaml | 4 ++-- backend/internal/service/project/service.go | 12 ++++++------ backend/internal/service/project/service_test.go | 7 ++++--- backend/internal/service/project/types.go | 14 +++++++------- frontend/src/api/schema.ts | 2 +- .../src/renderer/hooks/useWorkspaceQuery.test.tsx | 2 +- frontend/src/renderer/hooks/useWorkspaceQuery.ts | 2 +- 7 files changed, 22 insertions(+), 21 deletions(-) diff --git a/backend/internal/httpd/apispec/openapi.yaml b/backend/internal/httpd/apispec/openapi.yaml index 8d642702a0..91223a64c0 100644 --- a/backend/internal/httpd/apispec/openapi.yaml +++ b/backend/internal/httpd/apispec/openapi.yaml @@ -1968,14 +1968,14 @@ components: type: object ProjectSummary: properties: - config: - $ref: '#/components/schemas/ProjectConfig' id: type: string kind: 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 92a6abf30e..36f2110ff1 100644 --- a/backend/internal/service/project/service.go +++ b/backend/internal/service/project/service.go @@ -89,12 +89,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), - Config: projectConfigPtr(row.Config), + 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 diff --git a/backend/internal/service/project/service_test.go b/backend/internal/service/project/service_test.go index 8b22d4dea9..9feb02672b 100644 --- a/backend/internal/service/project/service_test.go +++ b/backend/internal/service/project/service_test.go @@ -407,13 +407,14 @@ func TestManager_SetConfig(t *testing.T) { wantCode(t, err, "PROJECT_NOT_FOUND") } -func TestManager_ListIncludesProjectConfig(t *testing.T) { +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 { @@ -427,8 +428,8 @@ func TestManager_ListIncludesProjectConfig(t *testing.T) { if len(list) != 1 { t.Fatalf("List len = %d, want 1", len(list)) } - if list[0].Config == nil || list[0].Config.Orchestrator.Harness != domain.HarnessCodex { - t.Fatalf("summary config = %#v, want orchestrator codex", list[0].Config) + if list[0].OrchestratorAgent != domain.HarnessCodex { + t.Fatalf("summary orchestrator agent = %q, want codex", list[0].OrchestratorAgent) } } diff --git a/backend/internal/service/project/types.go b/backend/internal/service/project/types.go index 4281ea5c7a..81e5da2be4 100644 --- a/backend/internal/service/project/types.go +++ b/backend/internal/service/project/types.go @@ -4,13 +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"` - Config *domain.ProjectConfig `json:"config,omitempty"` - 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/frontend/src/api/schema.ts b/frontend/src/api/schema.ts index 901c5698d9..bed0f37eed 100644 --- a/frontend/src/api/schema.ts +++ b/frontend/src/api/schema.ts @@ -691,10 +691,10 @@ export interface components { project: components["schemas"]["Project"]; }; ProjectSummary: { - config?: components["schemas"]["ProjectConfig"]; id: string; kind: string; name: string; + orchestratorAgent?: string; path: string; resolveError?: string; sessionPrefix: string; diff --git a/frontend/src/renderer/hooks/useWorkspaceQuery.test.tsx b/frontend/src/renderer/hooks/useWorkspaceQuery.test.tsx index 16599d36b6..138156056f 100644 --- a/frontend/src/renderer/hooks/useWorkspaceQuery.test.tsx +++ b/frontend/src/renderer/hooks/useWorkspaceQuery.test.tsx @@ -57,7 +57,7 @@ describe("useWorkspaceQuery", () => { id: "proj-1", name: "my-app", path: "/home/me/my-app", - config: { orchestrator: { agent: "codex" } }, + orchestratorAgent: "codex", }, ], }, diff --git a/frontend/src/renderer/hooks/useWorkspaceQuery.ts b/frontend/src/renderer/hooks/useWorkspaceQuery.ts index bff939ee20..faeb36db14 100644 --- a/frontend/src/renderer/hooks/useWorkspaceQuery.ts +++ b/frontend/src/renderer/hooks/useWorkspaceQuery.ts @@ -43,7 +43,7 @@ async function fetchWorkspaces(): Promise { id: project.id, name: project.name, path: project.path, - orchestratorAgent: project.config?.orchestrator?.agent ? toAgentProvider(project.config.orchestrator.agent) : undefined, + orchestratorAgent: project.orchestratorAgent ? toAgentProvider(project.orchestratorAgent) : undefined, sessions: (sessionsData?.sessions ?? []) .filter((session) => session.projectId === project.id) .map((session) => ({ From 5f9e23a9d8eadb83e980f016c5799108964513c4 Mon Sep 17 00:00:00 2001 From: nikhil achale Date: Fri, 3 Jul 2026 13:25:00 +0530 Subject: [PATCH 5/9] fix: block orchestrator replacement on runtime teardown failure --- backend/internal/session_manager/manager.go | 10 ++--- .../internal/session_manager/manager_test.go | 39 ++++++++++++++++++- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/backend/internal/session_manager/manager.go b/backend/internal/session_manager/manager.go index 28546df5cf..b059b60dd1 100644 --- a/backend/internal/session_manager/manager.go +++ b/backend/internal/session_manager/manager.go @@ -495,15 +495,15 @@ func (m *Manager) RetireForReplacement(ctx context.Context, id domain.SessionID) if err := m.store.DeleteSessionWorktrees(ctx, rec.ID); err != nil { return fmt.Errorf("retire replacement %s: clear restore markers: %w", id, err) } - if err := m.lcm.MarkTerminated(ctx, id); err != nil { - return fmt.Errorf("retire replacement %s: mark terminated: %w", id, err) - } handle := runtimeHandle(rec.Metadata) if handle.ID != "" { if err := m.runtime.Destroy(ctx, handle); err != nil { - m.logger.Warn("retire replacement: runtime destroy failed", "sessionID", rec.ID, "error", err) + 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 } @@ -517,7 +517,7 @@ func (m *Manager) RetireForReplacement(ctx context.Context, id domain.SessionID) handle := runtimeHandle(rec.Metadata) if handle.ID != "" { if err := m.runtime.Destroy(ctx, handle); err != nil { - m.logger.Warn("retire replacement: runtime destroy failed", "sessionID", rec.ID, "error", err) + return fmt.Errorf("retire replacement %s: runtime: %w", id, err) } } if err := m.workspace.ForceDestroy(ctx, ws); err != nil { diff --git a/backend/internal/session_manager/manager_test.go b/backend/internal/session_manager/manager_test.go index 00245e5948..04f193c19f 100644 --- a/backend/internal/session_manager/manager_test.go +++ b/backend/internal/session_manager/manager_test.go @@ -151,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. @@ -170,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 { @@ -1516,6 +1517,42 @@ func TestRetireForReplacementForceDestroyFailureLeavesSessionActive(t *testing.T } } +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. From 86d95926bf3f49163574c6334f7f631e1c902a4b Mon Sep 17 00:00:00 2001 From: nikhil achale Date: Fri, 3 Jul 2026 15:33:47 +0530 Subject: [PATCH 6/9] fix: enhance orchestrator handling in ProjectSettingsForm and SessionsBoard components --- .../components/ProjectSettingsForm.test.tsx | 118 +++++++++++++++++- .../components/ProjectSettingsForm.tsx | 31 ++++- .../src/renderer/components/SessionsBoard.tsx | 22 ++-- ...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 ++--- 8 files changed, 324 insertions(+), 41 deletions(-) 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/frontend/src/renderer/components/ProjectSettingsForm.test.tsx b/frontend/src/renderer/components/ProjectSettingsForm.test.tsx index 26776054dc..2be0b6b164 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( @@ -47,7 +54,9 @@ async function chooseOption(trigger: HTMLElement, optionName: string) { 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", () => { @@ -135,6 +144,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); @@ -168,6 +181,7 @@ describe("ProjectSettingsForm", () => { expect(await screen.findByText("invalid permissions")).toBeInTheDocument(); expect(screen.queryByText("Saved.")).not.toBeInTheDocument(); + expect(postMock).not.toHaveBeenCalled(); }); it("defaults worker and orchestrator to claude-code for projects missing role config", async () => { @@ -208,6 +222,104 @@ describe("ProjectSettingsForm", () => { }, }, }); + expect(postMock).not.toHaveBeenCalled(); expect(await screen.findByText("Saved.")).toBeInTheDocument(); }); -}); \ No newline at end of file + + 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 ea51b7fd4d..d5ac48a903 100644 --- a/frontend/src/renderer/components/ProjectSettingsForm.tsx +++ b/frontend/src/renderer/components/ProjectSettingsForm.tsx @@ -3,7 +3,9 @@ import { useState } from "react"; import type { components } from "../../api/schema"; import { apiClient, apiErrorMessage } from "../lib/api-client"; import { DEFAULT_PROJECT_AGENT } from "../lib/agent-options"; -import { workspaceQueryKey } from "../hooks/useWorkspaceQuery"; +import { useWorkspaceQuery, workspaceQueryKey } from "../hooks/useWorkspaceQuery"; +import { spawnOrchestrator } from "../lib/spawn-orchestrator"; +import { newestActiveOrchestrator } from "../types/workspace"; import { RequiredAgentField } from "./CreateProjectAgentSheet"; import { DashboardSubhead } from "./DashboardSubhead"; import { Button } from "./ui/button"; @@ -66,7 +68,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 [form, setForm] = useState({ defaultBranch: config.defaultBranch ?? project.defaultBranch ?? "", sessionPrefix: config.sessionPrefix ?? "", @@ -77,6 +82,8 @@ function SettingsBody({ project, projectId, onSaved }: { project: Project; proje reviewerHarness: config.reviewers?.[0]?.harness ?? "", }); const [savedAt, setSavedAt] = useState(null); + const [replacementError, setReplacementError] = useState(null); + const initialOrchestratorAgent = config.orchestrator?.agent || DEFAULT_PROJECT_AGENT; const mutation = useMutation({ mutationFn: async () => { @@ -100,9 +107,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); void queryClient.invalidateQueries({ queryKey: ["project", projectId] }); onSaved(); }, @@ -114,6 +135,7 @@ function SettingsBody({ project, projectId, onSaved }: { project: Project; proje onSubmit={(event) => { event.preventDefault(); setSavedAt(null); + setReplacementError(null); mutation.mutate(); }} > @@ -219,6 +241,9 @@ function SettingsBody({ project, projectId, onSaved }: { project: Project; proje {savedAt && !mutation.isPending && !mutation.isError && ( Saved. )} + {replacementError && !mutation.isPending && !mutation.isError && ( + Orchestrator restart failed: {replacementError} + )} ); @@ -300,4 +325,4 @@ function CenteredNote({ children }: { children: React.ReactNode }) { // rather than an empty {} the daemon would persist. function blankToUndefined(obj: T): T | undefined { return Object.values(obj).some((v) => v !== undefined) ? obj : undefined; -} \ No newline at end of file +} diff --git a/frontend/src/renderer/components/SessionsBoard.tsx b/frontend/src/renderer/components/SessionsBoard.tsx index 4897240d6c..bf4d69c576 100644 --- a/frontend/src/renderer/components/SessionsBoard.tsx +++ b/frontend/src/renderer/components/SessionsBoard.tsx @@ -16,6 +16,7 @@ import { DashboardSubhead } from "./DashboardSubhead"; 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"; @@ -129,20 +130,13 @@ export function SessionsBoard({ projectId }: SessionsBoardProps) { const restartOrchestrator = async () => { if (!projectId) return; - setProjectRestarting(projectId, true); - setOrchestratorReplacementError(projectId, null); - try { - const sessionId = await spawnOrchestrator(projectId, true); - await queryClient.invalidateQueries({ queryKey: workspaceQueryKey }); - void navigate({ - to: "/projects/$projectId/sessions/$sessionId", - params: { projectId, sessionId }, - }); - } catch (error) { - setOrchestratorReplacementError(projectId, error instanceof Error ? error.message : "Could not replace orchestrator"); - } finally { - setProjectRestarting(projectId, false); - } + await restartProjectOrchestrator({ + projectId, + queryClient, + navigate, + setProjectRestarting, + setOrchestratorReplacementError, + }); }; const handleTaskCreated = async (sessionId: string) => { 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 0000000000..e56be81e14 --- /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 0000000000..64e28a8265 --- /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 0000000000..8b85b9bf8e --- /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 0000000000..0d41f79811 --- /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 de0ed574e3..6c0fcba6bd 100644 --- a/frontend/src/renderer/routes/_shell.tsx +++ b/frontend/src/renderer/routes/_shell.tsx @@ -13,6 +13,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"; @@ -141,27 +143,16 @@ function ShellLayout() { const restartOrchestrator = useCallback( async (projectId: string) => { - setProjectRestarting(projectId, true); - setOrchestratorReplacementError(projectId, null); - try { - const sessionId = await spawnOrchestrator(projectId, true); - await queryClient.invalidateQueries({ queryKey: workspaceQueryKey }); - void navigate({ - to: "/projects/$projectId/sessions/$sessionId", - params: { projectId, sessionId }, - }); - } catch (error) { - const message = error instanceof Error ? error.message : "Could not replace orchestrator"; - setOrchestratorReplacementError(projectId, message); - void captureRendererException(error, { - source: "orchestrator-replace", - operation: "replace_orchestrator", - surface: "shell", - project_id: projectId, - }); - } finally { - setProjectRestarting(projectId, false); - } + await restartProjectOrchestrator({ + projectId, + queryClient, + navigate, + setProjectRestarting, + setOrchestratorReplacementError, + onError: (error) => { + captureOrchestratorReplacementFailure(error, projectId); + }, + }); }, [navigate, queryClient, setOrchestratorReplacementError, setProjectRestarting], ); From c65e948b44ccf00438e2b993743552f022e2291b Mon Sep 17 00:00:00 2001 From: nikhil achale Date: Sat, 4 Jul 2026 11:30:59 +0530 Subject: [PATCH 7/9] fix: rename parameter for clarity in projectConfigPtr function --- backend/internal/service/project/service.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/internal/service/project/service.go b/backend/internal/service/project/service.go index 05a5b4a6f3..3f8da5cee7 100644 --- a/backend/internal/service/project/service.go +++ b/backend/internal/service/project/service.go @@ -395,11 +395,11 @@ func (m *Service) projectFromRow(row domain.ProjectRecord) Project { return p } -func projectConfigPtr(config domain.ProjectConfig) *domain.ProjectConfig { - if config.IsZero() { +func projectConfigPtr(projectConfig domain.ProjectConfig) *domain.ProjectConfig { + if projectConfig.IsZero() { return nil } - cfg := config + cfg := projectConfig return &cfg } From cf3a81c2eb77fc475150a898cff143ac0aaa3bcc Mon Sep 17 00:00:00 2001 From: nikhil achale Date: Wed, 8 Jul 2026 20:26:51 +0530 Subject: [PATCH 8/9] fix: launch grok prompts interactively --- backend/internal/adapters/agent/grok/grok.go | 10 ++++----- .../internal/adapters/agent/grok/grok_test.go | 21 +++++++++++++++---- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/backend/internal/adapters/agent/grok/grok.go b/backend/internal/adapters/agent/grok/grok.go index 0f5ca4cfb3..bcb0d823a7 100644 --- a/backend/internal/adapters/agent/grok/grok.go +++ b/backend/internal/adapters/agent/grok/grok.go @@ -5,8 +5,8 @@ // hook installation (which writes .claude/settings.local.json with AO // hook commands). Grok will pick them up via its compat layer. // -// Launch uses `-p ` for the initial task (in-command delivery). -// Permission bypass uses `--always-approve`. We also pass `--no-auto-update` +// Launch uses a positional prompt for the initial task (in-command delivery). +// Permission handling uses `--permission-mode`. We also pass `--no-auto-update` // for headless/scripted use (parity with Codex no-update). // Restore prefers the hook-captured native session id via `-r `. // @@ -67,8 +67,8 @@ func (p *Plugin) Manifest() adapters.Manifest { } } -// GetLaunchCommand builds `grok --no-auto-update [--permission-mode ] -p `. -// Prompt is delivered via -p (in command). +// GetLaunchCommand builds `grok --no-auto-update [--permission-mode ] [prompt]`. +// Prompt is delivered positionally so Grok starts an interactive coding session. // // Uses --permission-mode (acceptEdits / auto / bypassPermissions) to match // `grok -h` output. Default omits the flag so Grok uses its config. @@ -82,7 +82,7 @@ func (p *Plugin) GetLaunchCommand(ctx context.Context, cfg ports.LaunchConfig) ( appendApprovalFlags(&cmd, cfg.Permissions) if cfg.Prompt != "" { - cmd = append(cmd, "-p", cfg.Prompt) + cmd = append(cmd, cfg.Prompt) } return cmd, nil diff --git a/backend/internal/adapters/agent/grok/grok_test.go b/backend/internal/adapters/agent/grok/grok_test.go index 2cac03dd26..adbc485ad4 100644 --- a/backend/internal/adapters/agent/grok/grok_test.go +++ b/backend/internal/adapters/agent/grok/grok_test.go @@ -58,10 +58,11 @@ func TestGetLaunchCommand(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - wantPrefix := []string{"grok", "--no-auto-update", "--permission-mode", "bypassPermissions", "-p", "do the thing"} + wantPrefix := []string{"grok", "--no-auto-update", "--permission-mode", "bypassPermissions", "do the thing"} if !reflect.DeepEqual(cmd, wantPrefix) { t.Fatalf("cmd = %#v, want prefix %#v", cmd, wantPrefix) } + assertNoPromptFlag(t, cmd) } func TestGetLaunchCommandDefaultPerms(t *testing.T) { @@ -72,12 +73,14 @@ func TestGetLaunchCommandDefaultPerms(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - if len(cmd) < 4 || cmd[0] != "grok" || cmd[1] != "--no-auto-update" || cmd[2] != "-p" { - t.Fatalf("cmd = %#v, want grok --no-auto-update -p ...", cmd) + want := []string{"grok", "--no-auto-update", "fix it"} + if !reflect.DeepEqual(cmd, want) { + t.Fatalf("cmd = %#v, want %#v", cmd, want) } if strings.Contains(strings.Join(cmd, " "), "permission-mode") { t.Fatal("should not have --permission-mode for default perms") } + assertNoPromptFlag(t, cmd) } func TestGetLaunchCommandAcceptEdits(t *testing.T) { @@ -89,10 +92,20 @@ func TestGetLaunchCommandAcceptEdits(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - want := []string{"grok", "--no-auto-update", "--permission-mode", "acceptEdits", "-p", "refactor auth"} + want := []string{"grok", "--no-auto-update", "--permission-mode", "acceptEdits", "refactor auth"} if !reflect.DeepEqual(cmd, want) { t.Fatalf("cmd = %#v, want %#v", cmd, want) } + assertNoPromptFlag(t, cmd) +} + +func assertNoPromptFlag(t *testing.T, cmd []string) { + t.Helper() + for _, arg := range cmd { + if arg == "-p" || arg == "--single" { + t.Fatalf("cmd = %#v unexpectedly contains single-turn prompt flag %q", cmd, arg) + } + } } func TestGetRestoreCommand(t *testing.T) { From 4ba58dad04d95cc5630a083ee04d547b45601cb6 Mon Sep 17 00:00:00 2001 From: nikhil achale Date: Wed, 8 Jul 2026 23:10:28 +0530 Subject: [PATCH 9/9] fix(grok): update launch command to properly handle prompts with leading dashes --- backend/internal/adapters/agent/grok/grok.go | 4 ++-- .../internal/adapters/agent/grok/grok_test.go | 21 ++++++++++++++++--- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/backend/internal/adapters/agent/grok/grok.go b/backend/internal/adapters/agent/grok/grok.go index bcb0d823a7..0c5a68d4f5 100644 --- a/backend/internal/adapters/agent/grok/grok.go +++ b/backend/internal/adapters/agent/grok/grok.go @@ -67,7 +67,7 @@ func (p *Plugin) Manifest() adapters.Manifest { } } -// GetLaunchCommand builds `grok --no-auto-update [--permission-mode ] [prompt]`. +// GetLaunchCommand builds `grok --no-auto-update [--permission-mode ] [-- prompt]`. // Prompt is delivered positionally so Grok starts an interactive coding session. // // Uses --permission-mode (acceptEdits / auto / bypassPermissions) to match @@ -82,7 +82,7 @@ func (p *Plugin) GetLaunchCommand(ctx context.Context, cfg ports.LaunchConfig) ( appendApprovalFlags(&cmd, cfg.Permissions) if cfg.Prompt != "" { - cmd = append(cmd, cfg.Prompt) + cmd = append(cmd, "--", cfg.Prompt) } return cmd, nil diff --git a/backend/internal/adapters/agent/grok/grok_test.go b/backend/internal/adapters/agent/grok/grok_test.go index adbc485ad4..9f542d9c68 100644 --- a/backend/internal/adapters/agent/grok/grok_test.go +++ b/backend/internal/adapters/agent/grok/grok_test.go @@ -58,7 +58,7 @@ func TestGetLaunchCommand(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - wantPrefix := []string{"grok", "--no-auto-update", "--permission-mode", "bypassPermissions", "do the thing"} + wantPrefix := []string{"grok", "--no-auto-update", "--permission-mode", "bypassPermissions", "--", "do the thing"} if !reflect.DeepEqual(cmd, wantPrefix) { t.Fatalf("cmd = %#v, want prefix %#v", cmd, wantPrefix) } @@ -73,7 +73,7 @@ func TestGetLaunchCommandDefaultPerms(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - want := []string{"grok", "--no-auto-update", "fix it"} + want := []string{"grok", "--no-auto-update", "--", "fix it"} if !reflect.DeepEqual(cmd, want) { t.Fatalf("cmd = %#v, want %#v", cmd, want) } @@ -92,7 +92,22 @@ func TestGetLaunchCommandAcceptEdits(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - want := []string{"grok", "--no-auto-update", "--permission-mode", "acceptEdits", "refactor auth"} + want := []string{"grok", "--no-auto-update", "--permission-mode", "acceptEdits", "--", "refactor auth"} + if !reflect.DeepEqual(cmd, want) { + t.Fatalf("cmd = %#v, want %#v", cmd, want) + } + assertNoPromptFlag(t, cmd) +} + +func TestGetLaunchCommandTerminatesFlagsBeforeLeadingDashPrompt(t *testing.T) { + plugin := &Plugin{resolvedBinary: "grok"} + cmd, err := plugin.GetLaunchCommand(context.Background(), ports.LaunchConfig{ + Prompt: "-add a health check", + }) + if err != nil { + t.Fatalf("err: %v", err) + } + want := []string{"grok", "--no-auto-update", "--", "-add a health check"} if !reflect.DeepEqual(cmd, want) { t.Fatalf("cmd = %#v, want %#v", cmd, want) }