Add crossplug: DSH <-> mcode bidirectional plugin converter - #15
Fectivnfy112357 wants to merge 3 commits into
Conversation
Agent-plugin contribution for the community registry: plugins/Fectivnfy112357/crossplug/
…cts, add tests Address the three review points: 1. Remove the installer: delete core/install.js and the install command from run.js (community policy forbids installers that write into user homes). The hosted plugin now only converts (writes --out) and lists (read-only). 2. Fix disclosure: README and the skill now document every subcommand and side effect. The package runs no subprocesses (dropped npm root -g), writes only inside --out, and bundles no installer. 3. Add automated tests: core/run.test.mjs runs with npm test (node --test) and covers input path boundaries, output overwrite, symlink/path traversal, generated manifest/MCP loadability, and failure safety, plus bidirectional real-load e2e (mcode discovery + tool call, DSH cordis load) that auto-skip when the runtime is absent.
|
您好，这是之前 #7 的重新提交。之前那个 PR 是我疏忽直接开在了 main 分支上，不符合社区规范，非常抱歉给您添麻烦了，这次改到了 feat/crossplug 分支重开。 按您的意见改了三点：
您之前的意见原文：
麻烦您有空再帮忙看看，谢谢。 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53e0df57fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const rewritten = rewriteVendorSource(vendorSource, vendorFileName, warnings); | ||
| fs.writeFileSync(path.join(vendorDir, vendorFileName), rewritten, 'utf8'); |
There was a problem hiding this comment.
When the input extension is .ts, this rewrites the original vendorSource and immediately overwrites the stripped .js file written above, so any normal TypeScript annotation remains in vendor/<name>.js. The generated bridge then imports that file with Node as JavaScript and fails at runtime with syntax errors such as Unexpected token ':', making supported TS conversions unusable unless the source happened to contain no TS syntax.
Useful? React with 👍 / 👎.
| // 复制源包同目录的辅助文件，让带相对导入的 extension 包（如 crossplug dsh2mcode 产物： | ||
| // vendor/dsh-plugin.js + typebox-shim.js）转换后自包含。 | ||
| // 不覆盖已生成的 vendorFileName（源 vendor/ 里若有同名文件以转换产物为准）。 | ||
| const srcVendorDir = path.join(srcDir, 'vendor'); |
There was a problem hiding this comment.
When converting a pi extension package whose entry imports same-directory files, e.g. import './helper.js', only the entry file and a vendor/ subdirectory are copied into the output. The generated DSH bridge imports ../vendor/<entry>, but that entry still resolves ./helper.js next to itself and fails with ERR_MODULE_NOT_FOUND, so multi-file extensions lose their tools even though they are a normal package shape.
Useful? React with 👍 / 👎.
| .replace(/^web-?search-/, '') | ||
| .replace(/-web-?search/, '') | ||
| .replace(/-provider$/, '') | ||
| .replace(/[^a-z0-9]+/g, '_') |
There was a problem hiding this comment.
When a DSH source/plugin name contains a hyphen, sanitizeToolName converts it to underscores and that value is reused as the mcpServers key; for example my-tool.js generates mcpServers.my_tool. The repository's MCP validator requires plugin-style names and rejects underscores, so the converted agent-plugins MCP package is invalid for common hyphenated plugin names.
Useful? React with 👍 / 👎.
| const main = path.resolve(dir, pkg.main); | ||
| if (fs.existsSync(main)) return { file: main, pkg }; |
There was a problem hiding this comment.
When converting a directory, a package can set main to a path like ../secret.js; resolving it directly lets the converter read and vendor a file outside the selected input tree. That violates the documented/read boundary for conversion and can leak unrelated local files into the generated output, so the resolved entry should be constrained to stay under the package directory.
Useful? React with 👍 / 👎.
| if (c === '(' || c === '{' || c === '[') { stack.push(c === '(' ? ')' : c === '{' ? '}' : ']'); i++; continue; } | ||
| if (c === ')' || c === '}' || c === ']') { | ||
| const want = stack.pop(); | ||
| if (want === undefined) return -1; | ||
| if (want !== c) return -1; |
There was a problem hiding this comment.
When a registered tool body contains a regex literal with a bracket or parenthesis, such as /\)/, this scanner treats those characters as real delimiters because it has no regex-literal state. matchParen then returns -1, findCalls drops the entire tools.register(...) call, and the converter produces no MCP tools for otherwise valid plugins that use regexes in their implementation.
Useful? React with 👍 / 👎.
| // 跳过作为字符串一部分的匹配：检查前一个非空白字符 | ||
| let k = m.index - 1; | ||
| while (k >= 0 && /\s/.test(source[k])) k--; | ||
| if (k >= 0 && (source[k] === '"' || source[k] === "'" || source[k] === '`')) continue; |
There was a problem hiding this comment.
Because findCalls only checks whether a match is preceded by a quote and does not track whether it is inside a line or block comment, a commented-out tools.register({ name: 'old_tool', ... }) is still extracted. The converter then advertises an MCP tool that the vendor plugin never registers, so tools/list and tools/call disagree for valid sources containing commented examples or disabled tools.
Useful? React with 👍 / 👎.
| if (typeof target === "function") target(mockCtx, {}); | ||
| else if (target && typeof target.apply === "function") target.apply(mockCtx, {}); |
There was a problem hiding this comment.
When a DSH plugin has an async apply that registers tools after awaiting I/O or setup, the generated MCP server starts accepting tools/call requests before the vendor registration has completed. tools/list is populated from static extraction, but dispatch still reads mockTools.registered, so early calls fail with unknown tool or tool not registered by vendor for plugins that initialize asynchronously.
Useful? React with 👍 / 👎.
| const name = propertyValue(args, 'name'); | ||
| const description = propertyValue(args, 'description'); | ||
| const parameters = propertyValue(args, 'parameters'); |
There was a problem hiding this comment.
When a tools.register object puts parameters before name and the parameter schema contains a property named name, propertyValue(args, 'name') matches the nested schema field instead of the tool's top-level name. The converter then treats the tool as missing a string name and emits no MCP package, even though the source registration is valid and simply orders its fields differently.
Useful? React with 👍 / 👎.
| if (name !== TOOLS[0].name) return respondError(id, -32602, "unknown tool: " + name); | ||
| if (!provider) return respondError(id, -32603, "SearXNG provider 未就绪：请设置 SEARXNG_URL（mcp.json env 或系统环境变量）"); | ||
| const signal = new AbortController().signal; | ||
| const result = await provider.search({ query: args.query }, signal); |
There was a problem hiding this comment.
The generated provider MCP tool advertises categories, language, and maxResults, but tools/call discards all of those arguments and passes only query to the provider. Users calling the converted search tool cannot actually limit result count or request language/category filters despite the schema saying those inputs are supported.
Useful? React with 👍 / 👎.
| lines.push(`// 工具执行走 vendor/${vendorFile} 加载 + 模拟 ctx 路径，`); | ||
| lines.push('// 闭包（cfg / recall / formatRecall 等）保留在原 apply() 作用域里，行为真实。'); | ||
| lines.push(''); | ||
| lines.push(`import dshPlugin from "./vendor/${vendorFile}";`); |
There was a problem hiding this comment.
For regular DSH source conversion, an ESM plugin that exports apply as a named function is valid for the generated MCP path, but the generated pi extension.js statically imports a default export from the vendor file. In that scenario the extension fails to import with does not provide an export named 'default', so pi users get no converted tools even though the same vendor module could be loaded via the fallback logic used elsewhere.
Useful? React with 👍 / 👎.
…nd dependency handling - Add relativeImports extraction to handle multi-file plugins with local dependencies - Implement async apply completion waiting to prevent tool call failures during initialization - Fix propertyValue parsing to handle top-level properties correctly and avoid nested conflicts - Update MCP server ID generation to use kebab-case without underscores for validation compliance - Convert vendor imports to dynamic await import for proper ESM module loading - Add process exit handling on stdin close to prevent orphaned MCP servers - Copy relative dependency files recursively to vendor directory for self-contained plugins - Improve regex literal detection in source parsing to avoid false matches in comments - Support different module formats (ESM/CJS) with appropriate file extensions - Add comprehensive test coverage for edge cases and integration scenarios - Fix search provider arguments mapping to include optional parameters like categories and language - Implement process tree killing for Windows compatibility when terminating MCP servers
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
hetaoBackend
left a comment
hetaoBackend
left a comment
There was a problem hiding this comment.
@Fectivnfy112357 当前版本不建议收录，存在已复现的数据边界和默认测试副作用：
- 相对依赖复制只做 lexical path 检查，没有
lstat/realpath边界校验；源树内 symlink 指向树外文件时，实测会把树外内容复制进转换产物。 - provider conversion 自动读取
SEARXNG_URL并明文写进生成的mcp.json，可能泄露 credential 或内部 endpoint。 - 默认 root test 检测到本机 MCode 后会写入
~/.minimax/plugins、运行真实mcode exec，并明确不清理，和 README 的无用户目录修改声明矛盾。 - 派生进程清理不可靠；真实 E2E 应显式 opt-in，并使用隔离 profile/dataDir。
- 多文件 provider 的普通相对 sidecar import 不会复制，生成产物可能启动失败。
请先缩小转换范围，修复 realpath/symlink、secret 落盘和测试隔离问题；所有产物建议 staging 校验后再原子替换。
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| lines.push(''); | ||
| lines.push('| DSH 行 | 转换结果 |'); | ||
| lines.push('| --- | --- |'); | ||
| for (const r of report) lines.push(`| ${r.split(' → ')[0].replace(/\|/g, '\\|')} | ${(r.split(' → ').slice(1).join(' → ') || '').replace(/\|/g, '\\|')} |`); |
| lines.push(''); | ||
| lines.push('| DSH 行 | 转换结果 |'); | ||
| lines.push('| --- | --- |'); | ||
| for (const r of report) lines.push(`| ${r.split(' → ')[0].replace(/\|/g, '\\|')} | ${(r.split(' → ').slice(1).join(' → ') || '').replace(/\|/g, '\\|')} |`); |
|
|
||
| const manifest = JSON.parse(await readFile(path.join(out, 'plugin.json'), 'utf8')); | ||
| assert.match(manifest.name, PLUGIN_NAME); | ||
| assert.ok(manifest.$schema.includes('agent-plugins.org')); |
3 participants
What it solves
crossplug converts plugins bidirectionally between DSH (DeepSeek Harness) and mcode (MiniMax Code / pi), keeping the original plugin logic intact via runtime bridging. Every mapping decision is recorded in
CONVERSION-REPORT.md.Changes since the first review
core/install.jsand theinstallcommand fromcore/run.js— community policy forbids installers. The hosted plugin now only converts (writes--out) and lists (read-only).README.mdand the skill document every subcommand and side effect: no subprocesses, no writes outside--out, no bundled installer.test/run.test.mjsruns withnpm test(node --test) and covers input path boundaries, output overwrite, symlink/path traversal, generated manifest/MCP loadability, and failure safety, plus bidirectional real-load e2e (mcode plugin discovery + tool call, DSH cordis load) that auto-skip when the runtime is absent.Dependencies and platforms
Network and data
list.--out.Test evidence
node --test plugins/Fectivnfy112357/crossplug/test/run.test.mjs— 19 tests; the e2e cases run where mcode/DSH are installed and skip cleanly otherwise.