Skip to content

Expose Xero leave balance import and status to CLI and MCP - #14

Merged
jernejk merged 7 commits into
mainfrom
agent/leave-balance-import
Aug 29, 2026
Merged

Expose Xero leave balance import and status to CLI and MCP#14
jernejk merged 7 commits into
mainfrom
agent/leave-balance-import

Conversation

@Freego1783

Copy link
Copy Markdown
Member

Surfaces the two endpoints added by SSW.TimePRO PR 3493 (POST /api/leave/balances/import, GET /api/leave/balances/status) as CLI commands and MCP tools.

What's new

CLI — a new balances branch, kept separate from the existing singular leave balance (per-employee stats):

tp leave balances status
tp leave balances import <PATH> --yes

MCPget_leave_balance_status and import_leave_balances, on the default leave surface. Authorization stays server-side, so the API's own 403 is the real gate.

Two things worth reviewing

A new PostRawAsync helper. The import endpoint reads the request stream as the raw Xero CSV, so it can't use the existing POST helpers — JsonContent would send an escaped string literal and fail the server-side parser. LeaveBalancesApiTests pins the request body and text/csv content type against exactly that regression.

Both surfaces take a path, not file contents. MCP tool arguments travel through the model's context, where a large CSV is expensive and liable to be silently truncated into a partial import that still reports success. LeaveBalanceImportService owns reading and validating the file so CLI and MCP behave identically, and translates the endpoint's 403 and 422 into readable messages.

No dry-run

The import replaces balances company-wide and the API has no dry-run, so the CLI confirms unless --yes is passed. A real preview would need a dryRun flag on the API side — everything the result DTO reports is already computed before the handler's single SaveChangesAsync. Deliberately left out of this PR rather than approximated client-side, which would mean duplicating XeroLeaveBalanceCsvParser and its locale-sensitive decimal handling.

Testing

28 new tests (unit, CLI, MCP, WireMock integration), all passing. Verified end to end against a live tenant: status reads correctly, a real Xero CSV export imports, and a non-CSV file is rejected before any network call.

Note: main currently has 23 pre-existing test failures (Guides/Skills and CheckLeaveTests). Confirmed present on a clean checkout — unrelated to this branch.

🤖 Generated with Claude Code

Freego1783 and others added 3 commits August 28, 2026 14:39
TimePro added POST /api/leave/balances/import and GET /api/leave/balances/status
(PR 3493). Surface both as CLI commands under a new "leave balances" branch, and
as MCP tools on the default leave surface.

The import endpoint reads the request stream as the raw Xero CSV export, so it
needs a PostRawAsync helper: every existing POST goes through JsonContent, which
would send an escaped string literal and fail the server-side parser. An
integration test pins the body and text/csv content type against that regression.

Both surfaces take a path to the CSV rather than its contents. MCP tool arguments
travel through an agent's context, where a large CSV is expensive and liable to be
silently truncated into a partial import that still reports success.
LeaveBalanceImportService owns reading and validating the file so the CLI and MCP
behave identically, and translates the endpoint's 403 and 422 into readable
messages. The import replaces balances company-wide with no server-side dry-run,
so the CLI confirms unless --yes is passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A shell that strips backslashes turns an absolute Windows path into a
drive-relative one ("C:\Users\me\x.csv" -> "C:Usersmex.csv"), which resolves
against the working directory. The resolved path alone gives no hint that this
happened, so report the original alongside it when the two differ.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
At 618 characters it was 2.4x the next longest tool description in the server
and 7x the median. Dropped the leave-admin pre-warning (the service already
returns a clear message when the API refuses), the pointer to
get_leave_balance_status (that tool's own description already says to check it
before importing), and the gloss on what unmatchedEmployees means.

Kept the parts a model gets wrong without them: the blast radius, that there is
no undo, that it takes a path rather than file contents, and that skipped rows
must be surfaced. 337 characters.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Freego1783
Freego1783 requested a review from jernejk August 28, 2026 14:47

@jernejk jernejk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jernejk
jernejk merged commit 47e1ddf into main Aug 29, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants