Skip to content

Commit d18e1a8

Browse files
fix: implement FORCE_GRAPH_REBUILD, which five documents already promised (#300)
Merge PR #300
1 parent 38e4572 commit d18e1a8

6 files changed

Lines changed: 144 additions & 15 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/).
2424

2525
### Fixed
2626

27+
- **`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.
28+
- **`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.
29+
- **`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.
30+
2731
- **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.
2832
- **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.
2933
- **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.

‎CLAUDE.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ Claude Workflow Engine is a 3-level LangGraph-based orchestration pipeline for a
2020
| **Status** | Active Development |
2121
| **Primary Location** | langgraph_engine/ |
2222
| **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/` |
23-
| **Total Python Files** | 248 (langgraph_engine/); 489 repo-wide |
24-
| **Test Files** | 101 (92 unit, 5 integration, 3 e2e, 1 load) |
23+
| **Total Python Files** | 248 (langgraph_engine/); 490 repo-wide |
24+
| **Test Files** | 102 (93 unit, 5 integration, 3 e2e, 1 load) |
2525
| **Call Graph** | 578 classes, 3,985 methods, 4 languages (Python/Java/TS/Kotlin) |
2626

2727
---
@@ -304,7 +304,7 @@ All integrations are configurable via environment variables (default: disabled):
304304
| `ENABLE_JENKINS` | `0` | Jenkins build validation (Step 5) |
305305
| `ENABLE_SONARQUBE` | `0` | SonarQube scan after implementation (Step 4) |
306306
| `ENABLE_FIGMA` | `0` | Figma design-to-code pipeline (Steps 4,5,6 -- extraction/injection now inside Step 1 template) |
307-
| `ENABLE_CI` | `false` | GitHub Actions CI pipeline |
307+
| `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` |
308308
| `ENABLE_HEALTH_SERVER` | `0` | Start HTTP /health + /readiness on HEALTH_PORT (8080) |
309309
| `ENABLE_METRICS` | `0` | Start Prometheus /metrics server on METRICS_PORT (9090) |
310310
| `ENABLE_TRACING` | `0` | Enable OpenTelemetry tracing (OTLP to OTEL_EXPORTER_OTLP_ENDPOINT) |

‎README.md‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
[![PyPI](https://img.shields.io/badge/PyPI-claude--workflow--engine-orange)](https://pypi.org/project/claude-workflow-engine/)
88
[![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)
99
[![License](https://img.shields.io/badge/License-MIT-yellow)](LICENSE)
10-
[![Tests](https://img.shields.io/badge/Tests-101%20files%20·%205%20integration-brightgreen)](tests/)
10+
[![Tests](https://img.shields.io/badge/Tests-102%20files%20·%205%20integration-brightgreen)](tests/)
1111
[![Discussions](https://img.shields.io/badge/Discussions-GitHub-blueviolet)](../../discussions)
1212

1313
---
@@ -461,8 +461,8 @@ claude-workflow-engine/ # 369 Python files total
461461
├── policies/ # non-.md policy state only (failure-kb.json) — see note below
462462
│ └── 03-execution-system/failure-prevention/failure-kb.json
463463
│
464-
├── tests/ # 101 test_*.py files (122 total Python files)
465-
│ ├── test_*.py # 92 unit tests
464+
├── tests/ # 102 test_*.py files (123 total Python files)
465+
│ ├── test_*.py # 93 unit tests
466466
│ ├── integration/ # 5 integration tests (GitHub, MCP, runtime verification)
467467
│ ├── e2e/ # 3 end-to-end scenario tests
468468
│ └── load/ # 1 concurrency / load test
@@ -620,11 +620,11 @@ ENABLE_HEALTH_SERVER=1 ENABLE_METRICS=1 LOG_FORMAT=json \
620620

621621
## Testing
622622

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

625625
| Category | Files | Notes |
626626
|---|:---:|---|
627-
| Unit tests | 92 | In `tests/test_*.py` — no external dependencies required |
627+
| Unit tests | 93 | In `tests/test_*.py` — no external dependencies required |
628628
| Integration tests | 5 | `tests/integration/` — require live GitHub token or MCP servers |
629629
| E2E tests | 3 | `tests/e2e/` — require full pipeline environment |
630630
| Load tests | 1 | `tests/load/` — enabled with `RUN_LOAD_TESTS=1` |

‎docs/guides/RUNBOOK_STALE_GRAPH.md‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,15 @@ signals `refresh_call_graph_if_stale()` to rebuild before serving data.
6868

6969
### Option A — Automatic (standard path)
7070

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

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

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

8081
### Option B — Force full rebuild
8182

‎langgraph_engine/sdlc_pipeline/call_graph_analyzer.py‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
"""
2121

2222
import logging
23+
import os
2324
from pathlib import Path
2425

2526
logger = logging.getLogger(__name__)
@@ -1389,14 +1390,30 @@ def refresh_call_graph_if_stale(state, project_root):
13891390
cached pre-implementation snapshot (e.g. step1_impact_analysis) should
13901391
call this helper instead of reading state directly.
13911392
1393+
FORCE_GRAPH_REBUILD=1 bypasses the flag and always rebuilds. That is the
1394+
mitigation ADR-002 records for its own stated risk: if call_graph_stale is
1395+
never set -- because the node that sets it crashed -- the guard cannot fire
1396+
and Step 5 reviews an outdated snapshot in silence. An operator recovering
1397+
from that has no in-state signal to correct, so the override has to come
1398+
from outside the state. RUNBOOK_STALE_GRAPH.md Option B is this variable.
1399+
1400+
It was documented in five files and implemented in none of them until
1401+
2026-08-07, so the runbook's recovery step did nothing when an operator ran
1402+
it. Only "1" enables it; any other value, including "0" and unset, leaves
1403+
the stale-flag behaviour untouched.
1404+
13921405
Args:
13931406
state: FlowState dict (or any dict with call_graph_stale key).
13941407
project_root: Project root directory (str or Path).
13951408
13961409
Returns:
1397-
dict - fresh snapshot from snapshot_call_graph() when stale,
1410+
dict - fresh snapshot from snapshot_call_graph() when forced or stale,
13981411
best available cached snapshot otherwise (never raises).
13991412
"""
1413+
if os.getenv("FORCE_GRAPH_REBUILD", "0") == "1":
1414+
logger.debug("refresh_call_graph_if_stale: FORCE_GRAPH_REBUILD=1, rebuilding graph")
1415+
return snapshot_call_graph(project_root)
1416+
14001417
if state.get("call_graph_stale"):
14011418
logger.debug("refresh_call_graph_if_stale: flag set, rebuilding graph")
14021419
return snapshot_call_graph(project_root)

‎tests/test_force_graph_rebuild.py‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
"""FORCE_GRAPH_REBUILD must actually bypass the stale flag.
2+
3+
The variable was documented in five files -- CLAUDE.md, ADR-002, the deployment
4+
guide, RUNBOOK_STALE_GRAPH.md and the troubleshooting guide -- and read by no
5+
code at all. RUNBOOK Option B instructs an operator to `export
6+
FORCE_GRAPH_REBUILD=1` while recovering from a stale-graph incident, and that
7+
command did nothing. A wrong default is confusing; a recovery procedure that
8+
silently fails is worse, because it is consulted precisely when something has
9+
already gone wrong.
10+
11+
The scenario ADR-002 names is the one that matters here, and it is the case the
12+
stale flag cannot cover: if `call_graph_stale` is never set -- because the node
13+
that sets it crashed -- the guard cannot fire, and Step 5 reviews an outdated
14+
snapshot in silence. There is no in-state signal for the operator to correct, so
15+
the override has to come from outside the state. Every test below therefore
16+
leaves `call_graph_stale` unset and supplies a usable cached snapshot: without
17+
the override, the cache is what the function returns.
18+
19+
Each assertion is paired with its opposite. "Forced rebuild returns fresh data"
20+
proves nothing on its own, because a function that always rebuilds satisfies it
21+
too -- so the unset and "0" cases assert that the cache is still honoured.
22+
"""
23+
24+
import sys
25+
from pathlib import Path
26+
from unittest.mock import patch
27+
28+
import pytest
29+
30+
_PROJECT_ROOT = Path(__file__).resolve().parent.parent
31+
if str(_PROJECT_ROOT) not in sys.path:
32+
sys.path.insert(0, str(_PROJECT_ROOT))
33+
34+
from langgraph_engine.sdlc_pipeline.call_graph_analyzer import refresh_call_graph_if_stale # noqa: E402
35+
36+
CACHED = {"nodes": ["cached_node"], "call_graph_available": True}
37+
FRESH = {"nodes": ["fresh_node"], "call_graph_available": True}
38+
39+
40+
@pytest.fixture()
41+
def not_stale_state():
42+
"""State with a usable cache and no stale flag -- ADR-002's crash scenario."""
43+
return {"step4_pre_change_graph": dict(CACHED)}
44+
45+
46+
def _run(state, tmp_path):
47+
"""Call the guard with snapshot_call_graph stubbed, returning (result, mock)."""
48+
with patch("langgraph_engine.sdlc_pipeline.call_graph_analyzer.snapshot_call_graph") as mock_snap:
49+
mock_snap.return_value = dict(FRESH)
50+
result = refresh_call_graph_if_stale(state, str(tmp_path))
51+
return result, mock_snap
52+
53+
54+
class TestTheOverrideBypassesTheStaleFlag:
55+
"""FORCE_GRAPH_REBUILD=1 rebuilds even when nothing marked the graph stale."""
56+
57+
def test_forced_rebuild_ignores_a_usable_cache(self, monkeypatch, not_stale_state, tmp_path):
58+
monkeypatch.setenv("FORCE_GRAPH_REBUILD", "1")
59+
result, mock_snap = _run(not_stale_state, tmp_path)
60+
mock_snap.assert_called_once_with(str(tmp_path))
61+
assert result["nodes"] == ["fresh_node"], "the cache was returned despite the override"
62+
63+
64+
class TestTheOverrideIsOffByDefault:
65+
"""The half that fails if the rebuild became unconditional."""
66+
67+
def test_unset_returns_the_cached_snapshot(self, monkeypatch, not_stale_state, tmp_path):
68+
monkeypatch.delenv("FORCE_GRAPH_REBUILD", raising=False)
69+
result, mock_snap = _run(not_stale_state, tmp_path)
70+
mock_snap.assert_not_called()
71+
assert result["nodes"] == ["cached_node"]
72+
73+
def test_zero_returns_the_cached_snapshot(self, monkeypatch, not_stale_state, tmp_path):
74+
"""Documented default is 0, so 0 must behave exactly like unset."""
75+
monkeypatch.setenv("FORCE_GRAPH_REBUILD", "0")
76+
result, mock_snap = _run(not_stale_state, tmp_path)
77+
mock_snap.assert_not_called()
78+
assert result["nodes"] == ["cached_node"]
79+
80+
def test_only_the_literal_1_enables_it(self, monkeypatch, not_stale_state, tmp_path):
81+
"""Matches the ENABLE_* convention used across the engine: == "1", not truthiness.
82+
83+
"true" is the value an operator is most likely to reach for by habit, so
84+
it is worth pinning that it does NOT enable the override rather than
85+
leaving the reader to infer it from the comparison.
86+
"""
87+
monkeypatch.setenv("FORCE_GRAPH_REBUILD", "true")
88+
result, mock_snap = _run(not_stale_state, tmp_path)
89+
mock_snap.assert_not_called()
90+
assert result["nodes"] == ["cached_node"]
91+
92+
93+
class TestTheOverrideDoesNotDisableTheStaleFlag:
94+
"""Setting it to 0 must not switch off ordinary stale-based rebuilding.
95+
96+
RUNBOOK Option A used to read as though `FORCE_GRAPH_REBUILD=0` turned that
97+
off and unsetting it turned it back on. It never did, and this pins that the
98+
two paths are independent so the corrected wording cannot silently drift
99+
back.
100+
"""
101+
102+
def test_stale_still_rebuilds_when_the_override_is_zero(self, monkeypatch, tmp_path):
103+
monkeypatch.setenv("FORCE_GRAPH_REBUILD", "0")
104+
state = {"call_graph_stale": True, "step4_pre_change_graph": dict(CACHED)}
105+
result, mock_snap = _run(state, tmp_path)
106+
mock_snap.assert_called_once_with(str(tmp_path))
107+
assert result["nodes"] == ["fresh_node"]

0 commit comments

Comments
 (0)