diff --git a/concore_cli/commands/_process_match.py b/concore_cli/commands/_process_match.py new file mode 100644 index 0000000..fd46282 --- /dev/null +++ b/concore_cli/commands/_process_match.py @@ -0,0 +1,34 @@ +import os + + +def has_concore_markers(cwd): + """True only if `cwd` looks like an actual concore-generated node + working directory, i.e. it contains the runtime files mkconcore.py + copies into every generated study (concore.iport/oport plus the + runtime module itself).""" + if not cwd: + return False + try: + if not os.path.isfile(os.path.join(cwd, "concore.iport")): + return False + return os.path.isfile(os.path.join(cwd, "concore.py")) or os.path.isfile( + os.path.join(cwd, "concoredocker.py") + ) + except OSError: + return False + + +def is_concore_process(cmdline, cwd): + """Decide whether a process is an actual concore node process. + + A plain substring check like "concore" in the joined cmdline used to + be used here, which matches anything launched from a directory that + merely happens to have "concore" in its path (the default clone + directory name for this repo among other things) and has nothing to + do with concore at all. Instead, only match the generated kill + script by exact filename, or a process whose working directory + actually contains the concore runtime marker files. + """ + if any(os.path.basename(str(item)).lower() == "concorekill.bat" for item in cmdline): + return True + return has_concore_markers(cwd) diff --git a/concore_cli/commands/status.py b/concore_cli/commands/status.py index 7ef1fca..94c8bc0 100644 --- a/concore_cli/commands/status.py +++ b/concore_cli/commands/status.py @@ -4,6 +4,8 @@ from rich.panel import Panel from datetime import datetime +from concore_cli.commands._process_match import is_concore_process + def show_status(console): console.print("[cyan]Scanning for concore processes...[/cyan]\n") @@ -18,24 +20,18 @@ def show_status(console): ): try: cmdline = proc.info.get("cmdline") or [] - name = proc.info.get("name", "").lower() if proc.info["pid"] == current_pid: continue cmdline_str = " ".join(cmdline) if cmdline else "" - is_concore = ( - "concore" in cmdline_str.lower() - or "concore.py" in cmdline_str.lower() - or any("concorekill.bat" in str(item) for item in cmdline) - or ( - name in ["python.exe", "python", "python3"] - and "concore" in cmdline_str - ) - ) + try: + cwd = proc.cwd() + except (psutil.NoSuchProcess, psutil.AccessDenied, OSError): + cwd = None - if is_concore: + if is_concore_process(cmdline, cwd): try: create_time = datetime.fromtimestamp(proc.info["create_time"]) uptime = datetime.now() - create_time diff --git a/concore_cli/commands/stop.py b/concore_cli/commands/stop.py index 27b5796..168eb87 100644 --- a/concore_cli/commands/stop.py +++ b/concore_cli/commands/stop.py @@ -4,6 +4,8 @@ import sys from rich.panel import Panel +from concore_cli.commands._process_match import is_concore_process + def stop_all(console): console.print("[cyan]Finding concore processes...[/cyan]\n") @@ -18,20 +20,13 @@ def stop_all(console): continue cmdline = proc.info.get("cmdline") or [] - name = proc.info.get("name", "").lower() - cmdline_str = " ".join(cmdline) if cmdline else "" - - is_concore = ( - "concore" in cmdline_str.lower() - or "concore.py" in cmdline_str.lower() - or any("concorekill.bat" in str(item) for item in cmdline) - or ( - name in ["python.exe", "python", "python3"] - and "concore" in cmdline_str - ) - ) - if is_concore: + try: + cwd = proc.cwd() + except (psutil.NoSuchProcess, psutil.AccessDenied, OSError): + cwd = None + + if is_concore_process(cmdline, cwd): processes_to_kill.append(proc) except (psutil.NoSuchProcess, psutil.AccessDenied): # Process already exited or access denied; continue diff --git a/tests/test_process_match.py b/tests/test_process_match.py new file mode 100644 index 0000000..9f39649 --- /dev/null +++ b/tests/test_process_match.py @@ -0,0 +1,75 @@ +"""Tests for concore_cli.commands._process_match (Issue #580). + +`concore stop` / `concore status` used to flag any process as a +"concore process" just because "concore" showed up somewhere in its +cmdline, which is true for basically anything launched from inside a +folder named "concore" (the default clone directory name for this +repo) and has nothing to do with a real concore node. +""" + +import os + +from concore_cli.commands._process_match import has_concore_markers, is_concore_process + + +class TestHasConcoreMarkers: + def test_true_when_iport_and_runtime_file_present(self, tmp_path): + (tmp_path / "concore.iport").write_text("{}") + (tmp_path / "concore.py").write_text("") + assert has_concore_markers(str(tmp_path)) is True + + def test_true_with_docker_runtime_file(self, tmp_path): + (tmp_path / "concore.iport").write_text("{}") + (tmp_path / "concoredocker.py").write_text("") + assert has_concore_markers(str(tmp_path)) is True + + def test_false_without_iport(self, tmp_path): + (tmp_path / "concore.py").write_text("") + assert has_concore_markers(str(tmp_path)) is False + + def test_false_without_runtime_file(self, tmp_path): + (tmp_path / "concore.iport").write_text("{}") + assert has_concore_markers(str(tmp_path)) is False + + def test_false_for_empty_cwd(self): + assert has_concore_markers(None) is False + assert has_concore_markers("") is False + + def test_false_for_unrelated_folder_literally_named_concore(self, tmp_path): + # A folder just happening to be named "concore" (e.g. a plain + # git clone of this repo) is not, by itself, a running node's + # working directory. + concore_dir = tmp_path / "concore" + concore_dir.mkdir() + (concore_dir / "README.md").write_text("") + assert has_concore_markers(str(concore_dir)) is False + + +class TestIsConcoreProcess: + def test_true_for_generated_kill_script(self): + cmdline = [r"C:\studies\run1\concorekill.bat"] + assert is_concore_process(cmdline, cwd=None) is True + + def test_true_when_cwd_has_markers(self, tmp_path): + (tmp_path / "concore.iport").write_text("{}") + (tmp_path / "concore.py").write_text("") + cmdline = ["python", "controller.py"] + assert is_concore_process(cmdline, cwd=str(tmp_path)) is True + + def test_false_for_unrelated_process_in_a_concore_named_folder(self, tmp_path): + # This is the actual bug: previously, having "concore" anywhere + # in the argv (e.g. a path under a folder named "concore") was + # enough to be treated as a concore process and get killed. + concore_dir = tmp_path / "concore" + concore_dir.mkdir() + cmdline = ["node", os.path.join(str(concore_dir), "tool.js")] + assert is_concore_process(cmdline, cwd=str(concore_dir)) is False + + def test_false_for_unrelated_python_script_mentioning_concore(self, tmp_path): + concore_dir = tmp_path / "concore" + concore_dir.mkdir() + cmdline = ["python", os.path.join(str(concore_dir), "unrelated_report.py")] + assert is_concore_process(cmdline, cwd=str(concore_dir)) is False + + def test_false_for_empty_cmdline_and_cwd(self): + assert is_concore_process([], None) is False