From 7693513b12032c4d5610e1832e6ae317e1af4b7d Mon Sep 17 00:00:00 2001 From: Rick Gao Date: Thu, 16 Jul 2026 23:29:01 -0700 Subject: [PATCH] refactor(config): extract shared config resolution Move the env-over-settings resolvers (project/API key/agent name, from cli.ts) and resolveDaemonConfig/resolveTraceBaseUrl/fingerprint (from daemon.ts) into src/config.ts; dedupe the resolver twins via resolveFromEnvOrSettings and share missingConfig between status/restart and runDaemon. cmdConfig loses a let-reassign and a whole-object cast. Behavior note: an empty-string env var now falls through to settings instead of winning with a mislabeled source. Co-Authored-By: Claude Fable 5 --- src/cli.ts | 112 ++++++++++------------------ src/config.ts | 138 +++++++++++++++++++++++++++++++++++ src/daemon.ts | 57 +-------------- tests/config-drift.test.ts | 2 +- tests/trace-base-url.test.ts | 2 +- 5 files changed, 180 insertions(+), 131 deletions(-) create mode 100644 src/config.ts diff --git a/src/cli.ts b/src/cli.ts index d19ed0e..673b8ed 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -12,7 +12,6 @@ import { CONFIG_DIR, SETTINGS_FILE, MARKETPLACE_NAME, - PLUGIN_NAME, VERSION, InstallSource, MarketplaceStatus, @@ -28,7 +27,17 @@ import { type PluginSource, } from './setup.js'; import { prompt, sendToSocket, requestFromSocket, probeUnixSocket, SocketState } from './utils.js'; -import { runDaemon, resolveDaemonConfig, daemonConfigFingerprint } from './daemon.js'; +import { runDaemon } from './daemon.js'; +import { + resolveProject, + resolveApiKey, + resolveAgentName, + resolveDaemonConfig, + daemonConfigFingerprint, + missingConfig, + WeaveProjectSource, + ApiKeySource, +} from './config.js'; import { DEFAULT_AGENT_NAME } from './genaiSpans.js'; // --------------------------------------------------------------------------- @@ -145,8 +154,8 @@ async function cmdInstall( process.exit(1); } - const effectiveProject = process.env['WEAVE_PROJECT'] ?? settings.weave_project ?? null; - const effectiveApiKey = process.env['WANDB_API_KEY'] ?? settings.wandb_api_key ?? null; + const effectiveProject = resolveProject(settings).value; + const effectiveApiKey = resolveApiKey(settings).value; if (nonInteractive) { console.log('\n- Non-interactive install: skipping setup prompts'); @@ -220,29 +229,6 @@ function maskSecret(value: string): string { return `${value.slice(0, 4)}…`; } -/** Where the effective agent name came from. Parallels `WeaveProjectSource` / - * `ApiKeySource`; has no `NotSet` member because agent_name always resolves - * to the built-in default. */ -enum AgentNameSource { - EnvVar = 'WEAVE_AGENT_NAME env var', - Settings = 'settings.json', - Default = 'default', -} - -/** - * Resolve the effective top-level agent name and where it came from. Mirrors - * the env-over-settings precedence used for `weave_project`, with the - * hardcoded `DEFAULT_AGENT_NAME` as the final fallback. Shared by - * `config show` and `config get` so both report the same value. - */ -function resolveAgentName(settings: Settings): { value: string; source: AgentNameSource } { - const fromEnv = process.env['WEAVE_AGENT_NAME']?.trim(); - if (fromEnv) return { value: fromEnv, source: AgentNameSource.EnvVar }; - const fromSettings = settings.agent_name?.trim(); - if (fromSettings) return { value: fromSettings, source: AgentNameSource.Settings }; - return { value: DEFAULT_AGENT_NAME, source: AgentNameSource.Default }; -} - async function cmdConfig(args: string[]): Promise { const action = args[0]; @@ -255,19 +241,8 @@ async function cmdConfig(args: string[]): Promise { process.exit(1); } - const effectiveProject = process.env['WEAVE_PROJECT'] ?? settings.weave_project ?? null; - const projectSource: WeaveProjectSource = process.env['WEAVE_PROJECT'] - ? WeaveProjectSource.EnvVar - : settings.weave_project - ? WeaveProjectSource.Settings - : WeaveProjectSource.NotSet; - - const effectiveApiKey = process.env['WANDB_API_KEY'] ?? settings.wandb_api_key ?? null; - const apiKeySource: ApiKeySource = process.env['WANDB_API_KEY'] - ? ApiKeySource.EnvVar - : settings.wandb_api_key - ? ApiKeySource.Settings - : ApiKeySource.NotSet; + const { value: effectiveProject, source: projectSource } = resolveProject(settings); + const { value: effectiveApiKey, source: apiKeySource } = resolveApiKey(settings); const apiKeyDisplay = effectiveApiKey ? `${maskSecret(effectiveApiKey)} [${apiKeySource}]` : `(not set)`; console.log('Current configuration:'); @@ -309,11 +284,9 @@ async function cmdConfig(args: string[]): Promise { process.exit(1); } if (key === 'weave_project') { - const effective = process.env['WEAVE_PROJECT'] ?? settings.weave_project ?? null; - console.log(effective ?? '(not set)'); + console.log(resolveProject(settings).value ?? '(not set)'); } else if (key === 'wandb_api_key') { - const effective = process.env['WANDB_API_KEY'] ?? settings.wandb_api_key ?? null; - console.log(effective ?? '(not set)'); + console.log(resolveApiKey(settings).value ?? '(not set)'); } else { console.log(value ?? '(not set)'); } @@ -328,11 +301,12 @@ async function cmdConfig(args: string[]): Promise { process.exit(1); } - const writableKeys = ['weave_project', 'wandb_api_key', 'agent_name', 'daemon_socket', 'debug']; - if (!writableKeys.includes(key)) { + const writableKeys: readonly (keyof Settings)[] = ['weave_project', 'wandb_api_key', 'agent_name', 'daemon_socket', 'debug']; + if (!writableKeys.includes(key as keyof Settings)) { console.error(`Cannot set '${key}'. Writable keys: ${writableKeys.join(', ')}`); process.exit(1); } + const writableKey = key as keyof Settings; if (key === 'weave_project' && !value.includes('/')) { console.error(`Invalid format for weave_project: '${value}'\nExpected: entity/project (e.g. my-entity/my-project)`); @@ -352,12 +326,16 @@ async function cmdConfig(args: string[]): Promise { process.exit(1); } - const coerced = key === 'debug' ? value === 'true' : value; - (settings as unknown as Record)[key] = coerced; + // `debug` is the only boolean Settings field; every other writable key is a + // string. Split the assignment so each branch's value type matches the + // narrowed property type (no whole-object cast needed). + if (writableKey === 'debug') { + settings.debug = value === 'true'; + } else { + settings[writableKey] = value; + } saveSettings(settings); - const displayValue = key === 'wandb_api_key' && typeof coerced === 'string' - ? maskSecret(coerced) - : coerced; + const displayValue = writableKey === 'wandb_api_key' ? maskSecret(value) : value; console.log(`✓ Set ${key} = ${displayValue}`); return; } @@ -370,18 +348,6 @@ async function cmdConfig(args: string[]): Promise { // status // --------------------------------------------------------------------------- -/** Where a configured value (project, API key) came from at gather time. */ -export enum WeaveProjectSource { - EnvVar = 'WEAVE_PROJECT env var', - Settings = 'settings.json', - NotSet = 'not set', -} -export enum ApiKeySource { - EnvVar = 'WANDB_API_KEY env var', - Settings = 'settings.json', - NotSet = 'not set', -} - /** Whether settings.json could be read at gather time. */ export enum ConfigState { Ok = 'ok', @@ -480,18 +446,17 @@ async function gatherStatus(): Promise { return snap; } - // Env vars take precedence over settings.json for both project and key. - const effectiveProject = process.env['WEAVE_PROJECT'] ?? settings.weave_project ?? null; + const { value: effectiveProject, source: projectSource } = resolveProject(settings); if (effectiveProject) { report.weave_project = effectiveProject; - report.weave_project_source = process.env['WEAVE_PROJECT'] ? WeaveProjectSource.EnvVar : WeaveProjectSource.Settings; + report.weave_project_source = projectSource; } - const effectiveApiKey = process.env['WANDB_API_KEY'] ?? settings.wandb_api_key ?? null; + const { value: effectiveApiKey, source: apiKeySource } = resolveApiKey(settings); if (effectiveApiKey) { report.api_key_configured = true; snap.api_key_masked = maskSecret(effectiveApiKey); - snap.api_key_source = process.env['WANDB_API_KEY'] ? ApiKeySource.EnvVar : ApiKeySource.Settings; + snap.api_key_source = apiKeySource; } report.agent_name = resolveAgentName(settings).value; @@ -582,10 +547,7 @@ function printPrettyStatus(snap: StatusSnapshot): void { } else if (socketState === SocketState.Stale) { console.log('Weave Claude Code — daemon socket stale (auto-recovers next session)'); } else { - const missing = [ - !report.weave_project && 'weave_project', - !report.api_key_configured && 'wandb_api_key', - ].filter(Boolean).join(', '); + const missing = missingConfig(!!report.weave_project, report.api_key_configured, 'wandb_api_key'); console.log('Weave Claude Code — configuration incomplete'); if (missing) console.log(` Set ${missing} to start tracing`); } @@ -832,10 +794,10 @@ async function cmdRestart(): Promise { } // Don't spawn a daemon that would just exit for lack of config (mirrors runDaemon). - const project = process.env['WEAVE_PROJECT'] ?? settings.weave_project ?? null; - const apiKey = process.env['WANDB_API_KEY'] ?? settings.wandb_api_key ?? null; + const project = resolveProject(settings).value; + const apiKey = resolveApiKey(settings).value; if (!project || !apiKey) { - const missing = [!project && 'weave_project', !apiKey && 'WANDB_API_KEY'].filter(Boolean).join(', '); + const missing = missingConfig(!!project, !!apiKey, 'WANDB_API_KEY'); console.error(`⚠ Not starting daemon, missing configuration: ${missing}`); console.error(' Set it with: weave-claude-code config set weave_project ENTITY/PROJECT'); process.exit(1); diff --git a/src/config.ts b/src/config.ts new file mode 100644 index 0000000..09314f8 --- /dev/null +++ b/src/config.ts @@ -0,0 +1,138 @@ +// SPDX-FileCopyrightText: 2026 CoreWeave, Inc. +// SPDX-License-Identifier: MIT +// SPDX-PackageName: weave-claude-code + +// Config resolution shared by the CLI and the daemon (env over +// settings.json). Lives here so both use one implementation without an +// import cycle (cli.ts imports the daemon entry point). + +import { DEFAULT_AGENT_NAME } from './genaiSpans.js'; +import { createHash } from 'crypto'; +import type { Settings } from './setup.js'; + +/** Where a resolved value came from, for user-facing "source" reporting. */ +export enum WeaveProjectSource { + EnvVar = 'WEAVE_PROJECT env var', + Settings = 'settings.json', + NotSet = 'not set', +} +export enum ApiKeySource { + EnvVar = 'WANDB_API_KEY env var', + Settings = 'settings.json', + NotSet = 'not set', +} +/** No `NotSet`: agent_name always resolves to the built-in default. */ +export enum AgentNameSource { + EnvVar = 'WEAVE_AGENT_NAME env var', + Settings = 'settings.json', + Default = 'default', +} + +/** Env-over-settings resolution shared by the project and API-key resolvers: + * a non-empty env value wins, then a non-empty settings value, else null. + * `sources` supplies the per-field labels for the matching branch. */ +function resolveFromEnvOrSettings( + envValue: string | undefined, + settingsValue: string | null | undefined, + sources: { env: S; settings: S; notSet: S }, +): { value: string | null; source: S } { + if (envValue) return { value: envValue, source: sources.env }; + if (settingsValue) return { value: settingsValue, source: sources.settings }; + return { value: null, source: sources.notSet }; +} + +/** Resolve the effective Weave project (WEAVE_PROJECT env beats + * settings.weave_project) and where it came from. */ +export function resolveProject( + settings: Settings, + env: NodeJS.ProcessEnv = process.env, +): { value: string | null; source: WeaveProjectSource } { + return resolveFromEnvOrSettings(env['WEAVE_PROJECT'], settings.weave_project, { + env: WeaveProjectSource.EnvVar, + settings: WeaveProjectSource.Settings, + notSet: WeaveProjectSource.NotSet, + }); +} + +/** Resolve the effective W&B API key (WANDB_API_KEY env beats + * settings.wandb_api_key) and where it came from. */ +export function resolveApiKey( + settings: Settings, + env: NodeJS.ProcessEnv = process.env, +): { value: string | null; source: ApiKeySource } { + return resolveFromEnvOrSettings(env['WANDB_API_KEY'], settings.wandb_api_key, { + env: ApiKeySource.EnvVar, + settings: ApiKeySource.Settings, + notSet: ApiKeySource.NotSet, + }); +} + +/** Resolve the effective top-level agent name (WEAVE_AGENT_NAME env beats + * settings.agent_name), falling back to `DEFAULT_AGENT_NAME`. */ +export function resolveAgentName( + settings: Settings, + env: NodeJS.ProcessEnv = process.env, +): { value: string; source: AgentNameSource } { + const fromEnv = env['WEAVE_AGENT_NAME']?.trim(); + if (fromEnv) return { value: fromEnv, source: AgentNameSource.EnvVar }; + const fromSettings = settings.agent_name?.trim(); + if (fromSettings) return { value: fromSettings, source: AgentNameSource.Settings }; + return { value: DEFAULT_AGENT_NAME, source: AgentNameSource.Default }; +} + +/** The config the daemon loads at startup and holds for its lifetime. */ +export type DaemonConfig = { + weaveProject: string | null; + apiKey: string | null; + baseUrl: string; + agentName: string; + debug: boolean; +}; + +/** Resolve the daemon config from settings + env, reusing the per-field + * resolvers so the env-over-settings precedence is defined once. */ +export function resolveDaemonConfig(settings: Settings, env: NodeJS.ProcessEnv): DaemonConfig { + return { + weaveProject: resolveProject(settings, env).value, + apiKey: resolveApiKey(settings, env).value, + baseUrl: resolveTraceBaseUrl(env), + agentName: resolveAgentName(settings, env).value, + debug: !!env['WEAVE_CLAUDE_DEBUG'] || settings.debug === true, + }; +} + +/** SaaS trace-ingest host: the default OTLP target, and what the routeless + * SaaS API host remaps to. */ +const DEFAULT_TRACE_BASE_URL = 'https://trace.wandb.ai'; + +/** Resolve the Weave trace server base URL for OTLP export. `WF_TRACE_SERVER_URL` + * wins when set. Otherwise `WANDB_BASE_URL` is used, but SaaS `api.wandb.ai` is + * the wandb API host with no OTLP route, so it maps to `trace.wandb.ai`; a + * self-hosted `WANDB_BASE_URL` passes through unchanged. */ +function resolveTraceBaseUrl(env: NodeJS.ProcessEnv): string { + const explicit = env['WF_TRACE_SERVER_URL']?.trim(); + if (explicit) return explicit.replace(/\/+$/, ''); + const base = (env['WANDB_BASE_URL'] ?? DEFAULT_TRACE_BASE_URL).replace(/\/+$/, ''); + return /^https?:\/\/api\.wandb\.ai$/i.test(base) ? DEFAULT_TRACE_BASE_URL : base; +} + +/** Comma-joined list of missing required config, for the "incomplete" + * status/startup messages. `apiKeyLabel` differs by call site + * (`wandb_api_key` for config-oriented messages, `WANDB_API_KEY` for + * env-oriented ones). */ +export function missingConfig(hasProject: boolean, hasApiKey: boolean, apiKeyLabel: string): string { + return [!hasProject && 'weave_project', !hasApiKey && apiKeyLabel].filter(Boolean).join(', '); +} + +/** Hex chars kept from the config hash. 16 (64 bits) is ample to detect a + * config change while staying compact for logs and the socket reply. */ +const CONFIG_FINGERPRINT_LENGTH = 16; + +/** Short, stable hash of a daemon config. The API key is hashed, not exposed, + * so the fingerprint is safe to send over the socket. */ +export function daemonConfigFingerprint(c: DaemonConfig): string { + return createHash('sha256') + .update(JSON.stringify([c.weaveProject, c.apiKey, c.baseUrl, c.agentName, c.debug])) + .digest('hex') + .slice(0, CONFIG_FINGERPRINT_LENGTH); +} diff --git a/src/daemon.ts b/src/daemon.ts index 9a317f3..86f3f8b 100644 --- a/src/daemon.ts +++ b/src/daemon.ts @@ -21,7 +21,8 @@ import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node'; import { BatchSpanProcessor } from '@opentelemetry/sdk-trace-base'; import { resourceFromAttributes } from '@opentelemetry/resources'; import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-proto'; -import { loadSettings, VERSION, type Settings } from './setup.js'; +import { loadSettings, VERSION } from './setup.js'; +import { resolveDaemonConfig, daemonConfigFingerprint, missingConfig } from './config.js'; import { appendToLog, deepEqual } from './utils.js'; import { parseSessionFd, @@ -33,7 +34,6 @@ import { import { TranscriptFile, readFirstTranscriptLine } from './transcriptFile.js'; import { ATTR, - DEFAULT_AGENT_NAME, CompactionAttrs, IntegrationBaggageSpanProcessor, createIntegrationBaggage, @@ -2312,57 +2312,6 @@ export class GlobalDaemon { } } -// ───────────────────────────────────────────────────────────────────────────── -// Config resolution and fingerprinting -// ───────────────────────────────────────────────────────────────────────────── - -/** The config the daemon loads at startup and holds for its lifetime. */ -type DaemonConfig = { - weaveProject: string | null; - apiKey: string | null; - baseUrl: string; - agentName: string; - debug: boolean; -} - -/** Resolve the effective daemon config from settings + env, applying the same - * env-over-settings precedence the daemon uses at startup. */ -export function resolveDaemonConfig(settings: Settings, env: NodeJS.ProcessEnv): DaemonConfig { - return { - weaveProject: env['WEAVE_PROJECT'] ?? settings.weave_project ?? null, - apiKey: env['WANDB_API_KEY'] ?? settings.wandb_api_key ?? null, - baseUrl: resolveTraceBaseUrl(env), - // `||` (not `??`) so an empty/whitespace value falls through to the default - // rather than producing a blank `invoke_agent ` span name. - agentName: env['WEAVE_AGENT_NAME']?.trim() || settings.agent_name?.trim() || DEFAULT_AGENT_NAME, - debug: !!env['WEAVE_CLAUDE_DEBUG'] || settings.debug === true, - }; -} - -/** Resolve the Weave trace server base URL for OTLP export. `WF_TRACE_SERVER_URL` - * wins when set. Otherwise `WANDB_BASE_URL` is used, but SaaS `api.wandb.ai` is - * the wandb API host with no OTLP route, so it maps to `trace.wandb.ai`; a - * self-hosted `WANDB_BASE_URL` passes through unchanged. */ -function resolveTraceBaseUrl(env: NodeJS.ProcessEnv): string { - const explicit = env['WF_TRACE_SERVER_URL']?.trim(); - if (explicit) return explicit.replace(/\/+$/, ''); - const base = (env['WANDB_BASE_URL'] ?? 'https://trace.wandb.ai').replace(/\/+$/, ''); - return /^https?:\/\/api\.wandb\.ai$/i.test(base) ? 'https://trace.wandb.ai' : base; -} - -/** Hex chars kept from the config hash. 16 (64 bits) is ample to detect a - * config change while keeping the value compact for logs and the socket reply. */ -const CONFIG_FINGERPRINT_LENGTH = 16; - -/** Short, stable hash of a daemon config. The API key is hashed, not exposed, - * so the fingerprint is safe to send over the socket. */ -export function daemonConfigFingerprint(c: DaemonConfig): string { - return createHash('sha256') - .update(JSON.stringify([c.weaveProject, c.apiKey, c.baseUrl, c.agentName, c.debug])) - .digest('hex') - .slice(0, CONFIG_FINGERPRINT_LENGTH); -} - // ───────────────────────────────────────────────────────────────────────────── // Entry point (invoked by `weave-claude-code daemon`) // ───────────────────────────────────────────────────────────────────────────── @@ -2376,7 +2325,7 @@ export async function runDaemon(): Promise { const { weaveProject, apiKey, baseUrl, agentName, debug } = resolveDaemonConfig(settings, process.env); if (!weaveProject || !apiKey) { - const missing = [!weaveProject && 'weave_project', !apiKey && 'WANDB_API_KEY'].filter(Boolean).join(', '); + const missing = missingConfig(!!weaveProject, !!apiKey, 'WANDB_API_KEY'); appendToLog(logFile, 'INFO', `Daemon not started — missing configuration: ${missing}`); process.exit(0); } diff --git a/tests/config-drift.test.ts b/tests/config-drift.test.ts index bec83e2..46ee18a 100644 --- a/tests/config-drift.test.ts +++ b/tests/config-drift.test.ts @@ -16,7 +16,7 @@ import * as net from 'node:net'; import * as path from 'node:path'; import { fileURLToPath } from 'node:url'; -import { resolveDaemonConfig, daemonConfigFingerprint } from '../src/daemon.ts'; +import { resolveDaemonConfig, daemonConfigFingerprint } from '../src/config.ts'; const HERE = path.dirname(fileURLToPath(import.meta.url)); const REPO_ROOT = path.resolve(HERE, '..'); diff --git a/tests/trace-base-url.test.ts b/tests/trace-base-url.test.ts index 6354303..bd31b13 100644 --- a/tests/trace-base-url.test.ts +++ b/tests/trace-base-url.test.ts @@ -8,7 +8,7 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { resolveDaemonConfig } from '../src/daemon.ts'; +import { resolveDaemonConfig } from '../src/config.ts'; const SETTINGS = { weave_project: 'e/p', wandb_api_key: 'k' }; const baseUrlFor = (env: Record): string =>