fix(updater): preserve bleeding-edge setup state - #218
Merged
Conversation
Owner
Author
|
Review complete for PR #218. I've submitted my review as a GitHub PR review. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesThe updater adds normalized bleeding-edge selection and persists it before building. GUI and non-GUI setup failures use centralized handling. Git synchronization now verifies remote branches, rebases local branches, and recovers failed synchronization by stashing and force-aligning. Updater flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GUI
participant main
participant selectAndBuildBleedingEdge
participant persist
participant build
participant setupErrorDialog
GUI->>main: choose bleeding-edge channel
main->>selectAndBuildBleedingEdge: submit normalized selection
selectAndBuildBleedingEdge->>persist: save selection
persist-->>selectAndBuildBleedingEdge: persistence result
selectAndBuildBleedingEdge->>build: build selection
build-->>selectAndBuildBleedingEdge: build result or error
selectAndBuildBleedingEdge-->>main: setup result
main->>setupErrorDialog: display setup error
sequenceDiagram
participant GitSync
participant fetchedRemoteBranch
participant targetBranch
participant stash
GitSync->>fetchedRemoteBranch: verify selected remote branch
GitSync->>targetBranch: checkout or create tracking branch
GitSync->>targetBranch: rebase onto fetched remote branch
targetBranch-->>GitSync: synchronization result
GitSync->>stash: stash tracked and untracked files after failure
GitSync->>targetBranch: force-align with fetched remote branch
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
nat-openclaw
force-pushed
the
fix/updater-bleeding-edge-flow
branch
from
August 5, 2026 04:08
9bfdfa0 to
0072b32
Compare
Owner
Author
Review by @nat-openclawSummaryThe updater changes look solid: they fix the silent launch-on-failure bug, persist bleeding-edge selection before the build starts, and replace blunt force-checkout with a sensible rebase-then-recover flow. Integration tests cover the main git-sync paths. Verdict0 critical, 0 important, 0 nits What Looks Good
Findings (0)No findings. |
Owner
Author
|
I hit an infra error before I could finish: React to this comment with 🚀 to retry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@updater/src/git-sync.ts`:
- Around line 72-120: The rebase recovery branch currently also handles failed
branch creation, causing force-alignment to run when the remote target branch
may not exist. Update the checkout/rebase flow around checkoutExisting,
checkoutResult, and rebaseResult so recovery and stash/force-align execute only
when an actual rebase fails; preserve and return the checkout error when both
checkout attempts fail.
In `@updater/src/main.ts`:
- Around line 1137-1140: Update the build failure branch in the createWindow
flow around showBleedingEdgeSetupError so a failed GUI setup re-arms the
choose-channel IPC listener before showing the channel picker. Preserve the
existing error display and return behavior, while ensuring the retry choice
starts the build flow again.
- Around line 1123-1126: Update the selection construction so the branch value
is trimmed before fallback evaluation, ensuring whitespace-only input uses
DEFAULT_BLEEDING_EDGE_BRANCH instead of persisting an empty string; preserve the
existing commit normalization and selection behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2adb889d-d1b5-4586-aa5f-5aea957f6bed
📒 Files selected for processing (14)
updater/src/bleeding-edge-flow.tsupdater/src/git-sync.tsupdater/src/main.tsupdater/test/bleeding-edge-flow.test.tsupdater/test/git-sync.test.tsweb/community/fatboy-unpack.mdweb/community/gemini-search.mdweb/community/steam-integration.mdweb/community/steamrip-addon.mdweb/community/template.mdweb/src/lib/community-addons.tsweb/src/pages/api/community.json.tsweb/src/pages/community.astroweb/src/pages/docs/for-users/community.md
💤 Files with no reviewable changes (5)
- web/community/steamrip-addon.md
- web/community/gemini-search.md
- web/community/steam-integration.md
- web/community/template.md
- web/community/fatboy-unpack.md
Comment on lines
+72
to
+120
| const checkoutExisting = yield* Effect.either( | ||
| runCommand('git', ['checkout', targetBranch], { cwd: repoDir }) | ||
| ); | ||
| const checkoutResult = | ||
| checkoutExisting._tag === 'Right' | ||
| ? checkoutExisting | ||
| : yield* Effect.either( | ||
| runCommand( | ||
| 'git', | ||
| ['checkout', '--track', '-b', targetBranch, remoteBranch], | ||
| { cwd: repoDir } | ||
| ) | ||
| ); | ||
| const rebaseResult = | ||
| checkoutResult._tag === 'Right' | ||
| ? yield* Effect.either( | ||
| runCommand('git', ['rebase', remoteBranch], { cwd: repoDir }) | ||
| ) | ||
| : checkoutResult; | ||
|
|
||
| if (rebaseResult._tag === 'Left') { | ||
| yield* runCommand('git', ['rebase', '--abort'], { | ||
| cwd: repoDir, | ||
| }).pipe(Effect.ignore); | ||
| yield* withOperation( | ||
| 'stash', | ||
| runCommand( | ||
| 'git', | ||
| [ | ||
| 'stash', | ||
| 'push', | ||
| '--include-untracked', | ||
| '-m', | ||
| 'OpenGameInstaller updater recovery', | ||
| ], | ||
| { cwd: repoDir } | ||
| ) | ||
| ); | ||
| // Rebase could not safely preserve the cache. Keep dirty files in the | ||
| // stash, then force-align the updater branch to the fetched remote. | ||
| yield* withOperation( | ||
| 'checkout', | ||
| runCommand( | ||
| 'git', | ||
| ['checkout', '--force', '-B', targetBranch, remoteBranch], | ||
| { cwd: repoDir } | ||
| ) | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect branch selection and all synchronization callers.
ast-grep outline updater/src --items all --type function --match 'syncBleedingEdgeRepo'
rg -n -C 5 '\bsyncBleedingEdgeRepo\s*\(|\btargetBranch\b|\bdefaultBranch\b' updater/src updater/test
# Inspect existing checkout/rebase failure coverage.
rg -n -C 5 "checkout.*--track|rebase.*origin|missing.*branch|stash.*force" updater/test/git-sync.test.tsRepository: Nat3z/OpenGameInstaller
Length of output: 13511
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== updater/src/git-sync.ts =="
sed -n '40,135p' updater/src/git-sync.ts
echo
echo "== updater/test/git-sync.test.ts recovery tests =="
sed -n '120,225p' updater/test/git-sync.test.ts
echo
echo "== git commands referenced in relevant source/test =="
python3 - <<'PY'
from pathlib import Path
for p in [Path('updater/src/git-sync.ts'), Path('updater/test/git-sync.test.ts')]:
print(f'-- {p} --')
for i,line in enumerate(p.read_text().splitlines(),1):
if any(x in line for x in ['runCommand(', 'git', 'checkout', 'rebase', 'stash']):
print(f'{i}: {line}')
PYRepository: Nat3z/OpenGameInstaller
Length of output: 9960
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== All branch-sync tests in updater/test/git-sync.test.ts =="
python3 - <<'PY'
from pathlib import Path
text = Path('updater/test/git-sync.test.ts').read_text().splitlines()
in_test = False
start = None
for i,line in enumerate(text,1):
if line.startswith('test(') or line.startswith('describe('):
if in_test and start:
print(f'{start}-{i-1}: {text[start-1]}')
print(f'{i}: {line}')
in_test = True
start = i
elif in_test and start and (line.startswith('});') and text[max(1,i-4):i]) <= 10):
pass
if in_test and start:
print(f'{start}-{len(text)}: {text[start-1]}')
PY
echo
echo "== Existing "missing branch"/"removed branch"/"origin/main" missing scenarios =="
rg -n -C 4 "missing|removed|deleted|no such|fatal|origin/main|origin/.*/'" updater/test/git-sync.test.ts
echo
echo "== runCommand failure handling =="
sed -n '20,55p' updater/test/git-sync.test.ts
echo
echo "== Deterministic path outline for current branch selection code =="
python3 - <<'PY'
paths = []
for checkout_existing_tag in ["Left", "Right"]:
paths.append({"initial_existing_checkout": checkout_existing_tag})
if checkout_existing_tag == "Left":
paths[-1]["new_checkout_tag"] = "Left"
paths[-1].update({"rebase_result_tag": "Left", "path": "stash abort, stash push, checkout --force -B"})
paths.append({"initial_existing_checkout": checkout_existing_tag})
paths[-1]["new_checkout_tag"] = "Right"
paths[-1].update({"rebase_result_tag": "Right", "path": "no stash/force-align"})
else:
paths.append({"initial_existing_checkout": checkout_existing_tag})
paths[-1]["new_checkout_tag"] = "Right"
paths[-1].update({"rebase_result_tag": "Left", "path": "stash abort, stash push, checkout --force -B"})
paths.append({"initial_existing_checkout": checkout_existing_tag})
paths[-1]["new_checkout_tag"] = "Left"
paths[-1].update({"rebase_result_tag": "Left", "path": "checkout --track failed -> checkoutResult = Left; rebaseResult = checkoutResult -> stash abort, stash push, checkout --force -B"})
for p in paths:
print(p)
PYRepository: Nat3z/OpenGameInstaller
Length of output: 448
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== updater/test/git-sync.test.ts line count and tests summary =="
wc -l updater/test/git-sync.test.ts
python3 - <<'PY'
from pathlib import Path
lines = Path('updater/test/git-sync.test.ts').read_text().splitlines()
for i,line in enumerate(lines,1):
if line.startswith('test('):
print(f'{i}: {line[:180]}')
PY
echo
echo "== Exact branch-related tests around current suite =="
sed -n '60,195p' updater/test/git-sync.test.ts
echo
echo "== Failure handling in runCommand helper =="
sed -n '30,60p' updater/test/git-sync.test.ts
echo
echo "== Static path analysis for current code shape =="
python3 - <<'PY'
cases = []
for checkout_existing in ["Left", "Right"]:
if checkout_existing == "Left":
checkout_track = "Left"
cases.append({
"checkout_existing": checkout_existing,
"checkout_track": checkout_track,
"rebase_result": checkout_track,
"stash_abort": True,
"stash_push": True,
"force_checkout": checkout_track,
"path": "checkout --track targetBranch refs/remotes/origin/targetBranch fails -> stash abort, stash push, checkout --force -B targetBranch refs/remotes/origin/targetBranch fails",
})
cases.append({
"checkout_existing": checkout_existing,
"checkout_track": "Right",
"rebase_result": "Right",
"stash_abort": False,
"stash_push": False,
"force_checkout": False,
"path": "checkout existing fails; checkout --track succeeds; no rebase -> no recovery",
})
else:
cases.append({
"checkout_existing": checkout_existing,
"checkout_track": "Right",
"rebase_result": "Left",
"stash_abort": True,
"stash_push": True,
"force_checkout": "Right",
"path": "checkout existing succeeds; rebase fails -> stash abort, stash push, checkout --force -B targetBranch refs/remotes/origin/targetBranch",
})
cases.append({
"checkout_existing": checkout_existing,
"checkout_track": "Left",
"rebase_result": "Left",
"stash_abort": True,
"stash_push": True,
"force_checkout": checkout_track,
"path": "checkout existing succeeds; rebase not attempted because branch cannot start?; rebaseResult = Left? path = stash abort, stash push, checkout --force -B targetBranch refs/remotes/origin/targetBranch fails",
})
for c in cases:
print(c)
PYRepository: Nat3z/OpenGameInstaller
Length of output: 7582
Separate checkout failure from rebase recovery.
rebaseResult can hold a failed checkout --track result. The rebaseResult._tag === 'Left' path then stashes dirty files and attempts checkout --force -B targetBranch remoteBranch, which also fails when origin/${targetBranch} is missing. Preserve the checkout error for return, or start force-align only after the rebase itself fails.
🤖 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 `@updater/src/git-sync.ts` around lines 72 - 120, The rebase recovery branch
currently also handles failed branch creation, causing force-alignment to run
when the remote target branch may not exist. Update the checkout/rebase flow
around checkoutExisting, checkoutResult, and rebaseResult so recovery and
stash/force-align execute only when an actual rebase fails; preserve and return
the checkout error when both checkout attempts fail.
Greptile SummaryThe PR preserves the selected bleeding-edge target before setup, keeps the GUI channel picker active after setup failures, and changes cached-repository synchronization to attempt a rebase before recovery.
Confidence Score: 5/5The PR appears safe to merge. The previously reported retry failure is resolved because each loop iteration registers a new one-shot channel listener before redisplaying the picker, and no blocking failure remains.
|
| Filename | Overview |
|---|---|
| updater/src/main.ts | Reworks GUI channel selection into a retry loop that registers a fresh listener before each picker display, persists bleeding-edge targets before building, and stops fallback launches after setup failures. |
| updater/src/bleeding-edge-flow.ts | Introduces selection normalization and an Effect-based persist-before-build sequence. |
| updater/src/git-sync.ts | Replaces unconditional branch reset with remote verification, checkout and rebase, followed by stash-and-force recovery on failure. |
| updater/test/bleeding-edge-flow.test.ts | Verifies default-branch normalization and persistence ordering when a build fails. |
| updater/test/git-sync.test.ts | Adds coverage for successful rebases, dirty-cache recovery, missing remotes, and detached-cache fallback. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Show channel picker] --> B[Wait for channel choice]
B --> C{Selected channel}
C -->|Stable or unstable| D[Persist channel state]
D --> E[Continue updater flow]
C -->|Bleeding edge| F[Persist branch and commit]
F --> G[Build selected target]
G -->|Success| H[Launch application]
G -->|Failure| I[Show native error]
I --> A
Reviews (2): Last reviewed commit: "fix(updater): address review feedback" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Owner
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
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
2 participants
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.
Description
Persist bleeding-edge branch and commit selection before setup begins, surface setup failures in a native popup, and stop silently launching the previous stable build. Cached repositories now rebase onto the fetched branch first, then stash dirty files and force-align only when rebase cannot complete.
Example
Next Steps
Summary by CodeRabbit
New Features
Bug Fixes