settled-reach/.claude/skills/pr-process/SKILL.md

---
name: pr-process
description: >
  Author-side PR lifecycle: commit, lint, push, create PR, AND pick up review
  comments from /pr-review. Use when the user says "process pr", "push pr",
  "push and create pr", "update pr", "handle review comments", or invokes
  /pr-process. Runs from the worktree. The counterpart to /pr-review which
  runs from main.
user-invocable: true
allowed-tools: Bash, Read, Grep, Glob, AskUserQuestion, Skill
---

# Process PR Skill

Push commits to remote and create or update a PR. Operates exclusively on the
current branch — never touches main.

## Safety Rules (NON-NEGOTIABLE)

- **NEVER merge a PR into main.** No `tea pr merge`, no `git merge` into main.
- **NEVER checkout or push to main.**
- **NEVER force-push** unless the user explicitly requests it.
- **NEVER use `--no-verify` or skip hooks.**
- Only push to the current working branch.

## Workflow

### 0. Dry-run mode check

If the user invokes `/pr-push --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."
- Stop. Do not push or create a PR.

---

### 1. Validate branch

```bash
git branch --show-current
```

If on `main`, stop: "You're on main. Switch to a topic 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
must be either fixed or suppressed with a commented justification.

**For client/visual branches:**
```bash
gdlint client/scripts/ client/ui/ 2>&1
```

If warnings remain, fix them before pushing. For warnings that cannot be
fixed (e.g. intentional long lines in data literals), add a `# gdlint:
ignore` comment with a reason.

**For server branches:**
```bash
cargo clippy -- -D warnings 2>&1
```

**For CI/tooling branches:**
```bash
ruff check tooling/ 2>&1
```

The goal is zero warnings in the pre-push output. Advisory warnings that
the pre-push hook reports as "(advisory, not blocking)" should still be
zero — they are advisory only because we haven't enforced them yet.

### 1c. Runtime smoke test (MANDATORY)

Before pushing, verify the game actually runs. This is non-negotiable —
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
# 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"
```

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`
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
```

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:
"Have you run `make game` and verified this works visually?"

### 2. Commit uncommitted changes

```bash
git status
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.

**CRITICAL: Do not trust "already done" claims without checking git state.**
If agents report that work was "already implemented in a prior commit," verify
by checking `git status` and `git diff --stat` first. Grepping source files
only proves the code exists on disk — it does NOT prove the code is committed.
Uncommitted working-tree changes look identical to committed code when you
read files. Only `git status` distinguishes "already shipped" from "just
written by a teammate."

If there are uncommitted changes (staged or unstaged), run the **commit skill**
first. Use the `/git-commit` skill to group changes into logical commits with
proper conventional commit messages. Wait for commit to complete before
proceeding.

If the working tree is clean (no uncommitted changes), skip to step 3.

### 3. Check for unpushed commits

```bash
git fetch --all
git log --oneline origin/<branch>..<branch>
```

If no unpushed commits, skip to step 5 (PR check).

### 4. Check for conflicts with main

```bash
git merge-tree --write-tree origin/main HEAD 2>&1
```

If conflicts reported, merge main into current branch:

```bash
git merge origin/main --no-edit
```

If merge conflicts, **stop and report** — let the user resolve.
If clean, continue.

### 4a. Regen systems.db if generator sources or data changed (#858)

Check whether any file in the **source-file watch list** was modified on this branch
versus `origin/main`. This list covers generator code AND the data files that feed them.

The generator-source paths below **must stay in sync** with `GENERATOR_SOURCES` in
`tooling/check-systems-db-stamp` (PR #136 review T7) — if you add a new source file
to the stamp, add it here too, and vice versa. Drift between the two lists reintroduces
exactly the silent-stale-DB class of bug this skill exists to prevent.

```bash
git diff --name-only origin/main...HEAD -- \
  tooling/economy-db/import_economics.py \
  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/src/bin/generate_brands/main.rs \
  server/src/bin/generate_brands/names.rs \
  tooling/generate-brands \
  server/data/systems-schema.sql \
  wiki/star-systems/ \
  wiki/economics/ \
  content/economics/
```

**If output is empty:** skip this step entirely.

**If any files appear in the output:** the DB must be regenerated on top of the
current main. Perform the following:

1. **Integrate main.** Step 4 merged main into the branch. If you find yourself
   on a branch that was NOT yet merged with main in step 4, do it now:
   ```bash
   git fetch origin
   git merge origin/main --no-edit
   ```
   If there are merge conflicts in source files, **stop and report which files
   conflict**. Ask the user to resolve manually — do not attempt to auto-resolve
   generator source conflicts.

2. **Regenerate the DB:**
   ```bash
   make regen-db
   ```
   `make regen-db` runs all three generators and stamps the meta table. It tolerates
   coverage gate failures (exit 2 = data quality warning, not an error). If it exits
   with any other non-zero code, stop and report the stderr output — do not push.

3. **Stage the updated DB:**
   ```bash
   git add server/data/systems.db
   ```

4. **Commit only if the DB actually changed:**
   ```bash
   git diff --cached --stat -- server/data/systems.db
   ```
   - If the diff shows changes: commit with `/git-commit`, message:
     `chore(db): regen systems.db against rebased sources`
   - If no diff (regen produced identical output — sources were self-consistent):
     unstage the file (`git restore --staged server/data/systems.db`) and skip the
     commit. The source changes alone are the PR content.

**In dry-run mode** (from step 0): report which watch-list files changed and
whether regen would be triggered. Do NOT run the regen or modify any files.

This step prevents the pre-push hook from rejecting a push where the branch modifies
a generator source or data file but did not regenerate the DB.

### 5. Push

```bash
git push origin <branch>
```

If push fails, stop and report. Never force-push without explicit request.

### 6. Check for existing PR

```bash
tea pr list --login schweitz --repo jpmschweitzer/settled-reach --state open --output simple
```

Match current branch name in PR list.

- **PR exists**: Report "Pushed N commits to `<branch>`. PR #X updated." Done.
- **No PR**: Continue to step 7.

### 7. Create a new PR

```bash
git log --oneline main..<branch>
git diff --stat main...<branch>
```

Draft title (`<type>(<scope>): <summary>`, max 70 chars) and description.

```bash
tea pr create \
  --repo jpmschweitzer/settled-reach \
  --login schweitz \
  --title "<title>" \
  --description "## Summary ..." \
  --base main \
  --head <branch>
```

Report PR URL when done.

### 8. Update ticket status to review

Scan all commit messages in the PR for ticket references (`#NNN`):

```bash
git log --oneline main..<branch>
```

Extract ticket IDs from `#NNN` patterns. For each ticket that is
currently `in_progress`, update it to `review`:

```bash
tooling/db/ticket status <id> review
```

Report which tickets were moved to review. Skip tickets that are
already `done`, `review`, `cancelled`, or `backlog` (only transition
`in_progress` → `review`).

### 9. Pick up review comments

Check if the PR already has review comments (from a prior `/pr-review` run):

```bash
tea pr --login schweitz --repo jpmschweitzer/settled-reach --comments -o simple <PR_NUMBER>
```

If comments exist and contain a review verdict (look for "CHANGES REQUESTED" or
"REQUEST_CHANGES" or a structured review table):

1. Parse each file-specific issue from the review comment
2. Present each issue to the user (or working agents)
3. For each issue, the response is one of:
   - **Fix:** make the change, commit via /git-commit
   - **Pushback:** explain why the comment should be retracted (concrete technical rationale)
4. After addressing all comments, re-run lint + smoke checks (steps 1b, 1c)
5. Push updated commits (step 5)
6. Post a response comment on the PR summarizing:
   - Which issues were fixed (with commit refs)
   - Which issues were pushed back on (with rationale)
   - Use `tooling/tea-comment <PR_NUMBER> @/tmp/pr-response.md`

If no review comments exist, or the review is APPROVED, skip this step.

### 10. Next steps

Suggest: "PR processed. Run `/pr-review` from main to review, or `/whats-next` for the next batch."

## Arguments

If the user passes arguments (e.g., `/pr-push "my title"`), use them as the
PR title instead of generating one.