Skip to content

fix(walker): report ARRIVED when a pass starts on the final path endpoint - #1959

Merged
chsami merged 1 commit into
developmentfrom
claude/walker-door-exit-race
Oct 13, 2026
Merged

chsami merged 1 commit into
developmentfrom
claude/walker-door-exit-race

Conversation

@chsami

@chsami chsami commented Oct 13, 2026

Copy link
Copy Markdown
Owner

Problem

The 2.6.32 release E2E (#1957, known issue) saw Rs2Walker.walkWithState return EXIT instead of ARRIVED for a walk through a closed door that ended exactly on the target. The log sequence was:

[WebWalk] clear | rs2walker:processWalk:reached-path-endpoint
[WebWalk] exit_dbg | trace=cancel:processWalk:after-stuck-check target=3227,3214 currentTarget=null
[WebWalk] exit | r=cancel:processWalk:after-stuck-check nullCur=true

Root cause: when a processWalk pass begins with the player standing on the final (non-partial) path endpoint and no interim target set, it calls setTarget(null, "reached-path-endpoint") and keeps going. A few lines later, walkCancelledDiag(..., "processWalk:after-stuck-check") sees currentTarget == null, takes it for an external cancel, and returns EXIT. The arrived-immediate check further down would have returned ARRIVED, but it never runs.

This happens every time a pass starts on the endpoint. It only looks timing-dependent because most walks arrive during a movement wait inside a pass, where arrived-within-distance or arrived-after-door-canvas-nudge handle it. After the door interaction in the failing run, the next pass started on the endpoint.

Change

The endpoint branch now goes through a small helper, settleReachedPathEndpoint:

  1. Cancel check first. It runs walkCancelledDiag(target, "processWalk:at-path-endpoint"). If another thread called setTarget(null), or the target changed, the thread was interrupted, the human took over input or a completion condition was met, the result is still EXIT with an accurate trace. Arrival never hides a real cancel.
  2. Arrival check. It returns ARRIVED when the player is within tightFinishThreshold(target, endpoint, distance) of the target. That is the same threshold arrived-within-distance uses later in the same pass.
  3. Otherwise EXIT as before. If the player is on the endpoint but outside that tight threshold, the result is still EXIT. The log now says path-endpoint-short-of-target instead of the misleading cancel:processWalk:after-stuck-check.

In all three cases the target is cleared with the same reached-path-endpoint reason as before.

I also checked the other places that clear the target. The tryDirectShortWalk arrivals and arrived-immediate, arrived-within-distance and arrived-after-door-canvas-nudge already return ARRIVED directly. off-path-cancel-instead, the rockfall and obstacle aborts, completion-condition-met and the error exits mean to stop the walk. The endpoint branch was the only one that cleared the target on success and then reached a cancel check.

To keep legacyProcessWalkCannotGrow passing, the logic lives in the helper and the call site in processWalk stays at its old length. A package-private processWalkForTesting seam drives one real processWalk call from the new test.

Testing

  • New Rs2WalkerPathEndpointArrivalTest runs the real processWalk loop with Microbot, Rs2Player, Rs2Tile and Rs2PathApi mocked:
    • standingOnTheEndpointThatIsTheTargetArrives: player on the endpoint, which is the target, distance 0. Expects ARRIVED. Fails without the fix (expected:<ARRIVED> but was:<EXIT>), with the same clear | reached-path-endpoint, then exit | r=cancel:processWalk:after-stuck-check nullCur=true sequence as the E2E log.
    • externalCancelBeforeTheEndpointCheckStillExits: currentTarget is set to null mid-pass, before the endpoint check. Expects EXIT, with the target not cleared as reached-path-endpoint. Also fails without the fix, because the old code cleared with that reason regardless.
    • endpointShortOfTheTightFinishThresholdDoesNotArrive: endpoint 3 tiles from the target with distance 5 (tight threshold 1). Expects EXIT. Passes before and after, which pins the unchanged behaviour.
  • ./gradlew :client:runUnitTests --offline: BUILD SUCCESSFUL. 2384 tests ran, 2382 passed, 2 skipped and none failed, WalkerArchitectureGuardTest and ClientThreadGuardrailTest included.
  • Live: MICROBOT_E2E_WORLD=301 scripts/run-release-e2e.sh quick walker-transport with a shaded jar built from this branch, run 4 times, PASSED 4/4 (59.7s to 1m05s each).

Not verified live: none of the 4 runs started a pass on the endpoint. In every run, arrival went through arrived-within-distance or tryDirectShortWalk, so the changed branch never ran live. The unit test is the evidence for the fix. The E2E runs only show that the door, short-walk and stairs flows still work.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LjtEMzQHGuq4M1Mpqk9pPV

…oint

processWalk cleared the target on reaching the path endpoint and let the
same pass fall through to the after-stuck-check, which read the null
currentTarget as an external cancel and returned EXIT even when the
player stood on the goal. The endpoint branch now checks for a genuine
cancel first, then returns ARRIVED when the player is within the tight
finish threshold of the target (EXIT otherwise, with an honest trace).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LjtEMzQHGuq4M1Mpqk9pPV
@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4f49075c-9fd6-4148-8ac2-72dc9a1fa110
📥 Commits

Reviewing files that changed from the base of the PR and between 00d5dad and f77b03c.

📒 Files selected for processing: 2
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java
  • runelite-client/src/test/java/net/runelite/client/plugins/microbot/util/walker/Rs2WalkerPathEndpointArrivalTest.java

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

Walkthrough

Rs2Walker now settles a reached path endpoint by checking cancellation and comparing the player’s distance to the goal with tightFinishThreshold. It clears the target and returns ARRIVED when the threshold is met, or EXIT otherwise. A package-private wrapper supports tests of processWalk. New tests cover arrival, cancellation before endpoint settlement, and a threshold shortfall.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to f77b0

The endpoint arrival change is ready to merge after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary walker fix: reporting ARRIVED when a pass starts on the final path endpoint.
Description check Passed The description is directly related to the changes. It explains the endpoint bug, the cancellation and arrival handling, the tests, and the reported validation results.
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Comment @coderabbitai help to get the list of available commands.

@chsami
chsami merged commit fdf8475 into development Oct 13, 2026
3 checks passed
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