From d912e692d84d9100d8cfaf0bdc12ba23d467d754 Mon Sep 17 00:00:00 2001 From: can1357 Date: Sun, 22 Feb 2026 16:58:46 +0100 Subject: [PATCH] feat(coding-agent): added per-command PTY control to bash tool - Added per-command `pty` parameter to bash tool for fine-grained PTY mode control. - Removed global `bash.virtualTerminal` setting in favor of per-command PTY parameter. - Fixed potential deadlock in shell session cleanup by replacing blocking lock with non-blocking try_lock. - Updated async session key generation to include jobId for improved session isolation. --- crates/pi-natives/src/shell.rs | 7 +++++- packages/coding-agent/CHANGELOG.md | 11 +++++++++ .../src/config/settings-schema.ts | 11 --------- packages/coding-agent/src/main.ts | 1 - .../src/modes/components/settings-defs.ts | 5 ---- packages/coding-agent/src/tools/bash.ts | 24 +++++++++++++------ 6 files changed, 34 insertions(+), 25 deletions(-) diff --git a/crates/pi-natives/src/shell.rs b/crates/pi-natives/src/shell.rs index 3a9e175d2..3033ba184 100644 --- a/crates/pi-natives/src/shell.rs +++ b/crates/pi-natives/src/shell.rs @@ -205,7 +205,12 @@ async fn run_shell_session( run_task.abort(); let _ = run_task.await; } - *session.lock().await = None; + // Use try_lock to avoid deadlocking if another task holds the session. + // If we can't acquire the lock, the session will be cleaned up when the + // holding task finishes. + if let Ok(mut guard) = session.try_lock() { + *guard = None; + } return Ok(ShellRunResult { exit_code: None, cancelled: matches!(reason, task::AbortReason::Signal), diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 3d6e4cde2..437459505 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,6 +1,17 @@ # Changelog ## [Unreleased] +### Added + +- Added `pty` parameter to bash tool to enable PTY mode for commands requiring a real terminal (e.g., sudo, ssh, top, less) + +### Changed + +- Changed bash tool to use per-command PTY control instead of global virtual terminal setting + +### Removed + +- Removed `bash.virtualTerminal` setting; use the `pty` parameter on individual bash commands instead ## [12.19.1] - 2026-02-22 ### Removed diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index fc0397ce2..88bd91b07 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -756,17 +756,6 @@ export const SETTINGS_SCHEMA = { // ───────────────────────────────────────────────────────────────────────── // Bash interceptor settings // ───────────────────────────────────────────────────────────────────────── - "bash.virtualTerminal": { - type: "enum", - values: ["on", "off"] as const, - default: "off", - ui: { - tab: "bash", - label: "Virtual terminal", - description: "Use PTY-backed interactive execution for bash", - submenu: true, - }, - }, "bashInterceptor.enabled": { type: "boolean", default: false, diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index 21307e1d0..0161ce606 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -538,7 +538,6 @@ export async function runRootCommand(parsed: Args, rawArgs: string[]): Promise Settings.init({ cwd })); if (parsedArgs.noPty) { - settings.override("bash.virtualTerminal", "off"); Bun.env.PI_NO_PTY = "1"; } const { diff --git a/packages/coding-agent/src/modes/components/settings-defs.ts b/packages/coding-agent/src/modes/components/settings-defs.ts index 96ea85c5e..6c1f5b39d 100644 --- a/packages/coding-agent/src/modes/components/settings-defs.ts +++ b/packages/coding-agent/src/modes/components/settings-defs.ts @@ -154,11 +154,6 @@ const OPTION_PROVIDERS: Partial> = { { value: "tool-only", label: "tool-only", description: "Interrupt only on tool-call argument matches" }, { value: "never", label: "never", description: "Never interrupt; inject warning after completion" }, ], - // Virtual terminal - "bash.virtualTerminal": [ - { value: "on", label: "On", description: "PTY-backed interactive execution" }, - { value: "off", label: "Off", description: "Standard non-interactive execution" }, - ], // Provider options "providers.webSearch": [ { diff --git a/packages/coding-agent/src/tools/bash.ts b/packages/coding-agent/src/tools/bash.ts index e6f81fe45..3867bb613 100644 --- a/packages/coding-agent/src/tools/bash.ts +++ b/packages/coding-agent/src/tools/bash.ts @@ -34,6 +34,11 @@ const bashSchemaBase = Type.Object({ cwd: Type.Optional(Type.String({ description: "Working directory (default: cwd)" })), head: Type.Optional(Type.Number({ description: "Return only first N lines of output" })), tail: Type.Optional(Type.Number({ description: "Return only last N lines of output" })), + pty: Type.Optional( + Type.Boolean({ + description: "Run in PTY mode when command needs a real terminal (e.g. sudo/ssh/top/less); default: false", + }), + ), }); const bashSchemaWithAsync = Type.Object({ @@ -54,6 +59,7 @@ export interface BashToolInput { head?: number; tail?: number; async?: boolean; + pty?: boolean; } export interface BashToolDetails { @@ -123,7 +129,15 @@ export class BashTool implements AgentTool { async execute( _toolCallId: string, - { command: rawCommand, timeout: rawTimeout = 300, cwd, head, tail, async: asyncRequested = false }: BashToolInput, + { + command: rawCommand, + timeout: rawTimeout = 300, + cwd, + head, + tail, + async: asyncRequested = false, + pty = false, + }: BashToolInput, signal?: AbortSignal, onUpdate?: AgentToolUpdateCallback, ctx?: AgentToolContext, @@ -188,7 +202,7 @@ export class BashTool implements AgentTool { try { const result = await executeBash(command, { cwd: commandCwd, - sessionKey: this.session.getSessionId?.() ?? undefined, + sessionKey: `${this.session.getSessionId?.() ?? ""}:async:${jobId}`, timeout: timeoutMs, signal: runSignal, env: extraEnv, @@ -229,11 +243,7 @@ export class BashTool implements AgentTool { const extraEnv = artifactsDir ? { ARTIFACTS: artifactsDir } : undefined; const { path: artifactPath, id: artifactId } = (await this.session.allocateOutputArtifact?.("bash")) ?? {}; - const usePty = - this.session.settings.get("bash.virtualTerminal") === "on" && - $env.PI_NO_PTY !== "1" && - ctx?.hasUI === true && - ctx.ui !== undefined; + const usePty = pty && $env.PI_NO_PTY !== "1" && ctx?.hasUI === true && ctx.ui !== undefined; const result: BashResult | BashInteractiveResult = usePty ? await runInteractiveBashPty(ctx.ui!, { command,