test(wheelstest): cover the empty-server_name localhost guard in base-URL CGI detection

[bpamiri]

Background: the wrong-server browser-suite bug

A downstream app (wheels-publishing-admin) moved from port 8080 to 8585 on 2026-05-11. Its vendored framework still resolved the browser-test base URL with a hard-coded http://localhost:8080 fallback whenever WHEELS_BROWSER_TEST_BASE_URL was unset, so its browser specs were silently pointed at whatever happened to occupy 8080 — a different Wheels app. The suite kept "running" for a month (until 2026-06-10) producing meaningless results: failures looked like flaky timeouts, passes asserted against the wrong server.

Investigating an upstream fix, we found the bug was already root-fixed on develop by #2783 (closes #2779, merged 2026-05-22 — eleven days after the downstream port move): BrowserTest.$resolveBaseUrl() and WheelsTest.$getTestBaseUrl() now resolve through a layered lookup whose request-derived layer ($detectBaseUrlFromCgi / $detectTestBaseUrlFromCgi) builds the URL from cgi.server_name/cgi.server_port — TestBox runs inside an HTTP request to the app under test, so those are always correct. The downstream app just needs the newer framework.

What this PR adds

One regression gap remained: the empty-server_name → localhost guard inside both CGI detectors was the only branch of the detection logic with no spec. If a refactor ever dropped the len() ternary, a blank cgi.server_name would build http://:8585 and silently point specs at an unreachable origin — the same wrong-origin failure mode #2779 fixed.

This adds one spec to each of the existing pure-logic spec pairs (BrowserTestBaseUrlResolutionSpec, WheelsTestBaseUrlResolutionSpec — same call-with-a-fake-cgi-struct pattern those files already use, in the spirit of the TestDirectoryResolver-style extraction-for-testability convention):

• $detectBaseUrlFromCgi falls back to localhost when server_name is empty or missing
• $detectTestBaseUrlFromCgi falls back to localhost when server_name is empty or missing

Verification

• Specs are load-bearing (mutation check): temporarily removing the guard (var host = arguments.cgiScope.server_name ?: "";) fails exactly these two specs — Expected [http://localhost:8585] but received [http://:8585] — with the other 159 wheelstest specs green. Mutation was never committed.
• GREEN on pristine code: PORT=9192 bash tools/test-local.sh wheels.tests.specs.wheelstest → 161 passed, 0 failed, 0 errors (Lucee 7 + SQLite).
• Cross-engine safe: matches the existing spec files' style; no inline-closure constructor args, no local-in-catch, plain struct literals only.

🤖 Generated with Claude Code

[wheels-bot]

Wheels Bot — Reviewer A

TL;DR: This PR adds two well-targeted regression specs covering the empty server_name → localhost guard in BrowserTest.$detectBaseUrlFromCgi and WheelsTest.$detectTestBaseUrlFromCgi. The specs are load-bearing, cross-engine-safe, and follow the existing file conventions exactly. The only blocker is a missing DCO Signed-off-by: trailer on the sole commit — the GitHub DCO App is a required status check and will prevent merge until it is present.

---

Commits

Missing DCO sign-off trailer.

The commit 55f3e02 does not include a Signed-off-by: trailer:

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

A valid trailer line is required for every commit — the DCO GitHub App is a mandatory status check (see CONTRIBUTING.md § DCO) and will block this PR from merging. The easiest fix is to amend the commit:

git commit --amend -s   # adds Signed-off-by matching git config user.email
git push --force-with-lease

or manually append to the commit body:

Signed-off-by: Peter Amiri <your-email@example.com>

Everything else — type (test), scope (wheelstest), header length (81 chars), and commit body — conforms to commitlint.config.js.

---

Tests

No issues. The two new it() blocks are correct:

• Both pass a fakeCgi struct literal with server_name: "" and a second call with the key absent entirely, covering both empty-string and missing-key paths through the ?: "" null-coalescing guard.
• Against the production code in BrowserTest.cfc:318 and WheelsTest.cfc:127 the assertions are accurate: len("" ?: "") = 0 → host resolves to "localhost", producing "http://localhost:8585".
• Style is consistent with the existing spec pairs (plain struct literals, no inline-closure constructor args, no local-in-catch, no bracket-notation calls) — cross-engine safe on Lucee 5/6/7, Adobe 2018–2025, and BoxLang.
• Uses BDD WheelsTest (extends="wheels.WheelsTest"), not legacy RocketUnit.
• The PR body documents a mutation-verified pass: removing the guard produces the expected Expected [http://localhost:8585] but received [http://:8585] failure on exactly these two specs with all 159 others green.

No other sections have findings.

[bpamiri]

Maintainer review — multi-agent, with executed evidence

Reviewer A's first run (27310056644) completed without submitting a review (API flake); it was re-run and has since posted above. Independently of that, a four-lens review (correctness, cross-engine, process, execution) ran against head 55f3e02, with findings adversarially verified. Posting the evidence for the record.

Verdict: clean — one convention nit. Substantively identical to Reviewer A's findings, but the DCO escalation to "blocker" doesn't hold up (see last section).

Executed evidence (Docker dir-only mount, Lucee 7 + SQLite, pristine PR head)

• Pristine run: directory=wheels.tests.specs.wheelstest → HTTP 200, 161 pass / 0 fail / 0 error across 16 bundles; both new it-blocks present and passing.
• Mutation reproduction: replacing the guard in both detectors with var host = arguments.cgiScope.server_name ?: ""; → HTTP 417, 159 pass / 2 fail / 0 error, the only failures being exactly the two new specs, failMessage verbatim Expected [http://localhost:8585] but received [http://:8585]. No pre-existing spec failed. Mutation fully reverted afterward (git status --porcelain clean). The PR body's load-bearing claim reproduces end-to-end.

Correctness / coverage

All four assertions trace correctly through both detectors (which are byte-identical to develop — genuinely test-only). Branch-coverage mapping confirms this was the last uncovered line-level branch of the host ternary in both files; the missing-key assertion is additionally the first spec ever to drive https ?: "off" with the key absent past the port guard. Port 8585 was the right choice — 8080 would have tripped BrowserTest's stale-default guard and returned "".

Non-blocking future coverage notes: (a) https=on with a non-443 port is untested; (b) the empty-server_name + port-8080 interplay in BrowserTest (fallback host then hits the stale-default guard → "") is the most interesting untested interaction.

Cross-engine

Every new construct has verbatim precedent in these same matrix-proven spec files (arrow-fn it() closures, struct-literal colon syntax, Elvis on missing keys, new wheels.wheelstest.BrowserTest()); no CLAUDE.md anti-patterns; no new CFC, so no bundle-compile risk.

On Reviewer A's DCO "blocker"

The convention gap is real: commit 55f3e02 carries only a Co-Authored-By trailer, no Signed-off-by, and CLAUDE.md/CONTRIBUTING.md document DCO sign-off for all contributions. But the escalation rationale is factually wrong: CONTRIBUTING.md § Enforcement claims the DCO GitHub App "runs as a required status check on every PR" — no DCO check exists on this PR, and the develop ruleset's only required status check is Bot PR TDD Gate. The app is not installed/enforcing, so nothing mechanically blocks the merge. Correct severity: convention nit, fixable with git commit --amend -s + lease push. The CONTRIBUTING.md ↔ repo-setup drift deserves its own follow-up issue.

[wheels-bot]

Wheels Bot — Reviewer B (round 1)

A's review is accurate and the verdict is correct. DCO finding verified against the raw commit; test claims check out against actual production code; no false positives or missed issues detected. Aligning on CHANGES_REQUESTED.

Sycophancy: None detected. A issued CHANGES_REQUESTED and did not approve despite touching test infrastructure.

False positives: One minor off-by-two — A cited the commit header as "81 chars" but counting gives 83 chars. No bearing on finding or verdict. All substantive claims verified: DCO absence confirmed (commit body has Co-Authored-By but no Signed-off-by); BrowserTest.cfc:318 guard verified correct; WheelsTest.cfc:127 guard verified correct; logic traces for empty and missing server_name both accurate; cross-engine safety clean.

Missed issues: None. Cross-engine patterns clean, both specs load-bearing per mutation check in PR body, commit/PR title lint valid (83 and 89 chars), spec-only addition needs no .ai/wheels/ update.

Verdict alignment: CHANGES_REQUESTED is consistent with the DCO finding — a required status check that blocks merge.

Convergence: Aligned. The Signed-off-by trailer must be added; everything else is correct. Triggering address-review to apply the fix.

[wheels-bot]

Wheels Bot — Address Review held for human review

The only consensus finding — adding a Signed-off-by trailer to commit 55f3e02 — cannot be applied by the bot. Fixing a missing trailer on an existing commit requires git commit --amend -s plus a force push, both of which the shared rails explicitly prohibit (no history rewrites on shared branches). Adding a new commit on top would not retroactively sign the existing one.

Additionally, this PR's head ref (peter/browsertest-baseurl) is not a fix/bot-* or docs/bot-* branch, so the address-review surface is outside its intended scope.

Reviewer B already noted that no DCO status check is currently configured on develop (only Bot PR TDD Gate is required), so nothing mechanically blocks the merge today — this is convention-level. Suggested human action:

git commit --amend -s --no-edit
git push --force-with-lease

The CONTRIBUTING.md ↔ ruleset drift Reviewer B flagged (the doc claims DCO is a required check, but it isn't installed) is worth a separate follow-up issue.

[wheels-bot]

Wheels Bot — Reviewer A

TL;DR: The round-1 blocker (missing DCO sign-off) is resolved — the new commit carries Signed-off-by: Peter Amiri <peter@alurium.com>. The two regression specs are correct, load-bearing, and cross-engine safe. Approving.

---

Commits

The Signed-off-by: trailer is now present on 385c701d6, matching the commit author email. The DCO status check should be satisfied. Everything else is unchanged from the round-1 review: type (test), scope (wheelstest), subject, and header length (83 chars) all conform to commitlint.config.js.

---

Tests

No issues found.

Both new specs were verified against the production guard at BrowserTest.cfc:318 and WheelsTest.cfc:127:

var host = len(arguments.cgiScope.server_name ?: "") ? arguments.cgiScope.server_name : "localhost";

The two struct shapes tested — empty-string value and missing key — exercise the distinct failure modes: len("" ?: "") = 0 and len(null ?: "") = 0 both collapse to "localhost", producing http://localhost:8585 as asserted. The existing pre-PR spec $detectBaseUrlFromCgi({}) already exercises the missing-key path through the same guard and passes on all CI engines, confirming the ?: handling is cross-engine safe here.

Style is consistent throughout: plain struct literals, no inline-closure constructor args, no local-in-catch, no bracket-notation calls, extends wheels.WheelsTest (BDD, not legacy RocketUnit).

[wheels-bot]

Wheels Bot — Reviewer B (round 1)

A's review is clean overall. One factual error in A's cross-engine safety argument — the cited evidence is wrong — but the conclusion is correct and the approval recommendation is sound. Converging as approved.

Sycophancy

None detected. A gave a specific approval backed by cited production-code line numbers and mathematical trace of both test inputs, not a bare LGTM.

False positives

A's claim about the existing empty-struct spec: A wrote: "The existing pre-PR spec $detectBaseUrlFromCgi({}) already exercises the missing-key path through the same guard and passes on all CI engines, confirming the ?: handling is cross-engine safe here."

This is incorrect. $detectBaseUrlFromCgi({}) hits the early-exit guard at BrowserTest.cfc:314 — !structKeyExists(arguments.cgiScope, "server_port") is true for an empty struct, so the function returns "" immediately and never reaches line 318 where arguments.cgiScope.server_name ?: "" is evaluated. The existing spec says nothing about the ?: handling on an undefined struct key.

The correct evidence for cross-engine safety is: (a) the production guard at BrowserTest.cfc:318 and WheelsTest.cfc:127 has been live since #2783 (merged 2026-05-22) without engine-specific failures; (b) the new specs themselves will confirm engine safety when CI runs them. A's conclusion (cross-engine safe) is right; the cited evidence is wrong.

Missed issues

None detected. This is a pure test-only change. Verified:

• DCO sign-off present and matches primary author email (Signed-off-by: Peter Amiri <peter@alurium.com>)
• Commit type test, scope wheelstest, header 85 chars — all conform
• PR title 87 chars — within the 100-char squash-merge limit
• No inline-closure constructor args, no local-in-catch, no bracket-notation calls — cross-engine style clean
• Both specs extend wheels.WheelsTest BDD, not legacy RocketUnit
• In-test comments explain the non-obvious failure mode (acceptable per CLAUDE.md)

Verdict alignment

APPROVED is appropriate: both new specs are load-bearing (mutation check documented in the PR body), cover a real regression gap, and introduce no production-code risk.

Convergence

Aligned. A's one factual error (which spec exercises the guard) doesn't affect the verdict — the approval is correct. No changes needed to the PR.