Centralize deployment-mode gating behind a capability definition#155
Open
CarsonDavis wants to merge 6 commits into
Open
Centralize deployment-mode gating behind a capability definition#155CarsonDavis wants to merge 6 commits into
CarsonDavis wants to merge 6 commits into
Conversation
Replace scattered isFull()/isLean() checks with a single capability definition (API/Backend/Utils/capabilities.js) that every gate reads. - Backend modules declare `capability`; the discovery seam (API/setups.js) decides what to wire. onceInit/onceStarted are gated; onceSynced runs unconditionally so gated-off tables still exist in both modes. - Partial gates (Missions mount, utils routes, mmgis-stac, WITH_* flags, adjacent-servers spawn/proxy) ask the definition by capability name. - localSidecars collapses the sidecar cluster; localMissions kept separate. - updateTools reads each tool's declared capability (Draw declares "draw"). - Configure twin extended; Panel/SaveBar/DeploymentsWatcher migrated off raw mode-string checks. - Upload storage fork left as a direct mode check; staticHandlers untouched. Unit suite: 674/674 green.
The capability centralization left one production mode check behind — the
S3-vs-disk upload storage fork in uploadRouter.js — plus the now-unused
isFull() export. Leaving them means the codebase still has two ways to ask
"what mode am I in" (raw MODE predicates and the capability accessor), which
is exactly the scattering this refactor set out to remove.
Route the upload fork through the capability definition like everything else
and drop the helpers entirely:
- capabilities.js: add `s3AssetUploads: ["lean"]` (uploads persist to the
shared S3 asset bucket in lean, local disk in full).
- uploadRouter.js: `isLean()` -> `enabled('s3AssetUploads')`.
- deploymentMode.js: remove isFull()/isLean(); it now exports only MODE and
documents that capabilities.js is the single interpreter of the mode.
- tests: deploymentMode.spec asserts on MODE only; capabilities.spec covers
the new lean-only capability; uploadRouterS3.spec busts the capabilities
require-cache too (the router now reads mode through it); e2e comments name
the capabilities instead of isFull()/isLean().
No behavior change in either mode. Unit suite: 674/674.
Three readers still answered "are the bundled sidecars on?" off the raw env
instead of the one capability definition. Bring the runtime + CI answers onto
enabled("localSidecars"); the build-time image question stays a separate axis.
- connection.js: gate the mmgis-stac connection on
enabled("localSidecars") && WITH_STAC, mirroring setup.js/init-db.js. In full
this reduces to today's WITH_STAC check; in lean the catalog is never built
even if a stray WITH_STAC=true leaks in. (required after dotenv so MODE
resolves from .env; no import cycle — capabilities -> deploymentMode only.)
- scripts/mode-env.js: emit the sidecar WITH_* disables implied by the mode
(lean -> all off, full -> nothing), derived from capabilities.js so the
env/bash world reads the same authority. The CI lean leg's five hand-written
echoes + paragraph note collapse to one `node scripts/mode-env.js >> .env`.
Loads dotenv before requiring capabilities (CI writes the mode into .env, not
the process env), same order as init-db.js.
- capabilities.js: drop the unused boolean-rule branch in coerce (no FEATURES
rule is a boolean; re-add with a test when one is).
- capabilitiesTwin.spec.js: assert the backend and Configure twin maps agree on
every shared capability in both modes, guarding silent drift.
- Fix the two now-stale isFull() comments in the workflow.
Deferred (different axis, documented in the PR): the Dockerfile build-time
ARG WITH_STAC, and the configuration/env.js WITH_TITILER frontend injectable
(already carries the correct value in lean via the env; gating it means a
frontend-consumer refactor).
No behavior change in either mode. Unit suite: 676/676. mode-env.js verified
emitting the same env the echo block did, with the mode supplied only via .env.
…newline can't merge the first flag onto its last line
…ration The seam comments justified unconditional model sync as 'so a later mode flip needs no migration.' That reason doesn't hold: deployments never switch modes, and sequelize.sync() is additive and runs every boot, so a missing table would self-heal anyway. The real reason is ADR D2 (keep, env-gated): gate route mounts only, leave models registered — a gated-off feature's tables are created but unused in the mode that gates it, and per-mode-gating model registration would re-scatter the mode logic this refactor removed. Comments only; no behavior change.
…apability-gating # Conflicts: # .github/workflows/playwright-tests.yml
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.
Closes #152.
What this does
Replaces the ~30 scattered
isFull()/isLean()deployment-mode checks with one capability definition that every gate reads, so "what does lean turn off?" is answered in one place and adding a mode is a one-table edit. Pure restructuring — no behavior change in either mode.API/Backend/Utils/capabilities.js): acapability → enabling mode(s)map resolved through one accessor; throws on an unknown capability so a typo fails at boot.API/setups.js): each gatedsetup.jsdeclarescapability: "<name>"and the seam decides.onceInit/onceStartedare gated;onceSyncedruns unconditionally — models register at require-time and sync in every mode (ADR D2: keep, env-gated), so every gated-off feature still creates its (then-unused) DB tables. Gating model registration per-mode would re-spread mode logic into every module for no benefit; deployments never switch modes, so this isn't about a migration-free flip.mmgis-staccreation, theWITH_*Configure flags, the adjacent-servers spawn/proxy, the upload S3-vs-disk storage fork) ask the definition by capability name.localSidecarscollapses the sidecar cluster;localMissionsis its own capability;s3AssetUploads(lean-only) drives the upload storage fork.updateToolsreads each tool's declaredcapability(Draw declares"draw") instead of a hardcoded name.configure/src/core/capabilities.js) is extended andPanel/SaveBar/DeploymentsWatchermigrated off raw mode-string checks. The twin uses the same list shape as the backend (warns + hides on unknown, rather than throwing — a render-time gate fails safe).After this PR,
isFull()/isLean()/isLeanMode()are gone from the codebase entirely.deploymentMode.jsis the one env read and now exports onlyMODE;capabilities.jsis the single place that interprets it. The only thing left untouched isstaticHandlers.js(a call's static disposition isn't a pure function of its capability).Fixed during review — last mode checks removed
The first cut of this PR deliberately left two raw mode reads behind: the upload S3-vs-disk storage fork (
uploadRouter.js, a directisLean()) and a now-unusedisFull()export. That leaves two ways to ask "what mode am I in" — exactly the scattering this refactor exists to remove — so it's finished here rather than deferred:uploadRouter.js:isLean()→enabled('s3AssetUploads'), withs3AssetUploads: ["lean"]added to the definition (uploads persist to the shared S3 asset bucket in lean, local disk in full — a lean-only thing, no other backends planned).deploymentMode.js:isFull()/isLean()deleted; it exports onlyMODEand documents thatcapabilities.jsis the sole interpreter. (isLeanModeon the Configure twin was already removed earlier in this PR.)deploymentMode.specasserts onMODEonly;capabilities.speccovers the new lean-only capability;uploadRouterS3.specbusts thecapabilitiesrequire-cache too (the router now reads mode through it).Net: zero
isFull/isLean/isLeanModereferences remain in code (only the historical ADR planning docs underdocs/adr/...still mention them, as a record of the original plan).Also: one authority for the sidecar on/off question (runtime + CI)
A further review pass found three readers still answering "are the bundled sidecars on?" off the raw
WITH_*env instead ofenabled("localSidecars"). Folded in so the capability is the single authority for the runtime and env/CI answer (the build-time image question stays a separate axis):connection.js: themmgis-stacconnection is gated onenabled("localSidecars") && WITH_STAC, mirroringsetup.js/init-db.js. Full reduces to today'sWITH_STACcheck; lean never builds the catalog even if a strayWITH_STAC=trueleaks in.scripts/mode-env.js(new): emits the sidecarWITH_*disables implied by the mode (lean → all off, full → nothing), derived fromcapabilities.js. The CI lean leg's five hand-writtenechoes + paragraph note collapse to onenode scripts/mode-env.js >> .env. This is a CI mechanism change (behavior-preserving for both legs), not pure restructuring — the env/bash world now reads the same definition the app does.capabilities.js: removed the unused boolean-rule branch incoerce.tests/unit/capabilitiesTwin.spec.js(new): asserts the backend and Configure capability maps agree on every shared capability in both modes (guards silent drift).isFull()comments in the workflow.Deferred — different axis, not folded in: the Dockerfile build-time
ARG WITH_STAC(whether STAC is baked into the image, vs. whether the sidecar is on at runtime — a later rename, not this PR), and theconfiguration/env.jsWITH_TITILERfrontend injectable (already carries the correct value in lean via the env, so it's a purity gap, not a live bug; gating it on the capability is a frontend-consumer refactor of its own).Verification
scripts/mode-env.jsverified emitting the same env the old echo block did (lean → the fiveWITH_*=false; full → nothing), with the mode supplied only via.envthe way CI invokes it.isLeanModeexport removed and the Configure twin DRY'd as part of review.Merge order
Stacked on #154 (base
tests-both-modes-ci). Merge after the test chain: #150 → #153 → #154 → this. The both-modes coverage from #154 is what gates this refactor in CI.