Skip to content

fix(quota): bound snapshot persistence under sustained updates - #763

Open
luvs01 wants to merge 1 commit into
devfrom
fix/quota-persist-starvation
Open

luvs01 wants to merge 1 commit into
devfrom
fix/quota-persist-starvation

Conversation

@luvs01

@luvs01 luvs01 commented Oct 23, 2026 •

Copy link
Copy Markdown
Owner

Summary

The Codex and provider quota snapshot writers restart their 250 ms debounce timer on every update. With continuous observations arriving more often than that, disk persistence can be postponed indefinitely. The live in-memory value advances while restart recovery continues to see an old snapshot.

Preserve the 250 ms idle debounce and add a 1,000 ms maximum scheduled deferral from the first pending update, matching the existing quota reset ledger policy. Use the monotonic clock so wall-clock corrections cannot extend the window. The callback snapshots the latest accepted state; each completed, failed or cancelled write resets its timing state. Clearing one Codex account preserves the deadline for the remaining rows, and clearing all accounts cancels it.

The snapshot formats, ownership/identity checks, quota calculations, hydration expiry and atomic replacement behavior remain unchanged. The bound is on timer scheduling: event-loop delays and filesystem failures remain best-effort. This is a pre-existing persistence issue verified on the pinned Bun 1.4.3 test runtime and production Bun 1.4.4, not a new Bun 1.4.4 regression.

Add real-filesystem regressions under controlled timers for idle bursts, consecutive busy windows, latest provider row/Kiro-verdict callbacks, account removal, cancellation/home reset and recovery after a failed write. Add the focused quota-snapshot authority document and its manifest/index entries. Existing broad ownership documents are already at their size limits; no file-size limit was raised.

Verification

Base: fork dev at c4d4bb2da2162e23c314befafc5ec104292902a0. Linux x64; official Bun binaries. Repository tests used the normal preload/shared lock in the host PID namespace.

Real-clock reproduction

A separate probe runs the exported Codex quota writer in an isolated temporary home, first persists value 1, then sends 25 updates at 100 ms intervals. Both probes use the same script except for the imported baseline/fixed source module. Actual snapshot files are read during the run.

Observation Unchanged source Updated source
Disk during the first busy second 1 Advances to 11
Disk during the second busy second 1 Advances to 21
Memory after all updates 26 26
Disk before the quiet period 1 21
Disk after 300 ms quiet 26 26

This confirms the issue outside fake timers on Bun 1.4.4. The committed regressions cover both writers and deterministically assert the latest disk contents across consecutive busy windows. Against unchanged source they produced 15 passes / 3 expected failures across two files.

Commands and results

The first, third and fourth commands use the pinned Bun 1.4.3 wrapper; the second directly uses official Bun 1.4.4.

Command Result
bun scripts/test.ts tests/codex-integration/codex-quota-persistence.test.ts tests/providers/provider-account-quota-persistence.test.ts tests/codex-integration/main-quota-provenance.test.ts 44 passed, 0 failed; 186 assertions, 3 files
bun test tests/codex-integration/codex-quota-persistence.test.ts tests/providers/provider-account-quota-persistence.test.ts — Bun 1.4.4 18 passed, 0 failed; 46 assertions, 2 files
bun scripts/test.ts --changed=dev quota kiro 2,231 passed, 0 failed; 10,781 assertions, 101 files
bun scripts/test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts 27 passed, 0 failed; 568 assertions, 2 files
bun run typecheck; bun run structure:check; bun run privacy:scan; bun scripts/file-size-ratchet.ts; git diff --cached --check Passed

For the scoped import-connected quota/Kiro selection, HTTP_PROXY, HTTPS_PROXY, ALL_PROXY and their lowercase variants were removed from the test environment; NO_PROXY remained intact. Production proxy policies and fixtures are unchanged.

The full repository suite was not rerun under the documented concurrent-worktree/resource exception. Validation covers the two writers, connected quota/Kiro paths, main-identity provenance, layout guards and static gates. Native Windows/macOS execution, live provider behavior and broader exact-head CI remain separate. No public setting, DTO or workflow changed; the internal persistence contract is documented in structure/quota-snapshots.md.

Independent review by a separate agent found no material issue. That review also probed a real 1 ms producer for 2.2 seconds against the staged provider writer: 1,995 observations on Bun 1.4.3 and 2,003 on Bun 1.4.4. The first snapshot was visible while updates continued at approximately 1,004 / 1,006 ms, and the final callback value was retained after the producer stopped. This checks minimum-cadence scheduling in addition to the 100 ms Codex probe; it does not turn the best-effort timer into a hard real-time guarantee. This is agent review, not maintainer approval or hosted bot review.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. This adjusts write scheduling only; identity/account ownership checks, snapshot serialization, configured-home resolution and atomic writes remain with the existing owners. Privacy scan passed.

Co-authored-by: luvs01 27862058+luvs01@users.noreply.github.com

Summary by CodeRabbit

  • Bug Fixes
    • Account quota snapshots now save after a short idle period, with a maximum one-second delay even when updates continue.
    • Clearing quotas or switching account homes cancels outdated pending saves, helping prevent stale data from being written.
    • A failed save no longer blocks later updates from being persisted.
  • Tests
    • Added coverage for save timing, cancellation, home changes, and recovery after failed writes.

Preserve idle debouncing while bounding scheduled deferral with a monotonic clock.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between c4d4bb2 and c938424.

📒 Files selected for processing: 9
  • scripts/test-layout/layout-codex-collision.json
  • src/codex/quota.ts
  • src/providers/account-quota-disk.ts
  • structure/INDEX.md
  • structure/manifest.json
  • structure/quota-snapshots.md
  • tests/codex-integration/codex-quota-persistence.test.ts
  • tests/fixtures/test-layout-expected.json
  • tests/providers/provider-account-quota-persistence.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Codex and provider quota snapshot persistence now combines a 250 ms idle debounce with a one-second maximum deferral. Tests cover timing, cancellation, home changes, and failed writes. New documentation describes snapshot contents and persistence rules.

Changes: Quota Snapshot Persistence

Layer / File(s) Summary
Bounded snapshot scheduling
src/codex/quota.ts, src/providers/account-quota-disk.ts, structure/quota-snapshots.md, structure/INDEX.md, structure/manifest.json
Both writers cap deferral from the first pending update at one second while retaining the 250 ms idle debounce. Codex clearing and provider cancellation reset scheduling state. The documentation describes snapshot contents, timing, cancellation, and expiry rules.
Persistence validation
tests/codex-integration/codex-quota-persistence.test.ts, tests/providers/provider-account-quota-persistence.test.ts, scripts/test-layout/layout-codex-collision.json, tests/fixtures/test-layout-expected.json
Fake-timer tests check debounce timing, sustained updates, cancellation, home changes, and recovery after failed writes. Test-layout mappings include the new Codex persistence test.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

Suggested reviewers: lidge-jun, ingwannu

Merge Risk: ⚪ Minimal · up to c9384

The bounded snapshot scheduling is mergeable after normal checks; no actionable issue is established.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 4 files. (5 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: bounding quota snapshot persistence deferral during sustained updates. It matches the changes in src/codex/quota.ts and src/providers/account…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title clearly and concisely describes the main change: bounding quota snapshot persistence deferral during sustained updates. It matches the changes in src/codex/quota.ts and src/providers/account-quota-disk.ts.

Full details: Docstring Coverage

Explanation

Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 4 files. (5 skipped: 5 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@github-actions github-actions Bot added the bug Something isn't working label Oct 23, 2026
@github-actions

Copy link
Copy Markdown

✅ Deterministic PR hygiene checks passed.

@luvs01

luvs01 commented Oct 23, 2026 •

Copy link
Copy Markdown
Owner Author

No description provided.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant