From f180b822b5cf54e1a936c67938a0539c2592c38a Mon Sep 17 00:00:00 2001 From: Yuan <20144414+baskduf@users.noreply.github.com> Date: Thu, 18 Jun 2026 16:38:22 +0900 Subject: [PATCH] refactor: share FableCodex state helpers Extract shared state, locking, and JSON helpers for the goals and findings ledgers. Strengthen CI workflow tests so the pinned source fetch and coverage validation steps stay semantically linked. --- .github/workflows/ci.yml | 1 + CONTRIBUTING.md | 1 + docs/RELEASING.md | 1 + .../codex-fable5/scripts/codex_fable_state.py | 98 ++++++++++++++++ .../codex-fable5/scripts/codex_findings.py | 105 +++-------------- .../codex-fable5/scripts/codex_goals.py | 107 +++--------------- tests/support.py | 41 +++++++ tests/test_ci_release.py | 27 +++++ tests/test_goals.py | 14 ++- 9 files changed, 213 insertions(+), 182 deletions(-) create mode 100644 plugins/codex-fable5/skills/codex-fable5/scripts/codex_fable_state.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1615788..8d38eeb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,6 +32,7 @@ jobs: - name: Compile scripts run: | python3 -m py_compile \ + plugins/codex-fable5/skills/codex-fable5/scripts/codex_fable_state.py \ plugins/codex-fable5/skills/codex-fable5/scripts/codex_findings.py \ plugins/codex-fable5/skills/codex-fable5/scripts/codex_goals.py \ plugins/codex-fable5/skills/codex-fable5/scripts/fable_coverage.py \ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d943e83..18bbecc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -23,6 +23,7 @@ Before opening a pull request, also run: ```bash python3 -m py_compile \ + plugins/codex-fable5/skills/codex-fable5/scripts/codex_fable_state.py \ plugins/codex-fable5/skills/codex-fable5/scripts/codex_findings.py \ plugins/codex-fable5/skills/codex-fable5/scripts/codex_goals.py \ plugins/codex-fable5/skills/codex-fable5/scripts/fable_coverage.py \ diff --git a/docs/RELEASING.md b/docs/RELEASING.md index 45904bd..f14894b 100644 --- a/docs/RELEASING.md +++ b/docs/RELEASING.md @@ -13,6 +13,7 @@ This project uses a lightweight release process because it is a small Codex plug ```bash python3 -m unittest discover -s tests -v python3 -m py_compile \ + plugins/codex-fable5/skills/codex-fable5/scripts/codex_fable_state.py \ plugins/codex-fable5/skills/codex-fable5/scripts/codex_findings.py \ plugins/codex-fable5/skills/codex-fable5/scripts/codex_goals.py \ plugins/codex-fable5/skills/codex-fable5/scripts/fable_coverage.py \ diff --git a/plugins/codex-fable5/skills/codex-fable5/scripts/codex_fable_state.py b/plugins/codex-fable5/skills/codex-fable5/scripts/codex_fable_state.py new file mode 100644 index 0000000..be75f9b --- /dev/null +++ b/plugins/codex-fable5/skills/codex-fable5/scripts/codex_fable_state.py @@ -0,0 +1,98 @@ +#!/usr/bin/env python3 +"""Shared Codex Fable5 local state helpers.""" + +from __future__ import annotations + +from contextlib import contextmanager +import json +import os +import sys +import tempfile +import time +from datetime import datetime, timezone +from pathlib import Path +from typing import Any, Iterator + +try: + import fcntl +except ImportError: # pragma: no cover - Windows fallback only. + fcntl = None + +STATE_DIR = Path(".codex-fable5") +GOALS_FILE = STATE_DIR / "goals.json" +FINDINGS_FILE = STATE_DIR / "findings.json" +LEDGER_FILE = STATE_DIR / "ledger.jsonl" +LOCK_FILE = STATE_DIR / "state.lock" +LOCK_TIMEOUT_SECONDS = 30.0 + + +def now() -> str: + return datetime.now(timezone.utc).isoformat() + + +@contextmanager +def locked_state() -> Iterator[None]: + STATE_DIR.mkdir(exist_ok=True) + if fcntl is None: + fallback = STATE_DIR / "state.lockdir" + deadline = time.monotonic() + LOCK_TIMEOUT_SECONDS + while True: + try: + fallback.mkdir() + break + except FileExistsError: + if time.monotonic() >= deadline: + sys.exit(f"codex-fable5: timed out waiting for state lock ({fallback}).") + time.sleep(0.05) + try: + yield + finally: + fallback.rmdir() + return + + with LOCK_FILE.open("a", encoding="utf-8") as handle: + fcntl.flock(handle, fcntl.LOCK_EX) + try: + yield + finally: + fcntl.flock(handle, fcntl.LOCK_UN) + + +def read_json(path: Path, label: str) -> Any: + try: + return json.loads(path.read_text(encoding="utf-8")) + except json.JSONDecodeError as exc: + sys.exit( + f"codex-fable5: {label} is not valid JSON " + f"({path}:{exc.lineno}:{exc.colno}: {exc.msg})." + ) + + +def write_json(path: Path, data: dict[str, Any]) -> None: + STATE_DIR.mkdir(exist_ok=True) + tmp_name = "" + with tempfile.NamedTemporaryFile( + "w", + encoding="utf-8", + dir=STATE_DIR, + prefix=f".{path.name}.", + delete=False, + ) as handle: + tmp_name = handle.name + handle.write(json.dumps(data, ensure_ascii=False, indent=2) + "\n") + handle.flush() + os.fsync(handle.fileno()) + Path(tmp_name).replace(path) + + +def append_event(event: str, **fields: Any) -> None: + STATE_DIR.mkdir(exist_ok=True) + record = {"ts": now(), "event": event, **fields} + with LEDGER_FILE.open("a", encoding="utf-8") as handle: + handle.write(json.dumps(record, ensure_ascii=False) + "\n") + + +def require_object(value: Any, path: Path, label: str) -> dict[str, Any]: + if not isinstance(value, dict): + sys.exit(f"codex-fable5: {label} must be a JSON object ({path}).") + return value diff --git a/plugins/codex-fable5/skills/codex-fable5/scripts/codex_findings.py b/plugins/codex-fable5/skills/codex-fable5/scripts/codex_findings.py index 86c5362..0d7bf5d 100755 --- a/plugins/codex-fable5/skills/codex-fable5/scripts/codex_findings.py +++ b/plugins/codex-fable5/skills/codex-fable5/scripts/codex_findings.py @@ -4,27 +4,25 @@ from __future__ import annotations import argparse -from contextlib import contextmanager -import json -import os import re import sys -import tempfile -import time -from datetime import datetime, timezone from pathlib import Path -from typing import Any, Iterator +from typing import Any + +from codex_fable_state import ( + FINDINGS_FILE, + GOALS_FILE, + LEDGER_FILE, + LOCK_FILE, + STATE_DIR, + append_event, + locked_state, + now, + read_json, + require_object, + write_json, +) -try: - import fcntl -except ImportError: # pragma: no cover - Windows fallback only. - fcntl = None - -STATE_DIR = Path(".codex-fable5") -GOALS_FILE = STATE_DIR / "goals.json" -FINDINGS_FILE = STATE_DIR / "findings.json" -LEDGER_FILE = STATE_DIR / "ledger.jsonl" -LOCK_FILE = STATE_DIR / "state.lock" OPEN_STATUSES = {"open"} BLOCKING_STATUSES = {"open", "blocked"} @@ -32,79 +30,6 @@ SEVERITY_ORDER = {"critical": 0, "high": 1, "medium": 2, "low": 3} FINDING_STATUSES = {"open", "blocked", "resolved", "rejected"} FINDING_REQUIRED_FIELDS = {"id", "goal", "title", "severity", "source", "status", "evidence"} -LOCK_TIMEOUT_SECONDS = 30.0 - - -def now() -> str: - return datetime.now(timezone.utc).isoformat() - - -@contextmanager -def locked_state() -> Iterator[None]: - STATE_DIR.mkdir(exist_ok=True) - if fcntl is None: - fallback = STATE_DIR / "state.lockdir" - deadline = time.monotonic() + LOCK_TIMEOUT_SECONDS - while True: - try: - fallback.mkdir() - break - except FileExistsError: - if time.monotonic() >= deadline: - sys.exit(f"codex-fable5: timed out waiting for state lock ({fallback}).") - time.sleep(0.05) - try: - yield - finally: - fallback.rmdir() - return - - with LOCK_FILE.open("a", encoding="utf-8") as handle: - fcntl.flock(handle, fcntl.LOCK_EX) - try: - yield - finally: - fcntl.flock(handle, fcntl.LOCK_UN) - - -def read_json(path: Path, label: str) -> Any: - try: - return json.loads(path.read_text(encoding="utf-8")) - except json.JSONDecodeError as exc: - sys.exit( - f"codex-fable5: {label} is not valid JSON " - f"({path}:{exc.lineno}:{exc.colno}: {exc.msg})." - ) - - -def write_json(path: Path, data: dict[str, Any]) -> None: - STATE_DIR.mkdir(exist_ok=True) - tmp_name = "" - with tempfile.NamedTemporaryFile( - "w", - encoding="utf-8", - dir=STATE_DIR, - prefix=f".{path.name}.", - delete=False, - ) as handle: - tmp_name = handle.name - handle.write(json.dumps(data, ensure_ascii=False, indent=2) + "\n") - handle.flush() - os.fsync(handle.fileno()) - Path(tmp_name).replace(path) - - -def append_event(event: str, **fields: Any) -> None: - STATE_DIR.mkdir(exist_ok=True) - record = {"ts": now(), "event": event, **fields} - with LEDGER_FILE.open("a", encoding="utf-8") as handle: - handle.write(json.dumps(record, ensure_ascii=False) + "\n") - - -def require_object(value: Any, path: Path, label: str) -> dict[str, Any]: - if not isinstance(value, dict): - sys.exit(f"codex-fable5: {label} must be a JSON object ({path}).") - return value def validate_findings(data: dict[str, Any], path: Path, label: str) -> dict[str, Any]: diff --git a/plugins/codex-fable5/skills/codex-fable5/scripts/codex_goals.py b/plugins/codex-fable5/skills/codex-fable5/scripts/codex_goals.py index 8d4699c..ad2b7ee 100755 --- a/plugins/codex-fable5/skills/codex-fable5/scripts/codex_goals.py +++ b/plugins/codex-fable5/skills/codex-fable5/scripts/codex_goals.py @@ -4,26 +4,24 @@ from __future__ import annotations import argparse -from contextlib import contextmanager -import json -import os import sys -import tempfile -import time -from datetime import datetime, timezone from pathlib import Path -from typing import Any, Iterator - -try: - import fcntl -except ImportError: # pragma: no cover - Windows fallback only. - fcntl = None - -STATE_DIR = Path(".codex-fable5") -GOALS_FILE = STATE_DIR / "goals.json" -FINDINGS_FILE = STATE_DIR / "findings.json" -LEDGER_FILE = STATE_DIR / "ledger.jsonl" -LOCK_FILE = STATE_DIR / "state.lock" +from typing import Any + +from codex_fable_state import ( + FINDINGS_FILE, + GOALS_FILE, + LEDGER_FILE, + LOCK_FILE, + STATE_DIR, + append_event, + locked_state, + now, + read_json, + require_object, + write_json, +) + OPEN_STATUSES = {"pending", "in_progress"} INCOMPLETE_TERMINAL_STATUSES = {"failed", "blocked"} BLOCKING_FINDING_STATUSES = {"open", "blocked"} @@ -31,85 +29,12 @@ GOAL_REQUIRED_FIELDS = {"id", "title", "objective", "status", "evidence", "verify_cmd", "verify_evidence"} FINDING_STATUSES = {"open", "blocked", "resolved", "rejected"} FINDING_REQUIRED_FIELDS = {"id", "goal", "title", "severity", "source", "status", "evidence"} -LOCK_TIMEOUT_SECONDS = 30.0 - - -def now() -> str: - return datetime.now(timezone.utc).isoformat() def safe_stamp() -> str: return now().replace(":", "").replace("+", "Z") -@contextmanager -def locked_state() -> Iterator[None]: - STATE_DIR.mkdir(exist_ok=True) - if fcntl is None: - fallback = STATE_DIR / "state.lockdir" - deadline = time.monotonic() + LOCK_TIMEOUT_SECONDS - while True: - try: - fallback.mkdir() - break - except FileExistsError: - if time.monotonic() >= deadline: - sys.exit(f"codex-fable5: timed out waiting for state lock ({fallback}).") - time.sleep(0.05) - try: - yield - finally: - fallback.rmdir() - return - - with LOCK_FILE.open("a", encoding="utf-8") as handle: - fcntl.flock(handle, fcntl.LOCK_EX) - try: - yield - finally: - fcntl.flock(handle, fcntl.LOCK_UN) - - -def read_json(path: Path, label: str) -> Any: - try: - return json.loads(path.read_text(encoding="utf-8")) - except json.JSONDecodeError as exc: - sys.exit( - f"codex-fable5: {label} is not valid JSON " - f"({path}:{exc.lineno}:{exc.colno}: {exc.msg})." - ) - - -def write_json(path: Path, data: dict[str, Any]) -> None: - STATE_DIR.mkdir(exist_ok=True) - tmp_name = "" - with tempfile.NamedTemporaryFile( - "w", - encoding="utf-8", - dir=STATE_DIR, - prefix=f".{path.name}.", - delete=False, - ) as handle: - tmp_name = handle.name - handle.write(json.dumps(data, ensure_ascii=False, indent=2) + "\n") - handle.flush() - os.fsync(handle.fileno()) - Path(tmp_name).replace(path) - - -def append_event(event: str, **fields: Any) -> None: - STATE_DIR.mkdir(exist_ok=True) - record = {"ts": now(), "event": event, **fields} - with LEDGER_FILE.open("a", encoding="utf-8") as handle: - handle.write(json.dumps(record, ensure_ascii=False) + "\n") - - -def require_object(value: Any, path: Path, label: str) -> dict[str, Any]: - if not isinstance(value, dict): - sys.exit(f"codex-fable5: {label} must be a JSON object ({path}).") - return value - - def validate_plan(data: dict[str, Any], path: Path, label: str) -> dict[str, Any]: if not isinstance(data.get("brief"), str): sys.exit(f"codex-fable5: {label} field 'brief' must be a string ({path}).") diff --git a/tests/support.py b/tests/support.py index 25added..0b0c73a 100644 --- a/tests/support.py +++ b/tests/support.py @@ -17,14 +17,19 @@ SKILL_ROOT = ROOT / "plugins" / "codex-fable5" / "skills" / "codex-fable5" SCRIPTS = SKILL_ROOT / "scripts" BIN = ROOT / "plugins" / "codex-fable5" / "bin" +if str(SCRIPTS) not in sys.path: + sys.path.insert(0, str(SCRIPTS)) def load_script(name: str): + if name in sys.modules: + return sys.modules[name] path = SCRIPTS / f"{name}.py" spec = importlib.util.spec_from_file_location(name, path) if spec is None or spec.loader is None: raise RuntimeError(f"could not load {path}") module = importlib.util.module_from_spec(spec) + sys.modules[name] = module spec.loader.exec_module(module) return module @@ -64,9 +69,45 @@ def read_readme_fable_pin() -> str: return match.group(1) +def parse_ci_workflow_steps(workflow: str) -> dict[str, str]: + steps: dict[str, str] = {} + matches = list( + re.finditer( + r"^ - name: (?P.+?)\r?\n(?P.*?)(?=^ - name: |\Z)", + workflow, + re.DOTALL | re.MULTILINE, + ) + ) + for match in matches: + steps[match.group("name")] = match.group("body") + return steps + + +def extract_ci_pin(step_body: str) -> str: + match = re.search(r'PIN="([0-9a-f]{40})"', step_body) + if match is None: + raise AssertionError("workflow step does not define a 40-character PIN") + return match.group(1) + + +def extract_ci_output_path(step_body: str) -> str: + match = re.search(r"\s-o\s+([^\s]+)", step_body) + if match is None: + raise AssertionError("workflow fetch step does not write to a concrete -o path") + return match.group(1).strip('"') + + +def extract_fable_coverage_source_arg(step_body: str) -> str: + match = re.search(r"fable_coverage\.py\s+--source\s+([^\s]+)", step_body) + if match is None: + raise AssertionError("workflow validate step does not pass --source to fable_coverage.py") + return match.group(1).strip('"') + + class ScriptTestBase(unittest.TestCase): @classmethod def setUpClass(cls) -> None: + cls.codex_fable_state = load_script("codex_fable_state") cls.fable_coverage = load_script("fable_coverage") cls.codex_goals = load_script("codex_goals") cls.codex_findings = load_script("codex_findings") diff --git a/tests/test_ci_release.py b/tests/test_ci_release.py index f86ede2..fcaaf90 100644 --- a/tests/test_ci_release.py +++ b/tests/test_ci_release.py @@ -10,7 +10,11 @@ ScriptTestBase, SimpleNamespace, json, + extract_ci_output_path, + extract_ci_pin, + extract_fable_coverage_source_arg, os, + parse_ci_workflow_steps, parse_routing_map, read_readme_fable_pin, read_skill_body, @@ -30,7 +34,11 @@ ScriptTestBase, SimpleNamespace, json, + extract_ci_output_path, + extract_ci_pin, + extract_fable_coverage_source_arg, os, + parse_ci_workflow_steps, parse_routing_map, read_readme_fable_pin, read_skill_body, @@ -50,6 +58,7 @@ def test_ci_workflow_runs_project_verification(self) -> None: self.assertIn("actions/setup-python@v6", workflow) self.assertIn("python3 -m unittest discover -s tests -v", workflow) self.assertIn("python3 -m py_compile", workflow) + self.assertIn("codex_fable_state.py", workflow) self.assertIn("codex_findings.py", workflow) self.assertIn("sh -n plugins/codex-fable5/bin/codex-fable5", workflow) self.assertIn("sh -n plugins/codex-fable5/bin/codex-findings", workflow) @@ -103,6 +112,24 @@ def test_ci_workflow_validates_against_pinned_source(self) -> None: "so the matrix is validated against the upstream FABLE-5 source", ) + def test_ci_workflow_fetch_and_validate_steps_are_semantically_linked(self) -> None: + workflow = (ROOT / ".github" / "workflows" / "ci.yml").read_text(encoding="utf-8") + steps = parse_ci_workflow_steps(workflow) + + fetch = steps["Fetch pinned FABLE-5 source"] + validate = steps["Validate coverage matrix"] + fetched_path = extract_ci_output_path(fetch) + + self.assertEqual("build/fable5/CLAUDE-FABLE-5.md", fetched_path) + self.assertEqual(read_readme_fable_pin(), extract_ci_pin(fetch)) + self.assertEqual( + fetched_path, + extract_fable_coverage_source_arg(validate), + "CI must validate the exact pinned source file it fetched, not a different path or matrix-only run", + ) + self.assertIn("grep -q \"Fable 5\"", fetch) + self.assertNotIn("Authorization:", fetch) + def test_release_checklist_validates_against_pinned_source(self) -> None: releasing = (ROOT / "docs" / "RELEASING.md").read_text(encoding="utf-8") diff --git a/tests/test_goals.py b/tests/test_goals.py index de054e3..4399f51 100644 --- a/tests/test_goals.py +++ b/tests/test_goals.py @@ -43,6 +43,18 @@ class GoalLedgerTests(ScriptTestBase): + def test_goal_and_findings_scripts_share_state_helpers(self) -> None: + state = self.codex_fable_state + for helper in ["locked_state", "read_json", "write_json", "append_event", "require_object", "now"]: + with self.subTest(helper=helper): + self.assertIs(getattr(self.codex_goals, helper), getattr(state, helper)) + self.assertIs(getattr(self.codex_findings, helper), getattr(state, helper)) + + for constant in ["STATE_DIR", "GOALS_FILE", "FINDINGS_FILE", "LEDGER_FILE", "LOCK_FILE"]: + with self.subTest(constant=constant): + self.assertEqual(getattr(self.codex_goals, constant), getattr(state, constant)) + self.assertEqual(getattr(self.codex_findings, constant), getattr(state, constant)) + def test_goal_ledger_flow_and_final_verification_gate(self) -> None: script = SCRIPTS / "codex_goals.py" with tempfile.TemporaryDirectory() as tmp: @@ -242,7 +254,7 @@ def run(*args: str) -> subprocess.CompletedProcess[str]: self.assertIn("Reopened G001 from blocked", reopened.stdout) def test_lock_fallback_times_out_on_stale_lockdir(self) -> None: - for module in [self.codex_findings, self.codex_goals]: + for module in [self.codex_fable_state]: with self.subTest(module=module.__name__): with tempfile.TemporaryDirectory() as tmp: cwd = Path(tmp)