fix(task): populate usage.cmd for subcommand-only tasks; share make_usage_ctx#9431
fix(task): populate usage.cmd for subcommand-only tasks; share make_usage_ctx#9431
Conversation
…sage_ctx
The early-return in parse_usage_values_from_task fired when both
spec.cmd.args and spec.cmd.flags were empty, before the new subcommand
handling. Tasks that defined only subcommands and referenced
{{ usage.cmd }} in deps got an empty IndexMap, so the value never
reached the Tera context.
Fix the early-return condition to also consider subcommands, then drop
the duplicated to_tera_value/snake_case iteration by reusing
TaskScriptParser::make_usage_ctx (now pub(crate)). Preserve the
defensive empty-cmd insertion when subcommands are defined but none was
selected, matching the existing behavior callers rely on.
Add an e2e regression test exercising the subcommand-only path via
{{ usage.cmd }} in a depends template.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Greptile SummaryThis PR fixes two issues in the task dependency template subsystem: (1) an early-return bug in Confidence Score: 5/5Safe to merge — targeted bug fix with no regressions and a new regression test covering the fixed path. Both changes (early-return guard and de-duplication) are small, correct, and mirrored by existing logic in No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[parse_usage_values_from_task] --> B{spec has args\nOR flags\nOR subcommands?}
B -- No --> C[return empty IndexMap]
B -- Yes --> D[parse args via usage::Parser]
D -- parse error --> C
D -- Ok --> E[make_usage_ctx &po\nargs + flags + cmd]
E --> F{spec has subcommands\nAND cmd key absent?}
F -- Yes --> G[insert cmd = empty string]
F -- No --> H[return values]
G --> H
style C fill:#f9f,stroke:#333
style H fill:#9f9,stroke:#333
Reviews (1): Last reviewed commit: "fix(task): populate usage.cmd for subcom..." | Re-trigger Greptile |
There was a problem hiding this comment.
Code Review
This pull request addresses a regression where usage.cmd was not correctly populated for tasks that only define subcommands, which caused issues in dependency templates. The changes refactor parse_usage_values_from_task to utilize TaskScriptParser::make_usage_ctx and ensure that usage.cmd is initialized as an empty string when subcommands are present but none are selected. Feedback was provided regarding a missing fallback in the error handling path and the need to maintain deterministic ordering of arguments and flags when converting from a HashMap to an IndexMap.
| return Ok(IndexMap::new()); | ||
| } | ||
| }; | ||
| let mut values = IndexMap::new(); | ||
| let to_tera_value = |val: &usage::parse::ParseValue| -> tera::Value { | ||
| use tera::Value; | ||
| use usage::parse::ParseValue::*; | ||
| match val { | ||
| MultiBool(v) => Value::Number(serde_json::Number::from(v.len())), | ||
| MultiString(v) => Value::Array(v.iter().map(|s| Value::String(s.clone())).collect()), | ||
| Bool(v) => Value::Bool(*v), | ||
| String(v) => Value::String(v.clone()), | ||
| } | ||
| }; | ||
| for (arg, val) in &po.args { | ||
| values.insert(arg.name.to_snake_case(), to_tera_value(val)); | ||
| } | ||
| for (flag, val) in &po.flags { | ||
| values.insert(flag.name.to_snake_case(), to_tera_value(val)); | ||
| } | ||
| if !spec.cmd.subcommands.is_empty() { | ||
| let cmd = po.cmds.iter().skip(1).map(|c| c.name.clone()).join(" "); | ||
| values.insert("cmd".to_string(), tera::Value::String(cmd)); | ||
| let mut values: IndexMap<String, tera::Value> = | ||
| TaskScriptParser::make_usage_ctx(&po).into_iter().collect(); | ||
| // `make_usage_ctx` only inserts `cmd` when a subcommand was actually selected. | ||
| // Templates referencing `{{ usage.cmd }}` should still resolve (to "") when | ||
| // subcommands are defined in the spec but none was selected. | ||
| if !spec.cmd.subcommands.is_empty() && !values.contains_key("cmd") { | ||
| values.insert("cmd".to_string(), tera::Value::String(String::new())); | ||
| } | ||
| Ok(values) |
There was a problem hiding this comment.
The current implementation misses the usage.cmd fallback in the Err case (line 2082). If a task with subcommands is called with invalid or missing arguments, usage::Parser::parse will fail, and the function will return an empty map. This causes Tera rendering errors in dependency templates that expect usage.cmd to be defined.
Additionally, reusing make_usage_ctx (which returns a HashMap) and collecting it into an IndexMap introduces non-determinism in the order of arguments/flags in the Tera context. While lookups are unaffected, any iteration over usage will be unordered, which is a regression from the previous implementation. I've added a .sorted_by_key() call with an owned key to ensure determinism and avoid lifetime issues in the closure.
| return Ok(IndexMap::new()); | |
| } | |
| }; | |
| let mut values = IndexMap::new(); | |
| let to_tera_value = |val: &usage::parse::ParseValue| -> tera::Value { | |
| use tera::Value; | |
| use usage::parse::ParseValue::*; | |
| match val { | |
| MultiBool(v) => Value::Number(serde_json::Number::from(v.len())), | |
| MultiString(v) => Value::Array(v.iter().map(|s| Value::String(s.clone())).collect()), | |
| Bool(v) => Value::Bool(*v), | |
| String(v) => Value::String(v.clone()), | |
| } | |
| }; | |
| for (arg, val) in &po.args { | |
| values.insert(arg.name.to_snake_case(), to_tera_value(val)); | |
| } | |
| for (flag, val) in &po.flags { | |
| values.insert(flag.name.to_snake_case(), to_tera_value(val)); | |
| } | |
| if !spec.cmd.subcommands.is_empty() { | |
| let cmd = po.cmds.iter().skip(1).map(|c| c.name.clone()).join(" "); | |
| values.insert("cmd".to_string(), tera::Value::String(cmd)); | |
| let mut values: IndexMap<String, tera::Value> = | |
| TaskScriptParser::make_usage_ctx(&po).into_iter().collect(); | |
| // `make_usage_ctx` only inserts `cmd` when a subcommand was actually selected. | |
| // Templates referencing `{{ usage.cmd }}` should still resolve (to "") when | |
| // subcommands are defined in the spec but none was selected. | |
| if !spec.cmd.subcommands.is_empty() && !values.contains_key("cmd") { | |
| values.insert("cmd".to_string(), tera::Value::String(String::new())); | |
| } | |
| Ok(values) | |
| let mut values = IndexMap::new(); | |
| if !spec.cmd.subcommands.is_empty() { | |
| values.insert("cmd".to_string(), tera::Value::String(String::new())); | |
| } | |
| return Ok(values); | |
| } | |
| }; | |
| let mut values: IndexMap<String, tera::Value> = TaskScriptParser::make_usage_ctx(&po) | |
| .into_iter() | |
| .sorted_by_key(|(k, _)| k.clone()) | |
| .collect(); | |
| // make_usage_ctx only inserts cmd when a subcommand was actually selected. | |
| // Templates referencing {{ usage.cmd }} should still resolve (to "") when | |
| // subcommands are defined in the spec but none was selected. | |
| if !spec.cmd.subcommands.is_empty() && !values.contains_key("cmd") { | |
| values.insert("cmd".to_string(), tera::Value::String(String::new())); | |
| } | |
| Ok(values) |
References
- The key returned by closures in iteration or sorting methods should be an owned value to prevent lifetime errors where the key needs to outlive the closure call.
Hyperfine Performance
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.4.24 x -- echo |
21.9 ± 0.4 | 21.2 | 24.2 | 1.00 |
mise x -- echo |
22.6 ± 0.5 | 21.5 | 24.4 | 1.03 ± 0.03 |
mise env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.4.24 env |
21.7 ± 0.6 | 20.8 | 27.2 | 1.00 |
mise env |
22.1 ± 0.3 | 21.4 | 23.7 | 1.02 ± 0.03 |
mise hook-env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.4.24 hook-env |
22.5 ± 0.5 | 21.4 | 26.5 | 1.00 |
mise hook-env |
23.6 ± 0.6 | 22.0 | 26.1 | 1.05 ± 0.04 |
mise ls
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.4.24 ls |
23.6 ± 0.5 | 22.4 | 25.7 | 1.00 |
mise ls |
23.6 ± 0.6 | 22.4 | 25.8 | 1.00 ± 0.03 |
xtasks/test/perf
| Command | mise-2026.4.24 | mise | Variance |
|---|---|---|---|
| install (cached) | 156ms | 160ms | -2% |
| ls (cached) | 79ms | 82ms | -3% |
| bin-paths (cached) | 81ms | 82ms | -1% |
| task-ls (cached) | 827ms | 805ms | +2% |
### 🚀 Features - **(task)** add --name-only flag to mise tasks ls by @jdx in [#9435](#9435) ### 🐛 Bug Fixes - **(Dockerfile)** install copr-cli via dnf for better dependency management by @bestagi in [#9421](#9421) - **(aqua)** drop empty-releases fallback to tags by @jdx in [#9443](#9443) - **(docs)** fix theme flicker on docs by @vhespanha in [#9427](#9427) - **(lockfile)** update global lockfile on upgrade by @jdx in [#9442](#9442) - **(ls-remote)** omit rolling/prerelease from JSON when false by @jdx in [#9439](#9439) - **(task)** support usage refs in dependency template tags by @jdx in [#9424](#9424) - **(task)** populate usage.cmd for subcommand-only tasks; share make_usage_ctx by @jdx in [#9431](#9431) - **(task)** resolve sandbox allow_read/allow_write against task dir by @jdx in [#9428](#9428) ### 📚 Documentation - **(site)** add self-hosted page tracker via Cloudflare Worker, drop GoatCounter by @jdx in [#9430](#9430) ### New Contributors - @vhespanha made their first contribution in [#9427](#9427) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Summary
Follow-up to #9424 addressing two issues raised in review (Cursor Bugbot):
Early-return bug —
parse_usage_values_from_taskreturned an empty map when bothspec.cmd.argsandspec.cmd.flagswere empty, before reaching the new subcommand handling. Tasks defined with only subcommands (no top-level args/flags) that referenced{{ usage.cmd }}in deps got an emptyIndexMap, so the value never landed in the Tera context. Fixed by includingspec.cmd.subcommands.is_empty()in the early-return condition.Duplicated conversion logic —
to_tera_valueand the args/flags-to-snake-case iteration inparse_usage_values_from_taskduplicatedTaskScriptParser::make_usage_ctx, and the two had already drifted slightly. Promotedmake_usage_ctxtopub(crate)and reused it. Preserved the defensive empty-cmdinsertion when subcommands are defined but none was selected.Test plan
mise run formatmise run lintcargo test --bin mise task::tests(34 tests pass)mise run test:e2e e2e/tasks/test_task_dep_args(includes new regression test for subcommand-only deps via{{ usage.cmd }})mise run test:e2e e2e/tasks/test_task_usage_cmd(no regression in pre-existing subcommand behavior)🤖 Generated with Claude Code
Note
Low Risk
Low risk: small, localized change to usage-context extraction plus an added e2e regression test; behavior only differs for tasks whose
usagedefines subcommands without root args/flags.Overview
Fixes dependency-template rendering for subcommand-only tasks by ensuring
parse_usage_values_from_taskno longer early-returns before considering subcommands, and by reusingTaskScriptParser::make_usage_ctxto build theusagemap.When a task defines subcommands but none is selected, the code now guarantees
usage.cmdis present (as an empty string) so{{ usage.cmd }}references in dependency templates consistently resolve.Adds an e2e regression test covering
dependstemplates that branch onusage.cmdfor subcommand-onlyusagespecs.Reviewed by Cursor Bugbot for commit 97fa3ff. Bugbot is set up for automated code reviews on this repo. Configure here.