Skip to content
Open
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
15 changes: 15 additions & 0 deletions frontend/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,21 @@ Log `info.detail` (provider text) and `info.code`, not the sentence shown on
screen. A successful-but-empty generation is also a visible outcome (`tone="info"`
notice), never silence.

**The document read is part of the request, so put it inside the `try`.** Every
page pulls fresh context (`refresh()` from `useDocContext`) at request time, and
on Google Docs that pull is an Apps Script round-trip that fails on its own
terms: a sidebar left open long enough loses its grant and every bridge call
rejects with `ScriptError: Authorization is required to perform that action.`
When that `await` sits outside the `try/finally` — as it did in `pages/chat` —
the rejection escapes into a `void`-ed handler, the `finally` never runs, and the
in-flight flag stays true: a permanently disabled input box, an unchanged
transcript, and nothing on screen saying why. `describeGenerationError` maps that
`ScriptError` to copy naming the one step that works (reopen the sidebar) with
`retryable: false`, since retrying in place cannot re-authorize. The one
exception is a read on a background timer (draft's auto-refresh), which skips the
cycle with a `console.warn` rather than interrupting the writer with a notice
they didn't ask for.

**The system prompt goes in `instructions`, never in `messages`.** Since ai@7, a
`role: 'system'` message inside `messages` fails validation before the request
leaves the browser — `InvalidPromptError: System messages are not allowed in the
Expand Down
28 changes: 28 additions & 0 deletions frontend/src/api/__tests__/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,34 @@ describe('describeGenerationError', () => {
expect(info.retryable).toBe(true);
});

it('tells the writer to reopen the sidebar when Apps Script authorization has lapsed', () => {
// What `google.script.run`'s failure handler hands back once a sidebar has
// been open long enough to lose its grant.
const scriptError = Object.assign(
new Error('Authorization is required to perform that action.'),
{ name: 'ScriptError' },
);
const info = describeGenerationError(scriptError);

expect(info.message).toMatch(/open it again from the Extensions menu/i);
// Retrying in place cannot re-authorize, so no Retry button.
expect(info.retryable).toBe(false);
expect(info.detail).toBe(
'Authorization is required to perform that action.',
);
});

it('treats other Apps Script failures as worth retrying', () => {
const info = describeGenerationError(
Object.assign(new Error('Service unavailable'), {
name: 'ScriptError',
}),
);

expect(info.message).toMatch(/would not let the add-in read/i);
expect(info.retryable).toBe(true);
});

it('keeps the raw text as detail for anything it cannot classify', () => {
const info = describeGenerationError(new Error('something odd'));

Expand Down
49 changes: 49 additions & 0 deletions frontend/src/api/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@
* `responseBody` is the provider's JSON error envelope as text.
* - Aborts and timeouts throw a `DOMException` named `AbortError`/`TimeoutError`.
* - A dead network throws `TypeError: Failed to fetch`.
* - The Google Docs bridge rejects with an Apps Script `ScriptError`, which
* carries a name and a message and nothing else. Pages read the document
* before they generate, so this failure reaches the writer at the same moment
* and through the same notice as a model failure; it is described here for
* that reason.
*
* {@link describeGenerationError} normalizes all of them into one
* {@link GenerationErrorInfo}: a plain-language sentence for the writer, the raw
Expand Down Expand Up @@ -176,6 +181,41 @@ function abortKind(err: unknown): 'timeout' | 'abort' | null {
return null;
}

/**
* Apps Script failures from the Google Docs bridge (`google.script.run`, wrapped
* in `sidebar.html`).
*
* The one that reaches writers is an expired authorization: a sidebar left open
* long enough loses its grant, and from then on *every* bridge call rejects with
* `ScriptError: Authorization is required to perform that action.` — including
* the document read each page does before it generates. Retrying in place cannot
* clear that, since the grant is re-obtained when the sidebar is opened, so it is
* `retryable: false` and the copy names the step that does work.
*/
function appsScriptFailure(
err: unknown,
): { message: string; retryable: boolean } | null {
const record = asRecord(err);
const message = asString(record?.message) ?? '';
const isScriptError =
asString(record?.name) === 'ScriptError' ||
message.startsWith('ScriptError');
if (!isScriptError) return null;

if (/authorization is required|do not have permission/i.test(message)) {
return {
message:
"The add-in's access to this document has expired, so it could not read your text. Close this sidebar and open it again from the Extensions menu to reconnect — nothing in your document has changed.",
retryable: false,
};
}
return {
message:
'Google Docs would not let the add-in read your document just now. Please try again; if it keeps failing, close the sidebar and open it again.',
retryable: true,
};
}

/**
* Normalize any thrown value (or streamed error part) into presentable form.
* Never throws; unrecognized input degrades to a generic message plus whatever
Expand All @@ -196,6 +236,15 @@ export function describeGenerationError(err: unknown): GenerationErrorInfo {
};
}

const appsScript = appsScriptFailure(err);
if (appsScript !== null) {
return {
message: appsScript.message,
detail: asString(asRecord(err)?.message) ?? String(err),
retryable: appsScript.retryable,
};
}

// APICallError keeps the provider's JSON body as text; everything else we
// read straight off the value we were handed.
const isApiCallError = APICallError.isInstance(err);
Expand Down
169 changes: 169 additions & 0 deletions frontend/src/pages/chat/__tests__/sendFailure.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
// @vitest-environment jsdom
/**
* What the chat does when the *document read* fails, as opposed to the model.
*
* On Google Docs every send begins with an Apps Script round-trip, and a sidebar
* left open long enough loses its authorization — so the read starts rejecting
* partway through a session that was working. It used to run outside the
* try/finally, which left `isSendingMessage` true forever: the writer got a
* permanently disabled input box, an unchanged transcript, and nothing on screen
* saying why.
*/
import {
act,
cleanup,
fireEvent,
render,
screen,
waitFor,
} from '@testing-library/react';
import { useMemo, useState } from 'react';
import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest';
import { ChatContext } from '@/contexts/chatContext';
import {
DocBriefContext,
type DocBriefContextValue,
EMPTY_DOC_BRIEF,
} from '@/contexts/docBriefContext';
import { EditorContext } from '@/contexts/editorContext';
import Chat from '..';

vi.mock('@/hooks/useLog', () => ({ useLog: () => vi.fn() }));
// The model is never reached in these tests; the point is the step before it.
vi.mock('@/api/openai', () => ({
languageModel: {},
openaiProviderOptions: {},
}));
const streamTextDeltas = vi.fn();
vi.mock('@/api/generate', () => ({
streamTextDeltas: (...args: unknown[]) => {
streamTextDeltas(...args);
// An empty stream, so a call that slips through ends rather than throwing
// something the page would report as a model failure.
return (async function* () {})();
},
}));

const DOC: DocContext = {
beforeCursor: 'My draft so far.',
selectedText: '',
afterCursor: '',
};

/** The rejection `google.script.run` produces once the grant has lapsed. */
function authorizationLapsed(): Error {
return Object.assign(
new Error('Authorization is required to perform that action.'),
{ name: 'ScriptError' },
);
}

/**
* A document bridge that starts healthy and lapses when the test says so —
* the shape of the real session, which reads the document fine for hours and
* then stops. Nothing here depends on *how many* reads the page makes.
*/
function lapsingBridge() {
let lapsed = false;
return {
lapse: () => {
lapsed = true;
},
getDocContext: () =>
lapsed
? Promise.reject(authorizationLapsed())
: Promise.resolve(DOC),
};
}

function renderChat(getDocContext: () => Promise<DocContext>) {
const editorAPI = { getDocContext } as unknown as EditorAPI;
const brief: DocBriefContextValue = {
brief: EMPTY_DOC_BRIEF,
setField: vi.fn(),
status: 'ready',
};

function Harness() {
// Real chat state, so a rolled-back turn is observable.
const [chatMessages, updateChatMessages] = useState<ChatMessage[]>([]);
const chat = useMemo(
() => ({ chatMessages, updateChatMessages }),
[chatMessages],
);
return (
<EditorContext.Provider value={editorAPI}>
<DocBriefContext.Provider value={brief}>
<ChatContext.Provider value={chat}>
<Chat />
</ChatContext.Provider>
</DocBriefContext.Provider>
</EditorContext.Provider>
);
}

render(<Harness />);
return screen.getByPlaceholderText<HTMLTextAreaElement>(
/Ask something about your document/,
);
}

async function send(input: HTMLTextAreaElement, text: string) {
fireEvent.change(input, { target: { value: text } });
const form = input.closest('form');
if (!form) throw new Error('chat input is not in a form');
fireEvent.submit(form);
await waitFor(() => {
expect(screen.queryByRole('alert')).not.toBeNull();
});
}

/** Opens the chat on a healthy bridge, then lapses it, as a real session does. */
async function openThenLapse() {
const bridge = lapsingBridge();
const input = renderChat(bridge.getDocContext);
// Let the mount-time reads settle, so the chat is seeded and quiet before
// anything fails — the failure under test belongs to the send.
await act(async () => {});
expect(screen.queryByRole('alert')).toBeNull();
bridge.lapse();
return input;
}

describe('Chat, when the document cannot be read', () => {
beforeAll(() => {
// jsdom does no layout, so the transcript's auto-scroll has nothing to
// call. It is irrelevant to what these tests assert.
Element.prototype.scrollTo = vi.fn();
});

afterEach(() => {
cleanup();
vi.clearAllMocks();
});

it('re-enables the input and says what happened, rather than hanging disabled', async () => {
const input = await openThenLapse();

await send(input, 'What is my argument?');

expect(screen.getByRole('alert').textContent).toMatch(
/open it again from the Extensions menu/i,
);
// The regression: this stayed true for the life of the page.
expect(input.disabled).toBe(false);
expect(streamTextDeltas).not.toHaveBeenCalled();
});

it('keeps the writer’s text instead of clearing it into a turn that never happened', async () => {
const input = await openThenLapse();

await send(input, 'What is my argument?');

expect(input.value).toBe('What is my argument?');
// Nothing was appended, so the transcript is still the welcome screen.
expect(
screen.getByText('What do you think about your document so far?'),
).toBeTruthy();
});
});
Loading
Loading