Skip to content

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

Open
darion-yaphet wants to merge 1 commit into
github:mainfrom
darion-yaphet:fix/idempotent-path-rewriting
Open

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

Conversation

@darion-yaphet

Copy link
Copy Markdown
Contributor

Description

`CommandRegistrar.rewrite_project_relative_paths()` previously relied on three consecutive `str.replace` calls followed by three separate `re.sub` passes, and

used .replace(".specify/.specify/", ".specify/").replace(".specify.specify/", ".specify/") as an ad-hoc post-processing fix. This "patch-on-patch" design was
fragile, caused redundant string scanning, and risked regressions when paths were processed multiple times.

This PR refactors the path normalization into a single unified regex match callback:
1. **Single-pass regex callback**: Consolidates multiple string and regex passes into one regex scan with a match handler.
2. **Native idempotency guard**: If a path prefix is already `.specify/`, it is returned unchanged without modification, preventing duplicate prefixing from the

source and eliminating the need for subsequent .replace() corrections.
3. Deterministic routing: Parent-relative paths (../) consistently route to root .specify/<target>/, while top-level / relative scripts/ preserve
extension-local scoping when extension_id is supplied.
4. Expanded delimiters: Recognizes common Markdown punctuation enclosures such as brackets [], parentheses (), braces {}, angle brackets <>, quotes,
and backticks.

Testing

<!-- How did you test your changes? -->

- [x] Ran existing tests with `pytest tests/test_extensions.py` (538 passed)
- [x] Ran new idempotency and delimiter test suite with `pytest tests/test_extensions.py -k "test_rewrite_project_relative_paths"` (5 passed)
- [x] Ran linter check with `ruff check src/specify_cli/agents.py tests/test_extensions.py` (0 errors)

## AI Disclosure

<!-- Per our Contributing guidelines, AI assistance must be disclosed. -->
<!-- See: https://github.com/github/spec-kit/blob/main/CONTRIBUTING.md#ai-contributions-in-spec-kit -->

- [ ] I **did not** use AI assistance for this contribution
- [x] I **did** use AI assistance (describe below)

…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.
@mnriem mnriem added the triage-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review label Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants