diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d5202f..54b9cc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **The theme validator agrees with lich's install on three edge cases.** + `formatVersion: 0` passes, as lich reads it as omitted; a `source` that does + not decode into `{url, version}` fails, as lich fails to parse the file, while + any decodable one passes, since lich replaces it on install; and a directory + named `*.json` in a theme repository is skipped instead of counted as a theme. + ## [0.18.2] - 2026-10-08 ### Fixed diff --git a/skills/theme/validate.mjs b/skills/theme/validate.mjs index ed6d004..3cee86e 100644 --- a/skills/theme/validate.mjs +++ b/skills/theme/validate.mjs @@ -42,13 +42,23 @@ if (!target) { function formatVersionError(document) { if (!("formatVersion" in document)) return null const version = document.formatVersion - if (!Number.isInteger(version) || version < 1) + // 0 is what lich decodes an omitted formatVersion to, so it reads as 1. + if (!Number.isInteger(version) || version < 0) return `theme formatVersion ${JSON.stringify(version)} must be a positive integer` if (version > FORMAT_VERSION) return `theme format ${version} is newer than this lich reads (up to ${FORMAT_VERSION}); update lich` return null } +// lich replaces a written source on install, so its contents are never checked; +// only one that does not decode into {url, version} fails, as a parse error. +function sourceDecodes(source) { + if (source === undefined || source === null) return true + if (typeof source !== "object" || Array.isArray(source)) return false + const decodesToString = (value) => value === undefined || value === null || typeof value === "string" + return decodesToString(source.url) && decodesToString(source.version) +} + function themeErrors(theme) { const errors = [] const format = formatVersionError(theme) @@ -60,6 +70,8 @@ function themeErrors(theme) { if ([...(theme.name ?? "")].length > 128) errors.push("name cannot exceed 128 characters") if (theme.scheme !== "light" && theme.scheme !== "dark") errors.push('scheme must be "light" or "dark"') + if (!sourceDecodes(theme.source)) + errors.push("parse theme JSON: source must be an object whose url and version are strings") const checkColors = (group, allowed, pattern, requireAll) => { const colors = theme[group] @@ -115,8 +127,9 @@ function checkRepository(dir) { `${MANIFEST}: manifest minLichVersion ${JSON.stringify(minimum)} must be MAJOR.MINOR.PATCH`, ) - const files = readdirSync(dir) - .filter((name) => name !== MANIFEST && extname(name) === ".json") + const files = readdirSync(dir, { withFileTypes: true }) + .filter((entry) => !entry.isDirectory() && entry.name !== MANIFEST && extname(entry.name) === ".json") + .map((entry) => entry.name) .sort() if (files.length === 0) errors.push(`no theme JSON next to ${MANIFEST}`) if (files.length > MAX_THEMES) errors.push(`more than ${MAX_THEMES} themes in one repository`) @@ -124,7 +137,6 @@ function checkRepository(dir) { const seen = new Map() for (const name of files) { const path = join(dir, name) - if (statSync(path).isDirectory()) continue errors.push(...checkTheme(path)) const { value } = read(path) const id = value?.id diff --git a/tests/theme-validate.test.mjs b/tests/theme-validate.test.mjs index 11a1853..3ba0c5b 100644 --- a/tests/theme-validate.test.mjs +++ b/tests/theme-validate.test.mjs @@ -1,12 +1,12 @@ -// Pins skills/theme/validate.mjs against the app tokens lich requires on -// import: a theme it calls ok must be one lich installs, and the other way round. +// Pins skills/theme/validate.mjs against what lich does when it installs a +// theme: a theme it calls ok must be one lich installs, and the other way round. // // Run: node --test tests/ import { test } from 'node:test' import assert from 'node:assert/strict' import { spawnSync } from 'node:child_process' -import { mkdtempSync, readFileSync, writeFileSync } from 'node:fs' +import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -22,12 +22,20 @@ const tones = { 'tone-wait': 'oklch(0.769 0.188 70.08)', } -function validate(app) { - const path = join(mkdtempSync(join(tmpdir(), 'lich-theme-')), 'theme.json') - writeFileSync(path, JSON.stringify({ ...template, id: 'tones', app })) +function validatePath(path) { return spawnSync(process.execPath, [validator, path], { encoding: 'utf8' }) } +function validateTheme(theme) { + const path = join(mkdtempSync(join(tmpdir(), 'lich-theme-')), 'theme.json') + writeFileSync(path, JSON.stringify({ ...template, ...theme })) + return validatePath(path) +} + +function validate(app) { + return validateTheme({ id: 'tones', app }) +} + test('a theme carrying the status tones passes', () => { const run = validate({ ...template.app, ...tones }) assert.equal(run.status, 0, run.stderr) @@ -39,3 +47,39 @@ test('a theme missing a status tone is rejected the way lich rejects it', () => assert.equal(run.status, 1) assert.match(run.stderr, /app\.tone-wait is required/) }) + +// From here on the rule is what lich does when it installs: readPackTheme and +// Import in lich's internal/themes, not the schema it generates. + +test('formatVersion 0 passes, the way lich reads an omitted one', () => { + const result = validateTheme({ formatVersion: 0 }) + assert.equal(result.status, 0, result.stderr) +}) + +test('a negative formatVersion is rejected with lich\'s message', () => { + const result = validateTheme({ formatVersion: -1 }) + assert.equal(result.status, 1) + assert.match(result.stderr, /theme formatVersion -1 must be a positive integer/) +}) + +test('a source lich cannot decode is rejected, as lich fails to parse the file', () => { + const result = validateTheme({ source: 'https://example.com/themes.git' }) + assert.equal(result.status, 1) + assert.match(result.stderr, /parse theme JSON: source/) +}) + +// lich replaces a written source on both install paths (Import drops it, +// readPackTheme stamps the repository's), so it never checks the URL in one. +test('a decodable source passes whatever it names, since lich discards it', () => { + const result = validateTheme({ source: { url: 'ext::sh -c id', version: '1.0.0-rc1' } }) + assert.equal(result.status, 0, result.stderr) +}) + +test('a directory named like a theme is no theme, so a pack of only that has none', () => { + const dir = mkdtempSync(join(tmpdir(), 'lich-pack-')) + writeFileSync(join(dir, 'lich-theme.json'), JSON.stringify({ name: 'Pack', version: '1.0.0' })) + mkdirSync(join(dir, 'assets.json')) + const result = validatePath(dir) + assert.equal(result.status, 1) + assert.match(result.stderr, /no theme JSON next to lich-theme\.json/) +})