cli: make burn terminal bidirectional

[eorry-bit]

Summary

• forward stdin to the active transport in normal U-Boot burn -t terminal mode
• put POSIX terminals in raw/no-echo mode and restore terminal state and the previous SIGINT handler on exit
• preserve transport-to-stdout streaming and support clean exit from Ctrl-C, stdin EOF, or transport disconnect
• support redirected stdin on all platforms while keeping interactive Windows console input
• leave framed download-command mode unchanged

Tests

• uv run pytest tests/ -x -q --ignore=tests/fuzz (730 passed, 2 skipped)
• uv run pytest tests/fuzz/ -x -q --hypothesis-seed=0 (16 passed)
• uv run ruff check src/ tests/
• uv run mypy src/defib/ --ignore-missing-imports
• make -C agent test HOST_CC=gcc (5412 passed)
• node --test web/protocol.test.js web/profile-parity.test.js (86 passed)

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Make burn terminal bidirectional with safe raw-mode cleanup

🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

AI Description

• Forward interactive stdin to active transports while preserving U-Boot output streaming.
• Handle raw mode, Windows input, Ctrl-C, EOF, and disconnect cleanup.
• Add PTY tests for bidirectional traffic and terminal/SIGINT restoration.

Diagram

sequenceDiagram
    actor User as Operator
    participant Term as Host Terminal
    participant Bridge as Raw Bridge
    participant Transport as Transport
    participant Device as U-Boot Device
    User->>Term: Type command
    Term->>Bridge: stdin bytes
    Bridge->>Transport: write bytes
    Transport->>Device: forward bytes
    Device-->>Transport: response bytes
    Transport-->>Bridge: read bytes
    Bridge-->>Term: stdout bytes
    User->>Term: Ctrl-C
    Term->>Bridge: stop
    Bridge-->>Term: restore state

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Thread-backed stdin reader

• ➕ Could provide one input path across POSIX and Windows.
• ➕ Avoids reliance on event-loop file-descriptor reader support.
• ➖ Blocking reads complicate prompt cancellation and shutdown.
• ➖ Thread lifecycle adds cleanup and terminal-restoration risk.

2. Adopt a terminal abstraction library

• ➕ Could centralize cross-platform raw input and key handling.
• ➕ May provide broader Windows console compatibility.
• ➖ Adds a dependency for a narrowly scoped byte bridge.
• ➖ Library key processing may interfere with transparent U-Boot byte forwarding.

Recommendation: Keep the native platform-specific bridge: it preserves byte-level behavior, integrates directly with the existing async transport, and avoids a new dependency. The main follow-up consideration is adding Windows-specific coverage because the new msvcrt path is not exercised by the POSIX PTY tests.

Files changed (3) +240 / -16

Enhancement (1) +151 / -0

terminal.pyAdd a cross-platform asynchronous raw terminal bridge +151/-0

Add a cross-platform asynchronous raw terminal bridge

• Introduces concurrent stdin-to-transport and transport-to-stdout pumps with POSIX and Windows input paths. It handles Ctrl-C, EOF, disconnects, transport timeouts, task cancellation, raw/no-echo mode, and restoration of terminal and SIGINT state.

src/defib/cli/terminal.py

Bug fix (1) +8 / -16

app.pyDelegate normal burn terminal mode to the bidirectional bridge +8/-16

Delegate normal burn terminal mode to the bidirectional bridge

• Replaces the output-only transport polling loop with 'run_raw_terminal', passing binary stdin and stdout. The framed download-command mode remains unchanged, while terminal closure messaging and KeyboardInterrupt handling are preserved.

src/defib/cli/app.py

Tests (1) +81 / -0

test_terminal.pyVerify PTY forwarding and terminal cleanup +81/-0

Verify PTY forwarding and terminal cleanup

• Adds POSIX PTY tests covering bidirectional byte forwarding, Ctrl-C termination, no-echo raw mode, and restoration of terminal attributes and the previous SIGINT handler.

tests/test_terminal.py

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

1. Disconnect escapes without cleanup 🐞 Bug ☼ Reliability

Description

_pump_transport catches only timeouts, so the repository's socket transport raises
TransportError on a normal remote disconnect and _bridge_terminal re-raises it via
task.result(). Since app.py catches only KeyboardInterrupt, terminal mode exits with an error
and skips the command's transport.close() call.

Code

src/defib/cli/terminal.py[R99-101]

+            data = await transport.read(256, timeout=0.1)
+        except TransportTimeout:
+            continue

Evidence

The bridge retries only TransportTimeout and explicitly re-raises completed pump failures. The
socket implementation raises TransportError rather than returning empty bytes when its peer
disconnects, while the app's close call is after the terminal block rather than in a surrounding
finally.

src/defib/cli/terminal.py[92-106]
src/defib/cli/terminal.py[123-131]
src/defib/transport/socket.py[65-86]
src/defib/cli/app.py[403-416]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Transport disconnects and I/O failures currently escape the terminal bridge, and the burn command does not close the transport when they do.

## Issue Context
`SocketTransport.read()` represents remote EOF as `TransportError`, while `_pump_transport()` only handles `TransportTimeout`; `_bridge_terminal()` then propagates the completed task's exception. Preserve useful error reporting as appropriate, but ensure a remote disconnect ends terminal mode cleanly and transport cleanup always runs.

## Fix Focus Areas
- src/defib/cli/terminal.py[92-106]
- src/defib/cli/terminal.py[123-131]
- src/defib/cli/app.py[403-416]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Windows stdin stream ignored 🐞 Bug ≡ Correctness

Description

The Windows branch starts a console-only msvcrt pump and never passes or reads the stdin object
supplied to run_raw_terminal. Consequently, redirected or substituted stdin on Windows is never
forwarded to the transport and cannot reach EOF to stop the bridge.

Code

src/defib/cli/terminal.py[R115-118]

+    if os.name == "nt":
+        stdin_task = asyncio.create_task(_pump_windows_stdin(transport, stop))
+    else:
+        stdin_task = asyncio.create_task(_pump_posix_stdin(transport, stdin.fileno(), stop))

Evidence

The app supplies its stdin buffer, but unlike the POSIX branch the Windows branch discards that
argument; _pump_windows_stdin has no stdin parameter and obtains all input directly from msvcrt.
The repository explicitly supports Windows.

src/defib/cli/app.py[403-409]
src/defib/cli/terminal.py[72-89]
src/defib/cli/terminal.py[109-119]
README.md[127-127]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
On Windows, terminal mode ignores the supplied stdin stream and only polls console keyboard input, so redirected stdin is dropped.

## Issue Context
The CLI passes `sys.stdin.buffer` into `run_raw_terminal`, but `_pump_windows_stdin` accepts no stream. Keep console key handling for an interactive console while adding a path that reads the supplied stream when stdin is redirected or otherwise non-console.

## Fix Focus Areas
- src/defib/cli/terminal.py[72-89]
- src/defib/cli/terminal.py[115-118]
- src/defib/cli/app.py[403-409]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗