fix(cli): give wheels test its own timeout budget instead of the bridge default

[bpamiri]

Closes #3352.

The cause is one line

conn.setReadTimeout(120000);   // makeHttpRequestWithStatus(), every caller

That matches the reported ~140s threshold exactly. The budget is right for the short request/response bridge commands, but a test run is the one command here whose duration is expected to scale with the project — and when it blew, the CLI produced no result document at all. Not a failure report, a crashed runner, indistinguishable from a hung app.

The fix

• --timeout=<seconds>, else WHEELS_TEST_TIMEOUT, else 900.
• Non-numeric or non-positive input falls back to the default rather than throwing — a mistyped timeout should not be the thing that stops a test run.
• The browser-test runner got the same budget. It makes the same long-running call over the same helper at a second call site; fixing only one would have left the identical bug in a sibling.
• The shared helper keeps its 120s default, so no other command's behaviour changes.

On a timeout the message now names the real situation instead of echoing the engine:

Test run timed out after 900s waiting for the suite to finish.
The specs may have passed — the CLI stopped waiting, the runner did not stop running.
Give it longer:  wheels test --timeout=1800
Or set WHEELS_TEST_TIMEOUT=<seconds> for the whole environment.
Or scope the run:  wheels test --filter=<subdirectory>

Two harness defects in the same family

Both of these bit me while verifying tonight's other PRs, and both match this issue's theme — a runner reporting something other than what happened:

1. tools/test-local.sh wrote results to a single fixed /tmp path shared by every checkout on the machine. Two working copies running the suite overwrite each other. This silently turned my first develop-vs-branch comparison into two copies of the same run — with identical totals, which read like a legitimate no-op result. Now keyed on the project root; override with WHEELS_TEST_RESULT_FILE.

2. A request that failed outright left the previous run's results in place, and even printed them. HTTP 000 (typically a server not yet up) does not truncate the file. I read one of those as a current result before noticing the numbers were implausible. The file is now cleared before the request, so a crashed run leaves no result rather than a stale one.

Verification

CLI suite via /wheels/cli/tests:

	result
develop ab901cff7	1143 pass / 0 fail / 0 error / 1203 specs
this branch	1146 pass / 0 fail / 0 error / 1206 specs

Exactly +3 — the new $resolveTestTimeout specs (default, explicit value, junk-input fallback). Core suite unaffected and unchanged.

A note on how I ran that: tools/test-cli-local.sh could not be used — it invokes a lucli binary that does not exist on a normal install (cli/CLAUDE.md: "wheels IS the binary… there is no separate lucli binary"). It fails with nohup: lucli: No such file or directory. Both runs above went through the endpoint against a server started with wheels server run. Worth its own issue; say the word.

Not included, from the issue's "adjacent" section

Deliberately left out so this PR stays one change:

• bundles= is accepted and silently ignored. You are right that this is worse than rejecting it. It is a server-side change in the test runner, so it wants its own PR and its own matrix run.
• ?directory= is undocumented. It is the main workaround for this bug and deserves surfacing in the guides — a docs change, and arguably less urgent now that the timeout no longer forces people into it.

Happy to take both next.

[wheels-bot]

Wheels Bot — Reviewer

TL;DR — This PR gives wheels test its own read-timeout budget (900s default, --timeout / WHEELS_TEST_TIMEOUT overridable) instead of inheriting the shared 120s bridge budget, applies the same budget to the browser-test runner, and hardens tools/test-local.sh against stale/overwritten result files. The CLI code change is correct, well-documented, and properly unit-tested. Verdict: comment — the timeout fix is solid and I'd merge it, but the PR also commits a stray rewrite.config at the repo root that is unrelated to the fix and should be dropped before merge.

Conventions

• rewrite.config (new file, repo root) is an accidental inclusion and should be removed from this PR. It is byte-for-byte identical to cli/lucli/templates/app/rewrite.config — i.e. the artifact LuCLI's CatalinaBaseConfigGenerator drops into a project root when you run wheels server run (which the PR body says was used to run the suite). It has nothing to do with the timeout fix, and the PR body itself states the intent to keep "this PR stays one change" and lists two other items deliberately deferred to their own PRs. Committing it also creates a latent footgun: because a root rewrite.config overrides LuCLI's bundled template verbatim, a tracked copy will silently go stale if the template ever changes. Recommend git rm rewrite.config from the branch (and consider adding it to .gitignore, since it's a per-environment generated file).

Cross-engine

No issues. This is CLI code on the bundled Lucee runtime; the new parameters (parsedTimeout, readTimeout, timeoutSeconds) avoid reserved scope names (anti-pattern #11), and makeHttpRequest/makeHttpRequestWithStatus are called with fully-named args, so no positional/named mixing. javacast("int", arguments.readTimeout) on 900 * 1000 = 900000 is well within int range.

Tests

Good coverage. TestCommandSpec.cfc::$resolveTestTimeout exercises the default ("", " "), explicit values ("1800", " 45 "), and the junk/non-positive fallback ("soon", "0", "-30") — matching the three documented branches of $resolveTestTimeout. New public method, new BDD specs extending the CLI BaseSpec; TDD expectation met.

Correctness

Verified the change does what the title claims:

• Module.cfc:5823 threads timeoutSeconds * 1000 into the test-runner request; the catch at :5869 matches (read timed out|SocketTimeout) — java.net.SocketTimeoutException surfaces as Read timed out, so the branch fires correctly and prints an actionable message instead of the raw engine error.
• The shared makeHttpRequestWithStatus keeps its readTimeout = 120000 default, so the other ~8 makeHttpRequest(...) bridge callers are unaffected — confirmed each still calls the single-arg form.
• tools/test-local.sh:39 keys RESULT_FILE on PROJECT_ROOT (defined at :28, before use) and :139 clears it before the curl. This file's RESULT_FILE is self-contained — CI reads results via tools/ci/run-tests.sh / compat-matrix.yml, which use their own paths, so no CI breakage.

Two minor, non-blocking notes:

• tools/test-local.sh:39 uses shasum, which is Perl-provided and not guaranteed on minimal Linux dev boxes; sha1sum is more universally present there. Cosmetic — the override env var is the real escape hatch.
• The browser-runner catch (Module.cfc:8087) still prints the generic "Failed to reach test runner… Is the server running?" on a timeout rather than the new timeout-specific guidance. Not a regression, just an inconsistency with the primary path.

Commits / Docs

• Commit dbc93a51e fix(cli): give wheels test its own timeout budget instead of the bridge default conforms to commitlint (valid type/scope, subject ≤100, not ALL-CAPS).
• Changelog fragment changelog.d/3352-test-timeout.fixed.md uses the correct <slug>.<type>.md fragment form (not a direct CHANGELOG.md edit). Good.

Nice work on the fix itself — clear rationale in the code comments and honest scoping in the PR body. Just drop the stray rewrite.config and this is ready.

[wheels-bot]

Wheels Bot — Reviewer

TL;DR — This PR gives wheels test its own read-timeout budget (900s default, overridable via --timeout / WHEELS_TEST_TIMEOUT) instead of inheriting the shared 120s bridge budget, extends the same budget to the browser-test runner, adds a timeout-specific error message, and hardens tools/test-local.sh against stale/overwritten result files. The previous review's only blocker — a stray rewrite.config at the repo root — has been dropped; the file list is now exactly the four files the fix needs. The change is correct, well-documented, and properly unit-tested. Verdict: approve.

Previously-blocking finding, now resolved

• rewrite.config (repo root) removed. The prior review flagged an accidental byte-for-byte copy of cli/lucli/templates/app/rewrite.config committed at the repo root. gh pr view 3359 --json files now returns only changelog.d/3352-test-timeout.fixed.md, cli/lucli/Module.cfc, cli/lucli/tests/specs/commands/TestCommandSpec.cfc, and tools/test-local.sh — the stray file is gone. Resolved.

Correctness

Verified the change does what the title claims:

• Module.cfc:5823 threads arguments.timeoutSeconds * 1000 into the test-runner request; runTests's new numeric timeoutSeconds = 900 param (:5754) is threaded positionally in call order from testCommand and defaults correctly.
• The catch at :5869 matches (read timed out|SocketTimeout); java.net.SocketTimeoutException surfaces its message as Read timed out, so the branch fires and prints actionable guidance rather than the raw engine error. runState.crashed = true still propagates to the Wheels.TestRunFailed throw at :5888, so a timed-out run exits non-zero — no silent green.
• makeHttpRequest (:7643) gains numeric readTimeout = 120000 and forwards it via named args to makeHttpRequestWithStatus; every other makeHttpRequest(...) bridge caller uses the single-arg form and keeps the 120s default, so no unrelated command changes behaviour. javacast("int", 900000) is well within int range.
• tools/test-local.sh:39 keys RESULT_FILE on PROJECT_ROOT (set at :28, before use) and :139 clears it before the curl, so a crashed run leaves no stale result to misread.

Conventions

Clean. $resolveTestTimeout is a public $-prefixed helper — the established CLI pattern for spec-reachable internals — and mcpHiddenTools()'s structural sweep (Module.cfc:200–215) hides every $-prefixed public function from the MCP surface, so it does not leak as a tool. Mirrors the existing $resolveTestBasePath / $normalizeTestFilter prior art the change explicitly follows.

Cross-engine

No issues. CLI code on the bundled Lucee runtime; new params (parsedTimeout, readTimeout, timeoutSeconds) avoid reserved scope names (anti-pattern #11), and the two forwarding calls use fully-named args (no positional/named mixing). val(" 45 ") trims correctly and isNumeric(trim(...)) guards the parse.

Tests

Good coverage. TestCommandSpec.cfc::$resolveTestTimeout exercises the default ("", " "), explicit values ("1800", " 45 "), and the junk/non-positive fallback ("soon", "0", "-30") — matching the three documented branches of $resolveTestTimeout. New public helper, new BDD specs extending the CLI BaseSpec; TDD expectation met.

Commits / Docs

• ce125db99 fix(cli): give wheels test its own timeout budget instead of the bridge default conforms to commitlint (valid type/scope, subject ≤100, not ALL-CAPS).
• Changelog fragment changelog.d/3352-test-timeout.fixed.md uses the correct <slug>.<type>.md form (not a direct CHANGELOG.md edit). Good.

Minor (non-blocking) nits

• tools/test-local.sh:39 uses shasum (Perl-provided, not guaranteed on minimal Linux boxes); sha1sum is more universally present. Cosmetic — WHEELS_TEST_RESULT_FILE is the real escape hatch.
• The browser-runner catch (Module.cfc:8087) still prints the generic "Failed to reach test runner… Is the server running?" rather than the new timeout-specific guidance, even though it now passes the longer budget. Not a regression, just an inconsistency with the primary path — worth a follow-up.

Nice, tightly-scoped fix with honest rationale in the code comments and PR body. Approving.