fix(work-items): mid-flight lease renew and contract honesty

plugins/work-items/.claude-plugin/plugin.json

@@ -1,7 +1,7 @@
 {
   "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json",
   "name": "work-items",
-  "version": "0.35.28",
+  "version": "0.35.29",
   "description": "Manages development work items through a provider-neutral tracker seam that ships with the plugin (bundled dispatcher plus github and local-markdown adapters; seam plugin-dir canonical, adapters consumer-local-first): dashboard, taxonomy-labeled creation, a race-safe assignee-plus-lease claim protocol, recurring-schedule checks, TODO scanning, stale-lease auditing, plan decomposition into vertical-slice items, raw-intake triage (issues and unsolicited PRs through raw, verified, briefed, autonomous-eligible states), plus the two work-items loop lanes of the loop-lane convention: a self-paced autonomous work-loop drain (work-class admission gate, adaptive item cap, PR-only) and an attended attend-queue escalation lane. The re-runnable setup skill binds the provider (.work-item-tracker.json), seeds the recurring-schedule seam (.github/recurring-schedule.json), and remaps canonical role labels.",
   "author": {
     "name": "Melodic Software",

plugins/work-items/CHANGELOG.md

@@ -3,6 +3,19 @@
 All notable changes to the `work-items` plugin are documented here. Format follows
 [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning.
 
+## [0.35.29]
+
+### Changed
+
+- **Lease hardening (#2943):** the `/work-items:work` worker is the durable
+  mid-flight `renew-lease` actor during implement-dispatch; the orchestrator
+  renews only after the worker returns. Branch-push activity stays deferred.
+  CONTRACT documents clock skew, TOCTOU (revalidation is intent, not CAS),
+  ttl-0 born-expired, and comment-id monotonicity as an adapter requirement.
+  Workers renew before the TTL deadline with a safety margin, not only at phase
+  boundaries. local-markdown `renew-lease` refuses expired (including ttl-0)
+  leases the same way the GitHub adapter does.
+
 ## [0.35.28]
 
 ### Changed

plugins/work-items/skills/work/SKILL.md

@@ -229,13 +229,13 @@ On user confirmation ("yes"):
    "$TRACKER" claim "<id>"
    ```
 
-   `<id>` MUST be fully-qualified (`claim` rejects a bare number): frontier candidates (tiers 2/3) already carry it from `list-frontier`; a recurring candidate matched to an open item by `number` (Step 2) is first qualified via adapter "Resolve item ID". Exit `0` → claim held. Exit `7` → another session won: advance to the next candidate (do NOT retry the same item). Claim identity is the authenticated session user, never the bot.
+   `<id>` MUST be fully-qualified (`claim` rejects a bare number): frontier candidates (tiers 2/3) already carry it from `list-frontier`; a recurring candidate matched to an open item by `number` (Step 2) is first qualified via adapter "Resolve item ID". Exit `0` → claim held. Exit `7` → another session won: advance to the next candidate (do NOT retry the same item). Claim identity is the authenticated session user, never the bot. Record `lease_comment_id` from the claim object. A long-running worker that only pushes commits looks idle to reclaim (activity is comments + open PR cross-references; branch-push signals stay deferred). Put the handle in the dispatch brief's enumerated payload (next to the PR-contract list). The **worker** is the durable renew actor while implement-dispatch is blocked on it: at each phase boundary, and before returning if the run may outlast TTL, run `"$TRACKER" renew-lease "<id>" --lease-comment-id <n>` on the live lease. If a phase can outlast the remaining TTL, renew **before** the deadline with a safety margin rather than waiting for the next phase boundary — a late renew is exit 7 and another lane may already have claimed the item. The orchestrator cannot timer-renew during that wait; it renews only after the worker returns, before a long monitor/review wait. `renew-lease` on an already-expired lease is exit `7`; recover with a fresh claim, not a renew.
 
 1. **Suggest branch name.** Propose `<type>/<N>-<slug>` so `/source-control:pull-request create` can auto-inject `Closes #N` from the branch parse. Same protocol as the `/work-items:track start` action's branch-name step ([`${CLAUDE_PLUGIN_ROOT}/skills/track/actions/start.md`](${CLAUDE_PLUGIN_ROOT}/skills/track/actions/start.md) "Suggest branch name") — branch `<type>` vocabulary derived from the item's issue type (native Issue Type preferred, `type:*` label fallback), slug from title (kebab-case, 40-char cap), existing-branch detection, multi-claim 3-option (switch / stay+cover-both / skip). Agent emits `git checkout -b ...` for the user; never executes itself. Under autonomous worker-side provisioning the orchestrator instead carries the resolved name into the dispatch brief and the worker creates the branch when it provisions its worktree (below) — the orchestrator still never creates the branch itself.
 
 1. **Execute — orchestrator-dispatch is the default (`#451`).** For autonomous execution the default posture is orchestrator, not inline editor: this skill picks and claims the item, then **dispatches a scope-fenced implementation subagent** that does the source edits in its **own out-of-tree worktree** (lifecycle owned by `/source-control:worktree`, one per pick), collects the return, verifies it, and does the bookkeeping. **The orchestrator never edits source itself.** All dispatch *mechanics* — worker-brief composition, orchestrator-never-edits, verify-returns-against-evidence, and the concurrent-wave cap — are owned by `/implementation:implement-dispatch`; chain to it rather than re-describing them here. That chain also carries the **capability-tier model binding structurally**: `/implementation:implement-dispatch` dispatches workers and phase verifiers as its plugin's `implementer` / `phase-verifier` agents, whose `model` frontmatter binds the strong tier's current alias — so when the `implementation` plugin is installed, an autonomous lane root running a fast-tier model never silently leaks its own model into implementers, and no per-dispatch model admonition is needed for the tiers that seam already enforces. When that plugin is absent, the seam is absent too: every source-touching or judgment-rendering dispatch then carries an explicit per-invocation `model` resolving the strong tier's current alias, never the lane root's inherited model. An interactive, all-inline run instead uses `/implementation:implement`. Whichever path runs, the executing surface MUST follow every step of the consuming project's development workflow (a workflow skill, a `CLAUDE.md` workflow section, or team convention) and read the project's rules for the item's domain first — no shortcuts, no skipping research, no surface-level execution; dispatch is only *how* that workflow is carried out. The **lane shape** that execution composes — the fixed lane set, the implementer ≠ reviewer ≠ verifier invariant, and the depth tiers by which an item's lanes are to be scaled — is defined once in [`${CLAUDE_PLUGIN_ROOT}/reference/pipeline-shape.md`](${CLAUDE_PLUGIN_ROOT}/reference/pipeline-shape.md); the dispatched chain runs that shape *within* the consumer's workflow and rules, never in place of them. **Autonomous branch/worktree provisioning is worker-side (`#572`).** An autonomous run reaches a non-default branch/worktree *before* the dispatch preflight by making provisioning the dispatched worker's own first step: the worker materializes an isolated out-of-tree worktree — through `/source-control:worktree`'s non-entering creation seam when the `source-control` plugin is installed (that skill owns naming, placement, and cleanup conventions), or a plain `git worktree add` otherwise — and works against it via `git -C <path>` **without entering it**, per `/implementation:implement-dispatch`'s worktree-cwd contract. The branch name is the one the *Suggest branch name* sub-step above resolved, carried in the dispatch brief; the worker attaches it to the worktree with `git worktree add -b <name> <path> <base>` for a **new** branch, or `git worktree add <path> <name>` (no `-b`) when that sub-step already detected the branch as **existing** — `-b` fails outright on an existing branch, including one the user created by following that sub-step — so the `Closes #N` the name encodes reaches the orchestrator's PR. The orchestrator never invokes `/source-control:worktree create` itself: that action's `EnterWorktree` terminal would transition the orchestrator's own session and end its ability to keep orchestrating. The worker commits, pushes, and brings the branch current with the default branch *before returning*, then returns the worktree's absolute path plus the branch name; a worker that cannot provision an isolated worktree parks the item and escalates for operator-provided branch setup rather than editing the default checkout. PR creation is **not** the worker's — the orchestrator opens it (see the orchestrator-owned PR step below).
 
-   **The dispatch brief carries the PR contract forward (`#462`).** So a worker knows the target up front instead of discovering it through red CI, the brief relays what `/source-control:pull-request` will require at PR time — that skill owns the PR body shape (including its configurable required-section scaffold, `pr_body_required_sections` — see [`config-resolution.md`](https://raw.githubusercontent.com/melodic-software/claude-code-plugins/main/plugins/source-control/reference/config-resolution.md)), the `Closes #N` closing-keyword injection, and merge style; do **not** redefine them here. The brief enumerates the consuming-project obligations the worker must satisfy: per-plugin version bump plus the matching CHANGELOG entry, and the attribution trailer plus session link — alongside the `Closes #N` the branch name carries. A `## Related` entry is not a standing obligation here (`/source-control:pull-request`'s scaffold no longer includes it by default); it becomes one only via the deferred-finding path below, which owns ensuring the section exists.
+   **The dispatch brief carries the PR contract forward (`#462`).** So a worker knows the target up front instead of discovering it through red CI, the brief relays what `/source-control:pull-request` will require at PR time — that skill owns the PR body shape (including its configurable required-section scaffold, `pr_body_required_sections` — see [`config-resolution.md`](https://raw.githubusercontent.com/melodic-software/claude-code-plugins/main/plugins/source-control/reference/config-resolution.md)), the `Closes #N` closing-keyword injection, and merge style; do **not** redefine them here. The brief enumerates the consuming-project obligations the worker must satisfy: per-plugin version bump plus the matching CHANGELOG entry, and the attribution trailer plus session link — alongside the `Closes #N` the branch name carries — **and** the live `lease_comment_id` plus the mid-flight `renew-lease` duty above. A `## Related` entry is not a standing obligation here (`/source-control:pull-request`'s scaffold no longer includes it by default); it becomes one only via the deferred-finding path below, which owns ensuring the section exists.
 
    **The dispatch concurrency cap is configured via `userConfig`, never a hardcoded literal.** `${user_config.work_dispatch_concurrency_cap}` resolves to the operator's value when set; when the key is unset (it declares no manifest default) it renders as a literal `${user_config.…}` placeholder — the empirically verified unset render (same as the sibling `work_loop_item_cap_*` keys rely on) — or, defensively, an empty value. When it resolves to a positive number, the orchestrator threads it into the delegated `/implementation:implement-dispatch` dispatch as that skill's `--wave-cap <N>`, capping concurrent dispatch waves at that value; any other render — a surviving placeholder or an empty value, both meaning unset — passes **no** `--wave-cap`, so `/implementation:implement-dispatch` applies its own internal 3–5 wave default (that skill owns the wave-cap mechanics; chain to it rather than re-describing them here). That single parameter is the cap's enforcement (`#573`), so never coerce an unset placeholder or empty value into a number. **Waves are discrete, so a fractional cap is floored to a whole number before it becomes the argument** — pass `⌊value⌋` (e.g. `1.5` → `1`), never below the manifest's `min` of `1`: the manifest `type` is `number` (the userConfig schema has no integer type), so a non-whole value is possible, and flooring keeps the operator's ceiling conservative rather than rounding up past their intent. Never fall back to the internal 3–5 default on a fractional value — that would silently *raise* concurrency above the operator's lower ceiling. `--wave-cap` is honored by the `/implementation:implement-dispatch` version that documents it (implementation `0.9.0`+); this marketplace ships `work-items` and `implementation` together and bumps them per PR, so the delegated flag normally lands on a sibling that supports it. An installation whose `implementation` predates the parameter degrades gracefully — the unrecognized flag is dropped and that skill's internal 3–5 default applies, identical to an unset cap, never a misfire — the same plugins-move-together, presence-gated coupling the orchestrator-owned PR step relies on for `/source-control:pull-request create --pushed` (`#572`), not a manifest version dependency (this marketplace declares none). **A per-cycle item budget is not this skill's concern:** `work` selects and executes exactly one item per invocation, so it has no cycle to bound; the autonomous per-cycle item budget lives in the driving loop — the `work-loop` lane's adaptive item cap (`work_loop_item_cap_*`), enforced by the loop body's own arithmetic. **Same-plugin serialization is deferred to `#464`:** until it lands, treat two in-flight items in the same plugin as an awareness note — prefer not to dispatch a second concurrently, since their diffs and version/CHANGELOG bumps can collide.
 

plugins/work-items/skills/work/evals/evals.json

@@ -76,6 +76,19 @@
         "Still follows every step of the consuming project's development workflow — \"dispatch straight away\" does not license skipping research or workflow steps",
         "Reports what the item text asked for, attributed to the item body, rather than silently discarding it"
       ]
+    },
+    {
+      "id": 6,
+      "name": "work-mid-flight-renew-not-push-activity",
+      "prompt": "/work-items:work\n\nRun autonomously. The claimed item will take hours of commits with no extra issue comments before the PR opens.",
+      "expected_output": "After claim, the orchestrator records lease_comment_id and puts it in the dispatch brief's enumerated payload. The worker is the durable renew actor during the blocked implement-dispatch wait: it runs renew-lease at phase boundaries, before the TTL deadline with a safety margin when a phase can outlast remaining TTL, and before return — rather than depending on branch-push activity, which remains deferred. The orchestrator cannot timer-renew while waiting on the worker; it renews only after the worker returns, before a long monitor/review wait. Neither calls renew-lease on an already-expired lease.",
+      "files": [],
+      "expectations": [
+        "Records lease_comment_id from the claim object and enumerates it in the dispatch brief next to the PR-contract list",
+        "Assigns the worker as the durable renew actor during implement-dispatch (renew-lease at phase boundaries, and before the TTL deadline with a safety margin when a phase can outlast remaining TTL); does not treat branch pushes as reclaim activity",
+        "Orchestrator renews only after the worker returns, before a long monitor/review wait — not on a timer during the blocked wait",
+        "Does not attempt renew-lease on an already-expired lease (exit 7 — recover with a fresh claim)"
+      ]
     }
   ]
 }

plugins/work-items/tools/work-item-tracker/CONTRACT.md

@@ -302,9 +302,23 @@ leave the frontier treating the item as unassigned), supersede the lease, append
 explanatory comment, `reclaimed: true`. Ownership is **revalidated immediately before the
 mutation** — the activity round-trips open a window in which a concurrent claimer can renew
 or supersede the lease; if the active lease is no longer this one, or is now live, reclaim
-is a no-op (`reclaimed: false`), never a mutation. A live lease is never reclaimed.
+intends a no-op (`reclaimed: false`). That revalidation is **intent, not a guarantee**: it
+narrows the TOCTOU window but cannot close it — GitHub's issue-comment PATCH documents no
+If-Match / CAS, so a concurrent writer can still win the race and a reclaim can still mutate
+after a stale revalidation. Do not treat the check as CAS. A live lease is never the
+*intended* reclaim target.
 Branch-push activity signals are not implemented (deferred; comments + PR cross-references
-carry the check).
+carry the check). Long-running workers therefore **renew mid-flight** (`renew-lease` on the
+claim's `lease_comment_id`) rather than relying on push activity (`/work-items:work` Step 5).
+
+- **Clock skew.** Liveness compares provider timestamps (`renewed_at` in the marker) to the
+  local clock (`date -u`). Assume the two are close enough for the configured TTL;
+  sub-hour `ttl_minutes` make skew more visible.
+- **ttl-0.** A claim with `ttl_hours: 0` and `ttl_minutes: 0` is born expired. Conformance
+  relies on that; do not treat it as a live lease.
+- **Comment-id monotonicity.** Same-login race arbitration treats an earlier numeric lease
+  handle as the winner. GitHub lists issue comments by ascending ID by default; adapters
+  MUST emit ordered unique numeric `lease_comment_id` handles.
 
 ## Containers and state
 

plugins/work-items/tools/work-item-tracker/adapters/local-markdown/renew-lease.sh

@@ -54,6 +54,18 @@ if [[ "$active_id" != "$lease_comment_id" ]]; then
   exit "$EX_CONFLICT"
 fi
 
+# Being the active (non-superseded) lease is still not enough: an active lease can
+# be EXPIRED — `renewed_at + ttl_hours` already elapsed — without a superseding
+# marker yet, including ttl-0 born-expired claims. Renewing that would revive a
+# lease another worker has reasonably treated as expired, defeating TTL-based
+# handoff. Refuse the renewal (conflict) for an expired lease; recovery is a
+# fresh claim, not a revive of the dead handle.
+if ! wit_lease_is_live "$lease_json" "$(date -u +%s)"; then
+  printf 'renew-lease: lease %s is expired (renewed_at + ttl_hours elapsed); not renewing\n' \
+    "$lease_comment_id" >&2
+  exit "$EX_CONFLICT"
+fi
+
 now="$(date -u +%Y-%m-%dT%H:%M:%SZ)"
 renewed="$(jq -c --arg ts "$now" '. + {renewed_at: $ts}' <<<"$lease_json")"
 new_line="${WIT_LEASE_MARKER}${renewed} -->"