SSWConsulting / SSWConsulting/TimePro.Tools
Share one implementation between CLI commands and MCP tools
Nobody has claimed this yet.
- Dominant language
- C#
- Stars
- 3
- Forks
- 0
- Avg merge
- 29m
- Merged PRs (30d)
- 28
Description
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
--jsonand MCP structured content. One serializer policy (OutputHelperomits 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 toCallToolResult.IsError = true. Today MCP returns{"error":"..."}strings that look like successful results. - Writes with previews follow the
LeaveUpdateServiceshape:PrepareAsync(no writes) thenApplyAsync. CLI--dry-runand MCPdryRuncall the same preparation. - Adapter-only: prompts,
--yes, table formatting, file vs base64 output, exit codes. MCP confirmations use client approval or elicitation; a model-suppliedconfirmed=trueis not consent.
Enforcement
- Architecture test: no
ITimeProApiClient,HttpClient, or console access fromFeatures/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/listnames, 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).
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Read AGENTS.md and the registrations in Features/Mcp/McpHostCommand.cs, then inspect LeaveUpdateService and the Timesheet update CLI and MCP adapters for migration slice 1. Done means the adapters use one shared service, the architecture and parity tests enforce delegation, and established --json wire shapes remain unchanged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- backend-api-design, cli, tooling
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100