fix: ensure idempotent project-relative path rewriting in CommandRegistrar by darion-yaphet · Pull Request #4553 · github/spec-kit

darion-yaphet · 2026-09-12T09:18:04Z

Description

Refactors CommandRegistrar.rewrite_project_relative_paths() from multi-step string replacements and sequential regex passes into a unified single-pass regex callback.

Key improvements:

Single-pass regex callback: Replaces the fragile pattern of three sequential string replacements followed by three re.sub passes and trailing .replace(".specify/.specify/", ".specify/") cleanup with a clean, unified regex match callback.
Deterministic routing: Consistently routes deeper parent-relative references (../../, ../../../) to root .specify/<target>/ while preserving extension-local scoping for top-level scripts when extension_id is supplied.
Expanded delimiter & assignment support: Recognizes Markdown punctuation enclosures ([], (), {}, <>), quotes, backticks, and option/variable assignment delimiters (=, e.g., --template=../../templates/spec.md and SCRIPT=../../scripts/bash/run.sh).
Idempotency guard: Naturally guards already-normalized .specify/ paths from duplicate prefixing across repeated passes without needing post-hoc string stripping.

Testing

Ran existing tests with pytest tests/test_extensions.py (542 passed)
Ran idempotency, delimiter, and assignment test suite with pytest tests/test_extensions.py -k "test_rewrite_project_relative_paths" (5 passed)
Ran linter check with ruff check src/specify_cli/agents.py tests/test_extensions.py (0 errors)

AI Disclosure

I did not use AI assistance for this contribution
I did use AI assistance (fill in the disclosure below)

AI disclosure: Implemented with Google Antigravity using Gemini 3.8 Flash (High) in human-supervised mode for code refactoring, test case generation, and addressing review feedback.

mnriem · 2026-09-15T15:46:01Z

The delimiter and deeper-parent-path improvements have useful regression coverage. Before merge, please preserve parent-relative paths following =: --template=../../templates/spec.md and SCRIPT=../../scripts/bash/run.sh were rewritten previously but are now left unchanged. Add regression cases for these alongside the existing idempotency checks.

Please also clarify the description: the supplied samples already remain stable under repeated rewriting on the old code; the demonstrated improvements are routing and delimiter handling. Finally, complete this PR’s AI disclosure with the tool, model, mode/settings, and extent of assistance—the checkbox alone does not describe the contribution.

Drafted for @mnriem by GitHub Copilot (model: GPT-6 Astra; comment drafting).

Copilot

🟢 Approval recommended

The refactor preserves routing behavior and includes focused regression coverage.

Pull request overview

Refactors project-relative path rewriting into an idempotent, single-pass implementation.

Changes:

Consolidates path normalization into one regex callback.
Adds regression coverage for idempotency, delimiters, and invalid inputs.

File summaries

File	Description
`src/specify_cli/agents.py`	Implements unified path rewriting.
`tests/test_extensions.py`	Adds comprehensive regression tests.

Review details

Files reviewed: 2/2 changed files
Comments generated: 0
Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…strar 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)

…lative_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)

darion-yaphet · 2026-09-20T08:45:50Z

Thanks for the thorough review and catching the = assignment case!

I have addressed the feedback in commit f6ecaa9:

Preserved = prefix: Added = to the boundary delimiter class ((^|[\s"'([{<=])), ensuring CLI option flags (like --template=../../templates/spec.md) and environment variable assignments (like SCRIPT=../../scripts/bash/run.sh`) are properly rewritten.
Added regression coverage: Added test cases for = followed by parent-relative and extension-scoped paths in tests/test_extensions.py, asserting correct transformation and idempotency across repeated passes.
Clarified PR description: Clarified in the description that the improvements focus on the single-pass regex architecture, deeper parent-path routing, and expanded delimiter/assignment handling.
Completed AI disclosure: Updated the PR body with the full AI disclosure (tool, model, supervised mode, and extent of assistance).

Drafted on behalf of @darion-yaphet by Google Antigravity (model: Gemini 3.8 Flash (High), human-supervised; comment drafting and code refinement).

Copilot

Copilot review overview

🟡 Changes recommended

Parent-relative paths after valid punctuation or shell operators are no longer rewritten.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

Handle parent-relative paths after all valid separators · New

darion-yaphet · 2026-09-22T03:20:35Z

-        return text.replace(".specify/.specify/", ".specify/").replace(
-            ".specify.specify/", ".specify/"
+        pattern = re.compile(
+            r"""(^|[\s`"'(\[{<=])(\.specify/|(?:\.\./)+|(?:\.?/))?(scripts|memory|templates)/"""


../ now matches independently of the delimiter allowlist, so parent-relative paths after ;, :, &&, and other operators rewrite to .specify/.... Bare scripts/ / memory/ / templates/ still require a recognized boundary. Covered in test_rewrite_project_relative_paths_punctuation_and_shell_operator_boundaries.

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)

darion-yaphet requested a review from mnriem as a code owner September 12, 2026 09:18

mnriem added triage-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review author-needs-disclosure AI use, or the agent/model/settings behind it, not disclosed per CONTRIBUTING author-awaiting Waiting on author response labels Sep 12, 2026

mnriem requested a balanced review from Copilot September 15, 2026 15:46

Copilot started reviewing on behalf of mnriem September 15, 2026 15:47 View session

Copilot AI reviewed Sep 15, 2026

View reviewed changes

darion-yaphet added 2 commits September 20, 2026 16:40

darion-yaphet force-pushed the fix/idempotent-path-rewriting branch from 661957c to f6ecaa9 Compare September 20, 2026 08:45

mnriem removed author-needs-disclosure AI use, or the agent/model/settings behind it, not disclosed per CONTRIBUTING author-awaiting Waiting on author response labels Sep 21, 2026

mnriem requested a balanced review from Copilot September 21, 2026 19:33

Copilot started reviewing on behalf of mnriem September 21, 2026 19:33 View session

Copilot AI reviewed Sep 21, 2026

View reviewed changes

Navigation Menu

fix: ensure idempotent project-relative path rewriting in CommandRegistrar - #4553

fix: ensure idempotent project-relative path rewriting in CommandRegistrar#4553
darion-yaphet wants to merge 3 commits into
github:mainfrom
darion-yaphet:fix/idempotent-path-rewriting

darion-yaphet commented Sep 12, 2026 •

edited

Loading

Uh oh!

mnriem commented Sep 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

darion-yaphet commented Sep 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

darion-yaphet Sep 22, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

darion-yaphet commented Sep 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

AI Disclosure

Uh oh!

mnriem commented Sep 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

darion-yaphet commented Sep 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

darion-yaphet Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

darion-yaphet commented Sep 12, 2026 •

edited

Loading