Fix sign-outs caused by overlapping session refreshes by jR4dh3y · Pull Request #167 · jR4dh3y/BoxBox

jR4dh3y · 2026-10-15T06:12:05Z

Problem

Two tabs can refresh the same shared cookie at once. The losing response used to return 401 and delete the winning response's replacement cookie, signing users out with a misleading “Session expired” message.

Solution

Return 409 REFRESH_SUPERSEDED for an unexpired, replaced refresh token belonging to an active session, without changing the cookie or issuing tokens. Refresh and logout retry once with the browser's current cookie. Repeated conflicts remain recoverable; expired tokens and removed sessions still return 401.

Cover page-start restoration, explicit and automatic refresh, and logout. Add a Chromium regression using the real Go server and bundled API client in two pages sharing a cookie jar, and run it in CI. Update the API and security documentation.

Verification

Reproduced before the fix: concurrent refresh returned 200/401, erased the shared cookie, and made the next refresh fail.
Chromium regression passes: concurrent refresh returns 200/409; both pages recover, retain their cookie, and refresh again. Explicit logout still ends the other page's session.
All 113 frontend tests pass, including bounded retry and page-start conflict recovery.
Frontend format, type check, lint, build, and bundle budget pass.
go vet ./..., go test ./..., and focused auth tests with -race pass.
Real-server auth smoke checks pass, including restart persistence, stale-token rejection, storage failures, private streams, and WebSockets.

Limits

The browser regression uses local HTTP. The affected deployed instance and its HTTPS proxy have not been tested. If the replacement response is still in flight after the bounded retry, the client preserves the session and reports a retryable error.

Created by gpt-6-astra with the Codex harness in T3 Code.

Summary by CodeRabbit

Bug Fixes
- Concurrent session refreshes no longer cause a valid session cookie to be cleared when an older refresh token is used. The request receives a 409 REFRESH_SUPERSEDED response, and the active session remains available.
- The app automatically retries refresh and logout once with the current cookie. If the conflict persists, it reports the conflict without clearing the existing cookie or ending the session.
Documentation
- Updated authentication guidance to explain refresh conflicts and how to retry.

Confidence Score: 5/5

The PR appears safe to merge.

Summary

The PR stops a losing refresh request from deleting another tab’s replacement cookie.

Replaced refresh tokens get a conflict response instead of ending an active session.
Refresh and logout retry once with the browser’s current cookie.

No actionable issues were found. This review checked code and tests; it did not run them.

Diagram

sequenceDiagram
    participant A as Tab A
    participant B as Tab B
    participant S as Server
    participant C as Shared cookie
    A->>S: Refresh with current cookie
    B->>S: Refresh with same cookie
    S-->>A: 200 with replacement cookie
    A->>C: Store replacement
    S-->>B: 409 REFRESH_SUPERSEDED, no cookie change
    B->>C: Use current cookie
    B->>S: Retry refresh once
    S-->>B: 200, or retryable conflict

_{Reviews (1) · Last reviewed commit: "Fix sign-outs caused by overlapping sess..." · Reviewed by Greptile}

vercel · 2026-10-15T06:12:11Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
box-box	Ready	Preview	Oct 15, 2026 8:22am UTC

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

coderabbitai · 2026-10-15T06:12:22Z

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 0289d997-f1e9-4175-903d-2fed578ce702

📥 Commits

Reviewing files that changed from the base of the PR and between 71730cb and 0bb7f3c.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: fba74a63-00ad-4a40-93a3-50804ef4a632

📥 Commits

Reviewing files that changed from the base of the PR and between 724f199 and 71730cb.

📒 Files selected for processing: 17

.github/workflows/ci.yml
backend/internal/handler/auth_test.go
backend/internal/handler/errors.go
backend/internal/model/error.go
backend/internal/service/auth.go
backend/internal/service/auth_sessions_test.go
backend/internal/service/auth_test.go
docs/api.md
docs/development.md
docs/security.md
docs/troubleshooting.md
frontend/scripts/auth-session-browser.ts
frontend/src/lib/api/client.test.ts
frontend/src/lib/api/client.ts
frontend/src/lib/api/earlyStart.test.ts
scripts/auth-smoke/http.ts
scripts/auth-smoke/scenarios/sessions.ts

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

⏰ Context from checks skipped due to timeout: 8

GitHub Check: Greptile Review
GitHub Check: Website build
GitHub Check: Docker build validation
GitHub Check: Backend Go tests
GitHub Check: Frontend check and build
GitHub Check: Analyze (javascript-typescript)
GitHub Check: Analyze (go)
GitHub Check: Analyze (actions)

🧰 Additional context used

🪛 golangci-lint (2.14.0)

backend/internal/handler/auth_test.go

[medium] 563-563: G124: http.Cookie missing or has insecure Secure, HttpOnly, or SameSite attribute

(gosec)

🪛 LanguageTool

docs/security.md

[grammar] ~28-~28: Ensure spelling is correct
Context: ...d cookie or issuing tokens. Refresh and logout retry once with the browser's current c...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🔇 Additional comments: 17

backend/internal/model/error.go: 1

18-18: LGTM!

backend/internal/service/auth.go: 1

28-28: LGTM!

Also applies to: 336-336

backend/internal/handler/errors.go: 1

58-58: LGTM!

backend/internal/handler/auth_test.go: 1

282-283: LGTM!

Also applies to: 319-320, 544-584

backend/internal/service/auth_sessions_test.go: 1

92-93: LGTM!

Also applies to: 281-281, 288-289, 537-537, 544-545

backend/internal/service/auth_test.go: 1

80-83: LGTM!

Also applies to: 103-104

docs/api.md: 1

65-66: LGTM!

docs/security.md: 1

28-28: LGTM!

docs/troubleshooting.md: 1

21-22: LGTM!

frontend/src/lib/api/client.ts: 1

99-114: LGTM!

Also applies to: 126-126, 171-171, 264-264, 302-302

frontend/src/lib/api/client.test.ts: 1

283-319: LGTM!

frontend/src/lib/api/earlyStart.test.ts: 1

354-362: LGTM!

scripts/auth-smoke/http.ts: 1

160-181: LGTM!

scripts/auth-smoke/scenarios/sessions.ts: 1

9-9: LGTM!

Also applies to: 82-82, 90-93, 115-115, 133-133, 138-138, 218-218, 336-336

frontend/scripts/auth-session-browser.ts: 1

1-111: LGTM!

.github/workflows/ci.yml: 1

111-114: LGTM!

docs/development.md: 1

175-178: LGTM!

📝 Walkthrough

The backend now distinguishes replaced refresh tokens from revoked tokens and returns HTTP 409 with REFRESH_SUPERSEDED. The frontend retries refresh and logout once for this response. Backend, frontend, smoke, and browser tests cover the updated behavior.

Changes: Refresh session recovery

Layer / File(s)	Summary
Backend superseded-token response `backend/internal/model/error.go`, `backend/internal/service/auth.go`, `backend/internal/handler/errors.go`, `backend/internal/handler/auth_test.go`, `backend/internal/service/auth_test.go`, `backend/internal/service/auth_sessions_test.go`, `docs/api.md`, `docs/security.md`, `docs/troubleshooting.md`	The service returns `ErrRefreshSuperseded` when a refresh token no longer matches the session token. The handler maps it to HTTP 409 with `REFRESH_SUPERSEDED`. Tests and documentation cover the response and cookie behavior.
Frontend retry behavior `frontend/src/lib/api/client.ts`, `frontend/src/lib/api/client.test.ts`, `frontend/src/lib/api/earlyStart.test.ts`	Refresh and logout retry once after a 409 `REFRESH_SUPERSEDED` response. The API client also removes `skipAuthRefresh`, so requests with `skipAuth` false follow the refresh-and-retry path after a 401. Tests cover retry outcomes and session restoration.
Session recovery verification `scripts/auth-smoke/*`, `frontend/scripts/auth-session-browser.ts`, `.github/workflows/ci.yml`, `docs/development.md`	Smoke tests check for a 409 response without `Set-Cookie` when a refresh token is superseded. A browser test checks overlapping refreshes across two pages and logout behavior. CI runs the browser test, and development documentation describes how to run it.

Priority: ➖ Normal

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant BrowserClient
  participant APIHandler
  participant AuthService
  BrowserClient->>APIHandler: POST refresh with cookie
  APIHandler->>AuthService: Validate refresh token
  AuthService-->>APIHandler: ErrRefreshSuperseded
  APIHandler-->>BrowserClient: 409 REFRESH_SUPERSEDED without Set-Cookie
  BrowserClient->>APIHandler: Retry once with current cookie
  APIHandler->>AuthService: Validate current refresh token
  AuthService-->>APIHandler: Return rotated tokens
  APIHandler-->>BrowserClient: Return successful refresh

Merge Risk: ⚪ Minimal · up to 71730

This change stops overlapping session refreshes from signing users out. The refresh and logout behavior is covered by tests and documentation. No concrete merge-blocking risk was found in the supplied material.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 12 files. (5 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely identifies the primary change: preventing sign-outs caused by overlapping session refreshes.
Description check		The description clearly explains the problem, solution, verification results, limitations, and user-facing impact. It does not follow the repository template: the Change type, Safety checklist, and Re…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

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

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

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.}

Fix sign-outs caused by overlapping session refreshes

71730cb

Copilot AI balanced review requested due to automatic review settings October 15, 2026 06:12

Copilot AI reviewed Oct 15, 2026

jR4dh3y added the bug Something isn't working label Oct 15, 2026

jR4dh3y self-assigned this Oct 15, 2026

Merge branch 'master' into fix/concurrent-session-refresh

0bb7f3c

vercel Bot deployed to Preview October 15, 2026 08:22

View deployment

jR4dh3y merged commit dc3b418 into master Oct 15, 2026
12 checks passed

Navigation Menu

Repository navigation

Fix sign-outs caused by overlapping session refreshes - #167

Fix sign-outs caused by overlapping session refreshes#167
jR4dh3y merged 2 commits into
masterfrom
fix/concurrent-session-refresh

jR4dh3y commented Oct 15, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

vercel Bot commented Oct 15, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

coderabbitai Bot commented Oct 15, 2026 •

edited

Loading

Review skipped

Sequence Diagram(s)

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jR4dh3y commented Oct 15, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Verification

Limits

Summary by CodeRabbit

Confidence Score: 5/5

Summary

Diagram

Uh oh!

vercel Bot commented Oct 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Oct 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Sequence Diagram(s)

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jR4dh3y commented Oct 15, 2026 •

edited by greptile-apps Bot

Loading

vercel Bot commented Oct 15, 2026 •

edited

Loading

coderabbitai Bot commented Oct 15, 2026 •

edited

Loading