refactor(timesheets): share timesheet read fetches between CLI and MCP

[jernejk]

Summary

Slice 4 of #38: the timesheet reads. ts get / GetTimesheets, ts suggest /
GetSuggestedTimesheets and ts check / CheckWeek now share their fetch and, for the week
check, their result document. Extraction only — no wire shape changes, so this is a patch.

Closes #74

What changed

• TimesheetLookup.ForRangeAsync is the single per-day range read. The weekend disagreement
  between the two surfaces is now an explicit WeekendPolicy argument rather than a duplicated
  continue: ts get passes Include, the MCP tool passes Skip (so a weekend-only range still
  answers [] without calling the API). Neither policy changed.
• TimesheetLookup.RefreshAndReadSuggestedAsync is the shared refresh-then-read-then-filter, with
  the refresh happening exactly once inside it, noted as the server-side write it is.
• WeekCheckResult / WeekCheckDay is the week-coverage document both ts check --json and
  CheckWeek serialise; CheckCommand's private DayJson and the MCP anonymous object are gone.
  leaveType keeps its always-emitted annotation, so the CLI still writes it as null.
• Parity table: GetTimesheets (weekday and Saturday) and GetSuggestedTimesheets become
  executable rows with their permitted differences declared; CheckWeek flips to
  ExpectParity = true.
• New architecture test TimesheetTools_DoNotCallTheApiClientDirectly_ExceptWhereAllowlisted.
  Constructor inspection cannot answer this (the shared services take the client as an argument),
  so ToolIlScanner walks the tools' IL — including async state machines and lambdas — and maps
  calls back to the owning tool. It fails closed: a call it cannot attribute to a tool method
  throws rather than being dropped. Allowlist: DeleteTimesheet (the delete write itself; its
  suggestion pre-check is already shared) and ListIterations (a one-endpoint pass-through on both
  surfaces, so there is nothing to share until the lookups slice). It may only shrink.
• The Northwind timesheet-list stub now echoes the requested date, so the Saturday golden shows
  Saturday-dated rows instead of Monday ones.

How verified

Tests: unit 589, integration 299, both green (+6 and +7 over origin/main).

Golden delta against origin/main is additions only — no existing golden changed:

$ git diff --name-status origin/main -- '*Goldens*'
A  .../Goldens/Mcp/Parity/GetSuggestedTimesheets.18.cli.json
A  .../Goldens/Mcp/Parity/GetSuggestedTimesheets.18.mcp.json
A  .../Goldens/Mcp/Parity/GetTimesheets.16.cli.json
A  .../Goldens/Mcp/Parity/GetTimesheets.16.mcp.json
A  .../Goldens/Mcp/Parity/GetTimesheets.17.cli.json
A  .../Goldens/Mcp/Parity/GetTimesheets.17.mcp.json
A  .../Goldens/Mcp/Tools/GetTimesheets.weekendDate.json

The branch was rebased after #73, which inserted a CreateLeave row ahead of these ones, so the
parity indices shifted by one (15/16/17 -> 16/17/18). The moves are pure renames with
byte-identical content — verified by diffing each pre-rebase file against its new name, all six
identical (git's own -M heuristic mislabels two of them because their whole content is []).
#71 tracks replacing the positional index in golden names with a stable key so a table insertion
stops renaming unrelated snapshots.

Forced failures, to show the new checks can fail:

• Flipping the MCP tool to WeekendPolicy.Include fails two goldens
  (Tools/GetTimesheets.weekendDate.json and the Saturday parity golden) — the weekend policy is
  locked, not incidental.
• Adding one _api call inside GetSuggestedTimesheets fails the architecture test:
  items {"GetSuggestedTimesheets"} are not part of the superset.
• Routing an _api call through a plain private helper used to pass (the scanner silently
  dropped callers it could not attribute). ToolIlScannerTests now pins both halves against
  fixture tool types: the direct call is attributed to its tool, and the helper case throws
  InvalidOperationException naming the method.

Staging (--tenant ssw-staging, apiUrl contains staging, isProduction: false; no writes
beyond the server-side suggestion refresh), showing the two weekend policies for the same Saturday:

$ dotnet run --project src/SSW.TimePro.Cli -- ts get --from 2026-09-12 --to 2026-09-12 --json --tenant ssw-staging
{
  "from": "2026-09-12",
  "to": "2026-09-12",
  "days": [
    { "date": "2026-09-12", "dayOfWeek": "Saturday", "timesheets": [], "totalHours": 0 }
  ]
}

# MCP get_timesheets {"date":"2026-09-12"} over stdio against the same tenant
[]

ts check --week --json and the MCP check_week over stdio returned the same document, compared
in full rather than by eye (json.load(cli) == json.load(mcp) -> True); trimmed, with the
employee id redacted:

{
  "empId": "<redacted>",
  "weekStart": "2026-09-07",
  "weekEnd": "2026-09-11",
  "errors": 5, "warnings": 0, "infos": 0,
  "allCovered": false, "pendingSuggestions": 0,
  "days": [ { "date": "2026-09-07", "dayOfWeek": "Monday", "totalHours": 0, "timesheetCount": 0,
              "suggestedCount": 0, "leaveHours": 0, "leaveType": null, "covered": false,
              "coverReason": "missing",
              "issues": [ { "severity": "error", "message": "No timesheets entered" } ] }, ... ]
}

ts suggest 2026-09-11 --json and MCP get_suggested_timesheets for the same date both returned
[] on staging (no suggestions there), so the shared refresh path ran end to end but the populated
grouping is covered by the WireMock goldens, not by staging.

The staging runs above were executed before the rebase and the scanner change; neither touches the
read paths they exercise, and the full WireMock suite was re-run after both.

Not done here: the CLI/MCP envelope shapes still differ for ts get and ts suggest (declared per
row in the parity table); aligning them is a contract change, not this patch.

Risks

ToolIlScanner is the novel piece: it throws on an unknown opcode and on an unattributable caller
rather than silently finding nothing, and the forced failures above show it detects calls through
async state machines and refuses private-helper indirection. GetCommand's render helpers now take
read-only collection types; behaviour is unchanged and the existing human-output tests cover them.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Two moderate findings remain regarding weekend fixture validity and IL scanner coverage.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Refactors timesheet reads so CLI and MCP share fetch logic and week-check serialization while preserving existing output contracts.

Changes:

• Adds shared range, suggestion, and week-check abstractions.
• Updates parity tests, fixtures, and goldens.
• Adds IL-based enforcement against direct API usage.

File summaries

File	Summary
tests/SSW.TimePro.Cli.Tests/Features/Timesheets/TimesheetLookupReadTests.cs	Tests shared lookup behavior.
tests/SSW.TimePro.Cli.Integration/Mcp/ToolIlScanner.cs	Scans MCP tool IL for direct API calls. Moderate finding (3 votes): unowned private helper callers may be ignored.
tests/SSW.TimePro.Cli.Integration/Mcp/NorthwindApi.cs	Provides MCP test API fixtures.
tests/SSW.TimePro.Cli.Integration/Mcp/McpToolCatalog.cs	Adds the weekend test case. Moderate finding (1 vote): the payload uses a non-Saturday date and may not validate date association.
tests/SSW.TimePro.Cli.Integration/Mcp/McpCliParityTable.cs	Defines parity cases and API allowlists.
tests/SSW.TimePro.Cli.Integration/Mcp/CliMcpParityTests.cs	Enforces CLI/MCP parity and delegation architecture.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/GetTimesheets.weekendDate.json	Adds weekend tool output coverage.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetTimesheets.16.mcp.json	Adds MCP weekend parity output.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetTimesheets.16.cli.json	Adds CLI weekend parity output.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetTimesheets.15.mcp.json	Adds MCP weekday parity output.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetTimesheets.15.cli.json	Adds CLI weekday parity output.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetSuggestedTimesheets.17.mcp.json	Adds MCP suggestion parity output.
tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetSuggestedTimesheets.17.cli.json	Adds CLI suggestion parity output.
src/SSW.TimePro.Cli/Features/Timesheets/WeekCheckResult.cs	Defines the shared week-check document.
src/SSW.TimePro.Cli/Features/Timesheets/TimesheetLookup.cs	Implements shared range and suggestion reads.
src/SSW.TimePro.Cli/Features/Timesheets/SuggestCommand.cs	Uses the shared suggestion lookup.
src/SSW.TimePro.Cli/Features/Timesheets/GetCommand.cs	Uses the shared range lookup.
src/SSW.TimePro.Cli/Features/Timesheets/CheckCommand.cs	Uses the shared week-check result.
src/SSW.TimePro.Cli/Features/Mcp/Tools/TimesheetMcpTools.cs	Delegates MCP reads to shared logic.
AGENTS.md	Documents shared-read conventions.

Review details

Suppressed comments (1)

tests/SSW.TimePro.Cli.Integration/Mcp/McpToolCatalog.cs:41

• This case uses the baseline NorthwindApi.Day() response, whose TimesheetItem.Date and times are hard-coded to 2026-03-16. The new Saturday CLI golden therefore places Monday entries inside a 2026-03-21 day, so it does not exercise a realistic weekend row and can hide date-association regressions. Arrange a Saturday-specific payload and assert the returned entry date matches WeekendDate.

        // A Saturday: the tool's weekend policy answers with an empty array and never calls the API.
        new("GetTimesheets", "weekendDate",
            (h, ct) => h.Timesheets.GetTimesheets(NorthwindApi.WeekendDate, ct: ct))
        {
            HasApiErrorCase = false
        },

• Files reviewed: 20/20 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.