feat(session): add accidental-leave grace period - #199
Conversation
|
Warning Review limit reached
Next review available in: 39 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds a 20-second grace period for empty-room departures. It updates end-reason attribution, stores rejoin deadlines, preserves host roles during rejoin, and updates the report, tests, and documentation. ChangesSession grace window and rejoin flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Peer
participant wireSessionRoom
participant sessionStore
participant Home
participant Report
participant rejoinSession
Peer->>wireSessionRoom: Leave or disconnect
wireSessionRoom->>sessionStore: Store end reason and rejoin deadline
Home->>sessionStore: Request valid rejoin credentials
sessionStore-->>Home: Return credentials before deadline
Home->>Report: Provide rejoin callback and deadline
Home->>rejoinSession: Rejoin with stored role
rejoinSession->>wireSessionRoom: Restore session lifecycle
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/routes/Home.tsx`:
- Around line 502-506: Make the Rejoin action expiry-aware in the Home flow:
track the end time or 20-second grace deadline for auto and user endings, pass
it into Report, and ensure late clicks are disabled or rejected before
joinSession is called. Update the onRejoin wiring around handleRejoin to enforce
this deadline, and add fake-timer coverage verifying late clicks after both auto
and user endings do not reconnect.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 916f69c2-e200-4f75-89c9-baf4a24a0e54
📒 Files selected for processing (8)
ARCHITECTURE.mdPLAN.mdsrc/features/session/Report.tsxsrc/features/session/lifecycle.tssrc/routes/Home.tsxsrc/stores/sessionStore.tssrc/strings.tstests/unit/session-grace.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (8)
- GitHub Check: Installer (macOS)
- GitHub Check: Installer (Windows)
- GitHub Check: Rust (Linux)
- GitHub Check: Rust (macOS)
- GitHub Check: Rust (Windows)
- GitHub Check: Frontend
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: All design-token values—colors, spacing, fonts, radii, shadows, motion, and z-indexes—must come fromsrc/design/tokens.ts; do not use raw hex values, arbitrarypx, or inlinecubic-beziervalues.
User-facing toast and notification copy must live insrc/strings.ts; prefer centralized strings for JSX text andaria-labelvalues.
Maintain WCAG AA contrast for every text/background pairing in both themes, do not convey information by color alone, and honor the global reduced-motion kill switch; new motion must be gated by default.
Treat peer wire formats and identity derivation as cross-version contracts; coordinate changes so older builds and existing stored data remain compatible.
Every component and feature component must have a Storybook story.
Files:
src/strings.tssrc/routes/Home.tsxsrc/features/session/Report.tsxsrc/stores/sessionStore.tssrc/features/session/lifecycle.ts
**/*.{ts,tsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,rs}: Never add telemetry; the application is local-only. Never instruct users to paste model files or BIP39 mnemonics into an AI chat service.
Add comments only when the reason is non-obvious; identifiers should carry the meaning and code should read top-to-bottom.
Maintain scope discipline: do not refactor adjacent code while implementing a feature, add abstractions for hypothetical needs, or expand a bug fix beyond the bug.
Files:
src/strings.tssrc/routes/Home.tsxsrc/features/session/Report.tsxtests/unit/session-grace.test.tssrc/stores/sessionStore.tssrc/features/session/lifecycle.ts
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Use one focused change per commit, with a Conventional Commit subject such as
feat:,fix:,chore:,docs:, orci:; PRs are squash-merged.
Files:
src/strings.tsPLAN.mdsrc/routes/Home.tsxARCHITECTURE.mdsrc/features/session/Report.tsxtests/unit/session-grace.test.tssrc/stores/sessionStore.tssrc/features/session/lifecycle.ts
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Do not create new documentation files unless explicitly requested; update canonical documentation,
CHANGELOG.md, orISSUES.mdwhen justified.
Files:
PLAN.mdARCHITECTURE.md
tests/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Vitest for unit and integration tests; component tests are not currently supported because the harness uses node-env without RTL/jsdom, so component behavior belongs in Storybook and axe-core checks.
Files:
tests/unit/session-grace.test.ts
🔇 Additional comments (8)
src/features/session/lifecycle.ts (1)
424-428: LGTM!Also applies to: 451-465, 478-503, 507-513, 534-541, 560-564
src/stores/sessionStore.ts (1)
15-19: LGTM!src/routes/Home.tsx (1)
275-277: LGTM!src/features/session/Report.tsx (1)
88-93: LGTM!src/strings.ts (1)
162-162: LGTM!PLAN.md (1)
62-62: LGTM!ARCHITECTURE.md (1)
752-776: LGTM!tests/unit/session-grace.test.ts (1)
177-177: LGTM!Also applies to: 193-224, 304-305
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/Home.tsx (1)
510-515: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHide Rejoin when no recovery deadline exists.
At Line 510, a solo
userend still receiveshandleRejoin.buildLeaveHandler()storesrejoinDeadline: nullwhen no peer joined. Line 515 converts that value toundefined, andReporttreats an undefined deadline as available. The enabled button then callshandleRejoin, butgetRejoinRequest()returnsnulland the click has no effect.Require a non-null
rejoinDeadlinebefore passingonRejoin.Proposed fix
onRejoin={ - sessionEndedBy === 'auto' || sessionEndedBy === 'user' + rejoinDeadline !== null && + (sessionEndedBy === 'auto' || sessionEndedBy === 'user') ? handleRejoin : undefined }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/Home.tsx` around lines 510 - 515, Update the onRejoin condition in the Report props to require rejoinDeadline to be non-null alongside sessionEndedBy being 'auto' or 'user'; otherwise pass undefined, while preserving the existing rejoinDeadline fallback and handleRejoin behavior when a recovery deadline exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/routes/Home.tsx`:
- Around line 510-515: Update the onRejoin condition in the Report props to
require rejoinDeadline to be non-null alongside sessionEndedBy being 'auto' or
'user'; otherwise pass undefined, while preserving the existing rejoinDeadline
fallback and handleRejoin behavior when a recovery deadline exists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 882522d4-9325-4e28-9c06-93b4aae3b8d6
📒 Files selected for processing (10)
ISSUES.mdsrc/features/session/Report.tsxsrc/features/session/index.tssrc/features/session/join.tssrc/features/session/lifecycle.tssrc/routes/Home.tsxsrc/stores/sessionStore.tssrc/strings.tstests/integration/session.test.tstests/unit/session-end-reason.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/strings.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
- GitHub Check: Installer (macOS)
- GitHub Check: Installer (Windows)
- GitHub Check: Rust (macOS)
- GitHub Check: Rust (Windows)
- GitHub Check: Rust (Linux)
- GitHub Check: Frontend
- GitHub Check: Analyze (rust)
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: All design-token values—colors, spacing, fonts, radii, shadows, motion, and z-indexes—must come fromsrc/design/tokens.ts; do not use raw hex values, arbitrarypx, or inlinecubic-beziervalues.
User-facing toast and notification copy must live insrc/strings.ts; prefer centralized strings for JSX text andaria-labelvalues.
Maintain WCAG AA contrast for every text/background pairing in both themes, do not convey information by color alone, and honor the global reduced-motion kill switch; new motion must be gated by default.
Treat peer wire formats and identity derivation as cross-version contracts; coordinate changes so older builds and existing stored data remain compatible.
Every component and feature component must have a Storybook story.
Files:
src/features/session/index.tssrc/routes/Home.tsxsrc/features/session/Report.tsxsrc/features/session/join.tssrc/stores/sessionStore.tssrc/features/session/lifecycle.ts
**/*.{ts,tsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,rs}: Never add telemetry; the application is local-only. Never instruct users to paste model files or BIP39 mnemonics into an AI chat service.
Add comments only when the reason is non-obvious; identifiers should carry the meaning and code should read top-to-bottom.
Maintain scope discipline: do not refactor adjacent code while implementing a feature, add abstractions for hypothetical needs, or expand a bug fix beyond the bug.
Files:
src/features/session/index.tstests/unit/session-end-reason.test.tssrc/routes/Home.tsxtests/integration/session.test.tssrc/features/session/Report.tsxsrc/features/session/join.tssrc/stores/sessionStore.tssrc/features/session/lifecycle.ts
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Use one focused change per commit, with a Conventional Commit subject such as
feat:,fix:,chore:,docs:, orci:; PRs are squash-merged.
Files:
src/features/session/index.tsISSUES.mdtests/unit/session-end-reason.test.tssrc/routes/Home.tsxtests/integration/session.test.tssrc/features/session/Report.tsxsrc/features/session/join.tssrc/stores/sessionStore.tssrc/features/session/lifecycle.ts
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Do not create new documentation files unless explicitly requested; update canonical documentation,
CHANGELOG.md, orISSUES.mdwhen justified.
Files:
ISSUES.md
tests/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Vitest for unit and integration tests; component tests are not currently supported because the harness uses node-env without RTL/jsdom, so component behavior belongs in Storybook and axe-core checks.
Files:
tests/unit/session-end-reason.test.tstests/integration/session.test.ts
🔇 Additional comments (9)
src/features/session/lifecycle.ts (1)
277-281: LGTM!Also applies to: 397-397, 439-439, 500-506, 517-517, 543-544, 569-569
src/stores/sessionStore.ts (1)
23-28: LGTM!Also applies to: 78-81, 130-132, 148-149, 164-164, 184-184, 215-215, 326-344
src/features/session/join.ts (1)
13-36: LGTM!Also applies to: 52-58
src/features/session/index.ts (1)
2-2: LGTM!Also applies to: 12-12
src/features/session/Report.tsx (1)
88-97: LGTM!Also applies to: 117-124, 156-156, 265-265, 279-279, 291-291, 321-334, 513-518
src/routes/Home.tsx (1)
54-54: LGTM!Also applies to: 87-91, 278-305
tests/integration/session.test.ts (1)
198-198: LGTM!Also applies to: 290-309
tests/unit/session-end-reason.test.ts (1)
6-7: LGTM!Also applies to: 89-120
ISSUES.md (1)
439-439: LGTM!
Summary
auto,peer, and eviction end reasonsValidation
npm run test— 1,088 passednpm run buildnpm run lintnpm run check-tokensnpm run check-stringsnpm run check-migrationsnpm run check-storiesnpm run check-contrastnpm run format:checknpm run build-storybooknpm run check-a11y— 318 passedcargo test— 118 passed across unit/integration suitescargo fmt --checkcargo clippycargo deny checkgit diff --checkManual validation
Not machine-walked across two desktop instances. The state transition is covered with a deterministic fake scheduler for signed Leave, expiry attribution, and rejoin-before-expiry.
Closes #190
Summary by CodeRabbit
New Features
Bug Fixes