Skip to content

Comments

Bxyu/profiling -> sdd/profiling#6

Open
bxyu-nvidia wants to merge 43 commits intosdd/profilingfrom
bxyu/profiling
Open

Bxyu/profiling -> sdd/profiling#6
bxyu-nvidia wants to merge 43 commits intosdd/profilingfrom
bxyu/profiling

Conversation

@bxyu-nvidia
Copy link
Collaborator

Summary of PR

Change Type

  • Bug fix
  • New feature
  • Breaking change
  • Refactor
  • Other (dependency update, docs, typo fixes, etc.)

Checklist

  • I have read and reviewed the code and I understand what the code is doing.
  • I have tested the code to the best of my ability and ensured it works as expected.

Fixes

Resolves #(issue)

Release Notes

  • Include this change in the Release Notes.

@sdevare-nv sdevare-nv force-pushed the sdd/profiling branch 2 times, most recently from 25cbc7d to d6bd12e Compare February 3, 2026 21:17
puririshi98 added a commit to puririshi98/nv-OpenHands that referenced this pull request Feb 19, 2026
This commit fixes all identified functionality bugs and adds comprehensive
test coverage to prevent regressions.

## Bugs Fixed:

**Fix #1: Exit code extraction no longer contaminates output buffer**
- Modified execute_command to save buffer before extracting exit code
- Created async _extract_exit_code_async method that doesn't pollute output
- Exit code extraction now uses separate buffer that gets discarded

**Fix sdevare-nv#2: Race conditions prevented with proper locking**
- Added async lock to execute_command method
- Prevents concurrent command execution on same session
- Ensures output from different commands doesn't get mixed

**Fix sdevare-nv#3: NameError in create_session error handling fixed**
- Initialize master_fd and process variables before try block
- Prevents NameError if exception occurs before PTY creation
- Also added process cleanup in error handler

**Fix sdevare-nv#4: Robust prompt detection with custom marker**
- Replaced brittle regex patterns ([$#>]$) with unique marker
- Set custom PS1 with PROMPT_MARKER after shell initialization
- Commands outputting $, #, or > no longer cause false timeouts
- Added _clean_output method to remove markers from user-visible output

**Fix sdevare-nv#5: Process state validation added**
- Check if shell process is still alive before executing commands
- Prevents hangs when process has crashed or been killed externally
- Added same check to send_input method

**Fix sdevare-nv#6: Buffer clearing timing issues resolved**
- Read and discard stale output before clearing buffers
- Prevents output from previous commands contaminating current command
- Fixed rapid successive command execution

**Fix sdevare-nv#7: Thread-safe singleton pattern implemented**
- Added async lock for get_session_manager_async
- Created separate sync version for backward compatibility
- Prevents race conditions in manager initialization

**Fix sdevare-nv#8: Complete resource cleanup on errors**
- Kill and wait for process in error handler
- Close file descriptors even if process creation fails
- No more zombie processes or fd leaks

**Fix sdevare-nv#9: Blocking sleep replaced with async sleep**
- Changed time.sleep to await asyncio.sleep in _extract_exit_code_async
- Prevents blocking the event loop
- Improved async performance

## Additional Improvements:

- Enhanced ANSI escape code handling in exit code extraction
- Added support for CSI sequences with ? character
- Improved regex to handle terminal control sequences
- Better exit code parsing for all edge cases

## Test Coverage:

Added comprehensive test suite (test_terminus_bugfixes.py) with 12 tests:
- 7 tests for specific bug fixes
- 5 tests for previously uncovered scenarios:
  * Concurrent command execution
  * Rapid successive commands
  * Environment persistence
  * Exit code accuracy
  * Multi-line output handling

All 12 tests pass successfully.

## Tested Scenarios:

✅ Exit code extraction doesn't contaminate subsequent commands
✅ Concurrent commands are properly serialized
✅ Error handling doesn't raise NameError
✅ Commands with prompt-like output ($, #, >) don't timeout
✅ Dead process detection works correctly
✅ Rapid successive commands don't mix output
✅ Resource cleanup is complete even on errors
✅ Environment variables persist across commands
✅ Exit codes (0, 1, custom) are captured accurately
✅ Multi-line command output is preserved

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant