Fix #330: ambient break event leaked gather reservations - #337
Merged
Conversation
The break branch of `maybe_fire_event()` cleared `worker.task` without releasing the resource reservation that `choose_task()` set up when the gather task was assigned. With the last unit of a resource reserved by an orphan, `gather_gather_tasks()` would skip the tile forever (`reserved >= available`) and the tile could never be drained — every ambient-break event that landed on a worker en route to a gather task leaked one more unit, soft-locking the resource permanently. `do_gather`, `do_haul`, and `do_build` all release or own their reservations; the event branch was the only task-clear path that leaked. Fix: in the `event_roll == 1` arm of `maybe_fire_event()`, mirror the release done in `do_gather()`'s early-return path before nulling the task. Only fires when the cleared task was actually a gather so other task types are unaffected. Regression test in `tests/test_reservations.gd`: - plants a 1-unit tree, force-assigns a gather task via `choose_task()` - seeds `rng` so `maybe_fire_event()` deterministically enters the break branch - verifies the reservation is released, the task is cleared, and the break window is granted - runs 40+ ticks and verifies `reserved_resources.wood` stays at 0 and the wood reaches the stockpile — proving the tile is no longer soft-locked and the invariant `reserved_resources == sum of active gather reservations` holds end-to-end Fixes #330 Signed-off-by: Saffron <263493777+itsmiso-ai@users.noreply.github.com>
![its-saffron[bot]](https://avatars.githubusercontent.com/in/3901073?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: dsv4f@https://litellm.jory.dev/v1 (openai) — primary route
Recommendation: Approve. The two-line production fix directly addresses the orphaned gather reservation described in PR 330, and the new regression test matches the issue's acceptance criteria. CI is green.
Change-by-change findings:
scripts/colony_sim.gd— In the ambient break branch, the patch now callsrelease_resource(worker.task.resource)before clearingworker.taskwhen the task is agather/gather_foodtask carrying aresource. This prevents the reservation from remaining stranded, which previously causedgather_gather_tasks()to permanently skip the last unit of a resource.tests/test_reservations.gd— Addstest_break_event_releases_gather_reservation(), covering the issue's regression scenario: one worker, a 1-unit wood tile, deterministic break-event firing, 40 ticks of normal processing,reserved_resourcesreturning to 0, and wood reaching the stockpile. Allassert_eqcalls include the required named argument per AGENTS.md.
Sources:
- Issue PR 330 (problem statement, evidence, acceptance criteria)
- PR diff (head 9a7b3e6)
- CI status API (all checks success, including Script test suite and macOS validation)
- AGENTS.md / CLAUDE.md repository conventions
Standards Compliance:
- AGENTS.md warns that
assert_eqrequires thenameargument; every new assertion uses the 3-argument form. - The PR touches exactly the files named in the issue's expected-files list (
scripts/colony_sim.gd,tests/test_reservations.gd). - No version/manifest changes are needed for this bugfix.
Linked Issue Fit:
- The issue's one-sentence ask — release the gather reservation when the ambient break event clears a worker's task — is implemented verbatim.
- Acceptance criterion 1 (regression test with 1-unit tile, event-style task clear, ≥40 ticks, reservation returns to 0, tile remains gatherable) is satisfied by the new test.
- Acceptance criterion 2 (invariant:
reserved_resourcesnever exceeds active gather commitments) is satisfied by releasing before the task is cleared. - Acceptance criterion 3 (existing suites stay green) is confirmed by CI.
CI:
All reported checks succeeded, including the full Script test suite, export validation, and macOS validation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The PR adds reservation tracking to ColonySim and updates tests to verify it. What: scripts/colony_sim.gd now creates reserved_resources dict, reserve_resource increments without auto-marking dirty, release_resource clamps to zero and marks dirty, get_reserved returns the coun…
Fixes #330
Opened by foreman on review GO (workload wl-misospace-windowstead-330).