Skip to content

test: pin serialized scheduled/forced refresh acceptance (CEL-1721) - #20

Open
mong-x wants to merge 1 commit into
mainfrom
mjnong/cel-1721-refresh-serialization
Open

test: pin serialized scheduled/forced refresh acceptance (CEL-1721)#20
mong-x wants to merge 1 commit into
mainfrom
mjnong/cel-1721-refresh-serialization

Conversation

@mong-x

@mong-x mong-x commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

CEL-1721 — serialize scheduled and forced customer-token refresh

Linear: CEL-1721 · Audit: docs/audits/2026-09-06-platform-audit-identity.md (Item 6, sessions)

Finding

The audit-era code (commit 1ef93c6, installed 0.14.0) had the defect exactly as reported: the refresh timer called performRefresh() directly while ensureAccessToken deduplicated only its own callers, so a timer tick + forced ensure before the response sent two POST /auth/refresh requests — and backend replay detection revokes the session on the second use of the shared refresh cookie.

The fix already landed on main via the CEL-1782/CEL-1853 session-resolution chain (d3cad5a..6de0850): the timer now routes through store.resolveSession({ refresh: true }), and scheduled, bootstrap, forced, and 401-retry callers all join the same refreshFlight end-to-end operation (through refreshed-identity resolution), with generation guards so a late response can neither resurrect a logged-out session nor overwrite a newer user's token.

This PR

Adds __tests__/refresh-serialization.test.ts — six tests through the real createAuthStore interface with intercepted transport and deferred refresh responses, pinning every CEL-1721 acceptance criterion so the serialization cannot regress:

  1. timer + forced ensureAccessToken overlap sends exactly ONE /auth/refresh
  2. forced + non-forced concurrent ensure callers share one result
  3. pending state resolves after completion — sequential legitimate refresh remains possible
  4. pending state resolves after error — a later refresh can retry and recover
  5. late refresh response cannot resurrect a session after logout (resolves superseded → null; store stays unauthorized)
  6. late refresh response cannot overwrite a newer user's token

TDD evidence

  • Red on audit commit 1ef93c6: tests 1, 5, 6 fail — two overlapping refresh POSTs, tok_late resurrecting after logout, tok_stale overwriting tok_newer_user.
  • Green on this branch: 6/6.

Checks

npm run typecheck ✓ · npm test ✓ (15 files, 189 tests) · npm run build ✓ · npx publint ✓ (all four CI steps)

Scope

Same-store defect only, per the ticket. Cross-origin dashboard/session isolation (separate store instances) is explicitly out of scope here. Publishing (currently 0.16.0) follows the repo release process via CI on merge; consumer pin cascades are tracked separately.


Summary by cubic

Adds a regression suite for CEL-1721 that pins the serialized refresh behavior now on main. The tests drive the real createAuthStore with an intercepted transport and deferred refresh responses, and fail on the audit-era code where overlapping refresh requests sent two POSTs, a late response resurrected a logged-out session, and a stale token overwrote a newer user's token.

  • Covers all six acceptance criteria: single request on timer/forced overlap, shared results across concurrent callers, pending state resolves after both completion and error, and late responses cannot resurrect or overwrite.
  • Tests are red against the old code and green on this branch; they use only the public store interface, so they guard the serialization contract without coupling to implementation internals.

Written for commit 05195d5. Summary will update on new commits.

Review in cubic

Adds a regression suite driving the real createAuthStore with an
intercepted transport and deferred /auth/refresh responses:

- timer + forced ensureAccessToken overlap sends exactly one request
- forced and non-forced ensure callers share one in-flight result
- pending state clears on completion AND on error; sequential
  legitimate refresh remains possible
- a late refresh response cannot resurrect a session after logout
- a late refresh response cannot overwrite a newer user's token

Verified red against the audit-era commit 1ef93c6 (two overlapping
refresh POSTs, session resurrection, stale-token overwrite) and green
on main, where the CEL-1782/CEL-1853 session-resolution chain already
routes scheduled, bootstrap, forced and 401-retry refresh through the
shared refreshFlight operation.
@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 8bff12c5-538e-45a5-89d7-45faead24bd2

📥 Commits

Reviewing files that changed from the base of the PR and between 6de0850 and 05195d5.

📒 Files selected for processing (1)
  • __tests__/refresh-serialization.test.ts

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


📝 Summary

Summary by CodeRabbit

  • Tests
    • Added coverage for concurrent authentication refresh requests.
    • Verified that overlapping refresh attempts share a single request and result.
    • Confirmed refresh retries work after success or failure.
    • Ensured late responses cannot restore cleared or outdated access tokens.

Walkthrough

The pull request adds integration tests for refresh serialization. The tests control deferred refresh responses and verify single-flight behavior, retry handling, logout safety, and protection against stale tokens.

Changes

Refresh serialization tests

Layer / File(s) Summary
Deferred refresh test harness
__tests__/refresh-serialization.test.ts
Adds controlled fetch responses, deferred refresh promises, call tracking, microtask flushing, and fake-timer setup.
Refresh serialization and stale-response scenarios
__tests__/refresh-serialization.test.ts
Tests shared refresh results, retry behavior after success or failure, and rejection of late responses after logout or token replacement.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🔵 Low · up to 05195

This PR adds coverage for refresh serialization and stale-token handling. The tests pass, but the direct fetch mock could affect later tests in a shared process, so mergeability is low risk with cleanup or isolation awareness.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the test change and its purpose: validating serialized scheduled and forced refresh behavior for CEL-1721.
Description check ✅ Passed The description directly explains the added regression tests, their acceptance criteria, implementation context, and verification results.
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mjnong/cel-1721-refresh-serialization

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

A rabbit guards the refresh gate
One request waits while callers wait
Late tokens find the door closed tight
Newer tokens keep their rightful light
Tests hop green through day and night

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

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Auto-approved: Test-only addition pinning refresh serialization behavior; no production code changes, bounded and clearly beneficial.

Re-trigger cubic

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.

1 participant