From 72403e8859367f554c72a7ac70899c2adc3b4626 Mon Sep 17 00:00:00 2001 From: huangruiteng <14976749+huangruiteng@users.noreply.github.com> Date: Sat, 26 Sep 2026 00:52:10 +0800 Subject: [PATCH 1/2] fix(launchagent): bound the LoopX service logs The status and chat LaunchAgents append to `~/Library/Logs/loopx/*.log` with no retention, so those files only ever grow. KeepAlive means they outlive every release, and a service that is noisy for a while leaves that output on disk indefinitely: on one host the two stdout logs had reached 59 MB and 49 MB. Rotate a log to `.1` once it passes a size limit, defaulting to 10 MiB and overridable with `LOOPX_LOG_MAX_BYTES`. The rotation runs inside each agent's own wrapper rather than in this installer, because launchd restarts the service on its own and those restarts never re-enter the installer -- which is exactly when the unbounded growth happens. It also copies the previous generation aside and truncates in place rather than renaming. launchd opens StandardOutPath before the wrapper runs and keeps appending to that descriptor, so a rename would send the service's output to the rotated copy and leave the live log missing. The smoke reproduces that descriptor and asserts the live path still receives post-rotation writes, so the rename mistake cannot come back. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com> Co-authored-by: Cursor --- ...acos-dashboard-launchagent-status-smoke.py | 56 +++++++++++++++++++ scripts/macos-dashboard-launchagent.sh | 26 ++++++++- 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/examples/macos-dashboard-launchagent-status-smoke.py b/examples/macos-dashboard-launchagent-status-smoke.py index 561aeac252..98e9de9563 100644 --- a/examples/macos-dashboard-launchagent-status-smoke.py +++ b/examples/macos-dashboard-launchagent-status-smoke.py @@ -45,6 +45,50 @@ 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 main() -> int: with tempfile.TemporaryDirectory(prefix="loopx-launchagent-status-smoke-") as raw_tmp: tmp = Path(raw_tmp) @@ -132,6 +176,18 @@ 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) + 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, + extra_env={"LOOPX_LOG_MAX_BYTES": str(rotation_limit)}, + ).stdout + run_script( fake_bin, home, diff --git a/scripts/macos-dashboard-launchagent.sh b/scripts/macos-dashboard-launchagent.sh index 1665370944..218c1c1b26 100755 --- a/scripts/macos-dashboard-launchagent.sh +++ b/scripts/macos-dashboard-launchagent.sh @@ -12,6 +12,7 @@ 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="${LOOPX_LOG_MAX_BYTES:-10485760}" uid="$(id -u)" launch_agents_dir="$HOME/Library/LaunchAgents" @@ -44,6 +45,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 +63,25 @@ 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. +log_rotation_prelude() { + local basename="$1" + printf 'for loopx_log in %s %s; do [ -f "$loopx_log" ] && [ "$(stat -f%%z "$loopx_log" 2>/dev/null || echo 0)" -gt %s ] && { cp -f "$loopx_log" "$loopx_log.1" 2>/dev/null; : >"$loopx_log"; }; done; unset loopx_log;' \ + "$(shell_quote "$logs_dir/$basename.out.log")" \ + "$(shell_quote "$logs_dir/$basename.err.log")" \ + "$log_max_bytes" +} + require_macos() { if [[ "$(uname -s)" != "Darwin" ]]; then echo "macOS LaunchAgent installation requires Darwin/macOS." >&2 @@ -221,8 +242,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" @@ -429,6 +450,7 @@ print_status() { echo "- $logs_dir/status.err.log" echo "- $logs_dir/chat.out.log" echo "- $logs_dir/chat.err.log" + echo "- retention: rotated to .1 at each agent start once a log exceeds $log_max_bytes bytes" } main() { From 829935702b53eab8d92eac5c91b9bcdbe3496f72 Mon Sep 17 00:00:00 2001 From: huangruiteng <14976749+huangruiteng@users.noreply.github.com> Date: Sun, 27 Sep 2026 02:11:37 +0800 Subject: [PATCH 2/2] fix(launchagent): keep the log when retention cannot back it up Retention truncated the live log whether or not the copy aside succeeded, and status reported the caller's environment rather than the installed policy. Both are corrected here, together with the input validation the first fix needs to be meaningful: - The wrapper only truncates after `cp` succeeded, and refuses to copy over an existing directory at the backup path (where `cp` would otherwise copy *into* it and report success). When the copy fails the live log keeps every byte, and the agent's error log gets a line naming the path so the operator can diagnose it; the next agent start retries. - `status` reads the retention size back out of the installed plist, so it reports what the running agents do. An explicitly different `LOOPX_LOG_MAX_BYTES` adds a note that it is not in effect yet, and a missing retention step is reported as unknown instead of as a default. - `install`/`restart` reject a `LOOPX_LOG_MAX_BYTES` that is not a positive byte count before writing a wrapper whose retention step could never match. The status smoke covers both failure injections for the copy (a directory at the backup path, and a logs directory that refuses new files), the installed -versus-override readback, and the rejected install leaving the previous wrapper in place. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com> --- ...acos-dashboard-launchagent-status-smoke.py | 82 ++++++++++++++++++- scripts/macos-dashboard-launchagent.sh | 46 ++++++++++- 2 files changed, 122 insertions(+), 6 deletions(-) diff --git a/examples/macos-dashboard-launchagent-status-smoke.py b/examples/macos-dashboard-launchagent-status-smoke.py index 98e9de9563..271096ed96 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, ) @@ -89,6 +90,79 @@ def check_log_rotation(home: Path, plist: Path, basename: str, limit: int) -> No 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) @@ -183,9 +257,11 @@ def main() -> int: 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, - extra_env={"LOOPX_LOG_MAX_BYTES": str(rotation_limit)}, ).stdout run_script( diff --git a/scripts/macos-dashboard-launchagent.sh b/scripts/macos-dashboard-launchagent.sh index 218c1c1b26..e2ecd00d41 100755 --- a/scripts/macos-dashboard-launchagent.sh +++ b/scripts/macos-dashboard-launchagent.sh @@ -12,7 +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="${LOOPX_LOG_MAX_BYTES:-10485760}" +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" @@ -74,14 +75,37 @@ shell_quote() { # 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" ] && [ "$(stat -f%%z "$loopx_log" 2>/dev/null || echo 0)" -gt %s ] && { cp -f "$loopx_log" "$loopx_log.1" 2>/dev/null; : >"$loopx_log"; }; done; unset loopx_log;' \ + 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 @@ -432,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" \ @@ -450,11 +475,26 @@ print_status() { echo "- $logs_dir/status.err.log" echo "- $logs_dir/chat.out.log" echo "- $logs_dir/chat.err.log" - echo "- retention: rotated to .1 at each agent start once a log exceeds $log_max_bytes bytes" + 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