Skip to content

feat: declare and classify skill-channel runtime prerequisites (5 plugins) - #329

Merged
kyle-sexton merged 5 commits into
mainfrom
feat/prereq-declarations-skills
Jul 18, 2026
Merged

feat: declare and classify skill-channel runtime prerequisites (5 plugins)#329
kyle-sexton merged 5 commits into
mainfrom
feat/prereq-declarations-skills

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Tranche B of the prerequisite-visibility wave: declare every skill/agent-channel runtime prerequisite at point of use and in the README, with absence classified per doctrine (PLUGIN-PHILOSOPHY § Prerequisites and failure behavior: required → stop with remediation; optional → visible warn + skip; not-applicable → quiet).

Per-plugin changes

Plugin Version Change
docs-hygiene 0.7.0 compress's markdownlint-cli2 reclassified required-for-correctness: it is the mandatory ship gate, so absence now stops at the entry point with an install remediation instead of being handled like a lint failure. README gains a Requirements section (ambient Bash/git/jq, compress-only markdownlint-cli2, optional caveman backend).
kindle-dedrm 0.3.0 gh (authenticated) declared in README; the setup workflow's DeDRM_tools download falls back to the pinned tag from references/versions.md when gh is unavailable instead of composing a malformed URL; the Key_Finder URL resolution gains the same empty-match guard (the tutorial source currently 404s — exactly the rot this guard and the check-drift action exist for). First CHANGELOG created.
source-control 0.7.1 README-only: declares jq, Bash (Git Bash on native Windows), and the CI-log fetch path's unzip requirement. The point-of-use gates already existed (fetch-failed-logs exit 5 "unzip required", fetch-annotations "jq required") — the audit FAIL was the undocumented README.
work-items 0.10.0 README declares Bash + jq with the Windows path (jq is a separate install in Git Bash) and classifies jq required-for-correctness. The tracker-seam reference gains an explicit entry-point presence check for the seam script with remediation pointers (CONTRACT.md, /work-items:setup).
review 0.13.0 README gains a Requirements section (git; authenticated gh; Bash via Git Bash). ci-log-auditor now verifies gh presence/auth as step 0 and stops with remediation instead of auditing from partial evidence.

Verification

  • scripts/validate-plugins.sh, node scripts/generate-catalog.mjs --check, and node scripts/validate-plugin-contracts.mjs (18 setup skills, 1428 files) all green.
  • Every install/remediation URL added in this tranche verified live this session (HTTP 200); the one 404 (techy-notes.com tutorial) is a pre-existing pinned source whose rot the new guard now degrades cleanly.
  • No script behavior changed in source-control (its gates already conformed); behavioral changes are instruction-level classification gates in docs-hygiene compress, kindle-dedrm workflow, work-items seam entry, and review ci-log-auditor.

Related

Part of #317. No linked issue is closed by this PR (tranche C remains before #317 can close).

🤖 Generated with Claude Code

@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: bffa2df156

ℹ️ 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/kindle-dedrm/skills/kindle-dedrm/references/workflow.md Outdated
Comment thread plugins/kindle-dedrm/skills/kindle-dedrm/references/workflow.md Outdated
…gins)

Prerequisite-visibility wave, dim-9 skill/agent channel: docs-hygiene
(markdownlint-cli2 is compress's ship gate — absence now stops at entry
with remediation, README declares it), kindle-dedrm (gh declared;
workflow download step falls back to the pinned tag instead of composing
a malformed URL; Key_Finder URL gains the same empty-match guard),
source-control (README declares jq + Bash + unzip with the existing
stop-with-remediation behavior), work-items (README declares Bash + jq
with Windows path; tracker-seam reference gains an entry-point presence
check), review (README gains a Requirements section; ci-log-auditor
checks gh presence/auth up front).

Part of #317 (tranche B).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton force-pushed the feat/prereq-declarations-skills branch from bffa2df to 29d29b9 Compare July 18, 2026 06:04

@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: 29d29b9426

ℹ️ 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/reference/tracker-seam.md Outdated
Codex review P2 x3 on the tranche-B changes: the DeDRM no-gh fallback
now requires the real pinned tag substituted from references/versions.md
and refuses to run with the placeholder (no malformed URL possible); a
missing Key_Finder URL now stops the block before the hash/extract steps
instead of failing later with misleading errors; the work-items seam
remediation no longer points at /work-items:setup (which does not create
the seam) and states the consumer-provisioning path instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@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: d12033ab5d

ℹ️ 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/kindle-dedrm/skills/kindle-dedrm/references/workflow.md Outdated
kyle-sexton added a commit that referenced this pull request Jul 18, 2026
)

Tranche C of the prerequisite-visibility wave: every dim-10
cross-platform FAIL from the conformance audit — undeclared
shell/coreutils assumptions get a stated requirement with the Git Bash
Windows path, and the two structural Windows hazards get real fixes.

## Per-plugin changes

| Plugin | Version | Change |
|---|---|---|
| mcp-tools | 0.2.1 | README Requirements section added (Bash +
coreutils, jq, Git Bash path). CHANGELOG created. |
| ai-briefing | 0.5.1 | README states the POSIX-shell requirement of
`setup --with-build-deps` (script's platform gate already accepts
MINGW/MSYS/CYGWIN — the doc prong was the gap). |
| education | 0.3.1 | README corrected from "none beyond Claude Code":
declares `sha256sum`/`shasum`, `realpath`, `tr`, `sed` with the Git Bash
path (bundles all of them). CHANGELOG created. |
| prototype | 0.2.4 | README declares the detection-script Bash
requirement and documents the no-Bash degrade honestly (pre-computed
context echoes "none detected"; the skills read the host project
directly — verified against `skills/logic/SKILL.md`, which never
branches on the detection output). |
| code-tidying | 0.4.3 | README declares Bash 4+ (`mapfile`,
case-conversion expansions) with the Git Bash path; notes the scripts'
existing CRLF/drive-letter handling. |
| knowledge | 0.7.1 | README declares the Git Bash shell mechanics
(`book-distill` hashes with `sha256sum` on every run) and the EPUB
branch's `unzip` requirement (not bundled with Git Bash); PDF-only use
needs neither. |
| event-storming | 0.5.3 | Simulation teardown phrased
shell-agnostically at both sites (`rm -rf` on POSIX/Git Bash,
`Remove-Item -Recurse -Force` on PowerShell) instead of unconditional
`rm -rf`. |
| repo-hygiene | 0.2.1 | `clean`'s PreToolUse destructive-guard hook
moves from a bare-`.sh` command to the interpreter-named exec form
(`command: "bash"`, `args: […]`) — the doctrine-prescribed Windows-safe
spawn shape, matching disk-hygiene's existing `command: "python"`
precedent. |

## Verification

- `scripts/validate-plugins.sh`, `node scripts/generate-catalog.mjs
--check`, and `node scripts/validate-plugin-contracts.mjs` (1429 files)
green.
- `plugins/repo-hygiene/.../destructive-guard.test.sh`: 30/30 passing
after the hook-form change.
- The dim-9 work landed separately: hook channel in #328, skill channel
in #329. With this tranche, every dim-9/dim-10 FAIL enumerated in #317
is addressed (actionlint's and work-items' dim-10 items were folded into
those PRs).

## Related

Closes #317 (together with #328 and #329, which carry tranches A and B).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Codex review P2: with no prerelease entries the jq filter emitted the
literal "null", which passed the empty-string guard and composed a
release URL containing "null". The filter now appends `// empty` and
the guard also rejects a literal "null".

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@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: 7a42f4120f

ℹ️ 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/README.md
Codex review P2: the README promised stop-with-remediation on missing
jq, but no loaded surface carried the check. tracker-seam.md (read at
every skill invocation's start) now requires the jq presence check
alongside the seam presence check, and setup's schedule path carries
its own gate for the one jq use that bypasses the seam reference.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@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: 2f147c50dc

ℹ️ 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/source-control/README.md
Codex review P2: the jq requirement lived only in the README, which is
not loaded at invocation; the merge/readiness references pipe gh api
output through jq directly. The skill's self-contained runtime line now
names jq and requires the command -v check with stop-with-remediation
before the first parsing phase.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton merged commit f3791e4 into main Jul 18, 2026
19 of 20 checks passed
@kyle-sexton
kyle-sexton deleted the feat/prereq-declarations-skills branch July 18, 2026 07:47
@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown

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


PR Review — feat/prereq-declarations-skills

  • Gather context and understand the request
  • Fetch PR diff and review changed files
  • Check correctness: logic, guards, fallback paths
  • Check security: credential handling, URL composition, injection risks
  • Check alignment with CLAUDE.md/plugin philosophy

Summary

This tranche correctly applies the three-tier prerequisite doctrine across all five plugins. The Codex review findings were all addressed before merge. A few residual observations worth noting.


✅ What's correct

Prerequisite classification is doctrinally consistent. All five plugins correctly distinguish required-for-correctness (stop + remediation), optional (visible warn + skip), and not-applicable (quiet). The classification placement — in README Requirements sections and at point-of-use instruction gates — follows the layered convention correctly.

Gate placement is load-path-correct:

  • tracker-seam.md is genuinely loaded at every work-items skill invocation (the document header says so), so the jq + seam-script gate there covers all five work-items skills without duplication.
  • ci-log-auditor.md's gh auth status is Step 0 in the agent's "Before auditing" section — correct position before any fetch.
  • source-control/skills/pull-request/SKILL.md's jq gate is in the self-contained runtime paragraph, which is read at invocation start.
  • work-items/skills/setup/SKILL.md adds its own jq gate for the one schedule-path jq use that happens outside the seam reference — a necessary supplement since the seam gate doesn't cover setup's direct writes.

kindle-dedrm fallback logic is sound after the multi-round Codex fixes:

  • // empty appended to the jq filter + "null" string guard in the if-check together correctly handle three failure modes: gh absent, gh returns an empty list, gh returns a prerelease-free list (jq yields literal "null").
  • The PINNED_TAG_FROM_VERSIONS_MD placeholder guard works correctly for an agent-executed instruction file: the agent is instructed to substitute the real tag before running; if it forgets, the inner [[ "${LATEST_TAG}" == "PINNED_TAG_FROM_VERSIONS_MD" ]] check fires and exits 1 with the remediation. The echo on line 47 ("using pinned DeDRM tag…") is the success path — reached only when substitution was made, confirming graceful degrade.
  • Key_Finder URL guard now exit 1 with remediation before sha256sum and unzip steps can run with no artifact. ✓

Security surfaces are clean:

  • ZIP_URL is extracted via a tightly-scoped grep -oE regex (https://techy-notes\.com/content/files/[0-9]+/[0-9]+/Kindle_Key_Finder_[0-9.]+\.JH\.zip) — only digits, dots, and slashes can appear in the interpolated URL; no injection surface.
  • LATEST_TAG comes from gh api over an authenticated channel and is checked for empty/null before URL composition. The only concern with tag content would be a rogue release tag containing URL metacharacters, but that is a trust boundary the user already accepts by pinning to the Satsuoni repo.
  • The gh auth status check in ci-log-auditor stops before any gh api calls, preventing partial evidence from an unauthenticated context from being presented as a complete audit.

No CLAUDE.md violations: no hardcoded paths, no PII, no ../ reach-outs, all state in ${CLAUDE_PLUGIN_ROOT} / ${CLAUDE_PLUGIN_DATA}, versions bumped.


⚠️ Observations (non-blocking, PR is merged)

1. PR description version table is incorrect for source-control.

The PR body states source-control | 0.7.1 but the actual bump was 0.8.0 → 0.8.1 (confirmed in plugin.json and CHANGELOG.md). The code is correct; the PR description table had a stale version number. No action needed in the codebase.

2. markdownlint-cli2 prerequisite gate is structurally buried in compress/SKILL.md.

The gate instruction says "STOP at the entry point" but the language lives in the "Hard rules" section (SKILL.md line 74), after the "Backend selection" section which describes step A (detect caveman) and step B (invoke backend). An LLM executing the SKILL.md sequentially could reach step A's detect-caveman.sh before encountering the hard rule. Since SKILL.md is always loaded in full into the model's context this is low-risk in practice, but a pre-flight section at the top (mirroring how tracker-seam.md and ci-log-auditor.md structure their entry gates) would make this structurally unambiguous. Not a regression from this PR — the language was already there; this PR only strengthened the wording.

3. work-items/skills/setup/SKILL.md jq gate position.

The gate is in the "Purpose" paragraph (line 18), before the task steps — correct placement. However, the instruction is embedded mid-paragraph rather than as a bullet or numbered pre-flight step. This is stylistically inconsistent with the seam's explicit two-bullet entry-point gate format. Low impact; mentioned for pattern consistency.


Verdict

The behavioral changes are correct (kindle-dedrm fallback, work-items seam jq+presence gate, ci-log-auditor gh gate, pull-request jq gate, docs-hygiene compress markdownlint classification). All three rounds of Codex fixes addressed real defects and each fix is sound. No blocking issues found.

@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown

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


PR Review — feat/prereq-declarations-skills

  • Gather context and understand the request
  • Fetch PR diff and review changed files
  • Check correctness: prerequisite gates, fallback paths, guard logic
  • Check security: URL composition, credential handling, injection surfaces
  • Check alignment with CLAUDE.md / plugin philosophy

Summary

This tranche correctly applies the three-tier prerequisite doctrine (required-for-correctness → stop + remediation; optional → visible warn + skip; not-applicable → quiet) across all five plugins. All four rounds of Codex fixes addressed real defects and each fix is sound. No blocking issues.


✅ Correctness

kindle-dedrm fallback logic (workflow.md:40–48) is correct after the three rounds of iterative Codex fixes:

  • // empty on the jq filter handles the no-prerelease-entry case — produces an empty string rather than null, which the [[ -z ]] guard catches.
  • || "${LATEST_TAG}" == "null" catches the literal "null" string jq would emit on a fully populated but prerelease-free release list.
  • The PINNED_TAG_FROM_VERSIONS_MD placeholder guard (inner if on line 43) correctly enforces agent-substitution before execution. The design is intentional: the instruction comment (lines 36–39) tells the agent to substitute the real tag before running; if forgotten, the inner guard fires and exits 1; if substituted, the == "PINNED_TAG_FROM_VERSIONS_MD" check is false and the success echo on line 47 runs. This is a sound pattern for agent-executed instruction files.

Key_Finder URL empty-match guard (workflow.md:58–61): exit 1 with mirror-procedure remediation before sha256sum and unzip steps. ✓

tracker-seam.md entry gates (lines 24–31): jq + seam-script presence checks are in the "Provider-neutral over the seam" section which the document header says every work-items skill reads at invocation start. The remediation for the missing seam correctly no longer points at /work-items:setup — it now states the seam is consumer-provisioned and that setup does not create it. ✓

ci-log-auditor.md Step 0 (line 14–17): gh auth status runs before any gh api calls — prevents partial evidence from an unauthenticated context being presented as a complete audit. ✓

pull-request/SKILL.md jq gate (line 27): The "Adapting to your environment" paragraph now reads: "jq is required for correctness (the merge and readiness paths pipe gh api output through it): check command -v jq before the first phase that parses — missing, stop with the install remediation…" — this is a prescriptive instruction rather than an imperative pre-flight block, but since the full SKILL.md is always loaded, the intent is clear and functionally equivalent.

work-items/skills/setup/SKILL.md jq gate (line 19): placed in the "Purpose" paragraph before any task steps — correctly covers setup's direct jq use on the schedule path, which the seam gate does not cover. ✓


✅ Security

ZIP_URL extraction via grep -oE 'https://techy-notes\.com/content/files/[0-9]+/[0-9]+/Kindle_Key_Finder_[0-9.]+\.JH\.zip' — only digits, dots, and slashes can appear in the captured URL. No injection surface.

LATEST_TAG from gh api: 2>/dev/null || true suppresses errors without silently passing bad values — the [[ -z ]] / == "null" guards below handle every failure mode before URL composition. The only residual risk (a rogue prerelease tag with URL metacharacters on the Satsuoni repo) is inside a trust boundary the user already accepts by pinning to that source.

gh auth status stop-before-fetch in ci-log-auditor prevents an unauthenticated gh api call from returning partial or misleading data. ✓


✅ CLAUDE.md alignment

  • No hardcoded paths, no PII, no ../ reach-outs. All state under ${CLAUDE_PLUGIN_ROOT} / ${CLAUDE_PLUGIN_DATA}.
  • All five plugins have explicit semver bumps in plugin.json. ✓
  • Remediation URLs point to official install/download pages, not consumer-specific values. ✓

⚠️ Observations (non-blocking, PR is merged)

1. compress/SKILL.md gate position. The markdownlint-cli2 STOP instruction is in the "Hard rules" section (line 74), which appears after the "Backend selection" section that already defines Steps A and B (detect caveman, invoke backend). Sequentially, an agent could encounter Step A before the hard rule. Since the full SKILL.md loads into context this is low-risk in practice, but a dedicated pre-flight section at the top (mirroring how tracker-seam.md and ci-log-auditor.md structure their entry gates) would remove the ambiguity. Not a regression — the language was already there; this PR strengthened the wording.

2. PR description version table. The body states source-control | 0.7.1; the actual bump recorded in plugin.json and CHANGELOG.md is 0.8.1. Code is correct; the PR body had a stale value.

3. setup/SKILL.md and pull-request/SKILL.md gate style. Both gates are embedded mid-paragraph rather than as explicit pre-flight bullets — minor stylistic inconsistency with the two-bullet gate format in tracker-seam.md. Low impact; noted for pattern consistency in future tranches.


Verdict

All behavioral changes are correct. The kindle-dedrm three-layer fallback, the work-items seam jq+presence gate, the ci-log-auditor gh gate, the pull-request jq gate, and the docs-hygiene markdownlint-cli2 classification are all sound. Every Codex fix round addressed a real defect. No blocking issues.

@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown

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


PR Review — feat/prereq-declarations-skills

  • Gather context and understand the request
  • Fetch PR diff and enumerate changed files
  • Review correctness: prerequisite gates, fallback logic, guard paths
  • Review security: URL composition, credential handling, injection surfaces
  • Check CLAUDE.md alignment: repo-agnostic, configurable, versioned, no PII

Summary

This tranche correctly applies the three-tier prerequisite doctrine (required-for-correctness → stop + remediation; optional → visible warn + skip; not-applicable → quiet) across all five plugins. All four rounds of Codex fixes addressed real defects and each fix is sound. Two minor structural observations carried over from prior reviews; no blocking issues.


✅ Correctness

kindle-dedrm fallback logic (workflow.md:40–48) correctly handles all three failure modes after the four rounds of Codex fixes:

  • // empty on the jq filter makes a prerelease-free release list produce an empty string (not literal null) so the [[ -z ]] guard catches it.
  • || "${LATEST_TAG}" == "null" catches the literal "null" string jq emits when the array is populated but contains no prerelease entries.
  • The PINNED_TAG_FROM_VERSIONS_MD placeholder guard (inner if on line 43) is an agent-substitution enforcement pattern: the comment on lines 36–39 instructs the executing agent to substitute the real tag before running; if substitution was skipped, the inner guard fires (exit 1); if it was made, LATEST_TAG no longer equals the placeholder string and the success echo on line 47 runs. The comment makes this intent explicit — sound pattern for instruction-file execution.

Key_Finder URL empty-match guard (workflow.md:58–61): exit 1 with mirror-procedure remediation before sha256sum/unzip. The regex is correctly tightly scoped. ✓

tracker-seam.md entry gates (lines 24–31): Both jq and seam-script presence checks are two explicit bullets in the "Provider-neutral over the seam" section, which the document header states is read at every work-items skill invocation start. The seam missing-remediation correctly no longer points at /work-items:setup — it now states the seam is consumer-provisioned and that setup does not create it. ✓

ci-log-auditor.md Step 0 (lines 14–17): gh auth status runs as the first step before any gh api calls in the "Before auditing" section, preventing partial evidence from an unauthenticated context. ✓

pull-request/SKILL.md jq gate (line 27): The "Adapting to your environment" paragraph declares jq required-for-correctness with an explicit command -v jq check and install remediation (jqlang.org/download). Since the full SKILL.md loads at invocation, the instruction is read before any parsing phase. ✓

work-items/skills/setup/SKILL.md jq gate (line 19): Placed in the "Purpose" paragraph before any task steps — correctly covers setup's direct jq use on the schedule path, which the seam gate does not cover (setup does not route through the seam). ✓

compress/SKILL.md markdownlint-cli2 gate (line 74): Language in the "Hard rules" section now reads "STOP at the entry point before compressing anything and surface the remediation" — the classification change from optional to required-for-correctness is reflected in the wording.


✅ Security

ZIP_URL extraction via grep -oE 'https://techy-notes\.com/content/files/[0-9]+/[0-9]+/Kindle_Key_Finder_[0-9.]+\.JH\.zip' — only digits, dots, and forward-slashes can appear in the matched URL; the fixed-hostname prefix prevents open-redirect composition. No injection surface.

LATEST_TAG composition: 2>/dev/null || true suppresses errors without silently passing bad values — the [[ -z ]] / == "null" guards handle every failure mode before LATEST_TAG is interpolated into the download URL. The residual risk (a rogue prerelease tag with URL metacharacters on the Satsuoni repo) falls inside a trust boundary the user already accepts by pinning to that source.

gh auth status stop-before-fetch in ci-log-auditor prevents an unauthenticated gh api call from returning partial or misleading data and being presented as a complete audit. ✓


✅ CLAUDE.md / Plugin philosophy alignment

  • No hardcoded paths, no PII, no ../ reach-outs. ✓
  • All state references use ${CLAUDE_PLUGIN_ROOT} / ${CLAUDE_PLUGIN_DATA}. ✓
  • All five plugins have explicit semver bumps in plugin.json: docs-hygiene 0.7.0, kindle-dedrm 0.3.0, source-control 0.8.1, work-items 0.10.0, review 0.13.0. ✓
  • Remediation URLs point to official install/download pages, not consumer-specific values. ✓

⚠️ Observations (non-blocking — PR is merged)

1. compress/SKILL.md gate position.

The markdownlint-cli2 STOP instruction lives in the "Hard rules" section (line 74), which appears after the "Backend selection" section (lines 28–49) that already defines Steps A and B (detect caveman, invoke backend). Sequentially, an agent could encounter Steps A and B before reading the hard rule. Since the full SKILL.md loads into context this is low-risk in practice — an agent will see the rule before acting — but a dedicated "Pre-flight" section at the top (mirroring ci-log-auditor.md's "Before auditing" or tracker-seam.md's two-bullet entry gate) would make this structurally unambiguous. Not a regression: the STOP language was already there; this PR strengthened the wording.

2. PR description version table.

The PR body states source-control | 0.7.1; the actual bump in plugin.json and CHANGELOG.md is 0.8.1 (the plugin was already at 0.8.0 before this PR). Code is correct; PR description had a stale value.

3. setup/SKILL.md gate style.

The jq gate is embedded mid-paragraph in the Purpose section (line 19) rather than as an explicit pre-flight bullet — minor stylistic inconsistency with the two-bullet format in tracker-seam.md. Low impact; noted for pattern consistency in future tranches.


Verdict

All behavioral changes are correct. The kindle-dedrm three-layer fallback, the work-items seam jq + presence gate, the ci-log-auditor gh gate, the pull-request jq gate, and the docs-hygiene markdownlint-cli2 reclassification are all sound. Every Codex fix round addressed a real defect. No blocking issues found.

@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown

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


PR Review — feat/prereq-declarations-skills

  • Gather context and understand the request
  • Fetch PR diff and review changed files
  • Check correctness: prerequisite gates, fallback paths, guard logic
  • Check security: URL composition, credential handling, injection surfaces
  • Check CLAUDE.md alignment: repo-agnostic, configurable, versioned, no PII
  • Synthesize findings and post verdict

Summary

This tranche applies the three-tier prerequisite doctrine (required-for-correctness → stop + remediation; optional → visible warn + skip; not-applicable → quiet) across five plugins. All four Codex fix rounds addressed real defects and each fix is sound. Three structural observations from earlier review rounds are re-confirmed as non-blocking.


✅ Correctness

kindle-dedrm fallback logic (workflow.md:40–48) correctly handles all three failure modes after the iterative Codex fixes:

  • // empty on the jq filter makes a prerelease-free release list produce an empty string rather than literal null, so the [[ -z "${LATEST_TAG}" ]] guard catches it.
  • || "${LATEST_TAG}" == "null" is a belt-and-suspenders catch for the literal "null" string.
  • The PINNED_TAG_FROM_VERSIONS_MD placeholder guard (inner if on line 43) is an agent-substitution enforcement pattern: the comment block (lines 35–39) instructs the executing agent to substitute the real tag before running; if substitution was skipped, "${LATEST_TAG}" == "PINNED_TAG_FROM_VERSIONS_MD" fires and exits 1; if it was made (e.g. LATEST_TAG="v10.0.20"), the inner guard is false and the success echo fires. This is a sound pattern for agent-executed instruction files.

Key_Finder URL empty-match guard (workflow.md:58–61): exit 1 with mirror-procedure remediation before sha256sum/unzip steps. The grep -oE regex is tightly scoped (only digits, dots, slashes, fixed hostname prefix) — no injection surface. ✓

tracker-seam.md entry gates (lines 24–31): Both jq and seam-script presence checks are two explicit bullets in the "Provider-neutral over the seam" section, which the document header states is read at every work-items skill invocation start. The seam missing-remediation correctly no longer points at /work-items:setup — it now states the seam is consumer-provisioned and that setup does not create it. ✓

ci-log-auditor.md Step 0 (lines 14–17): gh auth status runs as Step 0, before any gh api calls in the "Before auditing" section. Prevents partial evidence from an unauthenticated context being presented as a complete audit. ✓

pull-request/SKILL.md jq gate (line 27): The "Adapting to your environment" paragraph names jq required-for-correctness with an explicit command -v jq check and install remediation. Since the full SKILL.md loads into context at invocation, this instruction is read before any parsing phase. ✓

work-items/skills/setup/SKILL.md jq gate (line 19): Placed in the Purpose paragraph before any task steps, correctly covering setup's direct jq use on the schedule path — which routes outside the seam and is therefore not covered by tracker-seam.md's entry gate. ✓

compress/SKILL.md markdownlint-cli2 gate (line 74): Language in "Hard rules" now reads "STOP at the entry point before compressing anything" — the reclassification from lint-failure to required-for-correctness is reflected. See observation 1 below for the structural note.


✅ Security

ZIP_URL extraction: grep -oE 'https://techy-notes\.com/content/files/[0-9]+/[0-9]+/Kindle_Key_Finder_[0-9.]+\.JH\.zip' — fixed hostname prefix + digit/dot/slash character class only. No injection surface.

LATEST_TAG composition: 2>/dev/null || true suppresses errors without passing bad values; [[ -z ]] || == "null" guards handle every failure mode before the tag is interpolated into the download URL. The residual risk (a rogue prerelease tag with URL metacharacters on the Satsuoni repo) is inside a trust boundary the user already accepts by pinning to that source.

gh auth status stop-before-fetch in ci-log-auditor: prevents an unauthenticated gh api call's partial data from being presented as a complete audit. ✓


✅ CLAUDE.md / plugin philosophy

  • No hardcoded paths, no PII, no ../ reach-outs. ✓
  • State references use ${CLAUDE_PLUGIN_ROOT} / ${CLAUDE_PLUGIN_DATA}. ✓
  • Confirmed version bumps: docs-hygiene 0.7.0, kindle-dedrm 0.3.0, source-control 0.8.1, work-items 0.10.0, review 0.13.0. ✓
  • Remediation URLs point to official install/download pages, not consumer-specific values. ✓

⚠️ Observations (non-blocking — PR is merged)

1. compress/SKILL.md gate position.

The markdownlint-cli2 STOP instruction lives in the "Hard rules" section (line 74), which appears after the "Backend selection" section (lines 28–48) that already defines Steps A and B (detect caveman, invoke backend). An agent reading sequentially could encounter Steps A and B before the hard rule. Since the full SKILL.md loads into context this is low-risk in practice — the agent will see the rule before acting — but a dedicated pre-flight section at the top (mirroring ci-log-auditor.md's "Before auditing" block or tracker-seam.md's two-bullet entry gate) would make this structurally unambiguous. Not a regression from this PR — the STOP language was already there; this tranche strengthened the wording.

Fix this →

2. PR description version table stale for source-control.

The PR body states source-control | 0.7.1; the actual bump recorded in plugin.json is 0.8.1 (the plugin was already at 0.8.0 before this tranche). Code is correct; the PR description had a stale value. No action needed in the codebase.

3. setup/SKILL.md jq gate style.

The jq gate is embedded mid-paragraph in the Purpose section (line 19) rather than as an explicit pre-flight bullet — minor stylistic inconsistency with the two-bullet format in tracker-seam.md. Low impact; noted for pattern consistency in tranche C.


Verdict

All behavioral changes are correct. The kindle-dedrm three-layer fallback, the work-items seam jq + presence gate, the ci-log-auditor gh gate, the pull-request jq gate, and the docs-hygiene markdownlint-cli2 reclassification are all sound. Every Codex fix round addressed a real defect. No blocking issues found.

@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown

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


PR Review — feat/prereq-declarations-skills

  • Gather context and understand the request
  • Fetch PR diff and review changed files
  • Check correctness: prerequisite gates, fallback paths, guard logic
  • Check security: URL composition, credential handling, injection surfaces
  • Check CLAUDE.md alignment: repo-agnostic, configurable, versioned, no PII
  • Synthesize findings and post verdict

Summary

This tranche applies the three-tier prerequisite doctrine (required-for-correctness → stop + remediation; optional → visible warn + skip; not-applicable → quiet) across five plugins. All four Codex fix rounds addressed real defects and each fix is sound. Three structural observations carry over; two are worth addressing in a follow-up.


✅ Correctness

kindle-dedrm fallback logic (workflow.md:40–48) handles all three failure modes correctly after the iterative fixes:

  • // empty on the jq filter → prerelease-free release list produces "", caught by [[ -z ]].
  • || "${LATEST_TAG}" == "null" → belt-and-suspenders for the literal "null" string jq would otherwise emit.
  • PINNED_TAG_FROM_VERSIONS_MD inner guard (line 43): agent-substitution enforcement — the comment block (lines 35–39) instructs the executing agent to substitute the real tag; if forgotten, the inner == "PINNED_TAG_FROM_VERSIONS_MD" check fires and exits 1; if substituted correctly, the guard is false and the success echo fires. Sound pattern for instruction-file execution.

Key_Finder URL empty-match guard (workflow.md:58–61): exit 1 with mirror-procedure remediation before sha256sum/unzip steps can run with no artifact. ✓

tracker-seam.md entry gates (lines 24–31): Two explicit bullets — jq presence and seam-script presence — in the "Provider-neutral over the seam" section, which the document header states is read at every work-items skill invocation start. The seam missing-remediation correctly states the seam is consumer-provisioned (not created by /work-items:setup). ✓

ci-log-auditor.md Step 0 (lines 14–17): gh auth status as Step 0 — before any gh api calls in "Before auditing". Prevents partial evidence from an unauthenticated context being presented as a complete audit. ✓

pull-request/SKILL.md jq gate (line 27): "Adapting to your environment" paragraph declares jq required-for-correctness with command -v jq and install remediation. Since the full SKILL.md loads at invocation, this instruction is visible before any parsing phase. ✓

work-items/skills/setup/SKILL.md jq gate (line 19): In the Purpose paragraph before any task steps, correctly covering setup's direct jq use on the schedule path — which routes outside the seam and is not covered by tracker-seam.md's entry gate. ✓

compress/SKILL.md markdownlint-cli2 gate (line 74): "Hard rules" now reads "STOP at the entry point before compressing anything" — the reclassification from lint-failure to required-for-correctness is reflected. Gate position observation below.


✅ Security

ZIP_URL extraction: grep -oE 'https://techy-notes\.com/content/files/[0-9]+/[0-9]+/Kindle_Key_Finder_[0-9.]+\.JH\.zip' — fixed hostname prefix, character class restricted to digits/dots/forward-slashes. No injection surface.

LATEST_TAG composition: 2>/dev/null || true suppresses errors without passing bad values; [[ -z ]] || == "null" guards handle every failure mode before interpolation. Residual risk (a rogue prerelease tag with URL metacharacters on the Satsuoni repo) falls inside the trust boundary the user already accepts by pinning to that source.

gh auth status stop-before-fetch in ci-log-auditor: prevents an unauthenticated gh api call's partial or misleading data from being presented as a complete audit. ✓


✅ CLAUDE.md / plugin philosophy

  • No hardcoded paths, no PII, no ../ reach-outs. ✓
  • State references use ${CLAUDE_PLUGIN_ROOT} / ${CLAUDE_PLUGIN_DATA}. ✓
  • Confirmed version bumps: docs-hygiene 0.7.0, kindle-dedrm 0.3.0, source-control 0.8.1, work-items 0.10.0, review 0.13.0. ✓
  • Remediation URLs point to official install/download pages. ✓

⚠️ Observations (non-blocking — PR is merged)

1. compress/SKILL.md gate position (carried from prior reviews — worth a follow-up fix)

The markdownlint-cli2 STOP instruction lives in "Hard rules" (line 74), which appears after the "Backend selection" section (lines 23–49) that defines Steps A and B. An agent reading sequentially could encounter Steps A and B before the hard rule. Since the full SKILL.md loads into context this is low-risk in practice, but a dedicated "Pre-flight" section at the top (mirroring ci-log-auditor.md's "Before auditing" block or tracker-seam.md's two-bullet entry gate) would make this structurally unambiguous.

Fix this →

2. PR description version table stale for source-control

The PR body states source-control | 0.7.1; the actual bump in plugin.json is 0.8.1 (the plugin was already at 0.8.0 before this tranche). Code is correct; PR description had a stale value. No codebase action needed.

3. setup/SKILL.md jq gate style (low impact)

The jq gate is embedded mid-paragraph in the Purpose section (line 19) rather than as an explicit pre-flight bullet — minor stylistic inconsistency with the two-bullet gate format in tracker-seam.md. Noted for pattern consistency in tranche C.


Verdict

All behavioral changes are correct. The kindle-dedrm three-layer fallback, the work-items seam jq + presence gate, the ci-log-auditor gh gate, the pull-request jq gate, and the docs-hygiene markdownlint-cli2 reclassification are all sound. Every Codex fix round addressed a real defect. No blocking issues found.

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.

1 participant