feat(skills-editor): skills editor with MCP governance and accessibility hardening#397
Closed
anfibiacreativa wants to merge 22 commits into
Closed
feat(skills-editor): skills editor with MCP governance and accessibility hardening#397anfibiacreativa wants to merge 22 commits into
anfibiacreativa wants to merge 22 commits into
Conversation
ARIA tablist with keyboard nav, S2 tokens, tab-change event. Made-with: Cursor
Heading, subheading, badge and actions slots, S2 tokens. Made-with: Cursor
…ation Port skills lab from exp-workspace to nx2 with S2 tokens, semantic ARIA markup, and non-blocking data loading. Wire into canvas after-panel. Made-with: Cursor
- rename "handoff" → "suggestion" throughout (state, CSS, events, sessionStorage key) - fix SVG node sharing: store outerHTML string, render via unsafeHTML so each card gets its own node - fix timer leak: store _statusTimer, clear on disconnect and on re-call - add error handling to _onDeleteSkill and _onDeletePrompt - remove dead dynamic import of registerMcpServer (already statically imported) - declare _loadedKey and _statusTimer in constructor (no more implicit properties) - extract STATUS constants, replace all 'approved'/'draft' magic strings - fix textarea flex-fill: remove calc(100vh) hack, propagate flex through .textarea-wrap - remove redundant margin-top on .tool-row (parent gap already provides spacing) - use S2 checkmark SVG via loadHrefSvg for approved status icon (CSS circles) Made-with: Cursor
… sync - Full MCP editor with built-in servers, enable/disable toggles, edit mode - Generated tools panel with web worker sandbox execution and Try It panel - Dual-write skills to both .md file and config sheet on save/delete - Sync mechanism merges .md body with config sheet metadata on load - Replace loadHrefSvg/unsafeHTML with <img> tags for checkmark icons
Convert Skills Lab from a three-panel to a master-detail layout — slide-in flex drawer, per-tab dirty-form tracking, browser history + sessionStorage persistence, tools selector checkbox fix, and all lint errors resolved.
…tab removed Prompts - Show row action buttons always (remove opacity:0 hover reveal) - Add Delete button to each row; remove Associated Tools from editor form MCPs - Clicking a card row now opens the editor (was a no-op for custom MCPs) - Editor title shows 'Edit: <key>' vs 'Register MCP Server' — inline 'Editing XYZ' hint removed; _clearForm() now also resets MCP state so the label can't linger when switching tabs or clicking Register - Add Description field (textarea-sm) saved to config sheet Skills editor regression - _clearForm() was not clearing _editingMcpKey/_mcpKey/_mcpUrl; after visiting MCPs, switching to Skills and clicking a skill silently failed to open the editor — now fixed Generated Tools tab - Remove 'generated' from CATALOG_TABS; tab is no longer reachable Memory - Editor body gets .editor-body-memory class → justify-content:flex-end pushes memory cont the bottom of the viewport
…d code
API layer:
- deleteSkillMdFile now returns { ok, status } instead of void; callers
can detect failure (404 treated as success)
- loadAgentPresets spreads preset fields flat ({ id, ...preset }) so
_renderAgentCard reads label/description/tools without a .preset hop
- loadAgentPresets and fetchMcpToolsFromAgent fire-and-forget chains
now have .catch() to prevent unhandled rejection warnings
Naming conventions (boolean Lit reactive properties):
- _loading → _isLoading
- _suggestion → _hasSuggestion
- _editorOpen → _isEditorOpen
- _formDirty → _isFormDirty
- _saveBusy → _isSaveBusy
- _agentViewTools → _isAgentViewTools
- _formIsEdit → _isFormEdit
- _formPromptIsEdit → _isFormPromptEdit
CSS state modifier classprefix:
- drawer-open → is-drawer-open
- suggestion-wrap → is-suggestion
- dot-active → is-dot-active
- dot-inactive → is-dot-inactive
Correctness:
- Skill ID input now calls _markDirty() on change
- _loadMemory uses null for fetch error vs '' for empty file; renderer
handles the distinction correctly
- inert attribute added to .col-editor when drawer is closed
- _onToggleMcpEnabled redundant APPROVED double-check removed
- Dead tab === 'generated' branch removed from _renderListCol()
Polish:
- .card-menu-delete removed; uses .card-menu specificity
- Single-letter vars renamed: q→searchQuery/toolFilter, s→server,
t→tool, f→status
- 12px font sizes replaced with var(--s2-body-size-xs) token
- Badge padding 2px 8px replaced with spacing tokens
…I contract
Data integrity:
- Sequence skill writes (file first, then config) so a failing config
write never leaves an .md file without a config entry and vice versa
- Sequence skill deletes (file first, then config) for the same reason
- Prompt CRUD callers updated to use the new ok-shape consistently
API contract (fetchDaConfigSheets / all mutators):
- fetchDaConfigSheets now returns ok:false on 401 instead of EMPTY_CONFIG
(ok:true); callers can no longer mistake an auth failure for empty data
- All exported mutators (upsertSkillInConfig, deleteSkillFromConfig,
upsertPromptInConfig, deletePromptFromConfig, upsertGeneratedTool,
deleteGeneratedTool) now return { ok, error?, status? } consistently
- AUTH_FAIL sentinel extracted to avoid duplicating the 401 return shape
- boot.finally() now has .catch() to prevent unhandled rejection risk
- fetchDaConfigSheets bare catch now surfaces err.message in the return
- Single-letter vars renamed: o→payload, k/u→serverKey/serverUrl, def→toolDef
Stale async guards:
- _onEditSkill: capture skillId and tab at request time; discard response
if user navigated away before the readSkillMdFile fetch resolved
- _reload fire-and-forgets: loadAgentPresets and fetchMcpToolsFromAgent
now check the load key before writing state, preventing late responses
from a previous org/site from overwriting the current view
Form dirty tracking:
- Tool checkbox changes in _renderAssociatedToolsSelector now call
_markDirty() so tool selection edits are tracked and persisted
Focus management (keyboard accessibility):
- Opening the drawer moves focus to the first interactive element inside
- Closing the drawer restores focus to the element that triggered the open
- _editorTrigger stored on each open entry point (openEditor, onEditSkill,
openNewMcpEdetc.) and cleared after focus restoration
Filter state:
- _catalogFilter reset to 'all' on every tab change and back/forward
navigation; it was previously leaking across unrelated tabs
Misc:
- _renderAgentCard parameter renamed a→agent; tool list item t→tool
- _renderAgentsCatalog map var a→agent
…state, CSS deduplication
Critical fixes:
- Add compensating rollback to _onSaveSkill (delete orphan file if config save fails)
and _onDeleteSkill/_onDeleteSkillById (re-create file if config delete fails)
- Fix writeSkillMdFile to return { ok: false, error } on validation failure
- Replace raw _editorTrigger element reference with stable CSS selector (_editorTriggerSelector)
Simplification:
- Extract upsertSheetRow/deleteSheetRow generic helpers in skills-editor-api.js; all
config CRUD mutators now delegate to them, eliminating copy-pasted load/mutate/save flows
- Add FRESH_FORM_STATE constant; _clearForm, _captureForm, _restoreForm are now
tab-agnostic — no more per-mode branching when adding w field
- Add TAB_ACTIONS config map; list header renders one data-driven new-button instead of
four conditional blocks; _editorTitle() extracts drawer title logic from the template
- Extract .btn-icon CSS primitive; .close-btn, .more-btn, .row-action-btn now declare
only size/color overrides
Chat/event compatibility:
- Support both legacy (da-skills-lab-*) and new (da-skills-editor-*) event/sessionStorage
namespaces so suggestion handoff and prompt-to-chat work during the migration window
- chat.js listens for both prompt-add-to-chat and prompt-send families with dedup guard
Prompt edit parity:
- Track _formPromptOriginalTitle so prompt rename upserts the correct row
- Track _formPromptIcon so the icon field is preserved on save
- Restore Add to Chat button in prompt rows and editor footer
Agent parity:
- Store and render aut ignored in UI)
- loadAgentPresets() handles both label/tools and name/mcpServers preset shapes
- New Agent path now creates /.da/agents/{id}.json via saveAgentPresetFile()
- Dedicated _renderAgentForm() / _onSaveAgent() with id, name, and starter preset
Generated Tools tab remains removed (unreachable by design); data still loads.
Add MCP tool listing and per-tool enable/disable controls with config persistence, pass MCP auth headers through chat/agent requests, and stabilize the tree throttle test timing assertion.
…cessibility Normalize sheet boolean parsing for tool overrides and MCP enablement, make MCP cards keyboard-activatable, and stabilize unit tests against current skills-editor behavior.
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
|
aem-code-sync
Bot
temporarily deployed
to
feat/ew-skills-editor-recovered
anfibiacreativa
requested review from
hannessolo,
kozmaadrian,
mhaack and
sharanyavinod
Guard MCP card click/keydown activation so nested menu buttons do not trigger card open, and add regression tests for keyboard and click bubbling behavior.
Drop nx2/img/adobe.svg since it is not required for the skills-editor workstream.
…nd refresh Render skills-editor immediately from per-site session snapshot, refresh data in the background with an auto-refresh progress indicator, lazy-load agents/MCP tools by tab, and keep orphan-skill sync off the critical path.
aem-code-sync
Bot
temporarily deployed
to
feat/ew-skills-editor-recovered
Member
Author
|
Closed in favor of #398 |
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.
Summary
nx-skills-editorexperience with a master-detail layout for Skills, Prompts, Agents, MCPs, and Memory.Why
What Changed
nx2/blocks/shared/card/*reusable card component.nx2/blocks/shared/tabs/*reusable tabs component.nx2/blocks/skills-editor/*new Skills Editor block + API module.mcpServersandmcpServerHeadersin agent requests.da-skills-lab-*andda-skills-editor-*namespaces).test/utils/tree.test.js.nx2/test/unit/skills-editor/nx-skills-editor.test.js.Test Plan
npm run test:unit -- nx2/test/unit/skills-editor/nx-skills-editor.test.jsda-nxunit suite to verify shared card/tabs integration.tests/playwright-e2ebranch (keyboard/a11y and regression paths).e2e test live here -> https://github.com/adobe/da-live/compare/tests/playwright-e2e?expand=1
Manual:
Risks / Follow-ups
nx-skills-editordoes not mount; add clearer readiness skip messaging if needed. These tests are in da-live. Non-blocking to merge PRMust be integrated together with:
adobe-rnd/da-agent#23
Stage URLs:
https://da.live/apps/skills?nx=feat-ew-skills-editor-recovered&nxver=2#/exp-workspace/frescopa
https://da.live/canvas?nx=feat-ew-skills-editor-recovered&nxver=2#/exp-workspace/frescopa/index