From 804bba6ae82beb3958a5233cae82006eb3072b1a Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 20 Apr 2026 20:27:23 +0200 Subject: [PATCH] =?UTF-8?q?chore(skills):=20harden=20pr-push=20pre-checks?= =?UTF-8?q?=20=E2=80=94=20orphan=20processes,=20scanner=20errors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Sprint 36 lessons folded into the pr-push skill's pre-push workflow. 1a (new, mandatory). Orphan Godot process check. `ps -eo pid,etimes,cmd | awk` filter for `godot.*gdunit4-run` processes running longer than 5 minutes. Ask the user before killing. Blocks Sprint 36's failure mode where stale background test-runner invocations (from an earlier hung run) silently wedged fresh test runs by stealing CPU — an hour of verification time lost to exactly this. 1c (widened). Headless parse + scanner check. The old grep was `grep -i "SCRIPT ERROR"`, which missed Godot's resource scanner category errors like "Export type can only be built-in, a resource, a node, or an enum" — those surface as plain `ERROR` lines, not prefixed `SCRIPT ERROR`. Widened to `grep -iE "^(SCRIPT )?ERROR|Parse Error|Export type"` and filtered against the known pre-existing autoload class_name parse-order noise (Messagepack, LocalBridge, ServerProcess, Constants — per CLAUDE.md's documented trap). Commit 84105916 shipped an `@export var descriptor: CharacterVisualDescriptor` issue that the narrower grep missed; Tyre caught it five commits later. Co-Authored-By: Claude Opus 4.6 --- .claude/skills/pr-push/SKILL.md | 44 ++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/.claude/skills/pr-push/SKILL.md b/.claude/skills/pr-push/SKILL.md index b488643e9..f5c9adbcb 100644 --- a/.claude/skills/pr-push/SKILL.md +++ b/.claude/skills/pr-push/SKILL.md @@ -34,6 +34,28 @@ git branch --show-current If on `main`, stop: "You're on main. Switch to a team branch first." +### 1a. Orphan process check (MANDATORY) + +Stale Godot processes from prior test runs compete with fresh runs for CPU +and can silently wedge test-runner invocations. Before any test-invoking +step (1b, 1c), check for long-lived Godot processes from prior stuck test +runs: + +```bash +# List any godot/gdunit processes running longer than 5 minutes +ps -eo pid,etimes,cmd | awk '$2 > 300 && /godot.*gdunit4-run/ {print $1, $2"s", substr($0, index($0,$3))}' +``` + +If any are listed: they are almost certainly orphans from a prior test +run that hung. Ask the user before killing — they may be intentional. +Default: offer to `kill ` and wait a few seconds for the processes +to exit before proceeding. Re-run the check until empty. + +**Do not** proceed to 1b/1c with orphan Godot processes alive — they will +steal CPU from the fresh runs and may cause the new invocation to hang +indefinitely (Sprint 36 lost an hour of test verification to this exact +failure mode). + ### 1b. Zero warnings policy (MANDATORY) Before pushing, verify the branch has **zero lint warnings**. Any warning @@ -70,13 +92,29 @@ bugs (parse errors, depth sorting, scene tree failures). **For client/visual branches:** ```bash -# Headless parse check -godot --headless --path client --quit 2>&1 | grep -i "SCRIPT ERROR" +# Headless parse + scanner check. Godot's resource scanner emits +# category errors (e.g. "Export type can only be built-in, a resource, +# a node, or an enum" for @export on a RefCounted) that do NOT always +# prefix with SCRIPT ERROR — they appear as plain ERROR lines. Widen +# the grep to catch both, then filter known pre-existing noise from +# the autoload class_name parse-order trap (documented in CLAUDE.md). +godot --headless --path client --quit 2>&1 | \ + grep -iE "^(SCRIPT )?ERROR|Parse Error|Export type" | \ + grep -v "Failed loading resource: res://assets" | \ + grep -v "Cannot infer the type" | \ + grep -vE "(Messagepack|LocalBridge|ServerProcess|Constants)\" not declared" # If the branch has UI changes, also run the game briefly: -timeout 10 godot --path client res://scenes/main_menu.tscn 2>&1 | grep -i "ERROR\|SCRIPT ERROR" +timeout 10 godot --path client res://scenes/main_menu.tscn 2>&1 | \ + grep -iE "^(SCRIPT )?ERROR|Parse Error|Export type" ``` +Any lines that come through the filter represent new errors introduced +by this branch. Fix them before pushing — Sprint 36 shipped commit +`84105916` with an `@export var descriptor: CharacterVisualDescriptor` +scanner error that the old narrower grep missed; Tyre caught it five +commits later during W6 review. + **For server branches:** ```bash cd server && cargo test --lib 2>&1