T-KPVN-lease-library-v1-to-v2-hardening

Status: planning/needs-definition · Impact: medium · Complexity: medium

Goal

Three drafted tasks each proposed a tightening of the slice-1 lease library — ttl_seconds in the payload (schema bump), post-state validation in every primitive (defensive code), and the ADR’s full owner-rotation matrix (slice-1 implemented a deliberate approximation). All three edit plugin/lib/services/lease/primitives.ts, runtime.ts, or the payload schemas in schemas.ts; the ttl_seconds schema bump is the natural carrier for the other two changes. Shipping them as 3 PRs means 3 schema-bump churn cycles or 3 overlapping edits to the same files. This task bundles them into one hardening pass with a single schema bump and one canonical PR. The 3 originating drafts are closed/superseded with completion_note pointing here.

Today

Location	Role today
`plugin/lib/services/lease/primitives.ts`	Five git-transport primitives: `casCreate`, `casReplace`, `casDelete`, `fetchRef`, `fetchNamespace`. `casCreate` detects the “Everything up-to-date” short-circuit (same-SHA no-op masquerading as success) and throws `CASFailed`. The other 4 rely on git’s exit code alone — silent CAS losses are possible.
`plugin/lib/services/lease/schemas.ts`	Zod payload schemas, versioned by the semver `sdlc_version` each payload carries (constant in `version.ts`). No `ttl_seconds` field — a payload does not record the TTL it was written with.
`plugin/lib/services/lease/runtime.ts`	`acquireLease` / `transitionLease` / `heartbeatLease` / `reacquireLease`. Token rotation is phase-keyed: `transitionLease` rotates `lease_token` only on entry into claimed/working (`OWNER_ROTATING_PHASES`); `reacquireLease` rotates unconditionally. The ADR keys rotation to owner change instead.
`plugin/lib/services/lease/heartbeat-service.ts`	Heartbeat driver; passes its `--ttl` to `heartbeatLease`, which writes `expires_at = now + ttl`. Because the payload carries no TTL, the prior-heartbeat timestamp can only be approximated as `expires_at - <assumed ttl>` — a small error in the steady state, larger when a lease was stolen from an expired predecessor.
`plugin/lib/services/lease/reconcile.ts`	Declares the `heartbeat-rate-above-floor` anomaly category; its detector (currently a stub) needs the TTL a payload was written with and has nothing to read.
`plugin/lib/services/lease/tests/`	`primitives.test.ts` has the `casCreate` same-SHA regression; no equivalent for the other 4 primitives. `runtime.test.ts` covers a few rotation cases but no exhaustive matrix.

Proposed

Three concrete changes land as a single PR, carried by one payload-schema bump (a minor sdlc_version bump — the new field is additive):

1. Add ttl_seconds to the lease payload. The payload records the TTL that was set when the lease was acquired or refreshed. Heartbeat-floor enforcement reads it directly instead of approximating; lease history dumps include it for audit. Compatibility: the field is optional in schemas.ts, so pre-bump payloads (no ttl_seconds) still validate; readers fall back to the approximation only for those. Writers always emit it.

2. Post-state validation on every primitive. Each of casReplace, casDelete, fetchRef, fetchNamespace re-reads the ref state after the operation and validates the post-state matches the intended operation. The “Everything up-to-date” short-circuit currently caught only by casCreate gets caught by every primitive. Failure throws the same CASFailed (or the primitive’s specific failure exception) that casCreate throws today.

3. Full owner-rotation matrix per the ADR. Replace the phase-keyed rule with the ADR’s owner-keyed matrix. Concretely: cross-host steals always rotate the token (even if the phase doesn’t change); same-host re-acquires preserve; phase boundaries within a single host preserve. Document the matrix in the transitionLease / reacquireLease doc comments in runtime.ts.

Approach

Read the lease ADR’s rotation rules (docs/planning/decisions/github-ref-leases/protocol.md) and the phase-keyed rule in runtime.ts. Enumerate the cells where the implementation diverges; write down the target rule per cell.
Bump the payload schema. Add ttl_seconds: int (optional) to the payload schemas in schemas.ts; minor-bump SDLC_VERSION in version.ts. Absence on read means pre-bump payload — Zod .optional() covers it. Active leases are short-lived, so forward-compatible reads should suffice; if an explicit ref-payload migration turns out to be needed, it belongs beside migration.ts / ops/.
Add post-state validation to the 4 currently-unvalidated primitives. Each of casReplace, casDelete, fetchRef, fetchNamespace ends with a post-state re-read comparing the ref’s current SHA / existence to intent. Reuse casCreate’s detection as the model.
Replace the rotation rule with the full matrix. Refactor to a small lookup keyed by (owner change?, phase transition) → rotate-or-preserve. Cite the ADR in the doc comment.
Test coverage. Each primitive gets a same-SHA / no-op regression test (5 total, including casCreate’s existing one) in tests/primitives.test.ts. The rotation matrix gets a parametrised test in tests/runtime.test.ts enumerating every cell with an expected rotate/preserve outcome. tests/schemas.test.ts gets a pre-bump payload case that validates cleanly under the new schema.

Files to touch

Location	Kind	Change
`plugin/lib/services/lease/schemas.ts`	modify	Add optional `ttl_seconds` to the lease payload schemas.
`plugin/lib/services/lease/version.ts`	modify	Minor `SDLC_VERSION` bump (additive schema change).
`plugin/lib/services/lease/primitives.ts`	modify	Post-state validation on `casReplace`, `casDelete`, `fetchRef`, `fetchNamespace`.
`plugin/lib/services/lease/runtime.ts`	modify	Owner-keyed rotation matrix in `transitionLease` / `reacquireLease`; emit `ttl_seconds` on every payload write; document the matrix.
`plugin/lib/services/lease/reconcile.ts`	modify	When the `heartbeat-rate-above-floor` detector is ported, read `ttl_seconds` directly; approximate only for pre-bump payloads.
`plugin/lib/services/lease/tests/primitives.test.ts`	modify	Same-SHA / no-op regression cases for `casReplace`, `casDelete`, `fetchRef`, `fetchNamespace`.
`plugin/lib/services/lease/tests/runtime.test.ts`	modify	Parametrised test enumerating every cell of the rotation matrix.
`plugin/lib/services/lease/tests/schemas.test.ts`	modify	Pre-bump payload (no `ttl_seconds`) validates; new payloads carry the field.

Acceptance criteria

AC-1: Lease payload schema carries ttl_seconds and SDLC_VERSION is minor-bumped. A pre-bump payload (no ttl_seconds) validates cleanly and readers fall back to the TTL approximation for the heartbeat floor; a new payload’s ttl_seconds is read directly.
AC-2: Each of casReplace, casDelete, fetchRef, fetchNamespace (the 4 primitives in plugin/lib/services/lease/primitives.ts without post-state validation today) validates the ref’s post-state after the operation. A regression test per primitive exercises the same-SHA / no-op / short-circuit path and asserts the appropriate exception is thrown.
AC-3: The owner-rotation rule matches the ADR’s full matrix. A parametrised test enumerates every cell of the matrix and asserts the correct rotate-or-preserve outcome.
AC-4: Combined coverage on plugin/lib/services/lease/primitives.ts and the rotation paths in runtime.ts remains >90% after the new tests land.

Out of scope

Rewriting the primitives to use libgit2 or another transport. The wrapper-validates-post-state approach is sufficient.
Changing the lease ref namespace, the archive ref shape, or the CAS-CREATE/REPLACE/DELETE contract. Those are protocol-level concerns; this task is library-internal.
Heartbeat policy changes beyond reading ttl_seconds directly. The floor/ceiling values stay where they are.
Broader schema-version machinery. One additive field, one minor bump; anything more is a separate task.

Dependencies

none

Discovery context

Spawned 2026-05-28 from a consolidation audit. Three drafts (T-5IG1-lease-primitives-validate-git-actually-acted, T-954I-lease-payload-adds-ttl-seconds, T-1VF3-lease-token-rotation-full-policy) all touch the lease library’s primitives or payload schema. The ttl_seconds task forces a schema bump that the others can ride along on; merging avoids 3 rounds of review on overlapping files.

The superseded drafts:

← Back to Tasks