Keep isort runtime dependencies bundled during startup

[bschnurr]

Summary

• keep bundled runtime dependencies on the bundled sys.path in both the server and runner
• align server and runner path ordering so imports resolve consistently
• add regression coverage for the exact bundled path order
• add the requested reviewCritic agent file

Validation

• python -m pytest src/test/python_tests/test_sys_path_bootstrap.py
• python -m py_compile bundled/tool/lsp_server.py bundled/tool/lsp_runner.py

Fixes #650

[rchiodo]

Copilot generated:
Use full equality instead of a slice assertion. Since monkeypatch sets sys.path to exactly one element and configure_bundled_sys_path inserts exactly two, the result should be exactly three entries — assert that directly:

assert sys.path == [str(libs_dir), str(tool_dir), "env-site-packages"]

The [:3] slice would silently pass if an unexpected extra entry appeared (e.g., from a future fixture change or import side-effect).

[verified]

[rchiodo]

Copilot generated:
-24

This test proves the happy path but doesn't explicitly verify the regression scenario that #650 describes. Consider adding a second test case that sets monkeypatch.setenv("LS_IMPORT_STRATEGY", "fromEnvironment") and asserts bundled paths are still prepended. This documents the fix's intent: the env var no longer influences bootstrap ordering.

Additionally, consider testing the runner's configure_bundled_sys_path independently (even if the implementation is identical) to catch future drift between the two copies.

[verified]

[rchiodo]

Copilot generated:
-30

This changes the server's effective sys.path ordering from [tool, libs, ...] to [libs, tool, ...] (because the second insert(0, ...) pushes the first entry to index 1). This aligns with the runner's ordering — which is the stated goal — but is a behavioral change worth documenting. If any module name ever exists in both tool/ and libs/, resolution priority has flipped. Consider adding a brief comment noting this is intentional alignment with the runner.

[verified]

[rchiodo]

Copilot generated:
Unresolved prior feedback: Use full equality instead of a slice assertion. Since monkeypatch sets sys.path to exactly one element and configure_bundled_sys_path inserts exactly two, the result is deterministically three entries:

assert sys.path == [str(libs_dir), str(tool_dir), "env-site-packages"]

The [:3] slice silently passes if an unexpected extra entry appears from an import side-effect or future fixture change.

[verified]

[rchiodo]

Copilot generated:
-24

Unresolved prior feedback: Add a regression test for the fromEnvironment scenario that #650 describes. This documents the fix's intent and guards against future regressions:

def test_configure_bundled_sys_path_ignores_import_strategy(monkeypatch, tmp_path):
    bundle_dir = tmp_path / "bundled"
    tool_dir = bundle_dir / "tool"
    libs_dir = bundle_dir / "libs"
    tool_dir.mkdir(parents=True)
    libs_dir.mkdir(parents=True)

    monkeypatch.setattr(sys, "path", ["env-site-packages"])
    monkeypatch.setenv("LS_IMPORT_STRATEGY", "fromEnvironment")

    lsp_server.configure_bundled_sys_path(bundle_dir)

    assert sys.path == [str(libs_dir), str(tool_dir), "env-site-packages"]

Per the weighted review criteria, "test coverage proving the fix" is high-weight for bug-fix PRs. Also consider testing lsp_runner.configure_bundled_sys_path independently to catch future drift between the two copies.

[verified]

[rchiodo]

Copilot generated:
-30

Unresolved prior feedback: Add a brief comment noting the intentional ordering alignment with the runner. The successive insert(0, ...) calls produce [libs, tool, ...] on sys.path — a behavioral change from the previous [tool, libs, ...] order. Suggested:

def configure_bundled_sys_path(bundle_dir: pathlib.Path) -> None:
    """Ensure the server's runtime dependencies always come from the bundle."""
    # Insertion order matches lsp_runner.py; libs ends up at index 0.
    update_sys_path(os.fspath(bundle_dir / "tool"), "useBundled")
    update_sys_path(os.fspath(bundle_dir / "libs"), "useBundled")

[verified]