diff --git a/.claude/skills/pr-push/SKILL.md b/.claude/skills/pr-push/SKILL.md index f5c9adbcb..846e3b6da 100644 --- a/.claude/skills/pr-push/SKILL.md +++ b/.claude/skills/pr-push/SKILL.md @@ -91,7 +91,22 @@ Sprint 28 proved that code review without runtime testing misses critical bugs (parse errors, depth sorting, scene tree failures). **For client/visual branches:** + +First, **wipe the script class cache before parsing**. Sprint 36 close +caught this: the team added a new `class_name MetaScreen` base class and +six scripts extending it. Warm cache on developer machines parsed fine, +but CI / fresh clones / post-merge parses hit `Could not find base class +"MetaScreen"` because the autoload-vs-class_name registration order only +resolves correctly once the class cache is seeded. Wiping the cache here +(client-side, before push) simulates the cold-start path and catches the +bug locally — keeping the pre-push hook fast. + ```bash +# Cold-cache parse check. Deleting the cached class registry forces +# Godot to rebuild it from source on the next parse, matching the +# cold-start ordering CI and fresh clones see. +rm -f client/.godot/global_script_class_cache.cfg + # 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 @@ -109,6 +124,13 @@ timeout 10 godot --path client res://scenes/main_menu.tscn 2>&1 | \ grep -iE "^(SCRIPT )?ERROR|Parse Error|Export type" ``` +If the cold parse reports a "Could not find base class X" error, the fix +is almost always an autoload-order issue (see `CLAUDE.md` → GDScript +conventions → Autoload parse-order rule). Rebuilding the cache with +`godot --editor --headless --quit` will mask it locally but the same error +will re-surface post-merge — fix the actual ordering problem, don't paper +over it with a cache rebuild. + 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` diff --git a/.claude/skills/pr-review/SKILL.md b/.claude/skills/pr-review/SKILL.md index f2689655b..ed28b8c08 100644 --- a/.claude/skills/pr-review/SKILL.md +++ b/.claude/skills/pr-review/SKILL.md @@ -47,6 +47,28 @@ godot --headless --path client --quit 2>&1 | grep -i "SCRIPT ERROR" If script errors appear in the branch diff files, flag them immediately before spawning reviewers — no point reviewing code that doesn't parse. +#### 0b-i. Merge-path smoke test gate + +When the branch diff touches any of: +- pre-game flow (main menu → character creation → connect) +- scene transitions (`change_scene_to_file`, scene autoloads) +- save / load / new-game paths +- connection handshake (`sim_bridge`, protocol decode/encode) +- any code path executed in the first 30 seconds of a new session + +...the reviewer output MUST explicitly call out the state of author-side +manual smoke boxes in the PR test plan. If any merge-path smoke box is +unchecked, include a top-level note: + +> **Merge-path smoke not performed.** PR test plan has unchecked manual +> smoke box(es): [list]. A reviewer or the team must run the smoke before +> merge approval. Sprint 36 bug #872 (New Game hangs on 'connecting') +> landed exactly here — do not skip. + +Unchecked merge-path smoke boxes downgrade the verdict from APPROVED to +REQUEST_CHANGES even if reviewers have no code comments. The smoke is a +deliverable, not a suggestion. + ### 0c. Zero warnings check The project enforces a **zero warnings policy**. Before spawning reviewers,