Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions openspec/changes/crash-if-session-dir-missing/.openspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-06-04
38 changes: 38 additions & 0 deletions openspec/changes/crash-if-session-dir-missing/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
## Context

`saveSession()` in `src/session/saver.js` currently calls `stat()` + `mkdir()` on every save call. Since every message triggers a save, this means redundant filesystem checks and directory creation fire N times during a session. The desired behavior: ensure/verify the directory once at init, then `saveSession()` is a pure `writeFile` with no extra checks.

## Goals / Non-Goals

**Goals:**
- Create `ensureSessionsDir()` in `src/session/index.js` that calls `mkdir(dir, { recursive: true })` — used once at init.
- Call `ensureSessionsDir()` exactly once in `index.js` after config load, before agent/tool creation.
- Simplify `saveSession()` in `src/session/saver.js` to a pure `writeFile` — no `stat`, no `mkdir`. If the write fails, the unhandled error crashes.
- Shutdown handler propagates `saveSession()` errors as fatal (process exits with code 1).

**Non-Goals:**
- Adding retry or backoff for transient write failures.
- Changes to `writeMemoryFile()` (different code path).
- Changes to checkpoint/SQLite persistence behavior.

## Decisions

**Decision: `saveSession()` has no filesystem checks**
- `saveSession()` only resolves the output path and calls `writeFile()`. No `stat()`, no `mkdir()`.
- Rationale: `ensureSessionsDir()` at init guarantees the directory exists. If it later disappears, `writeFile()` throws naturally — the crash is the signal.

**Decision: `ensureSessionsDir()` is called exactly once**
- Called in `index.js` right after config load, before agent/tool initialization.
- Uses `mkdir(dir, { recursive: true })` to handle both "already exists" and "needs creation" in a single call.

**Decision: handling runtime write failures**
- `handleShutdown()` in `shutdown.js` catches errors from all shutdown steps. Change to re-throw `saveSession` errors.
- The TUI `onSaveSession` callback in `index.js:223` needs try/catch → `process.exit(1)`.

## Risks / Trade-offs

[Risk] If a file write fails on a non-directory issue (disk full, permissions), the crash provides no differentiation.
→ [Mitigation] Acceptable — the process is crashing anyway and the error message includes the underlying OS error.

[Risk] `writeMemoryFile()` in `index.js:135` still creates directories via the memory module.
→ [Mitigation] Out of scope. `saveSession` is the function this change targets.
22 changes: 22 additions & 0 deletions openspec/changes/crash-if-session-dir-missing/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
## Why

The session saver currently calls `mkdir(dir, { recursive: true })` on every save call. This means directory creation happens N times (once per conversation message) instead of once at init. After init, if the directory disappears, the agent continues running with no persistence but no signal either.

## What Changes

- Move the `mkdir(dir, { recursive: true })` call from `saveSession()` to a single initialization function (`ensureSessionsDir`) that runs once at app startup in `index.js`. After a successful init, the directory is considered guaranteed.
- If the directory disappears after successful init, `saveSession()` no longer silently creates it — it throws, and the shutdown handler treats this as fatal (crash).
- `saveSession()` becomes a pure write: it validates the directory exists via `stat()` and writes the file, with no mkdir fallback.

## Capabilities

### Modified Capabilities
- `session-management`: Shutdown and cleanup now requires the sessions directory to exist; missing directory is a crash condition.

## Impact

- `src/session/saver.js` — remove `mkdir`, replace with `stat()` validation (pure write)
- `src/session/index.js` — add `ensureSessionsDir()` for one-time init check/create
- `src/index.js` — call `ensureSessionsDir()` at startup once
- `src/session/shutdown.js` — re-throw `saveSession` errors as fatal
- `tests/unit/saver.test.js` — update tests for new failure semantics; add init-ensure test
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
## ADDED Requirements

### Requirement: Sessions Directory Ensured at Init
The system SHALL create the sessions directory (`memory/sessions/`) exactly once during application initialization via `ensureSessionsDir()`. This function SHALL use `mkdir` with the recursive option to create the directory if it does not exist. If it already exists, the function SHALL return without error.

#### Scenario: Directory exists at init
- **WHEN** `ensureSessionsDir()` is called and `memory/sessions/` already exists
- **THEN** the function returns successfully without creating the directory

#### Scenario: Directory does not exist at init
- **WHEN** `ensureSessionsDir()` is called and `memory/sessions/` does not exist
- **THEN** the function creates the directory and returns successfully

### Requirement: SaveSession Is Pure Write
The `saveSession()` function SHALL NOT perform any filesystem checks or directory creation. It SHALL resolve the output path, build the session file content, and call `writeFile()` directly. If the underlying filesystem operation fails for any reason — including a missing directory, disk full, or permission error — the error SHALL propagate unhandled.

#### Scenario: Save succeeds when directory exists
- **WHEN** `saveSession()` is called after successful init
- **THEN** the function writes the session file and returns

#### Scenario: Save fails and crashes when directory is missing
- **WHEN** `saveSession()` is called and the sessions directory no longer exists
- **THEN** `writeFile()` throws an error
- **THEN** the error propagates and crashes the process

## MODIFIED Requirements

### Requirement: Session Shutdown and Cleanup
On session termination, the system SHALL flush all pending telemetry spans, close file handles on memory files, and write a final conversation state to `memory/`. The shutdown process SHALL treat `saveSession()` errors as fatal and exit with code 1. The process SHALL NOT silently ignore save failures.

#### Scenario: Session flushes telemetry on exit
- **WHEN** the user exits the TUI (`Ctrl+C` or `:quit`)
- **THEN** the system signals the OpenTelemetry provider to export all pending spans and waits for completion

#### Scenario: Session writes final memory state
- **WHEN** the session terminates
- **THEN** the system appends any remaining unsaved conversation exchanges to the latest memory file

#### Scenario: Save error during shutdown causes fatal exit
- **WHEN** the session terminates and `saveSession()` throws
- **THEN** the shutdown handler propagates the error and exits with code 1 instead of silently ignoring the failure
28 changes: 28 additions & 0 deletions openspec/changes/crash-if-session-dir-missing/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
## 1. Add ensureSessionsDir at init

- [ ] 1.1 Create `ensureSessionsDir(sessionsDir)` function in `src/session/index.js` that resolves the path via `join(process.cwd(), sessionsDir)` and calls `mkdir(dir, { recursive: true })`
- [ ] 1.2 Export `ensureSessionsDir` from `src/session/index.js`
- [ ] 1.3 In `index.js`, call `ensureSessionsDir("memory/sessions/")` once after config load, before agent/tool creation

## 2. Simplify saveSession to pure writeFile

- [ ] 2.1 In `src/session/saver.js`, remove the `try { await stat(dir) } catch { await mkdir(...) }` block (lines 13-17)
- [ ] 2.2 Remove `mkdir` and `stat` from the import from `node:fs/promises`
- [ ] 2.3 Add `@throws {Error}` JSDoc to `saveSession()` documenting that unhandled filesystem errors propagate

## 3. Update shutdown handler to propagate fatal errors

- [ ] 3.1 In `src/session/shutdown.js`, modify `handleShutdown()` so that `saveSession()` errors are not caught — narrow the try/catch to only surround `flushTelemetry()`
- [ ] 3.2 Keep telemetry flush errors suppressed (log but don't throw) but let session save errors propagate

## 4. Update TUI save callback for fatal errors

- [ ] 4.1 In `index.js`, wrap the `onSaveSession` callback (line 223) in try/catch and call `process.exit(1)` on error

## 5. Write and update tests

- [ ] 5.1 Add test: `ensureSessionsDir` creates directory when missing (using a temp directory)
- [ ] 5.2 Add test: `ensureSessionsDir` returns successfully when directory already exists
- [ ] 5.3 Add test: `saveSession` writes file successfully when directory exists (existing behavior)
- [ ] 5.4 Add test: `saveSession` propagates writeFile errors unhandled
- [ ] 5.5 Add test: `handleShutdown` re-throws errors from `saveSession`