feat(role-play): add character/mod system, memory, session management…

[MinimaxLanbo]

… and UI improvements

• Add character manager with multi-character support and emotion system
• Add mod manager for configurable role-play scenarios (prologue, system prompt, tools)
• Add hybrid memory system (SP injection + save_memory tool) for cross-session persistence
• Add session reset API to clear all session data and restore initial state
• Migrate storage from IndexedDB to disk-based persistence (~/.openroom/sessions/)
• Decouple image generation tool from storage (generate only, save via file tools)
• Persist suggested replies in chat history to survive page refresh
• Redesign floating action buttons as horizontal bottom toolbar

Summary

Brief description of the changes.

Changes

Related Issues

Closes #

Checklist

• pnpm run lint passes
• pnpm build passes
• Documentation updated (if applicable)
• Self-reviewed the code changes
• AI code review passed (auto-triggered on PR)

[claude]

PR #8 Code Review

Overview

This PR introduces a character/mod system with memory management and session-based storage. The changes include:

• New Components: CharacterPanel, ModPanel
• New Libraries: characterManager, modManager, memoryManager, sessionPath, diskStorage
• Major Refactors: ChatPanel for character/mod/memory support, storage migration from IndexedDB to disk

---

Issues Found

1. Potential Stale Closure Issue (Medium)

File: apps/webuiapps/src/components/ChatPanel/index.tsx:1605

useEffect(() => {
  loadChatHistory(sessionPath).then(...);
  loadMemories(sessionPath).then(setMemories);
}, [sessionPath, modCollection, seedPrologue]);

The seedPrologue is in the dependency array but defined with [modCollection] dependency. When modCollection changes but sessionPath stays the same, this effect may not re-run properly, potentially causing chat history to not reload when switching mods.

Recommendation: Use a ref for seedPrologue or ensure the effect re-runs correctly when modCollection changes.

---

2. Race Condition in Memory Loading (Medium)

File: apps/webuiapps/src/lib/memoryManager.ts:4366-4381

const reads = jsonFiles.map(async (f) => {
  // ... fetches
});
await Promise.all(reads);

No concurrency limiting for memory file reads. With many memory files, this could cause network issues. The batchConcurrent utility from @/lib should be used here per project rules.

---

3. Missing Error Propagation (Medium)

File: apps/webuiapps/src/lib/memoryManager.ts:4392-4416

The saveMemory function silently catches all errors:

catch {
  // silently ignore
}
return entry;

Errors are silently ignored and the entry is returned as if successful. Callers have no way to know if the save actually succeeded.

---

4. No Input Validation on Session Path (Low)

File: apps/webuiapps/src/lib/sessionPath.ts:5035-5041

export function setSessionPath(path: string): void {
  _sessionPath = path;
}

No validation on the session path. Malicious input could potentially cause path traversal issues when combined with disk operations.

---

5. Module-Level Counter Not Persistent (Low)

File: apps/webuiapps/src/lib/characterManager.ts:3386-3389

let _nextId = 1;
export function generateCharacterId(): string {
  return `char_${Date.now()}_${_nextId++}`;
}

The _nextId counter resets on hot module reload (HMR), potentially causing ID collisions. Consider using a more unique identifier like crypto.randomUUID().

---

6. Inline Style Usage (Low)

File: apps/webuiapps/src/components/ChatPanel/index.tsx:145

<div style={{ flex: 1 }} />

The project uses SCSS modules consistently. This inline style breaks consistency.

---

7. Missing Cleanup for Async Operations (Low)

File: apps/webuiapps/src/components/ChatPanel/index.tsx:1521-1546

The debounced save effect doesn't handle component unmount during the timeout well:

return () => {
  if (saveTimerRef.current) clearTimeout(saveTimerRef.current);
};

If saveChatHistory is in-flight when unmounting, it could cause state updates on unmounted component.

---

8. Unused Variable (Low)

File: apps/webuiapps/src/components/ChatPanel/index.tsx:1477-1478

const sessionPath = buildSessionPath(charCollection.activeId, modCollection.activeId);
setSessionPath(sessionPath);

Calling setSessionPath during render (not in useEffect) causes the session path to be set on every render, which is inefficient and could cause issues.

---

Security

The path sanitization in vite.config.ts:5149 looks good:

const safePath = relPath.replace(/[^a-zA-Z0-9_\-./]/g, '_').replace(/\.\./g, '');

This prevents path traversal attacks.

---

Recommendations

1. High Priority: Fix the stale closure issue (build(deps): Bump actions/setup-node from 4 to 6 #1) and add concurrency limiting to memory loading (build(deps): Bump actions/checkout from 4 to 6 #2)
2. Medium Priority: Add error handling to saveMemory (build(deps-dev): Bump the dev-dependencies group with 24 updates #3), validate session path (build(deps): Bump the production-dependencies group with 16 updates #4)
3. Low Priority: Replace inline style with SCSS module (test: verify MiniMax M2.5 PR review workflow #6), fix session path setting to use useEffect (feat(role-play): add character/mod system, memory, session management… #8)

---

Positive Observations

• Good separation of concerns with dedicated managers for characters, mods, and memory
• Proper use of TypeScript interfaces for type safety
• Security considerations in place for path sanitization
• Session-scoped storage is a good architectural improvement

[claude]

PR #8 Code Review

Summary

This PR introduces a comprehensive role-play character/mod system with memory, session management, and UI improvements. The changes add:

• characterManager.ts: Character configuration management with persistence
• modManager.ts: Story/scenario mod system with stage progression
• memoryManager.ts: Long-term memory system for character conversations
• CharacterPanel.tsx & ModPanel.tsx: New UI components for character/mod management
• sessionPath.ts: Session path management for scoped file operations

---

Issues Found

1. TypeScript - Untyped Empty Catch Blocks (Medium)

Several files use empty catch blocks that silently swallow errors:

• characterManager.ts:190,206,219,233 - silent catch blocks in migration/load functions
• diskStorage.ts:45,60,63,84,98,123 - multiple silent catches

Recommendation: Add logging or return meaningful values from catch blocks to aid debugging.

2. React - Missing Dependency in useEffect (Medium)

In ChatPanel/index.tsx:392-442, the useEffect loading configs has an empty dependency array [], but calls seedPrologue() which depends on modCollection:

useEffect(() => {
  // ... loads modCollection
  if (loadedMessages.length === 0 && loadedHistory.length === 0) {
    seedPrologue(); // This function uses modCollection but modCollection is not in deps
  }
}, []); // Missing dependencies

This works because modCollection is loaded inside the effect, but it's fragile. Consider explicitly handling this or adding a comment explaining the pattern.

3. Memory Manager - Potential Race Condition (Low)

In memoryManager.ts:76-110, memory files are loaded and then fetched individually:

const reads = jsonFiles.map(async (f) => {
  // individual fetches
});
await Promise.all(reads);

This is fine for loading, but if files are added/deleted during the list operation, there could be inconsistencies. Not critical for typical usage.

4. characterManager - Unused Export (Low)

resolveEmotionImage in characterManager.ts:371-373 is marked @deprecated but still exported. Consider removing or keeping for backward compatibility.

5. ModManager - Mutable State in Class (Low)

In modManager.ts:424-492, the finishTarget method mutates this.state directly:

this.state.completed_targets.push(id);

While this works, it could cause issues if the same ModManager instance is used in multiple places. Consider immutable updates or defensive copying.

6. Hardcoded API Paths (Info)

API endpoints are hardcoded:

• CHARACTER_API = '/api/characters' in characterManager.ts
• MODS_API = '/api/mods' in modManager.ts
• API_PATH = '/api/session-data' in memoryManager.ts

These should work for the dev server, but consider if these need to be configurable for production.

---

Positive Observations

1. Good TypeScript typing throughout - interfaces are well-defined and consistent
2. Proper migration handling for old localStorage formats (characterManager.ts, modManager.ts)
3. Clean separation of concerns - storage logic, UI components, and managers are well-organized
4. Fallback patterns - load functions gracefully fall back to defaults when data is unavailable
5. Consistent API pattern - async/sync load pairs for each data type

---

Security Considerations

No major security concerns. The code:

• Uses localStorage with user-controlled data
• Makes fetch calls to internal API endpoints
• No direct exposure of sensitive data

---

Build & Compatibility

The changes appear compatible with the existing codebase. New exports are added to lib/index.ts appropriately.

---

Recommendations

1. Add logging to empty catch blocks for better observability
2. Consider adding comments explaining the useEffect dependency pattern in ChatPanel
3. Decide whether to keep or remove the deprecated resolveEmotionImage function
4. Consider using immutable state updates in ModManager for better predictability

[claude]

PR #8 Code Review

Summary

This PR adds a character/mod system, memory management, and session management to the OpenRoom application. The changes include:

• New libraries: characterManager.ts, modManager.ts, sessionPath.ts, memoryManager.ts
• New UI components: CharacterPanel.tsx, ModPanel.tsx
• Vite plugin updates: Character/mod persistence APIs, session data API

---

Issues Found

1. ID Collision Risk After HMR (Medium)

Files: characterManager.ts:56-59, modManager.ts:73-76

The _nextId counter is module-level state that persists across hot module replacement (HMR), which can cause ID collisions after reload.

let _nextId = 1;
export function generateCharacterId(): string {
  return `char_${Date.now()}_${_nextId++}`;
}

Recommendation: Use a more unique identifier like crypto.randomUUID() or reset _nextId on HMR.

---

2. Missing batchConcurrent Usage (Medium)

File: memoryManager.ts:88-102

The code uses Promise.all() directly for loading multiple memory files, which violates the concurrent execution rules (concurrent-execution.md). The project requires using batchConcurrent for >6 files.

const reads = jsonFiles.map(async (f) => { ... });
await Promise.all(reads);

Recommendation: Replace with batchConcurrent for better concurrency control.

---

3. Missing Error Boundaries in UI Panels (Low)

Files: CharacterPanel.tsx, ModPanel.tsx

The panel components don't handle edge cases gracefully:

• No validation when user deletes all characters (handled but could show toast)
• Missing input validation for empty character names

---

4. Silent Error Handling (Low)

Files: characterManager.ts:190-193, modManager.ts:217-220

Error handling in migration functions uses empty catch blocks, making debugging difficult:

} catch (e) {
  console.warn('[CharacterManager] migrateOldFormat failed:', e);
}

This is acceptable for migrations but could log more context.

---

5. Potential Memory Leak (Low)

File: characterManager.ts:317

The module-level _emotionVideoCache Map persists across component mounts. While it has a clear function, it could grow unbounded if many characters are created with different emotions.

---

6. Session Path Not Reset on Character/Mod Change (Low)

File: sessionPath.ts

When users switch characters or mods, the old session data isn't automatically cleared. This could lead to stale data being displayed.

---

7. React Performance (Low)

File: ChatPanel/index.tsx

Some callback functions may benefit from useCallback to prevent unnecessary re-renders, though this is likely not a significant issue.

---

Security: Good

• Path traversal protection implemented in vite.config.ts:94,218
• JSON validation before writing (line 51, 351)
• No user input directly used in dangerous operations

---

TypeScript: Good

• Proper type definitions throughout
• Interface alignments with backend (character.yaml, mod.yaml)

---

What's Working Well

1. Migration functions for backward compatibility
2. Proper separation of concerns (character, mod, memory managers)
3. Session-scoped data storage
4. Default character/mod data is well-structured

---

Action Items

1. High: Replace Promise.all with batchConcurrent in memoryManager.ts
2. Medium: Use crypto.randomUUID() for ID generation to avoid HMR issues
3. Low: Add input validation in CharacterPanel/ModPanel