🐛 (clear-signing-tester) [NO-ISSUE]: Skip pre-release OS versions in auto-resolution#1395
🐛 (clear-signing-tester) [NO-ISSUE]: Skip pre-release OS versions in auto-resolution#1395lcastillo-ledger wants to merge 1 commit intodevelopfrom
Conversation
…auto-resolution The app version resolver now filters out pre-release OS versions (e.g. 1.6.0-rc1) when auto-resolving the latest version, preventing selection of coin-apps requiring SDK api_levels not yet supported by the bundled Speculos. Falls back to pre-releases only if no stable versions exist. Explicitly requested versions via --os-version are unaffected. Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
There was a problem hiding this comment.
Pull request overview
This PR updates the clear-signing-tester’s OS auto-resolution behavior to prefer stable OS directory versions over pre-releases (e.g., -rcN) when selecting a default OS version from coin-apps.
Changes:
- Filter OS version directories to prefer stable SemVer versions (non-prerelease) during version discovery.
- Add debug/warn logging around skipped pre-release selection and fallback behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const stableVersions = allVersions.filter( | ||
| (v) => semver.valid(v) && semver.prerelease(v) === null, | ||
| ); | ||
|
|
||
| if (stableVersions.length > 0) { | ||
| const skipped = allVersions.length - stableVersions.length; | ||
| if (skipped > 0) { | ||
| this.logger.debug( | ||
| `Filtered out ${skipped} pre-release OS version(s) from auto-resolution`, | ||
| ); | ||
| } | ||
| return stableVersions; | ||
| } | ||
|
|
||
| this.logger.warn( | ||
| "No stable OS versions found, falling back to all versions including pre-releases", | ||
| ); | ||
| return allVersions; |
There was a problem hiding this comment.
The “fallback to pre-releases if no stable versions exist” logic here won’t take effect in auto-resolution: resolve() still calls getStableOsVersions(availableOsVersions) for the auto OS paths, which will filter out all pre-releases again (and can leave an empty list when only RCs exist). Consider moving the stable-vs-prerelease preference/fallback into getStableOsVersions (return stable if any, else return all), or stop filtering in resolve() and let the downstream selection logic handle the fallback.
| const stableVersions = allVersions.filter( | |
| (v) => semver.valid(v) && semver.prerelease(v) === null, | |
| ); | |
| if (stableVersions.length > 0) { | |
| const skipped = allVersions.length - stableVersions.length; | |
| if (skipped > 0) { | |
| this.logger.debug( | |
| `Filtered out ${skipped} pre-release OS version(s) from auto-resolution`, | |
| ); | |
| } | |
| return stableVersions; | |
| } | |
| this.logger.warn( | |
| "No stable OS versions found, falling back to all versions including pre-releases", | |
| ); | |
| return allVersions; | |
| const semverVersions = allVersions.filter((v) => semver.valid(v)); | |
| const skipped = allVersions.length - semverVersions.length; | |
| if (skipped > 0) { | |
| this.logger.debug( | |
| `Filtered out ${skipped} non-semver OS version(s) from auto-resolution`, | |
| ); | |
| } | |
| if (semverVersions.length === 0 && allVersions.length > 0) { | |
| this.logger.warn( | |
| "No semver-valid OS versions found in device directory; auto-resolution may fail", | |
| ); | |
| } | |
| return semverVersions; |
| const skipped = allVersions.length - stableVersions.length; | ||
| if (skipped > 0) { | ||
| this.logger.debug( | ||
| `Filtered out ${skipped} pre-release OS version(s) from auto-resolution`, |
There was a problem hiding this comment.
The debug message/skip count is misleading: stableVersions excludes both pre-releases and any non-semver directory names (because of semver.valid(v)), so skipped can include more than just pre-release versions. Either compute/log pre-release vs invalid counts separately, or adjust the filter so the message matches what’s actually being filtered out.
| const skipped = allVersions.length - stableVersions.length; | |
| if (skipped > 0) { | |
| this.logger.debug( | |
| `Filtered out ${skipped} pre-release OS version(s) from auto-resolution`, | |
| const nonStableVersions = allVersions.filter( | |
| (v) => !semver.valid(v) || semver.prerelease(v) !== null, | |
| ); | |
| const skipped = nonStableVersions.length; | |
| if (skipped > 0) { | |
| this.logger.debug( | |
| `Filtered out ${skipped} non-stable OS version(s) (pre-release or invalid semver directory names) from auto-resolution`, |
Danger Check ResultsFailures
Messages
LogDetails |
Summary
1.6.0-rc1,1.4.0-rc3) from theAppVersionResolverServiceauto-resolution, so the latest stable OS is selected by defaultledger-app-dev-tools:latest--os-versionare unaffectedContext
The
coin-appsrepository contains both stable and pre-release OS directories (e.g.flex/1.6.0-rc1/). The auto-resolver was picking1.6.0-rc1(api_level 26) over the stable1.5.1(api_level 25), causing Speculos to fail withinvalid SDK api_level: 26since the bundled Speculos only supports up to api_level 25.Test plan
flex/1.5.1instead offlex/1.6.0-rc1--os-version 1.6.0-rc1still works when intentionally requestedMade with Cursor