From f01aa3ad540445d991c77ce245a45842d8960bd3 Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Mon, 21 Sep 2026 19:17:43 +0100 Subject: [PATCH 01/24] Add skills as slash commands --- .changeset/skills-as-slash-commands.md | 5 + CLAUDE.md | 5 +- services/global_chat/PAYLOAD_SPEC.md | 56 +++++ services/global_chat/README.md | 12 + services/global_chat/global_chat.py | 13 + services/global_chat/planner.py | 19 +- services/global_chat/router.py | 43 +++- services/global_chat/skill_registry.py | 93 ++++++++ services/global_chat/skills/README.md | 12 + services/global_chat/skills/diagnose/SKILL.md | 13 + services/global_chat/skills/qa/SKILL.md | 14 ++ .../global_chat/tests/unit/test_planner.py | 1 + .../global_chat/tests/unit/test_skills.py | 222 ++++++++++++++++++ 13 files changed, 501 insertions(+), 7 deletions(-) create mode 100644 .changeset/skills-as-slash-commands.md create mode 100644 services/global_chat/skill_registry.py create mode 100644 services/global_chat/skills/README.md create mode 100644 services/global_chat/skills/diagnose/SKILL.md create mode 100644 services/global_chat/skills/qa/SKILL.md create mode 100644 services/global_chat/tests/unit/test_skills.py diff --git a/.changeset/skills-as-slash-commands.md b/.changeset/skills-as-slash-commands.md new file mode 100644 index 00000000..e6077641 --- /dev/null +++ b/.changeset/skills-as-slash-commands.md @@ -0,0 +1,5 @@ +--- +"apollo": minor +--- + +global_chat: accept a `skill` field naming a standard skill the user invoked by slash command. `/diagnose` and `/qa` ship with the agent; an invoked skill skips the router and leads the planner's turn. Skill instructions are per-turn, so the client re-sends `skill` on every turn it applies to diff --git a/CLAUDE.md b/CLAUDE.md index e9a36662..9ca1a5a4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -169,7 +169,10 @@ SSE to clients. - `global_chat/` - Orchestrator service and single entry point for OpenFn AI chat. Routes requests via a RouterAgent (Haiku) to specialized subagents, or escalates to a PlannerAgent (Sonnet) that coordinates multi-step tasks using - tool calls. Depends on `job_chat`, `workflow_chat`, and `search_docsite`. + tool calls. A `skill` in the payload names a standard skill the user invoked by + slash command (`skills//SKILL.md`, loaded by `skill_registry.py`); it + bypasses the router and leads the planner's turn. Depends on `job_chat`, + `workflow_chat`, and `search_docsite`. - `job_chat/` - AI chat service for OpenFn job code assistance. Supports conversational help and a code suggestions mode with auto-patching. Uses RAG via `search_docsite` and injects adaptor API docs. Streams responses. diff --git a/services/global_chat/PAYLOAD_SPEC.md b/services/global_chat/PAYLOAD_SPEC.md index 6fa64018..37905128 100644 --- a/services/global_chat/PAYLOAD_SPEC.md +++ b/services/global_chat/PAYLOAD_SPEC.md @@ -39,6 +39,10 @@ This document defines the input and output payload structure for the Global Agen } ], + "skill": { // Skill invoked this turn (optional) + "name": "diagnose" // Standard skill name: "diagnose" | "qa" + }, + "options": { // Runtime options (optional) "stream": false }, @@ -80,6 +84,11 @@ This document defines the input and output payload structure for the Global Agen The `type` is passed to the model as a label, so an unrecognised type is delivered rather than dropped. See [Attachment handling](#attachment-handling) for how they travel and why they are not persisted. +- **`skill`** (object, optional): A standard skill the user invoked by slash + command. `name` is the skill's name (`"diagnose"` or `"qa"`). An unknown name + is rejected with a `400` `UNKNOWN_SKILL`. See + [Skill invocation](#skill-invocation). + - **`options`** (object, optional): Runtime options. - **`stream`** (boolean): Enable streaming response (default: false). @@ -168,6 +177,53 @@ Each tool beat streams as: `thinking` spinner → `changes` (if the workflow was - **`tool_calls`** (array): List of `{tool, input}` objects for each tool the planner invoked (planner path only). - **`subagent_calls`** (array): Raw sub-agent result dicts including `_call_metadata`. On the planner path these are the full results, useful for debugging. On the router's direct job-code path it carries a single entry with just `_call_metadata` and `diff`, so a client can tell on either route whether a code edit actually landed (`diff.patches_applied`). - **`total_tool_calls`** (number): Total number of tool calls made by the planner (planner path only). + - **`skill`** (string): The skill invoked this turn (skill path only). `router_confidence` is absent on this path, because the router did not run. + +--- + +## Skill invocation + +A skill is a reusable instruction set that augments the model's context for one +turn when the user explicitly invokes it. Standard skills ship with Apollo, in +`services/global_chat/skills//SKILL.md`; they are immutable and upgrade +for everyone on deploy. + +The client recognises the command and names it in `skill` — **Apollo never +parses commands out of `content`**. Follow standard slash-command semantics: +recognise `/` only at the start of the message, against the known list; +the rest of the message is the request; one skill per message; the raw message +stays in the client's visible transcript. + +Sending `skill` changes the turn in three ways: + +1. **The router is skipped.** A slash command states the intent the router + would otherwise guess, so the turn goes straight to the planner. `meta.skill` + names the skill; `meta.router_confidence` is absent. +2. **The leading `/` token is stripped** from `content` before the model + or the returned history sees it. The rest of the message is untouched. +3. **The skill's instructions lead the user turn.** + +Skills travel per-turn, exactly like attachments: the instructions reach the +model but are never written to `history`, so **the client must re-send `skill` +on every turn it should apply to**. A follow-up turn sent without it is routed +normally. + +### Example + +```json +{ + "content": "/diagnose why did the last run fail?", + "skill": { "name": "diagnose" }, + "workflow_yaml": "name: My Workflow\njobs:\n fetch-data:\n ...\n", + "attachments": [ + { "type": "log", "content": "ERROR: Request failed with status 500" } + ] +} +``` + +The planner is asked `"why did the last run fail?"` with the `diagnose` +instructions ahead of it, and `meta` comes back as +`{"agents": ["planner"], "skill": "diagnose", ...}`. --- diff --git a/services/global_chat/README.md b/services/global_chat/README.md index 834fe181..4794da1e 100644 --- a/services/global_chat/README.md +++ b/services/global_chat/README.md @@ -140,6 +140,18 @@ Request to build a new multi-step workflow from scratch: ## Implementation +### Skills + +A `skill` field in the payload names a standard skill the user invoked by slash +command (`/diagnose`, `/qa`). Standard skills live in `skills//SKILL.md`, +in Anthropic's Agent Skills format, and are loaded by `skill_registry.py` at +import. + +An invoked skill skips the router — a slash command states the intent the router +would otherwise guess — and its instructions lead the planner's user turn. They +are per-turn context, like attachments, so the client re-sends `skill` on every +turn it should apply to. See [PAYLOAD_SPEC.md](PAYLOAD_SPEC.md#skill-invocation). + ### Routing Every request first passes through the `RouterAgent` (Claude Haiku), which diff --git a/services/global_chat/global_chat.py b/services/global_chat/global_chat.py index 1e9c11cd..29936efd 100644 --- a/services/global_chat/global_chat.py +++ b/services/global_chat/global_chat.py @@ -17,6 +17,7 @@ from langfuse_util import should_track, build_tags, build_generation_diff from global_chat.config_loader import ConfigLoader from global_chat.router import RouterAgent +from global_chat.skill_registry import Skill, get_skill logger = create_logger(__name__) @@ -33,6 +34,7 @@ class Payload: api_key: Optional[str] = None attachments: Optional[List[Dict]] = None metrics_opt_in: Optional[bool] = None + skill: Optional[Skill] = None @classmethod def from_dict(cls, data: Dict[str, Any]) -> "Payload": @@ -55,8 +57,18 @@ def from_dict(cls, data: Dict[str, Any]) -> "Payload": api_key=data.get("api_key"), attachments=data.get("attachments"), metrics_opt_in=data.get("metrics_opt_in"), + skill=cls._resolve_skill(data.get("skill")), ) + @staticmethod + def _resolve_skill(raw: object) -> Optional[Skill]: + """Resolve an invoked skill to its instructions, or reject the request.""" + if raw is None: + return None + if not isinstance(raw, dict) or not isinstance(raw.get("name"), str): + raise ApolloError(400, "skill must be an object with a name") + return get_skill(raw["name"]) + def get_stream(self) -> bool: """Extract stream flag from options.""" return (self.options or {}).get("stream", False) @@ -108,6 +120,7 @@ def main(data_dict: dict) -> dict: attachments=data.attachments or [], user=user_info, metrics_opt_in=data.metrics_opt_in, + skill=data.skill, ) if tracking: diff --git a/services/global_chat/planner.py b/services/global_chat/planner.py index 42ba790a..23672458 100644 --- a/services/global_chat/planner.py +++ b/services/global_chat/planner.py @@ -25,6 +25,7 @@ STATUS_PLANNING, ) from global_chat.config_loader import ConfigLoader +from global_chat.skill_registry import Skill from models import resolve_model from global_chat.tools.tool_definitions import TOOL_DEFINITIONS from yaml_utils import stitch_job_code, redact_job_bodies, find_job_in_yaml, get_step_name_from_page, inspect_job_code, job_keys_in_yaml @@ -240,6 +241,7 @@ def __init__(self, config_loader: ConfigLoader, api_key: Optional[str] = None): self.subagent_results = [] self._segments: List[Dict] = [] self._attachments: List[Dict] = [] + self._skill: Optional[Skill] = None logger.info(f"PlannerAgent initialized with model: {self.model}") @@ -255,6 +257,7 @@ def run( user: Optional[Dict] = None, metrics_opt_in: Optional[bool] = None, stream_manager: Optional[StreamManager] = None, + skill: Optional[Skill] = None, ) -> PlannerResult: """ Run the planner agent with tool-calling loop. @@ -270,6 +273,8 @@ def run( the ones the planner names for it. stream_manager: Optional shared stream manager from the router, so a handed-over request continues on the same stream + skill: A skill the user invoked this turn, whose instructions lead + the user message Returns: PlannerResult with response, attachments, history, usage, meta @@ -288,6 +293,7 @@ def run( self._user = user self._metrics_opt_in = metrics_opt_in self._segments: List[Dict] = [] + self._skill = skill stream_manager = StreamManager(model=self.model, stream=stream) if workflow_yaml: @@ -494,16 +500,21 @@ def run( ) def _build_user_content(self, content: str, page: Optional[str]) -> str: - """Augment the user message with this turn's attachments, the step the - user is viewing ("this step"), and the existing workflow structure - (bodies redacted). + """Augment the user message with an invoked skill's instructions, this + turn's attachments, the step the user is viewing ("this step"), and the + existing workflow structure (bodies redacted). Everything added here is per-turn: run() records the raw `content` in the returned history, so an attached log never becomes a permanent part - of the conversation. + of the conversation. A skill travels the same way, which is why the + client re-sends it on every turn it should apply to. """ user_content = content + if self._skill: + preamble = self._skill.as_preamble() + user_content = f"{preamble}\n\n{user_content}" if user_content else preamble + attachments = format_attachments(self._attachments) if attachments: user_content += f"\n\n{attachments}" diff --git a/services/global_chat/router.py b/services/global_chat/router.py index c2431862..8b6dd11e 100644 --- a/services/global_chat/router.py +++ b/services/global_chat/router.py @@ -21,6 +21,7 @@ from util import create_logger, ApolloError, sum_usage, attachments_to_context from streaming_util import StreamManager from global_chat.config_loader import ConfigLoader +from global_chat.skill_registry import Skill, strip_invocation from models import resolve_model from yaml_utils import get_step_name_from_page, get_page_view, find_job_in_yaml, stitch_job_code, workflow_has_job_code @@ -85,6 +86,7 @@ def route_and_execute( attachments: Optional[List[Dict]] = None, user: Optional[Dict] = None, metrics_opt_in: Optional[bool] = None, + skill: Optional[Skill] = None, ) -> RouterResult: """ Route request to appropriate handler and execute. @@ -96,6 +98,7 @@ def route_and_execute( history: Conversation history stream: Streaming flag attachments: Optional input attachments (e.g. logs, dataclips) + skill: A skill the user invoked by name, which routes itself Returns: RouterResult with response, attachments, history, usage, meta @@ -117,6 +120,9 @@ def route_and_execute( # a second message lifecycle. self._stream_manager = StreamManager(model=self.model, stream=stream) + if skill: + return self._route_to_skill(skill, content, workflow_yaml, page, history, stream) + try: decision = self._make_routing_decision(content, workflow_yaml, page, history) logger.info( @@ -465,6 +471,36 @@ def _track_reroute(self, metadata: Dict) -> None: except Exception: logger.warning("Failed to record reroute metadata in Langfuse") + def _route_to_skill( + self, + skill: Skill, + content: str, + workflow_yaml: Optional[str], + page: Optional[str], + history: List[Dict], + stream: bool, + ) -> RouterResult: + """Hand an invoked skill straight to the planner, skipping the router. + + The router guesses intent; a slash command states it, so there is + nothing to decide. Both standard skills are multi-step tasks, which is + what the planner is for. + """ + logger.info(f"Skill '{skill.name}' invoked, bypassing router") + + result = self._route_to_planner( + strip_invocation(content, skill.name), + workflow_yaml, + page, + history, + stream, + confidence=None, + skill=skill, + ) + result.meta["agents"] = [a for a in result.meta["agents"] if a != "router"] + result.meta["skill"] = skill.name + return result + def _route_to_planner( self, content: str, @@ -472,7 +508,8 @@ def _route_to_planner( page: Optional[str], history: List[Dict], stream: bool, - confidence: int, + confidence: Optional[int], + skill: Optional[Skill] = None, ) -> RouterResult: """Delegate to PlannerAgent for complex orchestration.""" from global_chat.planner import PlannerAgent @@ -492,12 +529,14 @@ def _route_to_planner( user=self._user, metrics_opt_in=self._metrics_opt_in, stream_manager=self._stream_manager, + skill=skill, ) total_usage = sum_usage(self.routing_usage, planner_result.usage) meta = planner_result.meta.copy() - meta["router_confidence"] = confidence + if confidence is not None: + meta["router_confidence"] = confidence return RouterResult( response=planner_result.response, diff --git a/services/global_chat/skill_registry.py b/services/global_chat/skill_registry.py new file mode 100644 index 00000000..3e40a096 --- /dev/null +++ b/services/global_chat/skill_registry.py @@ -0,0 +1,93 @@ +"""Standard skill registry. + +Standard skills ship with the agent: immutable, versioned in this repo, loaded +from disk once at import. They follow Anthropic's Agent Skills format (a folder +whose entry point is SKILL.md) so that a built-in and a user-defined skill are +the same artifact. + +A malformed built-in raises here, at import, rather than on the request that +first invokes it. +""" + +import re +from dataclasses import dataclass +from pathlib import Path + +import yaml + +from util import ApolloError + +SKILLS_DIR = Path(__file__).parent / "skills" + + +@dataclass(frozen=True) +class Skill: + """A loaded SKILL.md: its frontmatter and its instructions.""" + + name: str + description: str + body: str + + def as_preamble(self) -> str: + """The skill as it is injected ahead of the user's request.""" + return ( + f'\n{self.body}\n\n\n' + f"The user invoked the /{self.name} skill. Follow its instructions for this turn." + ) + + +def _parse_skill_file(path: Path, folder: str) -> Skill: + text = path.read_text() + + if not text.startswith("---"): + raise ValueError(f"{path} has no YAML frontmatter") + + _, frontmatter, body = text.split("---", 2) + meta = yaml.safe_load(frontmatter) or {} + + name = meta.get("name") + description = meta.get("description") + + # The folder name is what a slash command resolves against, so a + # frontmatter name that disagrees with it would be unreachable. + if name != folder: + raise ValueError(f"{path} declares name '{name}' but lives in '{folder}/'") + if not description: + raise ValueError(f"{path} has no description") + if not body.strip(): + raise ValueError(f"{path} has no instructions") + + return Skill(name=name, description=description, body=body.strip()) + + +def _load_skills() -> dict[str, Skill]: + return { + entry.name: _parse_skill_file(entry / "SKILL.md", entry.name) + for entry in sorted(SKILLS_DIR.iterdir()) + if entry.is_dir() + } + + +SKILLS = _load_skills() + + +def get_skill(name: str) -> Skill: + """Resolve a skill name, or reject the request.""" + skill = SKILLS.get(name) + if skill is None: + raise ApolloError( + 400, + f"Unknown skill '{name}'", + "UNKNOWN_SKILL", + {"available": sorted(SKILLS)}, + ) + return skill + + +def strip_invocation(content: str, name: str) -> str: + """Drop the leading /name token, leaving what the user actually asked. + + Only at the start of the message, and only the invoked skill's own name: + the command was recognised by the client, so this is not command parsing. + """ + return re.sub(rf"^\s*/{re.escape(name)}(?=\s|$)", "", content, count=1).lstrip() diff --git a/services/global_chat/skills/README.md b/services/global_chat/skills/README.md new file mode 100644 index 00000000..3d1c6dc8 --- /dev/null +++ b/services/global_chat/skills/README.md @@ -0,0 +1,12 @@ +# Standard skills + +One folder per skill, entry point `SKILL.md`, in Anthropic's Agent Skills +format: YAML frontmatter (`name`, `description`) then the instructions. The +format matches what a user-defined skill will look like in v1, so a built-in +and a custom skill are the same artifact. + +Invoked by name via the `skill` payload field, which bypasses the router and +hands the turn to the planner. + +**The bodies here are placeholders.** They exercise the mechanism; they are not +the real prompts. See https://github.com/OpenFn/apollo/issues/614. diff --git a/services/global_chat/skills/diagnose/SKILL.md b/services/global_chat/skills/diagnose/SKILL.md new file mode 100644 index 00000000..d76bf4a1 --- /dev/null +++ b/services/global_chat/skills/diagnose/SKILL.md @@ -0,0 +1,13 @@ +--- +name: diagnose +description: Diagnose a failed workflow run from its logs, dataclips and workflow YAML +--- + +Diagnose why the run the user is asking about failed. Do not fix anything +unless they ask. + +1. Read the run logs and dataclips attached to this turn. If none are attached, + say what you need and stop. +2. Find the first real failure, not the errors that followed from it. +3. Report the failing step, the cause, and the evidence you read it from. +4. Offer a fix as a next step, and wait to be asked. diff --git a/services/global_chat/skills/qa/SKILL.md b/services/global_chat/skills/qa/SKILL.md new file mode 100644 index 00000000..98293cf9 --- /dev/null +++ b/services/global_chat/skills/qa/SKILL.md @@ -0,0 +1,14 @@ +--- +name: qa +description: Review a workflow against OpenFn conventions and report what should change +--- + +Review the workflow the user is asking about and report what should change. Do +not edit it unless they ask. + +1. Read the workflow structure and the job code of every step. +2. Check each step for: unhandled errors, hardcoded values that belong in + state or credentials, operations that do not exist in the step's adaptor, + and steps doing work that belongs in another step. +3. Report findings worst first, each naming the step and what to change. +4. Say plainly if you found nothing worth changing. diff --git a/services/global_chat/tests/unit/test_planner.py b/services/global_chat/tests/unit/test_planner.py index 81fa0567..75f3b299 100644 --- a/services/global_chat/tests/unit/test_planner.py +++ b/services/global_chat/tests/unit/test_planner.py @@ -35,6 +35,7 @@ def make_planner() -> PlannerAgent: planner.subagent_results = [] planner._segments = [] planner._attachments = [] + planner._skill = None planner.api_key = "test-key" planner._user = None planner._metrics_opt_in = None diff --git a/services/global_chat/tests/unit/test_skills.py b/services/global_chat/tests/unit/test_skills.py new file mode 100644 index 00000000..f1a988bc --- /dev/null +++ b/services/global_chat/tests/unit/test_skills.py @@ -0,0 +1,222 @@ +"""Unit tests for skill invocation. + +A slash command states the intent the router would otherwise guess, so an +invoked skill skips the routing call and goes to the planner. The skill's +instructions are per-turn context, like attachments: the model sees them, the +returned history does not. + +Apollo never parses commands out of free text — the client recognises the +command and names it in the payload — so a message that merely mentions a +slash command routes normally. +""" + +from unittest.mock import patch + +import pytest +from global_chat.global_chat import Payload +from global_chat.planner import PlannerResult +from global_chat.router import RouterAgent, RouterDecision, RouterResult +from global_chat.skill_registry import SKILLS, get_skill, strip_invocation +from util import ApolloError + +from .test_planner import FakeResponse, FakeText, empty_usage, make_planner +from .test_router import make_router + + +# --- the registry --------------------------------------------------------- + + +def test_the_two_standard_skills_are_loaded() -> None: + assert {"diagnose", "qa"} <= set(SKILLS) + + +def test_every_skill_declares_the_name_it_is_invoked_by() -> None: + """A frontmatter name that disagrees with its folder is unreachable.""" + for name, skill in SKILLS.items(): + assert skill.name == name + assert skill.description + assert skill.body + + +def test_an_unknown_skill_is_rejected_rather_than_ignored() -> None: + with pytest.raises(ApolloError) as excinfo: + get_skill("sudo") + + assert excinfo.value.code == 400 + assert excinfo.value.type == "UNKNOWN_SKILL" + assert "diagnose" in excinfo.value.details["available"] + + +# --- the payload field ---------------------------------------------------- + + +def test_a_named_skill_arrives_resolved() -> None: + payload = Payload.from_dict({"content": "/qa", "skill": {"name": "qa"}}) + + assert payload.skill.body == get_skill("qa").body + + +def test_a_skill_that_is_not_an_object_with_a_name_is_rejected() -> None: + with pytest.raises(ApolloError) as excinfo: + Payload.from_dict({"content": "hi", "skill": "qa"}) + + assert excinfo.value.code == 400 + + +def test_no_skill_field_is_the_unchanged_payload() -> None: + assert Payload.from_dict({"content": "hi"}).skill is None + + +# --- stripping the command token ------------------------------------------ + + +def test_the_command_token_goes_and_the_request_stays() -> None: + assert strip_invocation("/diagnose why did this fail?", "diagnose") == "why did this fail?" + + +def test_a_bare_command_leaves_no_request() -> None: + assert strip_invocation("/qa", "qa") == "" + + +def test_a_command_token_is_only_stripped_at_the_start() -> None: + assert strip_invocation("tell me what /qa does", "qa") == "tell me what /qa does" + + +def test_a_longer_word_starting_with_the_name_is_not_the_command() -> None: + assert strip_invocation("/qa-checklist please", "qa") == "/qa-checklist please" + + +# --- the bypass ----------------------------------------------------------- + + +def stub_planner(captured: dict) -> type: + """A PlannerAgent that records the call instead of making one.""" + + class StubPlanner: + def __init__(self, *_args: object) -> None: + pass + + def run(self, **kwargs: object) -> PlannerResult: + captured.update(kwargs) + return PlannerResult( + response="done", + response_segments=[], + attachments=[], + history=[], + usage=empty_usage(), + meta={"agents": ["router", "planner"], "planner_iterations": 0}, + ) + + return StubPlanner + + +def invoke_skill(name: str, content: str) -> tuple[dict, RouterResult]: + router = make_router() + router.config_loader = None + captured: dict = {} + + with patch("global_chat.planner.PlannerAgent", stub_planner(captured)), \ + patch.object(RouterAgent, "_make_routing_decision") as decision: + result = router.route_and_execute( + content=content, + workflow_yaml=None, + page=None, + history=[], + stream=False, + skill=get_skill(name), + ) + + assert decision.call_count == 0 + return captured, result + + +def test_an_invoked_skill_never_pays_for_a_routing_call() -> None: + captured, result = invoke_skill("diagnose", "/diagnose why did this fail?") + + assert captured["skill"].name == "diagnose" + assert result.meta["skill"] == "diagnose" + # The router did not run, so it has neither a place in the path nor a + # confidence to report. + assert "router" not in result.meta["agents"] + assert "router_confidence" not in result.meta + + +def test_the_planner_is_asked_what_the_user_asked_not_the_command() -> None: + captured, _ = invoke_skill("diagnose", "/diagnose why did this fail?") + + assert captured["content"] == "why did this fail?" + + +def test_a_message_that_only_mentions_a_command_is_routed_normally() -> None: + router = make_router() + router.config_loader = None + captured: dict = {} + + with patch("global_chat.planner.PlannerAgent", stub_planner(captured)), \ + patch.object( + RouterAgent, + "_make_routing_decision", + return_value=RouterDecision(destination="planner", confidence=4), + ) as decision: + result = router.route_and_execute( + content="/qa looks odd on this workflow", + workflow_yaml=None, + page=None, + history=[], + stream=False, + ) + + assert decision.call_count == 1 + assert captured["skill"] is None + assert captured["content"] == "/qa looks odd on this workflow" + assert result.meta["router_confidence"] == 4 + + +# --- injection into the turn ---------------------------------------------- + + +def test_the_skill_instructions_lead_the_turn_the_model_sees() -> None: + planner = make_planner() + planner._skill = get_skill("qa") + + user_content = planner._build_user_content("check this", None) + + assert user_content.startswith('') + assert get_skill("qa").body in user_content + assert "check this" in user_content + + +def test_a_bare_invocation_still_carries_the_instructions() -> None: + planner = make_planner() + planner._skill = get_skill("qa") + + assert get_skill("qa").body in planner._build_user_content("", None) + + +def test_the_skill_body_is_not_replayed_on_later_turns() -> None: + """Like an attachment: the model sees it this turn, history keeps the words + the user typed. The client re-sends `skill` on each turn it applies to.""" + planner = make_planner() + planner.model = "test-model" + planner.max_tool_calls = 1 + seen: list = [] + + def fake_api(_system: object, messages: list, *_args: object, **_kwargs: object) -> FakeResponse: + # Copied: run() appends the reply to this same list afterwards. + seen.append(list(messages)) + return FakeResponse("end_turn", [FakeText("here is the review")]) + + with patch.object(planner, "_call_api", side_effect=fake_api), \ + patch.object(planner, "_build_system_prompt", return_value="sys"): + result = planner.run( + content="check this", + workflow_yaml=None, + page=None, + history=[], + stream=False, + skill=get_skill("qa"), + ) + + body = get_skill("qa").body + assert body in seen[0][-1]["content"] + assert [turn["content"] for turn in result.history] == ["check this", "here is the review"] From ef8572ae0391937a1612a37dd1905ef02fc60bcc Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Wed, 30 Sep 2026 19:24:08 +0100 Subject: [PATCH 02/24] Tell the user which skill is running --- services/global_chat/planner.py | 12 ++-- .../global_chat/tests/unit/test_planner.py | 8 ++- .../global_chat/tests/unit/test_skills.py | 55 ++++++++++++++++++- 3 files changed, 66 insertions(+), 9 deletions(-) diff --git a/services/global_chat/planner.py b/services/global_chat/planner.py index 23672458..32b78ac6 100644 --- a/services/global_chat/planner.py +++ b/services/global_chat/planner.py @@ -282,10 +282,6 @@ def run( logger.info("Planner.run() called") stream_manager = stream_manager or StreamManager(model=self.model, stream=stream) - if workflow_yaml: - stream_manager.send_thinking(STATUS_REVIEWING_WORKFLOW + STATUS_PLANNING) - else: - stream_manager.send_thinking(STATUS_NEW_WORKFLOW + STATUS_PLANNING) self.current_yaml = workflow_yaml self.yaml_modified = False @@ -295,8 +291,9 @@ def run( self._segments: List[Dict] = [] self._skill = skill - stream_manager = StreamManager(model=self.model, stream=stream) - if workflow_yaml: + if skill: + self._send_spinner(stream_manager, f"Running the /{skill.name} skill...") + elif workflow_yaml: self._send_spinner(stream_manager, STATUS_REVIEWING_WORKFLOW + STATUS_PLANNING) else: self._send_spinner(stream_manager, STATUS_NEW_WORKFLOW + STATUS_PLANNING) @@ -435,6 +432,9 @@ def run( if response.stop_reason != "end_turn": logger.warning(f"Loop exited without end_turn (reason: {response.stop_reason})") + + if skill: + self._send_settled(stream_manager, f"Ran the /{skill.name} skill") finally: stream_manager.end_stream() diff --git a/services/global_chat/tests/unit/test_planner.py b/services/global_chat/tests/unit/test_planner.py index 75f3b299..67640b72 100644 --- a/services/global_chat/tests/unit/test_planner.py +++ b/services/global_chat/tests/unit/test_planner.py @@ -63,13 +63,17 @@ def __init__(self, name: str, tool_input: dict, block_id: str = "tu_1"): class StubStreamManager: def __init__(self) -> None: self.statuses: list[dict] = [] + self.thinking: list = [] - def send_thinking(self, *_args: object, **_kwargs: object) -> None: - pass + def send_thinking(self, text: object = None, *_args: object, **_kwargs: object) -> None: + self.thinking.append(text) def send_changes(self, *_args: object, **_kwargs: object) -> None: pass + def end_stream(self, *_args: object, **_kwargs: object) -> None: + pass + def send_status( self, content: str, diff --git a/services/global_chat/tests/unit/test_skills.py b/services/global_chat/tests/unit/test_skills.py index f1a988bc..c0ba5c34 100644 --- a/services/global_chat/tests/unit/test_skills.py +++ b/services/global_chat/tests/unit/test_skills.py @@ -19,7 +19,15 @@ from global_chat.skill_registry import SKILLS, get_skill, strip_invocation from util import ApolloError -from .test_planner import FakeResponse, FakeText, empty_usage, make_planner +from streaming_util import STATUS_NEW_WORKFLOW, STATUS_PLANNING + +from .test_planner import ( + FakeResponse, + FakeText, + StubStreamManager, + empty_usage, + make_planner, +) from .test_router import make_router @@ -220,3 +228,48 @@ def fake_api(_system: object, messages: list, *_args: object, **_kwargs: object) body = get_skill("qa").body assert body in seen[0][-1]["content"] assert [turn["content"] for turn in result.history] == ["check this", "here is the review"] + + +# --- telling the user a skill is running ---------------------------------- + + +def run_turn(skill_name: str | None) -> tuple[StubStreamManager, object]: + """One planner turn that answers immediately, on a recording stream.""" + planner = make_planner() + planner.model = "test-model" + planner.max_tool_calls = 1 + stream_manager = StubStreamManager() + + with patch.object(planner, "_call_api", return_value=FakeResponse("end_turn", [FakeText("done")])), \ + patch.object(planner, "_build_system_prompt", return_value="sys"): + result = planner.run( + content="check this", + workflow_yaml=None, + page=None, + history=[], + stream=False, + stream_manager=stream_manager, + skill=get_skill(skill_name) if skill_name else None, + ) + + return stream_manager, result + + +def test_a_skill_turn_opens_by_naming_the_skill() -> None: + stream_manager, _ = run_turn("qa") + + assert stream_manager.thinking == ["Running the /qa skill..."] + + +def test_a_skill_turn_settles_in_the_transcript() -> None: + """Durable, so a reloaded turn still says why it was answered this way.""" + _, result = run_turn("qa") + + assert result.response_segments[-1] == {"type": "status", "content": "Ran the /qa skill"} + + +def test_a_turn_without_a_skill_opens_and_settles_as_before() -> None: + stream_manager, result = run_turn(None) + + assert stream_manager.thinking == [STATUS_NEW_WORKFLOW + STATUS_PLANNING] + assert [segment["type"] for segment in result.response_segments] == ["text"] From fd3533aaacc516b989b88701cd5e76a5b2b1aba9 Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Mon, 5 Oct 2026 09:58:07 +0100 Subject: [PATCH 03/24] Settle the skill status where it was announced --- services/global_chat/planner.py | 4 +--- services/global_chat/tests/unit/test_skills.py | 7 ++++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/services/global_chat/planner.py b/services/global_chat/planner.py index 32b78ac6..1a708cc5 100644 --- a/services/global_chat/planner.py +++ b/services/global_chat/planner.py @@ -293,6 +293,7 @@ def run( if skill: self._send_spinner(stream_manager, f"Running the /{skill.name} skill...") + self._send_settled(stream_manager, f"Ran the /{skill.name} skill") elif workflow_yaml: self._send_spinner(stream_manager, STATUS_REVIEWING_WORKFLOW + STATUS_PLANNING) else: @@ -432,9 +433,6 @@ def run( if response.stop_reason != "end_turn": logger.warning(f"Loop exited without end_turn (reason: {response.stop_reason})") - - if skill: - self._send_settled(stream_manager, f"Ran the /{skill.name} skill") finally: stream_manager.end_stream() diff --git a/services/global_chat/tests/unit/test_skills.py b/services/global_chat/tests/unit/test_skills.py index c0ba5c34..4191e5cc 100644 --- a/services/global_chat/tests/unit/test_skills.py +++ b/services/global_chat/tests/unit/test_skills.py @@ -261,11 +261,12 @@ def test_a_skill_turn_opens_by_naming_the_skill() -> None: assert stream_manager.thinking == ["Running the /qa skill..."] -def test_a_skill_turn_settles_in_the_transcript() -> None: - """Durable, so a reloaded turn still says why it was answered this way.""" +def test_a_skill_turn_settles_where_it_was_announced() -> None: + """Durable, and first: the invocation happens at the top of the turn, so + the settled line stays above the answer rather than trailing it.""" _, result = run_turn("qa") - assert result.response_segments[-1] == {"type": "status", "content": "Ran the /qa skill"} + assert result.response_segments[0] == {"type": "status", "content": "Ran the /qa skill"} def test_a_turn_without_a_skill_opens_and_settles_as_before() -> None: From 53a4a5f19b48a4b6da6a1dbbb1b52a00aee4b658 Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Mon, 5 Oct 2026 19:05:43 +0100 Subject: [PATCH 04/24] Let the planner load skills --- services/global_chat/PAYLOAD_SPEC.md | 3 +- services/global_chat/README.md | 4 ++- services/global_chat/planner.py | 14 +++++++- services/global_chat/skills/README.md | 3 +- .../global_chat/tests/unit/test_skills.py | 35 +++++++++++++++++++ .../global_chat/tools/tool_definitions.py | 26 +++++++++++++- 6 files changed, 80 insertions(+), 5 deletions(-) diff --git a/services/global_chat/PAYLOAD_SPEC.md b/services/global_chat/PAYLOAD_SPEC.md index 37905128..fd53cfd1 100644 --- a/services/global_chat/PAYLOAD_SPEC.md +++ b/services/global_chat/PAYLOAD_SPEC.md @@ -186,7 +186,8 @@ Each tool beat streams as: `thinking` spinner → `changes` (if the workflow was A skill is a reusable instruction set that augments the model's context for one turn when the user explicitly invokes it. Standard skills ship with Apollo, in `services/global_chat/skills//SKILL.md`; they are immutable and upgrade -for everyone on deploy. +for everyone on deploy. The planner can also load a skill itself, through its +`load_skill` tool; that needs nothing from the client. The client recognises the command and names it in `skill` — **Apollo never parses commands out of `content`**. Follow standard slash-command semantics: diff --git a/services/global_chat/README.md b/services/global_chat/README.md index 4794da1e..827d82cd 100644 --- a/services/global_chat/README.md +++ b/services/global_chat/README.md @@ -180,13 +180,15 @@ For straightforward requests, the router calls subagents directly: ### Planner For complex requests, the `PlannerAgent` (Claude Opus) runs an agentic -tool-calling loop with access to four tools: +tool-calling loop with access to five tools: - **`call_workflow_agent`** — create or modify workflow YAML structure - **`call_job_code_agent`** — write or edit job code for a specific job (requires an existing workflow with that job defined) - **`search_documentation`** — semantic search over the OpenFn docsite - **`inspect_job_code`** — read-only inspection of a job's current code +- **`load_skill`** — load a standard skill's instructions when a request + matches it The planner always calls `call_workflow_agent` first to establish the structure, then calls `call_job_code_agent` for each job that needs code. Job code is diff --git a/services/global_chat/planner.py b/services/global_chat/planner.py index 1a708cc5..61c1eb4a 100644 --- a/services/global_chat/planner.py +++ b/services/global_chat/planner.py @@ -25,7 +25,7 @@ STATUS_PLANNING, ) from global_chat.config_loader import ConfigLoader -from global_chat.skill_registry import Skill +from global_chat.skill_registry import SKILLS, Skill from models import resolve_model from global_chat.tools.tool_definitions import TOOL_DEFINITIONS from yaml_utils import stitch_job_code, redact_job_bodies, find_job_in_yaml, get_step_name_from_page, inspect_job_code, job_keys_in_yaml @@ -699,6 +699,12 @@ def _execute_tool(self, tool_use_block, stream_manager, total_usage, tool_calls_ tool_calls_meta.append({"tool": "inspect_job_code", "input": tool_use_block.input}) + elif tool_use_block.name == "load_skill": + skill = SKILLS.get(tool_use_block.input.get("name")) + tool_result = skill.body if skill else f"Error: Unknown skill. Available skills: {sorted(SKILLS)}" + + tool_calls_meta.append({"tool": "load_skill", "input": tool_use_block.input}) + else: logger.error(f"Unknown tool: {tool_use_block.name}") tool_result = f"Error: Unknown tool {tool_use_block.name}" @@ -917,6 +923,9 @@ def _tool_status_message(self, tool_use_block) -> str: return f"Reading code for {joined}..." return "Reading job code..." + if name == "load_skill" and inputs.get("name") in SKILLS: + return f"Running the /{inputs['name']} skill..." + return f"Running {name}..." def _settled_status_message(self, tool_use_block, yaml_before: str | None) -> str | None: @@ -949,6 +958,9 @@ def _settled_status_message(self, tool_use_block, yaml_before: str | None) -> st return f"Read code for {joined}" return "Read code" + if name == "load_skill" and inputs.get("name") in SKILLS: + return f"Ran the /{inputs['name']} skill" + return None def _display_name_for_job(self, job_key: str | None) -> str | None: diff --git a/services/global_chat/skills/README.md b/services/global_chat/skills/README.md index 3d1c6dc8..0ca0a627 100644 --- a/services/global_chat/skills/README.md +++ b/services/global_chat/skills/README.md @@ -6,7 +6,8 @@ format matches what a user-defined skill will look like in v1, so a built-in and a custom skill are the same artifact. Invoked by name via the `skill` payload field, which bypasses the router and -hands the turn to the planner. +hands the turn to the planner. The planner can also load a skill itself, through +its `load_skill` tool, when a request matches the skill's description. **The bodies here are placeholders.** They exercise the mechanism; they are not the real prompts. See https://github.com/OpenFn/apollo/issues/614. diff --git a/services/global_chat/tests/unit/test_skills.py b/services/global_chat/tests/unit/test_skills.py index 4191e5cc..bf63598e 100644 --- a/services/global_chat/tests/unit/test_skills.py +++ b/services/global_chat/tests/unit/test_skills.py @@ -17,6 +17,7 @@ from global_chat.planner import PlannerResult from global_chat.router import RouterAgent, RouterDecision, RouterResult from global_chat.skill_registry import SKILLS, get_skill, strip_invocation +from global_chat.tools.tool_definitions import LOAD_SKILL_TOOL from util import ApolloError from streaming_util import STATUS_NEW_WORKFLOW, STATUS_PLANNING @@ -24,6 +25,7 @@ from .test_planner import ( FakeResponse, FakeText, + FakeToolUse, StubStreamManager, empty_usage, make_planner, @@ -274,3 +276,36 @@ def test_a_turn_without_a_skill_opens_and_settles_as_before() -> None: assert stream_manager.thinking == [STATUS_NEW_WORKFLOW + STATUS_PLANNING] assert [segment["type"] for segment in result.response_segments] == ["text"] + + +# --- the planner loading a skill itself ----------------------------------- + + +def test_the_load_skill_tool_offers_every_registered_skill() -> None: + assert LOAD_SKILL_TOOL["input_schema"]["properties"]["name"]["enum"] == sorted(SKILLS) + for skill in SKILLS.values(): + assert f"- {skill.name}: {skill.description}" in LOAD_SKILL_TOOL["description"] + + +def test_loading_a_skill_returns_its_instructions() -> None: + planner = make_planner() + stream_manager = StubStreamManager() + + results = planner._execute_tool_blocks( + [FakeToolUse("load_skill", {"name": "qa"})], stream_manager, empty_usage(), [], + ) + + assert results[0]["content"] == get_skill("qa").body + assert stream_manager.thinking == ["Running the /qa skill..."] + assert stream_manager.statuses[0]["content"] == "Ran the /qa skill" + + +def test_loading_an_unknown_skill_names_the_real_ones() -> None: + planner = make_planner() + + result = planner._execute_tool( + FakeToolUse("load_skill", {"name": "sudo"}), StubStreamManager(), empty_usage(), [], + ) + + assert result.startswith("Error: Unknown skill") + assert "diagnose" in result diff --git a/services/global_chat/tools/tool_definitions.py b/services/global_chat/tools/tool_definitions.py index 8ad98f58..534a3a68 100644 --- a/services/global_chat/tools/tool_definitions.py +++ b/services/global_chat/tools/tool_definitions.py @@ -4,6 +4,8 @@ These are Claude API tool schemas that define what tools are available. """ +from global_chat.skill_registry import SKILLS + # Tool 1: Search documentation SEARCH_DOCUMENTATION_TOOL = { "name": "search_documentation", @@ -87,10 +89,32 @@ # agents explore the workflow with the exact same tool from yaml_utils import INSPECT_JOB_CODE_TOOL # noqa: E402 +# Tool 5: Load a skill — listed from the registry, so a new skill folder is +# offered to the model without editing this file +LOAD_SKILL_TOOL = { + "name": "load_skill", + "description": """Load a skill: instructions for a kind of task. When the user's request matches a skill below, load it first and follow its instructions for this turn. + +Available skills: +""" + "\n".join(f"- {skill.name}: {skill.description}" for skill in SKILLS.values()), + "input_schema": { + "type": "object", + "properties": { + "name": { + "type": "string", + "enum": sorted(SKILLS), + "description": "The skill to load", + }, + }, + "required": ["name"], + }, +} + # Export all tool definitions TOOL_DEFINITIONS = [ SEARCH_DOCUMENTATION_TOOL, CALL_WORKFLOW_AGENT_TOOL, CALL_JOB_CODE_AGENT_TOOL, - INSPECT_JOB_CODE_TOOL + INSPECT_JOB_CODE_TOOL, + LOAD_SKILL_TOOL, ] From 91711140cb39ba5c2f87270a4499300e87ff57fd Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Mon, 5 Oct 2026 19:13:36 +0100 Subject: [PATCH 05/24] Shorten skill wording --- services/global_chat/skill_registry.py | 2 +- services/global_chat/skills/README.md | 2 ++ services/global_chat/tools/tool_definitions.py | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/services/global_chat/skill_registry.py b/services/global_chat/skill_registry.py index 3e40a096..32fb7441 100644 --- a/services/global_chat/skill_registry.py +++ b/services/global_chat/skill_registry.py @@ -32,7 +32,7 @@ def as_preamble(self) -> str: """The skill as it is injected ahead of the user's request.""" return ( f'\n{self.body}\n\n\n' - f"The user invoked the /{self.name} skill. Follow its instructions for this turn." + f"The user invoked /{self.name}." ) diff --git a/services/global_chat/skills/README.md b/services/global_chat/skills/README.md index 0ca0a627..11180415 100644 --- a/services/global_chat/skills/README.md +++ b/services/global_chat/skills/README.md @@ -9,5 +9,7 @@ Invoked by name via the `skill` payload field, which bypasses the router and hands the turn to the planner. The planner can also load a skill itself, through its `load_skill` tool, when a request matches the skill's description. +Only `SKILL.md` is read. Add a file-read tool before bundling reference files. + **The bodies here are placeholders.** They exercise the mechanism; they are not the real prompts. See https://github.com/OpenFn/apollo/issues/614. diff --git a/services/global_chat/tools/tool_definitions.py b/services/global_chat/tools/tool_definitions.py index 534a3a68..fde9438d 100644 --- a/services/global_chat/tools/tool_definitions.py +++ b/services/global_chat/tools/tool_definitions.py @@ -93,7 +93,7 @@ # offered to the model without editing this file LOAD_SKILL_TOOL = { "name": "load_skill", - "description": """Load a skill: instructions for a kind of task. When the user's request matches a skill below, load it first and follow its instructions for this turn. + "description": """Load a skill: instructions for a kind of task. When the user's request matches a skill below, load it first and follow its instructions. Available skills: """ + "\n".join(f"- {skill.name}: {skill.description}" for skill in SKILLS.values()), From 47b0dba7851bbe3a7396221398f5cf3a763cc0f9 Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Mon, 5 Oct 2026 19:18:34 +0100 Subject: [PATCH 06/24] Keep skills and page prefixes in history --- .changeset/planner-page-prefix.md | 5 ++ .changeset/skills-as-slash-commands.md | 2 +- services/global_chat/PAYLOAD_SPEC.md | 20 +++-- services/global_chat/README.md | 4 +- services/global_chat/planner.py | 46 +++++++--- services/global_chat/router.py | 20 +++-- services/global_chat/skill_registry.py | 24 +++-- .../global_chat/tests/unit/test_planner.py | 1 + .../global_chat/tests/unit/test_skills.py | 87 ++++++++++++++----- services/job_chat/job_chat.py | 18 +--- services/util.py | 12 +++ 11 files changed, 166 insertions(+), 73 deletions(-) create mode 100644 .changeset/planner-page-prefix.md diff --git a/.changeset/planner-page-prefix.md b/.changeset/planner-page-prefix.md new file mode 100644 index 00000000..b513c435 --- /dev/null +++ b/.changeset/planner-page-prefix.md @@ -0,0 +1,5 @@ +--- +"apollo": patch +--- + +global_chat: planner turns now record the `[pg:...]` page prefix in the returned history, like the direct routes, so every route returns history in the same shape diff --git a/.changeset/skills-as-slash-commands.md b/.changeset/skills-as-slash-commands.md index e6077641..a858a532 100644 --- a/.changeset/skills-as-slash-commands.md +++ b/.changeset/skills-as-slash-commands.md @@ -2,4 +2,4 @@ "apollo": minor --- -global_chat: accept a `skill` field naming a standard skill the user invoked by slash command. `/diagnose` and `/qa` ship with the agent; an invoked skill skips the router and leads the planner's turn. Skill instructions are per-turn, so the client re-sends `skill` on every turn it applies to +global_chat: accept a `skill` field naming a standard skill the user invoked by slash command. `/diagnose` and `/qa` ship with the agent; an invoked skill skips the router and leads the planner's turn. Skill instructions are kept in the returned history, so the client sends `skill` only on the turn that invokes it diff --git a/services/global_chat/PAYLOAD_SPEC.md b/services/global_chat/PAYLOAD_SPEC.md index fd53cfd1..ada0315d 100644 --- a/services/global_chat/PAYLOAD_SPEC.md +++ b/services/global_chat/PAYLOAD_SPEC.md @@ -68,7 +68,7 @@ This document defines the input and output payload structure for the Global Agen - **`metrics_opt_in`** (boolean, optional): If `true`, enables Langfuse tracing for this session. The frontend is responsible for setting this; the backend tracks if and only if this flag is `true`. -- **`history`** (array, optional): Conversation history. Each turn has `role` and `content`. History is managed and returned by each agent internally. +- **`history`** (array, optional): Conversation history. Each turn has `role` and `content`. Send back the `history` from the previous response unchanged: Apollo owns its contents, which include `[pg:...]` page prefixes and any skill instructions in use. - **`attachments`** (array, optional): Input attachments providing additional context for the request. Each entry has a `type` and `content` field. Useful for passing logs, dataclips, run inputs/outputs, or other contextual data that the agent can use when processing the request. Currently supported types: - `log` — execution logs from a run @@ -166,7 +166,7 @@ Each tool beat streams as: `thinking` spinner → `changes` (if the workflow was - **`attachments`** (array): Artifacts produced during this turn. Each entry has a `type` and `content` field. An empty list `[]` means no artifacts were produced (e.g. a purely informational response). The only supported type is `workflow_yaml`: the full workflow YAML with any job code changes stitched in. Job code edits are never returned separately — the YAML is the single source of truth, which allows multi-step changes in one response. -- **`history`** (array): Updated conversation history including the latest exchange. Each entry has `content` as a string on every route. On the planner path the assistant entry contains only the final answer text — the pre-tool narration segments in `response` are not persisted to history. +- **`history`** (array): Updated conversation history including the latest exchange, in the shape the next request takes as input. Each entry has `content` as a string on every route, and every user entry carries a `[pg:...]` prefix naming the page it was sent from. On the planner path the assistant entry contains only the final answer text — the pre-tool narration segments in `response` are not persisted to history. - **`usage`** (object): Token usage aggregated across all agents invoked (router + planner + sub-agents). @@ -177,14 +177,14 @@ Each tool beat streams as: `thinking` spinner → `changes` (if the workflow was - **`tool_calls`** (array): List of `{tool, input}` objects for each tool the planner invoked (planner path only). - **`subagent_calls`** (array): Raw sub-agent result dicts including `_call_metadata`. On the planner path these are the full results, useful for debugging. On the router's direct job-code path it carries a single entry with just `_call_metadata` and `diff`, so a client can tell on either route whether a code edit actually landed (`diff.patches_applied`). - **`total_tool_calls`** (number): Total number of tool calls made by the planner (planner path only). - - **`skill`** (string): The skill invoked this turn (skill path only). `router_confidence` is absent on this path, because the router did not run. + - **`skill`** (string): The skill invoked this turn (skill path only). `router_confidence` is absent on this path, and on follow-ups to a skill, because the router did not run. --- ## Skill invocation -A skill is a reusable instruction set that augments the model's context for one -turn when the user explicitly invokes it. Standard skills ship with Apollo, in +A skill is a reusable instruction set that augments the model's context from +the turn the user invokes it. Standard skills ship with Apollo, in `services/global_chat/skills//SKILL.md`; they are immutable and upgrade for everyone on deploy. The planner can also load a skill itself, through its `load_skill` tool; that needs nothing from the client. @@ -204,10 +204,12 @@ Sending `skill` changes the turn in three ways: or the returned history sees it. The rest of the message is untouched. 3. **The skill's instructions lead the user turn.** -Skills travel per-turn, exactly like attachments: the instructions reach the -model but are never written to `history`, so **the client must re-send `skill` -on every turn it should apply to**. A follow-up turn sent without it is routed -normally. +Unlike attachments, the instructions are written into that user turn in the +returned `history`, as in a standard agent transcript, so they keep applying on +later turns. **Send `skill` only on the turn that invokes it.** A skill the +planner loads itself is kept the same way. While a skill is in the history, +every turn goes to the planner without a routing call, since the direct agents +never saw it. ### Example diff --git a/services/global_chat/README.md b/services/global_chat/README.md index 827d82cd..a2ec7439 100644 --- a/services/global_chat/README.md +++ b/services/global_chat/README.md @@ -149,8 +149,8 @@ import. An invoked skill skips the router — a slash command states the intent the router would otherwise guess — and its instructions lead the planner's user turn. They -are per-turn context, like attachments, so the client re-sends `skill` on every -turn it should apply to. See [PAYLOAD_SPEC.md](PAYLOAD_SPEC.md#skill-invocation). +are kept in the returned history, so they keep applying on later turns, which +also go to the planner. See [PAYLOAD_SPEC.md](PAYLOAD_SPEC.md#skill-invocation). ### Routing diff --git a/services/global_chat/planner.py b/services/global_chat/planner.py index 61c1eb4a..aaf0598e 100644 --- a/services/global_chat/planner.py +++ b/services/global_chat/planner.py @@ -17,7 +17,7 @@ sys.path.append(str(Path(__file__).parent.parent)) from langfuse import observe -from util import create_logger, ApolloError, sum_usage, mask_secrets, format_attachments +from util import create_logger, ApolloError, sum_usage, mask_secrets, format_attachments, add_page_prefix, job_code_page from streaming_util import ( StreamManager, STATUS_REVIEWING_WORKFLOW, @@ -242,6 +242,7 @@ def __init__(self, config_loader: ConfigLoader, api_key: Optional[str] = None): self._segments: List[Dict] = [] self._attachments: List[Dict] = [] self._skill: Optional[Skill] = None + self._loaded_skills: Dict[str, Skill] = {} logger.info(f"PlannerAgent initialized with model: {self.model}") @@ -290,6 +291,7 @@ def run( self._metrics_opt_in = metrics_opt_in self._segments: List[Dict] = [] self._skill = skill + self._loaded_skills = {} if skill: self._send_spinner(stream_manager, f"Running the /{skill.name} skill...") @@ -477,9 +479,15 @@ def run( attachments.append({"type": "workflow_yaml", "content": self.current_yaml}) # Return string-content history matching the direct routes, not the - # internal block-format messages used by the tool-calling loop. + # internal block-format messages used by the tool-calling loop. Skills + # used this turn are kept, so they still apply on later turns. + user_turn = "\n\n".join( + [self._with_skill(content), *(s.as_block() for s in self._loaded_skills.values())], + ) return_history = (history.copy() if history else []) - return_history.append({"role": "user", "content": content}) + return_history.append( + {"role": "user", "content": add_page_prefix(user_turn, self._page_context(page, workflow_yaml))}, + ) return_history.append({"role": "assistant", "content": final_text}) return PlannerResult( @@ -497,21 +505,33 @@ def run( }, ) + def _with_skill(self, content: str) -> str: + """The user's message with an invoked skill's instructions ahead of it.""" + if not self._skill: + return content + preamble = self._skill.as_preamble() + return f"{preamble}\n\n{content}" if content else preamble + + def _page_context(self, page: Optional[str], workflow_yaml: Optional[str]) -> Dict: + """The page as the direct routes record it, so a [pg:...] prefix reads + the same whichever agent served the turn.""" + step_name = get_step_name_from_page(page) + if step_name and workflow_yaml: + _, job = find_job_in_yaml(workflow_yaml, step_name) + if job: + return job_code_page(job.get("name"), job.get("adaptor")) + return {"type": "workflow"} + def _build_user_content(self, content: str, page: Optional[str]) -> str: """Augment the user message with an invoked skill's instructions, this turn's attachments, the step the user is viewing ("this step"), and the existing workflow structure (bodies redacted). - Everything added here is per-turn: run() records the raw `content` in - the returned history, so an attached log never becomes a permanent part - of the conversation. A skill travels the same way, which is why the - client re-sends it on every turn it should apply to. + Only the skill outlives this turn: run() records the message and skill + in the returned history, so an attached log never becomes a permanent + part of the conversation. """ - user_content = content - - if self._skill: - preamble = self._skill.as_preamble() - user_content = f"{preamble}\n\n{user_content}" if user_content else preamble + user_content = self._with_skill(content) attachments = format_attachments(self._attachments) if attachments: @@ -702,6 +722,8 @@ def _execute_tool(self, tool_use_block, stream_manager, total_usage, tool_calls_ elif tool_use_block.name == "load_skill": skill = SKILLS.get(tool_use_block.input.get("name")) tool_result = skill.body if skill else f"Error: Unknown skill. Available skills: {sorted(SKILLS)}" + if skill and skill != self._skill: + self._loaded_skills[skill.name] = skill tool_calls_meta.append({"tool": "load_skill", "input": tool_use_block.input}) diff --git a/services/global_chat/router.py b/services/global_chat/router.py index 8b6dd11e..9bcdecae 100644 --- a/services/global_chat/router.py +++ b/services/global_chat/router.py @@ -21,7 +21,7 @@ from util import create_logger, ApolloError, sum_usage, attachments_to_context from streaming_util import StreamManager from global_chat.config_loader import ConfigLoader -from global_chat.skill_registry import Skill, strip_invocation +from global_chat.skill_registry import Skill, has_skill, strip_invocation from models import resolve_model from yaml_utils import get_step_name_from_page, get_page_view, find_job_in_yaml, stitch_job_code, workflow_has_job_code @@ -120,7 +120,9 @@ def route_and_execute( # a second message lifecycle. self._stream_manager = StreamManager(model=self.model, stream=stream) - if skill: + # A skill stays in the conversation once used, so its follow-ups stay + # with the planner rather than reaching an agent that never saw it. + if skill or has_skill(history): return self._route_to_skill(skill, content, workflow_yaml, page, history, stream) try: @@ -473,23 +475,24 @@ def _track_reroute(self, metadata: Dict) -> None: def _route_to_skill( self, - skill: Skill, + skill: Optional[Skill], content: str, workflow_yaml: Optional[str], page: Optional[str], history: List[Dict], stream: bool, ) -> RouterResult: - """Hand an invoked skill straight to the planner, skipping the router. + """Hand a skill turn straight to the planner, skipping the router. The router guesses intent; a slash command states it, so there is nothing to decide. Both standard skills are multi-step tasks, which is - what the planner is for. + what the planner is for. `skill` is None on a follow-up to a skill + already in the history. """ - logger.info(f"Skill '{skill.name}' invoked, bypassing router") + logger.info(f"Skill turn ({skill.name if skill else 'from history'}), bypassing router") result = self._route_to_planner( - strip_invocation(content, skill.name), + strip_invocation(content, skill.name) if skill else content, workflow_yaml, page, history, @@ -498,7 +501,8 @@ def _route_to_skill( skill=skill, ) result.meta["agents"] = [a for a in result.meta["agents"] if a != "router"] - result.meta["skill"] = skill.name + if skill: + result.meta["skill"] = skill.name return result def _route_to_planner( diff --git a/services/global_chat/skill_registry.py b/services/global_chat/skill_registry.py index 32fb7441..8c30024d 100644 --- a/services/global_chat/skill_registry.py +++ b/services/global_chat/skill_registry.py @@ -28,12 +28,13 @@ class Skill: description: str body: str + def as_block(self) -> str: + """The instructions, tagged so a later turn can tell a skill is in use.""" + return f'\n{self.body}\n' + def as_preamble(self) -> str: - """The skill as it is injected ahead of the user's request.""" - return ( - f'\n{self.body}\n\n\n' - f"The user invoked /{self.name}." - ) + """The skill as it is injected ahead of the request that invoked it.""" + return f"{self.as_block()}\n\nThe user invoked /{self.name}." def _parse_skill_file(path: Path, folder: str) -> Skill: @@ -84,6 +85,19 @@ def get_skill(name: str) -> Skill: return skill +_SKILL_BLOCK = re.compile(r'') + + +def has_skill(history: list[dict]) -> bool: + """Whether a skill was invoked or loaded earlier in the conversation.""" + return any( + turn.get("role") == "user" + and isinstance(turn.get("content"), str) + and _SKILL_BLOCK.search(turn["content"]) + for turn in history + ) + + def strip_invocation(content: str, name: str) -> str: """Drop the leading /name token, leaving what the user actually asked. diff --git a/services/global_chat/tests/unit/test_planner.py b/services/global_chat/tests/unit/test_planner.py index 67640b72..a3a647d4 100644 --- a/services/global_chat/tests/unit/test_planner.py +++ b/services/global_chat/tests/unit/test_planner.py @@ -36,6 +36,7 @@ def make_planner() -> PlannerAgent: planner._segments = [] planner._attachments = [] planner._skill = None + planner._loaded_skills = {} planner.api_key = "test-key" planner._user = None planner._metrics_opt_in = None diff --git a/services/global_chat/tests/unit/test_skills.py b/services/global_chat/tests/unit/test_skills.py index bf63598e..8bec675f 100644 --- a/services/global_chat/tests/unit/test_skills.py +++ b/services/global_chat/tests/unit/test_skills.py @@ -2,8 +2,9 @@ A slash command states the intent the router would otherwise guess, so an invoked skill skips the routing call and goes to the planner. The skill's -instructions are per-turn context, like attachments: the model sees them, the -returned history does not. +instructions are kept in the returned history, as in a standard agent +transcript, so they still apply on later turns, and those turns stay with the +planner too. Apollo never parses commands out of free text — the client recognises the command and names it in the payload — so a message that merely mentions a @@ -16,7 +17,7 @@ from global_chat.global_chat import Payload from global_chat.planner import PlannerResult from global_chat.router import RouterAgent, RouterDecision, RouterResult -from global_chat.skill_registry import SKILLS, get_skill, strip_invocation +from global_chat.skill_registry import SKILLS, get_skill, has_skill, strip_invocation from global_chat.tools.tool_definitions import LOAD_SKILL_TOOL from util import ApolloError @@ -203,33 +204,79 @@ def test_a_bare_invocation_still_carries_the_instructions() -> None: assert get_skill("qa").body in planner._build_user_content("", None) -def test_the_skill_body_is_not_replayed_on_later_turns() -> None: - """Like an attachment: the model sees it this turn, history keeps the words - the user typed. The client re-sends `skill` on each turn it applies to.""" +def run_planner(skill_name: str | None = None, tool_calls: list | None = None, **kwargs: object) -> object: + """One planner turn: the given tool calls, if any, then the answer.""" planner = make_planner() planner.model = "test-model" - planner.max_tool_calls = 1 - seen: list = [] - - def fake_api(_system: object, messages: list, *_args: object, **_kwargs: object) -> FakeResponse: - # Copied: run() appends the reply to this same list afterwards. - seen.append(list(messages)) - return FakeResponse("end_turn", [FakeText("here is the review")]) + planner.max_tool_calls = 5 + responses = [FakeResponse("tool_use", [call]) for call in tool_calls or []] + responses.append(FakeResponse("end_turn", [FakeText("here is the review")])) - with patch.object(planner, "_call_api", side_effect=fake_api), \ + with patch.object(planner, "_call_api", side_effect=responses), \ patch.object(planner, "_build_system_prompt", return_value="sys"): - result = planner.run( + return planner.run( content="check this", + workflow_yaml=kwargs.get("workflow_yaml"), + page=kwargs.get("page"), + history=[], + stream=False, + skill=get_skill(skill_name) if skill_name else None, + ) + + +def test_an_invoked_skill_is_kept_in_history() -> None: + result = run_planner("qa") + + user_turn = result.history[0]["content"] + assert user_turn == f"[pg:workflow] {get_skill('qa').as_preamble()}\n\ncheck this" + assert has_skill(result.history) + + +def test_a_skill_the_planner_loaded_is_kept_in_history() -> None: + result = run_planner(tool_calls=[FakeToolUse("load_skill", {"name": "qa"})]) + + assert result.history[0]["content"] == f"[pg:workflow] check this\n\n{get_skill('qa').as_block()}" + + +def test_a_turn_without_a_skill_keeps_only_the_message() -> None: + result = run_planner() + + assert [turn["content"] for turn in result.history] == ["[pg:workflow] check this", "here is the review"] + assert not has_skill(result.history) + + +def test_a_planner_turn_on_a_step_records_the_page_like_job_chat() -> None: + yaml = 'jobs:\n fetch:\n name: Fetch Data\n adaptor: "@openfn/language-http@6.0.0"\n body: ""\n' + + result = run_planner(workflow_yaml=yaml, page="workflows/wf/fetch") + + assert result.history[0]["content"] == "[pg:job_code/Fetch Data/http@6.0.0] check this" + + +def test_a_follow_up_to_a_skill_stays_with_the_planner() -> None: + router = make_router() + router.config_loader = None + captured: dict = {} + history = [ + {"role": "user", "content": f"{get_skill('qa').as_preamble()}\n\ncheck this"}, + {"role": "assistant", "content": "which step?"}, + ] + + with patch("global_chat.planner.PlannerAgent", stub_planner(captured)), \ + patch.object(RouterAgent, "_make_routing_decision") as decision: + result = router.route_and_execute( + content="/qa the first one", workflow_yaml=None, page=None, - history=[], + history=history, stream=False, - skill=get_skill("qa"), ) - body = get_skill("qa").body - assert body in seen[0][-1]["content"] - assert [turn["content"] for turn in result.history] == ["check this", "here is the review"] + assert decision.call_count == 0 + assert captured["skill"] is None + # Not an invocation, so nothing is stripped and no skill is reported. + assert captured["content"] == "/qa the first one" + assert "skill" not in result.meta # --- telling the user a skill is running ---------------------------------- diff --git a/services/job_chat/job_chat.py b/services/job_chat/job_chat.py index 919040ed..bda188c3 100644 --- a/services/job_chat/job_chat.py +++ b/services/job_chat/job_chat.py @@ -19,7 +19,7 @@ import sentry_sdk from langfuse import observe, propagate_attributes, get_client as get_langfuse_client from langfuse_util import should_track, build_tags, build_generation_diff, mask_secrets -from util import ApolloError, create_logger, AdaptorSpecifier, add_page_prefix, APOLLO_VERSION +from util import ApolloError, create_logger, add_page_prefix, job_code_page, APOLLO_VERSION from yaml_utils import INSPECT_JOB_CODE_TOOL, inspect_job_code from .prompt import build_prompt, build_error_correction_prompt from .old_prompt import build_old_prompt @@ -766,21 +766,7 @@ def main(data_dict: dict) -> dict: if data.context is None: data.context = {} - # Construct current_page from context - page_name = data.context.get("page_name") - adaptor_string = data.context.get("adaptor") - - current_page = { - "type": "job_code", - "name": page_name - } - - if adaptor_string: - try: - adaptor = AdaptorSpecifier(adaptor_string) - current_page["adaptor"] = f"{adaptor.short_name}@{adaptor.version}" - except Exception as e: - logger.warning(f"Failed to parse adaptor string '{adaptor_string}': {e}") + current_page = job_code_page(data.context.get("page_name"), data.context.get("adaptor")) # Extract rag_data from meta if present rag_data = input_meta.get("rag") if isinstance(input_meta, dict) else None diff --git a/services/util.py b/services/util.py index 1be26a92..88040322 100644 --- a/services/util.py +++ b/services/util.py @@ -313,6 +313,18 @@ def add_page_prefix(content: str, page: dict | None) -> str: return f"{prefix} {content}" +def job_code_page(name: str | None, adaptor: str | None) -> dict: + """The page dict for a step's code page, shared so every route records it alike.""" + page = {"type": "job_code", "name": name} + if adaptor: + try: + spec = AdaptorSpecifier(adaptor) + page["adaptor"] = f"{spec.short_name}@{spec.version}" + except Exception as e: + create_logger("util").warning(f"Failed to parse adaptor string '{adaptor}': {e}") + return page + + def is_docsite_collection(name: str) -> bool: """ True for docsite collection (namespace) names created by embed_docsite: From ccc95db751c7469f2457924a084c8c6a04f55d7f Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Tue, 6 Oct 2026 20:03:18 +0100 Subject: [PATCH 07/24] Draft design, qa and diagnose skills --- services/global_chat/skills/design/SKILL.md | 52 +++++++++++++++++++ services/global_chat/skills/diagnose/SKILL.md | 32 +++++++++--- services/global_chat/skills/qa/SKILL.md | 39 ++++++++++---- 3 files changed, 106 insertions(+), 17 deletions(-) create mode 100644 services/global_chat/skills/design/SKILL.md diff --git a/services/global_chat/skills/design/SKILL.md b/services/global_chat/skills/design/SKILL.md new file mode 100644 index 00000000..5e79f89e --- /dev/null +++ b/services/global_chat/skills/design/SKILL.md @@ -0,0 +1,52 @@ +--- +name: design +description: Work out with the user what a workflow should do, when they don't yet know its shape +--- + +The user wants to think the design through before anything is built. Treat it +as a consultation: suggest ideas and sketch options freely, but build only +when they're ready. + +Find out where they are and meet them there. Some don't yet know what could be +automated or how many workflows it takes; others have the shape and need help +with the details. Follow their lead on how deep to go. + +If the user struggles to specify what they want, start from people and process +rather than systems. Wherever the conversation starts, make sure the design +rests on the process it serves, and question requirements against how the +result will be used; the simpler design is often enough. The result is often a mix +of automated steps and steps people keep doing. + +Areas to draw on as the conversation needs them, not a list to work through: +- Value: what problem automation solves, what's done by hand today and what it + costs in time, and what happens if nothing changes. This tells you whether + and how much to automate. +- Process: who does each step, what depends on what, and what's redundant. +- Access: how each system can be reached (API, webhooks, file exports, direct + database), what's in the way (hosting, VPN, firewall, IP allowlisting), and + whether there's a test environment. Suggest getting credentials early so + access can be tested. If a system or its API is still being built or + configured, say it may be too early to integrate with it. +- Timing and volume: whether it really needs to be immediate, whether data + flows one way or both, and how much moves per run, which decides whether API + limits and paging matter. +- Ownership: who owns each system and its data, and who will run and maintain + the workflow, how technical they are, and who decides on changes later. +- Data: real sample records from each side, whether records move one to one + or are summarised, fields the target needs that users never see, and shared + lists (regions, services) and where they should live. +- Failure: who hears about it, and how a run is redone without duplicates. + +Ask a few questions at a time, most consequential first. When there's a real +choice, recommend one option and name the alternative. Prefer the simplest +design that meets the need, and say when an idea goes beyond what was asked. + +Flag details that rest on assumptions or documentation and still need +checking, and help the user see how to check them, without holding up the +design. If you're unsure how the platform behaves, say so rather than design +around a guess. Note open decisions and who can answer each rather than +filling them in. + +When the design is agreed, summarise it at the level of detail you reached, in +a form the user could share with the people who need to agree to it, with any +open decisions. Then offer to build it. diff --git a/services/global_chat/skills/diagnose/SKILL.md b/services/global_chat/skills/diagnose/SKILL.md index d76bf4a1..f53572da 100644 --- a/services/global_chat/skills/diagnose/SKILL.md +++ b/services/global_chat/skills/diagnose/SKILL.md @@ -3,11 +3,27 @@ name: diagnose description: Diagnose a failed workflow run from its logs, dataclips and workflow YAML --- -Diagnose why the run the user is asking about failed. Do not fix anything -unless they ask. - -1. Read the run logs and dataclips attached to this turn. If none are attached, - say what you need and stop. -2. Find the first real failure, not the errors that followed from it. -3. Report the failing step, the cause, and the evidence you read it from. -4. Offer a fix as a next step, and wait to be asked. +Diagnose why the run failed, or why it didn't do what the user expected. + +Work from the run logs and dataclips attached to this turn; if none are +attached, say what you need and stop. You can't rerun anything, so a cause the +evidence doesn't show is a hypothesis, and you should call it one. + +Find the error that started the failure and trace it into the code: which step +and operation raised it, and what data it was handling (often state a previous +step didn't write under that name, an adaptor overwriting `data`, or input +carried over from an earlier scheduled run). Point outside the code (a +credential, the connection, a target system's configuration) when the error +itself shows the cause is there. If the platform stopped the run (killed for +time, memory or state size, or lost), the cause is usually how much the code +holds or how long it runs rather than a wrong line, and missing logs aren't +evidence of a code bug. + +Look past the first failure too: read the later steps against the data they +would receive. Mention what you only suspect as something to watch. + +Tell the user what failed, why, and what showed you. If you're not certain, +give the likely causes and the one check that would tell them apart. When the +fix is contained, make it, along with anything you know is wrong in the later +steps. When it would change behaviour beyond the bug or needs the user's +decision, propose it first. diff --git a/services/global_chat/skills/qa/SKILL.md b/services/global_chat/skills/qa/SKILL.md index 98293cf9..ace2257a 100644 --- a/services/global_chat/skills/qa/SKILL.md +++ b/services/global_chat/skills/qa/SKILL.md @@ -1,14 +1,35 @@ --- name: qa -description: Review a workflow against OpenFn conventions and report what should change +description: Review a workflow against what it is meant to do and report what should change --- -Review the workflow the user is asking about and report what should change. Do -not edit it unless they ask. +Review the workflow against what it's meant to do, taken from the user, a spec +or ticket they've shared, or the workflow itself. If you can't tell, ask. With +a spec, say which parts are met, partly met or missing. -1. Read the workflow structure and the job code of every step. -2. Check each step for: unhandled errors, hardcoded values that belong in - state or credentials, operations that do not exist in the step's adaptor, - and steps doing work that belongs in another step. -3. Report findings worst first, each naming the step and what to change. -4. Say plainly if you found nothing worth changing. +Work outwards in three stages: + +1. Each step on its own. Have the job code agent review and fix every step in + parallel. The goal is that each step is right on its own terms: valid code, + correct use of its adaptor, and doing what the spec or user asks of it. + Making the steps fit together is the next stage's job. +2. One run, end to end. Read every step's code yourself with + `inspect_job_code` and follow the data from the trigger to the last step. Each step should read what the steps before it + actually write, under the same names and in the same shape; edges should + route data where it should go; and no two steps should do the same work. To + fix a mismatch, decide which side is right and give every affected step the + same instruction. +3. Many runs, once live. Data lost, duplicated, corrupted or exposed without + anyone noticing (often a re-run that creates duplicates, a cursor or paging + that skips records, or a lookup that assumes exactly one match) matters + more than a step that fails loudly, as does work that runs away (unbounded + retries, loops or calls). So does anything left over from testing that + shouldn't go live. + +If dataclips are attached, check that the output means what the input meant. + +Tell the user what you fixed and what should still change, worst first, with +the step. Say when a problem is inferred from the code rather than seen in the +data. If nothing needs changing, say so. On a follow-up review, say which +earlier issues are fixed and which aren't. Propose a fix first when it would +change behaviour beyond the problem or needs the user's decision. From e54c4005bcf276c3141f86ec0c3abfe381eb2ddb Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Tue, 6 Oct 2026 20:04:52 +0100 Subject: [PATCH 08/24] Drop placeholder note from skills README --- services/global_chat/skills/README.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/services/global_chat/skills/README.md b/services/global_chat/skills/README.md index 11180415..67c7e268 100644 --- a/services/global_chat/skills/README.md +++ b/services/global_chat/skills/README.md @@ -10,6 +10,3 @@ hands the turn to the planner. The planner can also load a skill itself, through its `load_skill` tool, when a request matches the skill's description. Only `SKILL.md` is read. Add a file-read tool before bundling reference files. - -**The bodies here are placeholders.** They exercise the mechanism; they are not -the real prompts. See https://github.com/OpenFn/apollo/issues/614. From 7d76d4fd3681bb0e39f9b3fdcb1e46e263d5734d Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Wed, 7 Oct 2026 16:20:44 +0100 Subject: [PATCH 09/24] Let the planner load skills unprompted --- services/global_chat/prompts.yaml | 2 +- services/global_chat/tools/tool_definitions.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/global_chat/prompts.yaml b/services/global_chat/prompts.yaml index 26d9f845..320721bc 100644 --- a/services/global_chat/prompts.yaml +++ b/services/global_chat/prompts.yaml @@ -46,7 +46,7 @@ prompts: planner_system_prompt: | You are an OpenFn assistant helping users with workflows and job code. - Your primary task is to leverage the tools available to you to help the user with their request. + Your primary task is to leverage the tools and skills available to you to help the user with their request. Before using your tools, assess whether you have enough information to build the workflow structure (which systems are involved, the steps, triggers, and flow). diff --git a/services/global_chat/tools/tool_definitions.py b/services/global_chat/tools/tool_definitions.py index fde9438d..99d4e9d1 100644 --- a/services/global_chat/tools/tool_definitions.py +++ b/services/global_chat/tools/tool_definitions.py @@ -93,7 +93,7 @@ # offered to the model without editing this file LOAD_SKILL_TOOL = { "name": "load_skill", - "description": """Load a skill: instructions for a kind of task. When the user's request matches a skill below, load it first and follow its instructions. + "description": """Load a skill: instructions for a kind of task. When the task matches a skill below, load it first and follow its instructions. Available skills: """ + "\n".join(f"- {skill.name}: {skill.description}" for skill in SKILLS.values()), From 4e9214c8ef0133e5c5843f9126a1bf0d6199dc5b Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Thu, 8 Oct 2026 16:48:37 +0100 Subject: [PATCH 10/24] Let the planner hand skills to the job agent --- services/global_chat/skill_registry.py | 21 ++++- services/global_chat/skills/README.md | 5 ++ services/global_chat/skills/qa-code/SKILL.md | 23 ++++++ services/global_chat/skills/qa/SKILL.md | 82 ++++++++++++++----- services/global_chat/subagent_caller.py | 9 ++ .../global_chat/tests/unit/test_skills.py | 48 ++++++++++- .../global_chat/tools/tool_definitions.py | 9 +- 7 files changed, 171 insertions(+), 26 deletions(-) create mode 100644 services/global_chat/skills/qa-code/SKILL.md diff --git a/services/global_chat/skill_registry.py b/services/global_chat/skill_registry.py index 8c30024d..bf22f9c3 100644 --- a/services/global_chat/skill_registry.py +++ b/services/global_chat/skill_registry.py @@ -19,6 +19,9 @@ SKILLS_DIR = Path(__file__).parent / "skills" +# Subagents a skill can be written for, via `metadata.agent` in its frontmatter +SUBAGENTS = {"job_code"} + @dataclass(frozen=True) class Skill: @@ -27,6 +30,7 @@ class Skill: name: str description: str body: str + agent: str | None = None def as_block(self) -> str: """The instructions, tagged so a later turn can tell a skill is in use.""" @@ -36,6 +40,10 @@ def as_preamble(self) -> str: """The skill as it is injected ahead of the request that invoked it.""" return f"{self.as_block()}\n\nThe user invoked /{self.name}." + def as_subagent_preamble(self) -> str: + """The skill as it is injected ahead of the planner's message to a subagent.""" + return f"{self.as_block()}\n\nThe request below overrides these instructions where they conflict." + def _parse_skill_file(path: Path, folder: str) -> Skill: text = path.read_text() @@ -58,7 +66,11 @@ def _parse_skill_file(path: Path, folder: str) -> Skill: if not body.strip(): raise ValueError(f"{path} has no instructions") - return Skill(name=name, description=description, body=body.strip()) + agent = (meta.get("metadata") or {}).get("agent") + if agent is not None and agent not in SUBAGENTS: + raise ValueError(f"{path} targets unknown agent '{agent}'") + + return Skill(name=name, description=description, body=body.strip(), agent=agent) def _load_skills() -> dict[str, Skill]: @@ -69,7 +81,12 @@ def _load_skills() -> dict[str, Skill]: } -SKILLS = _load_skills() +_ALL_SKILLS = _load_skills() + +# What users invoke and the planner loads. A skill written for a subagent is +# only ever attached to that subagent's calls. +SKILLS = {name: skill for name, skill in _ALL_SKILLS.items() if skill.agent is None} +JOB_AGENT_SKILLS = {name: skill for name, skill in _ALL_SKILLS.items() if skill.agent == "job_code"} def get_skill(name: str) -> Skill: diff --git a/services/global_chat/skills/README.md b/services/global_chat/skills/README.md index 67c7e268..089fa645 100644 --- a/services/global_chat/skills/README.md +++ b/services/global_chat/skills/README.md @@ -9,4 +9,9 @@ Invoked by name via the `skill` payload field, which bypasses the router and hands the turn to the planner. The planner can also load a skill itself, through its `load_skill` tool, when a request matches the skill's description. +A skill with `metadata: {agent: job_code}` in its frontmatter is written for +the job code agent. It can't be invoked or loaded by the planner; the planner +names it on a `call_job_code_agent` call, and its instructions are put ahead of +the planner's message, which overrides them where they conflict. + Only `SKILL.md` is read. Add a file-read tool before bundling reference files. diff --git a/services/global_chat/skills/qa-code/SKILL.md b/services/global_chat/skills/qa-code/SKILL.md new file mode 100644 index 00000000..8353b518 --- /dev/null +++ b/services/global_chat/skills/qa-code/SKILL.md @@ -0,0 +1,23 @@ +--- +name: qa-code +description: Review and fix one step's code on its own terms, as part of a workflow review +metadata: + agent: job_code +--- + +Review this step and fix what's wrong within it, so that it's right on its own +terms. Other steps are being reviewed at the same time, and fitting them +together comes later, so hold the step to the spec and the user rather than to +what other steps currently do. + +Cover: +- valid code that follows OpenFn job-writing conventions and uses its adaptor + correctly; +- doing what the spec or user asks of this step; +- state overwritten inside loops; +- errors swallowed or never surfaced; +- values hardcoded that belong in configuration; +- comments that contradict the code. + +Report cleanup (duplication, dead code, unclear names) rather than making it. +Reply briefly with what you fixed and what should still change. diff --git a/services/global_chat/skills/qa/SKILL.md b/services/global_chat/skills/qa/SKILL.md index ace2257a..e35e05f0 100644 --- a/services/global_chat/skills/qa/SKILL.md +++ b/services/global_chat/skills/qa/SKILL.md @@ -5,31 +5,71 @@ description: Review a workflow against what it is meant to do and report what sh Review the workflow against what it's meant to do, taken from the user, a spec or ticket they've shared, or the workflow itself. If you can't tell, ask. With -a spec, say which parts are met, partly met or missing. +a spec, say which parts are met, partly met or missing, including edge cases +it implies but doesn't state. -Work outwards in three stages: +Work outwards in three stages. 1. Each step on its own. Have the job code agent review and fix every step in - parallel. The goal is that each step is right on its own terms: valid code, - correct use of its adaptor, and doing what the spec or user asks of it. - Making the steps fit together is the next stage's job. + parallel with the `qa-code` skill, which covers each step's own + correctness: OpenFn conventions, adaptor use, state in loops, swallowed + errors, hardcoded values and misleading comments. Its message only needs + what this conversation adds, such as what the spec asks of that step or + limits the user has set, like leaving comments alone. Making the steps fit + together is the next stage's job. + 2. One run, end to end. Read every step's code yourself with - `inspect_job_code` and follow the data from the trigger to the last step. Each step should read what the steps before it - actually write, under the same names and in the same shape; edges should - route data where it should go; and no two steps should do the same work. To - fix a mismatch, decide which side is right and give every affected step the - same instruction. -3. Many runs, once live. Data lost, duplicated, corrupted or exposed without - anyone noticing (often a re-run that creates duplicates, a cursor or paging - that skips records, or a lookup that assumes exactly one match) matters - more than a step that fails loudly, as does work that runs away (unbounded - retries, loops or calls). So does anything left over from testing that - shouldn't go live. + `inspect_job_code` and follow the data from the trigger to the last step. + To fix a mismatch, decide which side is right and give every affected step + the same instruction. Check that: + - the trigger is the right kind and enabled as intended; + - each step runs after everything it depends on; + - each step reads what the steps before it actually write, under the same + names, in the same shape, and as plain JSON, since anything else is lost + between steps; + - edges route data where it should go, so a failed or unmatched record + can't fall through; + - no two steps do the same work; + - each step uses its system's own adaptor where one exists rather than + plain HTTP; + - credentials are referenced, never written into code; + - names other workflows rely on, such as collections, match exactly. + +3. Many runs, once live. What fails quietly matters more than what fails + loudly. Check for: + - data lost, duplicated, corrupted or exposed without anyone noticing, + often through a re-run that creates duplicates, a cursor or paging that + skips records, or a lookup that assumes exactly one match; + - personal data logged or left on the final state, which OpenFn doesn't + scrub; + - work that runs away: unbounded retries, loops or calls; + - incoming data written without being validated; + - what happens when a system fails or returns malformed data; + - one bad record failing a whole batch, or disappearing silently, unless + that's intended; + - steps passing on more than the next one needs; + - adaptors on `@latest`, which can change under a live workflow and break + it. Pin the version the workflow was tested on, which run logs often + show; if you can't tell, say the pinned version is untested; + - anything left over from testing that shouldn't go live. If dataclips are attached, check that the output means what the input meant. -Tell the user what you fixed and what should still change, worst first, with -the step. Say when a problem is inferred from the code rather than seen in the -data. If nothing needs changing, say so. On a follow-up review, say which -earlier issues are fixed and which aren't. Propose a fix first when it would -change behaviour beyond the problem or needs the user's decision. +Tell the user what you fixed, then what should still change: +- Group it by whether it must, should or could change, worst first, naming the + step and the operation. Anything that breaks, loses or exposes data, or + breaks OpenFn's job-writing conventions, is a must. +- Offer cleanup as could-change in a line or two, and do it only if the user + wants that kind of tidying. +- List a problem shared by several steps once. +- For a suggested change, show only the lines that change, with the + recommended fix and any alternative in a line. +- Skip style nits a formatter would catch and speculative performance + concerns. +- Say when a problem is inferred from the code rather than seen in the data. +- If nothing needs changing, say so. On a follow-up review, say which earlier + issues are fixed and which aren't. + +Propose a fix first when it would change behaviour beyond the problem or needs +the user's decision. If you changed anything, remind the user to test the +workflow again before relying on it. diff --git a/services/global_chat/subagent_caller.py b/services/global_chat/subagent_caller.py index dc446439..0d6058aa 100644 --- a/services/global_chat/subagent_caller.py +++ b/services/global_chat/subagent_caller.py @@ -12,6 +12,7 @@ sys.path.append(str(Path(__file__).parent.parent.parent)) from langfuse import observe +from global_chat.skill_registry import JOB_AGENT_SKILLS from util import create_logger, ApolloError, attachments_to_context, select_attachments from yaml_utils import find_job_in_yaml @@ -106,6 +107,14 @@ def call_job_agent( if not user_message: raise ApolloError(400, "message is required") + skill_name = tool_input.get("skill") + if skill_name: + skill = JOB_AGENT_SKILLS.get(skill_name) + if skill: + user_message = f"{skill.as_subagent_preamble()}\n\n{user_message}" + else: + logger.warning(f"job_agent: ignoring unknown skill '{skill_name}'") + job_context = {} job_key = tool_input.get("job_key") diff --git a/services/global_chat/tests/unit/test_skills.py b/services/global_chat/tests/unit/test_skills.py index 470580c9..71536000 100644 --- a/services/global_chat/tests/unit/test_skills.py +++ b/services/global_chat/tests/unit/test_skills.py @@ -17,13 +17,16 @@ from global_chat.global_chat import Payload from global_chat.planner import PlannerResult from global_chat.router import RouterAgent, RouterDecision, RouterResult -from global_chat.skill_registry import SKILLS, get_skill, has_skill, strip_invocation -from global_chat.tools.tool_definitions import LOAD_SKILL_TOOL +from global_chat.skill_registry import JOB_AGENT_SKILLS, SKILLS, get_skill, has_skill, strip_invocation +from global_chat.subagent_caller import call_job_agent +from global_chat.tools.tool_definitions import CALL_JOB_CODE_AGENT_TOOL, LOAD_SKILL_TOOL from util import ApolloError from streaming_util import STATUS_NEW_WORKFLOW, STATUS_PLANNING +from .test_attachments import job_chat_result from .test_planner import ( + WORKFLOW_YAML, FakeResponse, FakeText, FakeToolUse, @@ -356,3 +359,44 @@ def test_loading_an_unknown_skill_names_the_real_ones() -> None: assert result.startswith("Error: Unknown skill") assert "diagnose" in result + + +# --- skills written for a subagent ---------------------------------------- + + +def test_a_subagent_skill_is_neither_invoked_nor_loaded_by_the_planner() -> None: + assert "qa-code" in JOB_AGENT_SKILLS + assert "qa-code" not in SKILLS + assert "qa-code" not in LOAD_SKILL_TOOL["input_schema"]["properties"]["name"]["enum"] + with pytest.raises(ApolloError): + get_skill("qa-code") + + +def test_the_job_code_agent_tool_offers_every_job_agent_skill() -> None: + skill_param = CALL_JOB_CODE_AGENT_TOOL["input_schema"]["properties"]["skill"] + assert skill_param["enum"] == sorted(JOB_AGENT_SKILLS) + + +def test_a_named_subagent_skill_leads_the_job_agents_message() -> None: + with patch("job_chat.job_chat.main", return_value=job_chat_result()) as mock_main: + call_job_agent( + {"message": "leave the comments alone", "job_key": "fetch-patients", + "attachments": [], "skill": "qa-code"}, + workflow_yaml=WORKFLOW_YAML, + ) + + content = mock_main.call_args[0][0]["content"] + assert content.startswith(JOB_AGENT_SKILLS["qa-code"].as_block()) + # The planner's message is what this conversation adds, so it wins + assert "overrides these instructions" in content + assert content.endswith("leave the comments alone") + + +def test_an_unknown_subagent_skill_is_dropped_not_fatal() -> None: + with patch("job_chat.job_chat.main", return_value=job_chat_result()) as mock_main: + call_job_agent( + {"message": "fix it", "job_key": "fetch-patients", "attachments": [], "skill": "sudo"}, + workflow_yaml=WORKFLOW_YAML, + ) + + assert mock_main.call_args[0][0]["content"] == "fix it" diff --git a/services/global_chat/tools/tool_definitions.py b/services/global_chat/tools/tool_definitions.py index 44ada27d..ceefab2b 100644 --- a/services/global_chat/tools/tool_definitions.py +++ b/services/global_chat/tools/tool_definitions.py @@ -4,7 +4,7 @@ These are Claude API tool schemas that define what tools are available. """ -from global_chat.skill_registry import SKILLS +from global_chat.skill_registry import JOB_AGENT_SKILLS, SKILLS # Tool 1: Search documentation SEARCH_DOCUMENTATION_TOOL = { @@ -85,6 +85,13 @@ "cache_control": {"type": "ephemeral"} } +if JOB_AGENT_SKILLS: + CALL_JOB_CODE_AGENT_TOOL["input_schema"]["properties"]["skill"] = { + "type": "string", + "enum": sorted(JOB_AGENT_SKILLS), + "description": "Instructions for this kind of work, attached to your message word for word. Name one when a skill you are following tells you to, and put only what this conversation adds in `message`.", + } + # Tool 4: Inspect job code — shared with job_chat's subagent mode so both # agents explore the workflow with the exact same tool from yaml_utils import INSPECT_JOB_CODE_TOOL # noqa: E402 From 010664dd5dce4a8dad8bb45e84580333f367aba6 Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Thu, 8 Oct 2026 18:52:51 +0100 Subject: [PATCH 11/24] Let the job agent load its own skills --- services/global_chat/router.py | 3 +- services/global_chat/skill_registry.py | 10 +++ services/global_chat/skills/qa-code/SKILL.md | 30 +++++---- services/global_chat/skills/qa/SKILL.md | 10 ++- services/global_chat/subagent_caller.py | 15 +++-- .../global_chat/tests/unit/test_skills.py | 20 ++++++ services/job_chat/job_chat.py | 58 ++++++++++++++--- services/job_chat/prompt.py | 4 +- services/job_chat/tests/unit/test_skills.py | 62 +++++++++++++++++++ 9 files changed, 176 insertions(+), 36 deletions(-) create mode 100644 services/job_chat/tests/unit/test_skills.py diff --git a/services/global_chat/router.py b/services/global_chat/router.py index c1b4108a..30e121bd 100644 --- a/services/global_chat/router.py +++ b/services/global_chat/router.py @@ -21,7 +21,7 @@ from util import create_logger, ApolloError, sum_usage, attachments_to_context from streaming_util import StreamManager from global_chat.config_loader import ConfigLoader -from global_chat.skill_registry import Skill, has_skill, strip_invocation +from global_chat.skill_registry import Skill, has_skill, job_agent_skills, strip_invocation from models import resolve_model from yaml_utils import get_step_name_from_page, get_page_view, find_job_in_yaml, stitch_job_code, workflow_has_job_code @@ -395,6 +395,7 @@ def _route_to_job_chat( "metrics_opt_in": self._metrics_opt_in, "subagent": True, "workflow_yaml": workflow_yaml, + "skills": job_agent_skills(), "_stream_manager": self._stream_manager, } diff --git a/services/global_chat/skill_registry.py b/services/global_chat/skill_registry.py index bf22f9c3..10ec12de 100644 --- a/services/global_chat/skill_registry.py +++ b/services/global_chat/skill_registry.py @@ -89,6 +89,16 @@ def _load_skills() -> dict[str, Skill]: JOB_AGENT_SKILLS = {name: skill for name, skill in _ALL_SKILLS.items() if skill.agent == "job_code"} +def job_agent_skills(attached: str | None = None) -> list[dict]: + """The job agent's skills in job_chat's payload shape, leaving out one + already attached to the message so it isn't offered twice.""" + return [ + {"name": skill.name, "description": skill.description, "body": skill.body} + for skill in JOB_AGENT_SKILLS.values() + if skill.name != attached + ] + + def get_skill(name: str) -> Skill: """Resolve a skill name, or reject the request.""" skill = SKILLS.get(name) diff --git a/services/global_chat/skills/qa-code/SKILL.md b/services/global_chat/skills/qa-code/SKILL.md index 8353b518..f15922a6 100644 --- a/services/global_chat/skills/qa-code/SKILL.md +++ b/services/global_chat/skills/qa-code/SKILL.md @@ -1,23 +1,31 @@ --- name: qa-code -description: Review and fix one step's code on its own terms, as part of a workflow review +description: A thorough review of one step's code, fixing what's wrong within it. Use when the user considers the step done and hands it over for a full review, not for a quick check or one specific change. metadata: agent: job_code --- -Review this step and fix what's wrong within it, so that it's right on its own -terms. Other steps are being reviewed at the same time, and fitting them -together comes later, so hold the step to the spec and the user rather than to -what other steps currently do. +Review this step's code and fix what's wrong within it. Hold it to the spec and +the user, and check it against the steps it reads from and writes for. Cover: -- valid code that follows OpenFn job-writing conventions and uses its adaptor - correctly; - doing what the spec or user asks of this step; -- state overwritten inside loops; -- errors swallowed or never surfaced; +- how it uses state: operations at the top level, never nested in callbacks; + callbacks that return state; state read lazily (`$` or a function) in + operation arguments; async work done through operations, not `async`/`await` + or promises; +- what it passes on: only plain JSON leaves the step, so helpers are defined in + the step rather than stored on state; no leftover or intermediate keys; no + personal data left on the final state; +- structure: several small operations that each do one thing rather than one + large `fn()`, and no `fn()` wrappers that add nothing; +- its adaptor: the adaptor's own functions rather than raw HTTP, used + correctly, and cursors set and advanced as intended; +- loops: state overwritten inside them, items or shared objects mutated while + iterating, `each()` used as intended; +- errors swallowed or never surfaced, and per-record failures in batches; - values hardcoded that belong in configuration; - comments that contradict the code. -Report cleanup (duplication, dead code, unclear names) rather than making it. -Reply briefly with what you fixed and what should still change. +Report cleanup (duplication, dead code, unclear names, magic numbers) rather +than making it. Reply briefly with what you fixed and what should still change. diff --git a/services/global_chat/skills/qa/SKILL.md b/services/global_chat/skills/qa/SKILL.md index e35e05f0..a2dffc4c 100644 --- a/services/global_chat/skills/qa/SKILL.md +++ b/services/global_chat/skills/qa/SKILL.md @@ -11,12 +11,10 @@ it implies but doesn't state. Work outwards in three stages. 1. Each step on its own. Have the job code agent review and fix every step in - parallel with the `qa-code` skill, which covers each step's own - correctness: OpenFn conventions, adaptor use, state in loops, swallowed - errors, hardcoded values and misleading comments. Its message only needs - what this conversation adds, such as what the spec asks of that step or - limits the user has set, like leaving comments alone. Making the steps fit - together is the next stage's job. + parallel with the `qa-code` skill. Tell it the other steps are being + reviewed at the same time and you'll fit them together, so it leaves + mismatches between steps to you. Add only what this conversation requires, + such as what the spec asks of that step or limits the user has set. 2. One run, end to end. Read every step's code yourself with `inspect_job_code` and follow the data from the trigger to the last step. diff --git a/services/global_chat/subagent_caller.py b/services/global_chat/subagent_caller.py index 0d6058aa..8805460f 100644 --- a/services/global_chat/subagent_caller.py +++ b/services/global_chat/subagent_caller.py @@ -12,7 +12,7 @@ sys.path.append(str(Path(__file__).parent.parent.parent)) from langfuse import observe -from global_chat.skill_registry import JOB_AGENT_SKILLS +from global_chat.skill_registry import JOB_AGENT_SKILLS, job_agent_skills from util import create_logger, ApolloError, attachments_to_context, select_attachments from yaml_utils import find_job_in_yaml @@ -107,13 +107,11 @@ def call_job_agent( if not user_message: raise ApolloError(400, "message is required") - skill_name = tool_input.get("skill") - if skill_name: - skill = JOB_AGENT_SKILLS.get(skill_name) - if skill: - user_message = f"{skill.as_subagent_preamble()}\n\n{user_message}" - else: - logger.warning(f"job_agent: ignoring unknown skill '{skill_name}'") + skill = JOB_AGENT_SKILLS.get(tool_input.get("skill")) + if skill: + user_message = f"{skill.as_subagent_preamble()}\n\n{user_message}" + elif tool_input.get("skill"): + logger.warning(f"job_agent: ignoring unknown skill '{tool_input['skill']}'") job_context = {} @@ -150,6 +148,7 @@ def call_job_agent( # the block and the inspect_job_code tool. "subagent": True, "workflow_yaml": workflow_yaml, + "skills": job_agent_skills(attached=skill.name if skill else None), } try: diff --git a/services/global_chat/tests/unit/test_skills.py b/services/global_chat/tests/unit/test_skills.py index 71536000..2ae52cbf 100644 --- a/services/global_chat/tests/unit/test_skills.py +++ b/services/global_chat/tests/unit/test_skills.py @@ -400,3 +400,23 @@ def test_an_unknown_subagent_skill_is_dropped_not_fatal() -> None: ) assert mock_main.call_args[0][0]["content"] == "fix it" + + +def test_the_job_agent_is_offered_its_skills_on_the_direct_route() -> None: + """A step question routed straight to the job agent can still use qa-code.""" + with patch("job_chat.job_chat.main", return_value=job_chat_result()) as mock_main: + make_router()._route_to_job_chat( + "is this step ready to go live?", WORKFLOW_YAML, "workflows/wf/fetch-patients", [], False, 5, + ) + + assert [s["name"] for s in mock_main.call_args[0][0]["skills"]] == sorted(JOB_AGENT_SKILLS) + + +def test_an_attached_skill_is_not_offered_again() -> None: + with patch("job_chat.job_chat.main", return_value=job_chat_result()) as mock_main: + call_job_agent( + {"message": "review", "job_key": "fetch-patients", "attachments": [], "skill": "qa-code"}, + workflow_yaml=WORKFLOW_YAML, + ) + + assert "qa-code" not in [s["name"] for s in mock_main.call_args[0][0]["skills"]] diff --git a/services/job_chat/job_chat.py b/services/job_chat/job_chat.py index bda188c3..52b54737 100644 --- a/services/job_chat/job_chat.py +++ b/services/job_chat/job_chat.py @@ -122,11 +122,35 @@ ), } -# Max API rounds in one generate() call: enough for a couple of -# inspect_job_code round-trips plus the final answer. +# Max API rounds in one generate() call: enough for a skill load and an +# inspect_job_code round-trip plus the final answer. _MAX_TOOL_ROUNDS = 4 +def _load_skill_tool(skills: List[Dict[str, str]]) -> Dict[str, Any]: + """The skills the caller gave this agent, offered the way the planner's are.""" + listing = "\n".join(f"- {s['name']}: {s['description']}" for s in skills) + return { + "name": "load_skill", + "description": ( + "Load a skill: instructions for a kind of task, holding OpenFn practice " + "you won't otherwise have. When the task matches a skill below, load it " + f"first and follow its instructions.\n\nAvailable skills:\n{listing}" + ), + "input_schema": { + "type": "object", + "properties": { + "name": { + "type": "string", + "enum": sorted(s["name"] for s in skills), + "description": "The skill to load", + }, + }, + "required": ["name"], + }, + } + + # Helper function for page navigation def extract_page_prefix_from_last_turn(history: List[Dict[str, str]]) -> Optional[str]: """Extract page prefix from last user message if present.""" @@ -165,6 +189,8 @@ class Payload: # inspect_job_code tool. workflow_yaml: Optional[str] = None subagent: Optional[bool] = False + # Skills the caller makes available to this agent: name, description, body + skills: Optional[List[Dict[str, str]]] = None @classmethod def from_dict(cls, data: Dict[str, Any]) -> "Payload": @@ -186,6 +212,7 @@ def from_dict(cls, data: Dict[str, Any]) -> "Payload": metrics_opt_in=data.get("metrics_opt_in"), workflow_yaml=data.get("workflow_yaml"), subagent=data.get("subagent", False), + skills=data.get("skills"), ) @@ -243,6 +270,7 @@ def generate( workflow_yaml: Optional[str] = None, subagent: Optional[bool] = False, stream_manager: Optional[StreamManager] = None, + skills: Optional[List[Dict[str, str]]] = None, ) -> ChatResponse: """ Generate a response using the Claude API with optional streaming. @@ -275,7 +303,8 @@ def generate( download_adaptor_docs=download_adaptor_docs, refresh_rag=refresh_rag, workflow_yaml=workflow_yaml, - subagent=subagent + subagent=subagent, + has_skills=bool(skills), ) else: @@ -300,11 +329,15 @@ def generate( tools.append(_EDIT_WORKFLOW_TOOL) if workflow_yaml: tools.append(_INSPECT_JOB_CODE_TOOL) + if skills: + tools.append(_load_skill_tool(skills)) + skill_bodies = {s["name"]: s["body"] for s in skills or []} tool_kwargs = {"tools": tools, "tool_choice": {"type": "auto"}} if tools else {} - # Without the subagent tools this loop runs exactly once: edit_job - # is terminal (its input IS the output), so only inspect_job_code - # triggers another round and only handover exits early. + # Without the subagent tools or skills this loop runs exactly once: + # edit_job is terminal (its input IS the output), so only + # inspect_job_code and load_skill trigger another round and only + # handover exits early. messages = prompt handover_reason = None text_parts = [] @@ -394,11 +427,11 @@ def generate( handover_reason = (handover_block.input or {}).get("goal") or "handover requested" break - inspect_blocks = [b for b in tool_uses if b.name == "inspect_job_code"] - if not inspect_blocks or round_index == _MAX_TOOL_ROUNDS - 1: + reading_blocks = [b for b in tool_uses if b.name in ("inspect_job_code", "load_skill")] + if not reading_blocks or round_index == _MAX_TOOL_ROUNDS - 1: break - # Answer the inspect calls and let the model continue. An + # Answer the reading calls and let the model continue. An # edit_job call made in the same round is deferred: the # model must re-issue it with its final answer. stream_manager.send_thinking(STATUS_REVIEWING_CODE) @@ -408,6 +441,12 @@ def generate( job_keys = (block.input or {}).get("job_keys") or [] logger.info("job_chat inspect_job_code: reading %s", job_keys) result_text = inspect_job_code(workflow_yaml, job_keys) + elif block.name == "load_skill": + skill_name = (block.input or {}).get("name") + logger.info("job_chat load_skill: %s", skill_name) + result_text = skill_bodies.get( + skill_name, f"Error: Unknown skill. Available skills: {sorted(skill_bodies)}", + ) else: result_text = ( "Not applied. Finish inspecting, then write your final reply " @@ -800,6 +839,7 @@ def main(data_dict: dict) -> dict: # In-process callers (global_chat) may inject a shared stream # manager so a handed-over request continues the same stream stream_manager=data_dict.get("_stream_manager"), + skills=data.skills, ) # Tag the trace when code was generated, so we can filter for it. diff --git a/services/job_chat/prompt.py b/services/job_chat/prompt.py index 115b855d..83ece793 100644 --- a/services/job_chat/prompt.py +++ b/services/job_chat/prompt.py @@ -460,7 +460,7 @@ def format_search_results(search_results): @observe(name="job_chat_build_prompt") def build_prompt(content, history, context, rag=None, api_key=None, stream_manager=None, download_adaptor_docs=True, refresh_rag=False, - workflow_yaml=None, subagent=False): + workflow_yaml=None, subagent=False, has_skills=False): retrieved_knowledge = { "search_results": [], "search_results_sections": [], @@ -510,6 +510,8 @@ def build_prompt(content, history, context, rag=None, api_key=None, stream_manag " If it needs changes beyond this step's code, call `edit_workflow` first;" " to merely read another step (to answer, or to edit this one), use `inspect_job_code`." ) + if has_skills: + reminder += " If one of your skills fits the task, load it first." prompt.append({ "role": "user", "content": f"{content}\n\n{reminder}", diff --git a/services/job_chat/tests/unit/test_skills.py b/services/job_chat/tests/unit/test_skills.py new file mode 100644 index 00000000..cbc54615 --- /dev/null +++ b/services/job_chat/tests/unit/test_skills.py @@ -0,0 +1,62 @@ +"""Unit tests for skills handed to job_chat by its caller. + +The caller passes the skills written for this agent; job_chat offers them +through a load_skill tool, and loading one returns its instructions and lets +the model carry on, the same way reading another step does. +""" + +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from job_chat.job_chat import AnthropicClient, ChatConfig + +SKILLS = [{"name": "qa-code", "description": "Review one step", "body": "Check the loops."}] + + +def tool_use(name: str, tool_input: dict) -> SimpleNamespace: + return SimpleNamespace(type="tool_use", id=f"tu_{name}", name=name, input=tool_input) + + +def text(value: str) -> SimpleNamespace: + return SimpleNamespace(type="text", text=value) + + +def message(*blocks: SimpleNamespace) -> SimpleNamespace: + usage = MagicMock() + usage.model_dump.return_value = {"input_tokens": 1, "output_tokens": 1} + usage.cache_creation_input_tokens = 0 + usage.cache_read_input_tokens = 0 + return SimpleNamespace(content=list(blocks), usage=usage, stop_reason="end_turn") + + +def run(responses: list, skills: list | None) -> MagicMock: + with patch("job_chat.job_chat.Anthropic"): + client = AnthropicClient(ChatConfig(api_key="test")) + client.client.messages.create.side_effect = responses + with patch("job_chat.job_chat.build_prompt", return_value=("system", [{"role": "user", "content": "hi"}], {})): + client.generate(content="is this ready?", suggest_code=True, subagent=True, skills=skills) + return client.client.messages.create + + +def test_given_skills_are_offered_as_a_load_skill_tool() -> None: + create = run([message(text("Looks fine."))], SKILLS) + + tools = {t["name"]: t for t in create.call_args.kwargs["tools"]} + assert tools["load_skill"]["input_schema"]["properties"]["name"]["enum"] == ["qa-code"] + assert "- qa-code: Review one step" in tools["load_skill"]["description"] + + +def test_without_skills_there_is_no_load_skill_tool() -> None: + create = run([message(text("Looks fine."))], None) + + assert "load_skill" not in [t["name"] for t in create.call_args.kwargs["tools"]] + + +def test_loading_a_skill_returns_its_instructions_and_continues() -> None: + create = run( + [message(tool_use("load_skill", {"name": "qa-code"})), message(text("Reviewed."))], + SKILLS, + ) + + tool_result = create.call_args.kwargs["messages"][-1]["content"][0] + assert tool_result["content"] == "Check the loops." From 1a34d1c232d083e267d70c7ae7deac19220534d1 Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Thu, 8 Oct 2026 19:13:50 +0100 Subject: [PATCH 12/24] Settle step contracts before the qa fix pass --- services/global_chat/skills/qa-code/SKILL.md | 14 +++- services/global_chat/skills/qa/SKILL.md | 77 ++++++++++---------- 2 files changed, 48 insertions(+), 43 deletions(-) diff --git a/services/global_chat/skills/qa-code/SKILL.md b/services/global_chat/skills/qa-code/SKILL.md index f15922a6..502d6585 100644 --- a/services/global_chat/skills/qa-code/SKILL.md +++ b/services/global_chat/skills/qa-code/SKILL.md @@ -15,16 +15,22 @@ Cover: operation arguments; async work done through operations, not `async`/`await` or promises; - what it passes on: only plain JSON leaves the step, so helpers are defined in - the step rather than stored on state; no leftover or intermediate keys; no - personal data left on the final state; + the step rather than stored on state; only what the next step needs, with no + leftover or intermediate keys; no personal data logged or left on the final + state, which OpenFn doesn't scrub; +- what it takes in: incoming data validated before it's written, and a failing + system or malformed response handled rather than passed on; - structure: several small operations that each do one thing rather than one large `fn()`, and no `fn()` wrappers that add nothing; - its adaptor: the adaptor's own functions rather than raw HTTP, used correctly, and cursors set and advanced as intended; - loops: state overwritten inside them, items or shared objects mutated while iterating, `each()` used as intended; -- errors swallowed or never surfaced, and per-record failures in batches; -- values hardcoded that belong in configuration; +- errors swallowed or never surfaced, and one bad record failing a whole batch + or disappearing silently, unless that's intended; +- work that runs away: unbounded retries, loops or calls; +- credentials written into code, values hardcoded that belong in + configuration, and anything left over from testing; - comments that contradict the code. Report cleanup (duplication, dead code, unclear names, magic numbers) rather diff --git a/services/global_chat/skills/qa/SKILL.md b/services/global_chat/skills/qa/SKILL.md index a2dffc4c..2a1fdba5 100644 --- a/services/global_chat/skills/qa/SKILL.md +++ b/services/global_chat/skills/qa/SKILL.md @@ -8,48 +8,47 @@ or ticket they've shared, or the workflow itself. If you can't tell, ask. With a spec, say which parts are met, partly met or missing, including edge cases it implies but doesn't state. -Work outwards in three stages. +Read first, then fix in one pass, then check. -1. Each step on its own. Have the job code agent review and fix every step in - parallel with the `qa-code` skill. Tell it the other steps are being - reviewed at the same time and you'll fit them together, so it leaves - mismatches between steps to you. Add only what this conversation requires, - such as what the spec asks of that step or limits the user has set. +1. Read every step's code yourself with `inspect_job_code` and follow the data + from the trigger to the last step, looking at what's below. Where steps + don't fit, decide which side is right, so each step's fix can be settled + before anything changes. -2. One run, end to end. Read every step's code yourself with - `inspect_job_code` and follow the data from the trigger to the last step. - To fix a mismatch, decide which side is right and give every affected step - the same instruction. Check that: - - the trigger is the right kind and enabled as intended; - - each step runs after everything it depends on; - - each step reads what the steps before it actually write, under the same - names, in the same shape, and as plain JSON, since anything else is lost - between steps; - - edges route data where it should go, so a failed or unmatched record - can't fall through; - - no two steps do the same work; - - each step uses its system's own adaptor where one exists rather than - plain HTTP; - - credentials are referenced, never written into code; - - names other workflows rely on, such as collections, match exactly. +2. Have the job code agent review and fix every step in parallel with the + `qa-code` skill, which covers everything within a step. Your message + carries what only you know: the contract each step keeps with its + neighbours (what it reads and writes, under which names), which side of a + mismatch it owns, that the other steps are being fixed at the same time so + it changes only its own, and what this conversation adds, such as what the spec + asks of the step or limits the user has set. Leave everything else in the + step to the job code agent. Make workflow changes, such as triggers, edges + or adaptor versions, through the workflow agent. -3. Many runs, once live. What fails quietly matters more than what fails - loudly. Check for: - - data lost, duplicated, corrupted or exposed without anyone noticing, - often through a re-run that creates duplicates, a cursor or paging that - skips records, or a lookup that assumes exactly one match; - - personal data logged or left on the final state, which OpenFn doesn't - scrub; - - work that runs away: unbounded retries, loops or calls; - - incoming data written without being validated; - - what happens when a system fails or returns malformed data; - - one bad record failing a whole batch, or disappearing silently, unless - that's intended; - - steps passing on more than the next one needs; - - adaptors on `@latest`, which can change under a live workflow and break - it. Pin the version the workflow was tested on, which run logs often - show; if you can't tell, say the pinned version is untested; - - anything left over from testing that shouldn't go live. +3. Read the changed steps again to confirm they now fit together. + +How the steps fit together: +- the trigger is the right kind and enabled as intended; +- each step runs after everything it depends on; +- each step reads what the steps before it actually write, under the same + names, in the same shape, and as plain JSON, since anything else is lost + between steps; +- edges route data where it should go, so a failed or unmatched record can't + fall through; +- no two steps do the same work; +- each step uses its system's own adaptor where one exists rather than plain + HTTP; +- names other workflows rely on, such as collections, match exactly. + +How it behaves live, over many runs. What fails quietly matters more than what +fails loudly, and these usually need the user's decision, so propose fixes +rather than prescribe them: +- a re-run that creates duplicates; +- a cursor or paging that skips records; +- a lookup that assumes exactly one match; +- adaptors on `@latest`, which can change under a live workflow and break it. + Pin the version the workflow was tested on, which run logs often show; if + you can't tell, say the pinned version is untested. If dataclips are attached, check that the output means what the input meant. From dbfa9798861eda2fca46e3c026151ca1e8b62815 Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Thu, 8 Oct 2026 21:46:39 +0100 Subject: [PATCH 13/24] Report skills used and add skill acceptance tests --- services/global_chat/global_chat.py | 6 + services/global_chat/planner.py | 8 ++ services/global_chat/router.py | 2 + services/global_chat/subagent_caller.py | 3 + .../test_design_continues_over_turns.md | 124 ++++++++++++++++++ .../skills/test_qa_whole_workflow.md | 115 ++++++++++++++++ .../test_specific_change_stays_specific.md | 107 +++++++++++++++ .../skills/test_step_review_when_done.md | 111 ++++++++++++++++ .../global_chat/tests/unit/test_skills.py | 31 +++++ services/job_chat/job_chat.py | 10 +- services/job_chat/tests/unit/test_skills.py | 12 ++ services/testing/spec_collector.py | 8 +- 12 files changed, 534 insertions(+), 3 deletions(-) create mode 100644 services/global_chat/tests/acceptance/skills/test_design_continues_over_turns.md create mode 100644 services/global_chat/tests/acceptance/skills/test_qa_whole_workflow.md create mode 100644 services/global_chat/tests/acceptance/skills/test_specific_change_stays_specific.md create mode 100644 services/global_chat/tests/acceptance/skills/test_step_review_when_done.md diff --git a/services/global_chat/global_chat.py b/services/global_chat/global_chat.py index 43674b18..fb5590a6 100644 --- a/services/global_chat/global_chat.py +++ b/services/global_chat/global_chat.py @@ -142,6 +142,12 @@ def main(data_dict: dict) -> dict: if diff_meta: langfuse.update_current_span(metadata=diff_meta) + # Tags, so traces can be filtered by the skills a turn used + skills_used = result.meta.get("skills") + if skills_used: + with propagate_attributes(tags=[f"skill:{name}" for name in skills_used]): + pass + # 5. Return structured response return { "response": result.response, diff --git a/services/global_chat/planner.py b/services/global_chat/planner.py index 7f2cbc11..4cae2aa4 100644 --- a/services/global_chat/planner.py +++ b/services/global_chat/planner.py @@ -550,6 +550,14 @@ def run( "total_tool_calls": tool_call_count, } + skills_used = [ + *([self._skill.name] if self._skill else []), + *self._loaded_skills, + *(name for result in self.subagent_results for name in (result.get("meta") or {}).get("skills", [])), + ] + if skills_used: + meta["skills"] = list(dict.fromkeys(skills_used)) + if response.stop_reason == "pause_turn": meta["truncated"] = True meta["stop_reason"] = "pause_turn" diff --git a/services/global_chat/router.py b/services/global_chat/router.py index 30e121bd..e8753b8f 100644 --- a/services/global_chat/router.py +++ b/services/global_chat/router.py @@ -421,6 +421,8 @@ def _route_to_job_chat( logger.warning(dropped) meta = {"agents": ["router", "job_code_agent"], "router_confidence": confidence} + if (result.get("meta") or {}).get("skills"): + meta["skills"] = result["meta"]["skills"] # In the shape the planner reports, so a client has one place to look. # This route is the shortcut for a single-step edit, so it is the case diff --git a/services/global_chat/subagent_caller.py b/services/global_chat/subagent_caller.py index 8805460f..547cc82d 100644 --- a/services/global_chat/subagent_caller.py +++ b/services/global_chat/subagent_caller.py @@ -160,6 +160,9 @@ def call_job_agent( logger.info(f"job_agent response: {response_preview}") result["_call_metadata"] = {"subagent": "job_agent", "job_key": job_key} + if skill: + meta = result.setdefault("meta", {}) + meta["skills"] = [skill.name, *meta.get("skills", [])] return result diff --git a/services/global_chat/tests/acceptance/skills/test_design_continues_over_turns.md b/services/global_chat/tests/acceptance/skills/test_design_continues_over_turns.md new file mode 100644 index 00000000..3f9b5576 --- /dev/null +++ b/services/global_chat/tests/acceptance/skills/test_design_continues_over_turns.md @@ -0,0 +1,124 @@ +--- +id: global-chat.skills.design-continues-over-turns +service: global_chat +judges: [general] +--- + +# notes + +The user started /design on the previous turn. Its instructions are in that +turn of the history, exactly as Apollo returned it, and must keep applying now +that the user has answered. The planner's own default is to build a workflow as +soon as the shape is guessable; /design overrides that until the user agrees on +a design. Judge the conversation, not the wording: there are many good ways to +continue a consultation. + +# quality_criteria + +- The reply continues the design conversation, asking follow-up questions or offering options, and does not build or change a workflow yet. +- It engages with the process the user described, a weekly manual copy of Kobo form data into DHIS2, rather than asking generic questions it could have asked before. +- It asks only a few questions at a time, rather than a long questionnaire. + +# settings + +## page + +workflows/new + +## meta.session_id + +sess-skills-design-continues-0001 + +# history + +## turn + +### role + +user + +### content + +[pg:workflow] +The user wants to think the design through before anything is built. Treat it +as a consultation: suggest ideas and sketch options freely, but build only +when they're ready. + +Find out where they are and meet them there. Some don't yet know what could be +automated or how many workflows it takes; others have the shape and need help +with the details. Follow their lead on how deep to go. + +If the user struggles to specify what they want, start from people and process +rather than systems. Wherever the conversation starts, make sure the design +rests on the process it serves, and question requirements against how the +result will be used; the simpler design is often enough. The result is often a mix +of automated steps and steps people keep doing. + +Areas to draw on as the conversation needs them, not a list to work through: +- Value: what problem automation solves, what's done by hand today and what it + costs in time, and what happens if nothing changes. This tells you whether + and how much to automate. +- Process: who does each step, what depends on what, and what's redundant. +- Access: how each system can be reached (API, webhooks, file exports, direct + database), what's in the way (hosting, VPN, firewall, IP allowlisting), and + whether there's a test environment. Suggest getting credentials early so + access can be tested. If a system or its API is still being built or + configured, say it may be too early to integrate with it. +- Timing and volume: whether it really needs to be immediate, whether data + flows one way or both, and how much moves per run, which decides whether API + limits and paging matter. +- Ownership: who owns each system and its data, and who will run and maintain + the workflow, how technical they are, and who decides on changes later. +- Data: real sample records from each side, whether records move one to one + or are summarised, fields the target needs that users never see, and shared + lists (regions, services) and where they should live. +- Failure: who hears about it, and how a run is redone without duplicates. + +Ask a few questions at a time, most consequential first. When there's a real +choice, recommend one option and name the alternative. Prefer the simplest +design that meets the need, and say when an idea goes beyond what was asked. + +Flag details that rest on assumptions or documentation and still need +checking, and help the user see how to check them, without holding up the +design. If you're unsure how the platform behaves, say so rather than design +around a guess. Note open decisions and who can answer each rather than +filling them in. + +When the design is agreed, summarise it at the level of detail you reached, in +a form the user could share with the people who need to agree to it, with any +open decisions. Then offer to build it. + + +The user invoked /design. + +I want to automate something with our clinic data but I'm not sure what yet + +## turn + +### role + +assistant + +### content + +Happy to help you figure this out. Let's start from how your clinic actually works, not the tech. + +A few questions to get oriented: + +1. **What's painful right now?** Where does someone on your team spend time copying data between systems, re-typing things, chasing reports, or reconciling records by hand? That's usually where automation pays off first. + +2. **What systems hold your clinic data?** For example an EMR/EHR (OpenMRS, DHIS2, CommCare, Bahmni, a custom system?), a spreadsheet, a lab system, a billing tool, an SMS service? Even a rough list helps. + +3. **Who are you reporting to or sharing data with?** Many clinics have to send aggregate numbers to a ministry/DHIS2, sync patient records somewhere, or send reminders to patients. Is anything like that part of your world? + +You don't need precise answers — just tell me what a typical frustrating task looks like, and we'll trace it back to what could be automated. + +# turn + +## role + +user + +## content + +Our nurses fill in a KoboToolbox form for every patient visit, and every Friday someone copies the totals into DHIS2 by hand. It takes most of the afternoon. diff --git a/services/global_chat/tests/acceptance/skills/test_qa_whole_workflow.md b/services/global_chat/tests/acceptance/skills/test_qa_whole_workflow.md new file mode 100644 index 00000000..50ede16a --- /dev/null +++ b/services/global_chat/tests/acceptance/skills/test_qa_whole_workflow.md @@ -0,0 +1,115 @@ +--- +id: global-chat.skills.qa-whole-workflow +service: global_chat +judges: [general] +--- + +# notes + +The user runs /qa on a two-step workflow and asks for its comments to be left +alone. The upload step reads `$.patient_list`, but the fetch step writes +`state.patients`, which is why the last run failed. Both steps use `@latest`; +the run log shows the version they were tested on. The planner hands each step +to the job code agent with its qa-code skill and fits the steps together +itself. Judge the outcome: which side of the mismatch changes doesn't matter as +long as the two steps agree, and the report can take any reasonable shape. + +# quality_criteria + +- After the changes, the upload step reads the patient list from the same key the fetch step writes it to, so the crash is fixed. +- The existing comment in the fetch step is still there and unchanged. +- Each `@latest` adaptor is pinned to 7.0.0, the version the run log shows, or the reply recommends pinning it to the tested version. +- The reply says what was fixed and what should still change, with the most serious problems first, and reminds the user to test the workflow again. + +# settings + +## page + +workflows/patient-sync + +## skill + +```json +{"name": "qa"} +``` + +## workflow_yaml + +```yaml +name: patient-sync +jobs: + fetch-patients: + id: job-fetch-patients-id + name: Fetch patients + adaptor: "@openfn/language-http@latest" + body: | + get('https://example.org/api/patients'); + // The API nests patients under results + fn(state => ({ ...state, patients: state.data.results })); + upload-patients: + id: job-upload-patients-id + name: Upload patients + adaptor: "@openfn/language-http@latest" + body: | + each( + $.patient_list, + post('https://target.example.org/api/people', state => ({ + name: state.data.name, + sex: state.data.gender, + })) + ); +triggers: + cron: + id: trigger-cron-id + type: cron + cron_expression: "0 2 * * *" + enabled: true +edges: + cron->fetch-patients: + id: edge-cron-fetch + source_trigger: cron + target_job: fetch-patients + condition_type: always + enabled: true + fetch-patients->upload-patients: + id: edge-fetch-upload + source_job: fetch-patients + target_job: upload-patients + condition_type: on_job_success + enabled: true +``` + +## attachments + +```json +[ + { + "type": "log", + "content": [ + "[CLI] Versions: node 22, worker 1.x, @openfn/language-http 7.0.0", + "[R/T] Starting step Fetch patients", + "[HTTP] GET https://example.org/api/patients - 200 in 312ms", + "[R/T] Completed step Fetch patients in 401ms", + "[R/T] Starting step Upload patients", + "[R/T] Error in runtime execution!", + "[R/T] TypeError: Cannot read properties of undefined (reading 'length')", + " at each (@openfn/language-common)", + "[R/T] Failed step Upload patients after 12ms" + ] + } +] +``` + +## meta.session_id + +sess-skills-qa-whole-workflow-0001 + +# turn + +## role + +user + +## content + +/qa please review this before it goes live, and leave any comments alone diff --git a/services/global_chat/tests/acceptance/skills/test_specific_change_stays_specific.md b/services/global_chat/tests/acceptance/skills/test_specific_change_stays_specific.md new file mode 100644 index 00000000..bb6a3ecd --- /dev/null +++ b/services/global_chat/tests/acceptance/skills/test_specific_change_stays_specific.md @@ -0,0 +1,107 @@ +--- +id: global-chat.skills.specific-change-stays-specific +service: global_chat +judges: [general] +--- + +# notes + +Same step as the review test, but the user asks for one specific change. This +is the negative case: the job agent should make the change, not run a full +review. Mentioning one serious problem it noticed is fine, briefly; a full +review of the step, or fixing things the user didn't ask about, is not. + +# quality_criteria + +- The upload step now maps the field to `gender` instead of `sex`. +- No other part of the code is changed. Judge the code itself: the workflow YAML may come back re-serialised (quoting, block style), which doesn't count as a change. +- The reply stays focused on the requested change rather than giving a full review of the step's other problems; at most it briefly mentions one. + +# settings + +## page + +workflows/patient-sync/upload-patients + +## workflow_yaml + +```yaml +name: patient-sync +jobs: + fetch-patients: + id: job-fetch-patients-id + name: Fetch patients + adaptor: "@openfn/language-http@7.0.0" + body: | + get('https://example.org/api/patients'); + fn(state => ({ ...state, patients: state.data.results })); + upload-patients: + id: job-upload-patients-id + name: Upload patients + adaptor: "@openfn/language-http@7.0.0" + body: | + each( + $.patients, + post('https://target.example.org/api/people', state => { + console.log('Uploading patient', state.data.name, state.data.phone); + return { + name: state.data.name, + sex: state.data.gender, + registered: '2024-01-01', + }; + }).catch((error, state) => state) + ); +triggers: + cron: + id: trigger-cron-id + type: cron + cron_expression: "0 2 * * *" + enabled: true +edges: + cron->fetch-patients: + id: edge-cron-fetch + source_trigger: cron + target_job: fetch-patients + condition_type: always + enabled: true + fetch-patients->upload-patients: + id: edge-fetch-upload + source_job: fetch-patients + target_job: upload-patients + condition_type: on_job_success + enabled: true +``` + +## attachments + +```json +[ + { + "type": "log", + "content": [ + "[CLI] Versions: node 22, worker 1.x, @openfn/language-http 7.0.0", + "[R/T] Starting step Fetch patients", + "[HTTP] GET https://example.org/api/patients - 200 in 280ms", + "[R/T] Completed step Fetch patients in 350ms", + "[R/T] Starting step Upload patients", + "[JOB] Uploading patient ", + "[HTTP] POST https://target.example.org/api/people - 201 in 190ms", + "[R/T] Completed step Upload patients in 410ms" + ] + } +] +``` + +## meta.session_id + +sess-skills-specific-change-0001 + +# turn + +## role + +user + +## content + +Rename the sex field to gender in the upload diff --git a/services/global_chat/tests/acceptance/skills/test_step_review_when_done.md b/services/global_chat/tests/acceptance/skills/test_step_review_when_done.md new file mode 100644 index 00000000..a5c8c1bb --- /dev/null +++ b/services/global_chat/tests/acceptance/skills/test_step_review_when_done.md @@ -0,0 +1,111 @@ +--- +id: global-chat.skills.step-review-when-done +service: global_chat +judges: [general] +--- + +# notes + +The user considers the upload step finished and hands it over for review, from +the step's own page, so the request goes straight to the job code agent. Its +last run succeeded, so nothing visibly fails; the step has problems only a +review would catch. The job agent should pick up its qa-code skill and review +the step properly. A good reply may fix some problems and only flag others; +either is fine as long as each problem is dealt with. It may also check the +upstream step. + +# quality_criteria + +- The reply deals with patient personal data being written to the run logs (the console.log of names and phone numbers), by removing it or clearly flagging it. +- The reply deals with failures being silently swallowed (the `.catch` that just returns state), by fixing it or clearly flagging it. +- The reply deals with the hardcoded `registered: '2024-01-01'` date, by fixing it or flagging it as a value that shouldn't be hardcoded. +- Anything changed in the code stays within the upload step and is related to a problem the reply names. + +# settings + +## page + +workflows/patient-sync/upload-patients + +## workflow_yaml + +```yaml +name: patient-sync +jobs: + fetch-patients: + id: job-fetch-patients-id + name: Fetch patients + adaptor: "@openfn/language-http@7.0.0" + body: | + get('https://example.org/api/patients'); + fn(state => ({ ...state, patients: state.data.results })); + upload-patients: + id: job-upload-patients-id + name: Upload patients + adaptor: "@openfn/language-http@7.0.0" + body: | + each( + $.patients, + post('https://target.example.org/api/people', state => { + console.log('Uploading patient', state.data.name, state.data.phone); + return { + name: state.data.name, + sex: state.data.gender, + registered: '2024-01-01', + }; + }).catch((error, state) => state) + ); +triggers: + cron: + id: trigger-cron-id + type: cron + cron_expression: "0 2 * * *" + enabled: true +edges: + cron->fetch-patients: + id: edge-cron-fetch + source_trigger: cron + target_job: fetch-patients + condition_type: always + enabled: true + fetch-patients->upload-patients: + id: edge-fetch-upload + source_job: fetch-patients + target_job: upload-patients + condition_type: on_job_success + enabled: true +``` + +## attachments + +```json +[ + { + "type": "log", + "content": [ + "[CLI] Versions: node 22, worker 1.x, @openfn/language-http 7.0.0", + "[R/T] Starting step Fetch patients", + "[HTTP] GET https://example.org/api/patients - 200 in 280ms", + "[R/T] Completed step Fetch patients in 350ms", + "[R/T] Starting step Upload patients", + "[JOB] Uploading patient ", + "[HTTP] POST https://target.example.org/api/people - 201 in 190ms", + "[R/T] Completed step Upload patients in 410ms" + ] + } +] +``` + +## meta.session_id + +sess-skills-step-review-when-done-0001 + +# turn + +## role + +user + +## content + +I think this step is done now. Can you review it before it goes live? diff --git a/services/global_chat/tests/unit/test_skills.py b/services/global_chat/tests/unit/test_skills.py index 2ae52cbf..985717d7 100644 --- a/services/global_chat/tests/unit/test_skills.py +++ b/services/global_chat/tests/unit/test_skills.py @@ -420,3 +420,34 @@ def test_an_attached_skill_is_not_offered_again() -> None: ) assert "qa-code" not in [s["name"] for s in mock_main.call_args[0][0]["skills"]] + + +# --- reporting the skills a turn used ------------------------------------- + + +def test_a_turn_reports_the_skill_the_planner_attached() -> None: + with patch("job_chat.job_chat.main", return_value=job_chat_result()): + result = call_job_agent( + {"message": "review", "job_key": "fetch-patients", "attachments": [], "skill": "qa-code"}, + workflow_yaml=WORKFLOW_YAML, + ) + + assert result["meta"]["skills"] == ["qa-code"] + + +def test_the_direct_route_reports_skills_the_job_agent_loaded() -> None: + loaded = {**job_chat_result(), "meta": {"skills": ["qa-code"]}} + with patch("job_chat.job_chat.main", return_value=loaded): + result = make_router()._route_to_job_chat( + "is this ready to go live?", WORKFLOW_YAML, "workflows/wf/fetch-patients", [], False, 5, + ) + + assert result.meta["skills"] == ["qa-code"] + + +def test_a_planner_turn_reports_the_skill_it_ran() -> None: + _, result = run_turn("qa") + assert result.meta["skills"] == ["qa"] + + _, result = run_turn(None) + assert "skills" not in result.meta diff --git a/services/job_chat/job_chat.py b/services/job_chat/job_chat.py index 52b54737..504d0c6f 100644 --- a/services/job_chat/job_chat.py +++ b/services/job_chat/job_chat.py @@ -233,6 +233,7 @@ class ChatResponse: diff: Optional[Dict[str, Any]] = None # Subagent mode only: reason the request was handed back to the caller handover: Optional[str] = None + skills_loaded: Optional[List[str]] = None class AnthropicClient: def __init__(self, config: Optional[ChatConfig] = None): @@ -332,6 +333,7 @@ def generate( if skills: tools.append(_load_skill_tool(skills)) skill_bodies = {s["name"]: s["body"] for s in skills or []} + skills_loaded = [] tool_kwargs = {"tools": tools, "tool_choice": {"type": "auto"}} if tools else {} # Without the subagent tools or skills this loop runs exactly once: @@ -444,6 +446,8 @@ def generate( elif block.name == "load_skill": skill_name = (block.input or {}).get("name") logger.info("job_chat load_skill: %s", skill_name) + if skill_name in skill_bodies and skill_name not in skills_loaded: + skills_loaded.append(skill_name) result_text = skill_bodies.get( skill_name, f"Error: Unknown skill. Available skills: {sorted(skill_bodies)}", ) @@ -568,7 +572,8 @@ def generate( history=updated_history, usage=usage, rag=retrieved_knowledge, - diff=diff + diff=diff, + skills_loaded=skills_loaded, ) def process_stream_event( @@ -875,6 +880,9 @@ def main(data_dict: dict) -> dict: if result.handover: response_dict["handover"] = result.handover + if result.skills_loaded: + response_dict["meta"]["skills"] = result.skills_loaded + return response_dict except ApolloError: diff --git a/services/job_chat/tests/unit/test_skills.py b/services/job_chat/tests/unit/test_skills.py index cbc54615..fc0119b4 100644 --- a/services/job_chat/tests/unit/test_skills.py +++ b/services/job_chat/tests/unit/test_skills.py @@ -60,3 +60,15 @@ def test_loading_a_skill_returns_its_instructions_and_continues() -> None: tool_result = create.call_args.kwargs["messages"][-1]["content"][0] assert tool_result["content"] == "Check the loops." + + +def test_a_loaded_skill_is_reported() -> None: + with patch("job_chat.job_chat.Anthropic"): + client = AnthropicClient(ChatConfig(api_key="test")) + client.client.messages.create.side_effect = [ + message(tool_use("load_skill", {"name": "qa-code"})), message(text("Reviewed.")), + ] + with patch("job_chat.job_chat.build_prompt", return_value=("system", [{"role": "user", "content": "hi"}], {})): + result = client.generate(content="is this ready?", suggest_code=True, subagent=True, skills=SKILLS) + + assert result.skills_loaded == ["qa-code"] diff --git a/services/testing/spec_collector.py b/services/testing/spec_collector.py index 1e616873..334fdf80 100644 --- a/services/testing/spec_collector.py +++ b/services/testing/spec_collector.py @@ -285,8 +285,12 @@ def _capture_response_text( if not isinstance(text, str) or not text.strip(): return None - agent_path = _format_agent_path(response.get("meta", {})) - body = f"agents: {agent_path}\n\n{text}" if agent_path else text + meta = response.get("meta") or {} + agent_path = _format_agent_path(meta) + header = [f"agents: {agent_path}"] if agent_path else [] + if meta.get("skills"): + header.append(f"skills: {', '.join(meta['skills'])}") + body = "\n".join(header) + f"\n\n{text}" if header else text output_dir.mkdir(parents=True, exist_ok=True) suffix = f"__run-{run_index}" if runs > 1 else "" From 8323a45f0a94ac9a5a8100bc1da2aea128385ada Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Mon, 12 Oct 2026 16:10:29 +0100 Subject: [PATCH 14/24] Share simulate-users and draft-assistant-skill skills --- .claude/skills/draft-assistant-skill/SKILL.md | 175 ++++++++++++++++++ .claude/skills/simulate-users/SKILL.md | 82 ++++++++ .../fixtures/commcare-dhis2-broken-log.json | 10 + .../fixtures/intake-to-postgres.json | 4 + .../fixtures/job-http-basic.json | 7 + .../fixtures/kobo-nutrition-dhis2.json | 4 + .../fixtures/shopify-netsuite-long.json | 4 + .claude/skills/simulate-users/roles.md | 9 + .gitignore | 4 +- 9 files changed, 298 insertions(+), 1 deletion(-) create mode 100644 .claude/skills/draft-assistant-skill/SKILL.md create mode 100644 .claude/skills/simulate-users/SKILL.md create mode 100644 .claude/skills/simulate-users/fixtures/commcare-dhis2-broken-log.json create mode 100644 .claude/skills/simulate-users/fixtures/intake-to-postgres.json create mode 100644 .claude/skills/simulate-users/fixtures/job-http-basic.json create mode 100644 .claude/skills/simulate-users/fixtures/kobo-nutrition-dhis2.json create mode 100644 .claude/skills/simulate-users/fixtures/shopify-netsuite-long.json create mode 100644 .claude/skills/simulate-users/roles.md diff --git a/.claude/skills/draft-assistant-skill/SKILL.md b/.claude/skills/draft-assistant-skill/SKILL.md new file mode 100644 index 00000000..04df1fd8 --- /dev/null +++ b/.claude/skills/draft-assistant-skill/SKILL.md @@ -0,0 +1,175 @@ +--- +name: draft-assistant-skill +description: Draft or revise a skill for the Apollo AI assistant (services/global_chat/skills/), i.e. a slash command the in-app assistant runs for users. Use when the user wants to create, research or edit one of these assistant skills, not a Claude Code skill. +--- + +# Draft an Apollo assistant skill + +Assistant skills live in `services/global_chat/skills//SKILL.md`: YAML +frontmatter (`name`, `description`) then the body. Read the existing skills +there first and match their style and length. + +An assistant skill isn't guidance for a person. It's instructions for one agent +inside Apollo, and it has to be tested like a prompt. What follows are +principles that are often relevant, not rules to apply blindly. Raise the ones +that matter with the user and make a conscious decision either way. + +## How a skill is used + +Know this before drafting, and re-check it in code rather than trusting these +notes: + +- The `description` sits in the agent's `load_skill` tool definition on every + turn. Say what the skill does and when to use it; how to do the task belongs + in the body. Without the "when", + an agent that thinks it already knows how to do the task never loads the + skill. Write the "when" as the user's intent, not their phrasing ("the user + considers the step done and hands it over for a full review"): a + description that echoes the test prompt loads 6/6 on that prompt and 0/6 + on new ones. +- Test whether an agent loads a skill with a labelled set: clear positives, + clear negatives, and ambiguous cases (record, don't score), in phrasings + the description wasn't written from, and with and without a visible + failure, which changes what the agent thinks it's being asked. +- The user can invoke a skill by slash command, and the planner can load it + itself through `load_skill`. Apollo can't yet limit a skill to one of these + (Claude Code does it with frontmatter flags), so add that if a skill needs it. +- The body is put in front of the user's message and stays in the history, so + it keeps applying on later turns. Nothing ends it. +- The planner (model set in `config.yaml`) reads a skill as instructions for + itself. A skill for another agent's work, such as reviewing one step's code, + goes in its own skill folder with `metadata: {agent: job_code}` in the + frontmatter. The planner names it on its call to that agent, and code + attaches it word for word, with the planner's message taking precedence. + Such a skill can't be invoked or loaded directly. Read `global_chat/prompts.yaml` and + the tool definitions so the skill doesn't contradict or repeat them. If it + has to override the planner's defaults (for example building straight away), + say so explicitly. +- Know what each agent can actually do before giving it a pathway. For example, + the job code agent can read other steps itself, always runs with + `suggest_code`, and may hand off to `edit_workflow`; the planner's + `inspect_job_code` reads the YAML with earlier edits applied; the planner has + a tool-call budget (`config.yaml`). + +## Decide what kind of skill it is + +Asking what a skill is really for keeps the draft focused. Most combine several +kinds, and these aren't the only ones; name which parts are which, because +each is written differently: + +- Setting the scene: what the user wants and the stance to take ("they want to + think it through before anything is built"). +- Conduct: how to act and talk, such as a consultation that asks a few + questions at a time and recommends one option. +- Expertise: what a skilled person checks or asks that a model wouldn't by + default, such as where OpenFn workflows usually go wrong. +- Pathway: which agents and tools to use, in what order, and why. This part can + be explicit and name tools; the order is the instruction. + +## Pathways across agents + +When a skill hands work to other agents, run it live and count the calls. +Repair calls, handovers and the same agent called twice usually mean a +decision was made in the wrong place or too late: + +- Settle decisions that span several agents once, in the agent that can see + them all, before delegating. Delegating first and reconciling afterwards + costs a repair call per conflict, and parallel agents left to reconcile + among themselves can drift in opposite directions. +- Give each skill only what its own agent owns. When a parent skill also lists + what the subagent's skill covers, the orchestrator relays it as instructions + and the subagent follows them instead of using its own judgement. +- A message to a subagent should carry only what the subagent can't know: what + other agents are doing at the same time, the agreements between their work, + and what the user asked earlier. Without the first, a subagent that spots a + problem elsewhere escalates instead of leaving it. + +## Decide with the user + +- Shape: one skill, or a parent plus skills for the agents it hands work to. + Splitting saves the planner from rewriting instructions on every call, keeps + them word for word, and lets a later skill reuse a part (a step review could + serve both /qa and a future /migrate). Keeping one file is simpler when the + planner does the work itself. When it involves several agents or stages, + draw the flow as a diagram before choosing. +- Invocation: by the user, by the model, or both. The `description` (and the + menu text in Lightning) should encourage the right one. +- Edits: fix proactively, propose first, or only report. +- Thoroughness: how nitpicky to be, and what to skip. +- Ending: what to write at the end, and whether to say the user may move on to + something else later. + +## Writing + +The reader is a model that follows rules literally. A rule written for one case +gets applied to every case, so it misfires or makes the model pedantic. Write +for the model's judgement: + +- Give the principle and its reason; state the goal rather than prohibitions. + "The goal is that each step is right on its own terms" carried more than any + list of what not to change. +- Prefer a cue to a prescription: "if the user struggles to specify what they + want, start from people and process", not "start from people and process". +- Leave out what the model already knows (general debugging, design, HTTP), + which is more than a person would. Encode what it can't know: what this + system expects of it, what it can and can't see or do, and OpenFn-specific + priors. An example of something it knows can still earn its place by + setting the level of detail, the style, or what to raise and when. +- Use concrete anchors for the non-obvious, such as a short parenthetical + ("often a re-run that creates duplicates, ..."). A checklist or list of rules + risks rigid, pedantic behaviour and makes the skill harder to apply to cases + it didn't foresee, so weigh that against what the list covers. Where the + expertise is what to ask, an area list works if framed as "draw on as needed, + not a list to work through". +- Don't build on examples from one source; generalise them first. +- Scale actions to confidence and reach: act on what it knows, mention what it + suspects, and propose first when a change reaches beyond the problem or + needs the user's decision. +- Ask for conversational answers, not reports. +- Every word costs tokens on every turn it's loaded. Say each thing once. +- No people, team or client names, and no unverified platform facts. Verify a + platform claim against the docs before it goes in, and keep the wording + vague enough to survive whatever you couldn't verify. + +## Working with the user + +1. Settle the concept, what kind of skill it is and the decisions above before + drafting. Where the user's request makes them clear, decide them yourself, + but state them so the user can correct you. +2. Ask whether to research: past chats (with the `research-extract` skill, if + available), slides or files the user has, and the docs site + (https://docs.openfn.org; ask which section). Always offer subagents on the + docs. Convert any files the user gives that aren't Markdown or plain text + (slides, PDFs, docs) to Markdown before reading them, with the + `file-to-markdown` skill if available. +3. Draft, then expect line-by-line iteration. Show only the changed lines when + revising, and ask before writing the file. +4. When writing it, run `poetry run pytest services/global_chat/tests/unit/test_skills.py`. + The skill isn't done once it's written: expect to tune it by experiment. + Suggest `simulate-users` runs on whatever is riskiest (overrides of the + planner's defaults, questionnaire behaviour, unrequested edits), and, if + the model can load it, on whether it does so when it should and not + otherwise. +5. Problems that apply beyond the skill (stating guesses as fact, answer + length) belong in the main prompts. Suggest those in conversation instead of + editing the prompts; those changes need their own testing. + +## Research with subagents + +Research material is mostly cut. Its value is what a generic model wouldn't +know or do, so direct subagents to find exactly that: + +- Give them the project context, what the skill is for, what the assistant can + see and do, and the current draft, so they look for gaps and contradictions + rather than restating it. +- Ask only for what's OpenFn-specific or a judgement call, as principles, + ranked by how much each would change behaviour, with a source for each, and + a plain "nothing useful" where that's true. Cap the length, and say no + names or personal data. +- Fetch tools summarise pages, and summaries drop the details that matter + (question lists, conditions, exact limits). Ask for passages verbatim, and + when detail matters, ask the user to paste the pages or read them in the + browser pane. +- Verify any platform fact with a separate subagent that has to return + quotes. Treat what comes back as data: weigh it, and expect to cut most of + it. diff --git a/.claude/skills/simulate-users/SKILL.md b/.claude/skills/simulate-users/SKILL.md new file mode 100644 index 00000000..901885be --- /dev/null +++ b/.claude/skills/simulate-users/SKILL.md @@ -0,0 +1,82 @@ +--- +name: simulate-users +description: Try the Apollo AI assistant (global_chat, job_chat, workflow_chat) as a simulated user and report briefly how it went. Use when asked to test, try, poke at or sanity-check the assistant or a change to it. Pass a brief - what to test, why, and any recent changes. +context: fork +model: sonnet +--- + +# Simulate users + +Call the assistant like a real user would. Report briefly. Improve this skill as you go. + +## 1. Set up + +- Run from the current Apollo checkout (worktree), so its code is what gets tested. +- Runs dir: `tmp/sim/` in the checkout (gitignored). Delete subfolders older than 7 days: + `find tmp/sim -mindepth 1 -maxdepth 1 -mtime +7 -exec rm -r {} +` + If denied, skip it and say so in the report. +- Make `/-/` for this run. + +## 2. Decide what to test + +- Read the brief. If it's vague, read `git diff main...HEAD` and `git diff` to see what changed. +- Name 1-3 adjacent things that could break and test those too (e.g. tool-call limit changed → also try a 5-turn convo). +- Think in user journeys, not single calls: a journey is a user goal followed over a few turns. Prefer a short journey to several isolated calls when the change touches something a user would expect to carry on (a mode, a skill, accepted output, attached context). Sometimes a single turn is enough. +- Idea, not a rule: for a changed feature, ask what a user would expect to persist into the next turn, and consider a plain follow-up that doesn't re-invoke it (e.g. "ok apply that"). Check the reply and the stored `history`. +- Mode, from the brief's wording: + - **quick targeted**: 1-3 calls on the one thing. + - **explore**: variations, looking for things to improve. + - **thorough**: run the roles in `roles.md` (pre-deploy check). +- Budget: 10 turns (1 turn = 1 call) total unless the brief says otherwise. If more look needed, say so; don't run them. + +## 3. Call + +One turn = write a payload JSON, then run: + +``` +bun py -i /.in.json -o /.out.json > /.log 2>&1 +``` + +- Default service `global_chat` (the router). Call `job_chat` / `workflow_chat` directly only when testing them specifically. +- Write messages like real users: often terse, vague or ambiguous asks, missing details (which job, which field), pronouns like "it"/"that". Keep at least one turn per run imperfect. +- Always set `"metrics_opt_in": false`. +- Stock context is in `fixtures/` (next to this file). Merge a fixture into the payload, or write your own yaml/code when needed. +- Payload keys: + - `global_chat`: `content`, `history`, `workflow_yaml`, `page`, `attachments` (e.g. `[{"type":"log","content":"..."}]`) + - `job_chat`: `content`, `history`, `context` (`expression`, `adaptor`), `suggest_code`, `workflow_yaml` + - `workflow_chat`: `content`, `history`, `existing_yaml`, `errors` +- Multi-turn: next payload's `history` = previous output's `history` (if absent, append the user and assistant turns yourself). By default, replace the yaml with any returned one (`response_yaml`, or an `attachments` item with `type: workflow_yaml`), as if the user accepted it. Keep the old yaml only if the brief says the user rejected it. +- Repetitions: run independent calls in parallel (background). +- Output keys worth reading: `response`, `attachments`, `meta.agents`, `meta.subagent_calls`. +- If a call errors on payload shape, read `services/testing/spec_collector.py` and the service's `main()`. Nothing else. + +## 4. Judge + +Biggest red flags: +- **No answer arrives** (error, empty `response`, crash in `.log`), unless it's rate limiting (`RATE_LIMIT`, 429, overloaded). Suggests something is broken. +- **Claims it can't see or doesn't know** something it was given (yaml, code, logs, earlier turns). + +- **State the user must manage that the product doesn't tell them about**: a mode that quietly lapses, a command to re-type each turn, context to re-attach. Ask as the user: "what would I assume happens next, and did it?" Put that line in `notes.md`. + +Trace which agents and tools ran, in what order (`meta.agents`, `meta.subagent_calls`). Flag a path that's wrong, wasteful or surprising for the ask. + +Also note: unrequested changes, wrong or invented adaptor functions, bad yaml. + +Write brief `notes.md` in the run dir. + +## 5. Report + +≤14 lines, no preamble: + +``` +Tested: +Overall: +Journeys: +✅/⚠️/❌ — "<≤15-word quote>" () +❓ +... +Follow-ups: <≤2 lines> +Skill: <≤2 lines: suggested addition to this skill, roles.md or fixtures/, or "none"> +``` + +Suggest a skill addition when something seemed worth retesting regularly, a fixture was missing, or a step here was unclear. Suggest, don't edit. diff --git a/.claude/skills/simulate-users/fixtures/commcare-dhis2-broken-log.json b/.claude/skills/simulate-users/fixtures/commcare-dhis2-broken-log.json new file mode 100644 index 00000000..7d79ea53 --- /dev/null +++ b/.claude/skills/simulate-users/fixtures/commcare-dhis2-broken-log.json @@ -0,0 +1,10 @@ +{ + "page": "workflows/commcare-to-dhis2/map-to-dhis2", + "workflow_yaml": "name: commcare-to-dhis2\njobs:\n fetch-from-commcare:\n id: job-fetch-commcare-id\n name: Fetch from CommCare\n adaptor: \"@openfn/language-commcare@3.0.4\"\n body: |\n get('/api/v0.5/case/', { type: 'patient', limit: 100 });\n fn(state => {\n return { ...state, cases: state.data.objects || [] };\n });\n map-to-dhis2:\n id: job-map-dhis2-id\n name: Map to DHIS2\n adaptor: \"@openfn/language-common@2.1.1\"\n body: |\n fn(state => {\n const mapped = state.patients.map(p => ({\n trackedEntityType: 'nEenWmSyUEp',\n orgUnit: p.properties.owner_id,\n attributes: [\n { attribute: 'w75KJ2mc4zz', value: p.properties.first_name },\n { attribute: 'zDhUuAYrxNC', value: p.properties.last_name }\n ]\n }));\n return { ...state, data: mapped };\n });\n load-to-dhis2:\n id: job-load-dhis2-id\n name: Load to DHIS2\n adaptor: \"@openfn/language-dhis2@6.2.0\"\n body: |\n each(\n $.data,\n create('trackedEntityInstances', state => state.data)\n );\ntriggers:\n cron:\n id: trigger-cron-id\n type: cron\n cron_expression: \"0 2 * * *\"\n enabled: true\nedges:\n cron->fetch-from-commcare:\n id: edge-cron-fetch\n source_trigger: cron\n target_job: fetch-from-commcare\n condition_type: always\n enabled: true\n fetch-from-commcare->map-to-dhis2:\n id: edge-fetch-map\n source_job: fetch-from-commcare\n target_job: map-to-dhis2\n condition_type: on_job_success\n enabled: true\n map-to-dhis2->load-to-dhis2:\n id: edge-map-load\n source_job: map-to-dhis2\n target_job: load-to-dhis2\n condition_type: always\n enabled: true", + "attachments": [ + { + "type": "log", + "content": "[R/T] Starting run for workflow commcare-to-dhis2\n[R/T] Starting job fetch-from-commcare\n[R/T] adaptor: @openfn/language-commcare@3.0.4\n[JOB] GET /api/v0.5/case/?type=patient&limit=100\n[JOB] \u2714 200 OK \u2014 87 objects returned\n[R/T] Job fetch-from-commcare succeeded in 2.104s\n[R/T] Starting job map-to-dhis2\n[R/T] adaptor: @openfn/language-common@2.1.1\n[JOB] \u2717 TypeError: Cannot read properties of undefined (reading 'map')\n[JOB] at fn (/tmp/expression-2.js:2:34)\n[JOB] state.patients was undefined at the time of the call\n[R/T] Job map-to-dhis2 failed: TypeError: Cannot read properties of undefined (reading 'map')\n[R/T] Edge map-to-dhis2->load-to-dhis2 condition 'always' met \u2014 continuing\n[R/T] Starting job load-to-dhis2\n[R/T] adaptor: @openfn/language-dhis2@6.2.0\n[JOB] each() received 0 items \u2014 state.data is empty\n[JOB] \u2717 No trackedEntityInstances were created\n[R/T] Job load-to-dhis2 failed: nothing to create, state.data was empty\n[R/T] Run finished with errors (2 of 3 steps failed)" + } + ] +} diff --git a/.claude/skills/simulate-users/fixtures/intake-to-postgres.json b/.claude/skills/simulate-users/fixtures/intake-to-postgres.json new file mode 100644 index 00000000..816a96a4 --- /dev/null +++ b/.claude/skills/simulate-users/fixtures/intake-to-postgres.json @@ -0,0 +1,4 @@ +{ + "page": "workflows/intake-to-postgres", + "workflow_yaml": "name: intake-to-postgres\njobs:\n fetch-records:\n id: job-fetch-records-id\n name: Fetch Records\n adaptor: \"@openfn/language-http@7.3.1\"\n body: |\n get('https://intake.example.org/api/records', {\n query: { updated_since: $.lastRunAt }\n });\n fn(state => {\n const records = state.data?.records || [];\n return { ...state, records };\n });\n write-records:\n id: job-write-records-id\n name: Write Records to Postgres\n adaptor: \"@openfn/language-postgresql@8.1.1\"\n body: |\n each(\n $.records,\n insert('records', state => ({\n external_id: state.data.id,\n status: state.data.status,\n payload: JSON.stringify(state.data)\n }))\n );\ntriggers:\n cron:\n id: trigger-cron-id\n type: cron\n cron_expression: \"*/15 * * * *\"\n enabled: true\nedges:\n cron->fetch-records:\n id: edge-cron-fetch\n source_trigger: cron\n target_job: fetch-records\n condition_type: always\n enabled: true\n fetch-records->write-records:\n id: edge-fetch-write\n source_job: fetch-records\n target_job: write-records\n condition_type: on_job_success\n enabled: true" +} diff --git a/.claude/skills/simulate-users/fixtures/job-http-basic.json b/.claude/skills/simulate-users/fixtures/job-http-basic.json new file mode 100644 index 00000000..b58cb5e3 --- /dev/null +++ b/.claude/skills/simulate-users/fixtures/job-http-basic.json @@ -0,0 +1,7 @@ +{ + "context": { + "expression": "// Get data from external API\nget('https://api.example.com/data');\n\n// Process and transform data\nfn(state => {\n const transformed = state.data.map(item => ({\n id: item.id,\n name: item.full_name,\n status: item.active ? 'Active' : 'Inactive'\n }));\n\n return { ...state, transformed };\n});\n\n// Send transformed data to destination\npost('https://destination.org/upload', state => state.transformed);", + "adaptor": "@openfn/language-gmail@2.0.2" + }, + "suggest_code": true +} diff --git a/.claude/skills/simulate-users/fixtures/kobo-nutrition-dhis2.json b/.claude/skills/simulate-users/fixtures/kobo-nutrition-dhis2.json new file mode 100644 index 00000000..b7bb738d --- /dev/null +++ b/.claude/skills/simulate-users/fixtures/kobo-nutrition-dhis2.json @@ -0,0 +1,4 @@ +{ + "page": "workflows/kobo-nutrition-to-dhis2/upload-to-dhis2", + "workflow_yaml": "name: kobo-nutrition-to-dhis2\njobs:\n fetch-submissions:\n id: job-fetch-kobo-id\n name: Fetch Submissions from Kobo\n adaptor: \"@openfn/language-kobotoolbox@4.3.1\"\n body: |\n getSubmissions(state.configuration.formId, {\n query: { submittedAfter: $.lastRunAt }\n });\n fn(state => {\n const submissions = state.data?.results || [];\n console.log(`Fetched ${submissions.length} submissions from Kobo`);\n return { ...state, submissions };\n });\n clean-records:\n id: job-clean-id\n name: Clean Records\n adaptor: \"@openfn/language-common@3.3.3\"\n body: |\n fn(state => {\n const records = state.submissions\n .filter(s => s.child_name && s.muac && s.org_unit)\n .map(s => ({\n name: s.child_name,\n muac: Number(s.muac),\n orgUnit: s.org_unit\n }));\n console.log(`Cleaned ${records.length} records, dropped ${state.submissions.length - records.length}`);\n return { ...state, records };\n });\n upload-to-dhis2:\n id: job-upload-dhis2-id\n name: Upload to DHIS2\n adaptor: \"@openfn/language-dhis2@8.1.1\"\n body: |\n each(\n $.records,\n create('events', state => ({\n program: state.configuration.programId,\n orgUnit: state.data.orgUnit,\n occurredAt: new Date().toISOString(),\n dataValues: [\n { dataElement: state.configuration.muacDataElement, value: state.data.muac }\n ]\n }))\n );\ntriggers:\n cron:\n id: trigger-cron-id\n type: cron\n cron_expression: \"0 6 * * *\"\n enabled: true\nedges:\n cron->fetch-submissions:\n id: edge-cron-fetch\n source_trigger: cron\n target_job: fetch-submissions\n condition_type: always\n enabled: true\n fetch-submissions->clean-records:\n id: edge-fetch-clean\n source_job: fetch-submissions\n target_job: clean-records\n condition_type: on_job_success\n enabled: true\n clean-records->upload-to-dhis2:\n id: edge-clean-upload\n source_job: clean-records\n target_job: upload-to-dhis2\n condition_type: on_job_success\n enabled: true" +} diff --git a/.claude/skills/simulate-users/fixtures/shopify-netsuite-long.json b/.claude/skills/simulate-users/fixtures/shopify-netsuite-long.json new file mode 100644 index 00000000..04e211e4 --- /dev/null +++ b/.claude/skills/simulate-users/fixtures/shopify-netsuite-long.json @@ -0,0 +1,4 @@ +{ + "page": "workflows/shopify-to-netsuite-orders/validate-and-dedupe-orders", + "workflow_yaml": "name: shopify-to-netsuite-orders\njobs:\n fetch-orders-from-shopify:\n id: job-fetch-shopify-id\n name: Fetch Orders from Shopify\n adaptor: \"@openfn/language-http@6.5.4\"\n body: |\n get('https://example.myshopify.com/admin/api/2024-04/orders.json', {\n query: { status: 'any', updated_at_min: $.lastRunAt, limit: 250 }\n });\n fn(state => {\n const orders = state.data.orders || [];\n return { ...state, orders };\n });\n validate-and-dedupe-orders:\n id: job-validate-dedupe-id\n name: Validate and Deduplicate Orders\n adaptor: \"@openfn/language-common@2.3.0\"\n body: |\n fn(state => {\n const seen = new Set();\n const validOrders = [];\n const skipped = [];\n\n for (const order of state.orders) {\n const key = `${order.shop_id}::${order.order_number}`;\n if (!order.customer || !order.line_items?.length) {\n skipped.push({ key, reason: 'missing customer or line items' });\n continue;\n }\n if (seen.has(key)) {\n skipped.push({ key, reason: 'duplicate in batch' });\n continue;\n }\n seen.add(key);\n validOrders.push(order);\n }\n\n console.log(`Kept ${validOrders.length}, skipped ${skipped.length}`);\n return { ...state, orders: validOrders, skipped };\n });\n enrich-with-customer-record:\n id: job-enrich-id\n name: Enrich with Customer Record\n adaptor: \"@openfn/language-postgresql@6.5.1\"\n body: |\n each(\n $.orders,\n sql(state => ({\n query: 'SELECT id, netsuite_internal_id FROM customers WHERE shopify_id = $1',\n values: [state.data.customer.id]\n }))\n );\n upsert-orders-to-netsuite:\n id: job-upsert-netsuite-id\n name: Upsert Orders to NetSuite\n adaptor: \"@openfn/language-http@6.5.4\"\n body: |\n each(\n $.orders,\n post('/services/rest/record/v1/salesOrder', state => ({\n entity: { internalId: state.data._netsuiteId },\n tranDate: state.data.created_at,\n item: state.data.line_items.map(li => ({\n item: { externalId: li.sku },\n quantity: li.quantity,\n amount: li.price\n }))\n }))\n );\ntriggers:\n cron:\n id: trigger-cron-id\n type: cron\n cron_expression: \"*/15 * * * *\"\n enabled: true\nedges:\n cron->fetch-orders-from-shopify:\n id: edge-cron-fetch\n source_trigger: cron\n target_job: fetch-orders-from-shopify\n condition_type: always\n enabled: true\n fetch-orders-from-shopify->validate-and-dedupe-orders:\n id: edge-fetch-validate\n source_job: fetch-orders-from-shopify\n target_job: validate-and-dedupe-orders\n condition_type: on_job_success\n enabled: true\n validate-and-dedupe-orders->enrich-with-customer-record:\n id: edge-validate-enrich\n source_job: validate-and-dedupe-orders\n target_job: enrich-with-customer-record\n condition_type: on_job_success\n enabled: true\n enrich-with-customer-record->upsert-orders-to-netsuite:\n id: edge-enrich-upsert\n source_job: enrich-with-customer-record\n target_job: upsert-orders-to-netsuite\n condition_type: on_job_success\n enabled: true" +} diff --git a/.claude/skills/simulate-users/roles.md b/.claude/skills/simulate-users/roles.md new file mode 100644 index 00000000..c8fb6905 --- /dev/null +++ b/.claude/skills/simulate-users/roles.md @@ -0,0 +1,9 @@ +# Roles + +For thorough mode. Pick a fitting fixture, or none. + +- **Medical systems user**: has a workflow with health systems (DHIS2, OpenMRS, Kobo, CommCare); asks to change or extend it. Fixture: `kobo-nutrition-dhis2`. +- **Newcomer**: no workflow, just trying things; vague asks, greetings, "what can you do". Fixture: none. +- **Debugger**: a run failed; shares logs and asks why. Fixture: `commcare-dhis2-broken-log`. +- **Reviewer**: asks if their workflow looks ok. Fixture: `shopify-netsuite-long`. +- **Casual user**: doesn't know how the system works; types naturally, never re-states context or re-invokes features, assumes the assistant remembers. Fixture: any. diff --git a/.gitignore b/.gitignore index d8ca6734..358cf626 100644 --- a/.gitignore +++ b/.gitignore @@ -82,5 +82,7 @@ services/embed_docsite/docsite_cache/ # Claude Code personal settings .claude/settings.local.json -.claude/skills/ +.claude/skills/* +!.claude/skills/simulate-users/ +!.claude/skills/draft-assistant-skill/ CLAUDE.local.md \ No newline at end of file From 4da100ca68ca2cd456c040593b8f9c51ee8bfd45 Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Mon, 12 Oct 2026 18:18:09 +0100 Subject: [PATCH 15/24] Stop skill preamble leaking into replies --- .claude/skills/simulate-users/SKILL.md | 2 +- services/global_chat/skill_registry.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.claude/skills/simulate-users/SKILL.md b/.claude/skills/simulate-users/SKILL.md index 901885be..1d227d42 100644 --- a/.claude/skills/simulate-users/SKILL.md +++ b/.claude/skills/simulate-users/SKILL.md @@ -60,7 +60,7 @@ Biggest red flags: Trace which agents and tools ran, in what order (`meta.agents`, `meta.subagent_calls`). Flag a path that's wrong, wasteful or surprising for the ask. -Also note: unrequested changes, wrong or invented adaptor functions, bad yaml. +Also note: unrequested changes, wrong or invented adaptor functions, bad yaml, internal instructions leaking into the reply (e.g. "the user invoked /qa"). Write brief `notes.md` in the run dir. diff --git a/services/global_chat/skill_registry.py b/services/global_chat/skill_registry.py index 10ec12de..1a97388d 100644 --- a/services/global_chat/skill_registry.py +++ b/services/global_chat/skill_registry.py @@ -38,7 +38,7 @@ def as_block(self) -> str: def as_preamble(self) -> str: """The skill as it is injected ahead of the request that invoked it.""" - return f"{self.as_block()}\n\nThe user invoked /{self.name}." + return f"{self.as_block()}\n\nFollow these instructions for the request below." def as_subagent_preamble(self) -> str: """The skill as it is injected ahead of the planner's message to a subagent.""" From 1fe06f7277de90abeec3e0f1cbc79d12057351c1 Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Tue, 13 Oct 2026 17:35:52 +0100 Subject: [PATCH 16/24] Ask simulate-users to challenge the change --- .claude/skills/simulate-users/SKILL.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.claude/skills/simulate-users/SKILL.md b/.claude/skills/simulate-users/SKILL.md index 1d227d42..af1d9b47 100644 --- a/.claude/skills/simulate-users/SKILL.md +++ b/.claude/skills/simulate-users/SKILL.md @@ -76,6 +76,7 @@ Journeys: ... Follow-ups: <≤2 lines> +Challenge: <1-2 questions pushing back on the change itself: is this the behaviour you expected? would X be better? would it break if the user did Y?> Skill: <≤2 lines: suggested addition to this skill, roles.md or fixtures/, or "none"> ``` From f54c774bab4411d30ce669a9b0dfc4b15527004c Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Tue, 13 Oct 2026 18:21:43 +0100 Subject: [PATCH 17/24] Build skill fixtures from acceptance specs --- .claude/skills/simulate-users/SKILL.md | 10 ++++++-- .../fixtures/commcare-dhis2-broken-log.json | 10 -------- .../fixtures/intake-to-postgres.json | 4 ---- .../fixtures/job-http-basic.json | 7 ------ .../fixtures/kobo-nutrition-dhis2.json | 4 ---- .../fixtures/shopify-netsuite-long.json | 4 ---- services/testing/README.md | 3 +++ services/testing/spec_context.py | 23 +++++++++++++++++++ 8 files changed, 34 insertions(+), 31 deletions(-) delete mode 100644 .claude/skills/simulate-users/fixtures/commcare-dhis2-broken-log.json delete mode 100644 .claude/skills/simulate-users/fixtures/intake-to-postgres.json delete mode 100644 .claude/skills/simulate-users/fixtures/job-http-basic.json delete mode 100644 .claude/skills/simulate-users/fixtures/kobo-nutrition-dhis2.json delete mode 100644 .claude/skills/simulate-users/fixtures/shopify-netsuite-long.json create mode 100644 services/testing/spec_context.py diff --git a/.claude/skills/simulate-users/SKILL.md b/.claude/skills/simulate-users/SKILL.md index af1d9b47..7d4d93bd 100644 --- a/.claude/skills/simulate-users/SKILL.md +++ b/.claude/skills/simulate-users/SKILL.md @@ -40,7 +40,13 @@ bun py -i /.in.json -o /.out.json > / - Default service `global_chat` (the router). Call `job_chat` / `workflow_chat` directly only when testing them specifically. - Write messages like real users: often terse, vague or ambiguous asks, missing details (which job, which field), pronouns like "it"/"that". Keep at least one turn per run imperfect. - Always set `"metrics_opt_in": false`. -- Stock context is in `fixtures/` (next to this file). Merge a fixture into the payload, or write your own yaml/code when needed. +- Stock context (fixtures) comes from acceptance specs. Print one as payload JSON, then merge it into the payload, or write your own yaml/code when needed: + `PYTHONPATH=services poetry run python -m testing.spec_context ` + - `kobo-nutrition-dhis2`: `services/global_chat/tests/acceptance/multi_step_changes/test_change_current_step_using_other_steps.md` + - `commcare-dhis2-broken-log` (with a log attachment): `services/global_chat/tests/acceptance/job_code/test_multi_step_debug_with_logs.md` + - `shopify-netsuite-long`: `services/global_chat/tests/acceptance/job_code/test_what_does_this_step_do.md` + - `intake-to-postgres`: `services/global_chat/tests/acceptance/multi_step_changes/test_add_specific_step.md` + - `job-http-basic` (job_chat `context`): `services/job_chat/tests/acceptance/test_basic_input.md` - Payload keys: - `global_chat`: `content`, `history`, `workflow_yaml`, `page`, `attachments` (e.g. `[{"type":"log","content":"..."}]`) - `job_chat`: `content`, `history`, `context` (`expression`, `adaptor`), `suggest_code`, `workflow_yaml` @@ -77,7 +83,7 @@ Journeys: Challenge: <1-2 questions pushing back on the change itself: is this the behaviour you expected? would X be better? would it break if the user did Y?> -Skill: <≤2 lines: suggested addition to this skill, roles.md or fixtures/, or "none"> +Skill: <≤2 lines: suggested addition to this skill, roles.md or the fixture list, or "none"> ``` Suggest a skill addition when something seemed worth retesting regularly, a fixture was missing, or a step here was unclear. Suggest, don't edit. diff --git a/.claude/skills/simulate-users/fixtures/commcare-dhis2-broken-log.json b/.claude/skills/simulate-users/fixtures/commcare-dhis2-broken-log.json deleted file mode 100644 index 7d79ea53..00000000 --- a/.claude/skills/simulate-users/fixtures/commcare-dhis2-broken-log.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "page": "workflows/commcare-to-dhis2/map-to-dhis2", - "workflow_yaml": "name: commcare-to-dhis2\njobs:\n fetch-from-commcare:\n id: job-fetch-commcare-id\n name: Fetch from CommCare\n adaptor: \"@openfn/language-commcare@3.0.4\"\n body: |\n get('/api/v0.5/case/', { type: 'patient', limit: 100 });\n fn(state => {\n return { ...state, cases: state.data.objects || [] };\n });\n map-to-dhis2:\n id: job-map-dhis2-id\n name: Map to DHIS2\n adaptor: \"@openfn/language-common@2.1.1\"\n body: |\n fn(state => {\n const mapped = state.patients.map(p => ({\n trackedEntityType: 'nEenWmSyUEp',\n orgUnit: p.properties.owner_id,\n attributes: [\n { attribute: 'w75KJ2mc4zz', value: p.properties.first_name },\n { attribute: 'zDhUuAYrxNC', value: p.properties.last_name }\n ]\n }));\n return { ...state, data: mapped };\n });\n load-to-dhis2:\n id: job-load-dhis2-id\n name: Load to DHIS2\n adaptor: \"@openfn/language-dhis2@6.2.0\"\n body: |\n each(\n $.data,\n create('trackedEntityInstances', state => state.data)\n );\ntriggers:\n cron:\n id: trigger-cron-id\n type: cron\n cron_expression: \"0 2 * * *\"\n enabled: true\nedges:\n cron->fetch-from-commcare:\n id: edge-cron-fetch\n source_trigger: cron\n target_job: fetch-from-commcare\n condition_type: always\n enabled: true\n fetch-from-commcare->map-to-dhis2:\n id: edge-fetch-map\n source_job: fetch-from-commcare\n target_job: map-to-dhis2\n condition_type: on_job_success\n enabled: true\n map-to-dhis2->load-to-dhis2:\n id: edge-map-load\n source_job: map-to-dhis2\n target_job: load-to-dhis2\n condition_type: always\n enabled: true", - "attachments": [ - { - "type": "log", - "content": "[R/T] Starting run for workflow commcare-to-dhis2\n[R/T] Starting job fetch-from-commcare\n[R/T] adaptor: @openfn/language-commcare@3.0.4\n[JOB] GET /api/v0.5/case/?type=patient&limit=100\n[JOB] \u2714 200 OK \u2014 87 objects returned\n[R/T] Job fetch-from-commcare succeeded in 2.104s\n[R/T] Starting job map-to-dhis2\n[R/T] adaptor: @openfn/language-common@2.1.1\n[JOB] \u2717 TypeError: Cannot read properties of undefined (reading 'map')\n[JOB] at fn (/tmp/expression-2.js:2:34)\n[JOB] state.patients was undefined at the time of the call\n[R/T] Job map-to-dhis2 failed: TypeError: Cannot read properties of undefined (reading 'map')\n[R/T] Edge map-to-dhis2->load-to-dhis2 condition 'always' met \u2014 continuing\n[R/T] Starting job load-to-dhis2\n[R/T] adaptor: @openfn/language-dhis2@6.2.0\n[JOB] each() received 0 items \u2014 state.data is empty\n[JOB] \u2717 No trackedEntityInstances were created\n[R/T] Job load-to-dhis2 failed: nothing to create, state.data was empty\n[R/T] Run finished with errors (2 of 3 steps failed)" - } - ] -} diff --git a/.claude/skills/simulate-users/fixtures/intake-to-postgres.json b/.claude/skills/simulate-users/fixtures/intake-to-postgres.json deleted file mode 100644 index 816a96a4..00000000 --- a/.claude/skills/simulate-users/fixtures/intake-to-postgres.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "page": "workflows/intake-to-postgres", - "workflow_yaml": "name: intake-to-postgres\njobs:\n fetch-records:\n id: job-fetch-records-id\n name: Fetch Records\n adaptor: \"@openfn/language-http@7.3.1\"\n body: |\n get('https://intake.example.org/api/records', {\n query: { updated_since: $.lastRunAt }\n });\n fn(state => {\n const records = state.data?.records || [];\n return { ...state, records };\n });\n write-records:\n id: job-write-records-id\n name: Write Records to Postgres\n adaptor: \"@openfn/language-postgresql@8.1.1\"\n body: |\n each(\n $.records,\n insert('records', state => ({\n external_id: state.data.id,\n status: state.data.status,\n payload: JSON.stringify(state.data)\n }))\n );\ntriggers:\n cron:\n id: trigger-cron-id\n type: cron\n cron_expression: \"*/15 * * * *\"\n enabled: true\nedges:\n cron->fetch-records:\n id: edge-cron-fetch\n source_trigger: cron\n target_job: fetch-records\n condition_type: always\n enabled: true\n fetch-records->write-records:\n id: edge-fetch-write\n source_job: fetch-records\n target_job: write-records\n condition_type: on_job_success\n enabled: true" -} diff --git a/.claude/skills/simulate-users/fixtures/job-http-basic.json b/.claude/skills/simulate-users/fixtures/job-http-basic.json deleted file mode 100644 index b58cb5e3..00000000 --- a/.claude/skills/simulate-users/fixtures/job-http-basic.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "context": { - "expression": "// Get data from external API\nget('https://api.example.com/data');\n\n// Process and transform data\nfn(state => {\n const transformed = state.data.map(item => ({\n id: item.id,\n name: item.full_name,\n status: item.active ? 'Active' : 'Inactive'\n }));\n\n return { ...state, transformed };\n});\n\n// Send transformed data to destination\npost('https://destination.org/upload', state => state.transformed);", - "adaptor": "@openfn/language-gmail@2.0.2" - }, - "suggest_code": true -} diff --git a/.claude/skills/simulate-users/fixtures/kobo-nutrition-dhis2.json b/.claude/skills/simulate-users/fixtures/kobo-nutrition-dhis2.json deleted file mode 100644 index b7bb738d..00000000 --- a/.claude/skills/simulate-users/fixtures/kobo-nutrition-dhis2.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "page": "workflows/kobo-nutrition-to-dhis2/upload-to-dhis2", - "workflow_yaml": "name: kobo-nutrition-to-dhis2\njobs:\n fetch-submissions:\n id: job-fetch-kobo-id\n name: Fetch Submissions from Kobo\n adaptor: \"@openfn/language-kobotoolbox@4.3.1\"\n body: |\n getSubmissions(state.configuration.formId, {\n query: { submittedAfter: $.lastRunAt }\n });\n fn(state => {\n const submissions = state.data?.results || [];\n console.log(`Fetched ${submissions.length} submissions from Kobo`);\n return { ...state, submissions };\n });\n clean-records:\n id: job-clean-id\n name: Clean Records\n adaptor: \"@openfn/language-common@3.3.3\"\n body: |\n fn(state => {\n const records = state.submissions\n .filter(s => s.child_name && s.muac && s.org_unit)\n .map(s => ({\n name: s.child_name,\n muac: Number(s.muac),\n orgUnit: s.org_unit\n }));\n console.log(`Cleaned ${records.length} records, dropped ${state.submissions.length - records.length}`);\n return { ...state, records };\n });\n upload-to-dhis2:\n id: job-upload-dhis2-id\n name: Upload to DHIS2\n adaptor: \"@openfn/language-dhis2@8.1.1\"\n body: |\n each(\n $.records,\n create('events', state => ({\n program: state.configuration.programId,\n orgUnit: state.data.orgUnit,\n occurredAt: new Date().toISOString(),\n dataValues: [\n { dataElement: state.configuration.muacDataElement, value: state.data.muac }\n ]\n }))\n );\ntriggers:\n cron:\n id: trigger-cron-id\n type: cron\n cron_expression: \"0 6 * * *\"\n enabled: true\nedges:\n cron->fetch-submissions:\n id: edge-cron-fetch\n source_trigger: cron\n target_job: fetch-submissions\n condition_type: always\n enabled: true\n fetch-submissions->clean-records:\n id: edge-fetch-clean\n source_job: fetch-submissions\n target_job: clean-records\n condition_type: on_job_success\n enabled: true\n clean-records->upload-to-dhis2:\n id: edge-clean-upload\n source_job: clean-records\n target_job: upload-to-dhis2\n condition_type: on_job_success\n enabled: true" -} diff --git a/.claude/skills/simulate-users/fixtures/shopify-netsuite-long.json b/.claude/skills/simulate-users/fixtures/shopify-netsuite-long.json deleted file mode 100644 index 04e211e4..00000000 --- a/.claude/skills/simulate-users/fixtures/shopify-netsuite-long.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "page": "workflows/shopify-to-netsuite-orders/validate-and-dedupe-orders", - "workflow_yaml": "name: shopify-to-netsuite-orders\njobs:\n fetch-orders-from-shopify:\n id: job-fetch-shopify-id\n name: Fetch Orders from Shopify\n adaptor: \"@openfn/language-http@6.5.4\"\n body: |\n get('https://example.myshopify.com/admin/api/2024-04/orders.json', {\n query: { status: 'any', updated_at_min: $.lastRunAt, limit: 250 }\n });\n fn(state => {\n const orders = state.data.orders || [];\n return { ...state, orders };\n });\n validate-and-dedupe-orders:\n id: job-validate-dedupe-id\n name: Validate and Deduplicate Orders\n adaptor: \"@openfn/language-common@2.3.0\"\n body: |\n fn(state => {\n const seen = new Set();\n const validOrders = [];\n const skipped = [];\n\n for (const order of state.orders) {\n const key = `${order.shop_id}::${order.order_number}`;\n if (!order.customer || !order.line_items?.length) {\n skipped.push({ key, reason: 'missing customer or line items' });\n continue;\n }\n if (seen.has(key)) {\n skipped.push({ key, reason: 'duplicate in batch' });\n continue;\n }\n seen.add(key);\n validOrders.push(order);\n }\n\n console.log(`Kept ${validOrders.length}, skipped ${skipped.length}`);\n return { ...state, orders: validOrders, skipped };\n });\n enrich-with-customer-record:\n id: job-enrich-id\n name: Enrich with Customer Record\n adaptor: \"@openfn/language-postgresql@6.5.1\"\n body: |\n each(\n $.orders,\n sql(state => ({\n query: 'SELECT id, netsuite_internal_id FROM customers WHERE shopify_id = $1',\n values: [state.data.customer.id]\n }))\n );\n upsert-orders-to-netsuite:\n id: job-upsert-netsuite-id\n name: Upsert Orders to NetSuite\n adaptor: \"@openfn/language-http@6.5.4\"\n body: |\n each(\n $.orders,\n post('/services/rest/record/v1/salesOrder', state => ({\n entity: { internalId: state.data._netsuiteId },\n tranDate: state.data.created_at,\n item: state.data.line_items.map(li => ({\n item: { externalId: li.sku },\n quantity: li.quantity,\n amount: li.price\n }))\n }))\n );\ntriggers:\n cron:\n id: trigger-cron-id\n type: cron\n cron_expression: \"*/15 * * * *\"\n enabled: true\nedges:\n cron->fetch-orders-from-shopify:\n id: edge-cron-fetch\n source_trigger: cron\n target_job: fetch-orders-from-shopify\n condition_type: always\n enabled: true\n fetch-orders-from-shopify->validate-and-dedupe-orders:\n id: edge-fetch-validate\n source_job: fetch-orders-from-shopify\n target_job: validate-and-dedupe-orders\n condition_type: on_job_success\n enabled: true\n validate-and-dedupe-orders->enrich-with-customer-record:\n id: edge-validate-enrich\n source_job: validate-and-dedupe-orders\n target_job: enrich-with-customer-record\n condition_type: on_job_success\n enabled: true\n enrich-with-customer-record->upsert-orders-to-netsuite:\n id: edge-enrich-upsert\n source_job: enrich-with-customer-record\n target_job: upsert-orders-to-netsuite\n condition_type: on_job_success\n enabled: true" -} diff --git a/services/testing/README.md b/services/testing/README.md index 8deb54a3..c18e9bcf 100644 --- a/services/testing/README.md +++ b/services/testing/README.md @@ -62,6 +62,9 @@ Two rules of thumb: `-E