diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index ed0539025..3024f0731 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -9,6 +9,7 @@ - Added an optional `provider` parameter to `generate_image` (`auto` | `openai` | `openai-codex` | `antigravity` | `xai` | `gemini` | `openrouter`) that overrides the `providers.image` setting **for a single request** — so "generate this using gemini / codex / xai" routes per-call without changing the global setting. Absent → the `providers.image` setting applies, unchanged; the named provider uses the same resolution semantics (falls back to auto-detect if it has no credentials). File: `tools/image-gen.ts` (`imageProviderSchema`, `findImageApiKey` `preference` arg). - Added OpenTelemetry log and metric export alongside the existing trace export. When `OTEL_EXPORTER_OTLP_LOGS_ENDPOINT` (or the shared `OTEL_EXPORTER_OTLP_ENDPOINT`) is set, `omp` registers a `LoggerProvider` and forwards every centralized-logger event as an OTLP log record (severity + attributes + active span context for log↔trace correlation, min level via `OTEL_LOG_LEVEL`, plus a structured `agent run completed` summary event). When `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` (or the shared endpoint) is set, it registers a `MeterProvider` with a `PeriodicExportingMetricReader` and records GenAI-semconv `gen_ai.client.token.usage` plus `pi.omp.agent.*` counters/histograms (runs, steps, chat/tool calls by name+status+finish reason, latencies, estimated cost, errors) from the agent run summary and per-chat usage hooks. Each signal honors its own `OTEL_*_EXPORTER=none` kill switch, the global `OTEL_SDK_DISABLED`, and declines non-`http/protobuf` protocols independently ([#4604](https://github.com/can1357/oh-my-pi/issues/4604)). - `retry.fallbackChains` wildcards now support id-prefixed targets and keys: a chain entry like `"openrouter/google/*"` re-prefixes the failing model's bare id (`google-antigravity/gemini-x` → `openrouter/google/gemini-x`), a plain `"provider/*"` entry falling back *from* an aggregator strips the vendor prefix when the target provider only knows the bare id (`openrouter/google/x` → `google-vertex/x`), and an id-prefixed key (`"openrouter/google/*"`) scopes a chain to that provider's ids under the prefix. +- The session tree selector (`/tree`, `/branch`) now supports Shift+Enter to summarize-and-switch in one step: it forks from the selected entry with a branch summary, with no extra prompt and regardless of `branchSummary.enabled`. Plain Enter keeps the current behavior (direct switch by default; the summary prompt only when `branchSummary.enabled` is on). ([#5152](https://github.com/can1357/oh-my-pi/issues/5152)) ### Changed diff --git a/packages/coding-agent/src/modes/components/tree-selector.ts b/packages/coding-agent/src/modes/components/tree-selector.ts index da99bef5f..fc7e76e1b 100644 --- a/packages/coding-agent/src/modes/components/tree-selector.ts +++ b/packages/coding-agent/src/modes/components/tree-selector.ts @@ -66,7 +66,7 @@ class TreeList implements Component { #activePathIds: Set = new Set(); #lastSelectedId: string | null = null; - onSelect?: (entryId: string) => void; + onSelect?: (entryId: string, options: { summarize: boolean }) => void; onCancel?: () => void; onLabelEdit?: (entryId: string, currentLabel: string | undefined) => void; @@ -792,10 +792,16 @@ class TreeList implements Component { } else if (matchesKey(keyData, "right")) { // Page down this.#selectedIndex = Math.min(this.#filteredNodes.length - 1, this.#selectedIndex + this.maxVisibleLines); + } else if (matchesKey(keyData, "shift+enter") || matchesKey(keyData, "shift+return")) { + // Summarize-and-switch: fork with a branch summary without the extra prompt. + const selected = this.#filteredNodes[this.#selectedIndex]; + if (selected && this.onSelect) { + this.onSelect(selected.node.entry.id, { summarize: true }); + } } else if (matchesKey(keyData, "enter") || matchesKey(keyData, "return") || keyData === "\n") { const selected = this.#filteredNodes[this.#selectedIndex]; if (selected && this.onSelect) { - this.onSelect(selected.node.entry.id); + this.onSelect(selected.node.entry.id, { summarize: false }); } } else if (matchesAppInterrupt(keyData)) { if (this.#searchQuery) { @@ -923,7 +929,7 @@ export class TreeSelectorComponent extends Container { tree: SessionTreeNode[], currentLeafId: string | null, terminalHeight: number, - onSelect: (entryId: string) => void, + onSelect: (entryId: string, options: { summarize: boolean }) => void, onCancel: () => void, private readonly onLabelChangeCallback?: (entryId: string, label: string | undefined) => void, initialFilterMode: FilterMode = "default", @@ -948,7 +954,7 @@ export class TreeSelectorComponent extends Container { new TruncatedText( theme.fg( "muted", - "Up/Down: move. Left/Right: page. Shift+L: label. Ctrl+O/Shift+Ctrl+O: filter. Alt+D/T/U/L/A: filter. Type to search", + "Enter: switch. Shift+Enter: summarize & switch. Shift+L: label. Ctrl+O: filter. Alt+D/T/U/L/A: filter. Type to search", ), 0, 0, diff --git a/packages/coding-agent/src/modes/controllers/selector-controller.ts b/packages/coding-agent/src/modes/controllers/selector-controller.ts index 43a9cb4c9..371f06ee4 100644 --- a/packages/coding-agent/src/modes/controllers/selector-controller.ts +++ b/packages/coding-agent/src/modes/controllers/selector-controller.ts @@ -1137,7 +1137,7 @@ export class SelectorController { tree, realLeafId, this.ctx.ui.terminal.rows, - async entryId => { + async (entryId, options) => { // Selecting the current leaf is a no-op (already there) if (entryId === realLeafId) { done(); @@ -1148,13 +1148,15 @@ export class SelectorController { // Ask about summarization done(); // Close selector first - // Loop until user makes a complete choice or cancels to tree - let wantsSummary = false; + // Loop until user makes a complete choice or cancels to tree. + // Shift+Enter in the tree selector pre-answers "Summarize" and + // skips the prompt entirely. + let wantsSummary = options.summarize; let customInstructions: string | undefined; const branchSummariesEnabled = settings.get("branchSummary.enabled"); - while (branchSummariesEnabled) { + while (!wantsSummary && branchSummariesEnabled) { const summaryChoice = await this.ctx.showHookSelector("Summarize branch?", [ "No summary", "Summarize", diff --git a/packages/coding-agent/test/selector-controller-tree-summary.test.ts b/packages/coding-agent/test/selector-controller-tree-summary.test.ts new file mode 100644 index 000000000..325c20200 --- /dev/null +++ b/packages/coding-agent/test/selector-controller-tree-summary.test.ts @@ -0,0 +1,184 @@ +import { afterEach, beforeAll, beforeEach, describe, expect, it, type Mock, vi } from "bun:test"; +import type { AgentMessage } from "@oh-my-pi/pi-agent-core"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { SelectorController } from "@oh-my-pi/pi-coding-agent/modes/controllers/selector-controller"; +import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; +import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types"; +import type { SessionTreeNode } from "@oh-my-pi/pi-coding-agent/session/session-entries"; +import { setKittyProtocolActive } from "@oh-my-pi/pi-tui/keys"; +import { beginSettingsTest, restoreSettingsTestState, type SettingsTestState } from "./helpers/settings-test-state"; + +const SHIFT_ENTER = "\x1b[13;2u"; + +let settingsState: SettingsTestState | undefined; + +beforeAll(() => { + initTheme(); +}); + +beforeEach(async () => { + settingsState = beginSettingsTest(); + await Settings.init({ inMemory: true }); + setKittyProtocolActive(true); +}); + +afterEach(() => { + setKittyProtocolActive(false); + restoreSettingsTestState(settingsState); + settingsState = undefined; +}); + +function userNode(id: string, parentId: string | null, text: string): SessionTreeNode { + const message: AgentMessage = { role: "user", content: text, timestamp: 1 }; + return { + entry: { + type: "message", + id, + parentId, + timestamp: "2026-01-01T00:00:00Z", + message, + }, + children: [], + }; +} + +type NavigateTree = ( + entryId: string, + options: { summarize: boolean; customInstructions: string | undefined }, +) => Promise<{ cancelled: boolean }>; +type ShowHookSelector = (title: string, options: string[]) => Promise; + +interface TreeSummaryHarness { + controller: SelectorController; + navigateTree: Mock; + navigation: Promise; + selector(): { handleInput(key: string): void }; + showHookSelector: Mock; +} + +function createHarness(summaryChoice = "No summary"): TreeSummaryHarness { + const navigation = Promise.withResolvers(); + const root = userNode("root", null, "Root prompt"); + const showHookSelector = vi.fn(async () => summaryChoice); + const navigateTree = vi.fn(async () => { + navigation.resolve(); + return { cancelled: false }; + }); + let selector: { handleInput(key: string): void } | undefined; + const ctx = { + sessionManager: { + getTree: () => [root], + getLeafId: () => null, + appendLabelChange: vi.fn(), + }, + ui: { + terminal: { rows: 40 }, + setFocus: vi.fn(), + requestRender: vi.fn(), + requestComponentRender: vi.fn(), + }, + editorContainer: { + clear: vi.fn(), + addChild: vi.fn(), + }, + editor: { + getText: () => "", + setText: vi.fn(), + onEscape: undefined, + }, + showStatus: vi.fn(), + showError: vi.fn(), + showHookSelector, + showHookEditor: vi.fn(), + chatContainer: { addChild: vi.fn() }, + statusContainer: { + addChild: vi.fn(), + disposeChildren: vi.fn(), + }, + renderInitialMessages: vi.fn(), + reloadTodos: vi.fn(async () => {}), + session: { + navigateTree, + abortBranchSummary: vi.fn(), + }, + } as unknown as InteractiveModeContext; + const controller = new SelectorController(ctx); + controller.showSelector = create => { + const result = create(() => {}); + selector = result.component as { handleInput(key: string): void }; + }; + return { + controller, + navigateTree, + navigation: navigation.promise, + selector: () => { + if (!selector) throw new Error("Expected tree selector to be shown"); + return selector; + }, + showHookSelector, + }; +} + +describe("SelectorController tree branch summaries", () => { + it("switches without a summary or prompt on plain enter by default", async () => { + const harness = createHarness(); + + harness.controller.showTreeSelector(); + harness.selector().handleInput("\r"); + await harness.navigation; + + expect(harness.showHookSelector).not.toHaveBeenCalled(); + expect(harness.navigateTree).toHaveBeenCalledWith("root", { + summarize: false, + customInstructions: undefined, + }); + }); + + it("summarizes and switches on shift+enter without showing the prompt", async () => { + const harness = createHarness(); + + harness.controller.showTreeSelector(); + harness.selector().handleInput(SHIFT_ENTER); + await harness.navigation; + + expect(harness.showHookSelector).not.toHaveBeenCalled(); + expect(harness.navigateTree).toHaveBeenCalledWith("root", { + summarize: true, + customInstructions: undefined, + }); + }); + + it("skips the summary prompt on shift+enter even when branchSummary.enabled is on", async () => { + Settings.instance.set("branchSummary.enabled", true); + const harness = createHarness(); + + harness.controller.showTreeSelector(); + harness.selector().handleInput(SHIFT_ENTER); + await harness.navigation; + + expect(harness.showHookSelector).not.toHaveBeenCalled(); + expect(harness.navigateTree).toHaveBeenCalledWith("root", { + summarize: true, + customInstructions: undefined, + }); + }); + + it("still offers the summary prompt on plain enter when branchSummary.enabled is on", async () => { + Settings.instance.set("branchSummary.enabled", true); + const harness = createHarness("Summarize"); + + harness.controller.showTreeSelector(); + harness.selector().handleInput("\r"); + await harness.navigation; + + expect(harness.showHookSelector).toHaveBeenCalledWith("Summarize branch?", [ + "No summary", + "Summarize", + "Summarize with custom prompt", + ]); + expect(harness.navigateTree).toHaveBeenCalledWith("root", { + summarize: true, + customInstructions: undefined, + }); + }); +});