diff --git a/.github/pr-images/issue-2552-windows-fallback-verification.png b/.github/pr-images/issue-2552-windows-fallback-verification.png new file mode 100644 index 000000000..91a1dc27e Binary files /dev/null and b/.github/pr-images/issue-2552-windows-fallback-verification.png differ diff --git a/headroom/cli/install.py b/headroom/cli/install.py index 2d9402054..50e897ad4 100644 --- a/headroom/cli/install.py +++ b/headroom/cli/install.py @@ -139,6 +139,12 @@ def _require_manifest(profile: str) -> DeploymentManifest: raise _missing_profile_error(profile, installed) +def _is_windows() -> bool: + """Return whether this command is running on Windows.""" + + return sys.platform.startswith("win") + + def _start_deployment(manifest: DeploymentManifest, *, assume_start_lock: bool = False) -> None: if not assume_start_lock: with acquire_runtime_start_lock(manifest.profile) as acquired: @@ -658,6 +664,16 @@ def install_apply( bedrock_profile=bedrock_profile, extra_env=combined_env, ) + if ( + preset == InstallPreset.PERSISTENT_SERVICE.value + and manifest.preset == InstallPreset.PERSISTENT_TASK.value + and _is_windows() + ): + click.echo( + "Warning: persistent-service is not supported on Windows because the " + "Python runner cannot act as a Windows service. Falling back to " + "persistent-task with Task Scheduler." + ) _apply_manifest(manifest) _echo_installed(manifest) diff --git a/headroom/cli/wrap.py b/headroom/cli/wrap.py index 0a240c2d0..8f1971a13 100644 --- a/headroom/cli/wrap.py +++ b/headroom/cli/wrap.py @@ -795,7 +795,7 @@ _RETIRED_CONTEXT_TOOL_MESSAGE = ( "rewrote shell commands through a third-party binary Headroom no longer " "manages. Drop --context-tool / --no-context-tool and unset " f"{_RETIRED_CONTEXT_TOOL_ENV}; `headroom wrap` uninstalls what they left " - "behind on first run." + "behind automatically." ) @@ -855,8 +855,10 @@ def _report_context_tool_purge() -> None: default: the Claude ``PreToolUse`` hook, the vendored binaries and the injected hint-file guidance are all durable on disk. Running this once per ``wrap`` / ``unwrap`` invocation is what actually makes the tools go away. - Silent when there is nothing to do, which is the steady state after the first - run, and never fatal — a cleanup failure must not block launching the tool. + Silent when there is nothing to do — the common case once the machine-global + half is stamped done, though the project- and config-directory-scoped half + still runs every launch — and never fatal: a cleanup failure must not block + launching the tool. Reports on **stderr**: some subcommands (``wrap/unwrap openclaw --prepare-only``) emit machine-readable JSON on stdout as their entire diff --git a/headroom/context_tool_cleanup.py b/headroom/context_tool_cleanup.py index d6860d6ff..6dbeb2f14 100644 --- a/headroom/context_tool_cleanup.py +++ b/headroom/context_tool_cleanup.py @@ -10,26 +10,87 @@ Deleting the code is not enough: everything above is *durable state on the user's disk*. Left alone, the Claude hooks keep rewriting every Bash command through binaries Headroom no longer manages, and the injected guidance keeps telling agents to use tools that may not resolve. So ``headroom wrap`` / -``headroom unwrap`` call :func:`purge_context_tool_artifacts` once per run to -remove what earlier versions installed. +``headroom unwrap`` call :func:`purge_context_tool_artifacts` on every run to +remove what earlier versions installed — machine-global artifacts (hooks, +binaries, Claude Code's MCP registration) are removed once per workspace and +then stamped done (see the stamp below), while project- and config-directory- +scoped guidance (``CODEX_HOME`` / ``OPENCODE_HOME`` hint files, Continue's +config) is inspected on every launch, since a later launch can sit in a +different project or point at a different ``CODEX_HOME`` / ``OPENCODE_HOME``. Everything here is idempotent, best-effort and deliberately conservative: * only files Headroom installed (or caused a context tool to install) are - deleted; + deleted. An MCP entry's ``command`` or a hook script's body counts as + Headroom's only when it names a path inside :func:`paths.bin_dir` + (:func:`_references_managed_bin` — which is where the matching rules and + the reasons behind them live); +* a hook entry is Headroom's when it names such a path directly, or — the + common case, since a hook command names a script rather than the binary — + when it names one of the ``~/.claude/hooks`` scripts already classified as + Headroom's, whose verdict it inherits + (:func:`_references_context_tool`, :func:`_names_a_managed_script`). A + Cursor ``hooks.json`` entry naming a script under ``~/.cursor`` cannot + inherit a verdict this way, since the map covers ``~/.claude/hooks`` only; + it is still caught when its ``command`` names the managed directory; +* ``.rtk-hook.sha256`` is never read for its own provenance (it holds a hex + digest, not a path) and instead inherits ``rtk-rewrite.sh``'s + classification; a ``.lean-ctx.bak`` backup inherits ````'s + (:func:`_classify_hook_scripts`); +* a hook script that exists but cannot be read is classified unknown — + deleted by nothing, and named in the report so the user can remove it by + hand; * ``~/.local/bin/{rtk,lean-ctx}`` is unlinked only when it is a symlink into Headroom's own bin directory — a user's own build is never touched; * a JSON config that does not parse is reported and **skipped**, never overwritten (a hand-edited typo must not cost the user their settings); * the tools' own backups of *config* files (``~/.claude.json.lean-ctx.bak`` and friends) are left in place — they hold the user's real settings history. Only - backups of the hook scripts being deleted are cleaned up. + backups of the hook scripts proven to be Headroom's are cleaned up; +* two cases cannot be decided at all, and are accepted as limits rather than + fixed: + + * ``get_lean_ctx_path`` used to check ``PATH`` before Headroom's own bin + directory, so on a machine that already had ``lean-ctx`` on ``PATH``, + the tool that ran was the user's own, and the config it wrote looks + exactly like config the user wrote by hand. That leftover survives the + purge — it still points at a binary that exists, so nothing dangles; + * an rtk hook written *after* #1698 execs a bare ``rtk`` and never mentions + :func:`paths.bin_dir`, so it reads exactly like a hook a user wrote by + hand, and ``rtk-rewrite.sh``, its ``.rtk-hook.sha256`` and its + ``settings.json`` entry all survive while step 3 removes the managed + binary — leaving a hook that silently no-ops (#487, #1698). Earlier + hooks are decidable: Headroom patched the absolute managed path into + them (``_patch_rtk_hook_absolute_path``, removed by #1698), so the + window this misses is rtk setups run between #1698 and the tools' + removal in #2677; +* the marker-fenced guidance block is the one step with no provenance check + to make — ```` is Headroom's own fence, + and no third party writes it. + +Removing the retired integration's machine-global footprint — hook +registrations, hook scripts, PATH symlinks, managed binaries and Claude +Code's own MCP registration — is a one-time migration: the first completed +run of that half stamps ``.context-tools-purged`` beside the managed bin +directory, and every later run skips that half outright. Without the stamp +this would keep rewriting the same machine-wide files on every ``wrap`` +invocation forever, and a user who installs one of these tools *after* the +migration would have Headroom auditing files at each launch for a leftover +that cannot exist there. + +Project- and config-directory-scoped state is not covered by that stamp: a +later invocation can sit in a different project, or point ``CODEX_HOME`` / +``OPENCODE_HOME`` somewhere the stamped run never inspected, and whatever +guidance an earlier Headroom left behind there is still worth removing — so +those steps run on every invocation instead (:func:`_purge_invocation_scoped`). """ from __future__ import annotations import json import os +import posixpath +import re from pathlib import Path from typing import Any @@ -69,52 +130,138 @@ _HOOK_SCRIPTS = ( "lean-ctx-redirect-native", ) +# rtk's integrity digest never names a path (see ``_classify_hook_scripts``) +# and rtk-rewrite.sh is the script it authenticates. +_RTK_DIGEST_NAME = ".rtk-hook.sha256" +_RTK_SCRIPT_NAME = "rtk-rewrite.sh" + # MCP server entries the tools registered, and the config files holding them. # lean-ctx registers itself as an MCP server during ``lean-ctx init``; rtk never # did, but it is matched too so a stale hand-added entry is cleaned up as well. _MCP_SERVER_NAMES = ("lean-ctx", "lean_ctx", "rtk") +# Report-line prefixes meaning "this one is not settled" — a config that would +# not parse, a script that would not read, a file that would not unlink. Such a +# run leaves a leftover behind, so it must not be stamped as the completed +# migration. Emitted by _purge_hook_config, _purge_mcp_entries, +# _purge_fenced_block, _purge_continue_system_messages and _remove_files. +_DEFERRED_PREFIXES = ("skipped ", "could not remove ") + def purge_context_tool_artifacts() -> list[str]: """Remove every rtk / lean-ctx artifact an earlier Headroom version installed. Returns human-readable descriptions of what was removed — plus a line for - any config that had to be skipped because the user must fix it by hand. An - empty list means there was nothing to do, which is the steady state after - the first run. + any config that had to be skipped because the user must fix it by hand. + Machine-global cleanup (hooks, binaries, Claude Code's MCP registration) + runs once and is then skipped via the stamp below; project- and config- + directory-scoped cleanup (hint files, ``CODEX_HOME`` / ``OPENCODE_HOME``, + Continue's config) runs on every call, so a later call in a different + project or a repointed ``CODEX_HOME`` / ``OPENCODE_HOME`` can still report + something even after the global half is long since stamped done. """ + marker = _purge_marker() home = Path.home() project = Path.cwd() report: list[str] = [] - # 1. Hook registrations (Claude Code's settings.json, Cursor's hooks.json). - for config in (home / ".claude" / "settings.json", home / ".cursor" / "hooks.json"): - report += _purge_hook_config(config) + if not marker.exists(): + # Classify every hook script's provenance once, up front: both step 1 + # (is a settings.json entry pointing at *our* script?) and step 2 (is + # the script itself ours?) need the same answer, and each file is + # read once. + hooks_dir = home / ".claude" / "hooks" + hook_classification = _classify_hook_scripts(hooks_dir) - # 2. The generated hook scripts, their integrity digests and stale backups. - hooks_dir = home / ".claude" / "hooks" - report += _remove_files( - *(hooks_dir / name for name in _HOOK_SCRIPTS), - *(hooks_dir / f"{name}.lean-ctx.bak" for name in _HOOK_SCRIPTS), - ) + # 1. Hook registrations (Claude Code's settings.json, Cursor's hooks.json). + for config in (home / ".claude" / "settings.json", home / ".cursor" / "hooks.json"): + report += _purge_hook_config(config, hooks_dir, hook_classification) - # 3. The PATH symlinks, then the managed binaries they pointed at. - for name in ("rtk", "lean-ctx"): - report += _remove_managed_path_link(home / ".local" / "bin" / name) - report += _remove_files(*(paths.bin_dir() / name for name in _BINARY_NAMES)) + # 2. The generated hook scripts, their integrity digests and stale + # backups — only the ones proven to reference Headroom's managed bin + # directory. + managed_names = [name for name in _HOOK_SCRIPTS if hook_classification.get(name)] + report += _remove_files( + *(hooks_dir / name for name in managed_names), + *(hooks_dir / f"{name}.lean-ctx.bak" for name in managed_names), + ) + for name in _HOOK_SCRIPTS: + if hook_classification.get(name, False) is not None: + continue + if name == _RTK_DIGEST_NAME: + report.append( + f"skipped {hooks_dir / name} (inherits {_RTK_SCRIPT_NAME}'s unreadable verdict)" + " — remove any stale hook script by hand" + ) + else: + report.append( + f"skipped {hooks_dir / name} (could not read to verify it was Headroom's)" + " — remove any stale hook script by hand" + ) - # 4. MCP server registrations (lean-ctx registers itself during init). - report += _purge_mcp_entries(home / ".claude.json", "mcpServers") + # 3. The PATH symlinks, then the managed binaries they pointed at. + for name in ("rtk", "lean-ctx"): + report += _remove_managed_path_link(home / ".local" / "bin" / name) + report += _remove_files(*(paths.bin_dir() / name for name in _BINARY_NAMES)) + + # 4. Claude Code's own MCP server registration (lean-ctx registers + # itself during init). OpenCode's is invocation-scoped — see below. + report += _purge_mcp_entries(home / ".claude.json", "mcpServers") + + # Only a global half that settled everything is the completed + # migration. One that could not read a script or parse a config left + # a leftover behind, and the user needs both the reminder on the next + # launch and the cleanup once the permissions or the typo are fixed. + # A deferral in the invocation-scoped half below must not withhold + # this stamp — that half re-runs every time regardless, so nothing is + # lost by stamping the global half done now. + if not any(line.startswith(_DEFERRED_PREFIXES) for line in report): + try: + # Cleanup must not become the first mutation on a pristine + # machine. In particular, ``wrap `` validates + # the binary after the wrap-group migration hook; creating + # ``~/.headroom`` merely to stamp an empty scan violates that + # command's no-side-effects-on-failure contract. Established + # Headroom installs already have the state directory and get + # the one-time fast path; clean machines cheaply rescan until + # some real Headroom state exists. + if marker.parent.is_dir(): + marker.touch() + except OSError: + pass # Unwritable workspace: the purge simply runs again next time. + + report += _purge_invocation_scoped(home, project) + return report + + +def _purge_invocation_scoped(home: Path, project: Path) -> list[str]: + """Steps the one-time stamp must never withhold. + + ``OPENCODE_HOME``'s config, the hint files in ``project`` / + ``CODEX_HOME`` / ``OPENCODE_HOME``, and Continue's config are all a + function of *this* invocation's cwd and environment, not of the machine — + a later run can sit in a different project or point ``CODEX_HOME`` / + ``OPENCODE_HOME`` somewhere the global-half stamp never inspected. Each + step is cheap and side-effect-free when nothing matches, so re-running + them on every invocation costs a handful of reads in the steady state. + """ + report: list[str] = [] report += _purge_mcp_entries(_opencode_home(home) / "opencode.json", "mcp") - - # 5. Marker-fenced guidance in every hint file the wrap harnesses wrote to. for hint_file in _instruction_files(home, project): report += _purge_fenced_block(hint_file) report += _purge_continue_system_messages(project / ".continue" / "config.json") - return report +def _purge_marker() -> Path: + """Path of the "already migrated" stamp. + + Derived from :func:`paths.bin_dir` rather than ``workspace_dir`` so it + cannot escape a temporary tree through ``HEADROOM_WORKSPACE_DIR``. + """ + return paths.bin_dir().parent / ".context-tools-purged" + + def _instruction_files(home: Path, project: Path) -> list[Path]: """Hint files the wrap subcommands injected the context-tool block into. @@ -151,15 +298,194 @@ def _opencode_home(home: Path) -> Path: # --- hook registrations ------------------------------------------------------- -def _references_context_tool(entry: Any) -> bool: - """Whether a hook entry's command is one a retired context tool registered.""" +# Characters that can never be part of a path: a match ending right before +# one of these (or at end-of-string) sits at a real word boundary. Quotes, +# `=`/`:` (`export PATH=":$PATH"`, `BIN=/x`), `;`/`,`/`|` (command +# joiners) and `()` (subshells) all end a path the same way whitespace does. +_PATH_BOUNDARY_CHARS = frozenset("\"'=:;,()|") + +# Splits a command into path-shaped tokens on whitespace plus the same +# boundary punctuation above — used by _names_a_managed_script, which (unlike +# _references_managed_bin's needle-anchored scan) tokenizes the whole command. +_PATH_TOKEN_SPLIT = re.compile(r"[\s" + re.escape("".join(_PATH_BOUNDARY_CHARS)) + r"]+") + + +def _norm_path_text(value: str) -> str: + """Case-fold ``value`` and give it one separator, so paths compare as text.""" + return os.path.normcase(value).replace("\\", "/") + + +def _references_managed_bin(text: str) -> bool: + """Whether ``text`` names a path inside Headroom's managed bin directory. + + A hook command or script body is free text we don't control, so this + scans ``text`` for raw occurrences of the managed directory — the + unresolved and resolved bin directory, and its ``~``-relative form + (home-relative, since a script may reference it unexpanded) — matched + against the text as given and as ``expanduser``'d, case-folded, with both + path separators. Deliberately *not* tokenized on whitespace first: a + quoted or ``$HOME``-derived path can itself contain a space, and slicing + the text into words before searching would sever it. + + A hit is only a real reference at a path boundary on *both* ends. The + character immediately before the match, if any, must be whitespace or a + :data:`_PATH_BOUNDARY_CHARS` character, or the match is just the tail of + some longer, unrelated path segment (e.g. ``/prefix/lean-ctx``) + and is rejected. The match must then be immediately followed by + end-of-string or a :data:`_PATH_BOUNDARY_CHARS` character (an exact + reference, e.g. a bare ``PATH=`` export), or by ``/`` — in which + case the run of characters up to the next boundary is lexically + normalized (``.``/``..`` collapsed) and re-compared, so neither a sibling + directory like ``bin-backup``/``binfoo`` nor a ``bin/../evil`` traversal + can borrow the managed prefix. + # ponytail: boundary-aware substring scan, not a shell parse — upgrade to + # shlex if a command ever embeds a managed path it does not execute. + """ + try: + bin_dir = paths.bin_dir() + resolved_bin_dir = bin_dir.resolve() + except OSError: + return False + + needles = {_norm_path_text(str(bin_dir)), _norm_path_text(str(resolved_bin_dir))} + home = Path.home() + for base in (bin_dir, resolved_bin_dir): + try: + needles.add(_norm_path_text(f"~/{base.relative_to(home).as_posix()}")) + except ValueError: + pass + + for haystack in (text, os.path.expanduser(text)): + normalized_haystack = _norm_path_text(haystack) + if any(_names_managed_dir(normalized_haystack, needle) for needle in needles): + return True + return False + + +def _names_managed_dir(haystack: str, needle: str) -> bool: + """Whether a normalized ``haystack`` names ``needle`` at a path boundary + on both ends: the character right before the match, if any, must be + whitespace or a :data:`_PATH_BOUNDARY_CHARS` character too, or a + user-owned path that merely has the managed directory as a substring + (e.g. ``/prefix/lean-ctx``) would be misread as naming it. + """ + search_from = 0 + while True: + index = haystack.find(needle, search_from) + if index < 0: + return False + end = index + len(needle) + search_from = index + 1 # keep scanning; occurrences may overlap + if index > 0 and not ( + haystack[index - 1].isspace() or haystack[index - 1] in _PATH_BOUNDARY_CHARS + ): + continue + following = haystack[end : end + 1] + if not following or following.isspace() or following in _PATH_BOUNDARY_CHARS: + return True + if following != "/": + continue + tail_end = end + while tail_end < len(haystack) and not ( + haystack[tail_end].isspace() or haystack[tail_end] in _PATH_BOUNDARY_CHARS + ): + tail_end += 1 + candidate = posixpath.normpath(haystack[index:tail_end]) + if candidate == needle or candidate.startswith(needle + "/"): + return True + + +def _classify_hook_scripts(hooks_dir: Path) -> dict[str, bool | None]: + """Classify each existing ``_HOOK_SCRIPTS`` file by whether it is Headroom's. + + ``True`` — the file exists and its body references the managed bin + directory. ``False`` — it exists and does not. ``None`` — it exists but + could not be read, so its provenance is unprovable. A basename with no + file on disk is simply absent from the map. Each file is read at most once. + + ``.rtk-hook.sha256`` holds a hex digest that can never reference a path, + so it is never read; it inherits ``rtk-rewrite.sh``'s classification. + """ + classification: dict[str, bool | None] = {} + for name in _HOOK_SCRIPTS: + if name == _RTK_DIGEST_NAME: + continue + path = hooks_dir / name + if not path.is_file(): + continue + try: + body = fsutil.read_text(path) + except OSError: + classification[name] = None + continue + classification[name] = _references_managed_bin(body) + + if (hooks_dir / _RTK_DIGEST_NAME).is_file(): + classification[_RTK_DIGEST_NAME] = classification.get(_RTK_SCRIPT_NAME, False) + + return classification + + +def _references_context_tool( + entry: Any, hooks_dir: Path, hook_classification: dict[str, bool | None] +) -> bool: + """Whether a hook entry is one a retired context tool registered. + + A command-marker hit alone is not enough — a user could author a script + with a matching name. It must also either name the managed bin directory + directly, or name — as a path resolving to that exact file, not merely + sharing its basename, so a same-named script of the user's own in a + different directory is never caught — a hook script in ``hooks_dir`` the + classification map marks ``True`` (see :func:`_names_a_managed_script`). + """ if not isinstance(entry, dict): return False - command = str(entry.get("command", "")).lower() - return any(marker in command for marker in _HOOK_COMMAND_MARKERS) + command = str(entry.get("command", "")) + if not any(marker in command.lower() for marker in _HOOK_COMMAND_MARKERS): + return False + if _references_managed_bin(command): + return True + return _names_a_managed_script(command, hooks_dir, hook_classification) -def _prune_hooks(hooks: Any) -> tuple[Any, bool]: +def _names_a_managed_script( + command: str, hooks_dir: Path, hook_classification: dict[str, bool | None] +) -> bool: + """Whether ``command`` names, by absolute path, a hook script the map marks ``True``. + + A real command is rarely the bare script path: ``bash