mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-06-13 14:23:58 +00:00
fix(apps): recover stale npm installs
This commit is contained in:
parent
d7bc1bcfb5
commit
57563b671f
@ -37,6 +37,7 @@ _MAX_TOOL_OUTPUT_CHARS = 12_000
|
|||||||
_MAX_ARTIFACT_SCAN_PATHS = 4_000
|
_MAX_ARTIFACT_SCAN_PATHS = 4_000
|
||||||
_MAX_ARTIFACT_REPORT = 12
|
_MAX_ARTIFACT_REPORT = 12
|
||||||
_SAFE_NAME_RE = re.compile(r"[^a-z0-9_-]+")
|
_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)
|
_MENTION_RE = re.compile(r"(^|[\s([{])@([a-z0-9_-]+)\b", re.IGNORECASE)
|
||||||
_SHELL_META_CHARS = ("|", "&&", "||", ";", "$(", "`", ">", "<")
|
_SHELL_META_CHARS = ("|", "&&", "||", ";", "$(", "`", ">", "<")
|
||||||
_ENDORSEMENT_WORD_RE = re.compile(r"\bofficial\s+", re.IGNORECASE)
|
_ENDORSEMENT_WORD_RE = re.compile(r"\bofficial\s+", re.IGNORECASE)
|
||||||
@ -740,6 +741,45 @@ class CliAppManager:
|
|||||||
return [npm, "install", "-g", package + "@latest"]
|
return [npm, "install", "-g", package + "@latest"]
|
||||||
return [npm, "uninstall", "-g", package]
|
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]:
|
def _split_safe_command(self, app: dict[str, Any], key: str, expected: str) -> list[str]:
|
||||||
command = str(app.get(key) or "")
|
command = str(app.get(key) or "")
|
||||||
if not command:
|
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):
|
if not self._install_supported(app):
|
||||||
raise CliAppError("this CLI app uses an unsupported install strategy")
|
raise CliAppError("this CLI app uses an unsupported install strategy")
|
||||||
strategy = self._strategy(app)
|
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":
|
if strategy == "bundled":
|
||||||
detect_cmd = str(app.get("detect_cmd") or app.get("entry_point") or "")
|
detect_cmd = str(app.get("detect_cmd") or app.get("entry_point") or "")
|
||||||
if detect_cmd and _command_exists(detect_cmd):
|
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")
|
argv = self._argv_for_action(app, "install")
|
||||||
assert argv is not None
|
assert argv is not None
|
||||||
result = self._run_argv(argv, timeout=self.runtime.install_timeout)
|
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:
|
if result.returncode != 0:
|
||||||
raise CliAppError(_truncate(result.stderr or result.stdout or "install failed"), status=500)
|
raise CliAppError(_truncate(result.stderr or result.stdout or "install failed"), status=500)
|
||||||
self._record_installed(app)
|
self._record_installed(app)
|
||||||
|
|||||||
@ -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")
|
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(
|
def test_install_records_entry_point_path_and_pip_distribution(
|
||||||
tmp_path: Path,
|
tmp_path: Path,
|
||||||
monkeypatch: pytest.MonkeyPatch,
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
|
|||||||
@ -40,7 +40,8 @@ async function request<T>(
|
|||||||
credentials: "same-origin",
|
credentials: "same-origin",
|
||||||
});
|
});
|
||||||
if (!res.ok) {
|
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") ?? "";
|
const contentType = res.headers?.get?.("content-type") ?? "";
|
||||||
if (contentType && !contentType.toLowerCase().includes("application/json")) {
|
if (contentType && !contentType.toLowerCase().includes("application/json")) {
|
||||||
|
|||||||
@ -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 () => {
|
it("serializes provider settings updates without returning secrets", async () => {
|
||||||
await updateProviderSettings("tok", {
|
await updateProviderSettings("tok", {
|
||||||
provider: "openrouter",
|
provider: "openrouter",
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user