mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
fix: preserve .vscode/settings.json and script +x bit on integration upgrade (#3020)
* fix: preserve .vscode/settings.json and script +x bit on integration upgrade During 'specify integration upgrade', Phase 2 stale-cleanup removes files present in the old manifest but absent from the new one. Copilot's setup() merges into an existing .vscode/settings.json and stops tracking it, so the file was being deleted on upgrade (destroying user settings). Add a stale_cleanup_exclusions() hook that integrations use to protect such conditionally-tracked merge targets. Also restore the executable bit on shared .sh scripts after the managed-refresh step on POSIX. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address review on stale-cleanup fix - Normalize stale_cleanup_exclusions() to POSIX before subtracting from manifest keys, so exclusions built with os.path.join / backslashes still match on Windows. - Strengthen test_upgrade_preserves_existing_vscode_settings to add a user-defined key and assert it survives the upgrade (via --force, exercising the merge + stale-cleanup path) instead of the brittle after == before check. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -2,6 +2,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
from pathlib import PurePath
|
||||
|
||||
import typer
|
||||
|
||||
@@ -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:
|
||||
@@ -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}
|
||||
|
||||
@@ -393,6 +393,18 @@ class IntegrationBase(ABC):
|
||||
"""
|
||||
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.
|
||||
|
||||
|
||||
@@ -282,6 +282,17 @@ class CopilotIntegration(IntegrationBase):
|
||||
"""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.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user