diff --git a/artifacts/ai-testing-academy/src/context/AuthContext.tsx b/artifacts/ai-testing-academy/src/context/AuthContext.tsx index 12e2ab4..6c1f1e1 100644 --- a/artifacts/ai-testing-academy/src/context/AuthContext.tsx +++ b/artifacts/ai-testing-academy/src/context/AuthContext.tsx @@ -176,6 +176,13 @@ export function AuthProvider({ children, clientId = googleClientId() }: AuthProv }, auto_select: false, cancel_on_tap_outside: true, + // The popup flow navigates to accounts.google.com, and on Android that + // host is a verified App Link for the Google app: Chrome hands the + // navigation to the OS, the reader gets an "Open with" chooser, and the + // sign-in never completes. FedCM has the browser draw the dialog itself, + // so there is no navigation for the OS to intercept. Browsers without + // FedCM fall back to the popup flow unchanged. + use_fedcm_for_button: true, }); api.renderButton(parent, { ...BUTTON_OPTIONS, locale }); }, diff --git a/artifacts/ai-testing-academy/src/lib/googleIdentity.ts b/artifacts/ai-testing-academy/src/lib/googleIdentity.ts index ca5d654..d9681c0 100644 --- a/artifacts/ai-testing-academy/src/lib/googleIdentity.ts +++ b/artifacts/ai-testing-academy/src/lib/googleIdentity.ts @@ -37,6 +37,12 @@ export interface GoogleIdentityApi { callback: (response: GoogleCredentialResponse) => void; auto_select?: boolean; cancel_on_tap_outside?: boolean; + /** + * Let the browser mediate the sign-in dialog instead of opening a popup to + * accounts.google.com. Where FedCM is unavailable, Google falls back to the + * popup flow on its own, so this is safe to ask for unconditionally. + */ + use_fedcm_for_button?: boolean; }): void; renderButton(parent: HTMLElement, options: GoogleButtonOptions): void; disableAutoSelect(): void; diff --git a/artifacts/ai-testing-academy/src/lib/locales/en.ts b/artifacts/ai-testing-academy/src/lib/locales/en.ts index e929b20..30b885f 100644 --- a/artifacts/ai-testing-academy/src/lib/locales/en.ts +++ b/artifacts/ai-testing-academy/src/lib/locales/en.ts @@ -235,6 +235,10 @@ export const en = { errBlockedTry: 'Try switching to a different provider.\n', errBlockedOpenUrl: '', errApiPrefix: 'API error (', + errProxyBusy: + 'The free AI allowance is used up for now. Wait a little, or connect your own provider key in Settings to keep going.', + errProxyUnavailable: + 'The academy’s own AI key is unavailable right now. Connect your own provider key in Settings to keep going.', errNoJson: 'Could not parse JSON from the model response. Please try again.', uploadReading: '⏳ Reading ', uploadPreparing: '⏳ Preparing to read ', diff --git a/artifacts/ai-testing-academy/src/lib/locales/he.ts b/artifacts/ai-testing-academy/src/lib/locales/he.ts index b0c4052..c5e1427 100644 --- a/artifacts/ai-testing-academy/src/lib/locales/he.ts +++ b/artifacts/ai-testing-academy/src/lib/locales/he.ts @@ -231,6 +231,10 @@ export const he: Locale = { errBlockedTry: 'נסה לעבור לספק אחר.\n', errBlockedOpenUrl: '', errApiPrefix: 'שגיאת API (', + errProxyBusy: + 'מכסת ה-AI החינמית נוצלה כרגע. המתינו מעט, או חברו מפתח ספק משלכם בהגדרות כדי להמשיך.', + errProxyUnavailable: + 'מפתח ה-AI של האקדמיה אינו זמין כרגע. חברו מפתח ספק משלכם בהגדרות כדי להמשיך.', errNoJson: 'לא ניתן לנתח JSON מתגובת המודל. נסה שוב.', uploadReading: '⏳ קורא ', uploadPreparing: '⏳ מתכונן לקרוא את ', diff --git a/artifacts/ai-testing-academy/src/lib/providers.ts b/artifacts/ai-testing-academy/src/lib/providers.ts index be025b5..5a47247 100644 --- a/artifacts/ai-testing-academy/src/lib/providers.ts +++ b/artifacts/ai-testing-academy/src/lib/providers.ts @@ -120,6 +120,22 @@ export async function loadServerConfig(): Promise { } } +/** + * What a proxy failure means to the reader, rather than what the wire said. + * + * The two statuses a visitor actually meets have an answer they can act on, and + * "API error (429)" is not it — it names a protocol they did not know they were + * speaking. Both point at the same way forward, because connecting a key of + * their own is what lets them carry on either way. Anything else keeps the + * status and the server's text, which is the useful thing to paste into a bug + * report; the prefix is translated, as it already is on the direct-call paths. + */ +function proxyFailure(status: number, body: string, S: Locale['s']): string { + if (status === 429) return S.errProxyBusy; + if (status === 503) return S.errProxyUnavailable; + return S.errApiPrefix + status + '): ' + body.slice(0, 300); +} + async function callServerProxy( model: string, system: string, @@ -135,7 +151,7 @@ async function callServerProxy( }); publishAnonymousQuota(res); const data = (await res.json()) as { text?: string; error?: string; truncated?: boolean }; - if (!res.ok) throw new Error(`API error (${res.status}): ${(data.error || '').slice(0, 300)}`); + if (!res.ok) throw new Error(proxyFailure(res.status, data.error || '', S)); // A truncated answer is not a shorter answer — it stops mid-sentence, and the // JSON repair downstream will happily patch the half-written object into // something that renders as if it were complete. Fail instead. diff --git a/artifacts/api-server/.replit-artifact/artifact.toml b/artifacts/api-server/.replit-artifact/artifact.toml index 46ed04a..9192f8c 100644 --- a/artifacts/api-server/.replit-artifact/artifact.toml +++ b/artifacts/api-server/.replit-artifact/artifact.toml @@ -10,7 +10,7 @@ name = "API Server" paths = ["/api"] [services.development] -run = "uv run --directory ../../server uvicorn app.main:app --host 0.0.0.0 --port 8080 --reload" +run = "uv run --directory server uvicorn app.main:app --host 0.0.0.0 --port 8080 --reload" [services.production] @@ -21,7 +21,7 @@ args = ["uv", "sync", "--project", "server", "--frozen", "--no-dev"] NODE_ENV = "production" [services.production.run] -args = ["uv", "run", "--directory", "../../server", "uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "8080"] +args = ["uv", "run", "--project", "server", "--directory", "server", "uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "8080"] [services.production.run.env] PORT = "8080" diff --git a/server/tests/test_replit_deployment.py b/server/tests/test_replit_deployment.py index e2b92e5..417bade 100644 --- a/server/tests/test_replit_deployment.py +++ b/server/tests/test_replit_deployment.py @@ -1,8 +1,21 @@ +"""The Replit artifact that publishes this API. + +Every command in it resolves from the **workspace root**, not from the artifact +directory. That is not a preference — it is what the platform does, and it was +established the hard way: a version using `../../server` built successfully and +then failed to start, because from the workspace root that path points outside +the repository entirely. The twelve other artifacts in this repo all run +`pnpm --filter @workspace/`, which likewise only resolves from the root. + +So a relative path with `..` in it is the specific mistake this file exists to +catch, and the assertions below name it rather than merely pinning a string. +""" + import tomllib from pathlib import Path -def test_replit_runs_the_api_locally_instead_of_relaying_to_an_absent_backend() -> None: +def _api_service() -> dict: artifact_path = ( Path(__file__).parents[2] / "artifacts" @@ -10,12 +23,44 @@ def test_replit_runs_the_api_locally_instead_of_relaying_to_an_absent_backend() / ".replit-artifact" / "artifact.toml" ) - artifact = tomllib.loads(artifact_path.read_text(encoding="utf-8")) + return tomllib.loads(artifact_path.read_text(encoding="utf-8"))["services"][0] + + +def test_no_command_escapes_the_workspace_root() -> None: + """`../../server` resolves outside the repository and the service never starts.""" + service = _api_service() + commands = { + "development run": service["development"]["run"], + "production build": " ".join(service["production"]["build"]["args"]), + "production run": " ".join(service["production"]["run"]["args"]), + } + + for name, command in commands.items(): + assert ".." not in command, f"the {name} command leaves the workspace root: {command}" - service = artifact["services"][0] + +def test_every_command_points_at_the_server_project_from_the_root() -> None: + service = _api_service() + + assert "--directory server" in service["development"]["run"] + assert service["production"]["build"]["args"][:4] == ["uv", "sync", "--project", "server"] + run_args = service["production"]["run"]["args"] + assert "--directory" in run_args + assert run_args[run_args.index("--directory") + 1] == "server" + + +def test_replit_runs_the_api_locally_instead_of_relaying_to_an_absent_backend() -> None: + service = _api_service() assert service["paths"] == ["/api"] run_args = service["production"]["run"]["args"] - assert run_args[:4] == ["uv", "run", "--directory", "../../server"] - assert run_args[4:6] == ["uvicorn", "app.main:app"] + assert run_args[:2] == ["uv", "run"] + assert run_args[-6:] == [ + "uvicorn", + "app.main:app", + "--host", + "0.0.0.0", + "--port", + "8080", + ] assert service["production"]["health"]["startup"]["path"] == "/api/healthz" assert "UPSTREAM_API_BASE_URL" not in service["production"]["run"]["env"] diff --git a/tests/component/GoogleSignIn.spec.tsx b/tests/component/GoogleSignIn.spec.tsx index 74fd5fa..fceed66 100644 --- a/tests/component/GoogleSignIn.spec.tsx +++ b/tests/component/GoogleSignIn.spec.tsx @@ -305,3 +305,23 @@ test.describe('when the server refuses the credential', () => { await expect(component.locator('.nav-account-name')).toHaveCount(0); }); }); + +test('asks the browser to mediate the dialog rather than opening a popup', async ({ + googleSignIn, +}) => { + /** + * The popup flow navigates to accounts.google.com. On Android that host is a + * verified App Link for the Google app, so Chrome hands the navigation to the + * OS: the reader gets an "Open with application" chooser and comes back not + * signed in. It reproduces in ordinary Chrome and not in Incognito, which is + * what identified it — Incognito does not do the app handoff. + * + * FedCM has the browser draw the dialog itself, so there is no navigation for + * the OS to intercept. Browsers without FedCM fall back to the popup flow, so + * asking for it costs nothing where it is not available. + */ + const component = await googleSignIn.mount(); + + await expect(component.locator('#fakeGoogleButton')).toBeVisible(); + expect((await googleSignIn.stub()).useFedcmForButton).toBe(true); +}); diff --git a/tests/component/fixtures.tsx b/tests/component/fixtures.tsx index 35cd85c..32b7e81 100644 --- a/tests/component/fixtures.tsx +++ b/tests/component/fixtures.tsx @@ -71,6 +71,7 @@ interface GoogleStubWindow extends Window { clientId?: string; locales: string[]; autoSelectDisabled: boolean; + useFedcmForButton?: boolean; }; __credential?: string; } @@ -92,7 +93,12 @@ type GoogleSignInHarness = { /** Starts sign-in and holds the API response until the returned function runs. */ beginSignInWith: (credential: string) => Promise<() => Promise>; /** What the stubbed Google client was told and asked to do. */ - stub: () => Promise<{ clientId?: string; locales: string[]; autoSelectDisabled: boolean }>; + stub: () => Promise<{ + clientId?: string; + locales: string[]; + autoSelectDisabled: boolean; + useFedcmForButton?: boolean; + }>; }; /** @@ -112,6 +118,7 @@ const GOOGLE_STUB = ` initialize(config) { window.__gsi.clientId = config.client_id; window.__gsi.callback = config.callback; + window.__gsi.useFedcmForButton = config.use_fedcm_for_button === true; }, renderButton(parent, options) { window.__gsi.locales.push(options.locale); diff --git a/tests/unit/providers.spec.ts b/tests/unit/providers.spec.ts index 2d56409..80ee49d 100644 --- a/tests/unit/providers.spec.ts +++ b/tests/unit/providers.spec.ts @@ -7,7 +7,7 @@ import { extractJSON, type ServerDefaults, } from '@academy/lib/providers'; -import { en } from '@academy/lib/locales'; +import { en, he } from '@academy/lib/locales'; import { stubFetch, jsonResponse, type FetchStub } from '../support/fetchStub'; /** @@ -509,3 +509,53 @@ test.describe('callGeminiGrounded', () => { expect(fetchStub.calls).toHaveLength(0); }); }); + +test.describe('a failure from the academy’s own proxy', () => { + const serverHasGroq: ServerDefaults = { groq: { available: true } }; + + /** + * The reader meets two of these in practice, and "API error (429): Too many + * AI requests" is not an answer to either — it names a protocol they did not + * know they were speaking, in English, inside a Hebrew page. Both have the + * same way forward, which is to connect a key of their own, so both say so. + */ + test('a spent allowance says what to do, not which status arrived', async () => { + fetchStub = stubFetch(() => jsonResponse({ error: 'Too many AI requests.' }, 429)); + + await expect( + callAI('groq', 'openai/gpt-oss-120b', '', false, serverHasGroq, S, 'sys', [ + { role: 'user', content: 'hi' }, + ]), + ).rejects.toThrow(S.errProxyBusy); + }); + + test('an unconfigured server key says so, rather than blaming usage', async () => { + fetchStub = stubFetch(() => jsonResponse({ error: 'No server-side Groq key' }, 503)); + + await expect( + callAI('groq', 'openai/gpt-oss-120b', '', false, serverHasGroq, S, 'sys', [ + { role: 'user', content: 'hi' }, + ]), + ).rejects.toThrow(S.errProxyUnavailable); + }); + + test('anything else keeps the status and the server’s own text', async () => { + // Worth pasting into a bug report, which the two above deliberately are not. + fetchStub = stubFetch(() => jsonResponse({ error: 'upstream exploded' }, 502)); + + await expect( + callAI('groq', 'openai/gpt-oss-120b', '', false, serverHasGroq, S, 'sys', [ + { role: 'user', content: 'hi' }, + ]), + ).rejects.toThrow(/502.*upstream exploded/); + }); + + test('the prefix is translated, as it is on the direct-call paths', () => { + // It was hardcoded English here while the two BYOK call sites used the + // locale, so a Hebrew reader got "API error (" from one path and + // "שגיאת API (" from the others. + expect(S.errApiPrefix).toBeTruthy(); + expect(en.s.errApiPrefix).toBe('API error ('); + expect(he.s.errApiPrefix).toBe('שגיאת API ('); + }); +});