fix(uci): abort the transaction on net_tcp_send timeout exits (#194)

[JC-000]

Closes #194.

Three bounded-wait exits in net_tcp_send returned with the command interface half-way through a transaction: uci_push_wait timing out on the SOCKET_WRITE response left it in state "01", and either drain timing out left it in "10" with the data phase open. Nothing on any error path called uci_abort — it had exactly one call site in the entire codebase, in net_init — so the only thing that ever cleared a mid-transaction bail was the next boot.

All three now share one exit that writes ABORT and then waits for idle. Carry, the C=1 contract and net_last_error ($89, already set by the wait that expired) are unchanged; only the state left behind for the next command changes.

ABORT rather than DATA_ACC

DATA_ACC is gated on state(1) = '1' in command_protocol.vhd, so it is a no-op on the push-wait exit (state "01"). The abort bit sets handshake_in(2) unconditionally and command_intf.cc's task answers with HANDSHAKE_RESET (0x87), whose bit 7 forces state <= "00".

Two further arguments, both found during review: HANDSHAKE_RESET also rewinds command_pointer where DATA_ACC does not, so a partially-pushed command cannot be prepended to the next; and NetworkTarget::abort() only calls discard_read_reply() and closes no socket (close_all_sockets() is reachable only from c64_reset()), so this cannot desync our socket id from the firmware's table — fw #814's socket table is unaffected.

The reset is a firmware round trip, so uci_abort's own 32-iteration settle (~160 cycles, shorter than one uci_fence at ~5,450) cannot cover it; the existing bounded uci_wait_idle does.

On the $86 claim in the issue: not demonstrated

No captured run pairs a send $89 with a following $86, and this PR does not claim one. What is shown is the mechanism: net_poll reaches PUSH_CMD through uci_wait_not_busy (mask $01), not uci_wait_idle (mask $31), so a left-open data phase does not hold it back — it pushes straight into else error_busy <= '1', the single setter of $DF1C bit 3. The test counts that rejection: 1 before the fix, 0 after.

A detail worth recording for whoever next chases a field $86: in the model the surviving net_last_error is usually $89, not $86, because net_poll's own drain then times out over the leftover queue and overwrites it. The byte that survives depends on what happens after the rejection, so the field signature is not reliably a literal $86. The test asserts on the rejection, not on the byte.

Known limits, stated in the code rather than discovered later

• The abort is serviced by the same FreeRTOS task that services commands. A task blocked in lwip_send — the likeliest reason uci_push_wait timed out in the first place — cannot action an abort either; then uci_wait_idle also expires and the interface stays dirty, 5 s spent and nothing recovered. This helps the transient-slowness class only.
• A bail can now block ~10 s where it blocked ~5 s: one extra wait per net_tcp_send call (not per chunk — @sb_bail exits the routine, not the chunk loop), and no caller retries (tls_send_record bcs @fail, http_get_plain bcs @plain_close_err).
• The test's model advances a TOD tenth per $DC08 read, so it proves the code waits at all, not that a real abort round trip fits inside 5 s. That assumption is unmeasured and not reachable on demand.

Scope

net_tcp_connect (4 exits) and net_tcp_close (1) have the same defect and are knowingly out of scope — tracked as #221. Review confirmed the scope choice is exactly right for the $86 mechanism: those three all open with uci_wait_idle (mask $31), and only a net_tcp_send bail leaves net_tcp_state at CONNECTED so polling continues. They cannot produce $86, but they can leave the interface unusable until reboot, with every later connect burning 5 s and returning CONNECT_FAIL. @sb_bail is the remedy to reuse there.

Red-green

Reproducible from history — the test commit is red by design:

git checkout 0a804bf && make clean && make BACKEND=uci USE_NISTCURVES_ONCHIP=1
python3 tools/test_uci_timeout_recovery.py     -> exit 1, 3/4 red, 14 assertions
                        at de4353e             -> 4/4, 18 assertions

Mutating the fix to abort without waiting is caught 3/4 red — the model answers after a delay, so a recovery that does not wait cannot pass by accident. test_uci_data_acc.py 3/3; bare pytest 157 passed.

The test runs the shipped 6502 bytes from build/c64-https.prg at build/labels.txt addresses, against a $DF1B-$DF1F model built from command_protocol.vhd and command_intf.cc: the four-state machine with its error_busy branch, a delayed-response firmware, and a running CIA1 TOD so the bounds expire on purpose. If the model is wrong the test is wrong with it, and the module's docstring says so. Review verified all four load-bearing firmware claims against primary source, and confirmed the model never producing state "11" is correct rather than a gap (SOCKET_WRITE's reply is single-block).

Hardware

rig_https_live.py vs github.com, U64E @ 48 MHz, uci-onchip PRG a3452651179e22d3328cda4298d2dfbf0b03f1d40747203cd3436dbd6ccc3a92 — HTTP 200, carry=0, net_last_error=$00 (no $86, no $89), CFIN 48.4 s, in line with CLAUDE.md's ~45 s onchip figure. Happy path unregressed on real silicon. The timeout paths themselves are not reachable on demand and were not exercised there.

The third commit is comments and test text only: all seven profiles rebuild byte-identical to the second, so the hardware result above still describes the tip — the SHA moved, the image did not.

Cost

+1 B of UCI_CODE on all three UCI profiles ($813 → $814; NET_CODE tail 366 → 365 B); every other region byte-identical, verified by diffing full segment lists rather than by arithmetic.

profile	before	after
uci-default	9eaee61d0fe4e40d2dfe2311abdefd0988718ee4692620d8980adfa14fdff960	240b383a2b7841439780f8d3f9a87957c2e8d2d3f535632a84d616520cddb236
uci-onchip	971b074f65c77eb53b84c9f8f10074bbff81105953e6a04c69533289937977dc	a3452651179e22d3328cda4298d2dfbf0b03f1d40747203cd3436dbd6ccc3a92
uci-comb	00e4370ce3086e8b30df243861e56725a09797f08d8e668535c5166c62882152	7c3addb092f14066b82f83419a439d963374edad3f67dac206e9db80db197317

All four ip65 PRGs — both profiles × default and en.wikipedia.org targets — byte-identical to master, as expected since nothing outside src/net/uci/ moved.

Review note

The adversarial pass found the suite's headline failure message printing a firmware-impossible claim under mutation: it sampled the interface state at assertion time while asserting it was the state at rejection time, so it reported a rejection from state "00", which command_protocol.vhd shows takes the accept branch. In the un-mutated red the two states coincide, which is exactly why it read correctly and hid the defect. Now captured at the pushes_rejected increment and reported separately from the end state, with a comment warning against reconstructing it from assertion-time state again.

🤖 Generated with Claude Code