From e34f27bfacad004e93ecd9f540cc8bfbc64ef950 Mon Sep 17 00:00:00 2001 From: Chris Dowin Date: Wed, 14 Oct 2026 21:02:28 -0400 Subject: [PATCH] Ship Godot UI skill and runtime installer in 3.9.0 --- CLAUDE.md | 2 +- README.md | 6 + pyproject.toml | 7 +- src/godot_devkit/__init__.py | 2 +- src/godot_devkit/cli.py | 7 + .../installables/skills/godot-ui/SKILL.md | 89 +++++++++ .../skills/godot-ui/agents/openai.yaml | 2 + src/godot_devkit/godot/skill_install.py | 184 ++++++++++++++++++ tests/test_docs_ratchet.py | 7 +- tests/test_skill_install.py | 129 ++++++++++++ 10 files changed, 429 insertions(+), 6 deletions(-) create mode 100644 src/godot_devkit/godot/installables/skills/godot-ui/SKILL.md create mode 100644 src/godot_devkit/godot/installables/skills/godot-ui/agents/openai.yaml create mode 100644 src/godot_devkit/godot/skill_install.py create mode 100644 tests/test_skill_install.py diff --git a/CLAUDE.md b/CLAUDE.md index b81bb04..08a6ffb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -114,7 +114,7 @@ incantation: `make help` lists every target, and a check that is not a target ge - **Docs for people live in the wiki**, https://github.com/cdowin/godot-devkit/wiki. You may read it. Only a local session publishes there; a cloud session drafts the wiki text in its PR body and the issue is marked blocked-by Chris (local session). This repo keeps no `docs/` and no `.md` outside `README.md`, `CLAUDE.md`, - `AGENTS.md` (`tests/test_docs_ratchet.py`); `README.md` stays under 80 lines. + `AGENTS.md` (`tests/test_docs_ratchet.py`); The bundled machine-loaded `godot-ui/SKILL.md` is also allowed. `README.md` stays under 80 lines. - **Release:** bump `version` in `pyproject.toml` and `__version__` together on `main`; `auto-tag.yml` tags it and `release.yml` publishes the wheel to the index. Never tag by hand. - Report in ASD-STE100: numbered NEEDS YOU list first, outcome first, one fact per sentence, numbers not adjectives. diff --git a/README.md b/README.md index 5b4dfa3..c2544f6 100644 --- a/README.md +++ b/README.md @@ -65,3 +65,9 @@ parser handles any Godot 4.x `.tscn`/`.tres`. ## License MIT — see [LICENSE](LICENSE). + +## Agent UI skill + +`godot-devkit install-skills --runtime codex` installs `godot-ui` in `.agents/skills/`; +`--runtime claude` uses `.claude/skills/`. `--dest ` selects another scope. +Use `--check` to verify installed files. The skill maps UI contracts to native scene proof. diff --git a/pyproject.toml b/pyproject.toml index 83c4703..889b3ad 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "godot-devkit" -version = "3.8.0" +version = "3.9.0" description = "Headless dev tooling for Godot 4.x - pure-parse scene introspection, surgical scene, TileMapLayer and autoload edits, the eight Godot gates, and the test-tier runners." readme = "README.md" requires-python = ">=3.11" @@ -39,4 +39,7 @@ markers = [ packages = ["src/godot_devkit"] # The installables are read at runtime via importlib.resources — they must be # in the wheel, not just the sdist. -artifacts = ["src/godot_devkit/godot/installables/*"] +artifacts = [ + "src/godot_devkit/godot/installables/*", + "src/godot_devkit/godot/installables/skills/godot-ui/**", +] diff --git a/src/godot_devkit/__init__.py b/src/godot_devkit/__init__.py index 61dc4a8..aa29a2f 100644 --- a/src/godot_devkit/__init__.py +++ b/src/godot_devkit/__init__.py @@ -1,2 +1,2 @@ """godot-devkit — headless scene introspection + static repo gates for Godot 4.x.""" -__version__ = '3.8.0' +__version__ = '3.9.0' diff --git a/src/godot_devkit/cli.py b/src/godot_devkit/cli.py index c93f1cd..21a6808 100644 --- a/src/godot_devkit/cli.py +++ b/src/godot_devkit/cli.py @@ -85,6 +85,10 @@ # library tools/dev/gdk_gate.sh. A game sets # GDK_CHECKS and includes Makefile.gates; # it needs nothing from agentic-sdlc + godot-devkit install-skills [--runtime codex|claude] [--dest ROOT] + [--check] [--force] [--diff] + # install the bundled godot-ui skill into + # a skills root; --check reports drift Static gates (exit 1 on findings; run from anywhere inside the repo): godot-devkit check uid [--fix] | tres | props | defaults | rng @@ -300,6 +304,9 @@ def main(argv: list[str] | None = None) -> int: if cmd == 'install-gates': from godot_devkit.godot import install return install.main_gates(rest) + if cmd == 'install-skills': + from godot_devkit.godot import skill_install + return skill_install.main(rest) if cmd in RETIRED_COMMANDS: return _retired(cmd) if cmd == 'check': diff --git a/src/godot_devkit/godot/installables/skills/godot-ui/SKILL.md b/src/godot_devkit/godot/installables/skills/godot-ui/SKILL.md new file mode 100644 index 0000000..5b6fbc0 --- /dev/null +++ b/src/godot_devkit/godot/installables/skills/godot-ui/SKILL.md @@ -0,0 +1,89 @@ +--- +name: godot-ui +description: Design, implement, or review native Godot menus, HUDs, dialogs, cards, and Control layouts. Translate approved wireframes into reusable scenes with explicit layout contracts and native visual proof. Use for resizing, clipping, overlap, scrolling, and selection-state layout defects; not simulation-only work or browser-only UI. +--- + +# Godot UI: component contract to native proof + +Treat the user's task and approved wireframe as the design input. Identify the spatial and +interaction intent before changing a dimension. A stable panel that obscures the task still fails. +Use this workflow for the affected surface; reuse existing contracts for a small edit. + +## Connect the existing tools + +- Read project instructions, the UI/theme contract, and the installed Godot version. Use the + project's sandboxed build, scenario and capture commands. This skill grants no extra permissions. +- Inventory the nearest native shell, identity badge, list row, picker, theme and component gallery. + Reuse or refactor those before introducing another component with the same responsibility. +- When available, read `ui-patterns` for shared interaction principles. Apply `visual-layout` and + `usability-review` to relevant native evidence when doing visual acceptance or a usability audit. + Their web/CSS capture sizes and tools do not replace the project's Godot viewports and input modes. + These optional skills are independent; this workflow also works without another skill package. +- Devkit scene inspection and `scene-diff` prove authored structure. Static gates do not establish + rendered fit or usability. Headless execution supplies behavior evidence, not rendered pixels. + +## Define the component contract + +Record a compact contract in the existing issue or design record, not a new documentation system: + +| Field | Define | +| --- | --- | +| Purpose | What the player needs to see or do; which approved design expresses it. | +| Composition | Named regions and reusable child components; map each to its native scene/node. | +| Reading and events | Data supplied by the presenter, derived counts, selection state, and emitted intents. Keep simulation facts in their existing owners. | +| Geometry owner | Which parent allocates each region; reserved identity lines and control slots; viewport/text-scale reflow rules. | +| Overflow | Which interior scrolls, wraps or truncates; how full text remains available; what must stay visible and pinned. | +| States | Relevant empty/populated, selected/unselected, long-content and enabled/disabled cases. | +| Motion and overlays | What may intentionally move, permitted overlays, and Reduce Motion behavior. | +| Evidence | State fixture and native check/capture that can refute each important claim. | + +Persistent chrome keeps its bounds across content and selection changes. Natural-height content +rows may grow inside their allocated scroller. A selected graph node may intentionally recenter +inside its graph region. Distinguish these cases instead of forbidding every size or position change. + +## Translate into Godot deliberately + +- Author layout in scenes using the project's conventions. In a `Container`, the parent manages + child positions and sizes; hand-set offsets are not a substitute for the right hierarchy. +- `custom_minimum_size` is a floor, not a fixed rectangle. Content, fonts and visible children can + contribute a larger minimum. `Expand`/`Fill` alone do not establish a bounded scroll viewport. +- Choose the sizing/minimum-propagation behavior for each allocated region. Put scroll boundaries + around content that may exceed that region; keep pinned headers, filters and actions outside it. + Use the project's existing bounded-region primitive when there is one. +- Reserve the same structural slots for empty and populated states. Hiding a child can remove its + contribution to a container's minimum size. Check name wrapping, portrait alignment and empty + versus filled controls without allowing their incidental minima to rearrange the whole surface. +- Use actual Theme font metrics and spacing. MarginContainer/StyleBox margins and container + separation are distinct; an arbitrary margin override on a BoxContainer does not create padding. +- Keep the wireframe's visual hierarchy, tokens, identity composition and information density. + Explicitly account for native scrollbar width, safe areas, focus, text scale and touch targets. +- Custom-drawn graph labels and marks still need allocated space and measurable bounds. A sibling + inside the outer card may obscure them. Report those bounds through a read-only probe if needed. + +## Build and prove the affected slice + +Use the approved fixture in the real components/gallery, then representative stress states from +real domain readings. Preserve real counts, roles and supported types; example wireframe data does +not establish a simulation feature. Scale proof to the change rather than multiplying boot runs. + +Extend the existing appropriate journey or component proof. Check relevant claims such as stable +region bounds across selection/content transitions, disjoint primary regions, required-visible +labels/actions, reserved slots, and scroll ownership. Account for declared overlays and intentionally +offscreen scroll rows. An outer-card containment check alone cannot establish these claims. + +Capture the native surface at the project's target viewport and representative enlarged text state. +Confirm the effective viewport/text scale, not just the requested window size. Use real controls or +the project's supported input signals; activating an invisible button does not prove a usable path. + +Read the actual scenario result and error count as well as the capture result. A PNG being written +only proves that an image exists; some capture runners deliberately use that as their success +criterion. Keep “image produced,” “scenario passed,” and “visually accepted” as separate claims. + +Compare the native capture to the approved design and walk the player's task. Inspect empty and +populated transitions, long names, scrolling, focus and relevant touch behavior. Reject unintended +movement, obscured content and misleading counts even when structural tests pass. State missing +render/input evidence as unverified; do not certify appearance from markup or headless output. + +Keep a small evidence handoff: requirement → component/state → check or capture → result. +Name material deviations and unresolved limits. Apply independent review when required by the +project or the change's risk; no blanket approval step is added by this skill. diff --git a/src/godot_devkit/godot/installables/skills/godot-ui/agents/openai.yaml b/src/godot_devkit/godot/installables/skills/godot-ui/agents/openai.yaml new file mode 100644 index 0000000..fcf9655 --- /dev/null +++ b/src/godot_devkit/godot/installables/skills/godot-ui/agents/openai.yaml @@ -0,0 +1,2 @@ +policy: + allow_implicit_invocation: true diff --git a/src/godot_devkit/godot/skill_install.py b/src/godot_devkit/godot/skill_install.py new file mode 100644 index 0000000..045311c --- /dev/null +++ b/src/godot_devkit/godot/skill_install.py @@ -0,0 +1,184 @@ +"""Install the bundled godot-ui skill into a Codex or Claude skills root. + +The destination is a skills root, not a project root. Existing differing files +require --force. Every path is checked before the mutation plan runs. +""" +from __future__ import annotations + +import difflib +import sys +from importlib import resources +from pathlib import Path + +from godot_devkit.core import apply +from godot_devkit.core.project import repo_root +from godot_devkit.godot.install import destination_defect, read_destination + +COMMAND = 'install-skills' +PACKAGE = 'godot_devkit.godot.installables' +FILES = ('SKILL.md', 'agents/openai.yaml') +USAGE = ('usage: godot-devkit install-skills [--runtime codex|claude] ' + '[--dest ROOT] [--check] [--force] [--diff]\n' + 'Install the bundled godot-ui skill. ROOT is a skills directory; ' + 'the default is .agents/skills for Codex or .claude/skills for Claude.') + + +def _source(name: str) -> str: + path = resources.files(PACKAGE) + for part in ('skills', 'godot-ui', *name.split('/')): + path = path.joinpath(part) + return path.read_text(encoding='utf-8') + + +def _options(argv: list[str]) -> tuple[str, Path | None, bool, bool, bool] | None: + runtime = 'codex' + dest: Path | None = None + check = force = diff = False + index = 0 + while index < len(argv): + arg = argv[index] + if arg in ('--runtime', '--dest'): + if index + 1 >= len(argv) or argv[index + 1].startswith('--'): + print(f'godot-devkit {COMMAND}: {arg} needs a value', file=sys.stderr) + return None + value = argv[index + 1] + if arg == '--runtime': + runtime = value + else: + dest = Path(value).expanduser() + index += 2 + continue + if arg == '--check': + check = True + elif arg == '--force': + force = True + elif arg == '--diff': + diff = True + else: + print(f'godot-devkit {COMMAND}: unknown argument {arg!r}', + file=sys.stderr) + return None + index += 1 + if runtime not in ('codex', 'claude'): + print(f'godot-devkit {COMMAND}: unknown runtime {runtime!r} ' + '(expected codex or claude)', file=sys.stderr) + return None + if check and force: + print(f'godot-devkit {COMMAND}: --check and --force cannot be combined', + file=sys.stderr) + return None + return runtime, dest, check, force, diff + + +def _difference(label: str, existing: str | None, desired: str) -> None: + if existing is None: + print(f'[install-skills] {label}: existing content is not UTF-8 text') + return + sys.stdout.writelines(difflib.unified_diff( + existing.splitlines(keepends=True), desired.splitlines(keepends=True), + fromfile=f'a/{label}', tofile=f'b/{label}')) + + +def _symlink_below_root(root: Path, target: Path) -> Path | None: + """Find a symlink in the installed path, excluding the chosen skills root. + + The root can itself be a deliberate symlink (for a global skills store). + Directories the verb creates below it, and each file slot, must stay inside + that selected tree even under --force. + """ + cursor = root + for part in target.relative_to(root).parts: + cursor = cursor / part + if cursor.is_symlink(): + return cursor + return None + + +def main(argv: list[str]) -> int: + if any(arg in ('-h', '--help', 'help') for arg in argv): + print(USAGE) + return 0 + options = _options(argv) + if options is None: + print(USAGE, file=sys.stderr) + return 2 + runtime, dest, check, force, diff = options + root = dest if dest is not None else repo_root() / ( + '.agents/skills' if runtime == 'codex' else '.claude/skills') + try: + entries = [(root / 'godot-ui' / name, f'godot-ui/{name}', _source(name)) + for name in FILES] + except (OSError, UnicodeError) as err: + print(f'godot-devkit {COMMAND}: bundled skill cannot be read: {err}', + file=sys.stderr) + return 2 + + defects: list[str] = [] + collisions: list[str] = [] + pending: list[tuple[Path, str, str]] = [] + drift = False + for target, label, desired in entries: + # A selected skills root can be a symlink. A symlink in the installed + # path below it can escape that root, including when it is a directory. + symlink = _symlink_below_root(root, target) + defect = (f'{symlink} is a symlink below the skills root' if symlink + else destination_defect(target)) + if defect: + defects.append(f'{label} {defect}') + continue + existing: str | None = None + if target.exists(): + existing, unreadable = read_destination(target) + if unreadable: + defects.append(f'{label} {unreadable}') + continue + if existing == desired: + print(f'[install-skills] {label} already current') + continue + drift = True + if diff: + if not target.exists(): + print(f'[install-skills] {label} missing — the whole file is an addition') + _difference(label, existing if target.exists() else '', desired) + elif check: + print(f'[install-skills] {label} ' + f'{"outdated" if target.exists() else "missing"}') + if target.exists() and not force: + collisions.append(label) + pending.append((target, label, desired)) + + if defects: + for defect in defects: + print(f'godot-devkit {COMMAND}: {defect}', file=sys.stderr) + for label in collisions: + print(f'godot-devkit {COMMAND}: {label} exists and differs; ' + 'move it aside or use --force', file=sys.stderr) + print(f'godot-devkit {COMMAND}: nothing was written', file=sys.stderr) + return 1 + if check: + return 1 if drift else 0 + if diff: + return 0 + if collisions: + for label in collisions: + print(f'godot-devkit {COMMAND}: {label} exists and differs; ' + 'move it aside or use --force', file=sys.stderr) + print(f'godot-devkit {COMMAND}: nothing was written', file=sys.stderr) + return 1 + + plan = apply.Plan() + for target, label, desired in pending: + plan.overwrite(target, desired, label=label) + outcome = plan.apply() + if outcome.blocked: + for blocked in outcome.blocked: + print(f'godot-devkit {COMMAND}: {blocked.describe()}', file=sys.stderr) + return 1 + for step in outcome.landed: + print(f'[install-skills] wrote {step.label}') + if outcome.failed is not None: + print(f'godot-devkit {COMMAND}: {outcome.failed.label} could not be ' + f'written ({outcome.error}); {len(outcome.landed)} file(s) landed', + file=sys.stderr) + return 1 + return 0 diff --git a/tests/test_docs_ratchet.py b/tests/test_docs_ratchet.py index c8af3f6..f037aef 100644 --- a/tests/test_docs_ratchet.py +++ b/tests/test_docs_ratchet.py @@ -1,14 +1,17 @@ """test_docs_ratchet.py — docs for people live in the wiki, so the repo cannot grow them back. Fails when a tracked file is a `docs/` path, a `CHANGELOG*` file, or a `.md` outside the -allowlist (README.md, CLAUDE.md, AGENTS.md, and `.md` fixtures under tests/fixtures/). +allowlist (root entrypoints, the bundled agent skill, and `.md` test fixtures). README.md is a landing page: at most README_MAX_LINES lines. """ import subprocess from pathlib import Path ROOT = Path(__file__).resolve().parent.parent -ALLOWED_MD = {"README.md", "CLAUDE.md", "AGENTS.md"} +ALLOWED_MD = { + "README.md", "CLAUDE.md", "AGENTS.md", + "src/godot_devkit/godot/installables/skills/godot-ui/SKILL.md", +} README_MAX_LINES = 80 diff --git a/tests/test_skill_install.py b/tests/test_skill_install.py new file mode 100644 index 0000000..048ceb8 --- /dev/null +++ b/tests/test_skill_install.py @@ -0,0 +1,129 @@ +"""The godot-ui skill installer: currentness, refusal, and root selection.""" +from __future__ import annotations + +from pathlib import Path + +from godot_devkit.godot import skill_install + + +def _bundled(name: str) -> str: + return {'SKILL.md': '# Bundled skill\n', + 'agents/openai.yaml': 'name: godot-ui\n'}[name] + + +def test_default_roots_check_and_idempotence(tmp_path, monkeypatch, capsys): + monkeypatch.setattr(skill_install, 'repo_root', lambda: tmp_path) + codex = tmp_path / '.agents' / 'skills' / 'godot-ui' + claude = tmp_path / '.claude' / 'skills' / 'godot-ui' + + assert skill_install.main(['--check']) == 1 + assert 'missing' in capsys.readouterr().out + assert not codex.exists() + assert skill_install.main([]) == 0 + capsys.readouterr() + assert (codex / 'SKILL.md').read_text() == skill_install._source('SKILL.md') + assert (codex / 'agents' / 'openai.yaml').read_text() == skill_install._source('agents/openai.yaml') + before = {p: p.stat().st_mtime_ns for p in codex.rglob('*') if p.is_file()} + assert skill_install.main([]) == 0 + assert 'already current' in capsys.readouterr().out + assert {p: p.stat().st_mtime_ns for p in before} == before + assert skill_install.main(['--check']) == 0 + capsys.readouterr() + + assert skill_install.main(['--runtime', 'claude']) == 0 + capsys.readouterr() + assert (claude / 'SKILL.md').read_text() == skill_install._source('SKILL.md') + assert (codex / 'SKILL.md').read_text() == skill_install._source('SKILL.md') + + +def test_collision_preflight_force_and_read_only_diff(tmp_path, monkeypatch, capsys): + monkeypatch.setattr(skill_install, '_source', _bundled) + root = tmp_path / 'global-skills' + skill = root / 'godot-ui' + skill.mkdir(parents=True) + prior = skill / 'SKILL.md' + prior.write_text('# Personal changes\n') + + assert skill_install.main(['--dest', str(root)]) == 1 + assert 'nothing was written' in capsys.readouterr().err + assert prior.read_text() == '# Personal changes\n' + assert not (skill / 'agents' / 'openai.yaml').exists() + + assert skill_install.main(['--dest', str(root), '--check', '--diff']) == 1 + output = capsys.readouterr().out + assert '--- a/godot-ui/SKILL.md' in output + assert 'missing' in output + assert prior.read_text() == '# Personal changes\n' + assert not (skill / 'agents' / 'openai.yaml').exists() + + assert skill_install.main(['--dest', str(root), '--force']) == 0 + capsys.readouterr() + assert prior.read_text() == _bundled('SKILL.md') + assert (skill / 'agents' / 'openai.yaml').read_text() == _bundled('agents/openai.yaml') + + +def test_defects_and_symlink_never_write_or_follow(tmp_path, monkeypatch, capsys): + monkeypatch.setattr(skill_install, '_source', _bundled) + root = tmp_path / 'skills' + skill = root / 'godot-ui' + skill.mkdir(parents=True) + elsewhere = tmp_path / 'elsewhere.md' + elsewhere.write_text('# Keep this\n') + (skill / 'SKILL.md').symlink_to(elsewhere) + + assert skill_install.main(['--dest', str(root), '--force']) == 1 + assert 'symlink' in capsys.readouterr().err + assert elsewhere.read_text() == '# Keep this\n' + assert not (skill / 'agents' / 'openai.yaml').exists() + assert skill_install.main(['--dest', str(root), '--check']) == 1 + capsys.readouterr() + assert elsewhere.read_text() == '# Keep this\n' + + (skill / 'SKILL.md').unlink() + (skill / 'SKILL.md').mkdir() + assert skill_install.main(['--dest', str(root), '--force']) == 1 + assert 'directory' in capsys.readouterr().err + assert not (skill / 'agents' / 'openai.yaml').exists() + + +def test_symlinked_skill_directories_refuse_but_selected_root_may_be_symlink( + tmp_path, monkeypatch, capsys): + monkeypatch.setattr(skill_install, '_source', _bundled) + for directory in ('godot-ui', 'agents'): + root = tmp_path / directory / 'skills' + skill = root / 'godot-ui' + outside = tmp_path / directory / 'outside' + outside.mkdir(parents=True) + if directory == 'godot-ui': + root.mkdir(parents=True) + skill.symlink_to(outside, target_is_directory=True) + protected = outside / 'SKILL.md' + else: + skill.mkdir(parents=True) + (skill / 'agents').symlink_to(outside, target_is_directory=True) + protected = outside / 'openai.yaml' + protected.write_text('personal content\n') + + assert skill_install.main(['--dest', str(root), '--force']) == 1 + assert 'symlink below the skills root' in capsys.readouterr().err + assert protected.read_text() == 'personal content\n' + if directory == 'agents': + assert not (skill / 'SKILL.md').exists() + + selected = tmp_path / 'selected-skills' + selected.mkdir() + link = tmp_path / 'explicit-root-link' + link.symlink_to(selected, target_is_directory=True) + assert skill_install.main(['--dest', str(link)]) == 0 + capsys.readouterr() + assert (selected / 'godot-ui' / 'SKILL.md').read_text() == _bundled('SKILL.md') + + +def test_usage_refuses_unknown_runtime_and_check_force(tmp_path, monkeypatch, capsys): + monkeypatch.setattr(skill_install, '_source', _bundled) + for argv in (['--runtime', 'unknown', '--dest', str(tmp_path)], + ['--check', '--force', '--dest', str(tmp_path)], + ['--dest']): + assert skill_install.main(argv) == 2 + capsys.readouterr() + assert list(tmp_path.iterdir()) == []