Skip to content

fix(annotate): resolve natural-language arguments or hand off to the agent - #1183

Merged
backnotprop merged 2 commits into
mainfrom
fix/1182-annotate-arg-handoff
Aug 3, 2026
Merged

backnotprop merged 2 commits into
mainfrom
fix/1182-annotate-arg-handoff

Conversation

@backnotprop

@backnotprop backnotprop commented Aug 3, 2026 •

Copy link
Copy Markdown
Owner

Problem

The Claude Code skill runs the CLI through a bash-substitution prefix (!`plannotator annotate $ARGUMENTS`) that executes before the model sees anything. $ARGUMENTS lands in argv unquoted, so /plannotator-annotate the aim doc died with File not found: the.

It is worse than an unhelpful error. Verified empirically with a scratch skill: when a !-prefixed command exits non-zero, Claude Code aborts the prompt before the model runs at all (num_turns: 0); neither the command output nor the skill body reaches the agent. When the command exits 0, both stdout and stderr are injected. So the old failure was invisible to the agent by construction, which is why it could never self-correct (the docs are silent on this; tested on the current CLI).

Design: three-tier resolution in the binary

The tolerant logic lives in the CLI's argument handling (plus the OpenCode/Pi plugin parsers), so every host benefits at once and the #872 guarantee (slash command always executes the binary) stays intact.

  1. Fast path: probe each whitespace-delimited token of the arguments; if exactly one names an existing file, URL, or folder, proceed with it directly. /plannotator-annotate look at notes.md please just opens notes.md, no model round-trip. Guardrails: single-token invocations run the classic pipeline unchanged (a lone typo'd path stays File not found with exit 1 in every non-strict mode); bare directory names only count when they are the sole argument, so a stray word matching a directory (or .) cannot hijack the fast path; unrecognized dash-prefixed tokens disable tolerance entirely, so a typo'd flag like --no-jna errors the way it did on base instead of being silently skipped; and selection receives the original argv tokens, so a quoted missing path ("my notes.md") can never be re-split into a silently resolving notes.md.
  2. Ambiguity: two or more tokens resolve (annotate a.md b.md); error naming every candidate and what it resolved to. Never guess.
  3. Handoff: two or more words and none resolves; instead of File not found: and, emit an agent-addressed message that echoes the words tried, states the command needs a concrete path, URL, or folder, and asks the agent reading it to interpret the request and re-run plannotator annotate <target>. Content flags (--markdown, --no-jina, --render-html) are echoed for the re-run; transport flags (--gate, --json, --hook) are not, so the suggestion never tells an agent to start a blocking interactive gate. In plain mode the message goes to stdout with exit 0, the only combination that reaches the model through the bang prefix (see above). In --json/--hook mode it goes to stderr with exit 1 so machine-readable stdout stays reserved for decision records; the droid wrapper forwards stderr into its command output on failure, so the droid agent still gets the handoff. On Amp the failure path is a human-facing notification (ctx.ui.notify; ctx.thread.append runs only on success), so there the handoff reaches the human rather than the model.

Mechanics:

  • packages/shared/annotate-target.ts (new, vendored to Pi): the token probe (URL, folder, HTML, document resolution with the literal-@ fallback, then file-only bare existence so existing-but-unsupported files keep their specific pipeline errors), the tier selection (including the unknown-flag flagged outcome), and the message builders. Word tokens without an annotatable extension never trigger a project walk, so probing "the aim doc" costs three stat-level checks.
  • apps/hook/server/annotate-resolution.ts (extracted from the annotate branch of index.ts): the unchanged resolution pipeline, returning typed outcomes instead of exiting, so the fallback can run it once with a selected candidate and so it is unit-testable. The strict-exit-code guard test enforces the invariant across both files.
  • OpenCode (apps/opencode-plugin/commands.ts) and Pi (apps/pi-extension/index.ts) wire the same shared selection in front of their existing pipelines (their first pass, the whole joined string, is unchanged). Their tier-3 message is the same text without the agent paragraph, since their errors surface as host notifications rather than model context.
  • Skill and command bodies (apps/skills/claude, apps/skills/core, apps/kiro-cli, apps/copilot, apps/gemini): one line each telling the agent that an "arguments could not be resolved" report means interpret the user's words and re-run with a concrete target.
  • Docs: AGENTS.md/CLAUDE.md describe the three tiers under Annotate Flow and the strict section's compatibility sentences were corrected; the marketing annotate doc mentions the tolerant arguments.

Invariants held

  • A bare correct invocation (plannotator annotate file.md) is byte-identical, and so is every single-token invocation, including failures: annotate nope.md stays exit 1 with File not found: nope.md (no handoff prose), so scripts gating on the exit code of a plain invocation are unaffected.
  • The strict gate contract is untouched: with --require-approval/--result-file the tolerance never runs; a typo'd path stays stderr File not found: ... with exit 2 and byte-empty stdout. This is now pinned by subprocess tests that spawn the real CLI entry.
  • Ambiguous filenames, unsupported types, oversized files, empty folders, and URL fetch failures keep their exact messages and exit codes; an existing-but-unsupported file among natural-language words still surfaces File type not supported rather than the handoff.

Verified

  • bun run typecheck green; full bun test green (2701 pass, 0 fail).
  • Unit tests: packages/shared/annotate-target.test.ts (probe incl. bare-directory gating and file-only existence, selection fast path with trailing and leading words, multi-resolve candidates, dedupe, unknown-flag flagged outcome incl. --no-jna, argv token preservation, zero-resolve words, message content incl. content-flag preservation) and apps/hook/server/annotate-resolution.test.ts (pipeline outcomes and terminal classification).
  • Subprocess tests: apps/hook/server/annotate-cli.test.ts spawns the real CLI: single-token typo exit 1 (plain and --gate), strict --require-approval and --result-file invocations with natural-language args exit 2 with empty stdout, --no-jna exit 1, quoted-token preservation, directory-hijack handoff. Creates placeholder dist files when a build is absent so it runs in CI.
  • bash apps/pi-extension/vendor.sh regenerates cleanly; bun pm pack --dry-run includes generated/annotate-target.ts.
  • CLI smoke (bun-run from a scratch dir): annotate notes.md and annotate notes.md please open the file; annotate a.md b.md exits 1 naming both candidates; annotate the aim doc exits 0 with the stdout handoff; annotate the aim doc --json exits 1 with the stderr handoff; annotate the aim doc --markdown --no-jina echoes the flags in the re-run suggestion; annotate nope.md and annotate nope.md --gate stay exit 1 File not found; annotate --no-jna https://... exits 1 File not found: --no-jna; annotate "my notes.md" runme hands off instead of opening notes.md; annotate please annotate . hands off instead of opening folder mode; annotate check script.py now keeps the File type not supported error; strict annotate nope-not-here.md --gate --json --require-approval stays exit 2 with empty stdout, as does the strict multi-token case.

Deliberate behavior change

plannotator annotate a.md b.md used to silently open a.md and drop b.md; it is now the tier-2 ambiguity error, per the never-guess guardrail.

Out of scope

  • plannotator-review and plannotator-last positional args: parseReviewArgs already ignores non-URL positionals and annotate-last reads none, so the bang shape does not bite there.
  • Bare /plannotator-annotate (no args) still prints usage and exits non-zero, which under the bang prefix aborts the prompt with the usage line visible in the UI. Turning that into a handoff changes plannotator annotate exit semantics for human terminals and deserves its own call.
  • The tier-3 message does not yet special-case "a token exists but is an unsupported type" when other words are present and no token resolves.

Closes #1182

Thanks @technicalpickles for the report, the history archaeology on #872, and the fix direction (option 1 plus 3), which this implements with the maintainer's exactly-one-resolves guardrail.

…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
@backnotprop

Copy link
Copy Markdown
Owner Author

Adversarial review (at 85897d32)

Verdict: needs changes. The design holds and the load-bearing empirical claim was independently reproduced with claude 2.1.220 and scratch skills: a ! command exiting non-zero yields num_turns: 0 (the model never runs), exit 0 injects the output. The shared module is clean, vendoring is correct (pack includes generated/annotate-target.ts, #1106 specifier convention), the strict-gate behavior is correct in every combination actually run, and mutation testing catches tier-2 tampering. Two high findings and a set of follow-ons.

H1 (required): single-token typo now fails open in the CLI. The CLI gates the handoff on words.length > 0 while OpenCode and Pi use > 1 (their comment even states the intent: a single unresolvable word falls through so its specific errors stay verbatim). Measured: annotate nope.md went from exit 1 + File not found to exit 0 + agent prose, so a script doing annotate docs/spec.md --gate || exit 1 silently passes after a rename, with no server started and no review performed. Not needed for #1182 ("the aim doc" is three words). Fix: match > 1 in the CLI, which also fixes the tier-3 wording being wrong for typos.

H2 (required): the strict-gate bypass has zero test coverage. Mutating strictAnnotate to false leaves the full suite green, and under that mutation --gate --json --require-approval with natural-language args opened a directory nobody named under a strict gate. The behavior is correct today; nothing pins it. Add a subprocess test: strict flags + multi-token natural language must exit 2 with File not found on stderr and byte-empty stdout.

M: unknown flag-like tokens are silently swallowed, including privacy-relevant ones. --no-jna (typo) used to error; now the URL is fetched via Jina anyway, which is exactly what --no-jina exists to prevent. Unknown - tokens should error as before, not be skipped.

M: Copilot and Gemini command bodies were not updated and their output enumeration tells the agent to treat handoff prose as annotation feedback; same one-line fix the other skills got.

M: docs drift. CLAUDE.md's strict-annotate section now contains two false sentences ("exit behavior remains unchanged when neither strict option is present", "startup sites exit 1 as before for non-strict"), the Annotate Flow section does not mention the three tiers or the deliberate a.md b.md ambiguity change, and the marketing annotate page is untouched.

M: the PR body's Amp claim is wrong. Amp's failure path is a UI notification; ctx.thread.append runs only on success, so the handoff reaches the human, not the model, on Amp. Correct the claim rather than the code.

L: quoted multi-word paths are re-split on whitespace ("my notes.md" missing can silently open notes.md; preserve argv tokens instead of join+split). The re-run suggestion omits transport flags and under --hook would tell an agent to start a blocking interactive session; echo content flags only and keep transport flags out of the suggestion. A token naming any directory (including .) wins tier 1 in multi-token mode; consider requiring a path separator or sole-token for directory candidates. Pi's ambiguous-name message diverges cosmetically from the CLI's.

Fixes are being applied on the branch; delta re-review to follow.

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
@backnotprop

Copy link
Copy Markdown
Owner Author

Delta re-review (at 8df9281b)

Verdict: merge as-is. All seven findings fixed and verified by re-running the original measured repros. Full suite 2701 pass / 0 fail, typecheck clean, vendoring byte-faithful and still packed.

  • Single-token typos are exit 1 + File not found again in plain mode and with --gate; sole quoted missing paths are byte-identical to base. the aim doc still hands off. All three hosts carry both guards.
  • The strict-gate wiring is now genuinely pinned: three separate mutations (strict bypass, tolerance threshold, flag handling) each fail the new subprocess suite, which spawns the real CLI entry and was confirmed to leave a real dist untouched.
  • --no-jna errors before any fetch; known flags pass through; content flags echo in the re-run suggestion while transport flags appear in no host's message.
  • Directory hijack closed: please annotate . hands off while sole and separator-bearing directory arguments keep folder mode; extensionless file names keep their exact errors.
  • Copilot and Gemini bodies updated in their own voice; AGENTS.md's two stale sentences corrected with the tiers documented; marketing page updated; PR body's Amp claim now accurate.

Four low observations on record, none blocking: (1) an unknown dash token disables tolerance entirely, so a.md b.md --unknownflag still silently opens a.md (base behavior, but the PR body's "eliminated" claim has this residual reach); (2) a typo'd flag like --markdwn reproduces the original invisible bang-prefix failure by design, worth one sentence in the tolerant-resolution docs so it is not rediscovered as a bug; (3) OpenCode and Pi still pass a joined string into selection, so the quoted-missing-path hazard survives there in the narrow quote-wrapped case (their parsers decline shell quoting, so low; a [rawFilePath] pass-through when quote-wrapped would close it); (4) the test placeholder-dist mechanism could leave invisible placeholder files if the process dies mid-suite since dist is gitignored; skipping the describe when dist is absent would be the safer shape. Also logged: a bare directory token in multi-token mode now hands off instead of surfacing directory-specific errors like No annotatable files, consistent with the new rule but a narrowing of the PR body's invariant.

@backnotprop
backnotprop merged commit 747b5ea into main Aug 3, 2026
14 checks passed
backnotprop added a commit that referenced this pull request Aug 3, 2026
…overage (#1187)

Ports five small items from the closed parallel PR #1185 into the
tolerant annotate argument resolution that landed in #1183 (#1182):

- Bug fix: the token probe tested the raw token against the URL regex,
  but the pipeline strips the @ reference marker and wrapping quotes
  first, so a multi-token 'annotate @https://example.com/page and
  summarize it' probed to nothing and emitted the handoff instead of
  opening the URL. The probe now unwraps with stripAtPrefix before the
  regex and returns the unwrapped form (the pipeline re-strips
  harmlessly). Tests cover @-prefixed and quote-wrapped URLs as
  multi-token candidates.
- Test ports: absolute-path candidate, the wider plain-text set (.txt,
  .yaml) guarding ANNOTATABLE_DOC_REGEX breadth, the scoped-package
  literal-@ fallback against a real @scope/ directory, and the
  whole-un-split-string preference over its own tokens ('Meeting
  Notes.md' wins over a resolving 'Notes.md' token) covering
  annotateInputNamesExistingTarget.
- Defensive scan: the strict-mode source-scan test now asserts the
  annotate startup block gates tolerance on !strictAnnotate via
  isStrictAnnotateInvocation, since an inverted gate cannot be
  spawn-tested without starting a server.
- DRY: the strict predicate was defined twice (strict-annotate-result
  exit-code helper and the index.ts tolerance bypass). Extracted
  isStrictAnnotateInvocation with a StrictAnnotateFlags type; both
  sites use it so the exit-code path and the tolerance bypass can
  never drift. Behavior byte-identical; existing subprocess tests
  unchanged.
- Docs: the tolerant-resolution section now cites #872 (commit
  aac5aac) for why the bang prefix is deliberate and states that
  argument-shape issues belong in the CLI's resolution, not the skill
  templates.

Refs #1185, #1182

Co-authored-by: Josh Nichols <josh.nichols+agent@gusto.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

/plannotator-annotate treats trailing natural language as a filename and fails

1 participant