feat(exetesion-agent): improve computer backends and task wakeups#916
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Walkthrough此 PR 实现了两个主要功能流：(1) 后台代理任务完成时自动唤醒父会话的完整管线，通过快照保存、事件派发、虚拟会话恢复与运行时暂停门控联动实现；(2) 计算机后端重构，移除基于 writableRoots 的作用域约束，改用系统临时目录，同时重写沙箱包装策略与文件存取权限模式，并新增 home 目录查询事件与相应的前端导航更新。 ChangesAgent Task Auto-Wakeup Pipeline
Computer Backend Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces an automatic wakeup feature for background agent tasks upon completion, refactors virtual session creation into a shared utility, and refines the computer agent's local, E2B, and open-terminal backend configurations and UI. Feedback on these changes highlights a critical bug where ctx.bots is incorrectly accessed as a dictionary instead of an array, and a UI issue where the files panel watcher needs to be immediate to load the home directory initially. Additionally, defensive checks for chatChain and improved isolation for temporary directories in the local backend are recommended to enhance stability and security.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/extension-agent/src/computer/backends/local/index.ts (1)
217-229:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
createTerminal()现在能直接在受限目录里启动交互 shell。这里的
cwd不再经过任何本地路径权限校验，而且这个 PTY 也没有经过wrapCommandWithSandbox()。调用方只要把options.cwd指到denyRoots或只读目录，终端就会以该目录启动并直接拿到交互式文件系统访问，这条路径会绕过denyRoots/readOnlyRoots。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/extension-agent/src/computer/backends/local/index.ts` around lines 217 - 229, createTerminal currently accepts an arbitrary options.cwd and launches an interactive shell via resolveInteractiveShellCommand + createLocalTerminal without performing path permission checks or wrapping the PTY in the sandbox, allowing callers to bypass denyRoots/readOnlyRoots; fix by validating and normalizing the requested cwd against the existing denyRoots/readOnlyRoots policy (use the same logic used elsewhere in this module) and then start the terminal using the sandbox wrapper (wrapCommandWithSandbox) or an equivalent sandboxed command path before calling createLocalTerminal; update createTerminal to resolveInteractiveShellCommand(this._cfg), enforce permission checks on options.cwd (fall back to this._cwd when denied), and pass the sandboxed command/env into createLocalTerminal so interactive shells cannot escape read-only/denied roots.
🧹 Nitpick comments (1)
packages/extension-agent/client/components/computer/files-panel.vue (1)
573-594: ⚡ Quick win考虑为异步控制台事件调用添加错误处理。
watch块中的send('chatluna-agent/getComputerHome', ...)调用没有错误处理。如果后端服务未就绪或事件失败，组件可能会中断。文件中其他异步操作（如loadDir、openFile、reloadActiveTab）都使用了try-catch块。为保持一致性并提升鲁棒性，建议添加错误处理：
建议的错误处理
watch( () => props.config.defaultProvider, async () => { + try { rootPath.value = clean( await send('chatluna-agent/getComputerHome', { backend: props.config.defaultProvider }) ) currentDir.value = rootPath.value pathInput.value = rootPath.value search.value = '' items.value = [] tabs.value = [] activeKey.value = '' selectedPath.value = '' history.value = [] historyIndex.value = -1 browserSize.value = 38 await loadDir(rootPath.value, 'push') + } catch { + ElMessage.error('获取根目录失败') + } }, { immediate: true } )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/extension-agent/client/components/computer/files-panel.vue` around lines 573 - 594, The watch handler that calls send('chatluna-agent/getComputerHome') lacks error handling and can break the component if the backend is unavailable; wrap the async body of the watcher in a try-catch (around the send and subsequent state resets and the await loadDir call), handle/log the error (e.g., via console.error or an existing logger) and ensure state remains consistent on failure (avoid clearing critical values or set safe defaults for rootPath, currentDir, pathInput, items, tabs, activeKey, selectedPath, history, historyIndex, browserSize) so watch, send('chatluna-agent/getComputerHome'), loadDir, rootPath and related state updates are guarded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/llm-core/agent/sub-agent.ts`:
- Line 873: 该行在调用 escapeXml(...) 的三元表达式被多余的括号和换行打断导致不符合 Prettier 格式，请在
sub-agent.ts 中定位使用 escapeXml 的三元表达式（查找 escapeXml( 出现处），移除不必要的括号并将三元运算符的条件、? 与 :
和结果放置为 Prettier 推荐的紧凑格式（即不要换行在不必要的位置），确保表达式在一行或按 Prettier 默认换行规则分行以通过格式校验。
In `@packages/extension-agent/src/computer/backends/local/sandbox.ts`:
- Around line 152-157: The current sandbox flags build in sandbox.ts return a
'--bind / /' when cfg.sandboxMode === 'writable', which exposes the entire host
FS; instead, always mount host root read-only (use the ro-bind pattern) and
explicitly add writable mounts only for allowed paths (e.g., the workdir and
tmp), and translate denyRoots/readOnlyRoots into additional bwrap mount flags to
enforce restrictions; update the array construction in the function that returns
the bwrap args (the block using quote(bwrap), cfg.sandboxMode, tmp) so it never
uses '--bind / /' — add an explicit writable '--bind' for the workdir and
convert denyRoots/readOnlyRoots into corresponding '--ro-bind'/'--bind' entries
so execute() and prepareBackgroundCommand() cannot write outside permitted
paths.
---
Outside diff comments:
In `@packages/extension-agent/src/computer/backends/local/index.ts`:
- Around line 217-229: createTerminal currently accepts an arbitrary options.cwd
and launches an interactive shell via resolveInteractiveShellCommand +
createLocalTerminal without performing path permission checks or wrapping the
PTY in the sandbox, allowing callers to bypass denyRoots/readOnlyRoots; fix by
validating and normalizing the requested cwd against the existing
denyRoots/readOnlyRoots policy (use the same logic used elsewhere in this
module) and then start the terminal using the sandbox wrapper
(wrapCommandWithSandbox) or an equivalent sandboxed command path before calling
createLocalTerminal; update createTerminal to
resolveInteractiveShellCommand(this._cfg), enforce permission checks on
options.cwd (fall back to this._cwd when denied), and pass the sandboxed
command/env into createLocalTerminal so interactive shells cannot escape
read-only/denied roots.
---
Nitpick comments:
In `@packages/extension-agent/client/components/computer/files-panel.vue`:
- Around line 573-594: The watch handler that calls
send('chatluna-agent/getComputerHome') lacks error handling and can break the
component if the backend is unavailable; wrap the async body of the watcher in a
try-catch (around the send and subsequent state resets and the await loadDir
call), handle/log the error (e.g., via console.error or an existing logger) and
ensure state remains consistent on failure (avoid clearing critical values or
set safe defaults for rootPath, currentDir, pathInput, items, tabs, activeKey,
selectedPath, history, historyIndex, browserSize) so watch,
send('chatluna-agent/getComputerHome'), loadDir, rootPath and related state
updates are guarded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c1f8f497-f62c-41b2-b0fd-8a5d3f6858b4
⛔ Files ignored due to path filters (3)
packages/core/package.jsonis excluded by!**/*.jsonpackages/core/src/locales/en-US.schema.ymlis excluded by!**/*.ymlpackages/core/src/locales/zh-CN.schema.ymlis excluded by!**/*.yml
📒 Files selected for processing (30)
.gitignorepackages/core/src/config.tspackages/core/src/index.tspackages/core/src/llm-core/agent/sub-agent.tspackages/core/src/llm-core/agent/wakeup.tspackages/core/src/middlewares/conversation/request_conversation.tspackages/core/src/utils/virtual_session.tspackages/extension-agent/client/components/computer/computer-page.vuepackages/extension-agent/client/components/computer/config-backends/backend-e2b.vuepackages/extension-agent/client/components/computer/config-backends/backend-local.vuepackages/extension-agent/client/components/computer/config-backends/backend-open-terminal.vuepackages/extension-agent/client/components/computer/configuration-panel.vuepackages/extension-agent/client/components/computer/files-panel.vuepackages/extension-agent/client/components/computer/status-panel.vuepackages/extension-agent/src/computer/backends/e2b.tspackages/extension-agent/src/computer/backends/local/index.tspackages/extension-agent/src/computer/backends/local/sandbox.tspackages/extension-agent/src/computer/backends/local/security.tspackages/extension-agent/src/computer/backends/local/store.tspackages/extension-agent/src/computer/backends/open_terminal.tspackages/extension-agent/src/computer/types.tspackages/extension-agent/src/config/defaults.tspackages/extension-agent/src/service/computer.tspackages/extension-agent/src/service/index.tspackages/extension-agent/src/service/sub_agent.tspackages/extension-agent/src/sub-agent/session.tspackages/extension-agent/src/trigger/session.tspackages/extension-agent/src/types.tspackages/extension-agent/src/types/computer.tspackages/extension-agent/src/webui/index.ts
💤 Files with no reviewable changes (3)
- packages/extension-agent/src/types/computer.ts
- packages/extension-agent/src/config/defaults.ts
- packages/extension-agent/src/computer/backends/local/security.ts
dingyi222666
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/middlewares/chat/stop_chat.ts`:
- Around line 64-67: 当前实现无条件在 stop_chat 中调用
ctx.parallel('chatluna/chat-stopped', { conversationId: conversation.id, session
})，导致在判定为“无活动会话”或未实际停止时也派发事件并触发下游 detachConversation；请将该 ctx.parallel
调用移动到仅在确实完成停止逻辑的分支（即在对 conversation.status 或 stop 成功判断后的分支）中触发，确保只有在实际停止成功（例如
status === 'stopped' 或 stop 操作返回成功）时才调用 ctx.parallel，并保留 conversation.id 与
session 作为事件载荷以保持行为一致。
In `@packages/extension-agent/src/commands/task.ts`:
- Line 158: The prompt string using `State: ${state(task, run)}. Send messages
here; use chatluna.agent.task.exit to leave.` references a non-existent command
and should be corrected to match the registered command; update the template to
instruct users to call `chatluna.agent.exit` (or, if you intended the dotted
command, rename the registration to `chatluna.agent.task.exit`) so the displayed
instruction and the registered handler (the code that registers
`chatluna.agent.exit`) match; ensure you update the template where `state(task,
run)` is used and any other helper text to reflect the correct command name.
In `@packages/extension-agent/src/sub-agent/task_attach.ts`:
- Around line 50-59: The call to this.ctx.chatluna_agent.subAgent.chatTask(...)
and the await runConfig(session, conversation) are not guarded, so exceptions
will bubble and break the session chain; wrap the whole await call in a
try/catch around the block that calls subAgent.chatTask and runConfig, log the
caught error using the module logger (e.g., logger.error(e)), and return or emit
a clear failure response to the caller (or set a session-level error result) so
the user/flow receives visible feedback instead of crashing; reference the call
site this.ctx.chatluna_agent.subAgent.chatTask(item.taskId, session.content, {
session, conversationId: item.parentConversationId, source: 'chatluna',
runConfig: await this.runConfig(session, conversation) }) when making the
change.
- Around line 17-29: The prepend middleware currently checks
session.argv?.command too early (constructor uses ctx.middleware(..., true))
which can cause commands to be forwarded as messages; change the middleware
registration to a normal middleware (remove the true prepend flag) or defer
reading session.argv until after command parsing, and remove/replace the if
(session.argv?.command) return next() check so exit commands aren't misrouted;
additionally, wrap awaits to runConfig(...) and subAgent.chatTask(...) in
try/catch inside the task_attach middleware (use the same
error-to-structured-result pattern as sub-agent.runTask) and handle failures by
cleaning up this._items, sending an error notice via session.send, and returning
next() to avoid unhandled rejections.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cc82944a-0593-4cea-bf48-8cdbc1892df2
📒 Files selected for processing (12)
packages/core/src/llm-core/agent/agent.tspackages/core/src/llm-core/agent/legacy-executor.tspackages/core/src/llm-core/agent/sub-agent.tspackages/core/src/llm-core/agent/types.tspackages/core/src/middlewares/chat/stop_chat.tspackages/core/src/services/types.tspackages/extension-agent/src/commands/agent.tspackages/extension-agent/src/commands/task.tspackages/extension-agent/src/computer/backends/local/sandbox.tspackages/extension-agent/src/index.tspackages/extension-agent/src/service/sub_agent.tspackages/extension-agent/src/sub-agent/task_attach.ts
✅ Files skipped from review due to trivial changes (1)
- packages/core/src/services/types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/llm-core/agent/sub-agent.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/src/llm-core/agent/sub-agent.ts (2)
695-723:⚠️ Potential issue | 🟠 Major | ⚡ Quick win不要让
refresh()改写任务最终状态。这里的
refresh是附属的控制台刷新，但它现在被放进了 run 的成功/失败收尾主路径里。只要options.runtime.refresh?.()在成功分支抛错，已完成的 run 就会落入catch被改写成failed，随后父会话收到的也是失败通知；而失败分支里的第二次refresh若再抛错，还会直接跳过notifyFinished(...)。建议把完成通知与 UI 刷新解耦，至少保证 refresh 失败不会影响run.state和完成事件投递。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/llm-core/agent/sub-agent.ts` around lines 695 - 723, The problem is that options.runtime.refresh?.() is called inside the main success/failure teardown so if refresh throws it can flip run.state or prevent notifyFinished; wrap each call to options.runtime.refresh?.() in its own try/catch that captures and logs errors without throwing (so it cannot change run.state or skip notifyFinished), and ensure notifyFinished(options, run, snapshot) is invoked regardless of refresh outcome (i.e., call notifyFinished before optional refresh or call refresh inside a non-throwing try/catch after notifyFinished); update locations around the success path (where formatTaskResult is returned) and the catch block (after run.state/run.error/trace/endedAt and cleanup) to use this non-throwing refresh pattern so refresh failures are decoupled from run completion and notification.
755-773:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift父子代理链路在父 run 已结束时会静默丢掉完成通知。
当
parentConversationId以subagent:开头时，这里只尝试把唤醒消息塞回父任务的MessageQueue。如果父任务已经结束，active查找失败后就直接return；而wakeup.ts的消费端又明确跳过subagent:payload，所以这类后台子任务的结果不会再冒泡回主对话。只要子代理允许继续发起 background task，这条分支就会把“结果自动送达”变成静默丢失。需要在父队列不存在时继续向更上层会话冒泡，或在快照里保留最终要唤醒的会话标识。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/llm-core/agent/sub-agent.ts` around lines 755 - 773, The branch that handles options.task.parentConversationId.startsWith('subagent:') silently returns when the parent active run (found via options.tasks and options.active) is missing, causing background subagent completions to be dropped; change the logic in sub-agent.ts so that if active is undefined you either (A) bubble the wakeup up to the next conversation by deriving the parent id (strip the 'subagent:' prefix or follow the stored parent chain) and attempting to locate/enqueue the same HumanMessage (constructed via formatAgentTaskWakeup(..., run)) into that higher-level MessageQueue, or (B) persist the intended wakeup target in the task snapshot/metadata so the next time the parent conversation is restored it will deliver the wakeup; ensure you keep the existing behavior of pushing the HumanMessage into active.queue when active exists and only fall back to the bubbling/persistence path when active is missing; reference symbols: options.task.parentConversationId, options.tasks, options.active, active.queue, HumanMessage, formatAgentTaskWakeup, run (and ensure wakeup.ts’s consumer will ignore subagent: payloads only when appropriate).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/llm-core/agent/sub-agent.ts`:
- Around line 60-116: The control methods stopTask, pauseTask, resumeTask, and
abortByParentConversation call options.refresh?.() (which returns an
Awaitable<void>) without awaiting it, causing possible unhandled rejection and
misleading immediate returns; make these APIs async (add async to their
declarations) and await the refresh call (await options.refresh?.()) before
returning, or alternatively consume errors by awaiting and catching rejections
(e.g., try { await options.refresh?.() } catch (e) { /* handle/log */ }); update
each of stopTask, pauseTask, resumeTask, and abortByParentConversation
accordingly so the caller receives the true completion result.
---
Outside diff comments:
In `@packages/core/src/llm-core/agent/sub-agent.ts`:
- Around line 695-723: The problem is that options.runtime.refresh?.() is called
inside the main success/failure teardown so if refresh throws it can flip
run.state or prevent notifyFinished; wrap each call to
options.runtime.refresh?.() in its own try/catch that captures and logs errors
without throwing (so it cannot change run.state or skip notifyFinished), and
ensure notifyFinished(options, run, snapshot) is invoked regardless of refresh
outcome (i.e., call notifyFinished before optional refresh or call refresh
inside a non-throwing try/catch after notifyFinished); update locations around
the success path (where formatTaskResult is returned) and the catch block (after
run.state/run.error/trace/endedAt and cleanup) to use this non-throwing refresh
pattern so refresh failures are decoupled from run completion and notification.
- Around line 755-773: The branch that handles
options.task.parentConversationId.startsWith('subagent:') silently returns when
the parent active run (found via options.tasks and options.active) is missing,
causing background subagent completions to be dropped; change the logic in
sub-agent.ts so that if active is undefined you either (A) bubble the wakeup up
to the next conversation by deriving the parent id (strip the 'subagent:' prefix
or follow the stored parent chain) and attempting to locate/enqueue the same
HumanMessage (constructed via formatAgentTaskWakeup(..., run)) into that
higher-level MessageQueue, or (B) persist the intended wakeup target in the task
snapshot/metadata so the next time the parent conversation is restored it will
deliver the wakeup; ensure you keep the existing behavior of pushing the
HumanMessage into active.queue when active exists and only fall back to the
bubbling/persistence path when active is missing; reference symbols:
options.task.parentConversationId, options.tasks, options.active, active.queue,
HumanMessage, formatAgentTaskWakeup, run (and ensure wakeup.ts’s consumer will
ignore subagent: payloads only when appropriate).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d000c78a-5924-4ddd-927e-7e14934b794d
📒 Files selected for processing (4)
packages/core/src/llm-core/agent/sub-agent.tspackages/core/src/middlewares/chat/stop_chat.tspackages/extension-agent/src/commands/task.tspackages/extension-agent/src/sub-agent/task_attach.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/core/src/middlewares/chat/stop_chat.ts
- packages/extension-agent/src/commands/task.ts
- packages/extension-agent/src/sub-agent/task_attach.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pr improves the agent computer backend workflow and adds controls for background sub-agent tasks.
New Features
Bug Fixes
Other Changes
Validation