feat: per-skill file lock for cross-process write safety

[jasstionzyf]

Problem

Under concurrent multi-user load (gateway with many active sessions), multiple processes/threads within the same profile can write to skill files simultaneously with no cross-process synchronization:

Writer	Path	Scenario
skill_manage tool	tools/skill_manager_tool.py	Agent-initiated or background review daemon
Curator	agent/curator.py	Auto-archive/delete stale skills
CLI	hermes skills install/update/uninstall	Manual operations
Background review	daemon thread → forked AIAgent → skill_manage	Triggers every ~10 turns per session; multiple sessions may fire concurrently

None of these paths hold any cross-process lock. A background review in session A writing skill_manage(action='patch', ...) races with curator archiving the same skill in another thread, or two background reviews from different sessions patching the same SKILL.md simultaneously.

Additionally, the terminal tool's cwd state is shared across sessions (tools/terminal_tool.py line 1076: "the terminal env is shared across sessions").

Proposed Solution: Per-Skill File Lock

Use fcntl.flock() on the skill directory FD (not a separate lock file), scoped per skill name so writes to different skills are fully concurrent:

write "python-tips"  →  flock(skills/python-tips/)    ← only this skill locked
write "git-workflow" →  flock(skills/git-workflow/)   ← no wait for python-tips
create new skill     →  flock(skills/)                ← parent-dir lock to prevent
                                                         duplicate names

Why directory FD instead of a lock file

• No extra file to create / clean up / manage
• fcntl.flock() on a directory fd works identically to file fd on Linux/macOS
• Kernel-managed: released on process death, no stale lock problem
• The directory is the natural serialization boundary — one skill = one dir

API

# agent/skill_lock.py
from contextlib import contextmanager

@contextmanager
def skill_write_lock(skill_name: str, timeout: float = 30):
    """Acquire a per-skill write lock, cross-process safe."""
    skill_dir = Path(get_hermes_home()) / "skills" / skill_name
    target = skill_dir if skill_dir.is_dir() else skill_dir.parent
    fd = os.open(str(target), os.O_RDONLY)
    try:
        _flock_wait(fd, timeout)
        yield
    finally:
        fcntl.flock(fd, fcntl.LOCK_UN)
        os.close(fd)

Integration points (3 files)

1. tools/skill_manager_tool.py — wrap all 6 actions (create/edit/patch/delete/write_file/remove_file)
2. agent/curator.py — wrap archive/delete operations
3. hermes_cli/skills.py — wrap install/update/uninstall

Delete handling

Before deleting a skill directory, os.rename to a temp location first, then shutil.rmtree — releases lock before slow deletion.

Why reads don't need the lock

Skill writes already use atomic write temp + rename internally. Readers always see either the old file or the new file, never a partial write. Curator moves are handled by the delete rename pattern above.

Windows fallback

Use msvcrt.locking() + PID-based lock file with stale detection.

Scope

• New file: agent/skill_lock.py (~60 lines)
• Modified: 3 existing files
• Tests: tests/agent/test_skill_lock.py

Notes

The same profile-level sharing problem also applies to terminal cwd state — tracked separately in tools/terminal_tool.py's ongoing "cwd rearchitecture" (step 1 in progress). This issue focuses on skills only.

If maintainers are interested, I'm willing to submit a PR implementing this plan.

[jasstionzyf]

If maintainers are open to this approach, I'm happy to submit a PR implementing the per-skill file lock as described above. Happy to adjust the design based on feedback.

[nankingjing]

I've read through the design carefully and I'm ready to implement this once a maintainer signals approval (noting the needs-decision label). A few observations from studying the existing codebase that may be useful for the decision:

Existing precedent: PR #68179 (merged, agent/learning_mutations.py) established the _file_lock pattern for memory writes — MemoryStore._file_lock(path) as a context manager wrapping atomic temp-file+rename. A SkillFileLock should follow the same shape for consistency.

Design notes on the proposed skill_write_lock:

1. Directory-FD flock is sound on Linux/macOS — kernel-managed, released on process death, no stale-lock cleanup. Agreed this is cleaner than a separate lockfile.

2. Delete-via-rename (os.rename to temp → shutil.rmtree) is the right call — it releases the lock before slow recursive deletion and prevents a racing creator from re-instantiating a half-deleted directory.

3. Integration points — the issue names tools/skill_manager_tool.py, agent/curator.py, hermes_cli/skills.py. A quick check of the repo layout suggests the actual paths may differ slightly (e.g., curator at agent/curator.py vs hermes_cli/curator.py); I'll confirm exact file locations against current main before writing code.

4. Windows fallback (msvcrt.locking + PID-based lockfile with stale detection) adds ~40 lines. Worth confirming maintainers want Windows support in scope, or whether POSIX-only (with a documented no-op fallback) is acceptable for the initial PR.

5. Scope check — the issue explicitly excludes the terminal-cwd sharing problem. I'll keep the PR strictly skills-only per "one PR one concern."

Happy to submit the PR as soon as a maintainer gives the go-ahead, and glad to adjust any of the above based on feedback.