Orchestrator (Claude) writes/verifies; a different model (astra, Codex gpt-6-astra) reviews adversarially. Every finding is reproduced against source before being accepted.

2026-10-03 — Milestones 0-2 firmware

# Sev Finding Verdict
1 High pdMS_TO_TICKS(2) is 0 at 100 Hz ticks, so the readiness wait is not ~100 ms and Shift+key back-to-back can time out Reproduced (CONFIG_FREERTOS_HZ=100 in generated sdkconfig). Fixed: 1 kHz ticks + deadline-based wait
2 High State mutated before send; a failed send makes the retry a silent no-op, leaving the host key held Confirmed by reading code. Fixed: snapshot/rollback on failed send (keys and buttons)
3 High release-all while suspended clears local state, sends nothing, returns OK Confirmed. Fixed: g_release_pending + housekeeping task retries until delivered
4 Med GET_REPORT always STALLs Confirmed. Fixed: returns current keyboard (8 B) / mouse (5 B) input report
5 Med SET_IDLE accepted but idle-rate retransmission not implemented Accepted as real, deferred to roadmap (not needed for M2; hosts normally do their own typematic repeat); hardware check added
6 Med Uppercase self-test yields lowercase if target Caps Lock is on Confirmed. Fixed: track Caps Lock from LED output report and invert Shift for letters; also documented as a prerequisite
7 Low xTaskCreate result ignored for self-test Confirmed. Fixed: logged; USB stays up

astra also reported no defect in endpoint addresses/sizes, esp_tinyusb 1.7 config fields, N16R8 flash/PSRAM settings, or the task-WDT interaction with the 8 s delay, and confirmed the 5-byte boot mouse report is permitted by HID 1.11 Appendix B (so the “least-certain” mouse item in hardware.md is lower risk than first stated, but still needs a real BIOS to confirm).

2026-10-03 — MVP 0.1.0: BLE link, protocol, desktop app (review round 2)

# Sev Finding Verdict / fix
1 High Encryption accepted without bonding; an unbonded peer could type Accepted. ENC_CHANGE requires sec_state.bonded, else disconnects
2 High Secure Connections enabled but not required (legacy fallback) Accepted. CONFIG_BT_NIMBLE_SM_LEGACY=n
3 High Pairing across window expiry kept the old controller’s bond Accepted. keep_only() runs on every bonded encryption, window or not
4 High Full queue could drop the session-end event, leaving keys held Accepted. Session start/end are not queue items; worker reconciles a generation counter
5 High Queued frames crossed connection boundaries Accepted. Queue items carry the connection generation; stale ones are dropped
6 High Seq wrap replayed a cached ACK instead of releasing a key Accepted. Dedup entries age out (PROTO_DEDUP_AGE_MS) and match seq+type
7 High Failed KEY_TAP release left the key held Accepted. Failed up path releases/cleans up before reporting
8 High Dropping the last client handle kept keepalives running Accepted. Drop for Inner stops the tasks so the firmware watchdog can release
9 Med Invalid traffic starved the keepalive watchdog Accepted. Watchdog runs on wall time every 250 ms, independent of traffic
10 Med Cache eviction let a retry repeat a non-idempotent action Accepted. Dedup cache retention covers the retry window
11 Med Client seq wrap overwrote outstanding requests Accepted. Occupied sequence numbers are not reused
12 High Unrelated ACK could confirm RELEASE_ALL Accepted. Replies are matched on seq and type (ERROR on its original type)
13 Med HELLO could not complete at the default ATT MTU Accepted. Preferred MTU 247; below the minimum the device answers MTU_TOO_SMALL instead of timing out
14 Med A 30-32 byte name disabled advertising Accepted. Scan-response name truncated to 29 bytes (marked incomplete)
15 Med Clicks could overtake accumulated motion Accepted. Pending motion is flushed before a button request
16 Med Secret defaults/literals serialised as plaintext Accepted. Serialisation redacts secrets
17 Med Debug exposed secrets and reconstructable keystrokes Accepted. Redacting Debug impls
18 Med Secret copies left in ordinary heap allocations Accepted. zeroize on substituted text and rejected characters
19 Med Nested repeats overflowed preview counts Accepted. Saturating arithmetic in preview/duration estimates
20 Med Horizontal scroll ACKed but not applied Accepted. h passed through to usb_hid_mouse_wheel

2026-10-03 — BLE bring-up on hardware; NimBLE connect panic; radio diagnostics

Review round 3 (astra, gpt-6-astra, high effort, read-only, static)

3a: first fix (MAX_CONNECTIONS=6 + app guards) and the diagnostic build. 3 findings.

# Sev Finding Verdict
1 High With 6 host slots, a refused second link could still bond: NimBLE persists keys in ble_sm_persist_keys() before ENC_CHANGE, ble_gap_terminate() failures were ignored, and CONNECT is delivered late (after remote-feature read), so a BOOT press could restart advertising while a link existed Confirmed against the v5.5 NimBLE source (persist order; store_write_cb called under the host lock; delayed CONNECT). Fixed in 3b, then superseded (see 3b)
2 Low Diagnostic log printed advertiser names verbatim (newline/ANSI injection) Confirmed. Names reduced to printable ASCII
3 Low KVMIT_RADIO_DIAG_SCAN_S accepted 0/negative/overflowing values Confirmed. Kconfig range 1 300

3b: app-level trust gate (a store_write_cb wrapper refusing keys unless one link and an open window; live_links() via NimBLE’s private ble_hs_conn_foreach; handle-bound restart). 6 findings; astra confirmed 3a-2 and 3a-3 fixed.

# Sev Finding Verdict
1 High NimBLE ignores store-write failures: a refused peer still gets ENC_CHANGE as bonded, keep_only() then deletes the real bond; partial OUR_SEC/PEER_SEC writes possible at window expiry Confirmed (ble_sm_persist_keys discards both return values)
2 High Counting links does not bind persistence to the admitted link; BOOT-task/host race between the count and adv_start; legitimate CCCD writes refused while two links exist Confirmed by reading the code paths
3 Med Security completed before the delayed CONNECT is discarded (g_encrypted reset) Plausible ordering; accepted
4 Med Advertising can stay stopped after the last link goes (refused-link DISCONNECT ignored; NimBLE suppresses DISCONNECT when CONNECT was never sent) Confirmed in source
5 Med The handle guard misses other raw-handle accesses: CONNECT with non-zero status, pre-CONNECT disconnect, and the g_max_* data-length arrays Confirmed: four more arrays indexed the same way
6 Low The gate also refused NimBLE’s local-IRK write at boot (no links yet) Confirmed

Resolution (all of 3a-1 and 3b): the app-level approach was abandoned. Both reviews named the structural alternative, and it removes every one of these orderings at once: firmware/patches/esp-idf-v5.5-nimble-conn- handle-index.patch moves slave_conn and g_max_{tx,rx}_{time,octets} into struct ble_hs_conn (zeroed on allocation, freed with the link), so no state is indexed by raw handle. CONFIG_BT_NIMBLE_MAX_CONNECTIONS is back to 1: NimBLE refuses to start connectable advertising while its one slot is used (ble_hs_conn_can_alloc() → BLE_HS_ENOMEM), so no second link can exist and the store gate, link counting, handle-bound restart and private API declarations were all removed from ble_link.c. scripts/fw.sh applies firmware/patches/*.patch in every container run; firmware/CMakeLists.txt fails configuration if the IDF is unpatched (checked by configuring against a clean image). nm confirms neither ELF contains the old arrays. Kept from the earlier fix: the handle in the connect log line and a logged ble_gap_terminate() failure.

3c: final firmware diff (NimBLE patch, MAX_CONNECTIONS=1) plus the GUI “Send keys” change. astra found the patch correct and complete (all 11 hunks match esp-nimble cc3ac541; no other consumers of the removed symbols in 348 host/port files; zero-init and locking sound; fw.sh wrapper sound) and raised 7 findings, mostly older lifecycle issues the earlier rounds had not reached:

# Sev Finding Verdict / fix
1 High NimBLE accepts SMP bonding as soon as a link exists, before the delayed CONNECT runs our window check, so a peer could bond with the window closed and keep_only() would then drop the real bond Accepted (predates today). ble_hs_cfg.sm_bonding is now 1 only while the window is open (set_window()); outside it no keys are persisted and the existing “peer did not bond” path disconnects. Bonded re-encryption is unaffected (hardware-checked)
2 Med A delayed CONNECT reset g_encrypted after encryption was already up Accepted. CONNECT adopts sec_state.encrypted && bonded and only requests security if not encrypted
3 Med A link lost before CONNECT gets no DISCONNECT (NimBLE suppresses it), so advertising stayed stopped Accepted. The worker’s 250 ms tick restarts advertising when idle and it should be advertising; adv_start treats EALREADY/ENOMEM as expected
4 Med GUI: captured Enter/Space could activate a focused “Send keys” button Accepted. Chord buttons disabled while capturing; focus surrendered when capture begins
5 Med GUI: chord key-up errors ignored, so a key could stay held Accepted. Any failure sends RELEASE_ALL and shows a notice
6 Med GUI: chords could interleave with scripts, Type, or capture release Accepted. chord_busy serialises them: chords disabled while a script/capture/chord runs; start_run and begin_capture refuse while a chord is in flight
7 Low The CMake guard accepted a half-applied patch Accepted. It also fails if ble_gap.c still contains the raw arrays

Also from 3c: the patch now sets data_len_chg.conn_handle (upstream left it 0), and docs no longer claim “no second link on any path” beyond the configuration used (extended advertising is disabled; its start path does not recheck capacity).

3d: verification of the 3c fixes (run after a reviewer usage-limit delay). Confirmed sound: NimBLE (cc3ac541) reads sm_bonding when building the responder’s pairing response and persists keys only if both sides bond; bonded re-encryption does not depend on it; delayed-CONNECT adoption; advertising recovery; chord error paths; patch hunks and the CMake guard. 6 findings, all accepted:

# Sev Finding Fix
1 High Window deadline and sm_bonding were written from three tasks (BOOT, esp_timer, host) without synchronisation; an interleaving could leave “window closed, bonding enabled” Every window/bonding/advertising change now runs on the NimBLE host task (the task that runs SMP): BOOT, the window timer, trust reset and the worker tick only post ble_npl_events (ev_open, ev_close, ev_reset, ev_reconcile)
2 Med The advertising-recovery tick could race window expiry and leave forbidden advertising running ev_reconcile on the host task both starts and stops advertising to match policy
3 Med During capture, Tab/Enter could still drive other sidebar widgets (Type, Disconnect, Release all) The whole sidebar is disabled while capturing
4 Med chord_busy did not cover capture’s queued keys/RELEASE_ALL draining, overlapping script starts, or Release all vs a pending chord Chords and Release all now run inside the ordered input pump; a pending counter (InputTx) tracks queued + in-flight items; input_idle() (no capture, no run, nothing pending) gates chords, Type and start_run
5 Med If a chord’s key-up and the follow-up RELEASE_ALL both failed, keepalives kept the held key alive The pump then closes the session (shutdown), so the adapter’s link-drop/keepalive release applies
6 Low Type cleared its text before a busy rejection Type is disabled unless input is idle, so start_run cannot reject it

3e: verification of the 3d fixes. Confirmed sound: host-task ownership removes the BOOT/ENC_CHANGE interleaving and the advertising check/start race; reconcile stops forbidden advertising; event init precedes the BOOT task’s first post; disabled sidebar cannot be re-enabled by nested widgets; one healthy pump orders chords, capture and RELEASE_ALL. 3 findings, all accepted:

# Sev Finding Fix
1 High After the deadline passed, sm_bonding stayed 1 until the queued ev_close ran, so a pairing admitted in that gap bonded after “expiry” The window is now host-task state: open until set_window(0) runs on the host task, the same step that clears sm_bonding; the timer only requests the close. “Window open” and “bonding allowed” can no longer disagree
2 Med Resetting a shared pending counter when replacing the pump let the old pump’s late completions corrupt (and wrap) it Each pump owns its counter (InputTx::pending); input_idle() reads the current pump’s
3 Med After an automatic reconnect the GUI kept using the pump bound to the closed Device Pumps are keyed by connection session (Device::same_session, Arc::ptr_eq), not adapter id

2026-10-03 — Pairing window at every power-on

Review round 3f (astra, on the two commits above). Confirmed sound: per-pump counters and session-keyed pumps; host-task serialisation; no pairing/disconnect ordering lets a second new controller use a consumed window (NimBLE persists the bond and delivers ENC_CHANGE in the same host execution); bonded reset classification is conservative (USB-JTAG, brownout, deep sleep, software, panic, watchdog excluded). 2 findings, both accepted:

# Sev Finding Fix
1 High A NimBLE host reset re-runs on_sync(); esp_reset_reason() still says POWERON, so the 15 s window would reopen without any physical action The power-on window is offered once per chip boot (boot_window_used); a host re-sync only restores advertising
2 High The deadline was soft: until the queued close ran on the host task, a fresh SMP request could still be admitted The window timer’s callback clears sm_bonding at the deadline (it only ever clears; the host task alone sets it, and ev_close undoes a stale clear after a reopen). NimBLE has no SMP-admission hook, so the residual gap is esp_timer dispatch latency

Hardware-verified after 3f (release build): reset, wait past 15 s, kvmit pair refused; reset, kvmit pair at ~1.2 s paired and closed the window; status 90 ms.

2026-10-03 — Typing over BLE into a real target

2026-10-03 — LED, trust reset, replug

2026-10-03 — Keepalive release of a held key (deterministic)

2026-10-03 — GUI check and release

2026-10-04 — 0.2.0 (Windows controller): review round 4

# Sev Finding Verdict
1 High A hung GUI keeps the hook swallowing the controller’s keyboard (release depended on the GUI) Confirmed; my “a hung GUI cannot trap the keyboard” comment was false. Fixed: the helper handles the release chord itself (stops swallowing in the hook), and stops swallowing if the GUI’s heartbeat (sent from the input-handling thread) stops for 3 s, telling the parent to release. Hardware-verified in the VM: GUI frozen with NtSuspendProcess, Win swallowed during the grace period, then opened the controller’s Start menu after the timeout; the thawed GUI ended capture
2 Med Keys held before capture: repeat becomes a captured press, release swallowed (stuck key); pre-held Ctrl breaks the chord Confirmed. Fixed: the helper seeds the tracker from GetAsyncKeyState; pre-held keys’ repeats/releases pass to the OS and count for the chord. Unit-tested, not hardware-tested
3 Med Alt+PrintScreen (0x54) and Ctrl+Pause (ext 0x46) scan codes missing Confirmed. Fixed + test
4 Med Windows silently removes a slow hook; capture stays “on” Plausible. Mitigated: a key press that reaches egui while the grab is on means the hook is not swallowing, so the GUI drops the grab and falls back to egui keys. Not hardware-tested (cannot force a hook timeout)
5 High Keys/clicks queued after the release chord are still forwarded Confirmed. Fixed: until_release makes the chord a terminal boundary (keys and the frame’s mouse events); unit-tested. The helper also stops swallowing at the chord
6 Med Hook keys and egui mouse buttons are two streams: Ctrl-click can reorder Confirmed; deferred. Needs one ordered stream (mouse buttons through the helper’s WH_MOUSE_LL). Documented limit: modifier+click ordering can break if a GUI frame takes longer than the click. Tracked in the roadmap backlog
7 Med Keyboard-only capture without video: no prompt GUI wakeup Plausible, fixed: the helper’s reader wakes the GUI per event, and capture repaints every 100 ms (also the heartbeat)
8 Med The 16-frame motion cap does not bound the unbounded transport queue Confirmed (inherited); deferred. The cap bounds work per frame (the observed link-killing failure); end-to-end backpressure needs a bounded transport channel. Backlog
9 Med Timer vs click: a click can overtake motion between take and send Confirmed (inherited, affects Linux). Fixed: motion is taken and sent under the accumulator lock; a test asserts wire order for 50 interleaved clicks. Linux not re-run on hardware
10 High A lost ReleaseAll at capture end leaves target keys held while keepalives continue Confirmed (inherited). Fixed: a failed ReleaseAll closes the session so the adapter’s link-drop release takes over
11 Med Pairing accepts numeric comparison without comparing Confirmed. Fixed: only ConfirmOnly (Just Works) is offered and accepted
12 High Dropping a Media Foundation capture can hang the GUI (blocking ReadSample) Plausible. Fixed: bounded wait (1.5 s) then detach. Not hardware-tested
13 Med MF_SOURCE_READERF_ERROR retried forever; frozen video stays “good” Confirmed. Fixed: terminal on ERROR/EOS or 20 failed reads; Capture::failed(); the GUI drops it and says so. Hardware-verified in the VM by unplugging the card mid-stream. The V4L2 backend has no equivalent yet
14 Low CoInitializeEx never balanced Confirmed. Fixed (guard calls CoUninitialize)
15 High hookcheck prints pressed-key identities Confirmed: a violation of the no-logging rule by my own example. Fixed: counts only, ends at the chord
16 Low linkstress can exit 0 despite failed pings/no video frames; hz misdescribed Confirmed. Fixed

Review round 5 (re-review of the round-4 fix commit d73c0b5)

astra (gpt-6-astra, high effort, read-only, static; git diff d19559d d73c0b5): 6 findings, 3 High, all in the fixes themselves (the reason STD-001 re-reviews after a fix round). Verdict “do not merge yet”. Adjudicated by the orchestrator:

# Sev Finding Verdict
1 High The egui fallback after a failed grab can leave a forwarded right-hand modifier held on the target Confirmed. Fixed differently from the suggestion: no fallback at all; the GUI ends capture (which sends release-all and drops the grab)
2 High A pre-held key’s repeat counted as “hook failed”; a late release notice could be lost in the same fallback Confirmed. Fixed: the evidence is now only a fresh (non-repeat) press of a key the grab maps, and the response is the same safe end-capture; unit-tested predicate
3 High The release chord and a click in the same frame re-capture at once Confirmed. Fixed: released_this_frame blocks begin_capture for that frame
4 Med Seeding held keys from virtual keys cannot tell Enter from keypad Enter (shared VK) Confirmed, inherent; accepted and documented (CHANGELOG known limitations): a key of such an alias held across the instant capture begins can misbehave once
5 Med The held-keys snapshot is taken before the hook is installed Confirmed (small). Fixed: sampled and installed under the state lock inside the hook thread
6 Med A blocked stdout write stops the heartbeat watchdog Confirmed. Fixed: the watchdog is its own thread and turns swallowing off itself

2026-10-04 — Video-device switch, logo, Linux AppImage: review round 6

Three branches reviewed read-only by a different model (codex gpt-6-astra) before merging: fix/video-device-switch (A), branding/logo (B), linux-appimage (C). Findings were adjudicated by reading the code; each fix is in the branch it belongs to.

Br Sev Finding Verdict
A Med Switching back to the demo after a failed open leaves the picture blank (last_seq kept, still source always seq 1) Confirmed. Fixed: sequence reset on every switch
A Med No way to reopen a stalled device now that the dropdown’s Open is gone Confirmed. Fixed: clicking the open row reopens it
A Med Demo path saved as last_video; video snap opens demo: without the env var Confirmed. Fixed: never saved; opening is opt-in like listing
A Med Oversized PNG can assert in the texture upload Confirmed. Fixed: dimensions checked (max 8192 a side) before decode; regular files only (also covers the FIFO remark)
A Low Non-UTF-8 demo path corrupted by lossy conversion Confirmed. Fixed: such a path disables the source
A High (inherited) Dropping the old capture under the shared mutex can hang on a stalled V4L card Partly fixed. The drop now happens outside the lock, so scripts’ screen() is not blocked; the V4L thread’s unbounded frame wait is unchanged and stays a known limitation (no hardware occurrence)
B — No substantive finding —
C Med --runtime-file not given: the embedded runtime floats Confirmed. Fixed: runtime pinned by URL and sha256 (digest taken on first use from the tagged release)
C Med glibc check passes if objdump fails (no pipefail in the container shell) Confirmed. Fixed: inspect separately, failure is an error
C Med Host libraries not bundled (e.g. libxkbcommon-x11) Confirmed, by design. Documented in README; the script prints what the binaries link; no bundling
C Med Not reproducible: floating base image, apt, file times Partly fixed. Base pinned by digest, times from the commit; apt floats and is documented as such

2026-10-04 — 0.3.0 flasher (feat/flash): review round 7

Read-only review by a different model (codex gpt-6-astra) of main...feat/flash at 8bc17e9: verdict “do not ship until device identity, native-USB detection, NVS preservation and flash-operation lifecycle are enforced”. 13 findings, all confirmed by reading; fixed in the commit after this log.

# Sev Finding Verdict
1 High A reused port name (/dev/ttyACM0) bypasses the checks: nothing re-identifies the device at flash time Confirmed. flash() now takes the chosen PortInfo (name, vid:pid, USB serial), re-enumerates, and refuses if the same name is another device or gone. A small window between the check and the open remains (stated, unavoidable)
2 High The native-USB rule is blind: the adapter’s native port is HID-only and is not a serial port at all Confirmed, and the worst one. Seen on the real board (303a:4008, no CDC). Now found by enumerating USB devices (nusb): the adapter’s pid, a “kvm-it” name, or an unreadable Espressif device blocks; a differently named Espressif board does not. Checked in the CLI before the prompt, in the wizard on open/rescan, and again inside flash(). A cable plugged in after the check is not detectable
3 High CH343 does not prove it is a kvm-it board; the chip is never checked Confirmed. After connecting: must be an ESP32-S3 with the image’s flash size and not in secure download mode, else nothing is written. Another ESP32-S3 with 16 MB behind a CH343 is still accepted; the person’s choice and confirmation remain the last line
4 High Validation ignores padding and erase sectors Confirmed. Sector footprints (4 KiB) are used for overlap and for the settings check
5 High A replaced partition table can move or shrink the settings Confirmed. The installed table is read from the chip and its nvs region must equal the new image’s, else a full erase is required
6 High “Firmware OK” accepts one-byte images Confirmed. Header checks (magic, segment count, ESP32-S3 chip id), a partition table with an app partition at the app’s offset that fits it, nvs required. No checksum/hash walk of the images; they come from our own build
7 High Manifest paths can read files outside the folder Confirmed. Relative-only, no .., symlinks resolved and contained, regular files only
8 Med Reads are unbounded; FIFOs hang; the wizard loads on the UI thread Confirmed, mostly. Manifest 64 KiB, 8 entries, files regular and at most the flash size, duplicate keys rejected. Loading is still on the UI thread, now bounded and cannot block on a FIFO
9 High Closing the wizard or the app mid-flash abandons a running write Confirmed. No close button while writing, no second wizard, and an app close request is cancelled with a notice while flashing
10 Med Flashing ignores scripts and the adapter connection Confirmed. Blocked while a script runs or the controller is connected/connecting; the wizard is hidden while capturing
11 Med “Verified” claimed even in secure download mode Confirmed. Secure download mode is refused
12 Med espflash needs Rust 1.95, the AppImage builder has 1.90 Confirmed. AppImage builder moved to 1.99; rebuilt, glibc floor still 2.35, cli flash --list runs from it
13 Med serialport is MPL-2.0 and nothing in the packages says so Confirmed. THIRD_PARTY_NOTICES.md (shipped in the zip, the MSI and the AppImage; verify-release.py expects it). A generated all-dependencies licence listing is still to do

Round 8 (re-review of the round-7 fixes, 8bc17e9..59a6f22)

The reviewer’s verdict: “do not ship until wrong-device/native-port gaps, temporary-file safety, validation/preservation holes and BLE cancellation are fixed and re-reviewed”. Of the 13 round-7 fixes it called 4 fully fixed, 7 partial and 1 regressed (round 7’s findings 3, 9, 11, 12, 13 fixed; 1, 2, 4, 5, 6, 8 partial; 10 regressed by the wizard’s own connection cancel). 13 new findings (F1-F13):

# Sev Finding Verdict
F1 High Bridges with no (or duplicate) USB serial number: a swapped board under the same port name compares equal Confirmed; narrowed, not closed. Without a serial two identical boards cannot be told apart by software. The app now says so beside the choice (GUI and CLI), re-checks the chip is an ESP32-S3 of the right flash size with a compatible settings layout, and prints the chip’s MAC when done. The residual (a physical cable swap in the seconds between choosing and confirming, onto a board that passes all of those) is accepted
F2 High The adapter’s ROM-mode native port (303a:1001, generic debug unit) was explicitly allowed Confirmed. My test blessed it as “unrelated”. Now any Espressif USB device blocks flashing (an adapter’s own port cannot be told from another ESP board), documented
F3 High USB enumeration is not a complete inventory (nusb drops unreadable devices on Linux; half-readable descriptors slipped through) Partly fixed. The descriptor-string heuristics are gone (vendor id alone decides). A device the OS will not let nusb describe can still be missing from the list, and a cable plugged in after the check is not seen: stated limits
F4 High The installed-table temp file is a predictable, replaceable path Confirmed. A private, exclusively created, owner-only folder, removed on drop; the file is read as a bounded regular file
F5 High Invalid images pass (bare 24-byte header; partition table with a flipped MD5) Confirmed. Images are walked like the ROM does (segments inside the file, XOR checksum, appended SHA-256); the table is parsed strictly (bad entries, MD5 verified, data after the MD5, ranges, overlaps, one nvs). The committed release images are checked by an always-on test
F6 High Equal NVS ranges do not prove the pairing stays reachable (labels, flags ignored) Confirmed. The settings partition must be identical (offset, size, name, flags) in the installed and new tables; exactly one nvs partition, named nvs (what the firmware opens)
F7 Med Table location accepted anywhere, read-back always at 0x8000 Confirmed. The table part must be at 0x8000
F8 Med Erase footprint arithmetic assumes aligned offsets Confirmed. Every part and partition must be 4 KiB aligned
F9 Med Opening the wizard does not cancel or exclude BLE work (retry in Failed, in-flight connect publishing late, Adapter popup still usable) Confirmed. An attempt cancelled while in flight disconnects and never publishes (also on its error paths); the wizard cancels Connecting and Failed loops without forgetting the remembered adapter; Scan/Connect are disabled while it is open. A live connection must still be Disconnected by the person
F10 Med Containment/size checks race the actual open Partly fixed. One open, checks on the handle, bounded read. A concurrent swap of a directory component needs write access to the firmware folder: accepted
F11 Med Implicit firmware lookup can pick attacker-controlled firmware (cwd; env var wins) Partly fixed. Release builds never look in the working directory; only a debug build tries firmware/release. KVMIT_FIRMWARE still overrides on purpose (shown in the wizard and the CLI before confirming); there is no runtime signature on firmware
F12 Med The CI freshness guard compares commit ids (stale images pass, squash merges fail, scripts not covered) Confirmed. A content hash of the firmware build inputs (sources, patches, lockfile, scripts/fw.sh) is recorded in FIRMWARE.txt and checked by CI and by verify-release.py (which the signed release runs). It binds the inputs to the record, not to the output bytes (builds are not reproducible)
F13 Med Windows autocrlf can rewrite the bundled JSON so its checksum is stale; verification never checks the inner sums Confirmed. .gitattributes marks firmware/release/** as -text; verify-release.py verifies the zip’s inner SHA256SUMS against its files

Round 9 (re-review of the round-8 fixes, 59a6f22..7b8eafa)

Verdict “do not ship yet”: F5, F9, F10, F12, F13 incomplete, two regressions of mine, and a coverage gap. 11 findings (+1 low):

# Sev Finding Verdict
1 Med A cancelled BLE attempt can still publish Connected (generation checked before taking the link lock) and its poll loop never notices cancellation Confirmed. Every link-state write now goes through one publish that checks the generation under the link lock; the cancel bumps it under the same lock; the connected poll loop exits when cancelled
2 Med pair_first paths write state unguarded; a failed handshake drops the connection without disconnecting (Connection has no Drop, its doc said it did) Confirmed. Pair paths guarded; session::connect disconnects on handshake failure; the doc is corrected. Cancelling does not abort an in-flight OS operation, it closes it when it completes
3 High Image walk accepts hash flag 2 (skipping SHA) and odd segment lengths Confirmed. Any nonzero flag means a digest is present and checked; segment lengths must be multiples of 4; checked arithmetic (also the 32-bit low finding)
4 High A table filled to its last slot (no terminator) passes Confirmed. An erased terminator entry inside the 0xC00 window is required
5 High App not on a 64 KiB boundary, a 0x1000 data/ota, bounds from a global maximum Confirmed. App partitions on 64 KiB boundaries, OTA data exactly 0x2000, partition bounds from the image’s flash size. This is a bounded rule set, not a reimplementation of ESP-IDF’s partition semantics
6 Med Valid installed tables without an MD5 entry are refused (needless “full erase”) Confirmed. The MD5 is required of the new table only; an installed table without one is accepted for the settings comparison
7 Med Opening a FIFO blocks before it can be rejected Confirmed (my regression). The file type is checked before opening, and again on the handle
8 Med The manifest is read unbounded after its size check Confirmed. One bounded reader for the manifest, parts and read-back; too big is an error, not a truncation
9 Med autocrlf rewrites the hash inputs on Windows, so the freshness check fails there Confirmed (my regression). .gitattributes: firmware/** and scripts/fw.sh are -text
10 Med Inner SHA256SUMS coverage not required complete or unique Confirmed. Must cover exactly the bundled files (except itself and FIRMWARE.txt); duplicates refused
11 High Native USB of an S3 already running other firmware under another VID is not detected Accepted, documented. Nothing in the descriptors says that device is an adapter; the rule stays “any Espressif device blocks”. A stated detection limit alongside the OS-hidden-device and late-cable ones

Round 10 (re-review of the round-9 fixes, 7b8eafa..18f2916)

Verdict: “Medium BLE lifecycle defects remain; no High defect remains within the reviewed scope and stated accepted residuals”. The reviewer re-derived that the shipped firmware passes every check and found no path to the wrong port, a default erase of the pairing, or typing into the flashing machine. 3 Medium, 2 Low, all confirmed, all in gui.rs start/cancel and the parser boundary:

# Sev Finding Verdict
1 Med A superseded attempt (start_link(B) while A is mid-publish) can still publish: the bump did not take the link lock Confirmed. start_link bumps the generation and sets Connecting under the link lock, in the same step
2 Med An attempt queued but not yet running (state still Disconnected) escapes the wizard’s cancel Confirmed. Same change: the attempt is registered as Connecting synchronously, so cancel_pending_link always sees it
3 Med A pairing that succeeds after cancellation leaves BlueZ’s link open with no owner Confirmed (Linux). After a cancelled pairing succeeds the controller calls the new backend::release(id) (BlueZ Disconnect; no-op elsewhere). Compiled, not exercised: the Bluetooth adapter belongs to the VM. (Superseded in round 11: removed.)
4 Low Cancelled attempts can still write status and notices Confirmed. Writes check the generation; stop_link and cancel_pending_link clear the status
5 Low The public parser accepts a slice longer than the 0xC00 window Confirmed, not exploitable (callers cap at 0xC00). The parser now refuses it

Round 11 (re-review of the round-10 fixes, 18f2916..21aa7c1)

Round-10 findings 1, 2 and 5 verified fixed. The new backend::release (round-10 finding 3) drew three Medium findings and the status-guard fix a Low one:

# Sev Finding Verdict
1 Med Cancel between pairing success and the connection loop bypasses the release cleanup Confirmed.
2 Med release identifies an address, not an owner: a stale pairing’s cleanup can disconnect a newer attempt’s wanted connection Confirmed (regression from round 10).
3 Med release re-resolves the default controller and can miss the one used for pairing (hci0/hci1); errors are discarded Confirmed (conditional on controller hotplug).
4 Low Generation checks are separate from the writes: a stale status/notice write can still land after a cancel Confirmed.

Decision: backend::release is removed rather than patched. It was Linux-only code that cannot be exercised on this rig and each fix invited another ownership question. A pairing cannot be cancelled and takes seconds, so the case is removed by construction: the Flash adapter button is disabled while a pairing is in flight (and Pair & connect is already disabled while the wizard is open). For finding 4, all writes an attempt makes (link state, status, notices) now run under the link lock only if the attempt is still current, and the cancels clear state under the same lock (lock order: link, then status/notice), so a write cannot slip between a check and the cancel.

Round 12 (re-review of the round-11 fixes, 21aa7c1..af10e5c)

Round-11 findings 2, 3 and 4 verified resolved (removing release removed its ownership and controller problems; attempt writes are atomic with the cancels). The replacement design’s pairing flag drew three Medium findings, all confirmed:

# Sev Finding Verdict
1 Med The flag is cleared as soon as pairing returns, before the connection loop owns anything: a cancel in that gap still strands the link Confirmed. The guard is now held until the first connection attempt has resolved (Connected published, or failed)
2 Med A queued pairing can start after the wizard opened (the flag is set inside the task; the entry generation gate was gone) Confirmed. The pairing is registered synchronously in start_link, before the task is spawned, and the task checks its generation before calling pair
3 Med One shared boolean: overlapping pairings clear each other’s flag Confirmed. A counter with a drop guard (PairingGuard): registered per attempt, released on drop even if the task panics or is dropped

Round 13 (final verification of the round-12 fixes, af10e5c..96cb43f) — reviewer: Claude Opus 5.5

Deviation from STD-001, recorded: the usual reviewer (codex gpt-6-astra) was out of quota until 23:55 on 2026-10-04 (round 13 failed with a usage limit and produced no findings). With the maintainer’s explicit agreement, this round was done by a Claude Opus 5.5 subagent (read-only, same scope and prompt shape as the Codex rounds), a different model from the one that wrote the code but in the same family. A Codex round 13 over the same diff is to be run after the quota resets and its result logged here; it also covers the follow-up below.

Result: the three round-12 findings are resolved; no High or Medium defect; three Low (plausible) items, accepted for 0.3.0 and listed in the CHANGELOG known limitations:

# Sev Finding Disposition
1 Low A pairing that is superseded (a deliberate second click on Connect while it runs) returns without connect and may leave BlueZ’s leftover link with no owner; the wizard only checks Connected, so it cannot see it Accepted for 0.3.0 (needs a second click inside the pairing seconds; the link is idle and flashing resets the board, which drops it). Fix planned: disable Pair & connect / Connect while a pairing is in flight
2 Low backend::connect returns early with ? after p.connect() succeeded (on discover_services/subscribe/notifications errors) without disconnecting; if that is the first attempt after a pairing the guard is released with the link open Accepted for 0.3.0 (older code outside this diff). Fix planned: disconnect on those error paths
3 Low The guard is held for as long as pair plus the first connect take; neither has an overall timeout, so a hang keeps Flash adapter… disabled until it returns Accepted; no path leaks the count permanently

Checked and found sound by the reviewer: guard lifetime and take() placement; no task leaves the state Connecting; lock order (link, then status/notice) and no await under a std mutex; cancel_pending_link/stop_link; auto-connect at startup; no path to flashing while connected or pairing other than the two orphan links above.

Release cut (0.3.0): verifier found a latent flaw

The first v0.3.0 release run failed in verify-release.py (“kvmit-gui.exe: the version 0.3.0 is not stamped into the binary”); signing, packaging and the new firmware checks had all passed. The check searches the executable for the version string; for 0.2.1 it passed only because the dependency path windows-future-0.2.1/... is in the binary. The GUI had never embedded its own version. Fix: the window title is kvm-it <version> (a test guards it). Nothing was ever attached to the v0.3.0 release, so with the maintainer’s agreement the tag and the empty release were deleted and 0.3.0 re-cut on the fixed commit. (The Opus-5.5 review did not cover this; it is one format string and a test.)

2026-10-05 - 0.4.0 boot drive (feat/ipxe): review round 14 (Claude Opus 5.5)

Deviation from STD-001, recorded: the usual reviewer (codex gpt-6-astra) was out of quota (it resets at 23:55), and the maintainer chose to release 0.4.0 on a Claude Opus 5.5 review (“if opus signs off … before codex”). A Codex round over the same diff is queued and its result will be logged here. The Opus subagent was read-only.

First report (main...feat/ipxe at a54bd50): no High; three Medium; six Low. Adjudication, all by reading and then by test:

# Sev Finding Verdict
M1 Med The new image script is a hash input but not -text: a Windows CRLF checkout breaks fw-source-hash.py --check in verify-release.py Confirmed (same class as round 9 #9). .gitattributes now covers scripts/build-ipxe*.sh
M2 Med The drive is always on and its default script boots over HTTP unattended, then drops to an iPXE shell forever on failure; legacy BIOS runs empty boot code; real-PC keyboard enumeration unchecked Confirmed. Default script is now inert: waits 5 s for n, else exit (and exit on failure); the boot sector is INT 18h and no partition is active; docs state the boot-order effect and the escape hatch (flash the 0.3.0 firmware; pairing kept); a switch to hide the drive is on the roadmap. Tested in a UEFI VM (no key: falls through to the boot menu; n: HTTPS demo, then a network Linux). Not exercised: a real PC’s keyboard enumeration with the composite device
M3 Med Licence wording (“additional permissions for UEFI”) is unsupported; a link plus “open an issue” is not a GPL source offer; the commit was only partly known Confirmed. Wording corrected (GPL-2.0-or-later, many files also UBDL). The exact commit was resolved (6262f1081fe1..., v2.0.0-375); the official binary did not reproduce, so iPXE is now built from that pinned unmodified commit by scripts/build-ipxe.sh (reproducible: SOURCE_DATE_EPOCH pinned), and the source archive is attached to the release
L1 Low TinyUSB overrides the read-error sense data Confirmed; documented in the code. Reads past the end are a host bug
L2 Low The write-refused counter cannot rise; “writes refused” evidence was the host’s own write-protect flag Confirmed. Tested properly on the real adapter with raw SCSI (SG_IO) bypassing the host: WRITE(10) refused as DATA PROTECT, WRITE(6)/(12), FORMAT UNIT, WRITE SAME, UNMAP as invalid commands, disk hash unchanged. Docs reworded
L3 Low README said a Windows target “only sees a USB keyboard and mouse” Confirmed, fixed
L4 Low refresh-firmware-release.py left ../ipxe/ipxe.img in the per-part manifest blocks Confirmed, fixed
L5 Low Nothing ensured ipxe.img was rebuilt from current inputs Confirmed; the refresh script now rebuilds it first
L6 Low Leftovers from the product-id change Doc comment fixed; any VM passthrough by vid:pid needs 400a
tests - The “never into the settings/phy_init” tests failed on an earlier check The data-image rule is now a function tested directly against a table with every partition kind, plus a four-part manifest end to end

Round 14, continued: Codex result, the off-by-default toggle, and the sign-off

2026-10-06 - 0.4.1 Secure Boot boot drive (feat/secure-boot, PR #15): review rounds 15-16 (Claude Opus 5.5)

Same STD-001 deviation as round 14 (Opus in place of the usual cross-model reviewer; a Codex round is still owed). The change swaps the unsigned iPXE we built for the iPXE project’s signed Secure Boot pair.

Round 15 (main...160688e): NO SIGN-OFF, four Mediums, each verified before accepting.

# Sev Finding Verdict
1 Med The shim’s stated source was wrong: it is iPXE’s fork (ipxe/shim tag ipxe-16.1, commit d0367b25), not rhboot/shim 16.1 (the commit I cited was the annotated tag object) Confirmed (the binary’s SBAT names ipxe/shim). Pins, notices and generated text corrected
2 Med The shim we shipped (from the v2.0.0 release image) carried only Microsoft’s 2011 CA signature; a dual-signed (2011 + 2023) copy of the same build exists in the ipxe/shim release Confirmed (Microsoft UEFI CA 2023 appears in one file and not the other; the earlier download from boot.ipxe.org’s root was that file). Now pinned from that release; re-run in the VM
3 Med Shim notices incomplete: it statically contains OpenSSL 1.0.2k, EDK2 crypto and gnu-efi code Confirmed. firmware/ipxe/signed/shim-NOTICES.txt carries the shim COPYRIGHT, OpenSSL 1.0.2k LICENSE and EDK2 License.txt; reproduced in every package’s ipxe-SOURCE.txt; the OpenSSL advertising acknowledgement is in THIRD_PARTY_NOTICES.md
4 Med Headlines said “a target with Secure Boot on accepts it” on VM-only evidence Confirmed. Everything now says “firmware that trusts Microsoft’s third-party UEFI CA, verified in a virtual machine only” (some locked-down PCs turn that CA off)
5-8 Low A stale hardware hash in the README row; stale build-ipxe.sh header; the licence statement cited a tool run on a different build; the exit 1 fall-through only seen in OVMF (shim may try MokManager on some error codes) 5-7 fixed. 8 stays open: the fall-through is VM-observed only; a real-PC check is on the test plan

PR #14 (the source-archive workflow’s relative-path bug found at the 0.4.0 release) was a prerequisite and was merged first.

Round 16 (160688e..4fcd07e): the pins, dual signature, notice texts (byte-compared with upstream), the generated ipxe-SOURCE.txt, image determinism, source hash and checksums all verified; the remaining blocker was the gnu-efi notice (Intel’s BSD-style licence requires reproducing it in binary distributions) plus three Low wording items. All fixed (the text is README.efilib at the shim’s pinned gnu-efi submodule commit dc7fd96f, which I confirmed against the shim tag). Verified in the VM with the exact final image: Secure Boot on (key press and no key), Secure Boot off, and the private Windows PE chain with Secure Boot on. Not run: this image on the real adapter, any real PC.

2026-10-06 - 0.4.1-0.4.5 video honesty, button contract, recording, themes (dev/0.4.x, PR #35): review round 17 (Codex gpt-6-astra)

The usual cross-model reviewer, read-only, high effort, over feat/secure-boot...dev/0.4.x (desktop only). Seven findings, each marked CONFIRMED by the reviewer and re-checked here against the source before accepting. Not run by the reviewer: Rust tests, Windows hardware.

# Sev Finding Verdict
1 High A failed recording’s cleanup deleted the final filename, which another instance (same second) could own Confirmed (cleanup was remove_file(final)). ffmpeg now writes .<pid>-<name>.part; cleanup removes only that; the finished file is published by hard link (never replaces; next free name on a clash; rename fallback on filesystems without links)
2 High Stopping a recording joined the encoder on the GUI thread (up to 120 s), including with input captured Confirmed. Stop now runs on a worker; the bar shows “Finishing the recording…”; quitting waits at most 20 s
3 Med A full queue dropped frames but the clock kept going: shorter, faster clip, and a wrong reported duration Confirmed. Lost slots are owed and repaid as repeats (bounded to 2 s); Finished.frames/duration count frames actually written
4 Med With the capture gone, nothing called push, so the duration cap and an encoder’s death went unnoticed Confirmed. Recorder::check runs when no frame arrives; a source gone for 10 s ends the clip with what exists
5 Med A stored device key that matched nothing fell back to the old /dev/videoN, which may now be a webcam Confirmed. The path is used only when no key was ever stored; regression test added
6 Med Audio lookup (pactl / ffmpeg) ran on the GUI thread Confirmed. Runs on a worker with a “looking for audio inputs…” state. Not done: ffmpeg -version at Start is still synchronous (a local binary; left, noted)
7 Med “No signal” (flat fill) blocked recording, but a black boot screen is valid video Confirmed (also a known gap from our own notes). Blank no longer blocks; the popup says it looks blank

Fixes verified: scripts/rs.sh test and clippy clean; the ffmpeg end-to-end tests pass against the host’s ffmpeg.

Round 18 (re-review of the round-17 fixes, same reviewer): NO SIGN-OFF, eight findings, each confirmed from the control flow and fixed:

# Sev Finding Fix
1 High Two recorders in one process/second/format chose the same final name, hence the same .part; one’s abort deleted the other’s file The final name is reserved atomically at start (create_new, an empty placeholder only this recording owns); the part file derives from it; cleanup removes only those two
2 High The rename fallback (no hard links) could replace another process’s recording Publishing is now a rename over our own reserved placeholder; the hard-link path is gone; a test starts and aborts two recorders in one second
3 High Quitting after a 20 s wait abandoned finalisation (orphan ffmpeg, stuck .part) A Canceller makes finish kill the encoder and remove the files; exit waits 20 s, cancels, waits 5 s more
4 Med Owed frames were discarded at stop or cap; repeated failed repayments inflated dropped finish repays what is owed (bounded 2 s) before closing; only newly lost slots count as dropped
5 Med The no-frame deadline did not cover a recording still awaiting its first frame Applies from the start; ends with NoFrames
6 Med Rescan could leave a stale index selecting a different audio device The list is read-only while a lookup runs, and the selection resets to “none” when a new list arrives
7 Med A hung pactl/ffmpeg lookup was never killed Lookups are killed after 5 s
8 Low An automatic stop kept showing the REC timer during encoding The worker flags finalising; the bar shows “Finishing the recording…”

Round 19 (re-review of the round-18 fixes): NO SIGN-OFF, five findings; four fixed, one accepted:

# Sev Finding Disposition
1 High A cancel requested before the recorder was registered was lost (a fresh flag replaced it) Fixed. The flag is made before the worker starts and handed to the recorder; a cancel can no longer be lost
2 Med The audio lookup’s 5 s did not bound the pipe readers if a descendant held them Fixed. Readers get 1 s after the tool is killed, then are abandoned. Not changed: the recorder’s own reader joins (ffmpeg spawns no descendants)
3 Med Owed-frame flush gave up after 2 s and ignored cancel Fixed. 10 s, cancel-aware. Residual debt after that is tolerated and shows in the dropped count
4 Med An empty file under the final name appeared immediately and survived a crash Fixed. The name is reserved by a hidden .<name>.reserved marker; nothing is visible under the final name until publication; a crash leaves only hidden files. A cancel now also wins over a clean exit
5 High (plausible) A user deleting the placeholder, then another program creating that name, got overwritten Fixed by the same change: cleanup never deletes the final name, and publication checks it is free (next free name otherwise; the check-then-rename gap against a non-cooperating writer remains, accepted)

Verification after the fixes: rs.sh test/clippy clean; ffmpeg end-to-end (4 tests) pass. No sign-off is claimed. Each round has found issues in the previous round’s fixes, with the severity of the remaining ones falling toward exotic races; a fourth round is the gate before release.

Round 20 (final gate on the round-19 fixes): one Medium, no other finding: publication was check-then-rename, which a program that is not us could race. Fixed: publication tries a hard link first (atomic, never replaces; a taken name moves on to the next free one) and uses check-then-rename only on filesystems without hard links. The reviewer’s verdict on round 20 was NO SIGN-OFF on that single finding; the fix has had no further review round, so no sign-off is claimed. The maintainer decides whether a fifth round is worth it before release.

2026-10-07 - macOS controller port (feat/macos, PR #38): review round 21 (Codex gpt-6-astra)

Static review (no Mac hardware exists for this project); every finding traced against the code and Apple’s API contracts before accepting. NO SIGN-OFF, eight findings, all fixed:

# Sev Finding Fix
1 High A 59.94/29.97 fps format rounded to 60/30 and 1/60 was set as the minimum frame duration: outside the range, AVFoundation throws (crash) The chosen format’s fastest AVFrameRateRange object is kept and its exact minFrameDuration applied; rounding only for scoring/display
2 High The first camera-permission prompt blocked the GUI up to 120 s; startRunning/stopRunning ran on the GUI thread Permission never waits (starts macOS’s prompt and says “answer it, then open again”); the session starts and stops on worker threads
3 High Event-tap start/stop could hang: a stop sent before the run loop ran was lost, and a slow start was joined without cancelling The tap thread runs its run loop in 0.1 s slices checking a shutdown flag; a failed or slow start sets it before joining
4 High A GUI stall let a modifier release pass to macOS unseen, then swallowing resumed with the key held on the target A stall or macOS disabling the tap now gives up for good: everything passes, and a release is queued so capture ends with release-all when the GUI resumes
5 Med Keys held before capture were not known (Ctrl held + Option+Esc did not release; a held key’s release could be swallowed) The tracker starts with the keys macOS’s HID state reports as down (CGEventSourceKeyState), as on Windows
6 Med The device was unlocked before the session started, so macOS could switch to its own format while the chip showed ours Apple’s macOS pattern: lock, set format and duration, startRunning, unlock (on the worker)
7 High (plausible) pair subscribed to TX, which is notify-only (not encrypted), so it could report success before bonding pair writes a harmless PING with response to RX (WRITE_ENC), which only succeeds once macOS has paired; 60 s bound
8 Med AVFoundation audio was selected by name: duplicate names collide and a name starting with digits is read as an index The index from ffmpeg’s listing is the id; the name is only for display

Verification after the fixes: scripts/rs.sh test, clippy, and scripts/rs.sh macos (clippy for aarch64-apple-darwin) clean. A re-review of the fixes is the next step.

Round 22 (re-review of the round-21 fixes): NO SIGN-OFF, three Medium, all confirmed and fixed:

# Sev Finding Fix
1 Med Start and stop workers were unordered: a capture dropped while starting could be stopped before its start ran, leaving it running The stop worker joins the start worker first
2 Med The 5 s stall clock ran during startup, and failed() latches, so a slow start discarded the capture for good The stall clock runs only once the start has finished; startup has its own 20 s limit
3 Med Nothing repainted the GUI when an async start finished, so the first frame or a start error could stay invisible while the app was idle The GUI polls every 100 ms while a capture is open with no frame yet (all backends)

Verification: rs.sh test, clippy, rs.sh macos clean.

Round 23 (re-review of the round-22 fixes): SIGN-OFF, no confirmed or plausible defect. Noted residual: the 20 s startup limit reports a native start that never returns, but cannot cancel it (that worker thread stays blocked; accepted). Static review only: no Mac hardware exists for this project, so the port ships in 0.4.6 as a labelled preview.

2026-10-07 - kvmit run --video, first multi-probe step (feat/run-video): review round 24 (Codex gpt-6-astra)

Static review of the diff; every finding traced against the source before accepting. NO SIGN-OFF, six findings, all confirmed and fixed:

# Sev Finding Fix
1 High Atomic saves stop torn files, not lost updates: the CLI and the GUI’s long-lived self.cfg wrote back whole stale snapshots over newer settings Config::update(f) applies the change to the file’s current contents and saves; every GUI and CLI save goes through it (unlocked: a same-instant race remains, documented)
2 High A remembered card that was missing fell back to the first card, which may be another target’s: its screen waits could pass on the wrong picture Remembered-but-missing is an error; the first card is used only when nothing is remembered
3 High The path picked by key could name another card if it was swapped between listing and opening After opening, the capture’s own key must equal the selected one, else the run stops
4 High A key without bus info is the bare card name, so two identical cards shared it and the first silently won select_video refuses a key that matches more than one card and asks for the path
5 Med Only top-level waits opened a card; waits inside repeat never got one (existing bug that defeats --video) exec::needs_video walks repeat bodies
6 Low The concurrency test could pass without ever reading during a write The test keeps reading while any writer runs, requires every read to parse, and checks no temp files remain; a new test covers keeping another process’s setting

Round 25 (re-review of the round-24 fixes): NO SIGN-OFF, five confirmed, all fixed. One of our own changes was also corrected: a rename retry added for Windows readers rested on a wrong claim (Rust opens files with delete sharing by default), so it was reverted.

# Sev Finding Fix
1 High A remembered key shared by two cards (no bus info) picked the first, bypassing the ambiguity check The CLI no longer reuses the GUI’s initial_video; video_state::run_video requires a remembered key to match exactly one card
2 High A config with only an old /dev/videoN path could select whatever card now has that number, and the post-open check passed A path-only config counts as nothing remembered: only a lone connected card is used, else --video is required
3 High With KVMIT_DEMO_VIDEO set, a missing remembered card fell back to the demo picture, so stable-screen waits passed on a still image The demo source is used only when named with --video
4 Med A --dry-run failed when the card was missing, although dry runs skip waits No card is opened for a dry run
5 Low The concurrency test could perform no read at all if the reader was descheduled Writers keep saving until the reader has done 500 reads

Verification: rs.sh test (201 passed), clippy clean.

Round 26 (re-review of the round-25 fixes): NO SIGN-OFF, three confirmed, all fixed; every round-25 fix verified.

# Sev Finding Fix
1 High A key with no bus info is just the card name, so a remembered card that is gone “matched” an identical card on another target run_video does not auto-select a key without a location (key == name); a swap between two such cards named by path stays undetectable, documented in the CHANGELOG
2 Med The concurrency test hung instead of failing: a panicking read skipped the stop flag and the scope joined writers forever; its comment overclaimed overlap A drop guard stops the writers; comment corrected
3 Low --help and getting-started still described “else the first” Both state the exact rule

Round 27 (re-review of the round-26 fixes): NO SIGN-OFF, one High, confirmed and fixed: device_key trims the card name but the listed name keeps its padding, so " Cam " with no bus gave key "Cam" != name and slipped past the bare-name refusal. run_video now compares with the trimmed name; regression test added. Round-26 fixes verified.

Round 28 (re-review of the round-27 fix): fix verified; one High raised, declined as a code change, fixed in the docs. Claim: the lone-card fallback (nothing remembered) accepts a key with no location. The location check exists to tell a remembered card from an identical one; with nothing remembered and one card connected there is nothing to tell it from, and a location would not help. The real risk underneath (with several targets, the lone card may be another target’s) applies to located keys too and is inherent to any no-flag default, so the docs and CHANGELOG now say to always pass --video with more than one target, and getting-started no longer overclaims that a locationless card always stops the run.

Round 29: round-28 reasoning upheld (“no code change is warranted”); one Low docs finding (the CHANGELOG listed “a key with no location” as stopping the run unconditionally; it is only a remembered one), fixed.

Round 30 (confirmation pass): SIGN-OFF, no remaining source-confirmed defect. Verification: rs.sh test (201 passed), clippy, rs.sh macos clean; host-tested only, no run against two physical adapters yet.

2026-10-08 - 0.4.8 polish (release/0.4.8): review round 31 (Codex gpt-6-astra)

The usual cross-model reviewer, read-only, high effort, over the uncommitted 0.4.8 diff on d5400d9 (Windows icon and version resource, plain-language Bluetooth errors, the generated licence listing, the firmware notices, the roadmap).

Round 31: NO SIGN-OFF, seven findings, each re-checked against the source here.

# Sev Finding Verdict and fix
1 High THIRD_PARTY_LICENSES.html was not in the tree, yet every package copies it Confirmed (generation was still queued). Generated with scripts/rs.sh licenses and committed
2 High Linux connect() powered BlueZ’s default adapter, which need not be the one btleplug then uses: with a blocked internal hci0 and a working USB hci1, connecting would fail where it used to work Confirmed. The up-front check is gone; a failed find is explained instead. Found alongside: the rfkill diagnosis blamed any blocked radio, so with a working second radio it misattributed failures; it now applies only when every Bluetooth radio is blocked (test added)
3 High The firmware notices named TinyUSB, FreeRTOS etc. but did not reproduce their copyright and licence texts (MIT and Apache-2.0 require it; NimBLE has a NOTICE file) Confirmed. New scripts/fw-licenses.py maps every archive in the app and bootloader linker maps to its licence files in the pinned ESP-IDF and managed components and writes THIRD_PARTY_FIRMWARE_LICENSES.txt (shipped in every package); an archive with no known source is an error, and the CI firmware job regenerates and compares it
4 Med The listing’s freshness hash covered only Cargo.lock and the cargo-about config Confirmed. It now also covers every manifest, desktop/vendor and the pinned cargo-about version
5 Med A listing truncated after its stamp line passed the check Confirmed (reproduced). The check (and --stamp) now requires the listing to name at least half the locked crates, including the app’s direct dependencies
6 Med The version-resource check searched for byte strings anywhere in the exe: clearing the resource directory still passed Confirmed (reproduced). verify-release.py now walks the PE resource directory, requires icon and group-icon resources, and checks VS_FIXEDFILEINFO and the ProductVersion, FileVersion, OriginalFilename and ProductName strings; tested against the built exes, a wrong version and a cleared directory
7 Low The packages’ licence files were only checked by name (zip) or not at all (AppImage, dmg) Confirmed. The zip, AppImage and dmg checks compare each licence file with the repository’s and run the staleness check. Not fixed: the MSI’s contents are not inspected; it is built from the same staged folder as the zip

Round 32 (re-review of the round-31 fixes, d5400d9..83ac3c4): NO SIGN-OFF, six findings, each re-checked here. Round-31 findings 1, 2 resolved; 3-7 partly, as below.

# Sev Finding Verdict and fix
1 High The licence-listing hash sorted Path objects, which order case-insensitively on Windows, so the Windows release check would reject a correct listing Confirmed. Sorts the normalised relative strings
2 High Firmware coverage stopped at libraries: third-party notices inside ESP-IDF sources (TLSF’s BSD-3-Clause, FreeRTOS’s Amazon copyright, a BSD file in NimBLE) were missing; an unknown managed component fell back to ESP-IDF’s licence Confirmed. fw-licenses.py now traces every linked object to its source through both builds’ compile_commands.json (an untraceable object is an error) and reproduces every non-Espressif header verbatim, with the licence files above it; SPDX-only headers get the standard text (BSD-3-Clause, MIT; any other identifier is an error). Only components inside ESP-IDF fall back to its licence
3 Med A listing cut after the serialport entry still passed (363/477 names) Confirmed (reproduced). The stamp also records a hash of the content; any edit or truncation fails
4 Med The PE walk ignored NumberOfRvaAndSizes, the resource directory size, entry flags and section raw sizes, and took the PE32 path for any magic Confirmed. All bounds-checked; directory/leaf flags enforced; unknown magic rejected. All four mutations (and a bad magic) now fail; the built exes pass
5 Med Version strings were found by regex, so a zero root wLength, a zero wValueLength or a renamed StringFileInfo still passed Confirmed. VS_VERSIONINFO is parsed as nested records honouring every declared length; the three mutations now fail
6 Low The dmg check compared the staging folder, not the image Confirmed. The macOS job mounts the finished .dmg and compares from it

Also fixed: CI’s bundled-firmware check failed because the licenses subcommand had been added to scripts/fw.sh, a hashed firmware input; it is now scripts/fw-licenses.sh and fw.sh is unchanged.

Round 33 (re-review of the round-32 fixes, 83ac3c4..f838d7d): NO SIGN-OFF, five Mediums, each reproduced here. Round-32 findings 1, 3 and 6 resolved; the reviewer also confirmed all 441 linked objects trace to sources and the generator gives the committed file under both local and GitHub workspace paths.

# Sev Finding Fix
1 Med 13 Mbed TLS sources open with a description comment and only then the copyright comment; the extractor read the first comment only and, finding no copyright, classed them as Espressif header() reads every comment before the first line of code; Espressif-only needs a positive Espressif copyright line; a file with no header is listed as such
2 Med SPDX expressions were split naively: (MIT OR BSD-3-Clause) AND Apache-2.0 failed, ... WITH LLVM-exception passed silently (neither occurs today) Tokenised parsing; WITH is an explicit error
3 Med Data-directory reads were not bounded by SizeOfOptionalHeader Bounded
4 Med Named and numeric resource entries were conflated Entries are checked against the directory’s named/ID counts; named ones are skipped, as Windows looks types up by number
5 Med The string table’s key was ignored (a renamed table passed) and a valid two-language resource was rejected Strings are read from the table named by a VarFileInfo\Translation pair, any number of tables allowed

Round 34 (re-review of the round-33 fixes, f838d7d..f44adcc): NO SIGN-OFF, four Mediums, each reproduced. Round-33 fixes verified: per-file notices grew from 104 to 119 sources (the 13 Mbed TLS files and two headerless ones), none lost; the regenerated file matches.

# Sev Finding Fix
1 Med Named entries were skipped at every level, so a valid string-named icon group (MAINICON) was rejected Only the type level is numeric-only
2 Med Only the first matching string table was checked; a second advertised language could carry a wrong version Every table named by a Translation pair is checked
3 Med pe_info read the security directory without the directory-count and optional-header bounds, so a crafted header could fake a signature blob for --require-signed Bounded; a directory pointing outside the file fails
4 Med A header with Apache boilerplate skipped the WITH check Exceptions are rejected before the full-text shortcut (synthetic case; none in this build)

Round 35 (re-review of the round-34 fixes, f44adcc..2fb2568): SIGN-OFF, no confirmed defect within scope. The prompt from this round on stated the tool’s input domain (binaries our own CI builds and signs, plus licence generation). The reviewer verified all four round-34 fixes, ran 18 negative cases, confirmed both built executables pass, checked the signature-size detection against two real Authenticode-signed PEs (x64 and PE32), and regenerated the firmware licence file byte-for-byte. Rounds 32-34 drifted towards hand-mutated inputs the release check never sees; that observation is recorded in corporate-strategy (state/products/kvm-it.md) as an STD-001 intake candidate.