Fix owner-scoped skill updates (#1240)

Co-authored-by: ghreprimand <203024559+ghreprimand@users.noreply.github.com>

routes/skills_routes.py

@@ -469,7 +469,7 @@ async def _run_skill_test_job(key, name, md, task, url, model, headers, owner, s
         conf = {"pass": 0.95, "needs_work": 0.6, "fail": 0.4}.get(v)
         if conf is not None:
             try:
-                skills_manager.update_skill(name, {"confidence": conf})
+                skills_manager.update_skill(name, {"confidence": conf}, owner=owner)
             except Exception:
                 pass
     job["status"] = "done"
@@ -638,7 +638,7 @@ def _audit_finalize_status(skills_manager, name: str, owner, verdict: str,
     c = float(confidence or 0.0)
     status = "published" if (auto_publish and necessary and verdict == "pass" and c >= min_conf) else "draft"
     try:
-        skills_manager.update_skill(name, {"status": status})
+        skills_manager.update_skill(name, {"status": status}, owner=owner)
     except Exception:
         pass
     return status
@@ -662,7 +662,7 @@ def _apply_skill_md(skills_manager, name: str, md: str, owner) -> bool:
             "teacher_model": sk.teacher_model, "owner": sk.owner or owner,
             "when_to_use": sk.when_to_use, "procedure": sk.procedure,
             "pitfalls": sk.pitfalls, "verification": sk.verification, "body_extra": sk.body_extra,
-        }))
+        }, owner=owner))
     except Exception as e:
         logger.warning(f"Audit: could not save edited skill {name}: {e}")
         return False
@@ -762,7 +762,7 @@ async def _audit_one_skill(skills_manager, skill, url, model, headers,
     # earns a bit less; a skill that still fails is marked low.
     def _set_conf(c):
         try:
-            skills_manager.update_skill(name, {"confidence": c})
+            skills_manager.update_skill(name, {"confidence": c}, owner=owner)
         except Exception:
             pass
 
@@ -799,7 +799,7 @@ async def _audit_one_skill(skills_manager, skill, url, model, headers,
     if generic_reason or duplicate_of or (isinstance(nec, dict) and nec.get("necessary") is False):
         reason = generic_reason or (f"Lower-priority duplicate of {duplicate_of}" if duplicate_of else str((nec or {}).get("reason") or "Unnecessary skill"))
         try:
-            skills_manager.update_skill(name, {"status": "draft", "confidence": 0.35})
+            skills_manager.update_skill(name, {"status": "draft", "confidence": 0.35}, owner=owner)
             skills_manager.set_audit(name, "skipped", by_teacher=False, worker_model=model)
             if duplicate_of:
                 skills_manager.set_necessity(name, False, [duplicate_of], reason)
@@ -901,7 +901,7 @@ async def _audit_one_skill(skills_manager, skill, url, model, headers,
 
     # Still failing → demote to draft + low confidence + flag (do NOT delete).
     try:
-        skills_manager.update_skill(name, {"status": "draft", "confidence": 0.35})
+        skills_manager.update_skill(name, {"status": "draft", "confidence": 0.35}, owner=owner)
     except Exception:
         pass
     skills_manager.set_audit(
@@ -1474,7 +1474,7 @@ def setup_skills_routes(skills_manager: SkillsManager) -> APIRouter:
             "pitfalls": sk.pitfalls,
             "verification": sk.verification,
             "body_extra": sk.body_extra,
-        })
+        }, owner=user)
         if not ok:
             raise HTTPException(500, "Update failed")
         # Manual markdown edits can create or substantially rewrite a draft
@@ -1496,7 +1496,7 @@ def setup_skills_routes(skills_manager: SkillsManager) -> APIRouter:
         updates = body.dict(exclude_none=True)
         if not updates:
             return {"ok": True}
-        ok = skills_manager.update_skill(match.get("name"), updates)
+        ok = skills_manager.update_skill(match.get("name"), updates, owner=user)
         if not ok:
             raise HTTPException(404, "Skill not found")
         if not match.get("audit_verdict"):

tests/test_skills_routes_owner_update.py

@@ -0,0 +1,136 @@
+import json
+import textwrap
+from pathlib import Path
+
+import pytest
+from fastapi import Request
+from fastapi.datastructures import State
+
+from routes.skills_routes import SkillUpdateRequest, setup_skills_routes
+from services.memory.skill_format import slugify
+from services.memory.skills import SkillsManager
+
+
+def _write_skill_md(skills_root: Path, category: str, name: str,
+                    owner: str, description: str = "test") -> Path:
+    skill_dir = skills_root / slugify(category or "general", fallback="general") / name
+    skill_dir.mkdir(parents=True, exist_ok=True)
+    md = textwrap.dedent(f"""\
+        ---
+        name: {name}
+        description: {description}
+        version: 1.0.0
+        category: {category}
+        tags: []
+        status: draft
+        confidence: 0.8
+        source: learned
+        owner: {owner}
+        created: 2026-01-01T00:00:00Z
+        ---
+
+        # When to use
+        test
+
+        # Procedure
+        - step 1
+        """)
+    path = skill_dir / "SKILL.md"
+    path.write_text(md, encoding="utf-8")
+    return path
+
+
+def _request(user: str, body=None) -> Request:
+    class DummyApp:
+        state = State()
+
+    payload = json.dumps(body).encode("utf-8") if body is not None else b""
+    sent = False
+
+    async def receive():
+        nonlocal sent
+        if sent:
+            return {"type": "http.request", "body": b"", "more_body": False}
+        sent = True
+        return {"type": "http.request", "body": payload, "more_body": False}
+
+    return Request(scope={
+        "type": "http",
+        "method": "POST" if body is not None else "PUT",
+        "headers": [(b"content-type", b"application/json")] if body is not None else [],
+        "app": DummyApp(),
+        "state": {"current_user": user},
+    }, receive=receive)
+
+
+def _route_handler(router, path: str, method: str):
+    return next(
+        route.endpoint for route in router.routes
+        if route.path == path and method in route.methods
+    )
+
+
+@pytest.mark.asyncio
+async def test_update_skill_route_passes_owner_to_manager(tmp_path):
+    skills_root = tmp_path / "skills"
+    alice_path = _write_skill_md(skills_root, "alice-cat", "caveman-mode", "alice", "alice original")
+    bob_path = _write_skill_md(skills_root, "bob-cat", "caveman-mode", "bob", "bob original")
+
+    sm = SkillsManager(str(tmp_path))
+    router = setup_skills_routes(sm)
+    update_route = _route_handler(router, "/api/skills/{skill_id}", "PUT")
+
+    result = await update_route(
+        _request("alice"),
+        "caveman-mode",
+        SkillUpdateRequest(status="published", description="alice updated"),
+    )
+
+    assert result == {"ok": True}
+    alice_after = alice_path.read_text(encoding="utf-8")
+    bob_after = bob_path.read_text(encoding="utf-8")
+    assert "status: published" in alice_after
+    assert "alice updated" in alice_after
+    assert "status: draft" in bob_after
+    assert "bob original" in bob_after
+
+
+@pytest.mark.asyncio
+async def test_save_skill_markdown_route_passes_owner_to_manager(tmp_path):
+    skills_root = tmp_path / "skills"
+    skill_path = _write_skill_md(skills_root, "general", "caveman-mode", "alice", "before")
+
+    sm = SkillsManager(str(tmp_path))
+    router = setup_skills_routes(sm)
+    save_route = _route_handler(router, "/api/skills/{skill_id}/markdown", "POST")
+    markdown = textwrap.dedent("""\
+        ---
+        name: caveman-mode
+        description: after
+        version: 1.0.0
+        category: general
+        tags: []
+        status: published
+        confidence: 0.9
+        source: user
+        owner: alice
+        created: 2026-01-01T00:00:00Z
+        ---
+
+        # When to use
+        after
+
+        # Procedure
+        - updated step
+        """)
+
+    result = await save_route(
+        _request("alice", {"markdown": markdown}),
+        "caveman-mode",
+    )
+
+    assert result == {"ok": True, "name": "caveman-mode"}
+    saved = skill_path.read_text(encoding="utf-8")
+    assert "description: after" in saved
+    assert "status: published" in saved
+    assert "- updated step" in saved