Skip to content

Commit e07b115

Browse files
dbarranteclaude
andauthored
Meta-loop: further harden the auth flow + UX beyond PR #47 (#53)
* loop(LOOP-01): route vault/vision auth through ApiClient, not raw fetch() downloadSpacesFile/uploadFile/deleteSpacesFile (vault.ts) and downloadMediaFile (vision.ts) issued their own raw fetch() calls via private-member casts into ApiClient's tokens/baseUrl/authHeaders, so they never got PR #47's refresh-on-401 retry or its distinct 401/402/403 hint UX (they threw plain Error, not HttpError, so errorHint()/hintFor() could never classify them). Add two public authed methods to ApiClient — getBinary() (streaming GET, accepts a relative path or an absolute media URL) and postForm() (authed multipart POST) — both going through the same refresh-on-401 retry and toHttpError() classification as request()/stream(). deleteSpacesFile now uses the existing deleteJson(). Delete vault.ts's _bearerToken/_baseUrl/ _authHeaders and vision.ts's authHeaders() cast entirely — no file outside transport.ts needs to know ApiClient's internal field names anymore. Also fold downloadSpacesFile's res.arrayBuffer() into downloadFile() and stream the response body straight to disk instead of buffering the whole file in memory (mirrors vision.ts's existing pipeline() pattern). Add test/vault_vision_401.test.ts mirroring auth_401.test.ts's fetch-stubbing pattern: 401-then-refresh-then-retry for all four functions, HttpError classification + hintFor 401 wording for deleteSpacesFile, and a regression check that downloadMediaFile still fails closed on an insecure (non-loopback http) media host. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * loop(LOOP-01,LOOP-06): stop masking device-poll network outages as authorization_pending pollForToken's catch block folded ANY thrown error into authorization_pending, including raw network failures (ECONNREFUSED, DNS, timeout) with no HTTP body — an outage during `aether auth login` read as silent normal polling for the whole expires_in window. Now only an HttpError carrying a real body (the documented 400 pending/slow_down/expired case) maps to ordinary poll state. Any other error counts as a network failure; after 3 in a row it prints a distinct "can't reach the server" warning, and if the deadline is hit while still failing it throws a distinct timeout message instead of the generic "login timed out — run `aether auth login` again" (which reads as "too slow"). Adds device.test.ts coverage for: a transient blip that still succeeds with no warning, a sustained outage that fails with the distinct message, and genuine authorization_pending polling to deadline with the original message. * loop(LOOP-06): distinguish a rejected session from a network outage in `aether auth status` renderAuthBox's /models probe folded a 401/403 (server actively rejected the stored token — expired or revoked session, the stale-AETHER_TOKEN case PR #47 fixed elsewhere) into the same bare catch used for a genuine network outage. Either way the panel printed "Authenticated" with Token/API rows, telling the user they're signed in when the server just said otherwise — `status` had no error state at all for this case. Now an HttpError with status 401 or 403 renders a distinct "Session expired" panel (swapped header/icon, a "sign in again" hint instead of Tier/Default), title included. Any other failure (no HttpError, or a different status, including 5xx) keeps the original silent local-only fallback — an outage or a plan-only 429/etc. is not proof the session itself is dead. Deliberately unchanged: cmdAuth's `status` exit code stays 0 in the expired case (the finding asked for a distinct rendered state, not an exit-code contract — changing that would need renderAuthBox to leak expired-ness out to its caller, which is out of scope here). Extends test/auth_401.test.ts (the repo's fetch-stubbing pattern) with four renderAuthBox cases: 401 -> Session expired, 403 -> Session expired, a real network throw -> still Authenticated (no regression), and a normal 200 -> Authenticated with tier/default. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * loop(LOOP-01,LOOP-06): bound ApiClient.request() by default, throw on malformed 2xx bodies Root fix (transport.ts): ApiClient.request() (backing getJson/postJson/ deleteJson) had no timeout at all, unlike stream()'s 120s default — a silently-dropped connection to /models, /auth/refresh, or the device-poll endpoint just hung forever. Now bounded by AETHER_REQUEST_TIMEOUT_MS (default 30s, mirrors AETHER_STREAM_TIMEOUT_MS; 0 disables it), using the same raceAgainst/net-controller pattern stream() already uses so a timeout releases the socket without ever being confused with a caller's own abort. Also fixed a latent bug this surfaced: raceAgainst's early-return for an already-aborted signal discarded its `promise` argument without a catch, which could leak an unhandled rejection — now swallowed, closing the same gap in stream()'s pre-existing use of raceAgainst too. Also: a 2xx response whose body fails to parse as JSON used to silently become `undefined as T`, so callers that trust T (slash.ts's getCatalog -> `cat.models`, auth.ts's authRefresh -> `r.session_token`) crashed one layer up with a raw property-access TypeError. request()/postForm() now throw a new MalformedResponseError (extends HttpError) instead, wired into errorHint()/hintFor() for a normal, hinted message. A genuinely EMPTY body (204 No Content, etc.) is unaffected — classified by reading the text first, not by sniffing JSON.parse's error message, so a body truncated mid-object by a dropped connection is correctly treated as malformed, not empty. loginWithPassword() (the headless --username/--password flow, a raw fetch with no ApiClient behind it) gets its own bounded default via the same env var, guarded so an explicit 0 doesn't become an instant AbortSignal.timeout(0). Regression-prevention carve-outs (required by the new default, not scope creep): a flat 30s would kill any non-streaming call that legitimately runs long. postJson/getJson/deleteJson gained an optional per-call timeoutMs override for exactly this. Applied it to: chat.ts's/brain_cloud.ts's/ client.ts's CHAT_PATH non-streaming chat fallback (a full LLM turn), and bench.ts/test_drive.ts/workflow.ts's project-conversion calls (block server-side on completed generation/iteration work, not a job handle) — each now opts into stream()'s own defaultStreamTimeoutMs() instead. Left orchestrator.ts's delegate/broadcast/gather and audit.ts's exportProof at the 30s default — those return a job handle or a fast confirmation, not completed heavy work. Tests: test/transport_request.test.ts (new — default timeout, per-call override in both directions, malformed-vs-empty-vs-truncated body classification, errorHint/hintFor wiring), test/long_running_timeout_overrides.test.ts (new — bench/test_drive/workflow survive a slow-but-healthy response), test/chat.test.ts (fallback survives past the metadata default), test/device.test.ts (a single stalled poll request times out instead of blocking pollForToken's loop), test/auth.test.ts (loginWithPassword times out / 0 disables it), test/abort.test.ts + test/slash_abort.test.ts (updated for request()'s new net-controller wiring — signal identity is no longer preserved end-to-end, same tradeoff stream() already made; behavioral equivalence asserted instead). Note for follow-up: startTestDrive is bounded at the 120s stream default, but it's an iterate-until-green loop that could plausibly run longer in practice; prior behavior was fully unbounded. Revisit if real TDD loops exceed 120s. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * loop(LOOP-01): unify injected-token TokenStore selection between client.ts and auth.ts AetherClient's constructor and tokenStoreFromEnv() each hand-maintained their own copy of the "injected AETHER_TOKEN -> TokenStore" decision, untested against each other and free to silently diverge. Factor the shared skeleton into auth.ts's new tokenStoreForInjected(injected, {persistOnLogin}), with the one axis that legitimately differs (whether an explicit login persists to disk) passed explicitly at each call site: tokenStoreFromEnv keeps persistOnLogin:true (PR #47's CLI-entry fix), AetherClient keeps persistOnLogin:false (a library embed's login() must not clobber the standalone CLI's on-disk session). Add regression tests pinning both directions so the two paths can't drift again. * loop(LOOP-06): unify printError and Renderer.error's terminal formatting Extract a shared formatErrorLine helper (src/ui/error_line.ts) so a client-caught error (chat.ts's printError) and a mid-stream server-emitted error frame (render.ts's Renderer.error) render the identical "✗ msg" glyph, dim " ⤷ hint" line, and trailing blank-line separator — the same "session expired" moment no longer reads differently depending on which path caught it. * loop(LOOP-06): document intentional 5xx/network hint divergence, dedupe isAbortError error_hints.ts's header comment implied hintFor and errors.errorHint were unified beyond 401/402/403/429 (the only statuses httpStatusHint actually owns). Their 5xx and network-failure wording was always intentionally separate per-surface (errorHint knows the baseUrl; hintFor's public signature doesn't), so make that explicit in comments instead of leaving it looking like an unfixed bug. Also stop maintaining a second, narrower isAbortError in error_hints.ts (only checked err.name, unlike errors.isAbortError's cause-wrapped/regex checks) — it was never imported anywhere in src/, only exercised by its own test. Re-export errors.isAbortError instead so there's one implementation to keep correct. * loop(LOOP-01): gate getBinary's bearer on same-origin, bound getBinary/postForm by timeout, dedupe aek_ prefix check - transport.ts: isCredentialSafeUrl() only checked scheme (any https host passes), so getBinary() attached the live session bearer to ANY absolute https URL a caller passed -- including a third-party media host completely different from the configured Aether API baseUrl. Gate attachment on the new isSameOrigin(target, baseUrl) instead; a cross-origin target is now fetched unauthenticated rather than leaking the token or failing the whole download closed (deliberate, security-relevant behavior change from the prior fail-closed InsecureTransportError on non-loopback http targets -- see the updated tests in vault_vision_401.test.ts for the new contract). - transport.ts: getBinary()/postForm() had no timeout/AbortSignal.timeout bound at all, unlike request()/stream(), reintroducing the "stalled connection hangs forever" bug a7b3621 fixed for request(). Both now accept a timeoutMs override (default AETHER_REQUEST_TIMEOUT_MS), mirroring postJson/getJson/deleteJson. getBinary additionally wraps its body with an idle/quiet-period timeout (stream()'s withIdleTimeout, re-wrapped as a Web ReadableStream) so a large-but-healthy download isn't killed by a flat overall cap -- only a genuinely quiet connection times out. - core/auth.ts: added the canonical isApiKeyToken() export; transport.ts's refreshSession and commands/auth.ts's isApiToken both now delegate to it instead of separately hand-checking the "aek_" prefix. Regression tests: test/transport_getbinary_postform.test.ts (new -- same- origin gating, cross-origin-no-auth-header, connect timeout, body idle timeout, large-healthy-download-not-killed, caller-abort-still-wins, postForm timeout); test/vault_vision_401.test.ts (updated for the new same-origin contract, cross-origin http/https no-credential-sent cases, and same-origin-insecure-baseUrl still fails closed); test/auth.test.ts (direct isApiKeyToken coverage, not just transitively through isApiToken). npm run build && npm test: 753/753 passing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * loop(LOOP-06): add StreamTimeoutError branch to errorHint — REPL now shows the retry/doctor hint for mid-turn stream timeouts, matching hintFor * loop(LOOP-06): sanitize+cap server-supplied login text before it hits the terminal loginWithPassword built its thrown Error directly from the server's raw `reason` field, bypassing the control-char-strip-and-cap step toHttpError applies to every other server-text path — login.ts's headless catch writes that message straight to stderr with no sanitization of its own, so a compromised/misconfigured backend could inject terminal escape sequences (including OSC 52 clipboard-hijack payloads) via `aether auth login --username/--password`. The success path had the same gap: `plan`/ `commitment_hash` reached login.ts's stdout write unsanitized too. Extract toHttpError's strip-and-cap regex into a shared sanitizeServerText() in transport.ts and apply it to reason/plan/commitment_hash in loginWithPassword, so any server-supplied text loginWithPassword surfaces gets the same treatment as every other HTTP-error path. Adds regression tests in test/auth.test.ts (loginWithPassword-level) and a new test/login.test.ts (full cmdLogin path, capturing process.stderr/stdout) asserting a malicious reason/plan cannot survive into the terminal. * loop(LOOP-01): route vault/media command failures through hintFor() — no more hardcoded "are you logged in?" vault.ts's fail() and media.ts's fail() printed the SAME "are you logged in?" hint for every failure, even though downloadFile/uploadFile/ deleteSpacesFile/dispatchGeneration etc. already throw a properly-classified HttpError (401/402/403/5xx). media.ts's mediaGenerate() and all 9 catch blocks in slash_media.ts printed only the raw err.message with no hint at all. All of these now route through core/error_hints.ts's hintFor(), the same fix already applied to workflow.ts's workflowNew, so a vault or media 402/403/5xx/network-outage shows the correct actionable hint instead of a misleading or missing one. * loop(LOOP-01): stream vault uploads via fs.openAsBlob() instead of readFileSync+Blob double-buffering uploadFile() read the entire local file into a Buffer (fs.readFileSync) and then copied it again into an in-memory Blob([data]) before any bytes went over the wire — an unaddressed resource-consumption gap on the upload side, even though downloadFile() was fixed to stream straight to disk in 1d33357. Now hands FormData an fs.openAsBlob() Blob backed by the open file handle itself, so a large vault upload no longer holds the whole file in memory twice. Zero-dependency, matches this repo's no-deps convention. Deliberately skipped the finding's "at minimum, add a file-size guard" fallback clause — that was only meant to apply if true streaming wasn't feasible; since openAsBlob() removes the buffering entirely, an arbitrary size cap would just reject legitimate large uploads for no remaining safety reason. Added a regression test that proves the fix bites: since fs.openAsBlob() blobs re-validate the file on disk at read time (and a Buffer-backed Blob copy does not), deleting the source file mid-upload now causes the Blob read to fail — verified this test fails against the pre-fix code and passes against the fix. * loop(LOOP-06): print a loading line before aether auth status's /models call renderAuthBox's first move is a network round-trip to /models with no cache to fall back on, and cmdAuth wrote nothing to stdout until the whole call resolved -- up to DEFAULT_REQUEST_TIMEOUT_MS (30s) of silence on a slow connection, "the REPL just looks hung" (the exact class PR #47 fixed for slash.ts's catalog fetch). Print theme.dim("checking session...") before awaiting renderAuthBox in both the "status" subcommand and bare `aether auth` branches, matching the loading-line convention already used by slash.ts/vault.ts/slash_media.ts. Adds cmdAuth-level regression tests proving the loading line is written while /models is still pending (not after), for both call sites. * loop(LOOP-06): distinguish a picker key-handler fault from a deliberate Escape cancel pickModel's onKey catch silently resolved null on ANY thrown error (render/ decode bug), exactly like a user pressing Escape. showPicker then printed the same "kept current session." for both, hiding a real internal fault from the user and from anyone debugging a picker regression. Now the catch writes a distinct dim diagnostic ("picker error — kept current session.") before resolving null, so the two paths are visibly different in the transcript even though both still resolve null and restore the REPL listeners the same way. * loop(LOOP-06): style confirmSwitch's tier-lock message, add /tier hint The "<id> is locked on tier <tier>" rejection in confirmSwitch was plain unstyled text with no next step, unlike every other auth-adjacent surface where a plan/tier restriction (HTTP 403 via httpStatusHint) gets a dim hint pointing at /tier or `aether models`. It was also the only line in this function not wrapped in theme.dim (the restart warning two lines down already is). Now the locked-item line is dim and appends the same actionable pointer httpStatusHint(403) uses, so a tier lock reached via the picker reads the same as one reached over the wire. confirmSwitch is exported (alongside the existing resolveSelection) so this can be unit-tested directly — the module-level catalog cache the /model tests above populate isn't force-refreshable in-band, so a differently- available item can't reach this branch through handleSlash alone. * loop(LOOP-06): reject a stream with no terminal frame — throw StreamIncompleteError runCloudTurn and CloudBrain's pump both let decodeSse's for-await loop exit cleanly (no throw) when the underlying byte stream just ends without ever sending a done or error frame — e.g. a proxy/load-balancer that time-boxes the SSE response and closes the socket inside the idle window. That rendered whatever partial delta output arrived as a fully successful turn. Both sites now track whether a terminal frame was actually observed and throw/report StreamIncompleteError when it wasn't, routed through errorHint/hintFor like any other failure. * loop(LOOP-01): narrow pollForToken's HttpError guard to the RFC 8628 400 pending shape — a 5xx no longer masquerades as authorization_pending * loop(LOOP-01): stop workflow.ts's vault CRUD wrappers from swallowing every failure listWorkflows/getWorkflow/deleteWorkflow caught ANY error from listSpaces/ getSpacesContent/deleteSpacesFile -- an expired-session 401, a network outage, or a 5xx -- and turned it into an empty list / null / false. The command layer then reported that as legitimate "no data": "No workflows found in vault.", "workflow not found: <name>", "delete failed -- workflow may not exist", with zero indication to run `aether auth login`. vault.ts's own functions never throw for a genuinely empty vault or a content-less file (200 with `files: []` / `content: null`), so a thrown error here is always a real failure -- let it propagate to the existing per-command try/catch that already calls fail(). workflowSave's getWorkflow call sat outside its own try block (the try only wrapped the later saveWorkflow call), so removing the swallow would have let a real failure there escape cmdWorkflow entirely and hit main.ts's bare top-level catch with no hint at all -- gave it its own try/catch too. Adds test/workflow_vault_error_propagation.test.ts covering list/view/ delete/save against a 401 and a 500, plus a control case confirming a genuinely empty vault still prints "No workflows found in vault." * loop(LOOP-06): check err.cause.code in hintFor — no more silent null for cause-shaped undici network errors * loop(LOOP-06): route login.ts + auth.ts's authRefresh through formatErrorLine Every error path in cmdLogin (headless --username/--password, device-code request, and the poll-for-token wait) and authRefresh's failure paths wrote a raw, unstyled `✗ <message>` straight to stderr — no errTheme color, no errorHint-derived next step, no /doctor pointer, and none of formatErrorLine's trailing blank-line separator. Every other auth-adjacent surface (chat.ts's printError, render.ts's Renderer.error) had already been unified onto that convention; login/refresh, the surfaces a user hits first, were left bare. Route every catch in cmdLogin, plus authRefresh's catch AND its 200-with-no- session_token branch (same bare ✗ line, no err object but the same fix applies), through formatErrorLine(msg, { hint: errorHint(err, baseUrl) }) — same convention chat.ts's printError uses, so a network failure during login or refresh now gets the same /doctor pointer and a rejected session gets the same re-login pointer as everywhere else. Adds regression tests in test/login.test.ts (all three cmdLogin catch paths) and test/auth_401.test.ts (authRefresh's catch + the no-session_token branch) asserting the printed line carries the ✗ glyph, the errorHint-derived hint where one applies, and the separator — and confirms a plain error with no mapped hint (denied-in-browser) still gets styled output with no fabricated hint. Deliberately left login.ts's "No token on stdin" (--with-token, empty stdin) unconverted: it's an exit-2 input-validation message with no underlying err to derive a hint from, matching the unstyled "usage:"-style convention used by other exit-2 paths elsewhere in the CLI (config.ts, vault.ts, workflow.ts), and it wasn't in the finding's cited evidence. * loop(LOOP-01): route workflow.ts's fail() through hintFor() — no more hardcoded auth hint for 402/403/5xx * loop(LOOP-01): uploadFile() opts into the 120s stream-class timeout — no more flat 30s cap on large vault uploads postForm() bounds the ENTIRE request (connect + send-body + wait-for-response) with one flat timeoutMs, unlike getBinary()/stream() which split into a connect-phase timeout plus a separate idle-timeout on the body. fetch() gives no hook to observe outgoing multipart-body progress, so that split isn't achievable for the upload path. Instead uploadFile() now explicitly passes defaultStreamTimeoutMs() (120s) to postForm(), the same pattern client.ts/ workflow.ts already use for their long-running CHAT_PATH fallback, instead of silently inheriting postForm()'s 30s metadata-call default — so a large-but- healthy vault upload is no longer always killed by RequestTimeoutError just for taking longer than 30s. * loop(LOOP-06): wrap pickModel's empty-state lines in theme.dim — matches the rest of the picker's styling * loop(META): simplify — consolidate duplicated baseUrl-independent error hints errorHint (errors.ts) and hintFor (error_hints.ts) each hand-duplicated four identical branches (MalformedResponseError, StreamTimeoutError, StreamIncompleteError, RequestTimeoutError) that don't depend on baseUrl — different loop rounds added these to both files without reconciling them, and the RequestTimeoutError wording had already drifted between the two copies. Extracted the shared branches into errors.ts's new nonHttpErrorHint(), called first by both errorHint and hintFor; only the genuinely baseUrl-dependent 5xx/network branches remain separate per file. 812 tests still pass; no test pinned the drifted exact wording. * loop(META): close terminal-escape-injection gap in getBinary + media error paths Independent security review of the meta-loop diff found that a hostile server-controlled media_url can make fetch()'s own URL parser throw a raw TypeError whose message echoes the target string verbatim — the one path left unsanitized after this same loop closed the equivalent gap everywhere else (sanitizeServerText/sanitizeTerm/formatErrorLine). - transport.ts: getBinary() now sanitizes any non-typed error (a raw fetch()-level throw) before it leaves the function, mirroring sanitizeServerText's existing treatment of every other server string. - media.ts: sanitize resp.media_url, resp.text/response, and the catch block's err.message before writing them to the terminal. - slash_media.ts: sanitize writeErr's err.message (shared by every media slash command's catch block). * loop(META): bound LLM-generation calls by the 120s stream timeout, not the new 30s default Independent review found a real regression: commit a7b3621 gave ApiClient.request() a 30s default timeout, and the loop was careful to carve out defaultStreamTimeoutMs() (120s) overrides for every long-running generation call it touched (chat.ts, brain_cloud.ts, client.ts, workflow.ts) — except three sibling LLM-generation calls it missed: vision.ts's dispatchGeneration/parseStoryboard (image/video/storyboard generation — aether image, aether video, /photogen, /videogen, /animate, /reframe, /recut, /storyboard) and the same class in scaffold.ts/port.ts (LLM-backed code generation and cross-language translation). Any of these that legitimately ran past 30s now failed with RequestTimeoutError where they previously succeeded. * loop(META): fix duplicate "kept current session." message on a picker fault Round 3's fix (9ee863e) had pickModel() print a distinct diagnostic on an internal key-handler fault, but its caller (slash.ts's showPicker) still unconditionally printed its own generic "kept current session." right after — so a real fault showed two back-to-back, redundant lines instead of one clear message, while a plain Escape correctly showed just one. pickModel() now resolves undefined (not null) on a fault, distinguishing it from a deliberate cancel; showPicker checks for undefined and skips its own message in that case. Added an end-to-end regression test exercising the full handleSlash -> showPicker -> pickModel path (the prior tests only covered pickModel in isolation, which is why this composition bug wasn't caught). --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent cacb6c2 commit e07b115

47 files changed

Lines changed: 3892 additions & 289 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/commands/auth.ts‎

Lines changed: 64 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,19 @@
99
import type { AppContext } from "../core/context.js";
1010
import { cmdLogin, cmdLogout, type LoginOpts } from "./login.js";
1111
import { MODELS_PATH, REFRESH_PATH } from "../core/transport.js";
12+
import { HttpError, errorHint, errorMessage } from "../core/errors.js";
13+
import { isApiKeyToken } from "../core/auth.js";
1214
import { box, titledBox, hyperlink, orange, green, darkBlue, brightWhite, lightBlue } from "../ui/box.js";
1315
import { CLOUD } from "../ui/logo.js";
1416
import { theme } from "../ui/theme.js";
17+
import { formatErrorLine } from "../ui/error_line.js";
1518

16-
/** A long-lived API token (PAT) starts with `aek_`; otherwise it's a session token. */
19+
/** A long-lived API token (PAT) starts with `aek_`; otherwise it's a session
20+
* token. Thin wrapper so callers of THIS module keep importing `isApiToken`
21+
* from here — the actual `aek_` prefix check is canonical in core/auth.ts's
22+
* isApiKeyToken (shared with transport.ts's refreshSession). */
1723
export function isApiToken(token: string | null | undefined): boolean {
18-
return typeof token === "string" && token.startsWith("aek_");
24+
return isApiKeyToken(token);
1925
}
2026

2127
function mask(t: string): string {
@@ -35,37 +41,61 @@ function centeredCloud(): string {
3541

3642
// ── Branded status panel ──
3743

38-
async function renderAuthBox(ctx: AppContext): Promise<string> {
44+
// Exported so tests can render the panel directly against a fake AppContext
45+
// without going through cmdAuth's stdout write.
46+
export async function renderAuthBox(ctx: AppContext): Promise<string> {
3947
const t = await ctx.tokens.get();
4048
if (!t) return renderLoggedOut();
4149

4250
// Fetch tier info for display
4351
let tier = "";
4452
let defaultModel = "";
53+
// A 401/403 here means the SERVER actively rejected this token (expired or
54+
// revoked session) — that is a different situation from "server
55+
// unreachable" and must not be swallowed the same way, or `status` would
56+
// claim "Authenticated" for a dead session (the stale-AETHER_TOKEN case PR
57+
// #47 fixed elsewhere). Any other failure (no HttpError, or a 5xx) is a
58+
// genuine network/server problem: keep the existing silent local-only
59+
// fallback for those.
60+
let sessionExpired = false;
4561
try {
4662
const cat = await ctx.api.getJson<{ tier?: string; default?: string }>(MODELS_PATH);
4763
tier = cat.tier ?? "";
4864
defaultModel = cat.default ?? "";
49-
} catch {
50-
// Server unreachable — still show what we know locally.
65+
} catch (err) {
66+
if (err instanceof HttpError && (err.status === 401 || err.status === 403)) {
67+
sessionExpired = true;
68+
}
69+
// Otherwise: server unreachable — still show what we know locally.
5170
}
5271

5372
const kind = isApiToken(t) ? "API key" : "session token";
73+
const header = sessionExpired
74+
? theme.yellow("⚠") + " " + theme.bold("Aether Agent — Session expired")
75+
: theme.iceBlue("☁") + " " + theme.bold("Aether Agent — Authenticated");
5476
const lines: string[] = [
5577
"",
56-
theme.iceBlue("\u2601") + " " + theme.bold("Aether Agent \u2014 Authenticated"),
78+
header,
5779
"",
5880
// No Account row: there is no account endpoint yet, and a hardcoded
59-
// "(fetching\u2026)" that never resolves is a fake loading state (PR #47 UX).
81+
// "(fetching…)" that never resolves is a fake loading state (PR #47 UX).
6082
" " + theme.dim("Token:") + " " + theme.bold(mask(t)) + " " + theme.dim(`(${kind})`),
6183
" " + theme.dim("API:") + " " + ctx.cfg.baseUrl.replace("https://", ""),
6284
];
6385

64-
if (tier) {
65-
lines.push(" " + theme.dim("Tier:") + " " + (tier === "free" ? theme.dim(tier) : theme.cyan(tier)));
66-
}
67-
if (defaultModel) {
68-
lines.push(" " + theme.dim("Default:") + " " + defaultModel);
86+
if (sessionExpired) {
87+
lines.push(
88+
"",
89+
" " + theme.yellow("Server rejected this token — sign in again:"),
90+
" " + theme.bold(theme.cyan("aether auth login")),
91+
);
92+
} else {
93+
if (tier) {
94+
lines.push(" " + theme.dim("Tier:") + " " + (tier === "free" ? theme.dim(tier) : theme.cyan(tier)));
95+
}
96+
if (defaultModel) {
97+
lines.push(" " + theme.dim("Default:") + " " + defaultModel);
98+
}
6999
}
70100

71101
lines.push(
@@ -78,7 +108,8 @@ async function renderAuthBox(ctx: AppContext): Promise<string> {
78108
"",
79109
);
80110

81-
return [centeredCloud(), "", titledBox(lines, "Authenticated", { width: BOX_W })].join("\n");
111+
const title = sessionExpired ? "Session expired" : "Authenticated";
112+
return [centeredCloud(), "", titledBox(lines, title, { width: BOX_W })].join("\n");
82113
}
83114

84115
// ── Logged-out welcome panel ──
@@ -140,6 +171,13 @@ export async function cmdAuth(
140171
case "logout":
141172
return cmdLogout(ctx);
142173
case "status": {
174+
// LOOP-06: renderAuthBox's first move is a network round-trip to
175+
// /models (no cache to fall back on for the CLI's one-shot status
176+
// command), and nothing was written to stdout until the whole thing
177+
// resolved — up to DEFAULT_REQUEST_TIMEOUT_MS of silence on a slow
178+
// connection, "the REPL just looks hung" (the exact class PR #47 fixed
179+
// for slash.ts's catalog fetch). Match that convention here.
180+
process.stdout.write(theme.dim("checking session…\n"));
143181
const panel = await renderAuthBox(ctx);
144182
process.stdout.write(panel + "\n");
145183
return 0;
@@ -152,8 +190,11 @@ export async function cmdAuth(
152190
printAuthHelp();
153191
return 0;
154192
default: {
155-
// Bare `aether auth` — show the branded panel.
193+
// Bare `aether auth` — show the branded panel. Same LOOP-06 loading
194+
// line as the "status" branch above — this hits renderAuthBox's same
195+
// /models call.
156196
if (sub === "") {
197+
process.stdout.write(theme.dim("checking session…\n"));
157198
const panel = await renderAuthBox(ctx);
158199
process.stdout.write("\n" + panel + "\n\n");
159200
return 0;
@@ -208,10 +249,17 @@ async function authRefresh(ctx: AppContext): Promise<number> {
208249
process.stdout.write("✓ Session refreshed.\n");
209250
return 0;
210251
}
211-
process.stderr.write("✗ Refresh failed — try: aether auth login\n");
252+
// Same formatErrorLine convention as the catch below — a 200 with no
253+
// session_token is still a refresh failure and must read the same as
254+
// one caught via a thrown error, not as the one bare ✗ line left in
255+
// this function (LOOP-06 round 3).
256+
process.stderr.write(formatErrorLine("Refresh failed", { hint: "run `aether auth login`" }));
212257
return 1;
213258
} catch (err) {
214-
process.stderr.write(`\u2717 ${err instanceof Error ? err.message : String(err)}\n`);
259+
// formatErrorLine (LOOP-06) owns the glyph/hint/separator convention,
260+
// same as chat.ts's printError, so a failed /auth/refresh reads like
261+
// any other auth-adjacent error instead of bare, uncolored text.
262+
process.stderr.write(formatErrorLine(errorMessage(err), { hint: errorHint(err, ctx.cfg.baseUrl) }));
215263
return 1;
216264
}
217265
}

‎src/commands/chat.ts‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,11 @@ import { StringDecoder } from "node:string_decoder";
77
import type { AppContext, GlobalFlags } from "../core/context.js";
88
import { theme, errTheme } from "../ui/theme.js";
99
import { buildChatRequest } from "../core/envelope.js";
10-
import { CHAT_STREAM_PATH, CHAT_PATH } from "../core/transport.js";
10+
import { CHAT_STREAM_PATH, CHAT_PATH, defaultStreamTimeoutMs } from "../core/transport.js";
1111
import { decodeSse } from "../core/stream.js";
1212
import { Renderer } from "../core/render.js";
13-
import { StreamUnavailableError, errorHint, isAbortError } from "../core/errors.js";
13+
import { StreamIncompleteError, StreamUnavailableError, errorHint, isAbortError } from "../core/errors.js";
14+
import { formatErrorLine } from "../ui/error_line.js";
1415
import { appendCustody } from "../core/custody.js";
1516
import { handleSlash, primeCatalog } from "./slash.js";
1617
import { applyPromptMode } from "./prompt_modes.js";
@@ -88,7 +89,10 @@ export async function resolveBackend(ctx: AppContext): Promise<BackendPath> {
8889
* `signal` cancels the turn client-side (stream AND the fail-soft fallback) —
8990
* orchestrator runs inherit cancelability through this same seam. Throws
9091
* ChatTurnError if the server streamed an `error` frame, so callers can exit
91-
* non-zero instead of treating a rendered "✗ msg" as a successful turn.
92+
* non-zero instead of treating a rendered "✗ msg" as a successful turn. Also
93+
* throws StreamIncompleteError if the stream ends without ever sending a
94+
* terminal `done` or `error` frame (LOOP-06 round 3) — a clean-looking
95+
* premature close must not render as a successful turn either.
9296
* `onPulsePaint` fires after every thinking-pulse repaint (see
9397
* ThinkingPulseOptions.onPaint) so the REPL can re-sync its own input-line
9498
* redraw — typing ahead during the pre-first-token window would otherwise
@@ -143,6 +147,13 @@ async function runCloudTurn(
143147
});
144148
pulse.start();
145149
let sawError: string | null = null;
150+
// Whether a terminal `done` or `error` frame was actually observed. decodeSse's
151+
// for-await loop exits normally (no throw) once the underlying byte stream
152+
// ends — including a premature-but-clean close (proxy/load-balancer time-box,
153+
// etc.) that never sent either. Without this, such a close renders whatever
154+
// partial output arrived and returns as if the turn completed successfully
155+
// (LOOP-06 round 3).
156+
let sawTerminal = false;
146157
try {
147158
const stream = await ctx.api.stream(CHAT_STREAM_PATH, req, signal);
148159
for await (const frame of decodeSse(stream)) {
@@ -154,15 +165,21 @@ async function runCloudTurn(
154165
// locally (best-effort, never breaks the chat).
155166
if (frame.type === "custody") appendCustody(frame.custody);
156167
if (frame.type === "error") sawError = frame.msg;
168+
if (frame.type === "error" || frame.type === "done") sawTerminal = true;
157169
onFrame?.(frame);
158170
renderer.frame(frame);
159171
}
160172
if (sawError) throw new ChatTurnError(sawError);
173+
if (!sawTerminal) throw new StreamIncompleteError();
161174
} catch (err) {
162175
if (err instanceof StreamUnavailableError) {
163176
// Contract fail-soft: fall back to the non-streaming request/response.
164-
// Same signal — the fallback leg is cancelable too (arena AT-3d).
165-
const r = await ctx.api.postJson<ChatJsonResponse>(CHAT_PATH, req, signal);
177+
// Same signal — the fallback leg is cancelable too (arena AT-3d). A
178+
// full LLM turn can legitimately run long, so this explicitly opts
179+
// into stream()'s own generous bound instead of request()'s 30s
180+
// metadata-call default (LOOP-01/LOOP-06 round-1) — otherwise a
181+
// perfectly healthy but slow completion would be killed early.
182+
const r = await ctx.api.postJson<ChatJsonResponse>(CHAT_PATH, req, signal, defaultStreamTimeoutMs());
166183
pulse.stop();
167184
process.stdout.write((r.response ?? "") + "\n");
168185
if (ctx.flags.audit && r.commitment_hash) {
@@ -957,11 +974,8 @@ async function replLines(ctx: AppContext): Promise<number> {
957974

958975
function printError(err: unknown, baseUrl: string): void {
959976
const msg = err instanceof Error ? err.message : String(err);
960-
process.stderr.write(`\n${errTheme.red("✗")} ${msg}\n`);
961-
const hint = errorHint(err, baseUrl);
962-
if (hint) process.stderr.write(errTheme.dim(` ⤷ ${hint}`) + "\n");
963-
// Trailing blank line: the REPL reprints its prompt right after this, and
964-
// without the separator the dim hint and the prompt fuse into one line
965-
// (the "⤷ run `aether auth login` … [user]_:" mess from PR #47's report).
966-
process.stderr.write("\n");
977+
// formatErrorLine (LOOP-06) owns the glyph/hint/separator convention so
978+
// this reads identically to a server-streamed error frame's Renderer.error
979+
// — see src/ui/error_line.ts for why both paths must agree.
980+
process.stderr.write(formatErrorLine(msg, { hint: errorHint(err, baseUrl) }));
967981
}

‎src/commands/login.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import { LOGOUT_PATH } from "../core/transport.js";
1313
import { requestDeviceCode, pollForToken } from "../core/device.js";
1414
import { openBrowser } from "../core/browser.js";
1515
import { theme } from "../ui/theme.js";
16+
import { errorHint, errorMessage } from "../core/errors.js";
17+
import { formatErrorLine } from "../ui/error_line.js";
1618

1719
export interface LoginOpts {
1820
token?: string;
@@ -75,7 +77,10 @@ export async function cmdLogin(ctx: AppContext, opts: LoginOpts): Promise<number
7577
warnEnvTokenShadow();
7678
return 0;
7779
} catch (err) {
78-
process.stderr.write(`✗ ${err instanceof Error ? err.message : String(err)}\n`);
80+
// formatErrorLine (LOOP-06) owns the glyph/hint/separator convention —
81+
// same as chat.ts's printError — so a headless login failure reads the
82+
// same as any other auth-adjacent error instead of bare, uncolored text.
83+
process.stderr.write(formatErrorLine(errorMessage(err), { hint: errorHint(err, ctx.cfg.baseUrl) }));
7984
return 1;
8085
}
8186
}
@@ -86,7 +91,7 @@ export async function cmdLogin(ctx: AppContext, opts: LoginOpts): Promise<number
8691
code = await requestDeviceCode(ctx.api);
8792
} catch (err) {
8893
process.stderr.write(
89-
`✗ could not start login: ${err instanceof Error ? err.message : String(err)}\n`,
94+
formatErrorLine(`could not start login: ${errorMessage(err)}`, { hint: errorHint(err, ctx.cfg.baseUrl) }),
9095
);
9196
return 1;
9297
}
@@ -103,7 +108,7 @@ export async function cmdLogin(ctx: AppContext, opts: LoginOpts): Promise<number
103108
warnEnvTokenShadow();
104109
return 0;
105110
} catch (err) {
106-
process.stderr.write(`✗ ${err instanceof Error ? err.message : String(err)}\n`);
111+
process.stderr.write(formatErrorLine(errorMessage(err), { hint: errorHint(err, ctx.cfg.baseUrl) }));
107112
return 1;
108113
}
109114
}

‎src/commands/media.ts‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type { AppContext } from "../core/context.js";
88
import type { CatalogResponse } from "../types.js";
99
import { MODELS_PATH } from "../core/transport.js";
1010
import { fail as coreFail } from "../core/errors.js";
11+
import { hintFor } from "../core/error_hints.js";
1112
import {
1213
filterMediaModels, resolveModelKey, autoRouteModel,
1314
buildMediaPrompt, dispatchGeneration, downloadMediaFile,
@@ -16,6 +17,7 @@ import {
1617
type MediaKind, type MediaModel, type GenFlags, type GenResult,
1718
} from "../core/vision.js";
1819
import { theme } from "../ui/theme.js";
20+
import { sanitizeTerm } from "../ui/text.js";
1921
import { basename } from "node:path";
2022
import { createInterface, type Interface } from "node:readline";
2123

@@ -196,17 +198,29 @@ async function mediaGenerate(ctx: AppContext, prompt: string, kind: MediaKind, f
196198
url: resp.media_url, timestamp: new Date().toISOString(), flags,
197199
};
198200
const entry = recordOutput(result);
201+
// resp.media_url and resp.text/response below are server-controlled
202+
// (an LLM generation response) — sanitized before hitting the
203+
// terminal, same as every other server-supplied string in this flow
204+
// (see sanitizeServerText's own doc comment in transport.ts).
199205
process.stdout.write(
200206
` ${theme.iceBlue("↓")} #${entry.index} ${entry.filename}\n` +
201-
` ${theme.dim(`url: ${resp.media_url}`)}\n\n`
207+
` ${theme.dim(`url: ${sanitizeTerm(resp.media_url)}`)}\n\n`
202208
);
203209
if (flags.open) openOutput(entry);
204210
} else {
205211
process.stdout.write(theme.dim(" no media URL in response\n"));
206-
process.stdout.write(theme.dim(` ${text.slice(0, 200)}\n`));
212+
process.stdout.write(theme.dim(` ${sanitizeTerm(text.slice(0, 200))}\n`));
207213
}
208214
} catch (err) {
209-
process.stdout.write(`✗ ${err instanceof Error ? err.message : String(err)}\n`);
215+
// LOOP-01 round 2: dispatchGeneration()/downloadMediaFile() already
216+
// throw a classified HttpError (401/402/403/5xx) — this used to print
217+
// only the raw message with no actionable next step at all. Sanitized:
218+
// a raw fetch()-level failure on a hostile media_url can otherwise
219+
// carry attacker-controlled bytes straight from err.message.
220+
const msg = err instanceof Error ? err.message : String(err);
221+
process.stdout.write(`✗ ${sanitizeTerm(msg)}\n`);
222+
const hint = hintFor(err);
223+
if (hint) process.stdout.write(theme.dim(` ⤷ ${hint}\n`));
210224
}
211225
}
212226
process.stdout.write(theme.dim(`\noutput: ${outdir}/\nview: aether output\n`));
@@ -230,6 +244,7 @@ function printMediaHelp(kind: MediaKind): void {
230244
].join("\n"));
231245
}
232246

247+
// See vault.ts's identical fix for why hintFor() replaces the hardcoded hint.
233248
function fail(err: unknown): number {
234-
return coreFail(err, "are you logged in? run: aether auth login");
249+
return coreFail(err, hintFor(err) ?? undefined);
235250
}

‎src/commands/slash.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,12 @@ async function showPicker(
374374
const items = byKind(cat, kind);
375375

376376
const picked = await pickModel(items, out);
377+
if (picked === undefined) {
378+
// pickModel hit an internal fault and already printed its own distinct
379+
// diagnostic — printing the generic "kept current session." below too
380+
// would show the same failure as two back-to-back, redundant lines.
381+
return null;
382+
}
377383
if (!picked) {
378384
// pickModel returned null — either cancelled (Esc) or non-TTY fallback.
379385
// If non-TTY, render a flat numbered list so the user can still /model <n>.
@@ -420,15 +426,19 @@ async function select(
420426

421427
/** Shared by showPicker/select once a target item is resolved: lock check,
422428
* restart warning, and the y/N gate that produces the caller's restart signal. */
423-
async function confirmSwitch(
429+
export async function confirmSwitch(
424430
ctx: AppContext,
425431
out: Writable,
426432
item: CatalogItem,
427433
kind: Kind,
428434
tier: string,
429435
): Promise<{ model?: string; agent?: string } | null> {
430436
if (!item.available) {
431-
out.write(`${item.id} is locked on tier ${tier}\n`);
437+
// Same dim styling + "check: /tier or `aether models`" pointer as
438+
// httpStatusHint(403) (errors.ts) — a tier lock reached via the picker
439+
// must read the same as the functionally identical 403 reached over the
440+
// wire, not as unstyled text with no next step. LOOP-06.
441+
out.write(theme.dim(`${item.id} is locked on tier ${tier} — check: /tier or \`aether models\`\n`));
432442
return null;
433443
}
434444
out.write(

0 commit comments

Comments
 (0)