[Deicyde]

A follow-up to #482: the servers half of sharing the process helpers between autoform_cli and servers/. In the CLI half, a separate PR once this one and the boundary test have landed, lean_probe loads servers/_process.py by file path through importlib, not with an import statement, so #499's third check allows it. It uses these helpers in place of its own, which differ in how they treat a process table or environment they cannot read, and adopts this module's fail-closed rule for environments. It is the caller the scan's new name parameter is for.

Change

• The token scan moves. _processes_with_token leaves servers/lsp/server.py for servers/_process.py as processes_with_token(name, token). The LSP daemon passes its own _SESSION_TOKEN_ENV. Next to the original, the moved function differs only in:

  • the name parameter, which replaces _SESSION_TOKEN_ENV;
  • the ValueError text, now "a process token must be a nonempty string";
  • the process-table error, now "cannot read the process table to find the processes carrying {name}";
  • two docstring phrases that said "session".

  The psutil.pids() handler still catches (OSError, psutil.Error), and an environment error other than "gone" or "denied" still fails the scan.

• Two shared names lose their underscore. _process_group_has_live_members and _inherit_clean_env are imported by other servers modules, so they become process_group_has_live_members and inherit_clean_env. Their bodies are unchanged. The callers in lsp/server.py, batch_diagnostics.py and repl/core.py use the new names, and their import lists stay sorted.

• The other five underscored names keep their underscore, to keep this PR to the two renames the CLI half needs: _kill_subprocesses, _reap_process, _terminate_process_group, _wait_for_live_process_group_exit and _wait_for_process.

• Docstring. _process.py's module docstring now says it also finds the processes carrying a token the caller set.

• Tests. tests/test_lsp_protocol.py and tests/test_repl_core_protocol.py patch and call the new names. No assertion is weakened, and the match= strings still hold. Two additions:

  • the abort retry test's fake scan records each (name, token) it is called with, and the test asserts after session.abort() that every call passed _SESSION_TOKEN_ENV and the session's token. Asserting inside the fake would not fail the test, because the abort loop catches the error and retries;
  • a new scan test with fake processes checks that processes_with_token reads the variable it is given: a process carrying the token under that name is returned, and one carrying it under _SESSION_TOKEN_ENV is not.

• No CHANGELOG entry: no command or flag changes, and the daemon code paths are unchanged apart from the move and the renames. _process.py and lsp/server.py are in the daemon's code fingerprint, so each install's daemon restarts once after the upgrade.

Net: +94/−68 across six files.

Verification

• The locked ruff 0.15.17 passes on autoform_cli servers tests, as CI runs it. The touched files compile.
• No file under servers/ or in the three daemon test files names _process_group_has_live_members, _inherit_clean_env or _processes_with_token. autoform_cli/lean_probe.py keeps its own group check until the CLI half.
• When called directly, the affected tests pass:
  • tests/test_lsp_protocol.py, every test whose name contains token or abort (the token scan, sweep and abort tests): 17 passed, 2 skipped that need real Lean;
  • tests/test_repl_core_protocol.py, the process-group cleanup tests and the header check that reaps descendants: 4 passed;
  • tests/test_batch_diagnostics.py, two runs that start a child with the clean environment: 2 passed.
• Two mutants fail the new assertions: a scan that reads _SESSION_TOKEN_ENV whatever name it is given fails the new scan test, and a daemon that passes another name fails the abort retry test at once instead of looping.
• CI coverage:
  • test (Python 3.10 and 3.13): the full unit suite, including the daemon tests above;
  • real-lean, LSP step: the real-Lean LSP tests, including the token sweeps that stop orphaned workers;
  • real-repl: starts the REPL through repl/core.py, which calls inherit_clean_env.

Overlap

No open PR touches servers/ code or these two test files. #489, which changed only servers/README.md under servers/, has merged and does not conflict.

[Deicyde]

Review at 3c8a837: merge.

This is a static review, the first on this PR, and leaves test results to CI, where all nine checks pass at this head; I found no blockers, and the move and renames match the body. The body's plan does not hold: the "CLI half", where autoform_cli/lean_probe.py imports these helpers, is what #499's test_autoform_cli_never_imports_servers rejects (#499 also bars servers from importing autoform_cli), so please describe this as an internal servers cleanup or name a home both rules allow, and reconsider the new name parameter if no other caller is coming. lean_probe's helpers are also not "copies": only _probe_environment matches inherit_clean_env, and its group check and token scan treat an unreadable process table or environment as live or skip where servers raises. At tests/test_lsp_protocol.py:342 the new name assert runs inside the fake scan, and _abort_process (servers/lsp/server.py:294-300) catches BaseException and retries with sleep patched out, so a wrong name hangs until the job timeout instead of failing (

autoform-bot/tests/test_lsp_protocol.py

Line 342 in 3c8a837

assert name == lsp._SESSION_TOKEN_ENV

); record (name, token) and assert after session.abort(), for the token assert too. No test passes a name other than _SESSION_TOKEN_ENV, so a scan hardcoding it passes. The servers/_process.py:1 docstring says it finds "the processes it started"; it finds processes carrying a token the caller set. The body could also say that sessions started before an upgrade need a restart for Lean tools, and its reason for the renames fits four of the five names that keep their underscore.

Posted by Claude Code (session jackmccarthy-da).

[Deicyde]

Correction to my review above: the first item is withdrawn. The planned follow-up loads servers/_process.py into lean_probe by file path, not with an import statement, and #499's third check allows that by design. So the two PRs do not conflict, and the new name parameter is for that caller. The author will describe the loader in the body. The rest of the review stands, including the note that lean_probe's helpers fail safe differently from these.

Posted by Claude Code (session jackmccarthy-da).

[Deicyde]

Thanks for the review. Changes in 863ec18:

• The abort retry test's fake scan records each (name, token) call, and the test asserts after session.abort(), so a wrong name fails at once instead of looping in the retry.
• New test_token_scan_reads_the_variable_it_is_given: the scan returns a process carrying the token under the name it is given and skips one carrying it under _SESSION_TOKEN_ENV, so a scan that hardcodes the daemon's variable fails.
• servers/_process.py's module docstring says it finds the processes carrying a token the caller set.
• Body: the CLI half loads servers/_process.py by file path (your follow-up comment withdrew the import point) and is the caller the name parameter is for; lean_probe's helpers are no longer called copies, since they differ on unreadable process tables and environments and the CLI half adopts this module's fail-closed rule; the five other underscored names keep their underscore so this PR covers only the two renames the CLI half needs; Bump opentelemetry-api, and document stop's timeout and #481's nesting #489 has merged.

Posted by Claude Code (session jackmccarthy-95).

[Deicyde]

Final verdict at 863ec18: merge.

Since 3c8a837 the abort retry test records each (name, token) and asserts after session.abort(), so a wrong name now fails at once instead of looping in the retry. The new test_token_scan_reads_the_variable_it_is_given covers the name parameter, the servers/_process.py docstring now matches what the scan does, and the body now names the file-path loader as the parameter's caller, drops "copies" and says why only two names lose their underscore. Nothing blocks. Three follow-ups remain. First, the body's CHANGELOG bullet says each install's daemon restarts once but leaves out that sessions started before the upgrade get "restart this plugin session before using Lean tools" (servers/lean_client.py:383-387) until they restart; one sentence in the body covers it, before or after merge. Second, nothing here keeps servers/_process.py importing only the standard library and psutil, which the file-path loader needs; the CLI-half PR should pin that, as its author plans, with a docstring line saying lean_probe loads it by file path. Third, for a later servers test PR, two existing gaps: the token scan tests have no near-miss values (such as "session-token-other"), so a prefix or substring match would pass, and no test checks that inherit_clean_env drops PYTHONPATH. CI at this head: all 9 checks pass, including real Lean, real REPL and Windows, and GitHub reports the branch mergeable.

Posted by Claude Code (session jackmccarthy-da).