diff --git a/docs/agent-skill-sharing-implementation-checklist.md b/docs/agent-skill-sharing-implementation-checklist.md index fed33fcf6fd..a24dd892eb7 100644 --- a/docs/agent-skill-sharing-implementation-checklist.md +++ b/docs/agent-skill-sharing-implementation-checklist.md @@ -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 diff --git a/src/renderer/src/components/settings/OwnedSkillShareLinks.test.tsx b/src/renderer/src/components/settings/OwnedSkillShareLinks.test.tsx index 2706ce10e31..26ae6acd50b 100644 --- a/src/renderer/src/components/settings/OwnedSkillShareLinks.test.tsx +++ b/src/renderer/src/components/settings/OwnedSkillShareLinks.test.tsx @@ -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() }) diff --git a/src/renderer/src/components/skills/SkillBundleInstallReview.tsx b/src/renderer/src/components/skills/SkillBundleInstallReview.tsx index d6c7bb8b3a7..153b68f141c 100644 --- a/src/renderer/src/components/skills/SkillBundleInstallReview.tsx +++ b/src/renderer/src/components/skills/SkillBundleInstallReview.tsx @@ -46,9 +46,11 @@ export function SkillBundleInstallReview(props: {
-
-

{manifest.bundleName}

-

{manifest.description}

+
+

{manifest.bundleName}

+

+ {manifest.description} +

{translate( @@ -131,7 +133,7 @@ export function SkillBundleInstallReview(props: { {manifest.bundleDigest}

{props.version.publisher ? ( -

+

{translate( 'auto.components.skills.SkillBundleInstallReview.01c5a11e07', 'Published by Orca user' @@ -148,7 +150,7 @@ export function SkillBundleInstallReview(props: {

) : null} {props.version.releaseNotes.trim() ? ( -

+

{translate( 'auto.components.skills.SkillBundleInstallReview.01c5a11e09', 'Release notes:' diff --git a/src/renderer/src/components/skills/SkillCloudManagementActions.test.tsx b/src/renderer/src/components/skills/SkillCloudManagementActions.test.tsx index b8a5a1f5975..9e7b0aa2605 100644 --- a/src/renderer/src/components/skills/SkillCloudManagementActions.test.tsx +++ b/src/renderer/src/components/skills/SkillCloudManagementActions.test.tsx @@ -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')) }) diff --git a/src/renderer/src/components/skills/SkillInstallDialog.test.tsx b/src/renderer/src/components/skills/SkillInstallDialog.test.tsx index 9f91acb0698..f3aa80a0da2 100644 --- a/src/renderer/src/components/skills/SkillInstallDialog.test.tsx +++ b/src/renderer/src/components/skills/SkillInstallDialog.test.tsx @@ -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) { - 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) { 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( 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() + + 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: { diff --git a/src/renderer/src/components/skills/SkillInstallManagementDialog.test.tsx b/src/renderer/src/components/skills/SkillInstallManagementDialog.test.tsx index c9612937551..df66769e27c 100644 --- a/src/renderer/src/components/skills/SkillInstallManagementDialog.test.tsx +++ b/src/renderer/src/components/skills/SkillInstallManagementDialog.test.tsx @@ -249,9 +249,13 @@ describe('SkillInstallManagementDialog', () => { render( 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( diff --git a/src/renderer/src/components/skills/SkillInstallReviewContent.tsx b/src/renderer/src/components/skills/SkillInstallReviewContent.tsx index 41f4dad1a9c..d4d7c38a6dc 100644 --- a/src/renderer/src/components/skills/SkillInstallReviewContent.tsx +++ b/src/renderer/src/components/skills/SkillInstallReviewContent.tsx @@ -147,9 +147,11 @@ export function SkillInstallReview({

-
-

{version.name}

-

{version.description}

+
+

{version.name}

+

+ {version.description} +

{translate( @@ -177,7 +179,7 @@ export function SkillInstallReview({ {version.packageDigest}

{version.publisher ? ( -

+

{translate( 'auto.components.skills.SkillInstallReviewContent.9daf13c180', 'Published by Orca user' @@ -193,6 +195,15 @@ export function SkillInstallReview({ .

) : null} + {version.releaseNotes.trim() ? ( +

+ {translate( + 'auto.components.skills.SkillInstallReviewContent.releaseNotes', + 'Release notes:' + )}{' '} + {version.releaseNotes} +

+ ) : null}

{translate( 'auto.components.skills.SkillInstallReviewContent.98ed90e523', diff --git a/src/renderer/src/components/skills/SkillInstallTargetFields.test.tsx b/src/renderer/src/components/skills/SkillInstallTargetFields.test.tsx new file mode 100644 index 00000000000..3292cc17950 --- /dev/null +++ b/src/renderer/src/components/skills/SkillInstallTargetFields.test.tsx @@ -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() + + 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() + const workspace = screen.getByRole('combobox', { name: 'Workspace' }) + expect(workspace).toHaveClass('w-full', 'min-w-0') + expect(workspace.textContent).toContain(longWorkspace) + }) +}) diff --git a/src/renderer/src/components/skills/SkillInstallTargetFields.tsx b/src/renderer/src/components/skills/SkillInstallTargetFields.tsx index 7a18a8dbbe0..4d070e93550 100644 --- a/src/renderer/src/components/skills/SkillInstallTargetFields.tsx +++ b/src/renderer/src/components/skills/SkillInstallTargetFields.tsx @@ -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([]) + const fieldId = useId() useEffect(() => { let active = true @@ -57,7 +58,7 @@ export function SkillInstallTargetFields(props: { <>

-
-