mirror of
https://github.com/Gitlawb/openclaude.git
synced 2026-08-24 02:34:15 -05:00
* fix(deps): ship a zero-warning, minimal install
The published package declared 62 runtime `dependencies`, but `dist/cli.mjs`
is a fully-bundled esbuild output that inlines almost all of them. End users
therefore installed ~476 transitive packages — including three subtrees the
bundle never needs at install time, each emitting an install warning:
- node-domexception (deprecated) via google-auth-library
- protobufjs (allow-scripts) via @grpc/* (already bundled into dist)
- sharp (allow-scripts) native image module
The repo's `overrides`/`allowScripts` silence these locally, but those are
root-only npm settings and are ignored when the package is installed as a
dependency — so end users saw the warnings.
Core changes:
- package.json: runtime dependencies trimmed 62 -> 3 (@orama/orama,
@orama/plugin-data-persistence, @vscode/ripgrep). Bundled packages, plus
the optional sharp/google-auth-library, move to devDependencies so they
are built/tested but not shipped.
- package.json: @anthropic-ai/sdk, @modelcontextprotocol/sdk, react and
react-reconciler declared as OPTIONAL peerDependencies — externalized by
the ./sdk bundle but bundled into the CLI. Optional peers keep the CLI
install minimal and warning-free while still resolving for ./sdk consumers.
- externals.ts: sharp, google-auth-library and @anthropic-ai/bedrock-sdk
marked OPTIONAL_RUNTIME_EXTERNALS (loaded on demand, not shipped).
- validate-externals.ts: runtime deps validate against externals; bundled
deps validate against dependencies + devDependencies.
- client.ts: load @anthropic-ai/bedrock-sdk via the runtime importer so
esbuild no longer inlines it and hoists its static @aws-sdk import into
the CLI bundle (that was a startup crash for default installs).
Optional-dependency UX (consistent, actionable errors):
- New src/utils/optionalRuntimeModule.ts exports importRuntimeModule and
importOptionalRuntimeModule. The optional variant translates a missing
package (code === 'ERR_MODULE_NOT_FOUND', specifier present in message)
into "<feature> requires "<pkg>" ... Run `npm i -g <pkg>`". Generic so
typed call sites keep their module types.
- Routed ALL optional-package load sites through it (previously only one
did): google-auth-library (client.ts, auth.ts, geminiAuth.ts),
@anthropic-ai/foundry-sdk + @azure/identity (client.ts), and the
@aws-sdk/* Bedrock paths (model/bedrock.ts, tokenEstimation.ts, aws.ts).
- imageProcessor.ts: sharp-missing error now says `npm i -g sharp`.
- docs/advanced-setup.md: new "Optional provider packages" table and a
Vertex note documenting the on-demand installs.
- Unit test for the helper (friendly error, success path, specifier match,
raw passthrough).
- knip.json: ignore google-auth-library (now loaded via runtime string).
Verified on the current tree:
- tsc, build/validate-externals, knip, and tests all pass.
- npm pack + install --omit=dev adds 8 packages, zero deprecation/
allow-scripts/funding warnings; --version/--help/mcp list run.
- With packages absent, CLAUDE_CODE_USE_BEDROCK and CLAUDE_CODE_USE_VERTEX
print the friendly `npm i -g <pkg>` error (verified end-to-end).
- ./sdk imports once its optional peers are present (24 exports, no warns).
- Bundled ajv + ajv-formats validate with no ajv installed; no unguarded
native runtime requires (fsevents absent in chokidar 4; bun:sqlite Bun-only).
Trade-off: image reads, AWS Bedrock, Azure Foundry and GCP/Vertex now prompt
a one-time `npm i -g <pkg>` instead of being shipped to every user.
Co-Authored-By: OpenClaude <openclaude@gitlawb.com>
Review fixes (CodeRabbit + jatmn):
- validate-externals: the INTENTIONALLY_BUNDLED exemption is now scoped per
bundle. The CLI exempts every bundled package; the SDK does NOT exempt
packages declared as peerDependencies (keyed on package.json, an independent
source of truth) so dropping react/@anthropic-ai/sdk from SDK_EXTERNALS now
fails validation instead of silently passing. Added an explicit minimal-
install contract check: bundled packages must be devDependencies-only — never
in `dependencies`, and only the SDK-external subset may be optional peers.
Validation logic extracted to scripts/externalsValidation.ts + tests.
- FileReadTool oversized-image fallback now loads via the shared
getImageProcessor() (not a raw import('sharp')) and re-throws
ImageProcessorUnavailableError, so a missing processor surfaces the
`npm i -g sharp` install hint instead of returning an over-budget image.
- optionalRuntimeModule: match the missing specifier as a QUOTED token, not a
raw substring, so a missing transitive package whose name contains the
requested one (sharp vs sharp-libvips, @aws-sdk/client-bedrock vs
@aws-sdk/client-bedrock-runtime) no longer triggers the wrong install hint.
Predicate extracted to isMissingSpecifierError() with regression tests.
- docs/advanced-setup.md: the Vertex auth section now shows both documented
paths (gcloud ADC and a GOOGLE_APPLICATION_CREDENTIALS service-account file).
Review fixes (round 2, CodeRabbit):
- validate-externals: assert the optional-peer install contract — every
peerDependency must be { optional: true } in peerDependenciesMeta
(validateOptionalPeers), so losing that flag fails the build instead of
silently reintroducing install warnings.
- validate-externals: hard-check OPTIONAL_RUNTIME_EXTERNALS placement
(validateOptionalRuntimeexternals). Anything esbuild can see statically must
stay external in BOTH bundles (dropping sharp/google-auth-library now fails);
the runtime-indirection-only subset (new RUNTIME_INDIRECTION_ONLY_EXTERNALS)
must stay OUT of externals so esbuild never re-exposes their static imports.
- Deeper-dig fix: @anthropic-ai/foundry-sdk was misclassified as
INTENTIONALLY_BUNDLED, but it is loaded only through the Function indirection
(esbuild never sees it, so it was never actually bundled) — its sole presence
in dist is the specifier string. Per the PR's own "Azure Foundry now prompts"
trade-off it is on-demand, so it now lives in OPTIONAL_RUNTIME_EXTERNALS +
RUNTIME_INDIRECTION_ONLY_EXTERNALS (mirroring bedrock-sdk). sandbox-runtime is
genuinely statically imported, so it stays bundled.
- Provider-routing coverage (scripts/optionalRuntimeSpecifiers.test.ts): a
static scan asserts every importOptionalRuntimeModule specifier is a declared
OPTIONAL_RUNTIME_EXTERNAL and never also INTENTIONALLY_BUNDLED — the
invariant that keeps a provider's optional package loadable on demand.
- All new validators extracted to scripts/externalsValidation.ts with tests.
Review fixes (round 3, CodeRabbit):
- client.ts: gate the Vertex google-auth-library import behind the non-skip
branch. CLAUDE_CODE_SKIP_VERTEX_AUTH (proxy/test) uses a mock GoogleAuth and
must not require the optional package; it was loaded unconditionally before.
- optionalRuntimeModule: drop the hard-coded `npm i -g`. The helper backs both
the global CLI and project-local ./sdk consumers, so the hint is now
context-neutral ("npm install <pkg>" / add -g for the global CLI).
- validate-externals: every SDK_ONLY_EXTERNALS entry must STAY a
peerDependency (a dropped peer leaves runtimeDeps while the SDK still
externalizes it); and OPTIONAL_RUNTIME_EXTERNALS must never be shipped (fail
on overlap with dependencies/peerDependencies). Both with tests + live-verified.
- optionalRuntimeSpecifiers.test: pin the EXACT set of optionally-loaded
specifiers instead of a >=5 count (a count passes even if a provider path
regresses).
- attachments: extract tryReadEditedImageAttachment() — background watched-file
image attachments DEGRADE to null on any failure (incl.
ImageProcessorUnavailableError) so a missing optional package never aborts a
turn, while the explicit FileReadTool path still surfaces the install hint.
Deterministic regression test (bad path -> null).
- docs: Bedrock row notes profile-based auth also needs
@aws-sdk/credential-providers; install-hint wording matches the new message.
Review fixes (round 4, CodeRabbit):
- attachments: stop sending the raw file path through the analytics
bypass-cast (tengu_watched_file_compression_failed). Send only the safe
file extension via getFileExtensionForAnalytics, matching the existing
tengu_file_read_dedup pattern, so no usernames/project paths can leak.
- externals.ts: corrected the OPTIONAL_RUNTIME_EXTERNALS header comment,
which still claimed all entries "remain in COMMON_EXTERNALS" — no longer
true since the indirection-only subset (bedrock/foundry) must stay OUT of
the externals lists.
(Other CodeRabbit comments on this push re-surface items already addressed in
prior commits: the peerDependenciesMeta-optional check (validateOptionalPeers),
the SDK-peers-present and optional-not-shipped validator rules, the
exact-specifier-set test, the attachments degrade contract + test, and the
context-neutral install hint are all present. The "assert every optional
external is a devDependency" suggestion is intentionally NOT applied: @aws-sdk/*
and @azure/identity are transitive devDeps via bedrock-sdk/foundry-sdk, so a
blanket assertion would be incorrect; source resolution is covered by the
build + tests that import these packages.)
Review fixes (round 5, CodeRabbit):
- attachments: stop leaking file paths via logError in the background-image
degrade path. readImageWithTokenBudget can throw path-bearing messages
(e.g. "Image file is empty: <path>") and logError persists message/stack, so
log only the error TYPE name now. (Analytics payload was already sanitized.)
- attachments: tryReadEditedImageAttachment takes an injectable reader so the
degrade contract is tested for the EXACT error types — ImageProcessorUnavailableError
and a path-bearing read error both degrade to null (not just ENOENT) — plus a
success case. No mocking.
- validate-externals: enforce the source-install half of the optional contract.
Non-transitive OPTIONAL_RUNTIME_EXTERNALS must be devDependencies so `bun
install` source builds resolve them. The new TRANSITIVE_OPTIONAL_EXTERNALS
documents the exemption (@aws-sdk/* via @anthropic-ai/bedrock-sdk, @azure/identity
via @anthropic-ai/foundry-sdk — provided transitively, not direct devDeps). A
blanket "all optionals are devDeps" check would have wrongly failed on those.
Tests + live-verified (dropping sharp from devDependencies now fails).
Review fixes (round 6, CodeRabbit + jatmn):
- optionalRuntimeSpecifiers.test: the call-site scan regex missed
generic-annotated calls (importOptionalRuntimeModule<...>(...)) in
model/bedrock.ts and tokenEstimation.ts, so the exact-set assertion was
incomplete. Regex now allows an optional generic; EXPECTED_SPECIFIERS adds
@aws-sdk/client-bedrock and @aws-sdk/client-bedrock-runtime (7 total).
- importOptionalRuntimeModule default generic is now <T = unknown> (was any),
so destructured imports are no longer silently any. Every call site now
supplies its module type — typeof import('<pkg>') where the package is
type-resolvable (bedrock-sdk, foundry-sdk, @aws-sdk/credential-providers,
google-auth-library), and a named minimal-shape alias for @azure/identity
(not a direct devDep, so typeof import can't resolve it). This gives
compile-time verification of each provider's module contract (export names,
shapes) — the structural answer to the "cover the provider branches" ask.
- attachments: tryReadEditedImageAttachment takes injectable {read,log,track};
a new test asserts the sanitized-telemetry contract directly — the logError
payload is path-free and the analytics payload carries only `ext`, never the
edited-image path.
* fix(deps): address optional runtime review findings
* test(deps): isolate optional runtime importer mocks
* fix(deps): clarify AWS optional auth labels
* fix(deps): close optional runtime review gaps
---------
Co-authored-by: jatmn <the@jat.mn>
92 lines
3.7 KiB
TypeScript
92 lines
3.7 KiB
TypeScript
/**
|
|
* Runtime loading for optional, on-demand dependencies.
|
|
*
|
|
* Some provider SDKs and native helpers are NOT shipped in the default install
|
|
* (see OPTIONAL_RUNTIME_EXTERNALS in scripts/externals.ts) — they are loaded
|
|
* only when a provider/feature that needs them is actually used. We import them
|
|
* through a `new Function` indirection so esbuild cannot see the specifier:
|
|
* that keeps the package out of the CLI bundle AND prevents esbuild from
|
|
* hoisting the package's own static imports (e.g. @anthropic-ai/bedrock-sdk's
|
|
* static `@aws-sdk/client-bedrock-runtime` import) into the bundle, which would
|
|
* otherwise make those packages required at startup for every user.
|
|
*/
|
|
|
|
// Hidden from esbuild's static analysis: resolved from node_modules at runtime.
|
|
const runtimeImport = new Function(
|
|
'specifier',
|
|
'return import(specifier)',
|
|
) as (specifier: string) => Promise<any>
|
|
|
|
export class OptionalRuntimeModuleUnavailableError extends Error {
|
|
constructor(
|
|
readonly feature: string,
|
|
readonly specifier: string,
|
|
) {
|
|
super(
|
|
`${feature} requires the "${specifier}" package, which is not installed. ` +
|
|
`Install it with \`npm install ${specifier}\` (add \`-g\` if you installed the CLI globally) to enable it.`,
|
|
)
|
|
this.name = 'OptionalRuntimeModuleUnavailableError'
|
|
}
|
|
}
|
|
|
|
export function isOptionalRuntimeModuleUnavailableError(
|
|
error: unknown,
|
|
): error is OptionalRuntimeModuleUnavailableError {
|
|
return error instanceof OptionalRuntimeModuleUnavailableError
|
|
}
|
|
|
|
/** Raw runtime import — rejects with the underlying error if the module is missing. */
|
|
export function importRuntimeModule(specifier: string): Promise<any> {
|
|
return runtimeImport(specifier)
|
|
}
|
|
|
|
/**
|
|
* True only when `error` is the resolver reporting that THIS specifier (not a
|
|
* lookalike transitive package) could not be found. Node reports the unresolved
|
|
* name quoted — `Cannot find package 'sharp' imported from ...` — so we match
|
|
* the quoted token rather than a raw substring. A bare includes() would
|
|
* misattribute a missing transitive package whose name merely contains ours
|
|
* (`sharp` ⊂ `sharp-libvips`, `@aws-sdk/client-bedrock` ⊂
|
|
* `@aws-sdk/client-bedrock-runtime`) and print an install hint for the wrong
|
|
* package instead of surfacing the real failure.
|
|
*/
|
|
export function isMissingSpecifierError(
|
|
error: unknown,
|
|
specifier: string,
|
|
): boolean {
|
|
const code = (error as { code?: string } | undefined)?.code
|
|
if (code !== 'ERR_MODULE_NOT_FOUND') return false
|
|
const message = error instanceof Error ? error.message : String(error)
|
|
return (
|
|
message.includes(`'${specifier}'`) || message.includes(`"${specifier}"`)
|
|
)
|
|
}
|
|
|
|
/**
|
|
* Import an optional runtime dependency. If the package itself is not installed,
|
|
* throw a clear, actionable error naming the feature and the install command
|
|
* instead of a cryptic ERR_MODULE_NOT_FOUND.
|
|
*
|
|
* The guard checks the error `code` AND that the failing specifier is the
|
|
* package we asked for — so a genuine missing-package error is reported with
|
|
* the install hint, while a broken transitive dependency inside an installed
|
|
* package surfaces its real error rather than a misleading "not installed".
|
|
*/
|
|
export async function importOptionalRuntimeModule<T = unknown>(
|
|
specifier: string,
|
|
feature: string,
|
|
): Promise<T> {
|
|
try {
|
|
return (await runtimeImport(specifier)) as T
|
|
} catch (e) {
|
|
if (isMissingSpecifierError(e, specifier)) {
|
|
// Context-neutral install hint: this helper backs both the globally
|
|
// installed CLI and the project-local ./sdk consumers, so don't prescribe
|
|
// `-g` (which is wrong for a local SDK install).
|
|
throw new OptionalRuntimeModuleUnavailableError(feature, specifier)
|
|
}
|
|
throw e
|
|
}
|
|
}
|