[dmccoystephenson]

Summary

• Environment::addEntityToLocation inferred placement by comparing the entity's grid id with the grid's after the call. An entity already carrying that grid id passed the check without being placed, so its environment id was overwritten, contrary to the README "Errors" section. Grid::addEntityToLocation now returns whether it placed the entity, and the environment uses that result.
• The void to bool change is source compatible for existing callers. The Strata frontend does not call the grid method directly, and its CI build with warnings as errors passed.
• Test 78 pins with static_assert that Environment cannot be copied or assigned, which guards the double delete behind Environment is copyable but owns a raw Grid*, so copying it double-deletes the grid #44.
• The README "Errors" section now records the new return value. The class table is unaffected.

Test plan

• CI "Library tests" failed on the tests-only commit 1f57797, aborting on the Test 77 assertion entity.getEnvironmentId() == -1 after Test 76 passed (exit 134)
• CI "Library tests" passed on the final commit 83356c6, with 79 of 79 registered tests reporting success, and the CMake ctest step passed
• CI "Strata frontend" and "Browser build" passed on 83356c6
• Test 78 was confirmed red locally: with both deleted declarations temporarily removed from environment.h, make failed on both static_asserts, and the header was then restored
• Test 79 was not observed red, since it does not compile against a void return. It exercises the new return value only.
• run_tests_asan.sh was not run: CI has no AddressSanitizer job (tracked in CI does not run run_tests_asan.sh, so AddressSanitizer failures are not caught on PRs #75), and local execution is denied. The change alters no allocation or entity lifetime.

Local execution of tests_executable is denied in this session, so CI is the test anchor.

Skipped

#75 was left for a later cycle. Its fix edits .github/workflows/ci.yml, which is on the do-not-auto-merge list, so it needs human review and is independent of this change.

Closes #77
Closes #78

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

🤖 Generated with Claude Code

---

drafted by Claude on behalf of Daniel Stephenson

[dmccoystephenson]

Self-review rubric, anchored on CI run 42370300793 at 83356c6 (79 of 79 tests reported success):

• Scope: PASS — the five changed files all serve Environment::addEntityToLocation claims an entity it did not place when its grid id already matches #77 or No test pins that Environment cannot be copied or assigned #78; src/tests.cpp keeps its CRLF endings and shows no churn outside the additions.
• Tests-new: PASS — the new bool return of Grid::addEntityToLocation is exercised for both outcomes by Test 79.
• Tests-fix: PASS — a red CI run stood in for stash-and-run: 1f57797 aborted on the Test 77 assertion entity.getEnvironmentId() == -1, and 83356c6 passed. Test 78 was red locally under make with the deleted declarations removed.
• Sibling structure: PASS — no new files.
• Sibling renames: PASS — nothing renamed.
• Docs: PASS — the README "Errors" paragraph gained the return value; the class table is unaffected.
• Issue resolution: PASS — the surface named in Environment::addEntityToLocation claims an entity it did not place when its grid id already matches #77 and the gap named in No test pins that Environment cannot be copied or assigned #78 are both changed; closingIssuesReferences is exactly 77 and 78.
• Local test suite: PASS — the CI "Library tests" job is the anchor, since local execution is denied.
• Header/definition parity: PASS — the declaration and definition of Grid::addEntityToLocation both changed to bool.
• Include guards: PASS — no new headers.
• Test registration: PASS — Tests 77 to 79 are called from main() and appear in the CI log.

Findings:

• src/tests.cpp:1308 — Test 78 can never fail at run time. A regression shows up as a compile failure of the suite, which is intended and which CI reports, but its "Success" line carries no run-time evidence.
• src/grid.cpp:100 — Environment::addEntityToLocation now differs from Grid::addEntityToLocation in return type. This was left as is, to keep this PR from changing the environment's public signature; the README states the difference.
• Not run: run_tests_asan.sh (tracked in CI does not run run_tests_asan.sh, so AddressSanitizer failures are not caught on PRs #75). The change alters no allocation or entity lifetime.

This review was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson