mirror of
https://github.com/backnotprop/plannotator.git
synced 2026-09-14 14:17:26 +08:00
747b5ea7e6
* fix(annotate): resolve natural-language arguments or hand off to the agent Claude Code skills run the CLI through a bash-substitution prefix that executes before the model sees anything, so any trailing natural language in /plannotator-annotate died with 'File not found: the'. Worse, a non-zero exit from that prefix aborts the whole prompt before the model runs (verified empirically), so the error was never even visible to the agent. Three-tier resolution in the binary's annotate argument handling, shared by every host via packages/shared/annotate-target.ts: 1. Fast path: probe each whitespace-delimited token; exactly one naming an existing file, URL, or folder proceeds with it directly. 2. Ambiguity: two or more tokens resolve; error naming every candidate, never guess. 3. Handoff: nothing resolves; emit an agent-addressed message echoing the words tried and asking the reading agent to interpret the request and re-run with a concrete target, preserving flags. In plain mode it lands on stdout with exit 0, the only combination that reaches the model through the bang prefix; in --json/--hook mode it goes to stderr with exit 1 so machine stdout stays clean. Single-token invocations run the unchanged pipeline first, so bare correct invocations are byte-identical. Strict gates (--require-approval or --result-file) bypass the tolerance entirely: a typo'd path stays a startup failure with exit 2 and no agent-facing prose. The CLI resolution pipeline moves to apps/hook/server/annotate-resolution.ts (returns typed outcomes instead of exiting) so the token fallback can run it once with a selected candidate; OpenCode and Pi wire the same shared selection into their own not-found paths. Skill bodies gain one line telling the agent to re-run with a concrete target when the command reports unresolvable arguments. Closes #1182 Reported-by: @technicalpickles * fix(annotate): harden tolerant resolution per review Review fixes for the three-tier annotate argument handling: - A single unresolvable token now falls through to the legacy pipeline verbatim: 'annotate nope.md' is exit 1 with 'File not found: nope.md' again in every non-strict mode, instead of an exit-0 handoff that fail-opened scripts gating on the exit code. The handoff fires only when two or more words resolve to nothing. - Unrecognized dash-prefixed tokens disable tolerance instead of being skipped, so a typo'd flag ('--no-jna') errors the way it did on base rather than silently fetching via Jina. Known flags are stripped before selection as before. - Token selection now receives the original argv tokens, so a quoted missing path ('my notes.md') is probed as one token and can never be re-split into a silently resolving 'notes.md'. - Bare directory names only count as fast-path candidates when they are the sole argument; a stray word matching a directory (or '.') hands off instead of opening folder mode. Explicit paths like 'src/' keep resolving, and the bare-existence probe fallback is file-only. - The handoff re-run suggestion echoes content flags only (--markdown, --no-jina, --render-html), never transport flags (--gate, --json, --hook). - New subprocess suite (annotate-cli.test.ts) spawns the real CLI entry and pins the contract: single-token typo exit 1, strict invocations (--require-approval and --result-file) exit 2 with empty stdout and no handoff prose, unknown-flag error, quoted-token preservation, and the directory-hijack case. Placeholder dist files are created when a build is absent so the suite runs in CI. - The copilot and gemini annotate command bodies gain the same handoff instruction as the Claude, core, and kiro skills. - AGENTS.md documents the three tiers under Annotate Flow and corrects the strict-section sentences that claimed non-strict behavior was fully unchanged; the marketing annotate doc mentions the tolerant arguments. Refs #1182
118 lines
3.9 KiB
TypeScript
118 lines
3.9 KiB
TypeScript
import { describe, test, expect, beforeAll, afterAll } from "bun:test";
|
|
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "fs";
|
|
import { tmpdir } from "os";
|
|
import { join } from "path";
|
|
import { resolveAnnotateTarget } from "./annotate-resolution";
|
|
|
|
let root: string;
|
|
|
|
beforeAll(() => {
|
|
root = mkdtempSync(join(tmpdir(), "plannotator-annotate-resolution-"));
|
|
mkdirSync(join(root, "docs"), { recursive: true });
|
|
mkdirSync(join(root, "notes"), { recursive: true });
|
|
mkdirSync(join(root, "empty"), { recursive: true });
|
|
writeFileSync(join(root, "plan.md"), "# Plan body");
|
|
writeFileSync(join(root, "docs/page.html"), "<p>hi</p>");
|
|
writeFileSync(join(root, "docs/dup.md"), "# A");
|
|
writeFileSync(join(root, "notes/dup.md"), "# B");
|
|
writeFileSync(join(root, "script.py"), "print()");
|
|
writeFileSync(join(root, "big.md"), "x".repeat(2 * 1024 * 1024 + 1));
|
|
});
|
|
|
|
afterAll(() => {
|
|
rmSync(root, { recursive: true, force: true });
|
|
});
|
|
|
|
function resolve(rawFilePath: string, overrides: { renderMarkdown?: boolean } = {}) {
|
|
return resolveAnnotateTarget({
|
|
rawFilePath,
|
|
projectRoot: root,
|
|
noJina: true,
|
|
renderMarkdown: overrides.renderMarkdown ?? false,
|
|
log: () => {},
|
|
});
|
|
}
|
|
|
|
describe("resolveAnnotateTarget", () => {
|
|
test("resolves a markdown file and reads its content", async () => {
|
|
const result = await resolve("plan.md");
|
|
expect(result.ok).toBe(true);
|
|
if (result.ok) {
|
|
expect(result.absolutePath).toBe(join(root, "plan.md"));
|
|
expect(result.markdown).toBe("# Plan body");
|
|
expect(result.annotateMode).toBe("annotate");
|
|
expect(result.isUrl).toBe(false);
|
|
}
|
|
});
|
|
|
|
test("resolves a folder into folder mode", async () => {
|
|
const result = await resolve("docs");
|
|
expect(result.ok).toBe(true);
|
|
if (result.ok) {
|
|
expect(result.annotateMode).toBe("annotate-folder");
|
|
expect(result.folderPath).toBe(join(root, "docs"));
|
|
}
|
|
});
|
|
|
|
test("resolves an HTML file as raw HTML by default and markdown with --markdown", async () => {
|
|
const raw = await resolve("docs/page.html");
|
|
expect(raw.ok).toBe(true);
|
|
if (raw.ok) {
|
|
expect(raw.rawHtml).toBe("<p>hi</p>");
|
|
expect(raw.markdown).toBe("");
|
|
}
|
|
const converted = await resolve("docs/page.html", { renderMarkdown: true });
|
|
expect(converted.ok).toBe(true);
|
|
if (converted.ok) {
|
|
expect(converted.rawHtml).toBeUndefined();
|
|
expect(converted.sourceConverted).toBe(true);
|
|
}
|
|
});
|
|
|
|
test("only the missing-target terminal reports notFound", async () => {
|
|
const missing = await resolve("missing.md");
|
|
expect(missing.ok).toBe(false);
|
|
if (!missing.ok) {
|
|
expect(missing.notFound).toBe(true);
|
|
expect(missing.message).toBe("File not found: missing.md");
|
|
}
|
|
|
|
const word = await resolve("the");
|
|
expect(word.ok).toBe(false);
|
|
if (!word.ok) {
|
|
expect(word.notFound).toBe(true);
|
|
}
|
|
});
|
|
|
|
test("target-specific failures keep notFound false and their messages", async () => {
|
|
const ambiguous = await resolve("dup.md");
|
|
expect(ambiguous.ok).toBe(false);
|
|
if (!ambiguous.ok) {
|
|
expect(ambiguous.notFound).toBe(false);
|
|
expect(ambiguous.message).toContain('Ambiguous filename "dup.md"');
|
|
expect(ambiguous.message).toContain("2 matches");
|
|
}
|
|
|
|
const unsupported = await resolve("script.py");
|
|
expect(unsupported.ok).toBe(false);
|
|
if (!unsupported.ok) {
|
|
expect(unsupported.notFound).toBe(false);
|
|
expect(unsupported.message).toContain("File type not supported: .py");
|
|
}
|
|
|
|
const oversized = await resolve("big.md");
|
|
expect(oversized.ok).toBe(false);
|
|
if (!oversized.ok) {
|
|
expect(oversized.notFound).toBe(false);
|
|
expect(oversized.message).toContain("File too large to annotate (max 2MB)");
|
|
}
|
|
|
|
const emptyFolder = await resolve("empty");
|
|
expect(emptyFolder.ok).toBe(false);
|
|
if (!emptyFolder.ok) {
|
|
expect(emptyFolder.notFound).toBe(false);
|
|
expect(emptyFolder.message).toContain("No annotatable files");
|
|
}
|
|
});
|
|
});
|