Skip to content

test: cover tool-handler request-shaping and response-mapping logic - #48

Merged
asachs01 merged 1 commit into
mainfrom
test/tool-handler-coverage
Aug 30, 2026
Merged

test: cover tool-handler request-shaping and response-mapping logic#48
asachs01 merged 1 commit into
mainfrom
test/tool-handler-coverage

Conversation

@asachs01

@asachs01 asachs01 commented Aug 28, 2026

Copy link
Copy Markdown
Member

Summary

  • afkbot-mcp registers 5 MCP tools (create_pto_request, list_pto_requests, get_pto_request, cancel_pto_request, team_calendar) whose surface was wired in src/index.ts and covered by external smoke/mcp-assert checks, but none of the individual handler functions in src/tools/*.ts were ever invoked by a test. The only existing test file (src/client/afkbotClient.test.ts) covers the client-level getBaseUrl/cleanEnv helpers, not the handlers.
  • This repo already uses Node's built-in node --test runner (not Vitest, unlike most of the fleet's other *-mcp repos) — confirmed via package.json's test script and the existing test file's conventions (node:test + node:assert/strict, .test.ts co-located next to source, compiled by tsc into dist/**/*.test.js).
  • Adds one test file per handler, following that existing convention: createPtoRequest.test.ts, listRequests.test.ts, getRequest.test.ts, cancelRequest.test.ts, teamCalendar.test.ts. Each mocks the underlying afkbotClient.ts exports via node:test's mock.module(), invokes the exported handler directly, and asserts both the outbound call shape (e.g. end_date defaulting to start_date, status: "all" becoming undefined, limit defaulting to 25) and the response transformation (JSON-wrapped content payload, team_calendar's summary mapping and empty-state message).
  • mock.module() is experimental and needs --experimental-test-module-mocks; added that flag to the test script. Verified working on both Node 20.20.2 and 22.23.2 (the CI matrix versions) before relying on it.
  • ci.yml previously ran npm ci / npm run lint / npm run build / docker build only — it never actually ran npm test, so the pre-existing client tests weren't enforced either. Added npm test as a CI step so this (and future) coverage is gated, not just present.

Noted, not fixed (scope discipline — flagging, not changing)

  • listRequests.ts: limit: params.limit || 25 will silently override an explicit limit: 0 with 25, since 0 is falsy. Probably harmless in practice (nobody wants zero results) but it's a real || vs ?? bug if anyone ever relies on limit: 0.
  • Test files are compiled into dist/ by tsc (per tsconfig.json's include: ["src/**/*"]) and the Dockerfile copies the whole dist/ directory into the production image, so compiled test files ship in the runtime image. This was already true before this PR (the existing afkbotClient.test.ts already had this property) — not introduced or changed here.

Test plan

  • npm run build (tsc) — clean on Node 20.20.2 and 22.23.2
  • npm run lint (tsc --noEmit) — clean on both
  • npm test — 23/23 passing on both Node 20.20.2 and 22.23.2
  • docker build -t afkbot-mcp:test . — succeeds

Note on repo location

The task that generated this PR assumed afkbot-mcp had migrated to the WYRE-AI GitHub org (per the 2026-08-24/25 org migration covering the other *-mcp repos). Verified this repo has not migrated — WYRE-AI/afkbot-mcp 404s; the live, non-archived, non-fork repo is still wyre-technology/afkbot-mcp. This PR is opened against the actual current location.

🤖 Generated with Claude Code


View with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is enabled.

The MCP tool surface (index.ts) was registered and smoke-tested but each
handler's own logic — default-value shaping of outbound requests and
mapping of client responses into MCP content — was never actually
invoked by a test. Only the client-level getBaseUrl/cleanEnv helpers had
coverage.

Adds one node:test file per tool handler (createPtoRequest, listRequests,
getRequest, cancelRequest, teamCalendar), using node:test's experimental
mock.module() to substitute afkbotClient.ts's exported functions and
invoking each handler directly. Verified working on both Node 20.20.2 and
22.23.2 (the CI matrix) with --experimental-test-module-mocks.

Also wires `npm test` into ci.yml — it previously ran lint/build/docker
build only, so the pre-existing client tests were never enforced by CI
either.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@asachs01
asachs01 merged commit 3c3b2f2 into main Aug 30, 2026
11 checks passed
@asachs01
asachs01 deleted the test/tool-handler-coverage branch August 30, 2026 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant