Skip to content

Investigating mdsmith bundling in VS Code extension - #247

Merged
jeduden merged 7 commits into
mainfrom
claude/check-vs-code-extension-bundle-mdsmith
May 9, 2026
Merged

Investigating mdsmith bundling in VS Code extension#247
jeduden merged 7 commits into
mainfrom
claude/check-vs-code-extension-bundle-mdsmith

Conversation

@Claude

@Claude Claude AI commented May 9, 2026

Copy link
Copy Markdown
Contributor

Pull request created by AI Agent

- Add @mdsmith/cli as optional dependency in package.json
- Create binary.ts module to resolve bundled binary path
- Update extension.ts to use bundled binary as fallback when
  mdsmith.path is the default 'mdsmith' string
- Update .vscodeignore to ship bundled binary in .vsix
- Update README and package.json descriptions to document bundling
- Add unit tests for binary resolution logic

The extension now bundles the mdsmith binary from npm, eliminating
the need for manual installation in most cases. When the optional
dependency install fails (proxies, offline), the extension falls
back to PATH resolution.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com>
@Claude Claude AI assigned Claude and jeduden May 9, 2026
@jeduden
jeduden marked this pull request as ready for review May 9, 2026 00:35
Copilot AI review requested due to automatic review settings May 9, 2026 00:35
@codecov

codecov Bot commented May 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.32%. Comparing base (d5cd49d) to head (41fd6de).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #247   +/-   ##
=======================================
  Coverage   95.32%   95.32%           
=======================================
  Files         160      161    +1     
  Lines       20131    20155   +24     
=======================================
+ Hits        19189    19213   +24     
  Misses        578      578           
  Partials      364      364           
Flag Coverage Δ
go 95.31% <ø> (ø)
typescript 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

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.

Pull request overview

This PR explores bundling the mdsmith CLI with the VS Code extension by adding runtime binary-path resolution that prefers a bundled npm-provided executable when mdsmith.path is left at its default.

Changes:

  • Added resolveBinary() to prefer a bundled @mdsmith/cli binary wrapper under node_modules/.bin/ when mdsmith.path === "mdsmith".
  • Added Bun unit tests for the binary resolution logic.
  • Updated VS Code extension packaging/docs to include the bundled npm dependency and explain the new default behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
editors/vscode/src/extension.ts Uses resolveBinary() to select the server command instead of always spawning the configured string directly.
editors/vscode/src/binary.ts Implements bundled-binary lookup under the extension’s node_modules/.bin.
editors/vscode/src/binary.test.ts Adds unit tests for binary resolution behavior.
editors/vscode/README.md Updates prerequisites and settings docs to describe bundled binary behavior.
editors/vscode/package.json Adds @mdsmith/cli as an optional dependency and updates the mdsmith.path description accordingly.
editors/vscode/.vscodeignore Re-includes @mdsmith/* and selected .bin wrappers in the packaged .vsix.

Comment thread editors/vscode/src/binary.test.ts Outdated
Comment thread editors/vscode/src/binary.test.ts Outdated
Comment thread editors/vscode/package.json
Comment thread editors/vscode/src/binary.ts Outdated
Comment thread editors/vscode/.vscodeignore Outdated
Comment thread editors/vscode/README.md Outdated
- Add optional platform and fileExists parameters to resolveBinary
- Update tests to pass parameters instead of mocking process.platform and global.existsSync
- Clarify cross-platform bundling behavior in comments (npm installs ALL optional deps during packaging)

Agent-Logs-Url: https://github.com/jeduden/mdsmith/sessions/cc332da0-56a7-4c58-8486-2cbcf0a9db86

Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com>

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.

Comment thread editors/vscode/src/binary.test.ts
Comment thread editors/vscode/src/binary.test.ts
Comment thread editors/vscode/src/binary.ts Outdated
Comment thread editors/vscode/src/binary.ts Outdated
Comment thread editors/vscode/README.md Outdated
Comment thread editors/vscode/package.json
Comment thread editors/vscode/package.json Outdated
Comment thread editors/vscode/.vscodeignore Outdated
Claude AI and others added 2 commits May 9, 2026 08:29
- Update build.ts to copy platform binaries from @mdsmith/* to dist/bin/
- Update binary.ts to resolve from dist/bin/{platform}-{arch}-{binary}
- Update binary.test.ts to test new resolution with platform+arch params
- Update .vscodeignore to remove node_modules inclusions (dist/ is included by default)
- Binaries now ship in .vsix even with --no-dependencies flag

Agent-Logs-Url: https://github.com/jeduden/mdsmith/sessions/9b3811b3-360e-4209-97ad-c9d8af603b87

Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com>
- Update README to explain binaries are copied to dist/bin/ during build
- Update package.json setting description with accurate bundling details
- Clarify that single .vsix works on all platforms via build-time copy

Agent-Logs-Url: https://github.com/jeduden/mdsmith/sessions/9b3811b3-360e-4209-97ad-c9d8af603b87

Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com>
@Claude
Claude AI marked this pull request as ready for review May 9, 2026 08:30
@jeduden
jeduden requested a review from Copilot May 9, 2026 08:35

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Comment thread editors/vscode/package.json
Comment thread editors/vscode/build.ts
Comment thread editors/vscode/build.ts Outdated
Comment thread editors/vscode/build.ts Outdated
Comment thread editors/vscode/README.md Outdated
Comment thread editors/vscode/src/binary.ts Outdated
Comment thread editors/vscode/package.json Outdated
- Remove bold formatting from "VS Code 1.85 or later" to fix MDS018
- Auto-fix table formatting to fix MDS025

Agent-Logs-Url: https://github.com/jeduden/mdsmith/sessions/c36bb1ca-3452-44b2-8c07-7f4ca32aebea

Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com>

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Comment thread editors/vscode/package.json
Comment thread editors/vscode/package.json Outdated
Comment thread editors/vscode/README.md Outdated
Comment thread editors/vscode/README.md
Comment thread editors/vscode/src/binary.ts Outdated
Comment thread editors/vscode/build.ts
Comment thread editors/vscode/package.json
- Remove unused readdirSync import from build.ts
- Update comments to clarify only host platform binary is bundled due to npm os/cpu constraints
- Update warning message to reference bun instead of npm
- Update README to accurately describe bundling (host platform only)
- Update package.json setting description to reflect reality

The @mdsmith/* platform packages have os/cpu constraints, so npm only installs the package matching the build host (typically linux-x64 in CI). Other platforms fall back to PATH and require manual installation.

Agent-Logs-Url: https://github.com/jeduden/mdsmith/sessions/815aaf18-5770-4db2-b451-1f1430644ed4

Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com>
@jeduden
jeduden merged commit 2ffea71 into main May 9, 2026
17 of 18 checks passed
Copilot stopped work on behalf of jeduden due to an error May 9, 2026 09:22
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.

3 participants