Skip to content

Commit ee9afea

Browse files
Quratulain-bilalQuratulain-bilal
authored andcommitted
fix(integration): address Copilot review on switch shared-infra refresh
- Clarify install_shared_infra docstring: force overwrites regular files but always preserves symlinks (safe-destination check refuses to follow). - Print refresh_hint only for preserved_user_files; skipped_files keeps the generic remediation. Avoids misleading guidance when files were merely skipped (not detected as customized). - Catch ValueError from the safe-destination check and bucket the path under a new symlinked_files warning instead of aborting the switch. - Restore templates/constitution-template.md to upstream (drop accidental leading blank lines).
1 parent 005f24c commit ee9afea

2 files changed

Lines changed: 45 additions & 15 deletions

File tree

src/specify_cli/shared_infra.py

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -250,9 +250,12 @@ def install_shared_infra(
250250
When ``refresh_managed`` is True, files whose on-disk hash still matches
251251
the previously recorded manifest hash are overwritten with the bundled
252252
version. Files whose hash diverges are treated as user customizations and
253-
preserved with a warning. ``force=True`` overwrites everything regardless.
254-
``refresh_hint`` is shown after the customization warning to tell the user
255-
which flag would overwrite their customizations.
253+
preserved with a warning. ``force=True`` overwrites every regular file
254+
(symlinks and symlinked-parent destinations are always preserved with a
255+
warning — the safe-destination check refuses to follow them so writes
256+
cannot escape the project root). ``refresh_hint`` is shown after the
257+
customization warning to tell the user which flag would overwrite their
258+
customizations.
256259
"""
257260
from .integrations.manifest import _sha256
258261

@@ -270,6 +273,7 @@ def _is_managed(rel: str, dst: Path) -> bool:
270273

271274
skipped_files: list[str] = []
272275
preserved_user_files: list[str] = []
276+
symlinked_files: list[str] = []
273277
planned_copies: list[tuple[Path, str, bytes, int]] = []
274278
planned_templates: list[tuple[Path, str, str]] = []
275279

@@ -287,6 +291,22 @@ def _decide_overwrite(rel: str, dst: Path) -> tuple[bool, str | None]:
287291
return False, "skip"
288292
return False, "skip"
289293

294+
def _safe_dest_or_bucket(dst: Path, rel: str, *, parent_must_exist: bool = True) -> bool:
295+
"""Run the safe-destination check and bucket symlinked paths.
296+
297+
Returns True when the destination is safe to consider (write or skip).
298+
Returns False (and records *rel* under ``symlinked_files``) when the
299+
destination or any of its ancestors is a symlink — those paths can't
300+
be written to safely, but they shouldn't abort the whole switch
301+
either. They're surfaced as a separate "symlinked" warning bucket.
302+
"""
303+
try:
304+
_ensure_safe_shared_destination(project_path, dst, parent_must_exist=parent_must_exist)
305+
except ValueError:
306+
symlinked_files.append(rel)
307+
return False
308+
return True
309+
290310
scripts_src = shared_scripts_source(core_pack=core_pack, repo_root=repo_root)
291311
if scripts_src.is_dir():
292312
dest_scripts = project_path / ".specify" / "scripts"
@@ -302,8 +322,9 @@ def _decide_overwrite(rel: str, dst: Path) -> tuple[bool, str | None]:
302322

303323
rel_path = src_path.relative_to(variant_src)
304324
dst_path = dest_variant / rel_path
305-
_ensure_safe_shared_destination(project_path, dst_path, parent_must_exist=False)
306325
rel = dst_path.relative_to(project_path).as_posix()
326+
if not _safe_dest_or_bucket(dst_path, rel, parent_must_exist=False):
327+
continue
307328
write, bucket = _decide_overwrite(rel, dst_path)
308329
if not write:
309330
if bucket == "preserved":
@@ -324,8 +345,9 @@ def _decide_overwrite(rel: str, dst: Path) -> tuple[bool, str | None]:
324345
continue
325346

326347
dst = dest_templates / src.name
327-
_ensure_safe_shared_destination(project_path, dst)
328348
rel = dst.relative_to(project_path).as_posix()
349+
if not _safe_dest_or_bucket(dst, rel):
350+
continue
329351
write, bucket = _decide_overwrite(rel, dst)
330352
if not write:
331353
if bucket == "preserved":
@@ -353,14 +375,24 @@ def _decide_overwrite(rel: str, dst: Path) -> tuple[bool, str | None]:
353375
)
354376
for path in skipped_files:
355377
console.print(f" {path}")
356-
if refresh_hint:
357-
console.print(refresh_hint)
358-
else:
359-
console.print(
360-
"To refresh shared infrastructure, run "
361-
"[cyan]specify init --here --force[/cyan] or "
362-
"[cyan]specify integration upgrade --force[/cyan]."
363-
)
378+
console.print(
379+
"To refresh shared infrastructure, run "
380+
"[cyan]specify init --here --force[/cyan] or "
381+
"[cyan]specify integration upgrade --force[/cyan]."
382+
)
383+
384+
if symlinked_files:
385+
console.print(
386+
f"[yellow]⚠[/yellow] Skipped {len(symlinked_files)} symlinked shared "
387+
"infrastructure file(s) — symlinks are never overwritten because they "
388+
"may resolve outside the project root:"
389+
)
390+
for path in symlinked_files:
391+
console.print(f" {path}")
392+
console.print(
393+
"To restore the bundled version, remove or replace the symlink manually, "
394+
"then re-run the command."
395+
)
364396

365397
if preserved_user_files:
366398
console.print(

templates/constitution-template.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
2-
31
# [PROJECT_NAME] Constitution
42
<!-- Example: Spec Constitution, TaskFlow Constitution, etc. -->
53

0 commit comments

Comments
 (0)