command_runner: duplicated keyboard-context implementations, PID-recycling bug in _USER_KILLED_PROCESSES, unsynchronized globals, stray sleep(1) and dead time.time()

File: code_puppy/tools/command_runner.py

Severity: Medium (correctness / race conditions)

Several global-state races and leaks around the shell-execution machinery:

1. Duplicated keyboard-context implementations. _shell_command_keyboard_context() (line ~518) and the refcounted pair _acquire_keyboard_context() / _release_keyboard_context() (+ _start_keyboard_listener / _stop_keyboard_listener, lines ~590-700) are two parallel implementations of the same feature, including duplicated Ctrl-X/SIGINT handler bodies (handle_ctrl_x_press vs _handle_ctrl_x_press, shell_sigint_handler vs _shell_sigint_handler). Production code only uses the refcounted pair; the context manager appears to be kept alive solely for tests (tests/tools/test_command_runner_*.py). One obvious way to do it - delete the context-manager variant or reimplement it as contextlib.contextmanager over acquire/release.

2. _USER_KILLED_PROCESSES grows forever and recycles PIDs. (lines 111, 238, 981). PIDs are added on kill but never removed, so (a) unbounded growth in long sessions, and (b) since OS PIDs are recycled, a later, unrelated command that happens to reuse a killed PID and exits non-zero will be falsely reported user_interrupted=True, which instructs the model to halt and ask the user. Also mutated without a lock from signal/listener threads. Fix: guard with _RUNNING_PROCESSES_LOCK, and discard the PID when constructing the result (_USER_KILLED_PROCESSES.discard(process.pid) after reading it).

3. Global listener state has no lock for the legacy context manager - _SHELL_CTRL_X_STOP_EVENT/_SHELL_CTRL_X_THREAD/_ORIGINAL_SIGINT_HANDLER are plain module globals written by both implementations; nested/parallel use clobbers the saved SIGINT handler (the second entry records the replacement handler as "original" and restores that).

4. Unexplained time.sleep(1) on every non-zero exit (line ~970): every failing command costs an extra blocking second inside the worker thread, with no comment explaining why. Either delete it or document it.

5. Dead statement else: time.time() (line ~1200) - the else-branch of the confirmation gate evaluates time.time() and discards it.

Filed by Zen Reviewer B (code-puppy-60635a)

Start in code_puppy/tools/command_runner.py, especially the keyboard-context implementations around lines 518-700, process handling near lines 111, 238, and 981, and the cleanup statements near lines 970 and 1200. Review tests/tools/test_command_runner_*.py before consolidating or synchronizing the affected state. Done means the duplicate context behavior, PID tracking, listener globals, unexplained sleep, and dead time.time() statement are addressed without changing the intended shell-execution behavior.