diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index eba99a611..899c8115a 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -115,6 +115,7 @@ - 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 Escape waiting for an in-flight `session_stop` extension handler to exhaust its timeout; abort now cancels the active stop pass without reporting a false timeout or applying stale continuation context ([#6489](https://github.com/can1357/oh-my-pi/issues/6489)). +- 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 a2010c276..d6602a4bb 100644 --- a/packages/coding-agent/src/modes/controllers/selector-controller.ts +++ b/packages/coding-agent/src/modes/controllers/selector-controller.ts @@ -1275,6 +1275,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 2d166031e..5c92b75a7 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -7339,6 +7339,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(); @@ -7489,6 +7497,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 @@ -7525,6 +7537,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 @@ -7570,6 +7583,14 @@ export class AgentSession { this.#branchSummaryAbortController = undefined; + // 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 // the session_before_tree guard above). @@ -7582,9 +7603,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 daa43ea0e..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 @@ -458,6 +458,75 @@ describe("AgentSession tree navigation onto an ask toolResult", () => { await ctx.cleanup(); } }); + + 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; + + // 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 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 caller resumes explicitly after rebuilding its UI. + session.resumeAfterAskReanswer(); + await session.waitForIdle(); + expect(continueSpy).toHaveBeenCalledTimes(1); + } finally { + await ctx.cleanup(); + } + }); + + 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; + + 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(); + expect(result.askReanswerCommitted).toBeFalsy(); + + await session.waitForIdle(); + expect(continueSpy).not.toHaveBeenCalled(); + } finally { + await ctx.cleanup(); + } + }); }); describe("AgentSession.buildAskReanswerContext", () => { 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(); + }); });