Expose Xero leave balance import and status to CLI and MCP - #14
Merged
Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
balancesbranch, kept separate from the existing singularleave balance(per-employee stats):MCP —
get_leave_balance_statusandimport_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
PostRawAsynchelper. The import endpoint reads the request stream as the raw Xero CSV, so it can't use the existing POST helpers —JsonContentwould send an escaped string literal and fail the server-side parser.LeaveBalancesApiTestspins the request body andtext/csvcontent 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.
LeaveBalanceImportServiceowns 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
--yesis passed. A real preview would need adryRunflag on the API side — everything the result DTO reports is already computed before the handler's singleSaveChangesAsync. Deliberately left out of this PR rather than approximated client-side, which would mean duplicatingXeroLeaveBalanceCsvParserand its locale-sensitive decimal handling.Testing
28 new tests (unit, CLI, MCP, WireMock integration), all passing. Verified end to end against a live tenant:
statusreads correctly, a real Xero CSV export imports, and a non-CSV file is rejected before any network call.Note:
maincurrently has 23 pre-existing test failures (Guides/Skills andCheckLeaveTests). Confirmed present on a clean checkout — unrelated to this branch.🤖 Generated with Claude Code