fix(terminal-server): bump session TTL to 90d and bind IPv6 dual-stack#87
fix(terminal-server): bump session TTL to 90d and bind IPv6 dual-stack#87dvlexp wants to merge 1 commit into
Conversation
- Default TTL increased from 24h to 90 days in both server.js and session-store.js; still overridable via TERMINAL_SESSION_TTL_HOURS env. - Bind address changed from '0.0.0.0' to '::' (IPv6 wildcard, dual-stack) so clients connecting via IPv6 (Tailscale, modern browsers) reach the server. IPv4 is still served via the dual-stack mapping.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideIncreases default terminal session persistence to 90 days and updates the server to listen on the IPv6 wildcard address for dual-stack connectivity, keeping env overrides intact and aligning on-disk session storage retention with the new TTL. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The 90-day TTL and max file age values are duplicated between
TerminalServerandSessionStore; consider centralizing these defaults in a shared constant to avoid future drift. - Binding the server to
'::'can behave differently across platforms (e.g., disabled dual-stack or missing IPv6), so it may be safer to allow the bind address to be configurable or to fall back to'0.0.0.0'when'::'fails.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The 90-day TTL and max file age values are duplicated between `TerminalServer` and `SessionStore`; consider centralizing these defaults in a shared constant to avoid future drift.
- Binding the server to `'::'` can behave differently across platforms (e.g., disabled dual-stack or missing IPv6), so it may be safer to allow the bind address to be configurable or to fall back to `'0.0.0.0'` when `'::'` fails.
## Individual Comments
### Comment 1
<location path="dashboard/terminal-server/src/server.js" line_range="539" />
<code_context>
return new Promise((resolve, reject) => {
- server.listen(this.port, '0.0.0.0', (err) => {
+ server.listen(this.port, '::', (err) => {
if (err) return reject(err);
this.server = server;
</code_context>
<issue_to_address>
**issue:** Binding to '::' may drop IPv4-only accessibility depending on OS and Node configuration.
This change depends on IPv4-mapped IPv6 support, which varies by OS and configuration. On some platforms it will serve only IPv6 and break IPv4-only clients. If you want dual-stack behavior, consider omitting the host argument or explicitly listening on both IPv4 and IPv6 instead of relying on v4-mapped v6.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| return new Promise((resolve, reject) => { | ||
| server.listen(this.port, '0.0.0.0', (err) => { | ||
| server.listen(this.port, '::', (err) => { |
There was a problem hiding this comment.
issue: Binding to '::' may drop IPv4-only accessibility depending on OS and Node configuration.
This change depends on IPv4-mapped IPv6 support, which varies by OS and configuration. On some platforms it will serve only IPv6 and break IPv4-only clients. If you want dual-stack behavior, consider omitting the host argument or explicitly listening on both IPv4 and IPv6 instead of relying on v4-mapped v6.
Problem
Two independent issues with the terminal server causing session loss and connectivity failures:
0.0.0.0(IPv4 only) drops connections from Tailscale and modern browsers that prefer IPv6Solution
dashboard/terminal-server/src/server.js24hto90 days; still overridable viaTERMINAL_SESSION_TTL_HOURSenv varserver.listenbind address changed from'0.0.0.0'to'::'— IPv6 wildcard enables dual-stack, so IPv4 clients are still served via the OS dual-stack mappingdashboard/terminal-server/src/utils/session-store.jssessionTtlMsdefault updated to match (90 days)maxFileAgeDaysupdated from 7 to 90 to stay consistentTest Plan
TERMINAL_SESSION_TTL_HOURS=1env override still works — sessions expire in 1hFiles Changed
dashboard/terminal-server/src/server.js'::'dashboard/terminal-server/src/utils/session-store.jsSummary by Sourcery
Increase terminal session persistence and enable IPv6 dual-stack binding for the terminal server.
New Features:
Bug Fixes:
Enhancements: