chore(meta): Claude estate cleanup — .claude review (T-1099) #175

Closed
jpmschweitzer wants to merge 0 commits from claude-estate-cleanup into main
Owner

Full sweep of CLAUDE-CLEANUP.md (~60 findings, each re-verified against the current tree) across epics T-1100-T-1104: settings/security, skill removals, workflow skills, asset/tooling skills, agent roster. Full breakdown in the first comment.

Full sweep of CLAUDE-CLEANUP.md (~60 findings, each re-verified against the current tree) across epics T-1100-T-1104: settings/security, skill removals, workflow skills, asset/tooling skills, agent roster. Full breakdown in the first comment.
jpmschweitzer added 5 commits 2026-07-13 17:12:58 +02:00
Remove dead docs-search allow; promote godot4/gdformat/atlas/validate-ron; add repo-tree Edit/Write + worktree-setup allows. .gitignore ignores .claude/scheduled_tasks.lock/.json. Part of T-1099 (Claude estate cleanup).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
clide SKILL.md re-synced to the clide repo's canonical copy (picks up the dock/D-87 slot; de-forks). d2-diagram/frame0-wireframe/skill-create deletions landed in the prior commit; their repo-specific tailoring is preserved in docs/design/diagram-wireframe-conventions.md. Part of T-1099.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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>
Live-command corrections (atlas corridor-status, real body-ID naming), failure-proofed glb-gen/sprite-gen render scripts, Trellis API reference extracted. image-gen: fixed the output-path bug and de-forked the local image_connector.py to the canonical tooling/db/ copy. ticket skill consolidated to point at ticket-cli.md (setparent-none fix applied there too). Part of T-1099.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cleared stale STANDBY markers, swept dead DISCUSSION.md pointers, disambiguated overlapping personas, corrected asset-gen paths. Armed 7 discussion agents (gestalt/gore/mellanie/miri/nigel/ozzie/paula) with Bash so they can run the pql preamble. Integrated inigo into the roster + briefing (audio, standby). Removed tiger — localization dropped from scope (R-013). Part of T-1099.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jpmschweitzer added 1 commit 2026-07-13 17:15:45 +02:00
An explicit --repo slug makes tea skip local-repo setup, but pr create unconditionally needs the local repo handle and dies with 'local repository required' — the documented 'all flags explicit' rule was the trap. Root-caused shipping PR #175. tea-cli.md + pr-process template fixed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Author
Owner

Claude estate cleanup — 2026-07-02 .claude review (T-1099)

Full sweep of the CLAUDE-CLEANUP.md estate-review brief (~60 findings). Every
finding was re-verified against the current tree before editing (the repo
moved since 2026-07-02; several findings no longer reproduced and were skipped,
not force-applied). Tracked as initiative T-1099 with epics T-1100–T-1104.

Three parallel agents (E3/E4/E5) drafted the workflow-skill, asset/tooling-skill,
and agent-roster sections; the lead handled settings/security and resolved all
product-call flags with the team lead.

T-1100 — settings / security

  • GEMINI_API_KEY invalidated + scrubbed to a placeholder (machine-local file, not in this PR)
  • settings.json: removed dead docs-search allow; promoted stable project-tool allows (godot4/gdformat/atlas/validate-ron); added repo-tree Edit/Write + worktree-setup allows
  • .gitignore: ignore .claude/scheduled_tasks.lock/.json

T-1101 — skill removals

  • Deleted d2-diagram, frame0-wireframe, skill-create (live at user scope); repo-specific tailoring preserved in docs/design/diagram-wireframe-conventions.md
  • clide skill refreshed from the clide-repo canonical (picks up the dock/D-87 slot; de-forks). Description improvement filed upstream as clide FR-2.

T-1102 — workflow skills

  • whats-next / workshop-start / pr-review / pr-process de-sprinted, repo-root paths made dynamic, checks aligned to the pre-push gate, Task→Agent renames
  • New helpers tooling/godot-cold-parse + tooling/pr-watchlist-diff (allowlist entries deferred to first-use)

T-1103 — asset / tooling skills

  • atlas / glb-gen / image-gen / audio-gen / sprite-gen / bug-report / ticket corrected against live commands; render scripts hardened
  • image-gen: output-path bug fixed; local image_connector.py fork removed in favour of canonical tooling/db/image_connector.py

T-1104 — agent roster

  • Cleared stale STANDBY markers + dead DISCUSSION.md pointers; disambiguated overlapping personas
  • Armed 7 discussion agents (gestalt/gore/mellanie/miri/nigel/ozzie/paula) with Bash so the pql "before starting work" preamble is runnable
  • Integrated inigo (audio, standby) into README/TEAM + briefing
  • Removed tigerlocalization dropped from scope (R-013): template-writing volume + Gemma integration prohibitively expensive for a solo dev

Deferred / ticketed follow-ups

image-gen --aspect needs a paid-API smoke test; workshop "scout critic" idea; body-ID moon-suffix naming standard; renderer/README §12 staleness.

🤖 Generated with Claude Code

## Claude estate cleanup — 2026-07-02 `.claude` review (T-1099) Full sweep of the `CLAUDE-CLEANUP.md` estate-review brief (~60 findings). Every finding was **re-verified against the current tree** before editing (the repo moved since 2026-07-02; several findings no longer reproduced and were skipped, not force-applied). Tracked as initiative **T-1099** with epics T-1100–T-1104. Three parallel agents (E3/E4/E5) drafted the workflow-skill, asset/tooling-skill, and agent-roster sections; the lead handled settings/security and resolved all product-call flags with the team lead. ### T-1100 — settings / security - `GEMINI_API_KEY` invalidated + scrubbed to a placeholder (machine-local file, not in this PR) - `settings.json`: removed dead `docs-search` allow; promoted stable project-tool allows (godot4/gdformat/atlas/validate-ron); added repo-tree Edit/Write + worktree-setup allows - `.gitignore`: ignore `.claude/scheduled_tasks.lock`/`.json` ### T-1101 — skill removals - Deleted `d2-diagram`, `frame0-wireframe`, `skill-create` (live at user scope); repo-specific tailoring preserved in `docs/design/diagram-wireframe-conventions.md` - `clide` skill refreshed from the clide-repo canonical (picks up the dock/D-87 slot; de-forks). Description improvement filed upstream as clide **FR-2**. ### T-1102 — workflow skills - `whats-next` / `workshop-start` / `pr-review` / `pr-process` de-sprinted, repo-root paths made dynamic, checks aligned to the pre-push gate, Task→Agent renames - New helpers `tooling/godot-cold-parse` + `tooling/pr-watchlist-diff` (allowlist entries deferred to first-use) ### T-1103 — asset / tooling skills - `atlas` / `glb-gen` / `image-gen` / `audio-gen` / `sprite-gen` / `bug-report` / `ticket` corrected against live commands; render scripts hardened - `image-gen`: output-path bug fixed; local `image_connector.py` fork removed in favour of canonical `tooling/db/image_connector.py` ### T-1104 — agent roster - Cleared stale STANDBY markers + dead `DISCUSSION.md` pointers; disambiguated overlapping personas - Armed 7 discussion agents (gestalt/gore/mellanie/miri/nigel/ozzie/paula) with Bash so the pql "before starting work" preamble is runnable - Integrated `inigo` (audio, standby) into README/TEAM + briefing - Removed `tiger` — **localization dropped from scope (R-013)**: template-writing volume + Gemma integration prohibitively expensive for a solo dev ### Deferred / ticketed follow-ups image-gen `--aspect` needs a paid-API smoke test; workshop "scout critic" idea; body-ID moon-suffix naming standard; `renderer/README` §12 staleness. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Author
Owner

Review: claude-estate-cleanup → main (type: code)

Hoshe (Code Quality): REQUEST_CHANGES

Execution rigorous in most areas — roster sweep, whats-next, and image_connector fixes verified against live CLI/code behavior (atlas corridor-status, setparent none, T-615 all check out). But several defects slipped re-verification.

# File Issue
H1 tooling/godot-cold-parse:33-45 pipefail + trailing `
H2 tooling/pr-watchlist-diff:20 mapfile < <(python3 …): process-substitution failure invisible to set -e — registry crash yields empty watchlist, silently disabling the DB-staleness net
H3 .claude/settings.json Brief §1.2 deny-list :* normalization ("deny list first") skipped without note; the new allow hunk itself mixes space-glob and :* syntaxes
H4 .claude/settings.json New mandatory helper scripts (godot-cold-parse, pr-watchlist-diff) got no allow entries — every /pr-process run prompts
H5 tooling/db/common.py:78-98 API-key trap moved, not killed: shared get_api_key() still falls back to tracked config.json and its error text steers users to commit the paid key
H6 TEAM.md:34-43 Standby table still lists STIG/DUDLEY/JUSTINE/MELLANIE — contradicts this PR's own README active/standby split
H7 .claude/agents/README.md:109-112 Troblum usage note still asserts "never works alone" — contradicts this PR's rewritten troblum.md solo-profiling routing
H8 .claude/skills/pr-review/SKILL.md:9 Frontmatter grants Task not Agent — the identical naming bug this PR fixed in workshop-start
H9 docs/design/diagram-wireframe-conventions.md Preservation doc wrong vs. real tree: no insert/ wireframe dir (files live under hud/); d2 taxonomy lists 3 of 6 real categories

Tyre (Architecture): REQUEST_CHANGES

Fidelity to the brief is real and the deviations show good judgment, but the tracked permission baseline regressed and there are concrete misses.

# File Issue
T1 .claude/settings.json = H3 — normalize deny list to :* (add-only, never drop a deny rule) or document why space-glob is safe
T2 .claude/settings.json:68-69 Repo-wide Edit/Write allow in the TRACKED file lets any teammate edit settings.json / the git-centralize guard itself (guard inspects Bash only; hook script re-read from disk each call). Machine-local ask-gates don't protect other checkouts. Exclude the leash files
T3 governance/rejected/scope.md R-013 missing house-style **Cross-reference:** — D-123 (Generative AI for NPC content) + D-138 (LLM Re-voicing) are the natural targets
T4 governance/README.md Rejected index ends at R-012 — R-013 not indexed; regeneration step not run/committed
T5 docs/briefings/dudley.md:18 Status section still "STANDBY", contradicting this PR's dudley.md rewrite; brief §15.1 named this file "in the same pass"
T6 .claude/skills/pr-process/SKILL.md step 7 The pr create block carries the omit---repo half of the tea rule but not run-from-main — the skill's cwd is the worktree, so as written step 7 hits the exact "local repository required" failure it cites
T7 .claude/settings.json = H4 — allow entries for the two new mandatory scripts

Verdict: CHANGES REQUESTED — 14 unique findings (T1=H3, T7=H4)

Lead spot-checks verified every sampled claim (R-011/R-012 Cross-reference precedent, dudley briefing line 18, README index ending at R-012, D-123/D-138 existence). No retractions — all 14 will be fixed on the branch.

Process notes: no game code in the diff (runtime/merge-path smoke N/A); tooling lint+tests passed at the push gate; worktree tip verified c9d9b5693 for both reviewers.

## Review: claude-estate-cleanup → main (type: code) ### Hoshe (Code Quality): REQUEST_CHANGES Execution rigorous in most areas — roster sweep, whats-next, and image_connector fixes verified against live CLI/code behavior (atlas `corridor-status`, `setparent none`, T-615 all check out). But several defects slipped re-verification. | # | File | Issue | |---|------|-------| | H1 | tooling/godot-cold-parse:33-45 | pipefail + trailing `|| true` masks godot's own exit code — a crash/missing binary still reports "clean". Check `${PIPESTATUS[0]}` | | H2 | tooling/pr-watchlist-diff:20 | `mapfile < <(python3 …)`: process-substitution failure invisible to `set -e` — registry crash yields empty watchlist, silently disabling the DB-staleness net | | H3 | .claude/settings.json | Brief §1.2 deny-list `:*` normalization ("deny list first") skipped without note; the new allow hunk itself mixes space-glob and `:*` syntaxes | | H4 | .claude/settings.json | New mandatory helper scripts (godot-cold-parse, pr-watchlist-diff) got no allow entries — every /pr-process run prompts | | H5 | tooling/db/common.py:78-98 | API-key trap moved, not killed: shared `get_api_key()` still falls back to tracked config.json and its error text steers users to commit the paid key | | H6 | TEAM.md:34-43 | Standby table still lists STIG/DUDLEY/JUSTINE/MELLANIE — contradicts this PR's own README active/standby split | | H7 | .claude/agents/README.md:109-112 | Troblum usage note still asserts "never works alone" — contradicts this PR's rewritten troblum.md solo-profiling routing | | H8 | .claude/skills/pr-review/SKILL.md:9 | Frontmatter grants `Task` not `Agent` — the identical naming bug this PR fixed in workshop-start | | H9 | docs/design/diagram-wireframe-conventions.md | Preservation doc wrong vs. real tree: no `insert/` wireframe dir (files live under `hud/`); d2 taxonomy lists 3 of 6 real categories | ### Tyre (Architecture): REQUEST_CHANGES Fidelity to the brief is real and the deviations show good judgment, but the tracked permission baseline regressed and there are concrete misses. | # | File | Issue | |---|------|-------| | T1 | .claude/settings.json | = H3 — normalize deny list to `:*` (add-only, never drop a deny rule) or document why space-glob is safe | | T2 | .claude/settings.json:68-69 | Repo-wide Edit/Write allow in the TRACKED file lets any teammate edit settings.json / the git-centralize guard itself (guard inspects Bash only; hook script re-read from disk each call). Machine-local ask-gates don't protect other checkouts. Exclude the leash files | | T3 | governance/rejected/scope.md | R-013 missing house-style `**Cross-reference:**` — D-123 (Generative AI for NPC content) + D-138 (LLM Re-voicing) are the natural targets | | T4 | governance/README.md | Rejected index ends at R-012 — R-013 not indexed; regeneration step not run/committed | | T5 | docs/briefings/dudley.md:18 | Status section still "STANDBY", contradicting this PR's dudley.md rewrite; brief §15.1 named this file "in the same pass" | | T6 | .claude/skills/pr-process/SKILL.md step 7 | The pr create block carries the omit---repo half of the tea rule but not run-from-main — the skill's cwd is the worktree, so as written step 7 hits the exact "local repository required" failure it cites | | T7 | .claude/settings.json | = H4 — allow entries for the two new mandatory scripts | ### Verdict: CHANGES REQUESTED — 14 unique findings (T1=H3, T7=H4) Lead spot-checks verified every sampled claim (R-011/R-012 Cross-reference precedent, dudley briefing line 18, README index ending at R-012, D-123/D-138 existence). No retractions — all 14 will be fixed on the branch. Process notes: no game code in the diff (runtime/merge-path smoke N/A); tooling lint+tests passed at the push gate; worktree tip verified c9d9b5693 for both reviewers.
jpmschweitzer added 1 commit 2026-07-13 22:12:26 +02:00
H1 godot-cold-parse exit-code guard (+bonus: import-pass for cold checkouts, found live); H2 pr-watchlist-diff loud registry failure; H3/T1 deny list :* normalization (add-only) + uniform allow syntax; H4/T7 helper-script allows; H5 get_api_key env-only (config.json is tracked — no secret fallback); H6 TEAM.md active/standby split; H7 troblum solo-profiling note; H8 pr-review frontmatter Task->Agent; H9 conventions doc taxonomies fixed vs real tree; T2 ask-gate leash files (settings/hooks) in tracked settings; T3 R-013 cross-reference; T4 governance README index regenerated (pql decisions sync); T5 dudley briefing ACTIVE; T6 pr-process step 7 run-from-main.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Author
Owner

Review round processed — all 14 findings fixed (commit 6c078fbec)

No retractions. Per-finding disposition:

# Fix
H1 godot-cold-parse: godot invocation captured separately, exit code checked before filtering (crash ≠ clean). Bonus fix found during live verification: a truly cold checkout (fresh clone/worktree) has no .godot/imported/ cache and false-positived on every imported asset — the script now runs a one-time --import pass when the cache is absent. Verified: cold worktree run → import pass → clean
H2 pr-watchlist-diff: registry loaded via plain assignment (visible to set -e) + explicit empty-output guard. Verified on the live PR range (correctly empty — no generator sources touched)
H3/T1 Deny list normalized add-only: every space-glob rule kept, :* and bare-command forms added (git push --force, git reset --hard, git clean -f argless were previously uncovered). New allow hunk made uniformly :*
H4/T7 Bash(tooling/godot-cold-parse:*) + Bash(tooling/pr-watchlist-diff:*) allows added
H5 get_api_key() is env-only; config.json fallback removed (tracked file = endpoints only), error message now points at the machine-local settings env block and explicitly warns against config.json
H6 TEAM.md split into Implementation Team (active: dudley/stig/justine/mellanie) + Standby (oscar/inigo) — now agrees with agents/README.md
H7 README troblum note: solo for profiling/benchmarks/stress-tests, paired with Tyre for architecture second opinions
H8 pr-review frontmatter TaskAgent
H9 Conventions doc corrected against the real trees: 6 d2 categories; wireframe categories enumerated with the insert HUD under hud/
T2 Tracked settings.json gained an ask tier for the leash files (settings.json, settings.local.json, .claude/hooks/**, .config/hooks/**, + worktree copies) — Edit/Write on those prompts on every checkout, closing the guard-bypass
T3 R-013 cross-reference added: D-123, D-138 + roster removal pointers
T4 pql decisions sync run — governance/README.md index regenerated, R-013 indexed (387 synced, 0 broken)
T5 dudley briefing: frontmatter description + Status rewritten ACTIVE (Phase-4 cascade workstream), stale 2026-03-13 stamp updated
T6 pr-process step 7 NB now carries both halves: omit --repo AND run from the main checkout (with the exact cd incantation from a worktree)

Verification: bash -n both scripts, settings.json JSON-parses, live runs of both scripts (above), R-013 confirmed in the regenerated index. Push gate (tooling tests + ruff) covers the common.py change.

## Review round processed — all 14 findings fixed (commit 6c078fbec) No retractions. Per-finding disposition: | # | Fix | |---|-----| | H1 | godot-cold-parse: godot invocation captured separately, exit code checked before filtering (crash ≠ clean). **Bonus fix found during live verification:** a truly cold checkout (fresh clone/worktree) has no `.godot/imported/` cache and false-positived on every imported asset — the script now runs a one-time `--import` pass when the cache is absent. Verified: cold worktree run → import pass → `clean` | | H2 | pr-watchlist-diff: registry loaded via plain assignment (visible to `set -e`) + explicit empty-output guard. Verified on the live PR range (correctly empty — no generator sources touched) | | H3/T1 | Deny list normalized **add-only**: every space-glob rule kept, `:*` and bare-command forms added (`git push --force`, `git reset --hard`, `git clean -f` argless were previously uncovered). New allow hunk made uniformly `:*` | | H4/T7 | `Bash(tooling/godot-cold-parse:*)` + `Bash(tooling/pr-watchlist-diff:*)` allows added | | H5 | `get_api_key()` is env-only; config.json fallback removed (tracked file = endpoints only), error message now points at the machine-local settings env block and explicitly warns against config.json | | H6 | TEAM.md split into Implementation Team (active: dudley/stig/justine/mellanie) + Standby (oscar/inigo) — now agrees with agents/README.md | | H7 | README troblum note: solo for profiling/benchmarks/stress-tests, paired with Tyre for architecture second opinions | | H8 | pr-review frontmatter `Task` → `Agent` | | H9 | Conventions doc corrected against the real trees: 6 d2 categories; wireframe categories enumerated with the insert HUD under `hud/` | | T2 | Tracked settings.json gained an `ask` tier for the leash files (settings.json, settings.local.json, `.claude/hooks/**`, `.config/hooks/**`, + worktree copies) — Edit/Write on those prompts on every checkout, closing the guard-bypass | | T3 | R-013 cross-reference added: D-123, D-138 + roster removal pointers | | T4 | `pql decisions sync` run — governance/README.md index regenerated, R-013 indexed (387 synced, 0 broken) | | T5 | dudley briefing: frontmatter description + Status rewritten ACTIVE (Phase-4 cascade workstream), stale 2026-03-13 stamp updated | | T6 | pr-process step 7 NB now carries both halves: omit `--repo` AND run from the main checkout (with the exact `cd` incantation from a worktree) | Verification: `bash -n` both scripts, settings.json JSON-parses, live runs of both scripts (above), R-013 confirmed in the regenerated index. Push gate (tooling tests + ruff) covers the common.py change.
jpmschweitzer closed this pull request 2026-07-13 22:51:45 +02:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: jpmschweitzer/settled-reach#175