fix(launchagent): bound the LoopX service logs

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,

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