diff --git a/.github/APPROVED_CONTRIBUTORS b/.github/APPROVED_CONTRIBUTORS index 37dc4fee8..faa1575c1 100644 --- a/.github/APPROVED_CONTRIBUTORS +++ b/.github/APPROVED_CONTRIBUTORS @@ -383,3 +383,5 @@ Mallikarjun-0 pr wutongyuonce pr Terminator666666 pr + +y-nk pr diff --git a/packages/agent/src/agent-loop.ts b/packages/agent/src/agent-loop.ts index 7a9d3516b..193e3faf9 100644 --- a/packages/agent/src/agent-loop.ts +++ b/packages/agent/src/agent-loop.ts @@ -518,6 +518,15 @@ async function executeToolCallsParallel( } finalizedCalls.push(async () => { + if (signal?.aborted) { + const finalized = { + toolCall, + result: createErrorToolResult("Operation aborted"), + isError: true, + } satisfies FinalizedToolCallOutcome; + await emitToolExecutionEnd(finalized, emit); + return finalized; + } const executed = await executePreparedToolCall(preparation, signal, emit); const finalized = await finalizeExecutedToolCall( currentContext, diff --git a/packages/ai/src/api/openai-responses.ts b/packages/ai/src/api/openai-responses.ts index 074b159aa..99838cfbb 100644 --- a/packages/ai/src/api/openai-responses.ts +++ b/packages/ai/src/api/openai-responses.ts @@ -75,6 +75,7 @@ function getCompat(model: Model<"openai-responses">): Required 2) { + return { host: trimmed, port: 0 }; + } + + const colonIndex = trimmed.lastIndexOf(":"); + if (colonIndex !== -1 && colonIndex === trimmed.indexOf(":")) { + const host = trimmed.slice(0, colonIndex); + const port = Number.parseInt(trimmed.slice(colonIndex + 1), 10); + if (!Number.isNaN(port)) { + return { host, port }; + } + } + + return { host: trimmed, port: 0 }; +} + function shouldProxyHostname(hostname: string, port: number, env?: ProviderEnv): boolean { const noProxy = getProxyEnv("no_proxy", env).toLowerCase(); if (!noProxy) { @@ -43,26 +80,38 @@ function shouldProxyHostname(hostname: string, port: number, env?: ProviderEnv): return false; } - return noProxy.split(/[,\s]/).every((proxy) => { - if (!proxy) { + const normalizedTargetHost = stripBrackets(hostname.toLowerCase()); + + return noProxy.split(/[,\s]/).every((entry) => { + const parsed = parseNoProxyEntry(entry); + if (!parsed) { return true; } - const parsedProxy = proxy.match(/^(.+):(\d+)$/); - let proxyHostname = parsedProxy ? parsedProxy[1] : proxy; - const proxyPort = parsedProxy ? Number.parseInt(parsedProxy[2]!, 10) : 0; - if (proxyPort && proxyPort !== port) { + if (parsed.port && parsed.port !== port) { return true; } - if (!/^[.*]/.test(proxyHostname)) { - return hostname !== proxyHostname; + let domain = stripBrackets(parsed.host); + if (domain.startsWith("*.")) { + domain = domain.slice(2); + } else if (domain.startsWith(".") || domain.startsWith("*")) { + domain = domain.slice(1); } - if (proxyHostname.startsWith("*")) { - proxyHostname = proxyHostname.slice(1); + if (!domain) { + return true; } - return !hostname.endsWith(proxyHostname); + + if (normalizedTargetHost === domain) { + return false; + } + + if (normalizedTargetHost.endsWith(`.${domain}`)) { + return false; + } + + return true; }); } @@ -73,7 +122,7 @@ function getProxyForUrl(targetUrl: string | URL, env?: ProviderEnv): string { } const protocol = parsedUrl.protocol.split(":", 1)[0]!; - const hostname = parsedUrl.host.replace(/:\d*$/, ""); + const hostname = stripBrackets(parsedUrl.hostname || parsedUrl.host.replace(/:\d*$/, "")); const port = Number.parseInt(parsedUrl.port, 10) || DEFAULT_PROXY_PORTS[protocol] || 0; if (!shouldProxyHostname(hostname, port, env)) { return ""; diff --git a/packages/ai/test/node-http-proxy.test.ts b/packages/ai/test/node-http-proxy.test.ts index a077a928b..5ed530e2f 100644 --- a/packages/ai/test/node-http-proxy.test.ts +++ b/packages/ai/test/node-http-proxy.test.ts @@ -73,4 +73,23 @@ describe("node HTTP proxy resolution", () => { UNSUPPORTED_PROXY_PROTOCOL_MESSAGE, ); }); + + it("handles subdomain wildcards, IPv6, and ports in NO_PROXY", () => { + resetProxyEnv(); + process.env.HTTPS_PROXY = "http://proxy.example:8080"; + process.env.NO_PROXY = "example.com, .wildcard.org, *.star.net, ::1, [2001:db8::1], 127.0.0.1:8080"; + + expect(resolveHttpProxyUrlForTarget("https://example.com")).toBeUndefined(); + expect(resolveHttpProxyUrlForTarget("https://api.example.com")).toBeUndefined(); + expect(resolveHttpProxyUrlForTarget("https://wildcard.org")).toBeUndefined(); + expect(resolveHttpProxyUrlForTarget("https://api.wildcard.org")).toBeUndefined(); + expect(resolveHttpProxyUrlForTarget("https://star.net")).toBeUndefined(); + expect(resolveHttpProxyUrlForTarget("https://api.star.net")).toBeUndefined(); + expect(resolveHttpProxyUrlForTarget("https://notexample.com")?.toString()).toBe("http://proxy.example:8080/"); + + expect(resolveHttpProxyUrlForTarget("https://[::1]:80")).toBeUndefined(); + expect(resolveHttpProxyUrlForTarget("https://[2001:db8::1]")).toBeUndefined(); + expect(resolveHttpProxyUrlForTarget("https://127.0.0.1:8080")).toBeUndefined(); + expect(resolveHttpProxyUrlForTarget("https://127.0.0.1:3000")?.toString()).toBe("http://proxy.example:8080/"); + }); }); diff --git a/packages/ai/test/openai-responses-compat.test.ts b/packages/ai/test/openai-responses-compat.test.ts index c98b2ed55..7ee204e22 100644 --- a/packages/ai/test/openai-responses-compat.test.ts +++ b/packages/ai/test/openai-responses-compat.test.ts @@ -522,3 +522,78 @@ describe("openai-responses provider defaults", () => { expect(result.usage.cost.total).toBe((model.cost.input + model.cost.output) * multiplier * tokenScale); }); }); + +describe("openai-responses max_output_tokens compat", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("sends max_output_tokens by default", async () => { + let capturedPayload: { max_output_tokens?: number } | undefined; + + vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response("data: [DONE]\n\n", { + status: 200, + headers: { "content-type": "text/event-stream" }, + }), + ); + + const stream = streamOpenAIResponses( + getModel("openai", "gpt-5.4"), + { + systemPrompt: "sys", + messages: [{ role: "user", content: "hi", timestamp: Date.now() }], + }, + { + apiKey: "test-key", + maxTokens: 1024, + onPayload: (payload) => { + capturedPayload = payload as { max_output_tokens?: number }; + }, + }, + ); + + for await (const event of stream) { + if (event.type === "done" || event.type === "error") break; + } + + expect(capturedPayload?.max_output_tokens).toBe(1024); + }); + + it("omits max_output_tokens when supportsMaxOutputTokens is false", async () => { + const baseModel = getModel("openai", "gpt-5.4"); + const model: Model<"openai-responses"> = { + ...baseModel, + compat: { ...baseModel.compat, supportsMaxOutputTokens: false }, + }; + let capturedPayload: { max_output_tokens?: number } | undefined; + + vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response("data: [DONE]\n\n", { + status: 200, + headers: { "content-type": "text/event-stream" }, + }), + ); + + const stream = streamOpenAIResponses( + model, + { + systemPrompt: "sys", + messages: [{ role: "user", content: "hi", timestamp: Date.now() }], + }, + { + apiKey: "test-key", + maxTokens: 1024, + onPayload: (payload) => { + capturedPayload = payload as { max_output_tokens?: number }; + }, + }, + ); + + for await (const event of stream) { + if (event.type === "done" || event.type === "error") break; + } + + expect(capturedPayload?.max_output_tokens).toBeUndefined(); + }); +}); diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 690c38e89..c04b7285d 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -22,6 +22,10 @@ - Added environment variables and advanced settings for overriding auto-detected terminal hyperlink, image, and truecolor capabilities ([#8665](https://github.com/earendil-works/pi/issues/8665)). - Added `fullscreenCopyOnSelect` to disable automatic fullscreen selection copy; when disabled, `Ctrl+X` copies the active text selection before falling back to the last assistant message, while `/tree` still copies the selected message ([#7720](https://github.com/earendil-works/pi/issues/7720)). +### Changed + +- Changed selectors in `/thinking`, `/model`, `/scoped-models`, `/trust`, per-model thinking settings, and theme settings to keep active options marked while browsing. `/scoped-models` now uses consistent per-item toggles and strikes through unavailable models ([#8900](https://github.com/earendil-works/pi/pull/8900)). + ### Fixed - Fixed toggling thinking visibility clearing partial output from running Bash tools ([#8611](https://github.com/earendil-works/pi/issues/8611)). diff --git a/packages/coding-agent/docs/terminal-setup.md b/packages/coding-agent/docs/terminal-setup.md index 29f7320ad..de91e3c5c 100644 --- a/packages/coding-agent/docs/terminal-setup.md +++ b/packages/coding-agent/docs/terminal-setup.md @@ -130,6 +130,21 @@ Add to `keybindings.json`: } ``` +## Zed (Integrated Terminal) + +Add these key bindings to your Zed `keymap.json`: + +```json +{ + "context": "Terminal", + "bindings": { + "shift-enter": ["terminal::SendText", "\u001b[13;2u"], + "ctrl--": ["terminal::SendText", "\u001b[45;5u"], + "ctrl-alt-]": ["terminal::SendText", "\u001b[93;7u"] + } +} +``` + ## Windows Terminal Pi uses Windows-style keybindings when running natively on Windows or in WSL: diff --git a/packages/coding-agent/src/core/agent-session-runtime.ts b/packages/coding-agent/src/core/agent-session-runtime.ts index c3b60569e..d2eb9a3fb 100644 --- a/packages/coding-agent/src/core/agent-session-runtime.ts +++ b/packages/coding-agent/src/core/agent-session-runtime.ts @@ -333,12 +333,12 @@ export class AgentSessionRuntime { } const sessionManager = this.session.sessionManager; + await this.teardownCurrent("fork", sessionManager.getSessionFile()); if (!targetLeafId) { - sessionManager.newSession({ parentSession: this.session.sessionFile }); + sessionManager.newSession({ parentSession: previousSessionFile }); } else { sessionManager.createBranchedSession(targetLeafId); } - await this.teardownCurrent("fork", sessionManager.getSessionFile()); this.apply( await this.createRuntime({ cwd: this.cwd, diff --git a/packages/coding-agent/src/core/model-config.ts b/packages/coding-agent/src/core/model-config.ts index 6036d5af9..4d426649b 100644 --- a/packages/coding-agent/src/core/model-config.ts +++ b/packages/coding-agent/src/core/model-config.ts @@ -121,6 +121,7 @@ const OpenAIResponsesCompatSchema = Type.Object({ supportsOpenAIGrammarTools: Type.Optional(Type.Boolean()), supportsAdditionalTools: Type.Optional(Type.Boolean()), supportsToolSearch: Type.Optional(Type.Boolean()), + supportsMaxOutputTokens: Type.Optional(Type.Boolean()), }); const AnthropicMessagesCompatSchema = Type.Object({ diff --git a/packages/coding-agent/src/core/tools/bash.ts b/packages/coding-agent/src/core/tools/bash.ts index ec94cabd5..d8866ae94 100644 --- a/packages/coding-agent/src/core/tools/bash.ts +++ b/packages/coding-agent/src/core/tools/bash.ts @@ -242,10 +242,16 @@ export function createShellToolDefinition( { command, timeout }: { command: string; timeout?: number }, signal?: AbortSignal, onUpdate?, - ctx?, + ctx?: ExtensionContext, ) { const resolvedCommand = commandPrefix ? `${commandPrefix}\n${command}` : command; - const spawnContext = resolveSpawnContext(resolvedCommand, cwd, spawnHook, exposeSessionEnvironment, ctx); + const spawnContext = resolveSpawnContext( + resolvedCommand, + ctx?.cwd || cwd, + spawnHook, + exposeSessionEnvironment, + ctx, + ); const output = new OutputAccumulator({ tempFilePrefix: config.tempFilePrefix }); let acceptingOutput = true; let updateTimer: NodeJS.Timeout | undefined; diff --git a/packages/coding-agent/src/core/tools/edit.ts b/packages/coding-agent/src/core/tools/edit.ts index 8d4fd9c8a..0fa50f1bb 100644 --- a/packages/coding-agent/src/core/tools/edit.ts +++ b/packages/coding-agent/src/core/tools/edit.ts @@ -4,7 +4,7 @@ import { access as fsAccess, readFile as fsReadFile, writeFile as fsWriteFile } import { type Static, Type } from "typebox"; import { splitBom } from "../../utils/text.ts"; import { getExperimentalToolSampling } from "../experimental.ts"; -import type { ToolDefinition } from "../extensions/types.ts"; +import type { ExtensionContext, ToolDefinition } from "../extensions/types.ts"; import { applyEditsToNormalizedContent, detectLineEnding, @@ -157,9 +157,9 @@ export function createEditToolDefinition( constrainedSampling: getExperimentalToolSampling(), renderShell: "self", prepareArguments: prepareEditArguments, - async execute(_toolCallId, input: EditToolInput, signal?: AbortSignal, _onUpdate?, _ctx?) { + async execute(_toolCallId, input: EditToolInput, signal?: AbortSignal, _onUpdate?, ctx?: ExtensionContext) { const { path, edits } = validateEditInput(input); - const absolutePath = resolveToCwd(path, cwd); + const absolutePath = resolveToCwd(path, ctx?.cwd || cwd); return withFileMutationQueue(absolutePath, async () => { // Do not reject from an abort event listener here: that would release the diff --git a/packages/coding-agent/src/core/tools/find.ts b/packages/coding-agent/src/core/tools/find.ts index 37887a587..8d44c491e 100644 --- a/packages/coding-agent/src/core/tools/find.ts +++ b/packages/coding-agent/src/core/tools/find.ts @@ -4,7 +4,7 @@ import { spawn } from "child_process"; import path from "path"; import { type Static, Type } from "typebox"; import { ensureTool } from "../../utils/tools-manager.ts"; -import type { ToolDefinition } from "../extensions/types.ts"; +import type { ExtensionContext, ToolDefinition } from "../extensions/types.ts"; import { pathExists, resolveToCwd } from "./path-utils.ts"; import { findRenderers } from "./renderers/find.ts"; import { wrapToolDefinition } from "./tool-definition-wrapper.ts"; @@ -83,7 +83,7 @@ export function createFindToolDefinition( { pattern, path: searchDir, limit }: { pattern: string; path?: string; limit?: number }, signal?: AbortSignal, _onUpdate?, - _ctx?, + ctx?: ExtensionContext, ) { return new Promise((resolve, reject) => { if (signal?.aborted) { @@ -108,7 +108,7 @@ export function createFindToolDefinition( (async () => { try { - const searchPath = resolveToCwd(searchDir || ".", cwd); + const searchPath = resolveToCwd(searchDir || ".", ctx?.cwd || cwd); const effectiveLimit = limit ?? DEFAULT_LIMIT; const ops = customOps ?? defaultFindOperations; diff --git a/packages/coding-agent/src/core/tools/grep.ts b/packages/coding-agent/src/core/tools/grep.ts index cc04025cd..44f7e212b 100644 --- a/packages/coding-agent/src/core/tools/grep.ts +++ b/packages/coding-agent/src/core/tools/grep.ts @@ -5,7 +5,7 @@ import { spawn } from "child_process"; import path from "path"; import { type Static, Type } from "typebox"; import { ensureTool } from "../../utils/tools-manager.ts"; -import type { ToolDefinition } from "../extensions/types.ts"; +import type { ExtensionContext, ToolDefinition } from "../extensions/types.ts"; import { resolveToCwd } from "./path-utils.ts"; import { grepRenderers } from "./renderers/grep.ts"; import { wrapToolDefinition } from "./tool-definition-wrapper.ts"; @@ -99,7 +99,7 @@ export function createGrepToolDefinition( }, signal?: AbortSignal, _onUpdate?, - _ctx?, + ctx?: ExtensionContext, ) { return new Promise((resolve, reject) => { if (signal?.aborted) { @@ -122,7 +122,7 @@ export function createGrepToolDefinition( return; } - const searchPath = resolveToCwd(searchDir || ".", cwd); + const searchPath = resolveToCwd(searchDir || ".", ctx?.cwd || cwd); const ops = customOps ?? defaultGrepOperations; let isDirectory: boolean; try { diff --git a/packages/coding-agent/src/core/tools/ls.ts b/packages/coding-agent/src/core/tools/ls.ts index 4d992c383..558ff9f9f 100644 --- a/packages/coding-agent/src/core/tools/ls.ts +++ b/packages/coding-agent/src/core/tools/ls.ts @@ -2,7 +2,7 @@ import { readdir as fsReaddir, stat as fsStat } from "node:fs/promises"; import type { AgentTool } from "@earendil-works/pi-agent-core"; import nodePath from "path"; import { type Static, Type } from "typebox"; -import type { ToolDefinition } from "../extensions/types.ts"; +import type { ExtensionContext, ToolDefinition } from "../extensions/types.ts"; import { pathExists, resolveToCwd } from "./path-utils.ts"; import { lsRenderers } from "./renderers/ls.ts"; import { wrapToolDefinition } from "./tool-definition-wrapper.ts"; @@ -67,7 +67,7 @@ export function createLsToolDefinition( { path, limit }: { path?: string; limit?: number }, signal?: AbortSignal, _onUpdate?, - _ctx?, + ctx?: ExtensionContext, ) { return new Promise((resolve, reject) => { if (signal?.aborted) { @@ -80,7 +80,7 @@ export function createLsToolDefinition( (async () => { try { - const dirPath = resolveToCwd(path || ".", cwd); + const dirPath = resolveToCwd(path || ".", ctx?.cwd || cwd); const effectiveLimit = limit ?? DEFAULT_LIMIT; // Check if path exists. diff --git a/packages/coding-agent/src/core/tools/read.ts b/packages/coding-agent/src/core/tools/read.ts index c622d397e..546cf46fc 100644 --- a/packages/coding-agent/src/core/tools/read.ts +++ b/packages/coding-agent/src/core/tools/read.ts @@ -6,7 +6,7 @@ import { type Static, Type } from "typebox"; import { processImage } from "../../utils/image-process.ts"; import { detectSupportedImageMimeTypeFromFile } from "../../utils/mime.ts"; import { getExperimentalToolSampling } from "../experimental.ts"; -import type { ToolDefinition } from "../extensions/types.ts"; +import type { ExtensionContext, ToolDefinition } from "../extensions/types.ts"; import { resolveReadPathAsync } from "./path-utils.ts"; import { readRenderers } from "./renderers/read.ts"; import { wrapToolDefinition } from "./tool-definition-wrapper.ts"; @@ -81,7 +81,7 @@ export function createReadToolDefinition( { path, offset, limit }: { path: string; offset?: number; limit?: number }, signal?: AbortSignal, _onUpdate?, - ctx?, + ctx?: ExtensionContext, ) { return new Promise<{ content: (TextContent | ImageContent)[]; details: ReadToolDetails | undefined }>( (resolve, reject) => { @@ -98,7 +98,7 @@ export function createReadToolDefinition( (async () => { try { - const absolutePath = await resolveReadPathAsync(path, cwd); + const absolutePath = await resolveReadPathAsync(path, ctx?.cwd || cwd); if (aborted) return; // Check if file exists and is readable. await ops.access(absolutePath); diff --git a/packages/coding-agent/src/core/tools/write.ts b/packages/coding-agent/src/core/tools/write.ts index d8e136d89..396227a06 100644 --- a/packages/coding-agent/src/core/tools/write.ts +++ b/packages/coding-agent/src/core/tools/write.ts @@ -3,7 +3,7 @@ import { mkdir as fsMkdir, writeFile as fsWriteFile } from "fs/promises"; import { dirname } from "path"; import { type Static, Type } from "typebox"; import { getExperimentalToolSampling } from "../experimental.ts"; -import type { ToolDefinition } from "../extensions/types.ts"; +import type { ExtensionContext, ToolDefinition } from "../extensions/types.ts"; import { withFileMutationQueue } from "./file-mutation-queue.ts"; import { resolveToCwd } from "./path-utils.ts"; import { writeRenderers } from "./renderers/write.ts"; @@ -61,9 +61,9 @@ export function createWriteToolDefinition( { path, content }: { path: string; content: string }, signal?: AbortSignal, _onUpdate?, - _ctx?, + ctx?: ExtensionContext, ) { - const absolutePath = resolveToCwd(path, cwd); + const absolutePath = resolveToCwd(path, ctx?.cwd || cwd); const dir = dirname(absolutePath); return withFileMutationQueue(absolutePath, async () => { // Do not reject from an abort event listener here: that would release the diff --git a/packages/coding-agent/src/modes/interactive/components/model-selector.ts b/packages/coding-agent/src/modes/interactive/components/model-selector.ts index d1f7788b1..96dd81d7b 100644 --- a/packages/coding-agent/src/modes/interactive/components/model-selector.ts +++ b/packages/coding-agent/src/modes/interactive/components/model-selector.ts @@ -316,19 +316,11 @@ export class ModelSelectorComponent extends Container implements Focusable { const isDefault = this.isDefaultModel(item.model); const defaultBadge = isDefault ? theme.fg("muted", " · default") : ""; - let line = ""; - if (isSelected) { - const prefix = theme.fg("accent", "→ "); - const modelText = `${item.id}`; - const providerBadge = theme.fg("muted", `[${item.provider}]`); - const checkmark = isCurrent ? theme.fg("success", " ✓") : ""; - line = `${prefix + theme.fg("accent", modelText)} ${providerBadge}${defaultBadge}${checkmark}`; - } else { - const modelText = ` ${item.id}`; - const providerBadge = theme.fg("muted", `[${item.provider}]`); - const checkmark = isCurrent ? theme.fg("success", " ✓") : ""; - line = `${modelText} ${providerBadge}${defaultBadge}${checkmark}`; - } + const cursor = isSelected ? theme.fg("accent", "→ ") : " "; + const currentMarker = isCurrent ? theme.fg("accent", "✓ ") : " "; + const modelText = isSelected ? theme.fg("accent", item.id) : item.id; + const providerBadge = theme.fg("muted", `[${item.provider}]`); + const line = `${cursor}${currentMarker}${modelText} ${providerBadge}${defaultBadge}`; this.listContainer.addChild(new Text(line, 0, 0)); } diff --git a/packages/coding-agent/src/modes/interactive/components/scoped-models-selector.ts b/packages/coding-agent/src/modes/interactive/components/scoped-models-selector.ts index e7dfc2714..2ab8e4e06 100644 --- a/packages/coding-agent/src/modes/interactive/components/scoped-models-selector.ts +++ b/packages/coding-agent/src/modes/interactive/components/scoped-models-selector.ts @@ -22,11 +22,16 @@ function isEnabled(enabledIds: EnabledIds, id: string): boolean { return enabledIds === null || enabledIds.includes(id); } -function toggle(enabledIds: EnabledIds, id: string): EnabledIds { - if (enabledIds === null) return [id]; // First toggle: start with only this one +/** Collapse an explicit list back to null (= all enabled) when it covers every available model. */ +function normalizeEnabled(result: string[], allIds: string[]): EnabledIds { + return result.length === allIds.length && result.every((id) => allIds.includes(id)) ? null : result; +} + +function toggle(enabledIds: EnabledIds, allIds: string[], id: string): EnabledIds { + if (enabledIds === null) return allIds.filter((modelId) => modelId !== id); const index = enabledIds.indexOf(id); if (index >= 0) return [...enabledIds.slice(0, index), ...enabledIds.slice(index + 1)]; - return [...enabledIds, id]; + return normalizeEnabled([...enabledIds, id], allIds); } function enableAll(enabledIds: EnabledIds, allIds: string[], targetIds?: string[]): EnabledIds { @@ -36,7 +41,7 @@ function enableAll(enabledIds: EnabledIds, allIds: string[], targetIds?: string[ for (const id of targets) { if (!result.includes(id)) result.push(id); } - return result.length === allIds.length && result.every((id) => allIds.includes(id)) ? null : result; + return normalizeEnabled(result, allIds); } function clearAll(enabledIds: EnabledIds, allIds: string[], targetIds?: string[]): EnabledIds { @@ -240,23 +245,16 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl Math.min(this.selectedIndex - Math.floor(this.maxVisible / 2), this.filteredItems.length - this.maxVisible), ); const endIndex = Math.min(startIndex + this.maxVisible, this.filteredItems.length); - const allEnabled = this.enabledIds === null; - for (let i = startIndex; i < endIndex; i++) { const item = this.filteredItems[i]!; const isSelected = i === this.selectedIndex; const prefix = isSelected ? theme.fg("accent", "→ ") : " "; const id = item.model?.id ?? item.fullId; - const modelText = isSelected ? theme.fg("accent", id) : id; + const styledId = item.model ? id : theme.strikethrough(id); + const modelText = isSelected ? theme.fg("accent", styledId) : styledId; const providerBadge = theme.fg("muted", item.model ? ` [${item.model.provider}]` : " [unavailable]"); - const status = item.model - ? allEnabled - ? "" - : item.enabled - ? theme.fg("success", " ✓") - : theme.fg("dim", " ✗") - : theme.fg("dim", " ✗"); - this.listContainer.addChild(new Text(`${prefix}${modelText}${providerBadge}${status}`, 0, 0)); + const status = item.model && item.enabled ? theme.fg("accent", "✓ ") : " "; + this.listContainer.addChild(new Text(`${prefix}${status}${modelText}${providerBadge}`, 0, 0)); } // Add scroll indicator if needed @@ -322,7 +320,7 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl if (kb.matches(data, "tui.select.confirm")) { const item = this.filteredItems[this.selectedIndex]; if (item) { - this.enabledIds = toggle(this.enabledIds, item.fullId); + this.enabledIds = toggle(this.enabledIds, this.allIds, item.fullId); this.isDirty = true; this.refresh(); this.notifyChange(); diff --git a/packages/coding-agent/src/modes/interactive/components/settings-selector.ts b/packages/coding-agent/src/modes/interactive/components/settings-selector.ts index 308bf8109..cd0e8e869 100644 --- a/packages/coding-agent/src/modes/interactive/components/settings-selector.ts +++ b/packages/coding-agent/src/modes/interactive/components/settings-selector.ts @@ -191,20 +191,23 @@ function modelItemLabel(model: Model): string { return `${model.id} ${theme.fg("muted", `[${model.provider}]`)}`; } -function themeItems(availableThemes: string[]): SelectItem[] { - return availableThemes.map((name) => ({ value: name, label: name })); +function themeItems(availableThemes: string[], currentTheme: string): SelectItem[] { + return availableThemes.map((name) => ({ + value: name, + label: `${name === currentTheme ? "✓ " : " "}${name}`, + })); } const AUTOMATIC_THEME_VALUE = "/"; -function singleModeThemeItems(availableThemes: string[]): SelectItem[] { +function singleModeThemeItems(availableThemes: string[], currentTheme: string): SelectItem[] { return [ { value: AUTOMATIC_THEME_VALUE, - label: "Automatic", + label: " Automatic", description: "Use separate themes for light and dark terminal appearance", }, - ...themeItems(availableThemes), + ...themeItems(availableThemes, currentTheme), ]; } @@ -285,7 +288,7 @@ class ThemeSubmenu extends Container { const menu = new SelectSubmenu( "Theme", "Select a theme, or choose Automatic to follow terminal appearance.", - singleModeThemeItems(this.availableThemes), + singleModeThemeItems(this.availableThemes, this.singleTheme), this.singleTheme, (value) => { if (value === AUTOMATIC_THEME_VALUE) { @@ -401,7 +404,7 @@ class ThemeSubmenu extends Container { return new SelectSubmenu( title, description, - themeItems(this.availableThemes), + themeItems(this.availableThemes, currentValue), currentValue, onSelect, () => { @@ -626,15 +629,16 @@ export class SettingsSelectorComponent extends Container { const levels = ( model.reasoning ? getSupportedThinkingLevels(model) : ["off"] ) as ThinkingLevel[]; + const activeLevel = currentModelThinkingLevels[ctx.model]; const items: SelectItem[] = levels.map((level) => ({ value: level, - label: level, + label: `${level === activeLevel ? "✓ " : " "}${level}`, description: THINKING_DESCRIPTIONS[level], })); if (currentModelThinkingLevels[ctx.model] !== undefined) { items.push({ value: CLEAR_OVERRIDE_VALUE, - label: "(clear override)", + label: " (clear override)", description: `Revert to global default (${config.thinkingLevel})`, }); } diff --git a/packages/coding-agent/src/modes/interactive/components/thinking-selector.ts b/packages/coding-agent/src/modes/interactive/components/thinking-selector.ts index c67ccc990..1fbf6e0b2 100644 --- a/packages/coding-agent/src/modes/interactive/components/thinking-selector.ts +++ b/packages/coding-agent/src/modes/interactive/components/thinking-selector.ts @@ -68,7 +68,7 @@ export class ThinkingSelectorComponent extends Container implements Focusable { this.allItems = availableLevels.map((level) => ({ value: level, - label: level, + label: `${level === currentLevel ? "✓ " : " "}${level}`, description: level === defaultThinkingLevel ? `${LEVEL_DESCRIPTIONS[level]} · default` : LEVEL_DESCRIPTIONS[level], })); @@ -110,7 +110,7 @@ export class ThinkingSelectorComponent extends Container implements Focusable { private applyFilter(query: string): void { const filtered = query - ? fuzzyFilter(this.allItems, query, (item) => `${item.label} ${item.description ?? ""}`) + ? fuzzyFilter(this.allItems, query, (item) => `${item.value} ${item.description ?? ""}`) : this.allItems; const selectedValue = this.selectList.getSelectedItem()?.value as ThinkingLevel | undefined; const newList = this.buildSelectList(filtered, selectedValue); diff --git a/packages/coding-agent/src/modes/interactive/components/trust-selector.ts b/packages/coding-agent/src/modes/interactive/components/trust-selector.ts index 92c232889..00bc9e067 100644 --- a/packages/coding-agent/src/modes/interactive/components/trust-selector.ts +++ b/packages/coding-agent/src/modes/interactive/components/trust-selector.ts @@ -107,10 +107,10 @@ export class TrustSelectorComponent extends Container { const isSelected = i === this.selectedIndex; const isCurrent = this.isSavedOption(option); - const checkmark = isCurrent ? theme.fg("success", " ✓") : ""; + const currentMarker = isCurrent ? theme.fg("accent", "✓ ") : " "; const prefix = isSelected ? theme.fg("accent", "→ ") : " "; const label = isSelected ? theme.fg("accent", option.label) : theme.fg("text", option.label); - this.listContainer.addChild(new Text(`${prefix}${label}${checkmark}`, 1, 0)); + this.listContainer.addChild(new Text(`${prefix}${currentMarker}${label}`, 1, 0)); } } diff --git a/packages/coding-agent/test/model-selector.test.ts b/packages/coding-agent/test/model-selector.test.ts index fd5550ac1..f25e9b103 100644 --- a/packages/coding-agent/test/model-selector.test.ts +++ b/packages/coding-agent/test/model-selector.test.ts @@ -1,5 +1,6 @@ -import type { TUI } from "@earendil-works/pi-tui"; -import { afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import { setKeybindings, type TUI } from "@earendil-works/pi-tui"; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { KeybindingsManager } from "../src/core/keybindings.ts"; import { ModelSelectorComponent } from "../src/modes/interactive/components/model-selector.ts"; import { initTheme } from "../src/modes/interactive/theme/theme.ts"; import { stripAnsi } from "../src/utils/ansi.ts"; @@ -16,11 +17,45 @@ describe("model selector", () => { initTheme("dark"); }); + beforeEach(() => { + setKeybindings(new KeybindingsManager()); + }); + afterEach(() => { harness?.cleanup(); harness = undefined; }); + it("keeps the current model marked while browsing", async () => { + harness = await createHarness({ + models: [ + { id: "current-model", name: "Current Model", reasoning: true }, + { id: "browsed-model", name: "Browsed Model", reasoning: true }, + ], + }); + const currentModel = harness.getModel("current-model")!; + const selector = new ModelSelectorComponent( + createFakeTui(), + currentModel, + harness.session.modelRuntime, + [], + () => {}, + () => {}, + ); + + const getModelRow = (id: string): string | undefined => + stripAnsi(selector.render(120).join("\n")) + .split("\n") + .find((line) => line.includes(`${id} [`)) + ?.trimEnd(); + + expect(getModelRow("current-model")).toBe(`→ ✓ current-model [${currentModel.provider}]`); + selector.handleInput("\x1b[B"); + expect(getModelRow("current-model")).toBe(` ✓ current-model [${currentModel.provider}]`); + expect(getModelRow("browsed-model")).toBe(`→ browsed-model [${currentModel.provider}]`); + selector.dispose(); + }); + it("lists every catalog that failed to refresh", async () => { harness = await createHarness(); vi.spyOn(harness.session.modelRuntime, "refresh").mockResolvedValue({ diff --git a/packages/coding-agent/test/scoped-models-selector.test.ts b/packages/coding-agent/test/scoped-models-selector.test.ts new file mode 100644 index 000000000..80c22cfa5 --- /dev/null +++ b/packages/coding-agent/test/scoped-models-selector.test.ts @@ -0,0 +1,126 @@ +import { setKeybindings } from "@earendil-works/pi-tui"; +import { afterEach, beforeAll, beforeEach, describe, expect, it } from "vitest"; +import { KeybindingsManager } from "../src/core/keybindings.ts"; +import { ScopedModelsSelectorComponent } from "../src/modes/interactive/components/scoped-models-selector.ts"; +import { initTheme } from "../src/modes/interactive/theme/theme.ts"; +import { stripAnsi } from "../src/utils/ansi.ts"; +import { createHarness, type Harness } from "./suite/harness.ts"; + +interface ModelState { + id: string; + name: string; + enabled: boolean; +} + +function getMarkerStates(selector: ScopedModelsSelectorComponent, models: ModelState[]): boolean[] { + const lines = stripAnsi(selector.render(120).join("\n")).split("\n"); + return models.map((model) => { + const line = lines.find((candidate) => candidate.includes(`${model.id} [`)); + if (!line) throw new Error(`Expected rendered row for ${model.id}`); + return line.slice(2).startsWith("✓ "); + }); +} + +describe("scoped models selector", () => { + let harness: Harness | undefined; + + beforeAll(() => { + initTheme("dark"); + }); + + beforeEach(() => { + setKeybindings(new KeybindingsManager()); + }); + + afterEach(() => { + harness?.cleanup(); + harness = undefined; + }); + + async function createSelector(models: ModelState[]) { + harness = await createHarness({ models }); + const provider = harness.models[0].provider; + const selector = new ScopedModelsSelectorComponent( + { + allModels: [...harness.models], + enabledModelIds: models.filter((model) => model.enabled).map((model) => `${provider}/${model.id}`), + }, + { + onChange: (enabledModelIds) => { + for (const model of models) { + model.enabled = enabledModelIds === null || enabledModelIds.includes(`${provider}/${model.id}`); + } + }, + onPersist: () => {}, + onCancel: () => {}, + }, + ); + return selector; + } + + it("marks every model after enabling all", async () => { + const models = [ + { id: "model-a", name: "Model A", enabled: true }, + { id: "model-b", name: "Model B", enabled: false }, + { id: "model-c", name: "Model C", enabled: false }, + ]; + const selector = await createSelector(models); + + selector.handleInput("\x01"); + + expect(models.map((model) => model.enabled)).toEqual([true, true, true]); + expect(getMarkerStates(selector, models)).toEqual([true, true, true]); + expect(stripAnsi(selector.render(120).join("\n"))).toContain("all enabled"); + }); + + it("disables only the selected model after enabling all", async () => { + const models = [ + { id: "model-a", name: "Model A", enabled: true }, + { id: "model-b", name: "Model B", enabled: false }, + { id: "model-c", name: "Model C", enabled: false }, + ]; + const selector = await createSelector(models); + + selector.handleInput("\x01"); + selector.handleInput("\r"); + + expect(models.map((model) => model.enabled)).toEqual([false, true, true]); + expect(getMarkerStates(selector, models)).toEqual([false, true, true]); + }); + + it("enables only the selected model after clearing all", async () => { + const models = [ + { id: "model-a", name: "Model A", enabled: true }, + { id: "model-b", name: "Model B", enabled: true }, + { id: "model-c", name: "Model C", enabled: true }, + ]; + const selector = await createSelector(models); + + selector.handleInput("\x18"); + expect(models.map((model) => model.enabled)).toEqual([false, false, false]); + expect(getMarkerStates(selector, models)).toEqual([false, false, false]); + + selector.handleInput("\r"); + expect(models.map((model) => model.enabled)).toEqual([true, false, false]); + expect(getMarkerStates(selector, models)).toEqual([true, false, false]); + }); + + it("restores the all-enabled state after re-enabling the last disabled model", async () => { + const models = [ + { id: "model-a", name: "Model A", enabled: true }, + { id: "model-b", name: "Model B", enabled: false }, + { id: "model-c", name: "Model C", enabled: false }, + ]; + const selector = await createSelector(models); + + selector.handleInput("\x01"); // enable all -> null + selector.handleInput("\r"); // disable model-a; enabled models re-sort first: [b, c, a] + selector.handleInput("\x1b[B"); + selector.handleInput("\x1b[B"); // move selection back to model-a + selector.handleInput("\r"); // re-enable model-a + + expect(models.map((model) => model.enabled)).toEqual([true, true, true]); + expect(getMarkerStates(selector, models)).toEqual([true, true, true]); + expect(stripAnsi(selector.render(120).join("\n"))).toContain("all enabled"); + }); +}); diff --git a/packages/coding-agent/test/settings-selector.test.ts b/packages/coding-agent/test/settings-selector.test.ts index c93178936..ffe201d88 100644 --- a/packages/coding-agent/test/settings-selector.test.ts +++ b/packages/coding-agent/test/settings-selector.test.ts @@ -1,5 +1,5 @@ import { setKeybindings } from "@earendil-works/pi-tui"; -import { beforeAll, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { KeybindingsManager } from "../src/core/keybindings.ts"; import { type SettingsCallbacks, @@ -7,13 +7,21 @@ import { SettingsSelectorComponent, } from "../src/modes/interactive/components/settings-selector.ts"; import { initTheme } from "../src/modes/interactive/theme/theme.ts"; +import { stripAnsi } from "../src/utils/ansi.ts"; +import { createHarness, type Harness } from "./suite/harness.ts"; describe("SettingsSelectorComponent", () => { + let harness: Harness | undefined; beforeAll(() => { initTheme("dark"); setKeybindings(new KeybindingsManager()); }); + afterEach(() => { + harness?.cleanup(); + harness = undefined; + }); + it("cycles through fullscreen settings", () => { const onExitOutputChange = vi.fn(); const onScrollbarChange = vi.fn(); @@ -48,4 +56,83 @@ describe("SettingsSelectorComponent", () => { cycle("Fullscreen copy on select", 2); expect(onCopyOnSelectChange.mock.calls.flat()).toEqual([false, true]); }); + + it("keeps the configured fixed theme marked while browsing", () => { + const config = { + defaultModel: "not set", + availableDefaultModels: [], + modelThinkingLevels: {}, + currentTheme: "dark", + terminalTheme: "dark", + availableThemes: ["dark", "light"], + warnings: {}, + } as unknown as SettingsConfig; + const callbacks = { onThemePreview: vi.fn(), onCancel: () => {} } as unknown as SettingsCallbacks; + const list = new SettingsSelectorComponent(config, callbacks).getSettingsList(); + + list.selectItem("theme"); + list.handleInput("\r"); + let output = stripAnsi(list.render(120).join("\n")); + expect(output).toContain(" Automatic"); + expect(output).toContain("→ ✓ dark"); + + list.handleInput("\x1b[B"); + output = stripAnsi(list.render(120).join("\n")); + expect(output).toContain(" ✓ dark"); + expect(output).toContain("→ light"); + }); + + it("keeps a configured automatic theme marked while browsing", () => { + const config = { + defaultModel: "not set", + availableDefaultModels: [], + modelThinkingLevels: {}, + currentTheme: "light/dark", + terminalTheme: "dark", + availableThemes: ["dark", "light", "other"], + warnings: {}, + } as unknown as SettingsConfig; + const callbacks = { onThemePreview: vi.fn(), onCancel: () => {} } as unknown as SettingsCallbacks; + const list = new SettingsSelectorComponent(config, callbacks).getSettingsList(); + + list.selectItem("theme"); + list.handleInput("\r"); + list.handleInput("\r"); + let output = stripAnsi(list.render(120).join("\n")); + expect(output).toContain("→ ✓ light"); + + list.handleInput("\x1b[B"); + output = stripAnsi(list.render(120).join("\n")); + expect(output).toContain(" ✓ light"); + expect(output).toContain("→ other"); + }); + + it("keeps the configured per-model thinking level marked while browsing", async () => { + harness = await createHarness({ + models: [{ id: "thinking-model", reasoning: true }], + }); + const model = harness.getModel("thinking-model")!; + const modelKey = `${model.provider}/${model.id}`; + const config = { + defaultModel: modelKey, + availableDefaultModels: [model], + thinkingLevel: "high", + modelThinkingLevels: { [modelKey]: "medium" }, + } as unknown as SettingsConfig; + const callbacks = { onCancel: () => {} } as unknown as SettingsCallbacks; + const list = new SettingsSelectorComponent(config, callbacks).getSettingsList(); + + list.selectItem("model-thinking"); + list.handleInput("\r"); + list.handleInput("\r"); + + let output = stripAnsi(list.render(120).join("\n")); + expect(output).toContain("→ ✓ medium"); + expect(output).toContain(" (clear override)"); + + list.handleInput("\x1b[B"); + output = stripAnsi(list.render(120).join("\n")); + expect(output).toContain(" ✓ medium"); + expect(output).toContain("→ high"); + }); }); diff --git a/packages/coding-agent/test/suite/regressions/3217-scoped-model-order.test.ts b/packages/coding-agent/test/suite/regressions/3217-scoped-model-order.test.ts index e9f33f90c..6553e8fad 100644 --- a/packages/coding-agent/test/suite/regressions/3217-scoped-model-order.test.ts +++ b/packages/coding-agent/test/suite/regressions/3217-scoped-model-order.test.ts @@ -95,7 +95,7 @@ describe("issue #3217 scoped model ordering", () => { .filter((line) => line.includes(`[${modelOne.provider}]`)); const orderedIds = renderedLines.slice(0, 3).map((line) => { const [modelId] = line.trim().replace(/^→\s*/, "").split(" ["); - return modelId?.trim() ?? ""; + return modelId?.replace(/^✓\s*/, "").trim() ?? ""; }); expect(orderedIds).toEqual([modelTwo.id, modelOne.id, modelThree.id]); diff --git a/packages/coding-agent/test/suite/regressions/6949-unavailable-scoped-model.test.ts b/packages/coding-agent/test/suite/regressions/6949-unavailable-scoped-model.test.ts index a2630b300..3597d4030 100644 --- a/packages/coding-agent/test/suite/regressions/6949-unavailable-scoped-model.test.ts +++ b/packages/coding-agent/test/suite/regressions/6949-unavailable-scoped-model.test.ts @@ -4,7 +4,7 @@ import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vite import { KeybindingsManager } from "../../../src/core/keybindings.ts"; import { ScopedModelsSelectorComponent } from "../../../src/modes/interactive/components/scoped-models-selector.ts"; import { InteractiveMode } from "../../../src/modes/interactive/interactive-mode.ts"; -import { initTheme } from "../../../src/modes/interactive/theme/theme.ts"; +import { initTheme, theme } from "../../../src/modes/interactive/theme/theme.ts"; import { stripAnsi } from "../../../src/utils/ansi.ts"; import { createHarness, type Harness } from "../harness.ts"; @@ -82,7 +82,9 @@ describe("issue #6949 unavailable scoped models", () => { }, ); - expect(stripAnsi(selector.render(100).join("\n"))).toContain(`${unavailableId} [unavailable] ✗`); + const rendered = selector.render(100).join("\n"); + expect(stripAnsi(rendered)).toContain(`${unavailableId} [unavailable]`); + expect(rendered).toContain(theme.strikethrough(unavailableId)); selector.handleInput("\r"); expect(changes).toEqual([[availableId]]); selector.handleInput("\x13"); @@ -104,7 +106,7 @@ describe("issue #6949 unavailable scoped models", () => { if (!selector) throw new Error("Expected scoped-model selector to open"); const rendered = stripAnsi(selector.render(100).join("\n")); for (const unavailableId of unavailableIds) { - expect(rendered).toContain(`${unavailableId} [unavailable] ✗`); + expect(rendered).toContain(`${unavailableId} [unavailable]`); } expect(getAvailableSnapshot).toHaveBeenCalled(); }); @@ -124,7 +126,7 @@ describe("issue #6949 unavailable scoped models", () => { const selector = getSelector(); if (!selector) throw new Error("Expected scoped-model selector to open"); - expect(stripAnsi(selector.render(100).join("\n"))).toContain(`${fullId} [unavailable] ✗`); + expect(stripAnsi(selector.render(100).join("\n"))).toContain(`${fullId} [unavailable]`); }); it("does not clear a partial scope when an enabled model is unavailable", async () => { diff --git a/packages/coding-agent/test/suite/regressions/7209-model-selector-filter-resets-selection.test.ts b/packages/coding-agent/test/suite/regressions/7209-model-selector-filter-resets-selection.test.ts index e853ac35a..50b49be54 100644 --- a/packages/coding-agent/test/suite/regressions/7209-model-selector-filter-resets-selection.test.ts +++ b/packages/coding-agent/test/suite/regressions/7209-model-selector-filter-resets-selection.test.ts @@ -15,7 +15,7 @@ function selectedModelId(rendered: string): string | undefined { const line = rendered.split("\n").find((l) => l.startsWith("→ ")); if (!line) return undefined; const rest = line.replace(/^→\s*/, ""); - const id = rest.split(" [")[0]; + const id = rest.split(" [")[0]?.replace(/^✓\s*/, ""); return id?.trim() || undefined; } diff --git a/packages/coding-agent/test/suite/regressions/8724-in-memory-fork-active-tool.test.ts b/packages/coding-agent/test/suite/regressions/8724-in-memory-fork-active-tool.test.ts new file mode 100644 index 000000000..0868b49a0 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/8724-in-memory-fork-active-tool.test.ts @@ -0,0 +1,100 @@ +import type { AgentTool, AgentToolResult } from "@earendil-works/pi-agent-core"; +import { fauxAssistantMessage, fauxToolCall } from "@earendil-works/pi-ai"; +import { Type } from "typebox"; +import { afterEach, describe, expect, it } from "vitest"; +import { + AgentSessionRuntime, + type AgentSessionServices, + type CreateAgentSessionRuntimeFactory, + createAgentSessionFromServices, +} from "../../../src/core/agent-session-runtime.ts"; +import { createHarness } from "../harness.ts"; + +describe("regression #8724: in-memory fork during an active tool turn", () => { + const cleanups: Array<() => Promise | void> = []; + + afterEach(async () => { + while (cleanups.length > 0) { + await cleanups.pop()?.(); + } + }); + + it("does not append the aborted turn to the replacement session", async () => { + let markToolStarted = () => {}; + const toolStarted = new Promise((resolve) => { + markToolStarted = resolve; + }); + const blockingTool: AgentTool = { + name: "block", + label: "Block", + description: "Wait until aborted", + parameters: Type.Object({}), + execute: (_toolCallId, _params, signal) => + new Promise>((resolve) => { + markToolStarted(); + signal?.addEventListener( + "abort", + () => resolve({ content: [{ type: "text", text: "tool aborted" }], details: {} }), + { once: true }, + ); + }), + }; + const harness = await createHarness({ tools: [blockingTool] }); + const services: AgentSessionServices = { + cwd: harness.tempDir, + agentDir: harness.tempDir, + modelRuntime: harness.session.modelRuntime, + settingsManager: harness.settingsManager, + resourceLoader: harness.session.resourceLoader, + diagnostics: [], + }; + const createRuntime: CreateAgentSessionRuntimeFactory = async ({ sessionManager, sessionStartEvent }) => ({ + ...(await createAgentSessionFromServices({ + services, + sessionManager, + sessionStartEvent, + model: harness.getModel(), + noTools: "all", + })), + services, + diagnostics: [], + }); + const runtime = new AgentSessionRuntime(harness.session, services, createRuntime); + cleanups.push(async () => { + if (runtime.session !== harness.session) { + await runtime.dispose(); + } + harness.cleanup(); + }); + + harness.setResponses([ + fauxAssistantMessage("first response"), + fauxAssistantMessage(fauxToolCall("block", {}), { stopReason: "toolUse" }), + fauxAssistantMessage("unused after abort"), + ]); + await runtime.session.prompt("first prompt"); + const firstUserEntryId = runtime.session.getUserMessagesForForking()[0]?.entryId; + expect(firstUserEntryId).toBeDefined(); + + const outgoingPrompt = runtime.session.prompt("start blocking tool"); + await toolStarted; + const forkResult = await runtime.fork(firstUserEntryId!); + await outgoingPrompt; + await runtime.session.bindExtensions({}); + + expect(forkResult).toEqual({ cancelled: false, selectedText: "first prompt" }); + expect(runtime.session.messages).toEqual([]); + expect(runtime.session.sessionManager.getEntries().filter((entry) => entry.type === "message")).toEqual([]); + + let capturedRoles: string[] = []; + harness.setResponses([ + (context) => { + capturedRoles = context.messages.map((message) => message.role); + return fauxAssistantMessage("next response"); + }, + ]); + await runtime.session.prompt("next prompt"); + + expect(capturedRoles).toEqual(["user"]); + }); +}); diff --git a/packages/coding-agent/test/suite/regressions/8935-parallel-preflight-abort.test.ts b/packages/coding-agent/test/suite/regressions/8935-parallel-preflight-abort.test.ts new file mode 100644 index 000000000..da10cf815 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/8935-parallel-preflight-abort.test.ts @@ -0,0 +1,72 @@ +import type { AgentTool } from "@earendil-works/pi-agent-core"; +import { fauxAssistantMessage, fauxToolCall } from "@earendil-works/pi-ai"; +import { Type } from "typebox"; +import { afterEach, describe, expect, it } from "vitest"; +import { createHarness, getMessageText, type Harness } from "../harness.ts"; + +describe("#8935 parallel preflight abort", () => { + const harnesses: Harness[] = []; + + afterEach(() => { + while (harnesses.length > 0) { + harnesses.pop()?.cleanup(); + } + }); + + it("does not start prepared tools after a later preflight aborts", async () => { + const executions: string[] = []; + const preflights: string[] = []; + const resultHooks: string[] = []; + const externalWrite: AgentTool = { + name: "external_write", + label: "External write", + description: "Perform an external write", + parameters: Type.Object({ value: Type.String() }), + execute: async (_toolCallId, params) => { + const value = + typeof params === "object" && params !== null && "value" in params ? String(params.value) : ""; + executions.push(value); + return { content: [{ type: "text", text: value }], details: { value } }; + }, + }; + const harness = await createHarness({ + tools: [externalWrite], + extensionFactories: [ + (pi) => { + pi.on("tool_call", async (event, ctx) => { + const value = "value" in event.input ? String(event.input.value) : ""; + preflights.push(value); + if (value === "second") ctx.abort(); + }); + pi.on("tool_result", async (event) => { + resultHooks.push(event.toolCallId); + }); + }, + ], + }); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage( + [fauxToolCall("external_write", { value: "first" }), fauxToolCall("external_write", { value: "second" })], + { stopReason: "toolUse" }, + ), + ]); + + await harness.session.prompt("run both writes"); + + expect(preflights).toEqual(["first", "second"]); + expect(executions).toEqual([]); + expect(resultHooks).toEqual([]); + + const starts = harness.eventsOfType("tool_execution_start"); + const ends = harness.eventsOfType("tool_execution_end"); + expect(starts).toHaveLength(2); + expect(ends).toHaveLength(2); + expect(new Set(ends.map((event) => event.toolCallId))).toEqual(new Set(starts.map((event) => event.toolCallId))); + expect(ends.every((event) => event.isError)).toBe(true); + + const toolResults = harness.session.messages.filter((message) => message.role === "toolResult"); + expect(toolResults.map((message) => message.toolCallId)).toEqual(starts.map((event) => event.toolCallId)); + expect(toolResults.map(getMessageText)).toEqual(["Operation aborted", "Operation aborted"]); + }); +}); diff --git a/packages/coding-agent/test/thinking-selector.test.ts b/packages/coding-agent/test/thinking-selector.test.ts new file mode 100644 index 000000000..a8b41bd96 --- /dev/null +++ b/packages/coding-agent/test/thinking-selector.test.ts @@ -0,0 +1,37 @@ +import { setKeybindings } from "@earendil-works/pi-tui"; +import { beforeAll, beforeEach, describe, expect, it } from "vitest"; +import { KeybindingsManager } from "../src/core/keybindings.ts"; +import { ThinkingSelectorComponent } from "../src/modes/interactive/components/thinking-selector.ts"; +import { initTheme } from "../src/modes/interactive/theme/theme.ts"; +import { stripAnsi } from "../src/utils/ansi.ts"; + +describe("thinking selector", () => { + beforeAll(() => { + initTheme("dark"); + }); + + beforeEach(() => { + setKeybindings(new KeybindingsManager()); + }); + + it("keeps the current thinking level marked while browsing", () => { + const selector = new ThinkingSelectorComponent( + "medium", + ["medium", "high"], + () => {}, + () => {}, + ); + const getLevelRow = (level: string): string | undefined => + selector + .getSelectList() + .render(80) + .map((line) => stripAnsi(line)) + .find((line) => line.includes(level)); + + expect(selector.getSelectList().getSelectedItem()?.label).toBe("✓ medium"); + expect(getLevelRow("medium")?.startsWith("→ ✓ medium")).toBe(true); + selector.handleInput("\x1b[B"); + expect(getLevelRow("medium")?.startsWith(" ✓ medium")).toBe(true); + expect(getLevelRow("high")?.startsWith("→ high")).toBe(true); + }); +}); diff --git a/packages/coding-agent/test/tools.test.ts b/packages/coding-agent/test/tools.test.ts index e9c395aa6..b249980b7 100644 --- a/packages/coding-agent/test/tools.test.ts +++ b/packages/coding-agent/test/tools.test.ts @@ -4,8 +4,20 @@ import { tmpdir } from "os"; import { join } from "path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { executeBashWithOperations } from "../src/core/bash-executor.ts"; -import { type BashOperations, createBashTool, createLocalBashOperations } from "../src/core/tools/bash.ts"; +import type { ExtensionContext } from "../src/core/extensions/types.ts"; +import { + type BashOperations, + createBashTool, + createBashToolDefinition, + createLocalBashOperations, +} from "../src/core/tools/bash.ts"; +import { createEditToolDefinition } from "../src/core/tools/edit.ts"; import { computeEditsDiff } from "../src/core/tools/edit-diff.ts"; +import { createFindToolDefinition } from "../src/core/tools/find.ts"; +import { createGrepToolDefinition } from "../src/core/tools/grep.ts"; +import { createLsToolDefinition } from "../src/core/tools/ls.ts"; +import { createReadToolDefinition } from "../src/core/tools/read.ts"; +import { createWriteToolDefinition } from "../src/core/tools/write.ts"; import { createEditTool, createFindTool, @@ -890,6 +902,116 @@ describe("Coding Agent Tools", () => { }); }); +function fakeCtx(cwd: string): ExtensionContext { + return { cwd } as ExtensionContext; +} + +describe("tool cwd resolution", () => { + let testDir: string; + + beforeEach(() => { + testDir = join(tmpdir(), `coding-agent-cwd-test-${Date.now()}`); + mkdirSync(testDir, { recursive: true }); + }); + + afterEach(() => { + rmSync(testDir, { recursive: true, force: true }); + }); + + it("read uses ctx.cwd when provided", async () => { + const testFile = join(testDir, "ctx-cwd-read.txt"); + writeFileSync(testFile, "hello from ctx.cwd"); + const tool = createReadToolDefinition("/"); + const result = await tool.execute( + "test-read-ctx-cwd", + { path: "ctx-cwd-read.txt" }, + undefined, + undefined, + fakeCtx(testDir), + ); + const output = getTextOutput(result); + expect(output).toContain("hello from ctx.cwd"); + }); + + it("write uses ctx.cwd when provided", async () => { + const tool = createWriteToolDefinition("/"); + await tool.execute( + "test-write-ctx-cwd", + { path: "ctx-cwd-write.txt", content: "written via ctx.cwd" }, + undefined, + undefined, + fakeCtx(testDir), + ); + const content = readFileSync(join(testDir, "ctx-cwd-write.txt"), "utf-8"); + expect(content).toBe("written via ctx.cwd"); + }); + + it("edit uses ctx.cwd when provided", async () => { + const testFile = join(testDir, "ctx-cwd-edit.txt"); + writeFileSync(testFile, "old text"); + const tool = createEditToolDefinition("/"); + await tool.execute( + "test-edit-ctx-cwd", + { path: "ctx-cwd-edit.txt", edits: [{ oldText: "old text", newText: "new text" }] }, + undefined, + undefined, + fakeCtx(testDir), + ); + const content = readFileSync(testFile, "utf-8"); + expect(content).toBe("new text"); + }); + + it("grep uses ctx.cwd when provided", async () => { + const testFile = join(testDir, "ctx-cwd-grep.txt"); + writeFileSync(testFile, "match in ctx.cwd"); + const tool = createGrepToolDefinition("/"); + const result = await tool.execute( + "test-grep-ctx-cwd", + { pattern: "match" }, + undefined, + undefined, + fakeCtx(testDir), + ); + const output = getTextOutput(result); + expect(output).toContain("ctx-cwd-grep.txt"); + }); + + it("find uses ctx.cwd when provided", async () => { + writeFileSync(join(testDir, "ctx-cwd-find.txt"), "find me"); + const tool = createFindToolDefinition("/"); + const result = await tool.execute( + "test-find-ctx-cwd", + { pattern: "ctx-cwd-find.txt" }, + undefined, + undefined, + fakeCtx(testDir), + ); + const output = getTextOutput(result); + expect(output).toContain("ctx-cwd-find.txt"); + }); + + it("ls uses ctx.cwd when provided", async () => { + writeFileSync(join(testDir, "ctx-cwd-ls.txt"), "list me"); + const tool = createLsToolDefinition("/"); + const result = await tool.execute("test-ls-ctx-cwd", {}, undefined, undefined, fakeCtx(testDir)); + const output = getTextOutput(result); + expect(output).toContain("ctx-cwd-ls.txt"); + }); + + it("bash uses ctx.cwd when provided", async () => { + const tool = createBashToolDefinition("/", { exposeSessionEnvironment: false }); + const result = await tool.execute( + "test-bash-ctx-cwd", + { command: "pwd" }, + undefined, + undefined, + fakeCtx(testDir), + ); + const output = getTextOutput(result); + expect(output).toContain(testDir); + }); +}); + describe("edit tool fuzzy matching", () => { let testDir: string; diff --git a/packages/coding-agent/test/trust-selector.test.ts b/packages/coding-agent/test/trust-selector.test.ts index 33be9a972..c4659001b 100644 --- a/packages/coding-agent/test/trust-selector.test.ts +++ b/packages/coding-agent/test/trust-selector.test.ts @@ -14,7 +14,7 @@ describe("TrustSelectorComponent", () => { setKeybindings(new KeybindingsManager()); }); - it("marks the saved trusted decision", () => { + it("keeps the saved trusted decision marked while browsing", () => { const selector = new TrustSelectorComponent({ cwd: "/project", savedDecision: { path: "/project", decision: true }, @@ -23,12 +23,16 @@ describe("TrustSelectorComponent", () => { onCancel: () => {}, }); - const output = stripAnsi(selector.render(120).join("\n")); - + let output = stripAnsi(selector.render(120).join("\n")); expect(output).toContain("Saved decision: trusted (/project)"); expect(output).toContain("Current session: trusted"); - expect(output).toContain("Trust ✓"); - expect(output).not.toContain("Do not trust ✓"); + expect(output).toContain("→ ✓ Trust"); + + selector.handleInput("\x1b[B"); + output = stripAnsi(selector.render(120).join("\n")); + expect(output).toContain("✓ Trust"); + expect(output).toContain("→ Trust parent folder (/)"); + expect(output).not.toContain("✓ Do not trust"); }); it("selects a trust decision", () => { @@ -72,7 +76,7 @@ describe("TrustSelectorComponent", () => { const output = stripAnsi(selector.render(120).join("\n")); expect(output).toContain("Saved decision: trusted (inherited from /parent)"); - expect(output).toContain("Trust parent folder (/parent) ✓"); + expect(output).toContain("✓ Trust parent folder (/parent)"); selector.handleInput("\n"); diff --git a/packages/tui/src/terminal-image.ts b/packages/tui/src/terminal-image.ts index f82946389..a6ac5967e 100644 --- a/packages/tui/src/terminal-image.ts +++ b/packages/tui/src/terminal-image.ts @@ -110,11 +110,7 @@ function detectCapabilitiesFromEnvironment(tmuxForwardsHyperlink: () => boolean) return { images: null, trueColor: true, hyperlinks: true }; } - if (termProgram === "vscode") { - return { images: null, trueColor: true, hyperlinks: true }; - } - - if (termProgram === "alacritty") { + if (termProgram === "alacritty" || termProgram === "vscode" || termProgram === "zed") { return { images: null, trueColor: true, hyperlinks: true }; } diff --git a/packages/tui/src/terminal.ts b/packages/tui/src/terminal.ts index f858bf9f2..70a54475c 100644 --- a/packages/tui/src/terminal.ts +++ b/packages/tui/src/terminal.ts @@ -41,6 +41,20 @@ export function isAppleTerminalSession(): boolean { return process.platform === "darwin" && process.env.TERM_PROGRAM === "Apple_Terminal"; } +/** + * Refresh terminal dimensions on POSIX platforms by sending SIGWINCH to this process. + * Best-effort: some environments (restricted seccomp or LSM policies) return EACCES + * for `kill(2)`; in that case the dimensions refresh is skipped rather than crashing. + */ +export function refreshTerminalDimensions(): void { + if (process.platform === "win32" || process.pid <= 0) return; + try { + process.kill(process.pid, "SIGWINCH"); + } catch { + // Signal delivery not permitted in this environment; ignore. + } +} + export function normalizeNativeShiftEnterInput( data: string, shouldDetectNativeShiftEnter: boolean, @@ -177,10 +191,8 @@ export class ProcessTerminal implements Terminal { process.stdout.on("resize", this.resizeHandler); // Refresh terminal dimensions - they may be stale after suspend/resume - // (SIGWINCH is lost while process is stopped). Unix only. - if (process.platform !== "win32") { - process.kill(process.pid, "SIGWINCH"); - } + // (SIGWINCH is lost while process is stopped). Unix only, best-effort. + refreshTerminalDimensions(); // On Windows, enable ENABLE_VIRTUAL_TERMINAL_INPUT so the console sends // VT escape sequences (e.g. \x1b[Z for Shift+Tab) instead of raw console diff --git a/packages/tui/test/regression-sigwinch-kill-eacces.test.ts b/packages/tui/test/regression-sigwinch-kill-eacces.test.ts new file mode 100644 index 000000000..bb22a509a --- /dev/null +++ b/packages/tui/test/regression-sigwinch-kill-eacces.test.ts @@ -0,0 +1,62 @@ +import assert from "node:assert"; +import { after, describe, it } from "node:test"; +import { refreshTerminalDimensions } from "../src/terminal.ts"; + +describe("refreshTerminalDimensions", () => { + const originalKill = process.kill; + const originalPlatform = process.platform; + + it("does not throw when kill(2) returns EACCES for self-signal", () => { + Object.defineProperty(process, "platform", { value: "linux" }); + process.kill = ((): typeof process.kill => { + return (pid, _signal) => { + if (pid === process.pid) { + const err = new Error("kill EACCES") as NodeJS.ErrnoException; + err.code = "EACCES"; + throw err; + } + return originalKill.call(process, pid, _signal); + }; + })(); + + assert.doesNotThrow(() => { + refreshTerminalDimensions(); + }); + }); + + it("does not call kill on win32", () => { + Object.defineProperty(process, "platform", { value: "win32" }); + let killCalled = false; + process.kill = ((): typeof process.kill => { + return () => { + killCalled = true; + return true; + }; + })(); + + refreshTerminalDimensions(); + + assert.strictEqual(killCalled, false, "kill should not be called on win32"); + }); + + it("preserves other error codes", () => { + Object.defineProperty(process, "platform", { value: "linux" }); + process.kill = ((): typeof process.kill => { + return () => { + const err = new Error("kill EPERM") as NodeJS.ErrnoException; + err.code = "EPERM"; + throw err; + }; + })(); + + // EPERM is also ignored - the refresh is best-effort + assert.doesNotThrow(() => { + refreshTerminalDimensions(); + }); + }); + + after(() => { + process.kill = originalKill; + Object.defineProperty(process, "platform", { value: originalPlatform }); + }); +}); diff --git a/packages/tui/test/terminal-image.test.ts b/packages/tui/test/terminal-image.test.ts index b00af7782..069136d00 100644 --- a/packages/tui/test/terminal-image.test.ts +++ b/packages/tui/test/terminal-image.test.ts @@ -401,6 +401,12 @@ describe("detectCapabilities", () => { }); }); + it("enables Alacritty capabilities for Zed", () => { + withEnv({ TERM_PROGRAM: "zed" }, () => { + assert.deepStrictEqual(detectCapabilities(), { images: null, trueColor: true, hyperlinks: true }); + }); + }); + it("enables truecolor and hyperlinks for Windows Terminal outside multiplexers", () => { withEnv({ WT_SESSION: "session", TERM: "xterm-256color" }, () => { const caps = detectCapabilities();