Skip to content

feat: execute issue #1996's accepted Opus 5 design decisions (I23 row, alias-abort UX, chapter discoverability) - #2017

Merged
kyle-sexton merged 3 commits into
mainfrom
feat/opus5-1996-decisions
Aug 8, 2026
Merged

feat: execute issue #1996's accepted Opus 5 design decisions (I23 row, alias-abort UX, chapter discoverability)#2017
kyle-sexton merged 3 commits into
mainfrom
feat/opus5-1996-decisions

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Closes #1996

Summary

Executes the interview outcomes over #1996's six deferred Opus 5 design candidates: decisions (b) effort-misconception audit row, (d) opus-5 chapter discoverability, and (e) family-alias abort UX are accepted and shipped here; (a) generic prior-model-workaround row and (c) vision-workaround row stay deferred-with-trigger; (f) the additive chapter-payload omissions are ratified as decided-not-missed. All six outcomes are recorded in a comment on the issue.

Fix

  • claude-config 0.25.0 (criteria 1.18.0 → 1.19.0):
    • New catalog row I27 — effort lowered to shorten the response (renumbered twice from I23 as main's feat: align the marketplace with the Fable 5 prompting guide #2000 and feat: align repo and audit catalog with the Sonnet 5 prompting guide #2015 landed I23–I26 concurrently), Model scope: opus-5 (both source statements are model-qualified; promotion gate unmet, unscope trigger on the row), with cost/latency-ground, length-instruction-only, audience-test, and config-value fences.
    • New instruction-scan.sh I27 family: effort-lowering directive + brevity token ANDed on one line, word-boundary conventions applied, doubled-consonant dropp? stem per review; header/usage docs updated; 11 I27 regression assertions (57 checks total).
    • Family-alias abort UX: the fail-loud abort on a version-ambiguous target now also names the normalized token the alias currently resolves to as a suggested --target-model value the user confirms — still never proceeds on a guess.
    • Stale evals/evals.json check-range fixed (I6-I16 → I6-I27; --opinion set corrected to I16, I19, I22).
  • playbooks 0.7.1: fable-5 description now names the per-model adaptation chapters (Opus 5, Opus 4.8, Sonnet 5) with Opus 5 trigger phrases — the opus-5 chapter was previously unreachable from the skill listing.

Verification

  • Both guide quotes verified against the LIVE Opus 5 prompting guide this session: raw .md 11,225 bytes, MD5 8579d63fc9f793784b8c56320fd74e71, byte-identical to the 2026-07-25 knowledge-corpus capture; the effort page's Opus 5 section corroborates the claim unhedged. A sibling fresh-context verifier independently re-fetched the page and reproduced every quote and both hashes.
  • instruction-scan.test.sh: 57/57 checks pass.
  • check-changelog-parity.sh --check, --check-bump origin/main, --check-order: all pass after each rebase (branch rebased over feat(claude-config): add the unhobble skill and codify instruction economy #2005, feat: align the marketplace with the Fable 5 prompting guide #2000, feat: align repo and audit catalog with the Sonnet 5 prompting guide #2015; versions re-slotted each time).
  • check-changed-skills.sh origin/main: 2 skills checked, 0 failed; fable-5 description 828 chars (cap 1,536); aggregate listing budget 1,801/8,000.
  • Fresh-context reviewer pass applied: prose-grammar fix in the seeding list, word-boundary consistency on the brevity ERE, brevity-token test-matrix completion, doubled-p stem fix, evals range fix.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01E5qiaLiGqP6fUTHkRYS19R

@cursor

cursor Bot commented Aug 8, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@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: 5f251b3351

ℹ️ 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/claude-config/skills/audit-instructions/scripts/instruction-scan.sh Outdated
@kyle-sexton
kyle-sexton force-pushed the feat/opus5-1996-decisions branch from 5f251b3 to 3ffa4e8 Compare August 8, 2026 19:11
Interview outcomes over the six deferred candidates (decisions b, d, e
accepted and shipped here; a, c stay deferred-with-trigger; f ratified —
all six recorded on the issue):

- claude-config 0.24.0: new I23 catalog row (effort lowered to shorten
  the response; Model scope: opus-5, promotion gate unmet with unscope
  trigger), seeded by a new instruction-scan.sh I23 family with ANDed
  effort+brevity patterns and regression cases (55 checks); family-alias
  abort message now suggests the alias's normalized resolution while
  still refusing to guess; stale evals.json check-range fixed.
- playbooks 0.6.23: fable-5 description names the per-model adaptation
  chapters with Opus 5 trigger phrases so the opus-5 chapter is
  discoverable from the skill listing.

Both guide quotes verified against the live page this session (raw .md
byte-identical to the 2026-07-25 corpus capture; effort-page Opus 5
section corroborates).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E5qiaLiGqP6fUTHkRYS19R
@kyle-sexton
kyle-sexton force-pushed the feat/opus5-1996-decisions branch from 3ffa4e8 to 6237308 Compare August 8, 2026 19:16
@claude

claude Bot commented Aug 8, 2026

Copy link
Copy Markdown

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


Security Review — PR #2017

Tasks

  • Gather context (PR diff, changed files)
  • Read changed files
  • Analyze for security issues
  • Post findings

Findings

No security issues found.

Scope reviewed (per git diff origin/main...HEAD, 10 files / 147+/15-):

  • plugins/claude-config/skills/audit-instructions/scripts/instruction-scan.sh and instruction-scan.test.sh — the only files with executable logic in this PR.
  • plugins/claude-config/skills/audit-instructions/{SKILL.md,reference/criteria.md,evals/evals.json} — prose/doctrine and eval fixtures for the new I27 catalog row and the alias-abort UX change.
  • Both plugins' .claude-plugin/plugin.json (version bumps only) and CHANGELOG.md.
  • plugins/playbooks/skills/fable-5/SKILL.md (description text only).

What I looked for and ruled out:

  • Injection surfaces in instruction-scan.sh: the new I27 detection (I27_EFFORT_ERE / I27_BREVITY_ERE) follows the exact pattern of the pre-existing I6/I8/I10 detectors — static ERE strings built from script-owned literals, passed to grep -niE "$pattern" "$file" with the file argument properly quoted. File content is only ever piped into grep -qiE, never eval'd or interpolated into a shell command. No new attack surface versus the existing scanner.
    I8_A_ERE="double[- ]check|${WB_L}re[- ]?verif|final verification step|(sub)?agent to verify|have (a |an )?(sub)?agent verify|verifier (sub)?agent|verify your (own )?work"
    I8_B_ERE="be conservative|(only report|report only) (the )?(high|critical)|(don('|’)?t|do not) nitpick"
    I8_C_ERE="(do not|don('|’)?t) (think|reason)|without thinking|skip the reasoning"
    # I27 effort-for-brevity: both patterns must hit the SAME line (ANDed in
    # scan_file). Stem forms (decreas, vebos-free "verbos") catch inflections;
    # over-production is the contract, as with I8.
    I27_EFFORT_ERE="(lower|reduc|decreas|dropp?)(e|ed|ing)? (the |your )?effort"
    I27_BREVITY_ERE="${WB_L}short|${WB_L}brief|${WB_L}concise|${WB_L}terse|${WB_L}length|verbos|${WB_L}wordy"
    rows=()
    scan_file() {
    local file="$1" hit lineno text
    [[ -f "$file" ]] || return 0
    while IFS= read -r hit; do
    [[ -n "$hit" ]] || continue
    lineno="${hit%%:*}"
    text="${hit#*:}"
    printf '%s\n' "$text" | grep -qiE "$RATIONALE_ERE" && continue
    rows+=("$file:$lineno:I6")
    done < <(grep -niE "$I6_ERE" "$file" 2>/dev/null)
    while IFS= read -r hit; do
    [[ -n "$hit" ]] || continue
    lineno="${hit%%:*}"
    rows+=("$file:$lineno:I10")
    done < <(grep -niE "$I10_ERE" "$file" 2>/dev/null)
    local fam ere
    for fam in a b c; do
    case "$fam" in
    a) ere="$I8_A_ERE" ;;
    b) ere="$I8_B_ERE" ;;
    c) ere="$I8_C_ERE" ;;
    *) continue ;;
    esac
    while IFS= read -r hit; do
    [[ -n "$hit" ]] || continue
    lineno="${hit%%:*}"
    rows+=("$file:$lineno:I8-$fam")
    done < <(grep -niE "$ere" "$file" 2>/dev/null)
    done
    while IFS= read -r hit; do
    [[ -n "$hit" ]] || continue
    lineno="${hit%%:*}"
  • Path handling: scan_file() still just does [[ -f "$file" ]] on caller-supplied paths with no traversal-relevant logic changed in this PR; no new path-construction code was added.
  • Alias-abort UX change (SKILL.md, decision e): this only changes what text a fail-loud abort message suggests to the user — it explicitly still never proceeds without user confirmation ("Suggesting is not guessing"). It's prose describing agent behavior, not code that resolves or fetches anything at scan-script layer; no credential, token, or auth-boundary handling is touched.
  • Secrets/credentials: none introduced or referenced anywhere in the diff.
  • GitHub Actions: no workflow files are touched by this PR (confirmed via the changed-files list and git diff --stat), so there's nothing in scope for pull_request_target/workflow_run, permissions widening, or script-injection-via-github-context concerns.
  • Supply chain: no dependency or action pin changes in this diff (defers to zizmor's static lane in any case per instructions).

The rest of the diff (criteria.md, evals.json, CHANGELOG.md, plugin.json version bumps, fable-5/SKILL.md description) is documentation/config content with no code-execution or trust-boundary implications.

@claude

claude Bot commented Aug 8, 2026

Copy link
Copy Markdown

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


Code review — PR #2017

Scope: Reviewed all 10 changed files via git diff origin/main...HEAD. This is the code-review lane; per REVIEW.md the security lane owns vulnerability/authz/secrets/injection findings (none applicable here — no security-relevant surface in this diff), so this pass covers correctness, design, tests, and maintainability only.

What the diff does

  • Adds catalog row I27 (effort lowered to shorten the response, Model scope: opus-5) to audit-instructions, seeded by a new instruction-scan.sh I27 family, with matching test cases, SKILL.md/evals.json cross-references, and a version bump (claude-config 0.24.0 → 0.25.0, criteria 1.18.0 → 1.19.0).
  • Adds the family-alias abort-UX enhancement (suggests the resolved token on a version-ambiguous target, never guesses).
  • Fixes a stale I6-I16 check-range and --opinion set in evals.json.
  • Makes the fable-5 chapter set discoverable from the skill listing (playbooks 0.7.0 → 0.7.1).

Note: the PR title/body still say I23 throughout, but the shipped code uses I27 — per the review thread this row renumbered twice after main picked up I23 (context-budget row, #2000) and then I24 during this PR's own review. The code, CHANGELOG.md, criteria.md, SKILL.md, and evals.json are all internally consistent on I27 — only the PR description text is stale. Not a code defect, just worth knowing when reading the PR history later.

Verification performed independently (not just re-stating the author's claims)

  • Manually traced the I27_EFFORT_ERE / I27_BREVITY_ERE regexes in instruction-scan.sh against every fixture line in instruction-scan.test.sh — all 11 lines behave as asserted, including the doubled-p fix for dropped/dropping that addresses the earlier Codex review comment on this PR (dropp? correctly matches the real double-p English spellings; the previously-flagged droping/droped typo forms were never the actual bug — the original bug was the correct spellings not matching).
  • Confirmed via Grep that I16, I19, and I22 are the only three OPINION-tier rows in criteria.md — matches the corrected evals.json claim.
  • Confirmed Surfaces: all / I6–I12 and I15–I27 apply to all surfaces line in criteria.md is self-consistent with the new row.
  • Validated all three changed/touched JSON files (claude-config/plugin.json, playbooks/plugin.json, evals.json) parse with jq empty — all valid.
  • Spot-checked the fable-5 description line byte length (850 bytes including the description: "..." wrapper) — consistent with the changelog's "well under 1,536" and roughly consistent with the PR's claimed 828-char count (the small delta is explained by multi-byte UTF-8 em-dashes; byte count vs. char count).
  • Could not run instruction-scan.test.sh or check-changelog-parity.sh myself — script execution requires interactive approval not available to this review lane. The PR's "55/55 checks pass" and the parity/changed-skills script output are therefore author-claimed and unverified by me; my regex trace above is independent corroboration but not a substitute for actually running the suite.

Findings (posted inline, both 🟡 Suggestion-level, non-blocking)

  1. instruction-scan.sh:115 — comment typo "vebos-free" and unclear rationale for why verbos alone skips the left word-boundary that every other brevity token has.
  2. instruction-scan.test.sh:192 — fixture path i24.md is a leftover from the row's earlier I24 numbering; everything else in the test case already says I27.

Both are cosmetic/maintainability nits from the mid-review renumbering churn, not functional bugs — the detection logic itself is correct against its own test fixtures. No architecture, error-handling, or test-coverage concerns beyond these two nits. criteria.md, SKILL.md, and evals.json are all mutually consistent, the abort-UX prose change is internally coherent with the existing never-guess contract, and the version/changelog bumps follow this repo's established format.
· Branch: feat/opus5-1996-decisions

Comment thread plugins/claude-config/skills/audit-instructions/scripts/instruction-scan.sh Outdated
Comment thread plugins/claude-config/skills/audit-instructions/scripts/instruction-scan.test.sh Outdated
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

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

…he I27 scanner

The spell-check gate flagged the "ANDed" shorthand and the bare decrease
stem in the I27 comments: reworded both comments in full words, and the
one flagged token that must stay — the stem inside the pattern string —
now carries the config-blessed spellchecker:disable-line pragma. Also
renames the I27 test fixture from its stale i24.md name.

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

claude Bot commented Aug 8, 2026

Copy link
Copy Markdown

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


Security Review — PR #2017 (HEAD 16da9f8)

  • Gather context (PR diff, 2f71a99...HEAD, matches the PR's stated 10 files / 148+/15-)
  • Read changed files
  • Analyze for security issues (injection, auth, secrets, GH Actions)
  • Post findings

No security issues found.

Scope reviewed (git diff 2f71a99...HEAD, 10 files):

  • plugins/claude-config/skills/audit-instructions/scripts/instruction-scan.sh and instruction-scan.test.sh — the only files with executable logic in this PR.
  • plugins/claude-config/skills/audit-instructions/{SKILL.md,reference/criteria.md,evals/evals.json} — prose/doctrine and eval fixtures for the new I27 catalog row and the alias-abort UX change.
  • Both plugins' .claude-plugin/plugin.json (version bumps only) and CHANGELOG.md.
  • plugins/playbooks/skills/fable-5/SKILL.md (description text only).

This synchronize event added one commit (16da9f8) on top of what was already reviewed at 6237308. That commit only touches cosmetic review nits — a comment typo fix and a test-fixture filename rename (i24.mdi27.md) in instruction-scan.test.sh — no logic or trust-boundary changes. Re-confirmed the full diff end to end rather than relying on that prior pass:

  • instruction-scan.sh I27 detection (I27_EFFORT_ERE/I27_BREVITY_ERE, L113-117, scan loop, L154-161): static ERE strings built from script-owned literals, passed to grep -niE "$pattern" "$file" with the file argument quoted throughout; matched text is only ever re-piped into grep -qiE, never eval'd or interpolated into a shell command. Same pattern as the pre-existing I6/I8/I10 detectors — no new attack surface.
  • Path handling: unchanged from the base — [[ -f "$file" ]] on caller-supplied paths, no new path-construction logic.
  • Alias-abort UX change (SKILL.md L106-116): prose-only change to what an abort message suggests; explicitly still "never a value the run proceeds on" without user confirmation. No code at scan-script layer resolves, fetches, or executes anything as a result — no credential/token/auth-boundary surface touched.
  • Secrets/credentials: none introduced or referenced anywhere in the diff.
  • GitHub Actions: no workflow files touched by this PR — nothing in scope for pull_request_target/workflow_run, permissions widening, or script-injection-via-github-context.
  • Supply chain: no dependency or action pin changes in this diff (zizmor's static lane covers this regardless).

The remainder of the diff (criteria.md, evals.json, CHANGELOG.md, plugin.json version bumps, fable-5/SKILL.md description) is documentation/config content with no code-execution or trust-boundary implications.

@claude

claude Bot commented Aug 8, 2026

Copy link
Copy Markdown

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


Code review — PR #2017 (re-review after push to 16da9f8)

  • Gather context (diff vs base 2f71a99, CLAUDE.md/AGENTS.md, prior review threads)
  • Verify the two previously-flagged nits (typo comment, i24.md fixture name) are actually fixed in 16da9f8
  • Re-review changed files for correctness, error handling, test coverage, maintainability
  • Post inline findings
  • Post summary comment

Scope: git diff 2f71a99..HEAD (10 files, +148/-15, matches the PR's stated diffstat). Code-review lane per REVIEW.md/custom instructions — security surface is out of scope here (already covered, cleanly, by the security lane's prior pass).

Verification of the two prior nits (both fixed correctly, 16da9f8)

  1. Typo/rationale comment (instruction-scan.sh:113-116) — "vebos-free" is gone; the comment now correctly explains both the doubled-p dropp? stem and the boundary-less verbos token. The added trailing # spellchecker:disable-line on line 117 matches this repo's own _typos.toml convention exactly (extend-ignore-re pattern ^.*(#|//)\s*spellchecker:disable-line$), so it will actually suppress the typos gate rather than being decorative.
  2. Stale fixture name (instruction-scan.test.sh:192) — i24.mdi27.md, now consistent with the I27F variable name and every I27 check id in the test case.

Independent verification performed

  • Manually traced I27_EFFORT_ERE/I27_BREVITY_ERE against all 11 instruction-scan.test.sh fixture lines (194-204) — all match/no-match as asserted, including the assert_not_contains cases (cost-ground line 4, brevity-alone line 5).
  • Cross-checked the ripgrep-pattern match independently (not just reading the assertions) using the Grep tool against the same fixture lines — confirms the 11 I27 assertions (207-217) are correct as written.
  • jq empty on all three touched/new JSON structures (claude-config/plugin.json, playbooks/plugin.json, evals/evals.json) — all parse.
  • Confirmed no leftover I23/I24 self-references remain anywhere in this PR's new I27 content in criteria.md/SKILL.md/evals.json/scripts (the I23/I24 hits still in criteria.md are the other, pre-existing rows from feat: align the marketplace with the Fable 5 prompting guide #2000/feat: align repo and audit catalog with the Sonnet 5 prompting guide #2015 — correctly untouched).
  • criteria.md's top-of-file surface line (I6–I12 and I15–I27 apply to all surfaces) and SKILL.md's two I6–I26I6–I27 edits are mutually consistent with the new row and with each other.
  • evals.json's corrected OPINION-gated set (I16, I19, I22) matches criteria.md's three Tier OPINION rows — verified by grep, not just accepted from the PR body.
  • Could not run instruction-scan.test.sh or the scanner script directly — execution requires interactive approval unavailable to this review lane. The PR's "57/57 checks pass" claim is therefore author-claimed and unverified by me; the independent regex/ripgrep trace above is corroboration, not a substitute for actually running the suite.

New finding (posted inline, 🟡 non-blocking)

  • instruction-scan.sh:117I27_EFFORT_ERE's suffix group (e|ed|ing)? has no -s alternative, so third-person-singular present-tense phrasing ("reduces effort", "lowers effort", "drops effort", "decreases effort") doesn't match. Verified with an independent ripgrep-pattern check against a scratch fixture (not committed): none of those three phrasings matched, while all 11 existing test lines did. This is the same class of gap as the doubled-p issue Codex caught and you already fixed — just the -s inflection this time. Given the row is explicitly documented as an advisory, over-producing-not-exhaustive pre-scan, this is optional to close now; flagging as a possible follow-up regression case.

Everything else

SKILL.md, criteria.md, evals.json, both CHANGELOG.mds, and both plugin.json version bumps are all internally consistent with each other and with the shipped I27 code — no architecture, error-handling, or test-coverage concerns beyond the one nit above. The family-alias abort-UX change (SKILL.md) and fable-5 discoverability change are prose-only and unchanged since the prior review pass, which already covered them.
· Branch: feat/opus5-1996-decisions

Comment thread plugins/claude-config/skills/audit-instructions/scripts/instruction-scan.sh Outdated
…stem

reduces/lowers/drops/decreases now match via -s/-es in the suffix group,
the same missed-inflection class as the doubled-p fix; three regression
cases added (60 checks).

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

claude Bot commented Aug 8, 2026

Copy link
Copy Markdown

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


Code review — PR #2017 (re-review after push to 9c1dc3f)

  • Gather context (diff vs base 2f71a99, prior review threads)
  • Verify the new commit 9c1dc3f addresses the outstanding third-person-inflection nit
  • Re-review changed files for correctness, error handling, test coverage, maintainability
  • Post inline findings (none warranted — see below)
  • Post summary comment

Scope: git diff 16da9f8..9c1dc3f — this synchronize event added exactly one commit, touching only instruction-scan.sh and instruction-scan.test.sh. All other files (SKILL.md, criteria.md, evals.json, both CHANGELOG.mds, both plugin.jsons, fable-5/SKILL.md) are unchanged since the prior review pass at 16da9f8, which already covered them cleanly.

What this commit does

Closes the last open nit from the prior review round: I27_EFFORT_ERE's suffix group changed from (e|ed|ing)? to (e|ed|es|ing|s)?, adding third-person-singular present-tense coverage (reduces, lowers, drops, decreases). Three matching regression cases were added to instruction-scan.test.sh (lines 12–14 of the i27.md fixture, asserted at instruction-scan.test.sh:212-214).

Independent verification performed

  • Traced the new regex directly with grep -niE against the three new fixture lines — all three match as asserted:
    • "This setting reduces effort to keep answers short."
    • "The flag lowers effort for briefer output."
    • "It drops effort to stay concise."
  • Confirmed the heredoc line numbers in the fixture (i27.md lines 12/13/14) line up exactly with the assert_contains ... "$I27F:12:I27" / :13 / :14 calls added below — no off-by-one.
  • Probed for regressions/false-positives from the new bare s alternative against adjacent non-matching phrasing (e.g. "Lowers the bar for effort.", "The effort was lowered.") — neither matches, consistent with the existing (the |your )?effort anchor immediately following the verb stem.
  • Confirmed no other files changed in this push (SKILL.md, criteria.md, evals.json, changelogs, plugin.jsons) — nothing else to re-review.
  • Could not run instruction-scan.test.sh directly in this environment (script execution requires interactive approval unavailable to this review lane) — the independent grep -niE trace above is corroboration, not a substitute for actually running the full 60-assertion suite.

Notes (non-blocking, informational only — not filed as inline findings)

  • The block comment above the regex (L112-116) still enumerates only the decrease/drop(doubled-p) inflections it was written for before this fix; it doesn't mention the new third-person -s/-es forms. Purely a comment-completeness nit, no functional impact.
  • The PR description's "11 I27 regression assertions (57 checks total)" is now stale by 3 assertions after this commit (would be 60) — informational only, doesn't block anything since it's PR body text, not code.

No functional, correctness, or test-coverage issues found in this commit. The fix is narrowly scoped, correctly closes the exact gap flagged in the prior review round, and its regression cases are consistent with the existing fixture/assertion pattern.

Branch: feat/opus5-1996-decisions

@claude

claude Bot commented Aug 8, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 34s —— View job


Security Review — PR #2017 (HEAD 9c1dc3f)

  • Gather context (diff since last-reviewed 16da9f89c1dc3f)
  • Read changed files
  • Analyze for security issues
  • Post findings

No security issues found.

Delta since the last reviewed commit (16da9f8): one commit, 9c1dc3f, touching only two files:

  • instruction-scan.sh:117I27_EFFORT_ERE's suffix alternation extended from (e|ed|ing)? to (e|ed|es|ing|s)?, closing the third-person-singular inflection gap flagged in the prior review pass (reduces/lowers/drops/decreases).
  • instruction-scan.test.sh — three new regression fixture lines and matching assert_contains checks for the third-person forms.

This is a static ERE literal string, same as the rest of the I27 family already reviewed: passed to grep -niE "$pattern" "$file" with the file argument quoted, matched output only re-piped into grep -qiE, never eval'd or shell-interpolated. Widening the character alternation inside the pattern has no injection, path-traversal, or trust-boundary implications — it only changes which lines the advisory pre-scan flags, not how the match is executed.

Full-PR scope (unchanged from the two prior full passes on this PR, both clean): the only files with executable logic are instruction-scan.sh / instruction-scan.test.sh; everything else (SKILL.md, criteria.md, evals.json, both CHANGELOG.mds, both plugin.json version bumps, fable-5/SKILL.md) is documentation/config content with no code-execution or trust-boundary surface. No workflow files are touched by this PR, so there's nothing in scope for pull_request_target/workflow_run, permissions widening, or script-injection-via-github-context. No secrets, credentials, or dependency/action pin changes anywhere in the diff.
· Branch: feat/opus5-1996-decisions

@kyle-sexton
kyle-sexton merged commit 0a7886e into main Aug 8, 2026
33 checks passed
@kyle-sexton
kyle-sexton deleted the feat/opus5-1996-decisions branch August 8, 2026 20:14
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.

claude-config(audit-instructions): deferred Opus 5 guide candidates — vision-workaround re-validation and late-window defensive-scaffolding rows

1 participant