From 33a8caecdde17e2d80bfb7a5e71ded975cebac7a Mon Sep 17 00:00:00 2001 From: wangchenguang Date: Sat, 12 Sep 2026 17:15:59 +0800 Subject: [PATCH 1/4] 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//` - 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) --- src/specify_cli/agents.py | 41 ++++++++++++---------- tests/test_extensions.py | 73 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 19 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 76f40abe06..cfeae8473a 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -201,33 +201,36 @@ def rewrite_project_relative_paths( if not isinstance(text, str) or not text: return text - for old, new in ( - ("../../memory/", ".specify/memory/"), - ("../../scripts/", ".specify/scripts/"), - ("../../templates/", ".specify/templates/"), - ): - text = text.replace(old, new) - - # Only rewrite top-level style references so existing generated paths - # like ".specify/extensions//scripts/..." remain intact. When - # rendering extension commands, top-level "scripts/" is extension-local. scripts_replacement = ( f".specify/extensions/{extension_id}/scripts/" if extension_id else ".specify/scripts/" ) - text = re.sub(r'(^|[\s`"\'(])(?:\.?/)?memory/', r"\1.specify/memory/", text) - text = re.sub( - r'(^|[\s`"\'(])(?:\.?/)?scripts/', rf"\1{scripts_replacement}", text - ) - text = re.sub( - r'(^|[\s`"\'(])(?:\.?/)?templates/', r"\1.specify/templates/", text - ) - return text.replace(".specify/.specify/", ".specify/").replace( - ".specify.specify/", ".specify/" + pattern = re.compile( + r"""(^|[\s`"'(\[{<])(\.specify/|(?:\.\./)+|(?:\.?/))?(scripts|memory|templates)/""" ) + def _replace(m: re.Match) -> str: + prefix = m.group(1) + rel = m.group(2) + target = m.group(3) + + if rel == ".specify/": + # Already normalized to project structure + return m.group(0) + + if rel and rel.startswith("../"): + # Explicit repo-relative path always maps to root .specify// + return f"{prefix}.specify/{target}/" + + # Top-level or ./ path + if target == "scripts": + return f"{prefix}{scripts_replacement}" + return f"{prefix}.specify/{target}/" + + return pattern.sub(_replace, text) + @staticmethod def rewrite_extension_paths( text: str, extension_id: str, extension_dir: Path diff --git a/tests/test_extensions.py b/tests/test_extensions.py index fb6da1803e..684c9aafe8 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -3513,6 +3513,79 @@ def test_rewrite_project_relative_paths_uses_extension_context_for_scripts(self) assert ".specify/scripts/bash/setup-plan.sh" in rewritten assert ".specify/templates/checklist.md" in rewritten + def test_rewrite_project_relative_paths_idempotency(self): + """Repeated applications must produce identical results with no double prefixing.""" + from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar + + samples = [ + ("Run scripts/bash/setup-plan.sh --json", None, "Run .specify/scripts/bash/setup-plan.sh --json"), + ("Run ./scripts/bash/setup-plan.sh --json", None, "Run .specify/scripts/bash/setup-plan.sh --json"), + ("Run ../../scripts/bash/setup-plan.sh", None, "Run .specify/scripts/bash/setup-plan.sh"), + ("Run ../../../scripts/bash/setup-plan.sh", None, "Run .specify/scripts/bash/setup-plan.sh"), + ("Read memory/constitution.md", None, "Read .specify/memory/constitution.md"), + ("Read /memory/constitution.md", None, "Read .specify/memory/constitution.md"), + ("Read ./memory/constitution.md", None, "Read .specify/memory/constitution.md"), + ("Read ../../memory/constitution.md", None, "Read .specify/memory/constitution.md"), + ("Read templates/spec.md", None, "Read .specify/templates/spec.md"), + ("Read ./templates/spec.md", None, "Read .specify/templates/spec.md"), + ("Read ../../templates/spec.md", None, "Read .specify/templates/spec.md"), + ("Run .specify/scripts/bash/setup-plan.sh", None, "Run .specify/scripts/bash/setup-plan.sh"), + ("Read .specify/memory/constitution.md", None, "Read .specify/memory/constitution.md"), + ("Read .specify/templates/spec.md", None, "Read .specify/templates/spec.md"), + ("Run scripts/tool.sh", "my-ext", "Run .specify/extensions/my-ext/scripts/tool.sh"), + ("Run ./scripts/tool.sh", "my-ext", "Run .specify/extensions/my-ext/scripts/tool.sh"), + ("Run ../../scripts/tool.sh", "my-ext", "Run .specify/scripts/tool.sh"), + ( + "Run .specify/extensions/my-ext/scripts/tool.sh", + "my-ext", + "Run .specify/extensions/my-ext/scripts/tool.sh", + ), + ] + + for text, ext_id, expected in samples: + once = AgentCommandRegistrar.rewrite_project_relative_paths(text, extension_id=ext_id) + assert once == expected + twice = AgentCommandRegistrar.rewrite_project_relative_paths(once, extension_id=ext_id) + assert twice == expected + thrice = AgentCommandRegistrar.rewrite_project_relative_paths(twice, extension_id=ext_id) + assert thrice == expected + assert ".specify/.specify/" not in thrice + assert ".specify.specify/" not in thrice + + def test_rewrite_project_relative_paths_various_delimiters(self): + """Paths enclosed by backticks, quotes, brackets, and parens should be rewritten.""" + from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar + + body = ( + "Inline `scripts/bash/run.sh` and \"scripts/bash/run.sh\" and 'scripts/bash/run.sh'\n" + "Parens (scripts/bash/run.sh) and brackets [scripts/bash/run.sh]\n" + "Braces {scripts/bash/run.sh} and angles \n" + "Start of text: scripts/bash/run.sh\n" + ) + rewritten = AgentCommandRegistrar.rewrite_project_relative_paths(body) + + assert "`.specify/scripts/bash/run.sh`" in rewritten + assert "\".specify/scripts/bash/run.sh\"" in rewritten + assert "'.specify/scripts/bash/run.sh'" in rewritten + assert "(.specify/scripts/bash/run.sh)" in rewritten + assert "[.specify/scripts/bash/run.sh]" in rewritten + assert "{.specify/scripts/bash/run.sh}" in rewritten + assert "<.specify/scripts/bash/run.sh>" in rewritten + assert rewritten.splitlines()[-1] == "Start of text: .specify/scripts/bash/run.sh" + + # Verify idempotency on multiline text with diverse delimiters + again = AgentCommandRegistrar.rewrite_project_relative_paths(rewritten) + assert again == rewritten + assert ".specify/.specify/" not in again + + def test_rewrite_project_relative_paths_non_string_or_empty(self): + """Non-string and falsy inputs should be returned as-is.""" + from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar + + assert AgentCommandRegistrar.rewrite_project_relative_paths("") == "" + assert AgentCommandRegistrar.rewrite_project_relative_paths(None) is None + assert AgentCommandRegistrar.rewrite_project_relative_paths(123) == 123 + def test_render_toml_command_handles_embedded_triple_double_quotes(self): """TOML renderer should stay valid when body includes triple double-quotes.""" from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar From f6ecaa9be9f7af10dcac1d2f67af0c3e2882dd4d Mon Sep 17 00:00:00 2001 From: wangchenguang Date: Sun, 20 Sep 2026 16:43:20 +0800 Subject: [PATCH 2/4] 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) --- src/specify_cli/agents.py | 2 +- tests/test_extensions.py | 25 ++++++++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index cfeae8473a..2043c272db 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -208,7 +208,7 @@ def rewrite_project_relative_paths( ) pattern = re.compile( - r"""(^|[\s`"'(\[{<])(\.specify/|(?:\.\./)+|(?:\.?/))?(scripts|memory|templates)/""" + r"""(^|[\s`"'(\[{<=])(\.specify/|(?:\.\./)+|(?:\.?/))?(scripts|memory|templates)/""" ) def _replace(m: re.Match) -> str: diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 684c9aafe8..fecceba458 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -3540,6 +3540,26 @@ def test_rewrite_project_relative_paths_idempotency(self): "my-ext", "Run .specify/extensions/my-ext/scripts/tool.sh", ), + ( + "--template=../../templates/spec.md", + None, + "--template=.specify/templates/spec.md", + ), + ( + "SCRIPT=../../scripts/bash/run.sh", + None, + "SCRIPT=.specify/scripts/bash/run.sh", + ), + ( + "--template=templates/spec.md", + None, + "--template=.specify/templates/spec.md", + ), + ( + "SCRIPT=scripts/bash/run.sh", + "my-ext", + "SCRIPT=.specify/extensions/my-ext/scripts/bash/run.sh", + ), ] for text, ext_id, expected in samples: @@ -3553,13 +3573,14 @@ def test_rewrite_project_relative_paths_idempotency(self): assert ".specify.specify/" not in thrice def test_rewrite_project_relative_paths_various_delimiters(self): - """Paths enclosed by backticks, quotes, brackets, and parens should be rewritten.""" + """Paths enclosed by backticks, quotes, brackets, parens, and = should be rewritten.""" from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar body = ( "Inline `scripts/bash/run.sh` and \"scripts/bash/run.sh\" and 'scripts/bash/run.sh'\n" "Parens (scripts/bash/run.sh) and brackets [scripts/bash/run.sh]\n" "Braces {scripts/bash/run.sh} and angles \n" + "Flag --template=../../templates/spec.md and assign SCRIPT=../../scripts/bash/run.sh\n" "Start of text: scripts/bash/run.sh\n" ) rewritten = AgentCommandRegistrar.rewrite_project_relative_paths(body) @@ -3571,6 +3592,8 @@ def test_rewrite_project_relative_paths_various_delimiters(self): assert "[.specify/scripts/bash/run.sh]" in rewritten assert "{.specify/scripts/bash/run.sh}" in rewritten assert "<.specify/scripts/bash/run.sh>" in rewritten + assert "--template=.specify/templates/spec.md" in rewritten + assert "SCRIPT=.specify/scripts/bash/run.sh" in rewritten assert rewritten.splitlines()[-1] == "Start of text: .specify/scripts/bash/run.sh" # Verify idempotency on multiline text with diverse delimiters From cbe0616a622131ce3b9204616f42c774dae89b65 Mon Sep 17 00:00:00 2001 From: wangchenguang Date: Tue, 22 Sep 2026 11:21:15 +0800 Subject: [PATCH 3/4] 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) --- src/specify_cli/agents.py | 22 +++++++++++------- tests/test_extensions.py | 48 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 2043c272db..ed1527c8e2 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -207,23 +207,29 @@ def rewrite_project_relative_paths( else ".specify/scripts/" ) + # ``../`` is an explicit repo-relative signal and is matched without + # the delimiter allowlist. A lookbehind only rejects identifier/dot + # glue (``not../scripts/``, ``..../scripts/``). Bare ``scripts/`` / + # ``memory/`` / ``templates/`` still require a recognized boundary + # so tokens such as ``myscripts/`` are not rewritten. pattern = re.compile( - r"""(^|[\s`"'(\[{<=])(\.specify/|(?:\.\./)+|(?:\.?/))?(scripts|memory|templates)/""" + r"""(?:(?(?:\.\./)+)|(?P^|[\s`"'(\[{<=])(?P\.specify/|(?:\.?/))?)(?Pscripts|memory|templates)/""" ) def _replace(m: re.Match) -> str: - prefix = m.group(1) - rel = m.group(2) - target = m.group(3) + target = m.group("target") + + if m.group("parent"): + # Explicit repo-relative path always maps to root .specify// + return f".specify/{target}/" + + prefix = m.group("boundary") + rel = m.group("rel") if rel == ".specify/": # Already normalized to project structure return m.group(0) - if rel and rel.startswith("../"): - # Explicit repo-relative path always maps to root .specify// - return f"{prefix}.specify/{target}/" - # Top-level or ./ path if target == "scripts": return f"{prefix}{scripts_replacement}" diff --git a/tests/test_extensions.py b/tests/test_extensions.py index fecceba458..d28d3d3b38 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -3601,6 +3601,54 @@ def test_rewrite_project_relative_paths_various_delimiters(self): assert again == rewritten assert ".specify/.specify/" not in again + def test_rewrite_project_relative_paths_punctuation_and_shell_operator_boundaries(self): + """Parent-relative paths rewrite after punctuation/shell operators. + + ``../`` is an explicit repo-relative signal and must not depend on the + delimiter allowlist. Bare ``scripts/`` / ``templates/`` / ``memory/`` + paths still require a recognized boundary so ``myscripts/`` and + ``run;scripts/`` stay untouched. + """ + from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar + + samples = [ + ("run;../../scripts/a.sh", None, "run;.specify/scripts/a.sh"), + ("path:../../templates/a.md", None, "path:.specify/templates/a.md"), + ("run&&../../scripts/a.sh", None, "run&&.specify/scripts/a.sh"), + ("run||../../scripts/a.sh", None, "run||.specify/scripts/a.sh"), + ("cmd|../../scripts/a.sh", None, "cmd|.specify/scripts/a.sh"), + ("x,../../memory/constitution.md", None, "x,.specify/memory/constitution.md"), + ("run;../../../scripts/a.sh", None, "run;.specify/scripts/a.sh"), + ( + "run;../../scripts/a.sh", + "my-ext", + "run;.specify/scripts/a.sh", + ), + ( + "foo/../../scripts/a.sh", + None, + "foo/.specify/scripts/a.sh", + ), + # Bare paths still need a recognized boundary. + ("run;scripts/a.sh", None, "run;scripts/a.sh"), + ("path:templates/a.md", None, "path:templates/a.md"), + ("run&&scripts/a.sh", None, "run&&scripts/a.sh"), + ("myscripts/a.sh", None, "myscripts/a.sh"), + # ``../`` must not match inside an identifier or extra dots. + ("not../scripts/a.sh", None, "not../scripts/a.sh"), + ("..../scripts/a.sh", None, "..../scripts/a.sh"), + ] + + for text, ext_id, expected in samples: + once = AgentCommandRegistrar.rewrite_project_relative_paths( + text, extension_id=ext_id + ) + assert once == expected, text + twice = AgentCommandRegistrar.rewrite_project_relative_paths( + once, extension_id=ext_id + ) + assert twice == expected, text + def test_rewrite_project_relative_paths_non_string_or_empty(self): """Non-string and falsy inputs should be returned as-is.""" from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar From e21c315c55a3ec4c176b3ccf7ca52c2e300fa85f Mon Sep 17 00:00:00 2001 From: wangchenguang Date: Wed, 23 Sep 2026 01:21:51 +0800 Subject: [PATCH 4/4] 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//. Assisted-by: Grok 4.7 (model: grok-4.7, autonomous) --- src/specify_cli/agents.py | 19 ++++++++++++------- tests/test_extensions.py | 17 +++++++++++++---- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index ed1527c8e2..134b4e6d2f 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -207,20 +207,25 @@ def rewrite_project_relative_paths( else ".specify/scripts/" ) - # ``../`` is an explicit repo-relative signal and is matched without - # the delimiter allowlist. A lookbehind only rejects identifier/dot - # glue (``not../scripts/``, ``..../scripts/``). Bare ``scripts/`` / - # ``memory/`` / ``templates/`` still require a recognized boundary - # so tokens such as ``myscripts/`` are not rewritten. + # Two or more ``../`` segments are the repo-root signal used by + # command templates (``../../scripts/...``) and are matched without + # the delimiter allowlist. A single ``../`` stays untouched: from a + # nested command file it means one directory up, which is not the + # repository root and must not be routed to ``.specify/scripts/``. + # A lookbehind only rejects identifier/dot glue (``not../scripts/``, + # ``..../scripts/``). Bare ``scripts/`` / ``memory/`` / ``templates/`` + # still require a recognized boundary so tokens such as + # ``myscripts/`` are not rewritten. pattern = re.compile( - r"""(?:(?(?:\.\./)+)|(?P^|[\s`"'(\[{<=])(?P\.specify/|(?:\.?/))?)(?Pscripts|memory|templates)/""" + r"""(?:(?(?:\.\./){2,})|(?P^|[\s`"'(\[{<=])(?P\.specify/|(?:\.?/))?)(?Pscripts|memory|templates)/""" ) def _replace(m: re.Match) -> str: target = m.group("target") if m.group("parent"): - # Explicit repo-relative path always maps to root .specify// + # Two or more ../ segments always map to root .specify//, + # including when extension_id would otherwise make scripts/ local. return f".specify/{target}/" prefix = m.group("boundary") diff --git a/tests/test_extensions.py b/tests/test_extensions.py index d28d3d3b38..46e5a076da 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -3604,10 +3604,13 @@ def test_rewrite_project_relative_paths_various_delimiters(self): def test_rewrite_project_relative_paths_punctuation_and_shell_operator_boundaries(self): """Parent-relative paths rewrite after punctuation/shell operators. - ``../`` is an explicit repo-relative signal and must not depend on the - delimiter allowlist. Bare ``scripts/`` / ``templates/`` / ``memory/`` - paths still require a recognized boundary so ``myscripts/`` and - ``run;scripts/`` stay untouched. + Two or more ``../`` segments are a repo-root signal and must not + depend on the delimiter allowlist. A single ``../`` stays untouched, + including when ``extension_id`` is set, so it is not routed to root + ``.specify/scripts/`` or to extension-local scripts. Bare + ``scripts/`` / ``templates/`` / ``memory/`` paths still require a + recognized boundary so ``myscripts/`` and ``run;scripts/`` stay + untouched. """ from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar @@ -3637,6 +3640,12 @@ def test_rewrite_project_relative_paths_punctuation_and_shell_operator_boundarie # ``../`` must not match inside an identifier or extra dots. ("not../scripts/a.sh", None, "not../scripts/a.sh"), ("..../scripts/a.sh", None, "..../scripts/a.sh"), + # One ``../`` is one directory up, not the repository root. + ("Run ../scripts/a.sh", None, "Run ../scripts/a.sh"), + ("Run ../scripts/a.sh", "my-ext", "Run ../scripts/a.sh"), + ("run;../scripts/a.sh", "my-ext", "run;../scripts/a.sh"), + ("Read ../memory/constitution.md", "my-ext", "Read ../memory/constitution.md"), + ("Read ../templates/spec.md", "my-ext", "Read ../templates/spec.md"), ] for text, ext_id, expected in samples: