From 7985f06946d93d9fa083f1d2dfb690a1d2d1e556 Mon Sep 17 00:00:00 2001
From: Dmitry Ilyin <widgetii@users.noreply.github.com>
Date: Fri, 15 May 2026 15:23:58 +0300
Subject: [PATCH] agent: surface ACK_FLASH_ERROR instead of returning empty
 bytes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`FlashAgentClient.read_memory` previously broke its accumulator loop on
any RSP_ACK and returned whatever the buffer held — `b""` when the agent
rejected the address (e.g. a probe outside the V3+/V4+ I/O whitelist in
`agent/main.c:addr_readable`). Callers couldn't tell a successful empty
read from a rejection.

Same shape in `FlashAgentClient.crc32`, where a clean RSP_ACK rejection
was reported as a generic "CRC32 response invalid".

Now both inspect the ACK status byte and raise a RuntimeError naming the
status, the address, and the size — surfaced while probing eMMC controller
MMIO at 0x10100000 on hi3516av300 returned empty bytes silently for ~30s.

Also adds the missing `ACK_FLASH_ERROR = 0x02` to the Python protocol
constants (already in agent/protocol.h) and updates the existing membw
rejection test to use the constant instead of the 0x02 literal.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/defib/agent/client.py    | 15 ++++++++++++++
 src/defib/agent/protocol.py  |  3 ++-
 tests/test_agent_protocol.py | 39 +++++++++++++++++++++++++++++++++++-
 3 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/src/defib/agent/client.py b/src/defib/agent/client.py
index a07f214..7557e63 100644
--- a/src/defib/agent/client.py
+++ b/src/defib/agent/client.py
@@ -15,6 +15,7 @@
 from typing import Callable
 
 from defib.agent.protocol import (
+    ACK_FLASH_ERROR,
     ACK_OK,
     CMD_CRC32,
     CMD_ERASE,
@@ -320,6 +321,13 @@ async def read_memory(
                 if on_progress:
                     on_progress(len(received), size)
             elif cmd == RSP_ACK:
+                status = data[0] if data else 0
+                if status != ACK_OK:
+                    detail = "outside agent's readable whitelist or flash read failed" if status == ACK_FLASH_ERROR else "agent error"
+                    raise RuntimeError(
+                        f"Read rejected by agent: status=0x{status:02x} "
+                        f"({detail}); addr={addr:#010x} size={size}"
+                    )
                 break
             else:
                 raise RuntimeError(f"Unexpected response: cmd=0x{cmd:02x}")
@@ -627,6 +635,13 @@ async def crc32(self, addr: int, size: int) -> int:
         # chips (DMA reads, multi-MB/s) still get plenty of headroom.
         timeout = max(15.0, 5.0 + size / (100 * 1024))
         cmd, data = await recv_response(self._transport, timeout=timeout)
+        if cmd == RSP_ACK:
+            status = data[0] if data else 0
+            detail = "outside agent's readable whitelist or flash read failed" if status == ACK_FLASH_ERROR else "agent error"
+            raise RuntimeError(
+                f"CRC32 rejected by agent: status=0x{status:02x} "
+                f"({detail}); addr={addr:#010x} size={size}"
+            )
         if cmd != RSP_CRC32 or len(data) < 4:
             raise RuntimeError("CRC32 response invalid")
         return int(struct.unpack("<I", data[:4])[0])
diff --git a/src/defib/agent/protocol.py b/src/defib/agent/protocol.py
index c7b96b1..19b0d3c 100644
--- a/src/defib/agent/protocol.py
+++ b/src/defib/agent/protocol.py
@@ -62,9 +62,10 @@
 RSP_SCAN = 0x86
 RSP_MEMBW = 0x87
 
-# ACK status
+# ACK status (must match agent/protocol.h)
 ACK_OK = 0x00
 ACK_CRC_ERROR = 0x01
+ACK_FLASH_ERROR = 0x02
 
 FRAME_DELIMITER = 0x00
 MAX_PACKET_SIZE = 1100  # Max COBS-encoded packet size
diff --git a/tests/test_agent_protocol.py b/tests/test_agent_protocol.py
index a46bc67..66ef909 100644
--- a/tests/test_agent_protocol.py
+++ b/tests/test_agent_protocol.py
@@ -11,6 +11,7 @@
 from defib.agent.protocol import (
     ACK_OK,
     ACK_CRC_ERROR,
+    ACK_FLASH_ERROR,
     CMD_INFO,
     CMD_READ,
     CMD_SELFUPDATE,
@@ -831,6 +832,42 @@ async def test_armv5_rejection_raises(self):
         client = FlashAgentClient(t)
         assert await client.connect(timeout=1.0)
 
-        t.enqueue_rx(make_device_packet(_RSP_ACK, bytes([0x02])))  # ACK_FLASH_ERROR
+        t.enqueue_rx(make_device_packet(_RSP_ACK, bytes([ACK_FLASH_ERROR])))
         with pytest.raises(RuntimeError, match="rejected"):
             await client.membw()
+
+
+class TestReadCrcRejection:
+    """When the agent rejects a CMD_READ / CMD_CRC32 (e.g. addr outside
+    addr_readable whitelist), the client must surface a clear error
+    rather than silently returning empty bytes or a generic 'invalid'."""
+
+    @pytest.mark.asyncio
+    async def test_read_memory_raises_on_flash_error_ack(self):
+        from defib.transport.mock import MockTransport
+        from defib.agent.client import FlashAgentClient
+
+        t = MockTransport(flush_clears_buffer=False)
+        t.enqueue_rx(make_device_packet(RSP_READY, b"DEFIB"))
+        client = FlashAgentClient(t)
+        assert await client.connect(timeout=1.0)
+
+        # Agent immediately rejects with ACK_FLASH_ERROR (e.g. whitelist miss)
+        t.enqueue_rx(make_device_packet(RSP_ACK, bytes([ACK_FLASH_ERROR])))
+        with pytest.raises(RuntimeError, match="Read rejected"):
+            # Use fast=False to skip baud-switch path
+            await client.read_memory(0x10100000, 64, fast=False)
+
+    @pytest.mark.asyncio
+    async def test_crc32_raises_on_flash_error_ack(self):
+        from defib.transport.mock import MockTransport
+        from defib.agent.client import FlashAgentClient
+
+        t = MockTransport(flush_clears_buffer=False)
+        t.enqueue_rx(make_device_packet(RSP_READY, b"DEFIB"))
+        client = FlashAgentClient(t)
+        assert await client.connect(timeout=1.0)
+
+        t.enqueue_rx(make_device_packet(RSP_ACK, bytes([ACK_FLASH_ERROR])))
+        with pytest.raises(RuntimeError, match="CRC32 rejected"):
+            await client.crc32(0x10100000, 64)