feat(moments): skills and stored processes declare the moments they are for (milestone 458 step 5, #4923)

Loading a procedure now also reaches the moment it is for. Loading the
reporting procedure is a report; loading the release procedure is a delivery.

- Bundled skills: each SKILL.md declares `metadata: moments:`. The same
  declaration ships as Skill defaults (BUNDLED_SKILL_MOMENTS), because the
  server never sees the plugin's files. test_skill_moments holds the two
  together and pins the plugin name that qualifies the skill.
- Stored processes: `moments` on create_process and update_process, stored
  in the note's data and returned by get_process. A `scribe-proc-<slug>`
  load resolves its process through the sync manifest at load time. The
  moments are not copied into the stub, which would go stale mid-session.
- reachable_tools lists the skill loader whenever anything is mounted, since
  a process's moments are known only when it loads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

tests/test_mcp_tool_processes.py

@@ -166,6 +166,88 @@ async def test_delete_process_refuses_a_plain_note():
     mock_delete.assert_not_awaited()
 
 
+# ── the moments a procedure is for (milestone 458 step 5) ────────────────
+
+@pytest.mark.asyncio
+async def test_create_process_stores_the_moments_it_is_for():
+    created = fake_note(title="Release", note_type="process",
+                        data={"moments": ["work.deliver"]})
+    with patch("scribe.mcp.tools.processes.dedup_svc.find_duplicate_note",
+               AsyncMock(return_value=None)), \
+         patch("scribe.services.notes.create_note",
+               AsyncMock(return_value=created)) as mock_create:
+        from scribe.mcp.tools.processes import create_process
+        out = await create_process(title="Release", body="steps",
+                                   moments=["Work.Deliver", "work.deliver"])
+    assert mock_create.await_args.kwargs["data"] == {"moments": ["work.deliver"]}
+    assert out["moments"] == ["work.deliver"]
+
+
+@pytest.mark.asyncio
+async def test_create_process_refuses_an_unknown_moment_before_writing():
+    """A typo'd moment would be stored and never fire — a procedure that looks
+    attached and is not."""
+    with patch("scribe.mcp.tools.processes.dedup_svc.find_duplicate_note",
+               AsyncMock(return_value=None)), \
+         patch("scribe.services.notes.create_note", AsyncMock()) as mock_create:
+        from scribe.mcp.tools.processes import create_process
+        with pytest.raises(ValueError, match="unknown moment"):
+            await create_process(title="Release", body="steps", moments=["work.shipped"])
+    mock_create.assert_not_awaited()
+
+
+@pytest.mark.asyncio
+async def test_create_process_without_moments_stores_no_data():
+    created = fake_note(title="Release", note_type="process")
+    with patch("scribe.mcp.tools.processes.dedup_svc.find_duplicate_note",
+               AsyncMock(return_value=None)), \
+         patch("scribe.services.notes.create_note",
+               AsyncMock(return_value=created)) as mock_create:
+        from scribe.mcp.tools.processes import create_process
+        out = await create_process(title="Release", body="steps")
+    assert mock_create.await_args.kwargs["data"] is None
+    assert out["moments"] == []
+
+
+async def _update(moments, data):
+    proc = fake_note(id=5, title="Release", note_type="process", data=data)
+    with patch("scribe.services.notes.get_note_for_user",
+               AsyncMock(return_value=(proc, "owner"))), \
+         patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)), \
+         patch("scribe.services.notes.update_note",
+               AsyncMock(return_value=proc)) as mock_update:
+        from scribe.mcp.tools.processes import update_process
+        await update_process(process_id=5, moments=moments)
+    return mock_update.await_args.kwargs
+
+
+@pytest.mark.asyncio
+async def test_update_process_replaces_moments_and_keeps_the_rest_of_data():
+    sent = await _update(["work.verify"], {"moments": ["work.deliver"], "other": 1})
+    assert sent["data"] == {"moments": ["work.verify"], "other": 1}
+
+
+@pytest.mark.asyncio
+async def test_update_process_empty_moments_clears_them():
+    assert (await _update([], {"moments": ["work.deliver"]}))["data"] is None
+    assert (await _update([], {"moments": ["work.deliver"], "other": 1}))["data"] == {"other": 1}
+
+
+@pytest.mark.asyncio
+async def test_update_process_without_moments_leaves_data_alone():
+    assert "data" not in await _update(None, {"moments": ["work.deliver"]})
+
+
+@pytest.mark.asyncio
+async def test_get_process_says_which_moments_it_is_for():
+    note = fake_note(id=7, title="Release", note_type="process",
+                     data={"moments": ["work.deliver"]})
+    with patch("scribe.services.notes.resolve_process", AsyncMock(return_value=(note, []))):
+        from scribe.mcp.tools.processes import get_process
+        out = await get_process("release")
+    assert out["moments"] == ["work.deliver"]
+
+
 def test_register_attaches_every_tool_in_the_module():
     """Derived from the module rather than listed: a tool written but never
     registered is invisible to an agent, and nothing else would notice."""

tests/test_moment_actions.py

@@ -109,9 +109,22 @@ def test_a_whole_tool_mapping_ignores_its_arguments():
 
 
 def test_loading_a_procedure_reaches_its_own_moment():
+    assert _reached("Skill", {"skill": "release-notes"}) == ["skill.release-notes"]
+
+
+def test_a_bundled_skill_also_reaches_the_moment_it_is_for():
+    """Loading the planning procedure IS planning: a rule mounted on work.plan
+    arrives on the load, not only on the plan it goes on to open."""
     assert _reached("Skill", {"skill": "scribe:writing-plans"}) == [
-        "skill.scribe:writing-plans",
+        "work.plan", "skill.scribe:writing-plans",
     ]
+    assert _reached("Skill", {"skill": "scribe:reporting-back"})[0] == "reply.report"
+
+
+def test_only_the_products_own_skill_reaches_a_bundled_default():
+    """Another plugin's `verification` skill is not the product's procedure;
+    the plugin's name is what makes the declaration ours."""
+    assert _reached("Skill", {"skill": "other:verification"}) == ["skill.other:verification"]
 
 
 def test_a_procedure_name_that_is_not_a_moment_reaches_nothing():
@@ -119,6 +132,79 @@ def test_a_procedure_name_that_is_not_a_moment_reaches_nothing():
     assert _reached("Skill", {}) == []
 
 
+# ── a stored process's own moments ───────────────────────────────────────
+
+def test_a_process_declares_moments_its_load_reaches():
+    hits = ma.resolve("Skill", {"skill": "scribe-proc-release"}, [], ["work.deliver"])
+    assert [(h["moment"], h["via"]) for h in hits] == [
+        ("work.deliver", ma.PROCESS), ("skill.scribe-proc-release", ma.DEFAULT),
+    ]
+    assert hits[0]["match"] == "skill=scribe-proc-release"
+
+
+def test_a_declared_moment_another_action_reaches_is_named_once():
+    rows = [_row("Skill", "skill=scribe-proc-release", "work.deliver")]
+    hits = ma.resolve("Skill", {"skill": "scribe-proc-release"}, rows, ["work.deliver"])
+    assert [h["moment"] for h in hits].count("work.deliver") == 1
+
+
+def test_declared_moments_only_reach_a_skill_load():
+    hits = ma.resolve("Bash", {"command": "ls"}, [], ["work.deliver"])
+    assert [h["moment"] for h in hits] == ["work.run"]
+
+
+@pytest.mark.parametrize("data, expected", [
+    (None, []),
+    ({}, []),
+    ({"moments": ["work.deliver", "Work.Deliver", "reply.report"]}, ["work.deliver", "reply.report"]),
+    ({"moments": "work.verify"}, ["work.verify"]),
+    ({"moments": ["work.finished", "skill.two words", "work.debug"]}, ["work.debug"]),
+])
+def test_a_process_field_is_read_defensively(data, expected):
+    """A moment the catalog no longer has stops firing; it does not break the load."""
+    assert ma.declared_moments(data) == expected
+
+
+async def test_only_a_process_skill_asks_for_its_process():
+    asked = AsyncMock(return_value=["work.deliver"])
+    with patch.object(ma, "list_mappings", AsyncMock(return_value=[])), \
+         patch.object(ma, "process_moments", asked):
+        reached = await ma.moments_for(1, "Skill", {"skill": "scribe-proc-release"})
+        await ma.moments_for(1, "Skill", {"skill": "scribe:verification"})
+        await ma.moments_for(1, "Bash", {"command": "git push"})
+    assert "work.deliver" in [h["moment"] for h in reached]
+    asked.assert_awaited_once_with(1, "scribe-proc-release")
+
+
+async def _process_moments(manifest, note=None, *, raises=False):
+    from scribe.services import notes as notes_svc
+    from scribe.services import plugin_context
+
+    built = AsyncMock(side_effect=RuntimeError("down")) if raises else AsyncMock(return_value=manifest)
+    loader = AsyncMock(return_value=(note, None) if note is not None else None)
+    with patch.object(plugin_context, "build_process_manifest", built), \
+         patch.object(notes_svc, "get_note_for_user", loader):
+        return await ma.process_moments(1, "scribe-proc-release"), loader
+
+
+async def test_a_process_slug_resolves_through_the_sync_manifest():
+    """The same manifest the sync script wrote the skill from, so the slug
+    names the same process here as it did on disk."""
+    note = SimpleNamespace(data={"moments": ["work.deliver"]})
+    found, loader = await _process_moments(
+        {"processes": [{"id": 7, "slug": "other"}, {"id": 9, "slug": "release"}]}, note)
+    assert found == ["work.deliver"]
+    loader.assert_awaited_once_with(1, 9)
+
+
+async def test_an_unknown_or_unreadable_process_reaches_nothing_more():
+    found, loader = await _process_moments({"processes": [{"id": 7, "slug": "other"}]})
+    assert found == []
+    loader.assert_not_awaited()
+    found, _ = await _process_moments({}, raises=True)
+    assert found == []
+
+
 # ── an install's own ─────────────────────────────────────────────────────
 
 def test_an_install_mapping_adds_a_moment():

tests/test_moment_delivery.py

@@ -194,11 +194,17 @@ async def test_an_install_with_nothing_mounted_keeps_the_hook_off_the_wire():
 
 
 async def test_only_the_tools_whose_moments_carry_a_mount_are_listed():
-    assert await _reachable({"work.deliver"}) == ["bash"]
-    assert await _reachable({"work.finish"}) == ["update_milestone", "update_task"]
+    assert await _reachable({"work.deliver"}) == ["bash", "skill"]
+    assert await _reachable({"work.finish"}) == ["skill", "update_milestone", "update_task"]
     assert await _reachable({"skill.release"}) == ["skill"]
 
 
+async def test_the_skill_loader_counts_whenever_anything_is_mounted():
+    """A stored process declares its own moments, resolved only when it loads
+    — so a mount on any moment may be reached by loading a procedure."""
+    assert "skill" in await _reachable({"work.debug"})
+
+
 async def test_an_installs_own_mapping_and_removal_both_count():
     own = SimpleNamespace(tool="mcp__deploy__ship", match="", moment="work.deliver", effect="add")
     assert "ship" in await _reachable({"work.deliver"}, [own])

tests/test_skill_moments.py

@@ -0,0 +1,84 @@
+"""The bundled skills declare the moments they are for (milestone 458 step 5).
+
+Loading the reporting procedure is a report; loading the planning procedure
+is planning. Each SKILL.md says which moment it is for in its frontmatter
+(`metadata: moments:`), where an author editing the skill sees it — but the
+server that resolves a skill load to its moments never sees the plugin's
+files, so the same declaration ships as defaults in
+`moment_actions.BUNDLED_SKILL_MOMENTS`. Two copies drift unless something
+holds them together; this does, in both directions, and every skill has to
+make the choice rather than inherit silence.
+"""
+from __future__ import annotations
+
+import json
+import re
+from pathlib import Path
+
+from scribe.services import moment_actions as ma
+from scribe.services import moments
+
+ROOT = Path(__file__).resolve().parents[1]
+SKILLS = ROOT / "plugin" / "skills"
+MANIFEST = ROOT / "plugin" / ".claude-plugin" / "plugin.json"
+
+_FRONT = re.compile(r"\A---\n(.*?)\n---\n", re.S)
+_METADATA = re.compile(r"^metadata:[ \t]*\n((?:[ \t]+\S.*(?:\n|\Z))*)", re.M)
+_MOMENTS = re.compile(r"^[ \t]+moments:[ \t]*(.*)$", re.M)
+
+
+def declared(text: str) -> tuple[str, ...]:
+    """The moments a SKILL.md's frontmatter declares; () when it declares none.
+
+    Agent Skills metadata maps strings to strings, so several moments are one
+    comma-separated value.
+    """
+    front = _FRONT.match(text)
+    meta = _METADATA.search(front.group(1)) if front else None
+    line = _MOMENTS.search(meta.group(1)) if meta else None
+    if not line:
+        return ()
+    return tuple(m.strip() for m in line.group(1).split(",") if m.strip())
+
+
+def _on_disk() -> dict[str, tuple[str, ...]]:
+    return {p.parent.name: declared(p.read_text()) for p in sorted(SKILLS.glob("*/SKILL.md"))}
+
+
+def test_the_reader_finds_a_declaration_and_only_in_the_frontmatter():
+    """The guard below is only as good as this reader, so it must be able to
+    say no: a declaration in the body, or none at all, reads as nothing."""
+    assert declared(
+        "---\nname: x\ndescription: y\nmetadata:\n  moments: work.plan, reply.report\n---\nbody\n"
+    ) == ("work.plan", "reply.report")
+    assert declared("---\nname: x\ndescription: y\n---\nmetadata:\n  moments: work.plan\n") == ()
+    assert declared("---\nname: x\nmetadata:\n  author: someone\n---\n") == ()
+    assert declared("no frontmatter\n") == ()
+
+
+def test_every_bundled_skill_declares_the_moments_it_is_for():
+    silent = [name for name, found in _on_disk().items() if not found]
+    assert not silent, (
+        f"{silent} declare no moment. Add `metadata:` / `  moments: <moment>` to "
+        f"the frontmatter (list_moments has the catalog) and the same entry to "
+        f"moment_actions.BUNDLED_SKILL_MOMENTS"
+    )
+
+
+def test_the_frontmatter_and_the_shipped_defaults_agree():
+    assert _on_disk() == ma.BUNDLED_SKILL_MOMENTS, (
+        "a SKILL.md's `metadata: moments:` and moment_actions.BUNDLED_SKILL_MOMENTS "
+        "disagree — the server reads only the latter, so change both together"
+    )
+
+
+def test_every_declared_moment_is_in_the_catalog():
+    bad = {name: found for name, found in _on_disk().items()
+           if any(m not in moments.MOMENTS for m in found)}
+    assert not bad, bad
+
+
+def test_the_defaults_qualify_skills_by_the_plugins_own_name():
+    """The harness names a plugin's skill `<plugin>:<skill>`, so the defaults
+    match only while this agrees with the manifest."""
+    assert json.loads(MANIFEST.read_text())["name"] == ma.BUNDLED_SKILL_PLUGIN