From 342c8b7f41f6df1a8c453694f5663008e2c6cb55 Mon Sep 17 00:00:00 2001 From: Josh Berman Date: Fri, 25 Sep 2026 12:43:09 +1000 Subject: [PATCH] feat(users): list active staff via user list --staff and the list_staff MCP tool Adds StaffDirectory, the roster a missing-timesheet check runs against: active employees minus uncategorised, O, OA-E, EXCON and WE accounts, and retired accounts renamed with a zz prefix. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 4 +- README.md | 4 +- .../Features/Mcp/Tools/LookupMcpTools.cs | 13 +++++ .../Features/Users/ListCommand.cs | 14 ++++- .../Features/Users/StaffDirectory.cs | 56 +++++++++++++++++++ .../Mcp/Discovery/tools-list.accounting.json | 13 ++++- .../Mcp/Discovery/tools-list.default.json | 13 ++++- .../Goldens/Mcp/Parity/ListStaff.19.cli.json | 7 +++ .../Goldens/Mcp/Parity/ListStaff.19.mcp.json | 7 +++ .../Goldens/Mcp/Tools/ListStaff.apiError.json | 6 ++ .../Goldens/Mcp/Tools/ListStaff.empty.json | 1 + .../Mcp/Tools/ListStaff.populated.json | 7 +++ .../Mcp/McpCliParityTable.cs | 12 +++- .../Mcp/McpStdioDiscoveryTests.cs | 4 +- .../Mcp/McpToolCatalog.cs | 6 ++ .../Mcp/NorthwindApi.cs | 37 +++++++++++- .../Features/Users/StaffDirectoryTests.cs | 45 +++++++++++++++ 17 files changed, 238 insertions(+), 11 deletions(-) create mode 100644 src/SSW.TimePro.Cli/Features/Users/StaffDirectory.cs create mode 100644 tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ListStaff.19.cli.json create mode 100644 tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ListStaff.19.mcp.json create mode 100644 tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListStaff.apiError.json create mode 100644 tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListStaff.empty.json create mode 100644 tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListStaff.populated.json create mode 100644 tests/SSW.TimePro.Cli.Tests/Features/Users/StaffDirectoryTests.cs diff --git a/AGENTS.md b/AGENTS.md index 3ae67db..8242f39 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -316,8 +316,8 @@ tests fail until you do. variants; `Goldens/Mcp/Tools/*.json` is the raw text each tool returned. An API failure escaping a tool as a protocol error rather than an `isError` payload is part of what is snapshotted. - `McpStdioClient` launches the real `tp mcp` with `TIMEPRO_CLI_CONFIG_DIR` pointing at a throwaway - config. `Goldens/Mcp/Discovery/` holds the `tools/list` snapshots with accounting off (18 tools) - and on (47); `Goldens/Mcp/Calls/` holds `tools/call` envelopes. + config. `Goldens/Mcp/Discovery/` holds the `tools/list` snapshots with accounting off (19 tools) + and on (48); `Goldens/Mcp/Calls/` holds `tools/call` envelopes. - `TimesheetToolsUsingApiDirectly` is the shrink-only allowlist of timesheet tools still calling `ITimeProApiClient` themselves; `ToolIlScanner` reads the tools' IL (constructor inspection cannot answer it, since the shared services take the client as an argument). diff --git a/README.md b/README.md index be6b1a0..f3a0aaf 100644 --- a/README.md +++ b/README.md @@ -156,7 +156,7 @@ tp ts get 2026-03-12 # Specific date | `tp whats-new [--url]` | Show embedded Markdown release notes, or print the latest known release-notes URL | | `tp skills create TARGET [--global] [--force]` | Generate unified agent skill files using enabled feature packs | | `tp user me` | Show current user info | -| `tp user list [QUERY]` | List users and match names/emails to EmpIDs (`--emp-id`, `--email`, `--all`, `--json`) | +| `tp user list [QUERY]` | List users and match names/emails to EmpIDs (`--emp-id`, `--email`, `--all`, `--staff`, `--json`) | | `tp user get EMP_ID` | Show focused user details by EmpID (`--json`) | | `tp blog list` | Latest blog posts (`--mine`, `--limit N`, `--all`) | | `tp mcp` | Start MCP server (stdio); `--tenant NAME` binds the session to a specific tenant config without changing the global active tenant | @@ -624,7 +624,7 @@ Current default tool groups include: | Group | Examples | |-------|----------| | 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 | +| Lookup | Search clients, list projects, get client rate, CRM bookings, location and repo mapping, `list_staff` (active staff expected to log timesheets) | | 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: diff --git a/src/SSW.TimePro.Cli/Features/Mcp/Tools/LookupMcpTools.cs b/src/SSW.TimePro.Cli/Features/Mcp/Tools/LookupMcpTools.cs index 63ebea6..0970e89 100644 --- a/src/SSW.TimePro.Cli/Features/Mcp/Tools/LookupMcpTools.cs +++ b/src/SSW.TimePro.Cli/Features/Mcp/Tools/LookupMcpTools.cs @@ -4,6 +4,7 @@ using ModelContextProtocol.Server; using SSW.TimePro.Cli.Features.Projects; using SSW.TimePro.Cli.Features.Rates; +using SSW.TimePro.Cli.Features.Users; using SSW.TimePro.Cli.Infrastructure.ApiClient; using SSW.TimePro.Cli.Infrastructure.Config; using SSW.TimePro.Cli.Infrastructure.Paths; @@ -91,6 +92,18 @@ public async Task GetCrmBookings( return JsonSerializer.Serialize(results, JsonOpts); } + [McpServerTool] + [Description("List active staff expected to log timesheets (empId, name, email). Excludes admin, service, work experience, contractor and retired accounts. Pair with check_week per empId to find missing timesheets.")] + public async Task ListStaff(CancellationToken ct = default) + { + var tenant = _config.LoadActiveTenantConfig(); + if (tenant?.EmployeeId is null) + return """{"error": "Not logged in"}"""; + + var staff = await StaffDirectory.ListAsync(_api, ct); + return JsonSerializer.Serialize(staff, JsonOpts); + } + [McpServerTool] [Description("Get the WFH/location defaults and repo mapping for a given path.")] public string GetLocationAndMapping( diff --git a/src/SSW.TimePro.Cli/Features/Users/ListCommand.cs b/src/SSW.TimePro.Cli/Features/Users/ListCommand.cs index 3418743..f6229f1 100644 --- a/src/SSW.TimePro.Cli/Features/Users/ListCommand.cs +++ b/src/SSW.TimePro.Cli/Features/Users/ListCommand.cs @@ -38,6 +38,10 @@ public class Settings : CommandSettings [Description("Include former employees")] public bool All { get; set; } + [CommandOption("--staff")] + [Description("Only active staff expected to log timesheets (excludes admin, service, work experience, contractor and retired accounts)")] + public bool Staff { get; set; } + [CommandOption("--limit ")] [Description("Maximum rows to show; 0 means no limit (default: 50)")] public int Limit { get; set; } = 50; @@ -60,9 +64,17 @@ protected override async Task ExecuteAsync(CommandContext context, Settings return 1; } + if (settings.Staff && settings.All) + { + OutputHelper.WriteError("--staff lists active employees only and cannot be combined with --all."); + return 1; + } + try { - var users = await _api.ListUsersAsync(settings.All, cancellationToken); + var users = settings.Staff + ? await StaffDirectory.ListAsync(_api, cancellationToken) + : await _api.ListUsersAsync(settings.All, cancellationToken); var filtered = ApplyFilters(users, settings).ToList(); var visible = settings.Limit == 0 ? filtered diff --git a/src/SSW.TimePro.Cli/Features/Users/StaffDirectory.cs b/src/SSW.TimePro.Cli/Features/Users/StaffDirectory.cs new file mode 100644 index 0000000..44d42cc --- /dev/null +++ b/src/SSW.TimePro.Cli/Features/Users/StaffDirectory.cs @@ -0,0 +1,56 @@ +using SSW.TimePro.Cli.Infrastructure.ApiClient; +using SSW.TimePro.Cli.Shared.Models; + +namespace SSW.TimePro.Cli.Features.Users; + +/// +/// The active employees expected to log timesheets — the roster a "who is missing timesheets" +/// check runs against. Shared by tp user list --staff and the ListStaff MCP tool. +/// +public static class StaffDirectory +{ + /// + /// Categories that never log timesheets: office/admin (O, OA-E), external + /// contractors (EXCON) and work experience (WE). Uncategorised accounts are + /// excluded too; they are service, bot and admin logins. + /// + private static readonly HashSet ExcludedCategories = + new(["O", "OA-E", "EXCON", "WE"], StringComparer.OrdinalIgnoreCase); + + /// Retired accounts are renamed with a "zz" prefix rather than given an end date. + private const string RetiredPrefix = "zz"; + + private const int MaxConcurrentDetailReads = 8; + + /// + /// Lists active staff. The dropdown carries no category, so each remaining employee's detail + /// is read (bounded concurrency); "zz" accounts are dropped first to skip their reads. + /// + public static async Task> ListAsync(ITimeProApiClient api, CancellationToken ct) + { + var candidates = (await api.ListUsersAsync(includeFormerEmployees: false, ct)) + .Where(u => !IsRetired(u.Name)) + .ToList(); + + var categories = new Dictionary(StringComparer.OrdinalIgnoreCase); + await Parallel.ForEachAsync( + candidates, + new ParallelOptions { MaxDegreeOfParallelism = MaxConcurrentDetailReads, CancellationToken = ct }, + async (user, token) => + { + var detail = await api.GetUserAsync(user.EmpId!, token); + lock (categories) + categories[user.EmpId!] = detail?.CategoryId; + }); + + return candidates + .Where(u => IsStaffCategory(categories.GetValueOrDefault(u.EmpId!))) + .ToList(); + } + + private static bool IsRetired(string? name) => + name?.TrimStart().StartsWith(RetiredPrefix, StringComparison.OrdinalIgnoreCase) == true; + + private static bool IsStaffCategory(string? categoryId) => + !string.IsNullOrWhiteSpace(categoryId) && !ExcludedCategories.Contains(categoryId.Trim()); +} diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Discovery/tools-list.accounting.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Discovery/tools-list.accounting.json index 3268e4b..1b38ed9 100644 --- a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Discovery/tools-list.accounting.json +++ b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Discovery/tools-list.accounting.json @@ -1,5 +1,5 @@ { - "count": 47, + "count": 48, "tools": [ { "description": "Accept a suggested timesheet, converting it into a real timesheet. Returns the entry as saved.", @@ -1219,6 +1219,17 @@ }, "name": "list_recurring_invoices" }, + { + "description": "List active staff expected to log timesheets (empId, name, email). Excludes admin, service, work experience, contractor and retired accounts. Pair with check_week per empId to find missing timesheets.", + "execution": { + "taskSupport": "optional" + }, + "inputSchema": { + "properties": {}, + "type": "object" + }, + "name": "list_staff" + }, { "description": "Query timesheets across empIds, clients, projects and a date range. Returns detailed rows including hours and sell price; sell prices/amounts are treated as ex-GST for invoice reconciliation. employeeIds is accepted as an alias for empIds.", "execution": { diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Discovery/tools-list.default.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Discovery/tools-list.default.json index 2a0e0e7..14ca3b2 100644 --- a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Discovery/tools-list.default.json +++ b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Discovery/tools-list.default.json @@ -1,5 +1,5 @@ { - "count": 18, + "count": 19, "tools": [ { "description": "Accept a suggested timesheet, converting it into a real timesheet. Returns the entry as saved.", @@ -529,6 +529,17 @@ }, "name": "list_iterations" }, + { + "description": "List active staff expected to log timesheets (empId, name, email). Excludes admin, service, work experience, contractor and retired accounts. Pair with check_week per empId to find missing timesheets.", + "execution": { + "taskSupport": "optional" + }, + "inputSchema": { + "properties": {}, + "type": "object" + }, + "name": "list_staff" + }, { "description": "Search for clients by name. Returns client IDs and names.", "execution": { diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ListStaff.19.cli.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ListStaff.19.cli.json new file mode 100644 index 0000000..3580c65 --- /dev/null +++ b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ListStaff.19.cli.json @@ -0,0 +1,7 @@ +[ + { + "email": "bob@northwind.example", + "empId": "BOB", + "name": "Bob Northwind" + } +] diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ListStaff.19.mcp.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ListStaff.19.mcp.json new file mode 100644 index 0000000..3580c65 --- /dev/null +++ b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ListStaff.19.mcp.json @@ -0,0 +1,7 @@ +[ + { + "email": "bob@northwind.example", + "empId": "BOB", + "name": "Bob Northwind" + } +] diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListStaff.apiError.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListStaff.apiError.json new file mode 100644 index 0000000..d7c7e05 --- /dev/null +++ b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListStaff.apiError.json @@ -0,0 +1,6 @@ +{ + "message": "TimePro API returned 500 Internal Server Error", + "responseBody": "{\u0022title\u0022:\u0022Server error\u0022,\u0022detail\u0022:\u0022Northwind API is unavailable\u0022}", + "statusCode": 500, + "threw": "ApiException" +} diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListStaff.empty.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListStaff.empty.json new file mode 100644 index 0000000..fe51488 --- /dev/null +++ b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListStaff.empty.json @@ -0,0 +1 @@ +[] diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListStaff.populated.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListStaff.populated.json new file mode 100644 index 0000000..3580c65 --- /dev/null +++ b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/ListStaff.populated.json @@ -0,0 +1,7 @@ +[ + { + "email": "bob@northwind.example", + "empId": "BOB", + "name": "Bob Northwind" + } +] diff --git a/tests/SSW.TimePro.Cli.Integration/Mcp/McpCliParityTable.cs b/tests/SSW.TimePro.Cli.Integration/Mcp/McpCliParityTable.cs index a905298..28e7677 100644 --- a/tests/SSW.TimePro.Cli.Integration/Mcp/McpCliParityTable.cs +++ b/tests/SSW.TimePro.Cli.Integration/Mcp/McpCliParityTable.cs @@ -46,7 +46,7 @@ public sealed record ParityRow(string ToolMethod, string? CliCommandPath) } /// -/// The CLI/MCP pairing for all 47 tools. Rows flip ExpectParity to true as the unification +/// The CLI/MCP pairing for all 48 tools. Rows flip ExpectParity to true as the unification /// slices land; the five tools with no CLI command at all are the shrink-only allowlist. /// public static class McpCliParityTable @@ -354,6 +354,16 @@ public static class McpCliParityTable new("GET", "/api/Timesheets/GetTimesheetListViewModel") ] }, + + // Appended after the executable rows: Parity goldens are named by row index. + new("ListStaff", "user list") + { + CliArgs = ["user", "list", "--staff", "--limit", "0", "--json"], + InvokeTool = (h, ct) => h.Lookups.ListStaff(ct), + ExpectParity = true, + Note = "Both surfaces project StaffDirectory.ListAsync." + }, + new("GetLocationAndMapping", "location info") { Note = "MCP merges location defaults and repo mapping." }, new("GetLeaveEntries", "leave list") { Note = "MCP returns the items array, CLI the envelope." }, new("GetLeaveBalance", "leave balance") { Note = "Separate projections." }, diff --git a/tests/SSW.TimePro.Cli.Integration/Mcp/McpStdioDiscoveryTests.cs b/tests/SSW.TimePro.Cli.Integration/Mcp/McpStdioDiscoveryTests.cs index a73d7cf..4a2d56c 100644 --- a/tests/SSW.TimePro.Cli.Integration/Mcp/McpStdioDiscoveryTests.cs +++ b/tests/SSW.TimePro.Cli.Integration/Mcp/McpStdioDiscoveryTests.cs @@ -12,8 +12,8 @@ namespace SSW.TimePro.Cli.Integration.Mcp; [Collection(McpStdioCollection.Name)] public class McpStdioDiscoveryTests { - public const int DefaultToolCount = 18; - public const int AccountingEnabledToolCount = 47; + public const int DefaultToolCount = 19; + public const int AccountingEnabledToolCount = 48; [Fact] public async Task ToolsList_WithAccountingDisabled_MatchesGolden() diff --git a/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolCatalog.cs b/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolCatalog.cs index b65779d..8a0be09 100644 --- a/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolCatalog.cs +++ b/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolCatalog.cs @@ -114,6 +114,12 @@ private static IReadOnlyList BuildPopulated() => EmptyBody = "[]" }, + new("ListStaff", "populated", (h, ct) => h.Lookups.ListStaff(ct)) + { + PrimaryRoute = "/api/Employees/DropDown", + EmptyBody = "[]" + }, + new("GetLocationAndMapping", "populated", (h, _) => Task.FromResult(h.Lookups.GetLocationAndMapping("~/code/traders-app"))) { diff --git a/tests/SSW.TimePro.Cli.Integration/Mcp/NorthwindApi.cs b/tests/SSW.TimePro.Cli.Integration/Mcp/NorthwindApi.cs index 6281be5..e437683 100644 --- a/tests/SSW.TimePro.Cli.Integration/Mcp/NorthwindApi.cs +++ b/tests/SSW.TimePro.Cli.Integration/Mcp/NorthwindApi.cs @@ -42,10 +42,11 @@ public static class NorthwindApi PropertyNamingPolicy = JsonNamingPolicy.CamelCase }; - /// Registers every route the 47 MCP tools can reach, populated with Northwind data. + /// Registers every route the 48 MCP tools can reach, populated with Northwind data. public static void StubAll(WireMockServer server) { StubIdentity(server); + StubStaff(server); StubLookups(server); StubTimesheets(server); StubLeave(server); @@ -106,6 +107,40 @@ private static void StubIdentity(WireMockServer server) }); } + /// Work-experience colleague the staff roster must drop by category. + public const string WorkExperienceEmpId = "TIM"; + + /// Retired colleague the staff roster must drop by the "zz" name prefix. + public const string RetiredEmpId = "ZZR"; + + private static void StubStaff(WireMockServer server) + { + Json(server, "/api/Employees/DropDown", "GET", new List + { + new() { Text = $"{EmpName} ({EmpId})", Value = EmpId }, + new() { Text = $"Tim Northwind ({WorkExperienceEmpId})", Value = WorkExperienceEmpId }, + new() { Text = $"zzRita zzNorthwind ({RetiredEmpId})", Value = RetiredEmpId } + }); + + Json(server, "/api/Employees/GetByIds", "POST", new List + { + new() { EmpId = EmpId, Name = EmpName, Email = EmpEmail }, + new() { EmpId = WorkExperienceEmpId, Name = "Tim Northwind", Email = "tim@northwind.example" }, + new() { EmpId = RetiredEmpId, Name = "zzRita zzNorthwind", Email = "rita@northwind.example" } + }); + + Json(server, $"/api/employees/{EmpId}", "GET", new EmployeeDetail + { + EmpId = EmpId, FirstName = "Bob", Surname = "Northwind", Email = EmpEmail, CategoryId = "PM-E" + }); + + Json(server, $"/api/employees/{WorkExperienceEmpId}", "GET", new EmployeeDetail + { + EmpId = WorkExperienceEmpId, FirstName = "Tim", Surname = "Northwind", + Email = "tim@northwind.example", CategoryId = "WE" + }); + } + // ───────────────────────── Lookups ───────────────────────── public const string SentinelDisplayText = "Empty - Please add the project"; diff --git a/tests/SSW.TimePro.Cli.Tests/Features/Users/StaffDirectoryTests.cs b/tests/SSW.TimePro.Cli.Tests/Features/Users/StaffDirectoryTests.cs new file mode 100644 index 0000000..de8ab38 --- /dev/null +++ b/tests/SSW.TimePro.Cli.Tests/Features/Users/StaffDirectoryTests.cs @@ -0,0 +1,45 @@ +using FluentAssertions; +using NSubstitute; +using SSW.TimePro.Cli.Features.Users; +using SSW.TimePro.Cli.Infrastructure.ApiClient; +using SSW.TimePro.Cli.Shared.Models; +using Xunit; + +namespace SSW.TimePro.Cli.Tests.Features.Users; + +public class StaffDirectoryTests +{ + [Fact] + public async Task ListAsync_KeepsOnlyCategorisedStaffOutsideExcludedCategories() + { + var api = Substitute.For(); + var ct = TestContext.Current.CancellationToken; + + api.ListUsersAsync(false, Arg.Any()).Returns( + [ + new EmployeeSummary { EmpId = "BOB", Name = "Bob Northwind" }, + new EmployeeSummary { EmpId = "OFF", Name = "Olive Northwind" }, + new EmployeeSummary { EmpId = "OAE", Name = "Oscar Northwind" }, + new EmployeeSummary { EmpId = "CON", Name = "Connie Northwind" }, + new EmployeeSummary { EmpId = "TIM", Name = "Tim Northwind" }, + new EmployeeSummary { EmpId = "SVC", Name = "Northwind Service" }, + new EmployeeSummary { EmpId = "ZZR", Name = "zzRita zzNorthwind" } + ]); + + Detail(api, "BOB", "PM-E"); + Detail(api, "OFF", "O"); + Detail(api, "OAE", "oa-e"); + Detail(api, "CON", "EXCON"); + Detail(api, "TIM", "WE"); + Detail(api, "SVC", null); + + var staff = await StaffDirectory.ListAsync(api, ct); + + staff.Select(s => s.EmpId).Should().Equal("BOB"); + await api.DidNotReceive().GetUserAsync("ZZR", Arg.Any()); + } + + private static void Detail(ITimeProApiClient api, string empId, string? categoryId) => + api.GetUserAsync(empId, Arg.Any()) + .Returns(new EmployeeDetail { EmpId = empId, CategoryId = categoryId }); +}