chore: add beenherebefore to AUTHOR_MAP

compute_next_run() ignored the last_run_at parameter for cron-type
schedules, always computing from _hermes_now() instead. This was
inconsistent with interval jobs which DO use last_run_at as the anchor.

After a crash or restart, cron jobs would compute next_run_at from
the arbitrary restart time rather than the actual last execution time.
While the stale detection in get_due_jobs() catches most cases, using
last_run_at as the croniter base eliminates edge cases and makes the
behavior consistent across schedule types.

Salvaged from #9014 (authored by @beenherebefore) onto current main.
The original PR branch was 2+ weeks stale and would have reverted
substantial unrelated work (jobs_file_lock, workdir/context_from/
enabled_toolsets, issue #16265 state=error recovery). Kept just the
7-line substantive fix and the regression test.

cron/jobs.py

@@ -319,7 +319,14 @@ def compute_next_run(schedule: Dict[str, Any], last_run_at: Optional[str] = None
                 schedule.get("expr"),
             )
             return None
-        cron = croniter(schedule["expr"], now)
+        # Use last_run_at as the croniter base when available, consistent
+        # with interval jobs.  This ensures that after a crash/restart,
+        # the next run is anchored to the actual last execution time
+        # rather than to an arbitrary restart time.
+        base_time = now
+        if last_run_at:
+            base_time = _ensure_aware(datetime.fromisoformat(last_run_at))
+        cron = croniter(schedule["expr"], base_time)
         next_run = cron.get_next(datetime)
         return next_run.isoformat()
 

cron/scheduler.py

@@ -1053,7 +1053,18 @@ def run_job(job: dict) -> tuple[bool, str, str, Optional[str]]:
         #
         # Uses the agent's built-in activity tracker (updated by
         # _touch_activity() on every tool call, API call, and stream delta).
-        _cron_timeout = float(os.getenv("HERMES_CRON_TIMEOUT", 600))
+        _raw_cron_timeout = os.getenv("HERMES_CRON_TIMEOUT", "").strip()
+        if _raw_cron_timeout:
+            try:
+                _cron_timeout = float(_raw_cron_timeout)
+            except (ValueError, TypeError):
+                logger.warning(
+                    "Invalid HERMES_CRON_TIMEOUT=%r; using default 600s",
+                    _raw_cron_timeout,
+                )
+                _cron_timeout = 600.0
+        else:
+            _cron_timeout = 600.0
         _cron_inactivity_limit = _cron_timeout if _cron_timeout > 0 else None
         _POLL_INTERVAL = 5.0
         _cron_pool = concurrent.futures.ThreadPoolExecutor(max_workers=1)

scripts/release.py

@@ -67,6 +67,8 @@ AUTHOR_MAP = {
     "beliefanx@gmail.com": "BeliefanX",
     "jefferson@heimdallstrategy.com": "Mind-Dragon",
     "steve.westerhouse@origami-analytics.com": "westers",
+    "yeyitech@users.noreply.github.com": "yeyitech",
+    "260878550+beenherebefore@users.noreply.github.com": "beenherebefore",
     "liuhao03@bilibili.com": "liuhao1024",
     "130918800+devorun@users.noreply.github.com": "devorun",
     "surat.s@itm.kmutnb.ac.th": "beesrsj2500",

tests/cron/test_compute_next_run_last_run_at.py

@@ -0,0 +1,87 @@
+"""Test that compute_next_run uses last_run_at for cron jobs.
+
+Regression test for: cron jobs computing next_run_at from _hermes_now()
+instead of from last_run_at, making them inconsistent with interval jobs.
+"""
+import pytest
+from datetime import datetime
+from zoneinfo import ZoneInfo
+
+pytest.importorskip("croniter")
+
+from cron.jobs import compute_next_run
+
+
+class TestCronComputeNextRunUsesLastRunAt:
+    """compute_next_run MUST use last_run_at as the croniter base for cron jobs,
+    consistent with how interval jobs work."""
+
+    def test_cron_uses_last_run_at_for_every_6h_schedule(self, monkeypatch):
+        """For a schedule like 'every 6 hours', the base time matters.
+        If last_run_at is Apr 6 14:10, next should be Apr 6 18:00.
+        If now is Apr 10 22:00, next should be Apr 11 00:00.
+        compute_next_run must use last_run_at, not now."""
+        morocco = ZoneInfo("Africa/Casablanca")
+
+        # Job last ran April 6 at 14:10
+        last_run = datetime(2026, 4, 6, 14, 10, 0, tzinfo=morocco)
+
+        # But now it's April 10 at 22:00 (e.g., gateway restarted)
+        now = datetime(2026, 4, 10, 22, 0, 0, tzinfo=morocco)
+        monkeypatch.setattr("cron.jobs._hermes_now", lambda: now)
+
+        schedule = {"kind": "cron", "expr": "0 */6 * * *"}  # every 6 hours
+
+        result = compute_next_run(schedule, last_run_at=last_run.isoformat())
+        assert result is not None
+        next_dt = datetime.fromisoformat(result)
+
+        # With last_run_at as base (Apr 6 14:10), next is Apr 6 18:00.
+        # With now as base (Apr 10 22:00), next is Apr 11 00:00.
+        # The fix should use last_run_at, returning Apr 6 18:00
+        # (stale detection in get_due_jobs() fast-forwards from there).
+        assert next_dt.date().isoformat() == "2026-04-06", (
+            f"Expected next run on Apr 6 (from last_run_at), got {next_dt}"
+        )
+        assert next_dt.hour == 18
+
+    def test_cron_without_last_run_at_uses_now(self, monkeypatch):
+        """When last_run_at is NOT provided, compute_next_run falls back to
+        _hermes_now() as the croniter base (existing behavior)."""
+        morocco = ZoneInfo("Africa/Casablanca")
+
+        now = datetime(2026, 4, 10, 22, 0, 0, tzinfo=morocco)
+        monkeypatch.setattr("cron.jobs._hermes_now", lambda: now)
+
+        schedule = {"kind": "cron", "expr": "0 */6 * * *"}
+
+        result = compute_next_run(schedule)
+        assert result is not None
+        next_dt = datetime.fromisoformat(result)
+
+        # Without last_run_at, should compute from now -> Apr 11 00:00
+        assert next_dt.date().isoformat() == "2026-04-11", (
+            f"Expected next run on Apr 11 (from now), got {next_dt}"
+        )
+        assert next_dt.hour == 0
+
+    def test_cron_weekly_consistent_with_interval(self, monkeypatch):
+        """Both cron and interval jobs should anchor to last_run_at when
+        provided, producing consistent behavior after a crash/restart."""
+        morocco = ZoneInfo("Africa/Casablanca")
+
+        last_run = datetime(2026, 4, 6, 14, 10, 0, tzinfo=morocco)
+        now = datetime(2026, 4, 10, 22, 0, 0, tzinfo=morocco)
+        monkeypatch.setattr("cron.jobs._hermes_now", lambda: now)
+
+        cron_schedule = {"kind": "cron", "expr": "0 14 * * 1"}
+        interval_schedule = {"kind": "interval", "minutes": 7 * 24 * 60}
+
+        cron_result = compute_next_run(cron_schedule, last_run_at=last_run.isoformat())
+        interval_result = compute_next_run(interval_schedule, last_run_at=last_run.isoformat())
+
+        # Both should be after last_run_at
+        cron_dt = datetime.fromisoformat(cron_result)
+        interval_dt = datetime.fromisoformat(interval_result)
+        assert cron_dt > last_run, f"Cron next {cron_dt} should be after last_run {last_run}"
+        assert interval_dt > last_run, f"Interval next {interval_dt} should be after last_run {last_run}"

tests/cron/test_cron_inactivity_timeout.py

@@ -169,10 +169,20 @@ class TestInactivityTimeout:
 
         assert result["final_response"] == "Done"
 
+    def _parse_cron_timeout(self, raw_value):
+        """Mirror the defensive parsing logic from cron/scheduler.py run_job()."""
+        if raw_value:
+            try:
+                return float(raw_value)
+            except (ValueError, TypeError):
+                return 600.0
+        return 600.0
+
     def test_timeout_env_var_parsing(self, monkeypatch):
         """HERMES_CRON_TIMEOUT env var is respected."""
         monkeypatch.setenv("HERMES_CRON_TIMEOUT", "1200")
-        _cron_timeout = float(os.getenv("HERMES_CRON_TIMEOUT", 600))
+        raw = os.getenv("HERMES_CRON_TIMEOUT", "").strip()
+        _cron_timeout = self._parse_cron_timeout(raw)
         assert _cron_timeout == 1200.0
 
         _cron_inactivity_limit = _cron_timeout if _cron_timeout > 0 else None
@@ -181,10 +191,27 @@ class TestInactivityTimeout:
     def test_timeout_zero_means_unlimited(self, monkeypatch):
         """HERMES_CRON_TIMEOUT=0 yields None (unlimited)."""
         monkeypatch.setenv("HERMES_CRON_TIMEOUT", "0")
-        _cron_timeout = float(os.getenv("HERMES_CRON_TIMEOUT", 600))
+        raw = os.getenv("HERMES_CRON_TIMEOUT", "").strip()
+        _cron_timeout = self._parse_cron_timeout(raw)
         _cron_inactivity_limit = _cron_timeout if _cron_timeout > 0 else None
         assert _cron_inactivity_limit is None
 
+    def test_timeout_invalid_value_falls_back_to_default(self, monkeypatch):
+        """HERMES_CRON_TIMEOUT=abc should fall back to 600s, not raise ValueError."""
+        monkeypatch.setenv("HERMES_CRON_TIMEOUT", "abc")
+        raw = os.getenv("HERMES_CRON_TIMEOUT", "").strip()
+        _cron_timeout = self._parse_cron_timeout(raw)
+        assert _cron_timeout == 600.0
+        _cron_inactivity_limit = _cron_timeout if _cron_timeout > 0 else None
+        assert _cron_inactivity_limit == 600.0
+
+    def test_timeout_empty_string_uses_default(self, monkeypatch):
+        """HERMES_CRON_TIMEOUT='' (empty) should use the 600s default."""
+        monkeypatch.setenv("HERMES_CRON_TIMEOUT", "")
+        raw = os.getenv("HERMES_CRON_TIMEOUT", "").strip()
+        _cron_timeout = self._parse_cron_timeout(raw)
+        assert _cron_timeout == 600.0
+
     def test_timeout_error_includes_diagnostics(self):
         """The TimeoutError message should include last activity info."""
         agent = SlowFakeAgent(

tests/tools/test_skill_manager_tool.py

@@ -566,3 +566,151 @@ class TestSecurityScanGate:
 
         with patch("hermes_cli.config.load_config", side_effect=RuntimeError("boom")):
             assert _guard_agent_created_enabled() is False
+
+
+# ---------------------------------------------------------------------------
+# External skills directories (skills.external_dirs) — mutations in place
+# ---------------------------------------------------------------------------
+
+
+@contextmanager
+def _two_roots(local_dir: Path, external_dir: Path):
+    """Patch the skill manager so local SKILLS_DIR = local_dir and
+    get_all_skills_dirs() returns [local_dir, external_dir] in order."""
+    with patch("tools.skill_manager_tool.SKILLS_DIR", local_dir), \
+         patch("agent.skill_utils.get_all_skills_dirs",
+               return_value=[local_dir, external_dir]):
+        yield
+
+
+def _write_external_skill(external_dir: Path, name: str = "ext-skill") -> Path:
+    skill_dir = external_dir / name
+    skill_dir.mkdir(parents=True)
+    (skill_dir / "SKILL.md").write_text(
+        f"---\nname: {name}\ndescription: An external skill.\n---\n\n"
+        "# External\n\nBody with OLD_MARKER here.\n"
+    )
+    return skill_dir
+
+
+class TestExternalSkillMutations:
+    """Verify skill_manage can patch/edit/write/remove/delete skills that live
+    under skills.external_dirs — in place, without duplicating to local.
+
+    Regression for issues #4759 and #4381: the read-only gate used to refuse
+    with 'Skill X is in an external directory and cannot be modified', which
+    caused agents to create duplicate copies in ~/.hermes/skills/ as a
+    workaround.
+    """
+
+    def test_patch_external_skill_writes_in_place(self, tmp_path):
+        local = tmp_path / "local"
+        external = tmp_path / "vault"
+        local.mkdir(); external.mkdir()
+        skill_dir = _write_external_skill(external)
+
+        with _two_roots(local, external):
+            result = _patch_skill("ext-skill", "OLD_MARKER", "NEW_MARKER")
+
+        assert result["success"] is True, result
+        assert "NEW_MARKER" in (skill_dir / "SKILL.md").read_text()
+        # No duplicate in local
+        assert not (local / "ext-skill").exists()
+
+    def test_edit_external_skill_writes_in_place(self, tmp_path):
+        local = tmp_path / "local"
+        external = tmp_path / "vault"
+        local.mkdir(); external.mkdir()
+        skill_dir = _write_external_skill(external)
+
+        new_content = (
+            "---\nname: ext-skill\ndescription: Rewritten.\n---\n\n"
+            "# Rewritten\n\nBrand new body.\n"
+        )
+        with _two_roots(local, external):
+            result = _edit_skill("ext-skill", new_content)
+
+        assert result["success"] is True, result
+        assert "Brand new body" in (skill_dir / "SKILL.md").read_text()
+        assert not (local / "ext-skill").exists()
+
+    def test_write_file_on_external_skill(self, tmp_path):
+        local = tmp_path / "local"
+        external = tmp_path / "vault"
+        local.mkdir(); external.mkdir()
+        skill_dir = _write_external_skill(external)
+
+        with _two_roots(local, external):
+            result = _write_file("ext-skill", "references/notes.md", "# Notes\n")
+
+        assert result["success"] is True, result
+        assert (skill_dir / "references" / "notes.md").read_text() == "# Notes\n"
+        assert not (local / "ext-skill").exists()
+
+    def test_remove_file_on_external_skill(self, tmp_path):
+        local = tmp_path / "local"
+        external = tmp_path / "vault"
+        local.mkdir(); external.mkdir()
+        skill_dir = _write_external_skill(external)
+        (skill_dir / "references").mkdir()
+        (skill_dir / "references" / "notes.md").write_text("# Notes\n")
+
+        with _two_roots(local, external):
+            result = _remove_file("ext-skill", "references/notes.md")
+
+        assert result["success"] is True, result
+        assert not (skill_dir / "references" / "notes.md").exists()
+
+    def test_delete_external_skill_removes_skill_not_root(self, tmp_path):
+        local = tmp_path / "local"
+        external = tmp_path / "vault"
+        local.mkdir(); external.mkdir()
+        skill_dir = _write_external_skill(external)
+
+        with _two_roots(local, external):
+            result = _delete_skill("ext-skill")
+
+        assert result["success"] is True, result
+        assert not skill_dir.exists()
+        # The external root must NOT be rmdir'd, even when empty after deletion
+        assert external.exists() and external.is_dir()
+
+    def test_delete_external_skill_cleans_empty_category(self, tmp_path):
+        """When a skill lives under external/<category>/<name>, deleting the
+        last skill in the category should rmdir the empty category dir but
+        stop at the external root."""
+        local = tmp_path / "local"
+        external = tmp_path / "vault"
+        local.mkdir(); external.mkdir()
+        cat_dir = external / "team"
+        cat_dir.mkdir()
+        skill_dir = cat_dir / "ext-skill"
+        skill_dir.mkdir()
+        (skill_dir / "SKILL.md").write_text(
+            "---\nname: ext-skill\ndescription: An external skill.\n---\n\n"
+            "# External\n\nBody.\n"
+        )
+
+        with _two_roots(local, external):
+            result = _delete_skill("ext-skill")
+
+        assert result["success"] is True, result
+        assert not skill_dir.exists()
+        assert not cat_dir.exists()  # empty category cleaned up
+        assert external.exists()     # but never the external root
+
+    def test_create_still_writes_to_local_root(self, tmp_path):
+        """Creating a new skill always lands in local SKILLS_DIR, never
+        external_dirs — create is unchanged by this PR."""
+        local = tmp_path / "local"
+        external = tmp_path / "vault"
+        local.mkdir(); external.mkdir()
+
+        with _two_roots(local, external):
+            result = _create_skill("fresh-skill", VALID_SKILL_CONTENT.replace(
+                "name: test-skill", "name: fresh-skill"))
+
+        assert result["success"] is True, result
+        assert (local / "fresh-skill" / "SKILL.md").exists()
+        assert not (external / "fresh-skill").exists()
+

tools/skill_manager_tool.py

@@ -109,16 +109,28 @@ MAX_NAME_LENGTH = 64
 MAX_DESCRIPTION_LENGTH = 1024
 
 
-def _is_local_skill(skill_path: Path) -> bool:
-    """Check if a skill path is within the local SKILLS_DIR.
-
-    Skills found in external_dirs are read-only from the agent's perspective.
+def _containing_skills_root(skill_path: Path) -> Path:
+    """Return the skills root directory (local or external_dirs entry) that
+    contains ``skill_path``.  Falls back to the local ``SKILLS_DIR`` if no
+    match is found (defensive — callers should have located the skill via
+    ``_find_skill`` first).
     """
+    from agent.skill_utils import get_all_skills_dirs
+
     try:
-        skill_path.resolve().relative_to(SKILLS_DIR.resolve())
-        return True
-    except ValueError:
-        return False
+        resolved = skill_path.resolve()
+    except OSError:
+        resolved = skill_path
+
+    for root in get_all_skills_dirs():
+        try:
+            resolved.relative_to(root.resolve())
+            return root
+        except (ValueError, OSError):
+            continue
+    return SKILLS_DIR
+
+
 MAX_SKILL_CONTENT_CHARS = 100_000   # ~36k tokens at 2.75 chars/token
 MAX_SKILL_FILE_BYTES = 1_048_576    # 1 MiB per supporting file
 
@@ -397,9 +409,6 @@ def _edit_skill(name: str, content: str) -> Dict[str, Any]:
     if not existing:
         return {"success": False, "error": f"Skill '{name}' not found. Use skills_list() to see available skills."}
 
-    if not _is_local_skill(existing["path"]):
-        return {"success": False, "error": f"Skill '{name}' is in an external directory and cannot be modified. Copy it to your local skills directory first."}
-
     skill_md = existing["path"] / "SKILL.md"
     # Back up original content for rollback
     original_content = skill_md.read_text(encoding="utf-8") if skill_md.exists() else None
@@ -440,9 +449,6 @@ def _patch_skill(
     if not existing:
         return {"success": False, "error": f"Skill '{name}' not found."}
 
-    if not _is_local_skill(existing["path"]):
-        return {"success": False, "error": f"Skill '{name}' is in an external directory and cannot be modified. Copy it to your local skills directory first."}
-
     skill_dir = existing["path"]
 
     if file_path:
@@ -522,15 +528,13 @@ def _delete_skill(name: str) -> Dict[str, Any]:
     if not existing:
         return {"success": False, "error": f"Skill '{name}' not found."}
 
-    if not _is_local_skill(existing["path"]):
-        return {"success": False, "error": f"Skill '{name}' is in an external directory and cannot be deleted."}
-
     skill_dir = existing["path"]
+    skills_root = _containing_skills_root(skill_dir)
     shutil.rmtree(skill_dir)
 
-    # Clean up empty category directories (don't remove SKILLS_DIR itself)
+    # Clean up empty category directories (don't remove the skills root itself)
     parent = skill_dir.parent
-    if parent != SKILLS_DIR and parent.exists() and not any(parent.iterdir()):
+    if parent != skills_root and parent.exists() and not any(parent.iterdir()):
         parent.rmdir()
 
     return {
@@ -567,9 +571,6 @@ def _write_file(name: str, file_path: str, file_content: str) -> Dict[str, Any]:
     if not existing:
         return {"success": False, "error": f"Skill '{name}' not found. Create it first with action='create'."}
 
-    if not _is_local_skill(existing["path"]):
-        return {"success": False, "error": f"Skill '{name}' is in an external directory and cannot be modified. Copy it to your local skills directory first."}
-
     target, err = _resolve_skill_target(existing["path"], file_path)
     if err:
         return {"success": False, "error": err}
@@ -604,9 +605,6 @@ def _remove_file(name: str, file_path: str) -> Dict[str, Any]:
     if not existing:
         return {"success": False, "error": f"Skill '{name}' not found."}
 
-    if not _is_local_skill(existing["path"]):
-        return {"success": False, "error": f"Skill '{name}' is in an external directory and cannot be modified."}
-
     skill_dir = existing["path"]
 
     target, err = _resolve_skill_target(skill_dir, file_path)