diff --git a/.changeset/local-llm-codes-in-process.md b/.changeset/local-llm-codes-in-process.md new file mode 100644 index 00000000..c89fe3e1 --- /dev/null +++ b/.changeset/local-llm-codes-in-process.md @@ -0,0 +1,43 @@ +--- +"@openspec-ui/core": minor +"@openspec-ui/webui": minor +"@openspec-ui/server": minor +"@openspec-ui/cli": minor +"openspec-ui-vscode": minor +--- + +The local LLM agent is built in. `local-llm-acp` no longer starts an +external `coding-agent` that you had to find and install: it is a coding +agent inside the product, against your OpenAI-compatible server, with +tools to read, write, replace in a file, list, search and run a command, +all confined to the change's working directory. Each tool call and its +result shows in the run. It reads a tool call the model wrote as text +when the server's parser did not recognise it, as SGLang's `hermes` +parser does with Qwen3.6. Turn on +`openspec-ui.localLlm.agent.askBeforeCommands` +(`OPENSPEC_UI_LOCAL_LLM_ASK_BEFORE_COMMANDS=1`) to allow each command +yourself. + +The model is optional for `local-llm` and `local-llm-acp`: a stage may +name one, the settings may, and otherwise the server is asked which model +it serves. A model id may now contain `/`, as Hugging Face names are +written. + +Agents can ignore the system proxy: `openspec-ui.agents.ignoreSystemProxy` +(`OPENSPEC_UI_IGNORE_SYSTEM_PROXY=1`). The local LLM agents then connect +directly, and CLI agents start without the proxy variables and with +`NO_PROXY=*`. + +A run that failed inside an ACP agent (any of them, not only +`local-llm-acp`) used to say only "Internal error" — the Agent Client +Protocol's own fixed text for an unhandled exception, with the actual +cause (an HTTP status, a bad key, a timeout) discarded. It now says that +cause. + +With `openspec-ui.localLlm.agent.askBeforeCommands` on, clicking Allow on +a direct (non-chain) run used to remove the prompt and then go nowhere — +the answer reached a fresh, unrelated agent instance instead of the one +actually waiting on it, so the run sat there until cancelled by hand. It +now reaches the right one. + + diff --git a/HARNESS.md b/HARNESS.md index d29f05c2..26ad6c99 100644 --- a/HARNESS.md +++ b/HARNESS.md @@ -739,6 +739,7 @@ carried here so a reader sees the value before choosing): | `gemini-cli` | — | — | — | — | | `gemini-cli-acp` | — | — | — | — | | `local-llm` | — | — | — | — | +| `local-llm-acp` | — | — | — | — | | `vscode-chat` | — | — | — | — | Even thirds — 1, 2/3, 1/3, 0 — rather than the tidier-looking 1, 0.75, @@ -959,8 +960,8 @@ binary here" column repeats `README.md`'s own agent table. | `copilot-cli` | `copilot` | Yes (`--model`) | `none`, `minimal`, `low`, `medium`, `high`, `xhigh`, `max` | `maxAiCredits` (`--max-ai-credits`, minimum 30) | Yes | | `codex-cli` | `codex` | No | `minimal`, `low`, `medium`, `high` (from OpenAI's documented config, not live-verified here) | No | **No — never** | | `gemini-cli` | `gemini` | No | No mechanism | No | **No — never** | -| `local-llm` | HTTP to an OpenAI-compatible `/v1/chat/completions`, with a bearer key where one is set; where, which model and which key are set outside the harness file, see "The local LLM" below | No: the model is set with the address | No mechanism | No | Yes, 2026-09-25: a Qwen server on the LAN answered a prompt with its key, and refused it without one. Chat only: it answers in text and edits no file | -| `local-llm-acp` | `coding-agent --base-url --model [limit flags] acp` (the Python coding agent, 0.3.0 or later: the first version that speaks ACP), with the address, model and key of "The local LLM" below; the key travels in the environment, never the command line | No: the model comes from the local LLM settings, not harness model selection | No mechanism | No mechanism | Yes, 2026-10-01, through this runner and its ACP driver against SGLang serving Qwen3.6 on the LAN: an implement run edited code, added tests, ran them, ticked its tasks and reported its tokens. No permission request is ever sent | +| `local-llm` | HTTP to an OpenAI-compatible `/v1/chat/completions`, with a bearer key where one is set; where, which model and which key are set outside the harness file, see "The local LLM" below | Yes, optional (in the request); see "The local LLM" for the order | No mechanism | No | Yes, 2026-09-25: a Qwen server on the LAN answered a prompt with its key, and refused it without one. Chat only: it answers in text and edits no file | +| `local-llm-acp` | Nothing: a coding agent built into the product (ADR 0038), run in process against the local LLM of "The local LLM" below, with tools to read, write, replace in a file, list, search and run a command, all confined to the run's working directory | Yes, optional (in the request); see "The local LLM" for the order | No mechanism | No mechanism | Yes, see the change `local-llm-codes-in-process` for the run that verified it. Asks before a command only when told to (below) | | `claude-cli-acp` | `claude --input-format stream-json --output-format stream-json` | Yes (`--model`) | Same as `claude-cli` | Same as `claude-cli` (`maxCostUsd`) | Progress only — no permission gate, see below | | `copilot-cli-acp` | `copilot --acp` | Yes (`--model`) | Same as `copilot-cli` | Same as `copilot-cli` (`maxAiCredits`) | Yes | | `codex-cli-acp` | externally installed `codex-acp` | No | No mechanism (deliberately empty — see below) | No mechanism (deliberately empty) | **No — never** | @@ -970,15 +971,43 @@ binary here" column repeats `README.md`'s own agent table. ### The local LLM -`local-llm` is set outside `agent-harness.json`: that file is committed, an address on the LAN is one machine's, and a key committed is a key published. +`local-llm` and `local-llm-acp` are set outside `agent-harness.json`: that file is committed, an address on the LAN is one machine's, and a key committed is a key published. | Setting | In VS Code | In the standalone and the CLI | When unset | | --- | --- | --- | --- | | Base URL, with its `/v1` or without | `openspec-ui.localLlm.baseUrl` | `OPENSPEC_UI_LOCAL_LLM_BASE_URL` | `http://localhost:30000` | -| Model, as its server names it | `openspec-ui.localLlm.model` | `OPENSPEC_UI_LOCAL_LLM_MODEL` | `default` | +| Model, as its server names it | `openspec-ui.localLlm.model` | `OPENSPEC_UI_LOCAL_LLM_MODEL` | the server is asked (below) | | API key | **OpenSpec Workbench: Set Local LLM API Key...**, kept in the editor's secret storage | `OPENSPEC_UI_LOCAL_LLM_API_KEY` | no `Authorization` header | +| Ask before each command (`local-llm-acp`) | `openspec-ui.localLlm.agent.askBeforeCommands` | `OPENSPEC_UI_LOCAL_LLM_ASK_BEFORE_COMMANDS=1` | commands run without asking | -In VS Code the editor's value wins and the environment variable is the fallback. All three are read when the window opens, or when the server or the CLI starts. The key goes to the request's `Authorization: Bearer` header and nowhere else: not to the audit log, not to a run log. The agent answers in text: it can review, and it cannot write a proposal or tick a task. +In VS Code the editor's value wins and the environment variable is the fallback. All of them are read when the window opens, or when the server or the CLI starts. The key goes to the request's `Authorization: Bearer` header and nowhere else: not to the audit log, not to a run log. + +**The model is optional.** A run takes the first of: the stage's own `model` (`stepAgents..model`, which both agents accept); the setting above; the model the server lists at `/v1/models` (the first, where it lists several; asked once per address while the window or the server is open); `default`. The run's first output names the model and where it came from, for example `Model QuantTrio/Qwen3.6-35B-A3B-AWQ (the model the server serves).` + +**`local-llm`** answers in text: it can review, and it cannot write a proposal or tick a task. + +**`local-llm-acp`** is a coding agent built into the product (ADR 0038): nothing to install. It runs the model in a loop with six tools, `read_file`, `write_file`, `replace_text`, `list_dir`, `search_text` and `run_command`, and streams each call and its result as the run's updates. + +- Every path is resolved by its real location, links included, and refused when that lies outside the run's working directory. +- A command runs in the working directory, is ended at `OPENSPEC_UI_LOCAL_LLM_ACP_COMMAND_TIMEOUT_SECONDS` (60 s by default), and its output is cut at `OPENSPEC_UI_LOCAL_LLM_ACP_MAX_COMMAND_OUTPUT_CHARS` (12000 by default). +- With **ask before each command** on, a command waits for Allow in the run's permission prompt; leave it off for a chain nobody watches. Writes inside the working directory never ask. +- A tool call the model wrote as text, which a server whose tool-call parser does not match the model passes through in `content` (Qwen3-Coder's `` form, or Hermes' JSON, inside ``), is read as a call. +- Its loop is bounded by the `OPENSPEC_UI_LOCAL_LLM_ACP_*` limits in `LIMITS.md`. + +### Ignoring the system proxy + +**`openspec-ui.agents.ignoreSystemProxy`** in VS Code, or **`OPENSPEC_UI_IGNORE_SYSTEM_PROXY=1`** for the standalone server and the CLI, tells agents to ignore the system proxy. Off by default. Turn it on when the proxy cannot reach your model, such as a server on your LAN behind a proxy that resets such requests. + +- `local-llm`, `local-llm-acp` and the check that says whether the local LLM is there connect directly, through a connection pool of their own, whatever proxy the editor applies to its own networking. +- A CLI agent is started with `HTTP_PROXY`, `HTTPS_PROXY` and `ALL_PROXY` removed from its environment, in either case, and `NO_PROXY=*`. Only a CLI that reads those variables honours that. + +| Agent | Honours the switch | +| --- | --- | +| `local-llm`, `local-llm-acp` | Yes: they run in the product | +| `claude-cli`, `claude-cli-acp`, `copilot-cli`, `copilot-cli-acp` | Through the environment: their vendors document reading the proxy variables | +| `codex-cli`, `codex-cli-acp`, `gemini-cli`, `gemini-cli-acp`, `deepseek-cli-acp`, `vscode-chat` | Unknown: not checked here; such a CLI may keep proxy settings of its own | + +The column is `HARNESS_AGENT_CAPABILITIES[*].systemProxy`. **On the "run against the real binary here" column, plainly: `codex` and `gemini` have never been run by this project at all**, raw or ACP-flavored — see `README.md`'s "Agent Selection" section for the full diff --git a/LIMITS.md b/LIMITS.md index 17afc09e..a53b71ff 100644 --- a/LIMITS.md +++ b/LIMITS.md @@ -147,7 +147,28 @@ conversion happens at all, so nothing to silently drift. | --- | --- | --- | --- | | `claude-cli`, `claude-cli-acp` | `maxCostUsd` | `--max-budget-usd` | Requires Claude Code v2.1.217 or later. | | `copilot-cli`, `copilot-cli-acp` | `maxAiCredits` | `--max-ai-credits` | Minimum 30 — a configured value below this is rejected before any run starts. | -| `codex-cli`, `gemini-cli`, `local-llm`, `codex-cli-acp`, `gemini-cli-acp`, `deepseek-cli-acp`, `vscode-chat` | Neither | — | No spending-cap mechanism at all; a `stepAgents` entry setting either field for one of these is rejected. | +| `codex-cli`, `gemini-cli`, `local-llm`, `local-llm-acp`, `codex-cli-acp`, `gemini-cli-acp`, `deepseek-cli-acp`, `vscode-chat` | Neither | — | No spending-cap mechanism at all; a `stepAgents` entry setting either field for one of these is rejected. | + +**`local-llm-acp`'s own loop is bounded instead** (ADR 0038). It runs in +the product, so these limits act on the loop itself rather than on a +process. Each is read from the environment when the window, the server or +the CLI starts; an absent or invalid value means the default. + +| Variable | Bounds | Default | +| --- | --- | --- | +| `OPENSPEC_UI_LOCAL_LLM_ACP_MAX_ITERATIONS` | Model turns in one run | 40 | +| `OPENSPEC_UI_LOCAL_LLM_ACP_MAX_TOOL_CALLS` | Tool calls in one run | 120 | +| `OPENSPEC_UI_LOCAL_LLM_ACP_MAX_SECONDS` | Seconds the loop may run | 1800 | +| `OPENSPEC_UI_LOCAL_LLM_ACP_COMMAND_TIMEOUT_SECONDS` | Seconds one command may run | 60 | +| `OPENSPEC_UI_LOCAL_LLM_ACP_MAX_COMMAND_OUTPUT_CHARS` | Characters of a command's output the model is given | 12000 | +| `OPENSPEC_UI_LOCAL_LLM_ACP_MAX_PROMPT_TOKENS`, `..._MAX_COMPLETION_TOKENS`, `..._MAX_TOTAL_TOKENS` | Tokens over the run, as the server reports them; `..._MAX_COMPLETION_TOKENS` is also sent as each request's `max_tokens` | none | + +A run that reaches one stops, says which, and ends with the protocol's +`max_turn_requests` or `max_tokens`. The harness's own `timeout` and +`maxStageAttempts` still bound the stage around it. The context limits +the external agent took (`..._MAX_CONTEXT_*`, `..._MIN_FREE_CONTEXT_TOKENS`) +are read but act on nothing: no OpenAI-compatible answer says how full the +context is. **A mismatched field is refused when the configuration resolves, not minutes into a run.** Setting `stepAgents.apply.budget.maxAiCredits` while @@ -332,6 +353,7 @@ spend, as `deepseek-cli-acp` does; the two are not the same column. | --- | --- | --- | | `deepseek-cli-acp` | Yes, on every turn | **Measured** 2026-09-23: `{"used":8202,"size":1000000,"sessionUpdate":"usage_update"}`, and `dsh-acp`'s own `usageUpdate` builds it from a context meter | | `claude-cli`, `copilot-cli`, `codex-cli`, `gemini-cli`, `local-llm`, `vscode-chat` | No | Certain — they speak no ACP at all, so no update of any kind arrives | +| `local-llm-acp` | No | Certain — it is the product's own agent and sends none: no OpenAI-compatible answer says how full the context is | | `copilot-cli-acp`, `claude-cli-acp`, `gemini-cli-acp`, `codex-cli-acp` | Not known | *Unobserved here.* Nothing is claimed either way, and no warning is raised: an ACP agent nobody has watched may well send one | | Agent | Reports usage | Evidence | Source | @@ -340,6 +362,7 @@ spend, as `deepseek-cli-acp` does; the two are not the same column. | `claude-cli-acp` | Cost (USD), input/output/cache tokens, per-model split | **Measured** — see below | `claude`'s own terminal `"result"` line (`total_cost_usd`, `usage`, `modelUsage`) | | `gemini-cli-acp`, `codex-cli-acp` | Whatever that CLI sends over ACP — token totals, a cost, or nothing | *Unobserved* | ACP's `PromptResponse.usage` and `usage_update` notifications | | `deepseek-cli-acp` | Nothing countable: no cost, no credits, no token split | **Measured** 2026-09-23, correcting 2026-09-22: a run through `dsh` 0.1.5-rc.2 does send `usage_update`, and what it carries is `used` and `size` - the tokens now in the session's context, against the model's 1,000,000-token window. One number, growing as the conversation grows: no input/output/cache/thought split, no per-model figures, no currency. The prompt's own answer carries `stopReason` and nothing else. `dsh`'s ACP layer builds that notification from its context meter alone, so there is nothing further to read. A context gauge is not a spend, and nothing here converts one into the other | ACP's `PromptResponse.usage` and `usage_update` notifications | +| `local-llm-acp` | Input and output **tokens**, summed over the run's model calls. **No cost.** | Certain — the product's own agent adds up what the server reports in each answer's `usage` | ACP's `PromptResponse.usage` | | `claude-cli`, `copilot-cli`, `codex-cli`, `gemini-cli`, `local-llm` | Nothing | Certain — plain text carries no figure to record | Plain text output | | `vscode-chat` | Nothing | Certain | The run is handed to VS Code chat; this project never sees its cost | diff --git a/README.md b/README.md index 0f8448e9..e66963f9 100644 --- a/README.md +++ b/README.md @@ -456,7 +456,7 @@ streams events over the same protocol already used for | Codex CLI | `codex` | **No — never** | | Gemini CLI | `gemini` | **No — never** | | Local LLM (OpenAI-compatible) | HTTP to `http://localhost:30000` by default | Not exercised live | -| Local LLM (ACP, OpenAI-compatible) | `coding-agent --base-url --model acp` (the Python coding agent, 0.3.0+) | Yes, against SGLang serving Qwen3.6; edits files, never asks permission | +| Local LLM agent (OpenAI-compatible, built in) | Nothing to install: a coding agent built into the product, against the same server (ADR 0038) | Yes, against SGLang serving Qwen3.6; edits files in the change's directory, asks before commands when told to | | Claude CLI (ACP) | `claude --input-format stream-json --output-format stream-json` | Progress only — no permission gate, see below | | GitHub Copilot CLI (ACP) | `copilot --acp` | Yes | | Codex CLI (ACP) | externally installed `codex-acp` | **No — never** | diff --git a/docs/adr/0038-the-local-model-codes-in-the-product.md b/docs/adr/0038-the-local-model-codes-in-the-product.md new file mode 100644 index 00000000..a2e4d790 --- /dev/null +++ b/docs/adr/0038-the-local-model-codes-in-the-product.md @@ -0,0 +1,127 @@ +# 0038: The Local Model Codes in the Product + +Status: Accepted + +Date: 2026-10-02 + +## Context + +A person with a model on their own network (SGLang, vLLM, anything that +answers OpenAI's `/v1/chat/completions`) has two agents to choose from, +and neither does the job: + +- **`local-llm`** sends the prompt once and streams the answer back. It + has no tools: it can describe a change and cannot make one. HARNESS.md + says so: "Chat only: it answers in text and edits no file". +- **`local-llm-acp`** starts an external process, `coding-agent`, and + speaks the Agent Client Protocol to it. It made a change on 2026-10-01, + against SGLang serving Qwen3.6, once `coding-agent` 0.3.0 learned the + protocol (see `local-llm-acp`). But `coding-agent` is a separate Python + package in a separate repository, published nowhere a user of this + product would find it. It needs Python 3.12 and an installation by hand, + and its version has to match ours. For everyone but its author the + agent reads "not detected". + +Three more facts came out of the same day: + +- **The tools of an external agent are outside this product's security + model.** ADR 0001 and the execution-core invariants require an + allowlist, a working-directory sandbox and an audit for what agents run. + For a CLI agent the product can only allowlist the command line it + starts. What the process then does is the process's business: + `coding-agent` writes files and runs commands without asking. +- **A model's tool calls do not always arrive as tool calls.** SGLang on + the LAN runs `--tool-call-parser hermes`, and Qwen3.6 writes its calls + in Qwen3-Coder's `` form, which that parser + does not recognise: 0 of 6 structured calls when measured. The calls + arrive as text in `content`. An agent for local models has to read them + there, whatever the server is configured with. +- **The system proxy is in the way of the LAN.** On the maintainer's + machine every HTTP client that obeys `HTTP_PROXY`, and the VS Code + extension host, which applies the editor's proxy to Node's networking, + send requests for `192.168.137.0/24` to a proxy that resets them. A + person has no way to tell an agent to go direct. + +## Decision + +1. **The local model's coding agent runs inside `packages/core`.** It is an + Agent Client Protocol agent implemented in TypeScript and run in + process. `AcpSessionDriver` already connects to an in-process `AgentApp` + (its tests do), so the run reaches the same events, the same stop + handling, the same permission prompts and the same audit as every ACP + agent. Nothing is installed besides the extension or the standalone + server. + +2. **It keeps the id `local-llm-acp`.** A harness file that names it goes + on working. The external `coding-agent` process, its allowlist entry + and its executable setting are removed. `coding-agent` stays a + separate tool for anyone who wants one, and this product no longer + uses it. + +3. **Its tools are the product's own, and they stay in the change's + working directory.** Read, write, replace in a file, list, search, and + run a command. A path is resolved and refused if its real location is + outside the run's `cwd`, links included. A command runs in `cwd` with a + time limit and an output cap. Every tool call is a `tool_call` update, + so it is seen, logged and counted. Commands run without asking by + default, as `copilot`, `claude` and `gemini` do under this product. A + setting makes the agent ask before each command through the protocol's + permission request, which the product already shows in both hosts. + +4. **It reads a tool call wherever the model put it.** A structured + `tool_calls` entry first. Where there is none, a call written in the + text inside ``, in Qwen3-Coder's XML form or as Hermes' + JSON, to a tool that was offered, is taken as a call. + +5. **The model is named or found.** A stage may name a model for + `local-llm` and `local-llm-acp`, as it may for CLIs that take + `--model`. Without one, the local LLM settings name it. Without those, + the agent asks the server's `/v1/models` and takes the model it serves + when it serves one. Only then does the old default, `default`, apply. + +6. **An agent can be told to ignore the system proxy.** One setting, off + by default, in the editor's settings and as an environment variable for + the standalone server and the CLI. When it is on: + - the in-process agents (`local-llm`, `local-llm-acp`) and the local + LLM's availability check connect directly, through their own + connection pool rather than the process's global one; + - a CLI agent is started with the proxy variables (`HTTP_PROXY`, + `HTTPS_PROXY`, `ALL_PROXY`, either case) removed from its + environment and `NO_PROXY=*`. That reaches only an agent that reads + those variables, so each agent's capability row says whether it does. + +## Alternatives Considered + +- **Publish `coding-agent` and keep it external.** Rejected: users would + still need Python, a second installation and matching versions. Its + tools would stay outside the allowlist, the sandbox and the permission + prompt. +- **Adopt an existing published agent (Qwen Code, opencode).** Rejected + for the same reasons, and because none was verified against the + protocol and the models this was written for. The decision does not + rule out adding one later as one more CLI agent. +- **Give `local-llm` tools and drop `local-llm-acp`.** Rejected: a harness + file that names `local-llm-acp` would break, and `local-llm` as a plain + chat is still the right agent for a stage that should not touch files. +- **Ignore the proxy always for the local LLM.** Rejected: a person whose + model is reached only through the proxy would lose it. The setting is + off by default, and the person turns it on. +- **Change the server's tool-call parser.** That is the server's business: + this product cannot assume it, and on the LAN the parser is kept for + reasons of its own. + +## Consequences + +- `packages/core` starts files being written and commands being run by + something other than an external CLI. The allowlist cannot cover that, + so the sandbox check, the command time limit and the optional + permission prompt are the agent's own and are tested as the security + model is. +- The agent loop, its tools and the text-call reader are code this + product maintains. They are ported from `coding-agent`, whose behaviour + was verified live on 2026-10-01. +- The `local-llm-acp` change that fixed the external command line is + superseded once this lands. Its `LOCAL_LLM_ACP_*` limits keep their + names and now limit the in-process loop. +- A person behind a proxy that breaks the LAN can turn it off for agents + in one place. diff --git a/docs/adr/README.md b/docs/adr/README.md index bed0ac02..da10551a 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -44,5 +44,6 @@ Alternatives / Consequences. | [0035](0035-a-landed-change-is-archived-for-you.md) | A change that has landed is archived for you | Accepted; decision 5 superseded by 0036 | | [0036](0036-the-product-merges-what-it-archives.md) | The product merges what it archives | Accepted | | [0037](0037-a-team-works-through-git.md) | A team works through git | Accepted | +| [0038](0038-the-local-model-codes-in-the-product.md) | The local model's coding agent runs in the product, and an agent can ignore the system proxy | Accepted | New architecture-impacting changes must add an ADR and reference it from the related OpenSpec change. diff --git a/openspec/changes/local-llm-codes-in-process/.openspec.yaml b/openspec/changes/local-llm-codes-in-process/.openspec.yaml new file mode 100644 index 00000000..bc882da5 --- /dev/null +++ b/openspec/changes/local-llm-codes-in-process/.openspec.yaml @@ -0,0 +1,7 @@ +schema: spec-driven +created: 2026-10-02 +follows: + - local-llm-acp + - the-local-llm-is-where-you-say +blocked_by: + - local-llm-acp diff --git a/openspec/changes/local-llm-codes-in-process/design.md b/openspec/changes/local-llm-codes-in-process/design.md new file mode 100644 index 00000000..93da16b4 --- /dev/null +++ b/openspec/changes/local-llm-codes-in-process/design.md @@ -0,0 +1,173 @@ +## Context + +See `proposal.md` and ADR 0038. What exists: + +- `AcpSessionDriver.run()` takes an `AcpConnectTarget`, which is either a + stdio `Stream` or an in-process `AgentApp`. Only the tests use the + second today. `runProcess()` spawns a process and calls `run()` with its + stdio. +- `local-llm` is a one-shot HTTP adapter with no tools. +- `local-llm-acp` (change `local-llm-acp`, PR #803) starts + `coding-agent --base-url --model [limits] acp`. +- `coding-agent` 0.3.0 (repository `CodingAgent`, its ADR 0002) is a + working reference: the agent loop, the tools, the ACP surface and the + text-call reader were verified live against SGLang serving Qwen3.6 on + 2026-10-01 (run `live-local-llm-acp-1790853181423`). +- The local LLM's base URL, model and key resolve from the host, then + the environment, then defaults (`resolveLocalLlmSettings`). +- A model is accepted for an agent only where its registry entry has a + `modelFlag` (`harness-config.ts`, `harness-config-schema.ts`, + `harness-settings-parts.tsx`). +- Nothing in the product handles a proxy. + +## Goals / Non-Goals + +**Goals:** + +- A local model makes a change from both hosts with nothing installed but + the product. +- Its tools are confined to the run's `cwd`, and every call is seen. +- Tool calls are read whether the server parsed them or not. +- A person may name the model, and need not. +- A person can tell agents to ignore the system proxy. + +**Non-Goals:** + +- No new command kind or event kind. The agent speaks ACP; `agentUpdate`, + `permissionRequest` and `usageReported` already carry what it says. +- No MCP servers for the in-process agent; `mcpServers` is accepted and + ignored, as `coding-agent` does. +- No change to `local-llm`'s one-shot chat, except how its model is chosen + and its proxy. +- No change to the server's tool-call parser (DocsAI keeps `hermes`). +- No per-host proxy list. One switch, and the agents that can honour it. +- `coding-agent` is not deleted or changed; this product stops using it. + +## Decisions + +1. **An in-process `AgentApp`, driven by the existing `AcpSessionDriver.run()`.** + - Chosen: `LocalLlmAcpAdapter` builds an `AgentApp` and runs it through + `driver.run({ target: agentApp, ... })`. + - Rejected: a new adapter shape that emits events directly. It would + duplicate the ACP translation, permission routing and usage handling + the driver has, which ADR 0013 put in one place. + - Rejected: spawning a bundled Node script over stdio. A process + boundary buys nothing in process and costs startup, a second + termination path and an allowlist entry for our own file. + +2. **The invocation is `{ kind: "in-process" }`, not a process.** + - Chosen: `AdapterInvocation` gains an `in-process` kind. The allowlist + check accepts it only for an adapter registered as in-process, and + the audit records it as such. + - Rejected: reusing `kind: "http"` with a fake URL. The audit would + record something that never happened. + - Protocol compatibility: `AdapterInvocation` is core-internal; no + server or extension adapter reads its kinds, and no wire format + changes. + +3. **Tools are ported from `coding-agent`, sandboxed by real path.** + - Chosen tools: `read_file`, `write_file`, `replace_text`, `list_dir`, + `search_text`, `run_command`. + - Not ported: git, background processes, delete and move. A run that + needs them can use `run_command`. Fewer tools leave the model less to + choose wrongly among. + - Every path is resolved with `realpath` (the parent's, for a file not + yet written) and refused unless inside `realpath(cwd)`. This is the + check `security.ts` already uses for change artifacts. + - `run_command` runs through the platform shell in `cwd`, with the + `commandTimeoutSeconds` limit (default 60) and a + `maxCommandOutputChars` cap (default 12000). On Windows the timeout + kills the process tree with `terminateProcessTree`, as cancellation + does. + - Rejected: no `run_command`. A coding agent that cannot run the tests + it writes cannot verify a task, and `tasks.md` items here are + verified by commands. + +4. **Commands ask first only when a person said so.** + - Chosen: `openspec-ui.localLlm.agent.askBeforeCommands` (default + false; `OPENSPEC_UI_LOCAL_LLM_ASK_BEFORE_COMMANDS=1`). When true, + `run_command` sends `session/request_permission` and runs only on + allow. Writes inside `cwd` never ask. + - Rejected: always ask. A chain under `autonomous` has nobody to + answer, and the run would stall at its first command (`config.yaml`: + a run never waits for a human). + - Rejected: never offer asking. Then the one agent that could honour a + permission gate would not, which the README already says of two + others. + +5. **Tool calls are read from the text when the server did not parse them.** + - Chosen: a port of `coding-agent`'s `tool_calls_in_text`. It reads + `` blocks in Qwen3-Coder's XML form or Hermes' JSON, takes + only offered tools, and reads a parameter as JSON only where the + tool's schema declares an array or an object. + - Rejected: require the server's parser to match. The product cannot + configure the server, and on the LAN it is configured otherwise. + +6. **The model: stage, then settings, then `/v1/models`, then `default`.** + - Chosen: `AgentDescriptor` gains `acceptsModel` (true where + `modelFlag` is set, and for `local-llm` and `local-llm-acp`). The + three places that read `modelFlag` to decide acceptance read + `acceptsModel` instead. The stage's model wins. Otherwise + `resolveLocalLlmSettings`. Otherwise `GET /v1/models` (with the + key, 10 s), taking the only model when one is served and the first + when several are, and saying which in the run's first update. + Otherwise `default`. + - The answer from `/v1/models` is cached per base URL for the life of + the host process, so a chain asks once. + - Rejected: make the model required. The owner asked for optional, and + a server serving one model needs no name. + +7. **Ignoring the system proxy: one switch, honoured where it can be.** + - Chosen: `openspec-ui.agents.ignoreSystemProxy` (default false) and + `OPENSPEC_UI_IGNORE_SYSTEM_PROXY=1`, read into + `DefaultRunnersConfig.ignoreSystemProxy`. + - When on, every request the local LLM agents and the availability + check make goes through a dedicated `undici` `Agent` with no proxy, + using `undici`'s own `fetch`, not the global one. The editor host may + have replaced the global `fetch` and dispatcher to apply its proxy. + - When on, `spawnAndStream` and `spawnAcpProcess` give a CLI an + environment without `HTTP_PROXY`, `HTTPS_PROXY`, `ALL_PROXY` (either + case) and with `NO_PROXY=*`. + - `HarnessAgentCapabilities` gains `systemProxy`: `"ignored"` for the + in-process agents, `"environment"` for a CLI known to read the + variables, `"unknown"` otherwise, and a finding states it when the + switch is on. + - Rejected: a host list. The owner asked for "ignore", and a list + would be a second network configuration to get wrong. + - Rejected: always direct for the local LLM. A model reached only + through a proxy would be lost. + +## Risks / Trade-offs + +- [Risk] The product now runs commands itself → mitigation: `cwd` + confinement by real path, a time limit, an output cap, every call + logged as a tool-call update, the optional permission prompt, and tests + that try `..`, absolute paths and links. +- [Risk] A text-call reader can take something that was not meant as a + call → only offered tools, only inside ``, and the text is + kept when nothing parses. +- [Risk] `/v1/models` names a model a person did not mean → the run's + first update says which model was chosen and from where. +- [Risk] Removing `HTTP_PROXY` breaks a CLI that needs the proxy to reach + its own vendor → the switch is off by default and is described as + applying to every agent. +- [Risk] `undici` becomes a direct dependency of `core` → it is the + library Node's own `fetch` is built on, pinned to the version the + pinned Node ships. +- [Trade-off] Fewer tools than `coding-agent` (no git, no background + processes) → `run_command` covers them. + +## Migration Plan + +1. `local-llm-acp` lands and is archived (blocked_by). +2. The in-process agent replaces the external process under the same id. + The `coding-agent` allowlist entry and the executable setting go. +3. Settings and schema changes ship in the same release. A harness file + that set no model behaves as before. One that set a model for + `local-llm` or `local-llm-acp` was refused before and is accepted now. + +## Open Questions + +- Whether the permission prompt should also cover writes outside + `openspec/changes//` for a `plan` command. Out of scope here: the + propose stage's instruction already forbids them. diff --git a/openspec/changes/local-llm-codes-in-process/proposal.md b/openspec/changes/local-llm-codes-in-process/proposal.md new file mode 100644 index 00000000..2433662f --- /dev/null +++ b/openspec/changes/local-llm-codes-in-process/proposal.md @@ -0,0 +1,95 @@ +## Why + +[ADR 0038](../../../docs/adr/0038-the-local-model-codes-in-the-product.md). +A model on a person's own network can chat (`local-llm`) but cannot make a +change unless that person finds, installs and keeps in step a separate +Python agent, `coding-agent`, which `local-llm-acp` starts. For every user +but its author that agent reads "not detected". Its tools also sit outside +this product's allowlist, sandbox and permission prompt, and it writes and +runs without asking. + +Two more things, from the live runs of 2026-10-01 against SGLang serving +Qwen3.6: + +- The model's tool calls arrived as text in `content`: the server's + `hermes` parser does not recognise Qwen3-Coder's + `` form (0 of 6 structured calls when + measured). An agent for local models has to read them there. +- On the maintainer's machine the system proxy resets every request to the + LAN, and nothing lets a person tell an agent to go direct. + +And one request from the owner: the model a local agent uses should be a +name a person may give, and need not give. + +## What Changes + +- **`local-llm-acp` becomes an agent that runs inside `packages/core`**: an + Agent Client Protocol agent in TypeScript, connected in process through + the existing `AcpSessionDriver`. It runs a tool loop against the local + LLM's `/v1/chat/completions` with tools to read, write, replace in a + file, list, search, and run a command. Every tool call streams as an ACP + update. Same id, so harness files keep working. The external + `coding-agent` process, its allowlist entry and its executable go. +- **Its tools stay in the run's working directory**: a path whose real + location is outside `cwd` is refused, links included. A command runs in + `cwd` with a time limit and an output cap. By default commands run + without asking, like the other agents here; a setting makes it ask + through the protocol's permission request, which both hosts show. +- **It reads a tool call wherever the model put it**: structured + `tool_calls` first, else a call to an offered tool written as text + inside ``, in Qwen3-Coder's XML form or Hermes' JSON. +- **The model is named or found** for `local-llm` and `local-llm-acp`. A + stage may name it (`stepAgents..model`), as for CLIs that take + `--model`. Otherwise the local LLM settings name it. Otherwise the agent + asks the server's `/v1/models` and uses the model it serves. Otherwise it + uses `default`, as today. The Harness Settings views offer the model + field for both agents. +- **An agent can be told to ignore the system proxy**: one setting, off by + default, `openspec-ui.agents.ignoreSystemProxy` in the editor and + `OPENSPEC_UI_IGNORE_SYSTEM_PROXY=1` for the standalone server and the + CLI. When on, the in-process agents and the local LLM's availability + check connect directly. A CLI agent is started with the proxy variables + removed from its environment and `NO_PROXY=*`, which reaches the agents + that read them. Each agent's capability row says whether it does. +- The `LOCAL_LLM_ACP_*` limits keep their names and limit the in-process + loop. + +## Capabilities + +### New Capabilities + +(none) + +### Modified Capabilities + +- `execution-core`: the local LLM's coding agent, its tools and their + sandbox, how it reads tool calls, how its model is chosen, and ignoring + the system proxy. +- `agentic-harness`: a stage may name a model for `local-llm` and + `local-llm-acp`. + +## Impact + +- `packages/core`: + - a new in-process agent (agent loop, tools, sandbox, text-call reader) + under `src/agents/`; + - `local-llm-acp.ts` and `default-runners.ts` stop starting a process; + - `local-llm-settings.ts` gains model discovery and the proxy setting; + - `agents/registry.ts` and `harness-config.ts` let `local-llm` and + `local-llm-acp` take a model; + - `shared.ts` and `acp-session-driver.ts` give a spawned CLI an + environment without proxy variables when asked; + - `agent-detection.ts` checks the local LLM directly when asked. +- `packages/extension`: the `openspec-ui.agents.ignoreSystemProxy` and + `openspec-ui.localLlm.agent.askBeforeCommands` settings, passed to + `buildDefaultAgentRunners`; the agent-harness schemas accept a model + for both agents. +- `packages/server`, `packages/cli`: the same two settings from the + environment. +- `packages/webui`: the Harness Settings views show the model field for + both agents. +- `HARNESS.md`, `LIMITS.md`, `README.md`. +- No command or event kind is added: the agent speaks ACP, whose updates + and permission requests the protocol already carries. +- Blocked by `local-llm-acp`, which must land and be archived first: both + change the local LLM's requirement in `execution-core`. diff --git a/openspec/changes/local-llm-codes-in-process/specs/agentic-harness/spec.md b/openspec/changes/local-llm-codes-in-process/specs/agentic-harness/spec.md new file mode 100644 index 00000000..a88fc37d --- /dev/null +++ b/openspec/changes/local-llm-codes-in-process/specs/agentic-harness/spec.md @@ -0,0 +1,22 @@ +## ADDED Requirements + +### Requirement: A stage may name the local model + +A `stepAgents` entry SHALL accept a model for `local-llm` and +`local-llm-acp`, as it does for an agent whose CLI takes `--model`, and the +Harness Settings views SHALL offer the model field for both. The model +SHALL be optional: an entry without one SHALL keep its meaning, and the +model SHALL then be found as `execution-core` describes. A model SHALL be +checked against the same permitted character set as any other. + +#### Scenario: A model for the local agent + +- **WHEN** a stage's entry is `{ "agent": "local-llm-acp", "model": "QuantTrio/Qwen3.6-35B-A3B-AWQ" }` +- **THEN** the configuration is read without error, and the stage's run + uses that model + +#### Scenario: No model for the local agent + +- **WHEN** a stage's entry is `"local-llm-acp"` alone +- **THEN** the configuration is read without error, and the model is + found from the settings or the server diff --git a/openspec/changes/local-llm-codes-in-process/specs/execution-core/spec.md b/openspec/changes/local-llm-codes-in-process/specs/execution-core/spec.md new file mode 100644 index 00000000..6e1b5812 --- /dev/null +++ b/openspec/changes/local-llm-codes-in-process/specs/execution-core/spec.md @@ -0,0 +1,112 @@ +## ADDED Requirements + +### Requirement: The local model makes a change with nothing installed + +`local-llm-acp` SHALL be an Agent Client Protocol agent that runs inside +`packages/core` and is driven by `AcpSessionDriver`, with no process +started and nothing to install besides the product. It SHALL run a tool +loop against the local LLM's `/v1/chat/completions`, with the base URL and +key the local LLM settings resolve. It SHALL report each piece of the +model's text, each tool call and each tool result as ACP updates, and the +tokens the model reported as usage. Its loop SHALL be bounded by the +`LOCAL_LLM_ACP_*` limits: iterations, tool calls, seconds, command +timeout, command output, and tokens. + +#### Scenario: A run that edits code + +- **WHEN** an `implement` command runs on `local-llm-acp` against a model + that calls `write_file` +- **THEN** the file is written in the run's `cwd`, a `tool_call` and its + `tool_call_update` reach the run's events, and the run completes + +#### Scenario: A limit is reached + +- **WHEN** the model keeps calling tools past the iteration limit +- **THEN** the run stops and says which limit stopped it + +### Requirement: The local agent's tools stay in the run's directory + +The tools of `local-llm-acp` SHALL read, write and search only inside the +run's `cwd`. A path SHALL be refused when its real location, links +resolved, is outside the real location of `cwd`. A command SHALL run in +`cwd`, SHALL be ended at the command time limit, and its output SHALL be +cut at the output cap. Where the host's `askBeforeCommands` setting is on, +a command SHALL run only after a permission request is allowed. + +#### Scenario: A path outside the directory + +- **WHEN** the model asks to read `../outside.txt`, an absolute path + elsewhere, or a link that resolves outside `cwd` +- **THEN** the tool answers with an error naming the path, and nothing + outside `cwd` is read or written + +#### Scenario: Asking before a command + +- **WHEN** `askBeforeCommands` is on and the model calls `run_command` +- **THEN** a `permissionRequest` naming the command reaches the run's + events, and the command runs only if it is allowed + +### Requirement: A tool call is read wherever the model put it + +The local agent SHALL take a model's structured `tool_calls`. Where an +answer carries none, it SHALL take a call written in the text inside +``, in Qwen3-Coder's `` form or as +Hermes' JSON, provided the tool was offered, and SHALL remove it from the +text. A parameter written as text SHALL be read as JSON only where the +tool declares an array or an object. + +#### Scenario: A server whose parser does not match the model + +- **WHEN** an answer's text holds `src/a.js` + and `tool_calls` is empty or null +- **THEN** the agent calls `read_file` with path `src/a.js` + +#### Scenario: A tool that was not offered + +- **WHEN** the text names a tool that was not offered +- **THEN** no call is made and the text is kept + +### Requirement: The local model is named or found + +For `local-llm` and `local-llm-acp`, the model SHALL be the stage's model +where the stage names one; else the one the local LLM settings name; else +the model the server lists at `/v1/models`, the only one where it serves +one and the first where it serves several; else `default`. A run SHALL +say, in its first update, which model it uses and where the name came +from. + +#### Scenario: Nothing names a model + +- **WHEN** no stage, setting or environment variable names a model, and + the server's `/v1/models` lists `QuantTrio/Qwen3.6-35B-A3B-AWQ` +- **THEN** the run uses that model and says it came from the server + +#### Scenario: The stage names one + +- **WHEN** the stage's entry names a model and the settings name another +- **THEN** the run uses the stage's + +### Requirement: An agent can be told to ignore the system proxy + +A host SHALL offer one switch, off by default, that tells agents to ignore +the system proxy: an editor setting, and `OPENSPEC_UI_IGNORE_SYSTEM_PROXY` +for the standalone server and the CLI. When it is on, the local LLM +agents and the local LLM's availability check SHALL connect directly +through a connection pool of their own. Each CLI agent SHALL be started +with `HTTP_PROXY`, `HTTPS_PROXY` and `ALL_PROXY` removed from its +environment, in either case, and `NO_PROXY=*`. Each agent's capability row +SHALL say whether it honours the switch: `ignored`, `environment` or +`unknown`. + +#### Scenario: The switch is on + +- **WHEN** the switch is on and `HTTPS_PROXY` is set in the host's + environment +- **THEN** the local LLM is reached directly, and a CLI agent's + environment has no proxy variable and has `NO_PROXY=*` + +#### Scenario: The switch is off + +- **WHEN** the switch is off +- **THEN** every agent's environment and connections are what they were + before this requirement diff --git a/openspec/changes/local-llm-codes-in-process/tasks.md b/openspec/changes/local-llm-codes-in-process/tasks.md new file mode 100644 index 00000000..e0fbef0b --- /dev/null +++ b/openspec/changes/local-llm-codes-in-process/tasks.md @@ -0,0 +1,262 @@ +Requested by the owner on 2026-10-02: the local model's coding agent +lives in the product (ADR 0038), its model is optional, and agents can +ignore the system proxy. Blocked by `local-llm-acp`. + +## 1. The decision + +- [x] 1.1 **Human-only**: the owner accepts ADR 0038 and this proposal; + `docs/adr/0038-the-local-model-codes-in-the-product.md` then reads + `Status: Accepted`, and `docs/adr/README.md`'s row says so. The owner + answered "go ahead" on 2026-10-02 to the proposal pushed as + `8ed05ab8`, which asked whether anything should change before + implementation; the ADR and its row now read Accepted. + +## 2. The in-process agent + +- [x] 2.1 `packages/core/src/agents/local-agent/text-tool-calls.ts`: + `toolCallsInText(content, parameterTypes)`, ported from + `coding-agent`'s `tool_calls_in_text`. Test in `text-tool-calls.test.ts`: + a Qwen3-Coder call, a Hermes call, JSON-looking file content kept as + text, a tool not offered left as text, several calls numbered. 5 passed. +- [x] 2.2 `packages/core/src/agents/local-agent/sandbox.ts`: + `resolveInside(cwd, path)` by real path, the nearest existing ancestor's + real path for a file not yet written. Test in `sandbox.test.ts`: `..`, + an absolute path elsewhere, a link pointing outside (a junction on + Windows), and paths inside, each answered. 4 passed. Do not resolve with + `path.resolve` alone: a link inside `cwd` can point outside it. +- [x] 2.3 `packages/core/src/agents/local-agent/tools.ts`: `read_file`, + `write_file`, `replace_text`, `list_dir`, `search_text`, `run_command`, + with their JSON schemas. `run_command` runs through the shell in `cwd` + and ends a command at `commandTimeoutSeconds` through + `terminateProcessTree`. Test in `tools.test.ts`: each tool, the output + cap, the time limit (a 60 s command ended at 1 s), a refused path, an + unknown tool and a missing argument. 7 passed. +- [x] 2.4 `packages/core/src/agents/local-agent/chat-client.ts`: one + `/v1/chat/completions` request with tools, reading `tool_calls`, + `"tool_calls": null`, `"usage": null` and null arguments as none, then + `toolCallsInText`. Test in `chat-client.test.ts` against a local HTTP + stand-in. 3 passed. +- [x] 2.5 `packages/core/src/agents/local-agent/agent-loop.ts`: the loop + bounded by `LocalLlmAcpLimits`, giving progress events (text, tool call, + tool result, usage) and a stop reason. Test in `agent-loop.test.ts`: a + turn that writes a file, the iteration limit, the tool-call limit, a + token limit. 4 passed. +- [x] 2.6 `packages/core/src/agents/local-agent/acp-agent.ts`: an + `AgentApp` serving `initialize`, `session/new` (the session's `cwd`), + `session/prompt` (updates `agent_message_chunk`, `tool_call`, + `tool_call_update`; response `stopReason` and `usage`) and + `session/cancel`; the run's abort signal ends the loop and kills a + running command. With `askBeforeCommands`, `run_command` sends + `session/request_permission`. Tested in `local-llm-acp.test.ts` through + `AcpSessionDriver.run()`, not around it: the events a run produces, the + model named in the first update, a call written as text, a refused path, + a permission allowed and one denied, the iteration limit. 8 passed. +- [x] 2.7 `packages/core/src/agents/local-llm-acp.ts`: + `LocalLlmAcpAdapter.buildInvocation` returns + `{ kind: "in-process", agent: "local-llm-acp" }`, and `execute` runs the + agent of 2.6 through `driver.run`. No process is started. + `local-llm-acp.test.ts` asserts it. +- [x] 2.8 `packages/core/src/agent-runner.ts` and `security.ts`: + `AdapterInvocation` gains `in-process`. `checkAllowlist` admits it only + through an `IN_PROCESS_SENTINEL` rule naming the agent itself, and the + audit records the invocation as it records any other. Tested in + `default-runners.test.ts` ("admits local-llm-acp only as itself"): in + process accepted for `local-llm-acp`, refused when it names another + agent, refused for `claude-cli`, and a `coding-agent` process refused. +- [x] 2.9 `packages/core/src/default-runners.ts`: the `coding-agent` + allowlist entry and `LOCAL_LLM_ACP_DEFAULT_EXECUTABLE` removed; + `default-runners.test.ts` updated. Agent detection + (`agent-detection.ts`) reports `local-llm-acp` present where the local + LLM's server answers, as it does `local-llm`. + +## 3. The model + +- [x] 3.1 `packages/core/src/agents/registry.ts`: + `AgentDescriptor.takesModel` and `acceptsModel(descriptor)`, true where + `modelFlag` is set and for `local-llm` and `local-llm-acp`. +- [x] 3.2 `packages/core/src/harness-config.ts` and + `harness-config-schema.ts` decide acceptance by `acceptsModel`, not + `modelFlag`. `MODEL_ID_PATTERN` admits `/`, since a local server names + models as Hugging Face does. `harness-config.test.ts`: a model with a + slash accepted for both local agents, still refused for `gemini-cli`. +- [x] 3.3 `packages/core/src/local-llm-settings.ts`: + `resolveLocalLlmModel(stageModel, settings, fetch)` (stage, then + settings, then `/v1/models` cached per base URL, then `default`), + returning the name and its source; `resolveLocalLlmSettings` no longer + defaults the model. `local-llm-settings.test.ts`: each source, the first + of several served, one request per server, an unreachable server asked + again next time. 17 passed. +- [x] 3.4 `packages/core/src/agents/local-llm.ts` and the agent of 2.6 use + 3.3, and the first output of a run names the model and its source + (`local-llm.test.ts`, `local-llm-acp.test.ts`). +- [x] 3.5 `packages/webui/src/components/harness-settings-parts.tsx` + offers the model field to agents with `takesModel`. Test in + `harness-settings-parts.test.tsx`: both local agents, `claude-cli` yes, + `gemini-cli` no. +- [x] 3.6 `packages/extension/schemas/agent-harness.schema.json` and + `change-harness.schema.json`, regenerated with + `npm run schemas --workspace packages/extension`, accept a model for + both local agents. + +## 4. The system proxy + +- [x] 4.1 `packages/core/src/direct-fetch.ts`: `localFetch(ignoreSystemProxy)` + returns `undici`'s `fetch` with an `Agent` of its own when asked, and the + process's `fetch` otherwise. `undici` 7 (Node 20.18+, within the pinned + 22.11; 8 needs 22.19). Test in `direct-fetch.test.ts`: with a dead proxy + installed as the process's global dispatcher, as an editor host can, the + process's `fetch` fails and `localFetch(true)` reaches the server. +- [x] 4.2 `packages/core/src/agents/shared.ts` and + `acp-session-driver.ts`: `agentSpawnEnvironment` gives a CLI agent an + environment without `HTTP_PROXY`, `HTTPS_PROXY` and `ALL_PROXY` in either + case and with `NO_PROXY=*` (`withoutSystemProxy`), in `spawnAndStream` + and `spawnAcpProcess`. Test in `proxy-policy.test.ts`: the variables + removed, and the environment that reaches the spawn. +- [x] 4.3 `packages/core/src/default-runners.ts`: + `DefaultRunnersConfig.ignoreSystemProxy` and `askBeforeCommands` reach + the adapters and the CLI spawn policy, and `agent-detection.ts` checks + the local LLM with `localFetch` under the same policy. Test in + `proxy-policy.test.ts`. +- [x] 4.4 `packages/core/src/harness-step-agent.ts`: + `HARNESS_AGENT_CAPABILITIES[*].systemProxy` for every registry id and + `vscode-chat` (`ignored`, `environment` or `unknown`), shown as the table + in `HARNESS.md`, "Ignoring the system proxy". No configuration finding: + the switch is the host's, not the harness file's, so + `findHarnessConfigLimits`, which reads only the harness file, cannot see + it. +- [x] 4.5 `packages/extension/package.json` and `extension.ts`: the + settings `openspec-ui.agents.ignoreSystemProxy` and + `openspec-ui.localLlm.agent.askBeforeCommands`, read by + `readAgentSwitches()` and passed to `buildDefaultAgentRunners`, and to + the optional local server's runners (`optional-server.ts`), so the two + agree. +- [x] 4.6 `packages/server` and `packages/cli`: no code of their own. + `buildDefaultAgentRunners` reads `OPENSPEC_UI_IGNORE_SYSTEM_PROXY` and + `OPENSPEC_UI_LOCAL_LLM_ASK_BEFORE_COMMANDS` where a host passes neither, + which both do. Test in `proxy-policy.test.ts`: the environment variable + applies where the host says nothing, and a host's own value wins. + +## 5. Documents + +- [x] 5.1 `HARNESS.md`: the two local agents' rows (the model optional, + `local-llm-acp` built in), "The local LLM" (the model order, the agent's + tools and sandbox, asking before commands, calls written as text), and a + new "Ignoring the system proxy" with each agent's `systemProxy`. +- [x] 5.2 `LIMITS.md`: the `LOCAL_LLM_ACP_*` limits as the in-process + loop's, with their defaults and the context limits that act on nothing; + `local-llm-acp` in the usage and context-gauge tables. +- [x] 5.3 `README.md`: the agent table's row for `local-llm-acp` says it + needs nothing installed. +- [x] 5.4 A changeset: core, webui, the server, the CLI and the extension, + minor. + +## 6. Checks + +- [x] 6.1 `npm run typecheck && npm run lint && npm run test` at the root, + after `git add`, unpiped, exit code 0. 2026-10-02: typecheck 0, lint 0, + test 0; the script tests fail 0, and the workspaces 21/192, 143/2010 + (core), 7/62, 36/498, 4/118, 76/687 files/tests passed. +- [x] 6.2 `openspec validate local-llm-codes-in-process --strict`, and the + merge gate locally with `--base origin/main`. 2026-10-02: "Change + 'local-llm-codes-in-process' is valid", exit 0. The gate, run with the + worktree's absolute path as `--cwd` (`--cwd .` resolves against + `packages/cli` under `npm run --workspace`, and checked nothing). Owed + only 6.4 as of that run; 6.4 is now closed (below), and 6.5/6.6 were + found and fixed after it, with their own checks recorded there. +- [x] 6.3 **Delegated to local-llm-acp**: an `implement` run through + `buildDefaultAgentRunners(...).get("local-llm-acp")`, with no model + named anywhere and `ignoreSystemProxy` on while `HTTPS_PROXY` points at + the system proxy, against SGLang on the LAN, on a scratch change with + open tasks. Record the run id, the model and its source from the first + update, the tool calls, the files written, the change's checks, and + that `coding-agent` is not on the PATH. + Record, 2026-10-02: run `live-in-process-1790906039976`, from this + branch's `packages/core`, with `HTTP_PROXY` and `HTTPS_PROXY` set to the + system proxy `http://127.0.0.1:2080` (which resets requests to the LAN), + `NO_PROXY` without the server's address, `OPENSPEC_UI_LOCAL_LLM_MODEL` + unset, the base URL `http://192.168.137.33:8000/v1` and the key from the + environment, and `coding-agent` not on the PATH. First update: "Model + QuantTrio/Qwen3.6-35B-A3B-AWQ (the model the server serves)." 56 s from + `started` to `completed`; 24 tool calls streamed with their results + (list_dir, read_file, replace_text, write_file, run_command), one of + them a `read_file` of a wrong path that failed and was retried; + `usageReported` inputTokens 72100, outputTokens 4101. The scratch change + `farewell-takes-a-name`: `src/farewell.mjs` now takes a name, falling + back to "Goodbye, world" for none or a blank one; `src/farewell.test.mjs` + added; `node --test src/farewell.test.mjs` passes 4 of 4; both tasks + ticked; `git status` shows only those two files and the change's + `tasks.md`. Task 1.2's own command, `node --test src/`, does not run on + Node 24 with a directory argument; the agent found that, tried other + forms and verified with the file. The task's wording was at fault, not + the agent. +- [x] 6.4 **Human-only**: in the Extension Development Host built from + this branch, with `openspec-ui.agents.ignoreSystemProxy` on and the + model left empty, run one stage on `local-llm-acp` from the picker: the + run edits files and its updates are shown. Then turn + `openspec-ui.localLlm.agent.askBeforeCommands` on and see a command + wait for Allow. + Record, 2026-10-02, confirmed by the owner: built and run in a real + Extension Development Host against a scratch change + `verify-local-llm-acp` (two open tasks: write `src/greet.mjs` and its + test), with `ignoreSystemProxy` on and the model left empty throughout. + Run `9a9335fb-b4cf-4514-b40a-872c48d531d2`: the dialog read "apply + (implements the tasks): local-llm-acp" / "Sets no model"; the agent + named the model itself ("Model QuantTrio/Qwen3.6-35B-A3B-AWQ, the model + the server serves"), wrote `src/greet.mjs` and `src/greet.test.mjs`, ran + `node --test` and ticked both tasks; outcome `completed`. With + `askBeforeCommands` on (a third task added, needing a command): run + `c37ff3bf-d141-40db-9241-eff8832f7f29` reached + `run_command node --test src/greet.test.mjs` and the panel showed + "Permission requested: run_command node --test src/greet.test.mjs" with + Allow/Deny — screenshotted. Allow unblocked the same command, which + passed, and the agent ticked the task; outcome `completed`. The first + attempt at the Allow half, before 6.6 below, sat on "Loading…" forever + after Allow — that is what 6.6 found and fixed; this record is of the + run taken after that fix. +- [x] 6.6 Found while attempting 6.4's `askBeforeCommands` half: clicking + Allow removed the permission prompt but the run never continued — no + child process, no further tool call, `tasks.md` never ticked past the + task that asked, and no `end` event ever reached the run's log. Root + cause in `packages/extension/src/webview/ai-panel.ts`'s `dispatchOrRun`: + a `resolvePermission` command carries no `agentId` (the + `permissionRequest` event it answers never named one), so for a run not + wrapped in a chain it resolved through `DEFAULT_AGENT_ID` to a *fresh* + `AgentRunner` instance — not the one the original command's `agentId` + built and recorded in `runAgentIds` (the lookup `"cancel"` already gets + for the same reason, `a-change-is-run-from-its-card`'s + "A cancel goes to the runner that owns the run"). The fresh instance's + `resolvePermission()` had never heard of this run's pending request, so + it silently did nothing; the real adapter's `session/request_permission` + promise never settled. Fix: `resolvePermission` now shares the same + `runAgentIds` lookup `"cancel"` already had. Test in `ai-panel.test.ts`: + "routes a resolvePermission to the runner that owns the run, not to the + default agent" — sends an `implement` naming `local-llm-acp`, then a + `resolvePermission` with no `agentId`, and asserts `resolveRunner` is + called with `"local-llm-acp"`, not the default. Reproduced and fixed + live in the Extension Development Host (see 6.4's record): before the + fix, Allow never unblocked the run; after it, the same scenario + completed in 13 s. `npm run typecheck` at the root: exit 0. + `packages/extension`: `npx vitest run` 36 files, 498 tests, 0 failed + (includes the new test and the three pre-existing ones this fix makes + pass again: "falls back to the generic single-stage path when a + resolvePermission's runId is not an active chain", "never registers a + Processes entry for a resolvePermission command", "forwards a + resolvePermission command straight to the resolved AgentRunner, exactly + like plan/review/implement" — all three already encoded the intended + behavior and had nothing wrong with them). +- [x] 6.5 Found while attempting 6.4: every run that failed inside an ACP + connection (any ACP agent, not only `local-llm-acp` — the shared + `AcpSessionDriver`) reported only "Internal error", whatever actually + failed. The Agent Client Protocol SDK wraps any handler exception into + a `RequestError` whose `message` is the fixed string "Internal error" + (JSON-RPC code -32603), keeping the real cause only in `data.details`. + `packages/core/src/agents/acp-session-driver.ts`'s `connectionFailureReason` + reads `data.details` where present. Reproduced before the fix with a + direct call to `LocalLlmAcpAdapter.execute` against the real LAN server + with a wrong key: `reason` was `"Internal error"`; after the fix, the + same call reports `reason: "HTTP 401 Unauthorized: {\"error\":\"Unauthorized\"}"`. + Test in `acp-session-driver.test.ts`: a handler that throws reports its + own message, not "Internal error". `npm run typecheck && npm run lint + && npm run test` at the root, unpiped: exit 0 (core 18/18 new+existing + in the two directly affected files; server 118/118; webui 687/687; no + new failures anywhere). diff --git a/package-lock.json b/package-lock.json index f4802898..8037ecf9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1659,21 +1659,6 @@ "node": "^22.20 || ^24.12 || >=25" } }, - "node_modules/@noble/hashes": { - "version": "1.8.0", - "resolved": "https://registry.npmjs.org/@noble/hashes/-/hashes-1.8.0.tgz", - "integrity": "sha512-jCs9ldd7NwzpgXDIf6P3+NrHh9/sD6CQdxHyjQI+h/6rDNo88ypBxxz45UDuZHz9r3tNz7N/VInSVoVdtXEI4A==", - "dev": true, - "license": "MIT", - "optional": true, - "peer": true, - "engines": { - "node": "^14.21.3 || >=16" - }, - "funding": { - "url": "https://paulmillr.com/funding/" - } - }, "node_modules/@nodelib/fs.scandir": { "version": "2.1.5", "resolved": "https://registry.npmjs.org/@nodelib/fs.scandir/-/fs.scandir-2.1.5.tgz", @@ -11814,6 +11799,7 @@ "@agentclientprotocol/sdk": "^1.5.1", "cross-spawn": "^7.0.6", "simple-git": "^3.27.0", + "undici": "^7.30.0", "yaml": "^2.9.1", "zod": "^4.6.5" }, @@ -11822,6 +11808,15 @@ "@types/node": "^26.6.3" } }, + "packages/core/node_modules/undici": { + "version": "7.30.0", + "resolved": "https://registry.npmjs.org/undici/-/undici-7.30.0.tgz", + "integrity": "sha512-dkrQXeHSaoamnItlYbmzG0wFYrM0ZwDxCIg0A7aKjTyyhh9svRzCNFEzV+Vm05/yehjCzjDZ31KXfGEjYSztDQ==", + "license": "MIT", + "engines": { + "node": ">=20.18.1" + } + }, "packages/extension": { "name": "openspec-ui-vscode", "version": "0.90.1", diff --git a/packages/core/package.json b/packages/core/package.json index f57e5f56..7af2297b 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -15,14 +15,15 @@ "test": "vitest run --project core && vitest run --project core-git-subprocess" }, "dependencies": { - "simple-git": "^3.27.0", - "cross-spawn": "^7.0.6", "@agentclientprotocol/sdk": "^1.5.1", - "zod": "^4.6.5", - "yaml": "^2.9.1" + "cross-spawn": "^7.0.6", + "simple-git": "^3.27.0", + "undici": "^7.30.0", + "yaml": "^2.9.1", + "zod": "^4.6.5" }, "devDependencies": { - "@types/node": "^26.6.3", - "@types/cross-spawn": "^6.0.6" + "@types/cross-spawn": "^6.0.6", + "@types/node": "^26.6.3" } } diff --git a/packages/core/src/agent-detection.ts b/packages/core/src/agent-detection.ts index f78b096e..9cc13ee5 100644 --- a/packages/core/src/agent-detection.ts +++ b/packages/core/src/agent-detection.ts @@ -9,7 +9,10 @@ import crossSpawn from "cross-spawn"; import { buildDefaultAllowlist } from "./default-runners.js"; +import { localFetch, type FetchLike } from "./direct-fetch.js"; import { localLlmHeaders, resolveLocalLlmSettings, type LocalLlmSettings } from "./local-llm-settings.js"; +import { IN_PROCESS_SENTINEL } from "./security.js"; +import { agentsIgnoreSystemProxy } from "./agents/shared.js"; /** Presence plus a best-effort version — see design.md, "Detection * reports a version; it does not gate on one". `version` is absent when @@ -46,6 +49,9 @@ const HTTP_TIMEOUT_MS = 1500; export interface AgentDetectionConfig { localLlmBaseUrl?: string; localLlmApiKey?: string; + /** Check the local LLM directly, ignoring the system proxy, as its + * agents will reach it (local-llm-codes-in-process). */ + ignoreSystemProxy?: boolean; } /** Spawns ` --version` exactly once (ADR 0017 decision 6 — no @@ -108,11 +114,11 @@ function detectCliAgent(executable: string): Promise { }); } -async function detectLocalLlm(settings: LocalLlmSettings): Promise { +async function detectLocalLlm(settings: LocalLlmSettings, fetchImpl: FetchLike): Promise { try { // Any answer at all is a server there; the key goes with the question // so a server that wants one does not hang up (the-local-llm-is-where-you-say). - await fetch(settings.baseUrl, { headers: localLlmHeaders(settings), signal: AbortSignal.timeout(HTTP_TIMEOUT_MS) }); + await fetchImpl(settings.baseUrl, { headers: localLlmHeaders(settings), signal: AbortSignal.timeout(HTTP_TIMEOUT_MS) }); return { detected: true }; } catch { return { detected: false }; @@ -132,11 +138,13 @@ export async function detectAvailableAgentsDetailed( Object.entries(allowlist).map(async ([id, rules]) => { const executable = rules[0]?.executable; if (!executable) return [id, { detected: false }] as const; - if (executable === HTTP_SENTINEL) { + // The local LLM, over HTTP or as the agent that runs in the product: + // present where its server answers (ADR 0038). + if (executable === HTTP_SENTINEL || executable === IN_PROCESS_SENTINEL) { return [id, await detectLocalLlm(resolveLocalLlmSettings({ ...(config.localLlmBaseUrl !== undefined ? { baseUrl: config.localLlmBaseUrl } : {}), ...(config.localLlmApiKey !== undefined ? { apiKey: config.localLlmApiKey } : {}), - }))] as const; + }), localFetch(config.ignoreSystemProxy ?? agentsIgnoreSystemProxy()))] as const; } return [id, await detectCliAgent(executable)] as const; }), diff --git a/packages/core/src/agent-runner.ts b/packages/core/src/agent-runner.ts index b653e7ba..ec5708a2 100644 --- a/packages/core/src/agent-runner.ts +++ b/packages/core/src/agent-runner.ts @@ -24,7 +24,11 @@ import { untilStopBoundary } from "./stop-boundary.js"; export type AdapterInvocation = | { kind: "process"; executable: string; args: string[] } - | { kind: "http"; url: string; method: string }; + | { kind: "http"; url: string; method: string } + /** An agent that runs inside the product and starts no process + * (local-llm-codes-in-process, ADR 0038). The allowlist admits it only + * for an agent registered as in-process, and the audit records it so. */ + | { kind: "in-process"; agent: string }; export interface AgentAdapter { /** The agent's name, as it appears in the workspace's allowlist config. */ diff --git a/packages/core/src/agents/acp-session-driver.test.ts b/packages/core/src/agents/acp-session-driver.test.ts index 9f3c3e91..250230af 100644 --- a/packages/core/src/agents/acp-session-driver.test.ts +++ b/packages/core/src/agents/acp-session-driver.test.ts @@ -294,4 +294,29 @@ describe("AcpSessionDriver — reporting what the agent said it spent", () => { { inputTokens: 7, outputTokens: 2, cost: { amount: 3, currency: "EUR" } }, ]); }); + + it("a failed event carries the handler's own message, not the SDK's generic 'Internal error'", async () => { + // The SDK's own catch-all wraps any handler exception into a + // `RequestError` whose `message` is the fixed string "Internal error" + // (JSON-RPC code -32603), keeping the real message only in + // `data.details`. A run has to say what actually failed, not that + // code. + const mockAgent = agent({ name: "mock-agent" }) + .onRequest(AGENT_METHODS.initialize, () => ({ protocolVersion: PROTOCOL_VERSION })) + .onRequest(AGENT_METHODS.session_new, () => ({ sessionId: "session-1" })) + .onRequest(AGENT_METHODS.session_prompt, async () => { + throw new Error("HTTP 401 Unauthorized: {\"error\":\"Unauthorized\"}"); + }); + + const driver = new AcpSessionDriver(); + const events = await collect( + driver.run({ target: mockAgent, cwd: "/tmp/work", runId: "run-e1", commandKind: "implement", prompt: "do it" }), + ); + + const failed = events.find((event) => event.kind === "failed"); + expect(failed).toBeDefined(); + if (failed?.kind === "failed") { + expect(failed.reason).toBe("HTTP 401 Unauthorized: {\"error\":\"Unauthorized\"}"); + } + }); }); diff --git a/packages/core/src/agents/acp-session-driver.ts b/packages/core/src/agents/acp-session-driver.ts index 2747dbae..d514ff3c 100644 --- a/packages/core/src/agents/acp-session-driver.ts +++ b/packages/core/src/agents/acp-session-driver.ts @@ -20,6 +20,7 @@ import { Readable, Writable } from "node:stream"; import { CLIENT_METHODS, PROTOCOL_VERSION, + RequestError, client, ndJsonStream, type AgentApp, @@ -29,12 +30,32 @@ import { } from "@agentclientprotocol/sdk"; import type { AgentUsage } from "../agent-usage.js"; import type { CommandKind, Event } from "../protocol.js"; -import { KILL_CONFIRMATION_TIMEOUT_MS, terminateProcessTree } from "./shared.js"; +import { agentSpawnEnvironment, KILL_CONFIRMATION_TIMEOUT_MS, terminateProcessTree } from "./shared.js"; function nowIso(): string { return new Date().toISOString(); } +/** A connection failure's reason, for a run's `failed` event. + * + * The SDK's own catch-all (`errorToResult`) turns any handler exception + * into a `RequestError` whose `message` is the fixed string "Internal + * error" — code -32603 always reads that, by the JSON-RPC spec — with + * the original exception's own message kept only in `data.details` (an + * `HTTP 401 Unauthorized: ...` from `chat-client.ts`, say). Reading only + * `.message`, as this driver did, reported every one of those as the same + * unhelpful "Internal error" no matter what actually failed underneath. */ +function connectionFailureReason(error: unknown): string { + if (error instanceof RequestError) { + const data = error.data; + if (typeof data === "object" && data !== null && "details" in data && typeof (data as { details: unknown }).details === "string") { + return (data as { details: string }).details; + } + return error.message; + } + return error instanceof Error ? error.message : String(error); +} + /** Either a live subprocess's stdio (production, via `spawnAcpProcess`) or * an in-process `AgentApp` (tests) — see this file's header comment. */ export type AcpConnectTarget = Stream | AgentApp; @@ -51,10 +72,11 @@ export function spawnAcpProcess( cwd: string, env?: Readonly>, ): { target: Stream; child: ChildProcessWithoutNullStreams } { + const spawnEnv = agentSpawnEnvironment(env); const child = crossSpawn(executable, args, { cwd, stdio: ["pipe", "pipe", "pipe"], - ...(env !== undefined ? { env: { ...process.env, ...env } } : {}), + ...(spawnEnv !== undefined ? { env: spawnEnv } : {}), ...(process.platform !== "win32" ? { detached: true } : {}), }) as ChildProcessWithoutNullStreams; // ACP only speaks over stdin/stdout — stderr is not part of the ACP @@ -249,7 +271,7 @@ export class AcpSessionDriver { // goes down after a compaction, so recording it as consumption // would under-count a long run exactly when it compacts. let streamedCost: { amount: number; currency: string } | undefined; - for (;;) { + for (; ;) { const message = await session.nextUpdate(); if (message.kind === "stop") break; const update = message.notification.update as unknown as Record; @@ -314,7 +336,7 @@ export class AcpSessionDriver { kind: "failed", runId, timestamp: nowIso(), - reason: item.error instanceof Error ? item.error.message : String(item.error), + reason: connectionFailureReason(item.error), }; } else { yield item; diff --git a/packages/core/src/agents/local-agent/acp-agent.ts b/packages/core/src/agents/local-agent/acp-agent.ts new file mode 100644 index 00000000..28975db4 --- /dev/null +++ b/packages/core/src/agents/local-agent/acp-agent.ts @@ -0,0 +1,185 @@ +// The local agent as an Agent Client Protocol agent, run in process +// (local-llm-codes-in-process, ADR 0038 decision 1). +// +// `AcpSessionDriver.run()` connects to it as to any ACP agent, so a run +// reaches the same `agentUpdate`, `permissionRequest` and `usageReported` +// events, the same stop handling and the same audit as an ACP CLI. One app +// is built per run, with that run's abort signal: cancelling the run ends +// the loop at its next step and kills a command it is running. + +import { AGENT_METHODS, CLIENT_METHODS, PROTOCOL_VERSION, agent, type AgentApp } from "@agentclientprotocol/sdk"; +import type { FetchLike } from "../../direct-fetch.js"; +import { + describeLocalLlmModel, + resolveLocalLlmModel, + type LocalLlmAcpLimits, + type LocalLlmSettings, +} from "../../local-llm-settings.js"; +import { runAgentLoop, type LoopEvent, type StopReason } from "./agent-loop.js"; +import type { ToolCall } from "./text-tool-calls.js"; +import { TOOL_PARAMETER_TYPES, TOOL_SCHEMAS } from "./tools.js"; + +export interface LocalAgentOptions { + settings: LocalLlmSettings; + /** The stage's model, where it names one. */ + stageModel?: string; + limits: LocalLlmAcpLimits; + fetch: FetchLike; + /** Ask through a permission request before each command. */ + askBeforeCommands: boolean; + /** The run's signal: aborting it ends the loop. */ + signal: AbortSignal; +} + +const TOOL_KINDS: Record = { + read_file: "read", + list_dir: "read", + write_file: "edit", + replace_text: "edit", + search_text: "search", + run_command: "execute", +}; + +/** The protocol's stop reason for each way the loop ends. */ +const STOP_REASONS: Record = { + completed: "end_turn", + cancelled: "cancelled", + max_iterations: "max_turn_requests", + max_tool_calls: "max_turn_requests", + max_seconds: "max_turn_requests", + max_prompt_tokens: "max_tokens", + max_completion_tokens: "max_tokens", + max_total_tokens: "max_tokens", +}; + +const TOOL_OUTPUT_SHOWN = 4000; + +function toolTitle(call: ToolCall): string { + for (const key of ["path", "command", "query"]) { + const value = call.arguments[key]; + if (typeof value === "string" && value.length > 0) return `${call.name} ${value.length > 80 ? `${value.slice(0, 77)}...` : value}`; + } + return call.name; +} + +/** The text of a prompt's content blocks: text blocks, and the text of an + * embedded text resource. Images and audio are not readable by a text + * model and are left out. */ +function promptText(blocks: ReadonlyArray): string { + return blocks + .map((block) => { + if (typeof block !== "object" || block === null) return ""; + const record = block as { type?: unknown; text?: unknown; resource?: { text?: unknown } }; + if (record.type === "text" && typeof record.text === "string") return record.text; + if (record.type === "resource" && typeof record.resource?.text === "string") return record.resource.text; + return ""; + }) + .filter((part) => part.length > 0) + .join("\n\n"); +} + +/** Builds the agent for one run. */ +export function createLocalAgent(options: LocalAgentOptions): AgentApp { + const sessions = new Map(); + let sessionCount = 0; + + return agent({ name: "openspec-workbench-local-agent" }) + .onRequest(AGENT_METHODS.initialize, () => ({ + protocolVersion: PROTOCOL_VERSION, + agentCapabilities: { loadSession: false, promptCapabilities: { image: false, audio: false, embeddedContext: true } }, + authMethods: [], + })) + .onRequest(AGENT_METHODS.session_new, ({ params }) => { + sessionCount += 1; + const sessionId = `local-${sessionCount}`; + sessions.set(sessionId, { cwd: params.cwd }); + return { sessionId }; + }) + .onNotification(AGENT_METHODS.session_cancel, () => { + // The run's own signal is what ends the loop; the driver aborts it + // when it is asked to cancel. + }) + .onRequest(AGENT_METHODS.session_prompt, async ({ params, client }) => { + const session = sessions.get(params.sessionId); + if (session === undefined) throw new Error(`Unknown session ${params.sessionId}`); + const say = (text: string) => + client.notify(CLIENT_METHODS.session_update, { + sessionId: params.sessionId, + update: { sessionUpdate: "agent_message_chunk", content: { type: "text", text } }, + }); + + const model = await resolveLocalLlmModel(options.stageModel, options.settings, options.fetch); + await say(`${describeLocalLlmModel(model)}\n\n`); + + const onEvent = async (event: LoopEvent) => { + if (event.type === "text") { + await say(event.text); + } else if (event.type === "tool_call") { + await client.notify(CLIENT_METHODS.session_update, { + sessionId: params.sessionId, + update: { + sessionUpdate: "tool_call", + toolCallId: event.call.id, + title: toolTitle(event.call), + kind: TOOL_KINDS[event.call.name] ?? "other", + status: "in_progress", + rawInput: event.call.arguments, + }, + }); + } else if (event.type === "tool_result") { + const output = event.result.output.length > TOOL_OUTPUT_SHOWN + ? `${event.result.output.slice(0, TOOL_OUTPUT_SHOWN)}\n... (${event.result.output.length - TOOL_OUTPUT_SHOWN} more characters)` + : event.result.output; + await client.notify(CLIENT_METHODS.session_update, { + sessionId: params.sessionId, + update: { + sessionUpdate: "tool_call_update", + toolCallId: event.call.id, + status: event.result.failed ? "failed" : "completed", + content: [{ type: "content", content: { type: "text", text: output } }], + }, + }); + } + }; + + const allowCommand = options.askBeforeCommands + ? async (command: string): Promise => { + const answer = await client.request(CLIENT_METHODS.session_request_permission, { + sessionId: params.sessionId, + toolCall: { toolCallId: `permission-${Date.now()}`, title: `run_command ${command}`, kind: "execute", rawInput: { command } }, + options: [ + { optionId: "allow", name: "Allow", kind: "allow_once" }, + { optionId: "deny", name: "Deny", kind: "reject_once" }, + ], + }); + return answer.outcome.outcome === "selected" && answer.outcome.optionId === "allow"; + } + : undefined; + + const result = await runAgentLoop(promptText(params.prompt), { + chat: { + settings: options.settings, + model: model.model, + fetch: options.fetch, + tools: TOOL_SCHEMAS, + parameterTypes: TOOL_PARAMETER_TYPES, + ...(options.limits.maxCompletionTokens !== undefined ? { maxCompletionTokens: options.limits.maxCompletionTokens } : {}), + }, + cwd: session.cwd, + limits: options.limits, + onEvent, + ...(allowCommand !== undefined ? { allowCommand } : {}), + signal: options.signal, + }); + + // A stop the model did not write is still said to the person. + if (result.stopReason !== "completed") await say(`\n\n${result.message}`); + return { + stopReason: STOP_REASONS[result.stopReason], + ...(result.usage.reported + ? { usage: { inputTokens: result.usage.promptTokens, outputTokens: result.usage.completionTokens, totalTokens: result.usage.totalTokens } } + : {}), + _meta: { localAgent: { stoppedReason: result.stopReason, model: model.model, modelSource: model.source } }, + }; + }); +} diff --git a/packages/core/src/agents/local-agent/agent-loop.test.ts b/packages/core/src/agents/local-agent/agent-loop.test.ts new file mode 100644 index 00000000..aeed8eca --- /dev/null +++ b/packages/core/src/agents/local-agent/agent-loop.test.ts @@ -0,0 +1,65 @@ +import { mkdtemp, readFile, rm } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { FetchLike } from "../../direct-fetch.js"; +import { runAgentLoop, type LoopEvent } from "./agent-loop.js"; +import { TOOL_PARAMETER_TYPES, TOOL_SCHEMAS } from "./tools.js"; + +// local-llm-codes-in-process 2.5. +// Temporary directories: the file took 206 ms alone on 2026-10-02; the ceiling is the +// one the other local-agent tests use, for a loaded machine. +vi.setConfig({ testTimeout: 20_000 }); +const roots: string[] = []; +afterEach(async () => { + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))); +}); + +function model(answers: Array>, usage = { prompt_tokens: 100, completion_tokens: 10, total_tokens: 110 }): FetchLike { + let turn = 0; + return async () => { + const message = answers[Math.min(turn, answers.length - 1)]; + turn += 1; + return new Response(JSON.stringify({ choices: [{ message }], usage }), { status: 200 }); + }; +} + +const writeCall = { content: "", tool_calls: [{ id: "c1", type: "function", function: { name: "write_file", arguments: '{"path":"a.txt","content":"x"}' } }] }; +const listCall = { content: "", tool_calls: [{ id: "c1", type: "function", function: { name: "list_dir", arguments: "{}" } }] }; + +async function run(fetchImpl: FetchLike, limits = {}) { + const cwd = await mkdtemp(path.join(os.tmpdir(), "openspec-loop-")); + roots.push(cwd); + const events: LoopEvent[] = []; + const result = await runAgentLoop("go", { + chat: { settings: { baseUrl: "http://x" }, model: "m", fetch: fetchImpl, tools: TOOL_SCHEMAS, parameterTypes: TOOL_PARAMETER_TYPES }, + cwd, + limits, + onEvent: (event) => { events.push(event); }, + }); + return { cwd, events, result }; +} + +describe("runAgentLoop", () => { + it("runs a turn that writes a file, and ends when the model answers without a call", async () => { + const { cwd, events, result } = await run(model([writeCall, { content: "Done." }])); + expect(result).toMatchObject({ stopReason: "completed", message: "Done.", usage: { promptTokens: 200, completionTokens: 20, totalTokens: 220, reported: true } }); + expect(events.map((event) => event.type)).toEqual(["usage", "tool_call", "tool_result", "usage", "text"]); + expect(await readFile(path.join(cwd, "a.txt"), "utf8")).toBe("x"); + }); + + it("stops at the iteration limit", async () => { + const { result } = await run(model([listCall]), { maxIterations: 3 }); + expect(result).toMatchObject({ stopReason: "max_iterations", message: "Stopped at the 3-iteration limit." }); + }); + + it("stops at the tool-call limit", async () => { + const { result } = await run(model([listCall]), { maxToolCalls: 2 }); + expect(result.stopReason).toBe("max_tool_calls"); + }); + + it("stops at a token limit", async () => { + const { result } = await run(model([listCall]), { maxTotalTokens: 300 }); + expect(result.stopReason).toBe("max_total_tokens"); + }); +}); diff --git a/packages/core/src/agents/local-agent/agent-loop.ts b/packages/core/src/agents/local-agent/agent-loop.ts new file mode 100644 index 00000000..90767625 --- /dev/null +++ b/packages/core/src/agents/local-agent/agent-loop.ts @@ -0,0 +1,127 @@ +// The local agent's loop (local-llm-codes-in-process, ADR 0038 decision 1): +// ask the model, run the tools it calls, give it their results, until it +// answers without a call or a limit stops it. Ported from `coding-agent`'s +// `CodingAgent.run_prompt`. + +import type { LocalLlmAcpLimits } from "../../local-llm-settings.js"; +import { completeTurn, type ChatClientOptions, type ChatMessage, type TurnUsage } from "./chat-client.js"; +import type { ToolCall } from "./text-tool-calls.js"; +import { DEFAULT_TOOL_LIMITS, runTool, type ToolResult } from "./tools.js"; + +export const SYSTEM_PROMPT = + "You are a coding agent working in a repository. Use the tools to read, change and verify files; " + + "every path is relative to the working directory, and nothing outside it can be reached. " + + "Run the commands that check your work. When the task is done, or cannot be done, end your turn with a " + + "concise summary of what you did and what is left."; + +const DEFAULT_MAX_ITERATIONS = 40; +const DEFAULT_MAX_TOOL_CALLS = 120; +const DEFAULT_MAX_SECONDS = 1800; + +/** Why the loop ended: `completed`, or the limit that ended it. */ +export type StopReason = + | "completed" + | "cancelled" + | "max_iterations" + | "max_tool_calls" + | "max_seconds" + | "max_prompt_tokens" + | "max_completion_tokens" + | "max_total_tokens"; + +export type LoopEvent = + | { type: "text"; text: string } + | { type: "tool_call"; call: ToolCall } + | { type: "tool_result"; call: ToolCall; result: ToolResult } + | { type: "usage"; usage: TurnUsage }; + +export interface LoopOptions { + chat: ChatClientOptions; + cwd: string; + limits: LocalLlmAcpLimits; + onEvent: (event: LoopEvent) => Promise | void; + /** Asked before `run_command` where the host said to ask; false means + * the command is not run, and the model is told so. */ + allowCommand?: (command: string) => Promise; + signal?: AbortSignal; + /** A clock, for tests. */ + now?: () => number; +} + +export interface LoopResult { + stopReason: StopReason; + message: string; + usage: Required & { reported: boolean }; +} + +function exceeded(limit: number | undefined, value: number): boolean { + return limit !== undefined && value > limit; +} + +/** Runs one prompt to its end. */ +export async function runAgentLoop(prompt: string, options: LoopOptions): Promise { + const now = options.now ?? Date.now; + const started = now(); + const limits = options.limits; + const maxIterations = limits.maxIterations ?? DEFAULT_MAX_ITERATIONS; + const maxToolCalls = limits.maxToolCalls ?? DEFAULT_MAX_TOOL_CALLS; + const maxSeconds = limits.maxSeconds ?? DEFAULT_MAX_SECONDS; + const toolLimits = { + commandTimeoutSeconds: limits.commandTimeoutSeconds ?? DEFAULT_TOOL_LIMITS.commandTimeoutSeconds, + maxCommandOutputChars: limits.maxCommandOutputChars ?? DEFAULT_TOOL_LIMITS.maxCommandOutputChars, + }; + const usage = { promptTokens: 0, completionTokens: 0, totalTokens: 0, reported: false }; + const end = (stopReason: StopReason, message: string): LoopResult => ({ stopReason, message, usage }); + + const messages: ChatMessage[] = [ + { role: "system", content: SYSTEM_PROMPT }, + { role: "user", content: prompt }, + ]; + let toolCalls = 0; + + for (let iteration = 1; iteration <= maxIterations; iteration += 1) { + if (options.signal?.aborted) return end("cancelled", "The run was cancelled."); + if ((now() - started) / 1000 > maxSeconds) return end("max_seconds", `Stopped at the ${maxSeconds} s limit.`); + + const turn = await completeTurn(options.chat, messages, options.signal); + if (turn.usage.promptTokens !== undefined || turn.usage.completionTokens !== undefined || turn.usage.totalTokens !== undefined) { + usage.reported = true; + usage.promptTokens += turn.usage.promptTokens ?? 0; + usage.completionTokens += turn.usage.completionTokens ?? 0; + usage.totalTokens += turn.usage.totalTokens ?? (turn.usage.promptTokens ?? 0) + (turn.usage.completionTokens ?? 0); + await options.onEvent({ type: "usage", usage: turn.usage }); + } + if (turn.text.trim().length > 0) await options.onEvent({ type: "text", text: turn.text }); + + messages.push({ + role: "assistant", + content: turn.text, + ...(turn.calls.length > 0 + ? { tool_calls: turn.calls.map((call) => ({ id: call.id, type: "function" as const, function: { name: call.name, arguments: JSON.stringify(call.arguments) } })) } + : {}), + }); + + if (exceeded(limits.maxPromptTokens, usage.promptTokens)) return end("max_prompt_tokens", `Stopped at the ${limits.maxPromptTokens}-token prompt limit.`); + if (exceeded(limits.maxCompletionTokens, usage.completionTokens)) return end("max_completion_tokens", `Stopped at the ${limits.maxCompletionTokens}-token completion limit.`); + if (exceeded(limits.maxTotalTokens, usage.totalTokens)) return end("max_total_tokens", `Stopped at the ${limits.maxTotalTokens}-token total limit.`); + + if (turn.calls.length === 0) return end("completed", turn.text.trim() || "Completed with an empty answer."); + + for (const call of turn.calls) { + if (options.signal?.aborted) return end("cancelled", "The run was cancelled."); + toolCalls += 1; + if (toolCalls > maxToolCalls) return end("max_tool_calls", `Stopped at the ${maxToolCalls}-tool-call limit.`); + await options.onEvent({ type: "tool_call", call }); + let result: ToolResult; + if (call.name === "run_command" && options.allowCommand !== undefined + && !(await options.allowCommand(typeof call.arguments.command === "string" ? call.arguments.command : ""))) { + result = { output: "The person did not allow this command; it was not run.", failed: true }; + } else { + result = await runTool(call.name, call.arguments, options.cwd, toolLimits, options.signal); + } + await options.onEvent({ type: "tool_result", call, result }); + messages.push({ role: "tool", tool_call_id: call.id, content: result.output }); + } + } + return end("max_iterations", `Stopped at the ${maxIterations}-iteration limit.`); +} diff --git a/packages/core/src/agents/local-agent/chat-client.test.ts b/packages/core/src/agents/local-agent/chat-client.test.ts new file mode 100644 index 00000000..75a5ff30 --- /dev/null +++ b/packages/core/src/agents/local-agent/chat-client.test.ts @@ -0,0 +1,64 @@ +import { createServer, type Server } from "node:http"; +import { afterEach, describe, expect, it } from "vitest"; +import { completeTurn } from "./chat-client.js"; +import { TOOL_PARAMETER_TYPES, TOOL_SCHEMAS } from "./tools.js"; + +// local-llm-codes-in-process 2.4: against a local HTTP stand-in, answering +// as SGLang does. +let server: Server | undefined; +afterEach(async () => { + await new Promise((resolve) => (server ? server.close(() => resolve()) : resolve())); + server = undefined; +}); + +async function serve(answer: unknown): Promise<{ base: string; requests: unknown[] }> { + const requests: unknown[] = []; + server = createServer((req, res) => { + let body = ""; + req.on("data", (chunk: Buffer) => { body += chunk.toString("utf8"); }); + req.on("end", () => { + requests.push({ url: req.url, auth: req.headers.authorization, body: JSON.parse(body) }); + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify(answer)); + }); + }); + await new Promise((resolve) => server?.listen(0, "127.0.0.1", resolve)); + const address = server.address(); + const port = typeof address === "object" && address !== null ? address.port : 0; + return { base: `http://127.0.0.1:${port}/v1`, requests }; +} + +const options = (base: string) => ({ + settings: { baseUrl: base, apiKey: "k" }, + model: "m", + fetch: (input: string, init?: RequestInit) => fetch(input, init), + tools: TOOL_SCHEMAS, + parameterTypes: TOOL_PARAMETER_TYPES, +}); + +describe("completeTurn", () => { + it("reads SGLang's null tool_calls and null usage as none", async () => { + const { base, requests } = await serve({ choices: [{ message: { content: "Done.", tool_calls: null } }], usage: null }); + const turn = await completeTurn(options(base), [{ role: "user", content: "hi" }]); + expect(turn).toEqual({ text: "Done.", calls: [], usage: {} }); + expect(requests[0]).toMatchObject({ url: "/v1/chat/completions", auth: "Bearer k", body: { model: "m", tool_choice: "auto" } }); + }); + + it("reads structured calls, a null argument as no arguments", async () => { + const { base } = await serve({ + choices: [{ message: { content: null, tool_calls: [{ id: "c1", type: "function", function: { name: "list_dir", arguments: null } }] } }], + usage: { prompt_tokens: 5, completion_tokens: 2, total_tokens: 7 }, + }); + const turn = await completeTurn(options(base), [{ role: "user", content: "hi" }]); + expect(turn).toEqual({ text: "", calls: [{ id: "c1", name: "list_dir", arguments: {} }], usage: { promptTokens: 5, completionTokens: 2, totalTokens: 7 } }); + }); + + it("reads a call written as text when there is no structured one", async () => { + const { base } = await serve({ + choices: [{ message: { content: "\n\n\na.txt\n\n\n", tool_calls: null } }], + }); + const turn = await completeTurn(options(base), [{ role: "user", content: "hi" }]); + expect(turn.calls).toEqual([{ id: "text-call-1", name: "read_file", arguments: { path: "a.txt" } }]); + expect(turn.text).toBe(""); + }); +}); diff --git a/packages/core/src/agents/local-agent/chat-client.ts b/packages/core/src/agents/local-agent/chat-client.ts new file mode 100644 index 00000000..7d874f3f --- /dev/null +++ b/packages/core/src/agents/local-agent/chat-client.ts @@ -0,0 +1,119 @@ +// One model turn against an OpenAI-compatible `/v1/chat/completions`, with +// tools (local-llm-codes-in-process). +// +// Read as servers send it, not as the schema says: SGLang answers a turn +// that calls no tool with `"tool_calls": null` and may send `"usage": +// null`; a call's arguments may be null. A call the server's parser did not +// recognise is read from the text (`toolCallsInText`). + +import type { FetchLike } from "../../direct-fetch.js"; +import { chatCompletionsUrl, localLlmHeaders, type LocalLlmSettings } from "../../local-llm-settings.js"; +import { toolCallsInText, type ParameterTypes, type ToolCall } from "./text-tool-calls.js"; +import type { ToolSchema } from "./tools.js"; + +export interface ChatMessage { + role: "system" | "user" | "assistant" | "tool"; + content: string; + tool_calls?: Array<{ id: string; type: "function"; function: { name: string; arguments: string } }>; + tool_call_id?: string; +} + +export interface TurnUsage { + promptTokens?: number; + completionTokens?: number; + totalTokens?: number; +} + +export interface AssistantTurn { + text: string; + calls: ToolCall[]; + usage: TurnUsage; +} + +export interface ChatClientOptions { + settings: Pick; + model: string; + fetch: FetchLike; + tools: readonly ToolSchema[]; + parameterTypes: ParameterTypes; + maxCompletionTokens?: number; +} + +function count(value: unknown): number | undefined { + return typeof value === "number" && Number.isFinite(value) ? value : undefined; +} + +function textOf(content: unknown): string { + if (typeof content === "string") return content; + if (Array.isArray(content)) { + return content + .map((part) => (typeof part === "object" && part !== null && typeof (part as { text?: unknown }).text === "string" ? (part as { text: string }).text : "")) + .filter((part) => part.length > 0) + .join("\n"); + } + return ""; +} + +/** Asks the model for its next turn. Throws on a transport failure or a + * non-2xx answer, with the status and the start of the body. */ +export async function completeTurn(options: ChatClientOptions, messages: readonly ChatMessage[], signal?: AbortSignal): Promise { + const body: Record = { + model: options.model, + messages, + tools: options.tools, + tool_choice: "auto", + }; + if (options.maxCompletionTokens !== undefined) body.max_tokens = options.maxCompletionTokens; + const response = await options.fetch(chatCompletionsUrl(options.settings.baseUrl), { + method: "POST", + headers: localLlmHeaders(options.settings), + body: JSON.stringify(body), + ...(signal !== undefined ? { signal } : {}), + }); + if (!response.ok) { + const detail = (await response.text().catch(() => "")).slice(0, 300); + throw new Error(`HTTP ${response.status} ${response.statusText}${detail ? `: ${detail}` : ""}`); + } + const data = (await response.json()) as { + choices?: Array<{ message?: { content?: unknown; tool_calls?: unknown } }>; + usage?: { prompt_tokens?: unknown; completion_tokens?: unknown; total_tokens?: unknown } | null; + }; + const message = data.choices?.[0]?.message ?? {}; + let text = textOf(message.content); + let calls: ToolCall[] = []; + const rawCalls = Array.isArray(message.tool_calls) ? message.tool_calls : []; + rawCalls.forEach((raw: unknown, index) => { + if (typeof raw !== "object" || raw === null) return; + const record = raw as { id?: unknown; function?: { name?: unknown; arguments?: unknown } | null }; + const fn = record.function ?? {}; + if (typeof fn.name !== "string" || fn.name.length === 0) return; + let args: unknown = fn.arguments ?? {}; + if (typeof args === "string") { + try { + args = args.trim().length > 0 ? JSON.parse(args) : {}; + } catch { + args = {}; + } + } + calls.push({ + id: typeof record.id === "string" && record.id.length > 0 ? record.id : `call-${index + 1}`, + name: fn.name, + arguments: typeof args === "object" && args !== null && !Array.isArray(args) ? (args as Record) : {}, + }); + }); + if (calls.length === 0) { + const fromText = toolCallsInText(text, options.parameterTypes); + calls = fromText.calls; + text = fromText.text; + } + const usage = data.usage ?? {}; + return { + text, + calls, + usage: { + ...(count(usage.prompt_tokens) !== undefined ? { promptTokens: count(usage.prompt_tokens) } : {}), + ...(count(usage.completion_tokens) !== undefined ? { completionTokens: count(usage.completion_tokens) } : {}), + ...(count(usage.total_tokens) !== undefined ? { totalTokens: count(usage.total_tokens) } : {}), + }, + }; +} diff --git a/packages/core/src/agents/local-agent/sandbox.test.ts b/packages/core/src/agents/local-agent/sandbox.test.ts new file mode 100644 index 00000000..6fe97e8d --- /dev/null +++ b/packages/core/src/agents/local-agent/sandbox.test.ts @@ -0,0 +1,54 @@ +import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { OutsideWorkingDirectoryError, resolveInside } from "./sandbox.js"; + +// local-llm-codes-in-process 2.2: by real path, not by `path.resolve` alone. +// Temporary directories: the file took 150 ms alone on 2026-10-02; the ceiling is the +// one the other local-agent tests use, for a loaded machine. +vi.setConfig({ testTimeout: 20_000 }); +const roots: string[] = []; +afterEach(async () => { + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))); +}); + +async function tree(): Promise<{ cwd: string; outside: string }> { + const root = await mkdtemp(path.join(os.tmpdir(), "openspec-sandbox-")); + roots.push(root); + const cwd = path.join(root, "work"); + const outside = path.join(root, "outside"); + await mkdir(path.join(cwd, "src"), { recursive: true }); + await mkdir(outside, { recursive: true }); + await writeFile(path.join(outside, "secret.txt"), "no", "utf8"); + return { cwd, outside }; +} + +describe("resolveInside", () => { + it("answers a path inside, existing or not yet written", async () => { + const { cwd } = await tree(); + expect(await resolveInside(cwd, "src")).toBe(path.join(await (await import("node:fs/promises")).realpath(cwd), "src")); + await expect(resolveInside(cwd, "src/new/deep.txt")).resolves.toContain(path.join("src", "new", "deep.txt")); + }); + + it("refuses ..", async () => { + const { cwd } = await tree(); + await expect(resolveInside(cwd, "../outside/secret.txt")).rejects.toBeInstanceOf(OutsideWorkingDirectoryError); + }); + + it("refuses an absolute path elsewhere", async () => { + const { cwd, outside } = await tree(); + await expect(resolveInside(cwd, path.join(outside, "secret.txt"))).rejects.toBeInstanceOf(OutsideWorkingDirectoryError); + }); + + it("refuses a link inside the directory that points outside it", async () => { + const { cwd, outside } = await tree(); + try { + await symlink(outside, path.join(cwd, "link"), process.platform === "win32" ? "junction" : "dir"); + } catch { + return; // A machine that cannot make links cannot be escaped through one. + } + await expect(resolveInside(cwd, "link/secret.txt")).rejects.toBeInstanceOf(OutsideWorkingDirectoryError); + await expect(resolveInside(cwd, "link/new.txt")).rejects.toBeInstanceOf(OutsideWorkingDirectoryError); + }); +}); diff --git a/packages/core/src/agents/local-agent/sandbox.ts b/packages/core/src/agents/local-agent/sandbox.ts new file mode 100644 index 00000000..1e4a36c9 --- /dev/null +++ b/packages/core/src/agents/local-agent/sandbox.ts @@ -0,0 +1,53 @@ +// Where the local agent's tools may reach: the run's working directory, +// by real path (local-llm-codes-in-process, ADR 0038 decision 3). +// +// `path.resolve` alone is not enough: a link inside `cwd` can point +// outside it. The real path of the target, or of its nearest existing +// ancestor for a file not yet written, must lie inside the real path of +// `cwd` - the check `security.ts` makes for a change's artifacts. + +import { realpath } from "node:fs/promises"; +import path from "node:path"; + +export class OutsideWorkingDirectoryError extends Error { + constructor(readonly requested: string) { + super(`"${requested}" is outside the working directory`); + } +} + +function isInside(root: string, candidate: string): boolean { + const relative = path.relative(root, candidate); + if (relative === "") return true; + if (path.isAbsolute(relative)) return false; + return relative !== ".." && !relative.startsWith(`..${path.sep}`); +} + +async function realOrNearestAncestor(target: string): Promise { + let current = target; + const rest: string[] = []; + for (;;) { + try { + return path.join(await realpath(current), ...rest.reverse()); + } catch { + const parent = path.dirname(current); + if (parent === current) return target; + rest.push(path.basename(current)); + current = parent; + } + } +} + +/** The absolute path `requested` names, relative to `cwd` or absolute, + * once its real location is known to be inside `cwd`'s. Throws + * `OutsideWorkingDirectoryError` otherwise. */ +export async function resolveInside(cwd: string, requested: string): Promise { + const root = await realpath(cwd); + const target = path.resolve(root, requested); + if (!isInside(root, target)) throw new OutsideWorkingDirectoryError(requested); + const real = await realOrNearestAncestor(target); + const sameCase = process.platform === "win32" + ? isInside(root.toLowerCase(), real.toLowerCase()) + : isInside(root, real); + if (!sameCase) throw new OutsideWorkingDirectoryError(requested); + return target; +} diff --git a/packages/core/src/agents/local-agent/text-tool-calls.test.ts b/packages/core/src/agents/local-agent/text-tool-calls.test.ts new file mode 100644 index 00000000..3e5cdbad --- /dev/null +++ b/packages/core/src/agents/local-agent/text-tool-calls.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from "vitest"; +import { toolCallsInText } from "./text-tool-calls.js"; + +// local-llm-codes-in-process 2.1. +const known = new Map>([ + ["read_file", { path: "string" }], + ["write_file", { path: "string", content: "string" }], + ["git_add", { paths: "array" }], +]); + +describe("toolCallsInText", () => { + it("takes a Qwen3-Coder call, as Qwen3.6 writes it through SGLang's hermes parser", () => { + const { calls, text } = toolCallsInText( + "Let me look first.\n\n\n\n\nsrc/greet.js\n\n\n", + known, + ); + expect(calls).toEqual([{ id: "text-call-1", name: "read_file", arguments: { path: "src/greet.js" } }]); + expect(text).toBe("Let me look first."); + }); + + it("takes a Hermes call, reading an array where the tool declares one", () => { + const { calls, text } = toolCallsInText('\n{"name": "git_add", "arguments": {"paths": ["a.txt"]}}\n', known); + expect(calls).toEqual([{ id: "text-call-1", name: "git_add", arguments: { paths: ["a.txt"] } }]); + expect(text).toBe(""); + }); + + it("keeps JSON-looking file content as the text to write", () => { + const { calls } = toolCallsInText( + "\n\n\na.json\n\n\n{\"a\": 1}\n\n\n", + known, + ); + expect(calls[0]?.arguments).toEqual({ path: "a.json", content: '{"a": 1}' }); + }); + + it("leaves a call to a tool that was not offered as text", () => { + const content = "\n\n\n"; + expect(toolCallsInText(content, known)).toEqual({ calls: [], text: content }); + }); + + it("numbers several calls in order", () => { + const one = "a"; + const two = "b"; + expect(toolCallsInText(`${one}${two}`, known).calls.map((call) => [call.id, call.arguments.path])).toEqual([ + ["text-call-1", "a"], + ["text-call-2", "b"], + ]); + }); +}); diff --git a/packages/core/src/agents/local-agent/text-tool-calls.ts b/packages/core/src/agents/local-agent/text-tool-calls.ts new file mode 100644 index 00000000..e414ff4e --- /dev/null +++ b/packages/core/src/agents/local-agent/text-tool-calls.ts @@ -0,0 +1,97 @@ +// Tool calls a model wrote into its text (local-llm-codes-in-process, ADR +// 0038 decision 4). +// +// A server whose tool-call parser does not match the model passes the +// model's calls through as text in `content`, with `tool_calls` empty. +// Measured 2026-10-01 against SGLang with `--tool-call-parser hermes` +// serving Qwen3.6, which writes Qwen3-Coder's form: 0 of 6 calls came back +// structured. Ported from `coding-agent`'s `tool_calls_in_text`, verified +// live the same day. + +/** A call the agent loop runs: the id it answers with, the tool, its + * arguments. */ +export interface ToolCall { + id: string; + name: string; + arguments: Record; +} + +/** Each offered tool's name, with the JSON type of each parameter. */ +export type ParameterTypes = ReadonlyMap>>; + +const TOOL_CALL_BLOCK = /\s*([\s\S]*?)\s*<\/tool_call>/g; +const XML_FUNCTION = /\s]+)>([\s\S]*?)<\/function>/; +const XML_PARAMETER = /\s]+)>([\s\S]*?)<\/parameter>/g; + +/** The calls written inside `` in Qwen3-Coder's + * `value` form or as Hermes' + * `{"name": ..., "arguments": ...}`, and the text without them. Only a call + * to an offered tool is taken; anything else stays text. */ +export function toolCallsInText(content: string, known: ParameterTypes): { calls: ToolCall[]; text: string } { + const calls: ToolCall[] = []; + const taken: Array<[number, number]> = []; + for (const block of content.matchAll(TOOL_CALL_BLOCK)) { + const call = parseCall(block[1] ?? "", known, calls.length + 1); + if (call !== undefined && block.index !== undefined) { + calls.push(call); + taken.push([block.index, block.index + block[0].length]); + } + } + if (calls.length === 0) return { calls, text: content }; + let text = content; + for (const [start, end] of [...taken].reverse()) text = text.slice(0, start) + text.slice(end); + return { calls, text: text.trim() }; +} + +function parseCall(body: string, known: ParameterTypes, index: number): ToolCall | undefined { + let name: string; + let args: Record; + const fn = XML_FUNCTION.exec(body); + if (fn) { + name = fn[1] ?? ""; + const types = known.get(name) ?? {}; + args = {}; + for (const parameter of (fn[2] ?? "").matchAll(XML_PARAMETER)) { + const key = parameter[1] ?? ""; + args[key] = parameterValue(parameter[2] ?? "", types[key] ?? "string"); + } + } else { + let parsed: unknown; + try { + parsed = JSON.parse(body); + } catch { + return undefined; + } + if (typeof parsed !== "object" || parsed === null) return undefined; + const record = parsed as { name?: unknown; arguments?: unknown }; + if (typeof record.name !== "string") return undefined; + name = record.name; + let raw = record.arguments ?? {}; + if (typeof raw === "string") { + try { + raw = JSON.parse(raw); + } catch { + return undefined; + } + } + args = typeof raw === "object" && raw !== null && !Array.isArray(raw) ? (raw as Record) : {}; + } + if (!known.has(name)) return undefined; + return { id: `text-call-${index}`, name, arguments: args }; +} + +/** A parameter's text, without the newline the format puts around it. Read + * as JSON only where the tool declares an array or an object: a file's + * content that looks like JSON is still the text to write. */ +function parameterValue(raw: string, jsonType: string): unknown { + let value = raw.startsWith("\n") ? raw.slice(1) : raw; + value = value.endsWith("\n") ? value.slice(0, -1) : value; + if (jsonType === "array" || jsonType === "object") { + try { + return JSON.parse(value); + } catch { + return value; + } + } + return value; +} diff --git a/packages/core/src/agents/local-agent/tools.test.ts b/packages/core/src/agents/local-agent/tools.test.ts new file mode 100644 index 00000000..6c9a0cf7 --- /dev/null +++ b/packages/core/src/agents/local-agent/tools.test.ts @@ -0,0 +1,76 @@ +import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { runTool, TOOL_PARAMETER_TYPES, TOOL_SCHEMAS } from "./tools.js"; + +// local-llm-codes-in-process 2.3. +vi.setConfig({ testTimeout: 20_000 }); + +const roots: string[] = []; +afterEach(async () => { + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))); +}); + +async function cwd(): Promise { + const root = await mkdtemp(path.join(os.tmpdir(), "openspec-tools-")); + roots.push(root); + return root; +} + +const limits = { commandTimeoutSeconds: 10, maxCommandOutputChars: 2000 }; + +describe("the local agent's tools", () => { + it("offers six tools, with their parameter types for calls written as text", () => { + expect(TOOL_SCHEMAS.map((tool) => tool.function.name)).toEqual(["read_file", "write_file", "replace_text", "list_dir", "search_text", "run_command"]); + expect(TOOL_PARAMETER_TYPES.get("replace_text")).toEqual({ path: "string", old_text: "string", new_text: "string", expected_replacements: "integer" }); + }); + + it("writes, reads, replaces, lists and searches inside the directory", async () => { + const dir = await cwd(); + expect(await runTool("write_file", { path: "src/a.js", content: "const a = 1;\nconst b = 1;\n" }, dir, limits)).toEqual({ output: "Wrote src/a.js", failed: false }); + expect((await runTool("read_file", { path: "src/a.js" }, dir, limits)).output).toContain("const b = 1;"); + expect(await runTool("replace_text", { path: "src/a.js", old_text: "= 1", new_text: "= 2", expected_replacements: 2 }, dir, limits)) + .toEqual({ output: "Replaced 2 occurrence(s) in src/a.js", failed: false }); + expect((await runTool("replace_text", { path: "src/a.js", old_text: "= 2", new_text: "= 3" }, dir, limits)).failed).toBe(true); + expect((await runTool("list_dir", {}, dir, limits)).output).toBe("src/"); + expect((await runTool("search_text", { query: "const b" }, dir, limits)).output).toBe("src/a.js:2: const b = 2;"); + }); + + it("refuses a path outside the directory, and says so", async () => { + const dir = await cwd(); + const result = await runTool("write_file", { path: "../escaped.txt", content: "x" }, dir, limits); + expect(result.failed).toBe(true); + expect(result.output).toContain("outside the working directory"); + await expect(readFile(path.join(dir, "..", "escaped.txt"), "utf8")).rejects.toThrow(); + }); + + it("runs a command in the directory and gives its exit code and output", async () => { + const dir = await cwd(); + await writeFile(path.join(dir, "here.txt"), "", "utf8"); + const result = await runTool("run_command", { command: "node -e \"console.log(require('fs').existsSync('here.txt'))\"" }, dir, limits); + expect(result).toEqual({ output: "Exit code 0.\ntrue", failed: false }); + }); + + it("cuts a command's output at the cap", async () => { + const dir = await cwd(); + const result = await runTool("run_command", { command: "node -e \"console.log('x'.repeat(5000))\"" }, dir, { ...limits, maxCommandOutputChars: 300 }); + expect(result.output.length).toBeLessThan(400); + expect(result.output).toContain("more characters"); + }); + + it("ends a command at the time limit", async () => { + const dir = await cwd(); + const started = Date.now(); + const result = await runTool("run_command", { command: "node -e \"setTimeout(() => {}, 60000)\"" }, dir, { ...limits, commandTimeoutSeconds: 1 }); + expect(result.failed).toBe(true); + expect(result.output).toContain("ended after 1 s"); + expect(Date.now() - started).toBeLessThan(15_000); + }); + + it("answers an unknown tool and a missing argument without throwing", async () => { + const dir = await cwd(); + expect(await runTool("format_disk", {}, dir, limits)).toEqual({ output: "Unknown tool: format_disk", failed: true }); + expect((await runTool("read_file", {}, dir, limits)).output).toContain('missing argument "path"'); + }); +}); diff --git a/packages/core/src/agents/local-agent/tools.ts b/packages/core/src/agents/local-agent/tools.ts new file mode 100644 index 00000000..058c2635 --- /dev/null +++ b/packages/core/src/agents/local-agent/tools.ts @@ -0,0 +1,207 @@ +// The local agent's tools (local-llm-codes-in-process, ADR 0038 decision 3). +// +// Ported from `coding-agent`, fewer of them: reading, writing, replacing in +// a file, listing, searching, and running a command. Git, background +// processes, delete and move are left to `run_command`; fewer tools leave a +// model less to choose wrongly among. Every path goes through +// `resolveInside`, so nothing outside the run's working directory is read +// or written. + +import { spawn } from "node:child_process"; +import { mkdir, readdir, readFile, stat, writeFile } from "node:fs/promises"; +import path from "node:path"; +import { terminateProcessTree } from "../shared.js"; +import { resolveInside } from "./sandbox.js"; + +export interface ToolLimits { + /** Seconds a command may run before it is ended. */ + commandTimeoutSeconds: number; + /** Characters of a command's output, or a search's, the model is given. */ + maxCommandOutputChars: number; +} + +export const DEFAULT_TOOL_LIMITS: ToolLimits = { commandTimeoutSeconds: 60, maxCommandOutputChars: 12_000 }; + +/** The OpenAI function schema of a tool. */ +export interface ToolSchema { + type: "function"; + function: { name: string; description: string; parameters: { type: "object"; properties: Record; required: string[] } }; +} + +function schema(name: string, description: string, properties: ToolSchema["function"]["parameters"]["properties"], required: string[]): ToolSchema { + return { type: "function", function: { name, description, parameters: { type: "object", properties, required } } }; +} + +export const TOOL_SCHEMAS: readonly ToolSchema[] = [ + schema("read_file", "Read a UTF-8 text file in the working directory.", { path: { type: "string" } }, ["path"]), + schema( + "write_file", + "Write a UTF-8 text file in the working directory, creating its directories.", + { path: { type: "string" }, content: { type: "string" } }, + ["path", "content"], + ), + schema( + "replace_text", + "Replace old_text with new_text in a file; expected_replacements must equal the number of occurrences.", + { path: { type: "string" }, old_text: { type: "string" }, new_text: { type: "string" }, expected_replacements: { type: "integer" } }, + ["path", "old_text", "new_text"], + ), + schema("list_dir", "List a directory in the working directory.", { path: { type: "string" } }, []), + schema( + "search_text", + "Search for a literal text in the files under a directory of the working directory.", + { query: { type: "string" }, path: { type: "string" } }, + ["query"], + ), + schema( + "run_command", + "Run a shell command in the working directory and return its exit code and output.", + { command: { type: "string" } }, + ["command"], + ), +]; + +/** Each tool's parameters with their JSON types, for reading calls written + * as text. */ +export const TOOL_PARAMETER_TYPES: ReadonlyMap>> = new Map( + TOOL_SCHEMAS.map((tool) => [ + tool.function.name, + Object.fromEntries(Object.entries(tool.function.parameters.properties).map(([key, value]) => [key, value.type])), + ]), +); + +/** A tool's answer; `failed` says it did not do what was asked. */ +export interface ToolResult { + output: string; + failed: boolean; +} + +function text(args: Record, key: string, fallback?: string): string { + const value = args[key]; + if (typeof value === "string") return value; + if (typeof value === "number" || typeof value === "boolean") return String(value); + if (fallback !== undefined) return fallback; + throw new Error(`missing argument "${key}"`); +} + +function cap(output: string, limit: number): string { + if (output.length <= limit) return output; + return `${output.slice(0, limit)}\n... (${output.length - limit} more characters)`; +} + +const SKIPPED_DIRECTORIES = new Set([".git", "node_modules", ".venv", "dist", "out", ".openspec-ui"]); + +async function* filesUnder(directory: string): AsyncGenerator { + const entries = await readdir(directory, { withFileTypes: true }); + for (const entry of entries) { + const full = path.join(directory, entry.name); + if (entry.isDirectory()) { + if (!SKIPPED_DIRECTORIES.has(entry.name)) yield* filesUnder(full); + } else if (entry.isFile()) { + yield full; + } + } +} + +/** Runs `command` through the platform's shell in `cwd`, ending it, and + * every process it started, at the time limit or when `signal` aborts. */ +export function runShellCommand(command: string, cwd: string, limits: ToolLimits, signal?: AbortSignal): Promise { + return new Promise((resolve) => { + // The shell reads the command line as a person would type it; the + // command is the model's, which is why it runs only in `cwd` and only + // for as long as the limit allows. + const child = spawn(command, { cwd, shell: true, stdio: ["ignore", "pipe", "pipe"], windowsHide: true, ...(process.platform !== "win32" ? { detached: true } : {}) }); + let output = ""; + let ended: string | undefined; + const end = (why: string) => { + ended = why; + if (child.pid !== undefined) void terminateProcessTree(child.pid); + }; + const timer = setTimeout(() => end(`ended after ${limits.commandTimeoutSeconds} s`), limits.commandTimeoutSeconds * 1000); + const onAbort = () => end("ended: the run was cancelled"); + signal?.addEventListener("abort", onAbort, { once: true }); + child.stdout?.on("data", (chunk: Buffer) => { output += chunk.toString("utf8"); }); + child.stderr?.on("data", (chunk: Buffer) => { output += chunk.toString("utf8"); }); + const finish = (code: number | null, error?: Error) => { + clearTimeout(timer); + signal?.removeEventListener("abort", onAbort); + const head = error ? `Could not run the command: ${error.message}` : ended ? `The command was ${ended}.` : `Exit code ${code ?? "unknown"}.`; + resolve({ output: cap(`${head}\n${output}`.trimEnd(), limits.maxCommandOutputChars), failed: error !== undefined || ended !== undefined || code !== 0 }); + }; + child.on("error", (error) => finish(null, error)); + child.on("close", (code) => finish(code)); + }); +} + +/** Runs one tool in `cwd`. A refused path, a missing argument or an + * unknown tool is an answer, not an exception: the model is told and + * goes on. */ +export async function runTool( + name: string, + args: Record, + cwd: string, + limits: ToolLimits, + signal?: AbortSignal, +): Promise { + try { + switch (name) { + case "read_file": { + const target = await resolveInside(cwd, text(args, "path")); + return { output: cap(await readFile(target, "utf8"), limits.maxCommandOutputChars * 4), failed: false }; + } + case "write_file": { + const requested = text(args, "path"); + const target = await resolveInside(cwd, requested); + await mkdir(path.dirname(target), { recursive: true }); + await writeFile(target, text(args, "content"), "utf8"); + return { output: `Wrote ${requested}`, failed: false }; + } + case "replace_text": { + const requested = text(args, "path"); + const target = await resolveInside(cwd, requested); + const oldText = text(args, "old_text"); + if (oldText.length === 0) return { output: "old_text is empty", failed: true }; + const content = await readFile(target, "utf8"); + const count = content.split(oldText).length - 1; + const expected = args.expected_replacements === undefined ? 1 : Number(args.expected_replacements); + if (count !== expected) { + return { output: `Expected ${expected} occurrence(s) of old_text in ${requested}, found ${count}; nothing replaced`, failed: true }; + } + await writeFile(target, content.split(oldText).join(text(args, "new_text")), "utf8"); + return { output: `Replaced ${count} occurrence(s) in ${requested}`, failed: false }; + } + case "list_dir": { + const requested = text(args, "path", "."); + const target = await resolveInside(cwd, requested); + const entries = await readdir(target, { withFileTypes: true }); + const lines = entries.map((entry) => (entry.isDirectory() ? `${entry.name}/` : entry.name)).sort(); + return { output: lines.join("\n") || "(empty)", failed: false }; + } + case "search_text": { + const query = text(args, "query"); + const target = await resolveInside(cwd, text(args, "path", ".")); + const found: string[] = []; + const roots = (await stat(target)).isDirectory() ? filesUnder(target) : (async function* () { yield target; })(); + for await (const file of roots) { + let content: string; + try { + content = await readFile(file, "utf8"); + } catch { + continue; + } + content.split(/\r?\n/u).forEach((line, index) => { + if (line.includes(query)) found.push(`${path.relative(cwd, file).split(path.sep).join("/")}:${index + 1}: ${line.trim()}`); + }); + if (found.length >= 200) break; + } + return { output: cap(found.join("\n") || "No match.", limits.maxCommandOutputChars), failed: false }; + } + case "run_command": + return await runShellCommand(text(args, "command"), cwd, limits, signal); + default: + return { output: `Unknown tool: ${name}`, failed: true }; + } + } catch (error) { + return { output: `Tool error: ${error instanceof Error ? error.message : String(error)}`, failed: true }; + } +} diff --git a/packages/core/src/agents/local-llm-acp.test.ts b/packages/core/src/agents/local-llm-acp.test.ts index 14445f27..a343e265 100644 --- a/packages/core/src/agents/local-llm-acp.test.ts +++ b/packages/core/src/agents/local-llm-acp.test.ts @@ -1,124 +1,182 @@ +// local-llm-codes-in-process 2.6-2.7: the local agent run through the real +// `AcpSessionDriver`, in process, against a stand-in model. No process is +// started for the agent, and no server: the model is a `fetch` that answers +// as an OpenAI-compatible server does. + +import { mkdtemp, readFile, rm } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; +import type { FetchLike } from "../direct-fetch.js"; import type { Command, Event } from "../protocol.js"; +import { LocalLlmAcpAdapter } from "./local-llm-acp.js"; + +vi.setConfig({ testTimeout: 20_000 }); -const runProcessMock = vi.fn(); -const resolvePermissionMock = vi.fn(); -vi.mock("./acp-session-driver.js", () => ({ - AcpSessionDriver: vi.fn().mockImplementation(() => ({ - runProcess: (...args: unknown[]) => runProcessMock(...args), - resolvePermission: (...args: unknown[]) => resolvePermissionMock(...args), - })), -})); - -afterEach(() => { - runProcessMock.mockReset(); - resolvePermissionMock.mockReset(); +const roots: string[] = []; +afterEach(async () => { + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))); }); -const { LocalLlmAcpAdapter } = await import("./local-llm-acp.js"); - -const command: Command = { - kind: "implement", - cwd: "/workspace/repo", - runId: "run-local-llm-acp-1", - context: { changeDir: "/workspace/repo/openspec/changes/x" }, -}; - -describe("LocalLlmAcpAdapter", () => { - it("builds a process invocation with the options before the acp subcommand", () => { - // `coding-agent` reads its options only before the subcommand: - // `coding-agent acp --base-url ...` exits with its usage. - const adapter = new LocalLlmAcpAdapter({ - executable: "coding-agent", - baseUrl: "http://gpu.lan:8000/v1", - model: "qwen2.5-coder", - limits: {}, - }); - expect(adapter.buildInvocation(command)).toEqual({ - kind: "process", - executable: "coding-agent", - args: ["--base-url", "http://gpu.lan:8000/v1", "--model", "qwen2.5-coder", "acp"], - }); +async function workspace(): Promise { + const root = await mkdtemp(path.join(os.tmpdir(), "openspec-local-agent-")); + roots.push(root); + return root; +} + +type Answer = { content?: string | null; tool_calls?: unknown; usage?: unknown }; + +/** A model that gives `answers` in turn, and lists `models` at /v1/models. */ +function standInModel(answers: Answer[], models: string[] = ["stand-in-model"]): { fetch: FetchLike; requests: Array<{ url: string; body?: unknown }> } { + const requests: Array<{ url: string; body?: unknown }> = []; + let turn = 0; + const fetchImpl: FetchLike = async (url, init) => { + const body = typeof init?.body === "string" ? JSON.parse(init.body) : undefined; + requests.push({ url, body }); + if (url.endsWith("/models")) return new Response(JSON.stringify({ data: models.map((id) => ({ id })) }), { status: 200 }); + const answer = answers[Math.min(turn, answers.length - 1)] ?? {}; + turn += 1; + return new Response( + JSON.stringify({ + choices: [{ message: { role: "assistant", content: answer.content ?? null, tool_calls: answer.tool_calls ?? null } }], + usage: answer.usage ?? { prompt_tokens: 100, completion_tokens: 10, total_tokens: 110 }, + }), + { status: 200 }, + ); + }; + return { fetch: fetchImpl, requests }; +} + +function call(name: string, args: Record, id = "c1") { + return [{ id, type: "function", function: { name, arguments: JSON.stringify(args) } }]; +} + +function command(cwd: string, extra: Partial = {}): Command { + return { kind: "implement", cwd, runId: `run-${Math.random()}`, context: { changeDir: path.join(cwd, "openspec", "changes", "x") }, ...extra }; +} + +async function collect(events: AsyncIterable, onEvent?: (event: Event) => void): Promise { + const out: Event[] = []; + for await (const event of events) { + out.push(event); + onEvent?.(event); + } + return out; +} + +function updates(events: Event[]): Array> { + return events.filter((e) => e.kind === "agentUpdate").map((e) => (e as { update: Record }).update); +} + +describe("LocalLlmAcpAdapter, in process", () => { + it("starts no process: its invocation is in-process", () => { + const adapter = new LocalLlmAcpAdapter({ settings: { baseUrl: "http://x" }, limits: {}, fetch: standInModel([]).fetch, askBeforeCommands: false }); + expect(adapter.buildInvocation(command("/tmp"))).toEqual({ kind: "in-process", agent: "local-llm-acp" }); }); - it("renders only configured loop limits and omits absent ones", () => { - const adapter = new LocalLlmAcpAdapter({ - executable: "coding-agent", - baseUrl: "http://gpu.lan:8000/v1", - model: "qwen2.5-coder", - limits: { - maxIterations: 40, - maxSeconds: 300, - maxContextShare: 0.8, - }, - }); - expect(adapter.buildInvocation(command)).toEqual({ - kind: "process", - executable: "coding-agent", - args: [ - "--base-url", - "http://gpu.lan:8000/v1", - "--model", - "qwen2.5-coder", - "--max-iterations", - "40", - "--max-seconds", - "300", - "--max-context-share", - "0.8", - "acp", - ], - }); + it("writes a file in the run's directory, streaming the tool call, its result and the tokens", async () => { + const cwd = await workspace(); + const model = standInModel([ + { content: "Writing it.", tool_calls: call("write_file", { path: "src/hello.txt", content: "hi" }) }, + { content: "Done." }, + ]); + const adapter = new LocalLlmAcpAdapter({ settings: { baseUrl: "http://gpu.lan:8000/v1", apiKey: "k" }, limits: {}, fetch: model.fetch, askBeforeCommands: false }); + const cmd = command(cwd); + + const events = await collect(adapter.execute(adapter.buildInvocation(cmd), cmd, "make hello.txt", new AbortController().signal)); + + expect(events[0]?.kind).toBe("started"); + expect(events.at(-1)?.kind).toBe("completed"); + const kinds = updates(events).map((u) => u.sessionUpdate); + expect(kinds).toEqual(["agent_message_chunk", "agent_message_chunk", "tool_call", "tool_call_update", "agent_message_chunk"]); + // The first update names the model and where it came from. + expect((updates(events)[0]?.content as { text: string }).text).toContain("Model stand-in-model (the model the server serves)"); + expect(updates(events)[2]).toMatchObject({ toolCallId: "c1", title: "write_file src/hello.txt", kind: "edit", status: "in_progress" }); + expect(updates(events)[3]).toMatchObject({ toolCallId: "c1", status: "completed" }); + expect(await readFile(path.join(cwd, "src", "hello.txt"), "utf8")).toBe("hi"); + expect(events.find((e) => e.kind === "usageReported")).toMatchObject({ usage: { inputTokens: 200, outputTokens: 20 } }); + // The stage's request carries the tools, the model and the key. + const chat = model.requests.find((r) => r.url.endsWith("/chat/completions")); + expect(chat?.url).toBe("http://gpu.lan:8000/v1/chat/completions"); + expect(chat?.body).toMatchObject({ model: "stand-in-model", tool_choice: "auto" }); }); - it("delegates execution to the shared ACP driver and forwards API key via env, not args", async () => { - async function* fakeEvents(): AsyncGenerator { - yield { kind: "started", runId: "run-local-llm-acp-1", timestamp: "t", command: "implement", cwd: "/workspace/repo" }; - yield { kind: "agentUpdate", runId: "run-local-llm-acp-1", timestamp: "t", update: { sessionUpdate: "tool_call" } }; - yield { kind: "completed", runId: "run-local-llm-acp-1", timestamp: "t" }; - } - runProcessMock.mockReturnValue(fakeEvents()); - - const adapter = new LocalLlmAcpAdapter({ - executable: "coding-agent", - baseUrl: "http://gpu.lan:8000/v1", - model: "qwen2.5-coder", - apiKey: "secret-key", - limits: {}, - }); - const invocation = adapter.buildInvocation(command); - - const events: Event[] = []; - for await (const e of adapter.execute(invocation, command, "FILE CONTENT HERE", new AbortController().signal)) { - events.push(e); - } - - expect(events.map((e) => e.kind)).toEqual(["started", "agentUpdate", "completed"]); - expect(runProcessMock).toHaveBeenCalledWith({ - executable: "coding-agent", - args: ["--base-url", "http://gpu.lan:8000/v1", "--model", "qwen2.5-coder", "acp"], - cwd: "/workspace/repo", - runId: "run-local-llm-acp-1", - commandKind: "implement", - prompt: expect.stringContaining("FILE CONTENT HERE"), - signal: expect.anything(), - env: { - CODING_AGENT_API_KEY: "secret-key", - CODING_AGENT_BASE_URL: "http://gpu.lan:8000/v1", - CODING_AGENT_MODEL: "qwen2.5-coder", - }, + it("uses the stage's model where the stage names one", async () => { + const cwd = await workspace(); + const model = standInModel([{ content: "ok" }]); + const adapter = new LocalLlmAcpAdapter({ settings: { baseUrl: "http://x", model: "from-settings" }, limits: {}, fetch: model.fetch, askBeforeCommands: false }); + const cmd = command(cwd, { model: "from-stage" }); + + await collect(adapter.execute(adapter.buildInvocation(cmd), cmd, "go", new AbortController().signal)); + + expect(model.requests.find((r) => r.url.endsWith("/chat/completions"))?.body).toMatchObject({ model: "from-stage" }); + expect(model.requests.some((r) => r.url.endsWith("/models"))).toBe(false); + }); + + it("reads a call the model wrote as text, as SGLang's hermes parser passes Qwen3.6's calls", async () => { + const cwd = await workspace(); + const model = standInModel([ + { content: "\n\n\na.txt\n\n\nx\n\n\n", tool_calls: null }, + { content: "Done." }, + ]); + const adapter = new LocalLlmAcpAdapter({ settings: { baseUrl: "http://x" }, limits: {}, fetch: model.fetch, askBeforeCommands: false }); + const cmd = command(cwd); + + await collect(adapter.execute(adapter.buildInvocation(cmd), cmd, "go", new AbortController().signal)); + + expect(await readFile(path.join(cwd, "a.txt"), "utf8")).toBe("x"); + }); + + it("refuses a path outside the run's directory, and tells the model", async () => { + const cwd = await workspace(); + const model = standInModel([{ tool_calls: call("write_file", { path: "../escaped.txt", content: "no" }) }, { content: "ok" }]); + const adapter = new LocalLlmAcpAdapter({ settings: { baseUrl: "http://x" }, limits: {}, fetch: model.fetch, askBeforeCommands: false }); + const cmd = command(cwd); + + const events = await collect(adapter.execute(adapter.buildInvocation(cmd), cmd, "go", new AbortController().signal)); + + expect(updates(events).find((u) => u.sessionUpdate === "tool_call_update")).toMatchObject({ status: "failed" }); + await expect(readFile(path.join(cwd, "..", "escaped.txt"), "utf8")).rejects.toThrow(); + }); + + it("asks before a command where told to, and runs it only when allowed", async () => { + const cwd = await workspace(); + const model = standInModel([{ tool_calls: call("run_command", { command: "node -e \"require('fs').writeFileSync('ran.txt','y')\"" }) }, { content: "ok" }]); + const adapter = new LocalLlmAcpAdapter({ settings: { baseUrl: "http://x" }, limits: {}, fetch: model.fetch, askBeforeCommands: true }); + const cmd = command(cwd); + + const events = await collect(adapter.execute(adapter.buildInvocation(cmd), cmd, "go", new AbortController().signal), (event) => { + if (event.kind === "permissionRequest") adapter.resolvePermission(cmd.runId, event.requestId, "allow"); }); + + expect(events.find((e) => e.kind === "permissionRequest")).toMatchObject({ description: expect.stringContaining("run_command node -e") }); + expect(await readFile(path.join(cwd, "ran.txt"), "utf8")).toBe("y"); }); - it("resolvePermission delegates to the shared driver", () => { - resolvePermissionMock.mockReturnValue(true); - const adapter = new LocalLlmAcpAdapter({ - executable: "coding-agent", - baseUrl: "http://gpu.lan:8000/v1", - model: "qwen2.5-coder", - limits: {}, + it("does not run a command the person denied", async () => { + const cwd = await workspace(); + const model = standInModel([{ tool_calls: call("run_command", { command: "node -e \"require('fs').writeFileSync('ran.txt','y')\"" }) }, { content: "ok" }]); + const adapter = new LocalLlmAcpAdapter({ settings: { baseUrl: "http://x" }, limits: {}, fetch: model.fetch, askBeforeCommands: true }); + const cmd = command(cwd); + + const events = await collect(adapter.execute(adapter.buildInvocation(cmd), cmd, "go", new AbortController().signal), (event) => { + if (event.kind === "permissionRequest") adapter.resolvePermission(cmd.runId, event.requestId, "deny"); }); - expect(adapter.resolvePermission("run-local-llm-acp-1", "perm-1", "allow")).toBe(true); - expect(resolvePermissionMock).toHaveBeenCalledWith("run-local-llm-acp-1", "perm-1", "allow"); + + expect(updates(events).find((u) => u.sessionUpdate === "tool_call_update")).toMatchObject({ status: "failed" }); + await expect(readFile(path.join(cwd, "ran.txt"), "utf8")).rejects.toThrow(); + }); + + it("stops at the iteration limit and says so", async () => { + const cwd = await workspace(); + const model = standInModel([{ tool_calls: call("list_dir", {}) }]); + const adapter = new LocalLlmAcpAdapter({ settings: { baseUrl: "http://x" }, limits: { maxIterations: 2 }, fetch: model.fetch, askBeforeCommands: false }); + const cmd = command(cwd); + + const events = await collect(adapter.execute(adapter.buildInvocation(cmd), cmd, "go", new AbortController().signal)); + + const said = updates(events).filter((u) => u.sessionUpdate === "agent_message_chunk").map((u) => (u.content as { text: string }).text).join(""); + expect(said).toContain("Stopped at the 2-iteration limit."); + expect(model.requests.filter((r) => r.url.endsWith("/chat/completions"))).toHaveLength(2); }); }); diff --git a/packages/core/src/agents/local-llm-acp.ts b/packages/core/src/agents/local-llm-acp.ts index 59912ac4..bd1f66bb 100644 --- a/packages/core/src/agents/local-llm-acp.ts +++ b/packages/core/src/agents/local-llm-acp.ts @@ -1,15 +1,25 @@ +// Adapter: the local LLM as a coding agent, run inside the product +// (local-llm-codes-in-process, ADR 0038). +// +// No process is started. The agent is an Agent Client Protocol agent built +// for each run (`createLocalAgent`) and driven by the shared +// `AcpSessionDriver`, so its text, tool calls, permission requests and +// usage reach the run as they do from an ACP CLI. It used to start an +// external `coding-agent` process, which a person had to find and install. + import type { AdapterInvocation, AgentAdapter } from "../agent-runner.js"; +import type { FetchLike } from "../direct-fetch.js"; +import type { LocalLlmAcpLimits, LocalLlmSettings } from "../local-llm-settings.js"; import type { Command, Event } from "../protocol.js"; -import type { LocalLlmAcpLimits } from "../local-llm-settings.js"; import { AcpSessionDriver } from "./acp-session-driver.js"; +import { createLocalAgent } from "./local-agent/acp-agent.js"; import { commandInstruction } from "./shared.js"; export interface LocalLlmAcpAdapterOptions { - executable: string; - baseUrl: string; - model: string; - apiKey?: string; + settings: LocalLlmSettings; limits: LocalLlmAcpLimits; + fetch: FetchLike; + askBeforeCommands: boolean; } export class LocalLlmAcpAdapter implements AgentAdapter { @@ -20,42 +30,28 @@ export class LocalLlmAcpAdapter implements AgentAdapter { constructor(private readonly options: LocalLlmAcpAdapterOptions) { } buildInvocation(_command: Command): AdapterInvocation { - // The options before the subcommand: `coding-agent` reads them only - // there, and `coding-agent acp --base-url ...` exits with its usage - // (fix-local-llm-acp). - const args = [ - "--base-url", - this.options.baseUrl, - "--model", - this.options.model, - ...renderLimits(this.options.limits), - "acp", - ]; - return { kind: "process", executable: this.options.executable, args }; + return { kind: "in-process", agent: this.name }; } async *execute(invocation: AdapterInvocation, command: Command, prompt: string, signal: AbortSignal): AsyncIterable { - if (invocation.kind !== "process") { - throw new Error("LocalLlmAcpAdapter expects invocation.kind === 'process'"); - } - - const env: Record = { - CODING_AGENT_BASE_URL: this.options.baseUrl, - CODING_AGENT_MODEL: this.options.model, - }; - if (this.options.apiKey !== undefined) { - env.CODING_AGENT_API_KEY = this.options.apiKey; + if (invocation.kind !== "in-process") { + throw new Error("LocalLlmAcpAdapter expects invocation.kind === 'in-process'"); } - - yield* this.driver.runProcess({ - executable: invocation.executable, - args: invocation.args, + const target = createLocalAgent({ + settings: this.options.settings, + ...(command.model !== undefined ? { stageModel: command.model } : {}), + limits: this.options.limits, + fetch: this.options.fetch, + askBeforeCommands: this.options.askBeforeCommands, + signal, + }); + yield* this.driver.run({ + target, cwd: command.cwd, runId: command.runId, commandKind: command.kind, prompt: `${commandInstruction(command.kind)}\n\n${prompt}`, signal, - env, }); } @@ -63,25 +59,3 @@ export class LocalLlmAcpAdapter implements AgentAdapter { return this.driver.resolvePermission(runId, requestId, outcome); } } - -function renderLimits(limits: LocalLlmAcpLimits): string[] { - const args: string[] = []; - const append = (flag: string, value: number | undefined) => { - if (value !== undefined) args.push(flag, String(value)); - }; - - append("--max-iterations", limits.maxIterations); - append("--max-tool-calls", limits.maxToolCalls); - append("--max-seconds", limits.maxSeconds); - append("--command-timeout-seconds", limits.commandTimeoutSeconds); - append("--max-command-output-chars", limits.maxCommandOutputChars); - append("--max-prompt-tokens", limits.maxPromptTokens); - append("--max-completion-tokens", limits.maxCompletionTokens); - append("--max-total-tokens", limits.maxTotalTokens); - append("--max-context-used-tokens", limits.maxContextUsedTokens); - append("--max-context-window-tokens", limits.maxContextWindowTokens); - append("--max-context-share", limits.maxContextShare); - append("--min-free-context-tokens", limits.minFreeContextTokens); - - return args; -} diff --git a/packages/core/src/agents/local-llm.test.ts b/packages/core/src/agents/local-llm.test.ts index d494b301..3b739f09 100644 --- a/packages/core/src/agents/local-llm.test.ts +++ b/packages/core/src/agents/local-llm.test.ts @@ -55,10 +55,12 @@ describe("LocalLlmAdapter", () => { events.push(e); } - expect(events.map((e) => e.kind)).toEqual(["started", "stdout", "stdout", "completed"]); - expect((events[1] as { chunk: string }).chunk).toBe("Hello"); - expect((events[2] as { chunk: string }).chunk).toBe(" world"); - expect((events[3] as { summary?: string }).summary).toBe("Hello world"); + // The first stdout names the model (local-llm-codes-in-process). + expect(events.map((e) => e.kind)).toEqual(["started", "stdout", "stdout", "stdout", "completed"]); + expect((events[1] as { chunk: string }).chunk).toContain("Model qwen (named in the local LLM settings)"); + expect((events[2] as { chunk: string }).chunk).toBe("Hello"); + expect((events[3] as { chunk: string }).chunk).toBe(" world"); + expect((events[4] as { summary?: string }).summary).toBe("Hello world"); const fetchMock = fetch as unknown as ReturnType; const [, init] = fetchMock.mock.calls[0] as [string, RequestInit]; @@ -97,7 +99,8 @@ describe("LocalLlmAdapter", () => { events.push(e); } - expect(events.map((e) => e.kind)).toEqual(["started", "stdout", "stdout", "completed"]); + // The first stdout names the model (local-llm-codes-in-process). + expect(events.map((e) => e.kind)).toEqual(["started", "stdout", "stdout", "stdout", "completed"]); }); it("emits failed on non-ok HTTP response", async () => { @@ -113,8 +116,8 @@ describe("LocalLlmAdapter", () => { events.push(e); } - expect(events.map((e) => e.kind)).toEqual(["started", "failed"]); - expect((events[1] as { reason: string }).reason).toContain("500"); + expect(events.map((e) => e.kind)).toEqual(["started", "stdout", "failed"]); + expect((events[2] as { reason: string }).reason).toContain("500"); }); it("emits failed when the network call itself throws", async () => { @@ -127,7 +130,7 @@ describe("LocalLlmAdapter", () => { events.push(e); } - expect(events.map((e) => e.kind)).toEqual(["started", "failed"]); - expect((events[1] as { reason: string }).reason).toBe("ECONNREFUSED"); + expect(events.map((e) => e.kind)).toEqual(["started", "stdout", "failed"]); + expect((events[2] as { reason: string }).reason).toBe("ECONNREFUSED"); }); }); diff --git a/packages/core/src/agents/local-llm.ts b/packages/core/src/agents/local-llm.ts index 550c295c..75cea2cb 100644 --- a/packages/core/src/agents/local-llm.ts +++ b/packages/core/src/agents/local-llm.ts @@ -8,7 +8,8 @@ import type { AdapterInvocation, AgentAdapter } from "../agent-runner.js"; import type { Command, Event } from "../protocol.js"; import { commandInstruction } from "./shared.js"; -import { chatCompletionsUrl, localLlmHeaders } from "../local-llm-settings.js"; +import type { FetchLike } from "../direct-fetch.js"; +import { chatCompletionsUrl, describeLocalLlmModel, localLlmHeaders, resolveLocalLlmModel } from "../local-llm-settings.js"; function nowIso(): string { return new Date().toISOString(); @@ -18,9 +19,14 @@ export interface LocalLlmAdapterOptions { /** The server's base URL, with its `/v1` or without, e.g. * http://hppii-gpu:30000 or http://hppii-gpu:8000/v1. */ baseUrl: string; - model: string; + /** Where the settings name one; otherwise the stage's, the server's or + * `default` (`resolveLocalLlmModel`, local-llm-codes-in-process). */ + model?: string; /** Sent as a bearer token, and nowhere else (the-local-llm-is-where-you-say). */ apiKey?: string; + /** How the server is reached: directly where agents ignore the system + * proxy (`localFetch`). The process's `fetch` where absent. */ + fetch?: FetchLike; } interface ChatCompletionChunk { @@ -56,13 +62,17 @@ export class LocalLlmAdapter implements AgentAdapter { yield { kind: "started", runId, timestamp: nowIso(), command: kind, cwd }; + const fetchImpl: FetchLike = this.options.fetch ?? ((input, init) => fetch(input, init)); + const model = await resolveLocalLlmModel(command.model, this.options, fetchImpl); + yield { kind: "stdout", runId, timestamp: nowIso(), chunk: `${describeLocalLlmModel(model)}\n\n` }; + let response: Response; try { - response = await fetch(invocation.url, { + response = await fetchImpl(invocation.url, { method: invocation.method, headers: localLlmHeaders(this.options), body: JSON.stringify({ - model: this.options.model, + model: model.model, stream: true, messages: [ { role: "system", content: commandInstruction(kind) }, diff --git a/packages/core/src/agents/proxy-policy.test.ts b/packages/core/src/agents/proxy-policy.test.ts new file mode 100644 index 00000000..bd82cde4 --- /dev/null +++ b/packages/core/src/agents/proxy-policy.test.ts @@ -0,0 +1,63 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +// local-llm-codes-in-process 4.2-4.3: what a CLI agent is started with when +// agents are told to ignore the system proxy. +const spawnMock = vi.fn(); +vi.mock("cross-spawn", () => ({ default: (...args: unknown[]) => spawnMock(...args) })); + +const { agentSpawnEnvironment, setAgentProxyPolicy, spawnAndStream } = await import("./shared.js"); +const { buildDefaultAgentRunners } = await import("../default-runners.js"); + +afterEach(() => { + setAgentProxyPolicy(false); + spawnMock.mockReset(); +}); + +describe("agentSpawnEnvironment", () => { + it("changes nothing while agents keep the system proxy", () => { + setAgentProxyPolicy(false); + expect(agentSpawnEnvironment()).toBeUndefined(); + }); + + it("removes the proxy variables and sets NO_PROXY=* when agents ignore it", () => { + vi.stubEnv("HTTPS_PROXY", "http://proxy:2080"); + vi.stubEnv("http_proxy", "http://proxy:2080"); + setAgentProxyPolicy(true); + const env = agentSpawnEnvironment({ EXTRA: "1" }); + expect(env?.HTTPS_PROXY).toBeUndefined(); + expect(env?.http_proxy).toBeUndefined(); + expect(env?.NO_PROXY).toBe("*"); + expect(env?.EXTRA).toBe("1"); + vi.unstubAllEnvs(); + }); + + it("reaches a CLI agent's spawn", async () => { + vi.stubEnv("HTTPS_PROXY", "http://proxy:2080"); + setAgentProxyPolicy(true); + spawnMock.mockImplementation(() => { + throw new Error("not started in a test"); + }); + for await (const _event of spawnAndStream({ executable: "claude", args: ["-p"], cwd: process.cwd(), runId: "r", commandKind: "plan" })) { /* drained */ } + const options = spawnMock.mock.calls[0]?.[2] as { env?: NodeJS.ProcessEnv }; + expect(options.env?.HTTPS_PROXY).toBeUndefined(); + expect(options.env?.NO_PROXY).toBe("*"); + vi.unstubAllEnvs(); + }); + + it("comes from OPENSPEC_UI_IGNORE_SYSTEM_PROXY where the host says nothing, as the standalone server and the CLI do", () => { + vi.stubEnv("OPENSPEC_UI_IGNORE_SYSTEM_PROXY", "1"); + buildDefaultAgentRunners({ workspaceRoot: process.cwd() }); + expect(agentSpawnEnvironment()?.NO_PROXY).toBe("*"); + // A host's own setting wins over the environment. + buildDefaultAgentRunners({ workspaceRoot: process.cwd(), ignoreSystemProxy: false }); + expect(agentSpawnEnvironment()).toBeUndefined(); + vi.unstubAllEnvs(); + }); + + it("is what buildDefaultAgentRunners is told", () => { + buildDefaultAgentRunners({ workspaceRoot: process.cwd(), ignoreSystemProxy: true }); + expect(agentSpawnEnvironment()?.NO_PROXY).toBe("*"); + buildDefaultAgentRunners({ workspaceRoot: process.cwd() }); + expect(agentSpawnEnvironment()).toBeUndefined(); + }); +}); diff --git a/packages/core/src/agents/registry.test.ts b/packages/core/src/agents/registry.test.ts index a2dc7d41..93d7f392 100644 --- a/packages/core/src/agents/registry.test.ts +++ b/packages/core/src/agents/registry.test.ts @@ -23,7 +23,7 @@ describe("AGENT_REGISTRY", () => { const codex = new CodexCliAdapter(); const gemini = new GeminiCliAdapter(); const localLlm = new LocalLlmAdapter({ baseUrl: "http://x", model: "m" }); - const localLlmAcp = new LocalLlmAcpAdapter({ executable: "coding-agent", baseUrl: "http://x", model: "m", limits: {} }); + const localLlmAcp = new LocalLlmAcpAdapter({ settings: { baseUrl: "http://x" }, limits: {}, fetch: async () => new Response(), askBeforeCommands: false }); const claudeAcp = new ClaudeCliAcpAdapter(); const copilotAcp = new CopilotCliAcpAdapter(); const codexAcp = new CodexCliAcpAdapter(); diff --git a/packages/core/src/agents/registry.ts b/packages/core/src/agents/registry.ts index 45841b56..0af4e35b 100644 --- a/packages/core/src/agents/registry.ts +++ b/packages/core/src/agents/registry.ts @@ -22,9 +22,14 @@ export interface AgentDescriptor { /** Matches the `AgentAdapter.name` of the corresponding adapter. */ id: string; label: string; - /** The CLI flag this adapter passes a model with; absent means this - * adapter accepts no model (see harness-step-models design.md). */ + /** The CLI flag this adapter passes a model with (see harness-step-models + * design.md). */ modelFlag?: string; + /** Whether a stage may name a model for this agent: true for every agent + * with a `modelFlag`, and for the local LLM agents, which take the model + * in their request rather than on a command line + * (local-llm-codes-in-process). Read through `acceptsModel`. */ + takesModel?: boolean; /** The CLI flag this adapter passes a custom agent with — a named * preset the person defined themselves. Absent means this adapter * accepts none, and offering one for it would be a setting nothing @@ -37,8 +42,9 @@ export const AGENT_REGISTRY: readonly AgentDescriptor[] = [ { id: "copilot-cli", label: "GitHub Copilot CLI", modelFlag: "--model", customAgentFlag: "--agent" }, { id: "codex-cli", label: "Codex CLI" }, { id: "gemini-cli", label: "Gemini CLI" }, - { id: "local-llm", label: "Local LLM (OpenAI-compatible)" }, - { id: "local-llm-acp", label: "Local LLM (ACP, OpenAI-compatible)" }, + { id: "local-llm", label: "Local LLM (OpenAI-compatible)", takesModel: true }, + // Runs inside the product: nothing to install (ADR 0038). + { id: "local-llm-acp", label: "Local LLM agent (OpenAI-compatible, built in)", takesModel: true }, // ACP-flavored adapters (acp-agent-adapters) — additional entries, not // replacements for the four above (see this file's header comment). { id: "copilot-cli-acp", label: "GitHub Copilot CLI (ACP)", modelFlag: "--model", customAgentFlag: "--agent" }, @@ -53,6 +59,11 @@ export const AGENT_REGISTRY: readonly AgentDescriptor[] = [ { id: "claude-cli-acp", label: "Claude CLI (ACP) — progress only, no permission gate", modelFlag: "--model", customAgentFlag: "--agent" }, ]; +/** Whether a stage may name a model for `descriptor`'s agent. */ +export function acceptsModel(descriptor: AgentDescriptor | undefined): boolean { + return descriptor !== undefined && (descriptor.modelFlag !== undefined || descriptor.takesModel === true); +} + /** Agent used when a `Command` does not specify `agentId`. Lives here * (not `default-runners.ts`) because it has no Node-only dependencies, so * it can be re-exported from `browser.ts` for the UI's agent picker. */ diff --git a/packages/core/src/agents/shared.ts b/packages/core/src/agents/shared.ts index 065d112c..4b469ca6 100644 --- a/packages/core/src/agents/shared.ts +++ b/packages/core/src/agents/shared.ts @@ -19,8 +19,35 @@ // the resulting command line in a shell. import crossSpawn from "cross-spawn"; import type { ChildProcessWithoutNullStreams } from "node:child_process"; +import { withoutSystemProxy } from "../direct-fetch.js"; import type { CommandKind, Event } from "../protocol.js"; +/** Whether a CLI agent is started without the system proxy (ADR 0038 + * decision 6). Process-wide: a host builds its runners once, from one + * configuration (`buildDefaultAgentRunners` sets it), and every adapter + * spawns through `spawnAndStream` or `spawnAcpProcess`, which read it. */ +let ignoreSystemProxyForAgents = false; + +export function setAgentProxyPolicy(ignoreSystemProxy: boolean): void { + ignoreSystemProxyForAgents = ignoreSystemProxy; +} + +/** Whether agents ignore the system proxy, as the host's runners were + * built: what agent detection checks the local LLM with. */ +export function agentsIgnoreSystemProxy(): boolean { + return ignoreSystemProxyForAgents; +} + +/** The environment an agent CLI is started with: the process's own, with + * `extra` laid over it, and without the proxy variables where agents are + * to ignore the system proxy. Undefined where nothing changes, so the spawn + * inherits the environment as it always did. */ +export function agentSpawnEnvironment(extra?: Readonly>): NodeJS.ProcessEnv | undefined { + if (!ignoreSystemProxyForAgents && extra === undefined) return undefined; + const merged = { ...process.env, ...(extra ?? {}) }; + return ignoreSystemProxyForAgents ? withoutSystemProxy(merged) : merged; +} + /** Ten seconds. Terminating a tree that can be terminated takes * milliseconds, so this is not a budget for the normal case — it is how * long to wait before admitting the process outlived the request. */ @@ -199,10 +226,12 @@ export async function* spawnAndStream(options: SpawnAndStreamOptions): AsyncGene } let child: ChildProcessWithoutNullStreams; + const env = agentSpawnEnvironment(); try { child = crossSpawn(executable, args, { cwd, stdio: ["pipe", "pipe", "pipe"], + ...(env !== undefined ? { env } : {}), // POSIX only: makes the child the leader of its own process group so // `terminateProcessTree` can kill the whole group. Windows tracks // parent/child relationships itself; `taskkill /T` needs no such flag. diff --git a/packages/core/src/default-runners.test.ts b/packages/core/src/default-runners.test.ts index 35afddd4..03fa5a22 100644 --- a/packages/core/src/default-runners.test.ts +++ b/packages/core/src/default-runners.test.ts @@ -41,12 +41,7 @@ describe("buildDefaultAllowlist", () => { const localLlmInvocation = new LocalLlmAdapter({ baseUrl: "http://x", model: "m" }).buildInvocation(command); expect(checkAllowlist("local-llm", localLlmInvocation, allowlist).allowed).toBe(true); - const localLlmAcpInvocation = new LocalLlmAcpAdapter({ - executable: "coding-agent", - baseUrl: "http://gpu.lan:8000/v1", - model: "qwen2.5-coder", - limits: {}, - }).buildInvocation(command); + const localLlmAcpInvocation = new LocalLlmAcpAdapter({ settings: { baseUrl: "http://x" }, limits: {}, fetch: async () => new Response(), askBeforeCommands: false }).buildInvocation(command); expect(checkAllowlist("local-llm-acp", localLlmAcpInvocation, allowlist).allowed).toBe(true); }); @@ -93,50 +88,13 @@ describe("buildDefaultAllowlist", () => { expect(decision.allowed).toBe(false); }); - it("rejects local-llm-acp invocation that omits --base-url", () => { + it("admits local-llm-acp only as itself, in process, and no process under its name", () => { + // ADR 0038: it starts nothing, so there is no command line to admit. const allowlist = buildDefaultAllowlist(); - const decision = checkAllowlist( - "local-llm-acp", - { kind: "process", executable: "coding-agent", args: ["--model", "qwen2.5-coder", "acp"] }, - allowlist, - ); - expect(decision.allowed).toBe(false); - }); - - it("rejects local-llm-acp invocation with the subcommand before its options", () => { - // The order `coding-agent` refuses, so the allowlist refuses it too. - const allowlist = buildDefaultAllowlist(); - const decision = checkAllowlist( - "local-llm-acp", - { kind: "process", executable: "coding-agent", args: ["acp", "--base-url", "http://gpu.lan:8000/v1", "--model", "qwen2.5-coder"] }, - allowlist, - ); - expect(decision.allowed).toBe(false); - }); - - it("allows local-llm-acp invocation with limits between the model and the subcommand", () => { - const allowlist = buildDefaultAllowlist(); - const invocation = new LocalLlmAcpAdapter({ - executable: "coding-agent", - baseUrl: "http://gpu.lan:8000/v1", - model: "qwen2.5-coder", - limits: { maxIterations: 40, maxContextShare: 0.8 }, - }).buildInvocation(command); - expect(checkAllowlist("local-llm-acp", invocation, allowlist).allowed).toBe(true); - }); - - it("rejects local-llm-acp invocation that appends an unknown flag", () => { - const allowlist = buildDefaultAllowlist(); - const decision = checkAllowlist( - "local-llm-acp", - { - kind: "process", - executable: "coding-agent", - args: ["--base-url", "http://gpu.lan:8000/v1", "--model", "qwen2.5-coder", "--sandbox", "off", "acp"], - }, - allowlist, - ); - expect(decision.allowed).toBe(false); + expect(checkAllowlist("local-llm-acp", { kind: "in-process", agent: "local-llm-acp" }, allowlist).allowed).toBe(true); + expect(checkAllowlist("local-llm-acp", { kind: "in-process", agent: "claude-cli" }, allowlist).allowed).toBe(false); + expect(checkAllowlist("local-llm-acp", { kind: "process", executable: "coding-agent", args: ["acp"] }, allowlist).allowed).toBe(false); + expect(checkAllowlist("claude-cli", { kind: "in-process", agent: "claude-cli" }, allowlist).allowed).toBe(false); }); it("rejects an invocation with extra/different args than the adapter builds", () => { diff --git a/packages/core/src/default-runners.ts b/packages/core/src/default-runners.ts index fcc1a608..7fb4ebb5 100644 --- a/packages/core/src/default-runners.ts +++ b/packages/core/src/default-runners.ts @@ -20,12 +20,14 @@ import { GeminiCliAdapter } from "./agents/gemini.js"; import { GeminiCliAcpAdapter } from "./agents/gemini-acp.js"; import { LocalLlmAcpAdapter } from "./agents/local-llm-acp.js"; import { LocalLlmAdapter } from "./agents/local-llm.js"; -import { resolveLocalLlmAcpSettings, resolveLocalLlmSettings } from "./local-llm-settings.js"; +import { AGENT_ENVIRONMENT, resolveLocalLlmAcpSettings, resolveLocalLlmSettings, switchFromEnvironment } from "./local-llm-settings.js"; import { DEFAULT_AGENT_ID } from "./agents/registry.js"; import { HARNESS_AGENT_CAPABILITIES, MODEL_ID_PATTERN } from "./harness-config.js"; import { createAgentRunner, type AgentRunner } from "./agent-runner.js"; import type { AllowlistConfig, AuditLog } from "./security.js"; -import { InMemoryAuditLog } from "./security.js"; +import { IN_PROCESS_SENTINEL, InMemoryAuditLog } from "./security.js"; +import { localFetch } from "./direct-fetch.js"; +import { setAgentProxyPolicy } from "./agents/shared.js"; export interface DefaultRunnersConfig { workspaceRoot: string; @@ -40,6 +42,13 @@ export interface DefaultRunnersConfig { /** Where each run's log is written (a-change-shows-its-run-logs). A host * passes its workspace's; absent, nothing is written. */ runLogs?: RunLogs; + /** Tell agents to ignore the system proxy (ADR 0038 decision 6): the + * local LLM agents connect directly, and a CLI agent is started without + * the proxy variables and with `NO_PROXY=*`. Off unless a host says so. */ + ignoreSystemProxy?: boolean; + /** The local agent asks through a permission request before each + * command (ADR 0038 decision 3). Off unless a host says so. */ + askBeforeCommands?: boolean; } export { DEFAULT_AGENT_ID }; @@ -91,33 +100,6 @@ function effortValidator(agentId: string): (value: string) => boolean { return (value) => accepted.includes(value); } -function nonEmpty(value: string): boolean { - return value.trim().length > 0; -} - -/** `--base-url --model [limits...] acp`: the options before - * the subcommand, which is last, as `coding-agent` reads them. */ -function localLlmAcpArgsAllowed(args: string[]): boolean { - if (args.length < 5) return false; - if (args[0] !== "--base-url" || args[2] !== "--model" || args[args.length - 1] !== "acp") return false; - if (!nonEmpty(args[1] ?? "") || !nonEmpty(args[3] ?? "")) return false; - const tail = args.slice(4, -1); - return exactWithOptionalArgs([], [ - { flag: "--max-iterations", validate: isPositiveInteger }, - { flag: "--max-tool-calls", validate: isPositiveInteger }, - { flag: "--max-seconds", validate: isPositiveInteger }, - { flag: "--command-timeout-seconds", validate: isPositiveInteger }, - { flag: "--max-command-output-chars", validate: isPositiveInteger }, - { flag: "--max-prompt-tokens", validate: isPositiveInteger }, - { flag: "--max-completion-tokens", validate: isPositiveInteger }, - { flag: "--max-total-tokens", validate: isPositiveInteger }, - { flag: "--max-context-used-tokens", validate: isPositiveInteger }, - { flag: "--max-context-window-tokens", validate: isPositiveInteger }, - { flag: "--max-context-share", validate: isPositiveDecimal }, - { flag: "--min-free-context-tokens", validate: isPositiveInteger }, - ])(tail); -} - /** Matches `-c model_reasoning_effort=""` for exactly codex's own * accepted levels — task 4.3: "match the whole pair including the key * ... and nothing else beginning with -c". Any other `-c key=value` @@ -157,10 +139,9 @@ export function buildDefaultAllowlist(): AllowlistConfig { }], "gemini-cli": [{ executable: "gemini", argsAllowed: exact(["--yolo"]) }], "local-llm": [{ executable: "__http__", argsAllowed: (args) => args[1] === "POST" }], - "local-llm-acp": [{ - executable: "coding-agent", - argsAllowed: localLlmAcpArgsAllowed, - }], + // Runs inside the product and starts no process (ADR 0038): the only + // thing to admit is the agent itself. + "local-llm-acp": [{ executable: IN_PROCESS_SENTINEL, argsAllowed: exact(["local-llm-acp"]) }], // `vscode-chat` is intentionally absent: it is a Harness step-runner // id that dispatches to VS Code chat and starts no subprocess, so // there is no executable/argv to allowlist. @@ -215,14 +196,28 @@ export function buildDefaultAgentRunners(config: DefaultRunnersConfig): Map { + await new Promise((resolve) => (server ? server.close(() => resolve()) : resolve())); + server = undefined; +}); + +describe("localFetch", () => { + it("goes direct even where the process's fetch has been sent through a proxy, as the editor host does", async () => { + server = createServer((_req, res) => { res.writeHead(200); res.end("direct"); }); + await new Promise((resolve) => server?.listen(0, "127.0.0.1", resolve)); + const address = server.address(); + const port = typeof address === "object" && address !== null ? address.port : 0; + const previous = getGlobalDispatcher(); + // A proxy that answers nothing, installed for the whole process. + setGlobalDispatcher(new ProxyAgent("http://127.0.0.1:9")); + try { + await expect(localFetch(false)(`http://127.0.0.1:${port}/`)).rejects.toThrow(); + expect(await (await localFetch(true)(`http://127.0.0.1:${port}/`)).text()).toBe("direct"); + } finally { + setGlobalDispatcher(previous); + } + }); + + it("reaches the server directly when told to ignore the system proxy, whatever the proxy variables say", async () => { + server = createServer((_req, res) => { res.writeHead(200); res.end("direct"); }); + await new Promise((resolve) => server?.listen(0, "127.0.0.1", resolve)); + const address = server.address(); + const port = typeof address === "object" && address !== null ? address.port : 0; + const saved = { HTTP_PROXY: process.env.HTTP_PROXY, HTTPS_PROXY: process.env.HTTPS_PROXY, NO_PROXY: process.env.NO_PROXY }; + // A proxy that answers nothing: a request through it would fail. + process.env.HTTP_PROXY = "http://127.0.0.1:9"; + process.env.HTTPS_PROXY = "http://127.0.0.1:9"; + delete process.env.NO_PROXY; + try { + const response = await localFetch(true)(`http://127.0.0.1:${port}/`); + expect(await response.text()).toBe("direct"); + } finally { + for (const [key, value] of Object.entries(saved)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + } + }); + + it("is the process's own fetch otherwise", async () => { + server = createServer((_req, res) => { res.writeHead(200); res.end("ok"); }); + await new Promise((resolve) => server?.listen(0, "127.0.0.1", resolve)); + const address = server.address(); + const port = typeof address === "object" && address !== null ? address.port : 0; + expect(await (await localFetch(false)(`http://127.0.0.1:${port}/`)).text()).toBe("ok"); + }); +}); + +describe("withoutSystemProxy", () => { + it("removes every proxy variable in either case and sets NO_PROXY=*", () => { + expect(withoutSystemProxy({ PATH: "p", HTTP_PROXY: "a", https_proxy: "b", All_Proxy: "c", no_proxy: "localhost" })).toEqual({ + PATH: "p", + NO_PROXY: "*", + no_proxy: "*", + }); + }); +}); diff --git a/packages/core/src/direct-fetch.ts b/packages/core/src/direct-fetch.ts new file mode 100644 index 00000000..f2fb1d73 --- /dev/null +++ b/packages/core/src/direct-fetch.ts @@ -0,0 +1,46 @@ +// Requests that may ignore the system proxy (local-llm-codes-in-process, +// ADR 0038 decision 6). +// +// The global `fetch` is not to be trusted to go direct: the editor's +// extension host applies the editor's proxy to Node's networking, and a +// standalone host may install a global dispatcher. So a request that must +// ignore the proxy goes through `undici`'s own `fetch` with a connection +// pool of its own, which nothing else in the process can replace. +// +// And the environment's proxy variables, which an agent CLI reads for +// itself, are removed from what a spawned agent is given. + +import { Agent, fetch as undiciFetch } from "undici"; + +export type FetchLike = (input: string, init?: RequestInit) => Promise; + +let directAgent: Agent | undefined; + +/** The `fetch` to reach the local LLM with: one that connects directly, + * whatever proxy the process has, when `ignoreSystemProxy` is set, and the + * process's own `fetch` otherwise. */ +export function localFetch(ignoreSystemProxy: boolean | undefined): FetchLike { + if (!ignoreSystemProxy) return (input, init) => fetch(input, init); + directAgent ??= new Agent(); + const dispatcher = directAgent; + return (input, init) => + undiciFetch(input, { ...(init as Record), dispatcher } as Parameters[1]) as unknown as Promise; +} + +const PROXY_VARIABLES = ["HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY"]; + +/** `env` with every proxy variable removed, in either case, and + * `NO_PROXY=*`: what an agent that reads the variables needs to go direct. + * An agent that reads none of them is unaffected, which is why each + * agent's capability row says whether it honours this. */ +export function withoutSystemProxy(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv { + const result: NodeJS.ProcessEnv = {}; + for (const [key, value] of Object.entries(env)) { + const upper = key.toUpperCase(); + if (PROXY_VARIABLES.includes(upper) || upper === "NO_PROXY") continue; + result[key] = value; + } + result.NO_PROXY = "*"; + result.no_proxy = "*"; + return result; +} diff --git a/packages/core/src/harness-config-schema.ts b/packages/core/src/harness-config-schema.ts index cc386148..a91dc5d6 100644 --- a/packages/core/src/harness-config-schema.ts +++ b/packages/core/src/harness-config-schema.ts @@ -15,7 +15,7 @@ // inside an object that the product would read and ignore. The same test // holds it to that with every sample the validator's own tests accept. -import { AGENT_REGISTRY } from "./agents/registry.js"; +import { acceptsModel, AGENT_REGISTRY } from "./agents/registry.js"; import { CHAIN_STEPS_REQUIRING_PARAM } from "./chain-steps.js"; import { TOP_LEVEL_CONFIG_KEYS } from "./harness-config.js"; import { CHAIN_STEP_NAMES, SKIPPABLE_STAGES, STAGES } from "./harness-stage.js"; @@ -61,7 +61,7 @@ function entryRulesFor(agentId: string): Schema { const descriptor = AGENT_REGISTRY.find((agent) => agent.id === agentId); const capabilities = HARNESS_AGENT_CAPABILITIES[agentId] ?? {}; const properties: Record = {}; - if (!descriptor?.modelFlag) properties.model = { ...NEVER, description: `${agentId} does not take a model.` }; + if (!acceptsModel(descriptor)) properties.model = { ...NEVER, description: `${agentId} does not take a model.` }; if (!descriptor?.customAgentFlag) properties.customAgent = { ...NEVER, description: `${agentId} does not take a custom agent.` }; const effort = capabilities.effort ?? []; properties.effort = effort.length === 0 diff --git a/packages/core/src/harness-config.test.ts b/packages/core/src/harness-config.test.ts index 2d3413f3..17d9b403 100644 --- a/packages/core/src/harness-config.test.ts +++ b/packages/core/src/harness-config.test.ts @@ -349,8 +349,18 @@ describe("stepAgents model support", () => { it("rejects a model set for an agent that accepts none, naming the stage and agent", async () => { const root = await temporaryRoot(); await expect( - writeGlobalHarnessConfig(root, { stepAgents: { apply: { agent: "local-llm", model: "some-model" } } }), - ).rejects.toThrow(/stepAgents\.apply.*"local-llm"/); + writeGlobalHarnessConfig(root, { stepAgents: { apply: { agent: "gemini-cli", model: "some-model" } } }), + ).rejects.toThrow(/stepAgents\.apply.*"gemini-cli"/); + }); + + it("accepts a model for both local LLM agents, with a slash in it (local-llm-codes-in-process)", async () => { + const root = await temporaryRoot(); + await expect(writeGlobalHarnessConfig(root, { + stepAgents: { + propose: { agent: "local-llm", model: "QuantTrio/Qwen3.6-35B-A3B-AWQ" }, + apply: { agent: "local-llm-acp", model: "QuantTrio/Qwen3.6-35B-A3B-AWQ" }, + }, + })).resolves.toBeUndefined(); }); it("lets a per-change harness.json model override the global one for that stage", async () => { diff --git a/packages/core/src/harness-config.ts b/packages/core/src/harness-config.ts index 12ef2989..b5aab5bb 100644 --- a/packages/core/src/harness-config.ts +++ b/packages/core/src/harness-config.ts @@ -1,6 +1,6 @@ import { mkdir, readFile, writeFile } from "node:fs/promises"; import path from "node:path"; -import { AGENT_REGISTRY } from "./agents/registry.js"; +import { acceptsModel, AGENT_REGISTRY } from "./agents/registry.js"; import { assertValidChangeName } from "./change-name.js"; import { CHAIN_STEPS_REQUIRING_PARAM } from "./chain-steps.js"; import type { ChangeLocation } from "./workbench.js"; @@ -604,7 +604,7 @@ function assertValidAgentEntry(label: string, entry: unknown, autonomyLevel: Har if (typeof model !== "string" || !MODEL_ID_PATTERN.test(model)) { throw new InvalidHarnessConfigError(`${label}.model "${String(model)}" is not a valid model id`); } - if (!AGENT_DESCRIPTORS_BY_ID.get(agentId)?.modelFlag) { + if (!acceptsModel(AGENT_DESCRIPTORS_BY_ID.get(agentId))) { throw new InvalidHarnessConfigError(`${label} sets a model, but agent "${agentId}" does not accept one`); } } diff --git a/packages/core/src/harness-step-agent.ts b/packages/core/src/harness-step-agent.ts index 0a8d9bb6..b2e8a521 100644 --- a/packages/core/src/harness-step-agent.ts +++ b/packages/core/src/harness-step-agent.ts @@ -107,8 +107,10 @@ export function stepAgentFor( * `-` (so it can never be read as a second flag) and cannot contain * whitespace or quotes (so it can never become a shell/quoting escape) * — see harness-step-models design.md, "Validation is a closed - * character set, not an escape". */ -export const MODEL_ID_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._:-]*$/; + * character set, not an escape". A / is admitted: a local server names its + * models as Hugging Face does, QuantTrio/Qwen3.6-35B-A3B-AWQ, and the + * character escapes nothing (local-llm-codes-in-process). */ +export const MODEL_ID_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._:/-]*$/; /** The union of every reasoning-effort value any registered agent * accepts, not the intersection — see harness-step-effort-and-budget @@ -174,6 +176,13 @@ export interface HarnessAgentCapabilities { * agent nobody has watched, for the same reason `reports` is. * Absent means `"unknown"`. */ contextGauge?: "sends" | "none" | "unknown"; + /** Whether this agent goes direct when agents are told to ignore the + * system proxy (ADR 0038 decision 6). `"ignored"`: it runs in the + * product, which connects directly. `"environment"`: a CLI documented to + * read `HTTP_PROXY`/`HTTPS_PROXY`/`NO_PROXY`, which the product removes + * and sets. `"unknown"`: a CLI nobody has checked, which may keep its + * own proxy settings. Absent means `"unknown"`. */ + systemProxy?: "ignored" | "environment" | "unknown"; } /** Live-verified for `claude-cli`/`copilot-cli` (`--help` on this @@ -200,26 +209,32 @@ export const HARNESS_AGENT_CAPABILITIES: Readonly { .toEqual({ baseUrl: "http://gpu.lan:8000/v1", model: "told", apiKey: "environment-key" }); }); - it("falls back to where the adapter always looked, with no key", () => { - expect(resolveLocalLlmSettings({}, {})).toEqual({ baseUrl: LOCAL_LLM_DEFAULT_BASE_URL, model: LOCAL_LLM_DEFAULT_MODEL }); + it("falls back to where the adapter always looked, with no key and no model", () => { + // The model is found when a run starts (local-llm-codes-in-process). + expect(resolveLocalLlmSettings({}, {})).toEqual({ baseUrl: LOCAL_LLM_DEFAULT_BASE_URL }); }); it("reads an empty value as none, so a cleared setting never sends an empty key", () => { expect(resolveLocalLlmSettings({ baseUrl: " ", apiKey: "" }, { OPENSPEC_UI_LOCAL_LLM_API_KEY: " " })) - .toEqual({ baseUrl: LOCAL_LLM_DEFAULT_BASE_URL, model: LOCAL_LLM_DEFAULT_MODEL }); + .toEqual({ baseUrl: LOCAL_LLM_DEFAULT_BASE_URL }); }); }); -describe("chatCompletionsUrl", () => { - it("accepts a base with its /v1 or without, and a trailing slash", () => { +describe("chatCompletionsUrl and modelsUrl", () => { + it("accept a base with its /v1 or without, and a trailing slash", () => { expect(chatCompletionsUrl("http://gpu.lan:8000/v1")).toBe("http://gpu.lan:8000/v1/chat/completions"); expect(chatCompletionsUrl("http://gpu.lan:8000/v1/")).toBe("http://gpu.lan:8000/v1/chat/completions"); expect(chatCompletionsUrl("http://gpu.lan:30000")).toBe("http://gpu.lan:30000/v1/chat/completions"); expect(chatCompletionsUrl("http://gpu.lan:30000/")).toBe("http://gpu.lan:30000/v1/chat/completions"); + expect(modelsUrl("http://gpu.lan:8000/v1/")).toBe("http://gpu.lan:8000/v1/models"); + expect(modelsUrl("http://gpu.lan:30000")).toBe("http://gpu.lan:30000/v1/models"); }); }); @@ -55,15 +60,10 @@ describe("localLlmHeaders", () => { }); describe("resolveLocalLlmAcpSettings", () => { - it("reuses base endpoint settings and defaults executable", () => { + it("reuses the endpoint settings", () => { expect(resolveLocalLlmAcpSettings({ model: "qwen2.5-coder" }, { OPENSPEC_UI_LOCAL_LLM_BASE_URL: "http://gpu.lan:8000/v1", - })).toEqual({ - baseUrl: "http://gpu.lan:8000/v1", - model: "qwen2.5-coder", - executable: LOCAL_LLM_ACP_DEFAULT_EXECUTABLE, - limits: {}, - }); + })).toEqual({ baseUrl: "http://gpu.lan:8000/v1", model: "qwen2.5-coder", limits: {} }); }); it("takes ACP limits from environment when host does not override", () => { @@ -73,13 +73,7 @@ describe("resolveLocalLlmAcpSettings", () => { OPENSPEC_UI_LOCAL_LLM_ACP_MAX_TOTAL_TOKENS: "120000", })).toEqual({ baseUrl: LOCAL_LLM_DEFAULT_BASE_URL, - model: LOCAL_LLM_DEFAULT_MODEL, - executable: LOCAL_LLM_ACP_DEFAULT_EXECUTABLE, - limits: { - maxIterations: 42, - maxContextShare: 0.8, - maxTotalTokens: 120000, - }, + limits: { maxIterations: 42, maxContextShare: 0.8, maxTotalTokens: 120000 }, }); }); @@ -89,12 +83,7 @@ describe("resolveLocalLlmAcpSettings", () => { }, { OPENSPEC_UI_LOCAL_LLM_ACP_MAX_ITERATIONS: "42", OPENSPEC_UI_LOCAL_LLM_ACP_MAX_SECONDS: "600", - })).toEqual({ - baseUrl: LOCAL_LLM_DEFAULT_BASE_URL, - model: LOCAL_LLM_DEFAULT_MODEL, - executable: LOCAL_LLM_ACP_DEFAULT_EXECUTABLE, - limits: { maxIterations: 7, maxSeconds: 60 }, - }); + })).toEqual({ baseUrl: LOCAL_LLM_DEFAULT_BASE_URL, limits: { maxIterations: 7, maxSeconds: 60 } }); }); it("drops invalid numeric ACP limits instead of guessing", () => { @@ -105,3 +94,55 @@ describe("resolveLocalLlmAcpSettings", () => { }).limits).toEqual({}); }); }); + +// local-llm-codes-in-process 3.3: the stage, then the settings, then the +// server, then `default`. +describe("resolveLocalLlmModel", () => { + const served = (ids: string[]) => vi.fn(async () => new Response(JSON.stringify({ data: ids.map((id) => ({ id })) }), { status: 200 })); + + it("takes the stage's model first", async () => { + const fetchImpl = served(["from-server"]); + expect(await resolveLocalLlmModel("from-stage", { baseUrl: "http://a:1", model: "from-settings" }, fetchImpl)) + .toEqual({ model: "from-stage", source: "stage" }); + expect(fetchImpl).not.toHaveBeenCalled(); + }); + + it("takes the settings' model when the stage names none", async () => { + expect(await resolveLocalLlmModel(undefined, { baseUrl: "http://a:2", model: "from-settings" }, served(["x"]))) + .toEqual({ model: "from-settings", source: "settings" }); + }); + + it("asks the server when nothing names one, with the key, and takes the first it serves", async () => { + const fetchImpl = served(["QuantTrio/Qwen3.6-35B-A3B-AWQ", "other"]); + expect(await resolveLocalLlmModel(undefined, { baseUrl: "http://a:3/v1", apiKey: "k" }, fetchImpl)) + .toEqual({ model: "QuantTrio/Qwen3.6-35B-A3B-AWQ", source: "server" }); + expect(fetchImpl).toHaveBeenCalledWith("http://a:3/v1/models", expect.objectContaining({ + headers: expect.objectContaining({ authorization: "Bearer k" }), + })); + }); + + it("asks the same server once", async () => { + const fetchImpl = served(["m"]); + await resolveLocalLlmModel(undefined, { baseUrl: "http://a:4" }, fetchImpl); + await resolveLocalLlmModel(undefined, { baseUrl: "http://a:4" }, fetchImpl); + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); + + it("falls back to default when the server cannot be asked, and asks again next time", async () => { + const failing = vi.fn(async () => { throw new Error("ECONNREFUSED"); }); + expect(await resolveLocalLlmModel(undefined, { baseUrl: "http://a:5" }, failing)).toEqual({ model: "default", source: "default" }); + await resolveLocalLlmModel(undefined, { baseUrl: "http://a:5" }, failing); + expect(failing).toHaveBeenCalledTimes(2); + }); + + it("says which model and why", () => { + expect(describeLocalLlmModel({ model: "m", source: "server" })).toBe("Model m (the model the server serves)."); + }); +}); + +describe("switchFromEnvironment", () => { + it("reads 1, true, yes and on as on, anything else as off", () => { + for (const on of ["1", "true", "YES", " on "]) expect(switchFromEnvironment(on)).toBe(true); + for (const off of [undefined, "", "0", "false", "no"]) expect(switchFromEnvironment(off)).toBe(false); + }); +}); diff --git a/packages/core/src/local-llm-settings.ts b/packages/core/src/local-llm-settings.ts index d11471e7..f0e4af5c 100644 --- a/packages/core/src/local-llm-settings.ts +++ b/packages/core/src/local-llm-settings.ts @@ -36,11 +36,11 @@ export const LOCAL_LLM_ACP_ENVIRONMENT = { minFreeContextTokens: "OPENSPEC_UI_LOCAL_LLM_ACP_MIN_FREE_CONTEXT_TOKENS", } as const; -export const LOCAL_LLM_ACP_DEFAULT_EXECUTABLE = "coding-agent"; - export interface LocalLlmSettings { baseUrl: string; - model: string; + /** Where a host or the environment named one. Absent otherwise: the run + * then asks the server (`resolveLocalLlmModel`). */ + model?: string; /** Sent as a bearer token where set; a server that wants none gets none. */ apiKey?: string; } @@ -61,7 +61,6 @@ export interface LocalLlmAcpLimits { } export interface LocalLlmAcpSettings extends LocalLlmSettings { - executable: string; limits: LocalLlmAcpLimits; } @@ -111,9 +110,10 @@ export function resolveLocalLlmSettings( environment: Readonly> = process.env, ): LocalLlmSettings { const apiKey = given(overrides.apiKey) ?? given(environment[LOCAL_LLM_ENVIRONMENT.apiKey]); + const model = given(overrides.model) ?? given(environment[LOCAL_LLM_ENVIRONMENT.model]); return { baseUrl: given(overrides.baseUrl) ?? given(environment[LOCAL_LLM_ENVIRONMENT.baseUrl]) ?? LOCAL_LLM_DEFAULT_BASE_URL, - model: given(overrides.model) ?? given(environment[LOCAL_LLM_ENVIRONMENT.model]) ?? LOCAL_LLM_DEFAULT_MODEL, + ...(model !== undefined ? { model } : {}), ...(apiKey !== undefined ? { apiKey } : {}), }; } @@ -167,11 +167,97 @@ export function resolveLocalLlmAcpSettings( Object.entries(resolvedLimits).filter(([, value]) => value !== undefined), ) as LocalLlmAcpLimits; - return { - ...base, - executable: LOCAL_LLM_ACP_DEFAULT_EXECUTABLE, - limits, + return { ...base, limits }; +} + +/** Where a run's model came from, said in its first update. */ +export type LocalLlmModelSource = "stage" | "settings" | "server" | "default"; + +export interface ResolvedLocalLlmModel { + model: string; + source: LocalLlmModelSource; +} + +/** The models endpoint of a base URL, written either way, as + * `chatCompletionsUrl` reads it. */ +export function modelsUrl(baseUrl: string): string { + const base = baseUrl.trim().replace(/\/+$/u, ""); + return /\/v1$/u.test(base) ? `${base}/models` : `${base}/v1/models`; +} + +const SERVED_MODELS_TIMEOUT_MS = 10_000; +const servedModels = new Map>(); + +/** The models the server lists at `/v1/models`, asked once per base URL + * for the life of the process, so a chain asks once. Undefined where the + * server could not be asked or answered with no list. */ +export function listServedModels( + settings: Pick, + fetchImpl: (input: string, init?: RequestInit) => Promise, +): Promise { + const url = modelsUrl(settings.baseUrl); + let pending = servedModels.get(url); + if (pending === undefined) { + pending = (async () => { + try { + const response = await fetchImpl(url, { + headers: localLlmHeaders(settings), + signal: AbortSignal.timeout(SERVED_MODELS_TIMEOUT_MS), + }); + if (!response.ok) return undefined; + const body = (await response.json()) as { data?: Array<{ id?: unknown }> }; + const ids = (body.data ?? []).map((entry) => entry.id).filter((id): id is string => typeof id === "string" && id.length > 0); + return ids.length > 0 ? ids : undefined; + } catch { + return undefined; + } + })(); + servedModels.set(url, pending); + // A failed answer is not kept: the server may be up at the next run. + void pending.then((ids) => { + if (ids === undefined) servedModels.delete(url); + }); + } + return pending; +} + +/** The model a local LLM run uses (local-llm-codes-in-process, ADR 0038 + * decision 5): the stage's, else the settings', else the one the server + * serves (the first, where it serves several), else `default`. */ +export async function resolveLocalLlmModel( + stageModel: string | undefined, + settings: LocalLlmSettings, + fetchImpl: (input: string, init?: RequestInit) => Promise, +): Promise { + const fromStage = given(stageModel); + if (fromStage !== undefined) return { model: fromStage, source: "stage" }; + const fromSettings = given(settings.model); + if (fromSettings !== undefined) return { model: fromSettings, source: "settings" }; + const served = await listServedModels(settings, fetchImpl); + if (served?.[0] !== undefined) return { model: served[0], source: "server" }; + return { model: LOCAL_LLM_DEFAULT_MODEL, source: "default" }; +} + +/** The sentence a run starts with, naming its model and why that one. */ +export function describeLocalLlmModel(resolved: ResolvedLocalLlmModel): string { + const why: Record = { + stage: "named by the stage", + settings: "named in the local LLM settings", + server: "the model the server serves", + default: "no model was named and the server listed none", }; + return `Model ${resolved.model} (${why[resolved.source]}).`; +} + +/** Variables a host with no settings of its own reads for the agents. */ +export const AGENT_ENVIRONMENT = { + ignoreSystemProxy: "OPENSPEC_UI_IGNORE_SYSTEM_PROXY", + askBeforeCommands: "OPENSPEC_UI_LOCAL_LLM_ASK_BEFORE_COMMANDS", +} as const; + +/** A switch from the environment: `1`, `true`, `yes` or `on`, in any case. */ +export function switchFromEnvironment(value: string | undefined): boolean { + return /^(1|true|yes|on)$/iu.test(value?.trim() ?? ""); } /** The chat completions endpoint of a base URL written either way an diff --git a/packages/core/src/security.ts b/packages/core/src/security.ts index 65181e48..ec08a52d 100644 --- a/packages/core/src/security.ts +++ b/packages/core/src/security.ts @@ -35,6 +35,11 @@ export interface AllowlistDecision { reason?: string; } +/** The allowlist's executable for an agent that runs in process and starts + * nothing; its one argument is the agent's own name + * (local-llm-codes-in-process). */ +export const IN_PROCESS_SENTINEL = "__in_process__"; + export function checkAllowlist( agentName: string, invocation: AdapterInvocation, @@ -44,6 +49,13 @@ export function checkAllowlist( if (!rules || rules.length === 0) { return { allowed: false, reason: `Agent "${agentName}" is not present in the workspace allowlist` }; } + if (invocation.kind === "in-process") { + const rule = rules.find((r) => r.executable === IN_PROCESS_SENTINEL); + if (!rule || !rule.argsAllowed([invocation.agent])) { + return { allowed: false, reason: `Agent "${agentName}" is not permitted to run in process` }; + } + return { allowed: true }; + } if (invocation.kind === "http") { const rule = rules.find((r) => r.executable === "__http__"); if (!rule) { diff --git a/packages/extension/package.json b/packages/extension/package.json index 70e22366..e06e7378 100644 --- a/packages/extension/package.json +++ b/packages/extension/package.json @@ -712,7 +712,17 @@ "openspec-ui.localLlm.model": { "type": "string", "default": "", - "description": "The model the local LLM (agent local-llm) is asked for, as its server names it. Empty means the environment variable OPENSPEC_UI_LOCAL_LLM_MODEL, and then \"default\". Read when the window opens." + "description": "The model the local LLM agents (local-llm and local-llm-acp) ask for, as the server names it, such as QuantTrio/Qwen3.6-35B-A3B-AWQ. Optional: a stage's own model wins over it, and where neither names one the server is asked which model it serves (its /v1/models). Empty means the environment variable OPENSPEC_UI_LOCAL_LLM_MODEL, then the server. Read when the window opens." + }, + "openspec-ui.localLlm.agent.askBeforeCommands": { + "type": "boolean", + "default": false, + "description": "The built-in local LLM agent (local-llm-acp) asks before each command it runs, and runs it only if you allow it. Off: commands run without asking, as the other agents' do, inside the change's working directory and within their time limit. Leave it off for a chain that runs unattended: nobody would answer. Same as OPENSPEC_UI_LOCAL_LLM_ASK_BEFORE_COMMANDS=1. Read when the window opens." + }, + "openspec-ui.agents.ignoreSystemProxy": { + "type": "boolean", + "default": false, + "description": "Agents ignore the system proxy. The local LLM agents (local-llm, local-llm-acp) and their availability check connect directly, whatever proxy the editor applies; a CLI agent is started with HTTP_PROXY, HTTPS_PROXY and ALL_PROXY removed and NO_PROXY=*, which only a CLI that reads those variables honours (see HARNESS.md). Turn it on when the proxy cannot reach your model, such as a server on your LAN. Same as OPENSPEC_UI_IGNORE_SYSTEM_PROXY=1. Read when the window opens." }, "openspec-ui.followSelectionInChangeGraph": { "type": "boolean", diff --git a/packages/extension/schemas/agent-harness.schema.json b/packages/extension/schemas/agent-harness.schema.json index 1624d011..a6c153d4 100644 --- a/packages/extension/schemas/agent-harness.schema.json +++ b/packages/extension/schemas/agent-harness.schema.json @@ -281,7 +281,7 @@ }, "model": { "type": "string", - "pattern": "^[A-Za-z0-9][A-Za-z0-9._:-]*$", + "pattern": "^[A-Za-z0-9][A-Za-z0-9._:/-]*$", "description": "The model the agent runs. Only for an agent that takes a model flag." }, "effort": { @@ -316,7 +316,7 @@ }, "customAgent": { "type": "string", - "pattern": "^[A-Za-z0-9][A-Za-z0-9._:-]*$", + "pattern": "^[A-Za-z0-9][A-Za-z0-9._:/-]*$", "description": "A custom agent definition the agent loads. Only for an agent that takes one." } }, @@ -503,10 +503,6 @@ }, "then": { "properties": { - "model": { - "not": {}, - "description": "local-llm does not take a model." - }, "customAgent": { "not": {}, "description": "local-llm does not take a custom agent." @@ -535,10 +531,6 @@ }, "then": { "properties": { - "model": { - "not": {}, - "description": "local-llm-acp does not take a model." - }, "customAgent": { "not": {}, "description": "local-llm-acp does not take a custom agent." diff --git a/packages/extension/schemas/change-harness.schema.json b/packages/extension/schemas/change-harness.schema.json index d6ea21f7..83a5576d 100644 --- a/packages/extension/schemas/change-harness.schema.json +++ b/packages/extension/schemas/change-harness.schema.json @@ -424,7 +424,7 @@ }, "model": { "type": "string", - "pattern": "^[A-Za-z0-9][A-Za-z0-9._:-]*$", + "pattern": "^[A-Za-z0-9][A-Za-z0-9._:/-]*$", "description": "The model the agent runs. Only for an agent that takes a model flag." }, "effort": { @@ -459,7 +459,7 @@ }, "customAgent": { "type": "string", - "pattern": "^[A-Za-z0-9][A-Za-z0-9._:-]*$", + "pattern": "^[A-Za-z0-9][A-Za-z0-9._:/-]*$", "description": "A custom agent definition the agent loads. Only for an agent that takes one." } }, @@ -646,10 +646,6 @@ }, "then": { "properties": { - "model": { - "not": {}, - "description": "local-llm does not take a model." - }, "customAgent": { "not": {}, "description": "local-llm does not take a custom agent." @@ -678,10 +674,6 @@ }, "then": { "properties": { - "model": { - "not": {}, - "description": "local-llm-acp does not take a model." - }, "customAgent": { "not": {}, "description": "local-llm-acp does not take a custom agent." diff --git a/packages/extension/src/extension.ts b/packages/extension/src/extension.ts index eb1841f1..16d10820 100644 --- a/packages/extension/src/extension.ts +++ b/packages/extension/src/extension.ts @@ -156,6 +156,18 @@ export interface ExtensionTestApi { * this name (the-local-llm-is-where-you-say). */ const LOCAL_LLM_API_KEY_SECRET = "openspec-ui.localLlm.apiKey"; +/** The two agent switches, from the settings, for every place this host + * builds runners (ADR 0038). Off unless a person turned them on; the + * environment's values apply only to a host with no settings. */ +export function readAgentSwitches(): { ignoreSystemProxy: boolean; askBeforeCommands: boolean } { + const agents = vscode.workspace.getConfiguration("openspec-ui.agents"); + const localAgent = vscode.workspace.getConfiguration("openspec-ui.localLlm.agent"); + return { + ignoreSystemProxy: agents.get("ignoreSystemProxy", false) === true, + askBeforeCommands: localAgent.get("askBeforeCommands", false) === true, + }; +} + export async function activate(context: vscode.ExtensionContext): Promise { const outputChannel = vscode.window.createOutputChannel("OpenSpec Workbench"); context.subscriptions.push(outputChannel); @@ -510,6 +522,7 @@ export async function activate(context: vscode.ExtensionContext): Promise 0 ? { localLlmBaseUrl } : {}), ...(localLlmModel.length > 0 ? { localLlmModel } : {}), ...(localLlmApiKey !== undefined && localLlmApiKey.length > 0 ? { localLlmApiKey } : {}), + ...readAgentSwitches(), }); context.subscriptions.push( vscode.commands.registerCommand("openspec-ui.setLocalLlmApiKey", async () => { diff --git a/packages/extension/src/optional-server.ts b/packages/extension/src/optional-server.ts index bee6a388..37bfcb3e 100644 --- a/packages/extension/src/optional-server.ts +++ b/packages/extension/src/optional-server.ts @@ -9,6 +9,9 @@ export class OptionalServerManager { constructor( private readonly workspaceRoot: string, private readonly distDir = path.resolve("dist"), + /** The agent switches the editor's settings say (ADR 0038): passed on, + * so the runners of this server agree with the window's own. */ + private readonly agentSwitches: { ignoreSystemProxy?: boolean; askBeforeCommands?: boolean } = {}, ) { } get isRunning(): boolean { @@ -41,7 +44,7 @@ export class OptionalServerManager { host: "127.0.0.1", port: 0, auditLog, - runners: buildDefaultAgentRunners({ workspaceRoot: this.workspaceRoot, auditLog, runLogs: createFileRunLogs(this.workspaceRoot) }), + runners: buildDefaultAgentRunners({ workspaceRoot: this.workspaceRoot, auditLog, runLogs: createFileRunLogs(this.workspaceRoot), ...this.agentSwitches }), staticAssets: { indexHtmlPath: path.join(this.distDir, "standalone", "index.html"), appJsPath: path.join(this.distDir, "standalone", "app.js"), diff --git a/packages/extension/src/webview/ai-panel.test.ts b/packages/extension/src/webview/ai-panel.test.ts index 41c75fe8..3d686d73 100644 --- a/packages/extension/src/webview/ai-panel.test.ts +++ b/packages/extension/src/webview/ai-panel.test.ts @@ -427,6 +427,62 @@ describe("AiPanel harness process tracking", () => { expect(resolveRunner).toHaveBeenCalledWith("copilot-cli-acp"); }); + it("routes a resolvePermission to the runner that owns the run, not to the default agent", () => { + // Same bug as the cancel case above, found while verifying + // local-llm-codes-in-process 6.4: a webview control answering + // Allow/Deny knows only the `runId` and `requestId` the + // `permissionRequest` event carried, which name no agent. Without + // reusing the run's remembered agent, this resolved to + // DEFAULT_AGENT_ID's own fresh adapter instance — whose + // `resolvePermission()` had never heard of this run's pending + // request, so it silently did nothing. The real adapter's + // `session/request_permission` promise never settled: the run + // sat on "Loading…" forever, and the task that asked was never + // ticked. + const panel = createPanelFixture(); + const runController = { + onEvent: vi.fn(() => vi.fn()), + run: vi.fn(), + }; + const resolveRunner = vi.fn((agentId: string | undefined) => ({ name: agentId ?? "claude-cli", run: vi.fn() })); + const aiPanel = new AiPanel({ + extensionUri: vscodeMock.Uri.file("/extension") as never, + runController: runController as never, + resolveRunner: resolveRunner as never, + chainRunner: createFakeChainRunner() as never, + getLocalServerUrl: () => undefined, + }); + aiPanel.reveal(); + const receiveMessage = panel.webview.onDidReceiveMessage.mock.calls[0]?.[0] as (message: unknown) => void; + + receiveMessage({ + type: "openspec-ui/command", + command: { + kind: "implement", + cwd: "/repo", + context: { changeDir: "/repo/openspec/changes/demo" }, + runId: "run-acp", + agentId: "local-llm-acp", + }, + }); + resolveRunner.mockClear(); + + // The webview's Allow/Deny carries no agentId — it does not know one. + receiveMessage({ + type: "openspec-ui/command", + command: { + kind: "resolvePermission", + cwd: "/repo", + context: { changeDir: "/repo/openspec/changes/demo" }, + runId: "run-acp", + permissionRequestId: "req-1", + permissionOutcome: "allow", + }, + }); + + expect(resolveRunner).toHaveBeenCalledWith("local-llm-acp"); + }); + it("does not register a process when no scheduler is supplied", () => { const panel = createPanelFixture(); const runController = { onEvent: vi.fn(() => vi.fn()), run: vi.fn() }; diff --git a/packages/extension/src/webview/ai-panel.ts b/packages/extension/src/webview/ai-panel.ts index 4326b728..0156021e 100644 --- a/packages/extension/src/webview/ai-panel.ts +++ b/packages/extension/src/webview/ai-panel.ts @@ -12,6 +12,7 @@ import { VSCODE_CHAT_STEP_AGENT_ID, type AgentRunner, type Command, + type CommandKind, type Event, type HarnessChainRunner, type HarnessStage, @@ -362,10 +363,18 @@ export class AiPanel { } } - // A cancel goes to the runner that owns the run, not to whichever - // agent `DEFAULT_AGENT_ID` names. `activeRuns` lives per runner - // instance, so asking the wrong one is the same as not asking. - const agentId = command.kind === "cancel" + // A cancel or a permission answer goes to the runner that owns the + // run, not to whichever agent `DEFAULT_AGENT_ID` names. `activeRuns` + // lives per runner instance, so asking the wrong one is the same as + // not asking — observed 2026-10-02: a `resolvePermission` command + // carries no `agentId` a webview control could know (the + // `permissionRequest` event it answers never named one), so it + // resolved to the default runner's own fresh adapter instance, whose + // `resolvePermission()` found no such pending request and silently + // did nothing. The run itself sat on the ACP agent's still-open + // promise forever; `askBeforeCommands`'s Allow had nothing to unblock. + const CARRIES_NO_OWN_AGENT_ID = new Set(["cancel", "resolvePermission"]); + const agentId = CARRIES_NO_OWN_AGENT_ID.has(command.kind) ? this.runAgentIds.get(command.runId) ?? command.agentId : command.agentId; const runner = this.deps.resolveRunner(agentId); @@ -378,7 +387,7 @@ export class AiPanel { }); return; } - if (command.kind !== "cancel") this.runAgentIds.set(command.runId, agentId); + if (!CARRIES_NO_OWN_AGENT_ID.has(command.kind)) this.runAgentIds.set(command.runId, agentId); this.trackHarnessProcess(command); void this.deps.runController.run(runner, command); } diff --git a/packages/webui/src/components/harness-settings-parts.test.tsx b/packages/webui/src/components/harness-settings-parts.test.tsx new file mode 100644 index 00000000..a8f0e213 --- /dev/null +++ b/packages/webui/src/components/harness-settings-parts.test.tsx @@ -0,0 +1,16 @@ +import { describe, expect, it } from "vitest"; +import { acceptsModel } from "./harness-settings-parts.js"; + +// local-llm-codes-in-process 3.5: the Harness Settings views offer the model +// field to the local LLM agents, as to the CLIs that take `--model`. +describe("acceptsModel", () => { + it("offers a model for both local LLM agents", () => { + expect(acceptsModel("local-llm")).toBe(true); + expect(acceptsModel("local-llm-acp")).toBe(true); + }); + + it("still offers one for a CLI with --model, and none for a CLI without", () => { + expect(acceptsModel("claude-cli")).toBe(true); + expect(acceptsModel("gemini-cli")).toBe(false); + }); +}); diff --git a/packages/webui/src/components/harness-settings-parts.tsx b/packages/webui/src/components/harness-settings-parts.tsx index 5df58985..d87c2f6a 100644 --- a/packages/webui/src/components/harness-settings-parts.tsx +++ b/packages/webui/src/components/harness-settings-parts.tsx @@ -124,7 +124,7 @@ export interface StageForms { /** Whether an agent's registry entry says its CLI takes a model. */ export function acceptsModel(agentId: string): boolean { - return AGENT_REGISTRY.some((agent) => agent.id === agentId && agent.modelFlag !== undefined); + return AGENT_REGISTRY.some((agent) => agent.id === agentId && (agent.modelFlag !== undefined || agent.takesModel === true)); } // A hand-edited config may carry the object form for a stage;