Release dev → main: SKU table, JetKVM Mini product, redirect fix - #77
Merged
Merged
Conversation
…act error (#69) The /releases response carried appCachedAt, appMaxSatisfying and their system counterparts, resolver bookkeeping that no client reads. Drop them from the response type and from ReleaseMetadata, where nothing read them either. Three code paths reported a version with no artifact for the requested SKU with two different messages, one of them claiming the version predates SKU support even for versions that have a skus/ folder. Route all three through one helper with one message.
releases.ts and sync-releases.ts each carried their own copy of the SKU list, the artifact file names and the recovery-image mapping, and the /releases handler spelled out the app and system artifacts in four places. Add src/skus.ts: one row per SKU listing the artifacts a device with that SKU can get, each as <prefix>/<file>. The prefix is the R2 folder and the Release.type column. The handlers iterate a SKU's over-the-air artifacts instead of naming app and system, and the two redirects read the prefix and file for the requested SKU from the same table. Unknown SKUs are now rejected at the query schema with a 400. Before, they fell through to a 404 from the artifact lookup, or to the legacy path when the version predated the skus/ layout. On the rollout path the latest and default lookups run sequentially so that, when a prefix has no releases, the 404 from the latest lookup always wins; the parallel form left the 404-versus-500 choice to query timing. Responses for the registered SKUs are unchanged.
The local-versus-production comparison still expected the fields and messages #69 removed: - normalize_body dropped only *CachedAt keys, so any constrained stable request reported the removed *MaxSatisfying fields as a body mismatch and, with FAIL_FAST, aborted the run. Drop *MaxSatisfying too. - body_diff_is_version_only_not_found canonicalized the two older no-compatible-release messages only. Add the unified message, the S3 path wording production still emits, and the mini prefix in the no-default pattern. Both are comparison-only changes; the API is untouched.
The Mini (ESP32-P4) ships one firmware image, and that image is the whole system: FreeRTOS, drivers, the application, the web UI and the Wi-Fi co-processor firmware. There is no separate app. Register it as a product with one over-the-air artifact, system, served in the system fields of the /releases response from the new prefix mini/<version>/skus/<sku>/jetkvm-mini.bin. The app fields are omitted. Two variants, one per board, because the boards are separate builds: jetkvm-mini-ethernet and jetkvm-mini-wireless. Its own prefix, not Mini variants under system/, so Mini version numbers never enter the JetKVM latest-release selection and staged rollout, and the Mini rolls out on its own schedule. A mini version without the skus/ layout is not a release and is skipped by the sync script. /releases/app/latest rejects Mini variants: no app artifact. The recovery redirect rejects them until a full-flash image exists.
Devices send X-Device-SKU on the signaling connection alongside X-App-Version. Keep it with the live connection state, validated against the product registry so an unknown value is logged and dropped rather than advertised, and report it from GET /devices and GET /devices/:id. A device that sends no SKU is the original hardware. The connection tuple in activeConnections becomes a named struct so the new field does not extend a positional array.
The Mini's recovery image is a merged full-flash binary: bootloader, partition table, the firmware in the first OTA slot and erased OTA selection data, written with esptool or a browser flasher at offset 0. It is built with the firmware and shares its version, so it lives next to it: mini/<version>/skus/<sku>/jetkvm-mini-full.bin. The recovery redirect already reads the prefix and file name from the product table, so registering the artifact is the whole change. JetKVM redirects are unchanged.
The latest-artifact redirects fetched the whole object on every cache miss to compare it with its .sha256 sidecar. Several concurrent misses on staging pulled multiple recovery images at once, the instance was marked unhealthy and the requests failed with 504. The sidecar is written by the release script that uploads the file, and the sync script verifies hash and signature when it registers a release, so a second check at request time added nothing. Check that the object exists and redirect.
…76) The comparison only requested the live baseline versions for the two registered JetKVM SKUs, so the pre-skus/ layout, semver ranges and the SKU table's handling of other SKUs were never exercised. Add, per device and default SKU, a caret range and a "<stable" range on each version axis, which reach the newest older release. Add EXTRA_SKUS (both Mini variants and one unregistered SKU) on /releases and both redirects, accepting any pair of 4xx error responses for them: the SKU table and the code before it refuse those SKUs with different status and wording, and a 200 on either side still fails. The accepted-deviation check returns its reason so the report names the rule that fired.
adamshiervani
marked this pull request as ready for review
September 18, 2026 20:36
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fde6e3a. Configure here.
| latest.artifacts.length > 0 && | ||
| (await isDeviceEligibleForLatestRelease(latest.rolloutPercentage, deviceId)); | ||
|
|
||
| offered[kind] = dbReleaseToMetadata(useLatest ? latest : fallback, sku); |
There was a problem hiding this comment.
Mini OTA 500s without 100% release
High Severity
After the first Mini release is synced at the default 10% rollout, /releases fails with a 500 for every Mini SKU. getDefaultRelease requires a 100% row and is always awaited, so even devices inside the rollout window never receive the staged build. JetKVM is unaffected because it already has 100% app/system releases; Mini has no such baseline.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit fde6e3a. Configure here.
Contributor
Author
There was a problem hiding this comment.
Confirmed, and not Mini-specific: any prefix with no row at 100% answered 500. Fix in #78: the default lookup returns null when nothing is at 100%, in-bucket devices get the staged release, out-of-bucket devices get a 404.
adamshiervani
added a commit
that referenced
this pull request
Sep 18, 2026
…78) A prefix with no release at 100% made the default lookup throw a 500 before eligibility was checked, so no device got the staged build. That is every device for a new prefix until its first release is fully rolled out, and every JetKVM device the day no app or system row sits at 100%. The default lookup now returns null when nothing is at 100%. A device inside the rollout bucket gets the staged release as before; a device outside it gets a 404 saying no release is rolled out yet for its SKU, instead of a 500. Found by Bugbot on the release PR (#77).
adamshiervani
added a commit
that referenced
this pull request
Sep 18, 2026
…78) (#79) A prefix with no release at 100% made the default lookup throw a 500 before eligibility was checked, so no device got the staged build. That is every device for a new prefix until its first release is fully rolled out, and every JetKVM device the day no app or system row sits at 100%. The default lookup now returns null when nothing is at 100%. A device inside the rollout bucket gets the staged release as before; a device outside it gets a 404 saying no release is rolled out yet for its SKU, instead of a 500. Found by Bugbot on the release PR (#77).
adamshiervani
added a commit
that referenced
this pull request
Sep 19, 2026
* fix(releases): serve staged releases before any release reaches 100% (#78) A prefix with no release at 100% made the default lookup throw a 500 before eligibility was checked, so no device got the staged build. That is every device for a new prefix until its first release is fully rolled out, and every JetKVM device the day no app or system row sits at 100%. The default lookup now returns null when nothing is at 100%. A device inside the rollout bucket gets the staged release as before; a device outside it gets a 404 saying no release is rolled out yet for its SKU, instead of a 500. Found by Bugbot on the release PR (#77). * Releases: sync new R2 releases from the API every 30 minutes (#81) * feat(releases): sync new R2 releases from the API every 30 minutes The release sync only ran when an operator invoked scripts/sync-releases.ts by hand, so a firmware upload sat in R2 until someone remembered to run it. The API process now runs the same sync on start and every 30 minutes, registering each new stable version at the default 10% rollout. The non-interactive core (bucket listing, artifact collection, DB insert) moves from the script into src/release-sync.ts so the API can import it; the script keeps the GPG check and the confirmation prompt and passes them in as the release decider. Both entry points share one R2 client via src/s3.ts. Scheduled runs skip a tick while the previous run is still going, log and survive a failed run, and treat a unique-constraint hit on insert as "already synced" so several API instances can race without an error. The existence check now precedes the R2 artifact walk, so a tick over an already-synced bucket costs one list call per prefix plus one DB lookup per version. * fix(release-sync): defer versions whose upload is still settling A scheduled tick can land while a version is still being uploaded and register it with a partial SKU set or a hash that changes afterwards. Sync never rewrites a row, so that snapshot would be permanent. Before collecting artifacts, list every object under the version folder and skip the version when the newest object changed in the last 10 minutes. The window is shorter than the schedule interval, so a real upload costs at most one extra tick. * refactor(release-sync): share R2 config, paginate via SDK, probe SKUs concurrently * src/s3.ts now exports bucketName and baseUrl next to the client, so the request handlers, the scheduler and the script read R2 config from one place instead of three. * The upload settle window moves into SyncConfig.uploadSettleMs. The scheduler sets it; the one-shot operator script leaves it unset because an operator sees the artifact list before confirming and has no next run. * newestUploadTime uses the SDK's paginateListObjectsV2 instead of a hand-rolled continuation loop. * collectReleaseArtifacts probes SKUs concurrently and folds the results in SKU order, so artifact order and the primary artifact are unchanged. * Tests drop empty per-prefix listing stubs that the file-level default already covers. * fix(release-sync): take the settle listing after the artifact scan Listing the version folder before the scan left a gap: an upload that started between the listing and the scan passed the check and was captured partially. Listing after the scan closes it, because any upload active during the scan leaves an object newer than the window and the snapshot is discarded instead of registered. * fix(release-sync): paginate the version listing ListObjectsV2 returns at most 1,000 common prefixes per page. Once a release prefix grows past that, versions on later pages were never seen on any tick. listStableVersions now walks every page with the SDK paginator, as newestUploadTime already does, and the mock in the sync tests can serve a truncated listing. * Release sync: a unique violation is only a race when the row exists (#82) * fix(release-sync): only treat a unique violation as a race when the row exists Sync caught every P2002 from the release insert as "created concurrently elsewhere". On staging the id sequences were behind the rows after a data import, so each insert failed on the primary key, was logged as a race, and left nothing in the table. After a unique violation, createRelease now looks the (version, type) row up. Present means another instance registered it first; absent means the insert really failed, and the error is rethrown with the type and version in its message so the scheduled run log names the release. * fix(release-sync): name the release in every per-version failure Wrapping only the insert error left the row lookup, and the S3 scan before it, free to escape without the type and version. syncReleases now wraps whatever createRelease throws for a version. * feat(releases): POST /releases/sync for the upload script (#83) The scheduled tick found a new version at most 30 minutes plus the settle window after upload. The upload script knows when its last object is written, so it can trigger the sync itself. One runner now owns the in-progress flag and serves both the timer and the endpoint. The endpoint is guarded by RELEASE_SYNC_TOKEN as a bearer token, compared in constant time, and is not registered without it. It runs with the settle window off and answers with the per-outcome counts, or 409 while a run is in progress. * fix(auth): accept the bearer scheme in any case (#85) Scheme names are case-insensitive (RFC 9110). The token is still compared exactly. * fix(releases): scope the sync endpoint's settle bypass to one version (#84) The endpoint turned the settle window off for every version in the bucket. A call made while a different upload was still running registered that upload half done, and sync never revisits a row. The body now names the version the caller finished, and only that version skips the window. Without a body the call is a normal tick.
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.
Release #69 through #76 to production. Use a merge commit.
Device-facing contract
/releasesresponse carries the wire fields only;*CachedAtand*MaxSatisfyingare gone (Releases: wire fields only, one missing-artifact error #69). No client read them.skuis a 400Unknown SKUinstead of a 404 from the artifact lookup (Releases: one SKU and product registry #70).jetkvm-mini-ethernetandjetkvm-mini-wireless:/releasesreturnssystem*fields frommini/<version>/skus/<sku>/jetkvm-mini.bin, noapp*fields;/releases/app/latestis a 400 for them;/releases/system_recovery/latestresolvesmini/<version>/skus/<sku>/jetkvm-mini-full.bin(Releases: JetKVM Mini as a single-component product #71, Releases: recovery image per product #73). No Mini release exists yet, so these answer 404 until one is uploaded and synced./releases/app/latestand/releases/system_recovery/latestno longer download the artifact to verify its hash; they check the object exists and redirect (Releases: redirect without downloading the artifact #75). Under concurrent cache misses the old behaviour took staging down with504 no_healthy_upstream.Cloud
GET /devicesandGET /devices/:idreportskufrom theX-Device-SKUsignaling header; unknown values are dropped (Devices: report the hardware SKU a connected device sends #72).Internal
src/skus.tsdrives/releases, both redirects andsync-releases.ts(Releases: one SKU and product registry #70).Release.typevalues and R2 paths are unchanged; no migration.scripts/compare-releases.shfollows the new contract and covers ranges, older versions and the extra SKUs (Compare script: follow the #69 response contract #74, Compare script: cover ranges, older versions and extra SKUs #76).Validation
2dbc67band laterfde6e3a) against production: 216 cases, 206 identical, 10 accepted (the extra-SKU cases, where both sides refuse with a 4xx), 0 failures. The 24 range and older-version cases, which reach the pre-skus/layout, all pass.Note
High Risk
Changes core OTA/release resolution, redirect behavior, and SKU validation across device update and recovery paths; incorrect mapping could serve wrong firmware or break Mini rollout until releases exist in R2/DB.
Overview
Introduces a central SKU table (
src/skus.ts) that drives/releases, latest redirects, andsync-releases, including JetKVM Mini (jetkvm-mini-ethernet/jetkvm-mini-wireless) with amini/OTA prefix and noapp*fields in/releases.Device-facing API changes: unknown
skuquery values return 400Unknown SKUat validation time;/releasesJSON drops internal*CachedAt/*MaxSatisfyingfields and uses one 404 message when a version has no artifact for the SKU. Mini and partial-SKU products only include the OTA kinds they ship./releases/app/latestand/releases/system_recovery/lateststop downloading artifacts to verify hashes—they HEAD for existence and redirect (recovery paths come from the SKU table, including Mini full-flash undermini/).Cloud / signaling: WebSocket connections parse
X-Device-SKU;GET /devicesandGET /devices/:idexpose liveskuandversionfrom the active connection (default SKU when legacy hardware omits the header).Ops / tests:
sync-releasesiterates all OTA prefixes (app,system,mini) with prefix-aware legacy rules;compare-releases.shadds semver range cases,EXTRA_SKUStolerance for expected 4xx drift vs prod, and ignores resolver-only JSON fields.Reviewed by Cursor Bugbot for commit fde6e3a. Bugbot is set up for automated code reviews on this repo. Configure here.