Fix: Station disconnect from long clusters or during cluster merges

[JB940]

Before opening a PR: discuss new features on Discord first, and file bugs or small improvements as issues. You must be assigned to an approved issue — unsolicited PRs will be auto-closed.

Add approved & assigned issue number here:

None - small fix, big issue.

Description:

On the main Discord many bug reports started coming in about factories not producing trains. This took me a good while to investigate because the tests did seem to test connecting. Thread with 9 reports Report 2 months ago

Bug origin

• Introduced in PR Fix cluster deletion #3185, Feb 13th
• Added cluster disconnection logic to TrainStation.setCluster
• Unconditionally called removeStation without checking if the station was already in the target cluster.

Mechanisms and triggers

• Cluster.addStation registers the station and invokes TrainStation.setCluster.
• Calling addStation on a registered station caused setCluster to delete it from stations.
• Empty tradeStations prevented factories from finding valid destinations, permanently stopping train production.
• Small clusters avoided the bug -- a 4-hop distance check suppressed duplicate neighbor connections.
• Snapping at rail junctions triggered the bug by processing multiple rail segments.
• Branching tracks or factories that closed loops longer than 4 hops triggered duplicate connection calls.
• Connecting networks across player borders forced cluster merges triggering the bug
• June 2026 updates expanded range to 110 tiles and track length to 155 tiles, causing long-loop connections to form more frequently.

Over-mocking in tests

• tests/core/game/RailNetwork.test.ts and tests/core/game/Cluster.test.ts replaced setCluster with dummy vi.fn().
• Tests only checked that setCluster was called, did not check cluster state.
• TrainStation and Cluster instances were never executed together during connection tests.

Fixes/prevention

• TrainStation.setCluster now checks that this.cluster != new cluster
• Duplicate addStation calls now retain all trade stations.
• tests/core/game/Cluster.test.ts updated to use TrainStation instances and directly test duplicate addStation.

Please complete the following:

• I have added relevant tests to the test directory

Please put your Discord username so you can be contacted if a bug or regression is found:

JB940

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 87a49e19-8789-4313-87db-7588e192cfef

📥 Commits

Reviewing files that changed from the base of the PR and between b47aaaf and d5df9e4.

📒 Files selected for processing (2)

• src/core/game/TrainStation.ts
• tests/core/game/Cluster.test.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

Walkthrough

TrainStation.setCluster now preserves membership when the same cluster is assigned again. Cluster tests use real stations and cover membership, duplicate additions, trade destinations, removal, and merging.

Changes

Cluster reassignment behavior

Layer / File(s)	Summary
Guard same-cluster assignment src/core/game/TrainStation.ts	setCluster removes the station from its current cluster only when the new cluster differs.
Validate cluster membership and idempotency tests/core/game/Cluster.test.ts	Tests use real TrainStation instances and verify cluster references, duplicate additions, trade destinations, removal, and merge behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to d5df9

This change prevents duplicate station registration from disconnecting stations while adding focused regression coverage; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: celant, developingtom, evanpelle

Poem

Same cluster, stay in place,
No needless trip through removal space.
Real stations join the test parade,
Trade paths remain safely made.
Merge and membership now align.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the station disconnection fix for long clusters and cluster merges.
Description check	✅ Passed	The description explains the bug, its triggers, the implementation fix, and the related test updates.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

---

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.}

[evanpelle]

nice catch!