refactor: codebase cleanup + offline-safe geoip feature split#26
Merged
Conversation
The 10 attribute macros each copy-pasted the same darling parse scaffold, duration parsing, and rate-limit extraction. Extract three shared helpers (parse_attrs, parse_optional_duration, extract_rate_limit) and route query/mutation/job/mcp_tool/daemon/workflow through them. Removes ~120 lines of duplication and a redundant rate-limit-key re-check in mcp_tool, with no behavior change.
The HandlerContext/AuthenticatedContext/Sealed impls were 120+ lines of hand-written pass-through boilerplate across 8 context types. Replace with three small macro_rules! generators, keeping MutationContext's bespoke db() explicit. Same impls, ~90 fewer lines.
All five stores (connection/query/subscription/job/workflow) plus the optimistic-mutation helper hand-rolled the same Set<subscriber>+notify()+ subscribe() plumbing. Introduce createStoreCore<T> (get/set/update/subscribe with an onLastUnsubscribe hook) and route every store through it. Teardown and callback-ordering semantics preserved exactly; tsc --strict clean.
…macros The eight Test*ContextBuilder types copied byte-for-byte identical auth (as_user/as_subject/with_role/with_roles/with_claim), env (with_env/with_envs/ with_pool), and tenant (with_tenant) setters. Extract three macro_rules! generators in context/mod.rs and have each builder opt into the clusters it supports. No public API change; 583 forge-core tests pass.
…tion init run() mutated self.config.gateway.port in place from the PORT env var, leaving the owned config inconsistent with how it was built. Resolve it into a local effective_port instead. Also lift the self-contained telemetry and migration prologue out of the 900-line run() into init_telemetry_subsystem() and apply_migrations(). The intricate cluster/worker/gateway wiring is left intact since it can only be safely validated by full integration tests.
The geoip feature bundled both the runtime MaxMind MMDB reader and the build-time-downloaded DB-IP country database, so it couldn't build offline and was excluded from the default 'full' preset entirely. Split them: - geoip (now in 'full'): pure-Rust maxminddb reader, offline-safe. Set signals.geoip_db_path to a GeoLite2-City MMDB for country/city enrichment. - geoip-embedded (opt-in): bakes in the DB-IP country DB; the only geoip option needing a build-time network fetch. GeoIpResolver gains a no-data Empty backend for the geoip-without-embedded case. Breaking: zero-config country enrichment now requires geoip-embedded. Updates binary-size + signals docs, skill api/frontend references, CHANGELOG.
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
1 participant
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.
What changed
A codebase cleanup pass plus one user-facing change to how GeoIP is built.
GeoIP feature split (breaking)
geoipis now offline-safe: it pulls only the pure-RustmaxminddbMMDB reader and is included in the defaultfullpreset, socargo buildworks in air-gapped/CI environments. The bundled DB-IP Country Lite database (and its build-time network download) moved to a new opt-ingeoip-embeddedfeature.Breaking: zero-config
countryenrichment now requires--features geoip-embedded. The default build enrichescountry/cityonly whensignals.geoip_db_pathpoints at a GeoLite2-City MMDB. TheGeoIpResolverbackend is now gated across three feature combinations (neither /geoip/geoip-embedded); match arms are exhaustive for each.Internal refactors (no public API change)
attrs.rs, used by the query/mutation/job/cron/workflow/daemon/mcp_tool macros.StoreCorepub-sub core; the connection/query/subscription/workflow/optimistic stores now build on it, with the last-unsubscribe cleanup hook preserved.Forge::run()no longer mutates its config in place; telemetry/migration init extracted.Verified
cargo fmt --all --checkcargo clippy --all-targets --all-features --workspace -- -D warningscargo test --workspace(SQLX_OFFLINE=true)Docs updated alongside code:
docs/docs/scale/binary-size.mdx,docs/docs/ship/signals.mdx, and the skill references (api.md,frontend.md). CHANGELOG updated under[Unreleased].Still open