Skip to content

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

Closed
jernejk wants to merge 2 commits into
test/mcp-regression-harnessfrom
fix/timesheet-create-service
Closed

fix(timesheets): shared create service for ts create and MCP#68
jernejk wants to merge 2 commits into
test/mcp-regression-harnessfrom
fix/timesheet-create-service

Conversation

@jernejk

@jernejk jernejk commented Sep 12, 2026

Copy link
Copy Markdown
Member

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

Based on #65 (test/mcp-regression-harness), not main — it uses the harness's CliRunner,
McpToolHost, NorthwindApi, McpCliParityTable and goldens. Merge #65 first; I will rebase onto
main on request.

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 with the
    tp rate create recovery command and writes nothing — no implicit rate, no timesheet.
  • TimesheetCreateService registered in both DI containers (Program.cs, McpHostCommand.cs).
  • scripts/e2e/mcp_smoke.py: dropped the wrapper that explained why create was expected to fail, and
    it now cross-checks the reported timesheetId 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.

How verified

By execution:

$ dotnet test tests/SSW.TimePro.Cli.Tests/
Passed!  - Failed: 0, Passed: 503 (was 482)

$ dotnet test tests/SSW.TimePro.Cli.Integration/
Passed!  - Failed: 0, Passed: 276 (was 263)

New tests:

  • CliMcpCreatePayloadTests (12) — 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: an expired rate produces zero
    SaveClientRate and zero SaveTimesheet requests on both surfaces    , and an empty write response
    is followed by a read-back, never a second SaveTimesheet.
  • TimesheetCreateServiceTests (21) — the service's own resolution rules and apply outcomes.
  • CliConsole: CliRunner redirects the process-wide Console.Out, so the two harness classes that
    run the real command tree now share one xUnit collection. Without it they raced and contaminated
    each other's CLI goldens.

Forced failure (the checks can fail): setting SellPrice = null in the service fails 4 unit tests,
the three pricing payload-equality cases and the create parity row.

Staging (--tenant ssw-staging, isProduction: false, apiUrl contains staging), CLI from source
on project 8W52M2 with an iteration:

$ tp ts create --client NWIND --project 8W52M2 --date <weekday> --start 04:00 --end 04:15 \
    --description "CLI verification, safe to delete" --iteration <id> --category WEBDEV --yes --json
{
  "success": true,
  "timesheetId": <id>,
  "timesheet": { "projectId": "8W52M2", "iteration": "Iteration 1", "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

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

$ 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
  project 8W52M2 found: Cloud Transition
  using iteration 258 (Epic 1 - AWS to Azure)
  client rate present for NWIND
  created and read back entry <id> on project 8W52M2
  deleted the smoke entry and verified its absence
MCP smoke passed

Both staging rows were deleted and their absence verified. No staging rate was created.

By inspection only: the interactive missing-rate prompt (ts create without --yes/--json) — the
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 new xUnit collection serialises two integration classes; suite duration is unchanged (~8s).

🤖 Generated with Claude Code

jernejk and others added 2 commits September 13, 2026 00:35
@jernejk @claude
test(mcp): regression harness for tools/list, tool results, CLI parit…
47616f3 
…y and staging stdio

Locks the observable MCP contract before the CLI/MCP unification slices touch it.

- Golden tools/list taken from the real stdio server, accounting off (18) and on (47)
- Per-tool result goldens for all 47 tools plus generated empty and API-error cases
- tools/call envelopes over stdio for six tools, proven identical to the direct calls
- CLI/MCP parity runner with per-case permitted differences and write-traffic assertions
- Reflection test mapping every tool to a registered CLI command, with a shrink-only allowlist
- scripts/e2e/test-mcp-smoke.sh: staging stdio gate with non-production check and cleanup

ConfigPaths honours TIMEPRO_CLI_CONFIG_DIR so a child MCP host can run against an
isolated config instead of the developer's own tenants and feature flags.

Closes #63

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jernejk @claude
fix(timesheets): shared create service for ts create and MCP
fb24fc9 
MCP create_timesheet built its own TimesheetRequest with no sell price, so a server that
cannot derive one answered 400 and entries created through MCP could differ from CLI ones
on the same inputs.

TimesheetCreateService now owns every resolution a create needs — sell price from the
client rate for the billable type, category from the repo mapping then recent entries,
location from the WFH defaults, deducted minutes, iteration, and the read-back that turns
an empty write response into the saved row. `ts create` and the MCP tool are adapters over
it. The interactive missing-rate workflow stays in the command: PrepareAsync reports a
missing rate and never writes one, and MCP answers with the `tp rate create` recovery
command instead.

The CreateTimesheet result golden changes deliberately. The tool returned the API's empty
body serialised as literal `null`, which no client could use; it now returns the CLI's
`{"success":true,"timesheetId":N,"timesheet":{...}}` document, serialised with the CLI's
options so the two are byte-identical. The create row in McpCliParityTable is at parity
with no permitted differences.

The populated golden case now writes the note and start time of the day's existing row so
the empty-body read-back has something to find.

Closes #57

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 12, 2026 14:55
@jernejk
jernejk deleted the branch test/mcp-regression-harness September 12, 2026 14:56
@jernejk jernejk closed this Sep 12, 2026
@jernejk jernejk mentioned this pull request Sep 12, 2026
Merged
@jernejk

jernejk commented Sep 12, 2026

Copy link
Copy Markdown
Member Author

Superseded by #70: same branch rebased onto main (on top of #65, #66 and #69) with the review fixes. GitHub auto-closed this one when the harness base branch was deleted and will not reopen it.

Copilot AI reviewed Sep 12, 2026
View reviewed changes

Copilot AI 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.

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds a shared timesheet creation service used by both CLI and MCP, including pricing, category/location resolution, and read-back.

Changes:

  • Refactors CLI and MCP creation through TimesheetCreateService.
  • Adds parity, golden, configuration-isolation, and staging smoke coverage.
  • Updates registrations and repository guidance.
File summaries
File Description
tests/SSW.TimePro.Cli.Tests/Infrastructure/ConfigPathsTests.cs Updated as part of this pull request.
tests/SSW.TimePro.Cli.Tests/Features/Timesheets/LessOptionTests.cs Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Mcp/WeekTokens.cs Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Mcp/TestConfigService.cs Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Mcp/McpToolResultGoldenTests.cs Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Mcp/McpToolInventory.cs Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Mcp/McpToolHost.cs Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Mcp/McpToolCase.cs Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Mcp/McpStdioToolCallTests.cs Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Mcp/McpStdioDiscoveryTests.cs Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Mcp/McpStdioClient.cs Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Mcp/JsonDiff.cs Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Mcp/IsolatedConfigDirectory.cs Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Mcp/Golden.cs Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Mcp/CliRunner.cs Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Mcp/CliMcpParityTests.cs Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Mcp/CliConsole.cs Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Mcp/CliBinary.cs Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/UpdateTimesheet.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/UpdateTimesheet.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/UpdateLeave.dryRun.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/UpdateLeave.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/SearchClients.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/SearchClients.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/SearchClients.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/QueryTimesheets.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/QueryTimesheets.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/QueryTimesheets.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListRecurringInvoices.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListRecurringInvoices.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListRecurringInvoices.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListProducts.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListProducts.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListProducts.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListPaidReceipts.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListPaidReceipts.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListPaidReceipts.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListLocations.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListLocations.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListLocations.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListIterations.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListIterations.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListIterations.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListInvoices.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListInvoices.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListInvoices.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListCreditNotes.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListCreditNotes.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListCreditNotes.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListClientRates.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListClientRates.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListClientRates.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListCategories.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListCategories.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListCategories.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListBillableTypes.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListBillableTypes.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListBillableTypes.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListAllSkus.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListAllSkus.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListAllSkus.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ImportLeaveBalances.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ImportLeaveBalances.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetUnpaidInvoicesByClient.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetUnpaidInvoicesByClient.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetUnpaidInvoicesByClient.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetUnbilledTimesheetsForClient.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetUnbilledTimesheetsForClient.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetUnbilledTimesheetsForClient.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetTimesheets.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetTimesheets.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetTimesheets.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetSuggestedTimesheets.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetSuggestedTimesheets.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetSuggestedTimesheets.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetRecurringInvoice.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetRecurringInvoice.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetRecurringInvoice.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetReceiptDetail.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetReceiptDetail.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetReceiptDetail.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetProjectsSummary.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetProjectsSummary.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetProjectsSummary.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetProjectsForClient.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetProjectsForClient.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetProjectsForClient.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetProductDiscountsForClient.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetProductDiscountsForClient.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetProductDiscountsForClient.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetProduct.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetProduct.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetProduct.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetPrepaidStatusPdf.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetPrepaidStatusPdf.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetPrepaidStatus.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetPrepaidStatus.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetPrepaidStatus.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetLocationAndMapping.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetLeaveEntries.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetLeaveEntries.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetLeaveEntries.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetLeaveBalanceStatus.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetLeaveBalanceStatus.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetLeaveBalanceStatus.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetLeaveBalance.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetLeaveBalance.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetLeaveBalance.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetInvoiceTimesheets.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetInvoiceTimesheets.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetInvoiceTimesheets.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetInvoicesByClient.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetInvoicesByClient.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetInvoicesByClient.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetInvoiceReceipts.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetInvoiceReceipts.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetInvoiceReceipts.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetInvoiceLines.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetInvoiceLines.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetInvoiceLines.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetInvoice.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetInvoice.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetInvoice.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetCurrentUser.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetCurrentUser.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetCurrentUser.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetCrmBookings.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetCrmBookings.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetCrmBookings.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetClientsWithOutstandingTime.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetClientsWithOutstandingTime.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetClientsWithOutstandingTime.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetClientRate.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetClientRate.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetClientRate.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetClientOutstanding.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetClientOutstanding.empty.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetClientOutstanding.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/DeleteTimesheet.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/DeleteTimesheet.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/CreateTimesheet.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/CreateTimesheet.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/CreateLeave.dryRun.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/CreateLeave.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/CheckWeek.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/CheckWeek.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/AcceptSuggestedTimesheet.populated.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/AcceptSuggestedTimesheet.apiError.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/UpdateTimesheet.0.mcp.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/UpdateTimesheet.0.cli.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/UpdateLeave.6.mcp.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/UpdateLeave.6.cli.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ImportLeaveBalances.7.mcp.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ImportLeaveBalances.7.cli.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetLeaveBalanceStatus.8.mcp.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetLeaveBalanceStatus.8.cli.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/DeleteTimesheet.3.mcp.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/DeleteTimesheet.3.cli.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/DeleteTimesheet.2.mcp.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/DeleteTimesheet.2.cli.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateTimesheet.9.mcp.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateTimesheet.9.cli.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateLeave.5.mcp.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateLeave.5.cli.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CheckWeek.4.mcp.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CheckWeek.4.cli.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/AcceptSuggestedTimesheet.1.mcp.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/AcceptSuggestedTimesheet.1.cli.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Calls/update_timesheet.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Calls/list_iterations.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Calls/get_timesheets.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Calls/get_leave_entries.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Calls/get_client_rate.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Calls/delete_timesheet.json Updated as part of this pull request.
tests/SSW.TimePro.Cli.Integration/Features/McpTimesheetWriteTests.cs Updated as part of this pull request.
src/SSW.TimePro.Cli/Program.cs Updated as part of this pull request.
src/SSW.TimePro.Cli/Infrastructure/Config/ConfigPaths.cs Updated as part of this pull request.
src/SSW.TimePro.Cli/Features/Timesheets/TimesheetCreateService.cs Updated as part of this pull request.
src/SSW.TimePro.Cli/Features/Mcp/Tools/TimesheetMcpTools.cs Updated as part of this pull request.
src/SSW.TimePro.Cli/Features/Mcp/McpHostCommand.cs Updated as part of this pull request.
scripts/e2e/test-mcp-smoke.sh Updated as part of this pull request.
AGENTS.md Updated as part of this pull request.
.gitignore Updated as part of this pull request.
Review details

Suppressed comments (5)

scripts/e2e/mcp_smoke.py:95

  • The timer does not enforce the advertised timeout here: readline() blocks until a line arrives, so the loop never gets back to deadline.is_set() while a hung MCP child is silent. Use a non-blocking/select-based read or a reader thread/queue so a stalled server cannot hang the E2E job indefinitely.
                if deadline.is_set():
                    raise SmokeFailure(f"timed out waiting for response {request_id}")

                line = self._process.stdout.readline()
                if line == "":

scripts/e2e/mcp_smoke.py:303

  • After a successful create, the failure path only reports created_id and closes the process; it never attempts deletion. Any exception after the write (for example, a read-back mismatch or a tool error) therefore leaves staging data behind, despite the script's cleanup guarantee. Move deletion into a failure-safe cleanup path and make cleanup failure fail the run.
    except SmokeFailure as failure:
        print(f"MCP smoke FAILED: {failure}", file=sys.stderr)
        if created_id is not None:
            print(
                f"LEFTOVER TEST DATA: timesheet {created_id} on {date} "

scripts/e2e/mcp_smoke.py:265

  • created_id is assigned only after the read-back succeeds, so if the create succeeds but get_timesheets fails or returns an unexpected result, the except path sees None and cannot identify the row for cleanup. Capture the returned timesheetId immediately, then replace it with the matched row's ID once read-back succeeds.
        after = call_tool(process, "get_timesheets", {"date": date})
        matches = [entry for entry in after if entry.get("notes") == note]

scripts/e2e/mcp_smoke.py:274

  • Allowing None here means the smoke gate passes even if create_timesheet regresses to the old null/empty result; it only verifies that some row exists. Since this change makes the returned ID part of the create result, require the reported ID to equal the row read back.
        if reported_id not in (None, created_id):

tests/SSW.TimePro.Cli.Integration/Mcp/McpStdioClient.cs:65

  • If InitializeAsync fails due to a timeout, cancellation, or a protocol error, StartAsync exits without disposing client or its child process. The MCP host can remain blocked on stdin after a failed test, and repeated failures leak processes; dispose the client in a catch before rethrowing.
  • Files reviewed: 191/192 changed files
  • Comments generated: 3
  • Review effort level: Lite

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

Comment thread scripts/e2e/mcp_smoke.py
READ_TIMEOUT_SECONDS = 90

CLIENT_ID = "NWIND"
PROJECT_ID = os.environ.get("TIMEPRO_MCP_SMOKE_PROJECT", "8W52M2")
Comment thread src/SSW.TimePro.Cli/Features/Timesheets/TimesheetCreateService.cs
Comment on lines +132 to +137
var active = rate?.Rate is not null
&& (string.IsNullOrEmpty(rate.ExpiryDate) || RateResolver.IsActive(DateTime.Parse(rate.ExpiryDate), date));

return active
? RateResolver.SellPriceFor(billableId, rate!.Rate ?? 0m, rate.PrepaidRate ?? 0m)
: null;
Comment thread scripts/e2e/test-mcp-smoke.sh
#
# TIMEPRO_MCP_SMOKE_TP how to invoke the candidate CLI (default: tp)
# TIMEPRO_MCP_SMOKE_TENANT tenant config to bind (default: ssw-staging)
# TIMEPRO_MCP_SMOKE_PROJECT Northwind project that uses iterations (default: 8W52M2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jernejk