fix(native-chat): stop an advisory refresh ending the click, and one click per row

Manual QA found the reveal never ran: the inventory refresh that precedes it
is an optimization, but its failure returned early with 'not available yet,
retry in a moment' — reinstating the dead end this PR removes, one step
earlier. A failed refresh now falls through to the reveal, which is the repair
and does not need the refresh to have worked.

The click can chain a refresh, a capability probe, a reveal and a second
refresh, each with its own timeout, while nothing on the row says it is
working. A per-session in-flight guard keeps an impatient second click from
running the whole sequence again and landing its own toast.

Also drops an unreachable owner scope: the snapshot apply discards any
worktree whose execution host is not local before it reads one, so naming a
remote scope there described a synchronisation that cannot happen.
This commit is contained in:
Merge Sim
2026-09-05 19:12:22 -07:00
parent 6e7f3d660e
commit b3284600e4
2 changed files with 80 additions and 12 deletions
@@ -77,6 +77,21 @@ describe('activateAiVaultStructuredSession', () => {
expect(parts.gone).not.toHaveBeenCalled()
})
it('still reveals when the inventory refresh itself fails', async () => {
// The refresh is an optimization. Letting its failure end the click reinstates the dead end
// this whole path exists to remove: a closed chat, and advice to retry that cannot come true.
const parts = deps({
activate: vi.fn().mockReturnValueOnce(false).mockReturnValue(true),
refresh: vi.fn().mockRejectedValueOnce(new Error('structured_session_restore_timeout'))
})
await expect(activateAiVaultStructuredSession(structuredSession, parts)).resolves.toBe(true)
expect(parts.reveal).toHaveBeenCalledOnce()
expect(parts.unavailable).not.toHaveBeenCalled()
expect(parts.gone).not.toHaveBeenCalled()
})
it('does not strand the click when the post-reveal refresh fails', async () => {
const parts = deps({
activate: vi.fn().mockReturnValueOnce(false).mockReturnValueOnce(false).mockReturnValue(true),
@@ -123,6 +138,26 @@ describe('activateAiVaultStructuredSession', () => {
expect(parts.hostCannotOpen).not.toHaveBeenCalled()
})
it('runs one activation per session however many times the row is clicked', async () => {
// Three clicks on a slow row used to run three full sequences and land three toasts.
let release!: (outcome: 'gone') => void
const pending = new Promise<'gone'>((resolve) => {
release = resolve
})
const parts = deps({ activate: vi.fn(() => false), reveal: vi.fn(() => pending) })
const clicks = [
activateAiVaultStructuredSession(structuredSession, parts),
activateAiVaultStructuredSession(structuredSession, parts),
activateAiVaultStructuredSession(structuredSession, parts)
]
release('gone')
await Promise.all(clicks)
expect(parts.reveal).toHaveBeenCalledOnce()
expect(parts.gone).toHaveBeenCalledOnce()
})
it('ignores a row that is not a structured chat', async () => {
const parts = deps()
@@ -88,15 +88,35 @@ export async function activateAiVaultStructuredSession(
if (!structured) {
return false
}
// Why: the click can chain a refresh, a capability probe, a reveal and a second refresh, each
// with its own timeout, and nothing on the row says it is working. Without this, an impatient
// second click runs the whole sequence again and lands its own toast.
const inFlight = activationsInFlight.get(structured.sessionId)
if (inFlight) {
return inFlight
}
const activation = activateStructuredSession(structured, deps)
activationsInFlight.set(structured.sessionId, activation)
try {
return await activation
} finally {
activationsInFlight.delete(structured.sessionId)
}
}
const activationsInFlight = new Map<string, Promise<boolean>>()
async function activateStructuredSession(
structured: NonNullable<AiVaultSession['structuredSession']>,
deps: StructuredSessionActivationDeps
): Promise<boolean> {
const target = { worktreeId: structured.workspaceId, sessionId: structured.sessionId }
if (!deps.activate(target)) {
try {
await deps.refresh(structured.workspaceId)
} catch {
deps.unavailable()
return true
}
if (!deps.activate(target)) {
// A refresh alone can answer, and costs one call instead of two. It is only ever an
// optimization, so a refresh that fails must fall through to the reveal rather than end the
// click: the host republishing the tab is the repair, and it does not need this to have worked.
const refreshed = await refreshedWithoutThrowing(deps, structured.workspaceId)
if (!refreshed || !deps.activate(target)) {
// The inventory genuinely does not carry this chat: it was closed, or this process never
// published it. Ask the host to republish the tab from the record it still holds on disk.
const revealed = await deps.reveal(target)
@@ -111,7 +131,7 @@ export async function activateAiVaultStructuredSession(
}
return true
}
await deps.refresh(structured.workspaceId).catch(() => undefined)
await refreshedWithoutThrowing(deps, structured.workspaceId)
if (!deps.activate(target)) {
deps.unavailable()
return true
@@ -124,6 +144,20 @@ export async function activateAiVaultStructuredSession(
return true
}
/** The inventory refresh is advisory at every call site here, so its failure is a `false`, never a
* thrown end to the click. */
async function refreshedWithoutThrowing(
deps: StructuredSessionActivationDeps,
worktreeId: string
): Promise<boolean> {
try {
await deps.refresh(worktreeId)
return true
} catch {
return false
}
}
/**
* Ask the host to republish a persisted chat's tab.
*
@@ -196,10 +230,9 @@ async function refreshStructuredSessionTabs(worktreeId: string): Promise<void> {
{ timeoutMs: STRUCTURED_SESSION_RESTORE_TIMEOUT_MS }
)
)
applyStructuredSessionTabSnapshots(
[snapshot],
environmentId ? `structured-session:${environmentId}` : undefined
)
// No owner scope: the apply discards any worktree whose execution host is not local before it
// reads one, so a paired workspace is carried by the subscription, not by this call.
applyStructuredSessionTabSnapshots([snapshot])
}
async function withStructuredSessionRestoreTimeout<T>(promise: Promise<T>): Promise<T> {