From c3c63257493bc2135b132aac569c5893153b1a11 Mon Sep 17 00:00:00 2001 From: DevOhMyCode Date: Fri, 23 Oct 2026 10:21:47 +0200 Subject: [PATCH 01/11] fix(install): validate skill names wherever an id is parsed --- src/catalog/packs.ts | 3 +-- src/install/plan.ts | 14 +++++++++++++- src/mcp.ts | 4 +--- src/selection.ts | 11 +---------- test/install-cli.test.ts | 28 ++++++++++++++++++++++++++++ test/install.test.ts | 7 +++++++ 6 files changed, 51 insertions(+), 16 deletions(-) create mode 100644 test/install-cli.test.ts diff --git a/src/catalog/packs.ts b/src/catalog/packs.ts index b48ddb4..87e6eba 100644 --- a/src/catalog/packs.ts +++ b/src/catalog/packs.ts @@ -5,7 +5,6 @@ import { z } from 'zod'; import bundled from '../../data/packs.json' with { type: 'json' }; import { configPath } from '../config.js'; import { parseSkillId } from '../install/plan.js'; -import { checkSkillName } from '../selection.js'; import type { Catalog, Skill } from '../types.js'; /** A curated set of skills for one kind of work, installed as a block. */ @@ -39,7 +38,7 @@ function parsePacks(raw: unknown): Pack[] { const result = Packs.safeParse(raw); if (!result.success) throw new Error(`Invalid packs: ${result.error.message}`); // A personal file is input to `npx skills add`: its ids are checked like a selection file's. - for (const p of result.data) for (const id of p.skills) checkSkillName(parseSkillId(id).name, id); + for (const p of result.data) for (const id of p.skills) parseSkillId(id); return result.data; } diff --git a/src/install/plan.ts b/src/install/plan.ts index c055910..ed2fa95 100644 --- a/src/install/plan.ts +++ b/src/install/plan.ts @@ -27,7 +27,18 @@ export function parseSkillId(id: string): { source: string; name: string } { if (!/^[A-Za-z0-9_][A-Za-z0-9_.-]*\/[A-Za-z0-9_.-]+$/.test(source) || source.split('/').some(p => p === '.' || p === '..')) { throw new Error(`invalid source "${source}" in "${id}" — expected owner/repo`); } - return { source, name: id.slice(at + 1) }; + const name = id.slice(at + 1); + checkSkillName(name, id); + return { source, name }; +} + +/** + * Throws on a skill name that is not safe to pass to `npx skills`, whoever wrote it (a file, a model): + * a leading dash would reach it as a flag, and a quote or control character could break the quoting + * of Windows's fallback shell. Real names have neither. + */ +export function checkSkillName(name: string, context = name): void { + if (!name || name.startsWith('-') || /["\u0000-\u001f\u007f]/.test(name) || name.length > 200) throw new Error(`invalid skill name in "${context}"`); } export function checkRevision(value: unknown): string { @@ -87,6 +98,7 @@ export function planUpdates(updates: Update[], opts: InstallOptions): InstallCom * unticking an installed skill means. */ export function planRemove(names: string[], opts: { scope: Scope; skillsVersion: string }): InstallCommand[] { + for (const name of names) checkSkillName(name); const skills = [...new Set(names)]; if (skills.length === 0) return []; const args = ['-y', `skills@${opts.skillsVersion}`, 'remove', '-s', ...skills, '-y']; diff --git a/src/mcp.ts b/src/mcp.ts index d0f619c..1371e3c 100644 --- a/src/mcp.ts +++ b/src/mcp.ts @@ -9,7 +9,6 @@ import { nameCollisions } from './install/collisions.js'; import { formatCommand, parseSkillId, planInstall, planRemove } from './install/plan.js'; import { isSuccess, listInstalled, runAll } from './install/run.js'; import { riskReport, riskWhere } from './risks.js'; -import { checkSkillName } from './selection.js'; import { SCOPES, type Catalog, type Scope, type Skill } from './types.js'; /** @@ -145,7 +144,7 @@ async function callTool(name: ToolName, args: unknown, opts: McpOptions, catalog }))); } case 'install_skills': { - for (const id of input.ids as string[]) checkSkillName(parseSkillId(id).name, id); + for (const id of input.ids as string[]) parseSkillId(id); const unknown = unknownAgents(input.agents); if (unknown.length) throw new Error(`unknown agents: ${unknown.join(', ')}`); const commands = planInstall(input.ids, { agents: input.agents, scope: input.scope, method: input.method ?? 'symlink', skillsVersion: opts.skillsVersion }); @@ -170,7 +169,6 @@ async function callTool(name: ToolName, args: unknown, opts: McpOptions, catalog return json({ ok: outcomes.every(isSuccess), results: outcomes.flatMap((o) => o.results), errors: outcomes.filter((o) => !isSuccess(o)).map((o) => o.stderr.slice(-500)) }); } case 'remove_skills': { - for (const n of input.names as string[]) checkSkillName(n); const outcomes = await (opts.run ?? runAll)(planRemove(input.names, { scope: input.scope, skillsVersion: opts.skillsVersion })); return json({ ok: outcomes.every(isSuccess), results: outcomes.flatMap((o) => o.results) }); } diff --git a/src/selection.ts b/src/selection.ts index c4f5789..ec28e6e 100644 --- a/src/selection.ts +++ b/src/selection.ts @@ -40,7 +40,7 @@ export function parseSelectionFile(text: string): { ids: string[]; agents?: stri } const file = File.safeParse(raw); if (!file.success) throw new Error('not a skills-atlas selection ({ "skillsAtlas": 1, "skills": [...] })'); - for (const id of file.data.skills) checkSkillName(parseSkillId(id).name, id); + for (const id of file.data.skills) parseSkillId(id); const ids = [...new Set(file.data.skills)]; const revisions: Record = {}; for (const [id, revision] of Object.entries(file.data.revisions ?? {})) { @@ -77,12 +77,3 @@ export function missingSkillsByAgent(wanted: string[], installed: InstalledSkill } return [...groups.values()]; } - -/** - * Throws on a skill name that is not safe to pass to `npx skills`, whoever wrote it (a file, a model): - * a leading dash would reach it as a flag, and a quote or control character could break the quoting - * of Windows's fallback shell. Real names have neither. - */ -export function checkSkillName(name: string, context = name): void { - if (name.startsWith('-') || /["\u0000-\u001f\u007f]/.test(name) || name.length > 200) throw new Error(`invalid skill name in "${context}"`); -} diff --git a/test/install-cli.test.ts b/test/install-cli.test.ts new file mode 100644 index 0000000..db0d336 --- /dev/null +++ b/test/install-cli.test.ts @@ -0,0 +1,28 @@ +import { createRequire } from 'node:module'; +import { expect, it } from 'vitest'; +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; + +it('install rejects a skill name that would reach npx skills as a flag, before any command', () => { + const cwd = mkdtempSync(join(tmpdir(), 'atlas-install-cli-')); + const preload = join(cwd, 'guard.mjs'); + // Any network call or child process would mean the bad id got past validation. + writeFileSync(preload, `globalThis.fetch = async (url) => { throw Error('unexpected fetch ' + url); };`); + const tsx = pathToFileURL(createRequire(import.meta.url).resolve('tsx')).href; + const run = (...args: string[]) => spawnSync(process.execPath, ['--import', tsx, '--import', pathToFileURL(preload).href, fileURLToPath(new URL('../src/cli.tsx', import.meta.url)), ...args], { + cwd, encoding: 'utf8', env: { ...process.env, SKILLS_ATLAS_LANG: 'en', SKILLS_ATLAS_OFFLINE: '1', XDG_CONFIG_HOME: join(cwd, 'config') }, + }); + try { + const bad = run('install', 'o/r@x', 'o/r@--global', '--agent', 'codex', '--project', '--symlink', '--yes', '--no-audit', '--dry-run'); + expect(bad.status).not.toBe(0); + expect(bad.stderr).toContain('invalid skill name in "o/r@--global"'); + expect(bad.stdout).not.toContain('npx'); + const good = run('install', 'o/r@x', '--agent', 'codex', '--project', '--symlink', '--yes', '--no-audit', '--dry-run'); + expect(good.stderr).toBe(''); + expect(good.stdout).toContain('add o/r -s x -a codex -y --json'); + expect(good.stdout).not.toContain(' -g'); + } finally { rmSync(cwd, { recursive: true, force: true }); } +}, 60_000); diff --git a/test/install.test.ts b/test/install.test.ts index a8e49a9..a1873ef 100644 --- a/test/install.test.ts +++ b/test/install.test.ts @@ -20,6 +20,13 @@ describe('parseSkillId', () => { expect(() => parseSkillId('pdf@')).toThrow(); expect(() => parseSkillId('a/b/c@x')).toThrow(); }); + it('rejects names npx skills would misread', () => { + for (const name of ['--global', '-s', 'a"b', 'a\u0007b', 'x'.repeat(201)]) expect(() => parseSkillId(`o/r@${name}`)).toThrow(/invalid skill name/); + expect(parseSkillId(`o/r@${'x'.repeat(200)}`).name).toHaveLength(200); + expect(parseSkillId('o/r@my skill.v2').name).toBe('my skill.v2'); + expect(() => planRemove(['', 'pdf'], { scope: 'project', skillsVersion: '1.7.0' })).toThrow(/invalid skill name/); + expect(() => planRemove(['--global'], { scope: 'project', skillsVersion: '1.7.0' })).toThrow(/invalid skill name/); + }); }); describe('planInstall', () => { From 41e466652dca6e87c078a34725fb4568c9f3615f Mon Sep 17 00:00:00 2001 From: DevOhMyCode Date: Fri, 23 Oct 2026 10:22:54 +0200 Subject: [PATCH 02/11] fix(mcp): run installs and removals one at a time --- src/mcp.ts | 10 ++++++++-- test/mcp.test.ts | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/mcp.ts b/src/mcp.ts index 1371e3c..fed5d8a 100644 --- a/src/mcp.ts +++ b/src/mcp.ts @@ -186,6 +186,7 @@ interface Request { export function createHandler(opts: McpOptions) { let cached: Promise | undefined; const catalog = () => (cached ??= opts.loadCatalog()); + let mutations: Promise = Promise.resolve(); return async (msg: Request): Promise => { if (msg.id === undefined || msg.id === null) return undefined; const reply = (result: object) => ({ jsonrpc: '2.0', id: msg.id, result }); @@ -205,8 +206,13 @@ export function createHandler(opts: McpOptions) { case 'tools/call': { const name = msg.params?.name as string; if (!(name in TOOLS)) return error(-32602, `unknown tool: ${name}`); + const call = () => callTool(name as ToolName, msg.params?.arguments, opts, catalog); + // One install or removal at a time: the next re-reads what the previous changed. Reads and dry runs do not wait. + const mutates = name === 'remove_skills' || (name === 'install_skills' && !msg.params?.arguments?.dry_run); + const result = mutates ? mutations.then(call) : call(); + if (mutates) mutations = result.catch(() => undefined); try { - return reply({ content: [{ type: 'text', text: await callTool(name as ToolName, msg.params?.arguments, opts, catalog) }] }); + return reply({ content: [{ type: 'text', text: await result }] }); } catch (err) { // A tool failure is a result the model can read and act on, not a protocol error. const text = err instanceof z.ZodError ? `invalid arguments: ${err.issues.map((i) => `${i.path.join('.')}: ${i.message}`).join('; ')}` : err instanceof Error ? err.message : String(err); @@ -233,7 +239,7 @@ export async function serveMcp(opts: McpOptions): Promise { process.stdout.write(`${JSON.stringify({ jsonrpc: '2.0', id: null, error: { code: -32700, message: 'parse error' } })}\n`); continue; } - // Calls run concurrently; each answer goes out whole, on its own line. + // Calls run concurrently (the handler queues installs and removals); each answer goes out whole, on its own line. const done = handle(msg).then((res) => res && process.stdout.write(`${JSON.stringify(res)}\n`)); pending.add(done); void done.finally(() => pending.delete(done)); diff --git a/test/mcp.test.ts b/test/mcp.test.ts index 62996c9..d37f4c7 100644 --- a/test/mcp.test.ts +++ b/test/mcp.test.ts @@ -91,4 +91,38 @@ describe('MCP server', () => { await call('install_skills', { ...args, replace: true }); expect(run).toHaveBeenCalledOnce(); }); + + it('runs one install or removal at a time, each re-reading what is installed', async () => { + const have: string[] = []; + let active = 0, peak = 0, release!: () => void; + const gate = new Promise((r) => (release = r)); + const run = vi.fn(async (cmds: { kind: string; skills: string[] }[]): Promise => { + peak = Math.max(peak, ++active); + await gate; + active--; + for (const c of cmds) if (c.kind === 'add') have.push(...c.skills); else if (c.skills.includes('boom')) throw new Error('boom'); + return []; + }); + const installed = vi.fn(async () => have.map((name) => ({ id: `anthropics/skills@${name}`, name, source: 'anthropics/skills', scope: 'global' as const, agents: ['claude-code'] }))); + const handle = createHandler({ version: '9.9.9', skillsVersion: '1.7.0', loadCatalog: async () => catalog, run: run as never, installed, readSkill: async () => '# Clean' }); + let id = 0; + const call = async (name: string, args: object) => ((await handle({ jsonrpc: '2.0', id: ++id, method: 'tools/call', params: { name, arguments: args } })) as any).result; + const first = call('install_skills', { ids: ['anthropics/skills@pdf'], agents: ['claude-code'], scope: 'global' }); + // Same name from another source: only a check made after the first install sees the collision. + const second = call('install_skills', { ids: ['a/b@pdf'], agents: ['claude-code'], scope: 'global' }); + const removal = call('remove_skills', { names: ['boom'], scope: 'global' }); + const after = call('remove_skills', { names: ['pdf'], scope: 'global' }); + await vi.waitFor(() => expect(run).toHaveBeenCalledOnce()); + // Reads and dry runs answer while the install is blocked. + expect((await call('search_skills', { query: 'pdf' })).isError).toBeUndefined(); + expect((await call('install_skills', { ids: ['c/d@deploy'], agents: ['claude-code'], scope: 'global', dry_run: true })).isError).toBeUndefined(); + expect(run).toHaveBeenCalledOnce(); + release(); + await first; + expect((await second).content[0].text).toMatch(/would replace anthropics\/skills@pdf \(installed\)/); + expect((await removal).isError).toBe(true); + expect((await after).isError).toBeUndefined(); + expect(peak).toBe(1); + expect(run).toHaveBeenCalledTimes(3); + }); }); From 8ee50899bdc702351d22fee4b45f17948fef6faf Mon Sep 17 00:00:00 2001 From: DevOhMyCode Date: Fri, 23 Oct 2026 10:25:08 +0200 Subject: [PATCH 03/11] fix(rollback): record only a version whose folder hash is proven --- src/cli.tsx | 7 +++- src/install/history.ts | 53 +++++++++++++++++++++----- src/install/updates.ts | 5 ++- test/history.test.ts | 28 +++++++++++--- test/rollback-cli.test.ts | 80 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 155 insertions(+), 18 deletions(-) create mode 100644 test/rollback-cli.test.ts diff --git a/src/cli.tsx b/src/cli.tsx index 4e75359..5be820b 100644 --- a/src/cli.tsx +++ b/src/cli.tsx @@ -14,7 +14,7 @@ import { formatNumber, getLang, langFlag, LANGUAGES, normalizeLang, setLang, sys import { formatCommand, parseSkillId, planInstall, planRemovals, planUpdates, type InstallCommand, type Update } from './install/plan.js'; import { nameCollisions } from './install/collisions.js'; import { findUpdates } from './install/updates.js'; -import { dropRevision, readHistory, recordBeforeUpdate } from './install/history.js'; +import { dropRevision, readHistory, recordBeforeUpdate, rememberRevision } from './install/history.js'; import { onPath } from './prereqs.js'; import { diagnoseInstalled } from './install/doctor.js'; import { isSuccess, listInstalled, runAll, type CommandOutcome } from './install/run.js'; @@ -810,7 +810,10 @@ installOptions(program.command('rollback')) const remembered = readConfig(); await apply([skill!.id], [], options, opts, [], revisions); if (!opts.dryRun) writeConfig(remembered); - if (!opts.dryRun && !process.exitCode) dropRevision(skill!.id, target!.revision); + if (!opts.dryRun && !process.exitCode) { + dropRevision(skill!.id, target!.revision); + rememberRevision(skill!.id, target!.revision); + } }); program diff --git a/src/install/history.ts b/src/install/history.ts index 6a288a0..deb3210 100644 --- a/src/install/history.ts +++ b/src/install/history.ts @@ -21,6 +21,9 @@ const KEEP = 5; const History = z.record(z.string(), z.array(z.object({ revision: z.string().regex(/^[a-f0-9]{40}$/i), replacedAt: z.string() }))); export const historyPath = (): string => join(dirname(configPath()), 'history.json'); +/** Commits skills-atlas installed exactly (a rollback), with the folder hash the lock recorded then. */ +export const knownPath = (): string => join(dirname(configPath()), 'revisions.json'); +const Known = z.record(z.string(), z.object({ revision: z.string().regex(/^[a-f0-9]{40}$/i), hash: z.string() })); export function readHistory(path = historyPath()): Record { try { @@ -31,7 +34,7 @@ export function readHistory(path = historyPath()): Record, path: string): void { +function writeHistory(history: object, path: string): void { mkdirSync(dirname(path), { recursive: true }); const tmp = `${path}.${process.pid}.tmp`; writeFileSync(tmp, `${JSON.stringify(history, null, 2)}\n`); @@ -56,21 +59,51 @@ export function dropRevision(id: string, revision: string, path = historyPath()) writeHistory(history, path); } +function readKnown(path: string): z.infer { + try { + const parsed = Known.safeParse(JSON.parse(readFileSync(path, 'utf8'))); + return parsed.success ? parsed.data : {}; + } catch { + return {}; + } +} + +/** + * After an install at an exact commit succeeded: ties that commit to the folder hash the + * lock now records, so it is still known after a restart. A skill missing from the lock is skipped. + */ +export function rememberRevision(id: string, revision: string, lock = lockPath('global', process.cwd()), path = knownPath()): void { + const s = readLockedSkills(lock).find((l) => `${l.source}@${l.name}`.toLowerCase() === id.toLowerCase()); + if (!s) return; + const known = readKnown(path); + known[id.toLowerCase()] = { revision: checkRevision(revision), hash: s.hash }; + writeHistory(known, path); +} + const Commits = z.array(z.object({ sha: z.string().regex(/^[a-f0-9]{40}$/i) })); +const CommitTree = z.object({ sha: z.string(), truncated: z.boolean().optional(), tree: z.array(z.object({ path: z.string(), type: z.string(), sha: z.string() })) }); /** - * The commit a skill was installed from: the last one touching its folder before the - * lock's install or update date, which was the repository's HEAD then. - * shortcut: a force-push since then can point elsewhere; compare the folder's tree SHA if that matters. + * The commit a skill was installed from, only when it is proven: a commit skills-atlas + * installed while the lock still records the same folder hash, the installer's own ref, + * or the last commit touching the folder before the install date whose folder hash + * matches the lock's. Anything else is undefined: a date alone does not prove a version. */ -export async function installedRevision(skill: LockedSkill, token = githubToken()): Promise { +export async function installedRevision(skill: LockedSkill, token = githubToken(), known = knownPath()): Promise { + const remembered = readKnown(known)[`${skill.source}@${skill.name}`.toLowerCase()]; + if (remembered?.hash === skill.hash) return remembered.revision.toLowerCase(); + if (skill.ref) return skill.ref; if (!skill.date) return undefined; + const headers = { accept: 'application/vnd.github+json', ...(token ? { authorization: `Bearer ${token}` } : {}) }; const params = new URLSearchParams({ until: skill.date, per_page: '1', ...(skill.dir ? { path: skill.dir } : {}) }); - const commits = Commits.safeParse(await fetchJson(`https://api.github.com/repos/${skill.source}/commits?${params}`, { - retries: 1, timeoutMs: 10_000, - headers: { accept: 'application/vnd.github+json', ...(token ? { authorization: `Bearer ${token}` } : {}) }, - })); - return commits.success ? commits.data[0]?.sha : undefined; + const commits = Commits.safeParse(await fetchJson(`https://api.github.com/repos/${skill.source}/commits?${params}`, { retries: 1, timeoutMs: 10_000, headers })); + const candidate = commits.success ? commits.data[0]?.sha.toLowerCase() : undefined; + if (!candidate) return undefined; + // Asked by commit SHA, the Trees API answers with the root tree: its SHA is a root skill's folder hash. + const tree = CommitTree.safeParse(await fetchJson(`https://api.github.com/repos/${skill.source}/git/trees/${candidate}${skill.dir ? '?recursive=1' : ''}`, { retries: 1, timeoutMs: 10_000, headers })); + if (!tree.success) return undefined; + const folder = skill.dir ? tree.data.tree.find((e) => e.type === 'tree' && e.path === skill.dir)?.sha : tree.data.sha; + return folder === skill.hash ? candidate : undefined; } /** diff --git a/src/install/updates.ts b/src/install/updates.ts index 0c9e57f..780e32d 100644 --- a/src/install/updates.ts +++ b/src/install/updates.ts @@ -15,6 +15,8 @@ export interface LockedSkill { hash: string; /** When it was last installed or updated, per the lock. */ date?: string; + /** The exact commit, when the installer recorded one. */ + ref?: string; } const Lock = z.object({ @@ -23,6 +25,7 @@ const Lock = z.object({ sourceType: z.string().optional(), skillPath: z.string().optional(), skillFolderHash: z.string().optional(), + ref: z.string().optional(), installedAt: z.string().optional(), updatedAt: z.string().optional(), })), @@ -45,7 +48,7 @@ export function readLockedSkills(path: string): LockedSkill[] { return Object.entries(lock.data.skills).flatMap(([name, e]) => { if (e.sourceType !== 'github' || !e.source || !/^[\w.-]+\/[\w.-]+$/.test(e.source)) return []; if (!e.skillPath || !e.skillFolderHash) return []; - return [{ name, source: e.source, dir: e.skillPath.replace(/\/?SKILL\.md$/i, ''), hash: e.skillFolderHash, date: e.updatedAt ?? e.installedAt }]; + return [{ name, source: e.source, dir: e.skillPath.replace(/\/?SKILL\.md$/i, ''), hash: e.skillFolderHash, date: e.updatedAt ?? e.installedAt, ...(e.ref && /^[a-f0-9]{40}$/i.test(e.ref) ? { ref: e.ref.toLowerCase() } : {}) }]; }); } diff --git a/test/history.test.ts b/test/history.test.ts index f2bfaff..dd67fd2 100644 --- a/test/history.test.ts +++ b/test/history.test.ts @@ -33,11 +33,29 @@ describe('update history', () => { expect(Object.keys(readHistory(path))).toEqual(['o/r@x']); }); - it('asks GitHub for the last commit touching the folder before the install date', async () => { + it('keeps the last commit touching the folder before the install date only when its folder hash matches the lock', async () => { const urls: string[] = []; - vi.stubGlobal('fetch', async (url: string) => { urls.push(url); return Response.json([{ sha: sha(4) }]); }); - expect(await installedRevision({ name: 'x', source: 'o/r', dir: 'skills/x', hash: 'h', date: '2026-10-01T00:00:00Z' }, '')).toBe(sha(4)); - expect(urls[0]).toBe('https://api.github.com/repos/o/r/commits?until=2026-10-01T00%3A00%3A00Z&per_page=1&path=skills%2Fx'); - expect(await installedRevision({ name: 'x', source: 'o/r', dir: 'x', hash: 'h' })).toBeUndefined(); + const known = join(tmp(), 'revisions.json'); + const tree = (folder: string, truncated = false) => ({ sha: 'root', truncated, tree: [{ path: 'skills/x', type: 'tree', sha: folder }] }); + let answer = tree('h'); + vi.stubGlobal('fetch', async (url: string) => { urls.push(url); return Response.json(url.includes('/commits?') ? [{ sha: sha(4) }] : answer); }); + const x = { name: 'x', source: 'o/r', dir: 'skills/x', hash: 'h', date: '2026-10-01T00:00:00Z' }; + expect(await installedRevision(x, '', known)).toBe(sha(4)); + expect(urls).toEqual([ + 'https://api.github.com/repos/o/r/commits?until=2026-10-01T00%3A00%3A00Z&per_page=1&path=skills%2Fx', + `https://api.github.com/repos/o/r/git/trees/${sha(4)}?recursive=1`, + ]); + answer = tree('other'); + expect(await installedRevision(x, '', known)).toBeUndefined(); + answer = { sha: 'root', truncated: true, tree: [] }; + expect(await installedRevision(x, '', known)).toBeUndefined(); + // A skill at the repository root: its folder hash is the commit's root tree. + answer = tree('h'); + expect(await installedRevision({ ...x, dir: '', hash: 'root' }, '', known)).toBe(sha(4)); + expect(urls.at(-1)).toBe(`https://api.github.com/repos/o/r/git/trees/${sha(4)}`); + expect(await installedRevision({ ...x, date: undefined }, '', known)).toBeUndefined(); + expect(await installedRevision({ ...x, date: undefined, ref: sha(7) }, '', known)).toBe(sha(7)); + vi.stubGlobal('fetch', async () => { throw new TypeError('fetch failed'); }); + await expect(installedRevision(x, '', known)).rejects.toThrow(); }); }); diff --git a/test/rollback-cli.test.ts b/test/rollback-cli.test.ts new file mode 100644 index 0000000..c5016f9 --- /dev/null +++ b/test/rollback-cli.test.ts @@ -0,0 +1,80 @@ +import { createRequire } from 'node:module'; +import { spawnSync } from 'node:child_process'; +import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; +import { afterEach, expect, it, vi } from 'vitest'; +import { installedRevision, pushRevision, readHistory, recordBeforeUpdate, rememberRevision } from '../src/install/history.js'; + +const A = 'a'.repeat(40), B = 'b'.repeat(40), C = 'c'.repeat(40); + +afterEach(() => vi.unstubAllGlobals()); + +it('after a rollback to A while main is at B, the next update records A, even after a restart', async () => { + const dir = mkdtempSync(join(tmpdir(), 'atlas-rollback-')); + const lock = join(dir, 'lock.json'), history = join(dir, 'history.json'), known = join(dir, 'revisions.json'); + const writeLock = (hash: string, updatedAt: string) => writeFileSync(lock, JSON.stringify({ skills: { + x: { source: 'o/r', sourceType: 'github', skillPath: 'skills/x/SKILL.md', skillFolderHash: hash, updatedAt }, + } })); + // GitHub: the folder last changed in B (hash hB); A had hash hA. + vi.stubGlobal('fetch', async (url: string) => Response.json(url.includes('/commits?') ? [{ sha: B }] + : { sha: 'root', tree: [{ path: 'skills/x', type: 'tree', sha: url.includes(B) ? 'hB' : 'hA' }] })); + const resolve = (s: Parameters[0]) => installedRevision(s, '', known); + try { + // Update from B: B is proven by its folder hash and recorded. + writeLock('hB', '2026-10-01T00:00:00Z'); + expect(await recordBeforeUpdate(['o/r@x'], lock, history, resolve)).toEqual([]); + // Rollback reinstalls A: the lock now has hA and a recent date that a date lookup would map to B. + writeLock('hA', '2026-10-20T00:00:00Z'); + rememberRevision('o/r@x', A, lock, known); + // Restart: nothing in memory, only the files. The next update records A, never B. + expect(await recordBeforeUpdate(['o/r@x'], lock, history, resolve)).toEqual([]); + expect(readHistory(history)['o/r@x']!.at(-1)!.revision).toBe(A); + // Updated again elsewhere (hash differs from what was remembered), and no proof on GitHub: no way back. + writeLock('hC', '2026-10-22T00:00:00Z'); + expect(await recordBeforeUpdate(['o/r@x'], lock, history, resolve)).toEqual(['o/r@x']); + // Offline: the version cannot be proven either. + vi.stubGlobal('fetch', async () => { throw new TypeError('fetch failed'); }); + expect(await recordBeforeUpdate(['o/r@x'], lock, history, resolve)).toEqual(['o/r@x']); + expect(readHistory(history)['o/r@x']!.map((r) => r.revision)).toEqual([B, A]); + } finally { rmSync(dir, { recursive: true, force: true }); } +}); + +it('rollback --dry-run records nothing', () => { + const cwd = mkdtempSync(join(tmpdir(), 'atlas-rollback-cli-')); + const config = join(cwd, 'config'); + const home = join(cwd, 'home'); + const skill = join(home, '.agents', 'skills', 'x'); + mkdirSync(skill, { recursive: true }); + writeFileSync(join(skill, 'SKILL.md'), '---\nname: x\n---\nsafe'); + writeFileSync(join(home, '.agents', '.skill-lock.json'), JSON.stringify({ version: 3, skills: { + x: { source: 'o/r', sourceType: 'github', skillPath: 'skills/x/SKILL.md', skillFolderHash: 'hB', updatedAt: '2026-10-01T00:00:00Z' }, + } })); + const preload = join(cwd, 'fetch.mjs'); + writeFileSync(preload, `globalThis.fetch = async url => { + if (String(url).startsWith('https://raw.githubusercontent.com/o/r/${A}/')) return new Response('---\\nname: x\\n---\\nsafe'); + return new Response('', { status: 404 }); + };`); + const historyFile = join(config, 'skills-atlas', 'history.json'); + mkdirSync(join(config, 'skills-atlas'), { recursive: true }); + pushRevision('o/r@x', A, historyFile); + const tsx = pathToFileURL(createRequire(import.meta.url).resolve('tsx')).href; + try { + const res = spawnSync(process.execPath, ['--import', tsx, '--import', pathToFileURL(preload).href, fileURLToPath(new URL('../src/cli.tsx', import.meta.url)), 'rollback', 'o/r@x', '--dry-run', '--symlink'], { + cwd, encoding: 'utf8', env: { ...process.env, SKILLS_ATLAS_LANG: 'en', SKILLS_ATLAS_OFFLINE: '1', XDG_CONFIG_HOME: config, HOME: home, USERPROFILE: home }, + }); + expect(res.stdout + res.stderr).toContain(`https://github.com/o/r/tree/${A}`); + expect(readHistory(historyFile)['o/r@x']!.map((r) => r.revision)).toEqual([A]); + expect(existsSync(join(config, 'skills-atlas', 'revisions.json'))).toBe(false); + } finally { rmSync(cwd, { recursive: true, force: true }); } +}, 60_000); + +it('remembers nothing for a skill the lock does not have', () => { + const dir = mkdtempSync(join(tmpdir(), 'atlas-rollback-')); + try { + writeFileSync(join(dir, 'lock.json'), JSON.stringify({ skills: {} })); + rememberRevision('o/r@x', C, join(dir, 'lock.json'), join(dir, 'revisions.json')); + expect(existsSync(join(dir, 'revisions.json'))).toBe(false); + } finally { rmSync(dir, { recursive: true, force: true }); } +}); From dd19fd527f90886e720bbf4b6a262763ff2ca3b3 Mon Sep 17 00:00:00 2001 From: DevOhMyCode Date: Fri, 23 Oct 2026 10:29:46 +0200 Subject: [PATCH 04/11] fix(catalog): check a catalog's shape when it loads --- src/catalog/load.ts | 53 +++++++++++++++++++++++++++++++++++++++++--- test/catalog.test.ts | 36 ++++++++++++++++++++++++++++++ test/remote.test.ts | 12 ++++++++++ 3 files changed, 98 insertions(+), 3 deletions(-) diff --git a/src/catalog/load.ts b/src/catalog/load.ts index 110bcba..bff1720 100644 --- a/src/catalog/load.ts +++ b/src/catalog/load.ts @@ -3,8 +3,51 @@ import { readFile, writeFile } from 'node:fs/promises'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { gunzipSync, gzipSync } from 'node:zlib'; +import { z } from 'zod'; +import { parseSkillId } from '../install/plan.js'; import type { Catalog } from '../types.js'; +// The shape of `Catalog` (src/types.ts). Unknown fields are kept, so that newer snapshots still load. +const str = z.string(); +const num = z.number(); +const Skill = z.looseObject({ + id: str, name: str, source: str, labels: z.array(str), origins: z.array(z.enum(['claude-plugins.dev', 'smithery', 'skills.sh'])), themes: z.array(str), + path: str.optional(), description: str.optional(), installs: num.optional(), stars: num.optional(), rank: num.optional(), growth: num.optional(), + requires: z.looseObject({ bins: z.array(str).optional(), compatibility: str.optional() }).optional(), + updatedAt: str.optional(), growthSuspect: num.optional(), + repo: z.looseObject({ license: str.optional(), archived: z.boolean().optional(), pushedAt: str.optional() }).optional(), + official: z.boolean().optional(), + recommendation: z.looseObject({ repository: str, reason: str, order: num }).optional(), +}); + +/** An entry `install` would refuse (a name read as a flag, a malformed source) or whose id is not its source@name. */ +function installable(s: { id: string; source: string; name: string }): boolean { + try { + const { source, name } = parseSkillId(s.id); + return source === s.source && name === s.name; + } catch { + return false; + } +} +const Theme = z.looseObject({ + id: str, label: str, description: str.optional(), + pack: z.looseObject({ available: num, total: num, missing: z.array(str) }).optional(), +}); +const CatalogSchema = z.looseObject({ + version: z.literal(1), + generatedAt: str.min(1), + themes: z.array(Theme), + skills: z.array(Skill), + stats: z.looseObject({ + sources: z.record(str, z.looseObject({ status: z.enum(['ok', 'error']), count: num, error: str.optional() })).optional(), + raw: z.record(str, num), + merged: num, + official: num.optional(), + growthSince: str.optional(), + byTheme: z.record(str, num), + }), +}); + /** Default snapshot: `data/catalog.json.gz`, found next to `dist/` or `src/catalog/`. */ export function defaultCatalogPath(): string { let dir = dirname(fileURLToPath(import.meta.url)); @@ -19,9 +62,13 @@ export function defaultCatalogPath(): string { export function decodeCatalog(buf: Buffer): Catalog { // gzip magic number: 1f 8b const text = buf[0] === 0x1f && buf[1] === 0x8b ? gunzipSync(buf).toString('utf8') : buf.toString('utf8'); - const catalog = JSON.parse(text) as Catalog; - if (catalog.version !== 1 || !Array.isArray(catalog.skills)) throw new Error('unsupported catalog format'); - return catalog; + const parsed = CatalogSchema.safeParse(JSON.parse(text)); + if (!parsed.success) { + const issues = parsed.error.issues.slice(0, 3).map((i) => `${i.path.join('.') || '(root)'}: ${i.message}`); + throw new Error(`invalid catalog: ${issues.join('; ')}${parsed.error.issues.length > 3 ? ` (+${parsed.error.issues.length - 3} more)` : ''}`); + } + // Entries that could never install are left out rather than failing the whole catalog. + return { ...parsed.data, skills: parsed.data.skills.filter(installable) } as Catalog; } /** Loads the catalog from a local path (`.json` or `.json.gz`) or an http(s) URL. */ diff --git a/test/catalog.test.ts b/test/catalog.test.ts index d96d273..bd5a155 100644 --- a/test/catalog.test.ts +++ b/test/catalog.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from 'vitest'; +import { decodeCatalog, loadCatalog } from '../src/catalog/load.js'; import { assemble } from '../src/catalog/build.js'; import { classify } from '../src/catalog/classify.js'; import { compareSkills, dedupe } from '../src/catalog/dedupe.js'; @@ -398,3 +399,38 @@ describe('i18n', () => { } }); }); + +describe('decodeCatalog', () => { + const skill = { id: 'o/r@x', name: 'x', source: 'o/r', labels: [], origins: ['skills.sh'], themes: ['documents'] }; + const valid = { version: 1, generatedAt: '2026-10-01', themes: [{ id: 'documents', label: 'Documents' }], skills: [skill], stats: { raw: { 'skills.sh': 1 }, merged: 1, byTheme: { documents: 1 } } }; + const decode = (c: object) => decodeCatalog(Buffer.from(JSON.stringify(c))); + + it('accepts an old snapshot without recent metadata, and keeps unknown fields', () => { + const got = decode({ ...valid, future: 1, skills: [{ ...skill, later: true }] }); + expect(got.skills).toHaveLength(1); + expect((got as any).future).toBe(1); + expect((got.skills[0] as any).later).toBe(true); + expect(decode({ ...valid, skills: [] }).skills).toEqual([]); + }); + + it('rejects missing or mistyped required fields with the path', () => { + expect(() => decode({ version: 1, skills: [{}] })).toThrow(/invalid catalog/); + expect(() => decode({ ...valid, version: 2 })).toThrow(/version/); + expect(() => decode({ ...valid, generatedAt: undefined })).toThrow(/generatedAt/); + expect(() => decode({ ...valid, themes: [{ id: 'x' }] })).toThrow(/themes\.0\.label/); + expect(() => decode({ ...valid, stats: { raw: {}, byTheme: {} } })).toThrow(/stats\.merged/); + expect(() => decode({ ...valid, skills: [{ ...skill, labels: 'a' }] })).toThrow(/skills\.0\.labels/); + expect(() => decode({ ...valid, skills: [{ ...skill, origins: ['elsewhere'] }] })).toThrow(/skills\.0\.origins/); + expect(() => decode({ ...valid, skills: [{ ...skill, installs: '12' }] })).toThrow(/skills\.0\.installs/); + expect(() => decode({ ...valid, skills: [{ ...skill, repo: { archived: 'yes' } }] })).toThrow(/skills\.0\.repo\.archived/); + }); + + it('leaves out entries install would refuse or whose id is not source@name', () => { + const bad = [{ ...skill, id: 'o/r@-x', name: '-x' }, { ...skill, id: 'o/r@y' }, { ...skill, id: 'o/r/z@x', source: 'o/r/z' }]; + expect(decode({ ...valid, skills: [...bad, skill] }).skills.map((s) => s.id)).toEqual(['o/r@x']); + }); + + it('loads the bundled snapshot', async () => { + expect((await loadCatalog()).skills.length).toBeGreaterThan(1000); + }); +}); diff --git a/test/remote.test.ts b/test/remote.test.ts index e88f313..a86b1aa 100644 --- a/test/remote.test.ts +++ b/test/remote.test.ts @@ -93,4 +93,16 @@ describe('latest catalog', () => { const got = await loadLatestCatalog({ dir: join(dir, 'cache'), bundledPath, fetch, now: 0 }); expect(got.generatedAt).toBe('2026-09-01T00:00:00Z'); }); + + it('rejects a download with the wrong shape before caching it, and keeps a valid cache', async () => { + const opts = { dir: join(dir, 'cache'), bundledPath, now: 0 }; + await loadLatestCatalog({ ...opts, fetch: async () => ok(catalog('2026-09-27T00:00:00Z')) }); + const cached = readFileSync(join(dir, 'cache', 'catalog.json.gz')); + const bad = { ...catalog('2026-10-01T00:00:00Z'), skills: [{}] }; + const failed: string[] = []; + const got = await loadLatestCatalog({ ...opts, refresh: true, onError: (e: Error) => failed.push(e.message), fetch: async () => new Response(gzipSync(JSON.stringify(bad)), { status: 200 }) }); + expect(got.generatedAt).toBe('2026-09-27T00:00:00Z'); + expect(failed[0]).toMatch(/invalid catalog: skills\.0\.id/); + expect(readFileSync(join(dir, 'cache', 'catalog.json.gz'))).toEqual(cached); + }); }); From d9ccf8f995120d77ed3a4f565785ba588c4cd336 Mon Sep 17 00:00:00 2001 From: DevOhMyCode Date: Fri, 23 Oct 2026 10:31:00 +0200 Subject: [PATCH 05/11] fix(crawl): refuse to replace the snapshot with an empty catalog --- src/catalog/build.ts | 7 +++++-- test/catalog-health.test.ts | 16 ++++++++++++++-- test/crawl.test.ts | 27 +++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 4 deletions(-) create mode 100644 test/crawl.test.ts diff --git a/src/catalog/build.ts b/src/catalog/build.ts index ed4ec0a..42b8ec4 100644 --- a/src/catalog/build.ts +++ b/src/catalog/build.ts @@ -52,7 +52,7 @@ export function assemble( /** * Crawls every source. A failing source is logged and skipped so that one - * registry being down does not block the snapshot; all of them failing does. + * registry being down does not block the snapshot; all of them failing, or no skill at all, does. */ export async function buildCatalog(opts: BuildOptions = {}): Promise { const log = opts.log ?? (() => {}); @@ -90,5 +90,8 @@ export async function buildCatalog(opts: BuildOptions = {}): Promise { merged = merged.filter((s) => !missing.has(s)); log(`enrich: ${filled} descriptions filled, ${missing.size} skills gone from their repository dropped`); } - return assemble(raw, merged, await officialOwners, sources); + const catalog = assemble(raw, merged, await officialOwners, sources); + // Sources that answer nothing (or skills all dropped) must not replace the previous snapshot with an empty one. + if (!catalog.skills.length) throw new Error('the crawl found no skill'); + return catalog; } diff --git a/test/catalog-health.test.ts b/test/catalog-health.test.ts index f577412..9960cf4 100644 --- a/test/catalog-health.test.ts +++ b/test/catalog-health.test.ts @@ -9,8 +9,10 @@ vi.mock('../src/catalog/sources/smithery.js', () => ({ fetchSmithery: vi.fn() }) vi.mock('../src/catalog/sources/claude-plugins-dev.js', () => ({ fetchClaudePluginsDev: vi.fn() })); vi.mock('../src/catalog/sources/official.js', () => ({ fetchOfficialOwners: vi.fn(async () => undefined), markOfficial: vi.fn() })); +const one = [{ origin: 'skills.sh' as const, name: 'pdf', source: 'anthropics/skills', installs: 10 }]; + beforeEach(() => { - vi.mocked(fetchSkillsSh).mockResolvedValue([]); + vi.mocked(fetchSkillsSh).mockResolvedValue(one); vi.mocked(fetchSmithery).mockResolvedValue([]); vi.mocked(fetchClaudePluginsDev).mockResolvedValue([]); }); @@ -20,7 +22,7 @@ describe('catalog source health', () => { vi.mocked(fetchSmithery).mockRejectedValue(new Error('Registry unavailable')); const catalog = await buildCatalog(); expect(catalog.stats.sources).toEqual({ - 'skills.sh': { status: 'ok', count: 0 }, + 'skills.sh': { status: 'ok', count: 1 }, 'claude-plugins.dev': { status: 'ok', count: 0 }, smithery: { status: 'error', count: 0, error: 'Registry unavailable' }, }); @@ -33,4 +35,14 @@ describe('catalog source health', () => { vi.mocked(fetchClaudePluginsDev).mockRejectedValue(new Error('down')); await expect(buildCatalog()).rejects.toThrow('every catalog source failed'); }); + + it('refuses a collection without any skill: one source empty and the others down, or all empty', async () => { + vi.mocked(fetchSkillsSh).mockResolvedValue([]); + vi.mocked(fetchSmithery).mockRejectedValue(new Error('down')); + vi.mocked(fetchClaudePluginsDev).mockRejectedValue(new Error('down')); + await expect(buildCatalog()).rejects.toThrow('the crawl found no skill'); + vi.mocked(fetchSmithery).mockResolvedValue([]); + vi.mocked(fetchClaudePluginsDev).mockResolvedValue([]); + await expect(buildCatalog()).rejects.toThrow('the crawl found no skill'); + }); }); diff --git a/test/crawl.test.ts b/test/crawl.test.ts new file mode 100644 index 0000000..86e15de --- /dev/null +++ b/test/crawl.test.ts @@ -0,0 +1,27 @@ +import { createRequire } from 'node:module'; +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; +import { expect, it } from 'vitest'; + +it('a crawl that finds no skill fails and leaves the previous snapshot byte for byte', () => { + const cwd = mkdtempSync(join(tmpdir(), 'atlas-crawl-')); + const out = join(cwd, 'catalog.json.gz'); + const previous = Buffer.from('previous snapshot'); + writeFileSync(out, previous); + // claude-plugins.dev answers an empty list; every other source is gone. + const preload = join(cwd, 'fetch.mjs'); + writeFileSync(preload, `globalThis.fetch = async (url) => String(url).startsWith('https://claude-plugins.dev/api/skills') + ? Response.json({ skills: [], total: 0 }) : new Response('gone', { status: 404 });`); + const tsx = pathToFileURL(createRequire(import.meta.url).resolve('tsx')).href; + try { + const res = spawnSync(process.execPath, ['--import', tsx, '--import', pathToFileURL(preload).href, fileURLToPath(new URL('../scripts/crawl.ts', import.meta.url)), '--enrich', '0', '--requires', '0', '--out', out], { + cwd, encoding: 'utf8', env: { ...process.env, GITHUB_TOKEN: 'test' }, + }); + expect(res.status).not.toBe(0); + expect(res.stderr).toContain('the crawl found no skill'); + expect(readFileSync(out)).toEqual(previous); + } finally { rmSync(cwd, { recursive: true, force: true }); } +}, 60_000); From 27278ee8cae73dadbf36141a498362f1a3388a45 Mon Sep 17 00:00:00 2001 From: DevOhMyCode Date: Fri, 23 Oct 2026 10:37:29 +0200 Subject: [PATCH 06/11] fix(update): report repositories it could not check instead of up to date --- src/agent-api.ts | 9 +++++---- src/cli.tsx | 21 ++++++++++++++++---- src/i18n/de.ts | 1 + src/i18n/en.ts | 1 + src/i18n/es.ts | 1 + src/i18n/fr.ts | 1 + src/i18n/it.ts | 1 + src/i18n/ja.ts | 1 + src/i18n/ko.ts | 1 + src/i18n/pt.ts | 1 + src/i18n/zh.ts | 1 + src/install/updates.ts | 43 ++++++++++++++++++++++++++++++---------- src/mcp.ts | 10 +++++----- src/ui/App.tsx | 8 ++++++-- test/mcp.test.ts | 11 ++++++++++ test/remote.test.ts | 2 +- test/ui.test.tsx | 3 ++- test/updates-cli.test.ts | 41 ++++++++++++++++++++++++++++++++++++++ test/updates.test.ts | 41 +++++++++++++++++++++++++++++--------- 19 files changed, 162 insertions(+), 36 deletions(-) create mode 100644 test/updates-cli.test.ts diff --git a/src/agent-api.ts b/src/agent-api.ts index d70184a..c8f394f 100644 --- a/src/agent-api.ts +++ b/src/agent-api.ts @@ -4,7 +4,7 @@ import { auditSkills } from './audit.js'; import { fetchSkillFiles, fetchSkillMarkdown, githubUrl } from './github.js'; import { parseSkillId } from './install/plan.js'; import { listInstalled } from './install/run.js'; -import { findUpdates } from './install/updates.js'; +import { findUpdates, type UpdateCheck } from './install/updates.js'; import type { Risk } from './risks.js'; import type { Catalog, Scope, Skill } from './types.js'; @@ -97,9 +97,10 @@ export function skillsByAgent(installed: InstalledSummary[]): { agent: string; n /** * Global skills with a newer version on GitHub. Updating one is installing it - * again: `skills-atlas install -g -a -y`. + * again: `skills-atlas install -g -a -y`. `unchecked` lists what + * could not be checked: the check is complete only when it is empty. */ -export async function outdatedSkills(): Promise { +export async function outdatedSkills(): Promise<{ skills: InstalledSummary[]; unchecked: UpdateCheck['unchecked'] }> { const [found, installed] = await Promise.all([findUpdates(), installedSkills(['global'])]); - return installed.filter((s) => s.source && found.has(`${s.source}@${s.name}`.toLowerCase())); + return { skills: installed.filter((s) => s.source && found.outdated.has(`${s.source}@${s.name}`.toLowerCase())), unchecked: found.unchecked }; } diff --git a/src/cli.tsx b/src/cli.tsx index 5be820b..d7d70fa 100644 --- a/src/cli.tsx +++ b/src/cli.tsx @@ -754,15 +754,24 @@ program } }); +/** Names, on stderr, what an update check could not check, and sets exit code 1. */ +function reportUnchecked(unchecked: { source: string; reason: string }[]): void { + if (!unchecked.length) return; + process.exitCode = 1; + console.error(paint('yellow', c.updatesUnchecked(unchecked.length))); + for (const u of unchecked) console.error(paint('dim', ` ? ${u.source}: ${u.reason}`)); +} + installOptions(program.command('update')) .argument('[ids...]', c.ids) .description(c.updateCmd) .action(async (ids: string[], opts: InstallFlags) => { - const outdated = await outdatedSkills(); + const { skills: outdated, unchecked } = await outdatedSkills(); + reportUnchecked(unchecked); const wanted = ids.map((id) => id.toLowerCase()); // Named skills are updated only if they are outdated: a reinstall of an up-to-date one would change nothing. const chosen = ids.length ? outdated.filter((s) => wanted.includes(s.id.toLowerCase())) : outdated; - if (!chosen.length) return console.log(paint('dim', c.upToDate)); + if (!chosen.length) return unchecked.length ? undefined : console.log(paint('dim', c.upToDate)); // What each update changes, and the warning signs it brings, before anything runs. const local = await listInstalled('global'); const catalog = await loadLatestCatalog({ offline: true }).catch(() => undefined); @@ -782,6 +791,8 @@ installOptions(program.command('update')) // Each skill keeps its own agents; these options only fill in what a skill's scan did not name. const options: InstallOptions = { agents: defaults.agents, scope: 'global', method: opts.copy ? 'copy' : opts.symlink ? 'symlink' : defaults.method, skillsVersion: opts.skillsVersion }; await apply([], [], options, opts, chosen.map((s) => ({ id: s.id, agents: s.agents }))); + // The confirmed updates ran; the rest could not be checked, so this is not a full success. + if (unchecked.length) console.error(paint('yellow', c.updatesUnchecked(unchecked.length))); }); installOptions(program.command('rollback')) @@ -821,9 +832,11 @@ program .description(c.outdatedCmd) .option('--json', c.json) .action(async (opts: { json?: boolean }) => { - const outdated = await outdatedSkills(); + const { skills: outdated, unchecked } = await outdatedSkills(); + // stdout keeps the array scripts read; what could not be checked goes to stderr, with exit code 1. + reportUnchecked(unchecked); if (opts.json) return void process.stdout.write(`${JSON.stringify(outdated, null, 2)}\n`); - if (!outdated.length) return console.log(paint('dim', c.upToDate)); + if (!outdated.length) return unchecked.length ? undefined : console.log(paint('dim', c.upToDate)); for (const s of outdated) console.log(` ${padEnd(s.id, 60)} ${paint('dim', s.agents.join(', '))}`); console.log(paint('dim', `\n${c.outdatedHint}`)); console.log(paint('dim', c.updateAllHint)); diff --git a/src/i18n/de.ts b/src/i18n/de.ts index 5c7492a..c4290a5 100644 --- a/src/i18n/de.ts +++ b/src/i18n/de.ts @@ -286,6 +286,7 @@ export const de: Messages = { outdatedCmd: "listet globale Skills mit neuerer Version auf GitHub", showFailed: (id, msg) => `SKILL.md von ${id} nicht lesbar: ${msg}`, upToDate: "Alle globalen Skills sind aktuell.", + updatesUnchecked: (n) => `Update-Prüfung unvollständig: ${n} Repository-Prüfung(en) fehlgeschlagen, diese Skills sind womöglich veraltet.`, outdatedHint: "Zum Aktualisieren neu installieren: skills-atlas install -g -a -y (oder u im Baum).", updateCmd: "aktualisiert veraltete globale Skills: alle oder die genannten", updateAllHint: "Alle aktualisieren: skills-atlas update", diff --git a/src/i18n/en.ts b/src/i18n/en.ts index 34c9821..f60090d 100644 --- a/src/i18n/en.ts +++ b/src/i18n/en.ts @@ -284,6 +284,7 @@ export const en = { outdatedCmd: "list global skills with a newer version on GitHub", showFailed: (id: string, msg: string) => `Cannot read the SKILL.md of ${id}: ${msg}`, upToDate: "Every global skill is up to date.", + updatesUnchecked: (n: number) => `Update check incomplete: ${n} repository check(s) failed, so those skills may be outdated.`, outdatedHint: "Update one by installing it again: skills-atlas install -g -a -y (or press u in the tree).", updateCmd: "update outdated global skills: all of them, or the ones named", updateAllHint: "Update them all: skills-atlas update", diff --git a/src/i18n/es.ts b/src/i18n/es.ts index 13038a5..4357850 100644 --- a/src/i18n/es.ts +++ b/src/i18n/es.ts @@ -286,6 +286,7 @@ export const es: Messages = { outdatedCmd: "lista las skills globales con una versión más reciente en GitHub", showFailed: (id, msg) => `No se puede leer el SKILL.md de ${id}: ${msg}`, upToDate: "Todas las skills globales están al día.", + updatesUnchecked: (n) => `Comprobación de actualizaciones incompleta: ${n} comprobación(es) de repositorio fallaron, esas skills pueden estar desactualizadas.`, outdatedHint: "Para actualizar una, reinstálala: skills-atlas install -g -a -y (o pulsa u en el árbol).", updateCmd: "actualiza las skills globales obsoletas: todas, o las indicadas", updateAllHint: "Actualizar todas: skills-atlas update", diff --git a/src/i18n/fr.ts b/src/i18n/fr.ts index ca77c7a..6c34d7e 100644 --- a/src/i18n/fr.ts +++ b/src/i18n/fr.ts @@ -286,6 +286,7 @@ export const fr: Messages = { outdatedCmd: "liste les skills globaux qui ont une version plus récente sur GitHub", showFailed: (id, msg) => `Impossible de lire le SKILL.md de ${id} : ${msg}`, upToDate: "Tous les skills globaux sont à jour.", + updatesUnchecked: (n) => `Vérification des mises à jour incomplète : ${n} contrôle(s) de dépôt impossible(s), ces skills sont peut-être obsolètes.`, outdatedHint: "Pour en mettre un à jour, réinstallez-le : skills-atlas install -g -a -y (ou touche u dans l'arbre).", updateCmd: "met à jour les skills globaux obsolètes : tous, ou ceux indiqués", updateAllHint: "Tout mettre à jour : skills-atlas update", diff --git a/src/i18n/it.ts b/src/i18n/it.ts index 74f2090..eba6c68 100644 --- a/src/i18n/it.ts +++ b/src/i18n/it.ts @@ -286,6 +286,7 @@ export const it: Messages = { outdatedCmd: "elenca le skill globali con una versione più recente su GitHub", showFailed: (id, msg) => `Impossibile leggere lo SKILL.md di ${id}: ${msg}`, upToDate: "Tutte le skill globali sono aggiornate.", + updatesUnchecked: (n) => `Controllo aggiornamenti incompleto: ${n} controllo/i di repository non riuscito/i, quelle skill potrebbero essere obsolete.`, outdatedHint: "Per aggiornarne una, reinstallala: skills-atlas install -g -a -y (o premi u nell'albero).", updateCmd: "aggiorna le skill globali obsolete: tutte, o quelle indicate", updateAllHint: "Aggiornarle tutte: skills-atlas update", diff --git a/src/i18n/ja.ts b/src/i18n/ja.ts index e43f0ef..dbdad8c 100644 --- a/src/i18n/ja.ts +++ b/src/i18n/ja.ts @@ -284,6 +284,7 @@ export const ja: Messages = { outdatedCmd: "GitHub に新しいバージョンがあるグローバルスキルを一覧表示", showFailed: (id, msg) => `${id} の SKILL.md を読めません: ${msg}`, upToDate: "すべてのグローバルスキルは最新です。", + updatesUnchecked: (n) => `更新の確認が不完全です: ${n} 件のリポジトリを確認できなかったため、その skill は古い可能性があります。`, outdatedHint: "更新するには再インストール: skills-atlas install -g -a -y（ツリーでは u キー）", updateCmd: "古いグローバルスキルを更新（すべて、または指定したもの）", updateAllHint: "すべて更新: skills-atlas update", diff --git a/src/i18n/ko.ts b/src/i18n/ko.ts index eda5fc3..a88c84b 100644 --- a/src/i18n/ko.ts +++ b/src/i18n/ko.ts @@ -284,6 +284,7 @@ export const ko: Messages = { outdatedCmd: "GitHub에 새 버전이 있는 전역 스킬 목록", showFailed: (id, msg) => `${id}의 SKILL.md를 읽을 수 없습니다: ${msg}`, upToDate: "모든 전역 스킬이 최신입니다.", + updatesUnchecked: (n) => `업데이트 확인이 불완전합니다: 저장소 ${n}곳을 확인하지 못해 해당 skill이 오래되었을 수 있습니다.`, outdatedHint: "업데이트하려면 다시 설치: skills-atlas install -g -a -y (트리에서는 u)", updateCmd: "오래된 전역 스킬 업데이트: 전부 또는 지정한 것", updateAllHint: "모두 업데이트: skills-atlas update", diff --git a/src/i18n/pt.ts b/src/i18n/pt.ts index a424f9d..3a3c048 100644 --- a/src/i18n/pt.ts +++ b/src/i18n/pt.ts @@ -286,6 +286,7 @@ export const pt: Messages = { outdatedCmd: "lista as skills globais com versão mais recente no GitHub", showFailed: (id, msg) => `Não foi possível ler o SKILL.md de ${id}: ${msg}`, upToDate: "Todas as skills globais estão atualizadas.", + updatesUnchecked: (n) => `Verificação de atualizações incompleta: ${n} verificação(ões) de repositório falharam, essas skills podem estar desatualizadas.`, outdatedHint: "Para atualizar uma, reinstale-a: skills-atlas install -g -a -y (ou tecle u na árvore).", updateCmd: "atualiza as skills globais desatualizadas: todas, ou as indicadas", updateAllHint: "Atualizar todas: skills-atlas update", diff --git a/src/i18n/zh.ts b/src/i18n/zh.ts index 88fa24e..95b0281 100644 --- a/src/i18n/zh.ts +++ b/src/i18n/zh.ts @@ -284,6 +284,7 @@ export const zh: Messages = { outdatedCmd: "列出 GitHub 上有新版本的全局技能", showFailed: (id, msg) => `无法读取 ${id} 的 SKILL.md：${msg}`, upToDate: "所有全局技能均为最新。", + updatesUnchecked: (n) => `更新检查不完整：${n} 个仓库无法检查，这些 skill 可能已过时。`, outdatedHint: "更新方法是重新安装：skills-atlas install -g -a -y（或在树中按 u）", updateCmd: "更新过时的全局技能：全部，或指定的技能", updateAllHint: "全部更新：skills-atlas update", diff --git a/src/install/updates.ts b/src/install/updates.ts index 780e32d..569678b 100644 --- a/src/install/updates.ts +++ b/src/install/updates.ts @@ -2,7 +2,7 @@ import { execFileSync } from 'node:child_process'; import { readFileSync } from 'node:fs'; import pLimit from 'p-limit'; import { z } from 'zod'; -import { fetchJson } from '../catalog/http.js'; +import { fetchJson, HttpError } from '../catalog/http.js'; import { lockPath } from './scan.js'; /** A skill of the global lock file, with the git tree SHA of its folder when it was installed. */ @@ -30,7 +30,13 @@ const Lock = z.object({ updatedAt: z.string().optional(), })), }); -const Tree = z.object({ tree: z.array(z.object({ path: z.string(), type: z.string(), sha: z.string() })) }); +const Tree = z.object({ sha: z.string(), truncated: z.boolean().optional(), tree: z.array(z.object({ path: z.string(), type: z.string(), sha: z.string() })) }); + +/** What an update check found: confirmed updates, and the repositories (or skills) it could not check. */ +export interface UpdateCheck { + outdated: Set; + unchecked: { source: string; reason: string }[]; +} /** * GitHub skills of a lock file that recorded their folder hash. Only the global @@ -57,7 +63,7 @@ export function outdated(skills: LockedSkill[], trees: Map(); for (const s of skills) { const sha = trees.get(s.source)?.get(s.dir); - // No SHA: repository unreachable, skill moved, or a skill at the repository root. + // No SHA: repository unreachable or skill moved, reported by findUpdates as unchecked. if (sha && sha !== s.hash) out.add(`${s.source}@${s.name}`.toLowerCase()); } return out; @@ -74,15 +80,26 @@ export function githubToken(): string | undefined { } } +/** Why a repository could not be checked, without the URL or any header. */ +function reason(err: unknown): string { + if (err instanceof HttpError) return `HTTP ${err.status}`; + if (err instanceof z.ZodError || err instanceof SyntaxError) return 'invalid response'; + if (err instanceof Error && (err.name === 'TimeoutError' || err.name === 'AbortError')) return 'timed out'; + return 'unreachable'; +} + /** * Globally installed skills with a newer version on GitHub: one Trees API call - * per repository. A repository that cannot be read reports nothing. + * per repository. A repository that cannot be read, a truncated tree or a skill + * folder GitHub no longer has is reported as unchecked, never as up to date. * shortcut: without a token GitHub allows 60 calls an hour, so many repositories - * checked twice in an hour go silent; cache the trees if that gets in the way. + * checked twice in an hour fail with HTTP 403; cache the trees if that gets in the way. */ -export async function findUpdates(path = lockPath('global', process.cwd()), token = githubToken()): Promise> { +export async function findUpdates(path = lockPath('global', process.cwd()), token = githubToken()): Promise { const skills = readLockedSkills(path); const trees = new Map>(); + const truncated = new Set(); + const unchecked: UpdateCheck['unchecked'] = []; const limit = pLimit(8); await Promise.all( [...new Set(skills.map((s) => s.source))].map((source) => @@ -95,12 +112,18 @@ export async function findUpdates(path = lockPath('global', process.cwd()), toke headers: { accept: 'application/vnd.github+json', ...(token ? { authorization: `Bearer ${token}` } : {}) }, }), ); - trees.set(source, new Map(tree.tree.filter((e) => e.type === 'tree').map((e) => [e.path, e.sha]))); - } catch { - // Unreachable, private or rate limited: no news for this repository. + // '' is the repository root, for a skill whose SKILL.md sits there. + trees.set(source, new Map([['', tree.sha], ...tree.tree.filter((e) => e.type === 'tree').map((e) => [e.path, e.sha] as [string, string])])); + if (tree.truncated) truncated.add(source); + } catch (err) { + unchecked.push({ source, reason: reason(err) }); } }), ), ); - return outdated(skills, trees); + for (const s of skills) { + const tree = trees.get(s.source); + if (tree && !tree.has(s.dir)) unchecked.push({ source: s.source, reason: `${s.name}: ${truncated.has(s.source) ? 'tree truncated' : `${s.dir} not found`}` }); + } + return { outdated: outdated(skills, trees), unchecked }; } diff --git a/src/mcp.ts b/src/mcp.ts index fed5d8a..55106ba 100644 --- a/src/mcp.ts +++ b/src/mcp.ts @@ -77,7 +77,7 @@ const TOOLS = { annotations: { readOnlyHint: true }, }, outdated_skills: { - description: 'List global skills with a newer version on GitHub, with the lines the update changes in SKILL.md and the warning signs it adds (new_risks). Tell the user about new_risks before updating. Update one with install_skills, scope global, with its own agents.', + description: 'List global skills with a newer version on GitHub, as { skills, unchecked }: each skill with the lines the update changes in SKILL.md and the warning signs it adds (new_risks); unchecked lists the repositories that could not be checked (source, reason), so the check is complete only when it is empty. Tell the user about new_risks and unchecked before updating. Update one with install_skills, scope global, with its own agents.', schema: z.object({}), inputSchema: { type: 'object', properties: {} }, annotations: { readOnlyHint: true, openWorldHint: true }, @@ -134,14 +134,14 @@ async function callTool(name: ToolName, args: unknown, opts: McpOptions, catalog return json(await (opts.installed ?? installedSkills)(input.scope ? [input.scope as Scope] : SCOPES)); case 'outdated_skills': { // Each update with what it changes: line counts and the warning signs the new version adds. - const outdated = await outdatedSkills(); - const local = await listInstalled('global'); + const { skills: outdated, unchecked } = await outdatedSkills(); + const local = outdated.length ? await listInstalled('global') : []; const c = await catalog(); const known = new Map(c.skills.map((s) => [s.id.toLowerCase(), s])); - return json(await Promise.all(outdated.map(async (s) => { + return json({ unchecked, skills: await Promise.all(outdated.map(async (s) => { const diff = await updateDiff(auditTarget(s.id, known), local).catch(() => undefined); return diff ? { ...s, lines_added: diff.added, lines_removed: diff.removed, new_risks: diff.risks.map(({ rule, level, label, line, excerpt }) => ({ rule, level, label, line, excerpt })) } : s; - }))); + })) }); } case 'install_skills': { for (const id of input.ids as string[]) parseSkillId(id); diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 898fc35..d10e71b 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -15,6 +15,7 @@ import { isMouseInput } from './mouse.js'; import { padEnd } from './width.js'; import { scopesLabel, TreeSelect, type TreeMemory } from './TreeSelect.js'; import { TERMINAL_PALETTE as PALETTE } from './palette.js'; +import type { UpdateCheck } from '../install/updates.js'; export interface WizardResult { /** Skills to install (`owner/repo@name`). */ @@ -49,7 +50,7 @@ export interface AppProps { /** Shared by the successive trees of one session: open groups and focused row. */ treeMemory?: TreeMemory; /** `owner/repo@name` (lower case) of the installed skills with a newer version; checked in the background. */ - findUpdates?: () => Promise>; + findUpdates?: () => Promise; /** Run the security audit on the review screen (on unless turned off). */ audit?: boolean; /** Reads a skill's SKILL.md for the audit; replaced in tests. */ @@ -104,6 +105,7 @@ export function App(props: AppProps) { const [removals, setRemovals] = useState([]); const [updates, setUpdates] = useState([]); const [outdated, setOutdated] = useState>(new Set()); + const [unchecked, setUnchecked] = useState(0); const [agents, setAgents] = useState(preset.agents); const [scope, setScope] = useState(preset.scope); const [method, setMethod] = useState(preset.method); @@ -163,7 +165,8 @@ export function App(props: AppProps) { .then((found) => { if (!live) return; const global = [...tree.installed].filter(([, e]) => e.scopes.includes('global')); - setOutdated(new Set(global.map(([id]) => id).filter((id) => found.has(id.toLowerCase())))); + setOutdated(new Set(global.map(([id]) => id).filter((id) => found.outdated.has(id.toLowerCase())))); + setUnchecked(found.unchecked.length); }) .catch(() => {}); return () => { @@ -191,6 +194,7 @@ export function App(props: AppProps) { {error && {error}} {notice && {notice}} + {unchecked > 0 && {t().cli.updatesUnchecked(unchecked)}} ({ + ...(await original()), + outdatedSkills: vi.fn(async () => ({ skills: [], unchecked: [{ source: 'o/r', reason: 'HTTP 403' }] })), +})); import type { Catalog } from '../src/types.js'; const skill = (id: string, installs: number, themes = ['documents']) => { @@ -125,4 +131,9 @@ describe('MCP server', () => { expect(peak).toBe(1); expect(run).toHaveBeenCalledTimes(3); }); + + it('returns outdated skills with what could not be checked', async () => { + const { call } = server(); + expect(JSON.parse((await call('outdated_skills', {})).text)).toEqual({ unchecked: [{ source: 'o/r', reason: 'HTTP 403' }], skills: [] }); + }); }); diff --git a/test/remote.test.ts b/test/remote.test.ts index a86b1aa..2b731f1 100644 --- a/test/remote.test.ts +++ b/test/remote.test.ts @@ -100,7 +100,7 @@ describe('latest catalog', () => { const cached = readFileSync(join(dir, 'cache', 'catalog.json.gz')); const bad = { ...catalog('2026-10-01T00:00:00Z'), skills: [{}] }; const failed: string[] = []; - const got = await loadLatestCatalog({ ...opts, refresh: true, onError: (e: Error) => failed.push(e.message), fetch: async () => new Response(gzipSync(JSON.stringify(bad)), { status: 200 }) }); + const got = await loadLatestCatalog({ ...opts, refresh: true, onError: (e: unknown) => void failed.push((e as Error).message), fetch: async () => new Response(gzipSync(JSON.stringify(bad)), { status: 200 }) }); expect(got.generatedAt).toBe('2026-09-27T00:00:00Z'); expect(failed[0]).toMatch(/invalid catalog: skills\.0\.id/); expect(readFileSync(join(dir, 'cache', 'catalog.json.gz'))).toEqual(cached); diff --git a/test/ui.test.tsx b/test/ui.test.tsx index c8fe90e..6b12037 100644 --- a/test/ui.test.tsx +++ b/test/ui.test.tsx @@ -602,7 +602,7 @@ describe('installed skills in the wizard', () => { it('flags an outdated skill, marks it with u and reinstalls it for its agents', async () => { let result: WizardResult | null = null; - const findUpdates = vi.fn(async () => new Set(['c/d@pentest'])); + const findUpdates = vi.fn(async () => ({ outdated: new Set(['c/d@pentest']), unchecked: [{ source: 'x/y', reason: 'HTTP 403' }] })); const { lastFrame, stdin } = render( (result = r)} />, @@ -610,6 +610,7 @@ describe('installed skills in the wizard', () => { await tick(); await tick(); expect(lastFrame()).toContain('↑ 1 update (u)'); + expect(lastFrame()).toContain('Update check incomplete: 1 repository check(s) failed'); await press(stdin, KEY.right, KEY.down); expect(lastFrame()).toContain('↑ update available'); await press(stdin, 'u'); diff --git a/test/updates-cli.test.ts b/test/updates-cli.test.ts new file mode 100644 index 0000000..463d8cc --- /dev/null +++ b/test/updates-cli.test.ts @@ -0,0 +1,41 @@ +import { createRequire } from 'node:module'; +import { spawnSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; +import { expect, it } from 'vitest'; + +it('outdated --json keeps the array on stdout, reports what it could not check on stderr, and exits 1', () => { + const cwd = mkdtempSync(join(tmpdir(), 'atlas-updates-cli-')); + const home = join(cwd, 'home'); + const skills = join(home, '.agents', 'skills'); + for (const name of ['x', 'y']) { + mkdirSync(join(skills, name), { recursive: true }); + writeFileSync(join(skills, name, 'SKILL.md'), `---\nname: ${name}\n---\n`); + } + writeFileSync(join(home, '.agents', '.skill-lock.json'), JSON.stringify({ version: 3, skills: { + x: { source: 'o/r', sourceType: 'github', skillPath: 'skills/x/SKILL.md', skillFolderHash: 'old' }, + y: { source: 'down/repo', sourceType: 'github', skillPath: 'skills/y/SKILL.md', skillFolderHash: 'old' }, + } })); + const preload = join(cwd, 'fetch.mjs'); + writeFileSync(preload, `globalThis.fetch = async (url) => String(url).includes('/repos/o/r/') + ? Response.json({ sha: 'root', tree: [{ path: 'skills/x', type: 'tree', sha: 'new' }] }) + : new Response('rate limited', { status: 403 });`); + const tsx = pathToFileURL(createRequire(import.meta.url).resolve('tsx')).href; + const run = (...args: string[]) => spawnSync(process.execPath, ['--import', tsx, '--import', pathToFileURL(preload).href, fileURLToPath(new URL('../src/cli.tsx', import.meta.url)), ...args], { + cwd, encoding: 'utf8', env: { ...process.env, SKILLS_ATLAS_LANG: 'en', SKILLS_ATLAS_OFFLINE: '1', XDG_CONFIG_HOME: join(cwd, 'config'), HOME: home, USERPROFILE: home, GITHUB_TOKEN: 'secret-token' }, + }); + try { + const json = run('outdated', '--json'); + expect(json.status).toBe(1); + expect(JSON.parse(json.stdout).map((s: { id: string }) => s.id)).toEqual(['o/r@x']); + expect(json.stderr).toContain('Update check incomplete'); + expect(json.stderr).toContain('down/repo: HTTP 403'); + expect(json.stderr).not.toContain('secret-token'); + const update = run('update', '--dry-run', '-y', '--no-audit', '--symlink'); + expect(update.status).toBe(1); + expect(update.stdout).toContain('add o/r -s x'); + expect(update.stdout).not.toContain('up to date'); + } finally { rmSync(cwd, { recursive: true, force: true }); } +}, 60_000); diff --git a/test/updates.test.ts b/test/updates.test.ts index 754565d..7dca1d0 100644 --- a/test/updates.test.ts +++ b/test/updates.test.ts @@ -40,21 +40,44 @@ describe('skill updates', () => { expect([...outdated(skills, trees)]).toEqual(['dietrichgebert/ponytail@ponytail']); }); - it('asks GitHub once per repository and survives one that fails', async () => { + const check = async (answer: (url: string) => Response | Promise, skills: object = lock.skills) => { const dir = mkdtempSync(join(tmpdir(), 'atlas-lock-')); const calls: string[] = []; - vi.stubGlobal('fetch', async (url: string) => { - calls.push(url); - if (url.includes('anthropics')) return new Response('', { status: 404 }); - return new Response(JSON.stringify({ tree: [{ path: 'skills/ponytail', type: 'tree', sha: 'new' }] })); - }); + vi.stubGlobal('fetch', async (url: string) => { calls.push(url); return answer(url); }); try { - writeFileSync(join(dir, 'lock.json'), JSON.stringify(lock)); - expect([...(await findUpdates(join(dir, 'lock.json'), 'token'))]).toEqual(['dietrichgebert/ponytail@ponytail']); - expect(calls).toHaveLength(2); + writeFileSync(join(dir, 'lock.json'), JSON.stringify({ ...lock, skills })); + const found = await findUpdates(join(dir, 'lock.json'), 'secret-token'); + return { outdated: [...found.outdated], unchecked: found.unchecked, calls }; } finally { vi.unstubAllGlobals(); rmSync(dir, { recursive: true, force: true }); } + }; + const tree = (...entries: [string, string][]) => Response.json({ sha: 'root', tree: entries.map(([path, sha]) => ({ path, type: 'tree', sha })) }); + + it('asks GitHub once per repository, keeps the confirmed updates, and reports one it could not read', async () => { + const got = await check((url) => (url.includes('anthropics') ? new Response('', { status: 403 }) : tree(['skills/ponytail', 'new']))); + expect(got.outdated).toEqual(['dietrichgebert/ponytail@ponytail']); + expect(got.unchecked).toEqual([{ source: 'anthropics/skills', reason: 'HTTP 403' }]); + expect(got.calls).toHaveLength(2); + expect(JSON.stringify(got.unchecked)).not.toContain('secret-token'); + }); + + it('never reads a failed, invalid, truncated or incomplete answer as up to date', async () => { + const timeout = Object.assign(new Error('The operation was aborted due to timeout'), { name: 'TimeoutError' }); + expect((await check(() => { throw timeout; })).unchecked.map((u) => u.reason)).toEqual(['timed out', 'timed out']); + expect((await check(() => new Response(''))).unchecked.map((u) => u.reason)).toEqual(['invalid response', 'invalid response']); + expect((await check(() => Response.json({ tree: [] }))).unchecked.map((u) => u.reason)).toEqual(['invalid response', 'invalid response']); + const truncated = await check(() => Response.json({ sha: 'root', truncated: true, tree: [] })); + expect(truncated.outdated).toEqual([]); + expect(truncated.unchecked.map((u) => u.reason)).toEqual(['ponytail: tree truncated', 'pdf: tree truncated']); + const moved = await check(() => tree(['skills/pdf', 'same'])); + expect(moved.unchecked).toEqual([{ source: 'DietrichGebert/ponytail', reason: 'ponytail: skills/ponytail not found' }]); + }); + + it('checks a skill at the repository root against the root tree', async () => { + const root = { r: { source: 'o/r', sourceType: 'github', skillPath: 'SKILL.md', skillFolderHash: 'old' } }; + expect(await check(() => tree(), root)).toMatchObject({ outdated: ['o/r@r'], unchecked: [] }); + expect((await check(() => Response.json({ sha: 'old', tree: [] }), root)).outdated).toEqual([]); }); }); From d233846cafc2901356e848de30497278c0e185be Mon Sep 17 00:00:00 2001 From: DevOhMyCode Date: Fri, 23 Oct 2026 10:39:27 +0200 Subject: [PATCH 07/11] fix(web): keep loaded rows when the next page fails --- src/web/page.ts | 13 ++++++++++++- test/web-ui.test.ts | 29 ++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/web/page.ts b/src/web/page.ts index afde274..ee1d0d2 100644 --- a/src/web/page.ts +++ b/src/web/page.ts @@ -188,6 +188,7 @@ a.source:hover { color: var(--accent); text-decoration: underline; } border: 1px solid var(--rule-2); background: var(--sheet); color: var(--ink); } .more:hover { border-color: var(--accent); color: var(--accent); } +.more-error { margin: 10px 0 0; text-align: center; color: var(--remove); } .empty { padding: 56px 24px; text-align: center; color: var(--ink-2); } .repo { border-top: 1px solid var(--rule); } .repo:first-child { border-top: 0; } @@ -355,6 +356,7 @@ dialog::backdrop { background: color-mix(in oklab, var(--paper) 40%, rgba(5,12,2
+ @@ -644,6 +646,7 @@ dialog::backdrop { background: color-mix(in oklab, var(--paper) 40%, rgba(5,12,2 offset: String(append ? view.items.length : 0), limit: String(local ? 1 : grouped() ? 500 : PAGE) }); return apiJson('/api/skills?' + p).then(function (r) { if (seq !== querySeq) return; + $('more-error').hidden = true; view.counts = r.counts; view.all = r.all; if (local) { var list = installedMatches(); @@ -654,7 +657,15 @@ dialog::backdrop { background: color-mix(in oklab, var(--paper) 40%, rgba(5,12,2 } renderRows(append && !local, r.items); renderThemes(); if (!append) document.querySelector('.list').scrollTop = 0; - }).catch(function (err) { if (seq === querySeq) { $('more').disabled = false; fail(err); } }); + }).catch(function (err) { + if (seq !== querySeq) return; + $('more').disabled = false; + // A failed next page keeps the rows already shown: the error goes under them, and Show more retries from the same offset. + if (!append) return fail(err); + var note = $('more-error'); + note.textContent = tr('error', { msg: err.message || String(err) }); + note.hidden = false; + }); } $('more').addEventListener('click', function () { load(true); }); diff --git a/test/web-ui.test.ts b/test/web-ui.test.ts index 3a7b7b3..6502d85 100644 --- a/test/web-ui.test.ts +++ b/test/web-ui.test.ts @@ -28,7 +28,7 @@ function browser(preview: unknown = { markdown: '', ris }; const script = [...PAGE_HTML.matchAll(/