fix: review notes on #52

src/beaconmcp/proxmox/client.py

@@ -181,8 +181,11 @@ def api_call(self, node_name: str, method: str, path: str, **kwargs: Any) -> Any
                 # already running, and a second POST would start it again.
                 if method != "get" and not _never_sent(e):
                     self._cache().pop(node_name, None)
+                    what = "did not answer in time" if isinstance(e, Timeout) else (
+                        "dropped the connection mid-request"
+                    )
                     return {
-                        "error": f"Node '{node_name}' did not answer in time: {e}. "
+                        "error": f"Node '{node_name}' {what}: {e}. "
                         "The request may still have been applied; check "
                         "proxmox_get_tasks before retrying."
                     }

src/beaconmcp/proxmox/system.py

@@ -187,10 +187,13 @@ def _agent_read_bytes(
     count = _FILE_READ_MAX_BYTES + 1
     result = client.get(node, api, file=path, count=count, decode=0)
     legacy = False
-    if isinstance(result, dict) and "error" in result and "decode" in str(result["error"]):
+    if isinstance(result, dict) and "error" in result and any(
+        name in str(result["error"]) for name in ("decode", "count")
+    ):
         # PVE before the `decode` parameter: content arrives decoded, one
-        # JSON code point per byte.
-        result = client.get(node, api, file=path, count=count)
+        # JSON code point per byte. `count` may be just as new, so it is left
+        # out too; the caller's length check still enforces the limit.
+        result = client.get(node, api, file=path)
         legacy = True
     if isinstance(result, dict) and "error" in result:
         return {"status": "error", "error": result["error"]}
@@ -321,10 +324,18 @@ async def connect_tcp(
                 f"Refusing to fetch from {host} ({local}): loopback and link-local "
                 "addresses are not allowed."
             )
-        return await self._inner.connect_tcp(
-            addrs[0], port, timeout=timeout,
-            local_address=local_address, socket_options=socket_options,
-        )
+        # Every address was checked above, so trying them in turn keeps the
+        # guarantee: an unreachable AAAA on an IPv4-only host must not end it.
+        last: Exception | None = None
+        for addr in addrs:
+            try:
+                return await self._inner.connect_tcp(
+                    addr, port, timeout=timeout,
+                    local_address=local_address, socket_options=socket_options,
+                )
+            except (httpcore.ConnectError, httpcore.ConnectTimeout, OSError) as e:
+                last = e
+        raise httpcore.ConnectError(f"cannot connect to {host}: {last}") from last
 
     async def sleep(self, seconds: float) -> None:
         await self._inner.sleep(seconds)
@@ -337,6 +348,9 @@ def _fetch_transport() -> httpx.AsyncBaseTransport:
     a proxy would resolve the name itself, out of the guard's reach.
     """
     transport = httpx.AsyncHTTPTransport()
+    # httpx has no public hook for the network backend; `_pool` is the pool
+    # every request goes through (checked against httpx 0.28.1 / httpcore
+    # 1.0.9, and pinned by test_upload_links' rebinding test).
     transport._pool = httpcore.AsyncConnectionPool(
         ssl_context=httpx.create_ssl_context(),
         network_backend=_GuardedNetworkBackend(),

tests/test_agent_file_io.py

@@ -41,8 +41,11 @@ def get(self, node: str, path: str, **params: Any) -> Any:
         if path == f"nodes/{node}/qemu/{VM}/status/current":
             return {"status": "running"}
         if path == f"nodes/{node}/qemu/{VM}/agent/file-read":
-            if "decode" in params and not self.has_decode_param:
-                return {"error": "400 Parameter verification failed: decode: property is not defined in schema"}
+            if not self.has_decode_param:
+                # Older qemu-server: neither `decode` nor `count` in the schema.
+                for name in ("decode", "count"):
+                    if name in params:
+                        return {"error": f"400 Parameter verification failed: {name}: property is not defined in schema"}
             data = self.files[params["file"]][: params.get("count", 16 * 1024 * 1024)]
             if params.get("decode", 1):
                 # Perl hands JSON the decoded bytes, one code point each.

tests/test_proxmox_client.py

@@ -225,3 +225,10 @@ def test_get_is_retried_after_a_read_timeout(monkeypatch: pytest.MonkeyPatch) ->
     ep = _Endpoint(ReadTimeout("read timed out"))
     _client_with(monkeypatch, ep).get("pve1", "nodes")
     assert ep.calls == 2
+
+
+def test_a_dropped_write_is_not_called_a_timeout(monkeypatch: pytest.MonkeyPatch) -> None:
+    ep = _Endpoint(RequestsConnectionError("Connection aborted: RemoteDisconnected"))
+    result = _client_with(monkeypatch, ep).post("pve1", "nodes")
+    assert ep.calls == 1
+    assert "dropped the connection mid-request" in result["error"]

tests/test_upload_links.py

@@ -544,3 +544,26 @@ async def test_guarded_connect_keeps_the_hostname(
     )
     assert res["status"] == "success", res
     assert env.ssh.guest_files["/f"] == f"LOOPBACK-ONLY-SECRET files.test:{loopback_server}".encode()
+
+
+async def test_fetch_tries_each_checked_address(monkeypatch) -> None:
+    # An unreachable first answer (an AAAA on an IPv4-only box) must not end
+    # the fetch: every address was checked, so the next one is still safe.
+    import httpcore
+
+    async def fake_dns(host):
+        return ["2001:db8::1", "198.51.100.7"]
+
+    monkeypatch.setattr(system, "_host_addresses", fake_dns)
+    backend = system._GuardedNetworkBackend()
+    tried: list[str] = []
+
+    async def connect(addr, port, **kw):
+        tried.append(addr)
+        if addr == "2001:db8::1":
+            raise httpcore.ConnectError("network unreachable")
+        return "stream"
+
+    monkeypatch.setattr(backend._inner, "connect_tcp", connect)
+    assert await backend.connect_tcp("example.com", 443) == "stream"
+    assert tried == ["2001:db8::1", "198.51.100.7"]