Skip to content

Commit 035fa44

Browse files
pallyoungpallyoung
committed
Refine supervisor edit save behavior
1 parent d565386 commit 035fa44

7 files changed

Lines changed: 146 additions & 87 deletions

File tree

‎packages/web/src/features/agent-panes/components/session-card.test.tsx‎

Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -384,44 +384,29 @@ describe("SessionCard", () => {
384384
);
385385
});
386386

387-
it("hydrates supervisor state for full-capability sessions and renders the card above the terminal", async () => {
388-
const sendCommand = vi.fn().mockImplementation(async (op: string) => {
389-
if (op === "supervisor.get") {
390-
return {
391-
supervisor: {
392-
id: "sup-1",
393-
sessionId: "sess_123456",
394-
workspaceId: "ws-123",
395-
state: "idle",
396-
objective: "Keep the agent on track",
397-
evaluatorProviderId: "claude",
398-
maxSupervisionCount: 0,
399-
completedSupervisionCount: 0,
400-
createdAt: Date.now(),
401-
updatedAt: Date.now(),
402-
},
403-
};
404-
}
405-
return undefined;
387+
it("does not hydrate supervisor state via supervisor.get on mount", async () => {
388+
const { store, sendCommand } = createSessionStore({
389+
state: "running",
390+
capability: "full",
391+
endedAt: undefined,
392+
terminalId: "term-live",
406393
});
407394

408-
const { store } = createSessionStore({ state: "running", capability: "full" }, sendCommand);
409-
410395
render(
411396
<Provider store={store}>
412397
<SessionCard sessionId="sess_123456" />
413398
</Provider>
414399
);
415400

416-
await waitFor(() => {
417-
expect(sendCommand).toHaveBeenCalledWith(
418-
"supervisor.get",
419-
{ sessionId: "sess_123456" },
420-
undefined
421-
);
401+
await act(async () => {
402+
await Promise.resolve();
422403
});
423404

424-
expect(screen.getByText("Supervisor")).toBeInTheDocument();
405+
expect(sendCommand).not.toHaveBeenCalledWith(
406+
"supervisor.get",
407+
{ sessionId: "sess_123456" },
408+
undefined
409+
);
425410
});
426411

427412
it("reacts to a pending-focus request by scrolling itself into view and pulsing, then clears the marker", async () => {

‎packages/web/src/features/supervisor/actions/use-objective-dialog-state.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,21 @@ export function useObjectiveDialogState({
118118
const isRecoverableTargetsLoading = dialog.isRecoverableTargetsLoading ?? false;
119119
const trimmedDraftObjective = dialog.draftObjective.trim();
120120
const hasObjectiveChanged = trimmedDraftObjective !== (dialog.initialObjective?.trim() ?? "");
121+
const normalizedDraftEvaluatorModel = (dialog.draftEvaluatorModel ?? "").trim();
122+
const normalizedSupervisorEvaluatorModel = supervisor?.evaluatorModel?.trim() ?? "";
123+
const draftMaxSupervisionCount = parseDraftMaxSupervisionCount(
124+
dialog.draftMaxSupervisionCount ?? "0"
125+
);
126+
const draftScheduledAt = parseDraftScheduledAt(dialog.draftScheduledAt ?? "") ?? null;
127+
const supervisorScheduledAt = supervisor?.scheduledAt ?? null;
128+
const hasSettingsChanged = Boolean(
129+
supervisor &&
130+
(dialog.draftEvaluatorProviderId !== supervisor.evaluatorProviderId ||
131+
normalizedDraftEvaluatorModel !== normalizedSupervisorEvaluatorModel ||
132+
draftMaxSupervisionCount !== supervisor.maxSupervisionCount ||
133+
draftScheduledAt !== supervisorScheduledAt)
134+
);
135+
const hasChanges = hasObjectiveChanged || hasSettingsChanged;
121136

122137
const close = useCallback(() => {
123138
const nextSessionId = dialog.returnToDetails ? dialog.sessionId : null;
@@ -295,6 +310,8 @@ export function useObjectiveDialogState({
295310
selectedRecoverableTargetId,
296311
isRecoverableTargetsLoading,
297312
hasObjectiveChanged,
313+
hasSettingsChanged,
314+
hasChanges,
298315
close,
299316
updateDraft,
300317
openRestoreStep,

‎packages/web/src/features/supervisor/actions/use-supervisor.ts‎

Lines changed: 3 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,67 +1,15 @@
11
import type { Session, Supervisor } from "@coder-studio/core";
2-
import { useAtomValue, useSetAtom } from "jotai";
3-
import { useCallback, useEffect } from "react";
4-
import { dispatchCommandAtom } from "../../../atoms/connection";
5-
import { supervisorDialogAtom, supervisorHydratedAtomFamily, supervisorsAtom } from "../atoms";
2+
import { useSetAtom } from "jotai";
3+
import { useCallback } from "react";
4+
import { supervisorDialogAtom } from "../atoms";
65
import { formatScheduledAtInput } from "./use-objective-dialog-state";
76

87
const EMPTY_SESSION_ID = "__supervisor-empty__";
98

109
export function useSupervisor(session: Session | null | undefined) {
1110
const sessionId = session?.id ?? EMPTY_SESSION_ID;
12-
const dispatch = useAtomValue(dispatchCommandAtom);
13-
const hydrated = useAtomValue(supervisorHydratedAtomFamily(sessionId));
14-
const setHydrated = useSetAtom(supervisorHydratedAtomFamily(sessionId));
15-
const setSupervisors = useSetAtom(supervisorsAtom);
1611
const setDialog = useSetAtom(supervisorDialogAtom);
1712

18-
useEffect(() => {
19-
if (!session) {
20-
return;
21-
}
22-
23-
if (
24-
hydrated ||
25-
session.capability !== "full" ||
26-
session.state === "draft" ||
27-
session.state === "ended"
28-
) {
29-
return;
30-
}
31-
32-
let cancelled = false;
33-
34-
void dispatch<{ supervisor: Supervisor | null }>("supervisor.get", {
35-
sessionId: session.id,
36-
}).then((result) => {
37-
if (cancelled || !result.ok) {
38-
return;
39-
}
40-
41-
const supervisor = result.data?.supervisor ?? null;
42-
43-
if (supervisor) {
44-
setSupervisors((prev) => {
45-
const next = new Map(prev);
46-
next.set(session.id, supervisor);
47-
return next;
48-
});
49-
} else {
50-
setSupervisors((prev) => {
51-
const next = new Map(prev);
52-
next.delete(session.id);
53-
return next;
54-
});
55-
}
56-
57-
setHydrated(true);
58-
});
59-
60-
return () => {
61-
cancelled = true;
62-
};
63-
}, [dispatch, hydrated, session, setHydrated, setSupervisors]);
64-
6513
const openDialog = useCallback(
6614
(mode: "enable" | "edit", supervisor?: Supervisor) => {
6715
setDialog({

‎packages/web/src/features/supervisor/components/objective-dialog.test.tsx‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,4 +653,61 @@ describe("ObjectiveDialog", () => {
653653
);
654654
});
655655
});
656+
657+
it("allows saving non-objective supervisor edits without reset confirmation", async () => {
658+
const user = userEvent.setup();
659+
const sendCommand = vi.fn().mockResolvedValue({
660+
supervisor: {
661+
...createSupervisor(),
662+
evaluatorProviderId: "codex",
663+
},
664+
});
665+
const store = createStore();
666+
window.localStorage.setItem("ui.locale", JSON.stringify("en"));
667+
store.set(localeAtom, "en");
668+
store.set(wsClientAtom, { sendCommand } as never);
669+
store.set(
670+
supervisorDialogAtom,
671+
createDialogState({
672+
mode: "edit",
673+
draftObjective: "Finish the server refactor",
674+
initialObjective: "Finish the server refactor",
675+
draftEvaluatorProviderId: "claude",
676+
})
677+
);
678+
store.set(supervisorsAtom, new Map([["sess-1", createSupervisor()]]));
679+
680+
render(
681+
<Provider store={store}>
682+
<ObjectiveDialog workspaceId="ws-1" />
683+
</Provider>
684+
);
685+
686+
await user.click(screen.getByRole("button", { name: "Evaluator Claude" }));
687+
await user.click(screen.getByRole("option", { name: "Codex" }));
688+
689+
const saveButton = screen.getByRole("button", { name: "Save" });
690+
expect(saveButton).toBeEnabled();
691+
692+
await user.click(saveButton);
693+
694+
expect(
695+
screen.queryByRole("heading", { name: "Save and reset supervisor progress?" })
696+
).not.toBeInTheDocument();
697+
698+
await waitFor(() => {
699+
expect(sendCommand).toHaveBeenCalledWith(
700+
"supervisor.update",
701+
{
702+
id: "sup-1",
703+
objective: "Finish the server refactor",
704+
evaluatorProviderId: "codex",
705+
evaluatorModel: null,
706+
maxSupervisionCount: 0,
707+
scheduledAt: null,
708+
},
709+
undefined
710+
);
711+
});
712+
});
656713
});

‎packages/web/src/features/supervisor/views/mobile/mobile-supervisor-sheet.test.tsx‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,4 +357,54 @@ describe("MobileSupervisorSheet", () => {
357357
expect(document.querySelector(".mobile-supervisor-sheet__actions")).toBeNull();
358358
expect(screen.getByRole("heading", { name: "Edit Supervisor", level: 2 })).toBeInTheDocument();
359359
});
360+
361+
it("saves non-objective edits from mobile without showing reset confirmation", async () => {
362+
const user = userEvent.setup();
363+
const sendCommand = vi.fn().mockResolvedValue({
364+
supervisor: {
365+
...createSupervisor(),
366+
evaluatorProviderId: "codex",
367+
},
368+
});
369+
const store = createStore();
370+
371+
window.localStorage.setItem("ui.locale", JSON.stringify("en"));
372+
store.set(localeAtom, "en");
373+
store.set(wsClientAtom, { sendCommand } as never);
374+
store.set(supervisorsAtom, new Map([["sess-1", createSupervisor()]]));
375+
376+
render(
377+
<Provider store={store}>
378+
<MobileSupervisorSheet sessionId="sess-1" workspaceId="ws-1" onClose={vi.fn()} />
379+
</Provider>
380+
);
381+
382+
await user.click(screen.getByRole("button", { name: "Edit Supervisor" }));
383+
await user.click(screen.getByRole("button", { name: "Evaluator Claude" }));
384+
await user.click(screen.getByRole("button", { name: "Codex" }));
385+
386+
const saveButton = screen.getByRole("button", { name: "Save" });
387+
expect(saveButton).toBeEnabled();
388+
389+
await user.click(saveButton);
390+
391+
expect(
392+
screen.queryByRole("heading", { name: "Save and reset supervisor progress?" })
393+
).not.toBeInTheDocument();
394+
395+
await waitFor(() => {
396+
expect(sendCommand).toHaveBeenCalledWith(
397+
"supervisor.update",
398+
{
399+
id: "sup-1",
400+
objective: "Reduce mobile regression bugs",
401+
evaluatorProviderId: "codex",
402+
evaluatorModel: null,
403+
maxSupervisionCount: 0,
404+
scheduledAt: null,
405+
},
406+
undefined
407+
);
408+
});
409+
});
360410
});

‎packages/web/src/features/supervisor/views/mobile/mobile-supervisor-sheet.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export function MobileSupervisorSheet({
3838
selectedRecoverableTargetId,
3939
isRecoverableTargetsLoading,
4040
hasObjectiveChanged,
41+
hasChanges,
4142
close,
4243
updateDraft,
4344
openRestoreStep,
@@ -192,7 +193,7 @@ export function MobileSupervisorSheet({
192193
<Button
193194
variant="primary"
194195
onClick={() => {
195-
if (mode === "edit" && restoreStep !== "restore") {
196+
if (mode === "edit" && restoreStep !== "restore" && hasObjectiveChanged) {
196197
setIsSaveConfirmOpen(true);
197198
return;
198199
}
@@ -214,7 +215,7 @@ export function MobileSupervisorSheet({
214215
isRecoverableTargetsLoading ||
215216
!isMaxSupervisionCountValid
216217
: mode === "edit"
217-
? !hasObjectiveChanged || !dialog.draftObjective.trim() || !isMaxSupervisionCountValid
218+
? !hasChanges || !dialog.draftObjective.trim() || !isMaxSupervisionCountValid
218219
: !dialog.draftObjective.trim() || !isMaxSupervisionCountValid
219220
}
220221
>

‎packages/web/src/features/supervisor/views/shared/objective-dialog.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export function ObjectiveDialog({ workspaceId, sessionId }: ObjectiveDialogProps
3434
selectedRecoverableTargetId,
3535
isRecoverableTargetsLoading,
3636
hasObjectiveChanged,
37+
hasChanges,
3738
close,
3839
updateDraft,
3940
openRestoreStep,
@@ -50,7 +51,7 @@ export function ObjectiveDialog({ workspaceId, sessionId }: ObjectiveDialogProps
5051
const isRestoreMode = restoreStep === "restore";
5152
const isSaveDisabled =
5253
mode === "edit"
53-
? !hasObjectiveChanged || !dialog.draftObjective.trim() || !isMaxSupervisionCountValid
54+
? !hasChanges || !dialog.draftObjective.trim() || !isMaxSupervisionCountValid
5455
: !dialog.draftObjective.trim() || !isMaxSupervisionCountValid;
5556

5657
return (
@@ -111,7 +112,7 @@ export function ObjectiveDialog({ workspaceId, sessionId }: ObjectiveDialogProps
111112
<Button
112113
variant="primary"
113114
onClick={() => {
114-
if (mode === "edit" && !isRestoreMode) {
115+
if (mode === "edit" && !isRestoreMode && hasObjectiveChanged) {
115116
setIsSaveConfirmOpen(true);
116117
return;
117118
}

0 commit comments

Comments
 (0)