mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(windows): pin UTF-8 encoding on text-mode subprocess calls (#1311)
Fixes #1310. ## Description On Windows, `headroom` startup crashes a subprocess reader thread: ``` UnicodeDecodeError: 'charmap' codec can't decode byte 0x8d in position 7894: character maps to <undefined> ... subprocess.py _readerthread -> buffer.append(fh.read()) ... encodings/cp1252.py ``` Text-mode `subprocess` calls omit `encoding=`, so Python decodes child output with the locale codec (**cp1252** on Windows). Children that emit UTF-8 ??? `cbm index_repository` (indexing sources with chars like `???`/`???`), `claude mcp get/add`, the memory-sync process ??? produce bytes invalid in cp1252 and kill the reader thread. Linux/macOS default to UTF-8, so it's invisible there. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - Add `encoding="utf-8", errors="replace"` to every text-mode (`text=True` / `universal_newlines=True`) subprocess call in the `headroom/` package (~50 call sites; several already had it). - `errors="replace"` (not `ignore`) so corrupt bytes surface as `???` rather than vanishing from parsed output. - Add `tests/test_cli/test_subprocess_utf8_encoding.py`: an AST guard asserting every text-mode subprocess call pins `encoding=`. The runtime crash can't reproduce on UTF-8 CI, so the invariant is enforced at the source level instead. ## Testing - [x] Unit tests pass (`pytest`) - New guard test passes (validates 51 call sites). - `tests/test_install`, `tests/test_cli/test_mcp.py`, `tests/test_mcp_registry` pass. (`test_runtime_start_lock_blocks_another_process` fails on this Windows box, but it fails identically on unmodified `main` ??? a pre-existing `msvcrt` lock flake, unrelated.) ### Test Output ```text > python -m pytest tests/test_cli/test_subprocess_utf8_encoding.py -q 1 passed in 0.12s > python -m pytest tests/test_install/ tests/test_cli/test_mcp.py tests/test_mcp_registry/ -q 133 passed, 2 skipped in 15.34s ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.13.13. - Exact command / steps: Started `headroom` without `PYTHONUTF8=1` on a repo with UTF-8 chars in indexable files. Observed the `UnicodeDecodeError` crash. Applied the fix (pinning `encoding="utf-8"` on all text-mode subprocess calls). Re-ran. No crash. The AST guard enforces the invariant on CI (which runs UTF-8 locales and cannot reproduce the cp1252 crash natively). - Observed result: Subprocess reader threads no longer crash on UTF-8 output under cp1252 locale. - Not tested: All third-party tools that `headroom` shells out to; each was given `errors="replace"` as a safety net. ## Workaround for affected users (before fix is deployed) `PYTHONUTF8=1` (PowerShell: `$env:PYTHONUTF8=1; headroom ...`). ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Parideboy
GitHub
parent
7c93c50c2c
commit
d633e8172c
26 changed files with 267 additions and 102 deletions
36
.github/actions/headroom-e2e-setup/action.yml
vendored
36
.github/actions/headroom-e2e-setup/action.yml
vendored
|
|
@ -51,6 +51,42 @@ runs:
|
|||
with:
|
||||
workspaces: ". -> target"
|
||||
|
||||
# macos-latest (macos-15) runners have varying Xcode versions installed.
|
||||
# The Rust cc crate probes the active Xcode for
|
||||
# .../lib/clang/<ver>/lib/darwin/libclang_rt.osx.a. Some Xcode versions
|
||||
# (notably 16.4 / clang 17 on certain runner images) lack this path.
|
||||
# 1. Find an Xcode whose clang runtime directory actually exists.
|
||||
# 2. If none found, locate libclang_rt.osx and create the expected symlink.
|
||||
- name: Fix clang_rt.osx linker path (macOS)
|
||||
if: runner.os == 'macOS'
|
||||
shell: bash
|
||||
run: |
|
||||
found=
|
||||
for app in /Applications/Xcode_*.app; do
|
||||
[ -d "$app" ] || continue
|
||||
clang_dir=$(ls -d "$app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/"*/lib/darwin 2>/dev/null | head -1)
|
||||
if [ -n "$clang_dir" ]; then
|
||||
sudo xcode-select -s "$app"
|
||||
echo "Selected Xcode: $app (has clang runtime at $clang_dir)"
|
||||
found=1
|
||||
break
|
||||
fi
|
||||
done
|
||||
if [ -z "$found" ]; then
|
||||
rt_lib=$(find /Applications -name "libclang_rt.osx*" 2>/dev/null | head -1)
|
||||
if [ -n "$rt_lib" ]; then
|
||||
xcode_ver=$(xcodebuild -version 2>/dev/null | head -1 | awk '{print $2}')
|
||||
clang_ver=$(clang --version 2>/dev/null | head -1 | grep -oP 'version \K\d+')
|
||||
exp_dir="/Applications/Xcode_${xcode_ver}.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/${clang_ver}/lib/darwin"
|
||||
sudo mkdir -p "$exp_dir"
|
||||
target="$exp_dir/$(basename "$rt_lib")"
|
||||
[ -f "$target" ] || sudo ln -sf "$rt_lib" "$target"
|
||||
echo "Symlinked $rt_lib -> $target"
|
||||
else
|
||||
echo "WARNING: libclang_rt.osx not found anywhere. Build may fail."
|
||||
fi
|
||||
fi
|
||||
|
||||
- name: Install headroom (editable, with proxy extras — builds Rust extension)
|
||||
if: ${{ inputs.install-mode == 'editable-proxy' }}
|
||||
shell: bash
|
||||
|
|
|
|||
16
headroom/_subprocess.py
Normal file
16
headroom/_subprocess.py
Normal file
|
|
@ -0,0 +1,16 @@
|
|||
import subprocess as _sp
|
||||
from typing import Any
|
||||
|
||||
|
||||
def run(*args: Any, **kwargs: Any) -> _sp.CompletedProcess:
|
||||
if kwargs.get("text") or kwargs.get("universal_newlines"):
|
||||
kwargs.setdefault("encoding", "utf-8")
|
||||
kwargs.setdefault("errors", "replace")
|
||||
return _sp.run(*args, **kwargs)
|
||||
|
||||
|
||||
def Popen(*args: Any, **kwargs: Any) -> _sp.Popen:
|
||||
if kwargs.get("text") or kwargs.get("universal_newlines"):
|
||||
kwargs.setdefault("encoding", "utf-8")
|
||||
kwargs.setdefault("errors", "replace")
|
||||
return _sp.Popen(*args, **kwargs)
|
||||
|
|
@ -37,6 +37,8 @@ from dataclasses import dataclass
|
|||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
from headroom._subprocess import run
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
__all__ = [
|
||||
|
|
@ -111,7 +113,7 @@ def _is_musl() -> bool:
|
|||
which is present on Alpine even when `ldd` is absent.
|
||||
"""
|
||||
try:
|
||||
out = subprocess.run(
|
||||
out = run(
|
||||
["ldd", "--version"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
|
|
|
|||
|
|
@ -16,6 +16,8 @@ from hashlib import sha1
|
|||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
from headroom._subprocess import run
|
||||
|
||||
try:
|
||||
import tomllib
|
||||
except ModuleNotFoundError: # Python < 3.11
|
||||
|
|
@ -604,12 +606,10 @@ def _marketplace_source() -> str:
|
|||
|
||||
def _run_checked(command: list[str], *, action: str) -> None:
|
||||
logger.debug("subprocess [%s]: %s", action, _command_string(command))
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
command,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
errors="replace",
|
||||
)
|
||||
logger.debug(
|
||||
"subprocess [%s] exit=%s stdout=%r stderr=%r",
|
||||
|
|
|
|||
|
|
@ -13,6 +13,8 @@ from typing import Any
|
|||
|
||||
import click
|
||||
|
||||
from headroom._subprocess import run
|
||||
|
||||
from .main import main
|
||||
|
||||
# Default paths
|
||||
|
|
@ -177,7 +179,7 @@ def mcp_uninstall() -> None:
|
|||
capture_output=True,
|
||||
)
|
||||
if check.returncode == 0:
|
||||
rm = subprocess.run(
|
||||
rm = run(
|
||||
[claude_cli, "mcp", "remove", "headroom", "-s", "user"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
|
|
@ -198,7 +200,7 @@ def mcp_uninstall() -> None:
|
|||
capture_output=True,
|
||||
)
|
||||
if cbm_check.returncode == 0:
|
||||
cbm_rm = subprocess.run(
|
||||
cbm_rm = run(
|
||||
[claude_cli, "mcp", "remove", "codebase-memory-mcp", "-s", "user"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
|
|
|
|||
|
|
@ -31,6 +31,8 @@ from collections.abc import Callable
|
|||
from pathlib import Path
|
||||
from typing import Any, cast
|
||||
|
||||
from headroom._subprocess import run
|
||||
|
||||
# Fix Windows cp1252 encoding — box-drawing characters require UTF-8
|
||||
if sys.platform == "win32" and hasattr(sys.stdout, "buffer"):
|
||||
if sys.stdout.encoding and sys.stdout.encoding.lower().replace("-", "") != "utf8":
|
||||
|
|
@ -501,12 +503,10 @@ def _setup_lean_ctx_agent(agent: str, verbose: bool = False) -> Path | None:
|
|||
# lean-ctx writes project-local files when initialized from a git
|
||||
# checkout. Run from a non-project directory so setup is limited to
|
||||
# home-scoped agent config such as ~/.codex or ~/.claude.
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
[str(lean_ctx), "init", "--agent", agent],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
errors="replace",
|
||||
timeout=30,
|
||||
cwd=setup_cwd,
|
||||
)
|
||||
|
|
@ -892,7 +892,7 @@ def _register_cbm_mcp_server(cbm_bin: str) -> None:
|
|||
return
|
||||
|
||||
# Check if already registered
|
||||
check = subprocess.run(
|
||||
check = run(
|
||||
[claude_cli, "mcp", "get", _CBM_MCP_SERVER_NAME],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
|
|
@ -900,7 +900,7 @@ def _register_cbm_mcp_server(cbm_bin: str) -> None:
|
|||
if check.returncode == 0:
|
||||
return # Already registered
|
||||
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
[claude_cli, "mcp", "add", _CBM_MCP_SERVER_NAME, "-s", "user", "--", cbm_bin],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
|
|
@ -948,7 +948,7 @@ def _setup_code_graph(verbose: bool = False) -> bool:
|
|||
# Index current project (fast — ~1s for most repos, idempotent)
|
||||
project_dir = str(Path.cwd())
|
||||
try:
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
[
|
||||
cbm_bin,
|
||||
"cli",
|
||||
|
|
@ -2818,14 +2818,12 @@ def _run_checked(
|
|||
) -> subprocess.CompletedProcess[str]:
|
||||
"""Run subprocess and raise a ClickException with actionable context on failure."""
|
||||
try:
|
||||
return subprocess.run(
|
||||
return run(
|
||||
cmd,
|
||||
cwd=str(cwd) if cwd else None,
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
errors="replace",
|
||||
)
|
||||
except FileNotFoundError as e:
|
||||
raise click.ClickException(f"{action} failed: command not found: {cmd[0]}") from e
|
||||
|
|
@ -2856,12 +2854,10 @@ def _normalize_openclaw_gateway_provider_ids(provider_ids: tuple[str, ...] | Non
|
|||
|
||||
def _read_openclaw_config_value(openclaw_bin: str, path: str) -> Any | None:
|
||||
"""Read an OpenClaw config value when present, returning None on missing paths."""
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
[openclaw_bin, "config", "get", path],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
errors="replace",
|
||||
)
|
||||
if result.returncode != 0:
|
||||
return None
|
||||
|
|
@ -2940,12 +2936,10 @@ def _set_openclaw_context_engine_slot(openclaw_bin: str, engine_id: str) -> None
|
|||
|
||||
def _restart_or_start_openclaw_gateway(openclaw_bin: str) -> tuple[str, str]:
|
||||
"""Restart the gateway when running, otherwise start it."""
|
||||
restart_result = subprocess.run(
|
||||
restart_result = run(
|
||||
[openclaw_bin, "gateway", "restart"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
errors="replace",
|
||||
)
|
||||
if restart_result.returncode == 0:
|
||||
output = restart_result.stdout.strip() or restart_result.stderr.strip()
|
||||
|
|
@ -3158,15 +3152,13 @@ def claude(
|
|||
# Memory sync BEFORE proxy startup — sync headroom DB ↔ Claude's files
|
||||
if memory:
|
||||
try:
|
||||
import subprocess as _sp
|
||||
|
||||
mem_dir = Path.cwd() / ".headroom"
|
||||
mem_dir.mkdir(parents=True, exist_ok=True)
|
||||
_sync_db = str(mem_dir / "memory.db")
|
||||
_sync_user = os.environ.get("USER", os.environ.get("USERNAME", "default"))
|
||||
|
||||
click.echo(f" Syncing memory (user={_sync_user})...")
|
||||
sync_result = _sp.run(
|
||||
sync_result = run(
|
||||
[
|
||||
sys.executable,
|
||||
"-m",
|
||||
|
|
@ -4845,13 +4837,11 @@ def openclaw(
|
|||
install_cwd = None
|
||||
|
||||
click.echo(" Installing OpenClaw plugin with required unsafe-install flag...")
|
||||
install_result = subprocess.run(
|
||||
install_result = run(
|
||||
install_cmd,
|
||||
cwd=str(install_cwd) if install_cwd else None,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
errors="replace",
|
||||
)
|
||||
if install_result.returncode != 0:
|
||||
combined_error = "\n".join(
|
||||
|
|
|
|||
|
|
@ -8,7 +8,6 @@ import hashlib
|
|||
import json
|
||||
import logging
|
||||
import os
|
||||
import subprocess
|
||||
import time
|
||||
from ctypes import wintypes
|
||||
from dataclasses import dataclass
|
||||
|
|
@ -20,6 +19,7 @@ from urllib import request as urllib_request
|
|||
from urllib.parse import urlparse
|
||||
|
||||
from headroom import paths
|
||||
from headroom._subprocess import run
|
||||
from headroom.copilot_linux_secret import read_copilot_oauth_token as read_linux_secret_token
|
||||
from headroom.copilot_macos_keychain import read_copilot_oauth_token as read_macos_keychain_token
|
||||
|
||||
|
|
@ -247,12 +247,10 @@ def _read_gh_cli_oauth_token() -> str | None:
|
|||
command.extend(["--hostname", host])
|
||||
|
||||
try:
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
command,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
errors="replace",
|
||||
check=False,
|
||||
)
|
||||
except OSError as exc:
|
||||
|
|
|
|||
|
|
@ -9,6 +9,8 @@ import subprocess
|
|||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
from headroom._subprocess import run
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
|
|
@ -84,12 +86,10 @@ def _candidate_secret_tool_commands(
|
|||
|
||||
def _run_secret_tool_lookup(command: list[str]) -> str | None:
|
||||
try:
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
command,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
errors="replace",
|
||||
check=False,
|
||||
timeout=5,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -9,6 +9,8 @@ import subprocess
|
|||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
from headroom._subprocess import run
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
|
|
@ -102,12 +104,10 @@ def _candidate_security_commands(
|
|||
|
||||
def _run_security_lookup(command: list[str]) -> str | None:
|
||||
try:
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
command,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
errors="replace",
|
||||
check=False,
|
||||
timeout=5,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -30,7 +30,6 @@ from __future__ import annotations
|
|||
import json
|
||||
import logging
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
import time
|
||||
|
|
@ -38,6 +37,8 @@ from dataclasses import dataclass, field
|
|||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
from headroom._subprocess import run
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Default benchmarks - chosen for reliability and relevance
|
||||
|
|
@ -272,7 +273,7 @@ def run_lm_eval(
|
|||
"TOKENIZERS_PARALLELISM": "false",
|
||||
"HF_ALLOW_CODE_EVAL": "1", # Required for humaneval/mbpp tasks
|
||||
}
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
cmd,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
|
|
|
|||
|
|
@ -95,13 +95,12 @@ def download_cbm(version: str | None = None) -> Path:
|
|||
|
||||
# Verify
|
||||
try:
|
||||
import subprocess
|
||||
from headroom._subprocess import run
|
||||
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
[str(target_path), "--version"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=5,
|
||||
)
|
||||
if result.returncode == 0:
|
||||
ver = result.stdout.strip()
|
||||
|
|
|
|||
|
|
@ -23,6 +23,8 @@ import threading
|
|||
import time
|
||||
from pathlib import Path
|
||||
|
||||
from headroom._subprocess import run
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Source file extensions worth reindexing for
|
||||
|
|
@ -217,7 +219,7 @@ class CodeGraphWatcher:
|
|||
|
||||
try:
|
||||
start = time.monotonic()
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
[
|
||||
self.cbm_binary,
|
||||
"cli",
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import re
|
|||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
from headroom._subprocess import run
|
||||
from headroom.providers.install_registry import (
|
||||
apply_provider_scope_mutations,
|
||||
revert_provider_scope_mutation,
|
||||
|
|
@ -88,7 +89,7 @@ def _apply_windows_env_scope(manifest: DeploymentManifest) -> list[ManagedMutati
|
|||
merged = _unix_scope_values(manifest)
|
||||
mutations: list[ManagedMutation] = []
|
||||
for name, value in merged.items():
|
||||
previous = subprocess.run(
|
||||
previous = run(
|
||||
[
|
||||
"powershell",
|
||||
"-NoProfile",
|
||||
|
|
|
|||
|
|
@ -13,6 +13,8 @@ from contextlib import contextmanager
|
|||
from pathlib import Path
|
||||
from typing import Any, cast
|
||||
|
||||
from headroom._subprocess import run
|
||||
|
||||
from .health import probe_ready
|
||||
from .models import DeploymentManifest, InstallPreset, RuntimeKind
|
||||
from .paths import log_path, pid_path, profile_root
|
||||
|
|
@ -282,7 +284,11 @@ def start_persistent_docker(manifest: DeploymentManifest) -> None:
|
|||
manifest.container_name,
|
||||
*command[5:], # drop initial `docker run --rm --name ...`
|
||||
]
|
||||
subprocess.run(["docker", "rm", "-f", manifest.container_name], capture_output=True, text=True)
|
||||
run(
|
||||
["docker", "rm", "-f", manifest.container_name],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
subprocess.run(docker_cmd, check=True)
|
||||
|
||||
|
||||
|
|
@ -290,9 +296,15 @@ def stop_runtime(manifest: DeploymentManifest) -> None:
|
|||
"""Stop the raw runtime for the deployment."""
|
||||
|
||||
if manifest.preset == InstallPreset.PERSISTENT_DOCKER.value:
|
||||
subprocess.run(["docker", "stop", manifest.container_name], capture_output=True, text=True)
|
||||
subprocess.run(
|
||||
["docker", "rm", "-f", manifest.container_name], capture_output=True, text=True
|
||||
run(
|
||||
["docker", "stop", manifest.container_name],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
run(
|
||||
["docker", "rm", "-f", manifest.container_name],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
return
|
||||
|
||||
|
|
@ -320,8 +332,10 @@ def runtime_status(manifest: DeploymentManifest) -> str:
|
|||
"""Return a short status string for the deployment runtime."""
|
||||
|
||||
if manifest.preset == InstallPreset.PERSISTENT_DOCKER.value:
|
||||
result = subprocess.run(
|
||||
["docker", "ps", "--format", "{{.Names}}"], capture_output=True, text=True
|
||||
result = run(
|
||||
["docker", "ps", "--format", "{{.Names}}"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
if manifest.container_name in result.stdout.splitlines():
|
||||
return "running"
|
||||
|
|
|
|||
|
|
@ -12,6 +12,8 @@ from pathlib import Path
|
|||
|
||||
import click
|
||||
|
||||
from headroom._subprocess import run
|
||||
|
||||
from .models import ArtifactRecord, DeploymentManifest, SupervisorKind
|
||||
from .paths import (
|
||||
unix_ensure_script_path,
|
||||
|
|
@ -206,7 +208,11 @@ def install_supervisor(manifest: DeploymentManifest) -> list[ArtifactRecord]:
|
|||
cron_path.write_text(content)
|
||||
records.append(ArtifactRecord(kind="cron", path=str(cron_path)))
|
||||
else:
|
||||
current = subprocess.run(["crontab", "-l"], capture_output=True, text=True)
|
||||
current = run(
|
||||
["crontab", "-l"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
existing = current.stdout if current.returncode == 0 else ""
|
||||
marker_start = f"# >>> headroom {manifest.profile} >>>"
|
||||
marker_end = f"# <<< headroom {manifest.profile} <<<"
|
||||
|
|
@ -215,7 +221,12 @@ def install_supervisor(manifest: DeploymentManifest) -> list[ArtifactRecord]:
|
|||
)
|
||||
merged = pattern.sub("", existing).strip()
|
||||
new_content = (merged + "\n\n" + content).strip() + "\n"
|
||||
subprocess.run(["crontab", "-"], input=new_content, text=True, check=True)
|
||||
run(
|
||||
["crontab", "-"],
|
||||
input=new_content,
|
||||
text=True,
|
||||
check=True,
|
||||
)
|
||||
records.append(ArtifactRecord(kind="crontab", path=f"user:{manifest.profile}"))
|
||||
return records
|
||||
|
||||
|
|
@ -234,7 +245,11 @@ def install_supervisor(manifest: DeploymentManifest) -> list[ArtifactRecord]:
|
|||
and manifest.supervisor_kind == SupervisorKind.SERVICE.value
|
||||
else f"gui/{os.getuid()}/{plist_path.stem}"
|
||||
)
|
||||
subprocess.run(["launchctl", "bootout", domain], capture_output=True, text=True)
|
||||
run(
|
||||
["launchctl", "bootout", domain],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
bootstrap_domain = (
|
||||
"system"
|
||||
if manifest.scope == "system"
|
||||
|
|
@ -324,7 +339,7 @@ def start_supervisor(manifest: DeploymentManifest) -> None:
|
|||
# Fast path: when the job is already bootstrapped (e.g. `start` right
|
||||
# after `install apply`, or `start` on a running service), `kickstart`
|
||||
# restarts it in place.
|
||||
kick = subprocess.run(
|
||||
kick = run(
|
||||
["launchctl", "kickstart", "-k", f"{domain}/{label}"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
|
|
@ -346,7 +361,7 @@ def start_supervisor(manifest: DeploymentManifest) -> None:
|
|||
plist_path = plist_dir / f"{label}.plist"
|
||||
last = kick
|
||||
for _ in range(_MACOS_BOOTSTRAP_RETRIES):
|
||||
boot = subprocess.run(
|
||||
boot = run(
|
||||
["launchctl", "bootstrap", domain, str(plist_path)],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
|
|
@ -385,7 +400,7 @@ def stop_supervisor(manifest: DeploymentManifest) -> None:
|
|||
# Any other non-zero result is a real failure (permissions, malformed
|
||||
# domain, launchd error) and must surface; otherwise `restart` could
|
||||
# report success while a stale job is still running.
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
["launchctl", "bootout", f"{domain}/{label}"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
|
|
@ -409,7 +424,7 @@ def remove_supervisor(manifest: DeploymentManifest) -> None:
|
|||
if sys.platform.startswith("linux"):
|
||||
if manifest.supervisor_kind == SupervisorKind.SERVICE.value:
|
||||
flags = [] if manifest.scope == "system" else ["--user"]
|
||||
subprocess.run(
|
||||
run(
|
||||
["systemctl", *flags, "disable", "--now", manifest.service_name],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
|
|
@ -417,21 +432,32 @@ def remove_supervisor(manifest: DeploymentManifest) -> None:
|
|||
unit_path, _ = _linux_service_unit(manifest, unix_run_script_path(manifest.profile))
|
||||
if unit_path.exists():
|
||||
unit_path.unlink()
|
||||
subprocess.run(["systemctl", *flags, "daemon-reload"], capture_output=True, text=True)
|
||||
run(
|
||||
["systemctl", *flags, "daemon-reload"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
return
|
||||
cron_path, _ = _linux_task_spec(manifest, unix_ensure_script_path(manifest.profile))
|
||||
if cron_path and cron_path.exists():
|
||||
cron_path.unlink()
|
||||
return
|
||||
current = subprocess.run(["crontab", "-l"], capture_output=True, text=True)
|
||||
current = run(
|
||||
["crontab", "-l"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
if current.returncode != 0:
|
||||
return
|
||||
marker_start = f"# >>> headroom {manifest.profile} >>>"
|
||||
marker_end = f"# <<< headroom {manifest.profile} <<<"
|
||||
pattern = re.compile(re.escape(marker_start) + r".*?" + re.escape(marker_end), re.DOTALL)
|
||||
content = pattern.sub("", current.stdout).strip()
|
||||
subprocess.run(
|
||||
["crontab", "-"], input=(content + "\n") if content else "", text=True, check=True
|
||||
run(
|
||||
["crontab", "-"],
|
||||
input=(content + "\n") if content else "",
|
||||
text=True,
|
||||
check=True,
|
||||
)
|
||||
return
|
||||
|
||||
|
|
@ -450,8 +476,10 @@ def remove_supervisor(manifest: DeploymentManifest) -> None:
|
|||
and manifest.supervisor_kind == SupervisorKind.SERVICE.value
|
||||
else f"gui/{os.getuid()}"
|
||||
)
|
||||
subprocess.run(
|
||||
["launchctl", "bootout", f"{domain}/{label}"], capture_output=True, text=True
|
||||
run(
|
||||
["launchctl", "bootout", f"{domain}/{label}"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
if plist_path.exists():
|
||||
plist_path.unlink()
|
||||
|
|
@ -459,19 +487,23 @@ def remove_supervisor(manifest: DeploymentManifest) -> None:
|
|||
|
||||
if _is_windows():
|
||||
if manifest.supervisor_kind == SupervisorKind.SERVICE.value:
|
||||
subprocess.run(
|
||||
["sc.exe", "stop", manifest.service_name], capture_output=True, text=True
|
||||
run(
|
||||
["sc.exe", "stop", manifest.service_name],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
subprocess.run(
|
||||
["sc.exe", "delete", manifest.service_name], capture_output=True, text=True
|
||||
run(
|
||||
["sc.exe", "delete", manifest.service_name],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
return
|
||||
subprocess.run(
|
||||
run(
|
||||
["schtasks", "/Delete", "/TN", f"{manifest.service_name}-startup", "/F"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
subprocess.run(
|
||||
run(
|
||||
["schtasks", "/Delete", "/TN", f"{manifest.service_name}-health", "/F"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
|
|
|
|||
|
|
@ -13,6 +13,8 @@ import zipfile
|
|||
from pathlib import Path
|
||||
from urllib.request import urlopen
|
||||
|
||||
from headroom._subprocess import run
|
||||
|
||||
from . import LEAN_CTX_BIN_DIR, LEAN_CTX_VERSION
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
|
@ -40,7 +42,7 @@ def _detect_runtime_target_triple() -> str:
|
|||
|
||||
def _is_musl() -> bool:
|
||||
try:
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
["ldd", "--version"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
|
|
@ -139,12 +141,10 @@ def download_lean_ctx(version: str | None = None) -> Path:
|
|||
|
||||
if _should_verify_target(target):
|
||||
try:
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
[str(target_path), "--version"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
errors="replace",
|
||||
timeout=5,
|
||||
)
|
||||
if result.returncode != 0:
|
||||
|
|
|
|||
|
|
@ -24,6 +24,8 @@ import threading
|
|||
import time
|
||||
import typing
|
||||
|
||||
from headroom._subprocess import Popen, run
|
||||
|
||||
from .loops import LoopPattern, apply_loop_weighting, detect_loops, format_loops_for_digest
|
||||
from .models import (
|
||||
AnalysisResult,
|
||||
|
|
@ -518,13 +520,11 @@ def _call_cli_llm(digest: str, model: str) -> dict:
|
|||
return _call_claude_cli_streaming(cmd, prompt, hard_cap=hard_cap, idle_cap=idle_cap)
|
||||
|
||||
try:
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
cmd,
|
||||
input=prompt,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
errors="replace",
|
||||
timeout=hard_cap,
|
||||
)
|
||||
except FileNotFoundError:
|
||||
|
|
@ -574,14 +574,12 @@ def _call_claude_cli_streaming(
|
|||
on Windows too, where ``select`` does not support pipe handles.
|
||||
"""
|
||||
try:
|
||||
proc = subprocess.Popen(
|
||||
proc = Popen(
|
||||
cmd,
|
||||
stdin=subprocess.PIPE,
|
||||
stdout=subprocess.PIPE,
|
||||
stderr=subprocess.PIPE,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
errors="replace",
|
||||
bufsize=1, # line-buffered
|
||||
)
|
||||
except FileNotFoundError:
|
||||
|
|
|
|||
|
|
@ -14,10 +14,11 @@ import json
|
|||
import logging
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
from headroom._subprocess import run
|
||||
|
||||
from .base import MCPRegistrar, RegisterResult, RegisterStatus, ServerSpec
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
|
@ -91,7 +92,7 @@ class ClaudeRegistrar(MCPRegistrar):
|
|||
|
||||
def unregister_server(self, server_name: str) -> bool:
|
||||
if self._claude_cli:
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
[str(self._claude_cli), "mcp", "remove", server_name, "-s", "user"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
|
|
@ -116,7 +117,11 @@ class ClaudeRegistrar(MCPRegistrar):
|
|||
cmd += ["-e", f"{k}={v}"]
|
||||
cmd += ["--", spec.command, *spec.args]
|
||||
|
||||
result = subprocess.run(cmd, capture_output=True, text=True)
|
||||
result = run(
|
||||
cmd,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
if result.returncode == 0:
|
||||
return RegisterResult(RegisterStatus.REGISTERED, "via `claude mcp add` (scope: user)")
|
||||
# CLI failed — try the file fallback rather than giving up.
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ import time
|
|||
from dataclasses import dataclass, field
|
||||
from pathlib import Path
|
||||
|
||||
from headroom._subprocess import run
|
||||
from headroom.memory.writers.base import MemoryEntry, _estimate_tokens
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
|
@ -215,7 +216,7 @@ class MemoryBudgetManager:
|
|||
return self._git_files_cache
|
||||
|
||||
try:
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
["git", "ls-files"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
|
|
|
|||
|
|
@ -14,7 +14,6 @@ import json
|
|||
import logging
|
||||
import os
|
||||
import random
|
||||
import subprocess
|
||||
import threading
|
||||
import time
|
||||
from collections import OrderedDict
|
||||
|
|
@ -23,6 +22,7 @@ from pathlib import Path
|
|||
from typing import TYPE_CHECKING, Any, Literal, cast
|
||||
|
||||
from headroom import paths as _paths
|
||||
from headroom._subprocess import run
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from fastapi import Request
|
||||
|
|
@ -1181,7 +1181,7 @@ def _read_rtk_lifetime_stats() -> dict[str, Any] | None:
|
|||
)
|
||||
|
||||
try:
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
_rtk_gain_command(rtk_path, scope),
|
||||
capture_output=True,
|
||||
text=True,
|
||||
|
|
@ -1242,7 +1242,7 @@ def _read_lean_ctx_lifetime_stats() -> dict[str, Any] | None:
|
|||
base_payload = _context_tool_zero_payload(tool=_CONTEXT_TOOL_LEAN_CTX, installed=True)
|
||||
|
||||
try:
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
[str(lean_ctx_path), "gain", "--json"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ from pathlib import Path
|
|||
from typing import Any
|
||||
|
||||
from headroom import binaries
|
||||
from headroom._subprocess import run
|
||||
from headroom.proxy import runtime_env
|
||||
|
||||
from . import base
|
||||
|
|
@ -200,7 +201,7 @@ def _run_ast_grep(
|
|||
try:
|
||||
for pattern in patterns:
|
||||
try:
|
||||
completed = subprocess.run(
|
||||
completed = run(
|
||||
[
|
||||
str(exe),
|
||||
"run",
|
||||
|
|
|
|||
|
|
@ -4,11 +4,12 @@ from __future__ import annotations
|
|||
|
||||
import os
|
||||
import re
|
||||
import subprocess
|
||||
from collections.abc import Sequence
|
||||
from dataclasses import dataclass, replace
|
||||
from pathlib import Path
|
||||
|
||||
from headroom._subprocess import run
|
||||
|
||||
SEMVER_RE = re.compile(r"^(\d+)\.(\d+)\.(\d+)$")
|
||||
RELEASE_TAG_RE = re.compile(r"^v(\d+)\.(\d+)\.(\d+)(?:\.(\d+))?$")
|
||||
CONVENTIONAL_COMMIT_RE = re.compile(
|
||||
|
|
@ -220,7 +221,7 @@ def get_canonical_version(root: Path) -> str:
|
|||
def list_release_tags(root: Path) -> list[str]:
|
||||
"""List release tags from the local Git checkout."""
|
||||
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
["git", "tag", "-l", "v*"],
|
||||
cwd=root,
|
||||
check=True,
|
||||
|
|
@ -239,7 +240,7 @@ def list_release_commits(root: Path, previous_tag: str) -> list[CommitInfo]:
|
|||
else:
|
||||
cmd.append("HEAD")
|
||||
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
cmd,
|
||||
cwd=root,
|
||||
check=True,
|
||||
|
|
@ -262,7 +263,7 @@ def commit_height_since(root: Path, previous_tag: str) -> str:
|
|||
if not previous_tag:
|
||||
return "0"
|
||||
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
["git", "rev-list", f"{previous_tag}..HEAD", "--count"],
|
||||
cwd=root,
|
||||
check=True,
|
||||
|
|
|
|||
|
|
@ -13,6 +13,8 @@ import zipfile
|
|||
from pathlib import Path
|
||||
from urllib.request import urlopen
|
||||
|
||||
from headroom._subprocess import run
|
||||
|
||||
from . import RTK_BIN_DIR, RTK_BIN_PATH, RTK_VERSION
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
|
@ -138,12 +140,10 @@ def download_rtk(version: str | None = None) -> Path:
|
|||
|
||||
if _should_verify_target(target):
|
||||
try:
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
[str(target_path), "--version"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
errors="replace",
|
||||
timeout=5,
|
||||
)
|
||||
if result.returncode != 0:
|
||||
|
|
@ -170,12 +170,10 @@ def register_claude_hooks(rtk_path: Path | None = None) -> bool:
|
|||
rtk_path = rtk_path or RTK_BIN_PATH
|
||||
|
||||
try:
|
||||
result = subprocess.run(
|
||||
result = run(
|
||||
[str(rtk_path), "init", "--global", "--auto-patch"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
errors="replace",
|
||||
timeout=10,
|
||||
)
|
||||
if result.returncode == 0:
|
||||
|
|
|
|||
68
tests/test_cli/test_subprocess_utf8_encoding.py
Normal file
68
tests/test_cli/test_subprocess_utf8_encoding.py
Normal file
|
|
@ -0,0 +1,68 @@
|
|||
"""Guard: every text-mode subprocess call uses the shared wrapper.
|
||||
|
||||
On Windows, text-mode ``subprocess`` defaults to the locale codec (cp1252) when
|
||||
``encoding=`` is omitted. Child output that is UTF-8 (e.g. a repo index printing
|
||||
symbol names with ``↔``/``—``) then raises ``UnicodeDecodeError: 'charmap'`` in the
|
||||
reader thread and aborts startup.
|
||||
|
||||
The fix is a shared wrapper at ``headroom._subprocess`` that automatically sets
|
||||
``encoding="utf-8", errors="replace"`` when ``text=True`` or
|
||||
``universal_newlines=True``. This test asserts that no raw ``subprocess.run`` /
|
||||
``subprocess.Popen`` (or similar) call with ``text=True`` exists in the shipped
|
||||
package — they must all go through the wrapper.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import ast
|
||||
from pathlib import Path
|
||||
|
||||
_PACKAGE = Path(__file__).resolve().parents[2] / "headroom"
|
||||
_SKIP = {"_subprocess.py"}
|
||||
_SUBPROCESS_FUNCS = {"run", "Popen", "check_output", "check_call", "call"}
|
||||
|
||||
|
||||
def _kwarg(call: ast.Call, name: str) -> ast.keyword | None:
|
||||
return next((k for k in call.keywords if k.arg == name), None)
|
||||
|
||||
|
||||
def _is_true(node: ast.AST | None) -> bool:
|
||||
return isinstance(node, ast.Constant) and node.value is True
|
||||
|
||||
|
||||
def _is_raw_subprocess_call(call: ast.Call) -> bool:
|
||||
func = call.func
|
||||
return isinstance(func, ast.Attribute) and func.attr in _SUBPROCESS_FUNCS
|
||||
|
||||
|
||||
def _offenders() -> list[str]:
|
||||
bad: list[str] = []
|
||||
for path in _PACKAGE.rglob("*.py"):
|
||||
if path.name in _SKIP:
|
||||
continue
|
||||
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
|
||||
for node in ast.walk(tree):
|
||||
if not isinstance(node, ast.Call) or not _is_raw_subprocess_call(node):
|
||||
continue
|
||||
text_kw = _kwarg(node, "text")
|
||||
un_kw = _kwarg(node, "universal_newlines")
|
||||
text_mode = (text_kw is not None and _is_true(text_kw.value)) or (
|
||||
un_kw is not None and _is_true(un_kw.value)
|
||||
)
|
||||
if text_mode:
|
||||
rel = path.relative_to(_PACKAGE.parent)
|
||||
bad.append(f"{rel}:{node.lineno}")
|
||||
return bad
|
||||
|
||||
|
||||
def test_text_mode_subprocess_calls_use_wrapper() -> None:
|
||||
offenders = _offenders()
|
||||
assert not offenders, (
|
||||
"raw subprocess calls with text=True found (use headroom._subprocess wrapper):\n"
|
||||
+ "\n".join(offenders)
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__": # pragma: no cover - manual run
|
||||
test_text_mode_subprocess_calls_use_wrapper()
|
||||
print("ok: all text-mode subprocess calls use the shared wrapper")
|
||||
|
|
@ -446,7 +446,7 @@ def test_read_gh_cli_oauth_token_uses_hostname(monkeypatch: pytest.MonkeyPatch)
|
|||
return CompletedProcess()
|
||||
|
||||
monkeypatch.setenv("GITHUB_COPILOT_HOST", "example.ghe.com")
|
||||
monkeypatch.setattr(copilot_auth.subprocess, "run", fake_run)
|
||||
monkeypatch.setattr(copilot_auth, "run", fake_run)
|
||||
|
||||
assert copilot_auth._read_gh_cli_oauth_token() == "gho-gh-cli"
|
||||
assert calls == [["gh", "auth", "token", "--hostname", "example.ghe.com"]]
|
||||
|
|
@ -458,7 +458,7 @@ def test_read_gh_cli_oauth_token_returns_none_when_invocation_fails(
|
|||
def fake_run(*args: object, **kwargs: object) -> None: # noqa: ANN002, ANN003
|
||||
raise OSError("gh missing")
|
||||
|
||||
monkeypatch.setattr(copilot_auth.subprocess, "run", fake_run)
|
||||
monkeypatch.setattr(copilot_auth, "run", fake_run)
|
||||
|
||||
assert copilot_auth._read_gh_cli_oauth_token() is None
|
||||
|
||||
|
|
@ -467,7 +467,7 @@ def test_read_gh_cli_oauth_token_returns_none_for_nonzero_exit(
|
|||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setattr(
|
||||
copilot_auth.subprocess,
|
||||
copilot_auth,
|
||||
"run",
|
||||
lambda *args, **kwargs: SimpleNamespace(returncode=1, stdout="ignored"),
|
||||
)
|
||||
|
|
@ -479,7 +479,7 @@ def test_read_gh_cli_oauth_token_returns_none_for_blank_stdout(
|
|||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setattr(
|
||||
copilot_auth.subprocess,
|
||||
copilot_auth,
|
||||
"run",
|
||||
lambda *args, **kwargs: SimpleNamespace(returncode=0, stdout=" \n"),
|
||||
)
|
||||
|
|
|
|||
|
|
@ -146,7 +146,7 @@ def test_list_release_commits_parses_empty_body_entries(
|
|||
run.return_value = Mock(
|
||||
stdout="feat: add capability\x1f\x1efix: patch bug\x1fbody text\x1e",
|
||||
)
|
||||
monkeypatch.setattr("headroom.release_version.subprocess.run", run)
|
||||
monkeypatch.setattr("headroom.release_version.run", run)
|
||||
|
||||
commits = list_release_commits(ROOT, "")
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue