add --require-spec to check-prerequisites - #4367
Conversation
the script resolves FEATURE_SPEC and reports it but never checks that the file is there, so analyze and converge pass the prerequisite check and then fail later reading a spec that does not exist, without the run specify first guidance the script gives for every other artifact the flag is opt in so nothing changes for callers that do not read the spec. analyze and converge pass it because they do added to all three script variants because the parity tests compare their help text and error output against each other
analyze and converge read spec.md so the py runner needs the same guard as sh and ps. the python script already had the flag but no caller passed it so the python variant kept the old late failure.
There was a problem hiding this comment.
馃煛 Changes recommended
The PowerShell script cannot parse, and its new behavior lacks parity coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Balanced
| @requires_bash | ||
| def test_python_require_spec_matches_bash(prereq_repo: Path) -> None: |
| # -RequireSpec Require spec.md to exist (for analysis phase) | ||
| # -RequireSpec Require spec.md to exist (for analysis phase) | ||
| -RequireTasks Require tasks.md to exist (for implementation phase) |
the require spec line got duplicated over the require tasks line and the second copy lost its leading hash, so line 14 was executable powershell sitting above CmdletBinding and the whole script stopped parsing. every test_ps_ test failed on ci because of it. parser says 2 errors before and none after: line 20 unexpected attribute CmdletBinding line 21 unexpected token param also add the powershell half of the require spec parity test, which is what would have caught this.
|
both findings were right and both are fixed in the parse break. exactly as described. the that is why every the missing powershell coverage. added worth saying plainly that this test is what would have caught the break, and i did not write it the first time. the bash and python halves passed the whole way through. local run after the fix, with pwsh 7.5.2: the two are same two, same way. they were also in the ubuntu 3.13 failure list, so on ci they may well have been collateral from the parse break, but either way they fail without this branch. |
Closes #4364
problem
check-prerequisitesvalidatesFEATURE_DIR,plan.md, andtasks.mdunder--require-tasks, each with an error naming the command to run. it resolvesFEATURE_SPEC, exports it in the json payload and prints it in text mode, but never checks that the file exists.analyzeandconvergeboth call it and then readspec.mddirectly. with the spec missing the prerequisite check exits 0, and the failure surfaces later without the "run specify first" guidance the script gives for everything else, which is the thing this script exists to prevent.fix
a
--require-specflag, shaped exactly like--require-tasks:it is opt in, as suggested in the issue, so nothing changes for callers that do not read the spec.
analyzeandconvergepass it because they do.all three variants
the flag is in bash, powershell and python. that is not gold plating, the parity tests compare help text, json output and error shapes across the three, so adding it to one would have broken them.
scripts/bash/check-prerequisites.sh--require-specscripts/powershell/check-prerequisites.ps1-RequireSpecscripts/python/check_prerequisites.py--require-spectemplates/commands/analyze.mdtemplates/commands/converge.mdtests
test_python_require_spec_matches_bashcovers three states across bash and python:it fails on
mainwithERROR: Unknown option '--require-spec'.verification
mainruff check scripts teststhe 10 pre existing failures are the composed template parity tests and are unrelated to this.
disclaimer: this contribution was prepared with the assistance of an ai agent. i reproduced the gap first, checked which callers actually read the spec, and ran the parity suite locally before opening this.