fix(ecosystem): preserve retry delays in parallel workers - #2673
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Castiron custom code✅ No new custom-code files detected. 32 mixed files remain; 0 existing customizations changed. Compared 32 existing customizations unchanged
A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 34072667685 --repo openai/openai-node \
--name castiron-custom-code-34072667685-1 --dir /tmp/castiron-custom-code-34072667685-1
git apply --stat /tmp/castiron-custom-code-34072667685-1/custom-code.patch
cat /tmp/castiron-custom-code-34072667685-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin fdb038e21509264bcdf81741a40988dd694188b1 2a4b8011c02b201ee5541851c6be8c393c176b9c
python3 scripts/castiron/custom_code_report.py report \
--base fdb038e21509264bcdf81741a40988dd694188b1 \
--head 2a4b8011c02b201ee5541851c6be8c393c176b9c --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-2a4b8011c02b
cat /tmp/castiron-custom-code-2a4b8011c02b/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
markstuart-oai
left a comment
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed the full diff against fdb038e and traced parsed retryDelay through both worker-selection modes into the project retry loop. The direct argument-array addition preserves explicit zero, custom/default delays, retry budgets, and existing credential/cleanup behavior without adding an abstraction. The existing fixture remains the right home: CLI 1238→1239 lines, tests 600→618; no new 1k-line crossing or substantive structural regression.
Independently reproduced two failures/two passing controls on the base using the strengthened fixture, then all four passing on head. All 71 ecosystem CLI tests pass on Node 24.20.0; the four focused cases also pass with Node 22.22.3 and 26.7.0. A separate offline real-worker/local-install probe passed seven scenarios, checking package versions, install counts, and exactly 1/2/3 project attempts for the relevant retry budgets. Canonical lint/format checks, TypeScript 6 no-emit checking, and typechecked CLI help pass.
Validation limits: these are synthetic project commands (including the typecheck script), not full ecosystem compiler suites or wall-clock timing assertions; the complete SDK suite was not rerun. Exact Node 22.0.0 fails CLI bootstrap with the same existing yargs ERR_REQUIRE_ESM on base and head, independently of the consumer SDK support floor. Reused matching installed dependencies; no live API calls, source edits, or signing/auth changes. No substantive findings.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Reviewed exact head 2a4b801. Forwarding retryDelay into parallel worker arguments is the missing link in the existing option flow and preserves explicit zero, custom values, the 1000 ms default, retry counts, and sequential behavior. Focused real-worker coverage and the full CI matrix are green, with no unresolved threads. No in-scope findings.
3 participants
Summary
Forward
--retryDelayto parallel ecosystem-test workers alongside--retry. The parent already parses this option, but workers previously reverted to the 1,000 ms default for ordinary project retries.This is one production-line addition. It preserves explicit
0, custom delays, and the existing default without changing retry counts, option parsing, credentials, cleanup, or SDK code.Regression coverage
Strengthen the existing offline local-package fixture so each project fails its first typecheck and then succeeds. It runs the actual CLI, actual pnpm worker processes, and real local npm installations, and still verifies the selected package version.
--jobs=2 --retryDelay=25and--parallel --retryDelay=0both report 1,000 ms project retries. The sequential custom-delay and parallel default-delay controls pass: two failures, two passes.Validation
--helpentrypoint pass.The first full run encountered an unrelated local SSH-signing-agent failure in a temporary Git fixture. The focused fixture and full suite passed with commit signing disabled only for synthetic test commits in that command's environment; repository signing configuration is unchanged.
Exact Node 22.0.0 cannot load this existing CLI's pinned ESM yargs dependency on either main or this branch. Repository tooling targets Node 24; this change does not alter the consumer SDK's Node 22.0.0 floor.