fix(mobile): the user's settings were stored, displayed, and ignored

Two defects found by an adversarial audit. Together they formed a closed
loop where every step reported success and the user could not escape it.

SETTINGS WERE NEVER USED. main.ts built the engine list from a
`facadeStub()` whose get() returned undefined, so every setting fell
through to a hardcoded default -- while the settings screen read the
real store and displayed the user's saved value back to them. The stub's
own comment said "Replaced by the real one immediately after". It was
not: the engine is constructed once, inside createApp, from that stub.
engineId was a string literal at the same call site, so switching
engines in settings changed nothing either.

Measured before the fix: a saved baseUrl of https://my-engine.example
read back correctly from settings, and the request went to
http://127.0.0.1:8765. An API key typed into the keychain was stored and
never sent.

Fixed structurally rather than by passing the right argument:
`AppDeps.engines` now takes the settings facade as a parameter, so there
is no way to build the engine list without the real one -- the same
shape as the persistence fix, and for the same reason. `engineId` comes
from settings, and `fallbackEngineId` is named for what it is: the value
used when settings name none, on a first launch.

facadeStub is deleted rather than left as a trap.

A FAILURE BEFORE `start` WAS REPORTED AS "no output". The remote engine
synthesises error{kind:"auth"} for a 401 precisely so the UI can offer
the settings link. The reducer dropped it as `before_start`, leaving no
outcome and no text, so finish() substituted error{kind:"empty"} -- "the
engine produced no output" -- which recoveryFor maps to Retry.

So the loop: key never sent -> 401 -> "the engine produced no output"
-> a Retry button that fails identically forever, and never the one link
that could fix it.

A terminal event before `start` now settles the turn. Non-terminal
events are still dropped, because a stray delta from a previous run must
not open a turn that never started.

837 tests, boundaries clean.

src/praisonai-mobile/app/src/boot.test.ts

@@ -39,9 +39,9 @@ function deps(over: Partial<AppDeps> = {}): AppDeps {
     secrets: createFakeSecrets(),
     time: createFakeTime(),
     shell: createFakeShell(),
-    engines: () => [{ id: "scripted", create: () => engine }],
+    engines: (_p, _s) => [{ id: "scripted", create: () => engine }],
     settingDefs: DEFS,
-    engineId: "scripted",
+    fallbackEngineId: "scripted",
     onPublish: () => {},
     now: () => 1000,
     newChatId: () => "c1",
@@ -63,7 +63,16 @@ test("a good boot returns an app with every collaborator wired", async () => {
 
 test("an unknown engine id fails the boot with the available ones named", async () => {
   // The user lands on a settings screen that says which and why, not a crash.
-  const result = await createApp(deps({ engineId: "nope" }));
+  //
+  // Driven through SETTINGS rather than the fallback, because that is the
+  // realistic route: a stored engineId naming an engine a later build removed.
+  // The fallback is only consulted when settings name nothing.
+  const storage = createFakeStorage();
+  await storage.write(
+    { namespace: "settings", id: "app" },
+    JSON.stringify({ engineId: "nope" }),
+  );
+  const result = await createApp(deps({ storage }));
   assert.equal(result.ok, false);
   if (result.ok) return;
   assert.equal(result.reason, "unknown_engine");
@@ -75,7 +84,7 @@ test("an engine too old to hold to the contract stops the boot", async () => {
   // halfway through the first answer leaves text on screen and no honest way
   // to explain it.
   const engine = engineWith({ id: "scripted", protocolVersion: MIN_ENGINE_PROTOCOL - 1 });
-  const result = await createApp(deps({ engines: () => [{ id: "scripted", create: () => engine }] }));
+  const result = await createApp(deps({ engines: (_p, _s) => [{ id: "scripted", create: () => engine }] }));
 
   assert.equal(result.ok, false);
   if (result.ok) return;
@@ -88,7 +97,7 @@ test("an engine that cannot state its protocol is refused, not assumed current",
   // what it speaks is not one that can be held to a contract.
   const engine = engineWith({ id: "scripted" });
   const broken = { ...engine, protocolVersion: undefined as unknown as number };
-  const result = await createApp(deps({ engines: () => [{ id: "scripted", create: () => broken }] }));
+  const result = await createApp(deps({ engines: (_p, _s) => [{ id: "scripted", create: () => broken }] }));
   assert.equal(result.ok, false);
 });
 
@@ -98,7 +107,7 @@ test("a NEWER engine is accepted, because unknown events degrade rather than bre
   // decode.ts drops an unrecognised event and keeps the answer rendering, so a
   // v3 client against a v4 engine loses an affordance, not the conversation.
   const engine = engineWith({ id: "scripted", protocolVersion: PROTOCOL_VERSION + 1 });
-  const result = await createApp(deps({ engines: () => [{ id: "scripted", create: () => engine }] }));
+  const result = await createApp(deps({ engines: (_p, _s) => [{ id: "scripted", create: () => engine }] }));
   assert.equal(result.ok, true);
   if (result.ok) await result.app.dispose();
 });
@@ -114,7 +123,7 @@ test("an engine rejected for its protocol is still disposed", async () => {
     protocolVersion: MIN_ENGINE_PROTOCOL - 1,
     dispose: async () => void (disposed = true),
   };
-  await createApp(deps({ engines: () => [{ id: "scripted", create: () => engine }] }));
+  await createApp(deps({ engines: (_p, _s) => [{ id: "scripted", create: () => engine }] }));
   assert.equal(disposed, true);
 });
 
@@ -123,7 +132,7 @@ test("a factory whose engine reports a different id is refused", async () => {
   // another writes transcripts attributing themselves to an engine the user
   // never selected.
   const engine = engineWith({ id: "something-else" });
-  const result = await createApp(deps({ engines: () => [{ id: "scripted", create: () => engine }] }));
+  const result = await createApp(deps({ engines: (_p, _s) => [{ id: "scripted", create: () => engine }] }));
   assert.equal(result.ok, false);
   if (result.ok) return;
   assert.match(result.detail, /something-else/);
@@ -139,7 +148,7 @@ test("settings are loaded before the engine is built", async () => {
   const engine = engineWith({ id: "scripted" });
   const result = await createApp(deps({
     storage,
-    engines: () => [{ id: "scripted", create: () => { order.push("engine"); return engine; } }],
+    engines: (_p, _s) => [{ id: "scripted", create: () => { order.push("engine"); return engine; } }],
   }));
 
   assert.equal(result.ok, true);
@@ -159,7 +168,7 @@ test("backgrounding stops the run", async () => {
     cancel: async () => { stopped = true; return true; },
   };
 
-  const result = await createApp(deps({ shell, engines: () => [{ id: "scripted", create: () => engine }] }));
+  const result = await createApp(deps({ shell, engines: (_p, _s) => [{ id: "scripted", create: () => engine }] }));
   assert.equal(result.ok, true);
   if (!result.ok) return;
 
@@ -275,3 +284,70 @@ test("the engine list cannot be built before the session exists", () => {
   factory({ record: async () => null });
   assert.deepEqual(built, ["has-persistence"]);
 });
+
+// ---- settings must actually reach the engine --------------------------------
+
+test("the engine is built from the SAVED settings, not from defaults", async () => {
+  // The defect this closes: `engines` was built from a stub whose get()
+  // returned undefined, so every setting fell through to a hardcoded default
+  // while the settings screen displayed the user's saved value back to them.
+  // A base URL typed by the user was stored, shown, and never used.
+  const storage = createFakeStorage();
+  await storage.write(
+    { namespace: "settings", id: "app" },
+    JSON.stringify({ baseUrl: "https://saved.example", engineId: "scripted" }),
+  );
+
+  let seen: string | undefined;
+  const result = await createApp(deps({
+    storage,
+    settingDefs: [{ key: "engineId", default: "scripted" }, { key: "baseUrl", default: "http://unused" }],
+    engines: (_persistence, settings) => {
+      seen = settings.get("baseUrl") as string | undefined;
+      return [{ id: "scripted", create: () => engineWith({ id: "scripted" }) }];
+    },
+  }));
+
+  assert.equal(result.ok, true);
+  assert.equal(seen, "https://saved.example", "the engine factory got the stub's default, not the saved value");
+  if (result.ok) await result.app.dispose();
+});
+
+test("the engine the user CHOSE is the one selected", async () => {
+  // Same defect, the other half: engineId was a string literal at the call
+  // site, so a user switching engines in settings changed nothing.
+  const storage = createFakeStorage();
+  await storage.write(
+    { namespace: "settings", id: "app" },
+    JSON.stringify({ engineId: "chosen" }),
+  );
+
+  const result = await createApp(deps({
+    storage,
+    settingDefs: [{ key: "engineId", default: "fallback" }],
+    fallbackEngineId: "fallback",
+    engines: () => [
+      { id: "fallback", create: () => engineWith({ id: "fallback" }) },
+      { id: "chosen", create: () => engineWith({ id: "chosen" }) },
+    ],
+  }));
+
+  assert.equal(result.ok, true);
+  if (!result.ok) return;
+  assert.equal(result.app.engine.id, "chosen");
+  await result.app.dispose();
+});
+
+test("the fallback is used only when settings name no engine", async () => {
+  // The pair: always using the fallback would satisfy nothing above, and
+  // always using settings would break a first launch, where there are none.
+  const result = await createApp(deps({
+    settingDefs: [{ key: "engineId", default: "" }],
+    fallbackEngineId: "scripted",
+    engines: () => [{ id: "scripted", create: () => engineWith({ id: "scripted" }) }],
+  }));
+  assert.equal(result.ok, true);
+  if (!result.ok) return;
+  assert.equal(result.app.engine.id, "scripted");
+  await result.app.dispose();
+});

src/praisonai-mobile/app/src/boot.ts

@@ -44,10 +44,21 @@ export interface AppDeps {
    * factory makes that impossible to express: there is no way to obtain the
    * engine list without being handed the thing engines write through.
    */
-  readonly engines: (persistence: RunPersistence) => readonly EngineChoice[];
+  readonly engines: (
+    persistence: RunPersistence,
+    settings: SettingsFacade,
+  ) => readonly EngineChoice[];
   readonly settingDefs: readonly SettingDef[];
   /** Which engine to start with, normally read from settings. */
-  readonly engineId: string;
+  /**
+   * The engine to use when settings do not name one -- a first launch, or a
+   * settings file that predates the key.
+   *
+   * NOT the engine to always use. It was exactly that, and `main.ts` passed a
+   * string literal, so the engine the user selected in settings was stored,
+   * displayed back to them, and ignored on every run.
+   */
+  readonly fallbackEngineId: string;
   readonly onPublish: (view: RunView) => void;
   /** Injected so boot is deterministic under test. */
   readonly now: () => number;
@@ -80,7 +91,7 @@ export async function createApp(deps: AppDeps): Promise<BootResult> {
   // `end.userIndex` is produced by whatever actually did the write.
   const session = createSession({
     storage: deps.storage,
-    engineId: deps.engineId,
+    engineId: deps.fallbackEngineId,
     now: deps.now,
     newChatId: deps.newChatId,
   });
@@ -93,7 +104,18 @@ export async function createApp(deps: AppDeps): Promise<BootResult> {
   //    engine writes THROUGH it. `main.ts` builds the engine list from
   //    `enginesFor({ ..., persistence: session })`, so a turn recorded by the
   //    engine and a chat read by the UI are the same store.
-  const selection = await selectEngine(deps.engineId, deps.engines(persistenceFor(session)));
+  // The REAL facade, not a stub. The engine list is built from the settings
+  // the user actually saved -- the base URL, the credentials, the choice of
+  // engine. Building it from a stub meant every one of those was read from a
+  // hardcoded default while the settings screen displayed the user's value.
+  const settings = facadeFor(settingsStore, deps.secrets);
+
+  // And the selection comes from settings too, falling back only when they do
+  // not name one.
+  const chosen = settings.get("engineId");
+  const engineId = typeof chosen === "string" && chosen !== "" ? chosen : deps.fallbackEngineId;
+
+  const selection = await selectEngine(engineId, deps.engines(persistenceFor(session), settings));
   if (!selection.ok) {
     return { ok: false, reason: selection.reason, detail: selection.detail };
   }
@@ -129,7 +151,7 @@ export async function createApp(deps: AppDeps): Promise<BootResult> {
       engine,
       controller,
       session,
-      settings: facadeFor(settingsStore, deps.secrets),
+      settings,
       router,
       shell: deps.shell,
 

src/praisonai-mobile/app/src/main.ts

@@ -15,7 +15,7 @@
  */
 import { createApp, type App } from "./boot.ts";
 import { detectPlatform, type Platform } from "./platform.ts";
-import { enginesFor, OPENAI_KEY, SETTING_DEFS } from "./registry.ts";
+import { ENGINE_REMOTE_HTTP, enginesFor, OPENAI_KEY, SETTING_DEFS } from "./registry.ts";
 import { intentFrom, type Actionable, type Intent } from "./intents.ts";
 import { applyOps, emptyNodes, type RowNodes } from "./dom.ts";
 import { installCrashHandler } from "./crash.ts";
@@ -154,9 +154,14 @@ export async function mount(deps: MountDeps): Promise<App | null> {
     shell: platform.shell,
     // The factory receives the session boot just built, so the engine that
     // records a turn and the repository the UI reads are the same store.
-    engines: (persistence) => enginesFor({ settings: facadeStub(), http: platform.http, persistence }),
+    // The facade comes FROM boot, built on the loaded settings store. It used
+    // to be a stub whose get() returned undefined, so every setting fell
+    // through to a hardcoded default while the settings screen displayed the
+    // user's saved value back to them.
+    engines: (persistence, settings) => enginesFor({ settings, http: platform.http, persistence }),
     settingDefs: SETTING_DEFS,
-    engineId: "remote-http",
+    // Only when settings do not name one. Boot reads the user's choice.
+    fallbackEngineId: ENGINE_REMOTE_HTTP,
     onPublish: publish,
     now: deps.now ?? (() => Date.now()),
     newChatId: deps.newChatId ?? (() => globalThis.crypto.randomUUID()),
@@ -245,20 +250,6 @@ export async function mount(deps: MountDeps): Promise<App | null> {
   return app;
 }
 
-/** A settings facade before settings exist, used only to build the engine
- *  list at boot. Replaced by the real one immediately after. */
-function facadeStub() {
-  return {
-    get: () => undefined,
-    set: async () => false,
-    defs: () => SETTING_DEFS,
-    subscribe: () => () => {},
-    hasSecret: async () => false,
-    setSecret: async () => {},
-    clearSecret: async () => {},
-    secretsAreHardwareBacked: false,
-  };
-}
 
 // Auto-mount when loaded as a page, never when imported by a test.
 const el = globalThis.document?.getElementById("root");

src/praisonai-mobile/core/src/run/transcript.test.ts

@@ -344,3 +344,46 @@ test("an approval survives an ERROR ending too, not just a cancellation", () =>
   assert.equal(s.approvals.length, 1);
   assert.equal(s.approvals[0]?.resolved, true);
 });
+
+// ---- a failure before the turn began ---------------------------------------
+
+test("an auth error before start keeps its kind, so the UI can offer settings", () => {
+  // The worst outcome in the package, before this: the remote engine
+  // synthesises error{kind:"auth"} for a 401 precisely so the UI can link to
+  // settings -- and the reducer dropped it as `before_start`, leaving no
+  // outcome and no text, so finish() substituted error{kind:"empty"}. That
+  // maps to Retry. A wrong API key produced a Retry button that would fail
+  // identically forever, and never the one link that could fix it.
+  const s = apply(initialTurn, {
+    type: "error", msgId: M, kind: "auth", message: "401 unauthorized",
+  } as RunEvent);
+
+  assert.equal(s.phase, "ended");
+  assert.equal(s.outcome?.type, "error");
+  assert.equal(s.outcome?.type === "error" && s.outcome.kind, "auth", "the kind must survive");
+  assert.deepEqual(s.dropped, [], "and it must not be recorded as dropped");
+});
+
+test("finish() does not overwrite a pre-start error with 'empty'", () => {
+  // The second half of the same bug: even settled, an empty-text turn used to
+  // be rewritten to error{empty} by finish().
+  const s = finish(apply(initialTurn, {
+    type: "error", msgId: M, kind: "rate_limit", message: "429",
+  } as RunEvent));
+  assert.equal(s.outcome?.type === "error" && s.outcome.kind, "rate_limit");
+});
+
+test("a cancellation before start is still a cancellation", () => {
+  // Not an error. A user who pressed Stop before the first token should not be
+  // shown a failure they did not cause.
+  const s = apply(initialTurn, { type: "cancelled", msgId: M, runId: "r1" } as RunEvent);
+  assert.equal(s.outcome?.type, "cancelled");
+});
+
+test("a NON-terminal event before start is still dropped", () => {
+  // The pair. Accepting everything early would let a stray delta from a
+  // previous run open a turn that never started.
+  const s = apply(initialTurn, { type: "delta", msgId: M, text: "stray" } as RunEvent);
+  assert.equal(s.phase, "idle");
+  assert.equal(s.dropped[0]?.reason, "before_start");
+});

src/praisonai-mobile/core/src/run/transcript.ts

@@ -142,8 +142,31 @@ export function apply(state: TurnState, event: RunEvent): TurnState {
   }
 
   if (state.phase === "idle") {
-    // engine/server.py emits `start` first, always. Anything before it is
-    // either a bug or a frame from a previous run arriving late.
+    // A TERMINAL event before `start` is the engine failing before it could
+    // begin, and it carries the only diagnosis the user will ever get. It is
+    // settled rather than dropped.
+    //
+    // Dropping it produced the worst outcome in the package: the remote engine
+    // synthesises `error{kind:"auth"}` for a 401 -- exactly so the UI can offer
+    // the settings screen -- and the reducer discarded it here, leaving no
+    // outcome and no text, so `finish()` substituted `error{kind:"empty"}`
+    // ("the engine produced no output"). `recoveryFor` maps that to Retry. So a
+    // wrong API key produced a Retry button that would fail identically
+    // forever, and never the one link that could fix it.
+    if (event.type === "error") {
+      return settle({ ...state, msgId: event.msgId }, {
+        type: "error",
+        kind: event.kind,
+        message: event.message,
+      });
+    }
+    if (event.type === "cancelled") {
+      return settle({ ...state, msgId: event.msgId, runId: event.runId }, { type: "cancelled" });
+    }
+
+    // Anything else before `start` is a stray: engine/server.py emits `start`
+    // first, always, so a non-terminal event here is a bug or a frame from a
+    // previous run arriving late.
     return drop(state, "before_start", event.type);
   }