From 9b22f373828adc3b8773dec1f782395dbe57cbb3 Mon Sep 17 00:00:00 2001 From: Amiel Peled Date: Thu, 20 Aug 2026 21:39:19 +0300 Subject: [PATCH 1/3] fix(deploy): resolve every artifact command from the workspace root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The published API built successfully and then failed to start. `../../server` resolves from the workspace root, where Replit runs these commands, to a path outside the repository — so uv had nothing to run. The production build had been using `--project server` all along, which is why the build half worked and only the start half did not. The twelve other artifacts in this repo all run `pnpm --filter @workspace/`, which likewise only resolves from the root. This one was the odd one out, and the only one that would not start. test_replit_deployment.py was pinning the broken value, so a sync would have reintroduced the outage on every merge. It now asserts the rule instead of a string: no command may contain `..`, and each must point at the server project from the root. Checked by mutation — putting `../../server` back fails it with the reason rather than a diff of two literals. I had recommended taking the repository's version over the deployment's during the merge conflict, on the grounds that the test was authoritative. It was not: it encoded an assumption no deployment had ever confirmed. 147 pytest pass. Co-Authored-By: Claude Opus 5 (1M context) --- .../api-server/.replit-artifact/artifact.toml | 4 +- server/tests/test_replit_deployment.py | 55 +++++++++++++++++-- 2 files changed, 52 insertions(+), 7 deletions(-) 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"] From 34f9b88b7321b77b7d3130d9961494c28137a2be Mon Sep 17 00:00:00 2001 From: Amiel Peled Date: Thu, 20 Aug 2026 21:51:39 +0300 Subject: [PATCH 2/3] fix(academy): let the browser mediate Google sign-in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Android, tapping the Google button opened an "Open with application" chooser and the reader came back not signed in. The popup flow navigates to accounts.google.com, and that host is a verified Android App Link for the Google app: Chrome hands the navigation to the OS rather than completing it. 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 sign-in dialog itself, so there is no navigation for the OS to intercept. `use_fedcm_for_button` is opt-in and browsers without FedCM fall back to the popup flow unchanged, so asking for it costs nothing where it is not available. None of the methods the FedCM migration requires removing are used here, and the CSP already allows accounts.google.com in both connect-src and frame-src. Two hypotheses were tested and discarded first: an in-app WebView (the report was ordinary Chrome) and a mis-hit in the mobile drawer (measured on the live site — 127px of clear space below the community links, and elementFromPoint at the button's centre returns Google's own iframe). 174 component tests pass. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/context/AuthContext.tsx | 7 +++++++ .../src/lib/googleIdentity.ts | 6 ++++++ tests/component/GoogleSignIn.spec.tsx | 20 +++++++++++++++++++ tests/component/fixtures.tsx | 9 ++++++++- 4 files changed, 41 insertions(+), 1 deletion(-) 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/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); From cbba4df484b33a1ec87b8c51942d2641f438bc72 Mon Sep 17 00:00:00 2001 From: Amiel Peled Date: Thu, 20 Aug 2026 22:05:52 +0300 Subject: [PATCH 3/3] fix(academy): say what a proxy failure means, in the reader's language MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The résumé tool showed "API error (429): Too many AI requests. Please wait before trying again." — an English sentence naming an HTTP status, inside a Hebrew page, with no way forward in it. Two things were wrong on that line. It built the message by hand instead of using S.errApiPrefix, which the two direct-call paths already use, so a Hebrew reader got "API error (" from the proxy and "שגיאת API (" from everywhere else. And it passed the wire's answer straight through for the two statuses a visitor actually meets. 429 and 503 now say what happened and what to do — connect a provider key of your own, which 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 and is exactly what the other two are not. This is not the cause of the 429 on the live site: that is still SharedRateLimiter failing closed for want of RATE_LIMIT_SALT, which /api/readyz reports. It is what the reader sees when a genuine allowance runs out, and with a 10-request anonymous daily quota, they will. 246 unit and 174 component tests pass. Co-Authored-By: Claude Opus 5 (1M context) --- .../ai-testing-academy/src/lib/locales/en.ts | 4 ++ .../ai-testing-academy/src/lib/locales/he.ts | 4 ++ .../ai-testing-academy/src/lib/providers.ts | 18 ++++++- tests/unit/providers.spec.ts | 52 ++++++++++++++++++- 4 files changed, 76 insertions(+), 2 deletions(-) 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/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 ('); + }); +});