feat: integrate Claude native SDK main and worker runtime - #545
feat: integrate Claude native SDK main and worker runtime#545lidge-jun wants to merge 32 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…tive-activity-claude # Conflicts: # devlog
# Conflicts: # devlog
…odex/native-activity-claude
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f34b5bd67d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const prepared: PreparedClaudeOptions = { cwd: spawnCwd || process.cwd(), binary: detected.path || 'claude', | ||
| env: spawnEnv, model: runtimeModel, systemPrompt: sysPrompt, permissions: permissions as PreparedClaudeOptions['permissions'], | ||
| fastMode: cfg.fastMode === true, ...(effort ? { effort: effort as PreparedClaudeOptions['effort'] } : {}) }; |
There was a problem hiding this comment.
Preserve deny mode for native Claude memory flushes
When perCli.claude.transport is native and the memory CLI resolves to Claude, runFlush calls spawnAgent with permissions: 'deny' (src/agent/memory-flush-controller.ts:418). This cast forwards that value unchanged, but validatePrepared accepts only auto or safe (claude-sdk-options.ts:46-47), so every such memory flush fails during acquisition before inference and never updates memory. Route deny to a native no-tools policy or keep this internal task on print instead of casting it to a supported type.
AGENTS.md reference: AGENTS.md:L184-L184
Useful? React with 👍 / 👎.
| } }).catch(failure => { | ||
| if (worker && failure instanceof ClaudeAcquireFailure) { | ||
| awaitingUnleasedCleanup = true; | ||
| void failure.cleanup.then(() => { |
There was a problem hiding this comment.
Retain main-run accounting through late acquisition cleanup
When a native Claude main run is cancelled or times out while createSession is still pending, ClaudeAcquireFailure.cleanup remains pending until the late factory result has physically closed. This worker-only condition skips that receipt for main runs, so finalization immediately removes the run control and activeMainProcesses entry; waitForAllProcessesEnd() can consequently report completion while the SDK factory or child is still alive. Track the cleanup receipt for main runs too; only the temporary-directory cleanup needs to remain worker-specific.
AGENTS.md reference: AGENTS.md:L184-L184
Useful? React with 👍 / 👎.
| if (!activeMainProcesses.has(scopeKey) && !hasClaudeRuns(scopeKey)) return Promise.resolve(); | ||
| return new Promise<void>(resolve => { | ||
| const check = setInterval(() => { | ||
| if (!activeMainProcesses.has(scopeKey)) { clearInterval(check); clearTimeout(deadline); resolve(); } | ||
| if (!activeMainProcesses.has(scopeKey) && !hasClaudeRuns(scopeKey)) { clearInterval(check); clearTimeout(deadline); resolve(); } |
There was a problem hiding this comment.
Exclude surviving workers from the main steer wait
When a native Claude worker is running in the same scope as a main turn, steering the main calls cancelClaudeScope(..., includeWorkers=false), intentionally leaving that worker alive, but this predicate waits for every Claude run in the scope. The steer path therefore waits until the unrelated worker finishes or the full 10-second steer timeout expires before starting the replacement turn. Use a main-run-specific predicate for steer waits while retaining the all-runs predicate for shutdown.
AGENTS.md reference: AGENTS.md:L184-L184
Useful? React with 👍 / 👎.
Claude native now runs through the application's main and worker paths, using the existing runtime pool, shared runner and final-delivery lifecycle. Main turns reuse an SDK query and can resume its provider session after disposal. The optional SDK dependency is pinned to 0.3.261 in a separate commit; print remains the default.
Each send captures its jaw identity and owner, projects tools/reasoning/usage, and routes bounded approvals and questions through the existing request API. The host claims an authoritative outcome before durable delivery and finalizes once; Stop preserves interrupted partial output before the exit barrier. Worker IDs and temporary directories stay owned until physical cleanup is confirmed, including late acquisition and close failure. Native errors never replay through print.
Agent/Task calls require explicit
run_in_background:false; explicit background Bash is also rejected by a pre-execution SDK hook. Background continuation needs a separate ownership design. This restriction concerns SDK options and is not an OS sandbox guarantee.Validation: 760 focused tests pass, including application main/worker integration and existing Code/Pi/Cursor regressions, plus root/frontend typecheck, strict-baseline, server build, assets and document counts. Compiled HTTP/SSE probes verified three turns on one real SDK query/process, a safe approval answered through HTTP, three stored final answers, no provisional messaging output and zero owned children after close. Separate authenticated SDK probes covered Stop/resume, deny/allow, questions in safe/auto modes, images, foreground children, background-hook denial and per-turn cost deltas. Independent pool and final application reviews passed. The Windows job includes SDK lifecycle and launch tests.
This draft targets the shared native-activity branch. The published parent
2c3dd0faand test-only followupef1fb23bare normally merged, preserving the prior certified Claude head. Scoped conflict resolutions retain parent pending-input cancellation before Claude callbacks and preserve the Claude fixture firewall. Current head isf34b5bd67d2c00d0086307f5c4e34f9baabd08c1; its focused checks, build and real compiled HTTP/SSE proof passed. Exact-head CI passed: Tests and Postinstall Platform Checks, including all producers and aggregates. The first WSL setup attempt received HTTP403 before application code; failed jobs were rerun successfully on the same SHA. Prior-head evidence remains historical. No PR merge, release or deployment is included.