Repository navigation
test(usage): make the shared account order between the usage checks deterministic - #2345
Merged
Merged
Conversation
…eterministic checkUsageMeters moves an account, and the app saves the shared account order (usageOrder) to the server once the settings rest. Whether that save landed before the check closed its page was a race. When it did, checkUsageSignInAgain on the same server inherited the order: its expired report then listed Antigravity first, and the notes assertion failed. The release PR #2337 and #2328 hit it in CI. checkUsageMeters now waits for that PUT /api/preferences response (the one carrying usageOrder) before its page closes, so the save has always landed, and checkUsageSignInAgain clears the shared order before it opens a page. With the wait and without the reset, the sign-in check fails every time; with both it passes every time.
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Preference save and reset scripts/usage-regression.ts |
checkUsageMeters waits for the usageOrder preference save and asserts a 200 response. checkUsageSignInAgain clears usageOrder and asserts a 200 response before testing sign-in behavior. |
Priority:
Estimated code review effort: 2 (Simple) | ~8 minutes
Change: Other
Possibly related PRs
- devswha/ovis#2309: Adds the sign-in-again check that this change synchronizes with the usage-order preference.
Suggested reviewers: nahwan-kim
Merge Risk: 
67fd6
-
The synchronization changes are ready to merge with no established blocking risk.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly describes the main test change: making shared account order deterministic between usage checks. Description check The description includes the required Change and Validation sections. It explains the race condition, the resulting behavior, the affected checks, and the validation results with specific commands and… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description includes the required Change and Validation sections. It explains the race condition, the resulting behavior, the affected checks, and the validation results with specific commands and outcomes.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 1 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
Owner
Author
|
Merging 67fd6eb (test-only). Ends the race in |
devswha
added a commit
that referenced
this pull request
Oct 29, 2026
## Release 0.4.20 The release PR, prepared by step 1 of [docs/development.md#releasing](https://github.com/devswha/ovis/blob/main/docs/development.md#releasing), initially on main `bd676637a16471e8744d62c7311026ddb43a39eb` (#2335, xterm 6), then merged with final-scope main `ef482f2e66b3344658fb548fa1110744a92929c9` (#2338) and test-only follow-up main `269e9ce19558b8d197959250075da56b25b9668f` (#2345): - **Version:** `version` is 0.4.20 in `package.json` and `herdr-plugin.toml`. - **Changelog:** `bun scripts/changelog-fold.ts 0.4.20` initially folded 122 fragments into the 2026-10-29 section of `CHANGELOG.md`, linked their PRs, and deleted the fragments. After merging main, the modified `letta-native-chat.md` from #2338 was folded by hand with its current wording and PR link. No other late fragment landed; `changelog.d` holds only `README.md`. Four additional entries for external PRs without their own fragments bring the section to 157 entries. - Added 22 `by @login` credits across all 19 outside-contributor PRs in the release range. Preserved the existing credit to @Yoonwoo-Ha for #1634. See the complete ancestry-based audit below. - **Release summaries:** en, ko, ja and zh each have 5 new, 5 improved and 5 fixed lines, matching 0.4.19's shape. They include the xterm 6 improvement: terminal frames no longer mix old and new output. Every English line is at most 80 characters. - **Remote bundle:** `REMOTE_BUNDLE_VERSION` rises from 38 to 39 because the bundle inputs changed since remote-v38. - **Telemetry:** the guard confirms no Worker deploy or deployed fingerprint update is needed. - **iOS upload note:** CURRENT_PROJECT_VERSION raised to 3 for App and OvisNotifications (last uploaded: 2, source: `docs/native-app.md:99`, commit `54fdadc1c65da5d50a6afef411e3ed8f23b6c255`). Both targets inherit project-level Debug and Release settings, and every `CURRENT_PROJECT_VERSION` entry is 3. `app/store/ios.md` now records the prepared build. This is the last documented upload, not a live App Store Connect query; check for later uploads before uploading. The marketing version remains 1.0. - The changelog explicitly distinguishes delivery: removing WebKit's form bar (#2109) requires the new iOS app build, while Korean composition (#2307) comes with the PC update and needs no new iOS build. The Korean-input summary says the same in all four languages, with no change to list counts. ## Validation - Final `dori heavy release-check -- bun run check fast`: passed, exit 0; workflow syntax, generated types/themes, typecheck, build and unit suite (8,591 pass, 0 fail, 34 platform skips in 549 files). This is the merged tree containing #2338, #2345, the contributor corrections and iOS build 3. - `bun test scripts/changelog-fold.test.ts server/release-notes.test.ts`: 33 pass, 0 fail. - `bun scripts/release-notes.ts 0.4.20`: passed, with all patch-note lists and the full changelog. - `bun scripts/release-guards.ts` and CI mode with `--base ef482f2`: passed on the final merged tree: "Release 0.4.20: remote bundle, telemetry Worker and release metadata are in order." - `bun scripts/release-guards.ts --release 0.4.20`: expected exit 1 only because "remote-v39 is not published." The merge pass publishes it after merging. ## Merge pass **Do not merge before the owner's OK.** - [ ] Merge only after the owner's OK and required checks pass. - [ ] After merging, push `remote-v39` on the merged commit and wait for the **Remote PC bundles** workflow to publish the manifest and all archives. - [ ] Then run **Release** on `main` with version `0.4.20`, without `v`. The Release workflow creates the app tag; do not push it by hand. - [ ] From this PR's merge until the Release run is done, merge nothing touching `server/`, `shared/`, `bun.lock` or `patches/`. - [ ] A changelog fragment merged after this PR must be moved into 0.4.20 by hand. This lane stops at the open PR. No tag, workflow dispatch, merge or telemetry deploy was performed. **Completed release follow-up:** merged main `ef482f2e` including #2338 (`fix/letta-no-live-box`), folded its revised fragment into 0.4.20, and retained the contributor-credit commit `184dc120` and iOS-build commit `0ffa3d45`. The Letta summary already describes conversation support without promising a live-terminal box, so it needs no change. Merged test-only #2345 from main `269e9ce1`: the two usage browser checks now wait for the earlier shared account-order save and start the sign-in check from a cleared order. No product behavior or late fragment was added; `changelog.d` still contains only `README.md`. The iOS delivery clarification and resolved review thread are retained. ## Complete external-contributor audit Checked every merged PR whose merge commit is in `git log v0.4.19..origin/main`: 133 PRs through final-scope main `ef482f2e`, or 134 including test-only #2345 on `269e9ce1`, with 19 PRs by 12 external non-bot authors. Both rescans found no additional external-author gaps; #2345 is by the maintainer. Added the four missing entries: #2271 and #2274 by @CharleoYang, #2290 by @DravenDou, and #2282 by @fivetaku. The tab-reopening summary now includes the history-page improvement in all four languages; list counts remain 5/5/5. Full external PR list checked: #1911 @jjsteffen; #2068 @YuuuXie; #2072 @Noobear; #2087 @JJLiebig; #2145 @mattmarcin; #2176 @jpcarranza94; #2231 @philipprass-prog; #2235 @vuongnmhvn; #2270, #2271, #2274 @CharleoYang; #2278 @jpcarranza94; #2282 @fivetaku; #2283, #2292, #2293 @YuuuXie; #2301 @jpcarranza94; #2321 @kilhyeonjun. Every PR has a linked `by @login` credit in 0.4.20. --------- Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
aNNdii
added a commit
to aNNdii/herdr-web-ui
that referenced
this pull request
Oct 29, 2026
chore(sync): merge upstream/main (36 commits up to devswha#2345) into next
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
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.
Change
Test-only,
scripts/usage-regression.ts:checkUsageMeterswaits for its shared account order to reach the server before its page closes. It watches for thePUT /api/preferencesresponse that carriesusageOrder(set up before the Settings edits, 10 s bound) and asserts it is 200.checkUsageSignInAgainfirst clears the shared account order on its server (PUT /api/preferenceswithusageOrder: [], checked for 200), so it no longer depends on what an earlier check saved.Why
scripts/ui-contexts-regression.tsrunscheckUsageMetersand thencheckUsageSignInAgainagainst one server. The first check moves an account, and the app saves the sharedusageOrderto the server once the settings rest (600 ms debounce). Whether that save landed before the check closed its page was a race. When it did, the second check's expired report sortedclaude:u1andantigravity@keychainfirst, because they are the ranked keys, and the notes assertion atusage-regression.ts:229failed:The release PR #2337 fails Browser regression (ui-regression) on it, and so does #2328 (run 40835368558). The flaky check is on main itself, not something either PR changed.
Can the earlier save land after the reset?
No.
checkUsageMeterswaits for the response to the save that carriesusageOrder, so by the time its page closes, the server has stored that order and answered. Nothing pending can carry it later: the sync sends a setting only while it differs from what the server last confirmed, and both contexts are closed beforecheckUsageSignInAgainstarts. The reset therefore always comes after the save, and nothing incheckUsageSignInAgainwrites the order. No fixed sleep: the wait is for that exact response.Validation
bun scripts/usage-regression.tsfailed 3 of 3 runs with exactly the CI diff.origin/mainthe same way.bun scripts/ui-contexts-regression.tspasses (both usage checks PASS).bun run check fastis green, includingscripts/conventions.test.ts(no fixed sleeps).Both callers of the check (
ui-contexts-checks.tsandusage-regression.ts's own run) use servers without a token, so the reset needs no credential.