From 4c9a343150cc191e3c05c012a8b1d0e8aa1fa9e9 Mon Sep 17 00:00:00 2001 From: Kai Xu Date: Fri, 8 May 2026 16:14:15 +0000 Subject: [PATCH 1/4] feat: close plugin-vs-CLI gaps (issue #199) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four fixes to bring the plugin path closer to CLI observability: 1. Event capture: add `factory emit` CLI subcommand so pipeline skills can log agent.started/completed events to events.jsonl 2. Playbook injection: generate_agent_content() now calls load_playbook() + inject_playbook() — plugin agents get the same behavioral playbooks as CLI agents 3. Model flexibility: omit model from generated frontmatter so plugin subagents inherit the parent session's model 4. Review persistence: pipeline-subagents skill now instructs writing output to .factory/reviews/-latest.md Co-Authored-By: Claude Opus 4.6 (1M context) --- factory/agents/plugin.py | 6 ++- factory/cli.py | 20 ++++++++++ skills/pipeline-subagents/SKILL.md | 9 ++++- tests/test_events.py | 62 ++++++++++++++++++++++++++++++ tests/test_plugin_agents.py | 6 +-- 5 files changed, 97 insertions(+), 6 deletions(-) diff --git a/factory/agents/plugin.py b/factory/agents/plugin.py index ada2e6d..cdae0cb 100644 --- a/factory/agents/plugin.py +++ b/factory/agents/plugin.py @@ -8,6 +8,7 @@ import yaml +from factory.ace.injector import inject_playbook, load_playbook from factory.agents.runner import _PROMPTS_DIR _AGENTS_YML = Path(__file__).parent / "agents.yml" @@ -52,8 +53,11 @@ def generate_agent_content(role: str) -> str: meta = config[role] prompt = (_PROMPTS_DIR / f"{role}.md").read_text() + playbook = load_playbook(role) + if playbook: + prompt = inject_playbook(prompt, playbook) frontmatter = yaml.dump( - {"name": role, "description": meta.description, "model": meta.model, "tools": meta.tools}, + {"name": role, "description": meta.description, "tools": meta.tools}, default_flow_style=False, sort_keys=False, allow_unicode=True, diff --git a/factory/cli.py b/factory/cli.py index c8acb7c..b2959c9 100644 --- a/factory/cli.py +++ b/factory/cli.py @@ -1106,6 +1106,18 @@ def cmd_explain(args: argparse.Namespace) -> int: return 0 +def cmd_emit(args: argparse.Namespace) -> int: + from factory.events import emit_event + + project_path = Path(args.project).resolve() + data = {} + if args.data: + import json as _json + data = _json.loads(args.data) + emit_event(project_path, args.event_type, agent=args.agent, data=data) + return 0 + + def cmd_vault_init(args: argparse.Namespace) -> int: from factory.obsidian.notes import init_vault @@ -2357,6 +2369,13 @@ def build_parser() -> argparse.ArgumentParser: p.add_argument("--port", type=int, default=8420, help="Server port (default: 8420)") p.add_argument("--host", default="0.0.0.0", help="Server host (default: 0.0.0.0)") + # emit — emit a structured event to .factory/events.jsonl + p = sub.add_parser("emit", help="Emit a structured event to .factory/events.jsonl") + p.add_argument("event_type", help="Event type (e.g. agent.started, agent.completed)") + p.add_argument("--agent", default=None, help="Agent role name") + p.add_argument("--project", default=".", help="Project path") + p.add_argument("--data", default=None, help="JSON string of additional event data") + # agent — invoke a specialist agent directly p = sub.add_parser("agent", help="Invoke a specialist agent with a task") p.add_argument("role", choices=["researcher", "strategist", "builder", "reviewer", @@ -2552,6 +2571,7 @@ def main(argv: list[str] | None = None) -> int: "install": cmd_install, "serve-mcp": cmd_serve_mcp, "dashboard": cmd_dashboard, + "emit": cmd_emit, "agent": cmd_agent, "ceo": cmd_ceo, "run": cmd_run, diff --git a/skills/pipeline-subagents/SKILL.md b/skills/pipeline-subagents/SKILL.md index 77b28c4..368f41f 100644 --- a/skills/pipeline-subagents/SKILL.md +++ b/skills/pipeline-subagents/SKILL.md @@ -104,11 +104,16 @@ Process steps in topological order: - Parallel batch: multiple Agent calls in same message - Archival: can use `run_in_background: true` 4. **Read results** — Agent tool returns subagent output directly -5. **Apply gate rule:** +5. **Persist review** — Write the agent's output to `.factory/reviews/-latest.md` to match the CLI path's artifact structure: + ```bash + mkdir -p .factory/reviews + ``` + Then use the Write tool to save the agent output to `.factory/reviews/-latest.md` +6. **Apply gate rule:** - **PROCEED**: Move to next step - **REDIRECT**: Re-invoke with corrections (max 2 per step) - **ABORT**: Skip downstream steps, jump to summary -6. **Repeat** until done +7. **Repeat** until done ### Error Recovery diff --git a/tests/test_events.py b/tests/test_events.py index 71ba596..d8833fd 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +from argparse import Namespace from datetime import datetime, timezone from factory.events import discover_factory_projects, emit_event, load_events @@ -155,3 +156,64 @@ def test_returns_sorted(self, tmp_path): projects = discover_factory_projects(tmp_path) assert projects[0].name == "alpha" assert projects[1].name == "zeta" + + +class TestCmdEmit: + def test_cmd_emit_writes_event(self, tmp_path): + from factory.cli import cmd_emit + + (tmp_path / ".factory").mkdir() + args = Namespace( + event_type="agent.started", + agent="researcher", + project=str(tmp_path), + data=None, + ) + rc = cmd_emit(args) + assert rc == 0 + + events_file = tmp_path / ".factory" / "events.jsonl" + assert events_file.exists() + + event = json.loads(events_file.read_text().strip()) + assert event["type"] == "agent.started" + assert event["agent"] == "researcher" + + def test_cmd_emit_with_data(self, tmp_path): + from factory.cli import cmd_emit + + (tmp_path / ".factory").mkdir() + args = Namespace( + event_type="agent.completed", + agent="builder", + project=str(tmp_path), + data='{"task": "fix bug", "duration": 42}', + ) + rc = cmd_emit(args) + assert rc == 0 + + events_file = tmp_path / ".factory" / "events.jsonl" + event = json.loads(events_file.read_text().strip()) + assert event["type"] == "agent.completed" + assert event["agent"] == "builder" + assert event["data"]["task"] == "fix bug" + assert event["data"]["duration"] == 42 + + def test_cmd_emit_without_agent(self, tmp_path): + from factory.cli import cmd_emit + + (tmp_path / ".factory").mkdir() + args = Namespace( + event_type="cycle.started", + agent=None, + project=str(tmp_path), + data='{"cycle": 1}', + ) + rc = cmd_emit(args) + assert rc == 0 + + events_file = tmp_path / ".factory" / "events.jsonl" + event = json.loads(events_file.read_text().strip()) + assert event["type"] == "cycle.started" + assert event["agent"] is None + assert event["data"]["cycle"] == 1 diff --git a/tests/test_plugin_agents.py b/tests/test_plugin_agents.py index c2f4a8a..3ce6963 100644 --- a/tests/test_plugin_agents.py +++ b/tests/test_plugin_agents.py @@ -86,7 +86,6 @@ def test_frontmatter_has_required_fields(self): fm = _parse_frontmatter(content) assert "name" in fm, f"{role}: missing name" assert "description" in fm, f"{role}: missing description" - assert "model" in fm, f"{role}: missing model" assert "tools" in fm, f"{role}: missing tools" def test_frontmatter_name_matches_role(self): @@ -108,8 +107,9 @@ def test_preserves_prompt_content(self): for role in ALL_ROLES: source = (_PROMPTS_DIR / f"{role}.md").read_text() generated = generate_agent_content(role) - assert generated.endswith(source), ( - f"{role}: generated file does not end with source prompt" + # Generated content may have playbook injected, so check source is in it + assert source in generated, ( + f"{role}: generated file does not include source prompt" ) def test_unknown_role_raises(self): From e145f56f466ae0811d4bc961c8706349ea29c128 Mon Sep 17 00:00:00 2001 From: Kai Xu Date: Fri, 8 May 2026 18:37:41 +0000 Subject: [PATCH 2/4] test: add parser tests for factory emit subcommand Covers the argparse definition lines to fix codecov/patch. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/test_cli.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_cli.py b/tests/test_cli.py index f6b9c00..206eb30 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -80,6 +80,21 @@ def test_finalize_with_scores(self): def test_no_command_returns_1(self): assert main([]) == 1 + def test_emit_subcommand(self): + parser = build_parser() + args = parser.parse_args(["emit", "agent.started", "--agent", "researcher", "--project", "/p"]) + assert args.command == "emit" + assert args.event_type == "agent.started" + assert args.agent == "researcher" + assert args.project == "/p" + + def test_emit_defaults(self): + parser = build_parser() + args = parser.parse_args(["emit", "cycle.started"]) + assert args.agent is None + assert args.project == "." + assert args.data is None + def test_ceo_mode_interactive(self): parser = build_parser() args = parser.parse_args(["ceo", "distributed eval runner", "--mode", "interactive"]) From be945bf4a56c8acd5570ad09ccb1d0c78a3323d8 Mon Sep 17 00:00:00 2001 From: Kai Xu Date: Fri, 8 May 2026 18:50:46 +0000 Subject: [PATCH 3/4] fix: pipeline-subagents skill now emits events via factory emit The Agent tool doesn't go through the factory runner, so events aren't emitted automatically. The skill now calls factory emit before/after each agent invocation for observability parity. Co-Authored-By: Claude Opus 4.6 (1M context) --- skills/pipeline-subagents/SKILL.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/skills/pipeline-subagents/SKILL.md b/skills/pipeline-subagents/SKILL.md index 368f41f..71e584f 100644 --- a/skills/pipeline-subagents/SKILL.md +++ b/skills/pipeline-subagents/SKILL.md @@ -99,14 +99,16 @@ Process steps in topological order: 1. **Identify next batch** — steps whose dependencies are all complete 2. **Build prompts** — incorporate output from prior steps (agent results are returned directly) -3. **Invoke agents:** - - Single step: one Agent call - - Parallel batch: multiple Agent calls in same message - - Archival: can use `run_in_background: true` +3. **Emit start event** and **invoke agents:** + ```bash + factory emit agent.started --agent --project "$(pwd)" + ``` + Then invoke via Agent tool (single, parallel batch, or background for archival). 4. **Read results** — Agent tool returns subagent output directly -5. **Persist review** — Write the agent's output to `.factory/reviews/-latest.md` to match the CLI path's artifact structure: +5. **Persist review and emit completion:** ```bash mkdir -p .factory/reviews + factory emit agent.completed --agent --project "$(pwd)" ``` Then use the Write tool to save the agent output to `.factory/reviews/-latest.md` 6. **Apply gate rule:** From 7e6f1ed9c5e8b59347cdf22f8a3ef9bb22b3a0da Mon Sep 17 00:00:00 2001 From: Kai Xu Date: Fri, 8 May 2026 18:53:08 +0000 Subject: [PATCH 4/4] fix: address PR #201 review feedback - Remove redundant local json import in cmd_emit, use module-level json - Add error handling for malformed --data JSON with friendly message - Only inject factory-default playbooks in generate_agent_content() (skip user-local ~/.factory/playbooks/) for deterministic sync output - Document why AgentMeta.model is retained despite not being in frontmatter - Add test for invalid JSON rejection Co-Authored-By: Claude Opus 4.6 (1M context) --- factory/agents/plugin.py | 15 ++++++++++----- factory/cli.py | 9 ++++++--- tests/test_events.py | 13 +++++++++++++ 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/factory/agents/plugin.py b/factory/agents/plugin.py index cdae0cb..838c140 100644 --- a/factory/agents/plugin.py +++ b/factory/agents/plugin.py @@ -8,7 +8,8 @@ import yaml -from factory.ace.injector import inject_playbook, load_playbook +from factory.ace.injector import inject_playbook +from factory.ace.paths import DEFAULTS_DIR as _PLAYBOOKS_DIR from factory.agents.runner import _PROMPTS_DIR _AGENTS_YML = Path(__file__).parent / "agents.yml" @@ -18,7 +19,7 @@ @dataclass(frozen=True) class AgentMeta: description: str - model: str + model: str # from agents.yml; not emitted in frontmatter (subagents inherit parent model) tools: list[str] @@ -53,9 +54,13 @@ def generate_agent_content(role: str) -> str: meta = config[role] prompt = (_PROMPTS_DIR / f"{role}.md").read_text() - playbook = load_playbook(role) - if playbook: - prompt = inject_playbook(prompt, playbook) + # Only inject factory-default playbooks (not user-local ~/.factory/playbooks/) + # so that sync_agents.py output is deterministic across machines. + playbook_path = _PLAYBOOKS_DIR / f"{role}.md" + if playbook_path.exists(): + playbook = playbook_path.read_text().strip() + if playbook: + prompt = inject_playbook(prompt, playbook) frontmatter = yaml.dump( {"name": role, "description": meta.description, "tools": meta.tools}, default_flow_style=False, diff --git a/factory/cli.py b/factory/cli.py index b2959c9..71b05c3 100644 --- a/factory/cli.py +++ b/factory/cli.py @@ -1110,10 +1110,13 @@ def cmd_emit(args: argparse.Namespace) -> int: from factory.events import emit_event project_path = Path(args.project).resolve() - data = {} + data: dict = {} if args.data: - import json as _json - data = _json.loads(args.data) + try: + data = json.loads(args.data) + except json.JSONDecodeError as e: + print(f"Error: --data is not valid JSON: {e}", file=sys.stderr) + return 1 emit_event(project_path, args.event_type, agent=args.agent, data=data) return 0 diff --git a/tests/test_events.py b/tests/test_events.py index d8833fd..93e6d3e 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -217,3 +217,16 @@ def test_cmd_emit_without_agent(self, tmp_path): assert event["type"] == "cycle.started" assert event["agent"] is None assert event["data"]["cycle"] == 1 + + def test_cmd_emit_rejects_invalid_json(self, tmp_path): + from factory.cli import cmd_emit + + (tmp_path / ".factory").mkdir() + args = Namespace( + event_type="agent.started", + agent="researcher", + project=str(tmp_path), + data="not valid json", + ) + rc = cmd_emit(args) + assert rc == 1