[bug-fix] Fix extension-skill-path-rewrite: rewrite extension refs for skills by github-actions[bot] · Pull Request #5364 · github/spec-kit

github-actions · 2026-11-10T15:36:50Z

Bug fix — extension-skill-path-rewrite

Proposed fix for issue #5363, applying the remediation from the bug assessment.

Verdict: valid · Severity: medium

Summary

Updated extension path rewriting so skill content rewrites only to existing extension paths, supports nested shorthand references via unique suffix resolution, and emits skill-directory-relative paths for generated SKILL.md files.

Changes

File	Change	Notes
`src/specify_cli/agents.py`	modified	Reworked `rewrite_extension_paths` to resolve existing paths (including nested suffix matches) and compute relative rewrites for skills.
`src/specify_cli/extensions/__init__.py`	modified	Passed `project_root`/`skill_dir` into rewrite call during extension skill generation.
`src/specify_cli/presets/_manager_skills.py`	modified	Passed `project_root`/`skill_dir` into rewrite call during extension-skill restore flow.
`tests/test_extension_skills.py`	modified	Updated existing assertions and added regressions for nested path rewriting and Copilot skill-relative path output.

Tests Added or Updated

tests/test_extension_skills.py::TestExtensionSkillRegistration::test_skill_registration_rewrites_extension_subdir_paths — now asserts skill-dir-relative installed paths.
tests/test_extension_skills.py::TestExtensionSkillRegistration::test_skill_registration_rewrites_nested_extension_refs — pins nested shorthand resolution.
tests/test_extension_skills.py::TestExtensionSkillRegistration::test_copilot_skill_registration_rewrites_relative_to_skill_dir — pins Copilot skill-relative rewrite behavior.

Local Verification

Commands run: python3 -m pytest -q tests/test_extension_skills.py -k "scripted_path_placeholders or rewrites_extension_subdir_paths or rewrites_nested_extension_refs or copilot_skill_registration_rewrites_relative_to_skill_dir" → failed (No module named pytest in this runner).
Commands run: uv run pytest -q tests/test_extension_skills.py -k "scripted_path_placeholders or rewrites_extension_subdir_paths or rewrites_nested_extension_refs or copilot_skill_registration_rewrites_relative_to_skill_dir" → blocked in this runner (permission-gated interactive approval unavailable).
Commands run: python3 -m py_compile src/specify_cli/agents.py src/specify_cli/extensions/__init__.py src/specify_cli/presets/_manager_skills.py tests/test_extension_skills.py → passed.

Deviations from Assessment

None.

Risks & Review Notes

The nested shorthand resolver intentionally rewrites only when a suffix maps to a single extension path; ambiguous shorthand remains unchanged.
Path rewrite behavior for skill outputs now depends on skill directory depth by design; reviewers should confirm expected behavior across integrations that consume skill files differently.

Refs #5363 · cc @markuswondrak

Fixes [Bug]: Extension skill references are rewritten to project-root-relative paths and miss nested directories #5363

Generated by 🛠️ Fix Bug from Labeled Issue for #5363 · copilot · gpt53codex · 53.4 AIC · ⌖ 0.745 AIC · ⊞ 26.5K · ◷

…L dir Apply the remediation from the bug assessment on issue #5363. Refs #5363 Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

🟡 Changes recommended

Path rewriting still mishandles trailing punctuation and expanded script paths, leaving generated references broken and preventing new rewrite tests from passing.

7 open issues

Update fixtures for existence-checked rewrites · New
Preserve sentence-ending punctuation in nested references · New
Rewrite extension paths after placeholder expansion · New
Rewrite paths during native-skill registration · New
Bound memory used by suffix indexing · New
Support directory and portable filename references · New
Test ambiguous suffixes remain unchanged · New

What changed in this PR

This PR addresses #5363 by updating extension reference rewriting so generated agent skills can locate files stored in the installed extension directory.

Changes:

Restricts rewrites to existing paths and adds unique-suffix resolution for nested references.
Passes project and skill-directory context during skill generation and restoration.
Updates path assertions and adds nested-reference and Copilot regression tests.

File	Description
`tests/test_extension_skills.py`	Updates relative-path expectations and adds nested-reference and Copilot coverage.
`src/specify_cli/presets/_manager_skills.py`	Supplies destination context when restoring extension skills.
`src/specify_cli/extensions/__init__.py`	Supplies destination context when generating extension skills.
`src/specify_cli/agents.py`	Adds existence checks, suffix lookup, and skill-relative path calculation.

_{Review effort: Balanced (auto)}

Address path punctuation, existing-target fixtures, native skill and alias rendering, script expansion, bounded suffix lookup, and ambiguous references. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

🟡 Changes recommended

Unresolved path-handling defects break fragment links, cross-drive Windows registration, and project-root script invocations, while reference resolution can consume excessive CPU.

6 open issues

5 resolved since last review

_{Review effort: Balanced (auto)}

Preserve link fragments and literal hash filenames, avoid reference-entry cross products, keep bundled scripts project-root-relative, and use absolute resource paths across Windows drives. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

🔵 Needs a closer look

The tokenizer mishandles bundled paths containing spaces or parentheses and exhibits quadratic behavior on malformed extension text, causing broken resource references and potentially CPU-bound installations.

2 open issues

2 previously missed issues

In code that hasn't changed since last review

Quoted paths with spaces or parentheses are not resolved

src/specify_cli/agents.py:270

The new tokenizer stops at spaces and parentheses even inside quoted or backtick-delimited paths, leaving existing bundled files such as agents/reviewer guide.md and agents/reviewer(v2).md unresolved. This regresses the previous prefix-based rewrite, so generated skills look for these resources beside SKILL.md instead of in the installed extension. Recognize complete delimited paths before checking existence, and add regressions for both filename forms.

Malformed extension text can trigger quadratic token scanning

src/specify_cli/agents.py:270

The token regex permits repeated rescanning and backtracking, giving it quadratic worst-case behavior on malformed extension command text. Processing downloaded extension bodies can therefore leave installation CPU-bound. Replace it with bounded token scanning that preserves supported filename and Markdown syntax, and add coverage that guards against excessive rescanning.

4 resolved since last review

_{Review effort: Balanced (auto)}

Handle delimited filenames and balanced Markdown paths with forward-only scanning. Bound punctuation resolution and compare execution directories by filesystem identity so Windows short paths do not fail CI. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

🟡 Changes recommended

The scanner can exhaust memory by retaining per-occurrence path data, and inline-command path references are no longer rewritten correctly.

4 open issues

_{Review effort: Balanced (auto)}

Scan unique tokens without retaining occurrences, stream replacements into an output buffer, and prefer complete delimited filenames before resolving embedded command arguments. Make the regression subprocess script concatenation explicit for CodeQL. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

🟡 Changes recommended

Generated commands mix project-root-relative executable paths with skill-relative operands, causing incorrect file resolution and potentially selecting unintended files outside the project.

1 open issue

Resolve command operands relative to the correct working directory · New

4 resolved since last review

_{Review effort: Balanced (auto)}

Keep cross-drive resource references project-root-relative and add explicit resolution guidance once. Cover generated Hermes skills, repeat rendering, missing references, and absence of local filesystem paths. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Use one project-root base for recognized inline-command executables and operands, retain skill-relative standalone resource links, and emit explicit command working-directory guidance. Exercise generated commands that read bundled files through scripts and tools. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

🔵 Needs a closer look

Resource paths in shell commands are rewritten relative to the skill directory despite an unchanged working directory, causing previously valid project-root commands to fail with FileNotFoundError.

No open issues 🎉

1 resolved since last review

Resolve command operands relative to the correct working directory

_{Review effort: Balanced (auto)}

Copilot

🔵 Needs a closer look

Human assessment is needed to evaluate memory-exhaustion risk from unbounded output expansion and correctness risks in command, Markdown, and URL path rewriting.

No open issues 🎉

_{Review effort: Balanced (auto)}

Fix extension-skill-path-rewrite: rewrite skill refs relative to SKIL…

d101b7a

…L dir Apply the remediation from the bug assessment on issue #5363. Refs #5363 Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions Bot added automated bug-fix Trigger the bug-fix agentic workflow labels Nov 10, 2026

Copilot AI auto review requested due to automatic review settings November 10, 2026 15:36

github-actions Bot added automated bug-fix Trigger the bug-fix agentic workflow labels Nov 10, 2026

github-actions Bot mentioned this pull request Nov 10, 2026

[Bug]: Extension skill references are rewritten to project-root-relative paths and miss nested directories #5363

Closed

Copilot started reviewing November 10, 2026 15:37

View session

Copilot AI reviewed Nov 10, 2026

View reviewed changes

Copilot AI auto review requested due to automatic review settings November 10, 2026 16:00

Copilot started reviewing November 10, 2026 16:01

View session

Copilot AI reviewed Nov 10, 2026

View reviewed changes

Comment thread src/specify_cli/agents.py Resolved

Comment thread src/specify_cli/agents.py Resolved

Comment thread src/specify_cli/agents.py Resolved

Comment thread src/specify_cli/agents.py Resolved

mnriem marked this pull request as ready for review November 10, 2026 16:14

mnriem self-requested a review as a code owner November 10, 2026 16:14

Copilot AI auto review requested due to automatic review settings November 10, 2026 16:19

Copilot started reviewing November 10, 2026 16:20

View session

github-advanced-security AI found potential problems Nov 10, 2026

View reviewed changes

Comment thread src/specify_cli/agents.py Resolved

Copilot AI reviewed Nov 10, 2026

View reviewed changes

mnriem added the triage-must-have Verdict: high-value, important work for Spec Kit — do first label Nov 10, 2026

Copilot AI auto review requested due to automatic review settings November 10, 2026 17:32

Copilot started reviewing November 10, 2026 17:33

View session

github-code-quality Bot found potential problems Nov 10, 2026

View reviewed changes

Comment thread tests/test_extensions.py Fixed

Copilot AI reviewed Nov 10, 2026

View reviewed changes

Comment thread src/specify_cli/agents.py Resolved

Comment thread src/specify_cli/agents.py Resolved

Copilot AI auto review requested due to automatic review settings November 10, 2026 17:51

Copilot started reviewing November 10, 2026 17:52

View session

github-advanced-security AI found potential problems Nov 10, 2026

View reviewed changes

Comment thread src/specify_cli/agents.py Resolved

Copilot AI reviewed Nov 10, 2026

View reviewed changes

Comment thread src/specify_cli/agents.py Resolved

Copilot AI auto review requested due to automatic review settings November 10, 2026 18:00

Copilot started reviewing November 10, 2026 18:00

View session

Copilot AI reviewed Nov 10, 2026

View reviewed changes

Copilot AI auto review requested due to automatic review settings November 10, 2026 18:06

Copilot started reviewing November 10, 2026 18:08

View session

Copilot AI reviewed Nov 10, 2026

View reviewed changes

markuswondrak mentioned this pull request Nov 10, 2026

[Feature]: Add extension.yml-controlled skill resource copying with legacy no-copy support #5366

Open

9 tasks

mnriem closed this Nov 10, 2026

Conversation

github-actions Bot commented Nov 10, 2026

Bug fix — extension-skill-path-rewrite

Summary

Changes

Tests Added or Updated

Local Verification

Deviations from Assessment

Risks & Review Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants