fix(tests): stop install-script E2E tests from killing live conductor processes by jrob5756 · Pull Request #333 · microsoft/conductor

Jason Robert (jrob5756) · 2026-07-21T21:20:29Z

Summary

Fixes#331 — the install-script E2E suite (@pytest.mark.install_scripts) drives install.sh/install.ps1's --auto-stop flag, whose find_running_conductor() scans the whole host process table and SIGTERM-kills every live conductor process it finds. Nothing deselected these tests by default, so a plain pytest, pytest -m "not performance", or even CI's main test job (-m "not real_api and not performance", which never mentions install_scripts) would run them and could silently kill an unrelated live conductor run --web-bg workflow.

Changes

tests/conftest.py: generalized the existing real_api auto-skip hook (issue real_api tests are not deselected by default; running the suite spawns real copilot subprocesses (can silently kill a live --web-bg workflow) #326) into a loop over ("real_api", "install_scripts"), using the same word-boundary -m opt-in detection. Both markers now auto-skip unless the caller's -m expression explicitly references them.
tests/test_integration/install_scripts_helpers.py: run_install_script(..., auto_stop=True) → default False (defense-in-depth). Only test_running_process_auto_stop_kills_and_continues needs the kill behavior and already passes auto_stop=True explicitly, so no other call sites changed.
tests/test_config/test_install_scripts_marker.py (new): regression suite mirroring test_real_api_marker.py's pytester-based approach — covers default skip, -m "not performance", CI's exact main-job expression, explicit -m install_scripts opt-in, -m "not install_scripts" (current make test filter), regex word-boundary robustness, and a "test-the-test" case proving a reverted hook would be caught.
Doc touch-ups in test_install_scripts.py / install_scripts_helpers.py docstrings reflecting the new safe-by-default behavior.

Out of scope

Changing install.sh/install.ps1's find_running_conductor host-wide scan breadth — explicitly called out as optional in the issue.
Makefile's existing -m "not install_scripts" filters — redundant with the new hook now but harmless, kept for explicitness.

Verification

New test_install_scripts_marker.py suite: 7/7 passed.
Existing test_real_api_marker.py: 6/6 passed (no regression from generalizing the hook).
uv run pytest -m "not performance" -q: all install_scripts-marked tests now show SKIPPED ... install_scripts test: opt in with -m install_scripts (previously they ran).
uv run pytest -m "not install_scripts and not performance": 4184 passed, 0 failed.
make lint, make typecheck: clean (one pre-existing, unrelated ty warning in dialog_evaluator.py confirmed present on main too).
Confirmed tests/test_performance.py::TestForEachPerformance::test_batching_scalability fails identically on unmodified main — pre-existing timing flake, unrelated to this change.

… processes The install_scripts-marked E2E suite runs install.sh/install.ps1's --auto-stop flag, whose find_running_conductor() scans the whole host process table and SIGTERM-kills every live conductor process it finds. Nothing deselected these tests by default, so a plain `pytest`, `pytest -m "not performance"`, or even CI's main test job (-m "not real_api and not performance", which never mentions install_scripts) would run them and could kill an unrelated live `conductor run --web-bg` workflow. - Generalize tests/conftest.py's real_api auto-skip hook (issue #326) to also cover the install_scripts marker, using the same word-boundary -m opt-in detection. - Default run_install_script(..., auto_stop=False) in install_scripts_helpers.py as defense-in-depth, so no call site triggers the host-wide kill unless a test explicitly opts in (only test_running_process_auto_stop_kills_and_continues does, and already passed auto_stop=True explicitly). - Add tests/test_config/test_install_scripts_marker.py, mirroring the existing real_api marker regression suite, to lock in the new auto-skip behavior (including a "test-the-test" case proving a reverted hook would be caught). Fixes#331 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Follow-up from code review of the #331 fix: - Add tests/test_integration/test_run_install_script_command.py: a fast, unmarked (non-install_scripts) test that mocks subprocess.run to verify run_install_script()'s auto_stop/force -> --auto-stop/--force command construction. This closes a gap where a future change to the auto_stop default wouldn't be caught by anything in the default (skipped install_scripts) test run - only by the E2E suite itself. Verified it fails when auto_stop's default is flipped back to True. - tests/conftest.py: trim duplicated rationale between the _OPT_IN_MARKER_NAMES comment and the pytest_collection_modifyitems docstring (defer to the docstring), and correct an imprecise claim that make test relies on this hook for install_scripts - it doesn't, since Makefile's own -m "not install_scripts" already deselects it natively; the hook is only load-bearing there for real_api. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Jason Robert (jrob5756) · 2026-07-21T21:20:29Z

Summary

Fixes#331 — the install-script E2E suite (@pytest.mark.install_scripts) drives install.sh/install.ps1's --auto-stop flag, whose find_running_conductor() scans the whole host process table and SIGTERM-kills every live conductor process it finds. Nothing deselected these tests by default, so a plain pytest, pytest -m "not performance", or even CI's main test job (-m "not real_api and not performance", which never mentions install_scripts) would run them and could silently kill an unrelated live conductor run --web-bg workflow.

Changes

tests/conftest.py: generalized the existing real_api auto-skip hook (issue real_api tests are not deselected by default; running the suite spawns real copilot subprocesses (can silently kill a live --web-bg workflow) #326) into a loop over ("real_api", "install_scripts"), using the same word-boundary -m opt-in detection. Both markers now auto-skip unless the caller's -m expression explicitly references them.
tests/test_integration/install_scripts_helpers.py: run_install_script(..., auto_stop=True) → default False (defense-in-depth). Only test_running_process_auto_stop_kills_and_continues needs the kill behavior and already passes auto_stop=True explicitly, so no other call sites changed.
tests/test_config/test_install_scripts_marker.py (new): regression suite mirroring test_real_api_marker.py's pytester-based approach — covers default skip, -m "not performance", CI's exact main-job expression, explicit -m install_scripts opt-in, -m "not install_scripts" (current make test filter), regex word-boundary robustness, and a "test-the-test" case proving a reverted hook would be caught.
Doc touch-ups in test_install_scripts.py / install_scripts_helpers.py docstrings reflecting the new safe-by-default behavior.

Out of scope

Changing install.sh/install.ps1's find_running_conductor host-wide scan breadth — explicitly called out as optional in the issue.
Makefile's existing -m "not install_scripts" filters — redundant with the new hook now but harmless, kept for explicitness.

Verification

New test_install_scripts_marker.py suite: 7/7 passed.
Existing test_real_api_marker.py: 6/6 passed (no regression from generalizing the hook).
uv run pytest -m "not performance" -q: all install_scripts-marked tests now show SKIPPED ... install_scripts test: opt in with -m install_scripts (previously they ran).
uv run pytest -m "not install_scripts and not performance": 4184 passed, 0 failed.
make lint, make typecheck: clean (one pre-existing, unrelated ty warning in dialog_evaluator.py confirmed present on main too).
Confirmed tests/test_performance.py::TestForEachPerformance::test_batching_scalability fails identically on unmodified main — pre-existing timing flake, unrelated to this change.

… processes The install_scripts-marked E2E suite runs install.sh/install.ps1's --auto-stop flag, whose find_running_conductor() scans the whole host process table and SIGTERM-kills every live conductor process it finds. Nothing deselected these tests by default, so a plain `pytest`, `pytest -m "not performance"`, or even CI's main test job (-m "not real_api and not performance", which never mentions install_scripts) would run them and could kill an unrelated live `conductor run --web-bg` workflow. - Generalize tests/conftest.py's real_api auto-skip hook (issue #326) to also cover the install_scripts marker, using the same word-boundary -m opt-in detection. - Default run_install_script(..., auto_stop=False) in install_scripts_helpers.py as defense-in-depth, so no call site triggers the host-wide kill unless a test explicitly opts in (only test_running_process_auto_stop_kills_and_continues does, and already passed auto_stop=True explicitly). - Add tests/test_config/test_install_scripts_marker.py, mirroring the existing real_api marker regression suite, to lock in the new auto-skip behavior (including a "test-the-test" case proving a reverted hook would be caught). Fixes#331 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Follow-up from code review of the #331 fix: - Add tests/test_integration/test_run_install_script_command.py: a fast, unmarked (non-install_scripts) test that mocks subprocess.run to verify run_install_script()'s auto_stop/force -> --auto-stop/--force command construction. This closes a gap where a future change to the auto_stop default wouldn't be caught by anything in the default (skipped install_scripts) test run - only by the E2E suite itself. Verified it fails when auto_stop's default is flipped back to True. - tests/conftest.py: trim duplicated rationale between the _OPT_IN_MARKER_NAMES comment and the pytest_collection_modifyitems docstring (defer to the docstring), and correct an imprecise claim that make test relies on this hook for install_scripts - it doesn't, since Makefile's own -m "not install_scripts" already deselects it natively; the hook is only load-bearing there for real_api. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Jason Robert (jrob5756) · 2026-07-21T21:20:29Z

Summary

Fixes#331 — the install-script E2E suite (@pytest.mark.install_scripts) drives install.sh/install.ps1's --auto-stop flag, whose find_running_conductor() scans the whole host process table and SIGTERM-kills every live conductor process it finds. Nothing deselected these tests by default, so a plain pytest, pytest -m "not performance", or even CI's main test job (-m "not real_api and not performance", which never mentions install_scripts) would run them and could silently kill an unrelated live conductor run --web-bg workflow.

Changes

tests/conftest.py: generalized the existing real_api auto-skip hook (issue real_api tests are not deselected by default; running the suite spawns real copilot subprocesses (can silently kill a live --web-bg workflow) #326) into a loop over ("real_api", "install_scripts"), using the same word-boundary -m opt-in detection. Both markers now auto-skip unless the caller's -m expression explicitly references them.
tests/test_integration/install_scripts_helpers.py: run_install_script(..., auto_stop=True) → default False (defense-in-depth). Only test_running_process_auto_stop_kills_and_continues needs the kill behavior and already passes auto_stop=True explicitly, so no other call sites changed.
tests/test_config/test_install_scripts_marker.py (new): regression suite mirroring test_real_api_marker.py's pytester-based approach — covers default skip, -m "not performance", CI's exact main-job expression, explicit -m install_scripts opt-in, -m "not install_scripts" (current make test filter), regex word-boundary robustness, and a "test-the-test" case proving a reverted hook would be caught.
Doc touch-ups in test_install_scripts.py / install_scripts_helpers.py docstrings reflecting the new safe-by-default behavior.

Out of scope

Changing install.sh/install.ps1's find_running_conductor host-wide scan breadth — explicitly called out as optional in the issue.
Makefile's existing -m "not install_scripts" filters — redundant with the new hook now but harmless, kept for explicitness.

Verification

New test_install_scripts_marker.py suite: 7/7 passed.
Existing test_real_api_marker.py: 6/6 passed (no regression from generalizing the hook).
uv run pytest -m "not performance" -q: all install_scripts-marked tests now show SKIPPED ... install_scripts test: opt in with -m install_scripts (previously they ran).
uv run pytest -m "not install_scripts and not performance": 4184 passed, 0 failed.
make lint, make typecheck: clean (one pre-existing, unrelated ty warning in dialog_evaluator.py confirmed present on main too).
Confirmed tests/test_performance.py::TestForEachPerformance::test_batching_scalability fails identically on unmodified main — pre-existing timing flake, unrelated to this change.

… processes The install_scripts-marked E2E suite runs install.sh/install.ps1's --auto-stop flag, whose find_running_conductor() scans the whole host process table and SIGTERM-kills every live conductor process it finds. Nothing deselected these tests by default, so a plain `pytest`, `pytest -m "not performance"`, or even CI's main test job (-m "not real_api and not performance", which never mentions install_scripts) would run them and could kill an unrelated live `conductor run --web-bg` workflow. - Generalize tests/conftest.py's real_api auto-skip hook (issue #326) to also cover the install_scripts marker, using the same word-boundary -m opt-in detection. - Default run_install_script(..., auto_stop=False) in install_scripts_helpers.py as defense-in-depth, so no call site triggers the host-wide kill unless a test explicitly opts in (only test_running_process_auto_stop_kills_and_continues does, and already passed auto_stop=True explicitly). - Add tests/test_config/test_install_scripts_marker.py, mirroring the existing real_api marker regression suite, to lock in the new auto-skip behavior (including a "test-the-test" case proving a reverted hook would be caught). Fixes#331 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Follow-up from code review of the #331 fix: - Add tests/test_integration/test_run_install_script_command.py: a fast, unmarked (non-install_scripts) test that mocks subprocess.run to verify run_install_script()'s auto_stop/force -> --auto-stop/--force command construction. This closes a gap where a future change to the auto_stop default wouldn't be caught by anything in the default (skipped install_scripts) test run - only by the E2E suite itself. Verified it fails when auto_stop's default is flipped back to True. - tests/conftest.py: trim duplicated rationale between the _OPT_IN_MARKER_NAMES comment and the pytest_collection_modifyitems docstring (defer to the docstring), and correct an imprecise claim that make test relies on this hook for install_scripts - it doesn't, since Makefile's own -m "not install_scripts" already deselects it natively; the hook is only load-bearing there for real_api. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Jason Robert (jrob5756) · 2026-07-21T21:20:29Z

Summary

Fixes#331 — the install-script E2E suite (@pytest.mark.install_scripts) drives install.sh/install.ps1's --auto-stop flag, whose find_running_conductor() scans the whole host process table and SIGTERM-kills every live conductor process it finds. Nothing deselected these tests by default, so a plain pytest, pytest -m "not performance", or even CI's main test job (-m "not real_api and not performance", which never mentions install_scripts) would run them and could silently kill an unrelated live conductor run --web-bg workflow.

Changes

tests/conftest.py: generalized the existing real_api auto-skip hook (issue real_api tests are not deselected by default; running the suite spawns real copilot subprocesses (can silently kill a live --web-bg workflow) #326) into a loop over ("real_api", "install_scripts"), using the same word-boundary -m opt-in detection. Both markers now auto-skip unless the caller's -m expression explicitly references them.
tests/test_integration/install_scripts_helpers.py: run_install_script(..., auto_stop=True) → default False (defense-in-depth). Only test_running_process_auto_stop_kills_and_continues needs the kill behavior and already passes auto_stop=True explicitly, so no other call sites changed.
tests/test_config/test_install_scripts_marker.py (new): regression suite mirroring test_real_api_marker.py's pytester-based approach — covers default skip, -m "not performance", CI's exact main-job expression, explicit -m install_scripts opt-in, -m "not install_scripts" (current make test filter), regex word-boundary robustness, and a "test-the-test" case proving a reverted hook would be caught.
Doc touch-ups in test_install_scripts.py / install_scripts_helpers.py docstrings reflecting the new safe-by-default behavior.

Out of scope

Changing install.sh/install.ps1's find_running_conductor host-wide scan breadth — explicitly called out as optional in the issue.
Makefile's existing -m "not install_scripts" filters — redundant with the new hook now but harmless, kept for explicitness.

Verification

New test_install_scripts_marker.py suite: 7/7 passed.
Existing test_real_api_marker.py: 6/6 passed (no regression from generalizing the hook).
uv run pytest -m "not performance" -q: all install_scripts-marked tests now show SKIPPED ... install_scripts test: opt in with -m install_scripts (previously they ran).
uv run pytest -m "not install_scripts and not performance": 4184 passed, 0 failed.
make lint, make typecheck: clean (one pre-existing, unrelated ty warning in dialog_evaluator.py confirmed present on main too).
Confirmed tests/test_performance.py::TestForEachPerformance::test_batching_scalability fails identically on unmodified main — pre-existing timing flake, unrelated to this change.

… processes The install_scripts-marked E2E suite runs install.sh/install.ps1's --auto-stop flag, whose find_running_conductor() scans the whole host process table and SIGTERM-kills every live conductor process it finds. Nothing deselected these tests by default, so a plain `pytest`, `pytest -m "not performance"`, or even CI's main test job (-m "not real_api and not performance", which never mentions install_scripts) would run them and could kill an unrelated live `conductor run --web-bg` workflow. - Generalize tests/conftest.py's real_api auto-skip hook (issue #326) to also cover the install_scripts marker, using the same word-boundary -m opt-in detection. - Default run_install_script(..., auto_stop=False) in install_scripts_helpers.py as defense-in-depth, so no call site triggers the host-wide kill unless a test explicitly opts in (only test_running_process_auto_stop_kills_and_continues does, and already passed auto_stop=True explicitly). - Add tests/test_config/test_install_scripts_marker.py, mirroring the existing real_api marker regression suite, to lock in the new auto-skip behavior (including a "test-the-test" case proving a reverted hook would be caught). Fixes#331 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Follow-up from code review of the #331 fix: - Add tests/test_integration/test_run_install_script_command.py: a fast, unmarked (non-install_scripts) test that mocks subprocess.run to verify run_install_script()'s auto_stop/force -> --auto-stop/--force command construction. This closes a gap where a future change to the auto_stop default wouldn't be caught by anything in the default (skipped install_scripts) test run - only by the E2E suite itself. Verified it fails when auto_stop's default is flipped back to True. - tests/conftest.py: trim duplicated rationale between the _OPT_IN_MARKER_NAMES comment and the pytest_collection_modifyitems docstring (defer to the docstring), and correct an imprecise claim that make test relies on this hook for install_scripts - it doesn't, since Makefile's own -m "not install_scripts" already deselects it natively; the hook is only load-bearing there for real_api. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Jason Robert (jrob5756) · 2026-07-21T21:20:29Z

Summary

Fixes#331 — the install-script E2E suite (@pytest.mark.install_scripts) drives install.sh/install.ps1's --auto-stop flag, whose find_running_conductor() scans the whole host process table and SIGTERM-kills every live conductor process it finds. Nothing deselected these tests by default, so a plain pytest, pytest -m "not performance", or even CI's main test job (-m "not real_api and not performance", which never mentions install_scripts) would run them and could silently kill an unrelated live conductor run --web-bg workflow.

Changes

tests/conftest.py: generalized the existing real_api auto-skip hook (issue real_api tests are not deselected by default; running the suite spawns real copilot subprocesses (can silently kill a live --web-bg workflow) #326) into a loop over ("real_api", "install_scripts"), using the same word-boundary -m opt-in detection. Both markers now auto-skip unless the caller's -m expression explicitly references them.
tests/test_integration/install_scripts_helpers.py: run_install_script(..., auto_stop=True) → default False (defense-in-depth). Only test_running_process_auto_stop_kills_and_continues needs the kill behavior and already passes auto_stop=True explicitly, so no other call sites changed.
tests/test_config/test_install_scripts_marker.py (new): regression suite mirroring test_real_api_marker.py's pytester-based approach — covers default skip, -m "not performance", CI's exact main-job expression, explicit -m install_scripts opt-in, -m "not install_scripts" (current make test filter), regex word-boundary robustness, and a "test-the-test" case proving a reverted hook would be caught.
Doc touch-ups in test_install_scripts.py / install_scripts_helpers.py docstrings reflecting the new safe-by-default behavior.

Out of scope

Changing install.sh/install.ps1's find_running_conductor host-wide scan breadth — explicitly called out as optional in the issue.
Makefile's existing -m "not install_scripts" filters — redundant with the new hook now but harmless, kept for explicitness.

Verification

New test_install_scripts_marker.py suite: 7/7 passed.
Existing test_real_api_marker.py: 6/6 passed (no regression from generalizing the hook).
uv run pytest -m "not performance" -q: all install_scripts-marked tests now show SKIPPED ... install_scripts test: opt in with -m install_scripts (previously they ran).
uv run pytest -m "not install_scripts and not performance": 4184 passed, 0 failed.
make lint, make typecheck: clean (one pre-existing, unrelated ty warning in dialog_evaluator.py confirmed present on main too).
Confirmed tests/test_performance.py::TestForEachPerformance::test_batching_scalability fails identically on unmodified main — pre-existing timing flake, unrelated to this change.

… processes The install_scripts-marked E2E suite runs install.sh/install.ps1's --auto-stop flag, whose find_running_conductor() scans the whole host process table and SIGTERM-kills every live conductor process it finds. Nothing deselected these tests by default, so a plain `pytest`, `pytest -m "not performance"`, or even CI's main test job (-m "not real_api and not performance", which never mentions install_scripts) would run them and could kill an unrelated live `conductor run --web-bg` workflow. - Generalize tests/conftest.py's real_api auto-skip hook (issue #326) to also cover the install_scripts marker, using the same word-boundary -m opt-in detection. - Default run_install_script(..., auto_stop=False) in install_scripts_helpers.py as defense-in-depth, so no call site triggers the host-wide kill unless a test explicitly opts in (only test_running_process_auto_stop_kills_and_continues does, and already passed auto_stop=True explicitly). - Add tests/test_config/test_install_scripts_marker.py, mirroring the existing real_api marker regression suite, to lock in the new auto-skip behavior (including a "test-the-test" case proving a reverted hook would be caught). Fixes#331 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Follow-up from code review of the #331 fix: - Add tests/test_integration/test_run_install_script_command.py: a fast, unmarked (non-install_scripts) test that mocks subprocess.run to verify run_install_script()'s auto_stop/force -> --auto-stop/--force command construction. This closes a gap where a future change to the auto_stop default wouldn't be caught by anything in the default (skipped install_scripts) test run - only by the E2E suite itself. Verified it fails when auto_stop's default is flipped back to True. - tests/conftest.py: trim duplicated rationale between the _OPT_IN_MARKER_NAMES comment and the pytest_collection_modifyitems docstring (defer to the docstring), and correct an imprecise claim that make test relies on this hook for install_scripts - it doesn't, since Makefile's own -m "not install_scripts" already deselects it natively; the hook is only load-bearing there for real_api. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Jason Robert (jrob5756) · 2026-07-21T21:20:29Z

Summary

Fixes#331 — the install-script E2E suite (@pytest.mark.install_scripts) drives install.sh/install.ps1's --auto-stop flag, whose find_running_conductor() scans the whole host process table and SIGTERM-kills every live conductor process it finds. Nothing deselected these tests by default, so a plain pytest, pytest -m "not performance", or even CI's main test job (-m "not real_api and not performance", which never mentions install_scripts) would run them and could silently kill an unrelated live conductor run --web-bg workflow.

Changes

tests/conftest.py: generalized the existing real_api auto-skip hook (issue real_api tests are not deselected by default; running the suite spawns real copilot subprocesses (can silently kill a live --web-bg workflow) #326) into a loop over ("real_api", "install_scripts"), using the same word-boundary -m opt-in detection. Both markers now auto-skip unless the caller's -m expression explicitly references them.
tests/test_integration/install_scripts_helpers.py: run_install_script(..., auto_stop=True) → default False (defense-in-depth). Only test_running_process_auto_stop_kills_and_continues needs the kill behavior and already passes auto_stop=True explicitly, so no other call sites changed.
tests/test_config/test_install_scripts_marker.py (new): regression suite mirroring test_real_api_marker.py's pytester-based approach — covers default skip, -m "not performance", CI's exact main-job expression, explicit -m install_scripts opt-in, -m "not install_scripts" (current make test filter), regex word-boundary robustness, and a "test-the-test" case proving a reverted hook would be caught.
Doc touch-ups in test_install_scripts.py / install_scripts_helpers.py docstrings reflecting the new safe-by-default behavior.

Out of scope

Changing install.sh/install.ps1's find_running_conductor host-wide scan breadth — explicitly called out as optional in the issue.
Makefile's existing -m "not install_scripts" filters — redundant with the new hook now but harmless, kept for explicitness.

Verification

New test_install_scripts_marker.py suite: 7/7 passed.
Existing test_real_api_marker.py: 6/6 passed (no regression from generalizing the hook).
uv run pytest -m "not performance" -q: all install_scripts-marked tests now show SKIPPED ... install_scripts test: opt in with -m install_scripts (previously they ran).
uv run pytest -m "not install_scripts and not performance": 4184 passed, 0 failed.
make lint, make typecheck: clean (one pre-existing, unrelated ty warning in dialog_evaluator.py confirmed present on main too).
Confirmed tests/test_performance.py::TestForEachPerformance::test_batching_scalability fails identically on unmodified main — pre-existing timing flake, unrelated to this change.

… processes The install_scripts-marked E2E suite runs install.sh/install.ps1's --auto-stop flag, whose find_running_conductor() scans the whole host process table and SIGTERM-kills every live conductor process it finds. Nothing deselected these tests by default, so a plain `pytest`, `pytest -m "not performance"`, or even CI's main test job (-m "not real_api and not performance", which never mentions install_scripts) would run them and could kill an unrelated live `conductor run --web-bg` workflow. - Generalize tests/conftest.py's real_api auto-skip hook (issue #326) to also cover the install_scripts marker, using the same word-boundary -m opt-in detection. - Default run_install_script(..., auto_stop=False) in install_scripts_helpers.py as defense-in-depth, so no call site triggers the host-wide kill unless a test explicitly opts in (only test_running_process_auto_stop_kills_and_continues does, and already passed auto_stop=True explicitly). - Add tests/test_config/test_install_scripts_marker.py, mirroring the existing real_api marker regression suite, to lock in the new auto-skip behavior (including a "test-the-test" case proving a reverted hook would be caught). Fixes#331 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Follow-up from code review of the #331 fix: - Add tests/test_integration/test_run_install_script_command.py: a fast, unmarked (non-install_scripts) test that mocks subprocess.run to verify run_install_script()'s auto_stop/force -> --auto-stop/--force command construction. This closes a gap where a future change to the auto_stop default wouldn't be caught by anything in the default (skipped install_scripts) test run - only by the E2E suite itself. Verified it fails when auto_stop's default is flipped back to True. - tests/conftest.py: trim duplicated rationale between the _OPT_IN_MARKER_NAMES comment and the pytest_collection_modifyitems docstring (defer to the docstring), and correct an imprecise claim that make test relies on this hook for install_scripts - it doesn't, since Makefile's own -m "not install_scripts" already deselects it natively; the hook is only load-bearing there for real_api. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Jason Robert (jrob5756) · 2026-07-21T21:20:29Z

Summary

Fixes#331 — the install-script E2E suite (@pytest.mark.install_scripts) drives install.sh/install.ps1's --auto-stop flag, whose find_running_conductor() scans the whole host process table and SIGTERM-kills every live conductor process it finds. Nothing deselected these tests by default, so a plain pytest, pytest -m "not performance", or even CI's main test job (-m "not real_api and not performance", which never mentions install_scripts) would run them and could silently kill an unrelated live conductor run --web-bg workflow.

Changes

tests/conftest.py: generalized the existing real_api auto-skip hook (issue real_api tests are not deselected by default; running the suite spawns real copilot subprocesses (can silently kill a live --web-bg workflow) #326) into a loop over ("real_api", "install_scripts"), using the same word-boundary -m opt-in detection. Both markers now auto-skip unless the caller's -m expression explicitly references them.
tests/test_integration/install_scripts_helpers.py: run_install_script(..., auto_stop=True) → default False (defense-in-depth). Only test_running_process_auto_stop_kills_and_continues needs the kill behavior and already passes auto_stop=True explicitly, so no other call sites changed.
tests/test_config/test_install_scripts_marker.py (new): regression suite mirroring test_real_api_marker.py's pytester-based approach — covers default skip, -m "not performance", CI's exact main-job expression, explicit -m install_scripts opt-in, -m "not install_scripts" (current make test filter), regex word-boundary robustness, and a "test-the-test" case proving a reverted hook would be caught.
Doc touch-ups in test_install_scripts.py / install_scripts_helpers.py docstrings reflecting the new safe-by-default behavior.

Out of scope

Changing install.sh/install.ps1's find_running_conductor host-wide scan breadth — explicitly called out as optional in the issue.
Makefile's existing -m "not install_scripts" filters — redundant with the new hook now but harmless, kept for explicitness.

Verification

New test_install_scripts_marker.py suite: 7/7 passed.
Existing test_real_api_marker.py: 6/6 passed (no regression from generalizing the hook).
uv run pytest -m "not performance" -q: all install_scripts-marked tests now show SKIPPED ... install_scripts test: opt in with -m install_scripts (previously they ran).
uv run pytest -m "not install_scripts and not performance": 4184 passed, 0 failed.
make lint, make typecheck: clean (one pre-existing, unrelated ty warning in dialog_evaluator.py confirmed present on main too).
Confirmed tests/test_performance.py::TestForEachPerformance::test_batching_scalability fails identically on unmodified main — pre-existing timing flake, unrelated to this change.

… processes The install_scripts-marked E2E suite runs install.sh/install.ps1's --auto-stop flag, whose find_running_conductor() scans the whole host process table and SIGTERM-kills every live conductor process it finds. Nothing deselected these tests by default, so a plain `pytest`, `pytest -m "not performance"`, or even CI's main test job (-m "not real_api and not performance", which never mentions install_scripts) would run them and could kill an unrelated live `conductor run --web-bg` workflow. - Generalize tests/conftest.py's real_api auto-skip hook (issue #326) to also cover the install_scripts marker, using the same word-boundary -m opt-in detection. - Default run_install_script(..., auto_stop=False) in install_scripts_helpers.py as defense-in-depth, so no call site triggers the host-wide kill unless a test explicitly opts in (only test_running_process_auto_stop_kills_and_continues does, and already passed auto_stop=True explicitly). - Add tests/test_config/test_install_scripts_marker.py, mirroring the existing real_api marker regression suite, to lock in the new auto-skip behavior (including a "test-the-test" case proving a reverted hook would be caught). Fixes#331 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Follow-up from code review of the #331 fix: - Add tests/test_integration/test_run_install_script_command.py: a fast, unmarked (non-install_scripts) test that mocks subprocess.run to verify run_install_script()'s auto_stop/force -> --auto-stop/--force command construction. This closes a gap where a future change to the auto_stop default wouldn't be caught by anything in the default (skipped install_scripts) test run - only by the E2E suite itself. Verified it fails when auto_stop's default is flipped back to True. - tests/conftest.py: trim duplicated rationale between the _OPT_IN_MARKER_NAMES comment and the pytest_collection_modifyitems docstring (defer to the docstring), and correct an imprecise claim that make test relies on this hook for install_scripts - it doesn't, since Makefile's own -m "not install_scripts" already deselects it natively; the hook is only load-bearing there for real_api. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Jason Robert (jrob5756) · 2026-07-21T21:20:29Z

Summary

Fixes#331 — the install-script E2E suite (@pytest.mark.install_scripts) drives install.sh/install.ps1's --auto-stop flag, whose find_running_conductor() scans the whole host process table and SIGTERM-kills every live conductor process it finds. Nothing deselected these tests by default, so a plain pytest, pytest -m "not performance", or even CI's main test job (-m "not real_api and not performance", which never mentions install_scripts) would run them and could silently kill an unrelated live conductor run --web-bg workflow.

Changes

tests/conftest.py: generalized the existing real_api auto-skip hook (issue real_api tests are not deselected by default; running the suite spawns real copilot subprocesses (can silently kill a live --web-bg workflow) #326) into a loop over ("real_api", "install_scripts"), using the same word-boundary -m opt-in detection. Both markers now auto-skip unless the caller's -m expression explicitly references them.
tests/test_integration/install_scripts_helpers.py: run_install_script(..., auto_stop=True) → default False (defense-in-depth). Only test_running_process_auto_stop_kills_and_continues needs the kill behavior and already passes auto_stop=True explicitly, so no other call sites changed.
tests/test_config/test_install_scripts_marker.py (new): regression suite mirroring test_real_api_marker.py's pytester-based approach — covers default skip, -m "not performance", CI's exact main-job expression, explicit -m install_scripts opt-in, -m "not install_scripts" (current make test filter), regex word-boundary robustness, and a "test-the-test" case proving a reverted hook would be caught.
Doc touch-ups in test_install_scripts.py / install_scripts_helpers.py docstrings reflecting the new safe-by-default behavior.

Out of scope

Changing install.sh/install.ps1's find_running_conductor host-wide scan breadth — explicitly called out as optional in the issue.
Makefile's existing -m "not install_scripts" filters — redundant with the new hook now but harmless, kept for explicitness.

Verification

New test_install_scripts_marker.py suite: 7/7 passed.
Existing test_real_api_marker.py: 6/6 passed (no regression from generalizing the hook).
uv run pytest -m "not performance" -q: all install_scripts-marked tests now show SKIPPED ... install_scripts test: opt in with -m install_scripts (previously they ran).
uv run pytest -m "not install_scripts and not performance": 4184 passed, 0 failed.
make lint, make typecheck: clean (one pre-existing, unrelated ty warning in dialog_evaluator.py confirmed present on main too).
Confirmed tests/test_performance.py::TestForEachPerformance::test_batching_scalability fails identically on unmodified main — pre-existing timing flake, unrelated to this change.

… processes The install_scripts-marked E2E suite runs install.sh/install.ps1's --auto-stop flag, whose find_running_conductor() scans the whole host process table and SIGTERM-kills every live conductor process it finds. Nothing deselected these tests by default, so a plain `pytest`, `pytest -m "not performance"`, or even CI's main test job (-m "not real_api and not performance", which never mentions install_scripts) would run them and could kill an unrelated live `conductor run --web-bg` workflow. - Generalize tests/conftest.py's real_api auto-skip hook (issue #326) to also cover the install_scripts marker, using the same word-boundary -m opt-in detection. - Default run_install_script(..., auto_stop=False) in install_scripts_helpers.py as defense-in-depth, so no call site triggers the host-wide kill unless a test explicitly opts in (only test_running_process_auto_stop_kills_and_continues does, and already passed auto_stop=True explicitly). - Add tests/test_config/test_install_scripts_marker.py, mirroring the existing real_api marker regression suite, to lock in the new auto-skip behavior (including a "test-the-test" case proving a reverted hook would be caught). Fixes#331 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Follow-up from code review of the #331 fix: - Add tests/test_integration/test_run_install_script_command.py: a fast, unmarked (non-install_scripts) test that mocks subprocess.run to verify run_install_script()'s auto_stop/force -> --auto-stop/--force command construction. This closes a gap where a future change to the auto_stop default wouldn't be caught by anything in the default (skipped install_scripts) test run - only by the E2E suite itself. Verified it fails when auto_stop's default is flipped back to True. - tests/conftest.py: trim duplicated rationale between the _OPT_IN_MARKER_NAMES comment and the pytest_collection_modifyitems docstring (defer to the docstring), and correct an imprecise claim that make test relies on this hook for install_scripts - it doesn't, since Makefile's own -m "not install_scripts" already deselects it natively; the hook is only load-bearing there for real_api. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>