fix(web): source sidebar orchestrator from API field, not session list (#1623)
* fix(web): source sidebar orchestrator from API field, not session list PR #1615 merged the initial implementation but missed the follow-up fix. The merged sidebar code looks up the orchestrator inside the `sessions` prop, but /api/sessions/route.ts strips ALL orchestrators from that array before returning (they're exposed via a separate `orchestrators` field on the same response). Result: the new menu entry — and the existing icon button next to the dashboard icon — never render for any project. Replace the broken in-sidebar derivation with a new `orchestrators` prop on ProjectSidebar. Each parent passes the data it already has: - Dashboard.tsx: passes `activeOrchestrators` (already in scope) - PullRequestsPage.tsx: passes `orchestratorLinks` (already in scope) - sessions/[id]/page.tsx: stores the `orchestrators` field already returned by /api/sessions and threads it through SessionPageShell and SessionDetail as `sidebarOrchestrators` Tests refocused: the sidebar now just renders what the prop says, so the live-vs-terminal selection lives in the API (selectPreferredOrchestratorId) and is no longer the sidebar's responsibility. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * style: fix indentation on sidebarOrchestrators prop in SessionPage Addresses Greptile review comment on PR #1623. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(web): use DashboardOrchestratorLink for API response type Addresses non-blocking review feedback on PR #1623. The /api/sessions response actually returns DashboardOrchestratorLink shape (which includes projectName), so use that as the response type instead of the narrower ProjectSidebarOrchestrator. The sidebar prop type stays narrow on purpose — it's the minimum the sidebar needs to render. Also document the "one orchestrator per project" Map invariant. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Harshit Singh Bhandari
GitHub
parent
b98d8eca8e
commit
7c7ffb5624
|
|
@ -5,9 +5,17 @@ import { useParams, usePathname, useRouter } from "next/navigation";
|
|||
import { ACTIVITY_STATE, SESSION_STATUS, isOrchestratorSession } from "@aoagents/ao-core/types";
|
||||
import { SessionDetail } from "@/components/SessionDetail";
|
||||
import { ErrorDisplay } from "@/components/ErrorDisplay";
|
||||
import { ProjectSidebar } from "@/components/ProjectSidebar";
|
||||
import {
|
||||
ProjectSidebar,
|
||||
type ProjectSidebarOrchestrator,
|
||||
} from "@/components/ProjectSidebar";
|
||||
import { useMediaQuery, MOBILE_BREAKPOINT } from "@/hooks/useMediaQuery";
|
||||
import { type DashboardSession, type ActivityState, getAttentionLevel } from "@/lib/types";
|
||||
import {
|
||||
type DashboardSession,
|
||||
type DashboardOrchestratorLink,
|
||||
type ActivityState,
|
||||
getAttentionLevel,
|
||||
} from "@/lib/types";
|
||||
import { activityIcon } from "@/lib/activity-icons";
|
||||
import type { ProjectInfo } from "@/lib/project-name";
|
||||
import { getSessionTitle } from "@/lib/format";
|
||||
|
|
@ -170,6 +178,7 @@ function SessionPageShell({
|
|||
projects,
|
||||
projectsLoading,
|
||||
sidebarSessions,
|
||||
sidebarOrchestrators,
|
||||
sidebarLoading,
|
||||
sidebarError,
|
||||
onRetrySidebar,
|
||||
|
|
@ -180,6 +189,7 @@ function SessionPageShell({
|
|||
projects: ProjectInfo[];
|
||||
projectsLoading: boolean;
|
||||
sidebarSessions: DashboardSession[] | null;
|
||||
sidebarOrchestrators?: ProjectSidebarOrchestrator[];
|
||||
sidebarLoading: boolean;
|
||||
sidebarError: boolean;
|
||||
onRetrySidebar: () => void;
|
||||
|
|
@ -251,6 +261,7 @@ function SessionPageShell({
|
|||
<ProjectSidebar
|
||||
projects={projects}
|
||||
sessions={sidebarSessions}
|
||||
orchestrators={sidebarOrchestrators}
|
||||
loading={sidebarLoading}
|
||||
error={sidebarError}
|
||||
onRetry={onRetrySidebar}
|
||||
|
|
@ -380,6 +391,9 @@ export default function SessionPage() {
|
|||
const [sidebarSessions, setSidebarSessions] = useState<DashboardSession[] | null>(
|
||||
() => cachedSidebarSessions,
|
||||
);
|
||||
const [sidebarOrchestrators, setSidebarOrchestrators] = useState<
|
||||
ProjectSidebarOrchestrator[] | undefined
|
||||
>(undefined);
|
||||
const [loading, setLoading] = useState(cachedSession === null);
|
||||
const [routeError, setRouteError] = useState<Error | null>(null);
|
||||
const [sessionMissing, setSessionMissing] = useState(false);
|
||||
|
|
@ -596,18 +610,19 @@ export default function SessionPage() {
|
|||
const controller = new AbortController();
|
||||
sidebarFetchControllerRef.current = controller;
|
||||
try {
|
||||
const body = await fetchJsonWithTimeout<{ sessions?: DashboardSession[] } | null>(
|
||||
"/api/sessions?fresh=true",
|
||||
{
|
||||
signal: controller.signal,
|
||||
timeoutMs: PROJECT_SIDEBAR_FETCH_TIMEOUT_MS,
|
||||
timeoutMessage: `Sidebar sessions request timed out after ${PROJECT_SIDEBAR_FETCH_TIMEOUT_MS}ms`,
|
||||
},
|
||||
);
|
||||
const body = await fetchJsonWithTimeout<{
|
||||
sessions?: DashboardSession[];
|
||||
orchestrators?: DashboardOrchestratorLink[];
|
||||
} | null>("/api/sessions?fresh=true", {
|
||||
signal: controller.signal,
|
||||
timeoutMs: PROJECT_SIDEBAR_FETCH_TIMEOUT_MS,
|
||||
timeoutMessage: `Sidebar sessions request timed out after ${PROJECT_SIDEBAR_FETCH_TIMEOUT_MS}ms`,
|
||||
});
|
||||
const restSessions = body?.sessions ?? [];
|
||||
const nextSessions =
|
||||
applyMuxSessionPatches(restSessions, pendingMuxSessionsRef.current ?? []) ?? restSessions;
|
||||
cachedSidebarSessions = nextSessions;
|
||||
setSidebarOrchestrators(body?.orchestrators);
|
||||
setSidebarError(false);
|
||||
setSidebarSessions((current) =>
|
||||
areSidebarSessionsEqual(current, nextSessions) ? current : nextSessions,
|
||||
|
|
@ -724,6 +739,7 @@ export default function SessionPage() {
|
|||
projects={projects}
|
||||
projectsLoading={projectsLoading}
|
||||
sidebarSessions={sidebarSessions}
|
||||
sidebarOrchestrators={sidebarOrchestrators}
|
||||
sidebarLoading={sidebarSessions === null}
|
||||
sidebarError={sidebarError}
|
||||
onRetrySidebar={fetchSidebarSessions}
|
||||
|
|
@ -754,6 +770,7 @@ export default function SessionPage() {
|
|||
projects={projects}
|
||||
projectsLoading={projectsLoading}
|
||||
sidebarSessions={sidebarSessions}
|
||||
sidebarOrchestrators={sidebarOrchestrators}
|
||||
sidebarLoading={sidebarSessions === null}
|
||||
sidebarError={sidebarError}
|
||||
onRetrySidebar={fetchSidebarSessions}
|
||||
|
|
@ -782,6 +799,7 @@ export default function SessionPage() {
|
|||
projects={projects}
|
||||
projectsLoading={projectsLoading}
|
||||
sidebarSessions={sidebarSessions}
|
||||
sidebarOrchestrators={sidebarOrchestrators}
|
||||
sidebarLoading={sidebarSessions === null}
|
||||
sidebarError={sidebarError}
|
||||
onRetrySidebar={fetchSidebarSessions}
|
||||
|
|
@ -819,6 +837,7 @@ export default function SessionPage() {
|
|||
projects={projects}
|
||||
projectsLoading={projectsLoading}
|
||||
sidebarSessions={sidebarSessions}
|
||||
sidebarOrchestrators={sidebarOrchestrators}
|
||||
sidebarLoading={sidebarSessions === null}
|
||||
sidebarError={sidebarError}
|
||||
onRetrySidebar={fetchSidebarSessions}
|
||||
|
|
@ -849,6 +868,7 @@ export default function SessionPage() {
|
|||
projectOrchestratorId={projectOrchestratorId}
|
||||
projects={projects}
|
||||
sidebarSessions={sidebarSessions}
|
||||
sidebarOrchestrators={sidebarOrchestrators}
|
||||
sidebarLoading={sidebarSessions === null}
|
||||
sidebarError={sidebarError}
|
||||
onRetrySidebar={fetchSidebarSessions}
|
||||
|
|
|
|||
|
|
@ -605,6 +605,7 @@ function DashboardInner({
|
|||
<ProjectSidebar
|
||||
projects={projects}
|
||||
sessions={sessions}
|
||||
orchestrators={activeOrchestrators}
|
||||
activeProjectId={projectId}
|
||||
activeSessionId={activeSessionId}
|
||||
collapsed={sidebarCollapsed}
|
||||
|
|
|
|||
|
|
@ -6,18 +6,30 @@ import { useRouter } from "next/navigation";
|
|||
import { cn } from "@/lib/cn";
|
||||
import type { ProjectInfo } from "@/lib/project-name";
|
||||
import { getAttentionLevel, type DashboardSession, type AttentionLevel } from "@/lib/types";
|
||||
import { isOrchestratorSession, isTerminalSession } from "@aoagents/ao-core/types";
|
||||
import { isOrchestratorSession } from "@aoagents/ao-core/types";
|
||||
import { getSessionTitle, humanizeBranch } from "@/lib/format";
|
||||
import { usePopoverClamp } from "@/hooks/usePopoverClamp";
|
||||
import { getOrchestratorSessionId } from "@/lib/orchestrator-utils";
|
||||
import { projectDashboardPath, projectSessionPath } from "@/lib/routes";
|
||||
import { ThemeToggle } from "./ThemeToggle";
|
||||
import { AddProjectModal } from "./AddProjectModal";
|
||||
import { ProjectSettingsModal } from "./ProjectSettingsModal";
|
||||
|
||||
/** Minimal shape needed to render an orchestrator link in the sidebar. */
|
||||
export interface ProjectSidebarOrchestrator {
|
||||
id: string;
|
||||
projectId: string;
|
||||
}
|
||||
|
||||
interface ProjectSidebarProps {
|
||||
projects: ProjectInfo[];
|
||||
sessions: DashboardSession[] | null;
|
||||
/**
|
||||
* Per-project orchestrator link. Sourced upstream from `/api/sessions`
|
||||
* (the `orchestrators` field), which already applies the canonical
|
||||
* "prefer live, fall back to terminal" selection. Not derivable from
|
||||
* `sessions`: the sessions endpoint strips orchestrators out.
|
||||
*/
|
||||
orchestrators?: ProjectSidebarOrchestrator[];
|
||||
activeProjectId: string | undefined;
|
||||
activeSessionId: string | undefined;
|
||||
loading?: boolean;
|
||||
|
|
@ -78,6 +90,7 @@ export function ProjectSidebar(props: ProjectSidebarProps) {
|
|||
function ProjectSidebarInner({
|
||||
projects,
|
||||
sessions,
|
||||
orchestrators,
|
||||
activeProjectId,
|
||||
activeSessionId,
|
||||
loading = false,
|
||||
|
|
@ -186,6 +199,14 @@ function ProjectSidebarInner({
|
|||
[visibleProjects],
|
||||
);
|
||||
|
||||
// The API (selectPreferredOrchestratorId) sends at most one entry per
|
||||
// project, so collapsing into a Map keyed by projectId is lossless. If a
|
||||
// future API change starts emitting multiples, the last one wins here.
|
||||
const orchestratorByProject = useMemo(
|
||||
() => new Map((orchestrators ?? []).map((o) => [o.projectId, o])),
|
||||
[orchestrators],
|
||||
);
|
||||
|
||||
const sessionsByProject = useMemo(() => {
|
||||
const map = new Map<string, DashboardSession[]>();
|
||||
// Build a set of valid project IDs to filter sessions strictly
|
||||
|
|
@ -390,21 +411,7 @@ function ProjectSidebarInner({
|
|||
const visibleSessions = workerSessions;
|
||||
const hasActiveSessions = visibleSessions.length > 0;
|
||||
|
||||
const projectPrefix = prefixByProject.get(project.id);
|
||||
const canonicalOrchestratorId = projectPrefix
|
||||
? getOrchestratorSessionId({ sessionPrefix: projectPrefix })
|
||||
: null;
|
||||
const orchestratorSession = sessions?.find(
|
||||
(s) => s.projectId === project.id && s.id === canonicalOrchestratorId,
|
||||
);
|
||||
const liveOrchestrator =
|
||||
orchestratorSession &&
|
||||
!isTerminalSession({
|
||||
status: orchestratorSession.status,
|
||||
activity: orchestratorSession.activity,
|
||||
})
|
||||
? orchestratorSession
|
||||
: null;
|
||||
const orchestratorLink = orchestratorByProject.get(project.id) ?? null;
|
||||
|
||||
return (
|
||||
<div key={project.id} className="project-sidebar__project">
|
||||
|
|
@ -506,9 +513,9 @@ function ProjectSidebarInner({
|
|||
) : null}
|
||||
|
||||
{/* Orchestrator button */}
|
||||
{!isDegraded && orchestratorSession && (
|
||||
{!isDegraded && orchestratorLink && (
|
||||
<Link
|
||||
href={projectSessionPath(project.id, orchestratorSession.id)}
|
||||
href={projectSessionPath(project.id, orchestratorLink.id)}
|
||||
onClick={(e) => {
|
||||
e.stopPropagation();
|
||||
onMobileClose?.();
|
||||
|
|
@ -571,17 +578,14 @@ function ProjectSidebarInner({
|
|||
role="menu"
|
||||
aria-label={`${project.name} actions`}
|
||||
>
|
||||
{liveOrchestrator ? (
|
||||
{orchestratorLink ? (
|
||||
<button
|
||||
type="button"
|
||||
className="project-sidebar__proj-menu-item"
|
||||
role="menuitem"
|
||||
onClick={() => {
|
||||
setProjectMenuOpenId(null);
|
||||
navigate(
|
||||
projectSessionPath(project.id, liveOrchestrator.id),
|
||||
liveOrchestrator,
|
||||
);
|
||||
navigate(projectSessionPath(project.id, orchestratorLink.id));
|
||||
}}
|
||||
>
|
||||
Open orchestrator
|
||||
|
|
|
|||
|
|
@ -115,6 +115,7 @@ export function PullRequestsPage({
|
|||
<ProjectSidebar
|
||||
projects={projects}
|
||||
sessions={sessions}
|
||||
orchestrators={orchestratorLinks}
|
||||
activeProjectId={projectId}
|
||||
activeSessionId={undefined}
|
||||
collapsed={sidebarCollapsed}
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ import type { ProjectInfo } from "@/lib/project-name";
|
|||
import { SidebarContext } from "./workspace/SidebarContext";
|
||||
import { projectDashboardPath, projectSessionPath } from "@/lib/routes";
|
||||
|
||||
import { ProjectSidebar } from "./ProjectSidebar";
|
||||
import { ProjectSidebar, type ProjectSidebarOrchestrator } from "./ProjectSidebar";
|
||||
import { MobileBottomNav } from "./MobileBottomNav";
|
||||
import {
|
||||
SessionDetailHeader,
|
||||
|
|
@ -44,6 +44,7 @@ interface SessionDetailProps {
|
|||
projectOrchestratorId?: string | null;
|
||||
projects?: ProjectInfo[];
|
||||
sidebarSessions?: DashboardSession[] | null;
|
||||
sidebarOrchestrators?: ProjectSidebarOrchestrator[];
|
||||
sidebarLoading?: boolean;
|
||||
sidebarError?: boolean;
|
||||
onRetrySidebar?: () => void;
|
||||
|
|
@ -56,6 +57,7 @@ export function SessionDetail({
|
|||
projectOrchestratorId = null,
|
||||
projects = [],
|
||||
sidebarSessions = [],
|
||||
sidebarOrchestrators,
|
||||
sidebarLoading = false,
|
||||
sidebarError = false,
|
||||
onRetrySidebar,
|
||||
|
|
@ -175,6 +177,7 @@ export function SessionDetail({
|
|||
<ProjectSidebar
|
||||
projects={projects}
|
||||
sessions={sidebarSessions}
|
||||
orchestrators={sidebarOrchestrators}
|
||||
loading={sidebarLoading}
|
||||
error={sidebarError}
|
||||
onRetry={onRetrySidebar}
|
||||
|
|
|
|||
|
|
@ -404,19 +404,12 @@ describe("ProjectSidebar", () => {
|
|||
expect(screen.queryByText("Orchestrator")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows 'Open orchestrator' in the project actions menu when a live orchestrator exists", async () => {
|
||||
it("shows 'Open orchestrator' in the project actions menu when the orchestrators prop has an entry", async () => {
|
||||
render(
|
||||
<ProjectSidebar
|
||||
projects={projects}
|
||||
sessions={[
|
||||
makeSession({
|
||||
id: "project-2-orchestrator",
|
||||
projectId: "project-2",
|
||||
summary: "Orchestrator",
|
||||
metadata: { role: "orchestrator" },
|
||||
status: "working",
|
||||
}),
|
||||
]}
|
||||
sessions={[]}
|
||||
orchestrators={[{ id: "project-2-orchestrator-1", projectId: "project-2" }]}
|
||||
activeProjectId="project-1"
|
||||
activeSessionId={undefined}
|
||||
/>,
|
||||
|
|
@ -429,11 +422,12 @@ describe("ProjectSidebar", () => {
|
|||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("omits 'Open orchestrator' from the menu when no orchestrator session exists", async () => {
|
||||
it("omits 'Open orchestrator' from the menu when no orchestrator entry exists for the project", async () => {
|
||||
render(
|
||||
<ProjectSidebar
|
||||
projects={projects}
|
||||
sessions={[]}
|
||||
orchestrators={[{ id: "project-1-orchestrator", projectId: "project-1" }]}
|
||||
activeProjectId="project-1"
|
||||
activeSessionId={undefined}
|
||||
/>,
|
||||
|
|
@ -445,44 +439,12 @@ describe("ProjectSidebar", () => {
|
|||
expect(screen.queryByRole("menuitem", { name: "Open orchestrator" })).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("omits 'Open orchestrator' when the orchestrator session is terminal", async () => {
|
||||
it("navigates to the orchestrator id from the prop when 'Open orchestrator' is clicked", async () => {
|
||||
render(
|
||||
<ProjectSidebar
|
||||
projects={projects}
|
||||
sessions={[
|
||||
makeSession({
|
||||
id: "project-2-orchestrator",
|
||||
projectId: "project-2",
|
||||
summary: "Orchestrator",
|
||||
metadata: { role: "orchestrator" },
|
||||
status: "killed",
|
||||
activity: "exited",
|
||||
}),
|
||||
]}
|
||||
activeProjectId="project-1"
|
||||
activeSessionId={undefined}
|
||||
/>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: /Project actions for Project Two/i }));
|
||||
|
||||
expect(await screen.findByRole("menuitem", { name: "Project settings" })).toBeInTheDocument();
|
||||
expect(screen.queryByRole("menuitem", { name: "Open orchestrator" })).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("navigates to the orchestrator session when 'Open orchestrator' is clicked", async () => {
|
||||
render(
|
||||
<ProjectSidebar
|
||||
projects={projects}
|
||||
sessions={[
|
||||
makeSession({
|
||||
id: "project-2-orchestrator",
|
||||
projectId: "project-2",
|
||||
summary: "Orchestrator",
|
||||
metadata: { role: "orchestrator" },
|
||||
status: "working",
|
||||
}),
|
||||
]}
|
||||
sessions={[]}
|
||||
orchestrators={[{ id: "project-2-orchestrator-1", projectId: "project-2" }]}
|
||||
activeProjectId="project-1"
|
||||
activeSessionId={undefined}
|
||||
/>,
|
||||
|
|
@ -491,7 +453,7 @@ describe("ProjectSidebar", () => {
|
|||
fireEvent.click(screen.getByRole("button", { name: /Project actions for Project Two/i }));
|
||||
fireEvent.click(await screen.findByRole("menuitem", { name: "Open orchestrator" }));
|
||||
|
||||
expect(mockPush).toHaveBeenCalledWith("/projects/project-2/sessions/project-2-orchestrator");
|
||||
expect(mockPush).toHaveBeenCalledWith("/projects/project-2/sessions/project-2-orchestrator-1");
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByRole("menuitem", { name: "Open orchestrator" })).not.toBeInTheDocument();
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue