feat: SDK Core — Permission System, Async Context, and Engine Extensions

[emsanakhchivan]

PR 2: SDK Core — Permission System, Async Context Isolation, and Engine Extensions

Summary

Adds the SDK's core runtime infrastructure — permission handling with external resolution support, AsyncLocalStorage-based context isolation for parallel SDK queries, and QueryEngine extensions for dynamic injection. Includes snake_case ↔ camelCase key mapping utilities for the SDK boundary layer. These modules form the glue between SDK consumers and the CLI's internal permission/runtime systems.

---

What changed

Status	File	Description
A	src/entrypoints/sdk/casing.ts	Snake_case ↔ camelCase key mappers (snakeToCamel, camelToSnake, mapKeysToCamel, mapKeysToSnake) — handles the naming convention conversion at the SDK boundary where internal runtime uses snake_case and public SDK API uses camelCase
A	src/entrypoints/sdk/shared.ts	SDK shared utilities: UUID session validation (assertValidSessionId), environment mutex with optional timeout for parallel query safety (acquireEnvMutex, releaseEnvMutex), SDK type definitions (SDKPermissionRequestMessage, SDKPermissionTimeoutMessage, SDKSessionInfo, etc.), and mapMessageToSDK() with runtime validation
A	src/entrypoints/sdk/permissions.ts	Permission handling for SDK: buildPermissionContext() maps SDK permission modes to internal modes, createExternalCanUseTool() supports external permission resolution via timeout + host callback, createPermissionTarget() factory for race-condition-safe promise resolution, createDefaultCanUseTool() implements secure-by-default denial, connectSdkMcpServers() for MCP server connection from SDK options with config validation
M	src/QueryEngine.ts	+127 lines: added injectMessages() for session fork/resume, injectAgents() for async agent loading, updateTools() for dynamic permission mode changes (transactional validation), setThinkingConfig() for thinking token budget control, fixed agentDefinitions.allAgents assignment, replaced lazy MessageSelector import with direct messageFilters import
M	src/bootstrap/state.ts	+40 lines: AsyncLocalStorage-based SDK context isolation — runWithSdkContext() overrides global STATE reads (sessionId, cwd, originalCwd, sessionProjectDir) for the current async execution context, enabling parallel SDK queries without cross-session contamination
M	src/tools.ts	+28 lines: defensive null checks — filter(Boolean) on tool arrays, null-safe isEnabled() checks, prevents crash if lazy getters return null/undefined during initialization timing edge cases
M	src/commands.ts	+20 lines: null-safe meetsAvailabilityRequirement() accepts nullable Command, defensive formatDescriptionWithSource() handles missing description, .filter(Boolean) on login/logout commands array
A	src/utils/messageFilters.ts	Extracted from MessageSelector.tsx — selectableUserMessagesFilter() and messagesAfterAreOnlySynthetic() moved to standalone utility module for SDK reuse without React/ink dependency
M	src/components/MessageSelector.tsx	-65 lines: filter functions moved to messageFilters.ts, imports from new module
M	src/screens/REPL.tsx	Import path update for messageFilters.ts
A	tests/sdk/casing.test.ts	92 lines: comprehensive tests for casing conversion including edge cases (consecutive underscores, trailing underscores, dunder names like __proto__)
A	tests/sdk/permissions.test.ts	416 lines: tests for permission context building, external canUseTool with race conditions, once-only resolve wrapper, timeout scenarios, MCP connection error handling, createPermissionTarget() factory validation
A	tests/sdk/shared-utils.test.ts	136 lines: tests for session ID validation, message mapping, mutex timeout behavior, concurrent acquisition, mutex state recovery after timeout

---

Why it changed

The SDK needs to integrate with the CLI's existing permission and runtime systems while maintaining isolation for parallel query execution. The permission system allows SDK consumers to either provide a synchronous canUseTool callback or handle permission requests asynchronously via timeout + host response pattern. AsyncLocalStorage enables per-query context that overrides global state without modifying the CLI's singleton-based architecture. The QueryEngine extensions support SDK-specific workflows like session forking/resume and dynamic permission mode switching. Casing utilities bridge the snake_case internal naming (JSONL files, session storage) with camelCase SDK API convention.

---

Impact

• User-facing impact: None — these are internal SDK infrastructure changes, no CLI behavior changes
• Developer/maintainer impact:
  • New SDK modules available at src/entrypoints/sdk/
  • QueryEngine has new injection methods for SDK use
  • Global state accessors now support AsyncLocalStorage override
  • Tools array handling more defensive against null
• SDK consumer impact: Permission handling defaults to deny-all (secure-by-default), requires explicit canUseTool or onPermissionRequest callback

---

Testing

• bun test tests/sdk/ — 70 pass, 0 fail (132 expect calls)
• bun run build — passes
• bun run smoke — not affected
• All race condition scenarios tested with timing-sensitive tests (50ms timeout + 25ms wait pattern)

---

Notes

• Provider/model path tested: N/A (no provider changes)
• Screenshots attached: N/A
• Follow-up work: This PR is part of a 3-PR stack:
  • PR 1: SDK Foundation — Type Declarations, Errors, and Utilities (merged)
  • PR 2 (this): SDK Core — Permission System, Async Context, Engine Extensions ← you are here
  • PR 3: SDK Runtime — query engine, sessions, build pipeline
• Key design decision: createPermissionTarget() factory applies onceOnlyResolve wrapper at registration time (not at timeout), ensuring both timeout handler and host response use the same wrapped resolve — prevents "promise already resolved" errors
• Known limitation: MCP config validation rejects null and arrays but doesn't validate individual fields against ScopedMcpServerConfig schema (deferred — would require importing Zod at runtime)
• Race condition safety: All permission timeout paths tested with explicit race scenarios where host responds at exact timeout threshold

[gnanam1990]

Pulled the branch — 70 SDK tests pass locally, CI green, no openclaude red flags introduced (no tengu_, no new network calls, no Anthropic fingerprints).

I'm not approving this in a single pass though. ~1.6k lines across 15 files touching permission handling, AsyncLocalStorage isolation, mutex acquisition with timeouts, and race-condition-safe promise resolution is too much surface for one reviewer — and the failure modes (permission bypass, cross-session state leak, unresolved promises) are exactly the kind that pass tests but bite in production.

Two requests:

1. Could you tag @kevincodex1 or @anandh8x for a second review? This needs more than one set of eyes by policy for a stack-of-3 SDK runtime PR.

2. The permission createDefaultCanUseTool deny-by-default is the right call, but I want to walk through the createPermissionTarget() once-only-resolve flow against acquireEnvMutex timeout interaction more carefully. Could you point me at a specific test that exercises: host responds after the SDK timeout has already fired and called the deny path? Want to confirm there's no double-resolve / leaked listener path.

Will do a focused second pass once a second reviewer is on board and you've pointed at the test for #2.

[emsanakhchivan]

Pulled the branch — 70 SDK tests pass locally, CI green, no openclaude red flags introduced (no tengu_, no new network calls, no Anthropic fingerprints).

I'm not approving this in a single pass though. ~1.6k lines across 15 files touching permission handling, AsyncLocalStorage isolation, mutex acquisition with timeouts, and race-condition-safe promise resolution is too much surface for one reviewer — and the failure modes (permission bypass, cross-session state leak, unresolved promises) are exactly the kind that pass tests but bite in production.

Two requests:

1. Could you tag @kevincodex1 or @anandh8x for a second review? This needs more than one set of eyes by policy for a stack-of-3 SDK runtime PR.
2. The permission createDefaultCanUseTool deny-by-default is the right call, but I want to walk through the createPermissionTarget() once-only-resolve flow against acquireEnvMutex timeout interaction more carefully. Could you point me at a specific test that exercises: host responds after the SDK timeout has already fired and called the deny path? Want to confirm there's no double-resolve / leaked listener path.

Will do a focused second pass once a second reviewer is on board and you've pointed at the test for #2.

Re #1 — I've requested a second reviewer.
Tagging @kevincodex1 / @anandh8x — can one of you take a look?

Re #2 — Here's the test you asked for:

tests/sdk/permissions.test.ts — two new tests covering the sequential timeout-then-host-response scenario:

Test 1: Integration-level (host response after timeout is safely ignored)

• Starts canUseTool with 50ms timeout.
• Grabs a stale reference to the resolve function before timeout fires (simulates host capturing the callback).
• Waits past the 50ms timeout → timeout fires, resolves deny, deletes map entry.
• Host calls staleResolve.resolve({ allow }) through the captured reference.
• createOnceOnlyResolve silently ignores it — no error, no mutation, result stays 'deny'.
• Map cleanup verified — no leaked listener.

Test 2: Unit-level (timeout-deny-then-host-allow: raw resolve called exactly once)

• Directly proves createOnceOnlyResolve prevents the raw resolve from executing twice.
• Wraps a counted spy function, calls wrapped resolve with deny, then with allow.
• Asserts rawCallCount === 1 — second call was a complete no-op.
• This test would fail if createOnceOnlyResolve was removed or broken.

---

The two tests are complementary: Test 1 proves the integration flow is safe, Test 2 proves why it's safe at the mechanism level.

For the acquireEnvMutex timeout interaction specifically — the permission system and mutex are functionally independent (no shared state, no cross-coupling). The mutex timeout (shared.ts:52-91) uses the same resolved flag pattern with queue cleanup on timeout. Both systems use independent once-only guards rather than a shared mechanism, so there's no compositional risk.

[kevincodex1]

lets have a look into this @Vasanthdev2004 @techbrewboss @devNull-bootloader

[kevincodex1]

why this change we are moving away from claude.json and will be using openclaude.json

[emsanakhchivan]

That was a stale merge-conflict
resolution. The comment should reference
~/.openclaude.json as you noted. Fixed in c725c48.

[Vasanthdev2004]

Thanks for the detailed follow-up. I did a targeted re-review of the current head, focused on the SDK permission flow and the tests you pointed to.

Verdict: Needs changes

Blocking issue:

1. createExternalCanUseTool() emits onPermissionRequest(...) before it registers the pending resolver with permissionTarget.registerPendingPermission(toolUseID). That means a host that responds synchronously/immediately from the permission request callback can lose the response because pendingPermissionPrompts does not contain the tool use yet. The request then waits until timeout and denies/falls back even though the host allowed it.

Minimal repro against current head c725c48:

bun --eval "const m = await import('./src/entrypoints/sdk/permissions.ts'); const target = m.createPermissionTarget(); const canUseTool = m.createExternalCanUseTool(undefined, async () => ({ behavior: 'deny', message: 'fallback' }), target, (message) => { const pending = target.pendingPermissionPrompts.get(message.tool_use_id); if (pending) pending.resolve({ behavior: 'allow' }); }, undefined, 5); const result = await canUseTool({ name: 'TestTool' }, {}, {}, {}, 'sync-response-id', undefined); console.log(JSON.stringify(result));"

Current output:

{"behavior":"deny","message":"fallback"}

Expected behavior would be allow. The fix should be to register the pending permission before emitting onPermissionRequest, then race the already-created pending promise against the timeout. Please add a regression test for the synchronous/immediate host response path, since the existing tests cover late response and timeout safety but not this registration-order case.

What I checked:

• Current head c725c48
• tests/sdk/permissions.test.ts, especially the timeout/late-response tests
• src/entrypoints/sdk/permissions.ts request/registration order
• bun test tests/sdk/permissions.test.ts tests/sdk/shared-utils.test.ts tests/sdk/casing.test.ts
• bun run build

The SDK shape is moving in the right direction, but this one is a real permission-flow blocker.

[emsanakhchivan]

Thanks for the detailed follow-up. I did a targeted re-review of the current head, focused on the SDK permission flow and the tests you pointed to.

Verdict: Needs changes

Blocking issue:

1. createExternalCanUseTool() emits onPermissionRequest(...) before it registers the pending resolver with permissionTarget.registerPendingPermission(toolUseID). That means a host that responds synchronously/immediately from the permission request callback can lose the response because pendingPermissionPrompts does not contain the tool use yet. The request then waits until timeout and denies/falls back even though the host allowed it.

Minimal repro against current head c725c48:

bun --eval "const m = await import('./src/entrypoints/sdk/permissions.ts'); const target = m.createPermissionTarget(); const canUseTool = m.createExternalCanUseTool(undefined, async () => ({ behavior: 'deny', message: 'fallback' }), target, (message) => { const pending = target.pendingPermissionPrompts.get(message.tool_use_id); if (pending) pending.resolve({ behavior: 'allow' }); }, undefined, 5); const result = await canUseTool({ name: 'TestTool' }, {}, {}, {}, 'sync-response-id', undefined); console.log(JSON.stringify(result));"

Current output:

{"behavior":"deny","message":"fallback"}

Expected behavior would be allow. The fix should be to register the pending permission before emitting onPermissionRequest, then race the already-created pending promise against the timeout. Please add a regression test for the synchronous/immediate host response path, since the existing tests cover late response and timeout safety but not this registration-order case.

What I checked:

• Current head c725c48
• tests/sdk/permissions.test.ts, especially the timeout/late-response tests
• src/entrypoints/sdk/permissions.ts request/registration order
• bun test tests/sdk/permissions.test.ts tests/sdk/shared-utils.test.ts tests/sdk/casing.test.ts
• bun run build

The SDK shape is moving in the right direction, but this one is a real permission-flow blocker.

Thanks for the thorough follow-up.

Fixed in d64a269: registerPendingPermission now runs before onPermissionRequest is emitted, so a host responding synchronously finds the entry immediately.

Added a regression test (permissions.test.ts — "synchronous host response from onPermissionRequest is received") that asserts pending is defined inside the callback and resolves with allow as expected.

[devNull-bootloader]

Took a deeper look into this.
Scope & verification

• I checked the PR diffs and the files changed by this branch. The PR adds src/entrypoints/sdk/* modules, tests under sdk, and modifies several core files (QueryEngine.ts, state.ts, tools.ts, commands.ts, messageFilters.ts, MessageSelector.tsx, REPL.tsx).
• I verified there are no changes to package.json or requirements.txt in the PR (no dependency changes detected in the PR's change list).
• I scanned the PR-modified files for security-sensitive patterns (eval/new Function, child_process/exec/spawn, direct fs writes, process.env assignments, unvalidated path usage). The modified/added SDK files do not introduce those high-risk constructs; the repository already contains child_process and fs usage elsewhere, but not newly in the SDK modules.

Blocking Issues

• Incomplete SDK context isolation (reads only): runWithSdkContext() provides AsyncLocalStorage-based overrides for reads (examples: getSessionId(), getSessionProjectDir(), getCwdState()), but many call sites still mutate the global STATE directly (e.g., regenerateSessionId, switchSession, setCwdState). Because writes still go to the global STATE, SDK-parallel queries that rely on runWithSdkContext() can observe cross-session leakage or race conditions when code mutates STATE. This is a correctness and potential safety issue in production and should be addressed before merging. See the relevant getters/setters in state.ts.

  • Fix options: (a) make setters context-aware (write into AsyncLocalStorage store when present), (b) centralize state mutations behind a scoped API that respects the SDK context, or (c) document and enforce that SDK-isolated execution must not call global setters. Until one of these is applied, parallel SDK usage risks incorrect session/cwd/sessionProjectDir state.

Non-blocking Issues / Recommendations

• Process-global schema cache invalidation: QueryEngine.updateTools() calls clearToolSchemaCache() which clears a process-wide map (QueryEngine.ts → toolSchemaCache.ts). For multi-engine or multi-session hosts this global clear can affect concurrent queries. Recommendation: use per-engine/per-session cache keys or a coordinated invalidation API, or document the global effect.

• request_id vs tool_use_id ambiguity: createExternalCanUseTool() emits a request_id while the pending promise is keyed by toolUseID. Hosts/adapters must correlate these IDs. Suggest using one canonical identifier or documenting the mapping to avoid adapter confusion. See permissions.ts.

• Env-mutex is cooperative: acquireEnvMutex() / releaseEnvMutex() in the SDK shared module protects env-mutation code paths that opt in. This is useful but not enforceable — existing code might mutate process.env without using the mutex. Recommendation: audit and wrap all env-mutation sites or add a linting/PR check to ensure the mutex is used where needed. See shared.ts.

• Node-only runtime surface: The AsyncLocalStorage usage (async_hooks) is Node-specific. If you expect the SDK to run in non-Node JS environments, add feature-guards or document the runtime requirement. See state.ts.

• Hard throws on agent/tool mismatch: QueryEngine.updateTools() validates agent-tool compatibility and throws on mismatch — good for safety but may be too harsh for some SDK-host scenarios. Consider defensive logging or a non-fatal mode for hosts that prefer best-effort behavior. See QueryEngine.ts.

• Casing utility behavior requires documentation: snakeToCamel() special-cases dunder names and trailing underscores (tests confirm behavior). This is intentionally opinionated; add docs so SDK consumers know exact mappings. See casing.ts and tests casing.test.ts.

• Permission race-handling appears robust but recommend integration testing: The PR adds createOnceOnlyResolve() and createPermissionTarget() and includes thorough unit tests that cover timeout vs late-host-response scenarios (permissions.test.ts). The mechanism prevents double-resolve, which mitigates the race. Still, add at least one end-to-end integration test that runs the real host/respond flow and a longer-running smoke test to ensure pending maps are always cleaned up.

• Console warnings in SDK surface: createDefaultCanUseTool() uses console.warn to notify missing permission callbacks. For an SDK library surface, prefer an injectable logger or an event/callback so hosts can control noise.

Security scan summary

• No new uses of eval / new Function / VM creation were added in this PR.
• The PR did not add child process or raw exec/spawn calls in the new SDK modules.
• No direct fs.writeFileSync or other dangerous file writes were introduced by the SDK additions (existing repository files continue to use fs/child_process in native/CLI parts).
• No direct path-join-with-untrusted-input patterns were introduced in the SDK modules; session ID validation (assertValidSessionId) is present in the SDK shared module (shared.ts), which is good to mitigate path traversal when used correctly.

Dependency check

• The PR did not change package.json or requirements.txt. No new external dependencies were introduced according to the PR change listing. I recommend running the full CI matrix locally/remote to confirm there are no hidden dependency or runtime issues.

Suggested next steps

1. Blocker fix: Make write operations context-aware or otherwise prevent global STATE mutations during SDK-isolated execution (see state.ts).
2. Convert the global tool-schema cache invalidation to scoped invalidation or add coordination/notes and add tests that exercise concurrent engine/tool updates.
3. Add an integration test covering the permission request/host-response race in a host-like environment and a memory-leak smoke test for pending permission prompts.
4. Document the AsyncLocalStorage runtime requirement and the casing utility behavior.
5. Consider swapping console.warn calls in SDK-facing code for an injectable logger or non-console event.

[techbrewboss]

I did another pass against current head d64a269 and found two things worth addressing before merge.

Blocking: throwing onPermissionRequest leaks a pending resolver

After registerPendingPermission() was moved before onPermissionRequest(), a throwing host callback now leaves a pending resolver behind in pendingPermissionPrompts.

Minimal repro on current head:

bun --eval "const m = await import('./src/entrypoints/sdk/permissions.ts'); const target = m.createPermissionTarget(); const canUseTool = m.createExternalCanUseTool(undefined, async () => ({ behavior: 'deny', message: 'fallback' }), target, () => { throw new Error('host boom') }, undefined, 5); try { await canUseTool({ name: 'TestTool' }, {}, {}, {}, 'throw-id', undefined); } catch (e) { console.log('threw=' + e.message); } console.log('pending=' + target.pendingPermissionPrompts.has('throw-id'));"

Current output:

threw=host boom
pending=true

Since onPermissionRequest is SDK-host-provided code, this should probably be wrapped so the pending map entry is deleted and the permission flow denies/falls back cleanly. Please add a regression test for a throwing onPermissionRequest callback.

Permission request shape mismatch

The permission request object emitted by createExternalCanUseTool() does not match the SDK generated message contract. coreTypes.generated.ts / coreSchemas.ts define permission_request with required uuid and session_id, but SDKPermissionRequestMessage in shared.ts omits uuid and makes session_id optional, and the current emitted request sends neither.

That means a host or future stream path validating against SDKMessageSchema would reject the SDK’s own permission request. Can we either include uuid + session_id when emitting the request, or adjust the generated schema/types so this callback-only message has one canonical shape?

[emsanakhchivan]

Took a deeper look into this. Scope & verification

• I checked the PR diffs and the files changed by this branch. The PR adds src/entrypoints/sdk/* modules, tests under sdk, and modifies several core files (QueryEngine.ts, state.ts, tools.ts, commands.ts, messageFilters.ts, MessageSelector.tsx, REPL.tsx).
• I verified there are no changes to package.json or requirements.txt in the PR (no dependency changes detected in the PR's change list).
• I scanned the PR-modified files for security-sensitive patterns (eval/new Function, child_process/exec/spawn, direct fs writes, process.env assignments, unvalidated path usage). The modified/added SDK files do not introduce those high-risk constructs; the repository already contains child_process and fs usage elsewhere, but not newly in the SDK modules.

Blocking Issues

• Incomplete SDK context isolation (reads only): runWithSdkContext() provides AsyncLocalStorage-based overrides for reads (examples: getSessionId(), getSessionProjectDir(), getCwdState()), but many call sites still mutate the global STATE directly (e.g., regenerateSessionId, switchSession, setCwdState). Because writes still go to the global STATE, SDK-parallel queries that rely on runWithSdkContext() can observe cross-session leakage or race conditions when code mutates STATE. This is a correctness and potential safety issue in production and should be addressed before merging. See the relevant getters/setters in state.ts.

  • Fix options: (a) make setters context-aware (write into AsyncLocalStorage store when present), (b) centralize state mutations behind a scoped API that respects the SDK context, or (c) document and enforce that SDK-isolated execution must not call global setters. Until one of these is applied, parallel SDK usage risks incorrect session/cwd/sessionProjectDir state.

Non-blocking Issues / Recommendations

• Process-global schema cache invalidation: QueryEngine.updateTools() calls clearToolSchemaCache() which clears a process-wide map (QueryEngine.ts → toolSchemaCache.ts). For multi-engine or multi-session hosts this global clear can affect concurrent queries. Recommendation: use per-engine/per-session cache keys or a coordinated invalidation API, or document the global effect.
• request_id vs tool_use_id ambiguity: createExternalCanUseTool() emits a request_id while the pending promise is keyed by toolUseID. Hosts/adapters must correlate these IDs. Suggest using one canonical identifier or documenting the mapping to avoid adapter confusion. See permissions.ts.
• Env-mutex is cooperative: acquireEnvMutex() / releaseEnvMutex() in the SDK shared module protects env-mutation code paths that opt in. This is useful but not enforceable — existing code might mutate process.env without using the mutex. Recommendation: audit and wrap all env-mutation sites or add a linting/PR check to ensure the mutex is used where needed. See shared.ts.
• Node-only runtime surface: The AsyncLocalStorage usage (async_hooks) is Node-specific. If you expect the SDK to run in non-Node JS environments, add feature-guards or document the runtime requirement. See state.ts.
• Hard throws on agent/tool mismatch: QueryEngine.updateTools() validates agent-tool compatibility and throws on mismatch — good for safety but may be too harsh for some SDK-host scenarios. Consider defensive logging or a non-fatal mode for hosts that prefer best-effort behavior. See QueryEngine.ts.
• Casing utility behavior requires documentation: snakeToCamel() special-cases dunder names and trailing underscores (tests confirm behavior). This is intentionally opinionated; add docs so SDK consumers know exact mappings. See casing.ts and tests casing.test.ts.
• Permission race-handling appears robust but recommend integration testing: The PR adds createOnceOnlyResolve() and createPermissionTarget() and includes thorough unit tests that cover timeout vs late-host-response scenarios (permissions.test.ts). The mechanism prevents double-resolve, which mitigates the race. Still, add at least one end-to-end integration test that runs the real host/respond flow and a longer-running smoke test to ensure pending maps are always cleaned up.
• Console warnings in SDK surface: createDefaultCanUseTool() uses console.warn to notify missing permission callbacks. For an SDK library surface, prefer an injectable logger or an event/callback so hosts can control noise.

Security scan summary

• No new uses of eval / new Function / VM creation were added in this PR.
• The PR did not add child process or raw exec/spawn calls in the new SDK modules.
• No direct fs.writeFileSync or other dangerous file writes were introduced by the SDK additions (existing repository files continue to use fs/child_process in native/CLI parts).
• No direct path-join-with-untrusted-input patterns were introduced in the SDK modules; session ID validation (assertValidSessionId) is present in the SDK shared module (shared.ts), which is good to mitigate path traversal when used correctly.

Dependency check

• The PR did not change package.json or requirements.txt. No new external dependencies were introduced according to the PR change listing. I recommend running the full CI matrix locally/remote to confirm there are no hidden dependency or runtime issues.

Suggested next steps

1. Blocker fix: Make write operations context-aware or otherwise prevent global STATE mutations during SDK-isolated execution (see state.ts).
2. Convert the global tool-schema cache invalidation to scoped invalidation or add coordination/notes and add tests that exercise concurrent engine/tool updates.
3. Add an integration test covering the permission request/host-response race in a host-like environment and a memory-leak smoke test for pending permission prompts.
4. Document the AsyncLocalStorage runtime requirement and the casing utility behavior.
5. Consider swapping console.warn calls in SDK-facing code for an injectable logger or non-console event.

Thank you for the thorough review @devNull-bootloader . I've addressed all issues raised:

Blocking Issue: STATE Write Isolation ✅ Fixed

Problem: runWithSdkContext() provided AsyncLocalStorage-based context for reads, but setters (regenerateSessionId, switchSession, setCwdState, setOriginalCwd) directly mutated global STATE.

Fix (commit 380fab3): All four setters now check getSdkContext() before writing. When context exists, writes go to the context object; otherwise, they write to global STATE (preserving backward compatibility).

Tests (commit b2e5981): Added tests/sdk/sdk-context-isolation.test.ts with 10 tests covering:

• Setters in/out of SDK context
• Parallel async contexts (no state leakage)
• End-to-end parallel session scenario

Non-Blocking Issues ✅ Addressed

1. Tool Schema Cache Global Clear (commit 2abd87b)

Replaced clearToolSchemaCache() in QueryEngine.updateTools() with selective invalidation via invalidateRemovedToolSchemas(validToolNames). Only removes entries for tools no longer in the set, preserving cached schemas for tools that remain. auth.ts and logout.tsx continue to use full clear (correct for auth/logout scenarios).

2. request_id vs tool_use_id Documentation (commit 543c4e1)

Added detailed JSDoc in shared.ts explaining:

• request_id: UUID for response correlation, passed to respondToPermission()
• tool_use_id: Instance identifier for tracking, used internally
• Both IDs present in every permission request message

3. Env-mutex Clarification (commit 543c4e1)

Documented that SDK itself does not mutate process.env; mutex is a utility for hosts who need per-query env modifications. Added usage example.

4. Console.warn → Injectable Logger (commit 543c4e1)

Added SDKLogger interface with warn() method. Both createExternalCanUseTool() and createDefaultCanUseTool() accept optional logger parameter, defaulting to console.warn if not provided.

5. Node-only AsyncLocalStorage Documentation (commit 543c4e1)

Documented runtime requirement in state.ts: requires Node.js 12.17.0+ or 14.0.0+ (async_hooks module).

Verification

All 88 SDK tests pass. Independent verification confirmed:

• All setters context-aware ✅
• Selective cache invalidation correct ✅
• No state leakage in concurrent contexts ✅
• Documentation present ✅
• TypeScript errors pre-existing (not introduced) ✅

Commits Summary

Commit	Description
380fab3	fix(sdk): make state setters context-aware for SDK isolation
b2e5981	test(sdk): add context-aware state isolation tests
2abd87b	fix(sdk): selective tool schema cache invalidation
543c4e1	docs(sdk): address non-blocking documentation and logging issues

Please re-review when you have time. Thank you.

[emsanakhchivan]

I did another pass against current head d64a269 and found two things worth addressing before merge.

Blocking: throwing onPermissionRequest leaks a pending resolver

After registerPendingPermission() was moved before onPermissionRequest(), a throwing host callback now leaves a pending resolver behind in pendingPermissionPrompts.

Minimal repro on current head:

bun --eval "const m = await import('./src/entrypoints/sdk/permissions.ts'); const target = m.createPermissionTarget(); const canUseTool = m.createExternalCanUseTool(undefined, async () => ({ behavior: 'deny', message: 'fallback' }), target, () => { throw new Error('host boom') }, undefined, 5); try { await canUseTool({ name: 'TestTool' }, {}, {}, {}, 'throw-id', undefined); } catch (e) { console.log('threw=' + e.message); } console.log('pending=' + target.pendingPermissionPrompts.has('throw-id'));"

Current output:

threw=host boom
pending=true

Since onPermissionRequest is SDK-host-provided code, this should probably be wrapped so the pending map entry is deleted and the permission flow denies/falls back cleanly. Please add a regression test for a throwing onPermissionRequest callback.

Permission request shape mismatch

The permission request object emitted by createExternalCanUseTool() does not match the SDK generated message contract. coreTypes.generated.ts / coreSchemas.ts define permission_request with required uuid and session_id, but SDKPermissionRequestMessage in shared.ts omits uuid and makes session_id optional, and the current emitted request sends neither.

That means a host or future stream path validating against SDKMessageSchema would reject the SDK’s own permission request. Can we either include uuid + session_id when emitting the request, or adjust the generated schema/types so this callback-only message has one canonical shape?

@techbrewboss Thanks for the thorough review. Both blocking issues have been addressed.

---

Issue 1: Throwing onPermissionRequest leaks a pending resolver

Fixed. The onPermissionRequest call is now wrapped in try-catch. On throw, the pending map entry is deleted and the flow returns a deny decision cleanly.

Verification with your minimal repro now shows:

• pending=false (previously was pending=true)

Regression test added: throwing onPermissionRequest cleans up pending resolver and denies in tests/sdk/permissions.test.ts

---

Issue 2: Permission request shape mismatch

Fixed. SDKPermissionRequestMessage now includes required uuid and session_id fields matching the generated schema.

Changes made:

• Updated type in shared.ts to make both fields required
• Added sessionId parameter to createExternalCanUseTool()
• Emit now sends uuid: randomUUID() and session_id: sessionId ?? ''

Regression test added: permission request message includes uuid and session_id matching schema verifies the emitted message shape.

---

Verification summary:

• 29 permissions tests pass (2 new regression tests added)
• 59 SDK tests pass total
• Build passes
• Your minimal repro confirmed fixed

---

Commit: e380af7 - fix(sdk): handle throwing onPermissionRequest and fix permission request shape

Please re-review when you have time. Thank you.

[Vasanthdev2004]

Thanks for the follow-up fixes. I did a targeted re-review of current head 69029f2.

Verdict: Needs changes

What looks fixed:

• The synchronous onPermissionRequest response race is fixed. The pending resolver is registered before the callback is emitted, and the regression test passes.
• Throwing onPermissionRequest now cleans up the pending resolver and denies cleanly.
• permission_request now includes uuid and session_id, and the no-session placeholder is clearer than an empty string.
• The focused SDK tests and build pass locally.

Remaining blocker:

1. SDK context isolation still leaks through parentSessionId. runWithSdkContext() now scopes sessionId, sessionProjectDir, cwd, and originalCwd, but regenerateSessionId({ setCurrentAsParent: true }) still writes STATE.parentSessionId globally, and getParentSessionId() always reads the global value. That means one SDK context can overwrite parent-session metadata for another context.

Minimal repro on current head:

bun --eval "const s = await import('./src/bootstrap/state.ts'); s.runWithSdkContext({ sessionId: '11111111-1111-4111-8111-111111111111', sessionProjectDir: null, cwd: 'C:/a', originalCwd: 'C:/a' }, () => { s.regenerateSessionId({ setCurrentAsParent: true }); }); const afterA = s.getParentSessionId(); s.runWithSdkContext({ sessionId: '22222222-2222-4222-8222-222222222222', sessionProjectDir: null, cwd: 'C:/b', originalCwd: 'C:/b' }, () => { s.regenerateSessionId({ setCurrentAsParent: true }); }); const afterB = s.getParentSessionId(); console.log(JSON.stringify({ afterA, afterB }));"

Current output:

{"afterA":"11111111-1111-4111-8111-111111111111","afterB":"22222222-2222-4222-8222-222222222222"}

That second context mutates the process-global parent session state. Since the stated goal is SDK-parallel isolation, parentSessionId should either be included in the SDK context and read/written context-locally, or the PR should explicitly prove/document that SDK-isolated execution cannot hit the parent-session path.

Verification run locally:

• bun test tests/sdk/permissions.test.ts tests/sdk/shared-utils.test.ts tests/sdk/casing.test.ts tests/sdk/sdk-context-isolation.test.ts tests/sdk/tool-schema-cache.test.ts passed 75/75
• bun run build passed

Happy to re-review once the parent-session state path is scoped or otherwise ruled out.

[techbrewboss]

Thanks for the quick fixes. I re-reviewed current head 69029f2 and confirmed the two issues from my comment are addressed: throwing onPermissionRequest now cleans up/denies cleanly, and permission_request now matches the SDK message shape with uuid + session_id / the explicit no-session placeholder.

I also ran the focused SDK tests against the PR head and they pass locally: 75 pass, 0 fail.

Other than the remaining parentSessionId context-isolation blocker called out in the review above, this looks good to merge from my side.

[emsanakhchivan]

Thanks for the follow-up fixes. I did a targeted re-review of current head 69029f2.

Verdict: Needs changes

What looks fixed:

• The synchronous onPermissionRequest response race is fixed. The pending resolver is registered before the callback is emitted, and the regression test passes.
• Throwing onPermissionRequest now cleans up the pending resolver and denies cleanly.
• permission_request now includes uuid and session_id, and the no-session placeholder is clearer than an empty string.
• The focused SDK tests and build pass locally.

Remaining blocker:

1. SDK context isolation still leaks through parentSessionId. runWithSdkContext() now scopes sessionId, sessionProjectDir, cwd, and originalCwd, but regenerateSessionId({ setCurrentAsParent: true }) still writes STATE.parentSessionId globally, and getParentSessionId() always reads the global value. That means one SDK context can overwrite parent-session metadata for another context.

Minimal repro on current head:

bun --eval "const s = await import('./src/bootstrap/state.ts'); s.runWithSdkContext({ sessionId: '11111111-1111-4111-8111-111111111111', sessionProjectDir: null, cwd: 'C:/a', originalCwd: 'C:/a' }, () => { s.regenerateSessionId({ setCurrentAsParent: true }); }); const afterA = s.getParentSessionId(); s.runWithSdkContext({ sessionId: '22222222-2222-4222-8222-222222222222', sessionProjectDir: null, cwd: 'C:/b', originalCwd: 'C:/b' }, () => { s.regenerateSessionId({ setCurrentAsParent: true }); }); const afterB = s.getParentSessionId(); console.log(JSON.stringify({ afterA, afterB }));"

Current output:

{"afterA":"11111111-1111-4111-8111-111111111111","afterB":"22222222-2222-4222-8222-222222222222"}

That second context mutates the process-global parent session state. Since the stated goal is SDK-parallel isolation, parentSessionId should either be included in the SDK context and read/written context-locally, or the PR should explicitly prove/document that SDK-isolated execution cannot hit the parent-session path.

Verification run locally:

• bun test tests/sdk/permissions.test.ts tests/sdk/shared-utils.test.ts tests/sdk/casing.test.ts tests/sdk/sdk-context-isolation.test.ts tests/sdk/tool-schema-cache.test.ts passed 75/75
• bun run build passed

Happy to re-review once the parent-session state path is scoped or otherwise ruled out.

@Vasanthdev2004 Thanks for the thorough review and the clean repro — that made the fix straightforward.

parentSessionId context isolation is now addressed in eaea430.

What changed

parentSessionId was the only session-scoped field missing from the SdkContext type. The fix follows the same pattern already used for sessionId, sessionProjectDir, cwd, and originalCwd:

1. SdkContext type — added parentSessionId?: SessionId as an optional field.

2. regenerateSessionId({ setCurrentAsParent: true }) — writes to ctx.parentSessionId when an SDK context is active, otherwise falls through to STATE.parentSessionId (unchanged CLI behavior).

3. getParentSessionId() — reads from the active context first via an explicit if (ctx) check. This deliberately avoids the ?? fallback pattern (ctx?.parentSessionId ?? STATE.parentSessionId) because that would leak the global value into a context that never set its own parentSessionId.

Why if (ctx) instead of ??

ctx?.parentSessionId is undefined in two cases: (a) no active context, or (b) context exists but parentSessionId was never set. With ??, case (b) would silently fall through to STATE.parentSessionId, which is exactly the cross-context leak we're fixing. The explicit if ensures each context gets undefined unless it explicitly called regenerateSessionId({ setCurrentAsParent: true }).

Verification

Your repro script now produces {} (global STATE.parentSessionId stays clean) instead of the previous {"afterA":"1111...","afterB":"2222..."} cross-contamination.

bun test tests/sdk/ → 79 pass, 0 fail (was 75, +4 new parentSessionId tests)
bun run build → clean

New tests cover: sequential context isolation, parallel context isolation, non-SDK CLI path regression, and a direct reproduction of your repro scenario.

Happy to address anything else you spot.

[Vasanthdev2004]

Thanks for the follow-up. I did a targeted re-review of current head eaea430, focused on the SDK context-isolation blocker I raised earlier plus the permission-flow fixes already discussed above.

Verdict: Approve-ready from my side

What I checked:

• parentSessionId is now included in the SDK context and read with an explicit active-context check, so an SDK context that has no parent session does not fall through to the global parent session.
• regenerateSessionId({ setCurrentAsParent: true }) now writes parent-session state into the active SDK context instead of process-global STATE.
• The previous synchronous permission response and throwing onPermissionRequest paths remain covered by tests.
• The exact repro from my previous review now prints {}, confirming the global parent-session state is not mutated by the SDK contexts.

Verification run locally:

• parent-session repro command passed with {} output
• bun test tests/sdk/permissions.test.ts tests/sdk/shared-utils.test.ts tests/sdk/casing.test.ts tests/sdk/sdk-context-isolation.test.ts tests/sdk/tool-schema-cache.test.ts passed 79/79
• bun run build passed

I do not see a remaining blocker from my side on the current head. If another maintainer still has a separate SDK-surface concern, we should respect that, but my requested-change item is resolved.

[emsanakhchivan]

@kevincodex1 @gnanam1990 Would you mind taking a look at this when you get a chance? Thanks!

[gnanam1990]

Thanks for the iteration depth here — the back-and-forth with @techbrewboss and @devNull-bootloader has substantially hardened this PR. Did a focused re-review against eaea430:

Verified locally

• Pulled the branch, bun install, ran bun test tests/sdk → 95 pass / 0 fail
• Walked through the cumulative deltas since my last comment:
  • registerPendingPermission() now runs before onPermissionRequest() is emitted (693b112) — fixes the synchronous-host-response race
  • Throwing onPermissionRequest cleans up cleanly (e380af7)
  • permission_request carries uuid + session_id / no-session placeholder (4b38af6)
  • parentSessionId is now per-SDK-context with explicit if-else (no ?? undefined fallback) — eaea430 looks right, and the parallel-context test is exactly the right shape

No openclaude red flags

• No tengu_*, no USER_TYPE === 'ant', no new outbound network calls in 3P paths, no Anthropic fingerprints, no telemetry deps added.

Why approving despite scope
This started as a 1.6k-line PR I wasn't comfortable single-passing. With two additional reviewers (@techbrewboss, @devNull-bootloader) running independent passes and every blocker landed in fix-up commits with regression tests, the safety margin is now where it needs to be.

LGTM. Recommend @kevincodex1 also gives this a sanity-check before merge given the SDK-runtime surface — happy to do a final pass after that if anything new comes up. 🚀

[kevincodex1]

this looks good to me. merging now.