Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/specify_cli/integrations/_migrate_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from __future__ import annotations

import os
from pathlib import PurePath

import typer

Expand Down Expand Up @@ -461,6 +462,9 @@ def integration_upgrade(
raise _SharedTemplateRefreshError(
f"Failed to refresh shared infrastructure for '{key}': {exc}"
) from exc
if os.name != "nt":
from .. import ensure_executable_scripts
ensure_executable_scripts(project_root)
new_manifest.save()
_write_integration_json(project_root, installed_key, installed_keys, settings)
if installed_key == key:
Expand All @@ -478,7 +482,13 @@ def integration_upgrade(
# Phase 2: Remove stale files from old manifest that are not in the new one
old_files = old_manifest.files
new_files = new_manifest.files
stale_keys = set(old_files) - set(new_files)
# Exclude integration-declared paths that use conditional manifest tracking
# (e.g. merge targets like .vscode/settings.json) so they are never deleted
# as "stale" while still being actively managed. Manifest keys are stored
# in POSIX form, so normalize the exclusions the same way before subtracting
# (an integration may build paths with os.path.join / backslashes).
exclusions = {PurePath(p).as_posix() for p in integration.stale_cleanup_exclusions()}
stale_keys = (set(old_files) - set(new_files)) - exclusions
if stale_keys:
stale_manifest = IntegrationManifest(key, project_root, version="stale-cleanup")
stale_manifest._files = {k: old_files[k] for k in stale_keys}
Expand Down
12 changes: 12 additions & 0 deletions src/specify_cli/integrations/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,18 @@ def command_filename(self, template_name: str) -> str:
"""
return f"speckit.{template_name}.md"

def stale_cleanup_exclusions(self) -> set[str]:
"""Return project-relative paths that upgrade must never stale-delete.

During ``integration upgrade``, files recorded in a previous manifest
but absent from the freshly written one are treated as stale and
removed. Conditionally-tracked files (e.g. a settings file that the
integration merges into when it already exists, and therefore stops
tracking) would otherwise be deleted even though they are still
managed. Subclasses list such paths here to protect them.
"""
return set()

def commands_dest(self, project_root: Path) -> Path:
"""Return the absolute path to the commands output directory.

Expand Down
11 changes: 11 additions & 0 deletions src/specify_cli/integrations/copilot/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,17 @@ def command_filename(self, template_name: str) -> str:
"""Copilot commands use ``.agent.md`` extension."""
return f"speckit.{template_name}.agent.md"

def stale_cleanup_exclusions(self) -> set[str]:
"""Protect ``.vscode/settings.json`` from upgrade stale-deletion.

``setup()`` records this file in the manifest only when it creates it;
when it already exists the file is merged and intentionally left
untracked. On upgrade the untracked-but-existing file would otherwise
be flagged stale and deleted, destroying user settings (and the file
the integration still manages).
"""
return {".vscode/settings.json"}

def post_process_skill_content(self, content: str) -> str:
"""Inject shared hook guidance into Copilot skill content.

Expand Down
52 changes: 52 additions & 0 deletions tests/integrations/test_integration_subcommand.py
Original file line number Diff line number Diff line change
Expand Up @@ -2272,6 +2272,58 @@ def test_upgrade_migrates_opencode_legacy_dir(self, tmp_path):
f"found: {[f.name for f in core_remaining]}"
)

def test_upgrade_preserves_existing_vscode_settings(self, tmp_path):
"""Regression: copilot upgrade must not stale-delete .vscode/settings.json.

On init the file is created and recorded in the manifest. On upgrade,
setup() merges into the now-existing file and intentionally stops
tracking it, so without ``stale_cleanup_exclusions()`` the Phase 2
stale cleanup would delete it (destroying the user's settings).
"""
project = _init_project(tmp_path, "copilot")
settings = project / ".vscode" / "settings.json"
assert settings.is_file(), "init should create .vscode/settings.json"
before = json.loads(settings.read_text(encoding="utf-8"))
assert before, "settings.json should contain managed defaults"

# Simulate a user editing their settings: add a custom key that the
# integration does not manage. It must survive the upgrade.
before["editor.fontSize"] = 17
settings.write_text(json.dumps(before), encoding="utf-8")

result = _run_in_project(project, [
"integration", "upgrade", "copilot",
"--script", "sh", "--force",
])
assert result.exit_code == 0, result.output

assert settings.is_file(), ".vscode/settings.json must survive upgrade"
after = json.loads(settings.read_text(encoding="utf-8"))
assert after.get("editor.fontSize") == 17, (
"user-defined settings must be preserved after upgrade"
)

def test_upgrade_restores_executable_bit_on_shared_scripts(self, tmp_path):
"""Regression: scripts refreshed by the managed-refresh step stay +x."""
if os.name == "nt":
pytest.skip("POSIX execute bits are not meaningful on Windows")
project = _init_project(tmp_path, "copilot")
script = project / ".specify" / "scripts" / "bash" / "check-prerequisites.sh"
assert script.is_file()
# Simulate a perms-losing install (e.g. wheel extraction dropping +x).
script.chmod(0o644)
assert not (script.stat().st_mode & 0o111)

result = _run_in_project(project, [
"integration", "upgrade", "copilot",
"--script", "sh",
])
assert result.exit_code == 0, result.output

assert script.stat().st_mode & 0o111, (
"shared .sh scripts must be executable after upgrade"
)


# ── Full lifecycle ───────────────────────────────────────────────────

Expand Down