mirror of
https://github.com/stablyai/orca.git
synced 2026-09-25 08:02:31 +00:00
fix(native-chat): wait for the runtime capability probe before resolving the creation launch route (#19819)
* fix(native-chat): wait for the runtime capability probe before resolving the launch route
A worktree created before the renderer's hydration-gated capability refresh
runs read the local capability set as null, which
resolveStructuredNativeChatSupport treats as a blocker, silently degrading
structured native chat to the legacy terminal-backed route. Creation submits
now await ensureLocalRuntimeCapabilities(), which probes the local runtime
when no answer has landed yet, so the route resolves on an actual answer.
Fixes #19154
* fix(native-chat): await the capability probe in the work-item direct launch route too
prepareDirectWorkItemAgentLaunch is the fourth creation-flow route owner and
already async; a pre-hydration submit-after-ready launch (fix-checks) read the
unprobed cache as unsupported and silently degraded to legacy. Draft-delivery
launches were unaffected (draft-prompt blocks structured before the capability
check). Same shape as the three creation-submit sites.
* fix(native-chat): keep the capability probe starting synchronously
The broken-bridge hardening wrapped the probe in Promise.resolve().then(...),
which deferred window.api.runtime.getStatus() by a microtask. The session-tabs
restore deliberately overlaps its inventory RPC with this refresh and relies on
the probe already being in flight when refresh returns, so the deferral broke it.
The bridge call is synchronous again; a synchronous throw becomes a rejection
instead, which is what the wrapper was actually for.
* fix(native-chat): hydrate local runtime capabilities at renderer boot
The capability cache's only writer was `useLocalStructuredSessionTabsSync`,
gated on workspaceSessionReady + terminalStartupRestorationReady + the
experimental flag. Every `resolveAgentLaunchRoute` reader treats an
unanswered cache as "unsupported", so the answer arriving seconds late is
what produces the bare-terminal create in #19154 — awaiting the probe at a
route decision guards four call sites but leaves the window open for the
three readers that are synchronous and cannot await.
Start the probe from the renderer boot chain, ungated, so the answer is
cached before any launch route is resolved. The per-call-site awaits stay
as the backstop for the residual window and for re-probing after a failed
probe.
Also: hoist the full-creation probe above its cancel gate so the gate stays
adjacent to createWorktree; pin the retry-after-failure, concurrent-ensure
and missing-bridge contracts; drop a stale microtask tick and correct two
comments that no longer described the code.
* test(native-chat): pin the cancel gate around the capability probe
The probe added an await to two composer creation paths. Full creation had
no gate between the route decision and createWorktree, so the earlier
revision opened a window where a dismissed composer still created a
worktree; the hoist that closed it was unpinned. Quick creation already
gated immediately before runBackgroundWorktreeCreation, so its inline
await is safe — pin that too, since nothing asserted it.
Both tests fail against origin/main (no probe) and the full-creation one
fails against the pre-hoist revision.
* fix(native-chat): close the folder-create cancel window the probe opened
The probe added the first `await` inside `submitFolderWorkspaceCreate`. On
`main` that function ran straight through to `createFolderWorkspace` with no
suspension of its own, so its caller's `isSubmissionCancelled()` gate and the
create call sat in the same turn. With the probe inline, a composer dismissed
while the probe is in flight still creates the folder workspace and launches
an agent — the same defect the full-creation hoist fixed on the git path.
Resolve capabilities in `folder-submit-orchestration` above its existing gate
and hand them down, so the create path's prefix is synchronous again. The
parameter stays optional: a caller without a cancel gate keeps the probe.
Both new tests fail against `origin/main` and against this branch's previous
head; the cancel-window one still fails with its probe-pending assertion
removed, so it pins the create, not just the probe.
* refactor(native-chat): require pre-resolved capabilities on the folder create path
The cancel-window fix in f492064432 left its invariant -- a caller that gates
on cancellation must resolve capabilities above its gate -- enforced only by a
comment, because `hostCapabilities` stayed optional with an inline probe as the
fallback. A future caller that owns a cancel gate and forgets the parameter
would silently reopen the window twice fixed already, and nothing would catch
it: the caller census test pins `resolveAgentLaunchRoute` callers, not this
function's, and `exactOptionalPropertyTypes` is off so even an explicit
`undefined` is legal.
Make it required and drop the now-unreachable inline probe. The sole
production caller already passes it, so runtime behaviour is unchanged: the
old ternary never evaluated its `await` when a value was supplied.
`null` keeps its meaning -- probed, genuinely unknown -- and still degrades to
the legacy route; only absence becomes impossible. The launch-route test that
covered the removed probe is replaced by one pinning that `null` contract with
the cache and the bridge both holding the structured capability, so only the
handed-in value can produce the legacy outcome. The cases in the sibling suite
are not about the route, so they go through one typed wrapper that supplies the
unknown answer rather than repeating it 21 times.
---------
Co-authored-by: Merge Sim <sim@local>
This commit is contained in:
co-authored by
Merge Sim
parent
ca2356c194
commit
cf20e089d2
@@ -36,6 +36,7 @@ import {
|
||||
import { mapWithConcurrency } from '../../../shared/map-with-concurrency'
|
||||
import type { OnboardingState } from '../../../shared/onboarding-state-types'
|
||||
import { restoreLocalStructuredSessionTabsOnce } from '../runtime/local-structured-session-tabs-sync'
|
||||
import { ensureLocalRuntimeCapabilities } from '../runtime/local-runtime-capabilities'
|
||||
|
||||
async function listRuntimeSessionHostIdsForStartup(): Promise<ExecutionHostId[]> {
|
||||
try {
|
||||
@@ -67,6 +68,12 @@ export function useAppStartupHydration(onOnboardingLoaded: (state: OnboardingSta
|
||||
|
||||
// Fetch initial data + hydrate GitHub cache from disk
|
||||
useEffect(() => {
|
||||
// Why first and ungated: the local capability set is a static fact the main process can answer
|
||||
// immediately, but its only other writer is the structured-session-tabs sync, which waits for
|
||||
// workspaceSessionReady + terminalStartupRestorationReady + the experimental flag. Every
|
||||
// `resolveAgentLaunchRoute` reader treats "not asked yet" as "unsupported", so leaving the
|
||||
// answer behind those gates degrades a pre-hydration create to a bare terminal (#19154).
|
||||
void ensureLocalRuntimeCapabilities()
|
||||
let cancelled = false
|
||||
// Why: declared outside the async block so cleanup can abort it — under StrictMode the first (unmounted) pass would otherwise keep spawning PTYs.
|
||||
const abortController = new AbortController()
|
||||
|
||||
@@ -367,6 +367,24 @@ describe('renderer startup runtime routing', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('probes local runtime capabilities before any startup gate can hold the answer back', () => {
|
||||
const source = readSource(STARTUP_HYDRATION_PATH)
|
||||
const probeIndex = source.indexOf('void ensureLocalRuntimeCapabilities()')
|
||||
const chainStart = source.indexOf('void (async () => {')
|
||||
const effectStart = source.lastIndexOf('useEffect(() => {', probeIndex)
|
||||
|
||||
expect(probeIndex).toBeGreaterThanOrEqual(0)
|
||||
// Why pinned here: the structured-session-tabs sync is the cache's only other writer and it
|
||||
// waits for workspaceSessionReady + terminalStartupRestorationReady + the experimental flag.
|
||||
// Every resolveAgentLaunchRoute reader — including the three that cannot await — reads an
|
||||
// unanswered cache as "unsupported", so a create in that window degrades to a bare
|
||||
// terminal (#19154). The probe must therefore start before the chain and outside its gates.
|
||||
expect(probeIndex).toBeLessThan(chainStart)
|
||||
expect(probeIndex).toBeLessThan(source.indexOf('await ', effectStart))
|
||||
expect(source.slice(effectStart, probeIndex)).not.toContain('if (')
|
||||
expect(source.slice(effectStart, probeIndex)).not.toContain('experimentalStructuredNativeChat')
|
||||
})
|
||||
|
||||
it('orders packaged restoration before adoption, projection, and default creation', () => {
|
||||
// Why this file: the startup sequence moved out of App.tsx into the hydration hook;
|
||||
// the ordering it asserts is unchanged, only the module that now spells it out.
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
ensureLocalRuntimeCapabilities,
|
||||
readLocalRuntimeCapabilities,
|
||||
readLocalRuntimeCapabilitiesOrUnknown,
|
||||
refreshLocalRuntimeCapabilities,
|
||||
@@ -82,4 +83,54 @@ describe('local runtime capabilities', () => {
|
||||
await refreshLocalRuntimeCapabilities()
|
||||
expect(readLocalRuntimeCapabilitiesOrUnknown()).toEqual(['agent-session.structured.v1'])
|
||||
})
|
||||
it('ensure probes the runtime when no answer has landed yet', async () => {
|
||||
setLocalRuntimeCapabilitiesForTests(null)
|
||||
const getStatus = vi.fn(async () => ({ capabilities: ['agent-session.structured.v1'] }))
|
||||
Object.assign(window, { api: { runtime: { getStatus } } })
|
||||
|
||||
await expect(ensureLocalRuntimeCapabilities()).resolves.toEqual(['agent-session.structured.v1'])
|
||||
expect(getStatus).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('ensure returns the cached answer without probing again', async () => {
|
||||
setLocalRuntimeCapabilitiesForTests(['agent-session.structured.v1'])
|
||||
const getStatus = vi.fn(async () => ({ capabilities: [] }))
|
||||
Object.assign(window, { api: { runtime: { getStatus } } })
|
||||
|
||||
await expect(ensureLocalRuntimeCapabilities()).resolves.toEqual(['agent-session.structured.v1'])
|
||||
expect(getStatus).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('ensure stays unknown after a failed probe and re-probes on the next call', async () => {
|
||||
setLocalRuntimeCapabilitiesForTests(null)
|
||||
const getStatus = vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(new Error('offline'))
|
||||
.mockResolvedValueOnce({ capabilities: ['agent-session.structured.v1'] })
|
||||
Object.assign(window, { api: { runtime: { getStatus } } })
|
||||
|
||||
// A failed probe is not evidence about the host, so it must not latch as a denial:
|
||||
// the answer stays unknown and the next caller pays for a fresh probe.
|
||||
await expect(ensureLocalRuntimeCapabilities()).resolves.toBeNull()
|
||||
await expect(ensureLocalRuntimeCapabilities()).resolves.toEqual(['agent-session.structured.v1'])
|
||||
expect(getStatus).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('ensure never rejects when the preload bridge is missing', async () => {
|
||||
setLocalRuntimeCapabilitiesForTests(null)
|
||||
Reflect.deleteProperty(window, 'api')
|
||||
|
||||
await expect(ensureLocalRuntimeCapabilities()).resolves.toBeNull()
|
||||
})
|
||||
|
||||
it('coalesces concurrent ensure callers onto one probe', async () => {
|
||||
setLocalRuntimeCapabilitiesForTests(null)
|
||||
const getStatus = vi.fn(async () => ({ capabilities: ['agent-session.structured.v1'] }))
|
||||
Object.assign(window, { api: { runtime: { getStatus } } })
|
||||
|
||||
await expect(
|
||||
Promise.all([ensureLocalRuntimeCapabilities(), ensureLocalRuntimeCapabilities()])
|
||||
).resolves.toEqual([['agent-session.structured.v1'], ['agent-session.structured.v1']])
|
||||
expect(getStatus).toHaveBeenCalledOnce()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -15,9 +15,37 @@ export function readLocalRuntimeCapabilitiesOrUnknown(): readonly RuntimeCapabil
|
||||
return localRuntimeCapabilities
|
||||
}
|
||||
|
||||
/** Like `readLocalRuntimeCapabilitiesOrUnknown`, but probes the local runtime when no answer
|
||||
* has landed yet, so a caller that can wait never reads "not asked yet" as "unsupported"
|
||||
* (#19154: that reads a structured-native-chat create as a bare terminal).
|
||||
*
|
||||
* The renderer boot chain calls this once, ungated, so the answer is normally already cached
|
||||
* by the time any launch route is resolved — including for the readers that are synchronous
|
||||
* and cannot await. Awaiting it at a route decision is the backstop for the residual window
|
||||
* and for re-probing after a failed one. Still `null` after an actually failed probe, and
|
||||
* never rejects. */
|
||||
export async function ensureLocalRuntimeCapabilities(): Promise<
|
||||
readonly RuntimeCapability[] | null
|
||||
> {
|
||||
if (localRuntimeCapabilities !== null) {
|
||||
return localRuntimeCapabilities
|
||||
}
|
||||
await refreshLocalRuntimeCapabilities()
|
||||
return localRuntimeCapabilities
|
||||
}
|
||||
|
||||
/** `refreshLocalRuntimeCapabilities` is not `async`, so a missing or broken preload bridge would
|
||||
* throw synchronously out of it instead of settling into the unknown state its catch owns. */
|
||||
function startLocalRuntimeCapabilityProbe(): ReturnType<typeof window.api.runtime.getStatus> {
|
||||
try {
|
||||
return window.api.runtime.getStatus()
|
||||
} catch (error) {
|
||||
return Promise.reject(error)
|
||||
}
|
||||
}
|
||||
|
||||
export function refreshLocalRuntimeCapabilities(): Promise<readonly RuntimeCapability[]> {
|
||||
refreshPromise ??= window.api.runtime
|
||||
.getStatus()
|
||||
refreshPromise ??= startLocalRuntimeCapabilityProbe()
|
||||
.then((status) => {
|
||||
localRuntimeCapabilities = [...(status.capabilities ?? [])]
|
||||
return localRuntimeCapabilities
|
||||
@@ -35,8 +63,8 @@ export function refreshLocalRuntimeCapabilities(): Promise<readonly RuntimeCapab
|
||||
}
|
||||
|
||||
export function setLocalRuntimeCapabilitiesForTests(
|
||||
capabilities: readonly RuntimeCapability[]
|
||||
capabilities: readonly RuntimeCapability[] | null
|
||||
): void {
|
||||
localRuntimeCapabilities = [...capabilities]
|
||||
localRuntimeCapabilities = capabilities === null ? null : [...capabilities]
|
||||
refreshPromise = null
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user