From 021435ac27ba771d129a6bbf97372535f2c1e350 Mon Sep 17 00:00:00 2001 From: William Valentin Date: Sun, 15 Feb 2026 22:31:47 -0800 Subject: [PATCH] test(lint): reduce warning debt in selected test suites --- .../2026-02-16-codebase-audit-report.md | 8 +-- docs/plans/state.json | 14 +++++ src/channels/telegram/adapter.test.ts | 58 +++++++++---------- src/mcp/manager.test.ts | 19 ++++-- src/tools/builtin/browser/tools.test.ts | 38 +++++++----- src/tools/builtin/system-info.test.ts | 21 ++++--- 6 files changed, 94 insertions(+), 64 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 c47f2d6..caee194 100644 --- a/docs/plans/analysis/2026-02-16-codebase-audit-report.md +++ b/docs/plans/analysis/2026-02-16-codebase-audit-report.md @@ -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`, `539 warnings`) +- `pnpm lint`: passing with warnings only (`0 errors`, `501 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`, `539 warnings` + - `pnpm -s lint` => `0 errors`, `501 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 pending: warning-burn-down remains (currently `539` warnings). +- Stage 2 in progress: warning-burn-down reduced to `501` warnings via targeted low-risk test cleanup (non-null assertion removal). ### F-005 Medium: ESLint browser globals mismatch causes avoidable UI lint failures @@ -449,7 +449,7 @@ pnpm -s lint Observed outcomes: - Typecheck/build/test: passing. -- Lint: passing with warnings only (`0` errors, `539` warnings). +- Lint: passing with warnings only (`0` errors, `501` 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 f494636..e545e57 100644 --- a/docs/plans/state.json +++ b/docs/plans/state.json @@ -2647,6 +2647,20 @@ "docs/plans/analysis/2026-02-16-codebase-audit-report.md" ], "test_status": "pnpm test:run src/gateway/server.test.ts src/tools/executor.test.ts src/backends/native/orchestrator.test.ts src/daemon/routing.test.ts + pnpm typecheck + pnpm lint passing (0 errors, warnings remain)" + }, + "audit-followup-lint-warning-reduction-pass-1": { + "status": "in_progress", + "date": "2026-02-16", + "updated": "2026-02-16", + "summary": "Started stage-2 lint warning reduction with low-risk test cleanup: removed non-null assertions and added explicit guards/helpers in selected tests, reducing warning count from 539 to 501 while keeping lint/typecheck/tests green.", + "files_modified": [ + "src/tools/builtin/browser/tools.test.ts", + "src/channels/telegram/adapter.test.ts", + "src/tools/builtin/system-info.test.ts", + "src/mcp/manager.test.ts", + "docs/plans/analysis/2026-02-16-codebase-audit-report.md" + ], + "test_status": "pnpm test:run src/tools/builtin/browser/tools.test.ts src/channels/telegram/adapter.test.ts src/tools/builtin/system-info.test.ts src/mcp/manager.test.ts + pnpm typecheck + pnpm lint passing (0 errors, 501 warnings)" } }, "overall_progress": { diff --git a/src/channels/telegram/adapter.test.ts b/src/channels/telegram/adapter.test.ts index 2329fc6..74d73c5 100644 --- a/src/channels/telegram/adapter.test.ts +++ b/src/channels/telegram/adapter.test.ts @@ -31,6 +31,22 @@ const baseConfig: TelegramAdapterConfig = { describe('TelegramAdapter', () => { let adapter: TelegramAdapter; + function getOnHandler(event: string) { + const call = mockOn.mock.calls.find((entry) => entry[0] === event); + if (!call) { + throw new Error(`Missing on-handler for event: ${event}`); + } + return call[1] as (ctx: unknown) => Promise; + } + + function getCommandHandler(command: string) { + const call = mockCommand.mock.calls.find((entry) => entry[0] === command); + if (!call) { + throw new Error(`Missing command handler: ${command}`); + } + return call[1] as (ctx: unknown) => Promise; + } + beforeEach(() => { vi.clearAllMocks(); adapter = new TelegramAdapter(baseConfig); @@ -153,7 +169,7 @@ describe('TelegramAdapter', () => { ); expect(textHandlerCall).toBeDefined(); - const textHandler = textHandlerCall![1]; + const textHandler = getOnHandler('message:text'); // Simulate a grammy context object const ctx = { @@ -179,11 +195,7 @@ describe('TelegramAdapter', () => { it('text handler does nothing when no message handler is registered', async () => { // Don't call onMessage — no handler await adapter.connect(); - - const textHandlerCall = mockOn.mock.calls.find( - (call) => call[0] === 'message:text', - ); - const textHandler = textHandlerCall![1]; + const textHandler = getOnHandler('message:text'); const ctx = { message: { message_id: 1, text: 'test' }, @@ -209,7 +221,7 @@ describe('TelegramAdapter', () => { const resetCall = mockCommand.mock.calls.find((call) => call[0] === 'reset'); expect(resetCall).toBeDefined(); - const resetHandler = resetCall![1]; + const resetHandler = getCommandHandler('reset'); const ctx = { message: { message_id: 99 }, @@ -237,7 +249,7 @@ describe('TelegramAdapter', () => { // Find the /model command handler const modelCall = mockCommand.mock.calls.find((call) => call[0] === 'model'); expect(modelCall).toBeDefined(); - const modelHandler = modelCall![1]; + const modelHandler = getCommandHandler('model'); const ctx = { message: { message_id: 123, text: '/model@flynn_bot default github/gpt-5-mini' }, @@ -298,10 +310,7 @@ describe('TelegramAdapter', () => { const startCall = mockStart.mock.calls[0][0]; startCall.onStart({ id: 12345, username: 'flynn_bot' }); - const textHandlerCall = mockOn.mock.calls.find( - (call) => call[0] === 'message:text', - ); - const textHandler = textHandlerCall![1]; + const textHandler = getOnHandler('message:text'); const ctx = { message: { message_id: 42, text: 'Hello everyone', reply_to_message: undefined }, @@ -326,10 +335,7 @@ describe('TelegramAdapter', () => { const startCall = mockStart.mock.calls[0][0]; startCall.onStart({ id: 12345, username: 'flynn_bot' }); - const textHandlerCall = mockOn.mock.calls.find( - (call) => call[0] === 'message:text', - ); - const textHandler = textHandlerCall![1]; + const textHandler = getOnHandler('message:text'); const ctx = { message: { message_id: 42, text: '@flynn_bot What is the weather?', reply_to_message: undefined }, @@ -355,10 +361,7 @@ describe('TelegramAdapter', () => { const startCall = mockStart.mock.calls[0][0]; startCall.onStart({ id: 12345, username: 'flynn_bot' }); - const textHandlerCall = mockOn.mock.calls.find( - (call) => call[0] === 'message:text', - ); - const textHandler = textHandlerCall![1]; + const textHandler = getOnHandler('message:text'); const ctx = { message: { @@ -392,10 +395,7 @@ describe('TelegramAdapter', () => { const startCall = mockStart.mock.calls[0][0]; startCall.onStart({ id: 12345, username: 'flynn_bot' }); - const textHandlerCall = mockOn.mock.calls.find( - (call) => call[0] === 'message:text', - ); - const textHandler = textHandlerCall![1]; + const textHandler = getOnHandler('message:text'); const ctx = { message: { message_id: 42, text: 'Hello everyone', reply_to_message: undefined }, @@ -421,10 +421,7 @@ describe('TelegramAdapter', () => { const startCall = mockStart.mock.calls[0][0]; startCall.onStart({ id: 12345, username: 'flynn_bot' }); - const textHandlerCall = mockOn.mock.calls.find( - (call) => call[0] === 'message:text', - ); - const textHandler = textHandlerCall![1]; + const textHandler = getOnHandler('message:text'); const ctx = { message: { message_id: 42, text: 'Hello Flynn', reply_to_message: undefined }, @@ -450,10 +447,7 @@ describe('TelegramAdapter', () => { const startCall = mockStart.mock.calls[0][0]; startCall.onStart({ id: 12345, username: 'flynn_bot' }); - const textHandlerCall = mockOn.mock.calls.find( - (call) => call[0] === 'message:text', - ); - const textHandler = textHandlerCall![1]; + const textHandler = getOnHandler('message:text'); const ctx = { message: { message_id: 42, text: '@flynn_bot tell me a joke', reply_to_message: undefined }, diff --git a/src/mcp/manager.test.ts b/src/mcp/manager.test.ts index 364dccc..a0544e0 100644 --- a/src/mcp/manager.test.ts +++ b/src/mcp/manager.test.ts @@ -32,6 +32,13 @@ describe('McpManager', () => { let registry: ToolRegistry; let manager: McpManager; + function assertDefined(value: T | undefined): T { + if (value === undefined) { + throw new Error('Expected value to be defined'); + } + return value; + } + beforeEach(() => { vi.clearAllMocks(); registry = new ToolRegistry(); @@ -48,8 +55,8 @@ describe('McpManager', () => { // Tool should be registered with mcp: prefix const tool = registry.get('mcp:test-server:do_thing'); expect(tool).toBeDefined(); - expect(tool!.name).toBe('mcp:test-server:do_thing'); - expect(tool!.description).toContain('[MCP:test-server]'); + expect(assertDefined(tool).name).toBe('mcp:test-server:do_thing'); + expect(assertDefined(tool).description).toContain('[MCP:test-server]'); }); it('startAll handles multiple servers', async () => { @@ -135,9 +142,9 @@ describe('McpManager', () => { const state = manager.getServerState('test-server'); expect(state).toBeDefined(); - expect(state!.config.name).toBe('test-server'); - expect(state!.config.args).toEqual(['--flag']); - expect(state!.tools).toHaveLength(1); + expect(assertDefined(state).config.name).toBe('test-server'); + expect(assertDefined(state).config.args).toEqual(['--flag']); + expect(assertDefined(state).tools).toHaveLength(1); }); it('getRegisteredTools returns all MCP tools', async () => { @@ -163,7 +170,7 @@ describe('McpManager', () => { const state = manager.getServerState('test-server'); expect(state).toBeDefined(); - expect(state!.config.args).toEqual(['--arg1']); + expect(assertDefined(state).config.args).toEqual(['--arg1']); expect(registry.get('mcp:test-server:do_thing')).toBeDefined(); }); diff --git a/src/tools/builtin/browser/tools.test.ts b/src/tools/builtin/browser/tools.test.ts index 1dcabb7..7d9dd2d 100644 --- a/src/tools/builtin/browser/tools.test.ts +++ b/src/tools/builtin/browser/tools.test.ts @@ -34,6 +34,14 @@ const mockManager = { describe('Browser tools', () => { let tools: ReturnType; + function getTool(name: string) { + const tool = tools.find(t => t.name === name); + if (!tool) { + throw new Error(`Tool not found: ${name}`); + } + return tool; + } + beforeEach(() => { vi.clearAllMocks(); tools = createBrowserTools(mockManager); @@ -51,7 +59,7 @@ describe('Browser tools', () => { }); it('browser.navigate navigates to URL', async () => { - const tool = tools.find(t => t.name === 'browser.navigate')!; + const tool = getTool('browser.navigate'); const result = await tool.execute({ url: 'https://example.com' }); expect(result.success).toBe(true); expect(result.output).toContain('example.com'); @@ -59,13 +67,13 @@ describe('Browser tools', () => { }); it('browser.navigate respects custom waitUntil', async () => { - const tool = tools.find(t => t.name === 'browser.navigate')!; + const tool = getTool('browser.navigate'); await tool.execute({ url: 'https://example.com', waitUntil: 'networkidle0' }); expect(mockGoto).toHaveBeenCalledWith('https://example.com', { waitUntil: 'networkidle0' }); }); it('browser.screenshot takes page screenshot', async () => { - const tool = tools.find(t => t.name === 'browser.screenshot')!; + const tool = getTool('browser.screenshot'); const result = await tool.execute({}); expect(result.success).toBe(true); expect(result.output).toContain('Screenshot captured'); @@ -73,7 +81,7 @@ describe('Browser tools', () => { }); it('browser.screenshot takes element screenshot', async () => { - const tool = tools.find(t => t.name === 'browser.screenshot')!; + const tool = getTool('browser.screenshot'); const result = await tool.execute({ selector: '#header' }); expect(result.success).toBe(true); expect(mock$).toHaveBeenCalledWith('#header'); @@ -81,28 +89,28 @@ describe('Browser tools', () => { it('browser.screenshot fails for missing element', async () => { mock$.mockResolvedValueOnce(null); - const tool = tools.find(t => t.name === 'browser.screenshot')!; + const tool = getTool('browser.screenshot'); const result = await tool.execute({ selector: '#nonexistent' }); expect(result.success).toBe(false); expect(result.error).toContain('Element not found'); }); it('browser.click clicks element', async () => { - const tool = tools.find(t => t.name === 'browser.click')!; + const tool = getTool('browser.click'); const result = await tool.execute({ selector: '#submit' }); expect(result.success).toBe(true); expect(mockClick).toHaveBeenCalledWith('#submit'); }); it('browser.type types into element', async () => { - const tool = tools.find(t => t.name === 'browser.type')!; + const tool = getTool('browser.type'); const result = await tool.execute({ selector: '#search', text: 'hello' }); expect(result.success).toBe(true); expect(mockType).toHaveBeenCalledWith('#search', 'hello'); }); it('browser.type clears field before typing when clear=true', async () => { - const tool = tools.find(t => t.name === 'browser.type')!; + const tool = getTool('browser.type'); await tool.execute({ selector: '#search', text: 'hello', clear: true }); expect(mockClick).toHaveBeenCalledWith('#search', { count: 3 }); expect(mockKeyboard.press).toHaveBeenCalledWith('Backspace'); @@ -110,7 +118,7 @@ describe('Browser tools', () => { }); it('browser.content returns page text', async () => { - const tool = tools.find(t => t.name === 'browser.content')!; + const tool = getTool('browser.content'); const result = await tool.execute({}); expect(result.success).toBe(true); expect(result.output).toContain('Page content here'); @@ -119,13 +127,13 @@ describe('Browser tools', () => { }); it('browser.content uses custom selector', async () => { - const tool = tools.find(t => t.name === 'browser.content')!; + const tool = getTool('browser.content'); await tool.execute({ selector: '#main' }); expect(mock$eval).toHaveBeenCalledWith('#main', expect.any(Function)); }); it('browser.eval evaluates JS', async () => { - const tool = tools.find(t => t.name === 'browser.eval')!; + const tool = getTool('browser.eval'); const result = await tool.execute({ expression: '1 + 1' }); expect(result.success).toBe(true); expect(result.output).toContain('42'); @@ -133,7 +141,7 @@ describe('Browser tools', () => { it('browser.eval returns string results directly', async () => { mockEvaluate.mockResolvedValueOnce('hello world'); - const tool = tools.find(t => t.name === 'browser.eval')!; + const tool = getTool('browser.eval'); const result = await tool.execute({ expression: '"hello world"' }); expect(result.success).toBe(true); expect(result.output).toBe('hello world'); @@ -141,7 +149,7 @@ describe('Browser tools', () => { it('handles navigation errors gracefully', async () => { mockGoto.mockRejectedValueOnce(new Error('Navigation failed')); - const tool = tools.find(t => t.name === 'browser.navigate')!; + const tool = getTool('browser.navigate'); const result = await tool.execute({ url: 'https://broken.example.com' }); expect(result.success).toBe(false); expect(result.error).toContain('Navigation failed'); @@ -149,14 +157,14 @@ describe('Browser tools', () => { it('handles click errors gracefully', async () => { mockClick.mockRejectedValueOnce(new Error('Element not found')); - const tool = tools.find(t => t.name === 'browser.click')!; + const tool = getTool('browser.click'); const result = await tool.execute({ selector: '#missing' }); expect(result.success).toBe(false); expect(result.error).toContain('Element not found'); }); it('returns aborted error when signal is already aborted', async () => { - const tool = tools.find(t => t.name === 'browser.navigate')!; + const tool = getTool('browser.navigate'); const controller = new AbortController(); controller.abort(); diff --git a/src/tools/builtin/system-info.test.ts b/src/tools/builtin/system-info.test.ts index eb29220..ac2d713 100644 --- a/src/tools/builtin/system-info.test.ts +++ b/src/tools/builtin/system-info.test.ts @@ -1,6 +1,13 @@ import { describe, it, expect } from 'vitest'; import { systemInfoTool } from './system-info.js'; +function getOutput(output: string | undefined): string { + if (!output) { + throw new Error('Expected output'); + } + return output; +} + describe('system.info tool', () => { // ── Metadata ───────────────────────────────────────────────────────────── @@ -17,14 +24,14 @@ describe('system.info tool', () => { expect(result.success).toBe(true); expect(typeof result.output).toBe('string'); - expect(result.output!.length).toBeGreaterThan(0); + expect(getOutput(result.output).length).toBeGreaterThan(0); }); it('output contains expected fields', async () => { const result = await systemInfoTool.execute({}); expect(result.success).toBe(true); - const output = result.output!; + const output = getOutput(result.output); const expectedFields = [ 'Date:', @@ -46,17 +53,17 @@ describe('system.info tool', () => { const result = await systemInfoTool.execute({}); expect(result.success).toBe(true); - const output = result.output!; + const output = getOutput(result.output); // Find the line containing 'ISO 8601:' const isoLine = output.split('\n').find((line) => line.includes('ISO 8601:')); expect(isoLine).toBeTruthy(); // Extract the ISO string and validate it - const isoMatch = isoLine!.match(/ISO 8601:\s*(.+)/); + const isoMatch = getOutput(isoLine).match(/ISO 8601:\s*(.+)/); expect(isoMatch).toBeTruthy(); - const isoString = isoMatch![1].trim(); + const isoString = getOutput(isoMatch?.[1]).trim(); const parsed = new Date(isoString); expect(parsed.toISOString()).toBe(isoString); }); @@ -78,7 +85,7 @@ describe('system.info tool', () => { expect(result2.success).toBe(true); expect(typeof result1.output).toBe('string'); expect(typeof result2.output).toBe('string'); - expect(result1.output!.length).toBeGreaterThan(0); - expect(result2.output!.length).toBeGreaterThan(0); + expect(getOutput(result1.output).length).toBeGreaterThan(0); + expect(getOutput(result2.output).length).toBeGreaterThan(0); }); });