Skip to content

test: cover the settings write queue retry and serialization contract#567

Merged
ndycode merged 2 commits into
mainfrom
claude/audit-48-settings-write-queue-tests
Jun 11, 2026
Merged

test: cover the settings write queue retry and serialization contract#567
ndycode merged 2 commits into
mainfrom
claude/audit-48-settings-write-queue-tests

Conversation

@ndycode

@ndycode ndycode commented Jun 10, 2026

Copy link
Copy Markdown
Owner

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.ts is the per-path serialization + retry primitive behind every settings write, with no direct tests. This adds test/settings-write-queue.test.ts (9 tests).

Two deliberate design choices:

  • Everything is driven through withQueuedRetry only — 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.
  • sleep is 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:

  • First-try success returns the task result without sleeping.
  • EBUSY/EACCES retry with the 50ms exponential backoff ([50, 100]).
  • A 429 retryAfterMs hint wins over backoff and clamps into the 10ms..30s range ([12345, 10, 30000] for hint values 12345 / 2 / 999999999).
  • Non-retryable errors rethrow after exactly one attempt.
  • Persistent retryable failures give up after four attempts with the last error, and there is no sleep after the final attempt ([50, 100, 200]).

Per-path serialization (the Windows-safety core):

  • Tasks for the same path run strictly in submission order — the second task does not start while the first is in flight (verified with a gate plus a macrotask turn that would expose a leak).
  • A failed predecessor does not block the next write on the same path.
  • Different paths proceed independently.
  • Every retry of a task happens inside its queue slot, ahead of the next queued task — a concurrent write can never interleave between a failure and its retry.

Validation

  • vitest run test/settings-write-queue.test.ts — 9/9 passing
  • npm run typecheck — clean
  • npx eslint test/settings-write-queue.test.ts --max-warnings=0 — clean

https://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 for withQueuedRetry, the per-path serialization and retry primitive behind all settings writes.

  • retry policy: covers first-try success, exponential backoff (EBUSY/EACCES), it.each for EPERM/EAGAIN/ENOTEMPTY, 429 retryAfterMs clamping, non-retryable immediate rethrow, and four-attempt exhaustion without a trailing sleep — addressing the gap flagged in a prior review comment.
  • serialization contract: gate-based proofs that same-path tasks run in submission order, a failed predecessor doesn't block the successor, different paths are fully independent, and retries stay inside their queue slot before the next queued task starts.
  • test infra: recordingSleep injects 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

Filename Overview
test/settings-write-queue.test.ts adds 9 vitest tests covering retry policy and per-path serialization for withQueuedRetry; sleep-injection keeps tests fully deterministic; previously flagged EPERM/EAGAIN gap is addressed via it.each; no logic bugs found

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]
Loading

Reviews (2): Last reviewed commit: "test: pin the full retryable Windows loc..." | Re-trigger Greptile

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
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@ndycode, we couldn't start this review because you've reached your PR review rate limit.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9e4ca0af-86ae-4867-bbc0-a56018cbdd51

📥 Commits

Reviewing files that changed from the base of the PR and between 2e7cfb5 and c68b3b4.

📒 Files selected for processing (1)
  • test/settings-write-queue.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/audit-48-settings-write-queue-tests
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/audit-48-settings-write-queue-tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread test/settings-write-queue.test.ts
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants