Merge pull request #187 from EvanProgramming/codex/issue-186-sanitize-provider-plans

fix: sanitize provider plan metadata

Author: EvanProgramming

interaction.py

@@ -195,35 +195,46 @@ def normalize_provider_control(text: str) -> tuple[str, dict[str, Any]] | None:
     if not isinstance(value, dict):
         return None
 
-    executable_keys = {"action", "args", "arguments", "command", "tool", "tool_calls"}
-    stack: list[Any] = [value]
-    while stack:
-        item = stack.pop()
-        if isinstance(item, dict):
-            if executable_keys & {str(key).lower() for key in item}:
-                return None
-            stack.extend(item.values())
-        elif isinstance(item, list):
-            stack.extend(item)
-
+    executable_keys = {
+        "action", "args", "arguments", "command", "commands", "content", "path", "tool", "tool_calls",
+    }
     mode = str(value.get("mode") or "").strip().lower()
     if mode == "plan" or value.get("do_not_execute_until_approved") is True:
+        if executable_keys & {str(key).lower() for key in value}:
+            return None
         steps = []
         for index, step in enumerate(value.get("steps") or [], 1):
             if not isinstance(step, dict):
                 return None
+            description = step.get("description") or step.get("details") or step.get("title")
+            if not description:
+                return None
             steps.append({
                 "id": str(step.get("id") or step.get("step_id") or f"step-{step.get('step', index)}"),
-                "title": step.get("title") or f"Step {index}",
-                "description": step.get("description") or step.get("details"),
-                "acceptance": step.get("acceptance") or step.get("acceptance_criteria"),
+                "title": step.get("title") or str(description)[:120],
+                "description": description,
+                "acceptance": (
+                    step.get("acceptance") or step.get("acceptance_criteria")
+                    or ["Step completed with observable evidence"]
+                ),
             })
         return "PlanProposal", {
             "title": value.get("title") or value.get("plan_name"),
-            "summary": value.get("summary") or value.get("overview"),
+            "summary": value.get("summary") or value.get("overview") or value.get("goal"),
             "assumptions": value.get("assumptions") or [],
             "steps": steps,
         }
+
+    stack: list[Any] = [value]
+    while stack:
+        item = stack.pop()
+        if isinstance(item, dict):
+            if executable_keys & {str(key).lower() for key in item}:
+                return None
+            stack.extend(item.values())
+        elif isinstance(item, list):
+            stack.extend(item)
+
     if mode == "ask" or value.get("ask_user") is True:
         return "AskUser", {"questions": value.get("questions")}
     return None

tests/test_interaction.py

@@ -187,7 +187,7 @@ def test_exact_acceptance_phrases_and_control_parser(self):
             'PlanProposal\n```json\n{"steps": []}\n```', "PlanProposal",
         ), {"steps": []})
 
-    def test_provider_shaped_plan_is_normalized_but_actions_are_not(self):
+    def test_provider_shaped_plan_discards_executable_step_metadata(self):
         shaped = json.dumps({
             "plan_name": "Status page", "mode": "plan",
             "do_not_execute_until_approved": True,
@@ -201,12 +201,21 @@ def test_provider_shaped_plan_is_normalized_but_actions_are_not(self):
         self.assertEqual(parsed["tool_calls"], [])
 
         executable = json.dumps({
-            "mode": "plan", "plan_name": "Unsafe", "overview": "Write now.",
+            "mode": "plan", "plan_name": "Safe proposal", "goal": "Write after acceptance.",
             "steps": [{"title": "Write", "details": "Write README.",
-                       "acceptance_criteria": ["written"], "action": "write_file"}],
+                       "action": "write_file", "path": "README.md", "content": "unsafe"}],
         })
-        rejected = main._observe_model_response(executable)
-        self.assertIsNone(rejected["plan_proposal"])
+        sanitized = main._observe_model_response(executable)
+        self.assertEqual(sanitized["tool_calls"], [])
+        self.assertEqual(sanitized["plan_proposal"]["title"], "Safe proposal")
+        self.assertEqual(
+            set(sanitized["plan_proposal"]["steps"][0]),
+            {"id", "title", "description", "acceptance"},
+        )
+        self.assertEqual(
+            sanitized["plan_proposal"]["steps"][0]["acceptance"],
+            ["Step completed with observable evidence"],
+        )
 
     def test_plan_mode_converts_plain_plan_and_discards_early_actions(self):
         mode_token = main._active_interaction_mode.set("plan")