feat(cmd): validate numeric subjects on PR/issue/browse verbs#415
Merged
barrettruth merged 1 commit intomainfrom Apr 26, 2026
Merged
feat(cmd): validate numeric subjects on PR/issue/browse verbs#415barrettruth merged 1 commit intomainfrom
barrettruth merged 1 commit intomainfrom
Conversation
Subject specs with kind 'pr' or 'issue' now reject non-numeric values at parse time with a clean error like 'invalid PR number: main' or 'invalid issue number: foo', mirroring the existing 'invalid branch:' and 'invalid commit:' phrasing. Driven by a kind -> pattern table in cmd.lua. Currently: - pr -> ^%d+$ (label: PR number) - issue -> ^%d+$ (label: issue number) Release tags (kind 'release') and CI run ids (kind 'run') stay unconstrained. The browse family's open verb gets kind = 'pr' on its subject so :Forge browse main now produces 'invalid PR number: main' instead of falling through to a 404 from the backend API call. The verb-typo heuristic in M.parse is updated: when the family has only its default verb (browse, review), the second token is always treated as a subject (no possible verb-typo), so subject validation fires cleanly. Multi-verb families (pr, issue, ci, release) keep the existing 'unknown <family> action: <token>' behavior for non-digit tokens, since 'main' could plausibly be a verb typo there.
This was referenced Apr 26, 2026
barrettruth
added a commit
that referenced
this pull request
Apr 26, 2026
…nd init.lua (#418) The cmd-layer subject validator added in #415 used a static subject_kind_labels = { pr = 'PR number' } table that was forge-blind and produced 'invalid PR number: main' even on GitLab where the user types ':Forge mr browse main' or sees 'MR' everywhere else. Per-forge mappings outside the backend modules are forbidden (see AGENTS.md backwards-compatibility section); the backend already exposes the label as forge.Forge.labels.pr_one ('PR' on github/codeberg, 'MR' on gitlab). Logging code consults that field directly via require('forge').detect(). Sites updated: - cmd.lua subject validation site (added in #415): now fetches the active forge inline and uses f.labels.pr_one for the kind == 'pr' branch - cmd.lua subject_error: same inline pattern at the family == 'pr' verb == 'edit' branch and the family == 'review' missing branch - init.lua resolve_pr_action_target: 'missing PR number' was hardcoded, now uses (forge or M.detect()).labels.pr_one No helper functions added. Each site does the lookup inline so there is no abstraction layer hiding which forge is being consulted. Tests cover: - GitLab (mock forge.detect returning labels.pr_one = 'MR'): all four PR-numbered error sites produce 'invalid MR number' / 'missing MR number'; issue-numbered sites unchanged - No forge / nil detect: falls back to 'PR' for both inline and fallback branches
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
1 participant
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.
Problem
Browse-by-number commands and other PR/issue verbs accepted any string
subject without validation:
:Forge browse mainfell through tobrowse_subject('main', scope),which fired an API call that 404'd with a confusing message.
:Forge issue browse maindid the same againstgh/glab/tea issue view.:Forge pr browse main(added in feat: add :Forge browse {num} with cross-forge auto-dispatch #407) had the same gap.The cmd dispatch layer accepted these subjects because the subject specs
(
subject = { kind = 'pr', min = 0, max = 1 }) only constrained arity, notcontent. Issue #410.
Solution
Per-kind validation in
lua/forge/cmd.lua. A newsubject_kind_patternstable drives validation:Validation runs in
M.parse()after the count check. When a subject'skindhas a pattern and the value doesn't match, parsing fails with aclean error mirroring the existing
invalid branch:/invalid commit:phrasing:
:Forge pr browse main->invalid PR number: main:Forge issue browse foo->invalid issue number: foo:Forge browse main->invalid PR number: mainRelease tags (
kind = 'release') and CI run ids (kind = 'run') stayunconstrained per the issue's guidance, since tags can be
v1.2.3etc.The
browsefamily'sopenverb getssubject = { kind = 'pr', min = 0, max = 1 }(was unkinded), so:Forge browse {num}participates in thesame numeric validation.
Verb-typo heuristic refinement.
M.parsehad an existing heuristicthat treats the second token after a family with
default_verbaseither a verb-typo or a subject, distinguishing by whether the token is
digits. With validation now producing a more useful error than the
404-from-CLI fallback, that heuristic is tightened: when a family has
only its default verb (
browse,review), the second token can only bea subject (no verb-typo possible), so it routes through subject
validation and produces
invalid PR number: maininstead of theopaque
unknown <family> action: main.For multi-verb families (
pr,issue,ci,release), theexisting
unknown <family> action: <token>behavior is preservedbecause
main/ed/chekoutcould plausibly be verb typos there.Tests
5 new tests covering:
browse,pr browse,pr ci,pr close,pr merge,review).(
issue browse,issue close,issue edit).(
release browse v1.2.3,release browse release-2024,ci open 24964024953,ci browse abc-123).(
pr main->unknown pr action: main).just ciclean: nix fmt, stylua, biome, selene, lua-language-server,vimdoc-language-server, 638/0/0 busted (up from 633).
Closes #410.