chore(skills): harden pr-push pre-checks — orphan processes, scanner errors
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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 <PIDs>` 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
|
||||
|
||||
Reference in New Issue
Block a user