fix: skip runtime version check without source repo#142
Conversation
There was a problem hiding this comment.
Code Review
This pull request wraps the call to validateAstrbotRuntimeVersion in an if (needsSourceRepo) condition within scripts/prepare-resources.mjs. There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Since
validateAstrbotRuntimeVersionis now conditional onneedsSourceRepo, consider moving that check inside the helper itself (or adding a wrapper) so callers don’t need to remember to guard the call and you avoid future regressions in other call sites. - The
needsSourceRepo-dependent behavior is now split between the validation block and thedesktopVersionOverride && needsSourceRepoblock; consider grouping or extracting these mode-specific paths to make the control flow for “no source repo” vs “source repo” modes easier to follow.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since `validateAstrbotRuntimeVersion` is now conditional on `needsSourceRepo`, consider moving that check inside the helper itself (or adding a wrapper) so callers don’t need to remember to guard the call and you avoid future regressions in other call sites.
- The `needsSourceRepo`-dependent behavior is now split between the validation block and the `desktopVersionOverride && needsSourceRepo` block; consider grouping or extracting these mode-specific paths to make the control flow for “no source repo” vs “source repo” modes easier to follow.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new
resolveAstrbotVersionForSynchelper takes a large flat parameter list; consider passing the existing context object or a smaller config object instead so future changes to inputs stay in sync withcreatePrepareResourcesContextand the call site more easily.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `resolveAstrbotVersionForSync` helper takes a large flat parameter list; consider passing the existing context object or a smaller config object instead so future changes to inputs stay in sync with `createPrepareResourcesContext` and the call site more easily.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider deriving
needsSourceRepoinsideresolveAstrbotVersionForSyncfromcontext.modeandcontext.desktopVersionOverrideso that the decision logic lives in a single place and can’t drift between the caller and helper. - Since
resolveAstrbotVersionForSyncis now responsible for source repo interaction and version validation, you might also move theensureStartupShellAssetscall into it to keep all sync-related side effects consolidated in one function.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider deriving `needsSourceRepo` inside `resolveAstrbotVersionForSync` from `context.mode` and `context.desktopVersionOverride` so that the decision logic lives in a single place and can’t drift between the caller and helper.
- Since `resolveAstrbotVersionForSync` is now responsible for source repo interaction and version validation, you might also move the `ensureStartupShellAssets` call into it to keep all sync-related side effects consolidated in one function.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new
resolveAstrbotVersionForSynchelper both resolves the version and performs several side effects (startup assets, repo sync, runtime validation), so consider either renaming it to reflect its broader responsibilities or splitting the side effects from the pure version-resolution logic for clearer intent and easier reuse. - Instead of passing the entire
contextobject intoresolveAstrbotVersionForSync, consider passing only the specific properties it needs (mode, projectRoot, sourceDir, etc.) to make the function’s dependencies more explicit and reduce coupling to the context shape.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `resolveAstrbotVersionForSync` helper both resolves the version and performs several side effects (startup assets, repo sync, runtime validation), so consider either renaming it to reflect its broader responsibilities or splitting the side effects from the pure version-resolution logic for clearer intent and easier reuse.
- Instead of passing the entire `context` object into `resolveAstrbotVersionForSync`, consider passing only the specific properties it needs (mode, projectRoot, sourceDir, etc.) to make the function’s dependencies more explicit and reduce coupling to the context shape.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary
Why
Scheduled Build Desktop Tauri runs fail in the Sync Repository Version job after resolving v4.26.1 because version-only mode skips vendor/AstrBot checkout but still tries to read vendor/AstrBot/astrbot/core/config/default.py.
Validation
Co-Authored-By: Warp agent@warp.dev
Summary by Sourcery
Skip AstrBot runtime version validation when version-only desktop sync runs without a checked-out source repository and reuse the resolved version for desktop asset syncing.
Bug Fixes:
Enhancements: