Renode driver: follow-up items from PR #533 review

[ambient-code]

Summary

Follow-up items from the review of PR #533 (Add Renode emulator driver) to be addressed in a separate PR.

---

Already addressed in PR #533

The following items from @raballew and @mangelajo were resolved in the PR itself:

• [HIGH] Command injection via load_command — validated against _ALLOWED_LOAD_COMMANDS allowlist
• [HIGH] Unrestricted monitor_cmd passthrough — gated behind allow_raw_monitor config flag (defaults to false)
• [HIGH] TCP socket leak on connect retry — stream closed before retrying in connect()
• [HIGH] execute() has no timeout — wrapped in fail_after(timeout) with 30s default
• [HIGH] execute() newline injection — rejects \n/\r with ValueError
• [HIGH] Missing workspace source entry — added to python/pyproject.toml
• [HIGH] Missing from jumpstarter-all — added to dependencies
• [MEDIUM] flash() defaults to LoadELF for all files — ELF magic byte detection added (_detect_load_command())
• [MEDIUM] _is_prompt false positives — constrained to _expected_prompts set
• [MEDIUM] Error detection only checks start of full response — now checks each line individually
• [MEDIUM] execute() newline injection — rejects newline characters
• Add Renode to CI workflows — install steps added for Linux (.deb) and macOS (brew)
• Convert ADR to JEP — replaced with docs/internal/jeps/JEP-0010-renode-integration.md
• [LOW] flash() deferred loading undocumented — docstring now explains store-for-later vs hot-load behavior

---

MEDIUM — Still open

• close() leaks SocketStream (driver.py, @raballew)
  close() sets self._monitor = None without closing the underlying TCP socket. off() properly calls disconnect(), but close() is synchronous and drops the reference. Restructure so shared cleanup lives in a single helper, or do a best-effort sync close of the socket.

• Private attribute traversal across sibling drivers (driver.py:96, @raballew)
  RenodeFlasher.flash() navigates self.parent.children["power"]._process and ._monitor directly. Same pattern in Renode.monitor_cmd().
  Fix: add a public method to RenodePower (e.g., send_monitor_command()) that encapsulates monitor interaction.

• Shared mutable parent state for firmware loading (driver.py:89, @raballew)
  RenodeFlasher writes self.parent._firmware_path / _load_command, RenodePower.on() reads them. Implicit contract with no interface enforcement.
  Fix: make firmware loading explicit — have RenodePower.on() accept firmware parameters, or coordinate through a method on Renode.

• Test gaps — error/cleanup paths (driver_test.py, @raballew)
  Missing tests for:

  1. on() fails mid-startup → verify process cleanup (the except Exception: await self.off(); raise path)
  2. connect() with persistent OSError → verify TimeoutError from fail_after
  3. _read_until_prompt with empty receive() → verify ConnectionError

• Idempotency tests lack assertions (driver_test.py, @raballew)
  test_power_on_idempotent and test_power_off_idempotent have zero assertions — they only verify "doesn't crash." Patch Popen/RenodeMonitor and assert they were NOT called.

LOW — Still open

• Dead hasattr guard (driver.py:91, @raballew)
  hasattr(self.parent.children["power"], "_process") is always True (dataclass field with default=None). Replace with power._process is not None and power._monitor is not None.

• off() inconsistent state on terminate() failure (driver.py:189, @raballew)
  If terminate() raises (e.g., ProcessLookupError on already-exited process), self._process = None is never reached. Wrap in try/finally.

• TOCTOU race in _find_free_port() (driver.py:47, @raballew)
  The ephemeral port is discovered by binding and immediately releasing a socket. A local process could race to bind the same port before Renode does. The QEMU driver avoids this with Unix domain sockets. Consider switching if Renode supports it.

• on() method too long (driver.py:167, @raballew)
  ~53 lines handling multiple responsibilities. Extract monitor setup into a private method like _configure_simulation().

• Decorative separator comments in tests (driver_test.py:28, @raballew)
  Six # 5.1 RenodeMonitor unit tests style separators reference internal plan task numbers and restate what the class name already says. Remove them.

• Minor test coverage gaps (driver_test.py, @raballew)
  Missing tests for: (1) _find_renode() when binary not on PATH; (2) RenodePower.read() NotImplementedError; (3) monitor_cmd() success path with allow_raw_monitor=True; (4) close() timeout-kill branch where wait() raises TimeoutExpired.

---

Source PR: #533
Reviewers: @raballew @mangelajo