Merge current dev and refresh PR #6738 from the 7011 integration

services/memory/memory_extractor.py

@@ -15,6 +15,7 @@ import json
 import logging
 import os
 import re
+import time
 from typing import Optional
 
 from src.memory import MemoryStoreUnreadable
@@ -78,11 +79,13 @@ EXTRACT_SYSTEM_PROMPT = (
     "things the assistant said, one-off tasks, opinions on the current topic.\n\n"
     "Rules:\n"
     "- MAX 2 facts per conversation — only the most important\n"
-    "- Only extract facts the USER stated or clearly implied\n"
+    "- Only extract facts the USER explicitly stated about themselves\n"
+    "- Questions, requests to inspect records, and tool/assistant output are not evidence\n"
     "- Each fact must be a single short sentence (under 15 words)\n"
     "- If a fact is similar to something likely already known, skip it\n"
     "- If nothing durable was revealed, return []\n\n"
-    "Return a JSON array of objects with 'text' and 'category' fields.\n"
+    "Return a JSON array of objects with 'text', 'category', and 'evidence' fields. "
+    "Evidence must be an exact quote from a user's own factual statement.\n"
     "Categories: 'identity', 'preference', 'fact', 'contact', 'project', 'goal'\n\n"
     "Return ONLY valid JSON, no markdown fences."
 )
@@ -213,7 +216,10 @@ def _fallback_memory_candidates(messages) -> list[dict]:
         if not text:
             continue
 
-        m = re.search(r"\bmy name is\s+([A-Za-z][A-Za-z0-9 .'\-]{1,50})\b", text, re.I)
+        m = re.search(
+            r"\bmy name is\s+([A-Za-z][A-Za-z0-9 '\-]{0,49}?)"
+            r"(?=\s+(?:and|but|please)\b|[.!?;,\n]|$)", text, re.I,
+        )
         if m:
             name = _clean_memory_value(m.group(1), 50)
             if name:
@@ -259,22 +265,183 @@ def _fallback_memory_candidates(messages) -> list[dict]:
     return candidates[:2]
 
 
+_USER_FACT_ASSERTION = re.compile(
+    r"\b(?:i(?:'m| am| live| work| use| have| prefer| like| love| hate| own| study| grew up)\b"
+    r"|my\s+[a-z][a-z -]{0,40}\s+(?:is|are|was|works|lives)\b"
+    r"|we\s+(?:are|live|work|have|own)\b"
+    r"|our\s+[a-z][a-z -]{0,40}\s+(?:is|are|was)\b)",
+    re.IGNORECASE,
+)
+
+
+def _has_user_fact_evidence(fact: dict, messages) -> bool:
+    """Require an exact user assertion before persisting an LLM-inferred memory."""
+    evidence = fact.get("evidence")
+    if not isinstance(evidence, str):
+        return False
+    evidence = re.sub(r"\s+", " ", evidence).strip()
+    if len(evidence.split()) < 3 or not _USER_FACT_ASSERTION.match(evidence):
+        return False
+    for message in messages:
+        if _message_role(message) != "user":
+            continue
+        utterance = re.sub(r"\s+", " ", _message_text(message)).strip()
+        position = utterance.casefold().find(evidence.casefold())
+        if position < 0:
+            continue
+        # Judge the clause containing the quote, so a later question does not
+        # erase an earlier factual statement in the same user turn.
+        clause_start = max(
+            utterance.rfind(mark, 0, position) for mark in (".", "?", "!", ";")
+        ) + 1
+        clause_end = min(
+            [end for mark in (".", "?", "!", ";")
+             if (end := utterance.find(mark, position)) >= 0] or [len(utterance)]
+        )
+        clause = utterance[clause_start:clause_end].strip()
+        # A question about one's records is not a statement of the answer.
+        if utterance[clause_end:clause_end + 1] == "?" or re.match(
+            r"^(?:who|what|where|when|why|how|do|does|did|can|could|would|"
+            r"show|list|find|search|tell|summarize|open|read)\b",
+            clause, re.IGNORECASE,
+        ):
+            continue
+        return True
+    return False
+
+
+_TOKEN_RE = re.compile(r"[a-z0-9]+")
+
+# Function words carry no distinguishing content. Dropping them before
+# comparing two facts keeps punctuation and harmless rephrasing from defeating
+# the duplicate check.
+_STOPWORDS = frozenset(
+    "a an the and or but if then of to in on at for with by from as is are was "
+    "were be been being do does did has have had it its this that these those "
+    "there here user users someone somebody they them their he she his her him i "
+    "we you your my our me us who whom whose what when where why how also "
+    "currently now still about into over under again more most very can could "
+    "may might must will would should".split()
+)
+
+# Words that frame a name/username statement; excluded when reading the value
+# so the actual name tokens can be compared.
+_NAME_TEMPLATE_WORDS = frozenset(
+    "name names named called call appears appear seems seem the user users his "
+    "her their it is are was were be been being username usernames login "
+    "account handle".split()
+)
+
+_NAME_KEYWORDS = frozenset({"name", "named", "called"})
+_USERNAME_KEYWORDS = frozenset({"username", "login"})
+
+
+def _content_tokens(text: str) -> set:
+    """Lowercased content tokens with punctuation and stopwords removed."""
+    toks = _TOKEN_RE.findall((text or "").lower())
+    return {t for t in toks if t not in _STOPWORDS and len(t) >= 2}
+
+
+def _name_slot(text: str):
+    """Return (slot, value_tokens) when text states a name/username, else None.
+
+    Tokens are scanned so "username" is never mistaken for "name", and the
+    first keyword decides the slot (a sentence about a server *named* X that
+    also mentions a *username* Y is a 'name' fact, not a username fact)."""
+    words = _TOKEN_RE.findall((text or "").lower())
+    slot = None
+    for w in words:
+        if w in _NAME_KEYWORDS:
+            slot = "name"
+            break
+        if w in _USERNAME_KEYWORDS:
+            slot = "username"
+            break
+    if not slot:
+        return None
+    value = {
+        w for w in words
+        if w not in _NAME_TEMPLATE_WORDS and w not in _STOPWORDS and len(w) >= 2
+    }
+    return (slot, value) if value else None
+
+
+def _name_slot_tokens(text: str):
+    slot = _name_slot(text)
+    return slot[1] if slot else None
+
+
+def _name_vals_close(a: set, b: set) -> bool:
+    """True when two name values state the same name: identical, or one is a
+    strict refinement of the other where the larger value is a clean name
+    (<=4 alphabetic tokens). Guards against merging a username into a fact
+    like 'a server named Odysseus ... with username pewds'."""
+    if a == b:
+        return True
+    if not a or not b:
+        return False
+    bigger = a if len(a) >= len(b) else b
+    clean = len(bigger) <= 4 and all(t.isalpha() for t in bigger)
+    return clean and (a <= b or b <= a)
+
+
 def _is_text_duplicate(new_text: str, existing: list, threshold: float = 0.6) -> bool:
-    """Check if new_text is too similar to any existing memory (Jaccard similarity)."""
-    new_tokens = set(new_text.lower().split())
+    """Check if new_text states the same fact as an existing memory.
+
+    Three tests, cheapest first: symmetric Jaccard over content tokens,
+    containment (one fact is a strict refinement of the other), and a
+    name/username-slot match. The naive whole-string comparison this replaces
+    kept punctuation (so "Alex." != "alex") and missed short-vs-full names
+    ("Felix" vs "FELIX ARVID ULF KJELLBERG" scored 0.43).
+    """
+    new_tokens = _content_tokens(new_text)
     if not new_tokens:
         return False
+    new_slot = _name_slot(new_text)
     for entry in _memory_dicts(existing):
-        old_tokens = set(entry.get("text", "").lower().split())
+        old_text = entry.get("text", "")
+        old_tokens = _content_tokens(old_text)
         if not old_tokens:
             continue
         intersection = new_tokens & old_tokens
+        if not intersection:
+            continue
         union = new_tokens | old_tokens
         if len(intersection) / len(union) >= threshold:
             return True
+        # Containment: one fact is a strict refinement of the other. Guard on
+        # the shorter side so two-token facts ("Likes Python") are not merged
+        # into longer sentences that merely start the same way.
+        if len(intersection) >= 3 and len(intersection) / min(len(new_tokens), len(old_tokens)) >= 0.85:
+            return True
+        # Name slot: both state the same kind of name and the values match
+        # (short vs full name, or the same name behind different phrasing).
+        old_slot = _name_slot(old_text)
+        if new_slot and old_slot and new_slot[0] == old_slot[0]:
+            if _name_vals_close(new_slot[1], old_slot[1]):
+                return True
     return False
 
 
+def _name_enrichment_target(new_text: str, existing: list):
+    """Return an existing entry whose name value is a strict subset of the
+    new fact's, so the fuller name replaces it in place; else None."""
+    ns = _name_slot(new_text)
+    if not ns:
+        return None
+    slot, new_val = ns
+    best = None
+    for entry in _memory_dicts(existing):
+        old_slot = _name_slot(entry.get("text", ""))
+        if not old_slot or old_slot[0] != slot:
+            continue
+        old_val = old_slot[1]
+        if old_val < new_val and _name_vals_close(old_val, new_val) and len(new_val) <= 4 and all(t.isalpha() for t in new_val):
+            if best is None or len(entry.get("text", "")) < len(best.get("text", "")):
+                best = entry
+    return best
+
+
 def _parse_extraction_json(raw: str) -> list:
     """Parse the extraction LLM's reply into a list of facts, tolerating
     reasoning-model noise.
@@ -331,6 +498,7 @@ async def extract_and_store(
 
     try:
         from src.llm_core import llm_call_async
+        from src.utility_effort import effort_for_call
 
         # Get last N messages from session
         messages = session.get_context_messages()
@@ -404,6 +572,9 @@ async def extract_and_store(
                 # enough once thinking has room.
                 max_tokens=4096,
                 headers=headers,
+                reasoning_effort=effort_for_call(
+                    endpoint_url, model, getattr(session, "owner", None), session
+                ),
             )
 
             # Parse JSON, tolerating reasoning-model noise (<think> blocks, a
@@ -416,8 +587,13 @@ async def extract_and_store(
         if not isinstance(facts, list):
             facts = []
 
+        # The extraction model sees assistant/tool text and can mistake a
+        # failed lookup or a one-off request for a durable personal fact.
+        # Only its claims with a quoted user assertion may enter memory.
+        facts = [fact for fact in facts if isinstance(fact, dict)
+                 and _has_user_fact_evidence(fact, stripped_recent)]
         if fallback_facts:
-            facts = list(facts) + fallback_facts
+            facts.extend(fallback_facts)
 
         if not facts:
             logger.info("Auto memory extraction ran: 0 candidates")
@@ -479,6 +655,25 @@ async def extract_and_store(
 
             # Text dedup fallback: exact match + fuzzy similarity
             user_existing = [e for e in existing if e.get("owner") == _owner or e.get("owner") is None] if _owner else existing
+
+            # Name enrichment: if an existing name fact is a strict prefix of
+            # this more complete one, upgrade it in place instead of adding a
+            # near-duplicate ("name is Felix" -> "name is Felix Arvid Ulf
+            # Kjellberg"). The vector store upserts by id, so re-adding is safe.
+            _enrich = _name_enrichment_target(fact_text, user_existing)
+            if _enrich is not None:
+                _enrich["text"] = fact_text
+                _enrich["timestamp"] = int(time.time())
+                if category:
+                    _enrich["category"] = category
+                if memory_vector and memory_vector.healthy:
+                    try:
+                        memory_vector.add(_enrich["id"], fact_text)
+                    except Exception as e:
+                        logger.warning(f"Memory vector update failed for {_enrich['id']}: {e}")
+                added += 1
+                continue
+
             if memory_manager.find_duplicates(fact_text, user_existing):
                 continue
             # Fuzzy text similarity check (catches rephrased duplicates when vector index is unavailable)
@@ -559,6 +754,7 @@ async def update_persona_memory(
 
     try:
         from src.llm_core import llm_call_async
+        from src.utility_effort import effort_for_call
         from src.text_helpers import strip_think
 
         custom = {}
@@ -607,10 +803,15 @@ async def update_persona_memory(
             temperature=0.1,
             max_tokens=1200,
             headers=headers,
+            reasoning_effort=effort_for_call(
+                endpoint_url, model, getattr(session, "owner", None), session
+            ),
         )
 
         updated = strip_think(str(raw or ""), prose=True, prompt_echo=True).strip()
-        updated = re.sub(r"^```(?:text|markdown)?\s*|\s*```$", "", updated, flags=re.I | re.S).strip()
+        # No leading `\s*` before the closing fence: .strip() drops that
+        # whitespace anyway, and scanning it from every offset was quadratic.
+        updated = re.sub(r"^```(?:text|markdown)?\s*|```$", "", updated, flags=re.I | re.S).strip()
         if len(updated) > 6000:
             updated = updated[:6000].rstrip()
         if updated == existing_memory:
@@ -641,6 +842,7 @@ async def audit_memories(
     """
     try:
         from src.llm_core import llm_call_async
+        from src.utility_effort import effort_for_call
 
         existing = memory_manager.load(owner=owner)
         if not existing:
@@ -689,13 +891,15 @@ async def audit_memories(
             # Bound the call so the Tidy whirlpool can't spin indefinitely on a
             # slow/large generation.
             timeout=120,
+            reasoning_effort=effort_for_call(endpoint_url, model, owner),
         )
 
         # Parse the JSON list, tolerating reasoning-model noise: <think> blocks,
         # markdown fences, leading prose, and trailing commas.
         import re as _re
+        from src.text_helpers import strip_closed_think_blocks
         text = (raw or "").strip()
-        text = _re.sub(r'<think(?:ing)?>[\s\S]*?</think(?:ing)?>', '', text, flags=_re.I).strip()
+        text = strip_closed_think_blocks(text).strip()
 
         def _loads_list(s):
             if not s:
@@ -711,7 +915,9 @@ async def audit_memories(
 
         cleaned = _loads_list(text)
         if cleaned is None:
-            _m = _re.search(r'```(?:json)?\s*\n?([\s\S]*?)```', text)
+            # Possessive `\s*+`: handing fence whitespace back to the body on a
+            # missing closing fence only rescanned the same tail (ReDoS).
+            _m = _re.search(r'```(?:json)?\s*+\n?([\s\S]*?)```', text)
             if _m:
                 cleaned = _loads_list(_m.group(1).strip())
         if cleaned is None:

services/memory/skill_extractor.py

@@ -149,6 +149,7 @@ async def maybe_extract_skill(
 
     try:
         from src.llm_core import llm_call_async
+        from src.utility_effort import effort_for_call
 
         # Get recent messages
         history = session.get_context_messages()
@@ -204,6 +205,7 @@ async def maybe_extract_skill(
             ],
             headers=headers,
             timeout=30,
+            reasoning_effort=effort_for_call(endpoint_url, model, owner, session),
         )
         logger.debug(
             "[skill-extract] LLM returned in %.1fs (len=%d, head=%r)",

services/memory/skills.py

@@ -674,6 +674,7 @@ class SkillsManager:
         *,
         active_toolsets: Optional[List[str]] = None,
         platform: Optional[str] = None,
+        local_model: Optional[bool] = None,
     ) -> List[Dict]:
         """Return the `[{name, description, category, status}]` list the
         agent sees in its system prompt.
@@ -710,6 +711,11 @@ class SkillsManager:
             # Platform gating
             if platform and s.get("platforms") and platform not in s["platforms"]:
                 continue
+            # remote-only tag: hide from local/private-endpoint models. Big
+            # on-demand reference lookups overwhelm small local contexts.
+            # local_model=None (API listings, UI) never gates.
+            if local_model and "remote-only" in (s.get("tags") or []):
+                continue
             # requires_toolsets: hide unless every required toolset is active.
             # active_toolsets=None means the caller doesn't know the active
             # set (API listings, chat preface) — don't gate in that case;
@@ -745,6 +751,7 @@ class SkillsManager:
         min_confidence: float = 0.0,
         available_toolsets: Optional[Iterable[str]] = None,
         platform: Optional[str] = None,
+        published_only: bool = False,
     ) -> List[Dict]:
         if skills is None:
             skills = self.load_all()
@@ -775,9 +782,15 @@ class SkillsManager:
                 if not skill.get("platforms") or platform in skill.get("platforms", [])
             ]
         # Prompt injection is fail-closed for user skills. Built-ins are
-        # shipped procedures; every other skill needs a passing audit and a
-        # confidence score at the user's current threshold.
-        if min_confidence > 0:
+        # shipped procedures. With Auto-approve OFF (published_only), the
+        # user's publish is the approval; otherwise a skill needs a passing
+        # audit and a confidence score at the user's current threshold.
+        if published_only:
+            skills = [
+                s for s in skills
+                if s.get("source") == "builtin" or s.get("status") == "published"
+            ]
+        elif min_confidence > 0:
             def _passes(s):
                 if s.get("source") == "builtin":
                     return True

services/skills/__init__.py

@@ -0,0 +1 @@
+"""Skills domain operations; persistence and lifecycle remain in SkillsManager."""

services/skills/agent_runner.py

@@ -0,0 +1,308 @@
+"""Agent execution, transcripts and same-task stream cleanup."""
+
+import logging
+from contextlib import aclosing
+from typing import Optional
+
+from .dependencies import AuditDependencies
+
+logger = logging.getLogger(__name__)
+
+_skill_test_jobs: dict = {}
+
+
+class SkillAuditUnavailable(RuntimeError):
+    """The test infrastructure failed; this is not evidence about a skill."""
+
+
+async def run_test_job(
+    key,
+    name,
+    md,
+    task,
+    url,
+    model,
+    headers,
+    owner,
+    skills_manager=None,
+    *,
+    messages=None,
+    transcript=None,
+    exact_approval=None,
+    request_authority=None,
+    deps: AuditDependencies,
+):
+    """Background coroutine: run the skill in an agent loop, capture a condensed
+    log + transcript, then have the judge grade it. Writes into deps.test_jobs."""
+    import json as _json
+    from src.agent_loop import stream_agent_loop
+    from src.agent_runtime.authority import RequestAuthority
+
+    job = deps.test_jobs.get(key)
+    if job is None:
+        return
+    log = job["log"]
+    transcript = transcript if isinstance(transcript, list) else []
+    say_buf = []
+    skill_stats = {"turns": 0, "tool_calls": 0}
+
+    def _flush_say():
+        if say_buf:
+            log.append({"type": "say", "text": "".join(say_buf)})
+            say_buf.clear()
+
+    messages = list(messages) if isinstance(messages, list) else deps.test_messages(md, task)
+    try:
+        # aclosing: pausing for an approval leaves this loop early, and the
+        # stream's ContextVar cleanup must run in this task, not in asyncio's
+        # finalizer (#6635).
+        async with aclosing(stream_agent_loop(
+            url, model, messages, headers=headers,
+            temperature=0.3, max_tokens=0, max_rounds=8, owner=owner,
+            exact_approval=exact_approval,
+            request_authority=(request_authority or (
+                exact_approval.pending.request_authority if exact_approval is not None else None
+            ) or RequestAuthority.empty(owner=owner)),
+        )) as stream:
+            async for chunk in stream:
+                if not chunk.startswith("data: ") or chunk.strip() == "data: [DONE]":
+                    continue
+                try:
+                    d = _json.loads(chunk[6:])
+                except Exception:
+                    continue
+                if d.get("delta"):
+                    say_buf.append(d["delta"]); transcript.append(d["delta"])
+                elif d.get("type") == "tool_start":
+                    _flush_say()
+                    skill_stats["tool_calls"] += 1
+                    cmd = str(d.get("command") or d.get("args") or "")[:300]
+                    log.append({"type": "tool_start", "tool": d.get("tool"), "command": cmd})
+                    transcript.append(f"\n[tool {d.get('tool')}] {cmd}\n")
+                elif d.get("type") == "tool_output":
+                    _flush_say()
+                    out = str(d.get("output") or "")[:600]
+                    tool_log = {"type": "tool_output", "output": out}
+                    approval = d.get("ask_user")
+                    if isinstance(approval, dict):
+                        tool_log["ask_user"] = approval
+                    log.append(tool_log)
+                    transcript.append(f"[output] {out}\n")
+                    if (
+                        isinstance(approval, dict)
+                        and approval.get("kind") == "tool_approval"
+                        and approval.get("approval_id")
+                    ):
+                        # Manual skill tests have their own polling UI instead of a
+                        # chat session. Pause the run and retain only server-side
+                        # continuation state until the same owner approves/denies
+                        # this exact sealed action.
+                        job["status"] = "awaiting_approval"
+                        job["approval"] = approval
+                        job["_transcript"] = transcript
+                        return
+                elif d.get("type") == "agent_step":
+                    _flush_say()
+                    try:
+                        skill_stats["turns"] = max(skill_stats["turns"], int(d.get("round") or 0))
+                    except (TypeError, ValueError):
+                        pass
+                    log.append({"type": "agent_step", "round": d.get("round")})
+                    transcript.append(f"\n--- round {d.get('round')} ---\n")
+                elif d.get("type") == "metrics":
+                    data = d.get("data") or {}
+                    try:
+                        skill_stats["turns"] = max(skill_stats["turns"], int(data.get("agent_rounds") or 0))
+                    except (TypeError, ValueError):
+                        pass
+                    try:
+                        skill_stats["tool_calls"] = max(skill_stats["tool_calls"], int(data.get("tool_calls") or 0))
+                    except (TypeError, ValueError):
+                        pass
+                if len(log) > 600:
+                    del log[0:len(log) - 600]
+        _flush_say()
+    except Exception as e:
+        _flush_say()
+        log.append({"type": "error", "error": str(e)})
+
+    job.pop("approval", None)
+    job.pop("_transcript", None)
+    job.pop("_run", None)
+    log.append({"type": "evaluating"})
+    try:
+        baseline_task = task
+        log.append({"type": "agent_step", "round": "baseline"})
+        baseline_transcript, baseline_stats, baseline_approval = await deps.run_arm(
+            deps.baseline_messages(baseline_task),
+            url,
+            model,
+            headers,
+            owner,
+        )
+        if baseline_approval is not None:
+            try:
+                from src.tool_approvals import tool_approval_store
+                tool_approval_store.consume(
+                    baseline_approval.get("approval_id"),
+                    decision="deny",
+                    owner=owner,
+                    session_id=None,
+                )
+            except Exception:
+                logger.debug("Could not retire manual-test baseline approval", exc_info=True)
+        job["verdict"] = await deps.evaluate_run(
+            md,
+            task,
+            "".join(transcript),
+            url,
+            model,
+            headers,
+            baseline_transcript=baseline_transcript,
+            skill_stats=skill_stats,
+            baseline_stats=baseline_stats,
+            owner=owner,
+        )
+    except Exception as e:
+        job["verdict"] = {"verdict": "unknown", "confidence": 0, "summary": f"Eval failed: {e}", "issues": []}
+    # Record the result so the card shows a 'verified' check (a manual test
+    # never involves the teacher) and nudge the confidence score to match the
+    # verdict — same scale as Audit-all's pass=0.95. inconclusive/unknown leave
+    # the score alone (missing-fixture or parse failures shouldn't punish it).
+    if skills_manager is not None:
+        v = (job["verdict"] or {}).get("verdict") or "unknown"
+        try:
+            skills_manager.set_audit(
+                name,
+                v,
+                by_teacher=False,
+                worker_model=model,
+                owner=owner,
+                **deps.verdict_efficiency(job.get("verdict")),
+            )
+        except Exception:
+            pass
+        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}, owner=owner)
+            except Exception:
+                pass
+    job["status"] = "done"
+
+
+async def run_audit_arm(messages: list[dict], url, model, headers, owner,
+                               workload: str = "foreground") -> tuple[str, dict, Optional[dict]]:
+    """Run one audit arm in the agent loop; return transcript, stats, approval."""
+    import json as _json
+    from src.agent_loop import stream_agent_loop
+    from src.agent_runtime.authority import RequestAuthority, active_request_authority
+    transcript = []
+    approval_required = None
+    stats = {"turns": 0, "tool_calls": 0}
+    try:
+        # max_tokens explicitly set: passing 0 lets some upstreams (Ollama,
+        # OpenAI-compat) generate an empty completion, which manifested as
+        # the skill test returning nothing while chat (which carries its
+        # preset's max_tokens) worked. 4096 matches the chat default.
+        # aclosing: stopping for an approval leaves this loop early (#6635).
+        async with aclosing(stream_agent_loop(
+            url, model, messages, headers=headers,
+            temperature=0.3, max_tokens=4096, max_rounds=8,
+            owner=owner, workload=workload, suppress_skills=True,
+            request_authority=(active_request_authority() or RequestAuthority.empty(owner=owner)),
+        )) as stream:
+            async for chunk in stream:
+                # Streams can include an SSE event line before the data line,
+                # notably `event: error`. Do not silently discard those failures.
+                payload = next((line[6:] for line in chunk.splitlines() if line.startswith("data: ")), None)
+                if payload is None or payload == "[DONE]":
+                    continue
+                try:
+                    d = _json.loads(payload)
+                except Exception:
+                    continue
+                if d.get("error") or d.get("type") == "error":
+                    raise SkillAuditUnavailable(str(d.get("error") or d.get("message") or "Audit stream failed"))
+                if d.get("delta"):
+                    transcript.append(d["delta"])
+                elif d.get("type") == "tool_start":
+                    stats["tool_calls"] += 1
+                    transcript.append(f"\n[tool {d.get('tool')}] {str(d.get('command') or d.get('args') or '')[:300]}\n")
+                elif d.get("type") == "tool_output":
+                    transcript.append(f"[output] {str(d.get('output') or '')[:600]}\n")
+                    approval = d.get("ask_user")
+                    if (
+                        isinstance(approval, dict)
+                        and approval.get("kind") == "tool_approval"
+                    ):
+                        approval_required = approval
+                        break
+                elif d.get("type") == "agent_step":
+                    try:
+                        stats["turns"] = max(stats["turns"], int(d.get("round") or 0))
+                    except (TypeError, ValueError):
+                        pass
+                    transcript.append(f"\n--- round {d.get('round')} ---\n")
+                elif d.get("type") == "metrics":
+                    data = d.get("data") or {}
+                    try:
+                        stats["turns"] = max(stats["turns"], int(data.get("agent_rounds") or 0))
+                    except (TypeError, ValueError):
+                        pass
+                    try:
+                        stats["tool_calls"] = max(stats["tool_calls"], int(data.get("tool_calls") or 0))
+                    except (TypeError, ValueError):
+                        pass
+    except SkillAuditUnavailable:
+        raise
+    except Exception as e:
+        raise SkillAuditUnavailable(str(e)) from e
+    return "".join(transcript), stats, approval_required
+
+
+async def run_test_once(md: str, task: str, url, model, headers, owner,
+                               workload: str = "foreground", *, deps: AuditDependencies) -> tuple:
+    """Run the skill once in the agent loop; return (transcript, verdict)."""
+    messages = deps.test_messages(md, task)
+    text, stats, approval_required = await deps.run_arm(
+        messages, url, model, headers, owner, workload=workload,
+    )
+    if approval_required is not None:
+        # Unattended audits have no authority to approve and no UI that could
+        # resume this record. Destructively deny it now instead of leaving a
+        # reusable opaque grant pending until TTL/cap eviction.
+        try:
+            from src.tool_approvals import tool_approval_store
+            tool_approval_store.consume(
+                approval_required.get("approval_id"),
+                decision="deny",
+                owner=owner,
+                session_id=None,
+            )
+        except Exception:
+            logger.debug("Could not retire unattended skill approval", exc_info=True)
+        return text, {
+            "verdict": "inconclusive",
+            "confidence": 1.0,
+            "summary": (
+                "This automated audit reached an exact action that requires "
+                "a human approval; no action was executed."
+            ),
+            "issues": [
+                "Run this skill's manual test and review the sealed action."
+            ],
+            "approval_required": True,
+        }
+    verdict = await deps.evaluate_run(
+        md,
+        task,
+        text,
+        url,
+        model,
+        headers,
+        skill_stats=stats,
+        workload=workload,
+        owner=owner,
+    )
+    return text, verdict

services/skills/audit_workflow.py

@@ -0,0 +1,439 @@
+"""Sequential skill audit coordination and transient owner-scoped jobs."""
+
+import logging
+from typing import Optional
+
+from services.memory.skills import SkillsManager
+from .dependencies import AuditDependencies
+
+from .agent_runner import SkillAuditUnavailable
+
+logger = logging.getLogger(__name__)
+
+_skill_audit_jobs: dict = {}
+
+
+async def audit_one_skill(skills_manager, skill, url, model, headers,
+                           teacher, owner, log, workload: str = "foreground", *, deps: AuditDependencies) -> dict:
+    """Test → judge → self-edit+retry → (teacher edit+retry) → flag. Never deletes;
+    a skill the teacher still can't fix is demoted to draft for manual review.
+    `teacher` is (url, model, headers) or None. `log(msg)` records progress."""
+    name = skill.get("name")
+
+    # Reflect the audit outcome in the skill's confidence so the main list
+    # updates: a clean pass earns high confidence; a pass that needed fixing
+    # earns a bit less; a skill that still fails is marked low.
+    def _set_conf(c):
+        try:
+            skills_manager.update_skill(name, {"confidence": c}, owner=owner)
+        except Exception:
+            pass
+
+    md = skills_manager.read_skill_md(name, owner=owner)
+    if not md:
+        log(f"{name}: no source — skipped")
+        return {"skill": name, "result": "skipped"}
+
+    # Cheap deterministic cleanup first. If this is an obvious lower-priority
+    # duplicate, do not spend LLM turns on necessity, retrieval precision,
+    # skill-vs-baseline testing, self-edit, or teacher escalation.
+    duplicate_of = deps.duplicate_blocker(skills_manager, name, owner)
+    if duplicate_of:
+        reason = f"Lower-priority duplicate of {duplicate_of}"
+        try:
+            skills_manager.update_skill(name, {"status": "draft", "confidence": 0.35}, owner=owner)
+            skills_manager.set_audit(name, "skipped", by_teacher=False, worker_model=model, owner=owner)
+            skills_manager.set_necessity(name, False, [duplicate_of], reason, owner=owner)
+        except Exception:
+            pass
+        log(f"{name}: draft — skipped audit ({reason[:100]})")
+        return {"skill": name, "result": "skipped_duplicate", "reason": reason, "confidence": 0.35, "status": "draft"}
+
+    # Advisory necessity/redundancy check — runs once, independent of the test
+    # outcome, and only records a flag the UI surfaces (never deletes/demotes).
+    others = []
+    nec = None
+    try:
+        # Only compare against skills the SAME owner can see, so the necessity
+        # judge never sees (or flags "redundant_with") another user's skills.
+        sk_owner = skill.get("owner")
+        others = [
+            {"name": s.get("name"), "description": s.get("description", "")}
+            for s in skills_manager.load(owner=owner)
+            if s.get("name") and s.get("name") != name
+            and (not sk_owner or not s.get("owner") or s.get("owner") == sk_owner)
+        ]
+        nec = await deps.evaluate_necessity(
+            md, others, url, model, headers, workload=workload,
+            owner=owner,
+        )
+        if nec is not None:
+            skills_manager.set_necessity(name, nec.get("necessary", True),
+                                         nec.get("redundant_with"), nec.get("reason"),
+                                         owner=owner)
+            if not nec.get("necessary", True):
+                log(f"{name}: possibly unnecessary — {nec.get('reason', '')[:80]}")
+    except Exception as e:
+        log(f"{name}: necessity check skipped — {e}")
+
+    # Retrieval precision check: if broad tags/trigger text would make this
+    # narrow skill over-inject, fix only metadata before the functional test.
+    try:
+        if deps.should_check_retrieval_precision(skill):
+            rp = await deps.evaluate_retrieval_precision(
+                md, others, url, model, headers, workload=workload,
+                owner=owner,
+            )
+            if rp and not rp.get("ok"):
+                issues = rp.get("issues") or ["metadata: retrieval: narrow tags and when_to_use to the intended trigger"]
+                log(f"{name}: narrowing retrieval metadata — {(rp.get('summary') or issues[0])[:80]}")
+                fixed = await deps.improve_markdown(md, {
+                    "verdict": "pass",
+                    "confidence": 1.0,
+                    "summary": rp.get("summary") or "Retrieval metadata is too broad.",
+                    "issues": issues,
+                }, "Retrieval audit only: the procedure may work, but matching metadata is too broad.",
+                    url, model, headers, workload=workload, owner=owner)
+                if fixed and fixed.strip() != md.strip() and deps.apply_markdown(skills_manager, name, fixed, owner):
+                    md = fixed
+                    refreshed = next((s for s in skills_manager.load(owner=owner) if s.get("name") == name), None)
+                    if refreshed:
+                        skill = refreshed
+    except Exception as e:
+        log(f"{name}: retrieval precision check skipped — {e}")
+
+    task = deps.test_task(skill)
+    log(f"{name}: testing…")
+    skill_messages = deps.test_messages(md, task)
+    transcript, skill_stats, approval_required = await deps.run_arm(
+        skill_messages,
+        url,
+        model,
+        headers,
+        owner,
+        workload=workload,
+    )
+    if approval_required is not None:
+        try:
+            from src.tool_approvals import tool_approval_store
+            tool_approval_store.consume(
+                approval_required.get("approval_id"),
+                decision="deny",
+                owner=owner,
+                session_id=None,
+            )
+        except Exception:
+            logger.debug("Could not retire unattended skill approval", exc_info=True)
+        verdict = {
+            "verdict": "inconclusive",
+            "confidence": 1.0,
+            "summary": (
+                "This automated audit reached an exact action that requires "
+                "a human approval; no action was executed."
+            ),
+            "issues": [
+                "Run this skill's manual test and review the sealed action."
+            ],
+            "approval_required": True,
+        }
+    else:
+        baseline_task = task
+        log(f"{name}: running no-skill baseline…")
+        baseline_transcript, baseline_stats, baseline_approval = await deps.run_arm(
+            deps.baseline_messages(baseline_task),
+            url,
+            model,
+            headers,
+            owner,
+            workload=workload,
+        )
+        if baseline_approval is not None:
+            try:
+                from src.tool_approvals import tool_approval_store
+                tool_approval_store.consume(
+                    baseline_approval.get("approval_id"),
+                    decision="deny",
+                    owner=owner,
+                    session_id=None,
+                )
+            except Exception:
+                logger.debug("Could not retire unattended baseline approval", exc_info=True)
+        verdict = await deps.evaluate_run(
+            md,
+            task,
+            transcript,
+            url,
+            model,
+            headers,
+            baseline_transcript=baseline_transcript,
+            skill_stats=skill_stats,
+            baseline_stats=baseline_stats,
+            workload=workload,
+            owner=owner,
+        )
+    v = verdict.get("verdict")
+    log(f"{name}: verdict = {v} ({verdict.get('summary', '')[:80]})")
+    if verdict.get("approval_required"):
+        # An unattended audit is not authority for an action influenced by the
+        # skill under test. Preserve the skill's current publication/confidence
+        # state and route the exact action to the manual test UI instead of
+        # letting a safety pause demote, rewrite, or auto-publish the skill.
+        skills_manager.set_audit(
+            name,
+            "inconclusive",
+            by_teacher=False,
+            worker_model=model,
+            owner=owner,
+            audit_summary=verdict.get("summary") or "The test requires approval for an external action.",
+        )
+        status = skill.get("status") or "draft"
+        log(f"{name}: {status} unchanged — exact action needs manual approval")
+        return {
+            "skill": name,
+            "result": "approval_required",
+            "verdict": verdict,
+            "status": status,
+        }
+    if v == "pass":
+        # Procedure works. If the reviewer still flagged metadata (tags/category/
+        # when_to_use/description), do ONE fixer pass to correct the frontmatter
+        # without re-testing — a metadata-only fix can't break a passing run.
+        meta_issues = [i for i in (verdict.get("issues") or []) if str(i).lower().lstrip().startswith("metadata:")]
+        if meta_issues:
+            log(f"{name}: pass, but fixing {len(meta_issues)} metadata issue(s)…")
+            fixed = await deps.improve_markdown(
+                md, verdict, transcript, url, model, headers, workload=workload,
+                owner=owner,
+            )
+            if fixed and fixed.strip() != md.strip():
+                deps.apply_markdown(skills_manager, name, fixed, owner)
+        _set_conf(0.95)
+        skills_manager.set_audit(
+            name,
+            "pass",
+            by_teacher=False,
+            worker_model=model,
+            owner=owner,
+            **deps.verdict_efficiency(verdict),
+        )
+        refreshed = next((s for s in skills_manager.load(owner=owner) if s.get("name") == name), None)
+        status = deps.finalize_status(skills_manager, name, owner, "pass", 0.95, (refreshed or {}).get("necessity"), verdict)
+        log(f"{name}: {status} — confidence 95%")
+        return {"skill": name, "result": "pass", "verdict": verdict, "confidence": 0.95, "status": status}
+    if v in ("unknown", "inconclusive"):
+        skills_manager.set_audit(
+            name,
+            "inconclusive",
+            by_teacher=False,
+            worker_model=model,
+            owner=owner,
+            **deps.verdict_efficiency(verdict),
+        )
+        status = deps.finalize_status(skills_manager, name, owner, "inconclusive", skill.get("confidence") or 0.0, skill.get("necessity"))
+        log(f"{name}: {status} — inconclusive")
+        return {"skill": name, "result": "inconclusive", "verdict": verdict, "status": status}
+
+    # Self-edit + retry.
+    log(f"{name}: self-editing to fix issues…")
+    new_md = await deps.improve_markdown(
+        md, verdict, transcript, url, model, headers, workload=workload,
+        owner=owner,
+    )
+    if new_md and new_md.strip() != md.strip() and deps.apply_markdown(skills_manager, name, new_md, owner):
+        md = new_md
+        transcript, verdict = await deps.run_test_once(
+            md, task, url, model, headers, owner, workload=workload,
+        )
+        v = verdict.get("verdict")
+        log(f"{name}: retry (self) = {v}")
+        if v == "pass":
+            _set_conf(0.85)
+            skills_manager.set_audit(
+                name,
+                "pass",
+                by_teacher=False,
+                worker_model=model,
+                owner=owner,
+                **deps.verdict_efficiency(verdict),
+            )
+            refreshed = next((s for s in skills_manager.load(owner=owner) if s.get("name") == name), None)
+            status = deps.finalize_status(skills_manager, name, owner, "pass", 0.85, (refreshed or {}).get("necessity"), verdict)
+            log(f"{name}: {status} — confidence 85% after self-edit")
+            return {"skill": name, "result": "pass_after_self_edit", "verdict": verdict, "confidence": 0.85, "status": status}
+
+    # Teacher escalation (if a distinct teacher model is configured). The
+    # teacher only REWRITES the skill — it does NOT run the test. The point is
+    # to verify the regular (student) model can now succeed with the teacher's
+    # improved procedure, so the retry runs on the worker model, not the teacher.
+    teacher_ran = False
+    if teacher and teacher[0] and teacher[1] and (teacher[1] != model or teacher[0] != url):
+        teacher_ran = True
+        t_url, t_model, t_headers = teacher
+        log(f"{name}: teacher {t_model} rewriting the skill…")
+        t_md = await deps.improve_markdown(
+            md, verdict, transcript, t_url, t_model, t_headers, workload=workload,
+            owner=owner,
+        )
+        if t_md and t_md.strip() != md.strip() and deps.apply_markdown(skills_manager, name, t_md, owner):
+            md = t_md
+        # Re-test with the STUDENT model (the model the skill runs under in use).
+            transcript, verdict = await deps.run_test_once(
+                md, task, url, model, headers, owner, workload=workload,
+            )
+        v = verdict.get("verdict")
+        log(f"{name}: retry on student after teacher rewrite = {v}")
+        if v == "pass":
+            _set_conf(0.8)
+            skills_manager.set_audit(
+                name,
+                "pass",
+                by_teacher=True,
+                worker_model=model,
+                teacher_model=t_model,
+                owner=owner,
+                **deps.verdict_efficiency(verdict),
+            )
+            refreshed = next((s for s in skills_manager.load(owner=owner) if s.get("name") == name), None)
+            status = deps.finalize_status(skills_manager, name, owner, "pass", 0.8, (refreshed or {}).get("necessity"), verdict)
+            log(f"{name}: {status} — confidence 80% after teacher rewrite")
+            return {"skill": name, "result": "pass_after_teacher", "verdict": verdict, "confidence": 0.8, "status": status}
+
+    # Still failing → demote to draft + low confidence + flag (do NOT delete).
+    try:
+        skills_manager.update_skill(name, {"status": "draft", "confidence": 0.35}, owner=owner)
+    except Exception:
+        pass
+    skills_manager.set_audit(
+        name, v or "fail", by_teacher=teacher_ran,
+        worker_model=model,
+        teacher_model=(teacher[1] if teacher_ran and teacher else ""),
+        owner=owner,
+        **deps.verdict_efficiency(verdict),
+    )
+    log(f"{name}: flagged — confidence lowered, kept as draft for manual review")
+    return {"skill": name, "result": "flagged", "verdict": verdict, "confidence": 0.35}
+
+
+async def run_audit_batch(key, skills_manager, names, url, model, headers, teacher, owner,
+                             workload: str = "foreground", *, deps: AuditDependencies):
+    """Background: audit each named skill in sequence, recording progress."""
+    import asyncio as _asyncio
+    import time as _time
+
+    job = deps.audit_jobs.get(key)
+    if job is None:
+        return
+
+    def log(msg):
+        job["log"].append(msg)
+        if len(job["log"]) > 1000:
+            del job["log"][0:len(job["log"]) - 1000]
+
+    cancelled = False
+    try:
+        for nm in names:
+            if job.get("cancel"):
+                cancelled = True
+                log("(cancelled)")
+                break
+            job["current"] = nm
+            skills = skills_manager.load(owner=owner)
+            sk = next((s for s in skills if s.get("name") == nm), None)
+            if not sk:
+                continue
+            try:
+                res = await deps.audit_one(
+                    skills_manager, sk, url, model, headers, teacher, owner, log,
+                    workload=workload,
+                )
+            except _asyncio.CancelledError:
+                cancelled = True
+                job["cancel"] = True
+                log("(cancelled)")
+                raise
+            except SkillAuditUnavailable as e:
+                job["unavailable"] = str(e)
+                log(f"Audit paused: {e}. Skill verdicts unchanged; retry when the model is available.")
+                break
+            except Exception as e:
+                log(f"{nm}: error — {e}")
+                res = {"skill": nm, "result": "error"}
+                skills_manager.set_audit(nm, "inconclusive", worker_model=model, owner=owner)
+            try:
+                refreshed = next((s for s in skills_manager.load(owner=owner) if s.get("name") == nm), None)
+                if refreshed:
+                    res["skill_state"] = {
+                        "name": refreshed.get("name"),
+                        "status": refreshed.get("status"),
+                        "confidence": refreshed.get("confidence"),
+                        "audit_verdict": refreshed.get("audit_verdict"),
+                        "audit_by_teacher": refreshed.get("audit_by_teacher"),
+                        "audit_worker_model": refreshed.get("audit_worker_model"),
+                        "audit_teacher_model": refreshed.get("audit_teacher_model"),
+                        "audited_at": refreshed.get("audited_at"),
+                        "saved_turns": refreshed.get("saved_turns"),
+                        "saved_tool_calls": refreshed.get("saved_tool_calls"),
+                        "baseline_verdict": refreshed.get("baseline_verdict"),
+                        "usefulness": refreshed.get("usefulness"),
+                        "necessity": refreshed.get("necessity"),
+                    }
+            except Exception:
+                pass
+            job["results"].append(res)
+            job["done"] = len(job["results"])
+    except _asyncio.CancelledError:
+        cancelled = True
+    finally:
+        if not cancelled and not job.get("cancel") and not job.get("unavailable"):
+            try:
+                deps.finalize_batch(skills_manager, job.get("results") or [], owner, log)
+            except Exception:
+                logger.warning("Could not finalize skills audit batch", exc_info=True)
+        job["current"] = None
+        job["status"] = "cancelled" if cancelled or job.get("cancel") else "error" if job.get("unavailable") else "done"
+        job["finished"] = _time.time()
+        job.pop("task", None)
+
+
+async def run_scheduled_skill_audit(skills_manager: SkillsManager,
+                                    owner: Optional[str] = None,
+                                    max_skills: int = 8, *, deps: AuditDependencies) -> dict:
+    """Nightly audit pass. Audits the LEAST-recently-audited skills first and
+    caps the batch so it rotates through the library over successive nights
+    instead of re-checking the same ones every run. Reuses the same job store
+    the manual 'Audit all' button uses, so its progress shows in the UI too."""
+    import time as _time
+
+    key = (owner or "",)
+    existing = deps.audit_jobs.get(key)
+    if existing and existing.get("status") == "running":
+        logger.info("Scheduled skill audit skipped — a run is already active.")
+        return {"status": "running", "skipped": True}
+
+    try:
+        url, model, headers, teacher = deps.resolve_models(owner=owner)
+    except ValueError as e:
+        logger.info(f"Scheduled skill audit skipped — {e}")
+        return {"status": "skipped", "reason": str(e)}
+
+    from services.memory.skill_lifecycle import automatic_audit_candidates
+    skills = automatic_audit_candidates(skills_manager.load(owner=owner), limit=max_skills)
+    names = [s["name"] for s in skills]
+    if not names:
+        return {"status": "done", "total": 0}
+
+    deps.audit_jobs[key] = {
+        "status": "running", "scope": "scheduled", "model": model,
+        "teacher": teacher[1] if teacher else None,
+        "total": len(names), "done": 0, "current": None,
+        "results": [], "log": [f"Nightly audit of {len(names)} least-recently-checked skill(s) with {model}"
+                               + (f"; teacher {teacher[1]}" if teacher else "")],
+        "started": _time.time(), "cancel": False,
+    }
+    logger.info(f"Scheduled skill audit starting: {len(names)} skill(s) (owner={owner or 'all'})")
+    await deps.run_batch(
+        key, skills_manager, names, url, model, headers, teacher, owner,
+        workload="background",
+    )
+    job = deps.audit_jobs.get(key, {})
+    return {"status": "done", "total": len(names), "results": job.get("results", [])}

services/skills/dependencies.py

@@ -0,0 +1,35 @@
+"""Explicit collaborators for execution and audit coordination.
+
+The transport compatibility bindings supply the original patchable functions.
+Services never import the skills transport module or copy credentials into a
+second state store.
+"""
+from dataclasses import dataclass
+from typing import Callable
+
+
+@dataclass(frozen=True)
+class AuditDependencies:
+    test_jobs: dict
+    audit_jobs: dict
+    test_task: Callable
+    test_messages: Callable
+    baseline_messages: Callable
+    evaluate_run: Callable
+    evaluate_necessity: Callable
+    evaluate_retrieval_precision: Callable
+    should_check_retrieval_precision: Callable
+    improve_markdown: Callable
+    apply_markdown: Callable
+    duplicate_blocker: Callable
+    finalize_status: Callable
+    finalize_batch: Callable
+    auto_publish_policy: Callable
+    utility_blocker: Callable
+    verdict_efficiency: Callable
+    run_test_job: Callable
+    run_arm: Callable
+    run_test_once: Callable
+    audit_one: Callable
+    run_batch: Callable
+    resolve_models: Callable

services/skills/evaluation.py

@@ -0,0 +1,464 @@
+"""Model grading and advisory skill assessment with explicit offline call seams."""
+
+import logging
+import re
+from typing import Optional
+
+from src.text_helpers import strip_closed_think_blocks
+from src.utility_effort import effort_for_call
+
+logger = logging.getLogger(__name__)
+
+# Last-resort verdict extraction from a teacher/verifier model's prose (run when
+# JSON parsing fails). `["\'\s:]*` already consumes whitespace, so the original
+# trailing `\s*` made two adjacent \s-matching quantifiers that backtrack O(n^2)
+# on a `verdict` + whitespace flood in untrusted model output (CodeQL
+# py/polynomial-redos). Without it a single unbounded quantifier remains — the
+# matched text is identical, and the scan is linear.
+_VERDICT_PROSE_RE = re.compile(
+    r'verdict["\'\s:]*["\']?(pass|needs_work|fail|inconclusive)', re.I
+)
+
+
+def verdict_efficiency(verdict: Optional[dict]) -> dict:
+    if not isinstance(verdict, dict):
+        return {}
+    out = {"audit_summary": str(verdict.get("summary") or "")[:2000]}
+    for key in ("saved_turns", "saved_tool_calls"):
+        if key not in verdict:
+            continue
+        try:
+            out[key] = int(verdict.get(key))
+        except (TypeError, ValueError):
+            pass
+    baseline = str(verdict.get("baseline_verdict") or "").lower().strip()
+    if baseline in {"better", "same", "worse", "unknown"}:
+        out["baseline_verdict"] = baseline
+    if "usefulness" in verdict:
+        try:
+            out["usefulness"] = max(0.0, min(1.0, float(verdict.get("usefulness"))))
+        except (TypeError, ValueError):
+            pass
+    return out
+
+
+def parse_run_verdict(raw: str):
+    """Return a final result dict on success, or None if unparseable."""
+    import json as _json
+    import re as _re
+    _VERDICTS = ("pass", "needs_work", "fail", "inconclusive")
+
+    text = (raw or '')
+    # Strip closed think blocks. If a <think> was opened but never closed
+    # (the model ran out of budget mid-reasoning), drop everything from it
+    # onward so its stray braces don't poison JSON extraction.
+    text = strip_closed_think_blocks(text)
+    text = _re.sub(r'<think(?:ing)?>[\s\S]*$', '', text, flags=_re.I).strip()
+
+    def _coerce(d):
+        return d if (isinstance(d, dict) and "verdict" in d) else None
+
+    data = None
+    # Scan every balanced {...} candidate and keep the LAST one that parses
+    # and carries a "verdict" — the transcript is full of JSON API bodies,
+    # so a naive first-brace/last-brace span almost never parses.
+    for m in _re.finditer(r'\{[\s\S]*?\}', text):
+        frag = m.group(0)
+        for cand in (frag, _re.sub(r',(\s*[}\]])', r'\1', frag)):
+            try:
+                d = _coerce(_json.loads(cand))
+            except Exception:
+                d = None
+            if d is not None:
+                data = d
+    # Fallback to the greedy outermost span (handles nested objects the
+    # non-greedy scan above splits apart).
+    if data is None:
+        a, b = text.find('{'), text.rfind('}')
+        if a >= 0 and b > a:
+            frag = text[a:b + 1]
+            for cand in (frag, _re.sub(r',(\s*[}\]])', r'\1', frag)):
+                try:
+                    d = _coerce(_json.loads(cand))
+                except Exception:
+                    d = None
+                if d is not None:
+                    data = d
+                    break
+
+    v = str(data.get("verdict", "")).lower().strip() if isinstance(data, dict) else None
+    # Last resort: pull the verdict keyword straight out of the prose so a
+    # clearly-decided run isn't thrown away as "unparseable".
+    if v not in _VERDICTS:
+        km = _VERDICT_PROSE_RE.search(text)
+        if km:
+            v = km.group(1).lower()
+            if data is None:
+                data = {}
+    if not isinstance(data, dict) or v not in _VERDICTS:
+        return None
+    try:
+        conf = float(data.get("confidence", 0))
+    except (TypeError, ValueError):
+        conf = 0
+    try:
+        usefulness = float(data.get("usefulness", 0.0))
+    except (TypeError, ValueError):
+        usefulness = 0.0
+    try:
+        saved_turns = int(data.get("saved_turns", 0))
+    except (TypeError, ValueError):
+        saved_turns = 0
+    try:
+        saved_tool_calls = int(data.get("saved_tool_calls", 0))
+    except (TypeError, ValueError):
+        saved_tool_calls = 0
+    return {
+        "verdict": v,
+        "confidence": max(0.0, min(1.0, conf)),
+        "summary": str(data.get("summary", ""))[:400],
+        "issues": [str(x)[:200] for x in (data.get("issues") or []) if str(x).strip()][:8],
+        "baseline_verdict": (
+            str(data.get("baseline_verdict", "unknown")).lower().strip()
+            if str(data.get("baseline_verdict", "unknown")).lower().strip() in {"better", "same", "worse", "unknown"}
+            else "unknown"
+        ),
+        "usefulness": max(0.0, min(1.0, usefulness)),
+        "saved_turns": saved_turns,
+        "saved_tool_calls": saved_tool_calls,
+    }
+
+
+async def evaluate_run(skill_md: str, task: str, transcript: str,
+                          url: str, model: str, headers: Optional[dict],
+                          baseline_transcript: str = "",
+                          skill_stats: Optional[dict] = None,
+                          baseline_stats: Optional[dict] = None,
+                          workload: str = "foreground", owner: Optional[str] = None,
+                          *, _llm_call=None, _effort_for_call=None) -> dict:
+    """LLM-as-judge: grade a skill test run from its transcript. Advisory only.
+
+    Robust against local reasoning models (strips <think>, lenient JSON,
+    generous token budget) — same defensive parsing used elsewhere.
+    """
+    if _llm_call is None:
+        from src.llm_core import llm_call_async
+        _llm_call = llm_call_async
+    _effort_for_call = _effort_for_call or effort_for_call
+
+    sys_prompt = (
+        "You are a strict QA reviewer judging whether an AI 'skill' (a reusable "
+        "procedure) actually works. You are given the SKILL, the TASK it was tested "
+        "on, and the TRANSCRIPT of the agent's run.\n\n"
+        "Judge honestly:\n"
+        "- The functional verdict judges ONLY whether the SKILL RUN completed the "
+        "task correctly and whether the skill procedure is usable. A correct skill "
+        "run is pass even when the baseline is equally good. Record comparative "
+        "value separately in baseline_verdict/usefulness.\n"
+        "- Did following the skill accomplish the task accurately?\n"
+        "- Are the steps clear, correct, and reproducible?\n"
+        "- Did it reference tools/commands that don't exist or that errored?\n"
+        "- Separately, compared with the baseline run WITHOUT the skill, did the skill make "
+        "the agent more accurate, use fewer turns/tool calls, or avoid avoidable "
+        "thrashing?\n"
+        "- Do NOT penalize a skill merely for being broad or generic. A broad "
+        "skill is useful if it makes the agent finish correctly in fewer turns "
+        "or with fewer tools than baseline.\n"
+        "- METADATA: do the frontmatter fields match what the skill actually does? "
+        "Flag wrong/misleading/missing tags, a wrong category, a when_to_use that "
+        "doesn't describe the real trigger, or a description that oversells or "
+        "mismatches the body. List each metadata problem in 'issues' (prefix it "
+        "with 'metadata:'). Metadata problems alone do NOT make the verdict 'fail' "
+        "if the procedure works — note them as issues on an otherwise-passing run.\n\n"
+        "Never use inconclusive merely because the baseline was the same or better. "
+        "IMPORTANT — fairness rule: if the SKILL RUN could NOT proceed because it lacked "
+        "an input or target the test never provided (e.g. there was no document/"
+        "email/data to act on, so the agent reasonably asked for it), that is NOT "
+        "the skill's fault. Return verdict \"inconclusive\" — do NOT mark it fail "
+        "or needs_work. Only judge the skill's PROCEDURE; reserve fail/needs_work "
+        "for when the steps themselves are wrong, vague, or reference missing tools.\n\n"
+        "If you need to reason, do it inside <think></think> FIRST. Then output "
+        "ONLY this JSON (no fences):\n"
+        'Set baseline_verdict to how the SKILL RUN compares to the BASELINE RUN.\n\n'
+        '{"verdict": "pass" | "needs_work" | "fail" | "inconclusive", '
+        '"confidence": 0.0-1.0, "summary": "one short sentence", '
+        '"issues": ["short issue", ...], '
+        '"baseline_verdict": "better" | "same" | "worse" | "unknown", '
+        '"usefulness": 0.0-1.0, "saved_turns": integer, "saved_tool_calls": integer}'
+    )
+    # Give the judge plenty of transcript, and when it must trim, keep the TAIL
+    # (the final result lives at the end) plus a bit of the head — truncating to
+    # a short prefix made the judge wrongly call complete runs "incomplete /
+    # missing sections" because it never saw the end.
+    def _clip(t: str, limit: int = 24000) -> str:
+        t = (t or "").strip() or "(no output produced)"
+        if len(t) <= limit:
+            return t
+        head = limit // 4
+        return t[:head] + "\n\n…[transcript trimmed for length]…\n\n" + t[-(limit - head):]
+    skill_stats = skill_stats or {}
+    baseline_stats = baseline_stats or {}
+    user_msg = (
+        f"=== SKILL ===\n{(skill_md or '')[:4000]}\n\n"
+        f"=== TASK ===\n{task}\n\n"
+        f"=== SKILL RUN STATS ===\n"
+        f"turns={skill_stats.get('turns', 'unknown')} "
+        f"tool_calls={skill_stats.get('tool_calls', 'unknown')}\n\n"
+        f"=== SKILL RUN TRANSCRIPT ===\n{_clip(transcript)}\n\n"
+        f"=== BASELINE RUN WITHOUT SKILL STATS ===\n"
+        f"turns={baseline_stats.get('turns', 'unknown')} "
+        f"tool_calls={baseline_stats.get('tool_calls', 'unknown')}\n\n"
+        f"=== BASELINE RUN WITHOUT SKILL TRANSCRIPT ===\n{_clip(baseline_transcript)}"
+    )
+
+    # Two attempts: the first lets the judge reason; if a heavy reasoning model
+    # burns its budget inside <think> and never emits the JSON, the second
+    # forbids thinking and demands the JSON immediately.
+    last_text = ""
+    last_err = None
+    for attempt in range(2):
+        msgs = [{"role": "system", "content": sys_prompt},
+                {"role": "user", "content": user_msg}]
+        if attempt == 1:
+            msgs[0]["content"] = (
+                sys_prompt + "\n\nDO NOT use <think> or any reasoning. Your reply "
+                "must START with '{' and be ONLY the JSON object, nothing else."
+            )
+        try:
+            raw = await _llm_call(
+                # Generous budget so a heavy reasoner can think AND still have
+                # room to emit the JSON afterwards (reasoning tokens come out of
+                # this same cap; the server clamps to its own max).
+                url, model, msgs,
+                temperature=0.1, max_tokens=32768, headers=headers, timeout=180,
+                workload=workload,
+                reasoning_effort=_effort_for_call(url, model, owner),
+            )
+        except Exception as e:
+            # Don't give up on a transient first-attempt error — let the second
+            # (no-think) attempt run before reporting failure.
+            last_err = e
+            continue
+        last_text = (raw or '')
+        parsed = parse_run_verdict(raw)
+        if parsed is not None:
+            return parsed
+
+    if last_err is not None and not last_text:
+        return {"verdict": "unknown", "confidence": 0, "summary": f"Review call failed: {last_err}", "issues": []}
+    return {"verdict": "unknown", "confidence": 0,
+            "summary": "Reviewer returned unparseable output.", "issues": [], "raw": last_text[:300]}
+
+
+async def evaluate_necessity(skill_md: str, others: list, url: str, model: str,
+                                headers: Optional[dict],
+                                workload: str = "foreground", owner: Optional[str] = None,
+                          *, _llm_call=None, _effort_for_call=None) -> Optional[dict]:
+    """Advisory judge: is this skill worth keeping, or is it redundant / trivially
+    unnecessary? Sees the OTHER skills' names+descriptions so it can spot
+    duplicates. Returns {necessary, redundant_with, reason} or None. Never acts —
+    purely a flag the UI surfaces."""
+    import json as _json
+    import re as _re
+    if _llm_call is None:
+        from src.llm_core import llm_call_async
+        _llm_call = llm_call_async
+    _effort_for_call = _effort_for_call or effort_for_call
+
+    catalog = "\n".join(f"- {o.get('name')}: {o.get('description', '')}" for o in others) or "(no other skills)"
+    sys_prompt = (
+        "You assess whether a reusable AI 'skill' (a saved procedure) is worth keeping. "
+        "A skill is UNNECESSARY if it essentially duplicates another skill in the library. "
+        "Do NOT call a skill unnecessary merely because it is broad or generic; broad "
+        "skills can be worth keeping when they make a capable assistant finish accurately "
+        "with fewer turns or fewer tool calls than it would without the skill. A skill IS "
+        "necessary if it captures a reusable trigger, procedure, tool sequence, or "
+        "hard-won detail that could improve future runs.\n\n"
+        "Be conservative: only call it unnecessary when you're confident. Reason in "
+        "<think></think> first if needed, then output ONLY this JSON:\n"
+        '{"necessary": true|false, "redundant_with": ["skill-name", ...], '
+        '"reason": "one short sentence"}'
+    )
+    user_msg = (
+        f"=== SKILL UNDER REVIEW ===\n{(skill_md or '')[:3000]}\n\n"
+        f"=== OTHER SKILLS IN THE LIBRARY ===\n{catalog[:4000]}"
+    )
+    try:
+        raw = await _llm_call(
+            url, model,
+            [{"role": "system", "content": sys_prompt}, {"role": "user", "content": user_msg}],
+            temperature=0.1, max_tokens=8192, headers=headers, timeout=120,
+            workload=workload,
+            reasoning_effort=_effort_for_call(url, model, owner),
+        )
+    except Exception as e:
+        logger.warning(f"Necessity check failed: {e}")
+        return None
+    text = strip_closed_think_blocks(raw or '')
+    text = _re.sub(r'<think(?:ing)?>[\s\S]*$', '', text, flags=_re.I).strip()
+    data = None
+    a, b = text.find('{'), text.rfind('}')
+    if a >= 0 and b > a:
+        frag = text[a:b + 1]
+        for cand in (frag, _re.sub(r',(\s*[}\]])', r'\1', frag)):
+            try:
+                data = _json.loads(cand)
+                break
+            except Exception:
+                continue
+    if not isinstance(data, dict) or "necessary" not in data:
+        return None
+    return {
+        "necessary": bool(data.get("necessary", True)),
+        "redundant_with": [str(x)[:80] for x in (data.get("redundant_with") or []) if str(x).strip()][:6],
+        "reason": str(data.get("reason", ""))[:200],
+    }
+
+
+def should_check_retrieval_precision(skill: dict) -> bool:
+    """Cheap prefilter for the expensive retrieval-precision judge.
+
+    Skills with broad tags like "network" or "document" are the ones most likely
+    to over-inject. Narrow command/vendor tags alone are fine.
+    """
+    broad = {
+        "arch", "arch linux", "linux", "network", "networking", "wifi",
+        "installation", "install", "system", "ssh", "document", "documents",
+        "search", "email", "calendar", "gpu", "server", "python",
+    }
+    if not isinstance(skill, dict):
+        return False
+    tags = {str(t or "").strip().lower() for t in (skill.get("tags") or [])}
+    if tags & broad:
+        return True
+    text = " ".join([
+        str(skill.get("name") or ""),
+        str(skill.get("description") or ""),
+        str(skill.get("when_to_use") or ""),
+    ]).lower()
+    return sum(1 for t in broad if t in text) >= 2
+
+
+async def evaluate_retrieval_precision(skill_md: str, others: list,
+                                          url: str, model: str,
+                                          headers: Optional[dict],
+                                          workload: str = "foreground", owner: Optional[str] = None,
+                          *, _llm_call=None, _effort_for_call=None) -> Optional[dict]:
+    """Advisory judge: would this skill's metadata make retrieval over-select it?
+
+    This is distinct from "does the procedure work?". It asks whether tags,
+    description, and when_to_use are specific enough that the skill won't be
+    injected into adjacent but wrong tasks.
+    """
+    import json as _json
+    import re as _re
+    if _llm_call is None:
+        from src.llm_core import llm_call_async
+        _llm_call = llm_call_async
+    _effort_for_call = _effort_for_call or effort_for_call
+
+    catalog = "\n".join(f"- {o.get('name')}: {o.get('description', '')}" for o in others[:80]) or "(no other skills)"
+    sys_prompt = (
+        "You are auditing retrieval metadata for a reusable AI skill. The app selects "
+        "skills by matching user requests against the skill name, description, tags, "
+        "when_to_use, and procedure text. Judge whether this skill is likely to be "
+        "over-selected for nearby but wrong requests.\n\n"
+        "Focus ONLY on metadata/retrieval precision, not whether the procedure works. "
+        "Flag broad tags such as network, installation, system, document, search, ssh, "
+        "python, gpu, or server when they would cause this narrow skill to match too "
+        "many adjacent tasks. Recommend narrower tags/when_to_use wording. Compare "
+        "against the other skills to spot boundaries.\n\n"
+        "Return ok=true only when the trigger metadata is narrow enough. If not ok, "
+        "issues MUST start with 'metadata: retrieval:' and be actionable. Output ONLY JSON:\n"
+        '{"ok": true|false, "summary": "one short sentence", "issues": ["metadata: retrieval: ..."]}'
+    )
+    user_msg = (
+        f"=== SKILL UNDER REVIEW ===\n{(skill_md or '')[:5000]}\n\n"
+        f"=== OTHER SKILLS IN LIBRARY ===\n{catalog[:5000]}\n\n"
+        "Decide if this skill's retrieval metadata should be narrowed so it only "
+        "fires for its intended scenario and not for adjacent skills above."
+    )
+    try:
+        raw = await _llm_call(
+            url, model,
+            [{"role": "system", "content": sys_prompt}, {"role": "user", "content": user_msg}],
+            temperature=0.1, max_tokens=4096, headers=headers, timeout=90,
+            workload=workload,
+            reasoning_effort=_effort_for_call(url, model, owner),
+        )
+    except Exception as e:
+        logger.warning(f"Retrieval precision check failed: {e}")
+        return None
+    text = strip_closed_think_blocks(raw or '')
+    text = _re.sub(r'<think(?:ing)?>[\s\S]*$', '', text, flags=_re.I).strip()
+    data = None
+    a, b = text.find('{'), text.rfind('}')
+    if a >= 0 and b > a:
+        frag = text[a:b + 1]
+        for cand in (frag, _re.sub(r',(\s*[}\]])', r'\1', frag)):
+            try:
+                data = _json.loads(cand)
+                break
+            except Exception:
+                continue
+    if not isinstance(data, dict) or "ok" not in data:
+        return None
+    return {
+        "ok": bool(data.get("ok")),
+        "summary": str(data.get("summary", ""))[:300],
+        "issues": [str(x)[:220] for x in (data.get("issues") or []) if str(x).strip()][:6],
+    }
+
+
+async def improve_markdown(skill_md: str, verdict: dict, transcript: str, url, model, headers,
+                            workload: str = "foreground", owner: Optional[str] = None,
+                          *, _llm_call=None, _effort_for_call=None):
+    """Have a model rewrite SKILL.md to fix the reviewer's issues. Returns the
+    corrected markdown, or None if it couldn't produce a usable change."""
+    import re as _re
+    if _llm_call is None:
+        from src.llm_core import llm_call_async
+        _llm_call = llm_call_async
+    _effort_for_call = _effort_for_call or effort_for_call
+    issues = "\n".join("- " + str(i) for i in (verdict.get("issues") or []))
+    sys_prompt = (
+        "You are improving a reusable AI SKILL written in Markdown (frontmatter + body). "
+        "A QA reviewer found problems after a test run. Rewrite the SKILL.md to fix them: "
+        "make vague steps concrete, correct or remove references to tools that don't exist, "
+        "ensure the procedure is reproducible. "
+        "Keep the `name` field EXACTLY as-is (it is the skill's identity / filename). You MAY "
+        "correct the OTHER frontmatter — tags, category, when_to_use, description — when the "
+        "reviewer flagged them (issues prefixed 'metadata:') or they don't match the body; keep "
+        "retrieval metadata narrow: remove broad tags that would over-select the skill, and make "
+        "`when_to_use` say when NOT to use the skill if adjacent tasks are easy to confuse. Keep "
+        "valid frontmatter structure. Do NOT invent capabilities the agent lacks. Reason in "
+        "<think></think> first if needed, then output ONLY the full corrected SKILL.md (no "
+        "fences, no commentary)."
+    )
+    user_msg = (
+        f"=== CURRENT SKILL.md ===\n{skill_md}\n\n"
+        f"=== REVIEWER VERDICT ===\n{verdict.get('summary', '')}\nIssues:\n{issues}\n\n"
+        f"=== TEST TRANSCRIPT ===\n{(transcript or '')[:6000]}"
+    )
+    try:
+        raw = await _llm_call(url, model,
+                                   [{"role": "system", "content": sys_prompt},
+                                    {"role": "user", "content": user_msg}],
+                                   temperature=0.2, max_tokens=16384, headers=headers, timeout=180,
+                                   workload=workload,
+                                   reasoning_effort=_effort_for_call(url, model, owner))
+    except Exception as e:
+        logger.warning(f"Audit: improve call failed: {e}")
+        return None
+    text = strip_closed_think_blocks(raw or '')
+    text = _re.sub(r'<think(?:ing)?>[\s\S]*$', '', text, flags=_re.I)
+    text = _re.sub(r'</think(?:ing)?>', '', text, flags=_re.I).strip()
+    if text.startswith("```"):
+        text = text.split("\n", 1)[-1].rsplit("```", 1)[0].strip()
+    # Some reasoning models still prepend analysis or echo the old skill before
+    # the final corrected document. Keep only the last complete-looking
+    # frontmatter document so reviewer prose never gets persisted into SKILL.md.
+    starts = list(_re.finditer(r'(?m)^---\s*\n(?=[\s\S]*?^name\s*:)', text))
+    if starts:
+        text = text[starts[-1].start():].strip()
+    return text or None

services/skills/job_coordinator.py

@@ -0,0 +1,216 @@
+"""Transient job transitions and public polling projections.
+
+HTTP ownership and approval validation happen before these operations. These
+functions use the supplied registries and preserve the existing task ownership.
+"""
+import asyncio as _asyncio
+import logging
+import time as _time
+
+from src.prompt_security import untrusted_context_message
+from .dependencies import AuditDependencies
+
+logger = logging.getLogger(__name__)
+
+
+def start_test_job(name, md, task, url, model, headers, user, skills_manager,
+                   request_authority, *, deps: AuditDependencies):
+    key = (user or "", name)
+    previous_job = deps.test_jobs.get(key) or {}
+    previous_approval = previous_job.get("approval") or {}
+    if previous_approval.get("approval_id"):
+        try:
+            from src.tool_approvals import tool_approval_store
+            tool_approval_store.consume(
+                previous_approval["approval_id"],
+                decision="deny",
+                owner=user,
+                session_id=None,
+            )
+        except Exception:
+            logger.debug("Could not retire replaced skill approval", exc_info=True)
+    deps.test_jobs[key] = {
+        "status": "running",
+        "task": task,
+        "model": model,
+        "skill": name,
+        "started": _time.time(),
+        "log": [{"type": "skill_test_start", "task": task, "skill": name, "model": model}],
+        "verdict": None,
+        "_run": {
+            "md": md,
+            "url": url,
+            "model": model,
+            "headers": headers,
+            "owner": user,
+            "request_authority": request_authority,
+        },
+    }
+    _asyncio.create_task(deps.run_test_job(
+        key, name, md, task, url, model, headers, user, skills_manager,
+        request_authority=request_authority))
+    return {"ok": True, "status": "running", "skill": name, "model": model}
+
+
+def continue_test_job(job, key, name, decision, expected, exact_approval,
+                      skills_manager, *, deps: AuditDependencies):
+    job.pop("approval", None)
+    if decision == "deny":
+        job.pop("_transcript", None)
+        job.pop("_run", None)
+        job["log"].append({
+            "type": "approval_denied",
+            "text": "Exact action denied; the skill test stopped without executing it.",
+        })
+        job["verdict"] = {
+            "verdict": "inconclusive",
+            "confidence": 1.0,
+            "summary": "The test stopped because its exact action was denied.",
+            "issues": [],
+        }
+        job["status"] = "done"
+        return {"ok": True, "status": "done", "decision": "deny"}
+
+    run = job.get("_run") or {}
+    transcript = job.pop("_transcript", [])
+    # stream_agent_loop owns its per-round message list internally. Rebuild
+    # continuation context from the original untrusted skill plus the
+    # accumulated transcript so repeated approvals do not lose earlier
+    # approved results, while keeping every transcript byte tainted.
+    messages = deps.test_messages(
+        run.get("md", ""),
+        job.get("task", ""),
+    )
+    if transcript:
+        messages.append(untrusted_context_message(
+            "skill test transcript",
+            "".join(str(item) for item in transcript),
+        ))
+    messages.extend([
+        {
+            "role": "assistant",
+            "content": str(expected.get("question") or "Allow this exact action once?"),
+        },
+        {
+            "role": "user",
+            "content": (
+                f"Approved the exact {exact_approval.pending.tool_name} "
+                "action shown above once."
+            ),
+        },
+    ])
+    job["status"] = "running"
+    job["log"].append({
+        "type": "approval_granted",
+        "text": (
+            f"Approved exact {exact_approval.pending.tool_name} action once; "
+            "resuming test."
+        ),
+    })
+    _asyncio.create_task(deps.run_test_job(
+        key,
+        name,
+        run.get("md", ""),
+        job.get("task", ""),
+        run.get("url"),
+        run.get("model"),
+        run.get("headers"),
+        run.get("owner"),
+        skills_manager,
+        messages=messages,
+        transcript=transcript,
+        exact_approval=exact_approval,
+    ))
+    return {"ok": True, "status": "running", "decision": "approve"}
+
+
+def test_status(user, name, *, deps: AuditDependencies):
+    job = deps.test_jobs.get((user or "", name))
+    if not job:
+        return {"status": "none"}
+    return {
+        "status": job["status"],
+        "task": job.get("task"),
+        "model": job.get("model"),
+        "log": job.get("log", []),
+        "verdict": job.get("verdict"),
+        "approval": job.get("approval"),
+    }
+
+
+def select_audit_names(skills_manager, user, scope, requested_names, skip_audited):
+    skills = skills_manager.load(owner=user)
+    # Built-ins are tracked, pre-approved application procedures. They do
+    # not consume audit turns and cannot be demoted by an audit result.
+    auditable_skills = [s for s in skills if s.get("source") != "builtin"]
+    by_name = {s.get("name"): s for s in auditable_skills if s.get("name")}
+    if isinstance(requested_names, list):
+        names = []
+        seen = set()
+        for raw in requested_names:
+            nm = str(raw or "").strip()
+            if not nm or nm in seen or nm not in by_name:
+                continue
+            if scope not in ("all", "selected") and (by_name[nm].get("status") or "draft") == "published":
+                continue
+            if skip_audited and by_name[nm].get("audit_verdict"):
+                continue
+            names.append(nm)
+            seen.add(nm)
+        scope = "selected" if requested_names else scope
+    elif scope == "all":
+        names = [
+            s.get("name") for s in auditable_skills
+            if s.get("name") and (not skip_audited or not s.get("audit_verdict"))
+        ]
+    else:
+        scope = "unchecked" if scope == "drafts" else scope
+        names = [
+            s.get("name") for s in auditable_skills
+            if s.get("name")
+            and (s.get("status") or "draft") != "published"
+            and not s.get("audit_verdict")
+        ]
+    return names, scope
+
+
+def start_audit_job(key, skills_manager, names, url, model, headers, teacher, user,
+                    scope, *, deps: AuditDependencies):
+    if not names:
+        return {"ok": True, "status": "done", "total": 0, "results": [], "log": ["No skills to audit."]}
+
+    deps.audit_jobs[key] = {
+        "status": "running", "scope": scope, "model": model,
+        "teacher": teacher[1] if teacher else None,
+        "total": len(names), "done": 0, "current": None,
+        "results": [], "log": [f"Auditing {len(names)} skill(s) with {model}" + (f"; teacher {teacher[1]}" if teacher else "")],
+        "started": _time.time(), "cancel": False,
+    }
+    task = _asyncio.create_task(deps.run_batch(key, skills_manager, names, url, model, headers, teacher, user))
+    deps.audit_jobs[key]["task"] = task
+    return {"ok": True, "status": "running", "total": len(names), "model": model}
+
+
+def audit_status(user, *, deps: AuditDependencies):
+    job = deps.audit_jobs.get((user or "",))
+    if not job:
+        return {"status": "none"}
+    return {
+        "status": job["status"], "scope": job.get("scope"),
+        "total": job.get("total", 0), "done": job.get("done", 0),
+        "current": job.get("current"), "model": job.get("model"), "teacher": job.get("teacher"),
+        "results": job.get("results", []), "log": job.get("log", []),
+        "started": job.get("started"), "finished": job.get("finished"),
+    }
+
+
+def cancel_audit(user, *, deps: AuditDependencies):
+    job = deps.audit_jobs.get((user or "",))
+    if job:
+        job["cancel"] = True
+        job["status"] = "cancelled"
+        job["current"] = None
+        task = job.get("task")
+        if task and not task.done():
+            task.cancel()
+    return {"ok": True, "status": "cancelled" if job else "none"}

services/skills/model_selection.py

@@ -0,0 +1,75 @@
+"""Owner-scoped audit worker and teacher endpoint resolution."""
+
+import logging
+
+
+logger = logging.getLogger(__name__)
+
+def resolve_audit_models(owner=None, model_spec=None, endpoint_url=None):
+    """Resolve (url, model, headers, teacher) for an audit run from Settings.
+
+    Worker = Utility model (falling back to Default, normalized to a served
+    model id); teacher = the optional Settings → Teacher Model config. Shared
+    by the manual /audit-all route and scheduled/event audits. Raises
+    ValueError if no worker model.
+    """
+    from src.endpoint_resolver import resolve_endpoint, resolve_utility_fallback_candidates
+    if model_spec and endpoint_url:
+        # Scheduled tasks store the endpoint URL and model separately. Resolve
+        # the endpoint directly so an explicit task choice cannot be replaced
+        # by the global Utility setting.
+        from src.endpoint_resolver import build_headers, resolve_endpoint_runtime
+        from src.database import ModelEndpoint, SessionLocal
+        from src.endpoint_resolver import normalize_base, same_endpoint_base
+        from src.auth_helpers import owner_filter
+        url = endpoint_url
+        model = model_spec
+        headers = {}
+        db = SessionLocal()
+        try:
+            query = db.query(ModelEndpoint).filter(ModelEndpoint.is_enabled == True)
+            for ep in owner_filter(query, ModelEndpoint, owner).all():
+                base = normalize_base(getattr(ep, "base_url", "") or "")
+                if same_endpoint_base(url, base):
+                    runtime_base, api_key = resolve_endpoint_runtime(ep, owner=owner)
+                    headers = build_headers(api_key, runtime_base or base)
+                    break
+        finally:
+            db.close()
+    elif model_spec:
+        from src.ai_interaction import _resolve_model
+        url, model, headers = _resolve_model(str(model_spec), owner=owner)
+    else:
+        url, model, headers = resolve_endpoint("utility", owner=owner)
+    if not url or not model:
+        # Utility fallbacks are an explicit part of the user's model
+        # configuration. Audits must use the same chain as other background
+        # work instead of treating an empty primary Utility slot as fatal.
+        for fallback_url, fallback_model, fallback_headers in resolve_utility_fallback_candidates(owner=owner):
+            url, model, headers = fallback_url, fallback_model, fallback_headers
+            break
+    if not url or not model:
+        raise ValueError("No model configured — set a Default or Utility model in Settings.")
+    try:
+        from src.llm_core import list_model_ids
+        import os as _os
+        _avail = list_model_ids(url, headers=headers)
+        if _avail and model not in _avail:
+            _base = _os.path.basename((model or "").rstrip("/"))
+            model = next((a for a in _avail if _os.path.basename(a.rstrip("/")) == _base), None) or _avail[0]
+    except Exception:
+        pass
+
+    teacher = None
+    try:
+        from src.settings import get_setting
+        if get_setting("teacher_enabled", False):
+            spec = (get_setting("teacher_model", "") or "").strip()
+            if spec:
+                from src.ai_interaction import _resolve_model
+                t_url, t_model, t_headers = _resolve_model(spec, owner=owner)
+                if t_url and t_model:
+                    teacher = (t_url, t_model, t_headers)
+    except Exception as e:
+        logger.warning(f"Audit teacher resolve failed: {e}")
+    return url, model, headers, teacher

services/skills/prompts.py

@@ -0,0 +1,52 @@
+"""Identical audit tasks and explicitly untrusted skill context."""
+
+from src.prompt_security import untrusted_context_message
+
+def _skill_test_task(skill: dict) -> str:
+    """Build the one shared task used by both sides of an audit comparison."""
+    if not isinstance(skill, dict):
+        skill = {}
+    ctx = (skill.get("when_to_use") or skill.get("description") or skill.get("name") or "").strip()
+    return (
+        "Complete this task end-to-end. Use this exact task context and do not ask "
+        "the user for more input. If a harmless fixture is needed, create the "
+        "smallest realistic one that satisfies the context, state it explicitly, "
+        "then complete and verify the result. Do not perform destructive or "
+        "externally visible actions. Task context: " + (ctx or "(general)")
+    )
+
+
+def _skill_baseline_task(skill: dict) -> str:
+    """Backward-compatible alias; both audit arms must receive identical text."""
+    return _skill_test_task(skill)
+
+
+def _skill_test_messages(md: str, task: str) -> list[dict]:
+    """Keep user-editable skill text out of the trusted system role."""
+    return [
+        {
+            "role": "system",
+            "content": (
+                "You are TESTING a skill. Follow the supplied reusable procedure "
+                "to complete the user's task for real, using available tools step "
+                "by step. If the skill is wrong, unclear, or references tools that "
+                "do not exist, do your best; the problems will be reviewed afterward."
+            ),
+        },
+        untrusted_context_message("skill under test", md),
+        {"role": "user", "content": task},
+    ]
+
+
+def _skill_baseline_messages(task: str) -> list[dict]:
+    return [
+        {
+            "role": "system",
+            "content": (
+                "You are completing a baseline audit run. Do not use any saved "
+                "skill text. Solve the user's task using only your normal tools "
+                "and reasoning."
+            ),
+        },
+        {"role": "user", "content": task},
+    ]

services/skills/publication.py

@@ -0,0 +1,349 @@
+"""Audit publication and duplicate coordination through the canonical SkillsManager."""
+
+import logging
+import re
+from typing import Optional
+
+from .dependencies import AuditDependencies
+
+logger = logging.getLogger(__name__)
+
+def auto_publish_policy(owner) -> tuple[bool, float]:
+    """Return (auto_publish_enabled, minimum_confidence) for audit finalization."""
+    try:
+        from routes.prefs_routes import _load_for_user
+        prefs = _load_for_user(owner) or {}
+    except Exception:
+        prefs = {}
+    try:
+        from src.settings import get_setting
+        default_min = get_setting("skill_autosave_min_confidence", 0.85)
+    except Exception:
+        default_min = 0.85
+    enabled = bool(prefs.get("auto_approve_skills", True))
+    try:
+        min_conf = float(prefs.get("skill_min_confidence", default_min))
+    except (TypeError, ValueError):
+        min_conf = 0.85
+    return enabled, max(0.0, min(1.0, min_conf))
+
+
+def duplicate_blocker(skills_manager, name: str, owner) -> Optional[str]:
+    """Cheap duplicate guard matching the UI's duplicate grouping.
+
+    The LLM necessity check catches semantic redundancy, but the UI also has a
+    cheap similarity pass. Use the same broad signal before auto-publishing so
+    a high-scoring lower-priority duplicate stays draft.
+    """
+    import re as _re
+
+    def _tokens(sk: dict) -> set[str]:
+        text = " ".join([
+            str(sk.get("name") or ""),
+            str(sk.get("description") or ""),
+            str(sk.get("when_to_use") or ""),
+            " ".join(sk.get("procedure") or []),
+            " ".join(sk.get("tags") or []),
+        ]).lower()
+        text = _re.sub(r"-\d+\b", "", text)
+        return {
+            t for t in _re.split(r"[^a-z0-9]+", text)
+            if len(t) > 2 and t not in {"the", "and", "with", "for", "from", "using"}
+        }
+
+    def _sim(a: dict, b: dict) -> float:
+        A, B = _tokens(a), _tokens(b)
+        if not A or not B:
+            return 0.0
+        return len(A & B) / max(1, len(A | B))
+
+    def _base(n: str) -> str:
+        return _re.sub(r"-\d+$", "", str(n or ""))
+
+    def _score(sk: dict) -> float:
+        return (
+            (100000 if (sk.get("status") == "published") else 0)
+            + int(sk.get("uses") or 0) * 100
+            + round(float(sk.get("confidence") or 0) * 100)
+            + (-5 if sk.get("audit_by_teacher") else 0)
+            - (len(str(sk.get("name") or "")) / 1000)
+        )
+
+    skills = [s for s in skills_manager.load(owner=owner) if s.get("status") != "binned"]
+    current = next((s for s in skills if (s.get("name") or s.get("id")) == name), None)
+    if not current:
+        return None
+    duplicates = []
+    cur_name = current.get("name") or current.get("id") or name
+    for other in skills:
+        other_name = other.get("name") or other.get("id")
+        if not other_name or other_name == cur_name:
+            continue
+        if _base(cur_name) == _base(other_name) or _sim(current, other) >= 0.38:
+            duplicates.append(other)
+    if not duplicates:
+        return None
+    keeper = sorted([current, *duplicates], key=_score, reverse=True)[0]
+    keeper_name = keeper.get("name") or keeper.get("id") or ""
+    if keeper_name and keeper_name != cur_name:
+        try:
+            skills_manager.set_necessity(
+                cur_name,
+                False,
+                [keeper_name],
+                f"Lower-priority duplicate of {keeper_name}",
+                owner=owner,
+            )
+        except Exception:
+            pass
+        return keeper_name
+    return None
+
+
+def finalize_batch(skills_manager, results: list[dict], owner, log, *, deps: AuditDependencies) -> None:
+    """Draft audited failures, bin duplicate losers, and publish the best copy.
+
+    Binned skills remain on disk for recovery and inspection, but the Skills
+    manager excludes them from retrieval. Only skills actually processed by
+    this audit job are changed here; an unrelated existing skill is never
+    moved just because it resembles an audited one.
+    """
+    import re as _re
+
+    auto_publish, min_conf = deps.auto_publish_policy(owner)
+    current = [
+        s for s in skills_manager.load(owner=owner)
+        if s.get("source") != "builtin" and s.get("status") != "binned"
+    ]
+    by_name = {s.get("name"): s for s in current if s.get("name")}
+    processed = {str(r.get("skill")) for r in results if r.get("skill")}
+    protected = {
+        str(r.get("skill")) for r in results
+        if r.get("skill") and r.get("result") == "approval_required"
+    }
+
+    def tokens(sk: dict) -> set[str]:
+        text = " ".join([
+            str(sk.get("name") or ""), str(sk.get("description") or ""),
+            str(sk.get("when_to_use") or ""), " ".join(sk.get("procedure") or []),
+            " ".join(sk.get("tags") or []),
+        ]).lower()
+        text = _re.sub(r"-\d+\b", "", text)
+        return {
+            t for t in _re.split(r"[^a-z0-9]+", text)
+            if len(t) > 2 and t not in {"the", "and", "with", "for", "from", "using"}
+        }
+
+    def similar(a: dict, b: dict) -> float:
+        left, right = tokens(a), tokens(b)
+        return len(left & right) / max(1, len(left | right)) if left and right else 0.0
+
+    def base(name: str) -> str:
+        return _re.sub(r"-\d+$", "", str(name or ""))
+
+    def score(sk: dict) -> float:
+        try:
+            confidence = float(sk.get("confidence") or 0)
+        except (TypeError, ValueError):
+            confidence = 0.0
+        return (
+            (100000 if sk.get("status") == "published" else 0)
+            + int(sk.get("uses") or 0) * 100
+            + round(confidence * 100)
+            + (-5 if sk.get("audit_by_teacher") else 0)
+            - len(str(sk.get("name") or "")) / 1000
+        )
+
+    # Anything that does not clear the configured policy stays a draft. Drafts
+    # are excluded from retrieval/injection by SkillsManager.index_for().
+    for name in processed - protected:
+        skill = by_name.get(name)
+        if not skill:
+            continue
+        verdict = str(skill.get("audit_verdict") or "").lower()
+        try:
+            confidence = float(skill.get("confidence") or 0)
+        except (TypeError, ValueError):
+            confidence = 0.0
+        if verdict in {"needs_work", "fail"} or (
+            verdict == "pass" and confidence < min_conf
+        ):
+            try:
+                skills_manager.update_skill(name, {"status": "draft"}, owner=owner)
+                log(f"{name}: kept as draft after audit")
+            except Exception:
+                logger.warning("Could not bin audited skill %s", name, exc_info=True)
+
+    # Build the same connected duplicate groups shown by the UI.
+    parent = {s["name"]: s["name"] for s in current}
+
+    def find(name: str) -> str:
+        while parent[name] != name:
+            parent[name] = parent[parent[name]]
+            name = parent[name]
+        return name
+
+    def unite(left: str, right: str) -> None:
+        left, right = find(left), find(right)
+        if left != right:
+            parent[right] = left
+
+    for index, left in enumerate(current):
+        for right in current[index + 1:]:
+            if base(left["name"]) == base(right["name"]) or similar(left, right) >= 0.38:
+                unite(left["name"], right["name"])
+    groups: dict[str, list[dict]] = {}
+    for skill in current:
+        groups.setdefault(find(skill["name"]), []).append(skill)
+
+    for group in groups.values():
+        if len(group) < 2:
+            continue
+        passing = []
+        for skill in group:
+            if skill["name"] not in processed or skill["name"] in protected:
+                continue
+            if str(skill.get("audit_verdict") or "").lower() != "pass":
+                continue
+            try:
+                confidence = float(skill.get("confidence") or 0)
+            except (TypeError, ValueError):
+                confidence = 0.0
+            if confidence >= min_conf:
+                passing.append(skill)
+        if not passing:
+            continue
+        keeper = max(passing, key=score)
+        if auto_publish:
+            try:
+                skills_manager.update_skill(keeper["name"], {"status": "published"}, owner=owner)
+                log(f"{keeper['name']}: auto-approved as best passing duplicate")
+            except Exception:
+                logger.warning("Could not auto-approve skill %s", keeper["name"], exc_info=True)
+        for skill in group:
+            name = skill["name"]
+            if name == keeper["name"] or name not in processed or name in protected:
+                continue
+            try:
+                skills_manager.update_skill(name, {"status": "binned"}, owner=owner)
+                log(f"{name}: moved to bin as duplicate of {keeper['name']}")
+            except Exception:
+                logger.warning("Could not bin duplicate skill %s", name, exc_info=True)
+
+
+def audit_flag_text(*parts) -> str:
+    text_parts = []
+    for part in parts:
+        if isinstance(part, dict):
+            text_parts.extend(str(v or "") for v in part.values())
+        elif isinstance(part, (list, tuple, set)):
+            text_parts.extend(str(v or "") for v in part)
+        else:
+            text_parts.append(str(part or ""))
+    return " ".join(text_parts).lower()
+
+
+def utility_blocker(skill: Optional[dict], necessity: Optional[dict],
+                           verdict_data: Optional[dict]) -> Optional[str]:
+    """Return a short reason when a passing skill still should stay draft.
+
+    Broad/generic wording is not a blocker by itself. The blocker is whether
+    the skill failed to improve the agent versus a no-skill baseline, or whether
+    it duplicates another skill.
+    """
+    generic_re = re.compile(
+        r"\b(duplicat\w*|redundan\w*|overlap\w*|same skill|same procedure)\b",
+        re.I,
+    )
+    if isinstance(necessity, dict):
+        reason = str(necessity.get("reason") or "")
+        if necessity.get("necessary") is False and generic_re.search(reason):
+            return reason or "Duplicate or redundant skill"
+
+    if isinstance(verdict_data, dict):
+        baseline_verdict = str(verdict_data.get("baseline_verdict") or "unknown").lower()
+        try:
+            usefulness = float(verdict_data.get("usefulness", 0.0) or 0.0)
+        except (TypeError, ValueError):
+            usefulness = 0.0
+        try:
+            saved_turns = int(verdict_data.get("saved_turns", 0) or 0)
+        except (TypeError, ValueError):
+            saved_turns = 0
+        try:
+            saved_tool_calls = int(verdict_data.get("saved_tool_calls", 0) or 0)
+        except (TypeError, ValueError):
+            saved_tool_calls = 0
+        if baseline_verdict == "worse":
+            return "Skill performed worse than the no-skill baseline"
+        verdict_text = audit_flag_text(
+            verdict_data.get("summary"),
+            verdict_data.get("issues") or [],
+        )
+        if generic_re.search(verdict_text):
+            return "Audit flagged the skill as duplicate or redundant"
+    return None
+
+
+def finalize_status(skills_manager, name: str, owner, verdict: str,
+                           confidence: Optional[float], necessity: Optional[dict] = None,
+                           verdict_data: Optional[dict] = None, *, deps: AuditDependencies) -> str:
+    """Apply the user's audit publishing policy.
+
+    Audit is the final pass: skills that pass at/above the threshold are
+    published; failing or unnecessary/redundant skills are returned to draft.
+    Inconclusive runs preserve the existing state because they provide no
+    evidence either way. The completed batch moves duplicate losers to the bin.
+    """
+    auto_publish, min_conf = deps.auto_publish_policy(owner)
+    necessary = True
+    current = next((s for s in skills_manager.load(owner=owner) if s.get("name") == name), None)
+    if verdict in {"inconclusive", "unknown"}:
+        return (current or {}).get("status") or "draft"
+    utility_reason = deps.utility_blocker(current, necessity, verdict_data)
+    if (
+        isinstance(necessity, dict)
+        and necessity.get("necessary") is False
+        and necessity.get("redundant_with")
+    ):
+        necessary = False
+    if utility_reason:
+        necessary = False
+        try:
+            skills_manager.set_necessity(name, False, [], utility_reason, owner=owner)
+        except Exception:
+            pass
+    duplicate_of = deps.duplicate_blocker(skills_manager, name, owner) if verdict == "pass" else None
+    if duplicate_of:
+        necessary = False
+    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}, owner=owner)
+    except Exception:
+        pass
+    return status
+
+
+def apply_markdown(skills_manager, name: str, md: str, owner) -> bool:
+    """Parse + persist an edited SKILL.md. Returns True on success."""
+    try:
+        from services.memory.skill_format import Skill, slugify
+        sk = Skill.from_markdown(md)
+        # Pin the identity: the audit's fixer is now allowed to edit frontmatter
+        # (tags/category/when_to_use/description), but it must NEVER rename the
+        # skill — a changed `name` would move the dir and orphan the usage/audit
+        # sidecar entries that the caller keeps writing under the original name.
+        sk.name = name
+        return bool(skills_manager.update_skill(name, {
+            "name": sk.name, "description": sk.description, "version": sk.version,
+            "category": sk.category, "tags": sk.tags, "platforms": sk.platforms,
+            "requires_toolsets": sk.requires_toolsets, "fallback_for_toolsets": sk.fallback_for_toolsets,
+            "status": sk.status, "confidence": sk.confidence, "source": sk.source,
+            "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