fix(timesheets): shared create service for ts create and MCP

[jernejk]

Summary

Slice 2 of #38. MCP create_timesheet built its own TimesheetRequest with no SellPrice, so a
real server that cannot derive one answered 400 — the regression harness's staging smoke gate was
red on exactly this. ts create meanwhile resolved the client rate and sell price, discovered the
category, handled deducted minutes and iteration, and returned the saved entry.

Both surfaces are now adapters over one TimesheetCreateService.

Closes #57

Supersedes #68, which GitHub auto-closed when its base branch (the harness branch) was deleted at
#65's merge. Same work, rebased onto main on top of #65, #66 and #69.

What changed

• Features/Timesheets/TimesheetCreateService.cs, same prepare/apply shape as
  TimesheetUpdateService: TimesheetCreateOptions → PrepareAsync → a plan carrying the fully
  resolved TimesheetRequest → ApplyAsync posts and re-reads. It owns sell price (regular rate for
  B/W, prepaid for BPP), category (explicit → repo mapping → last fortnight's entries),
  location (WFH defaults + alias resolution), deducted minutes → hours, iteration and the read-back.
• ts create is an adapter and keeps its preview, confirmation and interactive missing-rate
  workflow: PrepareAsync returns a missing-rate outcome and never creates a rate.
• MCP CreateTimesheet calls the same service. On a missing rate it returns an error carrying
  RateGuard's recovery recipe (the same shape the CLI envelope uses) and writes nothing — no
  implicit rate, no timesheet.
• A malformed date now raises a TimesheetValidationException instead of escaping as a
  FormatException, so MCP answers with its error payload and the CLI with its --json envelope and
  exit 1 rather than an unhandled exception.
• TimesheetCreateService registered in both DI containers (Program.cs, McpHostCommand.cs).
• scripts/e2e/mcp_smoke.py now cross-checks the timesheetId the create tool reports against the
  row it reads back.
• AGENTS.md "Timesheet writes" names the create service and the never-writes-a-rate rule.

Wire shape (per the agreed policy)

The CLI ts create --json document is unchanged. The MCP result does change, deliberately: the
tool used to serialise the API's empty body as literal null (locked in
Goldens/Mcp/Tools/CreateTimesheet.populated.json), which no client could use. It now returns the
CLI's {"success":true,"timesheetId":N,"timesheet":{...}}, serialised with the CLI's options so the
two documents are byte-identical — the policy's permitted exception, since null was never a usable
contract. The create row in McpCliParityTable is ExpectParity = true with no permitted
differences. The tools/list discovery goldens are untouched: no argument, description or schema
changed.

Golden renames

Goldens/Mcp/Parity/* carries the parity row index in its filename, so inserting the create row
shifts the five rows after it. Ten snapshots are renamed with their contents byte-identical —
git diff -M shows them as pure renames.

How verified

By execution:

$ dotnet test tests/SSW.TimePro.Cli.Tests/
Passed!  - Failed: 0, Passed: 568

$ dotnet test tests/SSW.TimePro.Cli.Integration/
Passed!  - Failed: 0, Passed: 287

New tests:

• CliMcpCreatePayloadTests (13) — canonicalised SaveTimesheet payload equality between
  ts create and the tool for B/BPP/W pricing, category precedence (explicit / repo mapping /
  recent entries, asserting the query is skipped when the mapping hits), location alias and WFH
  default, iteration, and the identical result document. Plus, on both surfaces: an expired rate
  produces zero SaveClientRate and zero SaveTimesheet requests; an empty write response is
  followed by a GetTimesheetListViewModel after the single POST and the returned document
  carries the saved entry's id, project, note and start time; a malformed date writes nothing and
  returns the surface's error shape.
• TimesheetCreateServiceTests (22) — the service's own resolution rules and apply outcomes.

Forced failures (the checks can fail):

• SellPrice = null in the service → 4 unit tests, the three pricing payload-equality cases and the
  create parity row fail.
• dropping the read-back in ApplyAsync → Create_WhenTheApiAnswersWithAnEmptyBody_... fails on
  both surfaces.

Staging (--tenant ssw-staging, isProduction: false, apiUrl contains staging), CLI from source
on a Northwind project that uses iterations:

$ tp ts create --client NWIND --project <northwind project> --date <weekday> \
    --start 05:00 --end 05:15 --description "CLI verification, safe to delete" \
    --iteration <id> --category WEBDEV --yes --json
{ "success": true, "timesheetId": <id>,
  "timesheet": { "projectId": "<northwind project>", "iteration": "<name>",
                 "locationId": "SSW", "totalTime": 0.25, ... } }

$ tp ts get --date <weekday> --json      # the row is there with the note and iteration
$ tp ts delete <id> --date <weekday> --yes --json
{ "success": true, "timesheetId": <id> }
$ tp ts get --date <weekday> --json      # absent, 0 rows on the day

The MCP stdio gate is green end to end for the first time:

$ TIMEPRO_MCP_SMOKE_PROJECT=<northwind project> \
  TIMEPRO_MCP_SMOKE_TP="dotnet src/SSW.TimePro.Cli/bin/Release/net10.0/SSW.TimePro.Cli.dll" \
  scripts/e2e/test-mcp-smoke.sh
  tenant ssw-staging resolves to a non-production host: <staging host>
  initialize negotiated protocol 2024-11-05
  tools/list returned 18 tools, all 18 default tools present
  the configured NWIND project is present
  selected 1 of N iterations on the project
  client rate present for NWIND
  marker '<unique note>' is unused on <weekday>
  created and read back entry <id>
  cleaned up entry <id> and verified its absence
MCP smoke passed

Every staging row created during verification was deleted and its absence verified; the day ends with
0 rows. No staging rate was created.

By inspection only: the interactive missing-rate prompt (ts create without --yes/--json) — its
non-interactive and --reject-if-rate-expired branches are covered by tests, the prompt itself is
not automatable here.

Left out

• MCP create_timesheet still has no deducted-minutes argument, so that dimension of the
  payload-equality suite is CLI-only. Adding the argument changes the tool schema and the discovery
  goldens, which the policy puts in a contract release, not this patch.
• DateTime.Parse on the rate expiry keeps its existing culture-sensitive behaviour; moving it to
  invariant is a separate fix.

Risks

• Create writes are the blast radius. Payload equality plus the write-traffic assertions (expected
  method/path/body, forbidden requests, call counts) are the guard, and staging confirmed a real
  create/read-back/delete on both surfaces.
• MCP clients parsing the old null create result see a JSON object instead. That is the intended,
  documented break; nothing could have been reading a value out of null.
• The parity golden renames are index bookkeeping, not behaviour: any future row inserted mid-table
  will do the same until the harness keys those snapshots on something stable.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

The MCP smoke check must require a present, matching reported timesheet ID; the create-path guidance should also account for ts copy.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR centralizes CLI and MCP timesheet creation through a shared service, including pricing, validation, resolution, missing-rate handling, and read-back.

Changes:

• Adds TimesheetCreateService and registers it in CLI, MCP, and test hosts.
• Aligns CLI/MCP payloads and missing-rate behavior.
• Adds unit, integration, parity, golden, and smoke-test coverage.
• Updates documentation and parity snapshots.

File summaries

File	Summary
tests/SSW.TimePro.Cli.Tests/Features/Timesheets/TimesheetCreateServiceTests.cs	Service unit tests
tests/SSW.TimePro.Cli.Tests/Features/Timesheets/LessOptionTests.cs	CLI test dependency updates
tests/SSW.TimePro.Cli.Integration/Mcp/McpToolHost.cs	Test MCP host wiring
tests/SSW.TimePro.Cli.Integration/Mcp/McpToolCatalog.cs	MCP fixture updates
tests/SSW.TimePro.Cli.Integration/Mcp/McpCliParityTable.cs	Create parity coverage
tests/SSW.TimePro.Cli.Integration/Mcp/CliRunner.cs	CLI test host registration
tests/SSW.TimePro.Cli.Integration/Mcp/CliMcpCreatePayloadTests.cs	CLI/MCP payload and result tests
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/CreateTimesheet.populated.json	Create result golden
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/SearchClients.13.mcp.json	Renumbered MCP parity snapshot
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/SearchClients.13.cli.json	Renumbered CLI parity snapshot
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ListIterations.12.mcp.json	Renumbered MCP parity snapshot
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ListIterations.12.cli.json	Renumbered CLI parity snapshot
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetProjectsForClient.10.mcp.json	Renumbered MCP parity snapshot
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetProjectsForClient.10.cli.json	Renumbered CLI parity snapshot
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetCrmBookings.14.mcp.json	Renumbered MCP parity snapshot
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetCrmBookings.14.cli.json	Renumbered CLI parity snapshot
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetClientRate.11.mcp.json	Renumbered MCP parity snapshot
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetClientRate.11.cli.json	Renumbered CLI parity snapshot
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateTimesheet.9.mcp.json	MCP create parity snapshot
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateTimesheet.9.cli.json	CLI create parity snapshot
tests/SSW.TimePro.Cli.Integration/Features/McpTimesheetWriteTests.cs	MCP write test updates
src/SSW.TimePro.Cli/Program.cs	CLI service registration
src/SSW.TimePro.Cli/Features/Timesheets/TimesheetCreateService.cs	Shared create orchestration
src/SSW.TimePro.Cli/Features/Timesheets/CreateCommand.cs	CLI adapter integration
src/SSW.TimePro.Cli/Features/Rates/RateGuard.cs	Missing-rate recovery
src/SSW.TimePro.Cli/Features/Mcp/Tools/TimesheetMcpTools.cs	MCP adapter integration
src/SSW.TimePro.Cli/Features/Mcp/McpHostCommand.cs	MCP service registration
scripts/e2e/mcp_smoke.py	MCP create/read-back validation
AGENTS.md	Timesheet write guidance

Review details

Suppressed comments (1)

AGENTS.md:117

• CopyCommand is another create surface: it still builds a TimesheetRequest in CopyCommand.cs:137 and calls CreateTimesheetAsync at line 153 without a SellPrice. That bypasses this service and leaves tp ts copy exposed to the same missing-price 400 this change fixes for ts create/MCP, while this new guidance says no create should build requests elsewhere. Please either route ts copy through the service or narrow this statement to the migrated surfaces.

read-back that turns an empty write response into the saved row. `ts create` and the
`CreateTimesheet` MCP tool are adapters over it; never build a `TimesheetRequest` for a create
elsewhere either.

• Files reviewed: 19/29 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.