fix: add periodic cleanup of expired/stale sessions from database

[JohnMcLear]

Summary

Adds periodic session cleanup to SessionStore that removes expired and stale sessions from the database, preventing unbounded growth.

Root Cause

Session storage grew indefinitely (16M+ records reported) because:

1. Sessions with no expiry (_expires: null) accumulated forever with no cleanup
2. In-memory expiration timeouts were lost on server restart
3. No DB-level cleanup mechanism existed

Fix

SessionStore._cleanup() runs periodically (chained setTimeout, not setInterval, to prevent overlapping runs) which:

• Removes sessions with expired cookies
• Removes sessions with no expiry that contain no data beyond the default cookie (the empty sessions from Sessionstorage is constantly growing #5010)
• Preserves sessions with user data or valid expiry dates

All timers use .unref() and are properly cancelled in shutdown().

Test plan

• Backend tests pass (759/759)
• Type check passes
• 5 new tests in SessionStore.ts: expired removed, empty removed, data preserved, valid preserved, shutdown cancels timer

Fixes #5010

🤖 Generated with Claude Code

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Add periodic cleanup of expired/stale sessions from database

🐞 Bug fix

Walkthroughs

Description

• Adds periodic cleanup of expired/stale sessions from database
• Removes sessions with expired cookies or no meaningful data
• Prevents unbounded session storage growth in database
• Cleanup runs hourly with initial run 5 seconds after startup

Diagram

flowchart LR
  A["SessionStore initialized"] --> B["startCleanup called"]
  B --> C["Initial cleanup after 5s"]
  B --> D["Periodic cleanup every 1 hour"]
  C --> E["Find all sessionstorage keys"]
  D --> E
  E --> F["Check each session"]
  F --> G["Remove if expired"]
  F --> H["Remove if stale and empty"]
  G --> I["Log removed count"]
  H --> I

Loading

File Changes

1. src/node/db/SessionStore.ts 🐞 Bug fix +65/-0

Implement periodic session cleanup mechanism

• Added constants for stale session max age 30 days and cleanup interval 1 hour
• Added _cleanupInterval property to track periodic cleanup timer
• Implemented startCleanup() method that runs cleanup on startup and periodically
• Implemented _cleanup() async method that removes expired and stale sessions
• Updated shutdown() to clear the cleanup interval timer

src/node/db/SessionStore.ts

---

2. src/node/hooks/express.ts 🐞 Bug fix +1/-0

Start session cleanup on server startup

• Call sessionStore.startCleanup() after SessionStore initialization
• Enables periodic cleanup of expired/stale sessions on server startup

src/node/hooks/express.ts

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (2) 📎 Requirement gaps (0) 🎨 UX Issues (0)

1. sessionCleanup not documented 📘 Rule violation ⚙ Maintainability

Description

A new configuration option cookie.sessionCleanup is introduced, but no corresponding documentation
update under doc/ is provided to describe the setting and its impact. This can cause operators to
miss the new behavior and how to control it.

Code

settings.json.template[R468-472]

+    /*
+     * Whether to periodically clean up expired and stale sessions from the
+     * database. Set to false to disable. Default: true.
+     */
+    "sessionCleanup": true

Evidence

PR Compliance ID 12 requires documentation updates when modifying configuration. This PR adds a new
sessionCleanup configuration setting, but existing configuration documentation (for example Docker
environment variable documentation) is not updated to include it.

settings.json.template[468-472]
doc/docker.md[186-193]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new config option (`cookie.sessionCleanup`) was added without updating `doc/` documentation to explain it.
## Issue Context
Operators rely on `doc/` (including Docker configuration docs) to understand available settings and defaults.
## Fix Focus Areas
- settings.json.template[468-472]
- doc/docker.md[186-193]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Cleanup survives shutdown race 🐞 Bug ☼ Reliability

Description

If shutdown() happens while the async cleanup callback is running, the callback will still schedule
the next cleanup run, leaving a “zombie” periodic cleanup loop running after restart/shutdown. This
can cause unexpected DB activity after sessionStore is nulled out and can overlap with the new
store’s cleanup after restartServer().

Code

src/node/db/SessionStore.ts[R48-68]

+  _scheduleCleanup(delay: number) {
+    this._cleanupTimer = setTimeout(async () => {
+      try {
+        await this._cleanup();
+      } catch (err) {
+        logger.error('Session cleanup error:', err);
+      }
+      // Schedule the next run only after this one completes.
+      this._scheduleCleanup(CLEANUP_INTERVAL_MS);
+    }, delay);
+    // Don't prevent Node.js from exiting.
+    if (this._cleanupTimer.unref) this._cleanupTimer.unref();
 }

 shutdown() {
   for (const {timeout} of this._expirations.values()) clearTimeout(timeout);
+    if (this._cleanupTimer) {
+      clearTimeout(this._cleanupTimer);
+      this._cleanupTimer = null;
+    }
+  }

Evidence

SessionStore.shutdown() clears only the currently stored timer handle, but the already-fired timer
callback unconditionally schedules the next run after awaiting _cleanup(). During restartServer(),
closeServer() calls sessionStore.shutdown() and then drops the reference (sessionStore = null), so
if shutdown occurs mid-cleanup there is no longer a reachable handle to cancel the newly scheduled
timer.

src/node/db/SessionStore.ts[44-68]
src/node/hooks/express.ts[32-65]
src/node/hooks/express.ts[99-107]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SessionStore._scheduleCleanup()` always reschedules itself after `_cleanup()` completes. If `shutdown()` is called while `_cleanup()` is in progress (the timer already fired), the callback will still reschedule another timer, so cleanup continues even after the store is shut down and dereferenced.
## Issue Context
This can happen during `restartServer()` because `closeServer()` calls `sessionStore.shutdown()` while the process continues running and then replaces the store.
## Fix Focus Areas
- src/node/db/SessionStore.ts[40-68]
- src/node/hooks/express.ts[32-65]
## Suggested fix
- Add a boolean like `_cleanupStopped` (or reuse `_cleanupRunning` + a new stop flag) that is set to `true` in `shutdown()`.
- In the timer callback, check the stop flag before calling `_scheduleCleanup()` again.
- Make `startCleanup()` idempotent (if a timer is already scheduled and not stopped, do nothing) to avoid multiple loops.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Stale cache deletes valid sessions 🐞 Bug ≡ Correctness

Description

_cleanup() deletes sessions based on the expires value returned by DB.get(), which can be stale in
multi-instance deployments if ueberdb client-side caching returns an outdated record. This can
incorrectly delete still-valid sessions whose expiration was extended by another instance.

Code

src/node/db/SessionStore.ts[R78-104]

+  async _cleanup() {
+    const keys = await DB.findKeys('sessionstorage:*', null);
+    if (!keys || keys.length === 0) return;
+    const now = Date.now();
+    let removed = 0;
+    for (const key of keys) {
+      const sess = await DB.get(key);
+      if (!sess) {
+        await DB.remove(key);
+        removed++;
+        continue;
+      }
+      const expires = sess.cookie?.expires;
+      if (expires) {
+        // Session has an expiry — remove if expired.
+        if (new Date(expires).getTime() <= now) {
+          await DB.remove(key);
+          removed++;
+        }
+      } else {
+        // Session has no expiry and no user data beyond the cookie — remove as empty/stale.
+        const hasData = Object.keys(sess).some((k) => k !== 'cookie');
+        if (!hasData) {
+          await DB.remove(key);
+          removed++;
+        }
+      }

Evidence

_cleanup() iterates over all sessionstorage:* keys, loads each session via DB.get(), and removes it
if its cookie.expires is in the past. The same file explicitly warns that ueberdb’s default
client-side caching can yield stale expiration times, leading to premature deletion in
multi-instance setups; _cleanup() has no mitigation (grace period, re-check, or cache bypass) before
deleting.

src/node/db/SessionStore.ts[78-105]
src/node/db/SessionStore.ts[125-132]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SessionStore._cleanup()` removes sessions when `cookie.expires <= now` based on `DB.get()` results. In multi-instance deployments, `DB.get()` can return stale cached data, so an expiration that was extended by another instance might still look expired and be deleted.
## Issue Context
The code already documents this exact risk for expiration-based deletion logic (ueberdb client-side caching). Cleanup is another deletion path that should be at least as conservative.
## Fix Focus Areas
- src/node/db/SessionStore.ts[78-105]
- src/node/db/SessionStore.ts[125-132]
## Suggested fix options (pick one)
1) **Grace period**: only delete if expired for some buffer (e.g., `expiresTime <= now - GRACE_MS`), reducing the chance that a short-lived stale cache causes deletion.
2) **Re-check before delete**: when an entry appears expired, perform a second read intended to bypass cache (if supported) or otherwise re-validate before removal.
3) **Config guidance / enforcement**: if the project supports clustered deployments, document and/or enforce disabling client-side DB caching for session records when `sessionCleanup` is enabled.
(1) is implementable purely within this PR’s code surface and is the safest default without requiring DB-layer changes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View more (2)

4. Cleanup missing regression test ☑ 📘 Rule violation ⚙ Maintainability

Description

This PR adds new session cleanup behavior (_cleanup() / startCleanup()), but the diff does not
include any automated regression test to prove the bug stays fixed. Without a regression test, the
cleanup logic can be unintentionally reverted or broken without detection.

Code

src/node/db/SessionStore.ts[R67-102]

+  async _cleanup() {
+    const keys = await DB.findKeys('sessionstorage:*', null);
+    if (!keys || keys.length === 0) return;
+    const now = Date.now();
+    let removed = 0;
+    for (const key of keys) {
+      const sess = await DB.get(key);
+      if (!sess) {
+        await DB.remove(key);
+        removed++;
+        continue;
+      }
+      const expires = sess.cookie?.expires;
+      if (expires) {
+        // Session has an expiry — remove if expired.
+        if (new Date(expires).getTime() <= now) {
+          await DB.remove(key);
+          removed++;
+        }
+      } else {
+        // Session has no expiry — remove if it has no meaningful data beyond the default cookie.
+        // These are the sessions that accumulate indefinitely (bug #5010).
+        // We can't know when they were created, so we check if they have any data beyond the
+        // cookie itself. If they only contain the cookie (no user session data), they're safe
+        // to remove as stale.
+        const hasData = Object.keys(sess).some((k) => k !== 'cookie');
+        if (!hasData) {
+          await DB.remove(key);
+          removed++;
+        }
+      }
+    }
+    if (removed > 0) {
+      logger.info(`Session cleanup: removed ${removed} expired/stale sessions out of ${keys.length}`);
+    }
}

Evidence

PR Compliance ID 2 requires a regression test for bug fixes. The diff adds substantive new cleanup
logic in SessionStore but does not show any accompanying test changes in the PR diff.

src/node/db/SessionStore.ts[67-102]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR introduces new session cleanup logic but does not include an automated regression test that would fail without the fix and pass with it.
## Issue Context
This change is intended to prevent unbounded growth of `sessionstorage:*` records (issue #5010). A regression test should validate that expired sessions and “empty cookie-only” non-expiring sessions are removed by the new cleanup mechanism.
## Fix Focus Areas
- src/node/db/SessionStore.ts[45-102]
- src/tests/backend/specs/SessionStore.ts[1-220]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. Cleanup overlaps and overloads DB ☑ 🐞 Bug ➹ Performance

Description

_cleanup() calls DB.findKeys('sessionstorage:*') to load all session keys into memory and then
performs per-key DB.get()/DB.remove() operations; on large datasets this can take longer than
CLEANUP_INTERVAL_MS, causing overlapping cleanups because startCleanup() uses setInterval(). This
can lead to sustained DB load, duplicated work, and potential memory exhaustion when the key list is
very large.

Code

src/node/db/SessionStore.ts[R48-98]

+    this._cleanupInterval = setInterval(
+        () => this._cleanup().catch((err) => logger.error('Session cleanup error:', err)),
+        CLEANUP_INTERVAL_MS);
+    // Don't prevent Node.js from exiting.
+    if (this._cleanupInterval.unref) this._cleanupInterval.unref();
}
shutdown() {
for (const {timeout} of this._expirations.values()) clearTimeout(timeout);
+    if (this._cleanupInterval) {
+      clearInterval(this._cleanupInterval);
+      this._cleanupInterval = null;
+    }
+  }
+
+  /**
+   * Remove expired and stale sessions from the database. Expired sessions have a cookie.expires
+   * date in the past. Stale sessions have no expiry and haven't been touched in STALE_SESSION_MAX_AGE_MS.
+   */
+  async _cleanup() {
+    const keys = await DB.findKeys('sessionstorage:*', null);
+    if (!keys || keys.length === 0) return;
+    const now = Date.now();
+    let removed = 0;
+    for (const key of keys) {
+      const sess = await DB.get(key);
+      if (!sess) {
+        await DB.remove(key);
+        removed++;
+        continue;
+      }
+      const expires = sess.cookie?.expires;
+      if (expires) {
+        // Session has an expiry — remove if expired.
+        if (new Date(expires).getTime() <= now) {
+          await DB.remove(key);
+          removed++;
+        }
+      } else {
+        // Session has no expiry — remove if it has no meaningful data beyond the default cookie.
+        // These are the sessions that accumulate indefinitely (bug #5010).
+        // We can't know when they were created, so we check if they have any data beyond the
+        // cookie itself. If they only contain the cookie (no user session data), they're safe
+        // to remove as stale.
+        const hasData = Object.keys(sess).some((k) => k !== 'cookie');
+        if (!hasData) {
+          await DB.remove(key);
+          removed++;
+        }
+      }
+    }

Evidence

startCleanup() schedules cleanup with setInterval without guarding against a previous run still
executing. _cleanup() first retrieves all keys matching the prefix in one call, which is treated as
an in-memory array elsewhere in the codebase, implying it is not streamed/paginated.

src/node/db/SessionStore.ts[45-52]
src/node/db/SessionStore.ts[67-98]
src/node/db/PadManager.ts[74-80]
src/node/security/SecretRotator.ts[176-183]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The periodic cleanup can overload the DB and the Node process:
1) It loads *all* `sessionstorage:*` keys into memory.
2) It can run overlapping executions because it is scheduled via `setInterval()` with no mutual exclusion.
### Issue Context
On instances with millions of session rows, a full scan plus per-key `get/remove` can exceed the 1-hour interval, causing multiple concurrent cleanups and compounding DB load.
### Fix Focus Areas
- Add a guard to prevent concurrent `_cleanup()` runs (e.g., `this._cleanupRunning` or storing the in-flight promise).
- Prefer `setTimeout` re-scheduling after completion instead of `setInterval` to avoid overlap by construction.
- Reduce peak memory/DB pressure by batching:
- If `findKeys` cannot paginate, consider processing and removing keys in chunks with a concurrency limit, and/or limiting the number of keys processed per run to bound runtime.
- src/node/db/SessionStore.ts[45-52]
- src/node/db/SessionStore.ts[67-98]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

6. startCleanup() lacks feature flag 📘 Rule violation ☼ Reliability

Description

Periodic database cleanup is started unconditionally at server startup, changing runtime behavior by
default. This violates the requirement that new functionality be gated behind a feature flag and
disabled by default.

Code

src/node/hooks/express.ts[R203-205]

sessionStore = new SessionStore(settings.cookie.sessionRefreshInterval);
+  sessionStore.startCleanup();
exports.sessionMiddleware = expressSession({

Evidence

PR Compliance ID 6 requires new functionality to be behind a feature flag and disabled by default.
The diff shows sessionStore.startCleanup() is called unconditionally during server startup, and
startCleanup() always schedules timers.

src/node/hooks/express.ts[203-205]
src/node/db/SessionStore.ts[45-53]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The session cleanup timers are started by default with no configuration/feature-flag guard.
## Issue Context
Compliance requires new behavior to be feature-flagged and disabled by default so deployments can opt in safely.
## Fix Focus Areas
- src/node/hooks/express.ts[203-205]
- src/node/db/SessionStore.ts[45-53]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

7. Startup cleanup timeout leaks ☑ 🐞 Bug ☼ Reliability

Description

SessionStore.startCleanup() schedules a one-off setTimeout() but does not store/clear it in
shutdown(), so it can still fire after closeServer() has shut down the store (and after
restartServer() has created a new one). This can trigger extra concurrent cleanups and log noise,
and the startup timeout is also not unref()'d so it can delay process exit for up to 5 seconds.

Code

src/node/db/SessionStore.ts[R45-61]

+  startCleanup() {
+    // Run once on startup (deferred to avoid blocking), then periodically.
+    setTimeout(() => this._cleanup().catch((err) => logger.error('Session cleanup error:', err)), 5000);
+    this._cleanupInterval = setInterval(
+        () => this._cleanup().catch((err) => logger.error('Session cleanup error:', err)),
+        CLEANUP_INTERVAL_MS);
+    // Don't prevent Node.js from exiting.
+    if (this._cleanupInterval.unref) this._cleanupInterval.unref();
}
shutdown() {
for (const {timeout} of this._expirations.values()) clearTimeout(timeout);
+    if (this._cleanupInterval) {
+      clearInterval(this._cleanupInterval);
+      this._cleanupInterval = null;
+    }
+  }

Evidence

startCleanup() creates a 5s timeout without keeping the returned handle; shutdown() only clears the
interval. closeServer() calls sessionStore.shutdown() during restarts and shutdowns, but the pending
timeout remains and can execute after the store is considered shut down.

src/node/db/SessionStore.ts[45-61]
src/node/hooks/express.ts[32-65]
src/node/hooks/express.ts[199-206]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`startCleanup()` schedules a one-shot startup `setTimeout()` but does not store its timer ID, so `shutdown()` cannot cancel it. This allows `_cleanup()` to run after shutdown/restart and creates avoidable concurrent cleanup work and log noise.
### Issue Context
- `closeServer()` calls `sessionStore.shutdown()` on restart/shutdown.
- `shutdown()` currently only clears the interval, not the startup timeout.
### Fix Focus Areas
- Add a `this._cleanupTimeout` field and assign the result of `setTimeout()`.
- In `shutdown()`, `clearTimeout(this._cleanupTimeout)` and set it to `null`.
- Consider calling `.unref()` on the startup timeout handle as well.
- src/node/db/SessionStore.ts[45-61]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

8. Misleading stale-age documentation ☑ 🐞 Bug ⚙ Maintainability

Description

The comment/docstring claim stale sessions are determined by “haven't been touched in
STALE_SESSION_MAX_AGE_MS”, but _cleanup() never uses STALE_SESSION_MAX_AGE_MS or any age/touch time
when deciding to remove non-expiring sessions. This mismatch is misleading for maintenance and
suggests incorrect behavior to future readers.

Code

src/node/db/SessionStore.ts[R12-16]

+// Sessions without an expiry date older than this are considered stale and will be cleaned up.
+const STALE_SESSION_MAX_AGE_MS = 30 * 24 * 60 * 60 * 1000; // 30 days
+
+// How often to run the cleanup of expired/stale sessions.
+const CLEANUP_INTERVAL_MS = 60 * 60 * 1000; // 1 hour

Evidence

STALE_SESSION_MAX_AGE_MS is introduced and referenced in the _cleanup() docstring, but the
implementation only checks whether the session has keys besides 'cookie' and does not consult any
timestamp or age threshold.

src/node/db/SessionStore.ts[12-16]
src/node/db/SessionStore.ts[63-66]
src/node/db/SessionStore.ts[86-97]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The code comments/docstring describe age-based stale cleanup using `STALE_SESSION_MAX_AGE_MS`, but the implementation does not use that constant or any age/touch timestamp.
### Issue Context
This is primarily a maintainability/documentation correctness issue that can mislead future changes.
### Fix Focus Areas
- Either remove `STALE_SESSION_MAX_AGE_MS` and update docstrings/comments to describe the actual behavior (cookie-only sessions), OR implement the intended age-based logic if a reliable timestamp can be derived/stored.
- src/node/db/SessionStore.ts[12-16]
- src/node/db/SessionStore.ts[63-66]
- src/node/db/SessionStore.ts[86-97]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[qodo-free-for-open-source-projects]

Persistent review updated to latest commit dd40743

[JohnMcLear]

Maybe some people want stale sessions so this should be under settings? Because this PR could be disruptive (shouldn't be could) I want approval from another maintainer before merging.

[SamTV12345]

Maybe some people want stale sessions so this should be under settings? Because this PR could be disruptive (shouldn't be could) I want approval from another maintainer before merging.

The session is only cleared after the user has interacted with it, then left it inactive for some time—so when they return, it may appear as though they are no longer the original author, correct? I think it is correct to make it as an option but enable it as default. So if there are usecases where you need the sessions to be persisted forever it does not cause any issues.

[JohnMcLear]

Maybe some people want stale sessions so this should be under settings? Because this PR could be disruptive (shouldn't be could) I want approval from another maintainer before merging.

The session is only cleared after the user has interacted with it, then left it inactive for some time—so when they return, it may appear as though they are no longer the original author, correct? I think it is correct to make it as an option but enable it as default. So if there are usecases where you need the sessions to be persisted forever it does not cause any issues.

I think it's this from settings.json but I'll confirm before merging:

    /*
     * How long (in milliseconds) after navigating away from Etherpad before the
     * user is required to log in again. (The express_sid cookie is set to
     * expire at time now + sessionLifetime when first created, and its
     * expiration time is periodically refreshed to a new now + sessionLifetime
     * value.) If requireAuthentication is false then this value does not really
     * matter.
     *
     * The "best" value depends on your users' usage patterns and the amount of
     * convenience you desire. A long lifetime is more convenient (users won't
     * have to log back in as often) but has some drawbacks:
     *   - It increases the amount of state kept in the database.
     *   - It might weaken security somewhat: The cookie expiration is refreshed
     *     indefinitely without consulting authentication or authorization
     *     hooks, so once a user has accessed a pad, the user can continue to
     *     use the pad until the user leaves for longer than sessionLifetime.
     *   - More historical keys (sessionLifetime / keyRotationInterval) must be
     *     checked when verifying signatures.
     *
     * Session lifetime can be set to infinity (not recommended) by setting this
     * to null or 0. Note that if the session does not expire, most browsers
     * will delete the cookie when the browser exits, but a session record is
     * kept in the database forever.
     */
    "sessionLifetime": 864000000, // = 10d * 24h/d * 60m/h * 60s/m * 1000ms/s

I do have a concern here that if we release this patch it might lock the CPU thread of Node to do clear up batches, it's not ideal but it's a one time suffer and probably needs to happen...

[JohnMcLear]

Good point. I'll add a cookie.sessionCleanup setting (default true) so admins can disable it if needed. The cleanup only removes sessions that have no data beyond the default cookie — sessions with plugin data (hasData check) are always preserved. But making it configurable is the safe approach.

[JohnMcLear]

/review

[qodo-free-for-open-source-projects]

Persistent review updated to latest commit 9cfe63d

[qodo-free-for-open-source-projects]

1. sessioncleanup not documented 📘 Rule violation ⚙ Maintainability

A new configuration option cookie.sessionCleanup is introduced, but no corresponding documentation
update under doc/ is provided to describe the setting and its impact. This can cause operators to
miss the new behavior and how to control it.

Agent Prompt

## Issue description
A new config option (`cookie.sessionCleanup`) was added without updating `doc/` documentation to explain it.

## Issue Context
Operators rely on `doc/` (including Docker configuration docs) to understand available settings and defaults.

## Fix Focus Areas
- settings.json.template[468-472]
- doc/docker.md[186-193]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[qodo-free-for-open-source-projects]

2. Cleanup survives shutdown race 🐞 Bug ☼ Reliability

If shutdown() happens while the async cleanup callback is running, the callback will still schedule
the next cleanup run, leaving a “zombie” periodic cleanup loop running after restart/shutdown. This
can cause unexpected DB activity after sessionStore is nulled out and can overlap with the new
store’s cleanup after restartServer().

Agent Prompt

## Issue description
`SessionStore._scheduleCleanup()` always reschedules itself after `_cleanup()` completes. If `shutdown()` is called while `_cleanup()` is in progress (the timer already fired), the callback will still reschedule another timer, so cleanup continues even after the store is shut down and dereferenced.

## Issue Context
This can happen during `restartServer()` because `closeServer()` calls `sessionStore.shutdown()` while the process continues running and then replaces the store.

## Fix Focus Areas
- src/node/db/SessionStore.ts[40-68]
- src/node/hooks/express.ts[32-65]

## Suggested fix
- Add a boolean like `_cleanupStopped` (or reuse `_cleanupRunning` + a new stop flag) that is set to `true` in `shutdown()`.
- In the timer callback, check the stop flag before calling `_scheduleCleanup()` again.
- Make `startCleanup()` idempotent (if a timer is already scheduled and not stopped, do nothing) to avoid multiple loops.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[qodo-free-for-open-source-projects]

3. Stale cache deletes valid sessions 🐞 Bug ≡ Correctness

_cleanup() deletes sessions based on the expires value returned by DB.get(), which can be stale in
multi-instance deployments if ueberdb client-side caching returns an outdated record. This can
incorrectly delete still-valid sessions whose expiration was extended by another instance.

Agent Prompt

## Issue description
`SessionStore._cleanup()` removes sessions when `cookie.expires <= now` based on `DB.get()` results. In multi-instance deployments, `DB.get()` can return stale cached data, so an expiration that was extended by another instance might still look expired and be deleted.

## Issue Context
The code already documents this exact risk for expiration-based deletion logic (ueberdb client-side caching). Cleanup is another deletion path that should be at least as conservative.

## Fix Focus Areas
- src/node/db/SessionStore.ts[78-105]
- src/node/db/SessionStore.ts[125-132]

## Suggested fix options (pick one)
1) **Grace period**: only delete if expired for some buffer (e.g., `expiresTime <= now - GRACE_MS`), reducing the chance that a short-lived stale cache causes deletion.
2) **Re-check before delete**: when an entry appears expired, perform a second read intended to bypass cache (if supported) or otherwise re-validate before removal.
3) **Config guidance / enforcement**: if the project supports clustered deployments, document and/or enforce disabling client-side DB caching for session records when `sessionCleanup` is enabled.

(1) is implementable purely within this PR’s code surface and is the safest default without requiring DB-layer changes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Add periodic cleanup of expired/stale sessions from database

🐞 Bug fix ✨ Enhancement

Walkthroughs

Description

• Adds periodic cleanup of expired/stale sessions from database
  - Removes sessions with expired cookies
  - Removes empty sessions with no expiry (bug #5010)
  - Uses chained setTimeout to prevent overlapping runs
• Implements configurable session cleanup via cookie.sessionCleanup setting
• Adds 5 regression tests for cleanup functionality
• Ensures timers use .unref() and are properly cancelled on shutdown

Diagram

flowchart LR
  A["SessionStore startup"] -->|"startCleanup()"| B["Schedule cleanup"]
  B -->|"every 1 hour"| C["_cleanup() runs"]
  C -->|"find expired"| D["Remove expired sessions"]
  C -->|"find empty"| E["Remove empty sessions"]
  D --> F["Log removed count"]
  E --> F
  F -->|"reschedule"| B
  G["shutdown()"] -->|"clearTimeout()"| H["Cancel cleanup timer"]

Loading

File Changes

1. src/node/db/SessionStore.ts ✨ Enhancement +72/-0

Add periodic session cleanup mechanism

• Added CLEANUP_INTERVAL_MS constant (1 hour) for periodic cleanup scheduling
• Added _cleanupTimer and _cleanupRunning instance variables to track cleanup state
• Implemented startCleanup() method to initiate periodic cleanup with 5s initial delay
• Implemented _scheduleCleanup() method using chained setTimeout with .unref() for non-blocking
 timers
• Implemented _cleanup() async method that removes expired sessions and empty no-expiry sessions
• Updated shutdown() to properly cancel cleanup timer and prevent resource leaks

src/node/db/SessionStore.ts

---

2. src/node/hooks/express.ts ⚙️ Configuration changes +5/-1

Gate session cleanup behind configuration setting

• Modified session store initialization to use local variable for type safety
• Added conditional check for settings.cookie.sessionCleanup setting
• Call store.startCleanup() only when cleanup is enabled (default true)

src/node/hooks/express.ts

---

3. src/node/utils/Settings.ts ⚙️ Configuration changes +2/-0

Add sessionCleanup boolean configuration option

• Added sessionCleanup: boolean field to cookie settings object in SettingsType
• Added default value sessionCleanup: true in settings configuration

src/node/utils/Settings.ts

---

View more (2)

4. src/tests/backend/specs/SessionStore.ts 🧪 Tests +54/-0

Add comprehensive cleanup regression tests

• Added startCleanup() and _cleanup() method signatures to Session type
• Added _cleanupTimer property to Session type
• Added 5 new regression tests for cleanup functionality:
 - Test that expired sessions are removed
 - Test that empty sessions with no expiry are removed
 - Test that sessions with user data are preserved
 - Test that non-expired sessions are preserved
 - Test that shutdown cancels pending cleanup timer

src/tests/backend/specs/SessionStore.ts

---

5. settings.json.template 📝 Documentation +7/-1

Document sessionCleanup configuration option

• Added comma after sessionRefreshInterval value for valid JSON
• Added new sessionCleanup configuration option with documentation
• Documented that cleanup is enabled by default and can be disabled by setting to false

settings.json.template

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (2) 📎 Requirement gaps (0) 🎨 UX Issues (0)

1. sessionCleanup not documented 📘 Rule violation ⚙ Maintainability

Description

A new configuration option cookie.sessionCleanup is introduced, but no corresponding documentation
update under doc/ is provided to describe the setting and its impact. This can cause operators to
miss the new behavior and how to control it.

Code

settings.json.template[R468-472]

+    /*
+     * Whether to periodically clean up expired and stale sessions from the
+     * database. Set to false to disable. Default: true.
+     */
+    "sessionCleanup": true

Evidence

PR Compliance ID 12 requires documentation updates when modifying configuration. This PR adds a new
sessionCleanup configuration setting, but existing configuration documentation (for example Docker
environment variable documentation) is not updated to include it.

settings.json.template[468-472]
doc/docker.md[186-193]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new config option (`cookie.sessionCleanup`) was added without updating `doc/` documentation to explain it.
## Issue Context
Operators rely on `doc/` (including Docker configuration docs) to understand available settings and defaults.
## Fix Focus Areas
- settings.json.template[468-472]
- doc/docker.md[186-193]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Cleanup survives shutdown race 🐞 Bug ☼ Reliability

Description

If shutdown() happens while the async cleanup callback is running, the callback will still schedule
the next cleanup run, leaving a “zombie” periodic cleanup loop running after restart/shutdown. This
can cause unexpected DB activity after sessionStore is nulled out and can overlap with the new
store’s cleanup after restartServer().

Code

src/node/db/SessionStore.ts[R48-68]

+  _scheduleCleanup(delay: number) {
+    this._cleanupTimer = setTimeout(async () => {
+      try {
+        await this._cleanup();
+      } catch (err) {
+        logger.error('Session cleanup error:', err);
+      }
+      // Schedule the next run only after this one completes.
+      this._scheduleCleanup(CLEANUP_INTERVAL_MS);
+    }, delay);
+    // Don't prevent Node.js from exiting.
+    if (this._cleanupTimer.unref) this._cleanupTimer.unref();
}
shutdown() {
  for (const {timeout} of this._expirations.values()) clearTimeout(timeout);
+    if (this._cleanupTimer) {
+      clearTimeout(this._cleanupTimer);
+      this._cleanupTimer = null;
+    }
+  }

Evidence

SessionStore.shutdown() clears only the currently stored timer handle, but the already-fired timer
callback unconditionally schedules the next run after awaiting _cleanup(). During restartServer(),
closeServer() calls sessionStore.shutdown() and then drops the reference (sessionStore = null), so
if shutdown occurs mid-cleanup there is no longer a reachable handle to cancel the newly scheduled
timer.

src/node/db/SessionStore.ts[44-68]
src/node/hooks/express.ts[32-65]
src/node/hooks/express.ts[99-107]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SessionStore._scheduleCleanup()` always reschedules itself after `_cleanup()` completes. If `shutdown()` is called while `_cleanup()` is in progress (the timer already fired), the callback will still reschedule another timer, so cleanup continues even after the store is shut down and dereferenced.
## Issue Context
This can happen during `restartServer()` because `closeServer()` calls `sessionStore.shutdown()` while the process continues running and then replaces the store.
## Fix Focus Areas
- src/node/db/SessionStore.ts[40-68]
- src/node/hooks/express.ts[32-65]
## Suggested fix
- Add a boolean like `_cleanupStopped` (or reuse `_cleanupRunning` + a new stop flag) that is set to `true` in `shutdown()`.
- In the timer callback, check the stop flag before calling `_scheduleCleanup()` again.
- Make `startCleanup()` idempotent (if a timer is already scheduled and not stopped, do nothing) to avoid multiple loops.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Stale cache deletes valid sessions 🐞 Bug ≡ Correctness

Description

_cleanup() deletes sessions based on the expires value returned by DB.get(), which can be stale in
multi-instance deployments if ueberdb client-side caching returns an outdated record. This can
incorrectly delete still-valid sessions whose expiration was extended by another instance.

Code

src/node/db/SessionStore.ts[R78-104]

+  async _cleanup() {
+    const keys = await DB.findKeys('sessionstorage:*', null);
+    if (!keys || keys.length === 0) return;
+    const now = Date.now();
+    let removed = 0;
+    for (const key of keys) {
+      const sess = await DB.get(key);
+      if (!sess) {
+        await DB.remove(key);
+        removed++;
+        continue;
+      }
+      const expires = sess.cookie?.expires;
+      if (expires) {
+        // Session has an expiry — remove if expired.
+        if (new Date(expires).getTime() <= now) {
+          await DB.remove(key);
+          removed++;
+        }
+      } else {
+        // Session has no expiry and no user data beyond the cookie — remove as empty/stale.
+        const hasData = Object.keys(sess).some((k) => k !== 'cookie');
+        if (!hasData) {
+          await DB.remove(key);
+          removed++;
+        }
+      }

Evidence

_cleanup() iterates over all sessionstorage:* keys, loads each session via DB.get(), and removes it
if its cookie.expires is in the past. The same file explicitly warns that ueberdb’s default
client-side caching can yield stale expiration times, leading to premature deletion in
multi-instance setups; _cleanup() has no mitigation (grace period, re-check, or cache bypass) before
deleting.

src/node/db/SessionStore.ts[78-105]
src/node/db/SessionStore.ts[125-132]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SessionStore._cleanup()` removes sessions when `cookie.expires <= now` based on `DB.get()` results. In multi-instance deployments, `DB.get()` can return stale cached data, so an expiration that was extended by another instance might still look expired and be deleted.
## Issue Context
The code already documents this exact risk for expiration-based deletion logic (ueberdb client-side caching). Cleanup is another deletion path that should be at least as conservative.
## Fix Focus Areas
- src/node/db/SessionStore.ts[78-105]
- src/node/db/SessionStore.ts[125-132]
## Suggested fix options (pick one)
1) **Grace period**: only delete if expired for some buffer (e.g., `expiresTime <= now - GRACE_MS`), reducing the chance that a short-lived stale cache causes deletion.
2) **Re-check before delete**: when an entry appears expired, perform a second read intended to bypass cache (if supported) or otherwise re-validate before removal.
3) **Config guidance / enforcement**: if the project supports clustered deployments, document and/or enforce disabling client-side DB caching for session records when `sessionCleanup` is enabled.
(1) is implementable purely within this PR’s code surface and is the safest default without requiring DB-layer changes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View more (2)

4. Cleanup missing regression test ☑ 📘 Rule violation ⚙ Maintainability

Description

This PR adds new session cleanup behavior (_cleanup() / startCleanup()), but the diff does not
include any automated regression test to prove the bug stays fixed. Without a regression test, the
cleanup logic can be unintentionally reverted or broken without detection.

Code

src/node/db/SessionStore.ts[R67-102]

+  async _cleanup() {
+    const keys = await DB.findKeys('sessionstorage:*', null);
+    if (!keys || keys.length === 0) return;
+    const now = Date.now();
+    let removed = 0;
+    for (const key of keys) {
+      const sess = await DB.get(key);
+      if (!sess) {
+        await DB.remove(key);
+        removed++;
+        continue;
+      }
+      const expires = sess.cookie?.expires;
+      if (expires) {
+        // Session has an expiry — remove if expired.
+        if (new Date(expires).getTime() <= now) {
+          await DB.remove(key);
+          removed++;
+        }
+      } else {
+        // Session has no expiry — remove if it has no meaningful data beyond the default cookie.
+        // These are the sessions that accumulate indefinitely (bug #5010).
+        // We can't know when they were created, so we check if they have any data beyond the
+        // cookie itself. If they only contain the cookie (no user session data), they're safe
+        // to remove as stale.
+        const hasData = Object.keys(sess).some((k) => k !== 'cookie');
+        if (!hasData) {
+          await DB.remove(key);
+          removed++;
+        }
+      }
+    }
+    if (removed > 0) {
+      logger.info(`Session cleanup: removed ${removed} expired/stale sessions out of ${keys.length}`);
+    }
}

Evidence

PR Compliance ID 2 requires a regression test for bug fixes. The diff adds substantive new cleanup
logic in SessionStore but does not show any accompanying test changes in the PR diff.

src/node/db/SessionStore.ts[67-102]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR introduces new session cleanup logic but does not include an automated regression test that would fail without the fix and pass with it.
## Issue Context
This change is intended to prevent unbounded growth of `sessionstorage:*` records (issue #5010). A regression test should validate that expired sessions and “empty cookie-only” non-expiring sessions are removed by the new cleanup mechanism.
## Fix Focus Areas
- src/node/db/SessionStore.ts[45-102]
- src/tests/backend/specs/SessionStore.ts[1-220]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. Cleanup overlaps and overloads DB ☑ 🐞 Bug ➹ Performance

Description

_cleanup() calls DB.findKeys('sessionstorage:*') to load all session keys into memory and then
performs per-key DB.get()/DB.remove() operations; on large datasets this can take longer than
CLEANUP_INTERVAL_MS, causing overlapping cleanups because startCleanup() uses setInterval(). This
can lead to sustained DB load, duplicated work, and potential memory exhaustion when the key list is
very large.

Code

src/node/db/SessionStore.ts[R48-98]

+    this._cleanupInterval = setInterval(
+        () => this._cleanup().catch((err) => logger.error('Session cleanup error:', err)),
+        CLEANUP_INTERVAL_MS);
+    // Don't prevent Node.js from exiting.
+    if (this._cleanupInterval.unref) this._cleanupInterval.unref();
}
shutdown() {
for (const {timeout} of this._expirations.values()) clearTimeout(timeout);
+    if (this._cleanupInterval) {
+      clearInterval(this._cleanupInterval);
+      this._cleanupInterval = null;
+    }
+  }
+
+  /**
+   * Remove expired and stale sessions from the database. Expired sessions have a cookie.expires
+   * date in the past. Stale sessions have no expiry and haven't been touched in STALE_SESSION_MAX_AGE_MS.
+   */
+  async _cleanup() {
+    const keys = await DB.findKeys('sessionstorage:*', null);
+    if (!keys || keys.length === 0) return;
+    const now = Date.now();
+    let removed = 0;
+    for (const key of keys) {
+      const sess = await DB.get(key);
+      if (!sess) {
+        await DB.remove(key);
+        removed++;
+        continue;
+      }
+      const expires = sess.cookie?.expires;
+      if (expires) {
+        // Session has an expiry — remove if expired.
+        if (new Date(expires).getTime() <= now) {
+          await DB.remove(key);
+          removed++;
+        }
+      } else {
+        // Session has no expiry — remove if it has no meaningful data beyond the default cookie.
+        // These are the sessions that accumulate indefinitely (bug #5010).
+        // We can't know when they were created, so we check if they have any data beyond the
+        // cookie itself. If they only contain the cookie (no user session data), they're safe
+        // to remove as stale.
+        const hasData = Object.keys(sess).some((k) => k !== 'cookie');
+        if (!hasData) {
+          await DB.remove(key);
+          removed++;
+        }
+      }
+    }

Evidence

startCleanup() schedules cleanup with setInterval without guarding against a previous run still
executing. _cleanup() first retrieves all keys matching the prefix in one call, which is treated as
an in-memory array elsewhere in the codebase, implying it is not streamed/paginated.

src/node/db/SessionStore.ts[45-52]
src/node/db/SessionStore.ts[67-98]
src/node/db/PadManager.ts[74-80]
src/node/security/SecretRotator.ts[176-183]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The periodic cleanup can overload the DB and the Node process:
1) It loads *all* `sessionstorage:*` keys into memory.
2) It can run overlapping executions because it is scheduled via `setInterval()` with no mutual exclusion.
### Issue Context
On instances with millions of session rows, a full scan plus per-key `get/remove` can exceed the 1-hour interval, causing multiple concurrent cleanups and compounding DB load.
### Fix Focus Areas
- Add a guard to prevent concurrent `_cleanup()` runs (e.g., `this._cleanupRunning` or storing the in-flight promise).
- Prefer `setTimeout` re-scheduling after completion instead of `setInterval` to avoid overlap by construction.
- Reduce peak memory/DB pressure by batching:
- If `findKeys` cannot paginate, consider processing and removing keys in chunks with a concurrency limit, and/or limiting the number of keys processed per run to bound runtime.
- src/node/db/SessionStore.ts[45-52]
- src/node/db/SessionStore.ts[67-98]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

6. startCleanup() lacks feature flag 📘 Rule violation ☼ Reliability

Description

Periodic database cleanup is started unconditionally at server startup, changing runtime behavior by
default. This violates the requirement that new functionality be gated behind a feature flag and
disabled by default.

Code

src/node/hooks/express.ts[R203-205]

sessionStore = new SessionStore(settings.cookie.sessionRefreshInterval);
+  sessionStore.startCleanup();
exports.sessionMiddleware = expressSession({

Evidence

PR Compliance ID 6 requires new functionality to be behind a feature flag and disabled by default.
The diff shows sessionStore.startCleanup() is called unconditionally during server startup, and
startCleanup() always schedules timers.

src/node/hooks/express.ts[203-205]
src/node/db/SessionStore.ts[45-53]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The session cleanup timers are started by default with no configuration/feature-flag guard.
## Issue Context
Compliance requires new behavior to be feature-flagged and disabled by default so deployments can opt in safely.
## Fix Focus Areas
- src/node/hooks/express.ts[203-205]
- src/node/db/SessionStore.ts[45-53]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

7. Startup cleanup timeout leaks ☑ 🐞 Bug ☼ Reliability

Description

SessionStore.startCleanup() schedules a one-off setTimeout() but does not store/clear it in
shutdown(), so it can still fire after closeServer() has shut down the store (and after
restartServer() has created a new one). This can trigger extra concurrent cleanups and log noise,
and the startup timeout is also not unref()'d so it can delay process exit for up to 5 seconds.

Code

src/node/db/SessionStore.ts[R45-61]

+  startCleanup() {
+    // Run once on startup (deferred to avoid blocking), then periodically.
+    setTimeout(() => this._cleanup().catch((err) => logger.error('Session cleanup error:', err)), 5000);
+    this._cleanupInterval = setInterval(
+        () => this._cleanup().catch((err) => logger.error('Session cleanup error:', err)),
+        CLEANUP_INTERVAL_MS);
+    // Don't prevent Node.js from exiting.
+    if (this._cleanupInterval.unref) this._cleanupInterval.unref();
}
shutdown() {
for (const {timeout} of this._expirations.values()) clearTimeout(timeout);
+    if (this._cleanupInterval) {
+      clearInterval(this._cleanupInterval);
+      this._cleanupInterval = null;
+    }
+  }

Evidence

startCleanup() creates a 5s timeout without keeping the returned handle; shutdown() only clears the
interval. closeServer() calls sessionStore.shutdown() during restarts and shutdowns, but the pending
timeout remains and can execute after the store is considered shut down.

src/node/db/SessionStore.ts[45-61]
src/node/hooks/express.ts[32-65]
src/node/hooks/express.ts[199-206]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`startCleanup()` schedules a one-shot startup `setTimeout()` but does not store its timer ID, so `shutdown()` cannot cancel it. This allows `_cleanup()` to run after shutdown/restart and creates avoidable concurrent cleanup work and log noise.
### Issue Context
- `closeServer()` calls `sessionStore.shutdown()` on restart/shutdown.
- `shutdown()` currently only clears the interval, not the startup timeout.
### Fix Focus Areas
- Add a `this._cleanupTimeout` field and assign the result of `setTimeout()`.
- In `shutdown()`, `clearTimeout(this._cleanupTimeout)` and set it to `null`.
- Consider calling `.unref()` on the startup timeout handle as well.
- src/node/db/SessionStore.ts[45-61]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

8. Misleading stale-age documentation ☑ 🐞 Bug ⚙ Maintainability

Description

The comment/docstring claim stale sessions are determined by “haven't been touched in
STALE_SESSION_MAX_AGE_MS”, but _cleanup() never uses STALE_SESSION_MAX_AGE_MS or any age/touch time
when deciding to remove non-expiring sessions. This mismatch is misleading for maintenance and
suggests incorrect behavior to future readers.

Code

src/node/db/SessionStore.ts[R12-16]

+// Sessions without an expiry date older than this are considered stale and will be cleaned up.
+const STALE_SESSION_MAX_AGE_MS = 30 * 24 * 60 * 60 * 1000; // 30 days
+
+// How often to run the cleanup of expired/stale sessions.
+const CLEANUP_INTERVAL_MS = 60 * 60 * 1000; // 1 hour

Evidence

STALE_SESSION_MAX_AGE_MS is introduced and referenced in the _cleanup() docstring, but the
implementation only checks whether the session has keys besides 'cookie' and does not consult any
timestamp or age threshold.

src/node/db/SessionStore.ts[12-16]
src/node/db/SessionStore.ts[63-66]
src/node/db/SessionStore.ts[86-97]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The code comments/docstring describe age-based stale cleanup using `STALE_SESSION_MAX_AGE_MS`, but the implementation does not use that constant or any age/touch timestamp.
### Issue Context
This is primarily a maintainability/documentation correctness issue that can mislead future changes.
### Fix Focus Areas
- Either remove `STALE_SESSION_MAX_AGE_MS` and update docstrings/comments to describe the actual behavior (cookie-only sessions), OR implement the intended age-based logic if a reliable timestamp can be derived/stored.
- src/node/db/SessionStore.ts[12-16]
- src/node/db/SessionStore.ts[63-66]
- src/node/db/SessionStore.ts[86-97]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more