Skip to content

Commit 45da1d6

Browse files
widgetiiclaude
andauthored
agent: surface ACK_FLASH_ERROR instead of returning empty bytes (#101)
## Summary - `FlashAgentClient.read_memory()` previously broke its accumulator loop on **any** `RSP_ACK` and returned the buffer as-is. When the agent rejected the address — e.g. a probe outside the V3+/V4+ I/O whitelist in `agent/main.c:addr_readable` — that meant `b\"\"` silently. Now it inspects the ACK status byte and raises `RuntimeError(\"Read rejected by agent: status=0x02 …; addr=… size=…\")`. - `FlashAgentClient.crc32()` had the same shape — a clean `RSP_ACK` rejection was reported as a generic \"CRC32 response invalid\". Now it explicitly detects the ACK and reports the status. - Adds the missing `ACK_FLASH_ERROR = 0x02` to `src/defib/agent/protocol.py` (it already existed in `agent/protocol.h`). Updates the existing membw rejection test to reference the constant. ## Context Surfaced while studying flash on an eMMC-based hi3516av300 over the running agent. The eMMC controller MMIO lives at `0x10100000..0x101FFFFF` which isn't in the agent's V3+/V4+ whitelist; probing there silently returned `b\"\"` for ~30s of wall time across multiple addresses, leading to wasted debugging. ## Test plan - [x] `uv run pytest tests/ -x --ignore=tests/fuzz` — 496 passed, 2 skipped - [x] `uv run ruff check src/defib/agent/ tests/test_agent_protocol.py` — clean - [x] `uv run mypy src/defib/agent/ --ignore-missing-imports` — clean - [x] Verified on real hardware (hi3516av300, eMMC D9D16): probing `0x10100000` now reports `Read rejected by agent: status=0x02 (outside agent's readable whitelist or flash read failed); addr=0x10100000 size=64` instead of returning empty bytes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Dmitry Ilyin <widgetii@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5d90121 commit 45da1d6

3 files changed

Lines changed: 55 additions & 2 deletions

File tree

src/defib/agent/client.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
from typing import Callable
1616

1717
from defib.agent.protocol import (
18+
ACK_FLASH_ERROR,
1819
ACK_OK,
1920
CMD_CRC32,
2021
CMD_ERASE,
@@ -320,6 +321,13 @@ async def read_memory(
320321
if on_progress:
321322
on_progress(len(received), size)
322323
elif cmd == RSP_ACK:
324+
status = data[0] if data else 0
325+
if status != ACK_OK:
326+
detail = "outside agent's readable whitelist or flash read failed" if status == ACK_FLASH_ERROR else "agent error"
327+
raise RuntimeError(
328+
f"Read rejected by agent: status=0x{status:02x} "
329+
f"({detail}); addr={addr:#010x} size={size}"
330+
)
323331
break
324332
else:
325333
raise RuntimeError(f"Unexpected response: cmd=0x{cmd:02x}")
@@ -627,6 +635,13 @@ async def crc32(self, addr: int, size: int) -> int:
627635
# chips (DMA reads, multi-MB/s) still get plenty of headroom.
628636
timeout = max(15.0, 5.0 + size / (100 * 1024))
629637
cmd, data = await recv_response(self._transport, timeout=timeout)
638+
if cmd == RSP_ACK:
639+
status = data[0] if data else 0
640+
detail = "outside agent's readable whitelist or flash read failed" if status == ACK_FLASH_ERROR else "agent error"
641+
raise RuntimeError(
642+
f"CRC32 rejected by agent: status=0x{status:02x} "
643+
f"({detail}); addr={addr:#010x} size={size}"
644+
)
630645
if cmd != RSP_CRC32 or len(data) < 4:
631646
raise RuntimeError("CRC32 response invalid")
632647
return int(struct.unpack("<I", data[:4])[0])

src/defib/agent/protocol.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,10 @@
6262
RSP_SCAN = 0x86
6363
RSP_MEMBW = 0x87
6464

65-
# ACK status
65+
# ACK status (must match agent/protocol.h)
6666
ACK_OK = 0x00
6767
ACK_CRC_ERROR = 0x01
68+
ACK_FLASH_ERROR = 0x02
6869

6970
FRAME_DELIMITER = 0x00
7071
MAX_PACKET_SIZE = 1100 # Max COBS-encoded packet size

tests/test_agent_protocol.py

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from defib.agent.protocol import (
1212
ACK_OK,
1313
ACK_CRC_ERROR,
14+
ACK_FLASH_ERROR,
1415
CMD_INFO,
1516
CMD_READ,
1617
CMD_SELFUPDATE,
@@ -831,6 +832,42 @@ async def test_armv5_rejection_raises(self):
831832
client = FlashAgentClient(t)
832833
assert await client.connect(timeout=1.0)
833834

834-
t.enqueue_rx(make_device_packet(_RSP_ACK, bytes([0x02]))) # ACK_FLASH_ERROR
835+
t.enqueue_rx(make_device_packet(_RSP_ACK, bytes([ACK_FLASH_ERROR])))
835836
with pytest.raises(RuntimeError, match="rejected"):
836837
await client.membw()
838+
839+
840+
class TestReadCrcRejection:
841+
"""When the agent rejects a CMD_READ / CMD_CRC32 (e.g. addr outside
842+
addr_readable whitelist), the client must surface a clear error
843+
rather than silently returning empty bytes or a generic 'invalid'."""
844+
845+
@pytest.mark.asyncio
846+
async def test_read_memory_raises_on_flash_error_ack(self):
847+
from defib.transport.mock import MockTransport
848+
from defib.agent.client import FlashAgentClient
849+
850+
t = MockTransport(flush_clears_buffer=False)
851+
t.enqueue_rx(make_device_packet(RSP_READY, b"DEFIB"))
852+
client = FlashAgentClient(t)
853+
assert await client.connect(timeout=1.0)
854+
855+
# Agent immediately rejects with ACK_FLASH_ERROR (e.g. whitelist miss)
856+
t.enqueue_rx(make_device_packet(RSP_ACK, bytes([ACK_FLASH_ERROR])))
857+
with pytest.raises(RuntimeError, match="Read rejected"):
858+
# Use fast=False to skip baud-switch path
859+
await client.read_memory(0x10100000, 64, fast=False)
860+
861+
@pytest.mark.asyncio
862+
async def test_crc32_raises_on_flash_error_ack(self):
863+
from defib.transport.mock import MockTransport
864+
from defib.agent.client import FlashAgentClient
865+
866+
t = MockTransport(flush_clears_buffer=False)
867+
t.enqueue_rx(make_device_packet(RSP_READY, b"DEFIB"))
868+
client = FlashAgentClient(t)
869+
assert await client.connect(timeout=1.0)
870+
871+
t.enqueue_rx(make_device_packet(RSP_ACK, bytes([ACK_FLASH_ERROR])))
872+
with pytest.raises(RuntimeError, match="CRC32 rejected"):
873+
await client.crc32(0x10100000, 64)

0 commit comments

Comments
 (0)