fix(skills): hide split icon and stop update-terminal respawn on refocus (#9082)
* fix(skills): hide split icon and stop update-terminal respawn on refocus
The "Update skills" modal embeds a one-off command terminal via
OnboardingInlineCommandTerminal. Two issues:
1. The pane header showed the regular terminal's split ("Split Terminal
Right") icon, which makes no sense for an ephemeral single-command
terminal. Add an `allowSplit` prop (default true) threaded
TerminalPane -> TerminalPaneHeaderOverlay, and have all onboarding
inline command terminals pass `allowSplit={false}` to omit it.
2. Alt-tabbing out of Orca and back respawned the terminal. On window
focus, useSkillFreshness republishes {inventory: null, loading: true}
to retract stale update authority; the dialog reacts by nulling the
draft command, then re-sets it after the scan, and the
`key={terminalCommand}` remount spawns a fresh PTY. Add a ref-counted
suspendSkillFreshnessFocusRescan(); the dialog holds focus rescans
while a live terminal is showing so refocus no longer tears it down.
Explicit Re-check and install-changed refreshes still run.
* fix(skills): preserve freshness checks without respawning terminal
* fix(skills): keep revalidated terminal safe and focused
This commit is contained in:
parent
5b75a87dd3
commit
3a74721f89
|
|
@ -1,11 +1,11 @@
|
|||
{
|
||||
"schemaVersion": 1,
|
||||
"appVersion": "1.4.144-rc.1",
|
||||
"appVersion": "1.4.144-rc.2",
|
||||
"skills": [
|
||||
{
|
||||
"name": "computer-use",
|
||||
"sourcePath": "skills/computer-use",
|
||||
"appVersion": "1.4.144-rc.1",
|
||||
"appVersion": "1.4.144-rc.2",
|
||||
"releaseRevision": 5,
|
||||
"packageDigest": "cd2809474d57fd7277adb277448e6fa446810d3cbad71ac0b473b9e8ff1bad68",
|
||||
"gitTreeSha": "306c0f8cb63bcac265a5b7975dc2f855be4f1344",
|
||||
|
|
@ -24,7 +24,7 @@
|
|||
{
|
||||
"name": "linear-tickets",
|
||||
"sourcePath": "skills/linear-tickets",
|
||||
"appVersion": "1.4.144-rc.1",
|
||||
"appVersion": "1.4.144-rc.2",
|
||||
"releaseRevision": 4,
|
||||
"packageDigest": "f198d7b22e5ee1673dac403f9cca0553b124e0a90e4fdd05d2c23b7344e32d2b",
|
||||
"gitTreeSha": "de9fc106bbb4e313a90ff9a9513a720909bbd176",
|
||||
|
|
@ -43,7 +43,7 @@
|
|||
{
|
||||
"name": "orca-cli",
|
||||
"sourcePath": "skills/orca-cli",
|
||||
"appVersion": "1.4.144-rc.1",
|
||||
"appVersion": "1.4.144-rc.2",
|
||||
"releaseRevision": 32,
|
||||
"packageDigest": "51740ff13f379ac5743d3fd28a14b17168dcef40f7048c20182dce166098c45f",
|
||||
"gitTreeSha": "ded93000a5f654e2b4f324501282459bd56afe19",
|
||||
|
|
@ -62,7 +62,7 @@
|
|||
{
|
||||
"name": "orca-emulator",
|
||||
"sourcePath": "skills/orca-emulator",
|
||||
"appVersion": "1.4.144-rc.1",
|
||||
"appVersion": "1.4.144-rc.2",
|
||||
"releaseRevision": 4,
|
||||
"packageDigest": "453b1d9aa20b51b8a4d32c7b6def6a93f7ef9c730de32abbcbc1788ad1b1820b",
|
||||
"gitTreeSha": "66be6abe99f1807da85934aee0e22daefc8f7656",
|
||||
|
|
@ -81,7 +81,7 @@
|
|||
{
|
||||
"name": "orca-emulator-android",
|
||||
"sourcePath": "skills/orca-emulator-android",
|
||||
"appVersion": "1.4.144-rc.1",
|
||||
"appVersion": "1.4.144-rc.2",
|
||||
"releaseRevision": 2,
|
||||
"packageDigest": "12272cf82e0731f11e424822b961882457034e730358cc65ea28e4eb9c8ff7f5",
|
||||
"gitTreeSha": "f7b0fc8cbf5cd78ca5156f6bbe3a20f1462d8f83",
|
||||
|
|
@ -100,7 +100,7 @@
|
|||
{
|
||||
"name": "orca-linear",
|
||||
"sourcePath": "skills/orca-linear",
|
||||
"appVersion": "1.4.144-rc.1",
|
||||
"appVersion": "1.4.144-rc.2",
|
||||
"releaseRevision": 2,
|
||||
"packageDigest": "d44d09e6ecb6a64da177083aad26a95f031cd1cf26ba059fdc888c2628aef64f",
|
||||
"gitTreeSha": "c34f42030f43e5a85737996fa375bbd79cb5bea8",
|
||||
|
|
@ -119,7 +119,7 @@
|
|||
{
|
||||
"name": "orca-per-workspace-env",
|
||||
"sourcePath": "skills/orca-per-workspace-env",
|
||||
"appVersion": "1.4.144-rc.1",
|
||||
"appVersion": "1.4.144-rc.2",
|
||||
"releaseRevision": 2,
|
||||
"packageDigest": "fa3b65a1a107fca3f0375c696852477b62f58c154b9eb5c0663c41edc4bcd30d",
|
||||
"gitTreeSha": "354e775b79ea6952ec63acac4d3ee8a9ae07a650",
|
||||
|
|
@ -138,7 +138,7 @@
|
|||
{
|
||||
"name": "orchestration",
|
||||
"sourcePath": "skills/orchestration",
|
||||
"appVersion": "1.4.144-rc.1",
|
||||
"appVersion": "1.4.144-rc.2",
|
||||
"releaseRevision": 24,
|
||||
"packageDigest": "9fbfa2ae3f3f99441563a4b8b1c6302107944480db8718ddb326a862a51f7ab9",
|
||||
"gitTreeSha": "086c41e0b353b4908d2963694b4a7c791d4b3982",
|
||||
|
|
|
|||
|
|
@ -294,6 +294,7 @@ export function OnboardingInlineCommandTerminal({
|
|||
cwd={cwd}
|
||||
isActive
|
||||
isVisible
|
||||
showSplitButton={false}
|
||||
onPtyExit={() => {
|
||||
onTerminalExit?.()
|
||||
closeTab(tabId, { recordInteraction: false, reason: 'pty-exit' })
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
// @vitest-environment happy-dom
|
||||
|
||||
import { act, useState, type ReactNode } from 'react'
|
||||
import { act, useLayoutEffect, useRef, useState, type ReactNode } from 'react'
|
||||
import { createRoot, type Root } from 'react-dom/client'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type {
|
||||
|
|
@ -24,6 +24,7 @@ const mocks = vi.hoisted(() => ({
|
|||
onInteracted?: (method: 'keyboard' | 'pointer', event?: { key?: string }) => void
|
||||
onTerminalExit?: () => void
|
||||
}[],
|
||||
terminalCommits: [] as [command: string, inert: boolean][],
|
||||
notifyChanged: vi.fn()
|
||||
}))
|
||||
|
||||
|
|
@ -42,8 +43,20 @@ vi.mock('@/hooks/useInstalledAgentSkills', () => ({
|
|||
|
||||
vi.mock('@/components/onboarding/OnboardingInlineCommandTerminal', () => ({
|
||||
OnboardingInlineCommandTerminal: (props: (typeof mocks.terminalProps)[number]) => {
|
||||
const terminalRef = useRef<HTMLDivElement>(null)
|
||||
mocks.terminalProps.push(props)
|
||||
return <div data-testid="update-terminal">{props.command}</div>
|
||||
useLayoutEffect(() => {
|
||||
mocks.terminalCommits.push([
|
||||
props.command,
|
||||
terminalRef.current?.parentElement?.inert ?? false
|
||||
])
|
||||
})
|
||||
return (
|
||||
<div ref={terminalRef} data-testid="update-terminal">
|
||||
{props.command}
|
||||
<textarea data-testid="update-terminal-input" />
|
||||
</div>
|
||||
)
|
||||
}
|
||||
}))
|
||||
|
||||
|
|
@ -155,6 +168,7 @@ describe('SkillFreshnessUpdateDialog', () => {
|
|||
mocks.refresh.mockReset()
|
||||
mocks.notifyChanged.mockReset()
|
||||
mocks.terminalProps.length = 0
|
||||
mocks.terminalCommits.length = 0
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
|
|
@ -200,6 +214,7 @@ describe('SkillFreshnessUpdateDialog', () => {
|
|||
it('shows the up-to-date state once every installation is current', async () => {
|
||||
await renderDialog()
|
||||
await openViaRequest()
|
||||
mocks.terminalCommits.length = 0
|
||||
|
||||
mocks.inventory = {
|
||||
schemaVersion: 1,
|
||||
|
|
@ -211,6 +226,7 @@ describe('SkillFreshnessUpdateDialog', () => {
|
|||
}
|
||||
await rerender()
|
||||
|
||||
expect(mocks.terminalCommits).toContainEqual(['npx skills update orca-cli --global', true])
|
||||
expect(container?.textContent).toContain('All installed Orca skills are up to date.')
|
||||
expect(container?.querySelector('[data-testid="update-terminal"]')).toBeNull()
|
||||
})
|
||||
|
|
@ -263,17 +279,47 @@ describe('SkillFreshnessUpdateDialog', () => {
|
|||
)
|
||||
})
|
||||
|
||||
it('removes an unsubmitted draft as soon as its inventory is invalidated', async () => {
|
||||
it('keeps the same terminal inert while inventory is revalidated', async () => {
|
||||
await renderDialog()
|
||||
await openViaRequest()
|
||||
expect(container?.querySelector('[data-testid="update-terminal"]')).not.toBeNull()
|
||||
const firstTerminal = container?.querySelector('[data-testid="update-terminal"]')
|
||||
const terminalInput = container?.querySelector<HTMLTextAreaElement>(
|
||||
'[data-testid="update-terminal-input"]'
|
||||
)
|
||||
const terminalWrapper = container?.querySelector<HTMLElement>('[data-skill-update-terminal]')
|
||||
terminalInput?.focus()
|
||||
expect(firstTerminal).not.toBeNull()
|
||||
expect(terminalWrapper?.hasAttribute('inert')).toBe(false)
|
||||
|
||||
mocks.inventory = null
|
||||
mocks.loading = true
|
||||
await rerender()
|
||||
|
||||
expect(container?.textContent).toContain('Checking installed Orca skills')
|
||||
expect(container?.querySelector('[data-testid="update-terminal"]')).toBeNull()
|
||||
expect(container?.querySelector('[data-testid="update-terminal"]')).toBe(firstTerminal)
|
||||
expect(terminalWrapper?.hasAttribute('inert')).toBe(true)
|
||||
terminalInput?.blur()
|
||||
|
||||
mocks.inventory = { ...eligibleInventory(), scannedAt: 2 }
|
||||
mocks.loading = false
|
||||
await rerender()
|
||||
|
||||
expect(container?.querySelector('[data-testid="update-terminal"]')).toBe(firstTerminal)
|
||||
expect(terminalWrapper?.hasAttribute('inert')).toBe(false)
|
||||
expect(document.activeElement).toBe(terminalInput)
|
||||
|
||||
mocks.inventory = null
|
||||
mocks.loading = true
|
||||
await rerender()
|
||||
const closeButton = Array.from(container?.querySelectorAll('button') ?? []).find(
|
||||
(candidate) => candidate.textContent?.trim() === 'Close'
|
||||
)
|
||||
closeButton?.focus()
|
||||
mocks.inventory = { ...eligibleInventory(), scannedAt: 3 }
|
||||
mocks.loading = false
|
||||
await rerender()
|
||||
|
||||
expect(document.activeElement).toBe(closeButton)
|
||||
})
|
||||
|
||||
it('shows a failed scan as an error instead of indefinite progress', async () => {
|
||||
|
|
|
|||
|
|
@ -138,6 +138,9 @@ export function SkillFreshnessUpdateDialog(): React.JSX.Element {
|
|||
const [terminalCommand, setTerminalCommand] = useState<string | null>(null)
|
||||
const [awaitingExitRefresh, setAwaitingExitRefresh] = useState(false)
|
||||
const terminalSubmittedRef = useRef(false)
|
||||
const terminalWrapperRef = useRef<HTMLDivElement>(null)
|
||||
const terminalFocusTargetRef = useRef<HTMLElement | null>(null)
|
||||
const terminalAuthorizationWasPendingRef = useRef(false)
|
||||
const inventoryAtTerminalExitRef = useRef<SkillFreshnessInventory | null>(null)
|
||||
const inventory = state.inventory
|
||||
const eligibleNames = useMemo(() => inventory?.eligibleUpdateNames ?? [], [inventory])
|
||||
|
|
@ -149,14 +152,26 @@ export function SkillFreshnessUpdateDialog(): React.JSX.Element {
|
|||
const hasBlockedGroup = groups.some((group) => group.status === 'cannot-update')
|
||||
const updateCommand = buildTargetedSkillUpdateCommand(eligibleNames)
|
||||
const summaryKind = summarizeInventory(inventory, hasBlockedGroup)
|
||||
const terminalAuthorizationPending =
|
||||
terminalCommand !== null &&
|
||||
!terminalSubmittedRef.current &&
|
||||
(state.loading ||
|
||||
state.error !== null ||
|
||||
inventory === null ||
|
||||
terminalCommand !== updateCommand)
|
||||
|
||||
useEffect(() => {
|
||||
if (!open) {
|
||||
return
|
||||
}
|
||||
if (state.loading || state.error || !inventory) {
|
||||
if (state.loading) {
|
||||
// Why: keep the PTY mounted across focus revalidation, but the inert
|
||||
// wrapper below prevents stale draft input until fresh authority returns.
|
||||
return
|
||||
}
|
||||
if (state.error || !inventory) {
|
||||
// Why: a scan invalidates the authorization behind an unsubmitted draft.
|
||||
// A running command keeps its PTY until exit, but stale drafts fail closed.
|
||||
// A failed scan cannot restore it, so stale drafts fail closed.
|
||||
if (!terminalSubmittedRef.current && terminalCommand !== null) {
|
||||
setTerminalCommand(null)
|
||||
}
|
||||
|
|
@ -186,6 +201,27 @@ export function SkillFreshnessUpdateDialog(): React.JSX.Element {
|
|||
updateCommand
|
||||
])
|
||||
|
||||
useEffect(() => {
|
||||
if (terminalCommand === null) {
|
||||
// Why: the dialog stays mounted while closed, so do not retain a detached
|
||||
// xterm subtree after the draft exits or loses authorization.
|
||||
terminalFocusTargetRef.current = null
|
||||
}
|
||||
const wasPending = terminalAuthorizationWasPendingRef.current
|
||||
terminalAuthorizationWasPendingRef.current = terminalAuthorizationPending
|
||||
if (!wasPending || terminalAuthorizationPending) {
|
||||
return
|
||||
}
|
||||
const focusTarget = terminalFocusTargetRef.current
|
||||
if (!focusTarget?.isConnected || !terminalWrapperRef.current?.contains(focusTarget)) {
|
||||
terminalFocusTargetRef.current = null
|
||||
return
|
||||
}
|
||||
// Why: Chromium drops focus when an ancestor becomes inert; unchanged
|
||||
// authorization should return focus to the preserved xterm instance.
|
||||
focusTarget.focus()
|
||||
}, [terminalAuthorizationPending, terminalCommand])
|
||||
|
||||
const handleOpenChange = (next: boolean): void => {
|
||||
// Why: closing is the natural point to re-observe bytes so a completed update
|
||||
// clears the state and the lingering nudge does not fire again.
|
||||
|
|
@ -237,28 +273,49 @@ export function SkillFreshnessUpdateDialog(): React.JSX.Element {
|
|||
)}
|
||||
|
||||
{terminalCommand ? (
|
||||
<OnboardingInlineCommandTerminal
|
||||
key={terminalCommand}
|
||||
command={terminalCommand}
|
||||
title={translate(
|
||||
'auto.components.skills.SkillFreshnessUpdateDialog.terminalTitle',
|
||||
'Update Orca skills'
|
||||
)}
|
||||
description={translate(
|
||||
'auto.components.skills.SkillFreshnessUpdateDialog.terminalDescription',
|
||||
'Review the pre-filled command, then press Enter to run it.'
|
||||
)}
|
||||
ariaLabel={translate(
|
||||
'auto.components.skills.SkillFreshnessUpdateDialog.terminalAria',
|
||||
'Orca skill update terminal'
|
||||
)}
|
||||
worktreeId="skill-freshness-update-terminal"
|
||||
terminalHeightPx={200}
|
||||
terminalTopMarginPx={0}
|
||||
autoScrollIntoView={false}
|
||||
onInteracted={handleTerminalInteraction}
|
||||
onTerminalExit={handleTerminalExit}
|
||||
/>
|
||||
<div
|
||||
ref={terminalWrapperRef}
|
||||
data-skill-update-terminal=""
|
||||
aria-busy={terminalAuthorizationPending}
|
||||
inert={terminalAuthorizationPending}
|
||||
className={terminalAuthorizationPending ? 'opacity-50' : undefined}
|
||||
onFocusCapture={(event) => {
|
||||
terminalFocusTargetRef.current = event.target as HTMLElement
|
||||
}}
|
||||
onBlurCapture={(event) => {
|
||||
// Why: a concrete next target means the user moved focus; inert
|
||||
// uses a null target, which should be restored after revalidation.
|
||||
if (
|
||||
event.relatedTarget instanceof Node &&
|
||||
!event.currentTarget.contains(event.relatedTarget)
|
||||
) {
|
||||
terminalFocusTargetRef.current = null
|
||||
}
|
||||
}}
|
||||
>
|
||||
<OnboardingInlineCommandTerminal
|
||||
key={terminalCommand}
|
||||
command={terminalCommand}
|
||||
title={translate(
|
||||
'auto.components.skills.SkillFreshnessUpdateDialog.terminalTitle',
|
||||
'Update Orca skills'
|
||||
)}
|
||||
description={translate(
|
||||
'auto.components.skills.SkillFreshnessUpdateDialog.terminalDescription',
|
||||
'Review the pre-filled command, then press Enter to run it.'
|
||||
)}
|
||||
ariaLabel={translate(
|
||||
'auto.components.skills.SkillFreshnessUpdateDialog.terminalAria',
|
||||
'Orca skill update terminal'
|
||||
)}
|
||||
worktreeId="skill-freshness-update-terminal"
|
||||
terminalHeightPx={200}
|
||||
terminalTopMarginPx={0}
|
||||
autoScrollIntoView={false}
|
||||
onInteracted={handleTerminalInteraction}
|
||||
onTerminalExit={handleTerminalExit}
|
||||
/>
|
||||
</div>
|
||||
) : null}
|
||||
|
||||
{/* Why: Radix reads defaultOpen only on mount. Remount when a scan
|
||||
|
|
|
|||
|
|
@ -215,6 +215,9 @@ type TerminalPaneProps = {
|
|||
// or persist to the layout snapshot, so returning to the workspace shows
|
||||
// the original split layout unchanged.
|
||||
isolatedPaneKey?: string | null
|
||||
// Why: ephemeral one-off command terminals don't need the regular pane header's
|
||||
// prominent split affordance, though standard split shortcuts remain available.
|
||||
showSplitButton?: boolean
|
||||
onPtyExit: (ptyId: string) => void
|
||||
onCloseTab: () => void
|
||||
}
|
||||
|
|
@ -281,6 +284,7 @@ export default function TerminalPane({
|
|||
isVisible = true,
|
||||
isWorktreeActive = isVisible,
|
||||
isolatedPaneKey = null,
|
||||
showSplitButton = true,
|
||||
onPtyExit,
|
||||
onCloseTab
|
||||
}: TerminalPaneProps): React.JSX.Element {
|
||||
|
|
@ -3208,6 +3212,7 @@ export default function TerminalPane({
|
|||
worktreeId={worktreeId}
|
||||
cwd={cwd ?? ''}
|
||||
showAlwaysOnHeaders={isActive && terminalContentVisible}
|
||||
showSplitButton={showSplitButton}
|
||||
paneCount={paneCount}
|
||||
activePaneId={activePane?.id}
|
||||
panes={managedPanes}
|
||||
|
|
|
|||
|
|
@ -44,6 +44,7 @@ function renderOverlay({
|
|||
paneTitles,
|
||||
paneCount = 2,
|
||||
showAlwaysOnHeaders = true,
|
||||
showSplitButton = true,
|
||||
onClosePane = vi.fn(),
|
||||
onRemoveTitle = vi.fn(),
|
||||
onRenameSubmit = vi.fn(),
|
||||
|
|
@ -53,6 +54,7 @@ function renderOverlay({
|
|||
paneTitles: Record<number, string>
|
||||
paneCount?: number
|
||||
showAlwaysOnHeaders?: boolean
|
||||
showSplitButton?: boolean
|
||||
onClosePane?: ReturnType<typeof vi.fn>
|
||||
onRemoveTitle?: ReturnType<typeof vi.fn>
|
||||
onRenameSubmit?: ReturnType<typeof vi.fn>
|
||||
|
|
@ -75,6 +77,7 @@ function renderOverlay({
|
|||
worktreeId="wt-1"
|
||||
cwd={path.join(path.sep, 'tmp')}
|
||||
showAlwaysOnHeaders={showAlwaysOnHeaders}
|
||||
showSplitButton={showSplitButton}
|
||||
paneCount={paneCount}
|
||||
activePaneId={1}
|
||||
panes={panes}
|
||||
|
|
@ -167,6 +170,16 @@ describe('TerminalPaneHeaderOverlay', () => {
|
|||
expect(onRemoveTitle).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('omits the split control when the header affordance is hidden', () => {
|
||||
const { container } = renderOverlay({
|
||||
paneTitles: { 1: '', 2: '' },
|
||||
paneCount: 1,
|
||||
showSplitButton: false
|
||||
})
|
||||
|
||||
expect(container.querySelector('button[aria-label="Split Terminal Right"]')).toBeNull()
|
||||
})
|
||||
|
||||
it('ignores IME composition Enter before submitting a pane title rename', () => {
|
||||
const { container, onRenameSubmit } = renderOverlay({
|
||||
paneTitles: { 1: 'server', 2: '' },
|
||||
|
|
|
|||
|
|
@ -20,6 +20,8 @@ type TerminalPaneHeaderOverlayProps = {
|
|||
worktreeId: string
|
||||
cwd: string
|
||||
showAlwaysOnHeaders: boolean
|
||||
/** Used by ephemeral one-off command terminals that omit the header affordance. */
|
||||
showSplitButton?: boolean
|
||||
paneCount: number
|
||||
activePaneId: number | null | undefined
|
||||
panes: readonly ManagedPane[]
|
||||
|
|
@ -60,6 +62,7 @@ export default function TerminalPaneHeaderOverlay({
|
|||
worktreeId,
|
||||
cwd,
|
||||
showAlwaysOnHeaders,
|
||||
showSplitButton = true,
|
||||
paneCount,
|
||||
activePaneId,
|
||||
panes,
|
||||
|
|
@ -272,7 +275,7 @@ export default function TerminalPaneHeaderOverlay({
|
|||
</TooltipContent>
|
||||
</Tooltip>
|
||||
) : null}
|
||||
{showAlwaysOnHeaders ? (
|
||||
{showAlwaysOnHeaders && showSplitButton ? (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<Button
|
||||
|
|
|
|||
Loading…
Reference in New Issue