fix(work-items): add row-level seam claim to attend-queue

[kyle-sexton]

Fixes #1290

Summary

Two concurrent attended sessions on one repository could both surface and mutate the same row because /work-items:attend-queue had no claim protocol while /work-items:work already used the seam assignee + lease (exit 7 → advance).

Documents the same seam claim protocol work uses, scoped to attend-queue row disposition: claim before mutate, release before flip to autonomous-eligible, session-start reclaim, binding routing. Bumps work-items to 0.35.16.

Test plan

• Eval feat(hook-telemetry): marketplace-wide telemetry contract + markdown-formatter producer #5 covers concurrent-session exit-7 skip behavior
• Single-session run needs no new required argument

Related

• work-items(attend-queue): no row-level claim — concurrent attended sessions collide on the same rows #1290
• Unblocked by work-items(work-loop): fixed telemetry title and sentinel make two lanes on one repo clobber durable state, including first_drain_complete #1295 / fix(lanes): key telemetry state per lane instance, not per lane type #1841

  

[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 5m 5s —— View job

---

Security review (/review:security-review)

• Gather context (PR diff via gh pr diff, changed files)
• Review security-relevant surface for logic/trust-boundary issues
• Post findings

Scope reviewed (per gh pr diff against this PR's base): the row-level seam-claim documentation added to plugins/work-items/skills/attend-queue/SKILL.md, and the HOOK_TELEMETRY_SINK telemetry wiring added to plugins/disk-hygiene's destructive_guard.py / guard_launch_monitor.py / new lib/hook_telemetry.py. (Note: this PR's computed diff also carries the disk-hygiene telemetry commit; it was in scope and reviewed since it's part of what gh pr diff returned for this PR.)

Finding (posted inline, CRITICAL)

destructive_guard.py#L1355 — the new telemetry call added inside main()'s own except BaseException fail-closed handler is itself unguarded. It calls _emit_guard_telemetry → hook_telemetry.emit → an unguarded threading.Thread(...).start(), which can raise RuntimeError under OS thread/resource exhaustion (a failure mode this same file's own docstring already treats as real, for the watchdog Timer). If that happens twice in a row (once inside _decide(), then again inside the except handler that reports the first failure), the second exception escapes main() uncaught, the process exits 1, and — per this file's own documented contract — Claude Code treats PreToolUse exit 1 as non-blocking, so the guarded destructive shell command proceeds without the guard's deny being applied. This is the exact fail-open shape issue #1423 (documented at length in this file's module docstring) was fixed to close, reintroduced via the new, unguarded telemetry side channel.

This was independently adversarially verified (producer ≠ verifier) before posting; the verifier traced the full call chain and confirmed no existing guard intercepts it.

Fix direction: make telemetry emission provably unable to raise on the calling thread — wrap threading.Thread(...).start() in hook_telemetry.py's _dispatch_sink in try/except BaseException: pass (mirroring the existing catch already inside the spawned thread's _run), consistent with telemetry being documented as "fire-and-forget."

No other security-relevant issues found in the reviewed diff. guard_launch_monitor.py's telemetry call sits inside an existing top-level except BaseException: return 0 (that hook is an advisory Stop-event detector, not a permission gate), so an exception there is already safely absorbed and does not create the same risk.

Branch: cursor/fix-1290-attend-queue-claim-2ae1

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4a19feb51c

ℹ️ 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".

[github-actions]

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

[claude]

Claude finished @cursor[bot]'s task in 3m 11s —— View job

---

Security review (/review:security-review)

• Gather context (PR diff via gh pr diff, changed files, delta since last reviewed SHA)
• Review security-relevant surface for logic/trust-boundary issues
• Post findings

Scope note. The branch was rebased since the last review (previously-reviewed head 4a19feb). git diff origin/main...HEAD now shows only the 4 work-items files listed in the PR description — the disk-hygiene HOOK_TELEMETRY_SINK code that carried the previous CRITICAL finding is no longer part of this PR's diff, so that finding no longer applies to this head (6c38de9).

Finding (posted inline, IMPORTANT)

SKILL.md#L105-L110 (reinforced at L318-321) — the documented protocol releases the row claim (supersede lease + remove @me assignee) before the "flip to agent-ready" label mutation. I traced this against the actual arbitration logic in claim.sh: a claim succeeds whenever there's no live lease and no other assignee — exactly the state produced by releasing. Because the label hasn't flipped yet at that point, the row still reads as [escalated]/[ratify] in another attended session's view, so that session can win a claim on it while the first session goes on to perform an now-unclaimed label-flip mutation. That's the same "two concurrent attended sessions... both mutate the same row" collision this PR exists to close (per the PR description), reintroduced at the final step. It also conflicts with this file's own general rule two lines above ("Mutation... requires holding the seam claim first").

This independently confirms (via direct trace of claim.sh, not just re-reading the doc) the same ordering issue Codex's review flagged on this PR. Fix direction: perform the single-edit label flip while still holding the claim, then release immediately after — the "blocks the frontier" concern in the Gotchas bullet is a one-cycle latency cost, not a correctness one, and is a better trade than reopening the claim race.

No other security-relevant issues found in the reviewed diff (plugin.json version bump, CHANGELOG.md, evals.json are non-functional; the TRACKER path-resolution snippet added to this file matches the existing pattern already used in work/SKILL.md verbatim, so it introduces no new injection surface).

Branch: cursor/fix-1290-attend-queue-claim-2ae1

[claude]

Claude finished @cursor[bot]'s task in 5m 1s —— View job

---

Code review (/review:code-review)

• Gather context (gh pr diff, changed files, CLAUDE.md/REVIEW.md)
• Confirm scope: repo carries .github/workflows/claude-security-review.yml, so security findings are out of this lane's scope (none found in the reviewed diff anyway)
• Review the 4 changed files against correctness/maintainability, cross-checked against the tracker seam contract (CONTRACT.md) and sibling /work-items:work skill
• Post findings as inline PR comments

Scope reviewed: plugins/work-items/skills/attend-queue/SKILL.md (the new row-claim protocol), evals/evals.json, plugin.json, CHANGELOG.md — the 4 files this PR actually changes per gh pr diff.

Findings (posted inline)

1. 🔴 Important — SKILL.md:105-110: the documented "release" step (supersede the live lease + unassign) calls a seam operation that doesn't exist — work-item-tracker.sh's fixed verb set has no way to voluntarily supersede a still-live lease early (reclaim explicitly refuses that; superseded_at is written only from claim's back-off path and reclaim's expired-lease path). As written, an agent would have to hand-craft a raw gh api edit of the lease comment, which is exactly what reference/tracker-seam.md forbids ("Do not hand-roll a label-based hold protocol" / "coordination goes through the seam"). Separately, the documented ordering (release, then flip the label) reopens the exact collision this PR closes: [escalated] membership is human-gated label + escalation marker, so between release and flip the row is unclaimed and still [escalated], open to a second session claiming and re-working it. Flip-while-claimed, release-after would close that window.

2. 🟡 Nit — SKILL.md:94-98: the binding-missing guidance is worded as "before the first claim", but the session-start reclaim sweep in the very next paragraph runs first and is itself a binding-dependent coordination verb — worth wording as "before the first coordination verb (reclaim, then claim)" so an implementer doesn't defer the check past the reclaim sweep, consistent with how /work-items:work's preflight treats reclaim as its first coordination verb.

No other correctness/maintainability issues found in the reviewed diff; the evals.json addition, plugin.json version bump, and CHANGELOG.md entry are consistent with each other and with repo conventions.

Branch: cursor/fix-1290-attend-queue-claim-2ae1

[claude]

Claude finished @cursor[bot]'s task in 1m 15s —— View job

---

Security review (/review:security-review)

• Gather context (git diff origin/main...HEAD, changed files, delta vs. previously reviewed heads)
• Review security-relevant surface for logic/trust-boundary issues
• Post findings

Scope. git diff origin/main...HEAD at f355a08 shows exactly the 4 work-items files in the PR description (plugin.json version bump, CHANGELOG.md, SKILL.md, evals.json). The disk-hygiene HOOK_TELEMETRY_SINK code that carried a CRITICAL finding on an earlier head is not part of this diff.

Finding (IMPORTANT — unresolved from prior review pass)

SKILL.md#L105-L110 (reinforced at L318-321) — this text is byte-for-byte unchanged from the previously reviewed head (6c38de9); the ordering issue flagged there (and independently by the Codex review on this PR) still applies at f355a08.

The documented protocol releases the row claim (supersede the lease + remove @me as assignee) before the "flip to agent-ready" label mutation (L141-145). I re-traced this against claim.sh at this head: a claim succeeds whenever there's no live lease and no other assignee — exactly the state releasing produces. Since the label hasn't flipped yet at that moment, the row still reads as [escalated]/[ratify] in another attended session's view, so that session can win a claim on it while the first session goes on to perform the now-unclaimed label-flip mutation — the same "two concurrent attended sessions... both mutate the same row" collision this PR exists to close (per its own description), reintroduced at the final step. It also conflicts with the general rule two lines above ("Mutation... requires holding the seam claim first").

Fix direction (unchanged from prior review, also flagged by Codex): perform the single-edit label flip while still holding the claim, then release (supersede lease + remove @me) immediately after. The "blocks the frontier" concern in the Gotchas bullet (L318-321) is a one-cycle latency cost, not a correctness one, and is a better trade than reopening the claim race.

Fix this →

No other security-relevant issues in the reviewed diff — plugin.json (version bump), CHANGELOG.md, and evals.json (new eval #5) are non-functional documentation/metadata changes.

Branch: cursor/fix-1290-attend-queue-claim-2ae1

[github-actions]

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

[claude]

Claude finished @cursor[bot]'s task in 2m 39s —— View job

---

Code review (/review:code-review)

• Gather context (PR diff via git diff origin/main...HEAD, changed files)
• Review changed files against CLAUDE.md / REVIEW.md criteria (correctness, maintainability — security scoped to the security lane)
• Post findings

Scope reviewed: the 4 files in this PR's diff — plugins/work-items/.claude-plugin/plugin.json, CHANGELOG.md, skills/attend-queue/SKILL.md, skills/attend-queue/evals/evals.json — against origin/main at head f355a08.

Findings

No new high-signal correctness or maintainability findings from this lane.

Note on the release-before-flip ordering (SKILL.md L105-110, reinforced L318-321): I independently traced this against claim.sh's arbitration and confirm the race is real — releasing the claim before the agent-ready label flip lets a second attended session win claim on the still-[escalated]/[ratify]-reading row before the first session's flip lands, reopening the exact collision this PR exists to close. This is already reported in detail, with an accurate trace and fix direction, by both the Codex review (P1, SKILL.md:109) and this repo's own /review:security-review lane (IMPORTANT, 00:21:01Z) on this same head — I'm not duplicating it as a new inline comment per the "one scope, never reported twice" rule, but flagging that it remains unresolved on f355a08 and should block merge until the flip happens while the claim is still held (both prior reviews agree on that fix direction).

Everything else in the diff checks out: the new claim/reclaim/renew-lease command names and exit codes (0/6/7) documented in the "Row claim" section match the actual scripts in tools/work-item-tracker/adapters/github/; the added eval (evals.json id 5) is valid, sequential, and matches the PR's stated exit-7-skip test-plan claim; the plugin.json version bump and CHANGELOG.md entry are consistent with each other and the existing changelog format.
· Branch: cursor/fix-1290-attend-queue-claim-2ae1

[claude]

Claude finished @cursor[bot]'s task in 1m 36s —— View job

---

Security review (/review:security-review)

• Gather context (diff since last-reviewed head f355a08 → current HEAD 5a65756)
• Review security-relevant surface for logic/trust-boundary issues
• Post findings

Scope. Commit 5a65756 ("flip while claimed and drop nonexistent release verb") is the only change since the last-reviewed head. It touches plugins/work-items/skills/attend-queue/SKILL.md, CHANGELOG.md, and evals/evals.json only — no disk-hygiene / telemetry code is in this PR's diff. This pass verifies whether it resolves the two prior findings (mine, Codex's, and the code-review lane's) and checks for anything new.

Prior finding 1 — release-before-flip race: resolved

SKILL.md#L106-L121 now documents performing the single-edit role-label flip while the claim is still held, clearing @me only after. I re-traced this against claim.sh's arbitration to confirm the window is actually closed, not just narrowed: after @me is cleared but before the lease's TTL expires, a second session's claim still loses — arbitration is by earliest live lease, not by assignee presence (claim.sh:81-102). So even with no assignee, session B's new lease comment is superseded by session A's still-live one and B backs off on exit 7. The label flip itself also now happens before any window opens, closing the gap Codex/prior reviews flagged. The residual effect — the row stays unselectable by the worker frontier for up to the lease TTL after disposition — is a liveness/latency cost, not a correctness or security hazard, and the doc says so explicitly (SKILL.md:113-116).

Prior finding 2 — nonexistent "release" seam verb: resolved

The doc no longer instructs superseding a live lease or hand-crafting lease-comment JSON. It now documents clearing the assignee via the adapter's --remove-assignee "@me" (SKILL.md:108-109) — I confirmed this is a real, already-used pattern (claim.sh:55,64,117, reclaim.sh:85) and is explicitly documented as a normal bare-gh operation for label/assignee edits in the adapter README.md:151-160, not a hand-rolled coordination primitive. This does not reintroduce the "inline provider commands bypass the seam" concern — the seam's own claim/reclaim scripts use this same raw gh call for assignee cleanup; coordination arbitration (the lease) is untouched.

Prior nit — binding check deferred past reclaim: resolved

SKILL.md:94-97 now gates the binding check on "the first coordination verb (session-start reclaim, then row claim)", matching /work-items:work's preflight ordering.

New issues in this diff

None found. evals.json (eval #2 and #5 updated to assert claim-then-flip-then-clear ordering and the binding/reclaim ordering) and CHANGELOG.md are consistent with the SKILL.md change and introduce no new surface.

Branch: cursor/fix-1290-attend-queue-claim-2ae1

[claude]

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

---

Code review (/review:code-review)

• Gather context (PR diff, changed files, custom instructions)
• Invoke /review:code-review skill and follow its instructions
• Apply skip gate

Skipped — PR is merged. The skill's skip gate stops early when "PR is closed or not open," posting nothing else. This PR (#2538) is in MERGED state, so no new review was performed and no inline comments were posted.

For reference, this PR already received two /review:code-review passes while open (00:33:25Z and 00:48:53Z UTC), the second confirming no new findings beyond the previously-reported release-before-flip ordering issue in SKILL.md (documented in detail by both the security-review lane and Codex).

Branch: cursor/fix-1290-attend-queue-claim-2ae1