fix(schema): forbid task-only bool fields on task templates#10242
Conversation
These fields parsed on templates but were never merged into tasks because Task stores them as plain bools. That mismatch is a bug, not dead code: schema allowed the keys while runtime silently ignored them. Reject them at parse time and in JSON Schema so templates cannot declare output behavior that does not apply.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRemove output-control flags from TaskTemplate; JSON schema forbids hide/quiet/raw/interactive/raw_args on templates; e2e tests and fixtures updated to reject templates with those flags; docs clarified that templates cannot set these options. ChangesOutput control flags removed from task templates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
e2e/config/test_schema_tombi (1)
159-166: ⚡ Quick winExpand schema e2e coverage to include
hideandrawtoo.This block currently asserts only
quiet, so schema regressions onhide/rawwould not be caught at e2e level.Suggested update
-include = ["mise-bad.toml", "mise-bad-age.toml", "mise-bad-env-directive.toml", "mise-bad-tmpl.toml", "mise-bad-tmpl-quiet.toml", "mise-bad-os.toml", "mise-bad-watch-files.toml"] +include = ["mise-bad.toml", "mise-bad-age.toml", "mise-bad-env-directive.toml", "mise-bad-tmpl.toml", "mise-bad-tmpl-output-flag.toml", "mise-bad-os.toml", "mise-bad-watch-files.toml"] @@ -# Verify that quiet/hide/raw are rejected on task_templates -cat >"$HOME/workdir/mise-bad-tmpl-quiet.toml" <<'TOML' -[task_templates.base] -quiet = true -TOML - -assert_fail "$TOMBI_LINT mise-bad-tmpl-quiet.toml" +# Verify that quiet/hide/raw are rejected on task_templates +for flag in quiet hide raw; do +cat >"$HOME/workdir/mise-bad-tmpl-output-flag.toml" <<TOML +[task_templates.base] +$flag = true +TOML +assert_fail "$TOMBI_LINT mise-bad-tmpl-output-flag.toml" +done🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/config/test_schema_tombi` around lines 159 - 166, The test only checks that a task_templates.base file with the "quiet" key is rejected, so add coverage for "hide" and "raw" by creating similar test inputs and asserting failure for each; specifically, update the e2e case that writes "$HOME/workdir/mise-bad-tmpl-quiet.toml" (and the assert_fail invoking "$TOMBI_LINT mise-bad-tmpl-quiet.toml") to also create equivalents containing task_templates.base.hide = true and task_templates.base.raw = true (or a single TOML with all three keys) and call assert_fail "$TOMBI_LINT <filename>" for each to ensure hide and raw are rejected like quiet.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@e2e/config/test_schema_tombi`:
- Around line 159-166: The test only checks that a task_templates.base file with
the "quiet" key is rejected, so add coverage for "hide" and "raw" by creating
similar test inputs and asserting failure for each; specifically, update the e2e
case that writes "$HOME/workdir/mise-bad-tmpl-quiet.toml" (and the assert_fail
invoking "$TOMBI_LINT mise-bad-tmpl-quiet.toml") to also create equivalents
containing task_templates.base.hide = true and task_templates.base.raw = true
(or a single TOML with all three keys) and call assert_fail "$TOMBI_LINT
<filename>" for each to ensure hide and raw are rejected like quiet.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 79603649-6fc3-4df2-9c74-afd08f2eacea
📒 Files selected for processing (4)
docs/tasks/templates.mde2e/config/test_schema_tombischema/mise.jsonsrc/task/task_template.rs
Greptile SummaryThis PR closes a long-standing mismatch between schema and runtime by forbidding the five task-only boolean flags (
Confidence Score: 5/5Safe to merge — only removes unused struct fields, tightens the schema, updates docs, and adds targeted tests; no runtime behavior changes. All three layers (schema, struct, docs/tests) are updated consistently. The five forbidden fields were either already ignored at runtime or never parsed, so removing them from TaskTemplate cannot break existing configs. The JSON Schema false-property + unevaluatedProperties: false pattern is valid and correctly rejects the flags in any compliant validator. No files require special attention. Important Files Changed
Reviews (6): Last reviewed commit: "Merge branch 'main' into fix/task-templa..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Remove the no-op struct fields and forbid them in JSON Schema only. Serde ignores unknown template keys at runtime; schema guides editors.
risu729
changed the title
risu729
changed the title
Extend task_template schema overrides and docs to cover these task-only bool fields. Same rationale as hide/quiet/raw: never merged from templates.
risu729
changed the title
|
This PR currently has failing checks. If this continues for 7 days, it will be closed automatically. This is warning day 1 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
|
This PR currently has failing checks. If this continues for 7 days, it will be closed automatically. This is warning day 2 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
|
This PR currently has failing checks. If this continues for 7 days, it will be closed automatically. This is warning day 3 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
|
This PR currently has failing checks. If this continues for 7 days, it will be closed automatically. This is warning day 4 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
|
This PR currently has failing checks. If this continues for 7 days, it will be closed automatically. This is warning day 5 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
|
This PR currently has failing checks. If this continues for 7 days, it will be closed automatically. This is warning day 6 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
2 participants
Summary
hide,quiet,raw,interactive, andraw_argson[task_templates.*]in JSON Schema so editors stop suggesting keys that never appliedTaskTemplatefields forhide/quiet/raw(they parsed but were never merged into tasks)Why these flags are not supported on templates
Task templates were added in #7873. From the start,
hide,quiet, andrawwere never merged from templates into tasks.interactive(#8491) andraw_args(#9118) were added to tasks later and were never wired into template merge either.All five share the same limitation:
Taskstores these as plainboolvalues (defaultfalse), so mise cannot tell "unset" from "explicitly set tofalse"quiet = falsewhen the template hasquiet = trueThe struct still parsed
hide/quiet/raw(copied from task schema), and JSON Schema inherited all oftask_propsintotask_template, so editors suggested every task flag even though runtime silently ignored the unsupported ones. Docs already said output flags are not carried over; schema disagreed.This PR fixes that mismatch. Runtime still ignores unknown template keys (no
deny_unknown_fields); schema lint catches invalid configs in editors.Proper inheritance would need tri-state
Taskfields (tracked separately).Changes
schema/mise.json: forbidhide/quiet/raw/interactive/raw_argsontask_templateviafalseproperty schemasTaskTemplate: remove unusedhide/quiet/rawfields; keep merge comment explaining why these bool fields are not mergeddocs/tasks/templates.md: document all five as not supported on templatese2e/config/test_schema_tombi: schema lint rejects each forbidden field on templatesTest plan
cargo test task_templatemise run test:e2e test_schema_tombiSummary by CodeRabbit
Documentation
Schema
Tests