From 89e9ce86475bdf1818fbe96b233b50c23bedc62e Mon Sep 17 00:00:00 2001 From: can1357 Date: Fri, 19 Jun 2026 17:12:04 +0200 Subject: [PATCH] feat(python/robomp): bootstrapped node_modules in bare worktrees - Added `ensure_workspace_dependencies` to automate `bun install` for new worktrees where dependencies are missing. - Configured the installer to use `--frozen-lockfile` and `--ignore-scripts` to preserve security and lockfile integrity. - Integrated the bootstrap step into the worker startup process to ensure workspace packages are resolvable. --- python/robomp/src/host_tools.py | 55 +++++++++++++ python/robomp/src/worker.py | 3 + python/robomp/tests/test_host_tools.py | 104 +++++++++++++++++++++++++ 3 files changed, 162 insertions(+) diff --git a/python/robomp/src/host_tools.py b/python/robomp/src/host_tools.py index ccc9c3b30..6c77b43b4 100644 --- a/python/robomp/src/host_tools.py +++ b/python/robomp/src/host_tools.py @@ -43,6 +43,8 @@ from robomp.sandbox import ( log = logging.getLogger(__name__) _PRE_PR_FIX_COMMAND = ("bun", "run", "fix") _PRE_PR_CHECK_COMMAND = ("bun", "check") +_BUN_INSTALL_COMMAND = ("bun", "install", "--frozen-lockfile", "--ignore-scripts") +_BUN_INSTALL_TIMEOUT_SECONDS = 300.0 _REPO_COMMAND_SCRUBBED_ENV_KEYS: tuple[str, ...] = ( "GITHUB_TOKEN", "GITHUB_WEBHOOK_SECRET", @@ -284,6 +286,59 @@ def _format_process_output(stdout: Any, stderr: Any) -> str: ) +def ensure_workspace_dependencies(bindings: ToolBindings) -> None: + """Bootstrap ``node_modules`` so the agent can resolve workspace packages. + + A per-issue worktree is a bare source checkout (``git worktree add`` off + the shared clone pool): it has the repo's ``package.json``/``bun.lock`` but + no ``node_modules``. With bun's ``hoisted`` linker the workspace links + (``@oh-my-pi/pi-*``) only exist after an install, so without one any + ``bun test``/``bun check`` the agent runs fails instantly with "Cannot find + package" — the agent then reports it could not verify. We install before + the agent starts, mirroring how the natives cache pre-populates ``.node`` + artifacts. The links resolve into *this* worktree's ``packages/*`` (not the + orchestrator's read-only ``/work/pi``), so tests exercise the PR's edited + source. + + ``--frozen-lockfile`` keeps the lockfile pristine (no spurious diff for the + agent to commit) and ``--ignore-scripts`` skips lifecycle scripts so an + untrusted PR's ``postinstall``/``prepare`` cannot execute as the slot and + the cached native build is not redone. Runs with the same scrubbed, + slot-owned env as the other repo-owned bun commands (``bun run fix`` / + ``bun check``). + + Skips non-bun repos. Otherwise runs unconditionally on every launch + (including ``--continue`` resumes): a frozen install verifies an intact + tree in ~20ms and re-links anything missing, so a previous install that + timed out or crashed half-way self-heals instead of being skipped forever + on a mere ``node_modules/`` directory existing. Best-effort: any failure + (offline, or a PR that bumped deps so the frozen lockfile is stale) is + logged and swallowed — the agent can still install itself or report the gap. + """ + repo_dir = bindings.workspace.repo_dir + if not (repo_dir / "package.json").is_file() or not (repo_dir / "bun.lock").is_file(): + return + try: + proc = _run_repo_command(bindings, _BUN_INSTALL_COMMAND, timeout=_BUN_INSTALL_TIMEOUT_SECONDS) + except FileNotFoundError: + log.warning("bun_install bootstrap skipped: bun not on PATH", extra={"issue": bindings.issue_key}) + return + except (OSError, subprocess.SubprocessError) as exc: + log.warning("bun_install bootstrap failed", extra={"issue": bindings.issue_key, "err": str(exc)}) + return + if proc.returncode != 0: + log.warning( + "bun_install bootstrap nonzero exit", + extra={ + "issue": bindings.issue_key, + "code": proc.returncode, + "output": _format_process_output(proc.stdout, proc.stderr), + }, + ) + return + log.info("bun_install bootstrap ok", extra={"issue": bindings.issue_key}) + + def _run_pre_publish_bun_fix( bindings: ToolBindings, args: Mapping[str, Any], diff --git a/python/robomp/src/worker.py b/python/robomp/src/worker.py index 2e281bfae..af05ff7fd 100644 --- a/python/robomp/src/worker.py +++ b/python/robomp/src/worker.py @@ -484,6 +484,9 @@ def _run_rpc_blocking( rpc_env.update(_prepare_slot_runtime_env(inputs.workspace, inputs.slot_uid)) rpc_env.update(_safe_directory_env(bindings.workspace.repo_dir)) rpc_env.update(_git_identity_env(inputs.settings.resolved_author_name, inputs.settings.git_author_email)) + # Bare worktrees have no node_modules; install (idempotently) so the agent + # can resolve workspace packages (@oh-my-pi/pi-*) and actually run tests. + host_tools.ensure_workspace_dependencies(bindings) resuming = _has_prior_session(bindings.workspace.session_dir) extra_args: tuple[str, ...] = ("--continue",) if resuming else () log.info( diff --git a/python/robomp/tests/test_host_tools.py b/python/robomp/tests/test_host_tools.py index cbfb45363..590364ec7 100644 --- a/python/robomp/tests/test_host_tools.py +++ b/python/robomp/tests/test_host_tools.py @@ -172,6 +172,110 @@ def test_run_repo_command_uses_slot_identity_kwargs( assert kwargs["env"]["BUN_INSTALL_CACHE_DIR"].endswith("/.omp-xdg/cache/bun-install") +def _write_bun_repo(repo_dir: Path) -> None: + repo_dir.mkdir(parents=True, exist_ok=True) + (repo_dir / "package.json").write_text('{"name":"x"}', encoding="utf-8") + (repo_dir / "bun.lock").write_text("{}", encoding="utf-8") + + +def test_ensure_workspace_dependencies_installs_when_missing( + db: Database, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + import subprocess + + bindings, loop, thread = _bindings(db, tmp_path, httpx.MockTransport(lambda _r: httpx.Response(500))) + _write_bun_repo(bindings.workspace.repo_dir) + captured: list[tuple[str, ...]] = [] + + def fake_run_repo_command( + _b: ToolBindings, cmd: list[str] | tuple[str, ...], *, timeout: float | None = None + ) -> subprocess.CompletedProcess[str]: + captured.append(tuple(cmd)) + return subprocess.CompletedProcess(list(cmd), 0, "449 packages installed", "") + + monkeypatch.setattr(host_tools, "_run_repo_command", fake_run_repo_command) + try: + host_tools.ensure_workspace_dependencies(bindings) + finally: + _stop_loop(loop, thread) + + assert captured == [("bun", "install", "--frozen-lockfile", "--ignore-scripts")] + + +def test_ensure_workspace_dependencies_reinstalls_when_node_modules_present( + db: Database, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + # A bare node_modules/ dir is NOT a "fully installed" sentinel: a prior + # install that timed out or crashed half-way leaves a partial tree. The + # frozen install must still run so bun re-links anything missing, instead + # of skipping forever and leaving the resolver broken. + import subprocess + + bindings, loop, thread = _bindings(db, tmp_path, httpx.MockTransport(lambda _r: httpx.Response(500))) + _write_bun_repo(bindings.workspace.repo_dir) + (bindings.workspace.repo_dir / "node_modules").mkdir() + captured: list[tuple[str, ...]] = [] + + def fake_run_repo_command( + _b: ToolBindings, cmd: list[str] | tuple[str, ...], *, timeout: float | None = None + ) -> subprocess.CompletedProcess[str]: + captured.append(tuple(cmd)) + return subprocess.CompletedProcess(list(cmd), 0, "Checked 449 packages", "") + + monkeypatch.setattr(host_tools, "_run_repo_command", fake_run_repo_command) + try: + host_tools.ensure_workspace_dependencies(bindings) + finally: + _stop_loop(loop, thread) + + assert captured == [("bun", "install", "--frozen-lockfile", "--ignore-scripts")] + + +def test_ensure_workspace_dependencies_skips_non_bun_repo( + db: Database, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + # repo_dir exists (created by _stub_workspace) but has no package.json/bun.lock. + bindings, loop, thread = _bindings(db, tmp_path, httpx.MockTransport(lambda _r: httpx.Response(500))) + called = False + + def fail_run(*_a: Any, **_k: Any) -> Any: + nonlocal called + called = True + raise AssertionError("must not install in a non-bun repo") + + monkeypatch.setattr(host_tools, "_run_repo_command", fail_run) + try: + host_tools.ensure_workspace_dependencies(bindings) + finally: + _stop_loop(loop, thread) + + assert called is False + + +def test_ensure_workspace_dependencies_swallows_install_failure( + db: Database, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + import subprocess + + bindings, loop, thread = _bindings(db, tmp_path, httpx.MockTransport(lambda _r: httpx.Response(500))) + _write_bun_repo(bindings.workspace.repo_dir) + + def failing_run( + _b: ToolBindings, cmd: list[str] | tuple[str, ...], *, timeout: float | None = None + ) -> subprocess.CompletedProcess[str]: + return subprocess.CompletedProcess(list(cmd), 1, "", "lockfile out of date") + + monkeypatch.setattr(host_tools, "_run_repo_command", failing_run) + try: + # A stale frozen lockfile (e.g. a PR that bumped deps) must not raise — + # the agent can still install itself or report the gap. + host_tools.ensure_workspace_dependencies(bindings) + finally: + _stop_loop(loop, thread) + + assert not (bindings.workspace.repo_dir / "node_modules").exists() + + def test_guarded_push_branch_rev_parse_runs_via_repo_command_and_passes_slot_uid( db: Database, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: