test: cover the settings write queue retry and serialization contract#567
Conversation
Driven entirely through withQueuedRetry (the one entry point external callers use; the helper exports are internal surface), with an injected recording sleep so the retry schedule is asserted rather than waited for: - first-try success returns without sleeping - EBUSY/EACCES retry with 50ms exponential backoff - a 429 retry-after hint wins over backoff and clamps into the 10ms..30s range - non-retryable errors rethrow after one attempt - persistent retryable failures give up after four attempts with no sleep after the last one - same-path tasks run strictly in submission order, a failed predecessor does not block the next write, different paths proceed independently, and every retry of a task stays ahead of the next queued task https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Warning Review limit reached
More reviews will be available in 17 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
EPERM (the most common Windows lock surface), EAGAIN, and ENOTEMPTY each get a retry case alongside the existing EBUSY/EACCES backoff test. https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB
Summary
Eighth suite in the direct-coverage push (siblings: #559, #560, #561, #563, #564, #565, #566; all independent, based on
main).lib/codex-manager/settings-write-queue.tsis the per-path serialization + retry primitive behind every settings write, with no direct tests. This addstest/settings-write-queue.test.ts(9 tests).Two deliberate design choices:
withQueuedRetryonly — it is the one entry point external callers use; the helper exports (readErrorNumber,getRetryAfterMs, …) are internal surface that refactor: prune the unused-export backlog knip recorded #556 prunes, so the suite stays valid either way.sleepis injected as a recording stub, so the retry schedule is asserted exactly (no real waiting, fully deterministic).Each test uses a unique path key so the module-level queue map never couples tests.
What the tests pin
Retry policy:
EBUSY/EACCESretry with the 50ms exponential backoff ([50, 100]).retryAfterMshint wins over backoff and clamps into the 10ms..30s range ([12345, 10, 30000]for hint values 12345 / 2 / 999999999).[50, 100, 200]).Per-path serialization (the Windows-safety core):
Validation
vitest run test/settings-write-queue.test.ts— 9/9 passingnpm run typecheck— cleannpx eslint test/settings-write-queue.test.ts --max-warnings=0— cleanhttps://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB
Generated by Claude Code
note: greptile review for oc-chatgpt-multi-auth. cite files like
lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.Greptile Summary
adds
test/settings-write-queue.test.ts— 9 deterministic vitest tests forwithQueuedRetry, the per-path serialization and retry primitive behind all settings writes.it.eachfor EPERM/EAGAIN/ENOTEMPTY, 429retryAfterMsclamping, non-retryable immediate rethrow, and four-attempt exhaustion without a trailing sleep — addressing the gap flagged in a prior review comment.recordingSleepinjects sleep as a recording stub (no real waiting);uniqueKey()prevents cross-test queue coupling via the module-level map.Confidence Score: 5/5
test-only addition with no production code changes; all 9 tests pass and the suite correctly models the windows-filesystem serialization contract.
the change is a new test file only. retry arithmetic, clamping logic, ordering assertions, and gate-based concurrency proofs all match the source implementation exactly. the previously flagged eperm/eagain gap is closed by the it.each block. no production paths are touched.
no files require special attention
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[withQueuedRetry called] --> B[enqueueSettingsWrite\nchains task onto per-path queue tail] B --> C[retry wrapper starts] C --> D{attempt < 4?} D -- no --> E[throw lastError] D -- yes --> F[call task] F -- success --> G[return result] F -- error --> H{isRetryable?} H -- no --> I[rethrow immediately] H -- yes --> J{last attempt?} J -- yes --> E J -- no --> K{has retryAfterMs?} K -- yes --> L[clamp 10ms..30s\nawait sleep] K -- no --> M[exponential backoff\n50ms * 2^attempt\nawait sleep] L --> D M --> D B --> N[queueTail always resolves\nnext path slot unblocked]Reviews (2): Last reviewed commit: "test: pin the full retryable Windows loc..." | Re-trigger Greptile