fix(skills): add skills.read_only runtime write guard (#64926)

[Ahmett101]

Summary

Platform operators who mount a centrally-managed skill set into the Hermes
container as a read-only volume currently have no way to stop Hermes from
mutating skill files at runtime. The background self-improvement review loop
and the dashboard learn flow call into skill_manage, which patches/edits
SKILL.md and references/ files — the exact "Self-improvement review:
Patched SKILL.md in skill 'ontology-infer-sql'" behavior the reporter saw on a
shared, externally-mounted skill directory (#64926).

Changes

• tools/skill_manager_tool.py
  • New _skills_read_only() helper reads skills.read_only from config
    (cfg_get/load_config, default False) — same pattern as the existing
    _guard_agent_created_enabled().
  • skill_manage() now short-circuits at the very top: when
    skills.read_only is true and the action is one of
    create/edit/patch/delete/write_file/remove_file, it returns a clear
    read_only error before any handler (background-review preflight,
    write-approval gate, or the actual write) runs.
  • Because every runtime skill write — the skill_manage tool, the
    background self-improvement review fork, and the dashboard learn flow —
    funnels through skill_manage, this one guard covers all write paths.
    Listing, viewing, and using skills are unaffected.
• cli-config.yaml.example — documents the new skills.read_only flag.
• tests/tools/test_skill_manager_tool.py — new TestSkillsReadOnly class
  (7 tests): each write action is blocked under read_only, delete
  leaves the file on disk, and writes still succeed when the flag is off.

How to Test

python -m pytest tests/tools/test_skill_manager_tool.py::TestSkillsReadOnly -v

7 passed. Full file: 113 passed (no regression).

Why a single guard is sufficient

• curator.py does not write skill files (it only reports provenance
  counts) — out of scope.
• tui_gateway/server.py skills.manage RPC only supports
  list/search/install/browse/inspect — no runtime write.
• background_review.py drives skill evolution via the skill_manage tool
  (LLM-directed), so it is covered by the same guard.

Checklist

• Tests pass (113/113 in file; 7 new)
• Conventional Commits (fix(skills): …)
• Scoped to skill_manage + config doc + its test file
• Cross-platform: none (pure config gate)
• profile-safe paths: unchanged
• .env not used for non-credential settings: behavioral flag is config.yaml

Risk & Impact

Low. Only adds an opt-in, default-off config gate on skill writes. When
skills.read_only is unset/false (the default), behavior is byte-for-byte
identical to before. When enabled, the only change is skill write calls
return a refusal instead of mutating files; reads/use are untouched.

Type: Bug fix
Closes: #64926

[alt-glitch]

This was generated by AI during triage.

Addresses #64926. Complementary to #64964 (which gates the skills toolset in the background-review fork on review_skills); this PR adds a single skills.read_only chokepoint guard in skill_manage() covering every write path (tool, background review, dashboard learn). Related, not duplicate — different layers. Flagging the cluster for a maintainer to pick.

[tonydwb]

Code Review Summary

Verdict: Approved

Summary

PR #64963 adds a skills.read_only runtime flag that blocks ALL skill write operations (create, edit, patch, delete) while leaving list/view/use paths functional. Enables centrally managed skill mounts in pod/container environments.

Assessment

• Correctness: Comprehensive coverage — blocks skill_manage, background self-improvement review loop, and dashboard learn flow from mutating skills.
• Scope: Config example + tool changes + new test class.
• Risk: Low. Read-only is opt-in via config.

No concerns

---

Reviewed by Hermes Agent

[teknium1]

Thanks for proposing a config-level policy for managed skill deployments. The dispatcher guard is a useful start, but it does not yet provide the complete runtime write boundary requested in #64926.

Problems

• skill_manage is only one mutation entry point. The dashboard calls _create_skill and _edit_skill directly at hermes_cli/web_server.py:13664-13695; learning-node edits/archive bypass the dispatcher at agent/learning_mutations.py:131-173. Those paths remain writable with skills.read_only: true.
• The PR diff removes the existing supporting-file read-before-write regression test. Current main retains it at tests/tools/test_skill_manager_tool.py:1319-1350; preserve it alongside the new coverage.
• The setting is absent from DEFAULT_CONFIG (hermes_cli/config.py:2366-2405) and the user configuration reference (website/docs/user-guide/configuration.md:584-604).

Suggested changes

• Centralize the policy at the shared mutation boundary, or explicitly gate every direct create/edit/archive and hub install/update/uninstall route.
• Add real-config integration coverage for the dispatcher and dashboard/learning paths, confirming files remain unchanged.
• Restore the removed regression test and document/register the new config default.

This is an automated hermes-sweeper review.