From b9be415d34625d0b50e3c82654f37a03c6ef280c Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Tue, 1 Sep 2026 20:22:18 +0800 Subject: [PATCH] fix(cli): surface install lifecycle warnings Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- cli/src/commands/install.ts | 13 ++++++- cli/src/commands/sync.ts | 1 + cli/src/services/sync-service.ts | 8 +++- cli/test/integration/sync-command.test.ts | 37 +++++++++++++++++++ .../unit/commands/install-command.test.ts | 19 ++++++++++ 5 files changed, 74 insertions(+), 4 deletions(-) diff --git a/cli/src/commands/install.ts b/cli/src/commands/install.ts index 009b9acc..3ab3d678 100644 --- a/cli/src/commands/install.ts +++ b/cli/src/commands/install.ts @@ -115,7 +115,16 @@ export async function installCommand( }) if (options.json) { - return JSON.stringify({ ok: true, namespace, slug, installed: result.installed }) + return JSON.stringify({ + ok: true, + namespace, + slug, + installed: result.installed, + ...(result.warnings?.length ? { warnings: result.warnings } : {}) + }) } - return result.installed.map(i => `Installed ${namespace}/${slug} -> ${i.dir} (${i.agent})`).join('\n') + return [ + ...result.installed.map(i => `Installed ${namespace}/${slug} -> ${i.dir} (${i.agent})`), + ...(result.warnings ?? []).map(warning => `Warning: ${warning}`) + ].join('\n') } diff --git a/cli/src/commands/sync.ts b/cli/src/commands/sync.ts index 3c998ea5..da63ff3a 100644 --- a/cli/src/commands/sync.ts +++ b/cli/src/commands/sync.ts @@ -145,6 +145,7 @@ function renderPullResult(result: PullResult, json: boolean, check: boolean): st ...result.entries .filter(entry => !result.actions.some(action => action.slug === entry.slug)) .map(entry => `${entry.status.padEnd(16)} ${entry.slug}`), + ...result.warnings.map(item => `warning ${item.slug}: ${item.message}`), ...result.failures.map(item => `failed ${item.slug}: ${item.message}`) ] return lines.join('\n') diff --git a/cli/src/services/sync-service.ts b/cli/src/services/sync-service.ts index ca7377bf..20cc9c0a 100644 --- a/cli/src/services/sync-service.ts +++ b/cli/src/services/sync-service.ts @@ -36,6 +36,7 @@ export interface PullResult { entries: SyncStatusEntry[] actions: Array<{ slug: string; action: 'installed' | 'updated' | 'pruned' }> failures: Array<{ slug: string; message: string }> + warnings: Array<{ slug: string; message: string }> } export interface PushResultItem { @@ -133,6 +134,7 @@ export async function pullNamespace(options: { check: boolean prune: boolean force: boolean + installSkillFn?: typeof installSkill }): Promise { const inspected = await inspectNamespaceWorkspace(options) const result: PullResult = { @@ -140,7 +142,8 @@ export async function pullNamespace(options: { rootDir: options.rootDir, entries: inspected.entries, actions: [], - failures: [] + failures: [], + warnings: [] } if (options.check) return result @@ -154,7 +157,7 @@ export async function pullNamespace(options: { const remote = remoteBySlug.get(entry.slug) if (!remote) continue try { - await installSkill({ + const installed = await (options.installSkillFn ?? installSkill)({ registry: options.registry, token: options.token, namespace: options.namespace, @@ -172,6 +175,7 @@ export async function pullNamespace(options: { force: entry.status !== 'not-installed' || options.force }) result.actions.push({ slug: entry.slug, action: entry.status === 'not-installed' ? 'installed' : 'updated' }) + result.warnings.push(...(installed.warnings ?? []).map(message => ({ slug: entry.slug, message }))) } catch (error) { result.failures.push({ slug: entry.slug, message: error instanceof Error ? error.message : 'install failed' }) } diff --git a/cli/test/integration/sync-command.test.ts b/cli/test/integration/sync-command.test.ts index a6e97b39..91da1da1 100644 --- a/cli/test/integration/sync-command.test.ts +++ b/cli/test/integration/sync-command.test.ts @@ -6,6 +6,8 @@ import { describe, expect, test } from 'bun:test' import { startFakeRegistry, type FakeSkill } from '../helpers/fake-registry' import { runCli } from '../helpers/run-cli' import { createTempHome } from '../helpers/temp-env' +import { SkillHubClient } from '../../src/clients/skillhub-client' +import { pullNamespace } from '../../src/services/sync-service' function makeSkill(body: string): { zipBytes: Uint8Array; fingerprint: string } { const content = strToU8(body) @@ -15,6 +17,41 @@ function makeSkill(body: string): { zipBytes: Uint8Array; fingerprint: string } } describe('sync command', () => { + test('pull propagates committed install warnings', async () => { + const env = await createTempHome() + const skillsDir = join(env.cwd, 'team-skills') + const fixture = makeSkill('---\nname: demo\ndescription: Demo\nversion: 1.0.0\n---\n') + const registry = await startFakeRegistry({ + token: 'token', + skills: [{ namespace: 'team-a', slug: 'demo', ...fixture }] + }) + + try { + const result = await pullNamespace({ + client: new SkillHubClient(registry.url, 'token'), + registry: registry.url, + token: 'token', + namespace: 'team-a', + rootDir: skillsDir, + check: false, + prune: false, + force: false, + installSkillFn: async () => ({ + installed: [{ agent: 'workspace', dir: join(skillsDir, 'demo') }], + warnings: ['target lock cleanup failed: simulated release failure'] + }) + }) + + expect(result.actions).toEqual([{ slug: 'demo', action: 'installed' }]) + expect(result.warnings).toEqual([{ + slug: 'demo', + message: 'target lock cleanup failed: simulated release failure' + }]) + } finally { + registry.stop() + } + }) + test('pull installs a namespace incrementally and writes workspace metadata', async () => { const env = await createTempHome() const skillsDir = join(env.cwd, 'team-skills') diff --git a/cli/test/unit/commands/install-command.test.ts b/cli/test/unit/commands/install-command.test.ts index 49911588..8d980610 100644 --- a/cli/test/unit/commands/install-command.test.ts +++ b/cli/test/unit/commands/install-command.test.ts @@ -146,6 +146,25 @@ describe('installCommand dependency injection', () => { }] as AgentCandidate[] } + test('renders post-commit warnings in human and JSON output', async () => { + const deps: InstallCommandDeps = { + isTTY: () => false, + resolveInstallTargets: fakeResolveInstallTargets(), + installSkill: async () => ({ + installed: [{ agent: 'codex', dir: '/home/u/.codex/skills/foo' }], + warnings: ['target lock cleanup failed: simulated release failure'] + }) + } + + const human = await installCommand('@global/foo', { registry: 'http://localhost' }, deps) + expect(human).toContain('Warning: target lock cleanup failed') + const json = JSON.parse(await installCommand('@global/foo', { + registry: 'http://localhost', + json: true + }, deps)) + expect(json.warnings).toEqual(['target lock cleanup failed: simulated release failure']) + }) + test('passes a namespaced coordinate to installSkill', async () => { let received: Parameters>[0] | undefined const deps: InstallCommandDeps = {