diff --git a/AGENTS.md b/AGENTS.md
index 8aedf80..5f7930c 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -299,6 +299,8 @@ Required: `SKILL_NAME`, `SKILL_TRIGGERS`, `SKILL_DESCRIPTION`. Optional: `SKILL_
### Adding a new skill
Drop a `.py` file in `~/.codec/skills/` (user) or `skills/` (built-in). Restart `codec-dashboard` and the main `open-codec` process. Hot-reload is not currently supported.
+**Two folders (2026-10-07, docs/USER-SKILLS-DIR-DESIGN.md).** Every registry reads the built-in folder (`codec_config.SKILLS_DIR`, the repo's `skills/` unless `config.json:skills_dir` says otherwise) and then the user folder (`codec_config.USER_SKILLS_DIR`: `config.json:user_skills_dir`, default `~/.codec/skills`). A built-in name always wins; a user file is never trusted by the manifest, so every load runs the AST gate; files starting with `_` and sub-folders are skipped. `/api/skill/approve` (review-and-approve, Settings > Learning) writes approved skills to the user folder (0700 folder, 0600 file), never the repo. The test suite points `CODEC_USER_SKILLS_DIR` at a temp folder (tests/conftest.py).
+
## 5. Memory contract
Single SQLite database at `~/.codec/memory.db`. Wrapper: `codec_memory.CodecMemory`.
diff --git a/codec_agents.py b/codec_agents.py
index 36560e5..b0e6448 100644
--- a/codec_agents.py
+++ b/codec_agents.py
@@ -39,9 +39,10 @@
# ── CONFIG ──
CONFIG_PATH = os.path.expanduser("~/.codec/config.json")
try:
- from codec_config import SKILLS_DIR
+ from codec_config import SKILLS_DIR, USER_SKILLS_DIR
except ImportError:
SKILLS_DIR = os.path.expanduser("~/.codec/skills")
+ USER_SKILLS_DIR = None
DB_PATH = os.path.expanduser("~/.codec/memory.db")
def _cfg():
@@ -408,7 +409,7 @@ def _shell_execute(cmd: str) -> str:
from codec_skill_registry import SkillRegistry
-_agents_registry = SkillRegistry(SKILLS_DIR)
+_agents_registry = SkillRegistry(SKILLS_DIR, USER_SKILLS_DIR)
def _make_lazy_fn(registry: "SkillRegistry", skill_name: str):
diff --git a/codec_autopilot.py b/codec_autopilot.py
index 6874f9e..e547806 100644
--- a/codec_autopilot.py
+++ b/codec_autopilot.py
@@ -39,7 +39,7 @@
sys.path.insert(0, str(_REPO / "skills"))
from codec_skill_registry import SkillRegistry
-from codec_config import SKILLS_DIR
+from codec_config import SKILLS_DIR, USER_SKILLS_DIR
from codec_audit import audit
log = logging.getLogger("codec-autopilot")
@@ -116,7 +116,7 @@ def _day_matches(days_spec: str, weekday: int) -> bool:
def _speak(text: str):
"""Best-effort TTS via tts_say skill; falls back to macOS `say`."""
try:
- registry = SkillRegistry(SKILLS_DIR)
+ registry = SkillRegistry(SKILLS_DIR, USER_SKILLS_DIR)
registry.scan()
mod = registry.load("tts_say")
if mod and hasattr(mod, "run"):
@@ -225,7 +225,7 @@ def main():
codec_lifecycle.install_handlers(
lambda: log.info("CODEC Autopilot graceful shutdown"),
name="codec-autopilot")
- registry = SkillRegistry(SKILLS_DIR)
+ registry = SkillRegistry(SKILLS_DIR, USER_SKILLS_DIR)
registry.scan()
while True:
try:
diff --git a/codec_config.py b/codec_config.py
index 07b4413..fd35ea2 100644
--- a/codec_config.py
+++ b/codec_config.py
@@ -361,6 +361,10 @@ def get_telegram_bot_token() -> str:
# Skills load from the repo directly — single source of truth
_REPO_DIR = os.path.dirname(os.path.abspath(__file__))
SKILLS_DIR = cfg.get("skills_dir", os.path.join(_REPO_DIR, "skills"))
+# The owner's skills (approved reviews, marketplace installs): read after the built-ins,
+# never trusted by the manifest. CODEC_USER_SKILLS_DIR lets the test suite point it away.
+USER_SKILLS_DIR = os.path.expanduser(os.environ.get("CODEC_USER_SKILLS_DIR")
+ or cfg.get("user_skills_dir") or "~/.codec/skills")
AUDIT_LOG = os.path.expanduser("~/.codec/audit.log")
# Features
diff --git a/codec_dashboard.html b/codec_dashboard.html
index 8c792cc..059390d 100644
--- a/codec_dashboard.html
+++ b/codec_dashboard.html
@@ -1032,7 +1032,7 @@
Proposed skills
-
Drafted by the self-improvement run when CODEC lacked a skill. Review shows the code; Approve and install writes it to your skills folder after the safety check, and CODEC loads it after the next restart.
+
Drafted by the self-improvement run when CODEC lacked a skill. Review shows the code; Approve and install writes it to your skills folder (~/.codec/skills) after the safety check, and CODEC loads it after the next restart.
@@ -3002,7 +3002,7 @@
macOS permissions
LRN.review = rid; LRN.name = p.name;
_learnDlg();
document.getElementById('learnTitle').textContent = 'Review ' + p.name;
- document.getElementById('learnWhat').textContent = 'Approve and install writes ' + fname + ' to your skills folder after the safety check; CODEC loads it after the next restart.' +
+ document.getElementById('learnWhat').textContent = 'Approve and install writes ' + fname + ' to your skills folder (~/.codec/skills) after the safety check; CODEC loads it after the next restart.' +
(p.truncated ? ' The code is longer than shown here.' : '');
document.getElementById('learnCode').textContent = p.code;
document.getElementById('learnErr').textContent = '';
diff --git a/codec_dispatch.py b/codec_dispatch.py
index 9bc1a3a..6d382b7 100644
--- a/codec_dispatch.py
+++ b/codec_dispatch.py
@@ -11,7 +11,7 @@
# Audit emits route through the unified log_event adapter (real, not no-op)
# per docs/PHASE1-STEP1-DESIGN.md.
from codec_audit import log_event
-from codec_config import SKILLS_DIR
+from codec_config import SKILLS_DIR, USER_SKILLS_DIR
from codec_hooks import HookVeto, run_with_hooks
from codec_skill_registry import SkillRegistry
from codec_skill_switches import is_off as _skill_off, off_message as _off_message
@@ -19,7 +19,7 @@
log = logging.getLogger('codec')
# Global registry instance shared across codec.py
-registry = SkillRegistry(SKILLS_DIR)
+registry = SkillRegistry(SKILLS_DIR, USER_SKILLS_DIR)
def load_skills():
diff --git a/codec_mcp.py b/codec_mcp.py
index bc7a88f..3f7d1c2 100644
--- a/codec_mcp.py
+++ b/codec_mcp.py
@@ -61,7 +61,7 @@ def _validate_mcp_input(tool_name: str, task: str, context: str = "") -> str | N
if _REPO_DIR not in sys.path:
sys.path.insert(0, _REPO_DIR)
-from codec_config import MCP_DEFAULT_ALLOW, MCP_ALLOWED_TOOLS, MCP_BLOCKED_TOOLS, SKILLS_DIR
+from codec_config import MCP_DEFAULT_ALLOW, MCP_ALLOWED_TOOLS, MCP_BLOCKED_TOOLS, SKILLS_DIR, USER_SKILLS_DIR
if SKILLS_DIR not in sys.path:
sys.path.insert(0, SKILLS_DIR)
from codec_skill_registry import SkillRegistry
@@ -78,7 +78,7 @@ def __iter__(self):
# Global registry for MCP skill tools
-_mcp_registry = SkillRegistry(SKILLS_DIR)
+_mcp_registry = SkillRegistry(SKILLS_DIR, USER_SKILLS_DIR)
def build_mcp(auth=None):
diff --git a/codec_self_improve.py b/codec_self_improve.py
index c1c3434..5b9224f 100644
--- a/codec_self_improve.py
+++ b/codec_self_improve.py
@@ -81,7 +81,7 @@
_REPO = Path(__file__).resolve().parent
sys.path.insert(0, str(_REPO))
-from codec_config import QWEN_BASE_URL, QWEN_MODEL, SKILLS_DIR, is_dangerous_skill_code
+from codec_config import QWEN_BASE_URL, QWEN_MODEL, SKILLS_DIR, USER_SKILLS_DIR, is_dangerous_skill_code
try:
from codec_audit import log_event as _si_log_event
@@ -106,7 +106,8 @@ def _si_log_event(*a, **kw): # type: ignore[no-redef]
def _existing_skill_names() -> set[str]:
out = set()
- for f in Path(SKILLS_DIR).glob("*.py"):
+ # Built-ins and the owner's folder (approved skills): never propose what is already there.
+ for f in [*Path(SKILLS_DIR).glob("*.py"), *Path(USER_SKILLS_DIR).glob("*.py")]:
if f.name.startswith("_") or f.name == "codec.py":
continue
out.add(f.stem)
diff --git a/codec_skill_registry.py b/codec_skill_registry.py
index b4b1e72..df47d52 100644
--- a/codec_skill_registry.py
+++ b/codec_skill_registry.py
@@ -105,8 +105,15 @@ class SkillRegistry:
mod = registry.load("weather") # first call imports; subsequent calls return cache
"""
- def __init__(self, skills_dir: str):
+ def __init__(self, skills_dir: str, user_dir: Optional[str] = None):
self.skills_dir = skills_dir
+ # The owner's folder (~/.codec/skills): read after the built-ins, a built-in
+ # name always wins, and its files are never trusted by the manifest (every
+ # load runs the AST gate). None, or the built-in folder itself, means off.
+ same = user_dir and os.path.realpath(user_dir) == os.path.realpath(skills_dir)
+ self.user_dir = None if (not user_dir or same) else user_dir
+ # names whose file is in user_dir (set at scan() time)
+ self._user_names: Set[str] = set()
# name -> metadata dict (always populated after scan)
self._meta: Dict[str, Dict[str, Any]] = {}
# name -> file path
@@ -129,9 +136,11 @@ def scan(self) -> int:
Returns the number of skills discovered."""
self._meta.clear()
self._paths.clear()
+ self._user_names.clear()
# Do NOT clear _modules — keep any already-loaded modules cached
- if not os.path.isdir(self.skills_dir):
+ has_user = bool(self.user_dir) and os.path.isdir(self.user_dir)
+ if not os.path.isdir(self.skills_dir) and not has_user:
return 0
# Reload the trusted-skill manifest on every scan so manifest edits
@@ -140,21 +149,42 @@ def scan(self) -> int:
# Reload user trigger overrides too (A-4) — same refresh-on-scan policy.
self._custom_triggers = self._load_custom_triggers()
- for fname in sorted(os.listdir(self.skills_dir)):
- if not fname.endswith(".py") or fname.startswith("_"):
- continue
- filepath = os.path.join(self.skills_dir, fname)
- meta = _extract_metadata(filepath)
- if meta is None:
- continue
- name = meta.get("SKILL_NAME", fname[:-3])
- self._meta[name] = meta
- self._paths[name] = filepath
+ if os.path.isdir(self.skills_dir):
+ for fname in sorted(os.listdir(self.skills_dir)):
+ if not fname.endswith(".py") or fname.startswith("_"):
+ continue
+ filepath = os.path.join(self.skills_dir, fname)
+ meta = _extract_metadata(filepath)
+ if meta is None:
+ continue
+ name = meta.get("SKILL_NAME", fname[:-3])
+ self._meta[name] = meta
+ self._paths[name] = filepath
+
+ if has_user:
+ for fname in sorted(os.listdir(self.user_dir)):
+ if not fname.endswith(".py") or fname.startswith("_"):
+ continue
+ filepath = os.path.join(self.user_dir, fname)
+ if not os.path.isfile(filepath):
+ continue
+ meta = _extract_metadata(filepath)
+ if meta is None:
+ continue
+ name = meta.get("SKILL_NAME", fname[:-3])
+ if name in self._meta and name not in self._user_names:
+ log.warning("User skill %s skipped: the built-in skill %r has that name", fname, name)
+ continue
+ self._meta[name] = meta
+ self._paths[name] = filepath
+ self._user_names.add(name)
log.info(
"Skill registry: %d skills discovered (metadata only), %d in trusted manifest",
len(self._meta), len(self._trusted_hashes),
)
+ if self._user_names:
+ log.info("Skill registry: %d of them from the user folder %s", len(self._user_names), self.user_dir)
return len(self._meta)
def _load_trusted_manifest(self) -> Set[str]:
@@ -283,7 +313,9 @@ def load(self, name: str) -> Optional[Any]:
return None
file_hash = hashlib.sha256(raw).hexdigest()
- trusted = file_hash in self._trusted_hashes
+ # A file in the user folder is never trusted by the manifest, even if its
+ # bytes equal a pinned built-in's: it always goes through the AST gate.
+ trusted = file_hash in self._trusted_hashes and name not in self._user_names
if not trusted:
# Decode for AST check; fail-safe on UnicodeDecodeError.
diff --git a/codec_slash_commands.py b/codec_slash_commands.py
index 6680253..e532989 100644
--- a/codec_slash_commands.py
+++ b/codec_slash_commands.py
@@ -162,11 +162,11 @@ def _cmd_skills(args: list[str]) -> str:
# Lazy import to avoid pulling skill registry at module load
try:
from codec_skill_registry import SkillRegistry
- from codec_config import SKILLS_DIR
+ from codec_config import SKILLS_DIR, USER_SKILLS_DIR
except Exception as e:
return f"Error: skill registry unavailable: {e}"
- reg = SkillRegistry(SKILLS_DIR)
+ reg = SkillRegistry(SKILLS_DIR, USER_SKILLS_DIR)
reg.scan()
# P3.6: the deny list the Skills page writes and every path enforces (codec_skill_switches).
import codec_skill_switches as switches
diff --git a/codec_voice.py b/codec_voice.py
index 7bc8fbc..35d5c62 100644
--- a/codec_voice.py
+++ b/codec_voice.py
@@ -198,9 +198,10 @@ def _clear_voice_session_marker() -> None:
KOKORO_VOICE = "am_adam"
KOKORO_SPEED = 1.15 # config:tts_speed overrides (normal mode; flash has its own)
try:
- from codec_config import SKILLS_DIR
+ from codec_config import SKILLS_DIR, USER_SKILLS_DIR
except ImportError:
SKILLS_DIR = os.path.expanduser("~/.codec/skills")
+ USER_SKILLS_DIR = None
_CONFIG_PATH = os.path.expanduser("~/.codec/config.json")
try:
@@ -511,7 +512,7 @@ def _save_for_resume(self):
def _load_skills(self):
from codec_skill_registry import SkillRegistry
- self._skill_registry = SkillRegistry(SKILLS_DIR)
+ self._skill_registry = SkillRegistry(SKILLS_DIR, USER_SKILLS_DIR)
self._skill_registry.scan()
# Build a lightweight dict with triggers only (no module imports)
for name in self._skill_registry.names():
diff --git a/docs/USER-SKILLS-DIR-DESIGN.md b/docs/USER-SKILLS-DIR-DESIGN.md
new file mode 100644
index 0000000..46d2261
--- /dev/null
+++ b/docs/USER-SKILLS-DIR-DESIGN.md
@@ -0,0 +1,56 @@
+# User skills folder: approved skills go to ~/.codec/skills, and CODEC loads it
+
+Asked by Mickael on 2026-10-07 ("point approve at ~/.codec/skills") after the Mac merge pass found that approved
+skills land in the repo's `skills/` folder (docs/known-issues.md).
+
+## Why
+
+- `POST /api/skill/approve` (the review-and-approve flow, and Settings > Learning's Approve and install) writes to
+ `codec_config.SKILLS_DIR`. That is `config.json:skills_dir`, or the repo's own `skills/` folder when the key is not
+ set, as on this Mac. An approved skill becomes an untracked file in the live git tree, next to the hash-pinned
+ built-ins.
+- Every process builds its skill registry with `SkillRegistry(SKILLS_DIR)`, and the registry scans that one folder.
+ So `~/.codec/skills`, which AGENTS.md calls the user folder and which the marketplace installs into, is never
+ loaded on this Mac. Writing approved skills there alone would make them dead files.
+
+## What
+
+1. `codec_config.USER_SKILLS_DIR`: `config.json:user_skills_dir`, default `~/.codec/skills`.
+2. `SkillRegistry(skills_dir, user_dir=None)`: when `user_dir` is given and differs from `skills_dir`, `scan()` reads
+ the built-in folder first, then the user folder. A user file whose skill name is already a built-in is skipped
+ (logged), so a user file can never shadow a built-in. User files are never trusted by the manifest: every load
+ runs the AST gate (`is_dangerous_skill_code`), as unmanifested files do today. Files starting with `_` and
+ sub-folders are not scanned, as today.
+3. The registries in `codec_dispatch`, `codec_mcp`, `codec_voice`, `codec_agents`, `codec_autopilot` and
+ `codec_slash_commands` pass `USER_SKILLS_DIR`.
+4. `/api/skill/approve` writes to `USER_SKILLS_DIR` (folder 0700, file 0600). The built-in-name refusal and the
+ write-time AST gate stay as they are. The audit line `skill_approved` gains `dir: "user"`.
+5. Settings > Learning's "installed" check and `codec_self_improve`'s existing-name scan look in both folders.
+6. AGENTS.md §4 and the Learning page text say where approved skills go.
+
+No change to the manifest, the built-in folder, the AST gate, the security lists or MCP exposure: a user skill's
+`SKILL_MCP_EXPOSE` is honoured as before, and over HTTP the blocked and ask-first lists apply.
+
+## Migration (needs Mickael's OK: it is his folder)
+
+`~/.codec/skills` holds 6 old files (March-April 2026) that have never been loaded on this Mac. Five fail the AST
+gate. `lucy.py` passes, so it would start running. Before the first restart they move to
+`~/.codec/skills/_dormant-2026-10-07/` (not scanned; nothing deleted). Any of them can be moved back by hand.
+
+## Deploy
+
+Restart `codec-dashboard` and `open-codec`. Until Mickael restarts `codec-mcp-http`, `codec-agent-runner` and
+`codec-autopilot`, those keep scanning the repo folder only, so an approved skill is not yet reachable from Claude
+over MCP, project agents or autopilot.
+
+## Test plan
+
+`tests/test_user_skills_dir.py`: the registry finds built-in and user skills; a built-in wins a name clash; a user
+file is never trusted by hash and a dangerous one is refused at load; `_` files and sub-folders are skipped; approve
+writes to the user folder with 0600 and still refuses a built-in name; the Learning installed flag and the
+self-improve name scan see user skills. Full suite with an empty HOME (CI has no ~/.codec).
+
+## Rollback
+
+Revert the PR. Skills approved meanwhile stay in `~/.codec/skills` and are no longer loaded; move them to the repo
+folder by hand if needed.
diff --git a/docs/known-issues.md b/docs/known-issues.md
index c2dacb3..fba080f 100644
--- a/docs/known-issues.md
+++ b/docs/known-issues.md
@@ -443,7 +443,7 @@ one waiting for approval or paused, where it is the time since it started. Fix:
"started N min ago" unless the project is running. Found in the Mac merge pass.
Revisit: the next item that touches the Activity board.
-## Approved skills are written into the repo's `skills/` folder (2026-10-07)
+## Approved skills are written into the repo's `skills/` folder (2026-10-07) — FIXED (user skills folder, docs/USER-SKILLS-DIR-DESIGN.md: approve writes to ~/.codec/skills and every registry reads it)
`POST /api/skill/approve` (the review-and-approve flow, and Settings > Learning's
Approve and install since P3.12) writes to `routes._shared._get_skills_dir()`,
diff --git a/routes/learning.py b/routes/learning.py
index 15db9f7..e4945c2 100644
--- a/routes/learning.py
+++ b/routes/learning.py
@@ -64,6 +64,11 @@ def _installed() -> set:
names |= {f[:-3] for f in os.listdir(_get_skills_dir()) if f.endswith(".py")}
except OSError:
pass
+ try: # approved skills go to the user folder; one approved before a restart is not in the registry yet
+ from codec_config import USER_SKILLS_DIR
+ names |= {f[:-3] for f in os.listdir(USER_SKILLS_DIR) if f.endswith(".py") and not f.startswith("_")}
+ except OSError:
+ pass
return names
diff --git a/routes/skills.py b/routes/skills.py
index d2a070b..6e11086 100644
--- a/routes/skills.py
+++ b/routes/skills.py
@@ -187,6 +187,14 @@ async def skill_review(request: Request):
}
+def _approved_skills_dir() -> str:
+ """Where an approved skill is written: the owner's folder (codec_config.USER_SKILLS_DIR,
+ ~/.codec/skills by default), never the repo's built-in folder. The registry reads it
+ after the built-ins (docs/USER-SKILLS-DIR-DESIGN.md)."""
+ from codec_config import USER_SKILLS_DIR
+ return USER_SKILLS_DIR
+
+
@router.post("/api/skill/approve")
async def skill_approve(request: Request):
"""Approve a pending skill review -- writes to disk and removes from pending."""
@@ -233,11 +241,14 @@ async def skill_approve(request: Request):
dangerous, reason = is_dangerous_skill_code(code)
if dangerous:
return JSONResponse({"error": f"Blocked: {reason}"}, status_code=400)
- skill_dir = _get_skills_dir()
- os.makedirs(skill_dir, exist_ok=True)
+ skill_dir = _approved_skills_dir()
+ os.makedirs(skill_dir, mode=0o700, exist_ok=True)
path = os.path.join(skill_dir, filename)
- with open(path, "w") as f:
+ tmp = path + ".tmp" # not a .py, so a scan never reads a half-written file
+ fd = os.open(tmp, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
+ with os.fdopen(fd, "w", encoding="utf-8") as f:
f.write(code)
+ os.replace(tmp, path)
# Only now that the skill is on disk do we clear the staged review (from both
# disk and the in-memory cache) so a crash mid-write leaves it re-approvable.
_delete_review(review_id)
@@ -247,7 +258,7 @@ async def skill_approve(request: Request):
"skill_approved", source="codec-routes-skills",
message=f"approved skill {filename} ({len(code)} bytes) → {path}",
level="info", outcome="ok",
- extra={"filename": filename, "review_id": review_id, "size": len(code)},
+ extra={"filename": filename, "review_id": review_id, "size": len(code), "dir": "user"},
)
except Exception:
pass
diff --git a/tests/conftest.py b/tests/conftest.py
index 4aa329b..dd63be3 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -35,6 +35,12 @@
# this variable when the module is imported.
os.environ.setdefault("CODEC_DASHBOARD_EXTRA_HOSTS", "testserver")
+# The owner's skills folder (~/.codec/skills) is read by every skill registry
+# and written by /api/skill/approve. Point it at an empty temp folder before
+# anything imports codec_config, so the suite neither loads nor writes the
+# owner's approved skills (tests that need user skills use their own tmp dirs).
+os.environ.setdefault("CODEC_USER_SKILLS_DIR", tempfile.mkdtemp(prefix="codec-test-user-skills-"))
+
def _isolate_audit_log() -> None:
"""Point the audit log at a throwaway file for the whole test session.
diff --git a/tests/test_learning.py b/tests/test_learning.py
index b8e3116..697c043 100644
--- a/tests/test_learning.py
+++ b/tests/test_learning.py
@@ -159,6 +159,7 @@ def test_review_then_approve_go_through_the_existing_gates(tmp_path, monkeypatch
skills_dir = tmp_path / "skills"
monkeypatch.setattr(rs, "_reviews_dir", lambda: str(tmp_path / "reviews"))
monkeypatch.setattr(rs, "_get_skills_dir", lambda: str(skills_dir))
+ monkeypatch.setattr(rs, "_approved_skills_dir", lambda: str(skills_dir))
monkeypatch.setattr(rs, "_pending_skills", {})
app = FastAPI()
app.include_router(rs.router)
diff --git a/tests/test_reaudit_low.py b/tests/test_reaudit_low.py
index cf8907a..07cba4e 100644
--- a/tests/test_reaudit_low.py
+++ b/tests/test_reaudit_low.py
@@ -40,6 +40,7 @@ def test_ast_gate_still_allows_safe_code():
# ── N20: skill_approve must FAIL CLOSED when the pinned-builtin manifest can't be read ──
def test_skill_approve_fails_closed_when_manifest_unreadable(tmp_path, monkeypatch):
monkeypatch.setattr(sr, "_get_skills_dir", lambda: str(tmp_path))
+ monkeypatch.setattr(sr, "_approved_skills_dir", lambda: str(tmp_path))
monkeypatch.setattr(sr, "_pinned_builtin_names", lambda: None) # simulate read failure
rid = "rev_failclosed"
sr._pending_skills[rid] = {"code": _BENIGN, "filename": "my_helper.py"}
diff --git a/tests/test_skill_overwrite.py b/tests/test_skill_overwrite.py
index f8433d1..c1d0371 100644
--- a/tests/test_skill_overwrite.py
+++ b/tests/test_skill_overwrite.py
@@ -34,6 +34,7 @@ def _approve(payload):
def test_skill_approve_refuses_pinned_builtin_name(tmp_path, monkeypatch):
monkeypatch.setattr(skills_routes, "_get_skills_dir", lambda: str(tmp_path))
+ monkeypatch.setattr(skills_routes, "_approved_skills_dir", lambda: str(tmp_path))
rid = "rev_pinned"
skills_routes._pending_skills[rid] = {"code": _BENIGN, "filename": "calculator.py"}
resp = _approve({"review_id": rid})
@@ -44,6 +45,7 @@ def test_skill_approve_refuses_pinned_builtin_name(tmp_path, monkeypatch):
def test_skill_approve_allows_non_pinned_name(tmp_path, monkeypatch):
monkeypatch.setattr(skills_routes, "_get_skills_dir", lambda: str(tmp_path))
+ monkeypatch.setattr(skills_routes, "_approved_skills_dir", lambda: str(tmp_path))
rid = "rev_ok"
skills_routes._pending_skills[rid] = {"code": _BENIGN, "filename": "my_unique_helper.py"}
resp = _approve({"review_id": rid})
diff --git a/tests/test_skill_reviews_persistence.py b/tests/test_skill_reviews_persistence.py
index e600f5e..9ac2cca 100644
--- a/tests/test_skill_reviews_persistence.py
+++ b/tests/test_skill_reviews_persistence.py
@@ -43,6 +43,7 @@ def client(tmp_path, monkeypatch):
reviews_dir = tmp_path / "skill_reviews"
skills_dir.mkdir()
monkeypatch.setattr(skills_routes, "_get_skills_dir", lambda: str(skills_dir))
+ monkeypatch.setattr(skills_routes, "_approved_skills_dir", lambda: str(skills_dir))
monkeypatch.setattr(skills_routes, "_reviews_dir", lambda: str(reviews_dir))
skills_routes._pending_skills.clear()
app = FastAPI()
diff --git a/tests/test_skill_routes.py b/tests/test_skill_routes.py
index 95aa8cd..a27a436 100644
--- a/tests/test_skill_routes.py
+++ b/tests/test_skill_routes.py
@@ -137,6 +137,7 @@ def test_skill_approve_writes_only_after_review(tmp_path, monkeypatch):
skills_dir = tmp_path / "skills"
skills_dir.mkdir()
monkeypatch.setattr(skills_routes, "_get_skills_dir", lambda: str(skills_dir))
+ monkeypatch.setattr(skills_routes, "_approved_skills_dir", lambda: str(skills_dir))
# Isolate the staged-review store to a tmp dir so the review→approve round
# trip doesn't touch the real ~/.codec/skill_reviews/.
reviews_dir = tmp_path / "skill_reviews"
diff --git a/tests/test_skills.py b/tests/test_skills.py
index 356337d..b303b78 100644
--- a/tests/test_skills.py
+++ b/tests/test_skills.py
@@ -4,8 +4,12 @@
import os
import sys
-SKILLS_DIR = os.path.expanduser("~/.codec/skills")
+# The user skills folder. conftest points CODEC_USER_SKILLS_DIR at an empty temp folder, so the
+# suite never imports (runs module code of) the owner's approved skills. The smoke tests below
+# import built-ins from the repo's skills/ folder.
+SKILLS_DIR = os.environ.get("CODEC_USER_SKILLS_DIR") or os.path.expanduser("~/.codec/skills")
sys.path.insert(0, SKILLS_DIR)
+sys.path.insert(0, os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "skills"))
def get_skill_files():
diff --git a/tests/test_user_skills_dir.py b/tests/test_user_skills_dir.py
new file mode 100644
index 0000000..d7b4693
--- /dev/null
+++ b/tests/test_user_skills_dir.py
@@ -0,0 +1,147 @@
+"""User skills folder (docs/USER-SKILLS-DIR-DESIGN.md).
+
+Every skill registry reads the built-in folder, then the owner's folder
+(~/.codec/skills): a built-in name always wins, a user file is never trusted
+by the manifest, and /api/skill/approve writes approved skills there. Nothing
+here touches ~/.codec: tmp folders only.
+"""
+from __future__ import annotations
+
+import asyncio
+import hashlib
+import json
+import os
+import stat
+from pathlib import Path
+
+import pytest
+
+REPO = Path(__file__).resolve().parent.parent
+
+SAFE = 'SKILL_TRIGGERS = ["say {word}"]\nSKILL_DESCRIPTION = "x"\n\ndef run(task, app="", ctx=""):\n return "said {word}"\n'
+DANGER = ('import subprocess\nSKILL_TRIGGERS = ["run it"]\nSKILL_DESCRIPTION = "x"\n\n'
+ 'def run(task, app="", ctx=""):\n return subprocess.run(["true"]).returncode\n')
+
+
+def _skill(folder: Path, fname: str, src: str) -> Path:
+ folder.mkdir(parents=True, exist_ok=True)
+ p = folder / fname
+ p.write_text(src, encoding="utf-8")
+ return p
+
+
+@pytest.fixture
+def dirs(tmp_path):
+ return tmp_path / "builtin", tmp_path / "user"
+
+
+def test_the_registry_reads_the_builtins_then_the_user_folder(dirs):
+ from codec_skill_registry import SkillRegistry
+ builtin, user = dirs
+ _skill(builtin, "alpha.py", SAFE.format(word="builtin"))
+ _skill(user, "beta.py", SAFE.format(word="beta"))
+ _skill(user, "alpha.py", SAFE.format(word="shadow")) # same name as a built-in
+ reg = SkillRegistry(str(builtin), str(user))
+ assert reg.scan() == 2 and sorted(reg.names()) == ["alpha", "beta"]
+ assert reg.load("alpha").run("") == "said builtin", "a built-in name always wins"
+ assert reg.load("beta").run("") == "said beta"
+
+
+def test_a_user_file_is_never_trusted_by_the_manifest(dirs):
+ from codec_skill_registry import SkillRegistry
+ builtin, user = dirs
+ _skill(builtin, "danger_tool.py", DANGER)
+ (builtin / ".manifest.json").write_text(json.dumps(
+ {"skills": {"danger_tool.py": hashlib.sha256(DANGER.encode()).hexdigest()}}))
+ _skill(user, "copycat.py", DANGER) # the same bytes, under another name
+ reg = SkillRegistry(str(builtin), str(user))
+ reg.scan()
+ assert reg.load("danger_tool") is not None, "the pinned built-in still loads"
+ assert reg.load("copycat") is None, "the user copy goes through the AST gate and is refused"
+
+
+def test_the_ast_gate_decides_for_user_files(dirs):
+ from codec_skill_registry import SkillRegistry
+ builtin, user = dirs
+ builtin.mkdir()
+ _skill(user, "ok_skill.py", SAFE.format(word="ok"))
+ _skill(user, "bad_skill.py", DANGER)
+ reg = SkillRegistry(str(builtin), str(user))
+ reg.scan()
+ assert reg.load("ok_skill").run("") == "said ok"
+ assert reg.load("bad_skill") is None
+
+
+def test_underscore_files_and_subfolders_are_not_read(dirs):
+ from codec_skill_registry import SkillRegistry
+ builtin, user = dirs
+ builtin.mkdir()
+ _skill(user, "_private.py", SAFE.format(word="p"))
+ _skill(user / "_dormant-2026-10-07", "old.py", SAFE.format(word="old"))
+ _skill(user / "sub", "nested.py", SAFE.format(word="n"))
+ reg = SkillRegistry(str(builtin), str(user))
+ assert reg.scan() == 0 and reg.names() == []
+
+
+def test_the_user_folder_is_off_when_it_is_the_builtin_folder_or_missing(dirs):
+ from codec_skill_registry import SkillRegistry
+ builtin, user = dirs
+ _skill(builtin, "alpha.py", SAFE.format(word="a"))
+ assert SkillRegistry(str(builtin), str(builtin)).user_dir is None
+ reg = SkillRegistry(str(builtin), str(user)) # user folder does not exist
+ assert reg.scan() == 1 and reg.names() == ["alpha"]
+
+
+class _Req:
+ def __init__(self, payload):
+ self._p = payload
+
+ async def json(self):
+ return self._p
+
+
+def test_approve_writes_to_the_user_folder_owner_only(tmp_path, monkeypatch):
+ import codec_config
+ import routes.skills as rs
+ user = tmp_path / "user_skills"
+ monkeypatch.setattr(codec_config, "USER_SKILLS_DIR", str(user))
+ monkeypatch.setattr(rs, "_reviews_dir", lambda: str(tmp_path / "reviews"))
+ code = 'SKILL_NAME = "moon"\nSKILL_DESCRIPTION = "x"\nSKILL_TRIGGERS = []\ndef run(task, app="", ctx=""):\n return "ok"\n'
+ rs._pending_skills["rev_user"] = {"code": code, "filename": "moon_helper.py"}
+ resp = asyncio.run(rs.skill_approve(_Req({"review_id": "rev_user"})))
+ assert getattr(resp, "status_code", 200) == 200, resp
+ path = user / "moon_helper.py"
+ assert path.read_text() == code and resp["path"] == str(path)
+ assert stat.S_IMODE(path.stat().st_mode) == 0o600 and stat.S_IMODE(user.stat().st_mode) == 0o700
+ assert not list(user.glob("*.tmp")), "no half-written file left behind"
+ assert not (REPO / "skills" / "moon_helper.py").exists(), "never the repo's built-in folder"
+ rs._pending_skills["rev_pinned"] = {"code": code, "filename": "calculator.py"}
+ resp = asyncio.run(rs.skill_approve(_Req({"review_id": "rev_pinned"})))
+ assert resp.status_code == 400 and not (user / "calculator.py").exists(), "a built-in name is still refused"
+
+
+def test_learning_and_self_improve_see_user_skills(tmp_path, monkeypatch):
+ import codec_config
+ import codec_self_improve
+ import routes.learning as rl
+ user = tmp_path / "user_skills"
+ _skill(user, "moon_helper.py", 'SKILL_NAME = "moon"\n' + SAFE.format(word="m"))
+ monkeypatch.setattr(codec_config, "USER_SKILLS_DIR", str(user))
+ monkeypatch.setattr(codec_self_improve, "USER_SKILLS_DIR", str(user))
+ assert "moon_helper" in rl._installed(), "an approved skill shows as installed before a restart"
+ names = codec_self_improve._existing_skill_names()
+ assert {"moon_helper", "moon"} <= names, "self-improve does not propose it again"
+
+
+def test_every_registry_gets_the_user_folder():
+ for mod in ("codec_dispatch.py", "codec_mcp.py", "codec_voice.py", "codec_agents.py", "codec_autopilot.py",
+ "codec_slash_commands.py"):
+ src = (REPO / mod).read_text(encoding="utf-8")
+ assert "SkillRegistry(SKILLS_DIR, USER_SKILLS_DIR)" in src and "SkillRegistry(SKILLS_DIR)" not in src, mod
+ import codec_config
+ import codec_dispatch
+ # The module-level registry keeps the folder it was built with (import order decides
+ # which test's value that is), so check what matters: it has a user folder, not the repo's.
+ assert codec_dispatch.registry.user_dir
+ assert os.path.realpath(codec_dispatch.registry.user_dir) != os.path.realpath(REPO / "skills")
+ assert codec_config.USER_SKILLS_DIR == os.environ["CODEC_USER_SKILLS_DIR"], "the suite never reads the owner's folder"