571c4221
Correct the lease plan: rows are a tenure ledger, never deleted
a73x 2026-09-06 08:00
Commit message
docs/superpowers/plans/2026-09-05-issue-leases.md
| Old | New | ||
|---|---|---|---|
| @@ -168,7 +168,7 @@ Schema (inside `open`, `CREATE TABLE IF NOT EXISTS`; also set `PRAGMA journal_mo | |||
| 168 | CREATE TABLE IF NOT EXISTS leases ( | 168 | CREATE TABLE IF NOT EXISTS leases ( |
| 169 | repo TEXT NOT NULL, | 169 | repo TEXT NOT NULL, |
| 170 | issue_id TEXT NOT NULL, | 170 | issue_id TEXT NOT NULL, |
| 171 | holder TEXT NOT NULL, | 171 | holder TEXT, -- NULL = free; the row keeps its token |
| 172 | token INTEGER NOT NULL, | 172 | token INTEGER NOT NULL, |
| 173 | acquired_at INTEGER NOT NULL, | 173 | acquired_at INTEGER NOT NULL, |
| 174 | expires_at INTEGER, | 174 | expires_at INTEGER, |
| @@ -180,8 +180,19 @@ Semantics to implement (each inside ONE transaction — `BEGIN IMMEDIATE` via `c | |||
| 180 | 180 | ||
| 181 | - `acquire`: read the row. No row, or row not `live(now)` → take tenure: `INSERT OR REPLACE` with `token = old_token + 1` (1 if no row), `acquired_at = now`, `expires_at = ttl_secs.map(|t| now + t)` → `Acquired`. Live row held by caller → idempotent: keep token, update `expires_at` from this call's `ttl_secs` → `Acquired`. Live row held by another → `Held`. | 181 | - `acquire`: read the row. No row, or row not `live(now)` → take tenure: `INSERT OR REPLACE` with `token = old_token + 1` (1 if no row), `acquired_at = now`, `expires_at = ttl_secs.map(|t| now + t)` → `Acquired`. Live row held by caller → idempotent: keep token, update `expires_at` from this call's `ttl_secs` → `Acquired`. Live row held by another → `Held`. |
| 182 | - `renew`: live row held by caller → update `expires_at` (same rule), keep token → `Renewed`. Anything else → `NotHolder` (with the live holder's name if there is one). | 182 | - `renew`: live row held by caller → update `expires_at` (same rule), keep token → `Renewed`. Anything else → `NotHolder` (with the live holder's name if there is one). |
| 183 | - `release`: live row held by another → `NotHolder`. Otherwise delete any row → `Released` (idempotent: releasing nothing succeeds — a client retrying after a dropped connection must not fail). | 183 | - `release`: live row held by another → `NotHolder`. Otherwise **free** the row (`holder = NULL`, `expires_at = NULL`), keeping its `token` → `Released` (idempotent: releasing nothing succeeds — a client retrying after a dropped connection must not fail). |
| 184 | - `current`/`list`: return only live rows; opportunistically `DELETE` expired rows they encounter (lazy reaping). | 184 | - `current`/`list`: return only held rows, by **filtering** on `holder IS NOT NULL AND (expires_at IS NULL OR expires_at > now)`. |
| 185 | |||
| 186 | > **Rows are a per-issue tenure ledger and are never deleted.** An earlier | ||
| 187 | > draft of this plan said `release` should delete the row and that | ||
| 188 | > `current`/`list` should opportunistically reap expired ones. Both reset | ||
| 189 | > `token` to 1 for the next holder, which destroys the only property a | ||
| 190 | > fencing token has: tenure 1's zombie could later present token 1 to a | ||
| 191 | > fresh tenure 1 and pass a check it must fail. The reaping path was the | ||
| 192 | > worse of the two — merely *reading* the lease list would have silently | ||
| 193 | > reset fencing. `holder IS NULL` means free; `token` only ever increments. | ||
| 194 | > The e2e test `release_frees_the_issue_and_bumps_the_next_tenure` is what | ||
| 195 | > caught it. | ||
| 185 | 196 | ||
| 186 | Unit tests in the same file (`#[cfg(test)]`, DB via `Connection::open_in_memory()` piped through the same schema init — factor `open` so tests can reuse the schema function): | 197 | Unit tests in the same file (`#[cfg(test)]`, DB via `Connection::open_in_memory()` piped through the same schema init — factor `open` so tests can reuse the schema function): |
| 187 | 198 | ||
| @@ -194,11 +205,14 @@ acquire_after_expiry_takes_over_and_bumps_token | |||
| 194 | renew_extends_expiry_keeps_token | 205 | renew_extends_expiry_keeps_token |
| 195 | renew_by_non_holder_returns_not_holder | 206 | renew_by_non_holder_returns_not_holder |
| 196 | renew_after_expiry_returns_not_holder | 207 | renew_after_expiry_returns_not_holder |
| 197 | release_by_holder_deletes / release_idempotent_when_absent | 208 | release_by_holder_frees_the_issue / release_idempotent_when_absent |
| 198 | release_by_non_holder_refused | 209 | release_by_non_holder_refused / release_of_expired_lease_by_anyone_succeeds |
| 210 | released_issue_is_free_to_anyone | ||
| 199 | list_shows_only_live_leases / current_none_after_expiry | 211 | list_shows_only_live_leases / current_none_after_expiry |
| 200 | open_ended_lease_never_expires (large `now`) | 212 | open_ended_lease_never_expires (large `now`) |
| 201 | tenure_token_monotonic_across_holders (a→expire→b→expire→a: tokens 1,2,3) | 213 | tenure_token_monotonic_across_holders (a→expire→b→expire→a: tokens 1,2,3) |
| 214 | token_does_not_reset_after_release | ||
| 215 | token_does_not_reset_after_expiry_and_reads (the reaping bug, pinned) | ||
| 202 | ``` | 216 | ``` |
| 203 | 217 | ||
| 204 | - [ ] **Step 3: Run tests to verify they fail** | 218 | - [ ] **Step 3: Run tests to verify they fail** |