From ce67b5d46fb1ea3d0edff0aea101f2b79282852a Mon Sep 17 00:00:00 2001 From: bhuvan-somisetty Date: Mon, 14 Sep 2026 21:49:09 +0530 Subject: [PATCH] fix: stop using a raw substring match to detect concore processes concore stop / concore status flagged any process as a concore process just because "concore" appeared anywhere in its cmdline. That matches anything run from a directory that happens to be named "concore" (the default clone directory name for this repo), which has nothing to do with a real concore node and would get force killed on the next concore stop. Match is now based on the generated concorekill.bat filename, or on the process's actual working directory containing the runtime marker files mkconcore.py writes into every generated study (concore.iport plus concore.py/concoredocker.py), instead of a plain text search. Fixes #580 --- concore_cli/commands/_process_match.py | 34 ++++++++++++ concore_cli/commands/status.py | 18 +++---- concore_cli/commands/stop.py | 21 +++----- tests/test_process_match.py | 75 ++++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 24 deletions(-) create mode 100644 concore_cli/commands/_process_match.py create mode 100644 tests/test_process_match.py 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