diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index 0a215695..6a729a8b 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -422,6 +422,8 @@ testsprite test create --plan-from ./checkout.plan.json --dry-run --output json Bulk-create frontend tests from a JSONL plan-steps file (or a directory of plan files with `--plan-from-dir`). Optional `--run --max-concurrency ` fans out triggers. `--max-concurrency`, `--wait` and `--timeout` act on those runs, so without `--run` they are refused (exit 5), like `--env`. Without `--wait`, each run is dispatched (`status: "queued"`) and the command exits 0 when every trigger is accepted — mirroring single `test run` without `--wait`; a trigger error still exits non-zero. With `--wait`, it polls every run to terminal and exits non-zero if any run does not pass. +With `--run`, each trigger's idempotency key is derived from the batch key (`:run:`), so retrying the command with the same `--idempotency-key` — or the auto-minted one printed to stderr under `--output json`, `--verbose` or `--debug` — returns the runs the first attempt already started instead of starting new, billed ones. An all-public several-id `test run` is one batch request that uses the key as is; a tunnel-pool run (several ids or `--all` through `--local` or a tunneled environment) triggers each test separately with `:`. A trigger routed through the tunnel also carries the tunnel's client id before the test id (`:run::` for `create-batch`, `::` for `test run`): a retry through the same tunnel (adopted with `--tunnel-client`) gets its runs back, while a retry through a new tunnel starts new runs, since the old ones cannot reach the app without their tunnel. A key too long to fit its suffix within 256 characters is shortened, with a digest of the full key kept so it stays unique. + ```bash testsprite test create-batch --plans ./plans.jsonl --run --max-concurrency 4 --output json testsprite test create-batch --plan-from-dir ./plans/ --dry-run --output json diff --git a/src/commands/test.create-batch-run.spec.ts b/src/commands/test.create-batch-run.spec.ts index 5f8232e9..5ef542bf 100644 --- a/src/commands/test.create-batch-run.spec.ts +++ b/src/commands/test.create-batch-run.spec.ts @@ -846,10 +846,10 @@ describe('runCreateBatch --run --wait: --timeout per-run', () => { }); // --------------------------------------------------------------------------- -// Idempotency: each per-run key is unique (regression guard) +// Idempotency: per-run keys are distinct and stable across retries // --------------------------------------------------------------------------- -describe('runCreateBatch --run: idempotency key uniqueness', () => { +describe('runCreateBatch --run: run idempotency keys', () => { let logSpy: ReturnType; let errorSpy: ReturnType; @@ -863,14 +863,17 @@ describe('runCreateBatch --run: idempotency key uniqueness', () => { errorSpy.mockRestore(); }); - it('each run trigger gets a distinct idempotency key, none matches the create key', async () => { + /** + * Run `create-batch --run --wait` for three FE specs against a fake server + * and record the idempotency key on the create POST and on each trigger. + */ + async function runCapturingKeys(idempotencyKey?: string) { const { credentialsPath } = makeCreds(); const testIds = ['test_f1', 'test_f2', 'test_f3']; const plansFile = writePlansJsonl([FE_SPEC, FE_SPEC, FE_SPEC]); - const createIdemKey = 'cli-create-batch-test-key'; - - const seenRunIdempotencyKeys: string[] = []; - let createIdempotencyKeySeen = ''; + const runKeys: Record = {}; + const stderr: string[] = []; + let createKey = ''; const fetchImpl = (async (input: FetchInput, init: RequestInit = {}) => { const url = @@ -882,7 +885,7 @@ describe('runCreateBatch --run: idempotency key uniqueness', () => { const headers = new Headers(init.headers as Record); if (url.includes('/tests/batch')) { - createIdempotencyKeySeen = headers.get('idempotency-key') ?? ''; + createKey = headers.get('idempotency-key') ?? ''; return new Response(JSON.stringify(makeBatchCreateResponse(testIds)), { status: 200, headers: { 'content-type': 'application/json' }, @@ -892,10 +895,8 @@ describe('runCreateBatch --run: idempotency key uniqueness', () => { const triggerMatch = /\/tests\/(test_[a-z0-9]+)\/runs$/.exec(url); if (triggerMatch?.[1]) { const testId = triggerMatch[1]; - const key = headers.get('idempotency-key') ?? ''; - seenRunIdempotencyKeys.push(key); - const runId = `run_${testId}`; - return new Response(JSON.stringify(makeTriggerResponse(testId, runId)), { + runKeys[testId] = headers.get('idempotency-key') ?? ''; + return new Response(JSON.stringify(makeTriggerResponse(testId, `run_${testId}`)), { status: 200, headers: { 'content-type': 'application/json' }, }); @@ -904,8 +905,7 @@ describe('runCreateBatch --run: idempotency key uniqueness', () => { const pollMatch = /\/runs\/(run_test_[a-z0-9]+)/.exec(url); if (pollMatch?.[1]) { const runId = pollMatch[1]; - const testId = runId.replace('run_', ''); - return new Response(JSON.stringify(makePassedRun(testId, runId)), { + return new Response(JSON.stringify(makePassedRun(runId.replace('run_', ''), runId)), { status: 200, headers: { 'content-type': 'application/json' }, }); @@ -928,31 +928,59 @@ describe('runCreateBatch --run: idempotency key uniqueness', () => { plans: plansFile, run: true, wait: true, - idempotencyKey: createIdemKey, + ...(idempotencyKey !== undefined ? { idempotencyKey } : {}), timeoutSeconds: 60, }, { credentialsPath, fetchImpl, stdout: () => {}, - stderr: () => {}, + stderr: line => stderr.push(line), sleep: instantSleep, }, ); + return { createKey, runKeys, stderr }; + } + + it('derives a distinct run key per test from the create key, never the create key itself', async () => { + const { createKey, runKeys } = await runCapturingKeys('cli-create-batch-test-key'); + + expect(createKey).toBe('cli-create-batch-test-key'); + expect(runKeys).toEqual({ + test_f1: 'cli-create-batch-test-key:run:test_f1', + test_f2: 'cli-create-batch-test-key:run:test_f2', + test_f3: 'cli-create-batch-test-key:run:test_f3', + }); + }); + + it('re-sends the same run keys when retried with the same --idempotency-key', async () => { + const first = await runCapturingKeys('ci-build-42'); + const retry = await runCapturingKeys('ci-build-42'); + + // Identical keys let the server return the runs the first attempt started + // instead of starting (and billing) them a second time. + expect(retry.runKeys).toEqual(first.runKeys); + }); + + it('re-sends the same run keys when retried with the printed auto-minted key', async () => { + const first = await runCapturingKeys(); + const printed = first.stderr + .find(line => line.startsWith('idempotency-key: ')) + ?.slice('idempotency-key: '.length); + expect(printed).toBe(first.createKey); + + const retry = await runCapturingKeys(printed); + expect(retry.runKeys).toEqual(first.runKeys); + }); + + it('keeps a derived run key within 256 characters when the create key is at the limit', async () => { + const { runKeys } = await runCapturingKeys('k'.repeat(256)); - // 3 run triggers → 3 run idempotency keys - expect(seenRunIdempotencyKeys).toHaveLength(3); - // All run keys must be unique - const uniqueKeys = new Set(seenRunIdempotencyKeys); - expect(uniqueKeys.size).toBe(3); - // None of the run keys matches the create key (regression guard) - expect(seenRunIdempotencyKeys).not.toContain(createIdemKey); - // The create key was used on the batch POST - expect(createIdempotencyKeySeen).toBe(createIdemKey); - // All run keys should start with 'cli-batch-run-' - for (const key of seenRunIdempotencyKeys) { - expect(key).toMatch(/^cli-batch-run-[0-9a-f-]{36}$/); + for (const [testId, key] of Object.entries(runKeys)) { + expect(key.length).toBeLessThanOrEqual(256); + expect(key.endsWith(`:run:${testId}`)).toBe(true); } + expect(new Set(Object.values(runKeys)).size).toBe(3); }); }); diff --git a/src/commands/test.environment-tunnel.spec.ts b/src/commands/test.environment-tunnel.spec.ts index 7574733c..ca9d6c37 100644 --- a/src/commands/test.environment-tunnel.spec.ts +++ b/src/commands/test.environment-tunnel.spec.ts @@ -41,6 +41,8 @@ function fixture( envStatus?: number; echo?: boolean; mintStatus?: number; + /** Client id the tunnel mint returns (a different one simulates a new tunnel). */ + tunnelClientId?: string; triggerStatus?: number; batchResponse?: (body: Record) => Record; envs?: Array>; @@ -138,7 +140,7 @@ function fixture( ? error(options.mintStatus, 'missing_scope') : json( { - clientId, + clientId: options.tunnelClientId ?? clientId, secret: 'fixture-secret', controlUrl: 'ws://tunnel.example/ws', tunnelAddr: 'tunnel.example:7400', @@ -146,7 +148,7 @@ function fixture( }, 201, ); - if (method === 'DELETE' && path === `/tunnel/${clientId}`) + if (method === 'DELETE' && path === `/tunnel/${options.tunnelClientId ?? clientId}`) return new Response(null, { status: 204 }); if (method === 'POST' && path.endsWith('/cancel')) return json({ @@ -1372,6 +1374,112 @@ describe('environment tunnel resolution', () => { expect(keys.every(key => key.length <= 256)).toBe(true); }); + /** Record the idempotency key each `POST /tests/{id}/runs` carries, by test id. */ + function captureTriggerKeys(f: ReturnType): Record { + const keys: Record = {}; + const original = f.deps.fetchImpl; + f.deps.fetchImpl = vi.fn(async (input: Parameters[0], init: RequestInit = {}) => { + const trigger = /\/tests\/([^/]+)\/runs$/.exec(String(input)); + if (trigger?.[1] && init.method === 'POST') + keys[trigger[1]] = new Headers(init.headers).get('idempotency-key')!; + return original(input, init); + }); + return keys; + } + + const threeTests = [ + { id: 'a', type: 'frontend', projectId: 'P' }, + { id: 'b', type: 'frontend', projectId: 'P' }, + { id: 'c', type: 'frontend', projectId: 'P' }, + ]; + + it('derives every trigger key of a several-id run from --idempotency-key, the tunnel and the test id', async () => { + const f = fixture({ tests: threeTests }); + const keys = captureTriggerKeys(f); + await f.run('run', 'a', 'b', 'c', '--skip-preflight', '--idempotency-key', 'ci-build-42'); + expect(keys).toEqual({ + a: `ci-build-42:${clientId}:a`, + b: `ci-build-42:${clientId}:b`, + c: `ci-build-42:${clientId}:c`, + }); + }); + + it('re-sends the same trigger keys when a several-id run is retried with the printed key', async () => { + const first = fixture({ tests: threeTests }); + const firstKeys = captureTriggerKeys(first); + await first.run('run', 'a', 'b', 'c', '--skip-preflight'); + const printed = first.stderr + .find(line => line.startsWith('idempotency-key: ')) + ?.slice('idempotency-key: '.length); + expect(printed).toMatch(/^cli-run-/); + + const retry = fixture({ tests: threeTests }); + const retryKeys = captureTriggerKeys(retry); + await retry.run('run', 'a', 'b', 'c', '--skip-preflight', '--idempotency-key', printed!); + + // Identical keys let the server return the runs the first attempt started + // instead of starting (and billing) every test a second time. + expect(Object.keys(firstKeys)).toHaveLength(3); + expect(retryKeys).toEqual(firstKeys); + expect(new Set(Object.values(firstKeys)).size).toBe(3); + }); + + it('uses new trigger keys when a several-id run is retried through a new tunnel', async () => { + const first = fixture({ tests: threeTests }); + const firstKeys = captureTriggerKeys(first); + await first.run('run', 'a', 'b', 'c', '--skip-preflight', '--idempotency-key', 'ci-build-42'); + + const retry = fixture({ tests: threeTests, tunnelClientId: 'new-tunnel-client' }); + const retryKeys = captureTriggerKeys(retry); + await retry.run('run', 'a', 'b', 'c', '--skip-preflight', '--idempotency-key', 'ci-build-42'); + + // The old runs are bound to a tunnel that is gone, and reusing their keys + // with a new tunnelClientId would be refused as IDEMPOTENCY_BODY_MISMATCH. + expect(retryKeys).toEqual({ + a: 'ci-build-42:new-tunnel-client:a', + b: 'ci-build-42:new-tunnel-client:b', + c: 'ci-build-42:new-tunnel-client:c', + }); + expect(Object.values(retryKeys).some(key => Object.values(firstKeys).includes(key))).toBe( + false, + ); + }); + + it('scopes create-batch --run trigger keys to the tunnel', async () => { + const f = fixture(); + const keys = captureTriggerKeys(f); + const plans = join(mkdtempSync(join(tmpdir(), 'env-plans-')), 'plans.jsonl'); + // eslint-disable-next-line security/detect-non-literal-fs-filename -- fixture-owned temporary directory. + writeFileSync( + plans, + ['a', 'b'] + .map(name => + JSON.stringify({ + projectId: 'P', + type: 'frontend', + name, + planSteps: [{ type: 'action', description: 'Open the app' }], + }), + ) + .join('\n'), + ); + await runCreateBatch( + { + ...common, + plans, + run: true, + wait: false, + idempotencyKey: 'ci-build-42', + skipPreflight: true, + }, + f.deps, + ); + expect(keys).toEqual({ + a: `ci-build-42:run:${clientId}:a`, + b: `ci-build-42:run:${clientId}:b`, + }); + }); + it('drains concurrent create-batch triggers before printing receipts and closing the tunnel', async () => { const f = fixture({ pollStatus: 404 }); const original = f.deps.fetchImpl; diff --git a/src/commands/test.local-batch.spec.ts b/src/commands/test.local-batch.spec.ts index a95e4a60..a5a5d327 100644 --- a/src/commands/test.local-batch.spec.ts +++ b/src/commands/test.local-batch.spec.ts @@ -1505,16 +1505,17 @@ describe('test run local batch', () => { expect(f.stderr.filter(line => line.startsWith('idempotency-key:'))).toEqual([]); }); - it('prints the first automatic trigger key once', async () => { + it('prints the automatic key once and derives every trigger key from it', async () => { const f = fixture(['a', 'b']); await f.run('a', 'b', '--local', '5173', '--skip-preflight'); const keys = f.calls .filter(c => c.method === 'POST' && /\/tests\/[^/]+\/runs/.test(c.url)) .map(c => c.idempotencyKey); - expect(keys[0]).not.toBe(keys[1]); - expect(f.stderr.filter(line => line.startsWith('idempotency-key:'))).toEqual([ - `idempotency-key: ${keys[0]}`, - ]); + const printed = f.stderr.filter(line => line.startsWith('idempotency-key:')); + expect(printed).toHaveLength(1); + const base = printed[0]!.slice('idempotency-key: '.length); + // Passing the printed key back with --idempotency-key reproduces these keys. + expect(keys).toEqual([`${base}:${f.clientId}:a`, `${base}:${f.clientId}:b`]); }); it('prints the text table without inventing a project dashboard link', async () => { diff --git a/src/commands/test.ts b/src/commands/test.ts index 359a5bd1..ffab7a82 100644 --- a/src/commands/test.ts +++ b/src/commands/test.ts @@ -4413,7 +4413,7 @@ export async function runCreateBatch( if (opts.run === true) { await withCreatedReceipt(opts, deps, response, runDeps => runBatchRun( - opts, + { ...opts, idempotencyKey }, response, runDeps.environmentTunnel?.client ?? client, makeOutput(opts.output, runDeps), @@ -4451,7 +4451,7 @@ export async function runCreateBatch( * that code only when ALL runs share the same code; otherwise 1. */ async function runBatchRun( - opts: CreateBatchOptions, + opts: CreateBatchOptions & { idempotencyKey: string }, createResponse: CliCreateBatchResponse, client: HttpClient, out: Output, @@ -4639,8 +4639,21 @@ async function runBatchRun( const pendingTriggers = new Set>(); async function triggerOne(testId: string): Promise { - // Mint a fresh idempotency key per run — MUST NOT reuse the create key. - const runIdempotencyKey = `cli-batch-run-${randomUUID()}`; + // Derived from the create key, never the create key itself (that would + // replay the create response), so a retry with the same key gets back the + // runs the first attempt started. A trigger routed through a tunnel names + // the tunnel's client id in its body, and the server refuses a reused key + // whose body changed (IDEMPOTENCY_BODY_MISMATCH), so that client id goes in + // the key too: a retry through the same tunnel (adopted with + // --tunnel-client) gets its runs back, and a retry through a new tunnel + // starts new runs, since the old ones cannot reach the app without theirs. + const tunnelScope = deps.environmentTunnel?.targets?.has(testId) + ? `:${deps.environmentTunnel.clientId}` + : ''; + const runIdempotencyKey = suffixedIdempotencyKey( + opts.idempotencyKey, + `:run${tunnelScope}:${testId}`, + ); if (opts.debug) { stderrFn(`[batch-run] ${testId} idempotency-key: ${runIdempotencyKey}`); } @@ -12436,12 +12449,11 @@ export async function runTestRunLocalBatch( const runOne = async (index: number) => { const row = results[index]!; let triggerStartedAtMs: number; - const suffix = `:${row.testId}`; - const key = opts.idempotencyKey - ? `${idempotencyBase.slice(0, Math.max(0, 256 - suffix.length))}${suffix}`.slice(-256) - : index === 0 - ? idempotencyBase - : `cli-run-${randomUUID()}`; + // Derived for every test, auto-minted base included, so a retry with + // the printed key re-sends the same keys; scoped to the tunnel when + // routed through one (see triggerOne in runBatchRun for why). + const tunnelScope = targets.has(row.testId) ? `:${liveSession.clientId}` : ''; + const key = suffixedIdempotencyKey(idempotencyBase, `${tunnelScope}:${row.testId}`); let response: TriggerRunResponse; try { if (shutdown.signal.aborted) throw shutdown.signal.reason; @@ -15050,7 +15062,7 @@ export function createTestCommand(deps: TestDeps = {}): Command { .option('--skip-preflight', `with --run: ${SKIP_PREFLIGHT_HELP}`, false) .option( '--idempotency-key ', - 'idempotency token for the batch (1-256 ASCII chars; default: random UUID, pin one for safe retries)', + 'idempotency token for the batch (1-256 ASCII chars; with --run, each trigger key is derived from it; default: random UUID, pin one for safe retries)', ) .addHelpText( 'after', diff --git a/test/__snapshots__/help.snapshot.test.ts.snap b/test/__snapshots__/help.snapshot.test.ts.snap index eefe4789..5900bbfa 100644 --- a/test/__snapshots__/help.snapshot.test.ts.snap +++ b/test/__snapshots__/help.snapshot.test.ts.snap @@ -812,7 +812,8 @@ Options: the cloud, so it can be wrong, e.g. behind an IP allowlist) (default: false) --idempotency-key idempotency token for the batch (1-256 ASCII - chars; default: random UUID, pin one for safe + chars; with --run, each trigger key is derived + from it; default: random UUID, pin one for safe retries) -h, --help display help for command