Watch

fix(control): reject non-finite CLI numbers before connecting #633

Open

NitnekLeBorgne wants to merge 1 commit from NitnekLeBorgne/Longshot-contributions:fix/ctl-finite-input-20261115 into main

NitnekLeBorgne commented

2026-11-15 01:22:47 +00:00

First-time contributor

Type: fix · Risk: low · Tested on: Linux, game-free Python fixtures (no GPU) · Needs: complete game-free suite and native Windows CLI validation

Why

A mistyped timeout such as nan, inf or 1e999 is accepted by the control CLI. It can reach a connection and produce non-standard JSON numbers or a non-finite reply deadline instead of a clear usage error. Stick coordinates have the same problem.

This is limited to validating these CLI numbers before connecting. It does not change the raw-command escape hatch, server validation or the separate event/deadline work in PR #538.

How to reproduce

Seen on: Linux, game-free source fixtures; no game or GPU involved.
Settings and switches: no live control endpoint is required for the fixed command.
Steps: run python -I -B scripts/longshot_ctl.py wait in_play --timeout nan. A test connection spy demonstrates that main reaches connect; an in-memory transport demonstrates NaN/Infinity serialization and a non-finite read deadline. The fixture response is synthetic, not a game response.
Expected: argparse reports invalid usage, exit 2, before any connection.
Actual: on main the non-finite argument reaches the connection path. The real transport's eventual error is not claimed or exercised here.

What changed

Reject non-finite CLI numbers before opening a connection. Finite values and the raw-command escape hatch keep their existing behavior.

scripts/longshot_ctl.py: one argparse conversion checks math.isfinite, used for the affected timeout/arrival arguments and both stick axes.
tests/test_longshot_ctl_finite.py: rejected inputs must not connect; finite values and normal command routing remain covered using memory-only mocks.
Finite values, including existing zero/negative semantics, are unchanged. No new magnitude limit, timeout renewal policy, launch/session behavior or bridge protocol is introduced.

Settings scope: No new flags or settings. Only non-finite values of existing numeric options are rejected earlier.

Measurements

Verdict: no new regression in the exact published-head/parent comparison; existing main failures are explicitly retained.

Hardware: Linux x86-64, 6 logical CPUs, 8074 MiB RAM reported by the live host; no GPU or GPU driver used for these game-free checks; GCC 13.3.
Scene: none; default upstream public fixtures, isolated HOME/XDG/TMP, no human saves or game execution.
Method: one complete normal default runner execution per exact commit, python -I -B tests/run_all.py --jobs 3; compare every check status, skip reason and failure signature. Not a performance median or timing claim.
How to reproduce: use the exact parent bbe6fdf23170310117c714958248e60ec0686a62 and published head cea1455ed65e0caa9001bac87929c50a4bfd0716 in the same recorded isolated environment. Keep AGENTS.md off disk and supplement its optional source scan read-only in memory from the public Git blob. Expect the named existing main failures and default skips below, not a wholly green suite.

	Before: main @ `bbe6fdf231`	After: this PR @ `cea1455ed6`
Complete runner checks	606	607
Passed	507	508
Failed, named below	14	14, same failure signatures
Default skips	85	85
Required C++20 checks	4 passed	4 passed

Both full-runner exits are 1; this is not a green CI claim. Existing main failures: tests/test_appimage_player_build.py, tests/test_bake_portability.py, tests/test_crash_dialog.py, tests/test_env_on.py, tests/test_lift_cleanup_error.py, tests/test_make_game_cache.py, tests/test_missing_nup.py, tests/test_mods_size_table.py, tests/test_options_budget_once.py, tests/test_pipeline_tour.py, tests/test_readback_late.py, tests/test_vk_occlusion_queries.py, tests/test_vk_pgo.py, tests/test_walk_key.py.

Source and modes unchanged; owned process groups empty. AGENTS.md was never materialized; two unchanged SwitchesRemoved methods passed with its public blob added only in memory on each exact commit. This declared source-only context is not literal exact-tree CI equivalence. The comparison and complete native stdout/stderr are retained.

Historical focused regression evidence

The original focused evidence below keeps its original commit labels. It is historical, not extra full-runner executions on the new parent.

Verdict: the same frozen regression test fails on main and passes with this patch. On main, 42 failing subcases belong to one of three unittest methods; the other two control methods pass. On the candidate all three methods pass. A separate actual CLI invocation returns usage exit 2 with argument --timeout: expected a finite number.

Hardware: Linux game-free fixtures; no GPU/driver used; CPU and RAM not reported in these receipts.
Scene: Synthetic regression/control inputs, not a game scene.
Method: Functional outcomes from the adopted receipts below, not timing benchmarks; no fabricated repetitions or medians.
How to reproduce: Replay the named focused tests described below on the pinned baseline and candidate with identical fixtures.

	Before: main @ `6c9edffe5d`	After: this PR @ `4ff1db9746`
New unittest methods	42 failing subcases within one method; two control methods pass	All three methods pass

Baseline: main 6c9edffe5da1f5b5383977aeab3f544c0de25d4c. Separate base/candidate witnesses, source/test hashes before and after execution, complete stdout/stderr, native exits and process completion were retained. These were new qualification runs; an initial evidence review requested stronger source-to-run binding, which was supplied without changing the patch.

Two existing targeted methods also pass: GamePaths.test_builder_and_development_layouts and LongshotCtl.test_input_waits_as_long_as_the_bridge. The latter was invoked without its class setup to avoid starting the fake game/socket. This is not a full execution of test_longshot_ctl.py.

No performance or live-transport measurement is claimed; this is parser validation.

Flags and settings

None

Testing

Tested: Linux game-free fixtures: the three new unittest methods with identical test bytes on base and candidate; finite-routing and watch controls; the two existing targeted methods named above; actual argparse usage diagnostic/exit 2. Independent static SPEC and QUALITY reviews completed after the evidence amendment. Neither reviewer claimed a test replay. A private clean patch application reproduces both candidate files exactly.

Not tested: the complete python -I -B tests/run_all.py suite, native Windows, a running game, real sockets/pipes, device input, GPU behavior and exceptionally large but finite transport timeouts. Raw JSON intentionally remains raw. This PR stays WIP; focused tests are not global coverage.

Must be tested:

Windows: in an isolated test folder, run the new test module and confirm invalid timeout/axis inputs exit 2 before opening a transport; verify ordinary finite command routing (by hand; no game needed), expecting invalid inputs to exit 2 before transport creation and finite routing to remain unchanged (by hand: an equipped contributor/tester inspects diagnostics and the stated outcomes before merge; not executed by this documentation edit).

Risks

What could break: a caller deliberately passing non-standard NaN/infinity values to these CLI arguments now receives usage exit 2. Valid finite argument routing is preserved.
Turning it off: revert the commit.
Compatibility: no save, ini, shader/pipeline cache, renderer ABI, authentication or server-side changes.

Checklist

Everyone

One topic.
The title is a Conventional Commit (type(scope): description) of the status line's type.
Why gives the reason plainly and links the issues that fit (the tracker searched).
Measurements has a verdict, or one line saying why there is nothing to measure.
Testing names what was tested, what wasn't, and what must still be tested, where and how.
Rebased on current main.
python -I -B tests/run_all.py passes (any failure that also fails on main is named).
No game files, dumps, build output or generated files.
third_party/ untouched (a new folder with a PROVENANCE.md instead).
The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

When it applies

AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
A game-free test added for the fix, failing on main and passing here.

Readiness provenance and checklist notes

Current main in the frozen public readback is aa95d35f3d021b27328b6d68109666a46c340aee; this PR head is 4ff1db97467401be53df3809e2396d3a8cfc6d69. The current-main box is unchecked: historical publication/base checks are not proof of current ancestry. Current-main updating and revalidation are needed before ready, but rebase/force-push/merge are not authorized; no such operation is promised or performed. The full runner box remains unchecked; no full-suite run or Windows/host qualification is claimed by this description edit. The normal project pr ready remains blocked and must not be forced. Manual pre-merge plans are future work, not completed evidence.

Historical checklist explanations (not current-main qualification):

The title is a Conventional Commit (type(scope): description) of the status line's type, after the WIP prefix.
Why gives the reason plainly and links the issues that fit (the open tracker and nearby work searched; PR #538 is distinct).
Rebased on current main (the exact unchanged base is checked again before publication).
python -I -B tests/run_all.py passes (not run; focused coverage only; WIP).
third_party/ untouched.
The license is OK: GPL-3.0-or-later; original small parser/test changes, nothing incompatible or decompiled.

Current completion: earlier limited-scope, not-rebased or no-full-run wording describes the original historical phase and is superseded by the exact published-head/parent qualification above. No present prohibition on authorized own-branch rebase is claimed. Remaining manual Windows/game/hardware plans are pre-merge work where not an applicable author checklist obligation; none is claimed executed.

**Type:** fix · **Risk:** low · **Tested on:** Linux, game-free Python fixtures (no GPU) · **Needs:** complete game-free suite and native Windows CLI validation ## Why A mistyped timeout such as `nan`, `inf` or `1e999` is accepted by the control CLI. It can reach a connection and produce non-standard JSON numbers or a non-finite reply deadline instead of a clear usage error. Stick coordinates have the same problem. This is limited to validating these CLI numbers before connecting. It does not change the raw-command escape hatch, server validation or the separate event/deadline work in PR #538. ## How to reproduce **Seen on:** Linux, game-free source fixtures; no game or GPU involved. **Settings and switches:** no live control endpoint is required for the fixed command. **Steps:** run `python -I -B scripts/longshot_ctl.py wait in_play --timeout nan`. A test connection spy demonstrates that main reaches `connect`; an in-memory transport demonstrates `NaN`/`Infinity` serialization and a non-finite read deadline. The fixture response is synthetic, not a game response. **Expected:** argparse reports invalid usage, exit 2, before any connection. **Actual:** on main the non-finite argument reaches the connection path. The real transport's eventual error is not claimed or exercised here. ## What changed Reject non-finite CLI numbers before opening a connection. Finite values and the raw-command escape hatch keep their existing behavior. - `scripts/longshot_ctl.py`: one argparse conversion checks `math.isfinite`, used for the affected timeout/arrival arguments and both stick axes. - `tests/test_longshot_ctl_finite.py`: rejected inputs must not connect; finite values and normal command routing remain covered using memory-only mocks. - Finite values, including existing zero/negative semantics, are unchanged. No new magnitude limit, timeout renewal policy, launch/session behavior or bridge protocol is introduced. Settings scope: No new flags or settings. Only non-finite values of existing numeric options are rejected earlier. ## Measurements **Verdict:** no new regression in the exact published-head/parent comparison; existing main failures are explicitly retained. Hardware: Linux x86-64, 6 logical CPUs, 8074 MiB RAM reported by the live host; no GPU or GPU driver used for these game-free checks; GCC 13.3. Scene: none; default upstream public fixtures, isolated HOME/XDG/TMP, no human saves or game execution. Method: one complete normal default runner execution per exact commit, `python -I -B tests/run_all.py --jobs 3`; compare every check status, skip reason and failure signature. Not a performance median or timing claim. How to reproduce: use the exact parent `bbe6fdf23170310117c714958248e60ec0686a62` and published head `cea1455ed65e0caa9001bac87929c50a4bfd0716` in the same recorded isolated environment. Keep AGENTS.md off disk and supplement its optional source scan read-only in memory from the public Git blob. Expect the named existing main failures and default skips below, not a wholly green suite. | | Before: main @ bbe6fdf23170310117c714958248e60ec0686a62 | After: this PR @ cea1455ed65e0caa9001bac87929c50a4bfd0716 | |---|---|---| | Complete runner checks | 606 | 607 | | Passed | 507 | 508 | | Failed, named below | 14 | 14, same failure signatures | | Default skips | 85 | 85 | | Required C++20 checks | 4 passed | 4 passed | Both full-runner exits are 1; this is not a green CI claim. Existing main failures: `tests/test_appimage_player_build.py`, `tests/test_bake_portability.py`, `tests/test_crash_dialog.py`, `tests/test_env_on.py`, `tests/test_lift_cleanup_error.py`, `tests/test_make_game_cache.py`, `tests/test_missing_nup.py`, `tests/test_mods_size_table.py`, `tests/test_options_budget_once.py`, `tests/test_pipeline_tour.py`, `tests/test_readback_late.py`, `tests/test_vk_occlusion_queries.py`, `tests/test_vk_pgo.py`, `tests/test_walk_key.py`. Source and modes unchanged; owned process groups empty. AGENTS.md was never materialized; two unchanged SwitchesRemoved methods passed with its public blob added only in memory on each exact commit. This declared source-only context is not literal exact-tree CI equivalence. The comparison and complete native stdout/stderr are retained. ### Historical focused regression evidence The original focused evidence below keeps its original commit labels. It is historical, not extra full-runner executions on the new parent. **Verdict:** the same frozen regression test fails on main and passes with this patch. On main, 42 failing subcases belong to one of three unittest methods; the other two control methods pass. On the candidate all three methods pass. A separate actual CLI invocation returns usage exit 2 with `argument --timeout: expected a finite number`. Hardware: Linux game-free fixtures; no GPU/driver used; CPU and RAM not reported in these receipts. Scene: Synthetic regression/control inputs, not a game scene. Method: Functional outcomes from the adopted receipts below, not timing benchmarks; no fabricated repetitions or medians. How to reproduce: Replay the named focused tests described below on the pinned baseline and candidate with identical fixtures. | | Before: main @ 6c9edffe5da1f5b5383977aeab3f544c0de25d4c | After: this PR @ 4ff1db97467401be53df3809e2396d3a8cfc6d69 | |---|---|---| | New unittest methods | 42 failing subcases within one method; two control methods pass | All three methods pass | Baseline: main `6c9edffe5da1f5b5383977aeab3f544c0de25d4c`. Separate base/candidate witnesses, source/test hashes before and after execution, complete stdout/stderr, native exits and process completion were retained. These were new qualification runs; an initial evidence review requested stronger source-to-run binding, which was supplied without changing the patch. Two existing targeted methods also pass: `GamePaths.test_builder_and_development_layouts` and `LongshotCtl.test_input_waits_as_long_as_the_bridge`. The latter was invoked without its class setup to avoid starting the fake game/socket. This is not a full execution of `test_longshot_ctl.py`. No performance or live-transport measurement is claimed; this is parser validation. ### Flags and settings None ## Testing **Tested:** Linux game-free fixtures: the three new unittest methods with identical test bytes on base and candidate; finite-routing and watch controls; the two existing targeted methods named above; actual argparse usage diagnostic/exit 2. Independent static SPEC and QUALITY reviews completed after the evidence amendment. Neither reviewer claimed a test replay. A private clean patch application reproduces both candidate files exactly. **Not tested:** the complete `python -I -B tests/run_all.py` suite, native Windows, a running game, real sockets/pipes, device input, GPU behavior and exceptionally large but finite transport timeouts. Raw JSON intentionally remains raw. This PR stays WIP; focused tests are not global coverage. **Must be tested:** - Windows: in an isolated test folder, run the new test module and confirm invalid timeout/axis inputs exit 2 before opening a transport; verify ordinary finite command routing (by hand; no game needed), expecting invalid inputs to exit 2 before transport creation and finite routing to remain unchanged (by hand: an equipped contributor/tester inspects diagnostics and the stated outcomes before merge; not executed by this documentation edit). ## Risks **What could break:** a caller deliberately passing non-standard NaN/infinity values to these CLI arguments now receives usage exit 2. Valid finite argument routing is preserved. **Turning it off:** revert the commit. **Compatibility:** no save, ini, shader/pipeline cache, renderer ABI, authentication or server-side changes. ## Checklist ### Everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`) of the status line's type. - [x] Why gives the reason plainly and links the issues that fit (the tracker searched). - [x] Measurements has a verdict, or one line saying why there is nothing to measure. - [x] Testing names what was tested, what wasn't, and what must still be tested, where and how. - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` passes (any failure that also fails on `main` is named). - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### When it applies - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] A game-free test added for the fix, failing on `main` and passing here. ## Readiness provenance and checklist notes Current main in the frozen public readback is `aa95d35f3d021b27328b6d68109666a46c340aee`; this PR head is `4ff1db97467401be53df3809e2396d3a8cfc6d69`. The current-main box is unchecked: historical publication/base checks are not proof of current ancestry. Current-main updating and revalidation are needed before ready, but rebase/force-push/merge are not authorized; no such operation is promised or performed. The full runner box remains unchecked; no full-suite run or Windows/host qualification is claimed by this description edit. The normal project pr ready remains blocked and must not be forced. Manual pre-merge plans are future work, not completed evidence. Historical checklist explanations (not current-main qualification): - The title is a Conventional Commit (`type(scope): description`) of the status line's type, after the WIP prefix. - Why gives the reason plainly and links the issues that fit (the open tracker and nearby work searched; PR #538 is distinct). - Rebased on current `main` (the exact unchanged base is checked again before publication). - `python -I -B tests/run_all.py` passes (not run; focused coverage only; WIP). - `third_party/` untouched. - The license is OK: GPL-3.0-or-later; original small parser/test changes, nothing incompatible or decompiled. Current completion: earlier limited-scope, not-rebased or no-full-run wording describes the original historical phase and is superseded by the exact published-head/parent qualification above. No present prohibition on authorized own-branch rebase is claimed. Remaining manual Windows/game/hardware plans are pre-merge work where not an applicable author checklist obligation; none is claimed executed.

NitnekLeBorgne added 1 commit

2026-11-15 01:22:47 +00:00

fix(control): reject non-finite CLI numbers before connecting

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

PR description / Description follows the template (pull_request_target) Successful in 5s

Details

CI / Conventional Commit title (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled

Details

CI / Workflow lint (actionlint) (pull_request) Has been cancelled

Details

Third-party parts / As their notes say (pull_request) Has been cancelled

Details

Third-party parts / The same as upstream (pull_request) Has been cancelled

Details

4ff1db9746

Reject NaN and infinities during argument parsing before bridge I/O.
Preserve finite routing and the intentionally raw command contract.
Add focused game-free regression and finite-routing controls.

Assisted-by: Hermes Agent (OpenAI GPT-6.1-sol)
Co-Authored-By: Claudia (OpenAI GPT-6.1-sol)

mooki added the

labels

2026-11-15 13:56:43 +00:00

HumanBean force-pushed fix/ctl-finite-input-20261115 from 4ff1db9746

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

PR description / Description follows the template (pull_request_target) Successful in 5s

Details

CI / Conventional Commit title (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled

Details

CI / Workflow lint (actionlint) (pull_request) Has been cancelled

Details

Third-party parts / As their notes say (pull_request) Has been cancelled

Details

Third-party parts / The same as upstream (pull_request) Has been cancelled

Details

to cea1455ed6

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped

Details

Third-party parts / The same as upstream (pull_request) Has been skipped

Details

Website / Website on Cloudflare (pull_request) Has been skipped

Details

CI / Conventional Commit title (pull_request) Successful in 3s

Details

CI / Workflow lint (actionlint) (pull_request) Successful in 5s

Details

Guide screenshots / Guide images and local UI source review (pull_request) Failing after 5s

Details

Third-party parts / As their notes say (pull_request) Successful in 8s

Details

PR description / Description follows the template (pull_request_target) Waiting to run

Details

2026-11-15 18:42:34 +00:00

Compare

HumanBean commented

2026-11-15 18:45:29 +00:00

Owner

Rebased onto main @ bbe6fdf: no conflicts. No tests were run here; the test boxes stay yours.

Rebased onto `main` @ bbe6fdf: no conflicts. No tests were run here; the test boxes stay yours.

NitnekLeBorgne changed title from ~~WIP: fix(control): reject non-finite CLI numbers before connecting~~ to fix(control): reject non-finite CLI numbers before connecting

2026-11-15 19:27:06 +00:00