Skip to content

Commit f7b8d55

Browse files
fix(test): reject empty code get --out and strip code-file BOM
When test code get --out receives an empty inline body, closeOutputFile left a zero-byte file with exit 0 — scripts and agents treated that as a successful download. Abort the sink, unlink any artifact, and surface VALIDATION_ERROR instead. Plan/steps JSON already strip a leading UTF-8 BOM from PowerShell 5.1 files; apply the same strip to --code-file reads so uploaded test source is not corrupted by an invisible U+FEFF prefix.
1 parent 18f6e6e commit f7b8d55

2 files changed

Lines changed: 102 additions & 4 deletions

File tree

src/commands/test.test.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1556,6 +1556,44 @@ describe('runCodeGet', () => {
15561556
),
15571557
).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
15581558
});
1559+
1560+
it('--out (text mode) rejects empty inline code with VALIDATION_ERROR and leaves no artifact', async () => {
1561+
const { credentialsPath } = makeCreds();
1562+
const emptyCode: CliTestCode = { ...TEST_CODE_INLINE, code: '' };
1563+
const fetchImpl = makeFetch(() => ({ body: emptyCode }));
1564+
const dir = mkdtempSync(join(tmpdir(), 'cli-test-code-empty-out-'));
1565+
const target = join(dir, 'empty.ts');
1566+
await expect(
1567+
runCodeGet(
1568+
{
1569+
profile: 'default',
1570+
output: 'text',
1571+
debug: false,
1572+
testId: 'test_fe',
1573+
out: target,
1574+
},
1575+
{ credentialsPath, fetchImpl },
1576+
),
1577+
).rejects.toMatchObject({
1578+
code: 'VALIDATION_ERROR',
1579+
exitCode: 5,
1580+
details: expect.objectContaining({ field: 'out' }),
1581+
});
1582+
expect(existsSync(target)).toBe(false);
1583+
});
1584+
1585+
it('text mode without --out still hints on stderr when inline code is empty', async () => {
1586+
const { credentialsPath } = makeCreds();
1587+
const emptyCode: CliTestCode = { ...TEST_CODE_INLINE, code: '' };
1588+
const fetchImpl = makeFetch(() => ({ body: emptyCode }));
1589+
const stderr: string[] = [];
1590+
const got = await runCodeGet(
1591+
{ profile: 'default', output: 'text', debug: false, testId: 'test_fe' },
1592+
{ credentialsPath, fetchImpl, stderr: line => stderr.push(line) },
1593+
);
1594+
expect(got.code).toBe('');
1595+
expect(stderr.join('\n')).toContain('no code generated yet');
1596+
});
15591597
});
15601598

15611599
describe('runCodePut', () => {
@@ -1610,6 +1648,30 @@ describe('runCodePut', () => {
16101648
expect(sent.headers.get('content-type')).toBe('application/json');
16111649
});
16121650

1651+
it('strips a UTF-8 BOM from --code-file before uploading (Windows PowerShell 5.1 default)', async () => {
1652+
const { credentialsPath } = makeCreds();
1653+
const dir = mkdtempSync(join(tmpdir(), 'cli-p4-bom-'));
1654+
const codeFile = join(dir, 'updated.spec.ts');
1655+
writeFileSync(codeFile, '\uFEFF' + 'updated body', 'utf8');
1656+
let seenBody: unknown;
1657+
const fetchImpl = makeFetch((_url, init) => {
1658+
seenBody = init.body ? JSON.parse(init.body as string) : undefined;
1659+
return { body: SAMPLE_RESPONSE };
1660+
});
1661+
await runCodePut(
1662+
{
1663+
profile: 'default',
1664+
output: 'json',
1665+
debug: false,
1666+
testId: 'test_alpha',
1667+
codeFile,
1668+
expectedVersion: 'v3',
1669+
},
1670+
{ credentialsPath, fetchImpl, stdout: () => undefined, stderr: () => undefined },
1671+
);
1672+
expect(seenBody).toEqual({ code: 'updated body' });
1673+
});
1674+
16131675
it('forwards --language in the body when set', async () => {
16141676
const { credentialsPath } = makeCreds();
16151677
const codeFile = writeCodeFile('print("hi")');

src/commands/test.ts

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
1-
import { createWriteStream, readFileSync, readdirSync, statSync, type WriteStream } from 'node:fs';
1+
import {
2+
createWriteStream,
3+
existsSync,
4+
readFileSync,
5+
readdirSync,
6+
statSync,
7+
unlinkSync,
8+
type WriteStream,
9+
} from 'node:fs';
210
import { stat } from 'node:fs/promises';
311
import { dirname, extname, isAbsolute, join, resolve } from 'node:path';
412
import { randomUUID } from 'node:crypto';
@@ -879,7 +887,7 @@ function readCodeFileGuarded(path: string): string {
879887

880888
function readCodeFile(path: string): string {
881889
try {
882-
return readFileSync(resolveAbsolute(path), 'utf8');
890+
return stripBom(readFileSync(resolveAbsolute(path), 'utf8'));
883891
} catch (err) {
884892
const code = (err as NodeJS.ErrnoException).code;
885893
if (code === 'ENOENT') {
@@ -3211,7 +3219,7 @@ export async function runCodeGet(opts: CodeGetOptions, deps: TestDeps = {}): Pro
32113219
return code;
32123220
}
32133221

3214-
const fileSink = opts.out !== undefined ? openOutputFile(opts.out) : null;
3222+
let fileSink = opts.out !== undefined ? openOutputFile(opts.out) : null;
32153223
const out = fileSink ? makeFileOutput(opts.output, fileSink) : makeOutput(opts.output, deps);
32163224
const client = makeClient(opts, deps);
32173225

@@ -3234,13 +3242,28 @@ export async function runCodeGet(opts: CodeGetOptions, deps: TestDeps = {}): Pro
32343242
// P2-10: draft test with no code yet — empty body would produce
32353243
// silent empty stdout. Print a friendly hint to stderr instead so
32363244
// the operator knows what happened, and keep exit 0.
3245+
//
3246+
// With `--out`, refuse to leave a zero-byte artifact behind: agents
3247+
// and scripts that check file size would otherwise treat exit 0 as
3248+
// a successful download.
3249+
if (fileSink) {
3250+
await abortOutputFile(fileSink);
3251+
fileSink = null;
3252+
throw localValidationError(
3253+
'out',
3254+
'test has no generated code yet — run the test first (refusing to write an empty --out file)',
3255+
);
3256+
}
32373257
const stderrFn = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`));
32383258
stderrFn('(no code generated yet — run the test first)');
32393259
} else {
32403260
await out.writeChunk(code.code);
32413261
}
32423262

3243-
if (fileSink) await closeOutputFile(fileSink);
3263+
if (fileSink) {
3264+
await closeOutputFile(fileSink);
3265+
fileSink = null;
3266+
}
32443267
return code;
32453268
} catch (err) {
32463269
if (fileSink) await closeOutputFile(fileSink).catch(() => undefined);
@@ -7842,6 +7865,19 @@ function closeOutputFile(sink: FileSink): Promise<void> {
78427865
});
78437866
}
78447867

7868+
/** Tear down an opened `--out` sink without leaving a zero-byte artifact. */
7869+
function abortOutputFile(sink: FileSink): Promise<void> {
7870+
return new Promise(resolve => {
7871+
sink.stream.destroy();
7872+
try {
7873+
if (existsSync(sink.path)) unlinkSync(sink.path);
7874+
} catch {
7875+
// Best-effort cleanup — the validation error is the primary signal.
7876+
}
7877+
resolve();
7878+
});
7879+
}
7880+
78457881
/** A presigned `code` body is any `https://` URL — never anything else. */
78467882
export function isPresignedCodeUrl(code: string): boolean {
78477883
return code.startsWith('https://');

0 commit comments

Comments
 (0)