Fix/academy review findings

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 });
     },

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;

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 ',

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: '⏳ מתכונן לקרוא את ',

artifacts/ai-testing-academy/src/lib/providers.ts

@@ -120,6 +120,22 @@ export async function loadServerConfig(): Promise<ServerDefaults> {
   }
 }
 
+/**
+ * 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.

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"

server/tests/test_replit_deployment.py

@@ -1,21 +1,66 @@
+"""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/<name>`, 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"
         / "api-server"
         / ".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"]

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);
+});

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<void>>;
   /** 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);

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 (');
+  });
+});