Skip to content

fix: persistence safety — resume status, test isolation, write race - #134

Merged
aterrylu merged 1 commit into
mainfrom
terry/persistence-safety
Apr 14, 2026
Merged

fix: persistence safety — resume status, test isolation, write race#134
aterrylu merged 1 commit into
mainfrom
terry/persistence-safety

Conversation

@aterrylu

Copy link
Copy Markdown
Owner

Summary

Three critical persistence fixes that caused data loss in production:

  • Resume status bug: createSession() now explicitly sets status: "running" when calling persistSession(), so resumed sessions clear their stale "exited" status instead of preserving it via the ?? merge fallback
  • Test isolation: All persistence tests now use isolated temp directories via _setConfigDirForTesting() instead of writing to production ~/.autonomos/sessions.json (which corrupted real agent data)
  • Write race: Added in-memory cache to persisted.ts — eliminates TOCTOU races where concurrent markSessionExited() and persistSession() calls could overwrite each other. Added backup-on-shrink before destructive writes

Changes

File Change
sessions.ts Add status: "running" to persistSession() call
configDir.ts Add getConfigDir() with test override support
persisted.ts In-memory cache, dynamic file path, backup-on-shrink, defensive copy on getPersistedSessions()
backward-compat.test.ts Use isolated temp directory
orgChart.test.ts Use isolated temp directory
duplicate-name.test.ts Use isolated temp directory for resume tests

Architecture

graph TD
    A[createSession] -->|status: running| B[persistSession]
    C[markSessionExited] --> D[readSessions - cached]
    B --> D
    D -->|first call| E[loadFromDisk]
    D -->|subsequent| F[return sessionsCache]
    B --> G[writeSessions]
    C --> G
    G -->|count decreased| H[copyFileSync backup]
    G --> I[writeFileSync]
    I --> J[sessionsCache = sessions]
Loading

Test plan

  • All 85 tests pass (make check)
  • Tests use isolated temp dirs — production sessions.json not modified
  • status: "running" explicitly set on session creation/resume
  • In-memory cache prevents concurrent write races
  • Backup created before writes that reduce session count
  • getPersistedSessions() returns defensive copy

🤖 Generated with Claude Code

Three critical fixes for production data loss:

1. Resume doesn't clear exited status: createSession() now passes
   status: "running" to persistSession(), so the merge logic no
   longer preserves the stale "exited" status on resume.

2. Test isolation: Tests from PR #131 wrote to production
   sessions.json. All test files now use isolated temp directories
   via _setConfigDirForTesting() / _resetCacheForTesting(). Added
   getConfigDir() with test override support to configDir.ts.

3. Write race on sessions.json: Added in-memory cache to
   persisted.ts — readSessions() populates once, all mutations go
   through the cached array, eliminating TOCTOU races. Added
   backup-on-shrink before destructive writes. getPersistedSessions()
   returns a defensive copy to prevent external cache corruption.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@aterrylu
aterrylu force-pushed the terry/persistence-safety branch from 1bc9c29 to b67aae8 Compare April 14, 2026 08:15
@aterrylu
aterrylu marked this pull request as ready for review April 14, 2026 08:15

@nox-0x nox-0x left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good, thorough fix. The in-memory cache is the right approach for the TOCTOU race, and explicitly setting status: "running" closes the resume bug cleanly. Test isolation to temp dirs is much safer — good call.

@aterrylu
aterrylu merged commit ef66d1a into main Apr 14, 2026
1 check passed
@aterrylu
aterrylu deleted the terry/persistence-safety branch April 14, 2026 08:25
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.

2 participants