Devel

docs/getting-started.md

@@ -66,6 +66,18 @@ export MATCREATOR_MEMORIZATION_FREQUENCY=0
 export MATCREATOR_REVIEW_FREQUENCY=20
 ```
 
+The web **Settings → MatCreator** tab exposes these knowledge frequencies,
+benchmark settings, and the tool-execution timeout. Python, Bash, and skill
+scripts time out after 3600 seconds by default; set a positive value there or
+persist the equivalent configuration:
+
+```yaml
+runtime:
+  execution_timeout_seconds: 3600
+```
+
+`MATCREATOR_EXEC_TIMEOUT_SECONDS` is the corresponding environment override.
+
 Start an interactive session in the current project workspace:
 
 ```bash

src/matcreator/agents/execution_agent/step_executor_runner.py

@@ -258,14 +258,30 @@ def _split_verified_artifacts(
 def _verify_step_result_artifacts(
     result: StepExecutorResult,
     allowed_roots: Optional[list[Path]] = None,
+    additional_artifacts: Optional[list[str]] = None,
 ) -> tuple[StepExecutorResult, list[str]]:
-    """Prevent successful step results from claiming nonexistent artifacts."""
+    """Prevent successful steps from claiming nonexistent output artifacts.
+
+    ``additional_artifacts`` covers paths returned by tools during the step
+    (notably ``plot_path``).  Those paths are surfaced by the frontend too, so
+    they must be held to the same existence and containment checks as the
+    executor's final artifact list.
+    """
     existing_artifacts, missing_artifacts = _split_verified_artifacts(
         result.artifacts,
         allowed_roots=allowed_roots,
     )
     result.artifacts = existing_artifacts
 
+    if additional_artifacts:
+        _, additional_missing_artifacts = _split_verified_artifacts(
+            additional_artifacts,
+            allowed_roots=allowed_roots,
+        )
+        for artifact in additional_missing_artifacts:
+            if artifact not in missing_artifacts:
+                missing_artifacts.append(artifact)
+
     if result.status == "success" and missing_artifacts:
         missing_text = ", ".join(missing_artifacts)
         result.status = "needs_replanning"
@@ -776,9 +792,24 @@ async def run_step_executor(
     if step_result_data:
         tool_context.state["_step_result"] = None  # State has no pop(); reset instead
         result = StepExecutorResult.model_validate(step_result_data)
+        allowed_artifact_roots = _artifact_allowed_roots(
+            step_workspace, suggested_skills, output_dir,
+        )
         result, missing_artifacts = _verify_step_result_artifacts(
             result,
-            allowed_roots=_artifact_allowed_roots(step_workspace, suggested_skills, output_dir),
+            allowed_roots=allowed_artifact_roots,
+            additional_artifacts=[*artifact_paths, *plot_paths],
+        )
+        # Do not persist or return unverified tool outputs.  In particular,
+        # this prevents a stale plot_path from being rendered as a broken image
+        # while the step is being replanned.
+        artifact_paths, _ = _split_verified_artifacts(
+            artifact_paths,
+            allowed_roots=allowed_artifact_roots,
+        )
+        plot_paths, _ = _split_verified_artifacts(
+            plot_paths,
+            allowed_roots=allowed_artifact_roots,
         )
         await asyncio.to_thread(
             graph.log_node_complete,

src/matcreator/config.py

@@ -81,6 +81,9 @@
     "compute.deepmd_machine": "BOHRIUM_DEEPMD_MACHINE",
     "compute.deepmd_model_path": "DEEPMD_MODEL_PATH",
     "benchmark.server_url":   "MAT_BENCH_SERVER_URL",
+    "benchmark.token":        "MAT_BENCH_TOKEN",
+    "benchmark.question_bank_root": "MAT_BENCH_QUESTION_BANK_ROOT",
+    "runtime.execution_timeout_seconds": "MATCREATOR_EXEC_TIMEOUT_SECONDS",
     "skills.module_root":     "MATCREATOR_MODULE_SKILLS_ROOT",
     "knowledge.memorization_frequency": "MATCREATOR_MEMORIZATION_FREQUENCY",
     "knowledge.review_frequency": "MATCREATOR_REVIEW_FREQUENCY",
@@ -89,7 +92,12 @@
 ENV_TO_YAML: dict[str, str] = {v: k for k, v in YAML_TO_ENV.items()}
 
 # Fields whose values should be masked when displayed.
-SENSITIVE_YAML_KEYS = frozenset({"llm.api_key", "bohrium.password", "bohrium.access_key"})
+SENSITIVE_YAML_KEYS = frozenset({
+    "llm.api_key",
+    "bohrium.password",
+    "bohrium.access_key",
+    "benchmark.token",
+})
 _USER_ENV_KEY_RE = re.compile(r"[A-Za-z_][A-Za-z0-9_]*")
 _PROTECTED_USER_ENV_KEYS = frozenset({
     "HOME",

src/matcreator/constants.py

@@ -41,6 +41,7 @@
 _bohrium_cfg = get_bohrium_config()
 _compute_cfg = get_compute_config()
 _knowledge_cfg = load_config().get("knowledge", {})
+_runtime_cfg = load_config().get("runtime", {})
 
 _yaml_to_env: dict[str, str | None] = {
     "LLM_MODEL":            _llm_cfg.get("model"),
@@ -61,11 +62,12 @@
     "DEEPMD_MODEL_PATH":    _compute_cfg.get("deepmd_model_path"),
     "MATCREATOR_MEMORIZATION_FREQUENCY": _knowledge_cfg.get("memorization_frequency"),
     "MATCREATOR_REVIEW_FREQUENCY": _knowledge_cfg.get("review_frequency"),
+    "MATCREATOR_EXEC_TIMEOUT_SECONDS": _runtime_cfg.get("execution_timeout_seconds"),
 }
 
 for _env_key, _yaml_val in _yaml_to_env.items():
     if _yaml_val and (_CONFIG_OVERRIDES_PRE_ENV or _env_key not in _pre_env):
-        os.environ[_env_key] = _yaml_val
+        os.environ[_env_key] = str(_yaml_val)
 
 for _env_key, _yaml_val in get_env_overrides().items():
     if not _USER_ENV_KEY_RE.fullmatch(_env_key) or _env_key in _PROTECTED_USER_ENV_KEYS:

src/matcreator/tools/workspace_tools.py

@@ -8,7 +8,7 @@
 -----------------
 * ``write_workspace_file``  only writes inside the workspace root (path
   traversal is rejected).
-* ``run_python`` / ``run_bash`` execute in a subprocess with a 60-second
+* ``run_python`` / ``run_bash`` execute in a subprocess with a configurable
   timeout.  The agent must always present the code/command to the user and
   obtain explicit approval before calling these tools (enforced by the
   thinking_agent instruction).
@@ -274,14 +274,26 @@ def create_skill(
 # Script execution tools
 # ---------------------------------------------------------------------------
 
-_EXEC_TIMEOUT = 3600  # seconds
+_DEFAULT_EXEC_TIMEOUT_SECONDS = 3600
+
+
+def _execution_timeout_seconds() -> int:
+    """Return the user-configured subprocess timeout, falling back safely."""
+    raw_value = os.environ.get("MATCREATOR_EXEC_TIMEOUT_SECONDS", "").strip()
+    if not raw_value:
+        return _DEFAULT_EXEC_TIMEOUT_SECONDS
+    try:
+        timeout = int(raw_value)
+    except ValueError:
+        return _DEFAULT_EXEC_TIMEOUT_SECONDS
+    return timeout if timeout > 0 else _DEFAULT_EXEC_TIMEOUT_SECONDS
 
 
 async def run_python(code: str, tool_context: ToolContext) -> str:
     """Execute a Python code snippet and return its stdout/stderr.
 
     IMPORTANT: Only call this tool after the user has explicitly approved the
-    code to be run.  The code runs in a subprocess with a 60-second timeout.
+    code to be run. The timeout is configured in Settings (default: 3600s).
 
     Args:
         code: Python source code to execute.
@@ -301,11 +313,12 @@ async def run_python(code: str, tool_context: ToolContext) -> str:
         cwd=cwd,
     )
     try:
-        stdout, stderr = await asyncio.wait_for(proc.communicate(), timeout=_EXEC_TIMEOUT)
+        timeout = _execution_timeout_seconds()
+        stdout, stderr = await asyncio.wait_for(proc.communicate(), timeout=timeout)
     except asyncio.TimeoutError:
         proc.kill()
         stdout, stderr = await proc.communicate()
-        output = f"[TimeoutExpired after {_EXEC_TIMEOUT}s]\n" + stdout.decode("utf-8", errors="replace") + stderr.decode("utf-8", errors="replace")
+        output = f"[TimeoutExpired after {timeout}s]\n" + stdout.decode("utf-8", errors="replace") + stderr.decode("utf-8", errors="replace")
     except asyncio.CancelledError:
         proc.kill()
         await proc.communicate()
@@ -321,7 +334,7 @@ async def run_bash(script: str, tool_context: ToolContext) -> str:
     """Execute a bash script snippet and return its stdout/stderr.
 
     IMPORTANT: Only call this tool after the user has explicitly approved the
-    script to be run.  The script runs in a subprocess with a 60-second timeout.
+    script to be run. The timeout is configured in Settings (default: 3600s).
 
     Args:
         script: Bash script content to execute.
@@ -341,11 +354,12 @@ async def run_bash(script: str, tool_context: ToolContext) -> str:
         cwd=cwd,
     )
     try:
-        stdout, stderr = await asyncio.wait_for(proc.communicate(), timeout=_EXEC_TIMEOUT)
+        timeout = _execution_timeout_seconds()
+        stdout, stderr = await asyncio.wait_for(proc.communicate(), timeout=timeout)
     except asyncio.TimeoutError:
         proc.kill()
         stdout, stderr = await proc.communicate()
-        output = f"[TimeoutExpired after {_EXEC_TIMEOUT}s]\n" + stdout.decode("utf-8", errors="replace") + stderr.decode("utf-8", errors="replace")
+        output = f"[TimeoutExpired after {timeout}s]\n" + stdout.decode("utf-8", errors="replace") + stderr.decode("utf-8", errors="replace")
     except asyncio.CancelledError:
         proc.kill()
         await proc.communicate()
@@ -379,11 +393,12 @@ async def run_python_file(relative_path: str) -> str:
         stderr=asyncio.subprocess.PIPE,
     )
     try:
-        stdout, stderr = await asyncio.wait_for(proc.communicate(), timeout=_EXEC_TIMEOUT)
+        timeout = _execution_timeout_seconds()
+        stdout, stderr = await asyncio.wait_for(proc.communicate(), timeout=timeout)
     except asyncio.TimeoutError:
         proc.kill()
         stdout, stderr = await proc.communicate()
-        output = f"[TimeoutExpired after {_EXEC_TIMEOUT}s]\n" + stdout.decode("utf-8", errors="replace") + stderr.decode("utf-8", errors="replace")
+        output = f"[TimeoutExpired after {timeout}s]\n" + stdout.decode("utf-8", errors="replace") + stderr.decode("utf-8", errors="replace")
     except asyncio.CancelledError:
         proc.kill()
         await proc.communicate()
@@ -471,11 +486,12 @@ async def run_skill_script(
         env=env,
     )
     try:
-        stdout, stderr = await asyncio.wait_for(proc.communicate(), timeout=_EXEC_TIMEOUT)
+        timeout = _execution_timeout_seconds()
+        stdout, stderr = await asyncio.wait_for(proc.communicate(), timeout=timeout)
     except asyncio.TimeoutError:
         proc.kill()
         stdout, stderr = await proc.communicate()
-        output = f"[TimeoutExpired after {_EXEC_TIMEOUT}s]\n" + stdout.decode("utf-8", errors="replace") + stderr.decode("utf-8", errors="replace")
+        output = f"[TimeoutExpired after {timeout}s]\n" + stdout.decode("utf-8", errors="replace") + stderr.decode("utf-8", errors="replace")
     except asyncio.CancelledError:
         proc.kill()
         await proc.communicate()

tests/test_matcreator_constants.py

@@ -77,6 +77,8 @@ def test_server_mode_config_overrides_container_defaults(monkeypatch, tmp_path:
     (matcreator_home / "config.yaml").write_text(
         "llm:\n"
         "  model: openai/user-model\n"
+        "runtime:\n"
+        "  execution_timeout_seconds: 7200\n"
         "env:\n"
         "  MP_API_KEY: user-mp-key\n",
         encoding="utf-8",
@@ -93,6 +95,7 @@ def test_server_mode_config_overrides_container_defaults(monkeypatch, tmp_path:
 
     assert constants.LLM_MODEL == "openai/user-model"
     assert constants.os.environ["LLM_MODEL"] == "openai/user-model"
+    assert constants.os.environ["MATCREATOR_EXEC_TIMEOUT_SECONDS"] == "7200"
     assert constants.os.environ["MP_API_KEY"] == "user-mp-key"
 
 

tests/test_step_executor_runner.py

@@ -98,6 +98,25 @@ def test_success_with_missing_artifact_requires_replanning(tmp_path, caplog):
     assert "claimed artifact path(s)" in caplog.text
 
 
+def test_success_with_missing_tool_plot_requires_replanning(tmp_path):
+    result = StepExecutorResult(
+        status="success",
+        key_results="Generated a plot.",
+        concise_summary="Generated a plot.",
+    )
+    missing_plot = tmp_path / "missing-plot.png"
+
+    verified, missing_artifacts = _verify_step_result_artifacts(
+        result,
+        allowed_roots=[tmp_path],
+        additional_artifacts=[str(missing_plot)],
+    )
+
+    assert verified.status == "needs_replanning"
+    assert missing_artifacts == [str(missing_plot)]
+    assert str(missing_plot) in (verified.replan_reason or "")
+
+
 def test_success_accepts_existing_file_and_directory_artifacts(tmp_path):
     file_artifact = tmp_path / "result.txt"
     file_artifact.write_text("ok", encoding="utf-8")

tests/test_web_session_access.py

@@ -8,6 +8,9 @@
 from pathlib import Path
 from types import SimpleNamespace
 
+import pytest
+from fastapi import HTTPException
+
 
 class _FakeImage:
     def __init__(self, image_id: str):
@@ -287,6 +290,48 @@ def test_local_adk_restart_env_uses_frontend_config(monkeypatch, tmp_path):
     assert "FRONTEND_SET_FLAG: visible-locally" in config_text
 
 
+def test_server_runtime_and_benchmark_settings_are_persisted(monkeypatch, tmp_path):
+    control_home = tmp_path / "control-plane" / ".matcreator"
+    control_home.mkdir(parents=True)
+    web_main = _load_web_main_server(monkeypatch, control_home, tmp_path / "server-data")
+
+    body = SimpleNamespace(values={
+        "MATCREATOR_EXEC_TIMEOUT_SECONDS": "7200",
+        "MATCREATOR_MEMORIZATION_FREQUENCY": "0",
+        "MATCREATOR_REVIEW_FREQUENCY": "25",
+        "MAT_BENCH_SERVER_URL": "http://127.0.0.1:8080/bench",
+        "MAT_BENCH_TOKEN": "benchmark-secret",
+        "MAT_BENCH_QUESTION_BANK_ROOT": "/tmp/question-bank",
+    })
+    asyncio.run(web_main.update_env_config(body, user_id="alice"))
+
+    config = web_main._load_config_for_user("alice")
+    assert config["runtime"]["execution_timeout_seconds"] == "7200"
+    assert config["knowledge"] == {
+        "memorization_frequency": "0",
+        "review_frequency": "25",
+    }
+    assert config["benchmark"] == {
+        "server_url": "http://127.0.0.1:8080/bench",
+        "token": "benchmark-secret",
+        "question_bank_root": "/tmp/question-bank",
+    }
+
+    response = asyncio.run(web_main.get_env_config(user_id="alice"))
+    values = json.loads(response.body)
+    assert values["MAT_BENCH_TOKEN"] == "***"
+
+
+def test_runtime_timeout_must_be_positive(monkeypatch, tmp_path):
+    control_home = tmp_path / "control-plane" / ".matcreator"
+    control_home.mkdir(parents=True)
+    web_main = _load_web_main_server(monkeypatch, control_home, tmp_path / "server-data")
+
+    body = SimpleNamespace(values={"MATCREATOR_EXEC_TIMEOUT_SECONDS": "0"})
+    with pytest.raises(HTTPException, match="positive integer"):
+        asyncio.run(web_main.update_env_config(body, user_id="alice"))
+
+
 def test_local_mode_lists_sessions_regardless_of_requested_user(monkeypatch, tmp_path):
     web_main = _load_web_main(monkeypatch)
     db_path = tmp_path / "session.db"
@@ -312,4 +357,4 @@ def test_local_mode_reads_session_detail_regardless_of_requested_user(monkeypatc
 
     assert payload["userId"] == "legacy-display-name"
     assert payload["state"] == {"answer": 42}
-    assert payload["events"] == [{"event": "persisted"}]
+    assert payload["events"] == [{"event": "persisted"}]

tests/test_workspace_tools.py

@@ -11,6 +11,20 @@ def __init__(self):
         self.state = {}
 
 
+def test_execution_timeout_uses_valid_user_setting(monkeypatch):
+    monkeypatch.delenv("MATCREATOR_EXEC_TIMEOUT_SECONDS", raising=False)
+    assert workspace_tools._execution_timeout_seconds() == 3600
+
+    monkeypatch.setenv("MATCREATOR_EXEC_TIMEOUT_SECONDS", "7200")
+    assert workspace_tools._execution_timeout_seconds() == 7200
+
+
+def test_execution_timeout_rejects_invalid_user_setting(monkeypatch):
+    for value in ("0", "-10", "not-a-number"):
+        monkeypatch.setenv("MATCREATOR_EXEC_TIMEOUT_SECONDS", value)
+        assert workspace_tools._execution_timeout_seconds() == 3600
+
+
 def test_set_session_output_dir_sets_output_state_under_workspace(tmp_path, monkeypatch):
     monkeypatch.setenv("MATCLAW_WORKSPACE", str(tmp_path))
     tool_context = _FakeToolContext()
@@ -104,4 +118,4 @@ def test_set_session_workdir_rejects_paths_outside_workspace(tmp_path, monkeypat
     assert absolute_result["status"] == "error"
     assert traversal_result["status"] == "error"
     assert root_result["status"] == "error"
-    assert tool_context.state == {}
+    assert tool_context.state == {}