[P3] Add test for stale reservation cleanup dirty flag, remove redundant guard

[itsmiso-ai]

What

Adds _clean_stale_reservations() to persist_state() in scripts/main.gd so that on every save/load cycle, builds whose reserved resources exceed their remaining needs have the excess cleared. Two new tests in tests/test_reservations.gd verify the cleanup behavio…

Fixes #297

Opened by foreman on review GO (workload wl-misospace-windowstead-297).

Superseded by a newer automated review for this pull request.

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: dsv4f@https://litellm.jory.dev/v1 (openai) — primary route

Review Summary

This PR addresses issue PR 297 by:

1. Removing the redundant reserved_resources guard in persist() (since ensure_defaults() already guarantees the key exists).
2. Adding a test that verifies _clean_stale_reservations() calls mark_dirty() so that stale reservation cleanup triggers a save.

The changes are minimal, focused, and align with the linked issue’s acceptance criteria. The test is well-structured and correctly resets the dirty flag before calling the method to avoid a false positive. CI is green across all platforms.

Change-by-Change Findings

scripts/main.gd

• Removal of redundant guard: The two lines that checked for reserved_resources and set it to {} are removed. This is safe because ensure_defaults() in colony_sim.gd already initializes the key. No other logic depends on this guard.

tests/test_reservations.gd

• New test test_stale_reservation_cleanup_marks_dirty: Creates a ColonySim instance with a build that has a stale reservation, resets sim.dirty to false, calls _clean_stale_reservations(), and asserts that sim.dirty is true. This directly validates the missing mark_dirty() call described in the issue.

Standards Compliance

• The test follows the existing pattern in test_reservations.gd (direct ColonySim instantiation, manual state setup).
• The removal of dead code aligns with the repository’s preference for clean, minimal code.
• No new dependencies or manifest changes are introduced.

Linked Issue Fit

• Acceptance criteria:
  • ✅ mark_dirty() call inside the build.has("reserved") branch in _clean_stale_reservations() – the test proves this call exists (the test passes, and the existing code in colony_sim.gd already includes the call).
  • ✅ Verify with a test that stale reservation cleanup triggers a dirty flag – the new test does exactly this.
  • ✅ Consider removing the redundant guard in persist() – the guard is removed.
• The PR fully satisfies the issue requirements.

Tool Harness Findings

Tool harness output was limited to file listing and diff reading; no issues were flagged.

Unknowns or Needs Verification

• The test only verifies the dirty flag is set; it does not verify that the reservation is actually cleaned up (i.e., that reserved is erased). However, the existing test test_stale_reservation_cleanup already covers that behavior, so this is acceptable.
• The PR does not add a test for the removed guard, but the guard was dead code and its removal is low-risk.

Recommendation: Approve. The changes are correct, well-tested, and address the linked issue completely.