Fixed web history rewriting incorrectly when started via a link

[arkivanov]

Fixes: #982

Summary by CodeRabbit

• Breaking Changes

  • Simplified onRewrite callback signature in web history navigation API—now accepts only the navigation history parameter

• Tests

  • Enhanced test coverage for web history navigation scenarios, including root replacement with child node transitions

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Simplified the onRewrite callback signature by removing the oldSize parameter and replacing its usage with computations based on currentIndex. This affects callback invocations in the web history subscription mechanism and internal history management logic.

Changes

Cohort / File(s)	Summary
WebHistory callback refactoring decompose/src/webMain/kotlin/com/arkivanov/decompose/router/webhistory/WebHistoryNavigation.kt, decompose/src/webMain/kotlin/com/arkivanov/decompose/router/webhistory/WebNavigation.kt	Removed oldSize parameter from onRewrite callback signature; replaced go(-oldSize + 1) with go(-currentIndex) logic; updated internal history change computation to derive values directly from newHistory.
Test infrastructure updates decompose/src/webTest/kotlin/com/arkivanov/decompose/router/webhistory/TestBrowserHistory.kt	Added assertion in go(delta) to validate index remains within bounds after update, capturing oldIndex for error messaging.
Test imports decompose/src/webTest/kotlin/com/arkivanov/decompose/router/webhistory/TestWebNavigation.kt	Added kotlin.test.assertEquals import.
Test case addition decompose/src/webTest/kotlin/com/arkivanov/decompose/router/webhistory/WebHistoryNavigationTest.kt	Added new test GIVEN_created_one_root_and_two_children_WHEN_root_replaced_with_with_one_child_THEN_one_item_in_history validating history state when root is replaced with a single child.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Extract BrowserHistory interface for testing #817: Established BrowserHistory/TestBrowserHistory abstraction that serves as the foundation for the history management changes in this PR.
• Added the new web history API with nested navigation support #821: Prior modifications to the web-history subsystem that introduced the original callback signatures now being refactored in this change.

Suggested reviewers

• chrisjenx

Poem

🐰 A history rewritten, simplified and bright,
No more oldSize weighing down the load,
Just currentIndex lighting up the night,
Hop along this cleaner, swifter road!
Browser back buttons now work just right! 🌙

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: fixing web history rewriting behavior when navigation is initiated via a link, which directly addresses the linked issue #982.
Linked Issues check	✅ Passed	The PR changes address the core issues: replacing oldSize parameter with currentIndex to fix rewrite logic when Details stack initializes with multiple elements, enabling proper URL updates and browser back navigation.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing web history rewriting: API signature updates for onRewrite callback, internal logic adjustments, test additions validating the fix, and minor test infrastructure updates.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix-web-history-rewrite

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

decompose/src/webMain/kotlin/com/arkivanov/decompose/router/webhistory/WebHistoryNavigation.kt (1)

63-76: ⚠️ Potential issue | 🟠 Major

onRewrite callback should protect against navigating into pre-app browser history.

browserHistory.length returns the total browser session history via window.history.length, which includes entries before the app loaded. The current code at line 71 (go(-historyLength + 1)) lacks the bounds protection used elsewhere.

Compare with line 47 (onPop), which correctly uses coerceAtMost(browserHistory.currentIndex()) to prevent navigation beyond app-tracked state. The onRewrite callback should apply the same pattern to ensure go() stays within app boundaries.

🤖 Fix all issues with AI agents

In
`@decompose/src/webTest/kotlin/com/arkivanov/decompose/router/webhistory/TestBrowserHistory.kt`:
- Around line 13-21: The length getter currently returns index + 1 which
reflects the current position, not the total session history; update override
val length: Int to return stack.size instead of index + 1 (in
TestBrowserHistory’s properties), so that length reflects the total number of
entries in stack; leave state, go, push/pop logic unchanged but run tests to
ensure callers expecting the old value are adjusted.

[arkivanov]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

decompose/src/webMain/kotlin/com/arkivanov/decompose/router/webhistory/WebHistoryNavigation.kt (1)> 63-76: ⚠️ Potential issue | 🟠 Major

onRewrite callback should protect against navigating into pre-app browser history.
browserHistory.length returns the total browser session history via window.history.length, which includes entries before the app loaded. The current code at line 71 (go(-historyLength + 1)) lacks the bounds protection used elsewhere.
Compare with line 47 (onPop), which correctly uses coerceAtMost(browserHistory.currentIndex()) to prevent navigation beyond app-tracked state. The onRewrite callback should apply the same pattern to ensure go() stays within app boundaries.

🤖 Fix all issues with AI agents

Fixed.