From f57cb2e4823231936e3f91102c90214374158be0 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 29 Jul 2026 09:00:37 -0700 Subject: [PATCH] refactor: use utils atomic writes in cron/skill_manager --- cron/jobs.py | 23 ++++++----------------- tools/skill_manager_tool.py | 24 +++++++----------------- 2 files changed, 13 insertions(+), 34 deletions(-) diff --git a/cron/jobs.py b/cron/jobs.py index 9b7f2f4befb4b..7766a87aaf2f4 100644 --- a/cron/jobs.py +++ b/cron/jobs.py @@ -39,7 +39,7 @@ from typing import Optional, Dict, List, Any, Set, Tuple, Union logger = logging.getLogger(__name__) from hermes_time import now as _hermes_now -from utils import atomic_replace +from utils import atomic_replace, atomic_write_text try: from croniter import croniter @@ -824,24 +824,13 @@ def compute_next_run(schedule: Dict[str, Any], last_run_at: Optional[str] = None def _atomic_write_epoch(path: Path) -> None: """Atomically write the current epoch time to ``path``. - Uses the same tmpfile + ``atomic_replace`` pattern as ``save_jobs`` so a - concurrent reader in another process (``hermes cron status``) never sees a - torn/truncated file. Best-effort: failures are swallowed by callers. + Delegates to :func:`utils.atomic_write_text` (tmpfile + fsync + + ``atomic_replace``, same pattern as ``save_jobs``) so a concurrent reader + in another process (``hermes cron status``) never sees a torn/truncated + file. Best-effort: failures are swallowed by callers. """ ensure_dirs() - fd, tmp_path = tempfile.mkstemp(dir=str(path.parent), suffix=".tmp", prefix=".hb_") - try: - with os.fdopen(fd, "w", encoding="utf-8") as f: - f.write(str(time.time())) - f.flush() - os.fsync(f.fileno()) - atomic_replace(tmp_path, path) - except BaseException: - try: - os.unlink(tmp_path) - except OSError: - pass - raise + atomic_write_text(path, str(time.time()), tmp_prefix=".hb_") def _atomic_write_counter(path: Path, value: int) -> None: diff --git a/tools/skill_manager_tool.py b/tools/skill_manager_tool.py index 3284cb9233a55..c201ebaffd6e0 100644 --- a/tools/skill_manager_tool.py +++ b/tools/skill_manager_tool.py @@ -814,16 +814,6 @@ def _resolve_skill_target(skill_dir: Path, file_path: str) -> Tuple[Optional[Pat return target, None -def _atomic_write_text(file_path: Path, content: str, encoding: str = "utf-8") -> None: - """Atomically write text content to a file. - - Thin wrapper around :func:`utils.atomic_write_text` so that every - destructive file rewrite in the codebase shares one implementation. - """ - atomic_write_text(file_path, content, encoding=encoding, - tmp_prefix=f".{file_path.name}.tmp.") - - # ============================================================================= # Core actions # ============================================================================= @@ -874,7 +864,7 @@ def _create_skill(name: str, content: str, category: str = None) -> Dict[str, An # Write SKILL.md atomically skill_md = skill_dir / "SKILL.md" - _atomic_write_text(skill_md, content) + atomic_write_text(skill_md, content) # Security scan — roll back on block scan_error = _security_scan_skill(skill_dir) @@ -935,13 +925,13 @@ def _edit_skill(name: str, content: str) -> Dict[str, Any]: # Back up original content for rollback original_content = skill_md.read_text(encoding="utf-8") if skill_md.exists() else None - _atomic_write_text(skill_md, content) + atomic_write_text(skill_md, content) # Security scan — roll back on block scan_error = _security_scan_skill(existing["path"]) if scan_error: if original_content is not None: - _atomic_write_text(skill_md, original_content) + atomic_write_text(skill_md, original_content) return {"success": False, "error": scan_error} # Extract description from new content for verbose notifications @@ -1057,12 +1047,12 @@ def _patch_skill( } original_content = content # for rollback - _atomic_write_text(target, new_content) + atomic_write_text(target, new_content) # Security scan — roll back on block scan_error = _security_scan_skill(skill_dir) if scan_error: - _atomic_write_text(target, original_content) + atomic_write_text(target, original_content) return {"success": False, "error": scan_error} result = { @@ -1226,13 +1216,13 @@ def _write_file(name: str, file_path: str, file_content: str) -> Dict[str, Any]: target.parent.mkdir(parents=True, exist_ok=True) # Back up for rollback original_content = target.read_text(encoding="utf-8") if target.exists() else None - _atomic_write_text(target, file_content) + atomic_write_text(target, file_content) # Security scan — roll back on block scan_error = _security_scan_skill(existing["path"]) if scan_error: if original_content is not None: - _atomic_write_text(target, original_content) + atomic_write_text(target, original_content) else: target.unlink(missing_ok=True) return {"success": False, "error": scan_error}