fix(skills): harden sharing accessibility

This commit is contained in:
Jinwoo-H
2026-08-12 09:00:25 -04:00
parent 34941ffc1f
commit 902aa36a7c
15 changed files with 291 additions and 38 deletions
@@ -908,13 +908,26 @@ does not mean the surrounding phase is complete.
### UX validation
- [ ] Test keyboard shortcuts and labels with platform-aware macOS versus Windows/Linux behavior.
- [x] Test keyboard shortcuts and labels with platform-aware macOS versus Windows/Linux behavior.
Skill sharing adds no modifier shortcut or shortcut label. Renderer coverage proves its
platform-neutral Escape navigation closes the page, leaves editable controls alone, and
remains owned by open dialogs; the compatibility scan found no hardcoded Meta-only binding,
macOS glyph, or platform-specific accelerator in the feature.
- [x] Test loading, error, cancellation, partial, conflict, and stale-preview states. Renderer
coverage includes scan loading, unavailable links, upload/install cancellation, partial
provider results, keep-local conflicts, capability loss after preview, and selection
reconciliation after a changed discovery scan; package tests cover source drift.
- [ ] Test screen-reader names, focus order, progress announcements, and destructive confirmations.
- [ ] Test long names, release notes, paths, organization names, and localized copy.
- [x] Test screen-reader names, focus order, progress announcements, and destructive confirmations.
The link field receives initial focus, Tab reaches the enabled primary action, Escape closes
the dialog, and machine, destination, WSL, and workspace selectors have programmatic names.
Upload progress exposes its phase and numeric value from 0 through publication; dialog-level
install tests announce authorization, installation, and current-skill position. Link revoke,
Cloud deletion, and local removal retain focus while switching to exact confirmation names.
- [x] Test long names, release notes, paths, organization names, and localized copy. Renderer
coverage exercises the 64-character bundle/skill-name boundary, a 10,000-character multiline
release note, long Unicode descriptions and organization IDs, long Windows-style skill paths,
and long workspace labels. Review copy wraps without losing full values, selectors constrain
labels to dialog width, and localization catalog, extraction, and coverage gates pass.
- [ ] Review trust and privacy wording with security and design.
## 9. Implement paired runtime installation
@@ -50,9 +50,13 @@ describe('OwnedSkillShareLinks', () => {
await user.click(screen.getByRole('button', { name: 'Copy link' }))
expect(writeClipboardText).toHaveBeenCalledWith('https://share.onorca.dev/skills/share/share_1')
await user.click(screen.getByRole('button', { name: 'Unshare' }))
const unshare = screen.getByRole('button', { name: 'Unshare' })
unshare.focus()
await user.click(unshare)
expect(revokeShare).not.toHaveBeenCalled()
await user.click(screen.getByRole('button', { name: 'Confirm unshare' }))
const confirm = screen.getByRole('button', { name: 'Confirm unshare' })
expect(confirm).toHaveFocus()
await user.click(confirm)
await waitFor(() => expect(revokeShare).toHaveBeenCalledWith('share_1'))
expect(screen.queryByText('team-skills')).not.toBeInTheDocument()
})
@@ -46,9 +46,11 @@ export function SkillBundleInstallReview(props: {
<div className="space-y-5">
<section className="space-y-2">
<div className="flex items-start justify-between gap-3">
<div>
<h3 className="text-sm font-semibold">{manifest.bundleName}</h3>
<p className="text-xs leading-5 text-muted-foreground">{manifest.description}</p>
<div className="min-w-0">
<h3 className="break-words text-sm font-semibold">{manifest.bundleName}</h3>
<p className="break-words text-xs leading-5 text-muted-foreground">
{manifest.description}
</p>
</div>
<Badge variant="outline">
{translate(
@@ -131,7 +133,7 @@ export function SkillBundleInstallReview(props: {
{manifest.bundleDigest}
</p>
{props.version.publisher ? (
<p className="text-xs text-muted-foreground">
<p className="break-words text-xs text-muted-foreground">
{translate(
'auto.components.skills.SkillBundleInstallReview.01c5a11e07',
'Published by Orca user'
@@ -148,7 +150,7 @@ export function SkillBundleInstallReview(props: {
</p>
) : null}
{props.version.releaseNotes.trim() ? (
<p className="text-xs leading-5 text-muted-foreground">
<p className="break-words whitespace-pre-wrap text-xs leading-5 text-muted-foreground">
{translate(
'auto.components.skills.SkillBundleInstallReview.01c5a11e09',
'Release notes:'
@@ -62,9 +62,13 @@ describe('SkillCloudManagementActions', () => {
const { skills } = setup()
expect(screen.getByText(/leaves installed copies unchanged/)).toBeTruthy()
expect(screen.queryByText('Access')).toBeNull()
fireEvent.click(screen.getByRole('button', { name: 'Unshare' }))
const unshare = screen.getByRole('button', { name: 'Unshare' })
unshare.focus()
fireEvent.click(unshare)
expect(skills.revokeShare).not.toHaveBeenCalled()
fireEvent.click(screen.getByRole('button', { name: 'Confirm unshare' }))
const confirm = screen.getByRole('button', { name: 'Confirm unshare' })
expect(document.activeElement).toBe(confirm)
fireEvent.click(confirm)
await waitFor(() => expect(skills.revokeShare).toHaveBeenCalledWith('share_1'))
})
@@ -1,8 +1,10 @@
// @vitest-environment happy-dom
import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { SkillCloudVersion } from '../../../../shared/skill-cloud-contract'
import type { SkillInstallProgress } from '../../../../shared/skill-sharing-contract'
import { useAppStore } from '@/store'
import { SkillInstallDialog } from './SkillInstallDialog'
@@ -80,9 +82,7 @@ function bundleVersion(): SkillCloudVersion {
}
function installApi(previewInstall: ReturnType<typeof vi.fn>) {
let progressListener:
| ((progress: { operationId: string; phase: 'authorizing' | 'installing' }) => void)
| null = null
let progressListener: ((progress: SkillInstallProgress) => void) | null = null
return {
resolveShare: vi
.fn()
@@ -108,8 +108,7 @@ function installApi(previewInstall: ReturnType<typeof vi.fn>) {
progressListener = null
}
}),
emitInstallProgress: (progress: { operationId: string; phase: 'authorizing' | 'installing' }) =>
progressListener?.(progress)
emitInstallProgress: (progress: SkillInstallProgress) => progressListener?.(progress)
}
}
@@ -140,6 +139,74 @@ afterEach(() => {
})
describe('SkillInstallDialog', () => {
it('renders long Unicode identity and multiline release notes before installation', async () => {
const sharedVersion = version()
const longName = `skill-${'n'.repeat(58)}`
const organizationId = `組織-${'o'.repeat(252)}`
const releaseNotes = `First line\n${'r'.repeat(9_989)}`
sharedVersion.name = longName
sharedVersion.description = `説明 ${'d'.repeat(512)}`
sharedVersion.releaseNotes = releaseNotes
sharedVersion.publisher = { userId: 'author_1', organizationId }
const skills = installApi(vi.fn())
skills.resolveShare.mockResolvedValue({
status: 'ok',
value: { id: 'share_1', version: sharedVersion }
})
Object.defineProperty(window, 'api', { configurable: true, value: { skills } })
render(<SkillInstallDialog open onOpenChange={() => undefined} />)
await inspectSkill(sharedVersion.description)
expect(screen.getByRole('heading', { name: longName })).toBeTruthy()
expect(
screen.getByText(
(_, element) =>
element?.tagName === 'P' &&
element.textContent ===
`Published by Orca user author_1 in organization ${organizationId}.`
)
).toBeTruthy()
expect(
screen.getByText(
(_, element) =>
element?.tagName === 'P' && element.textContent === `Release notes: ${releaseNotes}`
)
).toBeTruthy()
})
it('focuses the link first and programmatically names destination controls', async () => {
const user = userEvent.setup()
const onOpenChange = vi.fn()
const skills = installApi(
vi.fn().mockResolvedValue({
status: 'ok',
value: {
name: 'private-skill',
packageDigest: DIGEST,
destinationIdentity: 'local:global',
currentState: 'missing',
providers: []
}
})
)
Object.defineProperty(window, 'api', { configurable: true, value: { skills } })
render(<SkillInstallDialog open onOpenChange={onOpenChange} />)
expect(document.activeElement).toBe(screen.getByRole('textbox', { name: 'Orca skill link' }))
await user.type(
screen.getByRole('textbox', { name: 'Orca skill link' }),
'https://app.orca.dev/skills/share/share_1'
)
await user.tab()
expect(document.activeElement).toBe(screen.getByRole('button', { name: 'Inspect skill' }))
await user.keyboard('{Enter}')
await screen.findByText('A private skill')
expect(screen.getByRole('combobox', { name: 'Machine' })).toBeTruthy()
expect(screen.getByRole('combobox', { name: 'Destination' })).toBeTruthy()
await user.keyboard('{Escape}')
expect(onOpenChange).toHaveBeenCalledWith(false)
})
it('describes unavailable bearer links without asking recipients to sign in', async () => {
const skills = installApi(vi.fn())
skills.resolveShare.mockRejectedValue(new Error('skill_share_not_found'))
@@ -393,6 +460,14 @@ describe('SkillInstallDialog', () => {
const operationId = skills.installShare.mock.calls[0]?.[0].operationId
skills.emitInstallProgress({ operationId, phase: 'installing' })
await screen.findByText('Downloading, verifying, and installing…')
skills.emitInstallProgress({
operationId,
phase: 'installing',
currentSkill: { id: 'private-skill', name: 'private-skill', index: 1, total: 1 }
})
expect((await screen.findByRole('status')).textContent).toBe(
'Installing 1 of 1: private-skill…'
)
settleInstall?.({
status: 'ok',
value: {
@@ -249,9 +249,13 @@ describe('SkillInstallManagementDialog', () => {
render(<SkillInstallManagementDialog open onOpenChange={() => undefined} />)
await selectInstall('ver_2')
fireEvent.click(screen.getByRole('button', { name: 'Remove' }))
const remove = screen.getByRole('button', { name: 'Remove' })
remove.focus()
fireEvent.click(remove)
expect(skills.removeInstall).not.toHaveBeenCalled()
fireEvent.click(screen.getByRole('button', { name: 'Confirm remove' }))
const confirm = screen.getByRole('button', { name: 'Confirm remove' })
expect(document.activeElement).toBe(confirm)
fireEvent.click(confirm)
await waitFor(() => expect(skills.removeInstall).toHaveBeenCalledOnce())
expect(skills.removeInstall).toHaveBeenCalledWith(
@@ -147,9 +147,11 @@ export function SkillInstallReview({
<div className="space-y-5">
<section className="space-y-2">
<div className="flex items-start justify-between gap-3">
<div>
<h3 className="text-sm font-semibold">{version.name}</h3>
<p className="text-xs leading-5 text-muted-foreground">{version.description}</p>
<div className="min-w-0">
<h3 className="break-words text-sm font-semibold">{version.name}</h3>
<p className="break-words text-xs leading-5 text-muted-foreground">
{version.description}
</p>
</div>
<Badge variant="outline">
{translate(
@@ -177,7 +179,7 @@ export function SkillInstallReview({
{version.packageDigest}
</p>
{version.publisher ? (
<p className="text-xs text-muted-foreground">
<p className="break-words text-xs text-muted-foreground">
{translate(
'auto.components.skills.SkillInstallReviewContent.9daf13c180',
'Published by Orca user'
@@ -193,6 +195,15 @@ export function SkillInstallReview({
.
</p>
) : null}
{version.releaseNotes.trim() ? (
<p className="break-words whitespace-pre-wrap text-xs leading-5 text-muted-foreground">
{translate(
'auto.components.skills.SkillInstallReviewContent.releaseNotes',
'Release notes:'
)}{' '}
{version.releaseNotes}
</p>
) : null}
<p className="text-xs leading-5 text-muted-foreground">
{translate(
'auto.components.skills.SkillInstallReviewContent.98ed90e523',
@@ -0,0 +1,55 @@
// @vitest-environment happy-dom
import '@testing-library/jest-dom/vitest'
import { cleanup, render, screen, waitFor } from '@testing-library/react'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { SkillInstallTargetFields } from './SkillInstallTargetFields'
const callbacks = {
onEnvironmentChange: vi.fn(),
onScopeChange: vi.fn(),
onWorkspaceChange: vi.fn(),
onExecutionTargetChange: vi.fn()
}
afterEach(() => {
cleanup()
vi.restoreAllMocks()
Reflect.deleteProperty(window, 'api')
})
describe('SkillInstallTargetFields', () => {
it('names every conditional selector and contains long destination labels', async () => {
Object.defineProperty(window, 'api', {
configurable: true,
value: { skills: { listWslDistros: vi.fn().mockResolvedValue(['Ubuntu-24.04']) } }
})
const longWorkspace = `workspace-${'深'.repeat(240)}`
const props = {
...callbacks,
environmentId: 'local',
scope: 'global' as const,
workspace: '',
executionTarget: null,
runtimeEnvironments: [],
runtimeStatus: new Map(),
sshConnections: [],
workspaceChoices: [{ id: 'folder_1', label: longWorkspace, kind: 'folder' as const }]
}
const view = render(<SkillInstallTargetFields {...props} />)
expect(screen.getByRole('combobox', { name: 'Machine' })).toHaveClass('w-full', 'min-w-0')
expect(screen.getByRole('combobox', { name: 'Destination' })).toHaveClass('w-full', 'min-w-0')
await waitFor(() =>
expect(screen.getByRole('combobox', { name: 'Execution environment' })).toHaveClass(
'w-full',
'min-w-0'
)
)
view.rerender(<SkillInstallTargetFields {...props} scope="workspace" workspace="folder_1" />)
const workspace = screen.getByRole('combobox', { name: 'Workspace' })
expect(workspace).toHaveClass('w-full', 'min-w-0')
expect(workspace.textContent).toContain(longWorkspace)
})
})
@@ -1,4 +1,4 @@
import { useEffect, useState } from 'react'
import { useEffect, useId, useState } from 'react'
import { Label } from '@/components/ui/label'
import {
Select,
@@ -27,6 +27,7 @@ export function SkillInstallTargetFields(props: {
requiredCapability?: string
}): React.JSX.Element {
const [wslDistros, setWslDistros] = useState<string[]>([])
const fieldId = useId()
useEffect(() => {
let active = true
@@ -57,7 +58,7 @@ export function SkillInstallTargetFields(props: {
<>
<section className="grid gap-3 sm:grid-cols-2">
<div className="space-y-2">
<Label>
<Label htmlFor={`${fieldId}-machine`}>
{translate('auto.components.skills.SkillInstallTargetFields.b8a0b706ad', 'Machine')}
</Label>
<Select
@@ -68,7 +69,7 @@ export function SkillInstallTargetFields(props: {
props.onExecutionTargetChange(null)
}}
>
<SelectTrigger>
<SelectTrigger id={`${fieldId}-machine`} className="w-full min-w-0">
<SelectValue />
</SelectTrigger>
<SelectContent>
@@ -120,14 +121,14 @@ export function SkillInstallTargetFields(props: {
</Select>
</div>
<div className="space-y-2">
<Label>
<Label htmlFor={`${fieldId}-destination`}>
{translate('auto.components.skills.SkillInstallTargetFields.63cc9e31fe', 'Destination')}
</Label>
<Select
value={props.scope}
onValueChange={(value) => props.onScopeChange(value as 'global' | 'workspace')}
>
<SelectTrigger>
<SelectTrigger id={`${fieldId}-destination`} className="w-full min-w-0">
<SelectValue />
</SelectTrigger>
<SelectContent>
@@ -150,7 +151,7 @@ export function SkillInstallTargetFields(props: {
{props.scope === 'global' && wslDistros.length > 0 ? (
<section className="space-y-2">
<Label>
<Label htmlFor={`${fieldId}-execution`}>
{translate(
'auto.components.skills.SkillInstallTargetFields.cb47652227',
'Execution environment'
@@ -164,7 +165,7 @@ export function SkillInstallTargetFields(props: {
)
}
>
<SelectTrigger>
<SelectTrigger id={`${fieldId}-execution`} className="w-full min-w-0">
<SelectValue />
</SelectTrigger>
<SelectContent>
@@ -187,11 +188,11 @@ export function SkillInstallTargetFields(props: {
{props.scope === 'workspace' ? (
<section className="space-y-2">
<Label>
<Label htmlFor={`${fieldId}-workspace`}>
{translate('auto.components.skills.SkillInstallTargetFields.0e5b43a9e3', 'Workspace')}
</Label>
<Select value={props.workspace} onValueChange={props.onWorkspaceChange}>
<SelectTrigger>
<SelectTrigger id={`${fieldId}-workspace`} className="w-full min-w-0">
<SelectValue
placeholder={translate(
'auto.components.skills.SkillInstallTargetFields.5845cfe543',
@@ -113,6 +113,17 @@ afterEach(() => {
})
describe('SkillShareDialog', () => {
it('accepts multiline release notes up to the published limit', async () => {
setup([], false, [skill, secondSkill])
await screen.findByRole('heading', { name: 'Share skill bundle' })
const notes = screen.getByRole('textbox', { name: 'Release notes' }) as HTMLTextAreaElement
const value = `Release summary\n${'x'.repeat(9_984)}`
expect(notes.maxLength).toBe(10_000)
fireEvent.change(notes, { target: { value } })
expect(notes.value).toBe(value)
})
it('publishes a new immutable version for one exact managed-install match', async () => {
const { skills } = setup([managedInstall('global:local')])
@@ -172,6 +183,9 @@ describe('SkillShareDialog', () => {
fireEvent.click(screen.getByRole('button', { name: 'Publish skill' }))
await screen.findByRole('button', { name: 'Cancel upload' })
expect(
screen.getByRole('progressbar', { name: 'Uploading…' }).getAttribute('aria-valuenow')
).toBe('0')
emitProgress({
preparationId: preview.preparationId,
phase: 'uploading',
@@ -179,6 +193,9 @@ describe('SkillShareDialog', () => {
totalBytes: 96
})
await screen.findByText('50%')
expect(
screen.getByRole('progressbar', { name: 'Uploading…' }).getAttribute('aria-valuenow')
).toBe('50')
emitProgress({
preparationId: preview.preparationId,
phase: 'finalizing',
@@ -186,6 +203,9 @@ describe('SkillShareDialog', () => {
totalBytes: 96
})
await screen.findByText('Verifying package…')
expect(
screen.getByRole('progressbar', { name: 'Verifying package…' }).getAttribute('aria-valuenow')
).toBe('100')
emitProgress({
preparationId: preview.preparationId,
phase: 'publishing',
@@ -193,6 +213,7 @@ describe('SkillShareDialog', () => {
totalBytes: 96
})
await screen.findByText('Publishing link…')
expect(screen.getByRole('progressbar', { name: 'Publishing link…' })).toBeTruthy()
fireEvent.click(screen.getByRole('button', { name: 'Cancel upload' }))
await waitFor(() => expect(skills.cancelShare).toHaveBeenCalledWith(preview.preparationId))
rejectPublish(new Error('aborted'))
@@ -111,6 +111,7 @@ export function SkillSharePreparationReview({
progressPercent: number
}): React.JSX.Element {
const skillPreviews = preview.skills ?? []
const phaseLabel = publishingPhaseLabel(progress)
return (
<div className="space-y-5">
<section className="space-y-2">
@@ -227,10 +228,21 @@ export function SkillSharePreparationReview({
{publishing ? (
<section className="space-y-2" aria-live="polite">
<div className="flex justify-between text-xs text-muted-foreground">
<span>{publishingPhaseLabel(progress)}</span>
<span>{progress?.phase === 'uploading' ? `${progressPercent}%` : '100%'}</span>
<span id="skill-share-progress-label">{phaseLabel}</span>
<span>
{progress?.phase === 'finalizing' || progress?.phase === 'publishing'
? '100%'
: `${progressPercent}%`}
</span>
</div>
<Progress value={progress?.phase === 'uploading' ? progressPercent : 100} />
<Progress
aria-labelledby="skill-share-progress-label"
value={
progress?.phase === 'finalizing' || progress?.phase === 'publishing'
? 100
: progressPercent
}
/>
</section>
) : null}
</div>
@@ -127,6 +127,54 @@ afterEach(async () => {
})
describe('SkillsPage', () => {
it('uses platform-neutral Escape navigation without stealing editable input Escape', async () => {
const closeSkillsPage = vi.fn()
const discover = vi.fn().mockResolvedValue(discoveryResult(['alpha']))
useAppStore.setState({ closeSkillsPage })
Object.defineProperty(window, 'api', {
configurable: true,
value: { skills: skillsApi(discover), runtimeEnvironments: { call: vi.fn() } }
})
await renderPage()
await flushMicrotasks()
const search = container?.querySelector('input[placeholder="Search skills"]')
if (!(search instanceof HTMLInputElement)) {
throw new Error('Missing skill search')
}
await act(async () => {
search.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }))
})
expect(closeSkillsPage).not.toHaveBeenCalled()
await act(async () => {
window.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }))
})
expect(closeSkillsPage).toHaveBeenCalledOnce()
})
it('contains long cross-platform skill paths while preserving the full path', async () => {
const longPath = `C:\\Users\\orca\\${'nested-folder\\'.repeat(30)}SKILL.md`
const discover = vi.fn().mockResolvedValue({
skills: [skill('long-path', { skillFilePath: longPath })],
sources: [],
scannedAt: 1
} satisfies SkillDiscoveryResult)
Object.defineProperty(window, 'api', {
configurable: true,
value: { skills: skillsApi(discover), runtimeEnvironments: { call: vi.fn() } }
})
await renderPage()
await flushMicrotasks()
const path = [...(container?.querySelectorAll('[title]') ?? [])].find(
(element) => element.getAttribute('title') === longPath
)
expect(path?.classList.contains('truncate')).toBe(true)
expect(path?.textContent).toBe(longPath)
})
it('scans the connected remote runtime instead of the client disk', async () => {
const discover = vi.fn().mockResolvedValue(discoveryResult(['local-only']))
const call = vi.fn(
@@ -185,9 +185,10 @@ export default function SkillsPage(): React.JSX.Element {
if (hasVisibleOverlay()) {
return
}
const target = event.target as HTMLElement | null
const target = event.target
if (
target?.matches('input, textarea, select, [contenteditable="true"], [contenteditable=""]')
target instanceof HTMLElement &&
target.matches('input, textarea, select, [contenteditable="true"], [contenteditable=""]')
) {
return
}
@@ -11,6 +11,7 @@ function Progress({
return (
<ProgressPrimitive.Root
data-slot="progress"
value={value}
className={cn('relative h-2 w-full overflow-hidden rounded-full bg-primary/20', className)}
{...props}
>
+2 -1
View File
@@ -4232,7 +4232,8 @@
"69236de8d6": "Checking…",
"27672470d9": "Opening the link does not install anything. Review the immutable version first.",
"66cff7a804": "https://app.orca.dev/skills/share/…",
"93eb0fe8c7": "Orca skill link"
"93eb0fe8c7": "Orca skill link",
"releaseNotes": "Release notes:"
},
"SkillInstallTargetFields": {
"8e6a972229": "No workspaces are known on this machine.",