diff --git a/AGENTS.md b/AGENTS.md index 6b92974..42831c9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -53,6 +53,8 @@ Run `tp --help` for full command list. Key commands: - `tp leave cancel ID --reason "..." --yes` - Cancel leave - `tp leave list --filter UPCOMING --json` - List leave - `tp leave balance --emp-id JEK` - Leave-usage signal (days since last leave + hours taken in last 12 months) +- `tp leave balances status` - When leave balances were last imported from Xero, and whether they are stale +- `tp leave balances import ./LeaveBalances.csv --yes` - Import company-wide leave balances from a Xero CSV export (leave admins only) - `tp feature accounting enable` - Enable accounting skills and accounting MCP tools - `tp feature developer enable` - Enable developer diagnostics/environment comparison skills and timesheet/finance bug diagnostic skills - `tp mcp [--tenant NAME]` - Start MCP server (optional per-session tenant binding) @@ -98,6 +100,33 @@ validation and payload preparation, returns the proposed request, and must not c The cancel endpoint (`PUT /api/leave/{id}/cancel`) requires `LeaveId` (Guid) and `CancellationReason` in the request body. +## Leave Balances (Xero CSV Sync) + +`POST /api/leave/balances/import` takes the raw Xero "Leave Balances" CSV export as the +request body with content type `text/csv`. It is **not** a JSON endpoint and **not** a +multipart upload, so it must go through `PostRawAsync`, never the `PostAsync`/`PutAsync` +JSON helpers - `JsonContent` would send an escaped string literal and the server-side +parser would reject it. `LeaveBalancesApiTests` asserts the body and content type for +exactly this reason. + +The endpoint is leave-admin only (`403` otherwise) and returns `422` with the CSV parser's +message as a bare JSON string when the file cannot be read. Both are translated into +readable messages by `LeaveBalanceImportService`, which owns file reading and validation +for the CLI and MCP alike. Import replaces stored balances for every matched employee; +there is no server-side dry-run, so the CLI confirms unless `--yes` is passed. + +CLI and MCP surfaces take a **path** to the CSV, not its contents. 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 looks successful. `GET /api/leave/balances/status` +is the cheap read used to decide whether a re-import is due. + +The read-only MCP status tool is on the default leave surface. The destructive MCP import +tool is available only when the accounting feature pack is enabled. + +Rows whose Xero employee name matches no TimePro employee, or matches several, are returned +in `unmatchedEmployees` and skipped rather than guessed at; implausible balances are returned +in `warnings`. The import succeeds regardless, so both lists must be surfaced to the user. + The list endpoint (`GET /api/leave/`) returns per-entry `daysAway`, `updatedAt`, `optionalEmp`, `timeLessOverride`, `cancellationReason` (all bound on `LeaveEntry`) plus a top-level `cancelledCount` on the list envelope. These surface in `tp leave list --json`. ## Testing diff --git a/README.md b/README.md index 18a3d91..8fd407a 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ SSW TimePro is a time tracking and invoicing system. This CLI makes it fast to v - **Timesheet CRUD** — Create, update, delete timesheets with rate checking and lock detection - **Suggested Timesheets** — View and accept suggested timesheets to keep accuracy stats high - **CRM Bookings** — See your appointments from the CRM calendar -- **Leave Management** — Create, list, update, and cancel EasyLeave requests +- **Leave Management** — Create, list, update, and cancel EasyLeave requests; leave admins can sync company-wide leave balances from a Xero CSV export - **Repo Mapping** — Map git repos to clients/projects; auto-detects via path or remote URL, with git worktree support; optional `--issues-repo` for projects whose issues live in a different GitHub repo than the code - **Daily Scrum** — Generate an SSW-format daily scrum email from timesheets, CRM bookings and GitHub activity, with AutoScrum-inspired `--smart` selection, overridable per-tenant/client templates, rich-text / markdown / plain clipboard support and an interactive copy mode - **Location Defaults** — Set WFH days so location is auto-applied when creating timesheets @@ -132,6 +132,8 @@ tp ts get 2026-03-12 # Specific date | `tp leave create` | Create a leave request (see options and `--dry-run` below) | | `tp leave update ID` | Update a leave request while preserving unspecified API-returned fields; supports `--dry-run` | | `tp leave cancel ID` | Cancel a leave request (`--reason`) | +| `tp leave balances status` | Show when leave balances were last imported from Xero and whether they are stale | +| `tp leave balances import PATH` | Import leave balances for all employees from a Xero CSV export (leave admins only) | | `tp cl search QUERY` | Search for clients | | `tp proj list --client ID` | List projects for a client | | `tp proj recent` | Surface projects you've recently logged time against (likely picks for new entries) | @@ -244,6 +246,12 @@ tp leave update --start 2026-04-01 --end 2026-04-01 \ # Cancel a leave request tp leave cancel --reason "Plans changed" --yes + +# Check whether the stored leave balances are still current +tp leave balances status --json + +# Import the latest Xero leave balances export (leave admins only) +tp leave balances import ~/Downloads/LeaveBalances.csv --yes ``` Leave create options: @@ -266,6 +274,16 @@ explicit. Use `--clear-approved-by` or `--clear-cc` to remove those values, and `--dry-run`; combine it with `--json` to inspect the exact API payload without creating or changing leave. +**Leave balances (Xero sync).** `tp leave balances import` uploads the raw Xero "Leave +Balances" CSV export and replaces the balances stored in TimePro for every employee it can +match. It requires leave admin rights, has no dry-run and no undo, so it prompts unless you +pass `--yes`. Rows whose Xero name matches no TimePro employee — or matches more than one — +are reported as skipped rather than guessed at, and implausibly large balances are flagged +as warnings; the import still succeeds, so check both lists afterwards. Run +`tp leave balances status` first to see whether a re-import is actually due. The accounting +MCP tool takes the path to the CSV rather than its contents, so the file never has to pass +through an agent's context. + ### Week View Compact view shows one line per timesheet with totals: @@ -569,7 +587,7 @@ Current default tool groups include: |-------|----------| | Timesheets | Get, create, update, delete, suggested timesheets, accept suggestions, list iterations, `check_week` (leave-aware weekly coverage) | | Lookup | Search clients, list projects, get client rate, CRM bookings, location and repo mapping | -| Leave | List EasyLeave entries (optionally filtered by `empId`), create and safely update EasyLeave requests with dry-run previews, `get_leave_balance` (days since last leave + 12-month hours) | +| Leave | List EasyLeave entries (optionally filtered by `empId`), create and safely update EasyLeave requests with dry-run previews, `get_leave_balance` (days since last leave + 12-month hours), and `get_leave_balance_status` (Xero balance sync status) | Optional accounting MCP tools are enabled with: @@ -577,7 +595,7 @@ Optional accounting MCP tools are enabled with: tp feature accounting enable ``` -That adds invoices, receipts, credit notes, products/SKUs, client rates, unbilled time, timesheet queries, current user/reference-code reporting, recurring invoices, and prepaid drawdown status. More complex accounting diagnostics live in guide-backed Markdown skills so teams can extend the collection without adding a dedicated command for every report. +That adds invoices, receipts, credit notes, products/SKUs, client rates, unbilled time, timesheet queries, current user/reference-code reporting, recurring invoices, prepaid drawdown status, and the leave-admin-only `import_leave_balances` tool. More complex accounting diagnostics live in guide-backed Markdown skills so teams can extend the collection without adding a dedicated command for every report. Developer diagnostics are CLI/skill workflows. Enable the generated developer skills with: @@ -608,6 +626,7 @@ Then ask Claude things like: - "Accept the suggested timesheet for Monday" - "What's my billing rate for Northwind?" - "Move my upcoming leave to Wednesday and keep its other details" +- "Are the leave balances up to date? If not, import ~/Downloads/LeaveBalances.csv" ### VS Code (Copilot / Continue) diff --git a/docs/skill-generation.md b/docs/skill-generation.md index c7d39d7..5cd4fb9 100644 --- a/docs/skill-generation.md +++ b/docs/skill-generation.md @@ -105,6 +105,7 @@ The timesheets skill keeps: - `tp info --json` as the first health/update check, preferred over `tp --version` - `tp project recent --json` as the first project-selection step - the existing timesheet, booking, leave, repo mapping, scrum, and troubleshooting guidance +- read-only EasyLeave balance freshness via CLI and the default MCP status tool - Northwind-only examples (`NWIND`, `1I776Q`, `Northwind/traders-app`) The tenant setup skill keeps: @@ -119,7 +120,8 @@ The tenant setup skill keeps: The accounting skill keeps: - `allowed-tools: Bash(tp *)` -- instruction-only read-only accounting workflows +- instruction-only accounting workflows, read-only except for explicitly approved Xero leave-balance imports +- default-MCP balance status plus accounting-gated MCP import guidance - client billable-work threshold report guidance, including the `.rows` JSON envelope shape - deeper reconciliation diagnostics for Excel, CSV, Xero MCP, bank-feed MCP, or another external source - guidance to check `tp accounting guide` first, then use specific recipes under `guides/accounting/` diff --git a/src/SSW.TimePro.Cli/Features/Leave/BalancesImportCommand.cs b/src/SSW.TimePro.Cli/Features/Leave/BalancesImportCommand.cs new file mode 100644 index 0000000..eca8c64 --- /dev/null +++ b/src/SSW.TimePro.Cli/Features/Leave/BalancesImportCommand.cs @@ -0,0 +1,122 @@ +using System.ComponentModel; +using SSW.TimePro.Cli.Infrastructure.ApiClient; +using SSW.TimePro.Cli.Infrastructure.Config; +using SSW.TimePro.Cli.Infrastructure.Output; +using Spectre.Console; +using Spectre.Console.Cli; + +namespace SSW.TimePro.Cli.Features.Leave; + +[Description("Import leave balances for all employees from a Xero leave balances CSV export")] +public class BalancesImportCommand : AsyncCommand +{ + private readonly LeaveBalanceImportService _importService; + private readonly IConfigService _config; + + public class Settings : CommandSettings + { + [CommandArgument(0, "")] + [Description("Path to the Xero 'Leave Balances' CSV export")] + public string CsvPath { get; set; } = string.Empty; + + [CommandOption("--yes")] + [Description("Skip confirmation")] + public bool Yes { get; set; } + + [CommandOption("--json")] + [Description("Output as JSON")] + public bool Json { get; set; } + } + + public BalancesImportCommand(LeaveBalanceImportService importService, IConfigService config) + { + _importService = importService; + _config = config; + } + + protected override async Task ExecuteAsync(CommandContext context, Settings settings, CancellationToken cancellationToken) + { + if (_config.LoadActiveTenantConfig() is null) + { + WriteError(settings.Json, "Not logged in. Run 'tp login --tenant ' first."); + return 1; + } + + // Validate the file before prompting so an unusable path fails immediately. + try + { + _importService.ReadCsv(settings.CsvPath); + } + catch (LeaveBalanceImportValidationException ex) + { + WriteError(settings.Json, ex.Message); + return 1; + } + + // The import replaces stored balances for every matched employee and TimePro offers no + // dry-run, so confirm unless the caller has opted out. + if (!settings.Yes && !settings.Json) + { + AnsiConsole.MarkupLine( + $"About to import leave balances for [bold]all employees[/] from {Markup.Escape(settings.CsvPath)}."); + if (!AnsiConsole.Confirm("This replaces the balances currently stored in TimePro. Continue?", false)) + return 1; + } + + try + { + var result = await _importService.ImportAsync(settings.CsvPath, cancellationToken); + + OutputHelper.Render(result, settings.Json, r => + { + var table = new Table().NoBorder().HideHeaders().AddColumn("Key").AddColumn("Value"); + table.AddRow("[bold]Balances as at[/]", r.AsAtDate.ToString("yyyy-MM-dd")); + table.AddRow("[bold]Created[/]", r.Created.ToString()); + table.AddRow("[bold]Updated[/]", r.Updated.ToString()); + AnsiConsole.Write(table); + + foreach (var warning in r.Warnings ?? []) + OutputHelper.WriteWarning(warning); + + var unmatchedEmployees = r.UnmatchedEmployees ?? []; + if (unmatchedEmployees.Count > 0) + { + OutputHelper.WriteWarning( + $"{unmatchedEmployees.Count} row(s) were skipped because the name did not match exactly one TimePro employee:"); + foreach (var name in unmatchedEmployees) + AnsiConsole.MarkupLine($" - {Markup.Escape(name)}"); + } + + OutputHelper.WriteSuccess($"Imported leave balances ({r.Created} created, {r.Updated} updated)"); + }); + + return 0; + } + catch (LeaveBalanceImportValidationException ex) + { + WriteError(settings.Json, ex.Message); + return 1; + } + catch (LeaveBalanceImportUncertainException ex) + { + WriteError(settings.Json, ex.Message); + return 1; + } + catch (ApiException ex) + { + var detail = ApiErrorParser.ExtractDetail(ex.ResponseBody); + if (settings.Json) + OutputHelper.WriteJsonError($"API error: {ex.Message}", ex.StatusCode, detail); + OutputHelper.WriteError($"API error ({ex.StatusCode}): {ex.Message}" + + (detail is not null ? $" — {detail}" : "")); + return 1; + } + } + + private static void WriteError(bool json, string message) + { + if (json) + OutputHelper.WriteJsonError(message); + OutputHelper.WriteError(message); + } +} diff --git a/src/SSW.TimePro.Cli/Features/Leave/BalancesStatusCommand.cs b/src/SSW.TimePro.Cli/Features/Leave/BalancesStatusCommand.cs new file mode 100644 index 0000000..961f6b0 --- /dev/null +++ b/src/SSW.TimePro.Cli/Features/Leave/BalancesStatusCommand.cs @@ -0,0 +1,80 @@ +using System.ComponentModel; +using SSW.TimePro.Cli.Infrastructure.ApiClient; +using SSW.TimePro.Cli.Infrastructure.Config; +using SSW.TimePro.Cli.Infrastructure.Output; +using Spectre.Console; +using Spectre.Console.Cli; + +namespace SSW.TimePro.Cli.Features.Leave; + +[Description("Show when leave balances were last imported from Xero and whether they are stale")] +public class BalancesStatusCommand : AsyncCommand +{ + private readonly ITimeProApiClient _api; + private readonly IConfigService _config; + + public class Settings : CommandSettings + { + [CommandOption("--json")] + [Description("Output as JSON")] + public bool Json { get; set; } + } + + public BalancesStatusCommand(ITimeProApiClient api, IConfigService config) + { + _api = api; + _config = config; + } + + protected override async Task ExecuteAsync(CommandContext context, Settings settings, CancellationToken cancellationToken) + { + if (_config.LoadActiveTenantConfig() is null) + { + const string message = "Not logged in. Run 'tp login --tenant ' first."; + if (settings.Json) + OutputHelper.WriteJsonError(message); + else + OutputHelper.WriteError(message); + return 1; + } + + try + { + var status = await _api.GetLeaveBalanceStatusAsync(cancellationToken); + if (status?.LastImportedAt is null) + { + // Nothing imported yet is a valid state, not a failure. + if (settings.Json) + OutputHelper.WriteJson(new { imported = false }); + else + OutputHelper.WriteWarning("No leave balances have been imported yet."); + return 0; + } + + OutputHelper.Render(status, settings.Json, s => + { + var table = new Table().NoBorder().HideHeaders().AddColumn("Key").AddColumn("Value"); + table.AddRow("[bold]Balances as at[/]", s.AsAtDate?.ToString("yyyy-MM-dd") ?? "-"); + table.AddRow("[bold]Last imported[/]", + s.LastImportedAt?.ToLocalTime().ToString("yyyy-MM-dd HH:mm") ?? "never"); + table.AddRow("[bold]Employees[/]", s.EmployeeCount.ToString()); + table.AddRow("[bold]Stale[/]", s.IsStale ? "[yellow]yes[/]" : "no"); + AnsiConsole.Write(table); + + if (s.IsStale) + OutputHelper.WriteWarning("Balances are stale. Re-import the latest Xero export with 'tp leave balances import '."); + }); + + return 0; + } + catch (ApiException ex) + { + var detail = ApiErrorParser.ExtractDetail(ex.ResponseBody); + if (settings.Json) + OutputHelper.WriteJsonError($"API error: {ex.Message}", ex.StatusCode, detail); + OutputHelper.WriteError($"API error ({ex.StatusCode}): {ex.Message}" + + (detail is not null ? $" — {detail}" : "")); + return 1; + } + } +} diff --git a/src/SSW.TimePro.Cli/Features/Leave/LeaveBalanceImportService.cs b/src/SSW.TimePro.Cli/Features/Leave/LeaveBalanceImportService.cs new file mode 100644 index 0000000..358aeae --- /dev/null +++ b/src/SSW.TimePro.Cli/Features/Leave/LeaveBalanceImportService.cs @@ -0,0 +1,185 @@ +using System.Text; +using System.Text.Json; +using SSW.TimePro.Cli.Infrastructure.ApiClient; +using SSW.TimePro.Cli.Infrastructure.Paths; +using SSW.TimePro.Cli.Shared.Models; + +namespace SSW.TimePro.Cli.Features.Leave; + +public sealed class LeaveBalanceImportValidationException(string message) : Exception(message); + +public sealed class LeaveBalanceImportUncertainException(string message, Exception? innerException = null) + : Exception(message, innerException); + +/// +/// Reads a Xero "Leave Balances" CSV export from disk and imports it into TimePro. +/// +/// The file is read here rather than passed around as text so the MCP tool can take a path: +/// tool arguments travel through the model's context, where a large CSV is both expensive and +/// liable to be silently truncated. The CSV itself is parsed server-side and never stored. +/// +public sealed class LeaveBalanceImportService +{ + private const string UncertainImportMessage = + "The import request was sent and may have been applied, but TimePro did not return a complete result. " + + "Run 'tp leave balances status' before retrying."; + + private static readonly Encoding StrictUtf8 = new UTF8Encoding( + encoderShouldEmitUTF8Identifier: false, + throwOnInvalidBytes: true); + + /// + /// Guard against a caller pointing at the wrong file entirely (a database dump, a video). + /// A real Xero balance export for a company of any plausible size is far below this. + /// + internal const int MaxCsvBytes = 5 * 1024 * 1024; + + private readonly ITimeProApiClient _api; + + public LeaveBalanceImportService(ITimeProApiClient api) => _api = api; + + /// + /// Reads and sanity-checks the CSV at , throwing + /// before any network call if it is + /// missing, empty, oversized, or plainly not a CSV. + /// + public string ReadCsv(string csvPath) + { + if (string.IsNullOrWhiteSpace(csvPath)) + throw new LeaveBalanceImportValidationException("A path to the Xero leave balances CSV export is required."); + + string fullPath; + try + { + fullPath = Path.GetFullPath(PathExpander.ExpandHomeDirectory(csvPath)); + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException or PathTooLongException) + { + throw new LeaveBalanceImportValidationException($"'{csvPath}' is not a valid file path."); + } + + if (Directory.Exists(fullPath)) + throw new LeaveBalanceImportValidationException($"'{fullPath}' is a directory. Provide the path to the CSV file itself."); + + if (!File.Exists(fullPath)) + throw new LeaveBalanceImportValidationException($"File not found: {Describe(fullPath, csvPath)}"); + + var length = new FileInfo(fullPath).Length; + if (length == 0) + throw new LeaveBalanceImportValidationException($"File is empty: {fullPath}"); + + if (length > MaxCsvBytes) + { + throw new LeaveBalanceImportValidationException( + $"File is {length / 1024 / 1024}MB, which is larger than the {MaxCsvBytes / 1024 / 1024}MB limit for a leave balances CSV. Check that '{fullPath}' is the Xero export."); + } + + string content; + try + { + content = File.ReadAllText(fullPath, StrictUtf8); + } + catch (DecoderFallbackException) + { + throw new LeaveBalanceImportValidationException( + $"'{fullPath}' is not valid UTF-8. Export the Xero leave balances report as a UTF-8 CSV and try again."); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + throw new LeaveBalanceImportValidationException($"Could not read {fullPath}: {ex.Message}"); + } + + if (string.IsNullOrWhiteSpace(content)) + throw new LeaveBalanceImportValidationException($"File contains no data: {fullPath}"); + + if (content.Contains('\0')) + { + throw new LeaveBalanceImportValidationException( + $"'{fullPath}' looks like a binary file, not a CSV. Export the Xero leave balances report as CSV first."); + } + + return content; + } + + /// + /// Reads the CSV and sends it to TimePro. The server owns parsing and matching, so its + /// rejection messages are surfaced verbatim rather than second-guessed here. + /// + public async Task ImportAsync(string csvPath, CancellationToken ct = default) + { + var csv = ReadCsv(csvPath); + + try + { + var result = await _api.ImportLeaveBalancesAsync(csv, ct); + if (result is null || result.AsAtDate == default) + throw new LeaveBalanceImportUncertainException(UncertainImportMessage); + + // System.Text.Json can replace collection initializers with null when the API sends + // explicit nulls. Normalize here so every CLI/MCP caller receives the same safe shape. + result.Warnings ??= []; + result.UnmatchedEmployees ??= []; + return result; + } + catch (ApiException ex) when (ex.StatusCode == 401) + { + throw new LeaveBalanceImportValidationException( + "TimePro authentication failed. Run 'tp login --tenant ' again."); + } + catch (ApiException ex) when (ex.StatusCode == 403) + { + throw new LeaveBalanceImportValidationException( + "Importing leave balances requires leave admin rights in TimePro, and this account does not have them."); + } + catch (ApiException ex) when (ex.StatusCode == 422) + { + // The endpoint returns the CSV parser's own message as a bare JSON string. + throw new LeaveBalanceImportValidationException( + $"TimePro could not read the CSV: {DescribeUnprocessable(ex.ResponseBody)}"); + } + catch (LeaveBalanceImportUncertainException) + { + throw; + } + catch (Exception ex) + { + throw new LeaveBalanceImportUncertainException(UncertainImportMessage, ex); + } + } + + /// + /// Names the resolved path, and the original alongside it when the two differ. A shell that + /// strips backslashes turns an absolute Windows path into a drive-relative one that resolves + /// somewhere unexpected, which is impossible to spot from the resolved path alone. + /// + private static string Describe(string fullPath, string originalPath) + { + var trimmedOriginal = originalPath.Trim(); + return string.Equals(fullPath, trimmedOriginal, StringComparison.Ordinal) + ? fullPath + : $"{fullPath} (resolved from '{trimmedOriginal}')"; + } + + /// + /// Unwraps the bare JSON string body returned by the endpoint's 422, falling back to the + /// shared problem+json parsing for anything else. + /// + internal static string DescribeUnprocessable(string? responseBody) + { + if (string.IsNullOrWhiteSpace(responseBody)) + return "the file was rejected without a reason."; + + try + { + using var doc = JsonDocument.Parse(responseBody); + if (doc.RootElement.ValueKind == JsonValueKind.String) + return doc.RootElement.GetString() ?? responseBody; + } + catch (JsonException) + { + // Not JSON at all — fall through to the shared parser. + } + + return ApiErrorParser.ExtractDetail(responseBody) ?? responseBody; + } +} diff --git a/src/SSW.TimePro.Cli/Features/Mcp/McpHostCommand.cs b/src/SSW.TimePro.Cli/Features/Mcp/McpHostCommand.cs index 6b03ea0..8687d59 100644 --- a/src/SSW.TimePro.Cli/Features/Mcp/McpHostCommand.cs +++ b/src/SSW.TimePro.Cli/Features/Mcp/McpHostCommand.cs @@ -39,6 +39,7 @@ protected override async Task ExecuteAsync(CommandContext context, Settings builder.Services.AddHttpClient(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); + builder.Services.AddSingleton(); var mcpServer = builder.Services .AddMcpServer() diff --git a/src/SSW.TimePro.Cli/Features/Mcp/Tools/AccountingMcpTools.cs b/src/SSW.TimePro.Cli/Features/Mcp/Tools/AccountingMcpTools.cs index e89e666..c2bfe68 100644 --- a/src/SSW.TimePro.Cli/Features/Mcp/Tools/AccountingMcpTools.cs +++ b/src/SSW.TimePro.Cli/Features/Mcp/Tools/AccountingMcpTools.cs @@ -1,6 +1,7 @@ using System.ComponentModel; using System.Text.Json; using ModelContextProtocol.Server; +using SSW.TimePro.Cli.Features.Leave; using SSW.TimePro.Cli.Infrastructure.ApiClient; using SSW.TimePro.Cli.Infrastructure.Config; using SSW.TimePro.Cli.Shared.Models; @@ -8,7 +9,7 @@ namespace SSW.TimePro.Cli.Features.Mcp.Tools; /// -/// MCP tools for accountant-focused read-only operations. Also exposes cross-domain read +/// MCP tools for accountant-focused operations. Also exposes cross-domain read /// tools useful to accountants (timesheet queries, product lists, rate tables, etc.) that /// aren't already on or . /// @@ -20,6 +21,7 @@ public class AccountingMcpTools { private readonly ITimeProApiClient _api; private readonly IConfigService _config; + private readonly LeaveBalanceImportService _leaveBalanceImportService; private static readonly JsonSerializerOptions JsonOpts = new() { @@ -28,10 +30,14 @@ public class AccountingMcpTools DefaultIgnoreCondition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingNull }; - public AccountingMcpTools(ITimeProApiClient api, IConfigService config) + public AccountingMcpTools( + ITimeProApiClient api, + IConfigService config, + LeaveBalanceImportService leaveBalanceImportService) { _api = api; _config = config; + _leaveBalanceImportService = leaveBalanceImportService; } private bool NotAuthed(out string error) @@ -60,6 +66,46 @@ private static List ResolveEmpIds(string[]? empIds, string[]? employeeId .ToList() ?? []; } + // ─── Leave balance import ─────────────────────────────────────────────── + + [McpServerTool(Destructive = true, Idempotent = true, ReadOnly = false)] + [Description( + "Import leave balances for EVERY employee from a Xero 'Leave Balances' CSV export, replacing what TimePro currently stores. " + + "No dry run and no undo, so confirm with the user first. Pass the path to the CSV file, not its contents. " + + "Always report the returned unmatchedEmployees (rows skipped) and warnings - the import succeeds despite them.")] + public async Task ImportLeaveBalances( + [Description("Path to the Xero 'Leave Balances' CSV export on this machine")] string csvPath, + CancellationToken ct = default) + { + if (NotAuthed(out var err)) return err; + + try + { + var result = await _leaveBalanceImportService.ImportAsync(csvPath, ct); + return JsonSerializer.Serialize(new + { + success = true, + result.AsAtDate, + result.Created, + result.Updated, + result.UnmatchedEmployees, + result.Warnings + }, JsonOpts); + } + catch (LeaveBalanceImportValidationException ex) + { + return JsonSerializer.Serialize(new { error = ex.Message }, JsonOpts); + } + catch (LeaveBalanceImportUncertainException ex) + { + return JsonSerializer.Serialize(new + { + error = ex.Message, + mayHaveBeenApplied = true + }, JsonOpts); + } + } + // ─── Invoices ─────────────────────────────────────────────────────────── [McpServerTool] diff --git a/src/SSW.TimePro.Cli/Features/Mcp/Tools/LeaveMcpTools.cs b/src/SSW.TimePro.Cli/Features/Mcp/Tools/LeaveMcpTools.cs index 2cad8e0..df5e199 100644 --- a/src/SSW.TimePro.Cli/Features/Mcp/Tools/LeaveMcpTools.cs +++ b/src/SSW.TimePro.Cli/Features/Mcp/Tools/LeaveMcpTools.cs @@ -199,6 +199,20 @@ public async Task UpdateLeave( } } + [McpServerTool(ReadOnly = true, Destructive = false)] + [Description("Report when TimePro's leave balances were last imported from Xero, how many employees have a stored balance, and whether the data is stale. Read-only. Check this before importing so you can tell the user whether a re-import is actually needed.")] + public async Task GetLeaveBalanceStatus(CancellationToken ct = default) + { + if (_config.LoadActiveTenantConfig() is null) + return """{"error": "Not logged in. Run 'tp login --tenant ' first."}"""; + + var status = await _api.GetLeaveBalanceStatusAsync(ct); + if (status?.LastImportedAt is null) + return JsonSerializer.Serialize(new { imported = false }, JsonOpts); + + return JsonSerializer.Serialize(status, JsonOpts); + } + private static string? ResolveEmpId(string? empId, string? employeeId) { var requestedEmpId = !string.IsNullOrWhiteSpace(empId) ? empId : employeeId; diff --git a/src/SSW.TimePro.Cli/Features/Mcp/Tools/LookupMcpTools.cs b/src/SSW.TimePro.Cli/Features/Mcp/Tools/LookupMcpTools.cs index a06ef72..0109499 100644 --- a/src/SSW.TimePro.Cli/Features/Mcp/Tools/LookupMcpTools.cs +++ b/src/SSW.TimePro.Cli/Features/Mcp/Tools/LookupMcpTools.cs @@ -3,6 +3,7 @@ using ModelContextProtocol.Server; using SSW.TimePro.Cli.Infrastructure.ApiClient; using SSW.TimePro.Cli.Infrastructure.Config; +using SSW.TimePro.Cli.Infrastructure.Paths; namespace SSW.TimePro.Cli.Features.Mcp.Tools; @@ -100,8 +101,7 @@ public string GetLocationAndMapping( RepoMappingEntry? match = null; if (repoPath is not null) { - var normalized = repoPath.Replace("~", - Environment.GetFolderPath(Environment.SpecialFolder.UserProfile)); + var normalized = PathExpander.ExpandHomeDirectory(repoPath); match = RepoDetector.Detect(normalized, mappings); } diff --git a/src/SSW.TimePro.Cli/Features/RepoMap/RemoveCommand.cs b/src/SSW.TimePro.Cli/Features/RepoMap/RemoveCommand.cs index 3f4a9cb..e1c2119 100644 --- a/src/SSW.TimePro.Cli/Features/RepoMap/RemoveCommand.cs +++ b/src/SSW.TimePro.Cli/Features/RepoMap/RemoveCommand.cs @@ -1,6 +1,7 @@ using System.ComponentModel; using SSW.TimePro.Cli.Infrastructure.Config; using SSW.TimePro.Cli.Infrastructure.Output; +using SSW.TimePro.Cli.Infrastructure.Paths; using Spectre.Console.Cli; namespace SSW.TimePro.Cli.Features.RepoMap; @@ -22,11 +23,11 @@ public class Settings : CommandSettings protected override int Execute(CommandContext context, Settings settings, CancellationToken cancellationToken) { var mappings = _config.LoadRepoMappings(); - var normalizedPath = settings.Path.Replace("~", Environment.GetFolderPath(Environment.SpecialFolder.UserProfile)); + var normalizedPath = PathExpander.ExpandHomeDirectory(settings.Path); var removed = mappings.RemoveAll(m => - m.PathPattern.Equals(settings.Path, StringComparison.OrdinalIgnoreCase) || - m.PathPattern.Equals(normalizedPath, StringComparison.OrdinalIgnoreCase)); + PathExpander.ExpandHomeDirectory(m.PathPattern) + .Equals(normalizedPath, StringComparison.OrdinalIgnoreCase)); if (removed == 0) { diff --git a/src/SSW.TimePro.Cli/Features/RepoMap/SetCommand.cs b/src/SSW.TimePro.Cli/Features/RepoMap/SetCommand.cs index 4f26551..7836b08 100644 --- a/src/SSW.TimePro.Cli/Features/RepoMap/SetCommand.cs +++ b/src/SSW.TimePro.Cli/Features/RepoMap/SetCommand.cs @@ -1,6 +1,7 @@ using System.ComponentModel; using SSW.TimePro.Cli.Infrastructure.Config; using SSW.TimePro.Cli.Infrastructure.Output; +using SSW.TimePro.Cli.Infrastructure.Paths; using Spectre.Console.Cli; namespace SSW.TimePro.Cli.Features.RepoMap; @@ -52,15 +53,12 @@ protected override int Execute(CommandContext context, Settings settings, Cancel } var mappings = _config.LoadRepoMappings(); - var home = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); - string Expand(string p) => p.StartsWith("~") - ? home + p.AsSpan(1).ToString() - : p; - var normalizedInput = Expand(settings.Path); + var normalizedInput = PathExpander.ExpandHomeDirectory(settings.Path); // Match by expanded path so "~/foo" and "/Users/me/foo" dedupe. var existing = mappings.FirstOrDefault(m => - Expand(m.PathPattern).Equals(normalizedInput, StringComparison.OrdinalIgnoreCase)); + PathExpander.ExpandHomeDirectory(m.PathPattern) + .Equals(normalizedInput, StringComparison.OrdinalIgnoreCase)); if (existing is not null) { diff --git a/src/SSW.TimePro.Cli/Features/Skills/SkillModelBuilder.cs b/src/SSW.TimePro.Cli/Features/Skills/SkillModelBuilder.cs index 87cbeb3..4a12d23 100644 --- a/src/SSW.TimePro.Cli/Features/Skills/SkillModelBuilder.cs +++ b/src/SSW.TimePro.Cli/Features/Skills/SkillModelBuilder.cs @@ -16,13 +16,13 @@ public static class SkillModelBuilder public const string DeveloperTimesheetDiagnosticsName = "timepro-dev-timesheet-diagnostics"; public const string DeveloperFinanceDiagnosticsName = "timepro-dev-finance-diagnostics"; public const string EnvironmentCompareName = "timepro-env-compare"; - public const int CurrentSkillVersion = 1; + public const int CurrentSkillVersion = 2; private const string TimesheetsDescription = "Manage SSW TimePro timesheets with the tp CLI — view/accept/create entries, repo mappings, bookings, leave, and daily scrum. Use when entering, fixing, or reviewing timesheets."; private const string AccountingDescription = - "Explore SSW TimePro financial data via the tp CLI (read-only) — invoices with line items, billed timesheets, credit notes, receipts, sale products, client rates, aged debtors, unbilled time, recurring invoices, prepaid drawdowns and client billable-work threshold reports. Use for accountant-style questions. For raw HTTP/curl access (when tp isn't installed), use the timepro-accounting skill instead."; + "Explore SSW TimePro financial data via the tp CLI — read-only invoices, receipts, rates and reconciliation reports, plus explicitly approved Xero leave-balance imports. Use for accountant-style questions. For raw HTTP/curl access (when tp isn't installed), use the timepro-accounting skill instead."; private const string TenantSetupDescription = "Set up and switch TimePro tenant profiles with the tp CLI, including switching the active session to ssw-staging and using process-local --tenant/--env overrides without changing the active tenant."; diff --git a/src/SSW.TimePro.Cli/Features/Skills/Templates/timepro-accounting-cli.md b/src/SSW.TimePro.Cli/Features/Skills/Templates/timepro-accounting-cli.md index dc24ed8..bd0789e 100644 --- a/src/SSW.TimePro.Cli/Features/Skills/Templates/timepro-accounting-cli.md +++ b/src/SSW.TimePro.Cli/Features/Skills/Templates/timepro-accounting-cli.md @@ -6,7 +6,7 @@ Do not install this file directly as an agent skill. # TimePro Accounting (CLI) -Accountant-facing read-only access to SSW TimePro via the `tp` CLI. Pipe `--json` output into `jq` or Python to calculate totals, compare against Xero, or audit historical data. +Accountant-facing access to SSW TimePro via the `tp` CLI. The financial exploration workflows are read-only; the Xero leave-balance import is the one explicitly documented write and requires direct user approval. Pipe `--json` output into `jq` or Python to calculate totals, compare against Xero, or audit historical data. ## Setup This skill reuses the tenant config already configured for `tp`. If `tp login` has been run, nothing else is needed. Otherwise run `tp login --tenant ` first. @@ -72,10 +72,36 @@ tp prepaid status --output /tmp/prepaid.pdf # Cross-employee/client/project timesheet query tp query --from 2026-03-01 --to 2026-03-31 --json tp query --from 2026-03-01 --to 2026-03-31 --client --json + +# Xero leave balances +tp leave balances status --json +tp leave balances import ./LeaveBalances.csv --yes --json ``` ## Common workflows +### Import Xero leave balances (destructive) +This import replaces stored leave balances company-wide, has no dry-run, and cannot be +undone through TimePro. Do not run it from an inferred request. Obtain explicit user +approval for the exact file and tenant immediately before importing. + +1. Run `tp info --json` and report the selected tenant. +2. Run `tp leave balances status --json` and report the current as-at/import dates, + employee count, and staleness. +3. Confirm the local CSV path with the user. Do not paste the CSV into an agent prompt. +4. After approval, run: + +```bash +tp leave balances import ./LeaveBalances.csv --yes --json +``` + +Always report `created`, `updated`, `unmatchedEmployees`, and `warnings`. A successful +request may still skip unmatched rows. + +For MCP, `get_leave_balance_status` is on the default TimePro surface, while the destructive +`import_leave_balances` tool is available only after `tp feature accounting enable`. The +same explicit approval and result-reporting rules apply. + ### Drill into an invoice ```bash INV=142 @@ -220,8 +246,9 @@ Prefer the accounting guide before manually stitching primitives: - `guides/accounting/invoice-evidence-pack.md` assembles an invoice evidence pack from header, lines, allocated/write-off timesheets, receipts, and credit notes. - `guides/accounting/client-accounting-position.md` assembles client-level invoice, debt, unbilled, credit note, rate, and external comparison evidence. -If using `tp mcp` with accounting enabled, MCP exposes primitive read-only tools. -Use skills or guide-backed markdown to compose multi-step diagnostics locally. +If using `tp mcp` with accounting enabled, MCP exposes primitive read-only tools plus the +explicitly approved `import_leave_balances` write. Use skills or guide-backed markdown to +compose multi-step diagnostics locally. Enable this MCP surface once with: diff --git a/src/SSW.TimePro.Cli/Features/Skills/Templates/timepro-timesheets.md b/src/SSW.TimePro.Cli/Features/Skills/Templates/timepro-timesheets.md index 917803d..0e48e7b 100644 --- a/src/SSW.TimePro.Cli/Features/Skills/Templates/timepro-timesheets.md +++ b/src/SSW.TimePro.Cli/Features/Skills/Templates/timepro-timesheets.md @@ -60,6 +60,7 @@ tp bk list --week --json # Leave tp leave list --filter UPCOMING --json +tp leave balances status --json tp leave create --start 2026-03-30 --end 2026-03-30 --type 1 \ --note "Reason" --approved-by "approver@northwind.example" \ --cc "notify1@northwind.example,notify2@northwind.example" --yes @@ -70,6 +71,12 @@ tp leave update --note "Updated reason" --dry-run --json tp leave cancel --reason "Plans changed" --yes ``` +### Check EasyLeave balance freshness +Use `tp leave balances status --json` for read-only questions about when company-wide +EasyLeave balances were last imported and whether they are stale. The equivalent default +MCP tool is `get_leave_balance_status`. Do not import balances from this skill; the +destructive `import_leave_balances` workflow is accounting-gated. + ## Workflow: Enter Timesheets for the Week 1. Pick the project: `tp project recent --json`. 2. Verify repo mapping: `tp map detect`. If category is missing, run the repo mapping setup workflow below. diff --git a/src/SSW.TimePro.Cli/Infrastructure/ApiClient/TimeProApiClient.cs b/src/SSW.TimePro.Cli/Infrastructure/ApiClient/TimeProApiClient.cs index cd487a1..9a0831a 100644 --- a/src/SSW.TimePro.Cli/Infrastructure/ApiClient/TimeProApiClient.cs +++ b/src/SSW.TimePro.Cli/Infrastructure/ApiClient/TimeProApiClient.cs @@ -1,4 +1,5 @@ using System.Net.Http.Json; +using System.Text; using System.Text.Json; using SSW.TimePro.Cli.Infrastructure.Config; using SSW.TimePro.Cli.Shared.Models; @@ -47,6 +48,11 @@ public interface ITimeProApiClient Task CreateLeaveAsync(CreateLeaveRequest request, CancellationToken ct = default); Task UpdateLeaveAsync(UpdateLeaveRequest request, CancellationToken ct = default); Task CancelLeaveAsync(string leaveId, CancelLeaveRequest request, CancellationToken ct = default); + + // Leave balances (Xero CSV sync). Import is leave-admin only server-side and replaces + // stored balances; status is a cheap read used to decide whether a re-import is due. + Task GetLeaveBalanceStatusAsync(CancellationToken ct = default); + Task ImportLeaveBalancesAsync(string csvContent, CancellationToken ct = default); Task ExportTimesheetsCsvAsync(DateOnly? startDate, DateOnly? endDate, CancellationToken ct = default); Task> GetBlogsAsync(bool includeFormerEmployees = false, CancellationToken ct = default); Task> GetProjectsSummaryAsync(string employeeId, DateOnly startDate, DateOnly endDate, CancellationToken ct = default); @@ -384,6 +390,20 @@ public async Task CancelLeaveAsync(string leaveId, CancelLeaveRequest request, C await PutAsync($"/api/leave/{Uri.EscapeDataString(leaveId)}/cancel", request, ct); } + public async Task GetLeaveBalanceStatusAsync(CancellationToken ct = default) + { + return await GetAsync("/api/leave/balances/status", ct); + } + + public async Task ImportLeaveBalancesAsync( + string csvContent, CancellationToken ct = default) + { + // The endpoint reads the request body as the raw Xero CSV export, so this must not go + // through the JSON helpers — JsonContent would send an escaped string literal. + return await PostRawAsync( + "/api/leave/balances/import", csvContent, "text/csv", ct); + } + // ───────────────────────── Export ───────────────────────── public async Task ExportTimesheetsCsvAsync( @@ -802,6 +822,28 @@ private async Task GetBytesAsync(string relativeUrl, CancellationToken c return System.Text.Json.JsonSerializer.Deserialize(content, ReadJsonOptions); } + /// + /// POSTs a body verbatim under an explicit content type, for endpoints that read the raw + /// request stream rather than a JSON payload (currently the Xero leave balance CSV import). + /// + private async Task PostRawAsync(string relativeUrl, string content, string contentType, CancellationToken ct) + { + using var request = new HttpRequestMessage(HttpMethod.Post, relativeUrl) + { + Content = new StringContent(content, Encoding.UTF8, contentType) + }; + ConfigureRequest(request); + + using var response = await _http.SendAsync(request, ct); + await EnsureSuccessAsync(response, ct); + + var body = await response.Content.ReadAsStringAsync(ct); + if (string.IsNullOrWhiteSpace(body)) + return default; + + return JsonSerializer.Deserialize(body, ReadJsonOptions); + } + private async Task PutAsync(string relativeUrl, object body, CancellationToken ct) { using var request = new HttpRequestMessage(HttpMethod.Put, relativeUrl) diff --git a/src/SSW.TimePro.Cli/Infrastructure/Config/FeatureCatalog.cs b/src/SSW.TimePro.Cli/Infrastructure/Config/FeatureCatalog.cs index 331c263..8c5b9ea 100644 --- a/src/SSW.TimePro.Cli/Infrastructure/Config/FeatureCatalog.cs +++ b/src/SSW.TimePro.Cli/Infrastructure/Config/FeatureCatalog.cs @@ -17,7 +17,7 @@ public static class FeatureCatalog new( Accounting, "Accounting", - "Accounting skills, guide topics, and read-only accounting MCP tools.", + "Accounting skills, guide topics, and accounting MCP tools including guarded leave-balance import.", Version: 1, Aliases: ["accounts", "accountant"]), new( diff --git a/src/SSW.TimePro.Cli/Infrastructure/Config/RepoDetector.cs b/src/SSW.TimePro.Cli/Infrastructure/Config/RepoDetector.cs index 7f540f6..f713821 100644 --- a/src/SSW.TimePro.Cli/Infrastructure/Config/RepoDetector.cs +++ b/src/SSW.TimePro.Cli/Infrastructure/Config/RepoDetector.cs @@ -1,4 +1,5 @@ using System.Diagnostics; +using SSW.TimePro.Cli.Infrastructure.Paths; namespace SSW.TimePro.Cli.Infrastructure.Config; @@ -17,8 +18,6 @@ public static class RepoDetector /// public static RepoMappingEntry? Detect(string directory, List mappings) { - var home = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); - // Collect candidate paths: cwd + main worktree var candidatePaths = new List { directory }; var mainRepoPath = ResolveMainRepoPath(directory); @@ -34,7 +33,7 @@ public static class RepoDetector foreach (var m in mappings) { - var score = ScoreMapping(m, candidatePaths, remoteUrl, home); + var score = ScoreMapping(m, candidatePaths, remoteUrl); if (score > bestScore) { bestScore = score; @@ -46,14 +45,14 @@ public static class RepoDetector } private static int ScoreMapping( - RepoMappingEntry mapping, List paths, string? remoteUrl, string home) + RepoMappingEntry mapping, List paths, string? remoteUrl) { int bestScore = -1; // Check path-based matching if (!string.IsNullOrEmpty(mapping.PathPattern)) { - var pattern = mapping.PathPattern.Replace("~", home); + var pattern = PathExpander.ExpandHomeDirectory(mapping.PathPattern); foreach (var path in paths) { diff --git a/src/SSW.TimePro.Cli/Infrastructure/Paths/PathExpander.cs b/src/SSW.TimePro.Cli/Infrastructure/Paths/PathExpander.cs new file mode 100644 index 0000000..9fef420 --- /dev/null +++ b/src/SSW.TimePro.Cli/Infrastructure/Paths/PathExpander.cs @@ -0,0 +1,34 @@ +namespace SSW.TimePro.Cli.Infrastructure.Paths; + +/// +/// Expands portable path shorthand before paths are resolved or compared. +/// +public static class PathExpander +{ + /// + /// Expands a leading current-user home segment (~, ~/, or ~\). + /// Embedded tildes and named-user forms such as ~someone are left unchanged. + /// + public static string ExpandHomeDirectory(string path) + { + var trimmedPath = path.Trim(); + if (trimmedPath != "~" + && !trimmedPath.StartsWith("~/", StringComparison.Ordinal) + && !trimmedPath.StartsWith(@"~\", StringComparison.Ordinal)) + { + return trimmedPath; + } + + var userProfile = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); + if (string.IsNullOrWhiteSpace(userProfile)) + return trimmedPath; + + if (trimmedPath.Length == 1) + return userProfile; + + var relativePath = trimmedPath[2..] + .Replace('/', Path.DirectorySeparatorChar) + .Replace('\\', Path.DirectorySeparatorChar); + return Path.Combine(userProfile, relativePath); + } +} diff --git a/src/SSW.TimePro.Cli/Program.cs b/src/SSW.TimePro.Cli/Program.cs index f6ea7b4..6c7834f 100644 --- a/src/SSW.TimePro.Cli/Program.cs +++ b/src/SSW.TimePro.Cli/Program.cs @@ -29,6 +29,8 @@ using LeaveUpdate = SSW.TimePro.Cli.Features.Leave.UpdateCommand; using LeaveCancel = SSW.TimePro.Cli.Features.Leave.CancelCommand; using LeaveBalance = SSW.TimePro.Cli.Features.Leave.BalanceCommand; +using LeaveBalancesStatus = SSW.TimePro.Cli.Features.Leave.BalancesStatusCommand; +using LeaveBalancesImport = SSW.TimePro.Cli.Features.Leave.BalancesImportCommand; using InvList = SSW.TimePro.Cli.Features.Invoices.ListCommand; using InvGet = SSW.TimePro.Cli.Features.Invoices.GetCommand; using InvLines = SSW.TimePro.Cli.Features.Invoices.LinesCommand; @@ -113,6 +115,7 @@ services.AddHttpClient(); services.AddSingleton(); services.AddSingleton(); +services.AddSingleton(); var registrar = new TypeRegistrar(services); @@ -215,6 +218,17 @@ void RegisterLeaveCommands(IConfigurator branch) .WithDescription("Cancel a leave request"); branch.AddCommand("balance") .WithDescription("Show leave stats (days since last leave, leave taken in last 12 months)"); + + // "balances" (plural) is the company-wide Xero balance sync, distinct from the + // per-employee "balance" stats command above. + branch.AddBranch("balances", balances => + { + balances.SetDescription("Manage imported leave balances (Xero sync)"); + balances.AddCommand("status") + .WithDescription("Show when leave balances were last imported and whether they are stale"); + balances.AddCommand("import") + .WithDescription("Import leave balances from a Xero CSV export"); + }); } config.AddBranch("leave", lv => diff --git a/src/SSW.TimePro.Cli/Shared/Models/LeaveModels.cs b/src/SSW.TimePro.Cli/Shared/Models/LeaveModels.cs index 8c05809..4916d68 100644 --- a/src/SSW.TimePro.Cli/Shared/Models/LeaveModels.cs +++ b/src/SSW.TimePro.Cli/Shared/Models/LeaveModels.cs @@ -137,3 +137,40 @@ public class CancelLeaveRequest public string LeaveId { get; set; } = string.Empty; public string CancellationReason { get; set; } = string.Empty; } + +/// +/// Current state of the leave balance sync (API "GET /api/leave/balances/status"). +/// Null dates mean nothing has been imported yet. +/// +public class LeaveBalanceStatus +{ + /// Date the imported balances are accurate as at, taken from the Xero export. + public DateOnly? AsAtDate { get; set; } + + public DateTimeOffset? LastImportedAt { get; set; } + + /// Number of employees with a stored balance. + public int EmployeeCount { get; set; } + + /// Server's own judgement that the stored balances are too old to rely on. + public bool IsStale { get; set; } +} + +/// +/// Outcome of a leave balance import (API "POST /api/leave/balances/import"). +/// +public class ImportLeaveBalancesResult +{ + public DateOnly AsAtDate { get; set; } + public int Created { get; set; } + public int Updated { get; set; } + + /// + /// Xero rows whose employee name matched no TimePro employee, or matched more than one. + /// These rows were skipped, not imported. + /// + public List UnmatchedEmployees { get; set; } = []; + + /// Rows that were imported but look wrong (e.g. an implausibly large balance). + public List Warnings { get; set; } = []; +} diff --git a/tests/SSW.TimePro.Cli.Integration/Features/AccountingApiTests.cs b/tests/SSW.TimePro.Cli.Integration/Features/AccountingApiTests.cs index 7470398..3edcf52 100644 --- a/tests/SSW.TimePro.Cli.Integration/Features/AccountingApiTests.cs +++ b/tests/SSW.TimePro.Cli.Integration/Features/AccountingApiTests.cs @@ -118,7 +118,9 @@ public async Task GetInvoiceTimesheets_WriteOff_HitsWriteOffPath() var rows = await ApiClient.GetInvoiceTimesheetsAsync(142, "writeoff", CancellationToken.None); rows.Should().BeEmpty(); - WireMock.LogEntries.First().RequestMessage.AbsolutePath.Should().EndWith("/WriteOff"); + var requestMessage = WireMock.LogEntries.First().RequestMessage; + requestMessage.Should().NotBeNull(); + requestMessage!.AbsolutePath.Should().EndWith("/WriteOff"); } [Fact] diff --git a/tests/SSW.TimePro.Cli.Integration/Features/LeaveBalancesApiTests.cs b/tests/SSW.TimePro.Cli.Integration/Features/LeaveBalancesApiTests.cs new file mode 100644 index 0000000..b24c52d --- /dev/null +++ b/tests/SSW.TimePro.Cli.Integration/Features/LeaveBalancesApiTests.cs @@ -0,0 +1,148 @@ +using FluentAssertions; +using WireMock.RequestBuilders; +using WireMock.ResponseBuilders; +using Xunit; +using SSW.TimePro.Cli.Infrastructure.ApiClient; + +namespace SSW.TimePro.Cli.Integration.Features; + +public class LeaveBalancesApiTests : TestBase +{ + private const string Csv = "Employee,Leave Type,Units\nJane Doe,Annual Leave,76.00\nJohn Smith,Annual Leave,12.50\n"; + + [Fact] + public async Task ImportLeaveBalances_SendsRawCsvBodyWithCsvContentType() + { + WireMock.Given( + Request.Create() + .WithPath("/api/leave/balances/import") + .UsingPost() + ).RespondWith( + Response.Create() + .WithStatusCode(200) + .WithHeader("Content-Type", "application/json") + .WithBody(""" + { + "asAtDate": "2026-08-01", + "created": 1, + "updated": 2, + "unmatchedEmployees": ["Nobody Here"], + "warnings": ["Jane Doe has an unusually large balance (2500.00 hours) - please verify."] + } + """) + ); + + var result = await ApiClient.ImportLeaveBalancesAsync(Csv, CancellationToken.None); + + result.Should().NotBeNull(); + result!.AsAtDate.Should().Be(new DateOnly(2026, 8, 1)); + result.Created.Should().Be(1); + result.Updated.Should().Be(2); + result.UnmatchedEmployees.Should().Equal("Nobody Here"); + result.Warnings.Should().ContainSingle(); + + // The endpoint reads the request stream as CSV. If this ever regresses to the JSON + // helpers the body becomes an escaped string literal and the server-side parser fails. + var request = WireMock.LogEntries.Should().ContainSingle().Subject.RequestMessage; + request.Should().NotBeNull(); + request!.Body.Should().Be(Csv); + request.Headers.Should().NotBeNull(); + request.Headers!["Content-Type"].ToString().Should().Contain("text/csv"); + } + + [Fact] + public async Task ImportLeaveBalances_WhenForbidden_ThrowsApiExceptionWith403() + { + WireMock.Given( + Request.Create() + .WithPath("/api/leave/balances/import") + .UsingPost() + ).RespondWith( + Response.Create().WithStatusCode(403) + ); + + var act = () => ApiClient.ImportLeaveBalancesAsync(Csv, CancellationToken.None); + + (await act.Should().ThrowAsync()).Which.StatusCode.Should().Be(403); + } + + [Fact] + public async Task ImportLeaveBalances_WhenCsvRejected_ThrowsApiExceptionCarryingServerMessage() + { + WireMock.Given( + Request.Create() + .WithPath("/api/leave/balances/import") + .UsingPost() + ).RespondWith( + Response.Create() + .WithStatusCode(422) + .WithHeader("Content-Type", "application/json") + .WithBody("\"Missing required column 'Units'.\"") + ); + + var act = () => ApiClient.ImportLeaveBalancesAsync(Csv, CancellationToken.None); + + var exception = (await act.Should().ThrowAsync()).Which; + exception.StatusCode.Should().Be(422); + exception.ResponseBody.Should().Contain("Missing required column"); + } + + [Fact] + public async Task GetLeaveBalanceStatus_WithStoredBalances_ReturnsStatus() + { + WireMock.Given( + Request.Create() + .WithPath("/api/leave/balances/status") + .UsingGet() + ).RespondWith( + Response.Create() + .WithStatusCode(200) + .WithHeader("Content-Type", "application/json") + .WithBody(""" + { + "asAtDate": "2026-08-01", + "lastImportedAt": "2026-08-02T09:00:00+00:00", + "employeeCount": 42, + "isStale": true + } + """) + ); + + var status = await ApiClient.GetLeaveBalanceStatusAsync(CancellationToken.None); + + status.Should().NotBeNull(); + status!.AsAtDate.Should().Be(new DateOnly(2026, 8, 1)); + status.LastImportedAt.Should().Be(new DateTimeOffset(2026, 8, 2, 9, 0, 0, TimeSpan.Zero)); + status.EmployeeCount.Should().Be(42); + status.IsStale.Should().BeTrue(); + } + + [Fact] + public async Task GetLeaveBalanceStatus_WhenNothingImported_ReturnsNullDates() + { + WireMock.Given( + Request.Create() + .WithPath("/api/leave/balances/status") + .UsingGet() + ).RespondWith( + Response.Create() + .WithStatusCode(200) + .WithHeader("Content-Type", "application/json") + .WithBody(""" + { + "asAtDate": null, + "lastImportedAt": null, + "employeeCount": 0, + "isStale": false + } + """) + ); + + var status = await ApiClient.GetLeaveBalanceStatusAsync(CancellationToken.None); + + status.Should().NotBeNull(); + status!.AsAtDate.Should().BeNull(); + status.LastImportedAt.Should().BeNull(); + status.EmployeeCount.Should().Be(0); + } +} diff --git a/tests/SSW.TimePro.Cli.Integration/Features/LeaveTests.cs b/tests/SSW.TimePro.Cli.Integration/Features/LeaveTests.cs index 428572e..d5f7bbf 100644 --- a/tests/SSW.TimePro.Cli.Integration/Features/LeaveTests.cs +++ b/tests/SSW.TimePro.Cli.Integration/Features/LeaveTests.cs @@ -197,17 +197,20 @@ public async Task CreateLeave_WithValidRequest_SendsCorrectPayload() entries.Should().HaveCount(1); var logEntry = entries.First(); - logEntry.RequestMessage.Method.Should().Be("POST"); - logEntry.RequestMessage.Path.Should().Be("/api/leave/"); + var requestMessage = logEntry.RequestMessage; + requestMessage.Should().NotBeNull(); + requestMessage!.Method.Should().Be("POST"); + requestMessage.Path.Should().Be("/api/leave/"); // Verify headers - logEntry.RequestMessage.Headers.Should().ContainKey("x-timepro-tenant-id"); - logEntry.RequestMessage.Headers!["x-timepro-tenant-id"].Should().Contain("test"); - logEntry.RequestMessage.Headers.Should().ContainKey("x-timepro-api-key"); - logEntry.RequestMessage.Headers!["x-timepro-api-key"].Should().Contain("test-api-key"); + requestMessage.Headers.Should().NotBeNull(); + requestMessage.Headers!.Should().ContainKey("x-timepro-tenant-id"); + requestMessage.Headers["x-timepro-tenant-id"].Should().Contain("test"); + requestMessage.Headers.Should().ContainKey("x-timepro-api-key"); + requestMessage.Headers["x-timepro-api-key"].Should().Contain("test-api-key"); // Verify body contains required fields - var body = logEntry.RequestMessage.Body; + var body = requestMessage.Body; body.Should().NotBeNullOrEmpty(); var doc = JsonDocument.Parse(body!); var root = doc.RootElement; @@ -250,7 +253,9 @@ public async Task CreateLeave_WithMinimalRequest_OmitsNullFields() await ApiClient.CreateLeaveAsync(request, CancellationToken.None); // Assert - var body = WireMock.LogEntries.First().RequestMessage.Body; + var requestMessage = WireMock.LogEntries.First().RequestMessage; + requestMessage.Should().NotBeNull(); + var body = requestMessage!.Body; var doc = JsonDocument.Parse(body!); var root = doc.RootElement; @@ -410,11 +415,13 @@ public async Task CancelLeave_WithValidRequest_SendsCorrectPayload() entries.Should().HaveCount(1); var logEntry = entries.First(); - logEntry.RequestMessage.Method.Should().Be("PUT"); - logEntry.RequestMessage.Path.Should().Be($"/api/leave/{leaveId}/cancel"); + var requestMessage = logEntry.RequestMessage; + requestMessage.Should().NotBeNull(); + requestMessage!.Method.Should().Be("PUT"); + requestMessage.Path.Should().Be($"/api/leave/{leaveId}/cancel"); // Verify body contains LeaveId and CancellationReason - var body = logEntry.RequestMessage.Body; + var body = requestMessage.Body; body.Should().NotBeNullOrEmpty(); var doc = JsonDocument.Parse(body!); var root = doc.RootElement; @@ -500,9 +507,11 @@ public async Task CancelLeave_SetsAuthHeaders() // Assert var req = WireMock.LogEntries.First().RequestMessage; - req.Headers.Should().ContainKey("x-timepro-tenant-id"); - req.Headers!["x-timepro-tenant-id"].Should().Contain("test"); + req.Should().NotBeNull(); + req!.Headers.Should().NotBeNull(); + req.Headers!.Should().ContainKey("x-timepro-tenant-id"); + req.Headers["x-timepro-tenant-id"].Should().Contain("test"); req.Headers.Should().ContainKey("x-timepro-api-key"); - req.Headers!["x-timepro-api-key"].Should().Contain("test-api-key"); + req.Headers["x-timepro-api-key"].Should().Contain("test-api-key"); } } diff --git a/tests/SSW.TimePro.Cli.Integration/Features/TimesheetGetTests.cs b/tests/SSW.TimePro.Cli.Integration/Features/TimesheetGetTests.cs index f5acf94..7ce9203 100644 --- a/tests/SSW.TimePro.Cli.Integration/Features/TimesheetGetTests.cs +++ b/tests/SSW.TimePro.Cli.Integration/Features/TimesheetGetTests.cs @@ -78,10 +78,12 @@ public async Task GetTimesheets_SetsAuthHeaders() var entries = WireMock.LogEntries; entries.Should().HaveCount(1); var req = entries.First().RequestMessage; - req.Headers.Should().ContainKey("x-timepro-tenant-id"); - req.Headers!["x-timepro-tenant-id"].Should().Contain("test"); + req.Should().NotBeNull(); + req!.Headers.Should().NotBeNull(); + req.Headers!.Should().ContainKey("x-timepro-tenant-id"); + req.Headers["x-timepro-tenant-id"].Should().Contain("test"); req.Headers.Should().ContainKey("x-timepro-api-key"); - req.Headers!["x-timepro-api-key"].Should().Contain("test-api-key"); + req.Headers["x-timepro-api-key"].Should().Contain("test-api-key"); } [Fact] diff --git a/tests/SSW.TimePro.Cli.Tests/Features/Leave/BalancesCommandsTests.cs b/tests/SSW.TimePro.Cli.Tests/Features/Leave/BalancesCommandsTests.cs new file mode 100644 index 0000000..fc97572 --- /dev/null +++ b/tests/SSW.TimePro.Cli.Tests/Features/Leave/BalancesCommandsTests.cs @@ -0,0 +1,194 @@ +using System.Text; +using FluentAssertions; +using Microsoft.Extensions.DependencyInjection; +using NSubstitute; +using SSW.TimePro.Cli.Features.Leave; +using SSW.TimePro.Cli.Infrastructure.ApiClient; +using SSW.TimePro.Cli.Infrastructure.Config; +using SSW.TimePro.Cli.Infrastructure.DependencyInjection; +using SSW.TimePro.Cli.Shared.Models; +using Spectre.Console.Cli; +using Xunit; + +namespace SSW.TimePro.Cli.Tests.Features.Leave; + +public class BalancesCommandsTests : IDisposable +{ + private const string ValidCsv = "Employee,Leave Type,Units\nJane Doe,Annual Leave,76.00\n"; + + private readonly string _tempDir = Directory.CreateDirectory( + Path.Combine(Path.GetTempPath(), "tp-balance-cmd-tests", Guid.NewGuid().ToString("N"))).FullName; + + [Fact] + public async Task Import_WithValidCsvAndJson_CallsApiWithoutPrompting() + { + var api = Substitute.For(); + string? sent = null; + api.ImportLeaveBalancesAsync(Arg.Do(v => sent = v), Arg.Any()) + .Returns(new ImportLeaveBalancesResult { AsAtDate = new DateOnly(2026, 8, 1), Created = 3, Updated = 4 }); + var app = CreateApp(api); + var path = WriteFile("balances.csv", ValidCsv); + + // --json implies non-interactive, so no confirmation should be required. + var exitCode = await app.RunAsync(["import", path, "--json"], TestContext.Current.CancellationToken); + + exitCode.Should().Be(0); + sent.Should().Be(ValidCsv); + } + + [Fact] + public async Task Import_WhenApiReturnsNullCollections_StillSucceeds() + { + var api = Substitute.For(); + api.ImportLeaveBalancesAsync(Arg.Any(), Arg.Any()) + .Returns(new ImportLeaveBalancesResult + { + AsAtDate = new DateOnly(2026, 8, 1), + UnmatchedEmployees = null!, + Warnings = null! + }); + var app = CreateApp(api); + var path = WriteFile("balances.csv", ValidCsv); + + var exitCode = await app.RunAsync(["import", path, "--json"], TestContext.Current.CancellationToken); + + exitCode.Should().Be(0); + } + + [Fact] + public async Task Import_WhenSubmissionOutcomeIsUncertain_ReturnsFailureExitCode() + { + var api = Substitute.For(); + api.ImportLeaveBalancesAsync(Arg.Any(), Arg.Any()) + .Returns>(_ => throw new HttpRequestException("Connection dropped")); + var app = CreateApp(api); + var path = WriteFile("balances.csv", ValidCsv); + + var exitCode = await app.RunAsync(["import", path, "--json"], TestContext.Current.CancellationToken); + + exitCode.Should().Be(1); + } + + [Fact] + public async Task Import_WhenFileMissing_FailsBeforeCallingApi() + { + var api = Substitute.For(); + var app = CreateApp(api); + + var exitCode = await app.RunAsync( + ["import", Path.Combine(_tempDir, "nope.csv"), "--json"], + TestContext.Current.CancellationToken); + + exitCode.Should().Be(1); + api.ShouldNotHaveReceived(nameof(ITimeProApiClient.ImportLeaveBalancesAsync)); + } + + [Fact] + public async Task Import_WhenNotLoggedIn_FailsBeforeReadingFile() + { + var api = Substitute.For(); + var app = CreateApp(api, loggedIn: false); + var path = WriteFile("balances.csv", ValidCsv); + + var exitCode = await app.RunAsync(["import", path, "--json"], TestContext.Current.CancellationToken); + + exitCode.Should().Be(1); + api.ShouldNotHaveReceived(nameof(ITimeProApiClient.ImportLeaveBalancesAsync)); + } + + [Fact] + public async Task Import_WhenApiRejectsCsv_ReturnsFailureExitCode() + { + var api = Substitute.For(); + api.ImportLeaveBalancesAsync(Arg.Any(), Arg.Any()) + .Returns>(_ => throw new ApiException( + 422, "Unprocessable Entity", "\"Missing required column 'Units'.\"")); + var app = CreateApp(api); + var path = WriteFile("balances.csv", ValidCsv); + + var exitCode = await app.RunAsync(["import", path, "--json"], TestContext.Current.CancellationToken); + + exitCode.Should().Be(1); + } + + [Fact] + public async Task Status_WhenNothingImported_SucceedsWithoutError() + { + var api = Substitute.For(); + api.GetLeaveBalanceStatusAsync(Arg.Any()) + .Returns(new LeaveBalanceStatus + { + AsAtDate = null, + LastImportedAt = null, + EmployeeCount = 0, + IsStale = false + }); + var app = CreateApp(api); + + var exitCode = await app.RunAsync(["status", "--json"], TestContext.Current.CancellationToken); + + // "Never imported" is a valid state to report, not a failure. + exitCode.Should().Be(0); + } + + [Fact] + public async Task Status_WhenBalancesStored_Succeeds() + { + var api = Substitute.For(); + api.GetLeaveBalanceStatusAsync(Arg.Any()) + .Returns(new LeaveBalanceStatus + { + AsAtDate = new DateOnly(2026, 8, 1), + LastImportedAt = new DateTimeOffset(2026, 8, 2, 9, 0, 0, TimeSpan.Zero), + EmployeeCount = 42, + IsStale = true + }); + var app = CreateApp(api); + + var exitCode = await app.RunAsync(["status", "--json"], TestContext.Current.CancellationToken); + + exitCode.Should().Be(0); + await api.Received(1).GetLeaveBalanceStatusAsync(Arg.Any()); + } + + private string WriteFile(string name, string content) + { + var path = Path.Combine(_tempDir, name); + File.WriteAllText(path, content, new UTF8Encoding(false)); + return path; + } + + private static CommandApp CreateApp(ITimeProApiClient api, bool loggedIn = true) + { + var config = Substitute.For(); + config.LoadActiveTenantConfig().Returns(loggedIn + ? new TenantConfig + { + TenantId = "test", + ApiUrl = "https://timepro.example", + ApiKey = "test-api-key", + EmployeeId = "TST" + } + : null); + + var services = new ServiceCollection(); + services.AddSingleton(api); + services.AddSingleton(config); + services.AddSingleton(); + + var app = new CommandApp(new TypeRegistrar(services)); + app.Configure(configurator => + { + configurator.AddCommand("import"); + configurator.AddCommand("status"); + }); + return app; + } + + public void Dispose() + { + if (Directory.Exists(_tempDir)) + Directory.Delete(_tempDir, recursive: true); + GC.SuppressFinalize(this); + } +} diff --git a/tests/SSW.TimePro.Cli.Tests/Features/Leave/LeaveBalanceImportServiceTests.cs b/tests/SSW.TimePro.Cli.Tests/Features/Leave/LeaveBalanceImportServiceTests.cs new file mode 100644 index 0000000..6af8d3b --- /dev/null +++ b/tests/SSW.TimePro.Cli.Tests/Features/Leave/LeaveBalanceImportServiceTests.cs @@ -0,0 +1,269 @@ +using System.Text; +using FluentAssertions; +using NSubstitute; +using SSW.TimePro.Cli.Features.Leave; +using SSW.TimePro.Cli.Infrastructure.ApiClient; +using SSW.TimePro.Cli.Shared.Models; +using Xunit; + +namespace SSW.TimePro.Cli.Tests.Features.Leave; + +public class LeaveBalanceImportServiceTests : IDisposable +{ + private const string ValidCsv = "Employee,Leave Type,Units\nJane Doe,Annual Leave,76.00\n"; + + private readonly string _tempDir = Directory.CreateDirectory( + Path.Combine(Path.GetTempPath(), "tp-balance-import-tests", Guid.NewGuid().ToString("N"))).FullName; + + [Fact] + public async Task Import_WithValidCsv_SendsFileContentsVerbatim() + { + var api = Substitute.For(); + string? sent = null; + api.ImportLeaveBalancesAsync(Arg.Do(value => sent = value), Arg.Any()) + .Returns(new ImportLeaveBalancesResult + { + AsAtDate = new DateOnly(2026, 8, 1), + Created = 1, + Updated = 2, + UnmatchedEmployees = ["Nobody Here"], + Warnings = ["Jane Doe has an unusually large balance"] + }); + + var path = WriteFile("balances.csv", ValidCsv); + var service = new LeaveBalanceImportService(api); + + var result = await service.ImportAsync(path, TestContext.Current.CancellationToken); + + // The server parses the CSV, so it must arrive exactly as written — no re-encoding. + sent.Should().Be(ValidCsv); + result.Created.Should().Be(1); + result.Updated.Should().Be(2); + result.UnmatchedEmployees.Should().Equal("Nobody Here"); + result.Warnings.Should().ContainSingle(); + } + + [Fact] + public async Task Import_WhenApiReturnsNullCollections_NormalizesThemToEmpty() + { + var api = Substitute.For(); + api.ImportLeaveBalancesAsync(Arg.Any(), Arg.Any()) + .Returns(new ImportLeaveBalancesResult + { + AsAtDate = new DateOnly(2026, 8, 1), + UnmatchedEmployees = null!, + Warnings = null! + }); + var service = new LeaveBalanceImportService(api); + var path = WriteFile("balances.csv", ValidCsv); + + var result = await service.ImportAsync(path, TestContext.Current.CancellationToken); + + result.UnmatchedEmployees.Should().BeEmpty(); + result.Warnings.Should().BeEmpty(); + } + + [Fact] + public async Task Import_WhenFileMissing_DoesNotCallApi() + { + var api = Substitute.For(); + var service = new LeaveBalanceImportService(api); + + var act = () => service.ImportAsync(Path.Combine(_tempDir, "nope.csv"), TestContext.Current.CancellationToken); + + (await act.Should().ThrowAsync()) + .WithMessage("File not found:*"); + api.ShouldNotHaveReceived(nameof(ITimeProApiClient.ImportLeaveBalancesAsync)); + } + + [Fact] + public async Task Import_WhenRelativePathResolvesElsewhere_ReportsBothPaths() + { + var api = Substitute.For(); + var service = new LeaveBalanceImportService(api); + + // A shell that strips backslashes turns "C:\Users\me\x.csv" into a drive-relative + // "C:Usersmex.csv", which silently resolves against the working directory. + var act = () => service.ImportAsync("Usersmexero.csv", TestContext.Current.CancellationToken); + + (await act.Should().ThrowAsync()) + .WithMessage("*resolved from 'Usersmexero.csv'*"); + api.ShouldNotHaveReceived(nameof(ITimeProApiClient.ImportLeaveBalancesAsync)); + } + + [Fact] + public async Task Import_WhenPathIsDirectory_DoesNotCallApi() + { + var api = Substitute.For(); + var service = new LeaveBalanceImportService(api); + + var act = () => service.ImportAsync(_tempDir, TestContext.Current.CancellationToken); + + (await act.Should().ThrowAsync()) + .WithMessage("*is a directory*"); + api.ShouldNotHaveReceived(nameof(ITimeProApiClient.ImportLeaveBalancesAsync)); + } + + [Fact] + public async Task Import_WhenFileEmpty_DoesNotCallApi() + { + var api = Substitute.For(); + var service = new LeaveBalanceImportService(api); + var path = WriteFile("empty.csv", string.Empty); + + var act = () => service.ImportAsync(path, TestContext.Current.CancellationToken); + + (await act.Should().ThrowAsync()) + .WithMessage("File is empty:*"); + api.ShouldNotHaveReceived(nameof(ITimeProApiClient.ImportLeaveBalancesAsync)); + } + + [Fact] + public async Task Import_WhenFileIsWhitespaceOnly_DoesNotCallApi() + { + var api = Substitute.For(); + var service = new LeaveBalanceImportService(api); + var path = WriteFile("blank.csv", " \n\n"); + + var act = () => service.ImportAsync(path, TestContext.Current.CancellationToken); + + (await act.Should().ThrowAsync()) + .WithMessage("File contains no data:*"); + api.ShouldNotHaveReceived(nameof(ITimeProApiClient.ImportLeaveBalancesAsync)); + } + + [Fact] + public async Task Import_WhenFileIsBinary_DoesNotCallApi() + { + var api = Substitute.For(); + var service = new LeaveBalanceImportService(api); + var path = Path.Combine(_tempDir, "balances.xlsx"); + File.WriteAllBytes(path, [0x50, 0x4B, 0x03, 0x04, 0x00, 0x00, 0x01]); + + var act = () => service.ImportAsync(path, TestContext.Current.CancellationToken); + + (await act.Should().ThrowAsync()) + .WithMessage("*looks like a binary file*"); + api.ShouldNotHaveReceived(nameof(ITimeProApiClient.ImportLeaveBalancesAsync)); + } + + [Fact] + public async Task Import_WhenFileIsNotUtf8_ExplainsHowToExportIt() + { + var api = Substitute.For(); + var service = new LeaveBalanceImportService(api); + var path = Path.Combine(_tempDir, "windows-1252.csv"); + File.WriteAllBytes(path, + [ + .. Encoding.ASCII.GetBytes("Employee,Leave Type,Units\nJos"), + 0xE9, + .. Encoding.ASCII.GetBytes(",Annual Leave,76.00\n") + ]); + + var act = () => service.ImportAsync(path, TestContext.Current.CancellationToken); + + (await act.Should().ThrowAsync()) + .WithMessage("*not valid UTF-8*Export*UTF-8 CSV*"); + api.ShouldNotHaveReceived(nameof(ITimeProApiClient.ImportLeaveBalancesAsync)); + } + + [Fact] + public async Task Import_WhenApiReturnsUnauthorized_DirectsUserToLogin() + { + var api = Substitute.For(); + api.ImportLeaveBalancesAsync(Arg.Any(), Arg.Any()) + .Returns>(_ => throw new ApiException(401, "Unauthorized", null)); + var service = new LeaveBalanceImportService(api); + var path = WriteFile("balances.csv", ValidCsv); + + var act = () => service.ImportAsync(path, TestContext.Current.CancellationToken); + + (await act.Should().ThrowAsync()) + .WithMessage("*authentication failed*tp login*"); + } + + [Fact] + public async Task Import_WhenApiReturnsForbidden_ExplainsAdminRequirement() + { + var api = Substitute.For(); + api.ImportLeaveBalancesAsync(Arg.Any(), Arg.Any()) + .Returns>(_ => throw new ApiException(403, "Forbidden", null)); + var service = new LeaveBalanceImportService(api); + var path = WriteFile("balances.csv", ValidCsv); + + var act = () => service.ImportAsync(path, TestContext.Current.CancellationToken); + + (await act.Should().ThrowAsync()) + .WithMessage("*leave admin rights*"); + } + + [Fact] + public async Task Import_WhenApiRejectsCsv_SurfacesServerMessage() + { + var api = Substitute.For(); + api.ImportLeaveBalancesAsync(Arg.Any(), Arg.Any()) + .Returns>(_ => throw new ApiException( + 422, "Unprocessable Entity", "\"Missing required column 'Units'.\"")); + var service = new LeaveBalanceImportService(api); + var path = WriteFile("balances.csv", ValidCsv); + + var act = () => service.ImportAsync(path, TestContext.Current.CancellationToken); + + (await act.Should().ThrowAsync()) + .WithMessage("*Missing required column 'Units'.*"); + } + + [Fact] + public async Task Import_WhenApiFailsAfterSubmission_WarnsBeforeRetrying() + { + var api = Substitute.For(); + api.ImportLeaveBalancesAsync(Arg.Any(), Arg.Any()) + .Returns>(_ => throw new ApiException(500, "Server Error", null)); + var service = new LeaveBalanceImportService(api); + var path = WriteFile("balances.csv", ValidCsv); + + var act = () => service.ImportAsync(path, TestContext.Current.CancellationToken); + + var exception = await act.Should().ThrowAsync(); + exception.WithMessage("*may have been applied*tp leave balances status*before retrying*"); + exception.Which.InnerException.Should().BeOfType(); + } + + [Fact] + public async Task Import_WhenSuccessResponseIsIncomplete_WarnsBeforeRetrying() + { + var api = Substitute.For(); + api.ImportLeaveBalancesAsync(Arg.Any(), Arg.Any()) + .Returns(new ImportLeaveBalancesResult()); + var service = new LeaveBalanceImportService(api); + var path = WriteFile("balances.csv", ValidCsv); + + var act = () => service.ImportAsync(path, TestContext.Current.CancellationToken); + + (await act.Should().ThrowAsync()) + .WithMessage("*may have been applied*tp leave balances status*before retrying*"); + } + + [Theory] + [InlineData("\"Missing required column 'Units'.\"", "Missing required column 'Units'.")] + [InlineData("Not json at all", "Not json at all")] + [InlineData("", "the file was rejected without a reason.")] + public void DescribeUnprocessable_UnwrapsServerBody(string body, string expected) + { + LeaveBalanceImportService.DescribeUnprocessable(body).Should().Be(expected); + } + + private string WriteFile(string name, string content) + { + var path = Path.Combine(_tempDir, name); + File.WriteAllText(path, content, new UTF8Encoding(false)); + return path; + } + + public void Dispose() + { + if (Directory.Exists(_tempDir)) + Directory.Delete(_tempDir, recursive: true); + GC.SuppressFinalize(this); + } +} diff --git a/tests/SSW.TimePro.Cli.Tests/Features/Mcp/AccountingMcpToolsTests.cs b/tests/SSW.TimePro.Cli.Tests/Features/Mcp/AccountingMcpToolsTests.cs new file mode 100644 index 0000000..ebec19e --- /dev/null +++ b/tests/SSW.TimePro.Cli.Tests/Features/Mcp/AccountingMcpToolsTests.cs @@ -0,0 +1,149 @@ +using System.Text.Json; +using FluentAssertions; +using ModelContextProtocol.Server; +using NSubstitute; +using SSW.TimePro.Cli.Features.Leave; +using SSW.TimePro.Cli.Features.Mcp.Tools; +using SSW.TimePro.Cli.Infrastructure.ApiClient; +using SSW.TimePro.Cli.Infrastructure.Config; +using SSW.TimePro.Cli.Shared.Models; +using Xunit; + +namespace SSW.TimePro.Cli.Tests.Features.Mcp; + +public class AccountingMcpToolsTests +{ + [Fact] + public void BalanceTools_AreSeparatedBetweenDefaultAndAccountingSurfaces() + { + typeof(LeaveMcpTools).GetMethod(nameof(LeaveMcpTools.GetLeaveBalanceStatus)).Should().NotBeNull(); + typeof(LeaveMcpTools).GetMethod(nameof(AccountingMcpTools.ImportLeaveBalances)).Should().BeNull(); + typeof(AccountingMcpTools).GetMethod(nameof(AccountingMcpTools.ImportLeaveBalances)).Should().NotBeNull(); + } + + [Fact] + public void ImportLeaveBalances_IsMarkedDestructiveAndIdempotent() + { + var attribute = typeof(AccountingMcpTools) + .GetMethod(nameof(AccountingMcpTools.ImportLeaveBalances))! + .GetCustomAttributes(typeof(McpServerToolAttribute), inherit: false) + .Cast() + .Single(); + + attribute.Destructive.Should().BeTrue(); + attribute.Idempotent.Should().BeTrue(); + attribute.ReadOnly.Should().BeFalse(); + } + + [Fact] + public async Task ImportLeaveBalances_WithCsvPath_SendsFileContentsAndReportsSkippedRows() + { + var api = Substitute.For(); + var config = CreateConfig(); + const string csv = "Employee,Leave Type,Units\nJane Doe,Annual Leave,76.00\n"; + string? sent = null; + api.ImportLeaveBalancesAsync(Arg.Do(value => sent = value), Arg.Any()) + .Returns(new ImportLeaveBalancesResult + { + AsAtDate = new DateOnly(2026, 8, 1), + Created = 1, + Updated = 2, + UnmatchedEmployees = ["Nobody Here"], + Warnings = ["Jane Doe has an unusually large balance"] + }); + + var path = Path.Combine(Path.GetTempPath(), $"tp-mcp-balances-{Guid.NewGuid():N}.csv"); + await File.WriteAllTextAsync(path, csv, TestContext.Current.CancellationToken); + + try + { + var tools = CreateTools(api, config); + var json = await tools.ImportLeaveBalances(path, TestContext.Current.CancellationToken); + + sent.Should().Be(csv); + using var doc = JsonDocument.Parse(json); + doc.RootElement.GetProperty("success").GetBoolean().Should().BeTrue(); + doc.RootElement.GetProperty("created").GetInt32().Should().Be(1); + doc.RootElement.GetProperty("updated").GetInt32().Should().Be(2); + doc.RootElement.GetProperty("unmatchedEmployees").EnumerateArray() + .Select(e => e.GetString()).Should().Equal("Nobody Here"); + doc.RootElement.GetProperty("warnings").GetArrayLength().Should().Be(1); + } + finally + { + File.Delete(path); + } + } + + [Fact] + public async Task ImportLeaveBalances_WhenFileMissing_ReturnsErrorWithoutCallingApi() + { + var api = Substitute.For(); + var tools = CreateTools(api, CreateConfig()); + + var json = await tools.ImportLeaveBalances( + Path.Combine(Path.GetTempPath(), $"tp-missing-{Guid.NewGuid():N}.csv"), + TestContext.Current.CancellationToken); + + using var doc = JsonDocument.Parse(json); + doc.RootElement.GetProperty("error").GetString().Should().StartWith("File not found"); + await api.DidNotReceive().ImportLeaveBalancesAsync(Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task ImportLeaveBalances_WhenNotLoggedIn_ReturnsErrorWithoutCallingApi() + { + var api = Substitute.For(); + var config = Substitute.For(); + config.LoadActiveTenantConfig().Returns((TenantConfig?)null); + var tools = CreateTools(api, config); + + var json = await tools.ImportLeaveBalances("balances.csv", TestContext.Current.CancellationToken); + + using var doc = JsonDocument.Parse(json); + doc.RootElement.GetProperty("error").GetString().Should().Contain("Not logged in"); + await api.DidNotReceive().ImportLeaveBalancesAsync(Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task ImportLeaveBalances_WhenSubmissionOutcomeIsUncertain_ReturnsSafeRetryGuidance() + { + var api = Substitute.For(); + api.ImportLeaveBalancesAsync(Arg.Any(), Arg.Any()) + .Returns>(_ => throw new HttpRequestException("Connection dropped")); + var tools = CreateTools(api, CreateConfig()); + var path = Path.Combine(Path.GetTempPath(), $"tp-mcp-balances-{Guid.NewGuid():N}.csv"); + await File.WriteAllTextAsync(path, "Employee,Leave Type,Units\nJane Doe,Annual Leave,76.00\n", + TestContext.Current.CancellationToken); + + try + { + var json = await tools.ImportLeaveBalances(path, TestContext.Current.CancellationToken); + + using var doc = JsonDocument.Parse(json); + doc.RootElement.GetProperty("mayHaveBeenApplied").GetBoolean().Should().BeTrue(); + doc.RootElement.GetProperty("error").GetString() + .Should().Contain("tp leave balances status").And.Contain("before retrying"); + } + finally + { + File.Delete(path); + } + } + + private static IConfigService CreateConfig() + { + var config = Substitute.For(); + config.LoadActiveTenantConfig().Returns(new TenantConfig + { + TenantId = "test", + ApiUrl = "https://timepro.example", + ApiKey = "test-api-key", + EmployeeId = "TST" + }); + return config; + } + + private static AccountingMcpTools CreateTools(ITimeProApiClient api, IConfigService config) => + new(api, config, new LeaveBalanceImportService(api)); +} diff --git a/tests/SSW.TimePro.Cli.Tests/Features/Mcp/LeaveMcpToolsTests.cs b/tests/SSW.TimePro.Cli.Tests/Features/Mcp/LeaveMcpToolsTests.cs index 8a1496c..ac2c262 100644 --- a/tests/SSW.TimePro.Cli.Tests/Features/Mcp/LeaveMcpToolsTests.cs +++ b/tests/SSW.TimePro.Cli.Tests/Features/Mcp/LeaveMcpToolsTests.cs @@ -1,5 +1,6 @@ using System.Text.Json; using FluentAssertions; +using ModelContextProtocol.Server; using NSubstitute; using SSW.TimePro.Cli.Features.Leave; using SSW.TimePro.Cli.Features.Mcp.Tools; @@ -12,6 +13,19 @@ namespace SSW.TimePro.Cli.Tests.Features.Mcp; public class LeaveMcpToolsTests { + [Fact] + public void GetLeaveBalanceStatus_IsMarkedReadOnly() + { + var attribute = typeof(LeaveMcpTools) + .GetMethod(nameof(LeaveMcpTools.GetLeaveBalanceStatus))! + .GetCustomAttributes(typeof(McpServerToolAttribute), inherit: false) + .Cast() + .Single(); + + attribute.ReadOnly.Should().BeTrue(); + attribute.Destructive.Should().BeFalse(); + } + [Fact] public async Task CreateLeave_WhenProfileTimezoneAvailable_SendsDateOffsetsFromProfileTimezone() { @@ -265,6 +279,33 @@ public async Task UpdateLeave_WhenChangingNote_PreservesExistingFields() .NotContain(call => call.GetMethodInfo().Name == nameof(ITimeProApiClient.UpdateLeaveAsync)); } + [Fact] + public async Task GetLeaveBalanceStatus_WhenNothingImported_ReportsNotImported() + { + var api = Substitute.For(); + var config = Substitute.For(); + config.LoadActiveTenantConfig().Returns(new TenantConfig + { + TenantId = "test", + ApiUrl = "https://timepro.example", + ApiKey = "test-api-key", + EmployeeId = "TST" + }); + api.GetLeaveBalanceStatusAsync(Arg.Any()).Returns(new LeaveBalanceStatus + { + AsAtDate = null, + LastImportedAt = null, + EmployeeCount = 0, + IsStale = false + }); + var tools = CreateTools(api, config); + + var json = await tools.GetLeaveBalanceStatus(TestContext.Current.CancellationToken); + + using var doc = JsonDocument.Parse(json); + doc.RootElement.GetProperty("imported").GetBoolean().Should().BeFalse(); + } + private static LeaveMcpTools CreateTools(ITimeProApiClient api, IConfigService config) => new(api, config, new LeaveCreateService(api), new LeaveUpdateService(api)); diff --git a/tests/SSW.TimePro.Cli.Tests/Features/Skills/SkillGenerationTests.cs b/tests/SSW.TimePro.Cli.Tests/Features/Skills/SkillGenerationTests.cs index 5f3d278..df4bb3c 100644 --- a/tests/SSW.TimePro.Cli.Tests/Features/Skills/SkillGenerationTests.cs +++ b/tests/SSW.TimePro.Cli.Tests/Features/Skills/SkillGenerationTests.cs @@ -64,6 +64,10 @@ public void Frontmatter_ContainsNameDescriptionAndAllowedTools() output.Should().Contain("name: timepro-timesheets"); output.Should().Contain("description:"); output.Should().Contain("allowed-tools: Bash(tp *), Bash(sl *)"); + output.Should().Contain("tp leave balances status --json"); + output.Should().Contain("The equivalent default\nMCP tool is `get_leave_balance_status`"); + output.Should().Contain("`import_leave_balances` workflow is accounting-gated"); + output.Should().NotContain("tp leave balances import"); } [Fact] @@ -121,7 +125,12 @@ public void Accounting_ProducesAccountingSkillWithoutPrefetchBlock() output.Should().Contain("guides/accounting/tax-mismatch.md"); output.Should().Contain("guides/accounting/invoice-evidence-pack.md"); output.Should().Contain("guides/accounting/client-accounting-position.md"); - output.Should().Contain("MCP exposes primitive read-only tools"); + output.Should().Contain("Import Xero leave balances (destructive)"); + output.Should().Contain("tp leave balances status --json"); + output.Should().Contain("tp leave balances import ./LeaveBalances.csv --yes --json"); + output.Should().Contain("`get_leave_balance_status` is on the default TimePro surface"); + output.Should().Contain("`import_leave_balances` tool is available only after `tp feature accounting enable`"); + output.Should().Contain("explicitly approved `import_leave_balances` write"); output.Should().Contain("With another MCP such as Xero"); output.Should().Contain("tp feature accounting enable"); output.Should().NotContain("## Run these first"); diff --git a/tests/SSW.TimePro.Cli.Tests/Infrastructure/Paths/PathExpanderTests.cs b/tests/SSW.TimePro.Cli.Tests/Infrastructure/Paths/PathExpanderTests.cs new file mode 100644 index 0000000..80a938f --- /dev/null +++ b/tests/SSW.TimePro.Cli.Tests/Infrastructure/Paths/PathExpanderTests.cs @@ -0,0 +1,36 @@ +using FluentAssertions; +using SSW.TimePro.Cli.Infrastructure.Paths; +using Xunit; + +namespace SSW.TimePro.Cli.Tests.Infrastructure.Paths; + +public class PathExpanderTests +{ + [Theory] + [InlineData("~/Downloads/LeaveBalances.csv")] + [InlineData(@"~\Downloads\LeaveBalances.csv")] + public void ExpandHomeDirectory_WithTildePrefix_UsesCurrentUserProfile(string path) + { + var expected = Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), + "Downloads", + "LeaveBalances.csv"); + + PathExpander.ExpandHomeDirectory(path).Should().Be(expected); + } + + [Fact] + public void ExpandHomeDirectory_WithTildeOnly_ReturnsCurrentUserProfile() + { + PathExpander.ExpandHomeDirectory("~").Should().Be( + Environment.GetFolderPath(Environment.SpecialFolder.UserProfile)); + } + + [Theory] + [InlineData("~northwind/LeaveBalances.csv")] + [InlineData("exports/~/LeaveBalances.csv")] + public void ExpandHomeDirectory_WhenTildeIsNotTheFirstSegment_LeavesPathUnchanged(string path) + { + PathExpander.ExpandHomeDirectory(path).Should().Be(path); + } +}