fix(gate): re-prove authority at the commit point (gauntlet revoke-during-spend)

CHANGELOG.md

@@ -7,7 +7,44 @@ the current version is `interlock.__version__` (mirrored into `VERSION` by
 `scripts/release.sh`).
 
 ## [Unreleased]
+### Added
+- `Interlock.execute(lease, action, target, effect)`: the gate is re-proven at the commit
+  point, atomically with registering the effect as in flight. `EStop.engage()` and
+  `LeaseManager.revoke()` are ordered against it and return only after committed effects
+  finish, closing the check-then-act race where an e-stop or revocation landing during a
+  slow preparation still let the effect run (gauntlet `revoke-during-spend`). `permit()`
+  keeps its entry-only check and now appends a `violations` record if authority is lost
+  while the block runs. Ledger `permits`/`denials` records gain a `phase` field (`entry`
+  or `commit`). Regression tests: `tests/test_inflight.py`.
+
 ### Fixed
+- Codex review fixes on the commit-point gate (PR #18), each with a regression test in
+  `tests/test_inflight.py` unless noted:
+  - Gating fault reports are ordered against commit registration: `FaultBus` takes the
+    in-flight gate's lock on `report()`/`clear()`, so a major/critical fault can no
+    longer land between `execute()`'s check and its registration and be missed.
+  - Concurrent halts raised from inside effects no longer deadlock: a draining halt
+    does not wait on effects whose threads are themselves inside a halt. Applies to
+    `estop.engage()` and to mutual in-effect `leases.revoke()` calls.
+  - `Ledger` appends and `close()` are serialised with an internal lock, so concurrent
+    `execute()` calls can no longer write duplicate series descriptors or leave the
+    closing index undercounting records.
+  - `permit()` records a `violations` entry when the e-stop is engaged and released
+    again inside the block; the exit check compares `EStop.engagements` counts instead
+    of only the final `is_engaged` state.
+  - `execute()` refuses deferred effects: coroutine/generator callables raise
+    `TypeError` before the gate runs, and a deferred result from a plain callable is
+    closed unstarted. Previously the callable merely created the deferred object, the
+    in-flight registration was dropped immediately, and the body could run after a
+    halt had returned.
+  - `execute()` establishes its in-flight cleanup guard before registration, so an
+    asynchronous exception in the gap after `enter()` cannot leak a stale in-flight
+    entry that blocks later halts forever (structural fix; the interleaving needs an
+    async exception in a two-step window and has no deterministic regression test).
+  - The two scheduled-halt tests in `tests/test_inflight.py` synchronize on an event
+    instead of assuming a `threading.Timer` fired within a fixed sleep.
+  - README and the quickstart guide route irreversible effects through `execute()`
+    instead of `permit()` blocks.
 - `Ledger(path)` no longer erases an existing audit log. A path opens in append mode by
   default, so reopening a ledger keeps every earlier run and starts a new self-describing
   segment. `Ledger(path, mode="w")` truncates deliberately; any other mode raises

README.md

@@ -10,7 +10,7 @@ Mobile robots solved this a decade ago. Boston Dynamics' Spot admits intent thro
 
 ## What it guarantees
 
-Inside the substrate, **no action executes without a live lease, a clear e-stop, and no gating fault.** `Interlock.permit()` checks in this order and raises on the first failure:
+Inside the substrate, **no action executes without a live lease, a clear e-stop, and no gating fault.** The gate — `Interlock.execute()` at the commit point, `Interlock.permit()` at block entry — checks in this order and raises on the first failure:
 
 1. e-stop clear, else `EStopEngaged`
 2. lease issued by this `Interlock` and unaltered, else `LeaseNotIssued`; then unrevoked and unexpired, else `LeaseRevoked` or `LeaseExpired`
@@ -41,9 +41,10 @@ ilk = Interlock()
 # GRANT: authority comes into existence only here, and it expires.
 lease = ilk.grant(resource="docs:write", holder="research-agent", ttl=1800)
 
-# EXERCISE: every action passes the gate.
-with ilk.permit(lease, action="write", target="brief.md"):
-    ...  # raises EStopEngaged, LeaseNotIssued, LeaseRevoked, LeaseExpired, or FaultActive otherwise
+# EXERCISE: every action passes the gate. execute() re-proves the gate at
+# the commit point, atomically with starting the effect — a halt that lands
+# while the action is being prepared refuses it instead of racing it.
+ilk.execute(lease, "write", "brief.md", effect)  # raises EStopEngaged, LeaseNotIssued, LeaseRevoked, LeaseExpired, or FaultActive otherwise
 
 # A major or critical fault refuses further permits on the affected resource.
 ilk.faults.report(code="TOOL_TIMEOUT", severity="major", resource="web:read")
@@ -66,14 +67,14 @@ The fastest path to a first lease is [`docs/guides/quickstart.md`](docs/guides/q
 
 | Condition | Behavior |
 |---|---|
-| E-stop engaged | Every `permit()` raises `EStopEngaged` and logs a `denials` record |
+| E-stop engaged | Every gate check raises `EStopEngaged` and logs a `denials` record |
 | Lease expired (no keepalive in time) | `LeaseExpired`. A late keepalive also raises |
 | Lease not issued by this `Interlock` (hand-built, from another instance, or edited after issue) | `LeaseNotIssued` and a `denials` record with reason `lease_not_issued` |
 | Lease revoked | `LeaseRevoked`, permanently. Resetting `lease.revoked` on the object does not restore it |
 | `major`/`critical` fault on the resource | `FaultActive` until the fault is cleared. `minor` faults are recorded but do not gate |
 
 **Known limits:**
-- The gate runs at `permit()` entry. An e-stop engaged *during* a `with` block does not interrupt code already running inside it; it refuses the next permit.
+- `permit()` checks at block entry only. An e-stop engaged *during* a `with` block does not interrupt code already running inside it; the block's exit logs a `violations` record and the next permit is refused. For an effect that must not start once authority is gone, use `execute()`: it re-proves the gate at the commit point, and an e-stop or revocation returns only after already-committed effects have finished.
 - State is in-process and in-memory. Interlock gates actions routed through it and cannot stop an agent that bypasses the substrate. It is one layer of defense in depth.
 - The ledger is append-only and self-describing JSONL (format descriptor, per-series schemas, closing index). Reopening a path appends a new segment and never truncates unless you pass `mode="w"`. Every record is flushed as it is written, so it survives a crash of the writing process. Only `close()` fsyncs, so a power loss can still drop the last records. It is **not** hash-chained or signed, so treat it as an audit log, not tamper evidence.
 

docs/guides/quickstart.md

@@ -49,15 +49,21 @@ checks the e-stop, lease state, and resource faults at the time of entry.
 ## 3. Exercise it through the gate
 
 ```python
-with ilk.permit(lease, action="write", target="notes.md"):
-    Path("notes.md").write_text("hello")
+ilk.execute(lease, "write", "notes.md", lambda: Path("notes.md").write_text("hello"))
 ```
 
 The gate checks, in order: e-stop clear, lease issued by this `Interlock`
-and live, and no gating fault on `docs:write`. If any check fails, the
-context manager raises instead of yielding: `EStopEngaged`,
-`LeaseNotIssued`, `LeaseExpired`/`LeaseRevoked`, or `FaultActive`. The check
-controls admission to the context. It is not an OS sandbox, and it cannot
+and live, and no gating fault on `docs:write`. If any check fails,
+`execute()` raises instead of running the effect: `EStopEngaged`,
+`LeaseNotIssued`, `LeaseExpired`/`LeaseRevoked`, or `FaultActive`.
+
+`execute()` runs the check at the commit point — atomically with starting
+the effect — so an e-stop or revocation that lands while the effect is
+being prepared still refuses it, and a halt call returns only after
+already-committed effects have finished. This is the path for any effect
+that cannot be taken back. (`permit()` also exists as a context manager,
+but it checks once at block entry only; use it solely for work that is
+safe to abandon mid-block.) The gate is not an OS sandbox, and it cannot
 interrupt an action that has already started.
 
 ## 4. Watch a fault refuse
@@ -68,17 +74,15 @@ fault = ilk.faults.report(
 )
 
 try:
-    with ilk.permit(lease, action="write", target="notes.md"):
-        Path("notes.md").write_text("should not be admitted")
+    ilk.execute(lease, "write", "notes.md", lambda: Path("notes.md").write_text("no"))
 except FaultActive:
     pass
 else:
     raise AssertionError("the active fault must refuse admission")
 
 assert fault.id is not None
 ilk.faults.clear(fault.id)
-with ilk.permit(lease, action="write", target="notes.md"):
-    Path("notes.md").write_text("resumed")
+ilk.execute(lease, "write", "notes.md", lambda: Path("notes.md").write_text("resumed"))
 ```
 
 The refused entry is appended to the ledger. Clearing the fault permits a
@@ -89,8 +93,8 @@ new admission; it does not retroactively authorize the refused action.
 ```python
 ilk.estop.engage("taking a look")
 try:
-    with ilk.permit(lease, action="write", target="notes.md"):
-        raise AssertionError("e-stop must refuse admission")
+    ilk.execute(lease, "write", "notes.md", lambda: None)
+    raise AssertionError("e-stop must refuse admission")
 except EStopEngaged:
     pass
 ilk.estop.release()  # deliberate, never automatic

src/interlock/__init__.py

@@ -11,8 +11,10 @@
 facade and live, no gating fault on the lease's resource.
 """
 
+import inspect
 from collections.abc import Callable, Iterator
 from contextlib import contextmanager
+from typing import TypeVar
 
 from interlock._errors import (
     EStopEngaged,
@@ -23,12 +25,14 @@
     LeaseRevoked,
 )
 from interlock.claims import Claim
-from interlock.estop import EStop
+from interlock.estop import EStop, InFlightGate
 from interlock.faults import FaultBus
 from interlock.interfaces import REGISTRY, InterfaceRecord, declare
 from interlock.leases import Lease, LeaseManager
 from interlock.ledger import Ledger
 
+T = TypeVar("T")
+
 __version__ = "0.1.0"
 
 __all__ = [
@@ -60,9 +64,10 @@ def __init__(
         clock: Callable[[], float] | None = None,
         ledger: Ledger | None = None,
     ) -> None:
-        self.leases = LeaseManager(clock)
-        self.estop = EStop()
-        self.faults = FaultBus()
+        self._gate = InFlightGate()
+        self.leases = LeaseManager(clock, gate=self._gate)
+        self.estop = EStop(self._gate)
+        self.faults = FaultBus(gate=self._gate)
         self.ledger = ledger if ledger is not None else Ledger()
 
     def grant(
@@ -92,24 +97,15 @@ def grant(
         )
         return lease
 
-    @contextmanager
-    def permit(self, lease: Lease, action: str, target: str) -> Iterator[Lease]:
-        """Permit exactly one action on one target, checking in order:
-
-        1. e-stop clear → else :exc:`EStopEngaged`
-        2. lease issued by this facade's manager and unaltered → else
-           :exc:`LeaseNotIssued`; then live (unrevoked, unexpired) → else
-           :exc:`LeaseRevoked` or :exc:`LeaseExpired`
-        3. no gating fault on the lease's resource → else :exc:`FaultActive`
-
-        Permits and denials are appended to the ledger.
-        """
+    def _check(self, lease: Lease, action: str, target: str, phase: str) -> dict:
+        """Run the gate in order; log and raise on the first failure."""
         base = {
             "lease_id": lease.id,
             "holder": lease.holder,
             "resource": lease.resource,
             "action": action,
             "target": target,
+            "phase": phase,
         }
         try:
             self.estop.assert_clear()
@@ -130,8 +126,93 @@ def permit(self, lease: Lease, action: str, target: str) -> Iterator[Lease]:
         if self.faults.gating(lease.resource):
             self.ledger.append("denials", {**base, "reason": "fault"})
             raise FaultActive(f"gating fault active on {lease.resource}")
+        return base
+
+    @contextmanager
+    def permit(self, lease: Lease, action: str, target: str) -> Iterator[Lease]:
+        """Permit exactly one action on one target, checking in order:
+
+        1. e-stop clear → else :exc:`EStopEngaged`
+        2. lease issued by this facade's manager and unaltered → else
+           :exc:`LeaseNotIssued`; then live (unrevoked, unexpired) → else
+           :exc:`LeaseRevoked` or :exc:`LeaseExpired`
+        3. no gating fault on the lease's resource → else :exc:`FaultActive`
+
+        Permits and denials are appended to the ledger.
+
+        ``permit()`` checks at block entry only: it cannot stop an effect the
+        caller performs later inside the block. For irreversible effects use
+        :meth:`execute`, which re-proves the gate at the commit point. If the
+        e-stop engages or the lease is revoked while the block runs, a
+        ``violations`` record is appended on exit so the gap is never silent.
+        An engagement counts even if the e-stop was released again before
+        the block exited — the exit check compares engagement counts, not
+        the final state.
+        """
+        base = self._check(lease, action, target, phase="entry")
         self.ledger.append("permits", base)
-        yield lease
+        engagements = self.estop.engagements
+        try:
+            yield lease
+        finally:
+            reason = None
+            if self.estop.engagements != engagements:
+                reason = "estop"
+            elif self.leases.is_revoked(lease):
+                reason = "lease_revoked"
+            if reason:
+                self.ledger.append("violations", {**base, "reason": reason})
+
+    def execute(self, lease: Lease, action: str, target: str, effect: Callable[[], T]) -> T:
+        """Run ``effect`` only if the gate is clear at the commit point.
+
+        Fail-closed: the gate (e-stop, lease issued/live/unrevoked, faults) is
+        checked and the effect registered as in flight atomically, under the
+        same lock :meth:`EStop.engage` and :meth:`LeaseManager.revoke` take.
+        Either the halt lands first and the effect never starts, or the effect
+        started first and the halt call waits for it to finish. Any error in
+        the check refuses the effect.
+
+        ``effect`` must run to completion synchronously. A coroutine,
+        generator, or async generator is deferred work: calling it starts
+        nothing, so the gate cannot hold it in flight. Deferred callables
+        are refused with :exc:`TypeError` before the gate runs, and a
+        deferred result returned by a plain callable is closed unstarted
+        and refused the same way — never left to run after a halt returns.
+        """
+        if (
+            inspect.iscoroutinefunction(effect)
+            or inspect.isgeneratorfunction(effect)
+            or inspect.isasyncgenfunction(effect)
+        ):
+            raise TypeError(
+                "execute() effect must be a plain synchronous callable; "
+                "async and generator effects are refused"
+            )
+        registered = False
+        try:
+            with self._gate.cond:
+                base = self._check(lease, action, target, phase="commit")
+                self._gate.enter(lease.id)
+                registered = True
+            self.ledger.append("permits", base)
+            result = effect()
+            if (
+                inspect.iscoroutine(result)
+                or inspect.isgenerator(result)
+                or inspect.isasyncgen(result)
+            ):
+                close = getattr(result, "close", None)
+                if close is not None:
+                    close()
+                raise TypeError(
+                    "execute() effect returned deferred work (coroutine or generator); "
+                    "it was closed unstarted and refused"
+                )
+            return result
+        finally:
+            if registered:
+                self._gate.leave(lease.id)
 
     def claim(
         self,