feat(backend): Implement MCP Protocol Handler (Story 5.1)
Implemented JSON-RPC 2.0 protocol handler for MCP communication, enabling AI agents to communicate with ColaFlow using the Model Context Protocol. **Implementation:** - JSON-RPC 2.0 data models (Request, Response, Error, ErrorCode) - MCP protocol models (Initialize, Capabilities, ClientInfo, ServerInfo) - McpProtocolHandler with method routing and error handling - Method handlers: initialize, resources/list, tools/list, tools/call - ASP.NET Core middleware for /mcp endpoint - Service registration and dependency injection setup **Testing:** - 28 unit tests covering protocol parsing, validation, and error handling - Integration tests for initialize handshake and error responses - All tests passing with >80% coverage **Changes:** - Created ColaFlow.Modules.Mcp.Contracts project - Created ColaFlow.Modules.Mcp.Domain project - Created ColaFlow.Modules.Mcp.Application project - Created ColaFlow.Modules.Mcp.Infrastructure project - Created ColaFlow.Modules.Mcp.Tests project - Registered MCP module in ColaFlow.API Program.cs - Added /mcp endpoint via middleware **Acceptance Criteria Met:** ✅ JSON-RPC 2.0 messages correctly parsed ✅ Request validation (jsonrpc: "2.0", method, params, id) ✅ Error responses conform to JSON-RPC 2.0 spec ✅ Invalid requests return proper error codes (-32700, -32600, -32601, -32602) ✅ MCP initialize method implemented ✅ Server capabilities returned (resources, tools, prompts) ✅ Protocol version negotiation works (1.0) ✅ Request routing to method handlers ✅ Unit test coverage > 80% ✅ All tests passing **Story**: docs/stories/sprint_5/story_5_1.md 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
445
docs/stories/sprint_5/story_5_10.md
Normal file
445
docs/stories/sprint_5/story_5_10.md
Normal file
@@ -0,0 +1,445 @@
|
||||
---
|
||||
story_id: story_5_10
|
||||
sprint_id: sprint_5
|
||||
phase: Phase 3 - Tools & Diff Preview
|
||||
status: not_started
|
||||
priority: P0
|
||||
story_points: 5
|
||||
assignee: backend
|
||||
estimated_days: 2
|
||||
created_date: 2025-11-06
|
||||
dependencies: [story_5_3, story_5_9]
|
||||
---
|
||||
|
||||
# Story 5.10: PendingChange Management
|
||||
|
||||
**Phase**: Phase 3 - Tools & Diff Preview (Week 5-6)
|
||||
**Priority**: P0 CRITICAL
|
||||
**Estimated Effort**: 5 Story Points (2 days)
|
||||
|
||||
## User Story
|
||||
|
||||
**As a** User
|
||||
**I want** to approve or reject AI-proposed changes
|
||||
**So that** I maintain control over my project data
|
||||
|
||||
## Business Value
|
||||
|
||||
PendingChange management is the **approval workflow** for AI operations. It enables:
|
||||
- **Human-in-the-Loop**: AI proposes, human approves
|
||||
- **Audit Trail**: Complete history of all AI operations
|
||||
- **Safety**: Prevents unauthorized or erroneous changes
|
||||
- **Compliance**: Required for enterprise adoption
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
### AC1: PendingChange CRUD
|
||||
- [ ] Create PendingChange with Diff Preview
|
||||
- [ ] Query PendingChanges (by tenant, by status, by user)
|
||||
- [ ] Get PendingChange by ID
|
||||
- [ ] Approve PendingChange (execute operation)
|
||||
- [ ] Reject PendingChange (log reason)
|
||||
|
||||
### AC2: Approval Workflow
|
||||
- [ ] Approve action executes actual operation (create/update/delete)
|
||||
- [ ] Approve action updates status to Approved
|
||||
- [ ] Approve action logs approver and timestamp
|
||||
- [ ] Reject action updates status to Rejected
|
||||
- [ ] Reject action logs reason and rejecter
|
||||
|
||||
### AC3: Auto-Expiration
|
||||
- [ ] 24-hour expiration timer
|
||||
- [ ] Background job checks expired changes
|
||||
- [ ] Expired changes marked as Expired status
|
||||
- [ ] Expired changes NOT executed
|
||||
|
||||
### AC4: REST API Endpoints
|
||||
- [ ] `GET /api/mcp/pending-changes` - List (filter by status)
|
||||
- [ ] `GET /api/mcp/pending-changes/{id}` - Get details
|
||||
- [ ] `POST /api/mcp/pending-changes/{id}/approve` - Approve
|
||||
- [ ] `POST /api/mcp/pending-changes/{id}/reject` - Reject
|
||||
|
||||
### AC5: Testing
|
||||
- [ ] Unit tests for PendingChange service
|
||||
- [ ] Integration tests for CRUD operations
|
||||
- [ ] Integration tests for approval/rejection workflow
|
||||
- [ ] Test auto-expiration mechanism
|
||||
|
||||
## Technical Design
|
||||
|
||||
### Service Interface
|
||||
|
||||
```csharp
|
||||
public interface IPendingChangeService
|
||||
{
|
||||
Task<PendingChange> CreateAsync(
|
||||
string toolName,
|
||||
DiffPreview diff,
|
||||
CancellationToken cancellationToken);
|
||||
|
||||
Task<PendingChange?> GetByIdAsync(
|
||||
Guid id,
|
||||
CancellationToken cancellationToken);
|
||||
|
||||
Task<PaginatedList<PendingChange>> GetPendingChangesAsync(
|
||||
PendingChangeStatus? status,
|
||||
int page,
|
||||
int pageSize,
|
||||
CancellationToken cancellationToken);
|
||||
|
||||
Task ApproveAsync(
|
||||
Guid pendingChangeId,
|
||||
Guid approvedBy,
|
||||
CancellationToken cancellationToken);
|
||||
|
||||
Task RejectAsync(
|
||||
Guid pendingChangeId,
|
||||
Guid rejectedBy,
|
||||
string reason,
|
||||
CancellationToken cancellationToken);
|
||||
|
||||
Task ExpireOldChangesAsync(CancellationToken cancellationToken);
|
||||
}
|
||||
```
|
||||
|
||||
### Service Implementation
|
||||
|
||||
```csharp
|
||||
public class PendingChangeService : IPendingChangeService
|
||||
{
|
||||
private readonly IPendingChangeRepository _repository;
|
||||
private readonly ITenantContext _tenantContext;
|
||||
private readonly IMediator _mediator;
|
||||
private readonly ILogger<PendingChangeService> _logger;
|
||||
|
||||
public async Task<PendingChange> CreateAsync(
|
||||
string toolName,
|
||||
DiffPreview diff,
|
||||
CancellationToken ct)
|
||||
{
|
||||
var tenantId = _tenantContext.CurrentTenantId;
|
||||
var apiKeyId = (Guid)_httpContext.HttpContext.Items["ApiKeyId"]!;
|
||||
|
||||
var pendingChange = PendingChange.Create(
|
||||
toolName, diff, tenantId, apiKeyId);
|
||||
|
||||
await _repository.AddAsync(pendingChange, ct);
|
||||
await _repository.SaveChangesAsync(ct);
|
||||
|
||||
_logger.LogInformation(
|
||||
"PendingChange created: {Id} - {ToolName} {Operation} {EntityType}",
|
||||
pendingChange.Id, toolName, diff.Operation, diff.EntityType);
|
||||
|
||||
return pendingChange;
|
||||
}
|
||||
|
||||
public async Task ApproveAsync(
|
||||
Guid pendingChangeId,
|
||||
Guid approvedBy,
|
||||
CancellationToken ct)
|
||||
{
|
||||
var pendingChange = await _repository.GetByIdAsync(pendingChangeId, ct);
|
||||
if (pendingChange == null)
|
||||
throw new McpNotFoundException("PendingChange", pendingChangeId.ToString());
|
||||
|
||||
// Domain method (raises PendingChangeApprovedEvent)
|
||||
pendingChange.Approve(approvedBy);
|
||||
|
||||
await _repository.UpdateAsync(pendingChange, ct);
|
||||
await _repository.SaveChangesAsync(ct);
|
||||
|
||||
// Publish domain events (will trigger operation execution)
|
||||
foreach (var domainEvent in pendingChange.DomainEvents)
|
||||
{
|
||||
await _mediator.Publish(domainEvent, ct);
|
||||
}
|
||||
|
||||
_logger.LogInformation(
|
||||
"PendingChange approved: {Id} by {ApprovedBy}",
|
||||
pendingChangeId, approvedBy);
|
||||
}
|
||||
|
||||
public async Task RejectAsync(
|
||||
Guid pendingChangeId,
|
||||
Guid rejectedBy,
|
||||
string reason,
|
||||
CancellationToken ct)
|
||||
{
|
||||
var pendingChange = await _repository.GetByIdAsync(pendingChangeId, ct);
|
||||
if (pendingChange == null)
|
||||
throw new McpNotFoundException("PendingChange", pendingChangeId.ToString());
|
||||
|
||||
pendingChange.Reject(rejectedBy, reason);
|
||||
|
||||
await _repository.UpdateAsync(pendingChange, ct);
|
||||
await _repository.SaveChangesAsync(ct);
|
||||
|
||||
_logger.LogInformation(
|
||||
"PendingChange rejected: {Id} by {RejectedBy} - Reason: {Reason}",
|
||||
pendingChangeId, rejectedBy, reason);
|
||||
}
|
||||
|
||||
public async Task ExpireOldChangesAsync(CancellationToken ct)
|
||||
{
|
||||
var expiredChanges = await _repository.GetExpiredPendingChangesAsync(ct);
|
||||
|
||||
foreach (var change in expiredChanges)
|
||||
{
|
||||
change.Expire();
|
||||
await _repository.UpdateAsync(change, ct);
|
||||
|
||||
_logger.LogWarning(
|
||||
"PendingChange expired: {Id} - {ToolName}",
|
||||
change.Id, change.ToolName);
|
||||
}
|
||||
|
||||
await _repository.SaveChangesAsync(ct);
|
||||
|
||||
_logger.LogInformation(
|
||||
"Expired {Count} pending changes",
|
||||
expiredChanges.Count);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Approval Event Handler (Executes Operation)
|
||||
|
||||
```csharp
|
||||
public class PendingChangeApprovedEventHandler
|
||||
: INotificationHandler<PendingChangeApprovedEvent>
|
||||
{
|
||||
private readonly IMediator _mediator;
|
||||
private readonly ILogger<PendingChangeApprovedEventHandler> _logger;
|
||||
|
||||
public async Task Handle(PendingChangeApprovedEvent e, CancellationToken ct)
|
||||
{
|
||||
var diff = e.Diff;
|
||||
|
||||
_logger.LogInformation(
|
||||
"Executing approved operation: {Operation} {EntityType}",
|
||||
diff.Operation, diff.EntityType);
|
||||
|
||||
try
|
||||
{
|
||||
// Route to appropriate command handler based on operation + entity type
|
||||
object command = (diff.Operation, diff.EntityType) switch
|
||||
{
|
||||
("CREATE", "Story") => MapToCreateStoryCommand(diff),
|
||||
("UPDATE", "Story") => MapToUpdateStoryCommand(diff),
|
||||
("DELETE", "Story") => MapToDeleteStoryCommand(diff),
|
||||
("CREATE", "Epic") => MapToCreateEpicCommand(diff),
|
||||
("UPDATE", "Epic") => MapToUpdateEpicCommand(diff),
|
||||
// ... more mappings
|
||||
_ => throw new NotSupportedException(
|
||||
$"Unsupported operation: {diff.Operation} {diff.EntityType}")
|
||||
};
|
||||
|
||||
await _mediator.Send(command, ct);
|
||||
|
||||
_logger.LogInformation(
|
||||
"Operation executed successfully: {PendingChangeId}",
|
||||
e.PendingChangeId);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_logger.LogError(ex,
|
||||
"Failed to execute operation: {PendingChangeId}",
|
||||
e.PendingChangeId);
|
||||
|
||||
// TODO: Update PendingChange status to ExecutionFailed
|
||||
throw;
|
||||
}
|
||||
}
|
||||
|
||||
private CreateStoryCommand MapToCreateStoryCommand(DiffPreview diff)
|
||||
{
|
||||
var afterData = JsonSerializer.Deserialize<StoryDto>(
|
||||
JsonSerializer.Serialize(diff.AfterData));
|
||||
|
||||
return new CreateStoryCommand
|
||||
{
|
||||
ProjectId = afterData.ProjectId,
|
||||
Title = afterData.Title,
|
||||
Description = afterData.Description,
|
||||
Priority = afterData.Priority,
|
||||
// ... map all fields
|
||||
};
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Background Job (Expiration Check)
|
||||
|
||||
```csharp
|
||||
public class PendingChangeExpirationJob : BackgroundService
|
||||
{
|
||||
private readonly IServiceProvider _serviceProvider;
|
||||
private readonly ILogger<PendingChangeExpirationJob> _logger;
|
||||
|
||||
protected override async Task ExecuteAsync(CancellationToken ct)
|
||||
{
|
||||
while (!ct.IsCancellationRequested)
|
||||
{
|
||||
try
|
||||
{
|
||||
using var scope = _serviceProvider.CreateScope();
|
||||
var pendingChangeService = scope.ServiceProvider
|
||||
.GetRequiredService<IPendingChangeService>();
|
||||
|
||||
await pendingChangeService.ExpireOldChangesAsync(ct);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_logger.LogError(ex, "Error in PendingChange expiration job");
|
||||
}
|
||||
|
||||
// Run every 5 minutes
|
||||
await Task.Delay(TimeSpan.FromMinutes(5), ct);
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Tasks
|
||||
|
||||
### Task 1: PendingChangeRepository (3 hours)
|
||||
- [ ] Create `IPendingChangeRepository` interface
|
||||
- [ ] Implement `PendingChangeRepository` (EF Core)
|
||||
- [ ] CRUD methods (Add, Update, GetById, Query)
|
||||
- [ ] GetExpiredPendingChangesAsync() method
|
||||
|
||||
**Files to Create**:
|
||||
- `ColaFlow.Modules.Mcp.Infrastructure/Repositories/PendingChangeRepository.cs`
|
||||
|
||||
### Task 2: PendingChangeService (4 hours)
|
||||
- [ ] Implement `CreateAsync()`
|
||||
- [ ] Implement `ApproveAsync()`
|
||||
- [ ] Implement `RejectAsync()`
|
||||
- [ ] Implement `GetPendingChangesAsync()` with pagination
|
||||
- [ ] Implement `ExpireOldChangesAsync()`
|
||||
|
||||
**Files to Create**:
|
||||
- `ColaFlow.Modules.Mcp.Application/Services/PendingChangeService.cs`
|
||||
|
||||
### Task 3: PendingChangeApprovedEventHandler (4 hours)
|
||||
- [ ] Create event handler
|
||||
- [ ] Map DiffPreview to CQRS commands
|
||||
- [ ] Execute command via MediatR
|
||||
- [ ] Error handling (log failures)
|
||||
|
||||
**Files to Create**:
|
||||
- `ColaFlow.Modules.Mcp.Application/EventHandlers/PendingChangeApprovedEventHandler.cs`
|
||||
|
||||
### Task 4: REST API Controller (2 hours)
|
||||
- [ ] `GET /api/mcp/pending-changes` endpoint
|
||||
- [ ] `GET /api/mcp/pending-changes/{id}` endpoint
|
||||
- [ ] `POST /api/mcp/pending-changes/{id}/approve` endpoint
|
||||
- [ ] `POST /api/mcp/pending-changes/{id}/reject` endpoint
|
||||
|
||||
**Files to Create**:
|
||||
- `ColaFlow.Modules.Mcp/Controllers/PendingChangesController.cs`
|
||||
|
||||
### Task 5: Background Job (2 hours)
|
||||
- [ ] Create `PendingChangeExpirationJob` (BackgroundService)
|
||||
- [ ] Run every 5 minutes
|
||||
- [ ] Call `ExpireOldChangesAsync()`
|
||||
- [ ] Register in DI container
|
||||
|
||||
**Files to Create**:
|
||||
- `ColaFlow.Modules.Mcp.Infrastructure/Jobs/PendingChangeExpirationJob.cs`
|
||||
|
||||
### Task 6: Unit Tests (4 hours)
|
||||
- [ ] Test CreateAsync
|
||||
- [ ] Test ApproveAsync (happy path)
|
||||
- [ ] Test ApproveAsync (already approved throws exception)
|
||||
- [ ] Test RejectAsync
|
||||
- [ ] Test ExpireOldChangesAsync
|
||||
|
||||
**Files to Create**:
|
||||
- `ColaFlow.Modules.Mcp.Tests/Services/PendingChangeServiceTests.cs`
|
||||
|
||||
### Task 7: Integration Tests (3 hours)
|
||||
- [ ] Test full approval workflow (create → approve → verify execution)
|
||||
- [ ] Test rejection workflow
|
||||
- [ ] Test expiration (create → wait 24 hours → expire)
|
||||
- [ ] Test multi-tenant isolation
|
||||
|
||||
**Files to Create**:
|
||||
- `ColaFlow.Modules.Mcp.Tests/Integration/PendingChangeIntegrationTests.cs`
|
||||
|
||||
## Testing Strategy
|
||||
|
||||
### Integration Test Workflow
|
||||
|
||||
```csharp
|
||||
[Fact]
|
||||
public async Task ApprovalWorkflow_CreateStory_Success()
|
||||
{
|
||||
// Arrange
|
||||
var diff = new DiffPreview(
|
||||
operation: "CREATE",
|
||||
entityType: "Story",
|
||||
entityId: null,
|
||||
entityKey: null,
|
||||
beforeData: null,
|
||||
afterData: new { title = "New Story", priority = "High" },
|
||||
changedFields: new[] { /* ... */ }
|
||||
);
|
||||
|
||||
// Act - Create PendingChange
|
||||
var pendingChange = await _service.CreateAsync("create_issue", diff, CancellationToken.None);
|
||||
|
||||
Assert.Equal(PendingChangeStatus.PendingApproval, pendingChange.Status);
|
||||
|
||||
// Act - Approve
|
||||
await _service.ApproveAsync(pendingChange.Id, _userId, CancellationToken.None);
|
||||
|
||||
// Assert - Verify Story created
|
||||
var story = await _storyRepo.GetByIdAsync(/* ... */);
|
||||
Assert.NotNull(story);
|
||||
Assert.Equal("New Story", story.Title);
|
||||
}
|
||||
```
|
||||
|
||||
## Dependencies
|
||||
|
||||
**Prerequisites**:
|
||||
- Story 5.3 (MCP Domain Layer) - PendingChange aggregate
|
||||
- Story 5.9 (Diff Preview Service) - DiffPreview value object
|
||||
|
||||
**Used By**:
|
||||
- Story 5.11 (Core MCP Tools) - Creates PendingChange
|
||||
- Story 5.12 (SignalR Notifications) - Notifies on approval/rejection
|
||||
|
||||
## Risks & Mitigation
|
||||
|
||||
| Risk | Impact | Probability | Mitigation |
|
||||
|------|--------|-------------|------------|
|
||||
| Operation execution fails | Medium | Medium | Transaction rollback, error logging, retry mechanism |
|
||||
| Expiration job stops | Medium | Low | Health checks, monitoring, alerting |
|
||||
| Race condition (concurrent approval) | Low | Low | Optimistic concurrency, database constraints |
|
||||
|
||||
## Definition of Done
|
||||
|
||||
- [ ] All CRUD operations working
|
||||
- [ ] Approval executes operation correctly
|
||||
- [ ] Rejection logs reason
|
||||
- [ ] Expiration job running
|
||||
- [ ] API endpoints working
|
||||
- [ ] Unit tests passing (> 80%)
|
||||
- [ ] Integration tests passing
|
||||
- [ ] Code reviewed
|
||||
|
||||
## Notes
|
||||
|
||||
### Why This Story Matters
|
||||
- **Core M2 Feature**: Enables human-in-the-loop AI operations
|
||||
- **Safety**: Prevents AI mistakes
|
||||
- **Compliance**: Audit trail required
|
||||
- **User Trust**: Users control their data
|
||||
|
||||
### Key Design Decisions
|
||||
1. **Domain Events**: Approval triggers execution (loose coupling)
|
||||
2. **Background Job**: Auto-expiration runs every 5 minutes
|
||||
3. **24-Hour TTL**: Balance between user convenience and system cleanup
|
||||
4. **Status Enum**: Clear lifecycle (Pending → Approved/Rejected/Expired)
|
||||
Reference in New Issue
Block a user