Skip to content

feat(items): execution lease — atomic claim/checkout with expiry (#1221) - #1238

Draft
b4rk13 wants to merge 1 commit into
PerpetualSoftware:mainfrom
b4rk13:feat/item-lease
Draft

feat(items): execution lease — atomic claim/checkout with expiry (#1221)#1238
b4rk13 wants to merge 1 commit into
PerpetualSoftware:mainfrom
b4rk13:feat/item-lease

Conversation

@b4rk13

@b4rk13 b4rk13 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Draft PR for #1221, built to the shape you approved and the two decisions pinned in the thread (default TTL 15 minutes; Lease: line on show / glyph marker on list / full object in JSON). Relates to #1221 — leaving close-on-merge to you.

The primitive. POST .../items/{ref}/claim and .../release, with pad item claim <ref> [--holder <id>] [--ttl <dur>] / pad item release <ref> in front. The claim is a conditional UPDATE whose predicate is the arbiter — deliberately the same protocol as the event-outbox claim (migration 083 / TASK-2714) and BUG-2415's orphan GC, dialect-uniform, no FOR UPDATE SKIP LOCKED special case. A store-level test races 8 goroutines and asserts exactly one winner.

  • Contention answers 409 code=lease_held naming the live holder, acquired_at, and expires_at — same envelope discipline as update_conflict, so a loser can log who won and skip without a second call.
  • Refresh: a re-claim by the live holder moves the expiry forward and keeps acquired_at (heartbeat) — so long runs extend instead of everyone inflating the default TTL.
  • Expiry is the reaper: an expired lease reads as absent on every path (claim predicate, item GET, list decoration) with no sweep job — test-pinned including the "crashed holder, no reaper ran" case.
  • Release is idempotent (absent/expired → released:false, exit 0); releasing another holder's live lease is refused with the same 409.
  • Holder defaults to the authenticated user's email (then id) — the CLI: support multiple identities per server (PAD_TOKEN env override + named credential profiles) #879 tie-in; freeform string until named profiles land, exactly as discussed.
  • TTL: default 15m, per-call ttl_seconds, refused outside (0, 24h].

Display, per the thread: Lease: line under Assigned: in PrintItemMeta (absent when none); leased list rows get a cyan » on the ref/title cell via the existing pinned-* pattern — no new column, since renderItemTable's width budget would tax every row's title for a usually-absent state; "lease": {holder, acquired_at, expires_at} in JSON on get and list, key omitted when absent (one point-read on GET, one workspace query on list — the shared item scan's column lists are untouched).

Deliberate non-wirings, each open to your call:

  1. No updated_at bump, no version entry — a lease is coordination state, not content; a claim must never 409 a concurrent editor's expected_updated_at token.
  2. No SSE/activity event — a heartbeat re-claim every few minutes would spam the feed; if you want live web-UI lease indicators, I'd emit claim/release (not refresh) events as a follow-up.
  3. No MCP catalog exposure — that's a ToolSurfaceVersion decision and agents are the motivating users, so I'd rather add it to the catalog with your blessing than smuggle it in; say the word and it lands here or as a follow-up.
  4. No web UI rendering — the TS Item type gained the optional lease field, but no component consumes it yet.

Your codebase's own guards caught and shaped three things, which was pleasing to watch: TestNULColumnCensus demanded a ruling on the three new columns (lease_holder is caller text → census entry + regenerated 084 triggers; the two timestamps are server-composed RFC3339 → recorded exclusions with reasons), TestEveryRequestBodyReaderIsAccountedFor demanded the new decodeJSON reader be ledgered (it is — same shape as handlers_watches.go), and TestScanCostFiguresMatchTheList kept ScanNUL's cost figure honest (131→132).

Postgres note: pgmigrations/062_item_lease.sql mirrors the SQLite migration; I don't have a PG instance wired locally, so the PG path is written to the dialect-uniform pattern but only SQLite-tested here.

How to test

  1. Terminal A: pad item claim TASK-5 --holder runner-a → lease with holder + expiry. Terminal B: pad item claim TASK-5 --holder runner-b → exit non-zero, error names runner-a and the expiry.
  2. pad item show TASK-5Lease: runner-a (expires in 14m) under Assigned:; pad item list marks the row with »; --format json carries the lease object.
  3. pad item release TASK-5 --holder runner-a → released; re-run → "No live lease" no-op, exit 0. B claims successfully now.
  4. pad item claim TASK-5 --ttl 5s, wait 6s, claim as someone else → succeeds with no reaper.

Checklist

  • go build ./... passes
  • go test ./internal/store/ ./internal/server/ ./internal/cli/ ./cmd/pad/ — zero new failures by name vs a clean-main baseline run on the same Windows box (server and the new suites fully green; the pre-existing Windows HOME-assumption set unchanged)
  • New features have tests — store/server/CLI suites written first and watched fail (route 404s, undefined: compile errors); the display tests were written after their code, so I verified they bite by deliberately breaking the renderer and watching them fail before reverting
  • TypeScript types updated — ItemLease + optional Item.lease
  • CLI help text updated — command help + README (pad item claim/release rows)

…petualSoftware#1221)

Two pollers that both read 'unclaimed' could both proceed; whatever they
write next, the last writer wins silently. The lease makes 'someone is
executing this right now' first-class and time-bounded, acquired by a
conditional UPDATE whose predicate is the arbiter — the same protocol the
event-outbox claim (TASK-2714) and orphan GC (BUG-2415) established.

- pad item claim <ref> [--holder <id>] [--ttl <dur>]: succeeds iff
  unclaimed, expired, or already held by the caller (a holder re-claim
  refreshes expiry and keeps acquired_at — heartbeat). Contention answers
  409 code=lease_held naming the live holder and expiry, the same
  envelope discipline as update_conflict.
- pad item release <ref>: idempotent (absent/expired = no-op); releasing
  another holder's LIVE lease is refused.
- Expiry is the reaper: an expired lease reads as absent everywhere; no
  sweep job. TTL default 15m, max 24h, per-call ttl_seconds.
- Holder defaults to the authenticated identity (email) — the PerpetualSoftware#879
  tie-in; freeform string until named profiles land.
- Display: 'Lease:' line under Assigned: on item show; a cyan marker on
  leased list rows (no new column — it would tax every row's title
  budget for a usually-absent state); full lease object in JSON,
  key omitted when absent.
- Deliberately NOT wired: updated_at bump, version entry, SSE/activity
  event, MCP catalog exposure — a lease is coordination state, not
  content, and a claim must never 409 a concurrent editor's
  expected_updated_at token.
- NUL invariant: items.lease_holder is caller text — added to the census
  and the regenerated 084 triggers; the lease timestamps are
  server-composed RFC3339 and recorded as exclusions. The new body
  reader goes through decodeJSON and is accounted for in the ledger.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant