Files
backnotprop__plannotator/apps/hook/server/annotate-resolution.test.ts
Michael Ramos 747b5ea7e6 fix(annotate): resolve natural-language arguments or hand off to the agent (#1183)
* 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
2026-08-03 10:14:37 -07:00

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");
}
});
});