Skip to content

feat: close the setup-contract wave — final setups + contract gate - #362

Merged
kyle-sexton merged 1 commit into
mainfrom
feat/setup-contract-closing
Jul 19, 2026
Merged

feat: close the setup-contract wave — final setups + contract gate#362
kyle-sexton merged 1 commit into
mainfrom
feat/setup-contract-closing

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Summary

Closing tranche of wave #314 (epic #313, dim 8) — with this PR, every plugin in the fleet conforms to the uniform setup contract and the contract is machine-enforced.

  • disk-hygiene 0.4.0 — setup skill created (deferred until fix(disk-hygiene): unblock guarded engine lane and bounded large-root audits #286 landed): check probes Python 3.11+ (absolute-interpreter note), conditional Git, and the current OS family's documented lane (Linux lsof optional-execution note, macOS audit-only by design as INFO); guidance-only apply; disabled-toggle downgrade.
  • guardrails 0.7.0 — setup skill created (deferred until refactor(guardrails): source shared machine-path pattern bodies #305 landed): check reads the guard scripts + hooks.json as truth and probes Bash 5.0+, jq (absence = every guard fails open — surfaced as FAIL), all eight guard toggles, the cli-flag-verify scan surface, and the block-dangerous-git allowlist; guidance-only apply.
  • review 0.14.0 — its setup sat outside the wave list and was caught by the new gate on first run: adopts check | apply while keeping the standards-contract binding's by-reference discipline and explicit-confirmation gates intact.
  • Contract gatevalidate-plugin-contracts.mjs now enforces the shape on every skills/setup/SKILL.md: check leads the argument-hint, the read-only check action is documented, and apply exists unless the skill declares the doctrine's check-only userConfig-only carve-out (feat: check-only setup carve-out for userConfig-only plugins (bug-report, miro) #361). 33 setup skills pass fleet-wide; future setups are gated, not hand-reviewed.

Verification

validate-plugins.sh, validate-plugin-contracts.mjs (33 setup skills, 1541 files), markdownlint, typos: green locally. The gate change was validated by its own first catch (review).

Related

🤖 Generated with Claude Code

Closing tranche of wave #314 (epic #313, dim 8):

- disk-hygiene 0.4.0 and guardrails 0.7.0 gain their setup skills
  (unblocked now that #286/#305 landed): thin check-centric, guidance-
  only apply, platform-posture and per-guard-toggle reporting, jq
  fails-open surfaced as the FAIL it is.
- review 0.14.0: its setup — outside the wave list, caught by the new
  gate — adopts check/apply while keeping the standards-contract
  binding's by-reference discipline and explicit-confirmation gates.
- validate-plugin-contracts.mjs now enforces the contract shape on
  every setup skill: check leads the argument-hint, the read-only
  check action is documented, and apply exists unless the skill
  declares the doctrine's check-only userConfig-only carve-out.
  33 setup skills pass fleet-wide.

## Related

- Closes #314
- Part of #313

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton enabled auto-merge (squash) July 19, 2026 01:39
@claude

claude Bot commented Jul 19, 2026

Copy link
Copy Markdown

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


PR Review: feat/setup-contract-closing

  • Read changed files and gather context
  • Review contract gate (validate-plugin-contracts.mjs)
  • Review disk-hygiene setup skill
  • Review guardrails setup skill
  • Review review plugin setup skill changes
  • Summarize findings

Contract gate — validate-plugin-contracts.mjs

The three new checks are clean and minimal. A few observations:

Minor: indexOf-based body extraction doesn't anchor to line boundaries

const body = content.slice(content.indexOf("---", 3) + 3);

Line 48

indexOf("---", 3) will match the first occurrence of the three-char sequence --- anywhere in the file after position 3, not just at a line boundary. A description field that contains a literal newline followed by --- in its value (e.g. an embedded horizontal rule) would cause this to slice too early, putting frontmatter text in the body and body text after a ghost boundary. The frontmatter regex at line 38 correctly uses \r?\n--- anchoring. A consistent fix would be:

const body = content.replace(/^---[\s\S]*?\n---\n?/, "");

This is low risk for the current fleet since no description values contain \n---, but worth hardening before the gate sees more exotic skill files.

Positive: argument-hint regex handles all valid forms

/^argument-hint:\s*"check(?:\s*\||\s*\[|")/m

Line 45

Correctly accepts "check", "check | apply", and "check [verify-api]" while rejecting any non-check-leading hint. The \s* before | and [ is intentionally generous (covers both spaced and unspaced variants).

Positive: check-only carve-out is correctly anchored to the body

The !/check-only/i.test(body) escape hatch works correctly: bug-report and miro both include the literal text "Check-only" in their body (not just the frontmatter description), so the gate passes them as expected after the indexOf-based split.


disk-hygiene SKILL.md (new)

Well-structured. The "read the clean skill's bundled scripts first" instruction is the right way to avoid this file becoming a stale copy of requirements. The absolute-interpreter note for Python is an important detail (Bash aliases can't satisfy the guarded engine). The macOS audit-only note correctly framing it as INFO rather than a FAIL is precise.

One observation: the apply guidance for toggling uses <marketplace> as a placeholder rather than @melodic-software, which correctly avoids hardcoding the publisher in a reusable skill.

SKILL.md L56–60


guardrails SKILL.md (new)

The key security-relevant call is probe 2:

jq — FAIL if absent: per the README, every guard then fails OPEN (disabled) with a one-line stderr notice — the machine is unguarded.

Surfacing jq absence as a FAIL (not INFO) is the right call here — the consequence is silent guard bypass, not degraded functionality, so softening it to INFO would be misleading. The "What this skill does NOT do" section's final bullet ("Weaken a guard: it reports and routes; disabling is always the user's explicit act") reinforces this correctly.


review SKILL.md (modified)

The check behavior is woven into the Purpose section rather than a standalone ## \check`header (contrast with disk-hygiene and guardrails which have explicit## `check`headers). Both patterns satisfy the gate, and the review skill's check is intentionally thinner (it delegates to the standards-contract binding rather than owning the procedure), so the inline-in-Purpose approach is defensible. The explicit-confirmation gates forapply` (hand-authored README conversion, bootstrap writes) are preserved correctly.

The old argument-hint: "(no arguments — interactive interview)" was inconsistent with the contract even before this PR; good catch.


Minor: No ## \check`section heading inreview/SKILL.md`

Not a gate violation (the gate only checks for backtick-wrapped `check` in the body, not a section header), but the three new/modified setup skills are inconsistent in structure. disk-hygiene and guardrails have explicit ## \check` (read-only)` headers; review embeds the check description in the Purpose block. The inconsistency is cosmetic but could matter when the gate is later tightened to require a structural section marker.


Summary

The contract gate is sound and correctly enforces the three required structural invariants. The two new setup skills (disk-hygiene, guardrails) and the review update are conformant, the check-only carve-out from #361 continues to work, and all 33 fleet skills pass. The one actionable item is hardening the body-extraction regex to use a line-anchored split; everything else is positive or observational.

Fix body extraction →

@kyle-sexton
kyle-sexton merged commit 7b3a5ad into main Jul 19, 2026
14 checks passed
@kyle-sexton
kyle-sexton deleted the feat/setup-contract-closing branch July 19, 2026 01:41

@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: 172ed171de

ℹ️ 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 on lines +52 to +53
if (!/`apply`/.test(body) && !/check-only/i.test(body)) {
fail(path, "setup skills must document apply, or declare the check-only userConfig-only carve-out");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Tighten the apply-action contract check

This only checks whether the body contains the literal `apply`, rather than whether apply is an actual documented action or the setup explicitly satisfies the check-only carve-out. A future setup that says something incidental such as “do not apply changes” but exposes no apply action will pass this new gate even though it violates the required check + apply contract in docs/PLUGIN-PHILOSOPHY.md; parse an action heading/routing declaration, or require the carve-out wording when no apply action is present.

Useful? React with 👍 / 👎.

Comment on lines +38 to +42
3. **Platform posture** — detect the current OS family and report its documented lane per
the README: Windows (full, `lstat` reparse + Win32, never UAC), Linux (full when
`/proc/self/mountinfo` is readable; `lsof` needed only for the optional execution
lane — absent `lsof` is INFO with the reduced-capability note), macOS (audit/report
only by design — INFO, not a defect).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Report Windows as audit-only

On Windows, this check is instructed to report a “full” lane, but the README explicitly says that Windows “never enters the execution lane” and returns execution-platform-unsupported at preview. Consequently, a Windows user can receive a successful setup posture and only discover at cleanup time that deletion is unavailable; classify Windows as audit/report-only (an INFO, like macOS) and state that execution requires Linux.

Useful? React with 👍 / 👎.

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.

Conformance wave: setup-contract adoption (31 plugins)

1 participant