Fix top 5 dangerous bugs (terminal raw-mode, credential overwrite, firmware cleanup, fd leak, substring check)

[davmonk]

Summary

Five independent, high-confidence bug fixes found while auditing the codebase. Each is a real, demonstrable bug — no refactoring or speculative cleanup. All 164 unit + 85 integration tests still pass.

1. countdown.py — terminal can be left in raw mode

After child_process.terminate(), the code immediately calls child_process.close(). Process.close() raises ValueError if the process hasn't fully exited yet (the read-loop child blocks in _getch(), so it relies on terminate to die). When that races, close() raises and the subsequent os_specifics.finalize() — which restores the TTY — never runs, leaving the user's terminal stuck.

Fix: join(timeout=1.0) the child first, wrap close() in a try/except ValueError with a kill() fallback, and ensure finalize() always runs.

2. retroaccount.py — invalid server response can overwrite valid credentials

_build_mister_sync_transition did new_user_json = response.get('tokens', None) and then conditionally added device_id only when the value was a non-empty dict — but unconditionally passed save_user_json=new_user_json into the transition. An empty dict or non-dict tokens payload would still get written over the valid user.json, and on the next sync the missing device_id/refresh_token would trigger credentials_were_corrupted and force the user to re-login.

Fix: only set new_user_json (and therefore save_user_json) when tokens is a non-empty dict.

3. pocket_firmware_update.py — stale firmware files left on the Pocket SD

The cleanup loop iterates pocket_firmware*.bin and removes old ones, but breaks as soon as it finds the latest. Any stale firmware files that come after the latest one in glob order are never removed.

Fix: continue instead of break so the rest of the directory is still cleaned.

4. arcade_organizer.py — os.scandir() not closed in a recursive function

_remove_broken_symlinks calls itself for every subdirectory while holding an open os.scandir() iterator (no context manager). Each recursive call leaks a file descriptor until GC. On deep trees this can exhaust the per-process FD limit.

Fix: wrap in with os.scandir(directory) as entries:.

5. timeline.py — substring check instead of equality

if formatted_category in ('system'): is missing the trailing comma, so ('system') is the string 'system', not a 1-tuple. Python evaluates this as a substring check ('sys' in 'system' is True) rather than equality. Currently masked because no other categories happen to be substrings of 'system', but the intent is clearly equality.

Fix: formatted_category == 'system'.

Test plan

• python3 -m unittest discover -s test/unit — 164 tests pass
• python3 -m unittest discover -s test/integration — 85 tests pass
• Smoke-test on real MiSTer hardware (countdown abort, sync with stale token response, Pocket firmware update with multiple stale .bin files)

🤖 Generated with Claude Code