feat/batu-mcp

401-retry path clears tokenState but not mintingPromise

packages/mcp-server/src/client.ts:126

On a 401, this.tokenState = null is set before the retry but this.mintingPromise is not cleared. Safe today (stdio is serial; the prior mint's .finally() has already run by the time a 401 response arrives). If the client is used concurrently in the future, a stale mintingPromise could be reused on the retry path. Add this.mintingPromise = null alongside this.tokenState = null for defensive consistency.

BATU_API_URL accepted without HTTPS scheme validation

packages/mcp-server/src/client.ts:54

The constructor uses BATU_API_URL verbatim with no enforcement that the scheme is https://. An http:// address sends the API key in plaintext. A one-line guard (throw if the resolved URL does not start with https:// unless localhost) eliminates this credential-exposure class entirely.

BatuApiError.message reflected to model without length bound

packages/mcp-server/src/results.ts:20

e.message from the API response is passed verbatim to the model. A compromised or misconfigured API endpoint could embed prompt-injection payloads. Low risk when pointing at prod, but a length cap (e.g. 500 chars) and stripping of non-printable characters would be a cheap defence-in-depth.

Webhook signing secret flows into model context verbatim

packages/mcp-server/src/index.ts:318

batu_create_webhook surfaces the whsec_... signing secret through jsonResult into the model's context window, which may be logged by MCP hosts. Intentional by design (the operator needs the secret), but a README note warning that MCP session logs may contain signing secrets would help operators handle those logs appropriately.

In-code Claude Desktop example still floats @batu/mcp-server without version pin

packages/mcp-server/README.md

The README now recommends pinning @0.1.0, but any in-code documentation (e.g. tool descriptions) showing the bare package name without a version would send users to the floating latest. Confirm all installation examples consistently show the pinned form.

status/payment_status/source narrowed from multi-value CSV to single z.enum

packages/mcp-server/src/index.ts:62

The validator uses parseEnumCsv for status, payment_status, and source — all accept comma-separated multi-values (e.g. 'pending,validated'). The MCP exposes them as z.enum (single value). An agent cannot filter 'pending OR overdue' in one call. Fix: use z.string() with description 'comma-separated: pending,validated'.

run() has zero test coverage

packages/mcp-server/src/results.test.ts

results.ts exports jsonResult, errorResult, and run, but results.test.ts only covers the first two. run() is the hot path for all 33 tool handlers — it awaits fn() and routes success/throws. A bug where run() re-throws instead of catching would be invisible. Two tests (resolving fn and rejecting fn) cover the composition.

mintingPromise concurrent deduplication not tested

packages/mcp-server/src/client.test.ts

The mintingPromise ??= .finally() pattern serializes concurrent token mints. No test fires two parallel request() calls and asserts only one /auth/token call was made. The existing fetch-stub infrastructure in client.test.ts supports this: queue [tokenOk(), res(200,...), res(200,...)] then Promise.all two requests and count mint calls (expect 1).

Smoke test triggers full TypeScript build on every pnpm test run

packages/mcp-server/src/tools-list.smoke.test.ts:20

beforeAll(() => execSync('pnpm run build', ...), 120_000) runs tsc synchronously in every test run, adding 10-60s to CI. Concurrent test workers could also race on dist/. The build step belongs in a dedicated script (e.g. test:smoke) invoked explicitly, not inside beforeAll.

MIN_TOKEN_TTL_S floor and missing expires_in fallback not tested

packages/mcp-server/src/client.test.ts

Only expires_in=3600 is tested. Two edge cases are unguarded: (1) expires_in < TOKEN_REFRESH_MARGIN_S (e.g. 30s) should floor to MIN_TOKEN_TTL_S=5, not produce a negative TTL causing per-request re-minting; (2) expires_in absent should fall back to 3600. If MIN_TOKEN_TTL_S were deleted the regression would be invisible.

Smoke test asserts description string for limit, not the JSON Schema bound

packages/mcp-server/src/tools-list.smoke.test.ts:76

The test checks limit?.description matches /max 100/ as a proxy for the ceiling. But description is free text — changing .max(100) to .max(50) while keeping the description passes the test. The MCP SDK exposes JSON Schema numeric constraints in inputSchema.properties. Assert bills.limit?.maximum === 5000 and files.limit?.maximum === 100 to pin the actual enforcement.

Tool count assertion >= 33 does not catch accidental removals

packages/mcp-server/src/tools-list.smoke.test.ts:62

There are exactly 33 registered tools. expect(tools.length).toBeGreaterThanOrEqual(33) passes even if 29 of the non-enumerated tools are removed. Use .toBe(33) to catch accidental deletions and require explicit update when adding tools.

errorResult tests don't assert console.error was called

packages/mcp-server/src/results.test.ts:22

Both error path tests spy on console.error but don't assert it was invoked. If the console.error call were removed, operators would lose diagnostics but the tests would still pass. Add expect(console.error).toHaveBeenCalledOnce() to the non-BatuApiError test cases.

Smoke test child process not cleaned up on non-timeout failures

packages/mcp-server/src/tools-list.smoke.test.ts:24

child.kill() is only called on the timeout path. If the build artifact crashes on boot or stdout emits an error event, the promise hangs for 15s rather than failing fast. Add child.on('error', rej) and pipe stderr (not 'ignore') for faster diagnostics.

Inconsistent optional-field spreading: body uses conditional spread, query passes undefined directly

packages/mcp-server/src/index.ts:252

batu_collect_bills body uses ...(args.site_name ? { site_name: args.site_name } : {}) while batu_list_bills query passes undefined values directly (stripped by the request method). Both work but use different idioms. Harmonising to one approach (pass + strip is cleaner) would reduce future maintenance friction.

batu_list_files limit cap (100) not linked to validator source of truth

packages/mcp-server/src/index.ts:167

The mcp-server.md rule requires tool bounds to match the validator. The 100 cap matches parseListFilesQuery but there's no inline comment linking them, unlike batu_list_bills which has clear traceability. A brief comment referencing public-v1-validation.ts parseListFilesQuery would make the bound auditable.

granularity on batu_get_energy_summary is z.enum(['none']) — dead single-value enum

packages/mcp-server/src/index.ts:376

A single-value optional enum adds no information for the model — it can only ever send 'none' or omit the field. Either remove the field or add a comment explaining it as a forward-compat placeholder for future granularity options.

Conventional: batu_list_bills omits multi-value filters for tariff/site_customer_id/pricing_zone

packages/mcp-server/src/index.ts:52

parseBillsListQuery accepts tariff, site_customer_id, pricing_zone, and utility_service as free-text CSV filters. None are exposed. The mcp-server.md rule permits omitting niche catalog filters, but a comment in the tool marking these as intentionally omitted would make the curation decision auditable.