Meta-loop: further harden the auth flow + UX beyond PR #47

src/commands/auth.ts

@@ -9,13 +9,19 @@
 import type { AppContext } from "../core/context.js";
 import { cmdLogin, cmdLogout, type LoginOpts } from "./login.js";
 import { MODELS_PATH, REFRESH_PATH } from "../core/transport.js";
+import { HttpError, errorHint, errorMessage } from "../core/errors.js";
+import { isApiKeyToken } from "../core/auth.js";
 import { box, titledBox, hyperlink, orange, green, darkBlue, brightWhite, lightBlue } from "../ui/box.js";
 import { CLOUD } from "../ui/logo.js";
 import { theme } from "../ui/theme.js";
+import { formatErrorLine } from "../ui/error_line.js";
 
-/** A long-lived API token (PAT) starts with `aek_`; otherwise it's a session token. */
+/** A long-lived API token (PAT) starts with `aek_`; otherwise it's a session
+ *  token. Thin wrapper so callers of THIS module keep importing `isApiToken`
+ *  from here — the actual `aek_` prefix check is canonical in core/auth.ts's
+ *  isApiKeyToken (shared with transport.ts's refreshSession). */
 export function isApiToken(token: string | null | undefined): boolean {
-  return typeof token === "string" && token.startsWith("aek_");
+  return isApiKeyToken(token);
 }
 
 function mask(t: string): string {
@@ -35,37 +41,61 @@ function centeredCloud(): string {
 
 // ── Branded status panel ──
 
-async function renderAuthBox(ctx: AppContext): Promise<string> {
+// Exported so tests can render the panel directly against a fake AppContext
+// without going through cmdAuth's stdout write.
+export async function renderAuthBox(ctx: AppContext): Promise<string> {
   const t = await ctx.tokens.get();
   if (!t) return renderLoggedOut();
 
   // Fetch tier info for display
   let tier = "";
   let defaultModel = "";
+  // A 401/403 here means the SERVER actively rejected this token (expired or
+  // revoked session) — that is a different situation from "server
+  // unreachable" and must not be swallowed the same way, or `status` would
+  // claim "Authenticated" for a dead session (the stale-AETHER_TOKEN case PR
+  // #47 fixed elsewhere). Any other failure (no HttpError, or a 5xx) is a
+  // genuine network/server problem: keep the existing silent local-only
+  // fallback for those.
+  let sessionExpired = false;
   try {
     const cat = await ctx.api.getJson<{ tier?: string; default?: string }>(MODELS_PATH);
     tier = cat.tier ?? "";
     defaultModel = cat.default ?? "";
-  } catch {
-    // Server unreachable — still show what we know locally.
+  } catch (err) {
+    if (err instanceof HttpError && (err.status === 401 || err.status === 403)) {
+      sessionExpired = true;
+    }
+    // Otherwise: server unreachable — still show what we know locally.
   }
 
   const kind = isApiToken(t) ? "API key" : "session token";
+  const header = sessionExpired
+    ? theme.yellow("⚠") + "  " + theme.bold("Aether Agent — Session expired")
+    : theme.iceBlue("☁") + "  " + theme.bold("Aether Agent — Authenticated");
   const lines: string[] = [
     "",
-    theme.iceBlue("\u2601") + "  " + theme.bold("Aether Agent \u2014 Authenticated"),
+    header,
     "",
     // No Account row: there is no account endpoint yet, and a hardcoded
-    // "(fetching\u2026)" that never resolves is a fake loading state (PR #47 UX).
+    // "(fetching…)" that never resolves is a fake loading state (PR #47 UX).
     "  " + theme.dim("Token:") + "    " + theme.bold(mask(t)) + "  " + theme.dim(`(${kind})`),
     "  " + theme.dim("API:") + "      " + ctx.cfg.baseUrl.replace("https://", ""),
   ];
 
-  if (tier) {
-    lines.push("  " + theme.dim("Tier:") + "     " + (tier === "free" ? theme.dim(tier) : theme.cyan(tier)));
-  }
-  if (defaultModel) {
-    lines.push("  " + theme.dim("Default:") + "  " + defaultModel);
+  if (sessionExpired) {
+    lines.push(
+      "",
+      "  " + theme.yellow("Server rejected this token — sign in again:"),
+      "  " + theme.bold(theme.cyan("aether auth login")),
+    );
+  } else {
+    if (tier) {
+      lines.push("  " + theme.dim("Tier:") + "     " + (tier === "free" ? theme.dim(tier) : theme.cyan(tier)));
+    }
+    if (defaultModel) {
+      lines.push("  " + theme.dim("Default:") + "  " + defaultModel);
+    }
   }
 
   lines.push(
@@ -78,7 +108,8 @@ async function renderAuthBox(ctx: AppContext): Promise<string> {
     "",
   );
 
-  return [centeredCloud(), "", titledBox(lines, "Authenticated", { width: BOX_W })].join("\n");
+  const title = sessionExpired ? "Session expired" : "Authenticated";
+  return [centeredCloud(), "", titledBox(lines, title, { width: BOX_W })].join("\n");
 }
 
 // ── Logged-out welcome panel ──
@@ -140,6 +171,13 @@ export async function cmdAuth(
     case "logout":
       return cmdLogout(ctx);
     case "status": {
+      // LOOP-06: renderAuthBox's first move is a network round-trip to
+      // /models (no cache to fall back on for the CLI's one-shot status
+      // command), and nothing was written to stdout until the whole thing
+      // resolved — up to DEFAULT_REQUEST_TIMEOUT_MS of silence on a slow
+      // connection, "the REPL just looks hung" (the exact class PR #47 fixed
+      // for slash.ts's catalog fetch). Match that convention here.
+      process.stdout.write(theme.dim("checking session…\n"));
       const panel = await renderAuthBox(ctx);
       process.stdout.write(panel + "\n");
       return 0;
@@ -152,8 +190,11 @@ export async function cmdAuth(
       printAuthHelp();
       return 0;
     default: {
-      // Bare `aether auth` — show the branded panel.
+      // Bare `aether auth` — show the branded panel. Same LOOP-06 loading
+      // line as the "status" branch above — this hits renderAuthBox's same
+      // /models call.
       if (sub === "") {
+        process.stdout.write(theme.dim("checking session…\n"));
         const panel = await renderAuthBox(ctx);
         process.stdout.write("\n" + panel + "\n\n");
         return 0;
@@ -208,10 +249,17 @@ async function authRefresh(ctx: AppContext): Promise<number> {
       process.stdout.write("✓ Session refreshed.\n");
       return 0;
     }
-    process.stderr.write("✗ Refresh failed — try: aether auth login\n");
+    // Same formatErrorLine convention as the catch below — a 200 with no
+    // session_token is still a refresh failure and must read the same as
+    // one caught via a thrown error, not as the one bare ✗ line left in
+    // this function (LOOP-06 round 3).
+    process.stderr.write(formatErrorLine("Refresh failed", { hint: "run `aether auth login`" }));
     return 1;
   } catch (err) {
-    process.stderr.write(`\u2717 ${err instanceof Error ? err.message : String(err)}\n`);
+    // formatErrorLine (LOOP-06) owns the glyph/hint/separator convention,
+    // same as chat.ts's printError, so a failed /auth/refresh reads like
+    // any other auth-adjacent error instead of bare, uncolored text.
+    process.stderr.write(formatErrorLine(errorMessage(err), { hint: errorHint(err, ctx.cfg.baseUrl) }));
     return 1;
   }
 }

src/commands/chat.ts

@@ -7,10 +7,11 @@ import { StringDecoder } from "node:string_decoder";
 import type { AppContext, GlobalFlags } from "../core/context.js";
 import { theme, errTheme } from "../ui/theme.js";
 import { buildChatRequest } from "../core/envelope.js";
-import { CHAT_STREAM_PATH, CHAT_PATH } from "../core/transport.js";
+import { CHAT_STREAM_PATH, CHAT_PATH, defaultStreamTimeoutMs } from "../core/transport.js";
 import { decodeSse } from "../core/stream.js";
 import { Renderer } from "../core/render.js";
-import { StreamUnavailableError, errorHint, isAbortError } from "../core/errors.js";
+import { StreamIncompleteError, StreamUnavailableError, errorHint, isAbortError } from "../core/errors.js";
+import { formatErrorLine } from "../ui/error_line.js";
 import { appendCustody } from "../core/custody.js";
 import { handleSlash, primeCatalog } from "./slash.js";
 import { applyPromptMode } from "./prompt_modes.js";
@@ -88,7 +89,10 @@ export async function resolveBackend(ctx: AppContext): Promise<BackendPath> {
  * `signal` cancels the turn client-side (stream AND the fail-soft fallback) —
  * orchestrator runs inherit cancelability through this same seam. Throws
  * ChatTurnError if the server streamed an `error` frame, so callers can exit
- * non-zero instead of treating a rendered "✗ msg" as a successful turn.
+ * non-zero instead of treating a rendered "✗ msg" as a successful turn. Also
+ * throws StreamIncompleteError if the stream ends without ever sending a
+ * terminal `done` or `error` frame (LOOP-06 round 3) — a clean-looking
+ * premature close must not render as a successful turn either.
  * `onPulsePaint` fires after every thinking-pulse repaint (see
  * ThinkingPulseOptions.onPaint) so the REPL can re-sync its own input-line
  * redraw — typing ahead during the pre-first-token window would otherwise
@@ -143,6 +147,13 @@ async function runCloudTurn(
   });
   pulse.start();
   let sawError: string | null = null;
+  // Whether a terminal `done` or `error` frame was actually observed. decodeSse's
+  // for-await loop exits normally (no throw) once the underlying byte stream
+  // ends — including a premature-but-clean close (proxy/load-balancer time-box,
+  // etc.) that never sent either. Without this, such a close renders whatever
+  // partial output arrived and returns as if the turn completed successfully
+  // (LOOP-06 round 3).
+  let sawTerminal = false;
   try {
     const stream = await ctx.api.stream(CHAT_STREAM_PATH, req, signal);
     for await (const frame of decodeSse(stream)) {
@@ -154,15 +165,21 @@ async function runCloudTurn(
       // locally (best-effort, never breaks the chat).
       if (frame.type === "custody") appendCustody(frame.custody);
       if (frame.type === "error") sawError = frame.msg;
+      if (frame.type === "error" || frame.type === "done") sawTerminal = true;
       onFrame?.(frame);
       renderer.frame(frame);
     }
     if (sawError) throw new ChatTurnError(sawError);
+    if (!sawTerminal) throw new StreamIncompleteError();
   } catch (err) {
     if (err instanceof StreamUnavailableError) {
       // Contract fail-soft: fall back to the non-streaming request/response.
-      // Same signal — the fallback leg is cancelable too (arena AT-3d).
-      const r = await ctx.api.postJson<ChatJsonResponse>(CHAT_PATH, req, signal);
+      // Same signal — the fallback leg is cancelable too (arena AT-3d). A
+      // full LLM turn can legitimately run long, so this explicitly opts
+      // into stream()'s own generous bound instead of request()'s 30s
+      // metadata-call default (LOOP-01/LOOP-06 round-1) — otherwise a
+      // perfectly healthy but slow completion would be killed early.
+      const r = await ctx.api.postJson<ChatJsonResponse>(CHAT_PATH, req, signal, defaultStreamTimeoutMs());
       pulse.stop();
       process.stdout.write((r.response ?? "") + "\n");
       if (ctx.flags.audit && r.commitment_hash) {
@@ -957,11 +974,8 @@ async function replLines(ctx: AppContext): Promise<number> {
 
 function printError(err: unknown, baseUrl: string): void {
   const msg = err instanceof Error ? err.message : String(err);
-  process.stderr.write(`\n${errTheme.red("✗")} ${msg}\n`);
-  const hint = errorHint(err, baseUrl);
-  if (hint) process.stderr.write(errTheme.dim(`  ⤷ ${hint}`) + "\n");
-  // Trailing blank line: the REPL reprints its prompt right after this, and
-  // without the separator the dim hint and the prompt fuse into one line
-  // (the "⤷ run `aether auth login` … [user]_:" mess from PR #47's report).
-  process.stderr.write("\n");
+  // formatErrorLine (LOOP-06) owns the glyph/hint/separator convention so
+  // this reads identically to a server-streamed error frame's Renderer.error
+  // — see src/ui/error_line.ts for why both paths must agree.
+  process.stderr.write(formatErrorLine(msg, { hint: errorHint(err, baseUrl) }));
 }

src/commands/login.ts

@@ -13,6 +13,8 @@ import { LOGOUT_PATH } from "../core/transport.js";
 import { requestDeviceCode, pollForToken } from "../core/device.js";
 import { openBrowser } from "../core/browser.js";
 import { theme } from "../ui/theme.js";
+import { errorHint, errorMessage } from "../core/errors.js";
+import { formatErrorLine } from "../ui/error_line.js";
 
 export interface LoginOpts {
   token?: string;
@@ -75,7 +77,10 @@ export async function cmdLogin(ctx: AppContext, opts: LoginOpts): Promise<number
       warnEnvTokenShadow();
       return 0;
     } catch (err) {
-      process.stderr.write(`✗ ${err instanceof Error ? err.message : String(err)}\n`);
+      // formatErrorLine (LOOP-06) owns the glyph/hint/separator convention —
+      // same as chat.ts's printError — so a headless login failure reads the
+      // same as any other auth-adjacent error instead of bare, uncolored text.
+      process.stderr.write(formatErrorLine(errorMessage(err), { hint: errorHint(err, ctx.cfg.baseUrl) }));
       return 1;
     }
   }
@@ -86,7 +91,7 @@ export async function cmdLogin(ctx: AppContext, opts: LoginOpts): Promise<number
     code = await requestDeviceCode(ctx.api);
   } catch (err) {
     process.stderr.write(
-      `✗ could not start login: ${err instanceof Error ? err.message : String(err)}\n`,
+      formatErrorLine(`could not start login: ${errorMessage(err)}`, { hint: errorHint(err, ctx.cfg.baseUrl) }),
     );
     return 1;
   }
@@ -103,7 +108,7 @@ export async function cmdLogin(ctx: AppContext, opts: LoginOpts): Promise<number
     warnEnvTokenShadow();
     return 0;
   } catch (err) {
-    process.stderr.write(`✗ ${err instanceof Error ? err.message : String(err)}\n`);
+    process.stderr.write(formatErrorLine(errorMessage(err), { hint: errorHint(err, ctx.cfg.baseUrl) }));
     return 1;
   }
 }

src/commands/media.ts

@@ -8,6 +8,7 @@ import type { AppContext } from "../core/context.js";
 import type { CatalogResponse } from "../types.js";
 import { MODELS_PATH } from "../core/transport.js";
 import { fail as coreFail } from "../core/errors.js";
+import { hintFor } from "../core/error_hints.js";
 import {
   filterMediaModels, resolveModelKey, autoRouteModel,
   buildMediaPrompt, dispatchGeneration, downloadMediaFile,
@@ -16,6 +17,7 @@ import {
   type MediaKind, type MediaModel, type GenFlags, type GenResult,
 } from "../core/vision.js";
 import { theme } from "../ui/theme.js";
+import { sanitizeTerm } from "../ui/text.js";
 import { basename } from "node:path";
 import { createInterface, type Interface } from "node:readline";
 
@@ -196,17 +198,29 @@ async function mediaGenerate(ctx: AppContext, prompt: string, kind: MediaKind, f
           url: resp.media_url, timestamp: new Date().toISOString(), flags,
         };
         const entry = recordOutput(result);
+        // resp.media_url and resp.text/response below are server-controlled
+        // (an LLM generation response) — sanitized before hitting the
+        // terminal, same as every other server-supplied string in this flow
+        // (see sanitizeServerText's own doc comment in transport.ts).
         process.stdout.write(
           `  ${theme.iceBlue("↓")} #${entry.index}  ${entry.filename}\n` +
-          `  ${theme.dim(`url: ${resp.media_url}`)}\n\n`
+          `  ${theme.dim(`url: ${sanitizeTerm(resp.media_url)}`)}\n\n`
         );
         if (flags.open) openOutput(entry);
       } else {
         process.stdout.write(theme.dim("  no media URL in response\n"));
-        process.stdout.write(theme.dim(`  ${text.slice(0, 200)}\n`));
+        process.stdout.write(theme.dim(`  ${sanitizeTerm(text.slice(0, 200))}\n`));
       }
     } catch (err) {
-      process.stdout.write(`✗ ${err instanceof Error ? err.message : String(err)}\n`);
+      // LOOP-01 round 2: dispatchGeneration()/downloadMediaFile() already
+      // throw a classified HttpError (401/402/403/5xx) — this used to print
+      // only the raw message with no actionable next step at all. Sanitized:
+      // a raw fetch()-level failure on a hostile media_url can otherwise
+      // carry attacker-controlled bytes straight from err.message.
+      const msg = err instanceof Error ? err.message : String(err);
+      process.stdout.write(`✗ ${sanitizeTerm(msg)}\n`);
+      const hint = hintFor(err);
+      if (hint) process.stdout.write(theme.dim(`  ⤷ ${hint}\n`));
     }
   }
   process.stdout.write(theme.dim(`\noutput: ${outdir}/\nview: aether output\n`));
@@ -230,6 +244,7 @@ function printMediaHelp(kind: MediaKind): void {
   ].join("\n"));
 }
 
+// See vault.ts's identical fix for why hintFor() replaces the hardcoded hint.
 function fail(err: unknown): number {
-  return coreFail(err, "are you logged in? run: aether auth login");
+  return coreFail(err, hintFor(err) ?? undefined);
 }

src/commands/slash.ts

@@ -374,6 +374,12 @@ async function showPicker(
   const items = byKind(cat, kind);
 
   const picked = await pickModel(items, out);
+  if (picked === undefined) {
+    // pickModel hit an internal fault and already printed its own distinct
+    // diagnostic — printing the generic "kept current session." below too
+    // would show the same failure as two back-to-back, redundant lines.
+    return null;
+  }
   if (!picked) {
     // pickModel returned null — either cancelled (Esc) or non-TTY fallback.
     // If non-TTY, render a flat numbered list so the user can still /model <n>.
@@ -420,15 +426,19 @@ async function select(
 
 /** Shared by showPicker/select once a target item is resolved: lock check,
  * restart warning, and the y/N gate that produces the caller's restart signal. */
-async function confirmSwitch(
+export async function confirmSwitch(
   ctx: AppContext,
   out: Writable,
   item: CatalogItem,
   kind: Kind,
   tier: string,
 ): Promise<{ model?: string; agent?: string } | null> {
   if (!item.available) {
-    out.write(`${item.id} is locked on tier ${tier}\n`);
+    // Same dim styling + "check: /tier or `aether models`" pointer as
+    // httpStatusHint(403) (errors.ts) — a tier lock reached via the picker
+    // must read the same as the functionally identical 403 reached over the
+    // wire, not as unstyled text with no next step. LOOP-06.
+    out.write(theme.dim(`${item.id} is locked on tier ${tier} — check: /tier or \`aether models\`\n`));
     return null;
   }
   out.write(