diff --git a/crates/pi-natives/src/text.rs b/crates/pi-natives/src/text.rs index 9b92c109a..e1d076ba4 100644 --- a/crates/pi-natives/src/text.rs +++ b/crates/pi-natives/src/text.rs @@ -1431,7 +1431,8 @@ mod tests { #[test] fn test_wrap_text_with_ansi_resets_strike_without_resetting_colors() { - let data = to_u16("\x1b[38;5;196m\x1b[48;5;236m\x1b[9mstrikethrough content wraps\x1b[29m\x1b[0m"); + let data = + to_u16("\x1b[38;5;196m\x1b[48;5;236m\x1b[9mstrikethrough content wraps\x1b[29m\x1b[0m"); let lines = wrap_text_with_ansi_impl(&data, 12, DEFAULT_TAB_WIDTH); assert!(lines.len() > 1); diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index f40dbbe1a..06fba7dc7 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,8 +1,10 @@ # Changelog ## [Unreleased] + ### Added +- Enforced tool decision in plan mode—agent now requires calling either `ask` or `exit_plan_mode` when a turn ends without a required tool call - Auto-correction of escaped tab indentation in edits (enabled by default, controllable via `PI_HASHLINE_AUTOCORRECT_ESCAPED_TABS` environment variable) - Warning when suspicious Unicode escape placeholder `\uDDDD` is detected in edit content @@ -10,6 +12,10 @@ - Updated hashline documentation to clarify that `\t` in JSON represents a real tab character, not a literal backslash-t sequence +### Fixed + +- Cancelling the `ask` tool now aborts the current turn instead of returning a normal cancelled selection, while timeout-driven auto-cancel still returns without aborting + ## [13.5.2] - 2026-03-01 ### Added diff --git a/packages/coding-agent/src/config/prompt-templates.ts b/packages/coding-agent/src/config/prompt-templates.ts index ed52e2c0f..a059d6a71 100644 --- a/packages/coding-agent/src/config/prompt-templates.ts +++ b/packages/coding-agent/src/config/prompt-templates.ts @@ -255,7 +255,8 @@ handlebars.registerHelper("SECTION_SEPERATOR", (name: unknown): string => sectio */ function formatHashlineRef(lineNum: unknown, content: unknown): { num: number; text: string; ref: string } { const num = typeof lineNum === "number" ? lineNum : Number.parseInt(String(lineNum), 10); - const text = typeof content === "string" ? content : String(content ?? ""); + const raw = typeof content === "string" ? content : String(content ?? ""); + const text = raw.replace(/\\t/g, "\t").replace(/\\n/g, "\n").replace(/\\r/g, "\r"); const ref = `${num}#${computeLineHash(num, text)}`; return { num, text, ref }; } diff --git a/packages/coding-agent/src/prompts/system/plan-mode-tool-decision-reminder.md b/packages/coding-agent/src/prompts/system/plan-mode-tool-decision-reminder.md new file mode 100644 index 000000000..b6e500c56 --- /dev/null +++ b/packages/coding-agent/src/prompts/system/plan-mode-tool-decision-reminder.md @@ -0,0 +1,9 @@ + +Plan mode turn ended without a required tool call. + +You **MUST** choose exactly one next action now: +1. Call `{{askToolName}}` to gather required clarification, OR +2. Call `{{exitToolName}}` to finish planning and request approval + +You **MUST NOT** output plain text in this turn. + \ No newline at end of file diff --git a/packages/coding-agent/src/prompts/tools/hashline.md b/packages/coding-agent/src/prompts/tools/hashline.md index e6a12a99c..c4f738965 100644 --- a/packages/coding-agent/src/prompts/tools/hashline.md +++ b/packages/coding-agent/src/prompts/tools/hashline.md @@ -49,6 +49,7 @@ Every edit has `op`, `pos`, and `lines`. Range replaces also have `end`. Both `p - You **MUST NOT** set `end` to an interior line and then re-add the boundary token in `lines`; that duplicates the next surviving line. - To remove a line while keeping its neighbors, **delete** it (`lines: null`). You **MUST NOT** replace it with the content of an adjacent line — that line still exists and will be duplicated. 4. **Match surrounding indentation:** Leading whitespace in `lines` **MUST** be copied verbatim from adjacent lines in the `read` output. Do not infer or reconstruct indentation from memory — count the actual leading spaces on the lines immediately above and below the insertion or replacement point. +5. **Preserve idiomatic sibling spacing:** When inserting declarations between top-level siblings, you **MUST** preserve existing blank-line separators. If siblings are separated by one blank line, include a trailing `""` in `lines` so inserted code keeps the same spacing. @@ -121,18 +122,17 @@ Range — add `end`: {{hlinefull 62 " return null;"}} {{hlinefull 63 " }"}} ``` +Target only the inner lines that change — leave unchanged boundaries out of the range. ``` { path: "…", edits: [{ op: "replace", - pos: {{hlinejsonref 60 " } catch (err) {"}}, - end: {{hlinejsonref 63 " }"}}, + pos: {{hlinejsonref 61 " console.error(err);"}}, + end: {{hlinejsonref 62 " return null;"}}, lines: [ - " } catch (err) {", " if (isEnoent(err)) return null;", - " throw err;", - " }" + " throw err;" ] }] } @@ -141,39 +141,24 @@ Range — add `end`: ```ts -{{hlinefull 70 "if (ok) {"}} -{{hlinefull 71 " run();"}} -{{hlinefull 72 "}"}} -{{hlinefull 73 "after();"}} +{{hlinefull 70 "\tif (user.isAdmin) {"}} +{{hlinefull 71 "\t\tdeleteRecord(id);"}} +{{hlinefull 72 "\t}"}} +{{hlinefull 73 "\tafter();"}} ``` -Bad — `end` stops before `}` while `lines` already includes `}`: +The block grows by one line and the condition changes — two single-line ops would be needed otherwise. Since `}` appears in `lines`, `end` must include `72`: ``` { path: "…", edits: [{ op: "replace", - pos: {{hlinejsonref 70 "if (ok) {"}}, - end: {{hlinejsonref 71 " run();"}}, + pos: {{hlinejsonref 70 "\tif (user.isAdmin) {"}}, + end: {{hlinejsonref 72 "\t}"}}, lines: [ - "if (ok) {", - " runSafe();", - "}" - ] - }] -} -``` -Good — include original `}` in the replaced range when replacement keeps `}`: -``` -{ - path: "…", - edits: [{ - op: "replace", - pos: {{hlinejsonref 70 "if (ok) {"}}, - end: {{hlinejsonref 72 "}"}}, - lines: [ - "if (ok) {", - " runSafe();", - "}" + "\tif (user.isAdmin && confirmed) {", + "\t\tauditLog(id);", + "\t\tdeleteRecord(id);", + "\t}" ] }] } @@ -191,6 +176,7 @@ Also apply the same rule to `);`, `],`, and `},` closers: if replacement include {{hlinefull 49 " runY();"}} {{hlinefull 50 "}"}} ``` +Use a trailing `""` to preserve the blank line between top-level sibling declarations. ``` { path: "…", @@ -206,20 +192,6 @@ Also apply the same rule to `);`, `],`, and `},` closers: if replacement include }] } ``` -Result: -```ts -{{hlinefull 44 "function x() {"}} -{{hlinefull 45 " runX();"}} -{{hlinefull 46 "}"}} -{{hlinefull 47 ""}} -{{hlinefull 48 "function z() {"}} -{{hlinefull 49 " runZ();"}} -{{hlinefull 50 "}"}} -{{hlinefull 51 ""}} -{{hlinefull 52 "function y() {"}} -{{hlinefull 53 " runY();"}} -{{hlinefull 54 "}"}} -``` @@ -255,7 +227,7 @@ Leading whitespace in `lines` **MUST** be copied from the `read` output, not rec {{hlinefull 11 "\tbar() {"}} {{hlinefull 12 "\t\treturn 1;"}} {{hlinefull 13 "\t}"}} -{{hlinefull 14 "}}"}} +{{hlinefull 14 "}"}} ``` Bad — indent guessed as spaces; `\\t` emits literal backslash-t: ``` diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 015490322..496241038 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -82,6 +82,9 @@ import { normalizeDiff, normalizeToLF, ParseError, previewPatch, stripBom } from import type { PlanModeState } from "../plan-mode/state"; import planModeActivePrompt from "../prompts/system/plan-mode-active.md" with { type: "text" }; import planModeReferencePrompt from "../prompts/system/plan-mode-reference.md" with { type: "text" }; +import planModeToolDecisionReminderPrompt from "../prompts/system/plan-mode-tool-decision-reminder.md" with { + type: "text", +}; import ttsrInterruptTemplate from "../prompts/system/ttsr-interrupt.md" with { type: "text" }; import type { SecretObfuscator } from "../secrets/obfuscator"; import type { CheckpointState } from "../tools/checkpoint"; @@ -733,9 +736,13 @@ export class AgentSession { } // Check auto-retry and auto-compaction after agent completes - if (event.type === "agent_end" && this.#lastAssistantMessage) { - const msg = this.#lastAssistantMessage; + if (event.type === "agent_end") { + const fallbackAssistant = [...event.messages] + .reverse() + .find((message): message is AssistantMessage => message.role === "assistant"); + const msg = this.#lastAssistantMessage ?? fallbackAssistant; this.#lastAssistantMessage = undefined; + if (!msg) return; if (this.#skipPostTurnMaintenanceAssistantTimestamp === msg.timestamp) { this.#skipPostTurnMaintenanceAssistantTimestamp = undefined; @@ -1901,6 +1908,9 @@ export class AgentSession { : { role: "user" as const, content: userContent, timestamp: Date.now() }; await this.#promptWithMessage(message, expandedText, options); + if (!options?.synthetic) { + await this.#enforcePlanModeToolDecision(); + } } async promptCustomMessage( @@ -3329,6 +3339,52 @@ Be thorough - include exact file paths, function names, error messages, and tech this.#checkpointState = undefined; this.#pendingRewindReport = undefined; } + async #enforcePlanModeToolDecision(): Promise { + if (!this.#planModeState?.enabled) { + return; + } + const assistantMessage = this.#findLastAssistantMessage(); + if (!assistantMessage) { + return; + } + if (assistantMessage.stopReason === "error" || assistantMessage.stopReason === "aborted") { + return; + } + + const calledRequiredTool = assistantMessage.content.some( + content => content.type === "toolCall" && (content.name === "ask" || content.name === "exit_plan_mode"), + ); + if (calledRequiredTool) { + return; + } + + const askTool = this.#toolRegistry.get("ask"); + const exitPlanModeTool = this.#toolRegistry.get("exit_plan_mode"); + if (!askTool || !exitPlanModeTool) { + logger.warn("Plan mode enforcement skipped because ask/exit tools are unavailable", { + activeToolNames: this.agent.state.tools.map(tool => tool.name), + }); + return; + } + const forcedTools = [askTool, exitPlanModeTool]; + + const reminder = renderPromptTemplate(planModeToolDecisionReminderPrompt, { + askToolName: "ask", + exitToolName: "exit_plan_mode", + }); + + const previousTools = this.agent.state.tools; + this.agent.setTools(forcedTools); + try { + await this.prompt(reminder, { + synthetic: true, + expandPromptTemplates: false, + toolChoice: "required", + }); + } finally { + this.agent.setTools(previousTools); + } + } /** * Check if agent stopped with incomplete todos and prompt to continue. */ diff --git a/packages/coding-agent/src/utils/prompt-format.ts b/packages/coding-agent/src/utils/prompt-format.ts index ba477cd65..5f387a847 100644 --- a/packages/coding-agent/src/utils/prompt-format.ts +++ b/packages/coding-agent/src/utils/prompt-format.ts @@ -86,9 +86,10 @@ export function formatPromptContent(content: string, options: PromptFormatOption for (let i = 0; i < lines.length; i++) { let line = lines[i].trimEnd(); - const trimmed = line.trimStart(); + const trimmed = line.trim(); + const trimmedStart = line.trimStart(); - if (CODE_FENCE.test(trimmed)) { + if (CODE_FENCE.test(trimmedStart)) { inCodeBlock = !inCodeBlock; result.push(line); continue; @@ -103,29 +104,26 @@ export function formatPromptContent(content: string, options: PromptFormatOption line = replaceCommonAsciiSymbols(line); } - const isOpeningXml = OPENING_XML.test(trimmed) && !trimmed.endsWith("/>"); - if (isOpeningXml && line.length === trimmed.length) { - const match = OPENING_XML.exec(trimmed); + const isOpeningXml = OPENING_XML.test(trimmedStart) && !trimmedStart.endsWith("/>"); + if (isOpeningXml && line.length === trimmedStart.length) { + const match = OPENING_XML.exec(trimmedStart); if (match) topLevelTags.push(match[1]); } - const closingMatch = CLOSING_XML.exec(trimmed); + const closingMatch = CLOSING_XML.exec(trimmedStart); if (closingMatch) { const tagName = closingMatch[1]; if (topLevelTags.length > 0 && topLevelTags[topLevelTags.length - 1] === tagName) { - line = trimmed; topLevelTags.pop(); - } else { - line = line.trimEnd(); } - } else if (isPreRender && trimmed.startsWith("{{")) { - line = trimmed; - } else if (TABLE_SEP.test(trimmed)) { - line = compactTableSep(trimmed); - } else if (TABLE_ROW.test(trimmed)) { - line = compactTableRow(trimmed); - } else { - line = line.trimEnd(); + } else if (isPreRender && trimmedStart.startsWith("{{")) { + /* keep indentation as-is in pre-render for Handlebars markers */ + } else if (TABLE_SEP.test(trimmedStart)) { + const leadingWhitespace = line.slice(0, line.length - trimmedStart.length); + line = `${leadingWhitespace}${compactTableSep(trimmedStart)}`; + } else if (TABLE_ROW.test(trimmedStart)) { + const leadingWhitespace = line.slice(0, line.length - trimmedStart.length); + line = `${leadingWhitespace}${compactTableRow(trimmedStart)}`; } if (shouldBoldRfc2119) { diff --git a/packages/coding-agent/test/prompt-format.test.ts b/packages/coding-agent/test/prompt-format.test.ts index 6afbdeaaf..51ee5c72a 100644 --- a/packages/coding-agent/test/prompt-format.test.ts +++ b/packages/coding-agent/test/prompt-format.test.ts @@ -2,12 +2,28 @@ import { describe, expect, test } from "bun:test"; import { formatPromptContent } from "@oh-my-pi/pi-coding-agent/utils/prompt-format"; describe("formatPromptContent renderPhase", () => { - test("pre-render mode strips indentation from Handlebars block lines", () => { + test("pre-render preserves indentation on Handlebars block lines", () => { const input = "\n {{#if ok}}\n value\n {{/if}}\n"; const output = formatPromptContent(input, { renderPhase: "pre-render" }); - expect(output).toBe("\n{{#if ok}}\n value\n{{/if}}\n"); + expect(output).toBe("\n {{#if ok}}\n value\n {{/if}}\n"); + }); + + test("pre-render preserves leading tabs", () => { + const input = "\t\n\t {{#if ok}}\n\t value\n\t {{/if}}\n"; + + const output = formatPromptContent(input, { renderPhase: "pre-render" }); + + expect(output).toBe(input); + }); + + test("pre-render trims trailing whitespace", () => { + const input = "\t \n\t {{#if ok}}\t\n\t value \n\t {{/if}} \n"; + + const output = formatPromptContent(input, { renderPhase: "pre-render" }); + + expect(output).toBe("\t\n\t {{#if ok}}\n\t value\n\t {{/if}}\n"); }); test("post-render mode preserves indentation on Handlebars-like lines", () => {