Skip to content

refactor(api): route HabitTools via AgentOperationExecutor + add 6 IAiTools (#79) - #175

Merged
thomasluizon merged 1 commit into
mainfrom
issue-79
Jun 4, 2026
Merged

refactor(api): route HabitTools via AgentOperationExecutor + add 6 IAiTools (#79)#175
thomasluizon merged 1 commit into
mainfrom
issue-79

Conversation

@thomasluizon

Copy link
Copy Markdown
Owner

Refs thomasluizon/orbit-ui-mobile#79

What

Routes MCP HabitTools write methods through the shared agent executor instead of calling IMediator.Send directly, so MCP habit mutations now pass the same policy evaluation (read-only-credential denial, ownership pre-check, confirmation gating) and AgentAuditLogs audit trail as every other agent surface.

  • New thin McpExecutorBridge (Api/Mcp) builds an AgentExecuteOperationRequest with Surface = AgentExecutionSurface.Mcp and the four claim-derived credential fields (mirrors AgentTools.ExecuteAgentOperation), calls IAgentOperationExecutor.ExecuteAsync, and maps the outcome back to the legacy MCP string contract (success → caller-formatted message; denied/failed → Error: …; pending → a confirm_agent_operation_v2 prompt).
  • All 13 in-scope habit mutations route through the bridge: create_habit, update_habit, delete_habit, log_habit, skip_habit, create_sub_habit, duplicate_habit, update_checklist, reorder_habits, move_habit_parent, link_goals_to_habit, bulk_create_habits, bulk_delete_habits.
  • Read/query tools stay on MediatR. bulk_log_habits / bulk_skip_habits also stay on MediatR — their MCP contract accepts an explicit per-instance date that the current chat IAiTools do not model, so routing them would silently drop that capability. Documented at the top of HabitTools.cs.

Authors the 6 missing IAiTools (broad approach B)

update_checklist, reorder_habits, move_habit_parent, link_goals_to_habit, bulk_create_habits, bulk_delete_habits — each wraps its existing MediatR command, is DI-registered in ServiceCollectionExtensions, and is mapped into the matching capability's chatTools[] in AgentCatalogService so the build-time catalog enforcement tests (EveryChatTool_IsMappedToTheCatalog / EveryMcpTool_IsMappedToTheCatalog) pass.

Overlaps wave-2 #89, which targets the same toolset surface and needs these same 6 IAiTools. Whoever lands second rebases onto the other.

Tests

  • HabitToolsTests rewritten for the new constructor: write-path tests assert the method routes through the executor with Surface=Mcp and the correct operationId, and map Succeeded → success string / Failed/DeniedError: … / PendingConfirmation → confirmation prompt. Read-path and the two still-MediatR bulk methods are unchanged.
  • New McpHabitExecutorRoutingTests integration tests: (a) a successful MCP create_habit writes an AgentAuditLogs row (CapabilityId="habits.write", Surface=Mcp, OutcomeStatus=Succeeded); (b) a read-only API-key credential is denied with read_only_credential across three distinct toolsets (habits.write, goals.write, and the Destructive habits.bulk.delete, proving read-only wins before the confirmation gate).

dotnet build Orbit.slnx clean (0 errors, no new warnings). Unit projects green: Domain 344, Application 1679, Infrastructure 917. Integration tests authored but not executable locally — the entire integration suite currently fails at app startup with a pre-existing Jwt:SecretKey must be at least 32 bytes config error unrelated to this change (CI runs unit projects only).

🤖 Generated with Claude Code

…iTools (#79)

MCP habit mutations now run through McpExecutorBridge -> IAgentOperationExecutor (Surface=Mcp), so they share the agent policy layer (read-only-credential denial, ownership pre-check, confirmation gating) and the AgentAuditLogs trail instead of calling IMediator.Send directly. Reads stay on MediatR. bulk_log_habits and bulk_skip_habits stay on MediatR because their MCP contract supports an explicit per-instance date the chat IAiTools do not model.

Authors the 6 previously-unbacked habit IAiTools (update_checklist, reorder_habits, move_habit_parent, link_goals_to_habit, bulk_create_habits, bulk_delete_habits), registers them in DI, and maps them in AgentCatalogService so the build-time catalog enforcement passes. Overlaps wave-2 #89, which needs the same 6 tools.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Routes 13 MCP habit mutations through McpExecutorBridgeIAgentOperationExecutor with Surface=Mcp, giving them the same policy layer (read-only-credential denial, ownership pre-check, confirmation gating) and AgentAuditLogs trail as every other agent surface. The 6 new IAiTool implementations are correctly validated via JsonArgumentParser, registered in DI, and mapped into the catalog so the build-time enforcement tests pass. The bridge correctly extracts all four credential fields from ClaimsPrincipal, and the executor passes the Payload object reference through in-process (no JSON round-trip), so the BulkCreate/BulkDelete typed casts work as expected. BulkLogHabits and BulkSkipHabits are intentionally left on MediatR with clear documentation. Unit tests were rewritten to assert routing through the executor with Surface=Mcp and the correct OperationId, and integration tests cover the audit-row write and read-only-credential denial across three capability groups.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid execution of a real security improvement. The McpExecutorBridge is a clean, well-scoped adapter: it extracts the four claim-derived credential fields correctly, sets Surface = Mcp, and maps the three executor outcomes (Succeeded / PendingConfirmation / Failed|Denied) back to the MCP string contract without leaking domain types. All 13 in-scope mutations route through the policy evaluator, the intentional exceptions (bulk_log/skip staying on MediatR) are documented with a clear rationale, and the catalog is updated consistently with the new IAiTool registrations. The 6 new IAiTool implementations follow the existing parsing/validation pattern (JsonArgumentParser, TryGetProperty-before-use, ToolResult on failure). Unit tests verify the routing surface (Surface=Mcp, correct operationId) and all three outcome paths; integration tests cover the audit-row write and the read-only-credential denial ordering. Build is clean with 0 new warnings.

@thomasluizon
thomasluizon merged commit 5cf1f06 into main Jun 4, 2026
5 checks passed
@thomasluizon
thomasluizon deleted the issue-79 branch June 4, 2026 23:57
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