Skip to content

Commit 3b7d95a

Browse files
marcelsafinCopilot
andauthored
fix: rewrite extension-relative subdir paths in generated command bodies (#3444)
* fix: rewrite extension-relative subdir paths in generated command bodies Extension command bodies reference bundled files relative to the extension root (agents/, knowledge-base/, templates/, ...). Generated SKILL.md and command files emitted those paths verbatim, so agents resolved them against the workspace root where they do not exist. Add CommandRegistrar.rewrite_extension_paths, which rewrites references to subdirectories that actually exist in the installed extension to .specify/extensions/<id>/..., and call it once in register_commands so every output format and alias gets the fix. commands/, specs/ and dot-directories are never rewritten. Fixes #2101 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: only rewrite relative extension path references Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: use callable re.sub replacement for extension subdir rewrite subdir and extension_id come from filesystem directory names and were interpolated into a re.sub string replacement template. A directory name containing a backslash (e.g. assets\q) would raise re.error: bad escape, aborting command registration even when the body didn't reference it. Use a callable replacement so these values are treated literally. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: make subdir rewrite regression test cross-platform Renamed the test's subdir fixture from "assets\\q" to "assets[q]": on Windows, backslash is a path separator, so mkdir would create nested "assets/q" dirs instead of one literally-named directory, and iterdir() would only discover "assets", never exercising the rewrite. extension_id keeps a real backslash/"\\1" since it isn't used to create a directory, still verifying the callable replacement handles it literally. Added a sanity assertion for this assumption. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: apply extension subdir path rewrite in skills-mode renderer register_commands() rewrote extension-relative subdir references (agents/, knowledge-base/, etc.) via rewrite_extension_paths(), but _register_extension_skills() - the separate renderer used for active non-native skills agents (e.g. Claude with ai_skills: true) - never called it. Generated SKILL.md files left agents/... and knowledge-base/... unresolved, and mapped the extension's own templates/ through the generic project-level rewrite instead of its installed .specify/extensions/<id>/templates/ location. Reuse the existing rewrite_extension_paths() helper in _register_extension_skills() at the same point register_commands() applies it (before resolve_skill_placeholders' generic rewrite), and add a skills-mode regression test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: apply extension subdir path rewrite on preset restore/reconcile paths _unregister_skills() restored extension-backed SKILL.md content via resolve_skill_placeholders() without first calling rewrite_extension_paths(), so removing a preset override that shadowed an extension command restored the bare, unresolvable agents/... and knowledge-base/... references. Carried extension_id/extension_dir through _build_extension_skill_restore_index() and applied the same rewrite used at initial registration before restoring. Found the identical gap in _reconcile_composed_commands()'s non-skill agent path: when a removed preset's command reverts to an extension winner, register_commands_for_non_skill_agents() was called without extension_id, so the rewrite never ran for plain command-file agents either. Passed extension_id through there too. Added regression tests for both restore paths (skills-mode and non-skill-agent command files) in tests/test_presets.py. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: apply extension subdir path rewrite when composing over extension base PresetResolver.resolve_content() read the effective base layer's raw content directly via path.read_text() before composing append/prepend/ wrap overlays on top of it, and its outright-replace shortcut did the same. When that base layer was extension-provided, neither read path applied rewrite_extension_paths(), so composing a preset over an extension command (or an extension winning outright through resolve_content) left bare, unresolvable agents/... and knowledge-base/... references in the composed output. All three call sites (PresetManager._register_commands()'s composed path, _reconcile_composed_commands()'s composed path, and skills-mode reading the .composed file written by either) consume resolve_content's return value, so fixing the read at its source covers command output, skill output, and both initial-install and reconcile flows without threading extension identity through each caller. Tagged extension layers in collect_all_layers() with extension_id/ extension_dir, and added a _read_layer_content() helper in resolve_content() that applies rewrite_extension_paths() whenever a layer carries that extension identity — used at both raw-read sites (outright-replace shortcut and composition base). Composing (append/prepend/wrap) layers are never extension-provided (extensions are always inserted with strategy "replace"), so no other read site needs the rewrite. Added regression tests: a parametrized resolve_content() test covering append/prepend/wrap composing over an extension base, a skills-mode test asserting the composed SKILL.md resolves the extension's subdir references, and a non-skill-agent (Gemini) install-time test matching the reported live repro. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 086929e commit 3b7d95a

6 files changed

Lines changed: 665 additions & 2 deletions

File tree

src/specify_cli/agents.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,52 @@ def rewrite_project_relative_paths(
213213
".specify.specify/", ".specify/"
214214
)
215215

216+
@staticmethod
217+
def rewrite_extension_paths(
218+
text: str, extension_id: str, extension_dir: Path
219+
) -> str:
220+
"""Rewrite extension-relative paths to their installed locations.
221+
222+
Extension command bodies reference bundled files relative to the
223+
extension root (e.g. ``agents/control/commander.md``). After install
224+
those files live under ``.specify/extensions/<id>/``, so bare
225+
references would resolve against the workspace root and never be
226+
found (#2101).
227+
228+
Only directories that actually exist inside *extension_dir* are
229+
rewritten, keeping the behaviour conservative and avoiding false
230+
positives on prose. ``commands`` (slash-command sources), ``specs``
231+
(user project artifacts) and dot-directories are never rewritten.
232+
"""
233+
if not isinstance(text, str) or not text:
234+
return text
235+
236+
skip = {"commands", ".git", "specs"}
237+
try:
238+
subdirs = [
239+
entry.name
240+
for entry in extension_dir.iterdir()
241+
if entry.is_dir()
242+
and entry.name not in skip
243+
and not entry.name.startswith(".")
244+
]
245+
except OSError:
246+
return text
247+
248+
for subdir in subdirs:
249+
# Only rewrite relative references (subdir/... or ./subdir/...);
250+
# absolute paths like /subdir/... keep their meaning. Use a
251+
# callable replacement: subdir/extension_id come from the
252+
# filesystem and could contain backslashes or "\1"-like
253+
# sequences, which would corrupt a string replacement template.
254+
replacement = f".specify/extensions/{extension_id}/{subdir}/"
255+
text = re.sub(
256+
r'(^|[\s`"\'(])(?:\./)?' + re.escape(subdir) + "/",
257+
lambda m: m.group(1) + replacement,
258+
text,
259+
)
260+
return text
261+
216262
def render_markdown_command(
217263
self, frontmatter: dict, body: str, source_id: str, context_note: str = None
218264
) -> str:
@@ -639,6 +685,9 @@ def register_commands(
639685
frontmatter[key] = core_frontmatter[key]
640686
frontmatter.pop("strategy", None)
641687

688+
if extension_id:
689+
body = self.rewrite_extension_paths(body, extension_id, source_root)
690+
642691
frontmatter = self._adjust_script_paths(
643692
frontmatter, extension_id=extension_id
644693
)

src/specify_cli/extensions/__init__.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1078,6 +1078,11 @@ def _register_extension_skills(
10781078
frontmatter = registrar._adjust_script_paths(
10791079
frontmatter, extension_id=manifest.id
10801080
)
1081+
# Mirror the register_commands() rewrite (#2101): resolve
1082+
# extension-relative subdir references (agents/, knowledge-base/,
1083+
# etc.) to their installed .specify/extensions/<id>/ location
1084+
# before the generic placeholder/path resolution below.
1085+
body = registrar.rewrite_extension_paths(body, manifest.id, extension_dir)
10811086
body = registrar.resolve_skill_placeholders(
10821087
selected_ai, frontmatter, body, self.project_root, extension_id=manifest.id
10831088
)

src/specify_cli/presets/__init__.py

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -778,6 +778,7 @@ def _reconcile_composed_commands(self, command_names: List[str]) -> None:
778778
matching_cmds, ext_id, ext_dir,
779779
self.project_root,
780780
context_note=f"\n<!-- Extension: {ext_id} -->\n<!-- Config: .specify/extensions/{ext_id}/ -->\n",
781+
extension_id=ext_id,
781782
)
782783
registered = True
783784
except Exception:
@@ -1199,6 +1200,8 @@ def _build_extension_skill_restore_index(self) -> Dict[str, Dict[str, Any]]:
11991200
"command_name": cmd_name,
12001201
"source_file": source_file,
12011202
"source": f"extension:{manifest.id}",
1203+
"extension_id": manifest.id,
1204+
"extension_dir": ext_root,
12021205
}
12031206
modern_skill_name, legacy_skill_name = self._skill_names_for_command(cmd_name)
12041207
restore_index.setdefault(modern_skill_name, restore_info)
@@ -1463,6 +1466,17 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None:
14631466
if extension_restore:
14641467
content = extension_restore["source_file"].read_text(encoding="utf-8")
14651468
frontmatter, body = registrar.parse_frontmatter(content)
1469+
# Mirror the register-time rewrite (#2101): resolve
1470+
# extension-relative subdir references (agents/,
1471+
# knowledge-base/, etc.) to their installed location before
1472+
# the generic placeholder resolution below, otherwise
1473+
# restoring after a preset override removal would leave
1474+
# bare, unresolvable paths in the skill body.
1475+
body = registrar.rewrite_extension_paths(
1476+
body,
1477+
extension_restore["extension_id"],
1478+
extension_restore["extension_dir"],
1479+
)
14661480
if isinstance(selected_ai, str):
14671481
body = registrar.resolve_skill_placeholders(
14681482
selected_ai, frontmatter, body, self.project_root
@@ -3038,6 +3052,8 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]:
30383052
"path": candidate,
30393053
"source": source,
30403054
"strategy": "replace",
3055+
"extension_id": ext_id,
3056+
"extension_dir": ext_dir,
30413057
})
30423058

30433059
# Priority 4: Core templates (always "replace")
@@ -3157,10 +3173,32 @@ def resolve_content(
31573173
if not layers:
31583174
return None
31593175

3176+
def _read_layer_content(layer: Dict[str, Any]) -> str:
3177+
"""Read a layer's raw text, rewriting extension-relative subdir
3178+
references (agents/, knowledge-base/, etc.) to their installed
3179+
location when the layer is extension-provided (#2101).
3180+
3181+
Extension layers are always inserted with strategy "replace"
3182+
(see collect_all_layers), so a layer only ever needs this
3183+
rewrite when it wins outright above or serves as the
3184+
composition base below — never as a mid-stack composing
3185+
(append/prepend/wrap) layer.
3186+
"""
3187+
text = layer["path"].read_text(encoding="utf-8")
3188+
extension_id = layer.get("extension_id")
3189+
extension_dir = layer.get("extension_dir")
3190+
if extension_id and extension_dir:
3191+
from ..agents import CommandRegistrar
3192+
3193+
text = CommandRegistrar.rewrite_extension_paths(
3194+
text, extension_id, extension_dir
3195+
)
3196+
return text
3197+
31603198
# If the top (highest-priority) layer is replace, it wins entirely —
31613199
# lower layers are irrelevant regardless of their strategies.
31623200
if layers[0]["strategy"] == "replace":
3163-
return layers[0]["path"].read_text(encoding="utf-8")
3201+
return _read_layer_content(layers[0])
31643202

31653203
# Composition: build content bottom-up from the effective base.
31663204
# The base is the nearest replace layer scanning from highest priority
@@ -3183,7 +3221,7 @@ def resolve_content(
31833221

31843222
# Convert to reversed_layers index
31853223
base_reversed_idx = len(layers) - 1 - base_layer_idx
3186-
content = layers[base_layer_idx]["path"].read_text(encoding="utf-8")
3224+
content = _read_layer_content(layers[base_layer_idx])
31873225
# Compose only the layers above the base (higher priority = lower index in layers,
31883226
# higher index in reversed_layers). Process bottom-up from base+1.
31893227
start_idx = base_reversed_idx + 1

tests/test_extension_skills.py

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -911,6 +911,65 @@ def test_skill_registration_uses_extension_local_script_paths(self, project_dir,
911911
assert ".specify/scripts/bash/resolve-skill.sh" not in content
912912
assert ".specify/scripts/bash/ensure-skills.sh" not in content
913913

914+
def test_skill_registration_rewrites_extension_subdir_paths(self, project_dir, temp_dir):
915+
"""Auto-registered skills should resolve extension-relative subdir
916+
references (agents/, knowledge-base/) to their installed location,
917+
matching the rewrite already applied by register_commands() (#2101)."""
918+
_create_init_options(project_dir, ai="claude", ai_skills=True)
919+
skills_dir = _create_skills_dir(project_dir, ai="claude")
920+
921+
ext_dir = temp_dir / "path-ext"
922+
ext_dir.mkdir()
923+
manifest_data = {
924+
"schema_version": "1.0",
925+
"extension": {
926+
"id": "path-ext",
927+
"name": "Path Extension",
928+
"version": "1.0.0",
929+
"description": "Test",
930+
},
931+
"requires": {"speckit_version": ">=0.1.0"},
932+
"provides": {
933+
"commands": [
934+
{
935+
"name": "speckit.path-ext.run",
936+
"file": "commands/run.md",
937+
"description": "Run command",
938+
}
939+
]
940+
},
941+
}
942+
with open(ext_dir / "extension.yml", "w") as f:
943+
yaml.safe_dump(manifest_data, f)
944+
945+
(ext_dir / "commands").mkdir()
946+
(ext_dir / "agents" / "control").mkdir(parents=True)
947+
(ext_dir / "agents" / "control" / "commander.md").write_text("# Commander\n")
948+
(ext_dir / "knowledge-base").mkdir()
949+
(ext_dir / "knowledge-base" / "agent-scores.yaml").write_text("scores: {}\n")
950+
(ext_dir / "templates").mkdir()
951+
(ext_dir / "templates" / "kill-report.md").write_text("# Kill Report\n")
952+
953+
(ext_dir / "commands" / "run.md").write_text(
954+
"---\n"
955+
"description: Run command\n"
956+
"---\n\n"
957+
"Read agents/control/commander.md and knowledge-base/agent-scores.yaml.\n"
958+
"Use templates/kill-report.md as the report template.\n"
959+
)
960+
961+
manager = ExtensionManager(project_dir)
962+
manager.install_from_directory(ext_dir, "0.1.0", register_commands=False)
963+
964+
content = (skills_dir / "speckit-path-ext-run" / "SKILL.md").read_text()
965+
assert ".specify/extensions/path-ext/agents/control/commander.md" in content
966+
assert ".specify/extensions/path-ext/knowledge-base/agent-scores.yaml" in content
967+
# extension's own templates/ dir must resolve under the extension,
968+
# not the project-level .specify/templates/
969+
assert ".specify/extensions/path-ext/templates/kill-report.md" in content
970+
assert "Read agents/control" not in content
971+
assert "and knowledge-base/" not in content
972+
914973
def test_missing_command_file_skipped(self, skills_project, temp_dir):
915974
"""Commands with missing source files should be skipped gracefully."""
916975
project_dir, skills_dir = skills_project

0 commit comments

Comments
 (0)