From a44d291cb5c75d5ec006d0453d05ee1cec5319c7 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Tue, 4 Aug 2026 00:42:34 +0300 Subject: [PATCH 1/4] fix(sync): settle a guessed session directory once its owner is known MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Selecting a session whose directory this client has not indexed yet routes it through the active directory. That is a deliberate, documented guess: it keeps routing usable while the owning store bootstraps, and it is excluded from both the resolver and persistence. Nothing settled the guess afterwards. `setSessionDirectory` performs exactly that promotion, but only confirmed destinations call it — a completed move or a worktree this client created. A session whose directory the client learned about later, such as one in a worktree created outside this client, kept the guess forever: every message fetch was addressed to the parent repository, which does not own the session. Captured for such a session before this change, with the session already indexed and its owning store known: routedDirectory .../worktree/feature currentSessionDirectory /repo <- guess, never settled opencodeClientDirectory /repo conflict selected -> /repo and after: routedDirectory .../worktree/feature currentSessionDirectory .../worktree/feature opencodeClientDirectory .../worktree/feature conflict null Directory bootstrap completion is the moment the authoritative directory first becomes readable, so the promotion runs there. It only ever promotes a guess: a confirmed selection and a selection that has since moved on are both left alone, and tests cover both directions. This removes a real routing split-brain. It does not by itself fix the reported symptom of a session created mid-session never rendering; that remains open. --- .../sync/session-directory-adoption.test.ts | 82 +++++++++++++++++++ packages/ui/src/sync/session-ui-store.ts | 26 ++++++ packages/ui/src/sync/sync-context.tsx | 11 +++ 3 files changed, 119 insertions(+) create mode 100644 packages/ui/src/sync/session-directory-adoption.test.ts diff --git a/packages/ui/src/sync/session-directory-adoption.test.ts b/packages/ui/src/sync/session-directory-adoption.test.ts new file mode 100644 index 00000000..81821dc0 --- /dev/null +++ b/packages/ui/src/sync/session-directory-adoption.test.ts @@ -0,0 +1,82 @@ +import { beforeEach, describe, expect, test } from "bun:test" + +import { ChildStoreManager } from "./child-store" +import { setSyncRefs } from "./sync-refs" +import { useSessionUIStore } from "./session-ui-store" + +/** + * Selecting a session whose directory this client has not indexed yet routes it + * through the active directory as a deliberate guess. Nothing used to settle + * that guess once the owning directory finished bootstrapping, so every fetch + * stayed addressed to a directory that does not own the session and the session + * never rendered. + * + * These tests pin both directions: a guess is promoted once the authoritative + * directory becomes readable, and a confirmed selection is never rewritten. + */ + +const PARENT = "/repo" +const WORKTREE = "/repo/.worktrees/feature" +const SESSION_ID = "ses_directory_adoption" + +const indexSessionIn = (manager: ChildStoreManager, directory: string): void => { + const store = manager.ensureChild(directory, { bootstrap: false }) + store.setState({ + session: [{ id: SESSION_ID, directory, title: "test" } as never], + }) +} + +let manager: ChildStoreManager + +beforeEach(() => { + manager = new ChildStoreManager() + setSyncRefs({} as never, manager, PARENT) + useSessionUIStore.getState().setCurrentSession(null) +}) + +describe("adoptAuthoritativeSessionDirectory", () => { + test("promotes a guessed selection once the owning directory is indexed", () => { + useSessionUIStore.getState().setCurrentSession(SESSION_ID) + expect(useSessionUIStore.getState().currentSessionDirectory).not.toBe(WORKTREE) + + indexSessionIn(manager, WORKTREE) + useSessionUIStore.getState().adoptAuthoritativeSessionDirectory() + + expect(useSessionUIStore.getState().currentSessionDirectory).toBe(WORKTREE) + }) + + test("does nothing while the owning directory is still unknown", () => { + useSessionUIStore.getState().setCurrentSession(SESSION_ID) + const before = useSessionUIStore.getState().currentSessionDirectory + + useSessionUIStore.getState().adoptAuthoritativeSessionDirectory() + + expect(useSessionUIStore.getState().currentSessionDirectory).toBe(before) + }) + + test("never rewrites a selection that was confirmed at selection time", () => { + useSessionUIStore.getState().setCurrentSession(SESSION_ID, WORKTREE) + expect(useSessionUIStore.getState().currentSessionDirectory).toBe(WORKTREE) + + // A different directory claiming the session must not move a confirmed + // selection: the confirmed value outranks anything sync learns later. + indexSessionIn(manager, PARENT) + useSessionUIStore.getState().adoptAuthoritativeSessionDirectory() + + expect(useSessionUIStore.getState().currentSessionDirectory).toBe(WORKTREE) + }) + + test("is a no-op for a session that is no longer selected", () => { + useSessionUIStore.getState().setCurrentSession(SESSION_ID) + indexSessionIn(manager, WORKTREE) + useSessionUIStore.getState().setCurrentSession("ses_other") + + // Whatever the new selection resolved to, a late adoption for the previous + // session must not touch it. + const before = useSessionUIStore.getState().currentSessionDirectory + useSessionUIStore.getState().adoptAuthoritativeSessionDirectory(SESSION_ID) + + expect(useSessionUIStore.getState().currentSessionId).toBe("ses_other") + expect(useSessionUIStore.getState().currentSessionDirectory).toBe(before) + }) +}) diff --git a/packages/ui/src/sync/session-ui-store.ts b/packages/ui/src/sync/session-ui-store.ts index 4f65e98b..87721194 100644 --- a/packages/ui/src/sync/session-ui-store.ts +++ b/packages/ui/src/sync/session-ui-store.ts @@ -352,6 +352,12 @@ export type SessionUIState = { debugSessionMessages: (sessionId: string) => Promise pollForTokenUpdates: () => void setSessionDirectory: (sessionId: string, directory: string | null) => void + /** + * Replace a guessed selection directory with the authoritative one once sync + * has indexed the session. Safe to call at any time: it only ever promotes a + * guess, never overrides a confirmed selection. + */ + adoptAuthoritativeSessionDirectory: (sessionId?: string) => void } // --------------------------------------------------------------------------- @@ -1739,6 +1745,26 @@ export const useSessionUIStore = create()((set, get) => ({ // Handled by sync system's SSE stream }, + adoptAuthoritativeSessionDirectory: (sessionId) => { + const target = sessionId ?? get().currentSessionId + // Only a guess is promoted. A confirmed selection outranks anything sync + // learns later, and a selection that has since moved on must not be + // rewritten by a directory that finished bootstrapping in the background. + if (!target || target !== guessedSelectionSessionId) return + if (target !== get().currentSessionId) return + + const authoritative = getAuthoritativeSessionDirectory(target) + if (!authoritative) return + + // The selection stops being a guess even when the directory is unchanged: + // the value has now been confirmed by the store that owns the session. + guessedSelectionSessionId = null + if (authoritative !== get().currentSessionDirectory) { + set({ currentSessionDirectory: authoritative }) + } + writeRuntimeSessionMemory(runtimeMemoryKey(), { sessionId: target, directory: authoritative }) + }, + setSessionDirectory: (sessionId, directory) => { const normalized = normalizePath(directory) // Callers set this from a confirmed destination (a completed move, a diff --git a/packages/ui/src/sync/sync-context.tsx b/packages/ui/src/sync/sync-context.tsx index 9e1da2b4..b4ff6c19 100644 --- a/packages/ui/src/sync/sync-context.tsx +++ b/packages/ui/src/sync/sync-context.tsx @@ -34,6 +34,7 @@ import { countSyncPerformance } from "./performance-diagnostics" import { runBackgroundNetworkTask } from "@/lib/background-network" import { setActionRefs } from "./session-actions" import { setSyncRefs, getAllSyncSessions } from "./sync-refs" +import { useSessionUIStore } from "./session-ui-store" import { stripSessionDiffSnapshots } from "./sanitize" import { applySessionEventToGlobalSessions } from "./session-event-router" import { syncDebug } from "./debug" @@ -1949,6 +1950,16 @@ export function SyncProvider(props: { const result = await runBootstrap(0) if (result === "failed") throw new Error(`Directory bootstrap failed for ${directory}`) + + // Selecting a session whose directory this client had not indexed yet + // routes it through the active directory as a documented guess. This is + // the moment that guess can be settled: the owning store now holds the + // session, so the authoritative directory is finally readable. Without + // this the guess survives, every fetch is addressed to a directory that + // does not own the session, and the session never renders. + if (result === "complete") { + useSessionUIStore.getState().adoptAuthoritativeSessionDirectory() + } }, onDispose: (directory) => { messageLoader.invalidateDirectory(directory) From bf3186c679e95def8412f1623b2d3940dadaf806 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Tue, 4 Aug 2026 01:27:03 +0300 Subject: [PATCH 2/4] fix(sync): read session ownership from the record, not store membership A session created in a git worktree while the client was already running did not render: the message list stayed empty while the prompt and the assistant reply were both present in the session, visible on any fresh load. Reported as prompting in a worktree sometimes not working. Ownership was read from which child store holds the session. That is containment, not ownership. A project's session list includes the sessions of its worktrees so the sidebar can group them, so the parent repository holds worktree sessions too, and whichever store bootstrapped first won. Captured mid-failure, the two signals disagreed outright: owningDirectory /repo <- parent, merely holds it recordDirectory /repo/.worktrees/feature <- the session's own directory The parent won, so every fetch was addressed to a directory that does not own the session, the session id resolved to undefined there, and the requests failed as /api/session/undefined in a retry loop. The session's own record is now believed; store membership remains the fallback for a record that carries no directory. This also explains why the previous commit alone was not enough: settling the guessed directory adopted this same wrong value and then cleared the guess, which prevented any later correction. Verified against the reproduction rather than by reasoning. Before: three of four runs never rendered. After, on a clean build with the instrumentation removed: three of three rendered the reply live, each routed to its own worktree. Tests cover ownership disagreeing with containment, plus both directions of the guess promotion. --- .../sync/session-directory-adoption.test.ts | 20 +++++++++++++++++-- packages/ui/src/sync/session-ui-store.ts | 19 +++++++++++++++--- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/packages/ui/src/sync/session-directory-adoption.test.ts b/packages/ui/src/sync/session-directory-adoption.test.ts index 81821dc0..4857872c 100644 --- a/packages/ui/src/sync/session-directory-adoption.test.ts +++ b/packages/ui/src/sync/session-directory-adoption.test.ts @@ -19,10 +19,14 @@ const PARENT = "/repo" const WORKTREE = "/repo/.worktrees/feature" const SESSION_ID = "ses_directory_adoption" -const indexSessionIn = (manager: ChildStoreManager, directory: string): void => { +const indexSessionIn = ( + manager: ChildStoreManager, + directory: string, + recordDirectory: string = directory, +): void => { const store = manager.ensureChild(directory, { bootstrap: false }) store.setState({ - session: [{ id: SESSION_ID, directory, title: "test" } as never], + session: [{ id: SESSION_ID, directory: recordDirectory, title: "test" } as never], }) } @@ -45,6 +49,18 @@ describe("adoptAuthoritativeSessionDirectory", () => { expect(useSessionUIStore.getState().currentSessionDirectory).toBe(WORKTREE) }) + test("believes the session record over the store that merely holds it", () => { + // A project's session list includes the sessions of its worktrees so the + // sidebar can group them, so the parent store holds this session while the + // session itself reports the worktree. Ownership comes from the record. + useSessionUIStore.getState().setCurrentSession(SESSION_ID) + indexSessionIn(manager, PARENT, WORKTREE) + + useSessionUIStore.getState().adoptAuthoritativeSessionDirectory() + + expect(useSessionUIStore.getState().currentSessionDirectory).toBe(WORKTREE) + }) + test("does nothing while the owning directory is still unknown", () => { useSessionUIStore.getState().setCurrentSession(SESSION_ID) const before = useSessionUIStore.getState().currentSessionDirectory diff --git a/packages/ui/src/sync/session-ui-store.ts b/packages/ui/src/sync/session-ui-store.ts index 87721194..ef450f54 100644 --- a/packages/ui/src/sync/session-ui-store.ts +++ b/packages/ui/src/sync/session-ui-store.ts @@ -411,11 +411,24 @@ const getAttachmentForSession = (sessionId: string | null | undefined): SessionW * only then the session record's own fields. `null` means "not indexed yet", * never "no directory" — callers must fall back rather than treat it as empty. */ +/** + * The directory that owns a session, from the two server-backed signals. + * + * The session's own record wins. Holding a session in a child store proves + * containment, not ownership: a project's session list legitimately includes + * the sessions of its worktrees so the sidebar can group them, so the parent + * repository holds worktree sessions too. Reading ownership from store + * membership therefore reports the parent for a session that lives in a + * worktree, and every fetch is then addressed to a directory that does not own + * it. Store membership remains the fallback for a session whose record carries + * no directory. + */ const getAuthoritativeSessionDirectory = (sessionId: string): string | null => { - const owningDirectory = getSyncSessionDirectory(sessionId) - if (owningDirectory) return normalizePath(owningDirectory) const target = getAllSyncSessions().find((s) => s.id === sessionId) - return target ? resolveDirectoryKey(target) : null + const recordDirectory = target ? resolveDirectoryKey(target) : null + if (recordDirectory) return normalizePath(recordDirectory) + const owningDirectory = getSyncSessionDirectory(sessionId) + return owningDirectory ? normalizePath(owningDirectory) : null } /** From b6c58df9492b277bee7b12308612956a8f308c48 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Tue, 4 Aug 2026 01:33:56 +0300 Subject: [PATCH 3/4] fix(cli): give worktree provisioning a timeout that fits the work Creating a session with a worktree reported "Request to /api/openchamber/control timed out after 4000ms" while the worktree was in fact created, leaving the user with a failure message, a real worktree, and no session id. Reported alongside worktree creation appearing to take forever. The client HTTP timeout was extended only when the caller asked to wait for the session. Provisioning a worktree is slow on its own: it runs git against the repository and prepares a new directory. Measured on a cold path immediately after a restart it takes about four seconds, which lands exactly on the four second default and explains why this failed intermittently rather than always. A warm run finishes in well under two. The timeout now follows the work being requested rather than only the wait flag, and covers whichever of the two windows is longer. The server always completed the operation, so nothing about the outcome changes: only the client stops abandoning it. Verified by creating a worktree on the cold path immediately after a restart, which previously failed here: 4004 ms and 1376 ms, both reported ok. --- packages/web/bin/lib/cli-control.js | 17 +++++++++++++++-- packages/web/bin/lib/cli-control.test.js | 16 ++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/packages/web/bin/lib/cli-control.js b/packages/web/bin/lib/cli-control.js index 6cbbc864..2a6df6eb 100644 --- a/packages/web/bin/lib/cli-control.js +++ b/packages/web/bin/lib/cli-control.js @@ -10,14 +10,27 @@ const asNonEmptyString = (value) => { const DEFAULT_WAIT_TIMEOUT_SECONDS = 600; const WAIT_HTTP_TIMEOUT_BUFFER_MS = 30_000; +// Provisioning a worktree is not one of the instant control calls the short +// default is sized for: it runs git against the repository and prepares a new +// directory, which on a cold path takes longer than the default allows. The +// server finishes the work regardless of the client giving up, so a client-side +// timeout here reported a failure for a worktree that was in fact created. +const WORKTREE_PROVISION_TIMEOUT_MS = 120_000; + // The control service blocks server-side while wait is set, so the client // HTTP timeout must outlive the requested wait window instead of the short // default used for instant control calls. export const resolveControlTimeoutMs = (input, options) => { if (Number.isFinite(options?.timeoutMs) && options.timeoutMs > 0) return options.timeoutMs; - if (input?.wait !== true) return undefined; + const provisionsWorktree = asNonEmptyString(input?.worktree) !== null; + if (input?.wait !== true) { + return provisionsWorktree ? WORKTREE_PROVISION_TIMEOUT_MS : undefined; + } const waitSeconds = Number(input?.timeout) > 0 ? Number(input.timeout) : DEFAULT_WAIT_TIMEOUT_SECONDS; - return (waitSeconds * 1000) + WAIT_HTTP_TIMEOUT_BUFFER_MS; + const waitTimeoutMs = (waitSeconds * 1000) + WAIT_HTTP_TIMEOUT_BUFFER_MS; + // Waiting for the session and provisioning its worktree are additive, so the + // window must cover whichever is longer rather than only the wait. + return provisionsWorktree ? Math.max(waitTimeoutMs, WORKTREE_PROVISION_TIMEOUT_MS) : waitTimeoutMs; }; export const requestControlAction = async (port, action, input, options = {}) => { diff --git a/packages/web/bin/lib/cli-control.test.js b/packages/web/bin/lib/cli-control.test.js index 0e259e58..1ae93015 100644 --- a/packages/web/bin/lib/cli-control.test.js +++ b/packages/web/bin/lib/cli-control.test.js @@ -19,4 +19,20 @@ describe('resolveControlTimeoutMs', () => { it('never shrinks an explicitly requested HTTP timeout', () => { expect(resolveControlTimeoutMs({ wait: true, timeout: 30 }, { timeoutMs: 5000 })).toBe(5000); }); + + it('allows a worktree to be provisioned without waiting for the session', () => { + expect(resolveControlTimeoutMs({ worktree: 'feature' }, {})).toBe(120_000); + }); + + it('ignores a blank worktree name', () => { + expect(resolveControlTimeoutMs({ worktree: ' ' }, {})).toBeUndefined(); + }); + + it('covers worktree provisioning even when the wait window is shorter', () => { + expect(resolveControlTimeoutMs({ wait: true, timeout: 30, worktree: 'feature' }, {})).toBe(120_000); + }); + + it('keeps a longer wait window when it outlasts worktree provisioning', () => { + expect(resolveControlTimeoutMs({ wait: true, worktree: 'feature' }, {})).toBe(630_000); + }); }); From 3aeca4893e380dc5c3d05db0d2307511c494c5c8 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Tue, 4 Aug 2026 01:50:14 +0300 Subject: [PATCH 4/4] docs(sync): correct the ownership precedence the fix inverted Review found the owning documentation still describing the behaviour this branch replaced, in one case stacked directly above the new docstring saying the opposite. Holding a session proves containment, not ownership, so every text that called store membership the authoritative mapping was actively misleading for the module whose wrong answer misroutes every send. Corrected in the module docstring, the resolution module's precedence description, the sync-refs helper it points at, and the sync DOCUMENTATION.md table and rules. The debug report built its authoritative value membership-first, so for exactly the scenario this branch fixes it reported the parent directory and could raise a source-disagreement alert while routing was in fact correct. It now uses the same record-first order as the resolver. The CLI timeout comment claimed the wait and provisioning windows were additive while the code took the larger of the two. The server provisions the worktree inside session creation, before it waits for the session to go idle, so they do run in sequence: the windows are now summed and the tests pin both cases. --- packages/ui/src/lib/debug.ts | 6 +++++- packages/ui/src/sync/DOCUMENTATION.md | 4 ++-- .../ui/src/sync/session-directory-resolution.ts | 8 +++++--- packages/ui/src/sync/session-ui-store.ts | 8 +++----- packages/ui/src/sync/sync-refs.ts | 14 ++++++++------ packages/web/bin/lib/cli-control.js | 7 ++++--- packages/web/bin/lib/cli-control.test.js | 11 +++++------ 7 files changed, 32 insertions(+), 26 deletions(-) diff --git a/packages/ui/src/lib/debug.ts b/packages/ui/src/lib/debug.ts index cece345d..a455d61c 100644 --- a/packages/ui/src/lib/debug.ts +++ b/packages/ui/src/lib/debug.ts @@ -441,7 +441,11 @@ export const debugUtils = { const sources = { attachment, worktreeMetadata, - authoritative: owningStoreDirectory ?? recordDirectory, + // Record first, matching the resolver: holding a session proves + // containment, not ownership, so the parent repository holds its + // worktrees' sessions too. Reporting membership first made this + // diagnostic contradict the routing it exists to explain. + authoritative: recordDirectory ?? owningStoreDirectory, selected, remembered: remembered.runtime, }; diff --git a/packages/ui/src/sync/DOCUMENTATION.md b/packages/ui/src/sync/DOCUMENTATION.md index 96761339..04dd3088 100644 --- a/packages/ui/src/sync/DOCUMENTATION.md +++ b/packages/ui/src/sync/DOCUMENTATION.md @@ -207,7 +207,7 @@ The discriminator is whether the server confirmed the path, not whether the valu | Source | Meaning | |---|---| -| `authoritative` | The child store that actually holds the session, then its own record | +| `authoritative` | The session record's own directory, then a child store that holds it | | `selected` | Server-confirmed directory captured at selection; a guessed one is never passed | | `attachment` | Worktree attachment recorded by this client; the *requested* path | | `worktree-metadata` | Worktree captured when the session was created in one; the *requested* path | @@ -215,7 +215,7 @@ The discriminator is whether the server confirmed the path, not whether the valu Rules: -1. `getSyncSessionDirectory()` is the authoritative session→directory mapping: a session lives in exactly the child store for its directory, whether or not the server populated `session.directory`. `null` means "not indexed yet", never "no directory". +1. Ownership comes from the session record's own `directory`. `getSyncSessionDirectory()` reports *containment*, not ownership, and is only the fallback for a record without a directory: a project's session list includes the sessions of its worktrees so the sidebar can group them, so the parent repository holds worktree sessions too, and reading ownership from membership routes a worktree session to its parent. `null` means "not indexed yet", never "no directory". 2. `attachment` and `worktreeMetadata` hold the worktree path this client asked for, before the server canonicalized it. They are a hint for a session sync has not indexed yet, never a correction of a confirmed directory — otherwise a stale local path re-creates the very mismatch this precedence exists to prevent. 3. Never persist or rank a guessed directory. `selectSession` may fall back to the active directory to keep routing usable, but that value is not written to runtime memory, not written to the last-active snapshot, and not passed as `selected` — a persisted guess outlives the race that produced it and survives reloads and restarts. 4. Components must not read `currentSessionDirectory` to build request or queue keys; use `getDirectoryForSession()` so every consumer resolves identically. diff --git a/packages/ui/src/sync/session-directory-resolution.ts b/packages/ui/src/sync/session-directory-resolution.ts index d8069176..616c4aa7 100644 --- a/packages/ui/src/sync/session-directory-resolution.ts +++ b/packages/ui/src/sync/session-directory-resolution.ts @@ -13,8 +13,10 @@ * The ordering discriminator is **whether the server confirmed the path**, not * whether the value is local or synced: * - * 1. `authoritative` — the child store that actually holds the session, then - * the session's own record. Server-backed truth for an indexed session. + * 1. `authoritative` — the session's own record, then a child store that holds + * it. Server-backed truth for an indexed session. Record first because + * holding a session proves containment, not ownership: a project's session + * list includes its worktrees' sessions so the sidebar can group them. * 2. `selected` — the directory captured when the session was selected, but * only when it came from a server response (the directory `createSession` * returned, which may be a canonicalized form of what was requested). A @@ -42,7 +44,7 @@ export type SessionDirectorySource = | 'none' export type SessionDirectorySources = { - /** Directory of the child store that holds the session, or its own record. */ + /** The session record's own directory, or a store that holds it. */ authoritative?: string | null /** Server-confirmed directory captured at selection. Never a guessed one. */ selected?: string | null diff --git a/packages/ui/src/sync/session-ui-store.ts b/packages/ui/src/sync/session-ui-store.ts index ef450f54..dbef659c 100644 --- a/packages/ui/src/sync/session-ui-store.ts +++ b/packages/ui/src/sync/session-ui-store.ts @@ -406,14 +406,12 @@ const getAttachmentForSession = (sessionId: string | null | undefined): SessionW return useSessionWorktreeStore.getState().getAttachment(sessionId) } -/** - * Authoritative directory for a session: the child store that holds it, and - * only then the session record's own fields. `null` means "not indexed yet", - * never "no directory" — callers must fall back rather than treat it as empty. - */ /** * The directory that owns a session, from the two server-backed signals. * + * `null` means "not indexed yet", never "no directory" — callers must fall back + * rather than treat it as empty. + * * The session's own record wins. Holding a session in a child store proves * containment, not ownership: a project's session list legitimately includes * the sessions of its worktrees so the sidebar can group them, so the parent diff --git a/packages/ui/src/sync/sync-refs.ts b/packages/ui/src/sync/sync-refs.ts index 0d96911c..ccc236f2 100644 --- a/packages/ui/src/sync/sync-refs.ts +++ b/packages/ui/src/sync/sync-refs.ts @@ -123,13 +123,15 @@ export function getAllSyncSessionMap(): ReadonlyMap { } const waitSeconds = Number(input?.timeout) > 0 ? Number(input.timeout) : DEFAULT_WAIT_TIMEOUT_SECONDS; const waitTimeoutMs = (waitSeconds * 1000) + WAIT_HTTP_TIMEOUT_BUFFER_MS; - // Waiting for the session and provisioning its worktree are additive, so the - // window must cover whichever is longer rather than only the wait. - return provisionsWorktree ? Math.max(waitTimeoutMs, WORKTREE_PROVISION_TIMEOUT_MS) : waitTimeoutMs; + // The server provisions the worktree inside session creation, before it + // starts waiting for the session to go idle, so the two windows run in + // sequence rather than overlapping. The client window has to cover both. + return provisionsWorktree ? waitTimeoutMs + WORKTREE_PROVISION_TIMEOUT_MS : waitTimeoutMs; }; export const requestControlAction = async (port, action, input, options = {}) => { diff --git a/packages/web/bin/lib/cli-control.test.js b/packages/web/bin/lib/cli-control.test.js index 1ae93015..5b19853e 100644 --- a/packages/web/bin/lib/cli-control.test.js +++ b/packages/web/bin/lib/cli-control.test.js @@ -28,11 +28,10 @@ describe('resolveControlTimeoutMs', () => { expect(resolveControlTimeoutMs({ worktree: ' ' }, {})).toBeUndefined(); }); - it('covers worktree provisioning even when the wait window is shorter', () => { - expect(resolveControlTimeoutMs({ wait: true, timeout: 30, worktree: 'feature' }, {})).toBe(120_000); - }); - - it('keeps a longer wait window when it outlasts worktree provisioning', () => { - expect(resolveControlTimeoutMs({ wait: true, worktree: 'feature' }, {})).toBe(630_000); + it('covers provisioning and waiting in sequence when both are requested', () => { + // The server creates the worktree before it begins waiting for the session, + // so the client window must span both rather than the longer of the two. + expect(resolveControlTimeoutMs({ wait: true, timeout: 30, worktree: 'feature' }, {})).toBe(180_000); + expect(resolveControlTimeoutMs({ wait: true, worktree: 'feature' }, {})).toBe(750_000); }); });