Skip to content

fix: Add deleteSession safety guard and extend file tracking for session#232

Open
skargaklop wants to merge 1 commit into
vakovalskii:mainfrom
skargaklop:main
Open

fix: Add deleteSession safety guard and extend file tracking for session#232
skargaklop wants to merge 1 commit into
vakovalskii:mainfrom
skargaklop:main

Conversation

@skargaklop

Copy link
Copy Markdown

Pull Request: Critical Bug Fixes for Session Caching and Safety Guards in deleteSession()

1. Overview

RIP my claude code sessions DB...
This pull request addresses two severe issues discovered in the codbash session manager:

  1. Project/Database Deletion Bug: Under certain circumstances (e.g., malformed trailing-slash requests like /api/session/ or undefined values resolving to ""), a session deletion request recursively deleted the entire project directory along with all other sessions inside it, rather than just the targeted session.
  2. Infinite Session Cache/Refresh Stall: Active session updates and additions (especially on Windows) were ignored, resulting in the sessions list failing to refresh for days.

2. Source of the Problem & Investigation

Bug A: Directory Deletion in deleteSession()

  • The Root Cause: In codbash/src/server.js, the /api/session/:id DELETE endpoint splits the pathname to extract the sessionId:
    const sessionId = pathname.split('/').pop();
    If the endpoint is called with a trailing slash (e.g., /api/session/), .pop() returns an empty string ("").
  • Path-Join Fallacy: This empty sessionId was passed to deleteSession(sessionId, project) in codbash/src/data.js, resolving the companion directory path as:
    const sessionDir = path.join(PROJECTS_DIR, projectKey, sessionId);
    Since sessionId was "", path.join resolved this directly to PROJECTS_DIR/projectKey (the entire project directory!).
  • Destructive Command: Because the project directory exists, the guard fs.existsSync(sessionDir) && fs.statSync(sessionDir).isDirectory() evaluated to true, leading to:
    fs.rmSync(sessionDir, { recursive: true });
    This immediately deleted the entire project directory along with all its session records.

Bug B: Caching / Refresh Failure on Windows & Other Databases

  • The mtime Fallacy: The extended caching logic in loadSessions() bypasses re-scans if _sessionsNeedRescan() returns false. However, _sessionsNeedRescan() checked for directory-level modification times (mtimes) under PROJECTS_DIR. On Windows (and most other OS filesystems), appending or writing to an existing file inside a directory does not update its parent directory's mtime. Thus, continued sessions were completely missed.
  • Omitted Databases: The rescan checker completely ignored modifications of other supported platforms (Cursor's state.vscdb, OpenCode's opencode.db, Kilo's kilo.db, Kiro's data.sqlite3, and Qwen sessions), leaving the cache stalled forever even if these databases were modified.

3. The Fixes Applied

Fix A: Safety Guard in deleteSession() (src/data.js)

We introduced a strict validation guard at the top of deleteSession to prevent operations on empty, blank, or traversal-sensitive session IDs:

if (!sessionId || typeof sessionId !== 'string' || sessionId.trim() === '' || sessionId === '.' || sessionId === '..') {
  throw new Error('Invalid session ID specified for deletion');
}

Fix B: Fine-Grained File-Level Change & DB Tracking (src/data.js)

  1. File-Level Monitoring: We updated both _sessionsNeedRescan() and _updateScanMarkers() to track individual .jsonl session files inside each project folder, checking both mtimeMs and size to immediately invalidate the cache when an existing session is appended to.
  2. Multi-Agent DB Tracking: Added active tracking for:
    • Cursor: CURSOR_GLOBAL_DB (state.vscdb) and CURSOR_PROJECTS directory.
    • OpenCode: OPENCODE_DB and EXTRA_OPENCODE_DBS.
    • Kilo: KILO_DB and EXTRA_KILO_DBS.
    • Kiro: KIRO_DB and EXTRA_KIRO_DBS.
    • Qwen: All files discovered by listQwenSessionFiles().

4. Verification & Testing

We created a new regression-testing suite under codbash/test/delete-safety.test.js to assert the behavior of both fixes.

Test Coverage:

  1. Deletion Safety: Asserts that deleteSession throws a validation error for "", null, undefined, " ", ., and .. to prevent directory traversal or parent folder deletions.
  2. Rescan Cache Sensitivity: Asserts that cache-invalidation mechanisms are sensitive to updates.

Running the test suite verified all assertions pass successfully:

node --test codbash/test/delete-safety.test.js
✔ deleteSession throws an error on empty or invalid sessionId to prevent directory destruction (1.2006ms)
✔ _sessionsNeedRescan is sensitive to file-level changes and other agent databases (33.2578ms)
ℹ tests 2
ℹ suites 0
ℹ pass 2
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 252.9026

rescan

The deleteSession function now validates sessionId to prevent accidental
directory destruction from empty or traversal values like '.' and '..'.
Also tracks individual .jsonl files in project dirs and additional agent
databases (Cursor, Opencode, Kilo, Kiro, Qwen) for more accurate rescan
detection.
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