fix: concore stop can kill unrelated processes (false positive pattern matching)

concore_cli/commands/process_utils.py

@@ -0,0 +1,119 @@
+"""Shared concore process detection logic.
+
+The old substring check ("concore" in cmdline) was dangerously broad — it
+matched editors, git, IDEs, and anything else that happened to touch a file
+with "concore" in its path.  This module provides a strict detector that
+won't nuke your VS Code session when you run ``concore stop``.
+
+See: https://github.com/ControlCore-Project/concore/issues/XXX
+"""
+
+import ntpath
+import os
+import re
+import glob
+
+# Matches command lines that are actually *running* concore study code:
+#   python concore_xxx.py …  |  python -m concore …  |  python3 ./concore.py …
+# Does NOT match:  code concore/README.md  |  git -C concore pull
+_CONCORE_EXEC_RE = re.compile(
+    r"""
+    (?:python[23w]?[\d.]*)          # python interpreter (python, python3, python3.11, pythonw …)
+    (?:\.exe)?                      # optional .exe on Windows
+    \s+.*?                          # optional flags like -u, -m, etc.
+    (?:                             # the actual concore entry point — one of:
+        concore(?:_\w+)?\.py        #   concore.py, concore_plant.py, concore_controller.py …
+      | -m\s+concore                #   python -m concore …
+    )
+    """,
+    re.IGNORECASE | re.VERBOSE,
+)
+
+# Known concore CLI entry points — matched as standalone tokens so
+# "concore stop" or "concore build" are caught but "vim concore/foo" is not.
+_CLI_TOKEN_RE = re.compile(
+    r"(?:^|\s|[/\\])concore(?:\.exe)?\s+(?:stop|status|build|watch|init|validate|doctor|setup|inspect|editor)",
+    re.IGNORECASE,
+)
+
+# PID registry written by concore.py at study startup
+_PID_REGISTRY_FILENAME = "concorekill_pids.txt"
+
+
+def _load_registered_pids():
+    """Collect PIDs from all concorekill_pids.txt files found under cwd."""
+    pids = set()
+    # search current dir and one level deep (covers typical study layouts)
+    for pattern in [_PID_REGISTRY_FILENAME, os.path.join("*", _PID_REGISTRY_FILENAME)]:
+        for path in glob.glob(pattern):
+            try:
+                with open(path, "r") as f:
+                    for line in f:
+                        stripped = line.strip()
+                        if stripped.isdigit():
+                            pids.add(int(stripped))
+            except OSError:
+                continue
+    return pids
+
+
+def _has_concore_markers(cwd):
+    """True if cwd contains concore runtime files (concore.iport + concore.py).
+
+    mkconcore.py copies these into every generated study node directory,
+    so their presence is a reliable signal that the process is a real
+    concore node — not an editor or git operation.
+    """
+    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_study_process(proc_info, registered_pids=None, cwd=None):
+    """Return True only if *proc_info* is genuinely a concore study process.
+
+    Detection priority:
+      1. PID found in the concorekill_pids.txt registry  (most reliable)
+      2. Process is running concorekill.bat               (Windows kill script)
+      3. Command line matches a concore execution pattern  (regex, not substring)
+      4. CLI invocation (concore stop, concore build, …)
+      5. Working directory contains concore runtime markers (concore.iport)
+
+    Args:
+        proc_info: dict with keys "pid", "name", "cmdline" (from psutil).
+        registered_pids: optional pre-loaded set of ints from the PID registry.
+        cwd: optional working directory of the process (for marker-file check).
+    """
+    pid = proc_info.get("pid")
+    cmdline = proc_info.get("cmdline") or []
+    cmdline_str = " ".join(cmdline) if cmdline else ""
+
+    # 1. Trust the PID registry — concore.py registers every study process
+    if registered_pids and pid in registered_pids:
+        return True
+
+    # 2. concorekill.bat — ntpath.basename handles both / and \ separators
+    if any(ntpath.basename(str(arg)).lower() == "concorekill.bat" for arg in cmdline):
+        return True
+
+    # 3. Strict regex: must look like an actual concore execution
+    if _CONCORE_EXEC_RE.search(cmdline_str):
+        return True
+
+    # 4. CLI invocation (concore stop, concore build, etc.) — but NOT the
+    #    current process (already filtered by callers via current_pid check)
+    if _CLI_TOKEN_RE.search(cmdline_str):
+        return True
+
+    # 5. Working dir has concore runtime files (concore.iport + concore.py)
+    if _has_concore_markers(cwd):
+        return True
+
+    return False

concore_cli/commands/status.py

@@ -4,7 +4,7 @@
 from rich.panel import Panel
 from datetime import datetime
 
-from concore_cli.commands._process_match import is_concore_process
+from .process_utils import is_concore_study_process, _load_registered_pids
 
 
 def show_status(console):
@@ -14,6 +14,7 @@ def show_status(console):
 
     try:
         current_pid = os.getpid()
+        registered_pids = _load_registered_pids()
 
         for proc in psutil.process_iter(
             ["pid", "name", "cmdline", "create_time", "memory_info", "cpu_percent"]
@@ -24,14 +25,13 @@ def show_status(console):
                 if proc.info["pid"] == current_pid:
                     continue
 
-                cmdline_str = " ".join(cmdline) if cmdline else ""
-
                 try:
                     cwd = proc.cwd()
                 except (psutil.NoSuchProcess, psutil.AccessDenied, OSError):
                     cwd = None
 
-                if is_concore_process(cmdline, cwd):
+                # strict check — won't match editors or git touching concore files
+                if is_concore_study_process(proc.info, registered_pids, cwd):
                     try:
                         create_time = datetime.fromtimestamp(proc.info["create_time"])
                         uptime = datetime.now() - create_time
@@ -49,8 +49,9 @@ def show_status(console):
                         # Failed to get memory info
                         mem_str = "unknown"
 
+                    cmdline_joined = " ".join(cmdline) if cmdline else ""
                     command = (
-                        " ".join(cmdline[:3]) if len(cmdline) >= 3 else cmdline_str[:50]
+                        " ".join(cmdline[:3]) if len(cmdline) >= 3 else cmdline_joined[:50]
                     )
 
                     concore_processes.append(

concore_cli/commands/stop.py

@@ -4,14 +4,15 @@
 import sys
 from rich.panel import Panel
 
-from concore_cli.commands._process_match import is_concore_process
+from .process_utils import is_concore_study_process, _load_registered_pids
 
 
 def stop_all(console):
     console.print("[cyan]Finding concore processes...[/cyan]\n")
 
     processes_to_kill = []
     current_pid = os.getpid()
+    registered_pids = _load_registered_pids()
 
     try:
         for proc in psutil.process_iter(["pid", "name", "cmdline"]):
@@ -26,7 +27,8 @@ def stop_all(console):
                 except (psutil.NoSuchProcess, psutil.AccessDenied, OSError):
                     cwd = None
 
-                if is_concore_process(cmdline, cwd):
+                # strict check — won't match editors or git touching concore files
+                if is_concore_study_process(proc.info, registered_pids, cwd):
                     processes_to_kill.append(proc)
             except (psutil.NoSuchProcess, psutil.AccessDenied):
                 # Process already exited or access denied; continue

tests/test_process_detection.py

@@ -0,0 +1,112 @@
+"""Tests for the stricter concore process detection logic.
+
+Verifies that is_concore_study_process() correctly identifies real concore
+study processes while rejecting false positives like editors, git, and IDEs
+that happen to have "concore" in their command line.
+
+Import bypasses concore_cli.__init__ (which pulls in click) so the test
+runs even when click isn't installed — keeps the CI matrix lean.
+"""
+
+import sys
+import os
+import importlib
+import pytest
+
+# direct import avoids the concore_cli.__init__ → click dependency chain
+_mod_path = os.path.join(
+    os.path.dirname(__file__), os.pardir,
+    "concore_cli", "commands", "process_utils.py",
+)
+spec = importlib.util.spec_from_file_location("process_utils", _mod_path)
+process_utils = importlib.util.module_from_spec(spec)
+spec.loader.exec_module(process_utils)
+
+is_concore_study_process = process_utils.is_concore_study_process
+
+
+# -- Processes that SHOULD be detected (true positives) --
+
+@pytest.mark.parametrize("cmdline,desc", [
+    (["python", "concore_plant.py"], "typical study node"),
+    (["python3", "concore_controller.py", "--port", "5555"], "study node with args"),
+    (["python", "-u", "concore.py"], "unbuffered concore.py"),
+    (["python.exe", "concore.py"], "Windows python.exe"),
+    (["python3.11", "concore_sensor.py"], "versioned python"),
+    (["python", "-m", "concore", "run"], "module invocation"),
+    (["C:\\Python311\\python.exe", "concore_observer.py"], "full path python"),
+    (["concorekill.bat"], "Windows kill script"),
+    (["cmd.exe", "/c", "concorekill.bat"], "kill script via cmd"),
+    (["concore", "build", "workflow.graphml"], "CLI build command"),
+    (["concore", "stop"], "CLI stop command"),
+    (["concore", "watch", "./study"], "CLI watch command"),
+])
+def test_true_positives(cmdline, desc):
+    """Real concore processes must always be detected."""
+    proc = {"pid": 1234, "name": "python", "cmdline": cmdline}
+    assert is_concore_study_process(proc) is True, f"missed: {desc}"
+
+
+# -- Processes that MUST NOT be detected (false positives the old code hit) --
+
+@pytest.mark.parametrize("cmdline,desc", [
+    (["code", "concore/README.md"], "VS Code editing concore file"),
+    (["vim", "/home/user/concore/concore_base.py"], "vim editing concore source"),
+    (["git", "-C", "concore", "pull"], "git pull inside concore repo"),
+    (["git", "log", "--oneline", "concore/stop.py"], "git log on concore file"),
+    (["grep", "-r", "concore", "./src"], "grep searching for concore"),
+    (["cat", "/tmp/concore_debug.log"], "reading a concore log file"),
+    (["less", "concore/README.md"], "less viewing concore docs"),
+    (["find", ".", "-name", "concore*"], "find searching for concore files"),
+    (["notepad.exe", "C:\\repos\\concore\\concore.py"], "notepad editing concore"),
+    (["explorer.exe", "C:\\repos\\concore"], "Windows explorer in concore dir"),
+])
+def test_false_positives_rejected(cmdline, desc):
+    """Editors, git, and tools touching concore files must NOT be killed."""
+    proc = {"pid": 5678, "name": cmdline[0], "cmdline": cmdline}
+    assert is_concore_study_process(proc) is False, f"false positive: {desc}"
+
+
+# -- PID registry takes priority --
+
+def test_pid_registry_match():
+    """A PID in the registry should be detected even with an empty cmdline."""
+    proc = {"pid": 9999, "name": "python", "cmdline": []}
+    assert is_concore_study_process(proc, registered_pids={9999}) is True
+
+
+def test_pid_registry_miss():
+    """A PID NOT in the registry with innocent cmdline should be rejected."""
+    proc = {"pid": 1111, "name": "code", "cmdline": ["code", "concore/foo.py"]}
+    assert is_concore_study_process(proc, registered_pids={9999}) is False
+
+
+# -- Edge cases --
+
+def test_empty_cmdline():
+    """Process with no cmdline (zombie, kernel thread) should not match."""
+    proc = {"pid": 42, "name": "python", "cmdline": []}
+    assert is_concore_study_process(proc) is False
+
+
+def test_none_cmdline():
+    """Process where psutil returns None for cmdline."""
+    proc = {"pid": 42, "name": "python", "cmdline": None}
+    assert is_concore_study_process(proc) is False
+
+
+# -- cwd marker-file detection --
+
+def test_cwd_markers_match(tmp_path):
+    """Process in a dir with concore runtime files should be detected."""
+    # create the marker files mkconcore.py generates for study nodes
+    (tmp_path / "concore.iport").touch()
+    (tmp_path / "concore.py").touch()
+    proc = {"pid": 7777, "name": "python", "cmdline": ["python", "some_script.py"]}
+    assert is_concore_study_process(proc, cwd=str(tmp_path)) is True
+
+
+def test_cwd_markers_missing():
+    """Process with no cwd (or non-study dir) should not match on markers."""
+    proc = {"pid": 7777, "name": "python", "cmdline": ["python", "some_script.py"]}
+    assert is_concore_study_process(proc, cwd=None) is False