Skip to content

feat(work-items): seam contract hygiene — version handshake, ADR 0014, label-default SSOT - #2970

Merged
kyle-sexton merged 4 commits into
mainfrom
claude/pocock-shipping-breakdown-ukohlx
Aug 17, 2026
Merged

feat(work-items): seam contract hygiene — version handshake, ADR 0014, label-default SSOT#2970
kyle-sexton merged 4 commits into
mainfrom
claude/pocock-shipping-breakdown-ukohlx

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Closes #2942

Summary

Closes the four contract-hygiene findings on the work-item tracker seam (spec container #2933, seam-scrutiny findings F3.5–F3.7 plus the 2026-08-17 session finding): the dangling ADR citation, the missing contract-version handshake, the triple-defined role-label defaults, and the undocumented gh-absent degradation path.

Fix

  • F3.6 — contract-version handshake. The dispatcher previously read only .verbs from the adapter manifest, so a consumer-local, shadowing, or generated adapter at a different contract revision skewed silently. wit_check_contract_version (lib/json.sh) now runs before every dispatch: major skew refuses with exit 3 naming both versions and the direction-appropriate fix ("update the plugin" vs "update or regenerate the adapter"); a newer-minor manifest proceeds with a stderr notice (tolerant reader); older-minor proceeds silently; an unversioned manifest refuses. Documented both directions in CONTRACT.md "Contract-version handshake". Prerequisite for the adapter-onboarding generator (Adapter-onboarding skill — generator for the provider tail #2950).
  • F3.5 — ADR citation resolves in-tree. CONTRACT.md and the GitHub conformance binding cited "ADR 0022", a number docs/adr/ never reached. The direction-locking rationale is now recorded as ADR 0014 (engine plugin-canonical / adapters consumer-first, plus the no-standing-sandbox conformance note); both citations repointed.
  • F3.7 — label defaults single-sourced. The shipped role-label defaults (needs-human, agent-ready, recurring) were defined in three places (lib/binding.sh literals, lib/frontier.sh parameter default, dispatcher inline fallback). They now live once in lib/labels.sh; all three sites read the constants. Fixed-string postures for the [Maintenance] title prefix and the .github/recurring-schedule.json path are recorded as deliberate deferrals in label-taxonomy.md "Recorded postures".
  • Session finding — gh-absent degradation documented. CONTRACT.md "Degradation without gh" records that MCP-only sessions cannot run the github adapter at all, explicitly defers a REST fallback (rationale recorded), rejects MCP-as-adapter, and documents the supported backfill ritual (body-text Blocked by: edges + provenance comment, replayed via link-blocks/add-sub-item from the next gh ≥ 2.94 session; leases excluded).
  • work-items bumped to 0.35.25 with a CHANGELOG entry.

Verification

  • All 38 tracker test files pass (adapters github/jira/local-markdown, lib, dispatcher, conformance harness) — including 9 new dispatcher handshake assertions (unversioned / newer-major / older-major refuse with the right stderr; newer-minor proceeds with notice and intact passthrough).
  • Conformance suite green offline: local-markdown 76 cases / 0 failed, jira 29 cases / 0 failed — including the new synthetic-skew handshake cases — and both re-run clean under a gh/curl-blocking PATH shim.
  • shellcheck clean on every touched shell file; markdownlint-cli2 0 issues on the four touched markdown files.

Related

Refs #2933 (spec container), #2950 (adapter-onboarding generator — unblocked by the handshake), ADR 0014.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CnzwTKoTa6xNY7iyEzMYpm


Generated by Claude Code

…, label-default SSOT (#2942)

- Add a directional tolerant-reader contract-version handshake at the adapter
  seam (major skew refuses with exit 3 in both directions; newer-minor proceeds
  with a stderr notice; unversioned manifests cannot handshake), wired into the
  dispatcher before every dispatch, with unit tests and conformance skew
  coverage. Prerequisite for the adapter-onboarding generator (#2950). (F3.6)
- Write ADR 0014 (seam engine plugin-canonical / adapters consumer-first, plus
  the no-standing-sandbox conformance note) and repoint the dangling "ADR 0022"
  citations in CONTRACT.md and the GitHub conformance binding. (F3.5)
- Single-source the shipped role-label defaults in lib/labels.sh; binding
  resolution, the frontier filter default, and the dispatcher now read the
  constants instead of restating the literals. (F3.7)
- Record fixed-string postures ([Maintenance] title prefix, recurring-schedule
  path) in label-taxonomy.md; document gh-absent degradation honestly in
  CONTRACT.md with the supported backfill ritual (REST fallback explicitly
  deferred, MCP-as-adapter rejected). (#2942 session finding)
- work-items 0.35.25.

Closes #2942

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CnzwTKoTa6xNY7iyEzMYpm
@cursor

cursor Bot commented Aug 17, 2026

Copy link
Copy Markdown

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 Bot commented Aug 17, 2026

Copy link
Copy Markdown

Claude Code is working…

I'll analyze this and get back to you.

View job run

@claude

claude Bot commented Aug 17, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 5m 39s —— View job


Claude's review of PR #2970

  • Gather context (PR diff, CLAUDE.md, REVIEW.md, changed files)
  • Review contract-version handshake (lib/json.sh, dispatcher)
  • Review label-default SSOT (lib/labels.sh, lib/binding.sh, lib/frontier.sh)
  • Review ADR 0014 and CONTRACT.md docs updates
  • Review conformance/test changes
  • Post findings

This repo runs a dedicated security lane (claude-security-review.yml), so this review is scoped to correctness/maintainability per REVIEW.md.

One finding posted inline on plugins/work-items/tools/work-item-tracker/lib/json.sh:55:

🔴 Important — leading-zero version components crash wit_check_contract_version instead of refusing cleanly. The new handshake validates schema_version with ^[0-9]+\.[0-9]+$ (line 36), which accepts leading zeros like "1.08". The split components then go straight into bash ((...)) arithmetic (lines 45, 46, 55) with no 10# base-forcing. Bash treats a leading 0 as octal, and 08/09 are invalid octal digits, so ((...)) raises a fatal "value too great for base" error that aborts the script — even without set -e — instead of hitting any of the documented refuse/proceed paths in CONTRACT.md's "Contract-version handshake" table. Non-8/9 leading zeros (e.g. "1.010") silently misparse as octal instead of crashing, producing a wrong skew verdict. None of the new tests/conformance cases ("2.0", "0.9", "1.99") exercise a zero-padded value, so the gap isn't caught. This is squarely in scope since the handshake exists precisely for consumer-owned/generated adapters (the #2950 generator is the PR's own cited consumer). Fix: ((10#$a_major != 10#$core_major)) / ((10#$a_minor > 10#$core_minor)) style base-10 forcing on all four comparisons.
Permalink:

if ((a_major != core_major)); then
if ((a_major > core_major)); then
printf "work-item-tracker: adapter '%s' speaks contract v%s but this core speaks v%s — update the work-items plugin\n" \
"$provider" "$declared" "$WIT_SCHEMA_VERSION" >&2
else
printf "work-item-tracker: adapter '%s' speaks contract v%s but this core speaks v%s — update or regenerate the adapter\n" \
"$provider" "$declared" "$WIT_SCHEMA_VERSION" >&2
fi
return 1
fi
if ((a_minor > core_minor)); then
printf "work-item-tracker: adapter '%s' declares contract v%s, newer than core v%s — proceeding; unknown additive fields are ignored (update the work-items plugin to consume them)\n" \
"$provider" "$declared" "$WIT_SCHEMA_VERSION" >&2
fi

Everything else reviewed clean:

  • The label-default single-sourcing (lib/labels.shlib/binding.sh/lib/frontier.sh/dispatcher) is correctly wired — sourcing order and load guards avoid double-readonly errors, and WIT_HUMAN_GATED_LABEL is genuinely guaranteed by wit_read_binding before the dispatcher's new no-inline-default use of it.
  • ADR 0014 citations in CONTRACT.md and conformance/bindings/github.sh resolve to the new file; no more dangling ADR 0022.
  • New dispatcher/conformance test assertions match the implementation's actual stderr strings and exit codes.
  • conformance/run-conformance.sh's reindented case block (4-space → 2-space) is a pure whitespace change with no behavior difference — noted, not flagged.

Note: this environment's Bash tool blocked ad hoc script execution (only gh pr diff/view/comment/review were permitted), so the arithmetic-crash claim above is based on static analysis plus well-established bash ((...)) octal-literal semantics, not a live repro in this session — worth a quick local bash -c check before merge if there's any doubt.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 98e294d647

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

Comment thread plugins/work-items/tools/work-item-tracker/lib/json.sh
…view)

A manifest schema_version with a leading-zero component ("08.0") passes the
MAJOR.MINOR regex, but bare (( )) arithmetic reads 08 as octal and errors —
and an errored condition is falsy, so both refusal branches were skipped and
the major-incompatible adapter was waved through. Force base-10 with 10# at
parse time; regression-covered (leading-zero major now refuses with exit 3).

Refs #2942

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CnzwTKoTa6xNY7iyEzMYpm
@claude

claude Bot commented Aug 17, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 14s —— View job


Security review complete ✅

  • Gather context (PR diff, custom instructions)
  • Skip-gate check
  • Review changed files for security-relevant issues
  • Post findings

No security issues found.

This PR is contract-hygiene/documentation work on the work-item-tracker seam (version handshake, ADR citation fix, label-default consolidation, gh-absent degradation docs). Reviewed all 13 changed files against 8b78f6f (the PR's actual base, since local origin/main was stale in this sandbox):

  • lib/json.sh — new wit_check_contract_version: reads schema_version from the adapter manifest via jq, parses MAJOR/MINOR explicitly with 10# base-10 forcing before arithmetic. This already includes the fix for the leading-zero/octal issue flagged in the earlier Codex review comment on this PR (08.0 would previously hit Bash's octal interpretation in (( )) and error-fall-through past the refusal branch); commit 1eadc48 addresses it, and work-item-tracker.test.sh adds a dedicated leadingzero regression case plus unversioned/newer-major/older-major/newer-minor cases. No untrusted input reaches eval, command substitution built from adapter-controlled strings, or path traversal — $provider/$declared are only ever used in printf diagnostics or numeric context.
  • lib/binding.sh, lib/frontier.sh, lib/labels.sh — pure refactor consolidating the three previously-duplicated label-default literals into one sourced constants file. No new external input handling.
  • work-item-tracker.sh — wires the handshake into dispatch before the adapter runs; refusal maps to exit 3 and prevents dispatch, so a skewed adapter can no longer execute silently. No new attack surface.
  • conformance/run-conformance.sh, conformance/bindings/github.sh — formatting/ADR-citation changes plus new synthetic skew test cases using mktemp -d sandboxes; no injection surface (test fixtures, not consumer-facing).
  • CONTRACT.md, ADR 0014, CHANGELOG.md, label-taxonomy.md, plugin.json — documentation/version bump only.

Nothing here touches authn/authz, secrets handling, or GitHub Actions workflow permissions — out of scope for this lane's permissions:/pull_request_target/injection hunting, and there's no such surface in this diff regardless.

@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown

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

Comment thread plugins/work-items/tools/work-item-tracker/lib/json.sh
@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown

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

@claude

claude Bot commented Aug 17, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 49s —— View job


Code review — /review:code-review

  • Gather context (diff HEAD~2..HEAD, all 13 changed files)
  • Review handshake logic, label SSOT refactor, ADR/doc changes, and new tests
  • Verify the Codex P2 finding (octal parsing) against the fix already on this head

Findings

None. This PR is clean.

The one prior finding — Codex's P2 on lib/json.sh:48 about 08.0-style leading zeros being misread as octal by bare (( )) — is already fixed on this head (commit 1eadc48). The fix forces base-10 parsing with the 10# prefix on all four components (core_major, core_minor, a_major, a_minor), and a dedicated leadingzero case was added to work-item-tracker.test.sh asserting 08.0 refuses as major 8 rather than erroring/falling through. I re-verified the arithmetic by hand: 10#08 evaluates to 8 in all four assignment sites, so the leading-zero path now refuses correctly instead of silently waving through a skewed adapter.

Beyond that, I checked:

  • Handshake wiring (work-item-tracker.sh) — wit_check_contract_version runs after the manifest-exists check and before check_gh_version/verb dispatch, for every verb; failure maps to EX_CONFIG (3) as documented.
  • Label SSOTlib/labels.sh constants are sourced by both lib/binding.sh and lib/frontier.sh, and the only remaining inline fallback in work-item-tracker.sh (list-frontier) was correctly replaced with the binding-resolved WIT_HUMAN_GATED_LABEL (no bare literal remains at any of the three former sites).
  • ADR linkCONTRACT.md's relative path to ../../../../docs/adr/0014-...md resolves correctly (4 levels up from tools/work-item-tracker/ reaches the repo root), and 0014 is the next number in docs/adr/'s sequence (last existing was 0013) — no collision.
  • Conformance additions (run-conformance.sh) — the synthetic major/minor-skew cases correctly shadow the bound provider's name via WIT_ADAPTERS_DIR without touching the real adapter, and clean up their temp root.

No correctness, maintainability, or test-coverage gaps found in the changed surface.

@kyle-sexton
kyle-sexton enabled auto-merge (squash) August 17, 2026 20:15
Both #2975 (merged) and this branch claimed work-items 0.35.25. This branch's
contract-hygiene entries move to a new 0.35.26 section; plugin.json bumped to match.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CnzwTKoTa6xNY7iyEzMYpm
@claude

claude Bot commented Aug 17, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 24s —— View job


Security review — /review:security-review

  • Gather context (PR diff at HEAD c4155b4, skip-gate check)
  • Review changed files for security-relevant issues
  • Post findings

Skip gate hit: this head already has a successful security review that still applies.

I diffed the actual PR content (gh pr diff, 13 files) against 1eadc48 — the head the two prior security-review passes (17:39, 17:44 code-review pass confirming the fix) already covered, including the leading-zero/octal handshake bug (Codex + earlier Claude review) that was fixed and verified on that same head.

Every security-relevant shell file (lib/json.sh, work-item-tracker.sh, lib/binding.sh, lib/frontier.sh, lib/labels.sh, conformance/run-conformance.sh, conformance/bindings/github.sh, work-item-tracker.test.sh) is byte-identical between 1eadc48 and the current head c4155b4. The only change since is a533fec (exec-bit mode fix on lib/labels.sh, no content change) plus unrelated doc/version-bump content pulled in by the Merge origin/main commit that isn't part of this PR's actual diff (confirmed via gh pr diff).

No new security surface to review. No security issues found — consistent with the prior passes: no injection surface (provider/declared only reach printf format args or base-10-forced arithmetic, never eval or command substitution), no path traversal, no authz/secrets handling in this diff.

@claude

claude Bot commented Aug 17, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 41s —— View job


Claude's review of PR #2970 (/review:code-review)

  • Gather context (skip gate, diff vs prior reviewed head, CLAUDE.md/REVIEW.md)
  • Review changes since last reviewed head (1eadc48c4155b4)
  • Post findings

Skip-gate note: this lane already posted a clean review of this PR's content at 1eadc48 ("Code review — /review:code-review", findings: none). The head advanced since then to c4155b4 via a533fec (exec-bit fix on lib/labels.sh) plus a merge of origin/main resolving a CHANGELOG version collision (0.35.25 → 0.35.26). I re-reviewed the delta rather than re-running a full review from scratch, since the PR's own commit set (98e294d, 1eadc48, a533fec) is unchanged and only the merge is new.

No findings. Specifically checked:

  • a533fec only flips the exec bit on lib/labels.sh — no code change. Confirmed lib/json.sh and lib/labels.sh are both 755 at the current head.
  • The merge (c4155b4) pulls in unrelated origin/main history (context-budget plugin, guardrails, planning, etc.) plus PR docs(work-items): document local-markdown branch and worktree confinement #2975's local-markdown docs into CONTRACT.md. The conflict was isolated to plugins/work-items/CHANGELOG.md and plugin.json; resolution moved this PR's entries into a new [0.35.26] section ahead of docs(work-items): document local-markdown branch and worktree confinement #2975's [0.35.25], and plugin.json matches at 0.35.26. No duplication or corruption in the merged CONTRACT.md — header structure (## Prerequisites## Conformance) is intact and the new "Contract-version handshake" section merged cleanly alongside docs(work-items): document local-markdown branch and worktree confinement #2975's unrelated "Branch, worktree, and lease confinement" section.
  • Re-verified the previously-flagged octal/leading-zero bug (Codex P2, permalink) is still fixed at the current head: all four arithmetic sites (core_major, core_minor, a_major, a_minor) force 10# base-10 parsing before any ((...)) comparison, and the leadingzero regression case ("08.0" → refuses, exit 3) plus the full unversioned/newer-major/older-major/newer-minor matrix remain present in work-item-tracker.test.sh.
  • Re-checked the dispatcher wiring (work-item-tracker.sh:113-119) and label-SSOT wiring (lib/labels.shlib/binding.sh/lib/frontier.sh) — unchanged since the last review and still correctly ordered (handshake runs after manifest-exists check, before check_gh_version/verb dispatch; WIT_HUMAN_GATED_LABEL is exported by wit_read_binding before the dispatcher's frontier call reads it).

Nothing new to flag. This PR remains clean from a correctness/maintainability standpoint.

@kyle-sexton
kyle-sexton merged commit ae0cb4f into main Aug 17, 2026
48 checks passed
@kyle-sexton
kyle-sexton deleted the claude/pocock-shipping-breakdown-ukohlx branch August 17, 2026 20:26
kyle-sexton pushed a commit that referenced this pull request Aug 17, 2026
#2970 took 0.35.26 on main; this branch's ticket/issue-synonym entries move to a
new 0.35.27 section and plugin.json bumps to match.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CnzwTKoTa6xNY7iyEzMYpm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Seam: contract hygiene — dangling ADR 0022, version handshake, label-default SSOT

2 participants