Skip to content

feat(site/src/pages/AgentsPage): add personal skills slash menu#25386

Merged
ibetitsmike merged 1 commit into
mainfrom
mike/personal-skills-slash-menu
May 21, 2026
Merged

feat(site/src/pages/AgentsPage): add personal skills slash menu#25386
ibetitsmike merged 1 commit into
mainfrom
mike/personal-skills-slash-menu

Conversation

@ibetitsmike
Copy link
Copy Markdown
Collaborator

@ibetitsmike ibetitsmike commented May 15, 2026

Mux updated this PR on behalf of Mike.

Context

PR #25066 has merged. This branch is rebased onto main and now contains only the personal skills slash menu UI changes.

Summary

  • Add a / slash-trigger menu in the agent chat composer that filters personal skills by name and description.
  • Insert /<skill-name> on click, Enter, or Tab selection while preserving normal composer behavior when the menu is closed.
  • Keep Escape dismissal and post-selection suppression scoped to the current slash trigger, with menu anchor refresh on editor scroll and resize.
  • Share personal skill trigger formatting and parsing helpers with unit coverage.
  • Add Storybook coverage for open, filter, click, keyboard selection, Escape, error, empty, and filtered-empty states.

Validation

  • pre-commit hook
  • cd site && pnpm exec vitest run --project=unit src/pages/AgentsPage/components/ChatMessageInput/ChatMessageInput.test.tsx src/pages/AgentsPage/utils/personalSkills.test.ts
  • cd site && pnpm lint:types
  • cd site && pnpm lint:check

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

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: c70f11b799

ℹ️ 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 site/src/pages/AgentsPage/components/ChatMessageInput/SkillsTriggerPlugin.tsx Outdated
Copy link
Copy Markdown
Collaborator Author

ibetitsmike commented May 15, 2026

@ibetitsmike
Copy link
Copy Markdown
Collaborator Author

/coder-agents-review

Copy link
Copy Markdown
Contributor

@coder-agents-review coder-agents-review Bot left a comment

Choose a reason for hiding this comment

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

Well-structured feature: the slash-trigger detection, menu rendering, and filter logic are cleanly separated, and the story coverage is unusually thorough for a chat input plugin. The double-selection guard via hasSelectedRef, the replacement safety checks in replaceActiveSkillsTrigger, and the lazy enabled: skillsMenuOpen query gating are all solid.

1 P1, 2 P2, 7 P3, 6 Nit, 2 Note posted inline.

The P1 (cmdk double-highlight after arrow navigation) and two P2s (Enter submitting during loading, LexicalTypeaheadMenuPlugin reimplementation) need attention before merge. The cmdk issue is both visual and accessibility: aria-selected diverges from the actual keyboard selection.

Process note: commit 0c75e9cb6 ("fix personal skills slash menu selection") bundles two unrelated changes (drop description from trigger text + add hasSelectedRef guard). A commit per concern would help bisect.

"The menu presents itself as a modal interaction (blocking arrows, showing a popover), but Enter punches through the floor." Hisoka

🤖 This review was automatically generated with Coder Agents.

Comment thread site/src/pages/AgentsPage/components/ChatMessageInput/SkillsTriggerPlugin.tsx Outdated
Comment thread site/src/pages/AgentsPage/components/ChatMessageInput/SkillsTriggerPlugin.tsx Outdated
Comment thread site/src/pages/AgentsPage/components/ChatMessageInput/SkillsTriggerPlugin.tsx Outdated
Comment thread site/src/pages/AgentsPage/components/ChatMessageInput/ChatMessageInput.test.tsx Outdated
@ibetitsmike
Copy link
Copy Markdown
Collaborator Author

/coder-agents-review

1 similar comment
@ibetitsmike
Copy link
Copy Markdown
Collaborator Author

/coder-agents-review

Copy link
Copy Markdown
Contributor

@coder-agents-review coder-agents-review Bot left a comment

Choose a reason for hiding this comment

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

18 of 20 R1 findings addressed. The P1 cmdk double-highlight fix, both P2 fixes (Enter swallow, scroll/resize anchor refresh), sticky Escape dismissal, useEffectEvent refactor, filterPersonalSkills extraction, and all Nits are verified.

DEREM-7 (LexicalTypeaheadMenuPlugin): Panel closed 4/4 in favor of the author. Pariston read the library source and found its rendering contract incompatible with the project's Popover+cmdk pattern. Mafuuu, Nami, and Meruem independently verified every concrete bug is fixed and the maintenance cost is bounded.

1 P0 (failing test, likely the CI test-js failure), 2 P3 new.

"The integration constraints justify the custom code. The custom plugin is well-tested via 12 story-based interaction tests that exercise the keyboard paths." Pariston

🤖 This review was automatically generated with Coder Agents.

Comment thread site/src/pages/AgentsPage/utils/personalSkills.test.ts
@ibetitsmike
Copy link
Copy Markdown
Collaborator Author

/coder-agents-review

Copy link
Copy Markdown
Contributor

@coder-agents-review coder-agents-review Bot left a comment

Choose a reason for hiding this comment

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

All 26 prior findings resolved. R3 Netero scan clean. Panel (Bisky, Mafuuu, Kite) found one P3.

The feature is well-decomposed: trigger detection in SkillsTriggerPlugin, menu rendering in PersonalSkillsTriggerMenu, filtering in personalSkills.ts, state in ChatMessageInput. The useEffectEvent usage, controlled cmdk value prop, Enter/Tab behavioral split, sticky Escape dismissal, and scroll/resize anchor refresh are all verified working. 13 Storybook stories with play functions cover the full interaction matrix.

Approving with one minor P3 inline.

🤖 This review was automatically generated with Coder Agents.

Comment thread site/src/pages/AgentsPage/utils/personalSkills.test.ts
@ibetitsmike
Copy link
Copy Markdown
Collaborator Author

/coder-agents-review

Copy link
Copy Markdown
Contributor

@coder-agents-review coder-agents-review Bot left a comment

Choose a reason for hiding this comment

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

All 27 findings resolved across 4 rounds. Netero clean. Panel (Bisky, Mafuuu, Chopper) clean. No open findings.

27 findings raised, 22 author-fixed, 1 contested and panel-closed (4/4), 4 dropped by orchestrator. Zero regressions introduced across 4 fix commits.

🤖 This review was automatically generated with Coder Agents.

ibetitsmike added a commit that referenced this pull request May 16, 2026
> Mux updated this PR on behalf of Mike.

## Stack Context

This stack splits experimental personal skills into smaller reviewable
PRs. Personal skills are user-owned `SKILL.md` files stored by Coder and
injected into chatd alongside workspace skills.

Stack order:
1. #25362 personal skill resolver
2. #25363 storage, permissions, API, and SDK
3. #25365 API test coverage
4. #25366 chattool and chatd integration
5. #25066 settings UI and docs
6. #25386 personal skills slash menu

## What?

Adds the shared personal skill parser and resolver package, plus
reusable skill-name validation exported from `workspacesdk`.

The parser enforces the full personal skill contract: max raw size,
kebab-case name, max name length, and non-empty body.

## Why?

The rest of the stack needs one source-aware resolver for personal and
workspace skills, including collision handling and qualified aliases.
Keeping personal skill constraints in the parser prevents callers from
accidentally parsing invalid personal skills.

## Validation

- `go test ./coderd/x/skills ./codersdk/workspacesdk`
- pre-commit hooks on this branch
@ibetitsmike ibetitsmike force-pushed the mike/personal-skills-slash-menu branch from 5cf6084 to c77ce40 Compare May 19, 2026 15:38
@ibetitsmike ibetitsmike force-pushed the mike/personal-skills-ui branch from a6f2ec8 to 5f36739 Compare May 19, 2026 15:38
Copy link
Copy Markdown
Collaborator Author

ibetitsmike commented May 19, 2026

Dogfooded this against a real dev build (./scripts/develop.sh, web :8080) at head c77ce40d. 16/18 behavioral checks green, but one real bug worth flagging before merge.

Bug: Escape on the open menu blurs the editor

Repro:

  1. Focus the chat composer on /agents.
  2. Type / so the menu opens.
  3. Press Escape. Menu closes correctly.
  4. document.activeElement is now <body> — the contenteditable lost focus.
  5. Subsequent keystrokes don't reach the editor; user has to click the composer again.

Observed in both Google Chrome and Playwright's bundled chromium_headless_shell, so it's not a CDP quirk. As a side-effect, the "Escape dismissal is sticky for the current /" behavior promised in the PR description can't be exercised in a real browser, because the next keystroke never reaches the editor.

Why Storybook is green

EscapeClosesWithoutReplacing passes because storybook/test's userEvent.keyboard("{Escape}") dispatches synthesized events without performing real focus changes through CDP, so it never hits the path that loses focus. The story silently does not exercise the focus contract it appears to test.

Likely cause

SkillsTriggerPlugin.tsx (handleEscape, ~L212) calls event.preventDefault() and returns true from KEY_ESCAPE_COMMAND, but Escape still reaches Radix's DismissableLayer, whose dismiss path blurs the active element. onCloseAutoFocus={(e) => e.preventDefault()} on the PopoverContent only suppresses the return focus — it doesn't restore focus to the editor.

Sketch (un-validated):

const handleEscape = useEffectEvent((event: KeyboardEvent) => {
  if (!open) return false;
  event.preventDefault();
  dismissedTriggerRef.current = editor
    .getEditorState()
    .read(() => activeTriggerFromSelection());
  onTriggerChange(null);
  queueMicrotask(() => editor.focus()); // defer past Radix's blur
  return true;
});

If this is the right shape, the Storybook story should also be augmented with a real-browser interaction check (e.g., a Playwright story or @playwright/test spec) so the regression is actually catchable in CI.

Reported by Coder Agents (generated). Full dogfood notes + screenshots available on request.

@ibetitsmike ibetitsmike force-pushed the mike/personal-skills-ui branch from 5f36739 to 4f2cddf Compare May 19, 2026 16:10
@ibetitsmike ibetitsmike force-pushed the mike/personal-skills-slash-menu branch from c77ce40 to d35a725 Compare May 19, 2026 16:10
ibetitsmike added a commit that referenced this pull request May 19, 2026
> Mux updated this PR on behalf of Mike.

## Stack Context

This PR is the storage, permissions, API, and SDK layer for experimental
personal skills. #25362 has landed on `main`, so this branch is
restacked directly on `main`.

Stack order:
1. #25363 storage, permissions, API, and SDK
2. #25365 API test coverage
3. #25366 chattool and chatd integration
4. #25066 settings UI and docs
5. #25386 personal skills slash menu

## What?

Adds the `user_skills` database table, generated queries, RBAC resources
and scopes, audit resource handling, experimental user-scoped CRUD
endpoints, SDK types, and generated API/site types.

Follow-up review and restack fixes:
- Enforce a bounded personal skill description in parser and database
constraints.
- Return `403 Forbidden` for unauthorized create and update attempts.
- Return explicit conflict responses when soft-deleted users are
targeted.
- Keep user admins out of personal skills, while site owners can read
and delete but not create or update.
- Document trigger-raised constraint names and keep schema constants
covered by tests.
- Reuse `UserSkillMetadata` in the full `UserSkill` SDK response type.
- Generate user skill IDs in Go instead of relying on a database
default.
- Rebase on latest `main` and renumber the user skills migration to
`000502_user_skills`.

## Why?

Personal skills need durable user-owned storage with owner
authorization, limited site-owner moderation, and a hidden API surface
before chatd can consume them.

## Validation

- `make gen`
- `go test ./coderd/database -run '^TestUserSkillSchemaConstants$'
-count=1`
- `go test ./coderd/database/dbauthz -run
'^TestMethodTestSuite/TestUserSkills$' -count=1`
- `go test ./coderd -run '^TestPatchUserSkill$' -count=1`
- `go test ./codersdk ./coderd/database/db2sdk`
- `make lint`
- pre-commit hook on `97fd58108d`
@ibetitsmike ibetitsmike force-pushed the mike/personal-skills-ui branch from 4f2cddf to c96c28a Compare May 19, 2026 23:16
@ibetitsmike ibetitsmike requested a review from Emyrk as a code owner May 19, 2026 23:16
@ibetitsmike ibetitsmike force-pushed the mike/personal-skills-slash-menu branch from 86d3de7 to 41f8166 Compare May 19, 2026 23:17
@ibetitsmike ibetitsmike force-pushed the mike/personal-skills-ui branch from c96c28a to d702fb2 Compare May 20, 2026 00:44
@ibetitsmike ibetitsmike force-pushed the mike/personal-skills-slash-menu branch from 41f8166 to 03f1cb1 Compare May 20, 2026 00:45
ibetitsmike added a commit that referenced this pull request May 20, 2026
> Mux updated this PR on behalf of Mike.

## Stack Context

This PR is the API test coverage slice in the experimental personal
skills stack. The storage, schema, permissions, API, and SDK
implementation merged in #25363.

Stack order:
1. #25362 personal skill resolver
2. #25363 storage, permissions, API, and SDK
3. #25365 API test coverage
4. #25366 chattool and chatd integration
5. #25066 settings UI and docs
6. #25386 personal skills slash menu

## What?

Adds API and audit tests for personal skill CRUD, validation failures,
limits, authorization, soft-delete cleanup, and audit content tracking.

This PR is now test-only. It does not include migrations, generated
database code, or API implementation changes.

## Why?

The feature touches storage, permissions, and audit behavior. These
tests make the server behavior reviewable and protected without
re-reviewing the implementation that already merged in #25363.

## Validation

- `go test ./coderd -run '^(TestUserSkill|TestPatchUserSkill)' -count=1`
- `go test ./enterprise/coderd -run
'^TestUserSkillAuditDiffTracksContent$' -count=1`
- pre-commit hook via `gt modify --no-edit`
@ibetitsmike
Copy link
Copy Markdown
Collaborator Author

/coder-agents-review

Copy link
Copy Markdown
Contributor

@coder-agents-review coder-agents-review Bot left a comment

Choose a reason for hiding this comment

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

Good progress on the rebase. The author-reported Escape blur fix is verified in code (handleEscape now calls event.stopPropagation() + queueMicrotask(() => editor.focus())).

1 P2, 1 P4 new. The P2 is a structural issue: skill selection does not suppress the trigger re-detection that Escape suppresses via dismissedTriggerRef.

The code refactoring since R4 (extracted parsePersonalSkillTrigger, isPersonalSkillTriggerToken, isSameSkillsTrigger, removed hasSelectedRef) is clean. Netero found no mechanical issues.

"Escape sets dismissedTriggerRef.current before calling onTriggerChange(null), so refreshTrigger suppresses the re-detected trigger. Selection paths do not set the dismissed ref." Mafuuu

🤖 This review was automatically generated with Coder Agents.

@ibetitsmike
Copy link
Copy Markdown
Collaborator Author

/coder-agents-review

Copy link
Copy Markdown
Contributor

@coder-agents-review coder-agents-review Bot left a comment

Choose a reason for hiding this comment

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

All 29 findings resolved across 6 rounds. Netero clean. Panel (Mafuuu, Bisky, Razor) clean.

DEREM-28 fix verified by Mafuuu (who raised it): suppressedSkillsTriggerRef stores the trigger location before replacement. When Lexical's microtask-deferred update listener re-detects the replacement text, handleSkillsTriggerChange matches against the suppressed location and discards the re-trigger. The mechanism is timing-safe and one-shot (cleared on next trigger change).

29 findings raised, 24 author-fixed, 1 contested and panel-closed (4/4), 4 dropped by orchestrator. Zero open. Clean approval.

"Traced all four modes against the full diff. The dual-suppression design (dismissedTriggerRef for Escape, suppressedSkillsTriggerRef for selection) keeps their promises." Mafuuu

🤖 This review was automatically generated with Coder Agents.

ibetitsmike added a commit that referenced this pull request May 20, 2026
> Mux updated this PR on behalf of Mike.

## Stack Context

This PR builds on #25365 in the experimental personal skills stack and
completes the chat integration.

Stack order:
1. #25362 personal skill resolver
2. #25363 storage, permissions, API, and SDK
3. #25365 API test coverage
4. #25366 chattool and chatd integration
5. #25066 settings UI and docs
6. #25386 personal skills slash menu

## What?

Updates chattool skill formatting and `read_skill` resolution so tools
can read personal skills from the database, then injects personal skill
metadata into chatd prompts and registers the skill-reading tools when
skills are available.

This branch has also been merged with current `origin/main` to resolve
merge conflicts.

## Why?

The chattool and chatd changes need to land together so the intermediate
stack state stays buildable. This completes personal skill availability
in chats without syncing personal skills into workspace filesystems.

## Validation

- `go test -count=1 ./coderd/x/chatd/chattool -run
'TestFormatResolvedSkillIndex|TestReadSkillTool|TestReadSkillFileTool'`
- `go test -count=1 ./coderd/x/chatd -run
'TestPersonalSkillsInSystemPrompt|TestPersonalAndWorkspaceSkillCollisionInSystemPrompt|TestSkillIndexRefreshReplacesStaleAliases|TestFetchPersonalSkillMetadata|TestLoadPersonalSkillBody'`
- `go test -count=1 ./coderd -run 'Test.*UserSkill'`
- `git diff --cached --check`
- `make lint`
- pre-commit hook
Base automatically changed from mike/personal-skills-ui to main May 21, 2026 22:20
@ibetitsmike ibetitsmike force-pushed the mike/personal-skills-slash-menu branch from ec486ef to 795e45c Compare May 21, 2026 22:35
@ibetitsmike ibetitsmike merged commit fa9eb1a into main May 21, 2026
32 checks passed
@ibetitsmike ibetitsmike deleted the mike/personal-skills-slash-menu branch May 21, 2026 23:24
@github-actions github-actions Bot locked and limited conversation to collaborators May 21, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants