From 75bfb452d9e8e83765c8e7dbfb930c57e2bbf296 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:01:36 +0800 Subject: [PATCH] fix(cli): preserve sync safety guards under force Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- cli/README.md | 6 ++ cli/src/commands/sync.ts | 13 +++-- cli/src/services/sync-service.ts | 30 +++++----- cli/test/integration/sync-command.test.ts | 70 ++++++++++++++++++++++- 4 files changed, 97 insertions(+), 22 deletions(-) diff --git a/cli/README.md b/cli/README.md index a30cefec..d54f6045 100644 --- a/cli/README.md +++ b/cli/README.md @@ -302,6 +302,12 @@ skillhub sync push --all --namespace team-a --submit-review The default workspace is `/.agents/skills`. Pull never overwrites local changes unless `--force` is supplied. Remote removals are reported as `orphaned` and are retained unless `--prune` is supplied. Both destructive cases still require explicit flags. +Sync compares both the published version and package fingerprint. An exact match is `up-to-date`, +while a newer version is `update-available` even when its content is unchanged. An older remote +version, an unorderable version pair, or changed remote content without a version bump is `blocked`. +`--force` cannot bypass these release-safety checks; verify the release and use an explicit +`skillhub install` when replacement is intentional. + Workspace push is non-overwriting: an existing namespace/slug/version is reported as a conflict, including versions that are still uploaded or pending review. Other skills in the same `--all` run continue processing. Namespace sync writes `.skillhub/namespace-sync.json` in the workspace and per-skill `.skillhub/metadata.json` files. These files contain the registry coordinate, published version, aggregate fingerprint, and file hashes used by `status` and `diff`. diff --git a/cli/src/commands/sync.ts b/cli/src/commands/sync.ts index 1c91806e..183c4a3e 100644 --- a/cli/src/commands/sync.ts +++ b/cli/src/commands/sync.ts @@ -47,10 +47,15 @@ export async function syncPullCommand(options: SyncPullOptions): Promise const output = renderPullResult(result, Boolean(options.json), Boolean(options.check)) if (result.failures.length > 0) { process.stdout.write(`${output}\n`) - throw new CliError('namespace sync completed with failures', EXIT.generic, { - namespace: context.namespace, - failures: result.failures - }) + const blocked = result.entries.filter(entry => entry.status === 'blocked') + throw new CliError( + blocked.length > 0 ? 'namespace sync blocked by remote version safety checks' : 'namespace sync completed with failures', + blocked.length > 0 ? EXIT.validation : EXIT.generic, + { + namespace: context.namespace, + failures: result.failures + } + ) } return output } diff --git a/cli/src/services/sync-service.ts b/cli/src/services/sync-service.ts index aba95f6d..80a38521 100644 --- a/cli/src/services/sync-service.ts +++ b/cli/src/services/sync-service.ts @@ -89,22 +89,7 @@ export async function inspectNamespaceWorkspace(options: { } const snapshot = await snapshotSkillDirectory(skillDir) - if (snapshot.fingerprint !== metadata.fingerprint) { - entries.push({ - ...baseEntry(remote, 'local-changed'), - localVersion: metadata.version, - changedFiles: diffSkillFiles(metadata.files, snapshot.files) - }) - continue - } const versionOrder = compareSkillVersions(metadata.version, remote.version) - if (versionOrder === 'remote-newer') { - entries.push({ - ...baseEntry(remote, 'update-available'), - localVersion: metadata.version - }) - continue - } if (versionOrder === 'remote-older') { entries.push({ ...baseEntry(remote, 'blocked'), @@ -129,6 +114,21 @@ export async function inspectNamespaceWorkspace(options: { }) continue } + if (snapshot.fingerprint !== metadata.fingerprint) { + entries.push({ + ...baseEntry(remote, 'local-changed'), + localVersion: metadata.version, + changedFiles: diffSkillFiles(metadata.files, snapshot.files) + }) + continue + } + if (versionOrder === 'remote-newer') { + entries.push({ + ...baseEntry(remote, 'update-available'), + localVersion: metadata.version + }) + continue + } entries.push({ ...baseEntry(remote, 'up-to-date'), localVersion: metadata.version }) } diff --git a/cli/test/integration/sync-command.test.ts b/cli/test/integration/sync-command.test.ts index 09f76ad4..cfa70ab5 100644 --- a/cli/test/integration/sync-command.test.ts +++ b/cli/test/integration/sync-command.test.ts @@ -197,7 +197,7 @@ describe('sync command', () => { 'sync', 'pull', '--namespace', 'team-a', '--dir', skillsDir, '--force', '--registry', registry.url, '--token', 'token', '--json' ], { HOME: env.home }, { cwd: env.cwd }) - expect(pulled.exitCode).toBe(1) + expect(pulled.exitCode).toBe(6) expect(await readFile(join(skillsDir, 'demo', 'SKILL.md'), 'utf8')).toBe('# original\n') } finally { registry.stop() @@ -229,7 +229,7 @@ describe('sync command', () => { 'sync', 'pull', '--namespace', 'team-a', '--dir', skillsDir, '--force', '--registry', registry.url, '--token', 'token', '--json' ], { HOME: env.home }, { cwd: env.cwd }) - expect(pulled.exitCode).toBe(1) + expect(pulled.exitCode).toBe(6) expect(JSON.parse(pulled.stdout).entries[0]).toMatchObject({ status: 'blocked', localVersion: '2.0.0', @@ -267,7 +267,7 @@ describe('sync command', () => { 'sync', 'pull', '--namespace', 'team-a', '--dir', skillsDir, '--force', '--registry', registry.url, '--token', 'token', '--json' ], { HOME: env.home }, { cwd: env.cwd }) - expect(pulled.exitCode).toBe(1) + expect(pulled.exitCode).toBe(6) expect(JSON.parse(pulled.stdout).entries[0]).toMatchObject({ status: 'blocked', localVersion: 'release-a', @@ -279,6 +279,70 @@ describe('sync command', () => { } }) + test('hard remote guards cannot be bypassed by local changes and force', async () => { + const original = makeSkill('# original\n') + const variants = [ + { + name: 'downgrade', + initialVersion: '2.0.0', + remoteVersion: '1.0.0', + remote: original, + reason: 'remote version is older than the installed version; local files were kept' + }, + { + name: 'same-version drift', + initialVersion: '1.0.0', + remoteVersion: '1.0.0', + remote: makeSkill('# changed without a version bump\n'), + reason: 'remote content changed without a newer version; use explicit install after verifying the release' + }, + { + name: 'unknown version order', + initialVersion: 'release-a', + remoteVersion: 'release-b', + remote: original, + reason: 'cannot determine version order; use explicit install after verifying the release' + } + ] + + for (const variant of variants) { + const env = await createTempHome() + const skillsDir = join(env.cwd, 'team-skills') + const skill: FakeSkill = { + namespace: 'team-a', + slug: 'demo', + version: variant.initialVersion, + versionId: 1, + ...original + } + const registry = await startFakeRegistry({ token: 'token', skills: [skill] }) + try { + await runCli([ + 'sync', 'pull', '--namespace', 'team-a', '--dir', skillsDir, + '--registry', registry.url, '--token', 'token' + ], { HOME: env.home }, { cwd: env.cwd }) + await writeFile(join(skillsDir, 'demo', 'SKILL.md'), `# local edit before ${variant.name}\n`) + skill.version = variant.remoteVersion + skill.versionId = 2 + skill.fingerprint = variant.remote.fingerprint + skill.zipBytes = variant.remote.zipBytes + + const pulled = await runCli([ + 'sync', 'pull', '--namespace', 'team-a', '--dir', skillsDir, '--force', + '--registry', registry.url, '--token', 'token', '--json' + ], { HOME: env.home }, { cwd: env.cwd }) + const output = JSON.parse(pulled.stdout) + expect(pulled.exitCode, variant.name).toBe(6) + expect(output.entries[0], variant.name).toMatchObject({ status: 'blocked', reason: variant.reason }) + expect(output.actions, variant.name).toEqual([]) + expect(await readFile(join(skillsDir, 'demo', 'SKILL.md'), 'utf8')) + .toBe(`# local edit before ${variant.name}\n`) + } finally { + registry.stop() + } + } + }) + test('prune removes only unchanged managed orphan skills', async () => { const env = await createTempHome() const skillsDir = join(env.cwd, 'team-skills')