From 57563b671f94f9a3471cd4da5702b8de9f88eef7 Mon Sep 17 00:00:00 2001 From: Xubin Ren <52506698+Re-bin@users.noreply.github.com> Date: Fri, 29 May 2026 14:14:56 +0800 Subject: [PATCH] fix(apps): recover stale npm installs --- nanobot/apps/cli/service.py | 53 +++++++++++++++++++ tests/cli_apps/test_service.py | 95 ++++++++++++++++++++++++++++++++++ webui/src/lib/api.ts | 3 +- webui/src/tests/api.test.ts | 16 ++++++ 4 files changed, 166 insertions(+), 1 deletion(-) diff --git a/nanobot/apps/cli/service.py b/nanobot/apps/cli/service.py index 02c7ab2cb..f22e629ff 100644 --- a/nanobot/apps/cli/service.py +++ b/nanobot/apps/cli/service.py @@ -37,6 +37,7 @@ _MAX_TOOL_OUTPUT_CHARS = 12_000 _MAX_ARTIFACT_SCAN_PATHS = 4_000 _MAX_ARTIFACT_REPORT = 12 _SAFE_NAME_RE = re.compile(r"[^a-z0-9_-]+") +_SAFE_NPM_DIR_RE = re.compile(r"^[a-z0-9._-]+$", re.IGNORECASE) _MENTION_RE = re.compile(r"(^|[\s([{])@([a-z0-9_-]+)\b", re.IGNORECASE) _SHELL_META_CHARS = ("|", "&&", "||", ";", "$(", "`", ">", "<") _ENDORSEMENT_WORD_RE = re.compile(r"\bofficial\s+", re.IGNORECASE) @@ -740,6 +741,45 @@ class CliAppManager: return [npm, "install", "-g", package + "@latest"] return [npm, "uninstall", "-g", package] + def _cleanup_stale_npm_install(self, app: dict[str, Any]) -> bool: + npm = shutil.which("npm") + package = str(app.get("npm_package") or "").strip() + if not npm or not package or "/" in package or _SAFE_NPM_DIR_RE.match(package) is None: + return False + result = self._run_argv([npm, "root", "-g"], timeout=min(self.runtime.install_timeout, 30)) + if result.returncode != 0: + return False + root = Path(result.stdout.strip()).expanduser() + try: + root = root.resolve(strict=True) + except OSError: + return False + targets = [root / package, *root.glob(f".{package}-*")] + removed = False + for target in targets: + try: + resolved = target.resolve(strict=False) + if not is_path_within(resolved, root) or not target.is_dir(): + continue + shutil.rmtree(target) + removed = True + except OSError: + continue + return removed + + def _retry_stale_npm_install( + self, + app: dict[str, Any], + argv: list[str], + result: subprocess.CompletedProcess[str], + ) -> subprocess.CompletedProcess[str]: + output = f"{result.stderr}\n{result.stdout}" + if "ENOTEMPTY" not in output or "rename" not in output: + return result + if not self._cleanup_stale_npm_install(app): + return result + return self._run_argv(argv, timeout=self.runtime.install_timeout) + def _split_safe_command(self, app: dict[str, Any], key: str, expected: str) -> list[str]: command = str(app.get(key) or "") if not command: @@ -893,6 +933,17 @@ Use the `run_cli_app` tool with `name="{name}"` for command execution. Do not in if not self._install_supported(app): raise CliAppError("this CLI app uses an unsupported install strategy") strategy = self._strategy(app) + entry_point = str(app.get("entry_point") or "") + if entry_point and shutil.which(entry_point): + self._record_installed(app) + return self.payload() | { + "last_action": { + "ok": True, + "message": f"CLI for {app['display_name']} is already available.", + "installed": True, + "verification": ["entry_point_available", "state_recorded", "managed_paths_present"], + } + } if strategy == "bundled": detect_cmd = str(app.get("detect_cmd") or app.get("entry_point") or "") if detect_cmd and _command_exists(detect_cmd): @@ -910,6 +961,8 @@ Use the `run_cli_app` tool with `name="{name}"` for command execution. Do not in argv = self._argv_for_action(app, "install") assert argv is not None result = self._run_argv(argv, timeout=self.runtime.install_timeout) + if strategy == "npm" and result.returncode != 0: + result = self._retry_stale_npm_install(app, argv, result) if result.returncode != 0: raise CliAppError(_truncate(result.stderr or result.stdout or "install failed"), status=500) self._record_installed(app) diff --git a/tests/cli_apps/test_service.py b/tests/cli_apps/test_service.py index 1c68f2ec0..379389c47 100644 --- a/tests/cli_apps/test_service.py +++ b/tests/cli_apps/test_service.py @@ -302,6 +302,101 @@ def test_install_dispatches_safe_pip_and_installs_skill( assert 'run_cli_app` tool with `name="gimp"' in skill.read_text(encoding="utf-8") +def test_install_records_available_cli_without_reinstalling( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + manager = _manager(tmp_path) + _seed_catalog(manager) + resolved = tmp_path / "bin" / "lark-cli" + resolved.parent.mkdir() + resolved.write_text("#!/bin/sh\n", encoding="utf-8") + + def fail_run(argv: list[str], *, timeout: int) -> subprocess.CompletedProcess[str]: + raise AssertionError(f"unexpected install command: {argv}") + + monkeypatch.setattr(manager, "_run_argv", fail_run) + monkeypatch.setattr( + "nanobot.apps.cli.service.shutil.which", + lambda command: str(resolved) if command == "lark-cli" else None, + ) + + payload = manager.install("feishu") + + assert payload["last_action"]["ok"] is True + assert payload["last_action"]["installed"] is True + assert "entry_point_available" in payload["last_action"]["verification"] + installed = json.loads(manager.installed_path.read_text(encoding="utf-8"))["apps"] + assert installed["feishu"]["entry_point_path"] == str(resolved) + skill = manager.workspace / "skills" / "cli-app-feishu" / "SKILL.md" + assert skill.is_file() + assert 'run_cli_app` tool with `name="feishu"' in skill.read_text(encoding="utf-8") + + +def test_install_recovers_stale_npm_global_directory( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + manager = _manager(tmp_path) + _write_cache(manager._cache_path("harness"), {"meta": {"updated": "2026-04-16"}, "clis": []}) + _write_cache(manager._cache_path("public"), {"meta": {"updated": "2026-04-18"}, "clis": []}) + _write_cache( + manager._cache_path("extensions"), + { + "meta": {"updated": "2026-05-29"}, + "clis": [ + { + "name": "hyperframes", + "display_name": "HyperFrames", + "package_manager": "npm", + "npm_package": "hyperframes", + "install_cmd": "npm install -g hyperframes", + "entry_point": "hyperframes", + "skill_md": "skills/hyperframes/SKILL.md", + } + ], + }, + ) + npm = str(tmp_path / "bin" / "npm") + global_root = tmp_path / "global" + stale_package = global_root / "hyperframes" + stale_temp = global_root / ".hyperframes-broken" + stale_package.mkdir(parents=True) + stale_temp.mkdir() + install_attempts = 0 + + def fake_run(argv: list[str], *, timeout: int) -> subprocess.CompletedProcess[str]: + nonlocal install_attempts + if argv == [npm, "root", "-g"]: + return subprocess.CompletedProcess(argv, 0, stdout=str(global_root), stderr="") + if argv == [npm, "install", "-g", "hyperframes"]: + install_attempts += 1 + if install_attempts == 1: + return subprocess.CompletedProcess( + argv, + 1, + stdout="", + stderr="npm error ENOTEMPTY\nnpm error syscall rename", + ) + return subprocess.CompletedProcess(argv, 0, stdout="ok", stderr="") + raise AssertionError(f"unexpected command: {argv}") + + monkeypatch.setattr(manager, "_run_argv", fake_run) + monkeypatch.setattr( + "nanobot.apps.cli.service.shutil.which", + lambda command: npm if command == "npm" else None, + ) + + payload = manager.install("hyperframes") + + assert install_attempts == 2 + assert not stale_package.exists() + assert not stale_temp.exists() + assert payload["last_action"]["ok"] is True + installed = json.loads(manager.installed_path.read_text(encoding="utf-8"))["apps"] + assert installed["hyperframes"]["strategy"] == "npm" + + def test_install_records_entry_point_path_and_pip_distribution( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, diff --git a/webui/src/lib/api.ts b/webui/src/lib/api.ts index 4180d7ecf..14d59d4aa 100644 --- a/webui/src/lib/api.ts +++ b/webui/src/lib/api.ts @@ -40,7 +40,8 @@ async function request( credentials: "same-origin", }); if (!res.ok) { - throw new ApiError(res.status, `HTTP ${res.status}`); + const text = typeof res.text === "function" ? (await res.text()).trim() : ""; + throw new ApiError(res.status, text || `HTTP ${res.status}`); } const contentType = res.headers?.get?.("content-type") ?? ""; if (contentType && !contentType.toLowerCase().includes("application/json")) { diff --git a/webui/src/tests/api.test.ts b/webui/src/tests/api.test.ts index ea7973a2a..e22e672ef 100644 --- a/webui/src/tests/api.test.ts +++ b/webui/src/tests/api.test.ts @@ -134,6 +134,22 @@ describe("webui API helpers", () => { }); }); + it("surfaces API error response bodies", async () => { + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue({ + ok: false, + status: 500, + text: async () => "npm error ENOTEMPTY", + }), + ); + + await expect(runCliAppAction("tok", "install", "hyperframes")).rejects.toMatchObject({ + status: 500, + message: "npm error ENOTEMPTY", + }); + }); + it("serializes provider settings updates without returning secrets", async () => { await updateProviderSettings("tok", { provider: "openrouter",