Skip to content

feat: add Claude Code subprocess LLM auth mode (auth: "claude-code")#428

Closed
hoh-dev-bot wants to merge 25 commits intoCortexReach:masterfrom
hoh-dev-bot:feat/claude-code-llm-auth
Closed

feat: add Claude Code subprocess LLM auth mode (auth: "claude-code")#428
hoh-dev-bot wants to merge 25 commits intoCortexReach:masterfrom
hoh-dev-bot:feat/claude-code-llm-auth

Conversation

@hoh-dev-bot
Copy link
Copy Markdown

Summary

Add auth: "claude-code" to LlmClientConfig, enabling memory extraction via a local Claude Code subprocess instead of a remote HTTP endpoint.

Motivation

Users running Claude Code locally can use auth: "claude-code" to skip API key configuration entirely — the subprocess inherits ambient Claude Code auth (CLI subscription billing, ANTHROPIC_AUTH_TOKEN, or ANTHROPIC_API_KEY). No custom proxy or API endpoint needed.

This mirrors the approach used by claude-mem's SDKAgent.

Changes

src/llm-client.ts

  • Add auth: "claude-code" to LlmClientConfig union
  • Add claudeCodePath and stateDir config fields
  • Implement createClaudeCodeClient():
    • Dynamic import of @anthropic-ai/claude-agent-sdk (optional dep) with clear error if not installed
    • buildClaudeCodeEnv() — sanitizes process.env: strips CLAUDECODE* / CLAUDE_CODE_* vars to prevent nested-session errors; preserves ANTHROPIC_AUTH_TOKEN and CLAUDE_CODE_OAUTH_TOKEN for subscription billing; only strips ambient ANTHROPIC_API_KEY when an explicit key is configured (CI-safe)
    • resolveClaudeExecutable()which/where fallback + config override
    • Isolated cwd under stateDir/claude-code-sessions to keep memory-agent sessions out of user's claude history
    • Full disallowedTools list (Bash, Read, Write, Edit, etc.)
    • Collects final output from type: "result" message; falls back to last assistant message
    • Same JSON extraction + repair heuristics as other client types

index.ts

  • Extend auth type union with "claude-code"; add claudeCodePath field with JSDoc
  • Extract resolveLlmClientConfig() helper (eliminates duplicate config-resolution logic)
  • Default model falls back to "claude-sonnet-4-5" for claude-code (matching claude-mem)

package.json

  • Add @anthropic-ai/claude-agent-sdk as optionalDependencies

test/llm-claude-code-client.test.mjs (new)

  • 13 unit tests covering:
    • buildClaudeCodeEnv: ambient key preservation, explicit key override, CLAUDECODE strip, prefix strip, preserve list, auth warning
    • extractJsonFromResponse + repairCommonJson
    • Client instantiation

Usage

{
  "llm": {
    "auth": "claude-code",
    "model": "claude-sonnet-4-5"
  }
}

Prerequisites:

  • Claude Code installed: npm i -g @anthropic-ai/claude-code
  • SDK installed: npm i @anthropic-ai/claude-agent-sdk

Backward Compatibility

  • auth: "api-key" (default) and auth: "oauth" behavior is unchanged
  • @anthropic-ai/claude-agent-sdk is optional — users not using claude-code mode are unaffected

hohsiang Dev and others added 19 commits March 31, 2026 18:48
Add auth: 'claude-code' option that spawns a local Claude Code subprocess
via @anthropic-ai/claude-agent-sdk instead of calling a remote HTTP endpoint.
This enables memory extraction without any API key or custom proxy — it uses
ambient Claude Code authentication (CLI subscription or ANTHROPIC_API_KEY).

Changes:
- src/llm-client.ts:
  - Add 'claude-code' to LlmClientConfig.auth union
  - Add claudeCodePath and stateDir config fields
  - Implement createClaudeCodeClient() with:
    - Dynamic import of @anthropic-ai/claude-agent-sdk (optional dep)
    - buildClaudeCodeEnv(): sanitizes process.env, strips ANTHROPIC_API_KEY /
      CLAUDECODE* to prevent nested-session errors, preserves ANTHROPIC_AUTH_TOKEN
      and CLAUDE_CODE_OAUTH_TOKEN for subscription billing
    - resolveClaudeExecutable(): which/where fallback + configuredPath support
    - Isolated cwd under stateDir/claude-code-sessions to keep memory agent
      sessions out of user's claude history
    - disallowedTools list (Bash, Read, Write, Edit, WebFetch, etc.)
    - Same JSON extraction + repair heuristics as other client types
- index.ts:
  - Extend auth type union, add claudeCodePath field with JSDoc
  - Wire auth: 'claude-code' in both createLlmClient call sites
  - Default model falls back to 'claude-sonnet-4-5' (matching claude-mem)
- package.json: move @anthropic-ai/claude-agent-sdk to optionalDependencies
- test/llm-claude-code-client.test.mjs: unit tests for extraction logic

Usage:
  llm:
    auth: claude-code
    model: claude-sonnet-4-5   # optional, default: claude-sonnet-4-5
- ANTHROPIC_API_KEY handling: only strip ambient key when an explicit key
  is configured (llm.apiKey). Previously the ambient key was always stripped,
  which silently broke auth in environments (e.g. CI) that rely on it.
  Add warning log when no auth source is detected at all.
- buildClaudeCodeEnv: export for testability; accept optional log parameter
- mkdirSync: only ignore EEXIST, surface real errors (e.g. EACCES) instead
  of silently swallowing them
- Subprocess result collection: collect text from all assistant messages and
  prefer the final 'result' message (aggregated output) over breaking on the
  first assistant message. Improves robustness against SDK streaming changes.
- index.ts: extract resolveLlmClientConfig() helper to eliminate duplicate
  config-resolution logic across two createLlmClient() call sites
- Tests: expand from 4 to 13 tests; add full coverage for buildClaudeCodeEnv
  env sanitization (ambient key preservation, explicit key override, strip
  logic, auth warning), JSON extraction helpers, and repair heuristics
…lient

- [critical] Detect SDKResultError subtypes (error_during_execution,
  error_max_turns, error_max_budget_usd) and surface errors[] instead
  of silently falling through to "empty response" log
- [critical] Remove misleading EEXIST guard after mkdirSync({recursive:true});
  recursive mkdir never throws EEXIST, comment was dead/misleading code
- [important] Distinguish MODULE_NOT_FOUND from other import failures so
  ABI mismatches or corrupt packages don't say "SDK not installed"
- [important] Add AbortController-based timeout to the for-await SDK stream
  loop so timeoutMs config actually applies to claude-code mode
- [important] Cache resolved claude executable path per client instance to
  avoid forking a shell on every completeJson() call
- [important] Log OAuth JSON parse failures with body preview so response
  body content is not permanently lost on non-SSE error responses

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- [critical] Add existsSync check for llm.claudeCodePath before passing to
  subprocess; user-configured binary path is now validated at resolve time
- [critical] Add llm-claude-code-client.test.mjs to CI test script in package.json
- [important] Cache claude path resolution failure so repeated completeJson
  calls don't re-fork a shell after a permanent failure
- [important] Distinguish AbortError (timeout) from other errors in the outer
  catch so lastError messages include 'timed out after Nms' on timeout
- [important] Include original execSync error reason in resolveClaudeExecutable
  throw so callers see permission/PATH diagnostics not just a generic message
- [tests] Add behavior tests: claudeCodePath not found sets lastError, and
  subsequent calls use cached failure without re-running execSync
- [style] Remove unused before/after imports from test file

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- [critical] Log LLM client init failure in index.ts CLI path instead of
  silently swallowing the error with a bare catch block
- [important] Route WARNING-prefixed log messages to api.logger.warn so
  'no auth source' warnings are visible in production logs, not buried
  at debug level
- [important] Store cachedClaudePathError without a per-call label so
  replayed errors don't carry a stale label from the first failing call
- [important] Cache SDK import failure to avoid retrying a broken/missing
  module on every completeJson call
- [important] Fix Windows executable lookup: use 'where claude' not
  'where claude.cmd' — 'where' already searches all PATHEXT extensions

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t log

The refactoring that introduced resolveLlmClientConfig left two log
statements in index.ts still referencing the old local variables
llmModel and llmTimeoutMs which no longer exist. This caused a
ReferenceError on every plugin init, silently caught and reported as
'smart extraction init failed, falling back to regex'.

Fix: assign resolveLlmClientConfig() result to resolvedLlmConfig and
use resolvedLlmConfig.model / resolvedLlmConfig.timeoutMs in the log.

Also remove the redundant '|| "claude-sonnet-4-5"' fallback in
createClaudeCodeClient since LlmClientConfig.model is a required field.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…kError, use accessSync

- Cache successful SDK import in cachedQueryFn so repeated completeJson
  calls don't re-await dynamic import on every invocation
- Fix cachedSdkError label: store bare message and inject current label
  on replay, matching the cachedClaudePathError pattern to avoid stale
  labels in log messages from subsequent calls with different labels
- Use accessSync(path, X_OK) instead of existsSync for claudeCodePath
  validation so non-executable files are rejected at config time rather
  than producing cryptic subprocess errors
- Hoist CLAUDE_CODE_DISALLOWED_TOOLS to module level (static constant)
  to avoid reallocating the array on every completeJson call

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…LLM client

- Add logWarn to LlmClientConfig; route all lastError/failure logs through
  logWarn so claude-code runtime errors appear at warn level in production
- Remove WARNING string-sniffing from index.ts log routing (fragile coupling)
- Fix resolveClaudeExecutable to include accessSync error reason (CRITICAL-1)
- Fix execSync output to take first line only for Windows where multi-results (CRITICAL-2)
- Remove unused existsSync import from llm-client.ts
- Hoist resolvedLlmClient in index.ts; reuse for CLI instead of creating a
  second LlmClient instance (eliminates duplicate failure point)
- Fix noiseBank.init catch to log at warn level with err.message instead of String(err)
- Fix smart extraction init catch to include err.stack for better diagnostics

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Commit 494f7e0 removed the duplicate memory_compact tool registration
but left behind the closing `}` of the if (config.enableManagementTools)
block, causing a ParseError that prevented index.ts from loading.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Verify logWarn routing: errors go to logWarn, not log, when both provided
- Verify accessSync failure message includes system error reason (CRITICAL-1)
- Verify buildClaudeCodeEnv routes no-auth warning through logWarn (HIGH-3)
- Add MCP_SESSION_ID stripping test (previously untested strip exact key)
- Add CLAUDE_CODE_GIT_BASH_PATH preservation test (previously untested preserve key)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ests

- Fix resolveClaudeExecutable to throw when which/where returns empty string
  instead of silently caching "" as the claude path (MEDIUM-1)
- Add test: ANTHROPIC_AUTH_TOKEN presence suppresses no-auth warning
- Add test: createLlmClient throws synchronously for api-key without apiKey
- Add test: createLlmClient throws synchronously for oauth without oauthPath

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- If config.stateDir is empty/root, fall back to ~/.openclaw/memory-lancedb-pro
- Fixes: 'failed to create session dir /claude-code-sessions' error
- Load ANTHROPIC_AUTH_TOKEN + ANTHROPIC_BASE_URL from ~/.claude/settings.json
- Merge with ambient process.env (settings.json has priority)
- Fixes: 'Not logged in' error when using auth: claude-code with local OAuth
….env

- Warning should check the final env object (includes settings.json tokens)
- Prevents false-positive 'no auth found' warnings when token exists in ~/.claude/settings.json
- Load ANTHROPIC_AUTH_TOKEN + ANTHROPIC_BASE_URL from Claude Code's settings.json
- Merge with ambient process.env (settings.json values take precedence)
- Check constructed env object for auth warning instead of process.env
- Remove duplicate 'warn' variable declaration (ParseError fix)
- Fixes: 'Not logged in' error when using auth: claude-code with OAuth
- settings.json: apply shouldStripClaudeCodeEnvKey() filter to prevent nested session bypass
- settings.json: add runtime type check for env field (object, not array)
- settings.json: log SyntaxError/permission errors instead of silently swallowing
- tests: make 'no auth' tests resilient to settings.json presence on dev machines
Critical:
- settings.json OAuth tokens now take precedence over stale env vars
- extractJsonFromResponse now handles array JSON (e.g. [{...}]) correctly

Important:
- Log unknown SDK message types for debugging
- Log when falling back to assistant text without result message
- Log when result.result is not a string
- Distinguish SyntaxError/EACCES in settings.json error messages
- Clarify OAuth token impact when settings.json load fails
- Clear resolvedLlmClient on SmartExtractor init failure
- Include auth mode in smart extraction init failure log
@AliceLJY
Copy link
Copy Markdown
Collaborator

Great addition — the claude-code auth mode is well-implemented with solid env sanitization, proper error handling, and good test coverage. The resolveLlmClientConfig and parseJsonWithRepair refactors are welcome cleanups.

A few items before merging:

  1. Unintended memory_compact tool removal: The diff removes the entire memory_compact tool registration block (~80 lines). This isn't mentioned in the PR description — was this intentional? If not, please restore it. (Note: we're also tracking this removal in PR fix: remove duplicate memory_compact tool registration #433, but that PR specifically targets the duplicate registration. The tools.ts canonical version should remain.)

  2. bun.lock inclusion: The project doesn't appear to use Bun as its package manager. Was the bun.lock file intentionally added, or is it a local artifact? If the latter, consider removing it.

  3. Dead code — sessionDir: The mkdirSync(sessionDir, ...) call creates a directory that's never used since cwd is commented out. Either remove the directory creation or add a TODO comment explaining when cwd will be re-enabled.

  4. Minor: extractTextFromSdkMessage uses unsafe type casting — consider a runtime check for robustness against SDK changes.

Everything else looks clean. The env sanitization logic and test coverage are particularly strong. Happy to approve once the memory_compact question is resolved! 🙏

hohsiang Dev added 6 commits April 1, 2026 02:55
- Important CortexReach#6: resolveClaudeExecutable now distinguishes ENOENT/exit-127
  (not-found) from ENOMEM/EACCES (system error) in execSync broad catch
- Important CortexReach#7: stateDir.length <= 1 now logs a logWarn instead of silently
  ignoring the value (e.g. stateDir="/" would previously be silently dropped)
- Important CortexReach#8: add comment explaining CLAUDE_CODE_ENTRYPOINT strip+re-inject
  pattern (strip parent value, re-inject as sdk-ts to avoid nested-session error)
- Important CortexReach#9: test conditional assertions now use settingsPathOverride pointing
  to a nonexistent path so they always execute regardless of ~/.claude/settings.json

Also: expose settingsPathOverride param on buildClaudeCodeEnv for test isolation
… tests

- Critical #1: read OAuth response body once before ok-check to avoid
  second .text() call returning empty string (HTTP body can only be consumed once)
- Add buildClaudeCodeEnv settings.json loading tests using settingsPathOverride:
  * loads ANTHROPIC_API_KEY from settings.json with priority over ambient env
  * does not inject strip-listed keys (CLAUDE_CODE_SESSION) from settings.json
  * warns on settings.json JSON parse error without throwing
- Add stateDir guard test: warns when stateDir='/' and falls back to default
- execSync: distinguish EACCES/EPERM (permission) and ENOSPC (disk full) from not-found
- execSync: preserve stack trace for SDK import errors
- OAuth parse error: throw instead of silently returning null
- buildClaudeCodeEnv: add CLAUDE_CODE_ENV_AUTH_PRIORITY=1 escape hatch for CI
- buildClaudeCodeEnv: document settings.json precedence rationale
- Restore memory_compact tool registration block in index.ts
  (was unintentionally dropped in previous commits; not related to PR CortexReach#433)
- Remove bun.lock (local artifact, project uses npm)
- Re-enable cwd: sessionDir in Claude Code subprocess options
  (isolated cwd is ready; OAuth auth issue was resolved)
- Replace unsafe type casts in extractTextFromSdkMessage with
  explicit runtime property checks for robustness against SDK changes
…or on non-string result

- envAuthPriority was read from process.env on every iteration of the
  Object.entries(process.env) loop; hoist to a const before the loop
- result.result is not a string path set lastError but the generic
  empty-response check below would overwrite it; now only sets the
  generic message if lastError is not already populated
…SDK cache, stateDir default

- CLAUDE_CODE_ENV_AUTH_PRIORITY=1: env var wins over settings.json auth key
- CLAUDE_CODE_ENV_AUTH_PRIORITY unset: settings.json wins (default behavior)
- extractTextFromSdkMessage: block array, plain string, null content, non-object, unknown types
- SDK cache: second completeJson call returns identical cached error, no retry
- Export extractTextFromSdkMessage to allow direct unit testing

37/37 tests pass
@hoh-dev-bot
Copy link
Copy Markdown
Author

Thanks for the thorough review! All four points addressed:

1. memory_compact tool removal
Confirmed unintentional — restored the full registration block in index.ts. The removal was a rebase artifact. The canonical version in tools.ts was always intact; index.ts now correctly delegates via registerAllMemoryTools.

2. bun.lock removed
Local artifact from a bun install run — deleted from the branch.

3. sessionDir / dead code
Restored cwd: sessionDir — the isolated cwd is now active. The auth issue that caused the original disable has been resolved (settings.json env loading now correctly threads OAuth tokens into the subprocess env).

4. extractTextFromSdkMessage type safety
Replaced all unsafe casts with explicit runtime property checks starting from unknown. No more structural type assertions.

Bonus fixes from follow-up review:

  • envAuthPriority hoisted out of the Object.entries loop (was re-reading process.env on every iteration)
  • lastError from result.result is not a string path no longer overwritten by the generic empty-response message
  • Test coverage expanded to 37 tests: added CLAUDE_CODE_ENV_AUTH_PRIORITY priority override, extractTextFromSdkMessage (7 cases), and SDK module cache behavior

@rwmjhb
Copy link
Copy Markdown
Collaborator

rwmjhb commented Apr 1, 2026

Review: CLOSE

The feature idea is sound — reusing ambient Claude Code auth removes real friction for local users. However, this PR has significant quality concerns that make it unmergeable in current form:

  1. Bot-generated with 25 commits and 4+ rounds of AI review-fix cycles — the iterative fix pattern suggests the implementation was not well understood by the author, reducing confidence in correctness.

  2. 5 unrelated changes bundled in — OAuth body-read fix, extractJsonFromResponse rewrite, logWarn parameter refactor, noise-bank log level change, and OAuth error handling change are all significant behavioral modifications to existing code paths that have nothing to do with claude-code auth.

  3. Test suite depends on optional dependency@anthropic-ai/claude-agent-sdk is in optionalDependencies but the new test file is in the default npm test script, breaking CI for contributors without it.

If this feature is wanted, it should be reimplemented as a focused PR: just the claude-code auth mode + its tests, without bundled refactors to existing client code.

@rwmjhb
Copy link
Copy Markdown
Collaborator

rwmjhb commented Apr 1, 2026

Closing: bot-generated PR with quality concerns. See review comment above.

@rwmjhb rwmjhb closed this Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants