diff --git a/src/llm_augmented_workflows/engine.py b/src/llm_augmented_workflows/engine.py index 29bb6eb..1f3284c 100644 --- a/src/llm_augmented_workflows/engine.py +++ b/src/llm_augmented_workflows/engine.py @@ -38,9 +38,7 @@ def parse_execution(raw: Any) -> str: if raw is None: return DEFAULT_EXECUTION if not isinstance(raw, str): - raise ConfigError( - f"execution must be a string in {EXECUTION_MODES}, got {raw!r}" - ) + raise ConfigError(f"execution must be a string in {EXECUTION_MODES}, got {raw!r}") value = raw.strip().lower() if value not in EXECUTION_MODES: raise ConfigError(f"execution must be one of {EXECUTION_MODES}, got {raw!r}") diff --git a/src/llm_augmented_workflows/route.py b/src/llm_augmented_workflows/route.py index 29682ba..511c703 100644 --- a/src/llm_augmented_workflows/route.py +++ b/src/llm_augmented_workflows/route.py @@ -19,6 +19,7 @@ from .engine import ( ConfigError, + When, flatten_rules, load_flows, matches, @@ -37,6 +38,23 @@ def _write_output(name: str, value: str) -> None: print(f"{name}={value}") +def _is_label_stale(when: When, event_name: str, payload: dict) -> bool: + """Return True if a labeled event's trigger label is no longer on the issue. + + This prevents stale queued dispatch runs (issue #20) from matching rules + whose trigger label was already consumed by continuous mode. Event-driven + mode and legitimately re-added labels are unaffected because the label + genuinely remains on the issue. + """ + if when.label is None: + return False + if event_name != "issues" or payload.get("action") != "labeled": + return False + issue_labels = (payload.get("issue") or {}).get("labels") or [] + live_labels = {lb["name"] for lb in issue_labels if isinstance(lb, dict)} + return when.label not in live_labels + + def _load_payload() -> dict: path = os.environ.get("GITHUB_EVENT_PATH") if not path or not Path(path).exists(): @@ -66,7 +84,12 @@ def main() -> int: rules = [r for r in all_rules if r.id == force_id] if force_id else [] else: payload = _load_payload() - rules = [r for r in all_rules if matches(r.when, event_name, payload)] + rules = [ + r + for r in all_rules + if matches(r.when, event_name, payload) + and not _is_label_stale(r.when, event_name, payload) + ] matrix = [rule_to_matrix(r) for r in rules] diff --git a/tests/test_engine.py b/tests/test_engine.py index 188d925..5c6f98f 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -206,6 +206,28 @@ def test_matches_unspecified_fields_are_wildcards(): assert matches(when, "issues", {"action": "opened"}) +def test_matches_labeled_event_uses_payload_not_live_labels(): + """matches() keys on the event payload label, not the issue's live labels. + + This is the root cause of the redundant dispatch cascade described in + issue #20: a stale queued dispatch run whose trigger label was already + consumed by continuous mode still matches because the payload still + carries the original label name. matches() has no access to live label + state. + """ + when = parse_when( + {"event": "issues", "action": "labeled", "label": "llmaw:create-needs-assessment"} + ) + # Payload carries the label that triggered the event (even if it was + # later removed by continuous mode before this dispatch run starts). + payload = {"action": "labeled", "label": {"name": "llmaw:create-needs-assessment"}} + assert matches(when, "issues", payload) + + # matches() returns True even though the label is stale/unchecked + # against live state. The responsibility for staleness checking lies + # in the caller (route.py), not in the pure matching function. + + # --------------------------------------------------------------------------- # # flatten_rules end to end # --------------------------------------------------------------------------- # @@ -284,9 +306,7 @@ def test_normalize_label_step_rejects_bad_target(): # normalize_shell_step # --------------------------------------------------------------------------- # def test_normalize_shell_step_string_form(): - assert normalize_shell_step({"shell": "s.sh"}) == { - "shell": {"run": "s.sh", "args": []} - } + assert normalize_shell_step({"shell": "s.sh"}) == {"shell": {"run": "s.sh", "args": []}} def test_normalize_shell_step_list_form_with_args(): diff --git a/tests/test_route.py b/tests/test_route.py new file mode 100644 index 0000000..569e272 --- /dev/null +++ b/tests/test_route.py @@ -0,0 +1,103 @@ +"""Tests for the route module (event-to-rule matching).""" + +from __future__ import annotations + +import json +import textwrap + +from llm_augmented_workflows.route import main as route_main + + +def _write_flows(tmp_path, text: str) -> str: + p = tmp_path / "flows.yml" + p.write_text(textwrap.dedent(text)) + return str(p) + + +def test_route_skips_stale_labeled_event(tmp_path, monkeypatch): + """Regression test for issue #20: route skips rules whose trigger label + is no longer present on the issue's live label set. + + This simulates a stale queued dispatch run: the event payload still + carries the original trigger label, but the issue's live labels no + longer contain it (because continuous mode consumed it). + + The route should return 0 matches for the stale event. + """ + flows = _write_flows( + tmp_path, + """ + flows: + f: + rules: + - id: r1 + when: {event: issues, action: labeled, label: llmaw:create-needs-assessment} + run: [{skill: create-needs-assessment}] + """, + ) + monkeypatch.setenv("FLOWS_FILE", flows) + monkeypatch.setenv("GITHUB_EVENT_NAME", "issues") + monkeypatch.setenv("MODEL", "test-model") + monkeypatch.setenv("AGENTS_REPOSITORY", "test/agents") + + # Stale payload: label in event but removed from issue's live labels. + event_payload = { + "action": "labeled", + "label": {"name": "llmaw:create-needs-assessment"}, + "issue": {"labels": []}, + } + event_path = tmp_path / "event.json" + event_path.write_text(json.dumps(event_payload)) + monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_path)) + + matched_file = tmp_path / "matched.json" + monkeypatch.setenv("MATCHED_FILE", str(matched_file)) + + assert route_main() == 0 + + matched = json.loads(matched_file.read_text()) + assert len(matched) == 0, ( + f"Expected 0 matched rules (stale label filtered), got {len(matched)}." + ) + + +def test_route_matches_live_labeled_event(tmp_path, monkeypatch): + """Verify that legitimate labeled events still match after the stale-label guard. + + When the trigger label is genuinely present on the issue, the route should + match the rule normally. + """ + flows = _write_flows( + tmp_path, + """ + flows: + f: + rules: + - id: r1 + when: {event: issues, action: labeled, label: llmaw:create-needs-assessment} + run: [{skill: create-needs-assessment}] + """, + ) + monkeypatch.setenv("FLOWS_FILE", flows) + monkeypatch.setenv("GITHUB_EVENT_NAME", "issues") + monkeypatch.setenv("MODEL", "test-model") + monkeypatch.setenv("AGENTS_REPOSITORY", "test/agents") + + # Live payload: trigger label is present on the issue. + event_payload = { + "action": "labeled", + "label": {"name": "llmaw:create-needs-assessment"}, + "issue": {"labels": [{"name": "llmaw:create-needs-assessment"}]}, + } + event_path = tmp_path / "event.json" + event_path.write_text(json.dumps(event_payload)) + monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_path)) + + matched_file = tmp_path / "matched.json" + monkeypatch.setenv("MATCHED_FILE", str(matched_file)) + + assert route_main() == 0 + + matched = json.loads(matched_file.read_text()) + assert len(matched) == 1, f"Expected 1 matched rule (live label), got {len(matched)}." + assert matched[0]["id"] == "r1"