diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 8bdd8baa9..44dd3e8bf 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -120,4 +120,4 @@ Below are some specific details of individual PR contributions. * Jeffrey Phillips Freeman has contributed the `--format`/`-f` flag to `agents session tell` (issue #10466): adds JSON envelope output for machine-readable workflows alongside existing Rich console output, with Behave BDD test coverage verifying all four non-rich format paths (JSON, YAML, plain, table) and the short `-f` flag alias. * HAL 9000 has contributed the Semgrep guard for broad exception suppression (PR #9185 / issue #9103): added two new Semgrep rules (`python-no-suppressed-exception` and `python-no-suppress-exception`) to automate enforcement of error propagation guidelines, integrated Semgrep into `nox -s lint` in audit mode with migration plan for ~337 existing violations, and comprehensive BDD test coverage across all rule patterns and escape hatch scenarios. * HAL 9000 has contributed the `ProviderRegistry.FALLBACK_ORDER` fix (#10906): added the missing `ProviderType.GEMINI` to the fallback provider order list so that when only a Gemini API key is configured, the registry correctly selects it as the default provider. Includes BDD regression scenarios in `features/fallback_gemini_provider.feature`. -* HAL 9000 has contributed the StdioTransport subprocess cleanup on failed initialization fix (PR #11160 / issue #11160): after ``start()`` spawns a process with ``subprocess.Popen()``, the method now verifies the process is still alive via ``poll()``. If it died immediately during initialization, ``stop()`` is called to release the handle and an ``LspError`` with exit code is raised, preventing zombie processes and resource leaks. Includes BDD test coverage in ``features/lsp_transport_subprocess_cleanup.feature``. +* HAL 9000 has contributed the StdioTransport subprocess cleanup on failed initialization fix (PR #11185 / issue #11160): after ``start()`` spawns a process with ``subprocess.Popen()``, the method now verifies the process is still alive via ``poll()``. If it died immediately during initialization, ``stop()`` is called to release the handle and an ``LspError`` with exit code is raised, preventing zombie processes and resource leaks. Includes BDD test coverage in ``features/lsp_transport_subprocess_cleanup.feature``. diff --git a/features/lsp_transport_subprocess_cleanup.feature b/features/lsp_transport_subprocess_cleanup.feature index 869102381..32cdb82fa 100644 --- a/features/lsp_transport_subprocess_cleanup.feature +++ b/features/lsp_transport_subprocess_cleanup.feature @@ -1,3 +1,4 @@ +@tdd_issue @tdd_issue_11160 Feature: StdioTransport subprocess cleanup on failed initialization As a developer maintaining the LSP transport layer I need proper cleanup of subprocess handles when the process dies during start() @@ -27,14 +28,14 @@ Feature: StdioTransport subprocess cleanup on failed initialization When ltcov I try to start the transport Then ltcov the error should be an LspError with message "died on spawn" - Scenario: start raises LspError when process returnscode is zero (clean exit) + Scenario: start raises LspError when process returncode is zero (clean exit) Given ltcov I create a StdioTransport for command "quick_exit" And ltcov Popen is mocked to return a process that exited with code 0 When ltcov I try to start the transport Then ltcov the error should be an LspError with message "died on spawn" Scenario: stop() handles already-exited process and cleans up _process - Given ltcov I create a StdioTransport for command "dye_on_start" + Given ltcov I create a StdioTransport for command "die_on_start" And ltcov Popen is mocked to return a process that exited with code 42 When ltcov I try to start the transport Then ltcov the stop handles already-exited processes correctly diff --git a/features/steps/lsp_transport_coverage_steps.py b/features/steps/lsp_transport_coverage_steps.py index 2b2c21cdc..5bb7fa672 100644 --- a/features/steps/lsp_transport_coverage_steps.py +++ b/features/steps/lsp_transport_coverage_steps.py @@ -543,11 +543,6 @@ def step_ltcov_process_none_after_failed_start(context: Context) -> None: # --------------------------------------------------------------------------- -@then("ltcov the transport should be alive") -def step_ltcov_alive(context: Context) -> None: - assert context.ltcov_transport.is_alive, "Expected transport to be alive" - - @then("ltcov no error should have been raised") def step_ltcov_no_error(context: Context) -> None: assert context.ltcov_error is None, ( diff --git a/src/cleveragents/lsp/transport.py b/src/cleveragents/lsp/transport.py index 31c17bbc1..c34219757 100644 --- a/src/cleveragents/lsp/transport.py +++ b/src/cleveragents/lsp/transport.py @@ -107,8 +107,6 @@ class StdioTransport: if self._process is not None and self.is_alive: raise RuntimeError("Transport already started") - from cleveragents.lsp.errors import LspError - merged_env = {**os.environ, **self._env} cmd = [self._command, *self._args]