feat(work-items): bundle work-item-tracker seam in plugin (shape A)#224
Conversation
Move the work-item-tracker seam (dispatcher, lib/, CONTRACT.md, github +
local-markdown adapters, conformance) from medley into the work-items plugin so
installing the plugin delivers the engine and the shipped adapters — no per-repo
vendoring.
- Two-rule resolver (CONTRACT.md "Adapter resolution"): seam code plugin-dir
canonical with a project-root fallback; adapters consumer-local-first with a
plugin-bundled fallback, first match wins. WIT_ADAPTERS_DIR stays a single-root
test override. New dispatcher tests cover both adapter paths.
- /work-items:setup now seeds .work-item-tracker.json (provider + non-secret
config) as the once-per-repo binding step; the three obsolete "no setup
wizard / skills don't bind the provider" statements are corrected.
- Self-contained test harness (tools/work-item-tracker/tests/lib.sh) so the
seam's .test.sh files run wherever the seam resolves from, with no host-repo
test-tooling dependency.
- CONTRACT.md and the GitHub adapter reference de-medley-ised for portability;
skill citations point at ${CLAUDE_PLUGIN_ROOT}/tools/work-item-tracker/...
- plugin.json 0.7.0 -> 0.8.0; CHANGELOG + regenerated root catalog.
Executes shape A from melodic-software/medley#1502; the medley-side de-vendor
and the ADR-0022 reversal are left as a described follow-up in the PR body.
Refs melodic-software/medley#1505
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D9yeNF5Mp9U5voSUpHXMUk
Match the marketplace's proven executed-bash idiom for invoking a bundled
script (claude-config / claude-memory / claude-ops / context7 all use plain
"${CLAUDE_PLUGIN_ROOT}/skills/.../x.sh"). The ${VAR:+...} conditional form would
break if the harness text-substitutes the exact ${CLAUDE_PLUGIN_ROOT} token
instead of exporting it as a shell var; the plain form is robust either way —
the plugin copy when set, project-root fallback when unset.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D9yeNF5Mp9U5voSUpHXMUk
The seam relocation from medley dropped the executable bit on 54 shebang scripts under work-item-tracker/adapters/ during the move.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03fdd2b3db
ℹ️ 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".
Resolve work-items plugin conflicts between shape-A seam bundling and main's prerequisite-visibility wave + scan->scan-todos rename: - plugin.json / CHANGELOG: renumber the bundle-seam release 0.8.0 -> 0.11.0 (non-breaking feat, above main's shipped 0.10.0); keep the shape-A description. All of main's 0.10.0/0.9.0/0.8.1/0.8.0 changelog entries retained. - README / setup SKILL: compose both sides -- main's Bash+jq requirement/check plus the PR's bundled-seam rewrite and 3-concern setup restructure. - reference/tracker-seam: adopt the shape-A model and fold in main's jq + seam-script presence checks, adapting the seam-script remediation from "consumer-provided, copy it in" to "plugin-bundled; reinstall/repair if it resolves at neither the plugin path nor the project-root fallback." The "/work-items:setup does NOT create the seam" note is preserved.
|
Claude finished @kyle-sexton's task in 5m 8s —— View job PR Review — feat(work-items): bundle work-item-tracker seam in plugin (shape A)
OverviewThis is a well-structured relocation of the work-item tracker seam into the plugin. The two-rule resolver is correctly implemented, the conformance suite travels with the code it tests, shellcheck integration is thorough, and the MSYS/Git Bash process-substitution workaround is consistently applied where needed. The setup skill changes are comprehensive and correctly warn about the That said, two P1 correctness bugs need fixes before merge, one P2 bug needs addressing, and there is a security gap in provider name validation. P1 —
|
|
Claude finished @kyle-sexton's task in 2m 18s —— View job PR Review — feat(work-items): bundle work-item-tracker seam in plugin (shape A)
What the recent fix commits addressedTwo fixes landed since the original review comment:
Outstanding — P1, P2, and Security findings are still openThe four issues raised in the prior review are unchanged in the current tree. Confirming each against the live files: P1 —
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4ad2e3dd2
ℹ️ 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".
…tial claims Root-cause fixes for the codex/claude[bot] findings on PR #224: - lib/binding.sh: root a relative config.storage_dir against the binding file's own directory instead of the caller's CWD, so a verb invoked from a subdirectory resolves the same local-markdown store as one invoked from the repo root. Validate provider against ^[a-zA-Z0-9_-]+$ before it is used to build an adapter directory path (path-traversal hardening). Read and export config.role_labels["human-gated"] as WIT_HUMAN_GATED_LABEL, defaulting to "needs-human" when unset. - lib/frontier.sh + work-item-tracker.sh: wit_filter_frontier now takes the resolved human-gated label as a parameter instead of hardcoding "needs-human", so list-frontier --autonomous honors a repo's configured role-label remap. - adapters/github/claim.sh: install an EXIT-trap rollback of the @me assignment for the window between a successful assignment and a successful lease-comment write, so a failure in between no longer strands the issue assigned with no lease record. wit_run_gh's failure path calls `exit` directly rather than returning, and an explicit `exit` inside a function does not run the caller's ERR trap (verified empirically) — only EXIT does. New coverage: binding.test.sh (relative storage_dir rooting, provider validation, role_labels remap/default) and frontier.test.sh (configured human-gated label). Full work-items suite + local-markdown conformance (normal + gh/curl-blocking-shim runs) verified green locally.
This PR's own Provider binding section (skills/setup/SKILL.md) newly makes local-markdown a real, user-selectable binding, but local-markdown's capabilities.json declares reclaim: false (no reclaim.sh — claim.sh already race-checks pre-write instead) and the dispatcher's capability gate maps that to exit 6 (EX_CAPABILITY, CONTRACT.md "Exit codes"). track/actions/start.md, work/SKILL.md Step 0, and track/actions/audit.md all call reclaim unconditionally on every assigned item and only handled the "still assigned" outcome, not exit 6 — so a repo bound to local-markdown would stop at the very first reclaim pre-check in /work-items:track start, /work-items:work, and /work-items:track audit. Document exit 6 as the documented degradation (CONTRACT.md already defines it as "capability-unsupported", not an error) in all three call sites: skip the stale-lease check and continue rather than treating it as an unhandled failure. Flagged by chatgpt-codex-connector[bot] on PR #224 (capabilities.json:9, posted after the initial review round).
|
Claude finished @kyle-sexton's task in 2m 46s —— View job PR Review — feat(work-items): bundle work-item-tracker seam in plugin (shape A)
All prior findings resolvedThe two fix commits ( ✅ P1 —
|
|
Classification table for the findings in this review (per-finding, evidence + fix commit):
Fixes for 1-4: 9023fa7. Also fixed a distinct P1 codex flagged after this review ( Full |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f15a32999f
ℹ️ 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".
Follow-up to 9023fa7's partial-claim guard, flagged by codex on the fix itself: disarming the EXIT trap right after the lease comment is created closed the assign→lease-write window but reopened an equivalent one at the very next step — a failure re-reading lease comments for arbitration (`wit_list_lease_comments`) would exit with @me assigned and a live lease comment whose id was never returned to the caller, stranding the item the same way until TTL expiry. The rollback now stays armed through the arbitration read and disarms at exactly the two outcomes where keeping @me is correct: a same-login race (the earlier comment from this identity wins, so the assignee still matches the holder) and final success. The different-holder race still removes @me explicitly as before; the trap firing there too is a harmless redundant call.
|
Claude finished @kyle-sexton's task in 2m 59s —— View job PR Review — feat(work-items): bundle work-item-tracker seam in plugin (shape A)
What f676872 fixed✅ Codex P1 — Claim rollback stays armed through arbitration (fixed)
The prior code disarmed The fix correctly defers the disarm:
The comment at lines 41–47 explaining why it must be an EXIT trap (not ERR) is accurate and appropriately preserved from the prior commit. Outstanding — two Codex P2 findings remain openP2 —
|
|
Thanks for confirming. One more round landed since this review: codex caught a follow-on gap in the |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f676872e4f
ℹ️ 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".
Reconcile the work-items setup skill onto main's uniform check|apply contract while preserving this branch's provider-binding deliverable: apply binds the provider first (before the schedule and role-label passes), and check gains a read-only binding probe. plugin.json -> 0.12.0 (above main's current 0.11.0); CHANGELOG keeps main's 0.11.0 check/apply entry and adds a 0.12.0 entry for the bundle + provider-binding-via-apply.
|
Claude finished @kyle-sexton's task in 3m 46s —— View job PR Review — feat(work-items): bundle work-item-tracker seam in plugin (shape A)
What the merge commit introducedThe merge (
All prior P1/P2/Security fixes confirmed intactVerified against the live files at HEAD:
|
| Finding | Location | Status |
|---|---|---|
link-blocks.sh silently resolves foreign-namespace IDs |
adapters/local-markdown/link-blocks.sh:38 |
Deferred follow-up issue (pre-existing moved code; needs store-format design decision) |
create-item.sh body text parsed as structural dependency |
adapters/local-markdown/create-item.sh:115 |
Deferred follow-up issue (pre-existing moved code; same design-decision scope) |
local-markdown/claim.sh stale assignee after lease expiry |
adapters/local-markdown/claim.sh:48 |
Deferred follow-up issue (consequence of exit-6 reclaim gap; needs local reclaim design decision) |
adapters/github/common.sh gh-bot.sh always falls back to bare gh |
adapters/github/common.sh:20 |
Intentional-for-this-PR; dependency #1 / option (c) follow-up |
All four are tracked and confirmed not modified by the merge.
Summary
The merge correctly integrates the 0.11.0 check | apply contract from main with this branch's provider-binding deliverable. All four prior P1/P2 fixes are intact. The check mode is well-specified and the apply step ordering is correct. The two minor observations above (no check evals, local-markdown label-registry ambiguity in probe 6) are non-blocking for a draft gated on human review.
The PR remains draft-status pending: live GitHub conformance run against a throwaway sandbox, human confirmation that bare-gh attribution (dependency #1) is acceptable for this release, and the runtime verification of the ${CLAUDE_PLUGIN_ROOT}-canonical resolver in the target consumer.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce3c0c97f1
ℹ️ 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".
Both adapters' common.sh sourced the shared seam libs (id/json/lease) relative to the adapter's own dir. When a consumer shadows an adapter with a local copy (the resolver's supported consumer-local path) that cannot resolve the bundled lib/, the source failed silently (no set -e) and a verb could emit a malformed empty-id record with exit 0 — silent store corruption. Verify each required seam lib exists before sourcing; on a miss, print a diagnostic and exit 3 (setup/config error). The seam-wide design question of how a shadowed adapter should resolve shared libs is tracked as a follow-up.
|
Claude finished @kyle-sexton's task in 2m 10s —— View job PR Review — feat(work-items): bundle work-item-tracker seam in plugin (shape A)
What
|
| Finding | Location | Tracked |
|---|---|---|
gh-bot.sh falls back to bare gh (intentional) |
adapters/github/common.sh:37 |
#365 |
link-blocks.sh silently resolves foreign-namespace IDs |
adapters/local-markdown/link-blocks.sh:38 |
#366 |
create-item.sh body text parsed as structural dependency |
adapters/local-markdown/create-item.sh:115 |
#366 |
local-markdown/claim.sh stale assignee after lease expiry |
adapters/local-markdown/claim.sh:48 |
#367 |
local-markdown/claim.sh wit_fm_set failure not propagated |
adapters/local-markdown/claim.sh:62 |
#367 |
None of these files were touched by 9b8ed32. All remain on their tracked follow-up issues.
Summary
9b8ed32 correctly closes the Codex P1 flagged in the last round — the store-corruption risk on consumer-local adapter shadows is eliminated, with a clear diagnostic on failure and the right exit code. All prior P1/P2/Security fixes are intact. No regressions found.
The PR remains draft-status pending: live GitHub conformance run against a throwaway sandbox, human confirmation that bare-gh attribution (dependency #1, tracked in #365) is acceptable for this release, and runtime verification of the ${CLAUDE_PLUGIN_ROOT}-canonical resolver in the target consumer. No code correctness issues remain.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b8ed32795
ℹ️ 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".
Resolves the work-items collision with #224 (seam shape A landed 0.12.0): main's version and description win; this branch's cross-plugin token update rides on top as 0.12.1 with its changelog entry re-stacked above main's 0.12.0. New #224 files verified free of old invocation tokens. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- orchestration.md: replace repo-specific PR-number example (medley#1567/#224 couplings) in Cross-PR Dependency Signalling with a generic `owner/repo#123 ↔ #456` placeholder, per the repo-agnostic design rule in CLAUDE.md; also fixes the awkward mid-sentence line wrap. - pr_queue_snapshot.py: exit-code-3 docstring now states state is still written when requested, matching the explicit write contract already documented for codes 0 and 1 (write_state runs unconditionally on --write-state, before exit_code_for).
…rule, snapshot exit-code taxonomy, cross-PR dependency channel (#456) Closes #373. Implements the four enhancements plus one gotcha from issue #373 in the `babysit-prs` skill. The source retrospective the issue cites (`.work/handoffs/babysit-autopilot-retro.md`) is not present in the checkout (`.work/` is gitignored), so every edit was driven by the issue body plus the live skill files; the retro is provenance only. All doc edits are pointer-style (they point at the existing rule/clause, never restate it). ## (a) No-monitor hard STOP at the merge / gate-completion step Added in both `SKILL.md` (Guarded mutations) and `reference/safety.md` (Pinned-Command Degradation): once a PR is proven ready, or its merge is deferred to a human, report and stop — never arm a watch/monitor. Both point at the existing no-background-monitor clause in the Worker Contract (`reference/orchestration.md`) rather than restating it. ## (b) Bare-wrapper pipe rule `reference/safety.md` Guarded Mutation Wrappers now states that the wrapper's JSON must be parsed in a separate step — never `wrapper | python` / `| jq` — because an interpreter-in-pipeline trips the auto-mode safety classifier and blocks the call before the wrapper runs. ## (c) Snapshot exit-code taxonomy `scripts/pr_queue_snapshot.py` previously collapsed every non-fatal error into exit `1`, so a purely-advisory head-ref alias-check error was indistinguishable from a substantive per-PR hydration failure. The split: | code | meaning | state written | | --- | --- | --- | | `0` | valid snapshot, no errors | yes | | `1` | valid snapshot, ≥1 substantive error (per-PR hydration or discovery failure) | yes | | `2` | fatal — exception before a snapshot existed | no | | `3` | valid snapshot, advisory-only (head-ref alias cross-check) errors | yes | Substantive errors take precedence over advisory (a run with both returns `1`). The advisory marker is a single-source constant (`babysit_delta.HEAD_REF_ALIAS_ERROR_MARKER`) used at both message-construction sites (queue path in `babysit_delta.py`, single-PR path in `pr_queue_snapshot.py`) so they cannot drift. Taxonomy documented in the module docstring; new `scripts/tests/test_pr_queue_snapshot.py` covers 0/1/2/3 including the advisory-vs-substantive distinction and precedence. **Consumers checked** (exit codes are a caller contract): every `*.md`, `*.sh`, `*.py`, `engine.test.sh`, orchestration doc, and `bin/` wrapper referencing the snapshot. No live consumer keys on the script's `$?` — the docs (`orchestration.md`, `SKILL.md`, `loop.md`) invoke it and parse the JSON snapshot; `babysit-python.sh` uses `exec` (child exit code passes through unchanged); no `bin/` wrapper or sibling script branches on it. Introducing `3` therefore breaks no caller that treated `1` as "any error". ## (d) Worker→main cross-PR dependency channel New `Cross-PR Dependency Signalling` section in `reference/orchestration.md` formalizing a worker signalling a discovered cross-PR coupling (e.g. medley#1567 ↔ #1597, #224 ↔ #356) back to the main agent, which owns cross-PR ordering. Points at the existing `SendMessage` mechanism (documented there for main→worker), explicitly reversed. ## Gotcha: self-blocking CI check Recorded in the `SKILL.md` Gotchas list: a newly required check whose own fix PR carries that same check cannot be gate-merged and needs a one-time human admin-merge bootstrap. The concurrent-same-identity yield rationale is already covered by the Checkout/Push invariants (head-SHA recheck + worker lease + `foreign_activity` suppressor) and is deliberately not duplicated. ## Verification - `bash engine.test.sh`: `Ran 186 tests … OK`; ruff `All checks passed!`; guarded-wrapper behavior checks all PASS. - markdownlint-cli2 on the four changed `.md` files: 0 issues. - Plugin bumped `0.9.0` → `0.9.1` (`jq -e .version` confirms) with a matching `## [0.9.1]` CHANGELOG entry. - No trailing whitespace / final-newline issues in any changed file. ## Related - #322 (PR-B) — retro source for these enhancements. - #324 — sibling review-trigger reconciliation (F8 posting-guard follow-on territory). - #326, #327 — same babysit-prs safety-hardening cluster. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#680) ## Summary Follow-up from #224. Closes two related lease/assignee-integrity gaps in the `local-markdown` adapter's `claim.sh` (relocated byte-for-byte from medley by #224, not changed there), handled as the single hardening pass the issue calls for. No new adapter capability is introduced — this implements the issue's recommended, less-invasive candidate for Finding 1 (list-derivation projection), so it is a patch bump (`0.14.1` → `0.14.2`). ## Fix **Finding 1 (P1) — expired lease never cleared the assignee, so abandoned claims never returned to the frontier.** An expired lease still left `assignees` populated. Because `reclaim` is unsupported for this offline adapter (it has no coordination surface to run an activity check over) and the core `list-frontier` derivation always excludes assigned items, any abandoned local claim was permanently absent from selection once its TTL expired. - `list-items` now projects an expired-lease item's **effective** assignee as empty (`wit_emit_local_item` gained an opt-in `unassign-expired` mode), so the core frontier filter — which keys on `assignees` — returns the item to selection. No new capability, no stored mutation. - The projection is scoped to list/frontier derivation. `get-item` still reports the **stored** assignee verbatim, preserving parity with the GitHub adapter (whose assignee persists until `reclaim` runs) and keeping the raw claim record inspectable. - `CONTRACT.md`'s local-markdown degradation note now documents this offline compensation. **Finding 2 (P2) — a failed assignee write still reported a successful claim.** `claim.sh` appended the inline lease marker and then called `wit_fm_set … assignees` with no return-code check and no `set -e`, so a failed assignee write (store full/unwritable) was silently ignored and a success JSON was still emitted — leaving a live lease marker with an empty `assignees`, which `list-frontier` presents as available while later claims conflict on the live lease until it expires. - The marker append and assignee write are now one consistent operation (`wit_claim_write`): on a failed assignee write it best-effort rolls the just-appended marker back and fails the claim with **exit 1** (internal/unexpected store I/O, distinct from an exit-7 claim race), emitting no success record for a half-applied write. ## Verification New `adapters/local-markdown/claim-integrity.test.sh` covers both findings — Finding 1 end-to-end through the core CLI (`list-frontier`), Finding 2 as a deterministic, OS-portable unit test of the rollback (the abstract conformance suite runs against every adapter and cannot hold local-markdown-specific or fault-injection cases). All checks run via the repo harness (`plugins/**/*.test.sh`). New test (`bash claim-integrity.test.sh`), exit 0: ``` PASS: [1] expired-lease A returns to the frontier PASS: [2] live-lease B stays out of the frontier PASS: [3] get-item leaves the expired-lease assignee stored PASS: [4] successful claim write returns 0 PASS: [5] successful claim write keeps the lease marker PASS: [6] successful claim write records the assignee PASS: [7] failed assignee write fails the claim PASS: [8] failed assignee write rolls the lease marker back PASS: [9] failed claim leaves assignees empty ``` Full local-markdown conformance still green, including the offline gh/curl-blocking shim (`run-conformance.sh --binding local-markdown`): ``` Conformance (local-markdown): 61 cases, 0 failed ``` ShellCheck clean on all changed scripts (`--rcfile .shellcheckrc`); every `plugins/work-items/**/*.test.sh` passes; changed files carry no trailing whitespace, tabs, or comment issue/TODO markers. Closes #367 ## Related - #367 — this issue. - #224 — the bundle-seam PR that relocated this code (shape A); both gaps were deferred there by author delegation. 🤖 Generated with a Claude Code implementation subagent (issue #367) --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…686) ## Summary Follow-up from #224. Closes two lease-coordination correctness gaps in the GitHub adapter's `renew-lease.sh` and `reclaim.sh` — both in code relocated byte-for-byte from medley by #224 (not changed there), handled as the single hardening pass the issue calls for. No new adapter capability; a bug fix, so a patch bump (`0.14.2` → `0.14.3`). This is the GitHub-adapter sibling of #367 (local-markdown), shipped earlier this session. ## Fix **Finding 1 (P1) — `renew-lease` revived an expired lease.** `renew-lease` selected the active (newest non-superseded) lease, confirmed `active_id == lease_comment_id`, and PATCHed a fresh `renewed_at` — but never checked liveness first. A worker retaining its handle past `renewed_at + ttl_hours`, with no newer lease comment, still matched `active_id`, so the PATCH revived an expired lease and defeated TTL-based handoff: a crashed/delayed worker could reclaim an item another worker had reasonably treated as expired. - `renew-lease.sh` now checks `wit_lease_is_live` immediately before patching and returns a conflict (`exit 7`) for an expired-but-not-superseded active lease instead of reviving it. Recovery from an expired lease is a fresh claim/reclaim, not a revive of the dead handle. **Finding 2 (P1) — `reclaim` removed assignees that did not hold the expired lease.** On the expired-lease + no-activity path, `reclaim` read every assignee (`[.assignees[].login]`) and removed each one — not just the lease's `holder`. A user manually assigned after the old lease, or a concurrent claimer added before this assignee snapshot, was silently unassigned; in the concurrent case that claimer's live lease stayed while the frontier treated the item as unassigned (two workers on one item). - `reclaim.sh` now removes **only the expired lease's `holder`** (guarded on a fresh assignee read so an already-unassigned holder is a no-op), leaving co-assignees untouched. - Ownership is **revalidated immediately before mutating**: the lease was selected before the two activity round-trips, a TOCTOU window in which a concurrent claimer can renew or supersede it. `reclaim` re-reads the leases and confirms this is still the active *and still expired* lease; if a concurrent claim won the item in that window, reclaim is a no-op (`reclaimed: false`) — never a mutation that strips the new owner. Both verbs now share `wit_select_active_lease` (extracted to `lib/lease.sh`) instead of a third copy of the active-lease selection loop. `CONTRACT.md`'s "Lease protocol" documents both semantics (renew refuses an expired lease; reclaim is holder-only and revalidated). ## Verification New offline test `adapters/github/lease-coordination.test.sh` drives both fixes against a stubbed `gh` (the GitHub-adapter conformance binding requires a live target, so these decision-path cases live in a dedicated test — mirroring the sibling's `claim-integrity.test.sh` precedent). It asserts on the recorded write/remove call-log, not just exit codes. Against the fix — `bash adapters/github/lease-coordination.test.sh`, exit 0 (cases 14–17 cover the concurrent in-place-renew revalidation branch, added per review): ``` PASS: [1] renew-lease returns conflict (7) for an expired active lease PASS: [2] renew-lease does NOT revive the expired lease (no PATCH) PASS: [3] renew-lease succeeds (0) for a live active lease PASS: [4] renew-lease emits the renewed lease record PASS: [5] renew-lease PATCHes the live lease comment PASS: [6] reclaim succeeds (0) PASS: [7] reclaim reports reclaimed:true PASS: [8] reclaim removes the expired lease holder (alice) PASS: [9] reclaim leaves the co-assignee (bob) untouched PASS: [10] reclaim succeeds (0) on a concurrent-claim revalidation PASS: [11] reclaim reports reclaimed:false when the lease changed under it PASS: [12] reclaim removes NO assignee when revalidation fails PASS: [13] reclaim does not supersede when revalidation fails PASS: [14] reclaim succeeds (0) when the lease was renewed in place during the window PASS: [15] reclaim reports reclaimed:false when the lease was renewed in place PASS: [16] reclaim removes NO assignee when the lease was renewed in place PASS: [17] reclaim does not supersede when the lease was renewed in place ``` Discrimination proof — the (13-case) test against the **pre-fix** `renew-lease.sh`/`reclaim.sh` at `origin/main` (exit 1), confirming the cases fail without the fix and are not passing for the wrong reason: ``` FAIL: [1] renew-lease returns conflict (7) for an expired active lease — expected 7 got 0 FAIL: [2] renew-lease does NOT revive the expired lease (no PATCH) — expected no PATCH logged got PATCH logged PASS: [3] renew-lease succeeds (0) for a live active lease PASS: [4] renew-lease emits the renewed lease record PASS: [5] renew-lease PATCHes the live lease comment PASS: [6] reclaim succeeds (0) PASS: [7] reclaim reports reclaimed:true PASS: [8] reclaim removes the expired lease holder (alice) FAIL: [9] reclaim leaves the co-assignee (bob) untouched — forbidden REMOVE_ASSIGNEE bob present PASS: [10] reclaim succeeds (0) on a concurrent-claim revalidation FAIL: [11] reclaim reports reclaimed:false when the lease changed under it — expected false got true FAIL: [12] reclaim removes NO assignee when revalidation fails — forbidden REMOVE_ASSIGNEE present FAIL: [13] reclaim does not supersede when revalidation fails — forbidden PATCH present ``` Direct unit coverage of the extracted helper added to `lib/lease.test.sh` (newest-non-superseded selection, superseded back-off not masking an earlier active lease, empty out-params when none) — 15/15 pass. Full `plugins/work-items/**/*.test.sh` sweep green; ShellCheck clean (`--rcfile .shellcheckrc`) on every changed script; changed files carry no trailing whitespace or tabs. Closes #370 ## Related - #370 — this issue. - #224 — the bundle-seam PR (shape A) that relocated this code byte-for-byte; both findings were deferred there by author delegation. - #367 — the sibling `local-markdown` lease/assignee-integrity fix shipped earlier this session (same lease-integrity class, different adapter). 🤖 Generated with a Claude Code implementation subagent (issue #370) --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
… of adapter location (#826) ## Summary - The GitHub adapter's `WIT_GH_BOT` resolved the bot wrapper relative to the adapter's own directory (`${CLAUDE_PLUGIN_ROOT}/tools/github-auth/gh-bot.sh` in the normal bundled path), never checking the consumer-local override path CONTRACT.md already documented (`${CLAUDE_PROJECT_DIR}/tools/github-auth/gh-bot.sh`). - As a result, a consuming repo's own bot wrapper was never found, and all tracker writes (item create, lease/reclaim comments) silently fell back to the ambient `gh` (session-user) identity instead of bot attribution. ## Fix - `common.sh`: extracted `wit_gh_resolve_bot_wrapper`, which now resolves the bot wrapper **consumer-local-first, plugin-bundled fallback** — checking `${CLAUDE_PROJECT_DIR}/tools/github-auth/gh-bot.sh` first (independent of where the adapter itself resolved from, so a shadowed consumer-local adapter still finds the consumer's wrapper), then falling back to the bundled path beside the seam tree. This mirrors the existing two-rule adapter resolution documented in CONTRACT.md's "Adapter resolution" section. - `CONTRACT.md`: updated the "Identity routing (GitHub adapter)" section to document the corrected consumer-local-first/plugin-bundled-fallback resolution order (previously it documented the buggy adapter-relative-only behavior). - `common.test.sh`: added regression coverage for `wit_gh_resolve_bot_wrapper` — resolves the consumer-local wrapper when present, falls back to the bundled path when the consumer has none, and falls back when `CLAUDE_PROJECT_DIR` is unset. ## Verification Full adapter + dispatcher test suite run, all green: ``` $ bash adapters/github/common.test.sh PASS: [1] common.sh exposes gh_write PASS: [2] common.sh exposes wit_run_gh PASS: [3] common.sh exposes wit_resolve_repo PASS: [4] common.sh exposes wit_emit_item PASS: [5] common.sh exposes wit_lease_json PASS: [6] common.sh exposes wit_lease_is_live PASS: [7] common.sh exposes wit_list_lease_comments PASS: [8] common.sh exposes wit_help_if_requested PASS: [9] common.sh exposes wit_map_gh_error PASS: [10] lease marker constant PASS: [11] 404 → not-found (5) PASS: [12] rate limit → unavailable (8) PASS: [13] resolves consumer-local wrapper first when present PASS: [14] falls back to bundled path when consumer has none PASS: [15] falls back to bundled path when CLAUDE_PROJECT_DIR unset ``` All other `adapters/github/*.test.sh` files and `work-item-tracker.test.sh` (including its own consumer-local/adapter-resolution cases) also pass unchanged. `shellcheck` on both modified shell files: no findings. Closes #365 ## Related - #365 - Originates from the #224 review thread (deferred dependency #1, option (c)) Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Executes shape A of the tracker-seam distribution decision (
melodic-software/medley#1502): move thework-item-trackerseam out of medley and bundle it in thework-itemsplugin, so installing the plugin delivers the engine and the shipped adapters — no per-repo vendoring.Refs melodic-software/medley#1505 (non-closing — the medley-side de-vendor + the ADR-0022 reversal are described-but-not-executed here). Part of the shape-A decision under
melodic-software/medley#1502(EPIC-adjacent; not closed by this PR).Related
No linked issue in this repository. Related decision-log entries this PR does not close:
melodic-software/medley#1502— shape-A tracker-seam distribution decision (EPIC-adjacent).melodic-software/medley#1505— medley-side de-vendor + ADR-0022 reversal spec (described here; executed in a follow-up medley-side PR per the "Medley-side follow-up" section below).What changed (plugin side)
plugins/work-items/tools/work-item-tracker/— dispatcher,lib/,CONTRACT.md, thegithub+local-markdownadapters, and the conformance suite (byte-identical move;migrations/deliberately left in medley — medley-specific, out of scope).tools/work-item-tracker/tests/lib.sh(the 6 assertion helpers its.test.shfiles use + counter defaults + the git-env unset guard); every.test.shnow sources it self-locating (relative to its own dir), with zero dependency on any host-repo test tooling.CONTRACT.md"Adapter resolution"):"$TRACKER"this way, then invoke it).${CLAUDE_PROJECT_DIR}/tools/work-item-tracker/adapters/<provider>/), plugin-bundled fallback; first match wins.WIT_ADAPTERS_DIRremains a single-root test override. New dispatcher tests + a live throwaway-fixture demo cover consumer-local shadow, unshipped-provider addition, and bundled fallback.setupskill)./work-items:setupnow seeds.work-item-tracker.json(provider + non-secret config) as the once-per-repo binding step, before the recurring-schedule and role-label passes. Shape chosen to match the plugin's existing setup idiom (a re-runnable,disable-model-invocation: trueskill) rather than introduce a second setup surface.CONTRACT.mdand the GitHub adapter reference no longer cite a specific consuming repo's convention docs or bot-auth wrapper; the three now-obsolete "no setup wizard / skills don't bind the provider" statements are corrected; skill citations point at${CLAUDE_PLUGIN_ROOT}/tools/work-item-tracker/....plugin.json0.7.0 → 0.8.0; CHANGELOG + regenerated root catalog.Verification (all real, from the new plugin location)
.test.shpass headless (no gh/network) from the new location.run-conformance.sh --binding local-markdown→ 61 cases, 0 failed, offline.WIT_CONFORMANCE_GITHUB_REPO+ a throwaway sandbox +ghauth), and the live run creates/deletes real GitHub account state. The github adapter's unit surface passes and the move is byte-identical, so the local-markdown E2E already exercises the shared core path. Run live github conformance against a throwaway sandbox repo as part of human review, before merge — not a precondition for this staying a draft.jira) → resolves; no consumer-local dir → plugin-bundledlocal-markdown→ resolves; rule-1 seam code → plugin copy when${CLAUDE_PLUGIN_ROOT}set, project-root fallback when unset..shellcheckrc) clean on all 60 moved shell files (two findings fixed at root: a documented default case; one justified SC2016 for a literal shim body).validate-plugin-contracts.mjsandvalidate-plugins.shpass (catalog regenerated).scripts/run-plugin-tests.sh: the work-items suite (all 29 moved.test.sh) runs green locally via the runner's own discovery from repo root (CWD-independent); the full cross-plugin suite runs in CI.${CLAUDE_PLUGIN_ROOT}in executed skill-bash): verified against marketplace precedent —claude-config,claude-memory,claude-ops,context7,code-tidyingall invoke bundled scripts via plainbash "${CLAUDE_PLUGIN_ROOT}/skills/.../x.sh"in executed bash, so the mechanism is proven. The resolver uses that exact plain-token form (not a${VAR:+…}conditional, which could break under token text-substitution). Review item chore: initialize marketplace scaffold #1: confirm the plugin-canonical branch resolves in your target runtime (a non-vendoring consumer relies on it).Deferred / noted (not built here)
jira/adapter w/ config switch vsjira-mcp/jira-rest) — no Jira adapter exists; recorded as a future-extension decision, not blocking. The consumer-local-first resolver already lets a consumer drop one in.gh, no secret today. Design left open, not foreclosed.Medley-side follow-up (ready-to-execute spec — NOT done in this PR)
Precondition:
work-items>= 0.8.0 installed in medley (ships the seam + adapters + resolver).Goal: medley stops vendoring the seam engine and resolves it from the installed plugin (dogfoods the distribution path), keeping its GitHub binding working.
Remove from medley (now bundled in the plugin, byte-identical):
tools/work-item-tracker/work-item-tracker.shtools/work-item-tracker/lib/(binding.sh,frontier.sh,id.sh,json.sh,lease.sh,verb-test-helpers.sh+ their.test.sh)tools/work-item-tracker/adapters/github/andtools/work-item-tracker/adapters/local-markdown/(all)tools/work-item-tracker/conformance/(all)tools/work-item-tracker/CONTRACT.mdKeep in medley (medley-specific, NOT relocated):
.work-item-tracker.jsonat repo root (provider: github,lease_ttl_hours: 24) — the plugin's resolver climbs from CWD to find it. Unchanged.tools/github-auth/gh-bot.sh— medley's bot-identity wrapper.tools/work-item-tracker/migrations/— one-time label-governance tooling (out of scope) — but see dependency fix: correct plugin-system inaccuracies and harden references #2.tests/shell/lib.sh— its own harness (the seam carried its own copy into the plugin; medley's other tests still use this).Two dangling dependencies the removal creates — owner decisions to surface, not silently pick:
gh-bot identity on tracker WRITES. The plugin-bundled github adapter resolves its bot wrapper adapter-relative →
${CLAUDE_PLUGIN_ROOT}/github-auth/gh-bot.sh, which does not exist in the plugin → writes (item create, lease/reclaim comments) fall back to baregh(session-user identity). Medley currently gets bot-attributed writes because its vendored adapter resolvestools/github-auth/gh-bot.sh(present). After de-vendoring, that attribution changes. Options:ghwrites (session-user attribution) — simplest; fine if bot attribution on tracker writes isn't required.WIT_GH_BOTresolves back totools/github-auth/gh-bot.sh— but that re-vendors the whole adapter, defeating resolve-from-plugin.${CLAUDE_PROJECT_DIR}/tools/github-auth/gh-bot.sh) — the clean long-term fix that lets medley keep bot writes with zero vendoring. Deliberately not built here: it changes the live adapter's documented identity/write path and isn't one of #1505's enumerated deliverables.Interim: (a); long-term: (c).
migrations/migrate-blocked-labels.shdepends on the co-located dispatcher. Line 43readonly SEAM="$SCRIPT_DIR/../work-item-tracker.sh"(→tools/work-item-tracker/work-item-tracker.sh), used at line 360 (link-blocks) and line 508 (create-item). Removing the dispatcher dangles this. (migrate-type-labels.shandrename-drift-labels.shdo not reference the seam engine.) Options:migrate-blocked-labels.sh+ its.test.sh.SEAMto a plugin-first resolution — but migrations run as standalone scripts where${CLAUDE_PLUGIN_ROOT}may be unset, so this needs a robust plugin-dir discovery (or keep a minimal vendored dispatcher for migrations only).No other dangling references: medley CI (
.github/) does not reference the seam by path;tools/run-shell-tests.shglobs**/*.test.shgenerically (the seam tests simply vanish from medley's run); the only remaining non-tree reference is ADR 0022.ADR reversal (medley-owned, describe-only here). ADR 0022 records "the seam lives in the app repo" (dispatcher at
tools/work-item-tracker/work-item-tracker.shresolving a repo-root binding — amendment text at ~line 57–58). Add an amendment or superseding ADR recording: (1) canonical home is thework-itemsplugin; (2) the two-rule resolution (seam plugin-canonical/project-root-fallback; adapters consumer-local-first/plugin-bundled-fallback); (3) binding + consumer-local adapters stay repo-side; (4) the gh-bot identity consequence from dependency #1.Verify the medley PR: with 0.8.0 installed and the vendored tree removed, run
/work-items:track(dashboard + alist-frontier) and confirm the plugin resolves its bundled seam + adapters against medley's binding; confirm write identity matches the decision in dependency #1.🤖 Generated with Claude Code
https://claude.ai/code/session_01D9yeNF5Mp9U5voSUpHXMUk