From 5597fab9c6370ae3d3899b2008b49e7c3f72368d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 31 Jul 2026 01:00:43 +0000 Subject: [PATCH 1/2] fix(metadata): declare the `artifact-api` source as implemented, and fix its dead URL guard (#4246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MetadataPluginOptions.artifactSource` said "Only `local-file` is implemented now; `artifact-api` is reserved for M3/M4" while `_loadFromArtifactApi` and all three `start()` dispatch branches (`eager` / `lazy` / `artifact-only`) already shipped. That is Prime Directive #10's declared ≠ enforced running backwards — the declaration understating the runtime — and it is not self-correcting: `implementation-status.mdx` copied the claim, so each docs accuracy audit rediscovers the contradiction and can only re-file it. Resolved toward option 1 of the issue: the implementation is valid, so the declaration moves. The option doc, the docs bullet, the metadata-service page and the package ROADMAP now describe what `start()` does — both modes, the `environmentId` requirement, `?commit=` pinning and the Bearer token. The test guarding this passed for the wrong reason. "artifact-only bootstrap rejects the not-yet-implemented artifact-api source" asserted `rejects.toThrow(/artifact-api/)`, but the only throw on that path is the missing-`environmentId` pre-flight guard, whose message merely contains the string — it matched while proving nothing. Replaced with a suite pinning URL construction for both accepted input shapes, commit pinning, the Authorization header, dispatch from each bootstrap mode, a loud failure on a non-OK response, and the `environmentId` guard asserted on its own message. Auditing the loader to justify "valid" surfaced one real defect. The URL builder chose between appending the canonical path and using the URL as-is by testing for a `/api/v{n}/cloud/projects/` segment — a path the v5.0 `project → environment` rename deleted, leaving the guard unmatchable. Every already-resolved artifact URL therefore had the canonical path appended a second time and 404'd, so half of the option's documented input shape was dead. The check is now "does the path already name an artifact endpoint", restoring the intended dual form and keeping the `unlisted` public route (`/pub/v1/environments/:id/artifact`) addressable. Callers passing a control-plane base URL — the only form that worked — are unaffected. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FC9p9G2sHw1Tbf1wvLJhVK --- ...data-artifact-api-declared-not-reserved.md | 41 ++++ .../docs/protocol/kernel/metadata-service.mdx | 29 +++ .../docs/releases/implementation-status.mdx | 3 +- packages/metadata/ROADMAP.md | 2 +- packages/metadata/src/metadata.test.ts | 184 +++++++++++++++++- packages/metadata/src/plugin.ts | 52 ++++- 6 files changed, 298 insertions(+), 13 deletions(-) create mode 100644 .changeset/metadata-artifact-api-declared-not-reserved.md diff --git a/.changeset/metadata-artifact-api-declared-not-reserved.md b/.changeset/metadata-artifact-api-declared-not-reserved.md new file mode 100644 index 0000000000..4db365ca89 --- /dev/null +++ b/.changeset/metadata-artifact-api-declared-not-reserved.md @@ -0,0 +1,41 @@ +--- +"@objectstack/metadata": patch +--- + +fix(metadata): the `artifact-api` source is declared as what it is — implemented (#4246) + +`MetadataPluginOptions.artifactSource` carried a type comment saying "Only +`local-file` is implemented now; `artifact-api` is reserved for M3/M4". It had +outlived its subject: `_loadFromArtifactApi` ships, and all three bootstrap +branches (`eager` / `lazy` / `artifact-only`) dispatch to it. A caller reading +the comment would conclude `mode: 'artifact-api'` was inert; supplying it plus +`environmentId` boots the server off a real network fetch. + +This is Prime Directive #10's `declared ≠ enforced` running backwards — the +declaration understates the runtime instead of overstating it — and it is not +self-correcting: `implementation-status.mdx` copied the claim, so every docs +accuracy audit rediscovers the contradiction and can only re-file it (which is +how this issue was born). The comment, the docs bullet, and the package ROADMAP +now describe the shipped behaviour. + +**The test that was supposed to hold the line was passing for the wrong reason.** +`artifact-only bootstrap rejects the not-yet-implemented artifact-api source` +asserted `rejects.toThrow(/artifact-api/)`. The only throw on that path is the +missing-`environmentId` pre-flight guard, whose message merely *contains* the +string `artifact-api` — so the assertion matched while proving nothing about +implementation status. It is replaced by a suite that pins what the loader does: +URL construction from both accepted input shapes, `?commit=` pinning, the Bearer +header, dispatch from each of the three bootstrap modes, a loud failure on a +non-OK response, and the `environmentId` guard asserted on *its own* message. + +**One real defect fell out of the audit.** The URL builder chose between "append +the canonical path" and "use the URL as-is" by testing for a +`/api/v{n}/cloud/projects/` segment — a path the v5.0 `project → environment` +rename deleted. The guard had therefore been unmatchable since v5.0: an +already-resolved artifact URL got the canonical path appended a *second* time +(`…/artifact/api/v1/cloud/environments/…/artifact`) and 404'd, so half of this +option's documented input shape was dead. The check is now "does the path +already name an artifact endpoint", which restores the intended dual form and +keeps the `unlisted`-visibility public route (`/pub/v1/environments/:id/artifact`) +addressable. Callers passing a control-plane base URL — the common case, and the +only one that worked — are unaffected. diff --git a/content/docs/protocol/kernel/metadata-service.mdx b/content/docs/protocol/kernel/metadata-service.mdx index 816340d38e..68add6ad16 100644 --- a/content/docs/protocol/kernel/metadata-service.mdx +++ b/content/docs/protocol/kernel/metadata-service.mdx @@ -196,6 +196,35 @@ new MetadataPlugin({ }); ``` +#### Artifact sources + +Both `artifactSource` modes are implemented, and every bootstrap mode above +dispatches to them: + +| Mode | Reads from | Notes | +| :--- | :--- | :--- | +| `local-file` | `path` — a filesystem path, or an `http(s)` URL fetched verbatim | The `os dev` / `os serve` path. Only this mode gets the artifact-file HMR watcher (`artifactWatch`). Outside `artifact-only`, an absent local file is "nothing compiled yet", not a fault. | +| `artifact-api` | The control plane's `GET /api/v1/cloud/environments/:environmentId/artifact` | Requires `environmentId` on the plugin options; `start()` throws when it is missing. `token` is sent as `Authorization: Bearer`, `commitId` pins a published revision via `?commit=`. | + +```typescript +new MetadataPlugin({ + config: { bootstrap: 'artifact-only' }, + environmentId: 'env_42', + artifactSource: { + mode: 'artifact-api', + url: 'https://cloud.example.com', // control-plane base — canonical path appended + token: process.env.OS_CLOUD_TOKEN, + commitId: 'cmt_1a2b', // optional: pin a published revision + }, +}); +``` + +`url` may instead be an already-resolved endpoint — any URL whose path ends in +`/artifact` is used verbatim, which is how the `unlisted`-visibility public +route (`/pub/v1/environments/:id/artifact`) is reached. Both modes honor +`fetchTimeoutMs` (and `OS_ARTIFACT_FETCH_TIMEOUT_MS`, default 60 s) for remote +reads. + ### 2. Persistence Write Gates `MetadataManagerConfigSchema.persistence` is a two-axis runtime freeze. Both flags default to `true`. diff --git a/content/docs/releases/implementation-status.mdx b/content/docs/releases/implementation-status.mdx index 61688480d6..d95b45396a 100644 --- a/content/docs/releases/implementation-status.mdx +++ b/content/docs/releases/implementation-status.mdx @@ -72,7 +72,8 @@ This matrix is generated from actual codebase analysis and represents the curren - Metadata is loaded from config files or `dist/objectstack.json` at startup and held in memory - `MetadataPlugin` auto-provisions the `sys_metadata` / `sys_metadata_history` storage objects (plus the ADR-0067 commit log and the metadata audit trail) by default, so `PUT /api/v1/meta/{type}/{name}` always has somewhere to write. Per-project (cloud) kernels opt out with `registerSystemObjects: false`, because there the control plane owns those tables - `MetadataPlugin` does **not** auto-bridge ObjectQL to a `DatabaseLoader` — there is no automatic runtime project-DB bridge, and nothing in the packages calls the bridge during boot. Database-backed metadata is an explicit `MetadataManager` opt-in (`setDataEngine()` / `setDatabaseDriver()`, or a `datasource` + `driver` pair on the manager config), and even then it is control-plane only: both methods return early whenever an `environmentId` is supplied, so a per-project kernel never gets a database-backed metadata loader -- Future: production Artifact API loader (`artifactSource: { mode: 'artifact-api' }` is reserved) and durable local artifact cache +- Both artifact sources ship. `artifactSource: { mode: 'local-file', path }` reads a compiled artifact (a filesystem path, or an `http(s)` URL fetched verbatim); `artifactSource: { mode: 'artifact-api', url, token?, commitId? }` pulls the published artifact from the control plane (`GET /api/v1/cloud/environments/:environmentId/artifact`). The API source additionally requires `environmentId` on the plugin — it throws at `start()` when that is missing — and is dispatched from all three bootstrap modes (`eager` / `lazy` / `artifact-only`). This page used to call the API loader "Future / reserved", faithfully copying a type comment that outlived the implementation it described (#4246) +- Future: durable local artifact cache — the API source re-fetches on every boot ### System Services diff --git a/packages/metadata/ROADMAP.md b/packages/metadata/ROADMAP.md index 8fa22297ec..c5e5ad985a 100644 --- a/packages/metadata/ROADMAP.md +++ b/packages/metadata/ROADMAP.md @@ -31,7 +31,7 @@ | **Bootstrap modes** | `MetadataPluginConfig.bootstrap` = `eager` \| `lazy` \| `artifact-only` — supports edge / serverless / read-only deployments. | | **Persistence write gates** | `MetadataManagerConfig.persistence.{ writable, overlayWritable }` — runtime freeze for sealed kernels. | | **Single-source schema discipline** | Canonical `MetadataManagerConfigSchema` / `MetadataFallbackStrategySchema` live in `kernel/metadata-loader.zod.ts` and are re-exported from `system/metadata-persistence.zod.ts`. | -| **`artifact-api` runtime source** | `MetadataPlugin` can boot from a remote control-plane artifact (`artifactSource: { mode: 'artifact-api', url, projectId, commitId? }`). Wired across `eager` / `lazy` / `artifact-only` bootstrap modes. Configurable timeout via `fetchTimeoutMs` or `OS_ARTIFACT_FETCH_TIMEOUT_MS` (default 60 s). | +| **`artifact-api` runtime source** | `MetadataPlugin` can boot from a remote control-plane artifact (`artifactSource: { mode: 'artifact-api', url, token?, commitId? }` plus `environmentId` on the plugin options — the v5.0 rename retired `projectId`). Wired across `eager` / `lazy` / `artifact-only` bootstrap modes. `url` may be the control-plane base (canonical path appended) or an already-resolved `…/artifact` endpoint. Configurable timeout via `fetchTimeoutMs` or `OS_ARTIFACT_FETCH_TIMEOUT_MS` (default 60 s). | ### 🟡 Partially Implemented diff --git a/packages/metadata/src/metadata.test.ts b/packages/metadata/src/metadata.test.ts index 9a76c64fb9..5afca0d573 100644 --- a/packages/metadata/src/metadata.test.ts +++ b/packages/metadata/src/metadata.test.ts @@ -1,6 +1,6 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -839,22 +839,196 @@ describe('MetadataPlugin', () => { expect(manager.loadMany).not.toHaveBeenCalled(); }); - it('artifact-only bootstrap rejects the not-yet-implemented artifact-api source', async () => { + // #4246 — `artifact-api` IS implemented: `_loadFromArtifactApi` ships and all + // three bootstrap branches dispatch to it. The test that stood here — + // "artifact-only bootstrap rejects the not-yet-implemented artifact-api + // source" — passed for the wrong reason: the only throw on that path is the + // missing-`environmentId` guard, whose message happens to contain the string + // "artifact-api", so `rejects.toThrow(/artifact-api/)` matched while + // asserting nothing about implementation status. Supply `environmentId` and + // the plugin really does go to the network. The suite below pins what the + // loader actually does instead. + it('artifact-api requires environmentId — and fails for THAT reason, not as unimplemented', async () => { const { MetadataPlugin } = await import('./plugin.js'); const plugin = new MetadataPlugin({ rootDir: '/tmp/test', watch: false, config: { bootstrap: 'artifact-only' }, - artifactSource: { mode: 'artifact-api', url: 'https://example.com/artifact' }, + artifactSource: { mode: 'artifact-api', url: 'https://example.com' }, }); const manager = (plugin as any).manager; manager.loadMany = vi.fn().mockResolvedValue([]); + const fetchMock = vi.fn(); + const realFetch = globalThis.fetch; + globalThis.fetch = fetchMock as any; + const ctx = createMockPluginContext(); + try { + await plugin.init(ctx); + await expect(plugin.start(ctx)).rejects.toThrow(/requires options\.environmentId/); + // The guard is a pre-flight check, not a "mode unavailable" refusal — + // nothing was fetched, and nothing was scanned. + expect(fetchMock).not.toHaveBeenCalled(); + expect(manager.loadMany).not.toHaveBeenCalled(); + } finally { + globalThis.fetch = realFetch; + } + }); + }); + + // ---------- artifact-api source (#4246) ---------- + // + // The loader was reachable and functional long before this suite existed; the + // type comment on `MetadataPluginOptions.artifactSource` just claimed the + // opposite ("reserved for M3/M4"), and `implementation-status.mdx` copied the + // claim. These tests are the enforcement side of the corrected declaration. + describe('artifact-api source', () => { + const PROBE_OBJECT = { name: 'artifact_probe', label: 'Artifact Probe', fields: {} }; + const ARTIFACT_ENVELOPE = { + schemaVersion: '0.1', + environmentId: 'env_42', + commitId: 'cmt_1', + checksum: 'a'.repeat(64), + metadata: { objects: [PROBE_OBJECT] }, + }; + + let fetchMock: ReturnType; + let realFetch: typeof globalThis.fetch; + + beforeEach(() => { + realFetch = globalThis.fetch; + fetchMock = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + statusText: 'OK', + text: async () => JSON.stringify(ARTIFACT_ENVELOPE), + }); + globalThis.fetch = fetchMock as any; + }); + + afterEach(() => { + globalThis.fetch = realFetch; + }); + + async function startWith( + artifactSource: any, + config?: { bootstrap?: 'eager' | 'lazy' | 'artifact-only' }, + ) { + const { MetadataPlugin } = await import('./plugin.js'); + const plugin = new MetadataPlugin({ + rootDir: '/tmp/test', + watch: false, + environmentId: 'env_42', + ...(config ? { config } : {}), + artifactSource, + }); + (plugin as any).manager.loadMany = vi.fn().mockResolvedValue([]); const ctx = createMockPluginContext(); await plugin.init(ctx); - await expect(plugin.start(ctx)).rejects.toThrow(/artifact-api/); - expect(manager.loadMany).not.toHaveBeenCalled(); + await plugin.start(ctx); + return plugin; + } + + /** The single fetch the loader issued: [url, init]. */ + function fetchedCall() { + expect(fetchMock).toHaveBeenCalledTimes(1); + return fetchMock.mock.calls[0] as [string, { headers: Record }]; + } + + it('appends the canonical control-plane path to a base URL and registers the artifact', async () => { + const plugin = await startWith( + { mode: 'artifact-api', url: 'https://cloud.example.com/' }, + { bootstrap: 'artifact-only' }, + ); + + const [url] = fetchedCall(); + expect(url).toBe('https://cloud.example.com/api/v1/cloud/environments/env_42/artifact'); + // …and the payload really lands in the registry — the mode is not a stub. + expect((plugin as any).manager.register).toHaveBeenCalledWith( + 'object', + 'artifact_probe', + expect.objectContaining({ name: 'artifact_probe' }), + { notify: false }, + ); + }); + + // Regression for the dead guard found while resolving #4246: it tested for a + // `/api/v{n}/cloud/projects/` segment that the v5.0 `project → environment` + // rename deleted, so it never matched and an already-resolved URL had the + // canonical path appended a SECOND time. + it('uses an already-resolved artifact URL verbatim instead of appending the path twice', async () => { + await startWith({ + mode: 'artifact-api', + url: 'https://cloud.example.com/api/v1/cloud/environments/env_42/artifact', + }); + + const [url] = fetchedCall(); + expect(url).toBe('https://cloud.example.com/api/v1/cloud/environments/env_42/artifact'); + expect(url).not.toMatch(/artifact\/api/); + }); + + // Same rule keeps the `unlisted`-visibility public route addressable. + it('honors a non-canonical artifact endpoint (the /pub route)', async () => { + await startWith({ + mode: 'artifact-api', + url: 'https://cloud.example.com/pub/v1/environments/env_42/artifact', + }); + + expect(fetchedCall()[0]).toBe('https://cloud.example.com/pub/v1/environments/env_42/artifact'); + }); + + it('pins a published revision with ?commit= and sends the token as a Bearer header', async () => { + await startWith({ + mode: 'artifact-api', + url: 'https://cloud.example.com', + commitId: 'cmt_9/1', + token: 'tok_secret', + }); + + const [url, init] = fetchedCall(); + expect(url).toBe( + 'https://cloud.example.com/api/v1/cloud/environments/env_42/artifact?commit=cmt_9%2F1', + ); + expect(init.headers.Authorization).toBe('Bearer tok_secret'); + }); + + // The three dispatch sites in `start()` — the reason the "reserved" comment + // was actively misleading rather than merely stale. + it.each(['eager', 'lazy', 'artifact-only'] as const)( + '%s bootstrap loads through the artifact API when the source is set', + async (bootstrap) => { + const plugin = await startWith( + { mode: 'artifact-api', url: 'https://cloud.example.com' }, + { bootstrap }, + ); + + expect(fetchedCall()[0]).toBe( + 'https://cloud.example.com/api/v1/cloud/environments/env_42/artifact', + ); + expect((plugin as any).manager.register).toHaveBeenCalledWith( + 'object', + 'artifact_probe', + expect.objectContaining({ name: 'artifact_probe' }), + { notify: false }, + ); + // eager must not ALSO fall back to the filesystem scan + expect((plugin as any).manager.loadMany).not.toHaveBeenCalled(); + }, + ); + + it('fails loudly when the control plane rejects the request', async () => { + fetchMock.mockResolvedValue({ ok: false, status: 404, statusText: 'Not Found', text: async () => '' }); + const { MetadataPlugin } = await import('./plugin.js'); + const plugin = new MetadataPlugin({ + rootDir: '/tmp/test', + watch: false, + environmentId: 'env_42', + artifactSource: { mode: 'artifact-api', url: 'https://cloud.example.com' }, + }); + const ctx = createMockPluginContext(); + await plugin.init(ctx); + await expect(plugin.start(ctx)).rejects.toThrow(/Cannot load artifact from API .*HTTP 404/); }); }); }); diff --git a/packages/metadata/src/plugin.ts b/packages/metadata/src/plugin.ts index c6429dc9b4..898524f462 100644 --- a/packages/metadata/src/plugin.ts +++ b/packages/metadata/src/plugin.ts @@ -139,12 +139,35 @@ export interface MetadataPluginOptions { config?: Partial; /** Organization ID for metadata-scoped consumers; MetadataPlugin itself does not persist runtime metadata. */ organizationId?: string; - /** Project ID used by local artifact envelopes and metadata-scoped consumers. */ + /** + * Environment ID used by local artifact envelopes and metadata-scoped + * consumers, and REQUIRED by `artifactSource.mode: 'artifact-api'` — it + * addresses the control-plane endpoint. (The v5.0 rename retired the + * "project ID" wording this comment used to carry; see ADR-0006.) + */ environmentId?: string; /** * When set, MetadataPlugin loads metadata from an artifact instead of scanning - * the filesystem. Only `local-file` is implemented now; `artifact-api` is - * reserved for M3/M4. + * the filesystem. **Both modes are implemented** and are dispatched from all + * three bootstrap modes (`eager` / `lazy` / `artifact-only`) — see `start()`. + * + * - `local-file` — read `path` (a filesystem path, or an `http(s)://` URL + * fetched verbatim). The dev/`os serve` path; also the one mode the + * artifact-file HMR watcher ({@link artifactWatch}) applies to. + * - `artifact-api` — pull the published artifact from the cloud control + * plane (`GET /api/v1/cloud/environments/:environmentId/artifact`, the + * envelope declared by `EnvironmentArtifactSchema`). REQUIRES + * {@link environmentId}; `start()` throws when it is missing rather than + * fetching a URL it cannot address. `url` is the control-plane base URL + * (the canonical path is appended) or an already-resolved artifact + * endpoint — see `_loadFromArtifactApi`. `token` becomes a Bearer header, + * `commitId` pins a published revision, and both modes honor + * `fetchTimeoutMs` / `OS_ARTIFACT_FETCH_TIMEOUT_MS`. + * + * Keep this describing what `start()` actually does: it used to call + * `artifact-api` "reserved for M3/M4" long after the loader and all three + * dispatch sites shipped, which read as "this mode is inert" to callers and + * propagated into the docs (#4246). */ artifactSource?: | { mode: 'local-file'; path: string; fetchTimeoutMs?: number } @@ -718,7 +741,18 @@ export class MetadataPlugin implements Plugin { } /** - * P2: Load metadata from the cloud artifact API endpoint. + * Load metadata from the cloud control plane's artifact endpoint. Shipped + * and reachable from every bootstrap mode — see `MetadataPluginOptions. + * artifactSource` for the option contract and #4246 for why that doc + * comment used to claim otherwise. + * + * `src.url` accepts two forms, distinguished deterministically by whether + * the path already names an artifact endpoint: + * - **control-plane base** (`https://cloud.example.com`) — the canonical + * `/api/v1/cloud/environments/:environmentId/artifact` path is appended. + * - **already-resolved endpoint** (any URL whose path ends in `/artifact`) + * — used verbatim, so the `unlisted`-visibility public route + * (`/pub/v1/environments/:id/artifact`) is reachable too. */ private async _loadFromArtifactApi( ctx: PluginContext, @@ -732,8 +766,14 @@ export class MetadataPlugin implements Plugin { // Build the artifact URL: // ${url}/api/v1/cloud/environments/${environmentId}/artifact[?commit=${commitId}] let artifactUrl = src.url.replace(/\/+$/, ''); - // If the URL already contains /api/v1, use it as-is; otherwise append default path. - if (!/\/api\/v\d+\/cloud\/projects\//i.test(artifactUrl)) { + // A URL that already names an artifact endpoint is used as-is; anything + // else is a control-plane base and gets the canonical path appended. + // This guard used to test for `/api/v{n}/cloud/projects/` — a segment the + // v5.0 `project → environment` rename deleted, leaving it unmatchable. So + // every already-resolved URL silently had the canonical path appended a + // SECOND time (`…/artifact/api/v1/cloud/environments/…/artifact`) and + // 404'd: half of this option's documented input shape was dead (#4246). + if (!/\/artifact(?:$|\?)/i.test(artifactUrl)) { artifactUrl = `${artifactUrl}/api/v1/cloud/environments/${environmentId}/artifact`; } if (src.commitId) { From 660124f8b594d994e0ef4312b7ca828eb23d20f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 31 Jul 2026 02:22:47 +0000 Subject: [PATCH 2/2] refactor(metadata)!: remove the artifact-api artifact source (#4246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flips this branch's resolution of #4246 from "declare the implementation" to "remove it", on the owner's call: the one capability only artifact-api provided — a Bearer-authenticated pull of a private environment artifact — is not a supported need right now. The consumer audit that motivated the flip: no mode:'artifact-api' call site exists in this repo or in cloud. The cloud runtime pulls artifacts through its own ArtifactApiClient (TTL cache, singleflight, hostname resolution — a superset), and package distribution into a running OSS instance goes through @objectstack/cloud-connection (os package install). Public / commit-pinned artifact boot stays fully supported via the existing local-file URL form (the control plane's public /pub/v1/environments/:id/artifact route serves it verbatim). Removed: the artifact-api union member on MetadataPluginOptions. artifactSource, _loadFromArtifactApi, its environmentId pre-flight guard, and _fetchJson's token parameter. The three bootstrap dispatch sites collapse to the single local-file path, behind a new loud guard: a still-configured artifact-api source (reachable via JS or any-typed config) throws at start() with a migration pointer, because the old fall-through would have treated "unsupported source" as "no source" — under eager that silently scans the filesystem instead of loading the artifact the caller named. Tests pin the rejection in artifact-only and eager, and pin the migration target (local-file fetching an http(s) URL and registering the envelope) so the path the error message names stays real. Docs (implementation-status, metadata-service, package ROADMAP, the spec bootstrap comment) now describe the single local-file source, ending the docs-audit loop #4246 was filed to stop. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FC9p9G2sHw1Tbf1wvLJhVK --- ...data-artifact-api-declared-not-reserved.md | 41 ---- .../metadata-remove-artifact-api-source.md | 68 ++++++ .../docs/protocol/kernel/metadata-service.mdx | 40 ++-- .../docs/releases/implementation-status.mdx | 4 +- packages/metadata/ROADMAP.md | 10 +- packages/metadata/src/metadata.test.ts | 214 ++++++------------ packages/metadata/src/plugin.ts | 133 ++++------- .../spec/src/kernel/metadata-plugin.zod.ts | 3 +- 8 files changed, 209 insertions(+), 304 deletions(-) delete mode 100644 .changeset/metadata-artifact-api-declared-not-reserved.md create mode 100644 .changeset/metadata-remove-artifact-api-source.md diff --git a/.changeset/metadata-artifact-api-declared-not-reserved.md b/.changeset/metadata-artifact-api-declared-not-reserved.md deleted file mode 100644 index 4db365ca89..0000000000 --- a/.changeset/metadata-artifact-api-declared-not-reserved.md +++ /dev/null @@ -1,41 +0,0 @@ ---- -"@objectstack/metadata": patch ---- - -fix(metadata): the `artifact-api` source is declared as what it is — implemented (#4246) - -`MetadataPluginOptions.artifactSource` carried a type comment saying "Only -`local-file` is implemented now; `artifact-api` is reserved for M3/M4". It had -outlived its subject: `_loadFromArtifactApi` ships, and all three bootstrap -branches (`eager` / `lazy` / `artifact-only`) dispatch to it. A caller reading -the comment would conclude `mode: 'artifact-api'` was inert; supplying it plus -`environmentId` boots the server off a real network fetch. - -This is Prime Directive #10's `declared ≠ enforced` running backwards — the -declaration understates the runtime instead of overstating it — and it is not -self-correcting: `implementation-status.mdx` copied the claim, so every docs -accuracy audit rediscovers the contradiction and can only re-file it (which is -how this issue was born). The comment, the docs bullet, and the package ROADMAP -now describe the shipped behaviour. - -**The test that was supposed to hold the line was passing for the wrong reason.** -`artifact-only bootstrap rejects the not-yet-implemented artifact-api source` -asserted `rejects.toThrow(/artifact-api/)`. The only throw on that path is the -missing-`environmentId` pre-flight guard, whose message merely *contains* the -string `artifact-api` — so the assertion matched while proving nothing about -implementation status. It is replaced by a suite that pins what the loader does: -URL construction from both accepted input shapes, `?commit=` pinning, the Bearer -header, dispatch from each of the three bootstrap modes, a loud failure on a -non-OK response, and the `environmentId` guard asserted on *its own* message. - -**One real defect fell out of the audit.** The URL builder chose between "append -the canonical path" and "use the URL as-is" by testing for a -`/api/v{n}/cloud/projects/` segment — a path the v5.0 `project → environment` -rename deleted. The guard had therefore been unmatchable since v5.0: an -already-resolved artifact URL got the canonical path appended a *second* time -(`…/artifact/api/v1/cloud/environments/…/artifact`) and 404'd, so half of this -option's documented input shape was dead. The check is now "does the path -already name an artifact endpoint", which restores the intended dual form and -keeps the `unlisted`-visibility public route (`/pub/v1/environments/:id/artifact`) -addressable. Callers passing a control-plane base URL — the common case, and the -only one that worked — are unaffected. diff --git a/.changeset/metadata-remove-artifact-api-source.md b/.changeset/metadata-remove-artifact-api-source.md new file mode 100644 index 0000000000..b9c8e2eb4d --- /dev/null +++ b/.changeset/metadata-remove-artifact-api-source.md @@ -0,0 +1,68 @@ +--- +"@objectstack/metadata": major +--- + +refactor(metadata)!: remove the `artifact-api` artifact source (#4246) + +`MetadataPluginOptions.artifactSource` loses its `artifact-api` union member; +`{ mode: 'local-file', path }` is now the single artifact source. The +`_loadFromArtifactApi` loader, its `environmentId` pre-flight guard, and the +Bearer-token support in `_fetchJson` go with it. + +**Why removal, not the doc fix this branch first carried.** #4246 found the +declaration and the implementation contradicting each other — the option's +comment called `artifact-api` "reserved for M3/M4" while the loader shipped and +all three bootstrap modes dispatched to it — and asked the owner to pick a +direction. Auditing both repos to answer that settled it: + +- **Zero consumers anywhere.** No `mode: 'artifact-api'` call site exists in + this repo or in cloud. The two real "pull an artifact from the cloud" paths + both bypass it: the cloud runtime uses its own `ArtifactApiClient` (TTL + cache, singleflight, hostname resolution, runtime config injection — a + superset this option was never going to grow into), and package distribution + into a running OSS instance goes through `@objectstack/cloud-connection` + (`os package install`, ADR-0008). +- **Half its input contract had been dead since v5.0 with no one noticing.** + The URL builder decided "append the canonical path vs use as-is" by testing + for an `/api/v{n}/cloud/projects/` segment that the v5.0 + `project → environment` rename deleted, so every already-resolved URL got + the path appended a second time and 404'd. A year of silence on a bug like + that is consumer-count evidence of its own. +- **Its one non-replaceable capability was declined.** A Bearer-authenticated + pull of a *private* environment artifact is the single thing `local-file` + cannot do (`local-file` URLs fetch verbatim, unauthenticated). The owner + confirmed that sealed-private-artifact deployments are not a supported need + right now, which removed the last reason to keep the mode. + +**Migration.** Public or commit-pinned artifacts load through the existing +`local-file` URL form, which every bootstrap mode already honors: + +```ts +artifactSource: { + mode: 'local-file', + path: 'https://cloud.example.com/pub/v1/environments/env_42/artifact?commit=cmt_1a2b', +} +``` + +(`private` environments still serve exact-commit deep links through the same +`/pub` route; fully private pulls have no replacement — by decision, not +oversight.) For installing packages into a running runtime, use +`os package install` / `@objectstack/cloud-connection`. + +**The removal is loud, not silent.** A still-configured `artifact-api` source +(reachable from JS or `any`-typed config now that the TS union is +single-member) throws at `start()` with the migration pointer above. This +guard exists because the dispatch's old fall-through would have treated +"unsupported source" as "no source" — under `eager` that silently scans the +filesystem instead of loading the artifact the caller named. Tests pin the +rejection in `artifact-only` and `eager`, and pin the migration target +(`local-file` fetching an http(s) URL and registering the envelope) so the +path the error message points at stays real. + +Also replaces a test that passed for the wrong reason: "artifact-only +bootstrap rejects the not-yet-implemented artifact-api source" matched +`/artifact-api/` against the missing-`environmentId` guard's message — which +merely contained the string — proving nothing about implementation status. +The doc comment, `implementation-status.mdx`, `metadata-service.mdx`, and the +package ROADMAP now all describe the single `local-file` source, ending the +docs-audit loop #4246 was filed to stop. diff --git a/content/docs/protocol/kernel/metadata-service.mdx b/content/docs/protocol/kernel/metadata-service.mdx index 68add6ad16..363ed27432 100644 --- a/content/docs/protocol/kernel/metadata-service.mdx +++ b/content/docs/protocol/kernel/metadata-service.mdx @@ -187,7 +187,7 @@ Four orthogonal levers shape how `MetadataManager` and `MetadataPlugin` behave a | :--- | :--- | :--- | | `eager` (default) | Local dev, traditional servers | Scans filesystem (or hydrates from `artifactSource`) at boot. | | `lazy` | Cold-start sensitive runtimes, on-demand workloads | Skips the FS prime; reads flow through registered loaders (incl. DatabaseLoader cache). Honors `artifactSource` when set. | -| `artifact-only` | Edge / serverless / read-only production | Refuses to touch the filesystem. Requires `artifactSource` (`mode: 'local-file'` or `'artifact-api'`). | +| `artifact-only` | Edge / serverless / read-only production | Refuses to touch the filesystem. Requires `artifactSource` (`mode: 'local-file'`; `path` may be an `http(s)` URL). | ```typescript new MetadataPlugin({ @@ -196,34 +196,36 @@ new MetadataPlugin({ }); ``` -#### Artifact sources +#### Artifact source -Both `artifactSource` modes are implemented, and every bootstrap mode above -dispatches to them: - -| Mode | Reads from | Notes | -| :--- | :--- | :--- | -| `local-file` | `path` — a filesystem path, or an `http(s)` URL fetched verbatim | The `os dev` / `os serve` path. Only this mode gets the artifact-file HMR watcher (`artifactWatch`). Outside `artifact-only`, an absent local file is "nothing compiled yet", not a fault. | -| `artifact-api` | The control plane's `GET /api/v1/cloud/environments/:environmentId/artifact` | Requires `environmentId` on the plugin options; `start()` throws when it is missing. `token` is sent as `Authorization: Bearer`, `commitId` pins a published revision via `?commit=`. | +`artifactSource: { mode: 'local-file', path, fetchTimeoutMs? }` is the single +artifact source, honored by every bootstrap mode above. `path` is a filesystem +path or an `http(s)` URL fetched verbatim — the control plane's public +artifact route (`/pub/v1/environments/:id/artifact[?commit=]`) serves +exactly such URLs, so a sealed runtime can boot straight off a published +revision: ```typescript new MetadataPlugin({ config: { bootstrap: 'artifact-only' }, - environmentId: 'env_42', artifactSource: { - mode: 'artifact-api', - url: 'https://cloud.example.com', // control-plane base — canonical path appended - token: process.env.OS_CLOUD_TOKEN, - commitId: 'cmt_1a2b', // optional: pin a published revision + mode: 'local-file', + path: 'https://cloud.example.com/pub/v1/environments/env_42/artifact?commit=cmt_1a2b', }, }); ``` -`url` may instead be an already-resolved endpoint — any URL whose path ends in -`/artifact` is used verbatim, which is how the `unlisted`-visibility public -route (`/pub/v1/environments/:id/artifact`) is reached. Both modes honor -`fetchTimeoutMs` (and `OS_ARTIFACT_FETCH_TIMEOUT_MS`, default 60 s) for remote -reads. +Only non-URL paths get the artifact-file HMR watcher (`artifactWatch`); remote +reads honor `fetchTimeoutMs` (and `OS_ARTIFACT_FETCH_TIMEOUT_MS`, default +60 s). Outside `artifact-only`, an absent local file is "nothing compiled +yet", not a fault. + +A second `artifact-api` mode (a Bearer-authenticated control-plane pull) was +removed while resolving #4246: it had no consumers — the cloud runtime uses +its own `ArtifactApiClient`, and package distribution into a running OSS +instance goes through `@objectstack/cloud-connection` (`os package install`). +A still-configured `artifact-api` source fails loudly at `start()` rather +than silently falling back to the filesystem scan. ### 2. Persistence Write Gates diff --git a/content/docs/releases/implementation-status.mdx b/content/docs/releases/implementation-status.mdx index d95b45396a..f1d48997d6 100644 --- a/content/docs/releases/implementation-status.mdx +++ b/content/docs/releases/implementation-status.mdx @@ -72,8 +72,8 @@ This matrix is generated from actual codebase analysis and represents the curren - Metadata is loaded from config files or `dist/objectstack.json` at startup and held in memory - `MetadataPlugin` auto-provisions the `sys_metadata` / `sys_metadata_history` storage objects (plus the ADR-0067 commit log and the metadata audit trail) by default, so `PUT /api/v1/meta/{type}/{name}` always has somewhere to write. Per-project (cloud) kernels opt out with `registerSystemObjects: false`, because there the control plane owns those tables - `MetadataPlugin` does **not** auto-bridge ObjectQL to a `DatabaseLoader` — there is no automatic runtime project-DB bridge, and nothing in the packages calls the bridge during boot. Database-backed metadata is an explicit `MetadataManager` opt-in (`setDataEngine()` / `setDatabaseDriver()`, or a `datasource` + `driver` pair on the manager config), and even then it is control-plane only: both methods return early whenever an `environmentId` is supplied, so a per-project kernel never gets a database-backed metadata loader -- Both artifact sources ship. `artifactSource: { mode: 'local-file', path }` reads a compiled artifact (a filesystem path, or an `http(s)` URL fetched verbatim); `artifactSource: { mode: 'artifact-api', url, token?, commitId? }` pulls the published artifact from the control plane (`GET /api/v1/cloud/environments/:environmentId/artifact`). The API source additionally requires `environmentId` on the plugin — it throws at `start()` when that is missing — and is dispatched from all three bootstrap modes (`eager` / `lazy` / `artifact-only`). This page used to call the API loader "Future / reserved", faithfully copying a type comment that outlived the implementation it described (#4246) -- Future: durable local artifact cache — the API source re-fetches on every boot +- The artifact source is `artifactSource: { mode: 'local-file', path }` — a compiled artifact read from a filesystem path or fetched from an `http(s)` URL (the control plane's public `/pub/v1/environments/:id/artifact[?commit=…]` route serves exactly such URLs), honored by all three bootstrap modes (`eager` / `lazy` / `artifact-only`). A second `artifact-api` mode (a Bearer-authenticated control-plane pull) this page once called "Future / reserved" was **removed** while resolving #4246: it had zero consumers in any repo — the cloud runtime uses its own `ArtifactApiClient`, and package installs into a running OSS instance go through `@objectstack/cloud-connection` — and a still-configured `artifact-api` source now fails loudly at `start()` instead of lingering half-declared +- Future: durable local artifact cache — remote artifact URLs re-fetch on every boot ### System Services diff --git a/packages/metadata/ROADMAP.md b/packages/metadata/ROADMAP.md index c5e5ad985a..4fb4a9ecb4 100644 --- a/packages/metadata/ROADMAP.md +++ b/packages/metadata/ROADMAP.md @@ -31,7 +31,7 @@ | **Bootstrap modes** | `MetadataPluginConfig.bootstrap` = `eager` \| `lazy` \| `artifact-only` — supports edge / serverless / read-only deployments. | | **Persistence write gates** | `MetadataManagerConfig.persistence.{ writable, overlayWritable }` — runtime freeze for sealed kernels. | | **Single-source schema discipline** | Canonical `MetadataManagerConfigSchema` / `MetadataFallbackStrategySchema` live in `kernel/metadata-loader.zod.ts` and are re-exported from `system/metadata-persistence.zod.ts`. | -| **`artifact-api` runtime source** | `MetadataPlugin` can boot from a remote control-plane artifact (`artifactSource: { mode: 'artifact-api', url, token?, commitId? }` plus `environmentId` on the plugin options — the v5.0 rename retired `projectId`). Wired across `eager` / `lazy` / `artifact-only` bootstrap modes. `url` may be the control-plane base (canonical path appended) or an already-resolved `…/artifact` endpoint. Configurable timeout via `fetchTimeoutMs` or `OS_ARTIFACT_FETCH_TIMEOUT_MS` (default 60 s). | +| **Remote artifact boot** | `MetadataPlugin` boots from a compiled artifact via `artifactSource: { mode: 'local-file', path }`, where `path` may be an `http(s)` URL — e.g. the control plane's public `/pub/v1/environments/:id/artifact[?commit=…]` route. Wired across `eager` / `lazy` / `artifact-only` bootstrap modes. Configurable timeout via `fetchTimeoutMs` or `OS_ARTIFACT_FETCH_TIMEOUT_MS` (default 60 s). A dedicated `artifact-api` mode (Bearer-authenticated control-plane pull) was removed in #4246 — zero consumers in any repo; the cloud runtime uses its own `ArtifactApiClient`. | ### 🟡 Partially Implemented @@ -193,9 +193,11 @@ artifact persistence (`artifacts/${projectId}/${commitId}.json`). - [x] Object-storage abstraction available via `StorageServicePlugin` - [x] Cloud `artifact-api` reads/writes through `IStorageService` -- [x] `MetadataPlugin` consumes published artifacts via the - `artifactSource: { mode: 'artifact-api' }` source — no direct S3 - coupling needed in the metadata layer. +- [x] `MetadataPlugin` consumes published artifacts via + `artifactSource: { mode: 'local-file', path: '' }` — no + direct S3 coupling needed in the metadata layer. (The dedicated + `artifact-api` mode was removed in #4246 — zero consumers; the cloud + runtime pulls artifacts through its own `ArtifactApiClient` instead.) - See [Publish, Versioning & Preview](../../content/docs/deployment/publish-and-preview.mdx). --- diff --git a/packages/metadata/src/metadata.test.ts b/packages/metadata/src/metadata.test.ts index 5afca0d573..0b46a64e09 100644 --- a/packages/metadata/src/metadata.test.ts +++ b/packages/metadata/src/metadata.test.ts @@ -1,6 +1,6 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -839,22 +839,22 @@ describe('MetadataPlugin', () => { expect(manager.loadMany).not.toHaveBeenCalled(); }); - // #4246 — `artifact-api` IS implemented: `_loadFromArtifactApi` ships and all - // three bootstrap branches dispatch to it. The test that stood here — - // "artifact-only bootstrap rejects the not-yet-implemented artifact-api - // source" — passed for the wrong reason: the only throw on that path is the - // missing-`environmentId` guard, whose message happens to contain the string - // "artifact-api", so `rejects.toThrow(/artifact-api/)` matched while - // asserting nothing about implementation status. Supply `environmentId` and - // the plugin really does go to the network. The suite below pins what the - // loader actually does instead. - it('artifact-api requires environmentId — and fails for THAT reason, not as unimplemented', async () => { + // #4246 resolution — the `artifact-api` source is REMOVED, not reserved. + // The mode shipped through v17 with zero consumers in any repo (the cloud + // runtime uses its own ArtifactApiClient; package distribution into a + // running OSS instance goes through @objectstack/cloud-connection), so the + // enforce-or-remove call went to remove. What these tests pin is the + // failure MODE of the removal: a still-configured 'artifact-api' source — + // reachable only from JS callers or config plumbed through `any`, since + // the TS union is now single-member — must fail loudly at start(), never + // degrade into "no source". + it('artifact-only bootstrap rejects the removed artifact-api source with a migration message', async () => { const { MetadataPlugin } = await import('./plugin.js'); const plugin = new MetadataPlugin({ rootDir: '/tmp/test', watch: false, config: { bootstrap: 'artifact-only' }, - artifactSource: { mode: 'artifact-api', url: 'https://example.com' }, + artifactSource: { mode: 'artifact-api', url: 'https://example.com' } as any, }); const manager = (plugin as any).manager; @@ -866,169 +866,85 @@ describe('MetadataPlugin', () => { const ctx = createMockPluginContext(); try { await plugin.init(ctx); - await expect(plugin.start(ctx)).rejects.toThrow(/requires options\.environmentId/); - // The guard is a pre-flight check, not a "mode unavailable" refusal — - // nothing was fetched, and nothing was scanned. + await expect(plugin.start(ctx)).rejects.toThrow(/'artifact-api' source was removed/); + // Rejected before any load path was chosen: no fetch, no FS scan. expect(fetchMock).not.toHaveBeenCalled(); expect(manager.loadMany).not.toHaveBeenCalled(); } finally { globalThis.fetch = realFetch; } }); - }); - // ---------- artifact-api source (#4246) ---------- - // - // The loader was reachable and functional long before this suite existed; the - // type comment on `MetadataPluginOptions.artifactSource` just claimed the - // opposite ("reserved for M3/M4"), and `implementation-status.mdx` copied the - // claim. These tests are the enforcement side of the corrected declaration. - describe('artifact-api source', () => { - const PROBE_OBJECT = { name: 'artifact_probe', label: 'Artifact Probe', fields: {} }; - const ARTIFACT_ENVELOPE = { - schemaVersion: '0.1', - environmentId: 'env_42', - commitId: 'cmt_1', - checksum: 'a'.repeat(64), - metadata: { objects: [PROBE_OBJECT] }, - }; + // The dangerous degradation the guard exists to prevent: under `eager`, + // treating "unsupported source" as "no source" would fall through to the + // filesystem scan and boot with whatever happens to be on disk instead of + // the artifact the caller named. + it('eager bootstrap rejects the removed mode instead of silently falling back to the filesystem scan', async () => { + const { MetadataPlugin } = await import('./plugin.js'); + const plugin = new MetadataPlugin({ + rootDir: '/tmp/test', + watch: false, + artifactSource: { mode: 'artifact-api', url: 'https://example.com' } as any, + }); + + const manager = (plugin as any).manager; + manager.loadMany = vi.fn().mockResolvedValue([]); - let fetchMock: ReturnType; - let realFetch: typeof globalThis.fetch; + const ctx = createMockPluginContext(); + await plugin.init(ctx); + await expect(plugin.start(ctx)).rejects.toThrow(/not supported/); + expect(manager.loadMany).not.toHaveBeenCalled(); + }); - beforeEach(() => { - realFetch = globalThis.fetch; - fetchMock = vi.fn().mockResolvedValue({ + // The migration target named by the rejection message, pinned so it stays + // real: `local-file` with an http(s) URL fetches verbatim and registers + // the envelope-wrapped artifact — the shape the control plane's public + // /pub/v1/environments/:id/artifact route serves. + it('local-file accepts an http(s) URL and registers the fetched artifact envelope', async () => { + const { MetadataPlugin } = await import('./plugin.js'); + const url = 'https://cloud.example.com/pub/v1/environments/env_42/artifact?commit=cmt_1'; + const envelope = { + schemaVersion: '0.1', + environmentId: 'env_42', + commitId: 'cmt_1', + checksum: 'a'.repeat(64), + metadata: { objects: [{ name: 'artifact_probe', label: 'Artifact Probe', fields: {} }] }, + }; + const fetchMock = vi.fn().mockResolvedValue({ ok: true, status: 200, statusText: 'OK', - text: async () => JSON.stringify(ARTIFACT_ENVELOPE), + text: async () => JSON.stringify(envelope), }); + const realFetch = globalThis.fetch; globalThis.fetch = fetchMock as any; - }); - - afterEach(() => { - globalThis.fetch = realFetch; - }); - async function startWith( - artifactSource: any, - config?: { bootstrap?: 'eager' | 'lazy' | 'artifact-only' }, - ) { - const { MetadataPlugin } = await import('./plugin.js'); const plugin = new MetadataPlugin({ rootDir: '/tmp/test', watch: false, environmentId: 'env_42', - ...(config ? { config } : {}), - artifactSource, - }); - (plugin as any).manager.loadMany = vi.fn().mockResolvedValue([]); - const ctx = createMockPluginContext(); - await plugin.init(ctx); - await plugin.start(ctx); - return plugin; - } - - /** The single fetch the loader issued: [url, init]. */ - function fetchedCall() { - expect(fetchMock).toHaveBeenCalledTimes(1); - return fetchMock.mock.calls[0] as [string, { headers: Record }]; - } - - it('appends the canonical control-plane path to a base URL and registers the artifact', async () => { - const plugin = await startWith( - { mode: 'artifact-api', url: 'https://cloud.example.com/' }, - { bootstrap: 'artifact-only' }, - ); - - const [url] = fetchedCall(); - expect(url).toBe('https://cloud.example.com/api/v1/cloud/environments/env_42/artifact'); - // …and the payload really lands in the registry — the mode is not a stub. - expect((plugin as any).manager.register).toHaveBeenCalledWith( - 'object', - 'artifact_probe', - expect.objectContaining({ name: 'artifact_probe' }), - { notify: false }, - ); - }); - - // Regression for the dead guard found while resolving #4246: it tested for a - // `/api/v{n}/cloud/projects/` segment that the v5.0 `project → environment` - // rename deleted, so it never matched and an already-resolved URL had the - // canonical path appended a SECOND time. - it('uses an already-resolved artifact URL verbatim instead of appending the path twice', async () => { - await startWith({ - mode: 'artifact-api', - url: 'https://cloud.example.com/api/v1/cloud/environments/env_42/artifact', - }); - - const [url] = fetchedCall(); - expect(url).toBe('https://cloud.example.com/api/v1/cloud/environments/env_42/artifact'); - expect(url).not.toMatch(/artifact\/api/); - }); - - // Same rule keeps the `unlisted`-visibility public route addressable. - it('honors a non-canonical artifact endpoint (the /pub route)', async () => { - await startWith({ - mode: 'artifact-api', - url: 'https://cloud.example.com/pub/v1/environments/env_42/artifact', - }); - - expect(fetchedCall()[0]).toBe('https://cloud.example.com/pub/v1/environments/env_42/artifact'); - }); - - it('pins a published revision with ?commit= and sends the token as a Bearer header', async () => { - await startWith({ - mode: 'artifact-api', - url: 'https://cloud.example.com', - commitId: 'cmt_9/1', - token: 'tok_secret', + config: { bootstrap: 'artifact-only' }, + artifactSource: { mode: 'local-file', path: url }, }); + const manager = (plugin as any).manager; + manager.loadMany = vi.fn().mockResolvedValue([]); - const [url, init] = fetchedCall(); - expect(url).toBe( - 'https://cloud.example.com/api/v1/cloud/environments/env_42/artifact?commit=cmt_9%2F1', - ); - expect(init.headers.Authorization).toBe('Bearer tok_secret'); - }); - - // The three dispatch sites in `start()` — the reason the "reserved" comment - // was actively misleading rather than merely stale. - it.each(['eager', 'lazy', 'artifact-only'] as const)( - '%s bootstrap loads through the artifact API when the source is set', - async (bootstrap) => { - const plugin = await startWith( - { mode: 'artifact-api', url: 'https://cloud.example.com' }, - { bootstrap }, - ); - - expect(fetchedCall()[0]).toBe( - 'https://cloud.example.com/api/v1/cloud/environments/env_42/artifact', - ); - expect((plugin as any).manager.register).toHaveBeenCalledWith( + const ctx = createMockPluginContext(); + try { + await plugin.init(ctx); + await plugin.start(ctx); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(fetchMock.mock.calls[0][0]).toBe(url); + expect(manager.register).toHaveBeenCalledWith( 'object', 'artifact_probe', expect.objectContaining({ name: 'artifact_probe' }), { notify: false }, ); - // eager must not ALSO fall back to the filesystem scan - expect((plugin as any).manager.loadMany).not.toHaveBeenCalled(); - }, - ); - - it('fails loudly when the control plane rejects the request', async () => { - fetchMock.mockResolvedValue({ ok: false, status: 404, statusText: 'Not Found', text: async () => '' }); - const { MetadataPlugin } = await import('./plugin.js'); - const plugin = new MetadataPlugin({ - rootDir: '/tmp/test', - watch: false, - environmentId: 'env_42', - artifactSource: { mode: 'artifact-api', url: 'https://cloud.example.com' }, - }); - const ctx = createMockPluginContext(); - await plugin.init(ctx); - await expect(plugin.start(ctx)).rejects.toThrow(/Cannot load artifact from API .*HTTP 404/); + expect(manager.loadMany).not.toHaveBeenCalled(); + } finally { + globalThis.fetch = realFetch; + } }); }); }); diff --git a/packages/metadata/src/plugin.ts b/packages/metadata/src/plugin.ts index 898524f462..d5cfdc548d 100644 --- a/packages/metadata/src/plugin.ts +++ b/packages/metadata/src/plugin.ts @@ -141,37 +141,33 @@ export interface MetadataPluginOptions { organizationId?: string; /** * Environment ID used by local artifact envelopes and metadata-scoped - * consumers, and REQUIRED by `artifactSource.mode: 'artifact-api'` — it - * addresses the control-plane endpoint. (The v5.0 rename retired the - * "project ID" wording this comment used to carry; see ADR-0006.) + * consumers. (The v5.0 rename retired the "project ID" wording this + * comment used to carry; see ADR-0006.) */ environmentId?: string; /** - * When set, MetadataPlugin loads metadata from an artifact instead of scanning - * the filesystem. **Both modes are implemented** and are dispatched from all - * three bootstrap modes (`eager` / `lazy` / `artifact-only`) — see `start()`. + * When set, MetadataPlugin loads metadata from a compiled artifact instead + * of scanning the filesystem, honored by all three bootstrap modes + * (`eager` / `lazy` / `artifact-only`) — see `start()`. * - * - `local-file` — read `path` (a filesystem path, or an `http(s)://` URL - * fetched verbatim). The dev/`os serve` path; also the one mode the - * artifact-file HMR watcher ({@link artifactWatch}) applies to. - * - `artifact-api` — pull the published artifact from the cloud control - * plane (`GET /api/v1/cloud/environments/:environmentId/artifact`, the - * envelope declared by `EnvironmentArtifactSchema`). REQUIRES - * {@link environmentId}; `start()` throws when it is missing rather than - * fetching a URL it cannot address. `url` is the control-plane base URL - * (the canonical path is appended) or an already-resolved artifact - * endpoint — see `_loadFromArtifactApi`. `token` becomes a Bearer header, - * `commitId` pins a published revision, and both modes honor - * `fetchTimeoutMs` / `OS_ARTIFACT_FETCH_TIMEOUT_MS`. + * `path` is a filesystem path, or an `http(s)://` URL fetched verbatim — + * the control plane's public artifact route + * (`/pub/v1/environments/:id/artifact[?commit=…]`) serves exactly such + * URLs, so a sealed runtime can boot straight off a published revision. + * Remote reads honor `fetchTimeoutMs` / `OS_ARTIFACT_FETCH_TIMEOUT_MS`; + * the artifact-file HMR watcher ({@link artifactWatch}) applies to + * non-URL paths only. * - * Keep this describing what `start()` actually does: it used to call - * `artifact-api` "reserved for M3/M4" long after the loader and all three - * dispatch sites shipped, which read as "this mode is inert" to callers and - * propagated into the docs (#4246). + * `local-file` is the only mode. A second `artifact-api` mode (a + * Bearer-authenticated control-plane pull) existed here through v17 with + * zero consumers in any repo — the cloud runtime uses its own + * `ArtifactApiClient`, and package distribution into a running OSS + * instance goes through `@objectstack/cloud-connection` — and was removed + * when #4246 forced the declared-vs-enforced question. `start()` rejects + * an unknown mode loudly rather than silently scanning the filesystem + * instead. */ - artifactSource?: - | { mode: 'local-file'; path: string; fetchTimeoutMs?: number } - | { mode: 'artifact-api'; url: string; token?: string; commitId?: string; fetchTimeoutMs?: number }; + artifactSource?: { mode: 'local-file'; path: string; fetchTimeoutMs?: number }; /** * Register the `sys_metadata` + `sys_metadata_history` storage objects * on this kernel. Default `true` for backward compatibility. @@ -305,15 +301,33 @@ export class MetadataPlugin implements Plugin { artifactSource: src?.mode ?? 'none', }); + // Reject a non-`local-file` source before choosing any load path. The + // union is single-member so TypeScript callers can't get here, but JS + // callers and config plumbed through `any` can — and the fall-through + // below would otherwise treat "unsupported source" as "no source" + // (eager would silently scan the filesystem instead of loading the + // artifact the caller named). The removed `artifact-api` mode gets a + // pointed migration message (#4246). + if (src && (src as { mode: string }).mode !== 'local-file') { + const bad = (src as { mode: string }).mode; + throw new Error( + `[MetadataPlugin] artifactSource.mode '${bad}' is not supported` + + (bad === 'artifact-api' + ? " — the 'artifact-api' source was removed (#4246). Load the same artifact with" + + " { mode: 'local-file', path: '' } (e.g. the control plane's" + + " /pub/v1/environments/:id/artifact route), or install packages into a running" + + ' runtime via @objectstack/cloud-connection.' + : ". The only artifact source is { mode: 'local-file', path }."), + ); + } + if (mode === 'artifact-only') { // Sealed-runtime mode: ONLY load from a pre-compiled artifact. Never // touch the filesystem. Required for Edge / serverless / read-only // production deployments where the running process must not depend // on local source files. - if (src?.mode === 'local-file') { + if (src) { await this._loadFromLocalFile(ctx, src.path, src.fetchTimeoutMs); - } else if (src?.mode === 'artifact-api') { - await this._loadFromArtifactApi(ctx, src); } else { throw new Error('[MetadataPlugin] bootstrap=artifact-only requires options.artifactSource to be set'); } @@ -323,19 +337,15 @@ export class MetadataPlugin implements Plugin { // the DatabaseLoader read-through cache and any registered loaders. // An artifact source, if present, is still honored so projects can // pin a known set of metadata at boot without paying the FS scan. - if (src?.mode === 'local-file') { + if (src) { await this._loadFromLocalFile(ctx, src.path, src.fetchTimeoutMs, { optional: true }); - } else if (src?.mode === 'artifact-api') { - await this._loadFromArtifactApi(ctx, src); } else { ctx.logger.info('[MetadataPlugin] lazy bootstrap — skipping filesystem priming; metadata loads on demand'); } } else { // 'eager' (default): preserve historical behavior. - if (src?.mode === 'local-file') { + if (src) { await this._loadFromLocalFile(ctx, src.path, src.fetchTimeoutMs, { optional: true }); - } else if (src?.mode === 'artifact-api') { - await this._loadFromArtifactApi(ctx, src); } else { await this._loadFromFileSystem(ctx); } @@ -510,7 +520,7 @@ export class MetadataPlugin implements Plugin { /** * Fetch JSON content from a URL with configurable timeout. */ - private async _fetchJson(url: string, fetchTimeoutMs?: number, token?: string): Promise { + private async _fetchJson(url: string, fetchTimeoutMs?: number): Promise { const envTimeout = Number(process.env.OS_ARTIFACT_FETCH_TIMEOUT_MS); const timeoutMs = fetchTimeoutMs ?? (Number.isFinite(envTimeout) && envTimeout > 0 ? envTimeout : undefined) @@ -519,7 +529,6 @@ export class MetadataPlugin implements Plugin { const timer = timeoutMs > 0 ? setTimeout(() => controller.abort(), timeoutMs) : undefined; try { const headers: Record = { Accept: 'application/json, */*;q=0.5' }; - if (token) headers.Authorization = `Bearer ${token}`; const res = await fetch(url, { redirect: 'follow', signal: controller.signal, headers }); if (!res.ok) throw new Error(`HTTP ${res.status} ${res.statusText}`); const content = await res.text(); @@ -740,58 +749,6 @@ export class MetadataPlugin implements Plugin { await this._parseAndRegisterArtifact(ctx, raw, filePath); } - /** - * Load metadata from the cloud control plane's artifact endpoint. Shipped - * and reachable from every bootstrap mode — see `MetadataPluginOptions. - * artifactSource` for the option contract and #4246 for why that doc - * comment used to claim otherwise. - * - * `src.url` accepts two forms, distinguished deterministically by whether - * the path already names an artifact endpoint: - * - **control-plane base** (`https://cloud.example.com`) — the canonical - * `/api/v1/cloud/environments/:environmentId/artifact` path is appended. - * - **already-resolved endpoint** (any URL whose path ends in `/artifact`) - * — used verbatim, so the `unlisted`-visibility public route - * (`/pub/v1/environments/:id/artifact`) is reachable too. - */ - private async _loadFromArtifactApi( - ctx: PluginContext, - src: { url: string; token?: string; commitId?: string; fetchTimeoutMs?: number }, - ): Promise { - const environmentId = this.options.environmentId; - if (!environmentId) { - throw new Error('[MetadataPlugin] artifact-api source requires options.environmentId to be set'); - } - - // Build the artifact URL: - // ${url}/api/v1/cloud/environments/${environmentId}/artifact[?commit=${commitId}] - let artifactUrl = src.url.replace(/\/+$/, ''); - // A URL that already names an artifact endpoint is used as-is; anything - // else is a control-plane base and gets the canonical path appended. - // This guard used to test for `/api/v{n}/cloud/projects/` — a segment the - // v5.0 `project → environment` rename deleted, leaving it unmatchable. So - // every already-resolved URL silently had the canonical path appended a - // SECOND time (`…/artifact/api/v1/cloud/environments/…/artifact`) and - // 404'd: half of this option's documented input shape was dead (#4246). - if (!/\/artifact(?:$|\?)/i.test(artifactUrl)) { - artifactUrl = `${artifactUrl}/api/v1/cloud/environments/${environmentId}/artifact`; - } - if (src.commitId) { - artifactUrl += `${artifactUrl.includes('?') ? '&' : '?'}commit=${encodeURIComponent(src.commitId)}`; - } - - ctx.logger.info('[MetadataPlugin] Loading metadata from artifact API', { url: artifactUrl }); - - let raw: unknown; - try { - raw = await this._fetchJson(artifactUrl, src.fetchTimeoutMs, src.token); - } catch (e: any) { - throw new Error(`[MetadataPlugin] Cannot load artifact from API "${artifactUrl}": ${e.message}`); - } - - await this._parseAndRegisterArtifact(ctx, raw, artifactUrl); - } - private async _loadFromFileSystem(ctx: PluginContext): Promise { ctx.logger.info('Loading metadata from file system...'); diff --git a/packages/spec/src/kernel/metadata-plugin.zod.ts b/packages/spec/src/kernel/metadata-plugin.zod.ts index e85fdd9ce0..cce0f3c65f 100644 --- a/packages/spec/src/kernel/metadata-plugin.zod.ts +++ b/packages/spec/src/kernel/metadata-plugin.zod.ts @@ -499,7 +499,8 @@ export const MetadataPluginConfigSchema = lazySchema(() => z.object({ * is queried per request. * * - `artifact-only` — Load exclusively from the artifact source configured - * on the plugin (`artifactSource: { mode: 'local-file' | 'artifact-api' }`). + * on the plugin (`artifactSource: { mode: 'local-file' }`; the `path` may + * be a filesystem path or an `http(s)` artifact URL). * The filesystem is not scanned, and the database loader is not consulted * during bootstrap. Required for Edge / serverless / immutable-image * deployments where the running process must not perform write-back or