Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/).

### Fixed

- **`FORCE_GRAPH_REBUILD` now exists in code, not only in documentation.** The variable was described in five files -- `CLAUDE.md`, `ADR-002`, the deployment guide, `RUNBOOK_STALE_GRAPH.md` and the troubleshooting guide -- and read by nothing, since `a43595b` (2026-03-28). RUNBOOK Option B tells an operator to `export FORCE_GRAPH_REBUILD=1` while recovering from a stale-graph incident, and that command did nothing. It is now honoured by `refresh_call_graph_if_stale()`, which rebuilds unconditionally when it is `1`. This matters for the exact case ADR-002 names as its own risk and the stale flag cannot cover: when `call_graph_stale` is never set because the node that sets it crashed, the guard cannot fire and Step 5 reviews an outdated snapshot in silence — with no in-state signal for an operator to correct, so the override must come from outside the state. Implementing it made the ADR, the deployment guide and the troubleshooting guide true as written; none needed editing.
- **`RUNBOOK_STALE_GRAPH.md` Option A described behaviour the flag never had.** It implied that `FORCE_GRAPH_REBUILD=0` disabled stale-based rebuilding and that unsetting it re-enabled it. Stale rebuilding is driven by the `call_graph_stale` state flag and no environment variable; the override only ever *adds* a rebuild. Corrected, and pinned by a test so the wording cannot drift back.
- **`CLAUDE.md` documented the wrong default for `ENABLE_CI`.** The table said `false`; `ci.yml` reads `vars.ENABLE_CI` and falls back to `true`, so CI runs unless explicitly disabled. The row now also records that it is a repository variable rather than a process environment variable, which is why it behaves unlike every other flag in that table.

- **A dormant runtime-contract bug that predated this work** (fb7bbaa, 6982930). The `step0 -> step8` transition guard declared `orchestrator_result` as a `str` of at least 50 characters; the node has always written a `dict`, and `node_contracts.py` described it correctly as a dict -- so the two disagreed with each other and one disagreed with reality. It never fired only because `ENABLE_RUNTIME_VERIFICATION` defaults to `0`, which means the first operator to enable verification would have hit a failure that had nothing to do with their change. `verify_orchestrator_result()` carried the same `str` premise and is removed above.
- **A guard test that was pinning the bug rather than the behaviour** (6982930). `test_guard_step1_to_step2_pass` fed the guard a 60-character string -- exactly what the wrong spec asked for -- so it would have stayed green no matter how wrong the spec became. A positive test alone cannot detect a wrong type, because it builds its input from the same mistaken premise. Paired with a negative case asserting that a `str` `orchestrator_result` is reported as a violation, verified capable of failing by reverting the spec and watching it go red.
- **Step 1 Phase 2 no longer claims success it did not have** (e0d9372). `call_execution_script` reports failure as a status dict rather than raising, so the `try/except` meant to catch a failing decomposer could not fire and its warning never logged. Every script-level failure fell through to an empty TODO list with no diagnostic, after which the result was written with a hardcoded `"success": True` -- a decomposer that never ran was indistinguishable from a task with nothing to do.
Expand Down
6 changes: 3 additions & 3 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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/); 489 repo-wide |
| **Test Files** | 101 (92 unit, 5 integration, 3 e2e, 1 load) |
| **Total Python Files** | 248 (langgraph_engine/); 490 repo-wide |
| **Test Files** | 102 (93 unit, 5 integration, 3 e2e, 1 load) |
| **Call Graph** | 578 classes, 3,985 methods, 4 languages (Python/Java/TS/Kotlin) |

---
Expand Down Expand Up @@ -304,7 +304,7 @@ All integrations are configurable via environment variables (default: disabled):
| `ENABLE_JENKINS` | `0` | Jenkins build validation (Step 5) |
| `ENABLE_SONARQUBE` | `0` | SonarQube scan after implementation (Step 4) |
| `ENABLE_FIGMA` | `0` | Figma design-to-code pipeline (Steps 4,5,6 -- extraction/injection now inside Step 1 template) |
| `ENABLE_CI` | `false` | GitHub Actions CI pipeline |
| `ENABLE_CI` | `true` | GitHub Actions CI pipeline. A **repo variable**, not a process env var: `ci.yml` reads `vars.ENABLE_CI` and falls back to `true`, so CI runs unless it is explicitly set to `false` |
| `ENABLE_HEALTH_SERVER` | `0` | Start HTTP /health + /readiness on HEALTH_PORT (8080) |
| `ENABLE_METRICS` | `0` | Start Prometheus /metrics server on METRICS_PORT (9090) |
| `ENABLE_TRACING` | `0` | Enable OpenTelemetry tracing (OTLP to OTEL_EXPORTER_OTLP_ENDPOINT) |
Expand Down
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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-101%20files%20·%205%20integration-brightgreen)](tests/)
[![Tests](https://img.shields.io/badge/Tests-102%20files%20·%205%20integration-brightgreen)](tests/)
[![Discussions](https://img.shields.io/badge/Discussions-GitHub-blueviolet)](../../discussions)

---
Expand Down Expand Up @@ -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/ # 101 test_*.py files (122 total Python files)
│ ├── test_*.py # 92 unit tests
├── tests/ # 102 test_*.py files (123 total Python files)
│ ├── test_*.py # 93 unit tests
│ ├── integration/ # 5 integration tests (GitHub, MCP, runtime verification)
│ ├── e2e/ # 3 end-to-end scenario tests
│ └── load/ # 1 concurrency / load test
Expand Down Expand Up @@ -620,11 +620,11 @@ ENABLE_HEALTH_SERVER=1 ENABLE_METRICS=1 LOG_FORMAT=json \

## Testing

101 test_*.py files (122 total Python files in `tests/` including conftest and `__init__` files):
102 test_*.py files (123 total Python files in `tests/` including conftest and `__init__` files):

| Category | Files | Notes |
|---|:---:|---|
| Unit tests | 92 | In `tests/test_*.py` — no external dependencies required |
| Unit tests | 93 | 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` |
Expand Down
13 changes: 7 additions & 6 deletions docs/guides/RUNBOOK_STALE_GRAPH.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,15 @@ signals `refresh_call_graph_if_stale()` to rebuild before serving data.

### Option A — Automatic (standard path)

Ensure `FORCE_GRAPH_REBUILD` is not set to `0` (it defaults to rebuilding on
stale). If someone disabled it for debugging, re-enable:
Rebuilding on a stale graph is unconditional: it is driven by the
`call_graph_stale` state flag, not by any environment variable, so there is
nothing to re-enable. `FORCE_GRAPH_REBUILD` only ever *adds* a rebuild — setting
it to `0` or leaving it unset does not switch the stale path off.

```bash
unset FORCE_GRAPH_REBUILD
```
Re-run the pipeline from Step 10 onward and let the flag do its work.

Re-run the pipeline from Step 10 onward.
If that does not help, the flag itself is probably not being set — which is the
case Option B exists for.

### Option B — Force full rebuild

Expand Down
19 changes: 18 additions & 1 deletion langgraph_engine/sdlc_pipeline/call_graph_analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"""

import logging
import os
from pathlib import Path

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -1389,14 +1390,30 @@ def refresh_call_graph_if_stale(state, project_root):
cached pre-implementation snapshot (e.g. step1_impact_analysis) should
call this helper instead of reading state directly.

FORCE_GRAPH_REBUILD=1 bypasses the flag and always rebuilds. That is the
mitigation ADR-002 records for its own stated risk: if call_graph_stale is
never set -- because the node that sets it crashed -- the guard cannot fire
and Step 5 reviews an outdated snapshot in silence. An operator recovering
from that has no in-state signal to correct, so the override has to come
from outside the state. RUNBOOK_STALE_GRAPH.md Option B is this variable.

It was documented in five files and implemented in none of them until
2026-08-07, so the runbook's recovery step did nothing when an operator ran
it. Only "1" enables it; any other value, including "0" and unset, leaves
the stale-flag behaviour untouched.

Args:
state: FlowState dict (or any dict with call_graph_stale key).
project_root: Project root directory (str or Path).

Returns:
dict - fresh snapshot from snapshot_call_graph() when stale,
dict - fresh snapshot from snapshot_call_graph() when forced or stale,
best available cached snapshot otherwise (never raises).
"""
if os.getenv("FORCE_GRAPH_REBUILD", "0") == "1":
logger.debug("refresh_call_graph_if_stale: FORCE_GRAPH_REBUILD=1, rebuilding graph")
return snapshot_call_graph(project_root)

if state.get("call_graph_stale"):
logger.debug("refresh_call_graph_if_stale: flag set, rebuilding graph")
return snapshot_call_graph(project_root)
Expand Down
107 changes: 107 additions & 0 deletions tests/test_force_graph_rebuild.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
"""FORCE_GRAPH_REBUILD must actually bypass the stale flag.

The variable was documented in five files -- CLAUDE.md, ADR-002, the deployment
guide, RUNBOOK_STALE_GRAPH.md and the troubleshooting guide -- and read by no
code at all. RUNBOOK Option B instructs an operator to `export
FORCE_GRAPH_REBUILD=1` while recovering from a stale-graph incident, and that
command did nothing. A wrong default is confusing; a recovery procedure that
silently fails is worse, because it is consulted precisely when something has
already gone wrong.

The scenario ADR-002 names is the one that matters here, and it is the case the
stale flag cannot cover: if `call_graph_stale` is never set -- because the node
that sets it crashed -- the guard cannot fire, and Step 5 reviews an outdated
snapshot in silence. There is no in-state signal for the operator to correct, so
the override has to come from outside the state. Every test below therefore
leaves `call_graph_stale` unset and supplies a usable cached snapshot: without
the override, the cache is what the function returns.

Each assertion is paired with its opposite. "Forced rebuild returns fresh data"
proves nothing on its own, because a function that always rebuilds satisfies it
too -- so the unset and "0" cases assert that the cache is still honoured.
"""

import sys
from pathlib import Path
from unittest.mock import patch

import pytest

_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.call_graph_analyzer import refresh_call_graph_if_stale # noqa: E402

CACHED = {"nodes": ["cached_node"], "call_graph_available": True}
FRESH = {"nodes": ["fresh_node"], "call_graph_available": True}


@pytest.fixture()
def not_stale_state():
"""State with a usable cache and no stale flag -- ADR-002's crash scenario."""
return {"step4_pre_change_graph": dict(CACHED)}


def _run(state, tmp_path):
"""Call the guard with snapshot_call_graph stubbed, returning (result, mock)."""
with patch("langgraph_engine.sdlc_pipeline.call_graph_analyzer.snapshot_call_graph") as mock_snap:
mock_snap.return_value = dict(FRESH)
result = refresh_call_graph_if_stale(state, str(tmp_path))
return result, mock_snap


class TestTheOverrideBypassesTheStaleFlag:
"""FORCE_GRAPH_REBUILD=1 rebuilds even when nothing marked the graph stale."""

def test_forced_rebuild_ignores_a_usable_cache(self, monkeypatch, not_stale_state, tmp_path):
monkeypatch.setenv("FORCE_GRAPH_REBUILD", "1")
result, mock_snap = _run(not_stale_state, tmp_path)
mock_snap.assert_called_once_with(str(tmp_path))
assert result["nodes"] == ["fresh_node"], "the cache was returned despite the override"


class TestTheOverrideIsOffByDefault:
"""The half that fails if the rebuild became unconditional."""

def test_unset_returns_the_cached_snapshot(self, monkeypatch, not_stale_state, tmp_path):
monkeypatch.delenv("FORCE_GRAPH_REBUILD", raising=False)
result, mock_snap = _run(not_stale_state, tmp_path)
mock_snap.assert_not_called()
assert result["nodes"] == ["cached_node"]

def test_zero_returns_the_cached_snapshot(self, monkeypatch, not_stale_state, tmp_path):
"""Documented default is 0, so 0 must behave exactly like unset."""
monkeypatch.setenv("FORCE_GRAPH_REBUILD", "0")
result, mock_snap = _run(not_stale_state, tmp_path)
mock_snap.assert_not_called()
assert result["nodes"] == ["cached_node"]

def test_only_the_literal_1_enables_it(self, monkeypatch, not_stale_state, tmp_path):
"""Matches the ENABLE_* convention used across the engine: == "1", not truthiness.

"true" is the value an operator is most likely to reach for by habit, so
it is worth pinning that it does NOT enable the override rather than
leaving the reader to infer it from the comparison.
"""
monkeypatch.setenv("FORCE_GRAPH_REBUILD", "true")
result, mock_snap = _run(not_stale_state, tmp_path)
mock_snap.assert_not_called()
assert result["nodes"] == ["cached_node"]


class TestTheOverrideDoesNotDisableTheStaleFlag:
"""Setting it to 0 must not switch off ordinary stale-based rebuilding.

RUNBOOK Option A used to read as though `FORCE_GRAPH_REBUILD=0` turned that
off and unsetting it turned it back on. It never did, and this pins that the
two paths are independent so the corrected wording cannot silently drift
back.
"""

def test_stale_still_rebuilds_when_the_override_is_zero(self, monkeypatch, tmp_path):
monkeypatch.setenv("FORCE_GRAPH_REBUILD", "0")
state = {"call_graph_stale": True, "step4_pre_change_graph": dict(CACHED)}
result, mock_snap = _run(state, tmp_path)
mock_snap.assert_called_once_with(str(tmp_path))
assert result["nodes"] == ["fresh_node"]
Loading