fix(genui): fix flaky api-extractor dist-rewrite race (TS2307/TS7016)#2780
Conversation
`@lynx-js/genui#api-extractor` runs the root `tsc` after Turbo has scheduled `<sub>#build:api` for each subpackage. On CI the cached artifacts have been observed to land `.js` ahead of `.d.ts`, so the root `index.ts` import of `@lynx-js/genui/<sub>` would resolve to a typeless `.js` and the build would fail with TS7016. Read the subpackage `.d.ts` targets from the package.json `exports` and wait for each before invoking `tsc`.
🦋 Changeset detectedLatest commit: 5eeb6cd The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR stages lock JSON to a per-process temp file and atomically publishes it with ChangesLock Acquisition & Pipeline Alignment
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 617d45874a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Merging this PR will improve performance by 17.03%
Performance Changes
Tip Curious why this is faster? Comment Comparing Footnotes
|
UI JudgeGEQI weighted score: 62 / 100 across 8 examples.
DetailsResult 1
Result 2
Result 3
Result 4
Result 5
Result 6
Result 7
Result 8
|
This reverts commit 617d458.
`acquireLock` deleted the lock file whenever it failed to read/parse its
contents. That window is normal: between `open(wx)` (atomically creating
an empty file) and `writeFile` (populating the JSON payload), the holder
hasn't written anything yet, so a contender reads `""` and `JSON.parse`
throws. The old catch then `rm`-d the lock, letting both processes
"acquire" it.
In CI, Turbo schedules `<sub>#api-extractor` and `genui#api-extractor`
concurrently (they share only `//#build` in their dependency graph), so
both invoke this script. With the bug, `<sub>`'s `rslib build` rewrites
`<sub>/dist/index.{js,d.ts}` while `genui`'s `tsc` reads it, and tsc
catches the window where `dist/index.js` exists but `dist/index.d.ts`
does not, failing with TS7016.
Wait on read/parse failures instead of deleting; only clear the lock if
we can prove the holder's PID is dead. A standalone concurrency repro
(5 acquirers, 50ms simulated write delay) goes from `maxConcurrent=5`
to `maxConcurrent=1` with this change.
…ck cache race (#2794) ## Summary `pnpm turbo api-extractor` can flake on `code-style-check` / `test-api` with: ``` thread 'tokio-N' panicked at crates/rspack_storage/src/filesystem/db/transaction/mod.rs:53:7: Transaction already in progress by process M:rslib-node in directory '.../packages/genui/a2ui-prompt/node_modules/.cache/rspack/<hash>/.temp' Aborted (core dumped) [ELIFECYCLE] Command failed with exit code 134. ``` The race: `@lynx-js/genui#api-extractor` (in `packages/genui/turbo.json`) depends on both `//#build` *and* `@lynx-js/genui-a2ui-prompt#build:api`. The build chain triggers `genui-a2ui-prompt#build`, and at the same time turbo schedules `genui-a2ui-prompt#build:api`. Both scripts are literally `rslib build`, both produce the same rspack content hash, and both write to the same `node_modules/.cache/rspack/<hash>/.temp` directory. rspack's own filesystem-storage transaction lock catches the conflict and aborts the second process with `SIGABRT` (exit 134). Fix: add `dependsOn: ["build"]` to `build:api` in `packages/genui/a2ui-prompt/turbo.json` so turbo serializes the two. `build` runs first, releases the rspack transaction on exit; `build:api` then runs against a warm content-addressed cache. This is independent from the api-extractor lock TOCTOU fix in #2780 — that one fixed the JSON lockfile race in `run-api-extractor.mjs`; this one is rspack's internal cache directory, surfaced by turbo's task graph rather than my lock script. Scoped to just `genui-a2ui-prompt` because that's the only package where I have a panic trace. `openui`/`a2ui`/`a2ui-catalog-extractor` share the same `build`/`build:api` shape but use `tsc` not `rslib`, so they don't hit the rspack lock. If they start flaking we can extend the same dep. ## Test plan - [ ] `code-style-check` (which runs `pnpm turbo api-extractor`) passes on this PR. - [ ] No regression in `test-api` (which runs `pnpm turbo api-extractor -- --local`). - [ ] Local: `pnpm turbo api-extractor --filter=@lynx-js/genui` still produces the genui api reports without panic.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 404dea2a84
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/genui/scripts/run-api-extractor.mjs`:
- Around line 49-60: The staleHolder flag is computed from an earlier read and
can suffer a TOCTOU race before rm(lockPath); revalidate the lock immediately
before deleting by re-reading/parsing the lock file at lockPath (or checking its
current owner/pid via readFile and isProcessAlive) and only call rm(lockPath, {
force: true }) if the freshly-read pid is still a numeric dead process or the
file is unchanged; update the code around the existing readFile/staleHolder
logic (the variables/functions: readFile, lockPath, staleHolder, isProcessAlive,
rm) to perform this final check and skip deletion if the lock now belongs to a
live process or has changed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 45ff56cb-d828-401a-b11b-d1f3302daf78
📒 Files selected for processing (2)
.changeset/fix-genui-api-extractor-lock-race.mdpackages/genui/scripts/run-api-extractor.mjs
Stage the fully-written lock in a per-process temp file and publish it with link(2) instead of open(wx)+writeFile. Linking is atomic and fails with EEXIST when the lock exists, so the lock always has complete contents the moment it appears -- closing the window where a holder that dies between open() and writeFile() leaves an empty lock that wedges every later run until the 10-minute timeout. An unparsable lock can now only be a corrupt file, so it is reaped instead of waited on. Addresses Codex review on #2780.
… race genui-a2ui-prompt's build:api was identical to build (both 'rslib build') and ran right after it, rewriting the same dist/. With no turbo edge between build:api and genui-cli#build (both only depend on #build), they ran concurrently: rslib cleans dist and emits index.js before index.d.ts, so genui-cli's tsc could read dist/ in the window where the .d.ts was missing -> TS2307/TS7016 'Cannot find module @lynx-js/genui-a2ui-prompt'. Point the genui api-extractor task at #build instead and remove the duplicate build:api. dist is now written once, before its consumers, so the race is gone. This also subsumes #2794: with no second 'rslib build' there is no concurrent rspack cache transaction to conflict. Verified locally: 'turbo api-extractor --filter=@lynx-js/genui* --force -- --local' reproduced the TS error before, and passes 3/3 after.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccb0845867
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… race The real cause of the flaky 'Cannot find module @lynx-js/genui-a2ui-prompt' (TS2307/TS7016) in test-api: run-api-extractor.mjs ran 'pnpm run build' in-script. For a2ui-prompt that is 'rslib build', which cleans and rewrites dist/ (emitting index.js before index.d.ts). genui-cli#build is pulled into the api-extractor graph (via a2ui#api-extractor, whose build:catalog needs the genui CLI) and reads a2ui-prompt/dist with no turbo edge ordering it against that in-script rebuild -> its tsc could observe dist/ mid-rewrite with the .d.ts missing. Fix: stop building inside the script and make the api-extractor task depend on 'build' so turbo builds each package exactly once, before both api-extractor and every consumer build. dist/ now has a single writer with all readers ordered after it, so the race cannot occur regardless of timing. ensureMainEntryPoint stays as a last-resort build. Verified: 'turbo build' then 4x forced 'turbo api-extractor -- --local' over the genui graph -> 0 TS errors (reproduced reliably on CI before).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47c19ee6c1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/genui/scripts/run-api-extractor.mjs (1)
58-69:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRevalidate the lock immediately before deleting it.
staleHolderis derived from an earlier read, but Line 68 removes by path later. Two contenders can both classify the same dead lock as stale; one links a fresh lock, and the other then deletes that fresh lock, reopening concurrent entry into the critical section.🔧 Proposed fix
let staleHolder = false; try { const current = JSON.parse(await readFile(lockPath, 'utf8')); if (typeof current.pid === 'number' && !isProcessAlive(current.pid)) { staleHolder = true; } - } catch { - staleHolder = true; + } catch { + staleHolder = false; } if (staleHolder) { - await rm(lockPath, { force: true }); - continue; + try { + const latest = JSON.parse(await readFile(lockPath, 'utf8')); + if (typeof latest.pid === 'number' && !isProcessAlive(latest.pid)) { + await rm(lockPath, { force: true }); + continue; + } + } catch { + // Lock changed or disappeared; never delete blindly. + } } await sleep(retryDelayMs);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/genui/scripts/run-api-extractor.mjs` around lines 58 - 69, staleHolder is determined from an earlier read but the code calls rm(lockPath) later, allowing a race where another process creates a fresh lock; before deleting, revalidate by re-reading lockPath (using readFile) and re-checking the pid with isProcessAlive (or confirming contents unchanged) and only call rm(lockPath, { force: true }) if the re-read still shows a dead/stale holder; use the same variables (staleHolder, lockPath, readFile, isProcessAlive, rm) and bail out/continue if the revalidation indicates the lock is no longer stale.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/genui/scripts/run-api-extractor.mjs`:
- Around line 58-69: staleHolder is determined from an earlier read but the code
calls rm(lockPath) later, allowing a race where another process creates a fresh
lock; before deleting, revalidate by re-reading lockPath (using readFile) and
re-checking the pid with isProcessAlive (or confirming contents unchanged) and
only call rm(lockPath, { force: true }) if the re-read still shows a dead/stale
holder; use the same variables (staleHolder, lockPath, readFile, isProcessAlive,
rm) and bail out/continue if the revalidation indicates the lock is no longer
stale.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8400c3ec-ad0f-483e-80d2-283af6e7b647
📒 Files selected for processing (3)
packages/genui/scripts/run-api-extractor.mjspackages/genui/turbo.jsonturbo.json
✅ Files skipped from review due to trivial changes (1)
- packages/genui/turbo.json
The lock existed only to serialize the in-script 'pnpm run build' calls (concurrent rslib builds corrupting rspack's cache, #2794). That in-script build is gone — turbo now builds each package before its api-extractor task — so the api-extractor script only runs the extractor binary, which writes per-package outputs with no shared mutable state. Concurrent runs need no mutual exclusion, so the link/reap lock (and its stale-reaping TOCTOU flagged in review) is removed entirely. Verified: forced 'turbo api-extractor -- --local' across the genui graph -> 26/26 tasks, all 'API Extractor completed successfully', 0 TS errors.
packages/genui/a2ui/turbo.json defines its own api-extractor.dependsOn, and turbo replaces (not merges) array fields, so the root-level 'build' dep did not apply to @lynx-js/genui-a2ui#api-extractor. With the in-script build removed, a forced a2ui#api-extractor could run against a stale dist/index.d.ts. Add 'build' to a2ui's local api-extractor deps so its own package is rebuilt first too. Caught in review (Codex). Verified: every genui #api-extractor task now depends on its own #build, and forced 'turbo api-extractor -- --local' passes 26/26.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45d7648df4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The previous commit added 'build' to the root api-extractor task, which made every package's api-extractor build its package -- including react packages whose '^build' pulls 'react-transform#build:wasm' / 'swc-plugin-reactlynx#build' (cargo build-wasi). Those failed in the code-style-check / test-api jobs, which never built Rust before (the old graph only depended on '//#build', i.e. tsc). That is why CI went from the genui-cli TS2307 race to a cargo failure. Revert the broad 'build' deps (root, genui meta, a2ui) and instead scope a 'build' dep to a2ui-prompt's api-extractor only -- a2ui-prompt is Rust-free (no @lynx-js/react dep), so this orders a2ui-prompt#api-extractor after its single rslib build without pulling any cargo task. The genui-cli race is fixed by the already-removed in-script build (a2ui-prompt#build is now the sole dist writer, and genui-cli#build depends on it). a2ui/openui/meta dists build via the kept ensureMainEntryPoint fallback (single package, no turbo chain) when absent. Verified: full 'turbo api-extractor --dry=json' graph has zero cargo/react #build tasks; forced genui api-extractor runs pass 3/3 with 0 TS errors, 0 cargo invocations.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8dedae8827
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Same fix as a2ui-prompt: a2ui-catalog-extractor is rust-free, so add an explicit api-extractor -> build edge to extract from a freshly built dist instead of relying on the removed in-script build. Not applied to ui-judge: its build transitively pulls @lynx-js/react -> react-transform#build:wasm (cargo), which must not enter the api-extractor graph used by code-style-check; ui-judge relies on the ensureMainEntryPoint fallback. Verified: api-extractor dry-run graph has zero react/cargo #build tasks.
…race fix Rewrite the stale lock-TOCTOU description to the actual root cause (in-script rebuild racing consumer builds) and fix. Frontmatter stays empty: build-tooling only, no @lynx-js/genui API/runtime change.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5eeb6cd3f8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
Fixes the flaky
test-api/code-style-checkfailure where@lynx-js/genui-cli#build(andgenui#api-extractor) fail with:Root cause
packages/genui/scripts/run-api-extractor.mjsranpnpm run buildinside the api-extractor task. Fora2ui-promptthat isrslib build, which cleans and rewritesdist/(emittingindex.jsbeforeindex.d.ts).genui-cli#buildis pulled into the api-extractor graph (viaa2ui#api-extractor, whosebuild:catalogneeds the genui CLI) and readsa2ui-prompt/dist. Turbo places no edge between that in-script rebuild andgenui-cli#build, so the consumer'stsccan observedist/mid-rewrite —index.jspresent,index.d.tsmomentarily gone → TS2307/TS7016. (Only therslib-built subpackage manifests it;tsc-based subpackages don't cleandist.)The earlier theory — a TOCTOU in the file lock — was a real but secondary bug; it was not what broke CI.
Fix
pnpm run buildwas a second, racing writer of the samedist/. Removed it (keptensureMainEntryPointas a last-resort single-package build for a genuinely missing entry).a2ui-prompt,a2ui-catalog-extractor) via an explicitapi-extractor → buildedge. Not applied toa2ui/openui/ui-judge: their^buildtransitively pulls@lynx-js/react→react-transform#build:wasm(cargo), which must not enter the api-extractor graph thatcode-style-checkruns without a Rust toolchain.rslib builds (rspack cache collisions, fix(genui-a2ui-prompt): serialize build:api after build to avoid rspack cache race #2794). With no in-script build, concurrent api-extractor runs touch only their own per-package outputs and need no mutual exclusion — so the link/reap lock (and its stale-reaping TOCTOU) is gone.Net effect:
a2ui-prompt/distnow has a single writer (a2ui-prompt#build) with all readers ordered after it, so the race cannot occur regardless of timing.Test plan
turbo api-extractor --dry=jsongraph contains zero react/cargo#buildtasks (the regression that a too-broad root dependency introduced mid-review)turbo api-extractor --filter='@lynx-js/genui*' -- --local→ all api-extractor tasks succeed, 0 TS errors (reproduced the TS2307/TS7016 failure before the fix)test-apiandcode-style-checkpass