diff --git a/docs/skills/authoring-hooks.md b/docs/skills/authoring-hooks.md index 17d518478..5f155e722 100644 --- a/docs/skills/authoring-hooks.md +++ b/docs/skills/authoring-hooks.md @@ -245,12 +245,14 @@ export default function contextFilter(omp: HookAPI): void { | `select(title, options)` | Show a selection dialog | | `confirm(title, message)` | Show a yes/no dialog | | `input(title, placeholder?)` | Show a text input dialog | -| `editor(title, prefill?, { signal }?)` | Show a multi-line editor | +| `editor(title, prefill?, { signal }?, { promptStyle }?)` | Show a multi-line editor | | `setEditorText(text)` | Set the input editor content | | `getEditorText()` | Get current input editor content | | `custom(factory)` | Render a custom TUI component | | `theme` | Current theme object | +Pass `{ promptStyle: true }` as the fourth argument when Enter should submit and Shift+Enter should insert a newline. The default hook editor behavior keeps Enter as newline and Ctrl+Enter as submit. + `ctx.hasUI` is `false` in headless/print/subagent mode — always guard interactive calls. ## Further reading diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 5bc1e5b5f..15a97bbc9 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -146,6 +146,12 @@ - Fixed CLI PDF file arguments being decoded as raw bytes for local vision models; `.pdf` and other supported document files now go through the same Markit conversion path as the `read` tool before entering the prompt ([#1401](https://github.com/can1357/oh-my-pi/issues/1401)). - Fixed the `bash` tool hanging until the 305 s hard timeout when a command writes a file via heredoc on Windows (bodies > ~4 KiB) or macOS (bodies > 16-64 KiB). Root cause was in the embedded brush shell; see `@oh-my-pi/pi-natives` changelog for the underlying fix. +### Fixed + +- Fixed `/review` custom prompt orchestration text to use static prompt templates and consistently instruct reviewer task delegation. +- Fixed `/review` custom-instructions submission on terminals that cannot distinguish Ctrl+Enter by using prompt-style input where Enter submits and Shift+Enter inserts a newline. +- Fixed hook editor submissions sending large-paste placeholders such as `[paste #1 +27 lines]` instead of the pasted content. + ## [15.3.2] - 2026-05-25 ### Added @@ -8740,4 +8746,4 @@ Initial public release. - Git branch display in footer - Message queueing during streaming responses - OAuth integration for Gmail and Google Calendar access -- HTML export with syntax highlighting and collapsible sections \ No newline at end of file +- HTML export with syntax highlighting and collapsible sections diff --git a/packages/coding-agent/src/extensibility/custom-commands/bundled/review/index.ts b/packages/coding-agent/src/extensibility/custom-commands/bundled/review/index.ts index 1272c98c2..d97d6d63e 100644 --- a/packages/coding-agent/src/extensibility/custom-commands/bundled/review/index.ts +++ b/packages/coding-agent/src/extensibility/custom-commands/bundled/review/index.ts @@ -14,6 +14,8 @@ import { prompt } from "@oh-my-pi/pi-utils"; import type { CustomCommand, CustomCommandAPI } from "../../../../extensibility/custom-commands/types"; import type { HookCommandContext } from "../../../../extensibility/hooks/types"; +import reviewCustomRequestTemplate from "../../../../prompts/review-custom-request.md" with { type: "text" }; +import reviewHeadlessRequestTemplate from "../../../../prompts/review-headless-request.md" with { type: "text" }; import reviewRequestTemplate from "../../../../prompts/review-request.md" with { type: "text" }; import * as git from "../../../../utils/git"; @@ -225,6 +227,14 @@ function buildReviewPrompt(mode: string, stats: DiffStats, rawDiff: string, addi }); } +function buildCustomReviewPrompt(instructions: string): string { + return prompt.render(reviewCustomRequestTemplate, { instructions }); +} + +function buildHeadlessReviewPrompt(focus?: string): string { + return prompt.render(reviewHeadlessRequestTemplate, { focus }); +} + export class ReviewCommand implements CustomCommand { name = "review"; description = "Launch interactive code review"; @@ -233,8 +243,7 @@ export class ReviewCommand implements CustomCommand { async execute(args: string[], ctx: HookCommandContext): Promise { if (!ctx.hasUI) { - const base = "Use the Task tool to run the 'reviewer' agent to review recent code changes."; - return args.length > 0 ? `${base} Focus: ${args.join(" ")}` : base; + return buildHeadlessReviewPrompt(args.length > 0 ? args.join(" ") : undefined); } // Inline args act as additional instructions appended to the generated prompt. @@ -379,7 +388,12 @@ export class ReviewCommand implements CustomCommand { case 4: { // Custom instructions - still uses the old approach since user provides context - const instructions = await ctx.ui.editor("Enter custom review instructions", "Review the following:\n\n"); + const instructions = await ctx.ui.editor( + "Enter custom review instructions", + "Review the following:\n\n", + undefined, + { promptStyle: true }, + ); if (!instructions?.trim()) return undefined; // For custom, we still try to get current diff for context @@ -402,17 +416,7 @@ export class ReviewCommand implements CustomCommand { ); } - // No diff available, just pass instructions - return `## Code Review Request - -### Mode -Custom review instructions - -### Instructions - -${instructions} - -Use the Task tool with \`agent: "reviewer"\` to execute this review.`; + return buildCustomReviewPrompt(instructions); } default: diff --git a/packages/coding-agent/src/extensibility/hooks/types.ts b/packages/coding-agent/src/extensibility/hooks/types.ts index e2eee2165..5f762fb3b 100644 --- a/packages/coding-agent/src/extensibility/hooks/types.ts +++ b/packages/coding-agent/src/extensibility/hooks/types.ts @@ -139,9 +139,16 @@ export interface HookUIContext { * Supports Ctrl+G to open external editor ($VISUAL or $EDITOR). * @param title - Title describing what is being edited * @param prefill - Optional initial text + * @param options - Optional dialog controls such as an abort signal + * @param editorOptions - Optional editor behavior; `promptStyle` makes Enter submit and Shift+Enter insert a newline * @returns Edited text, or undefined if cancelled (Escape) */ - editor(title: string, prefill?: string, options?: { signal?: AbortSignal }): Promise; + editor( + title: string, + prefill?: string, + options?: { signal?: AbortSignal }, + editorOptions?: { promptStyle?: boolean }, + ): Promise; /** * Get the current theme for styling text with ANSI codes. diff --git a/packages/coding-agent/src/modes/components/hook-editor.ts b/packages/coding-agent/src/modes/components/hook-editor.ts index e175b2e26..fe0de86a0 100644 --- a/packages/coding-agent/src/modes/components/hook-editor.ts +++ b/packages/coding-agent/src/modes/components/hook-editor.ts @@ -17,6 +17,10 @@ export interface HookEditorOptions { promptStyle?: boolean; } +function isCtrlEnterSubmit(keyData: string): boolean { + return matchesKey(keyData, "ctrl+enter") || (keyData.charCodeAt(0) === 10 && keyData.length > 1); +} + export class HookEditorComponent extends Container { #editor: Editor; #onSubmitCallback: (value: string) => void; @@ -78,6 +82,10 @@ export class HookEditorComponent extends Container { } } + #submitCurrentText(): void { + this.#onSubmitCallback(this.#editor.getExpandedText()); + } + /** Prompt-style: raw Enter submits; Editor owns newline-producing sequences. */ #handlePromptStyleInput(keyData: string): void { // Prompt-style keeps Escape as an explicit cancel key and also honors app.interrupt remaps. @@ -94,7 +102,7 @@ export class HookEditorComponent extends Container { // Submit on any plain Enter encoding, including terminals that report unmodified Enter as LF. if (matchesKey(keyData, "enter") || matchesKey(keyData, "return")) { - this.#onSubmitCallback(this.#editor.getText()); + this.#submitCurrentText(); return; } @@ -105,8 +113,8 @@ export class HookEditorComponent extends Container { /** Hook-style: Enter=newline, Ctrl+Enter=submit (original behavior) */ #handleHookStyleInput(keyData: string): void { // Ctrl+Enter to submit. Use key matching so lock-key and keypad Enter variants work. - if (matchesKey(keyData, "ctrl+enter")) { - this.#onSubmitCallback(this.#editor.getText()); + if (isCtrlEnterSubmit(keyData)) { + this.#submitCurrentText(); return; } diff --git a/packages/coding-agent/src/prompts/review-custom-request.md b/packages/coding-agent/src/prompts/review-custom-request.md new file mode 100644 index 000000000..19bb5c306 --- /dev/null +++ b/packages/coding-agent/src/prompts/review-custom-request.md @@ -0,0 +1,22 @@ +## Code Review Request + +### Mode + +Custom review instructions + +### Distribution Guidelines + +Use the `task` tool with `agent: "reviewer"` and a `tasks` array. +Create exactly **1 reviewer task**. Its assignment must include the custom instructions below. + +### Reviewer Instructions + +Reviewer MUST: +1. Follow the custom instructions below +2. Read the referenced files or workspace context needed to evaluate them +3. Call `report_finding` per issue +4. Call `yield` with verdict when done + +### Custom Instructions + +{{instructions}} diff --git a/packages/coding-agent/src/prompts/review-headless-request.md b/packages/coding-agent/src/prompts/review-headless-request.md new file mode 100644 index 000000000..eb6b27ca0 --- /dev/null +++ b/packages/coding-agent/src/prompts/review-headless-request.md @@ -0,0 +1,16 @@ +## Code Review Request + +### Mode + +Headless review request + +### Distribution Guidelines + +Use the `task` tool with `agent: "reviewer"` and a `tasks` array. +Create exactly **1 reviewer task** for recent code changes. + +{{#if focus}} +### Focus + +{{focus}} +{{/if}} diff --git a/packages/coding-agent/src/prompts/review-request.md b/packages/coding-agent/src/prompts/review-request.md index e5dd95bb1..039fea7c1 100644 --- a/packages/coding-agent/src/prompts/review-request.md +++ b/packages/coding-agent/src/prompts/review-request.md @@ -23,14 +23,13 @@ _No files to review._ ### Distribution Guidelines -{{#when agentCount "==" 1}}Use **1 reviewer agent**.{{else}}Spawn **{{agentCount}} reviewer agents** in parallel.{{/when}} +Use the `task` tool with `agent: "reviewer"` and a `tasks` array. +{{#when agentCount "==" 1}}Create exactly **1 reviewer task**.{{else}}Spawn **{{agentCount}} reviewer agents** in parallel.{{/when}} {{#if multiAgent}} Group files by locality, e.g.: - Same directory/module → same agent - Related functionality → same agent - Tests with their implementation files → same agent - -You MUST use Task tool with `agent: "reviewer"` and `tasks` array. {{/if}} ### Reviewer Instructions diff --git a/packages/coding-agent/test/extensibility/custom-commands/review.test.ts b/packages/coding-agent/test/extensibility/custom-commands/review.test.ts new file mode 100644 index 000000000..65e71e5df --- /dev/null +++ b/packages/coding-agent/test/extensibility/custom-commands/review.test.ts @@ -0,0 +1,153 @@ +import { afterEach, describe, expect, it } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { $ } from "bun"; +import { ReviewCommand } from "../../../src/extensibility/custom-commands/bundled/review"; +import type { CustomCommandAPI } from "../../../src/extensibility/custom-commands/types"; +import type { HookCommandContext } from "../../../src/extensibility/hooks/types"; + +const LEGACY_TASK_INSTRUCTION = 'Use the Task tool with `agent: "reviewer"` to execute this review.'; +const REVIEWER_TASK_INSTRUCTION = 'Use the `task` tool with `agent: "reviewer"` and a `tasks` array.'; + +interface EditorCall { + title: string; + prefill: string | undefined; + editorOptions: { promptStyle?: boolean } | undefined; +} + +describe("ReviewCommand", () => { + let tmpDir: string | undefined; + + afterEach(async () => { + if (tmpDir) { + await fs.rm(tmpDir, { recursive: true, force: true }); + tmpDir = undefined; + } + }); + + async function createTempDir(): Promise { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-review-command-")); + return tmpDir; + } + + async function createGitRepoWithUncommittedChange(): Promise { + const dir = await createTempDir(); + await $`git init`.cwd(dir).quiet(); + await $`git config user.name Omp Test`.cwd(dir).quiet(); + await $`git config user.email omp-test@example.com`.cwd(dir).quiet(); + await Bun.write(path.join(dir, "review-target.ts"), "export const value = 1;\n"); + await $`git add review-target.ts`.cwd(dir).quiet(); + await $`git commit -m initial`.cwd(dir).quiet(); + await Bun.write(path.join(dir, "review-target.ts"), "export const value = 2;\n"); + return dir; + } + + function createContext(options?: { + selectedMode?: string; + editorValue?: string | undefined; + onEditorCall?: (call: EditorCall) => void; + }): HookCommandContext { + return { + hasUI: true, + ui: { + select: () => Promise.resolve(options?.selectedMode ?? "4. Custom review instructions"), + editor: ( + title: string, + prefill?: string, + _options?: { signal?: AbortSignal }, + editorOptions?: { promptStyle?: boolean }, + ) => { + options?.onEditorCall?.({ title, prefill, editorOptions }); + return Promise.resolve(options?.editorValue); + }, + notify: () => {}, + }, + } as unknown as HookCommandContext; + } + + it("uses prompt-style input for custom review instructions", async () => { + const dir = await createTempDir(); + let editorCall: EditorCall | undefined; + + const command = new ReviewCommand({ cwd: dir } as unknown as CustomCommandAPI); + const ctx = createContext({ + editorValue: "Check authentication boundaries", + onEditorCall: call => { + editorCall = call; + }, + }); + + const result = await command.execute([], ctx); + + expect(editorCall).toEqual({ + title: "Enter custom review instructions", + prefill: "Review the following:\n\n", + editorOptions: { promptStyle: true }, + }); + expect(result).toContain("Check authentication boundaries"); + }); + + it("renders custom review instructions through the reviewer task prompt when no diff is available", async () => { + const dir = await createTempDir(); + const command = new ReviewCommand({ cwd: dir } as unknown as CustomCommandAPI); + const ctx = createContext({ + editorValue: "Check authentication boundaries", + }); + + const result = await command.execute([], ctx); + + expect(result).toBeDefined(); + const promptText = result!; + expect(promptText).toContain("Custom review instructions"); + expect(promptText).toContain(REVIEWER_TASK_INSTRUCTION); + expect(promptText).toContain("Check authentication boundaries"); + expect(promptText).not.toContain(LEGACY_TASK_INSTRUCTION); + }); + + it("does not submit empty custom review instructions", async () => { + const values = [undefined, "", " \n\t "]; + + for (const editorValue of values) { + const dir = await createTempDir(); + const command = new ReviewCommand({ cwd: dir } as unknown as CustomCommandAPI); + const ctx = createContext({ editorValue }); + + const result = await command.execute([], ctx); + + expect(result).toBeUndefined(); + await fs.rm(dir, { recursive: true, force: true }); + tmpDir = undefined; + } + }); + + it("includes reviewer task orchestration for single-agent diff reviews", async () => { + const dir = await createGitRepoWithUncommittedChange(); + const command = new ReviewCommand({ cwd: dir } as unknown as CustomCommandAPI); + const ctx = createContext({ + selectedMode: "2. Review uncommitted changes", + }); + + const result = await command.execute([], ctx); + + expect(result).toBeDefined(); + const promptText = result!; + expect(promptText).toContain(REVIEWER_TASK_INSTRUCTION); + expect(promptText).toContain("Create exactly **1 reviewer task**"); + expect(promptText).not.toContain(LEGACY_TASK_INSTRUCTION); + }); + + it("renders headless review requests through the reviewer task prompt", async () => { + const command = new ReviewCommand({ cwd: "/tmp" } as unknown as CustomCommandAPI); + const ctx = { hasUI: false } as unknown as HookCommandContext; + + const result = await command.execute(["focus", "auth"], ctx); + + expect(result).toBeDefined(); + const promptText = result!; + expect(promptText).toContain("Headless review request"); + expect(promptText).toContain(REVIEWER_TASK_INSTRUCTION); + expect(promptText).toContain("focus auth"); + expect(promptText).not.toContain(LEGACY_TASK_INSTRUCTION); + }); +}); diff --git a/packages/coding-agent/test/hook-editor.test.ts b/packages/coding-agent/test/hook-editor.test.ts index 32cd69e83..e21752f43 100644 --- a/packages/coding-agent/test/hook-editor.test.ts +++ b/packages/coding-agent/test/hook-editor.test.ts @@ -37,6 +37,10 @@ function renderLines(component: HookEditorComponent, width = 120): string[] { return Bun.stripANSI(component.render(width).join("\n")).split("\n"); } +function largePasteText(): string { + return Array.from({ length: 11 }, (_, index) => `pasted line ${index + 1}`).join("\n"); +} + type TestContext = InteractiveModeContext & { editorContainer: { children: unknown[]; @@ -126,6 +130,35 @@ describe("HookEditorComponent default (hook) mode", () => { } }); + it("submits LF-prefixed modified Enter sequences", () => { + const onSubmit = vi.fn(); + const onCancel = vi.fn(); + const component = new HookEditorComponent(createTui(), "Prompt", "draft", onSubmit, onCancel); + + component.handleInput("\n\x1b[13;5u"); + + expect(onSubmit).toHaveBeenCalledTimes(1); + expect(onSubmit).toHaveBeenCalledWith("draft"); + expect(onCancel).not.toHaveBeenCalled(); + }); + + it("expands large paste markers when submitting on Ctrl+Enter", () => { + const onSubmit = vi.fn(); + const onCancel = vi.fn(); + const component = new HookEditorComponent(createTui(), "Prompt", undefined, onSubmit, onCancel); + const pasted = largePasteText(); + + component.handleInput(`\x1b[200~${pasted}\x1b[201~`); + + expect(renderText(component)).toContain("[paste #1 +11 lines]"); + + component.handleInput("\x1b[13;5u"); + + expect(onSubmit).toHaveBeenCalledTimes(1); + expect(onSubmit).toHaveBeenCalledWith(pasted); + expect(onCancel).not.toHaveBeenCalled(); + }); + it("cancels on Escape", () => { const onSubmit = vi.fn(); const onCancel = vi.fn(); @@ -185,6 +218,25 @@ describe("HookEditorComponent prompt-style mode", () => { expect(onCancel).not.toHaveBeenCalled(); }); + it("expands large paste markers when submitting on Enter", () => { + const onSubmit = vi.fn(); + const onCancel = vi.fn(); + const component = new HookEditorComponent(createTui(), "Prompt", undefined, onSubmit, onCancel, { + promptStyle: true, + }); + const pasted = largePasteText(); + + component.handleInput(`\x1b[200~${pasted}\x1b[201~`); + + expect(renderText(component)).toContain("[paste #1 +11 lines]"); + + component.handleInput("\r"); + + expect(onSubmit).toHaveBeenCalledTimes(1); + expect(onSubmit).toHaveBeenCalledWith(pasted); + expect(onCancel).not.toHaveBeenCalled(); + }); + it("inserts newline on Shift+Enter instead of submitting", () => { const onSubmit = vi.fn(); const onCancel = vi.fn();