Conversation
![chatgpt-codex-connector[bot]](https://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: 3d5bea8b42
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| include: ['src/**/*.{test,spec}.{js,jsx,ts,tsx}'], | ||
| include: [ | ||
| 'src/**/*.{test,spec}.{js,jsx,ts,tsx}', | ||
| 'tests/integration/**/*.{test,spec}.{js,jsx,ts,tsx}', |
There was a problem hiding this comment.
Adding tests/integration/** to the default vitest.config.ts causes pnpm run test:run to pick up live provider smoke tests that were meant for test:integration only. I checked .github/workflows/ci.yml and the desktop-lint job’s “Run Tests” step executes pnpm run test:run, so this change makes that path sensitive to provider credentials/CLI availability and can trigger expensive cargo build setup from the new integration files, turning a deterministic unit-test lane into a flaky external-integration lane.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3db9eb5fb5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…eout - loadDotenv() now resolves .env from the repository root via __dirname instead of process.cwd(), matching the old shell script behavior when run from ui/desktop - Strip surrounding quotes from dotenv values so KEY="value" works - Give flaky tests a 120s timeout so the try/catch handler runs before vitest kills the test Signed-off-by: Douwe Osinga <douwe@squareup.com>
DOsinga
left a comment
DOsinga
left a comment
There was a problem hiding this comment.
Clean port from bash to TypeScript. Pushed a fix commit to:
- Resolve
.envfrom the repo root via__dirnameinstead ofprocess.cwd(), matching the old shell script behavior when running fromui/desktop - Strip surrounding quotes from dotenv values so
KEY="value"works correctly - Give flaky tests a 120s timeout so the try/catch handler runs before vitest kills the test externally
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33aee214e2
ℹ️ 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".
| test.each(available)('$provider / $model', async (tc) => { | ||
| await runNormalTest(tc); | ||
| }); |
There was a problem hiding this comment.
test.each(available) inherits the integration config timeout (60s in ui/desktop/vitest.integration.config.ts), but these tests make real network/provider calls and can exceed 60 seconds under normal latency, causing false CI failures even when providers are healthy. The flaky branch already bumps timeout to 120s, so the non-flaky branch should also set an explicit higher timeout (or suite-level override) to avoid regressions versus the old script, which had no per-case 60s cutoff.
Useful? React with 👍 / 👎.
2 participants
test_providers.sh has evolved beyond the simplicity of a shell script.