fix(deps): pin react and react-dom to one version - #184
Merged
Conversation
Every workspace declares react and react-dom at exactly 19.2.7, but the root
node_modules held react 19.2.8 against react-dom 19.2.7. Dozens of packages ask
for react as a peer at ^19, npm hoisted that to the root, and the four
workspaces kept nested 19.2.7 copies. react-dom then resolved from the root and
found the root react, so every renderer test died at collection with:
Incompatible React versions: The "react" and "react-dom" packages must have
the exact same version. Instead got:
- react: 19.2.8
- react-dom: 19.2.7
This is not theoretical. On #183's run, JS & TS checks / web / check failed with
exactly that error (4 suites at collection), and apps/desktop / check:test:ui
failed the same way — the desktop case reproduced here on a pristine main
checkout with a clean npm ci, changes stashed.
Fixed with a root override, the instrument this repo already uses for exactly
this class of problem (lodash, undici, brace-expansion). The lockfile result is
one react in the tree instead of five: the four nested 19.2.7 copies collapse
and the root drops to 19.2.7.
Verified: apps/desktop --project ui now runs 396 files / 3489 tests, where
before every file failed at collection. web check 26 files / 165 tests, ui-tui
check 138 files / 1530 tests, both 0 lint errors. apps/desktop check:lint 0
errors and test:desktop:platforms 976 passed, unchanged — the override does not
disturb the electron side.
Not fixed here: ui-tui / check also fails on CI, but for an unrelated reason —
packages/hermes-ink/src/ink/ink-resize.test.ts asserts on terminal geometry and
reports "expected 128 to be less than -1" on the runner while passing locally.
That looks environment-dependent and needs its own diagnosis.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
dizhaky
marked this pull request as ready for review
August 11, 2026 22:36
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
9 tasks
github-actions Bot
pushed a commit
that referenced
this pull request
Aug 12, 2026
ink-resize.test.ts failed once on CI (on #183's run, in two of its three tests) with: AssertionError: expected 128 to be less than -1 meaning the erase landed at byte 128 and "hello" was never repainted. It is intermittent, not a standing red — it passed on #184's run along with every other JS lane. I could not reproduce it: 12 isolated runs, 8 under saturating CPU contention, 3 runs of the whole hermes-ink package, and the full ui-tui suite are all green. Two mechanisms were probed and ruled out locally. The repaint is not racing a flush — measured after onRender(), after one microtask, and after a 50ms macrotask, "hello" sits at index 39 in all three, so the `await tick()` is not load-bearing. And no deferred write from the initial render lands after the test clears its buffer, which would have explained the erase moving from byte 32 locally to 128 on CI: across five runs, zero bytes arrive after the clear. So the mechanism is unknown, and the remaining hypothesis — the erase landing in a frame whose repaint went down a diff path with nothing to write — is exactly the drift bug these tests exist to catch. That makes it possibly a real intermittent renderer race rather than a bad test. The assertion is therefore NOT loosened. Loosening it is the obvious way to stop a flake and the wrong move here: "erase written, content never repainted" is the precise condition being guarded, so a weaker check could hide a live bug. What changes is the diagnostics. The three duplicated assertions now go through expectErasedThenRepainted(), which asserts the same invariant in two steps — the text was repainted at all, then that it came after the erase — and carries the frame bytes into the failure message. Verified by negative control: fed the helper the exact CI failure shape and read the message it produces rather than assuming it renders. ui-tui check: 138 files / 1530 tests, 0 lint errors. Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8 Co-authored-by: Claude <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
2 participants
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.
What does this PR do?
Every workspace declares
reactandreact-domat exactly19.2.7, but the rootnode_modulesheld react 19.2.8 against react-dom 19.2.7. Dozens of packages ask for react as a peer at^19, npm hoisted that to the root, and the four workspaces kept nested19.2.7copies.react-domthen resolved from the root and found the rootreact, so every renderer test died at collection:This is not theoretical — it is failing CI right now. On #183's run:
JS & TS checks / web / checkJS & TS checks / apps/desktop / check:test:uiAll required checks passFixed with a root
override, which is the instrument this repo already uses for exactly this class of problem (lodash,undici,brace-expansion). The lockfile result is one react in the tree instead of five: the four nested19.2.7copies collapse and the root drops to19.2.7.Related Issue
No issue. Found while running the desktop gate for #183 and recorded there as a follow-up.
Type of Change
Changes Made
package.json—reactandreact-domadded to the rootoverrides, both19.2.7, matching every workspace's own declaration.package-lock.json— regenerated. The whole delta is the collapse described above; no package gains a version it did not already have somewhere in the tree.docs/system-log/2026-08-11.md— entry appended, including a correction to the previous entry (see below).How to Test
npm cinpm run --prefix apps/desktop test:ui— 396 files / 3489 tests pass. Onmain, every one of those files fails at collection.npm run --prefix web checkandnpm run --prefix ui-tui check.Checklist
Code
pytest tests/ -q— N/A, no Python touchedDocumentation & Housekeeping
docs/system-log/2026-08-11.md)cli-config.yaml.example,CONTRIBUTING.md/AGENTS.md, tool schemasScreenshots / Logs
Before (
main, cleannpm ci,--project ui): every file fails at collection.After:
Unchanged elsewhere, so the override doesn't disturb anything:
web26 files / 165 tests,ui-tui138 files / 1530 tests,apps/desktopcheck:lint0 errors andtest:desktop:platforms976 passed.Correction I owe on #183
#183's description said the React mismatch was local-only and left open "whether CI's js lanes are green only because they resolve the tree differently." They weren't green — CI was failing on this. I had called
get_job_logswithfailed_onlywhile that run was still in flight; it returned the five Python slices that had failed so far and read like a complete answer, while the JS jobs were still running. They failed a minute later. The finding was real and understated, not overstated.Not fixed here —
ui-tui / checkfails for a different reasonpackages/hermes-ink/src/ink/ink-resize.test.ts:111and:147fail on CI withAssertionError: expected 128 to be less than -1. That is not the React mismatch, and the file passes locally — it reads terminal geometry, so it looks environment-dependent (no TTY on the runner) rather than broken. It needs its own diagnosis rather than a drive-by fix here.Generated by Claude Code