mirror of
https://github.com/Gitlawb/openclaude.git
synced 2026-08-24 10:14:19 -05:00
2edec9a140
* 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>