-
Notifications
You must be signed in to change notification settings - Fork 1.2k
chore: bump react-doctor to 0.9.11 and clear new findings #1336
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -479,21 +479,47 @@ test("two processes at the post-approval management seam serialize instead of in | |
| console.log(JSON.stringify({ status: response.status, catalogRefresh: body.catalogRefresh })); | ||
| `; | ||
|
|
||
| const children = (["a", "b"] as const).map(marker => Bun.spawn( | ||
| [process.execPath, "--eval", routeScript(marker)], | ||
| { cwd: repoRoot, env: sandbox.env, stdout: "pipe", stderr: "pipe" }, | ||
| )); | ||
|
|
||
| const results = await Promise.all(children.map(async child => { | ||
| const [exitCode, stdout, stderr] = await Promise.all([ | ||
| child.exited, | ||
| new Response(child.stdout).text(), | ||
| new Response(child.stderr).text(), | ||
| ]); | ||
| return { exitCode, stdout, stderr }; | ||
| })); | ||
|
|
||
| for (const result of results) { | ||
| const isPreApprovalLoss = (stderr: string): boolean => | ||
| stderr.includes("CONFIG_MUTATION_LOCK_UNAVAILABLE") | ||
| || (stderr.includes("EEXIST") && stderr.includes("createOwnership")) | ||
| || /database (?:is|table is) locked/i.test(stderr) | ||
| || stderr.includes("SQLITE_BUSY"); | ||
|
Comment on lines
+482
to
+486
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 12 \
-e 'withConfigMutationLockSync|ConfigMutationLockError|CONFIG_MUTATION_LOCK_UNAVAILABLE|createOwnership|SQLITE_BUSY|database (is|table is) locked|convergeCodexCatalog' \
src tests/codex-retained-root-serialization.test.tsRepository: lidge-jun/opencodex Length of output: 50376 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the tests and nearby source without running repository code.
wc -l tests/codex-retained-root-serialization.test.ts
sed -n '220,255p' tests/codex-retained-root-serialization.test.ts
sed -n '428,548p' tests/codex-retained-root-serialization.test.ts
printf '\n--- source matches (focused) ---\n'
rg -n -C 8 \
'class ConfigMutationLockError|export class ConfigMutationLockError|const ERROR_CODE_CONFIG_MUTATION_LOCK|CONFIG_MUTATION_LOCK_UNAVAILABLE|function withConfigMutationLockSync|export function withConfigMutationLockSync|configGenerationFailureReason|database is locked|SQLITE_BUSY' \
src/config.ts src/storage src --glob '!**/*.png' --glob '!**/*.jpg' --glob '!**/*.gif' | sed -n '1,220p'
printf '\n--- catalog convergence imports/calls near test ---\n'
sed -n '1,80p' src/codex/convergence.ts 2>/dev/null || trueRepository: lidge-jun/opencodex Length of output: 23380 Make pre-approval classification phase-specific.
🤖 Prompt for AI Agents |
||
|
|
||
| // On macOS CI both children can still lose the config lock before approval even | ||
| // after the warm-up — that proves nothing about catalog serialization. Retry | ||
| // vacuous runs until at least one process reaches the post-approval seam. | ||
| const attemptDeadline = Date.now() + 20_000; | ||
| let results: Array<{ exitCode: number; stdout: string; stderr: string }> | undefined; | ||
| while (Date.now() < attemptDeadline) { | ||
| for (const marker of ["a", "b"] as const) { | ||
| rmSync(`${barrier}-${marker}`, { force: true }); | ||
| } | ||
| writeFileSync(catalogPath, seeded); | ||
|
|
||
| const children = (["a", "b"] as const).map(marker => Bun.spawn( | ||
| [process.execPath, "--eval", routeScript(marker)], | ||
| { cwd: repoRoot, env: sandbox.env, stdout: "pipe", stderr: "pipe" }, | ||
| )); | ||
|
|
||
| results = await Promise.all(children.map(async child => { | ||
| const [exitCode, stdout, stderr] = await Promise.all([ | ||
| child.exited, | ||
| new Response(child.stdout).text(), | ||
| new Response(child.stderr).text(), | ||
| ]); | ||
| return { exitCode, stdout, stderr }; | ||
| })); | ||
|
|
||
| if (results.some(result => result.exitCode === 0)) break; | ||
|
|
||
| for (const result of results) { | ||
| expect({ preApproval: isPreApprovalLoss(result.stderr), stderr: result.stderr }) | ||
| .toMatchObject({ preApproval: true }); | ||
| } | ||
| } | ||
|
|
||
| expect(results).toBeDefined(); | ||
| for (const result of results!) { | ||
| // A process can lose a race BEFORE approval and never reach the seam at all. | ||
| // The known cases come from `saveConfigPreservingClaudeCode`: the config mutation | ||
| // lock is already held, two cold processes create the ownership file at once, or | ||
|
|
@@ -508,11 +534,8 @@ test("two processes at the post-approval management seam serialize instead of in | |
| // message as "busy" rather than a database fault, so treating it as a seam failure | ||
| // here contradicted the product code and turned ordinary contention into a red build. | ||
| if (result.exitCode !== 0) { | ||
| const preApproval = result.stderr.includes("CONFIG_MUTATION_LOCK_UNAVAILABLE") | ||
| || (result.stderr.includes("EEXIST") && result.stderr.includes("createOwnership")) | ||
| || /database (?:is|table is) locked/i.test(result.stderr) | ||
| || result.stderr.includes("SQLITE_BUSY"); | ||
| expect({ preApproval, stderr: result.stderr }).toMatchObject({ preApproval: true }); | ||
| expect({ preApproval: isPreApprovalLoss(result.stderr), stderr: result.stderr }) | ||
| .toMatchObject({ preApproval: true }); | ||
| continue; | ||
| } | ||
| const parsed = JSON.parse(result.stdout.trim()) as { | ||
|
|
@@ -530,12 +553,12 @@ test("two processes at the post-approval management seam serialize instead of in | |
|
|
||
| // At least one process must have gotten through to the seam, or this test would | ||
| // be vacuous — two config-lock losers prove nothing about catalog serialization. | ||
| expect(results.some(r => r.exitCode === 0)).toBe(true); | ||
| expect(results!.some(r => r.exitCode === 0)).toBe(true); | ||
|
|
||
| // At least one process must reach a real commit, or the race proves nothing: | ||
| // the adapter is total, so a seam that only ever failed would still answer 2xx | ||
| // with a typed disposition and satisfy every assertion above. | ||
| const dispositions = results | ||
| const dispositions = results! | ||
| .filter(r => r.exitCode === 0) | ||
| .map(r => (JSON.parse(r.stdout.trim()) as { catalogRefresh: { status: string } }).catalogRefresh.status); | ||
| expect(dispositions).toContain("committed"); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Restore coverage for the provider-navigation contract.
The new assertions check styling and, in one case, text. They do not prove that the same control navigates to
providers. A wrong or missingnavigateHashhandler can pass all these tests.gui/tests/codex-auth-provider-enable.test.tsx#L62-L63: Render the invalid-provider control interactively, scope the label assertion tobutton.link-btn, click it, and assert#providersornavigateHash("providers").gui/tests/models-empty-provider.test.tsx#L537-L537: Apply the same interactive navigation assertion to the empty live-discovery case.gui/tests/models-empty-provider.test.tsx#L548-L548: Apply the same interactive navigation assertion to the failed HTTP discovery case.gui/tests/models-empty-provider.test.tsx#L587-L587: Apply the same interactive navigation assertion to the HTTP 401 discovery case.gui/tests/models-empty-provider.test.tsx#L617-L617: Apply the same interactive navigation assertion to the blocked discovery case.📍 Affects 2 files
gui/tests/codex-auth-provider-enable.test.tsx#L62-L63(this comment)gui/tests/models-empty-provider.test.tsx#L537-L537gui/tests/models-empty-provider.test.tsx#L548-L548gui/tests/models-empty-provider.test.tsx#L587-L587gui/tests/models-empty-provider.test.tsx#L617-L617🤖 Prompt for AI Agents