will/flynn
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

test(lint): remove non-null assertions in skills loader tests

Browse Source
This commit is contained in:
William Valentin
2026-02-15 22:33:37 -08:00
parent 021435ac27
commit 06ff94e197
3 changed files with 47 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/plans/analysis/2026-02-16-codebase-audit-report.md
+4 -4
View File
@@ -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
docs/plans/state.json
+3 -2
View File
@@ -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": {
src/skills/loader.test.ts
+40 -33
View File
@@ -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<typeof loadSkill>): NonNullable<ReturnType<typeof loadSkill>> {
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');
});
});