* docs: add investigation reports for 5 open GitHub issues Comprehensive analysis of issues #543, #544, #545, #555, and #557: - #557: settings.json not generated, module loader error (node/bun mismatch) - #555: Windows hooks not executing, hasIpc always false - #545: formatTool crashes on non-JSON tool_input strings - #544: mem-search skill hint shown incorrectly to Claude Code users - #543: /claude-mem slash command unavailable despite installation Each report includes root cause analysis, affected files, and proposed fixes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(logger): handle non-JSON tool_input in formatTool (#545) Wrap JSON.parse in try-catch to handle raw string inputs (e.g., Bash commands) that aren't valid JSON. Falls back to using the string as-is. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(context): update mem-search hint to reference MCP tools (#544) Update hint messages to reference MCP tools (search, get_observations) instead of the deprecated "mem-search skill" terminology. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(settings): auto-create settings.json on first load (#557, #543) When settings.json doesn't exist, create it with defaults instead of returning in-memory defaults. Creates parent directory if needed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(hooks): use bun runtime for hooks except smart-install (#557) Change hook commands from node to bun since hooks use bun:sqlite. Keep smart-install.js on node since it bootstraps bun installation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore: rebuild plugin scripts * docs: clarify that build artifacts must be committed * fix(docs): update build artifacts directory reference in CLAUDE.md * test: add test coverage for PR #558 fixes - Fix 2 failing tests: update "mem-search skill" → "MCP tools" expectations - Add 56 tests for formatTool() JSON.parse crash fix (Issue #545) - Add 27 tests for settings.json auto-creation (Issue #543) Test coverage includes: - formatTool: JSON parsing, raw strings, objects, null/undefined, all tool types - Settings: file creation, directory creation, schema migration, edge cases 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(tests): clean up flaky tests and fix circular dependency Phase 1 of test quality improvements: - Delete 6 harmful/worthless test files that used problematic mock.module() patterns or tested implementation details rather than behavior: - context-builder.test.ts (tested internal implementation) - export-types.test.ts (fragile mock patterns) - smart-install.test.ts (shell script testing antipattern) - session_id_refactor.test.ts (outdated, tested refactoring itself) - validate_sql_update.test.ts (one-time migration validation) - observation-broadcaster.test.ts (excessive mocking) - Fix circular dependency between logger.ts and SettingsDefaultsManager.ts by using late binding pattern - logger now lazily loads settings - Refactor mock.module() to spyOn() in several test files for more maintainable and less brittle tests: - observation-compiler.test.ts - gemini_agent.test.ts - error-handler.test.ts - server.test.ts - response-processor.test.ts All 649 tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor(tests): phase 2 - reduce mock-heavy tests and improve focus - Remove mock-heavy query tests from observation-compiler.test.ts, keep real buildTimeline tests - Convert session_id_usage_validation.test.ts from 477 to 178 lines of focused smoke tests - Remove tests for language built-ins from worker-spawn.test.ts (JSON.parse, array indexing) - Rename logger-coverage.test.ts to logger-usage-standards.test.ts for clarity 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs(tests): phase 3 - add JSDoc mock justification to test files Document mock usage rationale in 5 test files to improve maintainability: - error-handler.test.ts: Express req/res mocks, logger spies (~11%) - fallback-error-handler.test.ts: Zero mocks, pure function tests - session-cleanup-helper.test.ts: Session fixtures, worker mocks (~19%) - hook-constants.test.ts: process.platform mock for Windows tests (~12%) - session_store.test.ts: Zero mocks, real SQLite :memory: database Part of ongoing effort to document mock justifications per TESTING.md guidelines. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test(integration): phase 5 - add 72 tests for critical coverage gaps Add comprehensive test coverage for previously untested areas: - tests/integration/hook-execution-e2e.test.ts (10 tests) Tests lifecycle hooks execution flow and context propagation - tests/integration/worker-api-endpoints.test.ts (19 tests) Tests all worker service HTTP endpoints without heavy mocking - tests/integration/chroma-vector-sync.test.ts (16 tests) Tests vector embedding synchronization with ChromaDB - tests/utils/tag-stripping.test.ts (27 tests) Tests privacy tag stripping utilities for both <private> and <meta-observation> tags All tests use real implementations where feasible, following the project's testing philosophy of preferring integration-style tests over unit tests with extensive mocking. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * context update * docs: add comment linking DEFAULT_DATA_DIR locations Added NOTE comment in logger.ts pointing to the canonical DEFAULT_DATA_DIR in SettingsDefaultsManager.ts. This addresses PR reviewer feedback about the fragility of having the default defined in two places to avoid circular dependencies. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Alex Newman
@@ -2,22 +2,18 @@ import { describe, it, expect, beforeEach, afterEach } from 'bun:test';
|
||||
import { SessionStore } from '../src/services/sqlite/SessionStore.js';
|
||||
|
||||
/**
|
||||
* Session ID Usage Validation Tests
|
||||
* Session ID Usage Validation - Smoke Tests for Critical Invariants
|
||||
*
|
||||
* PURPOSE: Prevent confusion and bugs from mixing contentSessionId and memorySessionId
|
||||
*
|
||||
* CRITICAL ARCHITECTURE:
|
||||
* These tests validate the most critical behaviors of the dual session ID system:
|
||||
* - contentSessionId: User's Claude Code conversation session (immutable)
|
||||
* - memorySessionId: SDK agent's session ID for resume (captured from SDK response)
|
||||
*
|
||||
* INVARIANTS TO ENFORCE:
|
||||
* 1. memorySessionId starts as NULL (NEVER equals contentSessionId - that would inject memory into user transcript!)
|
||||
* 2. Resume MUST NOT be used when memorySessionId is NULL
|
||||
* 3. Resume MUST ONLY be used when hasRealMemorySessionId === true (memorySessionId is non-null)
|
||||
* 4. Observations are stored with memorySessionId (after updateMemorySessionId has been called)
|
||||
* 5. updateMemorySessionId() is required before storeObservation() or storeSummary() can work
|
||||
* CRITICAL INVARIANTS:
|
||||
* 1. Cross-contamination prevention: Observations from different sessions never mix
|
||||
* 2. Resume safety: Resume only allowed when memorySessionId is actually captured (non-NULL)
|
||||
* 3. 1:1 mapping: Each contentSessionId maps to exactly one memorySessionId
|
||||
*/
|
||||
describe('Session ID Usage Validation', () => {
|
||||
describe('Session ID Critical Invariants', () => {
|
||||
let store: SessionStore;
|
||||
|
||||
beforeEach(() => {
|
||||
@@ -28,164 +24,9 @@ describe('Session ID Usage Validation', () => {
|
||||
store.close();
|
||||
});
|
||||
|
||||
describe('Placeholder Detection - hasRealMemorySessionId Logic', () => {
|
||||
it('should identify placeholder when memorySessionId is NULL', () => {
|
||||
const contentSessionId = 'user-session-123';
|
||||
const sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Test prompt');
|
||||
|
||||
const session = store.getSessionById(sessionDbId);
|
||||
|
||||
// Initially, memory_session_id is NULL (placeholder state)
|
||||
// CRITICAL: memory_session_id must NEVER equal contentSessionId - that would inject memory into user transcript!
|
||||
expect(session?.memory_session_id).toBeNull();
|
||||
|
||||
// hasRealMemorySessionId would be FALSE (NULL is falsy)
|
||||
const hasRealMemorySessionId = session?.memory_session_id !== null;
|
||||
expect(hasRealMemorySessionId).toBe(false);
|
||||
});
|
||||
|
||||
it('should identify real memory session ID after capture', () => {
|
||||
const contentSessionId = 'user-session-456';
|
||||
const capturedMemoryId = 'sdk-generated-abc123';
|
||||
|
||||
const sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Test prompt');
|
||||
store.updateMemorySessionId(sessionDbId, capturedMemoryId);
|
||||
|
||||
const session = store.getSessionById(sessionDbId);
|
||||
|
||||
// After capture, memory_session_id is set (non-NULL)
|
||||
expect(session?.memory_session_id).toBe(capturedMemoryId);
|
||||
|
||||
// hasRealMemorySessionId would be TRUE
|
||||
const hasRealMemorySessionId = session?.memory_session_id !== null;
|
||||
expect(hasRealMemorySessionId).toBe(true);
|
||||
});
|
||||
|
||||
it('should never use contentSessionId as resume parameter when in placeholder state', () => {
|
||||
const contentSessionId = 'dangerous-session-789';
|
||||
const sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Test');
|
||||
|
||||
const session = store.getSessionById(sessionDbId);
|
||||
const hasRealMemorySessionId = session?.memory_session_id !== null;
|
||||
|
||||
// CRITICAL: This check prevents resuming when memory_session_id is not captured
|
||||
if (hasRealMemorySessionId) {
|
||||
// Safe to use for resume
|
||||
const resumeParam = session?.memory_session_id;
|
||||
expect(resumeParam).not.toBe(contentSessionId);
|
||||
} else {
|
||||
// Must NOT pass resume parameter
|
||||
// Resume should be undefined/null in SDK call
|
||||
expect(hasRealMemorySessionId).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('Observation Storage - MemorySessionId Usage', () => {
|
||||
it('should store observations with memorySessionId in memory_session_id column', () => {
|
||||
const contentSessionId = 'obs-content-session-123';
|
||||
const memorySessionId = 'obs-memory-session-123';
|
||||
const sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Test');
|
||||
store.updateMemorySessionId(sessionDbId, memorySessionId);
|
||||
|
||||
const obs = {
|
||||
type: 'discovery',
|
||||
title: 'Test Observation',
|
||||
subtitle: null,
|
||||
facts: ['Fact 1'],
|
||||
narrative: 'Testing',
|
||||
concepts: ['testing'],
|
||||
files_read: [],
|
||||
files_modified: []
|
||||
};
|
||||
|
||||
// storeObservation takes memorySessionId (after updateMemorySessionId has been called)
|
||||
const result = store.storeObservation(memorySessionId, 'test-project', obs, 1);
|
||||
|
||||
// Verify it's stored in the memory_session_id column with memorySessionId value
|
||||
const stored = store.db.prepare(
|
||||
'SELECT memory_session_id FROM observations WHERE id = ?'
|
||||
).get(result.id) as { memory_session_id: string };
|
||||
|
||||
// memory_session_id column contains the captured SDK session ID
|
||||
expect(stored.memory_session_id).toBe(memorySessionId);
|
||||
});
|
||||
|
||||
it('should be retrievable using memorySessionId', () => {
|
||||
const contentSessionId = 'retrieval-test-session';
|
||||
const memorySessionId = 'retrieval-memory-session';
|
||||
|
||||
const sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Test');
|
||||
store.updateMemorySessionId(sessionDbId, memorySessionId);
|
||||
|
||||
// Store observation with memorySessionId
|
||||
const obs = {
|
||||
type: 'feature',
|
||||
title: 'Observation',
|
||||
subtitle: null,
|
||||
facts: [],
|
||||
narrative: null,
|
||||
concepts: [],
|
||||
files_read: [],
|
||||
files_modified: []
|
||||
};
|
||||
store.storeObservation(memorySessionId, 'test-project', obs, 1);
|
||||
|
||||
// Observations are retrievable by memorySessionId
|
||||
const observations = store.getObservationsForSession(memorySessionId);
|
||||
expect(observations.length).toBe(1);
|
||||
expect(observations[0].title).toBe('Observation');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Resume Safety - Prevent contentSessionId Resume Bug', () => {
|
||||
it('should prevent resume with NULL memorySessionId', () => {
|
||||
const contentSessionId = 'safety-test-session';
|
||||
const sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Test');
|
||||
|
||||
const session = store.getSessionById(sessionDbId);
|
||||
|
||||
// Simulate hasRealMemorySessionId check - memory_session_id must be non-null
|
||||
const hasRealMemorySessionId = session?.memory_session_id !== null;
|
||||
|
||||
// MUST be false in placeholder state (memory_session_id is NULL)
|
||||
expect(hasRealMemorySessionId).toBe(false);
|
||||
|
||||
// Resume parameter should NOT be set
|
||||
// In SDK call: ...(hasRealMemorySessionId && { resume: session.memorySessionId })
|
||||
// This evaluates to an empty object, not a resume parameter
|
||||
const resumeOptions = hasRealMemorySessionId ? { resume: session?.memory_session_id } : {};
|
||||
expect(resumeOptions).toEqual({});
|
||||
});
|
||||
|
||||
it('should allow resume only after memory session ID is captured', () => {
|
||||
const contentSessionId = 'resume-ready-session';
|
||||
const capturedMemoryId = 'real-sdk-session-123';
|
||||
|
||||
const sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Test');
|
||||
|
||||
// Before capture - no resume (memory_session_id is NULL)
|
||||
let session = store.getSessionById(sessionDbId);
|
||||
let hasRealMemorySessionId = session?.memory_session_id !== null;
|
||||
expect(hasRealMemorySessionId).toBe(false);
|
||||
|
||||
// Capture memory session ID
|
||||
store.updateMemorySessionId(sessionDbId, capturedMemoryId);
|
||||
|
||||
// After capture - resume allowed
|
||||
session = store.getSessionById(sessionDbId);
|
||||
hasRealMemorySessionId = session?.memory_session_id !== null;
|
||||
expect(hasRealMemorySessionId).toBe(true);
|
||||
|
||||
// Resume parameter should be the captured ID
|
||||
const resumeOptions = hasRealMemorySessionId ? { resume: session?.memory_session_id } : {};
|
||||
expect(resumeOptions).toEqual({ resume: capturedMemoryId });
|
||||
expect(resumeOptions.resume).not.toBe(contentSessionId);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Cross-Contamination Prevention', () => {
|
||||
it('should never mix observations from different content sessions', () => {
|
||||
// Create two independent sessions
|
||||
const content1 = 'user-session-A';
|
||||
const content2 = 'user-session-B';
|
||||
const memory1 = 'memory-session-A';
|
||||
@@ -196,7 +37,7 @@ describe('Session ID Usage Validation', () => {
|
||||
store.updateMemorySessionId(id1, memory1);
|
||||
store.updateMemorySessionId(id2, memory2);
|
||||
|
||||
// Store observations in each session using memorySessionId
|
||||
// Store observations in each session
|
||||
store.storeObservation(memory1, 'project-a', {
|
||||
type: 'discovery',
|
||||
title: 'Observation A',
|
||||
@@ -219,7 +60,7 @@ describe('Session ID Usage Validation', () => {
|
||||
files_modified: []
|
||||
}, 1);
|
||||
|
||||
// Verify isolation
|
||||
// CRITICAL: Each session's observations must be isolated
|
||||
const obsA = store.getObservationsForSession(memory1);
|
||||
const obsB = store.getObservationsForSession(memory2);
|
||||
|
||||
@@ -227,145 +68,76 @@ describe('Session ID Usage Validation', () => {
|
||||
expect(obsB.length).toBe(1);
|
||||
expect(obsA[0].title).toBe('Observation A');
|
||||
expect(obsB[0].title).toBe('Observation B');
|
||||
});
|
||||
|
||||
it('should never leak memory session IDs between content sessions', () => {
|
||||
const content1 = 'content-session-1';
|
||||
const content2 = 'content-session-2';
|
||||
const memory1 = 'memory-session-1';
|
||||
const memory2 = 'memory-session-2';
|
||||
|
||||
const id1 = store.createSDKSession(content1, 'project', 'Prompt');
|
||||
const id2 = store.createSDKSession(content2, 'project', 'Prompt');
|
||||
|
||||
store.updateMemorySessionId(id1, memory1);
|
||||
store.updateMemorySessionId(id2, memory2);
|
||||
|
||||
const session1 = store.getSessionById(id1);
|
||||
const session2 = store.getSessionById(id2);
|
||||
|
||||
// Each session must have its own unique memory session ID
|
||||
expect(session1?.memory_session_id).toBe(memory1);
|
||||
expect(session2?.memory_session_id).toBe(memory2);
|
||||
expect(session1?.memory_session_id).not.toBe(session2?.memory_session_id);
|
||||
// Verify no cross-contamination: A's query doesn't return B's data
|
||||
expect(obsA.some(o => o.title === 'Observation B')).toBe(false);
|
||||
expect(obsB.some(o => o.title === 'Observation A')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Foreign Key Integrity', () => {
|
||||
it('should cascade delete observations when session is deleted', () => {
|
||||
const contentSessionId = 'cascade-test-session';
|
||||
const memorySessionId = 'cascade-memory-session';
|
||||
const sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Test');
|
||||
store.updateMemorySessionId(sessionDbId, memorySessionId);
|
||||
|
||||
// Store observation
|
||||
const obs = {
|
||||
type: 'discovery',
|
||||
title: 'Will be deleted',
|
||||
subtitle: null,
|
||||
facts: [],
|
||||
narrative: null,
|
||||
concepts: [],
|
||||
files_read: [],
|
||||
files_modified: []
|
||||
};
|
||||
store.storeObservation(memorySessionId, 'test-project', obs, 1);
|
||||
|
||||
// Verify observation exists
|
||||
let observations = store.getObservationsForSession(memorySessionId);
|
||||
expect(observations.length).toBe(1);
|
||||
|
||||
// Delete session (should cascade to observations)
|
||||
store.db.prepare('DELETE FROM sdk_sessions WHERE id = ?').run(sessionDbId);
|
||||
|
||||
// Verify observations were deleted
|
||||
observations = store.getObservationsForSession(memorySessionId);
|
||||
expect(observations.length).toBe(0);
|
||||
});
|
||||
|
||||
it('should maintain FK relationship between observations and sessions', () => {
|
||||
const contentSessionId = 'fk-test-session';
|
||||
const memorySessionId = 'fk-memory-session';
|
||||
const sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Test');
|
||||
store.updateMemorySessionId(sessionDbId, memorySessionId);
|
||||
|
||||
// This should succeed (FK exists)
|
||||
expect(() => {
|
||||
store.storeObservation(memorySessionId, 'test-project', {
|
||||
type: 'discovery',
|
||||
title: 'Valid FK',
|
||||
subtitle: null,
|
||||
facts: [],
|
||||
narrative: null,
|
||||
concepts: [],
|
||||
files_read: [],
|
||||
files_modified: []
|
||||
}, 1);
|
||||
}).not.toThrow();
|
||||
|
||||
// This should fail (FK doesn't exist)
|
||||
expect(() => {
|
||||
store.storeObservation('nonexistent-session-id', 'test-project', {
|
||||
type: 'discovery',
|
||||
title: 'Invalid FK',
|
||||
subtitle: null,
|
||||
facts: [],
|
||||
narrative: null,
|
||||
concepts: [],
|
||||
files_read: [],
|
||||
files_modified: []
|
||||
}, 1);
|
||||
}).toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Session Lifecycle - Memory ID Capture Flow', () => {
|
||||
it('should follow correct lifecycle: create → capture → resume', () => {
|
||||
const contentSessionId = 'lifecycle-session';
|
||||
|
||||
// STEP 1: Hook creates session (memory_session_id = NULL)
|
||||
describe('Resume Safety', () => {
|
||||
it('should prevent resume when memorySessionId is NULL (not yet captured)', () => {
|
||||
const contentSessionId = 'new-session-123';
|
||||
const sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'First prompt');
|
||||
let session = store.getSessionById(sessionDbId);
|
||||
expect(session?.memory_session_id).toBeNull(); // NULL - not captured yet
|
||||
|
||||
// STEP 2: First SDK message arrives with real session ID
|
||||
const realMemoryId = 'sdk-generated-session-xyz';
|
||||
store.updateMemorySessionId(sessionDbId, realMemoryId);
|
||||
session = store.getSessionById(sessionDbId);
|
||||
expect(session?.memory_session_id).toBe(realMemoryId); // Real ID
|
||||
|
||||
// STEP 3: Subsequent prompts can now resume
|
||||
const hasRealMemorySessionId = session?.memory_session_id !== null;
|
||||
expect(hasRealMemorySessionId).toBe(true);
|
||||
|
||||
// Resume parameter is safe to use
|
||||
const resumeParam = session?.memory_session_id;
|
||||
expect(resumeParam).toBe(realMemoryId);
|
||||
expect(resumeParam).not.toBe(contentSessionId);
|
||||
});
|
||||
|
||||
it('should handle worker restart by preserving captured memory session ID', () => {
|
||||
const contentSessionId = 'restart-test-session';
|
||||
const capturedMemoryId = 'persisted-memory-id';
|
||||
|
||||
// Simulate first worker instance
|
||||
const sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Prompt');
|
||||
store.updateMemorySessionId(sessionDbId, capturedMemoryId);
|
||||
|
||||
// Simulate worker restart - session re-fetched from database
|
||||
const session = store.getSessionById(sessionDbId);
|
||||
|
||||
// Memory session ID should be preserved
|
||||
expect(session?.memory_session_id).toBe(capturedMemoryId);
|
||||
// CRITICAL: Before SDK returns real session ID, memory_session_id must be NULL
|
||||
expect(session?.memory_session_id).toBeNull();
|
||||
|
||||
// Resume can work immediately
|
||||
// hasRealMemorySessionId check: only resume when non-NULL
|
||||
const hasRealMemorySessionId = session?.memory_session_id !== null;
|
||||
expect(hasRealMemorySessionId).toBe(false);
|
||||
|
||||
// Resume options should be empty (no resume parameter)
|
||||
const resumeOptions = hasRealMemorySessionId ? { resume: session?.memory_session_id } : {};
|
||||
expect(resumeOptions).toEqual({});
|
||||
});
|
||||
|
||||
it('should allow resume only after memorySessionId is captured', () => {
|
||||
const contentSessionId = 'resume-ready-session';
|
||||
const capturedMemoryId = 'sdk-returned-session-xyz';
|
||||
|
||||
const sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Prompt');
|
||||
|
||||
// Before capture
|
||||
let session = store.getSessionById(sessionDbId);
|
||||
expect(session?.memory_session_id).toBeNull();
|
||||
|
||||
// Capture memory session ID (simulates SDK response)
|
||||
store.updateMemorySessionId(sessionDbId, capturedMemoryId);
|
||||
|
||||
// After capture
|
||||
session = store.getSessionById(sessionDbId);
|
||||
const hasRealMemorySessionId = session?.memory_session_id !== null;
|
||||
|
||||
expect(hasRealMemorySessionId).toBe(true);
|
||||
expect(session?.memory_session_id).toBe(capturedMemoryId);
|
||||
expect(session?.memory_session_id).not.toBe(contentSessionId);
|
||||
});
|
||||
|
||||
it('should maintain consistent memorySessionId across multiple prompts in same conversation', () => {
|
||||
const contentSessionId = 'multi-prompt-session';
|
||||
const realMemoryId = 'consistent-memory-id';
|
||||
|
||||
// Prompt 1: Create session
|
||||
let sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Prompt 1');
|
||||
store.updateMemorySessionId(sessionDbId, realMemoryId);
|
||||
|
||||
// Prompt 2: Look up session (createSDKSession uses INSERT OR IGNORE + SELECT)
|
||||
sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Prompt 2');
|
||||
let session = store.getSessionById(sessionDbId);
|
||||
expect(session?.memory_session_id).toBe(realMemoryId);
|
||||
|
||||
// Prompt 3: Still same memory ID
|
||||
sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Prompt 3');
|
||||
session = store.getSessionById(sessionDbId);
|
||||
expect(session?.memory_session_id).toBe(realMemoryId);
|
||||
});
|
||||
});
|
||||
|
||||
describe('CRITICAL: 1:1 Transcript Mapping Guarantees', () => {
|
||||
it('should enforce UNIQUE constraint on memory_session_id (prevents duplicate memory transcripts)', () => {
|
||||
describe('UNIQUE Constraint Enforcement', () => {
|
||||
it('should prevent duplicate memorySessionId (protects against multiple transcripts)', () => {
|
||||
const content1 = 'content-session-1';
|
||||
const content2 = 'content-session-2';
|
||||
const sharedMemoryId = 'shared-memory-id';
|
||||
@@ -381,130 +153,26 @@ describe('Session ID Usage Validation', () => {
|
||||
store.updateMemorySessionId(id2, sharedMemoryId);
|
||||
}).toThrow(); // UNIQUE constraint violation
|
||||
|
||||
// Verify first session still has the ID
|
||||
// First session still has the ID
|
||||
const session1 = store.getSessionById(id1);
|
||||
expect(session1?.memory_session_id).toBe(sharedMemoryId);
|
||||
});
|
||||
|
||||
it('should prevent memorySessionId from being changed after real capture (single transition guarantee)', () => {
|
||||
const contentSessionId = 'single-capture-test';
|
||||
const firstMemoryId = 'first-sdk-session-id';
|
||||
const secondMemoryId = 'different-sdk-session-id';
|
||||
|
||||
const sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Test');
|
||||
|
||||
// First capture - should succeed
|
||||
store.updateMemorySessionId(sessionDbId, firstMemoryId);
|
||||
|
||||
let session = store.getSessionById(sessionDbId);
|
||||
expect(session?.memory_session_id).toBe(firstMemoryId);
|
||||
|
||||
// Second capture with DIFFERENT ID - should FAIL (or be no-op in proper implementation)
|
||||
// This test documents current behavior - ideally updateMemorySessionId should
|
||||
// check if memorySessionId already differs from contentSessionId and refuse to update
|
||||
store.updateMemorySessionId(sessionDbId, secondMemoryId);
|
||||
|
||||
session = store.getSessionById(sessionDbId);
|
||||
|
||||
// CRITICAL: If this allows the update, we could get multiple memory transcripts!
|
||||
// This test currently shows the vulnerability - in production, SDKAgent.ts
|
||||
// has the check `if (!session.memorySessionId)` which should prevent this,
|
||||
// but the database layer doesn't enforce it.
|
||||
//
|
||||
// For now, we document that the second update DOES go through (current behavior)
|
||||
expect(session?.memory_session_id).toBe(secondMemoryId);
|
||||
|
||||
// TODO: Add database-level protection via CHECK constraint or trigger
|
||||
// to prevent changing memory_session_id once it differs from content_session_id
|
||||
});
|
||||
|
||||
it('should use same memorySessionId for all prompts in a conversation (resume consistency)', () => {
|
||||
const contentSessionId = 'multi-prompt-session';
|
||||
const realMemoryId = 'consistent-memory-id';
|
||||
|
||||
// Prompt 1: Create session
|
||||
let sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Prompt 1');
|
||||
let session = store.getSessionById(sessionDbId);
|
||||
|
||||
// Initially NULL
|
||||
expect(session?.memory_session_id).toBeNull();
|
||||
|
||||
// Prompt 1: Capture real memory ID
|
||||
store.updateMemorySessionId(sessionDbId, realMemoryId);
|
||||
|
||||
// Prompt 2: Look up session by contentSessionId (simulates hook creating session again)
|
||||
sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Prompt 2');
|
||||
session = store.getSessionById(sessionDbId);
|
||||
|
||||
// Should get SAME memory ID (resume with this)
|
||||
expect(session?.memory_session_id).toBe(realMemoryId);
|
||||
|
||||
// Prompt 3: Again, same contentSessionId
|
||||
sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Prompt 3');
|
||||
session = store.getSessionById(sessionDbId);
|
||||
|
||||
// Should STILL get same memory ID
|
||||
expect(session?.memory_session_id).toBe(realMemoryId);
|
||||
|
||||
// All three prompts use the SAME memorySessionId → ONE memory transcript file
|
||||
const hasRealMemorySessionId = session?.memory_session_id !== null;
|
||||
expect(hasRealMemorySessionId).toBe(true);
|
||||
});
|
||||
|
||||
it('should lookup session by contentSessionId and retrieve memorySessionId for resume', () => {
|
||||
const contentSessionId = 'lookup-test-session';
|
||||
const capturedMemoryId = 'memory-for-resume';
|
||||
|
||||
// First prompt: Create and capture
|
||||
const sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'First');
|
||||
store.updateMemorySessionId(sessionDbId, capturedMemoryId);
|
||||
|
||||
// Second prompt: Hook provides contentSessionId, needs to lookup memorySessionId
|
||||
// The createSDKSession method IS the lookup (INSERT OR IGNORE + SELECT)
|
||||
const lookedUpSessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Second');
|
||||
|
||||
// Should be same DB row
|
||||
expect(lookedUpSessionDbId).toBe(sessionDbId);
|
||||
|
||||
// Get session to extract memorySessionId for resume
|
||||
const session = store.getSessionById(lookedUpSessionDbId);
|
||||
const resumeParam = session?.memory_session_id;
|
||||
|
||||
// This is what would be passed to SDK query({ resume: resumeParam })
|
||||
expect(resumeParam).toBe(capturedMemoryId);
|
||||
expect(resumeParam).not.toBe(contentSessionId);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Edge Cases - Session ID Equality', () => {
|
||||
it('should handle case where SDK returns session ID equal to contentSessionId', () => {
|
||||
// Edge case: SDK happens to generate same ID as content session
|
||||
// This shouldn't happen in practice, but we test it anyway
|
||||
const contentSessionId = 'same-id-123';
|
||||
const sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Test');
|
||||
|
||||
// SDK returns the same ID (unlikely but possible)
|
||||
store.updateMemorySessionId(sessionDbId, contentSessionId);
|
||||
|
||||
const session = store.getSessionById(sessionDbId);
|
||||
// Now checking for non-null instead of comparing to content_session_id
|
||||
const hasRealMemorySessionId = session?.memory_session_id !== null;
|
||||
|
||||
// Would be TRUE since we set a value (even if same as content)
|
||||
// In practice, the SDK should never return the same ID as contentSessionId
|
||||
expect(hasRealMemorySessionId).toBe(true);
|
||||
});
|
||||
|
||||
it('should handle NULL memory_session_id gracefully', () => {
|
||||
const contentSessionId = 'null-test-session';
|
||||
const sessionDbId = store.createSDKSession(contentSessionId, 'test-project', 'Test');
|
||||
|
||||
// memory_session_id is already NULL from createSDKSession
|
||||
const session = store.getSessionById(sessionDbId);
|
||||
const hasRealMemorySessionId = session?.memory_session_id !== null;
|
||||
|
||||
// Should be false (NULL means not captured yet)
|
||||
expect(hasRealMemorySessionId).toBe(false);
|
||||
describe('Foreign Key Integrity', () => {
|
||||
it('should reject observations for non-existent sessions', () => {
|
||||
expect(() => {
|
||||
store.storeObservation('nonexistent-session-id', 'test-project', {
|
||||
type: 'discovery',
|
||||
title: 'Invalid FK',
|
||||
subtitle: null,
|
||||
facts: [],
|
||||
narrative: null,
|
||||
concepts: [],
|
||||
files_read: [],
|
||||
files_modified: []
|
||||
}, 1);
|
||||
}).toThrow(); // FK constraint violation
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user