|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * A RELATIVE `plugins: [...]` entry is refused at load, naming the two |
| 5 | + * spellings that work (#10944). |
| 6 | + * |
| 7 | + * ## The defect, measured before the fix |
| 8 | + * |
| 9 | + * `plugins: [...]` in the served app's own `objectstack.config.ts` is THE |
| 10 | + * documented way to extend a deployment. A string entry that is not a bare |
| 11 | + * package name was handed straight to `import()` — and ESM resolves a relative |
| 12 | + * specifier against the file CONTAINING the call, which is |
| 13 | + * `@objectstack/cli/dist/commands/serve.js`. The served app's root never |
| 14 | + * entered the resolution at all. |
| 15 | + * |
| 16 | + * Measured on `origin/main` from a fixture app that really did carry |
| 17 | + * `local-plugin.js` next to its `package.json`: |
| 18 | + * |
| 19 | + * './local-plugin.js' -> Cannot find module '<cli>/src/commands/local-plugin.js' |
| 20 | + * imported from '<cli>/src/commands/serve.ts' |
| 21 | + * '../local-plugin.js' -> Cannot find module '<cli>/src/local-plugin.js' |
| 22 | + * '..' -> LOADED — this package's own command barrel |
| 23 | + * (CompileCommand, ValidateCommand, ServeCommand, …) |
| 24 | + * |
| 25 | + * The app's own file was never seen in any of them, and the last row is the |
| 26 | + * whole point stated as a positive: a relative entry CAN load a module — it can |
| 27 | + * only ever load one belonging to the CLI. The boot loop then catches the |
| 28 | + * failure, prints one red line naming a path inside the CLI's install |
| 29 | + * directory, and serves the app WITHOUT the plugin, so the deployment looks |
| 30 | + * healthy while quietly missing the extension its config declared. |
| 31 | + * |
| 32 | + * ## Why refusal and not app-root resolution |
| 33 | + * |
| 34 | + * Ruled at triage on #10944. Refusing expands no accepted set — the spelling |
| 35 | + * has never loaded an app's file — and converts a diagnostic about the CLI's |
| 36 | + * internals into an answer the author can act on. Resolving relative entries |
| 37 | + * against the served app's root is a capability addition with no measured pull |
| 38 | + * (nothing in `content/docs`, `examples/` or the test suite writes the |
| 39 | + * spelling); it stays a maintainer decision, and this refusal is where such a |
| 40 | + * request would come from. |
| 41 | + * |
| 42 | + * ## What the tests below are shaped to catch |
| 43 | + * |
| 44 | + * ⚠️ A suite that only asserts "the relative entry is refused" passes just as |
| 45 | + * green on an implementation that refuses `plugins: [...]` ENTIRELY. So the |
| 46 | + * headline test asserts the refusal and BOTH working spellings in one body — |
| 47 | + * remove the narrowing from `Serve.isRelativePluginSpecifier` and that test |
| 48 | + * reddens on the loads, not on the refusal. |
| 49 | + */ |
| 50 | + |
| 51 | +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; |
| 52 | +import { tmpdir } from 'node:os'; |
| 53 | +import { join } from 'node:path'; |
| 54 | +import { pathToFileURL } from 'node:url'; |
| 55 | +import { afterAll, describe, expect, it } from 'vitest'; |
| 56 | +import Serve from './serve.js'; |
| 57 | + |
| 58 | +const roots: string[] = []; |
| 59 | +afterAll(() => { |
| 60 | + for (const r of roots) rmSync(r, { recursive: true, force: true }); |
| 61 | +}); |
| 62 | + |
| 63 | +/** A name no workspace package can satisfy, so nothing passes by accident. */ |
| 64 | +const APP_ONLY = '@os-fixture/relative-refusal-probe'; |
| 65 | + |
| 66 | +/** |
| 67 | + * An app root that carries `local-plugin.js` NEXT TO its `package.json` — the |
| 68 | + * exact layout an author writing `plugins: ['./local-plugin.js']` has in mind — |
| 69 | + * and, when asked, declares and installs `APP_ONLY` as well. |
| 70 | + */ |
| 71 | +function makeApp(opts: { declare: boolean } = { declare: false }): string { |
| 72 | + const root = mkdtempSync(join(tmpdir(), 'os-10944-')); |
| 73 | + roots.push(root); |
| 74 | + writeFileSync( |
| 75 | + join(root, 'package.json'), |
| 76 | + JSON.stringify({ |
| 77 | + name: 'fixture-app', |
| 78 | + version: '1.0.0', |
| 79 | + type: 'module', |
| 80 | + ...(opts.declare ? { dependencies: { [APP_ONLY]: '1.0.0' } } : {}), |
| 81 | + }), |
| 82 | + ); |
| 83 | + writeFileSync(join(root, 'local-plugin.js'), 'export default { name: "app-local-plugin" };\n'); |
| 84 | + if (opts.declare) { |
| 85 | + const dir = join(root, 'node_modules', ...APP_ONLY.split('/')); |
| 86 | + mkdirSync(dir, { recursive: true }); |
| 87 | + writeFileSync( |
| 88 | + join(dir, 'package.json'), |
| 89 | + JSON.stringify({ name: APP_ONLY, version: '1.0.0', type: 'module', main: 'index.js' }), |
| 90 | + ); |
| 91 | + writeFileSync(join(dir, 'index.js'), 'export default { name: "declared-package" };\n'); |
| 92 | + } |
| 93 | + return root; |
| 94 | +} |
| 95 | + |
| 96 | +describe('os serve → a relative `plugins: [...]` entry is refused (#10944)', () => { |
| 97 | + it('refuses the relative entry AND keeps both working spellings loading — one body, on purpose', async () => { |
| 98 | + const root = makeApp({ declare: true }); |
| 99 | + const absolute = join(root, 'local-plugin.js'); |
| 100 | + |
| 101 | + // (1) The shape the card was filed against. The file really is there, in |
| 102 | + // the app root, and it is still refused — because the app root was never |
| 103 | + // the base and this spelling cannot be made to mean it. |
| 104 | + const err = await Serve.importConfigPlugin('./local-plugin.js', root).catch( |
| 105 | + (e: unknown) => e as Error, |
| 106 | + ); |
| 107 | + expect(err).toBeInstanceOf(Error); |
| 108 | + expect(err.message).toBe(Serve.relativePluginSpecifierRefusal('./local-plugin.js')); |
| 109 | + |
| 110 | + // (2) A DECLARED bare package name — spelling (a) the refusal names. |
| 111 | + const declared: any = await Serve.importConfigPlugin(APP_ONLY, root); |
| 112 | + expect(declared.default).toEqual({ name: 'declared-package' }); |
| 113 | + |
| 114 | + // (3) An absolute path and a `file://` URL — spelling (b) the refusal |
| 115 | + // names. Both are base-independent, and both load the app's OWN file: the |
| 116 | + // very module (1) refused to reach by the relative spelling. |
| 117 | + for (const spelling of [absolute, pathToFileURL(absolute).href]) { |
| 118 | + const mod: any = await Serve.importConfigPlugin(spelling, root); |
| 119 | + expect(mod.default, spelling).toEqual({ name: 'app-local-plugin' }); |
| 120 | + } |
| 121 | + }); |
| 122 | + |
| 123 | + it('names spelling (a) — a bare package name the app DECLARES — in the message', () => { |
| 124 | + const message = Serve.relativePluginSpecifierRefusal('./local-plugin.js'); |
| 125 | + expect(message).toContain('DECLARES in its own package.json'); |
| 126 | + expect(message).toContain("plugins: ['@mycompany/crm']"); |
| 127 | + }); |
| 128 | + |
| 129 | + it('names spelling (b) — an absolute path or `file://` URL — in the message', () => { |
| 130 | + const message = Serve.relativePluginSpecifierRefusal('./local-plugin.js'); |
| 131 | + expect(message).toContain('absolute path'); |
| 132 | + expect(message).toContain('file://'); |
| 133 | + // The form an author writing an app-local plugin file actually wants: the |
| 134 | + // CONFIG computes the absolute URL from its own location. Echoing the |
| 135 | + // author's own specifier makes the line copy-pasteable. |
| 136 | + expect(message).toContain("new URL('./local-plugin.js', import.meta.url).href"); |
| 137 | + }); |
| 138 | + |
| 139 | + it('says WHY, and does not promise app-root resolution is coming', () => { |
| 140 | + const message = Serve.relativePluginSpecifierRefusal('./local-plugin.js'); |
| 141 | + // The consequence, concretely — this is the half that lets an author stop |
| 142 | + // hunting for a missing file. |
| 143 | + expect(message).toMatch(/CLI's own installation directory/); |
| 144 | + expect(message).toMatch(/never against your app/); |
| 145 | + // …and it does not claim an import was attempted, because none was. |
| 146 | + expect(message).not.toContain('Failed to import plugin'); |
| 147 | + // Option A stays a maintainer decision; the message must not pre-announce |
| 148 | + // it (triage on #10944 is explicit about this). |
| 149 | + expect(message).not.toMatch(/coming soon|will be supported|in a future release/i); |
| 150 | + }); |
| 151 | + |
| 152 | + it('classifies every specifier shape that reaches this line — measured, not assumed', async () => { |
| 153 | + // ⚠️ REFUSED and ALLOWED below are the boundary of this card. Each entry |
| 154 | + // was run through `Serve.importConfigPlugin` on `origin/main` first; the |
| 155 | + // ALLOWED ones all resolve to the SAME module regardless of which file |
| 156 | + // imports them, which is exactly why they are none of this rule's business. |
| 157 | + const REFUSED = [ |
| 158 | + './local-plugin.js', // the filed shape |
| 159 | + '../local-plugin.js', // climbing out of the CLI's commands dir |
| 160 | + './nested/plugin.js', |
| 161 | + '.', // resolves to the CLI's own commands directory |
| 162 | + '..', // MEASURED: loaded the CLI's own command barrel |
| 163 | + '.\\local-plugin.js', // Node's URL resolution normalises `\` to `/` |
| 164 | + '..\\local-plugin.js', |
| 165 | + './', |
| 166 | + ]; |
| 167 | + const ALLOWED = [ |
| 168 | + '/abs/local-plugin.js', // absolute POSIX path |
| 169 | + 'file:///abs/local-plugin.js', // file:// URL |
| 170 | + 'C:\\app\\local-plugin.js', // Windows drive path — absolute |
| 171 | + 'C:/app/local-plugin.js', |
| 172 | + 'node:path', // builtin; MEASURED: loads |
| 173 | + 'data:text/javascript,export default 1', // MEASURED: loads |
| 174 | + '@mycompany/crm', // scoped bare package name |
| 175 | + 'chalk', // unscoped bare package name |
| 176 | + // NOT relative under ESM: a bare specifier, even one that looks like a |
| 177 | + // filename. It keeps the #4719 "declare it in that app's package.json" |
| 178 | + // answer, which is the right one for a bare name — deliberately left |
| 179 | + // alone rather than folded into this refusal. |
| 180 | + 'local-plugin.js', |
| 181 | + '.hidden-not-relative', // `.` not followed by a separator |
| 182 | + ]; |
| 183 | + |
| 184 | + for (const s of REFUSED) { |
| 185 | + expect(Serve.isRelativePluginSpecifier(s), `REFUSED: ${JSON.stringify(s)}`).toBe(true); |
| 186 | + } |
| 187 | + for (const s of ALLOWED) { |
| 188 | + expect(Serve.isRelativePluginSpecifier(s), `ALLOWED: ${JSON.stringify(s)}`).toBe(false); |
| 189 | + } |
| 190 | + |
| 191 | + // The predicate is what the loader consults, so prove the two agree rather |
| 192 | + // than trusting the wiring: every REFUSED shape throws the refusal text, |
| 193 | + // and no ALLOWED shape ever does. |
| 194 | + const root = makeApp(); |
| 195 | + for (const s of REFUSED) { |
| 196 | + const err = await Serve.importConfigPlugin(s, root).catch((e: unknown) => e as Error); |
| 197 | + expect(err, s).toBeInstanceOf(Error); |
| 198 | + expect(err.message, s).toBe(Serve.relativePluginSpecifierRefusal(s)); |
| 199 | + } |
| 200 | + for (const s of ALLOWED) { |
| 201 | + // Most of these cannot resolve from a temp fixture, and that is fine — |
| 202 | + // what must never happen is the REFUSAL text. |
| 203 | + const outcome = await Serve.importConfigPlugin(s, root).catch((e: unknown) => e as Error); |
| 204 | + const message = outcome instanceof Error ? outcome.message : ''; |
| 205 | + expect(message, s).not.toContain('Refused the plugin entry'); |
| 206 | + } |
| 207 | + }); |
| 208 | +}); |
| 209 | + |
| 210 | +describe('os serve → the refusal is wired into the load, not just exported (#10944)', () => { |
| 211 | + it('the boot loop still routes every string entry through the refusing helper', async () => { |
| 212 | + const { readFileSync } = await import('node:fs'); |
| 213 | + const source = readFileSync(new URL('./serve.ts', import.meta.url), 'utf8'); |
| 214 | + // Without this the behavioural tests above could all pass while the boot |
| 215 | + // loop bare-imported the entry and never reached the helper. |
| 216 | + expect(source).toContain('await Serve.importConfigPlugin(plugin, hostRoot)'); |
| 217 | + const helper = source.slice(source.indexOf('static async importConfigPlugin')); |
| 218 | + expect(helper).toContain('Serve.isRelativePluginSpecifier(pluginSpecifier)'); |
| 219 | + expect(helper).toContain('Serve.relativePluginSpecifierRefusal(pluginSpecifier)'); |
| 220 | + }); |
| 221 | +}); |
0 commit comments