refactor(dev-platform): one-way layering + namespaced config (epic #470 C3) - #557
Open
Weegy wants to merge 2 commits into
Open
refactor(dev-platform): one-way layering + namespaced config (epic #470 C3)#557Weegy wants to merge 2 commits into
Weegy wants to merge 2 commits into
Conversation
FIRST TIME the baseline rises for OUR OWN change rather than for something main added. That deserves the explanation to be in the diff, which is why the script refuses `--update` here and forces a hand-edit. Why it rose (+59): the refactor collapses 41 flat DEV_*/FLY_* config keys into one `config.devPlatform` object. The zod schema is unchanged — proven byte-identical, 562 lines, empty diff — so every key name still appears there, and the new mapping layer that builds the namespace names each one a SECOND time. Net: config.ts +48, the new devplatform/config.ts +36, against index.ts −33 and wireDevPlatform.ts −8. The eight moved routers are a wash: same zone, different path. Why it is worth taking: index.ts drops from ~20 threaded config values to one argument, and the layering arrow now points one way (zero src/devplatform → src/routes edges, zero src/routes → devplatform). Both make the P4 file-move mechanical instead of a rewrite. And every one of those +59 references is deleted at extraction — the schema keys, the mapping, and the type file all go with the tree. The honest tension: the ratchet counts identifiers, and this refactor concentrates identifiers rather than removing them. Letting that block a correct refactor would be the measurement driving the work. Documented in README so the next raise is judged against the same two criteria and nothing wider.
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.
Behaviour-preserving refactor that makes the later file-move mechanical. Nothing observable changes — same env var names, same defaults, same routes, same boot log lines.
Part 1 — the layering arrow now points one way
Eight dev-platform routers moved from
src/routes/intosrc/devplatform/routes/(viagit mv, history follows). Before:wireDevPlatform → ../routes/(4 imports) androutes/dev*.ts → ../devplatform/(27, not the 24 I estimated — two more live intriggers/). After: zerosrc/devplatform → src/routesedges, zerosrc/routes → devplatformedges.The three remaining outward edges are legitimate core deps that become extension points later:
services/githubAppJwt.js,issues/deviceFlowStore.js,issues/githubOAuthProvider.js.Rejected alternatives: injecting router factories from
index.ts(moves the coupling into the composition root and makes extraction harder — the routers would still live in core), movingwireDevPlatformup intoroutes/(inverts the wrong way), and a barrel re-export (hides the arrow without turning it).Part 2 — 40 config keys behind one namespace
index.tswent from ~20 threaded config values to a singleconfig: config.devPlatform. The newDevPlatformConfigtype lives insrc/devplatform/config.ts, owned by the tree that will move; core imports it type-only.The zod schema is unchanged — extracted from both revisions and diffed: 562 lines each, empty diff. A second runtime proof compared
JSON.stringify(config)under identical env before and after, remappingdevPlatform.*back to env names: 96 keys each, identical values.Three
DEV_/FLY_-prefixed keys are not dev-platform and stay in core:DEV_ENDPOINTS_ENABLED(core dev-graph endpoints),FLY_APP_NAME(identity of this app — its value is copied into the namespace, the key is not moved), andPLUGIN_DEV_DIR(shares only theDEVtoken).Two pre-existing findings surfaced and were carried rather than silently dropped:
DEV_JOB_MAX_EVENTSandDEV_ARTIFACT_MAX_BYTESare declared, defaulted and documented but never read by anything.The boot refusals
devPlatformBootRefusals()is unchanged and still called fromloadConfig. Its existing unit tests prove the function — but not that it is still wired in, which is exactly what a config refactor breaks silently. NewdevPlatformBootRefusal.test.tsboots the realconfig.tsin a child process with a clean env and asserts the process dies for each of the two credential-exposing modes. Mutation-checked: reverting thethrowturns both cases red.Route paths — proven identical
Every
app.use(...)/router.<verb>(...)literal extracted fromsrc/at both revisions, sorted and diffed: 295 before, 295 after, identical — the whole mount set, not just dev-platform. That matters because deployed runner images phone home to literal URLs; a rename would brick in-flight jobs with no compile-time signal.The ratchet went UP, deliberately — 3,306 → 3,365
First time it rises for our own change rather than for something main added. The script refuses
--updatehere and forces a hand-edit, so the reasoning lands in the diff.The refactor concentrates coupling rather than removing it. The zod schema keeps every key name (it is byte-identical), and the new mapping layer that builds the namespace names each key a second time:
config.ts+48, the new type file +36, againstindex.ts−33 andwireDevPlatform.ts−8. The eight moved routers are a wash — same zone, different path.All 59 are deleted at extraction: the schema keys, the mapping, and the type file all travel with the tree. Blocking a correct refactor because a proxy metric rose would be the measurement driving the work.
README.mdnow names the only two acceptable reasons for a raise so the next one is judged the same way.Test plan
5,322 pass / 0 fail;npx tsc --noEmitclean; build cleanrgNeed help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.