diff --git a/plugins/dsh/scripts/smoke-native-host.mjs b/plugins/dsh/scripts/smoke-native-host.mjs index 0ff433f1c..0c47f4626 100644 --- a/plugins/dsh/scripts/smoke-native-host.mjs +++ b/plugins/dsh/scripts/smoke-native-host.mjs @@ -122,10 +122,12 @@ export async function selectTarball(input) { export function safeAvailabilityCheckpoint(value) { if (!value || typeof value !== 'object') return undefined; const scenarios = ['setup', 'user-install', 'user-winner', 'project-install', 'project-winner', 'user-shadowed', 'remote', 'by-name', 'content-modified', 'skill-missing', 'directory-missing', 'policy', 'absent', 'incomplete-provider', 'failed-provider']; - const checkpoints = ['setup', 'install', 'install-poll', 'reference', 'before-digest', 'execute', 'assertion', 'after-digest', 'mutation']; + const checkpoints = ['setup', 'install', 'install-poll', 'reference', 'before-digest', 'execute', 'assertion', 'after-digest', 'mutation', 'mutation-apply', 'mutation-restore']; const assertions = ['none', 'isError', 'kind', 'schemaVersion', 'association', 'scope', 'integrity', 'discovery', 'name', 'reason', 'invocation', 'installation', 'namespace', 'slug', 'version', 'fingerprint', 'render', 'privacy', 'unchanged', 'accepted', 'completed', 'reference']; if (!scenarios.includes(value.scenario) || !checkpoints.includes(value.checkpoint) || !assertions.includes(value.assertion)) return undefined; const result = { scenario: value.scenario, checkpoint: value.checkpoint, assertion: value.assertion }; + const osCodes = ['EACCES', 'EPERM', 'EBUSY', 'ENOENT', 'ENOTEMPTY', 'EEXIST', 'EXDEV', 'EIO', 'ETIMEDOUT']; + for (const field of ['osCode', 'restoreOsCode']) if (osCodes.includes(value[field])) result[field] = value[field]; const statuses = ['accepted', 'not-confirmed', 'awaiting-confirmation', 'cancelled']; // Mirror the public installationFailureCodes contract; never forward error messages or IDs. const codes = ['CONTEXT_STALE', 'REQUEST_CONFLICT', 'SOURCE_NOT_CONFIGURED', 'SOURCE_DENIED', diff --git a/plugins/dsh/test/availability-mutation-fixture.test.ts b/plugins/dsh/test/availability-mutation-fixture.test.ts new file mode 100644 index 000000000..4973d517e --- /dev/null +++ b/plugins/dsh/test/availability-mutation-fixture.test.ts @@ -0,0 +1,74 @@ +import { expect, test } from 'bun:test'; +import { mkdtemp, mkdir, readFile, rename, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +// @ts-expect-error Development-only observer fixture has no TypeScript declarations. +import { withRenamedAvailabilityPath } from './integration/skill-availability-fixture.mjs'; +// @ts-expect-error Development-only Node script has no TypeScript declarations. +import { safeAvailabilityCheckpoint } from '../scripts/smoke-native-host.mjs'; + +function checkpoint(): { scenario: string; checkpoint: string; assertion: string; restoreOsCode?: string } { return { scenario: 'directory-missing', checkpoint: 'mutation', assertion: 'none' }; } + +test('failed initial rename never restores an absent backup or replaces the original error', async () => { + const state = checkpoint(); + const original = Object.assign(new Error('/private/locked/path'), { code: 'EPERM' }); + const calls: string[][] = []; + let inspected = false; + await expect(withRenamedAvailabilityPath(state, '/source', '/backup', async () => { inspected = true; }, async (...paths: string[]) => { + calls.push(paths); throw original; + })).rejects.toBe(original); + expect(calls).toEqual([['/source', '/backup']]); + expect(inspected).toBe(false); + expect(state.checkpoint).toBe('mutation-apply'); +}); + +test('failed restoration preserves the primary inspection error and its checkpoint', async () => { + const state = checkpoint(); + const original = Object.assign(new Error('/private/assertion'), { code: 'ERR_ASSERTION' }); + let calls = 0; + await expect(withRenamedAvailabilityPath(state, '/source', '/backup', async () => { + Object.assign(state, { checkpoint: 'assertion', assertion: 'integrity' }); + throw original; + }, async () => { if (++calls === 2) throw Object.assign(new Error('/private/restore'), { code: 'EBUSY' }); })).rejects.toBe(original); + expect(calls).toBe(2); + expect(state).toEqual({ scenario: 'directory-missing', checkpoint: 'assertion', assertion: 'integrity', restoreOsCode: 'EBUSY' }); +}); + +test('restoration failure after successful inspection identifies the restore phase', async () => { + const state = checkpoint(); + const original = Object.assign(new Error('/private/restore'), { code: 'EACCES' }); + let calls = 0; + await expect(withRenamedAvailabilityPath(state, '/source', '/backup', async () => {}, async () => { + if (++calls === 2) throw original; + })).rejects.toBe(original); + expect(state.checkpoint).toBe('mutation-restore'); +}); + +test('real directory rename and restore preserve fixture files without skipping inspection', async () => { + const root = await mkdtemp(join(tmpdir(), 'skillhub-availability-mutation-')); + const source = join(root, 'skill'); + const backup = join(root, 'backup'); + await mkdir(source); + await writeFile(join(source, 'SKILL.md'), 'original'); + try { + let inspected = false; + await withRenamedAvailabilityPath(checkpoint(), source, backup, async () => { + inspected = true; + expect(await readFile(join(backup, 'SKILL.md'), 'utf8')).toBe('original'); + await expect(readFile(join(source, 'SKILL.md'))).rejects.toHaveProperty('code', 'ENOENT'); + }, rename); + expect(inspected).toBe(true); + expect(await readFile(join(source, 'SKILL.md'), 'utf8')).toBe('original'); + } finally { await rm(root, { recursive: true, force: true }); } +}); + +test('mutation summary keeps only fixed OS codes and excludes paths or arbitrary error strings', () => { + const state = { ...checkpoint(), checkpoint: 'mutation-apply', osCode: 'EPERM', restoreOsCode: 'ENOENT', message: '/private/token' }; + expect(safeAvailabilityCheckpoint(state)).toEqual({ scenario: 'directory-missing', checkpoint: 'mutation-apply', assertion: 'none', osCode: 'EPERM', restoreOsCode: 'ENOENT' }); + for (const value of ['/private/token', 'secret', 'constructor', {}, undefined]) { + const safe = safeAvailabilityCheckpoint({ ...state, osCode: value, restoreOsCode: value }); + expect(safe.osCode).toBeUndefined(); + expect(safe.restoreOsCode).toBeUndefined(); + expect(JSON.stringify(safe)).not.toMatch(/private|token|secret/); + } +}); diff --git a/plugins/dsh/test/integration/skill-availability-fixture.mjs b/plugins/dsh/test/integration/skill-availability-fixture.mjs index 0262d0451..86a051892 100644 --- a/plugins/dsh/test/integration/skill-availability-fixture.mjs +++ b/plugins/dsh/test/integration/skill-availability-fixture.mjs @@ -15,10 +15,29 @@ export function apply(ctx, config) { const checkpoint = { scenario: 'setup', checkpoint: 'setup', assertion: 'none' }; const abort = new AbortController(); const timer = setTimeout(() => run(ctx, root, AbortSignal.any([abort.signal, AbortSignal.timeout(70000)]), checkpoint) - .catch(error => writeFile(join(root, 'skill-availability-result.json'), JSON.stringify({ passed: false, error: error.message, checkpoint, operation: checkpoint.operation }))), 1000); + .catch(error => writeFile(join(root, 'skill-availability-result.json'), JSON.stringify({ passed: false, error: error.message, checkpoint: { ...checkpoint, osCode: error.code }, operation: checkpoint.operation }))), 1000); return () => { clearTimeout(timer); abort.abort(); }; } +/** Preserve the failing mutation/inspection when cleanup also fails. No retry hides a Host failure. */ +export async function withRenamedAvailabilityPath(checkpoint, from, to, inspect, move = rename) { + Object.assign(checkpoint, { checkpoint: 'mutation-apply', assertion: 'none' }); + // A failed rename leaves no backup owned by this fixture; do not try to restore it. + await move(from, to); + let inspectionFailed = false; + try { return await inspect(); } + catch (error) { inspectionFailed = true; throw error; } + finally { + if (inspectionFailed) { + try { await move(to, from); } + catch (error) { checkpoint.restoreOsCode = error.code; } + } else { + Object.assign(checkpoint, { checkpoint: 'mutation-restore', assertion: 'none' }); + await move(to, from); + } + } +} + async function digest(directory) { const hash = createHash('sha256'); async function visit(path, relative = '') { @@ -151,13 +170,14 @@ async function run(ctx, root, signal, checkpoint) { checkpoint.scenario = mode; mark('mutation'); const backup = join(root, 'availability-backup-' + randomUUID()); - try { - if (mode === 'content-modified') await writeFile(skillPath, original + '\nAVAILABILITY_PRIVATE_SENTINEL'); - else await rename(mode === 'skill-missing' ? skillPath : target, backup); - await check(await reference('project'), { integrity: mode === 'content-modified' ? 'changed' : 'missing' }); - } finally { - if (mode === 'content-modified') await writeFile(skillPath, original); - else await rename(backup, mode === 'skill-missing' ? skillPath : target); + if (mode === 'content-modified') { + try { + await writeFile(skillPath, original + '\nAVAILABILITY_PRIVATE_SENTINEL'); + await check(await reference('project'), { integrity: 'changed' }); + } finally { await writeFile(skillPath, original); } + } else { + await withRenamedAvailabilityPath(checkpoint, mode === 'skill-missing' ? skillPath : target, backup, + async () => check(await reference('project'), { integrity: 'missing' })); } } checkpoint.scenario = 'policy';