add --require-spec to check-prerequisites - #4367
Open
HuzaifaChaudary wants to merge 1 commit into
Open
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.