Skip to content

fix(plugins): bundle official plugins#1627

Closed
zerob13 wants to merge 1 commit into
devfrom
codex/feishu-plugin-bundling
Closed

fix(plugins): bundle official plugins#1627
zerob13 wants to merge 1 commit into
devfrom
codex/feishu-plugin-bundling

Conversation

@zerob13
Copy link
Copy Markdown
Collaborator

@zerob13 zerob13 commented May 15, 2026

Summary

  • add generic official plugin ensure/bundle/verify script that scans plugin manifests
  • bundle Feishu on supported platforms and embed official plugin packages for all Electron targets
  • simplify CI to call generic bundle/verify steps instead of plugin-specific checks

Tests

  • node scripts/package-plugin.mjs --validate --release-version-from-root --target-platform darwin plugins/feishu
  • node scripts/package-plugin.mjs --validate --release-version-from-root --target-platform linux plugins/feishu
  • node scripts/package-plugin.mjs --validate --release-version-from-root --target-platform win32 plugins/feishu
  • node scripts/ensure-official-plugins.mjs --clean --target-platform linux --target-arch x64 --out /tmp/deepchat-official-bundle-test
  • node scripts/ensure-official-plugins.mjs --verify --target-platform linux --target-arch x64 --plugin-root /tmp/deepchat-official-bundle-test
  • pnpm exec vitest --config vitest.config.ts test/main/presenter/pluginPresenter.test.ts
  • pnpm run format
  • pnpm run i18n
  • pnpm run lint
  • pnpm run format:check

Summary by CodeRabbit

Release Notes

  • New Features

    • Official plugins (including Feishu/Lark) are now automatically bundled with the application across all platforms (Windows, Linux, macOS).
  • Chores

    • Improved internal plugin packaging and verification processes for official releases.
    • Updated development workflow to streamline plugin bundling during setup.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

📝 Walkthrough

Walkthrough

This PR introduces a reusable "official plugins" bundling infrastructure that generalizes plugin packaging beyond CUA to support Feishu/Lark plugins across Windows, Linux, and macOS. A new discovery and bundling script orchestrates manifest scanning, artifact naming, packaging, and verification; package.json and CI workflows wire this infrastructure into development and release pipelines; and updated documentation and tests validate the behavior.

Changes

Official Plugins Bundling System

Layer / File(s) Summary
Planning, specification, and bundling strategy
docs/issues/feishu-plugin-bundling/spec.md, docs/issues/feishu-plugin-bundling/plan.md, docs/issues/feishu-plugin-bundling/tasks.md
Specification, implementation plan, and task checklist documenting the Feishu plugin bundling user story, acceptance criteria, design approach, compatibility guarantees, and test coverage.
Official plugins bundling and verification script
scripts/ensure-official-plugins.mjs
Node.js ES module that discovers official plugins from manifests with source.type: deepchat-official, computes expected artifact names per platform/arch, bundles missing packages via external hooks, and verifies artifacts exist in workflows. Supports --clean, plugin filtering, and --verify mode.
Package.json script wiring for dev and builds
package.json
Development scripts now run plugin:official:ensure before starting electron-vite; macOS/Linux build scripts replaced with plugin:official:bundle invocations; new plugin:official:* scripts (ensure, bundle, package, verify) replace prior CUA bundling commands.
Electron Builder plugin packaging configuration
electron-builder.yml
Consolidated extraResources mapping ensures bundled .dcplugin files from build/bundled-plugins/ are packaged into app.asar.unpacked/plugins; removed duplicate prior mapping.
CI and release workflow bundling and verification
.github/workflows/build.yml, .github/workflows/release.yml
Windows, Linux, and macOS build and release workflows now invoke plugin:official:bundle before electron-builder and plugin:official:verify after build; replaces prior CUA-specific bundling and version-based verification steps with platform-agnostic approach.
Plugin packaging developer and user documentation
docs/guides/plugin-packaging.md
Guide updated with "Official Plugin Artifacts" section explaining discovery from manifests, per-platform bundling via plugin:official:bundle and plugin:official:package scripts, CI embedding path under app.asar.unpacked/plugins/, and expected Feishu artifact names for all target platforms/architectures.
Plugin Presenter test coverage for packaged plugins
test/main/presenter/pluginPresenter.test.ts
Added createPackagedFeishuFixture() helper that runs packaging script to produce Feishu .dcplugin artifacts; new test validates packaged Feishu plugin activation, MCP server materialization, and skill registration; wiring test asserts consistency across package.json, workflows, electron-builder, and guide docs around official plugins behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • ThinkInAIXYZ/deepchat#1613: Updates to PluginPresenter and tests for resolving/opening plugin settings and initializing Feishu MCP runtime align with this PR's new packaged Feishu plugin test coverage and settings materialization validation.

Poem

🐰 Official plugins bundled neat,
Platform-agnostic, pure, discreet!
Feishu flows where CUA once stood,
Build and verify working good.
Fresh manifests dance through the night—
Bundling logic: structured and right! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: implementing official plugin bundling across the codebase, including scripts, workflows, and configuration.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/feishu-plugin-bundling

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/ensure-official-plugins.mjs`:
- Around line 45-47: The CLI currently assumes a value follows the --out flag
and resolves argv[index + 1] even when missing, which can make --clean
dangerous; update the argument parsing around arg/argv/index so that when arg
=== '--out' you first validate argv[index + 1] exists, is not another flag
(doesn't start with '-') and is non-empty before calling path.resolve(rootDir,
...), otherwise exit with a clear error and non-zero status; additionally,
before performing the clean operation that uses args.outDir (the --clean logic),
validate that args.outDir is a subpath of rootDir and not equal to rootDir (or
outside it) and refuse to run clean if that check fails to prevent recursive
deletion of the workspace.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8d6d3556-7f44-4968-8d04-5ed6321f4f86

📥 Commits

Reviewing files that changed from the base of the PR and between b298666 and ab354c1.

📒 Files selected for processing (10)
  • .github/workflows/build.yml
  • .github/workflows/release.yml
  • docs/guides/plugin-packaging.md
  • docs/issues/feishu-plugin-bundling/plan.md
  • docs/issues/feishu-plugin-bundling/spec.md
  • docs/issues/feishu-plugin-bundling/tasks.md
  • electron-builder.yml
  • package.json
  • scripts/ensure-official-plugins.mjs
  • test/main/presenter/pluginPresenter.test.ts

Comment on lines +45 to +47
if (arg === '--out') {
args.outDir = path.resolve(rootDir, argv[index + 1] || '')
index += 1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Validate required flag values before resolving paths and cleaning.

Line 46 currently treats a missing --out value as '', which resolves to the repo root; with --clean (Line 224) this can recursively delete the workspace. Please fail fast on missing values and block unsafe clean targets.

Suggested fix
+function readRequiredValue(argv, index, flag) {
+  const value = argv[index + 1]
+  if (!value || value.startsWith('--')) {
+    throw new Error(`${flag} requires a value`)
+  }
+  return value
+}
+
 function parseArgs(argv) {
@@
     if (arg === '--out') {
-      args.outDir = path.resolve(rootDir, argv[index + 1] || '')
+      const outValue = readRequiredValue(argv, index, '--out')
+      args.outDir = path.resolve(rootDir, outValue)
       index += 1
       continue
     }
@@
     if (arg === '--plugin-root') {
-      args.pluginRoot = path.resolve(rootDir, argv[index + 1] || '')
+      const pluginRootValue = readRequiredValue(argv, index, '--plugin-root')
+      args.pluginRoot = path.resolve(rootDir, pluginRootValue)
       index += 1
       continue
     }
@@
   if (args.verify) {
     verifyPlugins(args)
   } else {
     if (args.clean) {
+      const resolvedOutDir = path.resolve(args.outDir)
+      const fsRoot = path.parse(resolvedOutDir).root
+      if (resolvedOutDir === rootDir || resolvedOutDir === fsRoot) {
+        throw new Error(`Refusing to clean unsafe output directory: ${resolvedOutDir}`)
+      }
       fs.rmSync(args.outDir, { recursive: true, force: true })
     }

Also applies to: 55-57, 223-225

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/ensure-official-plugins.mjs` around lines 45 - 47, The CLI currently
assumes a value follows the --out flag and resolves argv[index + 1] even when
missing, which can make --clean dangerous; update the argument parsing around
arg/argv/index so that when arg === '--out' you first validate argv[index + 1]
exists, is not another flag (doesn't start with '-') and is non-empty before
calling path.resolve(rootDir, ...), otherwise exit with a clear error and
non-zero status; additionally, before performing the clean operation that uses
args.outDir (the --clean logic), validate that args.outDir is a subpath of
rootDir and not equal to rootDir (or outside it) and refuse to run clean if that
check fails to prevent recursive deletion of the workspace.

@zerob13 zerob13 closed this May 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant