Fix issue #20 - #59
Conversation
|
Warning Review limit reached
Next review available in: 32 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe bridge now gates state requests on capability resolution, coalesces pending refreshes, retries failed bootstrap requests, and tracks connection generations. HELLO, timers, disconnects, and world entry use generation-aware bootstrap handling. ChangesBootstrap state flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant StateRequest
participant CapabilityResolution
participant BootstrapRetrieval
participant ConnectionLifecycle
StateRequest->>CapabilityResolution: queue refresh
CapabilityResolution->>BootstrapRetrieval: flush queued refreshes
BootstrapRetrieval->>BootstrapRetrieval: guard request by generation
BootstrapRetrieval-->>StateRequest: return bootstrap state or failure
BootstrapRetrieval->>BootstrapRetrieval: schedule bounded retry
ConnectionLifecycle->>ConnectionLifecycle: increment generation and clear state
BootstrapRetrieval->>BootstrapRetrieval: reject stale dispatch
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95912603a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Core/MultiBotComm.lua`:
- Around line 468-471: Update Comm.RequestState and Comm.RequestStates so
refresh requests made before state.capabilitiesResolved are queued or retried
after capability negotiation instead of returning false and dropping them.
Ensure the pending requests are replayed once capabilities resolve, and preserve
normal immediate request behavior after resolution.
- Around line 522-540: Update the bootstrap request lifecycle around
requestBootstrapStates so state.bootstrapStateRequested is cleared when the
Comm.RequestStates operation ends with ERR, STATE_ABORT, or timeout, allowing
subsequent bootstrap attempts to retry state retrieval while preserving the
existing in-flight guard.
- Around line 1614-1615: Make capability fallback in Comm.OnPlayerEnteringWorld
generation-scoped and retryable: track the active world-entry/connection
generation, ignore delayed callbacks from older generations, and allow fallback
retries until the current peer’s CAPS or HELLO_ACK is received. Preserve a
TimerAfter-based path when available and add an equivalent retry mechanism when
MultiBot.TimerAfter is unavailable, ensuring RequestState and RequestStates are
not permanently blocked.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 21ac8331-8ab1-4301-ad68-d1ed32d83453
📒 Files selected for processing (1)
Core/MultiBotComm.lua
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Core/MultiBotComm.lua (2)
344-366: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider
safeDelayso the retry also runs withoutMultiBot.TimerAfter.If
MultiBot.TimerAfteris absent,scheduleBootstrapStateRetryreturns without scheduling, sobootstrapStatePendingstays set until the next disconnect or world entry.safeDelayalready provides an immediate-invocation fallback, and the callback guards plusSTATE_BOOTSTRAP_MAX_AUTO_ATTEMPTSkeep the retry bounded.♻️ Proposed change
- if not (MultiBot and type(MultiBot.TimerAfter) == "function") then - return - end - - MultiBot.TimerAfter(STATE_BOOTSTRAP_RETRY_SECONDS, function() + safeDelay(STATE_BOOTSTRAP_RETRY_SECONDS, function() local bridge = ensureBridgeState() if bridge.connectionGeneration ~= generation or not bridge.connected or not bridge.capabilitiesResolved or bridge.bootstrapStateRequested or not bridge.bootstrapStatePending then return end requestBootstrapStates() end)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Core/MultiBotComm.lua` around lines 344 - 366, Update scheduleBootstrapStateRetry to use the existing safeDelay helper instead of returning when MultiBot.TimerAfter is unavailable, passing the retry delay and current callback. Preserve the callback’s connection, capability, request, and pending-state guards so retries remain bounded by STATE_BOOTSTRAP_MAX_AUTO_ATTEMPTS.
642-682: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReturn after a failed bootstrap request and schedule a retry.
If
requestBootstrapStates()fails at line 649, control falls through to line 660 and callsComm.RequestStates()again in the same tick.requestBootstrapStatesalready attempted that send. The re-queued global refresh at line 664 also has no retry, so it waits for the next periodic refresh inCore/MultiBotHandler.lua.♻️ Proposed change
if state.bootstrapStatePending and not state.bootstrapStateRequested then if requestBootstrapStates() then state.pendingStateRefreshAll = false state.pendingStateRefreshByBot = {} return true end + scheduleBootstrapStateRetry(state.connectionGeneration) + return false elseif state.bootstrapStateRequested then🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Core/MultiBotComm.lua` around lines 642 - 682, Update flushPendingStateRefreshes so a failed requestBootstrapStates call immediately re-queues the global state refresh and returns false, rather than falling through to Comm.RequestStates in the same tick. Preserve the existing successful bootstrap handling and ensure the queued refresh is retried by the normal subsequent flush cycle.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Core/MultiBotComm.lua`:
- Around line 556-560: Ensure capability fallback is armed before resolution is
attempted when HELLO_ACK is missed. In Core/MultiBotComm.lua lines 556-560,
update Comm.RequestState to call
armCapabilityFallback(state.connectionGeneration) before
maybeResolveCapabilityFallback; make the same change in Comm.RequestStates at
lines 591-595, and add the required forward declaration for
armCapabilityFallback alongside the existing declarations near lines 35-37.
---
Nitpick comments:
In `@Core/MultiBotComm.lua`:
- Around line 344-366: Update scheduleBootstrapStateRetry to use the existing
safeDelay helper instead of returning when MultiBot.TimerAfter is unavailable,
passing the retry delay and current callback. Preserve the callback’s
connection, capability, request, and pending-state guards so retries remain
bounded by STATE_BOOTSTRAP_MAX_AUTO_ATTEMPTS.
- Around line 642-682: Update flushPendingStateRefreshes so a failed
requestBootstrapStates call immediately re-queues the global state refresh and
returns false, rather than falling through to Comm.RequestStates in the same
tick. Preserve the existing successful bootstrap handling and ensure the queued
refresh is retried by the normal subsequent flush cycle.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 16ad2c27-e59c-4cb7-bc0b-ccc0951fc01d
📒 Files selected for processing (1)
Core/MultiBotComm.lua
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary by CodeRabbit