diff --git a/src/commands/init.test.ts b/src/commands/init.test.ts index 84646f49..622c81dc 100644 --- a/src/commands/init.test.ts +++ b/src/commands/init.test.ts @@ -21,6 +21,8 @@ import { resetDryRunBannerForTesting } from '../lib/client-factory.js'; import { readProfile, writeProfile } from '../lib/credentials.js'; import type { MeResponse } from './auth.js'; import type { AgentFs } from './agent.js'; +import type * as AgentModule from './agent.js'; +import { runInstall } from './agent.js'; import { Command } from 'commander'; import type { InitDeps } from './init.js'; import { runInit, addSetupOptions } from './init.js'; @@ -44,6 +46,13 @@ vi.mock('node:fs', async importOriginal => { }; }); +// runInstall gets the same pass-through double as the node:fs functions above, +// so a test can make the agent install step write a stray stdout line. +vi.mock('./agent.js', async importOriginal => { + const actual = await importOriginal(); + return { ...actual, runInstall: vi.fn(actual.runInstall) }; +}); + // --------------------------------------------------------------------------- // Fixtures // --------------------------------------------------------------------------- @@ -216,6 +225,7 @@ afterEach(() => { for (const fn of [mkdirSync, readFileSync, renameSync, unlinkSync, writeFileSync]) { vi.mocked(fn).mockReset(); } + vi.mocked(runInstall).mockReset(); }); describe('runInit — session-only environment credentials', () => { @@ -685,6 +695,130 @@ describe('runInit — happy path (interactive)', () => { expect(captured.stderr.some(line => line.includes('setup identity lookup failed'))).toBe(true); }); + /** + * Runs setup with the profile read in resolveReportedEndpoint failing: once + * the configure and whoami fetches are done, reading the credentials file throws. + */ + async function runInitWithFailingProfileRead( + debug: boolean, + ): Promise<{ captured: Captured; failedReads: number }> { + const { captured, deps } = makeCapture(); + const fetchMock = makeOkFetch()!; + const actual = await vi.importActual('node:fs'); + let fetchCount = 0; + let failedReads = 0; + const trackingFetch = vi.fn( + async (input: Parameters[0], init?: Parameters[1]) => { + fetchCount++; + return fetchMock(input, init); + }, + ); + + vi.mocked(readFileSync).mockImplementation((...args) => { + if (fetchCount >= 2 && args[0] === credentialsPath) { + failedReads++; + throw new Error('simulated credentials read error'); + } + return actual.readFileSync(...args); + }); + + await runInit( + makeBaseOpts({ apiKey: 'sk-user-json-test', debug, noAgent: true, output: 'json' }), + { + ...deps, + fetchImpl: trackingFetch, + credentialsPath, + isTTY: false, + }, + ); + + return { captured, failedReads }; + } + + it('debug mode reports resolve endpoint profile read failure without failing the command', async () => { + const { captured, failedReads } = await runInitWithFailingProfileRead(true); + + const parsed = JSON.parse(captured.stdout.join('\n')) as Record; + expect(parsed.status).toBe('initialized'); + expect(failedReads).toBeGreaterThan(0); + expect( + captured.stderr.some(line => + line.includes( + '[debug] resolve endpoint profile read failed: simulated credentials read error', + ), + ), + ).toBe(true); + }); + + it('keeps stderr free of [debug] lines when the profile read fails without --debug', async () => { + const { captured, failedReads } = await runInitWithFailingProfileRead(false); + + const parsed = JSON.parse(captured.stdout.join('\n')) as Record; + expect(parsed.status).toBe('initialized'); + // The read did fail; it just must not be reported. + expect(failedReads).toBeGreaterThan(0); + expect(captured.stderr.filter(line => line.includes('[debug]'))).toEqual([]); + }); + + /** + * Runs setup with the agent install step writing one stray non-JSON stdout + * line before its real result. runInstall prints a single JSON document in + * json mode, so the parse fallback is only reachable by faking that output. + */ + async function runInitWithStrayInstallOutput(debug: boolean): Promise { + const { captured, deps } = makeCapture(); + const { fs: agentFs } = makeMemFs(); + const actual = await vi.importActual('./agent.js'); + vi.mocked(runInstall).mockImplementationOnce(async (installOpts, installDeps) => { + installDeps?.stdout?.('not json'); + await actual.runInstall(installOpts, installDeps); + }); + + await runInit(makeBaseOpts({ apiKey: 'sk-user-json-test', debug, output: 'json' }), { + ...deps, + fetchImpl: makeOkFetch(), + credentialsPath, + isTTY: false, + cwd: CWD, + fs: agentFs, + }); + + return captured; + } + + it('debug mode reports a non-JSON line from the agent install step and keeps the summary', async () => { + const captured = await runInitWithStrayInstallOutput(true); + + const parsed = JSON.parse(captured.stdout.join('\n')) as { + status: string; + agent: { action: string; skills?: string[] } | null; + }; + expect(parsed.status).toBe('initialized'); + // The real install result after the stray line is still picked up. + expect(parsed.agent?.action).toBe('installed'); + expect(parsed.agent?.skills).toContain('testsprite-verify'); + expect(vi.mocked(runInstall)).toHaveBeenCalledTimes(1); + expect( + captured.stderr.filter(line => + line.includes('[debug] setup agent install output parse non-JSON:'), + ), + ).toHaveLength(1); + }); + + it('keeps stderr free of [debug] lines when the agent install step writes a non-JSON line without --debug', async () => { + const captured = await runInitWithStrayInstallOutput(false); + + const parsed = JSON.parse(captured.stdout.join('\n')) as { + status: string; + agent: { action: string } | null; + }; + expect(parsed.status).toBe('initialized'); + expect(parsed.agent?.action).toBe('installed'); + // The stray line was written; it just must not be reported. + expect(vi.mocked(runInstall)).toHaveBeenCalledTimes(1); + expect(captured.stderr.filter(line => line.includes('[debug]'))).toEqual([]); + }); + it('stops before skill installation when the identity request is interrupted', async () => { const { captured, deps } = makeCapture(); const { fs: agentFs, writeCalls, mkdirCalls } = makeMemFs(); diff --git a/src/commands/init.ts b/src/commands/init.ts index 29e92dfa..73d99f8e 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -56,13 +56,21 @@ const DEFAULT_API_URL = 'https://api.testsprite.com'; * configuring staging/dev (codex). On the real path this runs AFTER the profile * is written, so the persisted apiUrl is reflected faithfully. */ -function resolveReportedEndpoint(opts: InitOptions, deps: InitDeps): string { +function resolveReportedEndpoint( + opts: InitOptions, + deps: InitDeps, + stderrFn?: (line: string) => void, +): string { const env = deps.env ?? process.env; const envApiUrl = normalizeEnvVar(env.TESTSPRITE_API_URL); let existing: string | undefined; try { existing = readProfile(opts.profile, { path: deps.credentialsPath })?.apiUrl; - } catch { + } catch (err) { + if (opts.debug && stderrFn) { + const reason = err instanceof Error ? err.message : String(err); + stderrFn(`[debug] resolve endpoint profile read failed: ${reason}`); + } existing = undefined; } return opts.endpointUrl ?? envApiUrl ?? existing ?? DEFAULT_API_URL; @@ -456,7 +464,7 @@ export async function runInit(opts: InitOptions, deps: InitDeps = {}): Promise, --from-env (reads TESTSPRITE_API_KEY), or run interactively.', - resolveReportedEndpoint(opts, deps), + resolveReportedEndpoint(opts, deps, stderrFn), ); } // JSON-output guard: an interactive secret prompt writes to stdout and would @@ -542,7 +550,7 @@ export async function runInit(opts: InitOptions, deps: InitDeps = {}): Promise 0) { capturedInstallResults = parsed; } - } catch { + } catch (err) { + if (opts.debug) { + const reason = err instanceof Error ? err.message : String(err); + stderrFn(`[debug] setup agent install output parse non-JSON: ${reason}`); + } // ignore non-JSON lines (shouldn't happen in json mode, but be safe) } }; @@ -709,7 +721,7 @@ export async function runInit(opts: InitOptions, deps: InitDeps = {}): Promise { false, ); }); + + it('Fix 4 — emits debug log when duplicate-name advisory lookup fails in debug mode', async () => { + const { credentialsPath } = makeCreds(); + const codeFile = writeCodeFile('// test code'); + const fetchImpl = makeFetch((url, init) => { + if ((init.method ?? 'GET') === 'GET' && url.includes('/tests')) { + return { + status: 403, + body: { + error: { + code: 'AUTH_FORBIDDEN', + message: 'Forbidden', + nextAction: '', + requestId: 'r1', + details: {}, + }, + }, + }; + } + return { body: SAMPLE_RESPONSE }; + }); + + const stderrLines: string[] = []; + await runCreate( + { + profile: 'default', + output: 'json', + debug: true, + projectId: 'project_alice', + type: 'frontend', + name: 'another test', + codeFile, + }, + { credentialsPath, fetchImpl, stdout: () => {}, stderr: line => stderrLines.push(line) }, + ); + + expect( + stderrLines.some(l => l.includes('[debug] duplicate-name advisory lookup failed: Forbidden')), + ).toBe(true); + }); + + it('Fix 4 — keeps stderr free of [debug] lines when the lookup fails without --debug', async () => { + const { credentialsPath } = makeCreds(); + const codeFile = writeCodeFile('// test code'); + let listCalled = false; + const fetchImpl = makeFetch((url, init) => { + if ((init.method ?? 'GET') === 'GET' && url.includes('/tests')) { + listCalled = true; + return { + status: 403, + body: { + error: { + code: 'AUTH_FORBIDDEN', + message: 'Forbidden', + nextAction: '', + requestId: 'r1', + details: {}, + }, + }, + }; + } + return { body: SAMPLE_RESPONSE }; + }); + + const stderrLines: string[] = []; + const result = await runCreate( + { + profile: 'default', + output: 'json', + debug: false, + projectId: 'project_alice', + type: 'frontend', + name: 'another test', + codeFile, + }, + { credentialsPath, fetchImpl, stdout: () => {}, stderr: line => stderrLines.push(line) }, + ); + + // The lookup did fail and the create still went through; nothing is reported. + expect(listCalled).toBe(true); + expect(result.testId).toBe('test_new'); + expect(stderrLines.filter(l => l.includes('[debug]'))).toEqual([]); + }); }); // --------------------------------------------------------------------------- @@ -10198,6 +10281,62 @@ describe('runCreateFromPlan', () => { expect(result.testId).toBe(SAMPLE_RESPONSE.testId); }); + /** Creates from FE_PLAN while the dup-name listing (GET /tests) fails with 403. */ + async function createFromPlanWithFailingLookup(debug: boolean) { + const { credentialsPath } = makeCreds(); + let listAttempted = false; + const fetchImpl = makeFetch((url, init) => { + if ((init.method ?? 'GET') === 'GET' && url.includes('/tests')) { + listAttempted = true; + return { + status: 403, + body: { + error: { + code: 'AUTH_FORBIDDEN', + message: 'Forbidden', + nextAction: '', + requestId: 'r', + details: {}, + }, + }, + }; + } + return { body: SAMPLE_RESPONSE }; + }); + const stderrLines: string[] = []; + + const result = await runCreateFromPlan( + { profile: 'default', output: 'json', debug, planFrom: writePlanFile(FE_PLAN) }, + { + credentialsPath, + fetchImpl, + stdout: () => undefined, + stderr: line => stderrLines.push(line), + }, + ); + + return { result, stderrLines, listAttempted }; + } + + it('[finding-4] reports a failed dup-name lookup on the plan-from path under --debug', async () => { + const { result, stderrLines, listAttempted } = await createFromPlanWithFailingLookup(true); + + expect(listAttempted).toBe(true); + expect(result.testId).toBe(SAMPLE_RESPONSE.testId); + expect( + stderrLines.some(l => l.includes('[debug] duplicate-name advisory lookup failed: Forbidden')), + ).toBe(true); + }); + + it('[finding-4] keeps stderr free of [debug] lines when the plan-from lookup fails without --debug', async () => { + const { result, stderrLines, listAttempted } = await createFromPlanWithFailingLookup(false); + + // The lookup did fail and the create still went through; nothing is reported. + expect(listAttempted).toBe(true); + expect(result.testId).toBe(SAMPLE_RESPONSE.testId); + expect(stderrLines.filter(l => l.includes('[debug]'))).toEqual([]); + }); + it('[finding-4] skips dup-name lookup when dry-run is true', async () => { const { credentialsPath } = makeCreds(); let listAttempted = false; diff --git a/src/commands/test.ts b/src/commands/test.ts index ffab7a82..6047b596 100644 --- a/src/commands/test.ts +++ b/src/commands/test.ts @@ -1360,6 +1360,7 @@ async function emitDupNameAdvisoryIfNeeded( projectId: string | undefined, name: string | undefined, stderrFn: (line: string) => void, + debug?: boolean, ): Promise { if (!projectId || !name) return; // B: ordinary advisory failures must not block the create critical path. @@ -1397,6 +1398,10 @@ async function emitDupNameAdvisoryIfNeeded( } } catch (err) { if (err instanceof InterruptError) throw err; + if (debug) { + const reason = err instanceof Error ? err.message : String(err); + stderrFn(`[debug] duplicate-name advisory lookup failed: ${reason}`); + } // Other lookup failures are best-effort and must not block the create. } finally { clearTimeout(timer); @@ -1690,7 +1695,7 @@ export async function runCreate( // B3: best-effort duplicate-name advisory. Skip under --dry-run. if (!opts.dryRun) { const stderrFn = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`)); - await emitDupNameAdvisoryIfNeeded(client, projectId, opts.name, stderrFn); + await emitDupNameAdvisoryIfNeeded(client, projectId, opts.name, stderrFn, opts.debug); } const response = await client.post('/tests', { @@ -3735,7 +3740,7 @@ export async function runCreateFromPlan( // The plan's projectId + name are available after validation above. Skip // under dry-run (no network calls); swallow all errors (advisory only). if (!opts.dryRun) { - await emitDupNameAdvisoryIfNeeded(client, plan.projectId, plan.name, stderrFn); + await emitDupNameAdvisoryIfNeeded(client, plan.projectId, plan.name, stderrFn, opts.debug); } const response = await client.post('/tests', {