fix(integration): refresh shared infra on `integration switch` by Quratulain-bilal · Pull Request #2375 · github/spec-kit

Quratulain-bilal · 2026-04-27T02:35:27Z

Fixes #2293 — specify integration switch left stale vendored shared scripts under
.specify/scripts/{bash,powershell}/ from the previous integration, breaking the new one.

Root cause

integration_switch() called _install_shared_infra(...) with the default force=False. The merge-missing-only behavior
preserved older copies (e.g. update-agent-context.sh whose supported-agent table did not list newer agents like pi), so
the freshly installed integration wrapper delegated to a stale shared script and failed with Unknown agent type 'pi'.

Fix

Introduces a hash-aware refresh_managed mode in _install_shared_infra() and wires integration switch to use it.

Default switch behavior (refresh_managed=True, force=False):

Files whose on-disk SHA-256 still matches the previously recorded speckit.manifest.json hash are refreshed to the
bundled version (this fixes the bug — stale managed copies get replaced).
Files whose hash diverges from the recorded one are treated as user customizations and preserved with a warning. The
new manifest re-records the prior hash so future refreshes can still tell managed vs. customized apart.
Symlinked destinations are never overwritten (could escape the project root); they are treated as customizations.

Explicit override: a new --refresh-shared-infra flag on integration switch opts in to overwriting customized
shared-infra files as well. This is decoupled from --force, which retains its original meaning (force-remove modified
files during the previous integration's uninstall).

User content under specs/ is untouched in all modes.

Behavior on integration switch

No flags → stale managed files are refreshed; user customizations are preserved (warning printed).
--force → same as above for shared infra; --force only affects the previous integration's uninstall step
(force-removes modified command files).
--refresh-shared-infra → stale managed files refreshed AND user customizations overwritten with the bundled version.

Tests

test_switch_refreshes_stale_managed_shared_infra — regression: a managed file whose hash matches the manifest gets
refreshed.
test_switch_preserves_user_customized_shared_infra — customizations (hash divergence) survive switch.
test_switch_refresh_shared_infra_overwrites_customizations — --refresh-shared-infra overwrites customizations.
test_switch_force_alone_does_not_overwrite_shared_customizations — verifies --force and --refresh-shared-infra are
decoupled.

Full test sweep: pytest tests/integrations/ — all green.

Copilot

Pull request overview

Updates specify integration switch <key> to refresh the project’s vendored shared infrastructure so switching integrations can’t leave stale .specify/scripts/* copies that break newer integrations (per #2293).

Changes:

Make integration switch call _install_shared_infra(..., force=True, ...) so shared scripts/templates are overwritten with the bundled (CLI-version) copies.
Expand inline rationale in integration_switch() explaining why overwrite is needed during switches.

Show a summary per file

File	Description
`src/specify_cli/__init__.py`	Forces shared infra refresh during integration switching to prevent stale vendored scripts from breaking the newly selected integration.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 1/1 changed files
Comments generated: 2

mnriem · 2026-04-27T14:16:05Z

Please address Copilot feedback. Note that if the switch command will overwrite existing infra files we should force the user to use the --force upon switch. That way it is clear that they opted into overwriting modified infra files?

…ithub#2293) Address Copilot review on github#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 github#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.

Quratulain-bilal · 2026-04-27T14:39:52Z

@mnriem this is exactly what commit c072c4e implements:

Without --force — only files whose on-disk SHA-256 still matches the hash recorded in speckit.manifest.json
are refreshed (i.e. unmodified managed files). Any file with a divergent hash is treated as user-modified, preserved
as-is, and reported in a warning that hints at --force.
With --force — explicit opt-in to overwrite customizations, matching the existing --force semantics on
uninstall (help text updated to reflect both roles).

Net effect: the only files rewritten silently are the stale-but-unmodified vendored copies that caused #2293. Anything
the user touched is left alone unless they pass --force.

Covered by two new CLI-level tests: test_switch_refreshes_stale_managed_shared_infra and
test_switch_preserves_user_customized_shared_infra.

Happy to flip to a stricter contract (always require --force even for stale managed refreshes) if you prefer — just
say the word.

Copilot

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: 3

mnriem

See comments above

Three concerns from Copilot's review on github#2375: 1. Manifest hash retention: load the prior speckit manifest and seed the new manifest's _files dict with its hashes, so files we don't touch this run keep their tracking entry. Future refreshes can still tell managed-vs-customized apart. 2. Symlink safety: never overwrite a symlinked destination. Following it could read or write outside the project root. Treat symlinked dst paths as user customizations and preserve them. 3. Decouple --force: introduce a separate --refresh-shared-infra flag on `integration switch`. --force now only controls modified-file removal during the previous integration's uninstall (its original meaning). Users who want to overwrite their shared-infra customizations must opt in explicitly with --refresh-shared-infra. The preservation warning now points at the new flag. Tests: two new regression tests for the decoupling — one verifies --refresh-shared-infra overwrites customizations, the other verifies --force alone does not.

Copilot

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: 3

mnriem · 2026-04-28T12:38:56Z

Please address Copilot feedback. If not applicable, please explain why

… case Address Copilot follow-up on github#2375: the "re-run with --refresh-shared-infra" hint was hardcoded but `_install_shared_infra()` is also reached from `init`, `integration install`, and `integration upgrade` — none of which expose that flag. Symlinked destinations are never overwritten by any flag and need manual intervention. - Split the preserved-files warning into two: customized files (hash divergence) and symlinked files. They have different remediations. - Add a `refresh_hint` parameter so each caller passes its own remediation text. Only `integration switch` passes the `--refresh-shared-infra` hint. Other call sites omit it (no false suggestion of a flag they don't have). - Symlink message tells the user to remove/replace the symlink manually, since no flag will resolve it.

Quratulain-bilal · 2026-04-28T13:04:31Z

Addressed in a5adde5:
▎
▎ - Split the warning into two: customized shared files (hash divergence) and symlinked shared files — they have
▎ different remediations.
▎ - _install_shared_infra() now takes a refresh_hint parameter so each caller passes its own remediation text. Only
▎ integration switch suggests --refresh-shared-infra; init/install/upgrade no longer show that hint since they don't
▎ expose the flag.
▎ - Symlinked destinations now show a separate message telling the user to remove or replace the symlink manually, since
▎ no flag resolves that case.

Copilot

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (1)

src/specify_cli/init.py:868

Same symlink issue as the scripts loop: checking only dst.is_symlink() won’t prevent writing into a symlinked parent directory (e.g. .specify/templates being a symlink). That can lead to writing outside the project root and can also make manifest.record_existing() raise after the write. Add a pre-write containment check (resolved path within project root + no symlink components in the destination directory chain) and skip with a clear warning when unsafe.

                dst = dest_templates / f.name
                rel_posix = dst.relative_to(project_path).as_posix()
                if dst.is_symlink():
                    symlinked_files.append(rel_posix)
                    continue
                should_write = (
                    not dst.exists()
                    or force
                    or (refresh_managed and _is_managed(rel_posix, dst))
                )

Files reviewed: 2/2 changed files
Comments generated: 1

…d infra Address Copilot follow-up on github#2375: previous symlink guard only checked the leaf path (dst_path.is_symlink()). If an *ancestor* directory like .specify/scripts/ or .specify/templates/ is itself a symlink to a location outside the project root, the leaf check returns False and shutil.copy2 / write_text would escape the project. Add an _is_safe_dest() check that: - resolves the full destination path and confirms it's contained under project_path.resolve(); rejects paths that escape via any link. - walks every existing ancestor between project_path and dst.parent and rejects if any one of them is a symlink (defence in depth — covers the case where resolve() can't fully traverse a not-yet-created leaf but a higher ancestor is symlinked). Apply the check at both copy sites (scripts loop and templates loop). Unsafe paths are added to symlinked_files and reported with the existing "remove or replace the symlink manually" guidance. Add a regression test (Unix-only — Windows symlink creation needs admin) that symlinks .specify/scripts/bash to an outside directory and verifies switch refuses to write through it.

Copilot

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (1)

src/specify_cli/init.py:923

Same manifest-tracking issue as the scripts loop: if a bundled template already exists but isn’t in prior_hashes, the code skips it without recording it into the manifest. That prevents future refresh_managed runs from ever treating it as managed. Consider recording hashes for skipped-but-existing bundled templates (e.g., manifest.record_existing(rel_posix)) so the manifest can rehydrate missing entries.

                if not should_write:
                    if refresh_managed and dst.exists() and rel_posix in prior_hashes:
                        preserved_user_files.append(rel_posix)
                    else:
                        skipped_files.append(str(dst.relative_to(project_path)))

Files reviewed: 2/2 changed files
Comments generated: 4

Copilot · 2026-04-28T14:59:24Z

                if src_path.is_file():
                    rel_path = src_path.relative_to(variant_src)
                    dst_path = dest_variant / rel_path
-                    if dst_path.exists() and not force:
-                        skipped_files.append(str(dst_path.relative_to(project_path)))
+                    rel_posix = dst_path.relative_to(project_path).as_posix()
+                    # Refuse to write through a symlink at the leaf or via a
+                    # symlinked ancestor (e.g. .specify/scripts -> /etc). Both
+                    # cases could let shutil.copy2 escape the project root.
+                    # Symlinks need manual intervention — no flag will resolve
+                    # them — so track them separately.
+                    if dst_path.is_symlink() or not _is_safe_dest(dst_path):
+                        symlinked_files.append(rel_posix)
+                        continue


Even though per-file writes are guarded by _is_safe_dest(), the destination directories (dest_scripts/dest_variant) are created earlier unconditionally. If .specify/scripts is a symlink to a location outside the project root and the variant directory doesn’t exist yet, dest_variant.mkdir(...) can still create directories outside the project before this loop runs. Consider validating the destination root/variant directories with the same safe-path checks before any mkdir, and skipping the whole scripts install when unsafe (without creating anything).

See below for a potential fix:

dest_scripts_rel = Path(".specify") / "scripts" # Validate the scripts root before creating it. A symlinked # `.specify/scripts` could otherwise make mkdir() escape the project. if dest_scripts.is_symlink() or not _is_safe_dest(dest_scripts): symlinked_files.append(dest_scripts_rel.as_posix()) else: dest_scripts.mkdir(parents=True, exist_ok=True) variant_dir = "bash" if script_type == "sh" else "powershell" variant_src = scripts_src / variant_dir if variant_src.is_dir(): dest_variant = dest_scripts / variant_dir dest_variant_rel = dest_scripts_rel / variant_dir # Validate the variant directory before creating it for the # same reason as above. if dest_variant.is_symlink() or not _is_safe_dest(dest_variant): symlinked_files.append(dest_variant_rel.as_posix()) else: dest_variant.mkdir(parents=True, exist_ok=True) for src_path in variant_src.rglob("*"): if src_path.is_file(): rel_path = src_path.relative_to(variant_src) dst_path = dest_variant / rel_path rel_posix = dst_path.relative_to(project_path).as_posix() # Refuse to write through a symlink at the leaf or via a # symlinked ancestor (e.g. .specify/scripts -> /etc). Both # cases could let shutil.copy2 escape the project root. # Symlinks need manual intervention — no flag will resolve # them — so track them separately. if dst_path.is_symlink() or not _is_safe_dest(dst_path): symlinked_files.append(rel_posix) continue should_write = ( not dst_path.exists() or force or (refresh_managed and _is_managed(rel_posix, dst_path)) ) if not should_write: if refresh_managed and dst_path.exists() and rel_posix in prior_hashes: preserved_user_files.append(rel_posix) else: skipped_files.append(str(dst_path.relative_to(project_path))) else: dst_path.parent.mkdir(parents=True, exist_ok=True) shutil.copy2(src_path, dst_path) manifest.record_existing(rel_posix)

Copilot · 2026-04-28T14:59:24Z

        for f in templates_src.iterdir():
            if f.is_file() and f.name != "vscode-settings.json" and not f.name.startswith("."):
                dst = dest_templates / f.name
-                if dst.exists() and not force:
-                    skipped_files.append(str(dst.relative_to(project_path)))
+                rel_posix = dst.relative_to(project_path).as_posix()
+                # Same defence as the scripts loop: refuse symlinks at the
+                # leaf or anywhere in the parent chain.
+                if dst.is_symlink() or not _is_safe_dest(dst):
+                    symlinked_files.append(rel_posix)
+                    continue


Similar to the scripts path: dest_templates.mkdir(...) happens before the per-file _is_safe_dest() check. If .specify/templates (or an ancestor) is a symlink to a path outside the project root and doesn’t exist yet, this mkdir can create directories outside the project even though later file writes are skipped. Consider applying the safe-path/symlink checks before creating dest_templates, and skipping template installation when unsafe.

Copilot · 2026-04-28T14:59:24Z

+                        if refresh_managed and dst_path.exists() and rel_posix in prior_hashes:
+                            preserved_user_files.append(rel_posix)
+                        else:
+                            skipped_files.append(str(dst_path.relative_to(project_path)))


When refresh_managed=True and an existing destination file is skipped because it’s not considered managed (rel_posix not in prior_hashes), it is added to skipped_files but never recorded into the new speckit manifest. That means these paths remain untracked forever (and refresh_managed can never refresh them in future runs). Consider calling manifest.record_existing(rel_posix) for skipped-but-existing bundled paths (at least when refresh_managed=True) so the manifest converges back to tracking all shared infra files.

This issue also appears on line 919 of the same file.

Suggested change

skipped_files.append(str(dst_path.relative_to(project_path)))

skipped_files.append(str(dst_path.relative_to(project_path)))

if refresh_managed and dst_path.exists():

manifest.record_existing(rel_posix)

Copilot · 2026-04-28T14:59:25Z

            "[cyan]specify integration upgrade --force[/cyan]."
        )


The skipped_files remediation text always points to specify init --here --force / specify integration upgrade --force, but callers like integration switch now have a different supported flag (--refresh-shared-infra). Since refresh_hint is already plumbed for the customization warning, consider also using it (or a similar hook) for the skipped_files warning so the guidance matches the command the user just ran (especially when refresh_managed=True and prior_hashes is missing/incomplete).

See below for a potential fix:

if refresh_hint: console.print(refresh_hint) else: console.print( "To refresh shared infrastructure, run " "[cyan]specify init --here --force[/cyan] or " "[cyan]specify integration upgrade --force[/cyan]." )

mnriem · 2026-04-28T15:10:57Z

Please address Copilot feedback. If not applicable, please explain why

…efresh_hint Address three Copilot follow-ups on github#2375: 1. mkdir before safety check: dest_scripts.mkdir() and dest_templates.mkdir() ran before any symlink/safety validation. If `.specify/scripts/` or `.specify/templates/` were already symlinked outside the project, mkdir could resolve through the link and create directories outside project bounds. Wrap mkdir in a `_safe_mkdir()` helper that runs the same `_is_safe_dest()` + `is_symlink()` check before creating; on failure the directory is reported as symlinked and the loop is skipped. 2. Skipped-but-existing managed files lost tracking: when refresh_managed was on and a file already matched the prior hash but was skipped (e.g. no force, customizations branch), the new manifest never re-recorded it. Future runs would no longer recognize it as managed. Re-record on the skipped path so hash tracking is preserved across runs. 3. skipped_files warning ignored refresh_hint: it always suggested `specify init --here --force`, even when the caller (integration switch) exposes a different flag. Use refresh_hint when provided, fall back to the init/upgrade remediation otherwise.

Quratulain-bilal · 2026-04-28T15:17:04Z

Addressed in c7833ec:
▎
▎ 1. mkdir guard — added _safe_mkdir() helper that runs the symlink/safety check before creating any destination
▎ directory. If .specify/scripts/ or .specify/templates/ is already a symlink outside the project, the directory is
▎ reported and the loop is skipped instead of creating through the link.
▎ 2. Skipped-but-existing files — when refresh_managed=True and a file is skipped (already exists, no override flag), we
▎ now record_existing() so the next run can still match its hash and treat it as managed.
▎ 3. skipped_files hint — uses the caller-supplied refresh_hint when provided, falling back to the init/upgrade text
▎ only when no hint was passed. So integration switch now consistently suggests --refresh-shared-infra in both the
▎ customized and skipped warnings.

Copilot

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (1)

src/specify_cli/init.py:949

Same issue as in the scripts loop: when refresh_managed is enabled and an existing template file is preserved due to hash divergence, calling manifest.record_existing(rel_posix) updates the manifest to the customized hash. That makes future refreshes treat the customization as managed and overwrite it. Preserve the prior hash for customized files (already present from prior_hashes) and only record/update hashes for files you overwrite in this run.

                        if refresh_managed and dst.exists() and rel_posix in prior_hashes:
                            preserved_user_files.append(rel_posix)
                        else:
                            skipped_files.append(str(dst.relative_to(project_path)))
                        if refresh_managed and dst.exists():
                            manifest.record_existing(rel_posix)

Files reviewed: 2/2 changed files
Comments generated: 0 new

Quratulain-bilal · 2026-05-02T18:02:39Z

Hi @mnriem friendly check-in on this one. The PR sat green for about a 4 days ago, which is when the upstream
shared_infra.py refactor landed and introduced the conflict you're seeing now. I've just rebased onto the latest main
and threaded the new refresh_managed / refresh_hint params through that module so the behavior is preserved end-to-end.
All 56 integration-subcommand tests pass locally.

This fix addresses #2293 (stale vendored shared scripts breaking the target integration after a switch) happy to add
more tests or split the change if that helps the review. Whenever you have a moment, I'd really appreciate another look.
Thanks so much for your time on this!

Copilot

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 4/4 changed files
Comments generated: 4

+    preserved with a warning. ``force=True`` overwrites everything regardless.
+    ``refresh_hint`` is shown after the customization warning to tell the user
+    which flag would overwrite their customizations.


+        if refresh_hint:
+            console.print(refresh_hint)
+        else:
+            console.print(
+                "To refresh shared infrastructure, run "
+                "[cyan]specify init --here --force[/cyan] or "
+                "[cyan]specify integration upgrade --force[/cyan]."
+            )


+
+

                rel_path = src_path.relative_to(variant_src)
                dst_path = dest_variant / rel_path
                _ensure_safe_shared_destination(project_path, dst_path, parent_must_exist=False)
-                if dst_path.exists() and not force:
-                    skipped_files.append(dst_path.relative_to(project_path).as_posix())
+                rel = dst_path.relative_to(project_path).as_posix()
+                write, bucket = _decide_overwrite(rel, dst_path)


mnriem · 2026-05-04T19:16:04Z

Please address Copilot feedback. If not applicable, please explain why

- 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).

Copilot

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 3/3 changed files
Comments generated: 1

+        """
+        try:
+            _ensure_safe_shared_destination(project_path, dst, parent_must_exist=parent_must_exist)
+        except ValueError:


mnriem · 2026-05-05T16:36:57Z

Please address Copilot feedback. If not applicable, please explain why

Address Copilot feedback on shared_infra.py:305 — _safe_dest_or_bucket caught any ValueError as 'symlinked', which masked genuine safety errors (path escape, parent-not-a-directory). - Introduce SymlinkedSharedPathError(ValueError) raised only by the symlink-specific branches in _ensure_safe_shared_*(). - _safe_dest_or_bucket() now catches only SymlinkedSharedPathError; other ValueErrors propagate so the operation aborts with the real cause instead of being silently bucketed. - Wrap top-level dest_scripts/dest_variant/dest_templates mkdir calls in the same bucket helper so a symlinked .specify/scripts or .specify/templates is preserved with a warning rather than aborting the switch (matches the documented 'preserve customizations' behavior). - Update tests to expect the new bucket+warn behavior for leaf-level symlinked destinations.

Quratulain-bilal · 2026-05-05T21:11:45Z

@mnriem! Addressed Copilot's latest feedback in 93850a7.

The issue was that _safe_dest_or_bucket() caught any ValueError from
_ensure_safe_shared_destination() and bucketed it as "symlinked", which would have masked genuine
safety errors like path escape or parent-not-a-directory.

Fix: introduced SymlinkedSharedPathError(ValueError) raised only by the symlink-specific branches. The
bucket helper now catches just that subclass every other ValueError propagates and aborts the
operation with the real cause. Also wrapped the top-level dest_scripts / dest_variant /
dest_templates mkdir calls in the same bucket helper so a symlinked .specify/scripts or
.specify/templates is preserved with a warning instead of aborting the switch (matches the documented
"preserve customizations" behavior). Updated 4 tests for the new bucket+warn semantics. All 44 relevant
tests pass locally.

Copilot

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 4/4 changed files
Comments generated: 3

+    if symlinked_files:
+        console.print(
+            f"[yellow]⚠[/yellow]  Skipped {len(symlinked_files)} symlinked shared "
+            "infrastructure file(s) — symlinks are never overwritten because they "


        outside.write_text("# outside\n", encoding="utf-8")
        os.symlink(outside, scripts_dir / "z.sh")

-        with pytest.raises(ValueError, match="Refusing to overwrite symlinked"):
-            install_shared_infra(
-                project,
-                "sh",
-                version="test",
-                core_pack=core_pack,
-                repo_root=tmp_path / "unused",
-                console=_NoopConsole(),
-                force=True,
-            )
+        install_shared_infra(
+            project,


@@ -313,5 +411,28 @@ def install_shared_infra(
            "[cyan]specify integration upgrade --force[/cyan]."
        )


mnriem · 2026-05-05T21:49:29Z

Please address Copilot feedback

Address Copilot review on PR github#2375: - skipped_files hint now uses refresh_hint when refresh_managed=True so integration switch suggests --refresh-shared-infra instead of the generic init/upgrade flags. - symlinked-files warning header says "path(s)" rather than "file(s)" since symlinked directories (e.g. .specify/scripts/bash) are also bucketed there. - Rename test_shared_infra_install_preflights_before_writing to test_shared_infra_install_buckets_unsafe_destinations_and_continues to match the new bucket-and-continue semantics.

Quratulain-bilal · 2026-05-07T02:05:09Z

shared_infra.py:412 refresh_managed=True may refresh_hint use hota hai (init/upgrade hint sirf default path may)
shared_infra.py:417 "file(s)" → "path(s)" (directories b bucket may aati hain)
test_cli.py:449 test rename: test_shared_infra_install_buckets_unsafe_destinations_and_continues + new docstring

Copilot

Copilot's findings

Comments suppressed due to low confidence (2)

src/specify_cli/shared_infra.py:35

load_speckit_manifest() calls IntegrationManifest.load() when speckit.manifest.json exists, but IntegrationManifest.load() reads the manifest file without any symlink/containment preflight. If .specify/ or .specify/integrations/ is a symlink, this can end up reading JSON from outside the project root, even though writes are later blocked. Consider validating the manifest directory with the same symlink/escape checks (e.g., _validate_safe_shared_directory(project_path, project_path / '.specify' / 'integrations')) and raising early before attempting to read.

def load_speckit_manifest(
    project_path: Path,
    *,
    version: str,
    console: Any | None = None,
) -> IntegrationManifest:
    """Load the shared infrastructure manifest, preserving existing entries."""
    manifest_path = project_path / ".specify" / "integrations" / "speckit.manifest.json"
    if manifest_path.exists():
        try:
            manifest = IntegrationManifest.load("speckit", project_path)
            manifest.version = version
            return manifest

tests/integrations/test_cli.py:323

This test name/docstring says the installer "refuses" a symlinked template destination, but the implementation now skips/buckets symlinked paths with a warning instead of raising. Please rename the test/docstring to match the new semantics.

    @pytest.mark.skipif(not hasattr(os, "symlink"), reason="symlinks are unavailable")
    def test_shared_infra_refuses_symlinked_template_destination(self, tmp_path, capsys):
        """Shared template installs must not follow destination symlinks."""
        from specify_cli import _install_shared_infra

Files reviewed: 4/4 changed files
Comments generated: 1

    @pytest.mark.skipif(not hasattr(os, "symlink"), reason="symlinks are unavailable")
-    def test_shared_infra_refuses_symlinked_script_destination(self, tmp_path):
+    def test_shared_infra_refuses_symlinked_script_destination(self, tmp_path, capsys):
        """Shared script refreshes must not follow destination symlinks."""
        from specify_cli import _install_shared_infra



mnriem · 2026-05-07T17:10:36Z

Please address Copilot feedback

…ehavior The two file-bucketing tests at line 300/320 were named *_refuses_*, but the new behavior buckets symlinked file destinations with a warning while safe destinations in the same install still complete. Rename to *_buckets_* and update docstrings to match. The remaining *_refuses_* tests (line 342/362/381) genuinely raise on symlinked dirs/manifests and keep their names.

Copilot

Copilot's findings

Comments suppressed due to low confidence (1)

src/specify_cli/shared_infra.py:163

_ensure_safe_shared_destination() validates symlinks/escape but doesn’t reject existing non-regular destinations (e.g., a directory at the file path). In that case the later write path will fail at os.replace() with an OS error, which is harder to diagnose and may leave the operation partially applied. Consider adding an explicit if dest.exists() and not dest.is_file(): raise ValueError("Shared infrastructure destination path is not a file: …") (and possibly similar handling for other non-regular types) to fail fast with a clear message.

    label = _shared_destination_label(project_path, dest)
    if dest.is_symlink():
        raise SymlinkedSharedPathError(f"Refusing to overwrite symlinked shared infrastructure path: {label}")

    if dest.exists():
        try:
            dest.resolve().relative_to(root)
        except (OSError, ValueError):
            raise ValueError(f"Shared infrastructure destination escapes project root: {label}") from None

Files reviewed: 4/4 changed files
Comments generated: 1

+    # Refresh shared infrastructure to the current CLI version. Switching
+    # integrations is exactly when stale vendored shared scripts (e.g.
+    # update-agent-context.sh that pre-dates the target integration's
+    # supported-agent list) would silently break the new integration.
+    #
+    # Use refresh_managed=True so only files that match their previously
+    # recorded hash are overwritten — user customizations are detected via
+    # hash divergence and preserved with a warning. Pass
+    # --refresh-shared-infra to overwrite customizations as well. See #2293.
    _install_shared_infra_or_exit(
        project_root,
        selected_script,
+        force=refresh_shared_infra,
+        refresh_managed=True,
        invoke_separator=_invoke_separator_for_integration(
            target_integration, current, target, parsed_options
        ),
+        refresh_hint=(
+            "To overwrite customizations, re-run with "
+            "[cyan]specify integration switch ... --refresh-shared-infra[/cyan]."
+        ),
    )


Quratulain-bilal · 2026-05-07T19:51:51Z

Renamed test_shared_infra_refuses_symlinked_script_destination (line 300) and test_shared_infra_refuses_symlinked_template_destination (line 320) to test_shared_infra_buckets_symlinked_script_destination and test_shared_infra_buckets_symlinked_template_destination, with docstrings updated to reflect the bucket-and-continue behavior (symlinked destinations are bucketed with a warning while safe destinations in the same install still complete). The remaining *_refuses_* tests at lines 342/362/381 do still raise (symlinked .specify dir / shared manifest / template-only refresh) and were intentionally left unchanged. Pushed in 575d6ad.

Quratulain-bilal requested a review from mnriem as a code owner April 27, 2026 02:35

faizan-shah-10 approved these changes Apr 27, 2026

View reviewed changes

mnriem requested a review from Copilot April 27, 2026 13:55

Copilot started reviewing on behalf of mnriem April 27, 2026 13:56 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

Comment thread src/specify_cli/__init__.py Outdated

Comment thread src/specify_cli/__init__.py Outdated

mnriem requested a review from Copilot April 27, 2026 14:45

Copilot started reviewing on behalf of mnriem April 27, 2026 14:45 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

Comment thread src/specify_cli/__init__.py Outdated

Comment thread src/specify_cli/__init__.py Outdated

Comment thread src/specify_cli/__init__.py

mnriem requested changes Apr 27, 2026

View reviewed changes

mnriem requested a review from Copilot April 27, 2026 17:33

Copilot started reviewing on behalf of mnriem April 27, 2026 17:33 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

Comment thread src/specify_cli/__init__.py Outdated

Comment thread src/specify_cli/__init__.py Outdated

Comment thread src/specify_cli/__init__.py

mnriem requested a review from Copilot April 28, 2026 14:02

Copilot started reviewing on behalf of mnriem April 28, 2026 14:04 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

Comment thread src/specify_cli/__init__.py Outdated

mnriem requested a review from Copilot April 28, 2026 14:42

Copilot started reviewing on behalf of mnriem April 28, 2026 14:54 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

mnriem requested a review from Copilot April 28, 2026 15:30

Copilot AI reviewed Apr 28, 2026

View reviewed changes

fix(integration): refresh shared infra on integration switch

005f24c

Quratulain-bilal force-pushed the fix/switch-refresh-shared-infra branch from c7833ec to 005f24c Compare May 2, 2026 17:53

mnriem requested a review from Copilot May 4, 2026 12:47

Copilot started reviewing on behalf of mnriem May 4, 2026 12:48 View session

Copilot AI reviewed May 4, 2026

View reviewed changes

Quratulain-bilal force-pushed the fix/switch-refresh-shared-infra branch from ae4d373 to ee9afea Compare May 5, 2026 02:28

mnriem requested a review from Copilot May 5, 2026 14:34

Copilot started reviewing on behalf of mnriem May 5, 2026 14:35 View session

Copilot AI reviewed May 5, 2026

View reviewed changes

Comment thread src/specify_cli/shared_infra.py Outdated

"""

try:

_ensure_safe_shared_destination(project_path, dst, parent_must_exist=parent_must_exist)

except ValueError:

mnriem requested a review from Copilot May 5, 2026 21:17

Copilot started reviewing on behalf of mnriem May 5, 2026 21:18 View session

Copilot AI reviewed May 5, 2026

View reviewed changes

mnriem requested a review from Copilot May 7, 2026 15:10

Copilot AI reviewed May 7, 2026

View reviewed changes

mnriem requested a review from Copilot May 7, 2026 18:37

Copilot started reviewing on behalf of mnriem May 7, 2026 18:38 View session

Copilot AI reviewed May 7, 2026

View reviewed changes

		@@ -313,5 +411,28 @@ def install_shared_infra(
		"[cyan]specify integration upgrade --force[/cyan]."
		)

Conversation

Quratulain-bilal commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

mnriem commented Apr 27, 2026

Uh oh!

Quratulain-bilal commented Apr 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mnriem left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mnriem commented Apr 28, 2026

Uh oh!

Quratulain-bilal commented Apr 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

mnriem commented Apr 28, 2026

Uh oh!

Quratulain-bilal commented Apr 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Quratulain-bilal commented May 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

mnriem commented May 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

mnriem commented May 5, 2026

Uh oh!

Quratulain-bilal commented Apr 27, 2026 •

edited

Loading

Quratulain-bilal commented May 2, 2026 •

edited

Loading

mnriem commented May 7, 2026 •

edited

Loading