feat(btw): keep the work loop read-only and delegate writes - #206
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
c628e30 to
f52f4d0
Compare
BegoniaHe
left a comment
BegoniaHe
left a comment
There was a problem hiding this comment.
AI-assisted review
方向对，测试和 CI 也齐（coverage / quality / 三平台 blocking pytest 均为 pass），但委派执行器还没接到本仓库本地执行的安全基线，现在不能合。
必须先改的四件事在下面的 inline 评论里：沙箱逃逸、进程树杀不干净、产物收集、Dashboard 会写坏 coding_agents。另外请 rebase 当前 master（落后约 20 个提交，含本地沙箱修复）。
授权用 tool.local_exec + tool.file_write 而不是更弱的 agent.manage、只读目录和 handoff 共用 tool_blocked_in_loop、双语文档和 89/89 函数覆盖，这些是对的。
本条是 AI_POLICY.md 要求的独立 AI 辅助评审，不是作者说明，也不构成合入批准。
| work_loop_submission=loop_mode == "conversation" and is_work_loop_enabled(cfg), | ||
| # The work loop plans and reads; writes belong to a coding agent. | ||
| work_loop_read_only=work_loop_is_read_only(cfg, loop_mode), | ||
| work_loop_delegation=loop_mode == "work" and has_enabled_coding_agent(cfg), |
There was a problem hiding this comment.
这里只要工作环开了、且配置了编码代理，就会挂上 delegate_coding_task，没有区分 local 和 sandbox。
委派实际走的是宿主机 create_subprocess_exec（见 coding_runner.py），不经过 Computer Use booter。工作环 runtime 设成 sandbox 时，Shell/写文件会进沙箱，Claude Code / Codex / custom 命令却在宿主机跑，还可以 --add-dir 到任意 project_dir。这是沙箱逃逸，不是“sandbox 也算一种授权”。
至少只允许 local，或者把子进程送进同一个沙箱。文档和配置 hint 里 “local 或 sandbox” 的说法要一起改。
| process = await asyncio.create_subprocess_exec( | ||
| *invocation.argv, | ||
| cwd=str(invocation.cwd), | ||
| stdin=asyncio.subprocess.PIPE, | ||
| stdout=asyncio.subprocess.PIPE, | ||
| stderr=asyncio.subprocess.PIPE, | ||
| env=dict(os.environ, **invocation.env), |
There was a problem hiding this comment.
这条路径达不到本仓库本地执行的安全基线。
- 没有
start_new_session，Windows 上也没有CREATE_NEW_PROCESS_GROUP。现有本地 Shell 在astrbot/core/computer/booters/local.py里专门处理了进程组。Claude Code / Codex 会再拉起 node 子进程，父进程被terminate()/kill()后子进程可以继续写盘。验收写的是 timeout / 撤回 / 取消会停掉子进程；当前测试只用python -c，测不到进程树。 env=dict(os.environ, **invocation.env)把 AstrBot 进程里的全部环境（含提供商密钥）交给编码代理。至少应传白名单。
| # afford to await during unwinding. | ||
| if process.returncode is None: | ||
| with contextlib.suppress(ProcessLookupError): | ||
| process.kill() |
There was a problem hiding this comment.
取消路径里 kill() 之后没有 wait()。POSIX 上会留下僵尸进程。注释说 unwinding 时不能 await，但 CancelledError 的 finally 里仍应 reap；现有 _terminate() 已经在等，这里不应例外。
| def task_workspace(root: Path, session_id: str, agent_id: str) -> Path: | ||
| """Create and return the folder one delegated task runs in.""" | ||
| workspace = Path(root) / task_folder_name(session_id, agent_id) | ||
| workspace.mkdir(parents=True, exist_ok=True) |
There was a problem hiding this comment.
exist_ok=True 且文件夹名只有 {session}-{agent}。同一次工作 run 可以多次 delegate_coding_task，后一次会覆盖 TASK.md / output.log，并和前一次的文件混在一起。请加上 run id。
| continue | ||
| changed = await _git_artifacts(root) | ||
| artifacts.extend(changed if changed is not None else _listed_artifacts(root)) |
There was a problem hiding this comment.
产物收集没有 run 前的 baseline。
- git 工作树用
git status --porcelain，会把委派开始前已有的脏文件都算进去。 - 非 git 的
project_dir会rglob列出全部文件，大仓库上既慢又会把整个项目报成这次任务的产出。 git status超时后只取消communicate()，不杀 git 进程。
git 路径要做 before/after diff；非 git 的 project_dir 不得全量列举。
| "Report the task as failed and say so." | ||
| ) | ||
| await runtime_registry.record_work_artifacts( | ||
| event, [str(workspace / path) for path in result.artifacts] |
There was a problem hiding this comment.
record_work_artifacts 写入的是绝对路径，对话循环会原样汇报给用户。IM 面上会漏出主机目录。对用户可见的汇报用相对路径（相对 workspace_root），绝对路径留给内部记录。
|
|
||
| `btw.work_loop.read_only` 默认为 `true`。开启后工作循环的工具目录只保留读取与搜索能力：`astrbot_file_read_tool`、`astrbot_grep_tool`、网页搜索、记忆与知识库工具照常可用；Shell、Python、文件写入与编辑、上传下载、浏览器、CUA，以及 `readOnlyHint` 不为真的 MCP 工具都会被移除，`delegate_coding_task` 是唯一的写入途径。它只收紧能力：文件读取仍要求 `btw.work_loop.computer_use_runtime` 已授予 local 或 sandbox，角色、路径限制、沙箱、WebChat step-up 与逐项循环分配规则都不变。关闭 `read_only` 恢复原有的写入能力。 | ||
|
|
||
| `btw.work_loop.coding_agents` 声明可委派的本地 CLI 代理。每个条目包含 `id`、`type`（`claude_code`、`codex` 或 `custom`）、`command`、`permission_mode`（Claude Code 权限模式，默认 `acceptEdits`）、`sandbox`（Codex 沙箱，默认 `workspace-write`）、`project_dir`、`extra_args`、`env`、`timeout_seconds` 和 `providers`。默认权限模式只允许代理在任务目录内写入；`bypassPermissions` 与 `danger-full-access` 必须显式配置。委派会启动本地进程并向文件系统写入，因此 `delegate_coding_task` 按 `tool.local_exec` 与 `tool.file_write` 授权：这项工作循环的 Computer Use 运行时必须是 `local` 或 `sandbox`，WebChat step-up 等表面提升与逐项循环分配规则照常适用，`provider_settings.computer_use_runtime` 默认的 `none` 会同时关闭文件读取和委派。 |
There was a problem hiding this comment.
两处需要和实现对齐：
- “运行时必须是
local或sandbox” —— 委派在宿主机执行，sandbox 不应作为合法前置条件（见astr_main_agent.py那条评论）。 - “默认权限模式只允许代理在任务目录内写入” ——
acceptEdits/workspace-write不是 OS 隔离；配置了project_dir后--add-dir会把可写范围扩出去。
英文页 docs/en/dev/astrbot-config.md 第 254 行是同一段，请一起改。
|
|
||
| `btw.work_loop.read_only` defaults to `true`. With it on, the work loop's tool catalog keeps only reading and searching: `astrbot_file_read_tool`, `astrbot_grep_tool`, web search, memory, and knowledge-base tools stay available, while Shell, Python, file writing and editing, upload and download, browser, CUA, and MCP tools whose `readOnlyHint` is not true are removed. `delegate_coding_task` is then the only way to write. The setting only removes capabilities: file reading still requires `btw.work_loop.computer_use_runtime` to grant local or sandbox, and roles, path restrictions, sandboxing, WebChat step-up, and the per-capability loop assignments are unchanged. Turning `read_only` off restores the previous write capabilities. | ||
|
|
||
| `btw.work_loop.coding_agents` names the local CLI agents a task can be delegated to. Each entry carries `id`, `type` (`claude_code`, `codex`, or `custom`), `command`, `permission_mode` (the Claude Code permission mode, `acceptEdits` by default), `sandbox` (the Codex sandbox, `workspace-write` by default), `project_dir`, `extra_args`, `env`, `timeout_seconds`, and `providers`. The default permission modes let an agent write only inside its task folder; `bypassPermissions` and `danger-full-access` must be configured explicitly. A delegation starts a local process that writes to the filesystem, so `delegate_coding_task` is authorized as `tool.local_exec` and `tool.file_write`: this work loop's Computer Use runtime must be `local` or `sandbox`, surface elevation such as WebChat step-up and the per-capability loop assignments still apply, and the `none` default of `provider_settings.computer_use_runtime` turns off file reading and delegation together. |
There was a problem hiding this comment.
Same two overclaims as the Chinese page: sandbox is treated as a valid gate for a host subprocess, and default permission modes are described as a write jail. They are a prompt policy (Claude acceptEdits) plus Codex workspace-write, and --add-dir project_dir widens the writable roots. Please keep the two languages aligned after the runtime/process-group fixes.
|
|
||
| `btw.work_loop.coding_agents` 声明可委派的本地 CLI 代理。每个条目包含 `id`、`type`（`claude_code`、`codex` 或 `custom`）、`command`、`permission_mode`（Claude Code 权限模式，默认 `acceptEdits`）、`sandbox`（Codex 沙箱，默认 `workspace-write`）、`project_dir`、`extra_args`、`env`、`timeout_seconds` 和 `providers`。默认权限模式只允许代理在任务目录内写入；`bypassPermissions` 与 `danger-full-access` 必须显式配置。委派会启动本地进程并向文件系统写入，因此 `delegate_coding_task` 按 `tool.local_exec` 与 `tool.file_write` 授权：这项工作循环的 Computer Use 运行时必须是 `local` 或 `sandbox`，WebChat step-up 等表面提升与逐项循环分配规则照常适用，`provider_settings.computer_use_runtime` 默认的 `none` 会同时关闭文件读取和委派。 | ||
|
|
||
| 工作循环通过 `delegate_coding_task` 交办一次写入任务：任务文本写入 `<btw.work_loop.workspace_root>/<会话 ID>-<代理 ID>/TASK.md`，`workspace_root` 留空时使用数据目录下的 `btw/workspaces`。代理在该目录中运行，完整输出记录到同目录的 `output.log`。任务结束后工作循环读回状态、退出码、产物路径与代理的最终消息；产物在 git 工作树中取自 `git status --porcelain`，普通目录则列出文件，最多 50 项。 |
There was a problem hiding this comment.
这里写产物取自 git status --porcelain 或“列出文件”，和实现一致，但实现没有 run 前 baseline。非 git 的 project_dir 会把整个目录报成这次任务的产出；同 session 多次委派还会混文件。代码改完后这段要改成 before/after diff，并写明非 git 项目目录怎么处理。英文页第 256 行同步。
| async def test_a_cancelled_run_does_not_leave_the_agent_running(tmp_path): | ||
| slow_writer = ( | ||
| "import pathlib, time;" | ||
| "time.sleep(1.5);" | ||
| "pathlib.Path('sentinel.txt').write_text('x', encoding='utf-8')" | ||
| ) | ||
| task = asyncio.create_task( | ||
| cr.run_coding_task(_agent(slow_writer), workspace=tmp_path, task="t") | ||
| ) | ||
| await asyncio.sleep(0.3) | ||
| task.cancel() | ||
| with pytest.raises(asyncio.CancelledError): | ||
| _ = await task | ||
|
|
||
| # The child had most of its sleep left; a surviving one would still write. | ||
| await asyncio.sleep(1.8) | ||
| assert not (tmp_path / "sentinel.txt").exists() |
There was a problem hiding this comment.
这个测试只能证明单进程 python -c 会被 kill()。Claude Code / Codex 是进程树。请补：子进程再 spawn 一层时，取消/超时后孙子进程也不能继续写盘（POSIX 进程组 + Windows Job/process group）。顺手覆盖“非 git project_dir 不得全量列举”。
f52f4d0 to
725be6c
Compare
|
已按评审逐条处理，并 rebase 到当前 四件必须改的1. 沙箱逃逸（ 2. 进程树杀不干净（ 绑在子进程跑起来之前是必要的——先跑后绑会和启动器赛跑，而一个 spawn 完 同一处还改了环境变量： 3. 取消路径不 reap（ 4. Dashboard 写坏 其余 inline
CodeQL 四条随重构消失，当前 PR ref 上没有 open alert。 一处想请你定夺
我倾向 (a)：任务目录本身是可丢弃的，但 本机验证说明
|
|
|
||
| import asyncio | ||
| import contextlib | ||
| import ctypes |
ce5d33b to
9b14a6b
Compare
9b14a6b to
5db8773
Compare
The work loop ran the same full-capability Agent as the conversation loop, so once its Computer Use runtime granted local or sandbox it could also shell out, run Python, and write files, with no task boundary and no record of what changed. Planning and writing were the same loop. Remove every write-capable tool from the work loop and give it one way to change files: delegate_coding_task hands a self-contained task to a local coding agent CLI (Claude Code, Codex, or any custom command) that runs inside its own task folder. The loop keeps reading and searching, and btw.work_loop.read_only, on by default, is what makes the split real. Read tools are only kept when the loop's runtime already grants them, so the setting restricts capabilities and never adds one. The boundary is drawn by meaning rather than by name: a tool that declares no required_actions counts as a writer, plugin tools included, and the executor checks the same rule the catalog applied, so a tool carried into a request cannot be run in a loop that must not have it. A delegated run owns <workspace_root>/<session id>-<agent id>-<run id>/, so two delegations in one session cannot overwrite each other. The task text is kept in TASK.md, the agent's whole transcript in output.log, and what it changed is read back as the difference the run made: a git work tree is compared before and after, including anything committed during the run, and a plain folder is bounded by the run's start time rather than listed whole. The task travels on the child's stdin, so its length is not a command-line limit, and the report names artifacts relative to the workspace root rather than spelling out where AstrBot keeps its data. A run is a tree, not a process. The child is started in a session of its own on POSIX, and on Windows suspended at birth, bound to a Job Object, and resumed, so a launcher that starts a helper and exits cannot leave the helper outside the tree that is stopped. Stopping a run -- by timeout, by a withdrawn request, or by cancellation -- ends the whole tree and reaps it, whether the launcher was still running or had already returned. The child receives a whitelisted environment, so the provider keys AstrBot holds never travel to it, and only a bounded tail of its output is held in memory. Delegation starts a local process and writes files, so it is authorized as tool.local_exec and tool.file_write rather than the weaker agent.manage: the computer-use boundary, surface elevation, and the authorization decision all still gate it. Because that process runs on this host instead of inside the Computer Use booter, a work loop whose runtime is sandbox cannot delegate at all -- doing so would be the way out of the sandbox rather than a use of it -- so the feature is offered only where the runtime is local. Provider presets follow cc-switch: each agent carries a list and one active provider, handed to the CLI as a config layer it loads itself (--settings for Claude Code, --profile for Codex). The layer is named for the agent, so two agents running at once cannot overwrite each other's, and switching providers never rewrites the user's global CLI configuration. A preset with neither an endpoint nor a key keeps the user's own login. The Dashboard edits that list through an editor of its own. The generic list control edits strings, and an array of objects through it showed as [object Object] and would have been saved back as strings, discarding every configured agent. The conversation loop now owns the report: the work loop hands it the finished run, and it composes one message from the result, a completion marker, and the artifact paths. Streamed chunks pass through untouched, and one run carries the report exactly once. AI-Generated: true Generated-At: 2026-09-17T07:51:32Z
5db8773 to
532153b
Compare
`astrbot_memory_write_tool` and the schedule tool have declared
`required_actions=("tool.memory_write",)` and `("tool.schedule_write",)`
since the work loop was made read-only, and `tool_catalog.py` lists both,
so the executor was checking actions the registry had never heard of. A
tool whose action is not in `ACTIONS` has no entry in `ACTION_ROLE_GRANTS`
either, and the grants are what decide who may run it: the declaration
was enforced against a name with no tier behind it.
Both write inside the caller's own session -- one its memory, the other
its schedules -- so they sit one tier above the matching read, at
`_SESSION_OWNER_AND_ABOVE`, the same step `session.read` takes to
`session.manage`. This is the registration the tools were waiting for;
nothing else about the boundary changes.
AI-Generated: true
Generated-At: 2026-09-18T08:09:20Z
3 participants
Summary
Make the BTW work loop read-only and give it one way to write: hand the task to
a local coding agent CLI that runs inside its own task folder. The conversation
loop now owns the completion report the user sees.
Today the work loop runs the same full-capability Agent as the conversation
loop, so granting it
localorsandboxalso grants Shell, Python, and filewrites. Planning and writing are the same loop, a task has no boundary, and
nothing records what it changed.
Related issue
Fixes #205
Behavior
btw.work_loop.read_only(defaulttrue) drops every write-capable tool fromthe work loop's catalog: Shell, Python, file write and edit, upload and
download, browser, CUA, the Neo lifecycle tools, and MCP tools whose
readOnlyHintis not true. Read tools stay, and only where the loop's ownruntime already grants them, so the setting restricts capabilities and never
adds one.
delegate_coding_taskis the work loop's only way to change files. It carriestask(a self-contained brief) and an optionalagent_id.btw.work_loop.coding_agentsnames the agents and their provider presets.Each preset reaches the CLI as a config layer the CLI loads itself:
--settingsfor Claude Code,--profile astrbot-btwover$CODEX_HOME/astrbot-btw.config.tomlfor Codex. Only the endpoint and themodel are written; the credential is passed in the child's environment for the
length of one run and is never persisted. A preset with neither an endpoint
nor a key keeps the user's own login, and switching providers never rewrites
the user's global CLI config.
<workspace_root>/<session id>-<agent id>/, keepingTASK.mdandoutput.log, and reporting status, exit code, artifact paths,and the agent's final message back to the work loop.
btw.work_loop.report_via_conversation(defaulttrue) makes the conversationloop compose one message from the result, a completion marker, and the
artifact paths.
btw.work_loop.workspace_rootrelocates the task folders.them, and the delegation paragraph appears only when the tool is actually
reachable for that request.
submit_work_taskis now confined to the conversation loop, so a work run canno longer submit more work.
Non-goals
customagent type is theextension point.
coding_agents; the generic list rendererhandles it for now, as it already does for the BTW loop-route selectors.
step-up.
Implementation notes
tool_blocked_in_loop()intool_catalog.pyis the single policy both thecatalog and the tool executor consult, so a tool assembled into a request or
carried into a handoff cannot run in a loop that must not have it.
_apply_visibilityand_filter_handoff_tools_for_loopboth call it.resolve_run_status()inbtw/types.pyis shared by the work loop, whichrecords the terminal status, and the conversation loop, which reports it before
the record exists; one reading of one set of markers keeps the two from
disagreeing.
The
customagent type gets no generated config layer, because this module doesnot know that CLI's config format; it states its own credentials through the
agent's
envmap.Docs:
docs/{zh,en}/dev/astrbot-config.mddocument the read-only boundary, thedelegation path, and the provider layers;
docs/{zh,en}/use/command.mdnote thebehavior on
/work.Validation
tests/unitpassed 5550 with 6 skipped, and the blocking profile passed 5687with 6 skipped and 1 deselected. The new modules reach 89/89 functions (100%)
under the repository's function-coverage measure. Pyright reports 0 errors; its
one warning is the pre-existing
STAGES_ORDER__all__entry inastrbot/core/pipeline/__init__.py. Ruff, Prettier, markdownlint, and theDashboard config-metadata vitest are clean.
make checkandmake test-blockingcannot run from this checkout:.python-versionpins 3.14.6 while the available interpreter is 3.14.1, so everyuv runin the Makefile fails with "No interpreter found for Python 3.14.6".The equivalents above were run directly against the checkout's
.venv. GitHubCI validates the pinned toolchain.
The delegation path was exercised against a real Claude Code 2.1.219 on Windows:
a task that had to create a file returned
status: completed,exit_code: 0,artifacts: ['hello.txt'], and the file contained the requested text. Both CLIswere separately checked to accept the generated argument vectors. No live Codex
run and no live end-to-end run through the message pipeline were performed.
Compatibility and risk
No schema change and no OpenAPI change. Config gains four keys under
btw.work_loop; becausebtw.enabledandbtw.work_loop.enabledare bothfalseby default, an unconfigured instance is unaffected.Read-only is a restriction, not a grant: file reading still requires the loop's
Computer Use runtime to grant
localorsandbox, and roles, path limits,sandboxing, WebChat step-up, and the per-capability loop assignments are
unchanged.
Delegation starts a local process and writes files, so
delegate_coding_taskdeclares
tool.local_execandtool.file_writerather than the weakeragent.manage. This matters:agent.manageis granted to a session owner,while
tool.local_execrequires an instance operator and passes the samecomputer-use boundary, surface elevation, and authorization checks as any other
local executor. The tool is exempted only from the loop-level read-only rule,
because it is that loop's sanctioned write path.
Delegation is off in effect by default:
provider_settings.computer_use_runtimedefaults to
none, and a work loop without a granted runtime exposes neitherfile reading nor delegation.
btw.work_loop.report_via_conversation: falserestores the previous directdelivery, and
btw.work_loop.read_only: falserestores the previous writecapabilities.
Behavior change for existing BTW users: a work loop that previously had Shell or
file writes now has neither. This is the point of the change and is documented
in both languages.
Checklist
docs/zh/anddocs/en/.docs/public/openapi.json, and tests change together when routes or schemas change.pyproject.toml,requirements.txt, anduv.locktogether.!and aBREAKING CHANGE:footer.Agent note
Goal: the slice in #205, extending the #129/#130 capability direction into a
read/write split. One commit on a branch based on
master.Paths touched:
astrbot/core/agent/btw/coding_agents.py,coding_runner.py,and
conversation_report.py(new);astrbot/core/tools/coding_tools.py(new);btw/types.py,work_loop.py,work_sessions.py,runtime_policy.py,runtime_registry.py, andi18n.py;conversation_loop.py,tool_catalog.py,astr_agent_tool_exec.py,astr_main_agent.py,config/default.py,tools/registry.py; both config-metadata locales; bothlanguages of
dev/astrbot-config.mdanduse/command.md; five new test modulesplus
test_astr_agent_tool_exec.py.Checks run: the commands in Validation, with the results stated there. Residual
risk: no live Codex run, and no live end-to-end run through the message pipeline
(the delegation path was driven directly and the loop wiring is covered by unit
tests with fake events); the Dashboard has no dedicated editor for
coding_agents, so a nested entry is easiest to author in config. Tools used:Claude Code with the repository's scripts, pytest, ruff, pyright.