Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 79 additions & 5 deletions packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1902,11 +1902,9 @@ describe('DaemonSessionProvider', () => {
expect(sessionId).toBeDefined();
await act(async () => {
actions?.applyGoalSnapshot(sessionId!, created);
});
expect(connection?.goalState).toBe(created);

pendingGoal.resolve({ snapshot: { v: 2, goal: null, activity: 'idle' } });
await act(async () => {
pendingGoal.resolve({
snapshot: { v: 2, goal: null, activity: 'idle' },
});
await flushPromises();
});

Expand Down Expand Up @@ -8572,6 +8570,81 @@ describe('DaemonSessionProvider', () => {
expect(loadCalls[1]?.[3]).toBe('client-a');
});

it('hydrates Goal state without waiting for session metadata after a switch', async () => {
sdkMocks.sessions.push(createMockSession({ sessionId: 'session-a' }));
let actions: DaemonSessionActions | undefined;
let connection: DaemonConnectionState | undefined;

function Harness() {
actions = useDaemonActions();
connection = useDaemonConnection();
return null;
}

await renderWithProvider(<Harness />, { autoConnect: true });
await act(async () => {
await flushPromises();
});

const providers = createDeferred<unknown>();
const commands =
createDeferred<Awaited<ReturnType<MockSession['supportedCommands']>>>();
const context =
createDeferred<Awaited<ReturnType<MockSession['context']>>>();
sdkMocks.workspaceProviders.mockReturnValueOnce(providers.promise);
sdkMocks.sessions.push(
createMockSession({
sessionId: 'session-b',
supportedCommands: vi.fn(() => commands.promise),
context: vi.fn(() => context.promise),
}),
);

let loadSession: Promise<void> | undefined;
act(() => {
loadSession = requireActions(actions).loadSession('session-b');
});
await act(async () => {
await wait(5);
await loadSession;
await flushPromises();
});
const goalStateBeforeMetadata = connection?.goalState;

providers.resolve({
v: 1,
workspaceCwd: '/mock-workspace',
initialized: true,
providers: [],
});
commands.resolve({
v: 1,
sessionId: 'session-b',
availableCommands: [],
availableSkills: [],
});
context.resolve({
v: 1,
sessionId: 'session-b',
workspaceCwd: '/mock-workspace',
state: {},
});
await act(async () => {
await flushPromises();
});

expect(goalStateBeforeMetadata).toEqual({
v: 2,
goal: null,
activity: 'idle',
});
expect(connection?.goalState).toEqual({
v: 2,
goal: null,
activity: 'idle',
});
});

it('retries a session switch while the target session is closing', async () => {
const firstSession = createMockSession({ sessionId: 'session-a' });
const secondSession = createMockSession({ sessionId: 'session-b' });
Expand Down Expand Up @@ -9113,6 +9186,7 @@ describe('DaemonSessionProvider', () => {
expect(connection).toMatchObject({
sessionId: 'session-1',
displayName: 'Updated session',
goalState: { v: 2, goal: null, activity: 'idle' },
});
});

Expand Down
80 changes: 55 additions & 25 deletions packages/webui/src/daemon/session/DaemonSessionProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import {
type DaemonTurnCompleteData,
type DaemonUiEvent,
type DaemonUnrecognizedDiagnostic,
type GoalSnapshotV2,
} from '@qwen-code/sdk/daemon';
import {
createDaemonSessionActions,
Expand Down Expand Up @@ -2280,13 +2279,7 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) {
: activeSession.workspaceCwd
? client.workspaceByCwd(activeSession.workspaceCwd).workspaceGit()
: client.workspaceGit();
const [
providerResult,
commandResult,
contextResult,
gitResult,
goalResult,
] = await Promise.allSettled([
const metadataPromise = Promise.allSettled([
canReuseSessionMetadata
? Promise.resolve(undefined)
: client.workspaceProviders(),
Expand All @@ -2297,8 +2290,55 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) {
? Promise.resolve(undefined)
: activeSession.context(),
gitPromise,
activeSession.goal(),
]);
// Hydrate Goal ownership independently so unrelated metadata cannot
// leave Slash commands blocked. Reconcile against any Goal frame
// that landed while the read was in flight.
const goalPromise = activeSession
.goal()
.then(
(response) => response.snapshot,
() => undefined,
)
.then((goalState) => {
if (
disposed ||
abort.signal.aborted ||
sessionRef.current !== activeSession
) {
return goalState;
}
setConnection((current) => {
if (
sessionRef.current !== activeSession ||
current.sessionId !== activeSession.sessionId
) {
return current;
}
if (!goalState && goalStateAtLoadStart !== undefined) {
return current;
}
return {
...current,
goalState: goalState
? selectGoalStateFromRead(
current.goalState,
goalState,
goalStateAtLoadStart?.goal?.goalId,
)
: (current.goalState ?? {
v: 2,
goal: null,
activity: 'idle',
}),
};
});
return goalState;
});
const [
[providerResult, commandResult, contextResult, gitResult],
goalState,
] = await Promise.all([metadataPromise, goalPromise]);
if (
disposed ||
abort.signal.aborted ||
Expand All @@ -2322,22 +2362,10 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) {
gitResult?.status === 'fulfilled'
? (gitResult.value.branch ?? undefined)
: undefined;
const goalState =
goalResult.status === 'fulfilled'
? goalResult.value.snapshot
: undefined;
// A failed goal fetch on a session with no known state still needs a
// snapshot so consumers stop treating the state as hydrating; it must
// never reconcile against a state a frame installed meanwhile.
const goalStateFallback =
goalResult.status === 'fulfilled' ||
goalStateAtLoadStart !== undefined
? undefined
: ({
v: 2,
goal: null,
activity: 'idle',
} satisfies GoalSnapshotV2);
goalState === undefined && goalStateAtLoadStart === undefined
? ({ v: 2, goal: null, activity: 'idle' } as const)
: undefined;
const loadWarningTexts = [
providerResult?.status === 'rejected'
? loadWarningsRef.current?.models
Expand Down Expand Up @@ -2375,7 +2403,9 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) {

setConnection((current) => {
if (
sessionRef.current !== activeSession ||
abort.signal.aborted ||
(sessionRef.current !== undefined &&

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The guard here changed from sessionRef.current !== activeSession to (sessionRef.current !== undefined && sessionRef.current !== activeSession).

The new form allows the final setConnection to proceed when sessionRef.current is undefined — which happens during a state resync (handleStateResync at ~line 2547 sets sessionRef.current = undefined while keeping the same sessionId). In that window, the old guard would have blocked this update; the new guard does not, so the load's final commit can set status: \'connected\' and loadingTranscript: undefined over the resync's status: \'connecting\'.

Is this intentional? If the resync overlaps with a metadata load, the metadata from the original load should still be valid (same session), so proceeding seems reasonable. I want to confirm that the abort.signal.aborted check is sufficient to catch the cases where proceeding would be wrong.

sessionRef.current !== activeSession) ||
current.sessionId !== activeSession.sessionId
) {
return current;
Expand Down
Loading