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); }); });