Weekly tech debt audit: windowstead - 2026-05-20

[itsmiso-ai]

Summary / overall risk level

Overall risk: Medium. The repo is in better shape than the recent layout/UI churn suggests: current headless checks pass, Godot downloads are checksum-verified, and the app has meaningful script/layout/E2E coverage. The main risk is now systemic: core simulation, UI state, dock geometry, persistence, and rendering still converge in one very large script, so small model-generated patches can keep fixing symptoms while leaving shared invariants unowned.

No P0 findings this week.

Top findings

P1 — Worker task assignment has no reservation model, causing resource over-delivery / wasted work

Risk: Product correctness / simulation economy drift.

Evidence:

• scripts/main.gd:1114-1123 chooses the nearest task independently for each worker.
• scripts/main.gd:1147-1156 emits haul tasks whenever a build has any remaining need and stockpile has any amount, but does not account for resources already reserved/carried by another worker.
• scripts/main.gd:1199-1224 decrements stockpile and later increments build.delivered[resource] without clamping delivery to remaining need or refunding excess.
• Throwaway reproduction script run outside the repo (/tmp/windowstead_audit_overdeliver.gd) set up one garden needing 1 stone and two workers assigned the same haul task:

after_pickup resources.stone=0 carry=1,1
after_delivery delivered.stone=2 resources.stone=0 need.stone=1

Impact:

• A build can consume more resources than its configured cost.
• Workers can duplicate gather/haul trips against the same target.
• Current tests pass because they do not cover competing workers against one remaining resource/build need.

Recommended follow-up issue:

• fix: add task/resource reservations for worker gather and haul assignments
  • Track reserved build deliveries per resource.
  • Clamp delivery to remaining need and refund/return excess to stockpile.
  • Add regression coverage for two workers targeting one remaining resource/build need.

P1 — Validation matrix is inconsistent; important tests exist but are not all wired into the same gates

Risk: Regression leakage, especially layout and release regressions.

Evidence:

• Local .justfile validation runs only test + test-layout (.justfile:16-25), so it omits the E2E gameplay flow test and smoke startup test.
• PR CI runs test_runner.gd and test_e2e.gd (.github/workflows/test.yml:67-88), but does not run tests/test_layout_math.gd, despite that file being the targeted regression suite for dock geometry.
• macOS smoke test is explicitly continue-on-error: true (.github/workflows/test.yml:119-149), leaving only macOS script tests as the hard mac gate.
• Release workflow builds and uploads artifacts from tags (.github/workflows/release.yml:1-80) but does not run the same validation matrix before export/upload.
• tests/test_layout_math.gd:61-62 also has stale human-readable test text: it prints stockpile vertical: (2, 7) while the actual assertion expects (2, 9) (tests/test_layout_math.gd:256-258). Small, but it shows the test harness messaging can drift from behavior.

Recommended follow-up issue:

• ci: make validate/PR/release test gates use one canonical matrix
  • Add test-layout to PR CI.
  • Add E2E + smoke to just validate or create just ci and document the difference.
  • Run the canonical validation before release exports.
  • Fix stale layout test labels.

P1 — Save/version migration is partially centralized but still accepts unknown/raw shapes too easily

Risk: Save corruption/reset surprises as schema changes continue.

Evidence:

• scripts/game_state.gd:3-5 says current save key/version is v2.
• scripts/game_state.gd:47-59 migrates v1 to v2, but returns <1 and future/unknown versions raw for callers to interpret.
• scripts/main.gd:686-699 checks only structural compatibility after GameState.load_game(); is_save_compatible() later checks tile count and positions, not save version/schema semantics.
• Web load path in scripts/game_state.gd:25-33 returns parsed localStorage data directly instead of passing it through migrate_save(), unlike desktop file loads (scripts/game_state.gd:42-45).
• Docs drift: docs/DESIGN.md:105 still says save_version is currently 1 while code uses 2.
• Prior issue Version saves and migrate incompatible layouts safely #16 asked for predictable unknown/unmigratable behavior and migration-first ownership; the code is close, but not fully there.

Recommended follow-up issue:

• fix: harden save migration and schema compatibility checks
  • Run both desktop and web loads through the same migration path.
  • Treat unknown/future versions as explicit incompatible states with a user-facing event, not raw dictionaries.
  • Update docs to v2 and add tests for web-style localStorage parsing/migration.

P2 — scripts/main.gd is still the main blast-radius multiplier

Risk: Maintainability and model-generated inconsistency.

Evidence:

• scripts/main.gd is currently 1,862 lines and owns theme construction, startup/menu UI state, dock geometry, DisplayServer behavior, task selection, worker execution, rendering, persistence calls, save compatibility checks, and event text.
• docs/DESIGN.md:5 still describes the app as a single-scene / two-script architecture. That was reasonable for the MVP, but it is now where unrelated patches keep crossing wires.
• Recent closed issues/PRs show repeated UI-state/layout repair work: menu clipping/close flow (bug: main menu is clipped, scrollable, and lacks a clear close interaction #95/Fix main menu popup presentation #103), build/menu separation (bug: build button opens a merged menu instead of a separate build menu #93/joryirving/windowstead #93: separate build menu from main menu #100), placement soft-lock (bug: build placement can soft-lock controls after placing a hut #94/joryirving/windowstead #94: keep controls clickable after hut placement #96), dock/startup/layout consolidation (Add startup menu for new/load/settings/exit and lock dock orientation per game #106-Use LayoutMath as layout source of truth #114).

Recommended follow-up issue:

• refactor: split simulation, UI state, and dock geometry boundaries
  • Extract a pure simulation/task coordinator first, because it unlocks deterministic worker tests.
  • Keep LayoutMath as the model: small pure modules with direct tests.
  • Avoid a big rewrite PR; slice by invariants and tests.

P2 — Dependency/release supply-chain coverage is checksum-safe but manually maintained

Risk: Operational drift.

Evidence:

• Godot editor/template URLs and SHA256 values are hardcoded in both test/release workflows (.github/workflows/test.yml:9-14, .github/workflows/release.yml:21-29). Checksums are good, but version discovery/update is manual.
• Renovate dashboard Renovate Dashboard 🤖 #104 detects GitHub Actions and mise dependencies, but not these Godot binary/template URLs.
• Release workflow repeats download/install/export setup across Linux, Windows, and macOS jobs.

Recommended follow-up issue:

• ops: centralize Godot toolchain metadata and update path
  • Put Godot version/URLs/hashes in one reviewed source or composite action.
  • Document how/when to bump Godot and export templates.
  • Optional later: teach Renovate or a small scheduled check to flag Godot release drift.

P2 — Remote branch hygiene is noisy

Risk: Grooming noise and duplicate-work risk.

Evidence:

• git branch -r shows many old origin/fix/issue-* branches for already-closed work, alongside active Renovate branches.
• This does not break builds, but it makes it harder to distinguish active work from stale generated branches.

Recommended follow-up issue:

• ops: prune merged/generated fix branches after PR completion
  • Decide whether this belongs in GitHub settings, bot behavior, or a periodic maintenance task.
  • Do not delete active Renovate branches.

Recommended issue breakdown

1. P1 / area/product / area/testing — fix: add task/resource reservations for worker gather and haul assignments
2. P1 / area/ops / area/testing — ci: make validate/PR/release test gates use one canonical matrix
3. P1 / area/product / area/testing — fix: harden save migration and schema compatibility checks
4. P2 / area/product — refactor: split simulation, UI state, and dock geometry boundaries
5. P2 / area/security / area/ops — ops: centralize Godot toolchain metadata and update path
6. P2 / area/ops — ops: prune merged/generated fix branches after PR completion

Not worth doing yet

• No auth/authorization/CSRF/API-key follow-up for the game runtime right now. There is no backend/API surface in this repo; security energy is better spent on release supply chain and workflow permissions.
• No full rewrite of the Godot scene architecture. Extract pure seams with tests first; a grand rewrite would just manufacture a new swamp with nicer variable names.
• No screenshot/golden-image testing yet. Deterministic simulation/layout tests should come first; screenshots can wait until visual invariants stabilize.
• No urgent Godot engine upgrade just for freshness. First create a repeatable toolchain bump path, then evaluate upgrade cost.
• No branch deletion as part of this audit. Record the hygiene problem separately and clean it with explicit repo-owner intent.

Validation performed

Repository state:

git fetch origin && git checkout main && git pull origin main
HEAD: 20e7551 Merge pull request #120 from misospace/feat/pr-review-update
Working tree note: pre-existing untracked fix_menu.patch was present; audit did not modify repo files.

GitHub state inspected:

Open issues: only #104 Renovate Dashboard
Current-week audit issue search: none found before creating this issue
Recent PRs inspected: #120, #119, #118, #117, #116, #115, #114, #113, #112, #111, #110, etc.
Existing labels verified: enhancement, priority/p1, audit, area/ops, area/testing, area/security, area/product
Research helper run against current open issue #104 for repo/GitHub context.

Local checks:

git diff --check
# exit=0

./.tools/Godot_v4.2.2-stable_linux.x86_64 --version
# 4.2.2.stable.official.15073afe3

./.tools/Godot_v4.2.2-stable_linux.x86_64 --headless --path . --script res://tests/test_runner.gd
# === test_runner summary: 54 passed, 0 failed ===

./.tools/Godot_v4.2.2-stable_linux.x86_64 --headless --path . --script res://tests/test_layout_math.gd
# Passed: 30; Failed: 0

./.tools/Godot_v4.2.2-stable_linux.x86_64 --headless --path . --script res://tests/test_e2e.gd
# Results: 63/63 passed, 0 failed
# Existing Godot resource-leak warning emitted at exit.

timeout 15 ./.tools/Godot_v4.2.2-stable_linux.x86_64 --headless --path . --fixed-fps 60 --quit-after 300
# exit=0; no SCRIPT ERROR / ERROR lines

[itsmiso-ai]

Second-pass audit update — 2026-05-20

I found the current-week audit issue already existed, so this is an additive update rather than a duplicate issue.

Additional P1/P2 finding — AI reviewer workflow has overly broad tool/API scope

Risk: P1 if the GitHub App used by the reviewer has broad repo access; otherwise P2. The workflow is pinned and fork execution is disabled, which helps, but the tool layer is still allowed to call GitHub APIs across * repos.

Evidence:

• .github/workflows/ai-pr-review.yaml:13-15 grants pull-requests: write.
• .github/workflows/ai-pr-review.yaml:26-34 creates a GitHub App token from repo secrets.
• .github/workflows/ai-pr-review.yaml:50 sets tool_allowed_gh_api_repos: *.
• .github/workflows/ai-pr-review.yaml:58 sets tool_enable_for_forks: false, which mitigates fork PRs but does not constrain trusted/internal PR content or a compromised reviewer/tool path.

Recommended follow-up issue:

• security: constrain AI reviewer GitHub API tool scope
  • Replace tool_allowed_gh_api_repos: * with the minimum repo list needed for Windowstead review.
  • Document the reviewer threat model: which PR authors are trusted, what token can write, and which APIs the model/tool may call.
  • Keep fork tool execution disabled.
  • Consider a separate, least-privileged reviewer app/token if cross-repo access is genuinely needed.

This slots beside the existing release/supply-chain finding; it is not a runtime app auth issue, but it is a repo automation permission boundary.

Validation refresh

Fresh state checked from /data/git/windowstead:

git fetch origin && git checkout main && git pull origin main
HEAD: 20e7551 Merge pull request #120 from misospace/feat/pr-review-update
Working tree: pre-existing untracked fix_menu.patch only

Checks run:

python3 /home/node/.openclaw/workspace-saffron/scripts/research_before_task.py misospace/windowstead 121
# completed; issue context loaded

git diff --check
# exit 0

./.tools/Godot_v4.2.2-stable_linux.x86_64 --version
# 4.2.2.stable.official.15073afe3

./.tools/Godot_v4.2.2-stable_linux.x86_64 --headless --path . --script res://tests/test_runner.gd
# 54 passed, 0 failed

./.tools/Godot_v4.2.2-stable_linux.x86_64 --headless --path . --script res://tests/test_layout_math.gd
# 30 passed, 0 failed

./.tools/Godot_v4.2.2-stable_linux.x86_64 --headless --path . --script res://tests/test_e2e.gd
# 63/63 passed, 0 failed

timeout 15 ./.tools/Godot_v4.2.2-stable_linux.x86_64 --headless --path . --fixed-fps 60 --quit-after 300
# exit 0; no SCRIPT ERROR / ERROR lines
# emitted existing environment warning: libfontconfig.so.1 unavailable

Existing audit findings confirmed

I re-checked the existing issue body against current code and GitHub state. The task reservation, canonical validation matrix, save migration/schema hardening, main.gd blast radius, Godot toolchain drift, and stale branch hygiene findings are still valid. No P0 surfaced in this pass.

Not worth doing yet

• Do not disable the AI reviewer outright; narrow permissions/tool scope first.
• Do not chase the local libfontconfig.so.1 warning as a product bug unless it starts failing CI or packaged builds.
• Do not bundle the AI-reviewer hardening with gameplay refactors; keep automation security work isolated.