feat(work-items): loud binding-presence entry invariant + role-default warning (#449) - #690
Conversation
…t warning (#449) Add binding presence (.work-item-tracker.json) as a third loud entry invariant alongside jq / dispatcher-script, and convert the silent role-label-default fallback into a loud warning. Reconciles the ratified "stop-and-remediate" decision with this repo's own deliberately gh-native /work-items:work lane: a missing binding is discharged distinctly from jq/seam (which have no recovery path). Seam coordination verbs (claim, reclaim, renew-lease, create-item, list-frontier, capabilities) cannot run unbound, so before the first one the skill surfaces an actionable message distinguishing "setup never run" (run /work-items:setup) from a "deliberate gh-native operating mode" (proceed provider-mechanic-only, no race-safe claim/lease) instead of a raw mid-flow exit 3. Provider-mechanic operations (list/search/close, label/comment edits) run as raw gh and proceed unbound; their only degradation is role resolution, now a loud warning. local-markdown with no binding stays a hard stop. Full remote/no-checkout mode stays deferred. A first-class gh-native no-lease claim path for coordination-dependent lanes is parked for operator decision. Closes #449 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 4m 52s —— View job PR Review — feat(work-items): loud binding-presence entry invariant + role-default warning (#449)
SummaryThis PR correctly identifies and closes a real correctness gap: a missing Finding 1 — Medium |
|
Parked decision — gh-native no-lease claim path for coordination-dependent lanesContext. This PR ships the ratified fix (binding presence as a third loud entry invariant; silent Question. Should coordination-dependent lanes ( RECOMMENDED — (1) Defer with a recorded trigger (mirrors how full remote mode was deferred).
Labeling #449 |
…e/track role-warning parity (#449) Review findings (PR #690): - F1 (medium): work Step 0 reclaim is the lane's first coordination verb but ran structurally before the binding was read, so the invariant surfaced as a raw exit 3. Add an explicit binding preflight before Step 0 that discharges the setup-vs-gh-native routing (and stops rather than dispatching claimless, since the gh-native no-lease claim path for this coordination-dependent lane is parked). - F2 (low): triage applied canonical role labels without an inline resolution/ loud-warning reminder; add one mirroring decompose (reference layer already covered it — this restores site-level consistency). - F3 (low): track/SKILL.md parent role-invariant summary now carries the loud-warning clause its children (due/recheck/audit) already gained. CHANGELOG updated to reflect the added sites. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 4m 3s —— View job PR Review — feat(work-items): loud binding-presence entry invariant + role-default warning (#449)
Previous findings — status after second commit (
|
| Invariant | Recovery path | Discharge |
|---|---|---|
jq missing |
None | Hard stop + install remediation |
| Seam script missing | None | Hard stop + reinstall remediation |
| Binding missing | Exists (setup or gh-native) | Loud + routable; blocks coordination only, not provider-mechanic |
The coordination-vs-provider-mechanic split is principled (pre-existing routing table in tracker-seam.md, not invented here), and the work-lane override (coordination-dependent → stop both paths, not just coordination) is the correct application of that split for a lane whose entire purpose is claim-and-execute.
Coverage — complete. The CHANGELOG claims the loud-warning was "applied at every action-entry resolution site that inlines it." Verified across all 9 sites:
reference/tracker-seam.md"Role-label resolution" — ✅reference/label-taxonomy.md"Canonical roles" — ✅skills/work/SKILL.md"Role-label preflight" — ✅skills/triage/SKILL.md"Apply outcome" close — ✅skills/track/SKILL.mdsummary — ✅skills/track/actions/due.mdStep 1 — ✅skills/track/actions/recheck.mdStep 1 — ✅skills/track/actions/audit.md"Before any tracker read" — ✅skills/decompose/SKILL.mdclassification table note — ✅
Eval 4 — well-formed. Five expectations, each testing a distinct behavioral dimension:
- Detection before the first coordination verb (not mid-flow)
- Actionable message distinguishing the two arrival paths
- Naming the gh-native tradeoff (coordination unavailable; provider-mechanic proceeds)
- Loud role-default warning when binding absent
- No forged binding
Expectation 3 says the model should name the gh-native tradeoff, not that it should proceed for provider-mechanic operations within work — consistent with the skill file's "stop for the remediation" instruction for this coordination-dependent lane. The wording is precise.
CHANGELOG and version bump — appropriate. The 0.14.4 → 0.15.0 bump is warranted (behavioral change surfaced to operators). The CHANGELOG correctly flags the deferred gh-native no-lease claim path in a separate "Deferred" subsection rather than burying it.
One minor observation (informational, not blocking)
In triage/SKILL.md, the canonical-role resolution note lands at the end of Step 5 ("Apply outcome"), after the outcome table has already inlined (default agent-ready) and (default needs-human) four times in lines 124–127. A model reading linearly could apply those defaults before reaching the loud-warning instruction.
The mitigation is already in place: triage/SKILL.md instructs reading tracker-seam.md at invocation start, and tracker-seam.md's "Role-label resolution is an action-entry invariant" section fires before any step. The reference-layer coverage is real. This is an observation about asymmetry relative to how work structures the same guidance (inline, before the step that needs it), not a gap in the protection.
If triage gains evals in a future pass, eval coverage would validate this path independently of the reference-layer reliance.
Summary
The second commit (31a4a7b) cleanly closes the three findings from the first pass. The invariant design is correct, the coordination-vs-provider-mechanic split is principled, the coverage is verified complete across all 9 inlining sites, and eval 4 is well-specified. The deferred gh-native no-lease claim decision is properly parked, not silently resolved. Ready to merge.
|
Thanks — all three findings addressed in
CHANGELOG updated to list the added sites. Local gates re-run green: changed-skills (4 skills, 0 |
|
Post-green review pass — round 2 after fix-pass commit 31a4a7b.
Verdict: "Ready to merge." Coverage independently verified across all 9 inlining sites. No inline threads. Flipping to ready. |
|
Claude finished @kyle-sexton's task in 1m 24s —— View job PR Review — feat(work-items): loud binding-presence entry invariant + role-default warning (#449)
Final verificationThis PR has been through two prior review rounds. All three findings from round 1 were addressed in
One previously noted observation (still present, still non-blocking)In SummaryAll three round-1 findings are confirmed resolved on HEAD. The invariant design is correct, the coordination-vs-mechanic split is principled (pre-existing, not invented), coverage is verified across all 9 inlining sites, the deferred gh-native no-lease claim decision is properly parked with a recorded trigger, and eval 4 is well-specified. Ready to merge. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31a4a7ba92
ℹ️ 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".
…in the binding preflight (#449) The seam coordination-verb list in tracker-seam.md's binding-preflight section omitted get-item, link-blocks, and add-sub-item — all three already documented elsewhere in the same file as coordination verbs that go through the seam ("single-item fetch", "dependency links", "sub-items"), and all three require a binding at the dispatcher unconditionally (work-item-tracker.sh:98-102), before its verb-specific dispatch. An unbound /work-items:track start calls get-item before reclaim (start.md:20-21), so it still hit the raw exit 3 this invariant was meant to prevent. CHANGELOG's restatement of the same list is updated to match. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 2m 46s —— View job PR Review — feat(work-items): loud binding-presence entry invariant + role-default warning (#449)
Round 3 review — latest commit
|
| Dispatcher branch | Verbs |
|---|---|
case direct-pass |
create-item, get-item, claim, renew-lease, reclaim, link-blocks, add-sub-item, capabilities |
list-frontier → dispatches as list-items |
list-frontier |
All 9 verbs are now listed, and the binding check at lines 98–102 applies to all of them before the case statement runs. The list is exhaustive. ✅
CHANGELOG consistency — correct
The CHANGELOG.md restatement of the same verb list was updated in lockstep with tracker-seam.md. Both now agree on all 9 verbs. ✅
One informational observation (non-blocking, consistent with round-2 accepted note)
track/actions/start.md (not changed in this PR) calls get-item as its first coordination verb with no inline binding-preflight reminder — the protection comes from tracker-seam.md's invariant, read transitively via track/SKILL.md's instruction to read it at invocation start. This is the same reference-layer pattern noted as non-blocking for triage/SKILL.md in round 2. The locality is weaker than the explicit ## Binding preflight (before Step 0) section in work/SKILL.md, but it's covered. If track/actions/start.md gains a review pass in a future cycle, an inline reminder matching work/SKILL.md's pattern would strengthen it.
Full changeset summary (all 3 commits)
All three prior findings are confirmed resolved on HEAD. The Codex P2 finding (get-item/link-blocks/add-sub-item omission) is correctly closed. The invariant design is correct, the coordination-vs-provider-mechanic split is principled and pre-existing, coverage is verified complete across all 9 inlining sites and all 9 coordination verbs, the deferred gh-native no-lease claim decision is properly parked with a named trigger, and eval 4 is well-specified. Ready to merge.
…schedule (#700) ## Summary - Binds the work-item-tracker seam for this repo: `.work-item-tracker.json` with `provider: github`, `lease_ttl_hours: 24`. - Seeds `.github/recurring-schedule.json` as an empty `{"items":[]}` skeleton so `due`/`recheck`/`work` stop reporting "no recurring schedule configured" instead of hard-erroring. ## Fix `.work-item-tracker.json` was never committed on this repo (the flagship dogfood repo for the work-items plugin family), so every seam coordination verb (`list-frontier`, `claim`, `reclaim`) exit-3'd here. This forced the `/work-items:work` loop lanes to bypass the seam entirely via direct `gh` label queries. Per issue #560's decision brief (RECOMMENDED option, confirmed via a live human interview this session): - **Provider: `github`** — the only bundled adapter with `reclaim`/race-safe `claim`; `local-markdown` is never a coordination surface (`reclaim: false` in its manifest). - **`lease_ttl_hours: 24`** — reclaim's activity check (non-lease comments, open cross-referenced PRs) means TTL never steals active work; it only bounds a crashed session's stale claim. - **Empty recurring-schedule skeleton** — stops degradation without starting any cadence clock; real recurring items deferred to a separate seeding pass. - **No `config.role_labels` remap** — verified against `CONTRACT.md`'s `list-frontier` spec directly: `--autonomous` only excludes `needs-human`-labeled items, it never gates inclusion on the `autonomous-eligible` label. So this repo's `status: ready` (vs. the skill's `agent-ready` default, which is actually a medley-only `GovernedRepositories.cs` ExtraLabel, not part of the org `_core` taxonomy) is a cosmetic naming difference, not a functional gap. ## Verification - `jq .` on both new files: valid JSON. - `git check-ignore -v` on both paths: not ignored (exit 1). - Re-ran the setup `check` probes against the written files: binding parses, carries `schema_version`/`provider`/`config.lease_ttl_hours`; schedule parses with the `{"items":[]}` root shape. - No lint/build changes — pure config addition, no code touched. ## Related - Closes #560 - Cross-referenced in #560: #416 (wayfind seam-bypass), #478/#479 (absorb loop rules into skills), #449 (seam no-local-checkout mode, since resolved by #690) Closes #560 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Summary
The
work-itemstriage/work lanes assumed a resolvable, bound project context. When.work-item-tracker.jsonwas absent, canonical role labels fell to their defaults silently (acorrectness risk for any repo that remapped
config.role_labels) and seam coordination verbssurfaced a raw mid-flow
exit 3instead of an actionable message. This ships the operator-ratifiedfix — 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:worklane.Fix
reference/tracker-seam.md"Shared trackercontext"). It sits alongside the existing
jqand seam-script checks but is dischargeddistinctly: 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 —claim,reclaim,renew-lease,create-item,list-frontier,capabilities) cannot run unbound (the seam hard-errorsexit 3,CONTRACT.md"Exit codes"), sobefore 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 forprovider-mechanic operations only, accepting no race-safe claim/lease.
ghper the bound adapter reference, never read the binding, and proceed unbound.gh-backed provider — alocal-markdowntarget with no binding has no
config.storage_dirand stays a hard stop.because the binding or its
config.role_labelsentry is absent, the skills now warn loudly insteadof 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). Apresent-but-malformed / empty / non-string value remains a hard stop, unchanged.
skills/work/evals/evals.jsongains eval 4 asserting the unbound-binding behavior (loud,routable, distinguishes setup-vs-gh-native, warns on role default, never forges a binding).
0.14.4→0.15.0+CHANGELOG.mdentry.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:workdeliberately gh-native (plaingh 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 3here today). The coordination-vs-provider-mechanic splitis 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
ghand 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 themstopped 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,decomposeallPASS — 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 skillfiles.
check-jsonschemaagainstplugins/skill-quality/reference/evals.schema.json—work/evals.jsonvalidation done (ok).
markdownlint-cli2over all 14 changed/sibling markdown files — 0 errors.jq emptyonplugin.jsonandwork/evals.json— valid JSON.git diff --check— no trailingwhitespace.
Related
need a resolvable project context.
workskill change (claim-before-dispatch guard), the sibling of thisentry-invariant work.
Closes #449
🤖 Generated with a Claude Code implementation subagent (issue #449)
Co-authored-by: Claude Opus 4.8 noreply@anthropic.com