From e5a65a7297153c13921538e280b571a0c86b431e Mon Sep 17 00:00:00 2001 From: huaiju Date: Mon, 27 Jul 2026 18:21:15 +0800 Subject: [PATCH 01/29] feat(skills): hash-based publish/update without user version Align Doraemon skills with vercel-style content hashing: publish/upload pushes remote without requiring semver; re-publish is no-op when hash matches; bare update refreshes all lock-tracked skills by fingerprint. First publish requires category; CLI E2E verified against local registry. --- app/service/skillsRegistry.js | 112 +++++++++++++++++----- dt-skill/src/cli.ts | 28 ++++-- dt-skill/src/cli/commands/publish.test.ts | 26 ++++- dt-skill/src/cli/commands/publish.ts | 84 ++++++++++++++-- dt-skill/src/cli/commands/skills.ts | 104 +++++++++++--------- dt-skill/src/cli/installerPipeline.ts | 7 +- dt-skill/src/cli/ui.ts | 11 +++ dt-skill/src/lockfile.ts | 8 +- dt-skill/src/schema/schemas.ts | 14 ++- dt-skill/test/cliCommandTestKit.ts | 1 + test/skills-registry-contract.test.js | 88 ++++++++++++++++- 11 files changed, 386 insertions(+), 97 deletions(-) diff --git a/app/service/skillsRegistry.js b/app/service/skillsRegistry.js index ab30517..50cfeb2 100644 --- a/app/service/skillsRegistry.js +++ b/app/service/skillsRegistry.js @@ -156,6 +156,12 @@ class SkillsRegistryService extends Service { const stats = { stars: skill.stars || 0, downloads: 0 }; const createdAt = skill.created_at ? new Date(skill.created_at).getTime() : 0; const updatedAt = skill.updated_at ? new Date(skill.updated_at).getTime() : 0; + let fingerprint = null; + try { + fingerprint = await this.computeSkillFingerprint(skill.id); + } catch (err) { + this.ctx.logger.warn('[skillsRegistry] compute fingerprint failed:', err); + } const detail = { skill: { @@ -169,6 +175,8 @@ class SkillsRegistryService extends Service { updatedAt, isPackage: skill.is_package === 1, parentSlug: skill.parent_slug || null, + category: skill.category || '通用', + fingerprint, }, latestVersion: version ? { @@ -176,6 +184,7 @@ class SkillsRegistryService extends Service { createdAt: updatedAt, changelog: '', license: null, + fingerprint, } : null, owner: null, @@ -353,19 +362,46 @@ class SkillsRegistryService extends Service { return SEMVER_PATTERN.test(String(version || '').trim()); } - // Publish or update a skill + // Compatibility placeholder when client omits version (hash is the real change signal). + resolvePublishVersion(version) { + const raw = String(version || '').trim(); + if (!raw) return '0.0.0'; + if (!this.validateSemVer(raw)) { + this.ctx.throw(400, 'version 必须是有效的 SemVer 格式'); + } + return raw; + } + + resolvePublishCategory(category) { + const raw = String(category || '').trim(); + if (!raw) return null; + return raw; + } + + // Fingerprint of an in-memory multipart/processed upload set (same contract as stored files). + computeIncomingFingerprint(processedFiles) { + const storedLike = processedFiles.map((file) => ({ + file_path: file.relPath, + content: file.content, + is_binary: file.isBinary ? 1 : 0, + })); + const fingerprintIgnore = this.createFingerprintIgnore(storedLike); + return skillFingerprint.buildSkillFingerprintFromStoredFiles(storedLike, { + ignoreMatcher: fingerprintIgnore, + }); + } + + // Publish or update a skill (single-slot per slug; content hash is the change signal) async publishSkill(payload, files) { const { SkillsItem, SkillsFile, SkillsSource } = this.app.model; - const { slug, displayName, version, tags } = payload; + const { slug, displayName, tags } = payload; + const version = this.resolvePublishVersion(payload.version); + const category = this.resolvePublishCategory(payload.category); if (!SKILL_SLUG_PATTERN.test(String(slug || ''))) { this.ctx.throw(400, 'slug 格式无效'); } - if (!this.validateSemVer(version)) { - this.ctx.throw(400, 'version 必须是有效的 SemVer 格式'); - } - const parsedTags = Array.isArray(tags) ? tags : []; // file.content 直接给（内存形态，测试/部分调用方）优先；否则读磁盘临时文件（真实 multipart）。 @@ -415,6 +451,8 @@ class SkillsRegistryService extends Service { this.ctx.throw(400, `上传内容必须包含 SKILL.md。已上传: ${uploadedNames}`); } + const incomingFingerprint = this.computeIncomingFingerprint(processedFiles); + return await this.app.model.transaction(async (t) => { const [source] = await SkillsSource.findOrCreate({ where: { source_url: 'clawhub-publish' }, @@ -428,19 +466,40 @@ class SkillsRegistryService extends Service { let skill = await SkillsItem.findOne({ where: { slug }, transaction: t }); + // Same content already published → no-op (hash model) + if (skill && skill.is_delete === 0) { + const existingFingerprint = await this.computeSkillFingerprint(skill.id); + if (existingFingerprint && existingFingerprint === incomingFingerprint) { + const meta = {}; + if (displayName && displayName !== skill.name) meta.name = displayName; + if (payload.description != null) meta.description = payload.description || ''; + if (parsedTags.length) meta.tags = JSON.stringify(parsedTags); + if (category) meta.category = category; + if (Object.keys(meta).length) { + await skill.update(meta, { transaction: t }); + } + return { + ok: true, + skillId: String(skill.id), + versionId: `v${skill.version || version}`, + fingerprint: existingFingerprint, + unchanged: true, + }; + } + } + if (skill) { - await skill.update( - { - name: displayName, - description: payload.description || '', - version, - tags: JSON.stringify(parsedTags), - skill_md: skillMdFile.content || '', - is_delete: 0, - source_id: source.id, - }, - { transaction: t } - ); + const updatePayload = { + name: displayName, + description: payload.description || '', + version, + tags: JSON.stringify(parsedTags), + skill_md: skillMdFile.content || '', + is_delete: 0, + source_id: source.id, + }; + if (category) updatePayload.category = category; + await skill.update(updatePayload, { transaction: t }); // Delete old files await SkillsFile.update( { is_delete: 1 }, @@ -456,7 +515,7 @@ class SkillsRegistryService extends Service { version, tags: JSON.stringify(parsedTags), skill_md: skillMdFile.content || '', - category: '通用', + category: category || '通用', file_count: files.length, }, { transaction: t } @@ -486,6 +545,8 @@ class SkillsRegistryService extends Service { ok: true, skillId: String(skill.id), versionId: `v${version}`, + fingerprint: incomingFingerprint, + unchanged: false, }; }); } @@ -518,9 +579,16 @@ class SkillsRegistryService extends Service { }; } - const skillFingerprint = await this.computeSkillFingerprint(skill.id); - const match = skillFingerprint === hash ? { version: skill.version || '' } : null; - const latestVersion = skill.version ? { version: skill.version } : null; + const currentFingerprint = await this.computeSkillFingerprint(skill.id); + const version = skill.version || '0.0.0'; + const match = + hash && currentFingerprint === hash + ? { version, fingerprint: currentFingerprint } + : null; + const latestVersion = { + version, + fingerprint: currentFingerprint, + }; return { match, diff --git a/dt-skill/src/cli.ts b/dt-skill/src/cli.ts index f4f1e0d..f548ac8 100644 --- a/dt-skill/src/cli.ts +++ b/dt-skill/src/cli.ts @@ -187,11 +187,13 @@ registerCommand(program, ['install']) }); registerCommand(program, ['update']) - .description('Update installed skills') - .argument('[slug]', 'Skill slug') - .option('--all', 'Update all installed skills') - .option('--version ', 'Update to specific version (single slug only)') - .option('--force', 'Overwrite when local files do not match any version') + .description( + 'Update installed skills to registry content (by hash). Bare update = all tracked skills.' + ) + .argument('[slug]', 'Skill slug (omit to update all)') + .option('--all', 'Update all installed skills (same as bare update)') + .option('--version ', 'Update to specific version (single slug only, legacy)') + .option('--force', 'Overwrite when local files do not match registry content') .action(async (slug, options) => { const opts = await resolveGlobalOpts(); await cmdUpdate(opts, slug, options, isInputAllowed()); @@ -269,19 +271,23 @@ registerCommand(program, ['inspect']) }); registerCommand(program, ['publish']) - .description('Legacy alias: publish a skill from folder') + .description( + 'Publish a skill folder to the registry (push remote). Re-publish same slug overwrites by content hash.' + ) + .alias('upload') .argument('', 'Skill folder path') .option('--slug ', 'Skill slug') .option('--name ', 'Display name') .option('--owner ', 'Publish under an org/user publisher handle') .option('--migrate-owner', 'Move an existing skill to the selected owner when republishing') - .option('--version ', 'Version (semver)') + .option('--version ', 'Optional semver (compatibility; default 0.0.0, hash detects changes)') .option('--fork-of ', 'Mark as a fork of an existing skill') .option('--changelog ', 'Changelog text') .option('--clawscan-note ', CLAWSCAN_NOTE_HELP) .option('--tags ', 'Comma-separated tags', 'latest') .option('--all', 'Batch mode: upload all discovered skills without interactive selection') - .option('--category ', 'Category for batch upload') + .option('--category ', 'Category (required on first publish in non-interactive mode)') + .option('--yes', 'Skip overwrite confirmation') .action(async (folder, options) => { const opts = await resolveGlobalOpts(); await cmdPublish(opts, folder, options); @@ -333,17 +339,19 @@ registerCommand(program, ['unhide']) const skill = registerCommandGroup(program, ['skill']).description('Manage published skills'); registerCommand(skill, ['skill', 'publish']) - .description('Publish a skill from folder') + .description('Publish a skill from folder (same as publish/upload)') .argument('', 'Skill folder path') .option('--slug ', 'Skill slug') .option('--name ', 'Display name') .option('--owner ', 'Publish under an org/user publisher handle') .option('--migrate-owner', 'Move an existing skill to the selected owner when republishing') - .option('--version ', 'Version (semver)') + .option('--version ', 'Optional semver (compatibility; default 0.0.0)') .option('--fork-of ', 'Mark as a fork of an existing skill') .option('--changelog ', 'Changelog text') .option('--clawscan-note ', CLAWSCAN_NOTE_HELP) .option('--tags ', 'Comma-separated tags', 'latest') + .option('--category ', 'Category (required on first publish in non-interactive mode)') + .option('--yes', 'Skip overwrite confirmation') .action(async (folder, options) => { const opts = await resolveGlobalOpts(); await cmdPublish(opts, folder, options); diff --git a/dt-skill/src/cli/commands/publish.test.ts b/dt-skill/src/cli/commands/publish.test.ts index 4e0b524..258746f 100644 --- a/dt-skill/src/cli/commands/publish.test.ts +++ b/dt-skill/src/cli/commands/publish.test.ts @@ -57,16 +57,21 @@ describe('cmdPublish', () => { await writeFile(join(folder, 'SKILL.md'), skillContent, 'utf8'); await writeFile(join(folder, 'notes.md'), notesContent, 'utf8'); + // Existing skill lookup (GET) — miss so first-publish path needs category + httpMocks.apiRequest.mockRejectedValueOnce(new Error('not found')); httpMocks.apiRequestForm.mockResolvedValueOnce({ ok: true, skillId: 'skill_1', - versionId: 'ver_1', + versionId: 'v0.0.0', + fingerprint: 'abc123', + unchanged: false, }); await cmdPublish(makeOpts(workdir), 'my-skill', { slug: 'my-skill', name: 'My Skill', - version: '1.0.0', + category: '工程效率', + yes: true, changelog: '', tags: 'latest', clawscanNote: "This skill needs network access to call the user's configured API.", @@ -84,7 +89,8 @@ describe('cmdPublish', () => { const payload = JSON.parse(payloadEntry); expect(payload.slug).toBe('my-skill'); expect(payload.displayName).toBe('My Skill'); - expect(payload.version).toBe('1.0.0'); + expect(payload.version).toBe('0.0.0'); + expect(payload.category).toBe('工程效率'); expect(payload.changelog).toBe(''); expect(payload.clawScanNote).toBe( "This skill needs network access to call the user's configured API." @@ -106,14 +112,17 @@ describe('cmdPublish', () => { await writeFile(join(folder, 'SKILL.md'), '# Skill\n', 'utf8'); await writeFile(join(folder, 'assets', 'logo.png'), new Uint8Array([0, 1, 2, 255])); + httpMocks.apiRequest.mockRejectedValueOnce(new Error('not found')); httpMocks.apiRequestForm.mockResolvedValueOnce({ ok: true, skillId: 'skill_1', - versionId: 'ver_1', + versionId: 'v1.0.0', }); await cmdPublish(makeOpts(workdir), 'skill-with-assets', { version: '1.0.0', + category: '通用', + yes: true, }); const publishCall = httpMocks.apiRequestForm.mock.calls[0]; @@ -158,6 +167,7 @@ describe('cmdPublish', () => { await mkdir(folder, { recursive: true }); await writeFile(join(folder, 'SKILL.md'), '# Skill\n', 'utf8'); + httpMocks.apiRequest.mockRejectedValueOnce(new Error('not found')); httpMocks.apiRequestForm.mockResolvedValueOnce({ ok: true, skillId: 'skill_1', @@ -166,6 +176,8 @@ describe('cmdPublish', () => { await cmdPublish(makeOpts(workdir), 'existing-skill', { version: '1.0.1', + category: '通用', + yes: true, changelog: '', tags: 'latest', }); @@ -189,6 +201,7 @@ describe('cmdPublish', () => { await writeFile(join(folder, 'SKILL.md'), '# Skill\n', 'utf8'); await writeFile(join(folder, 'notes.md'), 'ignored notes\n', 'utf8'); + httpMocks.apiRequest.mockRejectedValueOnce(new Error('not found')); httpMocks.apiRequestForm.mockResolvedValueOnce({ ok: true, skillId: 'skill_1', @@ -199,6 +212,8 @@ describe('cmdPublish', () => { slug: 'ignored-manifest', name: 'Ignored Manifest', version: '1.0.0', + category: '通用', + yes: true, changelog: '', tags: 'latest', }); @@ -223,6 +238,7 @@ describe('cmdPublish', () => { await mkdir(folder, { recursive: true }); await writeFile(join(folder, 'SKILL.md'), '# Skill\n', 'utf8'); + httpMocks.apiRequest.mockRejectedValueOnce(new Error('not found')); httpMocks.apiRequestForm.mockResolvedValueOnce({ ok: true, skillId: 'skill_1', @@ -233,6 +249,8 @@ describe('cmdPublish', () => { owner: '@openclaw', migrateOwner: true, version: '1.0.1', + category: '通用', + yes: true, changelog: '', tags: 'latest', }); diff --git a/dt-skill/src/cli/commands/publish.ts b/dt-skill/src/cli/commands/publish.ts index 15801ab..a392c4d 100644 --- a/dt-skill/src/cli/commands/publish.ts +++ b/dt-skill/src/cli/commands/publish.ts @@ -3,10 +3,11 @@ import { readdir, readFile, stat } from 'node:fs/promises'; import { basename, dirname, join, resolve } from 'node:path'; import semver from 'semver'; -import { apiRequestForm } from '../../http.js'; +import { apiRequest, apiRequestForm } from '../../http.js'; import { ApiRoutes, ApiV1PublishResponseSchema, + ApiV1SkillResponseSchema, normalizeClawScanNote, } from '../../schema/index.js'; import { listPublishFiles } from '../../skills.js'; @@ -15,7 +16,22 @@ import { getRegistry } from '../registry.js'; import { findSkillFolders } from '../scanSkills.js'; import { sanitizeSlug, titleCase } from '../slug.js'; import type { GlobalOpts } from '../types.js'; -import { createSpinner, fail, formatError, isInteractive } from '../ui.js'; +import { createSpinner, fail, formatError, isInteractive, promptConfirm, selectCategory } from '../ui.js'; + +/** Closed category enum aligned with Doraemon skills market. */ +export const SKILL_CATEGORY_OPTIONS = [ + '通用', + '前端', + '后端', + '数据与AI', + '运维与系统', + '工程效率', + '安全', + '其他', +] as const; + +/** Internal compatibility version when author omits --version (hash is the change signal). */ +const DEFAULT_PUBLISH_VERSION = '0.0.0'; export async function cmdPublish( opts: GlobalOpts, @@ -32,6 +48,7 @@ export async function cmdPublish( migrateOwner?: boolean; all?: boolean; category?: string; + yes?: boolean; } ) { // Resolve folder path against the project base (parent of the canonical @@ -60,7 +77,9 @@ export async function cmdPublish( const slug = options.slug ?? sanitizeSlug(basename(folder)); const displayName = options.name ?? titleCase(basename(folder)); const ownerHandle = options.owner?.trim().replace(/^@+/, ''); - const version = options.version; + // Version is optional for authors; default is a compatibility placeholder. Change detection uses content hash. + let version = options.version?.trim() || DEFAULT_PUBLISH_VERSION; + if (!semver.valid(version)) fail('--version must be valid semver when provided'); const changelog = options.changelog ?? ''; let clawScanNote: string | undefined; try { @@ -79,9 +98,51 @@ export async function cmdPublish( if (!slug) fail('--slug required'); if (!displayName) fail('--name required'); - if (!version || !semver.valid(version)) fail('--version must be valid semver'); - const spinner = createSpinner(`Preparing ${slug}@${version}`); + // Detect whether slug already exists on registry (first publish needs category). + let existingCategory: string | null = null; + let skillExists = false; + try { + const existing = await apiRequest( + registry, + { method: 'GET', path: `${ApiRoutes.skills}/${encodeURIComponent(slug)}` }, + ApiV1SkillResponseSchema + ); + skillExists = Boolean(existing?.skill); + const skillMeta = existing?.skill as { category?: string } | undefined; + if (skillMeta?.category) existingCategory = String(skillMeta.category); + } catch { + skillExists = false; + } + + let category = options.category?.trim() || ''; + if (category && !SKILL_CATEGORY_OPTIONS.includes(category as (typeof SKILL_CATEGORY_OPTIONS)[number])) { + fail(`--category must be one of: ${SKILL_CATEGORY_OPTIONS.join(', ')}`); + } + if (!skillExists && !category) { + if (isInteractive()) { + const picked = await selectCategory(SKILL_CATEGORY_OPTIONS); + if (!picked) fail('Category required for first publish'); + category = picked; + } else { + fail('First publish requires --category in non-interactive mode'); + } + } + if (skillExists && !category && existingCategory) { + category = existingCategory; + } + + if (skillExists && isInteractive() && !options.yes) { + const ok = await promptConfirm( + `Skill "${slug}" already exists on the registry. Overwrite remote content if it changed?` + ); + if (!ok) { + console.log('Publish cancelled'); + return; + } + } + + const spinner = createSpinner(`Preparing ${slug}`); try { const filesOnDisk = await ensureRootManifestFile(folder, await listPublishFiles(folder)); if (filesOnDisk.length === 0) fail('No files found'); @@ -107,6 +168,7 @@ export async function cmdPublish( ...(clawScanNote ? { clawScanNote } : {}), acceptLicenseTerms: true, tags, + ...(category ? { category } : {}), ...(forkOf ? { forkOf } : {}), }) ); @@ -121,14 +183,22 @@ export async function cmdPublish( form.append('files', blob, file.relPath); } - spinner.text = `Publishing ${slug}@${version}`; + spinner.text = `Publishing ${slug}`; const result = await apiRequestForm( registry, { method: 'POST', path: ApiRoutes.skills, form }, ApiV1PublishResponseSchema ); - spinner.succeed(`OK. Published ${slug}@${version} (${result.versionId})`); + if (result.unchanged) { + spinner.succeed( + `OK. Already up to date ${slug}${result.fingerprint ? ` (${result.fingerprint.slice(0, 12)}…)` : ''}` + ); + } else { + spinner.succeed( + `OK. Published ${slug}${result.fingerprint ? ` hash=${result.fingerprint.slice(0, 12)}…` : ''} (${result.versionId})` + ); + } } catch (error) { spinner.fail(formatError(error)); throw error; diff --git a/dt-skill/src/cli/commands/skills.ts b/dt-skill/src/cli/commands/skills.ts index 3618c03..946d70a 100644 --- a/dt-skill/src/cli/commands/skills.ts +++ b/dt-skill/src/cli/commands/skills.ts @@ -711,9 +711,9 @@ export async function cmdUpdate( inputAllowed: boolean ) { const slug = slugArg ? normalizeSkillSlugOrFail(slugArg) : undefined; - const all = Boolean(options.all); - if (!slug && !all) fail('Provide or --all'); - if (slug && all) fail('Use either or --all'); + // Bare `update` == update all tracked skills (same as --all). --all kept for compatibility. + const all = Boolean(options.all) || !slug; + if (slug && options.all) fail('Use either or --all'); if (options.version && !slug) fail('--version requires a single '); if (options.version && !semver.valid(options.version)) fail('--version must be valid semver'); @@ -794,6 +794,9 @@ export async function cmdUpdate( localFingerprint = hashed.fingerprint; } } + if (!localFingerprint && lock.skills[entry]?.fingerprint) { + localFingerprint = lock.skills[entry].fingerprint ?? null; + } let resolveResult: ResolveResult; if (localFingerprint) { @@ -802,61 +805,61 @@ export async function cmdUpdate( resolveResult = { match: null, latestVersion: skillMeta.latestVersion ?? null }; } - const latest = resolveResult.latestVersion?.version ?? null; - const matched = - resolveResult.match?.version ?? - (localFingerprint && - existingOrigin?.fingerprint === localFingerprint && - existingOrigin.slug === entry - ? existingOrigin.installedVersion - : null); - - if (matched && lock.skills[entry]?.version !== matched) { - lock.skills[entry] = withPinnedMetadata( - matched, - lock.skills[entry]?.installedAt ?? Date.now(), - lock.skills[entry] - ); - } + const latest = + resolveResult.latestVersion?.version ?? skillMeta.latestVersion?.version ?? null; + const remoteFingerprint = + (resolveResult.latestVersion as { fingerprint?: string } | null | undefined) + ?.fingerprint ?? + (skillMeta.latestVersion as { fingerprint?: string } | undefined)?.fingerprint ?? + (skillMeta.skill as { fingerprint?: string } | undefined)?.fingerprint ?? + null; + + // Hash model: equal fingerprint (or resolve match) → up to date; else pull latest. + // Do NOT treat "disk matches install origin" as up-to-date — remote may have moved. + const hashMatches = + Boolean(localFingerprint) && + Boolean(remoteFingerprint) && + localFingerprint === remoteFingerprint; + const resolveMatched = Boolean(resolveResult.match?.version); + const contentUpToDate = hashMatches || resolveMatched; if (!latest) { spinner.fail(`${entry}: not found`); continue; } - if (!matched && localFingerprint && !options.force) { - spinner.stop(); - if (!allowPrompt) { - console.log(`${entry}: local changes (no match). Use --force to overwrite.`); - continue; - } - const confirm = await promptConfirm( - `${entry}: local changes (no match). Overwrite with ${ - options.version ?? latest - }?` + if (contentUpToDate && !options.version) { + const keepVersion = resolveResult.match?.version ?? latest; + lock.skills[entry] = withPinnedMetadata( + keepVersion, + lock.skills[entry]?.installedAt ?? Date.now(), + lock.skills[entry], + remoteFingerprint ?? localFingerprint ); - if (!confirm) { - console.log(`${entry}: skipped`); - continue; - } - spinner.start(`Updating ${entry} -> ${options.version ?? latest}`); + await writeLockfile(installWorkdir, lock); + spinner.succeed( + `${entry}: up to date${ + remoteFingerprint + ? ` (${remoteFingerprint.slice(0, 12)}…)` + : keepVersion + ? ` (${keepVersion})` + : '' + }` + ); + continue; } + // Explicit legacy version pin path const targetVersion = options.version ?? latest; - if (options.version) { - if (matched && matched === targetVersion) { - spinner.succeed(`${entry}: already at ${matched}`); - continue; - } - } else if (matched && semver.valid(matched) && semver.gte(matched, targetVersion)) { - spinner.succeed(`${entry}: up to date (${matched})`); + if (options.version && resolveMatched && resolveResult.match?.version === targetVersion) { + spinner.succeed(`${entry}: already at ${targetVersion}`); continue; } if (spinner.isSpinning) { - spinner.text = `Updating ${entry} -> ${targetVersion}`; + spinner.text = `Updating ${entry}`; } else { - spinner.start(`Updating ${entry} -> ${targetVersion}`); + spinner.start(`Updating ${entry}`); } const zip = await downloadZip(registry, { slug: entry, @@ -879,15 +882,26 @@ export async function cmdUpdate( fingerprint: installedFingerprint, }); await replaceSkillDirectory(preparedDir, target, exists); + lock.skills[entry] = withPinnedMetadata( + targetVersion, + Date.now(), + lock.skills[entry], + installedFingerprint + ); } catch (error) { await rm(preparedDir, { recursive: true, force: true }).catch(() => {}); throw error; } - lock.skills[entry] = withPinnedMetadata(targetVersion, Date.now(), lock.skills[entry]); // 每条成功更新后即持久化 lockfile，崩溃不再让磁盘与锁漂移 await writeLockfile(installWorkdir, lock); - spinner.succeed(`${entry}: updated -> ${targetVersion}`); + spinner.succeed( + `${entry}: updated${ + lock.skills[entry]?.fingerprint + ? ` hash=${lock.skills[entry].fingerprint!.slice(0, 12)}…` + : ` -> ${targetVersion}` + }` + ); } catch (error) { spinner.fail(formatError(error)); throw error; diff --git a/dt-skill/src/cli/installerPipeline.ts b/dt-skill/src/cli/installerPipeline.ts index 496401a..c98da67 100644 --- a/dt-skill/src/cli/installerPipeline.ts +++ b/dt-skill/src/cli/installerPipeline.ts @@ -82,6 +82,11 @@ export async function installExtractedSkill( mode: targets.mode, }); } - lock.skills[slug] = withPinnedMetadata(version, Date.now(), existingEntry); + lock.skills[slug] = withPinnedMetadata( + version, + Date.now(), + existingEntry, + installedFingerprint + ); await writeLockfile(canonicalWorkdir, lock); } diff --git a/dt-skill/src/cli/ui.ts b/dt-skill/src/cli/ui.ts index 3d01cff..9d95be5 100644 --- a/dt-skill/src/cli/ui.ts +++ b/dt-skill/src/cli/ui.ts @@ -198,6 +198,17 @@ export async function selectScope(): Promise { return scope as boolean; } +/** Skill market category selection (first publish). Returns null=cancelled. */ +export async function selectCategory(categories: readonly string[]): Promise { + if (!isInteractive()) return null; + const picked = await select({ + message: 'Select skill category', + options: categories.map((value) => ({ value, label: value })), + }); + if (isCancel(picked)) return null; + return picked as string; +} + /** Symlink vs Copy method selection. Returns null=cancelled. */ export async function selectInstallMethod(): Promise { if (!isInteractive()) return null; diff --git a/dt-skill/src/lockfile.ts b/dt-skill/src/lockfile.ts index 174d1c1..b11ae01 100644 --- a/dt-skill/src/lockfile.ts +++ b/dt-skill/src/lockfile.ts @@ -35,11 +35,17 @@ export function isPinned(entry?: LockfileEntry): boolean { export function withPinnedMetadata( version: string | null, installedAt: number, - existing?: LockfileEntry + existing?: LockfileEntry, + fingerprint?: string | null ): LockfileEntry { + const nextFingerprint = + fingerprint !== undefined && fingerprint !== null + ? fingerprint + : existing?.fingerprint; return { version, installedAt, + ...(nextFingerprint ? { fingerprint: nextFingerprint } : {}), ...(existing?.pinned ? { pinned: true } : {}), ...(existing?.pinned && existing.pinReason ? { pinReason: existing.pinReason } : {}), }; diff --git a/dt-skill/src/schema/schemas.ts b/dt-skill/src/schema/schemas.ts index 8179f0f..67885a1 100644 --- a/dt-skill/src/schema/schemas.ts +++ b/dt-skill/src/schema/schemas.ts @@ -22,6 +22,7 @@ export const LockfileSchema = type({ installedAt: 'number', pinned: 'boolean?', pinReason: 'string?', + fingerprint: 'string?', }, }, }); @@ -116,8 +117,8 @@ export const ApiCliSkillDeleteResponseSchema = type({ }); export const ApiSkillResolveResponseSchema = type({ - match: type({ version: 'string' }).or('null'), - latestVersion: type({ version: 'string' }).or('null'), + match: type({ version: 'string', fingerprint: 'string?' }).or('null'), + latestVersion: type({ version: 'string', fingerprint: 'string?' }).or('null'), }); export const CliTelemetrySyncRequestSchema = type({ @@ -216,6 +217,8 @@ export const ApiV1SkillResponseSchema = type({ updatedAt: 'number', isPackage: 'boolean?', parentSlug: 'string|null?', + category: 'string?', + fingerprint: 'string|null?', children: SkillChildSchema.array().optional(), }).or('null'), latestVersion: type({ @@ -223,6 +226,7 @@ export const ApiV1SkillResponseSchema = type({ createdAt: 'number', changelog: 'string', license: '"MIT-0"|null?', + fingerprint: 'string|null?', }).or('null'), owner: type({ handle: 'string|null', @@ -430,8 +434,8 @@ export const ApiV1SkillVersionResponseSchema = type({ export type ApiV1SkillVersionResponse = typeof ApiV1SkillVersionResponseSchema[inferred]; export const ApiV1SkillResolveResponseSchema = type({ - match: type({ version: 'string' }).or('null'), - latestVersion: type({ version: 'string' }).or('null'), + match: type({ version: 'string', fingerprint: 'string?' }).or('null'), + latestVersion: type({ version: 'string', fingerprint: 'string?' }).or('null'), }); export type ApiV1SkillResolveResponse = typeof ApiV1SkillResolveResponseSchema[inferred]; @@ -439,6 +443,8 @@ export const ApiV1PublishResponseSchema = type({ ok: 'true', skillId: 'string', versionId: 'string', + fingerprint: 'string?', + unchanged: 'boolean?', }); export const ApiV1DeleteResponseSchema = type({ diff --git a/dt-skill/test/cliCommandTestKit.ts b/dt-skill/test/cliCommandTestKit.ts index 05a6637..8b88cf5 100644 --- a/dt-skill/test/cliCommandTestKit.ts +++ b/dt-skill/test/cliCommandTestKit.ts @@ -93,6 +93,7 @@ export function createUiModuleMocks(options?: { interactive?: boolean }) { selectAgentsInteractive: vi.fn(async () => []), selectScope: vi.fn(async () => false), selectInstallMethod: vi.fn(async () => 'symlink' as const), + selectCategory: vi.fn(async () => '通用'), noteSummary: vi.fn(), isCancelledValue: (value: unknown) => typeof value === 'symbol', }), diff --git a/test/skills-registry-contract.test.js b/test/skills-registry-contract.test.js index 0fce663..629b4fd 100644 --- a/test/skills-registry-contract.test.js +++ b/test/skills-registry-contract.test.js @@ -287,6 +287,7 @@ test('getSkillDetail returns full skill object', async () => { }, }); service.ctx = createMockCtx(); + service.ctx.logger = { warn: () => {}, info: () => {}, error: () => {} }; const data = await service.getSkillDetail('my-skill'); assert.ok(data); @@ -298,11 +299,13 @@ test('getSkillDetail returns full skill object', async () => { assert.deepEqual(data.skill.stats, { stars: 42, downloads: 0 }); assert.equal(data.skill.createdAt, new Date('2026-05-21T10:00:00Z').getTime()); assert.equal(data.skill.updatedAt, new Date('2026-05-21T10:00:00Z').getTime()); + assert.equal(data.skill.fingerprint, null); assert.deepEqual(data.latestVersion, { version: '1.2.3', createdAt: new Date('2026-05-21T10:00:00Z').getTime(), changelog: '', license: null, + fingerprint: null, }); assert.equal(data.owner, null); assert.equal(data.moderation, null); @@ -612,6 +615,7 @@ test('publishSkill returns ok: true and string skillId and versionId', async () }, }); service.ctx = createMockCtx(); + service.ctx.logger = { warn: () => {}, info: () => {}, error: () => {} }; const result = await service.publishSkill( { slug: 'test-skill', displayName: 'Test Skill', version: '1.0.0' }, @@ -621,6 +625,83 @@ test('publishSkill returns ok: true and string skillId and versionId', async () assert.equal(result.ok, true); assert.equal(typeof result.skillId, 'string'); assert.equal(result.versionId, 'v1.0.0'); + assert.equal(typeof result.fingerprint, 'string'); + assert.equal(result.unchanged, false); +}); + +test('publishSkill accepts missing version (defaults to 0.0.0)', async () => { + const service = Object.create(SkillsRegistryService.prototype); + service.app = createMockApp({ + SkillsItem: { + findOne: async () => null, + create: async (data) => ({ id: 124, ...data, update: async () => {} }), + }, + SkillsSource: { + findOne: async () => ({ id: 1 }), + findOrCreate: async () => [{ id: 1 }], + }, + SkillsFile: { + create: async () => ({}), + }, + }); + service.ctx = createMockCtx(); + service.ctx.logger = { warn: () => {}, info: () => {}, error: () => {} }; + + const result = await service.publishSkill( + { slug: 'hash-skill', displayName: 'Hash Skill', category: '工程效率' }, + [{ filepath: 'SKILL.md', content: '# hash skill\n' }] + ); + + assert.equal(result.ok, true); + assert.equal(result.versionId, 'v0.0.0'); + assert.equal(typeof result.fingerprint, 'string'); +}); + +test('publishSkill same content is unchanged no-op', async () => { + const service = Object.create(SkillsRegistryService.prototype); + const skillRow = { + id: 50, + slug: 'same-skill', + name: 'Same', + version: '0.0.0', + is_delete: 0, + update: async () => {}, + }; + const files = [ + { + file_path: 'SKILL.md', + content: '# same\n', + is_binary: 0, + }, + ]; + service.app = createMockApp({ + SkillsItem: { + findOne: async () => skillRow, + }, + SkillsSource: { + findOrCreate: async () => [{ id: 1 }], + }, + SkillsFile: { + findAll: async () => files, + create: async () => { + throw new Error('should not create on no-op'); + }, + update: async () => { + throw new Error('should not soft-delete on no-op'); + }, + }, + }); + service.ctx = createMockCtx(); + service.ctx.logger = { warn: () => {}, info: () => {}, error: () => {} }; + + const result = await service.publishSkill( + { slug: 'same-skill', displayName: 'Same' }, + [{ filepath: 'SKILL.md', content: '# same\n' }] + ); + + assert.equal(result.ok, true); + assert.equal(result.unchanged, true); + assert.equal(typeof result.fingerprint, 'string'); }); // ============================================================ @@ -736,8 +817,8 @@ test('resolveFingerprint returns match and latestVersion for matching fingerprin const fp = await service.computeSkillFingerprint(1); const result = await service.resolveFingerprint('skill-a', fp); assert.ok(result); - assert.deepEqual(result.match, { version: '1.0.0' }); - assert.deepEqual(result.latestVersion, { version: '1.0.0' }); + assert.deepEqual(result.match, { version: '1.0.0', fingerprint: fp }); + assert.deepEqual(result.latestVersion, { version: '1.0.0', fingerprint: fp }); }); test('resolveFingerprint returns null match for unmatched fingerprint but valid slug', async () => { @@ -752,10 +833,11 @@ test('resolveFingerprint returns null match for unmatched fingerprint but valid }); service.ctx = createMockCtx(); + const fp = await service.computeSkillFingerprint(1); const result = await service.resolveFingerprint('skill-a', 'wrong_hash'); assert.ok(result); assert.equal(result.match, null); - assert.deepEqual(result.latestVersion, { version: '1.0.0' }); + assert.deepEqual(result.latestVersion, { version: '1.0.0', fingerprint: fp }); }); test('resolveFingerprint returns null values for missing slug', async () => { From 6c8ecf68280975fddcd54dfcd9481e8ab82b47d2 Mon Sep 17 00:00:00 2001 From: huaiju Date: Mon, 27 Jul 2026 19:35:05 +0800 Subject: [PATCH 02/29] fix(skills): address hash publish/update review findings Continue batch update after per-skill failures with a summary exit code, confirm overwrite only when content hash differs, keep content no-ops free of metadata churn, type resolve fingerprints, and validate category enums server-side. Expand CLI and registry contract tests accordingly. --- app/service/skillsRegistry.js | 31 ++++++--- dt-skill/src/cli/commands/publish.test.ts | 58 ++++++++++++++++ dt-skill/src/cli/commands/publish.ts | 50 ++++++++++---- dt-skill/src/cli/commands/skills.test.ts | 81 ++++++++++++++++++++++- dt-skill/src/cli/commands/skills.ts | 80 ++++++++++++++++------ dt-skill/src/cli/types.ts | 9 ++- test/skills-registry-contract.test.js | 24 +++++++ 7 files changed, 283 insertions(+), 50 deletions(-) diff --git a/app/service/skillsRegistry.js b/app/service/skillsRegistry.js index 50cfeb2..23edc16 100644 --- a/app/service/skillsRegistry.js +++ b/app/service/skillsRegistry.js @@ -8,6 +8,19 @@ const skillFingerprint = require('../../contracts/skill-fingerprint'); const SEMVER_PATTERN = /^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-[\w.-]+)?(?:\+[\w.-]+)?$/; const SKILL_SLUG_PATTERN = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; +/** Closed category enum (aligned with skills market / CLI). */ +const SKILL_CATEGORY_OPTIONS = [ + '通用', + '前端', + '后端', + '数据与AI', + '运维与系统', + '工程效率', + '安全', + '其他', +]; +/** Compatibility placeholder when client omits version; content hash is the change signal. */ +const DEFAULT_PUBLISH_VERSION = '0.0.0'; class SkillsRegistryService extends Service { // Well-Known Registry Metadata @@ -365,7 +378,7 @@ class SkillsRegistryService extends Service { // Compatibility placeholder when client omits version (hash is the real change signal). resolvePublishVersion(version) { const raw = String(version || '').trim(); - if (!raw) return '0.0.0'; + if (!raw) return DEFAULT_PUBLISH_VERSION; if (!this.validateSemVer(raw)) { this.ctx.throw(400, 'version 必须是有效的 SemVer 格式'); } @@ -375,6 +388,12 @@ class SkillsRegistryService extends Service { resolvePublishCategory(category) { const raw = String(category || '').trim(); if (!raw) return null; + if (!SKILL_CATEGORY_OPTIONS.includes(raw)) { + this.ctx.throw( + 400, + `category 无效，可选: ${SKILL_CATEGORY_OPTIONS.join(', ')}` + ); + } return raw; } @@ -466,18 +485,10 @@ class SkillsRegistryService extends Service { let skill = await SkillsItem.findOne({ where: { slug }, transaction: t }); - // Same content already published → no-op (hash model) + // Same content already published → pure no-op (no meta churn) if (skill && skill.is_delete === 0) { const existingFingerprint = await this.computeSkillFingerprint(skill.id); if (existingFingerprint && existingFingerprint === incomingFingerprint) { - const meta = {}; - if (displayName && displayName !== skill.name) meta.name = displayName; - if (payload.description != null) meta.description = payload.description || ''; - if (parsedTags.length) meta.tags = JSON.stringify(parsedTags); - if (category) meta.category = category; - if (Object.keys(meta).length) { - await skill.update(meta, { transaction: t }); - } return { ok: true, skillId: String(skill.id), diff --git a/dt-skill/src/cli/commands/publish.test.ts b/dt-skill/src/cli/commands/publish.test.ts index 258746f..fbb142e 100644 --- a/dt-skill/src/cli/commands/publish.test.ts +++ b/dt-skill/src/cli/commands/publish.test.ts @@ -47,6 +47,64 @@ afterEach(() => { }); describe('cmdPublish', () => { + it('does not prompt overwrite when remote fingerprint matches local content', async () => { + const workdir = await makeTmpWorkdir(); + try { + const folder = join(workdir, 'same-skill'); + await mkdir(folder, { recursive: true }); + const content = '# same\n'; + await writeFile(join(folder, 'SKILL.md'), content, 'utf8'); + + // Precompute fingerprint the same way publish will + const { hashSkillFiles } = await import('../../skills.js'); + const fp = hashSkillFiles([ + { relPath: 'SKILL.md', bytes: new Uint8Array(Buffer.from(content, 'utf8')) }, + ]).fingerprint; + + httpMocks.apiRequest.mockResolvedValueOnce({ + skill: { + slug: 'same-skill', + displayName: 'Same', + summary: null, + tags: [], + stats: {}, + createdAt: 1, + updatedAt: 1, + category: '通用', + fingerprint: fp, + }, + latestVersion: { + version: '0.0.0', + createdAt: 1, + changelog: '', + license: null, + fingerprint: fp, + }, + owner: null, + moderation: null, + }); + httpMocks.apiRequestForm.mockResolvedValueOnce({ + ok: true, + skillId: '1', + versionId: 'v0.0.0', + fingerprint: fp, + unchanged: true, + }); + + await cmdPublish(makeOpts(workdir), 'same-skill', { + slug: 'same-skill', + name: 'Same', + category: '通用', + // interactive kit defaults interactive:true; no --yes — should still not cancel + }); + + expect(uiMocks.promptConfirm).not.toHaveBeenCalled(); + expect(httpMocks.apiRequestForm).toHaveBeenCalled(); + } finally { + await rm(workdir, { recursive: true, force: true }); + } + }); + it('publishes SKILL.md from disk (mocked HTTP)', async () => { const workdir = await makeTmpWorkdir(); try { diff --git a/dt-skill/src/cli/commands/publish.ts b/dt-skill/src/cli/commands/publish.ts index a392c4d..97f7c7e 100644 --- a/dt-skill/src/cli/commands/publish.ts +++ b/dt-skill/src/cli/commands/publish.ts @@ -10,13 +10,20 @@ import { ApiV1SkillResponseSchema, normalizeClawScanNote, } from '../../schema/index.js'; -import { listPublishFiles } from '../../skills.js'; +import { hashSkillFiles, listPublishFiles } from '../../skills.js'; import { searchMultiselect } from '../prompts/search-multiselect.js'; import { getRegistry } from '../registry.js'; import { findSkillFolders } from '../scanSkills.js'; import { sanitizeSlug, titleCase } from '../slug.js'; import type { GlobalOpts } from '../types.js'; -import { createSpinner, fail, formatError, isInteractive, promptConfirm, selectCategory } from '../ui.js'; +import { + createSpinner, + fail, + formatError, + isInteractive, + promptConfirm, + selectCategory, +} from '../ui.js'; /** Closed category enum aligned with Doraemon skills market. */ export const SKILL_CATEGORY_OPTIONS = [ @@ -101,6 +108,7 @@ export async function cmdPublish( // Detect whether slug already exists on registry (first publish needs category). let existingCategory: string | null = null; + let existingFingerprint: string | null = null; let skillExists = false; try { const existing = await apiRequest( @@ -109,8 +117,9 @@ export async function cmdPublish( ApiV1SkillResponseSchema ); skillExists = Boolean(existing?.skill); - const skillMeta = existing?.skill as { category?: string } | undefined; - if (skillMeta?.category) existingCategory = String(skillMeta.category); + if (existing?.skill?.category) existingCategory = String(existing.skill.category); + existingFingerprint = + existing?.skill?.fingerprint ?? existing?.latestVersion?.fingerprint ?? null; } catch { skillExists = false; } @@ -132,16 +141,6 @@ export async function cmdPublish( category = existingCategory; } - if (skillExists && isInteractive() && !options.yes) { - const ok = await promptConfirm( - `Skill "${slug}" already exists on the registry. Overwrite remote content if it changed?` - ); - if (!ok) { - console.log('Publish cancelled'); - return; - } - } - const spinner = createSpinner(`Preparing ${slug}`); try { const filesOnDisk = await ensureRootManifestFile(folder, await listPublishFiles(folder)); @@ -155,6 +154,29 @@ export async function cmdPublish( fail('SKILL.md required'); } + // Confirm overwrite only when remote exists AND content hash differs (Decision 18). + const localFingerprint = hashSkillFiles( + filesOnDisk.map((file) => ({ + relPath: file.relPath, + bytes: file.bytes, + })) + ).fingerprint; + const contentChanged = + skillExists && + Boolean(existingFingerprint) && + localFingerprint !== existingFingerprint; + if (contentChanged && isInteractive() && !options.yes) { + spinner.stop(); + const ok = await promptConfirm( + `Skill "${slug}" exists and content changed. Overwrite remote?` + ); + if (!ok) { + console.log('Publish cancelled'); + return; + } + spinner.start(`Publishing ${slug}`); + } + const form = new FormData(); form.set( 'payload', diff --git a/dt-skill/src/cli/commands/skills.test.ts b/dt-skill/src/cli/commands/skills.test.ts index 752d089..148fd2a 100644 --- a/dt-skill/src/cli/commands/skills.test.ts +++ b/dt-skill/src/cli/commands/skills.test.ts @@ -304,7 +304,7 @@ describe('cmdUpdate', () => { vi.mocked(stat).mockResolvedValue({} as Awaited>); await expect(cmdUpdate(makeOpts(), 'demo', { force: true }, false)).rejects.toThrow( - 'download failed' + /Failed to update 1 skill|download failed/ ); expect(rm).not.toHaveBeenCalledWith('/work/.agents/skills/demo', { @@ -327,7 +327,7 @@ describe('cmdUpdate', () => { vi.mocked(extractZipToDir).mockRejectedValue(new Error('extract failed')); await expect(cmdUpdate(makeOpts(), 'demo', { force: true }, false)).rejects.toThrow( - 'extract failed' + /Failed to update 1 skill|extract failed/ ); expect(renameMock).not.toHaveBeenCalled(); @@ -371,7 +371,82 @@ describe('cmdUpdate', () => { other: { version: '2.0.0', installedAt: expect.any(Number) }, }, }); - expect(mockLog).toHaveBeenCalledWith('Skipped 1 pinned skill: demo'); + expect(mockLog).toHaveBeenCalledWith( + 'Update summary: 1 updated, 0 up to date, 1 pinned skipped, 0 failed' + ); + expect(mockLog).toHaveBeenCalledWith(' pinned skipped: demo'); + }); + + it('bare update equals --all (skips pinned, updates others)', async () => { + mockApiRequest.mockResolvedValue({ + latestVersion: { version: '2.0.0', fingerprint: 'remote-new' }, + moderation: null, + }); + mockDownloadZip.mockResolvedValue(new Uint8Array([1, 2, 3])); + vi.mocked(readLockfile).mockResolvedValue({ + version: 1, + skills: { + demo: { version: '0.1.0', installedAt: 123, pinned: true, pinReason: 'hold' }, + other: { version: '1.0.0', installedAt: 456 }, + }, + }); + vi.mocked(writeLockfile).mockResolvedValue(); + vi.mocked(readSkillOrigin).mockResolvedValue(null); + vi.mocked(writeSkillOrigin).mockResolvedValue(); + vi.mocked(extractZipToDir).mockResolvedValue(); + vi.mocked(listTextFiles).mockResolvedValue([]); + vi.mocked(hashSkillFiles).mockReturnValue({ fingerprint: 'local-old', files: [] }); + vi.mocked(stat).mockRejectedValue(new Error('missing')); + vi.mocked(rm).mockResolvedValue(); + + // bare update (no slug, no --all) + await cmdUpdate(makeOpts(), undefined, {}, false); + + expect(mockApiRequest).toHaveBeenCalledTimes(1); + const [, args] = mockApiRequest.mock.calls[0] ?? []; + expect(args?.path).toBe(`${ApiRoutes.skills}/${encodeURIComponent('other')}`); + expect(mockLog).toHaveBeenCalledWith( + expect.stringMatching(/Update summary: 1 updated/) + ); + }); + + it('continues updating remaining skills when one fails', async () => { + mockApiRequest + .mockResolvedValueOnce({ + latestVersion: { version: '2.0.0', fingerprint: 'fp-a' }, + moderation: null, + }) + .mockResolvedValueOnce({ + latestVersion: { version: '2.0.0', fingerprint: 'fp-b' }, + moderation: null, + }); + mockDownloadZip + .mockRejectedValueOnce(new Error('download failed')) + .mockResolvedValueOnce(new Uint8Array([1, 2, 3])); + vi.mocked(readLockfile).mockResolvedValue({ + version: 1, + skills: { + broken: { version: '1.0.0', installedAt: 1 }, + ok: { version: '1.0.0', installedAt: 2 }, + }, + }); + vi.mocked(writeLockfile).mockResolvedValue(); + vi.mocked(readSkillOrigin).mockResolvedValue(null); + vi.mocked(writeSkillOrigin).mockResolvedValue(); + vi.mocked(extractZipToDir).mockResolvedValue(); + vi.mocked(listTextFiles).mockResolvedValue([]); + vi.mocked(hashSkillFiles).mockReturnValue({ fingerprint: 'local', files: [] }); + vi.mocked(stat).mockRejectedValue(new Error('missing')); + vi.mocked(rm).mockResolvedValue(); + + await expect(cmdUpdate(makeOpts(), undefined, { all: true }, false)).rejects.toThrow( + /Failed to update 1 skill/ + ); + + expect(mockDownloadZip).toHaveBeenCalledTimes(2); + expect(mockLog).toHaveBeenCalledWith( + expect.stringMatching(/Update summary: 1 updated.*1 failed/) + ); }); it('uses path-based skill lookup when no local fingerprint is available', async () => { diff --git a/dt-skill/src/cli/commands/skills.ts b/dt-skill/src/cli/commands/skills.ts index 946d70a..49686e2 100644 --- a/dt-skill/src/cli/commands/skills.ts +++ b/dt-skill/src/cli/commands/skills.ts @@ -712,7 +712,6 @@ export async function cmdUpdate( ) { const slug = slugArg ? normalizeSkillSlugOrFail(slugArg) : undefined; // Bare `update` == update all tracked skills (same as --all). --all kept for compatibility. - const all = Boolean(options.all) || !slug; if (slug && options.all) fail('Use either or --all'); if (options.version && !slug) fail('--version requires a single '); if (options.version && !semver.valid(options.version)) fail('--version must be valid semver'); @@ -746,6 +745,11 @@ export async function cmdUpdate( return; } + const updated: string[] = []; + const alreadyCurrent: string[] = []; + const failed: Array<{ slug: string; error: string }> = []; + let lockDirty = false; + for (const entry of slugs) { const spinner = createSpinner(`Checking ${entry}`); try { @@ -764,6 +768,7 @@ export async function cmdUpdate( if (skillMeta.moderation?.isMalwareBlocked) { spinner.fail(`${entry}: blocked as malicious`); console.log(' This skill has been flagged as malware and cannot be updated.'); + failed.push({ slug: entry, error: 'blocked as malicious' }); continue; } @@ -798,24 +803,30 @@ export async function cmdUpdate( localFingerprint = lock.skills[entry].fingerprint ?? null; } + const metaFingerprint = + skillMeta.latestVersion?.fingerprint ?? skillMeta.skill?.fingerprint ?? null; + let resolveResult: ResolveResult; if (localFingerprint) { resolveResult = await resolveSkillVersion(registry, entry, localFingerprint); } else { - resolveResult = { match: null, latestVersion: skillMeta.latestVersion ?? null }; + resolveResult = { + match: null, + latestVersion: skillMeta.latestVersion + ? { + version: skillMeta.latestVersion.version, + fingerprint: metaFingerprint, + } + : null, + }; } const latest = resolveResult.latestVersion?.version ?? skillMeta.latestVersion?.version ?? null; const remoteFingerprint = - (resolveResult.latestVersion as { fingerprint?: string } | null | undefined) - ?.fingerprint ?? - (skillMeta.latestVersion as { fingerprint?: string } | undefined)?.fingerprint ?? - (skillMeta.skill as { fingerprint?: string } | undefined)?.fingerprint ?? - null; + resolveResult.latestVersion?.fingerprint ?? metaFingerprint ?? null; // Hash model: equal fingerprint (or resolve match) → up to date; else pull latest. - // Do NOT treat "disk matches install origin" as up-to-date — remote may have moved. const hashMatches = Boolean(localFingerprint) && Boolean(remoteFingerprint) && @@ -825,18 +836,27 @@ export async function cmdUpdate( if (!latest) { spinner.fail(`${entry}: not found`); + failed.push({ slug: entry, error: 'not found' }); continue; } if (contentUpToDate && !options.version) { const keepVersion = resolveResult.match?.version ?? latest; - lock.skills[entry] = withPinnedMetadata( - keepVersion, - lock.skills[entry]?.installedAt ?? Date.now(), - lock.skills[entry], - remoteFingerprint ?? localFingerprint - ); - await writeLockfile(installWorkdir, lock); + const nextFp = remoteFingerprint ?? localFingerprint ?? null; + const prev = lock.skills[entry]; + const needsLockWrite = + prev?.version !== keepVersion || + (nextFp != null && prev?.fingerprint !== nextFp); + if (needsLockWrite) { + lock.skills[entry] = withPinnedMetadata( + keepVersion, + prev?.installedAt ?? Date.now(), + prev, + nextFp + ); + lockDirty = true; + await writeLockfile(installWorkdir, lock); + } spinner.succeed( `${entry}: up to date${ remoteFingerprint @@ -846,6 +866,7 @@ export async function cmdUpdate( : '' }` ); + alreadyCurrent.push(entry); continue; } @@ -853,6 +874,7 @@ export async function cmdUpdate( const targetVersion = options.version ?? latest; if (options.version && resolveMatched && resolveResult.match?.version === targetVersion) { spinner.succeed(`${entry}: already at ${targetVersion}`); + alreadyCurrent.push(entry); continue; } @@ -894,6 +916,7 @@ export async function cmdUpdate( } // 每条成功更新后即持久化 lockfile，崩溃不再让磁盘与锁漂移 + lockDirty = true; await writeLockfile(installWorkdir, lock); spinner.succeed( `${entry}: updated${ @@ -902,18 +925,33 @@ export async function cmdUpdate( : ` -> ${targetVersion}` }` ); + updated.push(entry); } catch (error) { spinner.fail(formatError(error)); - throw error; + // Spec: partial failures continue remaining skills; non-zero exit after summary. + failed.push({ slug: entry, error: formatError(error) }); } } - await writeLockfile(installWorkdir, lock); + if (lockDirty) { + await writeLockfile(installWorkdir, lock); + } + + // Summary: updated / already-current / skipped-pinned / failed + console.log(''); + console.log( + `Update summary: ${updated.length} updated, ${alreadyCurrent.length} up to date, ${skippedPinned.length} pinned skipped, ${failed.length} failed` + ); + if (updated.length > 0) console.log(` updated: ${updated.join(', ')}`); + if (alreadyCurrent.length > 0) console.log(` up to date: ${alreadyCurrent.join(', ')}`); if (skippedPinned.length > 0) { - const suffix = skippedPinned.length === 1 ? '' : 's'; - console.log( - `Skipped ${skippedPinned.length} pinned skill${suffix}: ${skippedPinned.join(', ')}` - ); + console.log(` pinned skipped: ${skippedPinned.join(', ')}`); + } + if (failed.length > 0) { + for (const item of failed) { + console.log(` failed ${item.slug}: ${item.error}`); + } + fail(`Failed to update ${failed.length} skill(s)`); } } diff --git a/dt-skill/src/cli/types.ts b/dt-skill/src/cli/types.ts index dbbbdce..aa05ec4 100644 --- a/dt-skill/src/cli/types.ts +++ b/dt-skill/src/cli/types.ts @@ -16,7 +16,12 @@ export type GlobalOpts = { yes?: boolean; }; +export type ResolveVersionRef = { + version: string; + fingerprint?: string | null; +}; + export type ResolveResult = { - match: { version: string } | null; - latestVersion: { version: string } | null; + match: ResolveVersionRef | null; + latestVersion: ResolveVersionRef | null; }; diff --git a/test/skills-registry-contract.test.js b/test/skills-registry-contract.test.js index 629b4fd..abcc730 100644 --- a/test/skills-registry-contract.test.js +++ b/test/skills-registry-contract.test.js @@ -629,6 +629,30 @@ test('publishSkill returns ok: true and string skillId and versionId', async () assert.equal(result.unchanged, false); }); +test('publishSkill rejects invalid category', async () => { + const service = Object.create(SkillsRegistryService.prototype); + service.app = createMockApp(); + service.ctx = createMockCtx(); + service.ctx.throw = (status, message) => { + const err = new Error(message); + err.status = status; + throw err; + }; + + await assert.rejects( + () => + service.publishSkill( + { + slug: 'bad-cat', + displayName: 'Bad', + category: 'not-a-real-category', + }, + [{ filepath: 'SKILL.md', content: '# x\n' }] + ), + /category 无效/ + ); +}); + test('publishSkill accepts missing version (defaults to 0.0.0)', async () => { const service = Object.create(SkillsRegistryService.prototype); service.app = createMockApp({ From 70e53d9f28a4dd63c0e14802b2f36c30c9c847ed Mon Sep 17 00:00:00 2001 From: huaiju Date: Tue, 28 Jul 2026 11:16:32 +0800 Subject: [PATCH 03/29] =?UTF-8?q?refactor(skills):=20deepen=20hash=20sync?= =?UTF-8?q?=20(C1=E2=80=93C4=20architecture)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract SkillSync decide module and cmdUpdate command so the happy path compares content fingerprints only (no resolve dual-signal). Registry detail exposes fingerprint on skill alone; publishSkill splits normalize / noop / replace internals. skills.ts re-exports update; helpers share slug and directory replace utilities. --- app/service/skillsRegistry.js | 113 ++++---- dt-skill/src/cli/commands/publish.ts | 8 +- dt-skill/src/cli/commands/skillHelpers.ts | 70 +++++ dt-skill/src/cli/commands/skills.ts | 314 +--------------------- dt-skill/src/cli/commands/update.ts | 255 ++++++++++++++++++ dt-skill/src/cli/skillSync.test.ts | 71 +++++ dt-skill/src/cli/skillSync.ts | 87 ++++++ test/skills-registry-contract.test.js | 2 +- 8 files changed, 552 insertions(+), 368 deletions(-) create mode 100644 dt-skill/src/cli/commands/skillHelpers.ts create mode 100644 dt-skill/src/cli/commands/update.ts create mode 100644 dt-skill/src/cli/skillSync.test.ts create mode 100644 dt-skill/src/cli/skillSync.ts diff --git a/app/service/skillsRegistry.js b/app/service/skillsRegistry.js index 23edc16..dc26a6f 100644 --- a/app/service/skillsRegistry.js +++ b/app/service/skillsRegistry.js @@ -191,13 +191,13 @@ class SkillsRegistryService extends Service { category: skill.category || '通用', fingerprint, }, + // fingerprint lives only on skill (single-slot current content). latestVersion: version ? { version, createdAt: updatedAt, changelog: '', license: null, - fingerprint, } : null, owner: null, @@ -410,20 +410,8 @@ class SkillsRegistryService extends Service { }); } - // Publish or update a skill (single-slot per slug; content hash is the change signal) - async publishSkill(payload, files) { - const { SkillsItem, SkillsFile, SkillsSource } = this.app.model; - const { slug, displayName, tags } = payload; - const version = this.resolvePublishVersion(payload.version); - const category = this.resolvePublishCategory(payload.category); - - if (!SKILL_SLUG_PATTERN.test(String(slug || ''))) { - this.ctx.throw(400, 'slug 格式无效'); - } - - const parsedTags = Array.isArray(tags) ? tags : []; - - // file.content 直接给（内存形态，测试/部分调用方）优先；否则读磁盘临时文件（真实 multipart）。 + /** Normalize multipart/in-memory uploads into stored-file shape. Requires SKILL.md. */ + normalizePublishFiles(files) { const processedFiles = []; for (const file of files) { const originalName = file.filename || path.basename(file.filepath || ''); @@ -450,7 +438,6 @@ class SkillsRegistryService extends Service { this.ctx.throw(400, `读取上传文件 ${originalName} 失败`); } } else { - // I1: 既无 content 也无可读磁盘文件，必须报错而非静默存空 this.ctx.throw(400, `上传文件不存在: ${originalName}`); } processedFiles.push({ @@ -461,7 +448,6 @@ class SkillsRegistryService extends Service { }); } - // Check for SKILL.md const skillMdFile = processedFiles.find( (f) => f.filename && f.filename.toLowerCase().endsWith('skill.md') ); @@ -470,6 +456,58 @@ class SkillsRegistryService extends Service { this.ctx.throw(400, `上传内容必须包含 SKILL.md。已上传: ${uploadedNames}`); } + return { processedFiles, skillMdFile }; + } + + async tryPublishUnchanged(skill, incomingFingerprint, version) { + if (!skill || skill.is_delete !== 0) return null; + const existingFingerprint = await this.computeSkillFingerprint(skill.id); + if (!existingFingerprint || existingFingerprint !== incomingFingerprint) return null; + return { + ok: true, + skillId: String(skill.id), + versionId: `v${skill.version || version}`, + fingerprint: existingFingerprint, + unchanged: true, + }; + } + + async replaceSkillStoredFiles(skill, processedFiles, transaction) { + const { SkillsFile } = this.app.model; + await SkillsFile.update( + { is_delete: 1 }, + { where: { skill_id: skill.id }, transaction } + ); + for (const file of processedFiles) { + await SkillsFile.create( + { + skill_id: skill.id, + file_path: file.relPath, + language: this.detectLanguage(file.filename), + size: Buffer.byteLength(file.content, file.isBinary ? 'base64' : 'utf8'), + is_binary: file.isBinary ? 1 : 0, + encoding: file.isBinary ? 'base64' : 'utf8', + content: file.content, + }, + { transaction } + ); + } + await skill.update({ file_count: processedFiles.length }, { transaction }); + } + + // Publish or update a skill (single-slot per slug; content hash is the change signal) + async publishSkill(payload, files) { + const { SkillsItem, SkillsSource } = this.app.model; + const { slug, displayName, tags } = payload; + const version = this.resolvePublishVersion(payload.version); + const category = this.resolvePublishCategory(payload.category); + + if (!SKILL_SLUG_PATTERN.test(String(slug || ''))) { + this.ctx.throw(400, 'slug 格式无效'); + } + + const parsedTags = Array.isArray(tags) ? tags : []; + const { processedFiles, skillMdFile } = this.normalizePublishFiles(files); const incomingFingerprint = this.computeIncomingFingerprint(processedFiles); return await this.app.model.transaction(async (t) => { @@ -485,19 +523,8 @@ class SkillsRegistryService extends Service { let skill = await SkillsItem.findOne({ where: { slug }, transaction: t }); - // Same content already published → pure no-op (no meta churn) - if (skill && skill.is_delete === 0) { - const existingFingerprint = await this.computeSkillFingerprint(skill.id); - if (existingFingerprint && existingFingerprint === incomingFingerprint) { - return { - ok: true, - skillId: String(skill.id), - versionId: `v${skill.version || version}`, - fingerprint: existingFingerprint, - unchanged: true, - }; - } - } + const noop = await this.tryPublishUnchanged(skill, incomingFingerprint, version); + if (noop) return noop; if (skill) { const updatePayload = { @@ -511,11 +538,6 @@ class SkillsRegistryService extends Service { }; if (category) updatePayload.category = category; await skill.update(updatePayload, { transaction: t }); - // Delete old files - await SkillsFile.update( - { is_delete: 1 }, - { where: { skill_id: skill.id }, transaction: t } - ); } else { skill = await SkillsItem.create( { @@ -527,30 +549,13 @@ class SkillsRegistryService extends Service { tags: JSON.stringify(parsedTags), skill_md: skillMdFile.content || '', category: category || '通用', - file_count: files.length, - }, - { transaction: t } - ); - } - - // Save files - for (const file of processedFiles) { - await SkillsFile.create( - { - skill_id: skill.id, - file_path: file.relPath, - language: this.detectLanguage(file.filename), - size: Buffer.byteLength(file.content, file.isBinary ? 'base64' : 'utf8'), - is_binary: file.isBinary ? 1 : 0, - encoding: file.isBinary ? 'base64' : 'utf8', - content: file.content, + file_count: processedFiles.length, }, { transaction: t } ); } - // Update file count - await skill.update({ file_count: files.length }, { transaction: t }); + await this.replaceSkillStoredFiles(skill, processedFiles, t); return { ok: true, diff --git a/dt-skill/src/cli/commands/publish.ts b/dt-skill/src/cli/commands/publish.ts index 97f7c7e..f56aacb 100644 --- a/dt-skill/src/cli/commands/publish.ts +++ b/dt-skill/src/cli/commands/publish.ts @@ -37,6 +37,8 @@ export const SKILL_CATEGORY_OPTIONS = [ '其他', ] as const; +const SKILL_CATEGORY_SET = new Set(SKILL_CATEGORY_OPTIONS); + /** Internal compatibility version when author omits --version (hash is the change signal). */ const DEFAULT_PUBLISH_VERSION = '0.0.0'; @@ -118,14 +120,14 @@ export async function cmdPublish( ); skillExists = Boolean(existing?.skill); if (existing?.skill?.category) existingCategory = String(existing.skill.category); - existingFingerprint = - existing?.skill?.fingerprint ?? existing?.latestVersion?.fingerprint ?? null; + // Canonical: skill.fingerprint only (single-slot current content) + existingFingerprint = existing?.skill?.fingerprint ?? null; } catch { skillExists = false; } let category = options.category?.trim() || ''; - if (category && !SKILL_CATEGORY_OPTIONS.includes(category as (typeof SKILL_CATEGORY_OPTIONS)[number])) { + if (category && !SKILL_CATEGORY_SET.has(category)) { fail(`--category must be one of: ${SKILL_CATEGORY_OPTIONS.join(', ')}`); } if (!skillExists && !category) { diff --git a/dt-skill/src/cli/commands/skillHelpers.ts b/dt-skill/src/cli/commands/skillHelpers.ts new file mode 100644 index 0000000..de91952 --- /dev/null +++ b/dt-skill/src/cli/commands/skillHelpers.ts @@ -0,0 +1,70 @@ +import { mkdir, mkdtemp, rename, rm, stat } from 'node:fs/promises'; +import { basename, dirname, join } from 'node:path'; + +import { extractZipToDir } from '../../skills.js'; +import { fail } from '../ui.js'; + +export function normalizeSkillSlugOrFail(raw: string) { + const slug = raw.trim(); + if (!slug) fail('Slug required'); + if (slug.includes('/') || slug.includes('\\') || slug.includes('..')) { + fail(`Invalid slug: ${slug}`); + } + return slug; +} + +export function isSafeSkillSlug(slug: string) { + return Boolean(slug) && !slug.includes('/') && !slug.includes('\\') && !slug.includes('..'); +} + +export async function fileExists(path: string) { + try { + await stat(path); + return true; + } catch { + return false; + } +} + +export async function prepareSkillUpdate(zip: Uint8Array, target: string) { + await mkdir(dirname(target), { recursive: true }); + const preparedDir = await mkdtemp(join(dirname(target), `.${basename(target)}-update-`)); + try { + await extractZipToDir(zip, preparedDir); + return preparedDir; + } catch (error) { + await rm(preparedDir, { recursive: true, force: true }).catch(() => {}); + throw error; + } +} + +export async function replaceSkillDirectory( + preparedDir: string, + target: string, + targetExists: boolean +) { + const backupDir = `${preparedDir}-previous`; + let movedExisting = false; + + try { + if (targetExists) { + await rename(target, backupDir); + movedExisting = true; + } + await rename(preparedDir, target); + } catch (error) { + if (movedExisting) { + try { + await rename(backupDir, target); + } catch (rollbackError) { + throw new AggregateError( + [error, rollbackError], + `Failed to replace ${target} and restore the previous installation` + ); + } + } + throw error; + } + + await rm(backupDir, { recursive: true, force: true }).catch(() => {}); +} diff --git a/dt-skill/src/cli/commands/skills.ts b/dt-skill/src/cli/commands/skills.ts index 49686e2..62bd6b3 100644 --- a/dt-skill/src/cli/commands/skills.ts +++ b/dt-skill/src/cli/commands/skills.ts @@ -1,4 +1,4 @@ -import { lstat, mkdir, mkdtemp, rename, rm, stat } from 'node:fs/promises'; +import { lstat, mkdir, rm, stat } from 'node:fs/promises'; import { homedir } from 'node:os'; import { basename, dirname, join } from 'node:path'; import semver from 'semver'; @@ -17,8 +17,6 @@ import { ApiV1SearchResponseSchema, type ApiV1SkillListResponse, ApiV1SkillListResponseSchema, - type ApiV1SkillResolveResponse, - ApiV1SkillResolveResponseSchema, type ApiV1SkillResponse, ApiV1SkillResponseSchema, ApiV1SkillVersionResponseSchema, @@ -47,7 +45,7 @@ import { import { installExtractedSkill, type InstallTargets } from '../installerPipeline.js'; import { cancelSymbol, searchMultiselect } from '../prompts/search-multiselect.js'; import { getRegistry } from '../registry.js'; -import type { GlobalOpts, ResolveResult } from '../types.js'; +import type { GlobalOpts } from '../types.js'; import { createSpinner, fail, @@ -704,297 +702,8 @@ async function installOneSkill( } } -export async function cmdUpdate( - opts: GlobalOpts, - slugArg: string | undefined, - options: { all?: boolean; version?: string; force?: boolean }, - inputAllowed: boolean -) { - const slug = slugArg ? normalizeSkillSlugOrFail(slugArg) : undefined; - // Bare `update` == update all tracked skills (same as --all). --all kept for compatibility. - if (slug && options.all) fail('Use either or --all'); - if (options.version && !slug) fail('--version requires a single '); - if (options.version && !semver.valid(options.version)) fail('--version must be valid semver'); - - const installWorkdir = opts.workdir; - const installDir = opts.dir; - - const lock = await readLockfile(installWorkdir); - if (slug && isPinnedSkillEntry(lock.skills[slug])) { - fail(`skill "${slug}" is pinned; run \`dt-skill unpin ${slug}\` first`); - } - const allowPrompt = isInteractive() && inputAllowed; - - const registry = await getRegistry(opts, { cache: true }); - const requestedSlugs = slug ? [slug] : Object.keys(lock.skills).filter(isSafeSkillSlug); - const skippedPinned = slug - ? [] - : requestedSlugs.filter((entry) => isPinnedSkillEntry(lock.skills[entry])); - const slugs = slug - ? requestedSlugs - : requestedSlugs.filter((entry) => !isPinnedSkillEntry(lock.skills[entry])); - if (slugs.length === 0) { - if (skippedPinned.length > 0) { - const suffix = skippedPinned.length === 1 ? '' : 's'; - console.log( - `Skipped ${skippedPinned.length} pinned skill${suffix}: ${skippedPinned.join(', ')}` - ); - return; - } - console.log('No installed skills.'); - return; - } - - const updated: string[] = []; - const alreadyCurrent: string[] = []; - const failed: Array<{ slug: string; error: string }> = []; - let lockDirty = false; - - for (const entry of slugs) { - const spinner = createSpinner(`Checking ${entry}`); - try { - const target = join(installDir, entry); - const exists = await fileExists(target); - const existingOrigin = exists ? await readSkillOrigin(target) : null; - - // Always fetch skill metadata to check moderation status - const skillMeta = await apiRequest( - registry, - { method: 'GET', path: `${ApiRoutes.skills}/${encodeURIComponent(entry)}` }, - ApiV1SkillResponseSchema - ); - - // Check moderation status before proceeding - if (skillMeta.moderation?.isMalwareBlocked) { - spinner.fail(`${entry}: blocked as malicious`); - console.log(' This skill has been flagged as malware and cannot be updated.'); - failed.push({ slug: entry, error: 'blocked as malicious' }); - continue; - } - - if (skillMeta.moderation?.isSuspicious && !options.force) { - spinner.stop(); - console.log( - `\n⚠️ Warning: "${entry}" is flagged for ClawHub security review.\n` + - ' This skill may contain risky patterns (crypto keys, external APIs, eval, etc.)\n' - ); - if (allowPrompt) { - const confirm = await promptConfirm('Update anyway?'); - if (!confirm) { - console.log(`${entry}: skipped`); - continue; - } - spinner.start(`Checking ${entry}`); - } else { - console.log(`${entry}: skipped (use --force to update suspicious skills)`); - continue; - } - } - - let localFingerprint: string | null = null; - if (exists) { - const filesOnDisk = await listTextFiles(target); - if (filesOnDisk.length > 0) { - const hashed = hashSkillFiles(filesOnDisk); - localFingerprint = hashed.fingerprint; - } - } - if (!localFingerprint && lock.skills[entry]?.fingerprint) { - localFingerprint = lock.skills[entry].fingerprint ?? null; - } - - const metaFingerprint = - skillMeta.latestVersion?.fingerprint ?? skillMeta.skill?.fingerprint ?? null; - - let resolveResult: ResolveResult; - if (localFingerprint) { - resolveResult = await resolveSkillVersion(registry, entry, localFingerprint); - } else { - resolveResult = { - match: null, - latestVersion: skillMeta.latestVersion - ? { - version: skillMeta.latestVersion.version, - fingerprint: metaFingerprint, - } - : null, - }; - } - - const latest = - resolveResult.latestVersion?.version ?? skillMeta.latestVersion?.version ?? null; - const remoteFingerprint = - resolveResult.latestVersion?.fingerprint ?? metaFingerprint ?? null; - - // Hash model: equal fingerprint (or resolve match) → up to date; else pull latest. - const hashMatches = - Boolean(localFingerprint) && - Boolean(remoteFingerprint) && - localFingerprint === remoteFingerprint; - const resolveMatched = Boolean(resolveResult.match?.version); - const contentUpToDate = hashMatches || resolveMatched; - - if (!latest) { - spinner.fail(`${entry}: not found`); - failed.push({ slug: entry, error: 'not found' }); - continue; - } - - if (contentUpToDate && !options.version) { - const keepVersion = resolveResult.match?.version ?? latest; - const nextFp = remoteFingerprint ?? localFingerprint ?? null; - const prev = lock.skills[entry]; - const needsLockWrite = - prev?.version !== keepVersion || - (nextFp != null && prev?.fingerprint !== nextFp); - if (needsLockWrite) { - lock.skills[entry] = withPinnedMetadata( - keepVersion, - prev?.installedAt ?? Date.now(), - prev, - nextFp - ); - lockDirty = true; - await writeLockfile(installWorkdir, lock); - } - spinner.succeed( - `${entry}: up to date${ - remoteFingerprint - ? ` (${remoteFingerprint.slice(0, 12)}…)` - : keepVersion - ? ` (${keepVersion})` - : '' - }` - ); - alreadyCurrent.push(entry); - continue; - } - - // Explicit legacy version pin path - const targetVersion = options.version ?? latest; - if (options.version && resolveMatched && resolveResult.match?.version === targetVersion) { - spinner.succeed(`${entry}: already at ${targetVersion}`); - alreadyCurrent.push(entry); - continue; - } - - if (spinner.isSpinning) { - spinner.text = `Updating ${entry}`; - } else { - spinner.start(`Updating ${entry}`); - } - const zip = await downloadZip(registry, { - slug: entry, - version: targetVersion, - }); - const preparedDir = await prepareSkillUpdate(zip, target); - - try { - const installedFiles = await listTextFiles(preparedDir); - const installedFingerprint = - installedFiles.length > 0 - ? hashSkillFiles(installedFiles).fingerprint - : undefined; - await writeSkillOrigin(preparedDir, { - version: 1, - registry: existingOrigin?.registry ?? registry, - slug: existingOrigin?.slug ?? entry, - installedVersion: targetVersion, - installedAt: existingOrigin?.installedAt ?? Date.now(), - fingerprint: installedFingerprint, - }); - await replaceSkillDirectory(preparedDir, target, exists); - lock.skills[entry] = withPinnedMetadata( - targetVersion, - Date.now(), - lock.skills[entry], - installedFingerprint - ); - } catch (error) { - await rm(preparedDir, { recursive: true, force: true }).catch(() => {}); - throw error; - } - - // 每条成功更新后即持久化 lockfile，崩溃不再让磁盘与锁漂移 - lockDirty = true; - await writeLockfile(installWorkdir, lock); - spinner.succeed( - `${entry}: updated${ - lock.skills[entry]?.fingerprint - ? ` hash=${lock.skills[entry].fingerprint!.slice(0, 12)}…` - : ` -> ${targetVersion}` - }` - ); - updated.push(entry); - } catch (error) { - spinner.fail(formatError(error)); - // Spec: partial failures continue remaining skills; non-zero exit after summary. - failed.push({ slug: entry, error: formatError(error) }); - } - } - - if (lockDirty) { - await writeLockfile(installWorkdir, lock); - } - - // Summary: updated / already-current / skipped-pinned / failed - console.log(''); - console.log( - `Update summary: ${updated.length} updated, ${alreadyCurrent.length} up to date, ${skippedPinned.length} pinned skipped, ${failed.length} failed` - ); - if (updated.length > 0) console.log(` updated: ${updated.join(', ')}`); - if (alreadyCurrent.length > 0) console.log(` up to date: ${alreadyCurrent.join(', ')}`); - if (skippedPinned.length > 0) { - console.log(` pinned skipped: ${skippedPinned.join(', ')}`); - } - if (failed.length > 0) { - for (const item of failed) { - console.log(` failed ${item.slug}: ${item.error}`); - } - fail(`Failed to update ${failed.length} skill(s)`); - } -} - -async function prepareSkillUpdate(zip: Uint8Array, target: string) { - await mkdir(dirname(target), { recursive: true }); - const preparedDir = await mkdtemp(join(dirname(target), `.${basename(target)}-update-`)); - try { - await extractZipToDir(zip, preparedDir); - return preparedDir; - } catch (error) { - await rm(preparedDir, { recursive: true, force: true }).catch(() => {}); - throw error; - } -} - -async function replaceSkillDirectory(preparedDir: string, target: string, targetExists: boolean) { - const backupDir = `${preparedDir}-previous`; - let movedExisting = false; - - try { - if (targetExists) { - await rename(target, backupDir); - movedExisting = true; - } - await rename(preparedDir, target); - } catch (error) { - if (movedExisting) { - try { - await rename(backupDir, target); - } catch (rollbackError) { - throw new AggregateError( - [error, rollbackError], - `Failed to replace ${target} and restore the previous installation` - ); - } - } - throw error; - } - - if (movedExisting) { - await rm(backupDir, { recursive: true, force: true }).catch(() => {}); - } -} +/** Update lives in its own module (hash-only sync). */ +export { cmdUpdate } from './update.js'; export async function cmdList(opts: GlobalOpts) { const installWorkdir = opts.workdir; @@ -1249,21 +958,6 @@ function resolveExploreSort(raw?: string): { sort: ExploreSort; apiSort: ApiExpl ); } -async function resolveSkillVersion( - registry: string, - slug: string, - hash: string -): Promise { - const url = registryUrl(ApiRoutes.resolve, registry); - url.searchParams.set('slug', slug); - url.searchParams.set('hash', hash); - return apiRequest( - registry, - { method: 'GET', url: url.toString() }, - ApiV1SkillResolveResponseSchema - ); -} - async function fileExists(path: string) { try { await stat(path); diff --git a/dt-skill/src/cli/commands/update.ts b/dt-skill/src/cli/commands/update.ts new file mode 100644 index 0000000..3523550 --- /dev/null +++ b/dt-skill/src/cli/commands/update.ts @@ -0,0 +1,255 @@ +import { join } from 'node:path'; +import { rm } from 'node:fs/promises'; +import semver from 'semver'; + +import { apiRequest, downloadZip } from '../../http.js'; +import { + isPinned as isPinnedSkillEntry, + readLockfile, + withPinnedMetadata, + writeLockfile, +} from '../../lockfile.js'; +import { + ApiRoutes, + type ApiV1SkillResponse, + ApiV1SkillResponseSchema, +} from '../../schema/index.js'; +import { + hashSkillFiles, + listTextFiles, + readSkillOrigin, + writeSkillOrigin, +} from '../../skills.js'; +import { getRegistry } from '../registry.js'; +import { decideSkillSync, remoteCurrentFromDetail } from '../skillSync.js'; +import type { GlobalOpts } from '../types.js'; +import { + createSpinner, + fail, + formatError, + isInteractive, + promptConfirm, +} from '../ui.js'; +import { + fileExists, + isSafeSkillSlug, + normalizeSkillSlugOrFail, + prepareSkillUpdate, + replaceSkillDirectory, +} from './skillHelpers.js'; + +/** + * Update installed skills by content hash (main path). + * No resolve API — remote identity is skill.fingerprint from detail. + */ +export async function cmdUpdate( + opts: GlobalOpts, + slugArg: string | undefined, + options: { all?: boolean; version?: string; force?: boolean }, + inputAllowed: boolean +) { + const slug = slugArg ? normalizeSkillSlugOrFail(slugArg) : undefined; + if (slug && options.all) fail('Use either or --all'); + if (options.version && !slug) fail('--version requires a single '); + if (options.version && !semver.valid(options.version)) fail('--version must be valid semver'); + + const installWorkdir = opts.workdir; + const installDir = opts.dir; + + const lock = await readLockfile(installWorkdir); + if (slug && isPinnedSkillEntry(lock.skills[slug])) { + fail(`skill "${slug}" is pinned; run \`dt-skill unpin ${slug}\` first`); + } + const allowPrompt = isInteractive() && inputAllowed; + + const registry = await getRegistry(opts, { cache: true }); + const requestedSlugs = slug ? [slug] : Object.keys(lock.skills).filter(isSafeSkillSlug); + const skippedPinned = slug + ? [] + : requestedSlugs.filter((entry) => isPinnedSkillEntry(lock.skills[entry])); + const slugs = slug + ? requestedSlugs + : requestedSlugs.filter((entry) => !isPinnedSkillEntry(lock.skills[entry])); + + if (slugs.length === 0) { + if (skippedPinned.length > 0) { + const suffix = skippedPinned.length === 1 ? '' : 's'; + console.log( + `Skipped ${skippedPinned.length} pinned skill${suffix}: ${skippedPinned.join(', ')}` + ); + return; + } + console.log('No installed skills.'); + return; + } + + const updated: string[] = []; + const alreadyCurrent: string[] = []; + const failed: Array<{ slug: string; error: string }> = []; + let lockDirty = false; + + for (const entry of slugs) { + const spinner = createSpinner(`Checking ${entry}`); + try { + const target = join(installDir, entry); + const exists = await fileExists(target); + const existingOrigin = exists ? await readSkillOrigin(target) : null; + + const skillMeta = await apiRequest( + registry, + { method: 'GET', path: `${ApiRoutes.skills}/${encodeURIComponent(entry)}` }, + ApiV1SkillResponseSchema + ); + + if (skillMeta.moderation?.isMalwareBlocked) { + spinner.fail(`${entry}: blocked as malicious`); + console.log(' This skill has been flagged as malware and cannot be updated.'); + failed.push({ slug: entry, error: 'blocked as malicious' }); + continue; + } + + if (skillMeta.moderation?.isSuspicious && !options.force) { + spinner.stop(); + console.log( + `\n⚠️ Warning: "${entry}" is flagged for ClawHub security review.\n` + + ' This skill may contain risky patterns (crypto keys, external APIs, eval, etc.)\n' + ); + if (allowPrompt) { + const confirm = await promptConfirm('Update anyway?'); + if (!confirm) { + console.log(`${entry}: skipped`); + continue; + } + spinner.start(`Checking ${entry}`); + } else { + console.log(`${entry}: skipped (use --force to update suspicious skills)`); + continue; + } + } + + let localFingerprint: string | null = null; + if (exists) { + const filesOnDisk = await listTextFiles(target); + if (filesOnDisk.length > 0) { + localFingerprint = hashSkillFiles(filesOnDisk).fingerprint; + } + } + if (!localFingerprint && lock.skills[entry]?.fingerprint) { + localFingerprint = lock.skills[entry].fingerprint ?? null; + } + + const remote = remoteCurrentFromDetail(skillMeta); + const decision = decideSkillSync({ + localFingerprint, + remote, + explicitVersion: options.version, + }); + + if (decision.action === 'missing') { + spinner.fail(`${entry}: not found`); + failed.push({ slug: entry, error: 'not found' }); + continue; + } + + if (decision.action === 'up_to_date') { + const prev = lock.skills[entry]; + const needsLockWrite = + prev?.version !== decision.version || + (decision.fingerprint != null && prev?.fingerprint !== decision.fingerprint); + if (needsLockWrite) { + lock.skills[entry] = withPinnedMetadata( + decision.version, + prev?.installedAt ?? Date.now(), + prev, + decision.fingerprint + ); + lockDirty = true; + await writeLockfile(installWorkdir, lock); + } + spinner.succeed( + `${entry}: up to date${ + decision.fingerprint + ? ` (${decision.fingerprint.slice(0, 12)}…)` + : ` (${decision.version})` + }` + ); + alreadyCurrent.push(entry); + continue; + } + + // decision.action === 'update' + const targetVersion = decision.version; + if (spinner.isSpinning) { + spinner.text = `Updating ${entry}`; + } else { + spinner.start(`Updating ${entry}`); + } + + const zip = await downloadZip(registry, { + slug: entry, + version: targetVersion, + }); + const preparedDir = await prepareSkillUpdate(zip, target); + + try { + const installedFiles = await listTextFiles(preparedDir); + const installedFingerprint = + installedFiles.length > 0 + ? hashSkillFiles(installedFiles).fingerprint + : undefined; + await writeSkillOrigin(preparedDir, { + version: 1, + registry: existingOrigin?.registry ?? registry, + slug: existingOrigin?.slug ?? entry, + installedVersion: targetVersion, + installedAt: existingOrigin?.installedAt ?? Date.now(), + fingerprint: installedFingerprint, + }); + await replaceSkillDirectory(preparedDir, target, exists); + lock.skills[entry] = withPinnedMetadata( + targetVersion, + Date.now(), + lock.skills[entry], + installedFingerprint + ); + } catch (error) { + await rm(preparedDir, { recursive: true, force: true }).catch(() => {}); + throw error; + } + + lockDirty = true; + await writeLockfile(installWorkdir, lock); + spinner.succeed( + `${entry}: updated${ + lock.skills[entry]?.fingerprint + ? ` hash=${lock.skills[entry].fingerprint!.slice(0, 12)}…` + : ` -> ${targetVersion}` + }` + ); + updated.push(entry); + } catch (error) { + spinner.fail(formatError(error)); + failed.push({ slug: entry, error: formatError(error) }); + } + } + + if (lockDirty) { + await writeLockfile(installWorkdir, lock); + } + + console.log(''); + console.log( + `Update summary: ${updated.length} updated, ${alreadyCurrent.length} up to date, ${skippedPinned.length} pinned skipped, ${failed.length} failed` + ); + if (updated.length > 0) console.log(` updated: ${updated.join(', ')}`); + if (alreadyCurrent.length > 0) console.log(` up to date: ${alreadyCurrent.join(', ')}`); + if (skippedPinned.length > 0) { + console.log(` pinned skipped: ${skippedPinned.join(', ')}`); + } + if (failed.length > 0) { + for (const item of failed) { + console.log(` failed ${item.slug}: ${item.error}`); + } + fail(`Failed to update ${failed.length} skill(s)`); + } +} diff --git a/dt-skill/src/cli/skillSync.test.ts b/dt-skill/src/cli/skillSync.test.ts new file mode 100644 index 0000000..f990ee8 --- /dev/null +++ b/dt-skill/src/cli/skillSync.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from 'vitest'; + +import { decideSkillSync, remoteCurrentFromDetail } from './skillSync.js'; + +describe('remoteCurrentFromDetail', () => { + it('prefers skill.fingerprint only', () => { + const remote = remoteCurrentFromDetail({ + skill: { fingerprint: 'aaa', version: '0.0.0' }, + latestVersion: { fingerprint: 'bbb', version: '1.0.0' }, + }); + expect(remote.fingerprint).toBe('aaa'); + expect(remote.version).toBe('0.0.0'); + }); + + it('falls back to latestVersion.version when skill has no version', () => { + const remote = remoteCurrentFromDetail({ + skill: { fingerprint: 'aaa', version: null }, + latestVersion: { version: '2.0.0' }, + }); + expect(remote.version).toBe('2.0.0'); + expect(remote.fingerprint).toBe('aaa'); + }); +}); + +describe('decideSkillSync', () => { + it('up_to_date when hashes match', () => { + expect( + decideSkillSync({ + localFingerprint: 'same', + remote: { fingerprint: 'same', version: '0.0.0' }, + }) + ).toEqual({ action: 'up_to_date', version: '0.0.0', fingerprint: 'same' }); + }); + + it('update when hashes differ', () => { + expect( + decideSkillSync({ + localFingerprint: 'old', + remote: { fingerprint: 'new', version: '0.0.0' }, + }) + ).toEqual({ action: 'update', version: '0.0.0', fingerprint: 'new' }); + }); + + it('missing when remote has neither version nor fingerprint', () => { + expect( + decideSkillSync({ + localFingerprint: 'x', + remote: { fingerprint: null, version: null }, + }) + ).toEqual({ action: 'missing' }); + }); + + it('explicitVersion is a side door to update', () => { + expect( + decideSkillSync({ + localFingerprint: 'same', + remote: { fingerprint: 'same', version: '0.0.0' }, + explicitVersion: '9.9.9', + }) + ).toEqual({ action: 'update', version: '9.9.9', fingerprint: 'same' }); + }); + + it('update when local missing but remote present', () => { + expect( + decideSkillSync({ + localFingerprint: null, + remote: { fingerprint: 'new', version: '0.0.0' }, + }) + ).toEqual({ action: 'update', version: '0.0.0', fingerprint: 'new' }); + }); +}); diff --git a/dt-skill/src/cli/skillSync.ts b/dt-skill/src/cli/skillSync.ts new file mode 100644 index 0000000..292b3be --- /dev/null +++ b/dt-skill/src/cli/skillSync.ts @@ -0,0 +1,87 @@ +/** + * Skill content sync decision (hash-only happy path). + * One deep module: local fingerprint vs remote current fingerprint. + * Version is only a download token for the registry zip endpoint. + */ + +export type RemoteCurrent = { + /** Canonical content id for the skill's current slot. */ + fingerprint: string | null; + /** Download token (registry may still key zip by version string). */ + version: string | null; +}; + +export type SyncDecision = + | { action: 'missing' } + | { action: 'up_to_date'; version: string; fingerprint: string | null } + | { action: 'update'; version: string; fingerprint: string | null }; + +/** + * Canonical remote identity from skill detail. + * Prefer skill.fingerprint only — single field (C2). + */ +export function remoteCurrentFromDetail(skillMeta: { + skill?: { + fingerprint?: string | null; + version?: string | null; + } | null; + latestVersion?: { + version?: string | null; + fingerprint?: string | null; + } | null; +}): RemoteCurrent { + const fingerprint = + skillMeta.skill?.fingerprint != null && skillMeta.skill.fingerprint !== '' + ? skillMeta.skill.fingerprint + : null; + const version = + (skillMeta.skill?.version != null && skillMeta.skill.version !== '' + ? skillMeta.skill.version + : null) ?? + (skillMeta.latestVersion?.version != null && skillMeta.latestVersion.version !== '' + ? skillMeta.latestVersion.version + : null); + return { fingerprint, version }; +} + +/** + * Decide whether local install matches remote current content. + * Main path: compare fingerprints. No resolve API. + * explicitVersion: legacy side door — always request that version token. + */ +export function decideSkillSync(args: { + localFingerprint: string | null; + remote: RemoteCurrent; + explicitVersion?: string; +}): SyncDecision { + if (args.explicitVersion) { + return { + action: 'update', + version: args.explicitVersion, + fingerprint: args.remote.fingerprint, + }; + } + + const version = args.remote.version || '0.0.0'; + if (!args.remote.version && !args.remote.fingerprint) { + return { action: 'missing' }; + } + + if ( + args.localFingerprint && + args.remote.fingerprint && + args.localFingerprint === args.remote.fingerprint + ) { + return { + action: 'up_to_date', + version, + fingerprint: args.remote.fingerprint, + }; + } + + return { + action: 'update', + version, + fingerprint: args.remote.fingerprint, + }; +} diff --git a/test/skills-registry-contract.test.js b/test/skills-registry-contract.test.js index abcc730..25df87b 100644 --- a/test/skills-registry-contract.test.js +++ b/test/skills-registry-contract.test.js @@ -300,12 +300,12 @@ test('getSkillDetail returns full skill object', async () => { assert.equal(data.skill.createdAt, new Date('2026-05-21T10:00:00Z').getTime()); assert.equal(data.skill.updatedAt, new Date('2026-05-21T10:00:00Z').getTime()); assert.equal(data.skill.fingerprint, null); + // fingerprint is only on skill (single-slot current content) assert.deepEqual(data.latestVersion, { version: '1.2.3', createdAt: new Date('2026-05-21T10:00:00Z').getTime(), changelog: '', license: null, - fingerprint: null, }); assert.equal(data.owner, null); assert.equal(data.moderation, null); From c4af0e5db5f73cc3d5396b9b32e464dae435a06c Mon Sep 17 00:00:00 2001 From: huaiju Date: Tue, 28 Jul 2026 11:16:49 +0800 Subject: [PATCH 04/29] test(skills): mock SkillsFile.update for publishSkill create path --- test/skills-registry-contract.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/skills-registry-contract.test.js b/test/skills-registry-contract.test.js index 25df87b..88df447 100644 --- a/test/skills-registry-contract.test.js +++ b/test/skills-registry-contract.test.js @@ -612,6 +612,7 @@ test('publishSkill returns ok: true and string skillId and versionId', async () }, SkillsFile: { create: async () => ({}), + update: async () => ({}), }, }); service.ctx = createMockCtx(); @@ -666,6 +667,7 @@ test('publishSkill accepts missing version (defaults to 0.0.0)', async () => { }, SkillsFile: { create: async () => ({}), + update: async () => ({}), }, }); service.ctx = createMockCtx(); From bf31f956cd5e7e49bb23eb8bcb8ac49fcf412875 Mon Sep 17 00:00:00 2001 From: huaiju Date: Tue, 28 Jul 2026 11:18:31 +0800 Subject: [PATCH 05/29] refactor(skills): use shared slug helpers in skills.ts Drop the duplicated normalizeSkillSlugOrFail copy left after C4 split. --- dt-skill/src/cli/commands/skills.ts | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/dt-skill/src/cli/commands/skills.ts b/dt-skill/src/cli/commands/skills.ts index 62bd6b3..75739ea 100644 --- a/dt-skill/src/cli/commands/skills.ts +++ b/dt-skill/src/cli/commands/skills.ts @@ -59,20 +59,7 @@ import { selectInstallMethod, selectScope, } from '../ui.js'; - -function normalizeSkillSlugOrFail(raw: string) { - const slug = raw.trim(); - if (!slug) fail('Slug required'); - // Safety: never allow path traversal or nested paths to become filesystem operations. - if (slug.includes('/') || slug.includes('\\') || slug.includes('..')) { - fail(`Invalid slug: ${slug}`); - } - return slug; -} - -function isSafeSkillSlug(slug: string) { - return Boolean(slug) && !slug.includes('/') && !slug.includes('\\') && !slug.includes('..'); -} +import { normalizeSkillSlugOrFail } from './skillHelpers.js'; const SUSPICIOUS_WARNING = '\n⚠️ Warning: "{slug}" is flagged for ClawHub security review.\n' + From d34ce3d24fc341de6f93d0eaff3ff825bec61f44 Mon Sep 17 00:00:00 2001 From: huaiju Date: Tue, 28 Jul 2026 14:21:49 +0800 Subject: [PATCH 06/29] style: format skills hash changes with Prettier Align the seven files that failed CI Prettier --check so PR #87 can pass the pipeline. --- app/service/skillsRegistry.js | 10 ++-------- dt-skill/src/cli.ts | 5 ++++- dt-skill/src/cli/commands/publish.ts | 12 +++++++----- dt-skill/src/cli/commands/skills.test.ts | 4 +--- dt-skill/src/cli/commands/update.ts | 15 ++------------- dt-skill/src/lockfile.ts | 4 +--- test/skills-registry-contract.test.js | 7 +++---- 7 files changed, 20 insertions(+), 37 deletions(-) diff --git a/app/service/skillsRegistry.js b/app/service/skillsRegistry.js index dc26a6f..aeaae2b 100644 --- a/app/service/skillsRegistry.js +++ b/app/service/skillsRegistry.js @@ -389,10 +389,7 @@ class SkillsRegistryService extends Service { const raw = String(category || '').trim(); if (!raw) return null; if (!SKILL_CATEGORY_OPTIONS.includes(raw)) { - this.ctx.throw( - 400, - `category 无效，可选: ${SKILL_CATEGORY_OPTIONS.join(', ')}` - ); + this.ctx.throw(400, `category 无效，可选: ${SKILL_CATEGORY_OPTIONS.join(', ')}`); } return raw; } @@ -474,10 +471,7 @@ class SkillsRegistryService extends Service { async replaceSkillStoredFiles(skill, processedFiles, transaction) { const { SkillsFile } = this.app.model; - await SkillsFile.update( - { is_delete: 1 }, - { where: { skill_id: skill.id }, transaction } - ); + await SkillsFile.update({ is_delete: 1 }, { where: { skill_id: skill.id }, transaction }); for (const file of processedFiles) { await SkillsFile.create( { diff --git a/dt-skill/src/cli.ts b/dt-skill/src/cli.ts index f548ac8..2fa97c4 100644 --- a/dt-skill/src/cli.ts +++ b/dt-skill/src/cli.ts @@ -280,7 +280,10 @@ registerCommand(program, ['publish']) .option('--name ', 'Display name') .option('--owner ', 'Publish under an org/user publisher handle') .option('--migrate-owner', 'Move an existing skill to the selected owner when republishing') - .option('--version ', 'Optional semver (compatibility; default 0.0.0, hash detects changes)') + .option( + '--version ', + 'Optional semver (compatibility; default 0.0.0, hash detects changes)' + ) .option('--fork-of ', 'Mark as a fork of an existing skill') .option('--changelog ', 'Changelog text') .option('--clawscan-note ', CLAWSCAN_NOTE_HELP) diff --git a/dt-skill/src/cli/commands/publish.ts b/dt-skill/src/cli/commands/publish.ts index f56aacb..db45ca5 100644 --- a/dt-skill/src/cli/commands/publish.ts +++ b/dt-skill/src/cli/commands/publish.ts @@ -164,9 +164,7 @@ export async function cmdPublish( })) ).fingerprint; const contentChanged = - skillExists && - Boolean(existingFingerprint) && - localFingerprint !== existingFingerprint; + skillExists && Boolean(existingFingerprint) && localFingerprint !== existingFingerprint; if (contentChanged && isInteractive() && !options.yes) { spinner.stop(); const ok = await promptConfirm( @@ -216,11 +214,15 @@ export async function cmdPublish( if (result.unchanged) { spinner.succeed( - `OK. Already up to date ${slug}${result.fingerprint ? ` (${result.fingerprint.slice(0, 12)}…)` : ''}` + `OK. Already up to date ${slug}${ + result.fingerprint ? ` (${result.fingerprint.slice(0, 12)}…)` : '' + }` ); } else { spinner.succeed( - `OK. Published ${slug}${result.fingerprint ? ` hash=${result.fingerprint.slice(0, 12)}…` : ''} (${result.versionId})` + `OK. Published ${slug}${ + result.fingerprint ? ` hash=${result.fingerprint.slice(0, 12)}…` : '' + } (${result.versionId})` ); } } catch (error) { diff --git a/dt-skill/src/cli/commands/skills.test.ts b/dt-skill/src/cli/commands/skills.test.ts index 148fd2a..4f873dd 100644 --- a/dt-skill/src/cli/commands/skills.test.ts +++ b/dt-skill/src/cli/commands/skills.test.ts @@ -405,9 +405,7 @@ describe('cmdUpdate', () => { expect(mockApiRequest).toHaveBeenCalledTimes(1); const [, args] = mockApiRequest.mock.calls[0] ?? []; expect(args?.path).toBe(`${ApiRoutes.skills}/${encodeURIComponent('other')}`); - expect(mockLog).toHaveBeenCalledWith( - expect.stringMatching(/Update summary: 1 updated/) - ); + expect(mockLog).toHaveBeenCalledWith(expect.stringMatching(/Update summary: 1 updated/)); }); it('continues updating remaining skills when one fails', async () => { diff --git a/dt-skill/src/cli/commands/update.ts b/dt-skill/src/cli/commands/update.ts index 3523550..3ab7125 100644 --- a/dt-skill/src/cli/commands/update.ts +++ b/dt-skill/src/cli/commands/update.ts @@ -14,22 +14,11 @@ import { type ApiV1SkillResponse, ApiV1SkillResponseSchema, } from '../../schema/index.js'; -import { - hashSkillFiles, - listTextFiles, - readSkillOrigin, - writeSkillOrigin, -} from '../../skills.js'; +import { hashSkillFiles, listTextFiles, readSkillOrigin, writeSkillOrigin } from '../../skills.js'; import { getRegistry } from '../registry.js'; import { decideSkillSync, remoteCurrentFromDetail } from '../skillSync.js'; import type { GlobalOpts } from '../types.js'; -import { - createSpinner, - fail, - formatError, - isInteractive, - promptConfirm, -} from '../ui.js'; +import { createSpinner, fail, formatError, isInteractive, promptConfirm } from '../ui.js'; import { fileExists, isSafeSkillSlug, diff --git a/dt-skill/src/lockfile.ts b/dt-skill/src/lockfile.ts index b11ae01..9d121a9 100644 --- a/dt-skill/src/lockfile.ts +++ b/dt-skill/src/lockfile.ts @@ -39,9 +39,7 @@ export function withPinnedMetadata( fingerprint?: string | null ): LockfileEntry { const nextFingerprint = - fingerprint !== undefined && fingerprint !== null - ? fingerprint - : existing?.fingerprint; + fingerprint !== undefined && fingerprint !== null ? fingerprint : existing?.fingerprint; return { version, installedAt, diff --git a/test/skills-registry-contract.test.js b/test/skills-registry-contract.test.js index 88df447..ae35d0c 100644 --- a/test/skills-registry-contract.test.js +++ b/test/skills-registry-contract.test.js @@ -720,10 +720,9 @@ test('publishSkill same content is unchanged no-op', async () => { service.ctx = createMockCtx(); service.ctx.logger = { warn: () => {}, info: () => {}, error: () => {} }; - const result = await service.publishSkill( - { slug: 'same-skill', displayName: 'Same' }, - [{ filepath: 'SKILL.md', content: '# same\n' }] - ); + const result = await service.publishSkill({ slug: 'same-skill', displayName: 'Same' }, [ + { filepath: 'SKILL.md', content: '# same\n' }, + ]); assert.equal(result.ok, true); assert.equal(result.unchanged, true); From 485af120bf62bb879d37ee8397b64ed9260be196 Mon Sep 17 00:00:00 2001 From: huaiju Date: Tue, 28 Jul 2026 14:37:52 +0800 Subject: [PATCH 07/29] fix(skills): single lock write after update + fingerprint mock fidelity Write lockfile once when lockDirty after the update loop instead of mid-loop. Align update mocks with skill.fingerprint and cover hash match up_to_date. --- dt-skill/src/cli/commands/skills.test.ts | 52 ++++++++++++++++++++++-- dt-skill/src/cli/commands/update.ts | 4 +- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/dt-skill/src/cli/commands/skills.test.ts b/dt-skill/src/cli/commands/skills.test.ts index 4f873dd..6f86808 100644 --- a/dt-skill/src/cli/commands/skills.test.ts +++ b/dt-skill/src/cli/commands/skills.test.ts @@ -378,8 +378,10 @@ describe('cmdUpdate', () => { }); it('bare update equals --all (skips pinned, updates others)', async () => { + // Canonical remote id is skill.fingerprint (not latestVersion.fingerprint). mockApiRequest.mockResolvedValue({ - latestVersion: { version: '2.0.0', fingerprint: 'remote-new' }, + skill: { fingerprint: 'remote-new', version: '0.0.0' }, + latestVersion: { version: '0.0.0' }, moderation: null, }); mockDownloadZip.mockResolvedValue(new Uint8Array([1, 2, 3])); @@ -406,16 +408,58 @@ describe('cmdUpdate', () => { const [, args] = mockApiRequest.mock.calls[0] ?? []; expect(args?.path).toBe(`${ApiRoutes.skills}/${encodeURIComponent('other')}`); expect(mockLog).toHaveBeenCalledWith(expect.stringMatching(/Update summary: 1 updated/)); + // One write after the loop when lockDirty. + expect(writeLockfile).toHaveBeenCalledTimes(1); + }); + + it('reports up to date when local fingerprint matches skill.fingerprint', async () => { + const sharedFp = 'same-content-fp-abc'; + mockApiRequest.mockResolvedValue({ + skill: { fingerprint: sharedFp, version: '0.0.0' }, + latestVersion: { version: '0.0.0' }, + moderation: null, + }); + vi.mocked(readLockfile).mockResolvedValue({ + version: 1, + skills: { + demo: { version: '0.0.0', installedAt: 123, fingerprint: sharedFp }, + }, + }); + vi.mocked(writeLockfile).mockResolvedValue(); + vi.mocked(readSkillOrigin).mockResolvedValue({ + version: 1, + registry: 'https://example.com', + slug: 'demo', + installedVersion: '0.0.0', + installedAt: 123, + fingerprint: sharedFp, + }); + vi.mocked(listTextFiles).mockResolvedValue([ + { relPath: 'SKILL.md', bytes: new Uint8Array([1]) }, + ]); + vi.mocked(hashSkillFiles).mockReturnValue({ fingerprint: sharedFp, files: [] }); + vi.mocked(stat).mockResolvedValue({} as unknown as Awaited>); + + await cmdUpdate(makeOpts(), 'demo', {}, false); + + expect(mockDownloadZip).not.toHaveBeenCalled(); + expect(writeLockfile).not.toHaveBeenCalled(); + expect(mockLog).toHaveBeenCalledWith( + expect.stringMatching(/Update summary: 0 updated, 1 up to date/) + ); + expect(mockLog).toHaveBeenCalledWith(expect.stringMatching(/up to date: demo/)); }); it('continues updating remaining skills when one fails', async () => { mockApiRequest .mockResolvedValueOnce({ - latestVersion: { version: '2.0.0', fingerprint: 'fp-a' }, + skill: { fingerprint: 'fp-a', version: '0.0.0' }, + latestVersion: { version: '0.0.0' }, moderation: null, }) .mockResolvedValueOnce({ - latestVersion: { version: '2.0.0', fingerprint: 'fp-b' }, + skill: { fingerprint: 'fp-b', version: '0.0.0' }, + latestVersion: { version: '0.0.0' }, moderation: null, }); mockDownloadZip @@ -445,6 +489,8 @@ describe('cmdUpdate', () => { expect(mockLog).toHaveBeenCalledWith( expect.stringMatching(/Update summary: 1 updated.*1 failed/) ); + // Successful skill still dirties lock; one write after the loop. + expect(writeLockfile).toHaveBeenCalledTimes(1); }); it('uses path-based skill lookup when no local fingerprint is available', async () => { diff --git a/dt-skill/src/cli/commands/update.ts b/dt-skill/src/cli/commands/update.ts index 3ab7125..9d8b115 100644 --- a/dt-skill/src/cli/commands/update.ts +++ b/dt-skill/src/cli/commands/update.ts @@ -152,8 +152,8 @@ export async function cmdUpdate( prev, decision.fingerprint ); + // Persist once after the loop (lockDirty). lockDirty = true; - await writeLockfile(installWorkdir, lock); } spinner.succeed( `${entry}: up to date${ @@ -206,8 +206,8 @@ export async function cmdUpdate( throw error; } + // Persist once after the loop (lockDirty). lockDirty = true; - await writeLockfile(installWorkdir, lock); spinner.succeed( `${entry}: updated${ lock.skills[entry]?.fingerprint From 1ad3dc74f94a0c5a901df92681a1bd69bb0720d0 Mon Sep 17 00:00:00 2001 From: huaiju Date: Tue, 28 Jul 2026 14:49:22 +0800 Subject: [PATCH 08/29] fix(skills): clear ESLint errors after update command split Remove unused imports from skills.ts, use const for publish version, and fix update.ts import order so CI ESLint passes. --- dt-skill/src/cli/commands/publish.ts | 2 +- dt-skill/src/cli/commands/skills.ts | 13 ++----------- dt-skill/src/cli/commands/update.ts | 2 +- 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/dt-skill/src/cli/commands/publish.ts b/dt-skill/src/cli/commands/publish.ts index db45ca5..3fdf54b 100644 --- a/dt-skill/src/cli/commands/publish.ts +++ b/dt-skill/src/cli/commands/publish.ts @@ -87,7 +87,7 @@ export async function cmdPublish( const displayName = options.name ?? titleCase(basename(folder)); const ownerHandle = options.owner?.trim().replace(/^@+/, ''); // Version is optional for authors; default is a compatibility placeholder. Change detection uses content hash. - let version = options.version?.trim() || DEFAULT_PUBLISH_VERSION; + const version = options.version?.trim() || DEFAULT_PUBLISH_VERSION; if (!semver.valid(version)) fail('--version must be valid semver when provided'); const changelog = options.changelog ?? ''; let clawScanNote: string | undefined; diff --git a/dt-skill/src/cli/commands/skills.ts b/dt-skill/src/cli/commands/skills.ts index 75739ea..eb3b6fb 100644 --- a/dt-skill/src/cli/commands/skills.ts +++ b/dt-skill/src/cli/commands/skills.ts @@ -1,14 +1,12 @@ import { lstat, mkdir, rm, stat } from 'node:fs/promises'; import { homedir } from 'node:os'; -import { basename, dirname, join } from 'node:path'; -import semver from 'semver'; +import { dirname, join } from 'node:path'; import { apiRequest, downloadZip, registryUrl } from '../../http.js'; import { formatPinnedDetails, isPinned as isPinnedSkillEntry, readLockfile, - withPinnedMetadata, writeLockfile, } from '../../lockfile.js'; import { @@ -21,14 +19,7 @@ import { ApiV1SkillResponseSchema, ApiV1SkillVersionResponseSchema, } from '../../schema/index.js'; -import { - extractZipToDir, - hashSkillFiles, - listManualSkills, - listTextFiles, - readSkillOrigin, - writeSkillOrigin, -} from '../../skills.js'; +import { listManualSkills } from '../../skills.js'; import { AGENT_DEFINITIONS, type AgentType, diff --git a/dt-skill/src/cli/commands/update.ts b/dt-skill/src/cli/commands/update.ts index 9d8b115..2f9ea1c 100644 --- a/dt-skill/src/cli/commands/update.ts +++ b/dt-skill/src/cli/commands/update.ts @@ -1,5 +1,5 @@ -import { join } from 'node:path'; import { rm } from 'node:fs/promises'; +import { join } from 'node:path'; import semver from 'semver'; import { apiRequest, downloadZip } from '../../http.js'; From 0b14d4730684e7677ab33b1bf576709af7a5d363 Mon Sep 17 00:00:00 2001 From: huaiju Date: Tue, 28 Jul 2026 14:56:47 +0800 Subject: [PATCH 09/29] refactor(dt-skill): drop unused resolve CLI types Remove ResolveVersionRef/ResolveResult left from the pre-hash update path (ticket 01, PR #87 I1). --- dt-skill/src/cli/types.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/dt-skill/src/cli/types.ts b/dt-skill/src/cli/types.ts index aa05ec4..a1a711a 100644 --- a/dt-skill/src/cli/types.ts +++ b/dt-skill/src/cli/types.ts @@ -15,13 +15,3 @@ export type GlobalOpts = { /** Skip interactive prompts. */ yes?: boolean; }; - -export type ResolveVersionRef = { - version: string; - fingerprint?: string | null; -}; - -export type ResolveResult = { - match: ResolveVersionRef | null; - latestVersion: ResolveVersionRef | null; -}; From b52a53ca0ed3ce82eaa2fa6758ed76810ac705b8 Mon Sep 17 00:00:00 2001 From: huaiju Date: Tue, 28 Jul 2026 14:57:02 +0800 Subject: [PATCH 10/29] test(dt-skill): align update mock with single detail + disk fingerprint Remove unused resolve mock and rename case (ticket 03, PR #87 I3). --- dt-skill/src/cli/commands/skills.test.ts | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/dt-skill/src/cli/commands/skills.test.ts b/dt-skill/src/cli/commands/skills.test.ts index 6f86808..2bc630f 100644 --- a/dt-skill/src/cli/commands/skills.test.ts +++ b/dt-skill/src/cli/commands/skills.test.ts @@ -516,16 +516,13 @@ describe('cmdUpdate', () => { expect(args?.url).toBeUndefined(); }); - it('trusts the stored install fingerprint when the resolve endpoint cannot match', async () => { - mockApiRequest - .mockResolvedValueOnce({ - latestVersion: { version: '2.0.0' }, - moderation: null, - }) - .mockResolvedValueOnce({ - match: null, - latestVersion: { version: '2.0.0' }, - }); + it('updates when remote has no skill.fingerprint (uses version download token)', async () => { + // cmdUpdate calls detail once per skill; local disk hash is compared to skill.fingerprint only. + mockApiRequest.mockResolvedValueOnce({ + skill: { version: '2.0.0' }, + latestVersion: { version: '2.0.0' }, + moderation: null, + }); mockDownloadZip.mockResolvedValue(new Uint8Array([1, 2, 3])); vi.mocked(readLockfile).mockResolvedValue({ version: 1, @@ -545,15 +542,14 @@ describe('cmdUpdate', () => { vi.mocked(listTextFiles).mockResolvedValue([ { relPath: 'SKILL.md', bytes: new Uint8Array([1]) }, ]); + // Disk-computed local fingerprint (not origin alone) drives decideSkillSync. vi.mocked(hashSkillFiles).mockReturnValue({ fingerprint: 'hash', files: [] }); vi.mocked(stat).mockResolvedValue({} as unknown as Awaited>); vi.mocked(rm).mockResolvedValue(); await cmdUpdate(makeOpts(), 'demo', {}, false); - expect(mockLog).not.toHaveBeenCalledWith( - 'demo: local changes (no match). Use --force to overwrite.' - ); + expect(mockApiRequest).toHaveBeenCalledTimes(1); expect(mockDownloadZip).toHaveBeenCalledWith( 'https://example.com', expect.objectContaining({ slug: 'demo', version: '2.0.0' }) From 5988053c0f8d4a83728557ba1fc58190eb421028 Mon Sep 17 00:00:00 2001 From: huaiju Date: Tue, 28 Jul 2026 14:57:43 +0800 Subject: [PATCH 11/29] fix(skills): explicitly preserve category on re-publish When payload omits category, write the existing skill.category into the update payload so retention is a deliberate contract (ticket 02, I2). --- app/service/skillsRegistry.js | 7 +++- test/skills-registry-contract.test.js | 49 +++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/app/service/skillsRegistry.js b/app/service/skillsRegistry.js index aeaae2b..58cf87e 100644 --- a/app/service/skillsRegistry.js +++ b/app/service/skillsRegistry.js @@ -530,7 +530,12 @@ class SkillsRegistryService extends Service { is_delete: 0, source_id: source.id, }; - if (category) updatePayload.category = category; + // Explicit preserve: do not rely on partial-update omitting the field. + if (category) { + updatePayload.category = category; + } else if (skill.category) { + updatePayload.category = skill.category; + } await skill.update(updatePayload, { transaction: t }); } else { skill = await SkillsItem.create( diff --git a/test/skills-registry-contract.test.js b/test/skills-registry-contract.test.js index ae35d0c..1d72418 100644 --- a/test/skills-registry-contract.test.js +++ b/test/skills-registry-contract.test.js @@ -683,6 +683,55 @@ test('publishSkill accepts missing version (defaults to 0.0.0)', async () => { assert.equal(typeof result.fingerprint, 'string'); }); +test('publishSkill re-publish without category keeps existing category', async () => { + const service = Object.create(SkillsRegistryService.prototype); + const updatePayloads = []; + const skillRow = { + id: 51, + slug: 'keep-cat', + name: 'Keep Cat', + version: '0.0.0', + category: '安全', + is_delete: 0, + update: async (data) => { + updatePayloads.push(data); + Object.assign(skillRow, data); + }, + }; + service.app = createMockApp({ + SkillsItem: { + findOne: async () => skillRow, + }, + SkillsSource: { + findOrCreate: async () => [{ id: 1 }], + }, + SkillsFile: { + findAll: async () => [ + { + file_path: 'SKILL.md', + content: '# old\n', + is_binary: 0, + }, + ], + create: async () => ({}), + update: async () => ({}), + }, + }); + service.ctx = createMockCtx(); + service.ctx.logger = { warn: () => {}, info: () => {}, error: () => {} }; + + const result = await service.publishSkill( + { slug: 'keep-cat', displayName: 'Keep Cat' }, + [{ filepath: 'SKILL.md', content: '# new content\n' }] + ); + + assert.equal(result.ok, true); + assert.equal(result.unchanged, false); + const mainUpdate = updatePayloads.find((p) => Object.prototype.hasOwnProperty.call(p, 'name')); + assert.ok(mainUpdate, 'skill.update should run the main re-publish payload'); + assert.equal(mainUpdate.category, '安全'); +}); + test('publishSkill same content is unchanged no-op', async () => { const service = Object.create(SkillsRegistryService.prototype); const skillRow = { From e1e9d5aab65898107db6d7761350d32b1ad8cae6 Mon Sep 17 00:00:00 2001 From: huaiju Date: Tue, 28 Jul 2026 14:58:44 +0800 Subject: [PATCH 12/29] refactor(skills): share skill category enum via contracts Add contracts/skill-categories and wire registry, skills service, and CLI publish to one authoritative list (ticket 04, PR #87 I4). --- app/service/skills.js | 11 +------ app/service/skillsRegistry.js | 17 +++------- contracts/skill-categories/categories.json | 1 + contracts/skill-categories/index.d.ts | 3 ++ contracts/skill-categories/index.js | 20 ++++++++++++ dt-skill/scripts/build.mjs | 37 ++++++++++++---------- dt-skill/src/cli/commands/publish.ts | 16 ++-------- dt-skill/src/cli/skillCategories.ts | 16 ++++++++++ test/skill-categories-contract.test.js | 18 +++++++++++ 9 files changed, 88 insertions(+), 51 deletions(-) create mode 100644 contracts/skill-categories/categories.json create mode 100644 contracts/skill-categories/index.d.ts create mode 100644 contracts/skill-categories/index.js create mode 100644 dt-skill/src/cli/skillCategories.ts create mode 100644 test/skill-categories-contract.test.js diff --git a/app/service/skills.js b/app/service/skills.js index 814bad6..9cfe16b 100644 --- a/app/service/skills.js +++ b/app/service/skills.js @@ -27,16 +27,7 @@ const SKILLS_ROOT_DISCOVER_DEPTH_LIMIT = 8; const DISCOVER_MAX_DIR_COUNT = 3000; const SKILL_SLUG_PATTERN = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; -const SKILL_CATEGORY_OPTIONS = [ - '通用', - '前端', - '后端', - '数据与AI', - '运维与系统', - '工程效率', - '安全', - '其他', -]; +const { SKILL_CATEGORY_OPTIONS } = require('../../contracts/skill-categories'); const EXTENSION_LANGUAGE_MAP = { '.md': 'markdown', diff --git a/app/service/skillsRegistry.js b/app/service/skillsRegistry.js index 58cf87e..370ce54 100644 --- a/app/service/skillsRegistry.js +++ b/app/service/skillsRegistry.js @@ -5,20 +5,13 @@ const ignore = require('ignore'); const path = require('path'); const skillUtils = require('../utils/skill-utils'); const skillFingerprint = require('../../contracts/skill-fingerprint'); +const { + SKILL_CATEGORY_OPTIONS, + isValidSkillCategory, +} = require('../../contracts/skill-categories'); const SEMVER_PATTERN = /^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-[\w.-]+)?(?:\+[\w.-]+)?$/; const SKILL_SLUG_PATTERN = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; -/** Closed category enum (aligned with skills market / CLI). */ -const SKILL_CATEGORY_OPTIONS = [ - '通用', - '前端', - '后端', - '数据与AI', - '运维与系统', - '工程效率', - '安全', - '其他', -]; /** Compatibility placeholder when client omits version; content hash is the change signal. */ const DEFAULT_PUBLISH_VERSION = '0.0.0'; @@ -388,7 +381,7 @@ class SkillsRegistryService extends Service { resolvePublishCategory(category) { const raw = String(category || '').trim(); if (!raw) return null; - if (!SKILL_CATEGORY_OPTIONS.includes(raw)) { + if (!isValidSkillCategory(raw)) { this.ctx.throw(400, `category 无效，可选: ${SKILL_CATEGORY_OPTIONS.join(', ')}`); } return raw; diff --git a/contracts/skill-categories/categories.json b/contracts/skill-categories/categories.json new file mode 100644 index 0000000..aaa0a7e --- /dev/null +++ b/contracts/skill-categories/categories.json @@ -0,0 +1 @@ +["通用", "前端", "后端", "数据与AI", "运维与系统", "工程效率", "安全", "其他"] diff --git a/contracts/skill-categories/index.d.ts b/contracts/skill-categories/index.d.ts new file mode 100644 index 0000000..a481875 --- /dev/null +++ b/contracts/skill-categories/index.d.ts @@ -0,0 +1,3 @@ +export const SKILL_CATEGORY_OPTIONS: readonly string[]; +export const SKILL_CATEGORY_SET: ReadonlySet; +export function isValidSkillCategory(value: unknown): boolean; diff --git a/contracts/skill-categories/index.js b/contracts/skill-categories/index.js new file mode 100644 index 0000000..1b72910 --- /dev/null +++ b/contracts/skill-categories/index.js @@ -0,0 +1,20 @@ +'use strict'; + +const categories = require('./categories.json'); + +const SKILL_CATEGORY_OPTIONS = Object.freeze([...categories]); +const SKILL_CATEGORY_SET = new Set(SKILL_CATEGORY_OPTIONS); + +/** + * @param {unknown} value + * @returns {boolean} + */ +function isValidSkillCategory(value) { + return typeof value === 'string' && SKILL_CATEGORY_SET.has(value); +} + +module.exports = { + SKILL_CATEGORY_OPTIONS, + SKILL_CATEGORY_SET, + isValidSkillCategory, +}; diff --git a/dt-skill/scripts/build.mjs b/dt-skill/scripts/build.mjs index 75753de..403ac22 100644 --- a/dt-skill/scripts/build.mjs +++ b/dt-skill/scripts/build.mjs @@ -1,26 +1,31 @@ -import { spawnSync } from "node:child_process"; -import { cp, rename, rm } from "node:fs/promises"; -import { createRequire } from "node:module"; -import { dirname, resolve } from "node:path"; -import { fileURLToPath } from "node:url"; +import { spawnSync } from 'node:child_process'; +import { cp, rename, rm } from 'node:fs/promises'; +import { createRequire } from 'node:module'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; const require = createRequire(import.meta.url); -const packageRoot = resolve(dirname(fileURLToPath(import.meta.url)), ".."); -const distDir = resolve(packageRoot, "dist"); -const contractSourceDir = resolve(packageRoot, "..", "contracts", "skill-fingerprint"); -const contractDistDir = resolve(distDir, "contracts", "skill-fingerprint"); +const packageRoot = resolve(dirname(fileURLToPath(import.meta.url)), '..'); +const distDir = resolve(packageRoot, 'dist'); +const contractsRoot = resolve(packageRoot, '..', 'contracts'); + +const contractPackages = ['skill-fingerprint', 'skill-categories']; await rm(distDir, { recursive: true, force: true }); -const tscBin = require.resolve("typescript/bin/tsc"); -const result = spawnSync(process.execPath, [tscBin, "-p", "tsconfig.json"], { - cwd: packageRoot, - stdio: "inherit", +const tscBin = require.resolve('typescript/bin/tsc'); +const result = spawnSync(process.execPath, [tscBin, '-p', 'tsconfig.json'], { + cwd: packageRoot, + stdio: 'inherit', }); if (result.status !== 0) { - process.exit(result.status ?? 1); + process.exit(result.status ?? 1); } -await cp(contractSourceDir, contractDistDir, { recursive: true }); -await rename(resolve(contractDistDir, "index.js"), resolve(contractDistDir, "index.cjs")); +for (const name of contractPackages) { + const sourceDir = resolve(contractsRoot, name); + const destDir = resolve(distDir, 'contracts', name); + await cp(sourceDir, destDir, { recursive: true }); + await rename(resolve(destDir, 'index.js'), resolve(destDir, 'index.cjs')); +} diff --git a/dt-skill/src/cli/commands/publish.ts b/dt-skill/src/cli/commands/publish.ts index 3fdf54b..fe42e5e 100644 --- a/dt-skill/src/cli/commands/publish.ts +++ b/dt-skill/src/cli/commands/publish.ts @@ -14,6 +14,7 @@ import { hashSkillFiles, listPublishFiles } from '../../skills.js'; import { searchMultiselect } from '../prompts/search-multiselect.js'; import { getRegistry } from '../registry.js'; import { findSkillFolders } from '../scanSkills.js'; +import { SKILL_CATEGORY_OPTIONS, SKILL_CATEGORY_SET } from '../skillCategories.js'; import { sanitizeSlug, titleCase } from '../slug.js'; import type { GlobalOpts } from '../types.js'; import { @@ -25,19 +26,8 @@ import { selectCategory, } from '../ui.js'; -/** Closed category enum aligned with Doraemon skills market. */ -export const SKILL_CATEGORY_OPTIONS = [ - '通用', - '前端', - '后端', - '数据与AI', - '运维与系统', - '工程效率', - '安全', - '其他', -] as const; - -const SKILL_CATEGORY_SET = new Set(SKILL_CATEGORY_OPTIONS); +/** Re-export shared market category list (contracts/skill-categories). */ +export { SKILL_CATEGORY_OPTIONS }; /** Internal compatibility version when author omits --version (hash is the change signal). */ const DEFAULT_PUBLISH_VERSION = '0.0.0'; diff --git a/dt-skill/src/cli/skillCategories.ts b/dt-skill/src/cli/skillCategories.ts new file mode 100644 index 0000000..15cdfa2 --- /dev/null +++ b/dt-skill/src/cli/skillCategories.ts @@ -0,0 +1,16 @@ +import { createRequire } from 'node:module'; +import { dirname, resolve, sep } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const require = createRequire(import.meta.url); +const currentDir = dirname(fileURLToPath(import.meta.url)); +// Source: ../../dist when running tests from src; dist layout after build. +const contractPath = currentDir.endsWith(`${sep}src${sep}cli`) + ? resolve(currentDir, '../../dist/contracts/skill-categories/index.cjs') + : resolve(currentDir, '../contracts/skill-categories/index.cjs'); + +const contract = require(contractPath); + +export const SKILL_CATEGORY_OPTIONS = contract.SKILL_CATEGORY_OPTIONS as readonly string[]; +export const SKILL_CATEGORY_SET = contract.SKILL_CATEGORY_SET as ReadonlySet; +export const isValidSkillCategory = contract.isValidSkillCategory as (value: unknown) => boolean; diff --git a/test/skill-categories-contract.test.js b/test/skill-categories-contract.test.js new file mode 100644 index 0000000..1f0896d --- /dev/null +++ b/test/skill-categories-contract.test.js @@ -0,0 +1,18 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const { SKILL_CATEGORY_OPTIONS, isValidSkillCategory } = require('../contracts/skill-categories'); + +test('skill-categories contract lists the market enum', () => { + assert.ok(Array.isArray(SKILL_CATEGORY_OPTIONS)); + assert.ok(SKILL_CATEGORY_OPTIONS.includes('通用')); + assert.ok(SKILL_CATEGORY_OPTIONS.includes('工程效率')); + assert.equal(SKILL_CATEGORY_OPTIONS.length, 8); +}); + +test('isValidSkillCategory rejects unknown values', () => { + assert.equal(isValidSkillCategory('前端'), true); + assert.equal(isValidSkillCategory('not-a-real-category'), false); + assert.equal(isValidSkillCategory(''), false); + assert.equal(isValidSkillCategory(null), false); +}); From 122ffb36281169b332a8a1eead5916d164043a72 Mon Sep 17 00:00:00 2001 From: huaiju Date: Tue, 28 Jul 2026 14:58:54 +0800 Subject: [PATCH 13/29] style: prettier format re-publish category contract test --- test/skills-registry-contract.test.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/skills-registry-contract.test.js b/test/skills-registry-contract.test.js index 1d72418..337f252 100644 --- a/test/skills-registry-contract.test.js +++ b/test/skills-registry-contract.test.js @@ -720,10 +720,9 @@ test('publishSkill re-publish without category keeps existing category', async ( service.ctx = createMockCtx(); service.ctx.logger = { warn: () => {}, info: () => {}, error: () => {} }; - const result = await service.publishSkill( - { slug: 'keep-cat', displayName: 'Keep Cat' }, - [{ filepath: 'SKILL.md', content: '# new content\n' }] - ); + const result = await service.publishSkill({ slug: 'keep-cat', displayName: 'Keep Cat' }, [ + { filepath: 'SKILL.md', content: '# new content\n' }, + ]); assert.equal(result.ok, true); assert.equal(result.unchanged, false); From 5ee63cd5c56aa52e8b7e61790c08da5d32ed77f3 Mon Sep 17 00:00:00 2001 From: huaiju Date: Tue, 28 Jul 2026 18:06:16 +0800 Subject: [PATCH 14/29] feat(dt-skill): default registry to intranet deploy URL Use http://172.16.100.225:7001 when no flag, env, cache, or site discovery is set so CLI works out of the box (ticket 01). --- dt-skill/src/cli/registry.test.ts | 41 ++++++++++++++++++++----------- dt-skill/src/cli/registry.ts | 24 ++++++++++++++---- 2 files changed, 46 insertions(+), 19 deletions(-) diff --git a/dt-skill/src/cli/registry.test.ts b/dt-skill/src/cli/registry.test.ts index 84a4fa8..86b2ee5 100644 --- a/dt-skill/src/cli/registry.test.ts +++ b/dt-skill/src/cli/registry.test.ts @@ -17,7 +17,8 @@ vi.mock('../discovery.js', () => ({ discoverRegistryFromSite: (...args: unknown[]) => discoverRegistryFromSite(...args), })); -const { DEFAULT_REGISTRY, DEFAULT_SITE, getRegistry, resolveRegistry } = await import('./registry'); +const { DEFAULT_REGISTRY, DEFAULT_SITE, getRegistry, normalizeRegistryBase, resolveRegistry } = + await import('./registry'); function makeOpts(overrides: Partial = {}): GlobalOpts { return { @@ -37,9 +38,31 @@ beforeEach(() => { }); describe('registry resolution', () => { - it('has no static site or registry fallback', () => { + it('ships a non-empty built-in deploy registry and empty default site', () => { expect(DEFAULT_SITE).toBe(''); - expect(DEFAULT_REGISTRY).toBe(''); + expect(DEFAULT_REGISTRY).toBe('http://172.16.100.225:7001'); + expect(normalizeRegistryBase(`${DEFAULT_REGISTRY}/`)).toBe(DEFAULT_REGISTRY); + }); + + it('uses built-in default when no explicit, cache, or site discovery', async () => { + readGlobalConfig.mockResolvedValue(null); + discoverRegistryFromSite.mockResolvedValue(null); + + const registry = await resolveRegistry(makeOpts()); + + expect(registry).toBe('http://172.16.100.225:7001'); + expect(discoverRegistryFromSite).not.toHaveBeenCalled(); + }); + + it('getRegistry caches the built-in default when cache is empty', async () => { + readGlobalConfig.mockResolvedValue(null); + + const registry = await getRegistry(makeOpts(), { cache: true }); + + expect(registry).toBe('http://172.16.100.225:7001'); + expect(writeGlobalConfig).toHaveBeenCalledWith({ + registry: 'http://172.16.100.225:7001', + }); }); it('prefers explicit registry over discovery/cache', async () => { @@ -54,7 +77,7 @@ describe('registry resolution', () => { expect(discoverRegistryFromSite).not.toHaveBeenCalled(); }); - it('uses cached registry before site discovery', async () => { + it('uses cached registry before site discovery and built-in default', async () => { readGlobalConfig.mockResolvedValue({ registry: 'http://10.0.0.7:7001' }); discoverRegistryFromSite.mockResolvedValue({ apiBase: 'http://10.0.0.8:7001' }); @@ -78,16 +101,6 @@ describe('registry resolution', () => { }); }); - it('fails clearly when no explicit, cached, or discoverable registry exists', async () => { - readGlobalConfig.mockResolvedValue(null); - discoverRegistryFromSite.mockResolvedValue(null); - - await expect(getRegistry(makeOpts(), { cache: true })).rejects.toThrow( - 'Registry is not configured' - ); - expect(writeGlobalConfig).not.toHaveBeenCalled(); - }); - it('caches an explicit runtime registry even when another custom registry was cached', async () => { readGlobalConfig.mockResolvedValue({ registry: 'http://10.0.0.7:7001' }); diff --git a/dt-skill/src/cli/registry.ts b/dt-skill/src/cli/registry.ts index 97681a5..7ab9b14 100644 --- a/dt-skill/src/cli/registry.ts +++ b/dt-skill/src/cli/registry.ts @@ -3,21 +3,33 @@ import { discoverRegistryFromSite } from '../discovery.js'; import type { GlobalOpts } from './types.js'; export const DEFAULT_SITE = ''; -export const DEFAULT_REGISTRY = ''; +/** Built-in intranet deploy registry (overridable via --registry / DT_SKILL_REGISTRY). */ +export const DEFAULT_REGISTRY = 'http://172.16.100.225:7001'; + +/** Strip trailing slashes so join paths do not become `//api/...`. */ +export function normalizeRegistryBase(url: string): string { + return String(url || '') + .trim() + .replace(/\/+$/, ''); +} export async function resolveRegistry(opts: GlobalOpts) { const explicit = opts.registrySource !== 'default' ? opts.registry.trim() : ''; - if (explicit) return explicit; + if (explicit) return normalizeRegistryBase(explicit); const cfg = await readGlobalConfig(); const cached = cfg?.registry?.trim(); - if (cached) return cached; + if (cached) return normalizeRegistryBase(cached); const site = opts.site.trim(); if (site) { const discovery = await discoverRegistryFromSite(site).catch(() => null); const discovered = discovery?.apiBase?.trim(); - if (discovered) return discovered; + if (discovered) return normalizeRegistryBase(discovered); + } + + if (DEFAULT_REGISTRY.trim()) { + return normalizeRegistryBase(DEFAULT_REGISTRY); } throw new Error( @@ -31,6 +43,8 @@ export async function getRegistry(opts: GlobalOpts, params?: { cache?: boolean } if (!cache) return registry; const cfg = await readGlobalConfig(); const cached = cfg?.registry?.trim(); - if (!cached || cached !== registry) await writeGlobalConfig({ registry }); + if (!cached || normalizeRegistryBase(cached) !== registry) { + await writeGlobalConfig({ registry }); + } return registry; } From 8ab042de9e4495c8aa4e6402469173929ece3c40 Mon Sep 17 00:00:00 2001 From: huaiju Date: Tue, 28 Jul 2026 18:08:12 +0800 Subject: [PATCH 15/29] fix(dt-skill): simplify default registry fallback after review Always normalize and return DEFAULT_REGISTRY; throw only if empty. --- dt-skill/src/cli/registry.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/dt-skill/src/cli/registry.ts b/dt-skill/src/cli/registry.ts index 7ab9b14..38c11b4 100644 --- a/dt-skill/src/cli/registry.ts +++ b/dt-skill/src/cli/registry.ts @@ -28,13 +28,14 @@ export async function resolveRegistry(opts: GlobalOpts) { if (discovered) return normalizeRegistryBase(discovered); } - if (DEFAULT_REGISTRY.trim()) { - return normalizeRegistryBase(DEFAULT_REGISTRY); + // Built-in deploy default (ticket 01). Empty only if constant is cleared. + const fallback = normalizeRegistryBase(DEFAULT_REGISTRY); + if (!fallback) { + throw new Error( + 'Registry is not configured. Copy a command from the Doraemon Skills page or pass --registry .' + ); } - - throw new Error( - 'Registry is not configured. Copy a command from the Doraemon Skills page or pass --registry .' - ); + return fallback; } export async function getRegistry(opts: GlobalOpts, params?: { cache?: boolean }) { From cb76008b609af1b196290462334654dfc3fedb90 Mon Sep 17 00:00:00 2001 From: huaiju Date: Tue, 28 Jul 2026 18:08:30 +0800 Subject: [PATCH 16/29] test(dt-skill): cover registry overrides for local dev Assert --registry and DT_SKILL_REGISTRY beat default and cache (ticket 02). --- dt-skill/src/cli/registry.test.ts | 43 +++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/dt-skill/src/cli/registry.test.ts b/dt-skill/src/cli/registry.test.ts index 86b2ee5..7e12798 100644 --- a/dt-skill/src/cli/registry.test.ts +++ b/dt-skill/src/cli/registry.test.ts @@ -114,4 +114,47 @@ describe('registry resolution', () => { registry: 'http://10.0.0.8:7001', }); }); + + it('--registry (cli) overrides built-in default and cache', async () => { + readGlobalConfig.mockResolvedValue({ registry: DEFAULT_REGISTRY }); + + const registry = await resolveRegistry( + makeOpts({ + registry: 'http://127.0.0.1:7001/', + registrySource: 'cli', + }) + ); + + expect(registry).toBe('http://127.0.0.1:7001'); + }); + + it('DT_SKILL_REGISTRY (env) overrides built-in default', async () => { + readGlobalConfig.mockResolvedValue(null); + + const registry = await resolveRegistry( + makeOpts({ + registry: 'http://127.0.0.1:7001', + registrySource: 'env', + }) + ); + + expect(registry).toBe('http://127.0.0.1:7001'); + expect(registry).not.toBe(DEFAULT_REGISTRY); + }); + + it('explicit overrides still beat site discovery', async () => { + readGlobalConfig.mockResolvedValue(null); + discoverRegistryFromSite.mockResolvedValue({ apiBase: 'http://discovered:7001' }); + + const registry = await resolveRegistry( + makeOpts({ + registry: 'http://127.0.0.1:7001', + registrySource: 'env', + site: 'http://site.example', + }) + ); + + expect(registry).toBe('http://127.0.0.1:7001'); + expect(discoverRegistryFromSite).not.toHaveBeenCalled(); + }); }); From f3d7fb34e992df9fa9229587ab001a83fd4c0653 Mon Sep 17 00:00:00 2001 From: huaiju Date: Tue, 28 Jul 2026 18:10:13 +0800 Subject: [PATCH 17/29] fix(dt-skill): test real cli/env registry selection path Extract pickRegistryFromCliAndEnv used by cli.ts so override priority is unit-tested on the shipped selection helper (ticket 02 review). --- dt-skill/src/cli.ts | 8 ++-- dt-skill/src/cli/registry.test.ts | 68 +++++++++++++++++++------------ dt-skill/src/cli/registry.ts | 19 +++++++++ 3 files changed, 65 insertions(+), 30 deletions(-) diff --git a/dt-skill/src/cli.ts b/dt-skill/src/cli.ts index 2fa97c4..7eb1f5d 100644 --- a/dt-skill/src/cli.ts +++ b/dt-skill/src/cli.ts @@ -27,7 +27,7 @@ import { import { cmdStarSkill } from './cli/commands/star.js'; import { cmdUnstarSkill } from './cli/commands/unstar.js'; import { configureCommanderHelp, styleEnvBlock, styleTitle } from './cli/helpStyle.js'; -import { DEFAULT_REGISTRY, DEFAULT_SITE } from './cli/registry.js'; +import { DEFAULT_SITE, pickRegistryFromCliAndEnv } from './cli/registry.js'; import type { GlobalOpts } from './cli/types.js'; import { fail } from './cli/ui.js'; @@ -111,8 +111,10 @@ async function resolveGlobalOpts(): Promise { const dir = join(workdir, 'skills'); const site = raw.site ?? process.env.DT_SKILL_SITE ?? DEFAULT_SITE; - const registrySource = raw.registry ? 'cli' : process.env.DT_SKILL_REGISTRY ? 'env' : 'default'; - const registry = raw.registry ?? process.env.DT_SKILL_REGISTRY ?? DEFAULT_REGISTRY; + const { registry, registrySource } = pickRegistryFromCliAndEnv({ + cliRegistry: raw.registry, + envRegistry: process.env.DT_SKILL_REGISTRY, + }); return { workdir, dir, diff --git a/dt-skill/src/cli/registry.test.ts b/dt-skill/src/cli/registry.test.ts index 7e12798..2bb16c9 100644 --- a/dt-skill/src/cli/registry.test.ts +++ b/dt-skill/src/cli/registry.test.ts @@ -17,8 +17,14 @@ vi.mock('../discovery.js', () => ({ discoverRegistryFromSite: (...args: unknown[]) => discoverRegistryFromSite(...args), })); -const { DEFAULT_REGISTRY, DEFAULT_SITE, getRegistry, normalizeRegistryBase, resolveRegistry } = - await import('./registry'); +const { + DEFAULT_REGISTRY, + DEFAULT_SITE, + getRegistry, + normalizeRegistryBase, + pickRegistryFromCliAndEnv, + resolveRegistry, +} = await import('./registry'); function makeOpts(overrides: Partial = {}): GlobalOpts { return { @@ -115,41 +121,49 @@ describe('registry resolution', () => { }); }); - it('--registry (cli) overrides built-in default and cache', async () => { - readGlobalConfig.mockResolvedValue({ registry: DEFAULT_REGISTRY }); - - const registry = await resolveRegistry( - makeOpts({ - registry: 'http://127.0.0.1:7001/', - registrySource: 'cli', - }) - ); - - expect(registry).toBe('http://127.0.0.1:7001'); + it('pickRegistryFromCliAndEnv: cli beats env and default', () => { + const picked = pickRegistryFromCliAndEnv({ + cliRegistry: 'http://127.0.0.1:7001/', + envRegistry: 'http://env.example:7001', + }); + expect(picked).toEqual({ + registry: 'http://127.0.0.1:7001/', + registrySource: 'cli', + }); + // Full resolve still normalizes trailing slash. + expect(normalizeRegistryBase(picked.registry)).toBe('http://127.0.0.1:7001'); }); - it('DT_SKILL_REGISTRY (env) overrides built-in default', async () => { - readGlobalConfig.mockResolvedValue(null); - - const registry = await resolveRegistry( - makeOpts({ - registry: 'http://127.0.0.1:7001', - registrySource: 'env', + it('pickRegistryFromCliAndEnv: env beats built-in default', () => { + expect( + pickRegistryFromCliAndEnv({ + cliRegistry: undefined, + envRegistry: 'http://127.0.0.1:7001', }) - ); + ).toEqual({ + registry: 'http://127.0.0.1:7001', + registrySource: 'env', + }); + }); - expect(registry).toBe('http://127.0.0.1:7001'); - expect(registry).not.toBe(DEFAULT_REGISTRY); + it('pickRegistryFromCliAndEnv: falls back to DEFAULT_REGISTRY', () => { + expect(pickRegistryFromCliAndEnv({})).toEqual({ + registry: DEFAULT_REGISTRY, + registrySource: 'default', + }); }); - it('explicit overrides still beat site discovery', async () => { - readGlobalConfig.mockResolvedValue(null); + it('env-selected registry wins over cache and discovery end-to-end', async () => { + readGlobalConfig.mockResolvedValue({ registry: DEFAULT_REGISTRY }); discoverRegistryFromSite.mockResolvedValue({ apiBase: 'http://discovered:7001' }); + const picked = pickRegistryFromCliAndEnv({ + envRegistry: 'http://127.0.0.1:7001', + }); const registry = await resolveRegistry( makeOpts({ - registry: 'http://127.0.0.1:7001', - registrySource: 'env', + registry: picked.registry, + registrySource: picked.registrySource, site: 'http://site.example', }) ); diff --git a/dt-skill/src/cli/registry.ts b/dt-skill/src/cli/registry.ts index 38c11b4..54120a0 100644 --- a/dt-skill/src/cli/registry.ts +++ b/dt-skill/src/cli/registry.ts @@ -13,6 +13,25 @@ export function normalizeRegistryBase(url: string): string { .replace(/\/+$/, ''); } +/** + * Map CLI flag + env to registry opts (priority: --registry > DT_SKILL_REGISTRY > default). + * Pure helper so unit tests can exercise the real selection path without commander. + */ +export function pickRegistryFromCliAndEnv(args: { + cliRegistry?: string | null; + envRegistry?: string | null; +}): { registry: string; registrySource: 'cli' | 'env' | 'default' } { + const fromCli = String(args.cliRegistry ?? '').trim(); + if (fromCli) { + return { registry: fromCli, registrySource: 'cli' }; + } + const fromEnv = String(args.envRegistry ?? '').trim(); + if (fromEnv) { + return { registry: fromEnv, registrySource: 'env' }; + } + return { registry: DEFAULT_REGISTRY, registrySource: 'default' }; +} + export async function resolveRegistry(opts: GlobalOpts) { const explicit = opts.registrySource !== 'default' ? opts.registry.trim() : ''; if (explicit) return normalizeRegistryBase(explicit); From 64dc1341e4ef8cd0d4ac3728eb6b0f35f959f28b Mon Sep 17 00:00:00 2001 From: huaiju Date: Tue, 28 Jul 2026 18:10:44 +0800 Subject: [PATCH 18/29] docs(dt-skill): document default registry and dev override CLI help, README, and AGENTS.md cover built-in deploy URL and DT_SKILL_REGISTRY / --registry for local dev (ticket 03). --- AGENTS.md | 22 ++++++++++++++++++++++ dt-skill/README.md | 16 ++++++++++++++++ dt-skill/src/cli.ts | 23 +++++++++++++++++++++-- 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index cc0a54d..cd60924 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -129,6 +129,14 @@ test/ # 主项目测试文件（Node.js 内置 test runner） - **构建**: `node ./scripts/build.mjs`，输出到 `dist/`。 - **测试**: Vitest，配置在 `vitest.config.ts`（测试 `src/**/*.test.ts`）。 - **Node 版本要求**: `>=20`（与主项目的 `>=18` 不同）。 +- **默认 Registry**: 内网部署 `http://172.16.100.225:7001`（无 flag/env 时开箱即用）。 +- **本地开发覆盖**: + ```bash + export DT_SKILL_REGISTRY=http://127.0.0.1:7001 + # 或 + node bin/dt-skill.js --registry http://127.0.0.1:7001 search foo + ``` + 优先级：`--registry` > `DT_SKILL_REGISTRY` > 本机缓存 > site 发现 > 内置默认。 ## 测试 @@ -156,3 +164,17 @@ test/ # 主项目测试文件（Node.js 内置 test runner） - `dev`: 主开发分支。 - `feat_版本号_xxx`: 新特性分支，从 `master` 切出，开发完 PR 到 `dev`。 - `hotfix_版本号_xxx`: Bug 修复分支，从 `master` 切出，修复完 PR 到 `dev`，验证后合并到 `master`。 + +## Agent skills + +### Issue tracker + +Issues / specs / tickets live as **local markdown** under `.scratch//`. See `docs/agents/issue-tracker.md`. + +### Triage labels + +Default vocabulary: `needs-triage`, `needs-info`, `ready-for-agent`, `ready-for-human`, `wontfix` (written as `Status:` on local ticket files). See `docs/agents/triage-labels.md`. + +### Domain docs + +**Single-context** layout: optional root `CONTEXT.md` + `docs/adr/`. See `docs/agents/domain.md`. diff --git a/dt-skill/README.md b/dt-skill/README.md index 9233697..73bbd82 100644 --- a/dt-skill/README.md +++ b/dt-skill/README.md @@ -15,6 +15,22 @@ npx dt-skill --help node dt-skill/bin/dt-skill.js --help ``` +## Registry + +Out of the box the CLI talks to the intranet deploy registry: + +`http://172.16.100.225:7001` + +Override for local Doraemon (`npm run dev` on :7001) or another environment: + +```bash +export DT_SKILL_REGISTRY=http://127.0.0.1:7001 +# or one-shot +dt-skill --registry http://127.0.0.1:7001 search "query" +``` + +Priority: `--registry` → `DT_SKILL_REGISTRY` → cached global config → `--site` / `DT_SKILL_SITE` discovery → built-in default. + ## Skill Package Installation When you install a skill package (a parent skill containing multiple child skills), the CLI presents an interactive fuzzy-search multiselect prompt so you can choose which sub-skills to install: diff --git a/dt-skill/src/cli.ts b/dt-skill/src/cli.ts index 7eb1f5d..49d4b43 100644 --- a/dt-skill/src/cli.ts +++ b/dt-skill/src/cli.ts @@ -45,7 +45,7 @@ const program = new Command() .option('--workdir ', 'Working directory (default: cwd)') .option('--dir ', 'Skills directory (relative to workdir, default: skills)') .option('--site ', 'Doraemon site URL for registry discovery') - .option('--registry ', 'Registry API base URL') + .option('--registry ', 'Registry API base URL (overrides default and DT_SKILL_REGISTRY)') .option( '-a, --agent ', `Target agent(s) for symlinks (${ @@ -62,7 +62,26 @@ const program = new Command() .showSuggestionAfterError() .addHelpText( 'after', - styleEnvBlock('\nEnv:\n DT_SKILL_SITE\n DT_SKILL_REGISTRY\n DT_SKILL_WORKDIR\n') + styleEnvBlock( + [ + '', + 'Registry (first match wins):', + ' 1. --registry ', + ' 2. DT_SKILL_REGISTRY', + ' 3. cached global config', + ' 4. --site / DT_SKILL_SITE discovery', + ' 5. built-in default: http://172.16.100.225:7001', + '', + 'Dev example:', + ' export DT_SKILL_REGISTRY=http://127.0.0.1:7001', + '', + 'Env:', + ' DT_SKILL_SITE', + ' DT_SKILL_REGISTRY', + ' DT_SKILL_WORKDIR', + '', + ].join('\n') + ) ); configureCommanderHelp(program); From 5e9dc411d1b9b42ac28e9637f64ba1dfb6858bc6 Mon Sep 17 00:00:00 2001 From: huaiju Date: Tue, 28 Jul 2026 18:11:42 +0800 Subject: [PATCH 19/29] fix(dt-skill): align README Defaults with built-in registry Remove stale CLAWHUB_ / no-default wording; interpolate DEFAULT_REGISTRY in help. --- dt-skill/README.md | 11 +++++------ dt-skill/src/cli.ts | 8 ++++++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/dt-skill/README.md b/dt-skill/README.md index 73bbd82..2188dfa 100644 --- a/dt-skill/README.md +++ b/dt-skill/README.md @@ -172,9 +172,8 @@ dt-skill sync --root ../clawdis/skills --all --dry-run ## Defaults -- Registry: no static default. Commands copied from the Doraemon Skills page include `--registry `. -- Registry resolution order: `--registry`, `CLAWHUB_REGISTRY` (legacy `CLAWDHUB_REGISTRY`), cached config, then discovery from an explicit site. -- Explicit registry addresses are cached for later commands. Override the config path via `CLAWHUB_CONFIG_PATH` (legacy `CLAWDHUB_CONFIG_PATH`). -- Site discovery: opt in with `--site`, `CLAWHUB_SITE`, or legacy `CLAWDHUB_SITE`; the site must expose `/.well-known/clawhub.json`. -- Workdir: current directory (falls back to Clawdbot workspace if configured; override via `--workdir` or `CLAWHUB_WORKDIR`) -- Install dir: `./skills` under workdir (override via `--dir`) +- Registry built-in default: `http://172.16.100.225:7001` (intranet deploy). +- Registry resolution order: `--registry` → `DT_SKILL_REGISTRY` → cached global config → `--site` / `DT_SKILL_SITE` discovery → built-in default. +- Successful resolutions are cached for later commands. +- Workdir: current directory (override via `--workdir` or `DT_SKILL_WORKDIR`). +- Canonical skills dir: `/.agents/skills` (see CLI `--global` / project layout). diff --git a/dt-skill/src/cli.ts b/dt-skill/src/cli.ts index 49d4b43..b2b144a 100644 --- a/dt-skill/src/cli.ts +++ b/dt-skill/src/cli.ts @@ -27,7 +27,11 @@ import { import { cmdStarSkill } from './cli/commands/star.js'; import { cmdUnstarSkill } from './cli/commands/unstar.js'; import { configureCommanderHelp, styleEnvBlock, styleTitle } from './cli/helpStyle.js'; -import { DEFAULT_SITE, pickRegistryFromCliAndEnv } from './cli/registry.js'; +import { + DEFAULT_REGISTRY, + DEFAULT_SITE, + pickRegistryFromCliAndEnv, +} from './cli/registry.js'; import type { GlobalOpts } from './cli/types.js'; import { fail } from './cli/ui.js'; @@ -70,7 +74,7 @@ const program = new Command() ' 2. DT_SKILL_REGISTRY', ' 3. cached global config', ' 4. --site / DT_SKILL_SITE discovery', - ' 5. built-in default: http://172.16.100.225:7001', + ` 5. built-in default: ${DEFAULT_REGISTRY}`, '', 'Dev example:', ' export DT_SKILL_REGISTRY=http://127.0.0.1:7001', From 8379b85b836ee814016a4188e4151d21c0bd831a Mon Sep 17 00:00:00 2001 From: huaiju Date: Tue, 28 Jul 2026 18:22:23 +0800 Subject: [PATCH 20/29] style(dt-skill): prettier format cli help registry block --- dt-skill/src/cli.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/dt-skill/src/cli.ts b/dt-skill/src/cli.ts index b2b144a..f6d58b3 100644 --- a/dt-skill/src/cli.ts +++ b/dt-skill/src/cli.ts @@ -27,11 +27,7 @@ import { import { cmdStarSkill } from './cli/commands/star.js'; import { cmdUnstarSkill } from './cli/commands/unstar.js'; import { configureCommanderHelp, styleEnvBlock, styleTitle } from './cli/helpStyle.js'; -import { - DEFAULT_REGISTRY, - DEFAULT_SITE, - pickRegistryFromCliAndEnv, -} from './cli/registry.js'; +import { DEFAULT_REGISTRY, DEFAULT_SITE, pickRegistryFromCliAndEnv } from './cli/registry.js'; import type { GlobalOpts } from './cli/types.js'; import { fail } from './cli/ui.js'; From 5bc214d1c62d883a748fa3dec0d1f284a0d3a67a Mon Sep 17 00:00:00 2001 From: huaiju Date: Wed, 29 Jul 2026 09:18:31 +0800 Subject: [PATCH 21/29] chore(dt-skill): release 0.18.4 metadata for DTStack/doraemon Point repository/homepage/bugs at DTStack/doraemon, bump version to 0.18.4 for public npm, and include package LICENSE. --- dt-skill/LICENSE | 21 +++++++++++++++++++++ dt-skill/package.json | 8 ++++---- 2 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 dt-skill/LICENSE diff --git a/dt-skill/LICENSE b/dt-skill/LICENSE new file mode 100644 index 0000000..6c44e14 --- /dev/null +++ b/dt-skill/LICENSE @@ -0,0 +1,21 @@ +The MIT License (MIT) + +Copyright (c) 2017 sky. + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/dt-skill/package.json b/dt-skill/package.json index 3f911da..f9d3c81 100644 --- a/dt-skill/package.json +++ b/dt-skill/package.json @@ -1,15 +1,15 @@ { "name": "dt-skill", - "version": "0.1.0", + "version": "0.18.4", "description": "dt-skill CLI — install, update, search, and publish agent skills.", - "homepage": "https://github.com/TreeTreeDi/doraemon", + "homepage": "https://github.com/DTStack/doraemon/tree/master/dt-skill", "bugs": { - "url": "https://github.com/TreeTreeDi/doraemon/issues" + "url": "https://github.com/DTStack/doraemon/issues" }, "license": "MIT", "repository": { "type": "git", - "url": "git+https://github.com/TreeTreeDi/doraemon.git", + "url": "git+https://github.com/DTStack/doraemon.git", "directory": "dt-skill" }, "bin": { From 68f76f835024dc5b686beccaed9329158be9e671 Mon Sep 17 00:00:00 2001 From: huaiju Date: Wed, 29 Jul 2026 09:18:36 +0800 Subject: [PATCH 22/29] chore(dt-skill): ignore npm pack tarballs --- dt-skill/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/dt-skill/.gitignore b/dt-skill/.gitignore index ebe0103..73a6821 100644 --- a/dt-skill/.gitignore +++ b/dt-skill/.gitignore @@ -4,3 +4,4 @@ dist/ node_modules/ skills-lock.json +dt-skill-*.tgz From d43b2dca0b077c70a0f8f0c73e19369b4522066e Mon Sep 17 00:00:00 2001 From: huaiju Date: Wed, 29 Jul 2026 09:43:25 +0800 Subject: [PATCH 23/29] fix(skills): do not mark UTF-8 skills binary at 4k boundary --- app/service/skillsRegistry.js | 2 +- test/skills-registry-contract.test.js | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/service/skillsRegistry.js b/app/service/skillsRegistry.js index 370ce54..d4e1865 100644 --- a/app/service/skillsRegistry.js +++ b/app/service/skillsRegistry.js @@ -688,7 +688,7 @@ class SkillsRegistryService extends Service { const sample = buffer.subarray(0, Math.min(buffer.length, 4096)); if (sample.includes(0)) return true; try { - new TextDecoder('utf-8', { fatal: true }).decode(sample); + new TextDecoder('utf-8', { fatal: true }).decode(buffer); return false; } catch { return true; diff --git a/test/skills-registry-contract.test.js b/test/skills-registry-contract.test.js index 337f252..fce5788 100644 --- a/test/skills-registry-contract.test.js +++ b/test/skills-registry-contract.test.js @@ -573,6 +573,15 @@ test('isBinaryBuffer detects invalid UTF-8 content', () => { assert.equal(service.isBinaryBuffer(Buffer.from([0x61, 0x00, 0x62])), true); }); +test('isBinaryBuffer does not reject valid UTF-8 split at the 4096-byte sample boundary', () => { + const service = Object.create(SkillsRegistryService.prototype); + // 4094 ASCII + multi-byte 答 would be truncated mid-sequence if we only decoded 4096 bytes. + const prefix = 'a'.repeat(4094); + const buffer = Buffer.from(`${prefix}答后续内容 fan-out：已就绪`, 'utf8'); + assert.equal(buffer.subarray(0, 4096).toString('hex').endsWith('e7ad'), true); + assert.equal(service.isBinaryBuffer(buffer), false); +}); + test('publishSkill rejects missing SKILL.md', async () => { const service = Object.create(SkillsRegistryService.prototype); service.app = createMockApp({ From 7c29957880d30a8270c090aa903752eb891f6fff Mon Sep 17 00:00:00 2001 From: huaiju Date: Wed, 29 Jul 2026 18:14:21 +0800 Subject: [PATCH 24/29] feat(skills): set contributor from git user.name on CLI publish Local dt-skill publish (single and batch) fills contributor from cwd git config user.name when set, logs the value, and fails if the name exceeds 50 characters. Registry v1 publishSkill now persists contributor so the single-skill path matches import-file. --- app/service/skillsRegistry.js | 44 ++++--- dt-skill/src/cli/commands/publish.test.ts | 138 ++++++++++++++++++++++ dt-skill/src/cli/commands/publish.ts | 21 ++++ dt-skill/src/cli/gitContributor.test.ts | 103 ++++++++++++++++ dt-skill/src/cli/gitContributor.ts | 40 +++++++ test/skills-registry-contract.test.js | 90 ++++++++++++++ 6 files changed, 422 insertions(+), 14 deletions(-) create mode 100644 dt-skill/src/cli/gitContributor.test.ts create mode 100644 dt-skill/src/cli/gitContributor.ts diff --git a/app/service/skillsRegistry.js b/app/service/skillsRegistry.js index d4e1865..e71c8ad 100644 --- a/app/service/skillsRegistry.js +++ b/app/service/skillsRegistry.js @@ -14,6 +14,8 @@ const SEMVER_PATTERN = /^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-[\w.-]+)?(? const SKILL_SLUG_PATTERN = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; /** Compatibility placeholder when client omits version; content hash is the change signal. */ const DEFAULT_PUBLISH_VERSION = '0.0.0'; +/** Matches skills_items.contributor VARCHAR(50) and marketplace UI max length. */ +const MAX_CONTRIBUTOR_LENGTH = 50; class SkillsRegistryService extends Service { // Well-Known Registry Metadata @@ -482,12 +484,22 @@ class SkillsRegistryService extends Service { await skill.update({ file_count: processedFiles.length }, { transaction }); } + validateContributor(value) { + const contributor = String(value || '').trim(); + if (contributor.length > MAX_CONTRIBUTOR_LENGTH) { + this.ctx.throw(400, `贡献者不能超过 ${MAX_CONTRIBUTOR_LENGTH} 个字符`); + } + return contributor; + } + // Publish or update a skill (single-slot per slug; content hash is the change signal) async publishSkill(payload, files) { const { SkillsItem, SkillsSource } = this.app.model; const { slug, displayName, tags } = payload; const version = this.resolvePublishVersion(payload.version); const category = this.resolvePublishCategory(payload.category); + const hasContributor = Object.prototype.hasOwnProperty.call(payload, 'contributor'); + const contributor = hasContributor ? this.validateContributor(payload.contributor) : ''; if (!SKILL_SLUG_PATTERN.test(String(slug || ''))) { this.ctx.throw(400, 'slug 格式无效'); @@ -529,22 +541,26 @@ class SkillsRegistryService extends Service { } else if (skill.category) { updatePayload.category = skill.category; } + if (hasContributor) { + updatePayload.contributor = contributor || null; + } await skill.update(updatePayload, { transaction: t }); } else { - skill = await SkillsItem.create( - { - source_id: source.id, - slug, - name: displayName, - description: payload.description || '', - version, - tags: JSON.stringify(parsedTags), - skill_md: skillMdFile.content || '', - category: category || '通用', - file_count: processedFiles.length, - }, - { transaction: t } - ); + const createPayload = { + source_id: source.id, + slug, + name: displayName, + description: payload.description || '', + version, + tags: JSON.stringify(parsedTags), + skill_md: skillMdFile.content || '', + category: category || '通用', + file_count: processedFiles.length, + }; + if (hasContributor) { + createPayload.contributor = contributor || null; + } + skill = await SkillsItem.create(createPayload, { transaction: t }); } await this.replaceSkillStoredFiles(skill, processedFiles, t); diff --git a/dt-skill/src/cli/commands/publish.test.ts b/dt-skill/src/cli/commands/publish.test.ts index fbb142e..3cae934 100644 --- a/dt-skill/src/cli/commands/publish.test.ts +++ b/dt-skill/src/cli/commands/publish.test.ts @@ -18,10 +18,16 @@ const httpMocks = createHttpModuleMocks(); const uiMocks = createUiModuleMocks({ interactive: true }); const mockSearchMultiselect = vi.fn(); +const mockResolvePublishContributor = vi.fn((): string | null => null); vi.mock('../registry.js', () => registryMocks.moduleFactory()); vi.mock('../../http.js', () => httpMocks.moduleFactory()); vi.mock('../ui.js', () => uiMocks.moduleFactory()); +vi.mock('../gitContributor.js', () => ({ + MAX_CONTRIBUTOR_LENGTH: 50, + resolvePublishContributor: () => mockResolvePublishContributor(), + resolveGitUserName: () => mockResolvePublishContributor(), +})); vi.mock('../prompts/search-multiselect.js', async () => { const actual = await vi.importActual('../prompts/search-multiselect.js'); return { @@ -44,9 +50,106 @@ function makeOpts(workdir: string) { afterEach(() => { vi.restoreAllMocks(); vi.clearAllMocks(); + mockResolvePublishContributor.mockReset(); + mockResolvePublishContributor.mockReturnValue(null); }); describe('cmdPublish', () => { + it('includes contributor from git user.name in v1 publish payload', async () => { + const workdir = await makeTmpWorkdir(); + try { + const folder = join(workdir, 'with-contributor'); + await mkdir(folder, { recursive: true }); + await writeFile(join(folder, 'SKILL.md'), '# Skill\n', 'utf8'); + + mockResolvePublishContributor.mockReturnValue('张三'); + httpMocks.apiRequest.mockRejectedValueOnce(new Error('not found')); + httpMocks.apiRequestForm.mockResolvedValueOnce({ + ok: true, + skillId: '1', + versionId: 'v0.0.0', + fingerprint: 'abc', + unchanged: false, + }); + + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + await cmdPublish(makeOpts(workdir), 'with-contributor', { + category: '通用', + yes: true, + }); + + expect(logSpy).toHaveBeenCalledWith('contributor: 张三 (from git user.name)'); + const publishCall = httpMocks.apiRequestForm.mock.calls.find((call) => { + const req = call[1] as { path?: string } | undefined; + return req?.path === '/api/v1/skills'; + }); + const form = (publishCall?.[1] as { form?: FormData }).form as FormData; + const payload = JSON.parse(String(form.get('payload'))); + expect(payload.contributor).toBe('张三'); + logSpy.mockRestore(); + } finally { + await rm(workdir, { recursive: true, force: true }); + } + }); + + it('omits contributor when git user.name is unset', async () => { + const workdir = await makeTmpWorkdir(); + try { + const folder = join(workdir, 'no-contributor'); + await mkdir(folder, { recursive: true }); + await writeFile(join(folder, 'SKILL.md'), '# Skill\n', 'utf8'); + + mockResolvePublishContributor.mockReturnValue(null); + httpMocks.apiRequest.mockRejectedValueOnce(new Error('not found')); + httpMocks.apiRequestForm.mockResolvedValueOnce({ + ok: true, + skillId: '1', + versionId: 'v0.0.0', + fingerprint: 'abc', + unchanged: false, + }); + + await cmdPublish(makeOpts(workdir), 'no-contributor', { + category: '通用', + yes: true, + }); + + const publishCall = httpMocks.apiRequestForm.mock.calls.find((call) => { + const req = call[1] as { path?: string } | undefined; + return req?.path === '/api/v1/skills'; + }); + const form = (publishCall?.[1] as { form?: FormData }).form as FormData; + const payload = JSON.parse(String(form.get('payload'))); + expect(payload).not.toHaveProperty('contributor'); + } finally { + await rm(workdir, { recursive: true, force: true }); + } + }); + + it('fails publish when git user.name exceeds 50 characters', async () => { + const workdir = await makeTmpWorkdir(); + try { + const folder = join(workdir, 'long-name'); + await mkdir(folder, { recursive: true }); + await writeFile(join(folder, 'SKILL.md'), '# Skill\n', 'utf8'); + + mockResolvePublishContributor.mockImplementation(() => { + throw new Error('contributor 不能超过 50 个字符（当前 git user.name 为 51 字）'); + }); + + await expect( + cmdPublish(makeOpts(workdir), 'long-name', { + category: '通用', + yes: true, + }) + ).rejects.toThrow(/不能超过 50/); + expect(httpMocks.apiRequestForm).not.toHaveBeenCalled(); + } finally { + await rm(workdir, { recursive: true, force: true }); + } + }); + it('does not prompt overwrite when remote fingerprint matches local content', async () => { const workdir = await makeTmpWorkdir(); try { @@ -400,6 +503,41 @@ describe('cmdPublish', () => { } }); + it('includes contributor on import-file when git user.name is set', async () => { + const workdir = await makeTmpWorkdir(); + try { + const skillsDir = join(workdir, 'batch-contributor'); + await mkdir(join(skillsDir, 'skill-a'), { recursive: true }); + await mkdir(join(skillsDir, 'skill-b'), { recursive: true }); + await writeFile(join(skillsDir, 'skill-a', 'SKILL.md'), '# A\n', 'utf8'); + await writeFile(join(skillsDir, 'skill-b', 'SKILL.md'), '# B\n', 'utf8'); + + mockResolvePublishContributor.mockReturnValue('包作者'); + httpMocks.apiRequestForm.mockResolvedValueOnce({ + success: true, + data: { + importedCount: 2, + refreshedCount: 2, + importedSkills: [ + { slug: 'skill-a', name: 'skill-a' }, + { slug: 'skill-b', name: 'skill-b' }, + ], + }, + }); + + await cmdPublish(makeOpts(workdir), 'batch-contributor', { all: true }); + + const batchCall = httpMocks.apiRequestForm.mock.calls.find((call: any[]) => { + const req = call[1] as { path?: string } | undefined; + return req?.path === '/api/skills/import-file'; + }); + const form = (batchCall![1] as { form?: FormData }).form as FormData; + expect(form.get('contributor')).toBe('包作者'); + } finally { + await rm(workdir, { recursive: true, force: true }); + } + }); + it('packs selected skills into ZIP with correct structure (T005)', async () => { const workdir = await makeTmpWorkdir(); try { diff --git a/dt-skill/src/cli/commands/publish.ts b/dt-skill/src/cli/commands/publish.ts index fe42e5e..260bd75 100644 --- a/dt-skill/src/cli/commands/publish.ts +++ b/dt-skill/src/cli/commands/publish.ts @@ -12,6 +12,7 @@ import { } from '../../schema/index.js'; import { hashSkillFiles, listPublishFiles } from '../../skills.js'; import { searchMultiselect } from '../prompts/search-multiselect.js'; +import { resolvePublishContributor } from '../gitContributor.js'; import { getRegistry } from '../registry.js'; import { findSkillFolders } from '../scanSkills.js'; import { SKILL_CATEGORY_OPTIONS, SKILL_CATEGORY_SET } from '../skillCategories.js'; @@ -32,6 +33,22 @@ export { SKILL_CATEGORY_OPTIONS }; /** Internal compatibility version when author omits --version (hash is the change signal). */ const DEFAULT_PUBLISH_VERSION = '0.0.0'; +/** + * Resolve contributor from cwd git user.name (Decision: local publish only). + * Logs when set; silent when unset; fails when name exceeds server length limit. + */ +function resolveContributorForPublish(): string | null { + try { + const contributor = resolvePublishContributor(process.cwd()); + if (contributor) { + console.log(`contributor: ${contributor} (from git user.name)`); + } + return contributor; + } catch (error) { + fail(formatError(error)); + } +} + export async function cmdPublish( opts: GlobalOpts, folderArg: string, @@ -72,6 +89,7 @@ export async function cmdPublish( // Single skill mode (existing logic) const registry = await getRegistry(opts, { cache: true }); + const contributor = resolveContributorForPublish(); const slug = options.slug ?? sanitizeSlug(basename(folder)); const displayName = options.name ?? titleCase(basename(folder)); @@ -182,6 +200,7 @@ export async function cmdPublish( tags, ...(category ? { category } : {}), ...(forkOf ? { forkOf } : {}), + ...(contributor ? { contributor } : {}), }) ); @@ -298,6 +317,7 @@ export async function cmdPublishBatch( } ) { const registry = await getRegistry(opts, { cache: true }); + const contributor = resolveContributorForPublish(); let selectedSkills = discoveredSkills; @@ -351,6 +371,7 @@ export async function cmdPublishBatch( if (packageBaseName) form.set('packageName', packageBaseName); if (options.category) form.set('category', options.category); if (options.tags) form.set('tags', options.tags); + if (contributor) form.set('contributor', contributor); const result = await apiRequestForm<{ success: boolean; diff --git a/dt-skill/src/cli/gitContributor.test.ts b/dt-skill/src/cli/gitContributor.test.ts new file mode 100644 index 0000000..e218fbb --- /dev/null +++ b/dt-skill/src/cli/gitContributor.test.ts @@ -0,0 +1,103 @@ +/* @vitest-environment node */ + +import { spawnSync } from 'node:child_process'; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; + +import { + MAX_CONTRIBUTOR_LENGTH, + resolveGitUserName, + resolvePublishContributor, +} from './gitContributor.js'; + +function runGit(cwd: string, args: string[], env?: NodeJS.ProcessEnv) { + const result = spawnSync('git', ['-C', cwd, ...args], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + env: env ?? process.env, + }); + if (result.status !== 0) { + throw new Error(`git ${args.join(' ')} failed: ${result.stderr || result.stdout}`); + } +} + +const tempDirs: string[] = []; + +afterEach(async () => { + while (tempDirs.length > 0) { + const dir = tempDirs.pop(); + if (dir) await rm(dir, { recursive: true, force: true }); + } +}); + +async function makeIsolatedGitEnv() { + const home = await mkdtemp(join(tmpdir(), 'dt-skill-git-home-')); + tempDirs.push(home); + const globalConfig = join(home, 'gitconfig'); + await writeFile(globalConfig, '', 'utf8'); + return { + ...process.env, + HOME: home, + GIT_CONFIG_GLOBAL: globalConfig, + GIT_CONFIG_SYSTEM: '/dev/null', + GIT_CONFIG_NOSYSTEM: '1', + }; +} + +async function makeGitDir(localName: string | null, env: NodeJS.ProcessEnv) { + const dir = await mkdtemp(join(tmpdir(), 'dt-skill-git-contributor-')); + tempDirs.push(dir); + runGit(dir, ['init'], env); + runGit(dir, ['config', 'user.email', 'test@example.com'], env); + if (localName !== null) { + runGit(dir, ['config', 'user.name', localName], env); + } + return dir; +} + +describe('resolveGitUserName', () => { + it('reads user.name from the given cwd repo', async () => { + const env = await makeIsolatedGitEnv(); + const dir = await makeGitDir('张三', env); + expect(resolveGitUserName(dir, env)).toBe('张三'); + }); + + it('returns null when user.name is unset', async () => { + const env = await makeIsolatedGitEnv(); + const dir = await makeGitDir(null, env); + expect(resolveGitUserName(dir, env)).toBeNull(); + }); +}); + +describe('resolvePublishContributor', () => { + it('returns null when git user.name is unset', async () => { + const env = await makeIsolatedGitEnv(); + const dir = await makeGitDir(null, env); + expect(resolvePublishContributor(dir, env)).toBeNull(); + }); + + it('returns git user.name when within length limit', async () => { + const env = await makeIsolatedGitEnv(); + const dir = await makeGitDir('李四', env); + expect(resolvePublishContributor(dir, env)).toBe('李四'); + }); + + it('throws when git user.name exceeds MAX_CONTRIBUTOR_LENGTH', async () => { + const env = await makeIsolatedGitEnv(); + const longName = 'x'.repeat(MAX_CONTRIBUTOR_LENGTH + 1); + const dir = await makeGitDir(longName, env); + expect(() => resolvePublishContributor(dir, env)).toThrow(/不能超过 50/); + expect(() => resolvePublishContributor(dir, env)).toThrow( + new RegExp(`当前 git user.name 为 ${MAX_CONTRIBUTOR_LENGTH + 1} 字`) + ); + }); + + it('returns name at exactly MAX_CONTRIBUTOR_LENGTH', async () => { + const env = await makeIsolatedGitEnv(); + const exact = 'y'.repeat(MAX_CONTRIBUTOR_LENGTH); + const dir = await makeGitDir(exact, env); + expect(resolvePublishContributor(dir, env)).toBe(exact); + }); +}); diff --git a/dt-skill/src/cli/gitContributor.ts b/dt-skill/src/cli/gitContributor.ts new file mode 100644 index 0000000..38aa5a7 --- /dev/null +++ b/dt-skill/src/cli/gitContributor.ts @@ -0,0 +1,40 @@ +import { spawnSync } from 'node:child_process'; + +/** Matches server skills.contributor VARCHAR(50) / MAX_CONTRIBUTOR_LENGTH. */ +export const MAX_CONTRIBUTOR_LENGTH = 50; + +/** + * Read git user.name for the given working directory (repo-local, then global). + * Uses the caller's cwd semantics: pass process.cwd() at publish time. + */ +export function resolveGitUserName( + cwd: string = process.cwd(), + env: NodeJS.ProcessEnv = process.env +): string | null { + const result = spawnSync('git', ['-C', cwd, 'config', 'user.name'], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'ignore'], + env, + }); + if (result.status !== 0) return null; + const value = String(result.stdout || '').trim(); + return value || null; +} + +/** + * Contributor for local publish: git user.name, or null when unset. + * Throws when name exceeds MAX_CONTRIBUTOR_LENGTH (CLI fails before upload). + */ +export function resolvePublishContributor( + cwd: string = process.cwd(), + env: NodeJS.ProcessEnv = process.env +): string | null { + const name = resolveGitUserName(cwd, env); + if (!name) return null; + if (name.length > MAX_CONTRIBUTOR_LENGTH) { + throw new Error( + `contributor 不能超过 ${MAX_CONTRIBUTOR_LENGTH} 个字符（当前 git user.name 为 ${name.length} 字）` + ); + } + return name; +} diff --git a/test/skills-registry-contract.test.js b/test/skills-registry-contract.test.js index fce5788..8ab6636 100644 --- a/test/skills-registry-contract.test.js +++ b/test/skills-registry-contract.test.js @@ -786,6 +786,96 @@ test('publishSkill same content is unchanged no-op', async () => { assert.equal(typeof result.fingerprint, 'string'); }); +test('publishSkill stores contributor on create when payload includes it', async () => { + const service = Object.create(SkillsRegistryService.prototype); + let createdPayload = null; + service.app = createMockApp({ + SkillsItem: { + findOne: async () => null, + create: async (data) => { + createdPayload = data; + return { id: 201, ...data, update: async () => {} }; + }, + }, + SkillsSource: { + findOrCreate: async () => [{ id: 1 }], + }, + SkillsFile: { + create: async () => ({}), + update: async () => ({}), + }, + }); + service.ctx = createMockCtx(); + service.ctx.logger = { warn: () => {}, info: () => {}, error: () => {} }; + + const result = await service.publishSkill( + { + slug: 'with-contributor', + displayName: 'With Contributor', + version: '1.0.0', + contributor: ' 张三 ', + }, + [{ filepath: 'SKILL.md', content: '# c\n' }] + ); + + assert.equal(result.ok, true); + assert.equal(createdPayload.contributor, '张三'); +}); + +test('publishSkill does not set contributor on create when payload omits it', async () => { + const service = Object.create(SkillsRegistryService.prototype); + let createdPayload = null; + service.app = createMockApp({ + SkillsItem: { + findOne: async () => null, + create: async (data) => { + createdPayload = data; + return { id: 202, ...data, update: async () => {} }; + }, + }, + SkillsSource: { + findOrCreate: async () => [{ id: 1 }], + }, + SkillsFile: { + create: async () => ({}), + update: async () => ({}), + }, + }); + service.ctx = createMockCtx(); + service.ctx.logger = { warn: () => {}, info: () => {}, error: () => {} }; + + await service.publishSkill( + { slug: 'no-contributor', displayName: 'No Contributor', version: '1.0.0' }, + [{ filepath: 'SKILL.md', content: '# n\n' }] + ); + + assert.equal(Object.prototype.hasOwnProperty.call(createdPayload, 'contributor'), false); +}); + +test('publishSkill rejects contributor longer than 50 characters', async () => { + const service = Object.create(SkillsRegistryService.prototype); + service.app = createMockApp(); + service.ctx = createMockCtx(); + service.ctx.throw = (status, message) => { + const err = new Error(message); + err.status = status; + throw err; + }; + + await assert.rejects( + () => + service.publishSkill( + { + slug: 'long-contributor', + displayName: 'Long', + contributor: 'x'.repeat(51), + }, + [{ filepath: 'SKILL.md', content: '# x\n' }] + ), + /贡献者不能超过 50/ + ); +}); + // ============================================================ // Phase 6: US4 Resolve Tests (T028) // ============================================================ From 6f8d6929aa95e5e5d7526f097caf64e805d9612a Mon Sep 17 00:00:00 2001 From: huaiju Date: Wed, 29 Jul 2026 18:16:57 +0800 Subject: [PATCH 25/29] fix(skills): update contributor on content-unchanged publish When v1 publish is a content hash no-op, still persist contributor if the client sent it so git user.name attribution can change without a file edit. --- app/service/skillsRegistry.js | 20 ++++++++- test/skills-registry-contract.test.js | 59 ++++++++++++++++++++++++++- 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/app/service/skillsRegistry.js b/app/service/skillsRegistry.js index e71c8ad..6c2f341 100644 --- a/app/service/skillsRegistry.js +++ b/app/service/skillsRegistry.js @@ -451,10 +451,17 @@ class SkillsRegistryService extends Service { return { processedFiles, skillMdFile }; } - async tryPublishUnchanged(skill, incomingFingerprint, version) { + async tryPublishUnchanged(skill, incomingFingerprint, version, meta = {}, transaction) { if (!skill || skill.is_delete !== 0) return null; const existingFingerprint = await this.computeSkillFingerprint(skill.id); if (!existingFingerprint || existingFingerprint !== incomingFingerprint) return null; + // Content unchanged: still apply optional metadata (e.g. contributor) without re-storing files. + if (meta.hasContributor) { + await skill.update( + { contributor: meta.contributor || null }, + transaction ? { transaction } : undefined + ); + } return { ok: true, skillId: String(skill.id), @@ -522,7 +529,16 @@ class SkillsRegistryService extends Service { let skill = await SkillsItem.findOne({ where: { slug }, transaction: t }); - const noop = await this.tryPublishUnchanged(skill, incomingFingerprint, version); + const noop = await this.tryPublishUnchanged( + skill, + incomingFingerprint, + version, + { + hasContributor, + contributor, + }, + t + ); if (noop) return noop; if (skill) { diff --git a/test/skills-registry-contract.test.js b/test/skills-registry-contract.test.js index 8ab6636..25ae164 100644 --- a/test/skills-registry-contract.test.js +++ b/test/skills-registry-contract.test.js @@ -748,7 +748,9 @@ test('publishSkill same content is unchanged no-op', async () => { name: 'Same', version: '0.0.0', is_delete: 0, - update: async () => {}, + update: async () => { + throw new Error('should not update when content unchanged and no contributor'); + }, }; const files = [ { @@ -786,6 +788,61 @@ test('publishSkill same content is unchanged no-op', async () => { assert.equal(typeof result.fingerprint, 'string'); }); +test('publishSkill same content still updates contributor when provided', async () => { + const service = Object.create(SkillsRegistryService.prototype); + let metaUpdate = null; + const skillRow = { + id: 51, + slug: 'same-contrib', + name: 'Same', + version: '0.0.0', + is_delete: 0, + contributor: '旧署名', + update: async (payload) => { + metaUpdate = payload; + }, + }; + const files = [ + { + file_path: 'SKILL.md', + content: '# same\n', + is_binary: 0, + }, + ]; + service.app = createMockApp({ + SkillsItem: { + findOne: async () => skillRow, + }, + SkillsSource: { + findOrCreate: async () => [{ id: 1 }], + }, + SkillsFile: { + findAll: async () => files, + create: async () => { + throw new Error('should not create on content no-op'); + }, + update: async () => { + throw new Error('should not soft-delete on content no-op'); + }, + }, + }); + service.ctx = createMockCtx(); + service.ctx.logger = { warn: () => {}, info: () => {}, error: () => {} }; + + const result = await service.publishSkill( + { + slug: 'same-contrib', + displayName: 'Same', + contributor: '新署名', + }, + [{ filepath: 'SKILL.md', content: '# same\n' }] + ); + + assert.equal(result.ok, true); + assert.equal(result.unchanged, true); + assert.deepEqual(metaUpdate, { contributor: '新署名' }); +}); + test('publishSkill stores contributor on create when payload includes it', async () => { const service = Object.create(SkillsRegistryService.prototype); let createdPayload = null; From 119c578622dbee486bf0978ddc751a9d95ffdcdc Mon Sep 17 00:00:00 2001 From: huaiju Date: Wed, 29 Jul 2026 18:20:51 +0800 Subject: [PATCH 26/29] style(dt-skill): sort publish imports for eslint --- dt-skill/src/cli/commands/publish.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dt-skill/src/cli/commands/publish.ts b/dt-skill/src/cli/commands/publish.ts index 260bd75..b5d91b3 100644 --- a/dt-skill/src/cli/commands/publish.ts +++ b/dt-skill/src/cli/commands/publish.ts @@ -11,8 +11,8 @@ import { normalizeClawScanNote, } from '../../schema/index.js'; import { hashSkillFiles, listPublishFiles } from '../../skills.js'; -import { searchMultiselect } from '../prompts/search-multiselect.js'; import { resolvePublishContributor } from '../gitContributor.js'; +import { searchMultiselect } from '../prompts/search-multiselect.js'; import { getRegistry } from '../registry.js'; import { findSkillFolders } from '../scanSkills.js'; import { SKILL_CATEGORY_OPTIONS, SKILL_CATEGORY_SET } from '../skillCategories.js'; From 107fb414d2b5e46444d3c55293d9e0f03eeb46bf Mon Sep 17 00:00:00 2001 From: huaiju Date: Wed, 29 Jul 2026 18:43:43 +0800 Subject: [PATCH 27/29] chore(dt-skill): bump version to 0.18.5 Release CLI that fills contributor from git user.name on local publish. --- dt-skill/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dt-skill/package.json b/dt-skill/package.json index f9d3c81..5d6683d 100644 --- a/dt-skill/package.json +++ b/dt-skill/package.json @@ -1,6 +1,6 @@ { "name": "dt-skill", - "version": "0.18.4", + "version": "0.18.5", "description": "dt-skill CLI — install, update, search, and publish agent skills.", "homepage": "https://github.com/DTStack/doraemon/tree/master/dt-skill", "bugs": { From 68522b9ce6e68052a2ce0ad75f9621da40e52cce Mon Sep 17 00:00:00 2001 From: huaiju Date: Wed, 29 Jul 2026 18:53:07 +0800 Subject: [PATCH 28/29] refactor(dt-skill): import fileExists from skillHelpers Drop the duplicate local helper in skills.ts; update.ts already uses the shared skillHelpers implementation. --- dt-skill/src/cli/commands/skills.ts | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/dt-skill/src/cli/commands/skills.ts b/dt-skill/src/cli/commands/skills.ts index eb3b6fb..79b199b 100644 --- a/dt-skill/src/cli/commands/skills.ts +++ b/dt-skill/src/cli/commands/skills.ts @@ -1,4 +1,4 @@ -import { lstat, mkdir, rm, stat } from 'node:fs/promises'; +import { lstat, mkdir, rm } from 'node:fs/promises'; import { homedir } from 'node:os'; import { dirname, join } from 'node:path'; @@ -50,7 +50,7 @@ import { selectInstallMethod, selectScope, } from '../ui.js'; -import { normalizeSkillSlugOrFail } from './skillHelpers.js'; +import { fileExists, normalizeSkillSlugOrFail } from './skillHelpers.js'; const SUSPICIOUS_WARNING = '\n⚠️ Warning: "{slug}" is flagged for ClawHub security review.\n' + @@ -935,12 +935,3 @@ function resolveExploreSort(raw?: string): { sort: ExploreSort; apiSort: ApiExpl `Invalid sort "${raw}". Use newest, updated, downloads, rating, installs, installsAllTime, or trending.` ); } - -async function fileExists(path: string) { - try { - await stat(path); - return true; - } catch { - return false; - } -} From 6ea10ea11b79c7c4a4d187cf37f4cbe641106bd9 Mon Sep 17 00:00:00 2001 From: huaiju Date: Wed, 29 Jul 2026 18:54:23 +0800 Subject: [PATCH 29/29] fix(skills): read publish fingerprint inside transaction Pass the publish transaction into computeSkillFingerprint so the unchanged no-op decision and file listing share one consistent view. --- app/service/skillsRegistry.js | 14 ++++++++++---- test/skills-registry-contract.test.js | 9 ++++++++- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/app/service/skillsRegistry.js b/app/service/skillsRegistry.js index 6c2f341..a2f25f8 100644 --- a/app/service/skillsRegistry.js +++ b/app/service/skillsRegistry.js @@ -453,7 +453,9 @@ class SkillsRegistryService extends Service { async tryPublishUnchanged(skill, incomingFingerprint, version, meta = {}, transaction) { if (!skill || skill.is_delete !== 0) return null; - const existingFingerprint = await this.computeSkillFingerprint(skill.id); + // Read files in the same transaction as publish so concurrent writers cannot + // make the no-op decision against a non-transactional snapshot. + const existingFingerprint = await this.computeSkillFingerprint(skill.id, transaction); if (!existingFingerprint || existingFingerprint !== incomingFingerprint) return null; // Content unchanged: still apply optional metadata (e.g. contributor) without re-storing files. if (meta.hasContributor) { @@ -592,12 +594,16 @@ class SkillsRegistryService extends Service { } // Compute SHA256 fingerprint for a skill - async computeSkillFingerprint(skillId) { + async computeSkillFingerprint(skillId, transaction) { const { SkillsFile } = this.app.model; - const files = await SkillsFile.findAll({ + const query = { where: { skill_id: skillId, is_delete: 0 }, order: [['file_path', 'ASC']], - }); + }; + if (transaction) { + query.transaction = transaction; + } + const files = await SkillsFile.findAll(query); const fingerprintIgnore = this.createFingerprintIgnore(files); return skillFingerprint.buildSkillFingerprintFromStoredFiles(files, { diff --git a/test/skills-registry-contract.test.js b/test/skills-registry-contract.test.js index 25ae164..2a0b118 100644 --- a/test/skills-registry-contract.test.js +++ b/test/skills-registry-contract.test.js @@ -759,6 +759,7 @@ test('publishSkill same content is unchanged no-op', async () => { is_binary: 0, }, ]; + let findAllOptions = null; service.app = createMockApp({ SkillsItem: { findOne: async () => skillRow, @@ -767,7 +768,10 @@ test('publishSkill same content is unchanged no-op', async () => { findOrCreate: async () => [{ id: 1 }], }, SkillsFile: { - findAll: async () => files, + findAll: async (options) => { + findAllOptions = options; + return files; + }, create: async () => { throw new Error('should not create on no-op'); }, @@ -786,6 +790,9 @@ test('publishSkill same content is unchanged no-op', async () => { assert.equal(result.ok, true); assert.equal(result.unchanged, true); assert.equal(typeof result.fingerprint, 'string'); + // Fingerprint read must join the publish transaction (mock tx is {}). + assert.ok(findAllOptions); + assert.ok(Object.prototype.hasOwnProperty.call(findAllOptions, 'transaction')); }); test('publishSkill same content still updates contributor when provided', async () => {