diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index e69efc977..be37ae99f 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -83,6 +83,7 @@ ### Fixed - Fixed the advisor auto-resuming a run after the user deliberately interrupts it (Esc, or a cancel from collab/ACP/RPC/SDK/extension). A user interrupt now suppresses advisor `concern`/`blocker` auto-resume until the user next resumes (a typed message, `.`/`c` continue, or a steer/follow-up); the concern is still recorded as a visible, persisted advisor card — including one already steered into the run or arriving mid-abort — so it re-enters context on resume instead of being discarded. Natural yields are unchanged: the advisor can still steer and resume a stalled run. +- Fixed `/advisor on|off` not being session-local by overriding the setting instead of modifying global configuration, and fixed changes not updating the TUI status line immediately. - Fixed `plan.defaultOnStartup` setting schema configuration missing the required `ui.group` property. - Fixed auto-retry regenerating a large `write` call after the provider stream timed out mid-tool-call ([#2683](https://github.com/can1357/oh-my-pi/issues/2683)). - Fixed soft-expired `issue://` and `pr://` reads to refresh live before returning stale state, with an explicit stale warning when the live refresh fails ([#2684](https://github.com/can1357/oh-my-pi/issues/2684)). diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 4847e595a..a5f30688c 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -1008,6 +1008,7 @@ export class AgentSession { #goalModeState: GoalModeState | undefined; #goalRuntime: GoalRuntime; #advisorRuntime?: AdvisorRuntime; + #advisorEnabled = false; /** The advisor's own agent, retained so `/dump advisor` can serialize its transcript. Undefined when no advisor is active. */ #advisorAgent?: Agent; #advisorReadOnlyTools?: AgentTool[]; @@ -1492,7 +1493,8 @@ export class AgentSession { }, }); - if (this.settings.get("advisor.enabled")) this.#buildAdvisorRuntime(); + this.#advisorEnabled = this.settings.get("advisor.enabled") as boolean; + if (this.#advisorEnabled) this.#buildAdvisorRuntime(); // Always subscribe to agent events for internal handling // (session persistence, hooks, auto-compaction, retry logic) @@ -1506,7 +1508,7 @@ export class AgentSession { #buildAdvisorRuntime(seedToCurrent = false): boolean { if (this.#isDisposed) return false; if (this.#advisorRuntime) return true; - if (!this.settings.get("advisor.enabled")) return false; + if (!this.#advisorEnabled) return false; if (this.#agentKind !== "main" && !this.settings.get("advisor.subagents")) return false; const advisorSel = resolveRoleSelection( @@ -11293,18 +11295,16 @@ export class AgentSession { } /** - * Enable or disable the advisor for this session. The setting is persisted, + * Enable or disable the advisor for this session. The setting is overridden for the session, * and the runtime is started or stopped to match. * * @returns true when the advisor is actively running after the call. */ setAdvisorEnabled(enabled: boolean): boolean { + this.#advisorEnabled = enabled; if (enabled) { - this.settings.clearOverride("advisor.enabled"); - this.settings.set("advisor.enabled", true); return this.#buildAdvisorRuntime(true); } - this.settings.set("advisor.enabled", false); this.#stopAdvisorRuntime(); return false; } @@ -11315,7 +11315,14 @@ export class AgentSession { * @returns true when the advisor is actively running after the call. */ toggleAdvisorEnabled(): boolean { - return this.setAdvisorEnabled(!this.settings.get("advisor.enabled")); + return this.setAdvisorEnabled(!this.#advisorEnabled); + } + + /** + * Whether the advisor setting is enabled for this session. + */ + isAdvisorEnabled(): boolean { + return this.#advisorEnabled; } /** @@ -11332,7 +11339,7 @@ export class AgentSession { * Return structured advisor stats for the status command and TUI panel. */ getAdvisorStats(): AdvisorStats { - const configured = this.settings.get("advisor.enabled") as boolean; + const configured = this.#advisorEnabled; const advisor = this.#advisorAgent; if (!advisor) { return { diff --git a/packages/coding-agent/src/slash-commands/builtin-registry.ts b/packages/coding-agent/src/slash-commands/builtin-registry.ts index 5433fd042..9362327f1 100644 --- a/packages/coding-agent/src/slash-commands/builtin-registry.ts +++ b/packages/coding-agent/src/slash-commands/builtin-registry.ts @@ -435,7 +435,7 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray = [ const { verb, rest } = parseSubcommand(command.args); if (!verb || verb === "toggle") { const active = runtime.session.toggleAdvisorEnabled(); - const configured = runtime.session.settings.get("advisor.enabled") as boolean; + const configured = runtime.session.isAdvisorEnabled(); if (active) { await runtime.output("Advisor enabled."); } else if (configured) { @@ -473,7 +473,7 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray = [ const { verb, rest } = parseSubcommand(command.args); if (!verb || verb === "toggle") { const active = runtime.ctx.session.toggleAdvisorEnabled(); - const configured = runtime.ctx.session.settings.get("advisor.enabled") as boolean; + const configured = runtime.ctx.session.isAdvisorEnabled(); if (active) { runtime.ctx.showStatus("Advisor enabled."); } else if (configured) { @@ -481,6 +481,7 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray = [ } else { runtime.ctx.showStatus("Advisor disabled."); } + refreshStatusLine(runtime.ctx); runtime.ctx.editor.setText(""); return; } @@ -489,12 +490,14 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray = [ runtime.ctx.showStatus( active ? "Advisor enabled." : "Advisor setting enabled, but no model is assigned to the 'advisor' role.", ); + refreshStatusLine(runtime.ctx); runtime.ctx.editor.setText(""); return; } if (verb === "off") { runtime.ctx.session.setAdvisorEnabled(false); runtime.ctx.showStatus("Advisor disabled."); + refreshStatusLine(runtime.ctx); runtime.ctx.editor.setText(""); return; } diff --git a/packages/coding-agent/test/advisor-toggle.test.ts b/packages/coding-agent/test/advisor-toggle.test.ts index 05597dca8..fb7469439 100644 --- a/packages/coding-agent/test/advisor-toggle.test.ts +++ b/packages/coding-agent/test/advisor-toggle.test.ts @@ -67,7 +67,7 @@ describe("AgentSession advisor toggle", () => { it("starts with advisor disabled", () => { expect(session.isAdvisorActive()).toBe(false); - expect(session.settings.get("advisor.enabled")).toBe(false); + expect(session.isAdvisorEnabled()).toBe(false); expect(session.formatAdvisorStatus()).toBe("Advisor is disabled."); }); @@ -76,19 +76,28 @@ describe("AgentSession advisor toggle", () => { const active = session.toggleAdvisorEnabled(); expect(active).toBe(true); expect(session.isAdvisorActive()).toBe(true); - expect(session.settings.get("advisor.enabled")).toBe(true); + expect(session.isAdvisorEnabled()).toBe(true); expect(session.formatAdvisorStatus()).toContain("Advisor is enabled (anthropic/claude-sonnet-4-5)"); }); - it("explicit enable clears protocol default-off override", () => { + it("explicit enable overrides default-off setting for the session only", () => { session.settings.setModelRole("advisor", "anthropic/claude-sonnet-4-5"); session.settings.override("advisor.enabled", false); + const customSession = new AgentSession({ + agent: session.agent, + sessionManager, + settings: session.settings, + modelRegistry, + advisorReadOnlyTools: [], + }); + expect(customSession.isAdvisorEnabled()).toBe(false); - const active = session.setAdvisorEnabled(true); + const active = customSession.setAdvisorEnabled(true); expect(active).toBe(true); - expect(session.isAdvisorActive()).toBe(true); - expect(session.settings.get("advisor.enabled")).toBe(true); + expect(customSession.isAdvisorActive()).toBe(true); + expect(customSession.isAdvisorEnabled()).toBe(true); + expect(customSession.settings.get("advisor.enabled")).toBe(false); }); it("toggle disables the advisor and runtime", () => { @@ -97,16 +106,60 @@ describe("AgentSession advisor toggle", () => { const active = session.toggleAdvisorEnabled(); expect(active).toBe(false); expect(session.isAdvisorActive()).toBe(false); - expect(session.settings.get("advisor.enabled")).toBe(false); + expect(session.isAdvisorEnabled()).toBe(false); }); it("setAdvisorEnabled reports inactive when no advisor model is assigned", () => { const active = session.setAdvisorEnabled(true); expect(active).toBe(false); expect(session.isAdvisorActive()).toBe(false); - expect(session.settings.get("advisor.enabled")).toBe(true); + expect(session.isAdvisorEnabled()).toBe(true); expect(session.formatAdvisorStatus()).toBe( "Advisor setting is enabled, but no model is assigned to the 'advisor' role.", ); }); + + it("keeps sessions isolated when sharing a Settings instance", async () => { + const sharedSettings = Settings.isolated({ "compaction.enabled": false }); + sharedSettings.setModelRole("advisor", "anthropic/claude-sonnet-4-5"); + expect(sharedSettings.get("advisor.enabled")).toBe(false); + + const sessionA = new AgentSession({ + agent: session.agent, + sessionManager, + settings: sharedSettings, + modelRegistry, + advisorReadOnlyTools: [], + }); + const sessionB = new AgentSession({ + agent: session.agent, + sessionManager, + settings: sharedSettings, + modelRegistry, + advisorReadOnlyTools: [], + }); + + expect(sessionA.isAdvisorEnabled()).toBe(false); + expect(sessionB.isAdvisorEnabled()).toBe(false); + + const activeA = sessionA.setAdvisorEnabled(true); + expect(activeA).toBe(true); + expect(sessionA.isAdvisorEnabled()).toBe(true); + expect(sessionA.isAdvisorActive()).toBe(true); + + expect(sessionB.isAdvisorEnabled()).toBe(false); + expect(sessionB.isAdvisorActive()).toBe(false); + expect(sessionB.formatAdvisorStatus()).toBe("Advisor is disabled."); + + const activeB = sessionB.toggleAdvisorEnabled(); + expect(activeB).toBe(true); + expect(sessionB.isAdvisorEnabled()).toBe(true); + + sessionA.setAdvisorEnabled(false); + expect(sessionA.isAdvisorEnabled()).toBe(false); + expect(sessionA.isAdvisorActive()).toBe(false); + + expect(sessionB.isAdvisorEnabled()).toBe(true); + expect(sessionB.isAdvisorActive()).toBe(true); + }); });