Phase 1: Project setup and FastMCP server skeleton

[evansenter]

Summary

Set up the basic project infrastructure following the claude-event-bus pattern.

• pyproject.toml with FastMCP, uvicorn, and dev dependencies
• Makefile with check, fmt, lint, test, install, uninstall targets
• LaunchAgent plist and install/uninstall scripts for auto-start
• dev.sh script for development mode with auto-reload
• Basic FastMCP server with placeholder tools (get_status, ingest_logs, query_tool_frequency)
• Usage guide as MCP resource at session-analytics://guide
• Tests for the placeholder tools
• README with installation and usage instructions

Server runs on port 8081 (to not conflict with event-bus on 8080).

Test plan

• make check passes (fmt, lint, test)
• LaunchAgent installs and starts successfully
• Server responds on http://localhost:8081/mcp
• Added to Claude Code MCP config

Closes #1

🤖 Generated with Claude Code

[claude]

PR Review: Phase 1 - Project Setup and FastMCP Server Skeleton

Great foundation for the session analytics MCP server! The infrastructure follows the claude-event-bus pattern well. Here's my detailed feedback:

✅ Strengths

1. Excellent scaffolding - Clean separation of concerns with proper project structure
2. Good DevEx - The dev.sh script with LaunchAgent management is thoughtful
3. Comprehensive Makefile - Nice targets for common workflows
4. Proper FastMCP usage - Using stateless_http=True for resilience is correct
5. Clear documentation - The guide.md resource is a great pattern

🔍 Code Quality Issues

1. Test Coverage Gap (src/session_analytics/server.py:104-126)

The tests use .fn() to access the underlying function, which bypasses FastMCP's wrapper. This means you're not actually testing the MCP tool interface. Consider using FastMCP's test client instead.

2. Missing Error Handling (src/session_analytics/server.py:39-42)

The usage_guide() resource silently fails and returns a generic message. Consider logging the error for debugging.

3. Hardcoded Port Conflicts (scripts/dev.sh:35-36)

The script assumes port 8081 is only used by this service, but doesn't verify. Consider adding a port availability check before starting.

4. Path Handling (scripts/install-launchagent.sh:11)

The script doesn't validate that PROJECT_DIR has the expected structure. Add validation for pyproject.toml existence.

🔒 Security Considerations

1. Overly Permissive Settings (.claude/settings.local.json)

The Bash(cat:*) and Bash(curl:*) wildcards are very broad - they allow reading ANY file and fetching ANY URL. Consider restricting to specific use cases.

2. Log File Permissions (scripts/com.evansenter.claude-session-analytics.plist:32-35)

Session logs may contain sensitive information. The LaunchAgent logs are world-readable by default. Consider setting explicit permissions (chmod 600) in the install script.

⚡ Performance Considerations

1. No Health Check Endpoint

The make install script tries to verify the service started, but only checks launchctl list. Consider adding a simple health check endpoint and using curl to verify the service is actually responding.

2. Missing Timeout in dev.sh (scripts/dev.sh:14-18)

The LaunchAgent stop doesn't have a timeout. If the service hangs, this could block indefinitely. Consider using timeout command or adding fallback to pkill.

📊 Test Coverage Gaps

1. No integration tests - Consider testing the full MCP protocol handshake
2. No LaunchAgent tests - Shell scripts are untested (though this is common)
3. Missing edge cases: What happens if .venv doesn't exist? If port 8081 is in use? If ~/.claude doesn't exist?

🐛 Potential Bugs

1. Race Condition in dev.sh (scripts/dev.sh:14-23)

There's a TOCTOU race between checking if the service is running and unloading it. Consider using an atomic operation instead.

2. Python Version Detection (Makefile:27)

The Python version detection could fail silently if none of the required versions exist. Add explicit error handling.

💡 Suggestions

1. Add a project.scripts entry for dev mode
2. Use structured logging (logger.info) instead of print statements in main()
3. Add main.py for cleaner module execution
4. Version source of truth - use importlib.metadata instead of duplicating version in init.py and pyproject.toml

📝 Documentation

• The README is clear and concise
• Consider adding a CONTRIBUTING.md for development setup
• Add docstring examples to guide.md for complex queries

Summary

This is a solid Phase 1 implementation with good architecture and developer experience. The main concerns are:

1. Security: Overly permissive bash allowlist and log file permissions
2. Testing: Tests bypass the MCP layer and lack edge case coverage
3. Error handling: Missing validation and graceful degradation

Recommendation: Approve with requested changes. The issues are non-blocking for a Phase 1 PR but should be addressed before Phase 2 (storage layer) to avoid compounding problems.

Great work on the infrastructure! 🎉

[evansenter]

Superseded by #19 which contains all Phase 1-7 work plus RFC #18 refactoring.