diff --git a/frontend/CLAUDE.md b/frontend/CLAUDE.md index 69d7bbb1..f975b4a9 100644 --- a/frontend/CLAUDE.md +++ b/frontend/CLAUDE.md @@ -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 diff --git a/frontend/src/api/__tests__/errors.test.ts b/frontend/src/api/__tests__/errors.test.ts index e36016e4..e5811277 100644 --- a/frontend/src/api/__tests__/errors.test.ts +++ b/frontend/src/api/__tests__/errors.test.ts @@ -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')); diff --git a/frontend/src/api/errors.ts b/frontend/src/api/errors.ts index 0554a045..8236a457 100644 --- a/frontend/src/api/errors.ts +++ b/frontend/src/api/errors.ts @@ -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 @@ -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 @@ -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); diff --git a/frontend/src/pages/chat/__tests__/sendFailure.test.tsx b/frontend/src/pages/chat/__tests__/sendFailure.test.tsx new file mode 100644 index 00000000..d3ff2d2f --- /dev/null +++ b/frontend/src/pages/chat/__tests__/sendFailure.test.tsx @@ -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) { + 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([]); + const chat = useMemo( + () => ({ chatMessages, updateChatMessages }), + [chatMessages], + ); + return ( + + + + + + + + ); + } + + render(); + return screen.getByPlaceholderText( + /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(); + }); +}); diff --git a/frontend/src/pages/chat/index.tsx b/frontend/src/pages/chat/index.tsx index 9c70025d..73f54163 100644 --- a/frontend/src/pages/chat/index.tsx +++ b/frontend/src/pages/chat/index.tsx @@ -171,14 +171,24 @@ export default function Chat() { useEffect(() => { if (chatMessages.length !== 0) return; void (async () => { - const docContext = await refreshDocContext(); - updateChatMessages( - withCurrentDocContext( - [], - docContext, - formatDocBriefForPrompt(briefRef.current), - ), - ); + try { + const docContext = await refreshDocContext(); + updateChatMessages( + withCurrentDocContext( + [], + docContext, + formatDocBriefForPrompt(briefRef.current), + ), + ); + } catch (error) { + // The transcript stays unseeded, so without this the writer gets a + // normal-looking welcome screen backed by a chat that cannot answer. + console.error( + 'Could not read the document to start the chat:', + error, + ); + setErrorInfo(describeGenerationError(error)); + } })(); }, [chatMessages.length, refreshDocContext, updateChatMessages]); @@ -220,24 +230,35 @@ export default function Chat() { setErrorInfo(null); setFailedMessage(null); - // Pull the current document context at send time so the model sees the - // document as it is now, then inject it as the doc-context message. - const docContext = await refreshDocContext(); - let newMessages = [ - ...withCurrentDocContext( - chatMessages, - docContext, - formatDocBriefForPrompt(briefRef.current), - ), - { role: 'user', content: text }, - { role: 'assistant', content: '' }, - ]; - - updateChatMessages(newMessages); - setShowScrollButton(false); - updateMessage(''); + // Everything from here on runs inside the try: the document read can fail + // too (on Google Docs it is an Apps Script round-trip, which rejects + // outright once the sidebar's authorization has lapsed), and a failure + // outside it would leave `isSendingMessage` stuck true — a permanently + // disabled input box with no message on screen explaining why. + let newMessages: ChatMessage[] = []; + /** Whether the turn made it into the transcript, i.e. whether the catch + * has anything to roll back. False when the document read failed. */ + let turnAppended = false; try { + // Pull the current document context at send time so the model sees the + // document as it is now, then inject it as the doc-context message. + const docContext = await refreshDocContext(); + newMessages = [ + ...withCurrentDocContext( + chatMessages, + docContext, + formatDocBriefForPrompt(briefRef.current), + ), + { role: 'user', content: text }, + { role: 'assistant', content: '' }, + ]; + + turnAppended = true; + updateChatMessages(newMessages); + setShowScrollButton(false); + updateMessage(''); + const deltas = streamTextDeltas({ model: languageModel, providerOptions: openaiProviderOptions, @@ -262,12 +283,17 @@ export default function Chat() { return; } const info = describeGenerationError(error); - console.error('Error while streaming chat response:', error); - // Nothing streamed: roll the turn back (empty assistant bubble + the - // user message it was answering) and hand the text back to the input - // box, so Retry re-sends it once rather than duplicating the turn. - // If part of a reply did arrive, keep it and just show the error under it. - if (newMessages[newMessages.length - 1].content === '') { + console.error('Chat turn failed:', error); + if (!turnAppended) { + // Failed before the turn reached the transcript (the document read + // went first): there is nothing to roll back, and the writer's text + // is still sitting in the input box. + setFailedMessage({ text, source }); + } else if (newMessages[newMessages.length - 1].content === '') { + // Nothing streamed: roll the turn back (empty assistant bubble + the + // user message it was answering) and hand the text back to the input + // box, so Retry re-sends it once rather than duplicating the turn. + // If part of a reply did arrive, keep it and just show the error under it. updateChatMessages(newMessages.slice(0, -2)); updateMessage(text); setFailedMessage({ text, source }); diff --git a/frontend/src/pages/draft/index.tsx b/frontend/src/pages/draft/index.tsx index 36b1718d..a7a4aa69 100644 --- a/frontend/src/pages/draft/index.tsx +++ b/frontend/src/pages/draft/index.tsx @@ -391,7 +391,19 @@ export default function Draft() { } // Pull the current document context at refresh time rather than tracking // it continuously. - const docContext = await refreshDocContext(); + let docContext: DocContext; + try { + docContext = await refreshDocContext(); + } catch (err) { + // A background tick the writer did not ask for: skip this cycle rather + // than putting a notice on screen over work they are in the middle of. + // A failure they *did* ask for still reports itself. + console.warn( + 'Auto-refresh skipped, could not read the document:', + err, + ); + return; + } const request = { docContext, type: modesToShow[0], @@ -421,6 +433,44 @@ export default function Draft() { autoRefreshInterval, ); + /** Run one feature: read the document, then generate from what it says. */ + const requestSuggestion = useCallback( + async (mode: string) => { + setActiveMode(mode); + // Pull the current document context at click time. + let docContext: DocContext; + try { + docContext = await refreshDocContext(); + } catch (err) { + // The read is a request in its own right (an Apps Script round-trip + // on Google Docs), and it fails before `getSuggestion` — which owns + // the notice — is ever called. Report it here, or the click is + // silently lost. + console.error( + 'Could not read the document for a suggestion:', + err, + ); + setEmptyResult(false); + updateErrorInfo(describeGenerationError(err)); + return; + } + draftLog.suggestionRequested(log, { + generationType: mode, + docContext, + }); + resetAutoRefresh(); + void getSuggestion( + { + docContext, + type: mode, + brief: formatDocBriefForPrompt(briefRef.current), + }, + true, + ); + }, + [refreshDocContext, resetAutoRefresh, getSuggestion, log], + ); + return (
@@ -448,30 +498,7 @@ export default function Draft() { key={mode} className={`${classes.featureCard} ${isActive ? classes.active : ''}`} onClick={() => { - void (async () => { - setActiveMode(mode); - // Pull the current document context at click time. - const docContext = - await refreshDocContext(); - draftLog.suggestionRequested( - log, - { - generationType: mode, - docContext, - }, - ); - resetAutoRefresh(); - getSuggestion( - { - docContext, - type: mode, - brief: formatDocBriefForPrompt( - briefRef.current, - ), - }, - true, - ); - })(); + void requestSuggestion(mode); }} disabled={isLoading} type="button" diff --git a/frontend/src/pages/revise/index.tsx b/frontend/src/pages/revise/index.tsx index 45c6710d..451768ae 100644 --- a/frontend/src/pages/revise/index.tsx +++ b/frontend/src/pages/revise/index.tsx @@ -379,8 +379,38 @@ export default function Revise() { : `Go part-by-part through the document. For each part, please do the following: ${prompt.prompt}`; // Pull the current document context at request time rather than - // tracking it continuously. - const currentContext = await refreshDocContext(); + // tracking it continuously. This read can fail on its own — on Google + // Docs it is an Apps Script round-trip — and it happens before any card + // exists, so it needs its own guard: without one the writer clicks a + // feature and absolutely nothing happens. + let currentContext: DocContext; + try { + currentContext = await refreshDocContext(); + } catch (err) { + const info = describeGenerationError(err); + console.error( + 'Could not read the document to run a feature:', + err, + ); + // Report it the way every other failure here is reported: as the card + // the writer was expecting, carrying the error instead of a result. + // Its context is empty because that is the truth — the read is what + // failed. + const failed = new Visualization( + request, + { beforeCursor: '', selectedText: '', afterCursor: '' }, + prompt, + ); + failed.error = info; + failed.done = true; + setVisualizations((prev) => [...prev, failed]); + reviseLog.visualizationError(log, { + feature: prompt.keyword, + error: info.detail, + code: info.code, + }); + return; + } const newViz = new Visualization(request, currentContext, prompt); setVisualizations((prev) => [...prev, newViz]);