Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: BytePioneer-AI/codex-host/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (28)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (4)
🧰 Additional context used🪛 ast-grep (0.45.3)packages/adapters/cursor-cli/src/delegation-bridge.ts[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec. (detect-child-process-typescript) packages/host-runtime/test/delegation-skill.test.ts[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec. (detect-child-process-typescript) packages/host-runtime/test/delegation-cli-invocation.test.ts[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec. (detect-child-process-typescript) 🪛 OpenGrep (1.30.0)packages/host-runtime/test/delegation-skill.test.ts[ERROR] 74-74: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead. (coderabbit.command-injection.exec-js) 📝 SummarySummary by CodeRabbit
Walkthrough委派 CLI 现在通过 Host 提供的路径调用。npm 安装可通过指定的 Node 路径启动 CLI。Host Runtime 根据环境生成后续 Thread 命令，委派规范和 Skill 指引也已更新。 Changes委派 CLI 调用
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant HostRuntime
participant DelegationCliEnvironment
participant DelegationCli
HostRuntime->>DelegationCliEnvironment: 选择 CLI 与 Node 路径
DelegationCliEnvironment-->>HostRuntime: 返回子进程环境
HostRuntime->>DelegationCli: 执行委派命令
DelegationCli-->>HostRuntime: 返回 threadId 与 JSON 结果
HostRuntime->>HostRuntime: 生成 Thread read 和 wait 命令
Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable issue is established that would prevent merging after normal checks. Native Windows/Linux and real cross-Harness execution remain unverified, but are not confirmed defects. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change should make delegation use the active installation rather than another CLI on the user's PATH. Ordinary user edits to installed guidance remain protected, but a concurrent edit can be overwritten during an eligible upgrade. No externally reachable attack path was established. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
1 participant
Packaged installations can fail before delegation starts because the Skill invokes a bare
codexhost. A global npm installation can instead select another installation's CLI. Shipped v4 Skills also remain stuck because their digest is missing from the managed upgrade list.This fixes bootstrap and follow-up invocation while preserving the current main branch's watch instructions and user-edited Skill protection.
Changes
next.read/next.waitat the invoking CLI boundary, using its local executable paths rather than a potentially remote Runtime's paths. Quote executable paths and Thread arguments. Missing paths remain explicit environment references; they never fall back to PATH. Compact output and Runtime API shapes stay unchanged.CODEXHOST_CLI_NODE_PATH. Forward the pair through the broker and native delegation surfaces; Cursor executes the pair directly. Native packaged CLI invocation clears a stale Node override.Related PR
Related to #223, which identified the packaged CLI/PATH mismatch and missing v4 digest. This is an alternative implementation against current main, sharing that diagnosis and extending coverage to caller-local follow-ups, an explicit npm Node/script pair, mention and environment guidance, and executable regression tests. It does not require merging #223 first.
Validation
On macOS arm64:
npm run typecheck; changed-file ESLint/Prettier;node tools/check-boundaries.mjs;cargo fmt --all --check;git diff --check: passed.cargo check --locked --package codexhost-launcher: passed.tests/vitest.config.js --maxWorkers=1: 129 tests passed across 11 focused CLI, Skill, Runtime, broker, Cursor, CodeBuddy, WorkBuddy and npm-release files.The pre-existing Cursor shutdown test hit its one-second process-start wait under concurrent build/test load; it passed in isolation and in the complete focused run with one worker. No native Windows/Linux or real-model cross-Harness end-to-end run was performed; Windows coverage here checks command rendering and generated npm launcher behavior.