Fix execution skill: recognize "Completed" as a terminal success status (#52)

* Fix execution skill: recognize "Completed" as a terminal success status

trigger_workflow.py --wait and monitor_execution.py were checking only
for a "Succeeded" status, but live testing against the execution-results
API showed a normal successful execution reports "Completed" instead —
so both scripts polled to their full timeout and exited 1 on every
successful run, misreporting success as failure/timeout.

Add "completed" to TERMINAL_STATUSES and a new SUCCESS_STATUSES set in
get_execution_results.py (the shared source of truth both other scripts
import from), use it for monitor_execution.py's exit code, and document
the Succeeded/Completed distinction in SKILL.md.

Verified against two real executions: monitor_execution.py now reports
"Completed" within seconds (previously ran to the full timeout), and
trigger_workflow.py --wait no longer false-timeouts on a normal run.

* Add regression tests and changelog entry for terminal-status fix

- test_get_execution_results.py: guard "completed" is terminal, and add
  a TestSuccessStatuses class covering SUCCESS_STATUSES.
- test_monitor_execution.py: regression test reproducing the reported
  bug (a "Completed" result must exit 0, not fall to the non-zero branch).
- test_trigger_workflow.py: regression test for poll_results recognizing
  "Completed" as terminal (previously ran to the full --wait timeout).
- CHANGELOG.md: document the fix under [Unreleased] / Fixed.

* docs: reflect completed as a success status in console-verification table

get_execution_results.py now treats succeeded or completed as success (SUCCESS_STATUSES); update the console-verification 'Determine success' row to match.

---------

Co-authored-by: Matt Raible <matt.raible@crowdstrike.com>

Author: jbennett-cs

CHANGELOG.md

@@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/), and this
 
 ### Fixed
 
+- **`monitor_execution.py` and `trigger_workflow.py --wait` no longer misreport a successful execution as a failure/timeout.** Both polled only for a `Succeeded` terminal status, but live testing against the execution-results API showed a normal successful execution reports `Completed` — so every successful run polled to its full timeout and exited non-zero. `get_execution_results.py`'s shared `TERMINAL_STATUSES` now includes `completed`, and a new `SUCCESS_STATUSES` set (both scripts' single source of truth for the exit-code decision) treats `succeeded` and `completed` as success.
 - **`validate.py` now flags `WorkflowCustomVariable.<name>` references to variables that nothing declares** — a release-only failure. A reference to a custom variable that no `CreateVariable` (or `UpdateVariable` setter) declares imports and validates cleanly, then fails at release with `property "..." contains unknown variable "WorkflowCustomVariable.<name>"`. The validator now collects declared variable names and reports an undeclared reference before you deploy.
 - **`validate.py` no longer rejects valid action IDs that aren't 32-char hex.** The action-ID check assumed every ID was a 32-character hex string (or a `<hex>_<hex>` / `<hex>~<hex>` compound), but real catalog actions carry other shapes — a 26-character ULID joined to a hex id (custom IOC / API-integration actions), unequal compound halves (event query actions), and longer hex strings (RTR actions). Those imported fine yet were flagged as invalid locally. IDs are now treated as opaque catalog identifiers, so the check still rejects placeholders (UPPER_SNAKE tokens, punctuation, all-same-character) without blocking real IDs. Docs updated to describe action IDs as identifiers you look up rather than "32-char hex".
 

skills/deployment/references/console-verification.md

@@ -52,7 +52,7 @@ cannot do, and uses the API for everything deterministic:
 | Render-test (Signal/Scheduled/SubModel) | **Browser** | No API reports "did the canvas draw" — open the editor and watch the console |
 | Configure an HTTP-action credential | **Browser** | Fusion has no API to create an HTTP-action credential |
 | Execute an On-demand workflow | **API** | `trigger_workflow.py --wait` triggers it |
-| Determine success | **API** | `get_execution_results.py` checks `status == succeeded` — no UI eyeballing |
+| Determine success | **API** | `get_execution_results.py` treats `succeeded` or `completed` as success — no UI eyeballing |
 
 So a credential-gated On-demand workflow is handled in two steps: the browser
 configures the VirusTotal credential and publishes it, then the harness executes

skills/execution/SKILL.md

@@ -35,7 +35,9 @@ metadata:
 
 This skill runs Fusion workflows that are already deployed and released, watches them to completion, retrieves their output, and helps debug failures. Writing the YAML happens in the **authoring** skill; importing and releasing happens in the **deployment** skill.
 
-An execution moves through states and ends in a **terminal** state. The terminal states are `Succeeded`, `Failed`, `Canceled`, `NonRecoverable`, and `ActionRequired`. (`ActionRequired` is terminal for polling: it waits on human input and will not progress on its own.) Anything else means the execution is still running.
+An execution moves through states and ends in a **terminal** state. The terminal states are `Succeeded`, `Completed`, `Failed`, `Canceled`, `NonRecoverable`, and `ActionRequired`. (`ActionRequired` is terminal for polling: it waits on human input and will not progress on its own.) Anything else means the execution is still running.
+
+> **`Succeeded` vs `Completed`.** Live testing against the execution-results API found that a normal successful execution reports its top-level `status` as `Completed`, not `Succeeded` — the scripts in this skill treat both as success (see `SUCCESS_STATUSES` in `get_execution_results.py`). Don't assume `Succeeded` is the only success value when reading raw API output yourself.
 
 > **Running the scripts.** Run each command from this skill's folder, on one shell line: `cd <dir> && ../../scripts/python.sh scripts/<name>.py` (a sibling skill's script is `../<skill>/scripts/<name>.py`). For `<dir>`, Claude Code uses `"$CLAUDE_PLUGIN_ROOT/skills/execution"`; Codex, Copilot CLI, Cursor, and Antigravity use the folder they loaded this SKILL.md from (e.g. `~/.agents/skills/execution`). The wrapper bootstraps its own Python venv.
 
@@ -145,7 +147,7 @@ Parameters come from `--params` (a JSON string) or interactive prompts derived f
 ../../scripts/python.sh scripts/monitor_execution.py --execution-id <exec_id> --interval 10 --timeout 600 --json
 ```
 
-Defaults: `--interval 5`, `--timeout 300`. Prints status updates to stderr and the final result to stdout. Exits `0` only when the execution `Succeeded`; non-zero on any other terminal state or timeout, so CI can react.
+Defaults: `--interval 5`, `--timeout 300`. Prints status updates to stderr and the final result to stdout. Exits `0` only on a successful terminal state (`Succeeded` or `Completed`); non-zero on any other terminal state or timeout, so CI can react.
 
 ### get_execution_results.py
 

skills/execution/scripts/get_execution_results.py

@@ -34,8 +34,12 @@
 # Terminal statuses returned by the execution-results API. The API returns
 # capitalized values; callers should match case-insensitively. "ActionRequired"
 # is terminal for polling purposes — it waits on human input and won't progress
-# on its own.
-TERMINAL_STATUSES = {"succeeded", "failed", "canceled", "nonrecoverable", "actionrequired"}
+# on its own. Live testing confirmed a normal successful execution reports
+# "Completed" (not "Succeeded") at the top level — both are treated as success
+# below via SUCCESS_STATUSES; "succeeded" is kept for forward/back compat in
+# case another execution path ever returns it.
+TERMINAL_STATUSES = {"succeeded", "completed", "failed", "canceled", "nonrecoverable", "actionrequired"}
+SUCCESS_STATUSES = {"succeeded", "completed"}
 
 
 def fetch_results(execution_id):

skills/execution/scripts/monitor_execution.py

@@ -2,9 +2,9 @@
 Monitor a CrowdStrike Fusion workflow execution until it completes.
 
 Polls the execution-results API at a fixed interval until the execution reaches
-a terminal state (succeeded, failed, canceled, nonrecoverable, actionrequired)
-or the timeout elapses. Status updates are printed to stderr so the final
-result on stdout stays clean for piping.
+a terminal state (succeeded, completed, failed, canceled, nonrecoverable,
+actionrequired) or the timeout elapses. Status updates are printed to stderr so
+the final result on stdout stays clean for piping.
 
 Usage:
     python monitor_execution.py --execution-id <exec_id>
@@ -21,7 +21,7 @@
 # fetch_results and TERMINAL_STATUSES live alongside this script; they own the
 # auth client and API-response parsing, so this script needs no direct client.
 sys.path.insert(0, os.path.dirname(os.path.realpath(__file__)))
-from get_execution_results import fetch_results, TERMINAL_STATUSES  # pylint: disable=wrong-import-position
+from get_execution_results import fetch_results, TERMINAL_STATUSES, SUCCESS_STATUSES  # pylint: disable=wrong-import-position
 
 # Fix Windows console encoding
 sys.stdout.reconfigure(encoding="utf-8", errors="replace")
@@ -98,7 +98,7 @@ def main():
             print(f"  Output:\n{json.dumps(output, indent=4)}")
 
     # Non-zero exit for non-successful terminal states so callers/CI can react.
-    sys.exit(0 if status.lower() == "succeeded" else 1)
+    sys.exit(0 if status.lower() in SUCCESS_STATUSES else 1)
 
 
 if __name__ == "__main__":

tests/test_get_execution_results.py

@@ -91,6 +91,29 @@ def test_expected_terminal_statuses_present(self):
         """The documented terminal states are covered."""
         assert {"succeeded", "failed", "canceled"} <= get_execution_results.TERMINAL_STATUSES
 
+    def test_completed_is_terminal(self):
+        """Live testing showed the API reports "Completed" (not "Succeeded")
+        for a normal successful execution — it must be terminal or pollers
+        run to their full timeout on every successful run."""
+        assert "completed" in get_execution_results.TERMINAL_STATUSES
+
+
+class TestSuccessStatuses:
+    """Guard the success subset used to decide exit codes."""
+
+    def test_success_statuses_lowercase(self):
+        """All entries are lowercase so case-insensitive matching works."""
+        for status in get_execution_results.SUCCESS_STATUSES:
+            assert status == status.lower()
+
+    def test_success_statuses_are_terminal(self):
+        """Every success status must also be a terminal status."""
+        assert get_execution_results.SUCCESS_STATUSES <= get_execution_results.TERMINAL_STATUSES
+
+    def test_succeeded_and_completed_are_success(self):
+        """Both observed success values are treated as success."""
+        assert {"succeeded", "completed"} <= get_execution_results.SUCCESS_STATUSES
+
 
 class TestMain:
     """Test the CLI entry point and exit codes."""

tests/test_monitor_execution.py

@@ -26,6 +26,21 @@ def test_monitor_returns_terminal_result(self, monkeypatch):
         assert result is not None
         assert result["status"] == "Succeeded"
 
+    def test_monitor_returns_on_completed(self, monkeypatch):
+        """Regression: a terminal "Completed" status (the real API's success
+        value) must also end the loop immediately, not run to the timeout."""
+        mock_client = MagicMock()
+        mock_client.execution_results.return_value = {
+            "status_code": 200,
+            "body": {"resources": [{"status": "Completed"}], "errors": []},
+            "headers": {},
+        }
+        monkeypatch.setattr(get_execution_results, "get_client", lambda: mock_client)
+        monkeypatch.setattr(monitor_execution.time, "sleep", lambda _s: None)
+        result = monitor_execution.monitor("exec_123", interval=0.1, timeout=5)
+        assert result is not None
+        assert result["status"] == "Completed"
+
     def test_monitor_in_progress_then_complete(self, monkeypatch):
         """The loop keeps polling while non-terminal, then returns on completion."""
         statuses = ["Running", "Running", "Succeeded"]
@@ -119,6 +134,25 @@ def test_main_succeeded_exits_0(self, monkeypatch, capsys):
         assert "Succeeded" in out
         assert "done" in out
 
+    def test_main_completed_exits_0(self, monkeypatch, capsys):
+        """Regression: the real API's "Completed" success status must exit 0,
+        not fall through to the non-zero branch like it did before this fix
+        (which misreported every successful run as a failure)."""
+        monkeypatch.setattr(
+            "sys.argv",
+            ["monitor_execution.py", "--execution-id", "exec_123"],
+        )
+        monkeypatch.setattr(
+            monitor_execution,
+            "monitor",
+            lambda *_a, **_k: {"status": "Completed", "output": {"done": True}},
+        )
+        with pytest.raises(SystemExit) as exc:
+            monitor_execution.main()
+        assert exc.value.code == 0
+        out = capsys.readouterr().out
+        assert "Completed" in out
+
     def test_main_failed_exits_1(self, monkeypatch):
         """A non-succeeded terminal state exits non-zero for CI."""
         monkeypatch.setattr(

tests/test_trigger_workflow.py

@@ -398,6 +398,23 @@ def test_poll_succeeded_returns_result(self, monkeypatch):
         assert result["status"] == "Succeeded"
         assert result["output"] == {"key": "value"}
 
+    def test_poll_completed_returns_result(self, monkeypatch):
+        """Regression: live testing showed a normal successful execution
+        reports "Completed", not "Succeeded" — before this fix, poll_results
+        never recognized it as terminal and ran to the full timeout on every
+        successful --wait run."""
+        mock_client = MagicMock()
+        mock_client.execution_results.return_value = {
+            "status_code": 200,
+            "body": {"resources": [{"status": "Completed", "output": {"key": "value"}}]},
+            "headers": {},
+        }
+        monkeypatch.setattr(get_execution_results, "get_client", lambda: mock_client)
+        monkeypatch.setattr(trigger_workflow.time, "sleep", lambda _s: None)
+        result = trigger_workflow.poll_results("fake_id", timeout=5, interval=0.1)
+        assert result is not None
+        assert result["status"] == "Completed"
+
     def test_poll_failed_returns_result(self, monkeypatch):
         """Verify capitalized Failed status is terminal (not retried forever)."""
         mock_client = MagicMock()