diff --git a/README.md b/README.md index c936625..e8358cd 100644 --- a/README.md +++ b/README.md @@ -236,8 +236,9 @@ flow; `arbor doctor` diagnoses the install. Both `quickstart` and `setup` write `~/.arbor/config.yaml`, so day-to-day you can just run `arbor` with no flags. The first thing Arbor does is an **intake conversation** that turns your goal, target directory, metric, baseline, budget, dev/test discipline, and artifact -paths into a one-screen **Arbor Research Contract**. Once you confirm it, the live -dashboard takes over. +paths into a one-screen **Arbor Research Contract**. Read/discuss requests stay in a +scoped, read-only mode; experiment plans are staged and launch only after a later explicit +confirmation. Once you confirm the resolved contract, the live dashboard takes over. ```bash # Point at a benchmark directory and a config diff --git a/README.zh-CN.md b/README.zh-CN.md index c166fd6..1cb1691 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -194,7 +194,7 @@ arbor # 在当前目录启动交互式会话 已有模型提供商？`arbor setup` 提供完整的 提供商 / 模型 / base_url / API 密钥 配置流程； `arbor doctor` 用于诊断安装。`quickstart` 与 `setup` 都会将配置写入 -`~/.arbor/config.yaml`，此后日常使用直接运行 `arbor` 即可，无需额外参数。Arbor 启动后首先进行一次**任务摄入对话**，将你的目标、目标目录、评估指标、基线、预算、开发/测试纪律和产物路径整理成一份简洁的 **Arbor 研究合同**。确认后，实时仪表盘随即接管。 +`~/.arbor/config.yaml`，此后日常使用直接运行 `arbor` 即可，无需额外参数。Arbor 启动后首先进行一次**任务摄入对话**，将你的目标、目标目录、评估指标、基线、预算、开发/测试纪律和产物路径整理成一份简洁的 **Arbor 研究合同**。阅读/讨论请求保持在受限的只读模式；实验计划会先暂存，只有你在后续消息中明确确认后才会启动。确认完整合同后，实时仪表盘随即接管。 ```bash # 指定基准目录和配置文件 diff --git a/docs/cli.md b/docs/cli.md index 1a7d485..e67588f 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -30,10 +30,13 @@ changing eval or data"`). Omit it to start with the intake chat. ### Default flow -1. Open an interactive chat with the intake agent. -2. The agent confirms which project directory to work on (the `--cwd` flag is only a hint). -3. When you agree on a plan, the agent launches the experiment. -4. You confirm the research contract shown in the terminal. +1. Open an interactive chat with the intake agent. Read/discuss requests stay in scoped, + read-only discussion mode; launch planning is enabled only for an optimization/run goal. +2. The agent confirms which project directory to work on. `--cwd` is the default boundary; + external paths must be named explicitly and are re-authorized after chat resume. +3. The agent stages an exact plan and shows it to you. A later, explicit confirmation launches + that same staged plan; visible questions never execute hidden tool calls. +4. You confirm the fully resolved research contract shown in the terminal. 5. A quick preflight runs against the chosen project. 6. The coordinator runs to completion and writes `REPORT.md`. @@ -45,7 +48,7 @@ changing eval or data"`). Omit it to start with the intake chat. | `--config, -c PATH` | Project YAML config. Defaults to `research_config.yaml` / `arbor.yaml` / `autoresearch.yaml` in the target project. | | `--max-cycles N` | Max completed/skipped/failed idea experiments before finalizing. | | `--max-turns N` | Hard cap on coordinator ReAct turns — a cost/runaway safety valve. | -| `--intake-max-turns N` | Max planning-chat turns before launch (default `30`). | +| `--intake-max-turns N` | Max internal agent turns for each intake message (default `30`). | | `--run-name NAME` | Session name under `.arbor/sessions/`. Defaults to a timestamp. | | `--resume` | Resume an interrupted run from its checkpoint in the existing workspace/session. | | `--workspace-dir PATH` | Session/artifact directory override. Default `/.arbor/sessions/`. | diff --git a/docs/cli.zh.md b/docs/cli.zh.md index d273412..2a4be7a 100644 --- a/docs/cli.zh.md +++ b/docs/cli.zh.md @@ -29,10 +29,10 @@ data"`）。省略它则从接入对话开始。 ### 默认流程 -1. 与接入智能体打开一段交互式对话。 -2. 智能体确认要在哪个项目目录上工作（`--cwd` 参数只是一个提示）。 -3. 当你与智能体就计划达成一致后，智能体启动实验。 -4. 你确认终端里展示的研究契约。 +1. 与接入智能体打开一段交互式对话。阅读/讨论请求保持在有范围限制的只读讨论模式；只有优化或启动目标才进入运行规划模式。 +2. 智能体确认目标项目目录。`--cwd` 是默认权限边界；外部路径必须由你明确点名，恢复旧对话后也要重新授权。 +3. 智能体暂存并展示一份精确计划；你在后续消息中明确确认后，CLI 才启动同一份计划。显示问题时不会在后台继续执行工具。 +4. 你确认终端里展示的完整研究契约。 5. 针对所选项目跑一次快速预检。 6. Coordinator 运行至完成并写出 `REPORT.md`。 @@ -44,7 +44,7 @@ data"`）。省略它则从接入对话开始。 | `--config, -c PATH` | 项目 YAML 配置。默认取目标项目里的 `research_config.yaml` / `arbor.yaml` / `autoresearch.yaml`。 | | `--max-cycles N` | 定稿前最多完成/跳过/失败多少个想法实验。 | | `--max-turns N` | Coordinator ReAct 轮次的硬上限——一个成本/失控安全阀。 | -| `--intake-max-turns N` | 启动前规划对话的最多轮次（默认 `30`）。 | +| `--intake-max-turns N` | 每条 intake 消息允许的最大内部 Agent 轮数（默认 `30`）。 | | `--run-name NAME` | `.arbor/sessions/` 下的会话名。默认是时间戳。 | | `--resume` | 在现有工作空间/会话里从检查点续跑一次被中断的运行。 | | `--workspace-dir PATH` | 会话/产物目录覆盖。默认 `/.arbor/sessions/`。 | diff --git a/docs/preparing-a-benchmark.md b/docs/preparing-a-benchmark.md index 2696a00..24a02ae 100644 --- a/docs/preparing-a-benchmark.md +++ b/docs/preparing-a-benchmark.md @@ -33,18 +33,20 @@ score: 0.8123 A simple working baseline already in the repo is ideal — it gives Arbor a number to beat and confirms the eval actually runs. -!!! tip "Only have code? The intake agent can build the eval for you" +!!! tip "Only have code? Intake can plan the eval setup with you" You don't need an eval script — or even a dev/test split — *before* you start. If your repo is just code, launch `arbor` anyway: in the intake chat the agent asks what - "better" means, then offers to **scaffold a minimal eval** (and, if you want, carve a - **dev/held-out split**) for you to confirm. No held-out set and don't want one? You can + "better" means, then stages a plan whose first coordinator task can **scaffold a minimal + eval** (and, if you want, carve a **dev/held-out split**) after you confirm. No held-out + set and don't want one? You can iterate on a single split — the agent will just note that the final score has no held-out guard. See [Describe the task](#2-describe-the-task-readme-or-just-tell-the-cli). -!!! tip "Let the intake agent do the plumbing" +!!! tip "Let Arbor do the plumbing after approval" You don't have to pre-initialize git, `chmod +x` your script, or run the eval yourself. - When you launch `arbor`, the intake agent will quietly do those setup steps for you - (and confirm the eval produces a score) before the study starts. + Intake itself is read-only: it inspects files inside the approved scope and stages the + contract. After you confirm, deterministic preflight and the coordinator handle setup + and evaluation work; intake never runs shell commands behind a displayed question. ## 2. Describe the task — README *or* just tell the CLI diff --git a/docs/preparing-a-benchmark.zh.md b/docs/preparing-a-benchmark.zh.md index 4102b01..e7daefc 100644 --- a/docs/preparing-a-benchmark.zh.md +++ b/docs/preparing-a-benchmark.zh.md @@ -31,16 +31,17 @@ score: 0.8123 仓库里已经有一个能跑通的简单基线是最理想的——它为 Arbor 提供了一个要超越的基准数字，也确认评测确实能跑。 -!!! tip "只有代码？接入智能体能替你搭好评测" +!!! tip "只有代码？接入智能体能和你规划评测准备" 在你开始*之前*，你并不需要评测脚本——甚至不需要 dev/test 划分。如果你的仓库只有代码，照样 - 启动 `arbor`：在接入对话里，智能体会询问“更好”意味着什么，然后提出**搭建一个最小评测** - （如果你愿意，还会**切分出 dev/留出划分**）供你确认。没有留出集，也不打算准备？你可以在单一划分上 + 启动 `arbor`：在接入对话里，智能体会询问“更好”意味着什么，并暂存一份计划；经你确认后， + coordinator 的首个任务可以**搭建一个最小评测**（如果你愿意，还会**切分出 dev/留出划分**）。 + 没有留出集，也不打算准备？你可以在单一划分上 迭代——智能体只会提示最终分数缺少留出护栏。见 [描述任务](#2-describe-the-task-readme-or-just-tell-the-cli)。 -!!! tip "让接入智能体处理准备工作" - 你不必预先初始化 git、给脚本 `chmod +x`，或自己跑评测。当你启动 `arbor` 时，接入智能体会在 - 研究开始前自动替你完成这些准备步骤（并确认评测确实产出一个分数）。 +!!! tip "确认后让 Arbor 处理准备工作" + 接入阶段本身是只读的：只会在你批准的范围内检查文件并暂存研究契约。你确认之后，确定性的预检和 + coordinator 才会处理环境与评测工作；接入智能体不会在显示问题后偷偷执行 shell 命令。 ## 2. 描述任务——用 README *或者* 直接告诉 CLI { #2-describe-the-task-readme-or-just-tell-the-cli } diff --git a/src/cli/commands/run.py b/src/cli/commands/run.py index 8834b16..442234a 100644 --- a/src/cli/commands/run.py +++ b/src/cli/commands/run.py @@ -44,7 +44,7 @@ def run_command( max_turns: int | None = typer.Option(None, "--max-turns", help="Hard cap on coordinator ReAct turns. Use as a cost/runaway safety valve."), intake_max_turns: int = typer.Option(30, "--intake-max-turns", - help="Max planning-chat turns before launch. Rarely needed."), + help="Max internal agent turns per intake message. Rarely needed."), run_name: str | None = typer.Option(None, "--run-name", help="Session name under .arbor/sessions/. Defaults to timestamp."), resume: bool = typer.Option( False, "--resume", @@ -263,6 +263,11 @@ def _probe_config() -> CoordinatorConfig: plan_cwd = Path(outcome.cwd).resolve() unloaded_skills = list(outcome.unloaded_skills) refined_instruction = outcome.instruction + if outcome.notes: + refined_instruction += ( + "\n\nAdditional user constraints:\n- " + + "\n- ".join(outcome.notes) + ) selected_plugin = outcome.plugin selected_plugin_profile = outcome.plugin_profile selected_plugin_mode = outcome.plugin_mode @@ -302,10 +307,9 @@ def _probe_config() -> CoordinatorConfig: # ── 2. Quick essentials check against plan_cwd ───────────────── # - # The intake agent already runs its own sanity pass (git status, - # eval.sh, etc.) inline during the chat, so we don't show a - # ceremonial preflight section here — just run silently and only - # surface fatal issues as a red panel. + # Intake is deliberately read-only. Run the deterministic essentials + # preflight after the staged plan is approved, and surface only failures by + # default so planning stays conversational without hiding execution. # # Resolve the plugin's eval_contract so the contamination preflight can # warn (zero-network) when the benchmark is likely in pretraining data. @@ -790,6 +794,14 @@ def _resolve_effective_options( max_cycles: int | None, max_turns: int | None, ) -> dict[str, Any]: + # ``load_layered_defaults`` intentionally preserves structured blocks. + # Normalize the documented ``llm: {provider, model, ...}`` form onto the + # flat lookup surface used below, while keeping legacy top-level values as + # higher-precedence compatibility aliases. + nested_llm = project_defaults.get("llm") + if isinstance(nested_llm, dict): + project_defaults = {**nested_llm, **project_defaults} + project_defaults.pop("llm", None) if project_defaults.get("provider") is not None: provider_source = "project" eff_provider = project_defaults.get("provider") diff --git a/src/cli/intake/conversation_store.py b/src/cli/intake/conversation_store.py index 9ac4e91..d26fcc6 100644 --- a/src/cli/intake/conversation_store.py +++ b/src/cli/intake/conversation_store.py @@ -28,6 +28,7 @@ import json import os +import re import tempfile from dataclasses import dataclass from datetime import datetime, timezone @@ -45,6 +46,7 @@ META_NAME = "meta.json" _TITLE_MAX = 80 +_CONV_ID_RE = re.compile(r"conv_\d{8}_\d{6}(?:_\d+)?") def _utc_now_iso() -> str: @@ -135,8 +137,20 @@ def save_conversation( Updates ``rec`` in place (``updated_at``, ``turns``, ``title``, ``launched``) so the caller's handle stays current across repeated saves. """ - rec.dir.mkdir(parents=True, exist_ok=True) - write_messages(rec.messages_path, messages) + if not _CONV_ID_RE.fullmatch(rec.conv_id): + raise ValueError(f"invalid conversation id: {rec.conv_id!r}") + root = conversations_root(rec.cwd) + root.mkdir(parents=True, exist_ok=True) + if root.is_symlink(): + raise OSError(f"refusing symlinked conversation root: {root}") + try: + root.resolve(strict=True).relative_to(rec.cwd.resolve(strict=True)) + except (OSError, ValueError) as exc: + raise OSError(f"conversation root escapes project: {root}") from exc + rec.dir.mkdir(parents=False, exist_ok=True) + if rec.dir.is_symlink(): + raise OSError(f"refusing symlinked conversation directory: {rec.dir}") + write_messages(rec.messages_path, _messages_for_disk(messages)) rec.updated_at = _utc_now_iso() rec.turns = _count_user_turns(messages) @@ -158,15 +172,33 @@ def find_conversations(cwd: str | os.PathLike[str]) -> list[ConversationRecord]: Defensive: a dir without a parseable ``meta.json`` is skipped, never fatal. """ root = conversations_root(cwd) - if not root.is_dir(): + if not root.is_dir() or root.is_symlink(): + return [] + try: + resolved_root = root.resolve(strict=True) + resolved_root.relative_to(Path(cwd).resolve(strict=True)) + except (OSError, ValueError): return [] records: list[ConversationRecord] = [] for conv_dir in root.iterdir(): - if not conv_dir.is_dir(): + if ( + not conv_dir.is_dir() + or conv_dir.is_symlink() + or not _CONV_ID_RE.fullmatch(conv_dir.name) + ): + continue + try: + conv_dir.resolve(strict=True).relative_to(resolved_root) + except (OSError, ValueError): continue data = _load_json(conv_dir / META_NAME) - if not isinstance(data, dict) or "conv_id" not in data: + if ( + not isinstance(data, dict) + or data.get("conv_id") != conv_dir.name + or (conv_dir / META_NAME).is_symlink() + or (conv_dir / MESSAGES_NAME).is_symlink() + ): continue try: records.append(ConversationRecord.from_meta(Path(cwd), data)) @@ -189,13 +221,17 @@ def latest_unfinished(cwd: str | os.PathLike[str]) -> ConversationRecord | None: def _count_user_turns(messages: list[dict[str, Any]]) -> int: - return sum(1 for m in messages if m.get("role") == "user") + return sum( + 1 + for m in messages + if m.get("role") == "user" and not m.get("_internal") + ) def _derive_title(messages: list[dict[str, Any]]) -> str: """First user message, flattened to a short single line.""" for m in messages: - if m.get("role") != "user": + if m.get("role") != "user" or m.get("_internal"): continue text = _message_text(m.get("content")) text = " ".join(text.split()) @@ -217,6 +253,46 @@ def _message_text(content: Any) -> str: return "" +def _messages_for_disk(messages: list[dict[str, Any]]) -> list[dict[str, Any]]: + """Copy history while removing file contents returned by intake tools. + + The live agent keeps full results in memory for the current conversation. + Persisted chat is resumable context, not a second copy of every file the + user authorized Arbor to inspect. + """ + + sanitized: list[dict[str, Any]] = [] + for message in messages: + if message.get("_internal") == "context_summary": + sanitized.append({ + "role": "user", + "_internal": "context_summary", + "content": ( + "[compacted context omitted from persisted intake history; " + "restate the current goal and re-authorize any needed paths]" + ), + }) + continue + content = message.get("content") + if not isinstance(content, list): + sanitized.append(dict(message)) + continue + blocks: list[Any] = [] + for block in content: + if isinstance(block, dict) and block.get("type") == "tool_result": + blocks.append({ + **block, + "content": ( + "[tool result omitted from persisted intake history; " + "ask the user to re-authorize the path before re-reading]" + ), + }) + else: + blocks.append(block) + sanitized.append({**message, "content": blocks}) + return sanitized + + def _atomic_write_json(path: Path, data: dict[str, Any]) -> None: path.parent.mkdir(parents=True, exist_ok=True) payload = json.dumps(data, indent=2, ensure_ascii=False) diff --git a/src/cli/intake/launch_tool.py b/src/cli/intake/launch_tool.py index 495cc5a..1977bb1 100644 --- a/src/cli/intake/launch_tool.py +++ b/src/cli/intake/launch_tool.py @@ -15,7 +15,7 @@ class catches every tool exception and converts it into a tool error from dataclasses import dataclass, field from typing import Any, Literal -from ...core.tools.base import Tool +from ...core.tools.base import PathAuthorizer, Tool PluginMode = Literal["inherit", "load", "disabled"] @@ -43,6 +43,11 @@ class LaunchState: plugin_profile: str | None = None plugin_mode: PluginMode = "inherit" unloaded_skills: list[str] = field(default_factory=list) + # Launch authorization is controlled by the REPL, not inferred by the LLM. + # The tool stages exact arguments first; a later real user turn may approve + # that same immutable plan without another model call. + pending_plan: LaunchPlan | None = None + pending_plan_presented: bool = False # Set when the user runs `/resume` and picks a past session. Typed Any to # avoid importing resume_picker here; it holds a ``ResumableSession``. resume_target: Any = None @@ -59,20 +64,20 @@ def launched(self) -> bool: class LaunchExperimentTool(Tool): name = "LaunchExperiment" description = ( - "Call this tool when (a) you have confirmed with the user which project " - "directory the experiment runs against, (b) you have a precise refined " - "instruction, and (c) the user has explicitly approved starting. " - "Calling this tool ends the planning conversation and hands the plan " - "to the coordinator.\n" + "Stage the exact research plan that you want to show the user. Calling " + "this tool does NOT launch anything. It records immutable candidate " + "arguments; after the tool result, present that plan and ask the user " + "for approval. The CLI controller launches the same staged plan only " + "after a later, explicit user confirmation.\n" "\n" "Before calling this tool you MUST:\n" " 1. Know the absolute path of the target project (the `cwd` argument)\n" " 2. Have read enough of that project to write a precise instruction\n" - " 3. Have shown the user your proposed plan in plain language\n" - " 4. Have received explicit user confirmation (e.g. 'yes', 'go', 'start')\n" + " 3. Be ready to present the proposed plan in plain language after " + "this tool returns\n" "\n" - "Do not call this tool speculatively. If unsure, ask the user a clarifying " - "question first." + "Do not call this tool speculatively. If required details are unclear, " + "ask the user a clarifying question first." ) input_schema = { "type": "object", @@ -147,11 +152,27 @@ class LaunchExperimentTool(Tool): }, "required": ["cwd", "instruction"], } - is_read_only = True # Doesn't touch the filesystem + # Stateful control operation: serialize duplicate calls even though it does + # not write project files. + is_read_only = False + yield_after_execute = True - def __init__(self, *, cwd: str, workspace_dir: str | None = None, - state: LaunchState) -> None: - super().__init__(cwd=cwd, workspace_dir=workspace_dir) + def should_yield_after_execute(self, output: str) -> bool: + return output.startswith("STAGED —") + + def __init__( + self, + *, + cwd: str, + workspace_dir: str | None = None, + state: LaunchState, + path_authorizer: PathAuthorizer | None = None, + ) -> None: + super().__init__( + cwd=cwd, + workspace_dir=workspace_dir, + path_authorizer=path_authorizer, + ) self._state = state async def execute(self, **kwargs: Any) -> str: @@ -167,6 +188,11 @@ async def execute(self, **kwargs: Any) -> str: if self._state.launched: return ("ERROR: experiment was already launched in this session; " "you should stop now and let the coordinator run") + if self._state.pending_plan is not None: + return ( + "ERROR: a plan is already staged. Wait for the user's response " + "instead of staging another plan in the same turn." + ) # Validate the target dir exists. If not, tell the agent so it can # re-ask the user instead of launching against nothing. @@ -174,6 +200,10 @@ async def execute(self, **kwargs: Any) -> str: if not resolved.is_absolute(): return (f"ERROR: cwd must be an absolute path (got {target_cwd!r}). " f"Ask the user for the full path.") + authorized, blocked = self.authorize_path(str(resolved)) + if blocked: + return f"BLOCKED: {blocked}" + resolved = _P(authorized) if not resolved.exists() or not resolved.is_dir(): return (f"ERROR: cwd does not exist or is not a directory: {resolved}. " f"Ask the user to confirm the path.") @@ -192,11 +222,11 @@ async def execute(self, **kwargs: Any) -> str: plugin_mode=self._state.plugin_mode, unloaded_skills=list(self._state.unloaded_skills), ) - self._state.plan = plan + self._state.pending_plan = plan + self._state.pending_plan_presented = False return ( - "OK — plan accepted. The planning conversation is now over and the " - "coordinator will take over. End your turn with a brief confirmation " - "to the user; do not call any more tools." + "STAGED — no experiment has launched. The CLI will present this " + "exact plan and wait for a later, explicit user confirmation." ) diff --git a/src/cli/intake/repl.py b/src/cli/intake/repl.py index 06eec10..a73717e 100644 --- a/src/cli/intake/repl.py +++ b/src/cli/intake/repl.py @@ -27,7 +27,6 @@ from ...core.config import AgentConfig from ...core.llm.base import LLMProvider from ...core.tools.base import Tool -from ...core.tools.bash import BashTool from ...core.tools.file_read import FileReadTool from ...core.tools.glob_tool import GlobTool from ...core.tools.grep import GrepTool @@ -42,7 +41,13 @@ ) from .display import IntakeDisplay from .launch_tool import LaunchExperimentTool, LaunchPlan, LaunchState -from .system_prompt import build_system_prompt +from .scope import ( + IntakeMode, + IntakePathPolicy, + infer_intake_mode, + is_explicit_launch_approval, +) +from .system_prompt import build_discussion_system_prompt, build_system_prompt from ..resume_picker import ResumableSession @@ -82,8 +87,8 @@ async def run_intake( - ``ResumableSession`` — the user ran ``/resume`` and picked a past run - ``None`` — the user aborted - `starting_cwd` is the directory the user invoked the CLI from. It is - only a hint — the agent must confirm or correct it before launching. + `starting_cwd` is the directory the user invoked the CLI from. It is the + default project boundary until the user explicitly names another target. The intake workspace dir (tool persistence, etc.) lives next to it. The conversation itself is auto-saved every turn under @@ -97,31 +102,60 @@ async def run_intake( intake_workspace = (workspace_dir or (starting_cwd / CONFIG_DIR_NAME / "_intake")).resolve() intake_workspace.mkdir(parents=True, exist_ok=True) - # Tools are constructed with starting_cwd as the base for *relative* - # paths; absolute paths work unrestricted, which is what we want here. - tools: list[Tool] = [ - FileReadTool(cwd=str(starting_cwd), workspace_dir=str(intake_workspace)), - GlobTool(cwd=str(starting_cwd), workspace_dir=str(intake_workspace)), - GrepTool(cwd=str(starting_cwd), workspace_dir=str(intake_workspace)), - BashTool(cwd=str(starting_cwd), workspace_dir=str(intake_workspace)), - LaunchExperimentTool(cwd=str(starting_cwd), workspace_dir=str(intake_workspace), state=state), + path_policy = IntakePathPolicy(starting_cwd) + + # Intake is read-only. Every file tool shares a mutable, canonical path + # policy, so a user correction changes the enforced scope before the next + # model call. Shell access belongs to the launched coordinator, not to a + # planning/discussion chat. + file_tools: list[Tool] = [ + FileReadTool( + cwd=str(starting_cwd), + workspace_dir=str(intake_workspace), + path_authorizer=path_policy.authorize, + persist_results=False, + ), + GlobTool( + cwd=str(starting_cwd), + workspace_dir=str(intake_workspace), + path_authorizer=path_policy.authorize, + persist_results=False, + ), + GrepTool( + cwd=str(starting_cwd), + workspace_dir=str(intake_workspace), + path_authorizer=path_policy.authorize, + persist_results=False, + ), ] + launch_tool = LaunchExperimentTool( + cwd=str(starting_cwd), + workspace_dir=str(intake_workspace), + state=state, + path_authorizer=path_policy.authorize, + ) + tools: list[Tool] = [*file_tools, launch_tool] agent_config = AgentConfig( cwd=str(starting_cwd), provider=_provider_label(provider), model=getattr(provider, "model", "unknown"), max_turns=intake_max_turns, + yield_on_text=True, + premature_stop_nudges=False, llm_retry_attempts=INTAKE_LLM_RETRY_ATTEMPTS, llm_retry_base_delay=INTAKE_LLM_RETRY_BASE_DELAY, llm_retry_max_delay=INTAKE_LLM_RETRY_MAX_DELAY, - auto_git=False, # the intake agent is read-only — no commits + auto_git=False, # intake exposes no write/shell tools ) agent = Agent( provider=provider, tools=tools, - system_prompt=build_system_prompt(starting_cwd=str(starting_cwd)), + system_prompt=build_system_prompt( + starting_cwd=str(starting_cwd), + approved_scope=path_policy.describe(), + ), config=agent_config, ) @@ -142,6 +176,19 @@ async def run_intake( prior = latest_unfinished(starting_cwd) if prior is not None: agent.messages = seal_interrupted_tail(load_messages(prior)) + # Conversation prose is resumable; filesystem authority is not. + # Persisted transcripts and LLM-generated compaction summaries are + # untrusted inputs, so the user must name paths again in a fresh + # terminal turn before tools can read them. + path_policy.reset(mode=IntakeMode.DISCUSSION) + _configure_intake_agent( + agent, + mode=path_policy.mode, + starting_cwd=starting_cwd, + path_policy=path_policy, + file_tools=file_tools, + launch_tool=launch_tool, + ) conv = prior _console.print( f"[green]Continuing your last conversation[/] " @@ -185,10 +232,11 @@ def _persist() -> None: action = _handle_slash( user_text, agent, - tools, + list(agent.tools.values()), state, starting_cwd=starting_cwd, model=getattr(provider, "model", None), + path_policy=path_policy, ) if action == "quit": return None @@ -201,9 +249,61 @@ def _persist() -> None: # reloaded it into the agent. Switch onward autosaves to that # record and keep chatting in this same intake loop. conv = state.resume_conversation + _configure_intake_agent( + agent, + mode=path_policy.mode, + starting_cwd=starting_cwd, + path_policy=path_policy, + file_tools=file_tools, + launch_tool=launch_tool, + ) + continue + if action == "reset": + _configure_intake_agent( + agent, + mode=path_policy.mode, + starting_cwd=starting_cwd, + path_policy=path_policy, + file_tools=file_tools, + launch_tool=launch_tool, + ) continue continue + # Route every real user turn before the LLM sees it. Discussion mode + # has no launch capability and no implicit cwd access; planning mode is + # confined to the starting project plus explicitly named paths. + mode = infer_intake_mode(user_text, path_policy.mode) + path_policy.update(user_text, mode) + # Approve the exact staged plan in controller code. The confirmation + # never goes back through the model, so it cannot silently rewrite the + # cwd/instruction or attach a different tool call after the user says go. + if ( + mode == IntakeMode.PLANNING + and state.pending_plan is not None + and state.pending_plan_presented + and is_explicit_launch_approval(user_text) + ): + agent.messages.append({"role": "user", "content": user_text}) + state.plan = state.pending_plan + state.pending_plan = None + state.pending_plan_presented = False + _persist() + return state.plan + + # Any non-approval real message edits or rejects the candidate. The + # model must stage a fresh plan reflecting the new instruction. + state.pending_plan = None + state.pending_plan_presented = False + _configure_intake_agent( + agent, + mode=mode, + starting_cwd=starting_cwd, + path_policy=path_policy, + file_tools=file_tools, + launch_tool=launch_tool, + ) + # Hand off to the agent under a compact display. try: with IntakeDisplay(console=_console): @@ -213,6 +313,18 @@ def _persist() -> None: if typer.confirm("Quit intake?", default=False): return None continue + + if ( + mode == IntakeMode.PLANNING + and state.pending_plan is not None + and agent.stop_reason == "awaiting_user" + ): + state.pending_plan_presented = True + _print_research_contract(state.pending_plan) + _console.print( + "[bold]Start this exact staged plan?[/bold] " + "[dim](reply yes/go/start, or describe an edit)[/dim]\n" + ) if _is_llm_failure_reply(reply): _print_llm_failure_hint( provider=agent_config.provider, @@ -238,6 +350,31 @@ def _persist() -> None: # ── helpers ──────────────────────────────────────────────────────── +def _configure_intake_agent( + agent: Agent, + *, + mode: IntakeMode, + starting_cwd: Path, + path_policy: IntakePathPolicy, + file_tools: list[Tool], + launch_tool: LaunchExperimentTool, +) -> None: + """Atomically refresh prompt and capabilities for one user turn.""" + + active_tools = [*file_tools, launch_tool] if mode == IntakeMode.PLANNING else file_tools + agent.tools = {tool.name: tool for tool in active_tools} + if mode == IntakeMode.DISCUSSION: + agent.system_prompt = build_discussion_system_prompt( + starting_cwd=str(starting_cwd), + approved_scope=path_policy.describe(), + ) + else: + agent.system_prompt = build_system_prompt( + starting_cwd=str(starting_cwd), + approved_scope=path_policy.describe(), + ) + + def _is_llm_failure_reply(reply: str | None) -> bool: if not reply: return False @@ -541,7 +678,7 @@ def _print_plan_accepted(plan: LaunchPlan | None) -> None: for n in plan.notes: lines.append(f" - {n}") _console.print() - _console.print(Panel("\n".join(lines), title="Plan accepted — handing off", + _console.print(Panel("\n".join(lines), title="Staged plan", border_style="green")) @@ -596,6 +733,8 @@ def _print_resumed_history(messages: list[dict]) -> None: """ rendered = False for m in messages: + if m.get("_internal"): + continue text = _visible_text(m.get("content")) if not text: continue @@ -661,6 +800,7 @@ def _handle_slash( *, starting_cwd: Path | None = None, model: str | None = None, + path_policy: IntakePathPolicy | None = None, ) -> str: cmd = line.lower().split()[0] if cmd == "/help": @@ -695,6 +835,10 @@ def _handle_slash( return "continue" # user declined (N) → stay in intake if isinstance(chosen, ConversationRecord): agent.messages = seal_interrupted_tail(load_messages(chosen)) + if path_policy is not None: + path_policy.reset(mode=IntakeMode.DISCUSSION) + state.pending_plan = None + state.pending_plan_presented = False state.resume_conversation = chosen _console.print( f"[green]Resumed conversation[/] " @@ -716,25 +860,52 @@ def _handle_slash( _console.print(f" [dim]skills loaded[/dim] {escape(loaded_skills)}") _console.print(f" [dim]skills unloaded[/dim] {escape(unloaded_skills)}") _console.print(f" [dim]turns[/dim] {agent.total_turns}") - _console.print(f" [dim]pending plan[/dim] {'yes' if state.plan else 'no'}") + if path_policy is not None: + _console.print(f" [dim]mode[/dim] {escape(path_policy.mode.value)}") + _console.print( + f" [dim]approved scope[/dim] {escape(path_policy.describe())}" + ) + _console.print( + f" [dim]suppressed mixed tool calls[/dim] " + f"{len(agent.suppressed_tool_uses)}" + ) + pending = state.plan is not None or state.pending_plan is not None + _console.print(f" [dim]pending plan[/dim] {'yes' if pending else 'no'}") elif cmd == "/plugin": + before = (state.plugin, state.plugin_profile, state.plugin_mode) _handle_plugin_command(line, state, starting_cwd=starting_cwd) + after = (state.plugin, state.plugin_profile, state.plugin_mode) + if after != before: + state.pending_plan = None + state.pending_plan_presented = False elif cmd == "/skill": + before = tuple(state.unloaded_skills) _handle_skill_command(line, state, starting_cwd=starting_cwd) + if tuple(state.unloaded_skills) != before: + state.pending_plan = None + state.pending_plan_presented = False elif cmd in ("/quit", "/abort"): return "quit" elif cmd == "/reset": agent.messages.clear() + if path_policy is not None: + path_policy.reset() + state.plan = None + state.pending_plan = None + state.pending_plan_presented = False + agent.suppressed_tool_uses.clear() _console.print("[dim]history cleared[/dim]") + return "reset" elif cmd == "/tools": for t in tools: _console.print(f" - {t.name}: {t.description.splitlines()[0]}") elif cmd in ("/plan", "/contract"): - if state.plan: + preview = state.plan or state.pending_plan + if preview: if cmd == "/contract": - _print_research_contract(state.plan) + _print_research_contract(preview) else: - _print_plan_accepted(state.plan) + _print_plan_accepted(preview) else: _console.print("[dim]no contract yet[/dim]") else: diff --git a/src/cli/intake/scope.py b/src/cli/intake/scope.py new file mode 100644 index 0000000..2229d3a --- /dev/null +++ b/src/cli/intake/scope.py @@ -0,0 +1,352 @@ +"""Intent and filesystem-scope policy for the interactive intake chat. + +The intake agent is conversational, not an unattended code agent. This module +keeps two control decisions out of the LLM prompt: + +* whether the current conversation is discussion or launch planning; +* which canonical paths the user has actually authorized for file tools. + +Both decisions are deliberately conservative. Ambiguous requests keep their +current mode, and discussion mode has no implicit filesystem access. +""" + +from __future__ import annotations + +import os +import re +from dataclasses import dataclass +from enum import Enum +from pathlib import Path + + +class IntakeMode(str, Enum): + """The two supported intake interaction modes.""" + + DISCUSSION = "discussion" + PLANNING = "planning" + + +_NEGATED_LAUNCH_RE = re.compile( + r"(?:不要|无需|不需要|先不|暂不|别)(?:再)?(?:启动|运行|执行|跑)|" + r"(?:不要|别)做实验|" + r"\b(?:do\s+not|don't|dont|not\s+yet)\s+(?:launch|run|execute|start)\b", + re.IGNORECASE, +) +_EXPLICIT_LAUNCH_RE = re.compile( + r"(?:开始|启动|运行|执行|开跑)(?:吧|它|这个|一下|起来)?(?:$|[，。！？,.!?])|" + r"(?:启动|开始|运行|执行|跑)(?:这个|一次|一轮|新的)?(?:实验|评测|研究|agent|项目)|" + r"(?:提分|跑实验|开始实验)|" + r"\b(?:launch|start|run|execute)\s+(?:it|this|now)\b|" + r"\b(?:launch|start|run|execute)\s+(?:the\s+|an?\s+)?(?:experiment|benchmark|research\s+run|agent)\b|" + r"\b(?:go ahead|kick (?:it|this) off)\b", + re.IGNORECASE | re.DOTALL, +) +_PLANNING_RE = re.compile( + r"(?:优化|提高|提升|最大化|最小化).{0,20}(?:指标|分数|得分|准确率|score|metric)|" + r"\b(?:optimi[sz]e|maximi[sz]e|minimi[sz]e|improve|beat)\b.{0,40}" + r"\b(?:score|metric|accuracy|benchmark|baseline)\b", + re.IGNORECASE | re.DOTALL, +) +_DISCUSSION_RE = re.compile( + r"(?:只|先|帮我)?(?:阅读|读一下|看看|分析|讨论|评估|评价|梳理|排查)|" + r"(?:研究方向|论文方向|想法|思路|故事|新颖性|novelty|topic)|" + r"\b(?:read|review|discuss|analy[sz]e|brainstorm|explain|compare)\b", + re.IGNORECASE, +) + +_LAUNCH_APPROVAL_RE = re.compile( + r"(?:(?:yes|y|ok|okay|sounds good|approved?)" + r"(?:\s*,?\s*(?:please|go ahead|proceed|start|launch|do it|kick it off))?|" + r"go|go ahead|proceed|start|launch|do it|kick it off)|" + r"(?:(?:好|好的|可以|同意|没问题)" + r"(?:[，,\s]*(?:开始|启动|开跑|执行)(?:吧)?)?|" + r"(?:确认[，,\s]*)?(?:开始|启动|开跑|执行)(?:吧)?|就这样)", + re.IGNORECASE, +) + + +def infer_intake_mode( + message: str, + current: IntakeMode | None = None, +) -> IntakeMode: + """Infer the interaction mode while respecting explicit negative intent. + + Launch wording wins over generic discussion wording, except when the user + explicitly says not to launch or run anything. With no signal, preserve + the current mode; fresh sessions retain Arbor's historical planning default. + """ + + text = " ".join((message or "").split()) + if _NEGATED_LAUNCH_RE.search(text): + return IntakeMode.DISCUSSION + if _EXPLICIT_LAUNCH_RE.search(text): + return IntakeMode.PLANNING + if _DISCUSSION_RE.search(text): + return IntakeMode.DISCUSSION + if _PLANNING_RE.search(text): + return IntakeMode.PLANNING + return current or IntakeMode.PLANNING + + +def is_explicit_launch_approval(message: str) -> bool: + """Return whether *message* unambiguously approves the staged launch. + + Approval is intentionally a whole-message decision. A reply such as + ``"yes, but use a different split"`` changes the plan and must be staged + again instead of approving stale arguments. + """ + + text = " ".join((message or "").strip().split()).strip("。！!，,") + return bool(_LAUNCH_APPROVAL_RE.fullmatch(text)) + + +# Bare paths require at least one separator. This intentionally ignores lone +# filenames in prose: restricting to a guessed file would be worse than asking +# the user for an exact path. Backtick/quote extraction below still accepts a +# quoted lone filename when it exists. +_BARE_PATH_RE = re.compile( + r"(?:~[/\\]|/|\.\.?[/\\]|[A-Za-z0-9_.~-]+[/\\])" + r"[^\s`\"'<>|，。；;：:!?！？]+" +) +_QUOTED_PATH_RE = re.compile(r"`([^`\n]+)`|['\"]([^'\"\n]+)['\"]") +_TRAILING_PATH_PUNCTUATION = ".,;:!?，。；：！？)]}）】》" +_LEADING_PATH_PUNCTUATION = "([{（【《" + + +def extract_explicit_paths(message: str) -> list[str]: + """Return path-like strings explicitly present in a user message.""" + + found: list[str] = [] + + def _add(raw: str, *, quoted: bool = False) -> None: + candidate = raw.strip().lstrip(_LEADING_PATH_PUNCTUATION).rstrip( + _TRAILING_PATH_PUNCTUATION + ) + if not candidate or "://" in candidate or candidate.startswith("//"): + return + if "/" not in candidate and "\\" not in candidate: + # A quoted lone filename is still unambiguous enough to resolve. + if not Path(candidate).suffix: + return + elif not quoted and not _is_strong_path_candidate(candidate): + # Slash-separated prose such as ``client/server`` is common. Bare + # paths must carry a strong signal (absolute/dot-relative, Windows + # separators, or a filename suffix); quote a relative directory to + # authorize it deliberately. + return + if candidate not in found: + found.append(candidate) + + for match in _QUOTED_PATH_RE.finditer(message or ""): + _add(match.group(1) or match.group(2) or "", quoted=True) + for match in _BARE_PATH_RE.finditer(message or ""): + _add(match.group(0)) + return found + + +def _is_within(path: Path, root: Path) -> bool: + try: + path.relative_to(root) + return True + except ValueError: + return False + + +@dataclass +class IntakePathPolicy: + """Mutable, session-level path authorization for intake file tools. + + Planning mode may inspect the confirmed starting project. Discussion mode + may inspect only paths explicitly named by the user. Explicit paths replace + the previous discussion scope, so a correction narrows access instead of + silently accumulating old projects. + """ + + starting_cwd: Path + mode: IntakeMode = IntakeMode.PLANNING + explicit_paths: tuple[Path, ...] = () + explicit_directories: tuple[Path, ...] = () + unresolved_paths: tuple[str, ...] = () + target_confirmation_required: bool = False + + def __post_init__(self) -> None: + self.starting_cwd = self.starting_cwd.expanduser().resolve() + + def reset(self, *, mode: IntakeMode = IntakeMode.PLANNING) -> None: + self.mode = mode + self.explicit_paths = () + self.explicit_directories = () + self.unresolved_paths = () + self.target_confirmation_required = False + + def update(self, message: str, mode: IntakeMode) -> None: + """Apply a user turn to the current mode and approved scope.""" + + mode_changed = mode != self.mode + previous_paths = self.explicit_paths + self.mode = mode + candidates = extract_explicit_paths(message) + if not candidates: + # Scope does not flow implicitly between discussion and launch + # planning. The two modes grant different capabilities, so a + # transition without a path fails closed until the user names the + # target again (planning still has its explicit cwd default). + if mode_changed: + self.explicit_paths = () + self.explicit_directories = () + self.unresolved_paths = () + self.target_confirmation_required = bool( + mode == IntakeMode.PLANNING + and any( + not _is_within(path, self.starting_cwd) + for path in previous_paths + ) + ) + return + + resolved: list[Path] = [] + directories: list[Path] = [] + unresolved: list[str] = [] + for candidate in candidates: + path = self._resolve_user_path(candidate) + if path is None: + unresolved.append(candidate) + elif path not in resolved: + resolved.append(path) + if path.is_dir(): + directories.append(path) + + self.explicit_paths = tuple(resolved) + self.explicit_directories = tuple(directories) + self.unresolved_paths = tuple(unresolved) + self.target_confirmation_required = False + + def authorize(self, canonical_path: str) -> str | None: + """Return a user-facing denial when *canonical_path* is out of scope.""" + + path = Path(canonical_path) + allowed = self._allowed_roots() + if any( + path == root + or ( + (root == self.starting_cwd or root in self.explicit_directories) + and _is_within(path, root) + ) + for root in allowed + ): + return None + + if self.mode == IntakeMode.DISCUSSION: + if self.unresolved_paths and not self.explicit_paths: + detail = ", ".join(self.unresolved_paths) + return ( + "the path is outside the user's approved discussion scope; " + f"the explicitly named path(s) could not be resolved: {detail}" + ) + return ( + "the path is outside the user's approved discussion scope. " + "Only explicitly named files or directories may be inspected; " + "ask the user before expanding scope" + ) + roots = ", ".join(str(root) for root in allowed) or "none" + return ( + f"the path is outside the approved project scope ({roots}). " + "Ask the user to name the external path explicitly" + ) + + def describe(self) -> str: + """Render the current authorization boundary for the system prompt.""" + + allowed = self._allowed_roots() + if allowed: + lines = [f"- {path}" for path in allowed] + scope = "\n".join(lines) + else: + scope = "- no filesystem paths are currently approved" + unresolved = "" + if self.unresolved_paths: + unresolved = ( + "\nThe following user-supplied paths could not be resolved; ask for " + "a corrected absolute path instead of searching parent directories: " + + ", ".join(self.unresolved_paths) + ) + if self.target_confirmation_required: + unresolved += ( + "\nThe discussion concerned paths outside the launch project. " + "Ask the user to name the intended project directory explicitly " + "before inspecting files or staging a run." + ) + return scope + unresolved + + def _allowed_roots(self) -> tuple[Path, ...]: + if self.target_confirmation_required: + return () + if self.mode == IntakeMode.DISCUSSION: + return self.explicit_paths + if self.unresolved_paths: + return () + if self.explicit_paths: + external = tuple( + path + for path in self.explicit_paths + if ( + not _is_within(path, self.starting_cwd) + and path in self.explicit_directories + ) + ) + # An explicitly named external target redirects intake away from + # the launch directory. Do not retain broad access to both trees. + if external: + return external + external_files = tuple( + path + for path in self.explicit_paths + if not _is_within(path, self.starting_cwd) + ) + if external_files: + return external_files + return (self.starting_cwd,) + return (self.starting_cwd,) + + def _resolve_user_path(self, raw: str) -> Path | None: + normalized = raw.replace("\\", os.sep) if os.sep == "/" else raw + expanded = Path(normalized).expanduser() + if expanded.is_absolute(): + lexical_candidates = [expanded] + else: + # A common workspace layout has the CLI repo and user material as + # siblings. Resolve the exact, explicitly named path against the + # launch directory and its immediate parent only. Never glob or + # search ancestors, and never climb more than this one documented + # workspace-relative fallback. + bases = [self.starting_cwd, self.starting_cwd.parent] + lexical_candidates = [base / expanded for base in bases] + + for lexical in lexical_candidates: + absolute = Path(os.path.abspath(lexical)) + if not absolute.exists(): + continue + try: + canonical = absolute.resolve() + except OSError: + continue + # Do not let an in-project symlink turn implicit project permission + # into access outside the project. An explicitly named absolute or + # ancestor-relative external path remains allowed as itself. + if _is_within(absolute, self.starting_cwd) and not _is_within( + canonical, self.starting_cwd + ): + continue + return canonical + return None + + +def _is_strong_path_candidate(candidate: str) -> bool: + """Distinguish likely paths from conceptual slash-separated prose.""" + + normalized = candidate.replace("\\", "/") + return ( + normalized.startswith(("/", "~/", "./", "../")) + or "\\" in candidate + or bool(Path(normalized).suffix) + ) diff --git a/src/cli/intake/system_prompt.py b/src/cli/intake/system_prompt.py index f0d3325..850de5b 100644 --- a/src/cli/intake/system_prompt.py +++ b/src/cli/intake/system_prompt.py @@ -25,12 +25,11 @@ def _prior_experience_block(starting_cwd: str) -> str: ) -def build_system_prompt(*, starting_cwd: str) -> str: +def build_system_prompt(*, starting_cwd: str, approved_scope: str | None = None) -> str: """Build the planning-agent system prompt. - `starting_cwd` is just the directory from which the user invoked the CLI. - It is a *hint*, not the assumed target. The agent is responsible for - confirming (or finding) the actual project directory with the user. + `starting_cwd` is the default project root. File tools enforce the + user-approved scope supplied by the intake controller. """ return ( "You are the AutoResearch planning assistant. Your job is to help the " @@ -71,7 +70,8 @@ def build_system_prompt(*, starting_cwd: str) -> str: " - **No eval yet** → ask what 'better' means (which output, which " "metric, higher or lower is better), then offer to scaffold a minimal " "eval script that runs the code and prints a `score:` line. Show it, get " - "a yes, and verify it actually runs before launching.\n" + "a yes, and state in the plan that measurement setup is the first " + "coordinator task. Do not create or run it during intake.\n" " - **No held-out split** → explain in one sentence why iterating and " "scoring on the same data overfits, then offer to carve a dev/held-out " "split (e.g. a random or time-based partition) so the coordinator " @@ -83,9 +83,8 @@ def build_system_prompt(*, starting_cwd: str) -> str: "signal. State plainly in the instruction that there is no held-out " "guard, so the final number may be optimistic. Never refuse a run just " "because a split is missing.\n" - "Anything you scaffold here is *measurement* plumbing (an eval script, a " - "split helper), not the solution. You may create those files with the " - "user's confirmation; you must never pre-solve the task itself.\n" + "Measurement scaffolding belongs to the coordinator after launch, not " + "to this read-only intake chat. Never pre-solve the task yourself.\n" "\n" "## Fast path — propose, don't interrogate\n" "Most users want to get going, not fill out a form. Your DEFAULT " @@ -122,6 +121,8 @@ def build_system_prompt(*, starting_cwd: str) -> str: "\n" "## Target directory\n" f"The user launched you from: {starting_cwd}\n" + "The file tools currently enforce this approved scope:\n" + f"{approved_scope or f'- {starting_cwd}'}\n" "\n" + _prior_experience_block(starting_cwd) + "Treat that path as the default target. In the common case the user " @@ -132,43 +133,28 @@ def build_system_prompt(*, starting_cwd: str) -> str: "is off.\n" "\n" "Switch directories only when the user explicitly names a different " - "path, or when the cwd is clearly wrong (e.g. an empty home dir, no " - "code files anywhere). In those cases ask once, naturally, then move " - "on. The directory is just context for the plan, not a ceremony.\n" + "path. Never inspect parent or sibling directories to hunt for a more " + "relevant project. If a named path cannot be resolved, ask the user for " + "a corrected absolute path instead of searching around it.\n" "\n" "## Your tools\n" - "- Read, Glob, Grep: explore ANY directory with absolute paths. " - "Use them to ground your questions in real code.\n" - "- Bash: run shell commands in the user's working directory. " - "Prefer doing setup work YOURSELF rather than instructing the user. " - "(See the 'Bash usage' section below.)\n" - "- LaunchExperiment(cwd, instruction, …): end the conversation and " - "start the coordinator. The `cwd` argument is REQUIRED and must be the " + "- Read, Glob, Grep: inspect only the approved scope shown above. " + "Out-of-scope access is blocked even when an absolute path is used.\n" + "- LaunchExperiment(cwd, instruction, …): stage an exact candidate plan " + "for user approval. It does not launch by itself; the CLI presents the " + "staged plan and ends this internal turn. The `cwd` argument is " + "REQUIRED and must be the " "absolute path of the target project (default to the launch directory " "unless the user redirected you).\n" "\n" - "## Bash usage — assume the user is busy/lazy\n" - "If the project needs a one-line fix-up before the coordinator can run " - "(e.g. `git init` + initial commit, `bash setup.sh`, `chmod +x eval.sh`, " - "verifying `eval.sh` produces a `score:` line), JUST DO IT — don't lecture " - "the user with copy-paste instructions. Briefly say what you're about to " - "run, then run it.\n" - "\n" - "Hard rules for Bash:\n" - " - Allowed without asking: `git init`, `git add`, `git commit`, " - "`git status`, `git log`, `git diff`, `chmod`, `mkdir`, `bash setup.sh`, " - "running an eval script to confirm it works, listing/inspecting files.\n" - " - Confirm with the user before: installing packages (pip / brew / apt), " - "downloading data, modifying files outside the project dir, anything " - "with `sudo`, anything with `rm -rf`.\n" - " - Do NOT edit the project's *solution* code yourself — improving that " - "is the coordinator's job. ONE exception: if the repo has no eval harness " - "or no held-out split, you MAY scaffold those *measurement* files (an " - "eval script, a dev/held-out split helper) — but only after the user " - "confirms what counts as success, and never the solution logic itself. " - "The intake phase is for setup, measurement scaffolding, and planning.\n" - " - Use absolute paths or stay in the confirmed project dir; don't `cd` " - "around.\n" + "## Tool-turn protocol\n" + "This is an interactive chat. A visible text response hands control back " + "to the user. When you need a tool, return ONLY the tool call with no " + "preface, progress sentence, question, or other visible text. After the " + "tool result arrives you may either call another tool-only turn or answer " + "the user. Never combine a question with a tool call. You have no shell or " + "file-editing tool during intake; setup and execution happen only after " + "the user approves a launch.\n" "\n" "## Style\n" "- Talk like a thoughtful collaborator, not a form. Two or three short " @@ -304,8 +290,7 @@ def build_system_prompt(*, starting_cwd: str) -> str: "\n" "Background work you do silently (do NOT ask the user about these):\n" " - Locating the relevant code, the eval script, the dataset.\n" - " - Verifying git is clean / initializing it if missing.\n" - " - Confirming the eval produces a numeric score.\n" + " - Reading existing metadata that identifies the eval and baseline.\n" "These are checks, not questions. The user's screen time is " "reserved for the launch parameters above.\n" "\n" @@ -313,18 +298,11 @@ def build_system_prompt(*, starting_cwd: str) -> str: "at code — don't guess. If you find yourself asking about anything " "that ISN'T in the launch-parameter list, stop and rephrase.\n" "\n" - "## Quick environment check before launching\n" - "Run a fast sanity pass with Bash + Glob in the target dir, silently:\n" - " - `git status` — must be a clean repo. If `.git` is missing run " - "`git init && git add . && git commit -m baseline`. If dirty, ask the " - "user to commit/stash, OR commit on their behalf with a clear message.\n" - " - Look for an eval script (`eval.sh`, `evaluate.py`, etc.). If a " - "`setup.sh` exists, run it.\n" - " - Optional: run the eval once to confirm it produces a baseline " - "`score:` line (this catches broken eval scripts before the coordinator " - "wastes cycles on them).\n" - "Don't be alarmist — clean projects are the common case. Only surface " - "issues to the user when there's something they actually need to fix.\n" + "## Quick read-only check before launching\n" + "Use Glob/Read to locate existing eval and configuration files inside " + "the approved project scope. Do not run setup, eval, git, or shell " + "commands. A deterministic preflight runs after the plan is approved and " + "will surface environment problems before the coordinator starts.\n" "\n" "## When you propose the plan\n" "Don't use a rigid template. Summarize the plan naturally in 2-3 " @@ -339,17 +317,64 @@ def build_system_prompt(*, starting_cwd: str) -> str: "user should see *the goal handed off to the coordinator*, not *a " "method you committed to on their behalf*.\n" "\n" - "Then ask whether to start, in your own words. Vary the phrasing " - "between conversations — 'shall we?', 'good to go?', 'want me to " - "kick this off?', whatever fits the tone. Avoid mechanical phrases " - "like 'Goal: ... / Approach: ... / Budget: ... / Ready to start?' " - "every time — that reads like a form, not a chat.\n" - "Only after the user confirms, call LaunchExperiment with `cwd` set " - "to the agreed project path.\n" - "\n" - "## When NOT to call LaunchExperiment\n" - "- The user hasn't explicitly confirmed the plan.\n" - "- The user is still asking questions or pushing back.\n" + "When the plan is complete, call LaunchExperiment in a tool-only turn " + "to stage its exact arguments. Do not add a text preface or attempt to " + "summarize it afterward; the CLI renders the staged contract and asks " + "for approval itself. The CLI controller " + "handles the user's later confirmation and launches the staged plan " + "without another model turn.\n" + "\n" + "## When NOT to stage LaunchExperiment\n" + "- Required plan details are still ambiguous.\n" + "- The user is still asking questions or pushing back on the goal.\n" "- You haven't looked at any project code.\n" "Calling it speculatively wastes the user's time and money." ) + + +def build_discussion_system_prompt( + *, + starting_cwd: str, + approved_scope: str, +) -> str: + """Build the prompt for read-only research discussion mode.""" + + return ( + "You are Arbor's research discussion assistant. The user is asking you " + "to read, analyze, compare, or brainstorm — not to launch an autonomous " + "benchmark run. Answer the research question directly and thoughtfully.\n" + "\n" + "## Hard boundary\n" + "Do not turn this conversation into project setup. Do not search for an " + "implementation repo, benchmark, dataset, eval script, git history, or " + "related project unless the user explicitly asks for that exact thing. " + "Never inspect parent or sibling directories to infer what the user might " + "have meant. Do not ask how this discussion relates to other projects you " + "happen to discover.\n" + "\n" + f"The user launched Arbor from: {starting_cwd}\n" + "The only filesystem paths currently approved are:\n" + f"{approved_scope}\n" + "Read, Glob, and Grep are physically restricted to that scope. If a named " + "path could not be resolved, ask for the corrected absolute path; do not " + "search nearby directories. No shell, write, or launch tool is available " + "in discussion mode.\n" + "\n" + "## Tool-turn protocol\n" + "When you need to inspect an approved file, return ONLY the tool call — no " + "preface or other visible text. After the result arrives, answer the user. " + "A visible text response ends your turn and waits for the user. Never mix " + "a question or progress update with a tool call.\n" + "\n" + "## Discussion quality\n" + "Separate evidence from conjecture. Challenge novelty claims, identify " + "clean research questions, compare alternative explanations, and state " + "what evidence would distinguish them. Stay focused on the files and " + "research direction the user named. You may suggest experiments at a " + "conceptual level, but do not begin implementing or locating code.\n" + "\n" + "Use concise Markdown. Ask at most one focused question when a genuine " + "research ambiguity blocks a useful answer; otherwise answer directly. " + "If the user later explicitly asks to start or launch an experiment, the " + "CLI will switch you into planning mode on the next turn." + ) diff --git a/src/core/agent.py b/src/core/agent.py index 116bb26..b52bdd0 100644 --- a/src/core/agent.py +++ b/src/core/agent.py @@ -214,9 +214,11 @@ def __init__( # views (plain text and {name,input} tool calls). self.assistant_texts: list[str] = [] self.tool_uses: list[dict[str, Any]] = [] + self.suppressed_tool_uses: list[dict[str, Any]] = [] self.total_turns = 0 # Why the loop exited, surfaced to callers alongside total_turns: # "finished" — model produced a final answer with no tool calls + # "awaiting_user" — interactive text yielded control to the frontend # "max_turns" — exhausted the turn budget without a final answer # Stays None if the run was cancelled mid-loop (e.g. timeout), since the # coroutine never reaches a return. Callers detect that case otherwise. @@ -396,18 +398,46 @@ async def _run_loop(self, user_message: str) -> str: if response.stop_reason != "max_tokens": self._max_tokens_recovery_count = 0 - # 5. Print assistant text (if any) text = response.get_text() + tool_calls = response.get_tool_calls() + + # Interactive frontends use visible text as the explicit boundary + # between agent work and the human's next turn. A model may return + # text and tool calls together; executing those calls after showing + # a question makes the UI look idle while the agent keeps acting. + # Keep the prose/reasoning in history, remove the unexecuted calls + # so every persisted tool call remains paired with a result, and + # yield control immediately. Autonomous agents retain the original + # text+tool behavior because this option defaults to False. + if self.config.yield_on_text and text.strip(): + if tool_calls: + self.suppressed_tool_uses.extend( + {"name": call.name, "input": call.input} + for call in tool_calls + ) + self.messages[-1] = { + "role": "assistant", + "content": _without_tool_calls(response.raw_content), + } + _print_status( + "Visible assistant text ended the interactive turn; " + f"discarded {len(tool_calls)} accompanying tool call(s)." + ) + _print_assistant(text) + self.assistant_texts.append(text) + self.stop_reason = "awaiting_user" + return text + + # 5. Print assistant text (if any) if text: _print_assistant(text) self.assistant_texts.append(text) # 6. Check tool calls - tool_calls = response.get_tool_calls() - for _tc in tool_calls: - self.tool_uses.append({"name": _tc.name, "input": _tc.input}) if not tool_calls: if ( + self.config.premature_stop_nudges + and no_tool_nudges < 3 and turn < self.config.max_turns and _looks_like_premature_no_tool_stop(text) @@ -418,6 +448,7 @@ async def _run_loop(self, user_message: str) -> str: ) self.messages.append({ "role": "user", + "_internal": "premature_stop_nudge", "content": ( "Your previous response described future work but did not call any tools. " "Continue now by calling the appropriate tool(s). Do not report completion " @@ -442,6 +473,39 @@ async def _run_loop(self, user_message: str) -> str: no_tool_nudges = 0 + # A frontend control tool changes interaction state and must be the + # only action in its response. Reject the entire batch rather than + # executing ordinary tools before/alongside a staged launch. + if self.config.yield_on_text: + control_calls = [ + call + for call in tool_calls + if getattr(self.tools.get(call.name), "yield_after_execute", False) + ] + if control_calls and len(tool_calls) != 1: + self.suppressed_tool_uses.extend( + {"name": call.name, "input": call.input} + for call in tool_calls + ) + self.messages.append({ + "role": "user", + "content": [ + ToolResultBlock( + tool_use_id=call.id, + content=( + "Error: interactive control tools must be " + "called alone; no tools in this batch were executed." + ), + is_error=True, + ).to_content_block() + for call in tool_calls + ], + }) + continue + + for _tc in tool_calls: + self.tool_uses.append({"name": _tc.name, "input": _tc.input}) + # 6. Execute tools _print_status( f"Turn {turn}: executing {len(tool_calls)} tool(s) " @@ -456,6 +520,19 @@ async def _run_loop(self, user_message: str) -> str: "role": "user", "content": [r.to_content_block() for r in results], }) + result_by_id = {result.tool_use_id: result for result in results} + if self.config.yield_on_text and any( + tool is not None + and (result := result_by_id.get(call.id)) is not None + and tool.should_yield_after_execute(result.content) + for call in tool_calls + if (tool := self.tools.get(call.name)) is not None + ): + _print_status( + "Interactive control tool completed; yielding to the frontend." + ) + self.stop_reason = "awaiting_user" + return "" _print_status(f"Reached max turns ({self.config.max_turns}).") self.stop_reason = "max_turns" @@ -723,6 +800,34 @@ def _retry_delay(*, attempt: int, base_delay: float, max_delay: float) -> float: return round(delay + random.uniform(0, jitter), 2) +_TOOL_CALL_BLOCK_TYPES = frozenset({ + "tool_use", + "function_call", + "custom_tool_call", + "tool_search_call", +}) + + +def _without_tool_calls(content: Any) -> Any: + """Remove unexecuted tool-call blocks from assistant history. + + Providers use different history shapes: Anthropic/OpenAI-chat adapters + normalize calls to ``tool_use``, while the Responses API keeps native + ``function_call`` items. Interactive turns must not persist either shape + without a matching tool result. + """ + if not isinstance(content, list): + return content + return [ + block + for block in content + if not ( + isinstance(block, dict) + and block.get("type") in _TOOL_CALL_BLOCK_TYPES + ) + ] + + def _looks_like_premature_no_tool_stop(text: str) -> bool: """Detect assistant text that promises work but has not actually done it. diff --git a/src/core/config.py b/src/core/config.py index 2ed2f72..c20f6bc 100644 --- a/src/core/config.py +++ b/src/core/config.py @@ -41,6 +41,14 @@ class AgentConfig(ProxyModel): # ── Agent loop (per-spawn, not shared) ─────────────────────────── max_turns: int = 100 max_tool_concurrency: int = 10 + # Interactive frontends need a real user-turn boundary. When enabled, any + # visible assistant text ends the current run; tool calls returned beside + # that text are discarded rather than executed behind a displayed question. + yield_on_text: bool = False + # Autonomous agents benefit from a nudge when they promise work without + # calling a tool. Interactive chat agents must disable this: ordinary + # phrases such as "接下来" or "I will" often introduce a question. + premature_stop_nudges: bool = True # ── Experiment settings ────────────────────────────────────────── experiment_cmd: str | None = None diff --git a/src/core/context.py b/src/core/context.py index 193412b..20570c3 100644 --- a/src/core/context.py +++ b/src/core/context.py @@ -404,6 +404,7 @@ async def _summarize_old_messages( return [ { "role": "user", + "_internal": "context_summary", "content": ( f"[Conversation Summary — compaction #{self._compact_count + 1}]\n\n" f"{summary_text}" diff --git a/src/core/llm/claude.py b/src/core/llm/claude.py index 0ab9f9f..7f98700 100644 --- a/src/core/llm/claude.py +++ b/src/core/llm/claude.py @@ -239,7 +239,12 @@ def _cache_messages(messages: list[dict[str, Any]]) -> list[dict[str, Any]]: """ if not messages: return messages - out = [dict(m) for m in messages] + # Framework-only metadata (for example ``_internal`` control markers) + # is persisted locally but is not part of Anthropic's message schema. + out = [ + {key: value for key, value in m.items() if not key.startswith("_")} + for m in messages + ] last = out[-1] content = last.get("content") if isinstance(content, str): diff --git a/src/core/tools/base.py b/src/core/tools/base.py index 81fa42d..1aa58a3 100644 --- a/src/core/tools/base.py +++ b/src/core/tools/base.py @@ -5,7 +5,10 @@ import os import uuid from abc import ABC, abstractmethod -from typing import Any +from typing import Any, Callable + + +PathAuthorizer = Callable[[str], str | None] class Tool(ABC): @@ -25,10 +28,23 @@ class Tool(ABC): # If a result exceeds persist_threshold, save to disk and return preview. # Set to 0 to always persist, or float('inf') to never persist. persist_threshold: int = 30_000 - - def __init__(self, *, cwd: str, workspace_dir: str | None = None): + # Interactive agents may use a control tool whose completed result must be + # handed to the frontend before any further model call. Autonomous agents + # ignore this unless ``AgentConfig.yield_on_text`` is enabled. + yield_after_execute: bool = False + + def __init__( + self, + *, + cwd: str, + workspace_dir: str | None = None, + path_authorizer: PathAuthorizer | None = None, + persist_results: bool = True, + ): self.cwd = cwd self.workspace_dir = workspace_dir + self.path_authorizer = path_authorizer + self.persist_results = persist_results @abstractmethod async def execute(self, **kwargs: Any) -> str: @@ -52,6 +68,37 @@ async def aclose(self) -> None: """ return None + def should_yield_after_execute(self, output: str) -> bool: + """Whether this completed result should return control to the UI.""" + + return self.yield_after_execute + + def authorize_path(self, path: str) -> tuple[str, str | None]: + """Canonicalize *path* and apply global plus session-level guards. + + Most agents have no session authorizer and retain their existing path + behavior. Interactive frontends can supply one to enforce a dynamic + user-approved scope. Returning the canonical path ensures a symlink + cannot pass the check and then be opened through its lexical alias. + """ + from .path_guard import check_path_allowed + + blocked = check_path_allowed(path) + if blocked: + return path, blocked + if self.path_authorizer is None: + return path, None + + try: + canonical = os.path.realpath(path) + except (OSError, ValueError): + canonical = path + blocked = check_path_allowed(canonical) + if blocked: + return canonical, blocked + blocked = self.path_authorizer(canonical) + return canonical, blocked + def to_api_schema(self) -> dict[str, Any]: """Convert to the format expected by the LLM API (Anthropic tool schema).""" return { @@ -70,6 +117,8 @@ def process_result(self, text: str) -> str: """ if len(text) <= self.persist_threshold: return text + if not self.persist_results: + return self._truncate(text) # Persist to disk persist_root = self.workspace_dir or self.cwd diff --git a/src/core/tools/file_read.py b/src/core/tools/file_read.py index 6ce49a5..7f31ae1 100644 --- a/src/core/tools/file_read.py +++ b/src/core/tools/file_read.py @@ -13,10 +13,9 @@ class FileReadTool(Tool): name = "Read" description = ( - "Reads a file from the local filesystem. You can access any file " - "directly by using this tool.\n" - "Assume this tool is able to read all files on the machine. If the " - "user provides a path to a file assume that path is valid.\n" + "Reads a file from the local filesystem, subject to the active path " + "scope and safety policy. If access is blocked, ask the user to approve " + "the exact path instead of searching elsewhere.\n" "\n" "Usage:\n" "- The file_path parameter must be an absolute path, not a relative path\n" @@ -29,8 +28,8 @@ class FileReadTool(Tool): "at 1\n" "- This tool can read PDF files (.pdf). For large PDFs, provide the " "pages parameter to read specific page ranges (e.g., pages: \"1-5\")\n" - "- This tool can only read files, not directories. To read a directory, " - "use an ls command via the Bash tool.\n" + "- This tool can only read files, not directories. To find files in a " + "directory, use the Glob tool.\n" "- If you read a file that exists but has empty contents you will " "receive a warning." ) @@ -64,8 +63,7 @@ async def execute(self, **kwargs: Any) -> str: if not os.path.isabs(file_path): file_path = os.path.join(self.cwd, file_path) - from .path_guard import check_path_allowed - blocked = check_path_allowed(file_path) + file_path, blocked = self.authorize_path(file_path) if blocked: return f"BLOCKED: {blocked}" diff --git a/src/core/tools/glob_tool.py b/src/core/tools/glob_tool.py index e9c20ee..d93409f 100644 --- a/src/core/tools/glob_tool.py +++ b/src/core/tools/glob_tool.py @@ -44,17 +44,35 @@ async def execute(self, **kwargs: Any) -> str: if not os.path.isabs(path): path = os.path.join(self.cwd, path) - from .path_guard import check_path_allowed - blocked = check_path_allowed(path) + path, blocked = self.authorize_path(path) if blocked: return f"BLOCKED: {blocked}" + pattern_path = pathlib.PurePath(pattern) + if pattern_path.is_absolute() or ".." in pattern_path.parts: + return ( + "BLOCKED: glob patterns must stay below the authorized search " + "root; use the path argument for an explicitly approved root" + ) + base = pathlib.Path(path) if not base.exists(): return f"Error: Directory not found: {path}" if not base.is_dir(): return f"Error: {path} is not a directory." + # Authorize the non-wildcard prefix before globbing. This prevents a + # symlink such as ``approved/link -> /outside`` from being traversed. + prefix_parts: list[str] = [] + for part in pattern_path.parts: + if any(char in part for char in "*?["): + break + prefix_parts.append(part) + if prefix_parts: + _prefix, blocked = self.authorize_path(str(base.joinpath(*prefix_parts))) + if blocked: + return f"BLOCKED: {blocked}" + try: matches = list(base.glob(pattern)) except Exception as e: @@ -67,6 +85,9 @@ async def execute(self, **kwargs: Any) -> str: parts = m.relative_to(base).parts if any(p in skip_dirs for p in parts): continue + _canonical, blocked = self.authorize_path(str(m)) + if blocked: + continue if m.is_file(): filtered.append(m) diff --git a/src/core/tools/grep.py b/src/core/tools/grep.py index d3f4a54..0a5ebef 100644 --- a/src/core/tools/grep.py +++ b/src/core/tools/grep.py @@ -114,8 +114,7 @@ async def execute(self, **kwargs: Any) -> str: if not os.path.isabs(path): path = os.path.join(self.cwd, path) - from .path_guard import check_path_allowed - blocked = check_path_allowed(path) + path, blocked = self.authorize_path(path) if blocked: return f"BLOCKED: {blocked}" diff --git a/src/recall.py b/src/recall.py index f8c37ca..03b9eaa 100644 --- a/src/recall.py +++ b/src/recall.py @@ -19,6 +19,27 @@ "maximize", "minimize", "improve", "optimize", "score", "test", "dev", "task"} +def _safe_experience_files(cwd: str) -> list[Path]: + """Return non-symlink experience files contained by this project's root.""" + + sessions = Path(cwd).resolve() / ".arbor" / "sessions" + if not sessions.is_dir() or sessions.is_symlink(): + return [] + out: list[Path] = [] + for session in sessions.iterdir(): + if not session.is_dir() or session.is_symlink(): + continue + exp = session / "EXPERIENCE.md" + if not exp.is_file() or exp.is_symlink(): + continue + try: + exp.resolve(strict=True).relative_to(sessions) + except (OSError, ValueError): + continue + out.append(exp) + return out + + def _tokens(text: str) -> set[str]: return {w for w in re.findall(r"[a-z0-9]+", (text or "").lower()) if len(w) > 2 and w not in _STOP} @@ -31,12 +52,9 @@ def _score(topic: set[str], experience: set[str]) -> float: def find_similar(cwd: str, topic: str, *, limit: int = 3, threshold: float = 0.25) -> list[dict[str, Any]]: """Return prior sessions ranked by topic overlap: [{name, path, score, text}].""" - sessions = Path(cwd) / ".arbor" / "sessions" - if not sessions.is_dir(): - return [] tt = _tokens(topic) hits: list[dict[str, Any]] = [] - for exp in sessions.glob("*/EXPERIENCE.md"): + for exp in _safe_experience_files(cwd): text = exp.read_text(encoding="utf-8") s = _score(tt, _tokens(text)) if s >= threshold: @@ -47,11 +65,8 @@ def find_similar(cwd: str, topic: str, *, limit: int = 3, threshold: float = 0.2 def list_experiences(cwd: str, limit: int = 8) -> list[tuple[str, str]]: """[(session_name, first-line summary)] of prior runs that left experience.""" - sessions = Path(cwd) / ".arbor" / "sessions" out: list[tuple[str, str]] = [] - if not sessions.is_dir(): - return out - for exp in sorted(sessions.glob("*/EXPERIENCE.md"), reverse=True)[:limit]: + for exp in sorted(_safe_experience_files(cwd), reverse=True)[:limit]: desc = "" for ln in exp.read_text(encoding="utf-8").splitlines(): if ln.startswith("description:"): @@ -74,11 +89,11 @@ def compose_from_sessions(cwd: str, names: list[str]) -> str: project, so it picks which prior runs actually transfer. Falls back to nothing if the named sessions lack experience. """ - sessions = Path(cwd) / ".arbor" / "sessions" + available = {exp.parent.name: exp for exp in _safe_experience_files(cwd)} hits = [] for name in names or []: - exp = sessions / name / "EXPERIENCE.md" - if exp.exists(): + exp = available.get(str(name)) + if exp is not None: hits.append({"name": name, "score": "selected", "text": exp.read_text(encoding="utf-8")}) return _compose(hits) diff --git a/tests/test_claude_thinking.py b/tests/test_claude_thinking.py index 03fd9a2..65e6b60 100644 --- a/tests/test_claude_thinking.py +++ b/tests/test_claude_thinking.py @@ -54,3 +54,17 @@ def test_legacy_models_keep_budget_tokens(model): out = _thinking(model, reasoning_effort="low") assert out["thinking"] == {"type": "enabled", "budget_tokens": 1024} assert out["output_config"] is None + + +def test_internal_message_metadata_is_not_sent_to_anthropic(): + provider = _provider("claude-sonnet-4-6") + messages = provider._cache_messages([ + { + "role": "user", + "content": "continue", + "_internal": "premature_stop_nudge", + } + ]) + + assert "_internal" not in messages[0] + assert messages[0]["role"] == "user" diff --git a/tests/test_intake_conversation_store.py b/tests/test_intake_conversation_store.py index 6332cf1..aa5ab0f 100644 --- a/tests/test_intake_conversation_store.py +++ b/tests/test_intake_conversation_store.py @@ -49,6 +49,51 @@ def test_save_then_load_round_trips_messages(tmp_path): assert load_messages(rec) == messages +def test_save_redacts_tool_result_payloads(tmp_path): + rec = new_conversation(tmp_path) + messages = [ + {"role": "user", "content": "read the file"}, + { + "role": "assistant", + "content": [{"type": "tool_use", "id": "t1", "name": "Read", "input": {}}], + }, + { + "role": "user", + "content": [ + {"type": "tool_result", "tool_use_id": "t1", "content": "sensitive file body"} + ], + }, + ] + + save_conversation(rec, messages) + + persisted = load_messages(rec) + assert "sensitive file body" not in rec.messages_path.read_text(encoding="utf-8") + assert persisted[-1]["content"][0]["content"] == ( + "[tool result omitted from persisted intake history; " + "ask the user to re-authorize the path before re-reading]" + ) + + +def test_save_redacts_context_summaries_that_may_contain_tool_data(tmp_path): + rec = new_conversation(tmp_path) + save_conversation( + rec, + [ + {"role": "user", "content": "read"}, + { + "role": "user", + "_internal": "context_summary", + "content": "summary accidentally contains sensitive file body", + }, + ], + ) + + raw = rec.messages_path.read_text(encoding="utf-8") + assert "sensitive file body" not in raw + assert load_messages(rec)[-1]["_internal"] == "context_summary" + + def test_meta_records_title_turns_and_launched(tmp_path): rec = new_conversation(tmp_path) save_conversation(rec, _msgs("optimize the dev score please"), launched=False) @@ -126,3 +171,32 @@ def test_new_conversation_ids_are_unique(tmp_path): save_conversation(rec, _msgs("x")) ids.add(rec.conv_id) assert len(ids) == 5 + + +def test_find_conversations_rejects_metadata_id_mismatch(tmp_path): + rec = new_conversation(tmp_path) + save_conversation(rec, _msgs("safe")) + meta = json.loads(rec.meta_path.read_text(encoding="utf-8")) + meta["conv_id"] = "../../outside" + rec.meta_path.write_text(json.dumps(meta), encoding="utf-8") + + assert find_conversations(tmp_path) == [] + + +def test_find_conversations_rejects_symlinked_directory(tmp_path): + outside = tmp_path / "outside" + outside.mkdir() + (outside / "meta.json").write_text( + json.dumps({"conv_id": "conv_20260711_000000"}), + encoding="utf-8", + ) + root = conversations_root(tmp_path) + root.mkdir(parents=True) + link = root / "conv_20260711_000000" + try: + link.symlink_to(outside, target_is_directory=True) + except OSError as exc: # pragma: no cover - platform permission edge + import pytest + pytest.skip(f"symlink unavailable: {exc}") + + assert find_conversations(tmp_path) == [] diff --git a/tests/test_intake_safety.py b/tests/test_intake_safety.py new file mode 100644 index 0000000..1115589 --- /dev/null +++ b/tests/test_intake_safety.py @@ -0,0 +1,776 @@ +"""Regression tests for intake turn boundaries, intent routing, and path scope.""" + +from __future__ import annotations + +import asyncio +import copy +from pathlib import Path +from typing import Any + +import pytest + +from arbor.cli.intake import repl +from arbor.cli.intake.scope import ( + IntakeMode, + IntakePathPolicy, + extract_explicit_paths, + infer_intake_mode, + is_explicit_launch_approval, +) +from arbor.cli.intake.conversation_store import new_conversation, save_conversation +from arbor.core.agent import Agent +from arbor.core.config import AgentConfig +from arbor.core.llm.base import LLMResponse, TextBlock, ToolUseBlock, Usage +from arbor.core.tools.base import Tool +from arbor.core.tools.file_read import FileReadTool +from arbor.core.tools.glob_tool import GlobTool +from arbor.core.tools.grep import GrepTool + + +class _ScriptedProvider: + model = "scripted-model" + base_url = None + + def __init__(self, responses: list[LLMResponse]): + self.responses = list(responses) + self.calls: list[dict[str, Any]] = [] + + async def create(self, **kwargs: Any) -> LLMResponse: + self.calls.append(copy.deepcopy(kwargs)) + if not self.responses: + raise AssertionError("unexpected extra LLM call") + return self.responses.pop(0) + + def count_tokens(self, text: str) -> int: + return max(1, len(text) // 4) + + +class _RecordingTool(Tool): + name = "Probe" + description = "Record a synthetic read-only probe." + input_schema = { + "type": "object", + "properties": {"path": {"type": "string"}}, + "required": ["path"], + } + is_read_only = True + + def __init__(self, *, cwd: str): + super().__init__(cwd=cwd) + self.calls: list[dict[str, Any]] = [] + + async def execute(self, **kwargs: Any) -> str: + self.calls.append(kwargs) + return "probe result" + + +def _text_response(text: str) -> LLMResponse: + return LLMResponse( + content=[TextBlock(text=text)], + stop_reason="end_turn", + usage=Usage(), + raw_content=[{"type": "text", "text": text}], + ) + + +def _tool_response( + *, + text: str | None = None, + name: str = "Probe", + tool_input: dict[str, Any] | None = None, + tool_id: str = "tool-1", + native_responses_history: bool = False, +) -> LLMResponse: + tool_input = tool_input or {"path": "target"} + content = [] + raw_content: list[dict[str, Any]] = [] + if text is not None: + content.append(TextBlock(text=text)) + if native_responses_history: + raw_content.append({ + "type": "message", + "role": "assistant", + "content": [{"type": "output_text", "text": text}], + }) + else: + raw_content.append({"type": "text", "text": text}) + content.append(ToolUseBlock(id=tool_id, name=name, input=tool_input)) + if native_responses_history: + raw_content.append({ + "type": "function_call", + "call_id": tool_id, + "name": name, + "arguments": "{}", + }) + else: + raw_content.append({ + "type": "tool_use", + "id": tool_id, + "name": name, + "input": tool_input, + }) + return LLMResponse( + content=content, + stop_reason="tool_use", + usage=Usage(), + raw_content=raw_content, + ) + + +def _run_agent( + tmp_path: Path, + responses: list[LLMResponse], + *, + yield_on_text: bool, + premature_stop_nudges: bool, +) -> tuple[Agent, _ScriptedProvider, _RecordingTool, str]: + provider = _ScriptedProvider(responses) + tool = _RecordingTool(cwd=str(tmp_path)) + agent = Agent( + provider=provider, + tools=[tool], + system_prompt="test", + config=AgentConfig( + cwd=str(tmp_path), + max_turns=8, + auto_git=False, + llm_retry_attempts=1, + yield_on_text=yield_on_text, + premature_stop_nudges=premature_stop_nudges, + ), + ) + result = asyncio.run(agent.run("只阅读 topic1.md 和 topic2.md，不要查看其他项目。")) + return agent, provider, tool, result + + +def test_interactive_chinese_question_yields_without_hidden_nudge(tmp_path): + text = "两个文件已经读完。接下来需要确认：用公开数据还是内部数据？" + agent, provider, tool, result = _run_agent( + tmp_path, + [_text_response(text)], + yield_on_text=True, + premature_stop_nudges=False, + ) + + assert result == text + assert len(provider.calls) == 1 + assert tool.calls == [] + assert agent.stop_reason == "awaiting_user" + assert not any(message.get("_internal") for message in agent.messages) + + +@pytest.mark.parametrize("native_responses_history", [False, True]) +def test_interactive_mixed_text_and_tool_is_suppressed_and_history_is_valid( + tmp_path, native_responses_history +): + text = "我发现父目录还有一个相关项目，先进去看看。" + agent, provider, tool, result = _run_agent( + tmp_path, + [ + _tool_response( + text=text, + tool_input={"path": "../sibling"}, + native_responses_history=native_responses_history, + ) + ], + yield_on_text=True, + premature_stop_nudges=False, + ) + + assert result == text + assert len(provider.calls) == 1 + assert tool.calls == [] + assert agent.tool_uses == [] + assert agent.suppressed_tool_uses == [ + {"name": "Probe", "input": {"path": "../sibling"}} + ] + assistant_content = agent.messages[-1]["content"] + assert all( + block.get("type") not in {"tool_use", "function_call"} + for block in assistant_content + ) + + +def test_interactive_tool_only_turn_executes_then_visible_text_yields(tmp_path): + agent, provider, tool, result = _run_agent( + tmp_path, + [_tool_response(), _text_response("已根据工具结果完成分析。")], + yield_on_text=True, + premature_stop_nudges=False, + ) + + assert result == "已根据工具结果完成分析。" + assert len(provider.calls) == 2 + assert tool.calls == [{"path": "target"}] + assert agent.tool_uses == [{"name": "Probe", "input": {"path": "target"}}] + assert any( + isinstance(message.get("content"), list) + and any(block.get("type") == "tool_result" for block in message["content"]) + for message in agent.messages + if message.get("role") == "user" + ) + + +def test_interactive_control_tool_yields_without_another_model_call(tmp_path): + class _ControlTool(_RecordingTool): + yield_after_execute = True + + provider = _ScriptedProvider([_tool_response()]) + tool = _ControlTool(cwd=str(tmp_path)) + agent = Agent( + provider=provider, + tools=[tool], + system_prompt="test", + config=AgentConfig( + cwd=str(tmp_path), + max_turns=8, + auto_git=False, + llm_retry_attempts=1, + yield_on_text=True, + premature_stop_nudges=False, + ), + ) + + assert asyncio.run(agent.run("stage")) == "" + assert len(provider.calls) == 1 + assert tool.calls == [{"path": "target"}] + assert agent.stop_reason == "awaiting_user" + + +def test_interactive_control_tool_failure_returns_to_model(tmp_path): + class _FailingControlTool(_RecordingTool): + yield_after_execute = True + + async def execute(self, **kwargs: Any) -> str: + self.calls.append(kwargs) + return "BLOCKED: invalid target" + + def should_yield_after_execute(self, output: str) -> bool: + return output.startswith("STAGED") + + provider = _ScriptedProvider([ + _tool_response(), + _text_response("Please provide the correct target path."), + ]) + tool = _FailingControlTool(cwd=str(tmp_path)) + agent = Agent( + provider=provider, + tools=[tool], + system_prompt="test", + config=AgentConfig( + cwd=str(tmp_path), max_turns=4, auto_git=False, + llm_retry_attempts=1, yield_on_text=True, + premature_stop_nudges=False, + ), + ) + + assert asyncio.run(agent.run("stage")) == "Please provide the correct target path." + assert len(provider.calls) == 2 + + +def test_interactive_control_tool_batch_is_rejected_atomically(tmp_path): + class _ControlTool(_RecordingTool): + name = "Control" + yield_after_execute = True + + provider = _ScriptedProvider([ + LLMResponse( + content=[ + ToolUseBlock(id="control", name="Control", input={"path": "a"}), + ToolUseBlock(id="probe", name="Probe", input={"path": "b"}), + ], + stop_reason="tool_use", + usage=Usage(), + raw_content=[ + {"type": "tool_use", "id": "control", "name": "Control", "input": {"path": "a"}}, + {"type": "tool_use", "id": "probe", "name": "Probe", "input": {"path": "b"}}, + ], + ), + _text_response("I will retry with the control tool alone."), + ]) + control = _ControlTool(cwd=str(tmp_path)) + probe = _RecordingTool(cwd=str(tmp_path)) + agent = Agent( + provider=provider, + tools=[control, probe], + system_prompt="test", + config=AgentConfig( + cwd=str(tmp_path), max_turns=4, auto_git=False, + llm_retry_attempts=1, yield_on_text=True, + premature_stop_nudges=False, + ), + ) + + assert asyncio.run(agent.run("stage")) == "I will retry with the control tool alone." + assert control.calls == [] + assert probe.calls == [] + assert agent.tool_uses == [] + assert {entry["name"] for entry in agent.suppressed_tool_uses} == {"Control", "Probe"} + + +def test_default_autonomous_agent_keeps_mixed_text_tool_behavior(tmp_path): + agent, provider, tool, result = _run_agent( + tmp_path, + [ + _tool_response(text="Inspecting the target."), + _text_response("Analysis complete."), + ], + yield_on_text=False, + premature_stop_nudges=True, + ) + + assert result == "Analysis complete." + assert len(provider.calls) == 2 + assert tool.calls == [{"path": "target"}] + assert agent.suppressed_tool_uses == [] + + +def test_default_autonomous_agent_keeps_premature_stop_nudge(tmp_path): + agent, provider, tool, result = _run_agent( + tmp_path, + [_text_response("接下来我会检查目标。"), _text_response("检查完成。")], + yield_on_text=False, + premature_stop_nudges=True, + ) + + assert result == "检查完成。" + assert len(provider.calls) == 2 + assert tool.calls == [] + assert any( + message.get("_internal") == "premature_stop_nudge" + for message in agent.messages + ) + + +def test_autonomous_agent_config_defaults_are_unchanged(tmp_path): + provider = _ScriptedProvider([ + _tool_response(text="Inspecting the target."), + _text_response("Analysis complete."), + ]) + tool = _RecordingTool(cwd=str(tmp_path)) + agent = Agent( + provider=provider, + tools=[tool], + system_prompt="test", + config=AgentConfig( + cwd=str(tmp_path), + max_turns=4, + auto_git=False, + llm_retry_attempts=1, + ), + ) + + assert asyncio.run(agent.run("inspect")) == "Analysis complete." + assert tool.calls == [{"path": "target"}] + assert len(provider.calls) == 2 + + +@pytest.mark.parametrize( + ("message", "current", "expected"), + [ + ("请阅读两个 topic 并讨论 novelty", None, IntakeMode.DISCUSSION), + ("只讨论方案，不要运行实验", IntakeMode.PLANNING, IntakeMode.DISCUSSION), + ("improve validation score above baseline", None, IntakeMode.PLANNING), + ("现在开始实验吧", IntakeMode.DISCUSSION, IntakeMode.PLANNING), + ("go ahead", IntakeMode.DISCUSSION, IntakeMode.PLANNING), + ("继续", IntakeMode.DISCUSSION, IntakeMode.DISCUSSION), + ], +) +def test_intake_mode_routing(message, current, expected): + assert infer_intake_mode(message, current) == expected + + +@pytest.mark.parametrize( + "message", + ["go", "yes, please", "好的，开始吧", "确认启动", "没问题，执行吧"], +) +def test_explicit_launch_approval_accepts_only_confirmation_messages(message): + assert is_explicit_launch_approval(message) + + +@pytest.mark.parametrize( + "message", + ["yes, but use test", "可以先解释一下", "不要启动", "start with another repo"], +) +def test_explicit_launch_approval_rejects_plan_edits_and_questions(message): + assert not is_explicit_launch_approval(message) + + +def test_path_extraction_handles_mixed_windows_separator_without_tab(): + message = "请阅读 work/Ali/topics\\topic2.md 和 `work/Ali/topics/topic1.md`。" + paths = extract_explicit_paths(message) + assert "work/Ali/topics\\topic2.md" in paths + assert "work/Ali/topics/topic1.md" in paths + assert all("\t" not in path for path in paths) + + +def _topic_tree(tmp_path: Path) -> tuple[Path, Path, Path, Path]: + starting = tmp_path / "Arbor" + topics = tmp_path / "work" / "Ali" / "topics" + starting.mkdir() + topics.mkdir(parents=True) + topic1 = topics / "topic1.md" + topic2 = topics / "topic2.md" + other = topics / "other.md" + topic1.write_text("topic one", encoding="utf-8") + topic2.write_text("topic two", encoding="utf-8") + other.write_text("not approved", encoding="utf-8") + return starting, topic1, topic2, other + + +def test_discussion_scope_allows_only_explicit_files(tmp_path): + starting, topic1, topic2, other = _topic_tree(tmp_path) + policy = IntakePathPolicy(starting) + policy.update( + "请阅读 work/Ali/topics\\topic1.md 和 work/Ali/topics/topic2.md", + IntakeMode.DISCUSSION, + ) + + assert policy.authorize(str(topic1.resolve())) is None + assert policy.authorize(str(topic2.resolve())) is None + assert policy.authorize(str(other.resolve())) is not None + assert policy.authorize(str(topic1.parent.resolve())) is not None + + read = FileReadTool(cwd=str(starting), path_authorizer=policy.authorize) + assert "topic one" in asyncio.run(read.execute(file_path=str(topic1))) + assert asyncio.run(read.execute(file_path=str(other))).startswith("BLOCKED:") + + +def test_discussion_correction_replaces_old_scope(tmp_path): + starting, topic1, topic2, _other = _topic_tree(tmp_path) + policy = IntakePathPolicy(starting) + policy.update(str(topic1), IntakeMode.DISCUSSION) + assert policy.authorize(str(topic1.resolve())) is None + + policy.update(f"不要看之前的文件，只看 {topic2}", IntakeMode.DISCUSSION) + assert policy.authorize(str(topic1.resolve())) is not None + assert policy.authorize(str(topic2.resolve())) is None + + +def test_unquoted_conceptual_slash_does_not_grant_scope(tmp_path): + starting = tmp_path / "project" + conceptual = starting / "client" / "server" + conceptual.mkdir(parents=True) + policy = IntakePathPolicy(starting) + + policy.update("讨论 client/server 架构", IntakeMode.DISCUSSION) + + assert policy.explicit_paths == () + assert policy.authorize(str(conceptual.resolve())) is not None + + +def test_planning_scope_is_confined_to_project_and_explicit_redirect(tmp_path): + starting = tmp_path / "project" + sibling = tmp_path / "sibling" + starting.mkdir() + sibling.mkdir() + inside = starting / "inside.txt" + outside = sibling / "outside.txt" + inside.write_text("inside", encoding="utf-8") + outside.write_text("outside", encoding="utf-8") + + policy = IntakePathPolicy(starting) + assert policy.authorize(str(inside.resolve())) is None + assert policy.authorize(str(outside.resolve())) is not None + + policy.update(str(sibling), IntakeMode.PLANNING) + assert policy.authorize(str(outside.resolve())) is None + assert policy.authorize(str(inside.resolve())) is not None + + +def test_planning_external_file_does_not_authorize_its_directory(tmp_path): + starting = tmp_path / "project" + external = tmp_path / "external" + starting.mkdir() + external.mkdir() + named = external / "topic.md" + sibling = external / "secret.md" + named.write_text("topic", encoding="utf-8") + sibling.write_text("secret", encoding="utf-8") + + policy = IntakePathPolicy(starting) + policy.update(str(named), IntakeMode.PLANNING) + + assert policy.authorize(str(named.resolve())) is None + assert policy.authorize(str(sibling.resolve())) is not None + assert policy.authorize(str(external.resolve())) is not None + + +def test_unresolved_explicit_planning_path_does_not_fall_back_to_cwd(tmp_path): + starting = tmp_path / "project" + starting.mkdir() + inside = starting / "inside.txt" + inside.write_text("inside", encoding="utf-8") + + policy = IntakePathPolicy(starting) + policy.update("请在 missing/project.md 中提高 score", IntakeMode.PLANNING) + + assert policy.unresolved_paths == ("missing/project.md",) + assert policy.authorize(str(inside.resolve())) is not None + + +def test_glob_blocks_parent_pattern_and_symlink_escape(tmp_path): + starting = tmp_path / "project" + approved = starting / "approved" + outside = tmp_path / "outside" + approved.mkdir(parents=True) + outside.mkdir() + (approved / "inside.txt").write_text("inside", encoding="utf-8") + secret = outside / "secret.txt" + secret.write_text("secret", encoding="utf-8") + + policy = IntakePathPolicy(starting) + policy.update(str(approved), IntakeMode.DISCUSSION) + glob = GlobTool(cwd=str(starting), path_authorizer=policy.authorize) + + parent_result = asyncio.run(glob.execute(pattern="../outside/*", path=str(approved))) + assert parent_result.startswith("BLOCKED:") + + link = approved / "escape" + try: + link.symlink_to(outside, target_is_directory=True) + except OSError as exc: # pragma: no cover - platform permission edge + pytest.skip(f"symlink unavailable: {exc}") + + link_result = asyncio.run(glob.execute(pattern="escape/*", path=str(approved))) + assert link_result.startswith("BLOCKED:") + assert "secret.txt" not in link_result + + read = FileReadTool(cwd=str(starting), path_authorizer=policy.authorize) + assert asyncio.run(read.execute(file_path=str(link / "secret.txt"))).startswith( + "BLOCKED:" + ) + grep = GrepTool(cwd=str(starting), path_authorizer=policy.authorize) + assert asyncio.run( + grep.execute(pattern="secret", path=str(link / "secret.txt")) + ).startswith("BLOCKED:") + + +class _NoopDisplay: + def __init__(self, *args: Any, **kwargs: Any): + pass + + def __enter__(self): + return self + + def __exit__(self, *args: Any): + return False + + +def _scripted_reader(inputs: list[str]): + queue = list(inputs) + + async def _reader(_session): + if not queue: + raise EOFError + return queue.pop(0) + + return _reader + + +def _run_intake( + monkeypatch, + cwd: Path, + inputs: list[str], + responses: list[LLMResponse], +): + provider = _ScriptedProvider(responses) + monkeypatch.setattr(repl, "_read_user_line", _scripted_reader(inputs)) + monkeypatch.setattr(repl, "_build_session", lambda *args, **kwargs: None) + monkeypatch.setattr(repl, "_print_welcome", lambda *args, **kwargs: None) + monkeypatch.setattr(repl, "IntakeDisplay", _NoopDisplay) + outcome = asyncio.run( + repl.run_intake(provider=provider, starting_cwd=cwd, seed_message=None) + ) + return outcome, provider + + +def _tool_names(call: dict[str, Any]) -> set[str]: + return {tool["name"] for tool in call["tools"]} + + +def test_discussion_mode_exposes_read_only_scoped_tools(monkeypatch, tmp_path): + topic = tmp_path / "topic.md" + topic.write_text("research topic", encoding="utf-8") + outcome, provider = _run_intake( + monkeypatch, + tmp_path, + ["请阅读 `topic.md` 并讨论这个研究方向", "/quit"], + [_text_response("这是一个干净的研究问题。")], + ) + + assert outcome is None + assert len(provider.calls) == 1 + assert _tool_names(provider.calls[0]) == {"Read", "Glob", "Grep"} + assert "research discussion assistant" in provider.calls[0]["system"] + assert str(topic.resolve()) in provider.calls[0]["system"] + + +def test_intake_can_switch_from_discussion_to_planning(monkeypatch, tmp_path): + topic = tmp_path / "topic.md" + topic.write_text("research topic", encoding="utf-8") + outcome, provider = _run_intake( + monkeypatch, + tmp_path, + [ + "请阅读 `topic.md` 并讨论这个研究方向", + "现在开始实验吧", + "/quit", + ], + [ + _text_response("这个方向值得进一步验证。"), + _text_response("我已整理好启动计划，是否开始？"), + ], + ) + + assert outcome is None + assert len(provider.calls) == 2 + assert _tool_names(provider.calls[0]) == {"Read", "Glob", "Grep"} + assert _tool_names(provider.calls[1]) == { + "Read", + "Glob", + "Grep", + "LaunchExperiment", + } + assert "benchmark-grinding agent" in provider.calls[1]["system"] + assert "Bash" not in _tool_names(provider.calls[1]) + + +def test_intake_can_switch_from_planning_back_to_discussion(monkeypatch, tmp_path): + outcome, provider = _run_intake( + monkeypatch, + tmp_path, + [ + "improve validation score above baseline", + "先不要启动，只讨论这个研究方向", + "/quit", + ], + [ + _text_response("I need one metric clarification."), + _text_response("这个方向的核心假设是清楚的。"), + ], + ) + + assert outcome is None + assert "LaunchExperiment" in _tool_names(provider.calls[0]) + assert _tool_names(provider.calls[1]) == {"Read", "Glob", "Grep"} + assert "research discussion assistant" in provider.calls[1]["system"] + + +def test_planning_launch_flow_still_returns_plan(monkeypatch, tmp_path): + instruction = ( + "Maximize score from python eval.py on dev; baseline unknown; push as " + "high as possible; novelty-leaning; do not modify eval.py or test data." + ) + outcome, provider = _run_intake( + monkeypatch, + tmp_path, + ["improve validation score above baseline", "go"], + [ + _tool_response( + name="LaunchExperiment", + tool_input={"cwd": str(tmp_path), "instruction": instruction}, + ), + ], + ) + + assert outcome is not None + assert outcome.cwd == str(tmp_path.resolve()) + assert outcome.instruction == instruction + # The confirmation is handled by controller code; the model cannot rewrite + # the staged cwd/instruction after the user says go. + assert len(provider.calls) == 1 + assert all("Bash" not in _tool_names(call) for call in provider.calls) + conversations = list((tmp_path / ".arbor" / "conversations").glob("*/messages.jsonl")) + assert len(conversations) == 1 + assert '"content": "go"' in conversations[0].read_text(encoding="utf-8") + + +def test_mixed_launch_call_cannot_launch_behind_visible_text(monkeypatch, tmp_path): + outcome, provider = _run_intake( + monkeypatch, + tmp_path, + ["improve validation score", "/quit"], + [ + _tool_response( + text="Should I launch this plan?", + name="LaunchExperiment", + tool_input={"cwd": str(tmp_path), "instruction": "improve score"}, + ) + ], + ) + + assert outcome is None + assert len(provider.calls) == 1 + + +@pytest.mark.parametrize("marker", ["接下来", "下一步", "我会", "应该"]) +def test_repl_chinese_future_markers_do_not_auto_continue( + monkeypatch, tmp_path, marker +): + outcome, provider = _run_intake( + monkeypatch, + tmp_path, + ["只讨论这个研究问题", "/quit"], + [_text_response(f"{marker}先确认：你更关心理论还是实验？")], + ) + + assert outcome is None + assert len(provider.calls) == 1 + assert not any( + message.get("_internal") + for message in provider.calls[0]["messages"] + ) + + +def test_continue_restores_context_but_not_filesystem_authority( + monkeypatch, tmp_path +): + project = tmp_path / "project" + project.mkdir() + topic = tmp_path / "topic.md" + topic.write_text("private topic", encoding="utf-8") + rec = new_conversation(project) + save_conversation( + rec, + [ + {"role": "user", "content": f"请阅读 `{topic}` 并讨论"}, + {"role": "assistant", "content": [{"type": "text", "text": "旧回答"}]}, + ], + ) + provider = _ScriptedProvider([_text_response("请重新点名需要读取的路径。")]) + monkeypatch.setattr(repl, "_read_user_line", _scripted_reader(["继续", "/quit"])) + monkeypatch.setattr(repl, "_build_session", lambda *args, **kwargs: None) + monkeypatch.setattr(repl, "_print_welcome", lambda *args, **kwargs: None) + monkeypatch.setattr(repl, "IntakeDisplay", _NoopDisplay) + + outcome = asyncio.run( + repl.run_intake( + provider=provider, + starting_cwd=project, + seed_message=None, + continue_latest=True, + ) + ) + + assert outcome is None + assert "no filesystem paths are currently approved" in provider.calls[0]["system"] + assert str(topic.resolve()) not in provider.calls[0]["system"] + + +def test_reset_clears_scope_and_refreshes_prompt(monkeypatch, tmp_path): + topic = tmp_path / "topic.md" + topic.write_text("topic", encoding="utf-8") + outcome, provider = _run_intake( + monkeypatch, + tmp_path, + [f"请阅读 `{topic}` 并讨论", "/reset", "继续讨论", "/quit"], + [_text_response("第一轮。"), _text_response("请重新提供文件路径。")], + ) + + assert outcome is None + assert str(topic.resolve()) in provider.calls[0]["system"] + assert "no filesystem paths are currently approved" in provider.calls[1]["system"] + assert [m["content"] for m in provider.calls[1]["messages"] if m["role"] == "user"] == [ + "继续讨论" + ] diff --git a/tests/test_recall_safety.py b/tests/test_recall_safety.py new file mode 100644 index 0000000..03bd27c --- /dev/null +++ b/tests/test_recall_safety.py @@ -0,0 +1,37 @@ +"""Security regression tests for prior-experience discovery and composition.""" + +from __future__ import annotations + +import pytest + +from arbor.recall import compose_from_sessions, list_experiences + + +def test_compose_from_sessions_rejects_path_traversal(tmp_path): + outside = tmp_path / "outside" + outside.mkdir() + (outside / "EXPERIENCE.md").write_text("- secret", encoding="utf-8") + project = tmp_path / "project" + (project / ".arbor" / "sessions").mkdir(parents=True) + + assert compose_from_sessions(str(project), ["../../../outside"]) == "" + + +def test_experience_discovery_rejects_symlinked_session(tmp_path): + project = tmp_path / "project" + sessions = project / ".arbor" / "sessions" + sessions.mkdir(parents=True) + outside = tmp_path / "outside" + outside.mkdir() + (outside / "EXPERIENCE.md").write_text( + "description: leaked\n- secret", + encoding="utf-8", + ) + link = sessions / "evil" + try: + link.symlink_to(outside, target_is_directory=True) + except OSError as exc: # pragma: no cover - platform permission edge + pytest.skip(f"symlink unavailable: {exc}") + + assert list_experiences(str(project)) == [] + assert compose_from_sessions(str(project), ["evil"]) == "" \ No newline at end of file diff --git a/tests/test_run_config_resolution.py b/tests/test_run_config_resolution.py new file mode 100644 index 0000000..2aed2f1 --- /dev/null +++ b/tests/test_run_config_resolution.py @@ -0,0 +1,42 @@ +"""CLI configuration resolution regressions.""" + +from arbor.cli.commands.run import _resolve_effective_options + + +def test_effective_options_support_documented_nested_llm_block(): + result = _resolve_effective_options( + project_defaults={ + "llm": { + "provider": "openai-chat", + "model": "mock-model", + "base_url": "http://127.0.0.1:8765/v1", + "api_key": "dummy", + } + }, + user_llm={"provider": "anthropic", "model": "claude"}, + user_cli={}, + max_cycles=None, + max_turns=None, + ) + + assert result["provider"] == "openai-chat" + assert result["model"] == "mock-model" + assert result["base_url"] == "http://127.0.0.1:8765/v1" + assert result["api_key"] == "dummy" + + +def test_legacy_flat_llm_keys_override_nested_values(): + result = _resolve_effective_options( + project_defaults={ + "provider": "openai-chat", + "model": "flat-model", + "llm": {"provider": "anthropic", "model": "nested-model"}, + }, + user_llm={}, + user_cli={}, + max_cycles=None, + max_turns=None, + ) + + assert result["provider"] == "openai-chat" + assert result["model"] == "flat-model" \ No newline at end of file