fix(mobile): bound the ambiguous worktree.create replay by the host's dedupe window

The replay was bounded only by a retry count, but what makes a replay reconcile
instead of building a second worktree is wall clock: the host drops a settled
create's dedupe record 60s after it resolves, and past that the replay is just a
fresh create that the host's suffix loop happily duplicates — for a folder
workspace, into a second workspace with the very same name and no collision
check at all.

Two paths ran past that window:

- The request-timeout path. A silently dropped response frame leaves the socket
  alive, so nothing rejects until WORKTREE_CREATE_TIMEOUT_MS — ten minutes, with
  no bound at all on when the host actually resolved. This was previously the
  path that replayed *soonest*, short-circuiting the reconnect wait because the
  transport still looked healthy. Invert it: every path that reports a real drop
  has already left 'connected' by the time the rejection surfaces, so still being
  'connected' identifies the timeout and is now refused.
- The reported-drop path. Worst-case detection is a full liveness idle period
  plus the missed-probe budget before the client even learns the socket is dead,
  and the old 20s wait on top of that overran the record. Derive the wait from
  the watchdog constants and the TTL instead of hardcoding it, and anchor a
  single deadline at the first ambiguity so a second wait gets the remainder
  rather than restarting.

The TTL now has one definition shared by both processes, so the client asserts
its budget against the host's real window instead of a copied literal.
This commit is contained in:
Brennan Benson
2026-08-19 16:53:53 -07:00
parent 0af1a5fb93
commit 1099b5ff9b
4 changed files with 219 additions and 23 deletions
+159 -14
View File
@@ -3,7 +3,18 @@ import type { RpcClient } from '../transport/rpc-client'
import { markRpcDeliveryUnknown } from '../transport/rpc-delivery-ambiguity'
import { LogicalClientCutoverError } from '../transport/stable-logical-rpc-client'
import type { ConnectionState } from '../transport/types'
import { createWorktreeWithNameRetry } from './worktree-create-retry'
import { WORKTREE_CREATE_DEDUPE_TTL_MS } from '../../../src/shared/new-workspace/worktree-create-retry-policy'
import {
LIVENESS_IDLE_MS,
LIVENESS_PROBE_TIMEOUT_MS,
MISSED_PROBE_LIMIT
} from '../transport/rpc-session-liveness-watchdog'
import {
createWorktreeWithNameRetry,
WORKTREE_CREATE_AMBIGUOUS_RECONNECT_WAIT_MS,
WORKTREE_CREATE_AMBIGUOUS_REPLAY_WINDOW_MS
} from './worktree-create-retry'
import { WORKTREE_CREATE_TIMEOUT_MS } from './workspace-create-timeout'
type Attempt = { method: string; params: Record<string, unknown> }
@@ -41,7 +52,12 @@ async function flush(): Promise<void> {
// cutover). Records every call so tests can assert on the clientMutationId.
function scriptedClient(
outcomes: Array<
{ id: string } | { errorMessage: string } | { throws: unknown; dropsConnection?: boolean }
| { id: string }
| { errorMessage: string }
// takesMs models how long the ambiguity took to SURFACE — a clean close is
// instant, a half-open socket waits out the liveness watchdog or the timeout.
// reconnectsAfterMs brings the transport back so the next replay can run.
| { throws: unknown; dropsConnection?: boolean; takesMs?: number; reconnectsAfterMs?: number }
>,
attempts: Attempt[],
connection?: ReturnType<typeof connectionController>
@@ -56,8 +72,14 @@ function scriptedClient(
const outcome = outcomes[Math.min(call, outcomes.length - 1)]!
call += 1
if ('throws' in outcome) {
if (outcome.takesMs !== undefined) {
await new Promise((resolve) => setTimeout(resolve, outcome.takesMs))
}
if (outcome.dropsConnection) {
connection?.set('reconnecting')
if (outcome.reconnectsAfterMs !== undefined) {
setTimeout(() => connection?.set('connected'), outcome.reconnectsAfterMs)
}
}
throw outcome.throws
}
@@ -157,6 +179,8 @@ describe('createWorktreeWithNameRetry', () => {
expect(attempts).toHaveLength(6)
})
// Also the delivery-ambiguity guard: an unmarked error is a DEFINITE failure, so
// neither the cutover nor the replay path may re-issue it.
it('does not treat an ordinary transport error as a cutover', async () => {
const attempts: Attempt[] = []
const client = scriptedClient([{ throws: new Error('Request timed out') }], attempts)
@@ -284,24 +308,46 @@ describe('createWorktreeWithNameRetry', () => {
expect(attempts[0]!.params.clientMutationId).toBeUndefined()
})
it('gives up after the delivery-ambiguity replay budget and rethrows', async () => {
const attempts: Attempt[] = []
const client = scriptedClient(
[{ throws: markRpcDeliveryUnknown(new Error('Connection interrupted')) }],
attempts
)
await expect(
createWorktreeWithNameRetry({
it('gives up after the delivery-ambiguity replay budget, still inside the dedupe TTL', async () => {
vi.useFakeTimers()
try {
const attempts: Attempt[] = []
const connection = connectionController()
const client = scriptedClient(
[
{
throws: markRpcDeliveryUnknown(new Error('Connection interrupted')),
dropsConnection: true,
reconnectsAfterMs: 1_000
}
],
attempts,
connection
)
const startedAt = Date.now()
let settledAt = -1
const pending = createWorktreeWithNameRetry({
client,
baseName: 'barnacle',
buildParams: (name) => ({ repo: 'id:r', name }),
supportsIdempotentCutoverRetry: true,
mintMutationId: () => 'key-budget'
}).catch((error: unknown) => {
settledAt = Date.now()
throw error
})
).rejects.toThrow('Connection interrupted')
// Initial attempt + 2 replays; the window stays inside the host's post-success
// dedupe TTL so a replay can't outlive the record it is meant to match.
expect(attempts).toHaveLength(3)
const settled = expect(pending).rejects.toThrow('Connection interrupted')
await vi.advanceTimersByTimeAsync(WORKTREE_CREATE_DEDUPE_TTL_MS)
await settled
// Initial attempt + 2 replays.
expect(attempts).toHaveLength(3)
// The budget is a count, but what keeps a replay reconciling instead of building
// a second worktree is wall clock: every attempt has to land while the host still
// holds the record, with the watchdog's detection latency already deducted.
expect(settledAt - startedAt).toBeLessThanOrEqual(WORKTREE_CREATE_AMBIGUOUS_REPLAY_WINDOW_MS)
} finally {
vi.useRealTimers()
}
})
it('surfaces the original ambiguity when the transport never comes back', async () => {
@@ -336,4 +382,103 @@ describe('createWorktreeWithNameRetry', () => {
vi.useRealTimers()
}
})
it('does not replay an ambiguity that surfaced while the transport stayed connected', async () => {
vi.useFakeTimers()
try {
const attempts: Attempt[] = []
const connection = connectionController()
const client = scriptedClient(
[
{
// A silently dropped response frame leaves the socket alive, so nothing
// rejects until the request timeout — by then the host resolved at an
// unknowable point and its dedupe record is very likely gone.
throws: markRpcDeliveryUnknown(new Error('Request timed out: worktree.create')),
takesMs: WORKTREE_CREATE_TIMEOUT_MS
},
{ id: 'wt-duplicate' }
],
attempts,
connection
)
const pending = createWorktreeWithNameRetry({
client,
baseName: 'cuttlefish',
buildParams: (name) => ({ repo: 'id:r', name }),
supportsIdempotentCutoverRetry: true,
mintMutationId: () => 'key-expired'
})
const settled = expect(pending).rejects.toThrow('Request timed out')
await vi.advanceTimersByTimeAsync(WORKTREE_CREATE_TIMEOUT_MS)
await settled
// Never left 'connected', so no drop was ever detected and the staleness of the
// ambiguity is unbounded. Re-sending would miss the record and build a SECOND
// worktree — for a folder workspace, one with the very same name.
expect(connection.getState()).toBe('connected')
expect(attempts).toHaveLength(1)
} finally {
vi.useRealTimers()
}
})
it('clamps a later reconnect wait to what is left of the replay window', async () => {
vi.useFakeTimers()
try {
const attempts: Attempt[] = []
const connection = connectionController()
const spent = 5_000
const client = scriptedClient(
[
{
throws: markRpcDeliveryUnknown(new Error('Connection interrupted')),
dropsConnection: true,
reconnectsAfterMs: spent
},
{
// Second drop, and this time the phone never comes back.
throws: markRpcDeliveryUnknown(new Error('Connection interrupted')),
dropsConnection: true
}
],
attempts,
connection
)
const startedAt = Date.now()
let settledAt = -1
const pending = createWorktreeWithNameRetry({
client,
baseName: 'nautilus',
buildParams: (name) => ({ repo: 'id:r', name }),
supportsIdempotentCutoverRetry: true,
mintMutationId: () => 'key-clamped'
}).catch((error: unknown) => {
settledAt = Date.now()
throw error
})
const settled = expect(pending).rejects.toThrow('Connection interrupted')
await vi.advanceTimersByTimeAsync(WORKTREE_CREATE_AMBIGUOUS_REPLAY_WINDOW_MS)
await settled
expect(attempts).toHaveLength(2)
// The window is anchored at the FIRST ambiguity, so the second wait gets only the
// remainder — it must not restart a full wait and carry the replay past the record.
expect(settledAt - startedAt).toBe(WORKTREE_CREATE_AMBIGUOUS_REPLAY_WINDOW_MS)
} finally {
vi.useRealTimers()
}
})
it('leaves room for watchdog detection inside the host dedupe TTL', () => {
// Restates the budget from the watchdog's side: a half-open socket is only reported
// after a full idle period plus the missed-probe budget, and the wait runs *after*
// that. Raising the wait without re-deriving it against the TTL would let a replay
// land on a dropped record.
const worstCaseDetectionMs = LIVENESS_IDLE_MS + MISSED_PROBE_LIMIT * LIVENESS_PROBE_TIMEOUT_MS
expect(worstCaseDetectionMs + WORKTREE_CREATE_AMBIGUOUS_RECONNECT_WAIT_MS).toBeLessThanOrEqual(
WORKTREE_CREATE_DEDUPE_TTL_MS
)
expect(WORKTREE_CREATE_AMBIGUOUS_REPLAY_WINDOW_MS).toBeGreaterThan(0)
})
})
+49 -7
View File
@@ -7,8 +7,14 @@ import {
CLIENT_WORKTREE_CREATE_MAX_ATTEMPTS,
getClientWorktreeCreateCandidate,
getGeneratedWorktreeCreateRetryCandidate,
isRetryableWorktreeCreateConflict
isRetryableWorktreeCreateConflict,
WORKTREE_CREATE_DEDUPE_TTL_MS
} from '../../../src/shared/new-workspace/worktree-create-retry-policy'
import {
LIVENESS_IDLE_MS,
LIVENESS_PROBE_TIMEOUT_MS,
MISSED_PROBE_LIMIT
} from '../transport/rpc-session-liveness-watchdog'
import { WORKTREE_CREATE_TIMEOUT_MS } from './workspace-create-timeout'
// Why: server-side collision checks (branch already exists locally / on a remote
@@ -34,10 +40,23 @@ const WORKTREE_CREATE_CUTOVER_MAX_RETRIES = 5
// to be caught by one. Surfacing that as a failure is wrong: the host may well have
// finished the worktree. Replay on the same clientMutationId instead.
const WORKTREE_CREATE_AMBIGUOUS_MAX_RETRIES = 2
// Why: the host drops a completed create's dedupe record 60s after it resolves, so
// keep the whole replay window comfortably inside that or a replay lands as a fresh
// create and suffixes a duplicate. An in-flight create is deduped without any TTL.
const WORKTREE_CREATE_AMBIGUOUS_RECONNECT_WAIT_MS = 20_000
// Why: the host keeps a settled create's dedupe record for WORKTREE_CREATE_DEDUPE_TTL_MS
// after it resolves; a replay that lands later misses it and the host's suffix loop
// builds a SECOND worktree — and for folder workspaces a second one with the SAME name
// and no collision check at all. So what bounds a safe replay is not how long the create
// ran, but how STALE the client's knowledge is: the worst case between the host losing a
// response and the client noticing. A dead socket is detected by the liveness watchdog
// within one idle period plus its missed-probe budget, so that is the staleness ceiling
// for any drop the transport actually reports.
const WORKTREE_CREATE_AMBIGUITY_DETECTION_CEILING_MS =
LIVENESS_IDLE_MS + MISSED_PROBE_LIMIT * LIVENESS_PROBE_TIMEOUT_MS
// What is left of the host's record once that detection latency is spent. The reconnect
// wait plus any further replay has to fit inside it.
export const WORKTREE_CREATE_AMBIGUOUS_REPLAY_WINDOW_MS =
WORKTREE_CREATE_DEDUPE_TTL_MS - WORKTREE_CREATE_AMBIGUITY_DETECTION_CEILING_MS
export const WORKTREE_CREATE_AMBIGUOUS_RECONNECT_WAIT_MS =
WORKTREE_CREATE_AMBIGUOUS_REPLAY_WINDOW_MS
export type CreateWorktreeWithNameRetryArgs = {
client: RpcClient
@@ -105,6 +124,9 @@ async function sendWorktreeCreateResilient(
): Promise<RpcResponse> {
let migrationRetry = 0
let ambiguousRetry = 0
// Anchored at the FIRST ambiguity, not at the send: a create legitimately runs for
// minutes, and the host's record only starts ticking once it resolves.
let replayDeadlineAt: number | null = null
for (;;) {
try {
return await client.sendRequest('worktree.create', params, {
@@ -126,12 +148,32 @@ async function sendWorktreeCreateResilient(
if (!isRpcDeliveryUnknown(error) || ambiguousRetry >= WORKTREE_CREATE_AMBIGUOUS_MAX_RETRIES) {
throw error
}
// Why: every transport path that reports a *drop* leaves 'connected' before the
// rejection reaches us (rpc-client.ts:675/695/1213 set state first or reject via
// queueMicrotask; the relay's fail() publishes synchronously). So still being
// 'connected' here means the socket was healthy the whole time and only the
// response went missing — the request-timeout path, which surfaces after
// WORKTREE_CREATE_TIMEOUT_MS. That says nothing about when the host actually
// resolved, so the dedupe record may be long gone and a replay would build a
// second worktree instead of reconciling. Fail the create instead.
if (client.getState() === 'connected') {
throw error
}
replayDeadlineAt ??= Date.now() + WORKTREE_CREATE_AMBIGUOUS_REPLAY_WINDOW_MS
const remainingWindowMs = replayDeadlineAt - Date.now()
if (remainingWindowMs <= 0) {
throw error
}
ambiguousRetry += 1
// Why: unlike a cutover, no replacement session exists yet — resending now
// would just hit the dead one, so wait for the transport to come back and
// surface the original ambiguity if it does not.
// surface the original ambiguity if it does not. Clamped to the window so the
// wait itself cannot carry the replay past the host's record.
if (
!(await waitForRpcClientReconnected(client, WORKTREE_CREATE_AMBIGUOUS_RECONNECT_WAIT_MS))
!(await waitForRpcClientReconnected(
client,
Math.min(WORKTREE_CREATE_AMBIGUOUS_RECONNECT_WAIT_MS, remainingWindowMs)
))
) {
throw error
}
+4 -2
View File
@@ -1077,6 +1077,7 @@ import {
isGeneratedWorktreeCreateName,
WORKTREE_CREATE_MAX_SUFFIX_ATTEMPTS
} from '../worktree-create-candidates'
import { WORKTREE_CREATE_DEDUPE_TTL_MS } from '../../shared/new-workspace/worktree-create-retry-policy'
import {
failedWorktreeCreationNeedsRetirement,
getRetiredNameRegistryForRepo,
@@ -2015,8 +2016,9 @@ function getAgentLaunchPlatformForRepo(
const MOBILE_TERMINAL_CREATE_RESULT_TTL_MS = 60_000
// Why: same idempotency window for worktree.create — a phone whose create was
// interrupted by a connection migration retries with the same clientMutationId
// and reuses the just-created worktree instead of spawning a duplicate.
const WORKTREE_CREATE_RESULT_TTL_MS = 60_000
// and reuses the just-created worktree instead of spawning a duplicate. Shared with
// the mobile client so its replay budget is sized against the real window.
const WORKTREE_CREATE_RESULT_TTL_MS = WORKTREE_CREATE_DEDUPE_TTL_MS
const FOREGROUND_AGENT_WRAPPER_RETRY_INTERVAL_MS = 150
const FOREGROUND_AGENT_WRAPPER_RETRY_TIMEOUT_MS = 6_500
const BRACKETED_PASTE_BEGIN = '\x1b[200~'
@@ -65,3 +65,10 @@ export function getGeneratedWorktreeCreateRetryCandidate(
export function isRetryableWorktreeCreateConflict(message: string): boolean {
return RETRYABLE_WORKTREE_CREATE_CONFLICT_PATTERNS.some((pattern) => pattern.test(message))
}
// Why: the host drops a *settled* worktree.create dedupe record this long after it
// resolves, so a client replaying an idempotent create must land inside the window
// or the replay misses the record and the host's suffix loop builds a second
// worktree. Shared so the client's replay budget can be asserted against it instead
// of assuming a value that lives in another process.
export const WORKTREE_CREATE_DEDUPE_TTL_MS = 60_000