feat(markdown-format): add setup skill on the uniform contract (0.5.0)

[kyle-sexton]

Summary

Tranche 1 of wave #314 (epic #313, dim 8): the fleet's first conforming setup exemplar, on the doctrine flagship markdown-format.

The audit found no dim-8 PASS anywhere except zero-config exemptions — the uniform contract (setup skill, disable-model-invocation: true, check read-only + apply idempotent, non-interactive with complete arguments) had no reference implementation. This PR authors it as the template the remaining 30 plugins will follow:

• check — read-only probes with a PASS/FAIL/INFO table: Bash version (3.2+ floor, 5.0+ telemetry note), jq, markdownlint-cli2 via the hook's two sanctioned resolution paths (PATH or node_modules/.bin, never npx), which markdownlint config the repo's cascade discovers (+ the executable-config trust boundary), and the effective markdown_format_enabled value.
• apply — re-checks, then resolves. For system tools and the native toggle it is guidance-only (this skill never installs system packages, never writes user settings or pluginConfigs); its only write path is an explicitly requested npm install --save-dev markdownlint-cli2 in the consumer repository.
• Thin by design per doctrine's formatter/linter clause — the plugin owns no consumer-project configuration, so there is nothing else for apply to write.

Gate note: the contract gate's check/apply structural rule is deliberately deferred to the wave's final tranche — landing it now would fail the 19 not-yet-migrated shape-B setups. Recorded in the wave plan.

Verification

• validate-plugins.sh, validate-plugin-contracts.mjs (now 19 setup skills), markdownlint: green.

Related

• Part of Conformance wave: setup-contract adoption (31 plugins) #314
• Part of Epic: plugin fleet conformance audit — 50-plugin doctrine matrix and remediation waves #313

🤖 Generated with Claude Code

No linked issue: tranche 1 of #314 — the issue closes with the wave's final tranche.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bb6a90de90

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5d9afdc504

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d0efc44db4

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 482ca750b6

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 114d4e5d28

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 98c809eeaf

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8fd0127497

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d39f19a45a

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

[claude]

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

---

PR Review

• Gathering context
• Reading changed files
• Analyzing code quality, correctness, and security
• Checking alignment with CLAUDE.md / plugin philosophy
• Posting review findings

---

Summary

Strong design. The nine fix commits across this PR show disciplined iteration on Codex feedback, and the core architecture is sound: disable-model-invocation: true, clean check/apply separation, thorough package-manager detection, PnP guard via yarn config get nodeLinker, and verify-after-remediation as a backstop. The plugin-form rules in CLAUDE.md are respected throughout.

Three issues remain from the last two Codex review rounds (commits 8fd0127 and d39f19a) that weren't addressed, plus one ambiguity I found independently.

---

Findings

1. README install instructions omit apply install-lint — misleading for users with missing markdownlint-cli2

README.md:67

/markdown-format:setup apply resolves anything the check reports.

Plain apply is guidance-only for a missing markdownlint-cli2 shim — the only write path is the explicitly invoked apply install-lint. A user who follows this README line verbatim will read guidance output but won't get the package installed. This was called out in the Codex review at 8fd0127 and not addressed.

Fix: change to /markdown-format:setup apply [install-lint] or narrow the claim to "resolves what it can without installing packages; use apply install-lint to install markdownlint-cli2."

Fix this →

---

2. CHANGELOG 0.5.0 entry names npm install --save-dev as the only write path

CHANGELOG.md:16

npm install --save-dev markdownlint-cli2 as its only write path

The skill auto-detects the repo's package manager and uses pnpm add -D, yarn add -D, bun add -d, or npm install --save-dev as appropriate. A pnpm or bun consumer reading this changelog will try npm install --save-dev, introducing a competing lockfile — the exact problem the package-manager detection was built to prevent. Codex flagged this at 8fd0127 and it wasn't addressed in the subsequent d39f19a commit.

Fix: replace with something like "an explicitly invoked apply install-lint dependency install using the repository's own package manager."

Fix this →

---

3. SKILL.md check step 4 conflates advisory whole-tree scan with the hook's per-file walk

SKILL.md:37-41

mirror the hook's config walk: it loads configs from
an edited file's directory up to the repo root, so nested configs apply to nested files.
Search the whole tree (skip `node_modules`)…

These two instructions contradict each other. The hook's walk is file-specific (file directory → repo root); the check's whole-tree scan is a superset with no target file. "Mirror the hook's config walk" signals that the check reproduces the hook's exact behavior, but immediately "Search the whole tree" does something broader — an advisory scan of all configs the hook could load for any file in the repository.

This is distinct from the final Codex comment's claim (which I believe is based on a misread of the README — the README says config discovery starts "from the repository root", but the hook's collect_risky_configs and markdownlint-cli2's own behavior both walk from the file's directory up; nested configs genuinely apply to nested files). The SKILL.md's description of the walk direction is correct. The problem is the "mirror" framing misrepresents the scope.

Fix: drop "mirror the hook's config walk" and describe this as an advisory whole-tree scan, e.g. "Check step 4 does a comprehensive advisory scan of the entire repository (skip node_modules), surfacing every config the hook could load for any file. The hook's config walk starts from the edited file's directory and runs up to the repo root, so report each config with the file scope it applies to…"

Fix this →

---

4. Yarn Classic and yarn config get nodeLinker null return is implicit

SKILL.md:58-63

The SKILL.md says run yarn config get nodeLinker and treat pnp as PnP-path, node-modules/pnpm as install-path, and "Yarn Classic (which has no such setting)" as install-path. On Yarn Classic yarn config get nodeLinker returns null (the config key doesn't exist); the text doesn't name this expected output, so a model following the instructions must infer that null or a non-zero exit is the "no such setting" signal. This is a minor ambiguity that could cause a model to stop with unsupported-path guidance when it sees null instead of proceeding with the install.

Fix: add null to the install branch: `node-modules`/`pnpm`, `null` (no such key — Yarn Classic), or a non-zero exit → install.

---

Not a bug: README config-discovery direction

README.md:20 says markdownlint-cli2 walks "from the repository root" — this is imprecise (it walks from the file's directory). The SKILL.md's description of this walk at line 37 is more accurate than the README. Worth fixing in a follow-up, but it's a pre-existing README inaccuracy and not introduced by this PR.

---

What's working well

• disable-model-invocation: true is correctly set; the skill correctly stays in tool-call mode.
• Package manager detection ladder (lockfile → packageManager field → npm) is well-ordered and adds the packageManager-contradicts-lockfile guard.
• PnP detection via yarn config get nodeLinker is the right call; inferred from markers was correctly replaced.
• Trust-boundary criteria now match collect_risky_configs exactly (.cjs/.mjs + declarative configs with module-loading keys).
• Verify-after-remediation is a good backstop.
• apply writes nothing system-wide; the one write path requires explicit authorization.

[claude]

Claude finished @kyle-sexton's task in 3m 35s —— View job

---

PR Review

• Gathering context
• Reading changed files at current HEAD
• Checking if previous Claude review findings were addressed
• Analyzing code quality, correctness, and security
• Posting review findings

---

Status: 4 findings carried forward from prior review rounds, still open at HEAD 87fbd58

The merge commit (87fbd58) brought in CI/doc fixes from main only — no plugin files changed. All four issues flagged in the previous Claude review at 8fd0127 / d39f19a remain unaddressed.

---

Finding 1 — README claims plain apply resolves everything (misleading)

README.md:66-67

`/markdown-format:setup apply` resolves anything the check reports.

Plain apply is guidance-only for a missing markdownlint-cli2 shim. The only install write-path is the explicitly invoked apply install-lint. A user following this line verbatim will get guidance output but their prerequisite stays missing.

Fix: name both forms — e.g. apply for guidance + re-verify; apply install-lint to install the dependency.

Fix this →

---

Finding 2 — CHANGELOG names npm install --save-dev as the only write path

CHANGELOG.md:14-16

`apply` re-checks and resolves — guidance for system tools and the native
toggle, and an explicitly requested consumer-repo
`npm install --save-dev markdownlint-cli2` as its only write path.

The skill auto-detects and uses pnpm add -D, yarn add -D, bun add -d, or npm install --save-dev as appropriate. A pnpm or bun consumer reading this changelog entry will think npm is the right command, potentially introducing a competing lockfile.

Fix: replace with "an explicitly invoked apply install-lint dependency install using the repository's own package manager."

Fix this →

---

Finding 3 — SKILL.md check step 4: "mirror the hook's config walk" contradicts "Search the whole tree"

SKILL.md:37-38

4. **Consumer markdownlint config** — mirror the hook's config walk: it loads configs from
   an edited file's directory up to the repo root, so nested configs apply to nested files.
   Search the whole tree (skip `node_modules`), ...

"Mirror the hook's config walk" signals the check reproduces the hook's per-file walk, but the next instruction is a whole-tree advisory scan — broader scope, no target file. These contradict each other. (The walk direction described — file directory up to repo root — is correct; only the "mirror" framing misrepresents the check's scope.)

Fix: reframe this as an advisory scan, e.g. "advisory whole-tree scan (not a per-file walk). The hook's config discovery starts from the edited file's directory and walks up to the repo root (so nested configs apply to nested files). Search the whole tree (skip node_modules)…"

Fix this →

---

Finding 4 — SKILL.md: Yarn Classic null return from yarn config get nodeLinker not named

SKILL.md:61-63

`node-modules`/`pnpm`, or Yarn Classic (which has no such
setting and always materializes `node_modules`) → install.

On Yarn Classic, yarn config get nodeLinker returns null (the key doesn't exist). The text describes Yarn Classic parenthetically but doesn't name null as the expected output a model will see. A model that gets null from the command and tries to match it against pnp, node-modules, or pnpm may stop with unsupported-path guidance instead of proceeding to install.

Fix: add null and a non-zero exit to the install branch: `node-modules`/`pnpm`, `null` (no such key — Yarn Classic), or a non-zero exit → install.

Fix this →

---

What's working well

The core architecture is sound and the 9 fix commits from the Codex review rounds landed well:

• disable-model-invocation: true correctly set
• check | apply [install-lint] argument-hint documents the explicit install action
• Package manager detection ladder (lockfile → packageManager field → npm) is well-ordered with the contradicting-signals guard
• PnP detection via yarn config get nodeLinker (not inferred from marker files) is the right approach
• Positive-evidence PnP test (.pnp.cjs/.pnp.loader.mjs or nodeLinker: pnp) avoids false positives for Berry with a materializing linker
• Trust-boundary criteria match collect_risky_configs exactly (.cjs/.mjs + declarative configs with customRules/markdownItPlugins/outputFormatters)
• Verify-after-remediation backstop is solid
• apply writes nothing system-wide; the one write path requires explicit authorization

---

Note: I attempted to push fixes for all four items directly to this branch but the GitHub App lacks write access to this repository. The four [Fix this →] links above will pre-fill Claude Code with the exact changes needed.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 87fbd5842a

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

[claude]

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

---

PR Review

• Gathering context
• Reading changed files at current HEAD (42660b3)
• Checking if previous Claude review findings were addressed
• Checking new Codex finding (npm-shrinkwrap.json)
• Posting review findings

---

Status: 5 open findings — 4 carried forward, 1 new

The merge commit (42660b3) pulled only CI changes from main (0c1c1cc). No plugin files changed; git diff 87fbd58..42660b3 -- plugins/markdown-format/ is empty. All four findings from the prior Claude review at 87fbd58 remain open, and the Codex finding added at 87fbd58 (npm-shrinkwrap.json) is also open.

---

Finding 1 — README claims plain apply resolves everything (open since 8fd0127)

README.md:67

`/markdown-format:setup apply` resolves anything the check reports.

Plain apply is guidance-only for a missing markdownlint-cli2 shim — the only write path is the explicitly invoked apply install-lint. A user following this verbatim with markdownlint-cli2 absent will read guidance but their prerequisite will remain missing.

Fix: name both forms — e.g. `/markdown-format:setup apply` re-checks and resolves what it can (guidance for system tools and the toggle); use `/markdown-format:setup apply install-lint` to install `markdownlint-cli2` as a dev dependency using your repository's package manager.

Fix this →

---

Finding 2 — CHANGELOG names npm install --save-dev as the only write path (open since 8fd0127)

CHANGELOG.md:14-16

`apply` re-checks and resolves — guidance for system tools and the native
toggle, and an explicitly requested consumer-repo
`npm install --save-dev markdownlint-cli2` as its only write path.

The skill auto-detects the repository's package manager and uses pnpm add -D, yarn add -D, bun add -d, or npm install --save-dev as appropriate. A pnpm or bun consumer reading this changelog entry will think npm is correct and could introduce a competing lockfile.

Fix: replace the npm command with: and an explicitly invoked `apply install-lint` dependency install using the repository's own package manager as its only write path.

Fix this →

---

Finding 3 — SKILL.md check step 4: "mirror the hook's config walk" contradicts "Search the whole tree" (open since 8fd0127)

SKILL.md:36-37

4. **Consumer markdownlint config** — mirror the hook's config walk: it loads configs from
   an edited file's directory up to the repo root, so nested configs apply to nested files.
   Search the whole tree (skip `node_modules`), ...

"Mirror the hook's config walk" signals that the check reproduces the hook's per-file walk (file directory → repo root). The very next instruction says "Search the whole tree" — a repository-wide advisory scan with no target file. These describe different operations. The walk direction (file directory → repo root) is correct; only the "mirror" framing misrepresents the check's scope.

Fix: reframe as an advisory scan. Replace mirror the hook's config walk: it loads configs from an edited file's directory up to the repo root, so nested configs apply to nested files. with advisory whole-tree scan (not a per-file walk). The hook's config discovery starts from the edited file's directory and walks up to the repo root, so nested configs apply to nested files.

Fix this →

---

Finding 4 — SKILL.md: Yarn Classic null return from yarn config get nodeLinker not named (open since 87fbd58)

SKILL.md:62-63

`node-modules`/`pnpm`, or Yarn Classic (which has no such
setting and always materializes `node_modules`) → install.

On Yarn Classic, yarn config get nodeLinker outputs null (the key doesn't exist). The text describes what Yarn Classic does parenthetically but doesn't name null as the expected command output. A model that sees null and tries to match it against pnp, node-modules, or pnpm may fall through to unsupported-path guidance instead of proceeding with the install.

Fix: add null to the install branch: `node-modules`/`pnpm`, `null` (no such key — Yarn Classic, which always materializes `node_modules`), or a non-zero exit → install.

Fix this →

---

Finding 5 (new) — npm-shrinkwrap.json missing from the lockfile signal list

SKILL.md:53-55

lockfile (`pnpm-lock.yaml` → `pnpm add -D`, `yarn.lock` → `yarn add -D`,
`bun.lock`/`bun.lockb` → `bun add -d`, `package-lock.json` → `npm install --save-dev`)

npm-shrinkwrap.json is npm's dependency-locking mechanism (equivalent to package-lock.json). A repository that uses npm shrinkwrap will have npm-shrinkwrap.json as its npm lockfile but no package-lock.json. The current ladder doesn't match this signal, so the skill falls through to the "packageManager" field or the npm-only default — in the latter case it runs npm install --save-dev without detecting an existing competing lockfile scenario. This is the same class of competing-lockfile hazard the ladder was built to prevent.

Fix: add npm-shrinkwrap.json as an npm lockfile signal alongside package-lock.json, and include it in the multi-lockfile conflict check.

Fix this →

---

What's working well

The core design remains sound. All the substantive fixes from the nine Codex iterations landed well and are still in place:

• disable-model-invocation: true correctly set; argument-hint exposes install-lint
• Package manager ladder (lockfile → packageManager field → npm) with the contradicting-signals guard
• PnP detection via yarn config get nodeLinker rather than marker-file inference
• Positive-evidence PnP test (.pnp.cjs/.pnp.loader.mjs or nodeLinker: pnp) avoids false positives for Berry with a materializing linker
• Trust-boundary criteria match collect_risky_configs exactly (.cjs/.mjs + declarative configs with customRules/markdownItPlugins/outputFormatters)
• Verify-after-remediation backstop on every install path
• apply writes nothing system-wide; the one write path requires explicit invocation

---

Note: All five fixes are documentation/SKILL.md-only changes — no hook logic is affected.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 42660b3120

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

[claude]

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

---

PR Review

• Gathering context
• Reading changed files at current HEAD (52a1bb7)
• Checking if prior review findings (5 open) were addressed
• Analyzing new commit for correctness
• Posting review findings

---

Status: 3 of 5 findings fixed — 2 carry forward, 1 Codex finding still open

Commit 52a1bb7 ("round-7 review — docs match the skill, shrinkwrap rung") closed three of the five open issues from the previous round. Two documentation clarity issues in SKILL.md carry forward.

---

Fixed in 52a1bb7 ✓

Finding 1 — README apply claim — README.md:67-71 now names both forms explicitly — apply for guidance and apply install-lint for the dependency install. Closed.

Finding 2 — CHANGELOG hardcodes npm — CHANGELOG.md:14-18 now says "via the repository's own package manager (npm, pnpm, Yarn, or Bun, resolved from the repo's lockfile and packageManager field)." Closed.

Finding 5 — npm-shrinkwrap.json missing — SKILL.md:54 now reads `package-lock.json` or `npm-shrinkwrap.json` → `npm install --save-dev`. Closed.

Also fixed in 52a1bb7: the README config-cascade direction (previously "from the repository root") is now corrected to "per edited file, from the file's directory up through its parents — so a nested config governs its subtree." This resolves the pre-existing inaccuracy noted as "not a bug in this PR" in the prior round.

---

Still Open

Finding 3 — SKILL.md check step 4: "mirror the hook's config walk" contradicts "Search the whole tree"

SKILL.md:36-38

4. **Consumer markdownlint config** — mirror the hook's config walk: it loads configs from
   an edited file's directory up to the repo root, so nested configs apply to nested files.
   Search the whole tree (skip `node_modules`), ...

Unchanged since bb6a90d. The walk direction described (file directory → repo root) is correct — and now matches the updated README. But "mirror the hook's config walk" signals the check reproduces the hook's per-file behavior for one specific file, while "Search the whole tree" implements a broader advisory scan across all files in the repository. A model following these instructions faces a contradiction in scope.

Fix: replace mirror the hook's config walk: it loads configs from an edited file's directory up to the repo root, so nested configs apply to nested files. with advisory whole-tree scan (not a per-file walk). The hook's config discovery starts from the edited file's directory and walks up to the repo root (so nested configs apply to nested files).

Fix this →

---

Finding 4 — SKILL.md: Yarn Classic null return from yarn config get nodeLinker not named

SKILL.md:62-63

`node-modules`/`pnpm`, or Yarn Classic (which has no such
setting and always materializes `node_modules`) → install.

Unchanged since 87fbd58. On Yarn Classic, yarn config get nodeLinker outputs null (the config key doesn't exist in v1). The text identifies Yarn Classic parenthetically but doesn't name null as the expected command output a model will actually see. A model matching the command output against pnp, node-modules, or pnpm won't find null in the install branch and may fall through to unsupported-path guidance.

Fix: add null to the install branch: `node-modules`/`pnpm`, `null` (no such key — Yarn Classic, which always materializes `node_modules`), or a non-zero exit → install.

Fix this →

---

Codex finding (from 42660b3) — linter executable not validated end-to-end

SKILL.md:32-35

The check step 3 instruction says to resolve the shim "including its symlink/escape validation" — but doesn't say to verify the binary actually executes. A stale node_modules/.bin/markdownlint-cli2 shim (e.g. checked into version control without its node_modules subtree) will pass the path-existence and symlink checks but fail when the hook later invokes it because Node isn't installed or the module tree is absent. The check would report PASS while subsequent edits still skip.

This is less severe than findings 3 and 4 (requires a broken environment rather than a normal one) but worth a one-liner: instruct the check to run markdownlint-cli2 --version (or equivalent harmless invocation) against the resolved path and FAIL if the exit code is non-zero, to catch a shim whose runtime is absent.

---

What's working well

The three fixes in 52a1bb7 are clean and accurate. The README config-cascade correction is a nice bonus. The package manager detection ladder, PnP guard, trust-boundary criteria, and verify-after-remediation backstop are all sound. Two of the three remaining items are two-word fixes in SKILL.md.

[claude]

Claude finished @kyle-sexton's task in 3m 29s —— View job

---

PR Review

• Gathering context
• Reading changed files at current HEAD (435af2e)
• Checking if prior review findings (5 open) were addressed
• Checking new commits since last review (52a1bb7, 435af2e)
• Analyzing for new issues
• Posting review findings

---

Status: 4 of 5 prior findings resolved; 1 still open

Two commits landed since the last Claude review at 42660b3:

• 52a1bb7 ("round-7 review — docs match the skill, shrinkwrap rung") — addressed Findings 1, 2, 5, and the README config-walk description from Finding 3.
• 435af2e ("setup check adds a --version liveness probe") — addressed the new Codex finding about linter-execution verification.

---

Resolved since last review

Finding 1 — README apply vs apply install-lint — Fixed ✅

README.md:67-71 now correctly distinguishes both forms: apply for guidance + re-verify, apply install-lint to authorize the dev dependency install via the repo's own package manager.

Finding 2 — CHANGELOG names npm install --save-dev as the only write path — Fixed ✅

CHANGELOG.md:15-18 now correctly describes the install as package-manager-aware (npm, pnpm, Yarn, or Bun, resolved from lockfile and packageManager field).

Finding 5 — npm-shrinkwrap.json missing from lockfile signal — Fixed ✅

SKILL.md:57 now reads `package-lock.json` or `npm-shrinkwrap.json` → `npm install --save-dev`, covering the npm shrinkwrap case in both the detection ladder and the multi-lockfile ambiguity path.

Codex Finding (liveness probe) — apply install-lint can succeed while the shim is broken — Fixed ✅

SKILL.md:35-38 now requires running the resolved tool with --version after resolution, and SKILL.md:84-86 correctly scopes "the only execution check performs" so the "does NOT do" section stays consistent with the new probe.

---

Still open

Finding 4 — SKILL.md: Yarn Classic null return from yarn config get nodeLinker not named

SKILL.md:65-66

`node-modules`/`pnpm`, or Yarn Classic (which has no such
setting and always materializes `node_modules`) → install.

On Yarn Classic, yarn config get nodeLinker outputs null (the key does not exist in the config). The text describes Yarn Classic parenthetically but a model executing the command sees null — a value that doesn't match pnp, node-modules, or pnpm. Without null named as an install-branch signal, the model may fall through to unsupported-path guidance instead of proceeding with the install.

Fix: add null explicitly — e.g. `node-modules`/`pnpm`, `null` (no such key — Yarn Classic, which always materializes `node_modules`), or a non-zero exit → install.

Fix this →

---

Finding 3 — residual note (not re-raising as a blocker)

SKILL.md:39-40 still uses "mirror the hook's config walk" immediately before "Search the whole tree." The author refuted the directionality claim and corrected the README (which now correctly describes per-file discovery capping at the root). The interpretation — that "mirroring" means producing the same coverage as the hook would for any file, which requires the whole-tree scan — is defensible. Noting it for reference but not re-raising as a blocking issue.

---

What's working well

The two new commits are clean:

• 52a1bb7 addressed every documentation gap from the prior review without introducing new issues.
• 435af2e adds the --version liveness probe in the right place (step 3, inside check) and keeps the "does NOT do" section internally consistent — the probe is carved out of the "execute tools" negative, and "Download anything" correctly excludes the local probe that --version represents.

The core architecture remains solid: disable-model-invocation: true, argument-hint exposes the install-lint sub-action, package manager ladder is correctly ordered with the contradicting-signals guard, PnP detection queries yarn config get nodeLinker rather than inferring from marker files, trust-boundary criteria match collect_risky_configs exactly, and apply writes nothing system-wide without explicit authorization.

---

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 435af2ecb7

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

[chatgpt-codex-connector]

Verify that jq can execute before reporting it available

When jq resolves on PATH but cannot run (for example, a stale binary with a missing dynamic loader or an incompatible executable), this probe reports PASS. The hook uses the same command -v gate in hooks/markdown-format.sh:132-139, then relies on jq to parse file_path at lines 193-210; a failed parse makes the hook exit without formatting. Run a harmless jq execution/capability probe so setup does not report the prerequisite as configured in this case.

Useful? React with 👍 / 👎.