From 9943a280dbfbb600162ec2ca226c6d4c5095e00c Mon Sep 17 00:00:00 2001 From: can1357 Date: Thu, 7 May 2026 15:40:45 +0200 Subject: [PATCH] feat(coding-agent): added /loop count/duration arg parsing and budgeting - Added optional `/loop` `count|duration` command arguments and wired `command.args` into loop handling. - Implemented `loop-limit` parsing and runtime types/helpers for iteration and duration budget limits with validation. - Updated interactive mode to enforce loop limits per iteration, check duration expiry, and clear budget state on disable. - Added loop-limit parse/runtime tests and fixed `/loop` arg errors plus macOS `MallocStackLogging` environment leakage. --- packages/coding-agent/CHANGELOG.md | 12 ++ .../src/modes/interactive-mode.ts | 36 ++++- packages/coding-agent/src/modes/loop-limit.ts | 140 ++++++++++++++++++ packages/coding-agent/src/modes/types.ts | 4 +- .../src/slash-commands/builtin-registry.ts | 6 +- packages/coding-agent/test/loop-limit.test.ts | 71 +++++++++ 6 files changed, 262 insertions(+), 7 deletions(-) create mode 100644 packages/coding-agent/src/modes/loop-limit.ts create mode 100644 packages/coding-agent/test/loop-limit.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 8c679028e..3a180fe80 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,6 +1,18 @@ # Changelog ## [Unreleased] +### Added + +- Added optional `/loop` limits: `/loop 10` stops after 10 auto-iterations, while duration forms such as `/loop 10m` and `/loop 10min` stop after the time limit. + +### Changed + +- Changed `/loop` to include the configured limit and remaining budget in the enabled status message + +### Fixed + +- Fixed `/loop` handling of malformed count or duration arguments by showing usage errors instead of enabling unbounded loop mode +- Fixed inherited disabled macOS malloc stack logging variables leaking into shell sessions and spamming Bun subprocess output with `MallocStackLogging` warnings. ## [14.7.4] - 2026-05-07 diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index 5cbd05dc9..8af9a00b0 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -73,6 +73,15 @@ import { MCPCommandController } from "./controllers/mcp-command-controller"; import { SelectorController } from "./controllers/selector-controller"; import { SSHCommandController } from "./controllers/ssh-command-controller"; import { TodoCommandController } from "./controllers/todo-command-controller"; +import { + consumeLoopLimitIteration, + createLoopLimitRuntime, + describeLoopLimit, + describeLoopLimitRuntime, + isLoopDurationExpired, + type LoopLimitRuntime, + parseLoopLimitArgs, +} from "./loop-limit"; import { OAuthManualInputManager } from "./oauth-manual-input"; import { SessionObserverRegistry } from "./session-observer-registry"; import type { Theme } from "./theme/theme"; @@ -158,6 +167,7 @@ export class InteractiveMode implements InteractiveModeContext { planModePlanFilePath: string | undefined = undefined; loopModeEnabled = false; loopPrompt: string | undefined = undefined; + loopLimit: LoopLimitRuntime | undefined = undefined; #loopAutoSubmitTimer: NodeJS.Timeout | undefined; todoPhases: TodoPhase[] = []; hideThinkingBlock = false; @@ -535,25 +545,35 @@ export class InteractiveMode implements InteractiveModeContext { } async #runLoopIteration(action: "prompt" | "compact" | "reset", prompt: string): Promise { + if (!consumeLoopLimitIteration(this.loopLimit)) { + this.disableLoopMode("Loop limit reached. Loop mode disabled."); + return; + } + if (action === "compact") { await this.handleCompactCommand(); } else if (action === "reset") { await this.handleClearCommand(); } if (!this.loopModeEnabled || !this.onInputCallback) return; + if (isLoopDurationExpired(this.loopLimit)) { + this.disableLoopMode("Loop time limit reached. Loop mode disabled."); + return; + } this.onInputCallback(this.startPendingSubmission({ text: prompt })); } - disableLoopMode(): void { + disableLoopMode(message = "Loop mode disabled."): void { const wasEnabled = this.loopModeEnabled; this.loopModeEnabled = false; this.loopPrompt = undefined; + this.loopLimit = undefined; this.#cancelLoopAutoSubmit(); this.statusLine.setLoopModeStatus(undefined); this.updateEditorTopBorder(); this.ui.requestRender(); if (wasEnabled) { - this.showStatus("Loop mode disabled."); + this.showStatus(message); } } @@ -567,18 +587,26 @@ export class InteractiveMode implements InteractiveModeContext { this.#cancelLoopAutoSubmit(); } - async handleLoopCommand(): Promise { + async handleLoopCommand(args = ""): Promise { if (this.loopModeEnabled) { this.disableLoopMode(); return; } + const parsedLimit = parseLoopLimitArgs(args); + if (typeof parsedLimit === "string") { + this.showError(parsedLimit); + return; + } this.loopModeEnabled = true; this.loopPrompt = undefined; + this.loopLimit = createLoopLimitRuntime(parsedLimit); this.statusLine.setLoopModeStatus({ enabled: true }); this.updateEditorTopBorder(); this.ui.requestRender(); + const limitSuffix = parsedLimit ? ` Limited to ${describeLoopLimit(parsedLimit)}.` : ""; + const remainingSuffix = this.loopLimit ? ` ${describeLoopLimitRuntime(this.loopLimit)}.` : ""; this.showStatus( - "Loop mode enabled. Your next prompt will repeat after each turn. Esc cancels the current iteration; /loop again to disable.", + `Loop mode enabled.${limitSuffix}${remainingSuffix} Your next prompt will repeat after each turn. Esc cancels the current iteration; /loop again to disable.`, ); } diff --git a/packages/coding-agent/src/modes/loop-limit.ts b/packages/coding-agent/src/modes/loop-limit.ts new file mode 100644 index 000000000..289f73d5b --- /dev/null +++ b/packages/coding-agent/src/modes/loop-limit.ts @@ -0,0 +1,140 @@ +export type LoopLimitConfig = + | { + kind: "iterations"; + iterations: number; + } + | { + kind: "duration"; + durationMs: number; + }; + +export type LoopLimitRuntime = + | { + kind: "iterations"; + initial: number; + remaining: number; + } + | { + kind: "duration"; + durationMs: number; + deadlineMs: number; + }; + +const TIME_UNITS_MS = new Map([ + ["s", 1_000], + ["sec", 1_000], + ["secs", 1_000], + ["second", 1_000], + ["seconds", 1_000], + ["m", 60_000], + ["min", 60_000], + ["mins", 60_000], + ["minute", 60_000], + ["minutes", 60_000], + ["h", 3_600_000], + ["hr", 3_600_000], + ["hrs", 3_600_000], + ["hour", 3_600_000], + ["hours", 3_600_000], +]); + +export function parseLoopLimitArgs(args: string): LoopLimitConfig | undefined | string { + const trimmed = args.trim().toLowerCase(); + if (!trimmed) return undefined; + + const parts = trimmed.split(/\s+/); + if (parts.length > 2) { + return "Usage: /loop [count|duration]. Examples: /loop 10, /loop 10m, /loop 10min."; + } + + if (parts.length === 2) { + return parseDurationParts(parts[0], parts[1]); + } + + const token = parts[0]; + const iterationMatch = /^(\d+)$/.exec(token); + if (iterationMatch) { + const iterations = Number(iterationMatch[1]); + if (!Number.isSafeInteger(iterations) || iterations <= 0) { + return "Loop count must be a positive integer."; + } + return { kind: "iterations", iterations }; + } + + const durationMatch = /^(\d+)([a-z]+)$/.exec(token); + if (durationMatch) { + return parseDurationParts(durationMatch[1], durationMatch[2]); + } + + return "Usage: /loop [count|duration]. Examples: /loop 10, /loop 10m, /loop 10min."; +} + +function parseDurationParts(amountText: string, unitText: string): LoopLimitConfig | string { + if (!/^\d+$/.test(amountText)) { + return "Loop duration must use a positive integer amount."; + } + + const amount = Number(amountText); + if (!Number.isSafeInteger(amount) || amount <= 0) { + return "Loop duration must be positive."; + } + + const unitMs = TIME_UNITS_MS.get(unitText); + if (unitMs === undefined) { + return "Loop duration unit must be seconds, minutes, or hours."; + } + + return { kind: "duration", durationMs: amount * unitMs }; +} + +export function createLoopLimitRuntime( + config: LoopLimitConfig | undefined, + nowMs = Date.now(), +): LoopLimitRuntime | undefined { + if (!config) return undefined; + if (config.kind === "iterations") { + return { kind: "iterations", initial: config.iterations, remaining: config.iterations }; + } + return { kind: "duration", durationMs: config.durationMs, deadlineMs: nowMs + config.durationMs }; +} + +export function consumeLoopLimitIteration(limit: LoopLimitRuntime | undefined, nowMs = Date.now()): boolean { + if (!limit) return true; + if (limit.kind === "duration") { + return nowMs < limit.deadlineMs; + } + if (limit.remaining <= 0) return false; + limit.remaining -= 1; + return true; +} + +export function isLoopDurationExpired(limit: LoopLimitRuntime | undefined, nowMs = Date.now()): boolean { + return limit?.kind === "duration" && nowMs >= limit.deadlineMs; +} + +export function describeLoopLimit(config: LoopLimitConfig): string { + if (config.kind === "iterations") { + return `${config.iterations} ${config.iterations === 1 ? "iteration" : "iterations"}`; + } + return formatDuration(config.durationMs); +} + +export function describeLoopLimitRuntime(limit: LoopLimitRuntime): string { + if (limit.kind === "iterations") { + return `${limit.remaining} of ${limit.initial} ${limit.initial === 1 ? "iteration" : "iterations"} remaining`; + } + return `${formatDuration(limit.durationMs)} limit`; +} + +function formatDuration(durationMs: number): string { + if (durationMs % 3_600_000 === 0) { + const hours = durationMs / 3_600_000; + return `${hours} ${hours === 1 ? "hour" : "hours"}`; + } + if (durationMs % 60_000 === 0) { + const minutes = durationMs / 60_000; + return `${minutes} ${minutes === 1 ? "minute" : "minutes"}`; + } + const seconds = durationMs / 1_000; + return `${seconds} ${seconds === 1 ? "second" : "seconds"}`; +} diff --git a/packages/coding-agent/src/modes/types.ts b/packages/coding-agent/src/modes/types.ts index b57f4516a..3c4036ade 100644 --- a/packages/coding-agent/src/modes/types.ts +++ b/packages/coding-agent/src/modes/types.ts @@ -24,6 +24,7 @@ import type { HookInputComponent } from "./components/hook-input"; import type { HookSelectorComponent } from "./components/hook-selector"; import type { StatusLineComponent } from "./components/status-line"; import type { ToolExecutionHandle } from "./components/tool-execution"; +import type { LoopLimitRuntime } from "./loop-limit"; import type { OAuthManualInputManager } from "./oauth-manual-input"; import type { Theme } from "./theme/theme"; @@ -86,6 +87,7 @@ export interface InteractiveModeContext { planModeEnabled: boolean; loopModeEnabled: boolean; loopPrompt?: string; + loopLimit?: LoopLimitRuntime; planModePlanFilePath?: string; hideThinkingBlock: boolean; pendingImages: ImageContent[]; @@ -248,7 +250,7 @@ export interface InteractiveModeContext { openExternalEditor(): void; registerExtensionShortcuts(): void; handlePlanModeCommand(initialPrompt?: string): Promise; - handleLoopCommand(): Promise; + handleLoopCommand(args?: string): Promise; disableLoopMode(): void; pauseLoop(): void; handleExitPlanModeTool(details: ExitPlanModeDetails): Promise; diff --git a/packages/coding-agent/src/slash-commands/builtin-registry.ts b/packages/coding-agent/src/slash-commands/builtin-registry.ts index a385ee42d..96df2ca12 100644 --- a/packages/coding-agent/src/slash-commands/builtin-registry.ts +++ b/packages/coding-agent/src/slash-commands/builtin-registry.ts @@ -119,8 +119,10 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray = [ name: "loop", description: "Toggle loop mode. While enabled, the next prompt you send re-submits after every yield. Esc cancels the current iteration; /loop again to disable.", - handle: async (_command, runtime) => { - await runtime.ctx.handleLoopCommand(); + inlineHint: "[count|duration]", + allowArgs: true, + handle: async (command, runtime) => { + await runtime.ctx.handleLoopCommand(command.args); runtime.ctx.editor.setText(""); }, }, diff --git a/packages/coding-agent/test/loop-limit.test.ts b/packages/coding-agent/test/loop-limit.test.ts new file mode 100644 index 000000000..ff2033c19 --- /dev/null +++ b/packages/coding-agent/test/loop-limit.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, test, vi } from "bun:test"; +import { + consumeLoopLimitIteration, + createLoopLimitRuntime, + isLoopDurationExpired, + parseLoopLimitArgs, +} from "@oh-my-pi/pi-coding-agent/modes/loop-limit"; +import type { BuiltinSlashCommandRuntime } from "@oh-my-pi/pi-coding-agent/slash-commands/builtin-registry"; +import { executeBuiltinSlashCommand } from "@oh-my-pi/pi-coding-agent/slash-commands/builtin-registry"; + +describe("/loop slash command", () => { + test("accepts an optional limit argument", async () => { + const handleLoopCommand = vi.fn(async (_args?: string) => {}); + const runtime = { + ctx: { + handleLoopCommand, + editor: { setText: vi.fn() }, + }, + handleBackgroundCommand: vi.fn(), + } as unknown as BuiltinSlashCommandRuntime; + const result = await executeBuiltinSlashCommand("/loop 10min", runtime); + + expect(result).toBe(true); + expect(handleLoopCommand).toHaveBeenCalledWith("10min"); + }); +}); + +describe("loop limit parsing", () => { + test("parses a bare positive integer as an iteration limit", () => { + expect(parseLoopLimitArgs("10")).toEqual({ kind: "iterations", iterations: 10 }); + }); + + test("parses minute duration aliases", () => { + expect(parseLoopLimitArgs("10m")).toEqual({ kind: "duration", durationMs: 600_000 }); + expect(parseLoopLimitArgs("10min")).toEqual({ kind: "duration", durationMs: 600_000 }); + expect(parseLoopLimitArgs("10 minutes")).toEqual({ kind: "duration", durationMs: 600_000 }); + }); + + test("rejects zero, negative, and unknown limits", () => { + expect(parseLoopLimitArgs("0")).toBe("Loop count must be a positive integer."); + expect(parseLoopLimitArgs("-1")).toContain("Usage: /loop"); + expect(parseLoopLimitArgs("10fortnights")).toBe("Loop duration unit must be seconds, minutes, or hours."); + }); +}); + +describe("loop limit runtime", () => { + test("allows exactly the configured number of auto-submitted iterations", () => { + const config = parseLoopLimitArgs("3"); + expect(config).toEqual({ kind: "iterations", iterations: 3 }); + if (!config || typeof config === "string") throw new Error("expected parsed config"); + + const limit = createLoopLimitRuntime(config); + expect(consumeLoopLimitIteration(limit)).toBe(true); + expect(consumeLoopLimitIteration(limit)).toBe(true); + expect(consumeLoopLimitIteration(limit)).toBe(true); + expect(consumeLoopLimitIteration(limit)).toBe(false); + expect(limit).toEqual({ kind: "iterations", initial: 3, remaining: 0 }); + }); + + test("stops duration-limited loops at the configured deadline", () => { + const config = parseLoopLimitArgs("10m"); + expect(config).toEqual({ kind: "duration", durationMs: 600_000 }); + if (!config || typeof config === "string") throw new Error("expected parsed config"); + + const limit = createLoopLimitRuntime(config, 1_000); + expect(consumeLoopLimitIteration(limit, 600_999)).toBe(true); + expect(isLoopDurationExpired(limit, 600_999)).toBe(false); + expect(consumeLoopLimitIteration(limit, 601_000)).toBe(false); + expect(isLoopDurationExpired(limit, 601_000)).toBe(true); + }); +});