Skip to content

fix(education): correct changelog entries and restore teach action hint - #5243

Merged
kyle-sexton merged 8 commits into
mainfrom
fix/audit-education
Sep 29, 2026
Merged

kyle-sexton merged 8 commits into
mainfrom
fix/audit-education

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

No related issue: audit finding fix

Summary

Audit finding plugin-education (changelog-integrity, low): the education 0.11.6 changelog entry claimed more than the change did, the 0.11.3 entry ended in a bare (#4119) instead of a link, and the teach argument hint had dropped its action list. Ships as education 0.11.7.

Fix

  • plugins/education/CHANGELOG.md: [0.11.6] now says only that the examples moved into the skill body (the old hint carried only (e.g., ...) examples); [0.11.3] links #4119 like 0.11.4 to 0.11.6; new [0.11.7] entry.
  • plugins/education/skills/teach/SKILL.md: argument-hint lists the closed action set from the Action Router table (95 characters, inside the 100-character budget).
  • plugins/education/.claude-plugin/plugin.json: version 0.11.6 to 0.11.7.
  • plugins/education/skills/quiz-me/evals/evals.json case 9 lists a new auth-middleware diff fixture (evals/fixtures/auth-middleware-change/auth-middleware.diff: public-path guard, rate-limit-before-auth ordering, invalid-token 401 path) and names those behaviors in its first expectation, so the sandbox has a change to quiz on (planning request, Eval candidates: behavioral heuristics from the Finding Your Unknowns integration #3589 AC2).
  • plugins/education/skills/setup/evals/evals.json case 3: the escaped em dash in the prompt is replaced by a colon (work-items request, F10 of unit 4070).
  • In-place corrections to released entries: [0.11.6] reworded to say only that the examples moved into the skill body; [0.11.3] issue reference linked. Both are repeated in the [0.11.7] entry, as scripts/check-changelog-parity.sh requires.

Verification

  • bash scripts/check-changelog-parity.sh --check --check-order: pass.
  • bash scripts/check-changelog-parity.sh --check-bump origin/main: pass.
  • bash scripts/check-changelog-parity.sh --check-preserved origin/main: pass (45 headings compared).
  • bash scripts/validate-plugins.sh: all manifests and the catalog validated.
  • bash scripts/validate-plugin-contracts.test.sh: PASS=92 FAIL=0, zero argument-hint warnings in the shipping tree.
  • education has no test scripts of its own.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB

kyle-sexton and others added 4 commits September 29, 2026 01:07
…teach action hint

Reword the 0.11.6 entry to say only that the examples moved into the skill body,
link the 0.11.3 issue reference, and list the action set in the teach argument-hint.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
…orrections

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
…ash in setup eval

Case 9 listed no files, so the sandbox had no change to quiz on. It now
lists an auth-middleware diff fixture with a public-path guard, a
rate-limit-before-auth ordering, and an invalid-token failure path. The
setup case 3 prompt loses its escaped em dash. Extends the 0.11.7 entry.

Refs #3589

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
@kyle-sexton
kyle-sexton marked this pull request as ready for review September 29, 2026 07:12
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-29T13:26:30.224028Z 467b7ab Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Security review complete ✅

  • Gather context (PR diff, changed files)
  • Run /review:security-review criteria against the diff
  • Report findings

No security issues found.

This PR is documentation/metadata only: a CHANGELOG.md correction, a plugin.json version bump (0.11.6 → 0.11.7), an argument-hint string change in teach/SKILL.md, and eval fixture/prompt edits in quiz-me and setup. None of it touches authentication, authorization, secrets handling, or user-input processing in the plugin's own logic.

Two points I specifically checked and ruled out:

  • New fixture evals/fixtures/auth-middleware-change/auth-middleware.diff (permalink) is inert quiz content read by the quiz-me eval sandbox for question-generation, not code that gets executed or interpreted by the plugin.
  • Instruction-surface lens: the argument-hint and CHANGELOG.md edits don't remove or soften any standing guardrail (no rule, allowlist, or hook block-list is touched), so the instruction-exception-register check doesn't apply here.

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Claude finished the code-review lane

  • Gather context (PR body, changed files)
  • Invoke /review:code-review and follow its procedure
  • Post inline findings (if any) via inline comments
  • Post final summary

Summary

Reviewed all 6 changed files against origin/main...HEAD (467b7ab). This is a low-risk changelog/eval/argument-hint correction with no meaningful bugs found:

  • plugins/education/skills/teach/SKILL.md — new argument-hint (<topic|codebase|mission|glossary|resources|explain|primer|exercise|assess|resume|status> [args]) is exactly 95 characters (confirmed by counting) and lists all 11 actions in the Action Router table (lines 97-109) with no omissions or extras.
  • plugins/education/CHANGELOG.md — [0.11.7] entry is well-formed, dated, and links #3589; [0.11.6] and [0.11.3] in-place corrections read correctly and are consistent with the repeated corrections in [0.11.7] (matches the check-changelog-parity.sh --check-preserved requirement described in the PR body).
  • plugins/education/.claude-plugin/plugin.json — version bump to 0.11.7 matches the new changelog heading.
  • plugins/education/skills/quiz-me/evals/evals.json case 9 + new fixture evals/fixtures/auth-middleware-change/auth-middleware.diff — the fixture diff genuinely contains all three behaviors the updated expectation names: a PUBLIC_PATHS guard, rateLimit middleware moved before auth (ordering), and a caught/converted invalid_token 401 instead of a thrown error. The files path convention (evals/fixtures/...) matches existing usage in plugins/bugs/skills/write/evals/evals.json and plugins/overengineering/skills/justify/evals/evals.json.
  • plugins/education/skills/setup/evals/evals.json case 3 — em dash replaced with a colon, no other change.

No high-signal correctness or maintainability issues found on the changed lines, so no inline comments were posted.

Note on verification: this sandbox's permission mode denied bash scripts/check-changelog-parity.sh ..., validate-plugins.sh, and even a plain python3 -c "json.load(...)" sanity check, so I could not independently re-run the checks the PR body cites. I did visually confirm both evals.json diffs are structurally well-formed (matching brackets/commas, no dangling entries) via direct file reads. The PR's own script-run results are author-claimed and unverified by me.

@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: 467b7ab20c

ℹ️ 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/education/skills/setup/evals/evals.json Outdated
@kyle-sexton
kyle-sexton marked this pull request as draft September 29, 2026 13:21
@kyle-sexton
kyle-sexton marked this pull request as ready for review September 29, 2026 13:24
@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Code review lane — skipped (already reviewed)

  • Gather context (PR body, changed files, existing review comments)
  • Invoke /review:code-review and follow its procedure
  • Check skip gate
  • Post final summary

Per the /review:code-review skip gate, I stopped before a full re-review: this exact head, 467b7ab20c2b08a72050f6a227b12858e75182bc, already has a successful pass from this lane — the "Claude finished the code-review lane" comment from earlier today reviewed all 6 changed files against origin/main...HEAD (467b7ab) and found no high-signal correctness/maintainability issues. Since the head hasn't moved since that pass, it still applies, so I'm not duplicating it with a second lane pass or new inline comments.

For completeness, note that chatgpt-codex-connector (a separate automated reviewer, not this lane) flagged two items on this same head that this lane's criteria would also consider in-scope as correctness bugs in the eval fixtures:

  • plugins/education/skills/setup/evals/evals.json:31 — the prompt's trailing colon (/education:setup: set quiz_policy...) makes the slash-command token parse as /education:setup: rather than /education:setup, so the case could exercise command-routing failure instead of the intended refusal behavior.
  • plugins/education/skills/quiz-me/evals/fixtures/auth-middleware-change/auth-middleware.diff:5 — the first hunk header's line counts (-1,24 +1,41) don't match the actual hunk body (13 old / 32 new lines), which git apply --numstat rejects as a corrupt patch (corrupt patch at line 48), so diff-tooling-based eval runs can't parse the fixture as intended.

These are genuine, verifiable defects (I confirmed the header/line-count mismatch and the trailing-colon parse by reading the files directly) worth fixing before merge, but I'm surfacing rather than re-reporting them as this lane's own findings since they're already on the PR from Codex and this lane's pass on this head is complete.

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Security review complete ✅

  • Gather context (PR diff, changed files, custom instructions)
  • Run /review:security-review criteria against the diff at HEAD 467b7ab
  • Report findings

No security issues found.

This PR touches 6 files, all metadata, documentation, and eval-fixture content for the education plugin — no runtime logic, auth, authorization, secret handling, or user-input processing is changed:

  • plugins/education/.claude-plugin/plugin.json — version bump only.
  • plugins/education/CHANGELOG.md — text corrections and a new [0.11.7] entry.
  • plugins/education/skills/teach/SKILL.md — argument-hint string restored to the closed action list (permalink). No guardrail, allowlist, or hook block-list is touched, so the instruction-surface-deletion lens doesn't apply.
  • plugins/education/skills/quiz-me/evals/evals.json + new evals/fixtures/auth-middleware-change/auth-middleware.diff (permalink) — the fixture is fictional quiz content (src/middleware/auth.js) consumed only as text by the quiz-me eval sandbox for question generation. It is never parsed as a real diff, applied, executed, or interpreted by plugin logic, so the malformed hunk header Codex flagged (line-count mismatch) and the security-shaped pattern inside the fixture text (rate-limit-before-auth ordering) carry no exploit path in this repo.
  • plugins/education/skills/setup/evals/evals.json — prompt punctuation change only (em dash → colon); no code path.

No CRITICAL / IMPORTANT / SUGGESTION findings.

kyle-sexton and others added 2 commits September 29, 2026 09:50
…lash command

The auth-middleware fixture had unprefixed blank context lines and hunk
counts that did not match the hunk bodies, so git apply rejected it as a
corrupt patch. The setup eval 3 prompt put a colon directly after the
slash-command token, so the command parsed as /education:setup:.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Code review complete ✅

  • Gather context (PR body, changed files, existing comments)
  • Invoke /review:code-review and follow its procedure
  • Post inline findings via inline comments
  • Post final summary

No findings.

This lane's prior pass (on 467b7ab) is stale — the head moved to aaff99a, so I re-reviewed the full diff (origin/main...HEAD, 6 files) fresh rather than relying on the earlier "no findings" result.

Commit aaff99a fixes both defects Codex flagged on 467b7ab, and I independently confirmed both fixes by inspection:

  • evals/fixtures/auth-middleware-change/auth-middleware.diff — the first hunk header now reads @@ -1,13 +1,34 @@ (was -1,24 +1,41). I hand-counted the hunk body: 5 context + 8 removed = 13 old lines, 5 context + 29 added = 34 new lines, matching the header exactly. Blank context lines also got their leading space restored. The second hunk (@@ -9,7 +9,7 @@) checks out the same way (6 context + 1 removed = 7 old, 6 context + 1 added = 7 new).
  • plugins/education/skills/setup/evals/evals.json:31 — the prompt is now /education:setup set quiz_policy... (no trailing colon), so the slash-command token parses as /education:setup cleanly.

I also re-checked the rest of the diff for anything new:

  • teach/SKILL.md argument-hint: exactly 95 characters (counted), and its 11 listed actions match the Action Router table (SKILL.md#L99-L109) exactly — no extras, no omissions.
  • CHANGELOG.md: version headings remain in descending order (0.11.7 → 0.11.2), the [0.11.6] and [0.11.3] headings themselves are unchanged (only their body text was reworded/linked), and the new [0.11.7] entry restates those same corrections as the parity script requires.
  • plugin.json: version bump (0.11.6 → 0.11.7) matches the new changelog heading.
  • quiz-me/evals/evals.json case 9: files path (evals/fixtures/auth-middleware-change/auth-middleware.diff) matches the convention used elsewhere (e.g. plugins/bugs/skills/write/evals/evals.json), and the expectation text names exactly the three behaviors the fixture now contains (public-path guard, rate-limit-before-auth ordering, invalid-token → 401).

Not independently verified: I don't have shell permission in this session to run scripts/check-changelog-parity.sh or git apply against the fixture, so the PR body's cited script runs remain author-claimed. I instead hand-verified the hunk arithmetic and changelog heading order directly from file contents, which is sufficient to confirm the two previously-flagged defects are fixed and no new issues were introduced.
· branch fix/audit-education

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Security review complete ✅

  • Gather context (PR body, changed files, custom instructions)
  • Invoke /review:security-review criteria against the diff at HEAD aaff99a
  • Report findings

No security issues found.

This head (aaff99a) adds one commit on top of the previously-reviewed 467b7ab: it fixes the two non-security defects Codex flagged in that earlier review pass. I verified both fixes directly:

  • auth-middleware.diff — the first hunk header now reads @@ -1,13 +1,34 @@. I recounted the hunk body by hand: 13 old-file lines (3 context + 1 added-context + 8 removed + 2 trailing context — wait, precisely: lines 6,8,9 context, 10‑17 removed, 46‑47 context = 13) and 34 new-file lines (context + the 28 added lines + trailing context), both matching the header. The stray unprefixed blank context lines are also gone. This is inert eval-fixture text consumed only as quiz content by the quiz-me sandbox, never applied or executed by plugin logic — so the earlier malformed-patch issue was a fixture-quality bug, not a security bug, and it's now fixed.
  • setup/evals/evals.json:31 — the prompt is now /education:setup set quiz_policy... with a space, not a colon, after the command token. No command-routing ambiguity remains.

No other files changed since the prior review at 467b7ab, which already covered the plugin.json version bump, the CHANGELOG.md corrections, and the teach/SKILL.md argument-hint restoration — none of that touches auth, secrets, or user-input processing in plugin logic, and nothing in this PR narrows or removes a standing instruction/guardrail (the instruction-surface-deletion lens doesn't apply).

A blank context line in a diff is a single space, which the editorconfig
trailing-whitespace check rejects. The fixture carries no blank context
lines and its hunk headers match their bodies.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Security review complete ✅

  • Gather context (PR diff, changed files, custom instructions)
  • Check skip gate (prior review staleness vs HEAD 16618ac)
  • Run /review:security-review criteria against the diff
  • Report findings

No security issues found.

Reviewed the full diff (origin/main...HEAD, HEAD 16618ac2a0a05d070f8985f18c2742dfbda74dbf), which is one commit past the last-reviewed head (aaff99a). That final commit (16618ac) only strips the leading-space blank context lines from the auth-middleware.diff eval fixture (an editorconfig trailing-whitespace fix) and re-tightens the hunk headers to match — no new files, no logic, no security surface.

All 6 changed files remain metadata/documentation/eval-fixture content for the education plugin, with no runtime logic, auth, authorization, secret handling, or user-input processing:

No CRITICAL / IMPORTANT / SUGGESTION findings.
· branch fix/audit-education

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Code review complete ✅

  • Gather context (PR body, changed files, existing comments)
  • Invoke /review:code-review and follow its procedure
  • Post inline findings (if any) via inline comments
  • Post final summary

No findings.

The prior pass on this lane (aaff99a) is stale — head moved to 16618ac — so I re-reviewed the full diff (gh pr diff 5243, 6 files) fresh.

The only change since aaff99a is commit 16618ac ("drop blank context lines from the quiz-me diff fixture"), a cosmetic-only edit to auth-middleware.diff: added lines that are blank now carry no trailing space (avoids the trim_trailing_whitespace = true rule at .editorconfig:17), with no change to the tested behavior. I hand-verified the current fixture is a well-formed unified diff:

  • Hunk 1 header @@ -1,11 +1,32 @@: counted 11 old lines (3 context + 8 removed) and 32 new lines (3 context + 29 added) — matches.
  • Hunk 2 header @@ -9,5 +9,5 @@: counted 5 old / 5 new — matches.
  • No blank context line lacks its leading space (checked with cat -A).

This resolves both defects Codex flagged on 467b7ab (malformed hunk headers, trailing colon in the setup eval prompt) — both were already fixed in aaff99a and remain fixed here.

I also re-checked the rest of the diff:

  • teach/SKILL.md argument-hint: counted 95 characters, inside the 100-char budget. Its 11 listed actions (topic|codebase|mission|glossary|resources|explain|primer|exercise|assess|resume|status) match the Action Router table rows exactly, same order, no omissions/extras.
  • quiz-me/evals/evals.json case 9: files references evals/fixtures/auth-middleware-change/auth-middleware.diff, and the fixture genuinely contains all three behaviors the updated expectation names — the PUBLIC_PATHS guard, rateLimit moved before auth (ordering), and a caught invalid_token → 401 instead of a thrown error.
  • setup/evals/evals.json case 3: prompt is now /education:setup set quiz_policy to always in settings.json for me. — no trailing colon, parses as a clean /education:setup command token.
  • CHANGELOG.md / plugin.json: [0.11.7] heading, corrected [0.11.6]/[0.11.3] bodies, and the version bump are internally consistent.

Not independently verified: shell access in this session denied git fetch, git apply --check, and even python3 -c "json.load(...)", so I could not execute the PR body's cited check-changelog-parity.sh / validate-plugins.sh runs or a git apply dry-run against the fixture. Those remain author-claimed. I instead hand-verified the hunk arithmetic, JSON structure (via gh pr diff output — matching brackets/commas, no dangling entries), and character counts directly, which is sufficient to confirm no regressions from aaff99a and no new issues in 16618ac.
· branch fix/audit-education

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Security review complete ✅

  • Gather context (PR diff, changed files, custom instructions)
  • Check skip gate (prior review staleness vs HEAD 9404d81)
  • Run /review:security-review criteria against the diff
  • Report findings

No security issues found.

Skip gate applies. HEAD 9404d81c097858631a00d36b3d6cbbfdcdf77cf5 is a merge of origin/main into this branch on top of the already-reviewed 16618ac. git diff 16618ac 9404d81 --stat shows 119 files touched, all of them unrelated plugins pulled in from main (e.g. plugins/animation/, plugins/biome-format/, plugins/discovery/, plugins/typos-format/) — none under plugins/education/. gh pr diff 5243 confirms the PR's actual base-relative diff is unchanged: still the same 6 files as the prior full review at 16618ac (job run), which found no security issues:

Codex's two prior findings on this PR (malformed diff-fixture hunk headers, a trailing colon in the setup eval prompt) are eval-fixture correctness issues, not security, and were already fixed in commits aaff99a/16618ac.

No CRITICAL / IMPORTANT / SUGGESTION findings.
· branch fix/audit-education

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Code review — skipped (already reviewed)

  • Gather context (PR body, changed files, existing comments)
  • Invoke /review:code-review and follow its procedure
  • Check skip gate
  • Post final summary

Per the /review:code-review skip gate, I stopped before a full re-review. Current HEAD is 9404d81c097858631a00d36b3d6cbbfdcdf77cf5, a merge of origin/main into this branch. I diffed the plugins/education/ tree between the last-reviewed head (16618ac, reviewed in the "Code review complete" comment above — no findings) and 9404d81: it is empty. The merge only pulled in unrelated main commits (dfc6cad, 55e6e9b, 638ecd3, 2c871ec, …); none of the 6 files this PR touches changed.

That prior pass already confirmed:

  • plugins/education/skills/teach/SKILL.md — argument-hint is 95 chars, matches the Action Router table's 11 actions exactly.
  • plugins/education/CHANGELOG.md / plugin.json — [0.11.7] heading, corrected [0.11.6]/[0.11.3] bodies, and the version bump are internally consistent.
  • plugins/education/skills/quiz-me/evals/evals.json + evals/fixtures/auth-middleware-change/auth-middleware.diff — fixture is a well-formed unified diff (hunk headers @@ -1,11 +1,32 @@ and @@ -9,5 +9,5 @@ both verified against hunk bodies) containing all three behaviors the updated case-9 expectation names.
  • plugins/education/skills/setup/evals/evals.json — case 3 prompt has no trailing colon after the command token.

Both defects Codex flagged on the original head (467b7ab) — the malformed diff-fixture hunk headers and the trailing-colon prompt — were fixed in aaff99a and remain fixed. No new findings on this head.
· branch fix/audit-education

@kyle-sexton
kyle-sexton merged commit 17778a8 into main Sep 29, 2026
19 checks passed
@kyle-sexton
kyle-sexton deleted the fix/audit-education branch September 29, 2026 14:59
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