Skip to content

feat(sdk-typescript): add download methods, remove dead code#32

Open
shwetank-dev wants to merge 10 commits intomainfrom
refactor-typescript-sdk-and-implement-download
Open

feat(sdk-typescript): add download methods, remove dead code#32
shwetank-dev wants to merge 10 commits intomainfrom
refactor-typescript-sdk-and-implement-download

Conversation

@shwetank-dev
Copy link
Collaborator

@shwetank-dev shwetank-dev commented Mar 5, 2026

Summary

Adds three new public methods to MpakClient for downloading bundles and skills with SHA-256 integrity verification. Removes dead code that was not used by the CLI or any external consumer.

New methods

  • downloadContent(url, sha256) — Low-level fetch + SHA-256 verification. Returns a verified Buffer. Shared by both bundle and skill downloads.
  • downloadBundle(name, version?, platform?) — Downloads an MCP bundle by name. Defaults to latest version and auto-detected platform. Returns { bundleRaw, bundleMetadata }.
  • downloadSkillBundle(name, version?) — Downloads a skill bundle by name. Defaults to latest version. Returns { skillRaw, skillMetadata }. Internally uses getSkillVersionDownload with 'latest' to mirror how downloadBundle uses getBundleDownload.

Smoke tests

Added integration smoke tests for downloadBundle and downloadSkillBundle that hit the real registry.mpak.dev API:

  • downloadBundle — downloads @nimblebraininc/echo, verifies buffer, metadata, SHA-256, and size
  • downloadSkillBundle — downloads @nimblebraininc/ipinfo, verifies buffer, metadata, SHA-256, and size

Dead code commented out (with TODO markers)

The removed code (resolveSkillRef, extractSkillFromZip, GitHub/URL resolvers, etc.) was built around two assumptions we've moved away from:

  1. Bundles/skills could come from multiple sources (registry, GitHub, direct URL) — We only want to download from our registry. The SDK doesn't need to resolve from GitHub or arbitrary URLs.
  2. The SDK should peek inside bundle content (extract SKILL.md from ZIPs) — The SDK's job is to download and verify integrity, not inspect contents. What to do with the bytes (extract, write to disk, etc.) is the caller's decision.

Commented-out methods:

  • downloadSkillContent, resolveSkillRef, resolveMpakSkill, resolveGithubSkill, resolveUrlSkill, extractSkillFromZip, verifyIntegrityOrThrow, extractHash

Commented-out types:

  • SkillReference, MpakSkillReference, GithubSkillReference, UrlSkillReference, ResolvedSkill

The jszip dependency is no longer used at runtime.

Follow-up items

  • Merge getSkillDownload and getSkillVersionDownload into a single method mirroring getBundleDownload (blocked by CLI usage)
  • Update README.md with new method documentation
  • Delete commented-out code once approved
  • TODO: Add smoke tests (test:integration) to CI/CD pipeline

Test plan

  • All new methods have unit tests (happy path, integrity errors, 404 propagation, default values)
  • Smoke tests added for downloadBundle and downloadSkillBundle against live registry
  • pnpm typecheck passes across full monorepo (CLI still compiles)
  • pnpm test passes across full monorepo (51 SDK, 105 CLI, 65 registry, 64 schemas, 61 web)
  • SDK build size reduced from 16.83KB to 12.34KB (ESM)

🤖 Generated with Claude Code

shwetank-dev and others added 4 commits March 5, 2026 11:50
Add two new public methods to MpakClient:

- downloadContent(url, sha256): low-level fetch + SHA-256 verification,
  returns a Buffer. Works for any downloadable artifact.
- downloadBundle(name, version?, platform?): high-level method that
  resolves bundle download info via the registry API, then downloads
  and verifies the binary. Defaults to latest version and auto-detected
  platform. Returns { bundleRaw, bundleMetadata }.

Also update computeSha256 to accept Buffer in addition to string.

Includes unit tests for both methods covering happy path, integrity
errors, network errors, and 404 propagation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add downloadSkillBundle(name, version?) to MpakClient, mirroring the
downloadBundle method for consistency. Internally uses
getSkillVersionDownload with 'latest' as default version, matching
how downloadBundle uses getBundleDownload with 'latest'.

Both download methods now follow the same pattern:
- Resolve download info from registry API
- Fetch binary via downloadContent (shared fetch + SHA-256 verify)
- Return { raw, metadata } with verified buffer and artifact metadata

Includes unit tests covering happy path, version defaulting to latest,
404 propagation, and SHA-256 integrity error propagation.

All monorepo tests pass (55 SDK, 105 CLI, 65 registry, 64 schemas, 61 web).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment out the following methods and types that are not used by the
CLI or any external consumer, marked with TODO for removal once approved:

Methods removed from client.ts:
- downloadSkillContent: replaced by downloadContent + downloadSkillBundle
- resolveSkillRef: not used outside SDK
- resolveMpakSkill, resolveGithubSkill, resolveUrlSkill: private helpers for resolveSkillRef
- extractSkillFromZip: peeked inside zip content, no longer needed
- verifyIntegrityOrThrow, extractHash: private helpers for resolve methods

Types commented out from index.ts exports:
- SkillReference, MpakSkillReference, GithubSkillReference,
  UrlSkillReference, ResolvedSkill

Also:
- Remove jszip mention from class doc (no longer used)
- Update index.ts module example to show downloadBundle/downloadSkillBundle
- Comment out downloadSkillContent tests (replaced by downloadContent tests)
- Add TODO comment noting getSkillDownload and getSkillVersionDownload
  should be merged to mirror getBundleDownload once CLI is updated

SDK build size reduced from 16.83KB to 12.34KB (ESM).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment out SkillReferenceBase, MpakSkillReference, GithubSkillReference,
UrlSkillReference, SkillReference, and ResolvedSkill from types.ts.
These types supported the now-commented-out resolveSkillRef functionality
and are not used by the CLI or any external consumer.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
shwetank-dev and others added 2 commits March 5, 2026 13:39
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…SkillBundle

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

@mgoldsborough mgoldsborough left a comment

Choose a reason for hiding this comment

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

Clean up all the old code/comments etc - it's a relatively small PR but the diff is super messy with more than half the lines just comments.

@mgoldsborough
Copy link
Contributor

I'd also recommend switching Buffer to Uint8Array.

Three things to change:

  1. Buffer.from(await response.arrayBuffer()) → new Uint8Array(await response.arrayBuffer()) — standard JS equivalent
  2. Return types on downloadContent, downloadBundle, downloadSkillBundle — Buffer → Uint8Array
  3. computeSha256 signature — string | Buffer → string | Uint8Array. Node's createHash().update() already accepts Uint8Array, so the implementation doesn't change.

Plus test updates.

shwetank-dev and others added 4 commits March 10, 2026 10:50
…ip to devDependencies

Remove all commented-out methods, types, and TODO markers per PR review.
Move jszip from dependencies to devDependencies since it's only used in integration tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…eturn fields

Switch download methods from Node-specific Buffer to standard Uint8Array
for cross-runtime compatibility. Rename return fields from
bundleRaw/bundleMetadata and skillRaw/skillMetadata to data/metadata.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@shwetank-dev
Copy link
Collaborator Author

@mgoldsborough if you can clear this one, then it we can dogfood this in the CLI.

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.

2 participants