diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d20112f..ad939a3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Added + +- **Step 1 now verifies the orchestration prompt it emits.** `verify_orchestration_prompt()` existed, was exported and had four unit tests, and was called by nothing; it is now wired in at the point the prompt is stored, and its findings are recorded on `orchestrator_result["prompt_warnings"]` and logged. The gap it closes is narrow and specific: both degraded paths in the node already log (an `ERROR` from prompt-gen, and an empty response), so this is not for those. It is for a response that is **non-empty and therefore silent** but not a usable prompt — a truncated template, the wrong file, a stub — which until now was indistinguishable from a good one. Warnings are never fatal: `STEP1_CONTRACT` documents the short raw-task fallback as a legitimate degraded path, and it trips both checks by design, so failing here would convert a recoverable run into a dead one. + ### Removed - **`langgraph_engine/helper_nodes/` is deleted** (5 modules, 538 lines). All nine of its public node functions were also defined in `orchestrator.py`, which is the module the graph actually wires, and nothing imported the package -- it was an extraction that was started and never completed. Two documentation corrections existed **only** in the orphaned copy and were ported to `orchestrator.py` before the deletion rather than discarded with it: `save_workflow_memory`'s comment named `~/.claude/memory/sessions/` while its code writes to `memory/logs/sessions/`, and `optimize_context_after_level1` still said "Level 3" in three places, a level that stopped existing in the `06b0463` rename. The live module had the stale text in both cases. diff --git a/CLAUDE.md b/CLAUDE.md index 37a1b00b..79a2f49f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -20,8 +20,8 @@ Claude Workflow Engine is a 3-level LangGraph-based orchestration pipeline for a | **Status** | Active Development | | **Primary Location** | langgraph_engine/ | | **MCP Servers** | 13 servers -- all in separate repos under [techdeveloper-org](https://github.com/orgs/techdeveloper-org/repositories); 1 also keeps an in-engine copy in `src/mcp/` | -| **Total Python Files** | 248 (langgraph_engine/); 488 repo-wide | -| **Test Files** | 100 (91 unit, 5 integration, 3 e2e, 1 load) | +| **Total Python Files** | 248 (langgraph_engine/); 489 repo-wide | +| **Test Files** | 101 (92 unit, 5 integration, 3 e2e, 1 load) | | **Call Graph** | 578 classes, 3,985 methods, 4 languages (Python/Java/TS/Kotlin) | --- diff --git a/README.md b/README.md index 51469b91..767e3d20 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ [![PyPI](https://img.shields.io/badge/PyPI-claude--workflow--engine-orange)](https://pypi.org/project/claude-workflow-engine/) [![CI](https://github.com/techdeveloper-org/claude-workflow-engine/actions/workflows/ci.yml/badge.svg)](https://github.com/techdeveloper-org/claude-workflow-engine/actions/workflows/ci.yml) [![License](https://img.shields.io/badge/License-MIT-yellow)](LICENSE) -[![Tests](https://img.shields.io/badge/Tests-100%20files%20·%205%20integration-brightgreen)](tests/) +[![Tests](https://img.shields.io/badge/Tests-101%20files%20·%205%20integration-brightgreen)](tests/) [![Discussions](https://img.shields.io/badge/Discussions-GitHub-blueviolet)](../../discussions) --- @@ -461,8 +461,8 @@ claude-workflow-engine/ # 369 Python files total ├── policies/ # non-.md policy state only (failure-kb.json) — see note below │ └── 03-execution-system/failure-prevention/failure-kb.json │ -├── tests/ # 100 test_*.py files (121 total Python files) -│ ├── test_*.py # 91 unit tests +├── tests/ # 101 test_*.py files (122 total Python files) +│ ├── test_*.py # 92 unit tests │ ├── integration/ # 5 integration tests (GitHub, MCP, runtime verification) │ ├── e2e/ # 3 end-to-end scenario tests │ └── load/ # 1 concurrency / load test @@ -620,11 +620,11 @@ ENABLE_HEALTH_SERVER=1 ENABLE_METRICS=1 LOG_FORMAT=json \ ## Testing -100 test_*.py files (121 total Python files in `tests/` including conftest and `__init__` files): +101 test_*.py files (122 total Python files in `tests/` including conftest and `__init__` files): | Category | Files | Notes | |---|:---:|---| -| Unit tests | 91 | In `tests/test_*.py` — no external dependencies required | +| Unit tests | 92 | In `tests/test_*.py` — no external dependencies required | | Integration tests | 5 | `tests/integration/` — require live GitHub token or MCP servers | | E2E tests | 3 | `tests/e2e/` — require full pipeline environment | | Load tests | 1 | `tests/load/` — enabled with `RUN_LOAD_TESTS=1` | diff --git a/langgraph_engine/sdlc_pipeline/nodes/task_orchestration.py b/langgraph_engine/sdlc_pipeline/nodes/task_orchestration.py index b0cea808..d1d45876 100644 --- a/langgraph_engine/sdlc_pipeline/nodes/task_orchestration.py +++ b/langgraph_engine/sdlc_pipeline/nodes/task_orchestration.py @@ -236,13 +236,34 @@ def step1_task_analysis_node(state: FlowState) -> Dict[str, Any]: "--- BEGIN ORCHESTRATION TEMPLATE ---\n\n" + orchestration_prompt + "\n\n--- END ORCHESTRATION TEMPLATE ---" ) + # Structural check on what is about to be emitted. Both degraded paths above + # already log -- an ERROR from prompt_gen, or an empty response -- so this is + # not here to catch those. It is here for the case neither covers: a response + # that is non-empty and therefore silent, but not a usable orchestration + # prompt (a truncated template, the wrong file, a stub). Until now that was + # indistinguishable from a good prompt. + # + # Warnings are recorded and logged, never fatal. The raw-task fallback is a + # legitimate degraded path -- STEP1_CONTRACT documents it as such -- and it + # trips both checks by design, so failing here would turn a recoverable run + # into a dead one. + from ...runtime_verification.schema_verifier import verify_orchestration_prompt # noqa: PLC0415 + + prompt_warnings = verify_orchestration_prompt(orchestration_prompt) + orch_result: Dict[str, Any] = { "mode": "emitted", "success": True, "prompt_chars": len(_full_prompt), "template_source": "claude-global-library/ORCHESTRATION_TEMPLATE.md", "library_version": _library_version(), + "prompt_warnings": prompt_warnings, } + if prompt_warnings: + logger.warning( + "[v2] Step 1 emitted a structurally questionable orchestration prompt: {}", + "; ".join(prompt_warnings), + ) logger.info( "[v2] Step 1 emitted orchestration prompt: {} chars, library v{}", orch_result["prompt_chars"], diff --git a/tests/test_orchestration_prompt_verification.py b/tests/test_orchestration_prompt_verification.py new file mode 100644 index 00000000..f4319329 --- /dev/null +++ b/tests/test_orchestration_prompt_verification.py @@ -0,0 +1,104 @@ +"""Step 1 must actually run verify_orchestration_prompt on what it emits. + +The function was written, exported and unit-tested, and then called by nothing. +Its sibling verify_orchestrator_result was deleted for that reason plus a false +premise; this one encodes a true contract, so it was wired in instead. + +A wire-in needs a test that would notice the wire coming loose, otherwise it +recreates the original problem in a new place -- a validator that exists, passes +its own unit tests, and never sees production data. So every assertion here goes +through step1_task_analysis_node rather than calling the verifier directly: +tests/test_schema_verifier.py already covers the function in isolation, and that +coverage is exactly what failed to notice it had no callers. + +The two cases are a pair on purpose. A test that only asserts "a good prompt +produces no warnings" passes just as well when the verifier is never invoked, +because an unwired check and a satisfied check both produce an empty list. +""" + +import sys +from pathlib import Path +from unittest.mock import patch + +_PROJECT_ROOT = Path(__file__).resolve().parent.parent +if str(_PROJECT_ROOT) not in sys.path: + sys.path.insert(0, str(_PROJECT_ROOT)) + +# Long enough to clear the 200-char floor and carrying the "Phase" keyword the +# verifier looks for, so it stands in for a real master template. +WELL_FORMED_PROMPT = "Phase A -- foundation. " + ("orchestration detail. " * 40) + +# Non-empty, so neither degraded path in the node fires and nothing else logs: +# this is the case the wire-in exists for. +MALFORMED_PROMPT = "do the thing" + + +def _script_mock(prompt_text): + """Return a call_execution_script stand-in yielding the given prompt.""" + + def _side_effect(script_name, args=None, model_tier=None, silence_interval=None): + if script_name == "prompt_gen_expert_caller": + return {"status": "SUCCESS", "llm_response": prompt_text, "prompt": "raw"} + return {"status": "SUCCESS"} + + return _side_effect + + +def _run_node(prompt_text): + """Drive step1_task_analysis_node with a mocked prompt-gen response.""" + from langgraph_engine.sdlc_pipeline.nodes.task_orchestration import step1_task_analysis_node + + state = {"user_message": "Add a new endpoint", "project_root": str(_PROJECT_ROOT)} + with patch( + "langgraph_engine.sdlc_pipeline.helpers.call_execution_script", + side_effect=_script_mock(prompt_text), + ): + return step1_task_analysis_node(state) + + +class TestTheVerifierRunsOnWhatIsEmitted: + """The check must reach the emitted prompt, not merely exist.""" + + def test_a_well_formed_prompt_records_no_warnings(self): + result = _run_node(WELL_FORMED_PROMPT) + orch = result["orchestrator_result"] + assert "prompt_warnings" in orch, "the verifier result must reach orchestrator_result" + assert orch["prompt_warnings"] == [] + + def test_a_malformed_prompt_records_warnings(self): + """The half that fails if the verifier is never called. + + A non-empty but unusable prompt is the case neither degraded path in the + node covers -- prompt_gen returned something, so nothing else logs. If + this list came back empty the check would be unwired, and the pass case + above could not tell the difference. + """ + result = _run_node(MALFORMED_PROMPT) + warnings = result["orchestrator_result"]["prompt_warnings"] + assert warnings, "a 12-character prompt with no 'Phase' keyword must be reported" + joined = " ".join(warnings).lower() + assert "short" in joined or "200" in joined + assert "phase" in joined + + +class TestWarningsAreNotFatal: + """A questionable prompt is reported, not turned into a failed run.""" + + def test_the_step_still_emits_and_reports_success(self): + """The raw-task fallback trips both checks by design. + + STEP1_CONTRACT documents a short raw task as a legitimate degraded path, + so a verifier that failed the step would convert every recoverable run + into a dead one. + """ + result = _run_node(MALFORMED_PROMPT) + orch = result["orchestrator_result"] + assert orch["prompt_warnings"] + assert orch["success"] is True + assert orch["mode"] == "emitted" + assert result["orchestration_prompt"] == MALFORMED_PROMPT + + def test_prompt_chars_still_counts_the_full_emitted_prompt(self): + """Guards against the wire-in being placed where it truncates the result.""" + result = _run_node(WELL_FORMED_PROMPT) + assert result["orchestrator_result"]["prompt_chars"] > len(WELL_FORMED_PROMPT)