Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/defib/agent/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,12 +318,28 @@ async def read_memory(
await send_packet(self._transport, CMD_READ, payload)

received = bytearray()
# Track the 16-bit seq prefix the agent stamps on every RSP_DATA
# (`pkt[0..1] = seq` in agent/main.c handle_read). Without this,
# a COBS-CRC-rejected packet was silently dropped by recv_packet
# and read_memory returned short data with no error — see kaeru
# `agent-read-silent-packet-loss-on-long-uart-streams-2026-05-15`.
expected_seq = 0
while True:
cmd, data = await recv_packet(self._transport, timeout=60.0)
if cmd == RSP_READY:
continue
elif cmd == RSP_DATA and len(data) > 2:
seq = data[0] | (data[1] << 8)
if seq != expected_seq:
raise RuntimeError(
f"Read packet seq gap at offset {len(received)} "
f"(addr={addr + len(received):#010x}): expected "
f"seq={expected_seq}, got seq={seq}; likely UART "
f"corruption on a long stream. Try a smaller read "
f"or a lower baud."
)
received.extend(data[2:])
expected_seq = (expected_seq + 1) & 0xFFFF
if on_progress:
on_progress(len(received), size)
elif cmd == RSP_ACK:
Expand All @@ -334,6 +350,14 @@ async def read_memory(
f"Read rejected by agent: status=0x{status:02x} "
f"({detail}); addr={addr:#010x} size={size}"
)
# Final size check — catches the case where the agent
# ACKs early but we never saw a seq gap (shouldn't happen
# with the seq-check above, but cheap insurance).
if len(received) != size:
raise RuntimeError(
f"Read returned {len(received)} bytes but agent "
f"ACKed for {size}; addr={addr:#010x}"
)
break
else:
raise RuntimeError(f"Unexpected response: cmd=0x{cmd:02x}")
Expand Down
76 changes: 76 additions & 0 deletions tests/test_agent_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -871,3 +871,79 @@ async def test_crc32_raises_on_flash_error_ack(self):
t.enqueue_rx(make_device_packet(RSP_ACK, bytes([ACK_FLASH_ERROR])))
with pytest.raises(RuntimeError, match="CRC32 rejected"):
await client.crc32(0x10100000, 64)


class TestReadMemorySeqValidation:
"""When recv_packet silently drops a CRC-invalid RSP_DATA packet
mid-stream during a long read (e.g. UART corruption on a multi-MB
eMMC dump), read_memory must NOT return short data and pretend
success. It must spot the gap via the seq prefix the agent stamps
on every RSP_DATA and raise loudly so the caller can chunk + retry."""

@pytest.mark.asyncio
async def test_normal_sequential_seq_passes(self):
# seq=0,1,2 followed by ACK_OK — must succeed cleanly.
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)

# 3 packets of 4 bytes each => 12 bytes total
for seq in range(3):
seq_bytes = struct.pack("<H", seq)
payload = bytes([seq * 4, seq * 4 + 1, seq * 4 + 2, seq * 4 + 3])
t.enqueue_rx(make_device_packet(RSP_DATA, seq_bytes + payload))
t.enqueue_rx(make_device_packet(RSP_ACK, bytes([ACK_OK])))

data = await client.read_memory(0x80000000, 12, fast=False)
assert data == bytes(range(12))

@pytest.mark.asyncio
async def test_seq_gap_raises(self):
# seq=0 then seq=2 — packet 1 was dropped by recv_packet (CRC fail).
# Without the seq check the client would silently return 8 bytes;
# with it, we raise.
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_DATA, struct.pack("<H", 0) + b"AAAA"
))
t.enqueue_rx(make_device_packet(
RSP_DATA, struct.pack("<H", 2) + b"CCCC"
))
t.enqueue_rx(make_device_packet(RSP_ACK, bytes([ACK_OK])))

with pytest.raises(RuntimeError, match="seq gap"):
await client.read_memory(0x80000000, 12, fast=False)

@pytest.mark.asyncio
async def test_size_mismatch_at_ack_raises(self):
# No seq gap, but agent ACKs after fewer bytes than the host asked
# for. Real-hardware bug surfaced when the agent's handle_read
# short-circuited; client now fails loudly instead of returning
# truncated data.
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)

# Only 8 bytes delivered, but client asked for 16
t.enqueue_rx(make_device_packet(
RSP_DATA, struct.pack("<H", 0) + b"AAAABBBB"
))
t.enqueue_rx(make_device_packet(RSP_ACK, bytes([ACK_OK])))

with pytest.raises(RuntimeError, match="Read returned 8 bytes but agent ACKed for 16"):
await client.read_memory(0x80000000, 16, fast=False)
Loading