Skip to content

ci: use larger GitHub runners for Rust and browser journeys - #3090

Merged
jameskraus merged 1 commit into
mainfrom
jameskraus/larger-github-runners
Nov 9, 2026
Merged

jameskraus merged 1 commit into
mainfrom
jameskraus/larger-github-runners

Conversation

@jameskraus

@jameskraus jameskraus commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Move Rust/tool integration and browser journeys to Block's existing 16-core ubuntu-x86-runners pool. Test commands, coverage, sharding, workers, and measurement runners are unchanged.

One paired trial passed with matching test counts:

Job Standard → larger
Rust/tool integration 13m33s → 7m15s
Chromium shard 11m49s → 8m03s
WebKit shard 8m58s → 6m44s

Larger-runner trial compute cost approximately $1.01. These are preliminary results; full-matrix CI, repeatability, and human testing remain pending. Autoreview and local hooks passed.

Before/after full CI: standard runners vs. this PR. Blue jobs changed pools; all timelines use the same scale. Different PRs, so this is an observed comparison rather than a controlled benchmark.

CI job start/end timelines: standard versus larger runners, highlighting Rust and browser shards

@jameskraus
jameskraus marked this pull request as ready for review November 9, 2026 14:52
@jameskraus
jameskraus requested a review from a team as a code owner November 9, 2026 14:52

@dr0uleau dr0uleau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 approved at the checked commit. current CI passes

@kazemek21 kazemek21 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Kyle reviewed this PR and the Nexus review-tracker notes and approves.

Posted by Codex (gpt-6.1-sol) on behalf of kylez.

@jameskraus
jameskraus merged commit 345d938 into main Nov 9, 2026
42 checks passed
@jameskraus
jameskraus deleted the jameskraus/larger-github-runners branch November 9, 2026 19:51
baxen added a commit that referenced this pull request Nov 10, 2026
…3090)" (#3180)

Reverts #3090, moving Rust/tool integration and browser journeys back to
standard `ubuntu-24.04` runners.

**Why:** each CI run now asks the shared `ubuntu-x86-runners` pool for
28 runners at once (1 Rust + 9 Chromium + 18 WebKit). The pool can't
keep up:

- Since the merge, the Rust job typically waits **20–55 minutes** to get
a runner. Before, it started immediately.
- In the queued runs sampled, 20+ of the 27 browser shards were still
waiting after 20–45 minutes.
- Jobs on standard runners in those same runs (JavaScript, flake checks,
macOS) started within ~3 seconds.
- Renovate makes this worse: 108 of the 208 CI runs since 16:00 PT were
Renovate rebases, and each one asks for 28 large runners.

The larger runners did make each job faster (Rust 13.5 → 7 min), so a
narrower version, such as only Rust on the large pool, could come back
later.

Co-authored-by: ART <ea39c1c8acf57180161ef4ba4ed68abf8589f5291956b909a5aa991fc66664ca@buzz.block.builderlab.xyz>
gilgtm added a commit that referenced this pull request Nov 10, 2026
…nt-progress-rows

* origin/main:
  Revert "ci: use larger GitHub runners for Rust and browser journeys (#3090)" (#3180)
  Show Goose's unset thinking level as Default, not Off (#3124)
  ci(release): publish stable and beta desktop releases from v tags (#3150)
  fix(messages): end the New messages hold when no repair can sample (#3169)

Signed-off-by: Gil Birman <[email protected]>
damienrj added a commit that referenced this pull request Nov 10, 2026
…rminal-font-picker

* 'main' of https://github.com/block/buzz-app: (168 commits)
  Unify Membership sharing and live roster updates (#3147)
  fix(activity): show one working cue per agent request (#3166)
  Introduce opt-in first-agent setup (#2804)
  fix(codex): drop the security-review sign-in prompt from harness defaults (#3188)
  feat(releases): publish desktop download manifest (#3154)
  fix(mcp): preserve current attachments across edits (#2937)
  fix(agents): call the Claude and Codex first-run download "the integration" (#3165)
  chore(goose): bump bundled goose-acp to v1.58.1 (#3161)
  Revert "ci: use larger GitHub runners for Rust and browser journeys (#3090)" (#3180)
  Show Goose's unset thinking level as Default, not Off (#3124)
  ci(release): publish stable and beta desktop releases from v tags (#3150)
  fix(messages): end the New messages hold when no repair can sample (#3169)
  fix(search): make Command F search the Bestie conversation (#3084)
  fix(plugins): keep failed update checks on the plugin's Settings row instead of a toast (#3088)
  Show single-agent Stop on the activity bubble (#3140)
  fix: support explicit hosted browser origins (#3149)
  fix: keep Codex tool namespace descriptions bounded (#3128)
  fix(messages): show "New messages" in channels longer than one page (#3071)
  Keep shared agent guidance generic (#3134)
  fix(typing): a thread reply also ends the agent's channel-level typing (#3148)
  ...

Signed-off-by: damienrj <[email protected]>

# Conflicts:
#	src-tauri/Cargo.toml
matt2e added a commit that referenced this pull request Nov 12, 2026
## Why

`typing.spec.mjs:290`, "a growing channel composer lifts history with it
at the bottom", and its thread variant fail intermittently on main and
on unrelated PRs. The failure is an unexplained page error: `Minified
React error #185` (maximum update depth exceeded). It started showing up
in CI with the move to larger runners (#3090), but the bug is in the
product. Fast typing in a composer can throw this for real users too.

This unblocks CI on #3084 and the other PRs that hit the chromium 4/9
failure.

## Root cause

The loop is in `MessageComposer`'s `Composer`, in the effect added by
#2271 that retires oversized-paste feedback:

```ts
useEffect(() => {
  setPasteTooLong(false);
}, [draft, editing.target]);
```

1. The effect runs on every keystroke, because `draft` changes.
2. React only skips a same-value `setState` early when the component has
no pending work. While typing it always has some, so `false → false`
still queues a render.
3. Keystrokes commit synchronously, and a synchronous commit flushes
passive effects inside the commit. So every keystroke's commit ends with
that render still pending.
4. React counts a commit that leaves render work pending as a nested
update. It resets the count only after a commit that leaves none.
5. When keys arrive faster than the pending render can run by itself,
the count climbs across keystrokes. After 50 it throws #185 at the next
`setState`, which is `onDraftChange` in `EditableInput`'s
`dispatchTransaction`.

**How it was found.** A development React build never reproduced it
(0/16): it is slow enough that the pending render runs between keys. So
I patched `react-dom-client.production.js` temporarily in an unminified
production browser build. The patch logged each `scheduleUpdateOnFiber`
call (component, lane, execution context) and each commit that
increments the nested-update count. Every one of the 46 counted commits
had the same shape:

- `Composer`'s keystroke updates, at the synchronous lane.
- One more `Composer` update at the default lane, scheduled during
commit from `commitPassiveMountOnFiber`. This is the effect above.

## Fix

Clear the flag only while the feedback is shown: `if (pasteTooLong)
setPasteTooLong(false);`.

Behaviour is unchanged. A changed draft or edit target still retires
shown feedback, and hidden feedback no longer queues a render on each
keystroke. Deleting the effect's body passed every existing test, so the
effect had no coverage. I added `retires feedback when an edit replaces
the draft`. It passes with this change and fails if the effect stops
clearing.

There are no allowlist entries, retries or skips.

## Evidence

All runs are local: macOS, Chromium, `bin/pnpm test:browser
--project=chromium --no-deps tests/browser/typing.spec.mjs -g "a
growing" --repeat-each=10`.

| Snapshot | Result |
|---|---|
| main `b037c470b` | **19 failed, 1 passed**. Every failure is `Minified
React error #185` |
| this branch | **20 passed** |

Other checks:

- With the instrumented React build and the fix, 10/10 passed, and the
nested-update count never reached the logging threshold of 45. Before
the fix it reached it on every failing run.
- Full `typing.spec.mjs`, Chromium and WebKit: 10/10 passed.
- `vitest run src/features/messages/`: 99 files and 2,995 tests passed,
including the new test.
- `biome check --error-on-warnings` on the changed files and `pnpm
typecheck` pass.

Hooks were skipped (`--no-verify`), so the checks above were run by
hand.

## Not done

- I didn't try this in the desktop app. The behaviour change is the
absence of a crash under fast typing, which the journey covers.
- No human has tested this yet, so `buzz-review-completed` is not set.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: mtoohey <[email protected]>
Signed-off-by: Matt Toohey <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>
matt2e added a commit that referenced this pull request Nov 14, 2026
)

## Why

The Rust test
`credential_service::peer::tests::names_a_separate_process` failed nine
times in the "Rust and tool integration" job on 9 and 10 November, on
unrelated PRs. Every failure was `left: None` from the Linux peer
lookup: five at the helper-side lookup and four at the self-side one.

| Assertion | Sightings |
|---|---|
| Helper-side lookup returned `None` | [9
Nov](https://github.com/block/buzz-app/actions/runs/42724142961/job/129525467425),
[10
Nov](https://github.com/block/buzz-app/actions/runs/42725856302/job/129531176386),
[10
Nov](https://github.com/block/buzz-app/actions/runs/42727265967/job/129535975578),
[10
Nov](https://github.com/block/buzz-app/actions/runs/42735560453/job/129563804556),
[10
Nov](https://github.com/block/buzz-app/actions/runs/42749851762/job/129610646558)
|
| Self-side lookup returned `None` | [9
Nov](https://github.com/block/buzz-app/actions/runs/42701433869/job/129446890717),
[9
Nov](https://github.com/block/buzz-app/actions/runs/42723705988/job/129523822495),
[10
Nov](https://github.com/block/buzz-app/actions/runs/42725595645/job/129530288866),
[10
Nov](https://github.com/block/buzz-app/actions/runs/42733507790/job/129557054161)
|

The failures analysed in depth ran during the larger-runner trial
(#3090, reverted by #3180), which gave libtest 16 threads instead of 4.
The test is racy in its own right; the runner only raised the odds. The
first commit's message counts the four failures that were analysed in
depth.

## Root cause

The Linux lookup reads live `/proc` state, which is not a consistent
snapshot, while sibling tests in the same binary run alongside it. A
standalone copy of the lookup showed two mechanisms:

- **A row printed twice.** The kernel writes `/proc/net/tcp` in pieces.
While other sockets open and close, a read across a piece boundary can
print a row twice. `inode()` treated two matching rows as two sockets
even though both carried the same inode.
- **Two holders.** When a sibling test spawns a process, the child holds
copies of every descriptor until it execs, so the uniqueness check saw
two holders of the test's socket.

The first is a product defect in the pairing peer lookup on Linux: a
real caller can be refused when the table happens to repeat its row. The
second is a property of the test's process.

## Fix

All in `src-tauri/src/credential_service/peer.rs`.

- `inode()` accepts matching rows that all carry one inode as that one
socket, and still names nobody for several distinct inodes. It now
reports a missing row separately from an ambiguous one.
- `inode_of()` reads the table again, 1 ms apart, while it has no row
and the deadline has not passed. The deadline is the existing lookup
budget, so a peer that has already gone costs at most the 500 ms the
lookup was always allowed.
- Two holders are still not retried. A genuinely inherited descriptor is
meant to name nobody, and the uniqueness check is unchanged.
- `names_a_separate_process` re-execs the test binary to run its former
body alone with `--test-threads=1`, as the file already does for its
helper. The outer test checks the child's status and its "1 passed"
summary, so a renamed path cannot pass vacuously. No sibling test can
fork in that process while the sockets are scanned.

The second commit is a review follow-up to a new unit test. It asserted
more than one read for a table that never has the row, which needs the
first read to land inside a 50 ms window. It now asserts only the
outcome there: `None`, returned no earlier than the deadline. That still
fails if the retry is dropped, and the deterministic three-read case
proves the re-read.

## Tests

- `proc_net_tcp_matches_the_whole_connection` gains the repeated-row
case and asserts the missing-row result.
- `a_missing_row_is_read_again` drives `inode_of` with a reader that
returns the row on its third read, one that never has it, and one with
two sockets.

## Evidence

The branch-local check was not run: no `cargo test` or `cargo clippy`
ran on this branch's head, and the local pre-push clippy check was
skipped. The same change passed on the combined branch these two commits
were split from, and `peer.rs` here is byte-identical to that branch.
Hosted CI on this PR is the first run of this exact tree.

Combined branch, Linux, 32-core host, from the repository root with
`./bin/cargo test -p Buzz --lib --locked`:

| Scope | Result |
|---|---|
| `credential_service::peer -- --test-threads=16`, five runs after the
follow-up | 8 passed, 2 ignored each |
| `names_a_separate_process -- --test-threads=16`, 10 runs idle | 10/10
passed |
| Same, 10 runs under synthetic load | 10/10 passed |
| Full lib, `--test-threads=16`, 5 runs under the same load | 5/5, 1099
passed and 4 ignored each |
| `cargo clippy -p Buzz --locked --all-targets -- -D warnings`, `cargo
fmt --all --check` | clean |

The load was a standalone copy of the unfixed lookup: 16 threads
spawning processes, 8 churning loopback connections, and its own
lookups. Over 15,300 lookups per side it logged 193 rows printed two to
five times with a single inode and 2,644 two-holder results. Those are
the two mechanisms this change addresses, live on the host while the
fixed test passed.

## Not reproduced, not run

- The failure did not reproduce in the real test binary, fixed or
unfixed. The mechanism is shown by the standalone copy, and the runs
above guard against regression rather than reproduce CI timing.
- No lookup under load saw a table with no row at all, so the re-read
path is shown only by its unit test.
- Nothing was built or run on this branch's head, as stated above.
- The macOS build was not run. Its lookup path is untouched; it gains
only the re-exec, since the test is compiled for macOS and Linux.
- No human has tested this yet, so `buzz-review-completed` is not set.

No open PR touches `peer.rs`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Signed-off-by: mtoohey <[email protected]>
Signed-off-by: Matt Toohey <[email protected]>
Co-authored-by: Claude Fable 5.1 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants