From c2946d0dff1e38946ef41eb91b282068d9473f54 Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 24 Jul 2026 05:12:28 +0000 Subject: [PATCH 1/2] fix(coding-agent): resume agent after /tree ask re-answer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Committing a new answer to a past `ask` via `/tree` branched a fresh sibling toolResult and rebuilt context, but `navigateTree` never scheduled `agent.continue()`. Unlike a live `ask` — whose continuation is intrinsic to the streaming run loop — the /tree re-answer mutates the tree outside any running turn, so the model never consumed the new answer and the session sat idle until a manual prompt. Schedule an agent continue at the end of the reanswerAskResult completion, gated to the ask re-answer branch only so plain leaf moves and the read-only reopenAsk probe stay idle. Fixes #6483 --- packages/coding-agent/CHANGELOG.md | 1 + .../coding-agent/src/session/agent-session.ts | 18 ++++++ .../agent-session-tree-ask-reanswer.test.ts | 64 +++++++++++++++++++ 3 files changed, 83 insertions(+) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 27ede5c6d..202dd082e 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -15,6 +15,7 @@ - Fixed spilled tool-output artifact descriptors leaking on error/abort paths. `OutputSink.dump()` was the only path that closed the spill `Bun.FileSink`, but the bash and Python executors re-throw on failure and their `finally` blocks never closed the sink, so a large-output command that errored leaked the artifact descriptor until an unrelated read (e.g. a `SKILL.md` load) hit `EMFILE`. `OutputSink` now exposes an idempotent `dispose()` that closes the sink exactly once, wired into every executor's `finally` ([#6463](https://github.com/can1357/oh-my-pi/issues/6463)). - Fixed the first submitted prompt stalling while the local tiny-title worker started: the interactive submit handler now paints the pending user row before starting title generation, and startup prewarms an idle, unref'd worker so the first submit reuses a live subprocess instead of paying spawn latency ahead of the first frame ([#6462](https://github.com/can1357/oh-my-pi/issues/6462)). - Fixed legacy Pi extensions failing validation when importing the upstream `keyText` keybinding helper ([#6470](https://github.com/can1357/oh-my-pi/issues/6470)). +- Fixed the agent not resuming after re-answering a past `ask` from the session tree. Committing a new answer via `/tree` branched a fresh sibling `toolResult` and rebuilt context, but `navigateTree` never scheduled `agent.continue()` — unlike a live `ask`, whose continuation is intrinsic to the streaming run loop — so the model never consumed the new answer and the session sat idle until a manual prompt. The re-answer completion now schedules an agent continue (plain leaf moves and the read-only `reopenAsk` probe stay idle) ([#6483](https://github.com/can1357/oh-my-pi/issues/6483)). ## [17.1.0] - 2026-07-24 diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 97ed867ee..63d918953 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -7508,6 +7508,10 @@ export class AgentSession { // Determine the new leaf position based on target type let newLeafId: string | null; let editorText: string | undefined; + // Set when the second-pass `ask` re-answer branch below actually commits a + // new sibling answer — the trigger for resuming the agent afterwards so the + // model consumes it, mirroring a live `ask` completion (issue #6483). + let isAskReanswerCompletion = false; if (targetEntry.type === "message" && targetEntry.message.role === "user") { // User message: leaf = parent (null if root), text goes to editor @@ -7544,6 +7548,7 @@ export class AgentSession { timestamp: Date.now(), }; newLeafId = this.sessionManager.appendMessageToBranch(toolResultMessage, targetEntry.parentId); + isAskReanswerCompletion = true; } else { // Non-user message (or a user-invoked skill-prompt injection): land the // leaf on the selected node so it stays on the active branch. Skill @@ -7589,6 +7594,19 @@ export class AgentSession { this.#branchSummaryAbortController = undefined; + // Resume the agent after a committed `ask` re-answer. A live `ask` + // completes inside the streaming run loop, whose continuation after a + // tool result is intrinsic; a `/tree` re-answer instead mutates the tree + // outside any running turn, so nothing would pick up the new answer + // without an explicit continue. Scheduled as a post-prompt task (honoring + // the disposed/compacting guards) rather than awaited, so the interactive + // caller still rebuilds its UI synchronously first (issue #6483). Plain + // leaf moves and the read-only `reopenAsk` probe never reach here with the + // flag set, so they stay idle as before. + if (isAskReanswerCompletion) { + this.#scheduleAgentContinue(); + } + // Emit session_tree event; only handlers can mutate session entries, so skip // the emit and the context rebuild when no handlers are registered (mirrors // the session_before_tree guard above). diff --git a/packages/coding-agent/test/agent-session-tree-ask-reanswer.test.ts b/packages/coding-agent/test/agent-session-tree-ask-reanswer.test.ts index daa43ea0e..37c60f9a1 100644 --- a/packages/coding-agent/test/agent-session-tree-ask-reanswer.test.ts +++ b/packages/coding-agent/test/agent-session-tree-ask-reanswer.test.ts @@ -458,6 +458,70 @@ describe("AgentSession tree navigation onto an ask toolResult", () => { await ctx.cleanup(); } }); + + it("(k) schedules an agent continue after a successful ask re-answer so the model consumes the new answer", async () => { + const ctx = await createTestSession({ inMemory: true }); + try { + const { session, sessionManager } = ctx; + + // u1 -> a1(ask toolCall) -> tr1(stale answer) -> a2(next reply, leaf) + sessionManager.appendMessage(userMsg("please deploy")); + const askCallId = "ask-call-1"; + sessionManager.appendMessage(toolCallMsg(askCallId, "ask", { questions: ORIGINAL_QUESTIONS })); + const tr1Id = sessionManager.appendMessage( + toolResultMsg(askCallId, "ask", "User selected: staging", staleAnswerResult().details), + ); + sessionManager.appendMessage(assistantMsg("deploying to staging")); + + const continueSpy = vi.spyOn(session.agent, "continue").mockResolvedValue(undefined); + + const probe = await session.navigateTree(tr1Id, { allowAskReopen: true }); + expect(probe.reopenAsk).toBeDefined(); + // The read-only probe must NOT resume the agent — nothing was committed yet. + await session.waitForIdle(); + expect(continueSpy).not.toHaveBeenCalled(); + + const result = await session.navigateTree(tr1Id, { + allowAskReopen: true, + reanswerAskResult: newAnswerResult(), + }); + expect(result.cancelled).toBe(false); + + // The freshly-branched ask toolResult must drive a follow-up turn, matching + // how a live ask completion resumes the agent (issue #6483). + await session.waitForIdle(); + expect(continueSpy).toHaveBeenCalledTimes(1); + // The tail the continue resumes from is the new answer toolResult. + const messages = session.messages; + const last = messages[messages.length - 1]; + expect(last?.role).toBe("toolResult"); + } finally { + await ctx.cleanup(); + } + }); + + it("(l) does not schedule a continue for a plain non-ask leaf move", async () => { + const ctx = await createTestSession({ inMemory: true }); + try { + const { session, sessionManager } = ctx; + + sessionManager.appendMessage(userMsg("read the config")); + sessionManager.appendMessage(toolCallMsg("read-call-1", "read", { path: "config.txt" })); + const tr1Id = sessionManager.appendMessage(toolResultMsg("read-call-1", "read", "file body")); + sessionManager.appendMessage(assistantMsg("done reading")); + + const continueSpy = vi.spyOn(session.agent, "continue").mockResolvedValue(undefined); + + const result = await session.navigateTree(tr1Id, { allowAskReopen: true }); + expect(result.cancelled).toBe(false); + expect(result.reopenAsk).toBeUndefined(); + + await session.waitForIdle(); + expect(continueSpy).not.toHaveBeenCalled(); + } finally { + await ctx.cleanup(); + } + }); }); describe("AgentSession.buildAskReanswerContext", () => { From 7dee729ef72bd59be5b43221732fcb3c98e03ff0 Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 24 Jul 2026 05:27:00 +0000 Subject: [PATCH 2/2] fix(coding-agent): defer /tree ask re-answer resume until after ui rebuild Scheduling agent.continue() inside navigateTree started a post-prompt task before the interactive /tree handler rebuilt its transcript, so a fast provider's agent_start/turn_start could render against the stale pre-rebuild UI and then be clobbered by renderInitialMessages. navigateTree now reports the commit via askReanswerCommitted instead of resuming, and SelectorController.showTreeSelector calls the new AgentSession.resumeAfterAskReanswer() only after renderInitialMessages + reloadTodos, so the resumed turn always renders against the rebuilt transcript. Addresses chatgpt-codex-connector review on #6484. Fixes #6483 --- packages/coding-agent/CHANGELOG.md | 2 +- .../modes/controllers/selector-controller.ts | 9 +++ .../coding-agent/src/session/agent-session.ts | 56 ++++++++++++++----- .../agent-session-tree-ask-reanswer.test.ts | 27 +++++---- ...ector-controller-tree-ask-reanswer.test.ts | 53 +++++++++++++++++- 5 files changed, 118 insertions(+), 29 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 202dd082e..b2169c3fd 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -15,7 +15,7 @@ - Fixed spilled tool-output artifact descriptors leaking on error/abort paths. `OutputSink.dump()` was the only path that closed the spill `Bun.FileSink`, but the bash and Python executors re-throw on failure and their `finally` blocks never closed the sink, so a large-output command that errored leaked the artifact descriptor until an unrelated read (e.g. a `SKILL.md` load) hit `EMFILE`. `OutputSink` now exposes an idempotent `dispose()` that closes the sink exactly once, wired into every executor's `finally` ([#6463](https://github.com/can1357/oh-my-pi/issues/6463)). - Fixed the first submitted prompt stalling while the local tiny-title worker started: the interactive submit handler now paints the pending user row before starting title generation, and startup prewarms an idle, unref'd worker so the first submit reuses a live subprocess instead of paying spawn latency ahead of the first frame ([#6462](https://github.com/can1357/oh-my-pi/issues/6462)). - Fixed legacy Pi extensions failing validation when importing the upstream `keyText` keybinding helper ([#6470](https://github.com/can1357/oh-my-pi/issues/6470)). -- Fixed the agent not resuming after re-answering a past `ask` from the session tree. Committing a new answer via `/tree` branched a fresh sibling `toolResult` and rebuilt context, but `navigateTree` never scheduled `agent.continue()` — unlike a live `ask`, whose continuation is intrinsic to the streaming run loop — so the model never consumed the new answer and the session sat idle until a manual prompt. The re-answer completion now schedules an agent continue (plain leaf moves and the read-only `reopenAsk` probe stay idle) ([#6483](https://github.com/can1357/oh-my-pi/issues/6483)). +- Fixed the agent not resuming after re-answering a past `ask` from the session tree. Committing a new answer via `/tree` branched a fresh sibling `toolResult` and rebuilt context, but nothing ever continued the agent — unlike a live `ask`, whose continuation is intrinsic to the streaming run loop — so the model never consumed the new answer and the session sat idle until a manual prompt. `navigateTree` now reports the commit (`askReanswerCommitted`) and the interactive `/tree` handler resumes the agent via `resumeAfterAskReanswer()` *after* its transcript rebuild, so the resumed turn never renders against the stale pre-rebuild UI. Plain leaf moves and the read-only `reopenAsk` probe stay idle ([#6483](https://github.com/can1357/oh-my-pi/issues/6483)). ## [17.1.0] - 2026-07-24 diff --git a/packages/coding-agent/src/modes/controllers/selector-controller.ts b/packages/coding-agent/src/modes/controllers/selector-controller.ts index c1ac0c477..ae4da94e1 100644 --- a/packages/coding-agent/src/modes/controllers/selector-controller.ts +++ b/packages/coding-agent/src/modes/controllers/selector-controller.ts @@ -1267,6 +1267,15 @@ export class SelectorController { this.ctx.editor.setText(result.editorText); } this.ctx.showStatus("Navigated to selected point"); + + // Re-answering a past `ask` commits a new sibling answer but, + // unlike a live `ask`, leaves the agent idle. Resume it now — + // after the transcript rebuild above — so the model consumes the + // new answer without the resumed turn rendering against the stale + // pre-rebuild UI (issue #6483). + if (result.askReanswerCommitted) { + this.ctx.session.resumeAfterAskReanswer(); + } } catch (error) { this.ctx.showError(error instanceof Error ? error.message : String(error)); } finally { diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 63d918953..6cf47899e 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -7358,6 +7358,14 @@ export class AgentSession { * (issue #5642). */ reopenAsk?: { toolCallId: string; questions: AskToolInput["questions"] }; + /** + * `true` when this call committed a new sibling answer for an `ask` + * re-answer (`reanswerAskResult` was applied). The interactive caller + * resumes the agent via {@link resumeAfterAskReanswer} *after* rebuilding + * its transcript, so the resumed turn never renders against the stale + * pre-rebuild UI (issue #6483). + */ + askReanswerCommitted?: boolean; }> { await this.#bash.flushPending(); const oldLeafId = this.sessionManager.getLeafId(); @@ -7594,18 +7602,13 @@ export class AgentSession { this.#branchSummaryAbortController = undefined; - // Resume the agent after a committed `ask` re-answer. A live `ask` - // completes inside the streaming run loop, whose continuation after a - // tool result is intrinsic; a `/tree` re-answer instead mutates the tree - // outside any running turn, so nothing would pick up the new answer - // without an explicit continue. Scheduled as a post-prompt task (honoring - // the disposed/compacting guards) rather than awaited, so the interactive - // caller still rebuilds its UI synchronously first (issue #6483). Plain - // leaf moves and the read-only `reopenAsk` probe never reach here with the - // flag set, so they stay idle as before. - if (isAskReanswerCompletion) { - this.#scheduleAgentContinue(); - } + // Report a committed `ask` re-answer so the interactive caller can resume + // the agent via `resumeAfterAskReanswer()` *after* rebuilding its + // transcript. Scheduling the continue here instead would start a fresh + // streaming turn whose `agent_start`/`turn_start` events could render + // against the stale pre-rebuild UI and then be clobbered by the caller's + // `renderInitialMessages(...)` (issue #6483). Plain leaf moves and the + // read-only `reopenAsk` probe leave the flag unset. // Emit session_tree event; only handlers can mutate session entries, so skip // the emit and the context rebuild when no handlers are registered (mirrors @@ -7619,9 +7622,34 @@ export class AgentSession { fromExtension: summaryText ? fromExtension : undefined, }); const rawContext = this.sessionManager.buildSessionContext(); - return { editorText, cancelled: false, summaryEntry, sessionContext: rawContext }; + return { + editorText, + cancelled: false, + summaryEntry, + sessionContext: rawContext, + askReanswerCommitted: isAskReanswerCompletion, + }; } - return { editorText, cancelled: false, summaryEntry, sessionContext: stateContext }; + return { + editorText, + cancelled: false, + summaryEntry, + sessionContext: stateContext, + askReanswerCommitted: isAskReanswerCompletion, + }; + } + + /** + * Resume the agent after the interactive `/tree` caller has committed an + * `ask` re-answer (`navigateTree` returned `askReanswerCommitted`) and + * rebuilt its transcript. Mirrors how a live `ask` completion drives a + * follow-up turn, but is deferred to the caller so the resumed turn renders + * against the rebuilt UI rather than the stale pre-navigation transcript + * (issue #6483). The scheduled continue honors the same disposed/compacting + * guards as every other post-prompt continuation. + */ + resumeAfterAskReanswer(): void { + this.#scheduleAgentContinue(); } /** diff --git a/packages/coding-agent/test/agent-session-tree-ask-reanswer.test.ts b/packages/coding-agent/test/agent-session-tree-ask-reanswer.test.ts index 37c60f9a1..1108a421d 100644 --- a/packages/coding-agent/test/agent-session-tree-ask-reanswer.test.ts +++ b/packages/coding-agent/test/agent-session-tree-ask-reanswer.test.ts @@ -459,7 +459,7 @@ describe("AgentSession tree navigation onto an ask toolResult", () => { } }); - it("(k) schedules an agent continue after a successful ask re-answer so the model consumes the new answer", async () => { + it("(k) reports a committed re-answer and resumes the agent only via resumeAfterAskReanswer", async () => { const ctx = await createTestSession({ inMemory: true }); try { const { session, sessionManager } = ctx; @@ -477,30 +477,34 @@ describe("AgentSession tree navigation onto an ask toolResult", () => { const probe = await session.navigateTree(tr1Id, { allowAskReopen: true }); expect(probe.reopenAsk).toBeDefined(); - // The read-only probe must NOT resume the agent — nothing was committed yet. - await session.waitForIdle(); - expect(continueSpy).not.toHaveBeenCalled(); + // The read-only probe commits nothing, so it must not report a re-answer. + expect(probe.askReanswerCommitted).toBeFalsy(); const result = await session.navigateTree(tr1Id, { allowAskReopen: true, reanswerAskResult: newAnswerResult(), }); expect(result.cancelled).toBe(false); + // navigateTree reports the commit but does NOT resume on its own — the + // interactive caller owns the timing so the resumed turn renders against + // the rebuilt transcript (issue #6483). + expect(result.askReanswerCommitted).toBe(true); + await session.waitForIdle(); + expect(continueSpy).not.toHaveBeenCalled(); + // The tail the resume will continue from is the new answer toolResult. + const messages = session.messages; + expect(messages[messages.length - 1]?.role).toBe("toolResult"); - // The freshly-branched ask toolResult must drive a follow-up turn, matching - // how a live ask completion resumes the agent (issue #6483). + // The caller resumes explicitly after rebuilding its UI. + session.resumeAfterAskReanswer(); await session.waitForIdle(); expect(continueSpy).toHaveBeenCalledTimes(1); - // The tail the continue resumes from is the new answer toolResult. - const messages = session.messages; - const last = messages[messages.length - 1]; - expect(last?.role).toBe("toolResult"); } finally { await ctx.cleanup(); } }); - it("(l) does not schedule a continue for a plain non-ask leaf move", async () => { + it("(l) does not report a committed re-answer for a plain non-ask leaf move", async () => { const ctx = await createTestSession({ inMemory: true }); try { const { session, sessionManager } = ctx; @@ -515,6 +519,7 @@ describe("AgentSession tree navigation onto an ask toolResult", () => { const result = await session.navigateTree(tr1Id, { allowAskReopen: true }); expect(result.cancelled).toBe(false); expect(result.reopenAsk).toBeUndefined(); + expect(result.askReanswerCommitted).toBeFalsy(); await session.waitForIdle(); expect(continueSpy).not.toHaveBeenCalled(); diff --git a/packages/coding-agent/test/modes/controllers/selector-controller-tree-ask-reanswer.test.ts b/packages/coding-agent/test/modes/controllers/selector-controller-tree-ask-reanswer.test.ts index 5598731a9..34f94c93c 100644 --- a/packages/coding-agent/test/modes/controllers/selector-controller-tree-ask-reanswer.test.ts +++ b/packages/coding-agent/test/modes/controllers/selector-controller-tree-ask-reanswer.test.ts @@ -85,20 +85,35 @@ function createCtx(leafEntry: SessionEntry, navigateTreeResult: unknown = { canc const showStatus = vi.fn(); const showError = vi.fn(); const editorContainer = createEditorSlot(); + // Records the order of UI-rebuild vs agent-resume so a test can prove the + // re-answer continuation is deferred until after the transcript rebuild + // (issue #6483). + const order: string[] = []; + const renderInitialMessages = vi.fn(() => { + order.push("render"); + }); + const reloadTodos = vi.fn(async () => { + order.push("reloadTodos"); + }); + const resumeAfterAskReanswer = vi.fn(() => { + order.push("resume"); + }); const ctx = { - editor: { id: "editor" }, + editor: { id: "editor", getText: () => "", setText: vi.fn() }, editorContainer, sessionManager: { getTree: () => tree, getLeafId: () => leafEntry.id, getEntry: (id: string) => (id === leafEntry.id ? leafEntry : undefined), }, - session: { navigateTree }, + session: { navigateTree, resumeAfterAskReanswer }, ui: { setFocus: vi.fn(), requestRender: vi.fn(), terminal: { rows: 24 }, }, + renderInitialMessages, + reloadTodos, showStatus, showError, // No UI context available in this unit test — forces `#reanswerAsk` to @@ -109,7 +124,7 @@ function createCtx(leafEntry: SessionEntry, navigateTreeResult: unknown = { canc // itself (already covered at the session level). getToolUIContext: () => undefined, } as unknown as InteractiveModeContext; - return { ctx, editorContainer, navigateTree, showStatus, showError }; + return { ctx, editorContainer, navigateTree, showStatus, showError, resumeAfterAskReanswer, order }; } /** Grabs the `TreeSelectorComponent` mounted by the most recent `showTreeSelector()` call and fires its onSelect as if the user pressed Enter on `entryId`. */ @@ -160,4 +175,36 @@ describe("SelectorController.showTreeSelector re-answering the active ask leaf", expect(showError).toHaveBeenCalledWith("Ask tool UI is not ready"); expect(showStatus).toHaveBeenCalledWith("Re-answer cancelled"); }); + + it("resumes the agent only after rebuilding the transcript when navigateTree reports a committed re-answer", async () => { + const entry = plainUserEntry("leaf-user"); + const { ctx, editorContainer, showStatus, resumeAfterAskReanswer, order } = createCtx(entry, { + cancelled: false, + askReanswerCommitted: true, + }); + const controller = new SelectorController(ctx); + + controller.showTreeSelector(); + // A non-current target skips the no-op short-circuit and lands straight on + // the success path (navigateTree here returns a committed re-answer). + await pickEntry(editorContainer, "some-other-entry"); + + expect(showStatus).toHaveBeenCalledWith("Navigated to selected point"); + expect(resumeAfterAskReanswer).toHaveBeenCalledTimes(1); + // The resume must be deferred until after the transcript rebuild so the + // resumed turn never renders against the stale pre-rebuild UI (issue #6483). + expect(order.indexOf("render")).toBeGreaterThanOrEqual(0); + expect(order.indexOf("resume")).toBeGreaterThan(order.indexOf("render")); + }); + + it("does not resume the agent for a plain navigation without a committed re-answer", async () => { + const entry = plainUserEntry("leaf-user"); + const { ctx, editorContainer, resumeAfterAskReanswer } = createCtx(entry, { cancelled: false }); + const controller = new SelectorController(ctx); + + controller.showTreeSelector(); + await pickEntry(editorContainer, "some-other-entry"); + + expect(resumeAfterAskReanswer).not.toHaveBeenCalled(); + }); });