From f72f838933b0bf90969ad472c19b3ba53555b290 Mon Sep 17 00:00:00 2001 From: Dan Date: Sat, 1 Aug 2026 22:02:44 +0100 Subject: [PATCH] Recover interrupted skill delivery (#59) Closes #55 --- CHANGELOG.md | 1 + plugins/violin_guard/skill_receipts.py | 63 ++++++++++++++++++++++-- tests/guard/state/test_skill_receipts.py | 18 +++++++ 3 files changed, 78 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b49caab..4bf54d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## 3.0.0 +- Made interrupted skill preparation recoverable with expiring reservations and stale-owner protection; batch review now remains tied to the delivered execution receipt. - Stabilized the core engagement workflow: domain/URL-only scopes now validate, runtime execution cannot substitute another scope file, PTT/review CLI contracts carry skill metadata, and review reuses the active delivered binding. - Added receipt-backed skill routing, delivery, task binding, browser enforcement, Kali auto-backend selection, proof-based finding review, and semantic anti-stuck enforcement. - Replaced marker-file authorization with two-turn skill preparation and receipt diagnostics; legacy markers can only infer a unique session ID during migration. diff --git a/plugins/violin_guard/skill_receipts.py b/plugins/violin_guard/skill_receipts.py index 18c4eef..26d3b91 100644 --- a/plugins/violin_guard/skill_receipts.py +++ b/plugins/violin_guard/skill_receipts.py @@ -9,6 +9,7 @@ from __future__ import annotations import hashlib import json +import uuid from collections.abc import Callable from dataclasses import dataclass from datetime import UTC, datetime @@ -38,6 +39,7 @@ __all__ = [ _FILE_NAME = "skills.json" _SCHEMA_VERSION = 1 _MAX_DELIVERIES = 200 +_PREPARING_TTL_SECONDS = 300 def _now() -> str: @@ -48,6 +50,33 @@ def _digest(value: str) -> str: return "sha256:" + hashlib.sha256(value.encode("utf-8")).hexdigest() +def _preparing_expired(entry: dict[str, Any]) -> bool: + value = str(entry.get("expires_at") or "").strip() + if value: + try: + return datetime.fromisoformat(value.replace("Z", "+00:00")) <= datetime.now(UTC) + except ValueError: + return True + updated = str(entry.get("updated_at") or entry.get("created_at") or "").strip() + if not updated: + return True + try: + started = datetime.fromisoformat(updated.replace("Z", "+00:00")) + except ValueError: + return True + return (datetime.now(UTC) - started).total_seconds() >= _PREPARING_TTL_SECONDS + + +def _preparing_expires_at() -> str: + from datetime import timedelta + + return ( + (datetime.now(UTC) + timedelta(seconds=_PREPARING_TTL_SECONDS)) + .isoformat() + .replace("+00:00", "Z") + ) + + def _path(eng_dir: str | Path) -> Path: return state.resolve_eng_dir(eng_dir) / "state" / _FILE_NAME @@ -130,6 +159,7 @@ class DeliveryReservation: session_id: str context_generation: int owner: bool + owner_token: str = "" @dataclass(frozen=True) @@ -192,7 +222,7 @@ def prepare_delivery( current_session, generation = _context(data, session_id.strip()) identifier = _delivery_key(current_session, generation, skill, bundle_digest) existing = data["deliveries"].get(identifier) - if existing and existing.get("status") in {"preparing", "delivered"}: + if existing and existing.get("status") == "delivered": return DeliveryReservation( identifier, existing["status"], @@ -202,6 +232,18 @@ def prepare_delivery( generation, False, ) + if existing and existing.get("status") == "preparing" and not _preparing_expired(existing): + return DeliveryReservation( + identifier, + existing["status"], + skill, + bundle_digest, + current_session, + generation, + False, + ) + owner_token = uuid.uuid4().hex + now = _now() data["deliveries"][identifier] = { "id": identifier, "skill": skill, @@ -209,13 +251,22 @@ def prepare_delivery( "session_id": current_session, "context_generation": generation, "status": "preparing", - "created_at": _now(), - "updated_at": _now(), + "created_at": now, + "updated_at": now, + "expires_at": _preparing_expires_at(), + "owner_token": owner_token, "attempts": int((existing or {}).get("attempts") or 0) + 1, } _prune(data) return DeliveryReservation( - identifier, "preparing", skill, bundle_digest, current_session, generation, True + identifier, + "preparing", + skill, + bundle_digest, + current_session, + generation, + True, + owner_token, ) return _mutate(eng_dir, reserve) @@ -234,6 +285,10 @@ def complete_delivery( entry = data["deliveries"].get(reservation.id) if not entry or entry.get("status") != "preparing": raise ValueError("delivery reservation is no longer active") + if not reservation.owner or not reservation.owner_token: + raise ValueError("only the reservation owner may complete delivery") + if entry.get("owner_token") != reservation.owner_token: + raise ValueError("delivery reservation owner is stale; prepare a new delivery") entry["status"] = "delivered" if result.ready else "failed" entry["updated_at"] = _now() entry["delivered_turn_id"] = delivered_turn_id if result.ready else None diff --git a/tests/guard/state/test_skill_receipts.py b/tests/guard/state/test_skill_receipts.py index 1dc7a91..5f7ad3d 100644 --- a/tests/guard/state/test_skill_receipts.py +++ b/tests/guard/state/test_skill_receipts.py @@ -59,6 +59,24 @@ def test_concurrent_duplicate_only_has_one_owner(tmp_path: Path) -> None: assert duplicate.status == "preparing" +def test_expired_preparation_is_reclaimed_and_old_owner_cannot_complete(tmp_path: Path) -> None: + first = _reserve(tmp_path) + state_path = tmp_path / "state" / "skills.json" + data = json.loads(state_path.read_text(encoding="utf-8")) + data["deliveries"][first.id]["expires_at"] = "2000-01-01T00:00:00Z" + state_path.write_text(json.dumps(data), encoding="utf-8") + + reclaimed = _reserve(tmp_path) + assert reclaimed.owner + assert reclaimed.owner_token != first.owner_token + + with pytest.raises(ValueError, match="stale"): + complete_delivery(tmp_path, first, SkillViewResult(True, content="# old")) + + delivered = complete_delivery(tmp_path, reclaimed, SkillViewResult(True, content="# new")) + assert delivered.status == "delivered" + + def test_context_reset_requires_a_new_delivery(tmp_path: Path) -> None: old = _deliver(tmp_path) assert advance_context_generation(tmp_path, "session-a") == 1