feat: persist active rotating goal state

[itsmiso-ai]

Fixes #144

Persist active_goal and completed_goal_ids in game state so they survive save/load cycles. On load, validate the saved goal ID against the catalog and fall back to selecting a fresh goal if the saved state is missing or incompatible.

• persist(): save active_goal (deep copy) and completed_goal_ids before GameState.save_game()
• load_or_boot(): restore with safe fallback for empty/unknown goal IDs
• tests: cover persistence, deep copies, and load-time validation

[its-saffron]

AI Automated Review

Analysis engine: MiniMax@https://litellm.jory.dev/v1 (anthropic)

Recommendation: Approve

This PR cleanly implements the persistence of the active rotating goal state as specified in issue #144. The implementation is minimal, explicit, and well-tested, matching the issue's guidance to "keep save changes minimal and explicit."

Change-by-Change Findings

scripts/main.gd — persist() (line 1941-1944)

• Correctly deep-copies active_goal (only when non-empty) and always persists completed_goal_ids before GameState.save_game(). The guard if not active_goal.is_empty(): prevents overwriting a previously saved goal with an empty dict. Deep copy is appropriate to avoid reference-sharing with the live state.

scripts/main.gd — load_or_boot() (line 675-685)

• Validates the saved goal as a non-empty Dictionary, checks its id against RotatingGoal.GOAL_CATALOG, and confirms completed_goal_ids is an Array. Fallback path calls select_next_active_goal() with a fresh empty completed_goal_ids list, which is a safe "start over" behavior for incompatible/missing state.
• The validation against the catalog is the right call for forward-compat: if the catalog changes between versions, an unknown ID is discarded rather than causing a crash.

tests/test_goal_persistence.gd (new, 170 lines)

• Five targeted tests cover: active_goal persistence + deep copy, completed_goal_ids persistence + shallow-copy safety, empty active_goal not being written, and two fallback paths (missing key, unknown ID).
• Tests simulate the persist/load logic in isolation rather than exercising the actual persist() / load_or_boot() methods on a real main.gd instance. This is a minor coverage gap — a true round-trip test (boot → set goal → persist → reload → assert) would be stronger, but the simulation tests are still meaningful because they encode the exact decision tree.
• Minor: state_script is loaded on line 10 but never used. Safe to remove.

Sources

• Issue #144 acceptance criteria
• scripts/main.gd lines 78-79 (existing active_goal / completed_goal_ids declarations), 732-734 (existing boot-time initialization), 1080-1083 (existing rotation logic), 1941 (existing persist() context)
• scripts/rotating_goal.gd lines 46, 143-165 (existing select_next_active_goal / rotate_after_completion)
• scripts/game_state.gd save format conventions
• Repository git history: prior PR #145 introduced the rotating goal completion flow; this PR is a natural follow-on.

Standards Compliance

• Migration-first principle (AGENTS.md): The PR does not bump SAVE_VERSION and does not add explicit migration code. This is acceptable here because the new keys (active_goal, completed_goal_ids) are purely additive and optional — old saves without them simply hit the fallback path, which behaves identically to a fresh boot. This is consistent with the issue's instruction to not broaden migration work.
• Code style: Uses tabs, explicit type hints, and matches the existing if/else block style in main.gd.
• Test conventions: Follows the same SceneTree-based harness pattern as tests/test_rotating_goal.gd and tests/test_runner.gd, with custom assert helpers.

Linked Issue Fit

Acceptance Criterion	Status
Active goal survives save/load	✅ Persisted in persist(), restored in load_or_boot() with deep copy
Incompatible/missing goal state falls back safely	✅ Catalog ID validation + select_next_active_goal fallback for both empty and unknown IDs
Save version handling remains explicit	✅ No change to SAVE_VERSION; additive keys preserve backward compat
Tests cover save/load of active goal state	✅ 5 tests in test_goal_persistence.gd
Link back to #131	⚠️ PR body says "Fixes #144" but does not reference #131. If the issue intended an explicit cross-reference, it's missing from the description. The implementation does not appear to depend on #131's content.

Evidence Provider Findings

No evidence providers configured — no automated signals to weigh.

Tool Harness Findings

Planning warning noted but no planned requests, so no harness output to validate against the diff.

Unknowns / Needs Verification

• #131 dependency: Unclear whether the issue's "Link back to #131" requirement means the PR description should mention #131 or whether the implementation should reference code from #131. The PR diff does not appear to touch any #131 surface area, so this is likely a description-only nit.
• End-to-end round-trip test: The new tests simulate the logic but do not call the real persist() / load_or_boot() on a Main instance. A future hardening pass could add a true save/load round-trip test, but it's not a blocker for this issue.
• Catalog mutation safety: If GOAL_CATALOG entries are ever mutated at runtime (currently they are not), the catalog_ids list built at load time would be stale. Not a concern given current usage but worth a comment in a follow-up.

Overall: solid, minimal, correct implementation that satisfies the issue. The unused state_script variable and the simulation-based test approach are minor polish items, not blockers.