Conversation
![chatgpt-codex-connector[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 717f401772
ℹ️ 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".
sdk/typescript/tests/testCodex.ts
Outdated
| // sync does not race temp CODEX_HOME cleanup. | ||
| features: | ||
| featureOverrides && typeof featureOverrides === "object" && !Array.isArray(featureOverrides) | ||
| ? { plugins: false, ...featureOverrides } |
There was a problem hiding this comment.
The new mergeTestConfig helper is meant to disable plugin syncing for all SDK integration tests, but { plugins: false, ...featureOverrides } lets a caller-provided features.plugins value overwrite the safeguard. In tests that pass config.features.plugins = true (or undefined), plugin startup sync is re-enabled and the temp CODEX_HOME cleanup race this change is addressing can still occur. Apply user feature overrides first, then force plugins: false last so the disable is unconditional.
Useful? React with 👍 / 👎.
|
@codex review the latest version of this PR |
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
2 participants
Why
The TypeScript SDK tests create a fresh
CODEX_HOMEfor each Jest case and delete it during teardown. That cleanup has been flaking because the realcodexbinary can still be doing background curated-plugin startup sync under.tmp/plugins-clone-*, which races the test harness's recursive delete and leavesENOTEMPTYfailures behind.This path is unrelated to what the SDK tests are exercising, so letting plugin startup run during these tests only adds nondeterministic filesystem activity. This showed up recently in the
sdkCI lane for #16031.What Changed
sdk/typescript/tests/testCodex.tsto merge test config through a single helperfeatures.pluginsunconditionally for SDK integration tests so the CLI does not start curated-plugin sync in the temporaryCODEX_HOMEpluginsback tofalseVerification
pnpm test --runInBandpnpm lint