chore(skills): workflow skills sweep — whats-next/workshop-start/pr-review/pr-process (T-1102)

De-sprint pr-review, dynamic repo-root paths, gate-aligned checks; workshop-start Agent-tool rename + roster fixes (IMPROVEMENTS.md folded in and removed); whats-next pql-durability notes; pr-process orphan-check + full-suite alignment. New helper scripts tooling/godot-cold-parse + tooling/pr-watchlist-diff (allowlist entries deferred to first-use per permission policy). Part of T-1099.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
2026-07-13 12:16:34 +02:00
co-authored by Claude Fable 5
parent e6fd51161a
commit 32021bd550
9 changed files with 239 additions and 261 deletions
+4 -4
View File
@@ -118,22 +118,22 @@ systems.db is stale — run `make regen-db` before pushing.
```
Fix: run `make regen-db`, stage `server/data/systems.db`, amend or add a commit.
Or use `/pr-push` — it detects stale generator sources and reruns `make regen-db`
Or use `/pr-process` — it detects stale generator sources and reruns `make regen-db`
automatically before pushing.
The check script is `tooling/check-systems-db-stamp`. Run it interactively with
`make check-systems-db` or `python3 tooling/check-systems-db-stamp --verbose`. The
`GENERATOR_SOURCES` dict in `tooling/generator_sources.py` is the single registry
(T-1067) — the check script and the importer's stamp writer both import it, and
the `/pr-push` skill derives its source-file watch list from
the `/pr-process` skill derives its source-file watch list from
`python3 tooling/generator_sources.py --list`. When you add a new generator or
source file, register it there and nowhere else.
---
## /pr-push integration (T-858)
## /pr-process integration (T-858)
The `/pr-push` skill checks whether any generator source files are modified on the
The `/pr-process` skill checks whether any generator source files are modified on the
branch. If they are, it automatically runs `make regen-db` and stages the updated
`server/data/systems.db` before pushing — preventing pre-push hook rejections on
branches that modify generators without regenerating.
+37 -67
View File
@@ -27,14 +27,14 @@ current branch — never touches main.
### 0. Dry-run mode check
If the user invokes `/pr-push --dry-run`:
If the user invokes `/pr-process --dry-run`:
- Print: "Dry-run mode — inspecting state, nothing will be pushed or committed."
- Run steps 1 through 4a in **inspect-only** mode:
- Step 4: run `make check-systems-db` to check current stamp freshness (no merge)
- Step 4a: report which watched files changed vs origin/main; show whether `make regen-db`
would be triggered; do NOT run the regen, stage, or commit
- Print a summary: watched files changed (list), regen needed (yes/no), DB stamp fresh (yes/no)
- Print "Dry run complete — use /pr-push to apply."
- Print "Dry run complete — use /pr-process to apply."
- Stop. Do not push or create a PR.
---
@@ -55,8 +55,8 @@ 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))}'
# List any godot/gdUnit4 processes running longer than 5 minutes
ps -eo pid,etimes,cmd | awk '$2 > 300 && /godot.*(GdUnitCmdTool|[Gg]d[Uu]nit)/ {print $1, $2"s", substr($0, index($0,$3))}'
```
If any are listed: they are almost certainly orphans from a prior test
@@ -85,9 +85,12 @@ ignore` comment with a reason.
**For server branches:**
```bash
cargo clippy -- -D warnings 2>&1
cargo clippy --manifest-path server/Cargo.toml --all-targets -- -D warnings 2>&1
```
This must match the pre-push gate's own invocation exactly (`--all-targets` is
easy to drop locally and then miss warnings the gate still catches).
**For CI/tooling branches:**
```bash
ruff check tooling/ 2>&1
@@ -105,62 +108,41 @@ 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.
`tooling/godot-cold-parse` wipes the cached script-class registry (matching
the cold-start ordering CI / fresh clones see — Sprint 36 close caught a
`class_name` base-class registration bug that warm caches masked) and runs a
headless parse, filtering known pre-existing noise. Pass `--run-menu` if the
branch has UI changes to also launch the main menu briefly:
```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
# 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 -iE "^(SCRIPT )?ERROR|Parse Error|Export type"
tooling/godot-cold-parse --run-menu
```
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.
If it 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) — fix the ordering, don't just rebuild the cache
to mask it locally (the same error resurfaces post-merge).
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.
Any lines it reports are new errors introduced by this branch. Fix them
before pushing.
**For server branches:**
```bash
cd server && cargo test --lib 2>&1
cargo test --manifest-path server/Cargo.toml 2>&1
```
Full suite, not `--lib``--lib` skips the believability/derivation golden
harnesses (separate test binaries) and would silently under-test cascade
changes. This must match the pre-push gate's own invocation (`team-patterns.md`)
exactly; a weaker local pass here just gives false confidence right before the
gate catches it anyway.
If any errors are found, **stop and fix them before pushing**. Do not
push broken code for reviewers to find — that wastes everyone's time.
If the branch includes visual changes (character creation, UI, rendering),
the team should have manually launched the game and verified the change
works on screen before invoking `/pr-push`. If they haven't, ask:
works on screen before invoking `/pr-process`. If they haven't, ask:
"Have you run `make game` and verified this works visually?"
### 2. Commit uncommitted changes
@@ -171,9 +153,8 @@ git diff --stat
```
**Run both commands from the repo root** (`git rev-parse --show-toplevel`).
Running from a subdirectory can cause paths to not resolve, hiding real
changes — Sprint 30 proved this when `git diff HEAD -- server/src/bin/atlas.rs`
returned 0 lines from the wrong CWD, masking uncommitted agent work.
Running from a subdirectory can hide real changes (Sprint 30: a wrong-CWD
`git diff` returned 0 lines, masking uncommitted work).
**CRITICAL: Do not trust "already done" claims without checking git state.**
If agents report that work was "already implemented in a prior commit," verify
@@ -197,7 +178,7 @@ git fetch --all
git log --oneline origin/<branch>..<branch>
```
If no unpushed commits, skip to step 5 (PR check).
If no unpushed commits, skip to step 6 (PR check).
### 4. Check for conflicts with main
@@ -222,24 +203,13 @@ versus `origin/main`. This list covers generator code AND the data files that fe
The stamped generator sources come from the shared registry
`tooling/generator_sources.py` (T-1067) — the same module the stamp writer and
`tooling/check-systems-db-stamp` use, so the lists can no longer drift (PR #136
review T7). The CLI call below expands to one repo-relative path per line; the
extra hardcoded entries are non-stamped watch items (retired/one-time planet-gen
importers, the schema DDL whose SHA is stamped separately, and the data
directories that feed the generators).
review T7). `tooling/pr-watchlist-diff` wraps the comparison — its header comment
documents the non-stamped watch items it also checks (the surviving one-time
planet-gen importers, the schema DDL whose SHA is stamped separately, and the
wiki data directories that feed the generators):
```bash
git diff --name-only origin/main...HEAD -- \
$(python3 tooling/generator_sources.py --list) \
tooling/planet-gen/generate_atlas.py \
tooling/planet-gen/gemma_naming.py \
tooling/planet-gen/naming_core.py \
tooling/planet-gen/import_city_names.py \
tooling/planet-gen/import_heightmaps.py \
tooling/planet-gen/import_province_boundaries.py \
server/data/systems-schema.sql \
wiki/star-systems/ \
wiki/economics/ \
content/economics/
tooling/pr-watchlist-diff origin/main HEAD
```
**If output is empty:** skip this step entirely.
@@ -377,5 +347,5 @@ Suggest: "PR processed. Run `/pr-review` from main to review, or `/whats-next` f
## Arguments
If the user passes arguments (e.g., `/pr-push "my title"`), use them as the
If the user passes arguments (e.g., `/pr-process "my title"`), use them as the
PR title instead of generating one.
+56 -67
View File
@@ -1,12 +1,12 @@
---
name: pr-review
description: >
Review a branch diff with team-appropriate agents before merge. Use when the
user says "review-pr", "review this PR", "review this branch", or invokes
Review a Gitea branch diff with team-appropriate agents before merge. Use when
the user says "review-pr", "review this PR", "review this branch", or invokes
/pr-review. Spawns reviewers matched to the branch type (code, copy, visual,
audio) in parallel. Reports approve/reject with inline comments.
user-invocable: true
allowed-tools: Bash, Read, Grep, Glob, Task
allowed-tools: Bash, Read, Grep, Glob, Task, Write
---
# PR Review Skill
@@ -30,8 +30,8 @@ Stop and wait for the user to invoke `/pr-review` from main.
### 0b. Verify runtime smoke test was performed
Before spawning reviewers, check that the pushing team performed basic
runtime verification. This was the #1 process failure of Sprint 28
3 review rounds without anyone launching the game missed critical bugs.
runtime verification the #1 process failure of Sprint 28 (3 review
rounds shipped without anyone launching the game).
Ask: "Did the team run `make game` or a headless smoke test before
pushing this PR?"
@@ -40,13 +40,6 @@ If the PR description or commit messages don't mention runtime testing,
note this in the review output as a process gap. Reviewers should still
proceed (the PR exists and needs reviewing) but the gap should be visible.
For **client/visual branches**, run a quick headless parse check from main:
```bash
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:
@@ -69,18 +62,17 @@ 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
### 0c. Parse/lint — deferred to the push gate
The project enforces a **zero warnings policy**. Before spawning reviewers,
check if the branch introduces lint warnings:
- **client/visual:** `gdlint client/scripts/ client/ui/` should report 0 issues
- **server:** `cargo clippy -- -D warnings` should be clean
- **ci/tooling:** `ruff check tooling/` should be clean
If warnings exist, note the count in the review output. Reviewers should
flag any **new** warnings introduced by the branch as `warning` severity.
Pre-existing warnings are not PR blockers but should be tracked for cleanup.
`/pr-review` runs on an already-pushed branch (step 0's branch guard means the
reviewer's only local tree is **main**). Re-running `gdlint`/`cargo clippy`/`ruff`
here would check main's code, not the branch's — and `cargo clippy` fails outright
at the repo root (no root `Cargo.toml`; the server crate is `server/`). None of
that is necessary anyway: the pre-push hook (`.config/hooks/pre-push`, declared
authoritative by `team-patterns.md`) already ran `cargo fmt --check` / `cargo
clippy --all-targets -- -D warnings` / `cargo test` (server changes) or the full
gdUnit4 suite via `tests/run-godot` (client changes) before this push succeeded.
Trust the gate; don't duplicate it here with a weaker, wrong-tree variant.
### 1. Determine the branch to review
@@ -113,7 +105,9 @@ git diff --stat main...<branch>
Examine the changed file paths to determine the dominant change type:
- Mostly `server/` changes → **code** reviewers
- Mostly `server/` changes → **code** reviewers*except* `server/content/`
(game content data: campaigns, gauntlet, global, `content.yaml`), which routes
to **copy** reviewers despite living under `server/`
- Mostly `client/` changes (excluding `client/ui/` art assets) → **code** reviewers
- Mostly `wiki/`, `docs/atlas/`, `content/` changes → **copy** reviewers
- Mostly `client/ui/` or asset changes (`.tres`, `.tscn`, textures) → **visual** reviewers
@@ -140,59 +134,54 @@ If the diff is empty, report "No changes to review" and stop.
- `Cargo.lock` (auto-generated)
- `client/addons/gdUnit4/` (vendor test framework)
- `*.uid` (Godot-generated)
- `docs/backups/settledreach.db.backup` (binary)
- `docs/backups/*` (binary DB backups, e.g. `settledreach.db.backup.pre35`)
Three-dot diff with pathspec exclusions is unreliable. Instead, either:
1. Use `git diff main...<branch>` (full diff) and filter in the prompt, or
2. Read source files directly from the team directory (see below).
2. Read source files directly from the worktree (see below).
For large diffs (>1000 lines of source), provide **source files** rather than
raw diff to reviewers — cleaner context, better reviews.
**Reviewer agents read source files from the team worktree.**
**Reviewer agents read source files from the branch worktree — verified, not
assumed — or via `git show`.**
Sprint branches follow `sprint-{N}/{team}`. Mid-sprint, a worktree
of each branch exists at `$(dirname <repo_root>)/.sprint/sprint-{N}/{team}/`
— a *sibling* of the repo root, not a child. This worktree IS the
branch: Read/Grep on paths rooted there resolve against the branch's
checkout, not main's.
**Why this matters:** Sprint 37 PR #138 review produced 6 false-
positive findings because the reviewer defaulted to Read/Grep on the
main repo path (`/var/mnt/data/projects/settled-reach/main/`) instead
of the branch worktree. Every finding was a verbatim match against
main's state but irrelevant to the branch — the branch had already
cleaned the residue the reviewer flagged as "still present." Sending
those findings to the team would have caused busywork on already-clean
code, and more dangerously, the same drift hides *false negatives*
(branch-introduced bugs the reviewer never saw because it never read
the branch).
Fix: before spawning reviewers, resolve the worktree path and pass it
into every reviewer prompt with prominent language. The reviewer
reads from the worktree, not from main.
Topic branches from `/whats-next` may have a worktree at
`<repo_root>/.worktrees/<branch-name>` (the kanban convention, D-221 — see
`tooling/worktree-setup`):
```bash
# Find worktree for this branch (if one exists)
WORKTREE=$(git worktree list --porcelain | grep -B2 "branch refs/heads/<branch>" | grep "worktree " | sed 's/worktree //')
# Verify it exists and matches the branch tip
git -C "$WORKTREE" rev-parse HEAD # should equal `git rev-parse origin/<branch>`
REPO_ROOT="$(git rev-parse --show-toplevel)"
WORKTREE="$REPO_ROOT/.worktrees/<branch-name>"
```
If the worktree exists and its HEAD matches `origin/<branch>`, use it
as the reviewer's source of truth. If it doesn't exist (e.g. the
worktree has been torn down or you're reviewing a branch without a
worktree), fall back to `git show origin/<branch>:<path>` — explicitly
flag this fallback in the reviewer prompt so the reviewer knows
Read/Grep on any local path would be wrong.
If that path doesn't exist, fall back to the generic resolution (also used by
`/pr-review` §7a cleanup):
```bash
WORKTREE=$(git worktree list --porcelain | grep -B2 "branch refs/heads/<branch>" | grep "worktree " | sed 's/worktree //')
```
Use the worktree as the reviewer's source of truth **only if both** hold —
otherwise fall back to `git show origin/<branch>:<path>` and flag the fallback
in the reviewer prompt:
1. **HEAD matches the branch tip:** `git -C "$WORKTREE" rev-parse HEAD` equals `git rev-parse origin/<branch>`.
2. **Not a sparse checkout:** `git -C "$WORKTREE" sparse-checkout list` is empty or errors ("not a sparse checkout").
Two incidents motivate both checks — cited once, here, so they don't drift
back into two contradictory rules: Sprint 37 (a reviewer defaulted to reading
the main repo root instead of the branch worktree — 6 false positives) and
Sprint 33 (a sparse worktree silently excluded the wiki directory — a false
"missing prose" finding). One rule now covers both: verify HEAD *and*
sparse-checkout status, or use `git show`.
In the reviewer prompt, state the rule non-negotiably:
> **Read source from `<WORKTREE_PATH>` only.** Do NOT Read or Grep
> paths under the main repo root (`/var/mnt/data/projects/settled-reach/main/`).
> Those resolve to main, not the branch. The worktree at `<WORKTREE_PATH>`
> IS the branch — point all file tools there.
> **Read source from `<WORKTREE_PATH>` only** (or `git show
> origin/<branch>:<path>` if no valid worktree exists). Do NOT Read or Grep
> paths under the main repo root (`$(git rev-parse --show-toplevel)`) — that
> resolves to main, not the branch.
Also tell agents to read relevant `governance/**/*.md` files for context
(these can be read from either path — they're usually identical —
@@ -200,7 +189,7 @@ but for consistency, use the worktree path).
### 4. Spawn reviewers in parallel
Use the Task tool to spawn **all reviewers simultaneously** in a single message.
Use the Agent tool to spawn **all reviewers simultaneously** in a single message.
Read `references/reviewer-profiles.md` for the full per-branch-type reviewer
specifications (agent types, models, prompt focus areas). Match the branch type
@@ -269,7 +258,7 @@ Verdict rules:
something can be better, say so.
```
## 6. Posting results to Gitea
### 6. Posting results to Gitea
After presenting results to the user, post the review as a PR comment.
@@ -283,8 +272,8 @@ Post using the `tea-comment` wrapper (handles temp files and cleanup).
`tooling/tea-comment` in a separate short Bash call.
2. **Run `tooling/tea-comment` in the FOREGROUND, never with
`run_in_background`.** The background execution path silently fails —
the comment never reaches Gitea and the team never sees the review.
Sprint 38 lost an entire review round this way. Always foreground.
the comment never reaches Gitea and the team never sees the review
(Sprint 38 lost an entire review round this way).
```
# Step 1: Use the Write tool to create the file
@@ -297,7 +286,7 @@ tooling/tea-comment <PR_NUMBER> @/tmp/pr-review-<NUMBER>.md
Do NOT use `cat << 'EOF'` heredocs for review content — they create
massive permission prompts that are slow to render and often get stuck.
## 7. Merging approved PRs
### 7. Merging approved PRs
`tea pr merge` fails (405) when branches have conflicts with main. Merge
locally instead:
@@ -2,23 +2,18 @@
Use `model: sonnet` for all reviewers — sufficient for review, saves cost.
**Reviewer agents read source files via `git show` or from sprint
worktrees.** Sprint branches use `sprint-{N}/{team}` naming. Include
the branch name and a list of changed files in every prompt. The
default approach is `git show origin/<branch>:<path>`. If an active
worktree exists under `.sprint/`, agents can also use the Read tool
with the worktree path. **Always prefer `git show` over worktree
reads** — worktrees may use sparse checkouts that silently exclude
files, causing reviewers to miss content and produce false findings
(Sprint 33 lesson: Paula reported missing prose that was actually
present, because the worktree excluded the wiki directory).
**Reviewer agents read source files from the branch worktree or via `git
show`** — see SKILL.md §3 for the full resolution + validation rule (prefer
the worktree only when its HEAD matches the branch tip AND it isn't a sparse
checkout; otherwise `git show origin/<branch>:<path>`). Include the branch
name and a list of changed files in every prompt.
## Code reviews (`server`, `client`, `ci`)
**Hoshe (Code Quality)**
- `subagent_type`: `hoshe`, `model`: `sonnet`
- Prompt: Provide the team directory path, list of changed files, and commit log.
Tell Hoshe to read source files from the team directory, then review for:
- Prompt: Provide the worktree path, list of changed files, and commit log.
Tell Hoshe to read source files from the worktree, then review for:
- Correctness and bug risks
- Error handling gaps
- Test coverage (are new features tested?)
@@ -28,8 +23,8 @@ present, because the worktree excluded the wiki directory).
**Tyre (Architecture)**
- `subagent_type`: `tyre`, `model`: `sonnet`
- Prompt: Provide the team directory path, list of changed files, and commit log.
Tell Tyre to read the relevant `governance/**/*.md` files from the team directory
- Prompt: Provide the worktree path, list of changed files, and commit log.
Tell Tyre to read the relevant `governance/**/*.md` files from the worktree
first, then review for:
- Architectural consistency with project decisions
- API/interface design quality
@@ -41,8 +36,8 @@ present, because the worktree excluded the wiki directory).
**Hoshe (QA)**
- `subagent_type`: `hoshe`, `model`: `sonnet`
- Prompt: Provide the team directory path, list of changed files, and commit log.
Tell Hoshe to read the changed files from the team directory, then review for:
- Prompt: Provide the worktree path, list of changed files, and commit log.
Tell Hoshe to read the changed files from the worktree, then review for:
- Formatting consistency (markdown, file naming, frontmatter)
- Broken references or links
- Spelling and grammar
@@ -51,9 +46,9 @@ present, because the worktree excluded the wiki directory).
**Paula (Narrative Depth)**
- `subagent_type`: `paula`, `model`: `sonnet`
- Prompt: Provide the team directory path, list of changed files, commit log, and
- Prompt: Provide the worktree path, list of changed files, commit log, and
list of relevant `governance/**/*.md` files to read. Tell Paula to read all
files from the team directory using the Read tool, then review for:
files from the worktree using the Read tool, then review for:
- Narrative quality and character voice consistency
- Whether dialogue and monologue feel authentic to the characters
- Consequences and stakes — do choices carry weight?
@@ -62,9 +57,9 @@ present, because the worktree excluded the wiki directory).
**Miri (World Consistency)**
- `subagent_type`: `miri`, `model`: `sonnet`
- Prompt: Provide the team directory path, list of changed files, commit log, and
- Prompt: Provide the worktree path, list of changed files, commit log, and
list of relevant `governance/**/*.md` files to read. Tell Miri to read all
files from the team directory using the Read tool, then review for:
files from the worktree using the Read tool, then review for:
- Lore accuracy — do facts match established setting?
- Internal consistency across files
- IP originality — nothing should read as a copy from another franchise
@@ -75,8 +70,8 @@ present, because the worktree excluded the wiki directory).
**Hoshe (QA)**
- `subagent_type`: `hoshe`, `model`: `sonnet`
- Prompt: Provide the team directory path, list of changed files, and commit log.
Tell Hoshe to read the changed files from the team directory, then review for:
- Prompt: Provide the worktree path, list of changed files, and commit log.
Tell Hoshe to read the changed files from the worktree, then review for:
- File format and naming conventions
- Asset organization and directory structure
- Missing or broken references in scene/resource files
@@ -84,7 +79,7 @@ present, because the worktree excluded the wiki directory).
**Araminta (Art Direction)**
- `subagent_type`: `araminta`, `model`: `sonnet`
- Prompt: Provide the team directory path, list of changed files, and commit log.
- Prompt: Provide the worktree path, list of changed files, and commit log.
Tell Araminta to read the style guide and relevant design docs from the
worktree first, then review for:
- Visual consistency with the established style guide
@@ -97,8 +92,8 @@ present, because the worktree excluded the wiki directory).
**Hoshe (QA)**
- `subagent_type`: `hoshe`, `model`: `sonnet`
- Prompt: Provide the team directory path, list of changed files, and commit log.
Tell Hoshe to read the changed files from the team directory, then review for:
- Prompt: Provide the worktree path, list of changed files, and commit log.
Tell Hoshe to read the changed files from the worktree, then review for:
- File format and naming conventions
- Audio asset organization and directory structure
- Missing or broken references
@@ -106,8 +101,8 @@ present, because the worktree excluded the wiki directory).
**Ozzie (Player Experience)**
- `subagent_type`: `ozzie`, `model`: `sonnet`
- Prompt: Provide the team directory path, list of changed files, and commit log.
Tell Ozzie to read all files from the team directory using the Read tool, then
- Prompt: Provide the worktree path, list of changed files, and commit log.
Tell Ozzie to read all files from the worktree using the Read tool, then
review for:
- Emotional impact — does the audio enhance the moment?
- Atmosphere and tone — does it feel like the Settled Reach?
+22
View File
@@ -47,6 +47,11 @@ lowest-numbered non-`done` phase epic):
pql ticket list --under T-745 --status in_progress --fields id,type,status,title
```
> `--status` takes a SINGLE value (not a comma list — `backlog,in_progress` matches
> nothing and silently returns `[]`). Most tickets here sit in `backlog` or `done`;
> if you need multiple statuses at once (e.g. both `backlog` and `in_progress`), run
> the query once per status.
The phase epic is the `"type": "epic"` row. Use pql natively — no ad-hoc python/jq
projections over its output: since pql 1.11.0, `ticket list` omits `description` by
default and supports `--fields id,type,parent_id,priority,status,title` (the batch-
@@ -179,6 +184,23 @@ For each ticket in the batch (batch with commas: `pql ticket status T-1,T-2 in_p
pql does **not** enforce the WIP limit — check it yourself in step 1d and warn if the
batch pushes in-progress past 5.
> **Then make sure it persists.** Ticket mutations (status here, and any
> `ticket append` in Step 2) land only in the gitignored `.pql/pql.db`. The
> pre-commit hook runs `pql plan export --stage` automatically — the changelog
> is exported and staged on every commit, so never hand-run the export or
> `git add .pql/changelog`. The rule is simpler: the turn must land at least
> one commit (through the git-commit skill). A ticket-only turn with no commit
> leaves the mutations in `pql.db` only, and the post-checkout/post-merge hooks
> rebuild that DB from the committed changelog on the next branch switch —
> silently dropping them. See the pql skill's "Versioning planning state"
> section for the full mechanics.
>
> - Don't hand-export or hand-stage `.pql/changelog/` — the pre-commit hook does
> both on every commit. The real footgun is a turn that mutates tickets but
> never commits: `pql.db` is gitignored and gets rebuilt from the committed
> changelog on branch switch, silently dropping un-committed mutations. Land
> at least one commit per ticket-mutating turn.
### 3b. Create topic branch
Name the branch after the epic or logical grouping. Examples:
@@ -1,87 +0,0 @@
# workshop-start — Improvement Log
Running notes on friction and fixes observed while running workshops. Fold the
confirmed ones into `SKILL.md` periodically; delete once applied.
## From the `system-economic-specialization` relaunch (2026-05-31)
- **Tool name drift.** SKILL.md §5 says "Use the **Task tool** to spawn each
agent." The actual spawn tool is **`Agent`** (with `team_name` + `name` +
`subagent_type`). "Task" reads as the TaskCreate/TaskUpdate family and is
misleading. Rename to "Agent tool" throughout §5.
- **SI has no Round 1 work — say so.** The skill adds SI to every workshop but
SI's job is Round 2 ticket creation. Round 1 there is no SI task, so SI sits
with an empty TaskList and no instruction. Recommend the skill explicitly note:
"SI joins on standby; SI gets no task until Round 2 — spawn it in background
with a 'load context and wait for Round 2' prompt." Avoids an idle/confused
agent on spawn.
- **Spawn order vs. task creation.** Following the skill literally (§4 create
tasks → §5 spawn) matters: if you spawn agents before the tasks exist, they
check TaskList, find nothing, and may idle. Worth a one-line warning in §5:
"Create and assign all Round 1 tasks BEFORE spawning, or the agents wake to an
empty list." (Recoverable by messaging them after, but cleaner to order it
right.)
- **`TaskCreate` is one-task-per-call.** No batch/array form. A batch attempt is
rejected. The skill could state this so the lead emits N separate TaskCreate
calls (and assigns owners via TaskUpdate, since TaskCreate takes no `owner`).
- **Read-only participants can't write their own files.** Several workshop agent
types (paula, gore, nigel, ozzie, and the read-only miri variant) have NO Write
tool — yet SKILL.md §4 makes "write your full output to disk" a hard
requirement for every participant. Paula hit this: she produced her full Round 1
doc but had to send it to the lead to write `paula-round1.md`. The skill should
state up front: "Participants without a Write tool deliver their output via
SendMessage; the lead writes the file and marks the task complete on their
behalf." Or: assign a write-capable scribe. Either way, don't make disk-write a
per-agent requirement for read-only agent types — it guarantees a manual relay.
- **Participants self-claim, which races owner assignment.** Paula picked up
task #3 on her own before the lead assigned owners. Harmless here, but if tasks
aren't clearly scoped per-agent an agent could grab the wrong one. The skill's
per-agent task titles ("Round 1 — Miri …") mitigate this; keep titles
agent-named.
## Candidate feature — per-round "scout" critic (user idea, 2026-05-31)
Add an optional independent **scout** agent that runs per round, AFTER the round's
output files land and BEFORE the user checkpoint, to surface missed options,
simpler designs, internal contradictions, and external design prior-art.
- **Value is mostly the independent adversarial review**, not the web. Participants
build on each other and converge; a fresh agent with no stake catches blind
spots convergence hides. Web search is a bonus, useful for the *mechanism* (real
economic geography, how other games encode X, schema-granularity tradeoffs) —
NOT the lore.
- **Hard guardrails:** (1) lore is authored-canonical and fictional — the scout must
NOT "correct" the fiction with real-world/web facts; scope web to design/mechanism
prior-art only. (2) Only surface findings that would CHANGE a decision — a critic
that always finds 10 things gets ignored.
- **Shape:** cheap model (Haiku), `run_in_background: true`, reads the round files +
relevant D-records, writes `scout-round{N}.md`. Lead folds decision-changing items
into the checkpoint presented to the user. Escalate to Sonnet only to dig into a
real find.
- Pairs well with the quality-over-scope-creep principle: the scout is a cheap way
to pressure-test that "quality" claims actually hold and nothing simpler was missed.
## Wrap-up gotchas (2026-05-31)
- **`pql decisions claim` is side-effect-free** (it just prints the next available ID).
This fixes the old `tooling/db/decision claim` footgun, which burned an ID and appended
a placeholder on every call — retrying it created duplicate records. With pql, claim is
safe to re-run; the record only exists once you write the `### D-NNN: …` heading into the
governance file. Still claim each ID once and write it immediately so parallel branches
don't pick the same number.
- **Read-only documenter can still file** — the lead hands it the claimed ID(s); the
documenter writes the records into `governance/{decisions,questions,rejected}/<domain>.md`.
- **Background (`run_in_background`) teammates can be slow/unreliable to consume
`shutdown_request`.** At wrap-up, qatux + si (both spawned in background per the
skill's §5 guidance) stayed "active" through multiple shutdown sends, blocking
TeamDelete. Foreground participants shut down immediately. Options to fold into
the skill: (a) don't background the always-present agents, or (b) document that
wrap-up may need several shutdown retries / a longer settle, and that the lead
should not spin indefinitely — the work is already on disk, TeamDelete is just
resource cleanup. Consider whether the skill should note the team can be left to
expire rather than blocking the session on cleanup.
+18 -9
View File
@@ -4,7 +4,7 @@ description: >
Start a multi-agent design workshop from a workshop brief. Use when the user says
"start workshop", "run workshop", "let's start the workshop", or invokes /workshop-start.
Parses the workshop brief to extract participants, questions, and round format.
Creates a team, tasks, and spawns agents as teammates via the Task tool.
Creates a team, tasks, and spawns agents as teammates via the Agent tool.
---
# Start Workshop
@@ -24,7 +24,8 @@ Read the workshop brief. Extract:
- Workshop name (from directory name)
- Participant list (from `**Participants:**` line)
- Per-participant questions (scan for `**{AgentName}**:` patterns in questions sections)
- Round count and round descriptions (from `**Workshop Format**` section)
- Round count and round descriptions (from the `## Workshop Format` heading — a real
heading, not a bold marker; every brief under `docs/workshops/` uses this form)
### 2. Create Team
@@ -38,8 +39,8 @@ Two agents join every workshop regardless of the participant list:
| Agent | Role | Task | Participates in discussion? |
|-------|------|------|-----------------------------|
| **Qatux** | Documenter | Captures all decisions, questions, dissent, consensus. Writes `workshop-notes.md` per round, produces final `workshop-outcomes.md`. | No — observes and records only |
| **SI** | Sprint prep | Suggest adding when outputs include tickets. Creates tickets from decisions, links to sprint backlog. | No — execution prep only |
| **Qatux** | Documenter | Captures all decisions, questions, dissent, consensus. Writes `round-{N}-notes.md` per round, produces final `workshop-outcomes.md`. | No — observes and records only |
| **SI** | Refinement reviewer | Suggest adding when outputs include tickets. Reviews `workshop-outcomes.md` for ticket-worthy items and context completeness — does **not** create tickets (si.md's "What you do NOT do" list is explicit: no ticket creation or DB writes). The lead creates tickets via the `/ticket` skill; SI reviews them for gaps. | No — execution prep only |
Qatux and SI are **never dismissed early.** If the user reduces the team mid-workshop, keep qatux and si (if added). Documenting everything prevents loss of valuable information.
@@ -50,6 +51,7 @@ One task per participant containing:
- The specific questions assigned to that participant (extracted from all layers)
- The output format from the brief's round description
- **IMPORTANT — file output requirement:** Instruct each agent to write their full output to `docs/workshops/{name}/{agent}-round{N}.md` (e.g., `docs/workshops/test-architecture/gestalt-round1.md`). Agents must write to disk, not just send messages. This ensures Qatux and other agents can read all outputs directly without relying on message forwarding.
**Several participant types have no Write tool** (ozzie, gore, nigel, paula, miri — verify current grants in `.claude/agents/*.md` frontmatter). Participants without a Write tool deliver their output via SendMessage instead; the lead writes `{agent}-round{N}.md` on their behalf and marks the task complete.
One task for Qatux: "Document Round N — read all agent output files at `docs/workshops/{name}/*-round{N}.md` and capture decisions, questions, and dissent."
@@ -57,16 +59,21 @@ Assign all tasks using TaskUpdate with `owner` = agent name.
### 5. Spawn Agents
Use the Task tool to spawn each agent as a teammate. Each call should:
**Create and assign all Round 1 tasks (§4) before spawning.** Spawning first means
agents wake to an empty TaskList and may idle. SI joins on standby with no Round 1
task — SI's job starts at Round 2 ticket review — so give it a "load context and
wait" prompt rather than an empty task.
Use the Agent tool to spawn each agent as a teammate. Each call should:
- Set `team_name` to the workshop team name
- Set `name` to the agent name (e.g., "gestalt")
- Set `subagent_type` to the matching agent type (same as name — see reference table)
- **Set `model: "sonnet"`** — team-mode participants pin to Sonnet 4.6 for literal-rule-following discipline. Opus 4.7 was observed ignoring scope rules and failing to report back in team mode (Sprint 37). The *team lead* (this session) stays on whatever model the user has selected.
- Provide a prompt telling the agent to check TaskList for their assigned task
Spawn all agents in parallel (one Task call per agent in a single message). Agents will appear as teammates in the Claude Code UI and pick up their tasks from the shared task list.
Spawn all agents in parallel (one Agent call per agent in a single message). Agents will appear as teammates in the Claude Code UI and pick up their tasks from the shared task list.
For large workshops (>6 agents), spawn participants in batches to avoid overwhelming the system. Always-present agents (Qatux, SI) can run in background via `run_in_background: true`.
For large workshops (>6 agents), spawn participants in batches to avoid overwhelming the system. Always-present agents (Qatux, SI) can run in background via `run_in_background: true` — but background teammates can be slow or unreliable to consume `shutdown_request`, so wrap-up (§8) may need several shutdown retries. Don't spin indefinitely waiting on it: the work is already on disk, and the team can be left to expire rather than blocking the session on `TeamDelete`.
### 6. Monitor
@@ -96,9 +103,9 @@ When all Round N tasks are complete:
Before the user dismisses the team, the following are **hard requirements**:
1. **User reviews final outcomes** — Present `workshop-outcomes.md` content to the user via AskUserQuestion. Get explicit approval before proceeding to filing.
2. **D-records filed** — All new D-records, amendments, and supersessions are written to `governance/` domain files. This is non-negotiable — workshops that produce decisions MUST file them before shutdown.
2. **D-records filed** **Claim each ID first** with `pql decisions claim D <domain> "title"` (side-effect-free — safe to re-run, just prints the next free id) and write the record immediately after claiming, so parallel branches don't collide on the same number (CLAUDE.md requirement). All new D-records, amendments, and supersessions are written to `governance/` domain files. This is non-negotiable — workshops that produce decisions MUST file them before shutdown.
3. **Discussion captured** — Qatux produces final `workshop-outcomes.md` from accumulated notes. Qatux creates or updates diagrams (via `/d2-diagram`) for any new D-records produced by the workshop.
4. **Tickets created** — If SI is present, SI creates tickets from decided items and the user reviews the ticket list.
4. **Tickets created** The lead creates tickets from decided items via the `/ticket` skill, grounded in `workshop-outcomes.md`. If SI is present, SI reviews the drafted tickets for context completeness before the user reviews the ticket list.
5. **User gives explicit go-ahead to dismiss** — Only after steps 1-4 are complete AND the user confirms, send shutdown_request to all agents (qatux and si last).
6. TeamDelete to clean up.
@@ -156,3 +163,5 @@ Agent name maps directly to subagent_type:
| inigo | Sound design | Audio, soundscape, spatial audio |
| troblum | Tech consultant | Second opinion on architecture/tech choices |
| hoshe | QA/testing | Test plans, verification, quality |
| burnelli-sheldon | Economist | Market models, price formation, economic credibility |
| justine | Polish/deployment | Build, performance, release quality |
+52
View File
@@ -0,0 +1,52 @@
#!/usr/bin/env bash
# tooling/godot-cold-parse [--run-menu] — cold-cache headless parse check.
#
# Used by /pr-process step 1c before push. Deletes the cached script-class
# registry so the parse simulates the cold-start ordering CI / fresh clones
# see: Sprint 36 close caught a new `class_name MetaScreen` base class and
# six extending scripts that parsed fine on warm developer caches but hit
# `Could not find base class "MetaScreen"` post-merge, because the
# autoload-vs-class_name registration order only resolves correctly once the
# class cache is seeded (see CLAUDE.md -> GDScript conventions -> Autoload
# parse-order rule).
#
# Godot's resource scanner emits category errors (e.g. "Export type can only
# be built-in, a resource, a node, or an enum") that do NOT always prefix
# with SCRIPT ERROR — they appear as plain ERROR lines. The filter below
# catches both, then drops known pre-existing noise from the autoload
# class_name parse-order trap. Sprint 36 shipped a scanner error the old
# narrower grep missed; this is why the filter stays wide.
#
# --run-menu: also launch main_menu.tscn briefly (for branches with UI changes).
#
# Exit 0 + "clean" if no matches. Exit 1 + the matched lines if any are found.
set -euo pipefail
REPO_ROOT="$(git rev-parse --show-toplevel)"
RUN_MENU=false
[ "${1:-}" = "--run-menu" ] && RUN_MENU=true
rm -f "$REPO_ROOT/client/.godot/global_script_class_cache.cfg"
FILTER='^(SCRIPT )?ERROR|Parse Error|Export type'
MATCHES=$(godot --headless --path "$REPO_ROOT/client" --quit 2>&1 \
| grep -iE "$FILTER" \
| grep -v "Failed loading resource: res://assets" \
| grep -v "Cannot infer the type" \
| grep -vE '(Messagepack|LocalBridge|ServerProcess|Constants)" not declared' || true)
if [ "$RUN_MENU" = true ]; then
MENU_MATCHES=$(timeout 10 godot --path "$REPO_ROOT/client" res://scenes/main_menu.tscn 2>&1 \
| grep -iE "$FILTER" || true)
if [ -n "$MENU_MATCHES" ]; then
MATCHES="$MATCHES
$MENU_MATCHES"
fi
fi
if [ -n "$MATCHES" ]; then
echo "$MATCHES"
exit 1
fi
echo "godot-cold-parse: clean"
+28
View File
@@ -0,0 +1,28 @@
#!/usr/bin/env bash
# tooling/pr-watchlist-diff <base> <head> — list watch-list files changed
# between <base> and <head>.
#
# Used by /pr-process step 4a (T-858) to decide whether `make regen-db` must
# run before push. The stamped generator sources come from the shared
# registry tooling/generator_sources.py (T-1067) — imported live here so this
# list can't drift from the stamp writer / tooling/check-systems-db-stamp.
#
# The extra hardcoded paths below are non-stamped watch items: the surviving
# one-time planet-gen importers (import_heightmaps.py, import_province_
# boundaries.py — not part of `make regen-db`, but their data feeds the
# committed DB), the schema DDL (stamped separately via schema_sha), and the
# wiki data directories that feed the generators.
set -euo pipefail
BASE="${1:?usage: tooling/pr-watchlist-diff <base> <head>}"
HEAD="${2:?usage: tooling/pr-watchlist-diff <base> <head>}"
mapfile -t GENERATOR_SOURCES < <(python3 tooling/generator_sources.py --list)
git diff --name-only "$BASE...$HEAD" -- \
"${GENERATOR_SOURCES[@]}" \
tooling/planet-gen/import_heightmaps.py \
tooling/planet-gen/import_province_boundaries.py \
server/data/systems-schema.sql \
wiki/star-systems/ \
wiki/economics/