fix: honor template overrides for tasks-template (#2278)#2292
fix: honor template overrides for tasks-template (#2278)#2292Nimraakram22 wants to merge 10 commits intogithub:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to bring the /tasks command into parity with /plan by adding template override resolution for tasks-template.md (overrides → presets → extensions → core) and updating docs accordingly.
Changes:
- Updated
templates/commands/tasks.mdto reference aTASKS_TEMPLATEplaceholder instead of a hardcoded path. - Added new setup scripts for resolving
tasks-template(bash + PowerShell) and wired PowerShell prerequisite checks to run them. - Updated preset documentation to state that the override stack applies to
tasks-template.mdas well.
Show a summary per file
| File | Description |
|---|---|
| templates/commands/tasks.md | Replaces hardcoded tasks template path with a placeholder reference. |
| scripts/powershell/setup-tasks.ps1 | Adds PowerShell template resolution and writes a resolved template file. |
| scripts/powershell/check-prerequisites.ps1 | Attempts to run setup scripts as part of prerequisite checking. |
| scripts/bash/setup-tasks.sh | Adds bash template resolution/export for tasks-template. |
| docs/reference/presets.md | Documents tasks-template as participating in the same override stack as plan-template. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 5
Suggested fix for #2278 / PR #2292The current PR introduces Since Overview
1.
|
mnriem
left a comment
There was a problem hiding this comment.
See comments authored together with Copilot above
ce75d8d to
05b1ea8
Compare
|
Hi @mnriem, thank you for the detailed feedback. I've rewritten the PR from scratch following your suggested approach: What changed:
The bash script also now fails fast with a non-zero exit and a clear error message if the template cannot be resolved, as suggested by Copilot. Please let me know if anything needs further adjustment. Thank you! |
There was a problem hiding this comment.
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 3
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback
|
Thanks for the review @mnriem and Copilot. Fixed:
These were leftover artifacts from the heredoc used to write the files and would have caused a Not changed:
|
There was a problem hiding this comment.
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 2
|
Please address Copilot feedback |
|
Thanks again @mnriem and Copilot. Fixed:
|
There was a problem hiding this comment.
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
|
Please address Test & Lint errors |
|
Hi @mnriem, addressed the Test & Lint failures. The file inventory tests across 6 integration test files (
The 6 remaining failures in All 1388 previously passing tests continue to pass. |
There was a problem hiding this comment.
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
templates/commands/tasks.md:79
- The PR description mentions switching to
{{TASKS_TEMPLATE}}/ exporting via env vars, but this command template currently relies on reading aTASKS_TEMPLATEfield from the JSON emitted by{SCRIPT}. Please either update the PR description (and any related docs) to match this JSON-based flow, or adjust the template to use the mechanism described, so future maintainers aren’t misled about how the template path is provided.
4. **Generate tasks.md**: Read the tasks template from TASKS_TEMPLATE (from the JSON output above) and use it as structure. If TASKS_TEMPLATE is empty, fall back to `.specify/templates/tasks-template.md`. Fill with:
- Files reviewed: 9/9 changed files
- Comments generated: 1
There was a problem hiding this comment.
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 9/9 changed files
- Comments generated: 1
| echo "ERROR: Could not resolve required tasks-template in $REPO_ROOT" >&2 | ||
| echo "Expected shared core template at .specify/templates/tasks-template.md; run 'specify init' or reinstall shared infra to restore it." >&2 |
There was a problem hiding this comment.
The error message implies only the core template location is expected, but the resolver actually searches the full override stack (overrides/presets/extensions/core). If resolution fails, it would be more accurate/helpful to mention that the template was not found in any of those locations and point users to the override path as an alternative, not just re-running init.
| echo "ERROR: Could not resolve required tasks-template in $REPO_ROOT" >&2 | |
| echo "Expected shared core template at .specify/templates/tasks-template.md; run 'specify init' or reinstall shared infra to restore it." >&2 | |
| echo "ERROR: Could not resolve required tasks-template from the template override stack for $REPO_ROOT" >&2 | |
| echo "Template 'tasks-template' was not found in any supported location (overrides, presets, extensions, or shared core). Add an override at .specify/templates/tasks-template.md, or run 'specify init' / reinstall shared infra to restore the shared core template." >&2 |
mnriem
left a comment
There was a problem hiding this comment.
Retriggered with latest and Copilot still has some feedback to be addressed
|
Hi @mnriem, addressed the Copilot feedback. Fixed:
|
There was a problem hiding this comment.
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 9/9 changed files
- Comments generated: 0 new
mnriem
left a comment
There was a problem hiding this comment.
Retriggered with latest and Copilot still has some feedback to be addressed
|
Looks like the last hurdle is the Test & Lint errors that need to be fixed |
|
Hi @mnriem, I've investigated the Test & Lint failures and confirmed they are all pre-existing on Running the same failing tests against
None of these 7 tests reference |
|
I will have to put it on my task list for next week then as I cannot merge unless it is all green across the board |
|
@mnriem i m checking this what about my other fix(powershell): ensure UTF-8 templates are written without BOM |
- Add scripts/bash/setup-tasks.sh mirroring setup-plan.sh pattern - Add scripts/powershell/setup-tasks.ps1 mirroring setup-plan.ps1 pattern - Update tasks.md frontmatter to use dedicated setup-tasks scripts - Resolve tasks template via override stack and emit path as TASKS_TEMPLATE in JSON output - Reference resolved TASKS_TEMPLATE path in generate step instead of hardcoded path
af62833 to
5e7723b
Compare
|
Hi @mnriem, identified and fixed the CI failure. The branch was behind Also confirmed that all remaining failures ( All 1476 non-symlink/non-workflow tests pass on our branch. |
There was a problem hiding this comment.
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (2)
scripts/bash/setup-tasks.sh:60
- This error message points users to add an override at
.specify/templates/tasks-template.md, but the override location in the documented/implemented stack is.specify/templates/overrides/tasks-template.md(core is.specify/templates/tasks-template.md). Update the message so it directs users to the correct override path.
echo "ERROR: Could not resolve required tasks-template from the template override stack for $REPO_ROOT" >&2
echo "Template 'tasks-template' was not found in any supported location (overrides, presets, extensions, or shared core). Add an override at .specify/templates/tasks-template.md, or run 'specify init' / reinstall shared infra to restore the shared core template." >&2
exit 1
scripts/bash/setup-tasks.sh:59
- There are existing integration tests for setup-plan behavior; this new setup-tasks script adds template resolution (TASKS_TEMPLATE) but isn’t covered by tests. Consider adding pytest coverage to assert TASKS_TEMPLATE resolves correctly from overrides/presets/extensions/core and to guard against regressions.
# Resolve tasks template through override stack
TASKS_TEMPLATE=$(resolve_template "tasks-template" "$REPO_ROOT") || true
if [[ -z "$TASKS_TEMPLATE" ]] || [[ ! -f "$TASKS_TEMPLATE" ]]; then
echo "ERROR: Could not resolve required tasks-template from the template override stack for $REPO_ROOT" >&2
echo "Template 'tasks-template' was not found in any supported location (overrides, presets, extensions, or shared core). Add an override at .specify/templates/tasks-template.md, or run 'specify init' / reinstall shared infra to restore the shared core template." >&2
- Files reviewed: 9/9 changed files
- Comments generated: 2
| # Validate branch | ||
| check_feature_branch "$CURRENT_BRANCH" "$HAS_GIT" || exit 1 |
There was a problem hiding this comment.
setup-plan.{sh,ps1} skips feature-branch validation when .specify/feature.json pins a valid FEATURE_DIR (see common.sh feature_json_matches_feature_dir / common.ps1 Test-FeatureJsonMatchesFeatureDir). This new tasks setup script always enforces feature-branch naming, which will cause /tasks to fail on non-feature branches even when feature.json is valid. Mirror setup-plan’s conditional branch check so tasks works in the pinned-feature-dir flow too.
This issue also appears in the following locations of the same file:
- line 55
- line 58
| # Validate branch | |
| check_feature_branch "$CURRENT_BRANCH" "$HAS_GIT" || exit 1 | |
| # Validate branch unless .specify/feature.json pins the resolved feature directory | |
| if ! feature_json_matches_feature_dir "$FEATURE_DIR"; then | |
| check_feature_branch "$CURRENT_BRANCH" "$HAS_GIT" || exit 1 | |
| fi |
| if (-not (Test-FeatureBranch -Branch $paths.CURRENT_BRANCH -HasGit:$paths.HAS_GIT)) { | ||
| exit 1 |
There was a problem hiding this comment.
setup-plan.ps1 bypasses feature-branch validation when .specify/feature.json pins a valid FEATURE_DIR, but setup-tasks.ps1 always enforces the feature-branch pattern. This will make /tasks fail on custom branches even in the pinned-feature-dir workflow. Mirror setup-plan.ps1 by conditionally running Test-FeatureBranch only when Test-FeatureJsonMatchesFeatureDir is false.
| if (-not (Test-FeatureBranch -Branch $paths.CURRENT_BRANCH -HasGit:$paths.HAS_GIT)) { | |
| exit 1 | |
| if (-not (Test-FeatureJsonMatchesFeatureDir -FeatureDir $paths.FEATURE_DIR)) { | |
| if (-not (Test-FeatureBranch -Branch $paths.CURRENT_BRANCH -HasGit:$paths.HAS_GIT)) { | |
| exit 1 | |
| } |
|
Hi @mnriem, addressed the actionable Copilot feedback. Fixed:
Not applicable:
|
This PR resolves #2278 by implementing a unified template resolution flow for the tasks command, bringing it into parity with the plan command.
Key Changes:
Removed hardcoded path in templates/commands/tasks.md in favor of {{TASKS_TEMPLATE}}.
Added setup-tasks.ps1 and setup-tasks.sh to handle the resolution priority stack (Overrides > Presets > Core).
Exported the resolved path via environment variables in the prerequisite check.
Updated docs/reference/presets.md to include tasks-template.md in the documented override stack.
Verification:
Confirmed on Windows 64-bit that local overrides in .specify/templates/overrides/tasks-template.md are correctly loaded and utilized by the AI agent.