From 88c87f55c0e190b304ba0fe55ced7bd5ffccb4ef Mon Sep 17 00:00:00 2001 From: Kurt Overmier Date: Thu, 16 Apr 2026 08:19:38 -0500 Subject: [PATCH 1/2] feat(cli): STACKBILT_API_KEY env var auth; deprecate charter login MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Additive preparation for the 1.0 package split (see charter RFC TBD). No public API removed or renamed — follows OSS update policy. Why The CLI currently solicits a Stackbilt API key via `charter login`, which is the most eyebrow-raising OSS/commercial coupling in the package: an OSS governance tool writing a bearer token to ~/.charter/credentials.json. Supporting the standard env-var path lets users authenticate the commercial commands (`run`, `architect`) without the CLI persisting anything to disk, materially improving the audit story now while the full split to @stackbilt/build is scoped and sequenced separately. What changes credentials.ts + API_KEY_ENV_VAR = 'STACKBILT_API_KEY' + resolveApiKey() → { apiKey, source: 'env' | 'credentials', baseUrl? } | null Env var wins over stored credentials; empty/whitespace env var falls through. loadCredentials(), saveCredentials(), clearCredentials() preserved unchanged. commands/architect.ts, commands/run.ts - loadCredentials() call sites → resolveApiKey(). `run`'s useGateway decision now honors the env var path identically to stored creds. commands/login.ts - printDeprecationNotice() on stderr for every invocation except --logout. Command functionality unchanged. Help text reworked: env var is the "Preferred" path, stored credentials the "Deprecated alternative." If the env var is set and no --key is given, we report the env var as the authoritative source. http-client.ts - Scaffold auth-error string now points at STACKBILT_API_KEY first, `charter login` marked deprecated. Tests + credentials.test.ts — 4 tests covering env-var precedence, trimming, empty-string fallthrough, whitespace-only fallthrough. + login.test.ts — 2 tests covering the stderr deprecation notice and env-var reporting path. All 405 existing tests still pass. Docs - packages/cli/README.md: new "Authentication (optional)" section documenting the env var and login deprecation. Root README unchanged — that rewrite ships with the package split. - CHANGELOG.md: [Unreleased] section added with Added/Deprecated/Changed subsections. Follow-ups (not in this PR) - Root README rewrite when package split lands - RFC: extract commercial surface into @stackbilt/build - 1.0 cut removes login, credentials persistence, run/architect/scaffold Co-Authored-By: Claude Opus 4.6 (1M context) --- CHANGELOG.md | 13 ++++ packages/cli/README.md | 12 ++++ .../cli/src/__tests__/credentials.test.ts | 60 +++++++++++++++++++ packages/cli/src/__tests__/login.test.ts | 50 ++++++++++++++++ packages/cli/src/commands/architect.ts | 12 ++-- packages/cli/src/commands/login.ts | 37 ++++++++++-- packages/cli/src/commands/run.ts | 13 ++-- packages/cli/src/credentials.ts | 32 +++++++++- packages/cli/src/http-client.ts | 5 +- 9 files changed, 218 insertions(+), 16 deletions(-) create mode 100644 packages/cli/src/__tests__/credentials.test.ts create mode 100644 packages/cli/src/__tests__/login.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index e5cc766..03873c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,19 @@ All notable changes to this project are documented in this file. The format is based on Keep a Changelog and follows Semantic Versioning. +## [Unreleased] + +### Added +- **`STACKBILT_API_KEY` environment variable** — `charter run` and `charter architect` now resolve the API key from `STACKBILT_API_KEY` first, falling back to stored credentials only if the env var is absent or blank. This lets users authenticate the commercial commands without writing a token to `~/.charter/credentials.json`. +- `resolveApiKey()` helper exported from `@stackbilt/cli`'s credentials module (env-var precedence, trimmed, returns `{ apiKey, source: 'env' | 'credentials', baseUrl? }`). + +### Deprecated +- **`charter login`** — emits a deprecation notice on every invocation. Functionality unchanged; scheduled for removal in 1.0 when gateway-bound commands (`login`, `run`, `architect`, `scaffold`) move out of `@stackbilt/cli` into a separate `@stackbilt/build` package. + +### Changed +- Scaffold auth-error message now points users at `STACKBILT_API_KEY` as the primary path, with `charter login` marked deprecated. +- CLI README gains a short "Authentication (optional)" section documenting the env-var path. + ## [0.10.0] - 2026-04-09 Synchronized version bump for all `@stackbilt/*` packages to 0.10.0. diff --git a/packages/cli/README.md b/packages/cli/README.md index 68f7ae1..160b787 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -73,6 +73,18 @@ charter blast src/foo.ts --depth 3 # reverse dep graph → files affected by ch charter surface --markdown # extract routes (Hono/Express) + D1 schema as markdown ``` +## Authentication (optional) + +Governance commands (`validate`, `drift`, `blast`, `surface`, etc.) run locally and require no authentication. + +Commands that reach the Stackbilt engine (`run`, `architect`) read their API key from the `STACKBILT_API_KEY` environment variable: + +```bash +export STACKBILT_API_KEY=ea_xxx # or sb_live_xxx, sb_test_xxx +``` + +The legacy `charter login --key …` command still works but is deprecated and will be removed in `@stackbilt/cli` 1.0 when gateway-bound commands move to a separate package. + ## Human Onboarding (Copy/Paste) Run this in the target repository: diff --git a/packages/cli/src/__tests__/credentials.test.ts b/packages/cli/src/__tests__/credentials.test.ts new file mode 100644 index 0000000..8b9fd04 --- /dev/null +++ b/packages/cli/src/__tests__/credentials.test.ts @@ -0,0 +1,60 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { resolveApiKey, API_KEY_ENV_VAR } from '../credentials'; + +describe('resolveApiKey', () => { + const originalEnv = process.env[API_KEY_ENV_VAR]; + + beforeEach(() => { + delete process.env[API_KEY_ENV_VAR]; + }); + + afterEach(() => { + if (originalEnv === undefined) { + delete process.env[API_KEY_ENV_VAR]; + } else { + process.env[API_KEY_ENV_VAR] = originalEnv; + } + }); + + it('returns env var when set (env wins over stored credentials)', () => { + process.env[API_KEY_ENV_VAR] = 'ea_test_from_env_12345'; + + const result = resolveApiKey(); + + expect(result).not.toBeNull(); + expect(result!.source).toBe('env'); + expect(result!.apiKey).toBe('ea_test_from_env_12345'); + }); + + it('trims whitespace from the env var', () => { + process.env[API_KEY_ENV_VAR] = ' sb_test_abc '; + + const result = resolveApiKey(); + + expect(result).not.toBeNull(); + expect(result!.source).toBe('env'); + expect(result!.apiKey).toBe('sb_test_abc'); + }); + + it('treats an empty env var as unset (falls through to credentials or null)', () => { + process.env[API_KEY_ENV_VAR] = ''; + + const result = resolveApiKey(); + + // We can't predict whether this machine has stored credentials, so we + // only assert the env path was NOT taken. + if (result !== null) { + expect(result.source).toBe('credentials'); + } + }); + + it('treats a whitespace-only env var as unset', () => { + process.env[API_KEY_ENV_VAR] = ' \t '; + + const result = resolveApiKey(); + + if (result !== null) { + expect(result.source).toBe('credentials'); + } + }); +}); diff --git a/packages/cli/src/__tests__/login.test.ts b/packages/cli/src/__tests__/login.test.ts new file mode 100644 index 0000000..df2b231 --- /dev/null +++ b/packages/cli/src/__tests__/login.test.ts @@ -0,0 +1,50 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { loginCommand } from '../commands/login'; +import type { CLIOptions } from '../index'; +import { API_KEY_ENV_VAR } from '../credentials'; + +const options: CLIOptions = { + format: 'text', + configPath: '.charter', + ciMode: false, + yes: false, +}; + +describe('charter login — deprecation notice', () => { + const originalEnv = process.env[API_KEY_ENV_VAR]; + + beforeEach(() => { + delete process.env[API_KEY_ENV_VAR]; + }); + + afterEach(() => { + if (originalEnv === undefined) { + delete process.env[API_KEY_ENV_VAR]; + } else { + process.env[API_KEY_ENV_VAR] = originalEnv; + } + vi.restoreAllMocks(); + }); + + it('writes a deprecation notice to stderr when invoked without args', async () => { + const stderr = vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + vi.spyOn(console, 'log').mockImplementation(() => {}); + + await loginCommand(options, []); + + const stderrOutput = stderr.mock.calls.map((c) => String(c[0])).join(''); + expect(stderrOutput).toMatch(/deprecated/i); + expect(stderrOutput).toContain(API_KEY_ENV_VAR); + }); + + it('reports env-var usage when STACKBILT_API_KEY is set and no --key flag', async () => { + process.env[API_KEY_ENV_VAR] = 'ea_login_test_key'; + vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + + await loginCommand(options, []); + + const stdoutOutput = log.mock.calls.map((c) => String(c[0])).join('\n'); + expect(stdoutOutput).toMatch(new RegExp(`Using ${API_KEY_ENV_VAR} from environment`)); + }); +}); diff --git a/packages/cli/src/commands/architect.ts b/packages/cli/src/commands/architect.ts index 66e17f9..7208d3b 100644 --- a/packages/cli/src/commands/architect.ts +++ b/packages/cli/src/commands/architect.ts @@ -13,7 +13,7 @@ import * as path from 'node:path'; import type { CLIOptions } from '../index'; import { EXIT_CODE, CLIError } from '../index'; import { getFlag } from '../flags'; -import { loadCredentials } from '../credentials'; +import { resolveApiKey, API_KEY_ENV_VAR } from '../credentials'; import { EngineClient, type BuildRequest, type BuildResult } from '../http-client'; export async function architectCommand(options: CLIOptions, args: string[]): Promise { @@ -44,10 +44,14 @@ export async function architectCommand(options: CLIOptions, args: string[]): Pro const seedStr = getFlag(args, '--seed'); if (seedStr) request.seed = parseInt(seedStr, 10); - // Load credentials (optional — engine may not require auth yet) - const creds = loadCredentials(); + // Resolve API key — env var wins over stored credentials. Engine /build is + // currently unauthenticated, so missing credentials is not an error here. + const resolved = resolveApiKey(); const baseUrl = getFlag(args, '--url'); - const client = new EngineClient({ baseUrl: baseUrl ?? creds?.baseUrl, apiKey: creds?.apiKey }); + const client = new EngineClient({ + baseUrl: baseUrl ?? resolved?.baseUrl, + apiKey: resolved?.apiKey ?? null, + }); // Build let result: BuildResult; diff --git a/packages/cli/src/commands/login.ts b/packages/cli/src/commands/login.ts index e11a6fc..eccf5a0 100644 --- a/packages/cli/src/commands/login.ts +++ b/packages/cli/src/commands/login.ts @@ -1,17 +1,32 @@ /** * charter login — API key management for Stackbilt Engine. * + * DEPRECATED: on-disk credential storage moves out of this OSS package in 1.0. + * Set STACKBILT_API_KEY in the environment instead. + * * Usage: - * charter login --key sb_live_xxx Store API key + * charter login --key sb_live_xxx Store API key (deprecated) * charter login --logout Clear stored credentials */ import type { CLIOptions } from '../index'; import { EXIT_CODE, CLIError } from '../index'; import { getFlag } from '../flags'; -import { loadCredentials, saveCredentials, clearCredentials } from '../credentials'; +import { + loadCredentials, + saveCredentials, + clearCredentials, + API_KEY_ENV_VAR, +} from '../credentials'; import { EngineClient } from '../http-client'; +function printDeprecationNotice(): void { + process.stderr.write( + `[deprecated] 'charter login' will be removed in 1.0. ` + + `Set ${API_KEY_ENV_VAR} in the environment instead.\n`, + ); +} + export async function loginCommand(options: CLIOptions, args: string[]): Promise { if (args.includes('--logout')) { clearCredentials(); @@ -21,7 +36,14 @@ export async function loginCommand(options: CLIOptions, args: string[]): Promise const key = getFlag(args, '--key'); if (!key) { + printDeprecationNotice(); const existing = loadCredentials(); + const envKey = process.env[API_KEY_ENV_VAR]; + if (envKey && envKey.trim().length > 0) { + const masked = envKey.slice(0, 12) + '...' + envKey.slice(-4); + console.log(`Using ${API_KEY_ENV_VAR} from environment: ${masked}`); + return EXIT_CODE.SUCCESS; + } if (existing) { const masked = existing.apiKey.slice(0, 12) + '...' + existing.apiKey.slice(-4); console.log(`Logged in as: ${masked}`); @@ -29,15 +51,20 @@ export async function loginCommand(options: CLIOptions, args: string[]): Promise } else { console.log('Not logged in.'); console.log(''); - console.log('Usage: charter login --key ea_xxx'); - console.log(' charter login --key sb_live_xxx'); - console.log(' charter login --key sb_test_xxx'); + console.log(`Preferred: export ${API_KEY_ENV_VAR}=ea_xxx (or sb_live_xxx, sb_test_xxx).`); + console.log(''); + console.log('Deprecated alternative:'); + console.log(' charter login --key ea_xxx'); + console.log(' charter login --key sb_live_xxx'); + console.log(' charter login --key sb_test_xxx'); console.log(''); console.log('Get your API key from auth.stackbilt.dev (ea_) or the Stackbilt dashboard (sb_).'); } return EXIT_CODE.SUCCESS; } + printDeprecationNotice(); + const VALID_PREFIXES = ['ea_', 'sb_live_', 'sb_test_']; if (!VALID_PREFIXES.some((p) => key.startsWith(p))) { throw new CLIError( diff --git a/packages/cli/src/commands/run.ts b/packages/cli/src/commands/run.ts index 7c22066..12732ed 100644 --- a/packages/cli/src/commands/run.ts +++ b/packages/cli/src/commands/run.ts @@ -19,7 +19,7 @@ import * as path from 'node:path'; import type { CLIOptions } from '../index'; import { EXIT_CODE, CLIError } from '../index'; import { getFlag } from '../flags'; -import { loadCredentials } from '../credentials'; +import { resolveApiKey, API_KEY_ENV_VAR } from '../credentials'; import { EngineClient, type BuildRequest, type ScaffoldResult } from '../http-client'; // ─── Animation ────────────────────────────────────────────── @@ -98,13 +98,16 @@ export async function runCommand(options: CLIOptions, args: string[]): Promise; diff --git a/packages/cli/src/credentials.ts b/packages/cli/src/credentials.ts index c4b634f..5804d7f 100644 --- a/packages/cli/src/credentials.ts +++ b/packages/cli/src/credentials.ts @@ -1,7 +1,12 @@ /** * Credential storage for Stackbilt API key. * - * Persists to ~/.charter/credentials.json (mode 0o600). + * Two auth sources are supported: + * 1. STACKBILT_API_KEY environment variable (preferred; no on-disk state). + * 2. ~/.charter/credentials.json (mode 0o600; populated by `charter login`). + * + * `charter login` will be removed in 1.0 — on-disk credential storage moves + * out of this OSS package. New integrations should use the env var. */ import * as fs from 'node:fs'; @@ -15,6 +20,7 @@ export interface Credentials { const CRED_DIR = path.join(os.homedir(), '.charter'); const CRED_FILE = path.join(CRED_DIR, 'credentials.json'); +const API_KEY_ENV_VAR = 'STACKBILT_API_KEY'; export function loadCredentials(): Credentials | null { if (!fs.existsSync(CRED_FILE)) return null; @@ -40,3 +46,27 @@ export function clearCredentials(): void { fs.unlinkSync(CRED_FILE); } } + +export interface ResolvedApiKey { + apiKey: string; + source: 'env' | 'credentials'; + baseUrl?: string; +} + +/** + * Resolve the Stackbilt API key from env var (preferred) or stored credentials. + * Returns null when neither source has a key. + */ +export function resolveApiKey(): ResolvedApiKey | null { + const fromEnv = process.env[API_KEY_ENV_VAR]; + if (fromEnv && fromEnv.trim().length > 0) { + return { apiKey: fromEnv.trim(), source: 'env' }; + } + const stored = loadCredentials(); + if (stored) { + return { apiKey: stored.apiKey, source: 'credentials', baseUrl: stored.baseUrl }; + } + return null; +} + +export { API_KEY_ENV_VAR }; diff --git a/packages/cli/src/http-client.ts b/packages/cli/src/http-client.ts index 5f8ac35..5598181 100644 --- a/packages/cli/src/http-client.ts +++ b/packages/cli/src/http-client.ts @@ -136,7 +136,10 @@ export class EngineClient { async scaffold(request: { description: string; project_type?: string; complexity?: string; seed?: number }): Promise { if (!this.apiKey) { - throw new Error('API key required for scaffold. Run `charter login --key sb_live_xxx` first.'); + throw new Error( + 'API key required for scaffold. Set STACKBILT_API_KEY in the environment, ' + + 'or (deprecated) run `charter login --key sb_live_xxx`.', + ); } const res = await fetch(`${GATEWAY_BASE_URL}/api/scaffold`, { From 869eb1d304456fc547de6e2bb33124a518005a08 Mon Sep 17 00:00:00 2001 From: Kurt Overmier Date: Thu, 16 Apr 2026 12:57:03 -0500 Subject: [PATCH 2/2] review(cli): fix test isolation, add baseUrl env var, wiring tests Addresses feedback on PR #111: - credentials.test.ts: mock node:fs so loadCredentials returns deterministic results instead of reading the developer's real ~/.charter/credentials.json. Empty/whitespace-env-var cases are now pinned to the stored-credentials fallback with stubbed data rather than being conditionally skipped. Added explicit "env wins when both sources set" test that was claimed by the describe block but never exercised. - STACKBILT_API_BASE_URL companion env var: resolveApiKey now returns a baseUrl from the env when source='env'. Fixes the silent base-URL drop for users migrating from `charter login --url https://custom ...`. - login.ts: --logout now emits the deprecation notice before clearing credentials. One-line fix; users exiting the deprecated surface see the upgrade path. - CLI README: "Authentication (optional)" gains a one-line security caveat about env-var inheritance by child processes, recommending per-invocation setting in CI over global export in shared shells. - auth-wiring.test.ts (new): mocks resolveApiKey and EngineClient; pins the useGateway decision for `run` (scaffold vs build) and asserts architect forwards the resolved apiKey + baseUrl into the client constructor. 5 tests covering env/credentials/null. All 16 auth-related tests pass (credentials: 9, login: 2, auth-wiring: 5). Full suite: 413/415 pass; the two failures are the same WSL-flaky precommit-hook integration test already flagged on PR #110 and unrelated to this diff. Co-Authored-By: Claude Opus 4.6 (1M context) --- CHANGELOG.md | 1 + packages/cli/README.md | 7 +- .../cli/src/__tests__/auth-wiring.test.ts | 149 ++++++++++++++++++ .../cli/src/__tests__/credentials.test.ts | 119 +++++++++++--- packages/cli/src/commands/login.ts | 1 + packages/cli/src/credentials.ts | 13 +- 6 files changed, 268 insertions(+), 22 deletions(-) create mode 100644 packages/cli/src/__tests__/auth-wiring.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 03873c1..e2919c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ The format is based on Keep a Changelog and follows Semantic Versioning. ### Added - **`STACKBILT_API_KEY` environment variable** — `charter run` and `charter architect` now resolve the API key from `STACKBILT_API_KEY` first, falling back to stored credentials only if the env var is absent or blank. This lets users authenticate the commercial commands without writing a token to `~/.charter/credentials.json`. +- **`STACKBILT_API_BASE_URL` environment variable** — companion to `STACKBILT_API_KEY`; sets a custom engine base URL for env-var-authenticated callers. Preserves parity with the stored-credentials path (`charter login --url …`). - `resolveApiKey()` helper exported from `@stackbilt/cli`'s credentials module (env-var precedence, trimmed, returns `{ apiKey, source: 'env' | 'credentials', baseUrl? }`). ### Deprecated diff --git a/packages/cli/README.md b/packages/cli/README.md index 160b787..1a323f0 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -77,12 +77,15 @@ charter surface --markdown # extract routes (Hono/Express) + D1 schema Governance commands (`validate`, `drift`, `blast`, `surface`, etc.) run locally and require no authentication. -Commands that reach the Stackbilt engine (`run`, `architect`) read their API key from the `STACKBILT_API_KEY` environment variable: +Commands that reach the Stackbilt engine (`run`, `architect`) read their API key from the `STACKBILT_API_KEY` environment variable. A custom engine URL can be supplied via `STACKBILT_API_BASE_URL`: ```bash -export STACKBILT_API_KEY=ea_xxx # or sb_live_xxx, sb_test_xxx +export STACKBILT_API_KEY=ea_xxx # or sb_live_xxx, sb_test_xxx +export STACKBILT_API_BASE_URL=https://engine.example # optional, for self-hosted engines ``` +Environment variables are inherited by any child processes spawned from the same shell and may appear in `/proc//environ`. In CI, prefer setting the variable per-invocation (e.g., a job-scoped secret) rather than exporting it globally in a shared developer shell. + The legacy `charter login --key …` command still works but is deprecated and will be removed in `@stackbilt/cli` 1.0 when gateway-bound commands move to a separate package. ## Human Onboarding (Copy/Paste) diff --git a/packages/cli/src/__tests__/auth-wiring.test.ts b/packages/cli/src/__tests__/auth-wiring.test.ts new file mode 100644 index 0000000..d0a3b1d --- /dev/null +++ b/packages/cli/src/__tests__/auth-wiring.test.ts @@ -0,0 +1,149 @@ +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// Hoisted mock state: vi.mock factories run before any imports, so the +// EngineClient class mock must reach these via vi.hoisted. +const hoisted = vi.hoisted(() => ({ + buildFn: vi.fn(), + scaffoldFn: vi.fn(), + constructorArgs: [] as Array<{ baseUrl?: string; apiKey?: string | null }>, +})); + +vi.mock('../credentials', async () => { + const actual = await vi.importActual('../credentials'); + return { ...actual, resolveApiKey: vi.fn() }; +}); + +vi.mock('../http-client', () => { + return { + EngineClient: class { + constructor(opts: { baseUrl?: string; apiKey?: string | null }) { + hoisted.constructorArgs.push(opts); + } + build = hoisted.buildFn; + scaffold = hoisted.scaffoldFn; + health = vi.fn(); + catalog = vi.fn(); + }, + }; +}); + +import { resolveApiKey } from '../credentials'; +import { architectCommand } from '../commands/architect'; +import { runCommand } from '../commands/run'; +import type { CLIOptions } from '../index'; + +const mockedResolveApiKey = vi.mocked(resolveApiKey); + +const options: CLIOptions = { + format: 'json', + configPath: '.charter', + ciMode: false, + yes: true, +}; + +function fakeBuildResult() { + return { + stack: [], + compatibility: { + pairs: [], + totalScore: 0, + normalizedScore: 0, + dominant: '', + tensions: [], + }, + scaffold: {}, + seed: 1, + receipt: 'receipt', + requirements: { + description: 'anything', + keywords: [], + constraints: {}, + complexity: 'moderate', + }, + }; +} + +function fakeScaffoldResult() { + return { + files: [], + fileSource: 'engine' as const, + nextSteps: [], + }; +} + +let tmpCwd: string; + +beforeEach(() => { + tmpCwd = fs.mkdtempSync(path.join(os.tmpdir(), 'charter-wiring-')); + process.chdir(tmpCwd); + fs.mkdirSync(path.join(tmpCwd, '.charter'), { recursive: true }); + hoisted.buildFn.mockReset().mockResolvedValue(fakeBuildResult()); + hoisted.scaffoldFn.mockReset().mockResolvedValue(fakeScaffoldResult()); + hoisted.constructorArgs.length = 0; + mockedResolveApiKey.mockReset(); + vi.spyOn(console, 'log').mockImplementation(() => {}); + vi.spyOn(process.stdout, 'write').mockImplementation(() => true); + vi.spyOn(process.stderr, 'write').mockImplementation(() => true); +}); + +afterEach(() => { + vi.restoreAllMocks(); + process.chdir(os.tmpdir()); + fs.rmSync(tmpCwd, { recursive: true, force: true }); +}); + +describe('architect — auth wiring', () => { + it('forwards the env-sourced API key (and custom baseUrl) to EngineClient', async () => { + mockedResolveApiKey.mockReturnValue({ + apiKey: 'ea_env_wiring', + source: 'env', + baseUrl: 'https://engine.example', + }); + + await architectCommand(options, ['a simple project description']); + + expect(hoisted.constructorArgs).toHaveLength(1); + expect(hoisted.constructorArgs[0].apiKey).toBe('ea_env_wiring'); + expect(hoisted.constructorArgs[0].baseUrl).toBe('https://engine.example'); + }); + + it('passes apiKey=null to EngineClient when resolveApiKey returns null', async () => { + mockedResolveApiKey.mockReturnValue(null); + + await architectCommand(options, ['unauthenticated fallback']); + + expect(hoisted.constructorArgs[0].apiKey).toBeNull(); + }); +}); + +describe('run — gateway vs engine routing', () => { + it('uses the gateway (scaffold) when the env var provides an API key', async () => { + mockedResolveApiKey.mockReturnValue({ apiKey: 'ea_env_gateway', source: 'env' }); + + await runCommand(options, ['a description', '--dry-run']); + + expect(hoisted.scaffoldFn).toHaveBeenCalledTimes(1); + expect(hoisted.buildFn).not.toHaveBeenCalled(); + }); + + it('falls back to engine /build when no API key is resolved', async () => { + mockedResolveApiKey.mockReturnValue(null); + + await runCommand(options, ['a description', '--dry-run']); + + expect(hoisted.buildFn).toHaveBeenCalledTimes(1); + expect(hoisted.scaffoldFn).not.toHaveBeenCalled(); + }); + + it('uses the gateway when login-stored credentials are resolved (parity with env path)', async () => { + mockedResolveApiKey.mockReturnValue({ apiKey: 'sb_live_stored', source: 'credentials' }); + + await runCommand(options, ['a description', '--dry-run']); + + expect(hoisted.scaffoldFn).toHaveBeenCalledTimes(1); + expect(hoisted.buildFn).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/cli/src/__tests__/credentials.test.ts b/packages/cli/src/__tests__/credentials.test.ts index 8b9fd04..401f590 100644 --- a/packages/cli/src/__tests__/credentials.test.ts +++ b/packages/cli/src/__tests__/credentials.test.ts @@ -1,22 +1,56 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { resolveApiKey, API_KEY_ENV_VAR } from '../credentials'; + +// Isolate loadCredentials from the developer's real ~/.charter/credentials.json +// by mocking node:fs. Each test configures the fs mock explicitly. +vi.mock('node:fs', async () => { + const actual = await vi.importActual('node:fs'); + return { + ...actual, + existsSync: vi.fn(() => false), + readFileSync: vi.fn(), + }; +}); + +import * as fs from 'node:fs'; +import { resolveApiKey, API_KEY_ENV_VAR, API_BASE_URL_ENV_VAR } from '../credentials'; + +const mockedFs = fs as unknown as { + existsSync: ReturnType; + readFileSync: ReturnType; +}; + +function stubStoredCredentials(apiKey: string, baseUrl?: string): void { + mockedFs.existsSync.mockReturnValue(true); + mockedFs.readFileSync.mockReturnValue(JSON.stringify({ apiKey, baseUrl })); +} + +function stubNoStoredCredentials(): void { + mockedFs.existsSync.mockReturnValue(false); + mockedFs.readFileSync.mockImplementation(() => { + throw new Error('readFileSync should not be called when existsSync=false'); + }); +} describe('resolveApiKey', () => { - const originalEnv = process.env[API_KEY_ENV_VAR]; + const originalKeyEnv = process.env[API_KEY_ENV_VAR]; + const originalBaseUrlEnv = process.env[API_BASE_URL_ENV_VAR]; beforeEach(() => { delete process.env[API_KEY_ENV_VAR]; + delete process.env[API_BASE_URL_ENV_VAR]; + mockedFs.existsSync.mockReset(); + mockedFs.readFileSync.mockReset(); + stubNoStoredCredentials(); }); afterEach(() => { - if (originalEnv === undefined) { - delete process.env[API_KEY_ENV_VAR]; - } else { - process.env[API_KEY_ENV_VAR] = originalEnv; - } + if (originalKeyEnv === undefined) delete process.env[API_KEY_ENV_VAR]; + else process.env[API_KEY_ENV_VAR] = originalKeyEnv; + if (originalBaseUrlEnv === undefined) delete process.env[API_BASE_URL_ENV_VAR]; + else process.env[API_BASE_URL_ENV_VAR] = originalBaseUrlEnv; }); - it('returns env var when set (env wins over stored credentials)', () => { + it('returns env var when set', () => { process.env[API_KEY_ENV_VAR] = 'ea_test_from_env_12345'; const result = resolveApiKey(); @@ -26,6 +60,17 @@ describe('resolveApiKey', () => { expect(result!.apiKey).toBe('ea_test_from_env_12345'); }); + it('env var wins when both env var and stored credentials are present', () => { + process.env[API_KEY_ENV_VAR] = 'ea_env_wins'; + stubStoredCredentials('sb_live_should_be_ignored', 'https://stored.example'); + + const result = resolveApiKey(); + + expect(result).not.toBeNull(); + expect(result!.source).toBe('env'); + expect(result!.apiKey).toBe('ea_env_wins'); + }); + it('trims whitespace from the env var', () => { process.env[API_KEY_ENV_VAR] = ' sb_test_abc '; @@ -36,25 +81,63 @@ describe('resolveApiKey', () => { expect(result!.apiKey).toBe('sb_test_abc'); }); - it('treats an empty env var as unset (falls through to credentials or null)', () => { + it('empty env var falls through to stored credentials', () => { process.env[API_KEY_ENV_VAR] = ''; + stubStoredCredentials('sb_live_from_disk'); const result = resolveApiKey(); - // We can't predict whether this machine has stored credentials, so we - // only assert the env path was NOT taken. - if (result !== null) { - expect(result.source).toBe('credentials'); - } + expect(result).not.toBeNull(); + expect(result!.source).toBe('credentials'); + expect(result!.apiKey).toBe('sb_live_from_disk'); }); - it('treats a whitespace-only env var as unset', () => { + it('whitespace-only env var falls through to stored credentials', () => { process.env[API_KEY_ENV_VAR] = ' \t '; + stubStoredCredentials('sb_live_from_disk'); + + const result = resolveApiKey(); + + expect(result).not.toBeNull(); + expect(result!.source).toBe('credentials'); + expect(result!.apiKey).toBe('sb_live_from_disk'); + }); + + it('returns null when neither env var nor stored credentials are present', () => { + stubNoStoredCredentials(); + + const result = resolveApiKey(); + + expect(result).toBeNull(); + }); + + it('env-var path adopts STACKBILT_API_BASE_URL when set', () => { + process.env[API_KEY_ENV_VAR] = 'ea_with_custom_url'; + process.env[API_BASE_URL_ENV_VAR] = 'https://engine.internal.example'; + + const result = resolveApiKey(); + + expect(result).not.toBeNull(); + expect(result!.source).toBe('env'); + expect(result!.baseUrl).toBe('https://engine.internal.example'); + }); + + it('env-var path leaves baseUrl undefined when STACKBILT_API_BASE_URL is unset', () => { + process.env[API_KEY_ENV_VAR] = 'ea_without_custom_url'; + + const result = resolveApiKey(); + + expect(result).not.toBeNull(); + expect(result!.baseUrl).toBeUndefined(); + }); + + it('credentials path carries baseUrl from the stored file', () => { + stubStoredCredentials('sb_live_from_disk', 'https://engine.custom.example'); const result = resolveApiKey(); - if (result !== null) { - expect(result.source).toBe('credentials'); - } + expect(result).not.toBeNull(); + expect(result!.source).toBe('credentials'); + expect(result!.baseUrl).toBe('https://engine.custom.example'); }); }); diff --git a/packages/cli/src/commands/login.ts b/packages/cli/src/commands/login.ts index eccf5a0..ceb5e14 100644 --- a/packages/cli/src/commands/login.ts +++ b/packages/cli/src/commands/login.ts @@ -29,6 +29,7 @@ function printDeprecationNotice(): void { export async function loginCommand(options: CLIOptions, args: string[]): Promise { if (args.includes('--logout')) { + printDeprecationNotice(); clearCredentials(); console.log('Credentials cleared.'); return EXIT_CODE.SUCCESS; diff --git a/packages/cli/src/credentials.ts b/packages/cli/src/credentials.ts index 5804d7f..0f3fcbb 100644 --- a/packages/cli/src/credentials.ts +++ b/packages/cli/src/credentials.ts @@ -21,6 +21,7 @@ export interface Credentials { const CRED_DIR = path.join(os.homedir(), '.charter'); const CRED_FILE = path.join(CRED_DIR, 'credentials.json'); const API_KEY_ENV_VAR = 'STACKBILT_API_KEY'; +const API_BASE_URL_ENV_VAR = 'STACKBILT_API_BASE_URL'; export function loadCredentials(): Credentials | null { if (!fs.existsSync(CRED_FILE)) return null; @@ -56,11 +57,19 @@ export interface ResolvedApiKey { /** * Resolve the Stackbilt API key from env var (preferred) or stored credentials. * Returns null when neither source has a key. + * + * When the env path is taken, an optional STACKBILT_API_BASE_URL env var may + * carry a custom engine base URL — keeping parity with `charter login --url`. */ export function resolveApiKey(): ResolvedApiKey | null { const fromEnv = process.env[API_KEY_ENV_VAR]; if (fromEnv && fromEnv.trim().length > 0) { - return { apiKey: fromEnv.trim(), source: 'env' }; + const baseUrlFromEnv = process.env[API_BASE_URL_ENV_VAR]?.trim(); + return { + apiKey: fromEnv.trim(), + source: 'env', + baseUrl: baseUrlFromEnv && baseUrlFromEnv.length > 0 ? baseUrlFromEnv : undefined, + }; } const stored = loadCredentials(); if (stored) { @@ -69,4 +78,4 @@ export function resolveApiKey(): ResolvedApiKey | null { return null; } -export { API_KEY_ENV_VAR }; +export { API_KEY_ENV_VAR, API_BASE_URL_ENV_VAR };