From 7df2e41a0366bb0efde32996c2a5b7646545abde Mon Sep 17 00:00:00 2001 From: Coy Geek <65363919+coygeek@users.noreply.github.com> Date: Wed, 3 Jun 2026 20:40:14 -0700 Subject: [PATCH 1/2] fix(tui-gateway): preserve skill install review Make the non-interactive TUI gateway install route inspect-only so it cannot call the installer with review and confirmation disabled. Return an explicit review-required, installed-false result with skill metadata, and render that result truthfully in both TUI callers without claiming installation is underway or treating the required review as an error. --- tests/tui_gateway/test_protocol.py | 28 +++++++++++ tui_gateway/server.py | 19 ++++--- .../src/__tests__/createSlashHandler.test.ts | 44 +++++++++++++++++ ui-tui/src/app/slash/commands/ops.ts | 39 +++++++++++++-- ui-tui/src/components/skillsHub.tsx | 49 ++++++++++++++++--- 5 files changed, 163 insertions(+), 16 deletions(-) diff --git a/tests/tui_gateway/test_protocol.py b/tests/tui_gateway/test_protocol.py index 2b51d785e103..38cc63243131 100644 --- a/tests/tui_gateway/test_protocol.py +++ b/tests/tui_gateway/test_protocol.py @@ -1692,6 +1692,34 @@ def test_skills_manage_search_uses_tools_hub_sources(server): search.assert_called_once_with("showroom", ["source"], source_filter="all", limit=20) +def test_skills_manage_install_requires_review_surface(server): + inspect_skill = MagicMock(return_value={"name": "showroom", "skill_md_preview": "# showroom"}) + do_install = MagicMock() + fake_skills_hub = types.SimpleNamespace( + do_install=do_install, + inspect_skill=inspect_skill, + ) + + with patch.dict(sys.modules, {"hermes_cli.skills_hub": fake_skills_hub}): + resp = server.handle_request({ + "id": "skills-install", + "method": "skills.manage", + "params": {"action": "install", "query": "showroom"}, + }) + + assert "error" not in resp + assert resp["result"] == { + "info": {"name": "showroom", "skill_md_preview": "# showroom"}, + "installed": False, + "message": "Review the skill details before installing from the CLI.", + "name": "showroom", + "review_required": True, + "status": "review_required", + } + inspect_skill.assert_called_once_with("showroom") + do_install.assert_not_called() + + def test_command_dispatch_steer_fallback_sends_message(server): """command.dispatch /steer with no active agent falls back to send.""" sid = "test-session" diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 847479d46bd8..ea78c76f3316 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -15761,14 +15761,19 @@ def _(rid, params: dict) -> dict: }, ) if action == "install": - from hermes_cli.skills_hub import do_install - - class _Q: - def print(self, *a, **k): - pass + from hermes_cli.skills_hub import inspect_skill - do_install(query, skip_confirm=True, console=_Q()) - return _ok(rid, {"installed": True, "name": query}) + return _ok( + rid, + { + "installed": False, + "name": query, + "status": "review_required", + "review_required": True, + "message": "Review the skill details before installing from the CLI.", + "info": inspect_skill(query) or {}, + }, + ) if action == "browse": from hermes_cli.skills_hub import browse_skills diff --git a/ui-tui/src/__tests__/createSlashHandler.test.ts b/ui-tui/src/__tests__/createSlashHandler.test.ts index 7096798ecf93..ea7faa583c56 100644 --- a/ui-tui/src/__tests__/createSlashHandler.test.ts +++ b/ui-tui/src/__tests__/createSlashHandler.test.ts @@ -336,6 +336,50 @@ describe('createSlashHandler', () => { }) }) + it('shows review-required status for /skills install', async () => { + const rpc = vi.fn(() => + Promise.resolve({ + info: { + description: 'Demo skill', + identifier: 'example/foo', + name: 'foo', + skill_md_preview: '# foo\nreview me', + source: 'community' + }, + installed: false, + message: 'Review the skill details before installing from the CLI.', + review_required: true, + status: 'review_required' + }) + ) + + const ctx = buildCtx({ gateway: { ...buildGateway(), rpc } }) + + expect(createSlashHandler(ctx)('/skills install foo')).toBe(true) + expect(ctx.gateway.rpc).toHaveBeenCalledWith('skills.manage', { + action: 'install', + query: 'foo' + }) + expect(ctx.transcript.sys).toHaveBeenCalledWith('checking install requirements for foo…') + + await vi.waitFor(() => { + expect(ctx.transcript.panel).toHaveBeenCalledWith('Skill Review Required', [ + { + rows: [ + ['Name', 'foo'], + ['Source', 'community'], + ['Identifier', 'example/foo'] + ] + }, + { text: 'Demo skill' }, + { text: '# foo\nreview me', title: 'SKILL.md preview' } + ]) + expect(ctx.transcript.sys).toHaveBeenCalledWith( + 'review required for foo: Review the skill details before installing from the CLI.' + ) + }) + }) + it('opens the pet picker for /pet list only', () => { const ctx = buildCtx() diff --git a/ui-tui/src/app/slash/commands/ops.ts b/ui-tui/src/app/slash/commands/ops.ts index f6f1d5ed5be1..2512369fd81b 100644 --- a/ui-tui/src/app/slash/commands/ops.ts +++ b/ui-tui/src/app/slash/commands/ops.ts @@ -22,8 +22,11 @@ import type { SlashCommand } from '../types.js' interface SkillInfo { category?: string description?: string + identifier?: string name?: string path?: string + skill_md_preview?: string + source?: string } interface SkillsListResponse { @@ -39,8 +42,12 @@ interface SkillsSearchResponse { } interface SkillsInstallResponse { + info?: SkillInfo installed?: boolean + message?: string name?: string + review_required?: boolean + status?: string } interface SkillsBrowseItem { @@ -604,13 +611,39 @@ export const opsCommands: SlashCommand[] = [ return sys('usage: /skills install ') } - sys(`installing ${query}…`) + sys(`checking install requirements for ${query}…`) rpc('skills.manage', { action: 'install', query }) .then( - ctx.guarded(r => + ctx.guarded(r => { + if (r.review_required) { + const info = r.info ?? {} + const name = info.name ?? r.name ?? query + + const rows: [string, string][] = [ + ['Name', String(name)], + ['Source', String(info.source ?? '')], + ['Identifier', String(info.identifier ?? info.path ?? query)] + ] + + const sections: PanelSection[] = [{ rows }] + + if (info.description) { + sections.push({ text: String(info.description) }) + } + + if (info.skill_md_preview) { + sections.push({ text: String(info.skill_md_preview), title: 'SKILL.md preview' }) + } + + panel('Skill Review Required', sections) + sys(`review required for ${name}: ${r.message ?? 'inspect the skill before installing from the CLI'}`) + + return + } + sys(r.installed ? `installed ${r.name ?? query}` : 'install failed') - ) + }) ) .catch(ctx.guardedErr) diff --git a/ui-tui/src/components/skillsHub.tsx b/ui-tui/src/components/skillsHub.tsx index 941ee0b27529..03f2a435a8fc 100644 --- a/ui-tui/src/components/skillsHub.tsx +++ b/ui-tui/src/components/skillsHub.tsx @@ -20,6 +20,7 @@ export function SkillsHub({ gw, onClose, t }: SkillsHubProps) { const [info, setInfo] = useState(null) const [installing, setInstalling] = useState(false) const [err, setErr] = useState('') + const [notice, setNotice] = useState('') const [loading, setLoading] = useState(true) const { stdout } = useStdout() @@ -47,6 +48,7 @@ export function SkillsHub({ gw, onClose, t }: SkillsHubProps) { setStage('skill') setInfo(null) setErr('') + setNotice('') return } @@ -66,6 +68,7 @@ export function SkillsHub({ gw, onClose, t }: SkillsHubProps) { const inspect = (name: string) => { setInfo(null) setErr('') + setNotice('') gw.request<{ info?: SkillInfo }>('skills.manage', { action: 'inspect', query: name }) .then(r => setInfo(r?.info ?? { name })) @@ -75,9 +78,25 @@ export function SkillsHub({ gw, onClose, t }: SkillsHubProps) { const install = (name: string) => { setInstalling(true) setErr('') + setNotice('') - gw.request<{ installed?: boolean; name?: string }>('skills.manage', { action: 'install', query: name }) - .then(() => onClose()) + gw.request('skills.manage', { action: 'install', query: name }) + .then(r => { + if (r?.review_required) { + setInfo(r.info ?? { name }) + setNotice(r.message ?? 'Review required before installing this skill.') + + return + } + + if (r?.installed) { + onClose() + + return + } + + setErr('install failed') + }) .catch((e: unknown) => setErr(rpcErrorMessage(e))) .finally(() => setInstalling(false)) } @@ -92,6 +111,7 @@ export function SkillsHub({ gw, onClose, t }: SkillsHubProps) { setStage('skill') setInfo(null) setErr('') + setNotice('') return } @@ -285,11 +305,18 @@ export function SkillsHub({ gw, onClose, t }: SkillsHubProps) { {info?.category ?? selectedCat} {info?.description ? {info.description} : null} {info?.path ? path: {info.path} : null} - {!info && !err ? loading… : null} + {!info && !err && !notice ? loading… : null} + {notice ? review required: {notice} : null} {err ? error: {err} : null} - {installing ? installing… : null} - - i reinspect · x reinstall · Enter/Esc back · q close + {info?.skill_md_preview ? ( + <> + SKILL.md preview + {info.skill_md_preview} + + ) : null} + {installing ? checking install requirements… : null} + + i reinspect · x review install · Enter/Esc back · q close ) } @@ -299,6 +326,16 @@ interface SkillInfo { description?: string name?: string path?: string + skill_md_preview?: string +} + +interface SkillsInstallResponse { + info?: SkillInfo + installed?: boolean + message?: string + name?: string + review_required?: boolean + status?: string } interface SkillsHubProps { From caffe5fc90eb7728df6b28f96e2b0642925afd60 Mon Sep 17 00:00:00 2001 From: Coy Geek <65363919+coygeek@users.noreply.github.com> Date: Mon, 20 Jul 2026 21:07:50 -0700 Subject: [PATCH 2/2] chore(ci): retrigger transient desktop test