fix(review): normalize delivery-unknown preflight timeouts and guard Promise.all loser

- normalizePreflightError now matches the typed isRpcDeliveryUnknown marker, so relay
  timeouts (relay RPC timed out:) and socket-drop reasons surface the actionable preflight
  message instead of a raw wire string (reliability + adversarial reviewers)
- attach a no-op catch to the losing status/target preflight promise so a late rejection
  after Promise.all settles no longer becomes an unhandled promise rejection (CodeRabbit)
- add a regression test for delivery-unknown normalization

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinjing
2026-07-23 16:39:45 -07:00
co-authored by Orca
parent ab96e79054
commit da8ab164a3
2 changed files with 27 additions and 3 deletions
@@ -12,6 +12,7 @@ import {
import type { RpcClient } from '../transport/rpc-client'
import type { RpcFailure, RpcResponse } from '../transport/types'
import { LogicalClientCutoverError } from '../transport/stable-logical-rpc-client'
import { markRpcDeliveryUnknown } from '../transport/rpc-delivery-ambiguity'
import {
buildMobileFileMutationOwnership,
captureMobileFileMutationOwnership,
@@ -358,6 +359,21 @@ describe('mobile file mutation ownership', () => {
expect(sendRequest).toHaveBeenCalledTimes(3)
})
it('normalizes a delivery-unknown transport drop to the preflight timeout message', async () => {
// Why: relay timeouts / socket drops carry the delivery-unknown marker, not the direct-session
// timeout strings, so they must still surface the actionable preflight message.
const sendRequest = vi.fn(async (method: string) => {
if (method === 'status.get') {
return success({ capabilities: [FILE_MUTATION_OWNERSHIP_RUNTIME_CAPABILITY] })
}
throw markRpcDeliveryUnknown(new Error('relay RPC timed out: worktree.show'))
})
await expect(
captureMobileFileMutationOwnership({ sendRequest }, 'id:worktree-1')
).rejects.toThrow('before the request timed out')
})
it('routes folder workspaces through the folder record instead of worktree.show', async () => {
const { client, sendRequest } = clientWithResponses([
success({ capabilities: [FILE_MUTATION_OWNERSHIP_RUNTIME_CAPABILITY] }),
@@ -11,6 +11,7 @@ import { inferFolderWorkspacePathConnection } from '../../../src/shared/folder-w
import type { RuntimeStatus } from '../../../src/shared/runtime-types'
import type { SshConnectionState } from '../../../src/shared/ssh-types'
import { parseWorkspaceKey } from '../../../src/shared/workspace-scope'
import { isRpcDeliveryUnknown } from '../transport/rpc-delivery-ambiguity'
import type { RpcClient } from '../transport/rpc-client'
import {
isLogicalClientCutoverError,
@@ -123,6 +124,10 @@ async function captureMobileFileMutationOwnershipOnce(
return response
})
const targetPromise = requestMutationTargetHost(client, worktree, deadline)
// Why: Promise.all settles on the first rejection; swallow the loser's late rejection so it
// never surfaces as an unhandled promise rejection after the other branch already failed.
statusPromise.catch(() => {})
targetPromise.catch(() => {})
const [status, target] = await Promise.all([statusPromise, targetPromise])
assertCaptureTransportCurrent(client, transportGeneration)
for (const runtimeId of target.runtimeIds) {
@@ -276,10 +281,13 @@ function assertPreflightTimeRemaining(deadline: number): number {
}
function normalizePreflightError(error: unknown): Error {
// Why: relay timeouts and socket-drop reasons carry the delivery-unknown marker but not the
// direct-session timeout strings, so match the typed marker instead of substrings alone.
if (
error instanceof Error &&
(error.message.includes('Request timed out:') ||
error.message.includes('Timed out while connecting'))
isRpcDeliveryUnknown(error) ||
(error instanceof Error &&
(error.message.includes('Request timed out:') ||
error.message.includes('Timed out while connecting')))
) {
return new Error(PREFLIGHT_TIMEOUT_MESSAGE)
}