fix(replay): revert_csv handler delegates to revert_csv_pipeline (build 663)

RELEASE-BLOCKER FIX: _execute_replay_revert_csv in bin/wl_replay.py had
three production bugs in dead code that's wired into REPLAY_HANDLERS:

1. Line 226: `get_versions_dir()` called with no arguments, but the
   function signature in wl_versions.py:40 requires `csv_path: str`.
   Always raises TypeError caught by broad except → "revert_failed".

2. Line 236: `read_version_manifest(csv_file)` passed the filename
   instead of the resolved CSV path. The function would silently
   create a `_versions/` directory in cwd and return ({}, error).

3. Line 219/247: reads "version_id" from request_item, but the
   approval queue stores "version_filename" (see wl_handler.py:6768).
   Manifest schema also has no "version_id" field — the iteration
   at line 246-249 would fail even if reached.

Additionally, REPLAY_HANDLERS["revert_csv"] was wired but the handler's
process_approval path uses action_type="revert" (no _csv suffix, see
wl_handler.py:6761) — dispatch keys didn't match.

The fix replaces the entire broken body (109 lines) with a delegation
to revert_csv_pipeline, matching the pattern used by create_rule /
delete_rule / delete_csv handlers. This makes the replay path
bit-for-bit identical to the direct-handler revert path and eliminates
the dual-source-of-truth that allowed the bugs to ship.

Changes:
- bin/wl_replay.py: rewrite _execute_replay_revert_csv (109 → 85 lines)
- bin/wl_replay.py: add "revert" alias in REPLAY_HANDLERS
- bin/wl_replay.py: add "revert" to _csv_required_actions precondition set
- default/app.conf: build 662 → 663
- appserver/static/whitelist_manager.js: urlArgs _b=662 → _b=663
- tests/unit/test_replay.py: replace TestRevertCsvHandlerKnownBug
  (which pinned the broken behavior) with TestRevertCsvHandler — 6
  tests covering happy path, missing version_filename, pipeline
  failure, pipeline exception, legacy payload-nested submissions, and
  the new "revert" alias

Production impact at the user-facing path: ZERO, because the handler
short-circuits revert approvals at wl_handler.py:6761 by calling
self._revert_csv directly (it never routes through
execute_approved_action for reverts). The bugs were dormant. But the
REPLAY_HANDLERS dispatch table is part of the architectural contract
that future refactors will trust — leaving broken entries in it sets
a trap for the next contributor who tries to consolidate replay paths.

Coverage delta on bin/wl_replay.py: 79% → 92% (+13pp, on top of the
+59pp from G3 batch 3c). Tests: 734 → 738 passed (+4 net after
replacing 2 KnownBug pins with 6 fixed-behavior tests).

Doc-drift: OK (checked 33 docs against build 663).

Author: RelativisticJet

appserver/static/whitelist_manager.js

@@ -11,7 +11,7 @@
 // disk cache. Splunk serves /static/@<server-hash>/... with Cache-Control:
 // public, max-age=31536000; without urlArgs, bumped build numbers don't force
 // a re-fetch and clients run stale JS until they hard-refresh.
-require.config({ urlArgs: "_b=662" });
+require.config({ urlArgs: "_b=663" });
 require([
     "jquery",
     "underscore",

bin/wl_replay.py

@@ -73,7 +73,8 @@ def execute_approved_action(context: Dict[str, Any], request_item: Dict[str, Any
         }
 
     # Validate preconditions for actions requiring CSV files
-    _csv_required_actions = {"save_csv", "add_row", "remove_rows", "revert_csv"}
+    _csv_required_actions = {"save_csv", "add_row", "remove_rows",
+                             "revert_csv", "revert"}
     if action_type in _csv_required_actions:
         csv_file = request_item.get("csv_file", "")
         app_context = request_item.get("app_context", "")
@@ -205,114 +206,90 @@ def _execute_replay_save_csv(context: Dict[str, Any], request_item: Dict[str, An
 
 def _execute_replay_revert_csv(context: Dict[str, Any], request_item: Dict[str, Any]) -> Dict[str, Any]:
     """
-    Execute revert_csv action — restore CSV to a previous version.
+    Execute revert_csv action — delegates to revert_csv_pipeline.
 
-    Args:
-        context: Approval context with metadata
-        request_item: Contains csv_file, app_context, version_id, comment
+    Matches the delegation pattern used by create_rule / delete_rule /
+    delete_csv handlers. The pipeline owns snapshot/diff/audit logic
+    (single source of truth — keeps replay path and direct-handler
+    path bit-for-bit identical).
 
-    Returns:
-        Dict: {success: bool, message: str, data: dict (optional), error: str (optional)}
+    The approval queue stores `version_filename` and `version_display`
+    (set by the submitter's call to _submit_approval, see
+    wl_handler.py:6768). Older payloads that nested these under
+    `payload` are also honored.
     """
     csv_file = request_item.get("csv_file", "")
     app_context = request_item.get("app_context", "")
-    version_id = request_item.get("version_id", "")
-    comment = request_item.get("comment", "")
-
-    try:
-        # Get version manifest to find the version file
-        from wl_versions import read_version_manifest, get_versions_dir
-
-        versions_dir = get_versions_dir()
-        manifest_path = os.path.join(versions_dir, f"{csv_file}_versions.json")
-
-        if not os.path.isfile(manifest_path):
-            return {
-                "success": False,
-                "error": "Version history not found",
-                "error_type": "missing_versions"
-            }
-
-        manifest, _ = read_version_manifest(csv_file)
-        if not manifest:
-            return {
-                "success": False,
-                "error": "Failed to read version manifest",
-                "error_type": "manifest_error"
-            }
-
-        # Find version file matching version_id
-        version_file = None
-        for v in manifest.get("versions", []):
-            if v.get("version_id") == version_id:
-                version_file = os.path.join(versions_dir, v.get("filename", ""))
-                break
-
-        if not version_file or not os.path.isfile(version_file):
-            return {
-                "success": False,
-                "error": "Version file not found",
-                "error_type": "missing_version_file"
-            }
-
-        # Read version CSV data
-        headers, rows = read_csv(version_file)
-
-        # Write CSV to current location
-        path = resolve_csv_path(csv_file, app_context)
-        if path is None:
-            fallback = os.path.join(OWN_LOOKUPS, csv_file)
-            if os.path.isfile(fallback) and safe_realpath(fallback, APPS_DIR):
-                path = safe_realpath(fallback, APPS_DIR)
+    detection_rule = request_item.get("detection_rule", "")
 
-        if path is None:
-            return {
-                "success": False,
-                "error": "CSV file not found",
-                "error_type": "missing_csv"
-            }
+    payload = request_item.get("payload", {}) or {}
+    version_filename = (request_item.get("version_filename", "")
+                        or payload.get("version_filename", ""))
+    version_display = (request_item.get("version_display", "")
+                       or payload.get("version_display", ""))
+    revert_reason = (request_item.get("revert_reason", "")
+                     or payload.get("revert_reason", "")
+                     or payload.get("comment", "")
+                     or request_item.get("comment", "")
+                     or "Approved via approval queue")
+
+    if not version_filename:
+        return {
+            "success": False,
+            "error": "version_filename missing from approval payload",
+            "error_type": "missing_version_filename",
+        }
 
-        write_csv(path, headers, rows)
+    path = resolve_csv_path(csv_file, app_context)
+    if path is None:
+        fallback = os.path.join(OWN_LOOKUPS, csv_file)
+        if os.path.isfile(fallback) and safe_realpath(fallback, APPS_DIR):
+            path = safe_realpath(fallback, APPS_DIR)
+    if path is None:
+        return {
+            "success": False,
+            "error": "CSV file not found",
+            "error_type": "missing_csv",
+        }
 
-        # Create new version snapshot of reverted state (path, analyst, action_label)
-        analyst = context.get("original_analyst", "")
-        new_version_id, _ = snapshot_version(path, analyst, "revert_csv")
+    from wl_versions import revert_csv_pipeline
 
-        # Post audit event
-        session_key = context.get("session_key", "")
-        approving_admin = context.get("approving_admin", "")
-        request_id = context.get("request_id", "")
+    analyst = context.get("original_analyst", "")
+    session_key = context.get("session_key", "")
 
-        audit_evt = build_audit_event(
-            action="replay_revert_csv",
-            analyst=approving_admin,
-            detection_rule=request_item.get("detection_rule", ""),
+    try:
+        result = revert_csv_pipeline(
+            csv_path=path,
+            version_filename=version_filename,
+            version_display=version_display,
+            revert_reason=revert_reason,
+            analyst=analyst,
+            session_key=session_key,
             csv_file=csv_file,
             app_context=app_context,
-            comment=f"Reverted to {version_id} by {approving_admin}. {comment}",
-            request_id=request_id,
-            reverted_to_version=version_id,
-            new_record_version=new_version_id,
-            original_analyst=analyst
+            detection_rule=detection_rule,
         )
-
-        post_result, post_error = post_audit_event(session_key, audit_evt)
-        if not post_result:
-            _logger.error(f"Audit posting failed for replay_revert_csv: {post_error}")
-
+    except Exception as e:
+        _logger.error("revert_csv_pipeline raised: %s", e, exc_info=True)
         return {
-            "success": True,
-            "message": "CSV reverted successfully",
-            "data": {"version_id": new_version_id}
+            "success": False,
+            "error": str(e),
+            "error_type": "revert_failed",
         }
 
-    except Exception as e:
+    if not result.get("success"):
         return {
             "success": False,
-            "error": str(e),
-            "error_type": "revert_failed"
+            "error": result.get("error", "Revert failed"),
+            "error_type": "revert_failed",
         }
 
+    return {
+        "success": True,
+        "message": result.get("message", "CSV reverted successfully"),
+        "data": result.get("data", {}),
+    }
+
 
 def _execute_replay_create_rule(context: Dict[str, Any], request_item: Dict[str, Any]) -> Dict[str, Any]:
     """
@@ -523,4 +500,5 @@ def _execute_replay_create_csv(context: Dict[str, Any], request_item: Dict[str,
     "remove_csv": _execute_replay_delete_csv,   # Alias: approval queue uses remove_*
     "remove_rule": _execute_replay_delete_rule,  # Alias: approval queue uses remove_*
     "revert_csv": _execute_replay_revert_csv,
+    "revert": _execute_replay_revert_csv,  # Alias: handler stores action_type="revert"
 }

default/app.conf

@@ -5,7 +5,7 @@
 
 [install]
 is_configured = false
-build = 662
+build = 663
 
 [launcher]
 author       = Oleh Bezsonov

tests/unit/test_replay.py

@@ -618,59 +618,144 @@ def test_save_csv_failed_audit_post_still_returns_success(self):
         self.assertTrue(result["success"])
 
 
-class TestRevertCsvHandlerKnownBug(unittest.TestCase):
-    """Cover _execute_replay_revert_csv (lines 206-314).
-
-    KNOWN BUG (documented here to make the test stable against the fix):
-    Line 226 calls `get_versions_dir()` with no arguments, but the function
-    requires `csv_path: str`. In production this ALWAYS raises TypeError,
-    which the broad `except Exception` at line 309 catches and returns as
-    `revert_failed`. Lines 229-307 are effectively dead code in production
-    until the bug is fixed.
-
-    We test the bug-caused error path, plus we cover the dead-code path
-    by lazily patching the in-function imports of read_version_manifest
-    and get_versions_dir at their source module (wl_versions).
+class TestRevertCsvHandler(unittest.TestCase):
+    """Cover _execute_replay_revert_csv (delegation to revert_csv_pipeline).
+
+    History: an earlier implementation called `get_versions_dir()` with
+    no arguments (the function requires `csv_path: str`), and read
+    `version_id` from request_item although the approval queue stores
+    `version_filename`. Both bugs are fixed by delegating wholesale to
+    `revert_csv_pipeline` (same pattern as create_rule/delete_rule
+    handlers). These tests pin the FIXED behaviour.
     """
 
     def setUp(self):
         if execute_approved_action is None:
             self.skipTest("execute_approved_action not available")
 
-    def test_revert_csv_production_bug_returns_revert_failed(self):
-        """Document the dead-code bug: get_versions_dir() called with no args."""
+    def test_revert_csv_happy_path_delegates_to_pipeline(self):
+        """Successful pipeline → success result + data forwarded."""
+        import wl_replay
+        import wl_versions
+        with patch.object(wl_replay, "resolve_csv_path",
+                          return_value="/fake/x.csv"), \
+             patch.object(wl_versions, "revert_csv_pipeline",
+                          return_value={"success": True,
+                                        "message": "reverted",
+                                        "data": {"new_record_version":
+                                                 "2026-05-19 ..."}}) as mock_pipe:
+            result = execute_approved_action(
+                {"session_key": "sk",
+                 "original_analyst": "alice",
+                 "approving_admin": "bob"},
+                {"action_type": "revert_csv",
+                 "csv_file": "x.csv",
+                 "app_context": "wl_manager",
+                 "detection_rule": "DR100",
+                 "version_filename": "x_20260101_120000.csv",
+                 "version_display": "01-01-2026 12:00:00 (3 rows, by alice)",
+                 "revert_reason": "approved revert"},
+            )
+        self.assertTrue(result["success"])
+        self.assertEqual(result["message"], "reverted")
+        # Verify the delegation passes the expected kwargs
+        mock_pipe.assert_called_once()
+        kwargs = mock_pipe.call_args.kwargs
+        self.assertEqual(kwargs["csv_path"], "/fake/x.csv")
+        self.assertEqual(kwargs["version_filename"],
+                         "x_20260101_120000.csv")
+        self.assertEqual(kwargs["version_display"],
+                         "01-01-2026 12:00:00 (3 rows, by alice)")
+        self.assertEqual(kwargs["revert_reason"], "approved revert")
+        self.assertEqual(kwargs["analyst"], "alice")
+        self.assertEqual(kwargs["csv_file"], "x.csv")
+        self.assertEqual(kwargs["app_context"], "wl_manager")
+        self.assertEqual(kwargs["detection_rule"], "DR100")
+
+    def test_revert_csv_missing_version_filename_returns_error(self):
+        """No version_filename in payload → missing_version_filename error."""
         import wl_replay
         with patch.object(wl_replay, "resolve_csv_path",
                           return_value="/fake/x.csv"):
             result = execute_approved_action(
                 {"session_key": "sk"},
                 {"action_type": "revert_csv",
                  "csv_file": "x.csv",
-                 "version_id": "v123"},
+                 "app_context": "wl_manager",
+                 "payload": {}},  # no version_filename anywhere
+            )
+        self.assertFalse(result["success"])
+        self.assertEqual(result["error_type"], "missing_version_filename")
+
+    def test_revert_csv_pipeline_failure_returns_revert_failed(self):
+        """Pipeline failure surfaces as revert_failed error_type."""
+        import wl_replay
+        import wl_versions
+        with patch.object(wl_replay, "resolve_csv_path",
+                          return_value="/fake/x.csv"), \
+             patch.object(wl_versions, "revert_csv_pipeline",
+                          return_value={"success": False,
+                                        "error": "version file missing"}):
+            result = execute_approved_action(
+                {"session_key": "sk", "original_analyst": "alice"},
+                {"action_type": "revert_csv",
+                 "csv_file": "x.csv",
+                 "version_filename": "x_20260101.csv"},
             )
-        # The TypeError from get_versions_dir() is caught by the broad except
         self.assertFalse(result["success"])
         self.assertEqual(result["error_type"], "revert_failed")
-        self.assertIn("missing", result["error"].lower())
+        self.assertEqual(result["error"], "version file missing")
 
-    def test_revert_csv_missing_manifest_path_returns_missing_versions(self):
-        """If we patch around the bug, missing manifest → missing_versions."""
+    def test_revert_csv_pipeline_exception_returns_revert_failed(self):
+        """If the pipeline RAISES, wrapper catches and returns revert_failed."""
         import wl_replay
         import wl_versions
         with patch.object(wl_replay, "resolve_csv_path",
                           return_value="/fake/x.csv"), \
-             patch.object(wl_versions, "get_versions_dir",
-                          return_value="/fake/_versions"), \
-             patch("os.path.isfile", return_value=False):
+             patch.object(wl_versions, "revert_csv_pipeline",
+                          side_effect=RuntimeError("disk full")):
             result = execute_approved_action(
                 {"session_key": "sk"},
                 {"action_type": "revert_csv",
                  "csv_file": "x.csv",
-                 "version_id": "v123"},
+                 "version_filename": "x_20260101.csv"},
             )
-        # Either missing_versions (path patched OK) or missing_csv (precondition)
-        # — both are acceptable failure modes for this bug-around test.
         self.assertFalse(result["success"])
+        self.assertEqual(result["error_type"], "revert_failed")
+        self.assertIn("disk full", result["error"])
+
+    def test_revert_csv_legacy_payload_under_payload_key(self):
+        """Older queue entries nest version_filename under 'payload' — honored."""
+        import wl_replay
+        import wl_versions
+        with patch.object(wl_replay, "resolve_csv_path",
+                          return_value="/fake/x.csv"), \
+             patch.object(wl_versions, "revert_csv_pipeline",
+                          return_value={"success": True,
+                                        "message": "ok"}) as mock_pipe:
+            result = execute_approved_action(
+                {"session_key": "sk"},
+                {"action_type": "revert_csv",
+                 "csv_file": "x.csv",
+                 "payload": {"version_filename": "legacy_x_20260101.csv",
+                             "version_display": "legacy display"}},
+            )
+        self.assertTrue(result["success"])
+        kwargs = mock_pipe.call_args.kwargs
+        self.assertEqual(kwargs["version_filename"],
+                         "legacy_x_20260101.csv")
+
+    def test_revert_alias_dispatches_to_same_handler(self):
+        """action_type='revert' (no _csv suffix) routes to same handler.
+
+        The handler stores approval queue entries with action_type='revert'
+        (see wl_handler.py:6761), not 'revert_csv'. The dispatch table
+        must accept both.
+        """
+        import wl_replay
+        self.assertIn("revert", wl_replay.REPLAY_HANDLERS)
+        self.assertIs(wl_replay.REPLAY_HANDLERS["revert"],
+                      wl_replay.REPLAY_HANDLERS["revert_csv"])
 
 
 class TestCreateCsvHandler(unittest.TestCase):