smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(coding-agent): roll back failed plan model restores

Browse Source
This commit is contained in:
can1357
2026-07-17 03:57:03 +02:00
parent 51ac313a63
commit d8c02f2bb3
2 changed files with 46 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/src/modes/interactive-mode.ts
+21 -21
View File
@@ -2354,31 +2354,31 @@ export class InteractiveMode implements InteractiveModeContext {
if (this.#planModePreviousTools !== undefined) {
await this.session.setActiveToolsByName(this.#planModePreviousTools);
}
if (this.#planModePreviousModelState) {
if (!options?.deferModelRestore) {
await this.#restorePlanPreviousModel(this.#planModePreviousModelState);
}
// If #applyPlanModeModel queued a deferred switch to the plan-role model
// (because the session was streaming on entry), drop it now: we are
// leaving plan mode, so flushing it on the next agent_end would land the
// session on the plan-role model after the user has exited plan mode
// (issue #816). This runs even when deferModelRestore is set
// (compact-approval path): otherwise the stale plan switch survives and
// flushPendingModelSwitch() later clobbers the restored/execution model.
// Only clear when the pending target matches the plan-role model — leave
// any unrelated user-queued switch intact.
const pending = this.#pendingModelSwitch;
if (pending) {
const planResolution = this.session.resolveRoleModelWithThinking("plan");
if (planResolution.model && modelsAreEqual(pending.model, planResolution.model)) {
this.#pendingModelSwitch = undefined;
}
}
}
} catch (error) {
this.session.setPlanModeState(planModeState);
throw error;
}
if (this.#planModePreviousModelState) {
if (!options?.deferModelRestore) {
await this.#restorePlanPreviousModel(this.#planModePreviousModelState);
}
// If #applyPlanModeModel queued a deferred switch to the plan-role model
// (because the session was streaming on entry), drop it now: we are
// leaving plan mode, so flushing it on the next agent_end would land the
// session on the plan-role model after the user has exited plan mode
// (issue #816). This runs even when deferModelRestore is set
// (compact-approval path): otherwise the stale plan switch survives and
// flushPendingModelSwitch() later clobbers the restored/execution model.
// Only clear when the pending target matches the plan-role model — leave
// any unrelated user-queued switch intact.
const pending = this.#pendingModelSwitch;
if (pending) {
const planResolution = this.session.resolveRoleModelWithThinking("plan");
if (planResolution.model && modelsAreEqual(pending.model, planResolution.model)) {
this.#pendingModelSwitch = undefined;
}
}
}
this.session.setPlanProposalHandler?.(null);
this.planModeEnabled = false;
// Suppress cache-miss marker on the next turn: plan exit changes the system
packages/coding-agent/test/issue-816-repro.test.ts
+25
View File
@@ -94,6 +94,31 @@ describe("issue #816 — plan mode pendingModelSwitch leak", () => {
expect(setModelSpy).not.toHaveBeenCalled();
});
it("keeps plan state coherent when restoring the previous model fails", async () => {
const planModel = modelRegistry.find("anthropic", "claude-haiku-4-5");
if (!planModel) throw new Error("Expected claude-haiku-4-5 in registry");
vi.spyOn(session, "resolveRoleModelWithThinking").mockReturnValue({
model: planModel,
thinkingLevel: undefined,
explicitThinkingLevel: false,
warning: undefined,
});
vi.spyOn(session, "sendPlanModeContext").mockResolvedValue(undefined);
const setModelSpy = vi.spyOn(session, "setModelTemporary");
await mode.handlePlanModeCommand();
expect(mode.planModeEnabled).toBe(true);
expect(session.getPlanModeState()?.enabled).toBe(true);
setModelSpy.mockRejectedValueOnce(new Error("model restore failed"));
vi.spyOn(mode, "showHookConfirm").mockResolvedValue(true);
await expect(mode.handlePlanModeCommand()).rejects.toThrow("model restore failed");
expect(mode.planModeEnabled).toBe(true);
expect(session.getPlanModeState()?.enabled).toBe(true);
});
it("does not enter plan mode when plan.enabled is false", async () => {
session.settings.set("plan.enabled", false);
const warning = vi.spyOn(mode, "showWarning").mockImplementation(() => {});