diff --git a/examples/upgrade-plan-smoke.py b/examples/upgrade-plan-smoke.py index 824f90d909..a34d2f424e 100644 --- a/examples/upgrade-plan-smoke.py +++ b/examples/upgrade-plan-smoke.py @@ -438,15 +438,27 @@ def assert_codex_app_automation_is_discovered(registry_path: Path, codex_home: P codex_home, prompt=rendered, ) + invalid_path = codex_home / "automations" / "invalid-legacy" / "automation.toml" + invalid_path.parent.mkdir(parents=True) + invalid_path.write_text( + 'kind = "heartbeat"\nprompt = "C:\\Users\\alice"\n', + encoding="utf-8", + ) payload = build_upgrade_plan(registry_path=registry_path, cli_bin="loopx") assert payload["installed_manifest"]["source"] == "codex_app_automations", payload assert payload["installed_manifest"]["available"] is True, payload + assert payload["installed_manifest"]["parse_error_count"] == 1, payload + assert payload["installed_manifest"]["parse_errors"] == [ + {"automation_id": "invalid-legacy", "reason": "invalid_toml"} + ], payload + assert payload["installed_manifest"]["parse_errors_complete"] is True, payload auto_entry = payload["installed_manifest"]["entries"][0] assert "task_body" not in auto_entry, payload assert auto_entry["prompt_sha256"] == expected_sha, payload assert auto_entry["prompt_policy_audit"]["status"] == "clean", payload assert auto_entry["prompt_policy_audit"]["warning_count"] == 0, payload assert payload["summary"]["installed_manifest_entry_count"] == 1, payload + assert payload["summary"]["installed_manifest_parse_error_count"] == 1, payload assert payload["summary"]["installed_manifest_task_body_count"] == 0, payload assert payload["summary"]["installed_manifest_has_task_body"] is False, payload assert payload["summary"]["installed_prompt_policy_warning_count"] == 0, payload @@ -463,6 +475,9 @@ def assert_codex_app_automation_is_discovered(registry_path: Path, codex_home: P assert installed["automation_id"] == GOAL_ID, payload assert installed["installed"] is True, payload assert payload["summary"]["ready_for_default_promotion"] is True, payload + markdown = render_upgrade_plan_markdown(payload) + assert "installed_manifest_parse_error_count: `1`" in markdown, markdown + assert "parse_error automation_id=`invalid-legacy` reason=`invalid_toml`" in markdown, markdown def assert_codex_app_stale_policy_prompt_is_flagged(registry_path: Path, codex_home: Path) -> None: diff --git a/loopx/upgrade.py b/loopx/upgrade.py index 660c9eb2ba..98f6cb549d 100644 --- a/loopx/upgrade.py +++ b/loopx/upgrade.py @@ -4,6 +4,7 @@ import json import os import re +import tomllib from pathlib import Path from typing import Any @@ -55,6 +56,7 @@ STAGE_DEFERRED_ADAPTER_STATUSES = { "planned", } +_AUTOMATION_PARSE_ERROR_LIMIT = 20 def prompt_digest(text: str) -> str: @@ -177,27 +179,7 @@ def codex_home() -> Path: def parse_automation_toml(path: Path) -> dict[str, Any]: - values: dict[str, Any] = {} - for raw_line in path.read_text(encoding="utf-8").splitlines(): - line = raw_line.strip() - if not line or line.startswith("#") or "=" not in line: - continue - key, raw_value = line.split("=", 1) - key = key.strip() - value = raw_value.strip() - if value.startswith('"') and value.endswith('"'): - try: - values[key] = json.loads(value) - except json.JSONDecodeError: - values[key] = value[1:-1] - elif value in {"true", "false"}: - values[key] = value == "true" - else: - try: - values[key] = int(value) - except ValueError: - values[key] = value - return values + return tomllib.loads(path.read_text(encoding="utf-8")) def infer_goal_id_from_prompt(prompt: str) -> str | None: @@ -252,13 +234,34 @@ def load_codex_app_automation_manifest(root: Path | None = None) -> dict[str, An "entries": [], "reason": "no installed automation manifest provided and Codex App automations directory does not exist", "source": "codex_app_automations", + "parse_error_count": 0, + "parse_errors": [], + "parse_errors_complete": True, } entries: list[dict[str, Any]] = [] + parse_errors: list[dict[str, str]] = [] + parse_error_count = 0 for path in sorted(automations_root.glob("*/automation.toml")): try: automation = parse_automation_toml(path) except OSError: + parse_error_reason = "unreadable" + except UnicodeError: + parse_error_reason = "invalid_utf8" + except tomllib.TOMLDecodeError: + parse_error_reason = "invalid_toml" + else: + parse_error_reason = None + if parse_error_reason is not None: + parse_error_count += 1 + if len(parse_errors) < _AUTOMATION_PARSE_ERROR_LIMIT: + parse_errors.append( + { + "automation_id": path.parent.name, + "reason": parse_error_reason, + } + ) continue if automation.get("kind") != "heartbeat": continue @@ -299,7 +302,18 @@ def load_codex_app_automation_manifest(root: Path | None = None) -> dict[str, An "path": str(automations_root), "entries": entries, "source": "codex_app_automations", - "reason": None if entries else "no LoopX heartbeat automations discovered", + "reason": ( + None + if entries + else ( + "no readable LoopX heartbeat automations discovered" + if parse_error_count + else "no LoopX heartbeat automations discovered" + ) + ), + "parse_error_count": parse_error_count, + "parse_errors": parse_errors, + "parse_errors_complete": parse_error_count == len(parse_errors), } @@ -332,7 +346,7 @@ def resolve_codex_app_automation_rrule( and str(entry.get("rrule") or "").strip() ] if len(candidates) != 1: - return { + result = { "available": False, "reason": ( "Codex App heartbeat RRULE is ambiguous" @@ -341,6 +355,14 @@ def resolve_codex_app_automation_rrule( ), "candidate_count": len(candidates), } + parse_error_count = int(manifest.get("parse_error_count") or 0) + if parse_error_count: + result["manifest_parse_error_count"] = parse_error_count + result["manifest_parse_errors"] = manifest.get("parse_errors") or [] + result["manifest_parse_errors_complete"] = ( + manifest.get("parse_errors_complete") is True + ) + return result entry = candidates[0] return { "available": True, @@ -905,6 +927,9 @@ def build_upgrade_plan( "installed_manifest_available": manifest.get("available"), "installed_manifest_source": manifest.get("source"), "installed_manifest_entry_count": len(manifest_entries), + "installed_manifest_parse_error_count": int( + manifest.get("parse_error_count") or 0 + ), "installed_manifest_task_body_count": manifest_task_body_count, "installed_manifest_has_task_body": manifest_task_body_count > 0, "installed_prompt_policy_warning_count": policy_warning_count, @@ -954,6 +979,7 @@ def render_upgrade_plan_markdown(payload: dict[str, Any]) -> str: f"- stage_deferred_goal_count: `{summary.get('stage_deferred_goal_count')}`", f"- installed_manifest_source: `{summary.get('installed_manifest_source')}`", f"- installed_manifest_entry_count: `{summary.get('installed_manifest_entry_count')}`", + f"- installed_manifest_parse_error_count: `{summary.get('installed_manifest_parse_error_count')}`", f"- installed_manifest_has_task_body: `{summary.get('installed_manifest_has_task_body')}`", f"- installed_prompt_policy_warning_count: `{summary.get('installed_prompt_policy_warning_count')}`", f"- installed_prompt_policy_warning_prompt_count: `{summary.get('installed_prompt_policy_warning_prompt_count')}`", @@ -971,8 +997,17 @@ def render_upgrade_plan_markdown(payload: dict[str, Any]) -> str: f"- available: `{manifest.get('available')}`", f"- path: `{manifest.get('path')}`", f"- reason: `{manifest.get('reason')}`", + f"- parse_error_count: `{manifest.get('parse_error_count')}`", + f"- parse_errors_complete: `{manifest.get('parse_errors_complete')}`", ] ) + for error in manifest.get("parse_errors") or []: + if not isinstance(error, dict): + continue + lines.append( + f"- parse_error automation_id=`{error.get('automation_id')}` " + f"reason=`{error.get('reason')}`" + ) propagation = ( payload.get("default_upgrade_propagation") if isinstance(payload.get("default_upgrade_propagation"), dict) diff --git a/scripts/codex_app_apply_rrule.py b/scripts/codex_app_apply_rrule.py index 9095f49388..fd5aaf7f7e 100644 --- a/scripts/codex_app_apply_rrule.py +++ b/scripts/codex_app_apply_rrule.py @@ -26,6 +26,7 @@ import subprocess import sys import time +import tomllib from pathlib import Path from typing import Any @@ -131,11 +132,15 @@ def _heartbeat_task_body( body = payload.get("task_body") if isinstance(payload, dict) else None if not isinstance(body, str) or not body.strip(): raise SystemExit("loopx heartbeat-prompt returned no task_body") - if '"""' in body: - raise SystemExit("loopx heartbeat-prompt task_body contains triple quotes") return body +def _toml_string(value: str) -> str: + """Encode a string in the JSON-compatible subset of TOML basic strings.""" + + return json.dumps(value, ensure_ascii=False) + + def _write_automation_toml( path: Path, *, @@ -148,19 +153,28 @@ def _write_automation_toml( path.parent.mkdir(parents=True, exist_ok=True) text = ( "version = 1\n" - f'id = "{automation_id}"\n' + f"id = {_toml_string(automation_id)}\n" 'kind = "heartbeat"\n' - f'name = "{name}"\n' - f'prompt = """{prompt}"""\n' + f"name = {_toml_string(name)}\n" + f"prompt = {_toml_string(prompt)}\n" 'status = "ACTIVE"\n' - f'rrule = "{rrule}"\n' - f'target_thread_id = "{thread_id}"\n' + f"rrule = {_toml_string(rrule)}\n" + f"target_thread_id = {_toml_string(thread_id)}\n" f"created_at = {_now_ms()}\n" f"updated_at = {_now_ms()}\n" ) path.write_text(text, encoding="utf-8") +def _read_automation_prompt(path: Path) -> str | None: + try: + automation = tomllib.loads(path.read_text(encoding="utf-8")) + except (OSError, UnicodeError, tomllib.TOMLDecodeError) as exc: + raise SystemExit(f"automation TOML is unreadable or invalid: {path}") from exc + prompt = automation.get("prompt") + return prompt if isinstance(prompt, str) and prompt.strip() else None + + def _sqlite_row_exists(db_path: Path, automation_id: str) -> bool: if not db_path.exists(): return False @@ -287,9 +301,7 @@ def _ensure_automation( cwd = str(goal.get("repo") or "") name = f"{goal_id} LoopX" if toml_path.exists(): - text = toml_path.read_text(encoding="utf-8") - match = re.search(r'prompt = """(.*?)"""', text, re.DOTALL) - prompt = match.group(1) if match else _heartbeat_task_body( + prompt = _read_automation_prompt(toml_path) or _heartbeat_task_body( loopx=loopx, registry=registry, goal_id=goal_id, @@ -380,8 +392,9 @@ def _update_toml(toml_path: Path, rrule: str) -> None: pattern = re.compile(r'^rrule\s*=\s*".*?"', re.MULTILINE) if not pattern.search(text): raise SystemExit(f"automation TOML has no rrule field: {toml_path}") + replacement = f"rrule = {_toml_string(rrule)}" toml_path.write_text( - pattern.sub(f'rrule = "{rrule}"', text), + pattern.sub(lambda _: replacement, text), encoding="utf-8", ) diff --git a/skills/loopx-self-repair/references/repair-patterns.md b/skills/loopx-self-repair/references/repair-patterns.md index 73abd622ad..33d831fcd6 100644 --- a/skills/loopx-self-repair/references/repair-patterns.md +++ b/skills/loopx-self-repair/references/repair-patterns.md @@ -143,6 +143,7 @@ teaches a reusable control-plane lesson. | `pr_review_feature_gate_counterfactual_gap` | A review proves the feature-on path and approves an opt-in or default-off change, but users who never enable it still receive new schema requirements, prompt instructions, accepted inputs, persisted projections, scheduling decisions, or effects; scoped sub-agent behavior may also be published under a broader multi-agent protocol name. | Authoritative gate and default, identical disabled/enabled fixtures, pre-change disabled output, every shared changed builder or serializer, emitted schema/prompt/journal/effect differences, public protocol ids, actual actor lifecycle, and granted or excluded authority. | The review inferred whole-change isolation from the absence of one enabled topology or operation list, and treated broad protocol terminology as cosmetic even when it implied registered peers or durable coordination that the implementation did not grant. Feature-on tests then encoded the default-off leakage as expected behavior. | Require typed `default_off_isolation` and `authority_semantics` evidence for every code change, with explicit `not_applicable` only after checking scope. Trace all shared surfaces, run a paired feature-off/feature-on counterfactual, compare the disabled side with the pre-change contract, and block `not_isolated`, `misleading`, or `not_yet_proven`. Prefer established sub-agent/child terminology for ephemeral scoped execution and rename unshipped v0 protocols before compatibility cost accumulates. | | `slash_command_packet_projection_drift` | A slash command tool is called, but the agent pipes or summarizes its JSON into a narrow view such as `.summary` / `.review_sequence`, then ignores the packet's response contract, templates, evidence commands, or final-answer requirements. | Raw session tool calls, command stdout, slash-command catalog `agent_contract`, full CLI JSON packet, skill fallback instructions, final answer shape. | The agent treated the CLI as a queue/statistics helper instead of the authoritative interaction contract; key machine fields never entered model context. | Make the skill name the exact JSON command that preserves contract/template fields, add machine-readable `required_packet_fields_to_preserve`, forbid summary-only projection for the first pass, and cover the required fields with focused smoke tests. | | `loop_surface_activation_gap` | A thread reports LoopX setup success because `register-agent` or `quota should-run --agent-id` works, but no Codex App heartbeat automation, Codex CLI `/goal`, or other host loop has been installed for that goal. | `upgrade-plan` / host-loop activation status, `$CODEX_HOME/automations/*/automation.toml`, generated `heartbeat-prompt`, target thread id, recent setup final answer, registry coordination fields. | The control-plane identity layer was treated as the host scheduler layer; setup success criteria stopped at registry/quota instead of proving `host_loop_activation.activated=true` or surfacing a concrete host-tool gate. | Project setup and agent registration must surface `host_loop_activation`; create/update the host loop from generated scoped `heartbeat-prompt`, or report the exact missing host capability. Do not claim "connected" as autonomous until the host loop is active. | +| `codex_app_automation_toml_contract_gap` | A multiline Codex App heartbeat is visible under a permissive line reader, then disappears from upgrade or RRULE resolution after a standards-compliant TOML reader is introduced; prompts containing backslashes, quote delimiters, or escape-like text are common triggers. | Exact writer output, standard-library TOML readback, prompt digest/counts, manifest parse-error projection, and RRULE resolution before and after the reader change. | The writer interpolated arbitrary prompt text into a TOML basic string without serialization, while the old partial reader masked the invalid file and the strict reader's catch-and-skip path erased the difference between absent and malformed automation. | Treat writer and reader as one storage contract: encode every string through a TOML-safe serializer, parse with the standard library at all read sites, fail closed on malformed content, and expose bounded content-free parse diagnostics. Cover backslashes, embedded delimiters, line-ending escapes, exact prompt identity, and capped malformed-file reporting in the real writer-to-reader path. | | `scheduler_host_update_retry_loop` | The same Codex App heartbeat repeatedly hangs or fails while applying the same recommended RRULE against the same observed host RRULE, including when active-work and monitor-wait targets alternate. | `scheduler_hint.codex_app`, persisted scheduler state, host RRULE observation, recent heartbeat history, host-tool timeout/failure. | Host update failure was treated as turn-local advice or persisted as one scalar, so a cadence phase change overwrote the previous failed pair and the next phase retried it. | Persist a bounded, expiring set of failed target/observed-host pairs with `failure_hint.cli_args`, retain the latest scalar only for compatibility, suppress every retained exact pair without ACK or spend, invalidate old-host entries when the observation changes, and clear the cache after a successful ACK. | | `scheduler_implicit_host_default` | A generic or CLI caller omits scheduler ownership fields yet receives Codex App RRULE/backoff actions, so tests pass without proving the real host path uses the same contract. | `scheduler_hint.execution_context`, quota CLI argv, heartbeat generator output, host-loop activation packet, scheduler tests. | The shared resolver treated missing context as a legacy Codex App default instead of assigning compatibility at the actual App boundary. | Make missing generic context fail closed; preserve App behavior only through an explicit compact runtime profile emitted by the Codex App generator; pass typed context for other hosts; cover bare failure, profile behavior, generated command, and real CLI integration. | | `scheduler_followup_runtime_binding_gap` | A Codex App quota decision emits an ACK or failure command, but executing that generated command recomputes a generic fail-closed scheduler packet and rejects the writeback. | `scheduler_hint.codex_app.ack_hint.cli_args`, `failure_hint.cli_args`, the recomputed `before.scheduler_hint.execution_context`, and a real generated-command CLI smoke. | The initial `quota should-run` carried an explicit App runtime profile, but the follow-up command and current-hint handler dropped it during recomputation. | Carry the compact App profile in generated ACK/failure commands, resolve the same typed context in every quota command that recomputes scheduler state, and execute the generated command in a durable CLI smoke before claiming convergence. | diff --git a/tests/control_plane/test_scheduler_fallback_hint.py b/tests/control_plane/test_scheduler_fallback_hint.py index 6c54b08a60..cf60ef6582 100644 --- a/tests/control_plane/test_scheduler_fallback_hint.py +++ b/tests/control_plane/test_scheduler_fallback_hint.py @@ -18,7 +18,15 @@ scheduler_execution_context_for_runtime_profile, ) from loopx.control_plane.scheduler.scheduler_hint import build_scheduler_hint -from loopx.upgrade import resolve_codex_app_automation_rrule +from loopx.upgrade import ( + load_codex_app_automation_manifest, + prompt_digest, + resolve_codex_app_automation_rrule, +) +from scripts.codex_app_apply_rrule import ( + _read_automation_prompt, + _write_automation_toml, +) GOAL_ID = "fallback-hint-goal" AGENT_ID = "codex-fixture" @@ -283,3 +291,89 @@ def test_resolve_codex_app_automation_rrule_returns_automation_id( ) assert result["available"] is True assert result["automation_id"] == "fixture" + + +def test_codex_app_automation_writer_round_trips_multiline_prompt( + tmp_path: Path, +) -> None: + prompt = ( + "Advance `multiline-goal` from active state.\n\n" + "Keep C:\\Users\\alice and an embedded \"\"\" marker.\n" + "Preserve a line ending with a slash \\\n" + "Use `quota should-run --available-capability=network`.\n\n" + "Agent: `multiline-agent`\n" + ) + automation_path = tmp_path / "automations" / "multiline" / "automation.toml" + _write_automation_toml( + automation_path, + automation_id="multiline", + name="Multiline heartbeat", + prompt=prompt, + rrule="FREQ=MINUTELY;INTERVAL=3", + thread_id="multiline-thread", + ) + assert _read_automation_prompt(automation_path) == prompt + + manifest = load_codex_app_automation_manifest(tmp_path) + assert manifest["available"] is True + assert len(manifest["entries"]) == 1 + entry = manifest["entries"][0] + assert entry["goal_id"] == "multiline-goal" + assert entry["agent_id"] == "multiline-agent" + assert entry["target_thread_id"] == "multiline-thread" + assert entry["rrule"] == "FREQ=MINUTELY;INTERVAL=3" + assert entry["prompt_sha256"] == prompt_digest(prompt) + assert entry["char_count"] == len(prompt) + assert entry["line_count"] == len(prompt.splitlines()) + assert entry["available_capabilities"] == ["network"] + + resolved = resolve_codex_app_automation_rrule( + goal_id="multiline-goal", + agent_id="multiline-agent", + thread_id="multiline-thread", + root=tmp_path, + ) + assert resolved == { + "available": True, + "rrule": "FREQ=MINUTELY;INTERVAL=3", + "automation_id": "multiline", + "source": "codex_app_automation_manifest", + } + + +def test_codex_app_automation_manifest_reports_bounded_parse_errors( + tmp_path: Path, +) -> None: + automations = tmp_path / "automations" + for index in range(21): + path = automations / f"invalid-toml-{index:02d}" / "automation.toml" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + 'kind = "heartbeat"\nprompt = "C:\\Users\\alice"\n', + encoding="utf-8", + ) + invalid_utf8 = automations / "00-invalid-utf8" / "automation.toml" + invalid_utf8.parent.mkdir(parents=True, exist_ok=True) + invalid_utf8.write_bytes(b'kind = "heartbeat"\nprompt = "\xff"\n') + + manifest = load_codex_app_automation_manifest(tmp_path) + assert manifest["available"] is True + assert manifest["entries"] == [] + assert manifest["reason"] == "no readable LoopX heartbeat automations discovered" + assert manifest["parse_error_count"] == 22 + assert len(manifest["parse_errors"]) == 20 + assert manifest["parse_errors_complete"] is False + assert {error["reason"] for error in manifest["parse_errors"]} == { + "invalid_utf8", + "invalid_toml", + } + + resolved = resolve_codex_app_automation_rrule( + goal_id="missing-goal", + root=tmp_path, + ) + assert resolved["available"] is False + assert resolved["candidate_count"] == 0 + assert resolved["manifest_parse_error_count"] == 22 + assert len(resolved["manifest_parse_errors"]) == 20 + assert resolved["manifest_parse_errors_complete"] is False diff --git a/tests/test_codex_app_apply_rrule.py b/tests/test_codex_app_apply_rrule.py index 248f5c9595..2963ec81b3 100644 --- a/tests/test_codex_app_apply_rrule.py +++ b/tests/test_codex_app_apply_rrule.py @@ -3,6 +3,7 @@ import json import re import sqlite3 +import tomllib from pathlib import Path import pytest @@ -427,7 +428,9 @@ def fake_run(command, **kwargs): toml_text = toml_path.read_text(encoding="utf-8") assert 'rrule = "FREQ=MINUTELY;INTERVAL=5"' in toml_text assert 'target_thread_id = "thread-1"' in toml_text - assert 'prompt = """Advance `goal` from active state.' in toml_text + assert tomllib.loads(toml_text)["prompt"] == ( + "Advance `goal` from active state. Agent: `agent`." + ) connection = sqlite3.connect(str(db_path)) try: