mirror of
https://github.com/stablyai/orca.git
synced 2026-10-27 00:02:48 +00:00
Merge remote-tracking branch 'origin/main' into fix/renderer-crash-white-screen
This commit is contained in:
@@ -57,6 +57,32 @@ describe('skill discovery', () => {
|
||||
expect(rootPaths).toContain('/workspace/current/.claude/skills')
|
||||
})
|
||||
|
||||
it('scans each provider home skill root that npx skills --global writes to', () => {
|
||||
const roots = buildSkillDiscoverySources({
|
||||
homeDir: '/home/test',
|
||||
cwd: '/workspace/current'
|
||||
})
|
||||
|
||||
const rootPaths = roots.map((root) => root.path.replace(/\\/g, '/'))
|
||||
expect(rootPaths).toEqual(
|
||||
expect.arrayContaining([
|
||||
'/home/test/.grok/skills',
|
||||
'/home/test/.config/opencode/skills',
|
||||
'/home/test/.pi/agent/skills',
|
||||
'/home/test/.gemini/skills',
|
||||
'/home/test/.gemini/antigravity/skills',
|
||||
'/home/test/.cursor/skills'
|
||||
])
|
||||
)
|
||||
// Why: these live outside ~/.agents/skills, so they must carry the shared
|
||||
// agent-skills provider to feed per-agent orchestration coverage.
|
||||
for (const root of roots) {
|
||||
if (root.path.replace(/\\/g, '/') === '/home/test/.grok/skills') {
|
||||
expect(root.providers).toEqual(['agent-skills'])
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
it('discovers skill packages through symlinked skill directories', async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), 'orca-skills-'))
|
||||
const home = join(root, 'home')
|
||||
@@ -77,6 +103,27 @@ describe('skill discovery', () => {
|
||||
expect(skill?.directoryPath).toBe(linkedSkill)
|
||||
})
|
||||
|
||||
it('discovers a symlinked skill inside a provider home root (#8256/#8503)', async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), 'orca-skills-'))
|
||||
const home = join(root, 'home')
|
||||
const realSkill = join(root, 'central-skills', 'orchestration')
|
||||
const linkedSkill = join(home, '.pi', 'agent', 'skills', 'orchestration')
|
||||
await mkdir(realSkill, { recursive: true })
|
||||
await mkdir(join(home, '.pi', 'agent', 'skills'), { recursive: true })
|
||||
await writeFile(join(realSkill, 'SKILL.md'), '# orchestration\n\nCoordinate agents.')
|
||||
await symlink(realSkill, linkedSkill, process.platform === 'win32' ? 'junction' : 'dir')
|
||||
|
||||
const result = await discoverSkills({
|
||||
homeDir: home,
|
||||
cwd: join(root, 'missing-cwd')
|
||||
})
|
||||
|
||||
const skill = result.skills.find((entry) => entry.name === 'orchestration')
|
||||
expect(skill?.sourceKind).toBe('home')
|
||||
expect(skill?.directoryPath).toBe(linkedSkill)
|
||||
expect(skill?.providers).toEqual(['agent-skills'])
|
||||
})
|
||||
|
||||
it('discovers worktree .agents skill symlinks from the requested cwd', async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), 'orca-skills-'))
|
||||
const home = join(root, 'home')
|
||||
|
||||
@@ -41,7 +41,23 @@ export function buildSkillDiscoverySources(
|
||||
join(home, '.codex', 'plugins', 'cache'),
|
||||
'plugin',
|
||||
['codex', 'agent-skills']
|
||||
)
|
||||
),
|
||||
// Why: `npx skills add --global` writes into each agent's own home skills
|
||||
// directory, so coverage misses them unless we scan every provider root.
|
||||
source('home-grok', 'Grok home', join(home, '.grok', 'skills'), 'home', ['agent-skills']),
|
||||
source('home-opencode', 'OpenCode home', join(home, '.config', 'opencode', 'skills'), 'home', [
|
||||
'agent-skills'
|
||||
]),
|
||||
source('home-pi', 'Pi home', join(home, '.pi', 'agent', 'skills'), 'home', ['agent-skills']),
|
||||
source('home-gemini', 'Gemini home', join(home, '.gemini', 'skills'), 'home', ['agent-skills']),
|
||||
source(
|
||||
'home-antigravity',
|
||||
'Antigravity home',
|
||||
join(home, '.gemini', 'antigravity', 'skills'),
|
||||
'home',
|
||||
['agent-skills']
|
||||
),
|
||||
source('home-cursor', 'Cursor home', join(home, '.cursor', 'skills'), 'home', ['agent-skills'])
|
||||
]
|
||||
|
||||
const projectPaths = new Set<string>()
|
||||
|
||||
@@ -2,7 +2,7 @@ import { useCallback, useMemo, type Dispatch, type SetStateAction } from 'react'
|
||||
import { GitBranch } from 'lucide-react'
|
||||
import type { NestedRepoScanResult } from '../../../../shared/types'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { getRepoDisplayLabelsByPath } from '@/lib/repo-display-labels'
|
||||
import { getRepoDisplayLabelKey, getRepoDisplayLabelsByPath } from '@/lib/repo-display-labels'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
|
||||
function NestedRepoSelectAllRow({
|
||||
@@ -118,7 +118,7 @@ export function NestedRepoChecklist({
|
||||
selectedPaths.has(repo.path) ? 'text-foreground' : 'text-muted-foreground'
|
||||
)}
|
||||
>
|
||||
{displayLabelsByPath.get(repo.path) ?? repo.displayName}
|
||||
{displayLabelsByPath.get(getRepoDisplayLabelKey(repo)) ?? repo.displayName}
|
||||
</span>
|
||||
</label>
|
||||
</li>
|
||||
|
||||
@@ -32,7 +32,7 @@ import {
|
||||
import { cloneDefaultWorkspaceStatuses } from '../../../../shared/workspace-statuses'
|
||||
import type { AppState } from '../../store/types'
|
||||
import { getGitHubPRCacheKey, getLegacyGitHubPRCacheKey } from '../../store/slices/github-cache-key'
|
||||
import { getRepoDisplayLabelsByPath } from '@/lib/repo-display-labels'
|
||||
import { getRepoDisplayLabelKey, getRepoDisplayLabelsByPath } from '@/lib/repo-display-labels'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import { getExecutionHostLabel, getRepoExecutionHostId } from '../../../../shared/execution-host'
|
||||
import { parseWslUncPath } from '../../../../shared/wsl-paths'
|
||||
@@ -709,7 +709,9 @@ function withRepoSectionDisplayLabels(entries: readonly OrderedGroupEntry[]): Or
|
||||
const labelsByPath = getRepoDisplayLabelsByPath(repos)
|
||||
return entries.map(([key, group]) => [
|
||||
key,
|
||||
group.repo ? { ...group, label: labelsByPath.get(group.repo.path) ?? group.label } : group
|
||||
group.repo
|
||||
? { ...group, label: labelsByPath.get(getRepoDisplayLabelKey(group.repo)) ?? group.label }
|
||||
: group
|
||||
])
|
||||
}
|
||||
|
||||
|
||||
@@ -40,6 +40,7 @@ function createRejectableDeferred<T>(): {
|
||||
}
|
||||
|
||||
const HOOK_DONE_QUIET_MS = 1_500
|
||||
const CODEX_ATTENTION_QUIET_MS = 1_500
|
||||
|
||||
describe('agent completion coordinator', () => {
|
||||
beforeEach(() => {
|
||||
@@ -1638,6 +1639,283 @@ describe('agent completion coordinator', () => {
|
||||
expect(dispatchCompletion).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('cancels the debounced Codex attention notification when work resumes in the quiet window', () => {
|
||||
// Why: Codex fires PermissionRequest at the human-input boundary *before* the
|
||||
// approval decision. Under "Approve for me" the review agent approves and
|
||||
// Codex resumes within the quiet window, so the OS notification must be
|
||||
// debounced and canceled — no false "approval required" banner (issue #8387).
|
||||
const dispatchAttention = vi.fn()
|
||||
const dispatchHookLifecycle = vi.fn()
|
||||
const coordinator = createAgentCompletionCoordinator({
|
||||
paneKey: 'tab-1:leaf-1',
|
||||
getPtyId: () => 'pty-1',
|
||||
getSettings: () => null,
|
||||
inspectProcess: vi.fn(),
|
||||
dispatchCompletion: vi.fn(),
|
||||
dispatchAttention,
|
||||
dispatchHookLifecycle,
|
||||
isLive: () => true
|
||||
})
|
||||
|
||||
const turn = { prompt: 'fix the bug', agentType: 'codex' as const }
|
||||
coordinator.observeHookStatus({ state: 'working', ...turn })
|
||||
coordinator.observeHookStatus({
|
||||
state: 'waiting',
|
||||
...turn,
|
||||
toolName: 'exec_command',
|
||||
toolInput: 'git status'
|
||||
})
|
||||
|
||||
// Visual status still updates immediately even though the notification waits.
|
||||
expect(dispatchHookLifecycle).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ state: 'waiting', agentType: 'codex' })
|
||||
)
|
||||
expect(dispatchAttention).not.toHaveBeenCalled()
|
||||
|
||||
coordinator.observeHookStatus({
|
||||
state: 'working',
|
||||
...turn,
|
||||
toolName: 'exec_command',
|
||||
toolInput: 'git status'
|
||||
})
|
||||
vi.advanceTimersByTime(CODEX_ATTENTION_QUIET_MS)
|
||||
|
||||
expect(dispatchAttention).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('dispatches the debounced Codex attention notification after the quiet window elapses', () => {
|
||||
const dispatchAttention = vi.fn()
|
||||
const coordinator = createAgentCompletionCoordinator({
|
||||
paneKey: 'tab-1:leaf-1',
|
||||
getPtyId: () => 'pty-1',
|
||||
getSettings: () => null,
|
||||
inspectProcess: vi.fn(),
|
||||
dispatchCompletion: vi.fn(),
|
||||
dispatchAttention,
|
||||
isLive: () => true
|
||||
})
|
||||
|
||||
const turn = { prompt: 'fix the bug', agentType: 'codex' as const }
|
||||
coordinator.observeHookStatus({ state: 'working', ...turn })
|
||||
coordinator.observeHookStatus({
|
||||
state: 'waiting',
|
||||
...turn,
|
||||
toolName: 'exec_command',
|
||||
toolInput: 'apply patch'
|
||||
})
|
||||
expect(dispatchAttention).not.toHaveBeenCalled()
|
||||
|
||||
vi.advanceTimersByTime(CODEX_ATTENTION_QUIET_MS)
|
||||
|
||||
expect(dispatchAttention).toHaveBeenCalledTimes(1)
|
||||
expect(dispatchAttention).toHaveBeenCalledWith(
|
||||
'codex',
|
||||
expect.objectContaining({
|
||||
source: 'hook',
|
||||
agentStatus: expect.objectContaining({
|
||||
state: 'waiting',
|
||||
agentType: 'codex',
|
||||
toolInput: 'apply patch'
|
||||
})
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('dispatches a non-Codex attention notification immediately without debounce', () => {
|
||||
const dispatchAttention = vi.fn()
|
||||
const coordinator = createAgentCompletionCoordinator({
|
||||
paneKey: 'tab-1:leaf-1',
|
||||
getPtyId: () => 'pty-1',
|
||||
getSettings: () => null,
|
||||
inspectProcess: vi.fn(),
|
||||
dispatchCompletion: vi.fn(),
|
||||
dispatchAttention,
|
||||
isLive: () => true
|
||||
})
|
||||
|
||||
const turn = { prompt: 'fix the bug', agentType: 'cursor' as const }
|
||||
coordinator.observeHookStatus({ state: 'working', ...turn })
|
||||
coordinator.observeHookStatus({
|
||||
state: 'waiting',
|
||||
...turn,
|
||||
toolName: 'Shell',
|
||||
toolInput: 'pnpm test'
|
||||
})
|
||||
|
||||
expect(dispatchAttention).toHaveBeenCalledTimes(1)
|
||||
// Non-Codex attention must not arm the debounce timer at all.
|
||||
expect(vi.getTimerCount()).toBe(0)
|
||||
})
|
||||
|
||||
it('debounces a blocked Codex pause like waiting and fires after the quiet window', () => {
|
||||
const dispatchAttention = vi.fn()
|
||||
const coordinator = createAgentCompletionCoordinator({
|
||||
paneKey: 'tab-1:leaf-1',
|
||||
getPtyId: () => 'pty-1',
|
||||
getSettings: () => null,
|
||||
inspectProcess: vi.fn(),
|
||||
dispatchCompletion: vi.fn(),
|
||||
dispatchAttention,
|
||||
isLive: () => true
|
||||
})
|
||||
|
||||
const turn = { prompt: 'fix the bug', agentType: 'codex' as const }
|
||||
coordinator.observeHookStatus({ state: 'working', ...turn })
|
||||
coordinator.observeHookStatus({ state: 'blocked', ...turn, toolName: 'exec_command' })
|
||||
expect(dispatchAttention).not.toHaveBeenCalled()
|
||||
|
||||
vi.advanceTimersByTime(CODEX_ATTENTION_QUIET_MS)
|
||||
expect(dispatchAttention).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('cancels the debounced Codex attention when a completion lands in the window (no double notify)', () => {
|
||||
const dispatchAttention = vi.fn()
|
||||
const dispatchCompletion = vi.fn()
|
||||
const coordinator = createAgentCompletionCoordinator({
|
||||
paneKey: 'tab-1:leaf-1',
|
||||
getPtyId: () => 'pty-1',
|
||||
getSettings: () => null,
|
||||
inspectProcess: vi.fn(),
|
||||
dispatchCompletion,
|
||||
dispatchAttention,
|
||||
isLive: () => true
|
||||
})
|
||||
|
||||
const turn = { prompt: 'fix the bug', agentType: 'codex' as const }
|
||||
coordinator.observeHookStatus({ state: 'working', ...turn })
|
||||
coordinator.observeHookStatus({ state: 'waiting', ...turn, toolName: 'exec_command' })
|
||||
// A 'done' completing the turn inside the window must cancel the pending
|
||||
// attention so the pause never co-fires with the completion notification.
|
||||
coordinator.observeHookStatus({ state: 'done', ...turn })
|
||||
vi.advanceTimersByTime(CODEX_ATTENTION_QUIET_MS)
|
||||
|
||||
expect(dispatchAttention).not.toHaveBeenCalled()
|
||||
expect(dispatchCompletion).toHaveBeenCalledTimes(1)
|
||||
expect(vi.getTimerCount()).toBe(0)
|
||||
})
|
||||
|
||||
it('clears the pending Codex attention timer on dispose (no leak, no late fire)', () => {
|
||||
const dispatchAttention = vi.fn()
|
||||
const coordinator = createAgentCompletionCoordinator({
|
||||
paneKey: 'tab-1:leaf-1',
|
||||
getPtyId: () => 'pty-1',
|
||||
getSettings: () => null,
|
||||
inspectProcess: vi.fn(),
|
||||
dispatchCompletion: vi.fn(),
|
||||
dispatchAttention,
|
||||
isLive: () => true
|
||||
})
|
||||
|
||||
const turn = { prompt: 'fix the bug', agentType: 'codex' as const }
|
||||
coordinator.observeHookStatus({ state: 'working', ...turn })
|
||||
coordinator.observeHookStatus({ state: 'waiting', ...turn, toolName: 'exec_command' })
|
||||
expect(vi.getTimerCount()).toBe(1)
|
||||
|
||||
coordinator.dispose()
|
||||
expect(vi.getTimerCount()).toBe(0)
|
||||
|
||||
vi.advanceTimersByTime(CODEX_ATTENTION_QUIET_MS)
|
||||
expect(dispatchAttention).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('re-arms and fires a second distinct Codex pause after work resumed', () => {
|
||||
const dispatchAttention = vi.fn()
|
||||
const coordinator = createAgentCompletionCoordinator({
|
||||
paneKey: 'tab-1:leaf-1',
|
||||
getPtyId: () => 'pty-1',
|
||||
getSettings: () => null,
|
||||
inspectProcess: vi.fn(),
|
||||
dispatchCompletion: vi.fn(),
|
||||
dispatchAttention,
|
||||
isLive: () => true
|
||||
})
|
||||
|
||||
const turn = { prompt: 'fix the bug', agentType: 'codex' as const }
|
||||
coordinator.observeHookStatus({ state: 'working', ...turn })
|
||||
coordinator.observeHookStatus({ state: 'waiting', ...turn, toolName: 'exec_command', toolInput: 'ls' })
|
||||
// First pause auto-resolves before the window elapses.
|
||||
coordinator.observeHookStatus({ state: 'working', ...turn, toolName: 'exec_command', toolInput: 'ls' })
|
||||
vi.advanceTimersByTime(CODEX_ATTENTION_QUIET_MS)
|
||||
expect(dispatchAttention).not.toHaveBeenCalled()
|
||||
|
||||
// A later, genuinely-distinct pause must re-arm the debounce and fire.
|
||||
coordinator.observeHookStatus({ state: 'waiting', ...turn, toolName: 'apply_patch', toolInput: 'diff' })
|
||||
expect(dispatchAttention).not.toHaveBeenCalled()
|
||||
vi.advanceTimersByTime(CODEX_ATTENTION_QUIET_MS)
|
||||
expect(dispatchAttention).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('cancels the debounced Codex attention when a working-spinner title resumes', () => {
|
||||
// Why: a Codex resume can surface as a working title before the resume
|
||||
// 'working' hook lands; that title must also cancel the pending attention
|
||||
// so the self-resolving pause never fires a false banner (issue #8387).
|
||||
const dispatchAttention = vi.fn()
|
||||
const coordinator = createAgentCompletionCoordinator({
|
||||
paneKey: 'tab-1:leaf-1',
|
||||
getPtyId: () => 'pty-1',
|
||||
getSettings: () => null,
|
||||
inspectProcess: vi.fn(),
|
||||
dispatchCompletion: vi.fn(),
|
||||
dispatchAttention,
|
||||
isLive: () => true
|
||||
})
|
||||
|
||||
const turn = { prompt: 'fix the bug', agentType: 'codex' as const }
|
||||
coordinator.observeHookStatus({ state: 'working', ...turn })
|
||||
coordinator.observeHookStatus({ state: 'waiting', ...turn, toolName: 'exec_command' })
|
||||
expect(vi.getTimerCount()).toBe(1)
|
||||
|
||||
coordinator.observeTitleWorking()
|
||||
expect(vi.getTimerCount()).toBe(0)
|
||||
|
||||
vi.advanceTimersByTime(CODEX_ATTENTION_QUIET_MS)
|
||||
expect(dispatchAttention).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not let a null-foreground inspection blip drop the debounced Codex attention', async () => {
|
||||
// Why: guard for #8387 fail-open. In the pty-connection coordinator (real
|
||||
// process polling), a transient null/shell foreground blip — or a remote
|
||||
// inspection that cannot resolve the foreground — must not convert a genuine
|
||||
// Codex pause into a process-exit completion while the attention debounce is
|
||||
// still pending. Mirrors the pendingHookDoneTimer evidence-teardown guard.
|
||||
let foreground: string | null = 'codex'
|
||||
const dispatchAttention = vi.fn()
|
||||
const dispatchCompletion = vi.fn()
|
||||
const coordinator = createAgentCompletionCoordinator({
|
||||
paneKey: 'tab-1:leaf-1',
|
||||
getPtyId: () => 'pty-1',
|
||||
getSettings: () => null,
|
||||
inspectProcess: vi.fn(async () => processResult(foreground)),
|
||||
dispatchCompletion,
|
||||
dispatchAttention,
|
||||
isLive: () => true
|
||||
})
|
||||
|
||||
coordinator.startProcessTracking()
|
||||
// First cadence poll recognizes Codex as the foreground agent (active tier).
|
||||
await vi.advanceTimersByTimeAsync(2_000)
|
||||
await flushAsyncTicks()
|
||||
|
||||
// Codex pauses for a permission decision: the OS attention is debounced.
|
||||
const turn = { prompt: 'apply patch', agentType: 'codex' as const }
|
||||
coordinator.observeHookStatus({
|
||||
state: 'waiting',
|
||||
...turn,
|
||||
toolName: 'exec_command',
|
||||
toolInput: 'rm -rf build'
|
||||
})
|
||||
expect(dispatchAttention).not.toHaveBeenCalled()
|
||||
|
||||
// Foreground reads null for the whole window; without the guard this would
|
||||
// land a false process-exit completion racing/duplicating the pause banner.
|
||||
foreground = null
|
||||
await vi.advanceTimersByTimeAsync(CODEX_ATTENTION_QUIET_MS + 100)
|
||||
await flushAsyncTicks()
|
||||
|
||||
expect(dispatchCompletion).not.toHaveBeenCalled()
|
||||
expect(dispatchAttention).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('keeps a generic title completion pending long enough for the first remote inspection', async () => {
|
||||
const inspection = createDeferred<RuntimeTerminalProcessInspection>()
|
||||
const dispatchCompletion = vi.fn()
|
||||
|
||||
@@ -61,6 +61,12 @@ const PENDING_TITLE_TTL_MS = Math.max(2_000, INSPECTION_TIMEOUT_MS + 500)
|
||||
const PENDING_TITLE_MAX_TTL_MS = Math.max(30_000, PENDING_TITLE_TTL_MS)
|
||||
const COMPLETION_REPLAY_GUARD_MS = 1_000
|
||||
const HOOK_DONE_QUIET_MS = 1_500
|
||||
// Why: Codex fires its PermissionRequest hook at the human-input boundary before
|
||||
// the approval decision, so under "Approve for me" the review agent approves and
|
||||
// Codex resumes almost immediately. Debounce the OS attention notification behind
|
||||
// this quiet window so a self-resolving pause never raises a false "approval
|
||||
// required" banner (issue #8387). The visual status still updates immediately.
|
||||
const CODEX_ATTENTION_QUIET_MS = 1_500
|
||||
|
||||
const POLL_TIER_INTERVAL_MS: Record<PollCadenceTier, number> = {
|
||||
active: ACTIVE_POLL_INTERVAL_MS,
|
||||
@@ -106,6 +112,7 @@ export function createAgentCompletionCoordinator(
|
||||
let pendingHookDoneTimer: ReturnType<typeof setTimeout> | null = null
|
||||
let pendingHookDoneTitle: string | null = null
|
||||
let pendingHookDonePayload: AgentCompletionStatusSnapshot | null = null
|
||||
let pendingCodexAttentionTimer: ReturnType<typeof setTimeout> | null = null
|
||||
let pendingProcessExitAgent: RecognizedAgentProcess | null = null
|
||||
let pendingTitleSequence = 0
|
||||
let pendingTitle: {
|
||||
@@ -155,6 +162,13 @@ export function createAgentCompletionCoordinator(
|
||||
pendingHookDonePayload = null
|
||||
}
|
||||
|
||||
function clearPendingCodexAttention(): void {
|
||||
if (pendingCodexAttentionTimer !== null) {
|
||||
clearTimeout(pendingCodexAttentionTimer)
|
||||
pendingCodexAttentionTimer = null
|
||||
}
|
||||
}
|
||||
|
||||
function establishAgentEvidence(): void {
|
||||
agentIdentityEstablished = true
|
||||
hasAgentRunEvidence = true
|
||||
@@ -304,6 +318,9 @@ export function createAgentCompletionCoordinator(
|
||||
lastCompletedTurn = currentTurn
|
||||
lastCompletionSource = source
|
||||
workingStatusObserved = false
|
||||
// Why: any committed completion (hook/title/process-exit) ends the turn, so a
|
||||
// debounced Codex attention from an earlier pause must never fire after it.
|
||||
clearPendingCodexAttention()
|
||||
if (optionsOverride.completionIdentity) {
|
||||
lastCompletionIdentityByPaneKey.set(options.paneKey, optionsOverride.completionIdentity)
|
||||
}
|
||||
@@ -324,6 +341,13 @@ export function createAgentCompletionCoordinator(
|
||||
}
|
||||
}
|
||||
|
||||
function dispatchAttentionNotification(payload: AgentCompletionStatusSnapshot): void {
|
||||
options.dispatchAttention?.(payload.agentType ?? options.paneKey, {
|
||||
source: 'hook',
|
||||
agentStatus: payload
|
||||
})
|
||||
}
|
||||
|
||||
function dispatchAttention(payload: AgentCompletionStatusSnapshot): void {
|
||||
if (!options.dispatchAttention || !options.isLive() || !hasAgentRunEvidence) {
|
||||
return
|
||||
@@ -333,11 +357,25 @@ export function createAgentCompletionCoordinator(
|
||||
return
|
||||
}
|
||||
lastAttentionToken = token
|
||||
// Why: the visual "needs input" status must update immediately for every
|
||||
// agent; only the OS attention notification is debounced (Codex, below).
|
||||
options.dispatchHookLifecycle?.(payload)
|
||||
options.dispatchAttention(payload.agentType ?? options.paneKey, {
|
||||
source: 'hook',
|
||||
agentStatus: payload
|
||||
})
|
||||
if (payload.agentType === 'codex') {
|
||||
// Why: a Codex PermissionRequest that "Approve for me" auto-resolves fires a
|
||||
// working/completion hook inside this window, which cancels the pending
|
||||
// notification (see CODEX_ATTENTION_QUIET_MS). Scoped to Codex so every other
|
||||
// agent's genuine pause still notifies immediately.
|
||||
clearPendingCodexAttention()
|
||||
pendingCodexAttentionTimer = setTimeout(() => {
|
||||
pendingCodexAttentionTimer = null
|
||||
if (!options.isLive() || !hasAgentRunEvidence) {
|
||||
return
|
||||
}
|
||||
dispatchAttentionNotification(payload)
|
||||
}, CODEX_ATTENTION_QUIET_MS)
|
||||
return
|
||||
}
|
||||
dispatchAttentionNotification(payload)
|
||||
}
|
||||
|
||||
function scheduleHookDoneCompletion(title: string, payload: AgentCompletionStatusSnapshot): void {
|
||||
@@ -472,9 +510,12 @@ export function createAgentCompletionCoordinator(
|
||||
handleRecognizedProcess(recognized)
|
||||
return true
|
||||
}
|
||||
if (pendingHookDoneTimer !== null) {
|
||||
// Why: a pending quiet-window 'done' is the authoritative completion;
|
||||
// tearing down agent evidence here would make the timer drop it.
|
||||
if (pendingHookDoneTimer !== null || pendingCodexAttentionTimer !== null) {
|
||||
// Why: a pending quiet-window 'done' or debounced Codex attention is the
|
||||
// authoritative signal for this turn; tearing down agent evidence here (a
|
||||
// transient null/shell foreground blip before the agent process is
|
||||
// recognized) would make the timer's hasAgentRunEvidence guard silently
|
||||
// drop it. Keep the fail-open contract for #8387 as for the done timer.
|
||||
scheduleNextPoll()
|
||||
return false
|
||||
}
|
||||
@@ -696,6 +737,12 @@ export function createAgentCompletionCoordinator(
|
||||
) {
|
||||
return false
|
||||
}
|
||||
// Why: a genuine Codex resume can surface as a working-spinner title before
|
||||
// (or instead of) the resume 'working' hook, so cancel the debounced
|
||||
// attention here or the self-resolving pause still fires a false banner
|
||||
// (#8387). Placed after the replay guard so only an authoritative resume —
|
||||
// not a stale post-completion title replay — drops a still-pending banner.
|
||||
clearPendingCodexAttention()
|
||||
workingStatusObserved = true
|
||||
requiresFreshWorking = false
|
||||
lastCompletionIdentityByPaneKey.delete(options.paneKey)
|
||||
@@ -789,6 +836,7 @@ export function createAgentCompletionCoordinator(
|
||||
// so the quiet-window timer never fires a false completion notification.
|
||||
if (isAttentionHookState(payload.state)) {
|
||||
clearPendingHookDone()
|
||||
clearPendingCodexAttention()
|
||||
}
|
||||
return
|
||||
}
|
||||
@@ -797,6 +845,9 @@ export function createAgentCompletionCoordinator(
|
||||
}
|
||||
if (payload.state === 'working') {
|
||||
clearPendingHookDone()
|
||||
// Why: resumed work (e.g. Codex after "Approve for me") cancels the debounced
|
||||
// attention notification so the self-resolving pause never notifies.
|
||||
clearPendingCodexAttention()
|
||||
workingStatusObserved = true
|
||||
requiresFreshWorking = false
|
||||
lastCompletionIdentity = null
|
||||
@@ -814,6 +865,9 @@ export function createAgentCompletionCoordinator(
|
||||
return
|
||||
}
|
||||
if (isCompletionHookState(payload.state)) {
|
||||
// Why: the turn is ending, so a debounced attention from an earlier pause in
|
||||
// this turn must not fire after the completion notification.
|
||||
clearPendingCodexAttention()
|
||||
if (isRecognizedAgentType(payload.agentType)) {
|
||||
establishAgentEvidence()
|
||||
}
|
||||
@@ -885,6 +939,7 @@ export function createAgentCompletionCoordinator(
|
||||
|
||||
function resetCompletionState(options: { requireFreshWorking?: boolean } = {}): void {
|
||||
clearPendingHookDone()
|
||||
clearPendingCodexAttention()
|
||||
dropPendingTitle()
|
||||
agentIdentityEstablished = false
|
||||
hasAgentRunEvidence = false
|
||||
@@ -905,6 +960,7 @@ export function createAgentCompletionCoordinator(
|
||||
disposed = true
|
||||
clearPollTimer()
|
||||
clearPendingHookDone()
|
||||
clearPendingCodexAttention()
|
||||
dropPendingTitle()
|
||||
// Why: the dedup identity is module-scoped so it survives a live-stream remount
|
||||
// (dispose-then-recreate with the same paneKey while isLive() stays true). Only
|
||||
|
||||
@@ -56,6 +56,9 @@ type MockStoreState = {
|
||||
|
||||
let mockStoreState: MockStoreState
|
||||
const HOOK_DONE_QUIET_MS = 1_500
|
||||
// Why: Codex attention notifications are debounced (issue #8387), so a genuine
|
||||
// permission pause only notifies once this quiet window elapses without resuming.
|
||||
const CODEX_ATTENTION_QUIET_MS = 1_500
|
||||
|
||||
vi.mock('@/store', () => ({
|
||||
useAppStore: {
|
||||
@@ -106,6 +109,31 @@ function seedCodexPaneLaunchConfig(
|
||||
describe('agent hook completion notifications', () => {
|
||||
const paneKey = 'tab-1:11111111-1111-4111-8111-111111111111'
|
||||
|
||||
// Why: the Codex permission-pause tests share a working→pause→quiet-window
|
||||
// sequence; centralizing it keeps the debounce advance (issue #8387) in one spot.
|
||||
async function observeCodexPermissionPause(state: 'waiting' | 'blocked'): Promise<void> {
|
||||
const { observeAgentHookCompletionForNotification } = await import(
|
||||
'./agent-hook-completion-notifications'
|
||||
)
|
||||
observeAgentHookCompletionForNotification({
|
||||
paneKey,
|
||||
worktreeId: 'wt-1',
|
||||
payload: hookStatus('working')
|
||||
})
|
||||
observeAgentHookCompletionForNotification({
|
||||
paneKey,
|
||||
worktreeId: 'wt-1',
|
||||
payload: {
|
||||
state,
|
||||
prompt: 'implement notifications',
|
||||
agentType: 'codex',
|
||||
toolName: 'exec_command',
|
||||
toolInput: 'git status'
|
||||
}
|
||||
})
|
||||
vi.advanceTimersByTime(CODEX_ATTENTION_QUIET_MS)
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetModules()
|
||||
vi.useFakeTimers()
|
||||
@@ -560,50 +588,14 @@ describe('agent hook completion notifications', () => {
|
||||
|
||||
it('fails open for Codex auto-approved permission requests without launch proof', async () => {
|
||||
seedCodexPaneLaunchConfig(paneKey, YOLO_TUI_AGENT_ARGS.codex ?? '')
|
||||
const { observeAgentHookCompletionForNotification } =
|
||||
await import('./agent-hook-completion-notifications')
|
||||
|
||||
observeAgentHookCompletionForNotification({
|
||||
paneKey,
|
||||
worktreeId: 'wt-1',
|
||||
payload: hookStatus('working')
|
||||
})
|
||||
observeAgentHookCompletionForNotification({
|
||||
paneKey,
|
||||
worktreeId: 'wt-1',
|
||||
payload: {
|
||||
state: 'waiting',
|
||||
prompt: 'implement notifications',
|
||||
agentType: 'codex',
|
||||
toolName: 'exec_command',
|
||||
toolInput: 'git status'
|
||||
}
|
||||
})
|
||||
await observeCodexPermissionPause('waiting')
|
||||
|
||||
expect(dispatchTerminalNotification).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('still notifies for manual Codex permission requests', async () => {
|
||||
seedCodexPaneLaunchConfig(paneKey, '')
|
||||
const { observeAgentHookCompletionForNotification } =
|
||||
await import('./agent-hook-completion-notifications')
|
||||
|
||||
observeAgentHookCompletionForNotification({
|
||||
paneKey,
|
||||
worktreeId: 'wt-1',
|
||||
payload: hookStatus('working')
|
||||
})
|
||||
observeAgentHookCompletionForNotification({
|
||||
paneKey,
|
||||
worktreeId: 'wt-1',
|
||||
payload: {
|
||||
state: 'waiting',
|
||||
prompt: 'implement notifications',
|
||||
agentType: 'codex',
|
||||
toolName: 'exec_command',
|
||||
toolInput: 'git status'
|
||||
}
|
||||
})
|
||||
await observeCodexPermissionPause('waiting')
|
||||
|
||||
expect(dispatchTerminalNotification).toHaveBeenCalledTimes(1)
|
||||
expect(dispatchTerminalNotification).toHaveBeenCalledWith(
|
||||
@@ -618,25 +610,7 @@ describe('agent hook completion notifications', () => {
|
||||
|
||||
it('fails open for Codex auto-approved blocked permission requests without launch proof', async () => {
|
||||
seedCodexPaneLaunchConfig(paneKey, YOLO_TUI_AGENT_ARGS.codex ?? '')
|
||||
const { observeAgentHookCompletionForNotification } =
|
||||
await import('./agent-hook-completion-notifications')
|
||||
|
||||
observeAgentHookCompletionForNotification({
|
||||
paneKey,
|
||||
worktreeId: 'wt-1',
|
||||
payload: hookStatus('working')
|
||||
})
|
||||
observeAgentHookCompletionForNotification({
|
||||
paneKey,
|
||||
worktreeId: 'wt-1',
|
||||
payload: {
|
||||
state: 'blocked',
|
||||
prompt: 'implement notifications',
|
||||
agentType: 'codex',
|
||||
toolName: 'exec_command',
|
||||
toolInput: 'git status'
|
||||
}
|
||||
})
|
||||
await observeCodexPermissionPause('blocked')
|
||||
|
||||
expect(dispatchTerminalNotification).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { DiscoveredSkill } from '../../../shared/skills'
|
||||
import type { TuiAgent } from '../../../shared/types'
|
||||
import {
|
||||
agentHasOrchestrationSkill,
|
||||
getOrchestrationSkillAgentStatuses
|
||||
@@ -97,7 +98,102 @@ describe('orchestration skill agent coverage', () => {
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('matches Windows skill paths', () => {
|
||||
it('marks each provider-home agent from its own global skills location', () => {
|
||||
const cases: { agent: TuiAgent; rootPath: string; directoryPath: string }[] = [
|
||||
{
|
||||
agent: 'grok',
|
||||
rootPath: '/Users/test/.grok/skills',
|
||||
directoryPath: '/Users/test/.grok/skills/orchestration'
|
||||
},
|
||||
{
|
||||
agent: 'opencode',
|
||||
rootPath: '/Users/test/.config/opencode/skills',
|
||||
directoryPath: '/Users/test/.config/opencode/skills/orchestration'
|
||||
},
|
||||
{
|
||||
agent: 'pi',
|
||||
rootPath: '/Users/test/.pi/agent/skills',
|
||||
directoryPath: '/Users/test/.pi/agent/skills/orchestration'
|
||||
},
|
||||
{
|
||||
agent: 'gemini',
|
||||
rootPath: '/Users/test/.gemini/skills',
|
||||
directoryPath: '/Users/test/.gemini/skills/orchestration'
|
||||
},
|
||||
{
|
||||
agent: 'antigravity',
|
||||
rootPath: '/Users/test/.gemini/antigravity/skills',
|
||||
directoryPath: '/Users/test/.gemini/antigravity/skills/orchestration'
|
||||
},
|
||||
{
|
||||
agent: 'cursor',
|
||||
rootPath: '/Users/test/.cursor/skills',
|
||||
directoryPath: '/Users/test/.cursor/skills/orchestration'
|
||||
}
|
||||
]
|
||||
for (const { agent, rootPath, directoryPath } of cases) {
|
||||
const skills = [
|
||||
skill({ providers: ['agent-skills'], sourceKind: 'home', rootPath, directoryPath })
|
||||
]
|
||||
expect(agentHasOrchestrationSkill(agent, skills)).toBe(true)
|
||||
// Why: a provider-home install must not leak coverage to unrelated agents.
|
||||
expect(agentHasOrchestrationSkill('claude', skills)).toBe(false)
|
||||
}
|
||||
})
|
||||
|
||||
it('marks a multi-segment provider-home agent from a Windows-style path', () => {
|
||||
expect(
|
||||
agentHasOrchestrationSkill('opencode', [
|
||||
skill({
|
||||
providers: ['agent-skills'],
|
||||
sourceKind: 'home',
|
||||
rootPath: 'C:\\Users\\test\\.config\\opencode\\skills',
|
||||
directoryPath: 'C:\\Users\\test\\.config\\opencode\\skills\\orchestration'
|
||||
})
|
||||
])
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('keeps Gemini and Antigravity distinct despite sharing the ~/.gemini root', () => {
|
||||
const geminiInstall = [
|
||||
skill({
|
||||
providers: ['agent-skills'],
|
||||
sourceKind: 'home',
|
||||
rootPath: '/Users/test/.gemini/skills',
|
||||
directoryPath: '/Users/test/.gemini/skills/orchestration'
|
||||
})
|
||||
]
|
||||
const antigravityInstall = [
|
||||
skill({
|
||||
providers: ['agent-skills'],
|
||||
sourceKind: 'home',
|
||||
rootPath: '/Users/test/.gemini/antigravity/skills',
|
||||
directoryPath: '/Users/test/.gemini/antigravity/skills/orchestration'
|
||||
})
|
||||
]
|
||||
|
||||
// Why: `.gemini/skills` and `.gemini/antigravity/skills` are siblings, so a
|
||||
// segment matcher must not let one provider's install mark the other.
|
||||
expect(agentHasOrchestrationSkill('gemini', geminiInstall)).toBe(true)
|
||||
expect(agentHasOrchestrationSkill('antigravity', geminiInstall)).toBe(false)
|
||||
expect(agentHasOrchestrationSkill('antigravity', antigravityInstall)).toBe(true)
|
||||
expect(agentHasOrchestrationSkill('gemini', antigravityInstall)).toBe(false)
|
||||
})
|
||||
|
||||
it('marks Claude Agent Teams from ~/.claude/skills like Claude Code', () => {
|
||||
const skills = [
|
||||
skill({
|
||||
providers: ['claude'],
|
||||
sourceKind: 'home',
|
||||
rootPath: '/Users/test/.claude/skills',
|
||||
directoryPath: '/Users/test/.claude/skills/orchestration'
|
||||
})
|
||||
]
|
||||
|
||||
expect(agentHasOrchestrationSkill('claude-agent-teams', skills)).toBe(true)
|
||||
})
|
||||
|
||||
it('marks Windows skill paths', () => {
|
||||
expect(
|
||||
agentHasOrchestrationSkill('codex', [
|
||||
skill({
|
||||
|
||||
@@ -9,6 +9,12 @@ export type OrchestrationSkillLocationId =
|
||||
| 'codex-home'
|
||||
| 'codex-plugin-cache'
|
||||
| 'agents-home'
|
||||
| 'grok-home'
|
||||
| 'opencode-home'
|
||||
| 'pi-home'
|
||||
| 'gemini-home'
|
||||
| 'antigravity-home'
|
||||
| 'cursor-home'
|
||||
|
||||
export type OrchestrationSkillAgentStatus = {
|
||||
agent: TuiAgent
|
||||
@@ -45,6 +51,43 @@ const ORCHESTRATION_SKILL_LOCATIONS: readonly OrchestrationSkillLocationDefiniti
|
||||
matchesSkill: (skill) =>
|
||||
isGlobalOrchestrationSkill(skill) &&
|
||||
pathContainsSegments(skill.rootPath, ['.agents', 'skills'])
|
||||
},
|
||||
{
|
||||
id: 'grok-home',
|
||||
matchesSkill: (skill) =>
|
||||
isGlobalOrchestrationSkill(skill) && pathContainsSegments(skill.rootPath, ['.grok', 'skills'])
|
||||
},
|
||||
{
|
||||
id: 'opencode-home',
|
||||
matchesSkill: (skill) =>
|
||||
isGlobalOrchestrationSkill(skill) &&
|
||||
pathContainsSegments(skill.rootPath, ['.config', 'opencode', 'skills'])
|
||||
},
|
||||
{
|
||||
id: 'pi-home',
|
||||
matchesSkill: (skill) =>
|
||||
isGlobalOrchestrationSkill(skill) &&
|
||||
pathContainsSegments(skill.rootPath, ['.pi', 'agent', 'skills'])
|
||||
},
|
||||
{
|
||||
// Why: segment matching keeps `.gemini/skills` (Gemini CLI) distinct from the
|
||||
// sibling `.gemini/antigravity/skills`, so the two never cross-mark each other.
|
||||
id: 'gemini-home',
|
||||
matchesSkill: (skill) =>
|
||||
isGlobalOrchestrationSkill(skill) &&
|
||||
pathContainsSegments(skill.rootPath, ['.gemini', 'skills'])
|
||||
},
|
||||
{
|
||||
id: 'antigravity-home',
|
||||
matchesSkill: (skill) =>
|
||||
isGlobalOrchestrationSkill(skill) &&
|
||||
pathContainsSegments(skill.rootPath, ['.gemini', 'antigravity', 'skills'])
|
||||
},
|
||||
{
|
||||
id: 'cursor-home',
|
||||
matchesSkill: (skill) =>
|
||||
isGlobalOrchestrationSkill(skill) &&
|
||||
pathContainsSegments(skill.rootPath, ['.cursor', 'skills'])
|
||||
}
|
||||
]
|
||||
|
||||
@@ -52,8 +95,16 @@ const ORCHESTRATION_SKILL_LOCATION_IDS_BY_AGENT: Partial<
|
||||
Record<TuiAgent, readonly OrchestrationSkillLocationId[]>
|
||||
> = {
|
||||
claude: ['claude-home', 'agents-home'],
|
||||
// Why: Agent Teams runs Claude Code, so it reads the same ~/.claude skills.
|
||||
'claude-agent-teams': ['claude-home', 'agents-home'],
|
||||
openclaude: ['claude-home', 'agents-home'],
|
||||
codex: ['codex-home', 'codex-plugin-cache', 'agents-home']
|
||||
codex: ['codex-home', 'codex-plugin-cache', 'agents-home'],
|
||||
grok: ['grok-home', 'agents-home'],
|
||||
opencode: ['opencode-home', 'agents-home'],
|
||||
pi: ['pi-home', 'agents-home'],
|
||||
gemini: ['gemini-home', 'agents-home'],
|
||||
antigravity: ['antigravity-home', 'agents-home'],
|
||||
cursor: ['cursor-home', 'agents-home']
|
||||
}
|
||||
|
||||
function normalizeSkillName(value: string): string {
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { getRepoDisplayLabelsByPath } from './repo-display-labels'
|
||||
import { getRepoDisplayLabelKey, getRepoDisplayLabelsByPath } from './repo-display-labels'
|
||||
|
||||
describe('getRepoDisplayLabelsByPath', () => {
|
||||
it('keeps non-colliding repository names basename-only', () => {
|
||||
@@ -8,8 +8,10 @@ describe('getRepoDisplayLabelsByPath', () => {
|
||||
{ path: '/workspace/platform/worker', displayName: 'worker' }
|
||||
])
|
||||
|
||||
expect(labels.get('/workspace/platform/web')).toBe('web')
|
||||
expect(labels.get('/workspace/platform/worker')).toBe('worker')
|
||||
expect(labels.get(getRepoDisplayLabelKey({ path: '/workspace/platform/web' }))).toBe('web')
|
||||
expect(labels.get(getRepoDisplayLabelKey({ path: '/workspace/platform/worker' }))).toBe(
|
||||
'worker'
|
||||
)
|
||||
})
|
||||
|
||||
it('adds the minimal real parent suffix only for colliding basenames', () => {
|
||||
@@ -19,9 +21,13 @@ describe('getRepoDisplayLabelsByPath', () => {
|
||||
{ path: '/workspace/platform/billing/api', displayName: 'api' }
|
||||
])
|
||||
|
||||
expect(labels.get('/workspace/platform/web')).toBe('web')
|
||||
expect(labels.get('/workspace/platform/payments/api')).toBe('payments/api')
|
||||
expect(labels.get('/workspace/platform/billing/api')).toBe('billing/api')
|
||||
expect(labels.get(getRepoDisplayLabelKey({ path: '/workspace/platform/web' }))).toBe('web')
|
||||
expect(labels.get(getRepoDisplayLabelKey({ path: '/workspace/platform/payments/api' }))).toBe(
|
||||
'payments/api'
|
||||
)
|
||||
expect(labels.get(getRepoDisplayLabelKey({ path: '/workspace/platform/billing/api' }))).toBe(
|
||||
'billing/api'
|
||||
)
|
||||
})
|
||||
|
||||
it('expands colliding labels in lockstep without skipping shared segments', () => {
|
||||
@@ -30,8 +36,39 @@ describe('getRepoDisplayLabelsByPath', () => {
|
||||
{ path: '/workspace/team2/shared/api', displayName: 'api' }
|
||||
])
|
||||
|
||||
expect(labels.get('/workspace/team1/shared/api')).toBe('team1/shared/api')
|
||||
expect(labels.get('/workspace/team2/shared/api')).toBe('team2/shared/api')
|
||||
expect(labels.get(getRepoDisplayLabelKey({ path: '/workspace/team1/shared/api' }))).toBe(
|
||||
'team1/shared/api'
|
||||
)
|
||||
expect(labels.get(getRepoDisplayLabelKey({ path: '/workspace/team2/shared/api' }))).toBe(
|
||||
'team2/shared/api'
|
||||
)
|
||||
})
|
||||
|
||||
it('scopes labels by execution host so same-path repos on different hosts do not collide', () => {
|
||||
// Real SSH folder-repo shape: connectionId set, executionHostId unset — so
|
||||
// it must fall back to the connection host, not look identical to a local repo.
|
||||
const localRepo = { path: '/Users/alice', displayName: 'alice' }
|
||||
const sshRepo = { path: '/Users/alice', displayName: 'alice-prod', connectionId: 'prod-ssh' }
|
||||
const labels = getRepoDisplayLabelsByPath([localRepo, sshRepo])
|
||||
|
||||
expect(labels.get(getRepoDisplayLabelKey(localRepo))).toBe('alice')
|
||||
expect(labels.get(getRepoDisplayLabelKey(sshRepo))).toBe('alice-prod')
|
||||
expect(labels.size).toBe(2)
|
||||
})
|
||||
|
||||
it('keeps cross-host repos with identical path AND name as separate entries', () => {
|
||||
// Hardening: when paths are byte-identical the same-name collision loop runs
|
||||
// and re-sets each entry, so host scoping must survive that pass too — neither
|
||||
// host may overwrite the other. Label text can still coincide; that residual is
|
||||
// disambiguated by the host section/badge, not this map.
|
||||
const localRepo = { path: '/Users/alice', displayName: 'home' }
|
||||
const sshRepo = { path: '/Users/alice', displayName: 'home', connectionId: 'prod-ssh' }
|
||||
const labels = getRepoDisplayLabelsByPath([localRepo, sshRepo])
|
||||
|
||||
expect(getRepoDisplayLabelKey(localRepo)).not.toBe(getRepoDisplayLabelKey(sshRepo))
|
||||
expect(labels.size).toBe(2)
|
||||
expect(labels.get(getRepoDisplayLabelKey(localRepo))).toBeDefined()
|
||||
expect(labels.get(getRepoDisplayLabelKey(sshRepo))).toBeDefined()
|
||||
})
|
||||
|
||||
it('normalizes Windows separators to slash display labels', () => {
|
||||
@@ -40,7 +77,11 @@ describe('getRepoDisplayLabelsByPath', () => {
|
||||
{ path: 'C:\\workspace\\billing\\api', displayName: 'api' }
|
||||
])
|
||||
|
||||
expect(labels.get('C:\\workspace\\payments\\api')).toBe('payments/api')
|
||||
expect(labels.get('C:\\workspace\\billing\\api')).toBe('billing/api')
|
||||
expect(labels.get(getRepoDisplayLabelKey({ path: 'C:\\workspace\\payments\\api' }))).toBe(
|
||||
'payments/api'
|
||||
)
|
||||
expect(labels.get(getRepoDisplayLabelKey({ path: 'C:\\workspace\\billing\\api' }))).toBe(
|
||||
'billing/api'
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,6 +1,21 @@
|
||||
import { getRepoExecutionHostId, type ExecutionHostId } from '../../../shared/execution-host'
|
||||
|
||||
type RepoDisplayLabelItem = {
|
||||
path: string
|
||||
displayName: string
|
||||
connectionId?: string | null
|
||||
executionHostId?: ExecutionHostId | null
|
||||
}
|
||||
|
||||
// Why: two repos can share the same absolute path across hosts (e.g. a local
|
||||
// /Users/alice and an SSH host's /Users/alice). Keying labels by raw path alone
|
||||
// lets one repo's label overwrite the other's, so scope the key by execution
|
||||
// host. getRepoExecutionHostId returns 'local' for local repos and falls back to
|
||||
// the connectionId (ssh:<id>) for SSH folder-repos that leave executionHostId unset.
|
||||
export function getRepoDisplayLabelKey(
|
||||
item: Pick<RepoDisplayLabelItem, 'path' | 'connectionId' | 'executionHostId'>
|
||||
): string {
|
||||
return `${getRepoExecutionHostId(item)}::${item.path}`
|
||||
}
|
||||
|
||||
function normalizePathSegments(path: string): string[] {
|
||||
@@ -29,7 +44,7 @@ export function getRepoDisplayLabelsByPath(
|
||||
|
||||
for (const item of items) {
|
||||
const displayName = item.displayName || item.path
|
||||
labels.set(item.path, displayName)
|
||||
labels.set(getRepoDisplayLabelKey(item), displayName)
|
||||
const colliding = itemsByName.get(displayName) ?? []
|
||||
colliding.push({ ...item, displayName })
|
||||
itemsByName.set(displayName, colliding)
|
||||
@@ -49,7 +64,7 @@ export function getRepoDisplayLabelsByPath(
|
||||
nextLabels = collidingItems.map((item) => labelForDepth(item, depth))
|
||||
}
|
||||
collidingItems.forEach((item, index) => {
|
||||
labels.set(item.path, nextLabels[index] ?? item.displayName)
|
||||
labels.set(getRepoDisplayLabelKey(item), nextLabels[index] ?? item.displayName)
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -456,7 +456,7 @@ describe('createEditorSlice openDiff', () => {
|
||||
expect(store.getState().openFiles).toEqual([
|
||||
expect.objectContaining({
|
||||
id: 'wt-1::diff::unstaged::file.ts',
|
||||
runtimeEnvironmentId: undefined
|
||||
runtimeEnvironmentId: null
|
||||
}),
|
||||
expect.objectContaining({
|
||||
id: 'editor-diff:wt-1:env-1:unstaged:file.ts',
|
||||
@@ -498,6 +498,66 @@ describe('createEditorSlice openDiff', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('keeps a diff for an owner-less worktree off the focused global runtime', () => {
|
||||
const store = createEditorStore()
|
||||
// A remote runtime is globally focused, but wt-1's repo names no explicit
|
||||
// owner. The diff must stamp null (not undefined): null forces a LOCAL read
|
||||
// in settingsForRuntimeOwner, while undefined would inherit 'focused-env'.
|
||||
store.setState({
|
||||
settings: { activeRuntimeEnvironmentId: 'focused-env' } as AppState['settings']
|
||||
})
|
||||
|
||||
store.getState().openDiff('wt-1', '/repo/file.ts', 'file.ts', 'typescript', false)
|
||||
|
||||
expect(store.getState().openFiles[0]).toEqual(
|
||||
expect.objectContaining({
|
||||
id: 'wt-1::diff::unstaged::file.ts',
|
||||
runtimeEnvironmentId: null
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('routes an explicitly runtime-owned worktree diff to its owner over the focused runtime', () => {
|
||||
const store = createEditorStore()
|
||||
store.setState({
|
||||
settings: { activeRuntimeEnvironmentId: 'focused-env' } as AppState['settings'],
|
||||
repos: [
|
||||
{ id: 'repo-1', executionHostId: 'runtime:owner-env' }
|
||||
] as unknown as AppState['repos'],
|
||||
worktreesByRepo: {
|
||||
'repo-1': [{ id: 'repo-1::/srv/wt', repoId: 'repo-1', hostId: 'runtime:owner-env' }]
|
||||
} as unknown as AppState['worktreesByRepo']
|
||||
})
|
||||
|
||||
store.getState().openDiff('repo-1::/srv/wt', '/srv/wt/file.ts', 'file.ts', 'typescript', false)
|
||||
|
||||
expect(store.getState().openFiles[0]).toEqual(
|
||||
expect.objectContaining({ runtimeEnvironmentId: 'owner-env' })
|
||||
)
|
||||
})
|
||||
|
||||
it('keeps an SSH-owned worktree diff off the focused runtime so it routes via its connection', () => {
|
||||
const store = createEditorStore()
|
||||
// An SSH worktree is owned by its connection, not the focused runtime. Its
|
||||
// diff stamps null (not the focused env), so the read targets local IPC and
|
||||
// flows over connectionId rather than the focused runtime's RPC.
|
||||
store.setState({
|
||||
settings: { activeRuntimeEnvironmentId: 'focused-env' } as AppState['settings'],
|
||||
repos: [{ id: 'repo-ssh', connectionId: 'conn-1' }] as unknown as AppState['repos'],
|
||||
worktreesByRepo: {
|
||||
'repo-ssh': [{ id: 'repo-ssh::/srv/wt', repoId: 'repo-ssh', hostId: 'ssh:conn-1' }]
|
||||
} as unknown as AppState['worktreesByRepo']
|
||||
})
|
||||
|
||||
store
|
||||
.getState()
|
||||
.openDiff('repo-ssh::/srv/wt', '/srv/wt/file.ts', 'file.ts', 'typescript', false)
|
||||
|
||||
expect(store.getState().openFiles[0]).toEqual(
|
||||
expect.objectContaining({ runtimeEnvironmentId: null })
|
||||
)
|
||||
})
|
||||
|
||||
it('repairs an existing diff tab entry to the correct mode and staged state', () => {
|
||||
const store = createEditorStore()
|
||||
|
||||
|
||||
@@ -63,7 +63,7 @@ import {
|
||||
import { settingsForRuntimeOwner } from '@/runtime/runtime-rpc-client'
|
||||
import { notifyHostOfMirroredEditorClose } from '@/runtime/close-mirrored-editor-tab'
|
||||
import { findWorktreeById, getRepoIdFromWorktreeId } from './worktree-helpers'
|
||||
import { getRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner'
|
||||
import { getExplicitRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner'
|
||||
import {
|
||||
addAdditionalValidWorkspaceKeys,
|
||||
type WorkspaceSessionHydrationOptions
|
||||
@@ -335,9 +335,11 @@ function resolveDiffRuntimeEnvironmentId(
|
||||
if (explicitRuntimeEnvironmentId !== undefined) {
|
||||
return explicitRuntimeEnvironmentId
|
||||
}
|
||||
// Why: Source Control callers often know only the worktree. Runtime-host
|
||||
// diffs still need their owner stamped so content loads through runtime RPC.
|
||||
return getRuntimeEnvironmentIdForWorktree(state, worktreeId) ?? undefined
|
||||
// Why: route diffs by the worktree's EXPLICIT owner (#6957); owner-less/local
|
||||
// resolves to null so the read is forced LOCAL. undefined would instead
|
||||
// inherit the focused runtime in settingsForRuntimeOwner and read the diff
|
||||
// from the wrong host — the exact bug in #8484.
|
||||
return getExplicitRuntimeEnvironmentIdForWorktree(state, worktreeId) ?? null
|
||||
}
|
||||
|
||||
export type PendingEditorReveal = {
|
||||
|
||||
Reference in New Issue
Block a user