NS-21: fold test CI into gdscript.yml; document test-with-scripts policy

.cursor/rules/testing-expectations.md

@@ -1,5 +1,5 @@
 ---
-description: Unit vs integration tests; integration when data persistence/mutation exists; C# xUnit; Godot manual until harness.
+description: Unit vs integration tests; integration when data persistence/mutation exists; C# xUnit; Godot client unit tests with script changes (NS-21 harness + gdscript.yml CI).
 alwaysApply: true
 ---
 
@@ -23,8 +23,9 @@ alwaysApply: true
 
 ## Godot client (GDScript)
 
-- Until the project adopts a harness (**GUT**, Godot unit tests, or CI export checks), rely on **manual verification** for gameplay and scene changes.
+- **Harness (NS-21):** The **target** setup is a **unit test stack** under `client/` with **`client/README.md`** instructions and **`.github/workflows/gdscript.yml`** running **gdlint**, **gdformat**, and **headless unit tests** on the same path triggers. Until NS-21 is merged, CI may not run that suite yet—use **manual verification** and PR notes as today.
-- For non-trivial client work, note **how you verified** (steps or scenario) in the PR or story plan so reviewers can repeat it.
+- **Production script changes (after NS-21):** Any PR that **adds or changes** GDScript under **`client/scripts/`** (and any other **application** `.gd` outside **`client/addons/`** and **`client/test/`**) must **add or update** tests under **`client/test/`** in the same change set. If a change is **not** reasonably testable, say so in the PR (or story plan) with a short reason—do not skip tests silently.
+- **Scenes / assets / manual checks:** Scene tweaks, visuals, and flows that unit tests do not cover still deserve **manual verification** notes in the PR when behavior could regress.
 
 ## Cross-cutting
 

docs/plans/NS-21-implementation-plan.md

@@ -17,7 +17,7 @@
 
 - **Harness:** Add a maintained test stack (**GDUnit4** preferred for Godot 4.x headless + CI; **GUT** acceptable if GDUnit4 blocks on 4.6—decide during implementation and record in README). Vendor the addon under `client/addons/` (pinned release) so CI and clones match.
 - **Documentation:** `client/README.md` (or a short dedicated doc under `client/` linked from README) with **install** (if any beyond opening the project), **local run commands**, and how tests are laid out.
-- **CI:** At least one GitHub Actions job that runs the suite on **push/PR** to `main`, with **path filters** on `client/` (including `client/addons/` test plugin paths, `client/**/*.gd` tests, workflow file). Align **Godot binary version** with **4.6-stable** (same pattern as existing headless smoke notes in `client/README.md`).
+- **CI:** Extend **`.github/workflows/gdscript.yml`** only (no separate client test workflow): add a **job** (or extend the existing job) that downloads **Godot 4.6-stable** and runs the suite **headless**. **Path filters** on that workflow must include anything that should run tests—e.g. `client/scripts/**/*.gd`, `client/test/**/*.gd`, `client/addons/**`, `client/project.godot`, and the workflow file itself—so lint, format, and **unit tests** stay aligned whenever client code or tests change.
 - **Tests:** Cover current behavior where feasible for:
   - `client/scripts/player.gd` — public API: `snap_to_server`, `set_authoritative_nav_goal`, `clear_nav_goal`, and **physics-process outcomes** that can be asserted with a **minimal scene tree** (CharacterBody3D + child NavigationAgent3D matching `$NavigationAgent3D`) without the full `main.tscn`.
   - `client/scripts/position_authority_client.gd` — **HTTP flow** via a **test double** (subclass of `HTTPRequest` or injected node) that invokes `request_completed` with canned bodies/status codes: boot `GET` 200 → `authoritative_position_received` with `apply_as_snap == true`; `POST` 400 → `move_rejected`; happy `POST` 200 → `VERIFY_GET` → `GET` 200 → second arg `false`. Cover `_base_root` / URL trimming indirectly through successful request URLs if practical.
@@ -30,10 +30,12 @@
 - Replacing **server** tests or adding **E2E** browser/gameplay automation.
 - Changing **production** gameplay contracts (NS-16/19/23) except where a test exposes a real bug (unlikely in this story).
 
+**Ongoing policy (effective once this story merges):** Any change that **adds or modifies** production GDScript under **`client/scripts/`** (and any future app `.gd` outside **`client/addons/`** and **`client/test/`**) must **add or update** unit tests under **`client/test/`** in the same change set, unless the PR or plan **explicitly documents** a justified exception (e.g. generated or no-logic glue). **`client/README.md`** should state this expectation for contributors; **[`.cursor/rules/testing-expectations.md`](../../.cursor/rules/testing-expectations.md)** is updated to match.
+
 ## Acceptance criteria checklist
 
 - [ ] README or short doc under `client/` explains **install** (if needed) and **run** commands for the test suite.
-- [ ] At least **one CI workflow** runs tests on **PR/push** with **path filters** appropriate to `client/` and the new test assets.
+- [ ] **`.github/workflows/gdscript.yml`** runs **unit tests** (in addition to gdlint / gdformat) on **PR/push** with **path filters** that cover production scripts, **`client/test/`**, **`client/addons/`** (harness), **`client/project.godot`**, and the workflow file.
 - [ ] New tests cover **`player.gd`**, **`position_authority_client.gd`**, and **`ground_pick.gd`** where feasible; any **intentionally untested** surface (e.g. full `_input` ray pick, `main.gd`) is **briefly noted** in the plan or README.
 
 ## Technical approach
@@ -43,7 +45,7 @@
 3. **`player.gd`:** Build a small **SceneTree**-hosted rig in setup: `CharacterBody3D` + `NavigationAgent3D` named to match `player.gd`’s `$NavigationAgent3D`; call `snap_to_server` / goals; advance **physics frames** (`await` or GDUnit helpers) and assert position, `velocity`, and `_has_walk_goal` behavior where observable via public effects (arrival epsilon, clear goal).
 4. **`position_authority_client.gd`:** Replace or wrap **`HTTPRequest`** with a **double** that records `request()` args and simulates **`request_completed`**; drive `sync_from_server` / `submit_move_target` and assert signals and `_busy` / phase behavior.
 5. **`ground_pick.gd`:** Instantiate script on `Node3D`; build `StaticBody3D` (or plain `Node`) in/out of **`walkable`** group; use `call("_collider_is_walkable", collider)` **only if** needed for white-box tests, or test via a thin extracted helper if the team prefers no underscore calls (optional refactor—keep minimal).
-6. **CI:** Reuse the **official Linux 4.6-stable** Godot binary pattern from `client/README.md`; run the addon’s **headless test runner** CLI; fail the job on test failure. Optionally **merge** into `.github/workflows/gdscript.yml` as a second job or add `client-tests.yml`—whichever keeps caches and Godot download maintainable.
+6. **CI:** In **`.github/workflows/gdscript.yml`**, add a **test** step (new job or sequential step after lint—choose whichever keeps the YAML clear): download **Linux 4.6-stable** Godot per `client/README.md`, run the addon’s **headless test runner**, fail on test failure. **Extend the workflow’s `paths` / `paths-ignore`** so the workflow runs when tests, addons, `project.godot`, or production `.gd` under `client/` change.
 7. **Documentation:** Add a **“Automated tests (GDScript)”** section to `client/README.md` with exact commands.
 
 ## Files to add
@@ -60,14 +62,14 @@
 | Path | Rationale |
 |------|-----------|
 | `client/project.godot` | Enable **test plugin**; set **GDUnit4** (or GUT) project options / test root if required. |
-| `client/README.md` | Document **how to run tests locally** and CI pointer; note **`main.gd` / full pick** deferred. |
+| `client/README.md` | Document **how to run tests locally**, CI pointer, and **policy**: production GDScript changes require **matching test** adds/updates under `client/test/` (see **testing-expectations**); note **`main.gd` / full pick** deferred. |
-| `.github/workflows/gdscript.yml` and/or **new** `client`-focused workflow under `.github/workflows/` | Add job(s) to **download Godot 4.6** and **run the suite** with **path filters** covering `client/` (including addons + tests). |
+| `.github/workflows/gdscript.yml` | Add **Godot headless test run**; widen **path filters** so lint + format + tests run together whenever relevant `client/` assets change. |
 
 ## Tests
 
 | Test file | What it covers |
 |-----------|----------------|
-| `client/test/player_test.gd` | `snap_to_server` resets transform/velocity and nav target; `set_authoritative_nav_goal` / `clear_nav_goal`; movement toward goal / arrival within **ARIVE_EPS** / **VERT_ARRIVE_EPS** (using minimal tree + physics stepping). |
+| `client/test/player_test.gd` | `snap_to_server` resets transform/velocity and nav target; `set_authoritative_nav_goal` / `clear_nav_goal`; movement toward goal / arrival within **ARRIVE_EPS** / **VERT_ARRIVE_EPS** (using minimal tree + physics stepping). |
 | `client/test/position_authority_client_test.gd` | Boot `GET` success → `authoritative_position_received(..., true)`; `POST` `400` + JSON → `move_rejected`; `POST` `200` + verify `GET` `200` → `authoritative_position_received(..., false)`; malformed JSON does not emit position. |
 | `client/test/ground_pick_test.gd` | `_collider_is_walkable` true when ancestor in **`walkable`** group, false otherwise; document in-file if ray stepping remains manual-only. |