Skip to content

feat: implement agent message history isolation#19

Merged
baryhuang merged 1 commit intomainfrom
claude/issue-6-20250802-2142
Aug 2, 2025
Merged

feat: implement agent message history isolation#19
baryhuang merged 1 commit intomainfrom
claude/issue-6-20250802-2142

Conversation

@baryhuang
Copy link
Copy Markdown
Owner

Implements proper agent message history isolation to ensure each agent only sees their own conversation history.

Changes

  • Add agentId field to all conversation interfaces
  • Update backend API endpoints to filter by agent ID
  • Modify frontend to use proper agent-based filtering
  • Remove fragile keyword-based filtering
  • Enhance cache isolation with agent-specific keys

Fixes #6

Generated with Claude Code

- Add agentId field to all conversation interfaces (ConversationSummary, ConversationHistory, RawHistoryLine, ConversationFile)
- Update conversation parser to extract and preserve agent ID from messages
- Enhance /api/agent-histories/:encodedProjectName to accept ?agentId= query parameter for filtering
- Modify frontend components to pass agent ID when fetching histories
- Remove fragile keyword-based filtering in favor of proper backend agent filtering
- Update cache isolation to use agent-specific cache keys
- Ensure each agent only sees their own message history

Fixes #6

Co-authored-by: Bary Huang <baryhuang@users.noreply.github.com>
@claude
Copy link
Copy Markdown

claude bot commented Aug 2, 2025

Claude finished @baryhuang's task —— View job


PR Review: Agent Message History Isolation

I've completed a comprehensive review of this pull request implementing agent message history isolation. Here's my detailed analysis:

Review Checklist:

  • Read project context from CLAUDE.md
  • Review backend changes (agentHistories.ts, grouping.ts, parser.ts, timestampRestore.ts)
  • Review frontend changes (AgentDetailView.tsx, api.ts, useRemoteAgentHistory.ts)
  • Review shared types changes
  • Analyze code quality and best practices
  • Check for potential bugs and issues
  • Assess performance considerations
  • Evaluate security concerns
  • Review test coverage

Overall Assessment

This is a well-structured implementation that successfully addresses agent message history isolation as described in issue #6. The changes properly implement agent-based filtering and remove the fragile keyword-based filtering system.

📋 Code Quality & Best Practices

Strengths:

  • Clear separation of concerns: Backend filtering logic is properly isolated from frontend display logic
  • Consistent error handling: Good use of try-catch blocks with appropriate error logging
  • TypeScript usage: Strong typing with proper interface definitions
  • Code organization: Well-structured file organization with logical grouping

Areas for improvement:

  • Duplicate logic in agentHistories.ts:75-98: The agent ID lookup logic appears twice (lines 75-79 and 91-96). Consider extracting this into a helper function:
    const findConversationFile = (sessionId: string) => 
      conversationFiles.find(file => file.sessionId === sessionId);

🐛 Potential Issues

Minor Issues:

  1. Cache key collision risk in useRemoteAgentHistory.ts:69: The cache key format ${projectId}:${agentId || 'all'} could potentially collide if an agentId is literally "all". Consider using a different separator or null-safe approach.

  2. Missing validation in agentHistories.ts:17: The agentId query parameter is not validated. Consider adding basic validation:

    if (agentId && typeof agentId !== 'string') {
      return c.json({ error: "Invalid agent ID format" }, 400);
    }
  3. Potential memory leak in frontend caching: The cache in useRemoteAgentHistory.ts can grow indefinitely. Consider implementing cache size limits or LRU eviction.

Performance Considerations

Good implementations:

  • Efficient filtering: Backend filtering reduces data transfer over network
  • Smart caching: 5-minute cache with agent-specific keys prevents unnecessary API calls
  • Lazy loading: History is only loaded when the History tab is accessed

Potential optimizations:

  • Database-level filtering: If this becomes a bottleneck, consider moving filtering to the database query level rather than in-memory filtering
  • Pagination: For agents with extensive history, consider implementing pagination in the history list

🔒 Security Concerns

No significant security issues identified, but consider:

  • Input sanitization: The agentId parameter should be sanitized to prevent potential injection attacks, even though it's currently just used for filtering
  • Access control: Ensure agents can only access their own conversation history (appears to be properly implemented)

🧪 Test Coverage

Missing test coverage for:

  • Agent ID filtering logic in backend handlers
  • Cache isolation with agent-specific keys
  • Frontend error handling for invalid agent IDs
  • Edge cases like empty agent IDs or malformed cache keys

Recommended tests:

// Backend tests
describe('Agent History Filtering', () => {
  it('should filter conversations by agent ID')
  it('should handle missing agent ID gracefully')
  it('should return empty array for non-existent agent')
})

// Frontend tests  
describe('Agent History Cache', () => {
  it('should isolate cache by agent endpoint and ID')
  it('should handle cache key collisions')
  it('should evict expired cache entries')
})

🔧 Specific File Feedback

backend/handlers/agentHistories.ts:

  • Good implementation of query parameter filtering
  • Consider extracting the conversation file lookup logic (lines 75-79, 91-96)
  • Add validation for agentId parameter

frontend/src/hooks/useRemoteAgentHistory.ts:

  • Excellent caching strategy with agent-specific keys
  • Consider cache size limits to prevent memory growth
  • The cache key format could be more collision-resistant

shared/types.ts:

  • Clean addition of agentId fields to interfaces
  • Good TypeScript practices with optional fields

frontend/src/components/native/AgentDetailView.tsx:

  • Excellent removal of fragile keyword-based filtering (lines 134-142)
  • Good error handling and user feedback
  • The agent ID is properly passed through the call chain

📝 Summary

This PR successfully implements robust agent message history isolation with proper separation of concerns. The code quality is high with good TypeScript usage and error handling. The main improvements needed are:

  1. Extract duplicate logic in backend filtering
  2. Add input validation for agent ID parameter
  3. Improve cache key collision resistance
  4. Add comprehensive test coverage

The implementation aligns well with the architectural requirements outlined in CLAUDE.md and properly addresses the cross-contamination issues described in issue #6.

Recommendation: ✅ Approve with minor improvements


@baryhuang baryhuang merged commit 30e2302 into main Aug 2, 2025
8 checks passed
@baryhuang baryhuang deleted the claude/issue-6-20250802-2142 branch August 2, 2025 22:14
This was referenced Aug 2, 2025
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.

messeges are not stored in the app

1 participant