From 06ff94e1976ccf026e66436fb6f9302d262ba11d Mon Sep 17 00:00:00 2001 From: William Valentin Date: Sun, 15 Feb 2026 22:33:37 -0800 Subject: [PATCH] test(lint): remove non-null assertions in skills loader tests --- .../2026-02-16-codebase-audit-report.md | 8 +- docs/plans/state.json | 5 +- src/skills/loader.test.ts | 73 ++++++++++--------- 3 files changed, 47 insertions(+), 39 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 caee194..8a5732f 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`, `501 warnings`) +- `pnpm lint`: passing with warnings only (`0 errors`, `466 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`, `501 warnings` + - `pnpm -s lint` => `0 errors`, `466 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 `501` warnings via targeted low-risk test cleanup (non-null assertion removal). +- Stage 2 in progress: warning-burn-down reduced to `466` 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, `501` warnings). +- Lint: passing with warnings only (`0` errors, `466` 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 e545e57..d6be0dc 100644 --- a/docs/plans/state.json +++ b/docs/plans/state.json @@ -2652,15 +2652,16 @@ "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.", + "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 466 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", + "src/skills/loader.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)" + "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 src/skills/loader.test.ts + pnpm typecheck + pnpm lint passing (0 errors, 466 warnings)" } }, "overall_progress": { diff --git a/src/skills/loader.test.ts b/src/skills/loader.test.ts index def3b59..38ee314 100644 --- a/src/skills/loader.test.ts +++ b/src/skills/loader.test.ts @@ -118,6 +118,13 @@ describe('loadSkill', () => { // Objective: verify that a single skill directory is correctly loaded into a Skill object. let tmpDir: string; + function assertSkill(skill: ReturnType): NonNullable> { + if (!skill) { + throw new Error('Expected skill to be loaded'); + } + return skill; + } + afterEach(() => { if (tmpDir) { rmSync(tmpDir, { recursive: true, force: true }); @@ -138,12 +145,12 @@ describe('loadSkill', () => { const skill = loadSkill(skillDir, 'bundled'); expect(skill).not.toBeNull(); - expect(skill!.manifest.name).toBe('my-skill'); - expect(skill!.manifest.description).toBe('A test skill'); - expect(skill!.manifest.version).toBe('1.2.0'); - expect(skill!.instructions).toBe('# My Skill\nDo something useful.'); - expect(skill!.available).toBe(true); - expect(skill!.directory).toBe(skillDir); + expect(assertSkill(skill).manifest.name).toBe('my-skill'); + expect(assertSkill(skill).manifest.description).toBe('A test skill'); + expect(assertSkill(skill).manifest.version).toBe('1.2.0'); + expect(assertSkill(skill).instructions).toBe('# My Skill\nDo something useful.'); + expect(assertSkill(skill).available).toBe(true); + expect(assertSkill(skill).directory).toBe(skillDir); }); it('returns null when SKILL.md is missing', () => { @@ -171,10 +178,10 @@ describe('loadSkill', () => { const skill = loadSkill(skillDir, 'workspace'); expect(skill).not.toBeNull(); - expect(skill!.manifest.name).toBe('inferred-skill'); - expect(skill!.manifest.description).toBe('A useful skill description'); - expect(skill!.manifest.version).toBe('0.0.0'); - expect(skill!.manifest.tier).toBe('workspace'); + expect(assertSkill(skill).manifest.name).toBe('inferred-skill'); + expect(assertSkill(skill).manifest.description).toBe('A useful skill description'); + expect(assertSkill(skill).manifest.version).toBe('0.0.0'); + expect(assertSkill(skill).manifest.tier).toBe('workspace'); }); it('marks skill unavailable when manifest.json has invalid JSON', () => { @@ -188,8 +195,8 @@ describe('loadSkill', () => { const skill = loadSkill(skillDir, 'bundled'); expect(skill).not.toBeNull(); - expect(skill!.available).toBe(false); - expect(skill!.unavailableReasons?.join('\n')).toMatch(/manifest\.invalid_json|manifest\.invalid_json/i); + expect(assertSkill(skill).available).toBe(false); + expect(assertSkill(skill).unavailableReasons?.join('\n')).toMatch(/manifest\.invalid_json|manifest\.invalid_json/i); }); it('marks skill unavailable when manifest.json is missing required fields', () => { @@ -203,8 +210,8 @@ describe('loadSkill', () => { const skill = loadSkill(skillDir, 'bundled'); expect(skill).not.toBeNull(); - expect(skill!.available).toBe(false); - expect(skill!.unavailableReasons?.join('\n')).toMatch(/manifest\.missing_required_fields/i); + expect(assertSkill(skill).available).toBe(false); + expect(assertSkill(skill).unavailableReasons?.join('\n')).toMatch(/manifest\.missing_required_fields/i); }); it('marks skill unavailable when manifest.json has invalid permissions specification', () => { @@ -227,8 +234,8 @@ describe('loadSkill', () => { const skill = loadSkill(skillDir, 'bundled'); expect(skill).not.toBeNull(); - expect(skill!.available).toBe(false); - expect(skill!.unavailableReasons?.join('\n')).toMatch(/manifest\.invalid_permissions/i); + expect(assertSkill(skill).available).toBe(false); + expect(assertSkill(skill).unavailableReasons?.join('\n')).toMatch(/manifest\.invalid_permissions/i); }); it('marks skill unavailable when skill directory contains a symlink', () => { @@ -243,8 +250,8 @@ describe('loadSkill', () => { const skill = loadSkill(skillDir, 'bundled'); expect(skill).not.toBeNull(); - expect(skill!.available).toBe(false); - expect(skill!.unavailableReasons?.join('\n')).toMatch(/fs\.symlink/i); + expect(assertSkill(skill).available).toBe(false); + expect(assertSkill(skill).unavailableReasons?.join('\n')).toMatch(/fs\.symlink/i); }); it('marks skill unavailable when SKILL.md contains prompt injection markers', () => { @@ -256,8 +263,8 @@ describe('loadSkill', () => { const skill = loadSkill(skillDir, 'bundled'); expect(skill).not.toBeNull(); - expect(skill!.available).toBe(false); - expect(skill!.unavailableReasons?.join('\n')).toMatch(/prompt\./i); + expect(assertSkill(skill).available).toBe(false); + expect(assertSkill(skill).unavailableReasons?.join('\n')).toMatch(/prompt\./i); }); it('marks skill unavailable when a file exceeds the max size threshold', () => { @@ -272,8 +279,8 @@ describe('loadSkill', () => { const skill = loadSkill(skillDir, 'bundled'); expect(skill).not.toBeNull(); - expect(skill!.available).toBe(false); - expect(skill!.unavailableReasons?.join('\n')).toMatch(/fs\.oversize/i); + expect(assertSkill(skill).available).toBe(false); + expect(assertSkill(skill).unavailableReasons?.join('\n')).toMatch(/fs\.oversize/i); }); it('strips markdown heading markers from inferred description', () => { @@ -286,7 +293,7 @@ describe('loadSkill', () => { const skill = loadSkill(skillDir, 'bundled'); expect(skill).not.toBeNull(); - expect(skill!.manifest.description).toBe('Heading Skill Title'); + expect(assertSkill(skill).manifest.description).toBe('Heading Skill Title'); }); it('sets tier from the argument, not from manifest content', () => { @@ -303,7 +310,7 @@ describe('loadSkill', () => { const skill = loadSkill(skillDir, 'managed'); expect(skill).not.toBeNull(); - expect(skill!.manifest.tier).toBe('managed'); + expect(assertSkill(skill).manifest.tier).toBe('managed'); }); it('accepts valid manifest installers definitions', () => { @@ -329,7 +336,7 @@ describe('loadSkill', () => { const skill = loadSkill(skillDir, 'bundled'); expect(skill).not.toBeNull(); - expect(skill!.manifest.installers).toHaveLength(4); + expect(assertSkill(skill).manifest.installers).toHaveLength(4); }); it('marks skill unavailable when installers is not an array', () => { @@ -350,8 +357,8 @@ describe('loadSkill', () => { const skill = loadSkill(skillDir, 'bundled'); expect(skill).not.toBeNull(); - expect(skill!.available).toBe(false); - expect(skill!.unavailableReasons?.join('\n')).toMatch(/manifest\.invalid_installers/i); + expect(assertSkill(skill).available).toBe(false); + expect(assertSkill(skill).unavailableReasons?.join('\n')).toMatch(/manifest\.invalid_installers/i); }); it('marks skill unavailable when installer entries are invalid', () => { @@ -375,8 +382,8 @@ describe('loadSkill', () => { const skill = loadSkill(skillDir, 'bundled'); expect(skill).not.toBeNull(); - expect(skill!.available).toBe(false); - expect(skill!.unavailableReasons?.join('\n')).toMatch(/manifest\.invalid_installers/i); + expect(assertSkill(skill).available).toBe(false); + expect(assertSkill(skill).unavailableReasons?.join('\n')).toMatch(/manifest\.invalid_installers/i); }); it('marks skill unavailable when requirements are not met', () => { @@ -398,10 +405,10 @@ describe('loadSkill', () => { const skill = loadSkill(skillDir, 'bundled'); expect(skill).not.toBeNull(); - expect(skill!.available).toBe(false); - expect(skill!.unavailableReasons).toBeDefined(); - expect(skill!.unavailableReasons!.length).toBeGreaterThan(0); - expect(skill!.unavailableReasons![0]).toContain('nonexistent-binary-xyz123'); + expect(assertSkill(skill).available).toBe(false); + expect(assertSkill(skill).unavailableReasons).toBeDefined(); + expect((assertSkill(skill).unavailableReasons ?? []).length).toBeGreaterThan(0); + expect((assertSkill(skill).unavailableReasons ?? [])[0]).toContain('nonexistent-binary-xyz123'); }); });