Skip to content

Fix agent-skill directive: copy install command + learn more link#3392

Open
leemthompo wants to merge 8 commits into
elastic:mainfrom
leemthompo:fix/agent-skill-copy-install-command
Open

Fix agent-skill directive: copy install command + learn more link#3392
leemthompo wants to merge 8 commits into
elastic:mainfrom
leemthompo:fix/agent-skill-copy-install-command

Conversation

@leemthompo
Copy link
Copy Markdown
Member

@leemthompo leemthompo commented May 25, 2026

Summary

Fixes elastic/docs-content-internal#1206.

The {agent-skill} banner had three UX problems: the "Get the skill" button linked to a URL that 404'd (the @skill-name syntax isn't a valid GitHub path), there was no direct way to install a skill, and the "Learn more" link pointed back to the current page on some docs pages.

This PR:

  • Replaces the broken link button with a "Copy install command" button that copies npx skills add @skill-name to clipboard, extracted from the existing :url: property
  • Points the "Learn more" link to the #available-skills anchor on the agent skills overview page
  • Falls back to the original link behavior when no @skill-name is present in the URL

Test plan

  • Serve docs locally (dotnet run --project src/tooling/docs-builder -- serve --path docs) and navigate to the agent skill syntax page
  • Click "Copy install command" — verify npx skills add @elasticsearch-esql is copied to clipboard
  • Verify the button shows "Copied!" feedback briefly then resets
  • Verify "Learn more about agent skills for Elastic" links to /explore-analyze/ai-features/agent-skills#available-skills
  • All 25 AgentSkill unit tests pass

🤖 Generated with Claude Code

Demo

Screen.Recording.2026-05-25.at.14.41.51.mov

leemthompo and others added 3 commits May 25, 2026 14:49
The link was resolving to the current page on some docs pages instead
of the agent skills overview. Anchor to the correct section.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The "Get the skill" button linked to a URL that 404'd because the
@skill-name syntax isn't a valid GitHub path. Replace it with a button
that copies the actual install command (npx skills add @skill-name) to
clipboard. Falls back to the old link when no skill name is present.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cover SkillName extraction, InstallCommand generation, copy button
rendering, and the fallback to a link when the URL has no @skill-name.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@leemthompo leemthompo requested review from a team as code owners May 25, 2026 12:50
@leemthompo leemthompo requested a review from cotti May 25, 2026 12:50
@leemthompo leemthompo marked this pull request as draft May 25, 2026 12:52
@leemthompo leemthompo self-assigned this May 25, 2026
@leemthompo leemthompo added the fix label May 25, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 25, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds a "Copy install command" button to the agent-skill directive. When a skill URL contains an @skill-name segment, the directive extracts the name, generates an npx skills add @{skill-name} command, and renders a button that copies the command to the clipboard with transient "Copied!" feedback. If the URL lacks a skill name, it falls back to the existing "Get the skill" link. The implementation spans the directive model layer, view model wiring, Razor view, frontend TypeScript initialization, CSS styling for button states, and comprehensive tests covering both paths.

Sequence Diagram

sequenceDiagram
  participant Directive as Directive Parser
  participant ViewModel as View Model
  participant View as Razor View
  participant DOM as DOM/Button
  participant User as User
  participant Clipboard as navigator.clipboard

  Directive->>Directive: Extract skill name from `@segment`
  Directive->>ViewModel: InstallCommand = npx skills add `@name`
  ViewModel->>View: Render with InstallCommand
  alt Has Install Command
    View->>DOM: Render copy button + data-copy-text
  else No Install Command
    View->>DOM: Render fallback link
  end
  User->>DOM: Click copy button
  DOM->>Clipboard: writeText(InstallCommand)
  Clipboard-->>DOM: Success
  DOM->>DOM: Show "Copied!" + green background
  Note over DOM: 1.5 second timeout
  DOM->>DOM: Restore original button state
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main changes: adding a copy install command feature and fixing the learn more link in the agent-skill directive.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description clearly relates to the changeset, explaining the UX fixes and implementation details for the agent-skill directive.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai Bot previously requested changes May 25, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/syntax/agent-skill.md`:
- Line 67: Update the sentence that reads "The skill name is extracted from the
URL as the segment after `@`" to explicitly state that the parser uses the final
`@` in the URL (e.g., change to "after the final `@`") so URLs with multiple `@`
characters are parsed correctly; locate the phrase "The skill name is extracted
from the URL as the segment after `@`" (and the accompanying example
`https://github.com/elastic/agent-skills@elasticsearch-esql`) and replace it
with wording that clarifies extraction uses the final `@`.

In `@src/Elastic.Markdown/Myst/Directives/AgentSkill/AgentSkillBlock.cs`:
- Around line 27-38: ExtractSkillName currently returns the substring after the
final '@' including any URL fragment, query or trailing slash; update
ExtractSkillName so after computing name = url[(atIndex + 1)..] you first remove
any fragment or query by truncating at the first '#' or '?' if present, then
TrimEnd('/') to drop trailing slashes, and finally return null if the resulting
name is empty; this ensures SkillName (used where SkillName is assigned)
contains only the clean skill identifier.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0317685e-4d29-4d66-a43d-57542addaf27

📥 Commits

Reviewing files that changed from the base of the PR and between 477a319 and af51948.

📒 Files selected for processing (9)
  • docs/syntax/agent-skill.md
  • src/Elastic.Documentation.Site/Assets/agent-skill.ts
  • src/Elastic.Documentation.Site/Assets/main.ts
  • src/Elastic.Documentation.Site/Assets/markdown/agent-skill.css
  • src/Elastic.Markdown/Myst/Directives/AgentSkill/AgentSkillBlock.cs
  • src/Elastic.Markdown/Myst/Directives/AgentSkill/AgentSkillView.cshtml
  • src/Elastic.Markdown/Myst/Directives/AgentSkill/AgentSkillViewModel.cs
  • src/Elastic.Markdown/Myst/Directives/DirectiveHtmlRenderer.cs
  • tests/Elastic.Markdown.Tests/Directives/AgentSkillTests.cs

Comment thread docs/syntax/agent-skill.md Outdated
Comment thread src/Elastic.Markdown/Myst/Directives/AgentSkill/AgentSkillBlock.cs Outdated
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Parse from Uri.AbsolutePath instead of the raw URL string so fragments
(#readme) and query params don't leak into the skill name.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@leemthompo
Copy link
Copy Markdown
Member Author

Both CodeRabbit findings addressed in 01436e9:

  • ExtractSkillName now parses from Uri.AbsolutePath instead of the raw string, stripping fragments/query/trailing slashes
  • Docs updated to say "after the final @"

@leemthompo
Copy link
Copy Markdown
Member Author

@theletterf I think this might be a GitHub Actions permissions error?

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fork PRs cannot use deployment APIs or AWS OIDC credentials. Skip
assembler-preview and integration when the head branch is on a fork,
matching the pattern used in docs-preview-local.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@leemthompo leemthompo marked this pull request as ready for review May 26, 2026 11:45
Copy link
Copy Markdown

@wildemat wildemat left a comment

Choose a reason for hiding this comment

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

Looks great thanks Liam. Addresses complaints raised as part of reference search onboarding skill in docs starter guides

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants