diff --git a/backend/cli/src/auth/index.ts b/backend/cli/src/auth/index.ts index 402f8e27..574572b7 100644 --- a/backend/cli/src/auth/index.ts +++ b/backend/cli/src/auth/index.ts @@ -2,10 +2,25 @@ import path from "path" import { Global } from "../global" import { JsonStore } from "../util/jsonstore" import z from "zod" +import { Config } from "../config/config" +import { Log } from "../util/log" export const OAUTH_DUMMY_KEY = "synsc-oauth-dummy-key" +const log = Log.create({ service: "auth" }) + export namespace Auth { + /** A managed Atlas wallet credential (`thk_*`), as opposed to a user-owned + * (BYOK) key. Canonical home: `auth/index.ts` is a near-leaf module (only + * path/global/jsonstore/zod besides this file's own Config import), so + * `provider.ts` - which already imports Auth - depends on this instead of + * Auth duplicating or importing from Provider (a much heavier module: all + * the AI SDK loaders, plus Provider already imports Auth AND Config, so + * an Auth -> Provider edge would close two cycles through it at once). */ + export function isAtlasApiKey(key: unknown): key is string { + return typeof key === "string" && key.startsWith("thk_") + } + export const Oauth = z .object({ type: z.literal("oauth"), @@ -57,6 +72,44 @@ export namespace Auth { export async function set(key: string, info: Info) { await JsonStore.update(filepath, (data) => ({ ...data, [key]: info })) + + // Adding a real (non-Atlas) OpenRouter key while Managed spend is on + // means the user is bringing their own key - flip the toggle to Own + // keys so the added key actually wins routing immediately, instead of + // sitting unused behind the managed route until the user finds the + // Settings toggle. This is the ONE choke point both `openscience auth + // login` (CLI - calls Auth.set directly, see cli/cmd/auth.ts) and the + // Settings UI (PUT /auth/:providerID -> Auth.set) go through, so it + // belongs here rather than in the HTTP route. A `thk_` Atlas token is + // never "own key" material and must not flip the mode; other providers + // and OAuth credentials are untouched. + if (key === "openrouter" && info.type === "api" && !isAtlasApiKey(info.key)) { + try { + // Reads the GLOBAL config specifically (not the merged project+global + // Config.get(), which requires an active Instance/project context that + // most Auth.set callers - including every CLI auth command - don't + // have). billing.llm can also be set at project scope; a project-level + // override is invisible to this check, same asymmetry the byok guard + // in provider.ts lives with when read outside a project context. + const cfg = await Config.getGlobal() + if (cfg.billing?.llm === "managed") { + await Config.updateGlobal({ billing: { llm: "byok" } }) + } + } catch (e) { + // A malformed global config (a hand-edited openscience.jsonc with a + // trailing comma, say) makes Config.getGlobal()/updateGlobal() throw. + // That must not take down Auth.set - the credential above is already + // persisted, and Auth.set has 11 call sites, at least one with no + // try/catch of its own (the CLI's "paste the code" OAuth branch, + // cli/cmd/auth.ts:143-170). Degrade to "key saved, mode not flipped" + // rather than losing the key the user just added - but log it at + // warn: silently swallowing would leave the user's mode silently + // disagreeing with the key they just added, with no signal at all. + log.warn("failed to flip billing.llm to byok after adding an OpenRouter key", { + error: e instanceof Error ? e.message : String(e), + }) + } + } } export async function remove(key: string) { diff --git a/backend/cli/src/cli/cmd/models.ts b/backend/cli/src/cli/cmd/models.ts index c0941b1d..cb900764 100644 --- a/backend/cli/src/cli/cmd/models.ts +++ b/backend/cli/src/cli/cmd/models.ts @@ -1,4 +1,5 @@ import type { Argv } from "yargs" +import { Auth } from "../../auth" import { Instance } from "../../project/instance" import { Provider } from "../../provider/provider" import { ModelsDev } from "../../provider/models" @@ -30,7 +31,7 @@ const PROVIDER_LABELS: Record = { * * Detection rules: * - openai-codex routes via OAuth (Sign in with ChatGPT), neither. - * - key starts with "thk_" → managed (the proxy thumbprint Atlas hands + * - Auth.isAtlasApiKey(key) → managed (the proxy thumbprint Atlas hands * out on /api/cli/sync when the user has no BYOK key set). * - options.baseURL points at Atlas (/api/llm/proxy/) → managed. * - anything else with a key → BYOK. @@ -45,7 +46,9 @@ function routingLabel(providerID: string, provider: Provider.Info): string { // demo sentinel is not a real credential. const effective = Provider.effectiveKey(provider) const baseURL = (provider.options?.baseURL as string | undefined) ?? "" - if ((effective ?? "").toLowerCase().startsWith("thk_")) return "managed" + if (Auth.isAtlasApiKey(effective)) return "managed" + // Kept as a separate signal: a stale synced ANTHROPIC_BASE_URL can point at + // the Atlas proxy while the key itself is not a thk_ token. if (baseURL.includes("/api/llm/proxy/")) return "managed" // A config-registered local endpoint stores its key under options.apiKey (not // provider.key), so it would otherwise read as "unconfigured". diff --git a/backend/cli/src/config/config.ts b/backend/cli/src/config/config.ts index 0c79bafd..4f7372c5 100644 --- a/backend/cli/src/config/config.ts +++ b/backend/cli/src/config/config.ts @@ -1611,6 +1611,48 @@ export namespace Config { }, input) } + /** + * Dispose every open project instance after a GLOBAL config write and + * announce it. Awaited (not fire-and-forget): the per-directory + * Config.state cache (config.ts's `state`, backed by Instance.state) is + * only invalidated by Instance.dispose()/disposeAll() — resetting the + * `global` lazy singleton above is not enough on its own for an + * already-instantiated project directory. Callers of setMcp/setProvider/ + * setSandbox/unsetGlobal/updateGlobal/replaceGlobal rely on the write + * being visible to the very next Config.get(), not eventually-after-a- + * fire-and-forget-settles visible. + * + * The provider cache is dropped here too, and specifically BEFORE the + * announcement. Provider memoises the resolved provider/SDK map at module + * scope keyed only by directory + trust, which Instance.disposeAll() does + * not touch and this write does not change — so it outlives the write. The + * SPA refetches GET /provider the instant it sees `global.disposed`, and a + * refetch that lands in the gap re-memoises the PRE-write map (the key just + * added still missing, billing still reading managed) with nothing left to + * invalidate it afterwards. Announcing a disposal that the provider map has + * not honoured yet is the bug; the two belong together. + */ + async function disposeGlobalInstances() { + await Instance.disposeAll().catch(() => undefined) + // Lazy because provider.ts imports Config — the same cycle-break + // provider/models.ts and openscience/index.ts already use to reach it. + // Best-effort like the disposal above: the config file is already written + // by the time this runs, so a throw here (e.g. provider module init + // failing) must not turn a landed write into a rejected one. + await import("../provider/provider") + .then((m) => m.Provider.invalidate()) + .catch((e) => + log.warn("failed to invalidate provider cache", { error: e instanceof Error ? e.message : String(e) }), + ) + GlobalBus.emit("event", { + directory: "global", + payload: { + type: Event.Disposed.type, + properties: {}, + }, + }) + } + async function patchConfigPath(scope: Scope, target: string[], value: unknown) { const filepath = scope === "global" ? globalConfigFile() : projectConfigFile() const before = await Bun.file(filepath) @@ -1631,17 +1673,7 @@ export namespace Config { const parsed = parseConfig(updated, filepath) global.reset() if (scope === "global") { - void Instance.disposeAll() - .catch(() => undefined) - .finally(() => { - GlobalBus.emit("event", { - directory: "global", - payload: { - type: Event.Disposed.type, - properties: {}, - }, - }) - }) + await disposeGlobalInstances() } else { await Instance.dispose() } @@ -1728,17 +1760,7 @@ export namespace Config { await fs.mkdir(path.dirname(filepath), { recursive: true }) await Bun.write(filepath, content) global.reset() - void Instance.disposeAll() - .catch(() => undefined) - .finally(() => { - GlobalBus.emit("event", { - directory: "global", - payload: { - type: Event.Disposed.type, - properties: {}, - }, - }) - }) + await disposeGlobalInstances() return parsed } @@ -1800,18 +1822,7 @@ export namespace Config { })() global.reset() - - void Instance.disposeAll() - .catch(() => undefined) - .finally(() => { - GlobalBus.emit("event", { - directory: "global", - payload: { - type: Event.Disposed.type, - properties: {}, - }, - }) - }) + await disposeGlobalInstances() return next } diff --git a/backend/cli/src/project/project.ts b/backend/cli/src/project/project.ts index 7f8b7850..bae08343 100644 --- a/backend/cli/src/project/project.ts +++ b/backend/cli/src/project/project.ts @@ -53,6 +53,13 @@ export namespace Project { }), ) + export const DirectoryError = NamedError.create( + "ProjectDirectoryError", + z.object({ + directory: z.string(), + }), + ) + export const Info = z .object({ id: z.string(), @@ -169,12 +176,24 @@ export namespace Project { * A caller may include a legacy directory while migrating, but it must remain * inside the selected project's recorded roots. */ + /** + * Only a genuinely absent record means the project is gone. Any other read + * failure — a torn file from a concurrent writer, a transient fs error — must + * propagate, because reporting it as 410 tells the caller to stop asking + * about a project that is in fact fine, and the client empties the surfaces + * that depend on it. + */ + function absent(error: unknown) { + if (Storage.NotFoundError.isInstance(error)) return undefined + throw error + } + export async function resolve(projectID: string, directory?: string) { - const direct = await Storage.read(["project", projectID]).catch(() => undefined) - const link = await Storage.read>(["project_alias", projectID]).catch(() => undefined) + const direct = await Storage.read(["project", projectID]).catch(absent) + const link = await Storage.read>(["project_alias", projectID]).catch(absent) if (!direct && !link) throw new UnknownError({ projectID }) - const linked = link ? await Storage.read(["project", link.projectID]).catch(() => undefined) : undefined + const linked = link ? await Storage.read(["project", link.projectID]).catch(absent) : undefined const redirected = !!linked && (!direct || !projectID.startsWith("prj_")) const project = redirected ? linked : (direct ?? linked) if (!project) { @@ -210,6 +229,18 @@ export namespace Project { } } + /** + * Guard a caller-supplied root before it can mint a project. Anything that is + * not an absolute path gets resolved against the server's cwd, which turned + * junk from a stale deep link into a real-looking folder under the user's + * home and left a phantom project on their home list. + */ + export async function assertDirectory(input: string) { + if (!path.isAbsolute(input)) throw new DirectoryError({ directory: input }) + const stat = await fs.stat(input).catch(() => undefined) + if (!stat?.isDirectory()) throw new DirectoryError({ directory: input }) + } + export async function fromDirectory(input: string) { const directory = canonicalize(input) log.info("fromDirectory", { directory }) diff --git a/backend/cli/src/provider/inference.ts b/backend/cli/src/provider/inference.ts index 9b72c467..0bc9fd9d 100644 --- a/backend/cli/src/provider/inference.ts +++ b/backend/cli/src/provider/inference.ts @@ -25,7 +25,7 @@ export namespace Inference { export function classify(input: { providerID: string billing?: "managed" | "byok" | null - providerSource?: "env" | "config" | "custom" | "api" + providerSource?: "env" | "config" | "custom" | "api" | "managed" baseURL?: string auth?: Auth.Info["type"] }): Source { @@ -35,6 +35,11 @@ export namespace Inference { if (input.providerID === "openrouter" && input.billing === "managed") return "managed" if (input.auth === "oauth") return "oauth" if (input.auth === "api" || input.auth === "wellknown") return "byok" + // Auto-detect (billing unset) never sets `billing === "managed"` above, but a + // synced thk_ token with no own key still genuinely routes through the Atlas + // proxy — provider.source already says "managed" (provider.ts's openrouter + // loader), so trust it here too instead of falling through to "unknown". + if (input.providerSource === "managed") return "managed" if (input.providerSource === "env" || input.providerSource === "config" || input.providerSource === "api") { return "byok" } diff --git a/backend/cli/src/provider/provider.ts b/backend/cli/src/provider/provider.ts index 82642e5f..0c9ae24b 100644 --- a/backend/cli/src/provider/provider.ts +++ b/backend/cli/src/provider/provider.ts @@ -117,10 +117,6 @@ export namespace Provider { "@ai-sdk/github-copilot": createGitHubCopilotOpenAICompatible, } - function isAtlasApiKey(key: unknown): key is string { - return typeof key === "string" && key.startsWith("thk_") - } - const REMOVED_MODEL_IDS = new Set(["mistralai/mistral-small-3.2-24b-instruct"]) function isRemovedModel(modelID: string) { @@ -211,7 +207,7 @@ export namespace Provider { // proxy routing for it hard-failed every call with advice (`connect // sync`) that re-delivers the same env and can never fix it. const effective = effectiveKey(provider, options) - if (!isAtlasApiKey(effective)) return + if (!Auth.isAtlasApiKey(effective)) return if (isAtlasProxyBaseURL(options["baseURL"])) return throw new Error( `${provider.id} is using a managed Atlas key without an Atlas proxy URL. ` + @@ -222,7 +218,7 @@ export namespace Provider { /** A user-owned (BYOK) key: a real, non-managed credential. Excludes the * "public" sentinel used for the zero-cost openscience demo models. */ function isByokKey(key: unknown): key is string { - return typeof key === "string" && key.length > 0 && key !== "public" && !isAtlasApiKey(key) + return typeof key === "string" && key.length > 0 && key !== "public" && !Auth.isAtlasApiKey(key) } /** The credential that actually authenticates a provider: an explicit apiKey @@ -310,7 +306,7 @@ export namespace Provider { function pinByokToPublicEndpoint(provider: Info, options: Record, publicURL?: string) { const effective = effectiveKey(provider, options) // Managed (thk_*) keys must keep their Atlas proxy routing. - if (isAtlasApiKey(effective)) return + if (Auth.isAtlasApiKey(effective)) return if (!isByokKey(effective)) return if (hasManagedProxyPath(options["baseURL"])) { log.warn("refusing to route BYOK key through Atlas proxy — pinning to public endpoint", { @@ -336,6 +332,11 @@ export namespace Provider { autoload: boolean getModel?: CustomModelLoader options?: Record + // Overrides the reported `source` regardless of whether the provider was + // already registered by an earlier stage (env/api/plugin). Only the + // openrouter loader's managed-proxy branch sets this today — every other + // loader leaves it undefined and keeps the call site's default behavior. + source?: Info["source"] }> const CUSTOM_LOADERS: Record = { @@ -597,16 +598,29 @@ export namespace Provider { "HTTP-Referer": "https://syntheticsciences.ai/", "X-Title": "synsci", } - // OpenRouter is the ONE provider with both a managed and a BYOK route, and - // resolution is deterministic by key presence (mirrors the Atlas server's - // BYOK-first rule): the user's OWN OpenRouter key wins and hits public - // OpenRouter directly; with no own key, a logged-in session falls back to - // the Atlas managed proxy (thk_* token → wallet-billed). Deleting the own - // key restores the managed route automatically — nothing is latched. + // OpenRouter is the ONE provider with both a managed and a BYOK route. + // Resolution is gated on the explicit `billing.llm` spend toggle: an + // explicit "managed" opt-in refuses to route on a stored own key, so it + // resolves the Atlas managed proxy (thk_* token → wallet-billed) even + // when an own key exists. The key is retained in auth (never + // deleted/rewritten) and simply loses this branch. When managed spend is + // on but NO managed credential can be found — a lapsed Atlas session, + // say — this returns no credential at all, and the availability guard in + // init() then drops the provider outright; it must, because the earlier + // "load apikeys" stage has already stamped provider.key from auth.json + // and getSDK would otherwise fall back to it against public OpenRouter, + // billing the user's own key under a toggle that reads "Managed". + // `byok` and auto-detect (unset / null) are unchanged from the old + // key-presence rule: the user's OWN OpenRouter key wins and hits public + // OpenRouter directly; with no own key, a logged-in session falls back + // to the Atlas managed proxy. Switching billing.llm back to + // byok/auto-detect (or deleting the own key under those modes) restores + // the previous resolution automatically — nothing is latched. const auth = await Auth.get("openrouter").catch(() => undefined) const authKey = auth?.type === "api" ? auth.key : undefined const envKey = Env.get("OPENROUTER_API_KEY") - const ownKey = isByokKey(authKey) ? authKey : isByokKey(envKey) ? envKey : undefined + const managed = (await Config.get().catch(() => undefined))?.billing?.llm === "managed" + const ownKey = managed ? undefined : isByokKey(authKey) ? authKey : isByokKey(envKey) ? envKey : undefined if (ownKey) { // Honour a user's own OpenRouter-compatible gateway (custom // OPENROUTER_BASE_URL); only the Atlas proxy is swapped for the public @@ -623,10 +637,10 @@ export namespace Provider { // the session file is momentarily unreadable. const proxyBase = Env.get("OPENROUTER_BASE_URL") const session = await OpenScience.getSession().catch(() => null) - const managedKey = session?.api_key ?? (isAtlasApiKey(envKey) ? envKey : undefined) + const managedKey = session?.api_key ?? (Auth.isAtlasApiKey(envKey) ? envKey : undefined) if (managedKey) { const baseURL = isAtlasProxyBaseURL(proxyBase) ? proxyBase : managedOpenRouterBaseURL() - return { autoload: false, options: { apiKey: managedKey, baseURL, headers } } + return { autoload: false, options: { apiKey: managedKey, baseURL, headers }, source: "managed" } } // Neither an own key nor a managed route — nothing to route with. @@ -949,7 +963,7 @@ export namespace Provider { .object({ id: z.string(), name: z.string(), - source: z.enum(["env", "config", "custom", "api"]), + source: z.enum(["env", "config", "custom", "api", "managed"]), env: z.string().array(), key: z.string().optional(), options: z.record(z.string(), z.any()), @@ -1519,14 +1533,33 @@ export namespace Provider { if (result && (result.autoload || providers[providerID])) { if (result.getModel) modelLoaders[providerID] = result.getModel const opts = result.options ?? {} - const patch: Partial = providers[providerID] ? { options: opts } : { source: "custom", options: opts } + // A loader-reported source (e.g. openrouter's managed-proxy branch) + // always wins, even when an earlier stage (env/api) already + // registered the provider under a different source — that earlier + // credential is exactly what the managed route is overriding. + // Absent an explicit source, keep the existing rule: "custom" only + // when this is the provider's first registration. + const patch: Partial = providers[providerID] + ? { options: opts, ...(result.source ? { source: result.source } : {}) } + : { source: result.source ?? "custom", options: opts } mergeProvider(providerID, patch) } } // load config for (const [providerID, provider] of configProviders) { - const partial: Partial = { source: "config" } + // A provider already registered by an earlier stage under a genuinely + // credential-derived source (env/api/managed) must not be relabeled — + // a `config.provider` entry that only supplies a `whitelist`, `name`, + // etc. is not where the credential came from. "custom" is excluded from + // this protection: it's loader-assigned (not credential-derived) and an + // autoloaded provider (AWS-profile Bedrock, google-vertex, synsci, + // cloudflare-ai-gateway, gitlab, sap-ai-core, ...) that also appears in + // config.provider for its whitelist has always been, and must stay, + // "config" here. + const credentialSource = providers[providerID]?.source + const claimed = credentialSource === "env" || credentialSource === "api" || credentialSource === "managed" + const partial: Partial = claimed ? {} : { source: "config" } if (provider.env) partial.env = provider.env if (provider.name) partial.name = provider.name if (provider.options) partial.options = provider.options @@ -1547,7 +1580,30 @@ export namespace Provider { // credential the user never brought. BYOK must use the user's OWN keys // only; auto-detect (billing unset) is left alone so a thk_ key can still // resolve to managed there. - if (config.billing?.llm === "byok" && isAtlasApiKey(effectiveKey(provider))) { + if (config.billing?.llm === "byok" && Auth.isAtlasApiKey(effectiveKey(provider))) { + delete providers[providerID] + continue + } + + // The managed mirror of the guard above. Under an EXPLICIT managed + // toggle the OpenRouter loader declines to route on a stored own key, + // but declining is not enough on its own: "load apikeys" already stamped + // provider.key from auth.json, and getSDK picks that up with baseURL + // falling back to public OpenRouter. A user whose Atlas session lapsed + // would keep chatting on their OWN key while the toggle still reads + // "Managed" and the wallet is never touched. Drop the provider instead — + // seeing no OpenRouter models is honest, silently spending a BYOK key is + // not. Exempt the two provider classes this file already treats as + // BYOK-by-design, since neither can debit the wallet and both are + // deliberately kept in managed mode: the user's own ChatGPT subscription + // (see isProviderAllowed) and anything served from their own machine + // (see isLocalBaseURL). Auto-detect (billing unset / null) and byok never + // reach this branch. + const exempt = + providerID === "openai-codex" || + localProviderIds.has(providerID) || + isLocalBaseURL(provider.options?.["baseURL"]) + if (managedCuratedProvidersOnly && !exempt && isByokKey(effectiveKey(provider))) { delete providers[providerID] continue } diff --git a/backend/cli/src/server/project-selection.ts b/backend/cli/src/server/project-selection.ts index 01307734..e79b7e0b 100644 --- a/backend/cli/src/server/project-selection.ts +++ b/backend/cli/src/server/project-selection.ts @@ -38,10 +38,14 @@ export async function projectSelection( text(input.directory) ?? text(context.req.query("directory")) ?? text(context.req.header("x-openscience-directory")) const directory = decode(raw) - if (projectID) return Project.resolve(projectID, directory) + if (projectID) return { ...(await Project.resolve(projectID, directory)), selector: directory } return { project: undefined, directory: directory ? Project.canonicalize(directory) : undefined, alias: undefined, + // The caller-supplied root before canonicalization, so routes that mint an + // instance can reject it while the folder picker keeps reporting its own + // friendlier "path not found". + selector: directory, } } diff --git a/backend/cli/src/server/routes/settings/billing.ts b/backend/cli/src/server/routes/settings/billing.ts index b551c5b2..d27d48be 100644 --- a/backend/cli/src/server/routes/settings/billing.ts +++ b/backend/cli/src/server/routes/settings/billing.ts @@ -3,6 +3,7 @@ import { describeRoute, resolver, validator } from "hono-openapi" import z from "zod" import { Config } from "../../../config/config" import { OpenScience } from "../../../openscience" +import { Provider } from "../../../provider/provider" import { lazy } from "../../../util/lazy" import { Log } from "../../../util/log" @@ -74,6 +75,12 @@ export const BillingSettingsRoutes = lazy(() => // Persist only the delta. updateGlobal deep-merges into the raw file; // writing back Config.getGlobal() would bake resolved {env:}/{file:} // secrets into openscience.json in plaintext. + // Config.updateGlobal awaits its own per-directory config-cache + // invalidation (Instance.disposeAll()) AND drops the memoized provider + // map before announcing the disposal, so both the new billing.llm and + // a provider map rebuilt under it are visible to the next reader by + // the time this line completes — no separate disposeAll() or + // Provider.invalidate() needed here (see disposeGlobalInstances). await Config.updateGlobal({ billing: patch }) log.info("update", { keys: Object.keys(patch) }) @@ -89,6 +96,18 @@ export const BillingSettingsRoutes = lazy(() => await OpenScience.syncServices().catch((e) => log.warn("resync after billing change failed", { error: e instanceof Error ? e.message : String(e) }), ) + // syncServices() (openscience/index.ts) writes fresh credentials + // into process.env (e.g. OPENROUTER_API_KEY / OPENROUTER_BASE_URL) + // - invalidate() exists specifically to pick up env a background + // sync just wrote, so it must run again AFTER this, not just + // before. Without this second call, a Provider.list() that lands + // between the two calls above and this point (e.g. the frontend's + // global.disposed listener re-fetching providers while this + // request is still awaiting the Atlas round-trip) rebuilds and + // re-memoizes the cache from the PRE-sync env, and the synced + // credential is invisible until another auth.set, another billing + // PUT, or a restart. + Provider.invalidate() } return c.json(await readState()) }, diff --git a/backend/cli/src/server/server.ts b/backend/cli/src/server/server.ts index eac37244..fb78a13f 100644 --- a/backend/cli/src/server/server.ts +++ b/backend/cli/src/server/server.ts @@ -92,6 +92,18 @@ export namespace Server { () => // TODO: Break server.ts into smaller route files to fix type inference app + // 404/410 and friends are cacheable by default (RFC 7231 §6.1), and a + // JSON body with no Cache-Control is fair game for heuristic caching + // too. A browser that cached one stale-project 410 for /provider then + // answered every later request from its own cache — the server saw no + // traffic at all while the app stayed broken across restarts and + // reloads. Applied to JSON only, so the SPA's hashed assets keep their + // caching. + .use(async (c, next) => { + await next() + if (!c.res.headers.get("content-type")?.includes("application/json")) return + c.res.headers.set("cache-control", "no-store") + }) .onError((err, c) => { log.error("failed", { error: err, @@ -107,6 +119,7 @@ export namespace Server { else if (err.name === "ProjectUnknownError") status = 404 else if (err.name === "ProjectStaleError") status = 410 else if (err.name === "ProjectMismatchError") status = 409 + else if (err.name === "ProjectDirectoryError") status = 400 else if (err.name === "ProjectTrustDeniedError") status = 403 else if (err.name === "ProjectTrustRootMismatchError") status = 409 else if (err.name === "ExecutionAuthorityDeniedError") status = 403 @@ -214,6 +227,12 @@ export namespace Server { await Auth.set(providerID, info) // Don't depend on the client remembering to call global.sync — // stale provider state would keep serving the old credential. + // Auth.set writes the auth FILE, which no config write covers, so + // this call is still the one that makes the new key visible. (When + // it also flips billing.llm managed -> byok, Config's global write + // has already invalidated before announcing the disposal — see + // disposeGlobalInstances — so the SPA refetch that event triggers + // cannot beat us to a stale re-memoisation.) Provider.invalidate() return c.json(true) }, @@ -259,6 +278,7 @@ export namespace Server { .route("/api/repo", RepoRoutes()) .use(async (c, next) => { const selected = await projectSelection(c) + if (selected.selector) await Project.assertDirectory(selected.selector) const directory = selected.directory ?? process.cwd() return Instance.provide({ directory, diff --git a/backend/cli/src/storage/storage.ts b/backend/cli/src/storage/storage.ts index f759b5db..b4da3739 100644 --- a/backend/cli/src/storage/storage.ts +++ b/backend/cli/src/storage/storage.ts @@ -1,6 +1,7 @@ import { Log } from "../util/log" import path from "path" import fs from "fs/promises" +import { randomUUID } from "crypto" import { Global } from "../global" import { Filesystem } from "../util/filesystem" import { lazy } from "../util/lazy" @@ -180,6 +181,24 @@ export namespace Storage { }) } + /** + * Publish a record by rename. Lock is an in-process map, so it orders writers + * inside one process and nothing at all between processes — and several + * openscience processes share this directory routinely (a CLI run alongside a + * running server; `Project.fromDirectory` rewrites a record on every instance + * creation). A plain write truncates in place, so a reader in another process + * can observe a half-written file and fail to parse it. Rename is atomic, so + * every reader sees either the old record or the new one. + */ + async function publish(target: string, content: string) { + const tmp = `${target}.${process.pid}.${randomUUID()}.tmp` + await Bun.write(tmp, content) + await fs.rename(tmp, target).catch(async (error) => { + await fs.unlink(tmp).catch(() => {}) + throw error + }) + } + export async function update(key: string[], fn: (draft: T) => void) { const dir = await state().then((x) => x.dir) const target = path.join(dir, ...key) + ".json" @@ -187,7 +206,7 @@ export namespace Storage { using _ = await Lock.write(target) const content = await Bun.file(target).json() fn(content) - await Bun.write(target, JSON.stringify(content, null, 2)) + await publish(target, JSON.stringify(content, null, 2)) return content as T }) } @@ -197,7 +216,7 @@ export namespace Storage { const target = path.join(dir, ...key) + ".json" return withErrorHandling(async () => { using _ = await Lock.write(target) - await Bun.write(target, JSON.stringify(content, null, 2)) + await publish(target, JSON.stringify(content, null, 2)) }) } @@ -212,7 +231,16 @@ export namespace Storage { }) } - const glob = new Bun.Glob("**/*") + // Records are `.json` files and nothing else. `publish` stages every write as + // a sibling `...tmp` in the same directory — it has to, + // since rename is only atomic within one filesystem — so a bare `**/*` also + // matched the staging file during the write→rename window, and permanently + // when a writer died in between (nothing sweeps them). Every caller here + // strips a fixed 5 characters assuming ".json", so those entries became + // phantom keys whose `read` throws NotFoundError. Matching on the suffix + // makes that assumption true by construction, and unlike relocating temp + // files it also hides debris already left on disk by earlier runs. + const glob = new Bun.Glob("**/*.json") export async function list(prefix: string[]) { const dir = await state().then((x) => x.dir) try { diff --git a/backend/cli/test/auth/billing-flip.test.ts b/backend/cli/test/auth/billing-flip.test.ts new file mode 100644 index 00000000..e80a738d --- /dev/null +++ b/backend/cli/test/auth/billing-flip.test.ts @@ -0,0 +1,128 @@ +import { test, expect, afterEach } from "bun:test" +import path from "path" +import fs from "fs/promises" +import { Auth } from "../../src/auth" +import { Config } from "../../src/config/config" +import { Global } from "../../src/global" + +// Auth.set is the ONE choke point both `openscience auth login` (CLI - calls +// Auth.set directly, see cli/cmd/auth.ts) and the Settings UI +// (PUT /auth/:providerID -> Auth.set) go through, so the billing.llm flip +// lives there (auth/index.ts), not in the HTTP route. These tests call +// Auth.set directly rather than going through the server, since that's the +// actual behavior under test. + +// Config.global memoizes for the lifetime of the process (shared across every +// test file in this `bun test` run) - each test establishes its own +// precondition via Config.updateGlobal (which resets that cache) rather than +// assuming a blank slate. +// +// Cleanup removes all three candidate global config filenames +// (config.ts globalConfigFile() picks from openscience.jsonc / +// openscience.json / config.json, defaulting to creating the first when none +// exist yet) and resets the in-memory Config.global cache, so a later test in +// this file - or a later file in the same `bun test` run - does not inherit +// a flipped mode or a stray file that shadows another candidate. +async function resetGlobalConfig() { + for (const name of ["openscience.jsonc", "openscience.json", "config.json"]) { + await fs.rm(path.join(Global.Path.config, name), { force: true }).catch(() => {}) + } + Config.global.reset() +} + +afterEach(async () => { + await resetGlobalConfig() + await Auth.remove("openrouter").catch(() => {}) + await Auth.remove("anthropic").catch(() => {}) +}) + +test("a non-thk_ OpenRouter key added while billing.llm is managed flips the toggle to byok", async () => { + await Config.updateGlobal({ billing: { llm: "managed" } }) + + await Auth.set("openrouter", { type: "api", key: "sk-or-user-owned-key" }) + + const cfg = await Config.getGlobal() + expect(cfg.billing?.llm).toBe("byok") + // Never delete or rewrite the user's stored key - just the mode. + expect(await Auth.get("openrouter")).toEqual({ type: "api", key: "sk-or-user-owned-key" }) +}) + +test("a thk_ OpenRouter key added while billing.llm is managed does not flip the mode", async () => { + await Config.updateGlobal({ billing: { llm: "managed" } }) + + await Auth.set("openrouter", { type: "api", key: "thk_atlas-managed-token" }) + + const cfg = await Config.getGlobal() + expect(cfg.billing?.llm).toBe("managed") +}) + +test("an OAuth credential for openrouter while billing.llm is managed does not flip the mode", async () => { + await Config.updateGlobal({ billing: { llm: "managed" } }) + + await Auth.set("openrouter", { type: "oauth", refresh: "refresh-token", access: "access-token", expires: 123 }) + + const cfg = await Config.getGlobal() + expect(cfg.billing?.llm).toBe("managed") +}) + +test("a key added for a different provider does not flip the mode", async () => { + await Config.updateGlobal({ billing: { llm: "managed" } }) + + await Auth.set("anthropic", { type: "api", key: "sk-ant-user-owned-key" }) + + const cfg = await Config.getGlobal() + expect(cfg.billing?.llm).toBe("managed") +}) + +test("an OpenRouter key added while billing.llm is null (auto) does not write the config", async () => { + await Config.updateGlobal({ billing: { llm: null } }) + + await Auth.set("openrouter", { type: "api", key: "sk-or-user-owned-key" }) + + const cfg = await Config.getGlobal() + expect(cfg.billing?.llm ?? null).toBeNull() +}) + +test("an OpenRouter key added while billing.llm is byok does not write the config again", async () => { + // Seed openscience.json directly (bypassing Config.updateGlobal) rather + // than reading Config.getGlobal() afterwards - a plain re-read-and-compare + // would pass even if the "=== managed" gate were deleted entirely (byok + // written over byok is still byok). Byte comparison pins that Auth.set + // does not touch the file at all in this case, but needs the seed shaped + // to dodge two unrelated quirks in the write path: + // (1) config.ts's load() adds a missing $schema field to the file as an + // incidental side effect of ANY read (Auth.set's gate check itself + // calls Config.getGlobal()) - include it up front so that add doesn't + // masquerade as "Auth.set wrote something" below. + // (2) Config.updateGlobal's .jsonc branch patches the document + // surgically (jsonc-parser) and leaves an unchanged value's + // formatting untouched - "wrote byok over byok" and "wrote nothing" + // would be byte-identical there. .json's branch always re-serializes + // the whole object (JSON.stringify(merged, null, 2)), so a real write + // changes bytes even for an unchanged value - use that file. + const file = path.join(Global.Path.config, "openscience.json") + await fs.mkdir(Global.Path.config, { recursive: true }) + const seed = JSON.stringify({ $schema: "https://syntheticsciences.ai/config.json", billing: { llm: "byok" } }) + await fs.writeFile(file, seed) + Config.global.reset() + + await Auth.set("openrouter", { type: "api", key: "sk-or-user-owned-key" }) + + expect(await fs.readFile(file, "utf8")).toBe(seed) +}) + +test("an OpenRouter key added while the global config is malformed still persists the key and does not throw", async () => { + // A doubled trailing comma - jsonc-parser tolerates a single trailing + // comma (config.ts passes allowTrailingComma: true) but not two in a row - + // stands in for a hand-edited openscience.jsonc gone wrong. That makes + // Config.getGlobal() throw; the flip in Auth.set is wrapped in try/catch + // specifically so this doesn't take the key write down with it. + const file = path.join(Global.Path.config, "openscience.jsonc") + await fs.mkdir(Global.Path.config, { recursive: true }) + await fs.writeFile(file, `{ "billing": { "llm": "managed" },, }`) + Config.global.reset() + + await Auth.set("openrouter", { type: "api", key: "sk-or-user-owned-key" }) + + expect(await Auth.get("openrouter")).toEqual({ type: "api", key: "sk-or-user-owned-key" }) +}) diff --git a/backend/cli/test/config/config.test.ts b/backend/cli/test/config/config.test.ts index 60f0cf82..facff740 100644 --- a/backend/cli/test/config/config.test.ts +++ b/backend/cli/test/config/config.test.ts @@ -1672,3 +1672,62 @@ describe("OPENSCIENCE_DISABLE_PROJECT_CONFIG", () => { } }) }) + +// A global config write must be visible to the very next Config.get(), even +// for a project directory that was already instantiated before the write +// (i.e. an open session, not a fresh process). Config.get() is backed by a +// per-directory cache (config.ts's `state`, via Instance.state) that is only +// invalidated by Instance.dispose()/disposeAll() - resetting the `global` +// lazy singleton alone is not enough. The global-config writers used to fire +// that disposal without awaiting it (`void Instance.disposeAll().catch(...)`), +// so a caller who read Config.get() for a directory, then wrote global +// config, then immediately read Config.get() again for the SAME directory, +// could still observe the pre-write value. disposeGlobalInstances() now +// awaits it. +describe("global config writes are visible to the next Config.get()", () => { + async function cleanGlobalConfig() { + for (const name of ["openscience.jsonc", "openscience.json", "config.json"]) { + await fs.rm(path.join(Global.Path.config, name), { force: true }).catch(() => {}) + } + Config.global.reset() + } + + afterEach(cleanGlobalConfig) + + test("Config.updateGlobal", async () => { + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const before = await Config.get() + expect(before.billing?.llm).toBeUndefined() + + // No sleep, no retry, no intervening await besides the write itself + // - this is the exact race: Instance.disposeAll() used to be fired + // without being awaited inside Config.updateGlobal, so the very + // next Config.get() for this SAME already-instantiated directory + // could still return the pre-write value. + await Config.updateGlobal({ billing: { llm: "managed" } }) + + const after = await Config.get() + expect(after.billing?.llm).toBe("managed") + }, + }) + }) + + test("Config.setSandbox (patchConfigPath's global branch, shared by setMcp/setProvider/unsetGlobal)", async () => { + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const before = await Config.get() + expect(before.sandbox?.network).not.toBe("allow") + + await Config.setSandbox({ network: "allow" }) + + const after = await Config.get() + expect(after.sandbox?.network).toBe("allow") + }, + }) + }) +}) diff --git a/backend/cli/test/fixture/fixture.ts b/backend/cli/test/fixture/fixture.ts index e0d74227..d2da6503 100644 --- a/backend/cli/test/fixture/fixture.ts +++ b/backend/cli/test/fixture/fixture.ts @@ -39,7 +39,7 @@ export async function tmpdir(options?: TmpDirOptions) { const result = { [Symbol.asyncDispose]: async () => { await options?.dispose?.(dirpath) - // await fs.rm(dirpath, { recursive: true, force: true }) + await fs.rm(dirpath, { recursive: true, force: true }) }, path: realpath, extra: extra as T, diff --git a/backend/cli/test/fixture/spawn.test.ts b/backend/cli/test/fixture/spawn.test.ts new file mode 100644 index 00000000..95da8e76 --- /dev/null +++ b/backend/cli/test/fixture/spawn.test.ts @@ -0,0 +1,32 @@ +import { expect, test } from "bun:test" +import os from "os" +import { spawn } from "./spawn" + +// Bun.spawn defaults to the environment the process was started with, so the XDG +// overrides preload assigns at runtime never reached a child. Children that boot +// the CLI wrote projects and sessions into the developer's real +// ~/.local/share/openscience, which showed up as phantom entries on their home list. +test("gives a spawned child the sandboxed data directory", async () => { + const proc = spawn([process.execPath, "-e", "process.stdout.write(process.env.XDG_DATA_HOME ?? '')"], { + stdout: "pipe", + }) + const output = await new Response(proc.stdout).text() + await proc.exited + + expect(output).toBe(process.env["XDG_DATA_HOME"]!) + expect(output).not.toStartWith(os.homedir()) +}) + +test("keeps caller overrides on top of the sandboxed environment", async () => { + const proc = spawn( + [process.execPath, "-e", "process.stdout.write(`${process.env.HOME}|${process.env.XDG_DATA_HOME}`)"], + { + stdout: "pipe", + env: { HOME: "/tmp/elsewhere" }, + }, + ) + const output = await new Response(proc.stdout).text() + await proc.exited + + expect(output).toBe(`/tmp/elsewhere|${process.env["XDG_DATA_HOME"]}`) +}) diff --git a/backend/cli/test/fixture/spawn.ts b/backend/cli/test/fixture/spawn.ts new file mode 100644 index 00000000..5d1e8344 --- /dev/null +++ b/backend/cli/test/fixture/spawn.ts @@ -0,0 +1,23 @@ +/** + * Spawn a child that stays inside the suite's sandbox. + * + * `Bun.spawn` inherits the environment the test runner was launched with, not + * the one preload.ts assembles at import time — so a child booting the CLI + * resolved XDG paths against the developer's real home and wrote projects, + * sessions and auth into it. Always hand children the live `process.env`. + */ +export function spawn< + const In extends Bun.SpawnOptions.Writable = "ignore", + const Out extends Bun.SpawnOptions.Readable = "pipe", + const Err extends Bun.SpawnOptions.Readable = "inherit", +>( + command: string[], + options: Omit, "env"> & { + env?: Record + } = {}, +) { + return Bun.spawn(command, { + ...options, + env: { ...process.env, ...options.env }, + } as Bun.SpawnOptions.OptionsObject) +} diff --git a/backend/cli/test/mcp/inspect.test.ts b/backend/cli/test/mcp/inspect.test.ts index e0317ada..0d19f630 100644 --- a/backend/cli/test/mcp/inspect.test.ts +++ b/backend/cli/test/mcp/inspect.test.ts @@ -1,5 +1,6 @@ import { expect, test } from "bun:test" import { tmpdir } from "../fixture/fixture" +import { spawn } from "../fixture/spawn" test("inspect reports capabilities from a real local MCP server", async () => { await using tmp = await tmpdir() @@ -38,7 +39,7 @@ process.exit(0) `, ) - const proc = Bun.spawn([process.execPath, runner, tmp.path], { + const proc = spawn([process.execPath, runner, tmp.path], { cwd: tmp.path, stdout: "pipe", stderr: "pipe", diff --git a/backend/cli/test/preload.ts b/backend/cli/test/preload.ts index 074630ee..abdaa9a8 100644 --- a/backend/cli/test/preload.ts +++ b/backend/cli/test/preload.ts @@ -25,6 +25,12 @@ process.env["XDG_DATA_HOME"] = path.join(dir, "share") process.env["XDG_CACHE_HOME"] = path.join(dir, "cache") process.env["XDG_CONFIG_HOME"] = path.join(dir, "config") process.env["XDG_STATE_HOME"] = path.join(dir, "state") +// global/index.ts prefers OPENSCIENCE_CONFIG_DIR over XDG_CONFIG_HOME +// (Global.Path.config). A developer with that override set in their own +// shell (e.g. `export OPENSCIENCE_CONFIG_DIR=~/.config/openscience`) would +// otherwise have tests read AND write their real config directory instead +// of the isolated one above - delete it so XDG_CONFIG_HOME always wins here. +delete process.env["OPENSCIENCE_CONFIG_DIR"] // The Atlas CLI does not use XDG_CONFIG_HOME for this override; OpenScience's // session writer otherwise falls back to the real ~/.config/atlas-cli path. // Keep the companion CLI credential inside the same throwaway test sandbox. diff --git a/backend/cli/test/provider/inference.test.ts b/backend/cli/test/provider/inference.test.ts index 9719888e..e0859491 100644 --- a/backend/cli/test/provider/inference.test.ts +++ b/backend/cli/test/provider/inference.test.ts @@ -15,4 +15,10 @@ test("classifies the observable inference route without exposing credentials", ( expect(Inference.classify({ providerID: "anthropic", providerSource: "api", auth: "api" })).toBe("byok") expect(Inference.classify({ providerID: "github-copilot", providerSource: "custom", auth: "oauth" })).toBe("oauth") expect(Inference.classify({ providerID: "custom", providerSource: "custom" })).toBe("unknown") + // Auto-detect (billing.llm unset/null): no explicit "managed" opt-in, so the + // billing === "managed" shortcut above doesn't fire, and there's no stored + // own key (no `auth`) — the route is still genuinely managed (a synced thk_ + // token, no BYOK key), and provider.source already says so. Must not fall + // through to "unknown". + expect(Inference.classify({ providerID: "openrouter", providerSource: "managed" })).toBe("managed") }) diff --git a/backend/cli/test/provider/managed-routing.test.ts b/backend/cli/test/provider/managed-routing.test.ts index ca9be98b..578de4cf 100644 --- a/backend/cli/test/provider/managed-routing.test.ts +++ b/backend/cli/test/provider/managed-routing.test.ts @@ -23,8 +23,16 @@ mock.module("@gitlab/openscience-gitlab-auth", () => ({ default: mockPlugin })) import { tmpdir } from "../fixture/fixture" import { Instance } from "../../src/project/instance" import { Provider } from "../../src/provider/provider" +import { Inference } from "../../src/provider/inference" import { Env } from "../../src/env" +import { Auth } from "../../src/auth" import { API_BASE } from "../../src/openscience" +import { Config } from "../../src/config/config" +import { BillingSettingsRoutes } from "../../src/server/routes/settings/billing" +import { Global } from "../../src/global" +import { GlobalBus } from "../../src/bus/global" +import path from "path" +import fs from "fs/promises" function clearManagedLLMEnv() { for (const key of [ @@ -347,3 +355,468 @@ describe("managed session availability", () => { }) }) }) + +// ── billing.llm gates the own-key vs managed-proxy route (1a/1b/1c) ───────── + +/** Auth.json lives outside the per-test tmp project (it's keyed by XDG dirs + * isolated for the whole test *run*, not per test) — set and restore it like + * provider.test.ts's Codex OAuth cases so a stored own key never leaks + * across tests. */ +async function withOpenRouterOwnKey(key: string, fn: () => Promise): Promise { + const previous = await Auth.get("openrouter") + await Auth.set("openrouter", { type: "api", key }) + try { + return await fn() + } finally { + if (previous) await Auth.set("openrouter", previous) + else await Auth.remove("openrouter") + } +} + +describe("billing.llm gates OpenRouter's own-key vs managed-proxy route (1a/1b/1c)", () => { + test('managed: a stored own key is overridden — routes to the Atlas proxy with the thk_ token, and reports source "managed"; the own key is untouched in auth', async () => { + await withOpenRouterOwnKey("sk-or-own-key", async () => { + await using tmp = await tmpdir({ config: { billing: { llm: "managed" } } }) + await Instance.provide({ + directory: tmp.path, + init: async () => { + clearManagedLLMEnv() + Env.set("OPENROUTER_API_KEY", "thk_openrouter") + Env.set("OPENROUTER_BASE_URL", `${PROXY}/openrouter/v1`) + Provider.invalidate() + }, + fn: async () => { + const openrouter = (await Provider.list())["openrouter"] + expect(openrouter).toBeDefined() + // 1a: managed spend wins over the stored own key — Atlas proxy, thk_ token. + expect(openrouter.options.baseURL).toBe(`${PROXY}/openrouter/v1`) + expect(openrouter.options.baseURL).not.toBe("https://openrouter.ai/api/v1") + expect(openrouter.options.apiKey).toBe("thk_openrouter") + expect(openrouter.options.apiKey).not.toBe("sk-or-own-key") + // 1b: the managed route reports its true source. + expect(openrouter.source).toBe("managed") + }, + }) + // 1a: the own key is retained (never deleted/rewritten), just unused. + expect(await Auth.get("openrouter")).toEqual({ type: "api", key: "sk-or-own-key" }) + }) + }) + + test("managed with NO managed credential: the provider is dropped rather than silently billing the stored own key", async () => { + // The lapsed-Atlas-session case. billing.llm is explicitly "managed" and + // an own key sits in auth.json, but there is no thk_ token anywhere (no + // env, and OpenScience.getSession() cannot reach the hermetic API base + // from test/preload.ts). The loader returns headers only — no credential — + // so without the guard the "load apikeys" stage's provider.key survives + // into getSDK and pays for managed-labelled traffic out of the user's own + // pocket. No OpenRouter models is the honest outcome. + await withOpenRouterOwnKey("sk-or-own-key", async () => { + await using tmp = await tmpdir({ + config: { + billing: { llm: "managed" }, + provider: { openrouter: { whitelist: ["anthropic/claude-sonnet-5"] } }, + }, + }) + await Instance.provide({ + directory: tmp.path, + init: async () => { + clearManagedLLMEnv() + Provider.invalidate() + }, + fn: async () => { + expect((await Provider.list())["openrouter"]).toBeUndefined() + }, + }) + // The key itself is untouched — dropping the provider is a routing + // decision, not a credential edit. + expect(await Auth.get("openrouter")).toEqual({ type: "api", key: "sk-or-own-key" }) + }) + }) + + test("managed leaves a local provider alone even though its key reads as BYOK", async () => { + // Ollama's config block carries `apiKey: "local"`, which is a non-managed, + // non-"public" credential and therefore BYOK by isByokKey. It runs on the + // user's own hardware and is free, so the managed guard must not reach it — + // the same exemption isProviderAllowed already makes. + await using tmp = await tmpdir({ + config: { + billing: { llm: "managed" }, + provider: { + ollama: { + name: "Ollama (local)", + npm: "@ai-sdk/openai-compatible", + options: { baseURL: "http://localhost:11434/v1", apiKey: "local" }, + models: { "llama3.1": { name: "llama3.1", limit: { context: 8192, output: 2048 } } }, + }, + }, + }, + }) + await Instance.provide({ + directory: tmp.path, + init: async () => { + clearManagedLLMEnv() + Provider.invalidate() + }, + fn: async () => { + expect((await Provider.list())["ollama"]).toBeDefined() + }, + }) + }) + + test("auto-detect (billing.llm unset) with a stored own key and no managed credential keeps the provider — the guard is opt-in only", async () => { + // The legacy path this fix must not touch: identical to the managed case + // above except billing.llm is unset. Nothing is dropped, and the own key + // still routes to public OpenRouter. + await withOpenRouterOwnKey("sk-or-own-key", async () => { + await using tmp = await tmpdir({ + config: { provider: { openrouter: { whitelist: ["anthropic/claude-sonnet-5"] } } }, + }) + await Instance.provide({ + directory: tmp.path, + init: async () => { + clearManagedLLMEnv() + Provider.invalidate() + }, + fn: async () => { + const openrouter = (await Provider.list())["openrouter"] + expect(openrouter).toBeDefined() + expect(openrouter.options.apiKey).toBe("sk-or-own-key") + expect(openrouter.options.baseURL).toBe("https://openrouter.ai/api/v1") + }, + }) + }) + }) + + test('byok: a stored own key routes to public OpenRouter and reports source "api" (regression guard for 1c)', async () => { + await withOpenRouterOwnKey("sk-or-own-key", async () => { + // config.provider.openrouter must be genuinely present (here, via a + // synced whitelist) for this to actually exercise 1c — otherwise the + // "load config" loop never iterates openrouter at all and this test + // would pass even without the 1c fix. + await using tmp = await tmpdir({ + config: { + billing: { llm: "byok" }, + provider: { openrouter: { whitelist: ["anthropic/claude-sonnet-5"] } }, + }, + }) + await Instance.provide({ + directory: tmp.path, + init: async () => { + clearManagedLLMEnv() + Provider.invalidate() + }, + fn: async () => { + const openrouter = (await Provider.list())["openrouter"] + expect(openrouter).toBeDefined() + expect(openrouter.options.apiKey).toBe("sk-or-own-key") + expect(openrouter.options.baseURL).toBe("https://openrouter.ai/api/v1") + expect(openrouter.source).toBe("api") + }, + }) + }) + }) + + test("auto-detect (billing.llm unset): a stored own key still wins — unchanged from today's behaviour", async () => { + await withOpenRouterOwnKey("sk-or-own-key", async () => { + await using tmp = await tmpdir({ config: {} }) + await Instance.provide({ + directory: tmp.path, + init: async () => { + clearManagedLLMEnv() + Provider.invalidate() + }, + fn: async () => { + const openrouter = (await Provider.list())["openrouter"] + expect(openrouter).toBeDefined() + expect(openrouter.options.apiKey).toBe("sk-or-own-key") + expect(openrouter.options.baseURL).toBe("https://openrouter.ai/api/v1") + expect(openrouter.source).toBe("api") + }, + }) + }) + }) + + test('auto-detect (billing.llm unset) with a synced thk_ token and no own key genuinely IS managed — source "managed" and Inference.classify "managed", not "unknown"', async () => { + await using tmp = await tmpdir({ config: {} }) + await Instance.provide({ + directory: tmp.path, + init: async () => { + clearManagedLLMEnv() + Env.set("OPENROUTER_API_KEY", "thk_openrouter") + Env.set("OPENROUTER_BASE_URL", `${PROXY}/openrouter/v1`) + Provider.invalidate() + }, + fn: async () => { + const openrouter = (await Provider.list())["openrouter"] + expect(openrouter).toBeDefined() + expect(openrouter.options.apiKey).toBe("thk_openrouter") + expect(openrouter.options.baseURL).toBe(`${PROXY}/openrouter/v1`) + expect(openrouter.source).toBe("managed") + + // Feed the REAL provider.source this fixture produced straight into + // classify — not Inference.resolve(), whose baseURL fallback would + // resolve through this test harness's loopback OPENSCIENCE_API_BASE + // (test/preload.ts) and get short-circuited by the unrelated + // local()-heuristic before ever reaching the providerSource check. + expect(Inference.classify({ providerID: "openrouter", providerSource: openrouter.source })).toBe("managed") + }, + }) + }) + + test('1c must not overshoot: a provider whose key genuinely comes from config.provider still reports source "config"', async () => { + // No env var, no stored auth key, no billing toggle — OpenRouter's own + // custom loader declines to register the provider (nothing to route + // with), so the config loop below is the FIRST and only stage to claim + // it. That's the "genuinely config" case the asymmetry note describes. + await using tmp = await tmpdir({ + config: { + provider: { + openrouter: { + options: { apiKey: "config-owned-key" }, + }, + }, + }, + }) + await Instance.provide({ + directory: tmp.path, + init: async () => { + clearManagedLLMEnv() + Provider.invalidate() + }, + fn: async () => { + const openrouter = (await Provider.list())["openrouter"] + expect(openrouter).toBeDefined() + expect(openrouter.options.apiKey).toBe("config-owned-key") + expect(openrouter.source).toBe("config") + }, + }) + }) + + test('1c must not overshoot the other way: an env-registered provider that also appears in config.provider (for its whitelist) keeps source "env"', async () => { + await using tmp = await tmpdir({ + config: { + provider: { anthropic: { whitelist: ["claude-sonnet-4-6"] } }, + }, + }) + await Instance.provide({ + directory: tmp.path, + init: async () => { + clearManagedLLMEnv() + Env.set("ANTHROPIC_API_KEY", "sk-ant-env-key") + Provider.invalidate() + }, + fn: async () => { + const anthropic = (await Provider.list())["anthropic"] + expect(anthropic).toBeDefined() + expect(anthropic.key).toBe("sk-ant-env-key") + // The config entry only supplies a whitelist — the credential is genuinely env's. + expect(anthropic.source).toBe("env") + }, + }) + }) + + test('1c (narrowed): an autoloaded custom-loader provider that also appears in config.provider (for its whitelist) still reports source "config", not "custom"', async () => { + // google-vertex autoloads off GOOGLE_CLOUD_PROJECT alone (no auth.json + // entry, and its models.dev `env` array — GOOGLE_VERTEX_PROJECT etc. — + // never matches, so the "load env" stage never registers it either). + // CUSTOM_LOADERS is the first and only stage to register it, with + // source "custom" — exactly the loader-assigned (not credential-derived) + // case 1c must keep overwriting to "config" when a config.provider entry + // exists, per the narrowed protected set (env/api/managed only). + await using tmp = await tmpdir({ + config: { + provider: { "google-vertex": { whitelist: ["gemini-3.5-flash"] } }, + }, + }) + await Instance.provide({ + directory: tmp.path, + init: async () => { + clearManagedLLMEnv() + Env.set("GOOGLE_CLOUD_PROJECT", "test-project") + Provider.invalidate() + }, + fn: async () => { + try { + const vertex = (await Provider.list())["google-vertex"] + expect(vertex).toBeDefined() + expect(vertex.source).toBe("config") + } finally { + Env.remove("GOOGLE_CLOUD_PROJECT") + } + }, + }) + }) +}) + +// ── settings/billing.ts PUT invalidates the provider cache at runtime (2a) ── + +describe("billing PUT invalidates the provider cache — no restart needed", () => { + test("switching managed -> byok through the real route drops the thk_-keyed OpenRouter provider from the very next Provider.list()", async () => { + await using tmp = await tmpdir({ config: {} }) + try { + // Seed env once so the instance exists and OPENROUTER_* is in place - + // mirrors a project a user already has open before touching Settings. + await Instance.provide({ + directory: tmp.path, + init: async () => { + clearManagedLLMEnv() + Env.set("OPENROUTER_API_KEY", "thk_openrouter") + Env.set("OPENROUTER_BASE_URL", `${PROXY}/openrouter/v1`) + }, + fn: async () => {}, + }) + + // settings/billing.ts is mounted OUTSIDE the Instance.provide wrapper + // (see server.ts) - it never runs with an ambient project directory in + // production, so drive it the same way here rather than nesting it + // inside an Instance.provide() call. + const toManaged = await BillingSettingsRoutes().request("/", { + method: "PUT", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ llm: "managed" }), + }) + expect(toManaged.status).toBe(200) + + // The next request for this project (a fresh Instance.provide(), just + // like a real incoming chat/inference call) sees it immediately. + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const before = (await Provider.list())["openrouter"] + expect(before).toBeDefined() + expect(before.source).toBe("managed") + }, + }) + + // Flip to byok through the same route. The guard at provider.ts:1585 + // drops any provider whose effective credential is a managed thk_ + // token once byok is explicit — so openrouter must be gone on the + // very next Provider.list() call, with no process restart and no + // manual Provider.invalidate() from the test itself (the route is + // responsible for that now). + const toByok = await BillingSettingsRoutes().request("/", { + method: "PUT", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ llm: "byok" }), + }) + expect(toByok.status).toBe(200) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const after = await Provider.list() + expect(after["openrouter"]).toBeUndefined() + }, + }) + } finally { + // The route writes to the GLOBAL config (not the tmpdir project config + // every other test in this file relies on) - remove all three + // candidate filenames Config.updateGlobal's globalConfigFile() can + // pick (see config.ts) and reset the in-memory Config.global cache, so + // a later test in this file (or a later file in the same `bun test` + // run) does not inherit a flipped mode or a stray file that shadows + // another candidate. + for (const name of ["openscience.json", "openscience.jsonc", "config.json"]) { + await fs.rm(path.join(Global.Path.config, name), { force: true }).catch(() => {}) + } + Config.global.reset() + } + }) +}) + +// ── the invalidation must precede the announcement, not follow it ──────────── + +describe("global config writes invalidate the provider cache before announcing", () => { + test("a listener that refetches on global.disposed sees the map rebuilt under the new config", async () => { + await using tmp = await tmpdir({ config: {} }) + try { + await Instance.provide({ + directory: tmp.path, + init: async () => { + clearManagedLLMEnv() + Env.set("OPENROUTER_API_KEY", "thk_openrouter") + Env.set("OPENROUTER_BASE_URL", `${PROXY}/openrouter/v1`) + Env.set("ANTHROPIC_API_KEY", "sk-ant-byok-key") + Provider.invalidate() + }, + fn: async () => { + // Prime the module-level memo the way a long-running server has it + // primed before the user ever opens Settings. + expect((await Provider.list())["anthropic"]).toBeDefined() + + // Stand in for the SPA's `global.disposed` handler, which fires + // GET /provider the moment the event arrives. GlobalBus.emit + // dispatches synchronously, so this listener runs at exactly the + // point inside disposeGlobalInstances() where the announcement + // happens — the narrowest possible version of the real window. + let observed: ReturnType | undefined + const listener = (e: { payload?: { type?: string } }) => { + if (e.payload?.type !== "global.disposed") return + observed = Provider.list() + } + GlobalBus.on("event", listener) + try { + await Config.updateGlobal({ billing: { llm: "managed" } }) + } finally { + GlobalBus.off("event", listener) + } + + expect(observed).toBeDefined() + // Managed routes curated providers only, so a map rebuilt under the + // config just written cannot contain the BYOK Anthropic key. Seeing + // it means the refetch was handed the pre-write memo — and nothing + // invalidates after the announcement, so it would stay that way. + const refetched = await observed! + expect(refetched["anthropic"]).toBeUndefined() + expect(refetched["openrouter"]).toBeDefined() + }, + }) + } finally { + for (const name of ["openscience.json", "openscience.jsonc", "config.json"]) { + await fs.rm(path.join(Global.Path.config, name), { force: true }).catch(() => {}) + } + Config.global.reset() + Provider.invalidate() + } + }) + + test("a global write with no route behind it (replaceGlobal) invalidates on its own", async () => { + await using tmp = await tmpdir({ config: {} }) + try { + await Instance.provide({ + directory: tmp.path, + init: async () => { + clearManagedLLMEnv() + Env.set("OPENROUTER_API_KEY", "thk_openrouter") + Env.set("OPENROUTER_BASE_URL", `${PROXY}/openrouter/v1`) + Env.set("ANTHROPIC_API_KEY", "sk-ant-byok-key") + Provider.invalidate() + }, + fn: async () => { + expect((await Provider.list())["anthropic"]).toBeDefined() + + // replaceGlobal has no HTTP route and no caller anywhere that + // follows it with Provider.invalidate() — so this passes only if + // the write itself carries the invalidation. Per-call-site patching + // cannot make it pass; nothing here is racing anything. + await Config.replaceGlobal( + JSON.stringify({ + $schema: "https://syntheticsciences.ai/config.json", + billing: { llm: "managed" }, + }), + ) + + const after = await Provider.list() + expect(after["anthropic"]).toBeUndefined() + expect(after["openrouter"]).toBeDefined() + }, + }) + } finally { + for (const name of ["openscience.json", "openscience.jsonc", "config.json"]) { + await fs.rm(path.join(Global.Path.config, name), { force: true }).catch(() => {}) + } + Config.global.reset() + Provider.invalidate() + } + }) +}) diff --git a/backend/cli/test/sandbox/sandbox.test.ts b/backend/cli/test/sandbox/sandbox.test.ts index beec591f..f1f3d40e 100644 --- a/backend/cli/test/sandbox/sandbox.test.ts +++ b/backend/cli/test/sandbox/sandbox.test.ts @@ -3,6 +3,7 @@ import fs from "fs" import os from "os" import path from "path" import { Sandbox } from "../../src/sandbox/sandbox" +import { tmpdir } from "../fixture/fixture" const shell = "/bin/sh" @@ -104,6 +105,31 @@ describe("Sandbox.bubblewrapArgs", () => { }) expect(args).not.toContain(file) }) + + test.skipIf(Sandbox.backend() !== "bubblewrap")("produces an argv bwrap actually accepts", async () => { + await using tmp = await tmpdir() + const present = path.join(tmp.path, "auth.json") + await Bun.write(present, "{}") + + // The missing mask target has to sit on the read-only bind, the way a real + // ~/.local/share credential file does — a path under the sandbox's own + // tmpfs would be creatable and hide the failure. + const missing = path.join(os.homedir(), `.openscience-absent-${process.pid}.json`) + const args = Sandbox.bubblewrapArgs({ + writable: [tmp.path], + unreadable: [present, missing], + network: false, + }) + const proc = Bun.spawn(["bwrap", ...args, "--", "/bin/echo", "ok"], { stdout: "pipe", stderr: "pipe" }) + const [out, error, exit] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]) + + expect(exit, error).toBe(0) + expect(out.trim()).toBe("ok") + }) }) describe("Sandbox.backend/describe", () => { diff --git a/backend/cli/test/server/no-store.test.ts b/backend/cli/test/server/no-store.test.ts new file mode 100644 index 00000000..d1c122f3 --- /dev/null +++ b/backend/cli/test/server/no-store.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, test } from "bun:test" +import { Server } from "../../src/server/server" +import { Log } from "../../src/util/log" + +Log.init({ print: false }) + +const fetch = Server.internalFetch() + +// 410/404 are cacheable by default (RFC 7231 §6.1), and the API sent no +// Cache-Control at all. A browser cached one stale-project 410 for /provider and +// then answered every later request from its own cache — the server saw no +// traffic while the app stayed broken through restarts and reloads. +describe("API responses are never cached", () => { + test("a successful JSON response is marked no-store", async () => { + const response = await fetch("http://openscience.internal/provider") + + expect(response.status).toBe(200) + expect(response.headers.get("cache-control")).toBe("no-store") + }) + + test("an error response is marked no-store", async () => { + const response = await fetch("http://openscience.internal/provider", { + headers: { "x-openscience-project": `prj_missing_${crypto.randomUUID()}` }, + }) + + expect(response.status).toBeGreaterThanOrEqual(400) + expect(response.headers.get("cache-control")).toBe("no-store") + }) +}) diff --git a/backend/cli/test/server/project-selection-routes.test.ts b/backend/cli/test/server/project-selection-routes.test.ts index e2d2485c..09c03a57 100644 --- a/backend/cli/test/server/project-selection-routes.test.ts +++ b/backend/cli/test/server/project-selection-routes.test.ts @@ -1,6 +1,7 @@ import { $ } from "bun" import { describe, expect, test } from "bun:test" import fs from "fs/promises" +import os from "os" import path from "path" import { Project } from "../../src/project/project" import { Server } from "../../src/server/server" @@ -221,6 +222,46 @@ describe("pre-instance project selection routes", () => { }) }) + // A stale deep link decodes into junk that is neither absolute nor a real + // folder. Registering a project for it put a phantom entry on the home list. + test("refuses to register a project for a directory that does not exist", async () => { + const missing = path.join(os.tmpdir(), `openscience-missing-${crypto.randomUUID()}`) + const before = await Project.list() + + const response = await fetch("http://openscience.internal/project/current", { + headers: { + "x-openscience-directory": missing, + }, + }) + + expect(response.status).toBe(400) + expect(await response.json()).toEqual({ + name: "ProjectDirectoryError", + data: { + directory: missing, + }, + }) + expect(await Project.list()).toHaveLength(before.length) + }) + + // Resolving one against the server's cwd silently opened whatever folder + // happened to sit next to it, so a relative selector is never honoured. + test("refuses a directory selector that is not absolute", async () => { + const response = await fetch("http://openscience.internal/project/current", { + headers: { + "x-openscience-directory": "codes", + }, + }) + + expect(response.status).toBe(400) + expect(await response.json()).toEqual({ + name: "ProjectDirectoryError", + data: { + directory: "codes", + }, + }) + }) + test("accepts body project selection for repository mutations", async () => { await using tmp = await tmpdir({ git: true }) const created = await Project.fromDirectory(tmp.path) diff --git a/backend/cli/test/server/settings-billing.test.ts b/backend/cli/test/server/settings-billing.test.ts index 6a2c5916..455a69e7 100644 --- a/backend/cli/test/server/settings-billing.test.ts +++ b/backend/cli/test/server/settings-billing.test.ts @@ -2,12 +2,21 @@ import { test, expect, afterEach } from "bun:test" import path from "path" import fs from "fs/promises" import { Global } from "../../src/global" +import { Config } from "../../src/config/config" import { BillingSettingsRoutes } from "../../src/server/routes/settings/billing" const file = path.join(Global.Path.config, "openscience.json") afterEach(async () => { await fs.rm(file, { force: true }).catch(() => {}) + // globalConfigFile() (config.ts) also considers .jsonc and config.json - + // remove those too so a stray one left behind by another test/file in the + // same `bun test` run never shadows the openscience.json this file always + // writes and reads directly, and reset the in-memory Config.global cache + // (a deleted file alone does not un-memoize it). + await fs.rm(path.join(Global.Path.config, "openscience.jsonc"), { force: true }).catch(() => {}) + await fs.rm(path.join(Global.Path.config, "config.json"), { force: true }).catch(() => {}) + Config.global.reset() }) test("PUT persists the toggle without baking resolved secrets into the config file", async () => { diff --git a/backend/cli/test/storage/publish-temp-files.test.ts b/backend/cli/test/storage/publish-temp-files.test.ts new file mode 100644 index 00000000..9d8057a1 --- /dev/null +++ b/backend/cli/test/storage/publish-temp-files.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, test } from "bun:test" +import path from "path" +import fs from "fs/promises" +import { Global } from "../../src/global" +import { Project } from "../../src/project/project" +import { Storage } from "../../src/storage/storage" +import { Log } from "../../src/util/log" + +Log.init({ print: false }) + +// Storage.publish stages every write as `...tmp` inside the +// record directory and renames it into place. Storage.list used to glob `**/*` +// there and strip a fixed 5 characters off each hit assuming ".json", so a +// staging file — visible during the write→rename window, and forever if the +// writer crashed in between, since nothing sweeps them — became a phantom key +// whose Storage.read throws NotFoundError. The file is written directly here +// rather than raced against a real publish() so the regression is deterministic +// and also covers the crashed-writer case, which no timing trick can reach. +const stray = (dir: string, record: string) => + path.join(dir, `${record}.json.4242.9d1c0f2a-1111-4222-8333-444455556666.tmp`) + +describe("Storage.list ignores publish staging files", () => { + test("a stray temp file in the record directory is never enumerated as a key", async () => { + const id = "prj_storage_temp_probe" + await Storage.write(["project", id], { + id, + vcs: "git", + worktree: path.join(Global.Path.data, "storage-temp-probe-worktree"), + time: { created: Date.now(), initialized: Date.now() }, + }) + const dir = path.join(Global.Path.data, "storage", "project") + const temp = stray(dir, id) + await Bun.write(temp, "{}") + + try { + const keys = await Storage.list(["project"]) + expect(keys).toContainEqual(["project", id]) + expect(keys.filter((key) => key.some((part) => part.includes(".tmp")))).toEqual([]) + + // Project.list() reads every key Storage.list hands back with no .catch, + // so a phantom key surfaces to callers as a thrown NotFoundError. + const projects = await Project.list() + expect(projects.some((project) => project.id === id)).toBe(true) + } finally { + await fs.rm(temp, { force: true }) + await Storage.remove(["project", id]) + } + }) +}) diff --git a/frontend/ui/src/components/message-part.css b/frontend/ui/src/components/message-part.css index 728c835c..b03dd912 100644 --- a/frontend/ui/src/components/message-part.css +++ b/frontend/ui/src/components/message-part.css @@ -745,8 +745,18 @@ color: var(--text-base); outline: none; + /* This field frames itself — the border and radius above ARE the frame + — so it takes the lit frame, not the halo the app-wide rule hands + fields whose border box is just the glyphs (atlas.css). That rule is + !important, so out-specifying it is not enough; matching its weight + is the only way the treatment declared here reaches the screen at + all. Outline included: text inputs match :focus-visible on mouse + focus too, and without this the halo's offset ring survives on top + of the frame, at a different radius. */ &:focus { - border-color: var(--border-focus); + border-color: var(--focus-lit-ring); + outline: none !important; + box-shadow: var(--focus-lit) !important; } &::placeholder { diff --git a/frontend/ui/src/components/text-field.css b/frontend/ui/src/components/text-field.css index c94376be..e08513bf 100644 --- a/frontend/ui/src/components/text-field.css +++ b/frontend/ui/src/components/text-field.css @@ -52,11 +52,12 @@ background: var(--input-base); &:focus-within:not(:has([data-readonly])) { - border-color: transparent; - /* border/shadow-xs/select */ + /* Lit rather than outlined — see --focus-lit in theme.css. The invalid + state below keeps its accent border, so the only hard stroke a field + can show now is the one that means something went wrong. */ + border-color: var(--focus-lit-ring); box-shadow: - 0 0 0 3px var(--border-weak-selected), - 0 0 0 1px var(--border-selected), + var(--focus-lit), 0 1px 2px -1px rgba(19, 16, 16, 0.25), 0 1px 2px 0 rgba(19, 16, 16, 0.08), 0 1px 3px 0 rgba(19, 16, 16, 0.12); diff --git a/frontend/ui/src/styles/theme.css b/frontend/ui/src/styles/theme.css index 6dff0584..113b899d 100644 --- a/frontend/ui/src/styles/theme.css +++ b/frontend/ui/src/styles/theme.css @@ -229,6 +229,25 @@ --border-selected: #b85c3b; --border-disabled: var(--smoke-light-alpha-8); --border-focus: #b85c3b; + /* Focus lighting for text entry. A flat accent stroke around a field reads as + a validation error; light does not arrive as a hard edge. The four layers + stack into one falloff — a highlight where the light lands on the top edge, + a hairline, then two widening washes — so a focused field looks lit rather + than outlined. Light mode grounds the falloff in ink, because a white wash + is invisible on a white surface. */ + --focus-lit-edge: rgba(255, 255, 255, 0.9); + --focus-lit-ring: rgba(30, 30, 27, 0.18); + --focus-lit-wash: rgba(30, 30, 27, 0.07); + --focus-lit-bloom: rgba(30, 30, 27, 0.05); + --focus-lit: + inset 0 1px 0 var(--focus-lit-edge), 0 0 0 1px var(--focus-lit-ring), 0 0 0 4px var(--focus-lit-wash), + 0 0 16px 1px var(--focus-lit-bloom); + /* The halo behind an unframed field — one whose box is the glyphs themselves, + so nothing can be drawn on the box without landing on the text. Its crisp + edge is an outline instead, the only ring CSS lets you hold off the box + (outline-offset), which is what the accent stroke used before. This is just + the light spilling past it. */ + --focus-lit-halo: 0 0 0 5px var(--focus-lit-wash), 0 0 16px 4px var(--focus-lit-bloom); --border-weak-base: #3e2e2112; --border-strong-base: #3e2e2147; --border-strong-hover: var(--smoke-light-alpha-8); @@ -487,6 +506,11 @@ --border-selected: #d48765; --border-disabled: #f2f1ec59; --border-focus: #d48765; + /* On a dark surface the falloff is the light itself, so every layer is white. */ + --focus-lit-edge: rgba(255, 255, 255, 0.26); + --focus-lit-ring: rgba(255, 255, 255, 0.2); + --focus-lit-wash: rgba(255, 255, 255, 0.07); + --focus-lit-bloom: rgba(255, 255, 255, 0.06); --border-weak-base: #f2f1ec14; --border-strong-base: #f2f1ec4a; --border-strong-hover: #f2f1ec3f; @@ -632,3 +656,21 @@ --avatar-text-lime: #c4f042; } } + +/* The in-app theme toggle writes data-color-scheme onto ; the + prefers-color-scheme block above cannot see it. Restate the focus falloff for + both explicit choices, so a dark app on a light OS is lit with white rather + than ink — and the reverse. */ +html[data-color-scheme="dark"] { + --focus-lit-edge: rgba(255, 255, 255, 0.26); + --focus-lit-ring: rgba(255, 255, 255, 0.2); + --focus-lit-wash: rgba(255, 255, 255, 0.07); + --focus-lit-bloom: rgba(255, 255, 255, 0.06); +} + +html[data-color-scheme="light"] { + --focus-lit-edge: rgba(255, 255, 255, 0.9); + --focus-lit-ring: rgba(30, 30, 27, 0.18); + --focus-lit-wash: rgba(30, 30, 27, 0.07); + --focus-lit-bloom: rgba(30, 30, 27, 0.05); +} diff --git a/frontend/workspace/src/atlas/CommandPalette.tsx b/frontend/workspace/src/atlas/CommandPalette.tsx index 5e03935f..6b5cf85b 100644 --- a/frontend/workspace/src/atlas/CommandPalette.tsx +++ b/frontend/workspace/src/atlas/CommandPalette.tsx @@ -397,6 +397,9 @@ export function CommandPalette(props: CommandPaletteProps): JSX.Element { "font-family": FONT_MONO, "font-size": "13px", color: "var(--color-text)", + // `all: unset` leaves the box flush with the glyphs, so the + // caret starts on the edge and the focus ring lands on the text. + padding: "3px 10px", }} />