From 8c94bb51d03b8d90784ff3421a8b9ebdb643072b Mon Sep 17 00:00:00 2001 From: William Valentin Date: Sun, 15 Feb 2026 22:51:20 -0800 Subject: [PATCH] audit follow-up: continue warning burn-down in factory and service tests --- .../2026-02-16-codebase-audit-report.md | 12 ++++++---- docs/plans/state.json | 6 +++-- src/daemon/clientFactory.test.ts | 24 ++++++++++--------- src/gateway/handlers/services.test.ts | 22 +++++++++++++---- 4 files changed, 41 insertions(+), 23 deletions(-) diff --git a/docs/plans/analysis/2026-02-16-codebase-audit-report.md b/docs/plans/analysis/2026-02-16-codebase-audit-report.md index 0342602..2e08c06 100644 --- a/docs/plans/analysis/2026-02-16-codebase-audit-report.md +++ b/docs/plans/analysis/2026-02-16-codebase-audit-report.md @@ -20,7 +20,7 @@ Scope: Production-risk-first audit of bugs, code improvements, and feature oppor - ✅ F-015 addressed: retry defaults no longer classify timeout-style failures as non-retryable, improving resilience for transient timeout conditions. - ✅ F-011 addressed: Slack user-name resolution now uses bounded TTL+LRU caching to prevent unbounded growth. - ✅ F-013 addressed: shared channel utilities now cover reset normalization/building plus reusable allowlist, mention-gating, and pairing-gating flows across Discord/Slack/WhatsApp adapters. -- ◑ F-004 partially addressed: lint error baseline is restored (`pnpm lint` now passes with 0 errors) and warning burn-down has progressed from `466` to `247`; additional warning debt remains. +- ◑ F-004 partially addressed: lint error baseline is restored (`pnpm lint` now passes with 0 errors) and warning burn-down has progressed from `466` to `232`; additional warning debt remains. ## Executive Summary @@ -28,7 +28,7 @@ Current health snapshot: - `pnpm typecheck`: passing - `pnpm build`: passing - `pnpm test:run`: passing (`140/140` files, `1773/1773` tests) -- `pnpm lint`: passing with warnings only (`0 errors`, `247 warnings`) +- `pnpm lint`: passing with warnings only (`0 errors`, `232 warnings`) Top conclusions: - A critical Web UI security issue exists in markdown rendering (unsanitized HTML insertion). @@ -126,7 +126,7 @@ Remediation update (2026-02-16): - Severity: Medium - Impact: CI noise, reduced confidence in static analysis, and slower defect detection. - Evidence: - - `pnpm -s lint` => `0 errors`, `247 warnings` + - `pnpm -s lint` => `0 errors`, `232 warnings` - Error concentration: - `src/daemon/models.ts` (90 errors) - `src/cli/tui.ts` (25 errors) @@ -145,7 +145,7 @@ Remediation update (2026-02-16): Remediation update (2026-02-16): - Stage 1 complete: fixed all error-level ESLint violations in impacted high-error files so `pnpm lint` now passes with `0` errors. -- Stage 2 in progress: warning-burn-down reduced to `247` warnings via targeted hotspot cleanup in: +- Stage 2 in progress: warning-burn-down reduced to `232` warnings via targeted hotspot cleanup in: - `src/gateway/handlers/handlers.test.ts` - `src/daemon/routing.test.ts` - `src/frontends/tui/minimal.test.ts` @@ -154,6 +154,8 @@ Remediation update (2026-02-16): - `src/automation/cron.test.ts` - `src/automation/heartbeat.test.ts` - `src/frontends/tui/minimal.login.test.ts` + - `src/daemon/clientFactory.test.ts` + - `src/gateway/handlers/services.test.ts` ### F-005 Medium: ESLint browser globals mismatch causes avoidable UI lint failures @@ -458,7 +460,7 @@ pnpm -s lint Observed outcomes: - Typecheck/build/test: passing. -- Lint: passing with warnings only (`0` errors, `247` warnings). +- Lint: passing with warnings only (`0` errors, `232` warnings). Historical pre-remediation lint error concentration snapshot: - `src/daemon/models.ts`: 90 errors diff --git a/docs/plans/state.json b/docs/plans/state.json index afe2e4f..4c1b681 100644 --- a/docs/plans/state.json +++ b/docs/plans/state.json @@ -2652,7 +2652,7 @@ "status": "in_progress", "date": "2026-02-16", "updated": "2026-02-16", - "summary": "Continued stage-2 lint warning reduction with hotspot-focused cleanup in `gateway/handlers/handlers.test.ts`, `daemon/routing.test.ts`, `frontends/tui/minimal.test.ts`, `gateway/tailscale.test.ts`, `automation/webhooks.test.ts`, `automation/cron.test.ts`, `automation/heartbeat.test.ts`, and `frontends/tui/minimal.login.test.ts`. Replaced broad `any` casts with typed helper casts/unknown-path accessors and removed non-null assertions in high-warning tests. Warning count reduced from 466 to 247 (219 warnings burned down) with lint/test suites still green.", + "summary": "Continued stage-2 lint warning reduction with hotspot-focused cleanup in `gateway/handlers/handlers.test.ts`, `daemon/routing.test.ts`, `frontends/tui/minimal.test.ts`, `gateway/tailscale.test.ts`, `automation/webhooks.test.ts`, `automation/cron.test.ts`, `automation/heartbeat.test.ts`, `frontends/tui/minimal.login.test.ts`, `daemon/clientFactory.test.ts`, and `gateway/handlers/services.test.ts`. Replaced broad `any` casts with typed helper casts/unknown-path accessors and removed non-null assertions in high-warning tests. Warning count reduced from 466 to 232 (234 warnings burned down) with lint/test suites still green.", "files_modified": [ "src/tools/builtin/browser/tools.test.ts", "src/channels/telegram/adapter.test.ts", @@ -2667,9 +2667,11 @@ "src/automation/cron.test.ts", "src/automation/heartbeat.test.ts", "src/frontends/tui/minimal.login.test.ts", + "src/daemon/clientFactory.test.ts", + "src/gateway/handlers/services.test.ts", "docs/plans/analysis/2026-02-16-codebase-audit-report.md" ], - "test_status": "pnpm test:run src/channels/utils.test.ts src/channels/discord/adapter.test.ts src/channels/slack/adapter.test.ts src/channels/whatsapp/adapter.test.ts src/daemon/routing.test.ts src/gateway/handlers/handlers.test.ts src/frontends/tui/minimal.test.ts src/gateway/tailscale.test.ts src/automation/webhooks.test.ts src/automation/cron.test.ts src/automation/heartbeat.test.ts src/frontends/tui/minimal.login.test.ts + pnpm lint passing (0 errors, 247 warnings)" + "test_status": "pnpm test:run src/channels/utils.test.ts src/channels/discord/adapter.test.ts src/channels/slack/adapter.test.ts src/channels/whatsapp/adapter.test.ts src/daemon/routing.test.ts src/gateway/handlers/handlers.test.ts src/frontends/tui/minimal.test.ts src/gateway/tailscale.test.ts src/automation/webhooks.test.ts src/automation/cron.test.ts src/automation/heartbeat.test.ts src/frontends/tui/minimal.login.test.ts src/daemon/clientFactory.test.ts src/gateway/handlers/services.test.ts + pnpm lint passing (0 errors, 232 warnings)" } }, "overall_progress": { diff --git a/src/daemon/clientFactory.test.ts b/src/daemon/clientFactory.test.ts index fbf483d..34e6d1e 100644 --- a/src/daemon/clientFactory.test.ts +++ b/src/daemon/clientFactory.test.ts @@ -3,18 +3,20 @@ import { tmpdir } from 'os'; import { join } from 'path'; import { describe, expect, it, vi } from 'vitest'; -import { AnthropicClient } from '../models/anthropic.js'; -import { OpenAIClient } from '../models/openai.js'; -import { OllamaClient } from '../models/local/ollama.js'; -import { LlamaCppClient } from '../models/local/llamacpp.js'; -import { GeminiClient } from '../models/gemini.js'; -import { BedrockClient } from '../models/bedrock.js'; -import { GitHubModelsClient } from '../models/github.js'; - async function loadFactory(): Promise { return import('./index.js'); } +function getUseOAuth(client: unknown): boolean | undefined { + const obj = client as { useOAuth?: boolean }; + return obj.useOAuth; +} + +function getConstructorName(value: unknown): string { + const obj = value as { constructor?: { name?: string } }; + return obj.constructor?.name ?? ''; +} + describe('createClientFromConfig', () => { it('creates AnthropicClient for anthropic provider', async () => { const { createClientFromConfig } = await loadFactory(); @@ -283,7 +285,7 @@ describe('createClientFromConfig', () => { api_key: 'sk-test', }); expect(client.constructor.name).toBe('OpenAIClient'); - expect((client as any).useOAuth).toBe(false); + expect(getUseOAuth(client)).toBe(false); }); it('auth_mode oauth selects Anthropic auth_token without requiring api_key', async () => { @@ -338,7 +340,7 @@ describe('createClientFromConfig', () => { auth_mode: 'oauth', }); expect(client.constructor.name).toBe('OpenAIClient'); - expect((client as any).useOAuth).toBe(true); + expect(getUseOAuth(client)).toBe(true); } finally { process.env.HOME = originalHome; } @@ -407,7 +409,7 @@ describe('createAutoFallbackClient', () => { model: 'claude-sonnet-4-20250514', }); expect(client).toBeDefined(); - expect((client as any).constructor.name).toBe('GitHubModelsClient'); + expect(getConstructorName(client)).toBe('GitHubModelsClient'); }); it('returns undefined for non-anthropic providers', async () => { diff --git a/src/gateway/handlers/services.test.ts b/src/gateway/handlers/services.test.ts index 2451215..190fa0c 100644 --- a/src/gateway/handlers/services.test.ts +++ b/src/gateway/handlers/services.test.ts @@ -2,6 +2,11 @@ import { describe, it, expect } from 'vitest'; import { discoverServices } from './services.js'; import { ChannelRegistry } from '../../channels/registry.js'; import type { Config } from '../../config/index.js'; +import type { CronJobConfig } from '../../config/schema.js'; + +function withMutableConfig(config: Config): Config & Record { + return config as Config & Record; +} function makeBaseConfig(): Config { return { @@ -47,7 +52,7 @@ describe('discoverServices', () => { it('marks configured channels as disconnected when adapter is not registered', () => { const cfg = makeBaseConfig(); - (cfg as any).telegram = { bot_token: 'x', allowed_chat_ids: [123] }; + withMutableConfig(cfg).telegram = { bot_token: 'x', allowed_chat_ids: [123] }; const reg = new ChannelRegistry(); const services = discoverServices(cfg, reg); @@ -57,7 +62,7 @@ describe('discoverServices', () => { it('uses adapter status when channel adapter is registered', () => { const cfg = makeBaseConfig(); - (cfg as any).telegram = { bot_token: 'x', allowed_chat_ids: [123] }; + withMutableConfig(cfg).telegram = { bot_token: 'x', allowed_chat_ids: [123] }; const reg = new ChannelRegistry(); reg.register({ @@ -77,8 +82,8 @@ describe('discoverServices', () => { const cfg = makeBaseConfig(); cfg.automation.cron = [ { name: 'job', schedule: '0 0 * * *', message: 'hi', output: { channel: 'webchat', peer: 'x' }, enabled: true }, - ] as any; - cfg.mcp.servers = [{ name: 'srv', command: 'x', args: [] }] as any; + ] as CronJobConfig[]; + cfg.mcp.servers = [{ name: 'srv', command: 'x', args: [] }]; const reg = new ChannelRegistry(); const services = discoverServices(cfg, reg); @@ -90,7 +95,14 @@ describe('discoverServices', () => { it('marks audio transcription as configured and includes endpoint metadata', () => { const cfg = makeBaseConfig(); - (cfg as any).audio = { enabled: true, provider: { type: 'custom', endpoint: 'http://localhost:18801/v1/audio/transcriptions', model: 'whisper-1' } }; + withMutableConfig(cfg).audio = { + enabled: true, + provider: { + type: 'custom', + endpoint: 'http://localhost:18801/v1/audio/transcriptions', + model: 'whisper-1', + }, + }; const reg = new ChannelRegistry(); const services = discoverServices(cfg, reg);