fix: use missing_ok for temp file cleanup to avoid masking errors - #3803
Open
Quratulain-bilal wants to merge 3 commits into
Open
fix: use missing_ok for temp file cleanup to avoid masking errors#3803Quratulain-bilal wants to merge 3 commits into
Quratulain-bilal wants to merge 3 commits into
Conversation
The Python port's mtime fallback discovered plans with a one-level specs/*/plan.md glob, so a scoped layout created via SPECIFY_FEATURE_DIRECTORY (specs/<scope>/<feature>/plan.md) was missed when feature.json is absent — the fallback returned no plan and the managed context section omitted the 'at <plan>' line. The bash and PowerShell twins were already fixed to recurse (github#3024); the Python twin was left behind. Switch to specs.rglob('plan.md') with the same symlink-safe containment check the bash twin uses (resolve each candidate and confirm it stays within the project root before ranking by mtime), so a plan reached through a specs/ symlink pointing outside the project is not selected. Adds parity regression tests (vs bash and vs PowerShell) covering a nested specs/<scope>/<feature>/plan.md; both fail on the pre-fix one-level glob. Fixes github#3733
The recursive fallback resolves each candidate before the relative_to() containment check, but nothing exercised that path. Add a parity test for a plan reachable only through a specs/ symlink pointing outside the project: relative_to() is lexical and would accept it, emitting an in-project-looking path for an out-of-project file. Both the bash twin and the Python port skip it, so the "at <plan>" line is omitted. Also correct the module docstring, which still described the fallback as scanning specs/*/plan.md one level deep.
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
1 participant
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.
Replace if temp_path.exists(): temp_path.unlink() with emp_path.unlink(missing_ok=True) in 3 files: integrations/manifest.py, shared_infra.py, _utils.py. The old pattern could raise OSError if unlink() fails, masking the original exception in inally/except blocks.