Why
47 MCP tools (18 default, 29 accounting-gated) are registered in Features/Mcp/McpHostCommand.cs. Only four share a service with their CLI counterpart (LeaveCreateService, LeaveUpdateService, LeaveBalanceImportService, WeekCoverageService). The other 43 re-implement the orchestration the CLI command already does, so every fix lands twice or, as with #21, only once. AGENTS.md already states the rule ("MCP delegates to the same service"); this issue makes the codebase follow it and adds a test that keeps it that way.
Inventory (where sharing is missing)
| MCP tool |
CLI command |
Today |
| GetTimesheets |
ts get |
duplicate; MCP skips weekends, CLI does not |
| CreateTimesheet |
ts create |
duplicate; CLI also resolves rate, category, deducted time |
| UpdateTimesheet |
ts update |
duplicate, destructive (#21) |
| DeleteTimesheet / AcceptSuggestedTimesheet |
ts delete / ts accept |
duplicate; empty-response handling differs |
| GetSuggestedTimesheets |
ts suggest |
duplicate; note this refreshes server state, it is not read-only |
| CheckWeek |
ts check |
shared service, duplicated projection |
| SearchClients / GetProjectsForClient / GetClientRate / GetCrmBookings / ListIterations |
client search / project list / rate get / booking list / iteration list |
duplicate; missing-rate result differs |
| GetLeaveEntries / GetLeaveBalance / GetLeaveBalanceStatus |
leave list / leave balance / leave balances status |
duplicate |
| All accounting tools (invoices, receipts, credit notes, products, rates, unbilled, recurring, query, summary, prepaid) |
matching tp commands |
duplicate; prepaid calculation lives in TimeProApiClient |
| GetInvoicesByClient, GetUnpaidInvoicesByClient, ListCategories, ListBillableTypes, ListAllSkus |
none |
MCP-only, violates the "CLI first" rule |
| none |
leave cancel |
CLI-only |
Target architecture
- Per feature:
Services/ with a request record, result record and a service that owns validation, defaults, lookups, read-merge and API orchestration. No single giant service.
- CLI command = parse settings, build request, call service, render table or JSON, choose exit code.
- MCP tool = bind args, build the same request, call the same service, serialize the same result record. Never shell out to
tp.
- One result type feeds both
--json and MCP structured content. One serializer policy (OutputHelper omits nulls and stringifies enums; MCP classes currently differ).
- One application error type (category, nullable API status, safe message, recovery hint,
mayHaveBeenApplied). CLI maps it to the {"error":{...}} envelope plus exit code; MCP maps it to CallToolResult.IsError = true. Today MCP returns {"error":"..."} strings that look like successful results.
- Writes with previews follow the
LeaveUpdateService shape: PrepareAsync (no writes) then ApplyAsync. CLI --dry-run and MCP dryRun call the same preparation.
- Adapter-only: prompts,
--yes, table formatting, file vs base64 output, exit codes. MCP confirmations use client approval or elicitation; a model-supplied confirmed=true is not consent.
Enforcement
- Architecture test: no
ITimeProApiClient, HttpClient, or console access from Features/Mcp/Tools. Reflection can check dependencies; use Roslyn or IL inspection if delegation itself must be proven.
- Every
[McpServerTool] maps to a shared service and a registered CLI command, with a temporary allowlist for unmigrated tools that may only shrink.
- Adapter parity tests against the same fake service; snapshot
tools/list names, schemas and annotations.
Migration order
| # |
Slice |
Size |
| 1 |
TimesheetUpdateService, both update adapters (fixes #21; also home for #22 --iteration) |
M |
| 2 |
Timesheet create (shared rate/category/location logic; no implicit MCP writes) |
L |
| 3 |
Delete, accept, and normalising leave/import outcomes |
M |
| 4 |
Timesheet reads, suggestions, week coverage (explicit weekend policy) |
M |
| 5 |
Prepaid and accounting reads (move prepaid maths out of TimeProApiClient) |
M each |
| 6 |
Lookups and the MCP-only / CLI-only gaps above |
S each |
| 7 |
Remove allowlist and legacy projections |
S |
Do not change established --json wire shapes while extracting; schema changes are separate, explicit PRs. Do this before the SDK upgrade (#39).
Why
47 MCP tools (18 default, 29 accounting-gated) are registered in
Features/Mcp/McpHostCommand.cs. Only four share a service with their CLI counterpart (LeaveCreateService,LeaveUpdateService,LeaveBalanceImportService,WeekCoverageService). The other 43 re-implement the orchestration the CLI command already does, so every fix lands twice or, as with #21, only once. AGENTS.md already states the rule ("MCP delegates to the same service"); this issue makes the codebase follow it and adds a test that keeps it that way.Inventory (where sharing is missing)
ts getts createts updatets delete/ts acceptts suggestts checkclient search/project list/rate get/booking list/iteration listleave list/leave balance/leave balances statustpcommandsTimeProApiClientleave cancelTarget architecture
Services/with a request record, result record and a service that owns validation, defaults, lookups, read-merge and API orchestration. No single giant service.tp.--jsonand MCP structured content. One serializer policy (OutputHelperomits nulls and stringifies enums; MCP classes currently differ).mayHaveBeenApplied). CLI maps it to the{"error":{...}}envelope plus exit code; MCP maps it toCallToolResult.IsError = true. Today MCP returns{"error":"..."}strings that look like successful results.LeaveUpdateServiceshape:PrepareAsync(no writes) thenApplyAsync. CLI--dry-runand MCPdryRuncall the same preparation.--yes, table formatting, file vs base64 output, exit codes. MCP confirmations use client approval or elicitation; a model-suppliedconfirmed=trueis not consent.Enforcement
ITimeProApiClient,HttpClient, or console access fromFeatures/Mcp/Tools. Reflection can check dependencies; use Roslyn or IL inspection if delegation itself must be proven.[McpServerTool]maps to a shared service and a registered CLI command, with a temporary allowlist for unmigrated tools that may only shrink.tools/listnames, schemas and annotations.Migration order
TimesheetUpdateService, both update adapters (fixes #21; also home for #22--iteration)TimeProApiClient)Do not change established
--jsonwire shapes while extracting; schema changes are separate, explicit PRs. Do this before the SDK upgrade (#39).