fix(files): use a monotonic clock for processing timeout - #2657
Open
mukeshr06 wants to merge 2 commits into
Open
fix(files): use a monotonic clock for processing timeout#2657mukeshr06 wants to merge 2 commits into
mukeshr06 wants to merge 2 commits into
Conversation
gh-actions-shared Bot
pushed a commit
to xf-qubit/openai-node
that referenced
this pull request
Sep 6, 2026
## Summary - Use Node's monotonic clock for the Steady cache lock's existing 10-second acquisition budget. - Add two regression tests through the actual cache CLI for backward and forward wall-clock corrections. The test preloads and child program are static; paths and clock direction are passed as data. - Keep the 50-ms polling interval, cancellation, lease ownership, separate cleanup acquisition, and wall-clock file-aging timestamps unchanged. No generated files, SDK runtime code, dependency pins, or cache policy changes. ## Reproduction On current main, hold the lifecycle lock and release it after 12 real seconds. With no clock adjustment, the wrapper rejects the command with its existing lock-timeout error. With a single 60-second backward `Date.now()` correction after the acquisition deadline is established, the same wrapper incorrectly launches the command after roughly 12.1 seconds and exits successfully. This unaccelerated command-boundary reproduction fails before the fix on Node 22.0.0, 24.19.0, and 26.7.0. After the fix, both the normal-clock and corrected-clock cases reject late execution on all three runtimes, preserve the existing owner's lock until it is released, and leave no leases. The separate cleanup lock explains why the final process exit occurs after the fixture releases the lock; that lifecycle behavior is not changed here. ## Validation - Both new regressions failed before the production change and passed afterward. - New clock and existing cancellation tests: 8 passed on Node 22.0.0, 22.22.3, 24.19.0, and 26.7.0. An independent reviewer also repeated the clock tests five times successfully. - Canonical cache expiry, active-lease, and cleanup checks passed on Node 22.0.0, 24.19.0, and 26.7.0. - Full canonical handwritten suite on the final files: 7,800 tests passed across 205 files. - Canonical lint/format, TypeScript checking, CJS/ESM build, and `git diff --check` passed. - Independent adversarial review found no security, compatibility, lock-ownership, cancellation, or regression-test issues. Windows was reviewed for path/spawn compatibility but not executed locally; the generated suite will run in CI. This is separate from openai#2657's file-processing timeout and retains openai#2636's Steady cancellation behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes being requested
files.waitForProcessing()measuresmaxWaitwithDate.now(). A system clock correction can therefore cause a premature timeout or allow a response after the elapsed-time budget.Use
performance.now()in the handwrittensrc/lib/file-processing.tshelper to measure elapsed time. Preserve the existing polling interval, terminal statuses, initially terminal response behavior, and strict>timeout check after retrieval.Add public-client regression cases for forward and backward clock corrections, and update the existing elapsed-time mocks. Both new cases fail with the previous implementation and pass with this change. No generated SDK files or dependencies change.
Additional context & links
Validation on Node.js 24.19.0:
./scripts/test tests/lib/file-processing.test.ts: 18 passed../scripts/lint: passed../node_modules/.bin/tsc: passed../scripts/build: passed../scripts/test: all 7,751 handwritten tests passed. Generated tests could not start locally because the pinned Steady installation failed while fetching the@std/climanifest from JSR.git diff --check: passed.AI assistance: Codex was used to prepare and test this patch.