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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant