Skip to content

Commit c072c4e

Browse files
fix(integration): preserve customizations and add regression tests (#2293)
Address Copilot review on #2375: 1. Replace unconditional force=True with hash-aware refresh_managed mode in _install_shared_infra. Files whose on-disk hash matches the previously recorded manifest hash are refreshed; files whose hash has diverged are treated as user customizations and preserved with a clear warning. The user can opt back into a full overwrite by passing --force to switch. 2. Update --force help text on integration switch so the documented semantics include 'overwrite customized shared infrastructure files', not just uninstall behavior. 3. Add two CLI-level regression tests: - test_switch_refreshes_stale_managed_shared_infra reproduces #2293 by hashing a stale vendored common.sh into the manifest, then asserting it is replaced after switch. - test_switch_preserves_user_customized_shared_infra modifies the file without updating the manifest hash and asserts the customization survives the switch and the warning is emitted.
1 parent 3aa01f5 commit c072c4e

2 files changed

Lines changed: 142 additions & 16 deletions

File tree

src/specify_cli/__init__.py

Lines changed: 82 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -724,6 +724,7 @@ def _install_shared_infra(
724724
tracker: StepTracker | None = None,
725725
force: bool = False,
726726
invoke_separator: str = ".",
727+
refresh_managed: bool = False,
727728
) -> bool:
728729
"""Install shared infrastructure files into *project_path*.
729730
@@ -735,9 +736,14 @@ def _install_shared_infra(
735736
placeholders using *invoke_separator* (``"."`` for markdown agents,
736737
``"-"`` for skills agents).
737738
738-
When *force* is ``True``, existing files are overwritten with the
739-
latest bundled versions. When ``False`` (default), only missing
740-
files are added and existing ones are skipped.
739+
Overwrite policy:
740+
741+
* ``force=True`` — overwrite every existing file.
742+
* ``refresh_managed=True`` — overwrite only files whose on-disk hash
743+
still matches the previously recorded manifest hash (i.e. unmodified
744+
files installed by spec-kit). Files with diverging hashes are
745+
treated as user customizations and preserved with a warning.
746+
* Default — only add missing files; existing ones are skipped.
741747
742748
Returns ``True`` on success.
743749
"""
@@ -747,6 +753,30 @@ def _install_shared_infra(
747753
core = _locate_core_pack()
748754
manifest = IntegrationManifest("speckit", project_path, version=get_speckit_version())
749755

756+
# When refresh_managed is requested, load prior hashes so we only
757+
# overwrite files that are still unmodified relative to the previous
758+
# install.
759+
prior_hashes: dict[str, str] = {}
760+
if refresh_managed and not force:
761+
try:
762+
prior = IntegrationManifest.load("speckit", project_path)
763+
prior_hashes = prior.files
764+
except (FileNotFoundError, ValueError):
765+
prior_hashes = {}
766+
767+
def _is_managed(rel_posix: str, abs_path: Path) -> bool:
768+
"""True when the file's current hash matches the recorded one."""
769+
expected = prior_hashes.get(rel_posix)
770+
if not expected:
771+
return False
772+
try:
773+
from .integrations.manifest import _sha256
774+
return _sha256(abs_path) == expected
775+
except OSError:
776+
return False
777+
778+
preserved_user_files: list[str] = []
779+
750780
# Scripts
751781
if core and (core / "scripts").is_dir():
752782
scripts_src = core / "scripts"
@@ -768,13 +798,21 @@ def _install_shared_infra(
768798
if src_path.is_file():
769799
rel_path = src_path.relative_to(variant_src)
770800
dst_path = dest_variant / rel_path
771-
if dst_path.exists() and not force:
772-
skipped_files.append(str(dst_path.relative_to(project_path)))
801+
rel_posix = dst_path.relative_to(project_path).as_posix()
802+
should_write = (
803+
not dst_path.exists()
804+
or force
805+
or (refresh_managed and _is_managed(rel_posix, dst_path))
806+
)
807+
if not should_write:
808+
if refresh_managed and dst_path.exists() and rel_posix in prior_hashes:
809+
preserved_user_files.append(rel_posix)
810+
else:
811+
skipped_files.append(str(dst_path.relative_to(project_path)))
773812
else:
774813
dst_path.parent.mkdir(parents=True, exist_ok=True)
775814
shutil.copy2(src_path, dst_path)
776-
rel = dst_path.relative_to(project_path).as_posix()
777-
manifest.record_existing(rel)
815+
manifest.record_existing(rel_posix)
778816

779817
# Page templates (not command templates, not vscode-settings.json)
780818
if core and (core / "templates").is_dir():
@@ -789,16 +827,24 @@ def _install_shared_infra(
789827
for f in templates_src.iterdir():
790828
if f.is_file() and f.name != "vscode-settings.json" and not f.name.startswith("."):
791829
dst = dest_templates / f.name
792-
if dst.exists() and not force:
793-
skipped_files.append(str(dst.relative_to(project_path)))
830+
rel_posix = dst.relative_to(project_path).as_posix()
831+
should_write = (
832+
not dst.exists()
833+
or force
834+
or (refresh_managed and _is_managed(rel_posix, dst))
835+
)
836+
if not should_write:
837+
if refresh_managed and dst.exists() and rel_posix in prior_hashes:
838+
preserved_user_files.append(rel_posix)
839+
else:
840+
skipped_files.append(str(dst.relative_to(project_path)))
794841
else:
795842
content = f.read_text(encoding="utf-8")
796843
content = IntegrationBase.resolve_command_refs(
797844
content, invoke_separator
798845
)
799846
dst.write_text(content, encoding="utf-8")
800-
rel = dst.relative_to(project_path).as_posix()
801-
manifest.record_existing(rel)
847+
manifest.record_existing(rel_posix)
802848

803849
if skipped_files:
804850
console.print(
@@ -812,6 +858,17 @@ def _install_shared_infra(
812858
"[cyan]specify integration upgrade --force[/cyan]."
813859
)
814860

861+
if preserved_user_files:
862+
console.print(
863+
f"[yellow]⚠[/yellow] Preserved {len(preserved_user_files)} customized shared "
864+
"infrastructure file(s) (hash differs from previous install):"
865+
)
866+
for f in preserved_user_files:
867+
console.print(f" {f}")
868+
console.print(
869+
"To overwrite customizations, re-run with [cyan]--force[/cyan]."
870+
)
871+
815872
manifest.save()
816873
return True
817874

@@ -2291,7 +2348,7 @@ def integration_uninstall(
22912348
def integration_switch(
22922349
target: str = typer.Argument(help="Integration key to switch to"),
22932350
script: str | None = typer.Option(None, "--script", help="Script type: sh or ps (default: from init-options.json or platform default)"),
2294-
force: bool = typer.Option(False, "--force", help="Force removal of modified files during uninstall"),
2351+
force: bool = typer.Option(False, "--force", help="Force removal of modified files during uninstall and overwrite customized shared infrastructure files"),
22952352
integration_options: str | None = typer.Option(None, "--integration-options", help='Options for the target integration'),
22962353
):
22972354
"""Switch from the current integration to a different one."""
@@ -2384,10 +2441,19 @@ def integration_switch(
23842441
# Refresh shared infrastructure to the current CLI version. Switching
23852442
# integrations is exactly when stale vendored shared scripts (e.g.
23862443
# update-agent-context.sh that pre-dates the target integration's
2387-
# supported-agent list) would silently break the new integration, so
2388-
# always overwrite with the bundled copies. User content lives under
2389-
# specs/ and is not touched here. See #2293.
2390-
_install_shared_infra(project_root, selected_script, force=True, invoke_separator=target_integration.effective_invoke_separator(parsed_options))
2444+
# supported-agent list) would silently break the new integration.
2445+
#
2446+
# Use refresh_managed=True so only files that match their previously
2447+
# recorded hash are overwritten — user customizations are detected via
2448+
# hash divergence and preserved with a warning. Pass --force to
2449+
# overwrite customizations as well. See #2293.
2450+
_install_shared_infra(
2451+
project_root,
2452+
selected_script,
2453+
force=force,
2454+
refresh_managed=True,
2455+
invoke_separator=target_integration.effective_invoke_separator(parsed_options),
2456+
)
23912457
if os.name != "nt":
23922458
ensure_executable_scripts(project_root)
23932459

tests/integrations/test_integration_subcommand.py

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,66 @@ def test_switch_preserves_shared_infra(self, tmp_path):
356356
assert shared_script.exists()
357357
assert shared_script.read_text(encoding="utf-8") == shared_content
358358

359+
def test_switch_refreshes_stale_managed_shared_infra(self, tmp_path):
360+
"""Regression for #2293: stale managed shared scripts get refreshed on switch."""
361+
import hashlib
362+
363+
project = _init_project(tmp_path, "claude")
364+
shared_script = project / ".specify" / "scripts" / "bash" / "common.sh"
365+
bundled_bytes = shared_script.read_bytes()
366+
367+
# Simulate a stale vendored script: write truncated content as bytes
368+
# (write_text would translate \n→\r\n on Windows and break the hash)
369+
# and update the speckit manifest hash so the stale copy is treated
370+
# as "managed" (installed by spec-kit, not a user customization).
371+
stale_bytes = b"#!/usr/bin/env bash\n# stale vendored copy\n"
372+
shared_script.write_bytes(stale_bytes)
373+
374+
manifest_path = project / ".specify" / "integrations" / "speckit.manifest.json"
375+
manifest_data = json.loads(manifest_path.read_text(encoding="utf-8"))
376+
manifest_data["files"][".specify/scripts/bash/common.sh"] = (
377+
hashlib.sha256(stale_bytes).hexdigest()
378+
)
379+
manifest_path.write_text(json.dumps(manifest_data), encoding="utf-8")
380+
381+
old_cwd = os.getcwd()
382+
try:
383+
os.chdir(project)
384+
result = runner.invoke(app, [
385+
"integration", "switch", "copilot",
386+
"--script", "sh",
387+
], catch_exceptions=False)
388+
finally:
389+
os.chdir(old_cwd)
390+
assert result.exit_code == 0
391+
392+
# Stale managed file should be replaced by the bundled version
393+
assert shared_script.read_bytes() == bundled_bytes
394+
395+
def test_switch_preserves_user_customized_shared_infra(self, tmp_path):
396+
"""User customizations (hash divergence from manifest) survive switch without --force."""
397+
project = _init_project(tmp_path, "claude")
398+
shared_script = project / ".specify" / "scripts" / "bash" / "common.sh"
399+
400+
# User customization: append bytes but do NOT update manifest hash,
401+
# so on-disk hash diverges from the recorded one.
402+
original = shared_script.read_bytes()
403+
custom_bytes = original + b"\n# user customization\n"
404+
shared_script.write_bytes(custom_bytes)
405+
406+
old_cwd = os.getcwd()
407+
try:
408+
os.chdir(project)
409+
result = runner.invoke(app, [
410+
"integration", "switch", "copilot",
411+
"--script", "sh",
412+
], catch_exceptions=False)
413+
finally:
414+
os.chdir(old_cwd)
415+
assert result.exit_code == 0
416+
assert shared_script.read_bytes() == custom_bytes
417+
assert "Preserved" in result.output
418+
359419
def test_switch_from_nothing(self, tmp_path):
360420
"""Switch when no integration is installed should just install the target."""
361421
project = tmp_path / "bare"

0 commit comments

Comments
 (0)