fix(leave): return the created entry from leave create

[jernejk]

Summary

tp leave create --json and the MCP CreateLeave tool both answered a bare {"success":true}, because POST /api/leave/ returns an empty body. Both now re-read the created entry and return it.

Closes #48

What changed

• LeaveCreateService.ApplyAsync returns a LeaveCreateResult (success, leaveId, leave, warning). It calls the new LeaveLookup.FindCreatedAsync, which pages UPCOMING then PAST for the requesting employee and matches on start date, end date, leave type and note.
• Matching requires leave type, note, the all-day flag and the dates, plus the slot to the minute for a partial day — two partial-day requests on one day differ only by their times, so dates alone could return the earlier one's id as the new entry. All-day entries are compared on the calendar date only (the server normalises their times and may echo the day in its own offset); partial-day entries prefer the offset-free startDateWithoutOffset/endDateWithoutOffset values and otherwise compare instants shifted into the offset the request was built in.
• Exactly one match wins. No match or several matches still report success: true with leave: null and a warning pointing at tp leave list --filter ALL — never a second create, which would duplicate the request. The read-back swallows every failure except the caller's own cancellation: a completed write is reported as a success, because surfacing it as an error is what invites the duplicate.
• CLI --json: {"success":true,"leaveId":"<guid>","leave":{...}} (additive; success stays first). Human output prints the id, and the warning goes to stderr.
• MCP CreateLeave serialises the same result. Dry-run is untouched on both surfaces: same validation, same proposed request, no POST.

Wire-shape decision

The only committed create_leave goldens were CreateLeave.dryRun.json and CreateLeave.apiError.json — there was no snapshot of a successful create, so nothing client-visible is being replaced. Per §2 of the wire-shape policy this is the permitted case for adopting the CLI's saved-entry document, so Goldens/Mcp/Tools/CreateLeave.created.json is a new snapshot, generated deliberately with UPDATE_MCP_GOLDENS=1 and reviewed.

The new parity row (CreateLeave / leave create, real write) asserts exactly one POST /api/leave/ followed by a GET /api/leave/, and forbids PUT /api/leave/. It stays ExpectParity = false with five permitted paths only because the CLI omits nulls that MCP writes (cancellationReason, startDateWithoutOffset, endDateWithoutOffset, timeLessOverride, plus warning) — the same declared difference as the existing timesheet write rows. The row is appended to the table because parity goldens are named by row index; inserting it next to the other leave rows would rename every snapshot below it.

How verified

By execution:

• dotnet test tests/SSW.TimePro.Cli.Tests/ → 583 passed, 0 failed (+15: thirteen LeaveCreateServiceTests — match, PAST-filter match, no match, ambiguous, read-back failing with ApiException / TimeProConnectionException / a generic exception, offset-free dates preferred when the offset dates fall on another day, morning-vs-afternoon partial days resolving correctly, same-slot partial days staying ambiguous, an all-day request refusing a partial-day entry, a server echoing another offset still matching, prepare-writes-nothing — and two CreateCommandTests for the --json shape and for dry-run writing nothing and never reading back).
• dotnet test tests/SSW.TimePro.Cli.Integration/ → 292 passed, 0 failed (+5: three LeaveCreateReadbackTests covering both surfaces, the new tool golden case, the new parity row).
• Forced failure, twice. With the read-back removed from ApplyAsync, 3 integration and 4 unit tests fail. With the all-day flag and the partial-day slot dropped from the matcher (dates only, offset values only), 4 unit tests fail. Restored after each, both suites green.
• Staging dry-run unchanged (no leave was created on staging — the approver emails make that unsafe, so the write path is covered by WireMock only):

$ dotnet run --project src/SSW.TimePro.Cli -- leave create --start 2026-04-01 --end 2026-04-01 \
    --type 1 --note "CLI verification, safe to delete" --dry-run --json --tenant ssw-staging
{
  "dryRun": true,
  "request": {
    "requestedEmpId": "<emp>",
    "startDate": "2026-04-01T00:00:00.0000000+11:00",
    "endDate": "2026-04-01T23:59:00.0000000+11:00",
    "leaveTypeId": 1,
    "note": "CLI verification, safe to delete",
    "userStartTime": "09:00:00",
    "userEndTime": "18:00:00",
    "allDay": true,
    "optionalEmp": []
  }
}

The partial-day path was dry-run on staging too (--half-day --start-time 13:00 --end-time 17:00), returning allDay: false with the slot in startDate/endDate, which is the shape the new matcher compares. tp info --json --tenant ssw-staging confirmed the non-production host before both runs.

By inspection only:

• The real server's create-then-list behaviour: whether a just-created request is always visible on the UPCOMING page it was submitted for, and how its startDateWithoutOffset is rendered. The matcher prefers the offset-free dates when present and falls back to the date part of the offset dates, which the WireMock fixture exercises, but it has not been run against a real create.
• Multi-page read-back (FindCreatedAsync pages until TotalPages) is unit-tested with a single page only.

Risks

• A genuine duplicate — same dates, type, note and slot — makes the match ambiguous; the entry is then reported as unidentified rather than guessed at, and the write is not retried.
• The read-back adds one GET /api/leave/ per create.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Unresolved findings remain in matching, connection-failure handling, warning guidance, and test coverage.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Updates leave creation so the CLI and MCP tool return the newly created leave after the API’s empty POST response.

Changes:

• Adds shared result handling and paginated read-back lookup.
• Updates CLI/MCP output while preserving dry-run behavior.
• Adds tests, parity coverage, goldens, and documentation.

The review identified unresolved concerns around cross-filter matching, connection-failure handling, warning guidance, and pagination/date-field coverage.

File summaries

File	Description
tests/SSW.TimePro.Cli.Tests/Features/Mcp/LeaveMcpToolsTests.cs	Updates MCP leave tool test wiring.
tests/SSW.TimePro.Cli.Tests/Features/Leave/LeaveCreateServiceTests.cs	Tests create results and read-back outcomes.
tests/SSW.TimePro.Cli.Tests/Features/Leave/CreateCommandTests.cs	Tests CLI JSON and dry-run behavior.
tests/SSW.TimePro.Cli.Integration/Mcp/McpToolHost.cs	Updates integration dependency wiring.
tests/SSW.TimePro.Cli.Integration/Mcp/McpToolCatalog.cs	Adds successful create coverage.
tests/SSW.TimePro.Cli.Integration/Mcp/McpCliParityTable.cs	Adds CLI/MCP parity coverage.
tests/SSW.TimePro.Cli.Integration/Mcp/LeaveCreateReadbackTests.cs	Verifies read-back traffic and no duplicate writes.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/CreateLeave.created.json	Adds the created-leave MCP golden.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateLeave.15.mcp.json	Adds the MCP parity golden.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateLeave.15.cli.json	Adds the CLI parity golden.
src/SSW.TimePro.Cli/Features/Mcp/Tools/LeaveMcpTools.cs	Serializes the shared create result.
src/SSW.TimePro.Cli/Features/Leave/LeaveLookup.cs	Finds created leaves across filters and pages.
src/SSW.TimePro.Cli/Features/Leave/LeaveCreateService.cs	Performs creation and read-back result handling.
src/SSW.TimePro.Cli/Features/Leave/CreateCommand.cs	Outputs the created ID and warnings.
AGENTS.md	Documents the create/read-back contract.

Review details

Suppressed comments (2)

src/SSW.TimePro.Cli/Features/Leave/LeaveLookup.cs:92

• The new offset-free date preference is not exercised by the added tests: all fixtures leave StartDateLocal and EndDateLocal null, so only the fallback fields are tested. Add a case with the server's *DateWithoutOffset values populated (and differing from the offset date) to protect the real-server matching path.

    private static bool Matches(LeaveEntry entry, CreateLeaveRequest request) =>
        SameDay(entry.StartDateLocal ?? entry.StartDate, request.StartDate)
        && SameDay(entry.EndDateLocal ?? entry.EndDate, request.EndDate)
        && entry.LeaveType?.Id == request.LeaveTypeId
        && string.Equals(entry.Note?.Trim() ?? "", request.Note?.Trim() ?? "", StringComparison.Ordinal);

src/SSW.TimePro.Cli/Features/Leave/LeaveLookup.cs:61

• A network failure during this read-back is raised as TimeProConnectionException by TimeProApiClient, not ApiException. In that case the POST has already succeeded but the exception escapes ApplyAsync, so the CLI/MCP reports a failed create instead of the documented success-with-warning outcome; catch connection failures here as well while preserving cancellation behavior.

        catch (ApiException)
        {
            // The create already succeeded; a failed read-back must not turn it into an error.
        }

• Files reviewed: 15/15 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.