fix(node-agent): rsync --omit-dir-times — dir mtime on VPS not settable (aerbot-owned), caused false "shipping failed" despite successful transfer

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

services/node-agent/src/node_agent.py

@@ -543,6 +543,18 @@ class NodeAgent:
                       f"{VPS_EVENTS_PATH}/{self.node_name}/")
         cmd = [
             "rsync", "-az", "--remove-source-files",
+            # --omit-dir-times: the remote per-node event dir on VPS is owned
+            # by aerbot:aerbot; the ssh user (oskar) is only a group member,
+            # so it can write files into the dir but cannot chown/chmod/touch
+            # the dir itself. -a implies -t (preserve times) which also
+            # applies to directories, so rsync would otherwise try to set the
+            # dir's mtime, get EPERM, and return non-zero even though every
+            # file transferred fine. Files still keep their mtime via -t;
+            # this only skips the (harmless, doomed) directory mtime set.
+            # TODO tech-debt: fix ownership of /opt/homelab/events/<node> on
+            # VPS so this workaround isn't needed (see docs/backlog.md,
+            # "Tech-debt: globalny porządek uid/gid/uprawnień").
+            "--omit-dir-times",
             # -F /dev/null: skip ~/.ssh/config entirely.  The .ssh dir is
             # mounted from the host oskar user into the container which runs
             # as root; OpenSSH rejects config files owned by a different UID.

services/node-agent/tests/conftest.py

@@ -0,0 +1,28 @@
+"""Shared fixtures for node-agent tests."""
+from __future__ import annotations
+
+import os
+import sys
+import tempfile
+from pathlib import Path
+
+import pytest
+
+_SRC = Path(__file__).resolve().parents[1] / "src"
+sys.path.insert(0, str(_SRC))
+
+# node_agent.py reads RUNTIME_PATH/NODE_NAME as module-level constants at
+# import time, so these must be set before the first `import node_agent`.
+_RUNTIME = Path(tempfile.mkdtemp(prefix="node-agent-test-"))
+os.environ.setdefault("RUNTIME_PATH", str(_RUNTIME))
+os.environ.setdefault("NODE_NAME", "test-node")
+
+import node_agent  # noqa: E402
+
+
+@pytest.fixture
+def agent(monkeypatch):
+    monkeypatch.setattr(node_agent, "VPS_EVENTS_HOST", "vps.example.com")
+    monkeypatch.setattr(node_agent, "VPS_EVENTS_USER", "oskar")
+    monkeypatch.setattr(node_agent, "VPS_EVENTS_PATH", "/opt/homelab/events")
+    return node_agent.NodeAgent()

services/node-agent/tests/test_ship_events_to_vps.py

@@ -0,0 +1,59 @@
+"""Tests for NodeAgent._ship_events_to_vps rsync command construction.
+
+Covers the fix for the false "Event shipping failed" warning: the remote
+per-node events dir on VPS is owned by aerbot:aerbot, so the ssh user (a
+group member, not the owner) can write files into it but cannot touch the
+dir's own mtime. -a implies -t (preserve times), which rsync also applies to
+directories, so without --omit-dir-times rsync returns a non-zero exit code
+on every run even though the file transfer itself succeeded.
+"""
+from __future__ import annotations
+
+from unittest.mock import MagicMock
+
+import node_agent
+
+
+def test_omits_dir_times_flag(agent, monkeypatch):
+    captured = {}
+
+    def fake_run(cmd, **kwargs):
+        captured["cmd"] = cmd
+        return MagicMock(returncode=0, stderr="")
+
+    monkeypatch.setattr(node_agent.subprocess, "run", fake_run)
+    agent._ship_events_to_vps()
+
+    assert "--omit-dir-times" in captured["cmd"]
+
+
+def test_skips_when_vps_host_not_set(agent, monkeypatch):
+    monkeypatch.setattr(node_agent, "VPS_EVENTS_HOST", "")
+    fake_run = MagicMock()
+    monkeypatch.setattr(node_agent.subprocess, "run", fake_run)
+
+    agent._ship_events_to_vps()
+
+    fake_run.assert_not_called()
+
+
+def test_skips_on_vps_node(agent, monkeypatch):
+    agent.node_name = node_agent.VPS_NODE_NAME
+    fake_run = MagicMock()
+    monkeypatch.setattr(node_agent.subprocess, "run", fake_run)
+
+    agent._ship_events_to_vps()
+
+    fake_run.assert_not_called()
+
+
+def test_nonzero_returncode_logs_warning(agent, monkeypatch, caplog):
+    def fake_run(cmd, **kwargs):
+        return MagicMock(returncode=23, stderr="rsync: some partial transfer error")
+
+    monkeypatch.setattr(node_agent.subprocess, "run", fake_run)
+
+    with caplog.at_level("WARNING"):
+        agent._ship_events_to_vps()
+
+    assert "Event shipping failed" in caplog.text