Skip to content

test(http): stop the allow_html test waiting out the https retry - #14285

Merged
jdx merged 1 commit into
mainfrom
claude/mise-ci-failure-3afac4
Oct 10, 2026
Merged

jdx merged 1 commit into
mainfrom
claude/mise-ci-failure-3afac4

Conversation

@jdx

@jdx jdx commented Oct 10, 2026 •

Copy link
Copy Markdown
Owner

https://entire.io/gh/jdx/mise/trails/637

test_text_request_allow_html_returns_the_page (added in #14253) times out on every run, which fails unit-macos on main (run 38058688044) and reproduces locally on Linux.

Its second half checks that an HTML page is still rejected without allow_html(). For an http:// URL, TextRequest::send retries as https:// first. That retry sends a TLS hello to the plain-HTTP canned server, which drains until \r\n\r\n and so blocks until the 2s client timeout. nextest's CI ceiling is 1s, so both attempts are killed.

The test now uses a small inline listener for that half: it answers the first connection with the HTML and drops the second, so the https retry fails immediately. The test goes from timing out to 0.03s, and all 34 http::tests::retry tests pass. No production code changes.

AI-assisted — Tool: Claude Code; model: anthropic/claude-sonnet-5-5; version: 2.1.293.

🤖 Generated with Claude Code


Note

Low Risk
Test-only change in retry.rs; no runtime HTTP behavior is modified.

Overview
Fixes a flaky/timeout failure in test_text_request_allow_html_returns_the_page on the assertion that HTML is rejected without allow_html().

The second request still uses an http:// URL (so the client’s https retry runs), but the test no longer uses spawn_canned_server with two canned responses. It binds a one-off TcpListener that serves HTML on the first connection and leaves the second connection hanging without blocking until the client timeout—matching how a TLS hello on plain HTTP used to stall the canned server under nextest’s short CI deadline.

Comments in the test document that behavior. No production HTTP code changes.

Reviewed by Cursor Bugbot for commit a59cf87. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Tests
    • Updated coverage for HTTP-to-HTTPS retry behavior when an HTML response is followed by a dropped connection.

The second half of test_text_request_allow_html_returns_the_page sent the
https retry's TLS hello to a canned server that drains until a blank line,
so it hung until the 2s client timeout and tripped nextest's 1s ceiling.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Entire-Checkpoint: 01M4KYPBEDPXES6Z76ADJ5B135
@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c3e3ca0d-1920-43b3-a0c7-72fa8b98f78c

📥 Commits

Reviewing files that changed from the base of the PR and between 19e8c8f and a59cf87.


📒 Files selected for processing (1)
  • crates/mise-util/src/http/tests/retry.rs

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



📝 Walkthrough

Walkthrough

The HTML-rejection test replaces canned responses with a TCP listener. The listener returns HTML on the first connection and accepts, then drops, the second connection to model the documented HTTP-to-HTTPS retry.

Changes

HTTP retry test

Layer / File(s) Summary
Simulate the retry connection
crates/mise-util/src/http/tests/retry.rs
The test returns HTML on the first connection, then accepts and drops the second connection.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix

Suggested reviewers: jambalaya56562


Merge Risk: ⚪ Minimal · up to a59cf

The test now closes the HTTPS retry promptly after serving the expected HTML response, avoiding the timeout pattern. No actionable merge-blocking risk is evident.

Pre-merge checks | Passed 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: preventing the allow_html test from waiting for the HTTPS retry timeout.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

  • 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

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

@jdx
jdx enabled auto-merge (squash) October 10, 2026 22:40
@jdx
jdx merged commit e48d67c into main Oct 10, 2026
66 of 71 checks passed
@jdx
jdx deleted the claude/mise-ci-failure-3afac4 branch October 10, 2026 22:43
@github-actions

Copy link
Copy Markdown
Contributor

Instruction counts

benchmark trend instructions Δ wall (min) Δ
env █▁▃▄▁▁▂▄▃▄█ 27,896,842 → 27,932,607 +0.13% 19.61 → 20.08ms +2.41%
hook-env ▃▁▁██▇█▇█▇█ 30,205,788 → 30,228,904 +0.08% 20.89 → 20.88ms -0.04%
ls ▇▂▂▁▂▂▁▃▂▄█ 32,117,536 → 32,174,333 +0.18% 21.69 → 21.81ms +0.58%
registry ▇▁▅▁▁▁▁▁▃▄█ 40,175,424 → 40,242,053 +0.17% 17.72 → 18.13ms +2.33%
startup ▆▁▇▅█▇▅█▇▅▆ 11,077,051 → 11,079,873 +0.03% 12.14 → 12.42ms +2.33%

No instruction-count regression above 1%.

Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run.

Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.

a59cf8729a5f vs 19e8c8f0d03d · measured on the runner, not pushed to the history.

mise-en-dev added a commit that referenced this pull request Oct 11, 2026
<!-- entire-trail-link-start -->
https://entire.io/gh/jdx/mise/trails/628
<!-- entire-trail-link-end -->

### 🚀 Features

- **(runtime-symlinks)** add a global link to the version global config
selects by @jdx in [#14271](#14271)

### 🐛 Bug Fixes

- **(history)** skip rewriting unchanged checkpoint metadata on index
rebuild by @jdx in [#14249](#14249)
- **(http)** retry when a response body drops mid-stream by
@JamBalaya56562 in [#14253](#14253)
- **(lock)** decode percent-encoded release tags when checking a locked
URL by @jdx in [#14252](#14252)
- **(schema)** accept outputs = { auto = false } on tasks by
@JamBalaya56562 in [#14265](#14265)

### 📚 Documentation

- **(dotfiles)** add a recommended setup and a readable modes-and-keys
table by @jdx in [#14267](#14267)

### 🧪 Testing

- **(http)** stop the allow_html test waiting out the https retry by
@jdx in [#14285](#14285)

### 📦 Registry

- test ansible, azure and dvc against python 3.14 by @jdx in
[#14268](#14268)

## 📦 Aqua Registry Updates

### New Packages (2)

- [`runxhq/runx`](https://github.com/runxhq/runx)
- [`takumin/passt-bin`](https://github.com/takumin/passt-bin)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant