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

[kyle-sexton]

Closes #2943

Summary

Long-running /work-items:work claims stay alive via mid-flight renew-lease (issue-comment PATCH), not GitHub branch-push activity. The worker is the durable renew actor during the blocked implement-dispatch wait; the orchestrator renews only after the worker returns.

Fix

• Record lease_comment_id from claim and enumerate it in the dispatch brief next to the PR-contract list.
• Worker runs "$TRACKER" renew-lease "<id>" --lease-comment-id <n> at phase boundaries / before return. Orchestrator cannot timer-renew while waiting on the worker.
• Expired renew is exit 7 → fresh claim, not a revive.
• CONTRACT: GitHub PATCH has no If-Match/CAS (revalidation is intent, not a guarantee); clock skew; ttl-0 born-expired; comment-id monotonicity as an adapter requirement.
• Eval case work-mid-flight-renew-not-push-activity.
• work-items 0.35.27 → 0.35.28.

Stacked on #2978 (decompose deltas). Merge after that PR, or rebase onto main once #2935 lands.

Verification

• CHECK_SKILL_SKILLS_ROOT=$PWD bash plugins/skill-quality/scripts/check-skill.sh --require-evals plugins/work-items/skills/work — PASS (0 errors)
• bash plugins/skill-quality/scripts/check-evals-quality.sh plugins/work-items/skills/work/evals/evals.json — PASS
• bash plugins/work-items/tools/work-item-tracker/lib/lease.test.sh — 19/19 PASS
• wc -l plugins/work-items/skills/work/SKILL.md — 254 (under 500)

Related

Refs #2978
Refs #2933

  

[claude]

Claude Code is working…

I'll analyze this and get back to you.

View job run

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[claude]

Claude finished @kyle-sexton's task in 1m 25s —— View job

---

Security review — skip gate: no security-relevant surface

• Gather context (PR diff, changed files, CLAUDE.md/AGENTS.md instructions)
• Run skip-gate check (open PR? security-relevant surface? already reviewed?)
• Review diff for security issues per /review:security-review criteria
• Adversarially validate any candidate findings
• Post final results

I reviewed the full diff (git diff origin/main...HEAD, 15 files, +229/-24) against HEAD c2e1555. Every changed file is documentation or non-executable configuration:

• Skill/contract prose: plugins/work-items/skills/{work,decompose,setup,track}/SKILL.md, plugins/work-items/tools/work-item-tracker/CONTRACT.md, plugins/work-items/reference/agent-brief.md
• New docs-only adapter reference: plugins/work-items/tools/work-item-tracker/adapters/local-markdown/README.md
• Eval fixtures (JSON test prompts/expectations, no code): plugins/work-items/skills/{work,decompose}/evals/evals.json
• Bookkeeping: plugins/work-items/CHANGELOG.md, plugins/work-items/README.md, plugins/work-items/.claude-plugin/plugin.json (version bump only), docs/upstream/*.md

No shell scripts, GitHub Actions workflows, application/tool code, or credential-handling logic are touched in this PR — the actual renew-lease/claim shell implementation referenced in the PR description lives in files this diff doesn't modify (work-item-tracker.sh, lib/lease.sh, etc. are unchanged here). The CONTRACT.md additions are honesty/documentation clarifications about pre-existing behavior (GitHub PATCH lacking If-Match/CAS, TOCTOU window, clock skew, comment-id monotonicity) — they describe constraints, they don't introduce new attack surface.

Per the skip gate: change has no security-relevant surface after reading the diff. No findings to report.

[github-actions]

Last security-reviewed head: c2e155589cc108be3d3856c632e124c13288e347. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c2e155589c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @cursor[bot]'s task in 4m 10s —— View job

---

/review:code-review — findings

• Gather context (PR diff, CLAUDE.md/AGENTS.md, REVIEW.md)
• Run skip-gate check (open PR, non-trivial surface, no prior review of this head)
• Review diff for correctness / maintainability findings
• Adversarially validate candidate findings
• Post inline comments and final summary

Scope note: melodic-software/claude-code-plugins runs a dedicated claude-security-review lane, so this pass is scoped to correctness/maintainability per REVIEW.md's split; security-relevant surface is left to that lane.

Diff resolution note: this PR is stacked on #2978, which has since merged to main. gh pr diff 2979 against the current base now resolves to a 7-file delta (plugin.json, CHANGELOG.md, work/SKILL.md, work/evals/evals.json, CONTRACT.md, renew-lease.sh, renew-lease.test.sh) rather than the 11 files listed in the PR's file list (the other 4 are now identical to main). Review below covers that 7-file delta at HEAD ce119f4.

Both prior Codex review findings on this PR are now resolved by commits already on this branch:

• The P1 finding ("renew leases before their TTL deadline") — work/SKILL.md now instructs the worker to renew before the deadline with a safety margin rather than waiting for the next phase boundary (SKILL.md#L232).
• The P2 finding ("enforce the ttl-0 invariant in local-markdown renewals") — renew-lease.sh now calls the shared wit_lease_is_live guard before rewriting the marker (renew-lease.sh#L63-L67), matching the pattern the github adapter already used (adapters/github/renew-lease.sh#L70), and is covered by a new end-to-end test (renew-lease.test.sh) exercising a ttl-0 claim (refused, exit 7, marker unchanged) alongside a live control claim (succeeds).

No new findings. Traced through the supporting logic independently (lib/lease.sh's wit_lease_is_live/wit_lease_ttl_seconds, EX_CONFLICT=7 in adapters/local-markdown/common.sh, the claim/renew-lease exit-code contract in CONTRACT.md) and it's internally consistent: the new liveness check sits after the existing active-lease check and before the marker rewrite, refuses on ttl-0 (0-second TTL fails the ttl_seconds > 0 guard regardless of clock), and the new test's before/after marker diff correctly proves no revival occurs. CHANGELOG.md/plugin.json version bump (0.35.28 → 0.35.29) is consistent with the change and doesn't collide with the prior entry.

Not independently verified (author-claimed only): the PR description's check-skill.sh --require-evals, check-evals-quality.sh, and lease.test.sh (19/19) results — this lane's allowed-tools doesn't grant general script execution, only Read/Glob/Grep and specific gh pr * subcommands, so I read the test logic rather than running it.

No inline comments posted — nothing met the high-signal bar for this lane.

[github-actions]

Claude has reviewed this PR 2 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

Claude finished @cursor[bot]'s task in 35s —— View job

---

Code review — skip gate: PR already merged

• Gather context (PR diff, changed files, CLAUDE.md/AGENTS.md instructions)
• Run skip gate check (open PR? draft? trivial? already reviewed?)
• Post result

Per the pre-fetched context for this trigger, PR #2979 state is MERGED. Per /review:code-review's skip gate:

1. PR is closed or not open

This condition is met, so per the skill's instructions I'm stopping early and posting nothing further — no deep review of the diff was performed.

If you'd like a retrospective review of the merged changes anyway, let me know and I can run one explicitly outside the CI code-review lane's skip gate.