Skip to content

feat(teach): add multi-session learning-coach plugin#85

Merged
kyle-sexton merged 9 commits into
mainfrom
feat/publish-teach
Jul 12, 2026
Merged

feat(teach): add multi-session learning-coach plugin#85
kyle-sexton merged 9 commits into
mainfrom
feat/publish-teach

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Migrates the teach skill out of the medley repo into a repo-agnostic teach plugin (category: learning). Multi-session learning coach that runs a Knowledge → Skills → Wisdom progression grounded in the user's real goal, with persistent per-topic learning state; plus a single-session primer action.

What's here

  • plugins/teach/ — one skill (/teach:teach) with six context/ progressive-disclosure files.
  • Catalog registration (.claude-plugin/marketplace.json) + README catalog row.

Decoupling from the source repo (the migration work)

  • State → ${CLAUDE_PLUGIN_DATA}. All learning workspaces live under ${CLAUDE_PLUGIN_DATA}/<project-slug>/<topic>/ — survives plugin updates, stays out of the consumer's tree. Path resolution is inlined as the SKILL.md "Workspace layout" SSOT (replaces the source repo's learning-rules cross-reference, which would break under cache isolation).
  • Codebase mode is runtime discovery. Instead of a baked table of the source repo's paths, codebase mode discovers the consuming repo at teach-time (its README / CLAUDE.md / AGENTS.md / ADRs / source tree, language from manifest files) and persists what it finds into the workspace RESOURCES.md. Nothing about any project is assumed.
  • Cross-skill references degrade gracefully. Only /book-distill:book-distill is a sibling plugin (referenced). Code-exploration / research / implementation tooling degrade to prose.
  • HTML output routes to /frontend-design:frontend-design when installed, else a plain self-contained page; the durable teaching record stays markdown.

Decisions

Decision What it changes Basis
No userConfig, no setup action Ships zero config surface Wave-2 inventory found no repo coupling; confirmed during authoring — workspace location is a user nicety, not a repo-varying fact (playbook Rule of Three). Matches the book-distill analog, which ships zero userConfig.
Both topic and codebase workspaces scoped under <project-slug> Topic learning becomes associated with the launch project Simplest scoping; mirrors book-distill. A repo-independent global topic tier was the alternative — deferred as unneeded complexity.
disable-model-invocation: true preserved User owns session entry; skill never auto-invokes Pedagogy contract — the model deciding to start a teaching session is wrong.

Deferred config surface (not in v0.1)

Consumer-relocatable / shareable workspace directory — a userConfig directory (or equivalent) to place learning workspaces inside the consumer's project tree instead of ${CLAUDE_PLUGIN_DATA}, e.g. to track/share a curriculum in-repo. Deferred, not excluded. Default (${CLAUDE_PLUGIN_DATA}) serves everyone today. Re-open trigger (Rule of Three): a real, repeated need to keep learning workspaces in-project or share a curriculum across a team. Add as one declared directory knob at that point, per the playbook's extensibility model.

Validation

  • claude plugin validate ./plugins/teach — pass.
  • claude plugin validate --strict . (catalog manifest) — pass.
  • Smoke-tested with --plugin-dir in a throwaway Go repo (neither medley nor this repo): codebase-mode discovery grounded in that repo's own files (src/resizer.go, its README/CLAUDE.md/ADR) and resolved the workspace under ${CLAUDE_PLUGIN_DATA} — repo-agnosticism confirmed against the exact risk.

Refs melodic-software/medley#1371


Note

Low Risk
Documentation-only plugin addition with no runtime code in the marketplace repo; main operational risk is agent behavior writing under ${CLAUDE_PLUGIN_DATA} and reading consumer repos in codebase mode, not changes to existing plugins.

Overview
Adds a new teach marketplace plugin (category: learning) with /teach:teach — a user-invoked, multi-session coach (Knowledge → Skills → Wisdom) plus a one-shot primer action.

Catalog & packaging: Registers the plugin in marketplace.json and documents it in the root README catalog. Ships plugin.json, plugin README, one skill (SKILL.md), and six progressive-disclosure context/ files (mission, glossary, resources, lessons, exercises, assessment).

Repo-agnostic state: Learning workspaces persist under ${CLAUDE_PLUGIN_DATA}/<project-slug>/<mode>/<topic>/ (canonical path + hash for slug stability; separate topic vs codebase workspaces; slug-collision guards). Nothing is written into the consumer repo; there is no userConfig.

Behavior: topic uses verified external sources; codebase discovers the launch repo at runtime (README, agent docs, ADRs, source) and caches paths in workspace RESOURCES.md. Coaching rules include ZPD scaffolding, evidence-based learning records, lazy staleness re-verify on durable references/glossary, and optional HTML via /frontend-design:frontend-design when installed. disable-model-invocation: true is preserved so sessions only start on explicit user invoke.

Reviewed by Cursor Bugbot for commit 82cffa3. Bugbot is set up for automated code reviews on this repo. Configure here.

Migrate the `teach` skill out of the medley repo into a repo-agnostic
`teach` plugin (category: learning). Coaches a user through a
Knowledge-Skills-Wisdom progression across sessions with persistent
per-topic learning state, plus a single-session domain `primer`.

Decoupling from the source repo:
- All learning state persists under ${CLAUDE_PLUGIN_DATA}/<project-slug>/<topic>/
  (survives plugin updates; does not pollute the consumer repo). Path
  resolution is inlined as the SKILL.md "Workspace layout" SSOT, replacing
  the source repo's learning-rules cross-reference.
- Codebase mode is now a runtime discovery procedure over the consuming
  repo (its README / CLAUDE.md / AGENTS.md / ADRs / source tree) instead of
  a baked table of the source repo's paths — nothing about any project is
  assumed.
- Cross-skill references degrade gracefully: only /book-distill:book-distill
  is a sibling plugin; code-exploration / research / implementation tooling
  degrade to prose.
- HTML lesson output routes to /frontend-design:frontend-design when
  present, else a plain self-contained page; the durable record stays
  markdown.

No userConfig and no setup action: the inventory confirmed no repo-varying
configuration surface (Rule of Three). disable-model-invocation preserved
(the user owns session entry, a pedagogy contract).

Validated: `claude plugin validate ./plugins/teach` and
`claude plugin validate --strict .` both pass. Smoke-tested with
--plugin-dir in a throwaway Go repo (neither medley nor this repo):
codebase-mode discovery grounded in that repo's own files and resolved
the workspace under ${CLAUDE_PLUGIN_DATA}.

Refs melodic-software/medley#1371
@cursor

cursor Bot commented Jul 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_6bc1f710-464b-4e4b-ba63-b3ac96b7cbe5)

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown

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


PR Review — feat(teach): add multi-session learning-coach plugin

Tasks

  • Gather context and read changed files
  • Review plugin manifest and marketplace registration
  • Review SKILL.md and six context files
  • Check repo-agnosticism, security, and design rule compliance
  • Check fresh-docs mandate and plugin-form safety
  • Post final review

Overall Assessment

This is a high-quality migration that is well-structured, correctly isolated, and clearly reasoned. The pedagogy design (K-S-W progression, ZPD scaffolding, staleness handling) is strong and the decoupling decisions are well-argued. The plugin passes the migration gate and security review with no blocking issues. I have a few correctness and completeness observations below.


Plugin-acceptance security review

Check Result
Code execution (hooks/scripts) ✅ PASS — skill-only plugin, no hooks or scripts
Remote MCP servers ✅ PASS — no MCP servers declared
userConfig credentials ✅ PASS — no userConfig, no secrets surface
Cache isolation ✅ PASS — only ${CLAUDE_PLUGIN_DATA} and ${CLAUDE_PLUGIN_ROOT} referenced
Data egress ✅ PASS — model-controlled WebFetch in topic mode; no hook/script outbound calls
Provenance & trust ✅ PASS — author is Melodic Software, license MIT

Findings

1. Path canonicalization in workspace slug is underspecified (medium)

SKILL.md L44 instructs the model to hash "the resolved absolute project path" but doesn't say how to resolve it. If the same project is launched once via a symlink path and once via the real path, ${CLAUDE_PROJECT_DIR} will have different values, producing different 8-hex suffixes — the user gets two separate workspaces for the same project. The macOS/Linux fallback for sha256sum is thoughtful; the same care should apply to the path itself:

# suggested clarification in the prose:
# canonicalize first: realpath "${CLAUDE_PROJECT_DIR}" 2>/dev/null
#   || readlink -f "${CLAUDE_PROJECT_DIR}" 2>/dev/null
#   || echo "${CLAUDE_PROJECT_DIR}"
# then pipe that result to sha256sum / shasum -a 256

Adding one explicit canonicalization step prevents phantom workspace splits on macOS (where Homebrew/Docker often place repos under /private/var/… symlinks). Fix this →


2. status and resume actions are underspecified (low)

The action router at SKILL.md L70-L71 lists resume and status with one-liner descriptions but no context file link and no section in SKILL.md elaborating the behavior, unlike every other action. Two ambiguities a user will hit:

  • resume: which workspace? A user with rust-ownership and auth-flow workspaces who calls /teach resume gets no disambiguation guidance. Should it list candidates? Use the most recently modified workspace? Ask?
  • status: what constitutes "progress"? The number of learning records? The last lesson title? The frontier concepts? The current answer is "whatever the model decides."

These don't block v0.1 but will produce inconsistent behavior. Adding even a short prose paragraph for each (or a context/status.md and context/resume.md analogous to the other six) would pin the contract. Fix this →


3. explain/exercise without an active workspace — no error path (low)

SKILL.md L66-L68 says explain <concept> writes to concepts/<concept>/lesson.md but doesn't specify what happens when no workspace exists yet (i.e., the user hasn't run topic or codebase first). The model will need to pick a workspace to write into and currently has no guidance — it will likely infer or ask, but the behavior should be stated.


4. WebFetch permission requirement undocumented in README (low)

The topic mode instructs the model to fetch from resources and documentation URLs (SKILL.md "Knowledge" layer). Consumers who run Claude Code with a restrictive permission set may be surprised when /teach topic starts requesting WebFetch permission. README.md currently says "None beyond Claude Code" under Requirements, which understates this. Worth a single-sentence callout: "topic mode fetches documentation URLs; if your setup restricts WebFetch, allow it or pre-seed RESOURCES.md manually."


5. No CHANGELOG.md (informational)

The playbook's "Version pinning" section says "A version bump that changes behavior a consumer depends on records the change in the plugin's changelog (a CHANGELOG.md in the plugin)." The analogous book-distill plugin (v0.1.1) presumably added one at its first bump. Starting teach at v0.1.0 without a CHANGELOG.md is fine today, but adding an empty one now (## [0.1.0] — Initial release) avoids the first bump having a missing changelog file rather than an existing one to append to.


What passes cleanly

  • Manifest structure matches book-distill exactly — no skills array needed, auto-discovered by convention. ✓
  • disable-model-invocation: true correctly set. ✓
  • ${CLAUDE_PLUGIN_DATA} used as seam 4 (machine state, not configuration). ✓
  • Codebase mode reads only ${CLAUDE_PROJECT_DIR} — no reach-out beyond the consuming repo. ✓
  • Topic/concept slug sanitization instruction is clear ("lowercase alphanumerics and hyphens only; strip /, \, ..") — acceptable for skill-form, which cannot enforce at runtime. ✓
  • Cross-skill references (/book-distill:book-distill, /frontend-design:frontend-design) follow playbook degrade-to-prose pattern. ✓
  • Staleness handling (lazy verify-on-revisit, age × velocity judgment, no freshness frontmatter) is well-reasoned and avoids the anti-pattern of stored verified: dates. ✓
  • HTML output secret-leakage rule in context/lessons.md is explicit and correct. ✓
  • Zero userConfig is justified by Rule of Three and matches the book-distill analog. ✓
  • Pre-computed !ls context includes head -20 limit and a graceful || echo "none" fallback. ✓
  • Marketplace registration category "learning" and tags are appropriate. ✓

Recommendation: Approve with the path-canonicalization note addressed before or shortly after merge. Items 2–5 are quality improvements; only item 1 has a real correctness consequence for users with symlinked project directories.

@cursor cursor 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.

Stale comment

Risk: medium. Not approving because Cursor Bugbot skipped (usage limit reached) and did not complete automated review. Human review is needed before merge; no reviewers were assigned because no non-author human maintainers are available on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

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

ℹ️ 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/teach/skills/teach/SKILL.md Outdated
…/no-workspace paths

Address bot-review findings on PR #85:
- Canonicalize ${CLAUDE_PROJECT_DIR} (realpath/readlink) before hashing the
  project-slug, so a project opened via a symlink and via its real path share
  one workspace instead of splitting into two (macOS /private/var symlinks).
- Specify the resume (disambiguate when multiple workspaces), status (output
  shape), and explain/exercise-without-a-workspace behaviors, which were
  underspecified in the action router.
- Note topic-mode WebFetch usage under README Requirements.
@cursor

cursor Bot commented Jul 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_f6cddff1-7769-4dd9-b054-abd14d47d2cb)

@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Addressed in c38788a:

#5 CHANGELOG: skipped to match sibling precedent — no plugin here ships a CHANGELOG.md at 0.1.0; the playbook requires one at the first behavior-changing bump, added then.

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown

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


PR Re-Review — feat(teach): add multi-session learning-coach plugin

Tasks

  • Load tools
  • Read updated SKILL.md (c38788a fixes)
  • Review fixes for findings chore: initialize marketplace scaffold #1–4 — all confirmed addressed
  • Evaluate Codex comment on /teach vs /teach:teach — confirmed valid, fix prepared locally (push access denied)
  • Post final review

Findings #1–4 (from previous review): All Addressed ✓

The c38788a commit cleanly addresses all four correctness findings:


New Finding: Codex P2 — /teach/teach:teach in invocation examples (correctness)

The Codex reviewer is correct. Every other plugin in this repo uses the /<plugin>:<skill> form in its argument-hint (e.g., /diagnose:diagnose, /review-toolkit:code-review-fanout, /discovery:explore). The teach plugin uses bare /teach in six places. When installed, users who follow these examples will get a "command not found" error because Claude Code resolves plugin-namespaced skills as /teach:teach.

Affected locations (all in the PR diff):

File Location Current Correct
SKILL.md:4 argument-hint examples /teach topic rust-ownership, etc. /teach:teach topic rust-ownership, etc.
SKILL.md:85 Primer escalation offer offer /teach topic <domain> offer /teach:teach topic <domain>
SKILL.md:208-209 "What This Skill Does NOT Do" /teach and /teach codebase /teach:teach and /teach:teach codebase
README.md:8-9 Install/usage examples /teach topic rust-ownership, etc. /teach:teach topic rust-ownership, etc.
context/resources.md:42 Codebase Mode section For /teach codebase For /teach:teach codebase
context/mission.md:39 Codebase Mode Additions For /teach codebase <concept> For /teach:teach codebase <concept>

I prepared and committed this fix locally (d01b08b) but couldn't push (write access denied). The diff is minimal — a one-line change per file — and the fix is mechanical. Fix this →


What Passes Cleanly

The c38788a changes are all well-executed. No regressions introduced. The plugin is otherwise correct and ready to merge once the command-form fix is applied.

@cursor cursor 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.

Stale comment

Risk: medium. Not approving because Cursor Bugbot skipped (usage limit reached) and did not complete automated review. Human review is needed before merge; no reviewers were assigned because no non-author human maintainers are available on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

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

ℹ️ 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/teach/skills/teach/SKILL.md Outdated
…kspace path

Address Codex review findings on PR #85:
- Use the namespaced /teach:teach command in argument-hint, README, catalog
  row, and cross-references — bare /teach does not resolve for an installed
  plugin (matches the sibling /prototype:logic, /diagnose:diagnose convention).
- Insert a <mode> level (topic|codebase) into the workspace path so
  /teach:teach topic <x> and /teach:teach codebase <x> in the same project no
  longer resolve to one directory and seed over each other. Pre-computed
  workspace glob and HTML placement path updated to match.
@cursor

cursor Bot commented Jul 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_7119c9ed-187d-4ddc-ae6b-027c408023a4)

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown

PR Re-Review — feat(teach): add multi-session learning-coach plugin

Tasks

  • Load tools
  • Read updated files (307f8f5 fixes)
  • Verify /teach:teach namespacing fix
  • Verify <mode> workspace path fix (Codex P2 — topic/codebase collision)
  • Post final review

View job run

@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Addressed Codex P2 findings in 307f8f5 and resolved both threads:

  • /teach:teach namespacing: all command examples (argument-hint, README, catalog row, cross-refs) now use the namespaced form — matches the sibling /prototype:logic / /diagnose:diagnose convention. Verified during smoke test that bare /teach does not resolve.
  • mode disambiguation: workspace path now includes a <mode> level (<project-slug>/<mode>/<topic>/), so topic auth-flow and codebase auth-flow in one project no longer collide.

@cursor

cursor Bot commented Jul 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_c952bdcc-129e-4438-9b10-113680d8cce5)

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown

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


PR Final Re-Review — feat(teach): add multi-session learning-coach plugin

Tasks

  • Load tools
  • Read current state of all changed files (14e62f2 HEAD)
  • Verify all prior findings addressed (namespacing, mode path, canonicalization, resume/status, WebFetch)
  • Check for any new issues introduced by the merge commit
  • Post final review

All Prior Findings: Confirmed Addressed ✓

The 307f8f5 commit cleanly resolves every open item from the previous review cycles:

Finding Status Evidence
#1 Path canonicalization ✅ Fixed SKILL.md:44realpath … 2>/dev/null || readlink -f … 2>/dev/null || echo … chain inlined before the SHA-256 hash step
#2 Resume/Status underspecified ✅ Fixed SKILL.md:88–94 — "Resume, Status, and workspace resolution" section pins both contracts: resume with no arg lists by mtime and asks; status output shape (mode, topic, record count, frontier concept, last-touched date) is specified
#3 explain/exercise without workspace ✅ Fixed Same section at SKILL.md:94 — "If exactly one workspace exists, use it; if several exist, ask which; if none exists, ask whether to start one … before writing — never invent a workspace silently"
#4 WebFetch in README ✅ Fixed README.md:42–44 — "topic mode fetches documentation URLs … if your setup restricts WebFetch, allow it or seed RESOURCES.md manually"
Codex P2 – /teach/teach:teach ✅ Fixed All six locations updated: SKILL.md:4 (argument-hint), SKILL.md:86 (primer escalation), SKILL.md:210 ("What This Skill Does NOT Do"), context/mission.md:39, context/resources.md:42, README.md:8–9
Codex P2 – mode collision ✅ Fixed SKILL.md:27 workspace layout tree now shows <project-slug>/<mode>/<topic>/; SKILL.md:45 explains the isolation guarantee; SKILL.md:54 ls glob updated to */*/*/ (3 levels); context/lessons.md:50 HTML path also updated

Merge Commit (14e62f2)

The HEAD is a straightforward Merge remote-tracking branch 'origin/main' into feat/publish-teach. It brings in other plugin additions that landed on main after this branch was created. No conflicts with the teach plugin content — the merge is clean and introduces no regressions.


Plugin-Acceptance Security Review — Final Pass

Check Result
Code execution ✅ No hooks or scripts
Remote MCP servers ✅ None declared
userConfig credentials ✅ No userConfig
Cache isolation ✅ Only ${CLAUDE_PLUGIN_DATA} and ${CLAUDE_PLUGIN_ROOT}
Data egress ✅ Model-controlled WebFetch in topic mode only; documented
Provenance ✅ MIT, Melodic Software authorship

Recommendation: Ready to Merge ✓

Every correctness finding has been addressed, the namespacing and mode-isolation fixes are consistent across all six locations, the security review passes cleanly, and the merge from main is conflict-free. No open items remain.

@cursor cursor 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.

Stale comment

Risk: medium. Not approving because Cursor Bugbot skipped (usage limit reached) and did not complete automated review. Human review is needed before merge; no reviewers were assigned because no non-author human maintainers are available on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

# Conflicts:
#	.claude-plugin/marketplace.json
#	README.md

@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: 14e62f22e2

ℹ️ 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/teach/skills/teach/SKILL.md Outdated
@cursor

cursor Bot commented Jul 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_54a9d1e9-43de-457e-b6c6-05c66fb2242f)

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown

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


PR Final Re-Review — feat(teach): add multi-session learning-coach plugin

Tasks

  • Load tools
  • Read current SKILL.md — Codex finding confirmed valid
  • Fix slug prefix derivation to use canonical basename (SKILL.md:44) — prepared locally as bc1f1b9, push access denied
  • Post final review

Open Finding: Codex P2 — Slug basename must use canonical path (correctness)

The latest Codex comment (reviewed at 14e62f22e2) is correct and still unaddressed.

The bug: SKILL.md:44 canonicalizes ${CLAUDE_PROJECT_DIR} before hashing, but the human-readable basename prefix is still derived from the raw path. When the same project is opened via a symlink whose basename differs from the real directory name, the slug prefix diverges:

Open via Canonical path basename used hash resulting slug
/Users/me/mylink (symlink) /Users/me/dev/myproject mylink (wrong) abc12345 mylink-abc12345
/Users/me/dev/myproject (real) /Users/me/dev/myproject myproject abc12345 myproject-abc12345

Two different workspace directories for the same project — resume and status won't find each other's workspaces. The canonicalization added in c38788a for the hash is necessary but not sufficient; the prefix must also come from the canonical path.

The fix (prepared locally as bc1f1b9, push access denied): replace:

the basename of `${CLAUDE_PROJECT_DIR}` … SHA-256 of the canonicalized absolute project path

with:

CANONICAL=$(realpath "${CLAUDE_PROJECT_DIR}" 2>/dev/null || readlink -f "${CLAUDE_PROJECT_DIR}" 2>/dev/null || echo "${CLAUDE_PROJECT_DIR}")

then derive both the basename and hash from ${CANONICAL}:

basename "${CANONICAL}" | tr '[:upper:]' '[:lower:]' | tr -cs 'a-z0-9' '-' | sed 's/-\+/-/g;s/^-//;s/-$//'
printf '%s' "${CANONICAL}" | { sha256sum 2>/dev/null || shasum -a 256; } | cut -c1-8

Fix this →


All Prior Findings (from previous review cycles): Confirmed Addressed ✓

Finding Status
#1 Path canonicalization (hash) ✅ Fixed in c38788a
#2 Resume/Status underspecified ✅ Fixed in c38788a
#3 explain/exercise without workspace ✅ Fixed in c38788a
#4 WebFetch in README ✅ Fixed in c38788a
Codex P2 – /teach/teach:teach namespacing ✅ Fixed in 307f8f5
Codex P2 – mode collision (topic vs codebase same topic) ✅ Fixed in 307f8f5
Codex P2 – slug basename from canonical path Still open

Recommendation

Apply the single-line fix to SKILL.md:44 (apply locally as bc1f1b9) before merge. It's a mechanical one-liner: change "the basename of ${CLAUDE_PROJECT_DIR}" → "the basename of ${CANONICAL}" and introduce the CANONICAL=… assignment. Everything else is ready to merge.

@cursor cursor 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.

Stale comment

Risk: medium. Not approving because Cursor Bugbot skipped (usage limit reached) and did not complete automated review. Human review is needed before merge; no reviewers were assigned because no non-author human maintainers are available on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

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

ℹ️ 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/teach/skills/teach/SKILL.md Outdated
…to current project

Address Codex review findings on PR #85:
- Derive BOTH the project-slug basename and the hash from the canonicalized
  project path. Previously only the hash input was canonicalized, so a symlink
  alias with a different basename still split the workspace (alias-<hash> vs
  realname-<hash>).
- Scope the pre-computed "existing workspaces" glob to the current project's
  computed slug instead of listing every project's workspaces under the
  per-plugin data dir — honors the current-project scoping the resume/status
  actions promise.
@cursor

cursor Bot commented Jul 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_93f24259-752f-45a4-8208-35c6d9e8d6c8)

@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Both addressed in 6585002 and resolved:

  • slug prefix from canonical path: basename now derives from the canonicalized path (not raw ${CLAUDE_PROJECT_DIR}), so a symlink alias no longer splits the workspace.
  • prefetch scoping: the pre-computed "existing workspaces" glob now computes the current project's slug and lists only that project's workspaces.

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown

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


PR Final Re-Review — feat(teach): add multi-session learning-coach plugin

Tasks

  • Load tools
  • Read current SKILL.md (6585002 HEAD — slug basename + prefetch scope fixes)
  • Read README.md, plugin.json, context files
  • Verify all prior findings addressed
  • Verify 6585002 fixes: slug prefix from canonical path + prefetch scoped to current project
  • Post final review

6585002 Fixes — Both Confirmed ✓

The two Codex P2 findings from the previous round are addressed cleanly in 6585002.

Slug basename from canonical path (SKILL.md:44 + Pre-computed Context:54)

The pre-computed context shell expression now derives both $b (basename) and $h (hash) from $p (the canonical path), not from raw ${CLAUDE_PROJECT_DIR}:

p="$(realpath "${CLAUDE_PROJECT_DIR}" 2>/dev/null \
  || readlink -f "${CLAUDE_PROJECT_DIR}" 2>/dev/null \
  || printf '%s' "${CLAUDE_PROJECT_DIR}")"
b="$(basename "$p" | tr '[:upper:]' '[:lower:]' | tr -cs 'a-z0-9' '-' | sed 's/^-*//;s/-*$//')"
h="$(printf '%s' "$p" | { sha256sum 2>/dev/null || shasum -a 256; } | cut -c1-8)"
ls -d "${CLAUDE_PLUGIN_DATA}/$b-$h"/*/*/ 2>/dev/null | head -20 || echo "none"

The prose in SKILL.md:44 explicitly documents the intent: "derive BOTH the basename-slug and the hash from that one canonical path, so a project opened via a symlink and via its real path map to the same workspace (otherwise the alias basename would still split it — alias-<hash> vs realname-<hash>)". Implementation matches prose exactly. ✓

Prefetch scoped to current project (SKILL.md:54)

The ls -d glob is "${CLAUDE_PLUGIN_DATA}/$b-$h"/*/*/ — pinned to the current project slug $b-$h. A user with workspaces in multiple repos will see only the current project's topics listed as pre-computed context. ✓


All Findings — Complete Audit

Finding Introduced Status
#1 Path canonicalization for hash c38788a ✅ Fixed
#2 Resume/Status underspecified c38788a ✅ Fixed
#3 explain/exercise without workspace c38788a ✅ Fixed
#4 WebFetch undocumented in README c38788a ✅ Fixed
Codex P2 – /teach/teach:teach namespacing (6 locations) 307f8f5 ✅ Fixed
Codex P2 – topic/codebase mode collision (same topic name) 307f8f5 ✅ Fixed
Codex P2 – slug basename from canonical path 6585002 ✅ Fixed
Codex P2 – prefetch scoped to current project only 6585002 ✅ Fixed

Plugin-Acceptance Security Review — Final Pass

Check Result
Code execution (hooks/scripts) ✅ None
Remote MCP servers ✅ None declared
userConfig credentials ✅ No userConfig
Cache isolation ✅ Only ${CLAUDE_PLUGIN_DATA} and ${CLAUDE_PLUGIN_ROOT}
Data egress ✅ Model-controlled WebFetch in topic mode; documented in README
Provenance & trust ✅ MIT, Melodic Software

Recommendation: Ready to Merge ✓

Every finding across all review cycles is resolved. The 6585002 slug + prefetch fixes are correct and match the prose spec. No new issues introduced. The plugin is secure, repo-agnostic, and ready for catalog publication.
| Branch

@cursor cursor 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.

Stale comment

Risk: medium. Not approving because Cursor Bugbot skipped (usage limit reached) and did not complete automated review. Human review is needed before merge; no reviewers were assigned because no non-author human maintainers are available on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

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

ℹ️ 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/teach/skills/teach/SKILL.md Outdated
Address Codex review finding on PR #85: distinct subjects can normalize to the
same slug (C++/C# -> c; Node.js/Node JS -> node-js), silently sharing one
workspace. Record the exact raw subject/concept name as the collision source of
truth, and on a slug-directory collision with a different recorded name, append
a 4-hex discriminator derived from the raw name — isolating per-subject state
while leaving non-colliding slugs readable.
@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Addressed in f42df62 and resolved: topic/concept slugs now record the exact raw name and append a 4-hex discriminator on a real collision (e.g. C++ vs C#), so per-subject state stays isolated while non-colliding slugs stay readable.

@cursor

cursor Bot commented Jul 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_06f61209-7210-42e4-be84-8635e76ffef9)

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown

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


PR Final Re-Review — feat(teach): add multi-session learning-coach plugin

Tasks

  • Load tools
  • Read current SKILL.md (f42df62 — topic/concept slug collision fix)
  • Verify collision-disambiguation fix
  • Check consistency across context files
  • Post final review

f42df62 Fix — Confirmed Correct ✓

The one-line change to SKILL.md:46 cleanly addresses the Codex P2 finding on slug collision.

What the fix adds to the <topic> / <concept> path rule:

  1. Collision source of truth — the raw subject name is recorded in MISSION.md's # Mission: {Topic} header (for topics) and in the lesson's **Concept:** line (for concepts). These are readable by the skill on every workspace open, requiring no new file or metadata convention.

  2. Collision guard — before creating a workspace whose slug directory already exists, read the existing workspace's recorded raw name; if it differs, append - + first 4 hex chars of sha256("<raw-name>") to the new slug. The 4-hex discriminator is deterministic per subject, so resume sessions can replay the same collision-check logic and land on the right workspace.

Traced through the motivating examples:

Subject Slug attempt c/ exists? Recorded raw name Action
C++ (first) c No Create c/, write # Mission: C++ in MISSION.md
C# (first collision) c Yes "C++" ≠ "C#" Compute sha256("C#")[0:4] → append → create c-<hash>/
C# (resume) c Yes "C++" ≠ "C#" Same hash → c-<hash>/ → raw name "C#" matches → resume ✓

The discriminator is collision-resistant at this scale (65,536-value space across real-world topic names). A double-collision scenario (two subjects that share a slug AND have the same 4-hex hash) is not specified but is astronomically unlikely and not a v0.1 concern.


Complete Finding Audit — All Resolved ✓

Finding Introduced Status
#1 Path canonicalization (hash) c38788a ✅ Fixed
#2 Resume/Status underspecified c38788a ✅ Fixed
#3 explain/exercise without workspace c38788a ✅ Fixed
#4 WebFetch undocumented in README c38788a ✅ Fixed
Codex P2 – /teach/teach:teach namespacing (6 locations) 307f8f5 ✅ Fixed
Codex P2 – topic/codebase mode collision (same topic name) 307f8f5 ✅ Fixed
Codex P2 – slug basename from canonical path 6585002 ✅ Fixed
Codex P2 – prefetch scoped to current project only 6585002 ✅ Fixed
Codex P2 – topic/concept slug collision (C++/C#, Node.js/Node JS) f42df62 ✅ Fixed

Plugin-Acceptance Security Review — Final Pass

Check Result
Code execution (hooks/scripts) ✅ None
Remote MCP servers ✅ None declared
userConfig credentials ✅ No userConfig
Cache isolation ✅ Only ${CLAUDE_PLUGIN_DATA} and ${CLAUDE_PLUGIN_ROOT}
Data egress ✅ Model-controlled WebFetch in topic mode; documented in README
Provenance & trust ✅ MIT, Melodic Software

Recommendation: Ready to Merge ✓

Every finding across all review cycles is resolved. The f42df62 slug-collision fix is correct, deterministic on resume, and appropriately scoped for v0.1. No open items remain.
| Branch

@cursor cursor 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.

Stale comment

Risk: medium. Not approving because Cursor Bugbot skipped (usage limit reached) and did not complete automated review. Human review is needed before merge; no reviewers were assigned because no non-author human maintainers are available on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

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

ℹ️ 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/teach/skills/teach/SKILL.md Outdated
@cursor

cursor Bot commented Jul 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_dfff046b-bc5b-4349-ba2a-5afaf9078a0e)

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown

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


PR Final Re-Review — feat(teach): add multi-session learning-coach plugin

Tasks

  • Load tools
  • Read current SKILL.md and context files at HEAD (dab5af1)
  • Verify Codex finding — remaining bare /teach reference — is now fixed
  • Final comprehensive review pass
  • Post final review

dab5af1 Fix — Confirmed ✓

The Codex P2 finding from the previous round is addressed cleanly.

What changed (single-line diff in SKILL.md:209):

- **Does not replace `/book-distill:book-distill`** — that extracts book knowledge into skill reference files; `/teach` delivers knowledge interactively to the user
+ **Does not replace `/book-distill:book-distill`** — that extracts book knowledge into skill reference files; `/teach:teach` delivers knowledge interactively to the user

A grep for bare /teach (not followed by :) across the entire plugin directory returns no matches — the fix is complete and consistent with every other namespaced reference.


Complete Finding Audit — All Resolved ✓

Finding Fixed in Status
#1 Path canonicalization (hash) c38788a
#2 Resume/Status underspecified c38788a
#3 explain/exercise without workspace c38788a
#4 WebFetch undocumented in README c38788a
Codex P2 – /teach/teach:teach namespacing (6 locations) 307f8f5
Codex P2 – topic/codebase mode collision (same topic name) 307f8f5
Codex P2 – slug basename from canonical path 6585002
Codex P2 – prefetch scoped to current project only 6585002
Codex P2 – topic/concept slug collision (C++/C#, Node.js/Node JS) f42df62
Codex P2 – last bare /teach in "What This Skill Does NOT Do" dab5af1

Plugin-Acceptance Security Review — Final Pass

Check Result
Code execution (hooks/scripts) ✅ None
Remote MCP servers ✅ None declared
userConfig credentials ✅ No userConfig
Cache isolation ✅ Only ${CLAUDE_PLUGIN_DATA} and ${CLAUDE_PLUGIN_ROOT}
Data egress ✅ Model-controlled WebFetch in topic mode; documented in README
Provenance & trust ✅ MIT, Melodic Software

Recommendation: Ready to Merge ✓

Every finding across all review cycles is resolved. The dab5af1 fix is the last open item and is confirmed correct. No bare /teach references remain anywhere in the plugin. The manifest, marketplace registration, README catalog row, and all six context files are consistent. No open items remain.
| Branch

@cursor cursor 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.

Stale comment

Risk: medium. Not approving because Cursor Bugbot skipped (usage limit reached) and did not complete automated review. Human review is needed before merge; no reviewers were assigned because no non-author human maintainers are available on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@cursor

cursor Bot commented Jul 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_9164e9cf-19aa-42cf-9baf-cf3ac4818c8b)

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown

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


PR Final Re-Review — feat(teach): add multi-session learning-coach plugin

Tasks

  • Read current state of SKILL.md at HEAD (82cffa3 — merge commit)
  • Read README.md, plugin.json, and context files
  • Verify no regressions introduced by the merge from main
  • Confirm all prior findings still resolved
  • Post final review

Merge Commit (82cffa3)

This synchronize event is triggered by a clean merge of origin/main into the branch. The only net-new change from main was the kindle-dedrm plugin — no conflicts or regressions in the teach plugin content.


Final Verification Pass — All Findings Confirmed Resolved ✓

Confirmed at HEAD (82cffa3):

Finding Fixed in Status
#1 Path canonicalization (hash) c38788a SKILL.md:44realpath/readlink -f fallback chain
#2 Resume/Status underspecified c38788a SKILL.md:88–94 — contracts pinned
#3 explain/exercise without workspace c38788a SKILL.md:94 — resolve-one/ask/offer-to-start
#4 WebFetch undocumented in README c38788a README.md:42–44 — documented
Codex P2 – /teach/teach:teach namespacing 307f8f5 ✅ 6 locations updated
Codex P2 – topic/codebase mode collision 307f8f5 <mode> level in workspace path
Codex P2 – slug basename from canonical path 6585002 ✅ Both basename and hash derive from $p (canonical)
Codex P2 – prefetch scoped to current project 6585002 $b-$h pinned in ls -d glob
Codex P2 – topic/concept slug collisions (C++/C#) f42df62 ✅ 4-hex discriminator on collision
Codex P2 – last bare /teach in "Does NOT Do" dab5af1 /teach:teach throughout

Grep for bare /teach (not followed by :) across all plugin files: no matches. The namespace fix is complete and consistent.


Plugin-Acceptance Security Review — Final Pass

Check Result
Code execution (hooks/scripts) ✅ None
Remote MCP servers ✅ None declared
userConfig credentials ✅ No userConfig
Cache isolation ✅ Only ${CLAUDE_PLUGIN_DATA} and ${CLAUDE_PLUGIN_ROOT}
Data egress ✅ Model-controlled WebFetch in topic mode; documented in README
Provenance & trust ✅ MIT, Melodic Software

Recommendation: Ready to Merge ✓

The merge from main introduced no regressions. Every finding across all review cycles is resolved. No open items remain.
| Branch

@cursor cursor 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.

Risk: medium. Not approving because Cursor Bugbot skipped (usage limit reached) and did not complete automated review. Human review is needed before merge; no reviewers were assigned because no non-author human maintainers are available on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@kyle-sexton
kyle-sexton merged commit 323c540 into main Jul 12, 2026
22 checks passed
@kyle-sexton
kyle-sexton deleted the feat/publish-teach branch July 12, 2026 05:22

@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: 82cffa3c93

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


- **`resume [<topic>]`** — with a topic argument, resolve that workspace directly (disambiguating by `<mode>` if the same topic exists in both) and follow "Resume (subsequent sessions)". With no argument, list the workspaces sorted by most-recently-modified (git or filesystem mtime of the workspace files) and ask the user which to resume — never silently pick one when more than one exists.
- **`status`** — for each workspace, show its mode, topic, the count of `learning-records/`, the current frontier concept (from the latest records), and the last-touched date (mtime). One line per workspace; no file bodies loaded.
- **`explain <concept>` / `exercise` need an active workspace.** They write into `concepts/<concept>/` under a topic workspace. If exactly one workspace exists, use it; if several exist, ask which; if none exists, ask whether to start one (`topic` / `codebase`) before writing — never invent a workspace silently.

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 Disambiguate workspace for all scoped actions

When a project has multiple teach workspaces, this guard only forces selection for explain/exercise, but mission, glossary, resources, and assess also read or mutate workspace files. A standalone /teach:teach glossary or /teach:teach assess after multiple topics exist has no mandated ask-if-several behavior and can update the wrong topic’s state; apply the same active-workspace resolution rule to every workspace-scoped action.

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.

1 participant