Skip to content

feat(work-items): lease comments carry the phase of the work - #11897

Merged
kyle-sexton merged 2 commits into
mainfrom
work-resolve-issue/issue-11871-41281676229-1
Nov 1, 2026
Merged

kyle-sexton merged 2 commits into
mainfrom
work-resolve-issue/issue-11871-41281676229-1

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Closes #11871

Summary

Opened by the work-resolve-issue lane. The run's own summary:

The github and local-markdown claim verbs now take an optional --phase triage|discovery|implementation. The value is written into the lease JSON as phase, and a lease without the field is an implementation claim. renew-lease and the github reclaim already rebuild the lease with . + {...}, so they keep the field without a code change. Design choices and their basis: the issue names triage, discovery and implementation, so the values are a closed set of those three, and a typo can't split the per-phase measurements. One shared validator, wit_lease_phase_valid in lib/lease.sh, refuses any other value (exit 2) before anything is written. The field is metadata only, so arbitration and liveness ignore it. The claim and renew result objects stay unchanged, and readers take the phase from the lease. The linear adapter's claim was left alone because the issue names only github and local-markdown; the CONTRACT and the dispatcher usage say so.

Not verified

  • The new and changed test files (adapters/github/lease-coordination.test.sh, adapters/local-markdown/claim.test.sh, adapters/local-markdown/renew-lease.test.sh) and work-item-tracker.test.sh were not run under scripts/run-test-file.sh: this lane can run only its named wrappers. Left to the PR's CI.
  • The linear adapter's claim does not accept --phase (it exits 2 on the flag); the issue scopes the change to github and local-markdown, and the CONTRACT and dispatcher usage say so
  • No test covers reclaim keeping the phase. The issue's done-when asks only for renew-lease; github reclaim keeps it through the same . + {...} rebuild, but no test runs that path
  • tidy pass 5 (ai-slop:audit fix) could not run detect.sh or digest-guard.sh, since the lane runs only named wrappers. The listed markdown lines were read by hand against the catalog instead, and nothing needed fixing
  • CI-only checks check-affected lists (shellcheck-adjacent scanners, contract-clause-coverage, purged-em-dashes and others) are left to the PR's CI

Fix

  • plugins/work-items/tools/work-item-tracker/lib/lease.sh: new wit_lease_phase_valid helper accepting triage, discovery or implementation
  • plugins/work-items/tools/work-item-tracker/adapters/github/claim.sh: parse --phase (non-empty), validate it before any network call, write phase into the lease comment JSON when set
  • plugins/work-items/tools/work-item-tracker/adapters/local-markdown/claim.sh: same --phase parsing, validation before the store write, and phase in the stored lease marker
  • plugins/work-items/tools/work-item-tracker/work-item-tracker.sh: dispatcher usage documents --phase, scoped to github and local-markdown
  • plugins/work-items/tools/work-item-tracker/CONTRACT.md: Verbs signature gains [--phase <kind>]; Lease protocol marker example gains phase, plus a bullet documenting the field, its three values, absent = implementation, the exit-2 refusal, that renew-lease and the github reclaim keep it, and
  • plugins/work-items/tools/work-item-tracker/adapters/github/lease-coordination.test.sh: new stub-driven cases. claim --phase triage writes phase to the lease comment, renew-lease keeps it (checked after a real renewed_at bump), no --phase writes no field, and an unknown phase exits 2 with no POST
  • plugins/work-items/tools/work-item-tracker/adapters/local-markdown/claim.test.sh: claim --phase triage stores the phase, no --phase stores none, and --phase review or an empty --phase exits 2 and leaves the item file unchanged
  • plugins/work-items/tools/work-item-tracker/adapters/local-markdown/renew-lease.test.sh: claim --phase triage through the dispatcher, then renew-lease keeps phase triage in the store with the reported renewed_at
  • .changes/work-items/work-resolve-issue-issue-11871-a7c4d2e9.md: minor changelog fragment for the new --phase option (written by hand because new-changelog-fragment refuses a detached HEAD)

Verification

The runner ran each check from the base copy against 9b1e3cc:

  • scripts/check-changelog-fragments.sh --check: passed

The run reports:

  • /home/runner/work/claude-code-plugins/claude-code-plugins/.lane/checks/bin/check-changelog-fragments: All 55 changelog fragment(s) under .changes/ are valid (run before and after tidy and the review fixes)
  • /home/runner/work/claude-code-plugins/claude-code-plugins/.lane/checks/bin/check-affected: 12 passed, 6 skipped over 9 changed files (run before and after tidy and the review fixes)
  • fresh-context review subagent over the whole diff: no high or medium findings. Its read-only trace says the new github stub cases and local-markdown cases should pass and none can pass vacuously. Three low findings (Verbs signature missing --phase, empty --phase accepted, reclaim claim overstated fo
  • code-tidying:tidy-diff over all 24 changed hunks: no edits needed
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/41281676229

Left out: the items under Not verified.

Related

#11871

Lane-Activity: work-resolve-issue/resolve-issue
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review needs no new review of 47b71273c3bc91c44a0fe51a16283aaf213601af: no file of this pull request changed since the review of 9b1e3cc.

Security review: PR #11897, lease comments carry the phase of the work

No security issues found.

I read every hunk in all 9 changed files. None of them match a trusted-path glob. The diff doesn't touch any trust control, merge hold, kill switch, token request or plugin manifest. It doesn't delete or weaken any standing instruction either.

What I checked:

  • Input reaching the lease JSON: both adapters reject a --phase value outside a fixed set (triage, discovery, implementation). The check is wit_lease_phase_valid in lib/lease.sh, and it runs before any write. The value then goes to jq with --arg phase, so it is always quoted as a JSON string and is never parsed as jq code or shell. An empty --phase is rejected when the arguments are parsed. Nothing the caller passes can break out of the `` HTML comment, so the stored lease can't be malformed or carry injected text.
  • Arbitration and liveness: the new field is only metadata. wit_lease_is_live and the claim race arbitration don't read it, so phase can't bypass or extend a lease. A triage lease still blocks other claims like any live lease.
  • Command injection: the new code adds no eval and no unquoted expansion. phase only flows into jq --arg and a case match.
  • Tests: the test changes use local stubs and temporary stores, with no credentials and no network.

critical-findings: 0

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 1 inline finding(s) on 9b1e3cc57cd92e384119489502b59576a06378e5; the summary is in this lane's pull request comment.

claude-lane: claude-review

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

claude-review needs no new review of 47b71273c3bc91c44a0fe51a16283aaf213601af: no file of this pull request changed since the review of 9b1e3cc.

Code review: PR #11897, lease comments carry the phase of the work

I read every hunk of all 9 changed files. To check the PR's claims I also read the unchanged adapters/github/reclaim.sh, the renew-lease.sh jq rebuilds in both adapters, and the github test's gh stub.

Correctness. I found no defects.

  • Both claim.sh adapters check --phase with the shared wit_lease_phase_valid before any network call or store write, and an empty value is refused.
  • The jq builder adds phase only when it is set, so a lease without --phase stays byte-identical to today's.
  • renew-lease in both adapters, and both write paths of github reclaim (renew in place at reclaim.sh:56 and supersede at reclaim.sh:87), use . + {…}, so the field survives.
  • The new github stub scenarios use stateful mode, so the PATCH really rewrites the stored body and the renew assertion cannot pass vacuously.
  • The local-markdown "no --phase" check reuses $STORED from the first claim in the file, which had no --phase, so the check is valid.

Closing issue #11871, criteria:

  • CONTRACT documents the field and its values: met (CONTRACT.md Lease protocol bullet and marker example).
  • Tests show claim --phase triage writes the field and renew-lease keeps it, for github and local-markdown: met in code. That they pass under scripts/run-test-file.sh, and that work-item-tracker.test.sh passes, is claimed by the author but unverified. The PR says the tests were not run and leaves them to CI. I ran nothing.
  • Changelog fragment passes the check: the fragment is present with bump: minor, which fits a new optional flag. That it passes the check comes from the runner's report; I did not run it.
  • Fix section, "reclaim and renew-lease keep it": met in code, but no test covers reclaim (see the inline comment).

Path-scoped rules. None of the .claude/rules/ globs match a changed file.

Manifests. No plugin.json or marketplace.json changed, so there are no manifest lines.

Embedded text. Neither the PR body nor the issue contains a directive aimed at the review lane.

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The unattended merge path armed native auto-merge at head 9b1e3cc57cd92e384119489502b59576a06378e5.

@melodic-automation-lanes

melodic-automation-lanes Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor Author

The pr-unblock lane stopped address-feedback on this pull request: the pull request has an unresolved review thread the lane does not take (a reviewer it does not act on, a review of an older head, or a thread it already answered), so no lane resolves it and a session builder takes it. The lane labels it needs-session for a session builder, who makes the change this lane cannot, and the run ends in a failing pr-unblock/address-feedback status on the head the lane gated.

When a retry on this head can change the outcome, the lane runs one on its own, up to its per-head cap, and edits this comment if it stops again; a run that does not stop clears the status. When no retry can (a review thread the lane does not take or settle, or a failing check after the retries are spent), a person resolves the threads or gets the checks green, then dispatches the workflow below: that run finds the cause gone and clears the status. On a pull request the lanes App opened, the dispatchers start that run once per stop; on any other, a person does. A new commit to the branch starts clean.

To run it again on this head now, dispatch the pr-unblock workflow:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=11897 -f activity=address-feedback

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

A lane run on this pull request's head failed because the token broker refused it as workflow-modified: .github/workflows/pr-unblock.yml changed on main after the run's event, and a rerun keeps the old file. pr-merge pushes an empty commit so the next pr-unblock run uses main's files. It does this once per version of that file, so if a run fails the same way again, a person looks at it. The push starts no run itself: the conflict-resolution run starts at the next push to main or half-hourly pr-unblock-dispatch tick, or when a listed person runs it by hand, and an address-feedback run can start after both review lanes review the new head, if an unresolved thread is left for it.

The token broker refused a lane run on the previous head as workflow-modified (.github/workflows/pr-unblock.yml). This commit changes no file.
@melodic-automation-lanes melodic-automation-lanes Bot added the needs-session A pr-unblock stop a session builder takes: a change the lane cannot make. Not an owner stop. label Nov 1, 2026
@melodic-automation-lanes

melodic-automation-lanes Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor Author

Unattended merge: native auto-merge leaves this pull request unarmed at head 47b71273c3bc91c44a0fe51a16283aaf213601af. The lanes clear this reason on their own; the next scheduled run judges it again.

pr-merge's gate holds it on its risk score (evidence-unbound:claude-review: not bound: no-run;
  evidence-unbound:claude-security-review: not bound: no-run)

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The unattended merge path armed native auto-merge at head 47b71273c3bc91c44a0fe51a16283aaf213601af.

@melodic-automation-lanes
melodic-automation-lanes Bot added this pull request to the merge queue Nov 1, 2026
@kyle-sexton kyle-sexton removed the needs-session A pr-unblock stop a session builder takes: a change the lane cannot make. Not an owner stop. label Nov 1, 2026
@melodic-automation-lanes
melodic-automation-lanes Bot removed this pull request from the merge queue due to a manual request Nov 1, 2026
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit 75087c1 Nov 1, 2026
91 of 94 checks passed
@kyle-sexton
kyle-sexton deleted the work-resolve-issue/issue-11871-41281676229-1 branch November 1, 2026 10:46
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.

feat(work-items): lease comments carry the phase of the work

1 participant