refactor(storage): revert per-queue Factory; storage is global#189
Merged
Conversation
## Summary ### Why? PR #188 added `storage.Factory` (`For(queue) (Storage, error)`) and threaded it into every controller, leaving a `TODO(queue-aware)` in all 11 orchestrator controllers: each fell back to `stores.For("")` because storage is needed *before* the by-ID entity load, so the queue isn't known yet. That chicken-and-egg was the sole reason for the TODO. Behavioral extensions (mergechecker, buildrunner, scorer, conflict, changeprovider, pusher) are already correctly per-queue — resolved *after* the load from the loaded entity's queue. Storage, by contrast, does not vary by queue: every queue shares the same entity schema and tables. Per-queue storage, if ever needed, is better achieved by splitting the environment and wiring different stores per deployment, not an in-process Factory. ### What? - Remove the `storage.Factory` interface and delete the `staticFactory` / `NewStaticFactory` implementation (+ its test). Regenerate the storage mock (drops `MockFactory`). - Switch the 11 orchestrator controllers and the gateway `land` controller from `storage.Factory` to a single injected `storage.Storage`; drop the `stores.For("")` hack and the `TODO(queue-aware)` comments. Queue for logging and behavioral-extension resolution continues to come from the loaded entity. - Update example wiring (gateway + orchestrator `main.go`) to pass the store directly instead of `NewStaticFactory(store)`. - Behavioral-extension Factories are unchanged. No behavior change under today's single-store deployment — `For("")` already returned this exact store. ## Test Plan ✅ `make build`, `make test` (39 pass) ✅ `make gazelle` / `make mocks` / `make tidy` / `make fmt` (all in sync)
JamyDev
approved these changes
Jun 4, 2026
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
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.
Summary
Why?
PR #188 added
storage.Factory(For(queue) (Storage, error)) and threaded it into every controller, leaving aTODO(queue-aware)in all 11 orchestrator controllers: each fell back tostores.For("")because storage is needed before the by-ID entity load, so the queue isn't known yet. That chicken-and-egg was the sole reason for the TODO.Behavioral extensions (mergechecker, buildrunner, scorer, conflict, changeprovider, pusher) are already correctly per-queue — resolved after the load from the loaded entity's queue. Storage, by contrast, does not vary by queue: every queue shares the same entity schema and tables. Per-queue storage, if ever needed, is better achieved by splitting the environment and wiring different stores per deployment, not an in-process Factory.
What?
storage.Factoryinterface and delete thestaticFactory/NewStaticFactoryimplementation (+ its test). Regenerate the storage mock (dropsMockFactory).landcontroller fromstorage.Factoryto a single injectedstorage.Storage; drop thestores.For("")hack and theTODO(queue-aware)comments. Queue for logging and behavioral-extension resolution continues to come from the loaded entity.main.go) to pass the store directly instead ofNewStaticFactory(store).No behavior change under today's single-store deployment —
For("")already returned this exact store.Test Plan
✅
make build,make test(39 pass)✅
make gazelle/make mocks/make tidy/make fmt(all in sync)