fix: Make directory task include also support toml files#10219
Conversation
|
Linter diff in the way? Review this PR in Change Stack to focus on meaningful changes and expand context only when needed. 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:
📝 WalkthroughWalkthroughThis PR extends remote git includes to support individual TOML task files alongside directory includes. The implementation now partitions discovered files into TOML and executable scripts, loading TOML tasks first. Comprehensive E2E tests verify the functionality, and documentation is updated to explain the new capability with examples. ChangesRemote TOML Task File Includes
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
davidolrik
changed the title
Greptile SummaryThis PR fixes directory-based
Confidence Score: 5/5The change is well-scoped, covered by new e2e tests, and matches the documented behaviour — safe to merge. The core logic correctly partitions files and reuses the existing, tested No files require special attention beyond the self-referential doc link noted in Important Files Changed
Reviews (8): Last reviewed commit: "fix: Clarify URL format placeholder for ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for path in toml_files { | ||
| tasks.extend(load_task_file(config, &path, config_root, task_config_dir).await?); | ||
| } |
There was a problem hiding this comment.
config_root for directory-resident .toml tasks differs from neighbouring executable tasks
load_task_file sets task.config_root to the config_root parameter, which here is the parent config's root (the directory containing mise.toml). Executable tasks loaded from the same directory get root (the scanned directory itself) as their root via Task::from_path. Any relative path used in a toml task's run, dir, or similar fields will therefore resolve against the parent config root instead of the toml file's own directory, which is likely surprising and inconsistent with what script-based tasks in the same include directory do. The single-file toml path at line 2304 has the same characteristic, but the inconsistency becomes more visible now that both file types can coexist in one directory.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/config/mod.rs (1)
2325-2332: ⚡ Quick winMake directory file loading order deterministic.
Task precedence can become filesystem-order dependent here. Please sort each bucket (
toml_files,exec_files) before loading so overrides are stable across platforms.Suggested diff
- let (toml_files, exec_files): (Vec<_>, Vec<_>) = all_files + let (mut toml_files, mut exec_files): (Vec<_>, Vec<_>) = all_files .into_iter() .filter(|p| is_toml(p) || file::is_executable(p)) .partition(|p| is_toml(p)); + toml_files.sort(); + exec_files.sort();🤖 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 `@src/config/mod.rs` around lines 2325 - 2332, The code partitions all_files into toml_files and exec_files but doesn't sort them, making load order nondeterministic; sort both vectors (toml_files and exec_files) after partitioning (e.g., call sort_unstable or sort on the Vec<PathBuf> using their Ord impl or a string key) before iterating and calling load_task_file so file loading/override precedence is stable; ensure you still iterate toml_files first and then exec_files if that ordering is required.
🤖 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.
Inline comments:
In `@docs/tasks/task-configuration.md`:
- Line 761: The in-page anchor for the "task toml file format" link is
incorrect: update the link target in the sentence that currently reads "Included
`.toml` files use the [task toml file format](`#task-config-includes`)" to point
to the actual section anchor `### task_config.includes` (i.e. change the
fragment to `#task_config.includes`) so the link matches the generated header ID
and fixes the MD051 lint error.
---
Nitpick comments:
In `@src/config/mod.rs`:
- Around line 2325-2332: The code partitions all_files into toml_files and
exec_files but doesn't sort them, making load order nondeterministic; sort both
vectors (toml_files and exec_files) after partitioning (e.g., call sort_unstable
or sort on the Vec<PathBuf> using their Ord impl or a string key) before
iterating and calling load_task_file so file loading/override precedence is
stable; ensure you still iterate toml_files first and then exec_files if that
ordering is required.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 51733c89-fd4b-4948-b988-9775d64e2ba7
📒 Files selected for processing (4)
docs/tasks/task-configuration.mde2e/helpers/scripts/git_http_backend_server.pye2e/tasks/test_task_remote_git_includessrc/config/mod.rs
davidolrik
force-pushed
the
remote-toml-tasks
branch
from
1f06f29 to
5188034
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/tasks/task-configuration.md (1)
761-761:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix broken in-page anchor and invalid markdown syntax.
The link has two issues:
- Backticks around the URL make the markdown syntax invalid
- The fragment doesn't match the section anchor
Remove the backticks and update the fragment to match the generated anchor for
### task_config.includes.🔗 Proposed fix
-Included `.toml` files use the [task toml file format](`#task-config-includes`) (the keys are task names — there is no `[tasks.…]` prefix). The repository will be cloned and cached in `MISE_CACHE_DIR/remote-git-tasks-cache`. Tasks from the include will be loaded as if they were local. You can disable caching with `MISE_TASK_REMOTE_NO_CACHE=true` or the `--no-cache` flag. +Included `.toml` files use the [task toml file format](`#task_configincludes`) (the keys are task names — there is no `[tasks.…]` prefix). The repository will be cloned and cached in `MISE_CACHE_DIR/remote-git-tasks-cache`. Tasks from the include will be loaded as if they were local. You can disable caching with `MISE_TASK_REMOTE_NO_CACHE=true` or the `--no-cache` flag.🤖 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 `@docs/tasks/task-configuration.md` at line 761, Update the markdown link in the sentence that currently reads "Included `.toml` files use the [task toml file format](`#task-config-includes`)" by removing the backticks around the URL and fixing the fragment to the generated anchor for the section named "task_config.includes" (change the link target to "`#task_config.includes`"); keep the inline `.toml` code span but ensure the link syntax is valid and points to the correct anchor.
🤖 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.
Duplicate comments:
In `@docs/tasks/task-configuration.md`:
- Line 761: Update the markdown link in the sentence that currently reads
"Included `.toml` files use the [task toml file format](`#task-config-includes`)"
by removing the backticks around the URL and fixing the fragment to the
generated anchor for the section named "task_config.includes" (change the link
target to "`#task_config.includes`"); keep the inline `.toml` code span but ensure
the link syntax is valid and points to the correct anchor.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 10c2a47a-daac-430c-9ffb-763f53b0c1c4
📒 Files selected for processing (4)
docs/tasks/task-configuration.mde2e/helpers/scripts/git_http_backend_server.pye2e/tasks/test_task_remote_git_includessrc/config/mod.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- e2e/tasks/test_task_remote_git_includes
- e2e/helpers/scripts/git_http_backend_server.py
- src/config/mod.rs
davidolrik
force-pushed
the
remote-toml-tasks
branch
2 times, most recently
from
18e6807 to
9538884
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/tasks/task-configuration.md (1)
747-747:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify the URL format placeholder for the ref parameter.
The format shows
?<ref>but the examples use?ref=v1.0.0, whererefis the query parameter name. Consider updating to?ref=<ref>to make it explicit thatrefis the parameter name and<ref>is the value placeholder.📝 Suggested clarification
-URL format: `git::<protocol>://<url>//<path>?<ref>` +URL format: `git::<protocol>://<url>//<path>?ref=<ref>`🤖 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 `@docs/tasks/task-configuration.md` at line 747, Update the URL format example to explicitly show the query parameter name by replacing the ambiguous `?<ref>` with `?ref=<ref>` so it matches the examples (e.g., `?ref=v1.0.0`); ensure the documentation line `URL format: git::<protocol>://<url>//<path>?<ref>` is changed to `URL format: git::<protocol>://<url>//<path>?ref=<ref>` and update any nearby examples or references to use the same `?ref=<ref>` notation for consistency.
🤖 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.
Outside diff comments:
In `@docs/tasks/task-configuration.md`:
- Line 747: Update the URL format example to explicitly show the query parameter
name by replacing the ambiguous `?<ref>` with `?ref=<ref>` so it matches the
examples (e.g., `?ref=v1.0.0`); ensure the documentation line `URL format:
git::<protocol>://<url>//<path>?<ref>` is changed to `URL format:
git::<protocol>://<url>//<path>?ref=<ref>` and update any nearby examples or
references to use the same `?ref=<ref>` notation for consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5d2c07c7-70c4-456d-9188-73fab10ced8e
📒 Files selected for processing (4)
docs/tasks/task-configuration.mde2e/helpers/scripts/git_http_backend_server.pye2e/tasks/test_task_remote_git_includessrc/config/mod.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- e2e/tasks/test_task_remote_git_includes
- e2e/helpers/scripts/git_http_backend_server.py
- src/config/mod.rs
davidolrik
force-pushed
the
remote-toml-tasks
branch
2 times, most recently
from
5d933a8 to
d230e4d
Compare
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
davidolrik
force-pushed
the
remote-toml-tasks
branch
2 times, most recently
from
f299d22 to
6efedf7
Compare
davidolrik
force-pushed
the
remote-toml-tasks
branch
from
6efedf7 to
ed25ce1
Compare
2 participants
In the docs it states that
This PR makes that true
Summary by CodeRabbit
New Features
Documentation
Tests