Merge pull request 'feat(agent): delegate by default in proactive mode and support nested worktrees' (#6) from feat/proactive-delegation-default into main
13 files · +762 -90
M README.md +3 -3
| @@ -271,7 +271,7 @@ Print-mode stats contain `provider`, `model`, `prompt_tokens`, `reasoning_tokens | |||
| 271 | 271 | - `ast`: search or rewrite source code structurally with an [ast-grep](https://ast-grep.github.io/) pattern. Prefer it when the operation depends on code syntax; use `grep`/`edit` for plain text. `language` is optional; when omitted, ast-grep infers it from each file's extension. Zut starts an installed `ast-grep` executable, or `sg` when it identifies itself as ast-grep, directly without a shell. Searches return compact `path:line` matches. Rewrites are planned in memory, shown as a diff for confirmation, checked against the active filesystem scope, verified against the files' current contents, written through a temporary file and rename with file permissions preserved, and followed by bounded diagnostics. When `grep` is given a regex that looks like a call or definition search and `ast` is available, its result includes a short `ast` suggestion. Install ast-grep separately; the tool returns a clear error when it is not available. | |
| 272 | 272 | - `bash`: run a command in the session cwd with merged stdout/stderr. Every call must include a positive integer `timeout` in seconds; calls without it or with `timeout <= 0` are rejected, and the deadline is always enforced. On Unix, zut uses `/bin/bash -c` when available, then `bash -c` from `PATH`, and falls back to POSIX `/bin/sh -c` when Bash is unavailable. On Windows, it uses `cmd /C`. macOS ships Bash 3.2 by default, so newer Bash features may be unavailable. | |
| 273 | 273 | - `python`: execute inline Python 3 code with an existing interpreter, never installing one. Every call must include nonblank `code` and a positive integer `timeout` in seconds (shared by interpreter discovery and execution). Each call runs a fresh stateless subprocess directly (`-u -B -`, code on stdin) in the session cwd with merged stdout/stderr; there is no persistent interpreter state, no script file, and no bytecode cache. Interactive input is unsupported (stdin observes EOF). Output is bounded (50 KiB, 2,000 lines) with explicit truncation marks; truncated results persist the byte-capped buffer to a temp file referenced as `full output` with its path in `full_output_path`, mirroring `bash`. Discovery order: `python_interpreter` in `$ZUT_HOME/config.json` (one executable path; relative paths resolve against the session cwd), then the active `VIRTUAL_ENV`, then `./.venv`, then `python3`/`python` on `PATH`. On Windows, Store aliases, `py` launchers, and install-manager entrypoints are rejected; only positively identified installed interpreters run. Missing packages fail with the ordinary import traceback. Tool arguments and output are recorded in the session like other tools; files the code deliberately writes persist. Save reusable scripts with `write`/`edit` instead of using `python` as an opaque file-edit path. Unavailable to packaged agents, and rejected while jailed (see below). | |
| 274 | - | - `worktree`: manage Git worktrees with the `create`, `list`, and `cleanup` actions. Creation checks out a new branch under `$ZUT_HOME/worktrees/<repo-id>`; an absolute local Git setting, `zut.worktrees.path`, overrides that root per repository. Listing inventories registered worktrees. Cleanup requires exact paths, confirmation through the normal preview gate, and revalidation immediately before removal. It refuses the primary worktree, dirty or detached checkouts, and commits not merged into the current branch. It removes neither local branches nor unmerged work. | |
| 274 | + | - `worktree`: manage Git worktrees with the `create`, `list`, and `cleanup` actions. Creation checks out a new branch under `$ZUT_HOME/worktrees/<repo-id>`, starting from the current HEAD or from `base` (a branch, tag, or commit); an absolute local Git setting, `zut.worktrees.path`, overrides that root per repository. Listing inventories registered worktrees. Cleanup requires exact paths, confirmation through the normal preview gate, and revalidation immediately before removal. It refuses the primary worktree, dirty or detached checkouts, and commits not merged into the current branch, or into `into` when given. With `delete_branch`, it also deletes each removed worktree's local branch, only while the branch is still at the inspected commit; it never deletes the merge target or a branch checked out in another worktree, and it reports a branch-deletion failure while keeping the removed worktrees removed. It never removes unmerged work. For a feature-branch workflow, create `feat/x` from main, then create sub-worktrees with `base: "feat/x"` (for example `feat/x-api`). After committing on each sub-branch and merging it into `feat/x`, clean up with `into: "feat/x"` and `delete_branch: true`. `list` also accepts `into` to check eligibility against that branch. | |
| 275 | 275 | - `lsp`: query configured language servers and linters for diagnostics, definitions, references, hover information, symbols, renames, code actions, capabilities, and raw protocol requests. LSP servers use stdio JSON-RPC; project `lsp.json` files can add or override servers and CLI linters. Diagnostics are bounded, sorted, deduplicated by path/severity/code/start position, and repeated issues are grouped before they reach the model. Successful native edits, writes, renames, and applied code actions automatically attach bounded diagnostics when available. See [docs/lsp.md](docs/lsp.md). | |
| 276 | 276 | - `web_search`: search DuckDuckGo HTML and return bounded source titles, URLs, snippets, and opaque references. `web_open` opens either a complete public URL or a referenced page, while `web_find` and `web_click` respectively find literal text in an opened page and follow a displayed link. The four tools are enabled together by default for normal CLI sessions and unavailable to bot, default SDK, and packaged-agent runs. See [docs/web-search.md](docs/web-search.md). | |
| 277 | 277 | - `update_goal`: let the main agent start its own mission when none is active, set the next bounded goal within an active mission, pause work while a decision is pending, resume it once settled, mark the active goal `done` or `blocked`, or atomically supersede an invalid goal with a necessary in-mission replacement. Users can also pause and resume through `/goal`; clearing remains user-controlled. | |
| @@ -457,7 +457,7 @@ Each child runs without a cumulative token budget or step cap: it inherits the p | |||
| 457 | 457 | ||
| 458 | 458 | Worktree isolation captures a patch and changed-file list without applying it. Shared children use the host checkout. The available logical references are `subagent://<id>`, `subagent://<id>/history`, `subagent://<id>/result`, and `subagent://<id>/patch`. | |
| 459 | 459 | ||
| 460 | - | **Proactive delegation.** With `/settings` → **proactive delegation** on (or `/orchestrator`), the interactive primary remains the owner and implementer of the task. Process start reveals neither the task nor which workers it needs, so it routes when the user states the request: the reply that starts the work names the parts it keeps on the critical path and the profile that owns each delegable part, before implementing or spawning anything. It delegates only concrete sidecars that can progress while the primary performs a preselected non-overlapping task; immediate blockers and tightly coupled work stay local. An active worker owns its scope until completion, so the primary does not repeat that investigation, review, testing, or implementation. If an explicit user request or workflow hands blocking work to a child, the primary yields for the host completion update instead. The canonical resident `subagent` tool remains available for user-requested delegation when the setting is off, subject to launch-time permissions. Do not poll the status action or journal files solely to wait for completion. | |
| 460 | + | **Proactive delegation.** With `/settings` → **proactive delegation** on (or `/orchestrator`), the interactive primary acts as an orchestrator and delegates by default. Implementation, debugging, testing, review, and broad research go to workers; answers, quick lookups, edits of a few lines, and integration stay local. For each new request, the reply that starts the work names each part, its owner, and the profile for each delegated part. The primary maps dependencies, runs independent parts in parallel, and for multi-part features gives each worker a sub-worktree based on a feature branch, gates each result with a review before merging, and removes merged worktrees and branches when done. Blocking work is spawned with `required: true` and the primary yields for the host completion update. An active worker owns its scope until completion, so the primary does not repeat that investigation, review, testing, or implementation. The canonical resident `subagent` tool remains available for user-requested delegation when the setting is off, subject to launch-time permissions. Do not poll the status action or journal files solely to wait for completion. | |
| 461 | 461 | ||
| 462 | 462 | ### Named subagent profiles | |
| 463 | 463 | ||
| @@ -471,7 +471,7 @@ Opens a dialog with every persistent setting. `up`/`down` to navigate, `enter` o | |||
| 471 | 471 | - **render images when supported** — draw screenshots / `read`-returned images inline using the terminal's image protocol, or fall back to a text placeholder. Auto-detected from `TERM_PROGRAM`; the toggle overrides the detection. The row is greyed out and forced off on terminals that don't speak any image protocol. | |
| 472 | 472 | - **terminal alerts** — emit a terminal bell when the main session stops with work that may need attention, or when an extension raises a structured alert. Enabled by default; changes apply immediately and persist as `terminal_alerts_enabled`. Terminal emulators may render the bell audibly, visually, or not at all. | |
| 473 | 473 | - **AI terminal titles** — after the first real prompt of a fresh interactive session, make one small hidden request to the active model and set the terminal title to `zut: <title>`. The title is limited to 40 Unicode characters, persisted with the session, restored on resume, and never added to the conversation. Enabled by default; disable it to avoid the extra model request. The toggle applies immediately, but title generation still waits for the first real prompt and never starts from startup context, resumed history, or slash commands. Persists as `terminal_title_enabled`. | |
| 474 | - | - **proactive delegation** — let the interactive primary delegate concrete, bounded sidecars while it keeps ownership of the critical path. Because the task and the workers it needs are unknown until the user states them, the primary routes at that point: the reply that starts the work names the parts it keeps on the critical path and the profile that owns each delegable part, and it states when nothing is worth delegating instead of forcing workers. Before spawning, it identifies useful local work and gives the child a non-overlapping question, responsibility, package, or file set. Immediate blockers and tightly coupled work stay local; after spawning, the worker owns its scope until completion and the primary continues only its preselected independent task. If a user request or active workflow explicitly delegates blocking work, the primary yields rather than reproducing it. Off by default. When off, the permitted canonical subagent tools and profile manifest remain available for user-requested delegation or when an active skill workflow requires it. The tools accept named profiles, reasoning, per-spawn fast-mode overrides, and shared/worktree isolation where applicable; lifecycle and result references are persisted. Completion and required-work state are host-driven through `[auto-subagents update]` and `[required-subagents update]` messages; required work stays asynchronous, persists its outcome, and prevents the terminal parent response until a successful completion. If launch-time restrictions prevent spawning, the interactive primary continues locally. Explicit headless `--orchestrate` remains a separate strict manager-only mode and reports unavailable delegation rather than implementing directly. Do not poll or inspect dashboards, metadata, event logs, or files solely to wait. See `/subagents` and [docs/subagents.md](docs/subagents.md) for details. | |
| 474 | + | - **proactive delegation** — make the interactive primary an orchestrator that delegates substantial work by default and keeps small edits, lookups, and integration local. See **Proactive delegation** above. Off by default. When off, the permitted canonical subagent tools and profile manifest remain available for user-requested delegation or when an active skill workflow requires it. The tools accept named profiles, reasoning, per-spawn fast-mode overrides, and shared/worktree isolation where applicable; lifecycle and result references are persisted. Completion and required-work state are host-driven through `[auto-subagents update]` and `[required-subagents update]` messages; required work stays asynchronous, persists its outcome, and prevents the terminal parent response until a successful completion. If launch-time restrictions prevent spawning, the interactive primary continues locally. Explicit headless `--orchestrate` remains a separate strict manager-only mode and reports unavailable delegation rather than implementing directly. Do not poll or inspect dashboards, metadata, event logs, or files solely to wait. See `/subagents` and [docs/subagents.md](docs/subagents.md) for details. | |
| 475 | 475 | - **fast mode** — request the provider's fast tier where supported. It currently uses OpenAI's Fast service tier for OpenAI, OpenAI Responses, and OpenAI Codex models. Off by default; toggle it with `/fast` or `/settings`; changes apply on the next model call and persist as `fast_mode`. When a model profile is active, the value is saved in that profile and restored when switching profiles. Other providers currently return an unsupported-provider error. Fast mode may cost more and depends on the selected model/account. | |
| 476 | 476 | - **lsp in main session** — enable the built-in `lsp` tool and code diagnostics for the main agent. Enabled by default; changes persist as `lsp_enabled`. | |
| 477 | 477 | - **lsp in sub-agents** — allow newly spawned background sub-agents to use the built-in `lsp` tool. Enabled by default; changes persist as `subagent_lsp_enabled` and apply when a child starts. | |
| @@ -271,7 +271,7 @@ Print-mode stats contain `provider`, `model`, `prompt_tokens`, `reasoning_tokens | |||||
| 271 | - `ast`: search or rewrite source code structurally with an [ast-grep](https://ast-grep.github.io/) pattern. Prefer it when the operation depends on code syntax; use `grep`/`edit` for plain text. `language` is optional; when omitted, ast-grep infers it from each file's extension. Zut starts an installed `ast-grep` executable, or `sg` when it identifies itself as ast-grep, directly without a shell. Searches return compact `path:line` matches. Rewrites are planned in memory, shown as a diff for confirmation, checked against the active filesystem scope, verified against the files' current contents, written through a temporary file and rename with file permissions preserved, and followed by bounded diagnostics. When `grep` is given a regex that looks like a call or definition search and `ast` is available, its result includes a short `ast` suggestion. Install ast-grep separately; the tool returns a clear error when it is not available. | 271 | - `ast`: search or rewrite source code structurally with an [ast-grep](https://ast-grep.github.io/) pattern. Prefer it when the operation depends on code syntax; use `grep`/`edit` for plain text. `language` is optional; when omitted, ast-grep infers it from each file's extension. Zut starts an installed `ast-grep` executable, or `sg` when it identifies itself as ast-grep, directly without a shell. Searches return compact `path:line` matches. Rewrites are planned in memory, shown as a diff for confirmation, checked against the active filesystem scope, verified against the files' current contents, written through a temporary file and rename with file permissions preserved, and followed by bounded diagnostics. When `grep` is given a regex that looks like a call or definition search and `ast` is available, its result includes a short `ast` suggestion. Install ast-grep separately; the tool returns a clear error when it is not available. | ||
| 272 | - `bash`: run a command in the session cwd with merged stdout/stderr. Every call must include a positive integer `timeout` in seconds; calls without it or with `timeout <= 0` are rejected, and the deadline is always enforced. On Unix, zut uses `/bin/bash -c` when available, then `bash -c` from `PATH`, and falls back to POSIX `/bin/sh -c` when Bash is unavailable. On Windows, it uses `cmd /C`. macOS ships Bash 3.2 by default, so newer Bash features may be unavailable. | 272 | - `bash`: run a command in the session cwd with merged stdout/stderr. Every call must include a positive integer `timeout` in seconds; calls without it or with `timeout <= 0` are rejected, and the deadline is always enforced. On Unix, zut uses `/bin/bash -c` when available, then `bash -c` from `PATH`, and falls back to POSIX `/bin/sh -c` when Bash is unavailable. On Windows, it uses `cmd /C`. macOS ships Bash 3.2 by default, so newer Bash features may be unavailable. | ||
| 273 | - `python`: execute inline Python 3 code with an existing interpreter, never installing one. Every call must include nonblank `code` and a positive integer `timeout` in seconds (shared by interpreter discovery and execution). Each call runs a fresh stateless subprocess directly (`-u -B -`, code on stdin) in the session cwd with merged stdout/stderr; there is no persistent interpreter state, no script file, and no bytecode cache. Interactive input is unsupported (stdin observes EOF). Output is bounded (50 KiB, 2,000 lines) with explicit truncation marks; truncated results persist the byte-capped buffer to a temp file referenced as `full output` with its path in `full_output_path`, mirroring `bash`. Discovery order: `python_interpreter` in `$ZUT_HOME/config.json` (one executable path; relative paths resolve against the session cwd), then the active `VIRTUAL_ENV`, then `./.venv`, then `python3`/`python` on `PATH`. On Windows, Store aliases, `py` launchers, and install-manager entrypoints are rejected; only positively identified installed interpreters run. Missing packages fail with the ordinary import traceback. Tool arguments and output are recorded in the session like other tools; files the code deliberately writes persist. Save reusable scripts with `write`/`edit` instead of using `python` as an opaque file-edit path. Unavailable to packaged agents, and rejected while jailed (see below). | 273 | - `python`: execute inline Python 3 code with an existing interpreter, never installing one. Every call must include nonblank `code` and a positive integer `timeout` in seconds (shared by interpreter discovery and execution). Each call runs a fresh stateless subprocess directly (`-u -B -`, code on stdin) in the session cwd with merged stdout/stderr; there is no persistent interpreter state, no script file, and no bytecode cache. Interactive input is unsupported (stdin observes EOF). Output is bounded (50 KiB, 2,000 lines) with explicit truncation marks; truncated results persist the byte-capped buffer to a temp file referenced as `full output` with its path in `full_output_path`, mirroring `bash`. Discovery order: `python_interpreter` in `$ZUT_HOME/config.json` (one executable path; relative paths resolve against the session cwd), then the active `VIRTUAL_ENV`, then `./.venv`, then `python3`/`python` on `PATH`. On Windows, Store aliases, `py` launchers, and install-manager entrypoints are rejected; only positively identified installed interpreters run. Missing packages fail with the ordinary import traceback. Tool arguments and output are recorded in the session like other tools; files the code deliberately writes persist. Save reusable scripts with `write`/`edit` instead of using `python` as an opaque file-edit path. Unavailable to packaged agents, and rejected while jailed (see below). | ||
| 274 | - | - `worktree`: manage Git worktrees with the `create`, `list`, and `cleanup` actions. Creation checks out a new branch under `$ZUT_HOME/worktrees/<repo-id>`; an absolute local Git setting, `zut.worktrees.path`, overrides that root per repository. Listing inventories registered worktrees. Cleanup requires exact paths, confirmation through the normal preview gate, and revalidation immediately before removal. It refuses the primary worktree, dirty or detached checkouts, and commits not merged into the current branch. It removes neither local branches nor unmerged work. | 274 | + | - `worktree`: manage Git worktrees with the `create`, `list`, and `cleanup` actions. Creation checks out a new branch under `$ZUT_HOME/worktrees/<repo-id>`, starting from the current HEAD or from `base` (a branch, tag, or commit); an absolute local Git setting, `zut.worktrees.path`, overrides that root per repository. Listing inventories registered worktrees. Cleanup requires exact paths, confirmation through the normal preview gate, and revalidation immediately before removal. It refuses the primary worktree, dirty or detached checkouts, and commits not merged into the current branch, or into `into` when given. With `delete_branch`, it also deletes each removed worktree's local branch, only while the branch is still at the inspected commit; it never deletes the merge target or a branch checked out in another worktree, and it reports a branch-deletion failure while keeping the removed worktrees removed. It never removes unmerged work. For a feature-branch workflow, create `feat/x` from main, then create sub-worktrees with `base: "feat/x"` (for example `feat/x-api`). After committing on each sub-branch and merging it into `feat/x`, clean up with `into: "feat/x"` and `delete_branch: true`. `list` also accepts `into` to check eligibility against that branch. |
| 275 | - `lsp`: query configured language servers and linters for diagnostics, definitions, references, hover information, symbols, renames, code actions, capabilities, and raw protocol requests. LSP servers use stdio JSON-RPC; project `lsp.json` files can add or override servers and CLI linters. Diagnostics are bounded, sorted, deduplicated by path/severity/code/start position, and repeated issues are grouped before they reach the model. Successful native edits, writes, renames, and applied code actions automatically attach bounded diagnostics when available. See [docs/lsp.md](docs/lsp.md). | 275 | - `lsp`: query configured language servers and linters for diagnostics, definitions, references, hover information, symbols, renames, code actions, capabilities, and raw protocol requests. LSP servers use stdio JSON-RPC; project `lsp.json` files can add or override servers and CLI linters. Diagnostics are bounded, sorted, deduplicated by path/severity/code/start position, and repeated issues are grouped before they reach the model. Successful native edits, writes, renames, and applied code actions automatically attach bounded diagnostics when available. See [docs/lsp.md](docs/lsp.md). | ||
| 276 | - `web_search`: search DuckDuckGo HTML and return bounded source titles, URLs, snippets, and opaque references. `web_open` opens either a complete public URL or a referenced page, while `web_find` and `web_click` respectively find literal text in an opened page and follow a displayed link. The four tools are enabled together by default for normal CLI sessions and unavailable to bot, default SDK, and packaged-agent runs. See [docs/web-search.md](docs/web-search.md). | 276 | - `web_search`: search DuckDuckGo HTML and return bounded source titles, URLs, snippets, and opaque references. `web_open` opens either a complete public URL or a referenced page, while `web_find` and `web_click` respectively find literal text in an opened page and follow a displayed link. The four tools are enabled together by default for normal CLI sessions and unavailable to bot, default SDK, and packaged-agent runs. See [docs/web-search.md](docs/web-search.md). | ||
| 277 | - `update_goal`: let the main agent start its own mission when none is active, set the next bounded goal within an active mission, pause work while a decision is pending, resume it once settled, mark the active goal `done` or `blocked`, or atomically supersede an invalid goal with a necessary in-mission replacement. Users can also pause and resume through `/goal`; clearing remains user-controlled. | 277 | - `update_goal`: let the main agent start its own mission when none is active, set the next bounded goal within an active mission, pause work while a decision is pending, resume it once settled, mark the active goal `done` or `blocked`, or atomically supersede an invalid goal with a necessary in-mission replacement. Users can also pause and resume through `/goal`; clearing remains user-controlled. | ||
| @@ -457,7 +457,7 @@ Each child runs without a cumulative token budget or step cap: it inherits the p | |||||
| 457 | 457 | ||||
| 458 | Worktree isolation captures a patch and changed-file list without applying it. Shared children use the host checkout. The available logical references are `subagent://<id>`, `subagent://<id>/history`, `subagent://<id>/result`, and `subagent://<id>/patch`. | 458 | Worktree isolation captures a patch and changed-file list without applying it. Shared children use the host checkout. The available logical references are `subagent://<id>`, `subagent://<id>/history`, `subagent://<id>/result`, and `subagent://<id>/patch`. | ||
| 459 | 459 | ||||
| 460 | - | **Proactive delegation.** With `/settings` → **proactive delegation** on (or `/orchestrator`), the interactive primary remains the owner and implementer of the task. Process start reveals neither the task nor which workers it needs, so it routes when the user states the request: the reply that starts the work names the parts it keeps on the critical path and the profile that owns each delegable part, before implementing or spawning anything. It delegates only concrete sidecars that can progress while the primary performs a preselected non-overlapping task; immediate blockers and tightly coupled work stay local. An active worker owns its scope until completion, so the primary does not repeat that investigation, review, testing, or implementation. If an explicit user request or workflow hands blocking work to a child, the primary yields for the host completion update instead. The canonical resident `subagent` tool remains available for user-requested delegation when the setting is off, subject to launch-time permissions. Do not poll the status action or journal files solely to wait for completion. | 460 | + | **Proactive delegation.** With `/settings` → **proactive delegation** on (or `/orchestrator`), the interactive primary acts as an orchestrator and delegates by default. Implementation, debugging, testing, review, and broad research go to workers; answers, quick lookups, edits of a few lines, and integration stay local. For each new request, the reply that starts the work names each part, its owner, and the profile for each delegated part. The primary maps dependencies, runs independent parts in parallel, and for multi-part features gives each worker a sub-worktree based on a feature branch, gates each result with a review before merging, and removes merged worktrees and branches when done. Blocking work is spawned with `required: true` and the primary yields for the host completion update. An active worker owns its scope until completion, so the primary does not repeat that investigation, review, testing, or implementation. The canonical resident `subagent` tool remains available for user-requested delegation when the setting is off, subject to launch-time permissions. Do not poll the status action or journal files solely to wait for completion. |
| 461 | 461 | ||||
| 462 | ### Named subagent profiles | 462 | ### Named subagent profiles | ||
| 463 | 463 | ||||
| @@ -471,7 +471,7 @@ Opens a dialog with every persistent setting. `up`/`down` to navigate, `enter` o | |||||
| 471 | - **render images when supported** — draw screenshots / `read`-returned images inline using the terminal's image protocol, or fall back to a text placeholder. Auto-detected from `TERM_PROGRAM`; the toggle overrides the detection. The row is greyed out and forced off on terminals that don't speak any image protocol. | 471 | - **render images when supported** — draw screenshots / `read`-returned images inline using the terminal's image protocol, or fall back to a text placeholder. Auto-detected from `TERM_PROGRAM`; the toggle overrides the detection. The row is greyed out and forced off on terminals that don't speak any image protocol. | ||
| 472 | - **terminal alerts** — emit a terminal bell when the main session stops with work that may need attention, or when an extension raises a structured alert. Enabled by default; changes apply immediately and persist as `terminal_alerts_enabled`. Terminal emulators may render the bell audibly, visually, or not at all. | 472 | - **terminal alerts** — emit a terminal bell when the main session stops with work that may need attention, or when an extension raises a structured alert. Enabled by default; changes apply immediately and persist as `terminal_alerts_enabled`. Terminal emulators may render the bell audibly, visually, or not at all. | ||
| 473 | - **AI terminal titles** — after the first real prompt of a fresh interactive session, make one small hidden request to the active model and set the terminal title to `zut: <title>`. The title is limited to 40 Unicode characters, persisted with the session, restored on resume, and never added to the conversation. Enabled by default; disable it to avoid the extra model request. The toggle applies immediately, but title generation still waits for the first real prompt and never starts from startup context, resumed history, or slash commands. Persists as `terminal_title_enabled`. | 473 | - **AI terminal titles** — after the first real prompt of a fresh interactive session, make one small hidden request to the active model and set the terminal title to `zut: <title>`. The title is limited to 40 Unicode characters, persisted with the session, restored on resume, and never added to the conversation. Enabled by default; disable it to avoid the extra model request. The toggle applies immediately, but title generation still waits for the first real prompt and never starts from startup context, resumed history, or slash commands. Persists as `terminal_title_enabled`. | ||
| 474 | - | - **proactive delegation** — let the interactive primary delegate concrete, bounded sidecars while it keeps ownership of the critical path. Because the task and the workers it needs are unknown until the user states them, the primary routes at that point: the reply that starts the work names the parts it keeps on the critical path and the profile that owns each delegable part, and it states when nothing is worth delegating instead of forcing workers. Before spawning, it identifies useful local work and gives the child a non-overlapping question, responsibility, package, or file set. Immediate blockers and tightly coupled work stay local; after spawning, the worker owns its scope until completion and the primary continues only its preselected independent task. If a user request or active workflow explicitly delegates blocking work, the primary yields rather than reproducing it. Off by default. When off, the permitted canonical subagent tools and profile manifest remain available for user-requested delegation or when an active skill workflow requires it. The tools accept named profiles, reasoning, per-spawn fast-mode overrides, and shared/worktree isolation where applicable; lifecycle and result references are persisted. Completion and required-work state are host-driven through `[auto-subagents update]` and `[required-subagents update]` messages; required work stays asynchronous, persists its outcome, and prevents the terminal parent response until a successful completion. If launch-time restrictions prevent spawning, the interactive primary continues locally. Explicit headless `--orchestrate` remains a separate strict manager-only mode and reports unavailable delegation rather than implementing directly. Do not poll or inspect dashboards, metadata, event logs, or files solely to wait. See `/subagents` and [docs/subagents.md](docs/subagents.md) for details. | 474 | + | - **proactive delegation** — make the interactive primary an orchestrator that delegates substantial work by default and keeps small edits, lookups, and integration local. See **Proactive delegation** above. Off by default. When off, the permitted canonical subagent tools and profile manifest remain available for user-requested delegation or when an active skill workflow requires it. The tools accept named profiles, reasoning, per-spawn fast-mode overrides, and shared/worktree isolation where applicable; lifecycle and result references are persisted. Completion and required-work state are host-driven through `[auto-subagents update]` and `[required-subagents update]` messages; required work stays asynchronous, persists its outcome, and prevents the terminal parent response until a successful completion. If launch-time restrictions prevent spawning, the interactive primary continues locally. Explicit headless `--orchestrate` remains a separate strict manager-only mode and reports unavailable delegation rather than implementing directly. Do not poll or inspect dashboards, metadata, event logs, or files solely to wait. See `/subagents` and [docs/subagents.md](docs/subagents.md) for details. |
| 475 | - **fast mode** — request the provider's fast tier where supported. It currently uses OpenAI's Fast service tier for OpenAI, OpenAI Responses, and OpenAI Codex models. Off by default; toggle it with `/fast` or `/settings`; changes apply on the next model call and persist as `fast_mode`. When a model profile is active, the value is saved in that profile and restored when switching profiles. Other providers currently return an unsupported-provider error. Fast mode may cost more and depends on the selected model/account. | 475 | - **fast mode** — request the provider's fast tier where supported. It currently uses OpenAI's Fast service tier for OpenAI, OpenAI Responses, and OpenAI Codex models. Off by default; toggle it with `/fast` or `/settings`; changes apply on the next model call and persist as `fast_mode`. When a model profile is active, the value is saved in that profile and restored when switching profiles. Other providers currently return an unsupported-provider error. Fast mode may cost more and depends on the selected model/account. | ||
| 476 | - **lsp in main session** — enable the built-in `lsp` tool and code diagnostics for the main agent. Enabled by default; changes persist as `lsp_enabled`. | 476 | - **lsp in main session** — enable the built-in `lsp` tool and code diagnostics for the main agent. Enabled by default; changes persist as `lsp_enabled`. | ||
| 477 | - **lsp in sub-agents** — allow newly spawned background sub-agents to use the built-in `lsp` tool. Enabled by default; changes persist as `subagent_lsp_enabled` and apply when a child starts. | 477 | - **lsp in sub-agents** — allow newly spawned background sub-agents to use the built-in `lsp` tool. Enabled by default; changes persist as `subagent_lsp_enabled` and apply when a child starts. | ||
M docs/subagents.md +25 -17
| @@ -189,23 +189,31 @@ retries it automatically. | |||
| 189 | 189 | ||
| 190 | 190 | ## Delegation ownership | |
| 191 | 191 | ||
| 192 | - | Interactive proactive delegation keeps the primary agent on the critical path. | |
| 193 | - | Process start says nothing about the task, and the workers it needs only become | |
| 194 | - | knowable once the user states the request. Routing therefore happens then: the | |
| 195 | - | reply that starts the work names the parts the primary keeps on the critical | |
| 196 | - | path and the profile that owns each delegable part, before it implements or | |
| 197 | - | spawns anything. It states plainly when nothing is worth delegating rather than | |
| 198 | - | forcing a worker. The routing guidance is part of the prompt only when | |
| 199 | - | launch-time policy exposes the spawn action. | |
| 200 | - | Before spawning, it selects useful local work and delegates only a concrete, | |
| 201 | - | bounded sidecar with a non-overlapping question, responsibility, package, or | |
| 202 | - | file set. Immediate blockers and tightly coupled work stay local. While a child | |
| 203 | - | is active, it owns that scope; the primary continues only its preselected | |
| 204 | - | independent work instead of repeating the investigation, review, testing, or | |
| 205 | - | implementation. If an explicit user request or active workflow hands blocking | |
| 206 | - | work to a child, the primary yields for the completion update. Once a worktree | |
| 207 | - | child succeeds, the primary may apply and verify its captured patch without | |
| 208 | - | redoing the delegated task. | |
| 192 | + | Interactive proactive delegation makes the primary agent an orchestrator that | |
| 193 | + | delegates by default. Implementation, debugging, testing, review, and broad | |
| 194 | + | research go to workers; answers, quick lookups, edits of a few lines, and | |
| 195 | + | integration of worker results stay local. Blocking work is delegated too: the | |
| 196 | + | primary spawns it with `required: true` and yields for the completion update. | |
| 197 | + | ||
| 198 | + | Routing happens for each new request, not at process start: the reply that | |
| 199 | + | starts the work names each part, its owner, and the profile for each delegated | |
| 200 | + | part. The routing guidance is part of the prompt only when launch-time policy | |
| 201 | + | exposes the spawn action. | |
| 202 | + | ||
| 203 | + | Before spawning, the primary maps dependencies, runs independent parts in | |
| 204 | + | parallel, and gives every worker a self-contained task with a disjoint scope. | |
| 205 | + | For a multi-part feature it creates a feature worktree branch, then one | |
| 206 | + | sub-worktree per worker with `worktree` `create` and `base` set to the feature | |
| 207 | + | branch. It reviews each result, commits on the sub-branch, and merges it into | |
| 208 | + | the feature branch. It then removes merged sub-worktrees and their branches | |
| 209 | + | with `worktree` `cleanup`, `into` set to the feature branch and | |
| 210 | + | `delete_branch: true`, and checks with `list` that none are left behind. | |
| 211 | + | ||
| 212 | + | While a child is active, it owns that scope; the primary does not repeat its | |
| 213 | + | investigation, review, testing, or implementation. Once an | |
| 214 | + | `isolation: "worktree"` child succeeds, the primary may apply and verify its | |
| 215 | + | captured patch without redoing the task, then stop the idle child so its | |
| 216 | + | temporary worktree is removed. | |
| 209 | 217 | ||
| 210 | 218 | Explicit headless `--orchestrate` remains manager-only. Its primary divides the | |
| 211 | 219 | request into disjoint worker scopes, coordinates active children, and yields | |
| @@ -189,23 +189,31 @@ retries it automatically. | |||||
| 189 | 189 | ||||
| 190 | ## Delegation ownership | 190 | ## Delegation ownership | ||
| 191 | 191 | ||||
| 192 | - | Interactive proactive delegation keeps the primary agent on the critical path. | 192 | + | Interactive proactive delegation makes the primary agent an orchestrator that |
| 193 | - | Process start says nothing about the task, and the workers it needs only become | 193 | + | delegates by default. Implementation, debugging, testing, review, and broad |
| 194 | - | knowable once the user states the request. Routing therefore happens then: the | 194 | + | research go to workers; answers, quick lookups, edits of a few lines, and |
| 195 | - | reply that starts the work names the parts the primary keeps on the critical | 195 | + | integration of worker results stay local. Blocking work is delegated too: the |
| 196 | - | path and the profile that owns each delegable part, before it implements or | 196 | + | primary spawns it with `required: true` and yields for the completion update. |
| 197 | - | spawns anything. It states plainly when nothing is worth delegating rather than | 197 | + | |
| 198 | - | forcing a worker. The routing guidance is part of the prompt only when | 198 | + | Routing happens for each new request, not at process start: the reply that |
| 199 | - | launch-time policy exposes the spawn action. | 199 | + | starts the work names each part, its owner, and the profile for each delegated |
| 200 | - | Before spawning, it selects useful local work and delegates only a concrete, | 200 | + | part. The routing guidance is part of the prompt only when launch-time policy |
| 201 | - | bounded sidecar with a non-overlapping question, responsibility, package, or | 201 | + | exposes the spawn action. |
| 202 | - | file set. Immediate blockers and tightly coupled work stay local. While a child | 202 | + | |
| 203 | - | is active, it owns that scope; the primary continues only its preselected | 203 | + | Before spawning, the primary maps dependencies, runs independent parts in |
| 204 | - | independent work instead of repeating the investigation, review, testing, or | 204 | + | parallel, and gives every worker a self-contained task with a disjoint scope. |
| 205 | - | implementation. If an explicit user request or active workflow hands blocking | 205 | + | For a multi-part feature it creates a feature worktree branch, then one |
| 206 | - | work to a child, the primary yields for the completion update. Once a worktree | 206 | + | sub-worktree per worker with `worktree` `create` and `base` set to the feature |
| 207 | - | child succeeds, the primary may apply and verify its captured patch without | 207 | + | branch. It reviews each result, commits on the sub-branch, and merges it into |
| 208 | - | redoing the delegated task. | 208 | + | the feature branch. It then removes merged sub-worktrees and their branches |
| 209 | + | with `worktree` `cleanup`, `into` set to the feature branch and | |||
| 210 | + | `delete_branch: true`, and checks with `list` that none are left behind. | |||
| 211 | + | ||||
| 212 | + | While a child is active, it owns that scope; the primary does not repeat its | |||
| 213 | + | investigation, review, testing, or implementation. Once an | |||
| 214 | + | `isolation: "worktree"` child succeeds, the primary may apply and verify its | |||
| 215 | + | captured patch without redoing the task, then stop the idle child so its | |||
| 216 | + | temporary worktree is removed. | |||
| 209 | 217 | ||||
| 210 | Explicit headless `--orchestrate` remains manager-only. Its primary divides the | 218 | Explicit headless `--orchestrate` remains manager-only. Its primary divides the | ||
| 211 | request into disjoint worker scopes, coordinates active children, and yields | 219 | request into disjoint worker scopes, coordinates active children, and yields | ||
M packages/agent/modes/interactive_settings.go +1 -1
| @@ -284,7 +284,7 @@ func (i *Interactive) openSettingsDialog() { | |||
| 284 | 284 | { | |
| 285 | 285 | key: "auto_subagents_enabled", | |
| 286 | 286 | label: "proactive delegation", | |
| 287 | - | desc: "delegate independent sidecar work while keeping the critical path local", | |
| 287 | + | desc: "delegate substantial work to subagents; keep small edits local", | |
| 288 | 288 | value: autoSubagents, | |
| 289 | 289 | disabled: autoSubagentsDisabled, | |
| 290 | 290 | hint: autoSubagentsHint, | |
| @@ -284,7 +284,7 @@ func (i *Interactive) openSettingsDialog() { | |||||
| 284 | { | 284 | { | ||
| 285 | key: "auto_subagents_enabled", | 285 | key: "auto_subagents_enabled", | ||
| 286 | label: "proactive delegation", | 286 | label: "proactive delegation", | ||
| 287 | - | desc: "delegate independent sidecar work while keeping the critical path local", | 287 | + | desc: "delegate substantial work to subagents; keep small edits local", |
| 288 | value: autoSubagents, | 288 | value: autoSubagents, | ||
| 289 | disabled: autoSubagentsDisabled, | 289 | disabled: autoSubagentsDisabled, | ||
| 290 | hint: autoSubagentsHint, | 290 | hint: autoSubagentsHint, | ||
M packages/agent/settings_store.go +38 -26
| @@ -276,28 +276,40 @@ func AutoSubagentsEnabled() bool { | |||
| 276 | 276 | // ProactiveSubagentsRoutingAddendum is prepended to the interactive | |
| 277 | 277 | // collaboration contract only when launch-time policy exposes the spawn | |
| 278 | 278 | // action: naming a profile is meaningless when the primary cannot dispatch. | |
| 279 | - | const ProactiveSubagentsRoutingAddendum = `Route before you work: | |
| 280 | - | - Process start tells you nothing about the task or the workers it needs; routing happens when the user states the request you are about to work on. | |
| 281 | - | - Name the parts you keep on the critical path and the [subagents_list] profile that owns each delegable part, before you implement or spawn. | |
| 282 | - | - State that routing once, in the reply that starts the work, and let the tool calls that follow speak for themselves instead of narrating them. | |
| 283 | - | - Say plainly when nothing is worth delegating instead of inventing sidecars, and spawn a clearly described general worker when no profile matches a scope.` | |
| 284 | - | ||
| 285 | - | // ProactiveSubagentsSystemAddendum keeps the interactive primary agent on the | |
| 286 | - | // critical path and reserves delegation for genuinely parallel sidecar work. | |
| 287 | - | const ProactiveSubagentsSystemAddendum = `Proactive subagent delegation is enabled. You remain the primary owner and implementer of the user's task. | |
| 288 | - | ||
| 289 | - | Before delegating: | |
| 290 | - | - Form a short high-level plan and identify the immediate next task you will perform locally. | |
| 291 | - | - Delegate only a concrete, bounded sidecar that can progress independently while you perform useful local work. | |
| 292 | - | - Keep urgent, tightly coupled, or immediate blocking work local. If your next action depends on a result, do that work yourself instead of spawning and waiting for it. | |
| 293 | - | - Divide ownership by distinct question, responsibility, package, or disjoint file set. Give every worker a self-contained task. | |
| 294 | - | - If you cannot name useful non-overlapping local work, do not delegate. | |
| 295 | - | ||
| 296 | - | After delegating: | |
| 297 | - | - The worker owns its delegated scope until it completes. Continue only with the non-overlapping local task selected before delegation. | |
| 298 | - | - Do not search, read, test, review, edit, or otherwise reproduce the worker's task. You may dispatch additional workers only for genuinely distinct scopes. | |
| 299 | - | - If the user or an active workflow explicitly requires a worker to own blocking work, delegate it with required:true and end or yield your turn instead of duplicating it. | |
| 300 | - | - Integrate and, when appropriate, verify completed worker results before answering. You may apply a completed worktree patch without redoing the delegated task.` | |
| 279 | + | const ProactiveSubagentsRoutingAddendum = `Route every new task before you work: | |
| 280 | + | - Routing happens when the user states a request, not at process start. Re-route each new request, even late in a long session. | |
| 281 | + | - In the reply that starts the work, name each part, who owns it, and the [subagents_list] profile for each delegated part. Then spawn; do not narrate the tool calls that follow. | |
| 282 | + | - Spawn a clearly described general worker when no profile matches a scope. | |
| 283 | + | - Keep a task fully local only when it is small: an answer, a quick lookup, or an edit of a few lines. Say so in one line instead of forcing a worker.` | |
| 284 | + | ||
| 285 | + | // ProactiveSubagentsSystemAddendum makes the interactive primary agent an | |
| 286 | + | // orchestrator by default: substantial work goes to workers, while small | |
| 287 | + | // edits, exploration, and integration stay local. | |
| 288 | + | const ProactiveSubagentsSystemAddendum = `Proactive subagent delegation is enabled. You are the orchestrator of the user's task: delegate by default and keep work local by exception. | |
| 289 | + | ||
| 290 | + | Delegate: | |
| 291 | + | - Implementation, debugging, test writing and running, and code review that spans more than a few lines or more than one file. | |
| 292 | + | - Research that needs many reads or searches across packages or repositories. | |
| 293 | + | - Blocking work too. If your next step depends on a worker's result, spawn it with required:true and end or yield your turn; waiting for a worker is expected, not a reason to work locally. | |
| 294 | + | ||
| 295 | + | Keep local: | |
| 296 | + | - Answers, quick lookups, and the short exploration needed to write precise worker tasks. | |
| 297 | + | - Edits of a few lines in one file, and integrating, applying, or verifying worker results. | |
| 298 | + | ||
| 299 | + | Plan parallelism before spawning: | |
| 300 | + | - Map dependencies first: which parts are independent, which must run in order, and which files each part touches. Run independent parts in parallel and order dependent parts. | |
| 301 | + | - Split work by distinct question, responsibility, package, or disjoint file set, and give every worker a self-contained task with acceptance checks. | |
| 302 | + | - One worker editing code: shared workspace is fine. Several workers editing code at once: give each its own tree. | |
| 303 | + | - For a multi-part feature, create a feature worktree branch, then one sub-worktree per worker with the worktree tool's create action and base set to the feature branch, and tell each worker its exact path. Review each worker's result as a gate, commit on its branch, and merge it into the feature branch. | |
| 304 | + | - Use isolation:"worktree" for a throwaway parallel attempt whose patch you apply yourself. | |
| 305 | + | ||
| 306 | + | Clean up as soon as a worker's result is integrated: | |
| 307 | + | - Remove merged sub-worktrees and their branches with the worktree tool's cleanup action, setting into to the feature branch and delete_branch to true. | |
| 308 | + | - Before your final answer, check with the worktree tool's list action that no worktree you created is left behind unless the user should keep it. | |
| 309 | + | ||
| 310 | + | When you delegate: | |
| 311 | + | - A worker owns its scope until it completes. Do not search, read, test, review, edit, or otherwise reproduce it; only coordinate, dispatch disjoint work, or yield. | |
| 312 | + | - Integrate and verify completed results before answering. You may apply a completed worktree patch without redoing the delegated task.` | |
| 301 | 313 | ||
| 302 | 314 | // StrictOrchestratorSystemAddendum is used only by explicit headless | |
| 303 | 315 | // orchestration. The parent coordinates workers and never becomes an | |
| @@ -312,11 +324,11 @@ const StrictOrchestratorSystemAddendum = `Strict subagent orchestration is enabl | |||
| 312 | 324 | ||
| 313 | 325 | const subagentLifecycleAddendum = `Workers start without this conversation's context. Shared-worktree workers edit the same working directory, so coordinate dependent work and avoid conflicting edits. Use isolation:"worktree" for parallel coding when workers need separate trees; its changes are returned as a patch and never applied automatically. Do not invent feature scope beyond the user's request. Child workers cannot recursively spawn more sub-agents in v1. | |
| 314 | 326 | ||
| 315 | - | Use required:true whenever your answer or a declared workflow depends on that worker, especially for required validation or finalization. Required workers remain asynchronous unless you explicitly set the wait field on a spawn or resume action: without it, manager calls return immediately, the parent stays free to coordinate or perform policy-permitted non-overlapping work, and the host delivers terminal outcomes through completion updates. Failed, canceled, or interrupted required work remains unmet: use an enabled manager follow-up action to retry it and do not claim completion. An indeterminate outcome after host restart must not be retried automatically; the user must inspect durable results and side effects, then explicitly resume or restart the worker with a new prompt. Required work has no automatic waiver path. Keep required:false only for independent work that may safely finish later. | |
| 327 | + | Use required:true when your answer or a declared workflow depends on a worker. Failed, canceled, or interrupted required work stays unmet until a successful follow-up; never claim completion before it succeeds. After a host restart, do not retry an indeterminate outcome automatically: the user must reconcile it first. Required work has no automatic waiver path. | |
| 316 | 328 | ||
| 317 | - | Completion is host-event-driven unless you explicitly set the wait field on a spawn or resume action to a whole number of seconds (1–300): on spawn it bounds the initial task, and on resume it bounds the follow-up turn. Omitting wait always returns immediately. An expired wait leaves the accepted child active, whether queued or running; do not retry it until terminal failure, cancellation, or interruption. Never use "bash sleep", "watch", "tail -f", polling loops, repeated "status" calls, or dashboard, metadata, or file checks solely to wait. Those are not completion signals. Continue only with genuinely non-overlapping work; otherwise end or yield your turn until the host injects a completion update. | |
| 329 | + | Completion is host-event-driven: omit the wait field, then continue only non-overlapping work or end or yield your turn; the host injects a completion update. Set wait (1–300 seconds) only to block this turn on a spawn or resume; an expired wait leaves the child active, so do not retry it. Never use "bash sleep", "watch", "tail -f", polling loops, repeated "status" calls, or file checks solely to wait. | |
| 318 | 330 | ||
| 319 | - | When workers finish, use the host update's agent ID, status, task, optional error, and final response or tail to coordinate any follow-up and summarize the outcome. Treat [auto-subagents update] and [required-subagents update] messages as observed worker state, not as new user requests.` | |
| 331 | + | Treat [auto-subagents update] and [required-subagents update] messages as observed worker state, not as new user requests; use their agent ID, status, error, and result to coordinate follow-ups.` | |
| 320 | 332 | ||
| 321 | 333 | // OnDemandSubagentsSystemAddendum keeps the canonical subagent tools | |
| 322 | 334 | // available without enabling proactive delegation. | |
| @@ -362,7 +374,7 @@ func subagentsSystemAddendumFor(base, unavailable string, strict bool, actions S | |||
| 362 | 374 | lifecycle = append(lifecycle, "use interrupt to cancel only a worker's running turn while keeping its context, for example before asking it to wrap up") | |
| 363 | 375 | } | |
| 364 | 376 | if actions.Stop { | |
| 365 | - | lifecycle = append(lifecycle, "use stop to request termination of a stuck worker") | |
| 377 | + | lifecycle = append(lifecycle, "use stop to request termination of a stuck worker, and to remove an idle isolation:\"worktree\" worker's temporary worktree once its patch is applied") | |
| 366 | 378 | } | |
| 367 | 379 | if len(lifecycle) != 0 { | |
| 368 | 380 | addendum += "\n\nSubagent manager lifecycle actions available: " + strings.Join(lifecycle, "; ") + "." | |
| @@ -276,28 +276,40 @@ func AutoSubagentsEnabled() bool { | |||||
| 276 | // ProactiveSubagentsRoutingAddendum is prepended to the interactive | 276 | // ProactiveSubagentsRoutingAddendum is prepended to the interactive | ||
| 277 | // collaboration contract only when launch-time policy exposes the spawn | 277 | // collaboration contract only when launch-time policy exposes the spawn | ||
| 278 | // action: naming a profile is meaningless when the primary cannot dispatch. | 278 | // action: naming a profile is meaningless when the primary cannot dispatch. | ||
| 279 | - | const ProactiveSubagentsRoutingAddendum = `Route before you work: | 279 | + | const ProactiveSubagentsRoutingAddendum = `Route every new task before you work: |
| 280 | - | - Process start tells you nothing about the task or the workers it needs; routing happens when the user states the request you are about to work on. | 280 | + | - Routing happens when the user states a request, not at process start. Re-route each new request, even late in a long session. |
| 281 | - | - Name the parts you keep on the critical path and the [subagents_list] profile that owns each delegable part, before you implement or spawn. | 281 | + | - In the reply that starts the work, name each part, who owns it, and the [subagents_list] profile for each delegated part. Then spawn; do not narrate the tool calls that follow. |
| 282 | - | - State that routing once, in the reply that starts the work, and let the tool calls that follow speak for themselves instead of narrating them. | 282 | + | - Spawn a clearly described general worker when no profile matches a scope. |
| 283 | - | - Say plainly when nothing is worth delegating instead of inventing sidecars, and spawn a clearly described general worker when no profile matches a scope.` | 283 | + | - Keep a task fully local only when it is small: an answer, a quick lookup, or an edit of a few lines. Say so in one line instead of forcing a worker.` |
| 284 | - | 284 | + | ||
| 285 | - | // ProactiveSubagentsSystemAddendum keeps the interactive primary agent on the | 285 | + | // ProactiveSubagentsSystemAddendum makes the interactive primary agent an |
| 286 | - | // critical path and reserves delegation for genuinely parallel sidecar work. | 286 | + | // orchestrator by default: substantial work goes to workers, while small |
| 287 | - | const ProactiveSubagentsSystemAddendum = `Proactive subagent delegation is enabled. You remain the primary owner and implementer of the user's task. | 287 | + | // edits, exploration, and integration stay local. |
| 288 | - | 288 | + | const ProactiveSubagentsSystemAddendum = `Proactive subagent delegation is enabled. You are the orchestrator of the user's task: delegate by default and keep work local by exception. | |
| 289 | - | Before delegating: | 289 | + | |
| 290 | - | - Form a short high-level plan and identify the immediate next task you will perform locally. | 290 | + | Delegate: |
| 291 | - | - Delegate only a concrete, bounded sidecar that can progress independently while you perform useful local work. | 291 | + | - Implementation, debugging, test writing and running, and code review that spans more than a few lines or more than one file. |
| 292 | - | - Keep urgent, tightly coupled, or immediate blocking work local. If your next action depends on a result, do that work yourself instead of spawning and waiting for it. | 292 | + | - Research that needs many reads or searches across packages or repositories. |
| 293 | - | - Divide ownership by distinct question, responsibility, package, or disjoint file set. Give every worker a self-contained task. | 293 | + | - Blocking work too. If your next step depends on a worker's result, spawn it with required:true and end or yield your turn; waiting for a worker is expected, not a reason to work locally. |
| 294 | - | - If you cannot name useful non-overlapping local work, do not delegate. | 294 | + | |
| 295 | - | 295 | + | Keep local: | |
| 296 | - | After delegating: | 296 | + | - Answers, quick lookups, and the short exploration needed to write precise worker tasks. |
| 297 | - | - The worker owns its delegated scope until it completes. Continue only with the non-overlapping local task selected before delegation. | 297 | + | - Edits of a few lines in one file, and integrating, applying, or verifying worker results. |
| 298 | - | - Do not search, read, test, review, edit, or otherwise reproduce the worker's task. You may dispatch additional workers only for genuinely distinct scopes. | 298 | + | |
| 299 | - | - If the user or an active workflow explicitly requires a worker to own blocking work, delegate it with required:true and end or yield your turn instead of duplicating it. | 299 | + | Plan parallelism before spawning: |
| 300 | - | - Integrate and, when appropriate, verify completed worker results before answering. You may apply a completed worktree patch without redoing the delegated task.` | 300 | + | - Map dependencies first: which parts are independent, which must run in order, and which files each part touches. Run independent parts in parallel and order dependent parts. |
| 301 | + | - Split work by distinct question, responsibility, package, or disjoint file set, and give every worker a self-contained task with acceptance checks. | |||
| 302 | + | - One worker editing code: shared workspace is fine. Several workers editing code at once: give each its own tree. | |||
| 303 | + | - For a multi-part feature, create a feature worktree branch, then one sub-worktree per worker with the worktree tool's create action and base set to the feature branch, and tell each worker its exact path. Review each worker's result as a gate, commit on its branch, and merge it into the feature branch. | |||
| 304 | + | - Use isolation:"worktree" for a throwaway parallel attempt whose patch you apply yourself. | |||
| 305 | + | ||||
| 306 | + | Clean up as soon as a worker's result is integrated: | |||
| 307 | + | - Remove merged sub-worktrees and their branches with the worktree tool's cleanup action, setting into to the feature branch and delete_branch to true. | |||
| 308 | + | - Before your final answer, check with the worktree tool's list action that no worktree you created is left behind unless the user should keep it. | |||
| 309 | + | ||||
| 310 | + | When you delegate: | |||
| 311 | + | - A worker owns its scope until it completes. Do not search, read, test, review, edit, or otherwise reproduce it; only coordinate, dispatch disjoint work, or yield. | |||
| 312 | + | - Integrate and verify completed results before answering. You may apply a completed worktree patch without redoing the delegated task.` | |||
| 301 | 313 | ||||
| 302 | // StrictOrchestratorSystemAddendum is used only by explicit headless | 314 | // StrictOrchestratorSystemAddendum is used only by explicit headless | ||
| 303 | // orchestration. The parent coordinates workers and never becomes an | 315 | // orchestration. The parent coordinates workers and never becomes an | ||
| @@ -312,11 +324,11 @@ const StrictOrchestratorSystemAddendum = `Strict subagent orchestration is enabl | |||||
| 312 | 324 | ||||
| 313 | const subagentLifecycleAddendum = `Workers start without this conversation's context. Shared-worktree workers edit the same working directory, so coordinate dependent work and avoid conflicting edits. Use isolation:"worktree" for parallel coding when workers need separate trees; its changes are returned as a patch and never applied automatically. Do not invent feature scope beyond the user's request. Child workers cannot recursively spawn more sub-agents in v1. | 325 | const subagentLifecycleAddendum = `Workers start without this conversation's context. Shared-worktree workers edit the same working directory, so coordinate dependent work and avoid conflicting edits. Use isolation:"worktree" for parallel coding when workers need separate trees; its changes are returned as a patch and never applied automatically. Do not invent feature scope beyond the user's request. Child workers cannot recursively spawn more sub-agents in v1. | ||
| 314 | 326 | ||||
| 315 | - | Use required:true whenever your answer or a declared workflow depends on that worker, especially for required validation or finalization. Required workers remain asynchronous unless you explicitly set the wait field on a spawn or resume action: without it, manager calls return immediately, the parent stays free to coordinate or perform policy-permitted non-overlapping work, and the host delivers terminal outcomes through completion updates. Failed, canceled, or interrupted required work remains unmet: use an enabled manager follow-up action to retry it and do not claim completion. An indeterminate outcome after host restart must not be retried automatically; the user must inspect durable results and side effects, then explicitly resume or restart the worker with a new prompt. Required work has no automatic waiver path. Keep required:false only for independent work that may safely finish later. | 327 | + | Use required:true when your answer or a declared workflow depends on a worker. Failed, canceled, or interrupted required work stays unmet until a successful follow-up; never claim completion before it succeeds. After a host restart, do not retry an indeterminate outcome automatically: the user must reconcile it first. Required work has no automatic waiver path. |
| 316 | 328 | ||||
| 317 | - | Completion is host-event-driven unless you explicitly set the wait field on a spawn or resume action to a whole number of seconds (1–300): on spawn it bounds the initial task, and on resume it bounds the follow-up turn. Omitting wait always returns immediately. An expired wait leaves the accepted child active, whether queued or running; do not retry it until terminal failure, cancellation, or interruption. Never use "bash sleep", "watch", "tail -f", polling loops, repeated "status" calls, or dashboard, metadata, or file checks solely to wait. Those are not completion signals. Continue only with genuinely non-overlapping work; otherwise end or yield your turn until the host injects a completion update. | 329 | + | Completion is host-event-driven: omit the wait field, then continue only non-overlapping work or end or yield your turn; the host injects a completion update. Set wait (1–300 seconds) only to block this turn on a spawn or resume; an expired wait leaves the child active, so do not retry it. Never use "bash sleep", "watch", "tail -f", polling loops, repeated "status" calls, or file checks solely to wait. |
| 318 | 330 | ||||
| 319 | - | When workers finish, use the host update's agent ID, status, task, optional error, and final response or tail to coordinate any follow-up and summarize the outcome. Treat [auto-subagents update] and [required-subagents update] messages as observed worker state, not as new user requests.` | 331 | + | Treat [auto-subagents update] and [required-subagents update] messages as observed worker state, not as new user requests; use their agent ID, status, error, and result to coordinate follow-ups.` |
| 320 | 332 | ||||
| 321 | // OnDemandSubagentsSystemAddendum keeps the canonical subagent tools | 333 | // OnDemandSubagentsSystemAddendum keeps the canonical subagent tools | ||
| 322 | // available without enabling proactive delegation. | 334 | // available without enabling proactive delegation. | ||
| @@ -362,7 +374,7 @@ func subagentsSystemAddendumFor(base, unavailable string, strict bool, actions S | |||||
| 362 | lifecycle = append(lifecycle, "use interrupt to cancel only a worker's running turn while keeping its context, for example before asking it to wrap up") | 374 | lifecycle = append(lifecycle, "use interrupt to cancel only a worker's running turn while keeping its context, for example before asking it to wrap up") | ||
| 363 | } | 375 | } | ||
| 364 | if actions.Stop { | 376 | if actions.Stop { | ||
| 365 | - | lifecycle = append(lifecycle, "use stop to request termination of a stuck worker") | 377 | + | lifecycle = append(lifecycle, "use stop to request termination of a stuck worker, and to remove an idle isolation:\"worktree\" worker's temporary worktree once its patch is applied") |
| 366 | } | 378 | } | ||
| 367 | if len(lifecycle) != 0 { | 379 | if len(lifecycle) != 0 { | ||
| 368 | addendum += "\n\nSubagent manager lifecycle actions available: " + strings.Join(lifecycle, "; ") + "." | 380 | addendum += "\n\nSubagent manager lifecycle actions available: " + strings.Join(lifecycle, "; ") + "." | ||
M packages/agent/settings_store_test.go +16 -9
| @@ -8,7 +8,7 @@ import ( | |||
| 8 | 8 | func TestSubagentsSystemAddendaListOnlyEnabledLifecycleTools(t *testing.T) { | |
| 9 | 9 | // Substrings that identify the lifecycle action guidance. | |
| 10 | 10 | const ( | |
| 11 | - | stopAction = "use stop to request termination of a stuck worker" | |
| 11 | + | stopAction = "use stop to request termination of a stuck worker, and to remove an idle isolation:\"worktree\" worker's temporary worktree" | |
| 12 | 12 | resumeAction = "use resume with an agent id and follow-up prompt" | |
| 13 | 13 | interruptAction = "use interrupt to cancel only a worker's running turn" | |
| 14 | 14 | noSpawnNotice = "Spawning new workers is unavailable" | |
| @@ -70,7 +70,7 @@ func TestSubagentsSystemAddendaListOnlyEnabledLifecycleTools(t *testing.T) { | |||
| 70 | 70 | ||
| 71 | 71 | func TestSubagentPoliciesSeparateProactiveAndStrictOwnership(t *testing.T) { | |
| 72 | 72 | proactive := ProactiveSubagentsSystemAddendumFor(SubagentActions{Spawn: true, Stop: false, Resume: false}) | |
| 73 | - | for _, want := range []string{"primary owner and implementer", "immediate next task you will perform locally", "If you cannot name useful non-overlapping local work, do not delegate", "Do not search, read, test, review, edit"} { | |
| 73 | + | for _, want := range []string{"delegate by default and keep work local by exception", "spawn it with required:true and end or yield your turn", "Edits of a few lines in one file", "Do not search, read, test, review, edit", "Map dependencies first", "base set to the feature branch", "Review each worker's result as a gate", "into to the feature branch and delete_branch to true", "no worktree you created is left behind"} { | |
| 74 | 74 | if !strings.Contains(proactive, want) { | |
| 75 | 75 | t.Fatalf("proactive addendum missing %q:\n%s", want, proactive) | |
| 76 | 76 | } | |
| @@ -78,6 +78,13 @@ func TestSubagentPoliciesSeparateProactiveAndStrictOwnership(t *testing.T) { | |||
| 78 | 78 | if strings.Contains(proactive, "not an implementer") { | |
| 79 | 79 | t.Fatalf("proactive addendum retained strict orchestrator contract:\n%s", proactive) | |
| 80 | 80 | } | |
| 81 | + | // These lines vetoed delegation whenever the next step depended on a | |
| 82 | + | // result, which made the primary implement nearly every task itself. | |
| 83 | + | for _, unwanted := range []string{"primary owner and implementer", "do that work yourself instead of spawning", "do not delegate"} { | |
| 84 | + | if strings.Contains(proactive, unwanted) { | |
| 85 | + | t.Fatalf("proactive addendum kept delegation veto %q:\n%s", unwanted, proactive) | |
| 86 | + | } | |
| 87 | + | } | |
| 81 | 88 | ||
| 82 | 89 | strict := StrictOrchestratorSystemAddendumFor(SubagentActions{Spawn: true, Stop: false, Resume: false}) | |
| 83 | 90 | for _, want := range []string{"not an implementer", "non-overlapping worker scopes", "Once a worker is active", "Only coordinate workers"} { | |
| @@ -90,11 +97,11 @@ func TestSubagentPoliciesSeparateProactiveAndStrictOwnership(t *testing.T) { | |||
| 90 | 97 | func TestProactiveAddendumRoutesBeforeWorkOnlyWhenSpawningIsAvailable(t *testing.T) { | |
| 91 | 98 | proactive := ProactiveSubagentsSystemAddendumFor(SubagentActions{Spawn: true, Stop: false, Resume: false}) | |
| 92 | 99 | for _, want := range []string{ | |
| 93 | - | "Route before you work:", | |
| 94 | - | "routing happens when the user states the request you are about to work on", | |
| 95 | - | "the [subagents_list] profile that owns each delegable part", | |
| 96 | - | "let the tool calls that follow speak for themselves", | |
| 97 | - | "Say plainly when nothing is worth delegating", | |
| 100 | + | "Route every new task before you work:", | |
| 101 | + | "Re-route each new request, even late in a long session", | |
| 102 | + | "the [subagents_list] profile for each delegated part", | |
| 103 | + | "do not narrate the tool calls that follow", | |
| 104 | + | "Keep a task fully local only when it is small", | |
| 98 | 105 | } { | |
| 99 | 106 | if !strings.Contains(proactive, want) { | |
| 100 | 107 | t.Fatalf("proactive addendum missing %q:\n%s", want, proactive) | |
| @@ -115,7 +122,7 @@ func TestProactiveAddendumRoutesBeforeWorkOnlyWhenSpawningIsAvailable(t *testing | |||
| 115 | 122 | } { | |
| 116 | 123 | t.Run(tc.name, func(t *testing.T) { | |
| 117 | 124 | got := ProactiveSubagentsSystemAddendumFor(SubagentActions{Spawn: false, Stop: tc.stop, Resume: tc.resume}) | |
| 118 | - | for _, unwanted := range []string{"Route before you work:", "[subagents_list]", "spawn a clearly described general worker"} { | |
| 125 | + | for _, unwanted := range []string{"Route every new task before you work:", "[subagents_list]", "Spawn a clearly described general worker"} { | |
| 119 | 126 | if strings.Contains(got, unwanted) { | |
| 120 | 127 | t.Fatalf("spawn-withheld addendum kept %q:\n%s", unwanted, got) | |
| 121 | 128 | } | |
| @@ -138,7 +145,7 @@ func TestProactiveAddendumRoutesBeforeWorkOnlyWhenSpawningIsAvailable(t *testing | |||
| 138 | 145 | // Routing is the interactive collaboration step; the headless strict | |
| 139 | 146 | // contract divides scopes itself and must not inherit it. | |
| 140 | 147 | strict := StrictOrchestratorSystemAddendumFor(SubagentActions{Spawn: true, Stop: false, Resume: false}) | |
| 141 | - | if strings.Contains(strict, "Route before you work:") || strings.Contains(strict, "[subagents_list]") { | |
| 148 | + | if strings.Contains(strict, "Route every new task before you work:") || strings.Contains(strict, "[subagents_list]") { | |
| 142 | 149 | t.Fatalf("strict orchestrator inherited interactive routing guidance:\n%s", strict) | |
| 143 | 150 | } | |
| 144 | 151 | } | |
| @@ -8,7 +8,7 @@ import ( | |||||
| 8 | func TestSubagentsSystemAddendaListOnlyEnabledLifecycleTools(t *testing.T) { | 8 | func TestSubagentsSystemAddendaListOnlyEnabledLifecycleTools(t *testing.T) { | ||
| 9 | // Substrings that identify the lifecycle action guidance. | 9 | // Substrings that identify the lifecycle action guidance. | ||
| 10 | const ( | 10 | const ( | ||
| 11 | - | stopAction = "use stop to request termination of a stuck worker" | 11 | + | stopAction = "use stop to request termination of a stuck worker, and to remove an idle isolation:\"worktree\" worker's temporary worktree" |
| 12 | resumeAction = "use resume with an agent id and follow-up prompt" | 12 | resumeAction = "use resume with an agent id and follow-up prompt" | ||
| 13 | interruptAction = "use interrupt to cancel only a worker's running turn" | 13 | interruptAction = "use interrupt to cancel only a worker's running turn" | ||
| 14 | noSpawnNotice = "Spawning new workers is unavailable" | 14 | noSpawnNotice = "Spawning new workers is unavailable" | ||
| @@ -70,7 +70,7 @@ func TestSubagentsSystemAddendaListOnlyEnabledLifecycleTools(t *testing.T) { | |||||
| 70 | 70 | ||||
| 71 | func TestSubagentPoliciesSeparateProactiveAndStrictOwnership(t *testing.T) { | 71 | func TestSubagentPoliciesSeparateProactiveAndStrictOwnership(t *testing.T) { | ||
| 72 | proactive := ProactiveSubagentsSystemAddendumFor(SubagentActions{Spawn: true, Stop: false, Resume: false}) | 72 | proactive := ProactiveSubagentsSystemAddendumFor(SubagentActions{Spawn: true, Stop: false, Resume: false}) | ||
| 73 | - | for _, want := range []string{"primary owner and implementer", "immediate next task you will perform locally", "If you cannot name useful non-overlapping local work, do not delegate", "Do not search, read, test, review, edit"} { | 73 | + | for _, want := range []string{"delegate by default and keep work local by exception", "spawn it with required:true and end or yield your turn", "Edits of a few lines in one file", "Do not search, read, test, review, edit", "Map dependencies first", "base set to the feature branch", "Review each worker's result as a gate", "into to the feature branch and delete_branch to true", "no worktree you created is left behind"} { |
| 74 | if !strings.Contains(proactive, want) { | 74 | if !strings.Contains(proactive, want) { | ||
| 75 | t.Fatalf("proactive addendum missing %q:\n%s", want, proactive) | 75 | t.Fatalf("proactive addendum missing %q:\n%s", want, proactive) | ||
| 76 | } | 76 | } | ||
| @@ -78,6 +78,13 @@ func TestSubagentPoliciesSeparateProactiveAndStrictOwnership(t *testing.T) { | |||||
| 78 | if strings.Contains(proactive, "not an implementer") { | 78 | if strings.Contains(proactive, "not an implementer") { | ||
| 79 | t.Fatalf("proactive addendum retained strict orchestrator contract:\n%s", proactive) | 79 | t.Fatalf("proactive addendum retained strict orchestrator contract:\n%s", proactive) | ||
| 80 | } | 80 | } | ||
| 81 | + | // These lines vetoed delegation whenever the next step depended on a | |||
| 82 | + | // result, which made the primary implement nearly every task itself. | |||
| 83 | + | for _, unwanted := range []string{"primary owner and implementer", "do that work yourself instead of spawning", "do not delegate"} { | |||
| 84 | + | if strings.Contains(proactive, unwanted) { | |||
| 85 | + | t.Fatalf("proactive addendum kept delegation veto %q:\n%s", unwanted, proactive) | |||
| 86 | + | } | |||
| 87 | + | } | |||
| 81 | 88 | ||||
| 82 | strict := StrictOrchestratorSystemAddendumFor(SubagentActions{Spawn: true, Stop: false, Resume: false}) | 89 | strict := StrictOrchestratorSystemAddendumFor(SubagentActions{Spawn: true, Stop: false, Resume: false}) | ||
| 83 | for _, want := range []string{"not an implementer", "non-overlapping worker scopes", "Once a worker is active", "Only coordinate workers"} { | 90 | for _, want := range []string{"not an implementer", "non-overlapping worker scopes", "Once a worker is active", "Only coordinate workers"} { | ||
| @@ -90,11 +97,11 @@ func TestSubagentPoliciesSeparateProactiveAndStrictOwnership(t *testing.T) { | |||||
| 90 | func TestProactiveAddendumRoutesBeforeWorkOnlyWhenSpawningIsAvailable(t *testing.T) { | 97 | func TestProactiveAddendumRoutesBeforeWorkOnlyWhenSpawningIsAvailable(t *testing.T) { | ||
| 91 | proactive := ProactiveSubagentsSystemAddendumFor(SubagentActions{Spawn: true, Stop: false, Resume: false}) | 98 | proactive := ProactiveSubagentsSystemAddendumFor(SubagentActions{Spawn: true, Stop: false, Resume: false}) | ||
| 92 | for _, want := range []string{ | 99 | for _, want := range []string{ | ||
| 93 | - | "Route before you work:", | 100 | + | "Route every new task before you work:", |
| 94 | - | "routing happens when the user states the request you are about to work on", | 101 | + | "Re-route each new request, even late in a long session", |
| 95 | - | "the [subagents_list] profile that owns each delegable part", | 102 | + | "the [subagents_list] profile for each delegated part", |
| 96 | - | "let the tool calls that follow speak for themselves", | 103 | + | "do not narrate the tool calls that follow", |
| 97 | - | "Say plainly when nothing is worth delegating", | 104 | + | "Keep a task fully local only when it is small", |
| 98 | } { | 105 | } { | ||
| 99 | if !strings.Contains(proactive, want) { | 106 | if !strings.Contains(proactive, want) { | ||
| 100 | t.Fatalf("proactive addendum missing %q:\n%s", want, proactive) | 107 | t.Fatalf("proactive addendum missing %q:\n%s", want, proactive) | ||
| @@ -115,7 +122,7 @@ func TestProactiveAddendumRoutesBeforeWorkOnlyWhenSpawningIsAvailable(t *testing | |||||
| 115 | } { | 122 | } { | ||
| 116 | t.Run(tc.name, func(t *testing.T) { | 123 | t.Run(tc.name, func(t *testing.T) { | ||
| 117 | got := ProactiveSubagentsSystemAddendumFor(SubagentActions{Spawn: false, Stop: tc.stop, Resume: tc.resume}) | 124 | got := ProactiveSubagentsSystemAddendumFor(SubagentActions{Spawn: false, Stop: tc.stop, Resume: tc.resume}) | ||
| 118 | - | for _, unwanted := range []string{"Route before you work:", "[subagents_list]", "spawn a clearly described general worker"} { | 125 | + | for _, unwanted := range []string{"Route every new task before you work:", "[subagents_list]", "Spawn a clearly described general worker"} { |
| 119 | if strings.Contains(got, unwanted) { | 126 | if strings.Contains(got, unwanted) { | ||
| 120 | t.Fatalf("spawn-withheld addendum kept %q:\n%s", unwanted, got) | 127 | t.Fatalf("spawn-withheld addendum kept %q:\n%s", unwanted, got) | ||
| 121 | } | 128 | } | ||
| @@ -138,7 +145,7 @@ func TestProactiveAddendumRoutesBeforeWorkOnlyWhenSpawningIsAvailable(t *testing | |||||
| 138 | // Routing is the interactive collaboration step; the headless strict | 145 | // Routing is the interactive collaboration step; the headless strict | ||
| 139 | // contract divides scopes itself and must not inherit it. | 146 | // contract divides scopes itself and must not inherit it. | ||
| 140 | strict := StrictOrchestratorSystemAddendumFor(SubagentActions{Spawn: true, Stop: false, Resume: false}) | 147 | strict := StrictOrchestratorSystemAddendumFor(SubagentActions{Spawn: true, Stop: false, Resume: false}) | ||
| 141 | - | if strings.Contains(strict, "Route before you work:") || strings.Contains(strict, "[subagents_list]") { | 148 | + | if strings.Contains(strict, "Route every new task before you work:") || strings.Contains(strict, "[subagents_list]") { |
| 142 | t.Fatalf("strict orchestrator inherited interactive routing guidance:\n%s", strict) | 149 | t.Fatalf("strict orchestrator inherited interactive routing guidance:\n%s", strict) | ||
| 143 | } | 150 | } | ||
| 144 | } | 151 | } | ||
M packages/agent/tools/create_worktree.go +34 -5
| @@ -46,6 +46,7 @@ type CreateWorktreeTool struct { | |||
| 46 | 46 | ||
| 47 | 47 | type createWorktreeArgs struct { | |
| 48 | 48 | Branch string `json:"branch"` | |
| 49 | + | Base string `json:"base,omitempty"` | |
| 49 | 50 | } | |
| 50 | 51 | ||
| 51 | 52 | const createWorktreeSchema = `{ | |
| @@ -54,6 +55,10 @@ const createWorktreeSchema = `{ | |||
| 54 | 55 | "branch":{ | |
| 55 | 56 | "type":"string", | |
| 56 | 57 | "description":"Name of the new Git branch." | |
| 58 | + | }, | |
| 59 | + | "base":{ | |
| 60 | + | "type":"string", | |
| 61 | + | "description":"Branch name, tag, or commit to start the new branch from. Defaults to the current HEAD. Use a feature branch to create a sub-worktree based on it." | |
| 57 | 62 | } | |
| 58 | 63 | }, | |
| 59 | 64 | "required":["branch"] | |
| @@ -152,7 +157,8 @@ func (t *CreateWorktreeTool) planForExecute(ctx context.Context, raw json.RawMes | |||
| 152 | 157 | type createWorktreePlan struct { | |
| 153 | 158 | repoRoot string | |
| 154 | 159 | branch string | |
| 155 | - | base string | |
| 160 | + | base string // resolved commit | |
| 161 | + | baseRef string // requested base ref; empty for the current HEAD | |
| 156 | 162 | worktreeRoot string | |
| 157 | 163 | worktreePath string | |
| 158 | 164 | rootSource string | |
| @@ -206,9 +212,16 @@ func (t *CreateWorktreeTool) plan(ctx context.Context, raw json.RawMessage) (cre | |||
| 206 | 212 | if branch != branchInput { | |
| 207 | 213 | return createWorktreePlan{}, fmt.Errorf("create_worktree: invalid branch %q", branchInput) | |
| 208 | 214 | } | |
| 209 | - | base, err := createWorktreeGitOutput(ctx, repoRoot, "rev-parse", "--verify", "HEAD^{commit}") | |
| 215 | + | baseRef := strings.TrimSpace(args.Base) | |
| 216 | + | if strings.HasPrefix(baseRef, "-") { | |
| 217 | + | return createWorktreePlan{}, fmt.Errorf("create_worktree: invalid base %q", baseRef) | |
| 218 | + | } | |
| 219 | + | base, err := resolveWorktreeCommit(ctx, repoRoot, baseRef) | |
| 210 | 220 | if err != nil { | |
| 211 | - | return createWorktreePlan{}, fmt.Errorf("create_worktree: resolve current commit: %w", err) | |
| 221 | + | if baseRef == "" { | |
| 222 | + | return createWorktreePlan{}, fmt.Errorf("create_worktree: resolve current commit: %w", err) | |
| 223 | + | } | |
| 224 | + | return createWorktreePlan{}, fmt.Errorf("create_worktree: invalid base %q: %w", baseRef, err) | |
| 212 | 225 | } | |
| 213 | 226 | commonDir, err := createWorktreeGitOutput(ctx, repoRoot, "rev-parse", "--git-common-dir") | |
| 214 | 227 | if err != nil { | |
| @@ -283,11 +296,16 @@ func (t *CreateWorktreeTool) plan(ctx context.Context, raw json.RawMessage) (cre | |||
| 283 | 296 | ||
| 284 | 297 | displayRoot := filepath.ToSlash(worktreeRoot) | |
| 285 | 298 | displayPath := filepath.ToSlash(worktreePath) | |
| 286 | - | previewText := fmt.Sprintf("create worktree\nbranch: %s\nbase: %s\nworktree root: %s\nroot source: %s\npath: %s\n", branch, base, displayRoot, rootSource, displayPath) | |
| 299 | + | baseDisplay := base | |
| 300 | + | if baseRef != "" { | |
| 301 | + | baseDisplay = fmt.Sprintf("%s (%s)", baseRef, base) | |
| 302 | + | } | |
| 303 | + | previewText := fmt.Sprintf("create worktree\nbranch: %s\nbase: %s\nworktree root: %s\nroot source: %s\npath: %s\n", branch, baseDisplay, displayRoot, rootSource, displayPath) | |
| 287 | 304 | return createWorktreePlan{ | |
| 288 | 305 | repoRoot: repoRoot, | |
| 289 | 306 | branch: branch, | |
| 290 | 307 | base: base, | |
| 308 | + | baseRef: baseRef, | |
| 291 | 309 | worktreeRoot: worktreeRoot, | |
| 292 | 310 | worktreePath: worktreePath, | |
| 293 | 311 | rootSource: rootSource, | |
| @@ -299,6 +317,7 @@ func (t *CreateWorktreeTool) plan(ctx context.Context, raw json.RawMessage) (cre | |||
| 299 | 317 | "state": "ready", | |
| 300 | 318 | "branch": branch, | |
| 301 | 319 | "base": base, | |
| 320 | + | "base_ref": baseRef, | |
| 302 | 321 | "worktree_root": displayRoot, | |
| 303 | 322 | "root_source": rootSource, | |
| 304 | 323 | "repo_id": repoID, | |
| @@ -308,6 +327,15 @@ func (t *CreateWorktreeTool) plan(ctx context.Context, raw json.RawMessage) (cre | |||
| 308 | 327 | }, nil | |
| 309 | 328 | } | |
| 310 | 329 | ||
| 330 | + | // resolveWorktreeCommit resolves ref (HEAD when empty) to a commit id. Callers | |
| 331 | + | // must reject refs that begin with "-" first so a ref cannot become an option. | |
| 332 | + | func resolveWorktreeCommit(ctx context.Context, repoRoot, ref string) (string, error) { | |
| 333 | + | if ref == "" { | |
| 334 | + | ref = "HEAD" | |
| 335 | + | } | |
| 336 | + | return createWorktreeGitOutput(ctx, repoRoot, "rev-parse", "--verify", ref+"^{commit}") | |
| 337 | + | } | |
| 338 | + | ||
| 311 | 339 | func (t *CreateWorktreeTool) defaultRoot() (string, error) { | |
| 312 | 340 | root := strings.TrimSpace(t.DefaultRoot) | |
| 313 | 341 | if root == "" { | |
| @@ -449,6 +477,7 @@ func sameCreateWorktreePlan(a, b createWorktreePlan) bool { | |||
| 449 | 477 | return a.repoRoot == b.repoRoot && | |
| 450 | 478 | a.branch == b.branch && | |
| 451 | 479 | a.base == b.base && | |
| 480 | + | a.baseRef == b.baseRef && | |
| 452 | 481 | a.worktreeRoot == b.worktreeRoot && | |
| 453 | 482 | a.worktreePath == b.worktreePath && | |
| 454 | 483 | a.rootSource == b.rootSource && | |
| @@ -548,7 +577,7 @@ func createWorktreeBranchExists(ctx context.Context, repoRoot, branch string) (b | |||
| 548 | 577 | } | |
| 549 | 578 | ||
| 550 | 579 | func createWorktreeGitRef(ctx context.Context, repoRoot, ref string) (string, bool, error) { | |
| 551 | - | output, err := createWorktreeGitOutput(ctx, repoRoot, "rev-parse", "--verify", "--quiet", ref+"^{commit}") | |
| 580 | + | output, err := createWorktreeGitOutput(ctx, repoRoot, "rev-parse", "--verify", ref+"^{commit}") | |
| 552 | 581 | if err == nil { | |
| 553 | 582 | return output, true, nil | |
| 554 | 583 | } | |
| @@ -46,6 +46,7 @@ type CreateWorktreeTool struct { | |||||
| 46 | 46 | ||||
| 47 | type createWorktreeArgs struct { | 47 | type createWorktreeArgs struct { | ||
| 48 | Branch string `json:"branch"` | 48 | Branch string `json:"branch"` | ||
| 49 | + | Base string `json:"base,omitempty"` | |||
| 49 | } | 50 | } | ||
| 50 | 51 | ||||
| 51 | const createWorktreeSchema = `{ | 52 | const createWorktreeSchema = `{ | ||
| @@ -54,6 +55,10 @@ const createWorktreeSchema = `{ | |||||
| 54 | "branch":{ | 55 | "branch":{ | ||
| 55 | "type":"string", | 56 | "type":"string", | ||
| 56 | "description":"Name of the new Git branch." | 57 | "description":"Name of the new Git branch." | ||
| 58 | + | }, | |||
| 59 | + | "base":{ | |||
| 60 | + | "type":"string", | |||
| 61 | + | "description":"Branch name, tag, or commit to start the new branch from. Defaults to the current HEAD. Use a feature branch to create a sub-worktree based on it." | |||
| 57 | } | 62 | } | ||
| 58 | }, | 63 | }, | ||
| 59 | "required":["branch"] | 64 | "required":["branch"] | ||
| @@ -152,7 +157,8 @@ func (t *CreateWorktreeTool) planForExecute(ctx context.Context, raw json.RawMes | |||||
| 152 | type createWorktreePlan struct { | 157 | type createWorktreePlan struct { | ||
| 153 | repoRoot string | 158 | repoRoot string | ||
| 154 | branch string | 159 | branch string | ||
| 155 | - | base string | 160 | + | base string // resolved commit |
| 161 | + | baseRef string // requested base ref; empty for the current HEAD | |||
| 156 | worktreeRoot string | 162 | worktreeRoot string | ||
| 157 | worktreePath string | 163 | worktreePath string | ||
| 158 | rootSource string | 164 | rootSource string | ||
| @@ -206,9 +212,16 @@ func (t *CreateWorktreeTool) plan(ctx context.Context, raw json.RawMessage) (cre | |||||
| 206 | if branch != branchInput { | 212 | if branch != branchInput { | ||
| 207 | return createWorktreePlan{}, fmt.Errorf("create_worktree: invalid branch %q", branchInput) | 213 | return createWorktreePlan{}, fmt.Errorf("create_worktree: invalid branch %q", branchInput) | ||
| 208 | } | 214 | } | ||
| 209 | - | base, err := createWorktreeGitOutput(ctx, repoRoot, "rev-parse", "--verify", "HEAD^{commit}") | 215 | + | baseRef := strings.TrimSpace(args.Base) |
| 216 | + | if strings.HasPrefix(baseRef, "-") { | |||
| 217 | + | return createWorktreePlan{}, fmt.Errorf("create_worktree: invalid base %q", baseRef) | |||
| 218 | + | } | |||
| 219 | + | base, err := resolveWorktreeCommit(ctx, repoRoot, baseRef) | |||
| 210 | if err != nil { | 220 | if err != nil { | ||
| 211 | - | return createWorktreePlan{}, fmt.Errorf("create_worktree: resolve current commit: %w", err) | 221 | + | if baseRef == "" { |
| 222 | + | return createWorktreePlan{}, fmt.Errorf("create_worktree: resolve current commit: %w", err) | |||
| 223 | + | } | |||
| 224 | + | return createWorktreePlan{}, fmt.Errorf("create_worktree: invalid base %q: %w", baseRef, err) | |||
| 212 | } | 225 | } | ||
| 213 | commonDir, err := createWorktreeGitOutput(ctx, repoRoot, "rev-parse", "--git-common-dir") | 226 | commonDir, err := createWorktreeGitOutput(ctx, repoRoot, "rev-parse", "--git-common-dir") | ||
| 214 | if err != nil { | 227 | if err != nil { | ||
| @@ -283,11 +296,16 @@ func (t *CreateWorktreeTool) plan(ctx context.Context, raw json.RawMessage) (cre | |||||
| 283 | 296 | ||||
| 284 | displayRoot := filepath.ToSlash(worktreeRoot) | 297 | displayRoot := filepath.ToSlash(worktreeRoot) | ||
| 285 | displayPath := filepath.ToSlash(worktreePath) | 298 | displayPath := filepath.ToSlash(worktreePath) | ||
| 286 | - | previewText := fmt.Sprintf("create worktree\nbranch: %s\nbase: %s\nworktree root: %s\nroot source: %s\npath: %s\n", branch, base, displayRoot, rootSource, displayPath) | 299 | + | baseDisplay := base |
| 300 | + | if baseRef != "" { | |||
| 301 | + | baseDisplay = fmt.Sprintf("%s (%s)", baseRef, base) | |||
| 302 | + | } | |||
| 303 | + | previewText := fmt.Sprintf("create worktree\nbranch: %s\nbase: %s\nworktree root: %s\nroot source: %s\npath: %s\n", branch, baseDisplay, displayRoot, rootSource, displayPath) | |||
| 287 | return createWorktreePlan{ | 304 | return createWorktreePlan{ | ||
| 288 | repoRoot: repoRoot, | 305 | repoRoot: repoRoot, | ||
| 289 | branch: branch, | 306 | branch: branch, | ||
| 290 | base: base, | 307 | base: base, | ||
| 308 | + | baseRef: baseRef, | |||
| 291 | worktreeRoot: worktreeRoot, | 309 | worktreeRoot: worktreeRoot, | ||
| 292 | worktreePath: worktreePath, | 310 | worktreePath: worktreePath, | ||
| 293 | rootSource: rootSource, | 311 | rootSource: rootSource, | ||
| @@ -299,6 +317,7 @@ func (t *CreateWorktreeTool) plan(ctx context.Context, raw json.RawMessage) (cre | |||||
| 299 | "state": "ready", | 317 | "state": "ready", | ||
| 300 | "branch": branch, | 318 | "branch": branch, | ||
| 301 | "base": base, | 319 | "base": base, | ||
| 320 | + | "base_ref": baseRef, | |||
| 302 | "worktree_root": displayRoot, | 321 | "worktree_root": displayRoot, | ||
| 303 | "root_source": rootSource, | 322 | "root_source": rootSource, | ||
| 304 | "repo_id": repoID, | 323 | "repo_id": repoID, | ||
| @@ -308,6 +327,15 @@ func (t *CreateWorktreeTool) plan(ctx context.Context, raw json.RawMessage) (cre | |||||
| 308 | }, nil | 327 | }, nil | ||
| 309 | } | 328 | } | ||
| 310 | 329 | ||||
| 330 | + | // resolveWorktreeCommit resolves ref (HEAD when empty) to a commit id. Callers | |||
| 331 | + | // must reject refs that begin with "-" first so a ref cannot become an option. | |||
| 332 | + | func resolveWorktreeCommit(ctx context.Context, repoRoot, ref string) (string, error) { | |||
| 333 | + | if ref == "" { | |||
| 334 | + | ref = "HEAD" | |||
| 335 | + | } | |||
| 336 | + | return createWorktreeGitOutput(ctx, repoRoot, "rev-parse", "--verify", ref+"^{commit}") | |||
| 337 | + | } | |||
| 338 | + | ||||
| 311 | func (t *CreateWorktreeTool) defaultRoot() (string, error) { | 339 | func (t *CreateWorktreeTool) defaultRoot() (string, error) { | ||
| 312 | root := strings.TrimSpace(t.DefaultRoot) | 340 | root := strings.TrimSpace(t.DefaultRoot) | ||
| 313 | if root == "" { | 341 | if root == "" { | ||
| @@ -449,6 +477,7 @@ func sameCreateWorktreePlan(a, b createWorktreePlan) bool { | |||||
| 449 | return a.repoRoot == b.repoRoot && | 477 | return a.repoRoot == b.repoRoot && | ||
| 450 | a.branch == b.branch && | 478 | a.branch == b.branch && | ||
| 451 | a.base == b.base && | 479 | a.base == b.base && | ||
| 480 | + | a.baseRef == b.baseRef && | |||
| 452 | a.worktreeRoot == b.worktreeRoot && | 481 | a.worktreeRoot == b.worktreeRoot && | ||
| 453 | a.worktreePath == b.worktreePath && | 482 | a.worktreePath == b.worktreePath && | ||
| 454 | a.rootSource == b.rootSource && | 483 | a.rootSource == b.rootSource && | ||
| @@ -548,7 +577,7 @@ func createWorktreeBranchExists(ctx context.Context, repoRoot, branch string) (b | |||||
| 548 | } | 577 | } | ||
| 549 | 578 | ||||
| 550 | func createWorktreeGitRef(ctx context.Context, repoRoot, ref string) (string, bool, error) { | 579 | func createWorktreeGitRef(ctx context.Context, repoRoot, ref string) (string, bool, error) { | ||
| 551 | - | output, err := createWorktreeGitOutput(ctx, repoRoot, "rev-parse", "--verify", "--quiet", ref+"^{commit}") | 580 | + | output, err := createWorktreeGitOutput(ctx, repoRoot, "rev-parse", "--verify", ref+"^{commit}") |
| 552 | if err == nil { | 581 | if err == nil { | ||
| 553 | return output, true, nil | 582 | return output, true, nil | ||
| 554 | } | 583 | } | ||
M packages/agent/tools/create_worktree_test.go +5 -2
| @@ -265,8 +265,11 @@ func TestCreateWorktreeSchemaDropsBootstrapRoot(t *testing.T) { | |||
| 265 | 265 | if _, ok := parsed.Properties["branch"]; !ok { | |
| 266 | 266 | t.Fatalf("schema lacks branch: %s", schema) | |
| 267 | 267 | } | |
| 268 | - | if len(parsed.Properties) != 1 { | |
| 269 | - | t.Fatalf("schema properties = %v, want only branch", parsed.Properties) | |
| 268 | + | if _, ok := parsed.Properties["base"]; !ok { | |
| 269 | + | t.Fatalf("schema lacks base: %s", schema) | |
| 270 | + | } | |
| 271 | + | if len(parsed.Properties) != 2 { | |
| 272 | + | t.Fatalf("schema properties = %v, want only branch and base", parsed.Properties) | |
| 270 | 273 | } | |
| 271 | 274 | if len(parsed.Required) != 1 || parsed.Required[0] != "branch" { | |
| 272 | 275 | t.Fatalf("schema required = %v", parsed.Required) | |
| @@ -265,8 +265,11 @@ func TestCreateWorktreeSchemaDropsBootstrapRoot(t *testing.T) { | |||||
| 265 | if _, ok := parsed.Properties["branch"]; !ok { | 265 | if _, ok := parsed.Properties["branch"]; !ok { | ||
| 266 | t.Fatalf("schema lacks branch: %s", schema) | 266 | t.Fatalf("schema lacks branch: %s", schema) | ||
| 267 | } | 267 | } | ||
| 268 | - | if len(parsed.Properties) != 1 { | 268 | + | if _, ok := parsed.Properties["base"]; !ok { |
| 269 | - | t.Fatalf("schema properties = %v, want only branch", parsed.Properties) | 269 | + | t.Fatalf("schema lacks base: %s", schema) |
| 270 | + | } | |||
| 271 | + | if len(parsed.Properties) != 2 { | |||
| 272 | + | t.Fatalf("schema properties = %v, want only branch and base", parsed.Properties) | |||
| 270 | } | 273 | } | ||
| 271 | if len(parsed.Required) != 1 || parsed.Required[0] != "branch" { | 274 | if len(parsed.Required) != 1 || parsed.Required[0] != "branch" { | ||
| 272 | t.Fatalf("schema required = %v", parsed.Required) | 275 | t.Fatalf("schema required = %v", parsed.Required) | ||
M packages/agent/tools/manage_worktrees.go +193 -14
| @@ -25,8 +25,10 @@ type ManageWorktreesTool struct { | |||
| 25 | 25 | } | |
| 26 | 26 | ||
| 27 | 27 | type manageWorktreesArgs struct { | |
| 28 | - | Action string `json:"action"` | |
| 29 | - | Paths []string `json:"paths,omitempty"` | |
| 28 | + | Action string `json:"action"` | |
| 29 | + | Paths []string `json:"paths,omitempty"` | |
| 30 | + | Into string `json:"into,omitempty"` | |
| 31 | + | DeleteBranch bool `json:"delete_branch,omitempty"` | |
| 30 | 32 | } | |
| 31 | 33 | ||
| 32 | 34 | type managedWorktree struct { | |
| @@ -44,9 +46,17 @@ type managedWorktree struct { | |||
| 44 | 46 | type manageWorktreesPlan struct { | |
| 45 | 47 | repoRoot string | |
| 46 | 48 | action string | |
| 47 | - | selected []managedWorktree | |
| 48 | - | all []managedWorktree | |
| 49 | - | text string | |
| 49 | + | // into is the requested merge target; empty means the current HEAD. | |
| 50 | + | into string | |
| 51 | + | deleteBranch bool | |
| 52 | + | // intoCommit and intoRef pin the resolved merge target so a target that | |
| 53 | + | // moves between preview and execute is detected. intoRef is the symbolic | |
| 54 | + | // full name (empty for a commit or detached HEAD). | |
| 55 | + | intoCommit string | |
| 56 | + | intoRef string | |
| 57 | + | selected []managedWorktree | |
| 58 | + | all []managedWorktree | |
| 59 | + | text string | |
| 50 | 60 | } | |
| 51 | 61 | ||
| 52 | 62 | type manageWorktreesPreview struct { | |
| @@ -58,7 +68,9 @@ const manageWorktreesSchema = `{ | |||
| 58 | 68 | "type":"object", | |
| 59 | 69 | "properties":{ | |
| 60 | 70 | "action":{"type":"string","enum":["list","cleanup"],"description":"List repository worktrees, or remove explicitly selected worktrees that are clean and merged."}, | |
| 61 | - | "paths":{"type":"array","items":{"type":"string"},"description":"Exact worktree paths to remove. Required for cleanup."} | |
| 71 | + | "paths":{"type":"array","items":{"type":"string"},"description":"Exact worktree paths to remove. Required for cleanup."}, | |
| 72 | + | "into":{"type":"string","description":"Branch, tag, or commit the worktree branches must be merged into. Defaults to the current HEAD."}, | |
| 73 | + | "delete_branch":{"type":"boolean","description":"Also delete each removed worktree's local branch if it is still at the inspected commit."} | |
| 62 | 74 | }, | |
| 63 | 75 | "required":["action"] | |
| 64 | 76 | }` | |
| @@ -95,16 +107,115 @@ func (t *ManageWorktreesTool) Execute(ctx context.Context, raw json.RawMessage, | |||
| 95 | 107 | if plan.action == "list" { | |
| 96 | 108 | return manageWorktreesResult(plan, "listed"), nil | |
| 97 | 109 | } | |
| 110 | + | var removed, deleted []string | |
| 111 | + | var branchErrs []string | |
| 112 | + | // summary reports progress made so far; it is attached to every failure | |
| 113 | + | // return so a partial cleanup is never silently dropped. | |
| 114 | + | summary := func() string { | |
| 115 | + | var parts []string | |
| 116 | + | if len(branchErrs) > 0 { | |
| 117 | + | parts = append(parts, strings.Join(branchErrs, "; ")) | |
| 118 | + | } | |
| 119 | + | if len(removed) > 0 { | |
| 120 | + | parts = append(parts, "removed worktrees: "+strings.Join(removed, ", ")) | |
| 121 | + | } | |
| 122 | + | if len(deleted) > 0 { | |
| 123 | + | parts = append(parts, "deleted branches: "+strings.Join(deleted, ", ")) | |
| 124 | + | } | |
| 125 | + | if len(parts) == 0 { | |
| 126 | + | return "" | |
| 127 | + | } | |
| 128 | + | return "; " + strings.Join(parts, "; ") | |
| 129 | + | } | |
| 98 | 130 | for _, worktree := range plan.selected { | |
| 99 | 131 | if progress != nil { | |
| 100 | 132 | progress("Removing worktree " + filepath.ToSlash(worktree.Path) + "...\n") | |
| 101 | 133 | } | |
| 102 | 134 | if _, err := manageWorktreesGit(ctx, plan.repoRoot, "worktree", "remove", worktree.Path); err != nil { | |
| 103 | - | return core.ToolResult{}, fmt.Errorf("manage_worktrees: remove %q: %w", filepath.ToSlash(worktree.Path), err) | |
| 135 | + | _, _ = manageWorktreesGit(ctx, plan.repoRoot, "worktree", "prune") | |
| 136 | + | return core.ToolResult{}, fmt.Errorf("manage_worktrees: remove %q: %w%s", filepath.ToSlash(worktree.Path), err, summary()) | |
| 137 | + | } | |
| 138 | + | removed = append(removed, filepath.ToSlash(worktree.Path)) | |
| 139 | + | if !plan.deleteBranch { | |
| 140 | + | continue | |
| 104 | 141 | } | |
| 142 | + | if progress != nil { | |
| 143 | + | progress("Deleting branch " + worktree.Branch + "...\n") | |
| 144 | + | } | |
| 145 | + | if err := deleteMergedBranch(ctx, plan.repoRoot, worktree.Branch, worktree.Head); err != nil { | |
| 146 | + | branchErrs = append(branchErrs, fmt.Sprintf("delete branch %q: %v", worktree.Branch, err)) | |
| 147 | + | continue | |
| 148 | + | } | |
| 149 | + | deleted = append(deleted, worktree.Branch) | |
| 105 | 150 | } | |
| 106 | 151 | _, _ = manageWorktreesGit(ctx, plan.repoRoot, "worktree", "prune") | |
| 107 | - | return manageWorktreesResult(plan, "removed"), nil | |
| 152 | + | if len(branchErrs) > 0 { | |
| 153 | + | return core.ToolResult{}, fmt.Errorf("manage_worktrees: worktrees were removed but some branches were kept%s", summary()) | |
| 154 | + | } | |
| 155 | + | result := manageWorktreesResult(plan, "removed") | |
| 156 | + | if len(deleted) > 0 { | |
| 157 | + | text := plan.text + "deleted branches:\n" | |
| 158 | + | for _, branch := range deleted { | |
| 159 | + | text += "- " + branch + "\n" | |
| 160 | + | } | |
| 161 | + | result.Content = []provider.Content{provider.TextBlock{Text: text}} | |
| 162 | + | } | |
| 163 | + | if details, ok := result.Details.(map[string]any); ok { | |
| 164 | + | details["removed"] = removed | |
| 165 | + | details["deleted_branches"] = deleted | |
| 166 | + | } | |
| 167 | + | return result, nil | |
| 168 | + | } | |
| 169 | + | ||
| 170 | + | // deleteMergedBranch deletes refs/heads/<branch> only while it still points at | |
| 171 | + | // head, is a plain (non-symbolic) ref, and is not checked out in any worktree. | |
| 172 | + | // The checks run immediately before deletion because state can change after | |
| 173 | + | // planning and after the worktree was removed. | |
| 174 | + | func deleteMergedBranch(ctx context.Context, repoRoot, branch, head string) error { | |
| 175 | + | if branch == "" || head == "" { | |
| 176 | + | return errors.New("branch or inspected commit is unknown") | |
| 177 | + | } | |
| 178 | + | ref := "refs/heads/" + branch | |
| 179 | + | // A symbolic branch ref would make update-ref delete its target. | |
| 180 | + | if _, err := manageWorktreesGit(ctx, repoRoot, "symbolic-ref", "-q", ref); err == nil { | |
| 181 | + | return errors.New("branch ref is symbolic and was kept") | |
| 182 | + | } else if createWorktreeGitExitCode(err) != 1 { | |
| 183 | + | return fmt.Errorf("inspect branch ref: %w", err) | |
| 184 | + | } | |
| 185 | + | where, err := branchCheckedOutAt(ctx, repoRoot, ref) | |
| 186 | + | if err != nil { | |
| 187 | + | return err | |
| 188 | + | } | |
| 189 | + | if where != "" { | |
| 190 | + | return fmt.Errorf("branch is checked out in %q", filepath.ToSlash(where)) | |
| 191 | + | } | |
| 192 | + | if _, err := manageWorktreesGit(ctx, repoRoot, "update-ref", "--no-deref", "-d", ref, head); err != nil { | |
| 193 | + | return fmt.Errorf("branch changed since inspection or cannot be deleted: %w", err) | |
| 194 | + | } | |
| 195 | + | return nil | |
| 196 | + | } | |
| 197 | + | ||
| 198 | + | // branchCheckedOutAt returns the path of a worktree (primary included) that has | |
| 199 | + | // ref checked out, or "" when none does. | |
| 200 | + | func branchCheckedOutAt(ctx context.Context, repoRoot, ref string) (string, error) { | |
| 201 | + | output, err := manageWorktreesGit(ctx, repoRoot, "worktree", "list", "--porcelain") | |
| 202 | + | if err != nil { | |
| 203 | + | return "", fmt.Errorf("list worktrees: %w", err) | |
| 204 | + | } | |
| 205 | + | for _, block := range strings.Split(strings.TrimSpace(output), "\n\n") { | |
| 206 | + | path, checkedOut := "", false | |
| 207 | + | for _, line := range strings.Split(block, "\n") { | |
| 208 | + | if strings.HasPrefix(line, "worktree ") { | |
| 209 | + | path = strings.TrimPrefix(line, "worktree ") | |
| 210 | + | } else if line == "branch "+ref { | |
| 211 | + | checkedOut = true | |
| 212 | + | } | |
| 213 | + | } | |
| 214 | + | if checkedOut { | |
| 215 | + | return path, nil | |
| 216 | + | } | |
| 217 | + | } | |
| 218 | + | return "", nil | |
| 108 | 219 | } | |
| 109 | 220 | ||
| 110 | 221 | func (t *ManageWorktreesTool) plan(ctx context.Context, raw json.RawMessage) (manageWorktreesPlan, error) { | |
| @@ -143,11 +254,30 @@ func (t *ManageWorktreesTool) plan(ctx context.Context, raw json.RawMessage) (ma | |||
| 143 | 254 | if err := t.Sandbox.CheckReadPath(repoRoot); err != nil { | |
| 144 | 255 | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: %w", err) | |
| 145 | 256 | } | |
| 146 | - | items, err := inspectManagedWorktrees(ctx, repoRoot) | |
| 257 | + | if args.Action == "list" && args.DeleteBranch { | |
| 258 | + | return manageWorktreesPlan{}, errors.New("manage_worktrees: delete_branch applies only to cleanup") | |
| 259 | + | } | |
| 260 | + | into := strings.TrimSpace(args.Into) | |
| 261 | + | if strings.HasPrefix(into, "-") { | |
| 262 | + | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: invalid into %q", into) | |
| 263 | + | } | |
| 264 | + | target := into | |
| 265 | + | if target == "" { | |
| 266 | + | target = "HEAD" | |
| 267 | + | } | |
| 268 | + | intoCommit, err := manageWorktreesGit(ctx, repoRoot, "rev-parse", "--verify", target+"^{commit}") | |
| 269 | + | if err != nil { | |
| 270 | + | if into == "" { | |
| 271 | + | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: resolve current commit: %w", err) | |
| 272 | + | } | |
| 273 | + | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: invalid into %q: %w", into, err) | |
| 274 | + | } | |
| 275 | + | intoRef, _ := manageWorktreesGit(ctx, repoRoot, "rev-parse", "--symbolic-full-name", target) | |
| 276 | + | items, err := inspectManagedWorktrees(ctx, repoRoot, intoCommit, into) | |
| 147 | 277 | if err != nil { | |
| 148 | 278 | return manageWorktreesPlan{}, err | |
| 149 | 279 | } | |
| 150 | - | plan := manageWorktreesPlan{repoRoot: repoRoot, action: args.Action, all: items} | |
| 280 | + | plan := manageWorktreesPlan{repoRoot: repoRoot, action: args.Action, into: into, intoCommit: intoCommit, intoRef: intoRef, deleteBranch: args.DeleteBranch, all: items} | |
| 151 | 281 | if args.Action == "cleanup" { | |
| 152 | 282 | if len(args.Paths) == 0 { | |
| 153 | 283 | return manageWorktreesPlan{}, errors.New("manage_worktrees: cleanup requires at least one explicit path") | |
| @@ -184,6 +314,36 @@ func (t *ManageWorktreesTool) plan(ctx context.Context, raw json.RawMessage) (ma | |||
| 184 | 314 | seen[item.Path] = true | |
| 185 | 315 | } | |
| 186 | 316 | } | |
| 317 | + | if args.DeleteBranch { | |
| 318 | + | // Deleting a branch writes to Git ref storage, which lives outside the | |
| 319 | + | // worktree path. Require write access to it and fail closed. Jail mode | |
| 320 | + | // is a guardrail, not a sandbox: it cannot stop git from touching refs | |
| 321 | + | // once permitted, so this only enforces the declared scopes. | |
| 322 | + | commonDir, err := manageWorktreesGit(ctx, repoRoot, "rev-parse", "--git-common-dir") | |
| 323 | + | if err != nil { | |
| 324 | + | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: resolve git directory: %w", err) | |
| 325 | + | } | |
| 326 | + | if !filepath.IsAbs(commonDir) { | |
| 327 | + | commonDir = filepath.Join(repoRoot, commonDir) | |
| 328 | + | } | |
| 329 | + | commonDir, err = canonical(commonDir) | |
| 330 | + | if err != nil { | |
| 331 | + | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: resolve git directory: %w", err) | |
| 332 | + | } | |
| 333 | + | if err := t.Sandbox.CheckWritePath(commonDir); err != nil { | |
| 334 | + | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: %w", err) | |
| 335 | + | } | |
| 336 | + | for _, item := range plan.selected { | |
| 337 | + | if intoRef == "refs/heads/"+item.Branch { | |
| 338 | + | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: branch %q is the merge target and cannot be deleted", item.Branch) | |
| 339 | + | } | |
| 340 | + | for _, other := range items { | |
| 341 | + | if other.Branch == item.Branch && !seen[other.Path] { | |
| 342 | + | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: branch %q is checked out in %q and cannot be deleted", item.Branch, filepath.ToSlash(other.Path)) | |
| 343 | + | } | |
| 344 | + | } | |
| 345 | + | } | |
| 346 | + | } | |
| 187 | 347 | } | |
| 188 | 348 | plan.text = renderManagedWorktrees(plan) | |
| 189 | 349 | return plan, nil | |
| @@ -238,7 +398,10 @@ func WorktreeInventoryContext(ctx context.Context, cwd string) string { | |||
| 238 | 398 | return b.String() | |
| 239 | 399 | } | |
| 240 | 400 | ||
| 241 | - | func inspectManagedWorktrees(ctx context.Context, repoRoot string) ([]managedWorktree, error) { | |
| 401 | + | // inspectManagedWorktrees reports each worktree's state. A branch is merged | |
| 402 | + | // when its head is an ancestor of intoCommit; into names that target for | |
| 403 | + | // messages and is empty for the current HEAD. | |
| 404 | + | func inspectManagedWorktrees(ctx context.Context, repoRoot, intoCommit, into string) ([]managedWorktree, error) { | |
| 242 | 405 | output, err := manageWorktreesGit(ctx, repoRoot, "worktree", "list", "--porcelain") | |
| 243 | 406 | if err != nil { | |
| 244 | 407 | return nil, fmt.Errorf("manage_worktrees: list: %w", err) | |
| @@ -279,8 +442,15 @@ func inspectManagedWorktrees(ctx context.Context, repoRoot string) ([]managedWor | |||
| 279 | 442 | } else if status != "" { | |
| 280 | 443 | item.Dirty = true | |
| 281 | 444 | item.BlockedBy = "uncommitted changes" | |
| 282 | - | } else if _, mergeErr := manageWorktreesGit(ctx, repoRoot, "merge-base", "--is-ancestor", item.Head, "HEAD"); mergeErr != nil { | |
| 283 | - | item.BlockedBy = "branch is not merged into the current branch: " + firstErrorLine(mergeErr) | |
| 445 | + | } else if _, mergeErr := manageWorktreesGit(ctx, repoRoot, "merge-base", "--is-ancestor", item.Head, intoCommit); mergeErr != nil { | |
| 446 | + | target := "the current branch" | |
| 447 | + | if into != "" { | |
| 448 | + | target = fmt.Sprintf("%q", into) | |
| 449 | + | } | |
| 450 | + | item.BlockedBy = "branch is not merged into " + target | |
| 451 | + | if createWorktreeGitExitCode(mergeErr) != 1 { | |
| 452 | + | item.BlockedBy += ": " + firstErrorLine(mergeErr) | |
| 453 | + | } | |
| 284 | 454 | } else { | |
| 285 | 455 | item.Merged = true | |
| 286 | 456 | item.CleanupOK = true | |
| @@ -306,6 +476,9 @@ func renderManagedWorktrees(plan manageWorktreesPlan) string { | |||
| 306 | 476 | state := item.BlockedBy | |
| 307 | 477 | if item.CleanupOK { | |
| 308 | 478 | state = "clean and merged; cleanup eligible" | |
| 479 | + | if plan.into != "" { | |
| 480 | + | state = fmt.Sprintf("clean and merged into %q; cleanup eligible", plan.into) | |
| 481 | + | } | |
| 309 | 482 | } | |
| 310 | 483 | if state != "" { | |
| 311 | 484 | fmt.Fprintf(&b, ": %s", state) | |
| @@ -317,6 +490,12 @@ func renderManagedWorktrees(plan manageWorktreesPlan) string { | |||
| 317 | 490 | for _, item := range plan.selected { | |
| 318 | 491 | fmt.Fprintf(&b, "- %s\n", filepath.ToSlash(item.Path)) | |
| 319 | 492 | } | |
| 493 | + | if plan.deleteBranch { | |
| 494 | + | b.WriteString("delete branches:\n") | |
| 495 | + | for _, item := range plan.selected { | |
| 496 | + | fmt.Fprintf(&b, "- %s\n", item.Branch) | |
| 497 | + | } | |
| 498 | + | } | |
| 320 | 499 | } | |
| 321 | 500 | return b.String() | |
| 322 | 501 | } | |
| @@ -329,7 +508,7 @@ func manageWorktreesResult(plan manageWorktreesPlan, state string) core.ToolResu | |||
| 329 | 508 | } | |
| 330 | 509 | ||
| 331 | 510 | func sameManageWorktreesPlan(a, b manageWorktreesPlan) bool { | |
| 332 | - | if a.repoRoot != b.repoRoot || a.action != b.action || a.text != b.text || len(a.selected) != len(b.selected) || len(a.all) != len(b.all) { | |
| 511 | + | if a.repoRoot != b.repoRoot || a.action != b.action || a.into != b.into || a.intoCommit != b.intoCommit || a.intoRef != b.intoRef || a.deleteBranch != b.deleteBranch || a.text != b.text || len(a.selected) != len(b.selected) || len(a.all) != len(b.all) { | |
| 333 | 512 | return false | |
| 334 | 513 | } | |
| 335 | 514 | for index := range a.selected { | |
| @@ -25,8 +25,10 @@ type ManageWorktreesTool struct { | |||||
| 25 | } | 25 | } | ||
| 26 | 26 | ||||
| 27 | type manageWorktreesArgs struct { | 27 | type manageWorktreesArgs struct { | ||
| 28 | - | Action string `json:"action"` | 28 | + | Action string `json:"action"` |
| 29 | - | Paths []string `json:"paths,omitempty"` | 29 | + | Paths []string `json:"paths,omitempty"` |
| 30 | + | Into string `json:"into,omitempty"` | |||
| 31 | + | DeleteBranch bool `json:"delete_branch,omitempty"` | |||
| 30 | } | 32 | } | ||
| 31 | 33 | ||||
| 32 | type managedWorktree struct { | 34 | type managedWorktree struct { | ||
| @@ -44,9 +46,17 @@ type managedWorktree struct { | |||||
| 44 | type manageWorktreesPlan struct { | 46 | type manageWorktreesPlan struct { | ||
| 45 | repoRoot string | 47 | repoRoot string | ||
| 46 | action string | 48 | action string | ||
| 47 | - | selected []managedWorktree | 49 | + | // into is the requested merge target; empty means the current HEAD. |
| 48 | - | all []managedWorktree | 50 | + | into string |
| 49 | - | text string | 51 | + | deleteBranch bool |
| 52 | + | // intoCommit and intoRef pin the resolved merge target so a target that | |||
| 53 | + | // moves between preview and execute is detected. intoRef is the symbolic | |||
| 54 | + | // full name (empty for a commit or detached HEAD). | |||
| 55 | + | intoCommit string | |||
| 56 | + | intoRef string | |||
| 57 | + | selected []managedWorktree | |||
| 58 | + | all []managedWorktree | |||
| 59 | + | text string | |||
| 50 | } | 60 | } | ||
| 51 | 61 | ||||
| 52 | type manageWorktreesPreview struct { | 62 | type manageWorktreesPreview struct { | ||
| @@ -58,7 +68,9 @@ const manageWorktreesSchema = `{ | |||||
| 58 | "type":"object", | 68 | "type":"object", | ||
| 59 | "properties":{ | 69 | "properties":{ | ||
| 60 | "action":{"type":"string","enum":["list","cleanup"],"description":"List repository worktrees, or remove explicitly selected worktrees that are clean and merged."}, | 70 | "action":{"type":"string","enum":["list","cleanup"],"description":"List repository worktrees, or remove explicitly selected worktrees that are clean and merged."}, | ||
| 61 | - | "paths":{"type":"array","items":{"type":"string"},"description":"Exact worktree paths to remove. Required for cleanup."} | 71 | + | "paths":{"type":"array","items":{"type":"string"},"description":"Exact worktree paths to remove. Required for cleanup."}, |
| 72 | + | "into":{"type":"string","description":"Branch, tag, or commit the worktree branches must be merged into. Defaults to the current HEAD."}, | |||
| 73 | + | "delete_branch":{"type":"boolean","description":"Also delete each removed worktree's local branch if it is still at the inspected commit."} | |||
| 62 | }, | 74 | }, | ||
| 63 | "required":["action"] | 75 | "required":["action"] | ||
| 64 | }` | 76 | }` | ||
| @@ -95,16 +107,115 @@ func (t *ManageWorktreesTool) Execute(ctx context.Context, raw json.RawMessage, | |||||
| 95 | if plan.action == "list" { | 107 | if plan.action == "list" { | ||
| 96 | return manageWorktreesResult(plan, "listed"), nil | 108 | return manageWorktreesResult(plan, "listed"), nil | ||
| 97 | } | 109 | } | ||
| 110 | + | var removed, deleted []string | |||
| 111 | + | var branchErrs []string | |||
| 112 | + | // summary reports progress made so far; it is attached to every failure | |||
| 113 | + | // return so a partial cleanup is never silently dropped. | |||
| 114 | + | summary := func() string { | |||
| 115 | + | var parts []string | |||
| 116 | + | if len(branchErrs) > 0 { | |||
| 117 | + | parts = append(parts, strings.Join(branchErrs, "; ")) | |||
| 118 | + | } | |||
| 119 | + | if len(removed) > 0 { | |||
| 120 | + | parts = append(parts, "removed worktrees: "+strings.Join(removed, ", ")) | |||
| 121 | + | } | |||
| 122 | + | if len(deleted) > 0 { | |||
| 123 | + | parts = append(parts, "deleted branches: "+strings.Join(deleted, ", ")) | |||
| 124 | + | } | |||
| 125 | + | if len(parts) == 0 { | |||
| 126 | + | return "" | |||
| 127 | + | } | |||
| 128 | + | return "; " + strings.Join(parts, "; ") | |||
| 129 | + | } | |||
| 98 | for _, worktree := range plan.selected { | 130 | for _, worktree := range plan.selected { | ||
| 99 | if progress != nil { | 131 | if progress != nil { | ||
| 100 | progress("Removing worktree " + filepath.ToSlash(worktree.Path) + "...\n") | 132 | progress("Removing worktree " + filepath.ToSlash(worktree.Path) + "...\n") | ||
| 101 | } | 133 | } | ||
| 102 | if _, err := manageWorktreesGit(ctx, plan.repoRoot, "worktree", "remove", worktree.Path); err != nil { | 134 | if _, err := manageWorktreesGit(ctx, plan.repoRoot, "worktree", "remove", worktree.Path); err != nil { | ||
| 103 | - | return core.ToolResult{}, fmt.Errorf("manage_worktrees: remove %q: %w", filepath.ToSlash(worktree.Path), err) | 135 | + | _, _ = manageWorktreesGit(ctx, plan.repoRoot, "worktree", "prune") |
| 136 | + | return core.ToolResult{}, fmt.Errorf("manage_worktrees: remove %q: %w%s", filepath.ToSlash(worktree.Path), err, summary()) | |||
| 137 | + | } | |||
| 138 | + | removed = append(removed, filepath.ToSlash(worktree.Path)) | |||
| 139 | + | if !plan.deleteBranch { | |||
| 140 | + | continue | |||
| 104 | } | 141 | } | ||
| 142 | + | if progress != nil { | |||
| 143 | + | progress("Deleting branch " + worktree.Branch + "...\n") | |||
| 144 | + | } | |||
| 145 | + | if err := deleteMergedBranch(ctx, plan.repoRoot, worktree.Branch, worktree.Head); err != nil { | |||
| 146 | + | branchErrs = append(branchErrs, fmt.Sprintf("delete branch %q: %v", worktree.Branch, err)) | |||
| 147 | + | continue | |||
| 148 | + | } | |||
| 149 | + | deleted = append(deleted, worktree.Branch) | |||
| 105 | } | 150 | } | ||
| 106 | _, _ = manageWorktreesGit(ctx, plan.repoRoot, "worktree", "prune") | 151 | _, _ = manageWorktreesGit(ctx, plan.repoRoot, "worktree", "prune") | ||
| 107 | - | return manageWorktreesResult(plan, "removed"), nil | 152 | + | if len(branchErrs) > 0 { |
| 153 | + | return core.ToolResult{}, fmt.Errorf("manage_worktrees: worktrees were removed but some branches were kept%s", summary()) | |||
| 154 | + | } | |||
| 155 | + | result := manageWorktreesResult(plan, "removed") | |||
| 156 | + | if len(deleted) > 0 { | |||
| 157 | + | text := plan.text + "deleted branches:\n" | |||
| 158 | + | for _, branch := range deleted { | |||
| 159 | + | text += "- " + branch + "\n" | |||
| 160 | + | } | |||
| 161 | + | result.Content = []provider.Content{provider.TextBlock{Text: text}} | |||
| 162 | + | } | |||
| 163 | + | if details, ok := result.Details.(map[string]any); ok { | |||
| 164 | + | details["removed"] = removed | |||
| 165 | + | details["deleted_branches"] = deleted | |||
| 166 | + | } | |||
| 167 | + | return result, nil | |||
| 168 | + | } | |||
| 169 | + | ||||
| 170 | + | // deleteMergedBranch deletes refs/heads/<branch> only while it still points at | |||
| 171 | + | // head, is a plain (non-symbolic) ref, and is not checked out in any worktree. | |||
| 172 | + | // The checks run immediately before deletion because state can change after | |||
| 173 | + | // planning and after the worktree was removed. | |||
| 174 | + | func deleteMergedBranch(ctx context.Context, repoRoot, branch, head string) error { | |||
| 175 | + | if branch == "" || head == "" { | |||
| 176 | + | return errors.New("branch or inspected commit is unknown") | |||
| 177 | + | } | |||
| 178 | + | ref := "refs/heads/" + branch | |||
| 179 | + | // A symbolic branch ref would make update-ref delete its target. | |||
| 180 | + | if _, err := manageWorktreesGit(ctx, repoRoot, "symbolic-ref", "-q", ref); err == nil { | |||
| 181 | + | return errors.New("branch ref is symbolic and was kept") | |||
| 182 | + | } else if createWorktreeGitExitCode(err) != 1 { | |||
| 183 | + | return fmt.Errorf("inspect branch ref: %w", err) | |||
| 184 | + | } | |||
| 185 | + | where, err := branchCheckedOutAt(ctx, repoRoot, ref) | |||
| 186 | + | if err != nil { | |||
| 187 | + | return err | |||
| 188 | + | } | |||
| 189 | + | if where != "" { | |||
| 190 | + | return fmt.Errorf("branch is checked out in %q", filepath.ToSlash(where)) | |||
| 191 | + | } | |||
| 192 | + | if _, err := manageWorktreesGit(ctx, repoRoot, "update-ref", "--no-deref", "-d", ref, head); err != nil { | |||
| 193 | + | return fmt.Errorf("branch changed since inspection or cannot be deleted: %w", err) | |||
| 194 | + | } | |||
| 195 | + | return nil | |||
| 196 | + | } | |||
| 197 | + | ||||
| 198 | + | // branchCheckedOutAt returns the path of a worktree (primary included) that has | |||
| 199 | + | // ref checked out, or "" when none does. | |||
| 200 | + | func branchCheckedOutAt(ctx context.Context, repoRoot, ref string) (string, error) { | |||
| 201 | + | output, err := manageWorktreesGit(ctx, repoRoot, "worktree", "list", "--porcelain") | |||
| 202 | + | if err != nil { | |||
| 203 | + | return "", fmt.Errorf("list worktrees: %w", err) | |||
| 204 | + | } | |||
| 205 | + | for _, block := range strings.Split(strings.TrimSpace(output), "\n\n") { | |||
| 206 | + | path, checkedOut := "", false | |||
| 207 | + | for _, line := range strings.Split(block, "\n") { | |||
| 208 | + | if strings.HasPrefix(line, "worktree ") { | |||
| 209 | + | path = strings.TrimPrefix(line, "worktree ") | |||
| 210 | + | } else if line == "branch "+ref { | |||
| 211 | + | checkedOut = true | |||
| 212 | + | } | |||
| 213 | + | } | |||
| 214 | + | if checkedOut { | |||
| 215 | + | return path, nil | |||
| 216 | + | } | |||
| 217 | + | } | |||
| 218 | + | return "", nil | |||
| 108 | } | 219 | } | ||
| 109 | 220 | ||||
| 110 | func (t *ManageWorktreesTool) plan(ctx context.Context, raw json.RawMessage) (manageWorktreesPlan, error) { | 221 | func (t *ManageWorktreesTool) plan(ctx context.Context, raw json.RawMessage) (manageWorktreesPlan, error) { | ||
| @@ -143,11 +254,30 @@ func (t *ManageWorktreesTool) plan(ctx context.Context, raw json.RawMessage) (ma | |||||
| 143 | if err := t.Sandbox.CheckReadPath(repoRoot); err != nil { | 254 | if err := t.Sandbox.CheckReadPath(repoRoot); err != nil { | ||
| 144 | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: %w", err) | 255 | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: %w", err) | ||
| 145 | } | 256 | } | ||
| 146 | - | items, err := inspectManagedWorktrees(ctx, repoRoot) | 257 | + | if args.Action == "list" && args.DeleteBranch { |
| 258 | + | return manageWorktreesPlan{}, errors.New("manage_worktrees: delete_branch applies only to cleanup") | |||
| 259 | + | } | |||
| 260 | + | into := strings.TrimSpace(args.Into) | |||
| 261 | + | if strings.HasPrefix(into, "-") { | |||
| 262 | + | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: invalid into %q", into) | |||
| 263 | + | } | |||
| 264 | + | target := into | |||
| 265 | + | if target == "" { | |||
| 266 | + | target = "HEAD" | |||
| 267 | + | } | |||
| 268 | + | intoCommit, err := manageWorktreesGit(ctx, repoRoot, "rev-parse", "--verify", target+"^{commit}") | |||
| 269 | + | if err != nil { | |||
| 270 | + | if into == "" { | |||
| 271 | + | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: resolve current commit: %w", err) | |||
| 272 | + | } | |||
| 273 | + | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: invalid into %q: %w", into, err) | |||
| 274 | + | } | |||
| 275 | + | intoRef, _ := manageWorktreesGit(ctx, repoRoot, "rev-parse", "--symbolic-full-name", target) | |||
| 276 | + | items, err := inspectManagedWorktrees(ctx, repoRoot, intoCommit, into) | |||
| 147 | if err != nil { | 277 | if err != nil { | ||
| 148 | return manageWorktreesPlan{}, err | 278 | return manageWorktreesPlan{}, err | ||
| 149 | } | 279 | } | ||
| 150 | - | plan := manageWorktreesPlan{repoRoot: repoRoot, action: args.Action, all: items} | 280 | + | plan := manageWorktreesPlan{repoRoot: repoRoot, action: args.Action, into: into, intoCommit: intoCommit, intoRef: intoRef, deleteBranch: args.DeleteBranch, all: items} |
| 151 | if args.Action == "cleanup" { | 281 | if args.Action == "cleanup" { | ||
| 152 | if len(args.Paths) == 0 { | 282 | if len(args.Paths) == 0 { | ||
| 153 | return manageWorktreesPlan{}, errors.New("manage_worktrees: cleanup requires at least one explicit path") | 283 | return manageWorktreesPlan{}, errors.New("manage_worktrees: cleanup requires at least one explicit path") | ||
| @@ -184,6 +314,36 @@ func (t *ManageWorktreesTool) plan(ctx context.Context, raw json.RawMessage) (ma | |||||
| 184 | seen[item.Path] = true | 314 | seen[item.Path] = true | ||
| 185 | } | 315 | } | ||
| 186 | } | 316 | } | ||
| 317 | + | if args.DeleteBranch { | |||
| 318 | + | // Deleting a branch writes to Git ref storage, which lives outside the | |||
| 319 | + | // worktree path. Require write access to it and fail closed. Jail mode | |||
| 320 | + | // is a guardrail, not a sandbox: it cannot stop git from touching refs | |||
| 321 | + | // once permitted, so this only enforces the declared scopes. | |||
| 322 | + | commonDir, err := manageWorktreesGit(ctx, repoRoot, "rev-parse", "--git-common-dir") | |||
| 323 | + | if err != nil { | |||
| 324 | + | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: resolve git directory: %w", err) | |||
| 325 | + | } | |||
| 326 | + | if !filepath.IsAbs(commonDir) { | |||
| 327 | + | commonDir = filepath.Join(repoRoot, commonDir) | |||
| 328 | + | } | |||
| 329 | + | commonDir, err = canonical(commonDir) | |||
| 330 | + | if err != nil { | |||
| 331 | + | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: resolve git directory: %w", err) | |||
| 332 | + | } | |||
| 333 | + | if err := t.Sandbox.CheckWritePath(commonDir); err != nil { | |||
| 334 | + | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: %w", err) | |||
| 335 | + | } | |||
| 336 | + | for _, item := range plan.selected { | |||
| 337 | + | if intoRef == "refs/heads/"+item.Branch { | |||
| 338 | + | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: branch %q is the merge target and cannot be deleted", item.Branch) | |||
| 339 | + | } | |||
| 340 | + | for _, other := range items { | |||
| 341 | + | if other.Branch == item.Branch && !seen[other.Path] { | |||
| 342 | + | return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: branch %q is checked out in %q and cannot be deleted", item.Branch, filepath.ToSlash(other.Path)) | |||
| 343 | + | } | |||
| 344 | + | } | |||
| 345 | + | } | |||
| 346 | + | } | |||
| 187 | } | 347 | } | ||
| 188 | plan.text = renderManagedWorktrees(plan) | 348 | plan.text = renderManagedWorktrees(plan) | ||
| 189 | return plan, nil | 349 | return plan, nil | ||
| @@ -238,7 +398,10 @@ func WorktreeInventoryContext(ctx context.Context, cwd string) string { | |||||
| 238 | return b.String() | 398 | return b.String() | ||
| 239 | } | 399 | } | ||
| 240 | 400 | ||||
| 241 | - | func inspectManagedWorktrees(ctx context.Context, repoRoot string) ([]managedWorktree, error) { | 401 | + | // inspectManagedWorktrees reports each worktree's state. A branch is merged |
| 402 | + | // when its head is an ancestor of intoCommit; into names that target for | |||
| 403 | + | // messages and is empty for the current HEAD. | |||
| 404 | + | func inspectManagedWorktrees(ctx context.Context, repoRoot, intoCommit, into string) ([]managedWorktree, error) { | |||
| 242 | output, err := manageWorktreesGit(ctx, repoRoot, "worktree", "list", "--porcelain") | 405 | output, err := manageWorktreesGit(ctx, repoRoot, "worktree", "list", "--porcelain") | ||
| 243 | if err != nil { | 406 | if err != nil { | ||
| 244 | return nil, fmt.Errorf("manage_worktrees: list: %w", err) | 407 | return nil, fmt.Errorf("manage_worktrees: list: %w", err) | ||
| @@ -279,8 +442,15 @@ func inspectManagedWorktrees(ctx context.Context, repoRoot string) ([]managedWor | |||||
| 279 | } else if status != "" { | 442 | } else if status != "" { | ||
| 280 | item.Dirty = true | 443 | item.Dirty = true | ||
| 281 | item.BlockedBy = "uncommitted changes" | 444 | item.BlockedBy = "uncommitted changes" | ||
| 282 | - | } else if _, mergeErr := manageWorktreesGit(ctx, repoRoot, "merge-base", "--is-ancestor", item.Head, "HEAD"); mergeErr != nil { | 445 | + | } else if _, mergeErr := manageWorktreesGit(ctx, repoRoot, "merge-base", "--is-ancestor", item.Head, intoCommit); mergeErr != nil { |
| 283 | - | item.BlockedBy = "branch is not merged into the current branch: " + firstErrorLine(mergeErr) | 446 | + | target := "the current branch" |
| 447 | + | if into != "" { | |||
| 448 | + | target = fmt.Sprintf("%q", into) | |||
| 449 | + | } | |||
| 450 | + | item.BlockedBy = "branch is not merged into " + target | |||
| 451 | + | if createWorktreeGitExitCode(mergeErr) != 1 { | |||
| 452 | + | item.BlockedBy += ": " + firstErrorLine(mergeErr) | |||
| 453 | + | } | |||
| 284 | } else { | 454 | } else { | ||
| 285 | item.Merged = true | 455 | item.Merged = true | ||
| 286 | item.CleanupOK = true | 456 | item.CleanupOK = true | ||
| @@ -306,6 +476,9 @@ func renderManagedWorktrees(plan manageWorktreesPlan) string { | |||||
| 306 | state := item.BlockedBy | 476 | state := item.BlockedBy | ||
| 307 | if item.CleanupOK { | 477 | if item.CleanupOK { | ||
| 308 | state = "clean and merged; cleanup eligible" | 478 | state = "clean and merged; cleanup eligible" | ||
| 479 | + | if plan.into != "" { | |||
| 480 | + | state = fmt.Sprintf("clean and merged into %q; cleanup eligible", plan.into) | |||
| 481 | + | } | |||
| 309 | } | 482 | } | ||
| 310 | if state != "" { | 483 | if state != "" { | ||
| 311 | fmt.Fprintf(&b, ": %s", state) | 484 | fmt.Fprintf(&b, ": %s", state) | ||
| @@ -317,6 +490,12 @@ func renderManagedWorktrees(plan manageWorktreesPlan) string { | |||||
| 317 | for _, item := range plan.selected { | 490 | for _, item := range plan.selected { | ||
| 318 | fmt.Fprintf(&b, "- %s\n", filepath.ToSlash(item.Path)) | 491 | fmt.Fprintf(&b, "- %s\n", filepath.ToSlash(item.Path)) | ||
| 319 | } | 492 | } | ||
| 493 | + | if plan.deleteBranch { | |||
| 494 | + | b.WriteString("delete branches:\n") | |||
| 495 | + | for _, item := range plan.selected { | |||
| 496 | + | fmt.Fprintf(&b, "- %s\n", item.Branch) | |||
| 497 | + | } | |||
| 498 | + | } | |||
| 320 | } | 499 | } | ||
| 321 | return b.String() | 500 | return b.String() | ||
| 322 | } | 501 | } | ||
| @@ -329,7 +508,7 @@ func manageWorktreesResult(plan manageWorktreesPlan, state string) core.ToolResu | |||||
| 329 | } | 508 | } | ||
| 330 | 509 | ||||
| 331 | func sameManageWorktreesPlan(a, b manageWorktreesPlan) bool { | 510 | func sameManageWorktreesPlan(a, b manageWorktreesPlan) bool { | ||
| 332 | - | if a.repoRoot != b.repoRoot || a.action != b.action || a.text != b.text || len(a.selected) != len(b.selected) || len(a.all) != len(b.all) { | 511 | + | if a.repoRoot != b.repoRoot || a.action != b.action || a.into != b.into || a.intoCommit != b.intoCommit || a.intoRef != b.intoRef || a.deleteBranch != b.deleteBranch || a.text != b.text || len(a.selected) != len(b.selected) || len(a.all) != len(b.all) { |
| 333 | return false | 512 | return false | ||
| 334 | } | 513 | } | ||
| 335 | for index := range a.selected { | 514 | for index := range a.selected { | ||
M packages/agent/tools/manage_worktrees_test.go +202 -0
| @@ -140,3 +140,205 @@ func TestManageWorktreesRejectsCleanupWithoutExplicitPaths(t *testing.T) { | |||
| 140 | 140 | func newManageWorktreesTestTool(cwd string) *ManageWorktreesTool { | |
| 141 | 141 | return &ManageWorktreesTool{CWD: cwd, Sandbox: NewSandbox(cwd)} | |
| 142 | 142 | } | |
| 143 | + | ||
| 144 | + | // addMergedSideWorktree creates branch "side" at the primary HEAD in a new | |
| 145 | + | // worktree, so it is merged into HEAD and eligible for cleanup. | |
| 146 | + | func addMergedSideWorktree(t *testing.T, repo, branch string) string { | |
| 147 | + | t.Helper() | |
| 148 | + | path := filepath.Join(t.TempDir(), branch) | |
| 149 | + | gitTestOutput(t, repo, "worktree", "add", "-b", branch, path) | |
| 150 | + | return path | |
| 151 | + | } | |
| 152 | + | ||
| 153 | + | func branchExists(t *testing.T, repo, branch string) bool { | |
| 154 | + | t.Helper() | |
| 155 | + | return gitTestOutputAllowExit(t, repo, "show-ref", "--verify", "--quiet", "refs/heads/"+branch).exitCode == 0 | |
| 156 | + | } | |
| 157 | + | ||
| 158 | + | func TestDeleteMergedBranchRefusesSymbolicRef(t *testing.T) { | |
| 159 | + | if _, err := exec.LookPath("git"); err != nil { | |
| 160 | + | t.Skip("git is required") | |
| 161 | + | } | |
| 162 | + | repo := initWorktreeTestRepo(t) | |
| 163 | + | mainBranch := gitTestOutput(t, repo, "branch", "--show-current") | |
| 164 | + | head := gitTestOutput(t, repo, "rev-parse", "HEAD") | |
| 165 | + | gitTestOutput(t, repo, "branch", "side") | |
| 166 | + | gitTestOutput(t, repo, "symbolic-ref", "refs/heads/side", "refs/heads/"+mainBranch) | |
| 167 | + | ||
| 168 | + | err := deleteMergedBranch(context.Background(), repo, "side", head) | |
| 169 | + | if err == nil || !strings.Contains(err.Error(), "symbolic") { | |
| 170 | + | t.Fatalf("error = %v, want symbolic-ref rejection", err) | |
| 171 | + | } | |
| 172 | + | if !branchExists(t, repo, mainBranch) { | |
| 173 | + | t.Fatal("symbolic ref deletion removed the target branch") | |
| 174 | + | } | |
| 175 | + | } | |
| 176 | + | ||
| 177 | + | func TestManageWorktreesDeleteBranchKeepsTargetWhenBranchBecomesSymref(t *testing.T) { | |
| 178 | + | if _, err := exec.LookPath("git"); err != nil { | |
| 179 | + | t.Skip("git is required") | |
| 180 | + | } | |
| 181 | + | repo := initWorktreeTestRepo(t) | |
| 182 | + | mainBranch := gitTestOutput(t, repo, "branch", "--show-current") | |
| 183 | + | path := addMergedSideWorktree(t, repo, "side") | |
| 184 | + | tool := newManageWorktreesTestTool(repo) | |
| 185 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}, "delete_branch": true}) | |
| 186 | + | ||
| 187 | + | // The worktree is already removed when the delete progress is reported; | |
| 188 | + | // swap the branch for a symref to the primary branch at the same commit. | |
| 189 | + | progress := func(msg string) { | |
| 190 | + | if strings.HasPrefix(msg, "Deleting branch") { | |
| 191 | + | gitTestOutput(t, repo, "symbolic-ref", "refs/heads/side", "refs/heads/"+mainBranch) | |
| 192 | + | } | |
| 193 | + | } | |
| 194 | + | _, err := tool.Execute(context.Background(), args, progress) | |
| 195 | + | if err == nil || !strings.Contains(err.Error(), "symbolic") || !strings.Contains(err.Error(), "removed worktrees:") { | |
| 196 | + | t.Fatalf("error = %v, want symbolic-ref branch failure", err) | |
| 197 | + | } | |
| 198 | + | if !branchExists(t, repo, mainBranch) { | |
| 199 | + | t.Fatal("primary branch was deleted through a symbolic ref") | |
| 200 | + | } | |
| 201 | + | } | |
| 202 | + | ||
| 203 | + | func TestManageWorktreesDeleteBranchRefusesBranchCheckedOutAfterRemoval(t *testing.T) { | |
| 204 | + | if _, err := exec.LookPath("git"); err != nil { | |
| 205 | + | t.Skip("git is required") | |
| 206 | + | } | |
| 207 | + | for _, where := range []string{"primary", "worktree"} { | |
| 208 | + | t.Run(where, func(t *testing.T) { | |
| 209 | + | repo := initWorktreeTestRepo(t) | |
| 210 | + | path := addMergedSideWorktree(t, repo, "side") | |
| 211 | + | tool := newManageWorktreesTestTool(repo) | |
| 212 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}, "delete_branch": true}) | |
| 213 | + | progress := func(msg string) { | |
| 214 | + | if !strings.HasPrefix(msg, "Deleting branch") { | |
| 215 | + | return | |
| 216 | + | } | |
| 217 | + | if where == "primary" { | |
| 218 | + | gitTestOutput(t, repo, "checkout", "-q", "side") | |
| 219 | + | } else { | |
| 220 | + | gitTestOutput(t, repo, "worktree", "add", filepath.Join(t.TempDir(), "again"), "side") | |
| 221 | + | } | |
| 222 | + | } | |
| 223 | + | _, err := tool.Execute(context.Background(), args, progress) | |
| 224 | + | if err == nil || !strings.Contains(err.Error(), `delete branch "side"`) || !strings.Contains(err.Error(), "checked out") { | |
| 225 | + | t.Fatalf("error = %v, want checked-out branch failure", err) | |
| 226 | + | } | |
| 227 | + | if !branchExists(t, repo, "side") { | |
| 228 | + | t.Fatal("checked-out branch was deleted") | |
| 229 | + | } | |
| 230 | + | }) | |
| 231 | + | } | |
| 232 | + | } | |
| 233 | + | ||
| 234 | + | func TestManageWorktreesDeleteBranchRequiresRefStorageWriteAccess(t *testing.T) { | |
| 235 | + | if _, err := exec.LookPath("git"); err != nil { | |
| 236 | + | t.Skip("git is required") | |
| 237 | + | } | |
| 238 | + | repo := initWorktreeTestRepo(t) | |
| 239 | + | path := addMergedSideWorktree(t, repo, "side") | |
| 240 | + | scoped := func(write ...string) *ManageWorktreesTool { | |
| 241 | + | var p PermissionSet | |
| 242 | + | p.Bash.Mode = "ask" | |
| 243 | + | p.FS.Read = []string{repo, filepath.Dir(path)} | |
| 244 | + | p.FS.Write = write | |
| 245 | + | sandbox := NewSandbox(repo) | |
| 246 | + | sandbox.SetPermissions(&p) | |
| 247 | + | return &ManageWorktreesTool{CWD: repo, Sandbox: sandbox} | |
| 248 | + | } | |
| 249 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}, "delete_branch": true}) | |
| 250 | + | ||
| 251 | + | tool := scoped(path) | |
| 252 | + | if _, err := tool.Preview(context.Background(), args); err == nil || !strings.Contains(err.Error(), "outside declared scopes") { | |
| 253 | + | t.Fatalf("preview error = %v, want ref-storage write denial", err) | |
| 254 | + | } | |
| 255 | + | if _, err := tool.Execute(context.Background(), args, nil); err == nil || !strings.Contains(err.Error(), "outside declared scopes") { | |
| 256 | + | t.Fatalf("execute error = %v, want ref-storage write denial", err) | |
| 257 | + | } | |
| 258 | + | if _, err := os.Stat(path); err != nil { | |
| 259 | + | t.Fatalf("worktree was removed despite denial: %v", err) | |
| 260 | + | } | |
| 261 | + | if !branchExists(t, repo, "side") { | |
| 262 | + | t.Fatal("branch was deleted despite denial") | |
| 263 | + | } | |
| 264 | + | ||
| 265 | + | // Without delete_branch only the worktree path needs write access. | |
| 266 | + | keep := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}}) | |
| 267 | + | if _, err := tool.Preview(context.Background(), keep); err != nil { | |
| 268 | + | t.Fatalf("cleanup without delete_branch: %v", err) | |
| 269 | + | } | |
| 270 | + | // Granting the common git directory allows the full operation. | |
| 271 | + | tool = scoped(path, filepath.Join(repo, ".git")) | |
| 272 | + | if _, err := tool.Execute(context.Background(), args, nil); err != nil { | |
| 273 | + | t.Fatal(err) | |
| 274 | + | } | |
| 275 | + | if branchExists(t, repo, "side") { | |
| 276 | + | t.Fatal("branch remains after permitted cleanup") | |
| 277 | + | } | |
| 278 | + | } | |
| 279 | + | ||
| 280 | + | func TestManageWorktreesDetectsMovedMergeTargetAfterPreview(t *testing.T) { | |
| 281 | + | if _, err := exec.LookPath("git"); err != nil { | |
| 282 | + | t.Skip("git is required") | |
| 283 | + | } | |
| 284 | + | repo := initWorktreeTestRepo(t) | |
| 285 | + | path := addMergedSideWorktree(t, repo, "side") | |
| 286 | + | gitTestOutput(t, repo, "branch", "target") | |
| 287 | + | tool := newManageWorktreesTestTool(repo) | |
| 288 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}, "into": "target", "delete_branch": true}) | |
| 289 | + | if _, err := tool.Preview(context.Background(), args); err != nil { | |
| 290 | + | t.Fatal(err) | |
| 291 | + | } | |
| 292 | + | // Advance the unchecked-out target; side remains an ancestor, so the | |
| 293 | + | // worktree stays eligible and only the pinned target differs. | |
| 294 | + | tree := gitTestOutput(t, repo, "rev-parse", "target^{tree}") | |
| 295 | + | moved := gitTestOutput(t, repo, "commit-tree", tree, "-p", "target", "-m", "advance") | |
| 296 | + | gitTestOutput(t, repo, "update-ref", "refs/heads/target", moved) | |
| 297 | + | ||
| 298 | + | _, err := tool.Execute(context.Background(), args, nil) | |
| 299 | + | if err == nil || !strings.Contains(err.Error(), "changed since preview") { | |
| 300 | + | t.Fatalf("error = %v, want stale-state rejection", err) | |
| 301 | + | } | |
| 302 | + | if _, statErr := os.Stat(path); statErr != nil { | |
| 303 | + | t.Fatalf("worktree was removed: %v", statErr) | |
| 304 | + | } | |
| 305 | + | if !branchExists(t, repo, "side") { | |
| 306 | + | t.Fatal("branch was deleted") | |
| 307 | + | } | |
| 308 | + | } | |
| 309 | + | ||
| 310 | + | func TestManageWorktreesPartialFailureReportsBranchErrorsAndProgress(t *testing.T) { | |
| 311 | + | if _, err := exec.LookPath("git"); err != nil { | |
| 312 | + | t.Skip("git is required") | |
| 313 | + | } | |
| 314 | + | repo := initWorktreeTestRepo(t) | |
| 315 | + | first := addMergedSideWorktree(t, repo, "first") | |
| 316 | + | second := addMergedSideWorktree(t, repo, "second") | |
| 317 | + | // A stale lock makes deleting "first" fail after its worktree is removed. | |
| 318 | + | lock := filepath.Join(repo, ".git", "refs", "heads", "first.lock") | |
| 319 | + | if err := os.WriteFile(lock, nil, 0o644); err != nil { | |
| 320 | + | t.Fatal(err) | |
| 321 | + | } | |
| 322 | + | tool := newManageWorktreesTestTool(repo) | |
| 323 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{first, second}, "delete_branch": true}) | |
| 324 | + | // Dirty the second worktree so its removal fails. | |
| 325 | + | progress := func(msg string) { | |
| 326 | + | if strings.HasPrefix(msg, "Removing worktree") && strings.Contains(msg, filepath.ToSlash(second)) { | |
| 327 | + | if err := os.WriteFile(filepath.Join(second, "untracked.txt"), []byte("x"), 0o644); err != nil { | |
| 328 | + | t.Error(err) | |
| 329 | + | } | |
| 330 | + | } | |
| 331 | + | } | |
| 332 | + | _, err := tool.Execute(context.Background(), args, progress) | |
| 333 | + | if err == nil { | |
| 334 | + | t.Fatal("expected partial failure") | |
| 335 | + | } | |
| 336 | + | for _, want := range []string{"remove ", `delete branch "first"`, "removed worktrees: " + filepath.ToSlash(first)} { | |
| 337 | + | if !strings.Contains(err.Error(), want) { | |
| 338 | + | t.Fatalf("error missing %q:\n%v", want, err) | |
| 339 | + | } | |
| 340 | + | } | |
| 341 | + | if _, statErr := os.Stat(second); statErr != nil { | |
| 342 | + | t.Fatalf("second worktree should remain: %v", statErr) | |
| 343 | + | } | |
| 344 | + | } | |
| @@ -140,3 +140,205 @@ func TestManageWorktreesRejectsCleanupWithoutExplicitPaths(t *testing.T) { | |||||
| 140 | func newManageWorktreesTestTool(cwd string) *ManageWorktreesTool { | 140 | func newManageWorktreesTestTool(cwd string) *ManageWorktreesTool { | ||
| 141 | return &ManageWorktreesTool{CWD: cwd, Sandbox: NewSandbox(cwd)} | 141 | return &ManageWorktreesTool{CWD: cwd, Sandbox: NewSandbox(cwd)} | ||
| 142 | } | 142 | } | ||
| 143 | + | ||||
| 144 | + | // addMergedSideWorktree creates branch "side" at the primary HEAD in a new | |||
| 145 | + | // worktree, so it is merged into HEAD and eligible for cleanup. | |||
| 146 | + | func addMergedSideWorktree(t *testing.T, repo, branch string) string { | |||
| 147 | + | t.Helper() | |||
| 148 | + | path := filepath.Join(t.TempDir(), branch) | |||
| 149 | + | gitTestOutput(t, repo, "worktree", "add", "-b", branch, path) | |||
| 150 | + | return path | |||
| 151 | + | } | |||
| 152 | + | ||||
| 153 | + | func branchExists(t *testing.T, repo, branch string) bool { | |||
| 154 | + | t.Helper() | |||
| 155 | + | return gitTestOutputAllowExit(t, repo, "show-ref", "--verify", "--quiet", "refs/heads/"+branch).exitCode == 0 | |||
| 156 | + | } | |||
| 157 | + | ||||
| 158 | + | func TestDeleteMergedBranchRefusesSymbolicRef(t *testing.T) { | |||
| 159 | + | if _, err := exec.LookPath("git"); err != nil { | |||
| 160 | + | t.Skip("git is required") | |||
| 161 | + | } | |||
| 162 | + | repo := initWorktreeTestRepo(t) | |||
| 163 | + | mainBranch := gitTestOutput(t, repo, "branch", "--show-current") | |||
| 164 | + | head := gitTestOutput(t, repo, "rev-parse", "HEAD") | |||
| 165 | + | gitTestOutput(t, repo, "branch", "side") | |||
| 166 | + | gitTestOutput(t, repo, "symbolic-ref", "refs/heads/side", "refs/heads/"+mainBranch) | |||
| 167 | + | ||||
| 168 | + | err := deleteMergedBranch(context.Background(), repo, "side", head) | |||
| 169 | + | if err == nil || !strings.Contains(err.Error(), "symbolic") { | |||
| 170 | + | t.Fatalf("error = %v, want symbolic-ref rejection", err) | |||
| 171 | + | } | |||
| 172 | + | if !branchExists(t, repo, mainBranch) { | |||
| 173 | + | t.Fatal("symbolic ref deletion removed the target branch") | |||
| 174 | + | } | |||
| 175 | + | } | |||
| 176 | + | ||||
| 177 | + | func TestManageWorktreesDeleteBranchKeepsTargetWhenBranchBecomesSymref(t *testing.T) { | |||
| 178 | + | if _, err := exec.LookPath("git"); err != nil { | |||
| 179 | + | t.Skip("git is required") | |||
| 180 | + | } | |||
| 181 | + | repo := initWorktreeTestRepo(t) | |||
| 182 | + | mainBranch := gitTestOutput(t, repo, "branch", "--show-current") | |||
| 183 | + | path := addMergedSideWorktree(t, repo, "side") | |||
| 184 | + | tool := newManageWorktreesTestTool(repo) | |||
| 185 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}, "delete_branch": true}) | |||
| 186 | + | ||||
| 187 | + | // The worktree is already removed when the delete progress is reported; | |||
| 188 | + | // swap the branch for a symref to the primary branch at the same commit. | |||
| 189 | + | progress := func(msg string) { | |||
| 190 | + | if strings.HasPrefix(msg, "Deleting branch") { | |||
| 191 | + | gitTestOutput(t, repo, "symbolic-ref", "refs/heads/side", "refs/heads/"+mainBranch) | |||
| 192 | + | } | |||
| 193 | + | } | |||
| 194 | + | _, err := tool.Execute(context.Background(), args, progress) | |||
| 195 | + | if err == nil || !strings.Contains(err.Error(), "symbolic") || !strings.Contains(err.Error(), "removed worktrees:") { | |||
| 196 | + | t.Fatalf("error = %v, want symbolic-ref branch failure", err) | |||
| 197 | + | } | |||
| 198 | + | if !branchExists(t, repo, mainBranch) { | |||
| 199 | + | t.Fatal("primary branch was deleted through a symbolic ref") | |||
| 200 | + | } | |||
| 201 | + | } | |||
| 202 | + | ||||
| 203 | + | func TestManageWorktreesDeleteBranchRefusesBranchCheckedOutAfterRemoval(t *testing.T) { | |||
| 204 | + | if _, err := exec.LookPath("git"); err != nil { | |||
| 205 | + | t.Skip("git is required") | |||
| 206 | + | } | |||
| 207 | + | for _, where := range []string{"primary", "worktree"} { | |||
| 208 | + | t.Run(where, func(t *testing.T) { | |||
| 209 | + | repo := initWorktreeTestRepo(t) | |||
| 210 | + | path := addMergedSideWorktree(t, repo, "side") | |||
| 211 | + | tool := newManageWorktreesTestTool(repo) | |||
| 212 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}, "delete_branch": true}) | |||
| 213 | + | progress := func(msg string) { | |||
| 214 | + | if !strings.HasPrefix(msg, "Deleting branch") { | |||
| 215 | + | return | |||
| 216 | + | } | |||
| 217 | + | if where == "primary" { | |||
| 218 | + | gitTestOutput(t, repo, "checkout", "-q", "side") | |||
| 219 | + | } else { | |||
| 220 | + | gitTestOutput(t, repo, "worktree", "add", filepath.Join(t.TempDir(), "again"), "side") | |||
| 221 | + | } | |||
| 222 | + | } | |||
| 223 | + | _, err := tool.Execute(context.Background(), args, progress) | |||
| 224 | + | if err == nil || !strings.Contains(err.Error(), `delete branch "side"`) || !strings.Contains(err.Error(), "checked out") { | |||
| 225 | + | t.Fatalf("error = %v, want checked-out branch failure", err) | |||
| 226 | + | } | |||
| 227 | + | if !branchExists(t, repo, "side") { | |||
| 228 | + | t.Fatal("checked-out branch was deleted") | |||
| 229 | + | } | |||
| 230 | + | }) | |||
| 231 | + | } | |||
| 232 | + | } | |||
| 233 | + | ||||
| 234 | + | func TestManageWorktreesDeleteBranchRequiresRefStorageWriteAccess(t *testing.T) { | |||
| 235 | + | if _, err := exec.LookPath("git"); err != nil { | |||
| 236 | + | t.Skip("git is required") | |||
| 237 | + | } | |||
| 238 | + | repo := initWorktreeTestRepo(t) | |||
| 239 | + | path := addMergedSideWorktree(t, repo, "side") | |||
| 240 | + | scoped := func(write ...string) *ManageWorktreesTool { | |||
| 241 | + | var p PermissionSet | |||
| 242 | + | p.Bash.Mode = "ask" | |||
| 243 | + | p.FS.Read = []string{repo, filepath.Dir(path)} | |||
| 244 | + | p.FS.Write = write | |||
| 245 | + | sandbox := NewSandbox(repo) | |||
| 246 | + | sandbox.SetPermissions(&p) | |||
| 247 | + | return &ManageWorktreesTool{CWD: repo, Sandbox: sandbox} | |||
| 248 | + | } | |||
| 249 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}, "delete_branch": true}) | |||
| 250 | + | ||||
| 251 | + | tool := scoped(path) | |||
| 252 | + | if _, err := tool.Preview(context.Background(), args); err == nil || !strings.Contains(err.Error(), "outside declared scopes") { | |||
| 253 | + | t.Fatalf("preview error = %v, want ref-storage write denial", err) | |||
| 254 | + | } | |||
| 255 | + | if _, err := tool.Execute(context.Background(), args, nil); err == nil || !strings.Contains(err.Error(), "outside declared scopes") { | |||
| 256 | + | t.Fatalf("execute error = %v, want ref-storage write denial", err) | |||
| 257 | + | } | |||
| 258 | + | if _, err := os.Stat(path); err != nil { | |||
| 259 | + | t.Fatalf("worktree was removed despite denial: %v", err) | |||
| 260 | + | } | |||
| 261 | + | if !branchExists(t, repo, "side") { | |||
| 262 | + | t.Fatal("branch was deleted despite denial") | |||
| 263 | + | } | |||
| 264 | + | ||||
| 265 | + | // Without delete_branch only the worktree path needs write access. | |||
| 266 | + | keep := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}}) | |||
| 267 | + | if _, err := tool.Preview(context.Background(), keep); err != nil { | |||
| 268 | + | t.Fatalf("cleanup without delete_branch: %v", err) | |||
| 269 | + | } | |||
| 270 | + | // Granting the common git directory allows the full operation. | |||
| 271 | + | tool = scoped(path, filepath.Join(repo, ".git")) | |||
| 272 | + | if _, err := tool.Execute(context.Background(), args, nil); err != nil { | |||
| 273 | + | t.Fatal(err) | |||
| 274 | + | } | |||
| 275 | + | if branchExists(t, repo, "side") { | |||
| 276 | + | t.Fatal("branch remains after permitted cleanup") | |||
| 277 | + | } | |||
| 278 | + | } | |||
| 279 | + | ||||
| 280 | + | func TestManageWorktreesDetectsMovedMergeTargetAfterPreview(t *testing.T) { | |||
| 281 | + | if _, err := exec.LookPath("git"); err != nil { | |||
| 282 | + | t.Skip("git is required") | |||
| 283 | + | } | |||
| 284 | + | repo := initWorktreeTestRepo(t) | |||
| 285 | + | path := addMergedSideWorktree(t, repo, "side") | |||
| 286 | + | gitTestOutput(t, repo, "branch", "target") | |||
| 287 | + | tool := newManageWorktreesTestTool(repo) | |||
| 288 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}, "into": "target", "delete_branch": true}) | |||
| 289 | + | if _, err := tool.Preview(context.Background(), args); err != nil { | |||
| 290 | + | t.Fatal(err) | |||
| 291 | + | } | |||
| 292 | + | // Advance the unchecked-out target; side remains an ancestor, so the | |||
| 293 | + | // worktree stays eligible and only the pinned target differs. | |||
| 294 | + | tree := gitTestOutput(t, repo, "rev-parse", "target^{tree}") | |||
| 295 | + | moved := gitTestOutput(t, repo, "commit-tree", tree, "-p", "target", "-m", "advance") | |||
| 296 | + | gitTestOutput(t, repo, "update-ref", "refs/heads/target", moved) | |||
| 297 | + | ||||
| 298 | + | _, err := tool.Execute(context.Background(), args, nil) | |||
| 299 | + | if err == nil || !strings.Contains(err.Error(), "changed since preview") { | |||
| 300 | + | t.Fatalf("error = %v, want stale-state rejection", err) | |||
| 301 | + | } | |||
| 302 | + | if _, statErr := os.Stat(path); statErr != nil { | |||
| 303 | + | t.Fatalf("worktree was removed: %v", statErr) | |||
| 304 | + | } | |||
| 305 | + | if !branchExists(t, repo, "side") { | |||
| 306 | + | t.Fatal("branch was deleted") | |||
| 307 | + | } | |||
| 308 | + | } | |||
| 309 | + | ||||
| 310 | + | func TestManageWorktreesPartialFailureReportsBranchErrorsAndProgress(t *testing.T) { | |||
| 311 | + | if _, err := exec.LookPath("git"); err != nil { | |||
| 312 | + | t.Skip("git is required") | |||
| 313 | + | } | |||
| 314 | + | repo := initWorktreeTestRepo(t) | |||
| 315 | + | first := addMergedSideWorktree(t, repo, "first") | |||
| 316 | + | second := addMergedSideWorktree(t, repo, "second") | |||
| 317 | + | // A stale lock makes deleting "first" fail after its worktree is removed. | |||
| 318 | + | lock := filepath.Join(repo, ".git", "refs", "heads", "first.lock") | |||
| 319 | + | if err := os.WriteFile(lock, nil, 0o644); err != nil { | |||
| 320 | + | t.Fatal(err) | |||
| 321 | + | } | |||
| 322 | + | tool := newManageWorktreesTestTool(repo) | |||
| 323 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{first, second}, "delete_branch": true}) | |||
| 324 | + | // Dirty the second worktree so its removal fails. | |||
| 325 | + | progress := func(msg string) { | |||
| 326 | + | if strings.HasPrefix(msg, "Removing worktree") && strings.Contains(msg, filepath.ToSlash(second)) { | |||
| 327 | + | if err := os.WriteFile(filepath.Join(second, "untracked.txt"), []byte("x"), 0o644); err != nil { | |||
| 328 | + | t.Error(err) | |||
| 329 | + | } | |||
| 330 | + | } | |||
| 331 | + | } | |||
| 332 | + | _, err := tool.Execute(context.Background(), args, progress) | |||
| 333 | + | if err == nil { | |||
| 334 | + | t.Fatal("expected partial failure") | |||
| 335 | + | } | |||
| 336 | + | for _, want := range []string{"remove ", `delete branch "first"`, "removed worktrees: " + filepath.ToSlash(first)} { | |||
| 337 | + | if !strings.Contains(err.Error(), want) { | |||
| 338 | + | t.Fatalf("error missing %q:\n%v", want, err) | |||
| 339 | + | } | |||
| 340 | + | } | |||
| 341 | + | if _, statErr := os.Stat(second); statErr != nil { | |||
| 342 | + | t.Fatalf("second worktree should remain: %v", statErr) | |||
| 343 | + | } | |||
| 344 | + | } | |||
M packages/agent/tools/resident_tools_test.go +1 -1
| @@ -108,7 +108,7 @@ func TestResidentToolsUseManagerOnly(t *testing.T) { | |||
| 108 | 108 | ||
| 109 | 109 | func TestSubagentSpawnGuidanceRequiresIndependentOwnership(t *testing.T) { | |
| 110 | 110 | facade := &SubagentTool{} | |
| 111 | - | for _, want := range []string{"independent sidecar", "keep immediate blockers local", "never duplicate it in the parent", "end or yield the parent turn"} { | |
| 111 | + | for _, want := range []string{"self-contained task", "never duplicate it in the parent", "end or yield the parent turn"} { | |
| 112 | 112 | if got := facade.Description(); !strings.Contains(got, want) { | |
| 113 | 113 | t.Fatalf("description missing %q: %s", want, got) | |
| 114 | 114 | } | |
| @@ -108,7 +108,7 @@ func TestResidentToolsUseManagerOnly(t *testing.T) { | |||||
| 108 | 108 | ||||
| 109 | func TestSubagentSpawnGuidanceRequiresIndependentOwnership(t *testing.T) { | 109 | func TestSubagentSpawnGuidanceRequiresIndependentOwnership(t *testing.T) { | ||
| 110 | facade := &SubagentTool{} | 110 | facade := &SubagentTool{} | ||
| 111 | - | for _, want := range []string{"independent sidecar", "keep immediate blockers local", "never duplicate it in the parent", "end or yield the parent turn"} { | 111 | + | for _, want := range []string{"self-contained task", "never duplicate it in the parent", "end or yield the parent turn"} { |
| 112 | if got := facade.Description(); !strings.Contains(got, want) { | 112 | if got := facade.Description(); !strings.Contains(got, want) { | ||
| 113 | t.Fatalf("description missing %q: %s", want, got) | 113 | t.Fatalf("description missing %q: %s", want, got) | ||
| 114 | } | 114 | } | ||
M packages/agent/tools/subagent.go +1 -1
| @@ -26,7 +26,7 @@ type SubagentTool struct { | |||
| 26 | 26 | // subagentSpawnGuidance is the operational contract for delegation. It is the | |
| 27 | 27 | // facade description tail and the internal spawn tool description, so the text | |
| 28 | 28 | // cannot drift between them. | |
| 29 | - | const subagentSpawnGuidance = "Delegate a concrete, bounded scope to a resident sub-agent. For proactive delegation, use an independent sidecar only when the parent has useful non-overlapping work; keep immediate blockers local. A worker owns its scope until completion, so never duplicate it in the parent. If delegation owns the blocking task, end or yield the parent turn. Omit wait to return immediately and receive completion through [auto-subagents update]; set wait to an explicit 1–300 second value only when this turn should wait for the initial task. Set required=true when the outcome is mandatory before the parent's terminal response; failures remain recoverable through the resume action. Do not use resume solely to wait for an existing task or request its result again; yield for the automatic completion update instead. Resume requires a real new instruction; queue schedules a separate turn and another completion. Never use bash sleep, watch, tail -f, polling loops, repeated status calls, dashboard, metadata, or file checks solely to wait." | |
| 29 | + | const subagentSpawnGuidance = "Delegate a concrete, bounded scope to a resident sub-agent with a self-contained task. A worker owns its scope until completion, so never duplicate it in the parent. If delegation owns the blocking task, end or yield the parent turn. Omit wait to return immediately and receive completion through [auto-subagents update]; set wait to an explicit 1–300 second value only when this turn should wait for the initial task. Set required=true when the outcome is mandatory before the parent's terminal response; failures remain recoverable through the resume action. Do not use resume solely to wait for an existing task or request its result again; yield for the automatic completion update instead. Resume requires a real new instruction; queue schedules a separate turn and another completion. Never use bash sleep, watch, tail -f, polling loops, repeated status calls, dashboard, metadata, or file checks solely to wait." | |
| 30 | 30 | ||
| 31 | 31 | // subagentSchema merges the action schemas into the flat, model-facing schema | |
| 32 | 32 | // the project keeps for provider compatibility. Every property is annotated | |
| @@ -26,7 +26,7 @@ type SubagentTool struct { | |||||
| 26 | // subagentSpawnGuidance is the operational contract for delegation. It is the | 26 | // subagentSpawnGuidance is the operational contract for delegation. It is the | ||
| 27 | // facade description tail and the internal spawn tool description, so the text | 27 | // facade description tail and the internal spawn tool description, so the text | ||
| 28 | // cannot drift between them. | 28 | // cannot drift between them. | ||
| 29 | - | const subagentSpawnGuidance = "Delegate a concrete, bounded scope to a resident sub-agent. For proactive delegation, use an independent sidecar only when the parent has useful non-overlapping work; keep immediate blockers local. A worker owns its scope until completion, so never duplicate it in the parent. If delegation owns the blocking task, end or yield the parent turn. Omit wait to return immediately and receive completion through [auto-subagents update]; set wait to an explicit 1–300 second value only when this turn should wait for the initial task. Set required=true when the outcome is mandatory before the parent's terminal response; failures remain recoverable through the resume action. Do not use resume solely to wait for an existing task or request its result again; yield for the automatic completion update instead. Resume requires a real new instruction; queue schedules a separate turn and another completion. Never use bash sleep, watch, tail -f, polling loops, repeated status calls, dashboard, metadata, or file checks solely to wait." | 29 | + | const subagentSpawnGuidance = "Delegate a concrete, bounded scope to a resident sub-agent with a self-contained task. A worker owns its scope until completion, so never duplicate it in the parent. If delegation owns the blocking task, end or yield the parent turn. Omit wait to return immediately and receive completion through [auto-subagents update]; set wait to an explicit 1–300 second value only when this turn should wait for the initial task. Set required=true when the outcome is mandatory before the parent's terminal response; failures remain recoverable through the resume action. Do not use resume solely to wait for an existing task or request its result again; yield for the automatic completion update instead. Resume requires a real new instruction; queue schedules a separate turn and another completion. Never use bash sleep, watch, tail -f, polling loops, repeated status calls, dashboard, metadata, or file checks solely to wait." |
| 30 | 30 | ||||
| 31 | // subagentSchema merges the action schemas into the flat, model-facing schema | 31 | // subagentSchema merges the action schemas into the flat, model-facing schema | ||
| 32 | // the project keeps for provider compatibility. Every property is annotated | 32 | // the project keeps for provider compatibility. Every property is annotated | ||
M packages/agent/tools/worktree.go +18 -11
| @@ -22,14 +22,17 @@ const worktreeSchema = `{ | |||
| 22 | 22 | "properties":{ | |
| 23 | 23 | "action":{"type":"string","enum":["create","list","cleanup"],"description":"Create a branch worktree, list repository worktrees, or remove explicitly selected safe worktrees."}, | |
| 24 | 24 | "branch":{"type":"string","description":"New branch name. Required for create."}, | |
| 25 | - | "paths":{"type":"array","items":{"type":"string"},"description":"Exact registered worktree paths. Required for cleanup."} | |
| 25 | + | "base":{"type":"string","description":"create only: branch, tag, or commit the new branch starts from. Defaults to the current HEAD. For a sub-worktree of a feature branch (for example feat/x-api from feat/x), set base to that feature branch."}, | |
| 26 | + | "paths":{"type":"array","items":{"type":"string"},"description":"Exact registered worktree paths. Required for cleanup."}, | |
| 27 | + | "into":{"type":"string","description":"cleanup/list: branch, tag, or commit the worktree branches must be merged into to be removable. Defaults to the current HEAD. After merging sub-branches into a feature branch, set into to that feature branch."}, | |
| 28 | + | "delete_branch":{"type":"boolean","description":"cleanup only: also delete each removed worktree's local branch, only if it is still at the inspected commit. Never deletes the merge target or a branch checked out elsewhere."} | |
| 26 | 29 | }, | |
| 27 | 30 | "required":["action"] | |
| 28 | 31 | }` | |
| 29 | 32 | ||
| 30 | 33 | func (t *WorktreeTool) Name() string { return "worktree" } | |
| 31 | 34 | func (t *WorktreeTool) Description() string { | |
| 32 | - | return "Create, list, and safely clean up Git worktrees." | |
| 35 | + | return "Create, list, and safely clean up Git worktrees. Supports sub-worktrees based on a feature branch and cleanup of branches merged into it." | |
| 33 | 36 | } | |
| 34 | 37 | func (t *WorktreeTool) Schema() json.RawMessage { return json.RawMessage(worktreeSchema) } | |
| 35 | 38 | ||
| @@ -89,17 +92,21 @@ func parseWorktreeAction(raw json.RawMessage) (string, json.RawMessage, error) { | |||
| 89 | 92 | return "", nil, errors.New("worktree: action is required") | |
| 90 | 93 | } | |
| 91 | 94 | delete(values, "action") | |
| 92 | - | if action == "list" || action == "cleanup" { | |
| 93 | - | encoded, err := json.Marshal(map[string]any{"action": action, "paths": rawField(values, "paths")}) | |
| 94 | - | if err != nil { | |
| 95 | - | return "", nil, err | |
| 96 | - | } | |
| 97 | - | if action == "list" { | |
| 98 | - | encoded = json.RawMessage(`{"action":"list"}`) | |
| 95 | + | var fields map[string]any | |
| 96 | + | switch action { | |
| 97 | + | case "list": | |
| 98 | + | fields = map[string]any{"action": action, "into": rawField(values, "into")} | |
| 99 | + | case "cleanup": | |
| 100 | + | fields = map[string]any{ | |
| 101 | + | "action": action, | |
| 102 | + | "paths": rawField(values, "paths"), | |
| 103 | + | "into": rawField(values, "into"), | |
| 104 | + | "delete_branch": rawField(values, "delete_branch"), | |
| 99 | 105 | } | |
| 100 | - | return action, encoded, nil | |
| 106 | + | default: | |
| 107 | + | fields = map[string]any{"branch": rawField(values, "branch"), "base": rawField(values, "base")} | |
| 101 | 108 | } | |
| 102 | - | encoded, err := json.Marshal(map[string]any{"branch": rawField(values, "branch")}) | |
| 109 | + | encoded, err := json.Marshal(fields) | |
| 103 | 110 | return action, encoded, err | |
| 104 | 111 | } | |
| 105 | 112 | ||
| @@ -22,14 +22,17 @@ const worktreeSchema = `{ | |||||
| 22 | "properties":{ | 22 | "properties":{ | ||
| 23 | "action":{"type":"string","enum":["create","list","cleanup"],"description":"Create a branch worktree, list repository worktrees, or remove explicitly selected safe worktrees."}, | 23 | "action":{"type":"string","enum":["create","list","cleanup"],"description":"Create a branch worktree, list repository worktrees, or remove explicitly selected safe worktrees."}, | ||
| 24 | "branch":{"type":"string","description":"New branch name. Required for create."}, | 24 | "branch":{"type":"string","description":"New branch name. Required for create."}, | ||
| 25 | - | "paths":{"type":"array","items":{"type":"string"},"description":"Exact registered worktree paths. Required for cleanup."} | 25 | + | "base":{"type":"string","description":"create only: branch, tag, or commit the new branch starts from. Defaults to the current HEAD. For a sub-worktree of a feature branch (for example feat/x-api from feat/x), set base to that feature branch."}, |
| 26 | + | "paths":{"type":"array","items":{"type":"string"},"description":"Exact registered worktree paths. Required for cleanup."}, | |||
| 27 | + | "into":{"type":"string","description":"cleanup/list: branch, tag, or commit the worktree branches must be merged into to be removable. Defaults to the current HEAD. After merging sub-branches into a feature branch, set into to that feature branch."}, | |||
| 28 | + | "delete_branch":{"type":"boolean","description":"cleanup only: also delete each removed worktree's local branch, only if it is still at the inspected commit. Never deletes the merge target or a branch checked out elsewhere."} | |||
| 26 | }, | 29 | }, | ||
| 27 | "required":["action"] | 30 | "required":["action"] | ||
| 28 | }` | 31 | }` | ||
| 29 | 32 | ||||
| 30 | func (t *WorktreeTool) Name() string { return "worktree" } | 33 | func (t *WorktreeTool) Name() string { return "worktree" } | ||
| 31 | func (t *WorktreeTool) Description() string { | 34 | func (t *WorktreeTool) Description() string { | ||
| 32 | - | return "Create, list, and safely clean up Git worktrees." | 35 | + | return "Create, list, and safely clean up Git worktrees. Supports sub-worktrees based on a feature branch and cleanup of branches merged into it." |
| 33 | } | 36 | } | ||
| 34 | func (t *WorktreeTool) Schema() json.RawMessage { return json.RawMessage(worktreeSchema) } | 37 | func (t *WorktreeTool) Schema() json.RawMessage { return json.RawMessage(worktreeSchema) } | ||
| 35 | 38 | ||||
| @@ -89,17 +92,21 @@ func parseWorktreeAction(raw json.RawMessage) (string, json.RawMessage, error) { | |||||
| 89 | return "", nil, errors.New("worktree: action is required") | 92 | return "", nil, errors.New("worktree: action is required") | ||
| 90 | } | 93 | } | ||
| 91 | delete(values, "action") | 94 | delete(values, "action") | ||
| 92 | - | if action == "list" || action == "cleanup" { | 95 | + | var fields map[string]any |
| 93 | - | encoded, err := json.Marshal(map[string]any{"action": action, "paths": rawField(values, "paths")}) | 96 | + | switch action { |
| 94 | - | if err != nil { | 97 | + | case "list": |
| 95 | - | return "", nil, err | 98 | + | fields = map[string]any{"action": action, "into": rawField(values, "into")} |
| 96 | - | } | 99 | + | case "cleanup": |
| 97 | - | if action == "list" { | 100 | + | fields = map[string]any{ |
| 98 | - | encoded = json.RawMessage(`{"action":"list"}`) | 101 | + | "action": action, |
| 102 | + | "paths": rawField(values, "paths"), | |||
| 103 | + | "into": rawField(values, "into"), | |||
| 104 | + | "delete_branch": rawField(values, "delete_branch"), | |||
| 99 | } | 105 | } | ||
| 100 | - | return action, encoded, nil | 106 | + | default: |
| 107 | + | fields = map[string]any{"branch": rawField(values, "branch"), "base": rawField(values, "base")} | |||
| 101 | } | 108 | } | ||
| 102 | - | encoded, err := json.Marshal(map[string]any{"branch": rawField(values, "branch")}) | 109 | + | encoded, err := json.Marshal(fields) |
| 103 | return action, encoded, err | 110 | return action, encoded, err | ||
| 104 | } | 111 | } | ||
| 105 | 112 | ||||
M packages/agent/tools/worktree_test.go +225 -0
| @@ -2,8 +2,10 @@ package tools | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "os" | |
| 5 | 6 | "os/exec" | |
| 6 | 7 | "path/filepath" | |
| 8 | + | "strings" | |
| 7 | 9 | "testing" | |
| 8 | 10 | ) | |
| 9 | 11 | ||
| @@ -35,3 +37,226 @@ func TestWorktreeToolRequiresAction(t *testing.T) { | |||
| 35 | 37 | t.Fatal("missing action was accepted") | |
| 36 | 38 | } | |
| 37 | 39 | } | |
| 40 | + | ||
| 41 | + | // nestedWorktreeFixture creates feat/x and feat/x-api (based on feat/x) | |
| 42 | + | // through the public tool and returns their paths. | |
| 43 | + | func nestedWorktreeFixture(t *testing.T) (tool *WorktreeTool, repo, featPath, apiPath string) { | |
| 44 | + | t.Helper() | |
| 45 | + | if _, err := exec.LookPath("git"); err != nil { | |
| 46 | + | t.Skip("git is required") | |
| 47 | + | } | |
| 48 | + | repo = initWorktreeTestRepo(t) | |
| 49 | + | root := t.TempDir() | |
| 50 | + | tool = &WorktreeTool{ | |
| 51 | + | Create: &CreateWorktreeTool{CWD: repo, Sandbox: NewSandbox(repo), DefaultRoot: root}, | |
| 52 | + | Manage: &ManageWorktreesTool{CWD: repo, Sandbox: NewSandbox(repo)}, | |
| 53 | + | } | |
| 54 | + | ctx := context.Background() | |
| 55 | + | if _, err := tool.Execute(ctx, mustJSON(t, map[string]any{"action": "create", "branch": "feat/x"}), nil); err != nil { | |
| 56 | + | t.Fatal(err) | |
| 57 | + | } | |
| 58 | + | featPath = filepath.Join(root, testWorktreeRepoID(t, repo), "feat", "x") | |
| 59 | + | // Advance the feature branch so HEAD of the primary checkout differs. | |
| 60 | + | if err := os.WriteFile(filepath.Join(featPath, "feature.txt"), []byte("feature\n"), 0o644); err != nil { | |
| 61 | + | t.Fatal(err) | |
| 62 | + | } | |
| 63 | + | gitTestOutput(t, featPath, "add", "feature.txt") | |
| 64 | + | gitTestOutput(t, featPath, "commit", "-qm", "feature work") | |
| 65 | + | ||
| 66 | + | result, err := tool.Execute(ctx, mustJSON(t, map[string]any{"action": "create", "branch": "feat/x-api", "base": "feat/x"}), nil) | |
| 67 | + | if err != nil { | |
| 68 | + | t.Fatal(err) | |
| 69 | + | } | |
| 70 | + | details := result.Details.(map[string]any) | |
| 71 | + | featHead := gitTestOutput(t, featPath, "rev-parse", "HEAD") | |
| 72 | + | if details["base"] != featHead || details["base_ref"] != "feat/x" { | |
| 73 | + | t.Fatalf("details = %#v, want base %s from feat/x", details, featHead) | |
| 74 | + | } | |
| 75 | + | if text := resultText(result); !strings.Contains(text, "base: feat/x ("+featHead+")") { | |
| 76 | + | t.Fatalf("result text missing input ref and sha:\n%s", text) | |
| 77 | + | } | |
| 78 | + | apiPath = filepath.Join(root, testWorktreeRepoID(t, repo), "feat", "x-api") | |
| 79 | + | if got := gitTestOutput(t, apiPath, "rev-parse", "HEAD"); got != featHead { | |
| 80 | + | t.Fatalf("sub-worktree HEAD = %s, want %s", got, featHead) | |
| 81 | + | } | |
| 82 | + | return tool, repo, featPath, apiPath | |
| 83 | + | } | |
| 84 | + | ||
| 85 | + | func commitInWorktree(t *testing.T, path, name string) { | |
| 86 | + | t.Helper() | |
| 87 | + | if err := os.WriteFile(filepath.Join(path, name), []byte(name+"\n"), 0o644); err != nil { | |
| 88 | + | t.Fatal(err) | |
| 89 | + | } | |
| 90 | + | gitTestOutput(t, path, "add", name) | |
| 91 | + | gitTestOutput(t, path, "commit", "-qm", name) | |
| 92 | + | } | |
| 93 | + | ||
| 94 | + | func TestWorktreeToolRejectsInvalidBase(t *testing.T) { | |
| 95 | + | if _, err := exec.LookPath("git"); err != nil { | |
| 96 | + | t.Skip("git is required") | |
| 97 | + | } | |
| 98 | + | repo := initWorktreeTestRepo(t) | |
| 99 | + | tool := &WorktreeTool{Create: &CreateWorktreeTool{CWD: repo, Sandbox: NewSandbox(repo), DefaultRoot: t.TempDir()}} | |
| 100 | + | for _, base := range []string{"no-such-branch", "--output=/tmp/x", "-h"} { | |
| 101 | + | _, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{"action": "create", "branch": "feat/bad", "base": base}), nil) | |
| 102 | + | if err == nil || !strings.HasPrefix(err.Error(), "worktree: ") || !strings.Contains(err.Error(), "invalid base") { | |
| 103 | + | t.Fatalf("base %q: error = %v, want worktree invalid base", base, err) | |
| 104 | + | } | |
| 105 | + | } | |
| 106 | + | if got := gitTestOutputAllowExit(t, repo, "show-ref", "--verify", "--quiet", "refs/heads/feat/bad"); got.exitCode != 1 { | |
| 107 | + | t.Fatalf("branch was created: %#v", got) | |
| 108 | + | } | |
| 109 | + | } | |
| 110 | + | ||
| 111 | + | func TestWorktreeToolBasePreviewDetectsMovedBase(t *testing.T) { | |
| 112 | + | tool, _, featPath, _ := nestedWorktreeFixture(t) | |
| 113 | + | args := mustJSON(t, map[string]any{"action": "create", "branch": "feat/x-ui", "base": "feat/x"}) | |
| 114 | + | if _, err := tool.Preview(context.Background(), args); err != nil { | |
| 115 | + | t.Fatal(err) | |
| 116 | + | } | |
| 117 | + | commitInWorktree(t, featPath, "moved.txt") | |
| 118 | + | _, err := tool.Execute(context.Background(), args, nil) | |
| 119 | + | if err == nil || !strings.Contains(err.Error(), "changed since preview") { | |
| 120 | + | t.Fatalf("error = %v, want stale preview rejection", err) | |
| 121 | + | } | |
| 122 | + | } | |
| 123 | + | ||
| 124 | + | func TestWorktreeToolCleanupIntoFeatureBranchAndDeleteBranch(t *testing.T) { | |
| 125 | + | tool, repo, featPath, apiPath := nestedWorktreeFixture(t) | |
| 126 | + | ctx := context.Background() | |
| 127 | + | commitInWorktree(t, apiPath, "api.txt") | |
| 128 | + | cleanup := map[string]any{"action": "cleanup", "paths": []string{apiPath}} | |
| 129 | + | ||
| 130 | + | // Unmerged: blocked for both the default target and the feature branch. | |
| 131 | + | for _, args := range []map[string]any{cleanup, {"action": "cleanup", "paths": []string{apiPath}, "into": "feat/x"}} { | |
| 132 | + | _, err := tool.Preview(ctx, mustJSON(t, args)) | |
| 133 | + | if err == nil || !strings.Contains(err.Error(), "not merged") { | |
| 134 | + | t.Fatalf("args %v: error = %v, want unmerged rejection", args, err) | |
| 135 | + | } | |
| 136 | + | } | |
| 137 | + | gitTestOutput(t, featPath, "merge", "--no-edit", "feat/x-api") | |
| 138 | + | ||
| 139 | + | // Default HEAD (primary checkout) does not contain the merge. | |
| 140 | + | _, err := tool.Preview(ctx, mustJSON(t, cleanup)) | |
| 141 | + | if err == nil || !strings.Contains(err.Error(), "not merged into the current branch") { | |
| 142 | + | t.Fatalf("error = %v, want current-branch rejection", err) | |
| 143 | + | } | |
| 144 | + | // list with into reports eligibility against the feature branch. | |
| 145 | + | listed, err := tool.Execute(ctx, mustJSON(t, map[string]any{"action": "list", "into": "feat/x"}), nil) | |
| 146 | + | if err != nil || !strings.Contains(resultText(listed), `clean and merged into "feat/x"; cleanup eligible`) { | |
| 147 | + | t.Fatalf("list = %v, %q", err, resultText(listed)) | |
| 148 | + | } | |
| 149 | + | _, err = tool.Preview(ctx, mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{apiPath}, "into": "-bad"})) | |
| 150 | + | if err == nil || !strings.Contains(err.Error(), "invalid into") { | |
| 151 | + | t.Fatalf("error = %v, want invalid into", err) | |
| 152 | + | } | |
| 153 | + | ||
| 154 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{apiPath}, "into": "feat/x", "delete_branch": true}) | |
| 155 | + | preview, err := tool.Preview(ctx, args) | |
| 156 | + | if err != nil || !strings.Contains(resultText(preview), "delete branches:\n- feat/x-api") { | |
| 157 | + | t.Fatalf("preview = %v, %q", err, resultText(preview)) | |
| 158 | + | } | |
| 159 | + | result, err := tool.Execute(ctx, args, nil) | |
| 160 | + | if err != nil { | |
| 161 | + | t.Fatal(err) | |
| 162 | + | } | |
| 163 | + | if _, err := os.Stat(apiPath); !os.IsNotExist(err) { | |
| 164 | + | t.Fatalf("sub-worktree remains: %v", err) | |
| 165 | + | } | |
| 166 | + | if got := gitTestOutputAllowExit(t, repo, "show-ref", "--verify", "--quiet", "refs/heads/feat/x-api"); got.exitCode != 1 { | |
| 167 | + | t.Fatalf("branch remains: %#v", got) | |
| 168 | + | } | |
| 169 | + | if got := gitTestOutput(t, repo, "show-ref", "--verify", "refs/heads/feat/x"); got == "" { | |
| 170 | + | t.Fatal("feature branch was deleted") | |
| 171 | + | } | |
| 172 | + | if text := resultText(result); !strings.Contains(text, "deleted branches:\n- feat/x-api") { | |
| 173 | + | t.Fatalf("result text missing deleted branch:\n%s", text) | |
| 174 | + | } | |
| 175 | + | details := result.Details.(map[string]any) | |
| 176 | + | if deleted, _ := details["deleted_branches"].([]string); len(deleted) != 1 || deleted[0] != "feat/x-api" { | |
| 177 | + | t.Fatalf("details = %#v", details) | |
| 178 | + | } | |
| 179 | + | } | |
| 180 | + | ||
| 181 | + | func TestWorktreeToolCleanupWithoutDeleteBranchKeepsBranch(t *testing.T) { | |
| 182 | + | tool, repo, _, apiPath := nestedWorktreeFixture(t) | |
| 183 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{apiPath}, "into": "feat/x"}) | |
| 184 | + | if _, err := tool.Execute(context.Background(), args, nil); err != nil { | |
| 185 | + | t.Fatal(err) | |
| 186 | + | } | |
| 187 | + | if got := gitTestOutput(t, repo, "show-ref", "--verify", "refs/heads/feat/x-api"); got == "" { | |
| 188 | + | t.Fatal("branch removed without delete_branch") | |
| 189 | + | } | |
| 190 | + | } | |
| 191 | + | ||
| 192 | + | func TestWorktreeToolDeleteBranchRefusesMergeTarget(t *testing.T) { | |
| 193 | + | tool, repo, featPath, _ := nestedWorktreeFixture(t) | |
| 194 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{featPath}, "into": "feat/x", "delete_branch": true}) | |
| 195 | + | _, err := tool.Preview(context.Background(), args) | |
| 196 | + | if err == nil || !strings.Contains(err.Error(), "merge target") { | |
| 197 | + | t.Fatalf("error = %v, want merge-target rejection", err) | |
| 198 | + | } | |
| 199 | + | if _, statErr := os.Stat(featPath); statErr != nil { | |
| 200 | + | t.Fatal(statErr) | |
| 201 | + | } | |
| 202 | + | _ = repo | |
| 203 | + | } | |
| 204 | + | ||
| 205 | + | func TestWorktreeToolDeleteBranchRejectsMovedBranchAfterPreview(t *testing.T) { | |
| 206 | + | tool, repo, featPath, apiPath := nestedWorktreeFixture(t) | |
| 207 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{apiPath}, "into": "feat/x", "delete_branch": true}) | |
| 208 | + | if _, err := tool.Preview(context.Background(), args); err != nil { | |
| 209 | + | t.Fatal(err) | |
| 210 | + | } | |
| 211 | + | // A new commit after preview leaves the branch unmerged, so execution | |
| 212 | + | // must fail closed without removing the worktree or deleting the branch. | |
| 213 | + | commitInWorktree(t, apiPath, "late.txt") | |
| 214 | + | _, err := tool.Execute(context.Background(), args, nil) | |
| 215 | + | if err == nil || !strings.Contains(err.Error(), "not merged") { | |
| 216 | + | t.Fatalf("error = %v, want unmerged rejection", err) | |
| 217 | + | } | |
| 218 | + | if got := gitTestOutput(t, repo, "show-ref", "--verify", "refs/heads/feat/x-api"); got == "" { | |
| 219 | + | t.Fatal("branch was deleted") | |
| 220 | + | } | |
| 221 | + | if _, statErr := os.Stat(apiPath); statErr != nil { | |
| 222 | + | t.Fatalf("worktree was removed: %v", statErr) | |
| 223 | + | } | |
| 224 | + | _, _ = repo, featPath | |
| 225 | + | } | |
| 226 | + | ||
| 227 | + | func TestDeleteMergedBranchKeepsBranchThatMoved(t *testing.T) { | |
| 228 | + | _, repo, _, apiPath := nestedWorktreeFixture(t) | |
| 229 | + | inspected := gitTestOutput(t, apiPath, "rev-parse", "HEAD") | |
| 230 | + | commitInWorktree(t, apiPath, "moved.txt") | |
| 231 | + | gitTestOutput(t, repo, "worktree", "remove", apiPath) | |
| 232 | + | ||
| 233 | + | err := deleteMergedBranch(context.Background(), repo, "feat/x-api", inspected) | |
| 234 | + | if err == nil || !strings.Contains(err.Error(), "changed since inspection") { | |
| 235 | + | t.Fatalf("error = %v, want moved-branch rejection", err) | |
| 236 | + | } | |
| 237 | + | if got := gitTestOutput(t, repo, "show-ref", "--verify", "refs/heads/feat/x-api"); got == "" { | |
| 238 | + | t.Fatal("moved branch was deleted") | |
| 239 | + | } | |
| 240 | + | current := gitTestOutput(t, repo, "rev-parse", "refs/heads/feat/x-api") | |
| 241 | + | if err := deleteMergedBranch(context.Background(), repo, "feat/x-api", current); err != nil { | |
| 242 | + | t.Fatal(err) | |
| 243 | + | } | |
| 244 | + | } | |
| 245 | + | ||
| 246 | + | func TestWorktreeToolReportsBranchDeletionFailureAfterRemoval(t *testing.T) { | |
| 247 | + | tool, repo, _, apiPath := nestedWorktreeFixture(t) | |
| 248 | + | ctx := context.Background() | |
| 249 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{apiPath}, "into": "feat/x", "delete_branch": true}) | |
| 250 | + | // Lock the branch ref so update-ref cannot delete it after worktree removal. | |
| 251 | + | lock := filepath.Join(repo, ".git", "refs", "heads", "feat", "x-api.lock") | |
| 252 | + | if err := os.WriteFile(lock, nil, 0o644); err != nil { | |
| 253 | + | t.Fatal(err) | |
| 254 | + | } | |
| 255 | + | _, err := tool.Execute(ctx, args, nil) | |
| 256 | + | if err == nil || !strings.HasPrefix(err.Error(), "worktree: ") || !strings.Contains(err.Error(), `delete branch "feat/x-api"`) || !strings.Contains(err.Error(), "worktrees were removed") { | |
| 257 | + | t.Fatalf("error = %v, want clear branch-deletion failure", err) | |
| 258 | + | } | |
| 259 | + | if _, statErr := os.Stat(apiPath); !os.IsNotExist(statErr) { | |
| 260 | + | t.Fatalf("worktree remains: %v", statErr) | |
| 261 | + | } | |
| 262 | + | } | |
| @@ -2,8 +2,10 @@ package tools | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "os" | |||
| 5 | "os/exec" | 6 | "os/exec" | ||
| 6 | "path/filepath" | 7 | "path/filepath" | ||
| 8 | + | "strings" | |||
| 7 | "testing" | 9 | "testing" | ||
| 8 | ) | 10 | ) | ||
| 9 | 11 | ||||
| @@ -35,3 +37,226 @@ func TestWorktreeToolRequiresAction(t *testing.T) { | |||||
| 35 | t.Fatal("missing action was accepted") | 37 | t.Fatal("missing action was accepted") | ||
| 36 | } | 38 | } | ||
| 37 | } | 39 | } | ||
| 40 | + | ||||
| 41 | + | // nestedWorktreeFixture creates feat/x and feat/x-api (based on feat/x) | |||
| 42 | + | // through the public tool and returns their paths. | |||
| 43 | + | func nestedWorktreeFixture(t *testing.T) (tool *WorktreeTool, repo, featPath, apiPath string) { | |||
| 44 | + | t.Helper() | |||
| 45 | + | if _, err := exec.LookPath("git"); err != nil { | |||
| 46 | + | t.Skip("git is required") | |||
| 47 | + | } | |||
| 48 | + | repo = initWorktreeTestRepo(t) | |||
| 49 | + | root := t.TempDir() | |||
| 50 | + | tool = &WorktreeTool{ | |||
| 51 | + | Create: &CreateWorktreeTool{CWD: repo, Sandbox: NewSandbox(repo), DefaultRoot: root}, | |||
| 52 | + | Manage: &ManageWorktreesTool{CWD: repo, Sandbox: NewSandbox(repo)}, | |||
| 53 | + | } | |||
| 54 | + | ctx := context.Background() | |||
| 55 | + | if _, err := tool.Execute(ctx, mustJSON(t, map[string]any{"action": "create", "branch": "feat/x"}), nil); err != nil { | |||
| 56 | + | t.Fatal(err) | |||
| 57 | + | } | |||
| 58 | + | featPath = filepath.Join(root, testWorktreeRepoID(t, repo), "feat", "x") | |||
| 59 | + | // Advance the feature branch so HEAD of the primary checkout differs. | |||
| 60 | + | if err := os.WriteFile(filepath.Join(featPath, "feature.txt"), []byte("feature\n"), 0o644); err != nil { | |||
| 61 | + | t.Fatal(err) | |||
| 62 | + | } | |||
| 63 | + | gitTestOutput(t, featPath, "add", "feature.txt") | |||
| 64 | + | gitTestOutput(t, featPath, "commit", "-qm", "feature work") | |||
| 65 | + | ||||
| 66 | + | result, err := tool.Execute(ctx, mustJSON(t, map[string]any{"action": "create", "branch": "feat/x-api", "base": "feat/x"}), nil) | |||
| 67 | + | if err != nil { | |||
| 68 | + | t.Fatal(err) | |||
| 69 | + | } | |||
| 70 | + | details := result.Details.(map[string]any) | |||
| 71 | + | featHead := gitTestOutput(t, featPath, "rev-parse", "HEAD") | |||
| 72 | + | if details["base"] != featHead || details["base_ref"] != "feat/x" { | |||
| 73 | + | t.Fatalf("details = %#v, want base %s from feat/x", details, featHead) | |||
| 74 | + | } | |||
| 75 | + | if text := resultText(result); !strings.Contains(text, "base: feat/x ("+featHead+")") { | |||
| 76 | + | t.Fatalf("result text missing input ref and sha:\n%s", text) | |||
| 77 | + | } | |||
| 78 | + | apiPath = filepath.Join(root, testWorktreeRepoID(t, repo), "feat", "x-api") | |||
| 79 | + | if got := gitTestOutput(t, apiPath, "rev-parse", "HEAD"); got != featHead { | |||
| 80 | + | t.Fatalf("sub-worktree HEAD = %s, want %s", got, featHead) | |||
| 81 | + | } | |||
| 82 | + | return tool, repo, featPath, apiPath | |||
| 83 | + | } | |||
| 84 | + | ||||
| 85 | + | func commitInWorktree(t *testing.T, path, name string) { | |||
| 86 | + | t.Helper() | |||
| 87 | + | if err := os.WriteFile(filepath.Join(path, name), []byte(name+"\n"), 0o644); err != nil { | |||
| 88 | + | t.Fatal(err) | |||
| 89 | + | } | |||
| 90 | + | gitTestOutput(t, path, "add", name) | |||
| 91 | + | gitTestOutput(t, path, "commit", "-qm", name) | |||
| 92 | + | } | |||
| 93 | + | ||||
| 94 | + | func TestWorktreeToolRejectsInvalidBase(t *testing.T) { | |||
| 95 | + | if _, err := exec.LookPath("git"); err != nil { | |||
| 96 | + | t.Skip("git is required") | |||
| 97 | + | } | |||
| 98 | + | repo := initWorktreeTestRepo(t) | |||
| 99 | + | tool := &WorktreeTool{Create: &CreateWorktreeTool{CWD: repo, Sandbox: NewSandbox(repo), DefaultRoot: t.TempDir()}} | |||
| 100 | + | for _, base := range []string{"no-such-branch", "--output=/tmp/x", "-h"} { | |||
| 101 | + | _, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{"action": "create", "branch": "feat/bad", "base": base}), nil) | |||
| 102 | + | if err == nil || !strings.HasPrefix(err.Error(), "worktree: ") || !strings.Contains(err.Error(), "invalid base") { | |||
| 103 | + | t.Fatalf("base %q: error = %v, want worktree invalid base", base, err) | |||
| 104 | + | } | |||
| 105 | + | } | |||
| 106 | + | if got := gitTestOutputAllowExit(t, repo, "show-ref", "--verify", "--quiet", "refs/heads/feat/bad"); got.exitCode != 1 { | |||
| 107 | + | t.Fatalf("branch was created: %#v", got) | |||
| 108 | + | } | |||
| 109 | + | } | |||
| 110 | + | ||||
| 111 | + | func TestWorktreeToolBasePreviewDetectsMovedBase(t *testing.T) { | |||
| 112 | + | tool, _, featPath, _ := nestedWorktreeFixture(t) | |||
| 113 | + | args := mustJSON(t, map[string]any{"action": "create", "branch": "feat/x-ui", "base": "feat/x"}) | |||
| 114 | + | if _, err := tool.Preview(context.Background(), args); err != nil { | |||
| 115 | + | t.Fatal(err) | |||
| 116 | + | } | |||
| 117 | + | commitInWorktree(t, featPath, "moved.txt") | |||
| 118 | + | _, err := tool.Execute(context.Background(), args, nil) | |||
| 119 | + | if err == nil || !strings.Contains(err.Error(), "changed since preview") { | |||
| 120 | + | t.Fatalf("error = %v, want stale preview rejection", err) | |||
| 121 | + | } | |||
| 122 | + | } | |||
| 123 | + | ||||
| 124 | + | func TestWorktreeToolCleanupIntoFeatureBranchAndDeleteBranch(t *testing.T) { | |||
| 125 | + | tool, repo, featPath, apiPath := nestedWorktreeFixture(t) | |||
| 126 | + | ctx := context.Background() | |||
| 127 | + | commitInWorktree(t, apiPath, "api.txt") | |||
| 128 | + | cleanup := map[string]any{"action": "cleanup", "paths": []string{apiPath}} | |||
| 129 | + | ||||
| 130 | + | // Unmerged: blocked for both the default target and the feature branch. | |||
| 131 | + | for _, args := range []map[string]any{cleanup, {"action": "cleanup", "paths": []string{apiPath}, "into": "feat/x"}} { | |||
| 132 | + | _, err := tool.Preview(ctx, mustJSON(t, args)) | |||
| 133 | + | if err == nil || !strings.Contains(err.Error(), "not merged") { | |||
| 134 | + | t.Fatalf("args %v: error = %v, want unmerged rejection", args, err) | |||
| 135 | + | } | |||
| 136 | + | } | |||
| 137 | + | gitTestOutput(t, featPath, "merge", "--no-edit", "feat/x-api") | |||
| 138 | + | ||||
| 139 | + | // Default HEAD (primary checkout) does not contain the merge. | |||
| 140 | + | _, err := tool.Preview(ctx, mustJSON(t, cleanup)) | |||
| 141 | + | if err == nil || !strings.Contains(err.Error(), "not merged into the current branch") { | |||
| 142 | + | t.Fatalf("error = %v, want current-branch rejection", err) | |||
| 143 | + | } | |||
| 144 | + | // list with into reports eligibility against the feature branch. | |||
| 145 | + | listed, err := tool.Execute(ctx, mustJSON(t, map[string]any{"action": "list", "into": "feat/x"}), nil) | |||
| 146 | + | if err != nil || !strings.Contains(resultText(listed), `clean and merged into "feat/x"; cleanup eligible`) { | |||
| 147 | + | t.Fatalf("list = %v, %q", err, resultText(listed)) | |||
| 148 | + | } | |||
| 149 | + | _, err = tool.Preview(ctx, mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{apiPath}, "into": "-bad"})) | |||
| 150 | + | if err == nil || !strings.Contains(err.Error(), "invalid into") { | |||
| 151 | + | t.Fatalf("error = %v, want invalid into", err) | |||
| 152 | + | } | |||
| 153 | + | ||||
| 154 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{apiPath}, "into": "feat/x", "delete_branch": true}) | |||
| 155 | + | preview, err := tool.Preview(ctx, args) | |||
| 156 | + | if err != nil || !strings.Contains(resultText(preview), "delete branches:\n- feat/x-api") { | |||
| 157 | + | t.Fatalf("preview = %v, %q", err, resultText(preview)) | |||
| 158 | + | } | |||
| 159 | + | result, err := tool.Execute(ctx, args, nil) | |||
| 160 | + | if err != nil { | |||
| 161 | + | t.Fatal(err) | |||
| 162 | + | } | |||
| 163 | + | if _, err := os.Stat(apiPath); !os.IsNotExist(err) { | |||
| 164 | + | t.Fatalf("sub-worktree remains: %v", err) | |||
| 165 | + | } | |||
| 166 | + | if got := gitTestOutputAllowExit(t, repo, "show-ref", "--verify", "--quiet", "refs/heads/feat/x-api"); got.exitCode != 1 { | |||
| 167 | + | t.Fatalf("branch remains: %#v", got) | |||
| 168 | + | } | |||
| 169 | + | if got := gitTestOutput(t, repo, "show-ref", "--verify", "refs/heads/feat/x"); got == "" { | |||
| 170 | + | t.Fatal("feature branch was deleted") | |||
| 171 | + | } | |||
| 172 | + | if text := resultText(result); !strings.Contains(text, "deleted branches:\n- feat/x-api") { | |||
| 173 | + | t.Fatalf("result text missing deleted branch:\n%s", text) | |||
| 174 | + | } | |||
| 175 | + | details := result.Details.(map[string]any) | |||
| 176 | + | if deleted, _ := details["deleted_branches"].([]string); len(deleted) != 1 || deleted[0] != "feat/x-api" { | |||
| 177 | + | t.Fatalf("details = %#v", details) | |||
| 178 | + | } | |||
| 179 | + | } | |||
| 180 | + | ||||
| 181 | + | func TestWorktreeToolCleanupWithoutDeleteBranchKeepsBranch(t *testing.T) { | |||
| 182 | + | tool, repo, _, apiPath := nestedWorktreeFixture(t) | |||
| 183 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{apiPath}, "into": "feat/x"}) | |||
| 184 | + | if _, err := tool.Execute(context.Background(), args, nil); err != nil { | |||
| 185 | + | t.Fatal(err) | |||
| 186 | + | } | |||
| 187 | + | if got := gitTestOutput(t, repo, "show-ref", "--verify", "refs/heads/feat/x-api"); got == "" { | |||
| 188 | + | t.Fatal("branch removed without delete_branch") | |||
| 189 | + | } | |||
| 190 | + | } | |||
| 191 | + | ||||
| 192 | + | func TestWorktreeToolDeleteBranchRefusesMergeTarget(t *testing.T) { | |||
| 193 | + | tool, repo, featPath, _ := nestedWorktreeFixture(t) | |||
| 194 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{featPath}, "into": "feat/x", "delete_branch": true}) | |||
| 195 | + | _, err := tool.Preview(context.Background(), args) | |||
| 196 | + | if err == nil || !strings.Contains(err.Error(), "merge target") { | |||
| 197 | + | t.Fatalf("error = %v, want merge-target rejection", err) | |||
| 198 | + | } | |||
| 199 | + | if _, statErr := os.Stat(featPath); statErr != nil { | |||
| 200 | + | t.Fatal(statErr) | |||
| 201 | + | } | |||
| 202 | + | _ = repo | |||
| 203 | + | } | |||
| 204 | + | ||||
| 205 | + | func TestWorktreeToolDeleteBranchRejectsMovedBranchAfterPreview(t *testing.T) { | |||
| 206 | + | tool, repo, featPath, apiPath := nestedWorktreeFixture(t) | |||
| 207 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{apiPath}, "into": "feat/x", "delete_branch": true}) | |||
| 208 | + | if _, err := tool.Preview(context.Background(), args); err != nil { | |||
| 209 | + | t.Fatal(err) | |||
| 210 | + | } | |||
| 211 | + | // A new commit after preview leaves the branch unmerged, so execution | |||
| 212 | + | // must fail closed without removing the worktree or deleting the branch. | |||
| 213 | + | commitInWorktree(t, apiPath, "late.txt") | |||
| 214 | + | _, err := tool.Execute(context.Background(), args, nil) | |||
| 215 | + | if err == nil || !strings.Contains(err.Error(), "not merged") { | |||
| 216 | + | t.Fatalf("error = %v, want unmerged rejection", err) | |||
| 217 | + | } | |||
| 218 | + | if got := gitTestOutput(t, repo, "show-ref", "--verify", "refs/heads/feat/x-api"); got == "" { | |||
| 219 | + | t.Fatal("branch was deleted") | |||
| 220 | + | } | |||
| 221 | + | if _, statErr := os.Stat(apiPath); statErr != nil { | |||
| 222 | + | t.Fatalf("worktree was removed: %v", statErr) | |||
| 223 | + | } | |||
| 224 | + | _, _ = repo, featPath | |||
| 225 | + | } | |||
| 226 | + | ||||
| 227 | + | func TestDeleteMergedBranchKeepsBranchThatMoved(t *testing.T) { | |||
| 228 | + | _, repo, _, apiPath := nestedWorktreeFixture(t) | |||
| 229 | + | inspected := gitTestOutput(t, apiPath, "rev-parse", "HEAD") | |||
| 230 | + | commitInWorktree(t, apiPath, "moved.txt") | |||
| 231 | + | gitTestOutput(t, repo, "worktree", "remove", apiPath) | |||
| 232 | + | ||||
| 233 | + | err := deleteMergedBranch(context.Background(), repo, "feat/x-api", inspected) | |||
| 234 | + | if err == nil || !strings.Contains(err.Error(), "changed since inspection") { | |||
| 235 | + | t.Fatalf("error = %v, want moved-branch rejection", err) | |||
| 236 | + | } | |||
| 237 | + | if got := gitTestOutput(t, repo, "show-ref", "--verify", "refs/heads/feat/x-api"); got == "" { | |||
| 238 | + | t.Fatal("moved branch was deleted") | |||
| 239 | + | } | |||
| 240 | + | current := gitTestOutput(t, repo, "rev-parse", "refs/heads/feat/x-api") | |||
| 241 | + | if err := deleteMergedBranch(context.Background(), repo, "feat/x-api", current); err != nil { | |||
| 242 | + | t.Fatal(err) | |||
| 243 | + | } | |||
| 244 | + | } | |||
| 245 | + | ||||
| 246 | + | func TestWorktreeToolReportsBranchDeletionFailureAfterRemoval(t *testing.T) { | |||
| 247 | + | tool, repo, _, apiPath := nestedWorktreeFixture(t) | |||
| 248 | + | ctx := context.Background() | |||
| 249 | + | args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{apiPath}, "into": "feat/x", "delete_branch": true}) | |||
| 250 | + | // Lock the branch ref so update-ref cannot delete it after worktree removal. | |||
| 251 | + | lock := filepath.Join(repo, ".git", "refs", "heads", "feat", "x-api.lock") | |||
| 252 | + | if err := os.WriteFile(lock, nil, 0o644); err != nil { | |||
| 253 | + | t.Fatal(err) | |||
| 254 | + | } | |||
| 255 | + | _, err := tool.Execute(ctx, args, nil) | |||
| 256 | + | if err == nil || !strings.HasPrefix(err.Error(), "worktree: ") || !strings.Contains(err.Error(), `delete branch "feat/x-api"`) || !strings.Contains(err.Error(), "worktrees were removed") { | |||
| 257 | + | t.Fatalf("error = %v, want clear branch-deletion failure", err) | |||
| 258 | + | } | |||
| 259 | + | if _, statErr := os.Stat(apiPath); !os.IsNotExist(statErr) { | |||
| 260 | + | t.Fatalf("worktree remains: %v", statErr) | |||
| 261 | + | } | |||
| 262 | + | } | |||