Skip to content

Commit 4018fc1

Browse files
os-zhuangclaude
andauthored
fix(cloud-connection): derive features.installLocal from what is mounted, keep the option as a ceiling (#8388) (#8466)
`GET /api/v1/runtime/config` reported `installLocal` straight from the constructor option, beside a `marketplace` key #8356 had just made an observation — two flags in one object answered by different rules. The declared one is the key #8343 measured wrong on a real self-hosted deployment: `installLocal: true` with both verbs of `/api/v1/marketplace/install-local` returning 404. The flag is now observed per request off the serving app's route ledger, by a sibling predicate rather than a shared one: the browse predicate subtracts exactly the paths this one requires. The two share the prefix constant, so "what counts as install-local" has a single definition. The constructor option is kept (hosts pass it today) as a ceiling: omitted or `true` defers to the observation, `false` lowers it. It cannot raise the answer — the CLI's own frozen RUNTIME_CONFIG_OPTIONS passes `installLocal: true` unconditionally, so honouring `true` upward would have left the derivation inert on exactly the path #8343 reported. Repairs one pin in packages/cli that this change correctly invalidates: the "two offline guards are INDEPENDENT" case handed the resolver an install-local identity without mounting a route, which was faithful while the flag was declarative. It now mounts the real plugin; its assertions are unchanged. Claude-Session: https://claude.ai/code/session_01P7vaLs7bhBPi9m3JyzkhDj Co-authored-by: Claude <noreply@anthropic.com>
1 parent fc3a819 commit 4018fc1

5 files changed

Lines changed: 568 additions & 23 deletions

File tree

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
---
2+
"@objectstack/cloud-connection": patch
3+
---
4+
5+
fix(cloud-connection): `features.installLocal` is derived from what is mounted; the constructor option becomes a ceiling (#8388)
6+
7+
`GET /api/v1/runtime/config` reported `installLocal` straight from the
8+
constructor option, next to a `marketplace` key that #8356 had just made an
9+
observation. Two flags in one object, answered by different rules — and the
10+
declared one is the key #8343 actually measured wrong on a real self-hosted
11+
deployment:
12+
13+
```json
14+
{"features":{"installLocal":true,"marketplace":true, …}}
15+
```
16+
17+
```
18+
GET /api/v1/marketplace/install-local -> 404 {"error":"Not found"}
19+
POST /api/v1/marketplace/install-local -> 404 {"error":"Not found"}
20+
```
21+
22+
Nothing checked that `MarketplaceInstallLocalPlugin` was mounted on the kernel
23+
serving the response, so `new RuntimeConfigPlugin({ installLocal: true })` on a
24+
runtime that never mounted it announced a capability whose route 404s, and the
25+
Console rendered an install affordance that could not work.
26+
27+
The flag is now **observed** per request, off the route table of the app serving
28+
the response — the same seam #8356 built, read by a sibling predicate rather than
29+
a shared one, because the browse predicate subtracts exactly the paths this one
30+
requires. The two share the prefix constant, so "what counts as install-local"
31+
has one definition and the flags cannot both claim, or both disown, the same
32+
route.
33+
34+
**The `installLocal` constructor option is kept, as a ceiling.** Hosts pass it
35+
today, so it is not removed:
36+
37+
- omitted or `true` — report what is actually mounted (what every host passing
38+
`installLocal: true` already meant);
39+
- `false` — report `false` even where the plugin is mounted, for an operator who
40+
wants the affordance hidden.
41+
42+
It deliberately cannot raise the answer. A plain override would have left the
43+
measured defect standing: the CLI's own frozen `RUNTIME_CONFIG_OPTIONS` passes
44+
`installLocal: true` unconditionally, so honouring `true` upward would keep
45+
"declared `true`, route 404s" reachable on exactly the product path #8343
46+
reported, leaving the derivation inert where it is most needed.
47+
48+
**What changes for hosts.** A runtime that mounts an install-local surface
49+
reports exactly what it did before. A runtime that mounts none now reports
50+
`installLocal: false` instead of whatever it declared — the correction. An
51+
omitted option no longer means `false`: it defers to the observation, so a host
52+
that mounts the plugin and forgot the flag now gets the truthful `true` it should
53+
always have had.
54+
55+
**Escape hatch, unchanged.** The derivation is the base value, not a veto: the
56+
open-core `resolveFeatures` seam still merges over it (and over the ceiling), so a
57+
host on an adapter whose raw app exposes no route ledger — where both derived
58+
flags conservatively report `false`, with a warning logged once at mount time —
59+
can still declare the capability it knows it serves.

‎packages/cli/test/serve-marketplace-offline-runtime-config.test.ts‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,13 +311,35 @@ describe('#8389: the wiring plan mounts runtime-config on the offline arm', () =
311311
// One shared gate would have been the smaller change and would have
312312
// excluded exactly this box: it serves install-local (its own), serves no
313313
// runtime-config, and therefore has the #8389 defect in full.
314+
//
315+
// [#8388] "the host wires its OWN install-local" now has to mean the host
316+
// MOUNTS one. `INSTALL_LOCAL` below is only an identity handed to the
317+
// resolver, and since `features.installLocal` became an observation of the
318+
// route table rather than the constructor flag, an identity with no route
319+
// behind it models a box announcing a capability it cannot serve — #8343's
320+
// measured defect, which is not this case's subject. So the host's own
321+
// plugin is really started on the same app, ahead of the arm, exactly as
322+
// the positive control mounts the real proxy. The assertions below are
323+
// unchanged; what changed is that the fixture now earns them.
314324
const dir = tempStorageDir();
315325
try {
316-
const { wiring, app } = await bootOfflineArm({ plugins: [INSTALL_LOCAL], storageDir: dir });
326+
const { MarketplaceInstallLocalPlugin } = await import('@objectstack/cloud-connection');
327+
const { wiring, app } = await bootOfflineArm({
328+
plugins: [INSTALL_LOCAL],
329+
storageDir: dir,
330+
preMounted: [new MarketplaceInstallLocalPlugin({
331+
controlPlaneUrl: Serve.OFFLINE_CONTROL_PLANE,
332+
storageDir: dir,
333+
})],
334+
});
317335

318336
expect(wiring.offlineInstallLocal, "the host's own install-local is left alone").toBe(false);
319337
expect(wiring.offlineRuntimeConfig).toBe(true);
320338

339+
// ...and the host's own mount is the ONLY install-local surface here —
340+
// the arm added none, which is what `offlineInstallLocal: false` means.
341+
expect(app.routes.some((r) => r.path.startsWith('/api/v1/marketplace/install-local'))).toBe(true);
342+
321343
const body = await readConfig(app);
322344
expect(body.features.installLocal).toBe(true);
323345
expect(body.features.marketplace).toBe(false);

‎packages/cloud-connection/README.md‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,14 @@ const plugins = [
4040
// DEFAULT_CLOUD_URL. 'off' is one of the documented disable sentinels and
4141
// is the value that actually resolves to no cloud.
4242
new MarketplaceInstallLocalPlugin({ controlPlaneUrl: cloudUrl || 'off' }),
43-
// NOT cloud-gated: features.marketplace is derived from what is actually
44-
// mounted, not from this constructor call, so a cloud-less runtime reports
45-
// marketplace: false on its own — there is nothing here to keep in sync.
43+
// NOT cloud-gated: BOTH features.marketplace and features.installLocal are
44+
// derived from what is actually mounted, not from this constructor call, so
45+
// a cloud-less runtime reports marketplace: false and installLocal: true on
46+
// its own — there is nothing here to keep in sync.
47+
// `installLocal: true` is therefore a CEILING, not a declaration: it is the
48+
// default, and it cannot make the flag report a route this runtime never
49+
// mounted. Pass `false` to hide the affordance on a box that could serve it;
50+
// omitting it entirely behaves the same as `true`.
4651
// `''` here, unlike its neighbor above, is correct as-is: this plugin does
4752
// NOT re-resolve controlPlaneUrl through resolveCloudUrl(), so '' means
4853
// "stay on this origin" rather than "unset" — do not "fix" it to 'off'.

0 commit comments

Comments
 (0)