Fix two races breaking CI on main - #887
Merged
Merged
Conversation
Both were ~2% flakes, so they broke main intermittently rather than outright: run 30406401925 (#884) failed Tests (app) and run 30403074723 (#883) failed Tests (packages). useResourcePagination mirrored the derived page back into state from an effect. That write-back captured the pre-interaction page, so when a viewport measurement changed pageSize, the queued effect flushed after a click and overwrote it. A user clicking Next in the instant after a list first measures its row height had the click silently dropped, which is what PluginsOverview's pagination test caught. State is now an anchor -- the page plus the page size it was chosen under -- written only by setPage or a new projection, with the rendered page derived from it. The projection reset moved from an effect to a render-time adjustment so it cannot land after an interaction. The anchor also survives page-size changes, so remeasuring back returns the user to the page they picked instead of stranding them on the first one. The workflows CLI test walks the whole checkout while the packages CI shard runs every other package concurrently, so it read packages/plugin-registry/.vendor-fixture-*/app.tsx between the plugin registry test creating and deleting it. The walk now skips dot-directories -- covering VCS internals, tool caches, and the scratch trees siblings create in their own package roots -- and tolerates a path deleted mid-walk. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
CI on
mainwas failing intermittently from two independent races. Both are ~2% flakes, so they broke main sporadically rather than outright — main run 30406401925 (#884) failedTests (app), and run 30403074723 (#883) failedTests (packages).1.
useResourcePaginationdropped clicksuseResourcePaginationmirrored the derivedpageback into state from auseEffect. That write-back captured the pre-interaction page, so when a viewport measurement changedpageSize, the queued effect flushed after a click and overwrote it:pageSize10 → 15. That commit renderspage=0atpageSize=15while the storedpageSizeis still10, and schedules the write-back withpage=0captured.setPage(1)writes{page: 1, pageSize: 15}.current.page (1) !== page (0)and resets state to{page: 0, pageSize: 15}— discarding the click.Instrumenting
setPageprinted exactly this, and only in the failing runs:This is a real product bug, not just a test flake: a user clicking Next in the instant after a list first measures its row height has the click silently ignored. It affects every paginated resource list — plugins, skills, browse, and automations.
State is now an anchor (the page plus the page size it was chosen under), written only by
setPageor a new projection, with the rendered page derived from it. The projection reset moved from an effect to a render-time adjustment so it can't land after an interaction. A side benefit falls out: the anchor survives page-size changes, so remeasuring 15 → 10 rows returns you to the page you picked instead of stranding you on page 1.2. Cross-package fixture race
plugins/workflows/src/cli.test.tswalks the entire checkout, and thepackagesCI shard runs every other package's tests concurrently in one turbo invocation. It enumeratedpackages/plugin-registry/.vendor-fixture-*/app.tsxand then read it after the plugin registry'safterAlldeleted it →ENOENT.The walk now skips dot-directories — covering VCS internals, tool caches, and the scratch trees siblings create in their own package roots (
.vendor-fixture-*, and agent-runtime's.bb-codex-outside-*) — and tolerates a path deleted mid-walk..git/.turbodropped out of the explicit ignore set since the dot rule subsumes them.Verification
apps/app/src/components/ui/resource-pagination.test.tsxpins the anchor semantics; its first test fails on the original hook (expected +0 to be 1) and passes on the fix. It lives inapps/appbecauseshared-uihas no test runner, matching the existingapps/app/src/components/ui/theme.test.tsconvention..vendor-fixture-PROOF/app.tsxcontaining the removed command in place, the workflows test passes (fixture ignored); with the same string indocs/cli-guide-and-skill.mdit still fails as intended, so coverage is unchanged.turbo run typecheck linton app/shared-ui/workflows/automations: 0 errors.turbo run test: app 292 files, workflows 13, automations 3, plugin-registry 1 — all pass.packagesshard: 47/48, with the two previously-racing packages passing concurrently.Two failures left untouched, both pre-existing and local-only (they pass in CI):
@bb/desktop#testneeds an Electron binary that was never downloaded in my sandbox, andbb-plugin-tasks#testfails onwindow.localStoragebeing undefined — confirmed failing identically on a pristine HEAD.🤖 Generated with Claude Code