Skip to content

Commit de9f741

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-22727-formula-currency-result
2 parents 20d64a3 + a360cee commit de9f741

3 files changed

Lines changed: 244 additions & 4 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/cloud-connection': patch
3+
---
4+
5+
fix(cloud-connection): install-local's `hotLoaded` reports whether the running kernel loaded the package (#22695)
6+
7+
Clause-②: no
8+
9+
- **What was wrong.** `POST /api/v1/marketplace/install-local` always answered `hotLoaded: true`. When the manifest came from the cloud catalog and `manifest.register` threw, the install still wrote the package to its ledger and answered `200` (the lenient path). The package loads at the next restart, but the answer said the running kernel held it already, and its `note` said the app was "now available in this runtime".
10+
- **What it answers now.** On that path the answer carries `hotLoaded: false` and a new string key, `hotLoadError`, holding the register error's message. The `note` says the package is installed and cached, that the running kernel could not load it, and that the runtime registers it again at its next restart. A reader can then say "installed, loads at the next restart" instead of reporting a false success.
11+
- **What did not change.** An install whose register succeeds answers exactly as before: `hotLoaded: true`, and no `hotLoadError` key. An inline (file-import) manifest whose register throws is still refused with `422 PLUGIN_REGISTER_FAILED`, and nothing is written. The lenient path still installs: the ledger entry is written, and the package loads at the next restart.
Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #22695 — `POST /api/v1/marketplace/install-local` answers `hotLoaded` from
5+
* what its hot-register (step 3, `manifest.register`) actually did.
6+
*
7+
* ## What was wrong
8+
*
9+
* The answer wrote `hotLoaded: true` as a literal. A cloud-fetched manifest
10+
* whose register throws takes the LENIENT path — the install still persists its
11+
* ledger entry and answers `200`, and the package loads at the next restart —
12+
* so that answer told its caller the running kernel held a package it did not.
13+
* The console read it as "the app should now appear" and refreshed an app list
14+
* that did not have the app.
15+
*
16+
* ## What these cases pin
17+
*
18+
* - the lenient path: `200`, `hotLoaded: false`, the register error's message
19+
* as `hotLoadError`, the ledger entry written (the path stays lenient — ⛔
20+
* whether it should is not decided here), and the note no longer claims the
21+
* app is available;
22+
* - CONTROL, a register that resolves: `hotLoaded: true` and NO `hotLoadError`
23+
* key at all — omitted, not nulled, so the success answer is unchanged;
24+
* - CONTROL, an inline manifest whose register throws: still refused with
25+
* `422 PLUGIN_REGISTER_FAILED` in the declared envelope, nothing written.
26+
*/
27+
28+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
29+
import { mkdtempSync, rmSync, readdirSync } from 'node:fs';
30+
import { join } from 'node:path';
31+
import { tmpdir } from 'node:os';
32+
import { BaseResponseSchema, ApiErrorSchema, envelopeViolations } from '@objectstack/spec/api';
33+
// An install binds the package's handlers through `@objectstack/runtime` (a
34+
// lazy `import()` inside the plugin). Its first load is paid here, at module
35+
// top — never inside a clocked `it` (`scripts/check-test-source-alias.mjs`).
36+
import '@objectstack/runtime';
37+
import { MarketplaceInstallLocalPlugin } from './marketplace-install-local-plugin.js';
38+
import { LocalManifestSource } from './local-manifest-source.js';
39+
import { installerAuthService, withInstallerGrants } from './install-local-principal.fixtures.js';
40+
41+
type Handler = (c: any) => Promise<any>;
42+
43+
const ROUTE = '/api/v1/marketplace/install-local';
44+
const APP_ID = 'com.example.crm';
45+
const REGISTER_FAILURE = 'object crm_account: field owner references unknown object sys_owner';
46+
47+
function makeRawApp() {
48+
const routes = new Map<string, Handler>();
49+
return {
50+
routes,
51+
get: (p: string, h: Handler) => routes.set(`GET ${p}`, h),
52+
post: (p: string, h: Handler) => routes.set(`POST ${p}`, h),
53+
delete: (p: string, h: Handler) => routes.set(`DELETE ${p}`, h),
54+
};
55+
}
56+
57+
/**
58+
* Mount the plugin through its real lifecycle (`start()` + `kernel:ready`).
59+
* `failRegisterOf` names the ONE manifest id whose register throws — the
60+
* plugin's own UI bundle still registers at `kernel:ready`, as on a real boot.
61+
*/
62+
async function mount(dir: string, opts: { controlPlaneUrl?: string; failRegisterOf?: string } = {}) {
63+
const rawApp = makeRawApp();
64+
const hooks = new Map<string, any>();
65+
const registry: any[] = [];
66+
const register = vi.fn(async (m: any) => {
67+
if (opts.failRegisterOf !== undefined && m?.id === opts.failRegisterOf) {
68+
throw new Error(REGISTER_FAILURE);
69+
}
70+
registry.push({ manifest: m });
71+
});
72+
const syncSchemas = vi.fn(async () => undefined);
73+
const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn() };
74+
const services: Record<string, any> = {
75+
manifest: { register },
76+
auth: installerAuthService(),
77+
objectql: withInstallerGrants({ syncSchemas, registry: { getAllPackages: () => registry } }),
78+
};
79+
const ctx = {
80+
hook: (e: string, h: any) => hooks.set(e, h),
81+
getService: (name: string) => {
82+
if (name === 'http-server') return { getRawApp: () => rawApp };
83+
const svc = services[name];
84+
if (svc === undefined) throw new Error(`no ${name}`);
85+
return svc;
86+
},
87+
logger,
88+
};
89+
const plugin = new MarketplaceInstallLocalPlugin({ controlPlaneUrl: opts.controlPlaneUrl ?? 'off', storageDir: dir });
90+
await plugin.start(ctx as any);
91+
await hooks.get('kernel:ready')?.();
92+
const registeredAtBoot = register.mock.calls.length;
93+
return {
94+
install: (body: unknown) => rawApp.routes.get(`POST ${ROUTE}`)!(makeC(body)),
95+
logger,
96+
/** The manifest ids the install ASKED to register, whether or not the register resolved. */
97+
installRegisterAttempts: () => register.mock.calls.slice(registeredAtBoot).map((call) => call[0]?.id),
98+
};
99+
}
100+
101+
function makeC(body: any) {
102+
const json = vi.fn((payload: any, status?: number) => ({ payload, status: status ?? 200 }));
103+
return {
104+
req: {
105+
url: `http://localhost:3000${ROUTE}`,
106+
raw: new Request(`http://localhost:3000${ROUTE}`),
107+
json: async () => body,
108+
param: () => undefined,
109+
},
110+
json,
111+
};
112+
}
113+
114+
/** Serve one manifest snapshot from the cloud leg; the public fast-path is off. */
115+
function serveSnapshot(manifest: unknown) {
116+
vi.stubEnv('OS_MARKETPLACE_PUBLIC_BASE_URL', 'off');
117+
// A non-empty service key, so the credential read never reaches the
118+
// developer's own on-disk binding.
119+
vi.stubEnv('OS_CLOUD_API_KEY', 'svc-test-key');
120+
const fetchMock = vi.fn(async () => new Response(
121+
JSON.stringify({ data: { manifest, version: '1.0.0', version_id: 'ver_1' } }),
122+
{ status: 200, headers: { 'Content-Type': 'application/json' } },
123+
));
124+
vi.stubGlobal('fetch', fetchMock);
125+
return fetchMock;
126+
}
127+
128+
const MANIFEST = { id: APP_ID, version: '1.0.0', objects: [] };
129+
130+
let dir: string;
131+
beforeEach(() => { dir = mkdtempSync(join(tmpdir(), 'mil-hot-loaded-')); });
132+
afterEach(() => {
133+
rmSync(dir, { recursive: true, force: true });
134+
vi.unstubAllGlobals();
135+
vi.unstubAllEnvs();
136+
vi.restoreAllMocks();
137+
});
138+
139+
describe('install-local answers hotLoaded from what the hot-register did', () => {
140+
it('a cloud-fetched manifest whose register throws answers 200 with hotLoaded: false and the register error as hotLoadError', async () => {
141+
const fetchMock = serveSnapshot(MANIFEST);
142+
const h = await mount(dir, { controlPlaneUrl: 'http://cloud.test', failRegisterOf: APP_ID });
143+
144+
const res = await h.install({ packageId: 'pkg_crm' });
145+
146+
// Identity: the manifest came from the cloud leg, and the install
147+
// really asked the kernel to register it — the throw is step 3's.
148+
expect(fetchMock).toHaveBeenCalledTimes(1);
149+
expect(h.installRegisterAttempts()).toEqual([APP_ID]);
150+
151+
expect(res.status, JSON.stringify(res.payload)).toBe(200);
152+
expect(res.payload.success).toBe(true);
153+
expect(res.payload.data.manifestId).toBe(APP_ID);
154+
expect(res.payload.data.hotLoaded).toBe(false);
155+
expect(res.payload.data.hotLoadError).toBe(REGISTER_FAILURE);
156+
// The note makes the same claim `hotLoaded` does, so it no longer says
157+
// the app is available in a kernel that does not hold it.
158+
expect(res.payload.data.note).not.toContain('now available');
159+
160+
// Still lenient: the ledger entry is written, so the next restart
161+
// loads it. Whether this path should stay lenient is not decided here.
162+
expect(new LocalManifestSource(dir).read(APP_ID).entry?.manifestId).toBe(APP_ID);
163+
});
164+
165+
it('CONTROL — a register that resolves answers hotLoaded: true, with no hotLoadError key at all', async () => {
166+
serveSnapshot(MANIFEST);
167+
const h = await mount(dir, { controlPlaneUrl: 'http://cloud.test' });
168+
169+
const res = await h.install({ packageId: 'pkg_crm' });
170+
171+
expect(res.status, JSON.stringify(res.payload)).toBe(200);
172+
expect(res.payload.success).toBe(true);
173+
expect(h.installRegisterAttempts()).toEqual([APP_ID]);
174+
expect(res.payload.data.hotLoaded).toBe(true);
175+
expect(Object.keys(res.payload.data)).not.toContain('hotLoadError');
176+
expect(res.payload.data.note).toContain('now available');
177+
});
178+
179+
it('CONTROL — an inline manifest whose register throws is still refused: 422 PLUGIN_REGISTER_FAILED, nothing written', async () => {
180+
const h = await mount(dir, { failRegisterOf: APP_ID });
181+
182+
const res = await h.install({ manifest: MANIFEST });
183+
184+
expect(h.installRegisterAttempts()).toEqual([APP_ID]);
185+
expect(res.status).toBe(422);
186+
expect(BaseResponseSchema.safeParse(res.payload).success).toBe(true);
187+
expect(envelopeViolations(res.payload)).toEqual([]);
188+
expect(ApiErrorSchema.safeParse(res.payload.error).success).toBe(true);
189+
expect(res.payload.success).toBe(false);
190+
expect(res.payload.error.code).toBe('PLUGIN_REGISTER_FAILED');
191+
expect(res.payload.error.message).toContain(REGISTER_FAILURE);
192+
expect(readdirSync(dir)).toEqual([]);
193+
});
194+
});

‎packages/cloud-connection/src/marketplace-install-local-plugin.ts‎

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@
3838
* answers `OS_PROTOCOL_INCOMPATIBLE` (422) with the handshake's
3939
* diagnostic in `error.details`, the answer `POST /api/v1/packages`
4040
* gives (ADR-0087 D1, #21762), and nothing is registered or written.
41+
* The answer's `hotLoaded` says whether the running kernel's
42+
* `manifest.register` resolved (#22695). An inline manifest whose
43+
* register throws is refused (`PLUGIN_REGISTER_FAILED`, 422); a
44+
* cloud-fetched one is still installed (the lenient path) and answers
45+
* `200` with `hotLoaded: false` and the register error's message as
46+
* `hotLoadError` — in the ledger, loaded at the next restart.
4147
*
4248
* GET /api/v1/marketplace/install-local
4349
* → lists currently installed marketplace packages. Requires an
@@ -1249,24 +1255,40 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
12491255
// 3. Hot-register FIRST so a malformed inline manifest fails the
12501256
// install loudly rather than persisting a broken record that
12511257
// would also fail on every subsequent rehydrate.
1258+
//
1259+
// [#22695] What this step did is RECORDED here and carried to the
1260+
// answer: `hotLoadError` stays `undefined` exactly when the
1261+
// register resolved, and the answer's `hotLoaded` is derived from
1262+
// it — never a literal. Before this, `hotLoaded: true` was written
1263+
// unconditionally, so the lenient path below answered that the
1264+
// running kernel held a package it did not, and the console read
1265+
// that answer as "the app should now appear".
1266+
let hotLoadError: string | undefined;
12521267
try {
12531268
const manifestService = ctx.getService('manifest') as any;
12541269
// Awaited: register also bridges the manifest's objects into the
12551270
// metadata service — a caller that reads metadata right after a
12561271
// 200 (AI describe_object, Studio object list) must see them.
12571272
await manifestService.register(manifest);
12581273
} catch (err: any) {
1274+
const registerError = String(err?.message ?? err);
12591275
// For offline file imports we treat a register failure as a hard
12601276
// failure (don't persist). Cloud installs historically tolerated
12611277
// this (the on-disk record survives a restart), so keep that path
12621278
// lenient for backwards compatibility.
12631279
if (inlineManifest) {
12641280
return c.json({
12651281
success: false,
1266-
error: { code: 'PLUGIN_REGISTER_FAILED', message: `Failed to register imported manifest: ${err?.message ?? err}` },
1282+
error: { code: 'PLUGIN_REGISTER_FAILED', message: `Failed to register imported manifest: ${registerError}` },
12671283
}, 422);
12681284
}
1269-
ctx.logger?.warn?.(`[MarketplaceInstallLocal] hot-register failed for ${manifestId} (will load on next restart): ${err?.message ?? err}`);
1285+
// [#22695] The lenient path stays lenient — whether it should is a
1286+
// separate decision, ⛔ not made here. What changes is that the
1287+
// caller is TOLD: the answer carries `hotLoaded: false` and this
1288+
// message, so a reader can say "installed, loads at the next
1289+
// restart" instead of a false success.
1290+
hotLoadError = registerError;
1291+
ctx.logger?.warn?.(`[MarketplaceInstallLocal] hot-register failed for ${manifestId} (will load on next restart): ${registerError}`);
12701292
}
12711293

12721294
// 4. Persist on disk
@@ -1345,7 +1367,14 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
13451367
version,
13461368
versionId: resolvedVersionId,
13471369
installedAt: entry.installedAt,
1348-
hotLoaded: true,
1370+
// [#22695] Whether step 3's register resolved — `false` only on
1371+
// the lenient path, where the package is in the ledger and the
1372+
// running kernel does not hold it until the next restart.
1373+
hotLoaded: hotLoadError === undefined,
1374+
// The register error's message, beside `hotLoaded: false` and
1375+
// ONLY there: omitted, not nulled, on a hot-loaded install, so
1376+
// the success answer's key set is unchanged.
1377+
...(hotLoadError !== undefined ? { hotLoadError } : {}),
13491378
upgradedFrom: conflict === 'marketplace' ? 'previous-marketplace-version' : null,
13501379
translationsLoaded: seededSummary.translationsLoaded,
13511380
seeded: seededSummary.seeded,
@@ -1358,7 +1387,13 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
13581387
// Keep the two endpoints reading `this.storageDir` — a second
13591388
// derivation is how they diverged in the first place.
13601389
storageDir: this.storageDir,
1361-
note: 'App is now available in this runtime. Refresh the console to see it in the app switcher.',
1390+
// [#22695] The note makes the same claim `hotLoaded` does, so it
1391+
// follows the same record: it never says "now available" over a
1392+
// kernel that does not hold the package.
1393+
note: hotLoadError === undefined
1394+
? 'App is now available in this runtime. Refresh the console to see it in the app switcher.'
1395+
: 'App installed and cached on this runtime, but the running kernel could not load it '
1396+
+ '(`hotLoadError` says why). The runtime registers it again at its next restart.',
13621397
},
13631398
}, 200);
13641399
};

0 commit comments

Comments
 (0)