Skip to content

fix(config): treat empty env vars as unset in loadConfig #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zeshi-du merged 2 commits into from
Jul 6, 2026
Merged

fix(config): treat empty env vars as unset in loadConfig #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0a1ef36
fix(config): treat empty env vars as unset in loadConfig
SahilRakhaiya05 Jul 6, 2026
9feeacb
test: address review — dry-run whoami uses offline client; fix whites…
SahilRakhaiya05 Jul 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/commands/auth.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -850,6 +850,20 @@ describe('runWhoami', () => {
expect(printed).toEqual(sampleMe);
});

it('dry-run: whitespace-only TESTSPRITE_API_URL falls through to prod default endpoint', async () => {
const { capture, deps } = makeCapture();
await runWhoami(
{ profile: 'default', output: 'text', debug: false, dryRun: true },
{
...deps,
env: { TESTSPRITE_API_URL: ' ' },
credentialsPath,
},
);
const out = capture.stdout.join('\n');
expect(out).toContain('endpoint: https://api.testsprite.com');
});

Comment on lines +853 to +866

@coderabbitai coderabbitai Bot Jul 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect runWhoami's dry-run branch for an early return before client.get
ast-grep run --pattern 'export async function runWhoami($_, $_) { $$$ }' --lang typescript src/commands/auth.ts

Repository: TestSprite/testsprite-cli

Length of output: 163

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the relevant file and inspect the dry-run branch in src/commands/auth.ts.
ast-grep outline src/commands/auth.ts --view expanded || true

printf '\n--- auth.ts dry-run references ---\n'
rg -n "dryRun|client\.get\('/me'|resolvedEndpoint|runWhoami" src/commands/auth.ts

printf '\n--- auth.ts relevant slice ---\n'
sed -n '180,280p' src/commands/auth.ts

printf '\n--- auth.test.ts relevant slice ---\n'
sed -n '830,890p' src/commands/auth.test.ts

Repository: TestSprite/testsprite-cli

Length of output: 8300

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect docs / nearby implementation for dry-run and endpoint resolution behavior.
rg -n "dry-run|whitespace|TESTSPRITE_API_URL|normalize|whoami" DOCUMENTATION.md src/commands src/lib src/config* src -g '!**/dist/**' || true

printf '\n--- possible config normalization helpers ---\n'
rg -n "function normalize|const normalize|normalizeEnv|trim\\(" src -g '*.ts' || true

Repository: TestSprite/testsprite-cli

Length of output: 50382

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- client-factory dry-run slice ---'
sed -n '1,220p' src/lib/client-factory.ts

printf '\n%s\n' '--- client-factory request creation slice ---'
sed -n '220,360p' src/lib/client-factory.ts

printf '\n%s\n' '--- auth.ts surrounding makeHttpClient call ---'
sed -n '201,255p' src/commands/auth.ts

Repository: TestSprite/testsprite-cli

Length of output: 14032

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- dry-run sample registry for whoami ---'
sed -n '360,395p' src/lib/dry-run/samples.ts

printf '\n%s\n' '--- config normalization helper ---'
sed -n '1,120p' src/lib/config.ts

printf '\n%s\n' '--- auth test nearby whitespace/env tests ---'
sed -n '700,875p' src/commands/auth.test.ts

Repository: TestSprite/testsprite-cli

Length of output: 10713

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- client-factory dry-run behavior ---'
sed -n '1,220p' src/lib/client-factory.ts

printf '\n%s\n' '--- client-factory request path around dry-run handling ---'
sed -n '220,320p' src/lib/client-factory.ts

printf '\n%s\n' '--- whoami dry-run sample registry entry ---'
sed -n '372,388p' src/lib/dry-run/samples.ts

Repository: TestSprite/testsprite-cli

Length of output: 12401

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
text = Path('src/commands/auth.ts').read_text()
# Print the runWhoami function body only.
start = text.index('export async function runWhoami')
end = text.index('export async function runLogout')
print(text[start:end])
PY

Repository: TestSprite/testsprite-cli

Length of output: 2500

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline src/lib/client-factory.ts --view expanded || true
printf '\n--- makeHttpClient slice ---\n'
sed -n '120,320p' src/lib/client-factory.ts

Repository: TestSprite/testsprite-cli

Length of output: 8408

Drop the custom fetch stub in this dry-run test
runWhoami(..., dryRun: true) already uses the canned offline client, and whoami has a sample in src/lib/dry-run/samples.ts. Removing fetchImpl here keeps the test on the real --dry-run path instead of a local mock.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/commands/auth.test.ts` around lines 853 - 867, The dry-run whoami test is
still injecting a custom fetch stub, which bypasses the real offline dry-run
path. Update the test in auth.test.ts so the runWhoami call with dryRun: true
relies on the canned client/sample behavior from whoami and
src/lib/dry-run/samples.ts instead of passing fetchImpl:
makeFetch(meResponse()), while keeping the TESTSPRITE_API_URL whitespace
assertion intact.

Source: Path instructions

it('L1788: text output includes the resolved endpoint URL', async () => {
writeProfile(
'default',
Expand Down
7 changes: 4 additions & 3 deletions src/commands/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
readProfile,
writeProfile,
} from '../lib/credentials.js';
import { loadConfig } from '../lib/config.js';
import { loadConfig, normalizeEnvVar } from '../lib/config.js';
import { emitDeprecationNotice } from '../lib/deprecate.js';
import type { OutputMode } from '../lib/output.js';
import { GLOBAL_OPTS_HINT, Output, resolveOutputMode } from '../lib/output.js';
Expand Down Expand Up @@ -85,7 +85,7 @@ export async function runConfigure(opts: ConfigureOptions, deps: AuthDeps = {}):
// treated as unset. Without this, `''` (e.g. `export TESTSPRITE_API_URL=` in a
// shell profile) is non-nullish and would short-circuit the `??` chains below to
// an empty endpoint instead of falling through to the profile / prod default.
const envApiUrl = env.TESTSPRITE_API_URL?.trim() || undefined;
const envApiUrl = normalizeEnvVar(env.TESTSPRITE_API_URL);

// Dry-run: do not prompt, do not read env, do not write credentials.
// Print the canned success shape so an agent sees exactly the JSON it
Expand Down Expand Up @@ -208,7 +208,8 @@ export async function runWhoami(opts: CommonOptions, deps: AuthDeps = {}): Promi
// displayed URL always matches where requests actually go (dogfood L1788).
let resolvedEndpoint: string;
if (opts.dryRun) {
resolvedEndpoint = opts.endpointUrl ?? env.TESTSPRITE_API_URL ?? 'https://api.testsprite.com';
resolvedEndpoint =
opts.endpointUrl ?? normalizeEnvVar(env.TESTSPRITE_API_URL) ?? 'https://api.testsprite.com';
} else {
const credentialsPath = deps.credentialsPath ?? defaultCredentialsPath();
const config = loadConfig({
Expand Down
3 changes: 2 additions & 1 deletion src/commands/init.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
parseRequestTimeoutFlag,
type CommonOptions as FactoryCommonOptions,
} from '../lib/client-factory.js';
import { normalizeEnvVar } from '../lib/config.js';
import { emitDeprecationNotice } from '../lib/deprecate.js';
import { CLIError } from '../lib/errors.js';
import { GLOBAL_OPTS_HINT, Output, resolveOutputMode } from '../lib/output.js';
Expand All @@ -39,7 +40,7 @@ const DEFAULT_API_URL = 'https://api.testsprite.com';
*/
function resolveReportedEndpoint(opts: InitOptions, deps: InitDeps): string {
const env = deps.env ?? process.env;
const envApiUrl = env.TESTSPRITE_API_URL?.trim() || undefined;
const envApiUrl = normalizeEnvVar(env.TESTSPRITE_API_URL);
let existing: string | undefined;
try {
existing = readProfile(opts.profile, { path: deps.credentialsPath })?.apiUrl;
Expand Down
21 changes: 20 additions & 1 deletion src/lib/client-factory.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,6 @@ describe('makeHttpClient - API key validation', () => {
['smart dash', 'sk-user-abc\u2013def'],
['smart quote', 'sk-user-\u201cabc\u201d'],
['emoji', 'sk-user-abc\u{1f600}'],
['whitespace-only', ' '],
])('rejects a malformed configured API key with %s before fetch/retry', (_label, apiKey) => {
const fetchImpl = vi.fn();
let caught: unknown;
Expand All @@ -362,6 +361,26 @@ describe('makeHttpClient - API key validation', () => {
expect(apiErr.exitCode).toBe(5);
expect(apiErr.nextAction).toContain('api-key');
});

it('treats a whitespace-only TESTSPRITE_API_KEY env var as unset (AUTH_REQUIRED)', () => {
const fetchImpl = vi.fn();
let caught: unknown;
try {
makeHttpClient(
{ profile: 'default', output: 'json', debug: false, dryRun: false },
{
env: { TESTSPRITE_API_KEY: ' ' } as NodeJS.ProcessEnv,
credentialsPath: NO_CREDS_PATH,
fetchImpl,
},
);
} catch (err) {
caught = err;
}
expect(fetchImpl).not.toHaveBeenCalled();
expect(caught).toBeInstanceOf(ApiError);
expect((caught as ApiError).code).toBe('AUTH_REQUIRED');
});
});

describe('assertValidApiKeyHeaderValue', () => {
Expand Down
22 changes: 22 additions & 0 deletions src/lib/config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,28 @@ describe('loadConfig', () => {
const config = loadConfig({ profile: 'dev', env: {}, credentialsPath });
expect(config.apiKey).toBe('sk-dev');
});

it('treats empty / whitespace TESTSPRITE_API_URL as unset (falls through to profile)', () => {
writeProfile(
'default',
{ apiKey: 'sk-file', apiUrl: 'https://api.example.com:8443' },
{ path: credentialsPath },
);
const config = loadConfig({
env: { TESTSPRITE_API_URL: ' ' },
credentialsPath,
});
expect(config.apiUrl).toBe('https://api.example.com:8443');
});

it('treats empty / whitespace TESTSPRITE_API_KEY as unset (falls through to profile)', () => {
writeProfile('default', { apiKey: 'sk-file' }, { path: credentialsPath });
const config = loadConfig({
env: { TESTSPRITE_API_KEY: '' },
credentialsPath,
});
expect(config.apiKey).toBe('sk-file');
});
});

describe('defaultConfigPath', () => {
Expand Down
15 changes: 13 additions & 2 deletions src/lib/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ export interface LoadConfigOptions {

const DEFAULT_API_URL = 'https://api.testsprite.com';

/** Treat empty / whitespace-only env values as unset for `??` resolution chains. */
export function normalizeEnvVar(value: string | undefined): string | undefined {
return value?.trim() || undefined;
}

export function defaultConfigPath(): string {
return join(homedir(), '.testsprite', 'config');
}
Expand All @@ -38,9 +43,15 @@ export function loadConfig(options: LoadConfigOptions = {}): Config {
const credentialsPath = options.credentialsPath ?? defaultCredentialsPath();
const fileEntry = readProfile(profile, { path: credentialsPath });

// Empty / whitespace-only env vars are treated as unset so they do not
// short-circuit the `??` chain (e.g. `export TESTSPRITE_API_URL=` in a shell
// profile). Matches the normalization in auth configure and init/setup.
const envApiUrl = normalizeEnvVar(env.TESTSPRITE_API_URL);
const envApiKey = normalizeEnvVar(env.TESTSPRITE_API_KEY);

return {
apiUrl: options.endpointUrl ?? env.TESTSPRITE_API_URL ?? fileEntry?.apiUrl ?? DEFAULT_API_URL,
apiKey: env.TESTSPRITE_API_KEY ?? fileEntry?.apiKey,
apiUrl: options.endpointUrl ?? envApiUrl ?? fileEntry?.apiUrl ?? DEFAULT_API_URL,
apiKey: envApiKey ?? fileEntry?.apiKey,
profile,
};
}
Loading