From e91c5a107867f16ca183b11684c84d953d972cbf Mon Sep 17 00:00:00 2001 From: zeshi-du Date: Thu, 16 Jul 2026 14:56:49 -0700 Subject: [PATCH] release: v0.4.0 Private-Snapshot-RevId: afa3bf501545df536cfa638a86ae096fb7c34880 --- .github/workflows/backport-dispatch.yml | 83 ++ .github/workflows/ci-nudge.yml | 30 +- .github/workflows/ci.yml | 64 ++ .github/workflows/issue-triage.yml | 12 +- .github/workflows/pr-triage.yml | 24 +- .github/workflows/release.yaml | 28 +- .github/workflows/test-coverage.yml | 2 + .gitleaks.toml | 19 + CHANGELOG.md | 21 + CONTRIBUTING.md | 27 +- DOCUMENTATION.md | 74 +- README.md | 59 +- package-lock.json | 4 +- package.json | 2 +- scripts/README.md | 29 + skills/testsprite-verify.codex.md | 6 + skills/testsprite-verify.skill.md | 19 +- src/commands/agent.test.ts | 156 ++-- src/commands/auth.test.ts | 57 ++ src/commands/auth.ts | 14 + src/commands/doctor.test.ts | 45 + src/commands/doctor.ts | 60 +- src/commands/project.test.ts | 168 +++- src/commands/project.ts | 130 ++- src/commands/test.cancel.spec.ts | 309 +++++++ src/commands/test.quickwins.spec.ts | 110 +++ src/commands/test.rerun.spec.ts | 98 ++- src/commands/test.run.spec.ts | 162 +++- src/commands/test.test.ts | 103 +++ src/commands/test.ts | 815 ++++++++++++++++-- src/commands/test.wait.spec.ts | 122 ++- src/index.ts | 45 +- src/lib/bundle.test.ts | 19 + src/lib/bundle.ts | 19 +- src/lib/client-factory.ts | 23 + src/lib/dry-run/samples.test.ts | 33 + src/lib/dry-run/samples.ts | 43 +- src/lib/errors.test.ts | 22 + src/lib/errors.ts | 45 + src/lib/failing-fe-resolver.spec.ts | 193 +++++ src/lib/failing-fe-resolver.ts | 67 +- src/lib/http.test.ts | 246 +++++- src/lib/http.ts | 133 ++- src/lib/interrupt.test.ts | 101 ++- src/lib/interrupt.ts | 124 ++- src/lib/poll.spec.ts | 104 ++- src/lib/poll.ts | 95 +- src/lib/runs.types.ts | 14 + src/lib/v3-advisory.test.ts | 24 + src/lib/v3-advisory.ts | 25 + src/lib/version-notice.test.ts | 110 +++ src/lib/version-notice.ts | 111 +++ src/version.ts | 2 +- test/__snapshots__/help.snapshot.test.ts.snap | 72 +- test/cli.subprocess.test.ts | 5 +- test/e2e/signal.e2e.test.ts | 188 ++++ test/help.snapshot.test.ts | 5 +- test/helpers/execNpm.ts | 17 + 58 files changed, 4410 insertions(+), 327 deletions(-) create mode 100644 .github/workflows/backport-dispatch.yml create mode 100644 scripts/README.md create mode 100644 src/commands/test.cancel.spec.ts create mode 100644 src/lib/v3-advisory.test.ts create mode 100644 src/lib/v3-advisory.ts create mode 100644 src/lib/version-notice.test.ts create mode 100644 src/lib/version-notice.ts create mode 100644 test/e2e/signal.e2e.test.ts create mode 100644 test/helpers/execNpm.ts diff --git a/.github/workflows/backport-dispatch.yml b/.github/workflows/backport-dispatch.yml new file mode 100644 index 0000000..8b6a99a --- /dev/null +++ b/.github/workflows/backport-dispatch.yml @@ -0,0 +1,83 @@ +# backport-dispatch.yml — OPTIONAL latency optimization for DEV-352's +# auto-backport bot. Authored in atlas (release/public path), ships to the +# PUBLIC repo via the snapshot per invariant I0 — this workflow only ever +# runs on the public repo, never on atlas (see the repo guard below). +# +# NOT YET ARMED (2026-07-15): the atlas-side auto-backport.yml has a +# scheduled SWEEP (every 30 min) that scans merged public PRs anonymously +# and backports anything unabsorbed — the whole pipeline is fully +# functional with ZERO public-repo credentials via that path alone. This +# workflow instantly notifies atlas the moment a PR merges instead of +# waiting for the next sweep (seconds vs. up to 30 minutes) — a nice-to-have, +# not a requirement. +# +# Arming it means placing a GitHub App credential with contents:write on +# the PRIVATE atlas repo into THIS PUBLIC repo's secrets — a real trust +# decision (a compromised public-repo secret store would let an attacker +# push to atlas) that is deliberately left to the operator, not decided by +# this file. Until TESTSPRITE_HOB_APP_ID/TESTSPRITE_HOB_PRIVATE_KEY (or the +# ALFHEIM_AGENT_* fallback) exist on the PUBLIC repo with contents:write on +# atlas, this workflow no-ops cleanly (see the "not armed" step) — it does +# NOT fail loud, so it stays quiet for every contributor watching the +# Actions tab until the operator decides to enable it. +# +# SECURITY: this workflow reads ONLY event metadata (PR number, merge SHA, +# base ref) — it never checks out the merged PR's code. That is what makes +# `pull_request_target` safe here: the base-repo context (secrets, a +# write-capable token) is required because a fork-originated merged PR gets +# ZERO secrets under a plain `pull_request` trigger, even for the `closed` +# activity type — but nothing in this job ever runs untrusted fork code, so +# the classic pull_request_target RCE/secret-exfiltration risk (checking +# out and executing the fork's own head ref under a privileged context) +# does not apply. Do not add actions/checkout to this workflow. +name: Notify atlas of a merged public PR + +on: + pull_request_target: + types: [closed] + +permissions: + contents: read + +jobs: + notify: + if: github.repository == 'TestSprite/testsprite-cli' && github.event.pull_request.merged == true + runs-on: ubuntu-latest + steps: + - id: app-token + continue-on-error: true + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + with: + app-id: ${{ secrets.TESTSPRITE_HOB_APP_ID || secrets.ALFHEIM_AGENT_APP_ID }} + private-key: ${{ secrets.TESTSPRITE_HOB_PRIVATE_KEY || secrets.ALFHEIM_AGENT_PRIVATE_KEY }} + owner: TestSprite + repositories: testsprite-cli-atlas + # Minimum required for POST /repos/{owner}/{repo}/dispatches (verified + # against docs.github.com/en/rest/using-the-rest-api/permissions-required-for-github-apps — + # the endpoint is gated on Contents:write, not Contents:read). + permission-contents: write + + - name: Dispatch to atlas + if: steps.app-token.outputs.token != '' + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + # These two fields are fork-influenced (merge_commit_sha/base.ref come + # from a merged PR's event payload) — routed through env instead of + # spliced into the run: string to avoid the GHA script-injection class + # (a maliciously-crafted value could otherwise break out of the -F + # argument and inject arbitrary shell). PR_NUMBER is left inline: it is + # a GitHub-assigned integer, not attacker-authorable content. + MERGE_SHA: ${{ github.event.pull_request.merge_commit_sha }} + BASE_REF: ${{ github.event.pull_request.base.ref }} + run: | + set -euo pipefail + gh api repos/TestSprite/testsprite-cli-atlas/dispatches \ + -f event_type=public-pr-merged \ + -F 'client_payload[pr_number]=${{ github.event.pull_request.number }}' \ + -F "client_payload[merge_commit_sha]=$MERGE_SHA" \ + -F "client_payload[base_ref]=$BASE_REF" + + - name: Not armed yet — the atlas sweep will pick this up + if: steps.app-token.outputs.token == '' + run: | + echo "::notice::No cross-repo credential configured on this repo yet — atlas's scheduled sweep (auto-backport.yml, every 30 min) will pick up this merge instead of an instant dispatch. See DEV-352 / DEV-348 / DEV-350 for the operator decision to arm this." diff --git a/.github/workflows/ci-nudge.yml b/.github/workflows/ci-nudge.yml index d01ede3..f14461f 100644 --- a/.github/workflows/ci-nudge.yml +++ b/.github/workflows/ci-nudge.yml @@ -9,8 +9,10 @@ # CI workflows (CI + Test Coverage) can complete in any order. # # Bot identity: same App-token-first / GITHUB_TOKEN-fallback pattern as -# pr-triage.yml (the App needs the "Pull requests: Read & write" permission to -# post as testsprite-hob[bot]; until then comments come from github-actions[bot]). +# pr-triage.yml. The App has had the "Pull requests: Read & write" permission +# since 2026-07-02 (corrected 2026-07-15; this comment previously said the +# permission was still pending) — the fallback path stays as defense-in-depth +# for whenever the App/secrets are absent. name: CI failure nudge on: @@ -40,8 +42,20 @@ jobs: with: app-id: ${{ secrets.TESTSPRITE_HOB_APP_ID || secrets.ALFHEIM_AGENT_APP_ID }} private-key: ${{ secrets.TESTSPRITE_HOB_PRIVATE_KEY || secrets.ALFHEIM_AGENT_PRIVATE_KEY }} + # The script below only ever calls the App client (`appClient`) for + # rest.issues.updateComment/createComment — PR comments ride the + # issues API (see the header comment above). All reads (checks, + # pulls, commit->PR lookup) go through the ambient `github` client, + # governed by this workflow's top-level `permissions:` block, not + # this minted token. Scoping to checks:read/pull-requests:write here + # (matching the job's top-level block literally) would risk the mint + # itself failing if the App's installation doesn't hold Checks + # permission — which would silently drop the App-token path + # entirely (continue-on-error swallows the failure) and always fall + # back to github-actions[bot], defeating this bot's own purpose. + permission-issues: write - - uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.0.1 + - uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 env: APP_TOKEN: ${{ steps.app-token.outputs.token }} with: @@ -82,14 +96,20 @@ jobs: 'Build': 'run `npm run build` and fix the compile errors', 'Local E2E Tests': 'run `npm run test:e2e` (it builds first)', 'Coverage (>= 80%)': 'run `npm run test:coverage` — new code needs tests until every metric is back at 80%', + 'Secret scan (gitleaks)': 'run `gitleaks detect --no-git --redact --source .` locally (config: .gitleaks.toml) and remove the secret — or, for a provable false positive, add a narrowly-anchored allowlist regex', }; + // Matrix jobs publish check runs as "Unit Tests (Node 20)" etc. — + // exact lookup alone would silently skip them (and the nudge would + // say "all green" while they are red). Try exact first, then the + // name with a trailing parenthetical stripped (review round 2). + const fixFor = (name) => FIX[name] ?? FIX[name.replace(/\s*\([^)]*\)$/, '')]; // Recompute full CI state from this SHA's check runs, narrowed to the // CI jobs above — other workflows (e.g. pr-triage) also attach // github-actions check runs to the PR head and must not count here. const checks = await github.paginate(github.rest.checks.listForRef, { owner, repo, ref: run.head_sha, per_page: 100 }); - const ours = checks.filter(c => c.app && c.app.slug === 'github-actions' && FIX[c.name]); + const ours = checks.filter(c => c.app && c.app.slug === 'github-actions' && fixFor(c.name)); const failing = ours.filter(c => ['failure', 'timed_out'].includes(c.conclusion)); const pending = ours.filter(c => c.status !== 'completed'); @@ -110,7 +130,7 @@ jobs: const lines = failing .sort((a, b) => a.name.localeCompare(b.name)) .map(c => { - const fix = FIX[c.name] || 'see the logs for details'; + const fix = fixFor(c.name) || 'see the logs for details'; return `- **${c.name}** — ${fix} ([logs](${c.html_url}))`; }); const body = `${marker}\nThanks, @${pr.user.login}! CI is red on this PR — ` diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index edde076..3766b0f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,6 +17,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + with: + persist-credentials: false - name: Setup Node.js uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 @@ -37,6 +39,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + with: + persist-credentials: false - name: Setup Node.js uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 @@ -55,6 +59,8 @@ jobs: node-version: [20, 22] steps: - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + with: + persist-credentials: false - name: Setup Node.js uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 @@ -68,6 +74,27 @@ jobs: env: CI: true + test-windows: + name: Unit Tests (Windows) + runs-on: windows-latest + steps: + - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + with: + persist-credentials: false + + - name: Setup Node.js + uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + with: + node-version: 22 + cache: 'npm' + + - run: npm ci + - run: npm run build + - run: npm run typecheck + - run: npm test + env: + CI: true + build: name: Build (Node ${{ matrix.node-version }}) runs-on: ubuntu-latest @@ -76,6 +103,8 @@ jobs: node-version: [20, 22] steps: - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + with: + persist-credentials: false - name: Setup Node.js uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 @@ -96,6 +125,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + with: + persist-credentials: false - name: Setup Node.js uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 @@ -107,3 +138,36 @@ jobs: - run: npm run test:e2e env: CI: true + + gitleaks: + name: Secret scan (gitleaks) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + with: + persist-credentials: false + + # Same pinned version + checksum-verified install as + # divergence-sentinel.yml (the more recent/secure of the two gitleaks + # invocations already in this repo — release-build.yml pins an older + # 8.21.2 with no checksum check). Continuous, per-PR/per-push gate: a + # working-tree scan (--no-git), not a full-history scan — history + # scanning is too slow to run on every PR, and this mirrors the mode + # scripts/make-public-snapshot.sh already uses for the release-time scan + # (`gitleaks detect --no-git --no-banner --redact --source `). + - name: Install gitleaks + env: + GITLEAKS_VERSION: '8.28.0' + run: | + set -euo pipefail + BASE="https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}" + TARBALL="gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" + curl -sSL -o "$TARBALL" "${BASE}/${TARBALL}" + curl -sSL -o checksums.txt "${BASE}/gitleaks_${GITLEAKS_VERSION}_checksums.txt" + grep " ${TARBALL}\$" checksums.txt | sha256sum -c - + tar -xzf "$TARBALL" gitleaks + sudo mv gitleaks /usr/local/bin/gitleaks + gitleaks version + + - name: Scan working tree for secrets + run: gitleaks detect --no-git --no-banner --redact --source . diff --git a/.github/workflows/issue-triage.yml b/.github/workflows/issue-triage.yml index 6b6ce62..0da78c8 100644 --- a/.github/workflows/issue-triage.yml +++ b/.github/workflows/issue-triage.yml @@ -31,10 +31,18 @@ env: jobs: triage: + # Public repo only — this file also lives in the private mirror (atlas), + # where issue-first claiming doesn't apply; the three sibling bots + # (pr-triage.yml, ci-nudge.yml, stale.yml) all carry this same fence and + # this one was missing it (found in the 2026-07-15 OSS-P2 audit — the + # /assign bot was live on atlas too until this fix). # issues only (issue_comment also fires on PRs), and never react to a bot's own # comment — incl. our own testsprite-hob[bot], which (unlike GITHUB_TOKEN) would # otherwise re-trigger this workflow. `type == 'Bot'` covers both bot identities. - if: ${{ !github.event.issue.pull_request && github.event.comment.user.type != 'Bot' }} + if: >- + github.repository == 'TestSprite/testsprite-cli' && + !github.event.issue.pull_request && + github.event.comment.user.type != 'Bot' runs-on: ubuntu-latest steps: # Mint a token for the testsprite-hob App so comments post as testsprite-hob[bot]. @@ -49,7 +57,7 @@ jobs: # of inheriting the App's full installation permissions. permission-issues: write - - uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.0.1 + - uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 with: # App token when available (→ testsprite-hob[bot]); else the default # GITHUB_TOKEN (→ github-actions[bot]). Either way the logic is identical. diff --git a/.github/workflows/pr-triage.yml b/.github/workflows/pr-triage.yml index a9cd23d..cfd114d 100644 --- a/.github/workflows/pr-triage.yml +++ b/.github/workflows/pr-triage.yml @@ -13,12 +13,12 @@ # Runs with NO checkout — metadata-only, so it is safe under pull_request_target # (which is required for fork PRs to get a write-capable token). # -# Bot identity: posts as `testsprite-hob[bot]` when the App token works. The -# App currently has Issues R/W + Metadata only — commenting/labeling a PULL -# REQUEST needs the "Pull requests: Read & write" App permission (issues-API -# endpoints are permission-checked by target type). Until an org admin adds -# that permission and re-approves the installation, every call gracefully falls -# back to the default GITHUB_TOKEN and posts as github-actions[bot]. +# Bot identity: posts as `testsprite-hob[bot]` when the App token works (the +# App has had the "Pull requests: Read & write" permission since 2026-07-02 — +# corrected 2026-07-15; this comment previously said the permission was still +# pending). The App-token-first / GITHUB_TOKEN-fallback code path below is +# kept regardless, as defense-in-depth for whenever the App/secrets are +# absent or a future installation loses the permission. # Secrets: TESTSPRITE_HOB_* preferred; the ALFHEIM_AGENT_* fallbacks are the # original names from before the App was renamed (same App ID + key). name: PR triage (issue-link gate) @@ -60,8 +60,18 @@ jobs: with: app-id: ${{ secrets.TESTSPRITE_HOB_APP_ID || secrets.ALFHEIM_AGENT_APP_ID }} private-key: ${{ secrets.TESTSPRITE_HOB_PRIVATE_KEY || secrets.ALFHEIM_AGENT_PRIVATE_KEY }} + # Every write() call below (createComment/updateComment/addLabels/ + # removeLabel) is an `issues.*` Octokit method — GitHub gates all + # four (comments AND labels, even on a PR number) on the "Issues" + # repository permission, not "Pull requests" (verified against + # docs.github.com/en/rest/using-the-rest-api/permissions-required-for-github-apps). + # Scoping to issues:write alone is therefore both sufficient and + # minimal — narrower than this job's top-level `permissions:` block, + # which also carries pull-requests:write for the ambient-token + # fallback path (unused by this specific App-token mint). + permission-issues: write - - uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.0.1 + - uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 env: APP_TOKEN: ${{ steps.app-token.outputs.token }} with: diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index a0f1346..8ef6dc9 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -14,13 +14,33 @@ jobs: with: node-version: 22 registry-url: 'https://registry.npmjs.org' - # npm trusted publishing (OIDC) requires npm >= 11.5.1; Node 22 bundles npm 10.x - - run: npm install -g npm@latest + # npm trusted publishing (OIDC) requires npm >= 11.5.1; Node 22 never bundles + # that (it ships 10.9.x). Pin an EXACT version rather than floating on + # @latest: npm 12.0.0 shipped with a broken workspace symlink that pulled the + # wrong `sigstore` dependency into its own release tarball, so every + # `npm publish --provenance` failed with `Cannot find module 'sigstore'` + # (npm/cli#9722) — this is exactly what our v0.2.0 and v0.3.0 release.yaml + # runs hit, both AFTER `npm install -g npm@latest` reported success. Fixed + # in 12.0.1 (2026-07-10); bump this pin deliberately, never float it again. + - run: npm install -g npm@11.6.2 - run: npm ci - run: npm run lint - run: npm run typecheck - run: npm run format:check - run: npm run test:coverage - run: npm run build - # Auth via npm trusted publishing (OIDC) — no token needed; provenance is implied - - run: npm publish --provenance --access public + # Auth via npm trusted publishing (OIDC) — no token needed; provenance is + # implied. A prerelease tag (e.g. v0.3.0-rc.1, contains "-") must publish + # under an explicit dist-tag: current npm already refuses an implicit + # `latest` for a prerelease version, but failing mid-release is worse than + # not needing the guard, so this still passes --tag itself rather than + # relying on that enforcement alone. + - name: Publish + env: + REF_NAME: ${{ github.ref_name }} + run: | + if [[ "$REF_NAME" == *-* ]]; then + npm publish --provenance --access public --tag next + else + npm publish --provenance --access public + fi diff --git a/.github/workflows/test-coverage.yml b/.github/workflows/test-coverage.yml index ebb3331..0e20d9e 100644 --- a/.github/workflows/test-coverage.yml +++ b/.github/workflows/test-coverage.yml @@ -16,6 +16,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + with: + persist-credentials: false - name: Setup Node.js uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 diff --git a/.gitleaks.toml b/.gitleaks.toml index b3f68df..b31d49c 100644 --- a/.gitleaks.toml +++ b/.gitleaks.toml @@ -19,4 +19,23 @@ regexes = [ '''sk-secret-[0-9]+''', # Documented placeholder keys used by dry-run/sample responses '''sk-user-(test|DRY-RUN)''', + # Example values inside internal (never-shipped, dropped-before-publish) + # design docs. These are only ever scanned by ci.yml's continuous gitleaks + # job (new, 2026-07) — the release-time scan + # (scripts/make-public-snapshot.sh) already drops those internal docs + # wholesale before it ever runs gitleaks, so these never previously + # tripped a scan. Deliberately described here without naming the internal + # doc paths themselves (this file ships to the public repo, and the + # snapshot's own internal-doc-reference scan would flag such a path). + # A "Bearer " curl example whose placeholder token literal is the + # word "dev-token" — not a real key. (Allowlist regexes match against the + # extracted secret value, not the surrounding line — verified empirically + # with `gitleaks detect`.) + '''^dev-token$''', + # A literal base64 pagination-cursor EXAMPLE value in an internal OpenAPI + # spec (decodes to {"ek":"project_a47b2c11"} — spec-example data, not a + # credential). Anchored ^…$ so it only ever matches this exact literal — + # an unanchored version would also suppress any REAL secret that happens + # to contain this substring (review round 2, 2026-07-15; reproduced). + '''^eyJlayI6InByb2plY3RfYTQ3YjJjMTEifQ==$''', ] diff --git a/CHANGELOG.md b/CHANGELOG.md index e78347e..45803e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,27 @@ All notable changes to `@testsprite/testsprite-cli` are documented here. The for ## [Unreleased] +## [0.4.0] - 2026-07-16 + +### Added + +- **`test cancel `** — user-initiated cancel of in-flight runs (the real stop button; Ctrl-C only detaches). A single id renders the run card with status `cancelled` (plus an advisory when it was already cancelled); multiple ids print a `{cancelled, alreadyCancelled, conflicts, notFound}` summary. Exit codes: 4 when any id is not found, else 6 on conflicts, else 0. `--dry-run` supported. +- **Graceful Ctrl-C during `--wait`** — SIGINT/SIGTERM now detaches cleanly instead of killing the process mid-poll: the in-flight request aborts immediately, stdout gets the same partial `{runId, status: "running"}` envelope as the request-timeout path, and stderr states the truth — the server-side run keeps executing (and billing) — with a re-attach hint and a `test cancel` pointer. Exit 130/143/129 per the documented signal contract; a second signal forces a hard exit. Interrupting never cancels the server-side run — that's what `test cancel` is for. +- **`project delete --confirm`** — permanently delete a project and everything under it (its frontend/backend sub-projects, all their tests, and backend fixtures), mirroring the Portal's cascade delete. Requires `--confirm` (the CLI never prompts); `--dry-run` previews the response shape without a network call. Standard exit codes: 0 success, 3 auth, 4 not-found (or already-deleted), 5 validation (e.g. missing `--confirm`). +- **Backend stdout and traceback in results** — `test result` and the failure bundle now surface the backend test's captured stdout and Python traceback: full content in `--output json` (and in `result.json` / `failure.json` bundle files), and a bounded 20-line tail with a byte count in text mode. No change for frontend tests, passing runs, or older backends. +- **Backend dependency declarations are now readable and editable** — `test get` surfaces `produces` / `consumes` / `category`, and `test update` accepts `--produces` / `--needs` / `--category` (previously create-only). +- **Version-compatibility handshake** — the CLI reads the backend's advertised minimum-supported-version on every response and prints a one-line upgrade advisory on stderr when the running version is below the floor (honors the same opt-outs as the update notice; never alters exit status). A `CLIENT_TOO_OLD` rejection (HTTP 426) is now a first-class error: exit 14, non-retriable, rendered with upgrade guidance and the version gap. +- **V3 routing visibility** — `auth status` and `doctor` render a `routing: v2|v3` line when the backend reports the account's routing, and V3-routed accounts get one consolidated advisory listing the known V3-path behavior gaps. Text mode only — JSON consumers read `v3Enabled` from the `/me` payload; absent-safe against older backends. + +### Changed + +- **The `testsprite-verify` agent skill routes local-only changes to the TestSprite MCP** — the skill now states the reachability gate explicitly: the CLI verifies reachable deployed URLs only; when the change is only running locally, the skill hands off to the TestSprite MCP when available (an explicitly named tool always wins), instead of failing against localhost. + +### Fixed + +- **`project create --description` now fails fast with a clear validation error** — projects have no description field, so the flag's value was previously dropped silently; the error points at test-level descriptions (`test create --description`) instead. +- **Standalone backend run cards no longer show a misleading step summary** — `test run` / `test wait` / `test rerun` cards for backend tests render `steps: n/a (backend)` instead of `0/0 (passed=0, failed=0)` (backend tests have no per-step storage). + ## [0.3.0] - 2026-07-08 ### Added diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bed91fc..1479099 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -109,11 +109,36 @@ npm run format:check # Prettier (check only) npm run typecheck # tsc --noEmit ``` +## Developing on Windows + +Native Windows (no WSL, no Git Bash required) is a fully supported dev +environment — you only need **git** and **Node ≥ 20**. `npm ci`, `npm run +build`, `npm test`, `npm run lint`, and `npm run typecheck` all run the same +way as on macOS/Linux, and CI runs the unit suite on `windows-latest` as the +reference environment (see [CI gates](#ci-gates-required-for-merge) below) — +if it's green there, it's green on your machine. + +A couple of things worth knowing: + +- Line endings are normalized to LF on checkout via `.gitattributes` + (`core.autocrlf` doesn't need any special local configuration). +- `--out`/bundle-path flags accept native Windows paths (`C:\Users\...`) + including a trailing backslash and a bare drive root (`C:\`). +- A small number of tests that create real filesystem symlinks are skipped on + Windows (creating a symlink there needs Administrator rights or Developer + Mode, which isn't guaranteed on hosted CI) — the safety behavior they cover + is still exercised on the Linux/macOS CI jobs. + +Hit something that doesn't work on Windows? Please file it — see +[Questions & support](#questions--support) above, and feel free to tag it +`good first issue` if it looks like an isolated fix; we'd rather know than +have you route around it silently. + ## CI gates (required for merge) - ESLint + Prettier clean - TypeScript type-check clean -- All unit tests passing +- All unit tests passing on Linux (Node 20 + 22) **and** on Windows (`windows-latest`, Node 22) - Coverage ≥ 80% on lines / statements / functions / branches - Build + smoke test of the CLI binary diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index 2f6cd6c..bf6b1b5 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -180,7 +180,7 @@ Common flags: #### `testsprite test get ` -Get a single test by id. Test ids look like `test_xxxxxxxx` and come from `test list`. +Get a single test by id. Test ids look like `test_xxxxxxxx` and come from `test list`. Backend tests echo their dependency declarations — `produces` / `consumes` / `category` — when present. ```bash testsprite test get test_xxxxxxxx --output json @@ -210,7 +210,7 @@ Common flags: `--page-size`, `--starting-token`, `--max-items` — same shape as #### `testsprite test result ` -Get the latest result for a test — status, started / finished timestamps, video and failure-analysis URLs, summary counts (`passed / failed / skipped`), and correlation fields (`snapshotId`, `runId`, `codeVersion`). With `--include-analysis`, the response also carries an inline `analysis` block (root-cause hypothesis, recommended fix target, failure kind). +Get the latest result for a test — status, started / finished timestamps, video and failure-analysis URLs, summary counts (`passed / failed / skipped`), and correlation fields (`snapshotId`, `runId`, `codeVersion`). With `--include-analysis`, the response also carries an inline `analysis` block (root-cause hypothesis, recommended fix target, failure kind). Backend tests additionally surface the run's captured stdout (`apiOutput`) and Python traceback (`trace`): full content under `--output json` (and in `result.json` / `failure.json` inside failure bundles); text mode prints a bounded 20-line tail of each with a byte count. ```bash testsprite test result test_xxxxxxxx --output json @@ -293,7 +293,7 @@ testsprite test lint --steps ./refined.plan.json # the shape `test plan pu #### `testsprite test create` -Create a new test. Backend tests use `--code-file` (agents supply backend code directly); frontend tests use either `--code-file` or `--plan-from`. With `--run --wait`, the CLI chains create → trigger → poll in a single invocation. +Create a new test. Backend tests use `--code-file` (agents supply backend code directly); frontend tests use either `--code-file` or `--plan-from`. With `--run --wait`, the CLI chains create → trigger → poll in a single invocation. Backend tests can declare wave-ordering dependencies at create time — `--produces ` / `--needs ` (repeatable) and `--category ` — and amend them later via `test update`. ```bash # Backend test from a code file @@ -319,10 +319,11 @@ testsprite test create-batch --plan-from-dir ./plans/ --dry-run --output json #### `testsprite test update ` -Update test metadata (name, description). +Update test metadata (name, description, priority) — and, for **backend tests**, the dependency declarations: `--produces ` / `--needs ` (repeatable) and `--category `. Updated declarations are echoed back by `test get`. ```bash testsprite test update test_xxxxxxxx --name "Renamed test" --description "Updated" +testsprite test update test_be_xxxx --produces session_token --category setup testsprite test update test_xxxxxxxx --dry-run --output json ``` @@ -358,13 +359,22 @@ testsprite test plan put test_xxxxxxxx --steps ./refined.plan.json --dry-run --o #### `testsprite project create` / `project update` -Manage projects from the CLI. Both pre-flight `--url` against local addresses for fast feedback. Note the asymmetry: `--description` is **create-only** — `project update` accepts `--name`, `--url`, `--username`, `--password`, `--password-file`, and `--instruction`, but not `--description`. +Manage projects from the CLI. Both pre-flight `--url` against local addresses for fast feedback. Projects have **no description field** — `--description` is rejected client-side with a validation error (descriptions live on tests: `test create --description`). `project update` accepts `--name`, `--url`, `--username`, `--password`, `--password-file`, and `--instruction`. ```bash testsprite project create --type frontend --name "Checkout" --url https://staging.example.com testsprite project update proj_xxxxxxxx --name "Checkout v2" ``` +#### `testsprite project delete ` + +Permanently delete a project and **everything under it** — its frontend/backend sub-projects, all their tests, and backend fixtures (mirrors the Portal's cascade delete). There is **no restore window**. `--confirm` is required (the CLI never prompts); absent it, the CLI exits 5 with a local validation error. `--dry-run` previews the response shape without a network call. Exit codes: 0 success, 3 auth, 4 not found (or already deleted), 5 validation. + +```bash +testsprite project delete proj_xxxxxxxx --confirm +testsprite project delete proj_xxxxxxxx --dry-run --output json +``` + #### `testsprite project credential ` Set the **static backend credential** injected into every backend test in the project (free tier). Supported types: `public` (no credential), `"Bearer token"`, `"API key"`, `"basic token"`. @@ -521,6 +531,16 @@ testsprite test wait run_01hx3z9p8q4k2y7a --dry-run --output json With several ids, a per-member poll error (e.g. one id not found) is recorded as `error:` in that run's row and folded into exit 7, rather than aborting the whole batch. Polling is handled automatically — the CLI uses server-driven long-poll where supported and exponential backoff with jitter otherwise, honoring `Retry-After`. +#### `testsprite test cancel ` + +Cancel one or more in-flight runs — the counterpart to Ctrl-C, which only **detaches** (the server-side run keeps executing and billing). Cancelling is idempotent: an already-cancelled run reports `alreadyCancelled` as an advisory, not an error; a run that already reached a terminal verdict is a conflict — the verdict is never overwritten, and no credits are refunded. With one id, prints the run card; with several, prints a `{ cancelled, alreadyCancelled, conflicts, notFound }` summary. Exit codes: any unknown id → 4; else any conflict → 6; else 0. + +```bash +testsprite test cancel run_01hx3z9p8q4k2y7a +testsprite test cancel run_aaaa run_bbbb --output json +testsprite test cancel run_01hx3z9p8q4k2y7a --dry-run --output json +``` + #### `testsprite test artifact get ` Download the failure bundle for a specific `runId`. Same on-disk layout as `test failure get`, but addressed by `runId` instead of `testId`, so an agent can fetch the bundle for the exact run it just triggered — never a newer failure on the same test. Default `` is `./.testsprite/runs//`. The CLI enforces `meta.runId === ` as an integrity check; a mismatch exits 5 rather than silently writing the wrong bundle. @@ -603,6 +623,13 @@ check is skipped in CI, when stderr is not a TTY, under `--output json` / failure is silent: the notice can never break or delay a command. This is the only outbound call the CLI makes besides your configured API endpoint. +Separately, the backend advertises its **minimum supported CLI version** on +every `/api/cli/v1` response. When the running CLI is below that floor, a +one-line upgrade advisory is printed to stderr (same opt-outs as the update +notice; it never changes the exit status). A backend may also reject a +too-old client outright with HTTP 426 — surfaced as `CLIENT_TOO_OLD`, +exit `14`, non-retriable, with upgrade guidance. + ### Scopes API-key scopes gate the write and run surfaces: @@ -613,8 +640,8 @@ API-key scopes gate the write and run surfaces: | `read:projects` | `project list / get` | | `read:tests` | every `test *` read command | | `write:tests` | `test create / create-batch / update / delete / code put / plan put` | -| `write:projects` | `project create / update / credential / auto-auth` | -| `run:tests` | `test run / rerun / flaky / wait / artifact get` | +| `write:projects` | `project create / update / delete / credential / auto-auth` | +| `run:tests` | `test run / rerun / flaky / wait / cancel / artifact get` | New API keys include the full scope set. If a command returns `AUTH_FORBIDDEN`, the missing scope is named in `details.requiredScope` — regenerate your key from the dashboard to pick up new scopes. @@ -632,25 +659,26 @@ testsprite test wait "$RUN_ID" --timeout 600 --output json || echo "run did not ## Exit codes -| Code | Meaning | -| --------------------- | --------------------------------------------------------------------------- | -| `0` | Success | -| `1` | Generic failure / non-passed run status | -| `2` | Not yet implemented | -| `3` | Auth error | -| `4` | Not found | -| `5` | Validation error / payload too large | -| `6` | Conflict / precondition failed | -| `7` | Timeout / unsupported | -| `10` | Service unavailable | -| `11` | Rate limited (retriable) | -| `12` | Insufficient credits (non-retriable) | -| `13` | Feature gated (paid plan required) | -| `129` / `130` / `143` | Interrupted by a signal (SIGHUP / SIGINT / SIGTERM) — `128 + signal number` | +| Code | Meaning | +| --------------------- | ------------------------------------------------------------------------------------------------- | +| `0` | Success | +| `1` | Generic failure / non-passed run status | +| `2` | Not yet implemented | +| `3` | Auth error | +| `4` | Not found | +| `5` | Validation error / payload too large | +| `6` | Conflict / precondition failed | +| `7` | Timeout / unsupported | +| `10` | Service unavailable | +| `11` | Rate limited (retriable) | +| `12` | Insufficient credits (non-retriable) | +| `13` | Feature gated (paid plan required) | +| `14` | Client too old — the backend requires a newer CLI (HTTP 426 `CLIENT_TOO_OLD`); upgrade to proceed | +| `129` / `130` / `143` | Interrupted by a signal (SIGHUP / SIGINT / SIGTERM) — `128 + signal number` | ### Signals & pipes -On SIGINT (Ctrl-C), SIGTERM, or SIGHUP the CLI prints `Interrupted (). Any run already started keeps executing on the server; check it with 'testsprite test list' or 'testsprite test wait '.` and exits `128 + signal`. **Ctrl-C does not cancel the server-side run** — execution (and any credit spend) continues; there is no cancel command today, so re-attach with `test wait ` instead of re-triggering. A closed stdout pipe (`EPIPE`, e.g. `testsprite test list | head`) exits `0` silently rather than crashing. +During any `--wait`, SIGINT (Ctrl-C), SIGTERM, or SIGHUP triggers a **graceful detach**: the in-flight request aborts immediately, stdout gets the same partial `{ runId, status: "running" }` envelope as the request-timeout path (under `--output json`, stderr carries an `INTERRUPTED` envelope naming the signal), and stderr states the truth — the server-side run keeps executing, and any credit spend continues — with a re-attach hint (`test wait `) and a `test cancel ` pointer. The exit code is `128 + signal` (130 / 143 / 129). A second signal forces an immediate hard exit. Outside a `--wait` (prompts, one-shot commands), signals keep the pre-existing immediate-exit behavior. **Ctrl-C never cancels the server-side run** — `test cancel ` is the explicit stop. A closed stdout pipe (`EPIPE`, e.g. `testsprite test list | head`) exits `0` silently rather than crashing. ## Design principles diff --git a/README.md b/README.md index 78d293a..3bb465e 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,7 @@ TESTSPRITE_API_KEY=sk-... testsprite setup --from-env --yes --agent claude > **Pointing a coding agent (Claude Code, Cursor, Codex, Cline, …) at TestSprite?** Have it run `testsprite setup` first — that installs the verification skill, so the agent knows how to create, run, and triage tests on its own (instead of guessing from this README). New here? Start with the **[getting-started overview](https://docs.testsprite.com/cli/getting-started/overview)**. -> **Privacy note:** interactive runs check the npm registry at most once per 24 h to offer a "new version available" notice — package name only, never your key or data; `TESTSPRITE_NO_UPDATE_NOTIFIER=1` disables it. Details in [DOCUMENTATION.md → Update notice](./DOCUMENTATION.md#update-notice). +> **Privacy note:** interactive runs check the npm registry at most once per 24 h to offer a "new version available" notice — package name only, never your key or data; `TESTSPRITE_NO_UPDATE_NOTIFIER=1` disables it. The backend also advertises its minimum supported CLI version — a below-floor CLI prints a one-line upgrade advisory on stderr, and a too-old client may be rejected with exit 14 (`CLIENT_TOO_OLD`). Details in [DOCUMENTATION.md → Update notice](./DOCUMENTATION.md#update-notice). From there, the loop runs on its own — an example session, typed by the coding agent: @@ -91,34 +91,35 @@ Prefer to configure each step by hand (or learn the surface offline with `--dry- ## Commands -| Group | Command | What it does | -| --------- | --------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------- | -| **Setup** | `setup` | **Start here** — one command: configure your API key, verify it, and install the agent verification skill | -| | `doctor` | Environment diagnostic — CLI/Node versions, profile, endpoint, credentials, connectivity, agent skill; exits non-zero on failure | -| **Auth** | `auth status` | Resolve the active profile to its user, key, env, and scopes | -| | `auth remove` | Remove the active profile from the credentials file | -| | `usage` (alias `credits`) | Account pre-flight: identity, plus credit balance / plan info when the backend supplies them | -| **Read** | `project list` / `project get` | List projects / fetch one by id | -| | `test list` / `test get` | List tests under a project / fetch one by id | -| | `test code get` | Print (or write) the generated test source | -| | `test steps` | List the latest run's steps with screenshot / DOM pointers | -| | `test result` | Latest result; `--history` lists a test's prior runs | -| | `test failure get` | The agent entry point: one self-contained latest-failure bundle | -| | `test failure summary` | One-screen triage card (no media download) | -| | `test diff` | Compare two runs — verdict, failure kind, per-step status flips, code-version drift | -| **Write** | `test scaffold` / `test lint` | Author plans locally: emit a schema-correct starter, validate plan files offline — no network, no credentials | -| | `test create` / `test create-batch` | Create a test (or bulk-create from a plan file); `--produces` / `--needs` / `--category` wire BE dependency metadata | -| | `test update` / `test delete` / `test delete-batch` | Edit metadata / permanently delete (no restore window; `--confirm` required) | -| | `test code put` | Replace generated code (etag-guarded) | -| | `test plan put` | Replace a frontend test's plan-steps | -| | `project create` / `project update` | Manage projects | -| | `project credential` / `project auto-auth` | Configure backend-test auth: a static injected credential, or auto-refresh login (Pro) | -| **Run** | `test run` | Trigger a fresh run; `--wait` blocks until terminal; `--all --project ` runs all tests in a project in wave order | -| | `test rerun` | Cheap replay of one/many tests (FE verbatim; BE with deps); `--all --project ` reruns all tests | -| | `test flaky` | Replay a test several times (auto-heal off) and report a stability score | -| | `test wait` | Block on one or more `runId`s until terminal | -| | `test artifact get` | Download the failure bundle for a specific `runId` | -| **Agent** | `agent install` / `agent list` / `agent status` | Add, list, or health-check coding-agent skills (pure-local): `claude`, `codex`, `cursor`, `cline`, `antigravity`, `kiro`, `windsurf`, `copilot` | +| Group | Command | What it does | +| --------- | ------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Setup** | `setup` | **Start here** — one command: configure your API key, verify it, and install the agent verification skill | +| | `doctor` | Environment diagnostic — CLI/Node versions, profile, endpoint, credentials, connectivity, agent skill; exits non-zero on failure | +| **Auth** | `auth status` | Resolve the active profile to its user, key, env, and scopes | +| | `auth remove` | Remove the active profile from the credentials file | +| | `usage` (alias `credits`) | Account pre-flight: identity, plus credit balance / plan info when the backend supplies them | +| **Read** | `project list` / `project get` | List projects / fetch one by id | +| | `test list` / `test get` | List tests under a project / fetch one by id | +| | `test code get` | Print (or write) the generated test source | +| | `test steps` | List the latest run's steps with screenshot / DOM pointers | +| | `test result` | Latest result; `--history` lists a test's prior runs | +| | `test failure get` | The agent entry point: one self-contained latest-failure bundle | +| | `test failure summary` | One-screen triage card (no media download) | +| | `test diff` | Compare two runs — verdict, failure kind, per-step status flips, code-version drift | +| **Write** | `test scaffold` / `test lint` | Author plans locally: emit a schema-correct starter, validate plan files offline — no network, no credentials | +| | `test create` / `test create-batch` | Create a test (or bulk-create from a plan file); `--produces` / `--needs` / `--category` wire BE dependency metadata | +| | `test update` / `test delete` / `test delete-batch` | Edit metadata and BE dependency declarations (`--produces` / `--needs` / `--category`) / permanently delete (no restore window; `--confirm` required) | +| | `test code put` | Replace generated code (etag-guarded) | +| | `test plan put` | Replace a frontend test's plan-steps | +| | `project create` / `project update` / `project delete` | Manage projects; `delete` removes a project and everything under it (`--confirm` required, no restore window) | +| | `project credential` / `project auto-auth` | Configure backend-test auth: a static injected credential, or auto-refresh login (Pro) | +| **Run** | `test run` | Trigger a fresh run; `--wait` blocks until terminal; `--all --project ` runs all tests in a project in wave order | +| | `test rerun` | Cheap replay of one/many tests (FE verbatim; BE with deps); `--all --project ` reruns all tests | +| | `test flaky` | Replay a test several times (auto-heal off) and report a stability score | +| | `test wait` | Block on one or more `runId`s until terminal | +| | `test cancel` | Cancel one or more in-flight runs (Ctrl-C during `--wait` only detaches — `cancel` is the real stop) | +| | `test artifact get` | Download the failure bundle for a specific `runId` | +| **Agent** | `agent install` / `agent list` / `agent status` | Add, list, or health-check coding-agent skills (pure-local): `claude`, `codex`, `cursor`, `cline`, `antigravity`, `kiro`, `windsurf`, `copilot` | > The earlier command names — `init`, `auth configure`, `auth whoami`, `auth logout` — still work as hidden, deprecated aliases (each prints a one-line notice pointing at the new name), so existing scripts keep running. `auth configure` now runs the full `setup` (it also installs the skill). diff --git a/package-lock.json b/package-lock.json index b254e0d..4461016 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@testsprite/testsprite-cli", - "version": "0.2.0", + "version": "0.3.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@testsprite/testsprite-cli", - "version": "0.2.0", + "version": "0.3.0", "license": "Apache-2.0", "dependencies": { "commander": "^12.1.0", diff --git a/package.json b/package.json index e78ce17..18336cb 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@testsprite/testsprite-cli", - "version": "0.3.0", + "version": "0.4.0", "description": "Official TestSprite command-line interface", "type": "module", "main": "dist/index.js", diff --git a/scripts/README.md b/scripts/README.md new file mode 100644 index 0000000..7e17420 --- /dev/null +++ b/scripts/README.md @@ -0,0 +1,29 @@ +# `scripts/` — what runs where + +Every file here is classified below so nobody has to guess whether it's +safe/expected to run on a laptop, and so a Windows contributor knows exactly +what (if anything) they're missing. Per DEV-356 ("Windows-proof the +toolchain"): **the release/backport shell scripts are CI-only** — nothing in +this directory requires a human to run bash or perl locally. + +| File | Classification | Runs on | Notes | +| ------------------------- | -------------------------------------------------- | --------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `make-public-snapshot.sh` | **CI-ONLY** | the release pipeline's build job (`release-build.yml`) and the nightly divergence sentinel (dry-run mode) | Builds the scrubbed public snapshot. Never ships to the public repo (it's in its own DROP list). Not meant to be run by hand — see [`docs/internal/cli-oss/release-pipeline-ops.md`](../docs/internal/cli-oss/release-pipeline-ops.md) for the operator-facing flow. | +| `backport-public-pr.sh` | **CI-ONLY** (human-runnable for conflict recovery) | the auto-backport workflow, invoked per merged community PR | Also directly runnable by an operator resolving a cherry-pick conflict (`--record-only` after a manual fix) — that's an escape hatch, not the steady-state path. Requires `bash` + `gh` + `jq`; a Windows operator resolving a conflict does it via Git Bash/WSL, or asks another maintainer — this one script is the sole remaining bash dependency in the whole release flow. | +| `generate-version.mjs` | **HUMAN-RUN** (Node) | `npm run prebuild` / `npm run generate:version`, any OS | Pure Node — no shell-out, no bash/perl/BSD-vs-GNU assumptions. | +| `postbuild.mjs` | **HUMAN-RUN** (Node) | `npm run build`, any OS | Pure Node `fs` calls (sets the executable bit on `dist/index.js`; a no-op on Windows, which has no POSIX exec bit). | +| `p0-status-coverage.sql` | **DROPPED-INTERNAL** | ad hoc, by an operator, against Athena | Not part of any npm script or CI workflow. Never ships to the public repo (internal AWS resource references). | + +## The upshot for a Windows contributor / operator + +- **Local dev loop** (`npm ci`, `npm test`, `npm run build`, `npm run lint`, + `npm run typecheck`) needs only **git + Node ≥ 20** — nothing in this + directory runs as part of that loop. See CONTRIBUTING.md's "Developing on + Windows" section. +- **Releasing** does not require running `make-public-snapshot.sh` (or any + bash) on your own machine at all — it's a `workflow_dispatch` you trigger + and approve from a browser. See + [`docs/internal/cli-oss/release-pipeline-ops.md`](../docs/internal/cli-oss/release-pipeline-ops.md). +- The **only** scenario that still touches bash directly is resolving a rare + backport cherry-pick conflict by hand — everything else in the pipeline is + either Node or runs unattended in Actions. diff --git a/skills/testsprite-verify.codex.md b/skills/testsprite-verify.codex.md index f2c2456..5d28f16 100644 --- a/skills/testsprite-verify.codex.md +++ b/skills/testsprite-verify.codex.md @@ -10,6 +10,12 @@ docs/config and is about to be reported complete. Run after a feature or fix lands. Skip only for: docs-only edits, pure build/config changes, or when the repo has no TestSprite project linked. +The CLI only tests a reachable deployed URL (it rejects localhost). If the +change is only running locally, hand off to the TestSprite MCP when it's +available — it tunnels your local server; otherwise report the change as +unverified-because-undeployed and stop. If the user explicitly named a tool +(the CLI or the MCP), honor that over this reachability heuristic. + ## Core loop ### 1. Preflight diff --git a/skills/testsprite-verify.skill.md b/skills/testsprite-verify.skill.md index fe7c661..5020e42 100644 --- a/skills/testsprite-verify.skill.md +++ b/skills/testsprite-verify.skill.md @@ -21,6 +21,17 @@ Run the loop only once the change is live somewhere reachable (e.g. open the PR, let CI deploy the preview/staging environment) and pass that URL as `--target-url`. Running earlier verifies the previous build, not your change. +This CLI only tests a reachable deployed URL (it rejects localhost). If the +change is only running locally and isn't deployed anywhere reachable yet: + +- if the TestSprite MCP is available in this environment, hand off to it — it + tunnels your local server and tests the running app; +- otherwise report the change as unverified-because-undeployed and stop — don't + run against a stale deployment to manufacture a verdict. + +If the user explicitly named a tool (the CLI or the MCP), honor that over this +reachability heuristic. + ## When to skip The skip list is narrow: @@ -225,10 +236,10 @@ testsprite test create --type backend --project --name "fixture user ordering: trigger the set with `test run --all` (§4) and producers run before consumers, `teardown` last. Chaining `run A --wait && run B --wait` yourself loses the engine's variable passing and conflicts with concurrent runs. -- **Declarations are currently create-only.** `test update` cannot amend them and - `test get` / `test list` don't echo them back, so note what each test - produces/needs as you create it; changing the graph later means delete + - recreate. +- **Declarations are editable.** `test update` amends them (`--produces` / + `--needs` / `--category`, repeatable) and `test get` echoes them back — + still declare the graph at create time when you can, so wave ordering is + right on the first run. **Show the user the drafted plan / code before creating it** — creating writes to their project. One short confirmation; let them edit the tempfile first. diff --git a/src/commands/agent.test.ts b/src/commands/agent.test.ts index 24d8ec6..7326aeb 100644 --- a/src/commands/agent.test.ts +++ b/src/commands/agent.test.ts @@ -1108,75 +1108,87 @@ describe('runInstall — default AgentFs (real disk)', () => { expect(readFileSync(abs, 'utf8')).toBe(content); }); - it('refuses to write through a symlinked parent dir (real disk) — exit 5', async () => { - const tmpRoot = mkdtempSync(path.join(tmpdir(), 'agent-test-symlink-parent-')); - const outside = mkdtempSync(path.join(tmpdir(), 'agent-test-outside-')); - // `.claude` is a real symlink to a directory outside the project root. - symlinkSync(outside, path.join(tmpRoot, '.claude'), 'dir'); - const { deps } = makeCapture(); - - let thrown: unknown; - try { - await runInstall( - { - profile: 'default', - output: 'text', - debug: false, - dryRun: false, - target: ['claude'], - skills: ['testsprite-verify'], - force: false, - dir: tmpRoot, - }, - { ...deps }, - ); - } catch (err) { - thrown = err; - } - - expect(thrown).toBeInstanceOf(CLIError); - expect((thrown as CLIError).exitCode).toBe(5); - // Nothing was created through the symlink, outside --dir. - expect(existsSync(path.join(outside, 'skills'))).toBe(false); - }); - - it('refuses to overwrite a symlinked target file (real disk) with --force — exit 5', async () => { - const tmpRoot = mkdtempSync(path.join(tmpdir(), 'agent-test-symlink-target-')); - const outsideDir = mkdtempSync(path.join(tmpdir(), 'agent-test-outside-target-')); - const { path: relPath } = renderForTarget('claude', 'testsprite-verify'); - const abs = path.resolve(tmpRoot, relPath); - const nodeFs = await import('node:fs/promises'); - await nodeFs.mkdir(path.dirname(abs), { recursive: true }); - // SKILL.md is a real symlink to a file outside the project root. - const outsideFile = path.join(outsideDir, 'secret.txt'); - await nodeFs.writeFile(outsideFile, 'SECRET', 'utf8'); - symlinkSync(outsideFile, abs, 'file'); - const { deps } = makeCapture(); + // `fs.symlinkSync` needs elevated privileges or Developer Mode on Windows + // (EPERM otherwise) — not guaranteed on hosted CI runners. The underlying + // guard (`inspectTargetPath` fail-closing via `lstat`) is exercised on + // POSIX runners; TODO(DEV-356): revisit if/when a reliable Windows + // symlink-creation path (junctions, or an elevated runner) is available. + it.skipIf(process.platform === 'win32')( + 'refuses to write through a symlinked parent dir (real disk) — exit 5', + async () => { + const tmpRoot = mkdtempSync(path.join(tmpdir(), 'agent-test-symlink-parent-')); + const outside = mkdtempSync(path.join(tmpdir(), 'agent-test-outside-')); + // `.claude` is a real symlink to a directory outside the project root. + symlinkSync(outside, path.join(tmpRoot, '.claude'), 'dir'); + const { deps } = makeCapture(); + + let thrown: unknown; + try { + await runInstall( + { + profile: 'default', + output: 'text', + debug: false, + dryRun: false, + target: ['claude'], + skills: ['testsprite-verify'], + force: false, + dir: tmpRoot, + }, + { ...deps }, + ); + } catch (err) { + thrown = err; + } - let thrown: unknown; - try { - await runInstall( - { - profile: 'default', - output: 'text', - debug: false, - dryRun: false, - target: ['claude'], - skills: ['testsprite-verify'], - force: true, - dir: tmpRoot, - }, - { ...deps }, - ); - } catch (err) { - thrown = err; - } + expect(thrown).toBeInstanceOf(CLIError); + expect((thrown as CLIError).exitCode).toBe(5); + // Nothing was created through the symlink, outside --dir. + expect(existsSync(path.join(outside, 'skills'))).toBe(false); + }, + ); + + // Same Windows symlink-privilege caveat as the test above. + it.skipIf(process.platform === 'win32')( + 'refuses to overwrite a symlinked target file (real disk) with --force — exit 5', + async () => { + const tmpRoot = mkdtempSync(path.join(tmpdir(), 'agent-test-symlink-target-')); + const outsideDir = mkdtempSync(path.join(tmpdir(), 'agent-test-outside-target-')); + const { path: relPath } = renderForTarget('claude', 'testsprite-verify'); + const abs = path.resolve(tmpRoot, relPath); + const nodeFs = await import('node:fs/promises'); + await nodeFs.mkdir(path.dirname(abs), { recursive: true }); + // SKILL.md is a real symlink to a file outside the project root. + const outsideFile = path.join(outsideDir, 'secret.txt'); + await nodeFs.writeFile(outsideFile, 'SECRET', 'utf8'); + symlinkSync(outsideFile, abs, 'file'); + const { deps } = makeCapture(); + + let thrown: unknown; + try { + await runInstall( + { + profile: 'default', + output: 'text', + debug: false, + dryRun: false, + target: ['claude'], + skills: ['testsprite-verify'], + force: true, + dir: tmpRoot, + }, + { ...deps }, + ); + } catch (err) { + thrown = err; + } - expect(thrown).toBeInstanceOf(CLIError); - expect((thrown as CLIError).exitCode).toBe(5); - // The outside file was NOT overwritten (nor clobbered via the .bak path). - expect(readFileSync(outsideFile, 'utf8')).toBe('SECRET'); - }); + expect(thrown).toBeInstanceOf(CLIError); + expect((thrown as CLIError).exitCode).toBe(5); + // The outside file was NOT overwritten (nor clobbered via the .bak path). + expect(readFileSync(outsideFile, 'utf8')).toBe('SECRET'); + }, + ); }); // --------------------------------------------------------------------------- @@ -2036,12 +2048,12 @@ describe('[P3 round-2] codex --dry-run: composed-size precision + read-failure s const { capture, deps } = makeCapture(); const agentsAbs = path.resolve(CWD, TARGETS.codex.path); - // Old managed section with a 6 KiB body + 26 KiB of user prose. - // existing (~32.9 KiB) + new section (~4.8 KiB) > 32 KiB → the OLD formula - // would warn; the composed replace result (26 KiB + new section) is - // comfortably under budget → no warn expected. + // Old managed section with a 6 KiB body + 25 KiB of user prose. + // existing (~31.9 KiB) + new section (~6.2 KiB: verify codex body + + // onboard aggregate) > 32 KiB → the OLD formula would warn; the composed + // replace result (25 KiB + new section) is under budget → no warn expected. const oldSection = `${MANAGED_SECTION_BEGIN}\n${'o'.repeat(6 * 1024)}\n${MANAGED_SECTION_END}\n`; - const userProse = `# My own AGENTS.md\n${'u'.repeat(26 * 1024)}\n`; + const userProse = `# My own AGENTS.md\n${'u'.repeat(25 * 1024)}\n`; seedFile(agentsAbs, `${userProse}\n${oldSection}`); await runInstall({ ...BASE_OPTS_DRY, target: ['codex'] }, { cwd: CWD, fs: agentFs, ...deps }); diff --git a/src/commands/auth.test.ts b/src/commands/auth.test.ts index c0b4e00..6f8ce94 100644 --- a/src/commands/auth.test.ts +++ b/src/commands/auth.test.ts @@ -850,6 +850,63 @@ describe('runWhoami', () => { expect(printed).toEqual(sampleMe); }); + it('renders routing: v3 and the gap advisory when v3Enabled is true', async () => { + writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); + const { capture, deps } = makeCapture(); + const meV3 = new Response(JSON.stringify({ ...sampleMe, v3Enabled: true }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + await runWhoami( + { profile: 'default', output: 'text', debug: false }, + { ...deps, env: {}, credentialsPath, fetchImpl: makeFetch(meV3) }, + ); + expect(capture.stdout.join('\n')).toContain('routing: v3'); + expect(capture.stderr.join('\n')).toContain('[advisory]'); + expect(capture.stderr.join('\n')).toContain('test cancel'); + }); + + it('renders routing: v2 and NO advisory when v3Enabled is false', async () => { + writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); + const { capture, deps } = makeCapture(); + const meV2 = new Response(JSON.stringify({ ...sampleMe, v3Enabled: false }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + await runWhoami( + { profile: 'default', output: 'text', debug: false }, + { ...deps, env: {}, credentialsPath, fetchImpl: makeFetch(meV2) }, + ); + expect(capture.stdout.join('\n')).toContain('routing: v2'); + expect(capture.stderr.join('\n')).not.toContain('[advisory]'); + }); + + it('omits the routing line when the backend does not return v3Enabled', async () => { + writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); + const { capture, deps } = makeCapture(); + await runWhoami( + { profile: 'default', output: 'text', debug: false }, + { ...deps, env: {}, credentialsPath, fetchImpl: makeFetch(meResponse()) }, + ); + expect(capture.stdout.join('\n')).not.toContain('routing:'); + }); + + it('does not emit the advisory in JSON mode even when v3Enabled is true', async () => { + writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); + const { capture, deps } = makeCapture(); + const meV3 = new Response(JSON.stringify({ ...sampleMe, v3Enabled: true }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + await runWhoami( + { profile: 'default', output: 'json', debug: false }, + { ...deps, env: {}, credentialsPath, fetchImpl: makeFetch(meV3) }, + ); + expect(capture.stderr.join('\n')).not.toContain('[advisory]'); + const parsed = JSON.parse(capture.stdout.join('')) as MeResponse; + expect(parsed.v3Enabled).toBe(true); + }); + it('dry-run: whitespace-only TESTSPRITE_API_URL falls through to prod default endpoint', async () => { const { capture, deps } = makeCapture(); await runWhoami( diff --git a/src/commands/auth.ts b/src/commands/auth.ts index 59cd43f..e594066 100644 --- a/src/commands/auth.ts +++ b/src/commands/auth.ts @@ -22,6 +22,7 @@ import { emitDeprecationNotice } from '../lib/deprecate.js'; import type { OutputMode } from '../lib/output.js'; import { GLOBAL_OPTS_HINT, Output, resolveOutputMode } from '../lib/output.js'; import { promptSecret } from '../lib/prompt.js'; +import { emitV3RoutingAdvisory, routingLabel } from '../lib/v3-advisory.js'; export interface MeResponse { userId: string; @@ -37,6 +38,11 @@ export interface MeResponse { email?: string; /** Human-readable display name for the bound account. Absent-safe (dogfood L1866). */ displayName?: string; + /** + * Authoritative per-user V3 routing bit. Absent-safe: older backends omit it, + * so it is only rendered when present. + */ + v3Enabled?: boolean; } export interface AuthDeps { @@ -201,6 +207,7 @@ export async function runConfigure(opts: ConfigureOptions, deps: AuthDeps = {}): export async function runWhoami(opts: CommonOptions, deps: AuthDeps = {}): Promise { const out = makeOutput(opts.output, deps); const env = deps.env ?? process.env; + const stderr = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`)); // Resolve the endpoint URL so it can be surfaced in text output. // Dry-run uses the flag/env/default chain without touching credentials. @@ -244,6 +251,8 @@ export async function runWhoami(opts: CommonOptions, deps: AuthDeps = {}): Promi `endpoint: ${resolvedEndpoint}`, `env: ${m.env}`, `scopes: ${m.scopes.join(', ')}`, + // Authoritative routing mode, rendered only when the backend supplies it. + ...(m.v3Enabled !== undefined ? [`routing: ${routingLabel(m.v3Enabled)}`] : []), ]; // C2: warn in text mode when key cannot write/run const missingScopes = (['write:tests', 'run:tests'] as const).filter( @@ -256,6 +265,11 @@ export async function runWhoami(opts: CommonOptions, deps: AuthDeps = {}): Promi } return lines.join('\n'); }); + // When V3 routing is on, warn (text mode only) about the still-open behavior + // gaps. JSON consumers read `v3Enabled` directly; stdout stays pure. + if (opts.output !== 'json' && me.v3Enabled === true) { + emitV3RoutingAdvisory(stderr); + } return me; } diff --git a/src/commands/doctor.test.ts b/src/commands/doctor.test.ts index 230398a..7a508dd 100644 --- a/src/commands/doctor.test.ts +++ b/src/commands/doctor.test.ts @@ -78,6 +78,51 @@ describe('runDoctor — healthy environment', () => { expect(out).toContain('reached GET /me'); }); + it('adds a Routing check (v3) and the gap advisory when /me reports v3Enabled', async () => { + writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); + const { capture, deps } = makeCapture(); + const report = await runDoctor( + { profile: 'default', output: 'text', debug: false }, + { + ...healthyDeps(credentialsPath, { + fetchImpl: makeFetch({ ...OK_ME, v3Enabled: true }), + }), + ...deps, + }, + ); + expect(report.failures).toBe(0); + expect(report.checks.some(c => c.name === 'Routing' && c.detail.includes('v3'))).toBe(true); + expect(capture.stderr.join('\n')).toContain('[advisory]'); + expect(capture.stderr.join('\n')).toContain('test cancel'); + }); + + it('shows Routing v2 and no advisory when v3Enabled is false', async () => { + writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); + const { capture, deps } = makeCapture(); + const report = await runDoctor( + { profile: 'default', output: 'text', debug: false }, + { + ...healthyDeps(credentialsPath, { + fetchImpl: makeFetch({ ...OK_ME, v3Enabled: false }), + }), + ...deps, + }, + ); + expect(report.checks.some(c => c.name === 'Routing' && c.detail.includes('v2'))).toBe(true); + expect(capture.stderr.join('\n')).not.toContain('[advisory]'); + }); + + it('omits the Routing check when /me does not report v3Enabled', async () => { + writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); + const { deps } = makeCapture(); + const report = await runDoctor( + { profile: 'default', output: 'text', debug: false }, + { ...healthyDeps(credentialsPath), ...deps }, // OK_ME has no v3Enabled + ); + expect(report.checks.some(c => c.name === 'Routing')).toBe(false); + expect(report.warnings).toBe(0); + }); + it('never prints the API key anywhere in the report', async () => { writeProfile('default', { apiKey: 'sk-super-secret-value' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 2fa0c57..cceeb13 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -26,6 +26,7 @@ import { ApiError, CLIError, localValidationError } from '../lib/errors.js'; import type { FetchImpl } from '../lib/http.js'; import { GLOBAL_OPTS_HINT, Output, type OutputMode } from '../lib/output.js'; import { isVerifySkillInstalled } from '../lib/skill-nudge.js'; +import { emitV3RoutingAdvisory, routingLabel } from '../lib/v3-advisory.js'; import { VERSION } from '../version.js'; import { MIN_SUPPORTED_NODE_MAJOR, shouldRejectNodeVersion } from '../version-guard.js'; @@ -49,6 +50,7 @@ export interface DoctorReport { interface MeIdentity { userId?: string; keyId?: string; + v3Enabled?: boolean; } export interface DoctorDeps { @@ -81,6 +83,12 @@ export async function runDoctor(opts: CommonOptions, deps: DoctorDeps = {}): Pro }); const endpointCheck = checkEndpoint(config.apiUrl); const hasKey = Boolean(config.apiKey); + const stderr = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`)); + + const connectivity = await checkConnectivity(opts, deps, { + hasKey, + endpointOk: endpointCheck.status === 'ok', + }); const checks: DoctorCheck[] = [ { name: 'CLI version', status: 'ok', detail: VERSION }, @@ -88,19 +96,34 @@ export async function runDoctor(opts: CommonOptions, deps: DoctorDeps = {}): Pro { name: 'Profile', status: 'ok', detail: config.profile }, endpointCheck, checkCredentials(hasKey, config.profile, opts.dryRun ?? false), - await checkConnectivity(opts, deps, { - hasKey, - endpointOk: endpointCheck.status === 'ok', - }), - checkSkill(cwd, deps), + connectivity.check, ]; + // Informational routing line, only when the backend reported it (no new call). + if (connectivity.v3Enabled !== undefined) { + const label = routingLabel(connectivity.v3Enabled); + checks.push({ + name: 'Routing', + status: 'ok', + detail: + connectivity.v3Enabled === true + ? `${label} (V3 execution routing is ON)` + : `${label} (default routing)`, + }); + } + + checks.push(checkSkill(cwd, deps)); + const failures = checks.filter(check => check.status === 'fail').length; const warnings = checks.filter(check => check.status === 'warn').length; const report: DoctorReport = { checks, failures, warnings }; out.print(report, () => renderDoctor(report)); + if (connectivity.v3Enabled === true) { + emitV3RoutingAdvisory(stderr); + } + if (failures > 0) { // Non-zero exit so `testsprite doctor && ...` gates a CI step or an agent // preflight. The full report already printed above; this line is the stderr @@ -175,11 +198,13 @@ async function checkConnectivity( opts: CommonOptions, deps: DoctorDeps, ctx: { hasKey: boolean; endpointOk: boolean }, -): Promise { +): Promise<{ check: DoctorCheck; v3Enabled?: boolean }> { const name = 'Connectivity'; - if (opts.dryRun) return { name, status: 'warn', detail: 'skipped under --dry-run' }; - if (!ctx.hasKey) return { name, status: 'warn', detail: 'skipped; no API key to test with' }; - if (!ctx.endpointOk) return { name, status: 'warn', detail: 'skipped; endpoint URL is invalid' }; + if (opts.dryRun) return { check: { name, status: 'warn', detail: 'skipped under --dry-run' } }; + if (!ctx.hasKey) + return { check: { name, status: 'warn', detail: 'skipped; no API key to test with' } }; + if (!ctx.endpointOk) + return { check: { name, status: 'warn', detail: 'skipped; endpoint URL is invalid' } }; try { const client = makeHttpClient(opts, { @@ -190,7 +215,10 @@ async function checkConnectivity( }); const me = await client.get('/me'); const who = me.userId ? ` (userId ${me.userId})` : ''; - return { name, status: 'ok', detail: `reached GET /me, API key accepted${who}` }; + return { + check: { name, status: 'ok', detail: `reached GET /me, API key accepted${who}` }, + v3Enabled: me.v3Enabled, + }; } catch (error) { if (error instanceof ApiError) { if ( @@ -198,14 +226,16 @@ async function checkConnectivity( error.code === 'AUTH_INVALID' || error.code === 'AUTH_FORBIDDEN' ) { - return { name, status: 'fail', detail: `API key rejected (${error.code})` }; + return { check: { name, status: 'fail', detail: `API key rejected (${error.code})` } }; } - return { name, status: 'fail', detail: `GET /me failed (${error.code})` }; + return { check: { name, status: 'fail', detail: `GET /me failed (${error.code})` } }; } return { - name, - status: 'fail', - detail: `GET /me failed (${error instanceof Error ? error.message : String(error)})`, + check: { + name, + status: 'fail', + detail: `GET /me failed (${error instanceof Error ? error.message : String(error)})`, + }, }; } } diff --git a/src/commands/project.test.ts b/src/commands/project.test.ts index daea851..0f8c51c 100644 --- a/src/commands/project.test.ts +++ b/src/commands/project.test.ts @@ -6,11 +6,13 @@ import { ApiError } from '../lib/errors.js'; import { DRY_RUN_BANNER, resetDryRunBannerForTesting } from '../lib/client-factory.js'; import { type CliProject, + type CliDeleteProjectResponse, type CliUpdateProjectResponse, createProjectCommand, runAutoAuth, runCreate, runCredential, + runDelete, runGet, runList, runUpdate, @@ -74,10 +76,10 @@ describe('createProjectCommand', () => { errorSpy.mockRestore(); }); - it('exposes list, get, create, update, credential and auto-auth subcommands', () => { + it('exposes list, get, create, update, delete, credential and auto-auth subcommands', () => { const project = createProjectCommand(); const names = project.commands.map(c => c.name()).sort(); - expect(names).toEqual(['auto-auth', 'create', 'credential', 'get', 'list', 'update']); + expect(names).toEqual(['auto-auth', 'create', 'credential', 'delete', 'get', 'list', 'update']); }); it('list exposes the pagination flags from the design contract', () => { @@ -681,6 +683,34 @@ describe('runCreate', () => { ).rejects.toMatchObject({ exitCode: 5, code: 'VALIDATION_ERROR' }); expect(fetchImpl).not.toHaveBeenCalled(); }); + + it('rejects --description with VALIDATION_ERROR (exit 5), no network — projects have no description', async () => { + const { credentialsPath } = makeCreds(); + const fetchImpl = vi.fn(async () => { + throw new Error('should not hit network — validation must fire client-side'); + }); + + await expect( + runCreate( + { + profile: 'default', + output: 'json', + debug: false, + type: 'frontend', + name: 'Desc Project', + targetUrl: 'https://example.com', + description: 'a human description', + }, + { + credentialsPath, + fetchImpl: fetchImpl as unknown as typeof fetch, + stdout: () => {}, + stderr: () => {}, + }, + ), + ).rejects.toMatchObject({ exitCode: 5, code: 'VALIDATION_ERROR' }); + expect(fetchImpl).not.toHaveBeenCalled(); + }); }); // --------------------------------------------------------------------------- @@ -946,6 +976,140 @@ describe('runUpdate', () => { }); }); +describe('runDelete', () => { + it('refuses without --confirm and never hits the network (exit 5)', async () => { + const { credentialsPath } = makeCreds(); + let called = 0; + const fetchImpl = makeFetch(() => { + called += 1; + return { body: {} }; + }); + await expect( + runDelete( + { + profile: 'default', + output: 'json', + debug: false, + projectId: 'proj_alpha', + confirm: false, + }, + { credentialsPath, fetchImpl, stdout: () => {} }, + ), + ).rejects.toMatchObject({ + code: 'VALIDATION_ERROR', + exitCode: 5, + details: expect.objectContaining({ field: 'confirm' }), + }); + expect(called).toBe(0); + }); + + it('DELETEs /projects/{id} with a minted idempotency-key when --confirm is set', async () => { + const { credentialsPath } = makeCreds(); + const deleteResponse: CliDeleteProjectResponse = { + projectId: 'proj_alpha', + deletedAt: '2026-05-16T10:00:00.000Z', + }; + let seenUrl = ''; + let seenMethod = ''; + let seenIdemKey: string | null = null; + const fetchImpl = (async (input: Parameters[0], init: RequestInit = {}) => { + seenUrl = typeof input === 'string' ? input : (input as { url: string }).url; + seenMethod = init.method ?? 'GET'; + seenIdemKey = new Headers(init.headers).get('idempotency-key'); + return new Response(JSON.stringify(deleteResponse), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }) as typeof fetch; + + const result = await runDelete( + { + profile: 'default', + output: 'json', + debug: false, + projectId: 'proj_alpha', + confirm: true, + }, + { credentialsPath, fetchImpl, stdout: () => {}, stderr: () => {} }, + ); + + expect(seenMethod).toBe('DELETE'); + expect(seenUrl).toContain('/api/cli/v1/projects/proj_alpha'); + expect(seenIdemKey).toMatch(/^cli-delete-[0-9a-f-]{36}$/); + expect(result.projectId).toBe('proj_alpha'); + expect(result.deletedAt).toBe('2026-05-16T10:00:00.000Z'); + }); + + it('forwards a caller-supplied --idempotency-key verbatim', async () => { + const { credentialsPath } = makeCreds(); + let seenIdemKey: string | null = null; + const fetchImpl = (async (_input: Parameters[0], init: RequestInit = {}) => { + seenIdemKey = new Headers(init.headers).get('idempotency-key'); + return new Response( + JSON.stringify({ projectId: 'proj_alpha', deletedAt: '2026-05-16T10:00:00.000Z' }), + { status: 200, headers: { 'content-type': 'application/json' } }, + ); + }) as typeof fetch; + + await runDelete( + { + profile: 'default', + output: 'json', + debug: false, + projectId: 'proj_alpha', + confirm: true, + idempotencyKey: 'idem-del-001', + }, + { credentialsPath, fetchImpl, stdout: () => {}, stderr: () => {} }, + ); + + expect(seenIdemKey).toBe('idem-del-001'); + }); + + it('--dry-run bypasses --confirm and returns the canned sample without network', async () => { + resetDryRunBannerForTesting(); + const { credentialsPath } = makeCreds(); + const err: string[] = []; + // No fetchImpl → the client-factory dry-run fetch serves the samples.ts value. + const result = await runDelete( + { + profile: 'default', + output: 'json', + debug: false, + dryRun: true, + projectId: 'project_b3c91efa', + confirm: false, + }, + { credentialsPath, stdout: () => {}, stderr: line => err.push(line) }, + ); + + expect(result.projectId).toBe('project_b3c91efa'); + expect(result.deletedAt).toBe('2026-05-16T00:00:00.000Z'); + expect(err).toContain(DRY_RUN_BANNER); + }); + + it('renders text mode with projectId and deletedAt', async () => { + const { credentialsPath } = makeCreds(); + const fetchImpl = makeFetch(() => ({ + body: { projectId: 'proj_text', deletedAt: '2026-05-16T10:00:00.000Z' }, + })); + const out: string[] = []; + await runDelete( + { + profile: 'default', + output: 'text', + debug: false, + projectId: 'proj_text', + confirm: true, + }, + { credentialsPath, fetchImpl, stdout: line => out.push(line), stderr: () => {} }, + ); + const block = out.join('\n'); + expect(block).toContain('projectId proj_text'); + expect(block).toContain('deletedAt 2026-05-16T10:00:00.000Z'); + }); +}); + describe('runCredential', () => { interface Captured { url: string; diff --git a/src/commands/project.ts b/src/commands/project.ts index 96f2fc8..0a843e5 100644 --- a/src/commands/project.ts +++ b/src/commands/project.ts @@ -110,7 +110,8 @@ export interface CliCreateProjectRequest { type: 'frontend' | 'backend'; name: string; targetUrl?: string; - description?: string; + // `description` is intentionally not part of the wire request — projects have + // no description field. The `--description` flag is rejected client-side. username?: string; password?: string; instruction?: string; @@ -154,8 +155,13 @@ export async function runCreate( if (opts.name.length > 200) { throw localValidationError('--name must be at most 200 characters'); } - if (opts.description !== undefined && opts.description.length > 2000) { - throw localValidationError('--description must be at most 2000 characters'); + // `--description` is not supported on projects — no project entity stores a + // description, and the backend rejects it with a 422. Fail fast client-side + // with an actionable message instead of a wasted round trip. + if (opts.description !== undefined) { + throw localValidationError( + '--description is not supported for projects; omit it (test-level descriptions are set on `test create`)', + ); } // P2-7: guard --url against localhost/RFC1918/non-http(s) (same rules as @@ -210,7 +216,6 @@ export async function runCreate( type: opts.type, name: opts.name, ...(opts.targetUrl !== undefined ? { targetUrl: opts.targetUrl } : {}), - ...(opts.description !== undefined ? { description: opts.description } : {}), ...(opts.username !== undefined ? { username: opts.username } : {}), ...(password !== undefined ? { password } : {}), ...(opts.instruction !== undefined ? { instruction: opts.instruction } : {}), @@ -346,6 +351,80 @@ export async function runUpdate( return updated; } +// --------------------------------------------------------------------------- +// project delete +// --------------------------------------------------------------------------- + +export interface CliDeleteProjectResponse { + projectId: string; + deletedAt: string; +} + +interface DeleteOptions extends CommonOptions { + projectId: string; + /** Hard gate — required (unless `--dry-run` is set). No interactive prompts. */ + confirm: boolean; + /** Caller-supplied idempotency token; UUIDv4 minted client-side if absent. */ + idempotencyKey?: string; +} + +/** + * `project delete --confirm` — permanent cascade delete via + * DELETE /projects/{id}. + * + * The server deletes the project together with everything under it — its + * frontend/backend sub-projects, all their tests, and backend fixtures — + * matching the Portal's own delete behavior. There is no restore window. + * + * **`--confirm` is required** (unless `--dry-run`). Without either, the CLI + * exits 5 `VALIDATION_ERROR` with a typed envelope explaining the convention. + * The CLI never prompts interactively (CI-friendly contract). Re-delete on an already-deleted (or missing) project returns 404 from + * the server; the CLI surfaces the envelope as-is (exit 4), no client branching. + */ +export async function runDelete( + opts: DeleteOptions, + deps: ProjectDeps = {}, +): Promise { + assertIdempotencyKey(opts.idempotencyKey); + if (opts.projectId === undefined || opts.projectId.trim().length === 0) { + throw localValidationError(' is required'); + } + + if (!opts.confirm && !opts.dryRun) { + throw ApiError.fromEnvelope({ + error: { + code: 'VALIDATION_ERROR', + message: 'Refusing to delete without --confirm.', + nextAction: + 'This permanently deletes the project and everything under it — its ' + + 'sub-projects, all their tests, and backend fixtures (no restore window). ' + + 'The CLI convention is explicit confirmation for destructive operations. ' + + 'Re-run with --confirm. (--dry-run also works without --confirm.)', + requestId: 'local', + details: { field: 'confirm', reason: 'required for destructive operation' }, + }, + }); + } + + const idempotencyKey = opts.idempotencyKey ?? `cli-delete-${randomUUID()}`; + if (opts.idempotencyKey === undefined && (opts.output === 'json' || opts.verbose || opts.debug)) { + const stderr = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`)); + stderr(`idempotency-key: ${idempotencyKey}`); + } + + const client = makeClient(opts, deps); + const out = makeOutput(opts.output, deps); + const response = await client.delete( + `/projects/${encodeURIComponent(opts.projectId)}`, + { + headers: { 'idempotency-key': idempotencyKey }, + }, + ); + + out.print(response, data => renderDeleteText(data as CliDeleteProjectResponse)); + return response; +} + // --------------------------------------------------------------------------- // project credential — set the static backend credential // --------------------------------------------------------------------------- @@ -616,7 +695,10 @@ export function createProjectCommand(deps: ProjectDeps = {}): Command { .option('--type ', 'project type (required)') .option('--name ', 'project name (required)') .option('--url ', 'target URL (required for frontend)') - .option('--description ', 'optional human description') + .option( + '--description ', + 'not supported — projects have no description (test-level descriptions are set on `test create`)', + ) .option('--username ', 'optional auth username') .option('--password ', 'optional auth password (use --password-file for non-interactive)') .option('--password-file ', 'read password from file instead of inline flag') @@ -684,6 +766,35 @@ export function createProjectCommand(deps: ProjectDeps = {}): Command { ); }); + project + .command('delete ') + .description( + 'Permanently delete a project and everything under it (sub-projects,\n' + + 'their tests, and backend fixtures). Requires --confirm.\n' + + '\nExit codes:\n' + + ' 0 success\n' + + ' 3 auth error\n' + + ' 4 project not found (or already deleted)\n' + + ' 5 validation error (e.g., missing --confirm)', + ) + .option('--confirm', 'required: explicit confirmation for the destructive operation', false) + .option( + '--idempotency-key ', + 'opaque idempotency token. Defaults to a UUIDv4 minted per invocation.', + ) + .addHelpText('after', GLOBAL_OPTS_HINT) + .action(async (projectId: string, cmdOpts: DeleteFlagOpts, command: Command) => { + await runDelete( + { + ...resolveCommonOptions(command), + projectId, + confirm: cmdOpts.confirm === true, + idempotencyKey: cmdOpts.idempotencyKey, + }, + deps, + ); + }); + project .command('credential ') .description( @@ -808,6 +919,11 @@ interface UpdateFlagOpts { idempotencyKey?: string; } +interface DeleteFlagOpts { + confirm?: boolean; + idempotencyKey?: string; +} + interface CredentialFlagOpts { type: string; credential?: string; @@ -952,6 +1068,10 @@ function renderUpdateText(r: CliUpdateProjectResponse): string { ].join('\n'); } +function renderDeleteText(r: CliDeleteProjectResponse): string { + return [`projectId ${r.projectId}`, `deletedAt ${r.deletedAt}`].join('\n'); +} + function localValidationError(message: string): ApiError { return ApiError.fromEnvelope({ error: { diff --git a/src/commands/test.cancel.spec.ts b/src/commands/test.cancel.spec.ts new file mode 100644 index 0000000..72496b0 --- /dev/null +++ b/src/commands/test.cancel.spec.ts @@ -0,0 +1,309 @@ +/** + * Unit tests for `test cancel `. + * + * Covers: single-id happy path (text + json), `alreadyCancelled` advisory, + * multi-id mixed summary + exit precedence, 404, 409, dry-run. + */ + +import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { DRY_RUN_BANNER, resetDryRunBannerForTesting } from '../lib/client-factory.js'; +import { ApiError } from '../lib/errors.js'; +import type { CancelRunResponse } from '../lib/runs.types.js'; +import { runTestCancel, type CliCancelSummary } from './test.js'; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +type FetchInput = Parameters[0]; + +function makeFetch( + handler: (url: string, init: RequestInit) => { status?: number; body: unknown }, +): typeof globalThis.fetch { + return (async (input: FetchInput, init: RequestInit = {}) => { + const url = + typeof input === 'string' + ? input + : input instanceof URL + ? input.toString() + : (input as { url: string }).url; + const { status = 200, body } = handler(url, init); + return new Response(JSON.stringify(body), { + status, + headers: { 'content-type': 'application/json' }, + }); + }) as typeof globalThis.fetch; +} + +function makeCreds( + apiKey = 'sk-user-test', + apiUrl = 'http://localhost:13503', +): { credentialsPath: string } { + const dir = mkdtempSync(join(tmpdir(), 'cli-cancel-')); + const credentialsPath = join(dir, 'credentials'); + mkdirSync(dir, { recursive: true }); + writeFileSync(credentialsPath, `[default]\napi_url = ${apiUrl}\napi_key = ${apiKey}\n`, { + mode: 0o600, + }); + return { credentialsPath }; +} + +function makeCancelResponse( + runId: string, + overrides: Partial = {}, +): CancelRunResponse { + return { + runId, + testId: 'test_xyz', + projectId: 'project_1', + userId: 'user_1', + status: 'cancelled', + source: 'cli', + createdAt: '2026-05-15T10:00:00.000Z', + startedAt: '2026-05-15T10:00:01.000Z', + finishedAt: '2026-05-15T10:00:30.000Z', + codeVersion: 'v1', + targetUrl: 'https://example.com', + createdFrom: 'cli', + failedStepIndex: null, + failureKind: null, + error: null, + videoUrl: null, + stepSummary: { total: 5, completed: 2, passedCount: 2, failedCount: 0 }, + alreadyCancelled: false, + ...overrides, + }; +} + +function errorBody(code: string, details: Record = {}) { + const statusMap: Record = { + NOT_FOUND: 404, + CONFLICT: 409, + AUTH_FORBIDDEN: 403, + }; + return { + status: statusMap[code] ?? 400, + body: { + error: { + code, + message: `Error: ${code}`, + nextAction: 'do something', + requestId: 'req_test', + details, + }, + }, + }; +} + +/** Extract the runId embedded in a `POST /runs/{runId}/cancel` URL. */ +function runIdFromCancelUrl(url: string): string | undefined { + const match = /\/runs\/([^/]+)\/cancel/.exec(url); + return match?.[1]; +} + +// --------------------------------------------------------------------------- +// Single id — happy path +// --------------------------------------------------------------------------- + +describe('runTestCancel — single id happy path', () => { + it('CXL-1: fresh cancel of a queued/running run → 200, alreadyCancelled:false, no advisory', async () => { + const { credentialsPath } = makeCreds(); + let seenUrl = ''; + let seenMethod = ''; + const fetchImpl = makeFetch((url, init) => { + seenUrl = url; + seenMethod = init.method ?? 'GET'; + return { body: makeCancelResponse('run_abc') }; + }); + const stderrLines: string[] = []; + const result = (await runTestCancel( + { profile: 'default', output: 'json', debug: false, dryRun: false, runIds: ['run_abc'] }, + { credentialsPath, fetchImpl, stdout: () => {}, stderr: line => stderrLines.push(line) }, + )) as CancelRunResponse; + + expect(seenMethod).toBe('POST'); + expect(runIdFromCancelUrl(seenUrl)).toBe('run_abc'); + expect(result.status).toBe('cancelled'); + expect(result.alreadyCancelled).toBe(false); + expect(stderrLines.some(l => l.includes('already cancelled'))).toBe(false); + }); + + it('renders the run card in text mode with status cancelled', async () => { + const { credentialsPath } = makeCreds(); + const fetchImpl = makeFetch(() => ({ body: makeCancelResponse('run_abc') })); + const stdoutLines: string[] = []; + await runTestCancel( + { profile: 'default', output: 'text', debug: false, dryRun: false, runIds: ['run_abc'] }, + { credentialsPath, fetchImpl, stdout: line => stdoutLines.push(line), stderr: () => {} }, + ); + const block = stdoutLines.join('\n'); + expect(block).toContain('run_abc'); + expect(block).toContain('status'); + expect(block).toContain('cancelled'); + }); + + it('CXL-5: alreadyCancelled:true → [advisory] stderr line, still exit 0 (no throw)', async () => { + const { credentialsPath } = makeCreds(); + const fetchImpl = makeFetch(() => ({ + body: makeCancelResponse('run_abc', { alreadyCancelled: true }), + })); + const stderrLines: string[] = []; + const result = (await runTestCancel( + { profile: 'default', output: 'json', debug: false, dryRun: false, runIds: ['run_abc'] }, + { credentialsPath, fetchImpl, stdout: () => {}, stderr: line => stderrLines.push(line) }, + )) as CancelRunResponse; + expect(result.alreadyCancelled).toBe(true); + expect(stderrLines.some(l => l.includes('[advisory]') && l.includes('already cancelled'))).toBe( + true, + ); + }); + + it('CXL-6: unknown/cross-tenant runId → 404 propagates as ApiError exit 4', async () => { + const { credentialsPath } = makeCreds(); + const fetchImpl = makeFetch(() => errorBody('NOT_FOUND')); + const err = await runTestCancel( + { profile: 'default', output: 'json', debug: false, dryRun: false, runIds: ['run_ghost'] }, + { credentialsPath, fetchImpl, stdout: () => {}, stderr: () => {} }, + ).catch(e => e); + expect(err).toBeInstanceOf(ApiError); + expect((err as ApiError).exitCode).toBe(4); + }); + + it('CXL-4: already-terminal run → 409 propagates as ApiError exit 6', async () => { + const { credentialsPath } = makeCreds(); + const fetchImpl = makeFetch(() => errorBody('CONFLICT', { status: 'passed' })); + const err = await runTestCancel( + { profile: 'default', output: 'json', debug: false, dryRun: false, runIds: ['run_done'] }, + { credentialsPath, fetchImpl, stdout: () => {}, stderr: () => {} }, + ).catch(e => e); + expect(err).toBeInstanceOf(ApiError); + expect((err as ApiError).exitCode).toBe(6); + }); +}); + +// --------------------------------------------------------------------------- +// Multi-id — summary + exit precedence (CXL-11) +// --------------------------------------------------------------------------- + +describe('runTestCancel — multi-id summary + exit precedence (CXL-11)', () => { + it('all cancelled/alreadyCancelled → exit 0, summary buckets correct', async () => { + const { credentialsPath } = makeCreds(); + const fetchImpl = makeFetch(url => { + const runId = runIdFromCancelUrl(url)!; + if (runId === 'run_2') { + return { body: makeCancelResponse(runId, { alreadyCancelled: true }) }; + } + return { body: makeCancelResponse(runId) }; + }); + const result = (await runTestCancel( + { + profile: 'default', + output: 'json', + debug: false, + dryRun: false, + runIds: ['run_1', 'run_2'], + }, + { credentialsPath, fetchImpl, stdout: () => {}, stderr: () => {} }, + )) as CliCancelSummary; + expect(result.cancelled).toEqual(['run_1']); + expect(result.alreadyCancelled).toEqual(['run_2']); + expect(result.conflicts).toEqual([]); + expect(result.notFound).toEqual([]); + // Stable machine shape (codex finding 2): errors is ALWAYS present, + // an empty array on full success — never an absent key. + expect(result.errors).toEqual([]); + }); + + it('mixed: cancelled + conflict + notFound → notFound wins (exit 4), all buckets populated', async () => { + const { credentialsPath } = makeCreds(); + const fetchImpl = makeFetch(url => { + const runId = runIdFromCancelUrl(url)!; + if (runId === 'run_conflict') return errorBody('CONFLICT', { status: 'failed' }); + if (runId === 'run_ghost') return errorBody('NOT_FOUND'); + return { body: makeCancelResponse(runId) }; + }); + const err = await runTestCancel( + { + profile: 'default', + output: 'json', + debug: false, + dryRun: false, + runIds: ['run_ok', 'run_conflict', 'run_ghost'], + }, + { credentialsPath, fetchImpl, stdout: () => {}, stderr: () => {} }, + ).catch(e => e); + expect(err.exitCode).toBe(4); // notFound outranks conflict + expect(err.message).toContain('run_ghost'); + }); + + it('cancelled + conflict, no notFound → conflict wins (exit 6)', async () => { + const { credentialsPath } = makeCreds(); + const fetchImpl = makeFetch(url => { + const runId = runIdFromCancelUrl(url)!; + if (runId === 'run_conflict') return errorBody('CONFLICT', { status: 'blocked' }); + return { body: makeCancelResponse(runId) }; + }); + const err = await runTestCancel( + { + profile: 'default', + output: 'json', + debug: false, + dryRun: false, + runIds: ['run_ok', 'run_conflict'], + }, + { credentialsPath, fetchImpl, stdout: () => {}, stderr: () => {} }, + ).catch(e => e); + expect(err.exitCode).toBe(6); + expect(err.message).toContain('run_conflict'); + expect(err.message).toContain('blocked'); + }); + + it('JSON summary shape carries the conflicting run status', async () => { + const { credentialsPath } = makeCreds(); + const fetchImpl = makeFetch(url => { + const runId = runIdFromCancelUrl(url)!; + if (runId === 'run_conflict') return errorBody('CONFLICT', { status: 'passed' }); + return { body: makeCancelResponse(runId) }; + }); + const stdoutLines: string[] = []; + await runTestCancel( + { + profile: 'default', + output: 'json', + debug: false, + dryRun: false, + runIds: ['run_ok', 'run_conflict'], + }, + { + credentialsPath, + fetchImpl, + stdout: line => stdoutLines.push(line), + stderr: () => {}, + }, + ).catch(() => {}); + const summary = JSON.parse(stdoutLines.join('\n')) as CliCancelSummary; + expect(summary.cancelled).toEqual(['run_ok']); + expect(summary.conflicts).toEqual([{ runId: 'run_conflict', status: 'passed' }]); + }); +}); + +// --------------------------------------------------------------------------- +// Dry-run +// --------------------------------------------------------------------------- + +describe('runTestCancel — dry-run', () => { + it('single id: prints the dry-run banner and a canned cancelled envelope, no real network', async () => { + resetDryRunBannerForTesting(); + const stderrLines: string[] = []; + const result = (await runTestCancel( + { profile: 'default', output: 'json', debug: false, dryRun: true, runIds: ['run_dry'] }, + { stdout: () => {}, stderr: line => stderrLines.push(line) }, + )) as CancelRunResponse; + expect(stderrLines.some(l => l.includes(DRY_RUN_BANNER))).toBe(true); + expect(result.status).toBe('cancelled'); + expect(result.alreadyCancelled).toBe(false); + }); +}); diff --git a/src/commands/test.quickwins.spec.ts b/src/commands/test.quickwins.spec.ts index c781300..322000c 100644 --- a/src/commands/test.quickwins.spec.ts +++ b/src/commands/test.quickwins.spec.ts @@ -937,3 +937,113 @@ describe('runDeleteBatch (dogfood L1796)', () => { expect(flagNames).toContain('--status'); }); }); + +// --------------------------------------------------------------------------- +// DEV-331 (codex finding 2) — create-batch --run --wait interrupt partial +// carries the dispatched runIds, not empty placeholders +// --------------------------------------------------------------------------- + +describe('create-batch --run --wait — InterruptError partial names dispatched runIds (DEV-331)', () => { + it('interrupt mid-poll → partial rows carry the runIds recorded at trigger time', async () => { + const creds = makeCreds(); + const dir = mkdtempSync(join(tmpdir(), 'cli-dev331-cbrun-')); + for (let i = 0; i < 2; i++) { + writeFileSync( + join(dir, `plan_${i}.json`), + JSON.stringify({ ...PLAN_SPEC, name: `Plan ${i}` }), + 'utf8', + ); + } + + const { ShutdownController } = await import('../lib/interrupt.js'); + const { InterruptError } = await import('../lib/errors.js'); + const shutdown = new ShutdownController(); + + // batch create resolves; each trigger POST resolves with a per-test runId; + // every run poll hangs until the composed signal aborts. + const fetchImpl = (async (input: FetchInput, init: RequestInit = {}) => { + const url = + typeof input === 'string' + ? input + : input instanceof URL + ? input.toString() + : (input as { url: string }).url; + if (url.includes('/tests/batch') && init.method === 'POST') { + return new Response(JSON.stringify(batchCreateResp(2)), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + if (init.method === 'POST' && /\/tests\/[^/]+\/runs$/.test(url)) { + const testId = /\/tests\/([^/]+)\/runs$/.exec(url)![1]!; + return new Response( + JSON.stringify({ + runId: `run_${testId}`, + status: 'queued', + enqueuedAt: '2026-07-09T10:00:00.000Z', + codeVersion: 'v1', + targetUrl: 'https://example.com', + }), + { status: 200, headers: { 'content-type': 'application/json' } }, + ); + } + // GET /runs/{id} long-poll: hang until aborted. + return new Promise((_resolve, reject) => { + const signal = init.signal; + const rejectWithReason = (): void => { + const reason: unknown = signal?.reason; + reject(reason instanceof Error ? reason : new Error('aborted')); + }; + if (signal?.aborted) { + rejectWithReason(); + return; + } + signal?.addEventListener('abort', rejectWithReason, { once: true }); + }); + }) as typeof globalThis.fetch; + + const stdoutLines: string[] = []; + const stderrLines: string[] = []; + const pending = runCreateBatch( + { + plans: '', + planFromDir: dir, + run: true, + wait: true, + timeoutSeconds: 600, + output: 'json', + profile: 'default', + dryRun: false, + debug: false, + verbose: false, + }, + { + ...creds, + fetchImpl, + stdout: (l: string) => stdoutLines.push(l), + stderr: (l: string) => stderrLines.push(l), + sleep: () => Promise.resolve(), + shutdown, + }, + ); + setTimeout(() => shutdown.interrupt('SIGINT'), 25); + + const err = await pending.catch(e => e); + expect(err).toBeInstanceOf(InterruptError); + + // The partial must name the real runIds recorded at trigger time — + // members mid-poll have no settled result, but their runId is known. + const stdoutJson = JSON.parse(stdoutLines.join('\n')) as { + results: Array<{ testId: string; runId: string; status: string }>; + }; + const byTestId = new Map(stdoutJson.results.map(r => [r.testId, r])); + expect(byTestId.get('test_batch_0')?.runId).toBe('run_test_batch_0'); + expect(byTestId.get('test_batch_0')?.status).toBe('running'); + expect(byTestId.get('test_batch_1')?.runId).toBe('run_test_batch_1'); + + const stderrBlock = stderrLines.join('\n'); + expect(stderrBlock).toContain('billing'); + expect(stderrBlock).toContain('run_test_batch_0'); + expect(stderrBlock).toContain('run_test_batch_1'); + }); +}); diff --git a/src/commands/test.rerun.spec.ts b/src/commands/test.rerun.spec.ts index 9f16c4b..3797f14 100644 --- a/src/commands/test.rerun.spec.ts +++ b/src/commands/test.rerun.spec.ts @@ -9,7 +9,8 @@ import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { describe, expect, it } from 'vitest'; -import { ApiError, RequestTimeoutError } from '../lib/errors.js'; +import { ApiError, InterruptError, RequestTimeoutError } from '../lib/errors.js'; +import { ShutdownController } from '../lib/interrupt.js'; import type { RunResponse, RerunResponse, BatchRerunResponse } from '../lib/runs.types.js'; import type { FetchImpl } from '../lib/http.js'; import { runTestRerun, resolveWaitRequestTimeoutMs } from './test.js'; @@ -4881,3 +4882,98 @@ describe('[finding-5] batch rerun --wait: RequestTimeoutError during fan-out pol expect(parsed.accepted.every(r => r.status === 'timeout')).toBe(true); }); }); + +// --------------------------------------------------------------------------- +// DEV-331 piece 1 — graceful detach during batch rerun --wait (SIG-6) +// --------------------------------------------------------------------------- + +describe('R-BAT: batch rerun --wait — InterruptError partial lists all dispatched runIds (DEV-331)', () => { + it('interrupt mid fan-out → stdout partial covers every accepted runId, honest stderr, exit 130', async () => { + const creds = makeCreds(); + const shutdown = new ShutdownController(); + const batchResp: BatchRerunResponse = { + accepted: [ + { testId: 'test_1', runId: 'run_b1', enqueuedAt: '2026-06-03T10:00:00.000Z' }, + { testId: 'test_2', runId: 'run_b2', enqueuedAt: '2026-06-03T10:00:00.000Z' }, + ], + deferred: [], + conflicts: [], + closure: { byProject: [] }, + }; + + // Batch trigger resolves; every run poll hangs until the composed signal aborts. + const fetchImpl: FetchImpl = (async (input: unknown, init: RequestInit = {}) => { + const url = + typeof input === 'string' + ? input + : input instanceof URL + ? input.toString() + : (input as { url: string }).url; + if (url.includes('/tests/batch/rerun')) { + return new Response(JSON.stringify(batchResp), { + status: 202, + headers: { 'content-type': 'application/json' }, + }); + } + return new Promise((_resolve, reject) => { + const signal = init.signal; + const rejectWithReason = (): void => { + const reason: unknown = signal?.reason; + reject(reason instanceof Error ? reason : new Error('aborted')); + }; + if (signal?.aborted) { + rejectWithReason(); + return; + } + signal?.addEventListener('abort', rejectWithReason, { once: true }); + }); + }) as FetchImpl; + + const stdoutLines: string[] = []; + const stderrLines: string[] = []; + const pending = runTestRerun( + { + testIds: ['test_1', 'test_2'], + all: false, + wait: true, + timeoutSeconds: 600, + autoHeal: false, + autoHealExplicit: false, + skipDependencies: false, + maxConcurrency: 10, + output: 'json', + profile: 'default', + dryRun: false, + debug: false, + verbose: false, + }, + { + ...creds, + sleep: instantSleep, + fetchImpl, + stdout: line => stdoutLines.push(line), + stderr: line => stderrLines.push(line), + shutdown, + }, + ); + setTimeout(() => shutdown.interrupt('SIGINT'), 10); + + const err = await pending.catch(e => e); + expect(err).toBeInstanceOf(InterruptError); + expect((err as InterruptError).exitCode).toBe(130); + + // SIG-6: the partial lists ALL dispatched runIds, marked running. + const stdoutJson = JSON.parse(stdoutLines.join('\n')) as { + accepted: Array<{ runId: string; status: string }>; + }; + const byRunId = new Map(stdoutJson.accepted.map(r => [r.runId, r.status])); + expect(byRunId.get('run_b1')).toBe('running'); + expect(byRunId.get('run_b2')).toBe('running'); + + const stderrBlock = stderrLines.join('\n'); + expect(stderrBlock).toContain('Interrupted (SIGINT)'); + expect(stderrBlock).toContain('billing'); + expect(stderrBlock).toContain('run_b1'); + expect(stderrBlock).toContain('run_b2'); + }); +}); diff --git a/src/commands/test.run.spec.ts b/src/commands/test.run.spec.ts index c0ff083..51b527f 100644 --- a/src/commands/test.run.spec.ts +++ b/src/commands/test.run.spec.ts @@ -10,7 +10,8 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import type { Command } from 'commander'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { ApiError, RequestTimeoutError } from '../lib/errors.js'; +import { ApiError, InterruptError, RequestTimeoutError } from '../lib/errors.js'; +import { ShutdownController } from '../lib/interrupt.js'; import { DRY_RUN_BANNER, resetDryRunBannerForTesting } from '../lib/client-factory.js'; import type { FetchImpl } from '../lib/http.js'; import type { RunResponse, TriggerRunResponse, BatchRunFreshResponse } from '../lib/runs.types.js'; @@ -1699,6 +1700,88 @@ describe('C2 — backend run renders steps: n/a (backend) in text mode', () => { expect(out).toContain('steps n/a (backend)'); expect(out).not.toContain('0/0'); }); + + it('standalone BE run --wait: text probes /tests/{id} → n/a (backend); JSON never probes (DEV-282)', async () => { + // Standalone `test run ` supplies NO type hint, and the run row is + // terminal on the first poll (BE rows finalize server-side now), so + // `beFallbackUsed` stays false. In TEXT mode the card must still read + // `n/a (backend)`, resolved via a one-time `GET /tests/{id}` probe; in JSON + // mode the output is already correct and must NOT pay for that round-trip. + const { credentialsPath } = makeCreds(); + const passedBeRun: RunResponse = { + runId: 'run_282', + testId: 'be_282', + projectId: 'p1', + userId: 'u1', + status: 'passed', + source: 'cli', + createdAt: '2026-05-15T10:00:00.000Z', + startedAt: '2026-05-15T10:00:01.000Z', + finishedAt: '2026-05-15T10:00:02.000Z', + codeVersion: 'v1', + targetUrl: 'https://example.com', + createdFrom: null, + failedStepIndex: null, + failureKind: null, + error: null, + videoUrl: null, + stepSummary: { total: 0, completed: 0, passedCount: 0, failedCount: 0 }, + }; + const makeHandler = (urls: string[]) => (url: string) => { + urls.push(url); + if (url.includes('/tests/be_282/runs')) { + return { + body: { + runId: 'run_282', + status: 'queued', + enqueuedAt: '2026-05-15T10:00:00.000Z', + codeVersion: 'v1', + targetUrl: 'https://example.com', + }, + }; + } + if (url.includes('/runs/run_282')) return { body: passedBeRun }; + // Bare type probe: GET /tests/be_282 (no /runs, no /result suffix). + if (/\/tests\/be_282$/.test(url)) return { body: { id: 'be_282', type: 'backend' } }; + return { status: 404, body: {} }; + }; + const runOnce = async (output: 'text' | 'json') => { + const urls: string[] = []; + const stdoutLines: string[] = []; + await runTestRun( + { + profile: 'default', + output, + debug: false, + dryRun: false, + testId: 'be_282', + wait: true, + timeoutSeconds: 60, + }, + { + credentialsPath, + fetchImpl: makeFetch(makeHandler(urls)), + stdout: line => stdoutLines.push(line), + stderr: () => {}, + sleep: instantSleep, + }, + ); + return { urls, out: stdoutLines.join('\n') }; + }; + + // Text mode: probes the type and renders the honest backend placeholder. + const text = await runOnce('text'); + expect(text.out).toContain('steps n/a (backend)'); + expect(text.out).not.toContain('0/0'); + expect(text.urls.some(u => /\/tests\/be_282$/.test(u))).toBe(true); + + // JSON mode: no extra probe; the wire envelope ships stepSummary verbatim. + const json = await runOnce('json'); + expect(json.urls.some(u => /\/tests\/be_282$/.test(u))).toBe(false); + const parsed = JSON.parse(json.out); + expect(parsed.status).toBe('passed'); + expect(parsed.stepSummary).toEqual({ total: 0, completed: 0, passedCount: 0, failedCount: 0 }); + }); }); // --------------------------------------------------------------------------- @@ -3748,3 +3831,80 @@ describe('[finding-5] runTestRunAll --wait: RequestTimeoutError during fan-out p expect(parsed.accepted.every(r => r.status === 'timeout')).toBe(true); }); }); + +// --------------------------------------------------------------------------- +// DEV-331 piece 1 — graceful detach on SIGINT during test run --wait +// --------------------------------------------------------------------------- + +describe('runTestRun --wait — InterruptError graceful detach (DEV-331)', () => { + it('SIG-1: trigger succeeds, poll interrupted → partial to stdout + honest stderr + exit 130', async () => { + const { credentialsPath } = makeCreds(); + const shutdown = new ShutdownController(); + const stdoutLines: string[] = []; + const stderrLines: string[] = []; + + // Trigger POST resolves; the subsequent GET /runs/{id} long-poll hangs + // until the composed signal aborts (real-fetch contract). + const fetchImpl = (async (input: FetchInput, init: RequestInit = {}) => { + const url = + typeof input === 'string' + ? input + : input instanceof URL + ? input.toString() + : (input as { url: string }).url; + if (init.method === 'POST') { + return new Response(JSON.stringify(TRIGGER_RESP), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + } + void url; + return new Promise((_resolve, reject) => { + const signal = init.signal; + const rejectWithReason = (): void => { + const reason: unknown = signal?.reason; + reject(reason instanceof Error ? reason : new Error('aborted')); + }; + if (signal?.aborted) { + rejectWithReason(); + return; + } + signal?.addEventListener('abort', rejectWithReason, { once: true }); + }); + }) as typeof globalThis.fetch; + + const pending = runTestRun( + { + profile: 'default', + output: 'json', + debug: false, + dryRun: false, + testId: 'test_xyz', + wait: true, + timeoutSeconds: 600, + }, + { + credentialsPath, + fetchImpl, + stdout: line => stdoutLines.push(line), + stderr: line => stderrLines.push(line), + sleep: instantSleep, + shutdown, + }, + ); + setTimeout(() => shutdown.interrupt('SIGINT'), 5); + + const err = await pending.catch(e => e); + expect(err).toBeInstanceOf(InterruptError); + expect((err as InterruptError).exitCode).toBe(130); + + const stdoutJson = JSON.parse(stdoutLines.join('\n')) as { runId: string; status: string }; + expect(stdoutJson.runId).toBe('run_abc'); + expect(stdoutJson.status).toBe('running'); + + const stderrBlock = stderrLines.join('\n'); + expect(stderrBlock).toContain('Interrupted (SIGINT)'); + expect(stderrBlock).toContain('billing'); + expect(stderrBlock).toContain('testsprite test wait run_abc'); + }); +}); diff --git a/src/commands/test.test.ts b/src/commands/test.test.ts index f7c7602..9493908 100644 --- a/src/commands/test.test.ts +++ b/src/commands/test.test.ts @@ -120,6 +120,7 @@ describe('createTestCommand — surface', () => { const names = test.commands.map(c => c.name()).sort(); expect(names).toEqual([ 'artifact', + 'cancel', 'code', 'create', 'create-batch', @@ -834,6 +835,40 @@ describe('runGet', () => { expect(out.join('\n')).not.toContain('planSteps:'); }); + it('renders produces/consumes/category when the facade ships them', async () => { + const withDeps: CliTest = { + ...FE_TEST, + produces: ['user_id', 'order_id'], + consumes: ['session_token'], + category: 'teardown', + }; + const { credentialsPath } = makeCreds(); + const fetchImpl = makeFetch(() => ({ body: withDeps })); + const out: string[] = []; + await runGet( + { profile: 'default', output: 'text', debug: false, testId: 'test_fe' }, + { credentialsPath, fetchImpl, stdout: line => out.push(line) }, + ); + const block = out.join('\n'); + expect(block).toContain('produces: user_id, order_id'); + expect(block).toContain('consumes: session_token'); + expect(block).toContain('category: teardown'); + }); + + it('omits produces/consumes/category lines when absent', async () => { + const { credentialsPath } = makeCreds(); + const fetchImpl = makeFetch(() => ({ body: FE_TEST })); + const out: string[] = []; + await runGet( + { profile: 'default', output: 'text', debug: false, testId: 'test_fe' }, + { credentialsPath, fetchImpl, stdout: line => out.push(line) }, + ); + const block = out.join('\n'); + expect(block).not.toContain('produces:'); + expect(block).not.toContain('consumes:'); + expect(block).not.toContain('category:'); + }); + it('NOT_FOUND envelope from server propagates as ApiError exit 4', async () => { const { credentialsPath } = makeCreds(); const fetchImpl = makeFetch(() => ({ @@ -6275,6 +6310,74 @@ describe('runUpdate', () => { expect(seenBody).toEqual({ priority: 'p2' }); }); + it('threads --produces/--needs/--category into the PUT body with wire names', async () => { + const { credentialsPath } = makeCreds(); + let seenBody: unknown; + const fetchImpl = makeFetch((_url, init) => { + seenBody = init.body ? JSON.parse(init.body as string) : undefined; + return { body: SAMPLE_RESPONSE }; + }); + await runUpdate( + { + profile: 'default', + output: 'json', + debug: false, + testId: 'test_alpha', + produces: ['user_id', 'order_id'], + needs: ['session_token'], + category: 'teardown', + }, + { credentialsPath, fetchImpl, stdout: () => undefined }, + ); + expect(seenBody).toEqual({ + produces: ['user_id', 'order_id'], + consumes: ['session_token'], + category: 'teardown', + }); + }); + + it('accepts a dependency-only update (not rejected as a no-op)', async () => { + const { credentialsPath } = makeCreds(); + let seenBody: unknown; + const fetchImpl = makeFetch((_url, init) => { + seenBody = init.body ? JSON.parse(init.body as string) : undefined; + return { body: SAMPLE_RESPONSE }; + }); + await runUpdate( + { + profile: 'default', + output: 'json', + debug: false, + testId: 'test_alpha', + category: 'teardown', + }, + { credentialsPath, fetchImpl, stdout: () => undefined }, + ); + expect(seenBody).toEqual({ category: 'teardown' }); + }); + + it('omits empty dependency arrays from the body', async () => { + const { credentialsPath } = makeCreds(); + let seenBody: unknown; + const fetchImpl = makeFetch((_url, init) => { + seenBody = init.body ? JSON.parse(init.body as string) : undefined; + return { body: SAMPLE_RESPONSE }; + }); + await runUpdate( + { + profile: 'default', + output: 'json', + debug: false, + testId: 'test_alpha', + name: 'renamed', + produces: [], + needs: [], + }, + { credentialsPath, fetchImpl, stdout: () => undefined }, + ); + expect(seenBody).toEqual({ name: 'renamed' }); + }); + it('respects a caller-supplied --idempotency-key', async () => { const { credentialsPath } = makeCreds(); let seenKey: string | null = null; diff --git a/src/commands/test.ts b/src/commands/test.ts index 0602afc..837c170 100644 --- a/src/commands/test.ts +++ b/src/commands/test.ts @@ -38,10 +38,12 @@ import { import { ApiError, CLIError, + InterruptError, RequestTimeoutError, TransportError, localValidationError, } from '../lib/errors.js'; +import { globalShutdown, type ShutdownHandle } from '../lib/interrupt.js'; import { assertIdempotencyKey, requireArrayLength, @@ -75,6 +77,7 @@ import type { RunSource, BatchRunFreshResponse, BatchRunFreshAccepted, + CancelRunResponse, } from '../lib/runs.types.js'; import { RUN_SOURCES } from '../lib/runs.types.js'; import { assertNotLocal } from '../lib/target-url.js'; @@ -153,6 +156,14 @@ export interface CliTest { * no priority has been set. Text mode surfaces it only when truthy. */ priority?: string | null; + /** + * Backend-only dependency declarations that drive wave ordering. + * Optional on the wire so older facades that don't ship them still + * type-check; text mode surfaces them only when present/non-empty. + */ + produces?: string[] | null; + consumes?: string[] | null; + category?: string | null; } export type CliPublicStatus = @@ -292,6 +303,18 @@ export interface CliLatestResult { * former `{passed,failed,skipped}` count object). */ summary: string; + /** + * Captured stdout (`api_output`) from a backend-test execution. + * Present (possibly null) only for backend tests; omitted for FE/MCP and on + * older backends that omit them. Capped at 50 KB UTF-8 by the server. + */ + apiOutput?: string | null; + /** + * Python traceback from a backend-test execution (ends with the + * `ExceptionType: message` line). Present (possibly null) only for backend + * tests; omitted for FE/MCP and on older backends. + */ + trace?: string | null; /** * §6.5.1 (M2.1 piece 3) — inline failure analysis. Present when the * caller passed `--include-analysis` (`?includeAnalysis=true` on @@ -382,6 +405,14 @@ export interface CliFailureBlock { */ recommendedFixTarget: CliFixTarget | null; evidence: CliEvidence[]; + /** + * Backend-test execution artifacts, surfaced on the failure block + * so triage has stdout + traceback next to the hypothesis (mirrors + * `result.apiOutput` / `result.trace`). Present (possibly null) only for + * backend failures; omitted for FE and on older backends. + */ + apiOutput?: string | null; + trace?: string | null; } /** §6.7 wire shape — one atomic snapshot of the latest failing run. */ @@ -413,6 +444,36 @@ export interface TestDeps { * no-op in tests to avoid real delays. */ sleep?: (ms: number) => Promise; + /** + * Graceful-detach coordinator for the `--wait` paths (DEV-331 piece 1). + * Defaults to the process-wide `globalShutdown`; tests inject their own + * controller and abort it to simulate Ctrl-C deterministically (same + * pattern as `sleep` / `fetchImpl`). + */ + shutdown?: ShutdownHandle; +} + +/** The effective shutdown handle for a command invocation (DEV-331). */ +function shutdownOf(deps: TestDeps): ShutdownHandle { + return deps.shutdown ?? globalShutdown; +} + +/** + * The honest-detach stderr line (DEV-331 D1 — the heart of the ticket): + * a Ctrl-C detaches the local wait only; the server-side run keeps executing + * AND billing. Names both re-attach (`test wait`) and the real cancel + * (`test cancel`, piece 3) so the user always has a path to actually stop it. + */ +function interruptDetachMessage(err: InterruptError, runIds: string[]): string { + const subject = + runIds.length === 1 + ? `Run ${runIds[0]} is still executing on the server and will keep running (and billing) until it finishes.` + : `${runIds.length} runs are still executing on the server and will keep running (and billing) until they finish.`; + return ( + `Interrupted (${err.signal}). ${subject}\n` + + ` Re-attach with: testsprite test wait ${runIds.join(' ')}\n` + + ` Cancel with: testsprite test cancel ${runIds.join(' ')}` + ); } type CommonOptions = FactoryCommonOptions; @@ -1274,6 +1335,12 @@ interface UpdateOptions extends CommonOptions { description?: string; /** Optional new priority. Enum-validated CLI-side. */ priority?: CliCreatePriority; + /** Backend-only: variable names this test produces (repeatable --produces). */ + produces?: string[]; + /** Backend-only: variable names this test consumes (repeatable --needs); wire field `consumes`. */ + needs?: string[]; + /** Backend-only: free-text wave category (--category). */ + category?: string; /** Caller-supplied idempotency token; UUIDv4 minted client-side if absent. */ idempotencyKey?: string; } @@ -1330,11 +1397,14 @@ export async function runUpdate( const hasName = opts.name !== undefined; const hasDescription = opts.description !== undefined; const hasPriority = opts.priority !== undefined; - if (!hasName && !hasDescription && !hasPriority) { + const hasProduces = opts.produces !== undefined && opts.produces.length > 0; + const hasNeeds = opts.needs !== undefined && opts.needs.length > 0; + const hasCategory = opts.category !== undefined; + if (!hasName && !hasDescription && !hasPriority && !hasProduces && !hasNeeds && !hasCategory) { throw localValidationError( 'fields', - 'at least one of --name / --description / --priority must be set', - ['name', 'description', 'priority'], + 'at least one of --name / --description / --priority / --produces / --needs / --category must be set', + ['name', 'description', 'priority', 'produces', 'needs', 'category'], ); } @@ -1349,10 +1419,13 @@ export async function runUpdate( // is the intended wire shape — but we build the body deliberately // so the contract is auditable rather than dependent on // JSON.stringify undefined-skipping. - const body: Record = {}; + const body: Record = {}; if (hasName) body.name = opts.name!; if (hasDescription) body.description = opts.description!; if (hasPriority) body.priority = opts.priority!; + if (hasProduces) body.produces = opts.produces!; + if (hasNeeds) body.consumes = opts.needs!; + if (hasCategory) body.category = opts.category!; const client = makeClient(opts, deps); const out = makeOutput(opts.output, deps); @@ -2583,7 +2656,14 @@ async function runBatchRun( * * Returns a CliBatchRunResult. Never throws — errors are captured * into the result's `error` field so one failure doesn't abort siblings. + * (Exception: InterruptError rethrows so the collect point can print the + * partial — DEV-331.) */ + // testId → dispatched runId for members still mid-poll; the interrupt + // partial reads it because a member's runId is local to triggerOne until + // the poll settles (DEV-331, codex finding 2). + const dispatchedRunIds = new Map(); + 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()}`; @@ -2673,6 +2753,10 @@ async function runBatchRun( triggerResponse = result.body; break; // success — exit the outer retry loop } catch (err) { + // Interrupt must reject the fan-out (the collect point prints the + // partial for every spec), never flatten into a per-member outcome + // that would swallow the 128+signum exit (DEV-331). + if (err instanceof InterruptError) throw err; // RATE_LIMITED outer retry. Since the HTTP layer no longer retries // RATE_LIMITED (retryOnRateLimit: false above), every 429 reaches here // on the first attempt. @@ -2809,6 +2893,12 @@ async function runBatchRun( } } + // Record the dispatched runId (fresh trigger OR conflict-resume) so the + // interrupt partial at the collect point can name every in-flight run — + // a member's runId is otherwise local until its poll settles (DEV-331, + // codex finding 2). + if (triggerResponse.runId) dispatchedRunIds.set(testId, triggerResponse.runId); + if (!opts.wait) { // No-wait path: return the trigger response as-is. if (opts.output !== 'json') { @@ -2850,11 +2940,14 @@ async function runBatchRun( finalRun = await pollRunUntilTerminal(client, triggerResponse.runId, { timeoutSeconds: remainingSeconds, sleep: deps.sleep, + shutdown: shutdownOf(deps), onTransition: opts.verbose ? (msg: string) => stderrFn(`[batch-run][verbose] ${testId}: ${msg}`) : undefined, }); } catch (err) { + // Interrupt rejects the fan-out — see the trigger-stage catch above. + if (err instanceof InterruptError) throw err; if (err instanceof TimeoutError) { if (opts.output !== 'json') { stderrFn( @@ -2920,24 +3013,59 @@ async function runBatchRun( let nextIdx = 0; let inFlight = 0; - await new Promise((resolve, reject) => { - function startNext(): void { - while (inFlight < concurrencyLimit && nextIdx < testIds.length) { - const testId = testIds[nextIdx++]!; - inFlight++; - triggerOne(testId) - .then(result => { - batchRunResults.push(result); - inFlight--; - startNext(); - if (inFlight === 0 && nextIdx >= testIds.length) resolve(); - }) - .catch(reject); + try { + await new Promise((resolve, reject) => { + function startNext(): void { + while (inFlight < concurrencyLimit && nextIdx < testIds.length) { + const testId = testIds[nextIdx++]!; + inFlight++; + triggerOne(testId) + .then(result => { + batchRunResults.push(result); + inFlight--; + startNext(); + if (inFlight === 0 && nextIdx >= testIds.length) resolve(); + }) + .catch(reject); + } + } + startNext(); + if (testIds.length === 0) resolve(); + }); + } catch (fanOutErr) { + // Graceful detach (DEV-331): leave stdout parseable — settled members keep + // their real status, unfinished ones are marked running — then rethrow so + // index.ts exits 128+signum. + if (fanOutErr instanceof InterruptError) { + const settled = new Map(batchRunResults.map(r => [r.testId, r] as const)); + // Members mid-poll have no settled result yet — their runId comes from + // the dispatchedRunIds map recorded at trigger time (codex finding 2). + const partialResults = testIds.map( + (testId): CliBatchRunResult => + settled.get(testId) ?? { + testId, + runId: dispatchedRunIds.get(testId) ?? '', + status: dispatchedRunIds.has(testId) ? 'running' : 'not_dispatched', + codeVersion: '', + }, + ); + out.print({ results: partialResults }, () => + partialResults.map(r => `${r.testId} ${r.runId || '-'} ${r.status}`).join('\n'), + ); + const unfinished = partialResults + .filter(r => r.status === 'running' && r.runId) + .map(r => r.runId); + if (unfinished.length > 0) { + stderrFn(interruptDetachMessage(fanOutErr, unfinished)); + } else { + stderrFn( + `Interrupted (${fanOutErr.signal}). Already-triggered runs keep executing (and billing) server-side; ` + + `check them with: testsprite test list`, + ); } } - startNext(); - if (testIds.length === 0) resolve(); - }); + throw fanOutErr; + } // Sort by testId order (same as input order for stable output). batchRunResults.sort((a, b) => testIds.indexOf(a.testId) - testIds.indexOf(b.testId)); @@ -5100,6 +5228,36 @@ function renderRunResponseText( return lines.join('\n'); } +/** + * Best-effort resolve whether a finished run's test is a backend test, for the + * text run-card's step line + failure hint only (DEV-282). Returns true with no + * network call when already known (create-chain `--type`, or the BE wait + * fallback fired). Otherwise, in TEXT mode only, issues one `GET /tests/{id}`; + * any error → false (render the numeric step summary as before). JSON mode + * never probes — its envelope carries `stepSummary` verbatim and has no + * "n/a (backend)" concept, so it is already correct. + * + * This closes the standalone-path gap: `test run ` / `test wait ` + * never supply a type hint, and now that BE run rows finalize server-side + * (backend-v2.0 #551/#555) the wait fallback rarely fires — so a backend run + * card would otherwise show a misleading `steps 0/0 (passed=0, failed=0)`. + */ +async function resolveRunCardIsBackend( + client: ResultReadClient, + testId: string | undefined, + output: string, + alreadyKnown: boolean, +): Promise { + if (alreadyKnown) return true; + if (output === 'json' || !testId) return false; + try { + const test = await client.get(`/tests/${encodeURIComponent(testId)}`); + return test.type === 'backend'; + } catch { + return false; // best-effort — fall back to the numeric step summary. + } +} + /** * Render a `TriggerRunResponse` (no-wait path) to human-readable text. */ @@ -5259,7 +5417,8 @@ export async function runTestRun( stderrFn( `[advisory] Run already in flight (runId: ${currentRunId}, ` + `target: ${inFlightRun.targetUrl}). ` + - `Attaching to that run's --wait poll instead of creating a new one.`, + `Attaching to that run's --wait poll instead of creating a new one. ` + + `To stop it instead: testsprite test cancel ${currentRunId}`, ); triggerResponse = { runId: currentRunId, @@ -5285,12 +5444,16 @@ export async function runTestRun( // Auto-resume but emit a stronger advisory so the caller is aware // they are attaching to the project default. + // SIG-9 (DEV-331 final): the in-flight run is NOT auto-cancelled — + // name the real `test cancel` command instead of the old (false) + // "cancel with Ctrl-C" claim. stderrFn( `[advisory] Run already in flight (runId: ${currentRunId}` + (inFlightTargetUrl ? `, target: ${inFlightTargetUrl}` : '') + `). Auto-resuming wait on in-flight run. ` + - `If you needed a specific target URL, cancel with Ctrl-C and ` + - `re-trigger with --target-url.`, + `If you needed a specific target URL, cancel it with ` + + `testsprite test cancel ${currentRunId}, or re-trigger with ` + + `--target-url after it finishes.`, ); triggerResponse = { runId: currentRunId, @@ -5359,6 +5522,7 @@ export async function runTestRun( finalRun = await pollRunUntilTerminal(client, triggerResponse.runId, { timeoutSeconds: opts.timeoutSeconds, sleep: deps.sleep, + shutdown: shutdownOf(deps), onTransition: opts.verbose ? (msg: string) => stderrFn(`[verbose] ${msg}`) : undefined, onTick: (run, elapsedMs) => { const elapsed = Math.round(elapsedMs / 1000); @@ -5390,13 +5554,14 @@ export async function runTestRun( ]; if (p.targetUrl) lines.push(`targetUrl ${p.targetUrl}`); lines.push(`hint Re-attach with: testsprite test wait ${p.runId}`); + lines.push(`hint Cancel with: testsprite test cancel ${p.runId}`); return lines.join('\n'); }); throw ApiError.fromEnvelope({ error: { code: 'UNSUPPORTED', // exit 7 per errors.md message: `Timed out after ${opts.timeoutSeconds}s waiting for run ${triggerResponse.runId}.`, - nextAction: `Resume polling: testsprite test wait ${triggerResponse.runId}`, + nextAction: `Resume polling: testsprite test wait ${triggerResponse.runId}, or cancel it: testsprite test cancel ${triggerResponse.runId}`, requestId: 'local', details: { runId: triggerResponse.runId, timeoutSeconds: opts.timeoutSeconds }, }, @@ -5423,14 +5588,39 @@ export async function runTestRun( const lines = [`runId ${p.runId}`, `status ${p.status} (request timed out)`]; if (p.targetUrl) lines.push(`targetUrl ${p.targetUrl}`); lines.push(`hint Re-attach with: testsprite test wait ${p.runId}`); + lines.push(`hint Cancel with: testsprite test cancel ${p.runId}`); return lines.join('\n'); }); stderrFn( `Run ${triggerResponse.runId} is still in progress (request timed out). ` + - `Re-attach with: testsprite test wait ${triggerResponse.runId}`, + `Re-attach with: testsprite test wait ${triggerResponse.runId}, or cancel with: testsprite test cancel ${triggerResponse.runId}`, ); throw err; } + // Graceful detach on SIGINT/SIGTERM (DEV-331 piece 1): same partial- + // envelope shape as the timeout paths so stdout stays parseable, plus the + // honest "keeps running and billing" stderr line. Rethrow → index.ts + // renders the INTERRUPTED envelope and exits 128+signum. + if (err instanceof InterruptError) { + ticker.finalize(`Run ${triggerResponse.runId} — interrupted (${err.signal})`); + const partial = { + runId: triggerResponse.runId, + status: 'running' as const, + enqueuedAt: triggerResponse.enqueuedAt, + codeVersion: triggerResponse.codeVersion, + targetUrl: triggerResponse.targetUrl || null, + }; + printRunOrChain(out, partial, opts.createContext, data => { + const p = data as typeof partial; + const lines = [`runId ${p.runId}`, `status ${p.status} (interrupted)`]; + if (p.targetUrl) lines.push(`targetUrl ${p.targetUrl}`); + lines.push(`hint Re-attach with: testsprite test wait ${p.runId}`); + lines.push(`hint Cancel with: testsprite test cancel ${p.runId}`); + return lines.join('\n'); + }); + stderrFn(interruptDetachMessage(err, [triggerResponse.runId])); + throw err; + } ticker.finalize(); throw err; } @@ -5441,16 +5631,21 @@ export async function runTestRun( `Run ${finalRun.runId} — ${finalRun.status} (${s.completed}/${s.total} steps elapsed=${elapsed}s)`, ); + // BE detection: type hint (create-chain) OR beFallbackUsed (slow runs); on + // the standalone `test run ` path neither is set, so probe the test type + // once (text mode only, best-effort) — DEV-282. + const isBackend = await resolveRunCardIsBackend( + client, + opts.testId, + opts.output, + beFallbackUsed || opts.type === 'backend', + ); + printRunOrChain( out, withRunDashboardUrl(finalRun, resolveApiUrl(opts, deps)), opts.createContext, - data => - renderRunResponseText(data as RunResponse, { - // BE detection: type hint (create-chain) OR beFallbackUsed (slow runs). - // This ensures fast BE runs terminal on first poll still render n/a. - isBackend: beFallbackUsed || opts.type === 'backend', - }), + data => renderRunResponseText(data as RunResponse, { isBackend }), ); // Surface the trigger requestId under --verbose/--debug or JSON mode so @@ -5460,10 +5655,10 @@ export async function runTestRun( stderrFn(`requestId: ${triggerRequestId}`); if (finalRun.status === 'failed' || finalRun.status === 'blocked') { - // BE runs (resolved via the testId fallback) have no run-scoped artifact - // bundle — their failure bundle is addressed by testId, not runId. + // BE runs have no run-scoped artifact bundle — their failure bundle is + // addressed by testId, not runId. stderrFn( - beFallbackUsed + isBackend ? `Run finished with status: ${finalRun.status}. Backend failure artifacts are addressed by testId — use 'testsprite test failure get ${finalRun.testId}' to download the bundle.` : `Run finished with status: ${finalRun.status}. Use 'testsprite test artifact get ${finalRun.runId}' to download the failure bundle.`, ); @@ -5559,6 +5754,7 @@ export async function runTestWaitMany( const run = await pollRunUntilTerminal(client, runId, { timeoutSeconds: remainingSeconds, sleep: deps.sleep, + shutdown: shutdownOf(deps), onTransition: opts.verbose ? (msg: string) => stderrFn(`[verbose] ${msg}`) : undefined, onTick: (run, elapsedMs) => { const elapsed = Math.round(elapsedMs / 1000); @@ -5570,6 +5766,10 @@ export async function runTestWaitMany( } catch (err) { if (err instanceof TimeoutError) return { kind: 'timeout' }; if (err instanceof RequestTimeoutError) throw err; + // Interrupt must reject the fan-out (handled at the collect point), not + // be flattened into a per-member 'error' outcome that would swallow the + // 128+signum exit (DEV-331). + if (err instanceof InterruptError) throw err; if (err instanceof ApiError) return { kind: 'error', code: err.code, exitCode: err.exitCode }; return { kind: 'error', code: 'TRANSPORT', exitCode: 10 }; } @@ -5599,12 +5799,16 @@ export async function runTestWaitMany( if (opts.runIds.length === 0) resolve(); }); } catch (fanOutErr) { - if (fanOutErr instanceof RequestTimeoutError) { + if (fanOutErr instanceof RequestTimeoutError || fanOutErr instanceof InterruptError) { // Same contract as the batch pollers: leave stdout parseable before - // exiting 7. Members that already settled keep their real status; only + // exiting. Members that already settled keep their real status; only // the still-unfinished ids are marked running and named in the hint // (re-attaching to an already-terminal run would be a wasted command). - ticker.finalize('Multi-run wait — request timed out'); + ticker.finalize( + fanOutErr instanceof InterruptError + ? `Multi-run wait — interrupted (${fanOutErr.signal})` + : 'Multi-run wait — request timed out', + ); const partial = { results: opts.runIds.map((runId): CliMultiWaitResult => { const outcome = outcomes.get(runId); @@ -5620,7 +5824,13 @@ export async function runTestWaitMany( .filter(r => r.status === 'running' || r.status === 'timeout') .map(r => r.runId); if (unfinished.length > 0) { - stderrFn(`Re-attach with: testsprite test wait ${unfinished.join(' ')}`); + if (fanOutErr instanceof InterruptError) { + stderrFn(interruptDetachMessage(fanOutErr, unfinished)); + } else { + stderrFn( + `Re-attach with: testsprite test wait ${unfinished.join(' ')}, or cancel with: testsprite test cancel ${unfinished.join(' ')}`, + ); + } } } throw fanOutErr; @@ -5656,7 +5866,9 @@ export async function runTestWaitMany( .filter(r => r.status === 'timeout' || r.status.startsWith('error:')) .map(r => r.runId); if (unfinishedIds.length > 0) { - stderrFn(`Re-attach with: testsprite test wait ${unfinishedIds.join(' ')}`); + stderrFn( + `Re-attach with: testsprite test wait ${unfinishedIds.join(' ')}, or cancel with: testsprite test cancel ${unfinishedIds.join(' ')}`, + ); } // Worst-status exit: auth escalates (a rejected key fails every member the @@ -5684,6 +5896,190 @@ export async function runTestWaitMany( return payload; } +// --------------------------------------------------------------------------- +// DEV-331 piece 3 — `test cancel ` +// --------------------------------------------------------------------------- + +export interface RunTestCancelOptions extends CommonOptions { + runIds: string[]; +} + +/** One run's outcome in a multi-id `test cancel` summary. */ +export interface CliCancelResultRow { + runId: string; + status: 'cancelled' | 'alreadyCancelled' | 'conflict' | 'notFound' | 'error'; + /** Terminal status the run was already in, for a `conflict` row. */ + conflictStatus?: string; + error?: string; +} + +/** JSON payload for a multi-id `test cancel` — per piece-3 CXL-11. */ +export interface CliCancelSummary { + cancelled: string[]; + alreadyCancelled: string[]; + conflicts: Array<{ runId: string; status: string }>; + notFound: string[]; + /** + * Runs that errored for a reason other than 404/409 (e.g. auth, transport). + * Always present — an empty array on full success — so machine consumers + * can rely on a stable shape (DEV-331 codex finding 2). + */ + errors: Array<{ runId: string; message: string }>; +} + +/** + * `test cancel ` — DEV-331 piece 3. + * + * User-initiated cancel of one or more queued/running runs via + * `POST /api/cli/v1/runs/{runId}/cancel`. Naturally idempotent (D10): a + * repeat cancel of the same run is a 200 `alreadyCancelled` success, not an + * error. Dispatches serially (per-id volume is tiny — no batch endpoint, + * D8 "Batch-level cancel endpoint... YAGNI"). + * + * Single id: renders the returned run card (`status cancelled`); an + * `alreadyCancelled` response adds an `[advisory]` line. Exit code mirrors + * the server response directly — 0 on success (fresh or already-cancelled), + * 4 on 404 (unknown/cross-tenant), 6 on 409 (already terminal). + * + * Multi-id: prints a `{cancelled, alreadyCancelled, conflicts, notFound}` + * summary. Exit precedence (CXL-11): any `notFound` → 4 (outranks conflict — + * it signals a caller bug, wrong id/tenant); else any `conflicts` → 6; else 0. + */ +export async function runTestCancel( + opts: RunTestCancelOptions, + deps: TestDeps = {}, +): Promise { + const stderrFn = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`)); + const out = makeOutput(opts.output, deps); + const client = makeClient(opts, deps); + + if (opts.dryRun) { + emitDryRunBanner(stderrFn); + } + + if (opts.runIds.length === 1) { + const runId = opts.runIds[0]!; + const result = await client.cancelRun(runId); + out.print(result, data => { + const r = data as CancelRunResponse; + return renderRunResponseText(r); + }); + if (result.alreadyCancelled) { + stderrFn(`[advisory] run ${runId} was already cancelled`); + } + return result; + } + + const rows: CliCancelResultRow[] = []; + for (const runId of opts.runIds) { + try { + const result = await client.cancelRun(runId); + rows.push({ + runId, + status: result.alreadyCancelled ? 'alreadyCancelled' : 'cancelled', + }); + } catch (err) { + if (err instanceof ApiError && err.code === 'NOT_FOUND') { + rows.push({ runId, status: 'notFound' }); + continue; + } + if (err instanceof ApiError && err.code === 'CONFLICT') { + const conflictStatus = + err.getDetail('status', (v): v is string => typeof v === 'string') ?? 'unknown'; + rows.push({ runId, status: 'conflict', conflictStatus }); + continue; + } + const message = err instanceof Error ? err.message : String(err); + rows.push({ runId, status: 'error', error: message }); + } + } + + const errorRows = rows.filter(r => r.status === 'error'); + const summary: CliCancelSummary = { + cancelled: rows.filter(r => r.status === 'cancelled').map(r => r.runId), + alreadyCancelled: rows.filter(r => r.status === 'alreadyCancelled').map(r => r.runId), + conflicts: rows + .filter(r => r.status === 'conflict') + .map(r => ({ runId: r.runId, status: r.conflictStatus ?? 'unknown' })), + notFound: rows.filter(r => r.status === 'notFound').map(r => r.runId), + errors: errorRows.map(r => ({ runId: r.runId, message: r.error ?? 'unknown error' })), + }; + + out.print(summary, data => renderCancelSummaryText(data as CliCancelSummary)); + + const parts = [ + `${summary.cancelled.length} cancelled`, + `${summary.alreadyCancelled.length} already cancelled`, + ]; + if (summary.conflicts.length > 0) parts.push(`${summary.conflicts.length} conflict`); + if (summary.notFound.length > 0) parts.push(`${summary.notFound.length} not found`); + if (errorRows.length > 0) parts.push(`${errorRows.length} error`); + stderrFn(`Cancel summary: ${parts.join(', ')}.`); + + // Exit precedence (CXL-11): notFound outranks conflict — a caller-side bug + // (wrong id / wrong tenant) is more actionable to surface than "it already + // finished". A bare transport/auth error on any member also fails loudly + // rather than being silently absorbed into a 0 exit. + if (summary.notFound.length > 0) { + throw new CLIError( + `${summary.notFound.length} run id${summary.notFound.length !== 1 ? 's' : ''} not found: ${summary.notFound.join(' ')}`, + 4, + ); + } + if (errorRows.length > 0) { + throw new CLIError( + `${errorRows.length} cancel request${errorRows.length !== 1 ? 's' : ''} failed: ${errorRows.map(r => r.runId).join(' ')}`, + 1, + ); + } + if (summary.conflicts.length > 0) { + throw new CLIError( + `${summary.conflicts.length} run${summary.conflicts.length !== 1 ? 's' : ''} already terminal: ${summary.conflicts.map(c => `${c.runId} (${c.status})`).join(', ')}`, + 6, + ); + } + return summary; +} + +function renderCancelSummaryText(summary: CliCancelSummary): string { + const lines: string[] = []; + for (const runId of summary.cancelled) lines.push(`${runId} cancelled`); + for (const runId of summary.alreadyCancelled) lines.push(`${runId} alreadyCancelled`); + for (const c of summary.conflicts) lines.push(`${c.runId} conflict (${c.status})`); + for (const runId of summary.notFound) lines.push(`${runId} notFound`); + for (const e of summary.errors ?? []) lines.push(`${e.runId} error (${e.message})`); + return lines.join('\n'); +} + +export function createTestCancelCommand(deps: TestDeps): Command { + const cancel = new Command('cancel'); + cancel + .argument('', 'one or more run ids to cancel') + .description( + 'Cancel one or more queued/running runs.\n' + + '\nCtrl-C during --wait only detaches — it does NOT cancel the server-side\n' + + 'run. This is the real stop button. No refund is issued for the credits\n' + + 'already charged at trigger time (D3); an in-flight Lambda finishes on its\n' + + 'own and its result is discarded once cancelled.\n' + + '\nExit codes:\n' + + ' 0 cancelled (fresh or already-cancelled — naturally idempotent)\n' + + ' 4 run id not found (single id), or ANY id not found (multi-id — outranks conflict)\n' + + ' 6 run already terminal (passed/failed/blocked) — single id: 409; multi-id: any conflict\n' + + '\nMulti-id output is a summary: {cancelled, alreadyCancelled, conflicts, notFound}.', + ) + .addHelpText('after', GLOBAL_OPTS_HINT) + .action(async (runIds: string[], _cmdOpts: unknown, command: Command) => { + await runTestCancel( + { + ...resolveCommonOptions(command), + runIds, + }, + deps, + ); + }); + return cancel; +} + /** * `test wait ` — M3.3 piece-3. * @@ -5744,6 +6140,7 @@ export async function runTestWait( finalRun = await pollRunUntilTerminal(client, opts.runId, { timeoutSeconds: opts.timeoutSeconds, sleep: deps.sleep, + shutdown: shutdownOf(deps), onTransition: opts.verbose ? (msg: string) => stderrFn(`[verbose] ${msg}`) : undefined, onTick: (run, elapsedMs) => { const elapsed = Math.round(elapsedMs / 1000); @@ -5767,13 +6164,14 @@ export async function runTestWait( `runId ${p.runId}`, `status ${p.status} (timed out after ${opts.timeoutSeconds}s)`, `hint Re-attach with: testsprite test wait ${p.runId}`, + `hint Cancel with: testsprite test cancel ${p.runId}`, ].join('\n'); }); throw ApiError.fromEnvelope({ error: { code: 'UNSUPPORTED', // exit 7 per errors.md message: `Timed out after ${opts.timeoutSeconds}s waiting for run ${opts.runId}.`, - nextAction: `Resume polling: testsprite test wait ${opts.runId}`, + nextAction: `Resume polling: testsprite test wait ${opts.runId}, or cancel it: testsprite test cancel ${opts.runId}`, requestId: 'local', details: { runId: opts.runId, timeoutSeconds: opts.timeoutSeconds }, }, @@ -5791,14 +6189,31 @@ export async function runTestWait( `runId ${p.runId}`, `status ${p.status} (request timed out)`, `hint Re-attach with: testsprite test wait ${p.runId}`, + `hint Cancel with: testsprite test cancel ${p.runId}`, ].join('\n'); }); stderrFn( `Run ${opts.runId} is still in progress (request timed out). ` + - `Re-attach with: testsprite test wait ${opts.runId}`, + `Re-attach with: testsprite test wait ${opts.runId}, or cancel with: testsprite test cancel ${opts.runId}`, ); throw err; } + // Graceful detach on SIGINT/SIGTERM (DEV-331 piece 1) — see runTestRun. + if (err instanceof InterruptError) { + ticker.finalize(`Run ${opts.runId} — interrupted (${err.signal})`); + const partial = { runId: opts.runId, status: 'running' as const }; + out.print(partial, data => { + const p = data as typeof partial; + return [ + `runId ${p.runId}`, + `status ${p.status} (interrupted)`, + `hint Re-attach with: testsprite test wait ${p.runId}`, + `hint Cancel with: testsprite test cancel ${p.runId}`, + ].join('\n'); + }); + stderrFn(interruptDetachMessage(err, [opts.runId])); + throw err; + } ticker.finalize(); throw err; } @@ -5809,15 +6224,24 @@ export async function runTestWait( `Run ${finalRun.runId} — ${finalRun.status} (${s.completed}/${s.total} steps elapsed=${elapsed}s)`, ); + // `test wait` has no type hint; probe the test type once (text mode only, + // best-effort) so a backend run's card reads `n/a (backend)` — DEV-282. + const isBackend = await resolveRunCardIsBackend( + client, + finalRun.testId, + opts.output, + beFallbackUsed, + ); + out.print(withRunDashboardUrl(finalRun, resolveApiUrl(opts, deps)), data => - renderRunResponseText(data as RunResponse, { isBackend: beFallbackUsed }), + renderRunResponseText(data as RunResponse, { isBackend }), ); if (finalRun.status === 'failed' || finalRun.status === 'blocked') { - // BE runs (resolved via the testId fallback) have no run-scoped artifact - // bundle — their failure bundle is addressed by testId, not runId. + // BE runs have no run-scoped artifact bundle — their failure bundle is + // addressed by testId, not runId. stderrFn( - beFallbackUsed + isBackend ? `Run finished with status: ${finalRun.status}. Backend failure artifacts are addressed by testId — use 'testsprite test failure get ${finalRun.testId}' to download the bundle.` : `Run finished with status: ${finalRun.status}. Use 'testsprite test artifact get ${finalRun.runId}' to download the failure bundle.`, ); @@ -6276,6 +6700,7 @@ export async function runTestRunAll( const finalRun = await pollRunUntilTerminal(client, runId, { timeoutSeconds: remainingSeconds, sleep: deps.sleep, + shutdown: shutdownOf(deps), onTransition: opts.verbose ? (msg: string) => stderrFn(`[verbose] ${msg}`) : undefined, onTick: (run, elapsedMs) => { const elapsed = Math.round(elapsedMs / 1000); @@ -6305,6 +6730,10 @@ export async function runTestRunAll( }, }; } + // Interrupt must reject the fan-out (the collect point below prints the + // partial for every dispatched run), never flatten into a per-member + // outcome that would swallow the 128+signum exit (DEV-331). + if (err instanceof InterruptError) throw err; if (err instanceof RequestTimeoutError) { // Client-side per-request timeout during polling — classify as timeout // (exit 7) so the fan-out completes and stdout carries every runId. @@ -6340,24 +6769,45 @@ export async function runTestRunAll( let pollIdx = 0; let inFlight = 0; - await new Promise((resolve, reject) => { - function startNext(): void { - while (inFlight < concurrencyLimit && pollIdx < pollable.length) { - const entry = pollable[pollIdx++]!; - inFlight++; - pollFreshAccepted(entry) - .then(result => { - freshRunResults.push(result); - inFlight--; - startNext(); - if (inFlight === 0 && pollIdx >= pollable.length) resolve(); - }) - .catch(reject); + try { + await new Promise((resolve, reject) => { + function startNext(): void { + while (inFlight < concurrencyLimit && pollIdx < pollable.length) { + const entry = pollable[pollIdx++]!; + inFlight++; + pollFreshAccepted(entry) + .then(result => { + freshRunResults.push(result); + inFlight--; + startNext(); + if (inFlight === 0 && pollIdx >= pollable.length) resolve(); + }) + .catch(reject); + } } + startNext(); + if (pollable.length === 0) resolve(); + }); + } catch (fanOutErr) { + // Graceful detach (DEV-331): stdout stays parseable — settled members + // keep their real status, unfinished ones are marked running — and the + // honest stderr line names every runId still executing (and billing). + if (fanOutErr instanceof InterruptError) { + ticker.finalize(`Batch run — interrupted (${fanOutErr.signal})`); + const settled = new Map(freshRunResults.map(r => [r.runId, r] as const)); + const partialResults = pollable.map( + (e): CliBatchRunFreshResult => + settled.get(e.runId) ?? { testId: e.testId, runId: e.runId, status: 'running' }, + ); + out.print( + { accepted: partialResults, conflicts, deferred, skippedFrontend, skippedIntegration }, + () => partialResults.map(r => `${r.runId} ${r.status}`).join('\n'), + ); + const unfinished = pollable.filter(e => !settled.has(e.runId)).map(e => e.runId); + if (unfinished.length > 0) stderrFn(interruptDetachMessage(fanOutErr, unfinished)); } - startNext(); - if (pollable.length === 0) resolve(); - }); + throw fanOutErr; + } ticker.finalize(); @@ -6411,7 +6861,10 @@ export async function runTestRunAll( error: { code: 'UNSUPPORTED', message: `${timedOut} run${timedOut !== 1 ? 's' : ''} timed out.`, - nextAction: timedOutRunIds.map(rid => `Resume: testsprite test wait ${rid}`).join('\n'), + nextAction: [ + ...timedOutRunIds.map(rid => `Resume: testsprite test wait ${rid}`), + ...timedOutRunIds.map(rid => `Cancel: testsprite test cancel ${rid}`), + ].join('\n'), requestId: 'local', details: { timedOutRunIds, timeoutSeconds: opts.timeoutSeconds }, }, @@ -6799,6 +7252,7 @@ export async function runTestRerun( return await pollRunUntilTerminal(client, member.runId, { timeoutSeconds: opts.timeoutSeconds, sleep: deps.sleep, + shutdown: shutdownOf(deps), onTransition: opts.verbose ? (msg: string) => stderrFn(`[verbose] ${msg}`) : undefined, onTick: (run, elapsedMs) => { const elapsed = Math.round(elapsedMs / 1000); @@ -6894,8 +7348,35 @@ export async function runTestRerun( const reattachHints = closureMembers .map(m => `testsprite test wait ${m.runId}`) .join('\n'); + const cancelHints = closureMembers + .map(m => `testsprite test cancel ${m.runId}`) + .join('\n'); stderrFn( - `Closure members are still in progress (request timed out). Re-attach with:\n${reattachHints}`, + `Closure members are still in progress (request timed out). Re-attach with:\n${reattachHints}\n` + + `Or cancel with:\n${cancelHints}`, + ); + throw fanOutErr; + } + // Graceful detach (DEV-331): same partial shape as the timeout path — + // SIG-6 requires the partial to list ALL dispatched runIds. + if (fanOutErr instanceof InterruptError) { + ticker.finalize(`Closure fan-out — interrupted (${fanOutErr.signal})`); + const dispatchedRunIds = closureMembers.map(m => ({ + runId: m.runId, + testId: m.testId, + role: m.role, + status: 'running' as const, + })); + out.print({ runId: namedRunId, status: 'running', closure: dispatchedRunIds }, () => + dispatchedRunIds + .map(m => `${m.role.padEnd(9)} ${m.testId} (runId: ${m.runId}) — running`) + .join('\n'), + ); + stderrFn( + interruptDetachMessage( + fanOutErr, + closureMembers.map(m => m.runId), + ), ); throw fanOutErr; } @@ -6937,7 +7418,7 @@ export async function runTestRerun( error: { code: 'UNSUPPORTED', message: `Timed out after ${opts.timeoutSeconds}s waiting for rerun ${namedRunId}.`, - nextAction: `Resume polling: testsprite test wait ${namedRunId}`, + nextAction: `Resume polling: testsprite test wait ${namedRunId}, or cancel it: testsprite test cancel ${namedRunId}`, requestId: 'local', details: { runId: namedRunId, timeoutSeconds: opts.timeoutSeconds }, }, @@ -6957,7 +7438,10 @@ export async function runTestRerun( // as a whole was never observed to reach terminal. const timedOutMembers = closureFailures.filter(f => f.status === 'timeout'); if (timedOutMembers.length > 0) { - const resumeHints = timedOutMembers.map(f => `testsprite test wait ${f.runId}`).join('\n'); + const timedOutIds = timedOutMembers.map(f => f.runId); + const resumeHints = + timedOutIds.map(runId => `testsprite test wait ${runId}`).join('\n') + + `\nCancel instead: testsprite test cancel ${timedOutIds.join(' ')}`; throw ApiError.fromEnvelope({ error: { code: 'UNSUPPORTED', @@ -6995,6 +7479,7 @@ export async function runTestRerun( finalRun = await pollRunUntilTerminal(client, rerunResp.runId, { timeoutSeconds: opts.timeoutSeconds, sleep: deps.sleep, + shutdown: shutdownOf(deps), onTransition: opts.verbose ? (msg: string) => stderrFn(`[verbose] ${msg}`) : undefined, onTick: (run, elapsedMs) => { const elapsed = Math.round(elapsedMs / 1000); @@ -7012,7 +7497,7 @@ export async function runTestRerun( error: { code: 'UNSUPPORTED', message: `Timed out after ${opts.timeoutSeconds}s waiting for rerun ${rerunResp.runId}.`, - nextAction: `Resume polling: testsprite test wait ${rerunResp.runId}`, + nextAction: `Resume polling: testsprite test wait ${rerunResp.runId}, or cancel it: testsprite test cancel ${rerunResp.runId}`, requestId: 'local', details: { runId: rerunResp.runId, timeoutSeconds: opts.timeoutSeconds }, }, @@ -7029,14 +7514,31 @@ export async function runTestRerun( `runId ${p.runId}`, `status ${p.status} (request timed out)`, `hint Re-attach with: testsprite test wait ${p.runId}`, + `hint Cancel with: testsprite test cancel ${p.runId}`, ].join('\n'); }); stderrFn( `Run ${rerunResp.runId} is still in progress (request timed out). ` + - `Re-attach with: testsprite test wait ${rerunResp.runId}`, + `Re-attach with: testsprite test wait ${rerunResp.runId}, or cancel with: testsprite test cancel ${rerunResp.runId}`, ); throw err; } + // Graceful detach on SIGINT/SIGTERM (DEV-331 piece 1) — see runTestRun. + if (err instanceof InterruptError) { + ticker.finalize(`Run ${rerunResp.runId} — interrupted (${err.signal})`); + const partial = { runId: rerunResp.runId, status: 'running' as const }; + out.print(partial, data => { + const p = data as typeof partial; + return [ + `runId ${p.runId}`, + `status ${p.status} (interrupted)`, + `hint Re-attach with: testsprite test wait ${p.runId}`, + `hint Cancel with: testsprite test cancel ${p.runId}`, + ].join('\n'); + }); + stderrFn(interruptDetachMessage(err, [rerunResp.runId])); + throw err; + } ticker.finalize(); throw err; } @@ -7048,13 +7550,21 @@ export async function runTestRerun( `Run ${finalRun.runId} — ${finalRun.status} (${s.completed}/${s.total} steps replay)`, ); + // Probe the test type once (text mode only, best-effort) so a backend + // rerun's card reads `n/a (backend)` even when the fallback never fired + // (BE run rows finalize server-side now) — DEV-282. + const isBackend = await resolveRunCardIsBackend(client, testId, opts.output, beFallbackUsed); + out.print(withRunDashboardUrl(finalRun, resolveApiUrl(opts, deps)), data => - renderRunResponseText(data as RunResponse, { isBackend: beFallbackUsed }), + renderRunResponseText(data as RunResponse, { isBackend }), ); if (finalRun.status === 'failed' || finalRun.status === 'blocked') { + // BE reruns have no run-scoped artifact bundle — address by testId. stderrFn( - `Run finished with status: ${finalRun.status}. Use 'testsprite test artifact get ${finalRun.runId}' to download the failure bundle.`, + isBackend + ? `Run finished with status: ${finalRun.status}. Backend failure artifacts are addressed by testId — use 'testsprite test failure get ${testId}' to download the bundle.` + : `Run finished with status: ${finalRun.status}. Use 'testsprite test artifact get ${finalRun.runId}' to download the failure bundle.`, ); } @@ -7501,6 +8011,7 @@ export async function runTestRerun( const finalRun = await pollRunUntilTerminal(client, entry.runId, { timeoutSeconds: remainingSeconds, sleep: deps.sleep, + shutdown: shutdownOf(deps), onTransition: opts.verbose ? (msg: string) => stderrFn(`[verbose] ${msg}`) : undefined, onTick: (run, elapsedMs) => { const elapsed = Math.round(elapsedMs / 1000); @@ -7530,6 +8041,10 @@ export async function runTestRerun( }, }; } + // Interrupt must reject the fan-out (the collect point below prints the + // partial for every dispatched run), never flatten into a per-member + // outcome that would swallow the 128+signum exit (DEV-331). + if (err instanceof InterruptError) throw err; if (err instanceof RequestTimeoutError) { // Client-side per-request timeout during polling — classify as timeout // (exit 7) so the fan-out completes and stdout carries every runId. @@ -7565,24 +8080,44 @@ export async function runTestRerun( let acceptedIdx = 0; let inFlight = 0; - await new Promise((resolve, reject) => { - function startNext(): void { - while (inFlight < concurrencyLimit && acceptedIdx < accepted.length) { - const entry = accepted[acceptedIdx++]!; - inFlight++; - pollAccepted(entry) - .then(result => { - rerunResults.push(result); - inFlight--; - startNext(); - if (inFlight === 0 && acceptedIdx >= accepted.length) resolve(); - }) - .catch(reject); + try { + await new Promise((resolve, reject) => { + function startNext(): void { + while (inFlight < concurrencyLimit && acceptedIdx < accepted.length) { + const entry = accepted[acceptedIdx++]!; + inFlight++; + pollAccepted(entry) + .then(result => { + rerunResults.push(result); + inFlight--; + startNext(); + if (inFlight === 0 && acceptedIdx >= accepted.length) resolve(); + }) + .catch(reject); + } } + startNext(); + if (accepted.length === 0) resolve(); + }); + } catch (fanOutErr) { + // Graceful detach (DEV-331): stdout stays parseable — settled members + // keep their real status, unfinished ones are marked running — and the + // honest stderr line names every runId still executing (and billing). + if (fanOutErr instanceof InterruptError) { + ticker.finalize(`Batch rerun — interrupted (${fanOutErr.signal})`); + const settled = new Map(rerunResults.map(r => [r.runId, r] as const)); + const partialResults = accepted.map( + (e): CliRerunResult => + settled.get(e.runId) ?? { testId: e.testId, runId: e.runId, status: 'running' }, + ); + out.print({ accepted: partialResults, deferred, conflicts, notFound }, () => + partialResults.map(r => `${r.runId} ${r.status}`).join('\n'), + ); + const unfinished = accepted.filter(e => !settled.has(e.runId)).map(e => e.runId); + if (unfinished.length > 0) stderrFn(interruptDetachMessage(fanOutErr, unfinished)); } - startNext(); - if (accepted.length === 0) resolve(); - }); + throw fanOutErr; + } ticker.finalize(); @@ -7646,6 +8181,7 @@ export async function runTestRerun( // `test wait` accepts exactly one run id — emit one command per // timed-out run so the hint is always valid. ...(timedOut > 0 ? stillRunning.map(rid => `Resume: testsprite test wait ${rid}`) : []), + ...(timedOut > 0 ? stillRunning.map(rid => `Cancel: testsprite test cancel ${rid}`) : []), ] .filter(Boolean) .join('\n'), @@ -8312,6 +8848,22 @@ export function createTestCommand(deps: TestDeps = {}): Command { .option('--name ', 'new human-readable test name') .option('--description ', 'new human description (≤ 2000 chars)') .option('--priority ', 'new priority — one of: p0, p1, p2, p3') + .option( + '--produces ', + 'BE only: variable name this test captures (repeatable). Drives dependency-aware wave ordering.', + (val: string, prev: string[]) => [...(prev ?? []), val], + [] as string[], + ) + .option( + '--needs ', + 'BE only: variable name this test consumes (repeatable). Declares an upstream producer dependency.', + (val: string, prev: string[]) => [...(prev ?? []), val], + [] as string[], + ) + .option( + '--category ', + "BE only: test category. Use 'teardown' or 'cleanup' to mark a final-wave cleanup test.", + ) .option( '--idempotency-key ', 'opaque idempotency token (1-256 ASCII chars). Defaults to a UUIDv4 minted per invocation; pin one yourself for safe retries.', @@ -8327,6 +8879,9 @@ export function createTestCommand(deps: TestDeps = {}): Command { priority: parseEnumFlag(cmdOpts.priority, 'priority', CLI_CREATE_PRIORITIES) as | CliCreatePriority | undefined, + produces: cmdOpts.produces, + needs: cmdOpts.needs, + category: cmdOpts.category, idempotencyKey: cmdOpts.idempotencyKey, }, deps, @@ -8411,10 +8966,13 @@ export function createTestCommand(deps: TestDeps = {}): Command { ' 4 test not found\n' + ' 5 validation error (e.g., bad --target-url, or positional + --all both set)\n' + ' 6 conflict (already running — see nextAction for the active runId)\n' + - ' 7 timeout — resume with: testsprite test wait \n' + + ' 7 timeout — resume with: testsprite test wait , ' + + 'or stop it with: testsprite test cancel \n' + ' 10 transport/network failure (UNAVAILABLE) — retry the command\n' + ' 11 rate limited — honor Retry-After\n' + - '\nOn failure/blocked/cancelled, run: testsprite test artifact get ', + '\nOn failure/blocked/cancelled, run: testsprite test artifact get \n' + + '\nCtrl-C during --wait detaches only (the run keeps executing and billing);\n' + + 'stop it for real with: testsprite test cancel ', ) .option( '--target-url ', @@ -8574,7 +9132,9 @@ export function createTestCommand(deps: TestDeps = {}): Command { ' is recorded as error: in its row and folded into exit 7)\n' + ' 7 timeout or per-member poll error — resume with: testsprite test wait \n' + ' 10 transport/network failure (UNAVAILABLE) — retry the command\n' + - '\nOn failure/blocked/cancelled, run: testsprite test artifact get ', + '\nOn failure/blocked/cancelled, run: testsprite test artifact get \n' + + '\nCtrl-C detaches only (the run keeps executing and billing); stop it for\n' + + 'real with: testsprite test cancel ', ) .option('--timeout ', `max seconds to wait (1–3600, default ${DEFAULT_RUN_TIMEOUT_SECONDS})`) .option( @@ -8626,9 +9186,12 @@ export function createTestCommand(deps: TestDeps = {}): Command { ' 4 test not found\n' + ' 5 validation error\n' + ' 6 conflict (already running — see nextAction for the active runId)\n' + - ' 7 timeout or deferred — resume with: testsprite test wait \n' + + ' 7 timeout or deferred — resume with: testsprite test wait , ' + + 'or stop it with: testsprite test cancel \n' + ' 11 rate limited — honor Retry-After\n' + - '\nOn failure/blocked/cancelled, run: testsprite test artifact get ', + '\nOn failure/blocked/cancelled, run: testsprite test artifact get \n' + + '\nCtrl-C during --wait detaches only (the run keeps executing and billing);\n' + + 'stop it for real with: testsprite test cancel ', ) .option('--all', 'rerun all tests in the resolved project (requires --project)', false) .option( @@ -8802,6 +9365,7 @@ export function createTestCommand(deps: TestDeps = {}): Command { test.addCommand(createTestPlanCommand(deps)); test.addCommand(createTestFailureCommand(deps)); test.addCommand(createTestArtifactCommand(deps)); + test.addCommand(createTestCancelCommand(deps)); return test; } @@ -8937,12 +9501,20 @@ export async function runFlaky( const finalRun = await pollRunUntilTerminal(client, runId, { timeoutSeconds: opts.timeoutSeconds, sleep: deps.sleep, + shutdown: shutdownOf(deps), onTransition: opts.verbose ? (msg: string) => stderrFn(`[verbose] ${msg}`) : undefined, resolveAlternate, }); outcome = finalRun.status as FlakyOutcome; failureKind = finalRun.failureKind; } catch (err) { + // Graceful detach (DEV-331): clean up the ticker line, name the run + // still executing server-side, and let index.ts exit 128+signum. + if (err instanceof InterruptError) { + ticker.finalize(`Attempt ${i}/${opts.runs} — interrupted (${err.signal})`); + stderrFn(interruptDetachMessage(err, [runId])); + throw err; + } // A per-attempt deadline (poll TimeoutError) or a client-side request // timeout both count as a non-passing "timeout" outcome for this attempt. if (err instanceof TimeoutError || err instanceof RequestTimeoutError) { @@ -9015,6 +9587,9 @@ interface UpdateFlagOpts { name?: string; description?: string; priority?: string; + produces?: string[]; + needs?: string[]; + category?: string; idempotencyKey?: string; } @@ -9199,6 +9774,7 @@ function makeClient(opts: CommonOptions, deps: TestDeps): HttpClient { credentialsPath: deps.credentialsPath, fetchImpl: deps.fetchImpl, stderr: deps.stderr, + shutdownSignal: shutdownOf(deps).signal, }); } @@ -9506,6 +10082,16 @@ function renderTestText(t: CliTest): string { if (typeof t.planStepCount === 'number') { lines.push(`planSteps: ${t.planStepCount}`); } + // Surface backend dependency declarations when present. + if (Array.isArray(t.produces) && t.produces.length > 0) { + lines.push(`produces: ${t.produces.join(', ')}`); + } + if (Array.isArray(t.consumes) && t.consumes.length > 0) { + lines.push(`consumes: ${t.consumes.join(', ')}`); + } + if (t.category) { + lines.push(`category: ${t.category}`); + } lines.push(`createdAt: ${t.createdAt}`, `updatedAt: ${t.updatedAt}`); return lines.join('\n'); } @@ -9639,6 +10225,14 @@ function renderFailureContextText(ctx: CliFailureContext): string { lines.push(`evidence: ${ctx.failure.evidence.length} items (${breakdown})`); } if (ctx.result.videoUrl !== null) lines.push(`videoUrl: ${ctx.result.videoUrl}`); + // backend stdout + traceback (prefer the failure block, falling + // back to the embedded result; identical values). Bounded tail; full content + // in --output json / the written failure.json. + appendBackendArtifactLines( + lines, + ctx.failure.apiOutput ?? ctx.result.apiOutput, + ctx.failure.trace ?? ctx.result.trace, + ); return lines.join('\n'); } @@ -9760,6 +10354,9 @@ function renderResultText(r: CliLatestResult): string { lines.push(`summary: ${r.summary}`); if (r.videoUrl !== null) lines.push(`videoUrl: ${r.videoUrl}`); if (r.failureAnalysisUrl !== null) lines.push(`failureAnalysisUrl: ${r.failureAnalysisUrl}`); + // backend stdout + traceback (null/absent for FE, passed, and + // older backends). Full content always available via `--output json`. + appendBackendArtifactLines(lines, r.apiOutput, r.trace); if (r.analysis !== undefined) { // §6.5.1 (M2.1 piece 3) — render the inline analysis block under // the result summary. Only fires when the caller passed @@ -9783,6 +10380,44 @@ function renderResultText(r: CliLatestResult): string { * level null. Non-null wrappers render `kind=...` plus an optional * reference and indented rationale. */ +/** + * Render backend-test stdout + traceback in text mode. Shared by + * `renderResultText` and `renderFailureContextText`. Both are bounded to a + * tail (last {@link BACKEND_ARTIFACT_TAIL_LINES} lines) so a large stdout can't + * flood the terminal; the full, untruncated content is always in the + * `--output json` envelope. No-op when both are null/absent (FE / passed / + * older backends), so non-backend output stays byte-identical. + */ +const BACKEND_ARTIFACT_TAIL_LINES = 20; + +function appendArtifactTail( + lines: string[], + label: string, + value: string | null | undefined, +): void { + if (value == null || value === '') return; + const allLines = value.replace(/\n+$/, '').split('\n'); + const dropped = Math.max(0, allLines.length - BACKEND_ARTIFACT_TAIL_LINES); + const tail = dropped > 0 ? allLines.slice(-BACKEND_ARTIFACT_TAIL_LINES) : allLines; + const bytes = Buffer.byteLength(value, 'utf8'); + lines.push(''); + lines.push( + `${label} (${bytes} bytes${dropped > 0 ? `, showing last ${tail.length} lines` : ''}):`, + ); + for (const l of tail) lines.push(` ${l}`); + if (dropped > 0) + lines.push(` … ${dropped} earlier line(s) omitted — full content in --output json`); +} + +function appendBackendArtifactLines( + lines: string[], + apiOutput: string | null | undefined, + trace: string | null | undefined, +): void { + appendArtifactTail(lines, 'stdout', apiOutput); + appendArtifactTail(lines, 'trace', trace); +} + function appendFixTargetLines(lines: string[], fix: CliFixTarget | null, label: string): void { if (fix === null) { lines.push(`${label}— (analysis pipeline did not propose one)`); diff --git a/src/commands/test.wait.spec.ts b/src/commands/test.wait.spec.ts index c83195d..4830e50 100644 --- a/src/commands/test.wait.spec.ts +++ b/src/commands/test.wait.spec.ts @@ -10,7 +10,8 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { DRY_RUN_BANNER, resetDryRunBannerForTesting } from '../lib/client-factory.js'; -import { ApiError, RequestTimeoutError } from '../lib/errors.js'; +import { ApiError, InterruptError, RequestTimeoutError } from '../lib/errors.js'; +import { ShutdownController } from '../lib/interrupt.js'; import type { RunResponse } from '../lib/runs.types.js'; import { runTestWait } from './test.js'; @@ -1305,3 +1306,122 @@ describe('runTestWait — dashboardUrl on terminal output', () => { expect(printed.dashboardUrl).toBeUndefined(); }); }); + +// --------------------------------------------------------------------------- +// DEV-331 piece 1 — graceful detach on SIGINT/SIGTERM during test wait +// --------------------------------------------------------------------------- + +describe('runTestWait — InterruptError graceful detach (DEV-331)', () => { + function hangingFetch(): typeof globalThis.fetch { + return (async (_input: FetchInput, init: RequestInit = {}) => { + return new Promise((_resolve, reject) => { + const signal = init.signal; + const rejectWithReason = (): void => { + const reason: unknown = signal?.reason; + reject(reason instanceof Error ? reason : new Error('aborted')); + }; + if (signal?.aborted) { + rejectWithReason(); + return; + } + signal?.addEventListener('abort', rejectWithReason, { once: true }); + }); + }) as typeof globalThis.fetch; + } + + it('SIG-2/SIG-4: emits partial JSON to stdout, honest billing line to stderr, rethrows exit 130', async () => { + const { credentialsPath } = makeCreds(); + const shutdown = new ShutdownController(); + const stdoutLines: string[] = []; + const stderrLines: string[] = []; + + const pending = runTestWait( + { + profile: 'default', + output: 'json', + debug: false, + dryRun: false, + runId: 'run_abc', + timeoutSeconds: 600, + }, + { + credentialsPath, + fetchImpl: hangingFetch(), + stdout: line => stdoutLines.push(line), + stderr: line => stderrLines.push(line), + sleep: instantSleep, + shutdown, + }, + ); + // Simulate Ctrl-C while the long-poll fetch is in flight. + setTimeout(() => shutdown.interrupt('SIGINT'), 5); + + const err = await pending.catch(e => e); + expect(err).toBeInstanceOf(InterruptError); + expect((err as InterruptError).exitCode).toBe(130); + expect((err as InterruptError).signal).toBe('SIGINT'); + + // Stdout stays parseable and carries the runId for re-attach. + const stdoutJson = JSON.parse(stdoutLines.join('\n')) as { runId: string; status: string }; + expect(stdoutJson.runId).toBe('run_abc'); + expect(stdoutJson.status).toBe('running'); + + // The honest line: run keeps executing AND billing; re-attach hint. + const stderrBlock = stderrLines.join('\n'); + expect(stderrBlock).toContain('Interrupted (SIGINT)'); + expect(stderrBlock).toContain('billing'); + expect(stderrBlock).toContain('testsprite test wait run_abc'); + }); + + it('SIG-3: SIGTERM maps to exit 143', async () => { + const { credentialsPath } = makeCreds(); + const shutdown = new ShutdownController(); + const pending = runTestWait( + { + profile: 'default', + output: 'text', + debug: false, + dryRun: false, + runId: 'run_abc', + timeoutSeconds: 600, + }, + { + credentialsPath, + fetchImpl: hangingFetch(), + stdout: () => {}, + stderr: () => {}, + sleep: instantSleep, + shutdown, + }, + ); + setTimeout(() => shutdown.interrupt('SIGTERM'), 5); + const err = await pending.catch(e => e); + expect(err).toBeInstanceOf(InterruptError); + expect((err as InterruptError).exitCode).toBe(143); + }); + + it('arms the shutdown scope during the wait and disarms after a terminal result', async () => { + const { credentialsPath } = makeCreds(); + const shutdown = new ShutdownController(); + const fetchImpl = makeFetch(() => ({ body: makeRun('passed') })); + await runTestWait( + { + profile: 'default', + output: 'json', + debug: false, + dryRun: false, + runId: 'run_abc', + timeoutSeconds: 60, + }, + { + credentialsPath, + fetchImpl, + stdout: () => {}, + stderr: () => {}, + sleep: instantSleep, + shutdown, + }, + ); + expect(shutdown.isArmed).toBe(false); // disarmed after the poll completes + }); +}); diff --git a/src/index.ts b/src/index.ts index bf9a9aa..ae3f944 100644 --- a/src/index.ts +++ b/src/index.ts @@ -12,7 +12,7 @@ import { import { createProjectCommand } from './commands/project.js'; import { createTestCommand } from './commands/test.js'; import { createUsageCommand } from './commands/usage.js'; -import { ApiError, CLIError, RequestTimeoutError } from './lib/errors.js'; +import { ApiError, CLIError, InterruptError, RequestTimeoutError } from './lib/errors.js'; import { installBrokenPipeGuard, installSignalHandlers } from './lib/interrupt.js'; import { Output, isOutputMode } from './lib/output.js'; import { maybeInstallProxyAgent } from './lib/proxy.js'; @@ -164,9 +164,12 @@ program.hook('preAction', (_thisCommand, actionCommand) => { } }); -// Clean process lifecycle: a clear message + conventional exit code on SIGINT / -// SIGTERM / SIGHUP (instead of Node's silent abrupt kill) so an interrupted -// `test run --wait` explains the run continues server-side; plus an EPIPE guard +// Clean process lifecycle (DEV-331 piece 1, errors.md §8.1): during a `--wait` +// poll the scope is armed — the first SIGINT/SIGTERM/SIGHUP aborts gracefully +// and the wait path prints an honest partial (the run KEEPS executing and +// billing server-side) + re-attach hint before exiting 128+signum; a second +// signal hard-exits. Outside an armed scope: a clear one-line message + +// immediate exit (instead of Node's silent abrupt kill). Plus an EPIPE guard // so piping to a reader that closes early (`| head`) exits cleanly instead of // dumping a raw `write EPIPE` stack. installSignalHandlers(); @@ -209,10 +212,44 @@ try { process.stderr.write(` granted: ${(granted as string[]).join(', ')}\n`); } } + // Surface the version gap on CLIENT_TOO_OLD so the user sees exactly what + // moved without parsing the message string. (No "latest" line — the npm + // update-notice is the single source of truth for the newest release.) + if (err.code === 'CLIENT_TOO_OLD') { + const your = err.getDetail('yourVersion'); + const min = err.getDetail('minVersion'); + if (typeof your === 'string' && typeof min === 'string') { + process.stderr.write(` your version: ${your}, minimum supported: ${min}\n`); + } + } } process.exit(err.exitCode); } const output = new Output(mode); + if (err instanceof InterruptError) { + // Graceful detach (DEV-331 piece 1, errors.md §8.1): the wait-path catch + // block already printed the honest partial + re-attach hint. Exit with + // the conventional 128+signum code; `INTERRUPTED` is deliberately outside + // the error catalog. Note: Ctrl-C does NOT cancel the server-side run. + if (mode === 'json') { + const envelope = { + error: { + code: 'INTERRUPTED', + message: err.message, + nextAction: + 'The server-side run (if any) keeps executing and billing. ' + + 'Re-attach with: testsprite test wait , or stop it with: testsprite test cancel ' + + '(runId is in the partial JSON on stdout).', + requestId: 'local', + details: { signal: err.signal }, + }, + }; + process.stderr.write(`${JSON.stringify(envelope, null, 2)}\n`); + } else { + process.stderr.write(`Error: ${err.message}\n`); + } + process.exit(err.exitCode); + } if (err instanceof RequestTimeoutError) { // Structured rendering for per-request timeouts: JSON mode emits a // machine-readable envelope; text mode emits the message with a hint. diff --git a/src/lib/bundle.test.ts b/src/lib/bundle.test.ts index ff20e8c..d954381 100644 --- a/src/lib/bundle.test.ts +++ b/src/lib/bundle.test.ts @@ -601,6 +601,25 @@ describe('resolveBundleDir', () => { const out = resolveBundleDir('/tmp/x/'); expect(out).toBe('/tmp/x'); }); + + // `C:\...` is only recognized as absolute by node:path when the process + // itself is running on win32 (path.isAbsolute/resolve are platform-native, + // not path.win32.* explicitly) — these two assertions are only meaningful + // under an actual Windows runtime, hence gated rather than run everywhere. + it.runIf(process.platform === 'win32')( + 'strips a trailing backslash (native Windows path)', + () => { + const out = resolveBundleDir('C:\\Users\\me\\bundle\\'); + expect(out).toBe('C:\\Users\\me\\bundle'); + }, + ); + + it.runIf(process.platform === 'win32')( + 'preserves a bare Windows drive root instead of truncating it to a drive-relative path', + () => { + expect(resolveBundleDir('C:\\')).toBe('C:\\'); + }, + ); }); describe('streamUrlToFile retry', () => { diff --git a/src/lib/bundle.ts b/src/lib/bundle.ts index a3c0808..f44549e 100644 --- a/src/lib/bundle.ts +++ b/src/lib/bundle.ts @@ -306,6 +306,23 @@ export function applyFailedOnly(ctx: CliFailureContext): CliFailureContext { }; } +/** + * Strip trailing path separators, tolerating both `/` and `\` since a + * native Windows `--out` value (typed or pasted from Explorer) commonly + * ends in a backslash. Preserves a bare drive root (`C:\`) — stripping + * its separator would turn it into `C:`, which Windows resolves as + * "current directory on drive C", not the drive root. + */ +function stripTrailingSeparators(rawPath: string): string { + if (rawPath.length <= 1) return rawPath; + let end = rawPath.length; + while (end > 1 && (rawPath[end - 1] === '/' || rawPath[end - 1] === '\\')) { + if (end === 3 && rawPath[1] === ':' && /[A-Za-z]/.test(rawPath[0]!)) break; + end--; + } + return rawPath.slice(0, end); +} + /** * Resolve the user-supplied `--out` path into an absolute directory. * Empty strings are rejected with `VALIDATION_ERROR` for consistency @@ -325,7 +342,7 @@ export function resolveBundleDir(rawPath: string): string { }, }); } - const trimmed = rawPath.endsWith('/') ? rawPath.slice(0, -1) : rawPath; + const trimmed = stripTrailingSeparators(rawPath); return isAbsolute(trimmed) ? trimmed : resolve(process.cwd(), trimmed); } diff --git a/src/lib/client-factory.ts b/src/lib/client-factory.ts index 829410d..96d9dc0 100644 --- a/src/lib/client-factory.ts +++ b/src/lib/client-factory.ts @@ -24,8 +24,11 @@ import { REQUEST_TIMEOUT_MAX_MS, REQUEST_TIMEOUT_MIN_MS, } from './http.js'; +import { globalShutdown } from './interrupt.js'; import type { OutputMode } from './output.js'; import { createDryRunFetch } from './dry-run/fetch.js'; +import { noteServerVersion } from './version-notice.js'; +import { VERSION } from '../version.js'; export interface CommonOptions { profile: string; @@ -68,6 +71,12 @@ export interface ClientFactoryDeps { credentialsPath?: string; fetchImpl?: FetchImpl; stderr?: (line: string) => void; + /** + * Shutdown signal composed into every outgoing fetch (DEV-331 piece 1). + * Defaults to `globalShutdown.signal` so an armed SIGINT/SIGTERM aborts an + * in-flight request; tests inject their own controller's signal. + */ + shutdownSignal?: AbortSignal; } /** @@ -252,6 +261,7 @@ export function makeHttpClient(opts: CommonOptions, deps: ClientFactoryDeps = {} onDebug: opts.debug ? (event: DebugEvent) => stderr(formatDryRunDebug(event)) : undefined, onTransition: opts.verbose ? (msg: string) => stderr(`[verbose] ${msg}`) : undefined, requestTimeoutMs, + shutdownSignal: deps.shutdownSignal ?? globalShutdown.signal, }); } @@ -274,7 +284,20 @@ export function makeHttpClient(opts: CommonOptions, deps: ClientFactoryDeps = {} fetchImpl: deps.fetchImpl, onDebug: opts.debug ? (event: DebugEvent) => stderr(formatDebug(event)) : undefined, onTransition: opts.verbose ? (msg: string) => stderr(`[verbose] ${msg}`) : undefined, + // Warn once if the backend advertises a minimum supported version above + // this binary's. Gating (opt-out env, TTY, output mode, dry-run) lives in + // noteServerVersion; the client just forwards the observed headers. + onServerVersion: info => + noteServerVersion(info, { + currentVersion: VERSION, + env, + isTTY: process.stderr.isTTY === true, + outputMode: opts.output, + dryRun: opts.dryRun, + stderr, + }), requestTimeoutMs, + shutdownSignal: deps.shutdownSignal ?? globalShutdown.signal, }); } diff --git a/src/lib/dry-run/samples.test.ts b/src/lib/dry-run/samples.test.ts index a1838d6..078c653 100644 --- a/src/lib/dry-run/samples.test.ts +++ b/src/lib/dry-run/samples.test.ts @@ -333,6 +333,23 @@ describe('findSample', () => { updatedAt: expect.any(String), }); break; + case 'cancelRun': + // DEV-331 piece 3 — POST /runs/{runId}/cancel → CancelRunResponse + // (RunResponse shape + alreadyCancelled). + expect(body).toMatchObject({ + runId: expect.any(String), + testId: expect.any(String), + status: 'cancelled', + alreadyCancelled: false, + }); + break; + case 'deleteProject': + // DELETE /projects/{id} → CliDeleteProjectResponse shape. + expect(body).toMatchObject({ + projectId: expect.any(String), + deletedAt: expect.any(String), + }); + break; default: throw new Error(`Unexpected operationId in samples: ${e.operationId}`); } @@ -423,6 +440,22 @@ describe('findSample', () => { expect(body.stepSummary.failedCount).toBe(0); }); + // DEV-331 piece 3: POST /runs/{runId}/cancel must resolve to `cancelRun`, + // never fall through to the GET-only `getRun` entry despite sharing the + // `/runs/{runId}` path prefix — findSample filters by method first. + it('POST /runs/{runId}/cancel resolves cancelRun (not getRun)', () => { + const e = findSample('POST', 'https://api.testsprite.com/api/cli/v1/runs/run_xyz/cancel'); + expect(e?.operationId).toBe('cancelRun'); + const body = e?.body() as { status: string; alreadyCancelled: boolean; runId: string }; + expect(body.status).toBe('cancelled'); + expect(body.alreadyCancelled).toBe(false); + }); + + it('GET /runs/{runId}/cancel has no sample (cancel is POST-only)', () => { + const e = findSample('GET', 'https://api.testsprite.com/api/cli/v1/runs/run_xyz/cancel'); + expect(e).toBeUndefined(); + }); + it('getTest sample carries priority field (G1a)', () => { const e = findSample('GET', 'https://api.testsprite.com/api/cli/v1/tests/test_abc'); expect(e?.operationId).toBe('getTest'); diff --git a/src/lib/dry-run/samples.ts b/src/lib/dry-run/samples.ts index 4c7cbb0..f6f5ec2 100644 --- a/src/lib/dry-run/samples.ts +++ b/src/lib/dry-run/samples.ts @@ -16,7 +16,11 @@ * If the CLI OpenAPI spec changes, both this file AND * `test/mock-backend/fixtures.ts` must be updated in the same PR. */ -import type { CliProject, CliUpdateProjectResponse } from '../../commands/project.js'; +import type { + CliProject, + CliUpdateProjectResponse, + CliDeleteProjectResponse, +} from '../../commands/project.js'; import type { CliBulkDeleteSummary, CliFailureContext, @@ -36,6 +40,7 @@ import type { BatchRerunResponse, BatchRunFreshResponse, ListRunsResponse, + CancelRunResponse, } from '../runs.types.js'; const SAMPLE_USER_ID = '11111111-1111-4111-8111-111111111111'; @@ -104,6 +109,7 @@ const me: MeResponse = { keyId: SAMPLE_KEY_ID, scopes: ['read:projects', 'read:tests', 'write:tests', 'run:tests'], env: 'development', + v3Enabled: true, }; const projects: CliProject[] = [ @@ -397,6 +403,11 @@ const ENTRIES: DryRunSampleEntry[] = [ updatedFields: ['name'], updatedAt: '2026-05-16T00:00:00.000Z', } satisfies CliUpdateProjectResponse), + // DELETE /projects/{id} (cascade delete project + tests + fixtures). + entry('deleteProject', 'DELETE', '/projects/{projectId}', { + projectId: SAMPLE_PROJECT_ID, + deletedAt: '2026-05-16T00:00:00.000Z', + } satisfies CliDeleteProjectResponse), entry('listTests', 'GET', '/tests', pageOf(tests)), entry('getTestCode', 'GET', '/tests/{testId}/code', testCode), entry('listTestSteps', 'GET', '/tests/{testId}/steps', pageOf(testSteps)), @@ -747,6 +758,36 @@ const ENTRIES: DryRunSampleEntry[] = [ }, ], } satisfies RunResponse), + // DEV-331 piece 3 — POST /runs/{runId}/cancel. Method-guarded in + // `findSample` (POST vs `getRun`'s GET), so this can't be shadowed by the + // broader `/runs/{runId}` pattern above despite sharing its path prefix. + // `alreadyCancelled: false` — a fresh cancel is the more instructive shape + // for a dry-run learner than the idempotent no-op. + entry('cancelRun', 'POST', '/runs/{runId}/cancel', { + runId: SAMPLE_RUN_ID, + testId: SAMPLE_TEST_ID_PASSED, + projectId: SAMPLE_PROJECT_ID, + userId: SAMPLE_USER_ID, + status: 'cancelled', + source: 'cli', + createdAt: '2026-05-15T19:32:00.000Z', + startedAt: '2026-05-15T19:32:05.000Z', + finishedAt: '2026-05-15T19:33:12.000Z', + codeVersion: 'v1', + targetUrl: SAMPLE_TARGET_URL, + createdFrom: null, + failedStepIndex: null, + failureKind: null, + error: null, + videoUrl: null, + stepSummary: { + total: 8, + completed: 3, + passedCount: 3, + failedCount: 0, + }, + alreadyCancelled: false, + } satisfies CancelRunResponse), ]; function entry( diff --git a/src/lib/errors.test.ts b/src/lib/errors.test.ts index aa3926b..5c5ba66 100644 --- a/src/lib/errors.test.ts +++ b/src/lib/errors.test.ts @@ -59,6 +59,8 @@ describe('exitCodeFor', () => { ['UNAVAILABLE', 10], ['RATE_LIMITED', 11], ['INSUFFICIENT_CREDITS', 12], + ['FEATURE_GATED', 13], + ['CLIENT_TOO_OLD', 14], ['INTERNAL', 1], ] as const)('%s → exit %d', (code, expected) => { expect(exitCodeFor(code)).toBe(expected); @@ -138,6 +140,7 @@ describe('ApiError.fromEnvelope status fallback', () => { [403, 'AUTH_FORBIDDEN' as const], [404, 'NOT_FOUND' as const], [409, 'CONFLICT' as const], + [426, 'CLIENT_TOO_OLD' as const], [429, 'RATE_LIMITED' as const], [501, 'UNSUPPORTED' as const], [503, 'UNAVAILABLE' as const], @@ -157,6 +160,25 @@ describe('ApiError.fromEnvelope status fallback', () => { expect(err.exitCode).toBe(10); }); + it('recognizes a well-formed CLIENT_TOO_OLD (426) envelope and echoes the version details', () => { + const err = ApiError.fromEnvelope( + { + error: { + code: 'CLIENT_TOO_OLD', + message: 'Your CLI is older than the minimum supported version.', + nextAction: 'Upgrade with npm i -g @testsprite/testsprite-cli@latest.', + requestId: 'req_old', + details: { minVersion: '1.0.0', yourVersion: '0.9.0' }, + }, + }, + 426, + ); + // The code is in the union, so it is trusted directly — NOT remapped. + expect(err.code).toBe('CLIENT_TOO_OLD'); + expect(err.exitCode).toBe(14); + expect(err.details).toEqual({ minVersion: '1.0.0', yourVersion: '0.9.0' }); + }); + // Track A dogfood: NestJS raw 404 (route not registered) has the // shape `{ message, error: "Not Found", statusCode }`. Previously the // parser saw `obj.error = "Not Found"` (a string) and fell through to diff --git a/src/lib/errors.ts b/src/lib/errors.ts index f64d8e2..40b6c05 100644 --- a/src/lib/errors.ts +++ b/src/lib/errors.ts @@ -45,6 +45,12 @@ export const ERROR_CODES = [ // so the CLI can drop the client-side detection heuristic. 'FEATURE_GATED', 'UNSUPPORTED', + // The running CLI is older than the backend's minimum supported version. + // Backend emits this as HTTP 426 when version enforcement is enabled. Exit + // 14, non-retriable — a too-old client cannot self-heal by retrying; the + // user must upgrade. `nextAction` carries the upgrade instruction and + // `details` echoes { minVersion, latestVersion, yourVersion }. + 'CLIENT_TOO_OLD', 'INTERNAL', 'UNAVAILABLE', ] as const; @@ -77,6 +83,8 @@ export function exitCodeFor(code: ErrorCode): number { return 6; case 'UNSUPPORTED': return 7; + case 'CLIENT_TOO_OLD': + return 14; case 'UNAVAILABLE': return 10; case 'RATE_LIMITED': @@ -100,6 +108,41 @@ export class CLIError extends Error { } } +/** + * Termination signals with graceful handling, mapped to their conventional + * `128 + signum` exit code. sourceRef: POSIX signal numbers (SIGHUP=1, + * SIGINT=2, SIGTERM=15). Lives here (not interrupt.ts) so `InterruptError` + * needs no import cycle; `interrupt.ts` re-exports for back-compat. + */ +export const TERMINATION_EXIT_CODES = { + SIGINT: 130, // 128 + 2 + SIGTERM: 143, // 128 + 15 + SIGHUP: 129, // 128 + 1 +} as const; + +export type TerminationSignal = keyof typeof TERMINATION_EXIT_CODES; + +/** + * User-initiated interrupt (SIGINT/SIGTERM/SIGHUP) observed while a + * graceful-detach scope (the `--wait` polling window) was armed — see + * `interrupt.ts::ShutdownController`. The `--wait` catch blocks render the + * honest partial envelope + re-attach hint, then rethrow to `index.ts`. + * + * Deliberately NOT in `ERROR_CODES` — on a signal the CLI + * exits 130/143 without consulting the error catalog. The JSON-mode stderr + * envelope uses the out-of-catalog code `"INTERRUPTED"`. Same client-local + * synthetic category as `RequestTimeoutError`. + */ +export class InterruptError extends CLIError { + readonly signal: TerminationSignal; + + constructor(signal: TerminationSignal) { + super(`Interrupted by ${signal}.`, TERMINATION_EXIT_CODES[signal]); + this.name = 'InterruptError'; + this.signal = signal; + } +} + export class NotImplementedError extends CLIError { constructor(commandPath: string) { super(`Command not yet implemented: ${commandPath}`, 2); @@ -314,6 +357,8 @@ function codeFromHttpStatus(status: number | undefined): ErrorCode { return 'PRECONDITION_FAILED'; case 413: return 'PAYLOAD_TOO_LARGE'; + case 426: + return 'CLIENT_TOO_OLD'; case 429: return 'RATE_LIMITED'; case 501: diff --git a/src/lib/failing-fe-resolver.spec.ts b/src/lib/failing-fe-resolver.spec.ts index d6524e7..1fe05a9 100644 --- a/src/lib/failing-fe-resolver.spec.ts +++ b/src/lib/failing-fe-resolver.spec.ts @@ -43,6 +43,40 @@ function makeItem( return { id, type, status, updatedAt }; } +/** + * URL-routing fetch stub for the preferredId direct-probe path: + * `GET /tests/{id}` (no query string) is answered from `preferred`; + * `GET /tests?...` list calls are answered from `pages` in order. + * Call counts are exposed so tests can assert which endpoints were hit. + */ +function makeRoutedFetch(opts: { + preferred?: { status: number; body: unknown }; + preferredThrows?: boolean; + pages: Array<{ items: TestListItem[]; nextToken: string | null }>; +}) { + let listCalls = 0; + let preferredCalls = 0; + const impl = (async (url: string | URL | Request) => { + const u = String(url); + if (/\/tests\/[^/?]+$/.test(u)) { + preferredCalls++; + if (opts.preferredThrows) throw new Error('ECONNRESET (direct probe)'); + const p = opts.preferred ?? { status: 404, body: { error: 'not found' } }; + return new Response(JSON.stringify(p.body), { + status: p.status, + headers: { 'Content-Type': 'application/json' }, + }); + } + const page = opts.pages[listCalls] ?? { items: [], nextToken: null }; + listCalls++; + return new Response(JSON.stringify(page), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }) as typeof fetch; + return { impl, counts: () => ({ listCalls, preferredCalls }) }; +} + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -180,4 +214,163 @@ describe('resolveFailingFrontendTestId', () => { expect(result.testId).toBe('test_abc'); expect(result.reason).toContain('2026-05-27T09:00:00Z'); }); + + // DEV-393: dedicated pinned fixture must win over a fresher incidental + // failure — live-reproduced 2026-07-16 (a passingFrontendTestId fresh-run + // flip immediately outranked a dedicated "always red" fixture by + // updatedAt). See ResolverOptions.preferredId doc comment. + describe('preferredId (DEV-393 pinned-fixture preference)', () => { + test('prefers the pinned id over a fresher candidate when the direct probe is non-ok', async () => { + // Probe answers HTTP 500 (not ok, no throw) → falls through to the list + // scan, where the in-candidates preference must still pick the pinned id. + const routed = makeRoutedFetch({ + preferred: { status: 500, body: { error: 'internal' } }, + pages: [ + { + items: [ + makeItem('test_pinned', '2026-05-01T00:00:00Z'), + makeItem('test_incidental', '2026-05-20T00:00:00Z'), + ], + nextToken: null, + }, + ], + }); + const result = await resolveFailingFrontendTestId({ + ...BASE_OPTS, + fetchImpl: routed.impl, + preferredId: 'test_pinned', + }); + expect(result.testId).toBe('test_pinned'); + expect(result.reason).toContain('Preferred pinned'); + expect(routed.counts()).toEqual({ listCalls: 1, preferredCalls: 1 }); + }); + + test('falls back to freshest when the pinned id is not in the failed set', async () => { + // Probe returns 200 with a malformed body (no type/status fields) → + // falls through; pinned is absent from the list → freshest wins. + const routed = makeRoutedFetch({ + preferred: { status: 200, body: {} }, + pages: [{ items: [makeItem('test_incidental', '2026-05-20T00:00:00Z')], nextToken: null }], + }); + const result = await resolveFailingFrontendTestId({ + ...BASE_OPTS, + fetchImpl: routed.impl, + preferredId: 'test_pinned_but_now_passing', + }); + expect(result.testId).toBe('test_incidental'); + }); + + test('behaves exactly as before when preferredId is not supplied', async () => { + const older = makeItem('test_old', '2026-05-01T00:00:00Z'); + const newer = makeItem('test_new', '2026-05-20T00:00:00Z'); + const fetchImpl = makeFetchReturning([{ items: [older, newer], nextToken: null }]); + const result = await resolveFailingFrontendTestId({ + ...BASE_OPTS, + fetchImpl: fetchImpl as typeof fetch, + }); + expect(result.testId).toBe('test_new'); + }); + + // Codex F3: the direct probe makes the preference immune to the maxPages + // list-scan cap — a pinned fixture beyond the last scanned page must + // still win, without any list call at all on the happy path. + test('direct probe wins without any list call when the pinned test is failed', async () => { + const routed = makeRoutedFetch({ + preferred: { + status: 200, + body: { + ...makeItem('test_pinned', '2026-05-01T00:00:00Z'), + projectId: BASE_OPTS.projectId, + }, + }, + pages: [{ items: [makeItem('test_incidental', '2026-05-20T00:00:00Z')], nextToken: null }], + }); + const result = await resolveFailingFrontendTestId({ + ...BASE_OPTS, + fetchImpl: routed.impl, + preferredId: 'test_pinned', + maxPages: 1, + }); + expect(result.testId).toBe('test_pinned'); + expect(result.reason).toContain('direct GET'); + expect(routed.counts()).toEqual({ listCalls: 0, preferredCalls: 1 }); + }); + + test('direct probe on a now-passing pinned test falls through to freshest', async () => { + const routed = makeRoutedFetch({ + preferred: { + status: 200, + body: { + ...makeItem('test_pinned', '2026-05-25T00:00:00Z', 'frontend', 'passed'), + projectId: BASE_OPTS.projectId, + }, + }, + pages: [{ items: [makeItem('test_incidental', '2026-05-20T00:00:00Z')], nextToken: null }], + }); + const result = await resolveFailingFrontendTestId({ + ...BASE_OPTS, + fetchImpl: routed.impl, + preferredId: 'test_pinned', + }); + expect(result.testId).toBe('test_incidental'); + expect(routed.counts()).toEqual({ listCalls: 1, preferredCalls: 1 }); + }); + + test('direct probe error falls through to the list scan, where the pinned id still wins', async () => { + const routed = makeRoutedFetch({ + preferredThrows: true, + pages: [ + { + items: [ + makeItem('test_pinned', '2026-05-01T00:00:00Z'), + makeItem('test_incidental', '2026-05-20T00:00:00Z'), + ], + nextToken: null, + }, + ], + }); + const result = await resolveFailingFrontendTestId({ + ...BASE_OPTS, + fetchImpl: routed.impl, + preferredId: 'test_pinned', + }); + expect(result.testId).toBe('test_pinned'); + expect(result.reason).toContain('Preferred pinned'); + expect(routed.counts()).toEqual({ listCalls: 1, preferredCalls: 1 }); + }); + + test('direct probe on a pinned test from a DIFFERENT project falls through (codex round 2)', async () => { + // The pin points at a genuinely failed FE test — but in another + // project. The old list scan (projectId-filtered server-side) would + // never have returned it, so the direct probe must not let it win. + const routed = makeRoutedFetch({ + preferred: { + status: 200, + body: { ...makeItem('test_pinned', '2026-05-01T00:00:00Z'), projectId: 'proj_OTHER' }, + }, + pages: [{ items: [makeItem('test_incidental', '2026-05-20T00:00:00Z')], nextToken: null }], + }); + const result = await resolveFailingFrontendTestId({ + ...BASE_OPTS, + fetchImpl: routed.impl, + preferredId: 'test_pinned', + }); + expect(result.testId).toBe('test_incidental'); + expect(routed.counts()).toEqual({ listCalls: 1, preferredCalls: 1 }); + }); + + test('direct probe 404 falls through and freshest wins when pinned is absent everywhere', async () => { + const routed = makeRoutedFetch({ + preferred: { status: 404, body: { error: 'not found' } }, + pages: [{ items: [makeItem('test_incidental', '2026-05-20T00:00:00Z')], nextToken: null }], + }); + const result = await resolveFailingFrontendTestId({ + ...BASE_OPTS, + fetchImpl: routed.impl, + preferredId: 'test_pinned_deleted', + }); + expect(result.testId).toBe('test_incidental'); + expect(routed.counts()).toEqual({ listCalls: 1, preferredCalls: 1 }); + }); + }); }); diff --git a/src/lib/failing-fe-resolver.ts b/src/lib/failing-fe-resolver.ts index 14df49f..f8d6b5a 100644 --- a/src/lib/failing-fe-resolver.ts +++ b/src/lib/failing-fe-resolver.ts @@ -50,6 +50,21 @@ export interface ResolverOptions { * Each page uses pageSize=50, so 5 pages = 250 tests scanned. */ maxPages?: number; + /** + * DEV-393: the operator's pinned `failingFrontendTestId` (the static value + * from fixtures.local.json). "Freshest failed" alone is fragile in a shared + * project: any OTHER FE test that fails more recently than a dedicated, + * permanently-red fixture silently steals the slot (live-reproduced + * 2026-07-16 — a `passingFrontendTestId` fresh-run flip immediately + * outranked the dedicated fixture on `updatedAt`). When set, the pinned id + * is probed directly first (one GET /tests/{id} — immune to the maxPages + * list-scan cap); if it is currently `status:failed` it wins outright, + * regardless of timestamp. If the probe errors, the list scan below still + * prefers the pinned id when it appears among the candidates. Falls through + * to the existing freshest-wins behavior when unset or no longer failing — + * fully backward compatible. + */ + preferredId?: string; } export interface ResolverResult { @@ -67,9 +82,46 @@ export interface ResolverResult { * when no Failed test exists — never throws. */ export async function resolveFailingFrontendTestId(opts: ResolverOptions): Promise { - const { baseUrl, apiKey, projectId, maxPages = 5 } = opts; + const { baseUrl, apiKey, projectId, maxPages = 5, preferredId } = opts; const fetchImpl = opts.fetchImpl ?? fetch; + // DEV-393 (codex F3): probe the pinned fixture directly first, so the + // preference cannot be defeated by the maxPages list-scan cap (a pinned + // fixture beyond page `maxPages` would otherwise silently lose the slot to + // an incidental fresher failure). Any probe error or non-failed status + // falls through to the list scan; the in-candidates preference there + // remains as a second chance after a transient probe failure. + if (preferredId) { + try { + const resp = await fetchImpl(`${baseUrl}/tests/${encodeURIComponent(preferredId)}`, { + headers: { + 'x-api-key': apiKey, + 'x-request-id': `dev-e2e-resolver-preferred-${Date.now()}`, + accept: 'application/json', + }, + signal: AbortSignal.timeout(10_000), + }); + if (resp.ok) { + const test = (await resp.json()) as Partial & { projectId?: string }; + // The probe must honor the resolver's project scope (codex round 2): + // the list path filters by projectId server-side, so a pin that + // points at a failed FE test in a DIFFERENT project must not win + // here either. A response without a matching projectId (absent or + // mismatched) falls through to the project-scoped list scan. + if (test.type === 'frontend' && test.status === 'failed' && test.projectId === projectId) { + return { + testId: preferredId, + reason: + `Preferred pinned failingFrontendTestId (${preferredId}) confirmed status:failed ` + + `via direct GET (updatedAt=${test.updatedAt ?? 'unknown'}); list scan skipped`, + }; + } + } + } catch { + // Transient probe failure — fall through to the list scan below. + } + } + const candidates: TestListItem[] = []; let cursor: string | undefined; let pagesFetched = 0; @@ -134,6 +186,19 @@ export async function resolveFailingFrontendTestId(opts: ResolverOptions): Promi }; } + // DEV-393: a pinned, dedicated "always red" fixture wins over freshest + // whenever it is still in the failed set — see the doc comment on + // ResolverOptions.preferredId for why "freshest" alone is not enough. + if (preferredId) { + const pinned = candidates.find(c => c.id === preferredId); + if (pinned) { + return { + testId: pinned.id, + reason: `Preferred pinned failingFrontendTestId (${pinned.id}) is still status:failed (updatedAt=${pinned.updatedAt}); took priority over freshest-updatedAt candidate`, + }; + } + } + // Pick the most recently updated candidate — freshest failure is the // most likely to have a valid failure bundle attached. candidates.sort((a, b) => { diff --git a/src/lib/http.test.ts b/src/lib/http.test.ts index a9c1193..2baff2c 100644 --- a/src/lib/http.test.ts +++ b/src/lib/http.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest'; -import { ApiError, RequestTimeoutError, TransportError } from './errors.js'; +import { ApiError, InterruptError, RequestTimeoutError, TransportError } from './errors.js'; import type { DebugEvent } from './http.js'; import { HttpClient, REQUEST_TIMEOUT_DEFAULT_MS, buildUrl, parseRetryAfter } from './http.js'; import { VERSION } from '../version.js'; @@ -29,7 +29,11 @@ function errorEnvelopeResponse(status: number, code: string, init: ResponseInit function makeClient( fetchImpl: typeof fetch, - options: { apiKey?: string | null; onDebug?: (e: DebugEvent) => void } = {}, + options: { + apiKey?: string | null; + onDebug?: (e: DebugEvent) => void; + onServerVersion?: (info: { minVersion?: string }) => void; + } = {}, ): HttpClient { const apiKey = 'apiKey' in options ? (options.apiKey ?? undefined) : 'sk-test'; return new HttpClient({ @@ -39,9 +43,73 @@ function makeClient( sleep: () => Promise.resolve(), random: () => 0, onDebug: options.onDebug, + onServerVersion: options.onServerVersion, }); } +describe('CLIENT_TOO_OLD (426)', () => { + it('is not retried — fails fast with the typed error', async () => { + const fetchImpl = vi.fn().mockResolvedValue(errorEnvelopeResponse(426, 'CLIENT_TOO_OLD')); + const client = makeClient(fetchImpl as unknown as typeof fetch); + + const err = await client.get('/me').catch((e: unknown) => e); + + expect(err).toBeInstanceOf(ApiError); + expect((err as ApiError).code).toBe('CLIENT_TOO_OLD'); + expect((err as ApiError).exitCode).toBe(14); + expect(fetchImpl).toHaveBeenCalledTimes(1); // no retry + }); +}); + +describe('onServerVersion hook', () => { + it('fires with the parsed floor header on a 2xx response', async () => { + const onServerVersion = vi.fn(); + const fetchImpl = vi.fn().mockResolvedValue( + jsonResponse( + { ok: true }, + { + headers: { + 'content-type': 'application/json', + 'x-testsprite-cli-min-version': '1.0.0', + }, + }, + ), + ); + const client = makeClient(fetchImpl as unknown as typeof fetch, { onServerVersion }); + + await client.get('/me'); + + expect(onServerVersion).toHaveBeenCalledWith({ minVersion: '1.0.0' }); + }); + + it('fires on a non-2xx response too (the floor header rides on every response)', async () => { + const onServerVersion = vi.fn(); + const fetchImpl = vi.fn().mockResolvedValue( + errorEnvelopeResponse(404, 'NOT_FOUND', { + headers: { + 'content-type': 'application/json', + 'x-testsprite-cli-min-version': '1.0.0', + }, + }), + ); + const client = makeClient(fetchImpl as unknown as typeof fetch, { onServerVersion }); + + await client.get('/tests/missing').catch(() => undefined); + + expect(onServerVersion).toHaveBeenCalledWith({ minVersion: '1.0.0' }); + }); + + it('does not fire when the floor header is absent', async () => { + const onServerVersion = vi.fn(); + const fetchImpl = vi.fn().mockResolvedValue(jsonResponse({ ok: true })); + const client = makeClient(fetchImpl as unknown as typeof fetch, { onServerVersion }); + + await client.get('/me'); + + expect(onServerVersion).not.toHaveBeenCalled(); + }); +}); + describe('buildUrl', () => { it('handles trailing slashes and absolute paths', () => { expect(buildUrl('https://api.example.com/api/cli/v1', '/me')).toBe( @@ -641,3 +709,177 @@ describe('HttpClient per-request timeout', () => { expect(err).toBeInstanceOf(RequestTimeoutError); }); }); + +describe('HttpClient shutdown signal (DEV-331 graceful detach)', () => { + /** Stalled fetch that rejects with the effective signal's reason on abort. */ + function stalledFetch(callCounter?: { count: number }): typeof fetch { + return vi.fn(async (_input: unknown, init?: { signal?: AbortSignal }) => { + if (callCounter) callCounter.count += 1; + return new Promise((_resolve, reject) => { + const signal = init?.signal; + const rejectWithReason = (): void => { + const reason: unknown = signal?.reason; + if (reason instanceof Error) { + reject(reason); + return; + } + const err = new Error('aborted'); + err.name = 'AbortError'; + reject(err); + }; + if (signal?.aborted) { + rejectWithReason(); + return; + } + signal?.addEventListener('abort', rejectWithReason, { once: true }); + }); + }) as unknown as typeof fetch; + } + + function makeShutdownClient( + shutdownSignal: AbortSignal, + callCounter?: { count: number }, + ): HttpClient { + return new HttpClient({ + baseUrl: 'https://api.example.com/api/cli/v1', + apiKey: 'sk-test', + fetchImpl: stalledFetch(callCounter), + sleep: () => Promise.resolve(), + random: () => 0, + shutdownSignal, + }); + } + + it('aborts an in-flight fetch with the InterruptError reason (no retry, no re-wrap)', async () => { + const counter = { count: 0 }; + const shutdown = new AbortController(); + const client = makeShutdownClient(shutdown.signal, counter); + const pending = client.get('/runs/run_1'); + queueMicrotask(() => shutdown.abort(new InterruptError('SIGINT'))); + const err = await pending.catch(e => e); + expect(err).toBeInstanceOf(InterruptError); + expect((err as InterruptError).signal).toBe('SIGINT'); + expect((err as InterruptError).exitCode).toBe(130); + expect(counter.count).toBe(1); // never retried + }); + + it('classifies an anonymous AbortError as the interrupt when the shutdown signal fired', async () => { + // Some runtimes reject with a bare AbortError instead of the abort reason; + // rethrowIfAbort must still classify shutdown-first (never TransportError, + // never RequestTimeoutError). + const shutdown = new AbortController(); + const fetchImpl = vi.fn(async (_input: unknown, init?: { signal?: AbortSignal }) => { + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener( + 'abort', + () => { + const err = new Error('aborted'); + err.name = 'AbortError'; + reject(err); + }, + { once: true }, + ); + }); + }) as unknown as typeof fetch; + const client = new HttpClient({ + baseUrl: 'https://api.example.com/api/cli/v1', + apiKey: 'sk-test', + fetchImpl, + sleep: () => Promise.resolve(), + random: () => 0, + shutdownSignal: shutdown.signal, + }); + const pending = client.get('/runs/run_1'); + queueMicrotask(() => shutdown.abort(new InterruptError('SIGTERM'))); + const err = await pending.catch(e => e); + expect(err).toBeInstanceOf(InterruptError); + expect((err as InterruptError).signal).toBe('SIGTERM'); + }); + + it('skips dispatch entirely when the shutdown signal is already aborted', async () => { + const counter = { count: 0 }; + const shutdown = new AbortController(); + shutdown.abort(new InterruptError('SIGINT')); + const client = makeShutdownClient(shutdown.signal, counter); + const err = await client.get('/me').catch(e => e); + expect(err).toBeInstanceOf(InterruptError); + expect(counter.count).toBe(0); + }); + + it('passes an InterruptError from a caller-composed signal through untouched (poll path, no shutdownSignal configured)', async () => { + // The polling loop composes the shutdown signal into its per-iteration + // caller signal; the fetch then rejects with the InterruptError reason + // even though the client itself has no shutdownSignal. + const counter = { count: 0 }; + const fetchImpl = stalledFetch(counter); + const client = new HttpClient({ + baseUrl: 'https://api.example.com/api/cli/v1', + apiKey: 'sk-test', + fetchImpl, + sleep: () => Promise.resolve(), + random: () => 0, + }); + const caller = new AbortController(); + const pending = client.get('/runs/run_1', { signal: caller.signal }); + queueMicrotask(() => caller.abort(new InterruptError('SIGINT'))); + const err = await pending.catch(e => e); + expect(err).toBeInstanceOf(InterruptError); + expect(counter.count).toBe(1); // no transport retry + }); +}); + +describe('HttpClient retry-delay sleep bails on shutdown (DEV-331 codex finding 1)', () => { + it('a shutdown during a transport-retry sleep rejects with InterruptError immediately', async () => { + // First fetch throws a transport error → the client schedules a retry + // sleep. The injected sleep NEVER resolves, so only the shutdown race can + // end it — without the bail, the interrupt would wait out the delay + // (e.g. a Retry-After: 60) before surfacing. + const shutdown = new AbortController(); + let calls = 0; + const fetchImpl = vi.fn(async () => { + calls += 1; + throw new Error('socket hang up'); + }) as unknown as typeof fetch; + const client = new HttpClient({ + baseUrl: 'https://api.example.com/api/cli/v1', + apiKey: 'sk-test', + fetchImpl, + sleep: () => new Promise(() => {}), // never resolves + random: () => 0, + shutdownSignal: shutdown.signal, + }); + const pending = client.get('/me'); + // Let the first attempt fail and the retry sleep begin, then interrupt. + await new Promise(resolve => setTimeout(resolve, 10)); + shutdown.abort(new InterruptError('SIGINT')); + const err = await pending.catch(e => e); + expect(err).toBeInstanceOf(InterruptError); + expect(calls).toBe(1); // the retry never dispatched + }); + + it('a shutdown during a RATE_LIMITED Retry-After sleep rejects with InterruptError immediately', async () => { + const shutdown = new AbortController(); + let calls = 0; + const fetchImpl = vi.fn(async () => { + calls += 1; + return errorEnvelopeResponse(429, 'RATE_LIMITED', { + headers: { 'content-type': 'application/json', 'retry-after': '60' }, + }); + }) as unknown as typeof fetch; + const client = new HttpClient({ + baseUrl: 'https://api.example.com/api/cli/v1', + apiKey: 'sk-test', + fetchImpl, + sleep: () => new Promise(() => {}), // never resolves + random: () => 0, + shutdownSignal: shutdown.signal, + }); + const pending = client.get('/me'); + await new Promise(resolve => setTimeout(resolve, 10)); + shutdown.abort(new InterruptError('SIGTERM')); + const err = await pending.catch(e => e); + expect(err).toBeInstanceOf(InterruptError); + expect((err as InterruptError).signal).toBe('SIGTERM'); + expect(calls).toBe(1); + }); +}); diff --git a/src/lib/http.ts b/src/lib/http.ts index 7a9051a..f0139ca 100644 --- a/src/lib/http.ts +++ b/src/lib/http.ts @@ -1,6 +1,6 @@ import { randomUUID } from 'node:crypto'; import type { ErrorCode } from './errors.js'; -import { ApiError, RequestTimeoutError, TransportError } from './errors.js'; +import { ApiError, InterruptError, RequestTimeoutError, TransportError } from './errors.js'; import { VERSION } from '../version.js'; import type { TriggerRunBody, @@ -14,6 +14,7 @@ import type { BatchRunFreshResponse, ListRunsQuery, ListRunsResponse, + CancelRunResponse, } from './runs.types.js'; export type FetchImpl = typeof globalThis.fetch; @@ -74,6 +75,14 @@ export interface HttpClientOptions { * Stays silent when absent; wired to stderr at `--verbose` level. */ onTransition?: (msg: string) => void; + /** + * Optional callback fired with the backend's supported-floor header + * (`X-TestSprite-CLI-Min-Version`) when present on a response. Fires on both + * success and error responses. The client forwards the value verbatim — it + * applies no policy itself; the caller (client-factory) decides whether to + * warn the user. + */ + onServerVersion?: (info: { minVersion?: string }) => void; /** * Per-request wall-clock timeout in milliseconds applied to every outgoing * fetch. The signal fires independently of any caller-supplied signal — the @@ -87,6 +96,16 @@ export interface HttpClientOptions { * request is well within the 120s default. */ requestTimeoutMs?: number; + /** + * Process-lifetime shutdown signal (DEV-331 piece 1). Composed into every + * outgoing fetch so an armed SIGINT/SIGTERM aborts an in-flight request + * (a `--wait` long-poll can sit inside a single fetch for minutes) instead + * of waiting out its window. Aborts with an `InterruptError` reason, which + * is classified before the timeout-vs-caller logic and is never retried or + * re-wrapped as `TransportError`. Defaults off; production wiring passes + * `globalShutdown.signal` via the client factory. + */ + shutdownSignal?: AbortSignal; } export interface RequestOptions { @@ -167,19 +186,42 @@ export class HttpClient { private readonly random: () => number; private readonly onDebug?: (event: DebugEvent) => void; private readonly onTransition?: (msg: string) => void; + private readonly onServerVersion?: (info: { minVersion?: string }) => void; private readonly requestTimeoutMs: number; + private readonly shutdownSignal?: AbortSignal; constructor(options: HttpClientOptions) { this.baseUrl = trimTrailingSlash(options.baseUrl); this.apiKey = options.apiKey; + this.shutdownSignal = options.shutdownSignal; this.fetchImpl = options.fetchImpl ?? globalThis.fetch.bind(globalThis); this.sleep = options.sleep ?? defaultSleep; this.random = options.random ?? Math.random; this.onDebug = options.onDebug; this.onTransition = options.onTransition; + this.onServerVersion = options.onServerVersion; this.requestTimeoutMs = options.requestTimeoutMs ?? REQUEST_TIMEOUT_DEFAULT_MS; } + /** + * Read the backend's supported-floor header off a response and forward it to + * `onServerVersion` when present. Never throws — a bad header must not break + * the request. Called on every response (success or error) so the advisory + * covers all paths. (The backend advertises only the floor; "latest" is + * resolved client-side via the npm update-notice.) + */ + private captureServerVersion(response: Response): void { + if (!this.onServerVersion) return; + try { + const minVersion = response.headers.get('x-testsprite-cli-min-version') ?? undefined; + if (minVersion !== undefined) { + this.onServerVersion({ minVersion }); + } + } catch { + // Header parsing is best-effort; never let it affect the request. + } + } + async get(path: string, options: RequestOptions = {}): Promise { return this.requestWithMeta('GET', path, options).then(r => r.body); } @@ -382,6 +424,24 @@ export class HttpClient { }); } + /** + * POST /api/cli/v1/runs/{runId}/cancel + * User-initiated cancel of a queued/running run (DEV-331 piece 3). No + * body, no `Idempotency-Key` — the endpoint is naturally idempotent (D10): + * re-cancel → 200 `alreadyCancelled: true`; already-terminal (passed/ + * failed/blocked) → 409 CONFLICT; unknown/cross-tenant runId → 404. + * + * `retryOnConflict: false` — same rationale as `triggerRun`: a 409 here is + * a truthful terminal answer ("already finished"), not a transient + * snapshot conflict to paper over with a retry. + */ + async cancelRun(runId: string, options?: { signal?: AbortSignal }): Promise { + return this.post(`/runs/${encodeURIComponent(runId)}/cancel`, { + signal: options?.signal, + retryOnConflict: false, + }); + } + /** * Classify an error thrown while issuing OR reading a request. When it is an * abort/timeout and our per-request timeout signal fired (and the caller had @@ -397,6 +457,14 @@ export class HttpClient { requestId: string, effectiveSignal: AbortSignal = timeoutSignal, ): void { + // A user interrupt (DEV-331) outranks every other classification: once the + // shutdown signal fired, whatever error surfaced from the aborted fetch or + // body read is the interrupt. Throw its InterruptError reason so the wait + // paths can render the honest detach UX — never a RequestTimeoutError, and + // never fall through to the transport retry loop. + if (this.shutdownSignal?.aborted) { + throw this.shutdownSignal.reason; + } if (isAbortError(err) || isTimeoutError(err)) { const timeoutWon = timeoutSignal.aborted && @@ -422,6 +490,10 @@ export class HttpClient { let attempt = 0; while (true) { + // Bail before issuing (or re-issuing after a retry sleep) a request the + // user has already interrupted; the composed signal below would abort it + // immediately anyway, this just skips the wasted dispatch. + if (this.shutdownSignal?.aborted) throw this.shutdownSignal.reason; attempt += 1; this.debug({ kind: 'request', method, url, attempt, requestId }); const startedAt = Date.now(); @@ -436,8 +508,14 @@ export class HttpClient { // long-poll window (<=25s via ?waitSeconds), so it never bites polling. const requestTimeout = createRequestTimeout(this.requestTimeoutMs); const timeoutSignal = requestTimeout.signal; + const composedSignals = [timeoutSignal]; + if (options.signal != null) composedSignals.push(options.signal); + // Shutdown composition (DEV-331): an armed SIGINT/SIGTERM aborts the + // in-flight fetch immediately (reason: InterruptError) instead of + // letting a long-poll drain its window before the interrupt surfaces. + if (this.shutdownSignal != null) composedSignals.push(this.shutdownSignal); const effectiveSignal = - options.signal != null ? AbortSignal.any([timeoutSignal, options.signal]) : timeoutSignal; + composedSignals.length > 1 ? AbortSignal.any(composedSignals) : timeoutSignal; try { try { @@ -455,6 +533,13 @@ export class HttpClient { // A caller-supplied abort sets `name === 'AbortError'`. // We treat both abort variants together: if the timeout signal fired and // the caller hadn't already aborted, surface a clear RequestTimeoutError. + // A user interrupt is never retried and never re-wrapped as a + // TransportError (same passthrough discipline as RequestTimeoutError). + // The instanceof check matters even without `this.shutdownSignal`: + // the polling path composes the shutdown signal into its per- + // iteration caller signal, so the fetch can reject with the + // InterruptError reason directly (DEV-331). + if (err instanceof InterruptError) throw err; // A timeout/abort during the fetch itself: classify it (RequestTimeoutError // when our deadline fired; otherwise rethrow the caller's abort unmodified). this.rethrowIfAbort(err, timeoutSignal, options.signal, requestId, effectiveSignal); @@ -487,11 +572,15 @@ export class HttpClient { delayMs: decision.delayMs, }); requestTimeout.clear(); - await this.sleep(decision.delayMs); + await this.sleepBeforeRetry(decision.delayMs); continue; } const durationMs = Date.now() - startedAt; + // Surface the backend version-compatibility headers on every response + // (success or error) before branching, so the caller's advisory covers + // all paths. + this.captureServerVersion(response); if (response.ok) { this.debug({ kind: 'response', @@ -505,6 +594,8 @@ export class HttpClient { try { return { body: (await response.json()) as T, requestId, status: response.status }; } catch (err) { + // Interrupt passthrough (see the fetch catch above). + if (err instanceof InterruptError) throw err; // A timeout/abort can fire mid-body-read (headers received, stream stalls). this.rethrowIfAbort(err, timeoutSignal, options.signal, requestId, effectiveSignal); // Otherwise the successful response body was not valid JSON — a @@ -521,6 +612,8 @@ export class HttpClient { try { rawBody = await safeReadJson(response); } catch (err) { + // Interrupt passthrough (see the fetch catch above). + if (err instanceof InterruptError) throw err; // safeReadJson rethrows aborts/timeouts (it swallows only non-abort parse // errors), so a timeout fired mid-body-read on a non-OK response lands here. this.rethrowIfAbort(err, timeoutSignal, options.signal, requestId, effectiveSignal); @@ -559,7 +652,7 @@ export class HttpClient { delayMs: decision.delayMs, }); requestTimeout.clear(); - await this.sleep(decision.delayMs); + await this.sleepBeforeRetry(decision.delayMs); continue; } @@ -624,7 +717,7 @@ export class HttpClient { delayMs: decision.delayMs, }); requestTimeout.clear(); - await this.sleep(decision.delayMs); + await this.sleepBeforeRetry(decision.delayMs); } finally { requestTimeout.clear(); } @@ -652,6 +745,33 @@ export class HttpClient { return headers; } + /** + * Retry-delay sleep that bails the moment the shutdown signal fires + * (DEV-331, codex finding 1): a RATE_LIMITED `Retry-After: 60` or a + * transport backoff must not delay the honest-detach exit by up to a + * minute — reject with the InterruptError reason immediately. Mirrors + * poll.ts::sleepUnlessInterrupted. + */ + private sleepBeforeRetry(ms: number): Promise { + const signal = this.shutdownSignal; + if (signal == null) return this.sleep(ms); + if (signal.aborted) return Promise.reject(signal.reason); + return new Promise((resolve, reject) => { + const onAbort = (): void => reject(signal.reason); + signal.addEventListener('abort', onAbort, { once: true }); + this.sleep(ms).then( + () => { + signal.removeEventListener('abort', onAbort); + resolve(); + }, + err => { + signal.removeEventListener('abort', onAbort); + reject(err instanceof Error ? err : new Error(String(err))); + }, + ); + }); + } + private debug(event: DebugEvent): void { if (this.onDebug) this.onDebug(event); } @@ -856,6 +976,9 @@ function apiRetryDecision( case 'UNSUPPORTED': case 'INSUFFICIENT_CREDITS': case 'FEATURE_GATED': + case 'CLIENT_TOO_OLD': + // CLIENT_TOO_OLD: retrying re-sends the same too-old client — it can only + // self-heal by upgrading, so fail fast with the upgrade guidance. return { retry: false, delayMs: 0 }; case 'CONFLICT': // Read paths (e.g. GET /failure) retry once: 409 = mid-mutation snapshot. diff --git a/src/lib/interrupt.test.ts b/src/lib/interrupt.test.ts index 1fcf8eb..bd20bba 100644 --- a/src/lib/interrupt.test.ts +++ b/src/lib/interrupt.test.ts @@ -1,8 +1,10 @@ import { EventEmitter } from 'node:events'; import { writeSync } from 'node:fs'; import { describe, expect, it, vi } from 'vitest'; +import { InterruptError } from './errors.js'; import { SIGINT_EXIT_CODE, + ShutdownController, TERMINATION_EXIT_CODES, formatInterruptMessage, installBrokenPipeGuard, @@ -30,32 +32,37 @@ describe('formatInterruptMessage', () => { }); }); +/** Fresh handler map + controller per case: the disarmed path exits on the + * FIRST signal, so sequential signals on one install take the second-signal + * hard-exit branch (by design — DEV-331 SIG-5). */ +function install(shutdown = new ShutdownController()) { + const handlers = new Map void>(); + const stderr: string[] = []; + const exit = vi.fn(); + installSignalHandlers({ + on: (signal, handler) => handlers.set(signal, handler), + stderr: line => stderr.push(line), + exit, + shutdown, + }); + return { handlers, stderr, exit, shutdown }; +} + describe('installSignalHandlers', () => { it('registers SIGINT, SIGTERM and SIGHUP with the conventional 128+signum exit codes', () => { - const handlers = new Map void>(); - const stderr: string[] = []; - const exit = vi.fn(); - - installSignalHandlers({ - on: (signal, handler) => handlers.set(signal, handler), - stderr: line => stderr.push(line), - exit, - }); - - expect([...handlers.keys()].sort()).toEqual(['SIGHUP', 'SIGINT', 'SIGTERM']); - - handlers.get('SIGINT')!(); - expect(exit).toHaveBeenLastCalledWith(130); - handlers.get('SIGTERM')!(); - expect(exit).toHaveBeenLastCalledWith(143); - handlers.get('SIGHUP')!(); - expect(exit).toHaveBeenLastCalledWith(129); - - // Each handler emits a leading blank line then the explanation. - expect(stderr[0]).toBe(''); - expect(stderr.join('\n')).toContain('Interrupted (SIGINT)'); - expect(stderr.join('\n')).toContain('Interrupted (SIGTERM)'); - expect(stderr.join('\n')).toContain('Interrupted (SIGHUP)'); + for (const [signal, code] of [ + ['SIGINT', 130], + ['SIGTERM', 143], + ['SIGHUP', 129], + ] as const) { + const { handlers, stderr, exit } = install(); + expect([...handlers.keys()].sort()).toEqual(['SIGHUP', 'SIGINT', 'SIGTERM']); + handlers.get(signal)!(); + expect(exit).toHaveBeenLastCalledWith(code); + // Disarmed handler emits a leading blank line then the explanation. + expect(stderr[0]).toBe(''); + expect(stderr.join('\n')).toContain(`Interrupted (${signal})`); + } expect(SIGINT_EXIT_CODE).toBe(130); expect(TERMINATION_EXIT_CODES.SIGTERM).toBe(143); expect(TERMINATION_EXIT_CODES.SIGHUP).toBe(129); @@ -69,6 +76,7 @@ describe('installSignalHandlers', () => { installSignalHandlers({ on: (signal, handler) => handlers.set(signal, handler), exit, + shutdown: new ShutdownController(), }); handlers.get('SIGINT')!(); expect(exit).toHaveBeenCalledWith(130); @@ -78,6 +86,51 @@ describe('installSignalHandlers', () => { .join(''); expect(written).toContain('Interrupted (SIGINT)'); }); + + it('armed scope: first signal aborts with InterruptError and does NOT exit or print', () => { + const { handlers, stderr, exit, shutdown } = install(); + const disarm = shutdown.arm(); + handlers.get('SIGINT')!(); + + expect(exit).not.toHaveBeenCalled(); + expect(stderr).toEqual([]); + expect(shutdown.signal.aborted).toBe(true); + expect(shutdown.received).toBe('SIGINT'); + const reason = shutdown.signal.reason as InterruptError; + expect(reason).toBeInstanceOf(InterruptError); + expect(reason.signal).toBe('SIGINT'); + expect(reason.exitCode).toBe(130); + disarm(); + }); + + it('second signal during armed cleanup hard-exits with the second signal code (SIG-5)', () => { + const { handlers, exit, shutdown } = install(); + shutdown.arm(); + handlers.get('SIGINT')!(); + expect(exit).not.toHaveBeenCalled(); + handlers.get('SIGTERM')!(); + expect(exit).toHaveBeenCalledWith(143); + }); + + it('disposed scope reverts to the disarmed immediate-exit behavior', () => { + const { handlers, stderr, exit, shutdown } = install(); + const disarm = shutdown.arm(); + disarm(); + disarm(); // idempotent — double dispose must not underflow the counter + handlers.get('SIGTERM')!(); + expect(exit).toHaveBeenCalledWith(143); + expect(stderr.join('\n')).toContain('Interrupted (SIGTERM)'); + }); + + it('nested arms (fan-out members) stay armed until the last disposer runs', () => { + const shutdown = new ShutdownController(); + const a = shutdown.arm(); + const b = shutdown.arm(); + a(); + expect(shutdown.isArmed).toBe(true); + b(); + expect(shutdown.isArmed).toBe(false); + }); }); describe('installBrokenPipeGuard', () => { diff --git a/src/lib/interrupt.ts b/src/lib/interrupt.ts index cc5b4d5..19edb09 100644 --- a/src/lib/interrupt.ts +++ b/src/lib/interrupt.ts @@ -19,22 +19,100 @@ * without spawning a subprocess or sending a real signal. */ +import { setMaxListeners } from 'node:events'; import { writeSync } from 'node:fs'; +import { InterruptError, TERMINATION_EXIT_CODES, type TerminationSignal } from './errors.js'; + +export { TERMINATION_EXIT_CODES, type TerminationSignal } from './errors.js'; + +/** Back-compat alias: SIGINT's conventional exit code. */ +export const SIGINT_EXIT_CODE = TERMINATION_EXIT_CODES.SIGINT; /** - * Termination signals handled, mapped to their conventional `128 + signum` - * exit code. sourceRef: POSIX signal numbers (SIGHUP=1, SIGINT=2, SIGTERM=15). + * Structural view of {@link ShutdownController} threaded through the DI + * surfaces (`TestDeps`, `PollOptions`) — commands and the polling loop need + * only these members, and tests can supply a lightweight fake. */ -export const TERMINATION_EXIT_CODES = { - SIGINT: 130, // 128 + 2 - SIGTERM: 143, // 128 + 15 - SIGHUP: 129, // 128 + 1 -} as const; +export interface ShutdownHandle { + /** Aborts (reason: `InterruptError`) when a termination signal arrives while armed. */ + readonly signal: AbortSignal; + /** Enter a graceful-detach scope. Returns the disposer that leaves it. */ + arm(): () => void; +} -export type TerminationSignal = keyof typeof TERMINATION_EXIT_CODES; +/** + * Process-lifetime coordinator between the signal handler and the `--wait` + * polling paths (DEV-331 piece 1). + * + * Two modes, chosen by whether a graceful-detach scope is armed when the + * signal arrives: + * + * - **Armed** (inside `pollRunUntilTerminal`): the handler only aborts + * `signal` with an `InterruptError` — no I/O, no exit. The in-flight fetch + * and every backoff sleep bail immediately; the `--wait` catch blocks own + * the cleanup (finalize the ticker, print the honest partial envelope + + * re-attach hint, rethrow to `index.ts` → exit 130/143/129). + * - **Disarmed** (no wait in progress — prompts, one-shot commands, local + * FS work): the handler prints the generic explanation and exits + * immediately, preserving the pre-DEV-331 behavior. An abort nobody + * observes must never leave the process hanging at e.g. a readline prompt. + * + * A second signal while the armed cleanup is in flight is the documented + * escape hatch: immediate hard exit. + */ +export class ShutdownController { + private readonly controller = new AbortController(); + private armedCount = 0; + private receivedSignal: TerminationSignal | null = null; -/** Back-compat alias: SIGINT's conventional exit code. */ -export const SIGINT_EXIT_CODE = TERMINATION_EXIT_CODES.SIGINT; + constructor() { + // Every fetch and every poll iteration composes this signal via + // AbortSignal.any — a 50-run batch fan-out legitimately holds >10 + // concurrent listeners, so silence Node's MaxListeners warning. + setMaxListeners(0, this.controller.signal); + } + + get signal(): AbortSignal { + return this.controller.signal; + } + + /** The first termination signal received, or null if none yet. */ + get received(): TerminationSignal | null { + return this.receivedSignal; + } + + get isArmed(): boolean { + return this.armedCount > 0; + } + + /** + * Enter a graceful-detach scope (re-entrant: fan-out members overlap). + * Returns an idempotent disposer. + */ + arm(): () => void { + this.armedCount += 1; + let disposed = false; + return () => { + if (disposed) return; + disposed = true; + this.armedCount -= 1; + }; + } + + /** Record the signal and abort with an `InterruptError` carrying it. */ + interrupt(signal: TerminationSignal): void { + this.receivedSignal = signal; + this.controller.abort(new InterruptError(signal)); + } +} + +/** + * The process-wide instance: `index.ts` hands it to `installSignalHandlers`, + * and it is the default `shutdown` for `TestDeps` / `PollOptions` / + * `ClientFactoryDeps`, so production wiring is automatic. Tests inject their + * own `ShutdownController` (or a `ShutdownHandle` fake) instead. + */ +export const globalShutdown = new ShutdownController(); export function formatInterruptMessage(signal: TerminationSignal = 'SIGINT'): string { return ( @@ -50,11 +128,19 @@ export interface InterruptDeps { stderr?: (line: string) => void; /** Process exit. Defaults to `process.exit`. */ exit?: (code: number) => void; + /** Shutdown coordinator. Defaults to {@link globalShutdown}. */ + shutdown?: ShutdownController; } /** * Register handlers for SIGINT, SIGTERM and SIGHUP. Idempotent enough for a * single top-level call in `index.ts`; not designed to be installed twice. + * + * First signal, armed scope: abort-only — the `--wait` catch paths own the + * honest-detach UX and the exit (DEV-331 D1: Ctrl-C = detach, never cancel). + * First signal, disarmed: print the generic explanation + exit `128+signum`. + * Second signal (any mode): immediate hard exit — the escape hatch when the + * graceful cleanup itself wedges. */ export function installSignalHandlers(deps: InterruptDeps = {}): void { const on = @@ -75,9 +161,27 @@ export function installSignalHandlers(deps: InterruptDeps = {}): void { } }); const exit = deps.exit ?? ((code: number) => process.exit(code)); + const shutdown = deps.shutdown ?? globalShutdown; for (const signal of Object.keys(TERMINATION_EXIT_CODES) as TerminationSignal[]) { on(signal, () => { + if (shutdown.received !== null) { + // Second signal while graceful cleanup is in flight: hard exit now. + exit(TERMINATION_EXIT_CODES[signal]); + return; + } + if (shutdown.isArmed) { + // Graceful detach: abort only (sync, signal-safe — no I/O here so a + // pending stdout `drain` wait can settle); the armed catch paths + // finalize the ticker, print the partial + re-attach hint, and exit + // via index.ts with this signal's code. + shutdown.interrupt(signal); + return; + } + // Disarmed (no --wait in progress): legacy immediate exit. Record the + // signal first so a second one takes the hard-exit branch even when + // `exit` is injected and does not terminate (unit tests). + shutdown.interrupt(signal); // Blank line first so the message starts on its own row rather than // trailing the progress ticker's in-place line. stderr(''); diff --git a/src/lib/poll.spec.ts b/src/lib/poll.spec.ts index e6a14a1..93889eb 100644 --- a/src/lib/poll.spec.ts +++ b/src/lib/poll.spec.ts @@ -7,7 +7,8 @@ */ import { describe, expect, it } from 'vitest'; -import { ApiError } from './errors.js'; +import { ApiError, InterruptError } from './errors.js'; +import { ShutdownController } from './interrupt.js'; import { pollRunUntilTerminal, TimeoutError } from './poll.js'; import type { RunClient } from './poll.js'; import type { RunResponse } from './runs.types.js'; @@ -794,3 +795,104 @@ describe('pollRunUntilTerminal — resolveAlternate hook', () => { } }); }); + +// --------------------------------------------------------------------------- +// Graceful detach — shutdown handle (DEV-331 piece 1) +// --------------------------------------------------------------------------- + +describe('pollRunUntilTerminal — shutdown (SIGINT/SIGTERM graceful detach)', () => { + it('throws the InterruptError when the shutdown signal was already aborted (beats the deadline)', async () => { + const shutdown = new ShutdownController(); + shutdown.interrupt('SIGINT'); + // timeoutSeconds 0 → the deadline has also passed; the interrupt must win. + const err = await pollRunUntilTerminal(makeClient([makeRun('passed')]), RUN_ID, { + timeoutSeconds: 0, + sleep: instantSleep, + shutdown, + }).catch(e => e); + expect(err).toBeInstanceOf(InterruptError); + expect((err as InterruptError).signal).toBe('SIGINT'); + }); + + it('aborts an in-flight long-poll fetch and surfaces InterruptError (not TimeoutError)', async () => { + // getRun hangs until the composed per-iteration signal aborts — the same + // contract as a real fetch. Flag-checking between iterations would never + // notice; only the signal composition can interrupt this. + const client: RunClient = { + getRun: (_runId, opts) => + new Promise((_resolve, reject) => { + opts?.signal?.addEventListener('abort', () => reject(opts.signal!.reason), { + once: true, + }); + }), + }; + const shutdown = new ShutdownController(); + const poll = pollRunUntilTerminal(client, RUN_ID, { + timeoutSeconds: 30, + sleep: instantSleep, + shutdown, + }); + queueMicrotask(() => shutdown.interrupt('SIGINT')); + const err = await poll.catch(e => e); + expect(err).toBeInstanceOf(InterruptError); + expect((err as InterruptError).signal).toBe('SIGINT'); + expect((err as InterruptError).exitCode).toBe(130); + }); + + it('bails out of a retryAfterSeconds sleep immediately on interrupt', async () => { + // The injected sleep never resolves — only the shutdown race can end it. + const neverSleep = () => new Promise(() => {}); + const client = makeClient([makeRun('running', { retryAfterSeconds: 60 }), makeRun('running')]); + const shutdown = new ShutdownController(); + const poll = pollRunUntilTerminal(client, RUN_ID, { + timeoutSeconds: 300, + sleep: neverSleep, + shutdown, + }); + await new Promise(resolve => setTimeout(resolve, 10)); // let the loop reach the sleep + shutdown.interrupt('SIGTERM'); + const err = await poll.catch(e => e); + expect(err).toBeInstanceOf(InterruptError); + expect((err as InterruptError).signal).toBe('SIGTERM'); + expect((err as InterruptError).exitCode).toBe(143); + }); + + it('arms the graceful-detach scope for the poll duration and disarms after', async () => { + let armed = 0; + let disposedCount = 0; + const handle = { + signal: new AbortController().signal, + arm: () => { + armed += 1; + return () => { + disposedCount += 1; + }; + }, + }; + const run = await pollRunUntilTerminal(makeClient([makeRun('passed')]), RUN_ID, { + timeoutSeconds: 5, + sleep: instantSleep, + shutdown: handle, + }); + expect(run.status).toBe('passed'); + expect(armed).toBe(1); + expect(disposedCount).toBe(1); + }); + + it('disarms even when the poll throws (TimeoutError path)', async () => { + let disposedCount = 0; + const handle = { + signal: new AbortController().signal, + arm: () => () => { + disposedCount += 1; + }, + }; + const err = await pollRunUntilTerminal(makeClient([makeRun('running')]), RUN_ID, { + timeoutSeconds: 0, + sleep: instantSleep, + shutdown: handle, + }).catch(e => e); + expect(err).toBeInstanceOf(TimeoutError); + expect(disposedCount).toBe(1); + }); +}); diff --git a/src/lib/poll.ts b/src/lib/poll.ts index 0dfb0ac..92498e2 100644 --- a/src/lib/poll.ts +++ b/src/lib/poll.ts @@ -21,7 +21,8 @@ * Deadline exceeded → throw `TimeoutError`. */ -import { ApiError } from './errors.js'; +import { ApiError, InterruptError } from './errors.js'; +import type { ShutdownHandle } from './interrupt.js'; import type { RunResponse } from './runs.types.js'; import { isTerminalStatus } from './runs.types.js'; @@ -89,6 +90,16 @@ export interface PollOptions { elapsedMs: number, signal: AbortSignal, ) => Promise; + /** + * Graceful-detach coordinator (DEV-331 piece 1). While the poll runs, the + * scope is armed: a SIGINT/SIGTERM aborts `shutdown.signal` with an + * `InterruptError` instead of killing the process, and this loop surfaces + * it immediately — the in-flight long-poll fetch aborts (composed into the + * per-iteration signal) and every backoff/retry sleep bails early. The + * caller's catch block renders the honest partial + re-attach hint. + * Absent (tests, non-wait callers): behavior is unchanged. + */ + shutdown?: ShutdownHandle; } const LONG_POLL_WAIT_SECONDS = 25; @@ -108,9 +119,30 @@ export async function pollRunUntilTerminal( client: RunClient, runId: string, options: PollOptions, +): Promise { + // Arm the graceful-detach scope for the duration of the poll (DEV-331): + // while armed, a termination signal aborts instead of hard-killing the + // process, and the wait-path catch blocks own the honest detach UX. + const disarm = options.shutdown?.arm(); + try { + return await pollLoop(client, runId, options); + } finally { + disarm?.(); + } +} + +async function pollLoop( + client: RunClient, + runId: string, + options: PollOptions, ): Promise { const { timeoutSeconds, onTick, onTransition, resolveAlternate } = options; - const sleep = options.sleep ?? defaultSleep; + const shutdownSignal = options.shutdown?.signal; + const rawSleep = options.sleep ?? defaultSleep; + // Every sleep site (retryAfterSeconds, backoff schedule, not_yet_visible, + // 5xx retry) bails immediately on interrupt — a Ctrl-C must not sit out a + // 15s backoff before it is noticed. + const sleep = (ms: number): Promise => sleepUnlessInterrupted(rawSleep, ms, shutdownSignal); const startMs = Date.now(); const deadlineMs = startMs + timeoutSeconds * 1000; @@ -123,6 +155,9 @@ export async function pollRunUntilTerminal( let notYetVisibleRetries = 0; while (true) { + // Interrupt outranks the deadline: a Ctrl-C that raced the timeout must + // surface as the honest detach, not as a generic TimeoutError. + if (shutdownSignal?.aborted) throw shutdownSignal.reason; const now = Date.now(); if (now >= deadlineMs) { throw new TimeoutError(runId, timeoutSeconds); @@ -139,20 +174,33 @@ export async function pollRunUntilTerminal( const abortTimer = setTimeout(() => { abortController.abort(); }, remainingMs + TRANSPORT_CUSHION_MS); + // Compose the interrupt into the per-iteration signal: a `--wait` can sit + // inside one <=25s long-poll fetch (and the auto-raised per-request + // timeout means even longer for slow backends) — checking the flag + // between iterations is not enough, the in-flight fetch must abort. + const iterationSignal = + shutdownSignal != null + ? AbortSignal.any([abortController.signal, shutdownSignal]) + : abortController.signal; let run: RunResponse; try { if (useBackoff) { - run = await client.getRun(runId, { signal: abortController.signal }); + run = await client.getRun(runId, { signal: iterationSignal }); } else { const waitSeconds = Math.min(remainingSeconds, LONG_POLL_WAIT_SECONDS); - run = await client.getRun(runId, { waitSeconds, signal: abortController.signal }); + run = await client.getRun(runId, { waitSeconds, signal: iterationSignal }); } // Successful GET resets the consecutive-error counter. consecutiveErrors = 0; notYetVisibleRetries = 0; } catch (err) { clearTimeout(abortTimer); + // Interrupt classification precedes the timeout mapping: the composed + // signal makes the fetch reject on Ctrl-C, and that abort must surface + // as the InterruptError — not as a spurious TimeoutError. + if (err instanceof InterruptError) throw err; + if (shutdownSignal?.aborted) throw shutdownSignal.reason; // An AbortError from our per-iteration controller means the deadline // passed while the fetch was in flight — surface as TimeoutError. if (isAbortError(err)) { @@ -239,9 +287,16 @@ export async function pollRunUntilTerminal( } const altAbort = new AbortController(); const altTimer = setTimeout(() => altAbort.abort(), altRemainingMs); + // The alternate lookup aborts on interrupt too; its errors are swallowed + // by the fallback (best-effort), so the loop-top interrupt check above + // surfaces the InterruptError on the next iteration. + const altSignal = + shutdownSignal != null + ? AbortSignal.any([altAbort.signal, shutdownSignal]) + : altAbort.signal; let alternate: RunResponse | null = null; try { - alternate = await resolveAlternate(run, elapsedMs, altAbort.signal); + alternate = await resolveAlternate(run, elapsedMs, altSignal); } finally { clearTimeout(altTimer); } @@ -294,6 +349,36 @@ function defaultSleep(ms: number): Promise { return new Promise(resolve => setTimeout(resolve, ms)); } +/** + * Race a sleep against the shutdown signal: rejects with the signal's + * `InterruptError` reason the moment it fires, so no backoff/retry wait can + * delay the honest-detach UX. Wraps the injected `sleep` (tests keep their + * deterministic fakes); the underlying timer is left to fire harmlessly — + * the interrupt path exits the process long before it matters. + */ +function sleepUnlessInterrupted( + sleep: (ms: number) => Promise, + ms: number, + signal: AbortSignal | undefined, +): Promise { + if (signal == null) return sleep(ms); + if (signal.aborted) return Promise.reject(signal.reason); + return new Promise((resolve, reject) => { + const onAbort = (): void => reject(signal.reason); + signal.addEventListener('abort', onAbort, { once: true }); + sleep(ms).then( + () => { + signal.removeEventListener('abort', onAbort); + resolve(); + }, + err => { + signal.removeEventListener('abort', onAbort); + reject(err instanceof Error ? err : new Error(String(err))); + }, + ); + }); +} + /** * Detects an AbortError thrown when an AbortSignal fires. * Works for native fetch AbortErrors as well as `AbortController.abort()` diff --git a/src/lib/runs.types.ts b/src/lib/runs.types.ts index 7e556fe..15501c9 100644 --- a/src/lib/runs.types.ts +++ b/src/lib/runs.types.ts @@ -225,6 +225,20 @@ export interface RunResponse { steps?: RunStepDto[] | null; } +// --------------------------------------------------------------------------- +// DEV-331 piece 3 — cancel wire types +// --------------------------------------------------------------------------- + +/** + * Response from `POST /api/cli/v1/runs/{runId}/cancel`. + * Same shape as `GET /runs/{runId}` (`status: "cancelled"`, verdict + * untouched) plus `alreadyCancelled` distinguishing a fresh cancel + * (naturally idempotent) from a no-op re-cancel. + */ +export interface CancelRunResponse extends RunResponse { + alreadyCancelled: boolean; +} + /** Terminal states from the RunStatus union. */ export const TERMINAL_RUN_STATUSES: ReadonlySet = new Set([ 'passed', diff --git a/src/lib/v3-advisory.test.ts b/src/lib/v3-advisory.test.ts new file mode 100644 index 0000000..124720f --- /dev/null +++ b/src/lib/v3-advisory.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from 'vitest'; +import { routingLabel, V3_ROUTING_ADVISORY, emitV3RoutingAdvisory } from './v3-advisory.js'; + +describe('routingLabel', () => { + it('maps the boolean to v3 / v2', () => { + expect(routingLabel(true)).toBe('v3'); + expect(routingLabel(false)).toBe('v2'); + }); +}); + +describe('V3 routing advisory', () => { + it('names each open behavior gap (cancel, delete, target-url)', () => { + const text = V3_ROUTING_ADVISORY.join('\n'); + expect(text).toContain('test cancel'); + expect(text).toContain('test delete'); + expect(text).toContain('--target-url'); + }); + + it('emitV3RoutingAdvisory writes every line to the sink', () => { + const lines: string[] = []; + emitV3RoutingAdvisory(l => lines.push(l)); + expect(lines).toEqual(V3_ROUTING_ADVISORY); + }); +}); diff --git a/src/lib/v3-advisory.ts b/src/lib/v3-advisory.ts new file mode 100644 index 0000000..a8b75ed --- /dev/null +++ b/src/lib/v3-advisory.ts @@ -0,0 +1,25 @@ +/** + * Shared V3-routing text surfaces for `auth status` and `doctor`. + * + * `v3Enabled` on the `/me` response is the authoritative routing bit. When it + * is on, some commands behave differently while the V3 gaps stay open — the + * advisory names them. Copy lives here so both commands stay in sync. + */ + +/** One-word routing label for the text card. */ +export function routingLabel(v3Enabled: boolean): 'v3' | 'v2' { + return v3Enabled ? 'v3' : 'v2'; +} + +/** Consolidated advisory (stderr) emitted when V3 routing is on. */ +export const V3_ROUTING_ADVISORY: string[] = [ + '[advisory] V3 routing is on for this account. While these gaps are open:', + ' - `test cancel` may return 404', + ' - `test delete` may leave a zombie run', + ' - `--target-url` is ignored on frontend runs', +]; + +/** Write the advisory to a stderr sink, one line per call. */ +export function emitV3RoutingAdvisory(stderr: (line: string) => void): void { + for (const line of V3_ROUTING_ADVISORY) stderr(line); +} diff --git a/src/lib/version-notice.test.ts b/src/lib/version-notice.test.ts new file mode 100644 index 0000000..1ce473e --- /dev/null +++ b/src/lib/version-notice.test.ts @@ -0,0 +1,110 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { + formatBelowFloorNotice, + noteServerVersion, + resetBelowFloorNoticeState, + shouldWarnBelowFloor, + type VersionNoticeDeps, +} from './version-notice.js'; + +/** Baseline deps: all gates open, running version below the floor. */ +function baseDeps(overrides: Partial = {}): VersionNoticeDeps { + return { + currentVersion: '0.9.0', + env: {}, + isTTY: true, + outputMode: 'text', + dryRun: false, + ...overrides, + }; +} + +afterEach(() => { + resetBelowFloorNoticeState(); + vi.restoreAllMocks(); +}); + +describe('shouldWarnBelowFloor', () => { + it('warns when the running version is strictly below the floor', () => { + expect(shouldWarnBelowFloor({ minVersion: '1.0.0' }, baseDeps())).toBe(true); + }); + + it('does not warn when at or above the floor', () => { + expect(shouldWarnBelowFloor({ minVersion: '0.9.0' }, baseDeps())).toBe(false); // equal + expect(shouldWarnBelowFloor({ minVersion: '0.8.0' }, baseDeps())).toBe(false); // above + }); + + it('does not warn when no minVersion header was present', () => { + expect(shouldWarnBelowFloor({}, baseDeps())).toBe(false); + expect(shouldWarnBelowFloor({ minVersion: undefined }, baseDeps())).toBe(false); + }); + + it('does not warn on unparseable versions (garbage never warns)', () => { + expect(shouldWarnBelowFloor({ minVersion: 'not-a-version' }, baseDeps())).toBe(false); + expect( + shouldWarnBelowFloor({ minVersion: '1.0.0' }, baseDeps({ currentVersion: 'nope' })), + ).toBe(false); + }); + + it('is gated off by the opt-out env (any non-empty value)', () => { + expect( + shouldWarnBelowFloor( + { minVersion: '1.0.0' }, + baseDeps({ env: { TESTSPRITE_NO_UPDATE_NOTIFIER: '1' } }), + ), + ).toBe(false); + expect( + shouldWarnBelowFloor( + { minVersion: '1.0.0' }, + baseDeps({ env: { TESTSPRITE_NO_UPDATE_NOTIFIER: '0' } }), + ), + ).toBe(false); + }); + + it('is gated off under --output json, --dry-run, and non-TTY', () => { + expect(shouldWarnBelowFloor({ minVersion: '1.0.0' }, baseDeps({ outputMode: 'json' }))).toBe( + false, + ); + expect(shouldWarnBelowFloor({ minVersion: '1.0.0' }, baseDeps({ dryRun: true }))).toBe(false); + expect(shouldWarnBelowFloor({ minVersion: '1.0.0' }, baseDeps({ isTTY: false }))).toBe(false); + }); +}); + +describe('formatBelowFloorNotice', () => { + it('names the current version, the floor, and the npm upgrade command', () => { + const line = formatBelowFloorNotice('0.9.0', '1.0.0'); + expect(line).toContain('0.9.0'); + expect(line).toContain('minimum supported version 1.0.0'); + expect(line).toContain('npm install -g @testsprite/testsprite-cli'); + expect(line).toContain('TESTSPRITE_NO_UPDATE_NOTIFIER=1'); + }); + + it('does not name a target release (npm update-notice owns "latest")', () => { + const line = formatBelowFloorNotice('0.9.0', '1.0.0'); + expect(line).not.toContain('Upgrade to 1.'); + }); +}); + +describe('noteServerVersion', () => { + it('emits exactly one advisory line when below the floor', () => { + const stderr = vi.fn(); + noteServerVersion({ minVersion: '1.0.0' }, baseDeps({ stderr })); + expect(stderr).toHaveBeenCalledTimes(1); + expect(stderr).toHaveBeenCalledWith(expect.stringContaining('below the minimum supported')); + }); + + it('warns at most once per process', () => { + const stderr = vi.fn(); + const deps = baseDeps({ stderr }); + noteServerVersion({ minVersion: '1.0.0' }, deps); + noteServerVersion({ minVersion: '1.0.0' }, deps); + noteServerVersion({ minVersion: '1.0.0' }, deps); + expect(stderr).toHaveBeenCalledTimes(1); + }); + + it('stays silent when not below the floor', () => { + const stderr = vi.fn(); + noteServerVersion({ minVersion: '0.5.0' }, baseDeps({ stderr })); + expect(stderr).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/version-notice.ts b/src/lib/version-notice.ts new file mode 100644 index 0000000..0e492dc --- /dev/null +++ b/src/lib/version-notice.ts @@ -0,0 +1,111 @@ +/** + * Inline "your CLI is below the backend's minimum supported version" advisory. + * + * The backend advertises its supported floor on every `/api/cli/v1` response via + * the `X-TestSprite-CLI-Min-Version` header. Unlike the npm-registry update + * notice (`update-check.ts`, which runs in a Commander `preAction` hook before + * any HTTP request), this advisory reacts to a header observed *during* the + * request, so it is emitted from the HTTP layer's `onServerVersion` hook (wired + * in `client-factory.ts`) rather than the pre-request notifier — no cross-run + * cache needed. + * + * Distinct from the update notice on purpose, and non-redundant: this names the + * backend FLOOR and fires only when the running version is strictly below it (a + * serious "you will be rejected once enforcement is on" state); the update + * notice names the npm LATEST and fires whenever a newer release exists. Each + * owns its own number — the backend never advertises "latest". + * + * Reuses `compareSemver` and the `TESTSPRITE_NO_UPDATE_NOTIFIER` opt-out from + * `update-check.ts` so the gating and semver semantics stay consistent. + */ +import { compareSemver, UPDATE_CHECK_OPT_OUT_ENV } from './update-check.js'; +import { VERSION } from '../version.js'; + +/** Version-compatibility signal observed on a backend response (the floor). */ +export interface ServerVersionInfo { + minVersion?: string; +} + +export interface VersionNoticeDeps { + /** Version the running binary reports. Defaults to the built-in `VERSION`. */ + currentVersion?: string; + env?: NodeJS.ProcessEnv; + /** Whether stderr is an interactive terminal. */ + isTTY?: boolean; + /** The command's `--output` mode; `'json'` suppresses the advisory. */ + outputMode?: string; + /** Suppress under `--dry-run` (the dry-run fetch returns no real headers). */ + dryRun?: boolean; + /** Sink for the single advisory line. */ + stderr?: (line: string) => void; +} + +/** + * True when every gate passes and the running version is strictly below the + * advertised floor. Gates mirror the update notice: opt-out env, JSON output, + * dry-run, and non-TTY all suppress. Pure — no side effects, no process state. + */ +export function shouldWarnBelowFloor( + info: ServerVersionInfo, + deps: VersionNoticeDeps = {}, +): boolean { + const env = deps.env ?? process.env; + const currentVersion = deps.currentVersion ?? VERSION; + const isTTY = deps.isTTY ?? process.stderr.isTTY === true; + + const optOut = env[UPDATE_CHECK_OPT_OUT_ENV]; + if (optOut !== undefined && optOut !== '') return false; + if (deps.outputMode === 'json') return false; + if (deps.dryRun === true) return false; + if (!isTTY) return false; + + const minVersion = info.minVersion; + if (!minVersion) return false; + + // compareSemver returns -1 when the first arg is OLDER than the second. + // Unparseable input on either side compares as 0, so garbage never warns. + return compareSemver(currentVersion, minVersion) === -1; +} + +/** + * The single advisory line. Names the floor (the backend's authority) and the + * upgrade command — it deliberately does NOT name a target release; the npm + * update-notice is the single source of truth for the newest version. + */ +export function formatBelowFloorNotice(currentVersion: string, minVersion: string): string { + return ( + `Your testsprite-cli (${currentVersion}) is below the minimum supported version ${minVersion}. ` + + `Upgrade: npm install -g @testsprite/testsprite-cli ` + + `(disable this notice with ${UPDATE_CHECK_OPT_OUT_ENV}=1).` + ); +} + +/** Module-level guard: at most one below-floor advisory per process. */ +let warnedThisProcess = false; + +/** Test-only: reset the once-per-process guard between cases. */ +export function resetBelowFloorNoticeState(): void { + warnedThisProcess = false; +} + +/** + * Emit the below-floor advisory at most once per process. Wired to the HTTP + * client's `onServerVersion` hook. Never throws — an advisory must not break or + * delay the command it rides along with. + */ +export function noteServerVersion(info: ServerVersionInfo, deps: VersionNoticeDeps = {}): void { + try { + if (warnedThisProcess) return; + if (!shouldWarnBelowFloor(info, deps)) return; + + const minVersion = info.minVersion; + if (!minVersion) return; + + const currentVersion = deps.currentVersion ?? VERSION; + const stderr = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`)); + stderr(formatBelowFloorNotice(currentVersion, minVersion)); + warnedThisProcess = true; + } catch { + // Advisory is best-effort; never surface its failures to the command. + } +} diff --git a/src/version.ts b/src/version.ts index f60fdfd..5b301a6 100644 --- a/src/version.ts +++ b/src/version.ts @@ -1,3 +1,3 @@ // AUTO-GENERATED by scripts/generate-version.mjs — do not edit by hand. // Run `npm run build` (or `npm run generate:version`) to regenerate. -export const VERSION = '0.3.0'; +export const VERSION = '0.4.0'; diff --git a/test/__snapshots__/help.snapshot.test.ts.snap b/test/__snapshots__/help.snapshot.test.ts.snap index 62efe5d..b0681c5 100644 --- a/test/__snapshots__/help.snapshot.test.ts.snap +++ b/test/__snapshots__/help.snapshot.test.ts.snap @@ -146,6 +146,14 @@ Commands: get Get a project by id create [options] Create a new project update [options] Update project metadata + delete [options] Permanently delete a project and everything under it (sub-projects, + their tests, and backend fixtures). Requires --confirm. + + Exit codes: + 0 success + 3 auth error + 4 project not found (or already deleted) + 5 validation error (e.g., missing --confirm) credential [options] Set the static backend credential injected into every backend test (Bearer token / API key / Basic token / @@ -260,11 +268,14 @@ Commands: 4 test not found 5 validation error (e.g., bad --target-url, or positional + --all both set) 6 conflict (already running — see nextAction for the active runId) - 7 timeout — resume with: testsprite test wait + 7 timeout — resume with: testsprite test wait , or stop it with: testsprite test cancel 10 transport/network failure (UNAVAILABLE) — retry the command 11 rate limited — honor Retry-After On failure/blocked/cancelled, run: testsprite test artifact get + + Ctrl-C during --wait detaches only (the run keeps executing and billing); + stop it for real with: testsprite test cancel wait [options] Wait for one or more runs to reach a terminal status. With several run-ids the runs are polled concurrently under one shared @@ -282,6 +293,9 @@ Commands: 10 transport/network failure (UNAVAILABLE) — retry the command On failure/blocked/cancelled, run: testsprite test artifact get + + Ctrl-C detaches only (the run keeps executing and billing); stop it for + real with: testsprite test cancel rerun [options] [test-ids...] Re-execute a test (or multiple) as a cheap replay — FE replays the saved script (no credit), BE re-runs the dependency closure. Exit codes: @@ -291,10 +305,13 @@ Commands: 4 test not found 5 validation error 6 conflict (already running — see nextAction for the active runId) - 7 timeout or deferred — resume with: testsprite test wait + 7 timeout or deferred — resume with: testsprite test wait , or stop it with: testsprite test cancel 11 rate limited — honor Retry-After On failure/blocked/cancelled, run: testsprite test artifact get + + Ctrl-C during --wait detaches only (the run keeps executing and billing); + stop it for real with: testsprite test cancel flaky [options] Repeatedly replay a test to measure stability and surface flakiness. Replays run with auto-heal OFF (strict verbatim) so healed drift cannot mask nondeterministic pass/fail. @@ -309,10 +326,51 @@ Commands: failure Export the latest-failure agent bundle artifact Download run-scoped artifact bundles (M3.3 piece-4) + cancel Cancel one or more queued/running runs. + + Ctrl-C during --wait only detaches — it does NOT cancel the server-side + run. This is the real stop button. No refund is issued for the credits + already charged at trigger time (D3); an in-flight Lambda finishes on its + own and its result is discarded once cancelled. + + Exit codes: + 0 cancelled (fresh or already-cancelled — naturally idempotent) + 4 run id not found (single id), or ANY id not found (multi-id — outranks conflict) + 6 run already terminal (passed/failed/blocked) — single id: 409; multi-id: any conflict + + Multi-id output is a summary: {cancelled, alreadyCancelled, conflicts, notFound}. help [command] display help for command " `; +exports[`--help snapshots > test cancel 1`] = ` +"Usage: testsprite test cancel [options] + +Cancel one or more queued/running runs. + +Ctrl-C during --wait only detaches — it does NOT cancel the server-side +run. This is the real stop button. No refund is issued for the credits +already charged at trigger time (D3); an in-flight Lambda finishes on its +own and its result is discarded once cancelled. + +Exit codes: + 0 cancelled (fresh or already-cancelled — naturally idempotent) + 4 run id not found (single id), or ANY id not found (multi-id — outranks conflict) + 6 run already terminal (passed/failed/blocked) — single id: 409; multi-id: any conflict + +Multi-id output is a summary: {cancelled, alreadyCancelled, conflicts, notFound}. + +Arguments: + run-id one or more run ids to cancel + +Options: + -h, --help display help for command + +Global options (--dry-run, --output, --profile, --endpoint-url, --request-timeout, --verbose, --debug): + testsprite --help +" +`; + exports[`--help snapshots > test code get 1`] = ` "Usage: testsprite test code get [options] @@ -463,11 +521,14 @@ Exit codes: 4 test not found 5 validation error 6 conflict (already running — see nextAction for the active runId) - 7 timeout or deferred — resume with: testsprite test wait + 7 timeout or deferred — resume with: testsprite test wait , or stop it with: testsprite test cancel 11 rate limited — honor Retry-After On failure/blocked/cancelled, run: testsprite test artifact get +Ctrl-C during --wait detaches only (the run keeps executing and billing); +stop it for real with: testsprite test cancel + Options: --all rerun all tests in the resolved project (requires --project) (default: false) @@ -568,12 +629,15 @@ Exit codes: 4 test not found 5 validation error (e.g., bad --target-url, or positional + --all both set) 6 conflict (already running — see nextAction for the active runId) - 7 timeout — resume with: testsprite test wait + 7 timeout — resume with: testsprite test wait , or stop it with: testsprite test cancel 10 transport/network failure (UNAVAILABLE) — retry the command 11 rate limited — honor Retry-After On failure/blocked/cancelled, run: testsprite test artifact get +Ctrl-C during --wait detaches only (the run keeps executing and billing); +stop it for real with: testsprite test cancel + Options: --target-url override the project default env URL for this run (http/https only, no localhost/private IPs) diff --git a/test/cli.subprocess.test.ts b/test/cli.subprocess.test.ts index 959f212..3b04712 100644 --- a/test/cli.subprocess.test.ts +++ b/test/cli.subprocess.test.ts @@ -7,7 +7,7 @@ * and runs `auth whoami` against the mock." */ -import { execFileSync, spawn } from 'node:child_process'; +import { spawn } from 'node:child_process'; import { existsSync, mkdtempSync, rmSync, statSync } from 'node:fs'; import type { IncomingMessage, Server, ServerResponse } from 'node:http'; import { createServer } from 'node:http'; @@ -15,6 +15,7 @@ import { tmpdir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { execNpm } from './helpers/execNpm.js'; const __dirname = dirname(fileURLToPath(import.meta.url)); const REPO_ROOT = resolve(__dirname, '..'); @@ -37,7 +38,7 @@ beforeAll(async () => { // existsSync skip we used to do here let `dist` rot under // refactors and gave false-green on `project list` once // already. - execFileSync('npm', ['run', 'build'], { cwd: REPO_ROOT, stdio: 'pipe' }); + execNpm(['run', 'build'], { cwd: REPO_ROOT, stdio: 'pipe' }); server = createServer((req: IncomingMessage, res: ServerResponse) => { const url = req.url ?? '/'; if (url.startsWith('/api/cli/v1/projects/')) { diff --git a/test/e2e/signal.e2e.test.ts b/test/e2e/signal.e2e.test.ts new file mode 100644 index 0000000..6e03743 --- /dev/null +++ b/test/e2e/signal.e2e.test.ts @@ -0,0 +1,188 @@ +/** + * Local e2e tests for SIGINT/SIGTERM graceful detach during `--wait` + * (exit 130/143/129 per the documented signal contract). + * + * Spawns the real built binary (`dist/index.js`) against a local HTTP stub + * whose `GET /runs/{id}` long-poll hangs forever, sends a real signal to the + * child, and asserts the honest-detach contract: + * + * - stdout: parseable partial `{runId, status:"running"}` (JSON mode) + * - stderr: "keeps running (and billing)" + re-attach hint (+ INTERRUPTED + * envelope in JSON mode) + * - exit code 130 (SIGINT) / 143 (SIGTERM) + * + * Run via: `npm run test:e2e` (builds first). Excluded from `npm test`. + */ + +import { spawn } from 'node:child_process'; +import { existsSync } from 'node:fs'; +import { createServer, type Server } from 'node:http'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = resolve(__dirname, '../..'); +const BIN_PATH = join(REPO_ROOT, 'dist', 'index.js'); + +const RUN_ID = 'run_sig_e2e_01'; + +let server: Server; +let baseUrl = ''; +/** Resolvers waiting for the next hanging /runs request to arrive. */ +const runRequestWaiters: Array<() => void> = []; + +beforeAll(async () => { + if (!existsSync(BIN_PATH)) { + throw new Error('dist/index.js not found — run `npm run test:e2e` which builds first.'); + } + server = createServer((req, res) => { + // Hang every request (long-poll / stalled-backend simulation): the CLI's + // abort must cut it. Signal any test waiting for the request to arrive. + runRequestWaiters.splice(0).forEach(fn => fn()); + req.on('close', () => res.destroy()); + }); + await new Promise(resolveListen => { + server.listen(0, '127.0.0.1', () => resolveListen()); + }); + const address = server.address(); + if (address === null || typeof address === 'string') throw new Error('no server address'); + baseUrl = `http://127.0.0.1:${address.port}`; +}); + +afterAll(async () => { + await new Promise(resolveClose => { + server.close(() => resolveClose()); + server.closeAllConnections(); + }); +}); + +/** Resolves when the stub receives the next hanging GET /runs request. */ +function nextRunRequest(): Promise { + return new Promise(resolveWait => runRequestWaiters.push(resolveWait)); +} + +interface SpawnResult { + code: number | null; + signal: NodeJS.Signals | null; + stdout: string; + stderr: string; +} + +/** + * Spawn `testsprite test wait` against the hanging stub, deliver `signal` + * once the long-poll request is in flight, and collect the outcome. + */ +async function waitAndInterrupt( + signal: NodeJS.Signals, + extraArgs: string[] = [], +): Promise { + const child = spawn( + process.execPath, + [BIN_PATH, 'test', 'wait', RUN_ID, '--timeout', '120', ...extraArgs], + { + env: { + ...process.env, + TESTSPRITE_API_KEY: 'sk-e2e-signal', + TESTSPRITE_API_URL: baseUrl, + TESTSPRITE_NO_SKILL_WARNING: '1', + TESTSPRITE_NO_UPDATE_NOTIFIER: '1', + }, + stdio: ['ignore', 'pipe', 'pipe'], + }, + ); + let stdout = ''; + let stderr = ''; + child.stdout.on('data', (chunk: Buffer) => (stdout += chunk.toString())); + child.stderr.on('data', (chunk: Buffer) => (stderr += chunk.toString())); + + const arrived = nextRunRequest(); + const exited = new Promise<{ code: number | null; signal: NodeJS.Signals | null }>( + resolveExit => { + child.on('exit', (code, exitSignal) => resolveExit({ code, signal: exitSignal })); + }, + ); + + await arrived; // the long-poll fetch is in flight — the armed window is open + await new Promise(r => setTimeout(r, 150)); // let the request settle into the poll loop + child.kill(signal); + + const { code, signal: exitSignal } = await exited; + return { code, signal: exitSignal, stdout, stderr }; +} + +describe('signal e2e — graceful detach during test wait (DEV-331)', () => { + it('SIG-1/SIG-2: SIGINT → exit 130, partial JSON on stdout, honest stderr hint', async () => { + const result = await waitAndInterrupt('SIGINT', ['--output', 'json']); + expect(result.code).toBe(130); + + // stdout is a parseable partial naming the runId (file redirects never 0-byte). + const partial = JSON.parse(result.stdout) as { runId: string; status: string }; + expect(partial.runId).toBe(RUN_ID); + expect(partial.status).toBe('running'); + + // stderr: honest detach line + machine-readable INTERRUPTED envelope. + expect(result.stderr).toContain('Interrupted (SIGINT)'); + expect(result.stderr).toContain('billing'); + expect(result.stderr).toContain(`testsprite test wait ${RUN_ID}`); + expect(result.stderr).toContain('"code": "INTERRUPTED"'); + expect(result.stderr).toContain('"signal": "SIGINT"'); + }, 30_000); + + it('SIG-1 (text mode): SIGINT → exit 130, human-readable partial + hint', async () => { + const result = await waitAndInterrupt('SIGINT'); + expect(result.code).toBe(130); + expect(result.stdout).toContain(RUN_ID); + expect(result.stdout).toContain('running (interrupted)'); + expect(result.stderr).toContain('Interrupted (SIGINT)'); + expect(result.stderr).toContain('Error: Interrupted by SIGINT.'); + }, 30_000); + + it('SIG-3: SIGTERM → exit 143', async () => { + const result = await waitAndInterrupt('SIGTERM', ['--output', 'json']); + expect(result.code).toBe(143); + expect(result.stderr).toContain('Interrupted (SIGTERM)'); + expect(result.stderr).toContain('"signal": "SIGTERM"'); + }, 30_000); + + it('SIG-7: SIGINT during a non-wait command → immediate exit 130 with the generic explanation', async () => { + // `test list` is outside any armed --wait scope. The stub hangs its fetch; + // the disarmed handler must exit immediately with the generic explanation + // (no partial envelope — there is no runId to re-attach to). + const child = spawn(process.execPath, [BIN_PATH, 'test', 'list', '--project', 'p1'], { + env: { + ...process.env, + TESTSPRITE_API_KEY: 'sk-e2e-signal', + TESTSPRITE_API_URL: baseUrl, + TESTSPRITE_NO_SKILL_WARNING: '1', + TESTSPRITE_NO_UPDATE_NOTIFIER: '1', + }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + let stderr = ''; + child.stderr.on('data', (chunk: Buffer) => (stderr += chunk.toString())); + const exited = new Promise<{ code: number | null }>(resolveExit => { + child.on('exit', code => resolveExit({ code })); + }); + const arrived = nextRunRequest(); + await arrived; // the list fetch is in flight (disarmed — no poll running) + child.kill('SIGINT'); + const { code } = await exited; + expect(code).toBe(130); + expect(stderr).toContain('Interrupted (SIGINT)'); + expect(stderr).toContain('test wait'); + expect(stderr).not.toContain(' at '); // no stack trace / corrupted output + }, 30_000); + + it('SIG-8: detach then re-attach — the same runId can be waited on again (server unaffected)', async () => { + // First wait: interrupted. + const first = await waitAndInterrupt('SIGINT', ['--output', 'json']); + expect(first.code).toBe(130); + // Re-attach: the stub receives a fresh long-poll for the SAME runId — + // proof the detach was client-side only. (We interrupt again to end it.) + const second = await waitAndInterrupt('SIGINT', ['--output', 'json']); + expect(second.code).toBe(130); + const partial = JSON.parse(second.stdout) as { runId: string }; + expect(partial.runId).toBe(RUN_ID); + }, 60_000); +}); diff --git a/test/help.snapshot.test.ts b/test/help.snapshot.test.ts index ee89b3b..b6c23b4 100644 --- a/test/help.snapshot.test.ts +++ b/test/help.snapshot.test.ts @@ -13,6 +13,7 @@ import { execFileSync } from 'node:child_process'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { beforeAll, describe, expect, it } from 'vitest'; +import { execNpm } from './helpers/execNpm.js'; const __dirname = dirname(fileURLToPath(import.meta.url)); const REPO_ROOT = resolve(__dirname, '..'); @@ -43,11 +44,13 @@ const cases: Array<[string, string[]]> = [ // R5: regression guard for commands that gained new flag wording ['test create-batch', ['test', 'create-batch', '--help']], ['test run', ['test', 'run', '--help']], + // DEV-331 piece 3 + ['test cancel', ['test', 'cancel', '--help']], ]; describe('--help snapshots', () => { beforeAll(() => { - execFileSync('npm', ['run', 'build'], { cwd: REPO_ROOT, stdio: 'pipe' }); + execNpm(['run', 'build'], { cwd: REPO_ROOT, stdio: 'pipe' }); }); for (const [name, args] of cases) { diff --git a/test/helpers/execNpm.ts b/test/helpers/execNpm.ts new file mode 100644 index 0000000..0535112 --- /dev/null +++ b/test/helpers/execNpm.ts @@ -0,0 +1,17 @@ +import { execFileSync } from 'node:child_process'; + +/** + * Cross-platform `npm` invocation for test `beforeAll` build steps. + * On Windows, `npm` is a `.cmd` shim rather than a directly executable + * binary — `execFileSync('npm', ...)` fails with `ENOENT` there unless + * `shell: true` lets the OS resolve the shim through PATHEXT. + */ +export function execNpm( + args: string[], + options: { cwd: string; stdio?: 'pipe' | 'inherit' | 'ignore' }, +): Buffer | string { + return execFileSync('npm', args, { + ...options, + shell: process.platform === 'win32', + }); +}