refactor: realign runtime shell around upstream bootstrap data#4
Merged
bdtran2002 merged 20 commits intomainfrom Apr 22, 2026
Merged
refactor: realign runtime shell around upstream bootstrap data#4bdtran2002 merged 20 commits intomainfrom
bdtran2002 merged 20 commits intomainfrom
Conversation
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis refactoring simplifies the ForkOrFry runtime by removing Godot bridge snapshot polling, local bootstrap generation, and legacy burger-runtime scaffolding references. It introduces new state management helpers for bridge snapshot and session handling, reorganizes boot/resume logic into dedicated constructors, and updates documentation to establish Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extension/src/features/runtime-frame/upstream-runtime-state.ts`:
- Around line 102-116: In summarizeUpstreamRuntimeGameplayPackets, totalCount
currently uses packets.length which includes invalid entries; change it to count
only validated packets by introducing a validatedCount (or reuse actionCounts
size) that you increment inside the loop only when
isUpstreamRuntimeGameplayPacket(packet) is true, then return totalCount:
validatedCount (instead of packets.length) along with lastAction and
actionCounts so the summary reflects only validated gameplay packets.
- Around line 166-178: createResumeUpstreamRuntimeState currently preserves
restored.bridgeState when exportUrl is missing, which can rehydrate a stale
'error' state even though
resolveUpstreamRuntimeSessionState/restoreUpstreamBridgeSnapshotForSession drops
lastError; change the bridgeState assignment so that if restored.exportUrl is
truthy you set 'waiting' as before, otherwise normalize: if restored.bridgeState
=== 'error' set bridgeState to 'ready' else keep restored.bridgeState. Update
the bridgeState expression in createResumeUpstreamRuntimeState to use that
conditional (you can still use resolvedSession.bridgeSnapshot/resolvedSession
from resolveUpstreamRuntimeSessionState to locate the callsite).
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e30591cf-73fa-419e-bcda-cb3c1421e90d
⛔ Files ignored due to path filters (1)
extension/upstream/generated/burgers-inc-bootstrap.tsis excluded by!**/generated/**
📒 Files selected for processing (10)
AGENTS.mddocs/pivot-analysis.mddocs/runtime-host-branch-log.mdextension/src/features/runtime-frame/upstream-bridge.tsextension/src/features/runtime-frame/upstream-checkpoint.tsextension/src/features/runtime-frame/upstream-runtime-copy.tsextension/src/features/runtime-frame/upstream-runtime-state.tsextension/src/features/runtime-frame/upstream-runtime.tsextension/src/features/runtime-host/copy.tsextension/tests/upstream-runtime.test.ts
💤 Files with no reviewable changes (1)
- extension/src/features/runtime-frame/upstream-bridge.ts
Comment on lines
+102
to
+116
| export function summarizeUpstreamRuntimeGameplayPackets(packets: UpstreamRuntimeState['gameplayPackets']) { | ||
| const actionCounts: Record<string, number> = {} | ||
| let lastAction: string | null = null | ||
|
|
||
| for (const packet of packets) { | ||
| if (!isUpstreamRuntimeGameplayPacket(packet)) continue | ||
| actionCounts[packet.action] = (actionCounts[packet.action] ?? 0) + 1 | ||
| lastAction = packet.action | ||
| } | ||
|
|
||
| return { | ||
| totalCount: packets.length, | ||
| lastAction, | ||
| actionCounts, | ||
| } |
There was a problem hiding this comment.
Count only validated packets in the summary.
totalCount currently uses the raw array length even though invalid entries are skipped for actionCounts and lastAction. If restored state ever contains malformed packets, the summary becomes self-contradictory.
Proposed fix
export function summarizeUpstreamRuntimeGameplayPackets(packets: UpstreamRuntimeState['gameplayPackets']) {
const actionCounts: Record<string, number> = {}
let lastAction: string | null = null
+ let totalCount = 0
for (const packet of packets) {
if (!isUpstreamRuntimeGameplayPacket(packet)) continue
+ totalCount += 1
actionCounts[packet.action] = (actionCounts[packet.action] ?? 0) + 1
lastAction = packet.action
}
return {
- totalCount: packets.length,
+ totalCount,
lastAction,
actionCounts,
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function summarizeUpstreamRuntimeGameplayPackets(packets: UpstreamRuntimeState['gameplayPackets']) { | |
| const actionCounts: Record<string, number> = {} | |
| let lastAction: string | null = null | |
| for (const packet of packets) { | |
| if (!isUpstreamRuntimeGameplayPacket(packet)) continue | |
| actionCounts[packet.action] = (actionCounts[packet.action] ?? 0) + 1 | |
| lastAction = packet.action | |
| } | |
| return { | |
| totalCount: packets.length, | |
| lastAction, | |
| actionCounts, | |
| } | |
| export function summarizeUpstreamRuntimeGameplayPackets(packets: UpstreamRuntimeState['gameplayPackets']) { | |
| const actionCounts: Record<string, number> = {} | |
| let lastAction: string | null = null | |
| let totalCount = 0 | |
| for (const packet of packets) { | |
| if (!isUpstreamRuntimeGameplayPacket(packet)) continue | |
| totalCount += 1 | |
| actionCounts[packet.action] = (actionCounts[packet.action] ?? 0) + 1 | |
| lastAction = packet.action | |
| } | |
| return { | |
| totalCount, | |
| lastAction, | |
| actionCounts, | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extension/src/features/runtime-frame/upstream-runtime-state.ts` around lines
102 - 116, In summarizeUpstreamRuntimeGameplayPackets, totalCount currently uses
packets.length which includes invalid entries; change it to count only validated
packets by introducing a validatedCount (or reuse actionCounts size) that you
increment inside the loop only when isUpstreamRuntimeGameplayPacket(packet) is
true, then return totalCount: validatedCount (instead of packets.length) along
with lastAction and actionCounts so the summary reflects only validated gameplay
packets.
Comment on lines
+166
to
+178
| export function createResumeUpstreamRuntimeState(restored: UpstreamRuntimeState, fallbackSessionId: string): UpstreamRuntimeState { | ||
| const resolvedSession = resolveUpstreamRuntimeSessionState(restored.bridgeSnapshot, restored.sessionId || fallbackSessionId) | ||
|
|
||
| return { | ||
| ...restored, | ||
| sessionId: resolvedSession.sessionId, | ||
| phase: restored.exportUrl ? 'running' : 'ready', | ||
| bridgeState: (restored.exportUrl ? 'waiting' : restored.bridgeState), | ||
| bridgeSnapshot: resolvedSession.bridgeSnapshot, | ||
| detail: resolvedSession.reused | ||
| ? describeUpstreamRuntimeSession(resolvedSession.sessionId, true) | ||
| : restored.exportUrl ? 'Bundled Godot runtime loaded.' : 'Ready.', | ||
| } |
There was a problem hiding this comment.
Normalize bridgeState when resume clears bridge errors.
restoreUpstreamBridgeSnapshotForSession() always drops lastError, but this branch keeps restored.bridgeState unchanged when exportUrl is missing. A persisted 'error' state would then resume as phase: 'ready'/detail: 'Ready.' with bridgeState: 'error', which is internally inconsistent and can strand the host on an error path.
Proposed fix
export function createResumeUpstreamRuntimeState(restored: UpstreamRuntimeState, fallbackSessionId: string): UpstreamRuntimeState {
const resolvedSession = resolveUpstreamRuntimeSessionState(restored.bridgeSnapshot, restored.sessionId || fallbackSessionId)
+ const bridgeState =
+ restored.exportUrl
+ ? 'waiting'
+ : restored.bridgeState === 'error'
+ ? 'idle'
+ : restored.bridgeState
return {
...restored,
sessionId: resolvedSession.sessionId,
phase: restored.exportUrl ? 'running' : 'ready',
- bridgeState: (restored.exportUrl ? 'waiting' : restored.bridgeState),
+ bridgeState,
bridgeSnapshot: resolvedSession.bridgeSnapshot,
detail: resolvedSession.reused
? describeUpstreamRuntimeSession(resolvedSession.sessionId, true)
: restored.exportUrl ? 'Bundled Godot runtime loaded.' : 'Ready.',
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extension/src/features/runtime-frame/upstream-runtime-state.ts` around lines
166 - 178, createResumeUpstreamRuntimeState currently preserves
restored.bridgeState when exportUrl is missing, which can rehydrate a stale
'error' state even though
resolveUpstreamRuntimeSessionState/restoreUpstreamBridgeSnapshotForSession drops
lastError; change the bridgeState assignment so that if restored.exportUrl is
truthy you set 'waiting' as before, otherwise normalize: if restored.bridgeState
=== 'error' set bridgeState to 'ready' else keep restored.bridgeState. Update
the bridgeState expression in createResumeUpstreamRuntimeState to use that
conditional (you can still use resolvedSession.bridgeSnapshot/resolvedSession
from resolveUpstreamRuntimeSessionState to locate the callsite).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
Summary by CodeRabbit
New Features
Bug Fixes
Refactor