Skip to content

Commit 380aca5

Browse files
committed
fix: address review — containment check, deterministic prompts, manifest accuracy
- CopilotIntegration.setup() adds dest containment check (relative_to) - Companion prompts generated from templates list, not directory glob - _install_shared_infra() only records files actually copied (not pre-existing) - VS Code settings tests made unconditional (assert template exists) - Inventory tests use .as_posix() for cross-platform paths
1 parent 93e371e commit 380aca5

3 files changed

Lines changed: 26 additions & 21 deletions

File tree

src/specify_cli/__init__.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1241,10 +1241,8 @@ def _install_shared_infra(
12411241
else:
12421242
dst_path.parent.mkdir(parents=True, exist_ok=True)
12431243
shutil.copy2(src_path, dst_path)
1244-
for f in dest_variant.rglob("*"):
1245-
if f.is_file():
1246-
rel = f.relative_to(project_path).as_posix()
1247-
manifest.record_existing(rel)
1244+
rel = dst_path.relative_to(project_path).as_posix()
1245+
manifest.record_existing(rel)
12481246

12491247
# Page templates (not command templates, not vscode-settings.json)
12501248
if core and (core / "templates").is_dir():
@@ -1263,8 +1261,8 @@ def _install_shared_infra(
12631261
skipped_files.append(str(dst.relative_to(project_path)))
12641262
else:
12651263
shutil.copy2(f, dst)
1266-
rel = dst.relative_to(project_path).as_posix()
1267-
manifest.record_existing(rel)
1264+
rel = dst.relative_to(project_path).as_posix()
1265+
manifest.record_existing(rel)
12681266

12691267
if skipped_files:
12701268
import logging

src/specify_cli/integrations/copilot/__init__.py

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,14 @@ def setup(
6666
return []
6767

6868
dest = self.commands_dest(project_root)
69+
dest_resolved = dest.resolve()
70+
try:
71+
dest_resolved.relative_to(project_root_resolved)
72+
except ValueError as exc:
73+
raise ValueError(
74+
f"Integration destination {dest_resolved} escapes "
75+
f"project root {project_root_resolved}"
76+
) from exc
6977
dest.mkdir(parents=True, exist_ok=True)
7078
created: list[Path] = []
7179

@@ -82,10 +90,10 @@ def setup(
8290
)
8391
created.append(dst_file)
8492

85-
# 2. Generate companion .prompt.md files
93+
# 2. Generate companion .prompt.md files from the templates we just wrote
8694
prompts_dir = project_root / ".github" / "prompts"
87-
for agent_file in sorted(dest.glob("speckit.*.agent.md")):
88-
cmd_name = agent_file.name.removesuffix(".agent.md")
95+
for src_file in templates:
96+
cmd_name = f"speckit.{src_file.stem}"
8997
prompt_content = f"---\nagent: {cmd_name}\n---\n"
9098
prompt_file = self.write_file_and_record(
9199
prompt_content,

tests/integrations/test_copilot.py

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,13 @@ def test_agent_and_prompt_counts_match(self, tmp_path):
5757
def test_setup_creates_vscode_settings_new(self, tmp_path):
5858
from specify_cli.integrations.copilot import CopilotIntegration
5959
copilot = CopilotIntegration()
60+
assert copilot._vscode_settings_path() is not None
6061
m = IntegrationManifest("copilot", tmp_path)
6162
created = copilot.setup(tmp_path, m)
6263
settings = tmp_path / ".vscode" / "settings.json"
63-
if copilot._vscode_settings_path():
64-
assert settings.exists()
65-
assert settings in created
66-
assert any("settings.json" in k for k in m.files)
64+
assert settings.exists()
65+
assert settings in created
66+
assert any("settings.json" in k for k in m.files)
6767

6868
def test_setup_merges_existing_vscode_settings(self, tmp_path):
6969
from specify_cli.integrations.copilot import CopilotIntegration
@@ -75,12 +75,11 @@ def test_setup_merges_existing_vscode_settings(self, tmp_path):
7575
m = IntegrationManifest("copilot", tmp_path)
7676
created = copilot.setup(tmp_path, m)
7777
settings = tmp_path / ".vscode" / "settings.json"
78-
if copilot._vscode_settings_path():
79-
data = json.loads(settings.read_text(encoding="utf-8"))
80-
assert data["editor.fontSize"] == 14
81-
assert data["custom.setting"] is True
82-
assert settings not in created
83-
assert not any("settings.json" in k for k in m.files)
78+
data = json.loads(settings.read_text(encoding="utf-8"))
79+
assert data["editor.fontSize"] == 14
80+
assert data["custom.setting"] is True
81+
assert settings not in created
82+
assert not any("settings.json" in k for k in m.files)
8483

8584
def test_all_created_files_tracked_in_manifest(self, tmp_path):
8685
from specify_cli.integrations.copilot import CopilotIntegration
@@ -161,7 +160,7 @@ def test_complete_file_inventory_sh(self, tmp_path):
161160
finally:
162161
os.chdir(old_cwd)
163162
assert result.exit_code == 0
164-
actual = sorted(str(p.relative_to(project)) for p in project.rglob("*") if p.is_file())
163+
actual = sorted(p.relative_to(project).as_posix() for p in project.rglob("*") if p.is_file())
165164
expected = sorted([
166165
".github/agents/speckit.analyze.agent.md",
167166
".github/agents/speckit.checklist.agent.md",
@@ -221,7 +220,7 @@ def test_complete_file_inventory_ps(self, tmp_path):
221220
finally:
222221
os.chdir(old_cwd)
223222
assert result.exit_code == 0
224-
actual = sorted(str(p.relative_to(project)) for p in project.rglob("*") if p.is_file())
223+
actual = sorted(p.relative_to(project).as_posix() for p in project.rglob("*") if p.is_file())
225224
expected = sorted([
226225
".github/agents/speckit.analyze.agent.md",
227226
".github/agents/speckit.checklist.agent.md",

0 commit comments

Comments
 (0)