fix: make the multi-statement write paths atomic

[dmccoystephenson]

Summary

• A DataSourceTransactionManager is declared next to the pool it wraps in DataSourceConfig, so the write paths have a transaction to join. It is declared rather than left to Spring Boot's auto-configuration, which backs off as soon as a second DataSource candidate or another manager appears.
• Three multi-statement write paths are annotated @Transactional, so the runtime exception each already throws on the first failure now discards the whole sequence instead of leaving rows behind that no request could produce deliberately:
  • EnvironmentController.deleteEnvironment — the association/entity/location/grid cascade, previously abandoned partway through on the first failure.
  • EntityController.deleteEntity — EntityRepositoryImpl.deleteById clears the placement before deleting the entity, so a failed entity delete previously left the entity without its placement.
  • EnvironmentFactory.createEnvironment — the environment, its grids, its locations and every association between them, previously able to leave an environment missing grids or a grid missing locations.
• DbInteractions is unchanged: it already borrows through DataSourceUtils, so its calls join the surrounding transaction. That was the reason connection pooling landed first in refactor: pool database connections, and fix the blocked local-run configuration #198.
• The two controller tests stop mocking DbConfig, and the test datasource points at H2. The boundary opens a real connection before the mocked repository call, and a mocked DbConfig handed the pool a null JDBC URL — those requests failed with a 500 rather than exercising the controller.

What is deliberately not included

• Step 3 of Multi-statement writes are not atomic; a shared JDBC connection blocks adding transactions #194 — reporting the duplicate-placement loser in LocationController.addEntityToLocation as a 409 rather than a 500 — is left out. A boundary alone does not close that race under READ COMMITTED, and distinguishing a unique-key violation from any other failure requires DbInteractions.update to surface the SQLException it currently swallows, which changes error semantics for every repository. A follow-up issue is filed for it, and Multi-statement writes are not atomic; a shared JDBC connection blocks adding transactions #194 is therefore referenced rather than closed here.
• Wire OpenAPI-first codegen / spec-drift guard (rebuild-plan hygiene gap) #130 (OpenAPI codegen / spec-drift guard) was deferred this cycle: it is a build-tooling change touching pom.xml and .github/workflows, which is not coherent with this batch.

Notes on the sequences

Postgres sequences are not transactional, so a rolled-back creation still consumes the ids it drew. The resulting gap is expected and harmless; it is documented on the factory method.

Test plan

• mvn -B test — 413 tests, all passing.
• TransactionRollbackTest exercises the real EntityRepositoryImpl.deleteById against H2 through the production DataSourceConfig wiring, with a foreign key pointed at the entity so the second statement fails. The placement is restored inside a boundary; a control case in the same class shows it lost without one; a third case confirms the successful path still commits both statements.
• TransactionBoundaryWiringTest asserts each annotated method is genuinely advised — @Transactional is silent when it is not — and that exactly one transaction manager is bound to the pooled DataSource.
• Verified empirically: with the three @Transactional annotations reverted, TransactionBoundaryWiringTest fails 3 of its 4 cases; with them restored it passes.
• pytest — 107 tests, all passing. The Python client is untouched by this change, and CI does not cover it. (Collection requires the repository root on sys.path; --import-mode=importlib was used, a pre-existing environment quirk.)
• No API surface changed, so docs/openapi/viron-api.json, the Postman collection and the planning docs need no update; none of them documents persistence or transaction behaviour.

Part of #194.

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

---

drafted by Claude on behalf of Daniel Stephenson

[dmccoystephenson]

Self-review rubric

Scored against the diff and command output, not judgement. Two items failed on the first pass and were fixed in fdef53a; both are recorded below as found rather than silently corrected.

• Scope: PASS — eight of the ten files are the boundaries themselves and their tests. The two that are not obviously in scope (EntityControllerTest, EnvironmentControllerTest, and the test properties) are forced by the change: a transaction opens a connection before the mocked repository call, and those contexts mocked DbConfig, so the pool received a null JDBC URL and every delete returned 500. No unrelated formatting or renames.
• Tests-new: PASS — the one new production method, DataSourceConfig.transactionManager, is exercised by TransactionBoundaryWiringTest.contextExposesOneTransactionManagerBoundToThePooledDataSource (bean identity) and by all three TransactionRollbackTest cases (real rollback behaviour).
• Tests-fix: PASS — verified empirically rather than by reasoning. With the three @Transactional annotations stashed, TransactionBoundaryWiringTest failed 3 of 4 (EntityController.deleteEntity must carry a transaction attribute, EnvironmentController.deleteEnvironment must carry a transaction attribute, EnvironmentFactory must be proxied for its transaction boundary to apply); restored, all 4 pass. TransactionRollbackTest additionally carries its own control case: the same failed delete without a boundary leaves the placement gone, which is the defect being fixed, so a false-negative regression test would show up as two identical outcomes rather than a contrast.
• Sibling structure: PASS — TransactionRollbackTest follows DbInteractionsTest's established shape (production DataSourceConfig builds the H2 pool, @BeforeEach recreates the schema, @AfterEach closes the pool, a private h2Config() at the bottom with its own database name). TransactionBoundaryWiringTest follows DataSourceConfigTest's (@SpringBootTest, ApplicationContext assertions).
• Sibling renames: PASS (no signal) — no identifier was renamed.
• Docs: FAIL, then fixed — src/test/resources/application.properties:4 still opened with "Mirrors the runtime properties" after the datasource stopped doing so. Corrected in fdef53a. On the repo-wide table: no API surface changed, and no entry in docs/openapi/viron-api.json, docs/MVP.md, docs/PLANNING.md, docs/REBUILD_PLAN.md, tickets.md, README.md or the Postman collection describes persistence, connections or transactions — confirmed by grepping all of them for transaction, atomic, rollback, connection and DbInteractions, which returns nothing.
• Issue resolution: PASS — no Closes reference is claimed. Multi-statement writes are not atomic; a shared JDBC connection blocks adding transactions #194 is referenced as Part of, because step 3 is deliberately out of scope; see the PR body.
• CI: PASS — build green on head fdef53a (44s). Locally mvn -B test reports 413/413.

Repo-specific

• DTO boundary: PASS (no signal) — no controller return type changed; the diff adds only annotations and javadoc to the controllers.
• Spec alignment: PASS — no path, verb, parameter or body changed, so docs/openapi/viron-api.json is untouched by design. OpenApiSpecDriftTest passes.
• Java/Python parallelism: PASS — transaction management is server-side only and has no mirror in src/main/python/preponderous/viron/. The Python client is untouched; pytest was run anyway and reports 107/107.
• Override correctness: PASS (no signal) — no @Override was added or changed.

Findings folded in from the diff read

• src/test/java/preponderous/viron/controllers/EnvironmentControllerTest.java:52 — the comment replacing the removed mock claimed the boundaries on "deleteEnvironment and environment creation" both need a real DbConfig. EnvironmentFactory is mocked in that class, so the creation boundary is never reached there. Corrected in fdef53a to name only deleteEnvironment.
• src/test/resources/application.properties:4 — as scored above.
• src/test/resources/application.properties:18 — the surviving comment on management.health.db.enabled=false explains itself partly by SecurityConfigTest not depending on a reachable database, which is now less load-bearing since the test datasource is reachable. The line was left alone: it still mirrors the runtime setting, which is its main reason for existing.
• src/main/java/preponderous/viron/repositories/EntityRepositoryImpl.java:97 — DbInteractions.update swallows the SQLException and reports false, so a rolled-back path never surfaces why it failed. That is pre-existing, is what makes step 3 of Multi-statement writes are not atomic; a shared JDBC connection blocks adding transactions #194 (409 rather than 500) a separate change, and is not touched here.
• Four other context tests (GridControllerTest, LocationControllerTest, DebugControllerTest, SecurityConfigTest) still mock DbConfig. They were left alone deliberately: none of them reaches a transactional path, so none needs a working pool, and changing them would have pulled unrelated files into this PR.
• DebugController's seeding endpoints issue many writes, but through EnvironmentService, which calls the HTTP API — each call is already its own request and therefore its own transaction. No boundary is needed or added there.

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

---

drafted by Claude on behalf of Daniel Stephenson