From 6c82ff20ebd4a71af74dbfc048111d666718ec1b Mon Sep 17 00:00:00 2001 From: Nasyarobby Putra Date: Tue, 8 Sep 2026 22:00:26 +0700 Subject: [PATCH] fix(plugins): enhance plugin installation and script handling - Updated the `pnpm install` command in `plugin-install.js` to include the `--ignore-workspace` flag, ensuring proper installation of plugins within the app tree. - Improved the `instantiateScriptSource` function in `script-sandbox.js` to resolve `pluginDir` more effectively, allowing for better package management. - Added a new smoke test in `plugins-smoke.js` to validate the ability to require additional packages from plugin directories, enhancing testing coverage for plugin functionality. - Updated documentation in `AGENTS.md` to reflect changes in the installation command. --- AGENTS.md | 2 +- packages/server/plugin-install.js | 11 +++++- packages/server/script-sandbox.js | 9 ++++- packages/server/test/plugins-smoke.js | 49 +++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3002e5e..c9c22aa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -78,7 +78,7 @@ Copy the layout from `plugins/joplin-api`, `plugins/send-sms`, or `examples/plug Add `dependencies` only if the script `require()`s extra npm packages. Then: ```bash -pnpm install --dir plugins/ --ignore-scripts --prefer-offline +pnpm install --dir plugins/ --ignore-scripts --prefer-offline --ignore-workspace ``` `plugins/*/node_modules/` is gitignored. Host-allowlisted modules (`axios`, `jsonata`, …) come from the server; extra deps resolve from the plugin directory. diff --git a/packages/server/plugin-install.js b/packages/server/plugin-install.js index 2a49484..2c11d42 100644 --- a/packages/server/plugin-install.js +++ b/packages/server/plugin-install.js @@ -51,9 +51,18 @@ export async function pnpmInstallPlugin(dir) { } if (!hasDeps) return { skipped: true }; + // --ignore-workspace: plugins live under the app tree but are not workspace + // packages; without this, pnpm install --dir can no-op against the root monorepo. await execFileAsync( "pnpm", - ["install", "--dir", dir, "--ignore-scripts", "--prefer-offline"], + [ + "install", + "--dir", + dir, + "--ignore-scripts", + "--prefer-offline", + "--ignore-workspace", + ], { cwd: dir, env: { ...process.env, CI: "1" }, diff --git a/packages/server/script-sandbox.js b/packages/server/script-sandbox.js index b8af026..a45d2f7 100644 --- a/packages/server/script-sandbox.js +++ b/packages/server/script-sandbox.js @@ -557,16 +557,23 @@ function instantiateCompiled( * $workflows?: { trigger: (name: string, data?: unknown) => Promise }, * pluginDir?: string | null, * }} [opts] + * + * When `pluginDir` is omitted, it is resolved from `script` so inspect/dry-run + * of `plugin/` can `require()` extra packages from that plugin's node_modules. */ export function instantiateScriptSource(script, source, opts = {}) { const compiled = compileScriptSource(source, script); + const pluginDir = + "pluginDir" in opts + ? opts.pluginDir ?? null + : (resolveScriptRef(script).pluginDir ?? null); const fn = instantiateCompiled(compiled, { log: opts.log ?? inspectLog, script, workflowName: opts.workflowName ?? "inspect", owner: opts.owner ?? DEFAULT_OWNER, $workflows: opts.$workflows, - pluginDir: opts.pluginDir ?? null, + pluginDir, }); return { fn, ...extractScriptMeta(fn) }; } diff --git a/packages/server/test/plugins-smoke.js b/packages/server/test/plugins-smoke.js index 674ce88..b6e472a 100644 --- a/packages/server/test/plugins-smoke.js +++ b/packages/server/test/plugins-smoke.js @@ -9,6 +9,7 @@ */ import assert from "node:assert/strict"; import fs from "fs"; +import path from "node:path"; import { migrate, db } from "../db.js"; import { getAppVersion, satisfiesRange } from "../app-version.js"; import { @@ -18,6 +19,7 @@ import { uninstallPlugin, listInstalledPlugins, createBlankPlugin, + pluginDir, } from "../plugin-store.js"; import { installExamplePlugin } from "../plugin-install.js"; import { @@ -119,11 +121,58 @@ async function main() { ); assert.ok(meta); + const extra = createBlankPlugin( + "extra-require-smoke", + `const extra = require("smoke-extra"); +async function main() { + return { output: { n: extra.n } }; +} +main.meta = { + description: "smoke extra require", + config: {}, + input: {}, + output: { n: { type: "number" } }, + example: { data: {}, config: {} }, +}; +export default main; +`, + ); + assert.equal(extra.scriptRef, "plugin/extra-require-smoke"); + const extraPkg = path.join( + pluginDir("extra-require-smoke"), + "node_modules", + "smoke-extra", + ); + fs.mkdirSync(extraPkg, { recursive: true }); + fs.writeFileSync( + path.join(extraPkg, "package.json"), + `${JSON.stringify({ name: "smoke-extra", main: "index.js" })}\n`, + ); + fs.writeFileSync(path.join(extraPkg, "index.js"), "module.exports = { n: 9 };\n"); + clearScriptCache(); + const extraSource = fs.readFileSync( + path.join(pluginDir("extra-require-smoke"), "script.js"), + "utf8", + ); + const extraInspect = inspectScriptSource( + "plugin/extra-require-smoke", + extraSource, + ); + assert.equal(extraInspect.metaError, null, extraInspect.metaError); + assert.equal(extraInspect.meta?.description, "smoke extra require"); + const extraRun = await runScript( + "plugin/extra-require-smoke", + { data: null, context: {}, config: null }, + { log: silent, workflowName: "smoke", owner: "default" }, + ); + assert.equal(extraRun.output.n, 9); + assert.ok(listInstalledPlugins().some((p) => p.id === "get-current-time")); uninstallPlugin("jsonata-smoke-fork"); uninstallPlugin("blank-smoke"); uninstallPlugin("blank-smoke-copy"); + uninstallPlugin("extra-require-smoke"); uninstallPlugin("get-current-time"); console.log("plugins-smoke: ok");