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
150 changes: 110 additions & 40 deletions apps/web/src/components/ChatView.browser.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
ProviderInstanceId,
type ServerConfig,
type ServerLifecycleWelcomePayload,
type ServerProvider,
type ThreadId,
type TurnId,
WS_METHODS,
Expand Down Expand Up @@ -2357,7 +2358,7 @@ describe("ChatView timeline estimator parity (full app)", () => {
}
});

it("keeps dismiss-only composer banners aligned on mobile", async () => {
it("docks the version mismatch notice to the composer and dismisses it on mobile", async () => {
const mounted = await mountChatView({
viewport: COMPACT_FOOTER_VIEWPORT,
snapshot: createSnapshotForTargetUser({
Expand All @@ -2376,24 +2377,44 @@ describe("ChatView timeline estimator parity (full app)", () => {
});

try {
const banner = await waitForElement(
() =>
Array.from(document.querySelectorAll<HTMLElement>('[data-slot="alert"]')).find(
(element) => element.textContent?.includes("Client and server versions differ"),
) ?? null,
"Unable to find version mismatch banner.",
const dock = await waitForElement(
() => document.querySelector<HTMLElement>('[data-composer-notice-dock="true"]'),
"Unable to find the composer notice dock.",
);
const title = banner.querySelector<HTMLElement>('[data-slot="alert-title"]');
const description = banner.querySelector<HTMLElement>('[data-slot="alert-description"]');
const dismissButton = banner.querySelector<HTMLButtonElement>(
'button[aria-label="Dismiss version mismatch warning"]',
expect(dock.textContent).toContain("Client and server versions differ.");
// Version skew is informational, so it never claims a louder colour.
expect(
dock
.querySelector("[data-composer-notice-severity]")
?.getAttribute("data-composer-notice-severity"),
).toBe("info");

const composerSurface = document.querySelector<HTMLElement>(
"[data-chat-composer-mobile-collapsed]",
);
expect(composerSurface).toBeTruthy();
// Docked means attached: the notice sits directly on the composer's top
// edge, sharing its width.
expect(
Math.abs(dock.getBoundingClientRect().left - composerSurface!.getBoundingClientRect().left),
).toBeLessThan(2);
expect(
Math.abs(
dock.getBoundingClientRect().bottom - composerSurface!.getBoundingClientRect().top,
),
).toBeLessThan(2);

expect(title).toBeTruthy();
expect(description).toBeTruthy();
const dismissButton = dock.querySelector<HTMLButtonElement>(
'button[aria-label="Dismiss version mismatch warning"]',
);
expect(dismissButton).toBeTruthy();
expect(dismissButton!.getBoundingClientRect().top).toBeLessThan(
description!.getBoundingClientRect().top,
dismissButton!.click();

await vi.waitFor(
() => {
expect(document.querySelector('[data-composer-notice-dock="true"]')).toBeNull();
},
{ timeout: 8_000, interval: 16 },
);
} finally {
await mounted.cleanup();
Expand Down Expand Up @@ -3740,27 +3761,40 @@ describe("ChatView timeline estimator parity (full app)", () => {
}
});

it("holds back a send to a signed-out provider until the user chooses Send anyway", async () => {
async function mountSignedOutProviderSend(options: {
/** Providers the recheck behind "I've signed in" resolves with. */
refreshedProviders: (signedOut: ServerProvider) => ReadonlyArray<ServerProvider>;
}) {
setDraftThreadWithoutWorktree();
let signedOutProvider: ServerProvider | null = null;
const mounted = await mountChatView({
viewport: DEFAULT_VIEWPORT,
snapshot: createDraftOnlySnapshot(),
configureFixture: (nextFixture) => {
signedOutProvider = {
...nextFixture.serverConfig.providers[0]!,
status: "warning",
auth: { status: "unauthenticated" },
};
nextFixture.serverConfig = {
...nextFixture.serverConfig,
providers: [
{
...nextFixture.serverConfig.providers[0]!,
status: "warning",
auth: { status: "unauthenticated" },
},
],
providers: [signedOutProvider],
};
},
resolveRpc: (body) =>
body._tag === ORCHESTRATION_WS_METHODS.dispatchCommand
? { sequence: fixture.snapshot.snapshotSequence + 1 }
: undefined,
resolveRpc: (body) => {
if (body._tag === ORCHESTRATION_WS_METHODS.dispatchCommand) {
return { sequence: fixture.snapshot.snapshotSequence + 1 };
}
if (body._tag === WS_METHODS.serverRefreshProviders) {
return {
providers: encodeServerConfig({
...fixture.serverConfig,
providers: options.refreshedProviders(signedOutProvider!),
}).providers,
};
}
return undefined;
},
});

const turnStartRequests = () =>
Expand All @@ -3770,28 +3804,65 @@ describe("ChatView timeline estimator parity (full app)", () => {
request.type === "thread.turn.start",
);

try {
useComposerDraftStore.getState().setPrompt(THREAD_REF, "Explain this repo");
await waitForLayout();
useComposerDraftStore.getState().setPrompt(THREAD_REF, "Explain this repo");
await waitForLayout();

(await waitForSendButton()).click();
(await waitForSendButton()).click();

const sendAnyway = await waitForButtonByText("Send anyway");
// The turn never left the client, and the draft survived the interruption.
expect(turnStartRequests()).toHaveLength(0);
expect(document.body.textContent).toContain("Codex sign-in required");
expect(useComposerDraftStore.getState().draftsByThreadKey[THREAD_KEY]?.prompt).toBe(
"Explain this repo",
);
const confirmSignedIn = await waitForButtonByText("I've signed in");
// The turn never left the client, and the draft survived the interruption.
expect(turnStartRequests()).toHaveLength(0);
expect(document.body.textContent).toContain("Codex needs sign-in.");
expect(useComposerDraftStore.getState().draftsByThreadKey[THREAD_KEY]?.prompt).toBe(
"Explain this repo",
);

return { confirmSignedIn, mounted, turnStartRequests };
}

it("sends the held message once the recheck behind I've signed in comes back clean", async () => {
const { confirmSignedIn, mounted, turnStartRequests } = await mountSignedOutProviderSend({
refreshedProviders: (signedOut) => [
{ ...signedOut, status: "ready", auth: { status: "authenticated" } },
],
});

sendAnyway.click();
try {
confirmSignedIn.click();

await vi.waitFor(
() => {
expect(turnStartRequests()).toHaveLength(1);
},
{ timeout: 8_000, interval: 16 },
);
// The recheck is not a bypass: exactly one turn, through the normal gate.
await waitForLayout();
expect(turnStartRequests()).toHaveLength(1);
} finally {
await mounted.cleanup();
}
});

it("keeps holding the message when the recheck still reports a signed-out provider", async () => {
const { confirmSignedIn, mounted, turnStartRequests } = await mountSignedOutProviderSend({
refreshedProviders: (signedOut) => [signedOut],
});

try {
confirmSignedIn.click();

await vi.waitFor(
() => {
expect(document.body.textContent).toContain("Still signed out.");
},
{ timeout: 8_000, interval: 16 },
);
expect(turnStartRequests()).toHaveLength(0);
expect(document.body.textContent).toContain("The terminal shows where the sign-in stopped.");
expect(useComposerDraftStore.getState().draftsByThreadKey[THREAD_KEY]?.prompt).toBe(
"Explain this repo",
);
} finally {
await mounted.cleanup();
}
Expand Down Expand Up @@ -8378,7 +8449,6 @@ describe("ChatView timeline estimator parity (full app)", () => {
const actions = document.querySelector<HTMLElement>(
'[data-chat-composer-actions="right"]',
);

expect(footer?.dataset.chatComposerFooterCompact).toBe("true");
expect(actions?.dataset.chatComposerPrimaryActionsCompact).toBe("true");
},
Expand Down
10 changes: 5 additions & 5 deletions apps/web/src/components/ChatView.logic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import {
resolveSendEnvMode,
shouldConfirmTerminalKill,
shouldOfferFailedTurnRetry,
shouldRenderThreadErrorBanner,
shouldShowThreadErrorNotice,
shouldRefreshThreadDetailAfterEventLoopStall,
shouldWriteThreadErrorToCurrentServerThread,
THREAD_DETAIL_STALL_REFRESH_COOLDOWN_MS,
Expand Down Expand Up @@ -517,10 +517,10 @@ describe("deriveProviderAuthReconnectPrompt", () => {
});
});

describe("shouldRenderThreadErrorBanner", () => {
describe("shouldShowThreadErrorNotice", () => {
it("hides provider authentication errors already rendered in the timeline", () => {
expect(
shouldRenderThreadErrorBanner({
shouldShowThreadErrorNotice({
threadError:
"Your access token could not be refreshed because your refresh token was revoked.",
hasInlineProviderAuthError: true,
Expand All @@ -530,7 +530,7 @@ describe("shouldRenderThreadErrorBanner", () => {

it("keeps provider authentication errors as a fallback when no inline recovery is visible", () => {
expect(
shouldRenderThreadErrorBanner({
shouldShowThreadErrorNotice({
threadError: "Not logged in. Run `codex login` in a terminal, then retry.",
hasInlineProviderAuthError: false,
}),
Expand All @@ -539,7 +539,7 @@ describe("shouldRenderThreadErrorBanner", () => {

it("keeps unrelated thread errors visible even when the timeline contains an auth error", () => {
expect(
shouldRenderThreadErrorBanner({
shouldShowThreadErrorNotice({
threadError: "Could not stop the background process.",
hasInlineProviderAuthError: true,
}),
Expand Down
9 changes: 5 additions & 4 deletions apps/web/src/components/ChatView.logic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1482,9 +1482,10 @@ export interface ProviderSendPreflightPrompt {
* A signed-out CLI still accepts the turn and then burns half a minute of
* reconnect attempts before surfacing a raw `401`, so when the snapshot we
* already hold says the selected instance cannot serve the turn, we say so up
* front. Provider snapshots go stale, so this only ever guides: the caller
* keeps a "Send anyway" path, and an `available` verdict (which includes an
* auth state we cannot judge) never interrupts anything.
* front. Provider snapshots go stale, so the hold is never final: the caller
* offers a recheck that re-probes the provider and, on a clean answer, sends
* the held turn through this same gate. An `available` verdict (which includes
* an auth state we cannot judge) never interrupts anything.
*/
export function deriveProviderSendPreflight(input: {
readonly instanceId: ProviderInstanceId | null | undefined;
Expand Down Expand Up @@ -1514,7 +1515,7 @@ export function deriveProviderSendPreflight(input: {
};
}

export function shouldRenderThreadErrorBanner(input: {
export function shouldShowThreadErrorNotice(input: {
readonly threadError: string | null | undefined;
readonly hasInlineProviderAuthError: boolean;
}): boolean {
Expand Down
Loading
Loading