diff --git a/examples/macos-dashboard-launchagent-status-smoke.py b/examples/macos-dashboard-launchagent-status-smoke.py index 561aeac25..271096ed9 100644 --- a/examples/macos-dashboard-launchagent-status-smoke.py +++ b/examples/macos-dashboard-launchagent-status-smoke.py @@ -5,6 +5,7 @@ import os import plistlib +import shutil import subprocess import tempfile from pathlib import Path @@ -19,7 +20,7 @@ def write_executable(path: Path, body: str) -> None: path.chmod(0o755) -def run_script(fake_bin: Path, home: Path, args: list[str], *, schema_version: int, write_enabled: bool = False, extra_env: dict[str, str] | None = None) -> subprocess.CompletedProcess[str]: +def run_script(fake_bin: Path, home: Path, args: list[str], *, schema_version: int, write_enabled: bool = False, extra_env: dict[str, str] | None = None, check: bool = True) -> subprocess.CompletedProcess[str]: env = { **os.environ, "HOME": str(home), @@ -35,7 +36,7 @@ def run_script(fake_bin: Path, home: Path, args: list[str], *, schema_version: i [str(LAUNCHAGENT_SCRIPT), *args], cwd=REPO_ROOT, env=env, - check=True, + check=check, capture_output=True, text=True, ) @@ -45,6 +46,123 @@ def run_status(fake_bin: Path, home: Path, *, schema_version: int, write_enabled return run_script(fake_bin, home, ["status"], schema_version=schema_version, write_enabled=write_enabled).stdout +def log_rotation_prelude(plist: Path) -> str: + """The rotation step the agent wrapper runs before it execs the service.""" + command = plistlib.loads(plist.read_bytes())["ProgramArguments"][2] + prelude, separator, _ = command.partition(" export LOOPX_PYTHON=") + assert separator, command + return prelude + + +def check_log_rotation(home: Path, plist: Path, basename: str, limit: int) -> None: + logs_dir = home / "Library" / "Logs" / "loopx" + prelude = log_rotation_prelude(plist) + for stream in ("out", "err"): + assert str(logs_dir / f"{basename}.{stream}.log") in prelude, prelude + + # launchd opens StandardOutPath before the wrapper runs and keeps appending + # to that descriptor. Rotation must therefore truncate the live file rather + # than rename it, or the service's output follows the rotated copy and the + # live log stays empty until the next restart. Reproduce that descriptor. + live = logs_dir / f"{basename}.out.log" + live.parent.mkdir(parents=True, exist_ok=True) + live.write_bytes(b"O" * (limit + 1)) + descriptor = os.open(live, os.O_WRONLY | os.O_APPEND) + try: + subprocess.run(["zsh", "-c", prelude], check=True) + os.write(descriptor, b"after-rotation\n") + finally: + os.close(descriptor) + assert live.exists(), ( + "rotation must truncate the live log in place: launchd's descriptor " + "follows a rename, which would strand the service's output in the " + "rotated copy and leave this path missing" + ) + assert live.read_bytes() == b"after-rotation\n", live.read_bytes()[:80] + assert live.with_suffix(".log.1").read_bytes() == b"O" * (limit + 1) + + # A log under the limit keeps its history; rotation is retention, not a + # reset on every service start. + small = logs_dir / f"{basename}.err.log" + small.write_bytes(b"kept") + subprocess.run(["zsh", "-c", prelude], check=True) + assert not small.with_suffix(".log.1").exists() + assert small.read_bytes() == b"kept" + + +def check_retention_keeps_the_log_when_the_backup_fails(home: Path, plist: Path, basename: str, limit: int) -> None: + """A failed backup must leave the live log alone. + + Truncating on a failed copy would destroy the only record of the failure + the operator is trying to diagnose, so retention has to stand down and say + so instead. + """ + logs_dir = home / "Library" / "Logs" / "loopx" + prelude = log_rotation_prelude(plist) + live = logs_dir / f"{basename}.out.log" + live.parent.mkdir(parents=True, exist_ok=True) + original = b"E" * (limit + 1) + backup = live.with_suffix(".log.1") + + def run_prelude() -> subprocess.CompletedProcess[str]: + result = subprocess.run(["zsh", "-c", prelude], capture_output=True, text=True) + assert result.returncode == 0, (result.stdout, result.stderr) + assert live.read_bytes() == original, "a failed backup must not truncate the live log" + assert not backup.is_file(), "a failed backup must not leave a partial generation" + assert "skipped retention" in result.stderr, result.stderr + return result + + # The previous generation is not replaceable: a directory at the backup path + # is a real copy failure, and cp would otherwise copy *into* it. + live.write_bytes(original) + if backup.is_file(): + backup.unlink() + backup.mkdir() + try: + run_prelude() + finally: + shutil.rmtree(backup, ignore_errors=True) + + # And a directory that cannot accept a new file fails the same way. + if os.geteuid() != 0: + backup.unlink(missing_ok=True) + live.write_bytes(original) + logs_dir.chmod(0o500) + try: + run_prelude() + finally: + logs_dir.chmod(0o755) + + +def check_installed_retention_readback( + fake_bin: Path, home: Path, plist: Path, basename: str, limit: int +) -> None: + """Status reports the installed policy, not the caller's environment.""" + command = plistlib.loads(plist.read_bytes())["ProgramArguments"][2] + assert f"-gt {limit}" in command, command + installed = run_script(fake_bin, home, ["status"], schema_version=2).stdout + assert f"- retention: rotated to .1 at each agent start once a log exceeds {limit} bytes" in installed, installed + assert "not in effect" not in installed, installed + overridden = run_script( + fake_bin, home, ["status"], schema_version=2, extra_env={"LOOPX_LOG_MAX_BYTES": "4096"} + ).stdout + assert f"once a log exceeds {limit} bytes" in overridden, overridden + assert "LOOPX_LOG_MAX_BYTES=4096 is not in effect" in overridden, overridden + + +def check_invalid_retention_is_rejected(fake_bin: Path, home: Path, plist: Path, limit: int) -> None: + """An unusable threshold fails before a wrapper is written.""" + rejected = run_script( + fake_bin, home, ["install"], schema_version=2, + extra_env={"LOOPX_LOG_MAX_BYTES": "invalid"}, check=False, + ) + assert rejected.returncode != 0, rejected.stdout + assert "LOOPX_LOG_MAX_BYTES must be a positive byte count, got: invalid" in rejected.stderr, rejected.stderr + # The rejected install left the previously installed wrapper in place. + command = plistlib.loads(plist.read_bytes())["ProgramArguments"][2] + assert f"-gt {limit}" in command, command + + def main() -> int: with tempfile.TemporaryDirectory(prefix="loopx-launchagent-status-smoke-") as raw_tmp: tmp = Path(raw_tmp) @@ -132,6 +250,20 @@ def main() -> int: assert "/loopx-canary" not in default_plist, default_plist assert not (home / "Library" / "LaunchAgents" / "com.loopx.dashboard.plist").exists(), "retired dashboard LaunchAgent should not be installed" + # KeepAlive restarts never re-enter this installer, so each agent + # carries its own retention step for both of its streams. + rotation_limit = 1024 + run_script(fake_bin, home, ["install"], schema_version=2, + extra_env={"LOOPX_LOG_MAX_BYTES": str(rotation_limit)}) + for plist, basename in ((status_plist, "status"), (chat_plist, "chat")): + check_log_rotation(home, plist, basename, rotation_limit) + check_retention_keeps_the_log_when_the_backup_fails(home, plist, basename, rotation_limit) + check_installed_retention_readback(fake_bin, home, plist, basename, rotation_limit) + check_invalid_retention_is_rejected(fake_bin, home, status_plist, rotation_limit) + assert f"- retention: rotated to .1 at each agent start once a log exceeds {rotation_limit} bytes" in run_script( + fake_bin, home, ["status"], schema_version=2, + ).stdout + run_script( fake_bin, home, diff --git a/scripts/macos-dashboard-launchagent.sh b/scripts/macos-dashboard-launchagent.sh index 166537094..e2ecd00d4 100755 --- a/scripts/macos-dashboard-launchagent.sh +++ b/scripts/macos-dashboard-launchagent.sh @@ -12,6 +12,8 @@ chat_port="${LOOPX_CHAT_PORT:-8767}" host="${LOOPX_DASHBOARD_HOST:-127.0.0.1}" chat_runtime_endpoint="$host:$chat_port" label_prefix="${LOOPX_LAUNCH_LABEL_PREFIX:-com.loopx}" +log_max_bytes_override="${LOOPX_LOG_MAX_BYTES:-}" +log_max_bytes="${log_max_bytes_override:-10485760}" uid="$(id -u)" launch_agents_dir="$HOME/Library/LaunchAgents" @@ -44,6 +46,7 @@ Environment overrides: LOOPX_CHAT_PORT LOOPX_DASHBOARD_HOST LOOPX_LAUNCH_LABEL_PREFIX + LOOPX_LOG_MAX_BYTES Rotate an agent log once it exceeds this size (default 10 MiB) LOOPX_CHAT_CODEX_HOME Explicit managed Codex home (upgrades preserve the existing binding) EOF } @@ -61,6 +64,48 @@ shell_quote() { printf '%q' "$1" } +# Keep the agent logs bounded. KeepAlive means these files outlive every +# release: without a retention step they only ever grow, and a service that +# becomes noisy for a while leaves that output on disk forever. +# +# Rotation has to run inside the agent's own wrapper, because launchd restarts +# the service on its own and those restarts never re-enter this installer. +# It also must not rename the live file: launchd opens StandardOutPath before +# the wrapper runs and keeps appending to that descriptor, so renaming would +# send the service's output to the rotated copy and leave the live path empty. +# Copy the previous generation aside and truncate in place instead, which the +# append-mode descriptor follows back to offset zero. +# +# The truncate is conditional on the copy succeeding. A failed copy (read-only +# target, full disk) must leave the live log intact: dropping it would destroy +# the only record of the failure the operator is trying to diagnose. Retention +# is retried at the next agent start, and the warning goes to the agent's own +# error log. +log_rotation_prelude() { + local basename="$1" + printf 'for loopx_log in %s %s; do [ -f "$loopx_log" ] || continue; loopx_size="$(stat -f%%z "$loopx_log" 2>/dev/null || echo 0)"; case "$loopx_size" in [0-9]*) ;; *) continue; esac; [ "$loopx_size" -gt %s ] || continue; if [ ! -d "$loopx_log.1" ] && cp -f "$loopx_log" "$loopx_log.1" 2>/dev/null; then : >"$loopx_log"; else printf "loopx-launchagent: kept %%s and skipped retention: could not write %%s.1; the next start retries\\n" "$loopx_log" "$loopx_log" >&2; fi; done; unset loopx_log loopx_size;' \ + "$(shell_quote "$logs_dir/$basename.out.log")" \ + "$(shell_quote "$logs_dir/$basename.err.log")" \ + "$log_max_bytes" +} + +# The installed retention policy is whatever the installed wrapper runs; the +# caller's environment describes a future install. Read the value back out of +# the plist so status cannot report a setting that is not in effect. +installed_log_max_bytes() { + local plist="$1" value + [[ -f "$plist" ]] || return 1 + value="$(grep -o -- '-gt [0-9][0-9]*' "$plist" 2>/dev/null | head -n 1 | awk '{print $2}')" + [[ "$value" =~ ^[0-9]+$ ]] || return 1 + printf '%s' "$value" +} + +# Fail fast instead of writing a wrapper whose retention step can never match. +validate_log_max_bytes() { + [[ "$log_max_bytes" =~ ^[0-9]+$ ]] || return 1 + (( log_max_bytes > 0 )) +} + require_macos() { if [[ "$(uname -s)" != "Darwin" ]]; then echo "macOS LaunchAgent installation requires Darwin/macOS." >&2 @@ -221,8 +266,8 @@ write_plists() { fi chat_codex_home="$(resolve_chat_codex_home "$python_command")" codex_home_export=" export CODEX_HOME=$(shell_quote "$chat_codex_home"); export LOOPX_CHAT_CODEX_HOME=$(shell_quote "$chat_codex_home");" - status_shell="export LOOPX_PYTHON=$(shell_quote "$python_command"); export PATH=$(shell_quote "$path_prefix"):\$PATH; exec $(shell_quote "$status_command") --registry $(shell_quote "$registry") serve-status --global-registry --host $(shell_quote "$host") --port $(shell_quote "$status_port") --limit $(shell_quote "$status_limit")$control_plane_write_arg" - chat_shell="export LOOPX_PYTHON=$(shell_quote "$python_command");$codex_home_export export PATH=$(shell_quote "$path_prefix"):\$PATH; exec $(shell_quote "$status_command") --registry $(shell_quote "$registry") chat --global-registry --host $(shell_quote "$host") --port $(shell_quote "$chat_port") --codex-bin $(shell_quote "$codex_command") --claude-bin $(shell_quote "$claude_command")$lark_cli_arg --replace-existing-loopx-chat --no-open" + status_shell="$(log_rotation_prelude status) export LOOPX_PYTHON=$(shell_quote "$python_command"); export PATH=$(shell_quote "$path_prefix"):\$PATH; exec $(shell_quote "$status_command") --registry $(shell_quote "$registry") serve-status --global-registry --host $(shell_quote "$host") --port $(shell_quote "$status_port") --limit $(shell_quote "$status_limit")$control_plane_write_arg" + chat_shell="$(log_rotation_prelude chat) export LOOPX_PYTHON=$(shell_quote "$python_command");$codex_home_export export PATH=$(shell_quote "$path_prefix"):\$PATH; exec $(shell_quote "$status_command") --registry $(shell_quote "$registry") chat --global-registry --host $(shell_quote "$host") --port $(shell_quote "$chat_port") --codex-bin $(shell_quote "$codex_command") --claude-bin $(shell_quote "$claude_command")$lark_cli_arg --replace-existing-loopx-chat --no-open" mkdir -p "$launch_agents_dir" "$logs_dir" @@ -411,6 +456,7 @@ print_status_contract_health() { } print_status() { + local installed_log_max echo "LaunchAgents:" launchctl print "gui/$uid/$status_label" >/dev/null 2>&1 \ && echo "- $status_label: loaded" \ @@ -429,10 +475,26 @@ print_status() { echo "- $logs_dir/status.err.log" echo "- $logs_dir/chat.out.log" echo "- $logs_dir/chat.err.log" + if installed_log_max="$(installed_log_max_bytes "$status_plist")"; then + echo "- retention: rotated to .1 at each agent start once a log exceeds $installed_log_max bytes" + if [[ -n "$log_max_bytes_override" ]] && (( installed_log_max != log_max_bytes )); then + echo " note: LOOPX_LOG_MAX_BYTES=$log_max_bytes is not in effect; the installed value holds until the next install or restart" + fi + else + echo "- retention: unknown (the installed agents carry no retention step; run: $0 install)" + fi } main() { require_macos + case "${1:-}" in + install|restart) + if ! validate_log_max_bytes; then + echo "LOOPX_LOG_MAX_BYTES must be a positive byte count, got: $log_max_bytes" >&2 + exit 2 + fi + ;; + esac case "${1:-}" in install) write_plists