fix(deck): close 6 high-confidence findings from skill-refactor review

Architectural review (4 parallel Opus auditors) found that the skill_runner
core was already generic, but the deck SURFACE was still fused to
Editorial Monocle. Fixed:

* validator: now takes optional `grammar` param (DeckGrammar TypedDict);
  skill-agnostic by default (only checks file present, parses, ≥5
  slides, self-contained). Third-party deck skills (guizang, swiss)
  now pass validation cleanly. Editorial-specific rules opt-in via
  `EDITORIAL_MONOCLE_GRAMMAR`. (finding #2)
* skills/openkb-deck-editorial/SKILL.md: declares its grammar +
  output_path_template under `od:` frontmatter — `run_skill` reads
  these and applies them post-run.
* run_skill: now honors frontmatter `od.mode`, `od.output_path_template`,
  `od.deck_grammar`. When mode=="deck" and template is set, the runner
  injects the path into intent, verifies the file exists post-run, and
  runs validate_deck with the skill's grammar. Validation result is
  returned via new SkillRunResult dataclass. (findings #4, #5)
* `openkb deck new --skill <name>`: CLI flag accepts any installed deck
  skill (default openkb-deck-editorial). guizang and swiss now usable
  from the scripted CLI, not only freeform chat. (finding #1)
* `/deck new --skill <name>` chat slash: same flag, parsed positionally
  alongside --critique. (finding #1)
* tests/test_read_kb_file.py: 13 new tests mirroring test_write_kb_file
  for the read-side allow-list. Pins refusal of `.openkb/config.yaml`,
  `.env`, `raw/`, `..` traversal, absolute paths. (finding #6)
* Generator deck branch: no longer calls validate_deck directly; just
  propagates run_deck_create's SkillRunResult.validation up. Validation
  is now a property of "this skill declared mode=deck", not of "this
  CLI path was taken".

Existing tests updated:
* tests/test_deck_validator.py: explicit grammar arg on Editorial-
  specific tests; added test_guizang_shape_passes_generic_mode +
  test_missing_cover_ignored_in_generic_mode to pin both modes.
* tests/test_deck_creator.py: mocks return SkillRunResult; new
  test_run_deck_create_honors_skill_name_override for --skill flag.
* tests/test_generator.py: deck dispatch test mocks SkillRunResult.

Below-threshold findings deferred:
* Generator if/else → registry (score 70) — works, just not extensible
  via plugin; future.
* Iteration backup in chat freeform path (score 75) — needs write_kb_file
  hook; separate change.
* run_skill / scan_local_skills / _handle_slash_critique direct tests
  (scores 60-70) — covered indirectly by integration; can add later.

Regression: 538 tests pass (was 523 pre-fix; net +15 = 13 new
read_kb_file tests + 2 new validator-mode tests).

Author: KylinMountain

openkb/agent/chat.py

@@ -60,7 +60,7 @@
     "  /lint          Lint the knowledge base\n"
     "  /add <path>    Add a document or directory to the knowledge base\n"
     '  /skill new <name> "<intent>"   Compile a skill from the wiki\n'
-    '  /deck new [--critique] <name> "<intent>"   Generate an HTML deck from the wiki\n'
+    '  /deck new [--critique] [--skill <name>] <name> "<intent>"   Generate an HTML deck from the wiki\n'
     "  /critique <path-to-html>   Run html-critic skill on a file (CSS bugs, layout, self-containment)\n"
     "  /help          Show this"
 )
@@ -606,20 +606,29 @@ async def _handle_slash_deck(arg: str, kb_dir: Path, style: Style) -> None:
         _fmt(style, ("class:error", f"Unknown deck subcommand: {sub}. Try /deck new.\n"))
         return
 
-    # Parse optional --critique flag (anywhere among the remaining tokens
-    # is fine, but typically right after `new`).
+    # Parse optional --critique flag and --skill <name> option. Both can
+    # appear anywhere among the remaining tokens.
     rest = parts[1:]
     critique = False
+    skill_name: str | None = None
     filtered: list[str] = []
-    for tok in rest:
+    i = 0
+    while i < len(rest):
+        tok = rest[i]
         if tok == "--critique":
             critique = True
+        elif tok == "--skill" and i + 1 < len(rest):
+            skill_name = rest[i + 1]
+            i += 1
+        elif tok.startswith("--skill="):
+            skill_name = tok.split("=", 1)[1]
         else:
             filtered.append(tok)
+        i += 1
 
     if len(filtered) < 2:
         _fmt(style, ("class:error",
-            "Usage: /deck new [--critique] <name> \"<intent>\"\n"))
+            "Usage: /deck new [--critique] [--skill <skill>] <name> \"<intent>\"\n"))
         return
 
     name = filtered[0]
@@ -650,14 +659,19 @@ async def _handle_slash_deck(arg: str, kb_dir: Path, style: Style) -> None:
     model = config.get("model", DEFAULT_CONFIG["model"])
 
     from openkb.skill.generator import Generator
-    _fmt(style, ("class:slash.help", f"Generating deck '{name}'...\n"))
+    skill_label = skill_name if skill_name else "openkb-deck-editorial (default)"
+    _fmt(
+        style,
+        ("class:slash.help", f"Generating deck '{name}' via skill {skill_label}...\n"),
+    )
     gen = Generator(
         target_type="deck",
         name=name,
         intent=intent,
         kb_dir=kb_dir,
         model=model,
         critique=critique,
+        skill_name=skill_name,
     )
     try:
         await gen.run()

openkb/agent/skill_runner.py

@@ -4,16 +4,30 @@
 agent instructions) is loaded by ``run_skill``, which builds an Agent
 whose ``instructions`` are that body. The agent gets the standard wiki
 read-tool set plus a constrained ``write_file`` tool scoped to
-``wiki/explorations/**`` and ``output/**``.
+``wiki/explorations/**`` and ``output/**``, and ``read_output_or_skill_file``
+for inspecting prior artifacts.
 
 This decouples generators from hard-coded prompts. ``openkb deck new`` /
 ``openkb skill new`` / any future ``openkb <type> new`` command becomes a
 two-line wrapper around ``run_skill(skill_name=..., intent=...)``.
+
+Skill frontmatter that ``run_skill`` honours (all optional; under the
+top-level ``od:`` key):
+
+* ``mode`` — the artifact type. ``"deck"`` triggers deck-specific
+  post-run handling (output-path templating + validation). Unknown modes
+  are accepted; they just don't trigger extra hooks.
+* ``output_path_template`` — a path string with ``{slug}`` placeholder,
+  relative to KB root. When set, ``run_skill`` injects the resolved path
+  into the agent's intent and verifies the file exists post-run.
+* ``deck_grammar`` — passed to :func:`openkb.deck.validator.validate_deck`
+  when ``mode == "deck"``. See that module for the contract.
 """
 from __future__ import annotations
 
+from dataclasses import dataclass, field
 from pathlib import Path
-from typing import Optional
+from typing import Any, Optional
 
 from agents import Agent, Runner, function_tool
 
@@ -30,6 +44,23 @@ class SkillNotFoundError(RuntimeError):
     """Raised when the requested skill can't be located in any skill root."""
 
 
+@dataclass
+class SkillRunResult:
+    """Outcome of a :func:`run_skill` call.
+
+    ``validation`` is populated only when the skill declared a
+    deck-mode artifact and the post-run validator was therefore invoked;
+    otherwise ``None``. ``output_path`` is the KB-relative path the
+    runner enforced via ``output_path_template``, or ``None`` if the
+    skill left output placement to itself.
+    """
+
+    skill_name: str
+    output_path: Optional[Path] = None
+    validation: Optional[Any] = None  # openkb.deck.validator.ValidationResult
+    metadata: dict = field(default_factory=dict)  # skill's ``od:`` block
+
+
 async def run_skill(
     *,
     skill_name: str,
@@ -39,14 +70,23 @@ async def run_skill(
     language: str = "en",
     max_turns: int = MAX_TURNS,
     seed: Optional[str] = None,
+    slug: Optional[str] = None,
     extra_skill_roots: tuple[str | Path, ...] = (),
-) -> None:
-    """Load skill ``skill_name`` and run it as an agent with ``intent``.
+) -> SkillRunResult:
+    """Load ``skill_name`` and run it as an agent with ``intent``.
+
+    When the skill's frontmatter declares ``od.mode == "deck"`` and
+    ``od.output_path_template`` is set, ``run_skill``:
+
+      1. Templates the path with the provided ``slug``.
+      2. Adds a "Write to: <path>" line to the agent's intent so the
+         skill knows the expected destination.
+      3. After the agent run, checks the file exists and (if the skill
+         declared ``od.deck_grammar``) runs ``validate_deck`` against it.
 
     Args:
         skill_name: Name (frontmatter ``name:``) of the skill to invoke.
-        intent: Natural-language brief for what to produce. Appended to
-            the skill body as a "## User intent" section.
+        intent: Natural-language brief for what to produce.
         kb_dir: KB root. Used both for skill discovery and for the
             agent's wiki read-tools / write-file scoping.
         model: LiteLLM-formatted model string from KB config.
@@ -55,13 +95,21 @@ async def run_skill(
         max_turns: Hard cap on agent loop iterations.
         seed: Optional kick-off user message. Defaults to a short nudge
             that points the agent at its own instructions.
+        slug: Required when the skill declares
+            ``od.output_path_template`` (it substitutes ``{slug}``).
+            Otherwise ignored.
         extra_skill_roots: Additional directories to scan beyond the
             built-in ``<kb>/skills``, ``~/.openkb/skills``,
             ``~/.claude/skills``.
 
+    Returns:
+        A :class:`SkillRunResult` carrying the resolved output path (if
+        any) and the validation result (for deck-mode skills).
+
     Raises:
         SkillNotFoundError: if no skill with ``skill_name`` is found.
-        RuntimeError: if the agent run fails (turn-cap, model error).
+        RuntimeError: on turn-cap, model error, or missing
+            output file after a templated-path run.
     """
     skills = scan_local_skills(kb_dir, extra_roots=extra_skill_roots)
     match = next((s for s in skills if s["name"] == skill_name), None)
@@ -74,7 +122,19 @@ async def run_skill(
         )
 
     skill_md = Path(match["path"]) / "SKILL.md"
-    _meta, body = _parse_frontmatter(skill_md.read_text(encoding="utf-8"))
+    meta, body = _parse_frontmatter(skill_md.read_text(encoding="utf-8"))
+    od_meta: dict = (meta.get("od") or {}) if isinstance(meta, dict) else {}
+
+    # Resolve output path if the skill templated one.
+    output_path: Optional[Path] = None
+    template = od_meta.get("output_path_template")
+    if template and slug:
+        rel = template.format(slug=slug)
+        output_path = (kb_dir / rel).resolve()
+        intent = (
+            f"Output file (write the artifact here, full file in one "
+            f"write_file call): {rel}\n\n{intent}"
+        )
 
     wiki_root = str(kb_dir / "wiki")
     kb_root = str(kb_dir)
@@ -131,3 +191,27 @@ def read_output_or_skill_file(path: str) -> str:
             f"finishing. The intent may be too broad or the wiki too large; "
             f"try a tighter intent or split into smaller skills."
         ) from exc
+
+    result = SkillRunResult(
+        skill_name=skill_name,
+        output_path=output_path,
+        metadata=od_meta if isinstance(od_meta, dict) else {},
+    )
+
+    # Post-run hooks driven by skill frontmatter.
+    if output_path is not None and not output_path.is_file():
+        raise RuntimeError(
+            f"Skill {skill_name!r} finished but did not write the expected "
+            f"output file at {output_path}. The skill is either misconfigured "
+            f"or the wiki lacks content matching the intent."
+        )
+
+    if od_meta.get("mode") == "deck" and output_path is not None:
+        # Lazy import — skill_runner shouldn't pull in deck-specific code
+        # at import time, only when actually running a deck skill.
+        from openkb.deck.validator import DeckGrammar, validate_deck
+
+        grammar: Optional[DeckGrammar] = od_meta.get("deck_grammar")
+        result.validation = validate_deck(output_path.parent, grammar=grammar)
+
+    return result

openkb/cli.py

@@ -1934,8 +1934,18 @@ def deck():
     is_flag=True, default=False,
     help="Opt-in second-pass review via a critic agent (slower, higher quality).",
 )
+@click.option(
+    "--skill", "skill_name",
+    metavar="SKILL_NAME",
+    default=None,
+    help=(
+        "Which deck skill to use. Defaults to 'openkb-deck-editorial' "
+        "(the built-in). Pass e.g. 'deck-guizang-editorial' to route to "
+        "a third-party skill installed under ~/.openkb/skills/."
+    ),
+)
 @click.pass_context
-def deck_new(ctx, name, intent, yes_flag, critique_flag):
+def deck_new(ctx, name, intent, yes_flag, critique_flag, skill_name):
     """Generate a new HTML deck from this KB's wiki.
 
     NAME is a kebab-case slug used for the output directory.
@@ -1945,6 +1955,7 @@ def deck_new(ctx, name, intent, yes_flag, critique_flag):
 
       openkb deck new transformers-pitch "Explain attention to engineers"
       openkb deck new transformers-pitch "Explain attention to engineers" --critique
+      openkb deck new transformers-pitch "..." --skill deck-guizang-editorial
     """
     kb_dir = _find_kb_dir(ctx.obj.get("kb_dir_override"))
     if kb_dir is None:
@@ -2007,14 +2018,16 @@ def deck_new(ctx, name, intent, yes_flag, critique_flag):
 
     # Run the generator.
     from openkb.skill.generator import Generator
-    click.echo(f"Generating deck '{name}'...")
+    skill_label = skill_name if skill_name else "openkb-deck-editorial (default)"
+    click.echo(f"Generating deck '{name}' via skill {skill_label}...")
     gen = Generator(
         target_type="deck",
         name=name,
         intent=intent,
         kb_dir=kb_dir,
         model=model,
         critique=critique_flag,
+        skill_name=skill_name,
     )
     try:
         asyncio.run(gen.run())