diff --git a/CHANGELOG.md b/CHANGELOG.md index 20e16d78..5df2583f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -87,9 +87,73 @@ table and should be judged on its own merits. `claude` that could never run. The probe that decides all this is captured, unlike the trips that may follow - it. It reports nothing, and its everyday answer on a cold workspace ("tools - missing") reached the terminal as a red devpod - `fatal ... Process exited with status 1` describing the probe working. + it: what it prints is the answer the caller branches on rather than progress + anybody needs to watch. + +- **A baked `claude` counts as provisioned only if it is the real one.** The + probe used to ask `command -v claude` and believe the answer, which the + `claude-shim` downloader satisfies — so an image carrying the shim (including + any built from this repo's own `.devcontainer/claude-code/` feature) skipped + the lend and paid the ~285MB GCS download on first use, the exact cost the + lending exists to remove. It now answers one of three states: `provisioned` + when `gh` is on the login `PATH` **and** `claude` resolves to a binary the + official installer itself put in `~/.local/share/claude/versions/`; + `lendable` when both answer but the claude is a shim or wrapper; `absent` + otherwise. + + "The official install" is one definition, asked from both ends of the pipe. + The container reports two facts only it can know — where its `claude` + resolves to, and where that directory in its own home resolves to — and + which state those mean is decided on the host, by the same code that decides + what the host may lend. Two copies of that rule, one per language, is what a + shared constant alone does not prevent: a downloader parked at + `versions/latest/bin/claude` satisfies "somewhere under the versions + directory" while failing "a binary the installer wrote", and a probe holding + the looser of the two opinions trusts it. + + Both paths are compared fully resolved, which is what makes the upgrade + terminate on an image whose `$HOME` is reached through a symlink: a + `lendable` container is quietly upgraded — the host streams in its own binary + and the transfer's `~/.local/bin` `PATH` prepend is what makes it win from + then on — so the *next* launch probes `provisioned` and the tar is paid once + rather than on every launch for the life of the workspace. + + That prepend only actually happens because the guard in front of it now asks + the right question. **Every line devlaunch appends to a container's login + profile is written under a `# devlaunch:` mark, and the "have I already done + this?" guard is an exact match on that mark** — not, as before, a search for + the directory being added. Searching for the directory made the answer a base + image's to give, and it gave the wrong one: Ubuntu's stock `~/.profile` + prepends `~/.local/bin` itself, near the top of the file, so on + `mcr.microsoft.com/devcontainers/base:ubuntu-24.04` — the image this repo's + own devcontainer builds on — the transfer read that block as its own work, + skipped the prepend, and left the shim ahead of the binary it had just lent. + The workspace never converged: it answered `lendable`, re-paid the whole + transfer, and answered `lendable` again, every `devpod up`, forever. The + convergence test now lets the profile decide `PATH` — sourced from a home + seeded with that image's stock file plus the lines this repo's devcontainer + appends — because a test that builds `PATH` itself cannot see the ordering + the lend depends on. A profile some earlier devlaunch already edited gains + one duplicate `PATH` entry the first time it is seen, and nothing after that. + + If the host has + nothing to lend, or the lent binaries do not run there, the container is + accepted as it stands rather than falling through to the network install — + that install decides what to do with its own `command -v` guards, which a + shim already satisfies, so the trip would install nothing. + + The probe never executes the candidate `claude`; on a shim, *any* invocation + triggers the very download the probe exists to detect. It resolves the path + instead. It also exits 0 in every state now — including in an image that + never set `HOME` — which retires the red devpod + `fatal ... Process exited with status 1` that the old probe's everyday cold + answer painted on the terminal. + + Two things this deliberately does not do. It does not compare versions: a + real `claude` already in the container is left alone however old it is, since + keeping versions in sync would make this a package manager (the binary + self-updates, and a workspace rebuild re-provisions). And it does not make + the payload per-tool — an image with only `gh` still receives both. - **Two `up`s of one workspace serialize on a per-workspace lock.** Background prewarming makes concurrent `up`s of a single workspace an everyday event diff --git a/devlaunch/tools.py b/devlaunch/tools.py index c64b11b3..5843891a 100644 --- a/devlaunch/tools.py +++ b/devlaunch/tools.py @@ -24,10 +24,22 @@ lent binaries do not run there (a different arch or libc), does the old network path run: bootstrap pixi, `pixi global install` each tool. -The round trips this costs, by path: a warm workspace pays one (the probe, -which was always paid); a cold one pays two (probe, then transfer), or three -when the transfer cannot help and the network fallback runs -- against a -`devpod up` that already ran for seconds to minutes. +What "lacks them" means is the probe's job, and it has three answers rather +than two, because a `claude` on PATH is not necessarily a `claude` worth +keeping: the shim this repo's own devcontainer feature bakes satisfies +`command -v` while still owing the GCS download on first use. So the probe +answers `provisioned` only for the official install layout, `lendable` for a +container whose claude is something else, and `absent` when a tool is really +missing -- and a lendable container is quietly upgraded to the host's real +binary the next time it goes through `up`. The container reports what it +found and the host says what that means, so "the official install layout" is +one definition asked from both ends rather than one per side. + +The round trips this costs, by path: a provisioned workspace pays one (the +probe, which was always paid); a lendable or cold one pays two (probe, then +transfer), or three when the transfer cannot help a genuinely empty container +and the network fallback runs -- against a `devpod up` that already ran for +seconds to minutes. Two consequences worth knowing: @@ -42,6 +54,7 @@ make an exception of. """ +import enum import json import logging import os @@ -61,6 +74,19 @@ # The claude package lives in a personal channel rather than conda-forge. BLOOOP_CHANNEL = "https://prefix.dev/blooop" +# Where the official claude installer keeps one binary per version, relative to +# a home directory. The host reads it to decide what it may lend +# (`_claude_source`), writes into it when it lends (`transfer_script`), and the +# container is asked about it to decide whether it needs anything +# (`probe_script`). The *relation* that makes a path the official layout lives +# in `_is_official_claude`, for the same reason this string lives here: two +# copies of a definition are two probes with different opinions. +CLAUDE_VERSIONS_RELPATH = ".local/share/claude/versions" + +# What every line of a probe's report starts with, so the report survives being +# printed into the same stdout as a container login profile's banner. +PROBE_MARK = "devlaunch-probe" + # Which file a bash login shell will actually read. bash tries ~/.bash_profile, # ~/.bash_login and ~/.profile in that order and sources only the first that # exists, so appending to ~/.profile in an image that ships a ~/.bash_profile @@ -74,6 +100,20 @@ ] ) +# What devlaunch writes above every PATH line it appends to a container's login +# profile, and the only thing its "have I already done this?" guard looks for. +# The mark exists so the guard can ask about devlaunch's own work instead of +# about a directory name. Asking about the directory made the answer depend on +# a file devlaunch does not own: Ubuntu's stock ~/.profile prepends +# ~/.local/bin itself, so on this repo's own base image +# (mcr.microsoft.com/devcontainers/base:ubuntu-24.04) the transfer's guard read +# the image's block as its own, skipped the prepend, and left the claude shim +# in front of the binary just lent -- which made every `devpod up` re-pay a +# multi-hundred-megabyte transfer for the life of the workspace. The same +# lesson devpod_provider learned from grepping devpod's rendered table: a guard +# that reads someone else's artifact is only correct until they change it. +PROFILE_MARK = "# devlaunch:" + @dataclass(frozen=True) class Tool: @@ -107,6 +147,68 @@ def provisioning_disabled() -> bool: return os.environ.get(DISABLE_VAR, "").strip().lower() not in _FALSEY +def _all_present(tools: Sequence[Tool]) -> str: + """The bash test for "every one of these already answers on PATH".""" + return " && ".join(f"command -v {shlex.quote(tool.command)} >/dev/null 2>&1" for tool in tools) + + +def _is_official_claude(versions_dir: str, claude_binary: str) -> bool: + """Whether a resolved `claude` is a binary of the official install. + + The one definition of "the official layout", and both sides of the pipe + ask it: the host asks it of its own filesystem to decide what it may lend, + and the container's probe reports its two resolved paths so it can be asked + of those here. Written down once because a container-side copy of this + relation and a host-side copy are two probes with different opinions -- + which is how a downloader parked at `versions/latest/bin/claude` came to be + trusted by one while the other refused to lend the very same tree. + + A *direct child*, because that is what the installer creates: one binary + per version, named for the version. Anything deeper is somebody else's + tree that merely starts with the official path, and a downloader is free + to choose such a path. + + Both arguments must arrive resolved -- `readlink -f` in the container, + `Path.resolve` on the host -- because either home may be reached through a + symlink, and comparing a resolved binary against an unresolved home makes + a genuine install look like a shim forever. Anything that is not an + absolute path (the empty answer a container with no `readlink` gives, a + truncated line) is not the official layout: of the two ways to be wrong, + lending again costs seconds and trusting a shim ships a container that + still owes the download. + """ + if not versions_dir.startswith("/") or not claude_binary.startswith("/"): + return False + return pathlib.PurePosixPath(claude_binary).parent == pathlib.PurePosixPath(versions_dir) + + +def _profile_prepend(tag: str, line: str, on_failure: str = "") -> str: + """One PATH line appended to `$PROFILE` at most once, ever. + + The line is written under a `PROFILE_MARK` comment naming `tag`, and the + guard is an exact-line match on that comment -- so what decides whether the + edit has already been made is a line only these scripts ever write. + Substring-matching the directory being added cannot do that job: a base + image is free to mention, or even prepend, the same directory for its own + reasons, and then the guard skips an append the workspace still needs. + + Exact-line (`-x`) and fixed-string (`-F`) rather than a pattern, so nothing + in the mark is read as a regex and a longer line that merely contains it + does not count as a match. + + Appending under a *new* mark to a profile some older devlaunch already + edited duplicates one PATH entry, once: harmless (a directory twice on PATH + resolves the same), self-limiting (the mark is there from then on), and the + price of the guard no longer being able to lie. + """ + mark = f"{PROFILE_MARK} {tag}" + tail = f" || {on_failure}" if on_failure else "" + return ( + f'grep -qxF {shlex.quote(mark)} "$PROFILE" 2>/dev/null || ' + f"printf '%s\\n' {shlex.quote(mark)} {shlex.quote(line)} >> \"$PROFILE\"{tail}" + ) + + def _install_line(tool: Tool) -> str: args = " ".join(shlex.quote(arg) for arg in tool.install_args) return ( @@ -129,9 +231,7 @@ def provision_script(tools: Sequence[Tool] = REQUIRED_TOOLS) -> str: Exits 0 unless an install actually failed, so "nothing to do" and "all installs worked" are the same answer to the caller. """ - all_present = " && ".join( - f"command -v {shlex.quote(tool.command)} >/dev/null 2>&1" for tool in tools - ) + all_present = _all_present(tools) installs = "\n".join(_install_line(tool) for tool in tools) # The trampoline pixi writes into ~/.pixi/bin does not work for packages # that ship a shell script, which is why the env's own bin directory is @@ -146,11 +246,15 @@ def provision_script(tools: Sequence[Tool] = REQUIRED_TOOLS) -> str: # (since the check above is `command -v`) reinstalled from scratch # on every single launch. _PROFILE_RESOLUTION, - 'grep -q "\\.pixi/bin" "$PROFILE" 2>/dev/null || ' - 'echo \'export PATH="$HOME/.pixi/bin:$PATH"\' >> "$PROFILE" || failed=1', - 'grep -q "pixi/envs/claude-shim" "$PROFILE" 2>/dev/null || ' - 'echo \'[ -d "$HOME/.pixi/envs/claude-shim/bin" ] && ' - 'export PATH="$HOME/.pixi/envs/claude-shim/bin:$PATH"\' >> "$PROFILE" || failed=1', + _profile_prepend( + "pixi-bin", 'export PATH="$HOME/.pixi/bin:$PATH"', on_failure="failed=1" + ), + _profile_prepend( + "claude-shim-bin", + '[ -d "$HOME/.pixi/envs/claude-shim/bin" ] && ' + 'export PATH="$HOME/.pixi/envs/claude-shim/bin:$PATH"', + on_failure="failed=1", + ), ] ) return "\n".join( @@ -174,6 +278,109 @@ def provision_script(tools: Sequence[Tool] = REQUIRED_TOOLS) -> str: ) +class ProbeResult(enum.Enum): + """What one probe found in a workspace -- the whole answer, in one value. + + Three states rather than a boolean, because "there is a claude" and "there + is a claude worth keeping" are different questions, and collapsing them is + the bug this replaced: a container carrying the shim answered yes to the + first and was left owing the ~285MB download the lending exists to avoid. + A boolean plus a flag would have been the same two questions with a fourth, + meaningless combination available; three named states have no illegal one. + + Which state a report means is decided here and nowhere else. The container + reports what it found and never names a state, so there is no container- + side copy of "the official layout" to drift away from the host's. + """ + + PROVISIONED = "provisioned" + LENDABLE = "lendable" + ABSENT = "absent" + + @classmethod + def parse(cls, report: str) -> "ProbeResult": + """Read a probe's report. Total: anything unreadable is ABSENT. + + The report is marked `key value` lines rather than one token because a + token would mean the container had already decided -- with its own copy + of the relation `_is_official_claude` states once, for both sides. + What crosses the pipe is therefore two resolved paths only the + container can know, and what they mean is settled here. + + Marked lines, and read from anywhere in the output, because the probe + runs under `bash -lc`: the container's login profile is sourced first, + so an image whose profile prints a banner puts it on the same stdout. + + A garbled, empty or truncated report has to mean something, and ABSENT + is the only state whose worst case is harmless -- provisioning is + idempotent, so reading it wrongly costs a redundant round trip, where + a wrong PROVISIONED would silently skip the work the probe exists to + schedule. + """ + found = {} + for line in report.splitlines(): + mark, _, rest = line.strip().partition(" ") + if mark != PROBE_MARK: + continue + key, _, value = rest.partition(" ") + found[key] = value.strip() + if found.get("tools") != "present": + return cls.ABSENT + if _is_official_claude(found.get("versions", ""), found.get("claude", "")): + return cls.PROVISIONED + return cls.LENDABLE + + +def probe_script() -> str: + """The shell script that reports what a workspace already has. + + It reports; it does not decide. Whether a container counts as provisioned + turns on two facts only the container can know -- where its `claude` + resolves to, and where the official versions directory in its home + resolves to -- so those are what it prints, and `ProbeResult.parse` says + what they mean. That split is the point: the relation between the two + paths is stated once, in `_is_official_claude`, instead of once per + language, which is what let a shim be trusted by one side and refused by + the other. + + Exits 0 in every state: "this container has nothing" is an answer, not a + failure, and a probe that exits non-zero paints a red devpod + `fatal ... Process exited with status 1` on the terminal of every cold + launch, describing the probe working exactly as intended. Nothing here can + fail -- `$HOME` is expanded defensively because an image that never set it + would otherwise abort the script under `set -u`, and a path that will not + resolve is reported empty, which reads as `lendable`. + + It asks about the fixed pair this module lends rather than an arbitrary + tool set, for the same reason `host_payload` does: what can be lent is + `gh` and `claude`, so those are what there is a three-state answer for. + """ + return "\n".join( + [ + "set -u", + f"if ! {{ {_all_present(REQUIRED_TOOLS)} ; }}; then", + f' echo "{PROBE_MARK} tools missing"', + " exit 0", + "fi", + f'echo "{PROBE_MARK} tools present"', + # Both paths fully resolved, because the comparison they are for is + # an equality: a home reached through a symlink resolves one side + # and not the other, and a real install then reads `lendable` on + # every launch forever. + f'echo "{PROBE_MARK} versions ' + f'$(readlink -f "${{HOME-}}/{CLAUDE_VERSIONS_RELPATH}" 2>/dev/null || true)"', + # Resolved, never run. The shim on PATH answers `command -v` just + # as the real binary does, and telling them apart by asking either + # one for its version would trigger, on the shim, the very ~285MB + # download this answer exists to avoid. Where the name resolves to + # is the whole test. `readlink` failing (or missing) leaves this + # empty, which reads as `lendable` -- the answer that does more + # work, never the one that skips it. + f'echo "{PROBE_MARK} claude $(readlink -f "$(command -v claude)" 2>/dev/null || true)"', + ] + ) + + def _pixi_bootstrap() -> str: """Install pixi if the image has none, since every tool here comes from it. @@ -214,16 +421,23 @@ def _claude_source() -> Optional[Tuple[str, pathlib.Path]]: The official installer keeps one binary per version under `~/.local/share/claude/versions/` and points `~/.local/bin/claude` at the current one. Anything else on PATH answering to `claude` -- the pixi shim, - a wrapper script -- is exactly the kind of downloader this transfer exists - to skip, so only the official layout counts. + a wrapper script, a downloader that parked itself deeper inside that same + directory -- is exactly the kind of downloader this transfer exists to + skip, so only the official layout counts, and what counts as the official + layout is `_is_official_claude`: the same relation the container's report + is read through, so the two sides cannot come to different answers about + one tree. """ - link = pathlib.Path.home() / ".local/bin/claude" + home = pathlib.Path.home() + link = home / ".local/bin/claude" try: target = link.resolve(strict=True) except OSError: return None - versions_dir = (pathlib.Path.home() / ".local/share/claude/versions").resolve() - if target.parent != versions_dir or not os.access(target, os.X_OK): + versions_dir = (home / CLAUDE_VERSIONS_RELPATH).resolve() + if not _is_official_claude(str(versions_dir), str(target)): + return None + if not os.access(target, os.X_OK): return None return target.name, target @@ -268,7 +482,7 @@ def host_payload() -> Optional[HostPayload]: return HostPayload( claude_version=version, members=( - (claude_binary, f".local/share/claude/versions/{version}"), + (claude_binary, f"{CLAUDE_VERSIONS_RELPATH}/{version}"), (gh, ".local/bin/gh"), ), ) @@ -284,12 +498,16 @@ def transfer_script(payload: HostPayload) -> str: the caller falls back to the network install. """ version = payload.claude_version - claude_rel = f".local/share/claude/versions/{version}" + claude_rel = f"{CLAUDE_VERSIONS_RELPATH}/{version}" + # Prepended, and appended *last*, because winning is the whole point: the + # container this lend exists for already has a shim earlier on PATH, put + # there by a line further up this same file. The lent binary only becomes + # the `claude` a session finds -- and the next probe only reads + # `provisioned` -- because ~/.local/bin goes in front of it. profile_lines = "\n".join( [ _PROFILE_RESOLUTION, - 'grep -q "\\.local/bin" "$PROFILE" 2>/dev/null || ' - 'echo \'export PATH="$HOME/.local/bin:$PATH"\' >> "$PROFILE"', + _profile_prepend("local-bin", 'export PATH="$HOME/.local/bin:$PATH"'), ] ) return "\n".join( @@ -318,7 +536,7 @@ def transfer_script(payload: HostPayload) -> str: f'"$STAGE/{claude_rel}" --version >/dev/null', '"$STAGE/.local/bin/gh" --version >/dev/null', # Proven. Now they can be moved into place. - 'mkdir -p "$HOME/.local/bin" "$HOME/.local/share/claude/versions"', + f'mkdir -p "$HOME/.local/bin" "$HOME/{CLAUDE_VERSIONS_RELPATH}"', f'mv -f "$STAGE/{claude_rel}" "$HOME/{claude_rel}"', 'mv -f "$STAGE/.local/bin/gh" "$HOME/.local/bin/gh"', # The host's own symlink points through the host's home, so the @@ -341,22 +559,23 @@ def _write_payload_tar(payload: HostPayload, out: pathlib.Path) -> None: tar.add(source, arcname=arcname) -def _probe(workspace: str, runner, tools: Sequence[Tool]) -> bool: - """One round trip: are all `tools` already on the workspace's PATH? +def _probe(workspace: str, runner) -> ProbeResult: + """One round trip: what does this workspace still need? + + Captured, unlike the two trips that may follow -- here the output *is* the + answer the caller branches on, rather than progress a user needs to watch. - Captured, unlike the two trips that may follow. A probe has no progress to - report -- it is a yes/no question -- and "no" is the everyday answer on a - cold workspace, which devpod renders on the terminal as a red - `fatal ... Process exited with status 1`. That line describes the probe - working exactly as intended, so it is not shown. + A trip that fails is not an answer: the script exits 0 in all three states, + so a non-zero status means the ssh itself did not get through, and the + reading that costs a redundant trip is preferred to the one that skips the + work. """ - checks = " && ".join( - f"command -v {shlex.quote(tool.command)} >/dev/null 2>&1" for tool in tools - ) result = runner( - ["ssh", workspace, "--command", f"bash -lc {shlex.quote(checks)}"], capture=True + ["ssh", workspace, "--command", f"bash -lc {shlex.quote(probe_script())}"], capture=True ) - return result.returncode == 0 + if result.returncode != 0: + return ProbeResult.ABSENT + return ProbeResult.parse(result.stdout) def _transfer(workspace: str, runner, payload: HostPayload) -> bool: @@ -382,8 +601,12 @@ def _transfer(workspace: str, runner, payload: HostPayload) -> bool: return result.returncode == 0 -def ensure_tools(workspace: str, runner, tools: Sequence[Tool] = REQUIRED_TOOLS) -> bool: - """Make `tools` available in `workspace`. Returns whether they now are. +def ensure_tools(workspace: str, runner) -> bool: + """Make REQUIRED_TOOLS available in `workspace`. Returns whether they now are. + + Not parametric over a tool set, because the probe is not: it asks about the + fixed pair this module can lend, so a caller-supplied set would be probed + for one thing and installed for another. `runner` is dl.run_devpod, passed in rather than imported to keep this module off dl's import cycle and testable without a devpod. @@ -392,7 +615,13 @@ def ensure_tools(workspace: str, runner, tools: Sequence[Tool] = REQUIRED_TOOLS) provisioned workspace ever pays), then the host lending its own binaries (see the module docstring for why that is the fast path), then the network install for a host with nothing to lend or a container the lent binaries - cannot run in. + cannot run in. Which of them run is decided by the probe's three-state + answer, and only a genuinely `absent` container ever reaches the third. + + Version drift is deliberately not handled: a real claude already in the + container is left alone whatever its version. Keeping versions in sync + would make this a package manager, which the module docstring's "the host + first, the network second" is explicitly not. The network payload goes through `bash -lc` for the same reason workspace_ssh wraps its own: devpod runs --command under a shell that @@ -414,14 +643,39 @@ def ensure_tools(workspace: str, runner, tools: Sequence[Tool] = REQUIRED_TOOLS) return False try: - if _probe(workspace, runner, tools): + found = _probe(workspace, runner) + if found is ProbeResult.PROVISIONED: return True payload = host_payload() if payload is not None and _transfer(workspace, runner, payload): return True - script = provision_script(tools) + if found is ProbeResult.LENDABLE: + # Both tools already answer here; only the claude was the wrong + # one, and the lend that would have replaced it either had nothing + # to send or would not run in this container. The network fallback + # cannot help either way: it decides what to install with its own + # `command -v` guards (see provision_script), which both tools + # already satisfy, so the third trip would install nothing. Stop + # with what is there. + # + # Accepted residual: a container that keeps rejecting the lend -- + # a different libc or architecture -- while carrying a shim + # re-attempts one failing transfer on every `devpod up`, paying for + # the tar and the stream before the arch/libc gate refuses the + # binaries. What that costs end to end has not been measured; the + # figure this comment used to carry was inherited, not taken. The + # one part of it that has been measured is staging the tar (#158: + # ~0.13s warm, ~2.6s cold), which is a fraction of it. Breaking the + # loop needs per-container retry state persisted somewhere, which is + # more machinery than the case is worth, and ensure_tools never runs + # on the fast-attach path: this only ever rides an `up` that already + # took seconds to minutes. + return True + + # ProbeResult.ABSENT: the cold flow, with a tool genuinely missing. + script = provision_script() result = runner(["ssh", workspace, "--command", f"bash -lc {shlex.quote(script)}"]) except OSError as e: logging.debug("Could not install tools into %s: %s", workspace, e) @@ -432,7 +686,7 @@ def ensure_tools(workspace: str, runner, tools: Sequence[Tool] = REQUIRED_TOOLS) # session, not for an install. logging.warning( "Could not install %s into %s; the session will start without them.", - " and ".join(tool.command for tool in tools), + " and ".join(tool.command for tool in REQUIRED_TOOLS), workspace, ) return False diff --git a/test/unit/test_tools.py b/test/unit/test_tools.py index 7a3cb78e..49152bef 100644 --- a/test/unit/test_tools.py +++ b/test/unit/test_tools.py @@ -1,7 +1,9 @@ """What devlaunch installs into a workspace, and what it does when that fails.""" import io +import os import shlex +import shutil import subprocess import tarfile from typing import List, Optional @@ -166,6 +168,60 @@ def test_a_custom_tool_set_is_honoured(self): assert "command -v gh" not in script +class TestProfileGuards: + """Every "have I already edited this profile?" question, in both scripts. + + Asked once here rather than per script, because the property is the same + wherever it is asked and the answer decides whether a workspace ever stops + doing work it has already done. + """ + + @staticmethod + def _guards(script): + """The guard half of every line that appends to the login profile.""" + guards = [ + line.partition("||")[0] for line in script.splitlines() if '>> "$PROFILE"' in line + ] + assert guards, "a script that never edits the profile has nothing to guard" + return guards + + @pytest.mark.parametrize("name", ["provision", "transfer"]) + def test_a_guard_asks_about_devlaunchs_own_line_not_about_a_directory(self, name, tmp_path): + """The guard means "have *we* already prepended our directory here", and + the only evidence of that which devlaunch owns is the mark it writes. + + Asking instead whether the directory is mentioned anywhere in the file + makes the answer a base image's to give. Ubuntu's stock ~/.profile + prepends ~/.local/bin itself -- so on this repo's own base image that + question answered "already done" about work nobody had done, the lent + binary never reached the front of PATH, and every launch re-paid the + transfer. A directory name may appear in what is appended; it may not + appear in what decides whether to append. + """ + script = ( + provision_script() + if name == "provision" + else tools.transfer_script(fake_payload(tmp_path)) + ) + for guard in self._guards(script): + assert tools.PROFILE_MARK in guard + for owned_by_the_image in (".local/bin", ".pixi/bin", "pixi/envs/claude-shim"): + assert owned_by_the_image not in guard + + @pytest.mark.parametrize("name", ["provision", "transfer"]) + def test_a_guard_matches_a_whole_line_not_a_fragment_of_one(self, name, tmp_path): + """A mark is only devlaunch's if nothing longer can pass for it: an + exact-line, fixed-string match, so neither a regex metacharacter nor a + longer line that happens to contain the mark counts as a hit.""" + script = ( + provision_script() + if name == "provision" + else tools.transfer_script(fake_payload(tmp_path)) + ) + for guard in self._guards(script): + assert guard.startswith("grep -qxF ") + + def fake_payload(tmp_path) -> tools.HostPayload: """A payload made of scratch files, so no test ships a real 300MB binary.""" claude = tmp_path / "claude-2.0.1" @@ -181,23 +237,499 @@ def fake_payload(tmp_path) -> tools.HostPayload: ) +# What a container in each of the three states reports back over the pipe, +# written out as the probe prints it rather than rebuilt from the module: a +# fixture composed the way the parser splits lines would agree with any format +# the probe ever drifted into. `/ws` stands in for a container's $HOME. +REPORT_ABSENT = "devlaunch-probe tools missing\n" +REPORT_PROVISIONED = ( + "devlaunch-probe tools present\n" + "devlaunch-probe versions /ws/.local/share/claude/versions\n" + "devlaunch-probe claude /ws/.local/share/claude/versions/2.0.1\n" +) +REPORT_LENDABLE = ( + "devlaunch-probe tools present\n" + "devlaunch-probe versions /ws/.local/share/claude/versions\n" + "devlaunch-probe claude /ws/.pixi/envs/claude-shim/bin/claude\n" +) + +# ~/.profile exactly as mcr.microsoft.com/devcontainers/base:ubuntu-24.04 ships +# it -- the base image .devcontainer/Dockerfile builds on -- copied out of the +# pulled image verbatim. The load-bearing part is the last block: Ubuntu's +# default profile puts ~/.local/bin on PATH itself, near the top of the file, +# long before anything devlaunch or the devcontainer feature appends. A guard +# that looks for the *directory* rather than for its own line reads that block +# as its own work and skips an append the workspace needs. +UBUNTU_STOCK_PROFILE = """\ +# ~/.profile: executed by the command interpreter for login shells. +# This file is not read by bash(1), if ~/.bash_profile or ~/.bash_login +# exists. +# see /usr/share/doc/bash/examples/startup-files for examples. +# the files are located in the bash-doc package. + +# the default umask is set in /etc/profile; for setting the umask +# for ssh logins, install and configure the libpam-umask package. +#umask 022 + +# if running bash +if [ -n "$BASH_VERSION" ]; then + # include .bashrc if it exists + if [ -f "$HOME/.bashrc" ]; then +\t. "$HOME/.bashrc" + fi +fi + +# set PATH so it includes user's private bin if it exists +if [ -d "$HOME/bin" ] ; then + PATH="$HOME/bin:$PATH" +fi + +# set PATH so it includes user's private bin if it exists +if [ -d "$HOME/.local/bin" ] ; then + PATH="$HOME/.local/bin:$PATH" +fi +""" + +# What .devcontainer/Dockerfile appends to that same file, verbatim, and in +# this order. The shim directory is prepended *last*, so it wins over +# everything above it -- including Ubuntu's own ~/.local/bin block. +DEVCONTAINER_PROFILE_LINES = """\ +export PATH="$HOME/.pixi/bin:$PATH" +# Workaround: pixi trampoline fails for bash scripts, so add env bin directly +[ -d "$HOME/.pixi/envs/claude-shim/bin" ] && export PATH="$HOME/.pixi/envs/claude-shim/bin:$PATH" +""" + + +class TestProbeScript: + """The probe runs in someone else's container and decides whether to lend. + + Checked as a string (what it may never do) and by running it for real + against scratch `$HOME`s (what it answers), because both halves matter: an + answer that is right for the wrong reason still costs a 285MB download. + """ + + @staticmethod + def _home(tmp_path): + home = tmp_path / "home" + home.mkdir() + return home + + @staticmethod + def _bare_path(tmp_path): + """A PATH carrying the coreutils the probe needs and nothing else. + + A test of "no claude here" must not find the developer's own claude, + so the real system directories are kept off PATH entirely and the one + external the probe uses is linked in by hand. + """ + sysbin = tmp_path / "sysbin" + sysbin.mkdir(exist_ok=True) + for command in ("readlink",): + found = shutil.which(command) + assert found, f"the test host needs {command}" + link = sysbin / command + if not link.exists(): + link.symlink_to(found) + return [sysbin] + + _KEEP_HOME = object() + + def _answer(self, tmp_path, home, path_dirs=(), *, home_env=_KEEP_HOME): + """Run the probe for real and read its answer the way `dl` reads it. + + The script and the parser together, because that pair *is* the probe: + the container reports what it found and this side says what that means, + so a test of either half alone would not notice the two disagreeing. + + `home_env` replaces what `$HOME` is set to for the run, or removes the + variable entirely when it is None. + """ + env = {"PATH": ":".join(str(d) for d in [*path_dirs, *self._bare_path(tmp_path)])} + home_value = str(home) if home_env is self._KEEP_HOME else home_env + if home_value is not None: + env["HOME"] = home_value + # bash by absolute path: the stripped PATH the probe is handed cannot + # also be the one that finds the shell running it. + result = subprocess.run( + [shutil.which("bash") or "/bin/bash", "-c", tools.probe_script()], + env=env, + capture_output=True, + text=True, + check=False, + ) + # Exit 0 in every state: "no tools here" is an answer, not a failure, + # and a non-zero probe paints a red devpod `fatal` on the terminal of + # every cold launch. + assert result.returncode == 0, result.stderr + return tools.ProbeResult.parse(result.stdout) + + def _answer_after_login(self, tmp_path, home): + """The same answer, but with PATH built by the home's own login profile. + + `_probe` runs the script under `bash -lc`, so in a real container every + directory the probe searches was put there by the profile -- the base + image's block, the devcontainer's appended lines and the transfer's + prepend, in whatever order they ended up in the file. Handing the probe + a PATH instead (as the other cases here do, to keep them hermetic) + hides exactly that ordering, which is the thing a lend depends on. + + `$HOME/.profile` is sourced explicitly rather than using `bash -l`, + because `-l` would also source the *test host's* /etc/profile and drag + its system directories -- and whatever `claude` the developer has -- + into a run that is meant to see only this scratch home. The user + profile is the file under test and the only one either script writes. + """ + env = { + "HOME": str(home), + "PATH": ":".join(str(d) for d in self._bare_path(tmp_path)), + } + script = f'. "$HOME/.profile"\n{tools.probe_script()}' + result = subprocess.run( + [shutil.which("bash") or "/bin/bash", "-c", script], + env=env, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr + return tools.ProbeResult.parse(result.stdout) + + @staticmethod + def _official_claude(home, version="2.0.1"): + """What the official installer leaves behind: one binary per version + under ~/.local/share/claude/versions, with ~/.local/bin/claude + pointing at the current one.""" + binary = home / ".local/share/claude/versions" / version + binary.parent.mkdir(parents=True, exist_ok=True) + binary.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + binary.chmod(0o755) + link = home / ".local/bin/claude" + link.parent.mkdir(parents=True, exist_ok=True) + link.symlink_to(binary) + return link.parent + + @staticmethod + def _claude_shim(home): + """What .devcontainer/claude-code/install.sh bakes: something on PATH + answering to `claude` that fetches the real binary on first run.""" + shim = home / ".local/bin/claude" + shim.parent.mkdir(parents=True, exist_ok=True) + shim.write_text("#!/bin/sh\necho 'downloading 285MB' >&2\n", encoding="utf-8") + shim.chmod(0o755) + return shim.parent + + @staticmethod + def _gh(home): + gh = home / ".local/bin/gh" + gh.parent.mkdir(parents=True, exist_ok=True) + gh.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + gh.chmod(0o755) + return gh.parent + + def test_a_container_with_neither_tool_answers_absent(self, tmp_path): + home = self._home(tmp_path) + assert self._answer(tmp_path, home) is tools.ProbeResult.ABSENT + + def test_the_official_claude_layout_answers_provisioned(self, tmp_path): + """The prebuilt-image jackpot: nothing to do, one round trip.""" + home = self._home(tmp_path) + self._official_claude(home) + assert self._answer(tmp_path, home, [self._gh(home)]) is tools.ProbeResult.PROVISIONED + + def test_a_baked_shim_answers_lendable_rather_than_provisioned(self, tmp_path): + """The whole point of the three states. A `claude` that is a downloader + satisfies `command -v` while still owing the ~285MB the lending exists + to avoid, so it must not be mistaken for a provisioned workspace.""" + home = self._home(tmp_path) + self._claude_shim(home) + assert self._answer(tmp_path, home, [self._gh(home)]) is tools.ProbeResult.LENDABLE + + def test_a_real_claude_with_no_gh_is_absent_not_lendable(self, tmp_path): + """`lendable` means "replace the claude"; a container missing gh + outright needs the cold flow, which the network fallback can finish.""" + home = self._home(tmp_path) + self._official_claude(home) + assert self._answer(tmp_path, home, [home / ".local/bin"]) is tools.ProbeResult.ABSENT + + @staticmethod + def _lend(tmp_path, home, version="2.0.1"): + """Run the real transfer script against a scratch $HOME, for real. + + A host payload is built out of scratch binaries, tarred the way + `_transfer` tars it, and streamed into `transfer_script` on stdin -- + so what lands in `home`, and what the script writes into that home's + login profile, is what a real lend leaves behind. + """ + source = tmp_path / f"host-{version}" + source.mkdir(exist_ok=True) + binaries = {} + for name in (f"claude-{version}", "gh"): + binary = source / name + binary.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + binary.chmod(0o755) + binaries[name] = binary + payload = tools.HostPayload( + claude_version=version, + members=( + (binaries[f"claude-{version}"], f".local/share/claude/versions/{version}"), + (binaries["gh"], ".local/bin/gh"), + ), + ) + bundle = tmp_path / f"tools-{version}.tar" + with tarfile.open(bundle, mode="w") as tar: + for member_source, arcname in payload.members: + tar.add(member_source, arcname=arcname) + with open(bundle, "rb") as stream: + lend = subprocess.run( + [shutil.which("bash") or "/bin/bash", "-c", tools.transfer_script(payload)], + stdin=stream, + env={**os.environ, "HOME": str(home)}, + capture_output=True, + text=True, + check=False, + ) + assert lend.returncode == 0, lend.stderr + + def test_the_layout_a_lend_leaves_behind_answers_provisioned(self, tmp_path): + """Convergence, run for real end to end: the transfer script unpacks + into a scratch $HOME, and the probe is then asked about that same home. + + The two scripts have to agree or the lending never terminates -- a lend + the next probe does not recognise means every `up` for the rest of that + workspace's life re-pays the transfer. + """ + home = self._home(tmp_path) + self._lend(tmp_path, home) + assert self._answer(tmp_path, home, [home / ".local/bin"]) is tools.ProbeResult.PROVISIONED + + def test_a_lend_converges_on_the_image_this_repo_ships(self, tmp_path): + """Convergence where it actually has to hold: a shim container whose + login profile is Ubuntu's stock one plus the lines this repo's own + devcontainer appends, with PATH decided by *sourcing that profile* + rather than handed to the probe by the test. + + PATH order is the whole mechanism by which a lend takes effect -- the + lent binary only wins because the transfer prepends `~/.local/bin` to + the profile -- so a convergence test that builds PATH itself asserts a + world in which the profile edit cannot be wrong. This one lets the + profile decide, which is the only way the edit being skipped is + visible: skipped, the shim stays in front, every `up` re-pays the tar, + and the workspace never converges. + """ + home = self._home(tmp_path) + (home / ".profile").write_text( + UBUNTU_STOCK_PROFILE + DEVCONTAINER_PROFILE_LINES, encoding="utf-8" + ) + shim = home / ".pixi/envs/claude-shim/bin/claude" + shim.parent.mkdir(parents=True) + shim.write_text("#!/bin/sh\necho 'downloading 285MB' >&2\n", encoding="utf-8") + shim.chmod(0o755) + gh = home / ".pixi/bin/gh" + gh.parent.mkdir(parents=True) + gh.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + gh.chmod(0o755) + + assert self._answer_after_login(tmp_path, home) is tools.ProbeResult.LENDABLE + self._lend(tmp_path, home) + assert self._answer_after_login(tmp_path, home) is tools.ProbeResult.PROVISIONED + # And the second lend never happens: converged means converged, not + # converged-then-drifting-back. + assert self._answer_after_login(tmp_path, home) is tools.ProbeResult.PROVISIONED + + @staticmethod + def _nested_shim(home): + """A downloader that parked itself *inside* the versions directory. + + The official installer puts one binary per version directly in that + directory; anything deeper is somebody else's tree, and a downloader + is free to choose a path that merely starts with the official one. + """ + shim = home / ".local/share/claude/versions/latest/bin/claude" + shim.parent.mkdir(parents=True, exist_ok=True) + shim.write_text("#!/bin/sh\necho 'downloading 285MB' >&2\n", encoding="utf-8") + shim.chmod(0o755) + link = home / ".local/bin/claude" + link.parent.mkdir(parents=True, exist_ok=True) + link.symlink_to(shim) + return link.parent + + def test_a_shim_hiding_inside_the_versions_directory_is_lendable(self, tmp_path): + """`under the versions directory` is not the official layout -- being a + direct child of it is. A downloader is free to park itself at + `versions/latest/bin/claude`, and a probe that accepts any depth trusts + it and leaves the container owing the download the lend exists to + remove -- the exact illegal state the three-state probe replaced.""" + home = self._home(tmp_path) + self._nested_shim(home) + assert self._answer(tmp_path, home, [self._gh(home)]) is tools.ProbeResult.LENDABLE + + @pytest.mark.parametrize("layout", ["_official_claude", "_claude_shim", "_nested_shim"]) + def test_neither_side_of_the_pipe_can_disagree_about_one_tree( + self, tmp_path, monkeypatch, layout + ): + """One definition of "the official install", asked from both ends. + + The container decides whether to keep what it has; the host decides + what it may lend. Those are the same question about the same layout, + and an answer of `provisioned` for a tree the host would refuse to lend + is a shim nothing will ever replace -- so the two are pinned together + rather than separately. + """ + home = self._home(tmp_path) + getattr(self, layout)(home) + self._gh(home) + container = self._answer(tmp_path, home, [home / ".local/bin"]) + monkeypatch.setattr(tools.pathlib.Path, "home", staticmethod(lambda: home)) + host = tools._claude_source() # pylint: disable=protected-access + assert (container is tools.ProbeResult.PROVISIONED) is (host is not None) + + def test_a_real_install_reached_through_a_symlinked_home_is_provisioned(self, tmp_path): + """Images reach `$HOME` through a symlink often enough to matter, and + resolving the claude while comparing against an unresolved `$HOME` + makes a genuine install read `lendable` forever: the lend succeeds, + changes nothing the next probe recognises, and is re-paid on every + single `up` for the life of the workspace.""" + real = tmp_path / "real-home" + real.mkdir() + self._official_claude(real) + self._gh(real) + home = tmp_path / "home-link" + home.symlink_to(real) + answer = self._answer(tmp_path, home, [home / ".local/bin"]) + assert answer is tools.ProbeResult.PROVISIONED + + def test_a_trailing_slash_on_home_does_not_hide_the_official_install(self, tmp_path): + """A `$HOME` written with a trailing slash is a legal value of the + variable and names the same directory, so it must not turn a real + install into a lend.""" + home = self._home(tmp_path) + self._official_claude(home) + self._gh(home) + answer = self._answer(tmp_path, home, [home / ".local/bin"], home_env=f"{home}/") + assert answer is tools.ProbeResult.PROVISIONED + + def test_a_container_with_no_home_set_still_answers(self, tmp_path): + """`exits 0 in every state` has to survive an image that never set + `HOME`: under `set -u` an unguarded expansion aborts the script, which + is the red devpod `fatal` this probe was written to retire.""" + home = self._home(tmp_path) + self._official_claude(home) + assert ( + self._answer(tmp_path, home, [self._gh(home)], home_env=None) + is tools.ProbeResult.LENDABLE + ) + + def test_it_resolves_the_link_rather_than_trusting_the_path_entry(self): + """In the official layout `~/.local/bin/claude` is a symlink, so what + it points at is the only thing that says which install it belongs to.""" + assert "readlink -f" in tools.probe_script() + + def test_it_never_runs_the_candidate_claude(self): + """Shim-proofness is a property of the script's text, not of a run: + *any* invocation of the shim triggers the download the probe exists to + detect, so the only things the probe may do with `claude` are look its + path up and resolve it. + """ + script = tools.probe_script() + scrubbed = ( + script.replace("command -v claude", "") + .replace(tools.CLAUDE_VERSIONS_RELPATH, "") + .replace("devlaunch-probe claude", "") + ) + assert "claude" not in scrubbed + assert "--version" not in script + + def test_the_container_is_never_asked_to_name_a_state(self): + """The container reports two resolved paths and no verdict. + + A token would mean it had decided, and deciding needs its own copy of + "the official layout" -- the second opinion that let a shim parked + deeper under the versions directory be trusted by the container while + the host refused to lend the very same tree. + """ + script = tools.probe_script() + for state in tools.ProbeResult: + assert state.value not in script + + def test_the_official_layout_is_defined_once_for_both_sides_of_the_pipe(self, tmp_path): + """The container-side "is this the official install" and the host-side + one it mirrors must not be able to drift apart, so there is one of it: + both go through `_is_official_claude`, and the constant it is asked + about is likewise stated once.""" + assert tools.CLAUDE_VERSIONS_RELPATH == ".local/share/claude/versions" + assert tools.CLAUDE_VERSIONS_RELPATH in tools.probe_script() + assert tools.CLAUDE_VERSIONS_RELPATH in tools.transfer_script(fake_payload(tmp_path)) + + +class TestProbeResult: + """The probe's answer is one value with three states, not a boolean.""" + + def test_it_reads_the_state_out_of_what_the_container_reported(self): + assert tools.ProbeResult.parse(REPORT_ABSENT) is tools.ProbeResult.ABSENT + assert tools.ProbeResult.parse(REPORT_PROVISIONED) is tools.ProbeResult.PROVISIONED + assert tools.ProbeResult.parse(REPORT_LENDABLE) is tools.ProbeResult.LENDABLE + + def test_a_claude_deeper_under_the_versions_directory_is_not_the_install(self): + """`under` is not `in`. The installer writes one binary per version + directly into that directory, so a path that merely starts with it is + somebody else's -- a downloader's, in the case this ticket exists for.""" + report = ( + "devlaunch-probe tools present\n" + "devlaunch-probe versions /ws/.local/share/claude/versions\n" + "devlaunch-probe claude /ws/.local/share/claude/versions/latest/bin/claude\n" + ) + assert tools.ProbeResult.parse(report) is tools.ProbeResult.LENDABLE + + def test_a_container_that_could_resolve_nothing_is_not_provisioned(self): + """Two blanks are equal, and equality is the whole test -- so a + container with no `readlink`, which resolves neither path, must not + come out as the one perfect match.""" + report = "devlaunch-probe tools present\ndevlaunch-probe versions\ndevlaunch-probe claude\n" + assert tools.ProbeResult.parse(report) is tools.ProbeResult.LENDABLE + + def test_a_chatty_login_profile_does_not_hide_the_answer(self): + """The probe runs under `bash -lc`, so the container's profile is + sourced first and an image whose profile prints a banner puts that on + the same stdout. The marked lines are what the report is, which keeps + such an image on the one-trip path instead of re-provisioning it + forever.""" + report = "Welcome to this image!\n\n" + REPORT_PROVISIONED + assert tools.ProbeResult.parse(report) is tools.ProbeResult.PROVISIONED + + @pytest.mark.parametrize( + "report", + [ + "", + "bash: line 1: oh no", + "provisioned", + "devlaunch-probe tools\n", + "tools present\nversions /ws/.local/share/claude/versions\n", + ], + ) + def test_a_report_it_cannot_read_means_absent(self, report): + """Parsing is total, and it errs towards doing the work again: + provisioning is idempotent, so a wrong `absent` costs a redundant trip + where a wrong `provisioned` would silently skip the whole point.""" + assert tools.ProbeResult.parse(report) is tools.ProbeResult.ABSENT + + @pytest.mark.usefixtures("no_host_payload") class TestEnsureTools: """The three-trip flow: probe, host transfer, network install.""" def test_a_provisioned_workspace_pays_one_probe_and_nothing_else(self): """The common path: every launch after the first is one round trip.""" - runner = Runner(returncodes=(0,)) + runner = Runner(returncodes=(0,), stdout=REPORT_PROVISIONED) assert ensure_tools("myws", runner) is True assert len(runner.calls) == 1 assert runner.calls[0][:2] == ["ssh", "myws"] # A non-login shell has no ~/.pixi/bin, so every tool would look absent. - assert runner.script().startswith("bash -lc ") - assert "command -v gh" in runner.script() - assert "command -v claude" in runner.script() + argv = shlex.split(runner.script()) + assert argv[:2] == ["bash", "-lc"] + assert argv[2] == tools.probe_script() def test_with_nothing_to_lend_a_cold_workspace_gets_the_network_install(self): - runner = Runner(returncodes=(1, 0)) + runner = Runner(returncodes=(0, 0), stdout=REPORT_ABSENT) assert ensure_tools("myws", runner) is True assert len(runner.calls) == 2 # shlex.split is the inverse of the quoting ensure_tools applies. @@ -205,8 +737,17 @@ def test_with_nothing_to_lend_a_cold_workspace_gets_the_network_install(self): assert argv[:2] == ["bash", "-lc"] assert argv[2] == provision_script() + def test_a_probe_trip_that_fails_outright_is_read_as_absent(self): + """The script exits 0 in all three states, so a non-zero trip is the + trip failing rather than an answer -- and the cold flow is the safe + reading of no answer at all.""" + runner = Runner(returncodes=(1, 0), stdout=REPORT_PROVISIONED) + assert ensure_tools("myws", runner) is True + assert len(runner.calls) == 2 + assert shlex.split(runner.script(1))[2] == provision_script() + def test_a_host_with_the_tools_lends_them_instead_of_the_network(self, tmp_path): - runner = Runner(returncodes=(1, 0)) + runner = Runner(returncodes=(0, 0), stdout=REPORT_ABSENT) with patch.object(tools, "host_payload", return_value=fake_payload(tmp_path)): assert ensure_tools("myws", runner) is True assert len(runner.calls) == 2 @@ -217,22 +758,48 @@ def test_a_host_with_the_tools_lends_them_instead_of_the_network(self, tmp_path) def test_a_transfer_the_container_rejects_falls_back_to_the_network(self, tmp_path): """The lent binaries may not run there (arch, libc); the gate at the end of the transfer script reports it, and the old path still runs.""" - runner = Runner(returncodes=(1, 1, 0)) + runner = Runner(returncodes=(0, 1, 0), stdout=REPORT_ABSENT) with patch.object(tools, "host_payload", return_value=fake_payload(tmp_path)): assert ensure_tools("myws", runner) is True assert len(runner.calls) == 3 assert shlex.split(runner.script(2))[2] == provision_script() + def test_a_shim_workspace_is_lent_the_hosts_real_claude(self, tmp_path): + """`lendable` is the state this whole ticket exists for: both tools + answer, but the claude is a downloader, so the host replaces it.""" + runner = Runner(returncodes=(0, 0), stdout=REPORT_LENDABLE) + with patch.object(tools, "host_payload", return_value=fake_payload(tmp_path)): + assert ensure_tools("myws", runner) is True + assert len(runner.calls) == 2 + assert [stream is not None for stream in runner.streams] == [False, True] + assert "tar xf -" in runner.script(1) + + def test_a_shim_the_lend_could_not_replace_is_accepted_not_reinstalled(self, tmp_path): + """The container could not run the lent binaries, but it does have a + claude and a gh. The network fallback decides what to install with its + own `command -v` guards, which both already satisfy, so a third trip + would install nothing -- it is not taken.""" + runner = Runner(returncodes=(0, 1), stdout=REPORT_LENDABLE) + with patch.object(tools, "host_payload", return_value=fake_payload(tmp_path)): + assert ensure_tools("myws", runner) is True + assert len(runner.calls) == 2 + + def test_a_shim_with_nothing_to_lend_stops_after_the_probe(self): + """Nothing on the host to replace it with, and the network fallback + would no-op, so the shim stands and the launch costs one trip.""" + runner = Runner(returncodes=(0,), stdout=REPORT_LENDABLE) + assert ensure_tools("myws", runner) is True + assert len(runner.calls) == 1 + def test_the_install_output_reaches_the_user_but_the_probe_is_silent(self): """A cold install is tens of seconds; captured, it reads as a hung dl. The scripts' progress lines are the only thing on the terminal during it, so capturing them would be the same as having none. - The probe is the exception: it reports nothing, and its everyday - failure on a cold workspace reaches the terminal as a red devpod - `fatal` describing the probe doing its job. + The probe is the exception: its output is not progress but the answer + the caller branches on, which is exactly what has to be read back. """ - runner = Runner(returncodes=(1, 0)) + runner = Runner(returncodes=(0, 0), stdout=REPORT_ABSENT) ensure_tools("myws", runner) assert runner.captured == [True, False]