Skip to content

fix: Critical bug fixes for MCP server write operations (v0.4.1-v0.4.7)#48

Merged
vscarpenter merged 1 commit intomainfrom
mcp-bugfixes-clean
Oct 27, 2025
Merged

fix: Critical bug fixes for MCP server write operations (v0.4.1-v0.4.7)#48
vscarpenter merged 1 commit intomainfrom
mcp-bugfixes-clean

Conversation

@vscarpenter
Copy link
Owner

Summary

This PR contains 7 critical bug fixes discovered through end-to-end testing of write operations (v0.4.1-v0.4.7), plus comprehensive documentation updates. All fixes were required to make write operations fully functional after the initial v0.4.0 implementation.

Testing Confirmation ✅

All write operations tested end-to-end and working perfectly:

  • ✅ Create task (simple)
  • ✅ Create task with due date
  • ✅ Create task with subtasks
  • ✅ Update task
  • ✅ Complete task
  • ✅ Delete task

Bug Fixes

v0.4.1 - Push Payload Structure

Problem: Worker API expected different payload structure
Fix: Changed tasksoperations, vectorClockclientVectorClock, added type and taskId fields
Impact: Write operations were 100% non-functional (400 errors from Worker)

v0.4.2 - JWT Schema Mismatch

Problem: JWT parser used wrong field names
Fix: Changed user_idsub, device_iddeviceId (RFC 7519 standard)
Impact: Authentication failures prevented all operations

v0.4.3 - Missing Checksum

Problem: Worker requires SHA-256 checksum but it wasn't calculated
Fix: Added hash() method to CryptoManager
Impact: Tasks silently rejected by Worker (appeared successful but not stored)

v0.4.4 - Rejection Checking

Problem: MCP server didn't check Worker's rejected array
Fix: Added validation of rejection responses
Impact: Silent failures without error messages

v0.4.5 - Field Name Mismatches

Problem: MCP used quadrantId, frontend expects quadrant
Fix: Updated all field names and timestamp formats (number → ISO string)
Impact: Zod validation errors in webapp ("unrecognized key 'quadrantId'")

v0.4.6 - Type Mismatches

Problem: dueDate expected string but got null, subtasks used text instead of title
Fix: Changed dueDate: number | nulldueDate?: string, subtasks.text → subtasks.title
Impact: "Expected string, received null" validation errors

v0.4.7 - MCP Tool Schemas

Problem: MCP tool input schemas didn't match internal types
Fix: Updated all tool schemas to use correct types (string for dueDate, title for subtasks)
Impact: Claude Desktop sent wrong data types causing validation failures

Documentation Updates

  • Updated README to v0.4.7 production-ready status
  • Fixed all schema examples throughout documentation
  • Added comprehensive bug fix history
  • Updated all tool parameter documentation
  • Corrected field names in examples (quadrant, subtasks.title, ISO datetimes)

Files Changed

  • CHANGELOG.md - Complete bug fix history (v0.4.1-v0.4.7)
  • README.md - Updated documentation and examples
  • package.json - Version bumped to 0.4.7
  • src/index.ts - Fixed MCP tool schemas and version
  • src/tools.ts - Updated DecryptedTask interface
  • src/write-ops.ts - Fixed all input/output types
  • src/analytics.ts - Updated date handling for ISO strings
  • src/crypto.ts - Added hash() method
  • src/jwt.ts - Fixed JWT payload schema

Production Ready 🚀

  • All operations thoroughly tested
  • Schema compatibility verified end-to-end
  • Documentation accurate and complete
  • Ready for npm publication

🤖 Generated with Claude Code

This PR includes 7 critical bug fixes discovered through end-to-end testing
of write operations, plus comprehensive documentation updates.

## Bug Fixes

### v0.4.1 - Push Payload Structure
- Fixed Worker API payload structure mismatch
- Changed `tasks` → `operations` array
- Changed `vectorClock` → `clientVectorClock`
- Added `type` field to all operations
- Changed `id` → `taskId`
- Impact: Write operations were 100% non-functional due to 400 errors

### v0.4.2 - JWT Schema Mismatch
- Fixed JWT payload schema to match Worker
- Changed `user_id` → `sub` (RFC 7519 standard)
- Changed `device_id` → `deviceId` (camelCase)
- Added `email` and `jti` fields
- Added `getUserIdFromToken()` helper
- Impact: JWT parsing failed preventing all operations

### v0.4.3 - Missing Checksum
- Added SHA-256 checksum calculation for write operations
- Added `hash()` method to CryptoManager
- Worker requires checksum but schema marked optional
- Impact: Tasks silently rejected by Worker (appeared successful)

### v0.4.4 - Rejection Checking
- Added validation of Worker's `rejected` array
- Tasks could be rejected with HTTP 200 OK
- Now throws detailed errors with rejection reasons
- Impact: Silent failures without error messages

### v0.4.5 - Field Name Mismatches
- Changed `quadrantId` → `quadrant` (frontend uses 'quadrant')
- Changed `createdAt: number` → `createdAt: string` (ISO)
- Changed `updatedAt: number` → `updatedAt: string` (ISO)
- Renamed `deriveQuadrantId()` → `deriveQuadrant()`
- Impact: Zod validation errors in webapp ("unrecognized key 'quadrantId'")

### v0.4.6 - Type Mismatches
- Changed `dueDate: number | null` → `dueDate?: string` (optional ISO)
- Changed `subtasks[].text` → `subtasks[].title`
- Added `completedAt?: string` field
- Added `vectorClock?: Record<string, number>` field
- Impact: "Expected string, received null" errors for dueDate

### v0.4.7 - MCP Tool Schemas
- Fixed MCP tool input schemas to match internal types
- Updated `dueDate` from `type: 'number'` → `type: 'string'`
- Updated subtasks from `text` → `title` field
- Impact: Claude Desktop sent wrong data types

## Documentation Updates

- Updated README to reflect v0.4.7 production-ready status
- Fixed all schema examples (quadrant, subtasks.title, ISO datetimes)
- Updated tool documentation with correct parameter types
- Added comprehensive bug fix history to README
- Updated status section with all 7 bug fixes

## Testing Results

All write operations tested end-to-end and working:
- ✅ Create task (simple)
- ✅ Create task with due date
- ✅ Create task with subtasks
- ✅ Update task
- ✅ Complete task
- ✅ Delete task

## Files Changed

- CHANGELOG.md - Full bug fix history
- README.md - Updated docs and examples
- package.json - Version 0.4.7
- src/index.ts - MCP tool schemas, version
- src/tools.ts - Schema interfaces (quadrant, dueDate, subtasks)
- src/write-ops.ts - All write operation types
- src/analytics.ts - Date handling for ISO strings
- src/crypto.ts - Added hash() method
- src/jwt.ts - Fixed JWT payload schema

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@claude
Copy link

claude bot commented Oct 27, 2025

PR Review: Critical Bug Fixes for MCP Server Write Operations (v0.4.1-v0.4.7)

Overview

This PR addresses 7 critical bug fixes discovered through end-to-end testing of write operations. The changes are well-documented, and the thoroughness of testing is evident. Overall, this is solid bug-fix work that makes the write operations functional.


✅ Strengths

1. Excellent Documentation

  • CHANGELOG.md provides comprehensive detail for each bug fix with clear impact statements
  • README.md accurately updated with current status and capabilities
  • TEST_PLAN.md included (great addition for manual testing guidance)

2. Schema Alignment

  • JWT payload now matches RFC 7519 standard (sub instead of user_id)
  • Field naming consistent with frontend (quadrant not quadrantId, title not text)
  • Timestamp handling unified (ISO strings instead of mixed number/string)

3. Security & Data Integrity

  • Added SHA-256 checksum calculation (v0.4.3) prevents silent data corruption
  • Worker rejection checking (v0.4.4) surfaces actual errors instead of silent failures

🔍 Code Quality Issues

1. Missing Test Coverage ⚠️ HIGH PRIORITY

Problem: No automated tests found for the MCP server package.

Impact:

  • 7 bugs required manual end-to-end testing to discover
  • No regression prevention for these fixes
  • Future changes risk reintroducing these bugs

Recommendation: Add unit tests for write operations, JWT parsing, crypto functions, and schema validation.

2. Type Safety Concerns ⚠️ MEDIUM PRIORITY

Issue in write-ops.ts:212:
Empty string handling for dueDate could cause unexpected behavior with falsy checks.

Issue in write-ops.ts:276-282:
Need explicit empty string check when clearing dueDate field.

3. Error Handling Gaps ⚠️ MEDIUM PRIORITY

Issue in tools.ts:399-402:
Silent failures during decryption - no visibility to users when tasks fail to decrypt.

4. Code Duplication ⚠️ LOW PRIORITY

Completion status logic repeated (lines 285-289, 402-408). Extract to helper function.


🐛 Potential Bugs

1. Timezone Handling

ISO strings are always UTC. Verify frontend expects UTC timestamps.

2. Vector Clock Simplification

Empty vector clocks might cause sync conflicts. Verify multi-device concurrent editing works correctly.


🔒 Security Considerations

✅ Good Practices: Encryption passphrase never logged, SHA-256 checksums, JWT validation

⚠️ Concerns: Client-side JWT parsing without signature validation. Ensure server-side validation is robust.


📊 Performance

Bulk Operations: Fetches ALL tasks to update subset. For 1000+ task lists, this is inefficient.


🎯 Best Practices Alignment

Principle Status Notes
Testing (≥80% coverage) ❌ FAIL No automated tests
Functions ≤30 lines ✅ PASS Well-scoped
Descriptive naming ✅ PASS Clear names
Error handling ⚠️ PARTIAL Some silent failures
Security ✅ PASS Good practices
Documentation ✅ EXCELLENT Comprehensive

📝 Recommendations

Before Merge:APPROVED - Bug fixes are critical and well-tested manually

Immediate Follow-ups:

  1. Add test coverage for MCP server (targeting 80%+)
  2. Review empty string handling for optional fields
  3. Improve decryption error visibility
  4. Verify vector clock handling for multi-device sync

🚀 Conclusion

Recommendation: APPROVE with follow-up testing tasks

This PR successfully fixes 7 critical bugs. Code quality is good, documentation is excellent.

Suggested next steps:

  1. ✅ Merge this PR (fixes are production-critical)
  2. 📝 Create issue: "Add comprehensive test coverage for MCP server"
  3. 🔍 Create issue: "Review type safety for optional fields"

Great work on the thorough bug investigation! 🎉

@vscarpenter vscarpenter merged commit 1e11f26 into main Oct 27, 2025
1 check passed
@vscarpenter vscarpenter deleted the mcp-bugfixes-clean branch October 27, 2025 03:30
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.

1 participant