Skip to content

Commit b2de8ba

Browse files
fix: ensure idempotent project-relative path rewriting in CommandRegistrar (#4553)
* fix: ensure idempotent project-relative path rewriting in CommandRegistrar Replace the fragile pattern of three sequential string replacements followed by three re.sub calls and trailing `.replace(".specify/.specify/", ".specify/")` / `.replace(".specify.specify/", ".specify/")` patches in CommandRegistrar.rewrite_project_relative_paths. Consolidate the transformation into a unified regex match callback that: - Inspects matched path prefixes (`.specify/`, `../`, `./`, `/`, or bare) - Naturally guards already-normalized `.specify/` paths from double-prefixing - Directs parent relative references (`../`) to root `.specify/<target>/` - Preserves extension-local script scoping when extension_id is provided - Expands boundary delimiters to include Markdown brackets, parentheses, braces, angle brackets, and backticks Add unit tests in tests/test_extensions.py covering repeated passes for idempotency, markdown enclosure delimiters, and edge-case inputs. Assisted-by: Antigravity (supervised) * fix: preserve parent-relative paths following = in rewrite_project_relative_paths Extend the delimiter boundary character class in CommandRegistrar.rewrite_project_relative_paths to include '=', ensuring option flags (e.g., '--template=../../templates/spec.md') and environment variable assignments (e.g., 'SCRIPT=../../scripts/bash/run.sh') continue to be rewritten properly. Add regression test coverage in tests/test_extensions.py covering '=' assignments and verifying repeated passes for idempotency. Assisted-by: Google Antigravity (model: Gemini 3.8 Flash, supervised) * fix: rewrite parent-relative paths after punctuation separators The delimiter allowlist skipped inputs such as run;../../scripts. Match ../ independently of that list, keep the boundary guard for bare paths, and add punctuation/shell-operator regression cases. Assisted-by: Google Antigravity (model: Gemini 3.8 Flash, supervised) * fix: require two parent segments before repo-root rewrites A single ../ is one directory up, not the repository root. Match (?:\.\./){2,} so ../scripts/ stays unchanged even with extension_id, while ../../ and deeper still map to .specify/<target>/. Assisted-by: Grok 4.7 (model: grok-4.7, autonomous)
1 parent 0a1b557 commit b2de8ba

2 files changed

Lines changed: 186 additions & 19 deletions

File tree

‎src/specify_cli/agents.py‎

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -201,33 +201,47 @@ def rewrite_project_relative_paths(
201201
if not isinstance(text, str) or not text:
202202
return text
203203

204-
for old, new in (
205-
("../../memory/", ".specify/memory/"),
206-
("../../scripts/", ".specify/scripts/"),
207-
("../../templates/", ".specify/templates/"),
208-
):
209-
text = text.replace(old, new)
210-
211-
# Only rewrite top-level style references so existing generated paths
212-
# like ".specify/extensions/<ext>/scripts/..." remain intact. When
213-
# rendering extension commands, top-level "scripts/" is extension-local.
214204
scripts_replacement = (
215205
f".specify/extensions/{extension_id}/scripts/"
216206
if extension_id
217207
else ".specify/scripts/"
218208
)
219-
text = re.sub(r'(^|[\s`"\'(])(?:\.?/)?memory/', r"\1.specify/memory/", text)
220-
text = re.sub(
221-
r'(^|[\s`"\'(])(?:\.?/)?scripts/', rf"\1{scripts_replacement}", text
222-
)
223-
text = re.sub(
224-
r'(^|[\s`"\'(])(?:\.?/)?templates/', r"\1.specify/templates/", text
225-
)
226209

227-
return text.replace(".specify/.specify/", ".specify/").replace(
228-
".specify.specify/", ".specify/"
210+
# Two or more ``../`` segments are the repo-root signal used by
211+
# command templates (``../../scripts/...``) and are matched without
212+
# the delimiter allowlist. A single ``../`` stays untouched: from a
213+
# nested command file it means one directory up, which is not the
214+
# repository root and must not be routed to ``.specify/scripts/``.
215+
# A lookbehind only rejects identifier/dot glue (``not../scripts/``,
216+
# ``..../scripts/``). Bare ``scripts/`` / ``memory/`` / ``templates/``
217+
# still require a recognized boundary so tokens such as
218+
# ``myscripts/`` are not rewritten.
219+
pattern = re.compile(
220+
r"""(?:(?<![.\w])(?P<parent>(?:\.\./){2,})|(?P<boundary>^|[\s`"'(\[{<=])(?P<rel>\.specify/|(?:\.?/))?)(?P<target>scripts|memory|templates)/"""
229221
)
230222

223+
def _replace(m: re.Match) -> str:
224+
target = m.group("target")
225+
226+
if m.group("parent"):
227+
# Two or more ../ segments always map to root .specify/<target>/,
228+
# including when extension_id would otherwise make scripts/ local.
229+
return f".specify/{target}/"
230+
231+
prefix = m.group("boundary")
232+
rel = m.group("rel")
233+
234+
if rel == ".specify/":
235+
# Already normalized to project structure
236+
return m.group(0)
237+
238+
# Top-level or ./ path
239+
if target == "scripts":
240+
return f"{prefix}{scripts_replacement}"
241+
return f"{prefix}.specify/{target}/"
242+
243+
return pattern.sub(_replace, text)
244+
231245
@staticmethod
232246
def rewrite_extension_paths(
233247
text: str, extension_id: str, extension_dir: Path

‎tests/test_extensions.py‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3512,6 +3512,159 @@ def test_rewrite_project_relative_paths_uses_extension_context_for_scripts(self)
35123512
assert ".specify/scripts/bash/setup-plan.sh" in rewritten
35133513
assert ".specify/templates/checklist.md" in rewritten
35143514

3515+
def test_rewrite_project_relative_paths_idempotency(self):
3516+
"""Repeated applications must produce identical results with no double prefixing."""
3517+
from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar
3518+
3519+
samples = [
3520+
("Run scripts/bash/setup-plan.sh --json", None, "Run .specify/scripts/bash/setup-plan.sh --json"),
3521+
("Run ./scripts/bash/setup-plan.sh --json", None, "Run .specify/scripts/bash/setup-plan.sh --json"),
3522+
("Run ../../scripts/bash/setup-plan.sh", None, "Run .specify/scripts/bash/setup-plan.sh"),
3523+
("Run ../../../scripts/bash/setup-plan.sh", None, "Run .specify/scripts/bash/setup-plan.sh"),
3524+
("Read memory/constitution.md", None, "Read .specify/memory/constitution.md"),
3525+
("Read /memory/constitution.md", None, "Read .specify/memory/constitution.md"),
3526+
("Read ./memory/constitution.md", None, "Read .specify/memory/constitution.md"),
3527+
("Read ../../memory/constitution.md", None, "Read .specify/memory/constitution.md"),
3528+
("Read templates/spec.md", None, "Read .specify/templates/spec.md"),
3529+
("Read ./templates/spec.md", None, "Read .specify/templates/spec.md"),
3530+
("Read ../../templates/spec.md", None, "Read .specify/templates/spec.md"),
3531+
("Run .specify/scripts/bash/setup-plan.sh", None, "Run .specify/scripts/bash/setup-plan.sh"),
3532+
("Read .specify/memory/constitution.md", None, "Read .specify/memory/constitution.md"),
3533+
("Read .specify/templates/spec.md", None, "Read .specify/templates/spec.md"),
3534+
("Run scripts/tool.sh", "my-ext", "Run .specify/extensions/my-ext/scripts/tool.sh"),
3535+
("Run ./scripts/tool.sh", "my-ext", "Run .specify/extensions/my-ext/scripts/tool.sh"),
3536+
("Run ../../scripts/tool.sh", "my-ext", "Run .specify/scripts/tool.sh"),
3537+
(
3538+
"Run .specify/extensions/my-ext/scripts/tool.sh",
3539+
"my-ext",
3540+
"Run .specify/extensions/my-ext/scripts/tool.sh",
3541+
),
3542+
(
3543+
"--template=../../templates/spec.md",
3544+
None,
3545+
"--template=.specify/templates/spec.md",
3546+
),
3547+
(
3548+
"SCRIPT=../../scripts/bash/run.sh",
3549+
None,
3550+
"SCRIPT=.specify/scripts/bash/run.sh",
3551+
),
3552+
(
3553+
"--template=templates/spec.md",
3554+
None,
3555+
"--template=.specify/templates/spec.md",
3556+
),
3557+
(
3558+
"SCRIPT=scripts/bash/run.sh",
3559+
"my-ext",
3560+
"SCRIPT=.specify/extensions/my-ext/scripts/bash/run.sh",
3561+
),
3562+
]
3563+
3564+
for text, ext_id, expected in samples:
3565+
once = AgentCommandRegistrar.rewrite_project_relative_paths(text, extension_id=ext_id)
3566+
assert once == expected
3567+
twice = AgentCommandRegistrar.rewrite_project_relative_paths(once, extension_id=ext_id)
3568+
assert twice == expected
3569+
thrice = AgentCommandRegistrar.rewrite_project_relative_paths(twice, extension_id=ext_id)
3570+
assert thrice == expected
3571+
assert ".specify/.specify/" not in thrice
3572+
assert ".specify.specify/" not in thrice
3573+
3574+
def test_rewrite_project_relative_paths_various_delimiters(self):
3575+
"""Paths enclosed by backticks, quotes, brackets, parens, and = should be rewritten."""
3576+
from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar
3577+
3578+
body = (
3579+
"Inline `scripts/bash/run.sh` and \"scripts/bash/run.sh\" and 'scripts/bash/run.sh'\n"
3580+
"Parens (scripts/bash/run.sh) and brackets [scripts/bash/run.sh]\n"
3581+
"Braces {scripts/bash/run.sh} and angles <scripts/bash/run.sh>\n"
3582+
"Flag --template=../../templates/spec.md and assign SCRIPT=../../scripts/bash/run.sh\n"
3583+
"Start of text: scripts/bash/run.sh\n"
3584+
)
3585+
rewritten = AgentCommandRegistrar.rewrite_project_relative_paths(body)
3586+
3587+
assert "`.specify/scripts/bash/run.sh`" in rewritten
3588+
assert "\".specify/scripts/bash/run.sh\"" in rewritten
3589+
assert "'.specify/scripts/bash/run.sh'" in rewritten
3590+
assert "(.specify/scripts/bash/run.sh)" in rewritten
3591+
assert "[.specify/scripts/bash/run.sh]" in rewritten
3592+
assert "{.specify/scripts/bash/run.sh}" in rewritten
3593+
assert "<.specify/scripts/bash/run.sh>" in rewritten
3594+
assert "--template=.specify/templates/spec.md" in rewritten
3595+
assert "SCRIPT=.specify/scripts/bash/run.sh" in rewritten
3596+
assert rewritten.splitlines()[-1] == "Start of text: .specify/scripts/bash/run.sh"
3597+
3598+
# Verify idempotency on multiline text with diverse delimiters
3599+
again = AgentCommandRegistrar.rewrite_project_relative_paths(rewritten)
3600+
assert again == rewritten
3601+
assert ".specify/.specify/" not in again
3602+
3603+
def test_rewrite_project_relative_paths_punctuation_and_shell_operator_boundaries(self):
3604+
"""Parent-relative paths rewrite after punctuation/shell operators.
3605+
3606+
Two or more ``../`` segments are a repo-root signal and must not
3607+
depend on the delimiter allowlist. A single ``../`` stays untouched,
3608+
including when ``extension_id`` is set, so it is not routed to root
3609+
``.specify/scripts/`` or to extension-local scripts. Bare
3610+
``scripts/`` / ``templates/`` / ``memory/`` paths still require a
3611+
recognized boundary so ``myscripts/`` and ``run;scripts/`` stay
3612+
untouched.
3613+
"""
3614+
from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar
3615+
3616+
samples = [
3617+
("run;../../scripts/a.sh", None, "run;.specify/scripts/a.sh"),
3618+
("path:../../templates/a.md", None, "path:.specify/templates/a.md"),
3619+
("run&&../../scripts/a.sh", None, "run&&.specify/scripts/a.sh"),
3620+
("run||../../scripts/a.sh", None, "run||.specify/scripts/a.sh"),
3621+
("cmd|../../scripts/a.sh", None, "cmd|.specify/scripts/a.sh"),
3622+
("x,../../memory/constitution.md", None, "x,.specify/memory/constitution.md"),
3623+
("run;../../../scripts/a.sh", None, "run;.specify/scripts/a.sh"),
3624+
(
3625+
"run;../../scripts/a.sh",
3626+
"my-ext",
3627+
"run;.specify/scripts/a.sh",
3628+
),
3629+
(
3630+
"foo/../../scripts/a.sh",
3631+
None,
3632+
"foo/.specify/scripts/a.sh",
3633+
),
3634+
# Bare paths still need a recognized boundary.
3635+
("run;scripts/a.sh", None, "run;scripts/a.sh"),
3636+
("path:templates/a.md", None, "path:templates/a.md"),
3637+
("run&&scripts/a.sh", None, "run&&scripts/a.sh"),
3638+
("myscripts/a.sh", None, "myscripts/a.sh"),
3639+
# ``../`` must not match inside an identifier or extra dots.
3640+
("not../scripts/a.sh", None, "not../scripts/a.sh"),
3641+
("..../scripts/a.sh", None, "..../scripts/a.sh"),
3642+
# One ``../`` is one directory up, not the repository root.
3643+
("Run ../scripts/a.sh", None, "Run ../scripts/a.sh"),
3644+
("Run ../scripts/a.sh", "my-ext", "Run ../scripts/a.sh"),
3645+
("run;../scripts/a.sh", "my-ext", "run;../scripts/a.sh"),
3646+
("Read ../memory/constitution.md", "my-ext", "Read ../memory/constitution.md"),
3647+
("Read ../templates/spec.md", "my-ext", "Read ../templates/spec.md"),
3648+
]
3649+
3650+
for text, ext_id, expected in samples:
3651+
once = AgentCommandRegistrar.rewrite_project_relative_paths(
3652+
text, extension_id=ext_id
3653+
)
3654+
assert once == expected, text
3655+
twice = AgentCommandRegistrar.rewrite_project_relative_paths(
3656+
once, extension_id=ext_id
3657+
)
3658+
assert twice == expected, text
3659+
3660+
def test_rewrite_project_relative_paths_non_string_or_empty(self):
3661+
"""Non-string and falsy inputs should be returned as-is."""
3662+
from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar
3663+
3664+
assert AgentCommandRegistrar.rewrite_project_relative_paths("") == ""
3665+
assert AgentCommandRegistrar.rewrite_project_relative_paths(None) is None
3666+
assert AgentCommandRegistrar.rewrite_project_relative_paths(123) == 123
3667+
35153668
def test_render_toml_command_handles_embedded_triple_double_quotes(self):
35163669
"""TOML renderer should stay valid when body includes triple double-quotes."""
35173670
from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar

0 commit comments

Comments
 (0)