mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(coding-agent): preserve active operation UI during tree navigation
Follow-up to e687434a6.
Recheck compaction state after navigation dialogs and response cancellation before replacing status indicators or escape handlers. Document navigation conflicts and add regression coverage.
This commit is contained in:
parent
1f78cea7ad
commit
47acd8e6cf
5 changed files with 156 additions and 2 deletions
|
|
@ -255,7 +255,7 @@ Use `/session` in interactive mode to see the current session ID before reusing
|
|||
|
||||
### Branching
|
||||
|
||||
**`/tree`** - Navigate the session tree in-place. Select any previous point, continue from there, and switch between branches. All history preserved in a single file.
|
||||
**`/tree`** - Navigate the session tree in-place. Select any previous point, continue from there, and switch between branches. All history preserved in a single file. Selecting a point while the model is responding cancels that response. Navigation cannot proceed while compaction or another tree navigation is still running; wait for it to finish and retry.
|
||||
|
||||
<p align="center"><img src="docs/images/tree-view.png" alt="Tree View" width="600"></p>
|
||||
|
||||
|
|
|
|||
|
|
@ -1203,7 +1203,7 @@ Options:
|
|||
|
||||
### ctx.navigateTree(targetId, options?)
|
||||
|
||||
Navigate to a different point in the session tree:
|
||||
Navigate to a different point in the session tree. Rejects while an agent response, manual or automatic compaction, or another tree navigation is active, even with `summarize: false`. These conflicts leave the active branch unchanged and reject the promise rather than returning `{ cancelled: true }`. Wait for the active operation to finish (for example, with `await ctx.waitForIdle()` in a command handler) and retry:
|
||||
|
||||
```typescript
|
||||
const result = await ctx.navigateTree("entry-id-456", {
|
||||
|
|
|
|||
|
|
@ -111,6 +111,8 @@ interface AgentSession {
|
|||
}
|
||||
```
|
||||
|
||||
`session.navigateTree()` rejects while an agent response, manual or automatic compaction, or another tree navigation is active, even with `summarize: false`. It does not queue navigation or return `{ cancelled: true }` for these conflicts. Wait for the active operation to finish (for example, with `await session.waitForIdle()`) and retry. Rejection leaves the active branch unchanged.
|
||||
|
||||
Session replacement APIs such as new-session, resume, fork, and import live on `AgentSessionRuntime`, not on `AgentSession`.
|
||||
|
||||
### createAgentSessionRuntime() and AgentSessionRuntime
|
||||
|
|
|
|||
|
|
@ -5270,6 +5270,14 @@ export class InteractiveMode {
|
|||
await this.session.abort();
|
||||
}
|
||||
|
||||
// Recheck after the dialogs and streaming abort, before replacing another operation's UI.
|
||||
if (this.session.isCompacting) {
|
||||
this.showError(
|
||||
"Wait for the current compaction or tree navigation to finish before navigating the session tree.",
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// Set up escape handler and status indicator if summarizing
|
||||
let showingSummaryIndicator = false;
|
||||
const originalOnEscape = this.defaultEditor.onEscape;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,144 @@
|
|||
import { Container } from "@earendil-works/pi-tui";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { SessionManager } from "../src/core/session-manager.ts";
|
||||
import { SettingsManager } from "../src/core/settings-manager.ts";
|
||||
import type { StatusIndicator } from "../src/modes/interactive/components/status-indicator.ts";
|
||||
import type { TreeSelectorComponent } from "../src/modes/interactive/components/tree-selector.ts";
|
||||
import { InteractiveMode } from "../src/modes/interactive/interactive-mode.ts";
|
||||
import { initTheme } from "../src/modes/interactive/theme/theme.ts";
|
||||
import { assistantMsg, userMsg } from "./utilities.ts";
|
||||
|
||||
const busyMessage = "Wait for the current compaction or tree navigation to finish before navigating the session tree.";
|
||||
|
||||
function createTreeUI() {
|
||||
const sessionManager = SessionManager.inMemory();
|
||||
const targetId = sessionManager.appendMessage(userMsg("first"));
|
||||
sessionManager.appendMessage(assistantMsg("reply"));
|
||||
let selector: TreeSelectorComponent | undefined;
|
||||
const onEscape = vi.fn();
|
||||
const ui = {
|
||||
sessionManager,
|
||||
settingsManager: SettingsManager.inMemory(),
|
||||
session: {
|
||||
isStreaming: false,
|
||||
isCompacting: false,
|
||||
abort: vi.fn(async () => {
|
||||
ui.session.isStreaming = false;
|
||||
}),
|
||||
abortBranchSummary: vi.fn(),
|
||||
navigateTree: vi.fn(async () => {
|
||||
if (ui.session.isCompacting) throw new Error(busyMessage);
|
||||
return { cancelled: false };
|
||||
}),
|
||||
},
|
||||
defaultEditor: { onEscape },
|
||||
editor: { getText: () => "", setText: vi.fn() },
|
||||
chatContainer: new Container(),
|
||||
isInitialized: true,
|
||||
footer: { invalidate: vi.fn() },
|
||||
ui: { terminal: { rows: 24, setProgress: vi.fn() }, requestRender: vi.fn() },
|
||||
showSelector: (
|
||||
create: (done: () => void) => { component: TreeSelectorComponent; focus: TreeSelectorComponent },
|
||||
) => {
|
||||
selector = create(vi.fn()).component;
|
||||
},
|
||||
showExtensionSelector: vi.fn(async () => "No summary"),
|
||||
// Dispose newly created indicators so a failing regression cannot leak spinner timers.
|
||||
showStatusIndicator: vi.fn((indicator: StatusIndicator) => indicator.dispose()),
|
||||
clearStatusIndicator: vi.fn(),
|
||||
restoreQueuedMessagesToEditor: vi.fn(),
|
||||
renderInitialMessages: vi.fn(),
|
||||
showStatus: vi.fn(),
|
||||
showError: vi.fn(),
|
||||
flushCompactionQueue: vi.fn(async () => {}),
|
||||
};
|
||||
const showTreeSelector = Reflect.get(InteractiveMode.prototype, "showTreeSelector") as (this: typeof ui) => void;
|
||||
showTreeSelector.call(ui);
|
||||
|
||||
return {
|
||||
ui,
|
||||
onEscape,
|
||||
targetId,
|
||||
async select() {
|
||||
expect(selector).toBeDefined();
|
||||
await selector!.getTreeList().onSelect!(targetId);
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe("InteractiveMode tree navigation availability", () => {
|
||||
beforeEach(() => initTheme("dark"));
|
||||
|
||||
// Regression for #9178 / PR #9179: rejection must not replace the active operation's UI.
|
||||
it.each(["Summarize", "No summary"])("preserves operation UI when choosing %s while busy", async (choice) => {
|
||||
const { ui, onEscape, select } = createTreeUI();
|
||||
const originalLeafId = ui.sessionManager.getLeafId();
|
||||
ui.showExtensionSelector.mockImplementation(async () => {
|
||||
// Compaction or another navigation can start while the dialog is open.
|
||||
ui.session.isCompacting = true;
|
||||
return choice;
|
||||
});
|
||||
|
||||
await select();
|
||||
|
||||
expect(ui.showError).toHaveBeenCalledWith(busyMessage);
|
||||
expect(ui.showStatusIndicator).not.toHaveBeenCalled();
|
||||
expect(ui.clearStatusIndicator).not.toHaveBeenCalled();
|
||||
expect(ui.defaultEditor.onEscape).toBe(onEscape);
|
||||
expect(ui.session.navigateTree).not.toHaveBeenCalled();
|
||||
expect(ui.session.abort).not.toHaveBeenCalled();
|
||||
expect(ui.sessionManager.getLeafId()).toBe(originalLeafId);
|
||||
});
|
||||
|
||||
it("allows navigation when compaction finishes while the dialog is open", async () => {
|
||||
const { ui, targetId, select } = createTreeUI();
|
||||
ui.session.isCompacting = true;
|
||||
ui.showExtensionSelector.mockImplementation(async () => {
|
||||
ui.session.isCompacting = false;
|
||||
return "No summary";
|
||||
});
|
||||
|
||||
await select();
|
||||
|
||||
expect(ui.session.navigateTree).toHaveBeenCalledWith(targetId, {
|
||||
summarize: false,
|
||||
customInstructions: undefined,
|
||||
});
|
||||
expect(ui.showError).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("still aborts an active response before navigating", async () => {
|
||||
const { ui, select } = createTreeUI();
|
||||
ui.session.isStreaming = true;
|
||||
ui.session.navigateTree.mockImplementation(async () => {
|
||||
expect(ui.session.isStreaming).toBe(false);
|
||||
expect(ui.restoreQueuedMessagesToEditor).toHaveBeenCalledOnce();
|
||||
return { cancelled: false };
|
||||
});
|
||||
|
||||
await select();
|
||||
|
||||
expect(ui.session.abort).toHaveBeenCalledOnce();
|
||||
expect(ui.session.navigateTree).toHaveBeenCalledOnce();
|
||||
expect(ui.showError).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rechecks availability after the response abort settles", async () => {
|
||||
const { ui, onEscape, select } = createTreeUI();
|
||||
ui.session.isStreaming = true;
|
||||
ui.showExtensionSelector.mockResolvedValue("Summarize");
|
||||
ui.session.abort.mockImplementation(async () => {
|
||||
ui.session.isStreaming = false;
|
||||
ui.session.isCompacting = true;
|
||||
});
|
||||
|
||||
await select();
|
||||
|
||||
expect(ui.session.abort).toHaveBeenCalledOnce();
|
||||
expect(ui.showError).toHaveBeenCalledWith(busyMessage);
|
||||
expect(ui.showStatusIndicator).not.toHaveBeenCalled();
|
||||
expect(ui.clearStatusIndicator).not.toHaveBeenCalled();
|
||||
expect(ui.defaultEditor.onEscape).toBe(onEscape);
|
||||
expect(ui.session.navigateTree).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue