mirror of
https://github.com/logancyang/obsidian-copilot.git
synced 2026-07-22 07:50:24 +00:00
fix(agent-mode): disable mode/model picker while session is starting (#2485)
* fix(agent-mode): queue mode/model switch while session is still starting * fix(agent-mode): disable picker while session is starting instead of queueing Replace the await-session.ready queueing approach with a UI-level gate: canSwitchModel/Effort/Mode now return false while status === "starting", so the picker renders as disabled until the backend session is ready. The await-during-onChange path made the picker feel buggy because clicks appeared to do nothing for a beat before applying. * fix(agent-mode): re-render mode/model picker on session status change The picker hooks compute a memoization signal that React diffs to decide when to rebuild the override. Status was not part of the signal, so the starting → idle transition fired listeners but the snapshot string was unchanged and the picker stayed disabled forever. Include `getStatus()` in both signals so the picker re-enables once the session is ready.
This commit is contained in:
parent
ad4bd759a0
commit
0ebbc73424
4 changed files with 55 additions and 3 deletions
|
|
@ -848,6 +848,47 @@ describe("AgentSession intent capabilities", () => {
|
|||
}).canSwitchMode()
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("canSwitch* return false while the session status is starting", async () => {
|
||||
const mock = makeMockBackend();
|
||||
// Keep newSession pending so status stays "starting".
|
||||
let resolveNew: ((value: { sessionId: string; state: BackendState }) => void) | null = null;
|
||||
mock.newSession.mockReturnValueOnce(
|
||||
new Promise((resolve) => {
|
||||
resolveNew = resolve;
|
||||
})
|
||||
);
|
||||
mock.asBackend.isSetSessionModelSupported = () => true;
|
||||
mock.asBackend.isSetSessionConfigOptionSupported = () => true;
|
||||
mock.asBackend.isSetSessionModeSupported = () => true;
|
||||
const session = AgentSession.start({
|
||||
backend: mock.asBackend,
|
||||
cwd: "/vault",
|
||||
internalId: "internal-1",
|
||||
backendId: "test-backend",
|
||||
getDescriptor: () => undefined,
|
||||
});
|
||||
expect(session.getStatus()).toBe("starting");
|
||||
expect(session.canSwitchModel()).toBe(false);
|
||||
expect(session.canSwitchEffort()).toBe(false);
|
||||
expect(session.canSwitchMode()).toBe(false);
|
||||
|
||||
// After ready, the gate lifts and the underlying probes drive the answer.
|
||||
resolveNew!({
|
||||
sessionId: "acp-1",
|
||||
state: {
|
||||
model: null,
|
||||
mode: {
|
||||
current: "plan",
|
||||
options: [{ value: "plan", label: "Plan" }],
|
||||
apply: { plan: { kind: "setMode", nativeId: "plan" } },
|
||||
},
|
||||
},
|
||||
});
|
||||
await session.ready;
|
||||
expect(session.canSwitchModel()).toBe(true);
|
||||
expect(session.canSwitchMode()).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("AgentSession.setLabel", () => {
|
||||
|
|
|
|||
|
|
@ -333,6 +333,7 @@ export class AgentSession {
|
|||
|
||||
/** Whether the user can swap the active model on this session. */
|
||||
canSwitchModel(): boolean | null {
|
||||
if (this.status === "starting") return false;
|
||||
return this.backend.isSetSessionModelSupported();
|
||||
}
|
||||
|
||||
|
|
@ -342,6 +343,7 @@ export class AgentSession {
|
|||
* routing is encapsulated here — UI consumers ask intent only.
|
||||
*/
|
||||
canSwitchEffort(): boolean | null {
|
||||
if (this.status === "starting") return false;
|
||||
const descriptor = this.getDescriptor?.();
|
||||
if (!descriptor) return null;
|
||||
return descriptor.wire.effortConfigFor
|
||||
|
|
@ -355,6 +357,7 @@ export class AgentSession {
|
|||
* the dispatch path is consistent, so we sample the first option.
|
||||
*/
|
||||
canSwitchMode(): boolean | null {
|
||||
if (this.status === "starting") return false;
|
||||
const mode = this.currentState?.mode;
|
||||
if (!mode) return null;
|
||||
const sample = mode.options[0];
|
||||
|
|
|
|||
|
|
@ -29,9 +29,14 @@ function useAgentModeSignal(manager: AgentSessionManager | null): string {
|
|||
if (!manager) return "";
|
||||
const session = manager.getActiveSession();
|
||||
const state = session?.getState() ?? manager.getCachedBackendState(session?.backendId ?? "");
|
||||
return [session?.internalId ?? "", session?.backendId ?? "", modeStateSignature(state)].join(
|
||||
"|"
|
||||
);
|
||||
return [
|
||||
session?.internalId ?? "",
|
||||
session?.backendId ?? "",
|
||||
// Include status so the picker's `disabled` flips when the session
|
||||
// transitions out of "starting" (canSwitchMode gates on status).
|
||||
session?.getStatus() ?? "",
|
||||
modeStateSignature(state),
|
||||
].join("|");
|
||||
}, [manager]);
|
||||
|
||||
return useSyncExternalStore(subscribe, getSnapshot, getSnapshot);
|
||||
|
|
|
|||
|
|
@ -61,6 +61,9 @@ function useAgentModelSignal(
|
|||
const parts: string[] = [
|
||||
session?.internalId ?? "",
|
||||
session?.backendId ?? "",
|
||||
// Include status so the picker's `disabled` flips when the session
|
||||
// transitions out of "starting" (canSwitchModel/Effort gate on status).
|
||||
session?.getStatus() ?? "",
|
||||
session?.hasUserVisibleMessages() ? "1" : "0",
|
||||
];
|
||||
for (const d of descriptors) {
|
||||
|
|
|
|||
Loading…
Reference in a new issue