Commit Graph

5 Commits

Author SHA1 Message Date
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
Raúl 9a450a69e7 feat(annotate): preserve notes on structured approval (#1092)
* feat(annotate): add strict atomic result output

* feat(annotate): exit 2 for strict-gate usage and publication errors

Adopt the grep convention for the strict annotate gate's exit codes:
0 = approved, 1 = negative human outcome (annotated/dismissed under
--require-approval), 2 = the gate itself was misconfigured or could not
start/deliver a decision. Previously all usage/startup/validation
failures shared exit 1 with "reviewer did not approve", so callers could
not tell a denied review from a broken gate.

- parseStrictAnnotateOptions failures (bad flag combos, strict flags
  outside annotate --gate --json) now exit 2
- --result-file preflight failures (missing parent, pre-existing or
  dangling-symlink destination) now exit 2
- post-decision publication failures (destination raced into existence,
  hard links unavailable, stdout write failure) now exit 2: they deliver
  no decision record at all, so the code's own fail-closed handling
  presents them as environment errors, never as a reviewer outcome --
  and never approval, since only 0 means approved
- decision outcomes keep 0/1 exactly as before; signal deaths keep 128+n
- document the contract in AGENTS.md and the annotate-gates guide

Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk

* feat(annotate): preserve notes on structured approval

* test(pi): use exact annotate outcome import

* fix(annotate): exit 2 for strict-gate startup failures

The six startup-failure sites in the annotate path (missing path, unreachable
URL, empty folder, ambiguous name, missing/unsupported file, oversized file)
run after flag parsing and exited 1. Under --require-approval / --result-file,
1 is the "reviewer requested changes" signal, so a typo'd path made automation
misclassify a configuration error as a legitimate rejection.

Route those sites through exitAnnotateStartupFailure(), which picks its code
from the already-parsed strict options via the new pure helper
annotateStartupFailureExitCode(). Non-strict invocations still exit 1 with
byte-identical stderr; strict invocations exit STRICT_GATE_ERROR_EXIT_CODE (2).

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

* fix(annotate): emit the strict decision on stdout before publishing it

writeResultFile ran before the decision JSON reached stdout. On a filesystem
without hard links (exFAT, FAT32, most SMB/NFS, some container bind mounts)
publication fails deterministically, the catch exited 2 with nothing written
anywhere — and the reviewer's autosaved draft had already been deleted by the
feedback flow, so their completed decision was lost.

Emit the stdout record first, then publish the result file. Exit semantics are
unchanged: a publication failure still exits 2, but the decision has reached
stdout by then. Only a stdout write failure now leaves no record at all.

Correct the docs and comments that claimed exit 2 delivers no decision record:
it means the result *file* was not published. Also document the two publication
caveats: the 0600 mode is a no-op on Windows, and the atomic link/rename is not
followed by a parent-directory fsync, so publication is atomic but not
crash-durable.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

* fix(annotate): parse linked docs with the render-side frontmatter rule on export

buildCompleteAnnotateFeedback re-parsed each linked document with
parseMarkdownToBlocks(entry.markdown) — no options, so frontmatter
stripping defaulted on. The render side parses with
{ frontmatter: shouldStripFrontmatter(path) }.

For plain-text linked docs (.yaml/.json/.toml/…) a leading `---` is real
content, not frontmatter: a multi-document YAML opens with it. Stripping
it on the export side shifted every block id, so ordinary Send Feedback
and deny emitted wrong `(line N)` labels — or dropped them entirely when
the annotation's block no longer existed.

Pass the same shouldStripFrontmatter(filepath) option at the export call
site so both sides agree.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

* fix(annotate): carry the message scope through approve-with-notes

/api/feedback forwards selectedMessageId and feedbackScope; /api/approve
dropped them. Pi resolves the anchor message from those fields, so notes
delivered on the approve path anchored to the last message instead of the
one the reviewer picked in a multi-message annotate-last session — while
Send Feedback in the same session anchored correctly.

Forward both fields on the approve path in the Bun and Pi servers, and
have the client build the approval body with the same scope resolution
Send Feedback uses (extracted as getFeedbackMessageScope so the two can
no longer drift).

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

* docs(annotate): tell agents an approval may carry notes

The skill and slash-command files still described `"decision": "approved"`
as "acknowledge and stop", with no mention of the feedback field the gate
can now attach — so an agent reading them would silently drop the
reviewer's approval notes.

Update the Claude core/claude skills, the Copilot commands, the Gemini
annotate command, and the annotate command reference so the approved
branch names the optional feedback field and says what to do with it:
carry it into subsequent work, do not treat it as a change request.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

* docs(annotate): document the real approvedWithNotes default

The default annotate.approvedWithNotes template is
`{{contextBlock}}{{feedback}}`, not `{{context}}` on its own line, and
{{contextBlock}} was missing from the variable table entirely.

Show the actual default, add {{contextBlock}} to the variable table, and
explain why the default prefers it: it collapses to nothing for message
annotations instead of leaving a stray blank line.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

---------

Co-authored-by: Michael Ramos <mdramos8@gmail.com>
2026-07-26 21:09:28 -07:00
Kevin 0eda139cbe Add an option to disable AI features (#1129)
* feat: allow disabling Plannotator AI

* fix: enforce disabled AI review endpoints

* fix: fully disable AI review surfaces

---------

Co-authored-by: Kevin <kcosrdev@gmail.com>
2026-07-26 09:59:15 -07:00
Michael Ramos d102c5f709 feat(annotate): add --gate, --json, and --silent-approve flags (#570)
Adds an opt-in review gate flow to annotation mode with three composable flags:

- `--gate`: 3-way UX (Approve / Send Annotations / Close)
- `--json`: structured decision output (`{"decision":"approved|annotated|dismissed"}`)
- `--silent-approve`: suppresses plaintext approve marker for naive hooks

Includes shared arg parser, @-reference handling, updated templates across all
harnesses (Claude Code, Copilot, Gemini, OpenCode, Pi), and full documentation.

Closes #570

For provenance purposes, this commit was AI assisted.
2026-04-23 17:40:40 -07:00
Michael Ramos 1800c24d80 feat(gemini): add Gemini CLI plan review integration
* feat(gemini): add Gemini CLI plan review integration

Adds a new `apps/gemini-hook/` adapter that enables Plannotator plan
review for Gemini CLI users via the BeforeTool hook system.

The adapter reads the plan file from disk (Gemini provides a path, not
inline content), delegates to the shared @plannotator/server for the
browser-based review UI, and translates the decision back into Gemini's
hook output format.

Requires an upstream fix (google-gemini/gemini-cli#21802) that makes
`decision = "allow"` user policies work for exit_plan_mode, allowing
hooks to replace the built-in TUI approval dialog.

Includes:
- apps/gemini-hook/server/index.ts — stdin/stdout adapter
- apps/gemini-hook/hooks/ — policy TOML + settings snippet
- scripts/install.sh — Gemini binary download, policy install, settings config

For provenance purposes, this commit was AI assisted.

* refactor(gemini): use single binary with auto-detection instead of separate app

Removes apps/gemini-hook/ — the plannotator binary now auto-detects
Gemini CLI from stdin (plan_path = file on disk) vs Claude Code
(plan = inline content) and branches input parsing + output formatting.

Config fixtures live in apps/gemini/ (policy TOML + settings snippet).
Install script gates on ~/.gemini existing so Claude-only users are
unaffected.

For provenance purposes, this commit was AI assisted.

* test(gemini): add manual sandbox script for Gemini CLI integration

Three modes:
- --simulate: pipes BeforeTool JSON to hook, tests approve/deny output
- (default): runs local patched Gemini build
- --nightly: installs Gemini nightly and runs it

Backs up and restores ~/.gemini config on exit.

For provenance purposes, this commit was AI assisted.

* feat(gemini): add slash commands, marketing tab, and docs for Gemini CLI

- Add /plannotator-review and /plannotator-annotate slash commands (.toml)
- Install Gemini slash commands in all three install scripts (sh, ps1, cmd)
- Add Gemini tab to marketing landing page with icon
- Add Gemini CLI to top-level README install section
- Create apps/gemini/README.md with full setup and usage docs
- Remove stale dev:gemini script and regenerate bun.lock

For provenance purposes, this commit was AI assisted.

* fix(gemini): merge hook into existing settings.json instead of printing instructions

When ~/.gemini/settings.json already exists, use node to JSON-merge
the BeforeTool hook config rather than asking the user to do it manually.
Falls back to instructions only if node is unavailable.

For provenance purposes, this commit was AI assisted.

* fix(gemini): handle plan_filename rename and fix scoping bug

Gemini CLI nightly renamed plan_path to plan_filename in exit_plan_mode.
Accept both field names for forward/backward compatibility. Reconstruct
full plan path from transcript_path + session_id + plans/ + filename.

Also hoist planFilename variable out of try block so it's accessible
in the deny output path (was causing ReferenceError).

For provenance purposes, this commit was AI assisted.

* fix(gemini): dim approve button when annotations exist for Gemini CLI

Gemini's hook runner ignores systemMessage on the allow path, so
approve-with-feedback is silently dropped — same limitation as Claude
Code. Extend the existing UI gate to also apply for gemini-cli origin.

For provenance purposes, this commit was AI assisted.

* fix(gemini): add AGENT_CONFIG entry and fix sandbox simulate mode

Register "gemini-cli" in AGENT_CONFIG so the UI shows "Gemini CLI"
with proper badge styling instead of generic "Coding Agent" fallback.

Update sandbox simulate mode to match production input format:
use plan_filename instead of plan_path, include transcript_path,
and simulate the Gemini directory structure for path reconstruction.

For provenance purposes, this commit was AI assisted.
2026-04-02 15:24:10 -07:00