fix(gateway-test): preserve real SIGTERM coverage on Windows - #89
Merged
Merged
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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 |
PR Summary by QodoPreserve real gateway SIGTERM test coverage on Windows
AI Description
Diagram
High-Level Assessment
Files changed (10)
|
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Collaborator
Author
|
@greptileai Please review the complete current PR head 907590c. The previous credential finding is fixed: service URLs now travel only through the child environment, while Docker arguments contain variable names. Please verify the final diff and report any remaining actionable findings. |
edwardnewgate710
pushed a commit
that referenced
this pull request
Oct 3, 2026
Resolves docs/PROJECT_STATE.md only: #89's M15 Increment 87 entry and header lines are kept exactly; this PR's entry and its own header lines are renumbered to M15 Increment 88.
edwardnewgate710
pushed a commit
that referenced
this pull request
Oct 3, 2026
sayed710
added a commit
that referenced
this pull request
Oct 4, 2026
* fix(web): enforce player ownership on board interaction BoardInteraction fell back to the side to move when no player colour was given, and the game route mounted its board without one and enabled input whenever the game was not over. Spectators could select, drag and premove; the board was interactive before the joined role arrived; an off-turn player could pick up the opponent's pieces and queue premoves with them; and a finished-game join briefly enabled input. - Ownership is explicit: a colour, null (nobody) or 'side-to-move' (boards without players only). setPlayerColor drops the selection, a pending promotion and queued premoves on a real change; drop() re-checks the origin's owner. Legality stays oracle-driven for every variant. - BoardView.setPlayerColor closes an open promotion chooser and abandons a drag; focus, keyboard navigation and flip are untouched. mountBoard exposes it with the same churn guard as setInputEnabled. - The game route mounts fail-closed and derives owner and input from one GameSync snapshot from both onColor and onActionState, so callback order cannot open a window; a finished game latches. Tests: core, view and route suites plus a backend Playwright spec with a real two-player game and a spectator (click, keyboard and drag). The finished-board step of illegal-move-feedback.spec.ts relied on the old fallback and now tries White's own premove. * docs: record M15 Increment 87 player board ownership * fix(web): drop the trailing click of a gesture cut short by an owner change Qodo on 6068e0b: BoardView.setPlayerColor cancelled an in-progress pointer gesture, which removed its pointer-up handler, so the gesture's trailing click reached tap() and could select the new owner's piece (e.g. a swipe begun before `joined` landed). An owner change during a gesture now suppresses that click, and every pointerdown resets the suppression so an abandoned gesture cannot swallow the next real click. * docs: record the Increment 87 review correction and its validation * fix(web): clear click suppression when the release that set it makes no click Greptile on a623b11: a gesture cut short by an owner change and released off the board left suppressClick set, so the next click with no pointer events (assistive technology, programmatic activation) was swallowed. An ordinary drag dropped off the board leaked the same way. Suppression now covers only the click the release itself produces: it clears on the next tick, since the browser dispatches that click straight after pointer-up. An owner change waits for the cut-short gesture's release (or pointercancel) instead of setting a sticky flag; the wait ends at that release, at the next pointerdown, or on destroy. * docs: record the Greptile correction and Linux backend validation for Increment 87 * docs: correct Increment 87 over-claims and record the exact-head review * fix(web): tie click suppression to the pointer that produced the click Qodo and Greptile on 38e1bdd: clearing suppression on a next-tick timer assumed a release's click arrives in the same task, which touch does not guarantee, so a late touch click from a completed drag or from a gesture an owner change cut short could still act. Qodo also found that the cut-short wait ended on any pointer's release, so a second finger lifting first let the original finger's click through. Suppression now names the pointer whose next click must be swallowed and matches the click's own pointerId whenever it arrives. A click with no pointer behind it (pointerType '', e.g. assistive technology) is never swallowed. The wait ends only on that pointer's release or cancel, a new press by the same pointer, or destroy; a new press by the same pointer also clears a stale entry from a release that made no click. Engines whose clicks carry no pointer fields keep suppressing the release's own click. * docs: record the pointer-matched suppression fix and its Linux validation * fix(web): bound the no-pointer-id click fallback and ignore other pointers' drag releases Independent exact-head review of 79686a5: - On an engine whose click carries no pointerId, a stale suppression (e.g. a touch drag released off the board, which makes no click) matched the next tap's click and swallowed it once. In that fallback a click now counts as the release's own only if no press came after the release (a press counter, not a timer). Engines with pointer ids are unchanged. - The drag's pointerup listener accepted any pointer, so another finger's release could drop the carried piece at its own coordinates. It now ignores releases from other pointers. * docs: record the bounded click fallback and its Linux validation * fix(web): end a drag on its own pointercancel and on a new press Independent exact-head review of 066f046: drags never listened for pointercancel, so a touch drag the browser took over (a pan, a system gesture) stayed live with its floating piece; after the drag's pointerup started ignoring other pointers, no later event recovered it. A new press during a live drag also dropped the old drag's listeners without removing its floating piece. A drag now cancels on its own pointer's pointercancel, like a drop off the board (selection cleared, floating piece removed). A press that never became a drag keeps an existing selection. A new press calls cancelDrag() before starting, so no floating piece is left behind. * docs: record the drag cancellation fix and its Linux validation * fix(web): a new press abandons another pointer's gesture without acting on it Independent exact-head review of dd52355: a press during another pointer's live drag only detached that drag's listeners. Its selection stayed, its release went unwatched, and its trailing click reached tap() with the piece still selected, so an abandoned drag could submit a move. Owner changes and new presses now share abandonGesture(): wait for the abandoned pointer's release and swallow its click, and undo a drag it started (selection cleared, re-rendered). A re-press by the same pointer (its release was lost) only resets, so its own click still works. * docs: record the abandoned-gesture fix and its Linux validation * fix(web): track abandoned-gesture waits and click suppressions per pointer Independent exact-head review of b1441a4: a single wait slot and a single suppression slot meant a third simultaneous pointer could defeat them (abandoning A replaced the pending wait on C, so C's click still tapped), and setInputEnabled(false) still only cancelled a drag. Waits are now a set of awaited pointer ids served by one window listener pair (attached while any is awaited), and suppressions a map of pointer id to the press count at its release. Each pointer's release, click, re-press and the legacy no-pointer-id fallback work as before, per pointer. setInputEnabled(false) now abandons the gesture like an owner change. The PR #87 test that asserted no pointerup listener after a game ends now asserts the new contract: drag listeners go at once, and only the wait for the abandoned release remains until that release. * docs: record per-pointer gesture tracking and its Linux validation * fix(web): tie a pointer-less click to the last released pointer Greptile on 3bd6965: on an engine whose clicks carry no pointerId, the fallback matched clicks with a press count shared by all fingers. If finger A was abandoned after finger B pressed and A released without a click, A's suppression was recorded at B's press count and B's genuine tap was swallowed. A click without a pointer id is now taken to be the last released pointer's (a browser dispatches a click straight after its own pointer's release) and is swallowed only if that pointer is suppressed; the shared press counter is gone. An awaited pointer's pointercancel now ends its wait without recording a suppression, since a cancelled pointer never clicks (the optional LOW from the independent review of 3bd6965). Engines with pointer ids are unchanged. * docs: record the last-release click fallback and its Linux validation * fix(web): bound click records and put abandoned clicks first on engines without pointer ids Qodo on 8314d11: off-board drops (and, earlier, cancelled pointers) recorded click suppressions that no click could ever consume, and touch pointer ids are never reused, so records grew for the life of the board. Releases off the board now record nothing, and pending records are capped (oldest evicted), which also bounds on-board touch drags that make no click. Greptile on 8314d11: on an engine whose clicks carry no pointerId, finger A's abandoned gesture could produce a delayed click after finger B's release, and the last-released rule attributed it to B, so it acted. Without a pointer id a click cannot be attributed, so a choice is unavoidable: safety first. While an abandoned gesture's click is pending, a pointer-less click is taken to be that one (an abandoned gesture never acts; at worst one genuine tap is swallowed and repeated). Drag-release suppressions keep the last-released rule. Engines with pointer ids are unchanged. Tests now create stale entries with on-board no-click releases so every protection stays reachable; a redundant re-add line and a dead write are removed. * docs: record bounded click records and the safety-first pointer-less click rule * fix(web): only recent releases can claim a pointer-less click Independent exact-head review of 39c2f9d: on an engine whose clicks carry no pointerId (reportedly including Safari/iOS, unverified), an abandoned touch that never produced a click left an abandoned record with no expiry, so each later genuine tap was swallowed against one stale record, up to the cap: dead taps spread over any amount of time. Records now carry their release's time, and on the pointer-less path records older than CLICK_WINDOW_MS (1 s: a release's click comes within milliseconds, or ~300 ms when touch holds it for double-tap detection) are dropped before matching. Times come from the events' own timeStamp. Engines with pointer ids are unchanged. * docs: record time-bounded pointer-less click matching and its Linux validation * test(web): pin the pointer-less click window from both sides Independent exact-head review of 242ad1c: only one test used explicit event times, so shrinking CLICK_WINDOW_MS (e.g. to 50 ms) or flipping its comparison went unnoticed. An abandoned release's click 300 ms later must still be swallowed, and a click just past the window must act. * docs: record the pinned click-window tests and their Linux validation * docs: record the merge with #89 and its Linux validation * fix(web): an abandoned release swallows every pointer-less click in its window On engines whose clicks carry no pointer id, a newer finger's click could use up an abandoned finger's record before that finger's delayed click arrived, letting the abandoned click act. The record now stays until its window closes, so both ambiguous clicks are swallowed in either order. * docs(web): say when a suppressed click keeps its record * docs: record the abandoned-click fix and its Linux validation * docs: keep one current Increment 88 line in the project-state header --------- Co-authored-by: Hussein Mohamed <americanopbr@gmail.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
2 participants
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.
Windows
child.kill('SIGTERM')force-terminates the trust worker without delivering its JavaScript signal handler, so the existing integration test fails withnull !== 0. Gatewaynpm testnow routes the same complete suite through Linux Docker on Windows and runs it directly on POSIX hosts and Ubuntu CI. The signal test remains a single discovered copy and requires the handler's structured log, exit code 0 and no terminating signal.The test image follows the existing gateway dependency/build order. The topology guard verifies runtime discovery, Linux CI execution and change-filter coverage. Eight faulty alternatives are rejected by routing/source/discovery guards; five real Linux runtime mutants also fail, including forced kill and missing handler execution. Production shutdown behavior is unchanged.
Validation (sequential, zero unexpected skips):
null !== 0, no shutdown-handler log.check:*guards.Separate prerequisite from the frozen Arena work. Append-only Increment 87 and running/test architecture documentation included. Owner performs the manual merge; this PR must remain open until the exact-head CI, Qodo, Greptile and review-thread gates are satisfied.