fix(work-items): guard work Step 5 against loop-prompts that dispatch without claiming (#581) - #689
Conversation
… without claiming (#581) ## Summary `/work-items:work` Step 5 already sequences the seam `claim` (assignee + lease) first, before branch/dispatch — the skill's own contract is correct. The gap #581 documents sits one layer above: a hand-authored loop-prompt standing-rule that restates "dispatch every picked issue to a subagent in its own out-of-tree worktree" reads as a complete execution contract and never mentions claiming, so an orchestrator following that loop-prompt literally did the worktree isolation and skipped the seam's race-safe claim entirely — observed twice on live loop-lane sessions (issues actively worked and shipped as PRs while sitting unassigned with no lease). Documentation/guidance gap, not a skill-code defect (per triage). Patch bump `0.14.3` → `0.14.4`. ## Fix - **Prominent guard note at the head of Step 5** (`plugins/work-items/skills/work/SKILL.md`) stating the claim-before-dispatch invariant this skill enforces regardless of loop-prompt wording: an external loop-prompt/standing-rule restating "dispatch ... in its own out-of-tree worktree" describes only the execute sub-step, is not a complete execution contract, and is not a substitute for the claim; worktree isolation is not the race-safe collision signal between concurrent lanes, the seam claim is; dispatching a subagent before the claim is held is a defect even when the loop-prompt never named the claim step. Self-contained and placed before the numbered sub-steps (no renumbering). - **Eval 1 gains a matching expectation** (`plugins/work-items/skills/work/evals/evals.json`) so the fixture asserts the claim-is-a-non-optional-prerequisite invariant, not just claim-before-execute ordering. - Version bump + `CHANGELOG.md` entry. **Scope (per the issue and triage note).** #480 — the future loop-prompt-authoring skill that would host this guidance for hand-authored lane prompts — does not exist yet; not blocked on or created here. A repo-wide search for other in-repo surfaces that restate "dispatch ... out-of-tree worktree" as a full contract found none to patch: the `babysit-prs` loop is a different lane (PR checkout, not dispatch-to-worktree), and the `autonomy` runner design pack already models a mandatory `leased` (race-safe claim) state before `executing`/dispatch — it reinforces the invariant rather than dropping it. No new enforcement mechanism was invented: there is no execution seam between "agent reads SKILL.md" and "agent dispatches" that could gate on the claim, so the guidance layer is the only place the invariant can live — a code-level guard was considered and correctly declined. ## Verification - `bash scripts/validate-plugins.sh` — green (all plugin manifests + catalog). - `node scripts/validate-plugin-contracts.mjs` — green (33 setup skills, 1807 plugin files checked). - `bash scripts/check-changed-skills.sh origin/main` — `CHECK-SKILL work: PASS — 0 errors, 1 warning`. The lone WARN ("no Gotchas surface") is pre-existing and not introduced here. - `markdownlint-cli2 plugins/work-items/skills/work/SKILL.md` — 0 errors. - `evals.json` and `plugin.json` parse as valid JSON; all four changed files carry no trailing whitespace or tabs. Closes #581 ## Related - #480 — loop-prompt-authoring guidance (the natural home for calling this out to hand-authored lane prompts). Does not exist yet; related-but-out-of-scope, not created here. - #513 — role-separated mini-SDLC composition; adjacent (role separation during execution) but not this gap (the claim step being skippable at pick time). 🤖 Generated with a Claude Code implementation subagent (issue #581) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 17s —— View job PR Review
SummaryThis is a documentation/guidance patch to SKILL.md — guard noteThe blockquote added at
No issues. The note is dense but intentionally so — any LLM reader picking up only the Step 5 header will hit this before the sub-steps. evals/evals.json — new expectationThe added expectation in eval 1 correctly codifies the invariant as a discrete, testable behavior: "Treats the seam claim (assignee + lease) as a non-optional prerequisite of dispatch — an external loop-prompt restating \"dispatch in an out-of-tree worktree\" does not substitute for it, and dispatching a subagent before the claim is held is a defect even when the loop-prompt never named the claim step"One observation (non-blocking): the CHANGELOG.mdEntry is clear, accurate, and correctly scoped as a documentation-only change. The "observed twice on live loop-lane sessions" incident note is useful signal for readers auditing fix motivation. plugin.json — version bump
CorrectnessThe invariant being documented already existed in the sub-step ordering; this patch promotes it from an implied consequence to an explicit, named contract. The PR description correctly explains why a code-level guard isn't possible (no execution seam between "agent reads SKILL.md" and "agent dispatches") — the guidance layer is the only available surface, and this is the right use of it. SecurityNo egress, secrets, or code execution introduced. No PII. Plugin-form safe — all references use Guideline alignment
Verdict: LGTM. The patch is minimal, correctly targeted, and the only meaningful alternative (a code-level enforcement gate) was correctly ruled out. No blockers. |
|
Post-green review pass — 1 round, LGTM, no blockers. One non-blocking observation (eval No inline review threads. Flipping to ready. |
…site invariant (#581) Addresses the review-bot's non-blocking observation: eval 1's narrative expected_output now names the seam-claim-is-a-non-optional-prerequisite invariant already added to the expectations array, keeping the fixture's two descriptions coherent. 🤖 Generated with a Claude Code implementation subagent (issue #581) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 31s —— View job PR Review
SummaryMinimal documentation/guidance patch across four files. The behavioral claim-before-dispatch sequence already existed in SKILL.md's sub-step ordering; this PR promotes it from an implied consequence to an explicit, named invariant with a companion eval expectation. No code or seam behavior changes. SKILL.md — guard noteLine 130 — the blockquote is well-positioned (directly under the
The density is intentional and correct — a model picking up only the Step 5 header hits this before any sub-step instructions. evals/evals.json — new expectationLine 16 — placed correctly in the One observation (non-blocking): the CHANGELOG.mdLines 6–20 — accurate, consistent with the repo's dateless-version-header pattern, and correctly scoped as documentation-only. The "observed twice on live loop-lane sessions" incident note gives future readers the fix motivation. One minor stylistic note (non-blocking): Keep-a-Changelog's plugin.json
CorrectnessThe invariant documented here was already enforced by sub-step ordering; the patch surfaces it as an explicit contract and makes it resilient to external loop-prompt restatements that drop the claim. The PR description correctly rules out a code-level guard (no seam exists between "agent reads SKILL.md" and "agent dispatches") — the guidance layer is the only available surface. That reasoning is sound. SecurityNo egress, no secrets, no code execution. Plugin-form safe — no Guideline alignment
Verdict: LGTM. Two non-blocking observations noted above (eval |
|
Claude finished @kyle-sexton's task in 1m 27s —— View job PR Review
This is a two-commit documentation/guidance patch. The commits are cleanly separated: the first ( SKILL.md — guard note
No issues. evals/evals.json —
|
| Rule | Status |
|---|---|
| Repo-agnostic | ✓ |
| Configurable without editing | ✓ (no new userConfig keys) |
| Versioned with explicit semver | ✓ |
| No hardcoded paths | ✓ |
| Conventional Commits on both commits | ✓ |
Verdict: LGTM. No blockers. No changes needed.
|
Follow-up commit (d7ee1b4) addressed the eval |
…t warning (#449) (#690) ## Summary The `work-items` triage/work lanes assumed a resolvable, bound project context. When `.work-item-tracker.json` was absent, canonical role labels fell to their defaults **silently** (a correctness risk for any repo that remapped `config.role_labels`) and seam coordination verbs surfaced a raw mid-flow `exit 3` instead of an actionable message. This ships the operator-ratified fix — binding presence as a third loud entry invariant, and the silent role-default fallback as a loud warning — reconciled so it does not regress this repo's own deliberately gh-native `/work-items:work` lane. ## Fix - **Binding presence is a third loud entry invariant** (`reference/tracker-seam.md` "Shared tracker context"). It sits alongside the existing `jq` and seam-script checks but is discharged **distinctly**: the first two have no recovery path and stop; a missing binding is loud and routable, **never a silent default and never a raw `exit 3`**, without halting unconditionally — - **Seam coordination verbs** (`claim`, `reclaim`, `renew-lease`, `create-item`, `list-frontier`, `capabilities`) cannot run unbound (the seam hard-errors `exit 3`, `CONTRACT.md` "Exit codes"), so before the first one the skill surfaces a message distinguishing **(1) setup was never run** → run `/work-items:setup`, from **(2) a deliberate gh-native operating mode** → proceed for provider-mechanic operations only, accepting **no race-safe claim/lease**. - **Provider-mechanic operations** (list/search/close, label/assignee/comment edits) run as raw `gh` per the bound adapter reference, never read the binding, and proceed unbound. - **Caveat recorded:** the gh-native path presumes a `gh`-backed provider — a `local-markdown` target with no binding has no `config.storage_dir` and stays a hard stop. - **Silent role-label default becomes a loud warning.** When a canonical role resolves to its default because the binding or its `config.role_labels` entry is absent, the skills now warn loudly instead of substituting silently. Applied at every action-entry resolution site that inlines it (`skills/work/SKILL.md`, `skills/track/actions/{due,recheck,audit}.md`, `skills/decompose/SKILL.md`) and in the shared invariants (`reference/tracker-seam.md`, `reference/label-taxonomy.md`). A present-but-malformed / empty / non-string value remains a hard stop, unchanged. - **`skills/work/evals/evals.json`** gains eval 4 asserting the unbound-binding behavior (loud, routable, distinguishes setup-vs-gh-native, warns on role default, never forges a binding). - Version bump `0.14.4` → `0.15.0` + `CHANGELOG.md` entry. ### The reconciliation (why not an unconditional hard stop) The ratified decision reads "stop-and-remediate," but the last comment on #449 (filed *after* ratification) reports that this very repo runs `/work-items:work` **deliberately gh-native** (plain `gh issue`/`gh pr`, `status: ready`, no seam claim/reclaim) because it has no binding and never ran `/work-items:setup` — so a literal hard stop would regress a working, intentional mode every cycle (session-start reclaim already hits `exit 3` here today). The coordination-vs-provider-mechanic split is not invented for this fix — it is the pre-existing "Operation routing" seam (coordination verbs go through the dispatcher and need the binding; provider mechanics run as raw `gh` and never read it). Scoping the halt to coordination verbs, and making everything else warn-and-proceed, satisfies the ratified loud-surfacing intent (#455) **and** keeps the gh-native read/comment/label path working. ### Parked (see PR comment) One residual judgment call is **parked with `status: needs-decision`**, not silently resolved: whether to formally document a first-class gh-native **no-lease claim** path for coordination-*dependent* lanes (`/work-items:work`) so they too run unbound, versus keeping them stopped at the coordination check. RECOMMENDED: defer with the same trigger as the full remote mode. It does not block this PR — the loud invariant and role-default warning ship regardless. ## Verification - `bash scripts/check-changed-skills.sh origin/main` — `work`, `track`, `decompose` all **PASS — 0 errors** (the lone per-skill "no Gotchas surface" WARN is pre-existing). - `bash scripts/check-skill-portability.sh origin/main` — **No unexcused coupling tokens** in 5 skill files. - `check-jsonschema` against `plugins/skill-quality/reference/evals.schema.json` — `work/evals.json` **validation done (ok)**. - `markdownlint-cli2` over all 14 changed/sibling markdown files — **0 errors**. - `jq empty` on `plugin.json` and `work/evals.json` — valid JSON. `git diff --check` — no trailing whitespace. ## Related - #513 — convention-gap protocol (surface, never invent-and-proceed); its dispatched subagents also need a resolvable project context. - #455 — denials / context degradations must surface loudly to the human. - #415 — discover conventions via setup, never pre-prescribe. - #581 / #689 — most recent `work` skill change (claim-before-dispatch guard), the sibling of this entry-invariant work. Closes #449 🤖 Generated with a Claude Code implementation subagent (issue #449) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Summary
/work-items:workStep 5 already sequences the seamclaim(assignee + lease) first, before branch/dispatch — the skill's own contract is correct. The gap #581 documents sits one layer above: a hand-authored loop-prompt standing-rule that restates "dispatch every picked issue to a subagent in its own out-of-tree worktree" reads as a complete execution contract and never mentions claiming, so an orchestrator following that loop-prompt literally did the worktree isolation and skipped the seam's race-safe claim entirely — observed twice on live loop-lane sessions (issues actively worked and shipped as PRs while sitting unassigned with no lease). Documentation/guidance gap, not a skill-code defect (per triage). Patch bump0.14.3→0.14.4.Fix
plugins/work-items/skills/work/SKILL.md) stating the claim-before-dispatch invariant this skill enforces regardless of loop-prompt wording: an external loop-prompt/standing-rule restating "dispatch ... in its own out-of-tree worktree" describes only the execute sub-step, is not a complete execution contract, and is not a substitute for the claim; worktree isolation is not the race-safe collision signal between concurrent lanes, the seam claim is; dispatching a subagent before the claim is held is a defect even when the loop-prompt never named the claim step. Self-contained and placed before the numbered sub-steps (no renumbering).plugins/work-items/skills/work/evals/evals.json) so the fixture asserts the claim-is-a-non-optional-prerequisite invariant, not just claim-before-execute ordering.CHANGELOG.mdentry.Scope (per the issue and triage note). #480 — the future loop-prompt-authoring skill that would host this guidance for hand-authored lane prompts — does not exist yet; not blocked on or created here. A repo-wide search for other in-repo surfaces that restate "dispatch ... out-of-tree worktree" as a full contract found none to patch: the
babysit-prsloop is a different lane (PR checkout, not dispatch-to-worktree), and theautonomyrunner design pack already models a mandatoryleased(race-safe claim) state beforeexecuting/dispatch — it reinforces the invariant rather than dropping it. No new enforcement mechanism was invented: there is no execution seam between "agent reads SKILL.md" and "agent dispatches" that could gate on the claim, so the guidance layer is the only place the invariant can live — a code-level guard was considered and correctly declined.Verification
bash scripts/validate-plugins.sh— green (all plugin manifests + catalog).node scripts/validate-plugin-contracts.mjs— green (33 setup skills, 1807 plugin files checked).bash scripts/check-changed-skills.sh origin/main—CHECK-SKILL work: PASS — 0 errors, 1 warning. The lone WARN ("no Gotchas surface") is pre-existing and not introduced here.markdownlint-cli2 plugins/work-items/skills/work/SKILL.md— 0 errors.evals.jsonandplugin.jsonparse as valid JSON; all four changed files carry no trailing whitespace or tabs.Closes #581
Related
🤖 Generated with a Claude Code implementation subagent (issue #581)
Co-authored-by: Claude Opus 4.8 noreply@anthropic.com