From ee661cb418e389a392c9c6296fef47032273ee8e Mon Sep 17 00:00:00 2001 From: oldschoola Date: Thu, 18 Jun 2026 19:12:19 -0700 Subject: [PATCH] fix(coding-agent): handle todo paths and prompt inventory --- packages/coding-agent/CHANGELOG.md | 2 + .../controllers/todo-command-controller.ts | 19 +-- packages/coding-agent/src/sdk.ts | 3 + .../src/slash-commands/helpers/todo.ts | 18 +- packages/coding-agent/src/system-prompt.ts | 17 +- packages/coding-agent/src/tools/todo.ts | 6 + .../coding-agent/test/acp-builtins.test.ts | 115 +++++++++++++ .../todo-command-controller.test.ts | 154 ++++++++++++++++++ .../test/system-prompt-inventory.test.ts | 136 +++++++++++++++- packages/coding-agent/test/tools/todo.test.ts | 22 +++ 10 files changed, 464 insertions(+), 28 deletions(-) create mode 100644 packages/coding-agent/test/modes/controllers/todo-command-controller.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 31cb43b79..3b51e0c9d 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -134,6 +134,8 @@ - Fixed memory-leaking stale transcripts in the agent viewer when underlying files are deleted - Fixed the Agent Hub transcript viewer rendering the transcript body one column right of the "Agent Hub" title (and the title appearing to shift when scrolled to the top): the fullscreen viewer added its own outer gutter on top of the transcript rows, which already carry a 1-column left pad, so the header and body no longer shared a gutter. The viewer now renders the scroll body at full width without the extra gutter, and the file-mention row carries the same 1-column pad as every other row. +- Fixed `/todo export` and `/todo import` in ACP/text and TUI modes to resolve paths from the active session cwd, accept quoted paths with spaces, and report invalid path schemes without crashing. +- Fixed the SDK `buildSystemPrompt` helper to render caller-provided tools instead of falling back to the default prompt inventory, and to keep no-tools-map skills visible when `read` is available through the fallback. - Fixed the bash tool failing with `pi-natives:command: syntax error at end of input` on a valid `&&`/`;` chain whose later pipeline stage is a compound command, e.g. `echo x && git log | while read h; do …; done | head`. The minimizer's segmented-chain runner rebuilds each chain segment from the brush AST via `pipeline.to_string()` and re-executes that string, but `simple_segment` only validated the *first* pipeline stage — so a compound later stage (`while`/`for`/`if`/subshell) was re-serialized without its terminator and re-run as broken shell. Every stage is now required to be a Display-safe simple command, and — as a general guard against the recurring class of brush `Display` round-trip divergences (previously: quoted here-doc close tags, multi-byte char/byte offsets) — each reconstructed segment is now re-parsed and must match the original pipeline shape before the chain runner executes it; any divergence runs the command whole, unsegmented, instead of corrupting it. - Fixed `Ctrl+T` (toggle thinking blocks) and the `/settings` "Hide Thinking Blocks" toggle only collapsing/expanding thinking in the live region: blocks that had scrolled into committed native scrollback on ED3-risk terminals kept their pre-toggle snapshot, so scrolling up showed the old thinking state. Both paths now `resetDisplay()` after flipping each block's flag, forcing a full clear + replay of the whole transcript (matching the tool-output expansion toggle) so every block above the fold re-renders at its new height. - Fixed ACP mobile voice settings being unable to call `speech.models.list` by exposing the local STT/TTS model and voice catalog without triggering setup or downloads ([#3011](https://github.com/can1357/oh-my-pi/issues/3011)). diff --git a/packages/coding-agent/src/modes/controllers/todo-command-controller.ts b/packages/coding-agent/src/modes/controllers/todo-command-controller.ts index b105905a0..3f3855268 100644 --- a/packages/coding-agent/src/modes/controllers/todo-command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/todo-command-controller.ts @@ -1,10 +1,10 @@ import * as fs from "node:fs/promises"; -import { resolveToCwd } from "../../tools/path-utils"; import { applyOpsToPhases, getLatestTodoPhasesFromEntries, markdownToPhases, phasesToMarkdown, + resolveTodoMarkdownPath, type TodoItem, type TodoPhase, USER_TODO_EDIT_CUSTOM_TYPE, @@ -18,8 +18,8 @@ const USAGE = [ " /todo Show current todos", " /todo edit Open todos in $EDITOR", " /todo copy Copy todos as Markdown to clipboard", - " /todo export Write todos as Markdown to ", - " /todo import Replace todos from Markdown at ", + " /todo export [] Write todos to file (default: TODO.md)", + " /todo import [] Replace todos from file (default: TODO.md)", " /todo append [] Append a task; phase fuzzy-matched or auto-created", " /todo start Mark task in_progress (fuzzy content match)", " /todo done [] Mark task/phase/all completed", @@ -214,9 +214,7 @@ export class TodoCommandController { } #resolveTodoPath(rest: string): string { - const trimmed = rest.trim(); - const raw = trimmed || "TODO.md"; - return resolveToCwd(raw, this.ctx.sessionManager.getCwd()); + return resolveTodoMarkdownPath(rest, this.ctx.sessionManager.getCwd()); } async #exportToFile(rest: string): Promise { @@ -225,22 +223,23 @@ export class TodoCommandController { this.ctx.showWarning("No todos to export."); return; } - const target = this.#resolveTodoPath(rest); try { + const target = this.#resolveTodoPath(rest); await fs.writeFile(target, phasesToMarkdown(phases), "utf8"); this.ctx.showStatus(`Wrote todos to ${target}`); } catch (error) { - this.ctx.showError(`Failed to write ${target}: ${error instanceof Error ? error.message : String(error)}`); + this.ctx.showError(`Failed to write todos: ${error instanceof Error ? error.message : String(error)}`); } } async #importFromFile(rest: string): Promise { - const source = this.#resolveTodoPath(rest); + let source = ""; let content: string; try { + source = this.#resolveTodoPath(rest); content = await fs.readFile(source, "utf8"); } catch (error) { - this.ctx.showError(`Failed to read ${source}: ${error instanceof Error ? error.message : String(error)}`); + this.ctx.showError(`Failed to read todos: ${error instanceof Error ? error.message : String(error)}`); return; } const { phases, errors } = markdownToPhases(content); diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index cd2462163..cbf39ecf9 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -805,6 +805,7 @@ export interface BuildSystemPromptOptions { * as separate entries so providers can cache prompt prefixes without concatenating blocks. */ export async function buildSystemPrompt(options: BuildSystemPromptOptions = {}): Promise { + const toolMap = options.tools ? new Map(options.tools.map(tool => [tool.name, tool])) : undefined; return await buildSystemPromptInternal({ cwd: options.cwd, customPrompt: options.customPrompt, @@ -812,6 +813,8 @@ export async function buildSystemPrompt(options: BuildSystemPromptOptions = {}): contextFiles: options.contextFiles, appendSystemPrompt: options.appendPrompt, inlineToolDescriptors: options.inlineToolDescriptors, + toolNames: options.tools?.map(tool => tool.name), + tools: toolMap ? buildSystemPromptToolMetadata(toolMap) : undefined, }); } diff --git a/packages/coding-agent/src/slash-commands/helpers/todo.ts b/packages/coding-agent/src/slash-commands/helpers/todo.ts index 1fb836e7c..25ed0c36d 100644 --- a/packages/coding-agent/src/slash-commands/helpers/todo.ts +++ b/packages/coding-agent/src/slash-commands/helpers/todo.ts @@ -1,14 +1,14 @@ -import * as path from "node:path"; import type { TodoPhase } from "../../tools/todo"; import { applyOpsToPhases, getLatestTodoPhasesFromEntries, markdownToPhases, phasesToMarkdown, + resolveTodoMarkdownPath, USER_TODO_EDIT_CUSTOM_TYPE, } from "../../tools/todo"; import type { ParsedSlashCommand, SlashCommandResult, SlashCommandRuntime } from "../types"; -import { commandConsumed, parseSubcommand, usage } from "./parse"; +import { commandConsumed, errorMessage, parseSubcommand, usage } from "./parse"; type TodoMutationVerb = "done" | "drop" | "rm"; @@ -127,19 +127,25 @@ async function handleTodoExportCommand(restArgs: string, runtime: SlashCommandRu await runtime.output("No todos to export."); return commandConsumed(); } - const target = restArgs ? path.resolve(runtime.cwd, restArgs) : path.resolve(runtime.cwd, "TODO.md"); - await Bun.write(target, phasesToMarkdown(phases)); + let target: string; + try { + target = resolveTodoMarkdownPath(restArgs, runtime.sessionManager.getCwd()); + await Bun.write(target, phasesToMarkdown(phases)); + } catch (err) { + return usage(`Failed to write todos: ${errorMessage(err)}`, runtime); + } await runtime.output(`Wrote todos to ${target}`); return commandConsumed(); } async function handleTodoImportCommand(restArgs: string, runtime: SlashCommandRuntime): Promise { - const target = restArgs ? path.resolve(runtime.cwd, restArgs) : path.resolve(runtime.cwd, "TODO.md"); + let target: string; let content: string; try { + target = resolveTodoMarkdownPath(restArgs, runtime.sessionManager.getCwd()); content = await Bun.file(target).text(); } catch (err) { - return usage(`Failed to read ${target}: ${err instanceof Error ? err.message : String(err)}`, runtime); + return usage(`Failed to read todos: ${errorMessage(err)}`, runtime); } const { phases, errors } = markdownToPhases(content); if (errors.length > 0) return usage(`Could not parse ${target}:\n ${errors.join("\n ")}`, runtime); diff --git a/packages/coding-agent/src/system-prompt.ts b/packages/coding-agent/src/system-prompt.ts index bbeee28e5..9a258d6fc 100644 --- a/packages/coding-agent/src/system-prompt.ts +++ b/packages/coding-agent/src/system-prompt.ts @@ -327,6 +327,8 @@ export async function loadSystemPromptFiles(options: LoadContextFilesOptions = { return userLevel?.content ?? null; } +export const DEFAULT_SYSTEM_PROMPT_TOOL_NAMES = ["read", "bash", "eval", "edit", "write"] as const; + export interface SystemPromptToolMetadata { label: string; description: string; @@ -584,18 +586,11 @@ export async function buildSystemPrompt(options: BuildSystemPromptOptions = {}): const dateTime = date; const promptCwd = shortenPath(resolvedCwd.replace(/\\/g, "/")); - // Build tool metadata for system prompt rendering - // Priority: explicit list > tools map > defaults - // Default includes both bash and python; actual availability determined by settings in createTools + // Build tool metadata for system prompt rendering. + // Priority: explicit list > tools map > conservative SDK fallback. let toolNames = providedToolNames; if (!toolNames) { - if (tools) { - // Tools map provided - toolNames = Array.from(tools.keys()); - } else { - // Use defaults - toolNames = ["read", "bash", "eval", "edit", "write"]; // TODO: Why? - } + toolNames = tools ? Array.from(tools.keys()) : [...DEFAULT_SYSTEM_PROMPT_TOOL_NAMES]; } // Build tool descriptions for system prompt rendering. @@ -625,7 +620,7 @@ export async function buildSystemPrompt(options: BuildSystemPromptOptions = {}): // Filter skills for the rendered system prompt: // - require the `read` tool so the model can actually fetch skill content; // - drop skills with frontmatter `hide: true` (still loadable via skill:// and /skill:). - const hasRead = tools?.has("read"); + const hasRead = toolNames.includes("read"); const filteredSkills = hasRead ? skills.filter(skill => skill.hide !== true) : []; const effectiveSystemPromptCustomization = dedupePromptSource(systemPromptCustomization, [ diff --git a/packages/coding-agent/src/tools/todo.ts b/packages/coding-agent/src/tools/todo.ts index f38c3f3a1..04a343feb 100644 --- a/packages/coding-agent/src/tools/todo.ts +++ b/packages/coding-agent/src/tools/todo.ts @@ -11,6 +11,7 @@ import todoDescription from "../prompts/tools/todo.md" with { type: "text" }; import type { ToolSession } from "../sdk"; import type { SessionEntry } from "../session/session-entries"; import { framedBlock, renderStatusLine, renderTreeList } from "../tui"; +import { normalizePathLikeInput, resolveToCwd } from "./path-utils"; import { formatErrorDetail, PREVIEW_LIMITS } from "./render-utils"; // ============================================================================= @@ -428,6 +429,11 @@ const STATUS_TO_MARKER: Record = { abandoned: "-", }; +export function resolveTodoMarkdownPath(input: string, cwd: string): string { + const raw = normalizePathLikeInput(input) || "TODO.md"; + return resolveToCwd(raw, cwd); +} + /** Render todo phases as a Markdown checklist suitable for editing/copying. */ export function phasesToMarkdown(phases: TodoPhase[]): string { if (phases.length === 0) return "# Todos\n"; diff --git a/packages/coding-agent/test/acp-builtins.test.ts b/packages/coding-agent/test/acp-builtins.test.ts index aecc4bf09..7742d5674 100644 --- a/packages/coding-agent/test/acp-builtins.test.ts +++ b/packages/coding-agent/test/acp-builtins.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, spyOn } from "bun:test"; +import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; import type { @@ -544,6 +545,120 @@ describe("wave 3 commands", () => { expect(fakeSessionManager._customEntries[0]?.customType).toBe("user_todo_edit"); }); + it("/todo export: writes the default file under the active session cwd", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "pi-todo-export-")); + try { + const { output, session, fakeSessionManager, runtime } = createRuntime(); + fakeSessionManager._cwd = tempRoot; + session._todoPhases = [{ name: "Work", tasks: [{ content: "Ship it", status: "pending" }] }]; + + const result = await executeAcpBuiltinSlashCommand("/todo export", runtime); + + const target = path.join(tempRoot, "TODO.md"); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toBe(`Wrote todos to ${target}`); + expect(await fs.readFile(target, "utf8")).toBe("# Work\n- [ ] Ship it\n"); + } finally { + await fs.rm(tempRoot, { recursive: true, force: true }); + } + }); + + it("/todo export: writes a quoted path with spaces", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "pi-todo-export-quoted-")); + try { + const { output, session, runtime } = createRuntime(); + const target = path.join(tempRoot, "todo file.md"); + session._todoPhases = [{ name: "Work", tasks: [{ content: "Ship it", status: "pending" }] }]; + + const result = await executeAcpBuiltinSlashCommand(`/todo export "${target}"`, runtime); + + expect(result).toEqual({ consumed: true }); + expect(output[0]).toBe(`Wrote todos to ${target}`); + expect(await fs.readFile(target, "utf8")).toBe("# Work\n- [ ] Ship it\n"); + } finally { + await fs.rm(tempRoot, { recursive: true, force: true }); + } + }); + + it("/todo import: reads a quoted absolute path", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "pi-todo-import-")); + try { + const target = path.join(tempRoot, "todo file.md"); + await fs.writeFile(target, "# Imported\n- [/] Active task\n", "utf8"); + const { output, session, runtime } = createRuntime(); + + const result = await executeAcpBuiltinSlashCommand(`/todo import "${target}"`, runtime); + + expect(result).toEqual({ consumed: true }); + expect(output[0]).toBe(`Imported 1 phase(s), 1 task(s) from ${target}.`); + expect(session._todoPhases).toEqual([ + { name: "Imported", tasks: [{ content: "Active task", status: "in_progress" }] }, + ]); + } finally { + await fs.rm(tempRoot, { recursive: true, force: true }); + } + }); + + it("/todo import: reads the default file under the active session cwd", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "pi-todo-import-default-")); + try { + const target = path.join(tempRoot, "TODO.md"); + await fs.writeFile(target, "# Default\n- [ ] From cwd\n", "utf8"); + const { output, session, fakeSessionManager, runtime } = createRuntime(); + fakeSessionManager._cwd = tempRoot; + + const result = await executeAcpBuiltinSlashCommand("/todo import", runtime); + + expect(result).toEqual({ consumed: true }); + expect(output[0]).toBe(`Imported 1 phase(s), 1 task(s) from ${target}.`); + expect(session._todoPhases).toEqual([ + { name: "Default", tasks: [{ content: "From cwd", status: "in_progress" }] }, + ]); + } finally { + await fs.rm(tempRoot, { recursive: true, force: true }); + } + }); + + it("/todo import: reports parse errors without committing", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "pi-todo-import-invalid-")); + try { + const target = path.join(tempRoot, "TODO.md"); + await fs.writeFile(target, "# Imported\nnot a todo\n", "utf8"); + const { output, session, fakeSessionManager, runtime } = createRuntime(); + fakeSessionManager._cwd = tempRoot; + + const result = await executeAcpBuiltinSlashCommand("/todo import", runtime); + + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain(`Could not parse ${target}:`); + expect(session._todoPhases).toEqual([]); + } finally { + await fs.rm(tempRoot, { recursive: true, force: true }); + } + }); + + it("/todo export: reports invalid internal-scheme paths", async () => { + const { output, session, runtime } = createRuntime(); + session._todoPhases = [{ name: "Work", tasks: [{ content: "Ship it", status: "pending" }] }]; + + const result = await executeAcpBuiltinSlashCommand("/todo export artifact://1", runtime); + + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("Failed to write todos:"); + expect(output[0]).toContain("internal scheme"); + }); + + it("/todo import: reports invalid internal-scheme paths", async () => { + const { output, session, runtime } = createRuntime(); + + const result = await executeAcpBuiltinSlashCommand("/todo import artifact://1", runtime); + + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("Failed to read todos:"); + expect(output[0]).toContain("internal scheme"); + expect(session._todoPhases).toEqual([]); + }); + it("/todo edit: returns TUI-only usage message", async () => { const { output, runtime } = createRuntime(); const result = await executeAcpBuiltinSlashCommand("/todo edit", runtime); diff --git a/packages/coding-agent/test/modes/controllers/todo-command-controller.test.ts b/packages/coding-agent/test/modes/controllers/todo-command-controller.test.ts new file mode 100644 index 000000000..562d09a5b --- /dev/null +++ b/packages/coding-agent/test/modes/controllers/todo-command-controller.test.ts @@ -0,0 +1,154 @@ +import { afterEach, describe, expect, it, vi } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { TodoCommandController } from "@oh-my-pi/pi-coding-agent/modes/controllers/todo-command-controller"; +import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types"; +import { type TodoPhase, USER_TODO_EDIT_CUSTOM_TYPE } from "@oh-my-pi/pi-coding-agent/tools"; + +function createContext(cwd: string, phases: TodoPhase[]): InteractiveModeContext { + return { + agent: { + appendMessage: vi.fn(), + }, + session: { + getTodoPhases: () => phases, + setTodoPhases: vi.fn(), + }, + sessionManager: { + appendCustomEntry: vi.fn(), + appendMessage: vi.fn(), + getBranch: () => [], + getCwd: () => cwd, + }, + setTodos: vi.fn(), + showError: vi.fn(), + showStatus: vi.fn(), + showWarning: vi.fn(), + } as unknown as InteractiveModeContext; +} + +describe("TodoCommandController", () => { + let tempRoot = ""; + + afterEach(async () => { + if (tempRoot) await fs.rm(tempRoot, { recursive: true, force: true }); + tempRoot = ""; + }); + + it("advertises optional default todo import and export paths", async () => { + tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "pi-tui-todo-help-")); + const ctx = createContext(tempRoot, []); + const controller = new TodoCommandController(ctx); + + await controller.handleTodoCommand("help"); + + expect(ctx.showStatus).toHaveBeenCalledWith(expect.stringContaining("/todo export []")); + expect(ctx.showStatus).toHaveBeenCalledWith(expect.stringContaining("/todo import []")); + }); + + it("exports the default TODO.md under the active session cwd", async () => { + tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "pi-tui-todo-export-")); + const phases: TodoPhase[] = [{ name: "Work", tasks: [{ content: "Ship it", status: "pending" }] }]; + const ctx = createContext(tempRoot, phases); + const controller = new TodoCommandController(ctx); + + await controller.handleTodoCommand("export"); + + const target = path.join(tempRoot, "TODO.md"); + expect(await fs.readFile(target, "utf8")).toBe("# Work\n- [ ] Ship it\n"); + expect(ctx.showStatus).toHaveBeenCalledWith(`Wrote todos to ${target}`); + expect(ctx.showError).not.toHaveBeenCalled(); + }); + + it("exports a quoted path with spaces", async () => { + tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "pi-tui-todo-export-quoted-")); + const phases: TodoPhase[] = [{ name: "Work", tasks: [{ content: "Ship it", status: "pending" }] }]; + const target = path.join(tempRoot, "todo file.md"); + const ctx = createContext(tempRoot, phases); + const controller = new TodoCommandController(ctx); + + await controller.handleTodoCommand(`export "${target}"`); + + expect(await fs.readFile(target, "utf8")).toBe("# Work\n- [ ] Ship it\n"); + expect(ctx.showStatus).toHaveBeenCalledWith(`Wrote todos to ${target}`); + expect(ctx.showError).not.toHaveBeenCalled(); + }); + + it("imports the default TODO.md under the active session cwd", async () => { + tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "pi-tui-todo-import-")); + const target = path.join(tempRoot, "TODO.md"); + await fs.writeFile(target, "# Imported\n- [ ] From cwd\n", "utf8"); + const ctx = createContext(tempRoot, []); + const controller = new TodoCommandController(ctx); + + await controller.handleTodoCommand("import"); + + const expected: TodoPhase[] = [{ name: "Imported", tasks: [{ content: "From cwd", status: "in_progress" }] }]; + expect(ctx.session.setTodoPhases).toHaveBeenCalledWith(expected); + expect(ctx.setTodos).toHaveBeenCalledWith(expected); + expect(ctx.sessionManager.appendCustomEntry).toHaveBeenCalledWith(USER_TODO_EDIT_CUSTOM_TYPE, { + phases: expected, + }); + expect(ctx.agent.appendMessage).toHaveBeenCalledWith(expect.objectContaining({ role: "developer" })); + expect(ctx.sessionManager.appendMessage).toHaveBeenCalledWith(expect.objectContaining({ role: "developer" })); + expect(ctx.showStatus).toHaveBeenCalledWith(`Imported 1 phase(s), 1 task(s) from ${target}.`); + expect(ctx.showError).not.toHaveBeenCalled(); + }); + + it("imports a quoted path with spaces", async () => { + tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "pi-tui-todo-import-quoted-")); + const target = path.join(tempRoot, "todo file.md"); + await fs.writeFile(target, "# Quoted\n- [ ] From quoted path\n", "utf8"); + const ctx = createContext(tempRoot, []); + const controller = new TodoCommandController(ctx); + + await controller.handleTodoCommand(`import "${target}"`); + + const expected: TodoPhase[] = [ + { name: "Quoted", tasks: [{ content: "From quoted path", status: "in_progress" }] }, + ]; + expect(ctx.session.setTodoPhases).toHaveBeenCalledWith(expected); + expect(ctx.showStatus).toHaveBeenCalledWith(`Imported 1 phase(s), 1 task(s) from ${target}.`); + expect(ctx.showError).not.toHaveBeenCalled(); + }); + + it("reports import parse errors without committing", async () => { + tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "pi-tui-todo-import-invalid-")); + const target = path.join(tempRoot, "TODO.md"); + await fs.writeFile(target, "# Imported\nnot a todo\n", "utf8"); + const ctx = createContext(tempRoot, []); + const controller = new TodoCommandController(ctx); + + await controller.handleTodoCommand("import"); + + expect(ctx.showError).toHaveBeenCalledWith(expect.stringContaining(`Could not parse ${target}:`)); + expect(ctx.session.setTodoPhases).not.toHaveBeenCalled(); + expect(ctx.setTodos).not.toHaveBeenCalled(); + }); + + it("reports invalid internal-scheme import paths without committing", async () => { + tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "pi-tui-todo-import-scheme-")); + const ctx = createContext(tempRoot, []); + const controller = new TodoCommandController(ctx); + + await controller.handleTodoCommand("import artifact://todo"); + + expect(ctx.showError).toHaveBeenCalledWith(expect.stringContaining("Failed to read todos:")); + expect(ctx.showError).toHaveBeenCalledWith(expect.stringContaining("internal scheme")); + expect(ctx.session.setTodoPhases).not.toHaveBeenCalled(); + expect(ctx.setTodos).not.toHaveBeenCalled(); + }); + + it("reports invalid internal-scheme export paths", async () => { + tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "pi-tui-todo-export-invalid-")); + const phases: TodoPhase[] = [{ name: "Work", tasks: [{ content: "Ship it", status: "pending" }] }]; + const ctx = createContext(tempRoot, phases); + const controller = new TodoCommandController(ctx); + + await controller.handleTodoCommand("export artifact://todo"); + + expect(ctx.showError).toHaveBeenCalledWith(expect.stringContaining("Failed to write todos:")); + expect(ctx.showError).toHaveBeenCalledWith(expect.stringContaining("internal scheme")); + }); +}); diff --git a/packages/coding-agent/test/system-prompt-inventory.test.ts b/packages/coding-agent/test/system-prompt-inventory.test.ts index e10c3cda3..bb8d4f743 100644 --- a/packages/coding-agent/test/system-prompt-inventory.test.ts +++ b/packages/coding-agent/test/system-prompt-inventory.test.ts @@ -2,7 +2,13 @@ import { afterEach, beforeEach, describe, expect, it } from "bun:test"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; -import { buildSystemPrompt, type SystemPromptToolMetadata } from "@oh-my-pi/pi-coding-agent/system-prompt"; +import { buildSystemPrompt as buildSdkSystemPrompt } from "@oh-my-pi/pi-coding-agent/sdk"; +import { + buildSystemPrompt, + DEFAULT_SYSTEM_PROMPT_TOOL_NAMES, + type SystemPromptToolMetadata, +} from "@oh-my-pi/pi-coding-agent/system-prompt"; +import type { Tool } from "@oh-my-pi/pi-coding-agent/tools"; import { cleanupTempHome } from "./helpers/temp-home-cleanup"; const EMPTY_TREE = { @@ -32,6 +38,17 @@ const TOOLS = new Map([ ], ]); +const SDK_TOOL: Tool = { + name: "sdk_custom", + label: "SDK Custom", + description: "SDK-provided custom tool.", + parameters: { type: "object", properties: {} }, + approval: "read", + async execute() { + return { content: [{ type: "text", text: "ok" }] }; + }, +}; + describe("system prompt tool inventory", () => { let tempDir = ""; let tempHomeDir = ""; @@ -61,6 +78,14 @@ describe("system prompt tool inventory", () => { return systemPrompt.join("\n\n"); } + function inventoryFrom(text: string): string { + const inventoryStart = text.indexOf("# Inventory"); + expect(inventoryStart).toBeGreaterThan(-1); + const envStart = text.indexOf("ENV\n", inventoryStart); + expect(envStart).toBeGreaterThan(inventoryStart); + return text.slice(inventoryStart, envStart); + } + it("renders a compact name list only when native tools are active and descriptors stay in schemas", async () => { const text = await render({ nativeTools: true, inlineToolDescriptors: false }); expect(text).toContain("- Read: `read`"); @@ -87,6 +112,115 @@ describe("system prompt tool inventory", () => { expect(text).not.toContain("- Read: `read`"); }); + it("uses a conservative fallback inventory when no tools map is provided", async () => { + const { systemPrompt } = await buildSystemPrompt({ + cwd: tempDir, + contextFiles: [], + skills: [], + rules: [], + workspaceTree: { ...EMPTY_TREE, rootPath: tempDir }, + }); + const inventory = inventoryFrom(systemPrompt.join("\n\n")); + for (const toolName of DEFAULT_SYSTEM_PROMPT_TOOL_NAMES) { + expect(inventory).toContain(`- \`${toolName}\``); + } + expect(inventory).not.toContain("- `browser`"); + expect(inventory).not.toContain("- `task`"); + }); + + it("SDK wrapper renders provided tools instead of the fallback inventory", async () => { + const { systemPrompt } = await buildSdkSystemPrompt({ + cwd: tempDir, + contextFiles: [], + skills: [], + tools: [SDK_TOOL], + }); + const inventory = inventoryFrom(systemPrompt.join("\n\n")); + expect(inventory).toContain("- SDK Custom: `sdk_custom`"); + expect(inventory).not.toContain("- `read`"); + }); + + it("SDK wrapper preserves an explicit empty tool list", async () => { + const { systemPrompt } = await buildSdkSystemPrompt({ + cwd: tempDir, + contextFiles: [], + skills: [], + tools: [], + }); + const text = systemPrompt.join("\n\n"); + + expect(text).not.toContain("# Inventory"); + expect(text).not.toContain("- `read`"); + }); + + it("keeps visible skills when no tools map is provided", async () => { + const { systemPrompt } = await buildSystemPrompt({ + cwd: tempDir, + contextFiles: [], + skills: [ + { + name: "prompt-authoring", + description: "Prompt authoring workflow", + filePath: path.join(tempDir, "SKILL.md"), + baseDir: tempDir, + source: "test", + }, + ], + rules: [], + workspaceTree: { ...EMPTY_TREE, rootPath: tempDir }, + }); + const text = systemPrompt.join("\n\n"); + + expect(text).toContain("- prompt-authoring: Prompt authoring workflow"); + }); + + it("omits skills when active tool names exclude read", async () => { + const { systemPrompt } = await buildSystemPrompt({ + cwd: tempDir, + contextFiles: [], + skills: [ + { + name: "search-only-skill", + description: "Should not render without read", + filePath: path.join(tempDir, "SKILL.md"), + baseDir: tempDir, + source: "test", + }, + ], + rules: [], + toolNames: ["bash"], + tools: TOOLS, + workspaceTree: { ...EMPTY_TREE, rootPath: tempDir }, + }); + const text = systemPrompt.join("\n\n"); + + expect(text).not.toContain("search-only-skill"); + }); + + it("omits hidden skills even when read is active", async () => { + const { systemPrompt } = await buildSystemPrompt({ + cwd: tempDir, + contextFiles: [], + skills: [ + { + name: "hidden-workflow", + description: "Hidden prompt workflow", + filePath: path.join(tempDir, "SKILL.md"), + baseDir: tempDir, + source: "test", + hide: true, + }, + ], + rules: [], + toolNames: ["read"], + tools: TOOLS, + workspaceTree: { ...EMPTY_TREE, rootPath: tempDir }, + }); + const text = systemPrompt.join("\n\n"); + + expect(text).not.toContain("hidden-workflow"); + }); + it("tells the agent to read matching skills before work", async () => { const { systemPrompt } = await buildSystemPrompt({ cwd: tempDir, diff --git a/packages/coding-agent/test/tools/todo.test.ts b/packages/coding-agent/test/tools/todo.test.ts index 7b44bd8f9..8266667e8 100644 --- a/packages/coding-agent/test/tools/todo.test.ts +++ b/packages/coding-agent/test/tools/todo.test.ts @@ -1,8 +1,10 @@ import { beforeAll, describe, expect, it } from "bun:test"; +import * as path from "node:path"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { initTheme, theme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; import { + resolveTodoMarkdownPath, selectStickyTodoWindow, TODO_STRIKE_HOLD_FRAMES, type TodoItem, @@ -33,6 +35,26 @@ beforeAll(async () => { await initTheme(); }); +describe("resolveTodoMarkdownPath", () => { + it("defaults to TODO.md under cwd", () => { + const cwd = path.resolve("tmp", "todo-workspace"); + + expect(resolveTodoMarkdownPath("", cwd)).toBe(path.join(cwd, "TODO.md")); + }); + + it("strips surrounding double quotes before resolving", () => { + const cwd = path.resolve("tmp", "todo-workspace"); + + expect(resolveTodoMarkdownPath('"my todos.md"', cwd)).toBe(path.join(cwd, "my todos.md")); + }); + + it("rejects internal URL schemes", () => { + const cwd = path.resolve("tmp", "todo-workspace"); + + expect(() => resolveTodoMarkdownPath("artifact://todo", cwd)).toThrow("internal scheme"); + }); +}); + describe("TodoTool auto-start behavior", () => { it("auto-starts the first task after init", async () => { const tool = new TodoTool(createSession());