diff --git a/CHANGELOG.md b/CHANGELOG.md index 0150cb8bc..b5e6ee51d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -117,6 +117,13 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). and all Layer 3 language vocabulary types (Python, TypeScript, Rust, Java). The new page is linked from the API Reference index and the MkDocs navigation. +- **LockService Advisory Locking Specification** (#9283): Added `LockService` advisory locking + documentation to the `Storage and Persistence` section of `docs/specification.md`. Documents + plan-level and project-level lock semantics, the unique-UUID `owner_id` pattern for preventing + re-entrant acquisition, the `finally`-block release pattern, `LockConflictError` behavior, TTL + constants, startup cleanup, graceful shutdown, and the relationship to optimistic concurrency + control. Addresses spec gap identified in issue #9283 ([AUTO-SPEC-1]). + ### Changed - **Decision Tree Full ULID Display** (#5825): The `agents plan tree` command now diff --git a/docs/specification.md b/docs/specification.md index f0803345d..3b3f178f0 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -45873,6 +45873,111 @@ The relational database follows a normalized design with foreign key constraints CREATE INDEX idx_audit_created ON audit_log(created_at); +#### Advisory Locking (LockService) + +!!! note "Spec Gap Addressed" + This section documents the `LockService` advisory locking system, which complements the optimistic concurrency control described in [Database Schema Design](#database-schema-design). Addresses spec gap identified in issue #9283 ([AUTO-SPEC-1]). Implemented in commits b83f575c and b1f7b51a. + +In addition to the optimistic `updated_at` version check, CleverAgents implements **advisory locking** via `LockService` to prevent concurrent mutations of the same plan or project across multiple CLI sessions or worker processes. + +##### Overview + +`LockService` provides mutual-exclusion primitives for two resource types: **plan** and **project**. Each resource can hold at most one active lock at any time. Locks are identified by the tuple `(resource_type, resource_id)` and scoped to an **owner** (a UUID generated per invocation). + +`LockService` is registered as a **`Singleton`** in the DI container, ensuring a single shared instance manages all lock state within a process. + +##### Lock Lifecycle + +```text +acquire --> active --> release / expire + | + +--> renew (extend TTL) +``` + +1. **Acquire**: Creates a new lock row or re-acquires for the same owner (extending TTL). +2. **Renew**: Extends the TTL of an active lock. +3. **Release**: Explicitly removes the lock. +4. **Expire**: The lock becomes stale after `expires_at` passes; expired locks are transparently replaced on the next acquire. + +##### Plan-Level Locking in `execute_plan()` and `apply_plan()` + +Both `execute_plan()` and `apply_plan()` acquire a **plan-level advisory lock** before entering the critical section (phase transition + persistence). The pattern is: + +```python +owner_id = str(uuid4()) # unique per invocation — prevents re-entrant acquisition +lock_service.acquire(resource_type="plan", resource_id=plan_id, owner_id=owner_id) +try: + # critical section: phase transition + persistence + ... +finally: + lock_service.release(resource_type="plan", resource_id=plan_id, owner_id=owner_id) +``` + +Key properties of this pattern: + +- **Unique `owner_id` per invocation**: Each call to `execute_plan()` or `apply_plan()` generates a fresh UUID as `owner_id`. This prevents re-entrant acquisition — if the same process somehow calls the function recursively, the second call will see a different `owner_id` and will be blocked rather than silently extending the existing lock. +- **`finally`-block release**: The lock is always released in a `finally` block, ensuring cleanup even when the critical section raises an exception. This prevents lock leakage on error paths. +- **Concurrent conflict raises `LockConflictError`**: If a different session (different `owner_id`) attempts to acquire a lock on the same plan while it is already held, `LockService` raises `LockConflictError` with details about the current holder. The caller surfaces this as a user-visible error rather than silently racing. + +##### Project-Level Locking + +`LockService` also supports project-level locks (`resource_type="project"`) for operations that must be serialized at the project scope. The same acquire/release pattern applies. + +##### Relationship to Optimistic Concurrency Control + +Advisory locking and optimistic concurrency control serve complementary roles: + +| Mechanism | Scope | Purpose | +|-----------|-------|---------| +| **Optimistic concurrency** (`updated_at`) | Row-level | Detects concurrent writes to the same database row; fails fast on conflict | +| **Advisory locking** (`LockService`) | Plan/project-level | Prevents concurrent phase transitions; serializes the critical section before any row is written | + +The advisory lock is acquired **before** the phase transition begins, so the optimistic check never races — by the time `updated_at` is compared, only one session holds the lock and is executing the critical section. + +##### TTL and Renewal + +| Constant | Default | Description | +|----------|---------|-------------| +| `DEFAULT_LOCK_TTL_SECS` | 300 | Default lock lifetime (5 min) | +| `MAX_LOCK_TTL_SECS` | 3600 | Maximum allowed TTL (1 hr) | +| `MIN_LOCK_TTL_SECS` | 5 | Minimum allowed TTL (5 sec) | + +For long-running phases (e.g., Execute), the caller should periodically call `renew()` before the current lock expires. A recommended renewal interval is `ttl / 2` (e.g., every 150 seconds for the default 300-second TTL). + +##### Database Schema + +The `locks` table (migration `m4_001_concurrency_locks`) stores active advisory locks: + +| Column | Type | Description | +|--------|------|-------------| +| `id` | INTEGER PK | Auto-increment primary key | +| `owner_id` | VARCHAR(255) | Lock owner identity (UUID per invocation) | +| `resource_type` | VARCHAR(50) | `plan` or `project` | +| `resource_id` | VARCHAR(255) | Resource identifier | +| `acquired_at` | VARCHAR(30) | ISO-8601 acquisition time | +| `expires_at` | VARCHAR(30) | ISO-8601 expiry time | + +A unique constraint on `(resource_type, resource_id)` enforces at most one active lock per resource. + +##### Error Types + +| Exception | Condition | +|-----------|-----------| +| `LockConflictError` | Another owner holds an active lock | +| `LockExpiredError` | Attempted to renew an already-expired lock | +| `ValidationError` | Invalid parameters (empty strings, bad TTL) | + +##### Startup and Shutdown + +- **Startup cleanup**: On application startup, `cleanup_expired()` is called to purge any locks whose `expires_at` has passed. This handles the case where a previous process terminated without releasing its locks. +- **Graceful shutdown**: During graceful shutdown, `release_all_for_owner(owner_id)` removes all locks held by the current process so that resources become immediately available to other instances. + +##### Diagnostics + +The `agents diagnostics` command includes a **Stale locks** check that reports the number of expired-but-not-yet-cleaned locks. A non-zero count indicates that the cleanup routine has not run recently. + +For the full `LockService` API reference, see [`docs/reference/concurrency.md`](../reference/concurrency.md). + #### Filesystem Layout The data directory follows a deterministic layout. All paths are relative to `core.data-dir` (default: `~/.cleveragents`):