diff --git a/CLAUDE.md b/CLAUDE.md index c52c7b78..28cbb3c2 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/); 490 repo-wide | -| **Test Files** | 102 (93 unit, 5 integration, 3 e2e, 1 load) | +| **Total Python Files** | 248 (langgraph_engine/); 491 repo-wide | +| **Test Files** | 103 (94 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 981c5514..7eec65df 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ [![Python](https://img.shields.io/badge/Python-3.10%2B-green)](https://python.org) [![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-102%20files%20·%205%20integration-brightgreen)](tests/) +[![Tests](https://img.shields.io/badge/Tests-103%20files%20·%205%20integration-brightgreen)](tests/) [![Discussions](https://img.shields.io/badge/Discussions-GitHub-blueviolet)](../../discussions) --- @@ -106,20 +106,28 @@ yourself with `--invoked-by=`: ```bash # Hook Mode (default) — analysis, GitHub issue, branch -python scripts/3-level-flow.py --invoked-by=implement --message "Fix the login timeout bug" +python scripts/3-level-flow.py --invoked-by=implement --message="Fix the login timeout bug" # Full Mode — all 9 active steps end-to-end (implements + PR + closes issue) CLAUDE_HOOK_MODE=0 python scripts/3-level-flow.py --invoked-by=run-pipeline \ - --message "Fix the login timeout bug" + --message="Fix the login timeout bug" # Template Fast-Path — Step 0 detects the template and skips Step 1 planning python scripts/3-level-flow.py --invoked-by=run-pipeline \ - --message "Add document Q&A feature" \ + --message="Add document Q&A feature" \ --orchestration-template=orchestration_template.example.json ``` -Omitting `--invoked-by=` prints `[REFUSED]` and exits 0 without running anything. A -misspelled command name exits 2, so a typo costs an error rather than a silent no-op. +Both flags need the **equals form**. The parser matches on the `--message=` prefix, so a +space-separated `--message "..."` is silently ignored: the run proceeds with an empty +task, the orchestration prompt comes out empty, and Steps 2 and 3 skip while the run +still reports OK. + +`--invoked-by=` selects nothing — it only authorizes. Mode comes from +`CLAUDE_HOOK_MODE`, and the per-command step ranges listed above are the plugin +dispatcher's doing, not this flag's. Omitting it prints `[REFUSED]` and exits 0 without +running anything; a misspelled command name exits 2, so a typo costs an error rather +than a silent no-op. ### What happens when you run it @@ -175,7 +183,7 @@ flowchart TD ```mermaid flowchart TD - START(["Task Input\npython scripts/3-level-flow.py --task ..."]) + START(["Task Input\npython scripts/3-level-flow.py --invoked-by=... --message=..."]) subgraph LM1["Level 0 : Pre-Flight Sanity Guard"] direction LR @@ -335,11 +343,11 @@ flowchart LR ### Template Fast-Path -Pre-fill a JSON orchestration template once, and every subsequent run skips Step 0 entirely: +Pre-fill a JSON orchestration template once, and Step 0 detects it and skips Step 1 planning: ```bash -python scripts/3-level-flow.py \ - --message "add document Q&A feature" \ +python scripts/3-level-flow.py --invoked-by=run-pipeline \ + --message="add document Q&A feature" \ --orchestration-template=orchestration_template.example.json # Planning time: ~0s (Step 0 detects the template and skips Step 1) ``` @@ -461,8 +469,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/ # 102 test_*.py files (123 total Python files) -│ ├── test_*.py # 93 unit tests +├── tests/ # 103 test_*.py files (124 total Python files) +│ ├── test_*.py # 94 unit tests │ ├── integration/ # 5 integration tests (GitHub, MCP, runtime verification) │ ├── e2e/ # 3 end-to-end scenario tests │ └── load/ # 1 concurrency / load test @@ -593,7 +601,7 @@ Step 6: COMMENT "Implementation complete" with PR link ```bash docker build -t claude-workflow-engine . docker run --env-file .env claude-workflow-engine \ - python scripts/3-level-flow.py --message "your task" + python scripts/3-level-flow.py --invoked-by=run-pipeline --message="your task" ``` ### Docker Compose @@ -613,18 +621,18 @@ kubectl apply -f k8s/secret.yaml -f k8s/configmap.yaml \ ```bash ENABLE_HEALTH_SERVER=1 ENABLE_METRICS=1 LOG_FORMAT=json \ - python scripts/3-level-flow.py --message "your task" + python scripts/3-level-flow.py --invoked-by=run-pipeline --message="your task" ``` --- ## Testing -102 test_*.py files (123 total Python files in `tests/` including conftest and `__init__` files): +103 test_*.py files (124 total Python files in `tests/` including conftest and `__init__` files): | Category | Files | Notes | |---|:---:|---| -| Unit tests | 93 | In `tests/test_*.py` — no external dependencies required | +| Unit tests | 94 | 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/architecture/prompt_gen_expert_caller.py b/langgraph_engine/sdlc_pipeline/architecture/prompt_gen_expert_caller.py index d9bebf48..b3a1ffa5 100644 --- a/langgraph_engine/sdlc_pipeline/architecture/prompt_gen_expert_caller.py +++ b/langgraph_engine/sdlc_pipeline/architecture/prompt_gen_expert_caller.py @@ -171,6 +171,58 @@ def _load_template(): ) +def _render_sample(items, total=None, limit=10): + """Render up to ``limit`` items, stating how many were left out. + + The caller sends a sample rather than the full list, because the full lists + reach a million characters on a large repository and a command line cannot + carry that on Windows. The count travels with the sample so the grounding + header can say "10 of 940" instead of presenting ten as if they were all -- + a truncation the reader cannot see is worse than no truncation, because the + prompt then asserts something false about the codebase. + + Args: + items: The sample to render. + total: How many existed before sampling. None means unknown, in which + case the sample is assumed complete -- which is what an older caller + that sends no total implies. + limit: Maximum entries to render. + + Returns: + str: Comma-joined entries, suffixed with the omitted count, or "none". + """ + if not items: + return "none" + shown = list(items)[:limit] + rendered = ", ".join(_entry_label(entry) for entry in shown) + if total is not None and total > len(shown): + return "%s (showing %d of %d)" % (rendered, len(shown), total) + return rendered + + +def _entry_label(entry): + """Return the human-meaningful name for one call-graph entry. + + The analyser yields dicts -- ``fqn``, ``callers_count``, + ``callers_count_high_confidence``, and ``risk`` for affected methods -- not + strings. The previous code joined them directly, which raises + ``TypeError: sequence item 0: expected str instance, dict found``. That never + surfaced because the argument carrying them exceeded the Windows command-line + limit and the caller died before rendering; one bug stood in front of the + other. Falling back to str() rather than requiring the key, so a shape change + degrades to an ugly line instead of an exception in prompt assembly. + + Args: + entry: A call-graph entry, normally a dict carrying ``fqn``. + + Returns: + str: The entry's fully-qualified name, or its string form. + """ + if isinstance(entry, dict): + return str(entry.get("fqn") or entry.get("name") or entry) + return str(entry) + + def _render_kg_routing_block(kg_routing): """Render the KG ROUTING grounding block for the ``{kg_routing_block}`` placeholder. @@ -241,9 +293,9 @@ def _build_filled_prompt(template, args): affected_methods = call_graph.get("affected_methods", []) hot_nodes = call_graph.get("hot_nodes", []) - danger_zones_str = ", ".join(danger_zones) if danger_zones else "none" - affected_str = ", ".join(affected_methods[:10]) if affected_methods else "none" - hot_nodes_str = ", ".join(hot_nodes[:10]) if hot_nodes else "none" + danger_zones_str = _render_sample(danger_zones, call_graph.get("danger_zones_total")) + affected_str = _render_sample(affected_methods, call_graph.get("affected_methods_total")) + hot_nodes_str = _render_sample(hot_nodes, call_graph.get("hot_nodes_total")) runtime_block = json.dumps(runtime_context, indent=2, ensure_ascii=True) diff --git a/langgraph_engine/sdlc_pipeline/nodes/task_orchestration.py b/langgraph_engine/sdlc_pipeline/nodes/task_orchestration.py index d1d45876..a52c487c 100644 --- a/langgraph_engine/sdlc_pipeline/nodes/task_orchestration.py +++ b/langgraph_engine/sdlc_pipeline/nodes/task_orchestration.py @@ -161,11 +161,28 @@ def step1_task_analysis_node(state: FlowState) -> Dict[str, Any]: # NFR-2 / ADR-016 replace it: unbounded unless an operator configures a # silence interval, which measures no-progress rather than duration. _pg_silence = _env_optional_seconds("STEP1_PROMPT_GEN_SILENCE") + # Send a sample, not the whole graph. These lists go across a process + # boundary as a command-line argument, and on this repository they serialise + # to 1,031,711 characters -- 5,527 affected methods and 940 danger zones. + # Windows caps a command line at 32,767, so every Step 1 run on Windows died + # with WinError 206, fell back to the raw task, and left orchestration_prompt + # empty; Steps 2 and 3 then skipped issue and branch creation and the whole + # SDLC chain quietly did nothing while the run reported OK. + # + # Capping here rather than transporting it all and discarding it there. + # prompt_gen_expert_caller already rendered only affected_methods[:10], so + # that half of this is byte-identical to what it produced before. danger_zones + # it rendered in full, so the totals travel alongside the sample and the + # grounding header states how many were omitted -- a cap that hides its own + # size is how a prompt starts lying about the codebase it describes. + _CG_SAMPLE = 10 _call_graph_json = _json.dumps( { "risk_level": call_graph_risk_level, - "danger_zones": call_graph_danger_zones, - "affected_methods": call_graph_affected_methods, + "danger_zones": call_graph_danger_zones[:_CG_SAMPLE], + "danger_zones_total": len(call_graph_danger_zones), + "affected_methods": call_graph_affected_methods[:_CG_SAMPLE], + "affected_methods_total": len(call_graph_affected_methods), } ) _kg_routing_json = _json.dumps(kg_routing_result) diff --git a/tests/test_step1_callgraph_payload_size.py b/tests/test_step1_callgraph_payload_size.py new file mode 100644 index 00000000..ea41101f --- /dev/null +++ b/tests/test_step1_callgraph_payload_size.py @@ -0,0 +1,146 @@ +"""Step 1's call-graph argument must fit in a Windows command line. + +The payload is passed to prompt_gen_expert_caller as a command-line argument. On +this repository the ungated lists serialise to 1,031,711 characters -- 5,527 +affected methods and 940 danger zones -- against a Windows CreateProcess limit of +32,767. Every Step 1 run on Windows therefore died with WinError 206, fell back +to the raw task, and left orchestration_prompt empty. Steps 2 and 3 then skipped +issue and branch creation, and the run reported OK having done nothing. + +That failure was silent for a month. It is the reason these tests assert on the +SIZE of what is sent rather than only on the content: a correctness test would +have passed throughout, because the values were right and merely undeliverable. + +The receiver already truncated affected_methods to ten, so capping the sender +changes nothing about the rendered prompt for that list. danger_zones it rendered +in full, so the totals are sent alongside the sample and _render_sample states +what was omitted -- a truncation the reader cannot see is worse than none, since +the prompt then asserts something false about the codebase. +""" + +import json +import sys +from pathlib import Path + +_PROJECT_ROOT = Path(__file__).resolve().parent.parent +if str(_PROJECT_ROOT) not in sys.path: + sys.path.insert(0, str(_PROJECT_ROOT)) + +from langgraph_engine.sdlc_pipeline.architecture.prompt_gen_expert_caller import _render_sample # noqa: E402 + +WINDOWS_CMDLINE_LIMIT = 32767 + +# Shaped like the real thing on this repository, big enough that an uncapped +# payload cannot fit. Built rather than measured so the test does not depend on +# how large the repository happens to be on the day it runs. +MANY_METHODS = ["langgraph_engine.module_%d.Class%d.method_%d" % (i, i, i) for i in range(5527)] +MANY_ZONES = ["langgraph_engine/danger/zone_%d.py" % i for i in range(940)] + + +def _build_payload(danger_zones, affected_methods, sample=10): + """Build the argument exactly as step1_task_analysis_node builds it.""" + return json.dumps( + { + "risk_level": "high", + "danger_zones": danger_zones[:sample], + "danger_zones_total": len(danger_zones), + "affected_methods": affected_methods[:sample], + "affected_methods_total": len(affected_methods), + } + ) + + +class TestThePayloadFitsInACommandLine: + """The size property the outage was caused by.""" + + def test_a_large_graph_still_fits(self): + payload = _build_payload(MANY_ZONES, MANY_METHODS) + assert len(payload) < WINDOWS_CMDLINE_LIMIT, "payload is %d chars, over the %d Windows limit" % ( + len(payload), + WINDOWS_CMDLINE_LIMIT, + ) + + def test_the_uncapped_payload_would_not_fit(self): + """The half that proves the cap is what makes the difference. + + Without this, the test above passes on any repository small enough to fit + anyway, and would have passed throughout the outage. + """ + uncapped = json.dumps({"risk_level": "high", "danger_zones": MANY_ZONES, "affected_methods": MANY_METHODS}) + assert len(uncapped) > WINDOWS_CMDLINE_LIMIT + + def test_the_totals_survive_the_cap(self): + """A sample without its total cannot be reported honestly downstream.""" + payload = json.loads(_build_payload(MANY_ZONES, MANY_METHODS)) + assert payload["danger_zones_total"] == 940 + assert payload["affected_methods_total"] == 5527 + assert len(payload["danger_zones"]) == 10 + assert len(payload["affected_methods"]) == 10 + + +class TestTheRenderedSampleDeclaresWhatItOmitted: + """A cap the reader cannot see makes the prompt assert something false.""" + + def test_a_truncated_list_states_the_true_total(self): + rendered = _render_sample(MANY_ZONES[:10], total=940) + assert "showing 10 of 940" in rendered + + def test_a_complete_list_carries_no_suffix(self): + """The half that fails if the suffix were unconditional.""" + rendered = _render_sample(["a", "b"], total=2) + assert rendered == "a, b" + assert "showing" not in rendered + + def test_an_absent_total_is_treated_as_complete(self): + """An older caller sends no total; it must not be reported as truncated.""" + rendered = _render_sample(["a", "b"], total=None) + assert rendered == "a, b" + assert "showing" not in rendered + + def test_an_empty_list_renders_as_none(self): + assert _render_sample([], total=0) == "none" + assert _render_sample(None, total=None) == "none" + + +class TestEntriesAreDictsNotStrings: + """The second bug, which the first one hid. + + The analyser yields dicts carrying fqn / callers_count / risk. The previous + code joined them directly, which raises TypeError on a dict. It never + surfaced because the argument carrying them blew the Windows command-line + limit and the caller died before reaching the join -- so fixing only the size + would have swapped a WinError for a TypeError. + """ + + def test_a_dict_entry_renders_its_fqn(self): + entries = [{"fqn": "pkg.mod.Class.method", "callers_count": 42, "risk": "high"}] + assert _render_sample(entries, total=1) == "pkg.mod.Class.method" + + def test_the_old_join_would_have_raised_on_these(self): + """Pins why this is a separate defect rather than a style preference.""" + entries = [{"fqn": "pkg.mod.Class.method", "callers_count": 42}] + try: + ", ".join(entries) + except TypeError: + return + raise AssertionError("joining dicts must raise TypeError, or this test proves nothing") + + def test_a_plain_string_entry_still_works(self): + """Older callers, and hot_nodes, may still send strings.""" + assert _render_sample(["a.b.c"], total=1) == "a.b.c" + + def test_an_unexpected_shape_degrades_instead_of_raising(self): + """Prompt assembly must not die because an entry grew a new shape.""" + assert _render_sample([{"unexpected": 1}], total=1) != "" + + +class TestTheNodeSendsTheCappedShape: + """The cap must live in the node, not only in this test's helper.""" + + def test_the_node_source_caps_both_lists_and_sends_totals(self): + source = (_PROJECT_ROOT / "langgraph_engine" / "sdlc_pipeline" / "nodes" / "task_orchestration.py").read_text( + encoding="utf-8" + ) + assert "danger_zones_total" in source, "the node must send the danger_zones total" + assert "affected_methods_total" in source, "the node must send the affected_methods total" + assert "_CG_SAMPLE" in source, "the node must cap the lists before serialising"