[kieugiathinhphat]

What does this PR do?

Restore safe transport retries for naturally idempotent cleanup operations after transient network failures, without weakening the existing replay protection for arbitrary keyless mutations.

The shared HTTP client currently retries transport failures for GET/HEAD requests and writes with an Idempotency-Key. However, run cancellation and tunnel deletion are safe to repeat by their server contracts, but do not carry an idempotency key.

This PR:

• Introduces a narrowly scoped, opt-in transportRetrySafe request option, honored by canRetryTransport.
• Enables the opt-in for cancelRun() and deleteTunnel().
• Enables safe transport retries for the generic client.delete() calls used by deleteTunnelForCleanup() in src/commands/test.ts and src/commands/tunnel.ts.
• Keeps generic keyless POST/PUT/PATCH/DELETE requests non-retriable by default.

No runtime dependencies were added.

Related issue

Closes #520

This implementation follows the scope accepted by @zeshi-du in #520.

Season 4 CLI Improvement Bonus

• I'm entering this PR for the Season 4 CLI Improvement Bonus

Discord username: kieugiathinhphat

Type of change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that changes existing behavior)
• Documentation only
• Build / CI / chore

Checklist

• PR targets the main branch.
• Commits follow Conventional Commits.
• npm run lint and npm run format:check pass.
• npm run typecheck passes.
• npm test passes and coverage stays at or above the 80% gate.
• New behavior is covered by mock-based unit tests.
• No secrets, API keys, internal endpoints, or personal data are included.
• No user-facing documentation changes are needed.

Validation

Local validation on Ubuntu/WSL, Node.js 22, using the v0.15.0 codebase:

• Targeted regression suite: 222 tests passed.
• Full unit suite: 4,813 passed, 2 skipped.
• Local E2E suite: 110 passed, 1 skipped.
• Typecheck, ESLint, Prettier, build and diff whitespace checks: passed.
• Coverage: statements 90.08%, branches 85.08%, functions 83.64%, lines 92.27%.

RED → GREEN regression evidence

Test: cleanup transport retry invariants > honours the original four-attempt transport retry cap

RED — unmodified base d61e6df (v0.15.0):

AssertionError: expected "vi.fn()" to be called 4 times, but got 1 times
Test Files  1 failed (1)
Tests       1 failed | 6 skipped (7)

GREEN — fixed branch:

The same regression test passes, with four transport attempts as expected. All seven cleanup retry invariant tests pass.

Additional regression tests cover cancellation, tunnel deletion, teardown cleanup following ECONNRESET, bare HTTP 502 responses, unsafe generic mutations, terminal HTTP 409 responses, disabled retries, attempt limits and shutdown behavior.

Notes for reviewers

The change is intentionally limited to transport retry eligibility and the four approved cleanup call sites.

It preserves:

• API-envelope retry behavior tracked separately in [Hackathon] fix(http): 5xx API-error retries still replay keyless writes (duplicate tunnel mints, projects, tests) #490.
• Existing retry budgets and backoff.
• Non-retry handling of terminal HTTP 409 responses.
• Existing CLI exit codes.
• The 10-second teardown deadline and abort semantics.
• Default replay protection for generic keyless mutations.

Local checks passed on Linux/WSL. GitHub CI, including the Windows job, remains to be verified on this PR.

Summary by CodeRabbit

• Bug Fixes
  • Tunnel cleanup now retries once after transient connection or server errors, helping shutdown complete successfully.
  • Run cancellation is more resilient to temporary network failures and server errors.
  • Cleanup avoids retrying conflict responses and limits repeated attempts to prevent unintended duplicate operations.
  • Other operations, including tunnel creation and generic requests, remain protected from unsafe automatic retries.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: TestSprite/testsprite-cli/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: e2857b88-4089-4bdc-ba50-35fa40fc8de1

📥 Commits

Reviewing files that changed from the base of the PR and between d61e6df and cf41e58.

📒 Files selected for processing: 8

• src/commands/test.local.spec.ts
• src/commands/test.ts
• src/commands/tunnel.spec.ts
• src/commands/tunnel.ts
• src/lib/http.cancel-retry.spec.ts
• src/lib/http.cleanup-retry-invariants.spec.ts
• src/lib/http.ts
• src/lib/http.tunnel.spec.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

HttpClient now supports explicitly opted-in transport retries for tunnel deletion and run cancellation. The changes also configure tunnel cleanup callers and add tests for retry behavior, retry limits, conflicts, and abort handling.

Changes

Cleanup transport retries

Layer / File(s)	Summary
Define transport retry eligibility src/lib/http.ts, src/lib/http.cleanup-retry-invariants.spec.ts	HttpClient adds the opt-in transportRetrySafe option and includes it in transport retry eligibility. Tests cover conflict responses, explicit retry disabling, the four-attempt limit, and already-aborted signals.
Enable retries for cleanup endpoints src/lib/http.ts, src/lib/http.cancel-retry.spec.ts, src/lib/http.tunnel.spec.ts	deleteTunnel and cancelRun enable transport retries. Tests cover retries after ECONNRESET and HTTP 502, and confirm generic keyless writes remain non-retriable.
Apply retry options in cleanup callers src/commands/test.ts, src/commands/tunnel.ts, src/commands/test.local.spec.ts, src/commands/tunnel.spec.ts	Tunnel cleanup callers disable conflict retries and enable transport retries. Command tests verify cleanup retries after ECONNRESET.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant runTunnelStart
  participant HttpClient
  participant TunnelAPI
  runTunnelStart->>HttpClient: deleteTunnel
  HttpClient->>TunnelAPI: DELETE tunnel
  TunnelAPI-->>HttpClient: ECONNRESET
  HttpClient->>TunnelAPI: Retry DELETE tunnel
  TunnelAPI-->>HttpClient: 204 response
  HttpClient-->>runTunnelStart: Cleanup completes

Loading

Suggested reviewers: zeshi-du

Merge Risk: ⚪ Minimal · up to cf41e

The change restores retries for idempotent cleanup requests and leaves generic keyless writes non-retriable. No actionable merge-blocking risk remains in the supplied review.

Pre-merge checks: 4 · 1

Failed checks: 1 inconclusive

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 7 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: restoring transport retries for naturally idempotent cleanup operations.
Linked Issues check		The PR implements #520. RequestOptions adds transportRetrySafe, with the default disabled. The HTTP retry policy accepts this opt-in in addition to idempotent methods and keyed writes. `cancelRun(…
Out of Scope Changes check		The changes stay within #520. The source changes are limited to transport-retry eligibility and the approved cancellation, tunnel-deletion, and teardown cleanup call sites. The added tests directly ve…

Full details: Linked Issues check

Explanation

The PR implements #520. RequestOptions adds transportRetrySafe, with the default disabled. The HTTP retry policy accepts this opt-in in addition to idempotent methods and keyed writes. cancelRun() and deleteTunnel() enable the opt-in. Cleanup client.delete() calls also enable it. The added tests cover transient transport failures, HTTP 502, generic keyless mutations, keyed writes, terminal 409 responses, retry limits, aborts, and teardown cleanup. The implementation preserves the stated retry and abort behavior and does not change API-envelope retries from #490.

Full details: Out of Scope Changes check

Explanation

The changes stay within #520. The source changes are limited to transport-retry eligibility and the approved cancellation, tunnel-deletion, and teardown cleanup call sites. The added tests directly verify the requested behavior and safety protections. No unrelated feature, dependency, or API-envelope retry change is shown.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 7 files. (1 skipped: 1 too large.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🏆 Season 4 CLI Improvement Bonus: entry recorded. Thanks, @kieugiathinhphat!

To be counted, this PR needs the following by Oct 29, 11:59 PM PT:

• ✅ A linked issue assigned to you.
• ✅ Out of draft.
• ✅ A Discord username in the description.

Rewards go to merged PRs judged a genuine improvement, and up to three bonus-eligible PRs count per person. Untick the box to withdraw the entry. This comment refreshes when you edit the PR. Full rules.

[zeshi-du]

@kieugiathinhphat thanks, the fix is right and the tests are good. One change before merge: #624 (for #490) adds a replaySafe request option on the same call sites (cancelRun, deleteTunnel, both deleteTunnelForCleanup helpers), and we don't want two flags that both mean "this endpoint is safe to replay". The two PRs complement each other: #624 gates only the 5xx-envelope branch, yours gates the transport branch. Once #624 is merged, please rebase onto main, drop transportRetrySafe, and make canRetryTransport return true when options.replaySafe === true. Keep retryOnConflict: false on the teardown helpers, and keep your ECONNRESET, bare-502, 409 and retry-cap tests, switched to the new option.