refactor(guardrails): source shared machine-path pattern bodies - #305
Conversation
The per-OS regex bodies were a hand-synced copy of the same bodies carried by ci-workflows' machine-specific-paths action and medley's tools/shared/path-detection. They now live in machine-path-patterns.sh — the standards-managed materialization (melodic-software/standards#171) — and the lib keeps only its own scan wrapping. Closes #301 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S3Gt4Q3aggmM2Gb72aRmkr
|
Claude finished @kyle-sexton's task in 1m 46s —— View job Code Review
SummaryClean, correct refactor. The five ObservationsSource chain is correct
When a script is sourced, Pattern values are byte-identicalComparing the old inline definitions (visible in The PR description's "byte-identical" claim checks out; no silent regex change. SC2034 / SC1003 directives are correctly scoped
|
Resolve plugin.json conflict in guardrails: compose main's minor bump (0.6.0, block-dangerous-git guard + eight-guard description) with this branch's patch bump for the path-detection refactor, giving 0.6.1. The version bump is the delivery vehicle for the shared-source .sh refactor; resolving to a bare 0.6.0 would not deliver it to consumers already on 0.6.0. Description takes main's eight-guard text (unchanged by this branch).
|
Claude finished @kyle-sexton's task in 2m 1s —— View job Code Review
SummaryClean, correct refactor. The five FindingsSource chain is correct
When a script is sourced, Pattern values are byte-identicalComparing the removed inline definitions against The PR description's "byte-identical" claim checks out; no silent regex change. Shellcheck directives are correctly scoped
Plugin-form safety (CLAUDE.md compliance)
Test coverage
Version bumpThe PR commit bumps 0.4.0 → 0.4.1 (patch; correct for a behavior-preserving refactor). After the merge with Carry-over observation (non-blocking)No in-repo sync mechanism for the upstream canonical. The file header says this is "the standards-managed materialization" sourced from Minor nit (non-blocking)Module header ordering in Verdict: approve. The refactor is correct, safe, and well-documented. The sync-mechanism gap is a known trade-off acknowledged by the PR scope. |
… (0.6.2) Move the SC2034 pragma (with its rationale comment) after the module description block per shell=bash -> description -> pragma -> code convention, per claude[bot] review nit on PR #305. Comment-only change; patterns are unchanged.
|
Claude finished @kyle-sexton's task in 1m 5s —— View job Code Review
SummaryThe previous nit has been addressed. The commit FindingsHeader ordering nit — resolved ✓Before ( After ( The pragma now sits immediately before the code it guards, with its own rationale comment as context. This is the idiomatic placement. Version bump
Carry-over (non-blocking, same as prior reviews)No in-repo sync mechanism for the upstream canonical ( Verdict: approve. Prior nit addressed correctly; patterns are unchanged; version bump is appropriate. No new concerns. |
|
Classification of the 2026-07-17 review findings:
|
|
Classification of the 2026-07-18 review findings (carry-over of the same 2 findings from the prior review, plus a non-actionable description-staleness note):
|
|
Acknowledged — the 2026-07-18 22:03 re-review confirms the header-ordering fix in commit |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96a803a29d
ℹ️ 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".
Both versions bumped plugin.json without a changelog entry, so consumers updating deliberately (per docs/MIGRATION-PLAYBOOK.md's version-pinning model) had no way to see what changed. Per chatgpt-codex-connector's review on PR #305: document the SSOT refactor (0.6.1) and the header-ordering fix (0.6.2).
|
Claude finished @kyle-sexton's task in 1m 16s —— View job Code Review
SummaryThe triggering commit ( CHANGELOG additions — correct ✓0.6.2 entry accurately describes the comment-only header ordering fix ( 0.6.1 entry accurately describes the machine-path body extraction: names all five Both entries are placed in descending-version order above the existing 0.6.0 entry — format is consistent with the rest of the file. Prior findings status
Verdict: approve. The Codex P2 finding is addressed with accurate, well-scoped entries. No new concerns. |
…#355) ## Summary Tranche 2 of wave #314 (epic #313, dim 8): the battle-tested markdown-format exemplar from #342 (six adversarial review rounds), cloned to the nine collision-free shape-A plugins. | Plugin | Version | apply write path | |---|---|---| | actionlint | 0.4.0 | none — guidance only | | bash-format | 0.5.0 | none — guidance only | | powershell-format | 0.4.0 | none — guidance only (module install is user-scope guidance) | | eol-normalizer | 0.4.0 | none — guidance only | | desktop-notification | 0.4.0 | none — probes only the current OS family's channel deps | | biome-format | 0.4.0 | `apply install-biome` — repo package manager (lockfile → packageManager → npm; Plug'n'Play → guidance) | | ruff-format | 0.4.0 | `apply install-ruff` — only into a managed env the repo already uses; never creates one | | firecrawl | 0.3.0 | none — defers to the plugin's lazy-install design; reports `FIRECRAWL_API_KEY` presence without printing it | | playwright | 0.3.0 | `apply install-cli` — stated-before-running global npm install | Contract invariants everywhere: `check` reads the plugin's own hook/skill source as single source of truth and probes what it actually does (no hand-copied lists to drift); `apply` re-checks first, names every write path in the argument-hint, states it before running, re-verifies after every remediation; never writes the plugin cache, user settings, or `pluginConfigs`; non-interactive when the action is supplied; `disable-model-invocation: true`. Also backfills the undocumented `[0.3.1]` CHANGELOG entries in actionlint and powershell-format (#298's hook-utils resync, verified via `git log -S`). **Deferred for lane ownership** (per the org WIP survey): disk-hygiene (PR #286 touches its plugin.json/CHANGELOG) and guardrails (PR #305 ditto) — they join a later tranche after those PRs land. ## Verification - `validate-plugins.sh`, `validate-plugin-contracts.mjs` (28 setup skills, 1513 files), markdownlint (55 files), `typos`: all green locally. ## Related - Part of #314 - Part of #313 No linked issue: tranche 2 of #314 — the issue closes with the wave's final tranche. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
## Summary Tranche 3 of wave #314 (epic #313, dim 8): all 17 remaining shape-B setups onto the uniform check/apply contract — refactors that preserve each plugin's own verification logic, plus two creates where the audit showed the listing was imprecise (claude-config shipped no setup; kindle-dedrm's setup was a router action, now a dedicated skill with the router delegating to it). | Plugin | Version | Notable | |---|---|---| | machine-health | 0.6.0 | interview → apply path; non-interactive disable/deprecate/demote/approve args | | source-control | 0.9.0 | carry-in: stale `install --config` reconfigure advice → fresh-install-only semantics | | toolchain | 0.4.0 | delivers the apply action its 0.3.0 entry forecast; per-ecosystem non-interactive scope | | claude-config | 0.6.0 | CREATE (audit: no setup shipped); README gains the bash prereq (dim-10 nit) | | claude-ops | 0.12.0 | freshness riders preserved verbatim; evals re-routed to explicit `apply` | | skill-quality | 0.5.0 | fixes README's dim-6 mis-framing of settings writes | | songwriting | 0.5.0 | one deliberate interactive exception: destructive-collision confirmation | | ai-briefing | 0.6.0 | verify+install fusion split (`apply install-build-deps`); 4 stale cross-refs in generate updated | | context7 | 0.4.0 | CLI/MCP dual-path check; `apply install-cli` gated | | discovery | 0.7.0 | gains the missing non-interactive arg path | | knowledge | 0.8.0 | ingest provisioners behind `apply install-deps` (verified standalone + idempotent) | | code-tidying / codebase-health | 0.5.0 | lanes / merged-config checks split from interview | | kindle-dedrm | 0.4.0 | dedicated setup skill; pinned-tag + Key_Finder guards untouched (referenced, not restated) | | planning | 0.20.0 | two-concern check incl. standards-index version delta; toggle carries `--config` semantics | | repo-fleet-hygiene | 0.2.0 | check split over existing arg grammar | | work-items | 0.11.0 | probes the tracker-seam gates as authoritative, never restates them | Contract refinements from tranches 1–2 embedded throughout: opt-in absence = INFO vs broken = FAIL; verify-after-remediation; secrets presence-only; env-var changes defer verification to a fresh session; `--config` fresh-install-only reconfigure guidance; non-interactive with complete arguments. Write-behavior evals re-routed to explicit `apply`. Root README catalog regenerated. With this PR, 27 of 31 plugins conform. Remaining: bug-report + miro (check-only carve-out PR with its one-line doctrine sanction), disk-hygiene + guardrails (deferred for the #286/#305 lanes), then the contract-gate check/apply rule as the closing tranche. ## Verification - `validate-plugins.sh` (incl. regenerated catalog), `validate-plugin-contracts.mjs` (31 setup skills, 1537 files), markdownlint (725 files), `typos`: all green locally. Agents additionally ran skill-quality checks on the six plugins in group D (all PASS). ## Related - Part of #314 - Part of #313 No linked issue: tranche 3 of #314 — the issue closes with the wave's final tranche. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ort, miro) (#361) ## Summary Resolves the wave #314 design question (raised in the issue before mass-refactoring bug-report/miro): where a plugin's **entire** configuration surface is native `userConfig` — no tracked project config, no prerequisite an install could resolve — an `apply` action would have nothing to write except the `pluginConfigs` the setup contract itself forbids. - **Doctrine** (`docs/PLUGIN-PHILOSOPHY.md`, setup section): one added sentence sanctioning the check-only shape for exactly that case. Kept in its own PR so the doctrine change is reviewable in isolation. - **bug-report 0.5.0**: `check` is the sole action; the interactive "ask whether reports should be private or repo-resident" step becomes a stated tradeoff with a recommendation (non-interactive); reconfigure guidance states `--config`'s fresh-install-only semantics; rechecks defer to a fresh session since the rendered value is injected at load. - **miro 0.2.0**: `check [verify-api]` — the optional read-only credential probe (list one board, count only) becomes an explicit argument instead of an in-flow question; token never read, printed, or persisted (unchanged). MCP server version realigned across `package.json`, the server `Implementation` version, the lockfile, and the rebuilt committed bundle (drift check + typecheck green; lockfile delta is the two version fields). With this, 29 of 31 wave plugins conform. Remaining: disk-hygiene + guardrails (deferred behind the #286/#305 lanes) and the closing contract-gate rule. ## Verification `validate-plugins.sh`, `validate-plugin-contracts.mjs`, markdownlint, `typos`, miro bundle drift check + typecheck: all green locally. ## Related - Part of #314 - Part of #313 No linked issue: carve-out slice of #314 — the issue closes with the wave's final tranche. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
) ## 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 #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 #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 gate** — `validate-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 (#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 - Closes #314 - Part of #313 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The guardrails lib's per-OS regex bodies were a hand-synced copy of the same bodies carried by ci-workflows' machine-specific-paths action and medley's tools/shared/path-detection. The bodies now live in machine-path-patterns.sh — the standards-managed materialization added by melodic-software/standards#172 — and the lib keeps only its own scan wrapping (OS-context suppression, exclusion pipes). Byte-identical to the canonical, so the first sync reconciles as a no-op. Guardrails bumped to 0.4.1.
Closes #301
Related
🤖 Generated with Claude Code
https://claude.ai/code/session_01S3Gt4Q3aggmM2Gb72aRmkr