mirror of
https://github.com/Gitlawb/openclaude.git
synced 2026-08-24 10:14:19 -05:00
main
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  
|
ca7a7e0791 |
feat(install): enforce and guard the zero-warning npm install contract (#2019)
* feat(install): enforce and guard the zero-warning npm install contract
`npm install -g @gitlawb/openclaude` is verified zero-warning today, but
nothing kept it that way: the runtime deps were caret ranges resolved
fresh on every user install (the published tarball ships no lockfile),
no CI step ever installed the package, and registry-side drift (a
transitive dep deprecated after we ship) is invisible to file-based CI.
Static contract (fast, offline, every PR via `bun run build`):
- Pin the 3 runtime deps to exact versions so the verified resolution IS
the shipped resolution.
- New validators in scripts/externalsValidation.ts (unit-tested):
dependencies must equal RUNTIME_DEPENDENCY_CONTRACT exactly (no ranges,
no unreviewed additions), no consumer-run install hooks or funding
field, engines.node pinned. Wired into validate-externals.ts.
Runtime verification (scripts/verify-clean-install.ts, `install:verify`):
- Tarball mode (release gate) and published mode (registry watch), each
running cold-install and upgrade-over-previous scenarios in throwaway
prefixes with a cold cache and normalized env/flags.
- Strict output whitelist (summary lines only) with network failures
retried and reported as infra (exit 2), never as a hygiene verdict.
- Structural authority over the installed tree: any package declaring
install scripts fails, the installed manifest must match the static
contract, tarball payload/size asserted.
- Boot must be silent: --version prints the exact packed version;
--help (which, unlike the --version zero-import fast path, loads the
real bundle) must exit 0 with empty stderr.
CI: release publishes only after the verify passes on Node 22 (npm 10,
the supported floor — warning phrasing and EBADENGINE behavior differ
from npm 11) and Node 24, plus a final gate on the publishing machine
replacing `npm pack --dry-run`. A daily install-hygiene workflow
re-verifies the published @latest on {ubuntu, macos, windows} x
{Node 22, 24} — the only defense against post-release registry drift,
and the OS matrix covers the per-platform @vscode/ripgrep packages.
Found-by-the-guard fix: a fresh machine printed "Warning: ignoring saved
provider profile. OPENGATEWAY_API_KEY is required..." on every command
(even --help) because the injected fresh-install Opengateway default
fails validation without a key (#1651 chose ignore+warn). The default
env is still ignored, but the warning now only fires for genuinely
persisted profiles; published 0.24.0 carries the old noise, so the
verify script exempts exactly that version until the next release.
Co-Authored-By: OpenClaude <openclaude@gitlawb.com>
* fix(install): address CodeRabbit review on the install-hygiene guard
- release.yml: pin install-verify to least-privilege `contents: read` and
disable credential persistence on its checkout; same persist-credentials
hardening on the install-hygiene cron checkout.
- verify-clean-install: previousPublishedVersion now follows the same
retry/infra discipline as installWithRetry — transient registry failures
retry and then exit 2 (infra) instead of silently skipping the
upgrade-scenario coverage; a clean not-published answer still skips.
- providerProfile: the fresh-install warning suppression now keys on
explicit provenance (persisted profile resolved once in
applyStartupEnvFromProfile) instead of sniffing the
DEFAULT_STARTUP_PROVIDER_ENV_VAR marker, which a persisted profile's
env can inherit from a parent CLI process; regression test covers the
marker-collision case.
Co-Authored-By: OpenClaude <openclaude@gitlawb.com>
* test(install): cover previousPublishedVersion retry/skip/infra branches
CodeRabbit follow-up: the branches deciding whether the upgrade-install
scenario runs, skips, or aborts as infra were untested. Extract the loop
as resolvePreviousPublishedVersion with injected effects (runView,
onRetry, onInfraFailure) per the repo's dependency-injection testing
convention, guard main() behind import.meta.main so the test import does
not launch a real verification, and add regression tests: first-try
success, transient-infra retry then success, clean E404 → null skip
without retries, persistent infra → onInfraFailure (exit 2 in the real
wiring), and unparseable version output → null.
Co-Authored-By: OpenClaude <openclaude@gitlawb.com>
---------
Co-authored-by: OpenClaude <openclaude@gitlawb.com>
|
||
|
2edec9a140 |
fix(deps): ship a zero-warning, minimal install (#1784)
* 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>
|
||
 stamsamandGitHub
|
64ad44abaf | chore(build): reject stale bundled external entries (#1275) | ||
|
|
2c71e09394 |
chore(build): clean up external dependency validation warnings (#1124)
* chore(build): clean up external dependency validation warnings Remove 2 unused externals (@opentelemetry/sdk-trace-node, ink) and add 12 missing packages to package.json that are dynamically imported at runtime but weren't declared as dependencies. Also remove the unused @opentelemetry/sdk-trace-node dependency. This eliminates all 13 build validation warnings: - 8 missing OTel exporter deps (http, proto, grpc variants + prometheus) - 4 missing AWS SDK deps (bedrock, bedrock-runtime, sts, credential-providers) - 1 missing Azure dep (@azure/identity) - ink external pointed to local reimplementation, not npm package - sdk-trace-node was declared external but never imported Build validation now passes cleanly with 0 warnings. Co-Authored-By: OpenClaude <openclaude@gitlawb.com> * chore(build): eliminate external validation warnings Remove unused @opentelemetry/sdk-trace-node from externals and package.json (it's not imported anywhere in src/). Remove ink from SDK_ONLY_EXTERNALS (the project reimplements ink locally at src/ink/). Add OPTIONAL_RUNTIME_EXTERNALS list for packages that are dynamically imported but intentionally not direct deps — OTel protocol exporters and cloud provider SDKs are resolved from transitive deps or installed by users who need them. Validation now passes with 0 warnings instead of 13. Co-Authored-By: OpenClaude <openclaude@gitlawb.com> * feat(telemetry): full OpenTelemetry purge — remove all tracking dependencies Replace all @opentelemetry/* runtime dependencies with no-op stubs, delete OTel-only source modules, and remove 10 @opentelemetry packages from package.json plus @growthbook/growthbook. Key changes: - Delete 5 OTel-only modules (instrumentation, betaSessionTracing, bigqueryExporter, logger, firstPartyEventLoggingExporter) - Replace 9 modules with no-op stubs (sessionTracing, events, telemetryAttributes, firstPartyEventLogger, growthbook, index, sink, datadog, sinkKillswitch, perfettoTracing) - Remove all @opentelemetry/* imports from bootstrap/state.ts, entrypoints/init.ts, and ~20 caller files - Remove all OTel counter types, meter/provider state from state.ts - Clean externals.ts: remove 27 @opentelemetry/* entries - Clean build.ts: remove OTel native-stub namespace exports - Simplify no-telemetry-plugin.ts: remove redundant source-level stubs - Remove 10 @opentelemetry/* + @growthbook/growthbook from package.json - GrowthBook stub reads local ~/.claude/feature-flags.json for overrides Co-Authored-By: OpenClaude (zai-org-glm-5-1) <openclaude@gitlawb.com> * fix format * chore: regenerate lockfile after OTel dependency removal Co-Authored-By: OpenClaude (zai-org-glm-5-1) <openclaude@gitlawb.com> * fix(growthbook): route gate helpers through local flag overrides checkStatsigFeatureGate_CACHED_MAY_BE_STALE() and checkGate_CACHED_OR_BLOCKING() now resolve from ~/.claude/feature-flags.json like getFeatureValue_* does, so gates like tengu_thinkback, tengu_ccr_bridge, and VS Code upsells can be flipped on locally. Security gates (checkSecurityRestrictionGate) remain hard-false. Also adds 5 tests covering gate helper override behavior and unifies JSDoc wording for _getFlagValue-routed functions. Co-Authored-By: OpenClaude (zai-org-glm-5-1) <openclaude@gitlawb.com> --------- Co-authored-by: OpenClaude <openclaude@gitlawb.com> |
||
 
|
60c76b6599 |
feat: SDK Runtime — Query Engine, Sessions, and Build Pipeline (#984)
* feat(sdk): add SDK foundation — type declarations, errors, and utilities Adds standalone SDK building blocks with no SDK source dependencies: - sdk.d.ts: ambient type declarations for SDK bundle - coreSchemas.ts + coreTypes.generated.ts: Zod schemas and generated types - errors.ts: SDK-specific error classes - validation.ts: input validation utilities - messageFilters.ts: extracted message filter logic - handlePromptSubmit.ts: imports from messageFilters - 16 generated-types tests * fix(sdk): narrow assertFunction type from broad Function to callable signature Code review finding: assertFunction used `asserts value is Function` which accepts any function-like value without narrowing. Changed to `(...args: any[]) => any` for better type safety. * fix(sdk): update sdk.d.ts header — manually maintained, not generated Reviewer noted the header said "Generated from index.ts" but no generator produces this file. Updated to "Manually maintained — keep in sync with index.ts". Drift detection added in validate-externals.ts (PR 3). * fix(sdk): align sdk.d.ts types with canonical coreTypes.generated.ts Tighten SDK public type contract to resolve reviewer blockers: - PermissionResult: unknown[] → precise 6-shape discriminated union (addRules/replaceRules/removeRules/setMode/addDirectories/removeDirectories) - SDKSessionInfo: snake_case → camelCase (sessionId, lastModified, etc.) - ForkSessionResult: session_id → sessionId - SDKPermissionRequestMessage: uuid + session_id now required - SDKPermissionTimeoutMessage: added uuid + session_id - SessionMessage: parent_uuid → parentUuid - SDKMessage/SDKUserMessage/SDKResultMessage: replaced loose inline definitions with re-exports from coreTypes.generated.ts * feat(sdk): wire existing code modules + SDK shared utilities Modifies core modules for SDK integration: - QueryEngine, tools, state, commands: SDK type hooks - SDK shared utilities (shared.ts, permissions.ts) - 21 SDK tests (shared-utils, permissions) Stack: main ← pr1-foundation ← pr2-sdk-core * feat(sdk): add snake_case ↔ camelCase key mapping utilities casing.ts provides recursive key transformation for the SDK boundary layer. Internal runtime uses snake_case; public API exposes camelCase. Will be used by shared.ts, sessions.ts, query.ts at export boundaries. * test(sdk): add tests for snake_case ↔ camelCase mapping utilities Covers snakeToCamel, camelToSnake, mapKeysToCamel, mapKeysToSnake including nested objects, arrays, null/undefined, and round-trips. * feat(sdk): add SDK runtime — query engine, sessions, build pipeline Completes the SDK implementation: - SDK build target (dist/sdk.mjs) with TUI dependency stubbing - External dependency lists (scripts/externals.ts) - SDK type generation from Zod schemas (scripts/generate-sdk-types.ts) - External validation (scripts/validate-externals.ts) - SDK source: index, query, v2, sessions modules - agentSdkTypes: re-exports SDK functions (query, createSession, etc.) - 136 SDK tests + 7 build scanner tests Stack: main ← pr1-foundation ← pr2-sdk-core ← pr3-sdk-runtime * fix(sdk): align internal SDK types with camelCase public contract shared.ts: SDKSessionInfo, ForkSessionResult, SessionMessage fields now use camelCase matching sdk.d.ts. SDKPermissionRequestMessage and SDKPermissionTimeoutMessage gain required uuid + session_id fields. permissions.ts: onPermissionRequest/onTimeout callbacks now include uuid and session_id in emitted messages. * fix(sdk): update runtime modules to use camelCase field names sessions.ts: toSDKSessionInfo outputs camelCase keys, entryToSessionMessage uses parentUuid, forkSession returns sessionId. query.ts: reads sessionId from listSessions/forkSession results instead of snake_case session_id. * fix(test): update session tests to use camelCase field names session_id → sessionId in forkSession result assertions and getSessionMessages calls. * fix(sdk): prevent permission timeout race condition with once-only resolve wrapper Add createOnceOnlyResolve utility to prevent double-resolution of promises when timeout and host response happen simultaneously. This ensures deterministic behavior in the permission handling flow. * fix(sdk): improve race condition test robustness * fix(sdk): handle consecutive underscores in snakeToCamel conversion Changes: - Use _+([a-z]) regex to match multiple consecutive underscores before letters - Add lookahead (?=. ) to preserve underscore-letter pairs at string end - Handle dunder names (__proto__, __typename) by stripping wrapper and capitalizing - Add tests for consecutive underscores and trailing underscore preservation * fix(sdk): include original error message in permission callback denial When a canUseTool callback throws an error, the catch block now includes the original error message in the denial message, making debugging easier for SDK consumers. * feat(sdk): add optional timeout to env mutex for deadlock prevention Add timeout parameter to acquireEnvMutex() to prevent infinite waits in deadlock scenarios. The timeout is optional and defaults to no timeout (wait forever) for backward compatibility. Returns a MutexAcquireResult object with acquired status and optional timeout reason for failed acquisitions. * fix(sdk): remove timed-out callback from mutex queue to prevent deadlock * test(sdk): add missing error path and timeout scenario tests Add tests for timeout scenarios when host doesn't respond to permission requests, fallback behavior when no onPermissionRequest callback, and MCP connection edge cases for undefined/empty config. * fix(sdk): address code review issues - race conditions, validation, error handling - Add createPermissionTarget() factory that applies onceOnlyResolve at registration time, fixing race condition where timeout and host response could both try to resolve the same promise - Add try-catch to releaseEnvMutex() to prevent permanent lock if callback throws - Extract DEFAULT_PERMISSION_TIMEOUT_MS constant (30 seconds) - Add MCP config validation rejecting null, non-objects, and arrays - Preserve error stack traces in MCP connection failures - Add runtime validation to mapMessageToSDK for null/non-object/invalid type - Update tests to use createPermissionTarget and add validation tests * fix(sdk): syntax fixes and MCP connection error handling - Remove extra closing parenthesis in permissions.ts - Remove extra closing braces in shared.ts type definitions - Wrap MCP connection in try/catch to continue without MCP tools on failure * fix(sdk): syntax fixes, MCP error handling, and logic clarity - Remove extra closing parenthesis in permissions.ts - Remove extra closing braces in shared.ts type definitions - Wrap MCP connection in try/catch to continue without MCP tools on failure - Clarify thinkingConfig logic: use ?? true instead of !== false - Add explanatory comment about thinkingEnabled default behavior - Apply createOnceOnlyResolve wrapper in QueryImpl.registerPendingPermission * fix(sdk): comprehensive error handling and resource cleanup - Add try-catch around injectAgents() to gracefully handle plugin agent tool validation failures (prevents test crashes from unknown 'LS' tool) - Add console.warn logging to agent loading/injection catch blocks for debugging visibility (matches v2.ts pattern) - Add pendingPermissionPrompts.clear() to close() and interrupt() methods in both query.ts and v2.ts to prevent memory accumulation - Add close() method to SDKSession interface and SDKSessionImpl - Wrap MCP connection in query.ts with try-catch (matches v2.ts behavior) - Add timeoutQueue cleanup in finally blocks (query.ts + v2.ts) - Remove error.stack from MCP error messages to prevent internal path leak All 208 SDK tests pass. TypeScript errors are pre-existing. * fix(sdk): address code review non-blocking issues - Add SDKAgentLoadFailureMessage type for agent load failure events - Emit agent definition/injection failures to SDK message stream - Add tool name to permission timeout denial message - Replace 'as any' casts with proper typed state access - Fix supportedCommands to use correct mcp.commands/plugins.commands paths - Update test for correct AppState structure * fix(sdk): address code review blocking and non-blocking issues Blocking Issues Fixed: - MCP cleanup missing on session/query close - now disconnects MCP clients to prevent resource leaks in long-running processes with multiple sessions - Engine reference not cleared on close - now sets _engine = null to prevent memory leaks - Added MCP cleanup tests (9 new tests covering cleanup scenarios) Non-Blocking Issues Fixed: - Removed redundant catch block that just rethrew errors (query.ts) - Fixed inconsistent timeout denial message format (permissions.ts) - Fixed hardcoded tool name 'Bash' in test (permissions.test.ts) - Exported PermissionResolveDecision type for SDK consumers (index.ts) All 217 SDK tests pass. * fix(sdk): address code review type consistency issues - Add close() method to SDKSession interface (documented but missing from type) - Fix SDKSessionInfo, ForkSessionResult, SessionMessage field naming: snake_case → camelCase to match sdk.d.ts public contract and implementation - Add uuid and session_id to SDKPermissionTimeoutMessage for correlation - Fix JSDoc comment in forkSession to use sessionId (not session_id) These changes align internal types (shared.ts) with the public SDK contract (sdk.d.ts) and actual implementation output. The merge from origin/main introduced snake_case types that mismatched camelCase implementation and tests. * fix: restore openclaude.json comment in REPL.tsx Merge |