fix(tui): add 2s timeout to all clipboard subprocess calls#334
Conversation
…r64#294) Replace Popen+communicate (no timeout) with subprocess.run(timeout=2.0) across macOS (pbcopy), Linux (xclip/xsel/wl-copy) and Windows (clip). * On TimeoutExpired the exception is caught at the specific type instead of bare Exception, so the OSC 52 fallback always runs. * The Linux loop narrows its per-tool handler to (FileNotFoundError, CalledProcessError, TimeoutExpired) so a hung tool is skipped and the loop moves to the next candidate. * Outer handler narrows from bare Exception to (CalledProcessError, TimeoutExpired, OSError). * Docstring documents the 2-second bound and fallback behaviour. * test_tui_copy_to_clipboard updated to use subprocess.run mock. * test_tui_copy_to_clipboard_timeout added: monkeypatches subprocess.run to raise TimeoutExpired and asserts OSC 52 fallback fires without hang. Closes bitflicker64#294
|
Tip 👋 Hey @nayanraj864-cmyk — Miku's on it. Here are the most useful commands for this PR:
Note 🎓 This repo is part of ECSOC26. Run Note ⭐ Star this repo to unlock all commands. Say 📖 All commands🤖 AI-powered — 14 commands
🔧 Issue & PR management — 18 commands
🎉 Community & utility — 9 commands
|
|
| Filename | Overview |
|---|---|
| termstory/tui.py | Replaces Popen+communicate with subprocess.run(timeout=2.0) across macOS, Linux, and Windows clipboard paths; the Linux inner exception tuple is narrower than OSError which can silently abort the tool fallback chain on permission-style errors |
| termstory/insights.py | Removes a duplicate, buggy step 4+5 block (which would have caused a KeyError on project_id after deletion) and keeps only the correct implementation with seen_commits deduplication; also removes the now-unused project_ids_needing_commits variable |
| tests/test_tui.py | Updates existing clipboard test to mock subprocess.run instead of Popen and adds a new timeout test; the updated test no longer asserts the correct text was passed as input to the subprocess |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[copy_to_clipboard called] --> B[strip_ansi from text]
B --> C{sys.platform?}
C -- darwin --> D[subprocess.run pbcopy timeout=2.0]
D -- success --> G
D -- exception --> H[log debug]
H --> G
C -- linux --> E[Loop: xclip then xsel then wl-copy]
E --> E1[subprocess.run cmd timeout=2.0]
E1 -- success --> G
E1 -- FileNotFoundError or CalledProcessError or TimeoutExpired --> E2{more tools?}
E2 -- yes --> E1
E2 -- no --> G
E1 -- OSError non-FileNotFoundError --> H
C -- win32 --> F[subprocess.run clip timeout=2.0]
F -- success --> G
F -- exception --> H
G[super copy_to_clipboard OSC 52 fallback always runs]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[copy_to_clipboard called] --> B[strip_ansi from text]
B --> C{sys.platform?}
C -- darwin --> D[subprocess.run pbcopy timeout=2.0]
D -- success --> G
D -- exception --> H[log debug]
H --> G
C -- linux --> E[Loop: xclip then xsel then wl-copy]
E --> E1[subprocess.run cmd timeout=2.0]
E1 -- success --> G
E1 -- FileNotFoundError or CalledProcessError or TimeoutExpired --> E2{more tools?}
E2 -- yes --> E1
E2 -- no --> G
E1 -- OSError non-FileNotFoundError --> H
C -- win32 --> F[subprocess.run clip timeout=2.0]
F -- success --> G
F -- exception --> H
G[super copy_to_clipboard OSC 52 fallback always runs]
Reviews (2): Last reviewed commit: "fix(insights): remove duplicate Step 4+5..." | Re-trigger Greptile
…_commits variable The main branch had a bad merge state from PR bitflicker64#333: detect_late_night_chaotic_sessions contained two complete copies of the bulk-commit-fetch (Step 4) and the session association loop (Step 5). The first Step 5 deleted project_id from each session dict, then the second Step 4 tried to access it → KeyError: 'project_id'. Fix: collapse to a single clean Step 4 (with seen_commits dedup) and single Step 5. Also removes the dead project_ids_needing_commits set that was never read.
|
i solve this issue review and merge it @bitflicker64 |
|
:miku /ecsoc |
|
miku 🏷️ Added |
|
/rewrite |
|
miku ✅ PR description rewritten! Check the PR body above. PreviewSummaryFixes TUI freeze caused by clipboard daemon hangs (issue #294) by adding 2-second timeouts to all subprocess clipboard operations across macOS, Linux, and Windows. Also removes duplicate commit-fetching logic in ChangesCore
Insights
Tests
Architecture / Flowflowchart TD
A[copy_to_clipboard called] --> B{Platform?}
B -->|darwin| C[subprocess.run pbcopy timeout=2.0]
B -->|linux| D[Loop: xclip, xsel, wl-copy]
D --> E[subprocess.run cmd timeout=2.0]
E -->|success| F[Break]
E -->|TimeoutExpired/FileNotFoundError/CalledProcessError| G[Continue to next tool]
B -->|win32| H[subprocess.run clip timeout=2.0]
C --> I[OSC 52 fallback]
F --> I
G --> I
H --> I
I --> J[super().copy_to_clipboard]
J --> K[Done]
Fixes
TestingRun the clipboard tests to verify timeout handling and fallback behavior: pytest tests/test_tui.py::test_tui_copy_to_clipboard -v
pytest tests/test_tui.py::test_tui_copy_to_clipboard_timeout -vNotesThe OSC 52 fallback ( |
|
/lgtm /approve /triage |
|
miku ✅ LGTM! Label added by @bitflicker64. Use miku ✅ Approved! 🎉 miku ✅ Marked as triaged ( |
|
/rewrite |
|
miku ✅ PR description rewritten! Check the PR body above. PreviewSummaryFixes TUI freeze caused by clipboard daemon hangs (issue #294) by replacing unbounded ChangesCore
Insights
Tests
Architecture / Flowflowchart TD
A[copy_to_clipboard called] --> B{{Platform?}}
B -->|darwin| C[subprocess.run pbcopy timeout=2.0]
B -->|linux| D[Loop: xclip, xsel, wl-copy]
D --> E[subprocess.run cmd timeout=2.0]
E -->|success| F[Break]
E -->|TimeoutExpired/FileNotFoundError/CalledProcessError| G[Continue to next tool]
B -->|win32| H[subprocess.run clip timeout=2.0]
C --> I[OSC 52 fallback]
F --> I
G --> I
H --> I
I --> J[super().copy_to_clipboard]
J --> K[Done]
Fixes
TestingRun the clipboard tests to verify timeout handling and fallback behavior: pytest tests/test_tui.py::test_tui_copy_to_clipboard -v
pytest tests/test_tui.py::test_tui_copy_to_clipboard_timeout -vNotesThe OSC 52 fallback ( |
|
/LABEL good-pr |
|
miku 🏷️ Added label |
Summary
Fixes TUI freeze caused by clipboard daemon hangs (issue #294) by replacing unbounded
subprocess.Popencalls withsubprocess.run(timeout=2.0)across all platforms. Also removes duplicate commit-fetching logic indetect_late_night_chaotic_sessionsthat would have caused aKeyErroronproject_idand adds deduplication for chunked queries.Changes
Core
termstory/tui.py: Replaced allsubprocess.Popen+communicate()calls withsubprocess.run(..., timeout=2.0, check=True)incopy_to_clipboardmethodtermstory/tui.py: Changed exception handling from bareExceptionto specific types (subprocess.CalledProcessError,subprocess.TimeoutExpired,OSError)termstory/tui.py: Linux clipboard loop now skips hung tools and tries the next one instead of blockingtermstory/tui.py: Updated docstring to document the 2-second timeout bound and fallback behaviorInsights
termstory/insights.py: Removed duplicate commit-fetching code block that would have causedKeyErroronproject_idtermstory/insights.py: Addedseen_commitsset to deduplicate rows appearing in multiple chunk queries when >250 chaotic sessions share the same projecttermstory/insights.py: Fixed indentation in commentTests
tests/test_tui.py: Updatedtest_tui_copy_to_clipboardto mocksubprocess.runinstead ofPopento match new APItests/test_tui.py: Addedtest_tui_copy_to_clipboard_timeoutto verifyTimeoutExpiredis handled and OSC 52 fallback fires without hangingArchitecture / Flow
flowchart TD A["copy_to_clipboard called"] --> B{"Platform?"} B -->|darwin| C["subprocess.run pbcopy timeout=2.0"] B -->|Linux| D["Loop: xclip, xsel, wl-copy"] D --> E["subprocess.run cmd timeout=2.0"] E -->|Success| F["Break"] E -->|TimeoutExpired / FileNotFoundError / CalledProcessError| G["Continue to next tool"] B -->|win32| H["subprocess.run clip timeout=2.0"] C --> I["OSC 52 fallback"] F --> I G --> I H --> I I --> J["super().copy_to_clipboard"] J --> K["Done"]Fixes
Testing
Run the clipboard tests to verify timeout handling and fallback behavior:
Notes
The OSC 52 fallback (
super().copy_to_clipboard) always runs regardless of subprocess timeout or failure, ensuring clipboard operations work over SSH terminals where system clipboard daemons may be unavailable or hung. The 2-second timeout prevents indefinite UI thread blocking while still allowing reasonable time for clipboard daemons to respond.