PR-Review #463: an undeclared runspace-cancellation change shipped untested, and the gate.py test-file exclusion has no guard #472

PR: #463 — author: @Sev7eNup — status: merged

• high — src/NodePilot.Engine/PowerShell/RunspaceExecutionEngine.cs (ExecuteOnceAsync): an engine behaviour change rode along undeclared and without a test, and it shipped non-functional. The PR summary lists three changes (the "Test run" → "Run" rename, the llms.txt trailing slash, the gate.py hash). The diff additionally rewrites the cancellation path: a new 20 s StopGracePeriod after which the pipeline is abandoned and the step is reported cancelled while its statement may still be running, using var ps / using var results replaced by seven hand-placed Dispose() calls, and a new catch (OperationCanceledException) { throw; } that bypasses all of them. No test covers the new path — the only test edit raises a wait bound from 30 s to 90 s, which absorbs the symptom instead of asserting the behaviour. Fix the runspace cancellation grace period so it actually fires #465 then established that the grace timer never fired at all (token callbacks run sequentially on the cancelling thread, so a stuck ps.Stop() blocked the timer registered after it) and redid the whole change 30 minutes later, with tests that fail against the previous implementation. Suggestion: a change to engine control flow belongs in its own PR with its own test, and the PR description has to name every file in the diff — a reviewer reading this summary had no reason to look at the engine at all. The code residue itself is already resolved by Fix the runspace cancellation grace period so it actually fires #465; what is left to decide is how to keep an undeclared, untested engine change out of a doc/triage batch.
• medium — scripts/release-lab/computer-use/gate.py (inventory) and test_gate.py: the behaviour this PR changed is the one thing not pinned by a test. inventory() now excludes __tests__/** and *.test.tsx from uiSourceHash, so test-only UI changes stop invalidating the computer-use coverage review. test_gate.py already has test_catalog_source_hash_change_blocks, but nothing asserts the exclusion, so a later edit to inventory() can silently re-include test files and the gate goes back to tripping on every UI test change — with no failing test. Suggestion: add a case next to the existing hash test asserting that the hashed file list contains no __tests__ path and no *.test.tsx file (and, if cheap, that touching such a file leaves uiSourceHash unchanged).

Automatic review finding from the daily PR-review run — needs triage.

Generated by Claude Code