ref(skills): optimize the skill library — retire duplication, fix shipped dangling pointers - #299
Open
sergical wants to merge 9 commits into
Open
ref(skills): optimize the skill library — retire duplication, fix shipped dangling pointers#299sergical wants to merge 9 commits into
sergical wants to merge 9 commits into
Conversation
AI monitoring had three sources of truth: the sentry-setup-ai-monitoring
skill (2.6k words), concepts/ai-monitoring.md, and the per-platform
sdks/*/ai-monitoring.md references (7.3k words across node, python, php,
nextjs, nestjs). The skill predates the reference library and the platform
references now cover it in more depth.
It was also a trigger collision: sentry-instrument's description claims
AI/LLM monitoring while the skill sat behind the sentry-feature-setup
router, so the same request resolved differently depending on which door
the agent came through.
Delete the skill and let sentry-instrument own the signal:
- Move the tracesSampler / traces_sampler patterns from the skill's
references/sampling.md into concepts/ai-monitoring.md. This was the only
content with no home in the new tree — every platform's "Sampling
Strategy" section was a one-line stub, and sdks/ references are barred
from carrying sampling strategy by sdks/STRUCTURE.md.
- Carry the AI SDK vendor names into sentry-instrument's description; they
were the deleted skill's invocation surface.
- Drop the AI row and routing rule from sentry-feature-setup.
Also fixes a dead pointer that shipped in every plugin: the skill's
Laravel section pointed at ${SKILL_ROOT}/../../references/sdks/php/ai-monitoring.md,
but no build copies a top-level references/ dir — hydration only copies
into skills that declare a references.yml, and this skill declared none.
sdks/php/ai-monitoring.md named the skill back, which sdks/STRUCTURE.md
also forbids; it now points at the gen_ai conventions instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sentry-workflow routed to exactly one skill, sentry-sdk-upgrade, and its description was that skill's description reworded. That is one always-loaded description plus a file read plus a routing hop to reach one skill — strictly more expensive than exposing the skill directly, on both context and cognitive load. The router layer was sized for a 30+ skill library. Eight skills ship today; the 26 the docs still describe live in skills-legacy and are built by nothing. - Delete the router; sentry-sdk-upgrade becomes standalone (no category, parent, breadcrumb, or disable-model-invocation), matching the shape the newer skills already use. - Teach the generator to skip a category with no skills left, so an emptied section stops rendering a header and column titles the tree can't route to. - Drop the "Review code" option from the SKILL_TREE entry menu. The skills behind it were removed in 7141b95, so the menu was offering a capability that no longer exists. - Stop restating the greet-and-ask protocol in the tree header. sentry-get-started Step 0 owns it and does it better; two copies means the agent runs whichever it read first. sentry-feature-setup still fronts three skills and stays for now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…k the v10 upgrade path Three skills predate the current conventions and had drifted. **Restated invocation triggers.** sentry-create-alert and sentry-sdk-upgrade each carried an "Invoke This Skill When" section restating the frontmatter description in the body — where it cannot do invocation work, since the body is only read once the skill has already fired. Deleted; the description is the single source of truth for triggers, as it already is in the newer skills. **sentry-otel-exporter-setup.** - Step 5 steered by prohibition, listing the three exact wrong phrasings under "DO NOT say". Naming the phrasing makes it more available, not less. Now states the target phrasing plus one hard rule: the value written is always a placeholder. - Step numbering was self-contradictory: Step 4 said to record the config path "for use in Steps 5 and 6" while Step 5 said to read it "from Step 1", and Step 5 pointed at itself for validation. The buried validate-and-confirm gate is now its own Step 6, later steps renumbered, and the four values that thread through the setup are declared in one table up front. - Six restatements of "wait for the user's answer before proceeding" collapse into one protocol statement: every step ends on a recorded decision. - Verification now ends on telemetry seen in Sentry rather than a clean start. **sentry-sdk-upgrade.** The skill recommended "the latest major version (v9 as of this writing)" and detected only v7/v8 patterns, while references/v9-to-v10.md declared v10 unreleased — contradicting the SDK references in this repo, which document @sentry/node 10.61+. Rather than hardcode another version that will go stale, it now resolves the latest major from the changelog at run time, detects v9 patterns, and falls back to the official migration guide whenever a hop's reference is thin. The v10 reference says plainly that it is incomplete instead of implying there is nothing to migrate. Added a completion criterion to sentry-create-alert (read the workflow back, don't stop at 201) and sentry-sdk-upgrade (Sentry confirmed still capturing after the bump). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AGENTS.md (symlinked as CLAUDE.md, so it loads into every session in this repo) described a library that no longer exists: 26 of the skills in its two tables live in skills-legacy and are built by nothing, and its conventions still covered the code-review skills removed in 7141b95. It misdirects anyone who trusts it — including agents, which is its whole purpose. - Replace the skill tables with the eight that ship, split by the shape they actually take (standalone vs router-backed), and say plainly that skills-legacy is frozen and unbuilt. - Point contributors at the reference library: adding a platform or a signal usually means editing src/references/, not adding a skill. - Update Skill Tree Navigation, which still described three routers including sentry-sdk-setup (legacy) and sentry-workflow (now deleted). Record when hiding a skill behind a router pays for itself, so the single-child router doesn't come back. - Document the conventions this pass established: triggers live only in the description, skills end on a checkable completion criterion, prompt the target behaviour rather than the banned one. - Drop the "Skills use YAML frontmatter with allowed-tools" instruction. No shipped skill has ever carried it and no build or validator checks for it. Flagging rather than asserting: if Cursor does need it at runtime, the fix is to add it to the skills, not to keep an instruction nothing follows. README: drop the "Review code with Sentry context" example and the sentry-setup-ai-monitoring row, and fold sdk-upgrade into the core table now that it has no router. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A skill ships only what its references.yml declares, so a reference doc that links a sibling the manifest omits reads fine in this repo and dangles in every published plugin. 46 pointers were broken in the built output — the agent is told "see data-scrubbing.md" and finds nothing there. Two distinct causes: - `search-query-language.md` lives at references/ root, but six concepts/ docs linked it as a sibling. Fixed to `../search-query-language.md`. - The concepts/ docs cross-link each other, while the manifests hand-picked a subset. Each of the three manifests now takes `concepts/*.md` whole. The docs are small and hydration only puts files on disk — nothing is read until a pointer is followed — so presence costs nothing and SKILL.md still decides what gets read. The lean per-SDK scoping in sentry-get-started is unchanged. - sdks/index.md linked STRUCTURE.md, which is marked internal and deliberately never shipped. Dropped the pointer; the format contract is for maintainers, who read it in the repo. To keep it fixed, `check_skill_links` now runs at the end of all four plugin builds and fails on any Markdown link that doesn't resolve in the built tree. It runs last, against the plugin root, so breadcrumbs to ../../SKILL_TREE.md are checked where they actually land. Verified: all four builds pass, and the check fails as intended when a broken link is introduced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The skill covers the JavaScript SDK only — its own closing line says so, and it sends users elsewhere for Python, Ruby, or Go. The platform-generic name invited it to fire on "upgrade my Python Sentry SDK", where it has nothing to offer. This changes the name users type, so it is deliberately its own commit and can be dropped without losing the rest of the branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sergical
marked this pull request as ready for review
July 27, 2026 13:14
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
…mport The v9->v10 detection grepped for any `logger.info|log|warn|error` call site. That matches ordinary application loggers and `Sentry.logger.*` — the public structured logging API added in v9 — neither of which is renamed in v10. Only the internal `logger` export from `@sentry/core` becomes `debug`, so an agent acting on those matches could rewrite working logging calls. Key the detection off the import binding instead, and state in the v9-to-v10 reference that only the `@sentry/core` binding and its call sites get renamed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…g flow The recorded-values table lists the config file path as set in Steps 1 and 4, but Steps 6 and 7 both read it "from Step 1" only. In the no-config branch Step 1 never recorded a path — it just deferred creation to Step 4 — so an agent taking the fresh-`collector-config.yaml` route reaches validate and run with nothing recorded to read. Record the planned path in Step 1's no-config branch, have Step 4 reconcile the recorded value with what it actually wrote, and point Steps 6 and 7 at Steps 1 and 4 to match the table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ec3ed85. Configure here.
…k-gate Two gaps the record-and-reuse protocol and the ai-monitoring fold left behind. otel-exporter-setup: the recorded-values table lists the collector path *and* numeric version as set in Step 2, but Step 2 only told the agent to record the path. Steps 6 and 7 build validate and `docker run` from `<numeric_version>` and have no other source for it. Record it alongside the path, and add an end-of-step check covering the fresh-install route as well as the existing-collector one. ai-monitoring: folding sentry-setup-ai-monitoring carried over the tracesSampler patterns but dropped its Sampling Check gate, which read the current rate and asked before raising it. PII in the same doc still asks first; sampling did not, so an agent could raise a user's trace volume unprompted. Restore the detection greps and the ask. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
|
Three things I left alone that need your call:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reviewed all shipped skills against the skill-authoring principles and acted on what turned up.
sentry-get-started/sentry-instrument/sentry-debug-issueplusreferences.ymlhydration are the good new shape; most of this retires the pre-refactor half.1.
sentry-setup-ai-monitoringfolded intosentry-instrument(508b99e)Three sources of truth: the skill,
concepts/ai-monitoring.md, andsdks/*/ai-monitoring.md— the platform references now cover it in more depth. Also a trigger collision:sentry-instrumentclaims AI/LLM monitoring, but the skill sat behind thesentry-feature-setuprouter, so the same request resolved differently depending on the door.Kept the
tracesSamplerpatterns — every platform's Sampling Strategy section was a stub, andsdks/STRUCTURE.mdbars SDK references from carrying strategy. Also fixes a bug in every plugin: the Laravel section pointed at a top-levelreferences/path no build copies (b605c3c).2.
sentry-workflowrouter deleted (198454e)It fronted one skill, its description reworded. The router layer was sized for 30+ skills; eight ship today, and the 26 the docs described live in
skills-legacy/, built by nothing. Also: the generator skips empty categories, the dangling "Review code" menu option is gone (7141b95 removed its skills), and the tree header stops duplicatingsentry-get-startedStep 0.3. Hygiene sweep (
81b59b7)Restated "Invoke This Skill When" sections deleted — the body is only read after the skill fires.
sentry-otel-exporter-setup: target phrasing replaces the "DO NOT say" list, contradictory step threading fixed, the buried validate gate promoted to its own step, verification ends on telemetry seen in Sentry.sentry-js-sdk-upgraderecommended "v9 as of this writing" while this repo's SDK references document@sentry/node10.61+; it now resolves the latest major at run time rather than hardcoding one that goes stale. I did not invent v10 breaking changes — that reference says plainly it's incomplete.4. Every reference pointer resolves (
dceee65)46 dangled in the built output. Each manifest now takes
concepts/*.mdwhole; hydration only puts files on disk, so presence is free.check_skill_linksfails any unresolved link across all four builds — verified by introducing one.Also:
AGENTS.md+ README refreshed (cf75b10) — symlinked asCLAUDE.md, so it loads every session and described a library that no longer exists. Renamed tosentry-js-sdk-upgrade(c089add) — a JS-only skill under a platform-generic name invited firing on "upgrade my Python SDK"; last commit, droppable alone. Review fixes (c545b25,ec3ed85,f3c524f) scoped v10 logger detection to the@sentry/coreimport, threaded the OTel config path and collector version through the steps reading them, and restored the AI sampling ask-gate the fold dropped.Three open questions in a comment below.
🤖 Generated with Claude Code