fix(work-items): make the lease protocol runnable without GraphQL, and specify the overengineering product-code lane

[kyle-sexton]

Closes #2897

Summary

Working #2897 through /work-items:work was blocked at the very first step: the seam's claim verb could not run in a Claude Code web/remote session, so no race-safe claim could be acquired and the lane's claim-before-dispatch invariant could not be satisfied. This PR fixes the seam, then delivers #2897 with a real claim held.

Two independent changes, one per commit:

1. work-items 0.39.15 — the lease protocol no longer depends on GraphQL, plus two correctness defects found while verifying that port end to end.
2. overengineering 0.2.0 — the deferred product-code lane specification and the ADR deciding its shipping shape. Documentation only; no skill or behavior changes.

Fix

The lease protocol (work-items)

claim and reclaim resolved assignees through gh issue edit --add-assignee and gh issue view --json assignees. Both route through GitHub's GraphQL API, which sandboxed sessions serve only as a pinned set of PR-review operations, refusing the rest with HTTP 403. Both verbs now use the REST …/issues/<n>/assignees endpoints through shared wit_read_assignees / wit_add_assignee / wit_remove_assignee / wit_try_remove_assignee helpers.

Identity routing is unchanged: the helpers take the same read (bare gh, session identity) / write (bot wrapper) writer argument the adapter already used, so the carve-out that assigns the session user rather than the bot still holds. @me is resolved to the login explicitly because REST takes a literal login.

Verifying the port surfaced two defects that were not part of the original scope:

• claim did not verify its own assignment landed. REST POST …/assignees returns 201 and silently drops a login that cannot be assigned, where gh issue edit --add-assignee failed loudly. Unguarded, the port would have introduced a new race: claim reporting a held lease while list-frontier still saw the item unassigned, putting two workers on one item. A dropped assignment now exits 4 before any lease comment is posted.
• wit_lease_json matched only a body ENDING in -->. Any comment carrying the lease plus trailing text (a bot wrapper's attribution footer, a signature, a CI note) parsed as "not a lease". This was silent and unsafe rather than merely lossy: claim's arbitration found no incumbent and granted over a live holder, and renew-lease refused to renew a lease it had just written. Without this, the GraphQL fix alone would have restored a claim that was not actually race-safe. The match is now anchored on the FIRST --> after the marker, which is also strictly more correct since an HTML comment cannot contain -->.

claim.test.sh covered only --help and usage errors, so the protocol itself passed vacuously. lease-coordination.test.sh now drives it: the happy path, the foreign-assignee conflict with its rollback, and the dropped-assignment guard. Adding those cases exposed a latent trap in that file, fixed here: a set +e / set -e 2>/dev/null || true pair that ENABLES errexit rather than restoring the file's declared set -uo pipefail mode, so every later case expecting a non-zero exit aborted the suite instead of asserting on it.

The product-code lane (overengineering)

#2897 asked for three things, all deferred until V1 shipped and the shared method stabilized. Both conditions now hold.

context/product-code-lane.md supplies the four bindings scrutiny-method.md asks a lane for. The item is the abstraction, never the file, so an interface plus its implementations plus its call sites is one finding with a members list. Eight layers ordered by how cheaply their evidence settles: single-implementation, extension-points, configuration, generality, layering, speculative-api, dead-branches, premature-async. Protected classes extend §7 with the hazards specific to changing code that runs: published API surface, serialization and wire formats, concurrency primitives, error-containment boundaries, and testability seams.

The lane's signature evidence probe is whether the second implementation ever arrived. Speculative generality is a prediction that variation would come, and tier-2 history can falsify it directly, which is what makes the lane tractable rather than a matter of a reviewer's taste.

The boundary against /simplify, code-tidying, and architecture:improve is documented as three operational handoffs. The distinguishing axis is retrospective and evidence-gated versus prospective and judgment-gated; notably, this lane never runs on a diff, because a construct introduced in the diff has no history to weigh it with.

ADR 0017 decides the lane ships as its own skill rather than an argument of overengineering:audit: skill descriptions are the routing surface and are budgeted least-invoked-first, the two lanes' trigger vocabularies are disjoint, the protected-class sets do not map, and retirement means something different in each. The shared walk orchestration is handled by extraction when the skill lands, the same move the plugin already made for the method itself.

Verification

The seam fix is verified against the live GitHub API, in the session where it previously could not run at all:

• claim on overengineering: product-code lane — apply the scrutiny method to code-level overengineering in application code #2897 succeeded and returned lease_comment_id: 5383892028.
• renew-lease on that handle succeeded.
• A second claim while the lease was live correctly backed off with exit 7, naming the incumbent holder. Before the wit_lease_json fix this returned 0, granting over the live lease, which is the proof that race-safety is genuinely restored rather than only the transport being changed.

Gates run locally:

• Seam suite: 56/56 pass. adapters/gitea/list-items.test.sh is excluded because it hangs, and it hangs identically on a clean origin/main worktree, so it is pre-existing and unrelated to this change. Flagging it rather than silently skipping.
• shellcheck and shfmt clean on every changed shell file.
• markdownlint-cli2, typos, editorconfig-checker clean on every changed markdown file.
• Both plugin manifests validate against the plugin-manifest schema.
• The repo's own ai-slop detector reports 0 findings across the new prose.

Related

• Refs feat(overengineering): add the evidence-earned-keep audit + realign plugin for the enforcement surface #2961 — the V1 enforcement-surface lane whose shipping unblocked overengineering: product-code lane — apply the scrutiny method to code-level overengineering in application code #2897.
• docs/adr/0017-ship-the-product-code-lane-as-its-own-skill.md — added here; records the lane's shipping-shape decision.

Known limitation, deliberately not fixed here. Only the lease protocol is ported. get-item, list-items, list-sub-items, list-frontier, add-sub-item, and link-blocks still resolve GraphQL-only fields (issueType, blockedBy, parent, subIssues) or use gh issue list, so they remain unavailable under the same restriction. Because list-frontier is what /work-items:work uses to select, an unattended lane still cannot pick an item in a web session; only the named-item path works. The REST endpoints for most of this are served (issues list, native type, sub_issues, dependencies/blocked_by), but REST exposes no parent field, so parent_id needs its own resolution. That is a separate piece of work across five scripts and should be scoped on its own rather than widening this PR. Documented in the GitHub adapter README.

---

Generated by Claude Code

[claude]

Claude finished @kyle-sexton's task in 3m 17s —— View job

---

Security review in progress

• Gather context (PR diff, changed files)
• Apply skip gate check — has security-relevant surface (GitHub API auth/assignee operations), continuing full review
• Review shell scripts for command injection / unsafe gh usage
• Review GitHub Actions / workflow-adjacent changes — none present in this PR
• Review lease/assignment logic for auth/trust-boundary issues
• Adversarially validate candidate findings (independent pass running)
• Post findings / summary

[github-actions]

Last security-reviewed head: 1f17f7f95e7f247cea9dabd7bd3c31a569b01dba. 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 @kyle-sexton's task in 3m 25s —— View job

---

Code review complete

• Gather context (PR body, changed files)
• Read diff against origin/main (via the two feature commits e1a00db / 9772549, since a shallow clone plus network restrictions in this session blocked a direct origin/main...HEAD diff — see note below)
• Review work-items REST-lease changes (claim.sh, reclaim.sh, common.sh, lib/lease.sh)
• Review test changes (lease-coordination.test.sh, lease.test.sh)
• Review overengineering doc/ADR changes
• Post findings as inline comments (none warranted — see below)
• Post summary

Summary

I read this PR end to end against the two feature commits and did not find correctness bugs to flag. No inline comments were posted because nothing localizes to a single changed line as a defect — the following is prose review only.

work-items REST lease port

• The wit_read_assignees / wit_add_assignee / wit_remove_assignee / wit_try_remove_assignee helpers (common.sh#L158) correctly preserve the read/write writer-argument identity routing that the bare-gh-for-@me carve-out in the README depends on, and wit_try_remove_assignee intentionally bypasses wit_run_gh (always bare gh, swallows failure) which matches the || true best-effort rollback semantics it replaces.
• The dropped-assignment guard in claim.sh (claim.sh#L69) runs after the EXIT trap is armed, so a silently-dropped POST still triggers the (harmless, no-op) rollback — correctly sequenced, and it exits EX_AUTH (4) before any lease comment is posted, matching the stated invariant.
• wit_lease_json's switch to first---> anchoring (lease.sh#L20) is sound: since the marker opens an HTML comment and HTML comments cannot contain -->, the first occurrence always closes it, so trailing content after the marker no longer causes a live lease to parse as absent.
• reclaim.sh is a faithful call-site swap with no behavior change (still routes failures loudly through wit_run_gh, unlike the best-effort claim rollback).
• The set +e / set -e 2>/dev/null || true → || rc=$? fix in lease-coordination.test.sh is a real and correctly-diagnosed bug (that pattern enables errexit rather than restoring the file's set -uo pipefail baseline); the new claim.sh scenarios (happy path, foreign-assignee conflict + rollback, dropped-assignment guard) exercise real gaps claim.test.sh left uncovered.
• I did not find a new race introduced by the REST port: the co-assignment corner case (two workers both landing as co-assignees and both backing off on seeing "another login") is a property of the pre-existing sole-assignee-check design, unchanged by this port.

overengineering product-code lane + ADR 0017

• Cross-references resolve: product-code-lane.md's links to ../skills/audit/context/surface-walk.md and ../../../docs/adr/0017-...md, and the ADR's references back into product-code-lane.md, all point at real paths.
• scrutiny-method.md's "Lane binding" section update is consistent with the new document — the forward reference to "a future product-code lane" is replaced with a link to the concrete spec.
• Version/changelog bumps (work-items 0.39.13→0.39.15, overengineering 0.2.0→0.2.1 after merge with the delta-lane bump on main) are internally consistent between plugin.json and CHANGELOG.md.

Note on verification limits in this session: this sandbox's git fetch/git fetch --unshallow and running the shell test suites (bash lease.test.sh, bash lease-coordination.test.sh) both required interactive approval that wasn't available here, so I reviewed via the locally-available feature commits (e1a00db, 9772549) rather than a live origin/main...HEAD diff, and via static reading rather than executing the test suites myself. The PR body's own verification section (56/56 seam suite, shellcheck/shfmt/markdownlint clean, live-API verification against #2897) covers that gap.

Verdict: no blocking issues found. This is a clean, well-tested seam fix plus a doc-only spec addition with consistent cross-references.

Branch: claude/work-items-integration-h3yabz

[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 @kyle-sexton's task in 1m 5s —— View job

---

Security review complete

• Gather context (PR diff since last reviewed head)
• Apply skip gate check
• Post final summary

Skip gate: no re-review needed. The last full security review covered head e59ff938. The current head (1f17f7f) only merges in unrelated origin/main commits (24ace40, a source-control fix) — git diff e59ff938 1f17f7f touches none of this PR's substantive files. The only overlapping file is plugins/overengineering/CHANGELOG.md/plugin.json, and that diff is purely a version-bump conflict resolution (0.2.1 → 0.2.2) from main's independent changelog entry, with no new prose or logic.

None of the security-relevant surface — claim.sh, common.sh, lease.sh, reclaim.sh, lease-coordination.test.sh, lease.test.sh, the ADR, or product-code-lane.md — changed since that review. Its verdict stands: no blocking security issues found in the REST-lease assignee-identity routing, the dropped-assignment guard, or the wit_lease_json anchoring fix.

Verdict: no new security-relevant changes since the last review at e59ff938; that review's "no blocking issues" verdict applies unchanged to head 1f17f7f.

Branch: claude/work-items-integration-h3yabz

[claude]

Claude finished @kyle-sexton's task in 3m 43s —— View job

---

/review:code-review — CI code-review lane

• Gather context (PR body, changed files, custom REPO/PR/HEAD SHA header)
• Diff each changed file's blob (origin/main:<path> vs HEAD:<path>) — shallow clone has no merge-base for origin/main...HEAD, so per-file blob diffs were used instead; confirmed the 14-file set matches the PR's stated changed files
• Review work-items REST-lease port (common.sh, claim.sh, reclaim.sh, lease.sh) for correctness
• Review test changes (lease-coordination.test.sh, lease.test.sh) for coverage and stub correctness
• Review overengineering doc/ADR changes for internal consistency (section cross-references, version/changelog bumps)
• Post findings

Findings

None. This lane found no correctness or maintainability issues a careful reviewer would block on.

Specifically checked and confirmed sound:

• wit_lease_json's new first---> anchoring (lease.sh#L21-30) uses ${body%% -->*} — bash's %% removes the longest matching suffix, which is what anchors on the first --> occurrence rather than the last. This is the correct glob direction for the stated intent (not just plausible-sounding).
• claim.sh's dropped-assignment guard (claim.sh#L61-74) re-reads assignees via a fresh wit_read_assignees call (not the POST response body) before trusting the assignment landed, and runs after the rollback trap is armed, so a silent drop still gets cleaned up.
• The read/write writer-argument routing through the new wit_read_assignees/wit_add_assignee/wit_remove_assignee helpers (common.sh#L158-196) preserves the session-identity carve-out for claim/reclaim.sh call sites — parameter order matches at every call site.
• lease-coordination.test.sh's set +e / set -e 2>/dev/null || true → || rc=$? fix is a real bug fix (the old pattern enabled errexit rather than restoring the file's set -uo pipefail baseline), and the new claim.sh scenarios exercise real gaps claim.test.sh left uncovered.
• overengineering doc changes: all §N cross-references in product-code-lane.md and the ADR resolve to actual section numbers in scrutiny-method.md (1-12); plugin.json/CHANGELOG.md version bumps are consistent for both plugins (work-items 0.39.14→0.39.15, overengineering 0.2.1→0.2.2).

No inline comments posted — nothing localizes to a specific line as a defect.

Note: this sandbox's shallow clone has no merge-base with origin/main, so I diffed each changed file's blob content directly by path (git diff origin/main:<path> HEAD:<path>) rather than a single origin/main...HEAD diff. This is exact per-file but wouldn't by itself catch a file deleted by this PR that isn't in the stated changed-files list; the provided 14-file list was used as the scope boundary.

Branch: claude/work-items-integration-h3yabz