Repository navigation
fix(review): error loudly on unknown review options instead of silently dropping them - #1483
Merged
Merged
Conversation
…ly dropping them parseReviewArgs now reports argument-shape problems through an always-present errors[] field instead of letting unknown dash-prefixed tokens fall into the ignored positional list. All four host surfaces refuse to start a session on a parse error: the CLI and the opencode-review bridge exit 1 with the errors on stderr, and the Pi and OpenCode plugins notify through their hosts. Plain non-dashed words stay tolerated for slash-command hosts that forward raw user prose. The loop is index-based so value-taking flags can consume their value token.
1 task
backnotprop
added a commit
to oorestisime/plannotator
that referenced
this pull request
Sep 30, 2026
backnotprop#1483 kept plain positional words tolerated because slash-command hosts (Claude Code, Gemini, Copilot bang prefixes; OpenCode; Pi) forward raw user words verbatim. Directory targets now resolve through resolveReviewTarget, following the annotate tolerant-args precedent: - a sole word, or any path-shaped word (/, ., ~), is a directory candidate; a path-shaped typo or a file still errors - other words are ignored with a notice and the invoking cwd is reviewed as before - a PR/MR URL is found anywhere among the words; two targets error OpenCode: prose keeps the opencode-review command, so it still works on an old binary; the update hint also covers pre-0.27.11 binaries, which answer 'No plan content in hook event'. Tests realpath their tmp roots so they pass on macOS.
backnotprop
added a commit
that referenced
this pull request
Sep 30, 2026
* feat(review): accept repository and worktree directory targets * fix(review): keep prose tolerated when review words name no directory #1483 kept plain positional words tolerated because slash-command hosts (Claude Code, Gemini, Copilot bang prefixes; OpenCode; Pi) forward raw user words verbatim. Directory targets now resolve through resolveReviewTarget, following the annotate tolerant-args precedent: - a sole word, or any path-shaped word (/, ., ~), is a directory candidate; a path-shaped typo or a file still errors - other words are ignored with a notice and the invoking cwd is reviewed as before - a PR/MR URL is found anywhere among the words; two targets error OpenCode: prose keeps the opencode-review command, so it still works on an old binary; the update hint also covers pre-0.27.11 binaries, which answer 'No plan content in hook event'. Tests realpath their tmp roots so they pass on macOS. --------- Co-authored-by: Michael Ramos <mdramos8@gmail.com>
YeKc1M
pushed a commit
to YeKc1M/plannotator
that referenced
this pull request
Oct 1, 2026
…otprop#1644) * feat(review): accept repository and worktree directory targets * fix(review): keep prose tolerated when review words name no directory backnotprop#1483 kept plain positional words tolerated because slash-command hosts (Claude Code, Gemini, Copilot bang prefixes; OpenCode; Pi) forward raw user words verbatim. Directory targets now resolve through resolveReviewTarget, following the annotate tolerant-args precedent: - a sole word, or any path-shaped word (/, ., ~), is a directory candidate; a path-shaped typo or a file still errors - other words are ignored with a notice and the invoking cwd is reviewed as before - a PR/MR URL is found anywhere among the words; two targets error OpenCode: prose keeps the opencode-review command, so it still works on an old binary; the update hint also covers pre-0.27.11 binaries, which answer 'No plan content in hook event'. Tests realpath their tmp roots so they pass on macOS. --------- Co-authored-by: Michael Ramos <mdramos8@gmail.com>
backnotprop
added a commit
that referenced
this pull request
Oct 5, 2026
* feat(annotate): several files in one review (0.29 PR 4, bundles) plannotator annotate spec.md mock.html notes.md (every argument an existing file named by its path) opens ONE review of the files, in the typed order, with one decision. No flag, no file-count limit. - Shared rule (annotate-target.ts, vendored to Pi): probeAnnotateBundlePath, the bundle selection, resolveAnnotateBundleFiles (type and 2MB checks that name the file). Prose, URLs, folders and searched names keep the #1483 tiers; the ambiguity error gains a hint on hosts that open bundles. Strict gates take only a single target or only file paths (else exit 2). - Server mode annotate-bundle in Bun and Pi: /api/plan carries bundle, /api/doc follows the single-file rule per file, history saved per text file at open, per-file draft copies (#1710), host status documents, archive surface annotate-bundle with target.documents. - CLI: host result record gains documents; the agent message names every file. - Editor: first file opens by itself, header switcher (2 of 3 · name), the Files tab and All files panel in bundle order, export one section per file in bundle order, Ask AI surface line names file N of M. - Tool contract: description line for lists, plannotatorBundleSubject, isOlderCliBundleRefusal; Bash take-over maps several file paths to a list. Mod launches bundles and maps an older CLI's refusal to an update message. - Pi and OpenCode (embedded and CLI bridge) slash commands open bundles too. Co-Authored-By: Claude <noreply@anthropic.com> * fix(annotate): bundle review follow-ups - Move annotate-bundle out of @plannotator/core into packages/shared (no new core export, so the published-UI smoke passes without a core publish). - A list of file paths with a missing file fails naming it (shared 'missing' selection) on the CLI, Pi and OpenCode; tool list entries are passed as paths (bare word -> ./word) so the tool and Bash take-over get the same rule. - Strict refusal decided by probe only (no fetch, live probe or Resolved: line). - A symlinked bundle file is served: its real directory is a root for it. - The CLI uses the shared selection end to end (a.md ./a.md opens a.md). - .env named as refused; bundle files have no Close pill and leaving a linked document returns to the last bundle file; docs; engine-harness bundle case. Co-Authored-By: Claude <noreply@anthropic.com> * fix(annotate): an existing path is never reported missing A stray . or .. (or any existing directory) beside file paths was read as a missing file ('File not found: .'), where main opened the file. The missing selection now needs the new pathExists option (annotatePathExists): only a token with nothing at its path, and nothing found by search, is missing. Wired on the CLI, Pi and OpenCode; test rows on the shared rule and the CLI. Symlink-root comment corrected; AGENTS.md notes the dotted-word edge. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
parseReviewArgsnow reports argument-shape problems through an always-presenterrors: string[]field, and unknown dash-prefixed tokens become loud errors instead of being silently dropped. All four host surfaces refuse to start a session on a parse error:plannotator review): errors on stderr +Run 'plannotator review --help' for usage., exit 1client.app.logand returnsctx.ui.notifyand returns (vendored parser; a parity test guards a forgotten re-vendor)Plain non-dashed words stay tolerated — slash-command hosts forward raw user prose verbatim, and that tolerance is load-bearing (pinned by the existing test). The parser loop is now index-based so value-taking flags can consume their value token (prerequisite for
--base/--diff-type, which land separately and are not part of this PR).Release-note-worthy behavior change
plannotator review(and/plannotator-reviewon Claude Code, OpenCode, and Pi) now errors on unknown dash-prefixed options instead of silently ignoring them. Previouslyplannotator review --bse main— or any typo'd/unsupported flag — silently opened an ordinary review as if nothing happened; a dashed token could even shadow a PR URL (review --bse <PR_URL>silently became a local review). Now such invocations fail loudly withUnknown review option: --bse, matching the annotate contract ("unrecognized dash-prefixed tokens … error the way it always did"). Invocations like/plannotator-review --verbosethat previously "worked" by accident will now refuse.Why now
This is the prerequisite for the
--base/--diff-typereview open-state flags: without strict unknown-flag handling, an old binary or a stale vendored parser would silently swallow the new flags on some hosts — the degrade matrix must have no "silently ignored" cell.Tests
packages/shared/review-args.test.ts: existing whole-object assertions updated forerrors: []; new tests for the silent-swallow bug (--bse main),=-form flags, multiple unknown tokens, and PR-URL shadowing.apps/pi-extension/review-args-parity.test.ts(new): the vendored parser must report unknown tokens — catches a forgottenvendor.shrun.bun run typecheckclean; fullbun testunder a tempPLANNOTATOR_DATA_DIR: 4357 pass / 902 skip, 3 timing-flaky failures in unrelated server suites that did not reproduce on re-run (0 fails).