Skip to content

fix(links): prevent panic when page title equals path hint in RewriteWikiLinks#1164

Merged
perber merged 2 commits into
mainfrom
fix/wikilink-rename-panic
Jun 16, 2026
Merged

fix(links): prevent panic when page title equals path hint in RewriteWikiLinks#1164
perber merged 2 commits into
mainfrom
fix/wikilink-rename-panic

Conversation

@perber

@perber perber commented Jun 16, 2026

Copy link
Copy Markdown
Owner

When OldTitle and the normalised OldPath hint are the same string (e.g. title "intro", path "/intro"), both the title rewrite and the path-hint rewrite generated a regex that matched the identical byte range. applyReplacements then tried to slice content[End:Start] (e.g. [18:10]) and panicked.

Fix: skip the path-hint rewrite when the title rewrite already covers it (strings.EqualFold comparison), and add a defensive overlap-dedup pass after sorting replacements. Regression test added.

Copilot AI review requested due to automatic review settings June 16, 2026 11:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a panic in MarkdownRefactorEngine.RewriteWikiLinks that could occur when a rule’s OldTitle matches the normalized OldPath hint, causing overlapping replacements and an invalid slice operation during replacement application.

Changes:

  • Skip generating the path-hint rewrite when the title rewrite already covers the same target (case-insensitive).
  • Add an overlap-deduplication pass over sorted replacements to prevent slice-bounds panics.
  • Add a regression test reproducing the panic scenario and asserting correct rewrite output.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
internal/links/link_refactor.go Prevents overlapping rewrite replacements in RewriteWikiLinks via rule-level suppression and post-sort deduplication.
internal/links/link_refactor_wikilinks_test.go Adds a regression test covering the OldTitle == OldPath hint overlap/panic case.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/links/link_refactor.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread internal/links/link_refactor.go Outdated
Comment thread internal/links/link_refactor_wikilinks_test.go Outdated
…hint

Root cause: when OldTitle matched the normalized path hint (e.g. "intro" ==
"intro"), both the case-insensitive title regex and the path-hint regex matched
the same byte span. With sort.Slice (unstable), the two replacements could land
in arbitrary order, and the second one produced a backwards slice index
[18:10] → panic in applyReplacements.

Fix:
- Use sort.SliceStable so the title rewrite (inserted first) always wins
- Add a dedup pass that drops any replacement whose start falls inside the
  previous replacement's end range
- Remove the titleCoversHint pre-filter (too many edge cases); dedup guard
  handles all overlap cases uniformly
- Add overlap guard inside applyReplacements itself as a last-resort safety net

Performance (bulk rename):
- Extract CompiledWikiLinkRewrites + CompileWikiLinkRewrites so regexes are
  compiled once per operation (N) instead of once per page (M×N)
- RewriteWikiLinks becomes a thin wrapper; callers doing bulk renames use
  RewriteWikiLinksPrecompiled

Refactor:
- Add collectExcludedRanges helper; RewriteWikiLinksPrecompiled and
  FindWikiLinksForPath call it instead of discarding the candidates return
  value of collectCandidatesAndExcludedRanges
- Add debug log before bulk page loop: pages=N rules=M

Regression tests added for both panic scenario and path-only move with same title.
@perber
perber force-pushed the fix/wikilink-rename-panic branch from 6504672 to 3260c28 Compare June 16, 2026 13:15
@perber
perber requested a review from Copilot June 16, 2026 13:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread internal/links/link_refactor.go
Comment thread internal/links/link_refactor_wikilinks_test.go Outdated
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
@perber
perber merged commit f96c331 into main Jun 16, 2026
9 checks passed
@perber
perber deleted the fix/wikilink-rename-panic branch June 16, 2026 13:28
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.

2 participants