Skip to content

Commit 7a47f1f

Browse files
SoonIterclaude
andcommitted
refactor: simplify error wrapping and progress reporting
- Extract getErrorMessage() and createInstallError() to eliminate copy-paste error wrapping in fetchQueue, linkQueue, and pipeline - Flatten nested conditionals in bus.ts emitFetched with early return - Fix double computation of formatProgressLine in complete() - Fix implicit fall-through in render() with guard clause - Wrap JSON.parse in isSkillUpToDate with its own try/catch Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e590609 commit 7a47f1f

3 files changed

Lines changed: 15 additions & 20 deletions

File tree

packages/skills-package-manager/src/pipeline/fetchQueue.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { access, lstat, readFile, readlink } from 'node:fs/promises'
22
import path from 'node:path'
3-
import { ErrorCode, SpmError } from '../errors'
3+
import { createInstallError } from '../errors'
44
import { fetchSkill } from '../fetchers'
55
import { applySkillPatch } from '../patches/skillPatch'
66
import { createTaskQueue, type TaskQueue } from './queue'
@@ -17,9 +17,6 @@ async function isSkillUpToDate(
1717
const skillDir = path.join(rootDir, installDir, skillName)
1818

1919
try {
20-
// Verify SKILL.md exists — a stale marker alone is not sufficient
21-
await access(path.join(skillDir, 'SKILL.md'))
22-
2320
const stats = await lstat(skillDir)
2421

2522
if (entry.resolution.type === 'link') {
@@ -30,8 +27,16 @@ async function isSkillUpToDate(
3027
return resolvedTarget === expectedTarget
3128
}
3229

30+
// A marker alone is not sufficient — verify the skill content is present
31+
await access(path.join(skillDir, 'SKILL.md'))
32+
3333
const markerPath = path.join(skillDir, '.skills-pm.json')
34-
const marker = JSON.parse(await readFile(markerPath, 'utf8'))
34+
let marker: { installedBy?: string; digest?: string } | undefined
35+
try {
36+
marker = JSON.parse(await readFile(markerPath, 'utf8'))
37+
} catch {
38+
return false
39+
}
3540
return marker?.installedBy === 'skills-package-manager' && marker?.digest === entry.digest
3641
} catch {
3742
return false
@@ -79,12 +84,7 @@ export function createFetchTaskQueue(
7984
bus.emitFetched(result)
8085
return result
8186
} catch (error) {
82-
throw new SpmError({
83-
code: ErrorCode.INSTALL_ERROR,
84-
message: `Failed to fetch skill "${task.skillName}": ${error instanceof Error ? error.message : String(error)}`,
85-
cause: error instanceof Error ? error : undefined,
86-
context: { skillName: task.skillName, phase: 'fetch' },
87-
})
87+
throw createInstallError('fetch', task.skillName, error)
8888
}
8989
}
9090

packages/skills-package-manager/src/pipeline/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { access } from 'node:fs/promises'
22
import path from 'node:path'
33
import type { SkillsLock, SkillsLockEntry } from '../config/types'
4-
import { ErrorCode, SpmError } from '../errors'
4+
import { ErrorCode, getErrorMessage, SpmError } from '../errors'
55
import { writeInstallState } from '../install/installState'
66
import { pruneManagedSkills } from '../install/pruneManagedSkills'
77
import { installStageHooks } from '../install/withBundledSelfSkillLock'
@@ -155,7 +155,7 @@ export async function runPipeline(input: RunPipelineInput): Promise<PipelineResu
155155
code: ErrorCode.INSTALL_ERROR,
156156
message: `${errors.length} skills failed to install`,
157157
cause: first instanceof Error ? first : undefined,
158-
context: { errors: errors.map((e) => (e instanceof Error ? e.message : String(e))) },
158+
context: { errors: errors.map(getErrorMessage) },
159159
})
160160
}
161161

packages/skills-package-manager/src/pipeline/linkQueue.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { ErrorCode, SpmError } from '../errors'
1+
import { createInstallError } from '../errors'
22
import { linkSkill } from '../install/links'
33
import { createTaskQueue, type TaskQueue } from './queue'
44
import type { LinkResult, LinkTask, PipelineBus, WorkspaceContext } from './types'
@@ -23,12 +23,7 @@ export function createLinkTaskQueue(
2323
bus.emitLinked(result)
2424
return result
2525
} catch (error) {
26-
throw new SpmError({
27-
code: ErrorCode.INSTALL_ERROR,
28-
message: `Failed to link skill "${task.skillName}": ${error instanceof Error ? error.message : String(error)}`,
29-
cause: error instanceof Error ? error : undefined,
30-
context: { skillName: task.skillName, phase: 'link' },
31-
})
26+
throw createInstallError('link', task.skillName, error)
3227
}
3328
}
3429

0 commit comments

Comments
 (0)