fix(vendors): make vendor SDKs optional peer-deps - #54
Open
tylern91-kat wants to merge 6 commits into
Open
Conversation
* feat(vendors): add fail-soft loadSdk helper Optional vendor SDKs need a shared loader that turns a genuine missing module into an actionable "npm install -D <specifier>" error while re-throwing any other failure untouched — including a transitive ERR_MODULE_NOT_FOUND raised from inside an SDK that IS installed, which must not be misdiagnosed as the SDK itself being absent. * refactor(vendors): route SDK loaders through loadSdk Delegates each vendor's default-SDK loader to the shared loadSdk helper instead of calling await import(specifier) directly, so a missing SDK surfaces the same actionable install message everywhere. Pure refactor: the deps.* DI seams are untouched, so injected fakes still bypass the loader entirely in unit tests. * build(deps): move vendor SDKs to optional peer-deps The three vendor SDKs (claude-agent-sdk, copilot-sdk, codex-sdk) were hard dependencies, forcing every install to pull all three (~2GB) even though a user only drives one agent. Every runtime use is already behind a lazy await import() reached only when that vendor is selected, so the install-time coupling was the only thing forcing the full download. Move them to peerDependencies (optional) so a downstream install pulls only the SDK for the agent actually in use, and keep them pinned in devDependencies so probity's own build/dogfood/integration tests keep working. Mirrors the existing @ast-grep/lang-* pattern from ADR-0006. Versioning is left to the maintainer; CHANGELOG will flag both a major and minor option since this changes the default install contract. Refs: nizos#45 * docs: record ADR-0011 and README note for optional vendor SDKs Explains the peer-dep + fail-loud-loader decision for nizos#45, and tells users up front that installing an agent's SDK is now a separate step from installing probity itself. * style: apply prettier formatting Wrapping/emphasis-syntax fixes surfaced once real deps were installed. * chore(deps): refresh lockfile for the peer-dep move npm install after moving the SDKs to peerDependencies/devDependencies.
This reverts commit 47ac0f1.
* feat(vendors): add fail-soft loadSdk helper Optional vendor SDKs need a shared loader that turns a genuine missing module into an actionable "npm install -D <specifier>" error while re-throwing any other failure untouched — including a transitive ERR_MODULE_NOT_FOUND raised from inside an SDK that IS installed, which must not be misdiagnosed as the SDK itself being absent. * refactor(vendors): route SDK loaders through loadSdk Delegates each vendor's default-SDK loader to the shared loadSdk helper instead of calling await import(specifier) directly, so a missing SDK surfaces the same actionable install message everywhere. Pure refactor: the deps.* DI seams are untouched, so injected fakes still bypass the loader entirely in unit tests. * build(deps): move vendor SDKs to optional peer-deps The three vendor SDKs (claude-agent-sdk, copilot-sdk, codex-sdk) were hard dependencies, forcing every install to pull all three (~2GB) even though a user only drives one agent. Every runtime use is already behind a lazy await import() reached only when that vendor is selected, so the install-time coupling was the only thing forcing the full download. Move them to peerDependencies (optional) so a downstream install pulls only the SDK for the agent actually in use, and keep them pinned in devDependencies so probity's own build/dogfood/integration tests keep working. Mirrors the existing @ast-grep/lang-* pattern from ADR-0006. Versioning is left to the maintainer; CHANGELOG will flag both a major and minor option since this changes the default install contract. Refs: nizos#45 * docs: record ADR-0011 and README note for optional vendor SDKs Explains the peer-dep + fail-loud-loader decision for nizos#45, and tells users up front that installing an agent's SDK is now a separate step from installing probity itself. * chore(deps): refresh lockfile for the peer-dep move npm install after moving the SDKs to peerDependencies/devDependencies. * chore: retrigger CI Actions was disabled by default on this fork and has just been enabled manually. Empty commit to force a fresh workflow run against PR #3, which was pushed before Actions was turned on. --------- Co-authored-by: Tyler Nguyen <tyler.nguyen1191@gmail.com>
Author
|
Reopening at the author's request — leaving this open for discussion, see #45 (comment) |
Author
|
@nizos Could you please kindly review it? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #45.
Every vendor SDK is currently a hard
dependency, so installing probity pulls in everysupported vendor's SDK regardless of which agent you actually use. This adds a fail-soft
loadSdkhelper that vendoragent.tsmodules route through, moves the vendor SDKs tooptionalDependencies/peer-deps, and documents the tradeoff in ADR-0011.src/vendors/load-sdk.ts— fail-soft dynamic import; missing SDK degrades rather thanthrowing at module load time.
src/vendors/{claude-code,codex,github-copilot}/agent.ts— route their SDK loaders throughloadSdkinstead of a static top-level import.package.json/ lockfile — move vendor SDKs out of harddependencies.docs/adr/0011-optional-peer-deps-for-vendor-sdks.md+ a README note — record why.Test plan
src/vendors/load-sdk.test.tsexplicitly covers the missing-SDK degradation path (theproperty this fix is actually for, not incidental)
npm run checksgreen: lint, format, typecheck, and the full suite (59 files, 543passed / 26 skipped)