fix(lsp): address reviewer blockers — remove duplicate step, move LspError import, fix metadata
CI / lint (pull_request) Successful in 53s
CI / typecheck (pull_request) Successful in 1m2s
CI / security (pull_request) Successful in 1m5s
CI / quality (pull_request) Successful in 1m1s
CI / build (pull_request) Successful in 50s
CI / helm (pull_request) Successful in 51s
CI / push-validation (pull_request) Successful in 35s
CI / unit_tests (pull_request) Successful in 5m11s
CI / docker (pull_request) Successful in 1m37s
CI / integration_tests (pull_request) Successful in 8m57s
CI / coverage (pull_request) Failing after 19m24s
CI / status-check (pull_request) Has been cancelled

- Remove duplicate @then("ltcov the transport should be alive") in
  features/steps/lsp_transport_coverage_steps.py; the same step is
  already defined at features/steps/lsp_transport_post_spawn_cleanup_steps.py:92
  for issue #7044. The duplicate caused AmbiguousStep at behave load
  time, erroring every BDD feature in the suite (root cause of the
  CI unit_tests + coverage gate failures).
- Remove redundant inline `from cleveragents.lsp.errors import LspError`
  inside StdioTransport.start(); the module-level import at line 30
  is sufficient and inline imports violate the project import rule.
- Update CONTRIBUTORS.md: correct the PR number from #11160 (the
  linked issue) to #11185 (this PR).
- features/lsp_transport_subprocess_cleanup.feature: add the required
  `@tdd_issue @tdd_issue_11160` tags per the TDD bug-fix workflow,
  fix the `returnscode` typo in a scenario name, and rename the
  `dye_on_start` command label to `die_on_start`.

ISSUES CLOSED: #11160
This commit is contained in:
2026-06-15 00:39:43 -04:00
committed by drew
parent 007fd66b90
commit 10e8167ef7
4 changed files with 4 additions and 10 deletions
+1 -1
View File
@@ -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. * 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 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 `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``.
@@ -1,3 +1,4 @@
@tdd_issue @tdd_issue_11160
Feature: StdioTransport subprocess cleanup on failed initialization Feature: StdioTransport subprocess cleanup on failed initialization
As a developer maintaining the LSP transport layer As a developer maintaining the LSP transport layer
I need proper cleanup of subprocess handles when the process dies during start() 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 When ltcov I try to start the transport
Then ltcov the error should be an LspError with message "died on spawn" 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" Given ltcov I create a StdioTransport for command "quick_exit"
And ltcov Popen is mocked to return a process that exited with code 0 And ltcov Popen is mocked to return a process that exited with code 0
When ltcov I try to start the transport When ltcov I try to start the transport
Then ltcov the error should be an LspError with message "died on spawn" Then ltcov the error should be an LspError with message "died on spawn"
Scenario: stop() handles already-exited process and cleans up _process 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 And ltcov Popen is mocked to return a process that exited with code 42
When ltcov I try to start the transport When ltcov I try to start the transport
Then ltcov the stop handles already-exited processes correctly Then ltcov the stop handles already-exited processes correctly
@@ -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") @then("ltcov no error should have been raised")
def step_ltcov_no_error(context: Context) -> None: def step_ltcov_no_error(context: Context) -> None:
assert context.ltcov_error is None, ( assert context.ltcov_error is None, (
-2
View File
@@ -107,8 +107,6 @@ class StdioTransport:
if self._process is not None and self.is_alive: if self._process is not None and self.is_alive:
raise RuntimeError("Transport already started") raise RuntimeError("Transport already started")
from cleveragents.lsp.errors import LspError
merged_env = {**os.environ, **self._env} merged_env = {**os.environ, **self._env}
cmd = [self._command, *self._args] cmd = [self._command, *self._args]