fix: skip recovered files during refresh_managed overwrite check#2919
Merged
Conversation
_is_managed() in install_shared_infra now consults manifest.is_recovered() before treating a hash-matching file as managed. Files marked recovered (pre-existing on disk, not installed by Spec Kit) are no longer overwritten by integration use/switch even when their hash matches the manifest entry. This closes the gap documented in the manifest API: callers using refresh_managed MUST check is_recovered first. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a shared-infra refresh behavior where integration use/switch could overwrite files that were observed (not produced) during install and therefore recorded in the manifest as recovered=True.
Changes:
- Updated the shared-infra “managed file” check to treat manifest-recovered files as never-managed, even when their hash matches the manifest.
- Added a regression test ensuring
integration switchpreserves a recovered shared script without--refresh-shared-infra.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/shared_infra.py |
Adds a guard in _is_managed() to skip overwrite eligibility for manifest.is_recovered(rel) entries. |
tests/integrations/test_integration_subcommand.py |
Adds regression coverage for preserving recovered files during integration switch. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0
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 #2918
Problem
specify integration use/switchcan overwrite team-customized files under.specify/scripts/and.specify/templates/when those files are recorded withrecovered=Truein the manifest.The
_is_managed()function only checked hash equality to decide if a file is "managed" (safe to overwrite). It did not consultmanifest.is_recovered(), violating the manifest API's own documented requirement:Fix
Added a 2-line guard in
_is_managed()that returnsFalsefor recovered files — they are never treated as managed, regardless of hash match.Test
Added
test_switch_preserves_recovered_filesregression test confirming that a team-customized file recorded withrecovered=Truesurvivesintegration switchwithout--force.