Route applications by host/urlPath declared in the root config - #1964
Route applications by host/urlPath declared in the root config#1964kriszyp wants to merge 10 commits into
Conversation
Where an application is served is a deployment concern, not an application concern, but `host`/`urlPath` were only readable from the config file that declared a plugin. For an application that is its own `config.yaml`, so the hostname and mount point had to be checked into the app — unoverridable from outside it (the env-config overlay is root-config-only). Worse, `host`/`urlPath` on a root-config *application* entry were silently inert. An application's plugin scopes read the application's own config.yaml and nothing carried the root entry's routing down to them, so `deploy_component urlPath=/api` (#1113) persisted a value that changed nothing. The flow described in #1113 only ever held for a root-declared *plugin*, whose scope does read the root config. The root config is now authoritative for where an application is served: my-app: host: api.example.com urlPath: /v1 - `scopeMount.ts` — pure mount model. `host` is replaced outright (an operator remapping a hostname must win over a value the app shipped). `urlPath` is composed rather than replaced, because a plugin's `urlPath` doubles as its app-internal base path (static's asset root, fastify's route prefix); replacing it would silently relocate app-internal URLs and collapse distinct plugins onto one path. Mount `/v1` + `static: { urlPath: assets }` → `/v1/assets/`. The composed value is a fixed point of `resolveBaseURLPath`, so downstream consumers keep resolving it without compounding the prefix. - Overlaid in `OptionsWatcher`, not at each call site, so `scope.options.getAll()` is the one effective view of a plugin's config. static's redirects and external paths, the EntryHandler's entry URLs, and fastify's route prefix are all correct with no changes of their own. Composed from the freshly-parsed file on every read, so live reload cannot compound the prefix. - Applied on both load paths: the root-config `package` recursion and the components-root directory scan. The scan is the path that matters most — it loads apps with no root entry at all, so a mount works for a payload-deployed app, not just an installed one. - `deploy_component` accepts and persists `host` alongside `urlPath`, rejecting a host that carries a port or path (it would never match the router's host compare). Also fixes the Scope `server` proxy passing a raw config `urlPath` straight to the router: a plugin that spreads its whole config section into these options (REST does) handed the router the literal './', which normalized to the unmatchable route '/.'. The proxy now resolves whichever source supplied the value. Documented in HarperFast/documentation#595.
Addresses cross-model review findings (Codex) plus a design flaw they exposed. The mount was overlaid inside `OptionsWatcher`, so `scope.options.getAll()` returned the composed `urlPath` to *every* consumer — including the entry pipeline. But entry URL paths are what `graphqlSchema` and `jsResource` derive resource paths from, and the router strips the mount before REST resolves them. A mounted app therefore registered its table at `/v1/MountedThing` while REST looked up `MountedThing`: every mounted REST route 404'd. The single-app static test passed because static de-prefixes its own map keys, which hid it. The invariant is now explicit: the mount is applied at exactly one place, the routing boundary in Scope's `server` proxy. Everything inside the application stays mount-relative. Only code that emits an absolute URL to the client, or bypasses the routed chain, knows about it: - `Scope.mount` + `Scope.externalBasePath()` expose it deliberately and narrowly. - `static` keeps a mount-relative `baseURLPath` (so map keys still agree with entry paths) and uses `externalBasePath` for redirect `Location` headers. - `fastifyRoutes` registers on the bare server, so nothing strips the mount — its route prefix must be the full external path. - `OptionsWatcher` is untouched again; `applyScopeMount` is gone with it (dead code). Review findings fixed: - **REST registered once per process** (`started` boolean), so with two applications mounted at different paths only the first got a chain and the second's REST API silently 404'd — the headline use case. Registration is now keyed per route mount. - **`false` was promoted to a truthy object**, enabling a plugin the app explicitly disabled and making a live `true`→`false` edit read as no change (so no restart was requested). - **Host matching was case-sensitive** though hostnames are not (RFC 4343), making a configured `API.example.com` unmatchable by any real client. Also IPv6-safe now: `[::1]:9926` parsed to `[`. Mounts are normalized and the compare is case-insensitive. - **A nested component's mount was discarded** whenever a parent mount existed. Mounts now nest (`nestScopeMount`); the parent keeps hostname authority so a child cannot escape its host. - **Dot-segment mounts** (`.`, `./v1`) produced unreachable routes (`/.`) since clients strip those segments. Rejected at both the mount normalizer and the operation validator. - **Bracketed IPv6 hosts** were accepted by validation but never matchable; rejected in favor of the bare literal. Known limitation, deliberately not fixed here: legacy fastify routes register as a global fallback on the bare server and cannot be constrained by host, so a `host` mount does not isolate them. A `urlPath` mount does work (it becomes the route prefix). Loading a host-mounted application with `fastifyRoutes` now warns rather than implying isolation it can't provide. A mount is a routing prefix, not a resource namespace — exported tables remain instance-wide and reachable through any mount. Asserted in app-mount-two.test.ts so the boundary is explicit.
|
Reviewed; no blockers found. |
static reads `scope.externalBasePath()` to build client-facing redirect Locations, but unitTests/server/static.test.js stands in a hand-rolled scope object, so all 22 of its cases failed with "scope.externalBasePath is not a function". Caught by CI, not locally — that suite wasn't in the set I ran. The stand-in now mirrors the two members of the real Scope contract the plugin uses (`mount` and `externalBasePath`), and `fakeScope` takes a mount so mounted behavior is expressible.
Addresses PR review (claude + gemini-code-assist).
`mountKey` concatenated the mount and the plugin's *raw* urlPath with no separator, so distinct
routes collided: mount '/a' + 'bc' and mount '/ab' + 'c' both produced '/abc', and the second
application's REST registration would be skipped — reintroducing the very bug the per-mount
keying was added to fix.
Rather than separating the parts, key on the route the registration actually answers on. That
is injective for distinct routes and still dedupes configurations that genuinely resolve to the
same chain (a '/api' mount with no plugin path, and a plugin path of '/api' with no mount, are
one route and should register once).
To avoid two definitions of "which route is this", the composition now lives in
`Scope.routeFor()`, used by both `scope.server` and REST. A test asserts the proxy injects
exactly what `routeFor()` returns, so they cannot drift.
Also from review: hoist the `host` Joi schemas to module scope instead of re-compiling them on
every deploy_component, and use `{#label}` in the custom messages per repo convention.
The mount is applied at exactly one place (Scope.routeFor). Documents why it must not reach the entry pipeline — entry URL paths feed resource registration, and the router strips the mount before REST resolves them — plus the per-mount keying and isolation caveats. The static-only test blind spot is called out so the next change doesn't rediscover it.
…ross-mount isolation) - componentLoader: an invalid root-config mount now fails the application closed (skips loading it) instead of falling back to unmounted access, which silently dropped the operator-declared isolation. - REST: handleApplication closes over resources/httpOptions per call instead of a shared module-level var, and skips deploy pre-flight validation scopes entirely so they can't splice into the live request path or permanently mark a mount as started. - fastifyRoutes: a host-mounted legacy Fastify app now fails to load (rather than warn) since the fallback truly can't be constrained by host. - middlewareChain: before/after name resolution is scoped per mounted route group, with a global registry limited to genuinely unmounted entries (e.g. authentication) — same-named plugins in different mounted applications can no longer resolve into each other's chain. - static: the mount-root redirect now gates on the external base path so a root-level static plugin still redirects when the application itself carries a mount. Adds unit coverage for the middlewareChain cross-mount isolation and the static mount-root redirect, and records the new invariants in DESIGN.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Format Check was failing on emphasis markers (*text*) that prettier normalizes to underscores (_text_). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Ethan-Arrowood
left a comment
There was a problem hiding this comment.
The routing-boundary design is sound and docs coverage (documentation#612, including the case-sensitivity correction) is good. Two significant gaps first, then smaller items — all net-new, nothing from the earlier threads (those all resolved cleanly at b6f16619).
One question the inline comments raise: was HTTP/2 considered for host matching? If host mounts are h1-only by design, that limitation belongs in DESIGN.md and the docs PR.
Also from the PR description: it says fastify + host mount "warns", but the code throws (and DESIGN.md documents the throw — which is correct). Worth updating the description so the merge record matches.
Procedural note: both significant findings share a root cause — routing config has two entry paths (deploy op vs hand-edited root config) with validation on only one. A standing rule that a new root-config key must state where the hand-edited path is validated would catch this class at review time.
sent with Claude Fable 5
…rf, warning) - matchesRoute now reads request.host (Harper's Request getter, handles HTTP/2's :authority pseudo-header) with header fallbacks, instead of only headers.asObject.host — HTTP/2 clients never send Host, so host-mounted apps 404'd under h2 while h1 worked. - hostnameFromHeader strips a trailing dot (absolute-FQDN form). - buildRoutedChain precomputes hostLower/urlPath/urlPathWithSlash once per chain build instead of per request/per candidate, and extracts the request's authority once per request instead of once per candidate probe. - scopeMount.normalizeMountHost now validates against the same grammar as the deploy_component operation's host field (Joi hostname/IPv6), so a hand-edited root-config host with a port/scheme/path fails the application closed instead of loading it unreachably. - nestScopeMount logs a warning when a child's host is discarded by the parent-authority rule. Refs #1964 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… hooks Findings from the independent pre-push review of this branch: - toScopeMount silently treated a present but non-string host/urlPath (e.g. an unquoted YAML value like `host: 9926`) as absent, loading the application unconstrained instead of failing closed like a malformed string value already does. - The deprecated `start`/`startOnMainThread` extension API hands the plugin the raw, unmounted server directly (unlike the new Plugin API's handleApplication(scope)), so a mounted application using it would silently register routes outside the mount. Refuse to load rather than imply an isolation the hook can't honor — the same fail-closed pattern fastifyRoutes.ts already applies to its own legacy fallback. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Summary
Makes
host/urlPathon an application's root-config entry authoritative for where that application is served.Why
Where an application is served is a deployment concern, but
host/urlPathwere only readable from the config file that declared a plugin. For an application that is its ownconfig.yaml— so the hostname had to be checked into the app, unoverridable from outside it (the env-config overlay is root-config-only).Worse, these keys on a root-config application entry were silently inert. An application's plugin scopes read the application's own
config.yaml, and nothing carried the root entry's routing down to them — sodeploy_component urlPath=/api(#1113) persisted a value that changed nothing. The flow #1113 described only ever held for a root-declared plugin, whose scope does read the root config.Design
The mount is applied at exactly one place: the routing boundary, in
Scope'sserverproxy. The router strips it before a handler runs, so everything inside the application — entry URL paths, and the resource pathsgraphqlSchema/jsResourcederive from them — stays mount-relative.Scope.mount/Scope.externalBasePath()expose it narrowly for the only two cases that need it: emitting an absolute URL to the client (static's redirectLocation) and bypassing the routed chain (legacy fastify).hostreplaces the app's value outright — an operator remapping a hostname must win over one the app shipped.urlPathcomposes rather than replaces, because a plugin'surlPathdoubles as its app-internal base path (static's asset root, fastify's prefix). Mount/v1+static: { urlPath: assets }→/v1/assets/. Replacing would silently relocate app-internal URLs and collapse distinct plugins onto one path.packagerecursion and the components-root directory scan. The scan matters most: it loads apps with no root entry at all, so a mount works for a payload-deployed app too.deploy_componentaccepts and persistshostalongsideurlPath.Where to look
components/Scope.ts— the mount is applied here and nowhere else; that's the invariant the whole change rests on. The proxy also now resolves a config-suppliedurlPathinstead of passing it through, fixing REST spreading its config section and handing the router a literal'./'(→ unmatchable route'/.').server/REST.ts— registration was guarded by a module-levelstartedboolean, so with two applications mounted at different paths only the first got a chain and the second's REST API silently 404'd. Now keyed per mount. This is the headline use case, so it's worth a look.server/middlewareChain.ts— host matching was case-sensitive though hostnames aren't (RFC 4343), so a configuredAPI.example.comcould never match;[::1]:9926also parsed to[.components/scopeMount.ts— the composed path is deliberately a fixed point ofresolveBaseURLPath, since consumers resolve it again.Second commit is the cross-model review response — it reworked the approach (see below), so reading the commits in order may be easier than the squashed diff.
Open items for the reviewer
hostis a known limitation, not fixed here.fastifyRoutesregisters as a global fallback on the bare server, so it never joins a routed chain and ahostmount cannot constrain it — those routes answer on every hostname. AurlPathmount does work (it becomes the fastify prefix). Loading a host-mounted app withfastifyRoutesnow warns rather than implying isolation it can't provide. Fixing it properly means routing a rawhttp.Serverthrough a chain, which the chain doesn't support. Happy to file a follow-up if you'd rather block on it.app-mount-two.test.tsand documented — flagging in case you expected a namespace boundary.urlPathcomposition vs. replacement is the one semantic judgment call; rationale above. Replacement would be the alternative if you disagree.OptionsWatchersogetAll()was the single effective view. Codex review exposed that this also feeds the entry pipeline, sographqlSchemaregistered/v1/MountedThingwhile REST looked upMountedThing— every mounted REST route 404'd, hidden in the single-app static test because static de-prefixes its own keys. Reverted to the routing-boundary-only design above.Tests
unitTests/components/scopeMount.test.js(new), plus additions toScope,middlewareChain, anddeployComponentValidatorsuites. Integration:app-root-mount.test.ts(urlPath / host / both, and a mounted table round-trip) andapp-mount-two.test.ts(two apps, two mounts — the REST regression). 38/38 integration pass; affected unit suites green.componentLoader.test.jshangs locally on this machine (pre-existing watcher contention) — relying on CI for it.Docs: Route applications by host/urlPath in the root config (HarperFast/documentation#612)
Generated by Claude Opus 5, with cross-model review by OpenAI Codex (the Gemini leg timed out — no coverage from it).