MCP: lint and type-check tests and scripts like the web app

The MCP server had no ESLint, and since its tests left the tsconfig (to keep
them out of dist/) nothing type-checked them; scripts/ never was. tsconfig.json
now type-checks sources, tests and scripts (noEmit); tsconfig.build.json emits
the same dist/ as before (byte-identical) for npm run build and the Docker
image. ESLint (the web's versions: eslint 9.39.5, typescript-eslint 8.65.0)
runs the recommended rules plus the typed promise rules, and CI's mcp job runs
it with --max-warnings 0.

Findings fixed: 16 type errors in tests (a fixture missing three wire fields,
unknown catch results, untyped mocks); gracefulShutdown was async without
awaiting anything, so the signal handlers returned a promise; two dead imports;
a Function cast replaced by the exact pipeline() signature; an unused test
variable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Oliver Kieselbach
2026-10-10 16:42:13 +02:00
parent 224eb668a1
commit 69d876b0a6
20 changed files with 1408 additions and 32 deletions
+7
View File
@@ -182,9 +182,16 @@ jobs:
- run: npm ci
# tsconfig.json covers sources, tests and scripts; the build emits via
# tsconfig.build.json.
- name: Typecheck
run: npx tsc --noEmit
# Same rule as the web job: any warning fails — fix new findings, never
# raise the cap.
- name: Lint
run: npm run lint -- --max-warnings 0
# The vector-search tests tokenize with the real all-MiniLM-L6-v2 model
# (~23 MB tokenizer + ONNX weights). transformers.js caches under
# node_modules by default, which npm ci recreates — so every CI run
@@ -7,7 +7,7 @@ WORKDIR /app
COPY src/McpServer/autopilot-monitor-mcp/package.json src/McpServer/autopilot-monitor-mcp/package-lock.json ./
RUN npm ci --ignore-scripts
COPY src/McpServer/autopilot-monitor-mcp/tsconfig.json ./
COPY src/McpServer/autopilot-monitor-mcp/tsconfig.json src/McpServer/autopilot-monitor-mcp/tsconfig.build.json ./
COPY src/McpServer/autopilot-monitor-mcp/src/ ./src/
RUN npm run build
@@ -0,0 +1,41 @@
// Flat ESLint config, run via `npm run lint`. CI runs it with --max-warnings 0
// (see .github/workflows/ci.yml), so any new warning fails the mcp job.
// Fix findings; never raise the cap.
import js from '@eslint/js';
import tseslint from 'typescript-eslint';
const config = [
{
ignores: ['dist/**', 'node_modules/**', 'src/generated/**', 'src/*.generated.ts'],
},
js.configs.recommended,
...tseslint.configs.recommended,
{
rules: {
'@typescript-eslint/no-unused-vars': [
'error',
{ argsIgnorePattern: '^_', varsIgnorePattern: '^_', ignoreRestSiblings: true },
],
},
},
{
// An unhandled rejection ends the server process, so the promise rules run with type information.
files: ['src/**/*.ts', 'scripts/**/*.ts', 'vitest.config.ts'],
languageOptions: {
parserOptions: { projectService: true, tsconfigRootDir: import.meta.dirname },
},
rules: {
'@typescript-eslint/no-floating-promises': 'error',
'@typescript-eslint/no-misused-promises': 'error',
},
},
{
// Tests cast mocks and untyped backend JSON (the live tools-* suites); `any` stays allowed there.
files: ['src/**/*.test.ts', 'src/__tests__/**/*.ts'],
rules: {
'@typescript-eslint/no-explicit-any': 'off',
},
},
];
export default config;
File diff suppressed because it is too large Load Diff
@@ -10,11 +10,12 @@
"autopilot-monitor-mcp": "./dist/index.js"
},
"scripts": {
"build": "tsc",
"build": "tsc -p tsconfig.build.json",
"start": "node dist/index.js",
"dev": "tsx src/index.ts",
"docker:build": "docker build -f Dockerfile -t autopilotmonitor-mcp:local ../../../",
"docker:run": "docker run --rm -p 8080:8080 -e AUTOPILOT_API_URL=https://autopilotmonitor-api-eu.azurewebsites.net autopilotmonitor-mcp:local",
"lint": "eslint .",
"test": "vitest run",
"test:watch": "vitest"
},
@@ -32,12 +33,15 @@
"zod": "^4.6.5"
},
"devDependencies": {
"@eslint/js": "^9.39.5",
"@modelcontextprotocol/client": "^2.2.0",
"@types/compression": "^1.8.1",
"@types/express": "^5.0.6",
"@types/node": "^22.20.4",
"eslint": "^9.39.5",
"tsx": "^4.23.15",
"typescript": "^5.0.0",
"typescript-eslint": "^8.65.0",
"vitest": "^4.1.11"
},
"overrides": {
@@ -45,7 +45,7 @@ describe('apiFetch automatic retry', () => {
]);
const context = createToolCallContext('get_sessions', 'cid-tool');
const data = await asCaller(() => runWithToolCallContext(context, () => apiFetch<{ items: number[] }>('/api/sessions')));
const data = await asCaller(async () => runWithToolCallContext(context, () => apiFetch<{ items: number[] }>('/api/sessions')));
expect(data).toEqual({ items: [1] });
expect(calls).toHaveLength(2);
@@ -67,12 +67,12 @@ describe('apiFetch automatic retry', () => {
it('does not retry when Retry-After exceeds the cap, when the header is missing, or on other statuses', async () => {
const tooLong = stubFetchSequence([{ status: 429, body: envelope429, headers: { 'retry-after': String(MAX_AUTO_RETRY_AFTER_SECONDS + 1) } }]);
const e1 = await asCaller(() => apiFetch<unknown>('/api/x')).catch((e) => e as ApiError);
const e1 = await asCaller(() => apiFetch<never>('/api/x')).catch((e) => e as ApiError);
expect(tooLong).toHaveLength(1);
expect(e1.retryAfterSeconds).toBe(MAX_AUTO_RETRY_AFTER_SECONDS + 1);
const noHeader = stubFetchSequence([{ status: 429, body: envelope429 }]);
const e2 = await asCaller(() => apiFetch<unknown>('/api/x')).catch((e) => e as ApiError);
const e2 = await asCaller(() => apiFetch<never>('/api/x')).catch((e) => e as ApiError);
expect(noHeader).toHaveLength(1);
expect(e2.retryAfterSeconds).toBeNull();
@@ -107,18 +107,18 @@ describe('apiFetch automatic retry', () => {
describe('apiFetch body contract', () => {
it('refuses an empty or malformed 2xx body as ApiError instead of returning undefined', async () => {
stubFetchSequence([{ status: 200, text: '' }]);
const empty = await asCaller(() => apiFetch<unknown>('/api/x')).catch((e) => e as ApiError);
const empty = await asCaller(() => apiFetch<never>('/api/x')).catch((e) => e as ApiError);
expect(empty).toBeInstanceOf(ApiError);
expect(empty.body).toBe('Empty response body');
stubFetchSequence([{ status: 200, text: '<html>' }]);
const html = await asCaller(() => apiFetch<unknown>('/api/x')).catch((e) => e as ApiError);
const html = await asCaller(() => apiFetch<never>('/api/x')).catch((e) => e as ApiError);
expect(html.body).toBe('Malformed response body');
});
it('exposes the typed envelope fields on ApiError', async () => {
stubFetchSequence([{ status: 404, body: { error: 'Session not found.', code: 'NotFound', correlationId: 'body-cid', hint: 'Check the id.' } }]);
const err = await asCaller(() => apiFetch<unknown>('/api/x')).catch((e) => e as ApiError);
const err = await asCaller(() => apiFetch<never>('/api/x')).catch((e) => e as ApiError);
expect(err.parsed?.code).toBe('NotFound');
expect(err.parsed?.hint).toBe('Check the id.');
expect(err.correlationId).toBe('body-cid');
@@ -97,7 +97,7 @@ describe('collectDeploymentState', () => {
it('an unreachable live endpoint is unknown with the failure listed', async () => {
const fetchMock = stubFetch(ALL_IN_SYNC);
fetchMock.mockImplementationOnce(async () => { throw new Error('fetch failed'); });
fetchMock.mockRejectedValueOnce(new Error('fetch failed'));
const state = await collectDeploymentState(fetchMock as never);
@@ -145,7 +145,7 @@ describe('diag_zip_layout catalog', () => {
const appWorkload = DIAG_ZIP_MAP.files.find((f) => f.path.includes('AppWorkload'));
expect(appWorkload).toBeDefined();
expect(appWorkload!.read).toBe('grep-only');
expect(appWorkload!.warning).toMatch(/MB/);
expect(appWorkload).toHaveProperty('warning', expect.stringMatching(/MB/));
});
it('puts final-status.json at priority 1', () => {
@@ -198,7 +198,7 @@ describe('loadDocsCorpus', () => {
);
const headings = chunks.map((c) => c.metadata.heading);
expect(headings).toContain('General › Does keep-awake work during ESP?');
expect(headings.every((h) => h == null || (!h.includes('<') && !h.includes('>')))).toBe(true);
expect(headings.every((h) => typeof h !== 'string' || (!h.includes('<') && !h.includes('>')))).toBe(true);
});
it('never emits a heading from inside a fenced code block', async () => {
@@ -5,7 +5,7 @@
* a non-GA tool, or a non-tool request must do neither. Pure unit level: the caller
* context is set via runWithCaller, fetch is stubbed.
*/
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { describe, it, expect, vi, beforeEach, afterEach, type MockInstance } from 'vitest';
import { runWithCaller } from '../client.js';
import { attemptedToolName, isGaToolProbe, observeGaToolProbe, ACCESS_PROBE_PATH } from '../ga-tool-probe.js';
import { GA_STRICT_TOOL_NAMES } from '../tools/admin.js';
@@ -13,7 +13,7 @@ import { GA_STRICT_TOOL_NAMES } from '../tools/admin.js';
const call = (name: string) => ({ jsonrpc: '2.0', id: 1, method: 'tools/call', params: { name, arguments: {} } });
let fetchMock: ReturnType<typeof vi.fn>;
let errorSpy: ReturnType<typeof vi.spyOn>;
let errorSpy: MockInstance<typeof console.error>;
beforeEach(() => {
fetchMock = vi.fn(async () => new Response('{"error":"Forbidden"}', { status: 403 }));
@@ -95,7 +95,7 @@ describe('observeGaToolProbe', () => {
});
it('never throws when the probe request fails', async () => {
fetchMock.mockImplementation(async () => { throw new Error('network down'); });
fetchMock.mockRejectedValue(new Error('network down'));
expect(() => runWithCaller({ token: 'tok', isGlobalAdmin: false }, () => observeGaToolProbe(call('list_tables')))).not.toThrow();
await flush();
});
@@ -257,10 +257,11 @@ describe('sealed refresh tokens of a tenant client', () => {
it('ends a sealed token once the registration is deleted, with or without client_id', async () => {
const sealed = await sealedToken();
for (const form of [
const forms: Record<string, string>[] = [
{ grant_type: 'refresh_token', refresh_token: sealed },
{ grant_type: 'refresh_token', refresh_token: sealed, client_id: CLIENT_ID },
]) {
];
for (const form of forms) {
const outbound = stubOutbound({ lookup: 'missing' });
const r = await token(form);
expect(r.status).toBe(400);
@@ -3,11 +3,10 @@
* Tests against the live backend — requires AUTOPILOT_API_TOKEN env var.
*/
import { describe, it, expect, beforeAll } from 'vitest';
import { apiFetch, buildQuery, getToken } from './helpers.js';
import { apiFetch, getToken } from './helpers.js';
// Shared state discovered during tests
let knownSessionId: string;
let knownTenantId: string;
// Integration suite: only runs when a backend token is supplied. Without one it
// SKIPS (not errors), so an unattended CI run still goes green on the unit
@@ -29,7 +28,6 @@ suite('search_sessions', () => {
// Store for subsequent tests
knownSessionId = data.sessions[0].sessionId;
knownTenantId = data.sessions[0].tenantId;
// Validate session shape
const s = data.sessions[0];
@@ -7,6 +7,9 @@ function row(verdictPath: string, count: number, sharePct: number, extra: Partia
status: 'Succeeded',
count,
sharePct,
derivedCount: 0,
eligible7d: 0,
reEnrolled7d: 0,
overriddenByAdmin: 0,
overriddenByLateCompletion: 0,
overriddenOther: 0,
@@ -111,7 +111,7 @@ export function parseFrontmatter(input: string): { frontmatter: DocFrontmatter;
const match = /^([A-Za-z_][\w-]*):\s*(.*)$/.exec(lines[i]);
if (!match) continue;
const key = match[1];
let value = match[2].trim();
const value = match[2].trim();
// Folded / literal block scalar: consume the following indented lines.
if (value === '>-' || value === '>' || value === '|' || value === '|-') {
@@ -391,7 +391,7 @@ const server = app.listen(PORT, '0.0.0.0', () => {
// --- Graceful shutdown ---
async function gracefulShutdown(signal: string) {
function gracefulShutdown(signal: string) {
console.error(`[mcp] Received ${signal}, shutting down gracefully…`);
// Stateless mode: no long-lived transports to close. Just stop accepting
// new connections and let in-flight requests drain via their own res.on('close').
@@ -1,6 +1,6 @@
import { McpServer } from '@modelcontextprotocol/server';
import { z } from 'zod';
import { apiFetch, buildQuery, DEFAULT_FIRST_PAGE_SIZE, effectivePageSize, enforceDelegatedTenant, enforceDelegatedTenantForPage, followNextLink, getCallerUpnDomain, getDelegatedTenantIds, getHomeTenantId, pageSizeForCall, pickGlobalOrTenantPath, scanUntilMatch, scanWithTimeoutFallback, jsonBody } from '../client.js';
import { apiFetch, buildQuery, DEFAULT_FIRST_PAGE_SIZE, effectivePageSize, enforceDelegatedTenant, enforceDelegatedTenantForPage, followNextLink, getCallerUpnDomain, getDelegatedTenantIds, getHomeTenantId, pageSizeForCall, pickGlobalOrTenantPath, scanWithTimeoutFallback, jsonBody } from '../client.js';
import { withToolTelemetry } from '../telemetry.js';
import { getResourceContent, withEventTypeNote, RESOURCE_NAMES } from '../resource-catalog.js';
import { READ_ONLY, READ_ONLY_OPEN, MUTATING, MAX_RESULT_SIZE_CHARS, LEAN_RAW_EVENT_FIELDS, LEAN_RAW_EVENT_OMISSION, leanFieldSelection, toolResultText, SessionIdSchema, TenantGuidSchema, tenantIdDescription, pageSizeDescription, CONTINUATION_DESCRIPTION, daysDescription } from './shared.js';
@@ -1,6 +1,6 @@
import { McpServer } from '@modelcontextprotocol/server';
import { z } from 'zod';
import { ApiError, apiFetch, buildQuery, jsonBody, DEFAULT_FIRST_PAGE_SIZE, effectivePageSize, enforceDelegatedTenant, enforceDelegatedTenantForPage, followNextLink, pageSizeForCall, pickGlobalOrTenantPath, scanUntilMatch, scanWithTimeoutFallback } from '../client.js';
import { ApiError, apiFetch, buildQuery, jsonBody, DEFAULT_FIRST_PAGE_SIZE, effectivePageSize, enforceDelegatedTenant, enforceDelegatedTenantForPage, followNextLink, pageSizeForCall, pickGlobalOrTenantPath, scanWithTimeoutFallback } from '../client.js';
import { withToolTelemetry } from '../telemetry.js';
import { READ_ONLY, MAX_RESULT_SIZE_CHARS, LEAN_EVENT_FIELDS, LEAN_EVENT_OMISSION, leanFieldSelection, SUMMARY_EVENT_FIELDS, toolResultText, SessionIdSchema, isBenignHealthDetectionReport, foreignSuccessRunIds, isCorrectedForeignFailure, tenantIdDescription, pageSizeDescription, CONTINUATION_DESCRIPTION, daysDescription } from './shared.js';
import { toolError } from './error-handler.js';
@@ -20,8 +20,14 @@ export const MODEL_NAME = 'Xenova/all-MiniLM-L6-v2';
// 0.5 Gi container). A rejected load is cleared so the next call can retry.
let embedderPromise: Promise<FeatureExtractionPipeline> | null = null;
// The one pipeline() call this file makes. Calling the overloaded signature directly hits TS2590.
type FeatureExtractionFactory = (
task: 'feature-extraction',
model: string,
options: { dtype: 'q8' },
) => Promise<FeatureExtractionPipeline>;
async function createEmbedder(): Promise<FeatureExtractionPipeline> {
// Dynamic import + cast to avoid TS2590 from the overloaded pipeline() signature
const { pipeline, env } = await import('@huggingface/transformers');
// The library's default model cache lives inside node_modules/@huggingface/
// transformers/.cache — an npm-layout implementation detail. HF_CACHE_DIR pins
@@ -30,9 +36,7 @@ async function createEmbedder(): Promise<FeatureExtractionPipeline> {
if (process.env.HF_CACHE_DIR) {
env.cacheDir = process.env.HF_CACHE_DIR;
}
return (await (pipeline as Function)('feature-extraction', MODEL_NAME, {
dtype: 'q8',
})) as FeatureExtractionPipeline;
return (pipeline as unknown as FeatureExtractionFactory)('feature-extraction', MODEL_NAME, { dtype: 'q8' });
}
function getEmbedder(): Promise<FeatureExtractionPipeline> {
@@ -0,0 +1,11 @@
{
// Emit config: tests stay out of dist/, otherwise vitest finds and runs them a second time there.
"extends": "./tsconfig.json",
"compilerOptions": {
"noEmit": false,
"rootDir": "./src",
"outDir": "./dist"
},
"include": ["src/**/*"],
"exclude": ["node_modules", "dist", "src/**/*.test.ts", "src/__tests__/**"]
}
@@ -1,14 +1,14 @@
{
// Type-check config (editor, `tsc --noEmit` in CI): sources, tests and scripts. The build emits via tsconfig.build.json.
"compilerOptions": {
"target": "ES2022",
"module": "Node16",
"moduleResolution": "Node16",
"outDir": "./dist",
"rootDir": "./src",
"strict": true,
"esModuleInterop": true,
"skipLibCheck": true
"skipLibCheck": true,
"noEmit": true
},
"include": ["src/**/*"],
"exclude": ["node_modules", "dist", "src/**/*.test.ts", "src/__tests__/**"]
"include": ["src/**/*", "scripts/**/*", "vitest.config.ts"],
"exclude": ["node_modules", "dist"]
}