Repository navigation
fix(test): the sweep's two integration reds — one real, one my own guard - #96
Merged
Merged
Conversation
The first full-CI verdict this release line has ever produced came back red. Unit ×4 and vitest green; both integration shards failed. Both were worth having. REAL — tests/integration/api-routes-critical.test.ts was making a live HTTPS request to aihorde.net, three attempts counting the retry, on every run. `GET /api/v1/models` refreshes the AI Horde image catalog whenever `aihorde` is active, and it is active by default: a no-auth provider has no connection row to switch off. aiHordeImageCatalog exposes setFetch for exactly this case; the test now injects a stub. The route's own catch keeps the last good snapshot, so an empty worker list changes none of the assertions. FALSE POSITIVE, and mine — tests/integration/api-keys.test.ts sets CLOUD_URL to http://cloud.example on purpose, so the cloud-sync branch is taken and fails. `cloud.example` is reserved by RFC 2606 / RFC 6761: there is no delegation for it anywhere, so it cannot reach a host. The guard I added in #56 counted it as "the suite reached the network" and failed a file that never left the machine. The first fix I wrote for that was wrong, and three existing guard tests caught it: I exempted reserved names from being BLOCKED, which let the connection through to a real DNS lookup — more network activity, not less. Blocking and counting are two decisions. A reserved name is now still refused, and only the counting changes. The log line says which case it was, so a future reader does not mistake one for the other. This is not a hole: the exemption is not "hosts a test asked for", it is "names that by standard resolve to nothing", and a provider smuggled in under `.test` would be just as unreachable. A test asserts the exemption does not reach aihorde.net, api.openai.com, example.com or a literal IP. Also: the shard jobs now upload _artifacts/release-green/. Without the per-gate logs a red shard reports only its first failure line and the assertion dies with the runner — which is why both of these had to be reproduced locally before they could be read at all. 25/25 api-keys + api-routes-critical, 0 guard violations 20/20 block-network-guard + block-network-wiring before: 15 attempts to cloud.example:80, 3 to aihorde.net:443 after: 0 counted, 0 to aihorde.net YAML parses; prettier clean Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 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. Comment |
LMPrado-DZ23
added a commit
that referenced
this pull request
Sep 20, 2026
…l reds (#100) A-H1 said the release-green sweep could not produce a verdict, and I wrote that the two ways out were "neither reachable by editing a workflow". One of them was. #87 split the sweep — resolve → seven slow-suite jobs → an aggregator that merges their reports — and this line now has the full-CI verdict it never had. What that verdict found is the point of having had it: · api-routes-critical.test.ts was making a LIVE HTTPS request to aihorde.net on every run · api-keys.test.ts was a false positive of the network guard I wrote in #56 — cloud.example is RFC-reserved and resolves nowhere Both fixed in #96; the second sweep passed all seven shards. Also recorded, because it is the honest remainder: the aggregator passes every static and drift gate and then dies at check:pack-artifact, six minutes of silence and exit 143, in both sweeps. That gate falls back to a full `next build` and the hosted runner cannot fit this tree — build.yml has been manual-only since diegosouzapw#11946 for exactly that reason. #99 stops it discarding thirteen green gates and seven green suites on the way out, by recording the gate as unmeasured instead. A fully green verdict needs USE_VPS_RUNNER with that runner online. That is an external dependency and the owner's call, not pending work, and the document now says so rather than leaving a HIGH that reads like something I still owe. [doc-links] PASS — 172 docs, 1044 internal links Co-authored-by: zodyp <zodyprado@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The first full-CI verdict this release line has ever produced came back red. Unit ×4 and vitest green; both integration shards failed. Both were worth having.
Real — a live call to a third party on every run
tests/integration/api-routes-critical.test.tswas making a live HTTPS request toaihorde.net(3 attempts, counting the retry).GET /api/v1/modelsrefreshes the AI Horde image catalog wheneveraihordeis active — and it is active by default, because a no-auth provider has no connection row to switch off.aiHordeImageCatalogexposessetFetchfor exactly this case; the test now injects a stub. The route's owncatchkeeps the last good snapshot, so an empty worker list changes none of the assertions.False positive — and it was mine
tests/integration/api-keys.test.tssetsCLOUD_URLtohttp://cloud.exampleon purpose, so the cloud-sync branch is taken and fails.cloud.exampleis reserved by RFC 2606 / RFC 6761: there is no delegation for it anywhere, so it cannot reach a host. The guard I added in #56 counted it as "the suite reached the network" and failed a file that never left the machine.My first fix for that was wrong, and three existing guard tests caught it. I exempted reserved names from being blocked, which let the connection through to a real DNS lookup — more network activity, not less.
Blocking and counting are two decisions. A reserved name is now still refused, and only the counting changes. The log line says which case it was, so nobody later mistakes one for the other.
This is not a hole: the exemption is not "hosts a test asked for", it is "names that by standard resolve to nothing" — a provider smuggled in under
.testwould be just as unreachable. A test asserts the exemption does not reachaihorde.net,api.openai.com,example.com, or a literal IP.Also: the shards now upload their gate logs
Without
_artifacts/release-green/, a red shard reports only its first failure line and the assertion dies with the runner. That is why both of these had to be reproduced locally before they could be read at all.api-keys+api-routes-criticalblock-network-guard+block-network-wiringcloud.example:80, 3 toaihorde.net:443🤖 Generated with Claude Code