diff --git a/install.sh b/install.sh index 923c4ad4d82..26f483f6d9a 100755 --- a/install.sh +++ b/install.sh @@ -509,6 +509,12 @@ if wait $!; then die "Package installed but dependencies missing (aiohttp not importable). Try manually: $_venv/bin/pip install -e $KIROCREW_APP_DIR" fi + if [ ! -x "$_venv/bin/kirocrew" ]; then + die "Install incomplete: entry point $_venv/bin/kirocrew missing or not executable." + fi + if ! "$_venv/bin/python" -I -c "import kiro_crew" 2>/dev/null; then + die "Install incomplete: kiro_crew not importable." + fi else if [ -s "$_pip_log" ]; then echo "" diff --git a/src/kiro_crew/dep_sync.py b/src/kiro_crew/dep_sync.py index a7cd221bd71..243361d416b 100644 --- a/src/kiro_crew/dep_sync.py +++ b/src/kiro_crew/dep_sync.py @@ -106,6 +106,66 @@ #: silently tolerated. _SCRIPT = "kirocrew" + +def console_script_path(target_py: Path) -> Path: + """The console-script file *target_py*'s venv would install for ``kirocrew``. + + Platform-aware, matching the same ``sys.platform`` split ``locked_console_scripts`` + uses: on Windows the entry point is ``Scripts\\kirocrew.exe`` beside the + interpreter; on POSIX it is ``bin/kirocrew``. ``Path.with_name`` keeps it in the + interpreter's own directory (``bin`` or ``Scripts``) without hardcoding either. + A pure path computation -- no filesystem or subprocess -- so callers can stat it + cheaply. Avoids the POSIX-only ``bin/kirocrew`` assumption in a module that + exists precisely for the Windows locked-``Scripts\\kirocrew.exe`` case. + """ + + if sys.platform == "win32": + return Path(target_py).with_name(f"{_SCRIPT}.exe") + return Path(target_py).with_name(_SCRIPT) + + +def project_venv_python(repo: Path) -> Path: + """The interpreter path for this project's managed ``.venv``. + + Keep the gateway repair target and the dependency sync's ownership exception + on one platform-aware calculation. The exception is safe only for this exact + lexical path; resolving it would erase a POSIX venv's symlink identity. + """ + if sys.platform == "win32": + return Path(repo) / ".venv" / "Scripts" / "python.exe" + return Path(repo) / ".venv" / "bin" / "python" + + +def _is_redirecting_directory(path: Path) -> bool: + """Return whether *path* redirects writes outside its lexical directory.""" + try: + if path.is_symlink(): + return True + is_junction = getattr(path, "is_junction", None) + return bool(is_junction and is_junction()) + except OSError: + # An ownership exception must fail closed when its layout cannot be read. + return True + + +def _is_owned_project_venv_target(repo: Path, target_py: Path) -> bool: + """Verify the narrow filesystem target eligible for missing-package repair. + + The final interpreter is deliberately allowed to be a symlink: POSIX + ``python -m venv`` commonly creates it that way. The directories that contain + pip's writes are not; redirecting either ``.venv`` or ``bin``/``Scripts`` + would turn the lexical path equality below into authority over another tree. + """ + target = os.path.normcase(os.path.abspath(target_py)) + expected_py = project_venv_python(repo) + expected = os.path.normcase(os.path.abspath(expected_py)) + if target != expected: + return False + return not any( + _is_redirecting_directory(path) for path in (Path(repo) / ".venv", expected_py.parent) + ) + + #: This project's own distribution name, normalized. Asking pip for it is the one #: request that would rewrite the locked console script. _PROJECT = "kirocrew" @@ -420,9 +480,13 @@ def _probe_interpreter( ) -def interpreter_version(target_py: Path) -> tuple[int, int, int] | None: +def interpreter_version( + target_py: Path, timeout: float | None = None +) -> tuple[int, int, int] | None: """``(major, minor, micro)`` of *target_py*, or ``None`` if it cannot be asked.""" - proc = _probe_interpreter(target_py, "import sys;print('%d.%d.%d' % sys.version_info[:3])") + proc = _probe_interpreter( + target_py, "import sys;print('%d.%d.%d' % sys.version_info[:3])", timeout=timeout + ) if proc.returncode != 0: return None try: @@ -886,6 +950,8 @@ def sync_or_reinstall( target_py: Path, emit: Emit = _print_emit, timeout: float | None = None, + *, + allow_missing_package_repair: bool = False, ) -> int: """Bring ``target_py``'s venv up to date with ``repo``. 0 when it succeeded. @@ -910,6 +976,13 @@ def sync_or_reinstall( anywhere a user can see -- a dashboard progress feed, a log -- owns redacting it first: pip echoes index URLs, which carry credentials when the operator configured an authenticated index. + + ``allow_missing_package_repair`` is the gateway's narrow recovery contract for + an interrupted rebuild: the package may be absent only when ``target_py`` is + exactly ``/.venv``'s platform interpreter and that interpreter can run. + A foreign origin is never allowed, and an absent package at any other target + remains unproven and refused. This keeps the general ownership guard fail-closed + while allowing the half-built state this repair path exists to recover. """ # Establish that the venv about to be written to serves THIS checkout BEFORE # the branch, so both paths are covered. `sync()` asks the same question again @@ -920,9 +993,22 @@ def sync_or_reinstall( # # Without this, the reinstall branch would repeat the exact asymmetry this # change fixes at the Dev Fleet endpoint -- a guarded substitute beside an - # unguarded reinstall -- and three of this function's four callers take the - # checkout from configuration, so a repointed venv is reachable on all three. - foreign = venv_not_mapped_to(installed_package_origin(target_py), repo) + # unguarded reinstall -- and four of this function's five callers take the + # checkout from configuration, so a repointed venv is reachable on all four. + origin = installed_package_origin(target_py) + repairing_missing_package = False + if ( + allow_missing_package_repair + and origin is None + and _is_owned_project_venv_target(repo, target_py) + ): + try: + repairing_missing_package = interpreter_version(target_py, timeout=timeout) is not None + except (OSError, subprocess.TimeoutExpired): + # ``None`` can also mean the interpreter vanished or wedged. That + # is not the absent-package state this exception is allowed for. + repairing_missing_package = False + foreign = None if repairing_missing_package else venv_not_mapped_to(origin, repo) if foreign: return _refuse( emit, @@ -985,13 +1071,49 @@ def sync_or_reinstall( # 1, not pip's own code, for the same reason as the substitute branch: # pip's 2 (UNKNOWN_ERROR) is REFUSED's value, and this install ran. return 1 + # A full editable reinstall has one success contract: pip returned 0 AND the + # artifacts it promises actually landed. The locked-script branch returned + # through ``sync`` above, so this postcondition never asks a dependency-only + # substitute to rewrite the wrapper it deliberately leaves alone. Keeping the + # check here closes every full-reinstall caller, including auto-update -- the + # path whose interruption can create the half-built venv this guard repairs. + script = console_script_path(target_py) + if not os.access(script, os.X_OK): + emit( + f"dep-sync: pip install -e returned 0 but the {_SCRIPT!r} console " + f"script at {script} is missing or not executable", + True, + ) + return 1 + try: + importable = _probe_interpreter(target_py, "import kiro_crew", timeout=timeout) + except subprocess.TimeoutExpired: + emit("dep-sync: kiro_crew import check timed out after the pip install", True) + return 1 + if importable.returncode != 0: + emit( + "dep-sync: pip install -e returned 0 but kiro_crew is not importable " + "in the target venv", + True, + ) + return 1 return 0 def main(argv: list[str] | None = None) -> int: args = list(sys.argv[1:] if argv is None else argv) + if len(args) == 3 and args[0] == "--repair-missing-package": + return sync_or_reinstall( + Path(args[1]), + Path(args[2]), + allow_missing_package_repair=True, + ) if len(args) != 2: - print("usage: dep_sync ", file=sys.stderr) + print( + "usage: dep_sync | " + "dep_sync --repair-missing-package ", + file=sys.stderr, + ) return REFUSED return sync(Path(args[0]), Path(args[1])) diff --git a/src/kiro_crew/instances/token_mint.py b/src/kiro_crew/instances/token_mint.py index 9a3b5381ee2..93ccf89ed5b 100644 --- a/src/kiro_crew/instances/token_mint.py +++ b/src/kiro_crew/instances/token_mint.py @@ -203,6 +203,27 @@ def build_candidate_command( " fi;", "done;", f'echo "kirocrew binary not found in any of: {", ".join(candidates)}" >&2;', + 'echo "candidate diagnosis:" >&2;', + f"for b in {expanded}; do", + ' if [ -L "$b" ]; then', + ' __t=$(readlink -f "$b" 2>/dev/null);', + ' if [ -z "$__t" ] || [ ! -e "$__t" ]; then', + ' echo " $b: DANGLING symlink -> $(readlink "$b" 2>/dev/null) (target missing)" >&2;', + " else", + ' echo " $b: symlink -> $__t (not executable)" >&2;', + " fi;", + ' elif [ ! -e "$b" ]; then', + ' echo " $b: absent" >&2;', + " else", + ' echo " $b: present, NOT executable" >&2;', + " fi;", + ' case "$b" in', + " */.venv/bin/*)", + ' __v="${b%/bin/*}";', + ' if [ -x "$__v/bin/python" ]; then echo " $__v/bin/python present" >&2; else echo " $__v/bin/python MISSING" >&2; fi;', + " ;;", + " esac;", + "done;", "exit 127", ] ) diff --git a/src/kiro_crew/slack/gateway.py b/src/kiro_crew/slack/gateway.py index 35bd724a2e5..95dd067bf6e 100644 --- a/src/kiro_crew/slack/gateway.py +++ b/src/kiro_crew/slack/gateway.py @@ -262,7 +262,14 @@ ) from kiro_crew.providers.base import LLMEvent from kiro_crew.safety_override import flush_breadcrumb_writes, safety_override -from kiro_crew.sandbox import ensure_agents_slice_limits, warm_backend +from kiro_crew.sandbox import ( + SandboxUnavailableError, + create_subprocess_limited, + ensure_agents_slice_limits, + sandboxed_spawn_argv, + sandboxed_spawn_argv_async, + warm_backend, +) from kiro_crew.security import ( redact, redact_and_truncate, @@ -1743,6 +1750,10 @@ def __init__( self.channel_history: ChannelHistory | None = None self.dashboard_state: DashboardState | None = None self._background_tasks: set[asyncio.Task] = set() # prevent GC of fire-and-forget tasks + # Dedicated ownership for the repair's dep_sync/pip process tree. The + # general set only prevents task GC; shutdown must cancel and await this + # task so _check_console_script can kill and reap its child group. + self._console_script_repair_task: "asyncio.Task[None] | None" = None self._marker_write_task: "asyncio.Task[None] | None" = None # Set by the shutdown path when the marker write is still in flight: # tells the writer thread to self-clear after publishing, closing the @@ -2494,6 +2505,163 @@ async def _check_missing_deps(self) -> None: dep_err, _ = redact_credentials(dep_err) logger.error("Dep repair failed: %s", dep_err[:500]) + async def _check_console_script(self) -> None: + """Repair a venv whose ``kirocrew`` console script went missing. + + ``_check_missing_deps`` catches a git-reset-without-pip-install that left + an import missing, but not the failure mode where an interrupted venv + rebuild (e.g. a Python-version bump that reran ``python -m venv`` + a + killed ``pip install -e``) leaves a venv with a working interpreter but + no ``kirocrew`` entry point — the gateway then dies later with an + exit-127 "binary not found". This closes that gap at startup: if the + recorded pip install has no executable console script, run the same + in-place editable reinstall ``dep_sync`` uses, which is the one operation + that rewrites the entry point. + + Scoped to pip installs of a real project dir: a Brazil install owns its + own entry point, and an empty ``KIROCREW_PROJECT_DIR`` means there is no + checkout to reinstall from. + """ + proj = os.environ.get("KIROCREW_PROJECT_DIR", "") + if not proj or self._is_brazil_install(proj): + return + method_file = Path(proj) / ".install-method" + if not (method_file.is_file() and method_file.read_text().strip() == "pip"): + return + # The venv interpreter under the project, platform-aware (Scripts on + # Windows, bin on POSIX), shared with dep_sync's ownership exception. + venv_py = dep_sync.project_venv_python(Path(proj)) + script = dep_sync.console_script_path(venv_py) + if script.exists() and os.access(script, os.X_OK): + return + + logger.warning( + "kirocrew console script missing/not executable at %s — reinstalling", script + ) + print("👻 Repairing kirocrew install (console script missing)…") + # Run the stdlib-only module by absolute file path: the target venv may + # not currently contain an importable kiro_crew package. A dedicated + # child session lets cancellation own pip and its build descendants. + dep_sync_file = dep_sync.__file__ + if dep_sync_file is None: + raise RuntimeError("dep_sync module has no source path") + # Route through the sandbox chokepoint: this child EXECUTES + # ``venv_py`` (dep_sync probes the target interpreter via + # ``installed_package_origin``), and that interpreter lives in the + # project checkout, so its bytes are not ours to trust. The sandbox + # gives the whole repair subtree filesystem isolation plus a + # credential-scrubbed env, and ``create_subprocess_limited`` adds the + # kernel resource ceiling. ``mode="strict"`` hides the credential dirs + # AND ``.ssh`` while the untrusted interpreter runs -- the tightest + # tier, chosen because the child executes bytes we do not trust. It + # still leaves the project and its venv writable (pip must rewrite the + # entry point) and the network open (pip must reach the index); + # ``scrub_env`` is mode-independent, so ``PIP_INDEX_URL`` / proxy / SSL + # vars survive. ``strip_python_env`` stops an inherited PYTHONPATH from + # satisfying the child's import probe from outside the venv. No + # ``extra_writable_dirs``: the project is already writable, and a + # carve-out outside the sealed runtime parent would be refused anyway. + try: + argv, env, cleanup = await sandboxed_spawn_argv_async( + [ + sys.executable, + str(Path(dep_sync_file).resolve()), + "--repair-missing-package", + str(proj), + str(venv_py), + ], + mode="strict", + env=os.environ.copy(), + strip_python_env=True, + _prepare=sandboxed_spawn_argv, + ) + except SandboxUnavailableError as exc: + # Fail CLOSED. Running an untrusted interpreter unsandboxed is the + # exposure this routing exists to remove, so a host with no sandbox + # backend does not get the repair -- it gets a named reason instead + # of a silent no-op, leaving the pre-existing manual path. The typed + # ``kind``/``detail`` are logged rather than an inferred English + # guess: this PR exists to make this failure class diagnosable. + print("❌ kirocrew reinstall skipped (no sandbox available) — run: kirocrew update") + logger.error( + "Console-script repair skipped: sandbox unavailable (kind=%s): %s — " + "refusing to run the project venv interpreter unsandboxed", + exc.kind, + exc.detail, + ) + return + try: + proc = await create_subprocess_limited( + *argv, + cwd=proj, + env=env, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + start_new_session=platform_compat.IS_POSIX, + ) + try: + stdout, stderr = await asyncio.wait_for( + proc.communicate(), timeout=self._DEP_INSTALL_TIMEOUT_SECS + ) + except (TimeoutError, asyncio.TimeoutError): + await self._kill_startup_child(proc) + await self._reap_startup_child(proc) + print("❌ kirocrew reinstall timed out — run manually: kirocrew update") + logger.error( + "Console-script reinstall timed out after %.0fs", + self._DEP_INSTALL_TIMEOUT_SECS, + ) + return + except asyncio.CancelledError: + await self._kill_startup_child(proc) + await self._reap_startup_child(proc) + raise + finally: + if cleanup: + Path(cleanup).unlink(missing_ok=True) + + if proc.returncode != 0: + detail = b"\n".join(part for part in (stdout, stderr) if part).decode( + "utf-8", errors="replace" + ) + detail, _ = redact_exfiltration_urls(detail) + detail, _ = redact_credentials(detail) + print("❌ kirocrew reinstall failed — run manually: kirocrew update") + logger.error("Console-script reinstall failed: %s", detail[:500]) + else: + print("✅ kirocrew console script restored") + + def _schedule_console_script_repair(self) -> asyncio.Task[None]: + """Run the console-script repair after the HTTP socket has bound. + + The healthy path is a few filesystem probes, but repair can spend the + full pip timeout in its owned child process. Keep a strong reference so + the task is observable and shutdown cancellation reaches that child. + """ + existing = self._console_script_repair_task + if existing is not None and not existing.done(): + return existing + + async def _repair() -> None: + try: + await self._check_console_script() + except asyncio.CancelledError: + raise + except Exception: + logger.warning("Console-script check failed", exc_info=True) + + task = asyncio.create_task(_repair()) + self._console_script_repair_task = task + self._background_tasks.add(task) + + def _clear(done: asyncio.Task[None]) -> None: + self._background_tasks.discard(done) + if self._console_script_repair_task is done: + self._console_script_repair_task = None + + task.add_done_callback(_clear) + return task + # ------------------------------------------------------------------ # Service initialisation # ------------------------------------------------------------------ @@ -9327,6 +9495,15 @@ async def _shutdown(self) -> None: logger.debug("Dashboard slot save before shutdown failed", exc_info=True) self.dashboard_state.file_indexes.stop_all() + # The general _background_tasks set is retention, not lifecycle ownership. + # This task can own a dep_sync child plus pip/build descendants, so cancel + # and await it explicitly while _check_console_script still has a live loop + # on which to kill the process tree and perform its bounded reap. + repair_task = self._console_script_repair_task + if repair_task is not None and not repair_task.done(): + repair_task.cancel() + await asyncio.gather(repair_task, return_exceptions=True) + # Cancel in-flight handler tasks for t in list(self._handler_tasks): t.cancel() @@ -10965,6 +11142,12 @@ async def _backfill_unclean_session_telemetry() -> None: self._init_crew() else: await self._init_api_server() + + # The dashboard/API socket is bound now. A missing wrapper can take the + # full pip timeout to repair, so track that work without delaying READY. + # The task itself catches and logs failures; startup remains available. + self._schedule_console_script_repair() + # Record this gateway's own kirocrew launcher, keyed by the port it # serves, so a remote token-mint execs THIS install's venv instead of # a stale ~/.local/bin/kirocrew that may point at an uninstalled diff --git a/test/test_bootstrap.py b/test/test_bootstrap.py index 1b8188fad1d..60b784f78e9 100644 --- a/test/test_bootstrap.py +++ b/test/test_bootstrap.py @@ -174,18 +174,26 @@ def _import(): def test_self_heal_runs_fixed_pip_argv(monkeypatch, tmp_path): - """Where pip CAN rewrite the script, the heal is still the full reinstall.""" + """A full reinstall heals dependencies and satisfies its artifact contract.""" from kiro_crew import dep_sync - monkeypatch.setattr(_bootstrap.sys, "platform", "linux") # POSIX heal path + monkeypatch.setattr(_bootstrap.sys, "platform", "linux") monkeypatch.setattr(_bootstrap, "_source_checkout_root", lambda: tmp_path) _venv_maps(monkeypatch) monkeypatch.setattr(dep_sync, "locked_console_scripts", lambda target: []) + script = tmp_path / "venv" / "bin" / "kirocrew" + monkeypatch.setattr(dep_sync, "console_script_path", lambda target: script) seen: dict = {} def _fake_run(argv, **kwargs): - seen["argv"] = argv - seen["timeout"] = kwargs.get("timeout") + if argv[1:3] == ["-m", "pip"]: + seen["argv"] = argv + seen["timeout"] = kwargs.get("timeout") + script.parent.mkdir(parents=True) + script.write_text("#!/bin/sh\n") + script.chmod(0o755) + elif argv[1:] == ["-I", "-X", "utf8", "-c", "import kiro_crew"]: + seen["import_checked"] = True class _P: returncode = 0 @@ -198,6 +206,7 @@ class _P: assert _bootstrap._self_heal("defusedxml") is True assert seen["argv"][1:] == ["-m", "pip", "install", "-e", str(tmp_path), "--quiet"] assert seen["timeout"] == _bootstrap._PIP_TIMEOUT_SECS + assert seen["import_checked"] is True def test_self_heal_reports_pip_failure(monkeypatch, tmp_path): diff --git a/test/test_cli_server_more_coverage.py b/test/test_cli_server_more_coverage.py index 82c6204eafb..a6801218fe4 100644 --- a/test/test_cli_server_more_coverage.py +++ b/test/test_cli_server_more_coverage.py @@ -996,6 +996,10 @@ def __call__(self, argv, **kw): return subprocess.CompletedProcess(argv, self.rc.get("reset", 0), "", "dirty") if argv[0] == "kiro-cli": return subprocess.CompletedProcess(argv, 0, "", "") + if argv[1:] == ["-I", "-X", "utf8", "-c", "import kiro_crew"]: + # The full-reinstall success contract probes the target interpreter + # in isolation from the caller's CWD and PYTHONPATH. + return subprocess.CompletedProcess(argv, 0, b"", b"") if "pip" in argv: # BYTES, like the real call: the install captures without text=True so # a non-UTF-8 console cannot make pip's own error message undecodable. @@ -1036,6 +1040,12 @@ def git_checkout(monkeypatch, tmp_path): # they assert. `kirocrew update`'s own substitute behaviour is covered in # test/test_dep_sync.py. monkeypatch.setattr(cli_server.dep_sync, "locked_console_scripts", lambda target: []) + # A successful fake pip install must satisfy the shared artifact postcondition. + # The test interpreter is a real executable on every supported platform; + # dep_sync's own tests cover the path calculation and missing-script failure. + monkeypatch.setattr( + cli_server.dep_sync, "console_script_path", lambda target: Path(sys.executable) + ) # And the foreign-venv guard, which now runs before either install branch. # Its probe RUNS the target interpreter, which _GitStub intercepts into an # empty answer — read as "cannot be shown to serve this checkout" and refused. diff --git a/test/test_dep_sync.py b/test/test_dep_sync.py index 99809bbe619..e4c841073ec 100644 --- a/test/test_dep_sync.py +++ b/test/test_dep_sync.py @@ -595,6 +595,20 @@ class _Proc: assert dep_sync.main([str(repo), "py"]) == 0 +def test_main_explicit_missing_package_repair_mode(repo): + target_py = dep_sync.project_venv_python(repo) + + with patch.object(dep_sync, "sync_or_reinstall", return_value=0) as repair: + rc = dep_sync.main(["--repair-missing-package", str(repo), str(target_py)]) + + assert rc == 0 + repair.assert_called_once_with( + repo, + target_py, + allow_missing_package_repair=True, + ) + + def test_main_rejects_a_wrong_argument_count(): assert dep_sync.main(["only-one"]) == 2 assert dep_sync.main(["a", "b", "c"]) == 2 @@ -700,8 +714,11 @@ def test_sync_or_reinstall_prefers_the_reinstall_when_nothing_is_locked(tmp_path seen = {} def fake_run(argv, **kwargs): - seen["argv"] = argv - seen["timeout"] = kwargs.get("timeout") + # Record the pip install call specifically; the post-install import probe + # is a second subprocess.run and must not clobber what we assert on. + if argv[1:3] == ["-m", "pip"]: + seen["argv"] = argv + seen["timeout"] = kwargs.get("timeout") return SimpleNamespace(returncode=0, stdout=b"", stderr=b"") with ( @@ -710,6 +727,10 @@ def fake_run(argv, **kwargs): patch.object(dep_sync, "locked_console_scripts", return_value=[]), patch.object(dep_sync, "sync", side_effect=AssertionError("must not substitute")), patch.object(dep_sync.subprocess, "run", side_effect=fake_run), + # Post-install verification: the console script is present+executable and + # the package imports. Stubbed here so this test pins the "prefer the + # reinstall" contract, not the verification (which has its own tests). + patch.object(dep_sync.os, "access", return_value=True), ): rc = dep_sync.sync_or_reinstall(tmp_path, Path("/venv/bin/python"), timeout=42) @@ -718,11 +739,338 @@ def fake_run(argv, **kwargs): assert seen["timeout"] == 42 +def test_sync_or_reinstall_fails_when_entry_point_missing_after_pip_ok(tmp_path): + """A full reinstall is not successful until its executable entry point exists. + + This is the interrupted-venv-rebuild incident: the subprocess can report 0 + while the ``kirocrew`` artifact is absent or unusable. Every full-reinstall + caller gets this postcondition; the locked dependency-only branch returns + before it because it deliberately cannot rewrite the running wrapper. + """ + messages = [] + + def fake_run(argv, **kwargs): + return SimpleNamespace(returncode=0, stdout=b"", stderr=b"") + + with ( + _origin_stub(), + _maps(), + patch.object(dep_sync, "locked_console_scripts", return_value=[]), + patch.object(dep_sync.subprocess, "run", side_effect=fake_run), + patch.object(dep_sync.os, "access", return_value=False), + ): + rc = dep_sync.sync_or_reinstall( + tmp_path, + Path("/venv/bin/python"), + lambda m, e: messages.append((m, e)), + ) + + assert rc == 1 + joined = " ".join(m for m, _ in messages) + assert "console" in joined and "kirocrew" in joined + + +def test_sync_or_reinstall_fails_when_package_unimportable_after_pip_ok(tmp_path): + """The isolated target-venv probe must reject an unimportable package.""" + messages = [] + + with ( + _origin_stub(), + _maps(), + patch.object(dep_sync, "locked_console_scripts", return_value=[]), + patch.object( + dep_sync.subprocess, + "run", + return_value=SimpleNamespace(returncode=0, stdout=b"", stderr=b""), + ), + patch.object( + dep_sync, + "_probe_interpreter", + return_value=SimpleNamespace(returncode=1, stdout="", stderr="ModuleNotFoundError"), + ) as import_probe, + patch.object(dep_sync.os, "access", return_value=True), + ): + rc = dep_sync.sync_or_reinstall( + tmp_path, + Path("/venv/bin/python"), + lambda m, e: messages.append((m, e)), + ) + + assert rc == 1 + joined = " ".join(m for m, _ in messages) + assert "importable" in joined + # _probe_interpreter owns the -I, neutral-CWD, and PYTHONPATH isolation + # contract. Calling it here prevents a source checkout in the parent process + # from satisfying a postcondition about the target venv. + import_probe.assert_called_once_with(Path("/venv/bin/python"), "import kiro_crew", timeout=None) + + +def test_sync_or_reinstall_fails_when_import_probe_times_out(tmp_path): + """The post-install import probe is bounded by the caller's timeout.""" + messages = [] + + with ( + _origin_stub(), + _maps(), + patch.object(dep_sync, "locked_console_scripts", return_value=[]), + patch.object( + dep_sync.subprocess, + "run", + return_value=SimpleNamespace(returncode=0, stdout=b"", stderr=b""), + ), + patch.object( + dep_sync, + "_probe_interpreter", + side_effect=subprocess.TimeoutExpired(cmd=["python"], timeout=7), + ) as import_probe, + patch.object(dep_sync.os, "access", return_value=True), + ): + rc = dep_sync.sync_or_reinstall( + tmp_path, + Path("/venv/bin/python"), + lambda m, e: messages.append((m, e)), + timeout=7, + ) + + assert rc == 1 + assert any("import check timed out" in m for m, _ in messages) + import_probe.assert_called_once_with(Path("/venv/bin/python"), "import kiro_crew", timeout=7) + + +def test_sync_or_reinstall_repairs_absent_package_in_verified_project_venv(tmp_path): + """The gateway can recover the exact half-built venv it owns. + + An interrupted venv rebuild can leave a runnable ``/.venv`` before the + package or wrapper lands. The ordinary ownership guard must stay fail-closed; + only explicit repair intent plus the exact managed path and a runnable + interpreter admit this absent-origin state. + """ + target_py = dep_sync.project_venv_python(tmp_path) + + with ( + patch.object(dep_sync, "installed_package_origin", return_value=None), + patch.object(dep_sync, "interpreter_version", return_value=(3, 12, 0)) as version, + patch.object(dep_sync, "locked_console_scripts", return_value=[]), + patch.object( + dep_sync.subprocess, + "run", + return_value=SimpleNamespace(returncode=0, stdout=b"", stderr=b""), + ) as pip_run, + patch.object( + dep_sync, + "_probe_interpreter", + return_value=SimpleNamespace(returncode=0, stdout="", stderr=""), + ), + patch.object(dep_sync.os, "access", return_value=True), + ): + rc = dep_sync.sync_or_reinstall( + tmp_path, + target_py, + timeout=42, + allow_missing_package_repair=True, + ) + + assert rc == 0 + version.assert_called_once_with(target_py, timeout=42) + assert pip_run.call_args.args[0][1:3] == ["-m", "pip"] + + +def test_sync_or_reinstall_still_refuses_absent_package_without_repair_intent( + tmp_path, +): + """A location alone does not prove ownership for ordinary callers.""" + with ( + patch.object(dep_sync, "installed_package_origin", return_value=None), + patch.object( + dep_sync, + "locked_console_scripts", + side_effect=AssertionError("must refuse before lock probing"), + ), + patch.object( + dep_sync.subprocess, + "run", + side_effect=AssertionError("must not install"), + ), + ): + rc = dep_sync.sync_or_reinstall(tmp_path, dep_sync.project_venv_python(tmp_path)) + + assert rc == dep_sync.REFUSED + + +def _symlink_or_skip(link: Path, target: Path, *, directory: bool) -> None: + """Create a symlink or skip where the test account cannot create one.""" + try: + link.symlink_to(target, target_is_directory=directory) + except OSError as exc: + pytest.skip(f"symlinks unavailable: {exc}") + + +def test_missing_package_repair_refuses_a_symlinked_project_venv(tmp_path): + """Lexical equality cannot authorize pip writes through a redirected .venv.""" + external_venv = tmp_path / "external-venv" + external_venv.mkdir() + _symlink_or_skip(tmp_path / ".venv", external_venv, directory=True) + target_py = dep_sync.project_venv_python(tmp_path) + + with ( + patch.object(dep_sync, "installed_package_origin", return_value=None), + patch.object( + dep_sync, + "interpreter_version", + side_effect=AssertionError("redirected venv must not be probed as owned"), + ), + patch.object( + dep_sync, + "locked_console_scripts", + side_effect=AssertionError("must refuse before lock probing"), + ), + patch.object( + dep_sync.subprocess, + "run", + side_effect=AssertionError("must not install"), + ), + ): + rc = dep_sync.sync_or_reinstall( + tmp_path, + target_py, + allow_missing_package_repair=True, + ) + + assert rc == dep_sync.REFUSED + + +def test_missing_package_repair_refuses_a_symlinked_scripts_directory(tmp_path): + """Redirecting bin/Scripts is as unsafe as redirecting the whole venv.""" + target_py = dep_sync.project_venv_python(tmp_path) + target_py.parent.parent.mkdir() + external_scripts = tmp_path / "external-scripts" + external_scripts.mkdir() + _symlink_or_skip(target_py.parent, external_scripts, directory=True) + + with ( + patch.object(dep_sync, "installed_package_origin", return_value=None), + patch.object( + dep_sync, + "interpreter_version", + side_effect=AssertionError("redirected scripts dir must not be probed as owned"), + ), + patch.object( + dep_sync, + "locked_console_scripts", + side_effect=AssertionError("must refuse before lock probing"), + ), + patch.object( + dep_sync.subprocess, + "run", + side_effect=AssertionError("must not install"), + ), + ): + rc = dep_sync.sync_or_reinstall( + tmp_path, + target_py, + allow_missing_package_repair=True, + ) + + assert rc == dep_sync.REFUSED + + +def test_missing_package_repair_allows_the_standard_interpreter_symlink(tmp_path): + """POSIX venvs commonly symlink bin/python; only directories redirect writes.""" + target_py = dep_sync.project_venv_python(tmp_path) + target_py.parent.mkdir(parents=True) + base_python = tmp_path / "base-python" + base_python.write_bytes(b"") + _symlink_or_skip(target_py, base_python, directory=False) + + with ( + patch.object(dep_sync, "installed_package_origin", return_value=None), + patch.object(dep_sync, "interpreter_version", return_value=(3, 12, 0)), + patch.object(dep_sync, "locked_console_scripts", return_value=[]), + patch.object( + dep_sync.subprocess, + "run", + return_value=SimpleNamespace(returncode=0, stdout=b"", stderr=b""), + ), + patch.object( + dep_sync, + "_probe_interpreter", + return_value=SimpleNamespace(returncode=0, stdout="", stderr=""), + ), + patch.object(dep_sync.os, "access", return_value=True), + ): + rc = dep_sync.sync_or_reinstall( + tmp_path, + target_py, + allow_missing_package_repair=True, + ) + + assert rc == 0 + + +def test_missing_package_repair_refuses_a_target_outside_project_venv(tmp_path): + """Repair intent cannot turn a configured foreign target into an owned venv.""" + foreign_target = tmp_path / "other-venv" / "bin" / "python" + + with ( + patch.object(dep_sync, "installed_package_origin", return_value=None), + patch.object( + dep_sync, + "interpreter_version", + side_effect=AssertionError("foreign target must not be probed as owned"), + ), + patch.object( + dep_sync, + "locked_console_scripts", + side_effect=AssertionError("must refuse before lock probing"), + ), + patch.object( + dep_sync.subprocess, + "run", + side_effect=AssertionError("must not install"), + ), + ): + rc = dep_sync.sync_or_reinstall( + tmp_path, + foreign_target, + allow_missing_package_repair=True, + ) + + assert rc == dep_sync.REFUSED + + +def test_missing_package_repair_refuses_an_unrunnable_project_venv(tmp_path): + """An exact path is insufficient when the interpreter itself cannot run.""" + target_py = dep_sync.project_venv_python(tmp_path) + + with ( + patch.object(dep_sync, "installed_package_origin", return_value=None), + patch.object(dep_sync, "interpreter_version", return_value=None), + patch.object( + dep_sync, + "locked_console_scripts", + side_effect=AssertionError("must refuse before lock probing"), + ), + patch.object( + dep_sync.subprocess, + "run", + side_effect=AssertionError("must not install"), + ), + ): + rc = dep_sync.sync_or_reinstall( + tmp_path, + target_py, + allow_missing_package_repair=True, + ) + + assert rc == dep_sync.REFUSED + + def test_sync_or_reinstall_substitutes_when_a_script_is_locked(tmp_path): - """A locked script routes to the substitute, and the caller is told why. + """A locked wrapper uses dependency-only sync without reinstall postconditions. - The reinstall must not merely fail here: pip's uninstall is not atomic, so - reaching the locked script means the editable .pth is already gone. + pip cannot atomically replace a running Windows console script. This branch + deliberately leaves that wrapper alone, so neither the entry-point stat nor + the import probe from the full-reinstall success contract may run here. """ messages = [] @@ -731,6 +1079,7 @@ def test_sync_or_reinstall_substitutes_when_a_script_is_locked(tmp_path): _maps(), patch.object(dep_sync, "locked_console_scripts", return_value=[r"C:\v\kirocrew.exe"]), patch.object(dep_sync, "sync", return_value=0) as sync_mock, + patch.object(dep_sync.os, "access", side_effect=AssertionError("must not verify wrapper")), patch.object(dep_sync.subprocess, "run", side_effect=AssertionError("must not reinstall")), ): rc = dep_sync.sync_or_reinstall( @@ -746,7 +1095,7 @@ def test_sync_or_reinstall_guards_the_reinstall_branch_too(tmp_path): """The foreign-venv refusal covers the branch pip can still run. Guarding only the substitute would rebuild, inside this shared function, the - exact asymmetry it was written to remove: three of its four callers take the + exact asymmetry it was written to remove: four of its five callers take the checkout from configuration, so a venv serving a DIFFERENT checkout is reachable on all three, and `pip install -e ` against it silently repoints that other checkout's editable install at this repo. @@ -1111,3 +1460,23 @@ def test_module_imports_stdlib_only(): # rather than breaking the module. third_party = roots - set(sys.stdlib_module_names) - {"tomli"} assert not third_party, f"dep_sync must import stdlib only; found {sorted(third_party)}" + + +def test_console_script_path_is_platform_aware(): + """The console-script path resolves to Scripts\\kirocrew.exe on Windows and + bin/kirocrew on POSIX -- not a hardcoded POSIX layout in a module that exists + for the Windows locked-script case. + """ + posix_py = Path("/home/u/proj/.venv/bin/python") + win_py = Path(r"C:\proj\.venv\Scripts\python.exe") + + with patch.object(dep_sync.sys, "platform", "linux"): + p = dep_sync.console_script_path(posix_py) + assert p.name == "kirocrew" + assert not p.name.endswith(".exe") + assert p == posix_py.with_name("kirocrew") + + with patch.object(dep_sync.sys, "platform", "win32"): + w = dep_sync.console_script_path(win_py) + assert w.name == "kirocrew.exe" + assert w == win_py.with_name("kirocrew.exe") diff --git a/test/test_installer_python_floor.py b/test/test_installer_python_floor.py index 8f316d2af4d..f1b6159a20a 100644 --- a/test/test_installer_python_floor.py +++ b/test/test_installer_python_floor.py @@ -333,13 +333,16 @@ def test_editable_reinstall_proceeds_when_the_venv_meets_the_floor(tmp_path): patch.object(dep_sync, "requires_python", return_value=">=3.12"), patch.object(dep_sync, "interpreter_version", return_value=(3, 12, 3)), patch.object(dep_sync, "subprocess") as sp, + # Post-install verification (console script present + package imports) + # is stubbed green here; it has its own dedicated tests in test_dep_sync. + patch.object(dep_sync.os, "access", return_value=True), ): sp.run.return_value.returncode = 0 rc = dep_sync.sync_or_reinstall(repo, Path("py")) assert rc == 0 assert sp.run.called - argv = sp.run.call_args[0][0] + argv = sp.run.call_args_list[0][0][0] # the pip install call, not the import probe assert argv[1:5] == ["-m", "pip", "install", "-e"] @@ -378,6 +381,8 @@ def test_no_floor_declared_does_not_block_the_reinstall(tmp_path): patch.object(dep_sync, "locked_console_scripts", return_value=[]), patch.object(dep_sync, "requires_python", return_value=None), patch.object(dep_sync, "subprocess") as sp, + # Post-install verification stubbed green; covered in test_dep_sync. + patch.object(dep_sync.os, "access", return_value=True), ): sp.run.return_value.returncode = 0 rc = dep_sync.sync_or_reinstall(repo, Path("py")) @@ -468,3 +473,55 @@ def test_the_give_up_branch_names_the_provisioner_route(script, var): """Once the fallback can decline to run, the exit has to say how to enable it.""" body = script.read_text(encoding="utf-8") assert "mise.jdx.dev/installing-mise.html" in body + + +# --------------------------------------------------------------------------- +# install.sh: the .install-method=pip record is gated on a verified entry point +# --------------------------------------------------------------------------- + + +def test_install_sh_verifies_the_console_script_before_recording_pip(): + """A pip install that leaves no `kirocrew` entry point must not be recorded. + + The interrupted-venv-rebuild incident: a killed `pip install -e` can leave a + venv with a working interpreter but no console script, yet the old install.sh + still wrote `.install-method=pip` and symlinked a dangling `kirocrew`. The + gateway then dies later with exit 127. install.sh must die before recording + the method unless BOTH the executable entry point and the `import kiro_crew` + check pass. + """ + body = INSTALL_SH.read_text(encoding="utf-8") + + entry_guard = '[ ! -x "$_venv/bin/kirocrew" ]' + import_guard = '"$_venv/bin/python" -I -c "import kiro_crew"' + unsafe_import_guard = '"$_venv/bin/python" -c "import kiro_crew"' + record = 'echo "pip" > "$KIROCREW_APP_DIR/.install-method"' + + assert entry_guard in body, "missing the entry-point executable guard" + assert import_guard in body, "the import guard must ignore inherited PYTHONPATH" + assert unsafe_import_guard not in body, "PYTHONPATH can spoof the venv postcondition" + assert record in body, "the .install-method=pip record moved or was renamed" + + # Both guards must run BEFORE the method is recorded, or the record is not + # actually gated. This is the property that breaks if either guard is + # reverted (mutation check). + assert body.index(entry_guard) < body.index(record) + assert body.index(import_guard) < body.index(record) + + # And each guard must abort (die) rather than merely warn. + entry_stmt = body.split(entry_guard, 1)[1].split("fi", 1)[0] + assert "die " in entry_stmt, "the entry-point guard must die, not warn" + import_stmt = body.split(import_guard, 1)[1].split("fi", 1)[0] + assert "die " in import_stmt, "the import guard must die, not warn" + + +def test_install_sh_entry_point_guard_is_not_embedded_in_the_ec2_template(): + """The CFN UserData git-clones and runs install.sh; it must not duplicate it. + + The bootstrap template has a hard 16KB UserData ceiling, and the guard belongs + in install.sh (which the template invokes), not copied into the template. + """ + template = REPO / "src" / "kiro_crew" / "cloud" / "templates" / "kirocrew-ec2.yaml" + body = template.read_text(encoding="utf-8") + assert "Install incomplete: entry point" not in body + assert '-c "import kiro_crew"' not in body diff --git a/test/test_instances.py b/test/test_instances.py index fb9b7fd786a..9ea8cc3eac2 100644 --- a/test/test_instances.py +++ b/test/test_instances.py @@ -3443,6 +3443,36 @@ def test_generic_builders_and_token_delegation(self): # token builder emits identical strings via the generic builders it delegates to assert 'exec "$b" token --ttl 20h;' in build_remote_token_command("", ttl="20h") + def test_build_candidate_command_emits_per_candidate_diagnostics(self): + """When no candidate is executable, the snippet must explain WHY per path. + + The exit-127 incident gave the operator only "binary not found"; the real + state (a dangling symlink into an interrupted venv rebuild, an entry point + that never got written) was invisible. The failure branch now diagnoses + each candidate to stderr before exiting 127. + """ + from kiro_crew.instances.token_mint import build_candidate_command + + cmd = build_candidate_command("token") + + # Diagnosis header, symlink handling, and the distinct .venv Python probe. + assert 'echo "candidate diagnosis:" >&2;' in cmd + assert "DANGLING symlink" in cmd + assert "readlink -f" in cmd + assert "*/.venv/bin/*)" in cmd + assert "$__v/bin/python present" in cmd + assert "entry-point present" not in cmd + assert "entry-point MISSING" not in cmd + assert "symlink -> $__t (executable)" not in cmd + assert "present, executable" not in cmd + + # Ordering (mutation check): the diagnosis runs AFTER the not-found echo + # and BEFORE `exit 127`, i.e. only on the failure path. + not_found = cmd.index("kirocrew binary not found") + diagnosis = cmd.index("candidate diagnosis:") + exit_127 = cmd.index("exit 127") + assert not_found < diagnosis < exit_127 + def test_run_remote_kirocrew(self, monkeypatch): from kiro_crew.instances import token_mint as tm diff --git a/test/test_security_posture.py b/test/test_security_posture.py index 9956b9a37c0..223ac1d8dde 100644 --- a/test/test_security_posture.py +++ b/test/test_security_posture.py @@ -269,7 +269,7 @@ def _gate_side_baseline_log_sites( "mcp_tools/skills.py": 2, "messaging/sessions_view.py": 1, "slack/events.py": 2, - "slack/gateway.py": 6, + "slack/gateway.py": 7, "slack/handler.py": 3, "subagent_manager/admission.py": 4, "voice_reply.py": 4, diff --git a/test/test_slack_gateway.py b/test/test_slack_gateway.py index 2ea05c7250d..24aeb927df1 100644 --- a/test/test_slack_gateway.py +++ b/test/test_slack_gateway.py @@ -4,6 +4,7 @@ import ast import asyncio +import inspect import json import logging import os @@ -809,6 +810,50 @@ async def test_shutdown_cancels_handler_tasks(self): await orch._shutdown() assert task.cancelled() + @pytest.mark.asyncio + async def test_shutdown_cancels_and_reaps_console_script_repair(self, tmp_path): + """Shutdown owns the repair task, not only its direct cancellation path.""" + (tmp_path / ".install-method").write_text("pip") + venv_py = gw.dep_sync.project_venv_python(tmp_path) + venv_py.parent.mkdir(parents=True) + started = asyncio.Event() + never = asyncio.Event() + proc = _fake_async_proc() + + async def _communicate(): + started.set() + await never.wait() + return b"", b"" + + proc.communicate = AsyncMock(side_effect=_communicate) + orch = _make_orchestrator() + + async def fake_prepare(cmd, **kwargs): + return list(cmd), {}, None + + with ( + patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": str(tmp_path)}, clear=False), + patch.object(gw, "sandboxed_spawn_argv_async", side_effect=fake_prepare), + patch.object( + gw, + "create_subprocess_limited", + new_callable=AsyncMock, + return_value=proc, + ), + patch.object(orch, "_kill_startup_child", new_callable=AsyncMock) as kill, + patch.object(orch, "_reap_startup_child", new_callable=AsyncMock) as reap, + ): + repair_task = orch._schedule_console_script_repair() + await asyncio.wait_for(started.wait(), timeout=1) + await orch._shutdown() + await asyncio.sleep(0) + + assert repair_task.cancelled() + kill.assert_awaited_once_with(proc) + reap.assert_awaited_once_with(proc) + assert orch._console_script_repair_task is None + assert repair_task not in orch._background_tasks + @pytest.mark.asyncio async def test_shutdown_disarms_watchdog_before_reaping(self): # The loop-stall watchdog's armed dump-then-exit timer MUST be cancelled @@ -1066,6 +1111,272 @@ def test_check_missing_deps_brazil_skips(self): ): asyncio.run(orch._check_missing_deps()) # should not raise, skips pip + # --- _check_console_script ------------------------------------------------- + + def test_check_console_script_skips_when_no_project_dir(self): + orch = _make_orchestrator() + with patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": ""}, clear=False): + with patch.object(gw, "sandboxed_spawn_argv_async", new_callable=AsyncMock) as spawn: + asyncio.run(orch._check_console_script()) + spawn.assert_not_awaited() + + def test_check_console_script_skips_brazil(self, tmp_path): + (tmp_path / ".install-method").write_text("brazil") + orch = _make_orchestrator() + with patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": str(tmp_path)}, clear=False): + with patch.object(gw, "sandboxed_spawn_argv_async", new_callable=AsyncMock) as spawn: + asyncio.run(orch._check_console_script()) + spawn.assert_not_awaited() + + def test_check_console_script_skips_non_pip(self, tmp_path): + # No .install-method (or a non-pip one) means this is not a pip install. + orch = _make_orchestrator() + with patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": str(tmp_path)}, clear=False): + with patch.object(GatewayOrchestrator, "_is_brazil_install", return_value=False): + with patch.object(gw, "sandboxed_spawn_argv_async", new_callable=AsyncMock) as spawn: + asyncio.run(orch._check_console_script()) + spawn.assert_not_awaited() + + def test_check_console_script_skips_when_script_present_and_executable(self, tmp_path): + (tmp_path / ".install-method").write_text("pip") + venv_py = gw.dep_sync.project_venv_python(tmp_path) + script = gw.dep_sync.console_script_path(venv_py) + script.parent.mkdir(parents=True) + script.write_text("#!/bin/sh\n") + script.chmod(0o755) + orch = _make_orchestrator() + with patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": str(tmp_path)}, clear=False): + with patch.object(gw, "sandboxed_spawn_argv_async", new_callable=AsyncMock) as spawn: + asyncio.run(orch._check_console_script()) + spawn.assert_not_awaited() + + def test_check_console_script_reinstalls_when_missing(self, tmp_path): + (tmp_path / ".install-method").write_text("pip") + venv_py = gw.dep_sync.project_venv_python(tmp_path) + # The interpreter directory exists but the entry point is absent. + venv_py.parent.mkdir(parents=True) + proc = _fake_async_proc() + orch = _make_orchestrator() + seen = {} + + async def fake_prepare(cmd, **kwargs): + seen["argv"] = list(cmd) + seen["kwargs"] = kwargs + return list(cmd), {"SCRUBBED": "1"}, None + + with patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": str(tmp_path)}, clear=False): + with patch.object(gw, "sandboxed_spawn_argv_async", side_effect=fake_prepare): + with patch.object( + gw, + "create_subprocess_limited", + new_callable=AsyncMock, + return_value=proc, + ) as spawn: + asyncio.run(orch._check_console_script()) + + # The dep_sync argv is asserted at the sandbox seam's input, which is + # where it is now composed. + assert seen["argv"] == [ + sys.executable, + str(Path(gw.dep_sync.__file__).resolve()), + "--repair-missing-package", + str(tmp_path), + str(venv_py), + ] + spawn.assert_awaited_once() + assert spawn.await_args.args == tuple(seen["argv"]) + assert spawn.await_args.kwargs["cwd"] == str(tmp_path) + assert spawn.await_args.kwargs["env"] == {"SCRUBBED": "1"} + assert spawn.await_args.kwargs["start_new_session"] is gw.platform_compat.IS_POSIX + + def test_check_console_script_reinstalls_on_dangling_symlink(self, tmp_path): + (tmp_path / ".install-method").write_text("pip") + venv_py = gw.dep_sync.project_venv_python(tmp_path) + script = gw.dep_sync.console_script_path(venv_py) + script.parent.mkdir(parents=True) + # Dangling symlink: exists() is False, so the reinstall must fire. + try: + script.symlink_to(tmp_path / "does-not-exist") + except OSError as exc: + pytest.skip(f"symlinks unavailable: {exc}") + proc = _fake_async_proc() + orch = _make_orchestrator() + + async def fake_prepare(cmd, **kwargs): + return list(cmd), {}, None + + with patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": str(tmp_path)}, clear=False): + with patch.object(gw, "sandboxed_spawn_argv_async", side_effect=fake_prepare): + with patch.object( + gw, + "create_subprocess_limited", + new_callable=AsyncMock, + return_value=proc, + ) as spawn: + asyncio.run(orch._check_console_script()) + spawn.assert_awaited_once() + + def test_check_console_script_skipped_when_sandbox_unavailable(self, tmp_path): + """Fail CLOSED: no sandbox backend means the repair does NOT run. + + Running the project's own venv interpreter unsandboxed is the exposure + the routing removes, so an unavailable sandbox must skip the repair + rather than fall back to a bare spawn. + """ + (tmp_path / ".install-method").write_text("pip") + venv_py = gw.dep_sync.project_venv_python(tmp_path) + venv_py.parent.mkdir(parents=True) + orch = _make_orchestrator() + unavailable = gw.SandboxUnavailableError( + "no sandbox backend", + "no_backend", + "unshare(CLONE_NEWNS) failed with errno 1 (EPERM)", + ) + + with patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": str(tmp_path)}, clear=False): + with patch.object( + gw, + "sandboxed_spawn_argv_async", + side_effect=unavailable, + ): + with patch.object( + gw, "create_subprocess_limited", new_callable=AsyncMock + ) as spawn: + with patch.object(gw.logger, "error") as log_error: + asyncio.run(orch._check_console_script()) + + spawn.assert_not_awaited() + # The skip must name the machine-readable reason, not just that it + # skipped -- an undiagnosable failure is the class #8409 is about. + logged = log_error.call_args.args + assert "no_backend" in logged + assert any("EPERM" in str(part) for part in logged) + + @pytest.mark.asyncio + async def test_check_console_script_cancellation_kills_and_reaps_child(self, tmp_path): + """Gateway shutdown must not leave dep_sync or its pip descendants running.""" + (tmp_path / ".install-method").write_text("pip") + venv_py = gw.dep_sync.project_venv_python(tmp_path) + venv_py.parent.mkdir(parents=True) + started = asyncio.Event() + never = asyncio.Event() + proc = _fake_async_proc() + + async def _communicate(): + started.set() + await never.wait() + return b"", b"" + + proc.communicate = AsyncMock(side_effect=_communicate) + orch = _make_orchestrator() + cleanup = tmp_path / "launcher-profile" + cleanup.write_text("") + + async def fake_prepare(cmd, **kwargs): + return list(cmd), {}, str(cleanup) + + with ( + patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": str(tmp_path)}, clear=False), + patch.object(gw, "sandboxed_spawn_argv_async", side_effect=fake_prepare), + patch.object( + gw, + "create_subprocess_limited", + new_callable=AsyncMock, + return_value=proc, + ), + patch.object(orch, "_kill_startup_child", new_callable=AsyncMock) as kill, + patch.object(orch, "_reap_startup_child", new_callable=AsyncMock) as reap, + ): + task = asyncio.create_task(orch._check_console_script()) + await asyncio.wait_for(started.wait(), timeout=1) + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + + kill.assert_awaited_once_with(proc) + reap.assert_awaited_once_with(proc) + # The sandbox launcher temp file must not leak, even on cancellation. + assert not cleanup.exists() + + @pytest.mark.asyncio + async def test_check_console_script_is_sandbox_routed_and_limited(self, tmp_path): + """The repair spawn MUST route through the sandbox chokepoint. + + The child EXECUTES the project's own venv interpreter (dep_sync probes + it), so it must get OS isolation, a scrubbed env, and the kernel + resource ceiling. Reverting to a bare ``asyncio.create_subprocess_exec`` + fails this two ways: the seams below are never awaited, and the poisoned + ``create_subprocess_exec`` raises the moment the old path touches it. + """ + (tmp_path / ".install-method").write_text("pip") + venv_py = gw.dep_sync.project_venv_python(tmp_path) + venv_py.parent.mkdir(parents=True) + proc = _fake_async_proc() + orch = _make_orchestrator() + prepared = {} + + async def fake_prepare(cmd, **kwargs): + prepared["argv"] = list(cmd) + prepared["kwargs"] = kwargs + return list(cmd), {"SCRUBBED": "1"}, None + + def _boom(*args, **kwargs): # a revert to the bare exec path lands here + raise AssertionError("bare asyncio.create_subprocess_exec is forbidden") + + with ( + patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": str(tmp_path)}, clear=False), + patch.object(gw, "sandboxed_spawn_argv_async", side_effect=fake_prepare) as prep, + patch.object( + gw, + "create_subprocess_limited", + new_callable=AsyncMock, + return_value=proc, + ) as limited, + patch("asyncio.create_subprocess_exec", side_effect=_boom), + ): + await orch._check_console_script() + + # Routed through the chokepoint, with the real synchronous preparer. + prep.assert_awaited_once() + assert prepared["kwargs"]["_prepare"] is gw.sandboxed_spawn_argv + # Keeps the project/venv writable so pip can rewrite the entry point, + # and refuses to leak our interpreter paths into the child's probe. + assert prepared["kwargs"]["mode"] == "strict" + assert prepared["kwargs"]["strip_python_env"] is True + # Spawned via the resource-limited launcher with the scrubbed env. + limited.assert_awaited_once() + assert limited.await_args.kwargs["env"] == {"SCRUBBED": "1"} + assert limited.await_args.kwargs["start_new_session"] is gw.platform_compat.IS_POSIX + + @pytest.mark.asyncio + async def test_console_script_repair_task_is_tracked_without_blocking(self): + orch = _make_orchestrator() + started = asyncio.Event() + release = asyncio.Event() + + async def _repair(): + started.set() + await release.wait() + + with patch.object(orch, "_check_console_script", side_effect=_repair): + task = orch._schedule_console_script_repair() + await asyncio.wait_for(started.wait(), timeout=1) + assert task in orch._background_tasks + assert not task.done() + release.set() + await task + await asyncio.sleep(0) + + assert task not in orch._background_tasks + assert orch._console_script_repair_task is None + + def test_console_script_repair_is_scheduled_only_after_http_bind(self): + """The potentially 300-second repair must never gate socket readiness.""" + source = inspect.getsource(GatewayOrchestrator.run) + scheduled = source.index("self._schedule_console_script_repair()") + assert source.index("await self._init_dashboard()") < scheduled + assert source.index("await self._init_api_server()") < scheduled + # ═══════════════════════════════════════════════════════════════════════════ # Tests: _init_cron