[Hackathon] fix(cli): reject blank --expected-step-count instead of sending If-Match-Step-Count: 0 #646

Verified against (commit SHA or released version)

Commit a18d37c8183c1cc39190cfdf2a7a82897e4e113c, package @testsprite/testsprite-cli 0.15.0, Node 24.19.0, Linux cloud container. Production source and lockfile were unmodified. The parser on current main was also read before filing and still uses the same conversion; the runtime evidence below is specifically for the pinned commit.

Discord Username / User ID

frankzhu94

What does this improvement do?

Reject an explicitly blank --expected-step-count locally instead of silently turning it into the meaningful precondition If-Match-Step-Count: 0.

An unset quoted shell variable can produce this input:

EXPECTED_STEP_COUNT=""
testsprite test plan put test_1 --steps steps.json \
  --expected-step-count "$EXPECTED_STEP_COUNT" --output json

Expected: local VALIDATION_ERROR, exit 5, no HTTP request. Explicit decimal 0 must remain valid.

Observed against an injected success fixture: exit 0 and:

PUT /api/cli/v1/tests/test_1/plan-steps
if-match-step-count: 0

{"planSteps":[{"type":"action","description":"Open fixture"}]}

Two spaces behave identically. This silently changes a missing value into a real zero-step precondition and sends a mutation request instead of failing locally.

This report establishes CLI request construction only. The success response is fixture data: no hosted TestSprite call, actual mutation, server 412, test execution, credit use, or billing outcome was tested. The help documents 412 on a differing server count, so a conflict is a possible contractual consequence, not an observed backend result.

Details / implementation notes

Root cause and focused scope

• parseNumericFlag calls Number(raw); both empty and whitespace-only strings convert to zero.
• CLI wiring supplies that converted value to runPlanPut.
• Typed validation correctly accepts nonnegative integers, but can no longer distinguish blank from explicit zero.
• Header construction serializes the converted zero.

Suggested fix: reject raw blank/whitespace for this flag before conversion, preserving valid zero and existing range rules. Decimal-only grammar for hex/exponent/plus spellings is a separate compatibility decision, not required for this bug. Do not change fractional --request-timeout support, strict local-port whitespace behavior, or documented TTL server clamping.

Existing tests in src/commands/test.test.ts:7907–7953 exercise numeric 4 and -1 directly through runPlanPut; they miss the blank raw-argv conversion. Add subprocess/parser regression cases that assert exit 5 and zero requests for blanks, and preserve explicit zero and positive-count controls.

Executed controls

Baseline checks also passed on the pinned source: build; 4,798 unit tests passed / 2 skipped; 110 e2e tests passed / 1 skipped; typecheck; lint (0 errors, 215 warnings); format check. This does not imply the blank-input behavior is covered.

Self-contained offline reproduction

After building the pinned checkout, save the script below outside the checkout as repro-blank-count.mjs, then run:

node /path/to/repro-blank-count.mjs /absolute/path/to/testsprite-cli

It runs the actual compiled CLI entry point with disposable HOME/USERPROFILE/XDG directories and dummy credentials. Fetch is fully replaced, with unexpected hosts/routes rejected before network I/O. No hosted service or DNS request is made. It prints exact exits, output, headers and bodies for all six cases. Its assertions describe the currently observed upstream behavior, so exit 0 means the reproduction and controls matched, not that the desired fix is present. This six-case script was executed successfully in the cloud before filing.

import assert from 'node:assert/strict';
import { spawnSync } from 'node:child_process';
import { mkdtempSync, writeFileSync, readFileSync, rmSync, mkdirSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join, resolve } from 'node:path';
const root = resolve(process.argv[2] ?? '.');
const temp = mkdtempSync(join(tmpdir(), 'testsprite-blank-count-'));
try {
  const fixture = join(temp, 'fixture.mjs');
  writeFileSync(join(temp, 'steps.json'), JSON.stringify([{type:'action', description:'Open fixture'}]));
  writeFileSync(fixture, `
import {writeFileSync} from 'node:fs';
const seen=[];
process.on('exit',()=>writeFileSync(process.env.REPRO_TRACE,JSON.stringify(seen)));
globalThis.fetch=async(input,init={})=>{
  const url=new URL(typeof input==='string'?input:input.url??input.href);
  if(url.hostname!=='fixture.invalid')throw Error('Unexpected destination');
  const row={method:init.method??'GET',path:url.pathname,headers:Object.fromEntries(new Headers(init.headers)),body:JSON.parse(String(init.body))};
  seen.push(row);
  if(row.method!=='PUT'||row.path!=='/api/cli/v1/tests/test_1/plan-steps')throw Error('Unexpected route');
  return new Response(JSON.stringify({testId:'test_1',planStepsHash:'fixture',stepCount:1,updatedAt:'2026-10-22T00:00:00Z'}),{status:200,headers:{'content-type':'application/json'}});
};
`);
  const results=[];
  for(const [i,raw] of ['', '  ', '0', '2', '-1', '1.5'].entries()) {
    const home=join(temp,'home-'+i); mkdirSync(home);
    const trace=join(temp,'trace-'+i+'.json');
    const args=['--import',fixture,join(root,'dist/index.js'),'test','plan','put','test_1','--steps',join(temp,'steps.json'),'--expected-step-count',raw,'--output','json'];
    const run=spawnSync(process.execPath,args,{cwd:temp,encoding:'utf8',timeout:15000,
      env:{PATH:process.env.PATH,HOME:home,USERPROFILE:home,XDG_CONFIG_HOME:home,XDG_CACHE_HOME:home,
        CI:'1',DO_NOT_TRACK:'1',TESTSPRITE_NO_TELEMETRY:'1',TESTSPRITE_NO_UPDATE_NOTIFIER:'1',TESTSPRITE_NO_SKILL_WARNING:'1',
        TESTSPRITE_API_KEY:'sk-user-cloud-dummy',TESTSPRITE_API_URL:'https://fixture.invalid',REPRO_TRACE:trace}});
    if(run.error)throw run.error;
    const http=JSON.parse(readFileSync(trace,'utf8'));
    const invalid=raw==='-1'||raw==='1.5';
    assert.equal(run.status,invalid?5:0);
    assert.equal(http.length,invalid?0:1);
    if(!invalid)assert.equal(http[0].headers['if-match-step-count'],String(Number(raw)));
    results.push({raw,exit:run.status,http,stdout:run.stdout,stderr:run.stderr});
  }
  console.log(JSON.stringify(results,null,2));
} finally { rmSync(temp,{recursive:true,force:true}); }

Related reports / duplicate review

Reviewed a 637-record open/closed issue-and-PR snapshot, related discussion and relevant PR reviews. Refreshed issue and PR searches for expected-step-count immediately before filing found no matching report.

• [Hackathon] fix(test): test steps --run-id accepts invalid --page-size / --max-items values and skips the bare path's validation #603: run-scoped pagination skips validation, a different call path.
• fix(history): reject fractional page sizes #55: fractional page sizes.
• fix(cli): validate --request-timeout flag and de-duplicate its parser #17: request-timeout parsing, where fractional seconds remain intentional.
• [Hackathon] fix(cli): warn when --ttl is outside the effective 60–28800 range instead of silently relying on server clamping #408 / PR fix(cli): warn on out-of-range --ttl for tunnel start #413: TTL advisory/clamping. [Hackathon] fix(cli): warn when --ttl is outside the effective 60–28800 range instead of silently relying on server clamping #408 was closed not-planned; this proposal does not revisit that policy.

No matching report was found in the checked public records; this is not a claim about private/internal work. Implementation is pending maintainer triage under CONTRIBUTING.md. No production patch or PR accompanies this report.

AI disclosure: Codex assisted with source review, cloud-only execution, duplicate checking and this write-up.

Confirmations

• I have searched existing issues and this is not a duplicate.
• I have provided my Discord identity above for reward coordination.