fix(coding-agent): handle todo paths and prompt inventory
This commit is contained in:
@@ -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)).
|
||||
|
||||
@@ -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 <path> Write todos as Markdown to <path>",
|
||||
" /todo import <path> Replace todos from Markdown at <path>",
|
||||
" /todo export [<path>] Write todos to file (default: TODO.md)",
|
||||
" /todo import [<path>] Replace todos from file (default: TODO.md)",
|
||||
" /todo append [<phase>] <task...> Append a task; phase fuzzy-matched or auto-created",
|
||||
" /todo start <task> Mark task in_progress (fuzzy content match)",
|
||||
" /todo done [<task|phase>] 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<void> {
|
||||
@@ -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<void> {
|
||||
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);
|
||||
|
||||
@@ -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<BuildSystemPromptResult> {
|
||||
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,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -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<SlashCommandResult> {
|
||||
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);
|
||||
|
||||
@@ -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:<name>).
|
||||
const hasRead = tools?.has("read");
|
||||
const hasRead = toolNames.includes("read");
|
||||
const filteredSkills = hasRead ? skills.filter(skill => skill.hide !== true) : [];
|
||||
|
||||
const effectiveSystemPromptCustomization = dedupePromptSource(systemPromptCustomization, [
|
||||
|
||||
@@ -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<TodoStatus, string> = {
|
||||
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";
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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 [<path>]"));
|
||||
expect(ctx.showStatus).toHaveBeenCalledWith(expect.stringContaining("/todo import [<path>]"));
|
||||
});
|
||||
|
||||
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"));
|
||||
});
|
||||
});
|
||||
@@ -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<string, SystemPromptToolMetadata>([
|
||||
],
|
||||
]);
|
||||
|
||||
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,
|
||||
|
||||
@@ -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());
|
||||
|
||||
Reference in New Issue
Block a user