diff --git a/CHANGELOG.md b/CHANGELOG.md index 12e5a59d4..fae9ddf83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,15 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename reports directly, not by re-parsing the parent-facing report's prose. Removes the `isXxxSubAgentReport` classifier family and per-reason parent hint functions in favor of a single structured switch. +### Internal + +- Removed the dead Ink-era kill ring copy (`src/tui/kill-ring.ts`); the OpenTUI + prompt kill ring (`src/tui/prompt-kill-ring.ts`) is the sole implementation. +- Extracted the shared timeout-race helper (`src/util/budget-race.ts`) used by + the shell-guard search budget and the tool-execution watchdog, replacing two + independent copies of the same `AbortController` + `setTimeout` race. +- `runtime-bridge.ts` now re-exports `mapReactorLike` from `stream-event-map.ts` + instead of wrapping it in an identical local function. ## [0.2.108] - 2026-08-24 diff --git a/src/plugins/shell-guard-plugin.ts b/src/plugins/shell-guard-plugin.ts index 1ab91531e..f96363aa7 100644 --- a/src/plugins/shell-guard-plugin.ts +++ b/src/plugins/shell-guard-plugin.ts @@ -1,7 +1,8 @@ import { spawn, type ChildProcess } from "node:child_process"; import { realpathSync } from "node:fs"; import type { ToolPlugin } from "@intx/tools-posix"; -import { formatSearchTimeoutMessage } from "./tool-time-budget.js"; +import { formatSearchTimeoutMessage, TIMEOUT_PREFIX } from "./tool-time-budget.js"; +import { BUDGET_EXPIRED, budgetExpiry, withTimeout } from "../util/budget-race.js"; import type { ToolDefinition } from "@intx/types/runtime"; import { assertShellCwdUsable, @@ -325,38 +326,6 @@ function optionalNumber(value: unknown): number | undefined { return typeof value === "number" && Number.isFinite(value) ? value : undefined; } -function withTimeout( - signal: AbortSignal, - timeoutMs: number, -): { signal: AbortSignal; dispose: () => void } { - const controller = new AbortController(); - const onParentAbort = () => controller.abort(); - signal.addEventListener("abort", onParentAbort, { once: true }); - const timer = setTimeout(() => controller.abort(), timeoutMs); - if (signal.aborted) controller.abort(); - return { - signal: controller.signal, - dispose: () => { - clearTimeout(timer); - signal.removeEventListener("abort", onParentAbort); - }, - }; -} - -const BUDGET_EXPIRED = Symbol("search-budget-expired"); - -function budgetExpiry(signal: AbortSignal): Promise { - return new Promise((resolve) => { - if (signal.aborted) { - resolve(BUDGET_EXPIRED); - return; - } - signal.addEventListener("abort", () => resolve(BUDGET_EXPIRED), { - once: true, - }); - }); -} - /** * Replaces stock run_shell with a hard-capped implementation, and applies a * 10s wall-clock budget to grep/search_files when the agent does not abort @@ -510,7 +479,7 @@ export function shellGuardPlugin( if ( outcome.isError === true && typeof outcome.content === "string" && - outcome.content.includes("[timed out before completing]") + outcome.content.includes(TIMEOUT_PREFIX) ) { return outcome; } diff --git a/src/plugins/tool-time-budget.ts b/src/plugins/tool-time-budget.ts index 94c4034fc..b957ff918 100644 --- a/src/plugins/tool-time-budget.ts +++ b/src/plugins/tool-time-budget.ts @@ -2,7 +2,7 @@ export type ScopedSearchTool = "grep" | "search_files"; -const TIMEOUT_PREFIX = "[timed out before completing]"; +export const TIMEOUT_PREFIX = "[timed out before completing]"; export function scopedSearchRetryHints(tool: ScopedSearchTool): string { const base = diff --git a/src/tui/kill-ring.test.ts b/src/tui/kill-ring.test.ts deleted file mode 100644 index d282ceb91..000000000 --- a/src/tui/kill-ring.test.ts +++ /dev/null @@ -1,134 +0,0 @@ -import { describe, test, expect } from "bun:test"; -import { - KILL_RING_MAX, - beginYank, - breakKillSequence, - emptyKillRing, - recordKill, - rotateYank, - type KillRing, -} from "./kill-ring.js"; - -describe("recordKill", () => { - test("pushes killed text to the front", () => { - let ring = recordKill(emptyKillRing, "one", "forward"); - ring = breakKillSequence(ring); - ring = recordKill(ring, "two", "forward"); - expect(ring.entries).toEqual(["two", "one"]); - }); - - test("empty text does not create an entry", () => { - const ring = recordKill(emptyKillRing, "", "forward"); - expect(ring.entries).toEqual([]); - }); - - test("consecutive forward kills append to one entry", () => { - let ring = recordKill(emptyKillRing, "foo ", "forward"); - ring = recordKill(ring, "bar", "forward"); - expect(ring.entries).toEqual(["foo bar"]); - }); - - test("consecutive backward kills prepend to one entry", () => { - let ring = recordKill(emptyKillRing, "bar", "backward"); - ring = recordKill(ring, "foo ", "backward"); - expect(ring.entries).toEqual(["foo bar"]); - }); - - test("mixed directions still accumulate into one entry", () => { - let ring = recordKill(emptyKillRing, "mid", "forward"); - ring = recordKill(ring, "pre ", "backward"); - ring = recordKill(ring, " post", "forward"); - expect(ring.entries).toEqual(["pre mid post"]); - }); - - test("an intervening non-kill command starts a new entry", () => { - let ring = recordKill(emptyKillRing, "one", "forward"); - ring = breakKillSequence(ring); - ring = recordKill(ring, "two", "forward"); - expect(ring.entries).toEqual(["two", "one"]); - }); - - test("ring is capped at KILL_RING_MAX entries", () => { - let ring: KillRing = emptyKillRing; - for (let i = 0; i < KILL_RING_MAX + 3; i++) { - ring = breakKillSequence(ring); - ring = recordKill(ring, `kill${i}`, "forward"); - } - expect(ring.entries.length).toBe(KILL_RING_MAX); - expect(ring.entries[0]).toBe(`kill${KILL_RING_MAX + 2}`); - }); - - test("a new kill resets yank rotation to the newest entry", () => { - let ring = recordKill(emptyKillRing, "one", "forward"); - ring = breakKillSequence(ring); - ring = recordKill(ring, "two", "forward"); - const yank = beginYank(ring, 0)!; - const rotated = rotateYank(yank.ring)!; - expect(rotated.ring.yankIndex).toBe(1); - let next = breakKillSequence(rotated.ring); - next = recordKill(next, "three", "forward"); - expect(beginYank(next, 0)!.text).toBe("three"); - }); -}); - -describe("beginYank", () => { - test("returns null when nothing has been killed", () => { - expect(beginYank(emptyKillRing, 0)).toBeNull(); - }); - - test("yanks the most recent kill and records the span", () => { - const ring = recordKill(emptyKillRing, "hello", "forward"); - const yank = beginYank(ring, 3)!; - expect(yank.text).toBe("hello"); - expect(yank.ring.lastYankSpan).toEqual({ start: 3, end: 8 }); - }); - - test("re-yank after rotation uses the rotated entry", () => { - let ring = recordKill(emptyKillRing, "one", "forward"); - ring = breakKillSequence(ring); - ring = recordKill(ring, "two", "forward"); - const first = beginYank(ring, 0)!; - const rotated = rotateYank(first.ring)!; - const settled = breakKillSequence(rotated.ring); - expect(beginYank(settled, 0)!.text).toBe("one"); - }); -}); - -describe("rotateYank", () => { - test("returns null when the previous command was not a yank", () => { - const ring = recordKill(emptyKillRing, "one", "forward"); - expect(rotateYank(ring)).toBeNull(); - }); - - test("returns null on an empty ring", () => { - expect(rotateYank(emptyKillRing)).toBeNull(); - }); - - test("replaces the yanked span with the next-older kill", () => { - let ring = recordKill(emptyKillRing, "one", "forward"); - ring = breakKillSequence(ring); - ring = recordKill(ring, "two", "forward"); - const yank = beginYank(ring, 5)!; - const rotated = rotateYank(yank.ring)!; - expect(rotated.span).toEqual({ start: 5, end: 8 }); - expect(rotated.text).toBe("one"); - expect(rotated.ring.lastYankSpan).toEqual({ start: 5, end: 8 }); - }); - - test("wraps around to the newest entry", () => { - let ring = recordKill(emptyKillRing, "one", "forward"); - ring = breakKillSequence(ring); - ring = recordKill(ring, "two", "forward"); - const yank = beginYank(ring, 0)!; - const r1 = rotateYank(yank.ring)!; - const r2 = rotateYank(r1.ring)!; - expect(r2.text).toBe("two"); - }); - - test("any non-yank command ends the rotation window", () => { - const ring = recordKill(emptyKillRing, "one", "forward"); - const yank = beginYank(ring, 0)!; - const broken = breakKillSequence(yank.ring); - expect(rotateYank(broken)).toBeNull(); - }); -}); diff --git a/src/tui/kill-ring.ts b/src/tui/kill-ring.ts deleted file mode 100644 index a50b6ad48..000000000 --- a/src/tui/kill-ring.ts +++ /dev/null @@ -1,100 +0,0 @@ -// Readline-style kill ring backing the prompt's kill and yank commands. -// Entries persist across kills and submissions; the transient fields track -// whether the previous command was a kill (so consecutive kills accumulate -// into one entry) or a yank (so Meta+Y can rotate through earlier kills). - -export const KILL_RING_MAX = 10; - -export interface YankSpan { - start: number; - end: number; -} - -export interface KillRing { - /** Killed strings, most recent first. */ - entries: string[]; - /** Ring entry inserted by the most recent yank; Meta+Y advances it. */ - yankIndex: number; - lastAction: "kill-forward" | "kill-backward" | "yank" | "other"; - /** Buffer span occupied by the last yank; null unless the previous command was a yank. */ - lastYankSpan: YankSpan | null; -} - -export const emptyKillRing: KillRing = { - entries: [], - yankIndex: 0, - lastAction: "other", - lastYankSpan: null, -}; - -/** Any command that is not a kill or yank ends accumulation and rotation. */ -export function breakKillSequence(ring: KillRing): KillRing { - if (ring.lastAction === "other" && ring.lastYankSpan === null) return ring; - return { ...ring, lastAction: "other", lastYankSpan: null }; -} - -// Consecutive kills grow a single ring entry the way readline does: forward -// kills append, backward kills prepend, so C-k C-k ... C-y restores the -// killed region in original order. -export function recordKill( - ring: KillRing, - text: string, - direction: "forward" | "backward", -): KillRing { - if (text.length === 0) return breakKillSequence(ring); - const accumulating = - (ring.lastAction === "kill-forward" || ring.lastAction === "kill-backward") && - ring.entries.length > 0; - const entries = accumulating - ? [ - direction === "forward" ? ring.entries[0]! + text : text + ring.entries[0]!, - ...ring.entries.slice(1), - ] - : [text, ...ring.entries].slice(0, KILL_RING_MAX); - return { - entries, - yankIndex: 0, - lastAction: direction === "forward" ? "kill-forward" : "kill-backward", - lastYankSpan: null, - }; -} - -/** C-y: text to insert at the cursor, or null when nothing has been killed. */ -export function beginYank(ring: KillRing, cursor: number): { ring: KillRing; text: string } | null { - const index = ring.yankIndex < ring.entries.length ? ring.yankIndex : 0; - const text = ring.entries[index]; - if (text === undefined) return null; - return { - text, - ring: { - ...ring, - yankIndex: index, - lastAction: "yank", - lastYankSpan: { start: cursor, end: cursor + text.length }, - }, - }; -} - -/** - * Meta+Y immediately after a yank: the caller replaces the returned span with - * the next-older kill. Rotation persists, so the next C-y yanks that entry. - */ -export function rotateYank( - ring: KillRing, -): { ring: KillRing; span: YankSpan; text: string } | null { - if (ring.lastAction !== "yank" || ring.lastYankSpan === null) return null; - if (ring.entries.length === 0) return null; - const nextIndex = (ring.yankIndex + 1) % ring.entries.length; - const text = ring.entries[nextIndex]!; - const span = ring.lastYankSpan; - return { - text, - span, - ring: { - ...ring, - yankIndex: nextIndex, - lastAction: "yank", - lastYankSpan: { start: span.start, end: span.start + text.length }, - }, - }; -} diff --git a/src/tui/prompt-kill-ring.ts b/src/tui/prompt-kill-ring.ts index 84f86f7c0..a0f1c3de7 100644 --- a/src/tui/prompt-kill-ring.ts +++ b/src/tui/prompt-kill-ring.ts @@ -15,9 +15,8 @@ * `rotateYank` hand back the text to splice in; shell.ts performs the splice * against the InputRenderable directly. * - * Mirrors the semantics of src/tui/kill-ring.ts (the Ink reference) without - * importing from it — the two prompt implementations are independent trees - * during the OpenTUI cutover. + * Sole kill ring implementation (the former Ink-era src/tui/kill-ring.ts + * copy was retired once the OpenTUI cutover made it dead code). */ export const KILL_RING_MAX = 10; diff --git a/src/tui/runtime-bridge.ts b/src/tui/runtime-bridge.ts index d56f64b5a..08ac127c2 100644 --- a/src/tui/runtime-bridge.ts +++ b/src/tui/runtime-bridge.ts @@ -86,14 +86,15 @@ import { PRODUCTION_REACTOR_TYPES, createStreamMapContext, mapProductionEvent, - mapReactorLike as mapReactorLikeImpl, + mapReactorLike, type BridgeInboundEvent, type ReactorLikeEvent, type StreamMapContext, } from "./stream-event-map.js"; -/** Re-export map types so existing `from "./runtime-bridge"` imports keep working. */ +/** Re-export map types/fn so existing `from "./runtime-bridge"` imports keep working. */ export type { BridgeInboundEvent, ReactorLikeEvent, StreamMapContext }; +export { mapReactorLike }; /** Outbound actions the UI asks the session runtime to perform. */ export interface SessionPort { @@ -260,14 +261,6 @@ function isBridgeInbound(event: { type: string }): event is BridgeInboundEvent { } } -/** - * Map a reactor-like event into zero or more canonical bridge events. - * Stateless (fixture-friendly). Live sessions use a StreamMapContext via handle. - */ -export function mapReactorLike(event: ReactorLikeEvent): readonly BridgeInboundEvent[] { - return mapReactorLikeImpl(event); -} - function rowFromInbound(event: BridgeInboundEvent): StreamRow | null { switch (event.type) { case "user": diff --git a/src/tui/tool-execution-watchdog.ts b/src/tui/tool-execution-watchdog.ts index 29b6a5f13..4cfb6ebdd 100644 --- a/src/tui/tool-execution-watchdog.ts +++ b/src/tui/tool-execution-watchdog.ts @@ -2,7 +2,10 @@ import { AsyncLocalStorage } from "node:async_hooks"; import { formatMcpToolTimeoutMessage, formatToolExecutionTimeoutMessage, + TIMEOUT_PREFIX, } from "../plugins/tool-time-budget.js"; +import { BUDGET_EXPIRED, budgetExpiry, withTimeout } from "../util/budget-race.js"; +export { withTimeout }; import { isMcpToolName } from "../mcp/tool-name.js"; import type { ToolCall, ToolResult } from "@intx/types/runtime"; @@ -58,8 +61,6 @@ export const TOOL_EXECUTION_SALVAGE_GRACE_MS = 5_000; */ export const MAX_TOOL_APPROVAL_PAUSE_MS = 1_800_000; -const BUDGET_EXPIRED = Symbol("tool-execution-budget-expired"); - /** * Wall-clock budget for one tool `run()`, or undefined to leave the timer unarmed. * Parent cancel, maxTurns, and eval `--agent-timeout-ms` still bound the run. @@ -132,24 +133,6 @@ export function resolveWaitForApproval(config?: ToolWatchdogConfig): boolean { return config?.waitForApproval !== false; } -export function withTimeout( - signal: AbortSignal, - timeoutMs: number, -): { signal: AbortSignal; dispose: () => void } { - const controller = new AbortController(); - const onParentAbort = () => controller.abort(); - signal.addEventListener("abort", onParentAbort, { once: true }); - const timer = setTimeout(() => controller.abort(), timeoutMs); - if (signal.aborted) controller.abort(); - return { - signal: controller.signal, - dispose: () => { - clearTimeout(timer); - signal.removeEventListener("abort", onParentAbort); - }, - }; -} - /** * Identifies the pause generation a `pause()` call belonged to. A forced * ceiling resume bumps the generation, so a `resume(token)` call made after @@ -311,16 +294,6 @@ export function getToolApprovalBudget(): ToolApprovalBudget | undefined { return toolApprovalBudgetAls.getStore(); } -function budgetExpiry(signal: AbortSignal): Promise { - return new Promise((resolve) => { - if (signal.aborted) { - resolve(BUDGET_EXPIRED); - return; - } - signal.addEventListener("abort", () => resolve(BUDGET_EXPIRED), { once: true }); - }); -} - function isAbortLikeToolError(content: string): boolean { return /abort/i.test(content); } @@ -458,7 +431,7 @@ export async function runWithToolExecutionWatchdog( if ( outcome.isError === true && typeof outcome.content === "string" && - outcome.content.includes("[timed out before completing]") + outcome.content.includes(TIMEOUT_PREFIX) ) { return outcome; } diff --git a/src/util/budget-race.ts b/src/util/budget-race.ts new file mode 100644 index 000000000..c4a33255a --- /dev/null +++ b/src/util/budget-race.ts @@ -0,0 +1,35 @@ +// Shared timeout-race primitives: race a promise against an AbortSignal-driven +// wall-clock budget. Used by both the shell-guard search budget and the +// tool-execution watchdog, which otherwise reimplemented the same race twice. + +export const BUDGET_EXPIRED = Symbol("budget-expired"); + +/** Resolves with BUDGET_EXPIRED once `signal` aborts (immediately if already aborted). */ +export function budgetExpiry(signal: AbortSignal): Promise { + return new Promise((resolve) => { + if (signal.aborted) { + resolve(BUDGET_EXPIRED); + return; + } + signal.addEventListener("abort", () => resolve(BUDGET_EXPIRED), { once: true }); + }); +} + +/** Derives a signal that aborts when `signal` aborts or after `timeoutMs`, whichever first. */ +export function withTimeout( + signal: AbortSignal, + timeoutMs: number, +): { signal: AbortSignal; dispose: () => void } { + const controller = new AbortController(); + const onParentAbort = () => controller.abort(); + signal.addEventListener("abort", onParentAbort, { once: true }); + const timer = setTimeout(() => controller.abort(), timeoutMs); + if (signal.aborted) controller.abort(); + return { + signal: controller.signal, + dispose: () => { + clearTimeout(timer); + signal.removeEventListener("abort", onParentAbort); + }, + }; +}