Make the just recipe generator arity-aware for parameterized recipes - #308
Make the just recipe generator arity-aware for parameterized recipes#308warp-agent-staging[bot] wants to merge 3 commits into
Conversation
`just.json` points its variadic recipe argument at the Rust `recipes` generator, which was a static script: it answered every position with the same recipe list. After a parameterized recipe that is the wrong answer — with `deploy env:` in the justfile, `just deploy <TAB>` offered another recipe name where the `env` parameter belongs. Read the parameters `just --dump --dump-format json` already declares and build the recipes command from the command-line tokens, relaying them to the output callback on a context line. The callback replays how `just` binds arguments — each recipe greedily takes up to its declared parameter count, `+`/`*` absorbs the rest — and offers recipe names only where one belongs. `just --list` spells its parameters out too, so that tier stays arity-aware; `just --summary` carries none and keeps completing recipe names. Co-Authored-By: Warp Agent <agent@warp.dev>
|
@warp-agent-staging[bot] I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR makes the Just recipe generator arity-aware by relaying committed tokens into the generator output, parsing recipe arities from JSON/list output, and suppressing recipe suggestions while a recipe parameter is being filled. No approved spec context was provided, and I did not find security-specific findings.
Concerns
- Option values that happen to look like parameterized recipe names can be misclassified as recipes, suppressing completions after valid
justoptions. - The
--listfallback records arity only for canonical recipe names, so aliases shown in list output still behave as zero-context tokens and can offer recipes at alias parameter positions.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Two gaps in the arity replay, from review of #308. An option's value can spell a recipe name. `just --justfile deploy <TAB>` relayed `deploy` as a chosen recipe and suppressed the first recipe completion. Relaying now tracks how many following tokens each option claims — one for most, two for `--set`, the rest of the line for `--command` and friends — and collapses every claimed token to the opaque placeholder. A `--name=value` token carries its value inline and claims nothing. Switches are untouched, so the recipe right after one still owns its parameters. `just --list` renders a recipe's aliases into its doc comment as `[alias: d]` or `[aliases: d, dep]`, either side of the doc text depending on `--alias-style`. Those are now parsed into the arity map, so `just d <TAB>` for an alias of `deploy env:` stops offering recipe names where `env` belongs, matching what the JSON dump tier already did. A real recipe of the same name always wins, so doc text that merely reads like alias metadata cannot overwrite it. Co-Authored-By: Warp Agent <agent@warp.dev>
Summary
Follow-up to #307 (merged as
50fe37c), which wired"generatorName": "recipes"onto the variadic recipe position injson/just.json. That madejust <TAB>list Justfile recipes, but the generator was a static script — it answered every variadic position with the same recipe list. With a justfile recipedeploy env:,just deploy <TAB>offered another recipe name where theenvparameter belongs. The Fig-style JS generator #307 replaced was arity-aware; the Rust one was not.Root cause. A generator's
on_complete_callbackonly receives its command's output — it never sees the command line — so the callback had no way to tell a recipe position from a parameter position.The fix.
recipesbecomes aGenerator::command_from_tokens:warp-just-context:line printed ahead of the recipe data. Only tokens that could name ajustrecipe ([A-Za-z_][A-Za-z0-9_-]*) are relayed; everything else collapses to a-placeholder, so arbitrary command-line text never reaches the generated shell command.just --dump --dump-format jsonalready declares each recipe'sparameters(withkind: singular | plus | star) and itsaliases, so the callback now reads an arity per invocable name — private recipes and aliases included, since both can appear on the command line even though neither is suggested.justbinds arguments (Justfile::group_by_recipe): each recipe greedily takes up to its declared parameter count,+/*absorbs everything remaining, and unclaimed tokens arejust's own options and their values. Recipe names are offered only where a recipe actually belongs.Defaulted parameters count toward the arity, matching
just's greedy binding (just test buildpassesbuildtotest filter="", it does not runbuild).Fallback tiers.
just --listspells its parameters out in each entry (clean *paths,release version bump="patch"), so that tier stays arity-aware too.just --summarycarries no parameter data at all, so it degrades to today's behavior and keeps completing recipe names rather than suppressing positions it cannot reason about. Every tier failing (no justfile, or nojust) still yields an empty list.No change to
json/just.json— the spec already points at this generator.Verification
script/presubmitis green: prettier,cargo fmt --check,cargo clippy --all-targets --all-features -D warnings, andcargo test(135 passed, 0 failed).Regression test:
test_recipes_are_not_offered_at_a_parameter_position—just deploy <TAB>against the parameterizeddeploy env:recipe. Verified failing before the change and passing after:The
justgenerator suite grew from 9 to 29 tests, covering each acceptance criterion:+/*claims every remaining positionjust --show <TAB>still completes recipe namesThe JSON and
--listfixtures were captured from a realjust1.58.0 run against a justfile exercising documented, undocumented, parameterized, defaulted, variadic, aliased, grouped, and both flavors of private recipe. Headless library change with no rendered UI surface, so no computer-use visual proof applies.Follow-up
Shipping this to the client needs a
warp-command-signaturesrev bump inwarpdotdev/warp, as with #307.Rework changes
Review of the first pass raised two behavioral gaps in the arity replay. Both are fixed in
cc33aa8, each with a regression test verified failing-before / passing-after.Option values could shadow the first recipe position. Relaying preserved any recipe-shaped token, so
just --justfile deploy <TAB>replayeddeploy— the justfile path — as a chosen recipe and suppressed the recipe completion that belonged there. Relaying now tracks how many following tokens each option claims: one value for the single-value options (--justfile/-f,--working-directory/-d,--shell, and ~25 more), two for--set <VARIABLE> <VALUE>, and the rest of the line for the value-list options (--command/-c,--show/-s,--usage,--clean,--list/-l,--evaluate). Every claimed token collapses to the opaque placeholder; a--name=valuetoken carries its value inline and claims nothing after it. This is the general case, not a special case for--justfile. Switches are deliberately untouched, sojust --unstable deploy <TAB>still suppresses recipe names atenv— a blanket "anything after a flag is opaque" rule would have reintroduced the original bug there. New tests:test_an_option_value_never_claims_the_recipe_position,test_a_switch_does_not_swallow_the_recipe_that_follows_it.--listaliases did not bind their target's arity. The fallback tier kept alias metadata only in the suggestion description, sojust d <TAB>for an alias ofdeploy env:still offered recipe names whereenvbelongs.parse_listnow parses[alias: d]/[aliases: d, dep]out of each entry's doc comment — either side of the doc text, since--alias-stylerenders itright(default) orleft— and registers those names with the target's arity, matching the JSON dump tier.--alias-style separatealready worked, since it emits each alias as its own entry line. A real recipe of the same name always wins, so doc text that merely reads like alias metadata cannot overwrite a genuine arity. New tests:test_list_output_binds_an_alias_to_the_target_arity,test_list_output_binds_an_alias_rendered_before_the_doc_text.script/presubmitgreen again after the rework: 135 tests passed, 0 failed.Refs: #307, APP-5208
Originating thread: https://warpdev.slack.com/archives/C0BDQDW8V5E/p1785982310270809
Conversation: https://staging.warp.dev/conversation/e8509844-861d-4373-87e5-c3436f0da125
Run: https://oz.staging.warp.dev/runs/019fd4f3-2f06-734a-a881-2ec117613831
This PR was generated with Oz.