From f50415dd7079cd7d1581e04b79b51550bd05fa48 Mon Sep 17 00:00:00 2001 From: John Flavan Date: Mon, 26 Oct 2026 22:07:03 -0500 Subject: [PATCH 01/64] feat: claude.mcp_tools, allowed in every session, offered during onboarding Sessions load the machine's and the repo's MCP servers, but a -p session cannot ask, so every MCP call was refused outside plan mode. MCP allow rules in claude.mcp_tools now reach every tool set, judges included. Entries must be mcp__ rules without commas, since the list reaches the CLI comma-joined. The operator skill lists the MCP tools its session has and writes the ones the engineer picks. Co-Authored-By: Claude Opus 5.5 --- README.md | 4 ++-- docs/decisions.md | 6 ++++++ src/config.ts | 5 +++++ src/phases.ts | 14 ++++++++++---- templates/config.yaml | 3 +++ templates/skill/SKILL.md | 11 ++++++----- tests/unit/config.test.ts | 7 +++++++ tests/unit/phases.test.ts | 18 ++++++++++++++++++ 8 files changed, 57 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 290ddcdf..0f51fe89 100644 --- a/README.md +++ b/README.md @@ -298,14 +298,14 @@ The full design is in `docs/superpowers/specs/2026-09-28-loopstra-design.md`, wi - `main_branch`, `poll_seconds` - `commands`: test (required), install, lint, build, run -- `claude`: models (`cheap`, `default`, `strong`, `max`: by default the aliases `haiku`, `sonnet`, `opus` and `fable`, which Claude Code resolves to the newest model of each family; a full name such as `claude-opus-5-5` pins one), `timeout_minutes`, optional spending limits (`max_budget_usd` per session, `max_budget_usd_per_day` for the loop's sessions together; unset means no limit), and `allowed_tools` for build sessions (the configured commands are always added; anything else a build needs, like `"Bash(make *)"`, goes here) +- `claude`: models (`cheap`, `default`, `strong`, `max`: by default the aliases `haiku`, `sonnet`, `opus` and `fable`, which Claude Code resolves to the newest model of each family; a full name such as `claude-opus-5-5` pins one), `timeout_minutes`, optional spending limits (`max_budget_usd` per session, `max_budget_usd_per_day` for the loop's sessions together; unset means no limit), and `allowed_tools` for build sessions (the configured commands are always added; anything else a build needs, like `"Bash(make *)"`, goes here), and `mcp_tools`: MCP tools every session may use, judges included (`mcp__` for all of a server's tools, `mcp____` for one). Sessions load the MCP servers, skills and CLAUDE.md files of the machine and repository they run in, but without an allow rule an MCP call is refused (a session cannot ask). A server can still be connecting when a session starts, so its tools may only appear after the first turn - `gates`: spec, plan, merge, done, each with `human` (`status` or `none`; merge also `pr`) and `agent` (independent reviewer); merge also `method` (`squash` or `merge`) - `stages`: per-stage model, `steps` (a model for single steps; see Models for each step), skills, `before`/`after` commands, and loop limits - `speed`: work that runs at the same time to save waiting; only `cheap_judges: first` changes who decides (once earned, a cheaper judge's pass stands). `overlap_judges` (on) runs the first review alongside the verifier (in a copy of the tested commit), and lessons alongside the final check; when the verifier fails, the review running beside it is stopped at once and round 1 runs after the fix. With `claude.max_budget_usd_per_day` set, they overlap only when what is left of the day covers two whole sessions (`claude.max_budget_usd`), so never without a session limit. `parallel_checks` (off) starts test, lint and build together, and stops the rest once one fails; leave it off if they write the same folder. `plan_ahead` (on) writes the first plan while the spec is being checked. The plan is stopped if the spec does not pass, and it is used only if the code on `main` it read (outside `intent/` and `loopstra/config.yaml`) is still the same commit's code when planning starts. It follows the same daily budget rule, and does not run with a person on the spec gate, without a spec judge, with plan `before` commands, or while another change is about to merge. `cheap_judges` (off) asks a cheaper model (`claude.models.default`) the same question as the spec check, the plan challenge and the review: `shadow` only records whether it agreed with the strong judge, which still decides, and `first` lets it pass something alone once it has also caught the strong judge's last 30 rejections, sending anything it rejects or is unsure of to the strong judge (`loopstra judges` shows the record). In `first`, every 5th call still runs both judges as an audit, with the strong judge deciding, so a later miss sends it back to `shadow`. Like the overlaps, `shadow` and the audits run only with room for every session at once (three when the early review runs beside the verifier, or the spec check beside a plan written ahead), so never with a daily budget and no session limit; then the strong judge decides alone, and in `first` the cheaper judge never decides alone. `think_ahead` (on) runs design and plan steps beside the changes running a step (see How it works). `max_parallel_changes` (1, up to 4) is how many changes may each run a step at the same time. Raise it only if the project's commands (test, lint, build, install) can run twice at once on this machine: no fixed ports, no shared test database, no output outside the worktree. Each command, and each session that may run commands, gets `LOOPSTRA_SLUG` and `LOOPSTRA_SLOT`: a small number (0, 1, ...) that no other change running at the same time has, so it can pick its own ports, database and Docker project names (port `3100 + LOOPSTRA_SLOT`, say). Main's health check runs alone and gets no slot. More changes at once also spend the daily budget and the subscription's usage limits faster. With `claude.max_budget_usd_per_day` set, only as many run at once as what is left of the day covers whole sessions (`claude.max_budget_usd`), so one at a time without a session limit; each overlap above also counts the sessions of the other changes running. - `signals`: how often main's health check runs - `chat`: the orchestrator's model (`model`), the model of the write-up (`steps.write-intent`), optional spending limits (`max_budget_usd_per_day`, `max_budget_usd_per_session`), and the Slack and Discord bots under `transports` (see Chat) -`loopstra setup` asks about the commands, spending limits, gates, merging, chat and models (the four names, each stage's model, and, if you say yes, a model for each of its steps); edit the file by hand for the rest (`poll_seconds`, `allowed_tools`, stage skills and commands, speed, signals). +`loopstra setup` asks about the commands, spending limits, gates, merging, chat and models (the four names, each stage's model, and, if you say yes, a model for each of its steps); edit the file by hand for the rest (`poll_seconds`, `allowed_tools`, `mcp_tools`, stage skills and commands, speed, signals). During onboarding the `loopstra` skill lists the MCP tools its own session has, says which only read and which change things, and writes the ones you pick to `mcp_tools`. **Models for each step.** The four names are `cheap`, `default`, `strong` and `max`. Each stage has a model, and so does chat; a step can have its own under `stages..steps.` (or `chat.steps.write-intent`), set to one of the four names. A step with none of its own runs on its default below. Nothing is put on `max` by a default: only a person does that. diff --git a/docs/decisions.md b/docs/decisions.md index 515681d5..5a070d63 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -808,3 +808,9 @@ A record (a status, an artifact, a conversation entry, a person's edits, chat's - **One change at a time, with a version check.** A change is refused if the file changed since the page read it, or if `loopstra/config.yaml` has uncommitted edits in the main checkout. The uncommitted-edits check runs inside the record write, after a cut-short record is settled, so a kill does not cause a false refusal. If the loop is not running the change waits and can be cancelled. - **A bad model name is its own failure.** A name Claude Code does not accept fails the step with reason `model`: not an outage, not retried, with a note naming the setting and where to change it. In chat the same sentence is the reply. - **Resume across models.** `--resume` with another `--model` was checked on the real CLI (Claude Code 2.1.309): a session started on haiku and resumed with `--model sonnet` kept its session id and ran the turn on sonnet. So fix, reconcile and revise keep resuming the build's session when their model differs; the cost is that the first turn on the new model is not cached. The resumed session's reported cost included the earlier turn's cost, so a total of the two would count it twice. + +## MCP tools for every session (2026-10-26) + +- **`claude.mcp_tools`, allowed in every tool set.** Sessions load the user's and the repository's MCP servers (no `--strict-mcp-config`), but a `-p` session cannot ask, and no allow list had an `mcp__` rule, so every MCP call was refused outside plan mode (checked on the real CLI: refused in `default` and `acceptEdits`, allowed in `plan` for a tool marked read-only). The rules are added after each set's own tools, judges' included: a judge may need the same lookup (an issue, a doc) as the build. They are not limited to read-only tools; what an MCP tool may change is the owner's call when they list it. +- **Rules, not tool lists.** Each entry must start with `mcp__`, so the setting cannot widen a judge's shell or file access; `allowed_tools` stays the place for those, build sessions only. +- **The skill offers the MCP tools; setup does not ask.** `loopstra setup` cannot see an MCP server's tools without connecting to it (`claude mcp list` names servers only). The operator skill runs in a Claude Code session that already has them, so it lists them by server, says which only read and which change things, and writes the engineer's picks. None is picked by default. A project's `.mcp.json` servers load in `-p` sessions without approval (checked on the real CLI), so committing the file is enough for the loop to see them. diff --git a/src/config.ts b/src/config.ts index 6dd3c6c0..d14cd622 100644 --- a/src/config.ts +++ b/src/config.ts @@ -125,6 +125,11 @@ export const ConfigSchema = z.object({ max_budget_usd_per_day: z.number().positive().optional(), // Git that only reads: the runtime makes every commit, branch, and merge itself. allowed_tools: z.array(z.string()).default(["Read", "Edit", "Write", "Glob", "Grep", "Bash(bun *)", "Bash(git diff *)", "Bash(git log *)", "Bash(git show *)", "Bash(git status *)"]), + /** + * MCP allow rules (`mcp__` or `mcp____`) added to every session's allow list, + * judges' included. No commas or spaces: the list reaches the CLI joined with commas. + */ + mcp_tools: z.array(z.string().regex(/^mcp__[\w*-]+$/, "must be an MCP rule, like mcp__github or mcp__github__get_issue")).default([]), }).strict().prefault({}), // There is no intent gate to set: a person always accepts a change by setting its status to accepted. gates: z.object({ diff --git a/src/phases.ts b/src/phases.ts index 3b98eefa..f72cd2ee 100644 --- a/src/phases.ts +++ b/src/phases.ts @@ -26,7 +26,8 @@ import { toolTarget } from "./tools"; /** * read: look only. read+git: look, plus read-only git (the reviewer). read+commands: look, the * configured project commands except install, and read-only git (verify, done-check). build: - * `claude.allowed_tools` plus every configured project command, install included. + * `claude.allowed_tools` plus every configured project command, install included. Every set also + * gets `claude.mcp_tools`. */ type ToolSet = "read" | "read+git" | "read+commands" | "build"; @@ -96,8 +97,13 @@ function commandParts(cmd: string): string[] { return parts.map((p) => p.trim()).filter((p) => p); } -/** Tools a session may use without asking. Always a fresh array. */ +/** Tools a session may use without asking: the set's own, then `claude.mcp_tools`, each once. Always a fresh array. */ export function toolsFor(ctx: StepContext, set: ToolSet): string[] { + return [...new Set([...setTools(ctx, set), ...ctx.cfg.claude.mcp_tools])]; +} + +/** The tool set's own tools, before the MCP rules every set gets. */ +function setTools(ctx: StepContext, set: ToolSet): string[] { if (set === "read") return [...READ_TOOLS]; if (set === "read+git") return [...READ_TOOLS, ...GIT_READ]; const { test, lint, build, run, install } = ctx.cfg.commands; @@ -106,9 +112,9 @@ export function toolsFor(ctx: StepContext, set: ToolSet): string[] { const rules = (cmds: Array) => cmds.filter((c): c is string => !!c).flatMap((c) => [...new Set([c, ...commandParts(c)])]).map((c) => `Bash(${c} *)`); // A build session runs the project's own commands (tests, install) whatever the allow list says. - if (set === "build") return [...new Set([...ctx.cfg.claude.allowed_tools, ...rules([test, lint, build, run, install])])]; + if (set === "build") return [...ctx.cfg.claude.allowed_tools, ...rules([test, lint, build, run, install])]; // The install command is not for judges. - return [...new Set([...READ_TOOLS, ...rules([test, lint, build, run]), ...GIT_READ])]; + return [...READ_TOOLS, ...rules([test, lint, build, run]), ...GIT_READ]; } /** diff --git a/templates/config.yaml b/templates/config.yaml index 58325b84..84dccd86 100644 --- a/templates/config.yaml +++ b/templates/config.yaml @@ -18,6 +18,9 @@ claude: # What a build session may do without asking. The commands above (test, lint, build, run, install) # are always added. Git stays read-only: Loopstra makes every commit itself. The default: # allowed_tools: [Read, Edit, Write, Glob, Grep, "Bash(bun *)", "Bash(git diff *)", "Bash(git log *)", "Bash(git show *)", "Bash(git status *)"] + # MCP tools every session may use, read-only judges included (none by default; an MCP call + # without a rule here is refused). mcp__ allows all of a server's tools: + # mcp_tools: [mcp__github, "mcp__claude_ai_Claude_Docs__guide"] # Spending limits, in US dollars, are off unless set (`loopstra setup budgets` sets them in minutes or dollars): # max_budget_usd: what one session may spend # max_budget_usd_per_day: what the loop's sessions may spend together in a day diff --git a/templates/skill/SKILL.md b/templates/skill/SKILL.md index 1fa746af..8840c0d6 100644 --- a/templates/skill/SKILL.md +++ b/templates/skill/SKILL.md @@ -29,12 +29,13 @@ draft -> accepted -> designing -> spec-review -> spec-approved -> planning -> pl ## Onboard 1. Run `loopstra init` at the repository's top folder (in a folder below it, init names the top and writes nothing) and read what it printed. Files it reports as "kept" already existed and were left alone. The first steps are always in this order: setup, commit, start. If it says the repo was set up by an older Loopstra, have them run `loopstra upgrade` (see Upgrade). 2. Have them run `loopstra setup` in a terminal (`init` offers it at the end). It walks through budgets, commands, gates, GitHub, chat and models (Enter takes the suggestion; `-` leaves an optional answer empty, and a command left out that way stays out), saves `loopstra/config.yaml` keeping its comments, and checks what it can: `claude`, `commands.test` on main, the git remote and `gh`, the chat tokens. "To fix" names the `loopstra setup
` to run again. One section at a time: `loopstra setup budgets` (or `commands`, `gates`, `github`, `chat`, `models`; `loopstra setup --help` lists them); `loopstra setup --check` changes nothing and only checks. If it says "Fix these in loopstra/config.yaml first:", those problems are outside what its questions ask (an unknown key, for example): help them fix the file, then run it again. Your part is to help them decide, in their context, and tell them which section to run; do not edit `config.yaml` yourself for anything setup asks. - Build sessions may always run the configured commands; anything else they need (another tool, a script) goes in `claude.allowed_tools`, for example `"Bash(make *)"`. Setup does not ask about it, so that one is edited in the file. Refused commands show in the console and `loopstra tail`. + Build sessions may always run the configured commands; anything else they need (another tool, a script) goes in `claude.allowed_tools`, for example `"Bash(make *)"`. Setup does not ask about it, so that one is edited in the file; so is `claude.mcp_tools` (step 5). Refused commands show in the console and `loopstra tail`. 3. A person always accepts an intent (draft to accepted). The other gates (spec, plan, merge, done) are unattended by default; setup's `gates` and `github` sections change that. With a git remote every change goes through a pull request, so `gh` must be signed in. Budgets are off by default (no limit; `timeout_minutes` still ends a session); suggest limits only if they pay per use with an API key or want a ceiling. 4. Ask which skills in `.claude/skills/` each stage should load and list them under `stages..skills`. -5. Read `CLAUDE.md`; make sure its Commands block matches the config. -6. Commit what `init` wrote, with the config as setup left it and the edits above, on main: the loop works in its own checkouts, which only see what is committed, and `loopstra start` refuses until the config, `loopstra/stamp.json` and any customized prompt are committed. Commit the config again whenever setup changes it. -7. Tell them to start the loop in a terminal with `loopstra start` and to watch it with `loopstra status` or `loopstra ui`. Product owners can use `loopstra chat` or the console's orchestrator (Ctrl J) instead of writing intents by hand (see Chat). +5. Offer the MCP tools. The loop's sessions may call only the MCP tools in `claude.mcp_tools`; any other MCP call is refused. List every MCP tool your own session has, grouped by server: the ones you were given and the deferred ones (search with ToolSearch, for example the query `mcp__`). Run `claude mcp list` too, and name the servers that need sign-in or failed to connect, since you cannot see their tools. For each tool, say in a few words what it does and whether it only reads or also changes something (sends, creates, edits, deletes). Ask the engineer which ones the loop may use. Nothing is allowed unless they pick it, and none is fine. Say that every session gets the picks, read-only judges included, so any session can call a tool that sends or deletes. Write their choice under `claude:` in `loopstra/config.yaml` as `mcp_tools: [...]`, using the names as your tools show them: `mcp__` for all of a server's tools, `mcp____` for one. The loop's sessions get the MCP servers of the machine and user that run `loopstra start`, plus the repository's committed `.mcp.json`. If the loop will run on another machine or as another user, say that a server missing there is unavailable to the loop, whatever the config allows. +6. Read `CLAUDE.md`; make sure its Commands block matches the config. +7. Commit what `init` wrote, with the config as setup left it and the edits above, on main: the loop works in its own checkouts, which only see what is committed, and `loopstra start` refuses until the config, `loopstra/stamp.json` and any customized prompt are committed. Commit the config again whenever setup changes it. +8. Tell them to start the loop in a terminal with `loopstra start` and to watch it with `loopstra status` or `loopstra ui`. Product owners can use `loopstra chat` or the console's orchestrator (Ctrl J) instead of writing intents by hand (see Chat). ## Draft an intent Interview the person in plain language: what is wrong today and for whom, what should be true when it is done, how they would check it is done, who and what it touches, any constraints, and open questions. Write `intent//intent.md` from the template in `intent/README.md` with a short hyphenated slug they agree to. One intent is designed, planned, and built in one build session within the stage's time limit (and any spending limit), so a request the size of a whole product will block partway: split it into several intents, each something one person could build in a day or two, and give each later one a `depends_on: [, ...]` line so it waits until those are merged. Leave `status: draft`. Tell them to set `accepted` when they are ready. Do not design or plan anything. @@ -60,7 +61,7 @@ If the loop line says "Paused — " followed by "The assistant is unavailable", Read the `note` in the change's `intent.md`. Explain the choices: follow the status the note names (it says which status to set to try again, or the two ways on); `resume_from`, the last approved state, is the fallback when the note names none; fix something first and then retry; or `closed`. Make the edit only when the person says which. For details, read `.loopstra/runs//events.jsonl` or the phase folders under `.loopstra/runs//phases/`. ## Tune -Edit `loopstra/config.yaml`, or a phase's prompt. Prompts ship with Loopstra: to change one, have them run `loopstra customize ` (`loopstra customize` lists them), which copies it to `loopstra/prompts/.md` and records it in `loopstra/stamp.json`; edit that copy and commit both. Deleting the copy goes back to the shipped prompt. Changes take effect on the next pass; commit them. Keep prompts short and explicit; keep gate defaults deterministic. +Edit `loopstra/config.yaml`, or a phase's prompt. Prompts ship with Loopstra: to change one, have them run `loopstra customize ` (`loopstra customize` lists them), which copies it to `loopstra/prompts/.md` and records it in `loopstra/stamp.json`; edit that copy and commit both. Deleting the copy goes back to the shipped prompt. Changes take effect on the next pass; commit them. Keep prompts short and explicit; keep gate defaults deterministic. To change which MCP tools the loop may use, list them and ask again as in Onboard step 5. ## Upgrade `loopstra upgrade` installs the latest Loopstra and brings this repo up to it in one commit on main: the skill, `intent/README.md`, `REVIEW.md` and customized prompts are updated, or merged where the repo edited them (a conflict leaves markers in that file, out of the commit, for a person to resolve). In a repo set up by an older Loopstra it also drops unedited prompt copies and the old test hook; until it has run there, `loopstra start`, `chat` and `ui` refuse and say to run it. Have them stop the loop first and run it on main with Loopstra's files committed; it refuses otherwise and says why. `loopstra upgrade --check` changes nothing and says what would happen. `loopstra status` says when an upgrade is due. diff --git a/tests/unit/config.test.ts b/tests/unit/config.test.ts index ed61f97e..c4f0f172 100644 --- a/tests/unit/config.test.ts +++ b/tests/unit/config.test.ts @@ -20,6 +20,7 @@ describe("loadConfig", () => { expect(cfg.gates.spec.agent).toBe(true); // Build sessions may read git history but not change it: the runtime owns commits and branches. expect(cfg.claude.allowed_tools).toEqual(["Read", "Edit", "Write", "Glob", "Grep", "Bash(bun *)", "Bash(git diff *)", "Bash(git log *)", "Bash(git show *)", "Bash(git status *)"]); + expect(cfg.claude.mcp_tools).toEqual([]); expect(cfg.stages.build.max_fix_loops).toBe(3); expect(cfg.claude.models.cheap).toBe("haiku"); // The cheap model often ends a verify session without calling the structured-output tool (#7). @@ -142,6 +143,12 @@ describe("loadConfig", () => { "commands.run: must be a command, or the line removed", "commands.install: must be a command, or the line removed", ]); expect(problems({ commands: { test: "bun test" } })).toEqual(["version: must be 1"]); + expect(problems({ ...ok, claude: { mcp_tools: ["mcp__github", "mcp__docs__*", "Bash(rm *)", "mcp__", "mcp__x,Bash"] } })).toEqual([ + "claude.mcp_tools.2: must be an MCP rule, like mcp__github or mcp__github__get_issue", + "claude.mcp_tools.3: must be an MCP rule, like mcp__github or mcp__github__get_issue", + // The list reaches the CLI joined with commas: this one would allow Bash. + "claude.mcp_tools.4: must be an MCP rule, like mcp__github or mcp__github__get_issue", + ]); expect(problems({ ...ok, version: 2 })).toEqual(["version: must be 1"]); const id = "must be a user or channel id; put long numeric ids in quotes"; expect(problems({ ...ok, chat: { transports: { slack: {} } } })).toEqual([`chat.transports.slack.channel: ${id}`]); diff --git a/tests/unit/phases.test.ts b/tests/unit/phases.test.ts index b054919d..cd748d07 100644 --- a/tests/unit/phases.test.ts +++ b/tests/unit/phases.test.ts @@ -174,6 +174,24 @@ describe("agentPhase tools and prompt", () => { trace.close(); repo.cleanup(); }); + test("claude.mcp_tools are allowed in every tool set, judges' included, once each", async () => { + const { repo, ctx, trace } = await setup(); + ctx.cfg.claude.mcp_tools = ["mcp__github", "mcp__docs__guide", "mcp__github"]; + for (const set of ["read", "read+git", "read+commands", "build"] as const) { + const tools = toolsFor(ctx, set); + expect(tools.slice(-2)).toEqual(["mcp__github", "mcp__docs__guide"]); + expect(new Set(tools).size).toBe(tools.length); + } + // MCP rules are not shell commands. + expect(commandsFor(ctx, "read")).toBe("none"); + + const argsFile = join(repo.path, "args.json"); + await agentPhase(ctx, { name: "intake", model: "cheap", permissionMode: "default", tools: "read", vars: {}, env: { LOOPSTRA_FAKE_ARGS: argsFile } }); + const recorded = await Bun.file(argsFile).json(); + expect(recorded.args).toEqual(expect.arrayContaining(["--allowedTools", "Read,Glob,Grep,mcp__github,mcp__docs__guide"])); + trace.close(); repo.cleanup(); + }); + test("a chained command also allows each of its parts, since Claude Code checks each part on its own", async () => { const { repo, ctx, trace } = await setup(); ctx.cfg.commands.test = "dotnet test api/App.sln && npm --prefix web test || echo failed; npm --prefix e2e test"; From 627a60425483297641c6f1dc06cc316688acc4be Mon Sep 17 00:00:00 2001 From: John Flavan Date: Mon, 26 Oct 2026 22:10:19 -0500 Subject: [PATCH 02/64] docs: design for agent setup per step Co-Authored-By: Claude Opus 5.5 --- .../specs/2026-10-26-agent-setup-design.md | 214 ++++++++++++++++++ 1 file changed, 214 insertions(+) create mode 100644 docs/superpowers/specs/2026-10-26-agent-setup-design.md diff --git a/docs/superpowers/specs/2026-10-26-agent-setup-design.md b/docs/superpowers/specs/2026-10-26-agent-setup-design.md new file mode 100644 index 00000000..4160647c --- /dev/null +++ b/docs/superpowers/specs/2026-10-26-agent-setup-design.md @@ -0,0 +1,214 @@ +# Agent setup for every step + +## Why + +Every step of the loop (intake, design, spec-check, plan, plan-challenge, build, reconcile, fix, review, revise, verify, done-check, lessons, and chat's orchestrator and write-intent) runs as one plain Claude Code session. Per stage a person can set a model, a "Use these skills" line, and before/after commands; per step, a model. That is a good baseline, but projects and teams differ: one wants its review done by three specialist reviewers, another wants its build session to play a house "senior engineer" role with the team's own skills, a third wants the loop to run the same agents on every machine regardless of who starts it. + +The owner's words: "this tool should allow for a high degree of flexibility to accommodate different projects and teams. What's going on right now is a good baseline." The goal is flexibility, not one prescribed way of working. + +## Goals + +- For each step, an engineer can set: + - **the agent** the session plays (a role: a system prompt, and optionally a narrower tool list); + - **the subagents** it may hand work to, one by one or as named **teams**; + - **the skills** it uses; + - **extra tools** and **MCP tools** on top of the step's baseline. +- Settings layer: one block for every step, then each step's own. A step with nothing set behaves exactly as today. +- Engineers can choose whether loop sessions also load the personal setup (skills, agents, plugins, MCP servers, CLAUDE.md) of whoever runs `loopstra start`, or only what the repository declares. +- Agents, teams and skills can be set up in three places: `loopstra setup`, the loopstra operator skill during onboarding, and a new configuration screen in the console. +- A skill or agent the repository does not have yet can be added there: copied from the person's machine or a plugin, fetched from a git repository or folder, or (through the operator skill) written new. +- Everything stays human readable: agent and skill definitions are Claude Code's own files in the repository; `loopstra/config.yaml` only names which ones each step uses. + +## Non-goals + +- Claude Code's experimental agent teams (peers with their own sessions messaging each other). In CLI 2.1.311 the Agent tool's `team_name` is deprecated and ignored; no team tools exist. A loopstra team is a named group of subagents under the step's session. Revisit if Claude Code ships teams again. +- Changing what a step is for, its prompt contract, or its structured output. Prompts stay customizable as today (`loopstra customize`). +- A marketplace or registry of loopstra's own. +- Per-change agent setups (an intent choosing its own agents). Settings are per repository. + +## Anti-goals (a failure even if it works) + +- **Weakening the loop's safety.** The runtime alone commits, branches and merges. Read-only steps (intake, design, spec-check, plan, plan-challenge, review, verify, done-check, lessons, and the chat steps) never get write tools, whatever an agent, subagent, team or tools list says. Checked on the real CLI: tools in `--disallowedTools` are also refused to the session's subagents and to a session run with `--agent`. +- **Breaking the structured output.** Every step must still return its envelope. +- **Hidden behaviour.** What a step ran as (agent, subagents, skills, tools) must be visible in the trace and the console, like its model is today. +- **Making the simple case harder.** A repository that sets nothing sees no new questions it must answer, no new required files, and no change in behaviour. + +## What the CLI does (checked on Claude Code 2.1.311, `-p`, stream-json) + +| Fact | Consequence | +|---|---| +| The repo's `.claude/agents/*.md` and `.claude/skills/*/SKILL.md` load in `-p` sessions; the `init` event lists `agents` and `skills`. | Definitions can live in the repo; the init event tells what a session actually saw. | +| Subagents run through the Task/Agent tool without any allow rule. | Delegation needs no new permission plumbing. | +| `--agents ''` adds inline agent definitions for one session. | Loopstra can hand a session agents built from config or adjusted files. | +| `--agent ` runs the main session as that agent; the session's `--model` wins over the agent's `model:`. | Model choice stays loopstra's (one source of truth, already shown in the console). | +| An agent whose `tools:` list is narrow (e.g. `Read`) loses the structured-output tool: the session ends with `structured_output: null`. | Loopstra must not use `--agent` with a definition's own tool list as-is. See "Running a step as an agent". | +| `--disallowedTools` applies to subagents and to `--agent`: a judge's subagent with `Write` in its definition could not write. | The safety guard holds without extra work. | +| `--setting-sources project` drops user skills, agents and plugins (106 skills down to 20, user agents gone); project skills and agents still load. | "Repo only" is one flag. | +| Project `.mcp.json` servers load in `-p` without approval. | Committing `.mcp.json` is enough for repo-declared MCP servers. | +| `--forward-subagent-text` forwards subagents' text and thinking into the stream. | The console can show what subagents do. | + +## The settings + +A new top-level `agents` block in `loopstra/config.yaml`. Everything in it is optional. + +```yaml +agents: + # all: the personal setup of whoever runs `loopstra start` plus the repo's (today's behaviour). + # repo: only what the repository declares (its .claude/, .mcp.json, CLAUDE.md), the same on every machine. + sources: all + + # Named groups of subagents. `parallel`: each member gets the same task and the session weighs + # their answers. `pick`: the session hands each piece of work to the member that fits it. + teams: + reviewers: + members: [security-reviewer, performance-reviewer, correctness-reviewer] + way: parallel + + # Applies to every step, then each step's own settings are added on top. + all: + skills: [house-style] + + steps: + review: + agent: lead-reviewer # .claude/agents/lead-reviewer.md + teams: [reviewers] + subagents: [Explore] # single subagents, by name + skills: [threat-modeling] + tools: ["WebFetch"] + mcp_tools: [mcp__github__get_pull_request] + build: + agent: senior-engineer + subagents: [test-writer] +``` + +Rules: + +- **Keys under `steps`** are the step keys of `STEPS` in `src/models.ts` (all fifteen, including `orchestrator` and `write-intent`). An unknown key is a config error naming the known ones. The chat steps follow their own rules (see "Chat steps"). +- **Layering:** `all`, then the existing `stages..skills` exactly where they reach today (design and plan writing, build, both reviews, verify, done-check, lessons; not intake, spec-check, plan-challenge, reconcile, fix or revise), then `steps.`. Lists (`skills`, `subagents`, `teams`, `tools`, `mcp_tools`) add up, each name once, in that order. `agent` is a single value: the step's own wins over `all`. +- **`claude.mcp_tools`** (added on branch `feat/mcp-tools`) stays and means the same thing as `agents.all.mcp_tools`. *Builder's call:* keep both as aliases, or move `claude.mcp_tools` into `agents.all.mcp_tools` with an upgrade migration before that branch is released. Prefer the migration if the branch has not shipped in a release. +- **`tools`** are Claude Code allow rules, as in `claude.allowed_tools`. On a read-only step, `Edit`, `Write`, `NotebookEdit` and a bare `Bash` (or `Bash(*)`) are config errors that name the step and say why; narrower `Bash( *)` rules are allowed, as judges already run the configured commands. Write tools stay removed from read-only steps by `--disallowedTools` regardless. +- **`mcp_tools`** entries follow the existing rule (`mcp__` plus letters, digits, `_`, `-`, `*`; no commas). +- **Names** of agents, subagents and skills are what Claude Code shows: `lead-reviewer`, a plugin's `plugin:name`, built-ins like `Explore`. Team names are loopstra's own: lowercase words joined by dashes. A team name that is also an agent name is an error. +- **Defaults:** with no `agents` block, or an empty one, every session is started exactly as today (same flags, same prompt). This must be pinned by a test. + +## What a step's session gets + +For a step with settings, `agentPhase` (src/phases.ts) adds: + +1. **Allow list:** the step's tool set as today (`toolsFor`), plus the layered `tools` and `mcp_tools`, each once. `disallowedFor` is unchanged. +2. **Setting sources:** with `sources: repo`, `--setting-sources project` (not `local`: `.claude/settings.local.json` is personal). Loopstra's own `--settings` (the test-protection hook) still applies, since `--settings` is not a setting source. *Builder verifies* with the real CLI whether user MCP servers also drop out under `--setting-sources project`; if not, add `--strict-mcp-config --mcp-config .mcp.json` (when the file exists) for `repo`. +3. **The agent (role):** see the next section. +4. **Prompt lines,** before the prompt template, replacing today's single skills line: + - `Use these skills: \`a\`, \`b\`.` (as today); + - `Hand work to these subagents when it helps: \`x\` (its description), ...`; + - per team: `For , use the team \`reviewers\`: give each of \`a\`, \`b\`, \`c\` the same task at the same time, then weigh their answers yourself.` (or, for `pick`, `hand each piece of work to whichever of ... fits it`). The wording is the builder's; it must say which agents, how to use them, and that the session still returns its own structured output. +5. **Subagent visibility:** `--forward-subagent-text` when the step has subagents or teams, so the console and trace show their work. + +A step without agent settings gets none of these, and today's skills line is produced exactly as now. + +## Running a step as an agent + +`--agent ` cannot be passed as is: a definition with its own `tools:` list removes the structured-output tool (checked), and its `model:` would be shown nowhere. Instead loopstra reads the definition and starts the session with an inline copy: + +- Find the definition on main, never on the change's branch (see "Settled after review", 2): the step's snapshot of main first, then, with `sources: all`, `~/.claude/agents/.md`. A plugin's agent (`plugin:name`) is found through the init event of an earlier session or by the builder's lookup of installed plugins; *builder's call* how, but a plugin agent must work with `sources: all`. +- Build `--agents '{"": {...}}'` from it: its description and prompt as written; its `tools`, when it lists any, with the structured-output tool added and intersected with the step's allow list (a definition can narrow a step's tools, never widen them); no `model` (the step's model applies). Then pass `--agent `. +- If the definition cannot be found, the step does not start: it fails with reason `setup` (a new failure reason: not an outage, not retried), and the owner note names the step, the missing agent, and where to add it. The same for a missing skill or subagent (see Checks). +- *Builder verifies* on the real CLI that an inline agent's `prompt` and the session's `--json-schema` together still produce `structured_output`, and that an inline definition with the same name as a file definition takes precedence. If inline-over-file precedence does not hold, give the inline copy a loopstra-specific name (`loopstra-`). + +## Checks + +- **When the config loads:** shape only (names, step keys, tools rules, team members not empty, no team named like an agent). +- **`loopstra setup --check`, `loopstra start`, and the console's agent screen** check that every named agent, subagent, team member and skill exists: in the repo's `.claude/` (committed, since the loop works from commits), or, with `sources: all`, in the user's `~/.claude/` or an installed plugin. Missing ones are listed with the step that names them. `start` refuses to start while a name used by any step is missing, with the list and the fix (`loopstra setup agents`, or commit the file). A name found only on this machine (personal) gets a warning, not a refusal: it will not exist on another machine. +- **At session start:** the `init` event lists the agents and skills the session actually loaded. If a named one is missing there, loopstra ends the session at once (before it spends anything meaningful) and fails the step with reason `setup`, as above. This catches what the file check cannot (a plugin that failed to load). + +## Getting skills and agents + +Two kinds of things: **definitions in the repo** (`.claude/agents/.md`, `.claude/skills//`), and **the step settings** that name them. Adding a skill or agent always ends with its files in the repo's `.claude/`, committed, so every machine and every loop checkout has it. Sources: + +1. **This machine:** a personal skill or agent (`~/.claude/skills//`, `~/.claude/agents/.md`) or one from an installed plugin, copied into the repo. The copy is the whole skill folder (scripts and references included). +2. **A git repository or a local folder:** the person gives a URL (and optionally a path inside it) or a folder; loopstra clones it shallowly into a temporary folder, lists the skills and agents it finds (any `SKILL.md`, any `agents/*.md`), and copies the ones picked. The source is recorded in the copied skill folder (a line in a `SOURCE.md`, or in the agent file's frontmatter as `source:`; *builder's call* which, as long as it is human readable) so a later update can find it. +3. **New:** a new agent from a short form (name, description, role text, tools, optional subagents of its own). A new skill only through the operator skill (step 4 of its Onboard, below), which can write one with the person; the CLI and console offer an empty skill (name, description, body) as a plain file at most. + +Downloaded content is untrusted: copies go through a temporary folder, nothing in them runs, and the person sees the files' list before anything is written to the repo. + +*Non-goal for now:* installing Claude Code plugins at project scope (`enabledPlugins` in `.claude/settings.json`). Copying is simpler, reviewable in the diff, and works with `sources: repo`. A plugin's skills and agents can still be used with `sources: all` on machines that have it. + +## Setup (`loopstra setup agents`) + +A new section, listed with the others and runnable alone (`loopstra setup agents`). It does not run in the first full walk-through unless the person says yes to "Set up agents for the loop's steps now? (y/N)", so the simple case gets one extra question at most. + +1. **Sources:** all (default, today's behaviour) or repo only, with one line on what each means. +2. **What is available:** lists the agents and skills found (repo, personal, plugins), marking which are in the repo. +3. **Per step or for all steps:** the person picks a step (or "all steps") from the step table, then sets its agent, subagents, teams and skills by picking from the list (numbers or names; `-` clears), and tools and MCP tools as text. Teams are made or edited in the same flow (name, members, way). +4. **Adding:** at any pick, "add one" offers the sources above (copy from this machine, from a git repository or folder, or a new agent from the short form). +5. Saves the config keeping its comments (as setup does today), writes the copied or new files, then runs the checks and lists anything missing. It does not commit; like the rest of setup, it says to commit `loopstra/config.yaml` and `.claude/`. + +The `setup` prompt style, `-` for empty, Enter for the suggestion, and per-section re-runs stay as they are. + +## The operator skill (onboarding) + +`templates/skill/SKILL.md` gets an Onboard step after the MCP step: "Set up the agents." The agent: + +- lists the agents and skills its own session has (it sees them, and the repo's `.claude/`), and the steps with what each does; +- asks what the team wants from the loop in their words, and suggests a setup (for example a team of reviewers, or a role for the build), always saying what each choice costs (subagents and teams spend more per step); +- adds missing skills or agents: copies, fetches, or writes new ones with the person (it can author a skill; the CLI cannot); +- writes the `agents` block and the files, then has them run `loopstra setup --check` and commit. + +The Tune section says the same flow is how to change agents later. + +## The console: an Agents settings screen + +The existing `/agents` page shows live sessions; this is configuration, so it sits in Settings as a second section next to the models, titled "Agents". + +It shows: + +- **Sources** (all / repo only), switchable. +- **The step table**, the same rows as the models: for each step, its agent, subagents, teams, skills, extra tools and MCP tools, each shown as set on the step, from `all`, or from the stage's skills, so the person sees where a value comes from (like the models page shows "follows design"). Editing a row changes `agents.steps.`; an "All steps" row edits `agents.all`. +- **Teams:** list, create, edit (members, way), delete (refused while a step uses it, naming the steps). +- **Library:** the agents and skills available, grouped as In the repo / On this machine / From plugins, each with its description and the steps that use it. Actions: "Add to the repo" (copy), "Add from a git repository or folder", "New agent" (the short form), "Edit" for a repo agent (its frontmatter and role text), and "Remove from the repo" (refused while a step uses it). +- **Problems:** the check's result (missing names, personal-only names), at the top, with the fix. + +Writing: the console never writes the repository itself. As with the models page, each change is a request in `.loopstra/requests/` that the loop applies and commits: + +- config changes extend the existing `settings` request (path and value with the file's version) to the `agents.*` paths; list values are sent whole (the new list), not as edits; +- file changes are a new `agent-files` request: the files to write or remove under `.claude/agents/` and `.claude/skills/` (paths checked to stay inside those folders), with the version of each file read (or "absent"); refused if any changed since. Fetching from a git repository happens in the console server into a temporary folder and is shown before the request is sent. +- The loop commits them like other settings changes: `loopstra(_settings): agents.steps.review.teams → [reviewers] by from the dashboard`, and `loopstra(_settings): add .claude/agents/lead-reviewer.md by from the dashboard`. + +Settings are read fresh per session (as models are since step models), so a change applies to sessions that start after it. + +## Observability + +- The trace's phase start records the step's effective agent setup (agent, subagents, teams, skills, tools, mcp_tools, sources), like the model. +- The session page in the console shows it in the session's facts, and shows subagents' work (from `--forward-subagent-text` and the Task tool events, which `src/tools.ts` already names "Handing off") nested under the call that started it. +- `loopstra status` and `tail` show nothing new unless a step failed with `setup`, whose note says what is missing. +- Cost: subagents' cost is part of a session's cost. *Builder verifies* that `total_cost_usd` includes subagents in `-p` (spike: run a step with a team and compare with the sum of usage). Budget caps apply to the whole session as today. + +## Constraints + +- Bun, TypeScript, Zod config, comment-preserving YAML edits, Preact console: as the codebase uses. +- Windows, macOS and Linux (paths to `~/.claude`, CRLF in agent files). +- `bun run check` green; new behaviour has tests with the fake CLI (`LOOPSTRA_FAKE_ARGS` records flags); the CLI facts above that the builder must verify get a short live check recorded in `docs/decisions.md`. +- `docs/decisions.md` gets a section for this feature; the README's config section and the setup section describe the new block and screen. + +## Settled after review + +A builder's read of this document raised these; each is decided here. + +1. **Chat steps** (`orchestrator`, `write-intent`) run through `runChatAgent` (src/chat/agents.ts), in the main checkout, steered by people in Slack and Discord channels, with their own fixed allow and deny lists and secrets removed from the environment. So: + - `agents.all` does not reach them, and neither does `claude.mcp_tools` (it does not today either). + - On their own step entries they take `agent`, `skills`, `subagents` and `teams`. Their own `tools` and `mcp_tools` are allowed only on those entries, never inherited, and setup and the console say in plain words that anyone allowed to chat can steer those tools. `CHAT_DENIED` and the environment scrubbing always apply on top. + - A missing name in a chat turn does not block anything: the turn answers that the chat agent's setup is broken, names what is missing, and is traced under `_chat`. +2. **Definitions come from main.** The loop's settings are main's, so are the definitions: loopstra reads every agent it names (the step's agent, its subagents, team members) from the step's snapshot of main (or the user's folder or plugins with `sources: all`) and passes them inline with `--agents`, under a loopstra name (`loopstra-`) so a same-named file in a change's worktree never wins. A change's branch cannot rewrite the role that reviews it. Skills are loaded by Claude Code from the working directory, so for steps in a worktree the builder must make main's skills the ones used: *verify* whether `--add-dir ` exposes its `.claude/skills`; if not, fail the step with `setup` when a named skill is missing from the worktree or differs from main's, with a note to reconcile. Either way a change editing `.claude/` already counts as drift unless its plan lists those files. +3. **Subagents' models and other frontmatter.** A subagent or team member keeps its own `model:` when its file sets one (a team mixing cheap and strong members is a feature); without one it runs on the step's model. Every subagent's model is recorded in the trace and shown in the console with the step's. Inline copies carry only `description`, `prompt`, `tools` (intersected with the step's allow list), `model` and `skills`; `permissionMode`, `hooks` and `mcpServers` are dropped, and the checks warn that they were dropped, naming the file. +4. **What the safety line covers.** On every step, `tools` may not hold git commands that change history or remotes (`git commit`, `push`, `merge`, `rebase`, `reset`, `checkout`, `switch`, `branch`, `tag`, `stash`, a bare `Bash(git *)`): the runtime owns those. On read-only steps, also no `Edit`, `Write`, `NotebookEdit`, bare `Bash` or `Bash(*)`. Narrower `Bash( *)` rules and MCP tools stay the owner's call (as `claude.mcp_tools` already is), and setup, the console and the operator skill say which picks can change things. +5. **Fix, reconcile and revise resume the build's session**, so they run as the build's agent: `agent` is a config error on those three steps (the message says they continue the build session). Their other lists (skills, subagents, teams, tools, mcp_tools) layer as for any step, since each send passes its own flags; the inline agents are passed again on every resume. +6. **Stage skills** keep reaching exactly the steps they reach today (Layering, above), so a config with only `sources: repo` changes no prompt. +7. **Sign-in under `sources: repo`.** *Builder verifies* whether `--setting-sources project` drops a user's `apiKeyHelper` or `env` from `~/.claude/settings.json`. If it does, loopstra passes those two through its own `--settings`, so sessions still sign in. + +## Left to the builder + +- Module layout (a new `src/agents.ts` for layering, lookup and the inline definition is the obvious home), exact prompt wording, and the console layout within the Settings page's existing look. +- How plugin agents and skills are found on disk (or read from a recent init event). +- The `claude.mcp_tools` alias or migration (above). +- Whether `parallel`/`pick` need a third way; keep to these two unless a test shows one is missing. +- Splitting the work into PRs. A suggested order: (1) config, layering, session flags and prompt lines, checks, trace; (2) running a step as an agent; (3) setup section and adding skills/agents; (4) console screen and requests; (5) operator skill and docs. From 05138d5ee696b8fd57c3b21de4aefbc4015cefc3 Mon Sep 17 00:00:00 2001 From: John Flavan Date: Mon, 26 Oct 2026 22:15:36 -0500 Subject: [PATCH 03/64] docs: plan for agent setup, part 1 (runtime) Co-Authored-By: Claude Opus 5.5 --- .../plans/2026-10-26-agent-setup-1-runtime.md | 546 ++++++++++++++++++ 1 file changed, 546 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-26-agent-setup-1-runtime.md diff --git a/docs/superpowers/plans/2026-10-26-agent-setup-1-runtime.md b/docs/superpowers/plans/2026-10-26-agent-setup-1-runtime.md new file mode 100644 index 00000000..1e74ad0c --- /dev/null +++ b/docs/superpowers/plans/2026-10-26-agent-setup-1-runtime.md @@ -0,0 +1,546 @@ +# Agent Setup, Part 1: Runtime Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Each loop and chat step can run as a named agent, with its own subagents, teams, skills, tools and MCP tools, as set in a new `agents` block of `loopstra/config.yaml`; with nothing set, sessions start exactly as today. + +**Architecture:** A pure layer (`src/agents.ts`) turns the config into one `AgentSetup` per step and into prompt lines. A disk layer (`src/agent-files.ts`) finds agent and skill definitions (main's committed tree, the user's `~/.claude`, installed plugins), parses agent files, builds the inline `--agents` JSON, and checks that every named definition exists. `runPhase` gains the flags and an init-event check that ends a session which did not load what it was told to, with the new failure reason `setup`. `agentPhase` and `runChatAgent` wire it in. + +**Tech Stack:** Bun, TypeScript, Zod 4, `yaml`, bun:test with the fake CLI (`tests/fake-claude/claude.ts`, `LOOPSTRA_FAKE_ARGS`). + +**Spec:** `docs/superpowers/specs/2026-10-26-agent-setup-design.md` (read its "Settled after review" first). Parts 2-4 (setup section, console screen, operator skill and docs) get their own plans. + +## Global Constraints + +- With no `agents` block, or an empty one, every session's argv and prompt are byte-identical to today's (pinned by a test). +- The runtime alone commits, branches and merges; read-only steps never get `Edit`, `Write`, `NotebookEdit`, bare `Bash` or `Bash(*)`; no step's `tools` may hold history- or remote-changing git (`commit`, `push`, `merge`, `rebase`, `reset`, `checkout`, `switch`, `branch`, `tag`, `stash`, bare `git`). +- `mcp_tools` entries match `/^mcp__[\w*-]+$/` (no commas: the allow list reaches the CLI comma-joined). +- Inline agents are named `loopstra-` with `:` replaced by `-` (`superpowers:code-reviewer` → `loopstra-superpowers-code-reviewer`). +- A main agent's inline `tools`, when its file lists any, always include `StructuredOutput` (checked on CLI 2.1.311: without it `structured_output` is null). +- `agents.all` and today's `claude.mcp_tools` never reach chat steps; chat steps keep `CHAT_DENIED` and `dropEnv`. +- `agent` is a config error on `fix`, `reconcile`, `revise` (they resume the build's session and run as the build's agent). +- Stage skills (`stages..skills`) keep reaching exactly the steps they reach today (callers pass them as `spec.skills`, unchanged). +- Windows, macOS, Linux: paths via `node:path`, home via `os.homedir()` (tests pass their own), CRLF agent files parse. +- Comments and naming follow the codebase: plain sentences, `/** */` on exports. + +## CLI facts this plan relies on (checked on Claude Code 2.1.311, `-p`, stream-json) + +- `--agents ''` + `--agent ` run the main session as an inline agent; `--model` wins over the agent's `model`. +- `--disallowedTools` also binds subagents and `--agent`. +- `--add-dir ` loads that dir's `.claude/skills`; when the working dir has a skill of the same name, the working dir's wins (the other shows as `:`). +- `--setting-sources project` drops user skills, agents, plugins and user MCP servers; project ones load. +- `total_cost_usd` includes subagents (verified: haiku main + sonnet subagent, total = sum of `modelUsage`). +- The `system`/`init` event carries `agents: string[]` and `skills: string[]`; the fake CLI's fixtures carry neither (treat absent as "cannot tell"). + +## Review Focus + +1. A config with only `sources: all` or `agents: {}` → argv and prompt unchanged for every step. (Task 6.) +2. An agent file with CRLF line endings, no frontmatter, or frontmatter without `tools` → parsed; no `tools` key in the inline copy, so the agent gets the session's tools. (Task 3.) +3. A named skill that exists only as a Claude Code built-in or claude.ai-synced skill (not on disk) → the file check warns, does not refuse; the init check decides. (Task 4.) +4. A plugin agent named `plugin:name` whose plugin is installed but disabled in `enabledPlugins` → "missing", with the fix in words. (Task 3.) +5. A worktree step naming a skill only main has → `--add-dir ` supplies it; the session's init list has it under its plain name. (Task 6.) + +--- + +## File structure + +- Create `src/agents.ts`: config types and checks for the `agents` block helpers, `resolveSetup`, `setupLines`, `isEmptySetup`, `inlineName`. Pure. +- Create `src/agent-files.ts`: `parseAgentFile`, `findAgent`, `findSkill`, `inlineAgents`, `checkAgentSetup`, `BUILTIN_AGENTS`, `userSettingsPassthrough`. Reads disk. +- Modify `src/config.ts`: the `agents` block; drop `claude.mcp_tools` (moved to `agents.all.mcp_tools`; never released). +- Modify `src/models.ts`: `withModels` also carries `agents`; `ModelSettings` gains `agents`. +- Modify `src/claude.ts`: `RunPhaseInput` flags, `checkInit`, `FailureReason` `"setup"`. +- Modify `src/sessions.ts`: nothing unless `"setup"` needs a branch (it does not: it is a plain failure). +- Modify `src/phases.ts`: `toolsFor` drops `claude.mcp_tools`; `phasePrompt` uses `setupLines`; `attempt` passes the setup; `ownerNote("setup")`. +- Modify `src/chat/agents.ts`, `src/chat/orchestrator.ts`: chat wiring and the reply for `setup`. +- Modify `src/scheduler.ts`: `start` refuses on missing agents. +- Modify `templates/config.yaml`, `templates/skill/SKILL.md` (step 5 writes `agents.all.mcp_tools`), `README.md`, `docs/decisions.md`. +- Tests: `tests/unit/agents.test.ts`, `tests/unit/agent-files.test.ts` (new); `tests/unit/config.test.ts`, `tests/unit/phases.test.ts`, `tests/unit/claude-run.test.ts`, `tests/unit/chat-*.test.ts` (whichever covers `runChatAgent`), `tests/unit/scheduler*.test.ts` (whichever covers `start`). + +--- + +### Task 1: The `agents` config block + +**Files:** +- Modify: `src/config.ts` (the `claude` object, around `mcp_tools`; add `agents` to `ConfigSchema`) +- Create: `src/agents.ts` (types and the step-key list only, so config can import them) +- Test: `tests/unit/config.test.ts` + +**Interfaces:** +- Produces in `src/agents.ts`: + - `export const TEAM_WAYS = ["parallel", "pick"] as const; export type TeamWay = (typeof TEAM_WAYS)[number];` + - `export const READ_ONLY_STEPS: ReadonlySet`: every step except `build`, `reconcile`, `fix`, `revise`. + - `export const CONTINUES_BUILD: ReadonlySet = new Set(["fix", "reconcile", "revise"]);` + - `export const CHAT_STEPS: ReadonlySet = new Set(["orchestrator", "write-intent"]);` + - `export function toolProblem(rule: string, readOnly: boolean): string | null`: the reason a `tools` rule is refused, or null. +- Produces in `src/config.ts`: `Config["agents"]` shaped + `{ sources: "all" | "repo"; teams: Record; all: StepAgents; steps: Partial> }` where + `StepAgents = { agent?: string; subagents: string[]; teams: string[]; skills: string[]; tools: string[]; mcp_tools: string[] }` (lists default `[]`). Export `type StepAgents`. +- `claude.mcp_tools` is removed. + +- [ ] **Step 1: Write the failing tests** in `tests/unit/config.test.ts` + +```ts +test("the agents block defaults to nothing set", async () => { + const cfg = validateConfig({ version: 1, commands: { test: "bun test" } }); + expect(cfg.agents).toEqual({ sources: "all", teams: {}, all: { subagents: [], teams: [], skills: [], tools: [], mcp_tools: [] }, steps: {} }); + expect("mcp_tools" in cfg.claude).toBe(false); +}); + +test("agents settings are refused in plain words", () => { + const ok = { version: 1, commands: { test: "bun test" } }; + const problems = (agents: unknown) => { try { validateConfig({ ...ok, agents }); return []; } catch (e) { return (e as ConfigError).problems.map((p) => p.text); } }; + expect(problems({ steps: { reviews: {} } })[0]).toStartWith("agents.steps: unknown step reviews; the steps are intake, design,"); + expect(problems({ steps: { fix: { agent: "x" } } })).toEqual(["agents.steps.fix.agent: fix continues the build session and runs as the build's agent; set the agent on build"]); + expect(problems({ steps: { review: { tools: ["Edit"] } } })).toEqual(["agents.steps.review.tools.0: review is read-only; Edit would let it change files"]); + expect(problems({ steps: { review: { tools: ["Bash"] } } })).toEqual(["agents.steps.review.tools.0: review is read-only; a bare Bash rule would let it run anything"]); + expect(problems({ steps: { build: { tools: ["Bash(git push *)"] } } })).toEqual(["agents.steps.build.tools.0: only Loopstra commits, branches, merges and pushes; git push is not allowed"]); + expect(problems({ all: { tools: ["Write"] } })).toEqual(["agents.all.tools.0: these reach read-only steps too; Write would let them change files"]); + expect(problems({ all: { mcp_tools: ["mcp__x,Bash"] } })).toEqual(["agents.all.mcp_tools.0: must be an MCP rule, like mcp__github or mcp__github__get_issue"]); + expect(problems({ teams: { Reviewers: { members: ["a"] } } })).toEqual(["agents.teams.Reviewers: a team name is lowercase words joined by dashes"]); + expect(problems({ teams: { r: { members: [] } } })).toEqual(["agents.teams.r.members: a team needs at least one member"]); + expect(problems({ steps: { review: { teams: ["nope"] } } })).toEqual(["agents.steps.review.teams.0: there is no team nope under agents.teams"]); + expect(problems({ sources: "mine" })).toEqual(["agents.sources: must be all or repo"]); + // Narrow commands and read-only git are fine anywhere; Bash(git diff *) is not history-changing. + expect(problems({ steps: { review: { tools: ["Bash(npm run e2e *)", "Bash(git diff *)", "WebFetch"] } } })).toEqual([]); +}); +``` + +Also update the existing `claude.mcp_tools` assertions in this file to `agents.all.mcp_tools` (same three refused entries, same message). + +- [ ] **Step 2: Run them to see them fail** + +Run: `bun test tests/unit/config.test.ts` +Expected: FAIL (`cfg.agents` undefined). + +- [ ] **Step 3: Implement** + +In `src/agents.ts`, `toolProblem(rule, readOnly)`: +- `Bash(git ...)` or `Bash(git)`/`Bash(git *)` where `` is in `commit push merge rebase reset checkout switch branch tag stash` or absent → `only Loopstra commits, branches, merges and pushes; git is not allowed` (`` is `*` text as written for a bare git rule: use "git" alone → `git is not allowed`). +- When `readOnly`: `Edit`/`Write`/`NotebookEdit` (with or without a `(...)` specifier) → ` would let it change files`; `Bash` or `Bash(*)` → `a bare Bash rule would let it run anything`. +- Otherwise null. The config layer prefixes `" is read-only; "` for a step and `"these reach read-only steps too; "` for `all` (with `readOnly: true`). For build steps only the git check applies. + +In `src/config.ts`: a `stepAgents` Zod object (`.strict()`, lists `.default([])`, `agent` optional trimmed non-empty), `agents` object (`sources` enum with error `must be all or repo`, `teams` record with name check via `superRefine`, `all`, `steps` as a record checked against `STEP_KEYS` with the "unknown step" message listing `STEP_KEYS` joined by `, `), and a `superRefine` on `agents` for: `agent` on `CONTINUES_BUILD`, `toolProblem` per `tools` entry, team references. Reuse the existing `mcp_tools` regex and message for both `all.mcp_tools` and `steps.*.mcp_tools`. `prefault({})` the whole block. + +- [ ] **Step 4: Run tests** + +Run: `bun test tests/unit/config.test.ts && bun run typecheck` +Expected: PASS. Typecheck then fails where `cfg.claude.mcp_tools` is read (`toolsFor` in `src/phases.ts`): make `toolsFor` return the set's own tools only (Task 6 adds the setup's), and delete the mcp test in `tests/unit/phases.test.ts` (Task 6 brings it back on `agents.all.mcp_tools`). + +- [ ] **Step 5: Commit** + +```bash +git add src/agents.ts src/config.ts src/phases.ts tests/unit/config.test.ts tests/unit/phases.test.ts +git commit -m "feat(config): agents block for per-step agent setup" +``` + +--- + +### Task 2: Layering and prompt lines + +**Files:** +- Modify: `src/agents.ts` +- Test: `tests/unit/agents.test.ts` (new) + +**Interfaces:** +- Consumes: `Config["agents"]`, `StepAgents`, `CHAT_STEPS`, `CONTINUES_BUILD` (Task 1). +- Produces: + - `export interface Team { name: string; members: string[]; way: TeamWay }` + - `export interface AgentSetup { sources: "all" | "repo"; agent: string | null; subagents: string[]; teams: Team[]; skills: string[]; tools: string[]; mcpTools: string[] }` + - `export function resolveSetup(agents: Config["agents"], step: StepKey, stageSkills: readonly string[]): AgentSetup` + - `export function isEmptySetup(s: AgentSetup): boolean`: no agent, subagents, teams, skills, tools, mcpTools, and `sources: "all"`. + - `export function inlineName(name: string): string`: `loopstra-` + name with `:` → `-`. + - `export function setupLines(s: AgentSetup, describe: (name: string) => string | null): string`: the text put before the prompt template; `""` when there are no skills, subagents or teams. + +- [ ] **Step 1: Write the failing tests** + +```ts +const agents = (over: object = {}) => validateConfig({ version: 1, commands: { test: "t" }, agents: over }).agents; + +test("all, then stage skills, then the step's own; each name once", () => { + const a = agents({ all: { skills: ["house"], mcp_tools: ["mcp__docs"] }, steps: { review: { skills: ["threat", "house"], subagents: ["Explore"], agent: "lead" } } }); + expect(resolveSetup(a, "review", ["stage-skill"])).toEqual({ sources: "all", agent: "lead", subagents: ["Explore"], teams: [], skills: ["house", "stage-skill", "threat"], tools: [], mcpTools: ["mcp__docs"] }); +}); + +test("fix, reconcile and revise run as the build's agent; chat steps never get all", () => { + const a = agents({ all: { skills: ["house"], tools: ["WebFetch"] }, steps: { build: { agent: "senior" }, orchestrator: { skills: ["chat-tone"] } } }); + expect(resolveSetup(a, "fix", []).agent).toBe("senior"); + expect(resolveSetup(a, "revise", []).skills).toEqual(["house"]); + expect(resolveSetup(a, "orchestrator", [])).toMatchObject({ agent: null, skills: ["chat-tone"], tools: [] }); +}); + +test("teams resolve to their members; an empty setup is empty", () => { + const a = agents({ teams: { reviewers: { members: ["sec", "perf"], way: "parallel" } }, steps: { review: { teams: ["reviewers"] } } }); + expect(resolveSetup(a, "review", []).teams).toEqual([{ name: "reviewers", members: ["sec", "perf"], way: "parallel" }]); + expect(isEmptySetup(resolveSetup(agents(), "build", []))).toBe(true); + expect(isEmptySetup(resolveSetup(agents({ sources: "repo" }), "build", []))).toBe(false); +}); + +test("prompt lines: skills as today, then subagents and teams by their inline names", () => { + const s = { sources: "all", agent: null, subagents: ["Explore", "test-writer"], teams: [{ name: "reviewers", members: ["sec", "perf"], way: "parallel" }], skills: ["a", "b"], tools: [], mcpTools: [] } as AgentSetup; + const lines = setupLines(s, (n) => (n === "test-writer" ? "Writes tests" : null)); + expect(lines).toStartWith("Use these skills: `a`, `b`.\n\n"); + expect(lines).toContain("`Explore`"); // built-ins keep their own name + expect(lines).toContain("`loopstra-test-writer` (Writes tests)"); + expect(lines).toContain("`loopstra-sec`, `loopstra-perf`"); + expect(lines).toContain("same task"); // parallel wording + expect(lines).toContain("structured output"); // the session still reports itself + expect(lines).toEndWith("\n\n"); + expect(setupLines({ ...s, subagents: [], teams: [] }, () => null)).toBe("Use these skills: `a`, `b`.\n\n"); +}); +``` + +- [ ] **Step 2: Run to see them fail** + +Run: `bun test tests/unit/agents.test.ts` +Expected: FAIL (`resolveSetup` not exported). + +- [ ] **Step 3: Implement** in `src/agents.ts`. + +`setupLines` needs to know built-ins: define `export const BUILTIN_AGENTS: ReadonlySet = new Set(["general-purpose", "Explore", "Plan", "claude", "statusline-setup"]);` here in `src/agents.ts` (`src/agent-files.ts` imports from this file, never the other way). The skills line is exactly today's (`Use these skills: \`a\`, \`b\`.\n\n`). Wording for subagents and teams is yours; it must name each agent by its inline name (built-ins by their own), give the description when `describe` returns one, say for `parallel` that every member gets the same task at the same time and the session weighs their answers, for `pick` that each piece of work goes to the member that fits it, and end with one sentence that the session still returns its own structured output. + +- [ ] **Step 4: Run tests** + +Run: `bun test tests/unit/agents.test.ts` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add src/agents.ts tests/unit/agents.test.ts +git commit -m "feat(agents): layer each step's agent setup and its prompt lines" +``` + +--- + +### Task 3: Finding and parsing definitions, inline agents + +**Files:** +- Modify: `src/agent-files.ts` +- Test: `tests/unit/agent-files.test.ts` (new) + +**Interfaces:** +- Consumes: `AgentSetup`, `inlineName` (Task 2). +- Consumes also: `BUILTIN_AGENTS` (Task 2, `src/agents.ts`). +- Produces: + - `export interface AgentDefinition { name: string; description: string; prompt: string; tools: string[] | null; model: string | null; skills: string[] | null; dropped: string[]; from: "repo" | "personal" | "plugin"; path: string }` + - `export function parseAgentFile(name: string, text: string, from: AgentDefinition["from"], path: string): AgentDefinition` (frontmatter via `yaml`; `tools` accepts a comma-separated string or a list; keys other than `name description tools model skills color` go to `dropped`, in file order) + - `export interface Where { repo: string; home: string; sources: "all" | "repo" }` (`repo`: a checkout of main: the step's snapshot, or the root) + - `export async function findAgent(name: string, where: Where): Promise`: `BUILTIN_AGENTS` → null is not used for them (callers skip built-ins); order: `/.claude/agents/.md`; with `sources: "all"`: `/.claude/agents/.md`, then for `plugin:agent` the plugin's `agents/.md` (see Step 3). + - `export async function findSkill(name: string, where: Where): Promise<"repo" | "personal" | "plugin" | null>`: same order, `skills//SKILL.md`. + - `export function inlineAgents(main: AgentDefinition | null, others: AgentDefinition[]): Record`: keys by `inlineName`; `main`'s `tools`, when not null, gain `StructuredOutput` if missing; `model` only for `others` (a main agent runs on the step's model); `skills` copied when set. + +- [ ] **Step 1: Write the failing tests** + +```ts +test("an agent file parses with CRLF, a tools string, and unknown keys dropped", () => { + const d = parseAgentFile("lead", "---\r\nname: lead\r\ndescription: Leads\r\ntools: Read, Grep\r\nmodel: opus\r\npermissionMode: bypassPermissions\r\nhooks: {}\r\n---\r\nYou lead.\r\n", "repo", "/r/.claude/agents/lead.md"); + expect(d).toMatchObject({ description: "Leads", prompt: "You lead.", tools: ["Read", "Grep"], model: "opus", dropped: ["permissionMode", "hooks"] }); + expect(parseAgentFile("x", "Just a prompt.\n", "repo", "p")).toMatchObject({ description: "", prompt: "Just a prompt.", tools: null, model: null, dropped: [] }); +}); + +test("findAgent looks in the repo, then the user's folder, then installed and enabled plugins", async () => { + const t = tempDir(); const repo = join(t.path, "repo"); const home = join(t.path, "home"); + write(join(repo, ".claude/agents/lead.md"), "---\ndescription: repo lead\n---\nR"); + write(join(home, ".claude/agents/lead.md"), "---\ndescription: home lead\n---\nH"); + write(join(home, ".claude/agents/mine.md"), "---\ndescription: mine\n---\nM"); + const plug = join(home, "plugins-cache/sp"); + write(join(plug, "agents/code-reviewer.md"), "---\ndescription: cr\n---\nC"); + write(join(home, ".claude/plugins/installed_plugins.json"), JSON.stringify({ version: 2, plugins: { "sp@market": [{ scope: "user", installPath: plug }] } })); + write(join(home, ".claude/settings.json"), JSON.stringify({ enabledPlugins: { "sp@market": true } })); + const all = { repo, home, sources: "all" as const }; + expect((await findAgent("lead", all))?.description).toBe("repo lead"); + expect((await findAgent("mine", all))?.from).toBe("personal"); + expect((await findAgent("sp:code-reviewer", all))?.from).toBe("plugin"); + expect(await findAgent("mine", { ...all, sources: "repo" })).toBeNull(); + write(join(home, ".claude/settings.json"), JSON.stringify({ enabledPlugins: { "sp@market": false } })); + expect(await findAgent("sp:code-reviewer", all)).toBeNull(); + t.cleanup(); +}); + +test("inline agents: loopstra names, StructuredOutput for the main agent, its model left to the step", () => { + const lead = parseAgentFile("lead", "---\ndescription: L\ntools: [Read]\nmodel: opus\n---\nP", "repo", "p"); + const sec = parseAgentFile("sec", "---\ndescription: S\nmodel: haiku\n---\nQ", "repo", "p"); + expect(inlineAgents(lead, [sec])).toEqual({ + "loopstra-lead": { description: "L", prompt: "P", tools: ["Read", "StructuredOutput"] }, + "loopstra-sec": { description: "S", prompt: "Q", model: "haiku" }, + }); +}); +``` + +(`write` and `tempDir` come from `tests/helpers.ts`; add a two-line `write(path, text)` that makes the folder there if it has none.) + +- [ ] **Step 2: Run to see them fail** + +Run: `bun test tests/unit/agent-files.test.ts` +Expected: FAIL. + +- [ ] **Step 3: Implement** + +Plugins: read `/.claude/plugins/installed_plugins.json` (`plugins: Record<"@", Array<{ installPath: string }>>`) and `/.claude/settings.json` `enabledPlugins` (`Record`); a plugin counts when its key's value is `true`. For `plugin:agent` take the entry whose key starts with `@`, its first `installPath`. A file that does not exist or does not parse is "not there", never a throw. + +- [ ] **Step 4: Run tests** + +Run: `bun test tests/unit/agent-files.test.ts` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add src/agent-files.ts tests/unit/agent-files.test.ts tests/helpers.ts +git commit -m "feat(agents): find and parse agent and skill definitions, build inline agents" +``` + +--- + +### Task 4: Checking a whole config's names + +**Files:** +- Modify: `src/agent-files.ts` +- Test: `tests/unit/agent-files.test.ts` + +**Interfaces:** +- Consumes: `resolveSetup` (Task 2), `findAgent`, `findSkill` (Task 3), `STEPS`/`STEP_KEYS` (`src/models.ts`). +- Produces: + - `export interface SetupProblem { level: "fail" | "warn"; step: StepKey | "all"; name: string; text: string }` + - `export async function checkAgentSetup(cfg: Config, where: Where): Promise`: for every step, the resolved setup's agent, subagents and team members (skipping `BUILTIN_AGENTS`) and skills. + +Rules (one problem per name per step, steps in `STEP_KEYS` order): +- agent not found → `fail`: `` ``, the agent runs as, is not in .claude/agents/ on [ or on this machine]. Add it with `loopstra setup agents`, or commit it. `` +- subagent or team member not found → `fail`, same wording with "a subagent of " / "in the team of ". +- found only `personal` or `plugin` → `warn`: `` `` () is only on this machine, so a loop running elsewhere will not have it. `` +- a definition with `dropped` keys → `warn`: `` : Loopstra leaves out when it runs this agent. `` +- skill not found → `warn`: `` The skill `` () is not in .claude/skills/[ or on this machine]; it may be built in to Claude Code. If a session does not load it, the step stops and says so. `` +- `[ or on this machine]` appears only with `sources: all`. + +- [ ] **Step 1: Write the failing test** + +```ts +test("checkAgentSetup: missing agents fail, machine-only ones and unknown skills warn", async () => { + const t = tempDir(); const repo = join(t.path, "repo"); const home = join(t.path, "home"); + write(join(repo, ".claude/agents/lead.md"), "---\ndescription: L\nhooks: {}\n---\nP"); + write(join(home, ".claude/agents/mine.md"), "M"); + const cfg = validateConfig({ version: 1, commands: { test: "t" }, agents: { steps: { review: { agent: "lead", subagents: ["mine", "Explore", "gone"], skills: ["pdf"] } } } }); + const p = await checkAgentSetup(cfg, { repo, home, sources: "all" }); + expect(p.map((x) => [x.level, x.step, x.name])).toEqual([ + ["warn", "review", "lead"], ["warn", "review", "mine"], ["fail", "review", "gone"], ["warn", "review", "pdf"], + ]); + expect(p[2]!.text).toContain("Add it with `loopstra setup agents`"); + t.cleanup(); +}); +``` + +- [ ] **Step 2: Run to see it fail.** `bun test tests/unit/agent-files.test.ts` → FAIL. +- [ ] **Step 3: Implement `checkAgentSetup`.** +- [ ] **Step 4: Run tests.** → PASS. +- [ ] **Step 5: Commit** `feat(agents): check that every named agent and skill exists`. + +--- + +### Task 5: Session flags and the init check + +**Files:** +- Modify: `src/claude.ts` (`FailureReason`, `RunPhaseInput`, `claudeCommand`, `runSession`'s `onLine` and return) +- Modify: `src/phases.ts` (`ownerNote`, `BlockingReason` stays `Exclude` so it gains `"setup"`; `"setup"` is not in `RETRIED`) +- Test: `tests/unit/claude-run.test.ts` + +**Interfaces:** +- Produces on `RunPhaseInput`: + - `agents?: Record` → `--agents ` when it has keys + - `agent?: string` → `--agent ` + - `settingSources?: string` → `--setting-sources ` + - `addDirs?: string[]` → `--add-dir ` per dir + - `forwardSubagentText?: boolean` → `--forward-subagent-text` + - `checkInit?: (init: { agents?: string[]; skills?: string[] }) => string | null`: called with the first `system`/`init` event; a string ends the session: reading stops (`onLine` returns true), and the result is a failure with reason `"setup"` and that string as detail. +- `FailureReason` gains `"setup"`. `ownerNote("setup")`: `"This step's agent setup is not complete on this machine."` (callers append the detail). +- Flags go after `--permission-mode` and the tool lists, before `--settings`, in the order listed above; none appear when unset. + +- [ ] **Step 1: Write the failing tests** (fake CLI; follow the file's existing `LOOPSTRA_FAKE_ARGS` pattern) + +```ts +test("agent flags reach the CLI only when set", async () => { + // run once without them: none of the five flags in args + // run with { agents: { "loopstra-x": { description: "d", prompt: "p" } }, agent: "loopstra-x", settingSources: "project", addDirs: ["/snap"], forwardSubagentText: true } + expect(args).toEqual(expect.arrayContaining(["--agents", JSON.stringify({ "loopstra-x": { description: "d", prompt: "p" } }), "--agent", "loopstra-x", "--setting-sources", "project", "--add-dir", "/snap", "--forward-subagent-text"])); +}); + +test("an init event that lacks a named skill ends the session as a setup failure", async () => { + // fixture: init with skills: ["a"], agents: ["loopstra-x"], then a long tail of events (use an existing slow fixture, or LOOPSTRA_FAKE_DELAY_MS) + const r = await runPhase({ ...base, checkInit: (i) => (i.skills?.includes("b") ? null : "this session did not load the skill `b`") }); + expect(r).toMatchObject({ ok: false, reason: "setup", detail: "this session did not load the skill `b`" }); +}); + +test("an init event without lists is not judged", async () => { + // the default fixture's init has no agents or skills + const r = await runPhase({ ...base, checkInit: () => "would fail" }); + // checkInit is not called when both lists are absent + expect(r.ok).toBe(true); +}); +``` + +Add the fixture `tests/fake-claude/fixtures/init-lists.jsonl`: an init event with `"skills":["a"],"agents":["loopstra-x"]`, then the lines of `simple-success.jsonl`. + +- [ ] **Step 2: Run to see them fail.** `bun test tests/unit/claude-run.test.ts` → FAIL. +- [ ] **Step 3: Implement.** In `runSession`, keep a `setupProblem: string | null`; in `onLine`, for the first `system`/`init` event that has `agents` or `skills`, call `checkInit`; a string sets `setupProblem` and returns `true`. After the loop, `setupProblem` wins over every other outcome except a stop or cancel: `return failure(collector.finish(), { started, nudged }, "setup", setupProblem)`. `checkInit` is not called on resent processes (`--resume` sends) after the first passed. +- [ ] **Step 4: Run tests.** `bun test tests/unit/claude-run.test.ts && bun run typecheck` → PASS. +- [ ] **Step 5: Commit** `feat(claude): agent flags and the init check, with failure reason setup`. + +--- + +### Task 6: Wire the setup into loop sessions + +**Files:** +- Modify: `src/models.ts` (`withModels` carries `agents`; `ModelSettings` = `Pick`; its doc says model and agent settings) +- Modify: `src/context.ts` (`models()` doc: model and agent settings) +- Modify: `src/agent-files.ts` (`userSettingsPassthrough`) +- Modify: `src/phases.ts` (`toolsFor`, `phasePrompt`, `attempt`) +- Test: `tests/unit/phases.test.ts` + +**Interfaces:** +- Consumes: Tasks 2-5. +- Produces: + - `export function userSettingsPassthrough(home: string): { apiKeyHelper?: string; env?: Record }`: those two keys from `/.claude/settings.json`, when present (with `sources: repo`, `--setting-sources project` would drop them, and a machine that signs in with them could not start sessions). + - `export async function stepSetup(ctx: StepContext, spec: AgentPhaseSpec): Promise` in `src/phases.ts`: `resolveSetup((await ctx.models()).agents, spec.name as StepKey, spec.skills ?? [])`. (Every `PhaseName` used with `agentPhase` is a `StepKey`; assert with a type guard, not a cast, and fall back to an empty setup for any that is not.) + - `toolsFor(ctx, set, setup?: AgentSetup)`: the set's own tools, then `setup.tools`, then `setup.mcpTools`, each once. + +`attempt` gets the setup once per attempt and, when `isEmptySetup(setup)`, passes nothing new (today's argv). Otherwise: +1. `allowedTools: toolsFor(ctx, spec.tools, setup)`. +2. `where = { repo: await ctx.snapshot(), home: os.homedir(), sources: setup.sources }`. Definitions: main agent via `findAgent(setup.agent)`; subagents and team members via `findAgent` (built-ins skipped). Any non-built-in not found → return `{ ok: false, reason: "setup", note: " ", sessionId: null }` before starting a session (trace an `error` event `{ where: traceName, reason: "setup", detail }` as `missing-prompt` does). +3. `agents: inlineAgents(main, others)` when any definition; `agent: inlineName(setup.agent)` when there is a main agent. +4. `settingSources: "project"` and settings `JSON.stringify({ ...JSON.parse(hookSettings()), ...userSettingsPassthrough(home) })` when `sources === "repo"`. +5. `addDirs: [snapshot]` when `cwd !== snapshot` and `setup.skills.length`. +6. `forwardSubagentText: true` when subagents or teams. +7. `checkInit`: every expected inline agent name (and built-in) must be in `init.agents` when that list is present; every skill in `init.skills` when present (a skill counts if listed as `name` or `:name`); the problem names what is missing and says `` Add it with `loopstra setup agents`, then retry. `` +8. After `startPhase`, `ctx.trace.event(ctx.slug, "agent_setup", { agent, subagents, teams: teams.map((t) => t.name), skills, tools, mcp_tools: mcpTools, sources, models: { : model } }, seq)`, only when not empty. +9. `phasePrompt` uses `setupLines(setup, describe)` in place of today's `skillsLine`. `describe` is synchronous: `phasePrompt` first looks up the subagents' and team members' definitions with `findAgent` (same `where` as above) and passes `(n) => found.get(n)?.description || null`. With an empty setup `setupLines` returns today's skills line, so prompts do not change. +10. A failed `"setup"` attempt's note is `` `${ownerNote("setup")} ${detail}` ``. + +- [ ] **Step 1: Write the failing tests** in `tests/unit/phases.test.ts` (use the file's `setup()` and `LOOPSTRA_FAKE_ARGS`) + +```ts +test("with nothing set, a session's args and prompt are what they were", async () => { + // run intake and build once with cfg.agents as validated from {} and once from { sources: "all", all: {}, steps: {} }: + // identical args arrays and identical prompt.md text; neither has --agents, --agent, --setting-sources, --add-dir, --forward-subagent-text +}); + +test("a step's agent, subagents, team and tools reach its session", async () => { + // repo: commit .claude/agents/lead.md (tools: Read), sec.md, perf.md on main + // cfg.agents = { teams: { reviewers: { members: ["sec", "perf"], way: "parallel" } }, steps: { review: { agent: "lead", teams: ["reviewers"], tools: ["WebFetch"], mcp_tools: ["mcp__gh"] } } } + // run the review phase (tools "read+git", cwd: the worktree) + expect(args[args.indexOf("--agent") + 1]).toBe("loopstra-lead"); + expect(Object.keys(JSON.parse(args[args.indexOf("--agents") + 1]))).toEqual(["loopstra-lead", "loopstra-sec", "loopstra-perf"]); + expect(JSON.parse(args[args.indexOf("--agents") + 1])["loopstra-lead"].tools).toEqual(["Read", "StructuredOutput"]); + expect(allowed).toEqual(expect.arrayContaining(["WebFetch", "mcp__gh"])); + expect(args).toContain("--forward-subagent-text"); + expect(prompt).toContain("`loopstra-sec`, `loopstra-perf`"); +}); + +test("agent files come from main, not from the change's branch", async () => { + // main: .claude/agents/lead.md prompt "MAIN"; the worktree branch commits lead.md with prompt "BRANCH" + // build-stage step in the worktree: the inline lead prompt is "MAIN" +}); + +test("a named agent nobody has stops the step before a session starts", async () => { + // steps: { design: { agent: "ghost" } } → result { ok: false, reason: "setup" }, note mentions `ghost` and `loopstra setup agents`; no fake call recorded +}); + +test("repo-only sessions load project settings and keep the user's sign-in", async () => { + // a temp home with .claude/settings.json { apiKeyHelper: "/bin/key", env: { A: "1" }, theme: "x" }; the code reads os.homedir(), + // so set process.env.HOME and process.env.USERPROFILE to it for this test and restore them after + expect(args[args.indexOf("--setting-sources") + 1]).toBe("project"); + expect(JSON.parse(args[args.indexOf("--settings") + 1])).toMatchObject({ apiKeyHelper: "/bin/key", env: { A: "1" }, hooks: expect.any(Object) }); + expect(JSON.parse(args[args.indexOf("--settings") + 1]).theme).toBeUndefined(); +}); + +test("a worktree step that names skills also gets main's snapshot as an added dir", async () => { + // all: { skills: ["house"] } → build session args contain --add-dir ; a snapshot-run step (design) does not +}); + +test("mcp rules from agents.all reach every tool set, once each", async () => { + // the former claude.mcp_tools test, moved: ctx.cfg.agents.all.mcp_tools = ["mcp__github", "mcp__docs__guide", "mcp__github"] +}); +``` + +Fill each with the setup and run calls the neighbouring tests in this file use; the assertions above are the required ones. + +- [ ] **Step 2: Run to see them fail.** `bun test tests/unit/phases.test.ts` → FAIL. +- [ ] **Step 3: Implement** as listed above. +- [ ] **Step 4: Run tests.** `bun test tests/unit/phases.test.ts && bun run check` → PASS (the whole suite: stage tests must be untouched by an empty setup). +- [ ] **Step 5: Commit** `feat(phases): run each step with its agent setup`. + +--- + +### Task 7: Chat steps + +**Files:** +- Modify: `src/chat/agents.ts` (`runChatAgent`) +- Modify: `src/chat/orchestrator.ts` (the failure reply around line 277) +- Test: the test file that covers `runChatAgent` (find it with `grep -l runChatAgent tests/unit`) + +**Interfaces:** +- Consumes: `resolveSetup` (it already leaves `all` out of chat steps), `findAgent`, `inlineAgents`, `inlineName`, `setupLines`, `userSettingsPassthrough`. +- Behaviour: + - Setup for `o.name`; definitions with `where = { repo: o.root, home: os.homedir(), sources }` (chat runs in the main checkout). + - `allowedTools: [...CHAT_TOOLS, ...setup.tools, ...setup.mcpTools]` (only the chat step's own entries, by `resolveSetup`); `disallowedTools: CHAT_DENIED` unchanged; `dropEnv` unchanged. + - Same flags and `checkInit` as Task 6; prompt lines prepended to `o.prompt` when not empty. + - A missing definition or a `setup` failure: `{ ok: false, reason: "setup", detail }` with no session (or the init check's detail). The orchestrator replies: `` My setup is not complete: ``. Write-intent's failure path shows the same detail (follow how it shows other failures). + +- [ ] **Step 1: Write the failing tests** + +```ts +test("a chat step gets only its own tools; agents.all never reaches it", async () => { + // cfg.agents = { all: { tools: ["WebFetch"], mcp_tools: ["mcp__x"] }, steps: { orchestrator: { skills: ["tone"], mcp_tools: ["mcp__docs"] } } } + // allowed = CHAT_TOOLS + mcp__docs; no WebFetch, no mcp__x; disallowed is CHAT_DENIED; prompt starts with "Use these skills: `tone`." +}); + +test("a chat agent that cannot be found is answered in words", async () => { + // steps: { orchestrator: { agent: "ghost" } } → the orchestrator's reply starts with "My setup is not complete:" and names `ghost` +}); +``` + +- [ ] **Step 2: Run to see them fail.** +- [ ] **Step 3: Implement.** +- [ ] **Step 4: Run tests.** `bun test tests/unit/chat-*.test.ts` → PASS. +- [ ] **Step 5: Commit** `feat(chat): agent setup for the orchestrator and the intent writer`. + +--- + +### Task 8: `start` refuses while a named agent is missing + +**Files:** +- Modify: `src/scheduler.ts` (`start`, before `takeLoopLock`) +- Test: the test file covering `start` refusals (`grep -l "await start(" tests`) + +**Interfaces:** +- Consumes: `checkAgentSetup` (Task 4). +- Behaviour: load the config; when `agents` is not empty, make a detached worktree of `main_branch` in `.loopstra/snapshots/_start` (reuse `addDetachedWorktree`/`removeWorktree` from `src/git.ts`, removed in a `finally`), run `checkAgentSetup` with it as `repo`. Any `fail` → return `"Some steps name agents that are not set up:\n" + fails.map((p) => `- ${p.text}`).join("\n")`. `warn`s are printed with `log` and do not refuse. A config that does not load is left to the existing handling (no new refusal). + +- [ ] **Step 1: Write the failing test** + +```ts +test("start refuses while a step names an agent that main does not have", async () => { + // config with agents.steps.design.agent: "ghost", committed on main + const refused = await start(repo.path, { once: true, installSignals: false }); + expect(refused).toStartWith("Some steps name agents that are not set up:\n- `ghost`, the agent design runs as"); +}); +``` + +- [ ] **Step 2: Run to see it fail.** +- [ ] **Step 3: Implement.** +- [ ] **Step 4: Run tests.** → PASS. +- [ ] **Step 5: Commit** `feat(start): refuse while a step names a missing agent`. + +--- + +### Task 9: Templates, docs, and a live check + +**Files:** +- Modify: `templates/config.yaml` (replace the commented `mcp_tools` lines under `claude:` with a commented `agents:` block: `sources`, a `teams` example, `all.mcp_tools`, one `steps.review` example; every line commented out) +- Modify: `templates/skill/SKILL.md` (Onboard step 5 writes `agents.all.mcp_tools` under `agents:`; step 2's mention of `claude.mcp_tools` becomes `agents.all.mcp_tools`) +- Modify: `README.md` (config section: replace the `mcp_tools` sentence under `claude` with an `agents` bullet: sources, teams, all, steps; the safety rules; chat's rule; fix/reconcile/revise; the `setup` failure) +- Modify: `docs/decisions.md` (new section "Agent setup for every step (2026-10-26)": the CLI facts above, `claude.mcp_tools` moved before release, definitions from main, inline names, `StructuredOutput`, skills by `--add-dir`, the sign-in passthrough, file check warns for skills) +- Test: `tests/unit/templates*.test.ts` stay green. + +- [ ] **Step 1: Edit the four files.** +- [ ] **Step 2: Run** `bun run check` → all green. +- [ ] **Step 3: Live check with the real CLI** in a scratch repo (`loopstra init`, a committed `.claude/agents/lead.md` with `tools: Read`, `agents.steps.intake.agent: lead`, `agents.steps.build.agent: lead` and `subagents: [Explore]`, one accepted intent, `loopstra start --once` until build has run and a fix has resumed if one happens). Record in `docs/decisions.md`: the intake envelope parsed (StructuredOutput through an inline agent), the build's `agent_setup` trace event, whether `--resume` with the same `--agent` kept the session id. If resume with `--agent` fails, stop and report it before changing anything. +- [ ] **Step 4: Commit** `docs: agent setup in the config template, skill, README and decisions`. From 44ec356b4e9a8bc7cf412493089e12f02e619127 Mon Sep 17 00:00:00 2001 From: John Flavan Date: Mon, 26 Oct 2026 22:21:10 -0500 Subject: [PATCH 04/64] feat(config): agents block for per-step agent setup Co-Authored-By: Claude Opus 5.5 --- src/agents.ts | 34 +++++++++++++++++++ src/config.ts | 69 +++++++++++++++++++++++++++++++++++---- src/phases.ts | 7 ++-- tests/unit/agents.test.ts | 27 +++++++++++++++ tests/unit/config.test.ts | 35 +++++++++++++++++--- tests/unit/phases.test.ts | 18 ---------- 6 files changed, 157 insertions(+), 33 deletions(-) create mode 100644 src/agents.ts create mode 100644 tests/unit/agents.test.ts diff --git a/src/agents.ts b/src/agents.ts new file mode 100644 index 00000000..5a27ae77 --- /dev/null +++ b/src/agents.ts @@ -0,0 +1,34 @@ +import { STEP_KEYS, type StepKey } from "./models"; + +/** How a team's members work: all at once and merged, or each tries and the best is kept. */ +export const TEAM_WAYS = ["parallel", "pick"] as const; +export type TeamWay = (typeof TEAM_WAYS)[number]; + +/** Steps that change files: the rest are read-only. */ +const WRITING_STEPS: readonly StepKey[] = ["build", "reconcile", "fix", "revise"]; + +/** Every step except those that change files. */ +export const READ_ONLY_STEPS: ReadonlySet = new Set(STEP_KEYS.filter((k) => !WRITING_STEPS.includes(k))); + +/** Steps that continue the build's session, so they run as the build's agent. */ +export const CONTINUES_BUILD: ReadonlySet = new Set(["fix", "reconcile", "revise"]); + +/** Steps that are chat turns rather than steps of a change. */ +export const CHAT_STEPS: ReadonlySet = new Set(["orchestrator", "write-intent"]); + +const HISTORY_GIT = new Set(["commit", "push", "merge", "rebase", "reset", "checkout", "switch", "branch", "tag", "stash"]); + +/** The reason a `tools` rule is refused, or null. A read-only step also refuses file changes and a bare Bash. */ +export function toolProblem(rule: string, readOnly: boolean): string | null { + const git = /^Bash\(git(?:\s+([^\s)]*))?[^)]*\)$/.exec(rule); + if (git) { + const sub = git[1]; + if (sub === undefined || sub === "" || sub === "*") return "only Loopstra commits, branches, merges and pushes; git is not allowed"; + if (HISTORY_GIT.has(sub)) return `only Loopstra commits, branches, merges and pushes; git ${sub} is not allowed`; + } + if (!readOnly) return null; + const edit = /^(Edit|Write|NotebookEdit)(\(.*\))?$/.exec(rule); + if (edit) return `${edit[1] ?? rule} would let it change files`; + if (/^Bash(\(\*?\))?$/.test(rule)) return "a bare Bash rule would let it run anything"; + return null; +} diff --git a/src/config.ts b/src/config.ts index 5c3035a0..48422771 100644 --- a/src/config.ts +++ b/src/config.ts @@ -2,7 +2,8 @@ import { existsSync } from "node:fs"; import { join } from "node:path"; import { parse } from "yaml"; import { z } from "zod"; -import { MODEL_REFS, stepsOf, type ModelRef, type StageName, type StepGroup } from "./models"; +import { CONTINUES_BUILD, READ_ONLY_STEPS, TEAM_WAYS, toolProblem } from "./agents"; +import { MODEL_REFS, STEP_KEYS, stepsOf, type ModelRef, type StepKey, type StageName, type StepGroup } from "./models"; import { errorText } from "./shell"; /** One thing wrong with a config: where (["claude", "timeout_minutes"]), and in words. */ @@ -102,6 +103,66 @@ const modelName = (fallback: string) => z.string().trim().min(1, "must be a mode /** How long the loop waits between ticks when the config does not say (or cannot be loaded). */ export const DEFAULT_POLL_SECONDS = 60; +const mcpRule = z.string().regex(/^mcp__[\w*-]+$/, "must be an MCP rule, like mcp__github or mcp__github__get_issue"); +const nameList = z.array(z.string().trim().min(1, "must be a name")).default([]); + +const stepAgents = z.object({ + /** The agent this step runs as. Unset: no agent. */ + agent: z.string().trim().min(1, "must be an agent name").optional(), + subagents: nameList, + teams: nameList, + skills: nameList, + /** Tool allow rules added to this step's own. */ + tools: z.array(z.string()).default([]), + /** + * MCP allow rules (`mcp__` or `mcp____`). No commas or spaces: the list + * reaches the CLI joined with commas. + */ + mcp_tools: z.array(mcpRule).default([]), +}).strict(); + +/** One step's agent setup, or what `agents.all` gives every step. */ +export type StepAgents = z.infer; + +const agents = z.object({ + sources: z.enum(["all", "repo"], { error: "must be all or repo" }).default("all"), + teams: z.record(z.string(), z.object({ + members: z.array(z.string().trim().min(1, "must be an agent name")).min(1, "a team needs at least one member"), + way: z.enum(TEAM_WAYS).default("parallel"), + }).strict()).default({}), + all: stepAgents.prefault({}), + steps: z.record(z.string(), stepAgents).default({}), +}).strict().superRefine((a, ctx) => { + const bad = (path: (string | number)[], message: string) => ctx.addIssue({ code: "custom", path, message }); + for (const name of Object.keys(a.teams)) { + if (!/^[a-z0-9]+(-[a-z0-9]+)*$/.test(name)) bad(["teams", name], "a team name is lowercase words joined by dashes"); + } + const tools = (path: string[], set: StepAgents, readOnly: boolean, who: string, it: string) => { + set.tools.forEach((rule, i) => { + const problem = toolProblem(rule, readOnly); + if (!problem) return; + // A git rule is refused the same way everywhere; the rest depend on the step being read-only. + bad([...path, "tools", i], problem.startsWith("only Loopstra") ? problem : `${who}; ${problem.replace(" would let it ", ` would let ${it} `)}`); + }); + set.teams.forEach((t, i) => { + if (!(t in a.teams)) bad([...path, "teams", i], `there is no team ${t} under agents.teams`); + }); + }; + tools(["all"], a.all, true, "these reach read-only steps too", "them"); + for (const [step, set] of Object.entries(a.steps)) { + if (!(STEP_KEYS as readonly string[]).includes(step)) { + bad(["steps"], `unknown step ${step}; the steps are ${STEP_KEYS.join(", ")}`); + continue; + } + const key = step as StepKey; + if (set.agent !== undefined && CONTINUES_BUILD.has(key)) { + bad(["steps", step, "agent"], `${step} continues the build session and runs as the build's agent; set the agent on build`); + } + const readOnly = READ_ONLY_STEPS.has(key); + tools(["steps", step], set, readOnly, `${step} is read-only`, "it"); + } +}); + export const ConfigSchema = z.object({ version: z.literal(1, { error: "must be 1" }), main_branch: z.string().trim().min(1, "must be a branch name, like main").default("main"), @@ -128,12 +189,8 @@ export const ConfigSchema = z.object({ max_budget_usd_per_day: z.number().positive().optional(), // Git that only reads: the runtime makes every commit, branch, and merge itself. allowed_tools: z.array(z.string()).default(["Read", "Edit", "Write", "Glob", "Grep", "Bash(bun *)", "Bash(git diff *)", "Bash(git log *)", "Bash(git show *)", "Bash(git status *)"]), - /** - * MCP allow rules (`mcp__` or `mcp____`) added to every session's allow list, - * judges' included. No commas or spaces: the list reaches the CLI joined with commas. - */ - mcp_tools: z.array(z.string().regex(/^mcp__[\w*-]+$/, "must be an MCP rule, like mcp__github or mcp__github__get_issue")).default([]), }).strict().prefault({}), + agents: agents.prefault({}), // There is no intent gate to set: a person always accepts a change by setting its status to accepted. gates: z.object({ spec: z.object({ human: statusGate.default("none"), agent: z.boolean().default(true) }).strict().prefault({}), diff --git a/src/phases.ts b/src/phases.ts index 8f835af2..9ec60ba5 100644 --- a/src/phases.ts +++ b/src/phases.ts @@ -26,8 +26,7 @@ import { toolTarget } from "./tools"; /** * read: look only. read+git: look, plus read-only git (the reviewer). read+commands: look, the * configured project commands except install, and read-only git (verify, done-check). build: - * `claude.allowed_tools` plus every configured project command, install included. Every set also - * gets `claude.mcp_tools`. + * `claude.allowed_tools` plus every configured project command, install included. */ type ToolSet = "read" | "read+git" | "read+commands" | "build"; @@ -97,9 +96,9 @@ function commandParts(cmd: string): string[] { return parts.map((p) => p.trim()).filter((p) => p); } -/** Tools a session may use without asking: the set's own, then `claude.mcp_tools`, each once. Always a fresh array. */ +/** Tools a session may use without asking: the set's own, each once. Always a fresh array. */ export function toolsFor(ctx: StepContext, set: ToolSet): string[] { - return [...new Set([...setTools(ctx, set), ...ctx.cfg.claude.mcp_tools])]; + return [...new Set([...setTools(ctx, set)])]; } /** The tool set's own tools, before the MCP rules every set gets. */ diff --git a/tests/unit/agents.test.ts b/tests/unit/agents.test.ts new file mode 100644 index 00000000..5b824ebf --- /dev/null +++ b/tests/unit/agents.test.ts @@ -0,0 +1,27 @@ +import { expect, test } from "bun:test"; +import { CHAT_STEPS, CONTINUES_BUILD, READ_ONLY_STEPS, toolProblem, type TeamWay } from "../../src/agents"; +import { validateConfig, type StepAgents } from "../../src/config"; + +test("the step sets", () => { + expect([...READ_ONLY_STEPS].some((k) => ["build", "reconcile", "fix", "revise"].includes(k))).toBe(false); + expect(READ_ONLY_STEPS.has("review")).toBe(true); + expect([...CONTINUES_BUILD]).toEqual(["fix", "reconcile", "revise"]); + expect([...CHAT_STEPS]).toEqual(["orchestrator", "write-intent"]); +}); + +test("toolProblem names why a rule is refused", () => { + expect(toolProblem("Bash(git)", false)).toBe("only Loopstra commits, branches, merges and pushes; git is not allowed"); + expect(toolProblem("Bash(git *)", false)).toBe("only Loopstra commits, branches, merges and pushes; git is not allowed"); + expect(toolProblem("Bash(git status *)", true)).toBeNull(); + expect(toolProblem("Write(src/*)", true)).toBe("Write would let it change files"); + expect(toolProblem("Bash(*)", true)).toBe("a bare Bash rule would let it run anything"); + expect(toolProblem("Edit", false)).toBeNull(); +}); + +test("a team keeps its way", () => { + const way: TeamWay = "pick"; + const cfg = validateConfig({ version: 1, commands: { test: "bun test" }, agents: { teams: { r: { members: ["a"], way } } } }); + const all: StepAgents = cfg.agents.all; + expect(cfg.agents.teams.r).toEqual({ members: ["a"], way: "pick" }); + expect(all.tools).toEqual([]); +}); diff --git a/tests/unit/config.test.ts b/tests/unit/config.test.ts index c4f0f172..2bc02bbd 100644 --- a/tests/unit/config.test.ts +++ b/tests/unit/config.test.ts @@ -20,7 +20,8 @@ describe("loadConfig", () => { expect(cfg.gates.spec.agent).toBe(true); // Build sessions may read git history but not change it: the runtime owns commits and branches. expect(cfg.claude.allowed_tools).toEqual(["Read", "Edit", "Write", "Glob", "Grep", "Bash(bun *)", "Bash(git diff *)", "Bash(git log *)", "Bash(git show *)", "Bash(git status *)"]); - expect(cfg.claude.mcp_tools).toEqual([]); + expect("mcp_tools" in cfg.claude).toBe(false); + expect(cfg.agents.all.mcp_tools).toEqual([]); expect(cfg.stages.build.max_fix_loops).toBe(3); expect(cfg.claude.models.cheap).toBe("haiku"); // The cheap model often ends a verify session without calling the structured-output tool (#7). @@ -143,11 +144,11 @@ describe("loadConfig", () => { "commands.run: must be a command, or the line removed", "commands.install: must be a command, or the line removed", ]); expect(problems({ commands: { test: "bun test" } })).toEqual(["version: must be 1"]); - expect(problems({ ...ok, claude: { mcp_tools: ["mcp__github", "mcp__docs__*", "Bash(rm *)", "mcp__", "mcp__x,Bash"] } })).toEqual([ - "claude.mcp_tools.2: must be an MCP rule, like mcp__github or mcp__github__get_issue", - "claude.mcp_tools.3: must be an MCP rule, like mcp__github or mcp__github__get_issue", + expect(problems({ ...ok, agents: { all: { mcp_tools: ["mcp__github", "mcp__docs__*", "Bash(rm *)", "mcp__", "mcp__x,Bash"] } } })).toEqual([ + "agents.all.mcp_tools.2: must be an MCP rule, like mcp__github or mcp__github__get_issue", + "agents.all.mcp_tools.3: must be an MCP rule, like mcp__github or mcp__github__get_issue", // The list reaches the CLI joined with commas: this one would allow Bash. - "claude.mcp_tools.4: must be an MCP rule, like mcp__github or mcp__github__get_issue", + "agents.all.mcp_tools.4: must be an MCP rule, like mcp__github or mcp__github__get_issue", ]); expect(problems({ ...ok, version: 2 })).toEqual(["version: must be 1"]); const id = "must be a user or channel id; put long numeric ids in quotes"; @@ -194,3 +195,27 @@ describe("per-step models", () => { await expect(load("chat:\n steps:\n orchestrator: max\n")).rejects.toThrow(/chat\.steps.*orchestrator/); }); }); + +test("the agents block defaults to nothing set", async () => { + const cfg = validateConfig({ version: 1, commands: { test: "bun test" } }); + expect(cfg.agents).toEqual({ sources: "all", teams: {}, all: { subagents: [], teams: [], skills: [], tools: [], mcp_tools: [] }, steps: {} }); + expect("mcp_tools" in cfg.claude).toBe(false); +}); + +test("agents settings are refused in plain words", () => { + const ok = { version: 1, commands: { test: "bun test" } }; + const problems = (agents: unknown) => { try { validateConfig({ ...ok, agents }); return []; } catch (e) { return (e as ConfigError).problems.map((p) => p.text); } }; + expect(problems({ steps: { reviews: {} } })[0]).toStartWith("agents.steps: unknown step reviews; the steps are intake, design,"); + expect(problems({ steps: { fix: { agent: "x" } } })).toEqual(["agents.steps.fix.agent: fix continues the build session and runs as the build's agent; set the agent on build"]); + expect(problems({ steps: { review: { tools: ["Edit"] } } })).toEqual(["agents.steps.review.tools.0: review is read-only; Edit would let it change files"]); + expect(problems({ steps: { review: { tools: ["Bash"] } } })).toEqual(["agents.steps.review.tools.0: review is read-only; a bare Bash rule would let it run anything"]); + expect(problems({ steps: { build: { tools: ["Bash(git push *)"] } } })).toEqual(["agents.steps.build.tools.0: only Loopstra commits, branches, merges and pushes; git push is not allowed"]); + expect(problems({ all: { tools: ["Write"] } })).toEqual(["agents.all.tools.0: these reach read-only steps too; Write would let them change files"]); + expect(problems({ all: { mcp_tools: ["mcp__x,Bash"] } })).toEqual(["agents.all.mcp_tools.0: must be an MCP rule, like mcp__github or mcp__github__get_issue"]); + expect(problems({ teams: { Reviewers: { members: ["a"] } } })).toEqual(["agents.teams.Reviewers: a team name is lowercase words joined by dashes"]); + expect(problems({ teams: { r: { members: [] } } })).toEqual(["agents.teams.r.members: a team needs at least one member"]); + expect(problems({ steps: { review: { teams: ["nope"] } } })).toEqual(["agents.steps.review.teams.0: there is no team nope under agents.teams"]); + expect(problems({ sources: "mine" })).toEqual(["agents.sources: must be all or repo"]); + // Narrow commands and read-only git are fine anywhere; Bash(git diff *) is not history-changing. + expect(problems({ steps: { review: { tools: ["Bash(npm run e2e *)", "Bash(git diff *)", "WebFetch"] } } })).toEqual([]); +}); diff --git a/tests/unit/phases.test.ts b/tests/unit/phases.test.ts index 6ef52485..5f66dc06 100644 --- a/tests/unit/phases.test.ts +++ b/tests/unit/phases.test.ts @@ -174,24 +174,6 @@ describe("agentPhase tools and prompt", () => { trace.close(); repo.cleanup(); }); - test("claude.mcp_tools are allowed in every tool set, judges' included, once each", async () => { - const { repo, ctx, trace } = await setup(); - ctx.cfg.claude.mcp_tools = ["mcp__github", "mcp__docs__guide", "mcp__github"]; - for (const set of ["read", "read+git", "read+commands", "build"] as const) { - const tools = toolsFor(ctx, set); - expect(tools.slice(-2)).toEqual(["mcp__github", "mcp__docs__guide"]); - expect(new Set(tools).size).toBe(tools.length); - } - // MCP rules are not shell commands. - expect(commandsFor(ctx, "read")).toBe("none"); - - const argsFile = join(repo.path, "args.json"); - await agentPhase(ctx, { name: "intake", model: "cheap", permissionMode: "default", tools: "read", vars: {}, env: { LOOPSTRA_FAKE_ARGS: argsFile } }); - const recorded = await Bun.file(argsFile).json(); - expect(recorded.args).toEqual(expect.arrayContaining(["--allowedTools", "Read,Glob,Grep,mcp__github,mcp__docs__guide"])); - trace.close(); repo.cleanup(); - }); - test("a chained command also allows each of its parts, since Claude Code checks each part on its own", async () => { const { repo, ctx, trace } = await setup(); ctx.cfg.commands.test = "dotnet test api/App.sln && npm --prefix web test || echo failed; npm --prefix e2e test"; From 51c7aa3fa1af26eabe6a9aeacc042aede1cd3be4 Mon Sep 17 00:00:00 2001 From: John Flavan Date: Mon, 26 Oct 2026 22:24:16 -0500 Subject: [PATCH 05/64] fix(config): tokenise git tool rules; test step mcp_tools and build edits Co-Authored-By: Claude Opus 5.5 --- src/agents.ts | 29 +++++++++++++++++++++++------ tests/unit/agents.test.ts | 14 ++++++++++++++ tests/unit/config.test.ts | 3 +++ 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/src/agents.ts b/src/agents.ts index 5a27ae77..ca23c94f 100644 --- a/src/agents.ts +++ b/src/agents.ts @@ -18,14 +18,31 @@ export const CHAT_STEPS: ReadonlySet = new Set(["orchestrator", "write- const HISTORY_GIT = new Set(["commit", "push", "merge", "rebase", "reset", "checkout", "switch", "branch", "tag", "stash"]); +/** Git options that take their value as the next word when written without `=`. */ +const GIT_VALUE_OPTIONS = new Set(["-C", "-c", "--git-dir", "--work-tree", "--namespace", "--exec-path"]); + +/** + * The git subcommand a `Bash(git ...)` rule covers, "" when it covers any (a bare git rule, or only + * options), and null when the rule is not about git. Global options before the subcommand are skipped. + */ +function gitSubcommand(rule: string): string | null { + const m = /^Bash\(([\s\S]*)\)$/.exec(rule); + if (!m) return null; + const words = (m[1] ?? "").trim().replace(/:\*$/, "").split(/\s+/); + if (!/^git\*?$/.test(words[0] ?? "")) return null; + if (words[0] === "git*") return ""; + let i = 1; + while (i < words.length && (words[i] ?? "").startsWith("-")) { + i += GIT_VALUE_OPTIONS.has(words[i] ?? "") ? 2 : 1; + } + return (words[i] ?? "").replace(/(:\*|\*)$/, ""); +} + /** The reason a `tools` rule is refused, or null. A read-only step also refuses file changes and a bare Bash. */ export function toolProblem(rule: string, readOnly: boolean): string | null { - const git = /^Bash\(git(?:\s+([^\s)]*))?[^)]*\)$/.exec(rule); - if (git) { - const sub = git[1]; - if (sub === undefined || sub === "" || sub === "*") return "only Loopstra commits, branches, merges and pushes; git is not allowed"; - if (HISTORY_GIT.has(sub)) return `only Loopstra commits, branches, merges and pushes; git ${sub} is not allowed`; - } + const sub = gitSubcommand(rule); + if (sub === "") return "only Loopstra commits, branches, merges and pushes; git is not allowed"; + if (sub !== null && HISTORY_GIT.has(sub)) return `only Loopstra commits, branches, merges and pushes; git ${sub} is not allowed`; if (!readOnly) return null; const edit = /^(Edit|Write|NotebookEdit)(\(.*\))?$/.exec(rule); if (edit) return `${edit[1] ?? rule} would let it change files`; diff --git a/tests/unit/agents.test.ts b/tests/unit/agents.test.ts index 5b824ebf..65354ebe 100644 --- a/tests/unit/agents.test.ts +++ b/tests/unit/agents.test.ts @@ -25,3 +25,17 @@ test("a team keeps its way", () => { expect(cfg.agents.teams.r).toEqual({ members: ["a"], way: "pick" }); expect(all.tools).toEqual([]); }); + +test("toolProblem sees through git spellings, and leaves other tools alone", () => { + const why = (r: string) => toolProblem(r, false); + for (const r of ["Bash(git push:*)", "Bash(git push*)", "Bash(git commit*)", "Bash(git reset:*)", "Bash(git -C . push *)", "Bash(git -c k=v commit *)", "Bash( git push *)", "Bash(git --git-dir=x push)", "Bash(git --no-pager stash *)"]) { + expect(why(r)).toMatch(/^only Loopstra commits/); + } + expect(why("Bash(git -C . push *)")).toBe("only Loopstra commits, branches, merges and pushes; git push is not allowed"); + for (const r of ["Bash(git:*)", "Bash(git*)", "Bash(git -C .)", "Bash(git -P *)"]) { + expect(why(r)).toBe("only Loopstra commits, branches, merges and pushes; git is not allowed"); + } + for (const r of ["Bash(gitk *)", "Bash(gitleaks detect)", "Bash(git diff *)", "Bash(git -C . log *)", "Bash(git status:*)"]) { + expect(why(r)).toBeNull(); + } +}); diff --git a/tests/unit/config.test.ts b/tests/unit/config.test.ts index 2bc02bbd..27dee05c 100644 --- a/tests/unit/config.test.ts +++ b/tests/unit/config.test.ts @@ -216,6 +216,9 @@ test("agents settings are refused in plain words", () => { expect(problems({ teams: { r: { members: [] } } })).toEqual(["agents.teams.r.members: a team needs at least one member"]); expect(problems({ steps: { review: { teams: ["nope"] } } })).toEqual(["agents.steps.review.teams.0: there is no team nope under agents.teams"]); expect(problems({ sources: "mine" })).toEqual(["agents.sources: must be all or repo"]); + expect(problems({ steps: { review: { mcp_tools: ["mcp__x,Bash"] } } })).toEqual(["agents.steps.review.mcp_tools.0: must be an MCP rule, like mcp__github or mcp__github__get_issue"]); + // Editing and a bare Bash are fine on a step that changes files. + expect(problems({ steps: { build: { tools: ["Edit", "Write", "Bash"] } } })).toEqual([]); // Narrow commands and read-only git are fine anywhere; Bash(git diff *) is not history-changing. expect(problems({ steps: { review: { tools: ["Bash(npm run e2e *)", "Bash(git diff *)", "WebFetch"] } } })).toEqual([]); }); From f5530bdfd21e71941283e8096d7ca72954429c1d Mon Sep 17 00:00:00 2001 From: John Flavan Date: Mon, 26 Oct 2026 22:27:41 -0500 Subject: [PATCH 06/64] feat(agents): layer each step's agent setup and its prompt lines Co-Authored-By: Claude Opus 5.5 --- src/agents.ts | 84 +++++++++++++++++++++++++++++++++++++++ tests/unit/agents.test.ts | 66 +++++++++++++++++++++++++----- 2 files changed, 140 insertions(+), 10 deletions(-) diff --git a/src/agents.ts b/src/agents.ts index ca23c94f..8e38a377 100644 --- a/src/agents.ts +++ b/src/agents.ts @@ -1,3 +1,4 @@ +import type { Config, StepAgents } from "./config"; import { STEP_KEYS, type StepKey } from "./models"; /** How a team's members work: all at once and merged, or each tries and the best is kept. */ @@ -49,3 +50,86 @@ export function toolProblem(rule: string, readOnly: boolean): string | null { if (/^Bash(\(\*?\))?$/.test(rule)) return "a bare Bash rule would let it run anything"; return null; } + +/** The agents Claude Code ships with: they keep their own names in a prompt. */ +export const BUILTIN_AGENTS: ReadonlySet = new Set(["general-purpose", "Explore", "Plan", "claude", "statusline-setup"]); + +export interface Team { + name: string; + members: string[]; + way: TeamWay; +} + +/** Everything one step's session is set up with. */ +export interface AgentSetup { + sources: "all" | "repo"; + agent: string | null; + subagents: string[]; + teams: Team[]; + skills: string[]; + tools: string[]; + mcpTools: string[]; +} + +const once = (names: readonly string[]) => [...new Set(names)]; + +/** + * A step's setup: `all`, then the stage's skills, then the step's own, each name once. Chat steps get + * nothing from `all`. Fix, reconcile and revise run as the build step's agent. + */ +export function resolveSetup(agents: Config["agents"], step: StepKey, stageSkills: readonly string[]): AgentSetup { + const chat = CHAT_STEPS.has(step); + const empty: StepAgents = { subagents: [], teams: [], skills: [], tools: [], mcp_tools: [] }; + const all = chat ? empty : agents.all; + const own = agents.steps[step] ?? empty; + const agent = CONTINUES_BUILD.has(step) + ? (agents.steps["build"]?.agent ?? all.agent ?? null) + : (own.agent ?? all.agent ?? null); + return { + sources: agents.sources, + agent, + subagents: once([...all.subagents, ...own.subagents]), + teams: once([...all.teams, ...own.teams]).flatMap((name) => { + const t = agents.teams[name]; + return t ? [{ name, members: [...t.members], way: t.way }] : []; + }), + skills: once([...all.skills, ...stageSkills, ...own.skills]), + tools: once([...all.tools, ...own.tools]), + mcpTools: once([...all.mcp_tools, ...own.mcp_tools]), + }; +} + +/** True when the setup changes nothing about how a session runs. */ +export function isEmptySetup(s: AgentSetup): boolean { + return s.sources === "all" && s.agent === null && !s.subagents.length && !s.teams.length && !s.skills.length && !s.tools.length && !s.mcpTools.length; +} + +/** The name an agent file written for Loopstra is given inline: `loopstra-` and the name, colons as dashes. */ +export function inlineName(name: string): string { + return `loopstra-${name.replaceAll(":", "-")}`; +} + +/** The text put before the prompt template: skills, subagents and teams. "" when there are none. */ +export function setupLines(s: AgentSetup, describe: (name: string) => string | null): string { + const name = (n: string) => (BUILTIN_AGENTS.has(n) ? n : inlineName(n)); + const quoted = (n: string) => `\`${name(n)}\``; + const parts: string[] = []; + if (s.skills.length) parts.push(`Use these skills: ${s.skills.map((k) => `\`${k}\``).join(", ")}.`); + if (s.subagents.length) { + const list = s.subagents.map((n) => { + const d = describe(n); + return d ? `${quoted(n)} (${d})` : quoted(n); + }); + parts.push(`You can hand work to these subagents: ${list.join(", ")}.`); + } + for (const t of s.teams) { + const members = t.members.map(quoted).join(", "); + const how = t.way === "parallel" + ? "Give every member the same task at the same time, then weigh their answers." + : "Give each piece of work to the member that fits it best."; + parts.push(`Team \`${t.name}\`: ${members}. ${how}`); + } + if (!parts.length) return ""; + if (s.subagents.length || s.teams.length) parts.push("You still return your own structured output; subagents help, they do not replace it."); + return parts.join("\n\n") + "\n\n"; +} diff --git a/tests/unit/agents.test.ts b/tests/unit/agents.test.ts index 65354ebe..c54aae71 100644 --- a/tests/unit/agents.test.ts +++ b/tests/unit/agents.test.ts @@ -1,6 +1,6 @@ import { expect, test } from "bun:test"; -import { CHAT_STEPS, CONTINUES_BUILD, READ_ONLY_STEPS, toolProblem, type TeamWay } from "../../src/agents"; -import { validateConfig, type StepAgents } from "../../src/config"; +import { BUILTIN_AGENTS, CONTINUES_BUILD, inlineName, type Team, type TeamWay, CHAT_STEPS, READ_ONLY_STEPS, isEmptySetup, resolveSetup, setupLines, toolProblem, type AgentSetup } from "../../src/agents"; +import { validateConfig } from "../../src/config"; test("the step sets", () => { expect([...READ_ONLY_STEPS].some((k) => ["build", "reconcile", "fix", "revise"].includes(k))).toBe(false); @@ -18,14 +18,6 @@ test("toolProblem names why a rule is refused", () => { expect(toolProblem("Edit", false)).toBeNull(); }); -test("a team keeps its way", () => { - const way: TeamWay = "pick"; - const cfg = validateConfig({ version: 1, commands: { test: "bun test" }, agents: { teams: { r: { members: ["a"], way } } } }); - const all: StepAgents = cfg.agents.all; - expect(cfg.agents.teams.r).toEqual({ members: ["a"], way: "pick" }); - expect(all.tools).toEqual([]); -}); - test("toolProblem sees through git spellings, and leaves other tools alone", () => { const why = (r: string) => toolProblem(r, false); for (const r of ["Bash(git push:*)", "Bash(git push*)", "Bash(git commit*)", "Bash(git reset:*)", "Bash(git -C . push *)", "Bash(git -c k=v commit *)", "Bash( git push *)", "Bash(git --git-dir=x push)", "Bash(git --no-pager stash *)"]) { @@ -39,3 +31,57 @@ test("toolProblem sees through git spellings, and leaves other tools alone", () expect(why(r)).toBeNull(); } }); + +const agents = (over: object = {}) => validateConfig({ version: 1, commands: { test: "t" }, agents: over }).agents; + +test("all, then stage skills, then the step's own; each name once", () => { + const a = agents({ all: { skills: ["house"], mcp_tools: ["mcp__docs"] }, steps: { review: { skills: ["threat", "house"], subagents: ["Explore"], agent: "lead" } } }); + expect(resolveSetup(a, "review", ["stage-skill"])).toEqual({ sources: "all", agent: "lead", subagents: ["Explore"], teams: [], skills: ["house", "stage-skill", "threat"], tools: [], mcpTools: ["mcp__docs"] }); +}); + +test("fix, reconcile and revise run as the build's agent; chat steps never get all", () => { + const a = agents({ all: { skills: ["house"], tools: ["WebFetch"] }, steps: { build: { agent: "senior" }, orchestrator: { skills: ["chat-tone"] } } }); + expect(resolveSetup(a, "fix", []).agent).toBe("senior"); + expect(resolveSetup(a, "revise", []).skills).toEqual(["house"]); + expect(resolveSetup(a, "orchestrator", [])).toMatchObject({ agent: null, skills: ["chat-tone"], tools: [] }); +}); + +test("the step's own agent wins over all's; build's agent falls back to all's; chat steps ignore all's", () => { + const a = agents({ all: { agent: "general" }, steps: { review: { agent: "lead" } } }); + expect(resolveSetup(a, "review", []).agent).toBe("lead"); + expect(resolveSetup(a, "plan", []).agent).toBe("general"); + expect(resolveSetup(a, "fix", []).agent).toBe("general"); + expect(resolveSetup(a, "orchestrator", []).agent).toBeNull(); +}); + +test("teams resolve to their members; an empty setup is empty", () => { + const a = agents({ teams: { reviewers: { members: ["sec", "perf"], way: "parallel" } }, steps: { review: { teams: ["reviewers"] } } }); + expect(resolveSetup(a, "review", []).teams).toEqual([{ name: "reviewers", members: ["sec", "perf"], way: "parallel" }]); + expect(isEmptySetup(resolveSetup(agents(), "build", []))).toBe(true); + expect(isEmptySetup(resolveSetup(agents({ sources: "repo" }), "build", []))).toBe(false); +}); + +test("prompt lines: skills as today, then subagents and teams by their inline names", () => { + const s = { sources: "all", agent: null, subagents: ["Explore", "test-writer"], teams: [{ name: "reviewers", members: ["sec", "perf"], way: "parallel" }], skills: ["a", "b"], tools: [], mcpTools: [] } as AgentSetup; + const lines = setupLines(s, (n) => (n === "test-writer" ? "Writes tests" : null)); + expect(lines).toStartWith("Use these skills: `a`, `b`.\n\n"); + expect(lines).toContain("`Explore`"); + expect(lines).toContain("`loopstra-test-writer` (Writes tests)"); + expect(lines).toContain("`loopstra-sec`, `loopstra-perf`"); + expect(lines).toContain("same task"); + expect(lines).toContain("structured output"); + expect(lines).toEndWith("\n\n"); + expect(setupLines({ ...s, subagents: [], teams: [] }, () => null)).toBe("Use these skills: `a`, `b`.\n\n"); + expect(setupLines({ ...s, skills: [], subagents: [], teams: [] }, () => null)).toBe(""); + const pick = setupLines({ ...s, subagents: [], teams: [{ name: "t", members: ["x"], way: "pick" }] }, () => null); + expect(pick).toContain("fits"); +}); + +test("inline names: built-ins keep theirs, others get the loopstra prefix", () => { + const way: TeamWay = "pick"; + const team: Team = { name: "t", members: ["x"], way }; + expect(team.way).toBe("pick"); + expect(inlineName("a:b")).toBe("loopstra-a-b"); + expect(BUILTIN_AGENTS.has("Explore")).toBe(true); + expect(BUILTIN_AGENTS.has("test-writer")).toBe(false); +}); From e7157d64a2f7f560ea90fcf5645e2fce6b5858a5 Mon Sep 17 00:00:00 2001 From: John Flavan Date: Mon, 26 Oct 2026 22:32:18 -0500 Subject: [PATCH 07/64] feat(agents): find and parse agent and skill definitions, build inline agents Co-Authored-By: Claude Opus 5.5 --- src/agent-files.ts | 128 +++++++++++++++++++++++++++++++++ tests/helpers.ts | 6 ++ tests/unit/agent-files.test.ts | 57 +++++++++++++++ 3 files changed, 191 insertions(+) create mode 100644 src/agent-files.ts create mode 100644 tests/unit/agent-files.test.ts diff --git a/src/agent-files.ts b/src/agent-files.ts new file mode 100644 index 00000000..babade7f --- /dev/null +++ b/src/agent-files.ts @@ -0,0 +1,128 @@ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { parse } from "yaml"; +import { inlineName } from "./agents"; + +/** An agent definition read from a Claude Code agent file. */ +export interface AgentDefinition { + name: string; + description: string; + prompt: string; + tools: string[] | null; + model: string | null; + skills: string[] | null; + /** Frontmatter keys Loopstra does not carry over, in file order. */ + dropped: string[]; + from: "repo" | "personal" | "plugin"; + path: string; +} + +/** Where to look: `repo` is a checkout of main (the step's snapshot, or the root). */ +export interface Where { repo: string; home: string; sources: "all" | "repo" } + +const KEPT = new Set(["name", "description", "tools", "model", "skills", "color"]); +const FRONTMATTER = /^---\r?\n(?:([\s\S]*?)\r?\n)?---[ \t]*(?:\r?\n([\s\S]*))?$/; + +/** A comma-separated string or a list as a list of names; null when absent or empty. */ +function names(v: unknown): string[] | null { + const list = typeof v === "string" ? v.split(",") : Array.isArray(v) ? v : []; + const out = list.filter((x): x is string | number => typeof x === "string" || typeof x === "number").map((x) => String(x).trim()).filter(Boolean); + return out.length ? out : null; +} + +export function parseAgentFile(name: string, text: string, from: AgentDefinition["from"], path: string): AgentDefinition { + const m = FRONTMATTER.exec(text); + let front: Record = {}; + if (m) { + try { + const v: unknown = parse(m[1] ?? ""); + if (v && typeof v === "object" && !Array.isArray(v)) front = v as Record; + } catch { /* unreadable frontmatter counts as none */ } + } + const body = m ? (m[2] ?? "") : text; + return { + name, + description: typeof front.description === "string" ? front.description.trim() : "", + prompt: body.trim(), + tools: names(front.tools), + model: typeof front.model === "string" && front.model.trim() ? front.model.trim() : null, + skills: names(front.skills), + dropped: Object.keys(front).filter((k) => !KEPT.has(k)), + from, + path, + }; +} + +function readText(path: string): string | null { + try { return readFileSync(path, "utf8"); } catch { return null; } +} + +function readJson(path: string): Record | null { + const text = readText(path); + if (text === null) return null; + try { + const v: unknown = JSON.parse(text); + return v && typeof v === "object" ? (v as Record) : null; + } catch { return null; } +} + +/** The install folder of an installed and enabled plugin, by its short name (before the `@`). */ +function pluginRoot(plugin: string, home: string): string | null { + const installed = readJson(join(home, ".claude/plugins/installed_plugins.json"))?.plugins; + const enabled = readJson(join(home, ".claude/settings.json"))?.enabledPlugins; + if (!installed || typeof installed !== "object" || !enabled || typeof enabled !== "object") return null; + for (const [key, entries] of Object.entries(installed)) { + if (!key.startsWith(`${plugin}@`) || (enabled as Record)[key] !== true || !Array.isArray(entries)) continue; + const path = (entries[0] as { installPath?: unknown } | undefined)?.installPath; + if (typeof path === "string") return path; + } + return null; +} + +/** The places a definition may live, in lookup order, for a file under `//…`. */ +function candidates(name: string, where: Where, file: (n: string) => string, dir: string): { from: AgentDefinition["from"]; path: string }[] { + const colon = name.indexOf(":"); + const local = colon < 0 ? name : null; + if (name.includes("/") || name.includes("\\") || name.includes("..")) return []; + const out: { from: AgentDefinition["from"]; path: string }[] = []; + if (local !== null) out.push({ from: "repo", path: join(where.repo, ".claude", dir, file(local)) }); + if (where.sources !== "all") return out; + if (local !== null) out.push({ from: "personal", path: join(where.home, ".claude", dir, file(local)) }); + else { + const root = pluginRoot(name.slice(0, colon), where.home); + if (root) out.push({ from: "plugin", path: join(root, dir, file(name.slice(colon + 1))) }); + } + return out; +} + +export async function findAgent(name: string, where: Where): Promise { + for (const c of candidates(name, where, (n) => `${n}.md`, "agents")) { + const text = readText(c.path); + if (text !== null) return parseAgentFile(name, text, c.from, c.path); + } + return null; +} + +export async function findSkill(name: string, where: Where): Promise { + for (const c of candidates(name, where, (n) => join(n, "SKILL.md"), "skills")) { + if (readText(c.path) !== null) return c.from; + } + return null; +} + +export interface InlineAgent { description: string; prompt: string; tools?: string[]; model?: string; skills?: string[] } + +/** The `--agents` JSON: the main agent, then the others, keyed by their Loopstra names. */ +export function inlineAgents(main: AgentDefinition | null, others: AgentDefinition[]): Record { + const out: Record = {}; + const entry = (d: AgentDefinition, isMain: boolean): InlineAgent => { + const e: InlineAgent = { description: d.description, prompt: d.prompt }; + if (d.tools) e.tools = isMain && !d.tools.includes("StructuredOutput") ? [...d.tools, "StructuredOutput"] : d.tools; + if (d.model && !isMain) e.model = d.model; + if (d.skills) e.skills = d.skills; + return e; + }; + if (main) out[inlineName(main.name)] = entry(main, true); + for (const d of others) out[inlineName(d.name)] = entry(d, false); + return out; +} diff --git a/tests/helpers.ts b/tests/helpers.ts index 651d7988..cdba61fb 100644 --- a/tests/helpers.ts +++ b/tests/helpers.ts @@ -22,6 +22,12 @@ export function tempDir(prefix = "loopstra-"): { path: string; cleanup: () => vo return { path, cleanup: () => rmSync(path, { recursive: true, force: true }) }; } +/** Writes a file, making its folder first. */ +export function write(path: string, text: string): void { + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, text); +} + /** A shipped fake-claude fixture's text. */ export function fixture(name: string): string { return readFileSync(join(dirname(FAKE_CLAUDE), "fixtures", `${name}.jsonl`), "utf8"); diff --git a/tests/unit/agent-files.test.ts b/tests/unit/agent-files.test.ts new file mode 100644 index 00000000..a03a3a8a --- /dev/null +++ b/tests/unit/agent-files.test.ts @@ -0,0 +1,57 @@ +import { expect, test } from "bun:test"; +import { join } from "node:path"; +import { findAgent, findSkill, inlineAgents, parseAgentFile } from "../../src/agent-files"; +import { tempDir, write } from "../helpers"; + +test("an agent file parses with CRLF, a tools string, and unknown keys dropped", () => { + const d = parseAgentFile("lead", "---\r\nname: lead\r\ndescription: Leads\r\ntools: Read, Grep\r\nmodel: opus\r\npermissionMode: bypassPermissions\r\nhooks: {}\r\n---\r\nYou lead.\r\n", "repo", "/r/.claude/agents/lead.md"); + expect(d).toMatchObject({ description: "Leads", prompt: "You lead.", tools: ["Read", "Grep"], model: "opus", dropped: ["permissionMode", "hooks"] }); + expect(parseAgentFile("x", "Just a prompt.\n", "repo", "p")).toMatchObject({ description: "", prompt: "Just a prompt.", tools: null, model: null, dropped: [] }); +}); + +test("findAgent looks in the repo, then the user's folder, then installed and enabled plugins", async () => { + const t = tempDir(); const repo = join(t.path, "repo"); const home = join(t.path, "home"); + write(join(repo, ".claude/agents/lead.md"), "---\ndescription: repo lead\n---\nR"); + write(join(home, ".claude/agents/lead.md"), "---\ndescription: home lead\n---\nH"); + write(join(home, ".claude/agents/mine.md"), "---\ndescription: mine\n---\nM"); + const plug = join(home, "plugins-cache/sp"); + write(join(plug, "agents/code-reviewer.md"), "---\ndescription: cr\n---\nC"); + write(join(home, ".claude/plugins/installed_plugins.json"), JSON.stringify({ version: 2, plugins: { "sp@market": [{ scope: "user", installPath: plug }] } })); + write(join(home, ".claude/settings.json"), JSON.stringify({ enabledPlugins: { "sp@market": true } })); + const all = { repo, home, sources: "all" as const }; + expect((await findAgent("lead", all))?.description).toBe("repo lead"); + expect((await findAgent("mine", all))?.from).toBe("personal"); + expect((await findAgent("sp:code-reviewer", all))?.from).toBe("plugin"); + expect(await findAgent("mine", { ...all, sources: "repo" })).toBeNull(); + write(join(home, ".claude/settings.json"), JSON.stringify({ enabledPlugins: { "sp@market": false } })); + expect(await findAgent("sp:code-reviewer", all)).toBeNull(); + t.cleanup(); +}); + +test("findSkill looks in the same places, and a missing or broken file is not a throw", async () => { + const t = tempDir(); const repo = join(t.path, "repo"); const home = join(t.path, "home"); + write(join(repo, ".claude/skills/a/SKILL.md"), "A"); + write(join(home, ".claude/skills/b/SKILL.md"), "B"); + const plug = join(home, "cache/sp"); + write(join(plug, "skills/tdd/SKILL.md"), "T"); + write(join(home, ".claude/plugins/installed_plugins.json"), JSON.stringify({ plugins: { "sp@market": [{ installPath: plug }] } })); + write(join(home, ".claude/settings.json"), "{ not json"); + const all = { repo, home, sources: "all" as const }; + expect(await findSkill("a", all)).toBe("repo"); + expect(await findSkill("b", all)).toBe("personal"); + expect(await findSkill("sp:tdd", all)).toBeNull(); + write(join(home, ".claude/settings.json"), JSON.stringify({ enabledPlugins: { "sp@market": true } })); + expect(await findSkill("sp:tdd", all)).toBe("plugin"); + expect(await findSkill("b", { ...all, sources: "repo" })).toBeNull(); + expect(await findSkill("nope", all)).toBeNull(); + t.cleanup(); +}); + +test("inline agents: loopstra names, StructuredOutput for the main agent, its model left to the step", () => { + const lead = parseAgentFile("lead", "---\ndescription: L\ntools: [Read]\nmodel: opus\n---\nP", "repo", "p"); + const sec = parseAgentFile("sec", "---\ndescription: S\nmodel: haiku\n---\nQ", "repo", "p"); + expect(inlineAgents(lead, [sec])).toEqual({ + "loopstra-lead": { description: "L", prompt: "P", tools: ["Read", "StructuredOutput"] }, + "loopstra-sec": { description: "S", prompt: "Q", model: "haiku" }, + }); +}); From ba5c63b916243df8b4ae01001ab5a2c7364efed7 Mon Sep 17 00:00:00 2001 From: John Flavan Date: Mon, 26 Oct 2026 22:34:43 -0500 Subject: [PATCH 08/64] test(agents): pin inline tools, skills and unsafe names; document exports Co-Authored-By: Claude Opus 5.5 --- src/agent-files.ts | 4 ++++ tests/unit/agent-files.test.ts | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/agent-files.ts b/src/agent-files.ts index babade7f..4363fa0b 100644 --- a/src/agent-files.ts +++ b/src/agent-files.ts @@ -30,6 +30,7 @@ function names(v: unknown): string[] | null { return out.length ? out : null; } +/** Parses an agent file: frontmatter via yaml (CRLF fine, none is fine), the body as the prompt. */ export function parseAgentFile(name: string, text: string, from: AgentDefinition["from"], path: string): AgentDefinition { const m = FRONTMATTER.exec(text); let front: Record = {}; @@ -95,6 +96,7 @@ function candidates(name: string, where: Where, file: (n: string) => string, dir return out; } +/** The agent definition for a name (`agent` or `plugin:agent`), or null when none is found. Built-in agents are the caller's to skip. */ export async function findAgent(name: string, where: Where): Promise { for (const c of candidates(name, where, (n) => `${n}.md`, "agents")) { const text = readText(c.path); @@ -103,6 +105,7 @@ export async function findAgent(name: string, where: Where): Promise { for (const c of candidates(name, where, (n) => join(n, "SKILL.md"), "skills")) { if (readText(c.path) !== null) return c.from; @@ -110,6 +113,7 @@ export async function findSkill(name: string, where: Where): Promise { + const bare = parseAgentFile("bare", "---\ndescription: B\nskills: [tdd]\n---\nP", "repo", "p"); + expect(inlineAgents(bare, [])["loopstra-bare"]).toEqual({ description: "B", prompt: "P", skills: ["tdd"] }); + expect("tools" in inlineAgents(bare, [])["loopstra-bare"]!).toBe(false); + const has = parseAgentFile("has", "---\ndescription: H\ntools: Read, StructuredOutput\n---\nP", "repo", "p"); + expect(inlineAgents(has, [])["loopstra-has"]!.tools).toEqual(["Read", "StructuredOutput"]); +}); + +test("findAgent finds nothing for names that climb out of the agent folders", async () => { + const t = tempDir(); const repo = join(t.path, "repo"); const home = join(t.path, "home"); + write(join(repo, "secret.md"), "---\ndescription: s\n---\nS"); + const all = { repo, home, sources: "all" as const }; + for (const n of ["../../secret", "a/b", "a\\b", "..", "sp:../x"]) expect(await findAgent(n, all)).toBeNull(); + t.cleanup(); +}); From 4aa8389369962445708d091c885e2f04370a45b4 Mon Sep 17 00:00:00 2001 From: John Flavan Date: Mon, 26 Oct 2026 22:38:59 -0500 Subject: [PATCH 09/64] feat(agents): check that every named agent and skill exists Co-Authored-By: Claude Opus 5.5 --- src/agent-files.ts | 48 +++++++++++++++++++++++++++++++++- tests/unit/agent-files.test.ts | 16 +++++++++++- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/src/agent-files.ts b/src/agent-files.ts index 4363fa0b..3e4a2574 100644 --- a/src/agent-files.ts +++ b/src/agent-files.ts @@ -1,7 +1,9 @@ import { readFileSync } from "node:fs"; import { join } from "node:path"; import { parse } from "yaml"; -import { inlineName } from "./agents"; +import { BUILTIN_AGENTS, inlineName, resolveSetup } from "./agents"; +import type { Config } from "./config"; +import { STEP_KEYS, STEPS, type StepKey } from "./models"; /** An agent definition read from a Claude Code agent file. */ export interface AgentDefinition { @@ -113,6 +115,50 @@ export async function findSkill(name: string, where: Where): Promise { + const problems: SetupProblem[] = []; + const places = `on ${cfg.main_branch}${where.sources === "all" ? " or on this machine" : ""}`; + for (const step of STEP_KEYS) { + const group = STEPS.find((s) => s.key === step)!.group; + const stageSkills = group === "chat" ? [] : cfg.stages[group].skills; + const setup = resolveSetup(cfg.agents, step, stageSkills); + const roles: { name: string; role: string }[] = [ + ...(setup.agent ? [{ name: setup.agent, role: `the agent ${step} runs as` }] : []), + ...setup.subagents.map((name) => ({ name, role: `a subagent of ${step}` })), + ...setup.teams.flatMap((t) => t.members.map((name) => ({ name, role: `in the team ${t.name} of ${step}` }))), + ]; + const seen = new Set(); + for (const { name, role } of roles) { + if (BUILTIN_AGENTS.has(name) || seen.has(name)) continue; + seen.add(name); + const def = await findAgent(name, where); + if (!def) { + problems.push({ level: "fail", step, name, text: `\`${name}\`, ${role}, is not in .claude/agents/ ${places}. Add it with \`loopstra setup agents\`, or commit it.` }); + } else if (def.from !== "repo") { + problems.push({ level: "warn", step, name, text: `\`${name}\` (${step}) is only on this machine, so a loop running elsewhere will not have it.` }); + } else if (def.dropped.length) { + problems.push({ level: "warn", step, name, text: `${def.path}: Loopstra leaves out ${def.dropped.join(", ")} when it runs this agent.` }); + } + } + for (const name of setup.skills) { + const from = await findSkill(name, where); + if (from === null) { + problems.push({ level: "warn", step, name, text: `The skill \`${name}\` (${step}) is not in .claude/skills/${where.sources === "all" ? " or on this machine" : ""}; it may be built in to Claude Code. If a session does not load it, the step stops and says so.` }); + } else if (from !== "repo") { + problems.push({ level: "warn", step, name, text: `\`${name}\` (${step}) is only on this machine, so a loop running elsewhere will not have it.` }); + } + } + } + return problems; +} + /** One entry of the `--agents` JSON. */ export interface InlineAgent { description: string; prompt: string; tools?: string[]; model?: string; skills?: string[] } diff --git a/tests/unit/agent-files.test.ts b/tests/unit/agent-files.test.ts index 2900744a..e43bb74f 100644 --- a/tests/unit/agent-files.test.ts +++ b/tests/unit/agent-files.test.ts @@ -1,6 +1,7 @@ import { expect, test } from "bun:test"; import { join } from "node:path"; -import { findAgent, findSkill, inlineAgents, parseAgentFile } from "../../src/agent-files"; +import { checkAgentSetup, findAgent, findSkill, inlineAgents, parseAgentFile } from "../../src/agent-files"; +import { validateConfig } from "../../src/config"; import { tempDir, write } from "../helpers"; test("an agent file parses with CRLF, a tools string, and unknown keys dropped", () => { @@ -71,3 +72,16 @@ test("findAgent finds nothing for names that climb out of the agent folders", as for (const n of ["../../secret", "a/b", "a\\b", "..", "sp:../x"]) expect(await findAgent(n, all)).toBeNull(); t.cleanup(); }); + +test("checkAgentSetup: missing agents fail, machine-only ones and unknown skills warn", async () => { + const t = tempDir(); const repo = join(t.path, "repo"); const home = join(t.path, "home"); + write(join(repo, ".claude/agents/lead.md"), "---\ndescription: L\nhooks: {}\n---\nP"); + write(join(home, ".claude/agents/mine.md"), "M"); + const cfg = validateConfig({ version: 1, commands: { test: "t" }, agents: { steps: { review: { agent: "lead", subagents: ["mine", "Explore", "gone"], skills: ["pdf"] } } } }); + const p = await checkAgentSetup(cfg, { repo, home, sources: "all" }); + expect(p.map((x) => [x.level, x.step, x.name])).toEqual([ + ["warn", "review", "lead"], ["warn", "review", "mine"], ["fail", "review", "gone"], ["warn", "review", "pdf"], + ]); + expect(p[2]!.text).toContain("Add it with `loopstra setup agents`"); + t.cleanup(); +}); From cb807f8cf5ae30927f237a1f4c1e127ed12d1654 Mon Sep 17 00:00:00 2001 From: John Flavan Date: Mon, 26 Oct 2026 22:46:45 -0500 Subject: [PATCH 10/64] feat(claude): agent flags and the init check, with failure reason setup runPhase passes --agents, --agent, --setting-sources, --add-dir and --forward-subagent-text when set (after the tool lists, before --settings). checkInit judges the session's first init event that lists agents or skills; a problem ends the session at once as a "setup" failure. A stop or a cancel still wins; setup is not an outage and is not retried. Co-Authored-By: Claude Opus 5.5 --- src/claude.ts | 47 ++++++++- src/phases.ts | 1 + tests/fake-claude/fixtures/init-lists.jsonl | 3 + tests/unit/claude-run.test.ts | 104 +++++++++++++++++++- 4 files changed, 153 insertions(+), 2 deletions(-) create mode 100644 tests/fake-claude/fixtures/init-lists.jsonl diff --git a/src/claude.ts b/src/claude.ts index 372237a4..6f3ef044 100644 --- a/src/claude.ts +++ b/src/claude.ts @@ -116,8 +116,9 @@ export type PermissionMode = "default" | "manual" | "plan" | "acceptEdits" | "do * - missing-prompt: the phase's prompt file is missing (raised by agentPhase) * - environment: the assistant could not be used at all (see ENVIRONMENT_PATTERNS); not the agent's doing * - model: Claude Code does not accept the model name (see MODEL_PATTERN); a setting to change, not an outage + * - setup: the step's agent setup is not complete on this machine (see RunPhaseInput.checkInit); not an outage */ -export type FailureReason = "not-started" | "timeout" | "budget" | "no-session" | "crash" | "invalid-envelope" | "agent-fail" | "missing-prompt" | "environment" | "model"; +export type FailureReason = "not-started" | "timeout" | "budget" | "no-session" | "crash" | "invalid-envelope" | "agent-fail" | "missing-prompt" | "environment" | "model" | "setup"; /** * Text that means the assistant itself could not be used, so a failed session is the environment's @@ -173,6 +174,21 @@ export interface RunPhaseInput { disallowedTools?: string[]; /** Extra Claude Code settings for this session, as JSON (`claude --settings`): the test hook. */ settings?: string; + /** Subagents defined for this session only, by name (`claude --agents`); none when empty. */ + agents?: Record; + /** The agent the session itself runs as (`claude --agent`). */ + agent?: string; + /** Which settings Claude Code loads (`claude --setting-sources`, e.g. "user,project"). */ + settingSources?: string; + /** More directories the session's tools may reach (`claude --add-dir`, once per directory). */ + addDirs?: string[]; + /** Subagents' text is forwarded into the session's stream (`claude --forward-subagent-text`). */ + forwardSubagentText?: boolean; + /** + * Judges the session's first init event that lists its agents or skills (never one that lists neither, and + * not again once one passed): a string ends the session at once as a "setup" failure, with it as the detail. + */ + checkInit?: (init: { agents?: string[]; skills?: string[] }) => string | null; timeoutMs: number; /** The session's spending cap. Absent or Infinity: no --max-budget-usd, and the timeout is the only stop. */ maxBudgetUsd?: number; @@ -406,6 +422,11 @@ function claudeCommand(exe: string, input: RunPhaseInput): string[] { ]; if (input.allowedTools.length) args.push("--allowedTools", input.allowedTools.join(",")); if (input.disallowedTools?.length) args.push("--disallowedTools", input.disallowedTools.join(",")); + if (input.agents && Object.keys(input.agents).length) args.push("--agents", JSON.stringify(input.agents)); + if (input.agent) args.push("--agent", input.agent); + if (input.settingSources !== undefined) args.push("--setting-sources", input.settingSources); + for (const dir of input.addDirs ?? []) args.push("--add-dir", dir); + if (input.forwardSubagentText) args.push("--forward-subagent-text"); if (input.settings) args.push("--settings", input.settings); // Every user message the session takes is echoed, so the runner knows which result answers it. if (input.live) args.push("--input-format", "stream-json", ...(input.live.replay === false ? [] : ["--replay-user-messages"])); @@ -442,6 +463,9 @@ async function runSession(input: RunPhaseInput, live: LiveSession | null): Promi let r: SpawnResult; // How many times the session was sent again after it stopped at a notification's result. let resends = 0; + // Whether an init event that lists agents or skills was judged (see checkInit), and what it found missing. + let initJudged = false; + let setupProblem: string | null = null; for (;;) { if (live?.stoppedBy) throw new SessionStopped(live.stoppedBy, sessionNow()); // spawnBounded checks these too, but what it throws cannot name the session a resend resumes. @@ -473,6 +497,15 @@ async function runSession(input: RunPhaseInput, live: LiveSession | null): Promi if (!e) return false; if (live && e.session_id) live.sessionId = e.session_id; if (live && e.type === "user" && e.isReplay === true) live.replays++; + if (input.checkInit && !initJudged && e.type === "system" && e.subtype === "init") { + const lists = initLists(e); + if (lists) { + initJudged = true; + setupProblem = input.checkInit(lists); + // The session lacks part of its setup: reading ends here, and so does the session. + if (setupProblem !== null) return true; + } + } // Any event of the process while it waits for an echo starts the wait again; once nothing waits // for one (the echo came), the wait is over, so a quiet stretch later in that turn never closes stdin. if (echoTimer !== undefined && !echoGaveUp) { @@ -497,6 +530,8 @@ async function runSession(input: RunPhaseInput, live: LiveSession | null): Promi clearTimeout(echoTimer); live?.done(); if (live?.stoppedBy) throw new SessionStopped(live.stoppedBy, sessionNow()); + // A failed init check ends the session (a stop or a cancel still wins: see below). + if (setupProblem !== null) break; if (live && live.redirectTo !== null && !r.stopped && !input.signal?.aborted && r.started) { // What the killed process spent counts toward the cap, as a resend's does: its newest result's // running total, or about its minutes when it gave none (never more than it could spend). @@ -533,9 +568,19 @@ async function runSession(input: RunPhaseInput, live: LiveSession | null): Promi collector = new StreamCollector(); resends++; } + if (setupProblem !== null && !r.stopped && !r.cancelled) return failure(sends.total(collector.finish()), { started, nudged }, "setup", setupProblem, r.code, r.err); return classify(sends.total(collector.finish()), r, { started, timeoutMs: input.timeoutMs, resume, nudged }); } +/** An init event's agent and skill names, each list only when the event has it; null when it has neither. */ +function initLists(e: StreamEvent): { agents?: string[]; skills?: string[] } | null { + const names = (v: unknown) => (Array.isArray(v) ? v.filter((x): x is string => typeof x === "string") : undefined); + const agents = names(e.agents); + const skills = names(e.skills); + if (!agents && !skills) return null; + return { ...(agents ? { agents } : {}), ...(skills ? { skills } : {}) }; +} + /** What classify needs besides the session's result and its last process's. */ interface ClassifyContext { /** When runPhase started (Date.now()): durationMs counts from it. */ diff --git a/src/phases.ts b/src/phases.ts index 9ec60ba5..80d0f7aa 100644 --- a/src/phases.ts +++ b/src/phases.ts @@ -165,6 +165,7 @@ export function ownerNote(reason: BlockingReason): string { case "model": return "Claude Code did not accept this step's model. Change it in Settings or `loopstra setup models`, then retry."; case "invalid-envelope": return "The assistant's report could not be read."; case "missing-prompt": return "A prompt file for this step is missing."; + case "setup": return "This step's agent setup is not complete on this machine."; // The agent's own summary is not written for the owner; it goes to the trace. case "agent-fail": return "The assistant reported it could not finish this step."; } diff --git a/tests/fake-claude/fixtures/init-lists.jsonl b/tests/fake-claude/fixtures/init-lists.jsonl new file mode 100644 index 00000000..e7cf03d0 --- /dev/null +++ b/tests/fake-claude/fixtures/init-lists.jsonl @@ -0,0 +1,3 @@ +{"type":"system","subtype":"init","session_id":"fake-session-1","cwd":"/","tools":[],"model":"fake","skills":["a"],"agents":["loopstra-x"]} +{"type":"assistant","message":{"role":"assistant","content":[{"type":"tool_use","name":"StructuredOutput","input":{"status":"success","summary":"did the thing","priority":"normal","question":""}}]},"session_id":"fake-session-1"} +{"type":"result","subtype":"success","is_error":false,"session_id":"fake-session-1","total_cost_usd":0.01,"usage":{"input_tokens":10,"output_tokens":20},"structured_output":{"status":"success","summary":"did the thing","priority":"normal","question":""},"result":"ok"} diff --git a/tests/unit/claude-run.test.ts b/tests/unit/claude-run.test.ts index 6ae4143e..14bc7748 100644 --- a/tests/unit/claude-run.test.ts +++ b/tests/unit/claude-run.test.ts @@ -4,7 +4,7 @@ import { join } from "node:path"; import { ENVIRONMENT_PATTERNS, FAKE_CLAUDE_ENV, runPhase, unavailable, type SessionControl } from "../../src/claude"; import { MEANWHILE_HEADING } from "../../src/console/steerable"; import { MIN_SESSION_USD, USD_PER_MINUTE } from "../../src/budget"; -import { SessionStopped } from "../../src/stop"; +import { Cancelled, SessionStopped } from "../../src/stop"; import { until } from "../console-helpers"; import { FAKE_CLAUDE as FAKE, fixture, fixtureDir, tempDir } from "../helpers"; @@ -216,6 +216,108 @@ describe("runPhase", () => { } finally { t.cleanup(); } }); + describe("agent setup", () => { + const base = (cwd: string) => ({ cwd, schema: {}, model: "haiku", permissionMode: "default" as const, allowedTools: ["Read"], settings: "{}", timeoutMs: 10_000, maxBudgetUsd: 1, executable: FAKE }); + const AGENT_FLAGS = ["--agents", "--agent", "--setting-sources", "--add-dir", "--forward-subagent-text"]; + /** The init-lists fixture's init line, with the session id `id`. */ + const listsInit = (id: string) => fixture("init-lists").split("\n")[0]!.replace("fake-session-1", id); + + test("agent flags reach the CLI only when set, after the tool lists and before --settings", async () => { + const t = tempDir(); + try { + const plain = join(t.path, "plain.json"); + await runPhase({ ...base(t.path), prompt: "FIXTURE:simple-success", env: { LOOPSTRA_FAKE_ARGS: plain } }); + const without = ((await Bun.file(plain).json()) as { args: string[] }).args; + for (const flag of AGENT_FLAGS) expect(without).not.toContain(flag); + // An empty agents map is no --agents either. + const empty = join(t.path, "empty.json"); + await runPhase({ ...base(t.path), prompt: "FIXTURE:simple-success", agents: {}, addDirs: [], forwardSubagentText: false, env: { LOOPSTRA_FAKE_ARGS: empty } }); + const none = ((await Bun.file(empty).json()) as { args: string[] }).args; + for (const flag of AGENT_FLAGS) expect(none).not.toContain(flag); + + const set = join(t.path, "set.json"); + const agents = { "loopstra-x": { description: "d", prompt: "p" } }; + await runPhase({ ...base(t.path), prompt: "FIXTURE:simple-success", agents, agent: "loopstra-x", settingSources: "project", addDirs: ["/snap", "/other"], forwardSubagentText: true, env: { LOOPSTRA_FAKE_ARGS: set } }); + const args = ((await Bun.file(set).json()) as { args: string[] }).args; + const flags = ["--agents", JSON.stringify(agents), "--agent", "loopstra-x", "--setting-sources", "project", "--add-dir", "/snap", "--add-dir", "/other", "--forward-subagent-text"]; + const at = args.indexOf("--agents"); + expect(args.slice(at, at + flags.length)).toEqual(flags); + expect(at).toBeGreaterThan(args.indexOf("--allowedTools")); + expect(args.indexOf("--settings")).toBe(at + flags.length); + } finally { t.cleanup(); } + }); + + test("an init event that lacks a named skill ends the session as a setup failure", async () => { + const t = tempDir(); + try { + const seen: Array<{ agents?: string[]; skills?: string[] }> = []; + const r = await runPhase({ ...base(t.path), prompt: "FIXTURE:init-lists", checkInit: (i) => { + seen.push(i); + return i.skills?.includes("b") ? null : "this session did not load the skill `b`"; + } }); + expect(r).toMatchObject({ ok: false, reason: "setup", detail: "this session did not load the skill `b`", sessionId: "fake-session-1" }); + expect(seen).toEqual([{ agents: ["loopstra-x"], skills: ["a"] }]); + expect(unavailable("setup")).toBe(false); + } finally { t.cleanup(); } + }); + + test("a failed init check stops reading there: the session does not run on to its deadline", async () => { + const t = tempDir(); + // After its init line the session would wait for a file that never comes. + const fixtures = fixtureDir({ fix: [listsInit("held"), JSON.stringify({ type: "fake_action", wait_for: join(t.path, "never") })].join("\n") }); + try { + const started = Date.now(); + const r = await runPhase({ ...base(t.path), prompt: "go", timeoutMs: 30_000, exitGraceMs: 200, + env: { LOOPSTRA_PHASE: "fix", LOOPSTRA_FAKE_FIXTURE_DIR: fixtures.path }, checkInit: () => "missing" }); + expect(r).toMatchObject({ ok: false, reason: "setup", detail: "missing", sessionId: "held" }); + expect(Date.now() - started).toBeLessThan(15_000); + } finally { fixtures.cleanup(); t.cleanup(); } + }, 60_000); + + test("an init event without lists is not judged", async () => { + const t = tempDir(); + try { + let calls = 0; + // The default fixture's init has no agents or skills. + const r = await runPhase({ ...base(t.path), prompt: "FIXTURE:simple-success", checkInit: () => { calls++; return "would fail"; } }); + expect(r.ok).toBe(true); + expect(calls).toBe(0); + } finally { t.cleanup(); } + }); + + test("an init check that passed is not made again when the session is sent again", async () => { + const t = tempDir(); + // The first send finishes without structured output, so the session is asked again (resumed) for it. + const [, ...noEnvelope] = fixture("no-envelope").replaceAll("no-envelope-session", "lists-session").split("\n"); + const fixtures = fixtureDir({ "fix-1": [listsInit("lists-session"), ...noEnvelope].join("\n"), "fix-2": fixture("init-lists") }); + try { + const callsFile = join(t.path, "calls.jsonl"); + let calls = 0; + const r = await runPhase({ ...base(t.path), prompt: "go", env: { LOOPSTRA_PHASE: "fix", LOOPSTRA_FAKE_FIXTURE_DIR: fixtures.path, LOOPSTRA_FAKE_CALLS: callsFile }, + checkInit: () => (++calls === 1 ? null : "checked again") }); + expect(r.ok).toBe(true); + expect(calls).toBe(1); + const sent = (await Bun.file(callsFile).text()).trim().split("\n").map((l) => JSON.parse(l) as string[]); + expect(sent[1]).toEqual(expect.arrayContaining(["--resume", "lists-session"])); + } finally { fixtures.cleanup(); t.cleanup(); } + }); + + test("a stop or a cancel still wins over a failed init check", async () => { + const t = tempDir(); + try { + // A person stops the live session as its init event is judged. + let ctl: SessionControl | null = null; + const stopped = await runPhase({ ...base(t.path), prompt: "FIXTURE:init-lists", live: { attach: (c) => { ctl = c; return () => {}; } }, + checkInit: () => { ctl!.stop("John"); return "missing"; } }).then(() => null, (e: unknown) => e); + expect(stopped).toBeInstanceOf(SessionStopped); + const abort = new AbortController(); + const cancelled = await runPhase({ ...base(t.path), prompt: "FIXTURE:init-lists", signal: abort.signal, + checkInit: () => { abort.abort(); return "missing"; } }).then(() => null, (e: unknown) => e); + expect(cancelled).toBeInstanceOf(Cancelled); + } finally { t.cleanup(); } + }, 60_000); + }); + test("sessions run with background tasks turned off", async () => { const t = tempDir(); const argsFile = join(t.path, "args.json"); From c0adc35d839d9f3638479fbb673472d82a2acfa3 Mon Sep 17 00:00:00 2001 From: John Flavan Date: Mon, 26 Oct 2026 22:57:44 -0500 Subject: [PATCH 11/64] feat(phases): run each step with its agent setup Each attempt reads its step's setup fresh (StepContext.models now carries agents, as it does models). An empty setup passes nothing new, so argv and prompt stay byte-identical. Otherwise the session gets the setup's tools and mcp_tools, its agent and every subagent and team member inline from main's snapshot (--agents, --agent), --setting-sources project plus the user's apiKeyHelper and env for sources: repo, main's snapshot as --add-dir for a worktree step with skills, --forward-subagent-text with subagents or teams, and an init check. A named agent that is not found fails the step with reason setup before a session starts. The setup is traced as agent_setup on the phase row; prompts carry setupLines in place of the skills line. Co-Authored-By: Claude Opus 5.5 --- src/agent-files.ts | 111 ++++++++++++++++++--- src/context.ts | 6 +- src/models.ts | 10 +- src/phases.ts | 105 ++++++++++++++++++-- src/trace.ts | 2 +- tests/unit/models.test.ts | 3 +- tests/unit/phases.test.ts | 203 +++++++++++++++++++++++++++++++++++++- 7 files changed, 409 insertions(+), 31 deletions(-) diff --git a/src/agent-files.ts b/src/agent-files.ts index 3e4a2574..f5531473 100644 --- a/src/agent-files.ts +++ b/src/agent-files.ts @@ -1,7 +1,8 @@ import { readFileSync } from "node:fs"; +import { homedir } from "node:os"; import { join } from "node:path"; import { parse } from "yaml"; -import { BUILTIN_AGENTS, inlineName, resolveSetup } from "./agents"; +import { BUILTIN_AGENTS, inlineName, resolveSetup, type AgentSetup } from "./agents"; import type { Config } from "./config"; import { STEP_KEYS, STEPS, type StepKey } from "./models"; @@ -118,29 +119,50 @@ export async function findSkill(name: string, where: Where): Promise ({ name, role: `a subagent of ${step}` })), + ...setup.teams.flatMap((t) => t.members.map((name) => ({ name, role: `in the team ${t.name} of ${step}` }))), + ]; + const seen = new Set(); + return roles.filter(({ name }) => { + if (seen.has(name)) return false; + seen.add(name); + return true; + }); +} + +/** Where a named agent was looked for, in words: "on main", and "or on this machine" with `sources: all`. */ +function placesText(mainBranch: string, where: Where): string { + return `on ${mainBranch}${where.sources === "all" ? " or on this machine" : ""}`; +} + +/** What a person is told about a named agent that is not found. */ +function missingAgentText(name: string, role: string, places: string): string { + return `\`${name}\`, ${role}, is not in .claude/agents/ ${places}. Add it with \`loopstra setup agents\`, or commit it.`; +} + /** * Every agent, subagent, team member and skill each step would use, checked against `where`. * One problem per name per step: missing, else only on this machine, else keys Loopstra leaves out. */ export async function checkAgentSetup(cfg: Config, where: Where): Promise { const problems: SetupProblem[] = []; - const places = `on ${cfg.main_branch}${where.sources === "all" ? " or on this machine" : ""}`; + const places = placesText(cfg.main_branch, where); for (const step of STEP_KEYS) { const group = STEPS.find((s) => s.key === step)!.group; const stageSkills = group === "chat" ? [] : cfg.stages[group].skills; const setup = resolveSetup(cfg.agents, step, stageSkills); - const roles: { name: string; role: string }[] = [ - ...(setup.agent ? [{ name: setup.agent, role: `the agent ${step} runs as` }] : []), - ...setup.subagents.map((name) => ({ name, role: `a subagent of ${step}` })), - ...setup.teams.flatMap((t) => t.members.map((name) => ({ name, role: `in the team ${t.name} of ${step}` }))), - ]; - const seen = new Set(); - for (const { name, role } of roles) { - if (BUILTIN_AGENTS.has(name) || seen.has(name)) continue; - seen.add(name); + for (const { name, role } of setupRoles(setup, step)) { + if (BUILTIN_AGENTS.has(name)) continue; const def = await findAgent(name, where); if (!def) { - problems.push({ level: "fail", step, name, text: `\`${name}\`, ${role}, is not in .claude/agents/ ${places}. Add it with \`loopstra setup agents\`, or commit it.` }); + problems.push({ level: "fail", step, name, text: missingAgentText(name, role, places) }); } else if (def.from !== "repo") { problems.push({ level: "warn", step, name, text: `\`${name}\` (${step}) is only on this machine, so a loop running elsewhere will not have it.` }); } else if (def.dropped.length) { @@ -159,6 +181,71 @@ export async function checkAgentSetup(cfg: Config, where: Where): Promise { + let main: AgentDefinition | null = null; + const others: AgentDefinition[] = []; + for (const { name, role } of setupRoles(setup, step)) { + if (BUILTIN_AGENTS.has(name)) continue; + const def = await findAgent(name, where); + if (!def) return { missing: missingAgentText(name, role, placesText(mainBranch, where)) }; + if (name === setup.agent) main = def; + else others.push(def); + } + return { main, others }; +} + +/** + * What a session's init event says it did not load of a setup, or null when it loaded everything (or + * the event does not list it). Agents are listed by their inline names, built-ins by their own; a + * skill counts under its name or as `:`. A built-in agent the step runs as is not passed, so + * not expected. + */ +export function initProblem(setup: AgentSetup, init: { agents?: string[]; skills?: string[] }): string | null { + const missing: string[] = []; + if (init.agents) { + const names = [...new Set([ + ...(setup.agent && !BUILTIN_AGENTS.has(setup.agent) ? [setup.agent] : []), + ...setup.subagents, ...setup.teams.flatMap((t) => t.members), + ])]; + for (const name of names) { + if (!init.agents.includes(BUILTIN_AGENTS.has(name) ? name : inlineName(name))) missing.push(`the agent \`${name}\``); + } + } + if (init.skills) { + const listed = init.skills; + for (const name of setup.skills) { + if (!listed.some((k) => k === name || k.endsWith(`:${name}`))) missing.push(`the skill \`${name}\``); + } + } + return missing.length ? `The session did not load ${missing.join(", ")}. Add it with \`loopstra setup agents\`, then retry.` : null; +} + +/** The user's home folder, read as each session starts (a test may change HOME or USERPROFILE meanwhile). */ +export function userHome(): string { + return (process.platform === "win32" ? process.env.USERPROFILE : process.env.HOME) || homedir(); +} + +/** + * The sign-in settings of `/.claude/settings.json` (`apiKeyHelper`, `env`), when it has them. + * A `sources: repo` session loads only project settings, so Loopstra passes these through its own + * `--settings`: a machine that signs in with them can still start sessions. + */ +export function userSettingsPassthrough(home: string): { apiKeyHelper?: string; env?: Record } { + const settings = readJson(join(home, ".claude", "settings.json")); + const out: { apiKeyHelper?: string; env?: Record } = {}; + if (typeof settings?.apiKeyHelper === "string") out.apiKeyHelper = settings.apiKeyHelper; + const env = settings?.env; + if (env && typeof env === "object" && !Array.isArray(env)) { + out.env = Object.fromEntries(Object.entries(env).filter((e): e is [string, string] => typeof e[1] === "string")); + } + return out; +} + /** One entry of the `--agents` JSON. */ export interface InlineAgent { description: string; prompt: string; tools?: string[]; model?: string; skills?: string[] } diff --git a/src/context.ts b/src/context.ts index 26778dd4..a981002f 100644 --- a/src/context.ts +++ b/src/context.ts @@ -43,9 +43,9 @@ export class StepContext { async reload(): Promise { this.intent = await readIntent(this.root, this.slug); } /** - * The step's config with its model settings read again from loopstra/config.yaml, so a change a - * person saved mid-step reaches the next session. When the file does not load now, the step's own - * copy stands, and the problem is traced once while it stays the same. + * The step's config with its model and agent settings read again from loopstra/config.yaml, so a + * change a person saved mid-step reaches the next session. When the file does not load now, the + * step's own copy stands, and the problem is traced once while it stays the same. */ async models(): Promise { try { diff --git a/src/models.ts b/src/models.ts index 05fc2fc1..264afd00 100644 --- a/src/models.ts +++ b/src/models.ts @@ -50,8 +50,8 @@ export function stepsOf(group: StepGroup): readonly StepInfo[] { return STEPS.filter((s) => s.group === group); } -/** The parts of a config that say which model runs what. */ -export type ModelSettings = Pick; +/** The parts of a config that say which model runs what, and the agent settings each step's session gets. */ +export type ModelSettings = Pick; /** The model of a stage, or of chat's orchestrator. */ export function groupModel(cfg: ModelSettings, group: StepGroup): ModelRef { @@ -74,7 +74,10 @@ export function stepModel(cfg: ModelSettings, step: StepKey): ModelRef { return "ref" in by ? by.ref : groupModel(cfg, by.follows); } -/** `base` with every model setting (the names, each stage's model and steps, chat's model and steps) taken from `fresh`. */ +/** + * `base` with every model setting (the names, each stage's model and steps, chat's model and steps) + * and the agent settings (`agents`) taken from `fresh`. + */ export function withModels(base: Config, fresh: ModelSettings): Config { const stages = { ...base.stages }; for (const name of STAGE_NAMES) stages[name] = { ...base.stages[name], model: fresh.stages[name].model, steps: fresh.stages[name].steps } as never; @@ -83,6 +86,7 @@ export function withModels(base: Config, fresh: ModelSettings): Config { claude: { ...base.claude, models: fresh.claude.models }, stages, chat: { ...base.chat, model: fresh.chat.model, steps: fresh.chat.steps }, + agents: fresh.agents, }; } diff --git a/src/phases.ts b/src/phases.ts index 80d0f7aa..38e24f28 100644 --- a/src/phases.ts +++ b/src/phases.ts @@ -2,9 +2,11 @@ import { AsyncLocalStorage } from "node:async_hooks"; import { readFileSync } from "node:fs"; import { join } from "node:path"; import { CHAT_SLUG, limitOf, MIN_SESSION_USD, roomBeside, staleBefore, startOfToday } from "./budget"; -import { runPhase, unavailable, type FailureReason, type PermissionMode } from "./claude"; +import { runPhase, unavailable, type FailureReason, type PermissionMode, type RunPhaseInput } from "./claude"; import { modelFor, type Config, type ModelRef } from "./config"; -import { badModelNote, stepModel } from "./models"; +import { badModelNote, STEP_KEYS, stepModel, type StepKey } from "./models"; +import { BUILTIN_AGENTS, inlineName, isEmptySetup, resolveSetup, setupLines, type AgentSetup } from "./agents"; +import { findAgent, initProblem, inlineAgents, setupAgents, userHome, userSettingsPassthrough, type AgentDefinition, type Where } from "./agent-files"; import type { Trace } from "./trace"; import { GitTimeout } from "./git"; import { CRASH_NOTE, TIMEOUT_NOTE, type StepContext } from "./context"; @@ -96,12 +98,15 @@ function commandParts(cmd: string): string[] { return parts.map((p) => p.trim()).filter((p) => p); } -/** Tools a session may use without asking: the set's own, each once. Always a fresh array. */ -export function toolsFor(ctx: StepContext, set: ToolSet): string[] { - return [...new Set([...setTools(ctx, set)])]; +/** + * Tools a session may use without asking: the set's own, then the step's setup's `tools`, then its + * `mcp_tools` (see stepSetup), each once. Always a fresh array. + */ +export function toolsFor(ctx: StepContext, set: ToolSet, setup?: AgentSetup): string[] { + return [...new Set([...setTools(ctx, set), ...(setup?.tools ?? []), ...(setup?.mcpTools ?? [])])]; } -/** The tool set's own tools, before the MCP rules every set gets. */ +/** The tool set's own tools, before the setup's. */ function setTools(ctx: StepContext, set: ToolSet): string[] { if (set === "read") return [...READ_TOOLS]; if (set === "read+git") return [...READ_TOOLS, ...GIT_READ]; @@ -177,6 +182,36 @@ const RETRIED: ReadonlySet = new Set(["timeout", "crash"]); /** What heads conversation.md in a work session's prompt (the shipped templates say the same). */ export const CONVERSATION_HEADING = "What people told earlier sessions of this change (conversation.md); take it into account:"; +const isStepKey = (name: string): name is StepKey => (STEP_KEYS as readonly string[]).includes(name); + +/** + * The agent setup a phase's session gets (see resolveSetup): its step's, from the agent settings as + * loopstra/config.yaml has them now (see StepContext.models), with the stage's skills the spec names. + * A phase that is not a step gets the spec's skills alone. + */ +export async function stepSetup(ctx: StepContext, spec: Pick): Promise { + if (!isStepKey(spec.name)) return { sources: "all", agent: null, subagents: [], teams: [], skills: [...(spec.skills ?? [])], tools: [], mcpTools: [] }; + return resolveSetup((await ctx.models()).agents, spec.name, spec.skills ?? []); +} + +/** Where a step's named agents and skills are looked for: the step's snapshot of main, then (with `sources: all`) this machine. */ +async function setupWhere(ctx: StepContext, setup: AgentSetup): Promise { + return { repo: await ctx.snapshot(), home: userHome(), sources: setup.sources }; +} + +/** Each subagent's and team member's description, from its file (see setupWhere), for the prompt's lines; null when it has none. */ +async function describer(ctx: StepContext, setup: AgentSetup): Promise<(name: string) => string | null> { + const names = new Set([...setup.subagents, ...setup.teams.flatMap((t) => t.members)].filter((n) => !BUILTIN_AGENTS.has(n))); + if (!names.size) return () => null; + const where = await setupWhere(ctx, setup); + const found = new Map(); + for (const name of names) { + const def = await findAgent(name, where); + if (def) found.set(name, def); + } + return (name) => found.get(name)?.description || null; +} + /** A note a phase's prompt must carry: `text` is added at its end when there is a `note` and the prompt file has no `{{variable}}` to carry it. */ export type PromptNote = { variable: string; note: string; text: string }; @@ -189,11 +224,13 @@ export type PromptNote = { variable: string; note: string; text: string }; export async function phasePrompt(ctx: StepContext, spec: AgentPhaseSpec, extra?: PromptNote): Promise { const path = promptPath(ctx.root, spec.name); if (path === null) return null; - const skillsLine = (spec.skills ?? []).length ? `Use these skills: ${(spec.skills ?? []).map((s) => `\`${s}\``).join(", ")}.\n\n` : ""; + // With only skills, or none, this is the skills line prompts always had. + const setup = await stepSetup(ctx, spec); + const setupText = setupLines(setup, await describer(ctx, setup)); const record: PromptVars = steerable(spec.name) ? { conversation: readConversation(ctx.root, ctx.slug) } : {}; const vars: PromptVars = { slug: ctx.slug, main_branch: ctx.cfg.main_branch, commands: commandsFor(ctx, spec.tools), ...record, ...spec.vars }; const template = await Bun.file(path).text(); - let rendered = skillsLine + renderPrompt(template, vars); + let rendered = setupText + renderPrompt(template, vars); const said = vars.conversation?.trim() ?? ""; if (said && !template.includes("{{conversation}}")) rendered = `${rendered.trimEnd()}\n\n${CONVERSATION_HEADING}\n\n${said}\n`; const prompt = withContract(rendered); @@ -444,9 +481,21 @@ async function attempt(ctx: StepContext, spec: AgentPhaseSp throwIfCancelled(signal); throwIfChangeStopped(ctx); const cwd = spec.cwd ?? (await ctx.snapshot()); - // Read as the session starts: a model a person changed since the step began applies from here. + // Read as the session starts: a model or agent setting a person changed since the step began applies from here. const model = await phaseModel(ctx, spec); + const setup = await stepSetup(ctx, spec); + const extra = isEmptySetup(setup) ? null : await setupRun(ctx, spec, setup, cwd); + if (extra && "missing" in extra) { + ctx.trace.event(ctx.slug, "error", { where: traceName, reason: "setup", detail: extra.missing }); + return { ok: false, reason: "setup", note: `${ownerNote("setup")} ${extra.missing}`, sessionId: null }; + } const { seq, capUsd, fromDay } = startPhase(ctx, traceName); + if (extra) { + ctx.trace.event(ctx.slug, "agent_setup", { + agent: setup.agent, subagents: setup.subagents, teams: setup.teams.map((t) => t.name), skills: setup.skills, + tools: setup.tools, mcp_tools: setup.mcpTools, sources: setup.sources, models: extra.models, + }, seq); + } const dir = join(ctx.runDir, "phases", `${seq}-${traceName}`); // The protect-tests hook writes here the test files it refused to let a fix session edit. const protectedLog = join(dir, "protected-tests.txt"); @@ -461,10 +510,12 @@ async function attempt(ctx: StepContext, spec: AgentPhaseSp schema: jsonSchemaFor(spec.name), model: model.name, permissionMode: spec.permissionMode, - allowedTools: toolsFor(ctx, spec.tools), + allowedTools: toolsFor(ctx, spec.tools, setup), disallowedTools: disallowedFor(spec.tools), // The test-protection hook, from the installed package; it acts only in a fix session. settings: hookSettings(), + // The step's agent setup (see setupRun), which may also replace the settings. + ...extra?.run, timeoutMs: ctx.cfg.claude.timeout_minutes * 60_000, maxBudgetUsd: capUsd, resume: spec.resume, @@ -509,6 +560,8 @@ async function attempt(ctx: StepContext, spec: AgentPhaseSp const reason = out.reason as BlockingReason; // The step's own words: which named model, which CLI name, which step. if (reason === "model") return { ok: false, reason, note: badModelNote(model.ref, model.name, spec.name), sessionId: out.sessionId }; + // What the session did not load, in the init check's words (see initProblem). + if (reason === "setup") return { ok: false, reason, note: `${ownerNote(reason)} ${out.detail}`, sessionId: out.sessionId }; const tests = protectedTests(protectedLog); return { ok: false, reason, note: tests.length ? `${ownerNote(reason)} ${protectedNote(ctx, tests)}` : ownerNote(reason), sessionId: out.sessionId }; } @@ -516,6 +569,38 @@ async function attempt(ctx: StepContext, spec: AgentPhaseSp return { ok: true, envelope: out.value, sessionId: out.sessionId, costUsd: out.costUsd, model: model.name, seq, ...(question ? { asked: question } : {}) }; } +/** + * What a session's run gets from its step's setup when that is not empty, and the models of its + * inline agents for the trace; `missing`, instead, when an agent it names is not found. + * - `tools` and `mcp_tools` go on the allow list (see toolsFor). + * - Every agent it names is read from main (see setupWhere), never the change's branch, and passed + * inline under its Loopstra name (see inlineAgents); the session runs as its agent with `--agent`. + * - `sources: repo` loads project settings only, so the user's sign-in settings ride along in `--settings`. + * - A session outside the snapshot of main (a worktree) also loads main's skills (`--add-dir`). + * - Subagents' text reaches the trace; the init event must list every agent and skill (see initProblem). + */ +async function setupRun(ctx: StepContext, spec: AgentPhaseSpec, setup: AgentSetup, cwd: string): Promise<{ run: Partial; models: Record } | { missing: string }> { + const run: Partial = { checkInit: (init) => initProblem(setup, init) }; + let models: Record = {}; + const usesMain = setup.agent !== null || setup.subagents.length > 0 || setup.teams.length > 0 || setup.skills.length > 0; + const where = usesMain ? await setupWhere(ctx, setup) : null; + if (where) { + const found = await setupAgents(setup, spec.name, where, ctx.cfg.main_branch); + if ("missing" in found) return found; + const agents = inlineAgents(found.main, found.others); + if (Object.keys(agents).length) run.agents = agents; + if (found.main) run.agent = inlineName(found.main.name); + models = Object.fromEntries(Object.entries(agents).flatMap(([name, a]) => (a.model ? [[name, a.model]] : []))); + if (setup.skills.length && cwd !== where.repo) run.addDirs = [where.repo]; + } + if (setup.sources === "repo") { + run.settingSources = "project"; + run.settings = JSON.stringify({ ...(JSON.parse(hookSettings()) as Record), ...userSettingsPassthrough(userHome()) }); + } + if (setup.subagents.length || setup.teams.length) run.forwardSubagentText = true; + return { run, models }; +} + /** * Throws SessionStopped when a person stopped the change (see writeChangeStop in src/inbox.ts): checked * before each phase starts and once it ends, so a stop ends the step whichever phase it came in. diff --git a/src/trace.ts b/src/trace.ts index 5ef65c4e..b2043de5 100644 --- a/src/trace.ts +++ b/src/trace.ts @@ -8,7 +8,7 @@ import { errorCode, errorText } from "./shell"; export type EventType = | "tick" | "phase_start" | "claude_event" | "command" | "gate_check" | "status_change" | "phase_end" | "error" | "signal" | "stop" | "person-changed-status" | "pause" | "stale-lock-removed" - | "chat-message" | "chat-request" | "chat-pr" | "request" | "judge" | "steer" | "question"; + | "chat-message" | "chat-request" | "chat-pr" | "request" | "judge" | "steer" | "question" | "agent_setup"; export interface EventRow { id: number; slug: string; phase_seq: number | null; type: EventType; ts: string; payload: string; diff --git a/tests/unit/models.test.ts b/tests/unit/models.test.ts index d1190d57..f1e3dc14 100644 --- a/tests/unit/models.test.ts +++ b/tests/unit/models.test.ts @@ -54,7 +54,7 @@ test("a group's model is its stage's, or chat's", () => { test("withModels takes the model settings and keeps the rest", () => { const base = validateConfig({ ...BASE, poll_seconds: 5, stages: { build: { max_fix_loops: 7 } } }); - const fresh = validateConfig({ ...BASE, claude: { models: { max: "x" } }, stages: { build: { model: "strong", steps: { fix: "max" } } }, chat: { model: "cheap", steps: { "write-intent": "max" } } }); + const fresh = validateConfig({ ...BASE, agents: { all: { skills: ["house"] } }, claude: { models: { max: "x" } }, stages: { build: { model: "strong", steps: { fix: "max" } } }, chat: { model: "cheap", steps: { "write-intent": "max" } } }); const got = withModels(base, fresh); expect(got.poll_seconds).toBe(5); expect(got.stages.build.max_fix_loops).toBe(7); @@ -62,4 +62,5 @@ test("withModels takes the model settings and keeps the rest", () => { expect(got.stages.build.steps).toEqual({ fix: "max" }); expect(got.claude.models.max).toBe("x"); expect(got.chat).toMatchObject({ model: "cheap", steps: { "write-intent": "max" } }); + expect(got.agents).toBe(fresh.agents); }); diff --git a/tests/unit/phases.test.ts b/tests/unit/phases.test.ts index 5f66dc06..f32d21e1 100644 --- a/tests/unit/phases.test.ts +++ b/tests/unit/phases.test.ts @@ -6,7 +6,7 @@ import { StepContext } from "../../src/context"; import { Git } from "../../src/git"; import { readIntent } from "../../src/intents"; import { badModelNote, stepModel } from "../../src/models"; -import { agentPhase, alongside, alongsideFirst, codePhase, commandsFor, CONTRACT_LINES, CONVERSATION_HEADING, disallowedFor, ownerNote, phaseModel, probeAssistant, projectCommand, toolsFor, withContract } from "../../src/phases"; +import { agentPhase, alongside, alongsideFirst, codePhase, commandsFor, CONTRACT_LINES, CONVERSATION_HEADING, disallowedFor, ownerNote, phaseModel, probeAssistant, projectCommand, stepSetup, toolsFor, withContract, type AgentPhaseSpec } from "../../src/phases"; import { activePause, pauseAfterUnavailable, readPause } from "../../src/heartbeat"; import { hookSettings } from "../../src/package"; import { claim, onCheckout, oneMerge, realPathKey, release } from "../../src/lanes"; @@ -846,3 +846,204 @@ describe("the step's snapshot of main", () => { } finally { trace.close(); repo.cleanup(); } }); }); + +describe("a step's agent setup", () => { + /** Writes loopstra/config.yaml with this `agents` block (none when undefined); sessions read it fresh. */ + const writeAgents = (root: string, agents?: unknown) => + Bun.write(configPath(root), `version: 1\ncommands:\n test: echo ok\n${agents === undefined ? "" : `agents: ${JSON.stringify(agents)}\n`}`); + /** Commits agent files on main, before the step's snapshot of main is made. */ + const commitAgents = async (root: string, files: Record) => { + mkdirSync(join(root, ".claude", "agents"), { recursive: true }); + for (const [name, text] of Object.entries(files)) await Bun.write(join(root, ".claude", "agents", `${name}.md`), text); + await new Git(root).commitAll("agents"); + }; + /** Runs a phase with its argv recorded: the args, the prompt sent, and the allow list. */ + const recorded = async (ctx: StepContext, spec: Omit & { env?: Record }) => { + const file = join(ctx.root, "..", `args-${Date.now()}-${Math.random()}.json`); + try { + const r = await agentPhase(ctx, { ...spec, vars: {}, env: { LOOPSTRA_FAKE_ARGS: file, ...spec.env } }); + if (!existsSync(file)) return { r, args: null, prompt: null, allowed: [] as string[] }; + const { args, prompt } = (await Bun.file(file).json()) as { args: string[]; prompt: string }; + const allowed = (args[args.indexOf("--allowedTools") + 1] ?? "").split(","); + return { r, args, prompt, allowed }; + } finally { rmSync(file, { force: true }); } + }; + /** The change's worktree, on its own branch from main. */ + const worktree = async (ctx: StepContext) => { + await ctx.git.run(["branch", ctx.branch]); + await ctx.git.ensureWorktree(ctx.worktreeDir, ctx.branch); + }; + const NEW_FLAGS = ["--agents", "--agent", "--setting-sources", "--add-dir", "--forward-subagent-text"]; + + test("with nothing set, a session's args and prompt are what they were", async () => { + const { repo, ctx, trace } = await setup(); + try { + await worktree(ctx); + const runs: { args: string[] | null; prompt: string | null }[][] = []; + for (const agents of [undefined, {}, { sources: "all", all: {}, steps: {} }]) { + await writeAgents(repo.path, agents); + const intake = await recorded(ctx, { name: "intake", model: "cheap", permissionMode: "default", tools: "read" }); + const build = await recorded(ctx, { name: "build", permissionMode: "acceptEdits", tools: "build", cwd: ctx.worktreeDir }); + runs.push([intake, build].map(({ args, prompt }) => ({ args, prompt }))); + } + expect(runs[1]).toEqual(runs[0]); + expect(runs[2]).toEqual(runs[0]); + for (const { args } of runs[0]!) { + expect(args).not.toBeNull(); + for (const flag of NEW_FLAGS) expect(args).not.toContain(flag); + expect(args![args!.indexOf("--settings") + 1]).toBe(hookSettings()); + } + expect(trace.events("x").some((e) => e.type === "agent_setup")).toBe(false); + } finally { trace.close(); repo.cleanup(); } + }); + + test("a step's agent, subagents, team and tools reach its session", async () => { + const { repo, ctx, trace } = await setup(); + try { + await commitAgents(repo.path, { + lead: "---\nname: lead\ndescription: Leads the review\ntools: Read\n---\nYou lead.\n", + sec: "---\ndescription: Looks for security holes\nmodel: haiku\n---\nSecurity.\n", + perf: "---\ndescription: Looks for slow code\n---\nSpeed.\n", + }); + await writeAgents(repo.path, { + teams: { reviewers: { members: ["sec", "perf"], way: "parallel" } }, + steps: { review: { agent: "lead", teams: ["reviewers"], tools: ["WebFetch"], mcp_tools: ["mcp__gh"] } }, + }); + await Bun.write(join(repo.path, "loopstra", "prompts", "review.md"), "Review {{slug}}.\n"); + await worktree(ctx); + const { r, args, prompt, allowed } = await recorded(ctx, { name: "review", permissionMode: "default", tools: "read+git", cwd: ctx.worktreeDir }); + expect(r.ok).toBe(true); + expect(args![args!.indexOf("--agent") + 1]).toBe("loopstra-lead"); + const inline = JSON.parse(args![args!.indexOf("--agents") + 1]!) as Record; + expect(Object.keys(inline)).toEqual(["loopstra-lead", "loopstra-sec", "loopstra-perf"]); + expect(inline["loopstra-lead"]!.tools).toEqual(["Read", "StructuredOutput"]); + expect(allowed).toEqual(expect.arrayContaining(["WebFetch", "mcp__gh"])); + expect(args).toContain("--forward-subagent-text"); + expect(prompt).toContain("`loopstra-sec`, `loopstra-perf`"); + expect(prompt).toStartWith("Team `reviewers`: `loopstra-sec`, `loopstra-perf`."); + // The setup is traced once, on the session's phase row. + const events = trace.events("x").filter((e) => e.type === "agent_setup"); + expect(events).toHaveLength(1); + expect(JSON.parse(events[0]!.payload)).toEqual({ + agent: "lead", subagents: [], teams: ["reviewers"], skills: [], tools: ["WebFetch"], mcp_tools: ["mcp__gh"], sources: "all", + models: { "loopstra-sec": "haiku" }, + }); + expect(events[0]!.phase_seq).toBe(trace.phases("x")[0]!.seq); + } finally { trace.close(); repo.cleanup(); } + }); + + test("subagents are described in the prompt by their files", async () => { + const { repo, ctx, trace } = await setup(); + try { + await commitAgents(repo.path, { sec: "---\ndescription: Looks for security holes\n---\nSecurity.\n", bare: "No frontmatter.\n" }); + await writeAgents(repo.path, { steps: { intake: { subagents: ["sec", "bare", "Explore"] } } }); + const { r, args, prompt } = await recorded(ctx, { name: "intake", model: "cheap", permissionMode: "default", tools: "read" }); + expect(r.ok).toBe(true); + expect(prompt).toStartWith("You can hand work to these subagents: `loopstra-sec` (Looks for security holes), `loopstra-bare`, `Explore`."); + // A built-in is Claude Code's own: no inline copy, and no --agent without a main agent. + expect(Object.keys(JSON.parse(args![args!.indexOf("--agents") + 1]!) as object)).toEqual(["loopstra-sec", "loopstra-bare"]); + expect(args).not.toContain("--agent"); + } finally { trace.close(); repo.cleanup(); } + }); + + test("agent files come from main, not from the change's branch", async () => { + const { repo, ctx, trace } = await setup(); + try { + await commitAgents(repo.path, { lead: "---\ndescription: Builds\n---\nMAIN\n" }); + await writeAgents(repo.path, { steps: { build: { agent: "lead" } } }); + await worktree(ctx); + await Bun.write(join(ctx.worktreeDir, ".claude", "agents", "lead.md"), "---\ndescription: Builds\n---\nBRANCH\n"); + await new Git(ctx.worktreeDir).commitAll("the branch rewrites its builder"); + const { r, args } = await recorded(ctx, { name: "build", permissionMode: "acceptEdits", tools: "build", cwd: ctx.worktreeDir }); + expect(r.ok).toBe(true); + const inline = JSON.parse(args![args!.indexOf("--agents") + 1]!) as Record; + expect(inline["loopstra-lead"]!.prompt).toBe("MAIN"); + } finally { trace.close(); repo.cleanup(); } + }); + + test("a named agent nobody has stops the step before a session starts", async () => { + const { repo, ctx, trace } = await setup(); + try { + await writeAgents(repo.path, { steps: { design: { agent: "ghost" } } }); + const { r, args } = await recorded(ctx, { name: "design", permissionMode: "plan", tools: "read" }); + expect(args).toBeNull(); + expect(r.ok).toBe(false); + if (!r.ok) { + expect(r.reason).toBe("setup"); + expect(r.note).toStartWith(ownerNote("setup")); + expect(r.note).toContain("`ghost`"); + expect(r.note).toContain("`loopstra setup agents`"); + } + expect(trace.phases("x")).toHaveLength(0); + const error = trace.events("x").find((e) => e.type === "error"); + expect(JSON.parse(error!.payload)).toMatchObject({ where: "design", reason: "setup", detail: expect.stringContaining("`ghost`") }); + } finally { trace.close(); repo.cleanup(); } + }); + + test("a session that did not load a named skill or agent stops as a setup failure, not retried", async () => { + const { repo, ctx, trace } = await setup(); + try { + await writeAgents(repo.path, { all: { skills: ["house", "a"] } }); + // The init event lists skills ["a"] and agents ["loopstra-x"]. + const { r } = await recorded(ctx, { name: "intake", model: "cheap", permissionMode: "default", tools: "read", env: { LOOPSTRA_FAKE_FIXTURE: join(FAKE, "..", "fixtures", "init-lists.jsonl") } }); + expect(r.ok).toBe(false); + if (!r.ok) { + expect(r.reason).toBe("setup"); + expect(r.note).toStartWith(ownerNote("setup")); + expect(r.note).toContain("`house`"); + expect(r.note).not.toContain("`a`"); + expect(r.note).toContain("Add it with `loopstra setup agents`, then retry."); + } + expect(trace.phases("x")).toHaveLength(1); + } finally { trace.close(); repo.cleanup(); } + }); + + test("repo-only sessions load project settings and keep the user's sign-in", async () => { + const { repo, ctx, trace } = await setup(); + const home = tempDir(); + mkdirSync(join(home.path, ".claude"), { recursive: true }); + await Bun.write(join(home.path, ".claude", "settings.json"), JSON.stringify({ apiKeyHelper: "/bin/key", env: { A: "1" }, theme: "x" })); + const restore = setEnv({ HOME: home.path, USERPROFILE: home.path }); + try { + await writeAgents(repo.path, { sources: "repo" }); + const { r, args } = await recorded(ctx, { name: "intake", model: "cheap", permissionMode: "default", tools: "read" }); + expect(r.ok).toBe(true); + expect(args![args!.indexOf("--setting-sources") + 1]).toBe("project"); + const settings = JSON.parse(args![args!.indexOf("--settings") + 1]!) as Record; + expect(settings).toMatchObject({ apiKeyHelper: "/bin/key", env: { A: "1" }, hooks: expect.any(Object) }); + expect(settings.theme).toBeUndefined(); + expect(settings.hooks).toEqual((JSON.parse(hookSettings()) as { hooks: unknown }).hooks); + } finally { restore(); home.cleanup(); trace.close(); repo.cleanup(); } + }); + + test("a worktree step that names skills also gets main's snapshot as an added dir", async () => { + const { repo, ctx, trace } = await setup(); + try { + await writeAgents(repo.path, { all: { skills: ["house"] } }); + await worktree(ctx); + const build = await recorded(ctx, { name: "build", permissionMode: "acceptEdits", tools: "build", cwd: ctx.worktreeDir }); + expect(build.args![build.args!.indexOf("--add-dir") + 1]).toBe(await ctx.snapshot()); + expect(build.prompt).toStartWith("Use these skills: `house`.\n\n"); + const design = await recorded(ctx, { name: "design", permissionMode: "plan", tools: "read" }); + expect(design.args).not.toContain("--add-dir"); + await ctx.dropSnapshot(); + } finally { trace.close(); repo.cleanup(); } + }); + + test("mcp rules from agents.all reach every tool set, once each", async () => { + const { repo, ctx, trace } = await setup(); + try { + await writeAgents(repo.path, { all: { mcp_tools: ["mcp__github", "mcp__docs__guide", "mcp__github"] } }); + const setup = await stepSetup(ctx, { name: "intake" }); + for (const set of ["read", "read+git", "read+commands", "build"] as const) { + const tools = toolsFor(ctx, set, setup); + expect(tools.slice(-2)).toEqual(["mcp__github", "mcp__docs__guide"]); + expect(new Set(tools).size).toBe(tools.length); + } + // MCP rules are not shell commands. + expect(commandsFor(ctx, "read")).toBe("none"); + const { allowed } = await recorded(ctx, { name: "intake", model: "cheap", permissionMode: "default", tools: "read" }); + expect(allowed).toEqual(["Read", "Glob", "Grep", "mcp__github", "mcp__docs__guide"]); + } finally { trace.close(); repo.cleanup(); } + }); +}); From e60bece471638501000b96763f3231ce6e2eb3ff Mon Sep 17 00:00:00 2001 From: John Flavan Date: Mon, 26 Oct 2026 23:03:38 -0500 Subject: [PATCH 12/64] fix(phases): stage skills alone keep today's argv; built-in main agents refused A setup changes how a session runs only when the step's agent settings give it something; a stage's skills alone are the prompt's first line, as before (no snapshot, --add-dir or init check). {{commands}} also lists the Bash rules from the step's setup tools. A built-in agent as a step's (or all's) agent is a config error: its fixed tools have no StructuredOutput. Co-Authored-By: Claude Opus 5.5 --- src/config.ts | 6 +++++- src/phases.ts | 30 +++++++++++++++++++++--------- tests/unit/config.test.ts | 4 ++++ tests/unit/phases.test.ts | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 10 deletions(-) diff --git a/src/config.ts b/src/config.ts index 48422771..0a1d9cc9 100644 --- a/src/config.ts +++ b/src/config.ts @@ -2,7 +2,7 @@ import { existsSync } from "node:fs"; import { join } from "node:path"; import { parse } from "yaml"; import { z } from "zod"; -import { CONTINUES_BUILD, READ_ONLY_STEPS, TEAM_WAYS, toolProblem } from "./agents"; +import { BUILTIN_AGENTS, CONTINUES_BUILD, READ_ONLY_STEPS, TEAM_WAYS, toolProblem } from "./agents"; import { MODEL_REFS, STEP_KEYS, stepsOf, type ModelRef, type StepKey, type StageName, type StepGroup } from "./models"; import { errorText } from "./shell"; @@ -147,6 +147,10 @@ const agents = z.object({ set.teams.forEach((t, i) => { if (!(t in a.teams)) bad([...path, "teams", i], `there is no team ${t} under agents.teams`); }); + // A built-in's fixed tool list has no StructuredOutput, so a step running as one could not report. + if (set.agent !== undefined && BUILTIN_AGENTS.has(set.agent)) { + bad([...path, "agent"], `${set.agent} is a built-in agent; a step runs as an agent defined in .claude/agents/ (built-ins can be subagents)`); + } }; tools(["all"], a.all, true, "these reach read-only steps too", "them"); for (const [step, set] of Object.entries(a.steps)) { diff --git a/src/phases.ts b/src/phases.ts index 38e24f28..e5d8e5b7 100644 --- a/src/phases.ts +++ b/src/phases.ts @@ -122,11 +122,12 @@ function setTools(ctx: StepContext, set: ToolSet): string[] { } /** - * The shell commands a tool set lets a session run, for the prompt's `{{commands}}`: "`bun test`, - * `git diff`" (each with any arguments), "any command" for a bare Bash rule, "none" without Bash. + * The shell commands a tool set, and the step's setup (see toolsFor), let a session run, for the + * prompt's `{{commands}}`: "`bun test`, `git diff`" (each with any arguments), "any command" for a + * bare Bash rule, "none" without Bash. */ -export function commandsFor(ctx: StepContext, set: ToolSet): string { - const tools = toolsFor(ctx, set); +export function commandsFor(ctx: StepContext, set: ToolSet, setup?: AgentSetup): string { + const tools = toolsFor(ctx, set, setup); if (tools.includes("Bash")) return "any command"; const cmds = tools.map((t) => /^Bash\((.+?)(?: \*|:\*)?\)$/.exec(t)?.[1]).filter((c): c is string => !!c); return cmds.length ? [...new Set(cmds)].map((c) => `\`${c}\``).join(", ") : "none"; @@ -190,8 +191,19 @@ const isStepKey = (name: string): name is StepKey => (STEP_KEYS as readonly stri * A phase that is not a step gets the spec's skills alone. */ export async function stepSetup(ctx: StepContext, spec: Pick): Promise { - if (!isStepKey(spec.name)) return { sources: "all", agent: null, subagents: [], teams: [], skills: [...(spec.skills ?? [])], tools: [], mcpTools: [] }; - return resolveSetup((await ctx.models()).agents, spec.name, spec.skills ?? []); + return (await setupOf(ctx, spec)).setup; +} + +/** + * A phase's setup (see stepSetup), and whether it changes how the session runs. A stage's skills alone + * do not: they are the prompt's first line, as they always were. Once the step's agent settings give it + * anything, the stage's skills are checked and loaded with the rest (see setupRun). + */ +async function setupOf(ctx: StepContext, spec: Pick): Promise<{ setup: AgentSetup; applies: boolean }> { + const skills = spec.skills ?? []; + if (!isStepKey(spec.name)) return { setup: { sources: "all", agent: null, subagents: [], teams: [], skills: [...skills], tools: [], mcpTools: [] }, applies: false }; + const agents = (await ctx.models()).agents; + return { setup: resolveSetup(agents, spec.name, skills), applies: !isEmptySetup(resolveSetup(agents, spec.name, [])) }; } /** Where a step's named agents and skills are looked for: the step's snapshot of main, then (with `sources: all`) this machine. */ @@ -228,7 +240,7 @@ export async function phasePrompt(ctx: StepContext, spec: AgentPhaseSpec, extra? const setup = await stepSetup(ctx, spec); const setupText = setupLines(setup, await describer(ctx, setup)); const record: PromptVars = steerable(spec.name) ? { conversation: readConversation(ctx.root, ctx.slug) } : {}; - const vars: PromptVars = { slug: ctx.slug, main_branch: ctx.cfg.main_branch, commands: commandsFor(ctx, spec.tools), ...record, ...spec.vars }; + const vars: PromptVars = { slug: ctx.slug, main_branch: ctx.cfg.main_branch, commands: commandsFor(ctx, spec.tools, setup), ...record, ...spec.vars }; const template = await Bun.file(path).text(); let rendered = setupText + renderPrompt(template, vars); const said = vars.conversation?.trim() ?? ""; @@ -483,8 +495,8 @@ async function attempt(ctx: StepContext, spec: AgentPhaseSp const cwd = spec.cwd ?? (await ctx.snapshot()); // Read as the session starts: a model or agent setting a person changed since the step began applies from here. const model = await phaseModel(ctx, spec); - const setup = await stepSetup(ctx, spec); - const extra = isEmptySetup(setup) ? null : await setupRun(ctx, spec, setup, cwd); + const { setup, applies } = await setupOf(ctx, spec); + const extra = applies ? await setupRun(ctx, spec, setup, cwd) : null; if (extra && "missing" in extra) { ctx.trace.event(ctx.slug, "error", { where: traceName, reason: "setup", detail: extra.missing }); return { ok: false, reason: "setup", note: `${ownerNote("setup")} ${extra.missing}`, sessionId: null }; diff --git a/tests/unit/config.test.ts b/tests/unit/config.test.ts index 27dee05c..fc05070a 100644 --- a/tests/unit/config.test.ts +++ b/tests/unit/config.test.ts @@ -208,6 +208,10 @@ test("agents settings are refused in plain words", () => { expect(problems({ steps: { reviews: {} } })[0]).toStartWith("agents.steps: unknown step reviews; the steps are intake, design,"); expect(problems({ steps: { fix: { agent: "x" } } })).toEqual(["agents.steps.fix.agent: fix continues the build session and runs as the build's agent; set the agent on build"]); expect(problems({ steps: { review: { tools: ["Edit"] } } })).toEqual(["agents.steps.review.tools.0: review is read-only; Edit would let it change files"]); + // A built-in cannot be the agent a step runs as (it could not report), but can be a subagent. + expect(problems({ steps: { design: { agent: "Explore" } } })).toEqual(["agents.steps.design.agent: Explore is a built-in agent; a step runs as an agent defined in .claude/agents/ (built-ins can be subagents)"]); + expect(problems({ all: { agent: "general-purpose" } })).toEqual(["agents.all.agent: general-purpose is a built-in agent; a step runs as an agent defined in .claude/agents/ (built-ins can be subagents)"]); + expect(problems({ all: { subagents: ["Explore"] }, steps: { review: { subagents: ["Plan"] } } })).toEqual([]); expect(problems({ steps: { review: { tools: ["Bash"] } } })).toEqual(["agents.steps.review.tools.0: review is read-only; a bare Bash rule would let it run anything"]); expect(problems({ steps: { build: { tools: ["Bash(git push *)"] } } })).toEqual(["agents.steps.build.tools.0: only Loopstra commits, branches, merges and pushes; git push is not allowed"]); expect(problems({ all: { tools: ["Write"] } })).toEqual(["agents.all.tools.0: these reach read-only steps too; Write would let them change files"]); diff --git a/tests/unit/phases.test.ts b/tests/unit/phases.test.ts index f32d21e1..7a09cdb3 100644 --- a/tests/unit/phases.test.ts +++ b/tests/unit/phases.test.ts @@ -893,10 +893,39 @@ describe("a step's agent setup", () => { for (const flag of NEW_FLAGS) expect(args).not.toContain(flag); expect(args![args!.indexOf("--settings") + 1]).toBe(hookSettings()); } + // A stage's skills alone (spec.skills) change the prompt's first line only, as they always did. + await writeAgents(repo.path); + const skilled = await recorded(ctx, { name: "build", permissionMode: "acceptEdits", tools: "build", cwd: ctx.worktreeDir, skills: ["house"] }); + expect(skilled.args).toEqual(runs[0]![1]!.args); + expect(skilled.prompt).toBe(`Use these skills: \`house\`.\n\n${runs[0]![1]!.prompt}`); + expect(existsSync(join(repo.path, ".loopstra", "snapshots", "x"))).toBe(true); // intake's, not build's expect(trace.events("x").some((e) => e.type === "agent_setup")).toBe(false); } finally { trace.close(); repo.cleanup(); } }); + test("a stage's skills alone make no snapshot for a worktree step", async () => { + const { repo, ctx, trace } = await setup(); + try { + await worktree(ctx); + const { r, args } = await recorded(ctx, { name: "build", permissionMode: "acceptEdits", tools: "build", cwd: ctx.worktreeDir, skills: ["house"] }); + expect(r.ok).toBe(true); + for (const flag of NEW_FLAGS) expect(args).not.toContain(flag); + expect(existsSync(join(repo.path, ".loopstra", "snapshots", "x"))).toBe(false); + } finally { trace.close(); repo.cleanup(); } + }); + + test("the setup's Bash rules are among the prompt's commands", async () => { + const { repo, ctx, trace } = await setup(); + try { + await Bun.write(join(repo.path, "loopstra", "prompts", "intake.md"), "commands={{commands}}\n"); + await writeAgents(repo.path, { steps: { intake: { tools: ["Bash(make lint *)", "WebFetch"], mcp_tools: ["mcp__gh"] } } }); + expect(commandsFor(ctx, "read", await stepSetup(ctx, { name: "intake" }))).toBe("`make lint`"); + expect(commandsFor(ctx, "read")).toBe("none"); + const { prompt } = await recorded(ctx, { name: "intake", model: "cheap", permissionMode: "default", tools: "read" }); + expect(prompt).toStartWith("commands=`make lint`\n"); + } finally { trace.close(); repo.cleanup(); } + }); + test("a step's agent, subagents, team and tools reach its session", async () => { const { repo, ctx, trace } = await setup(); try { @@ -1026,6 +1055,10 @@ describe("a step's agent setup", () => { expect(build.prompt).toStartWith("Use these skills: `house`.\n\n"); const design = await recorded(ctx, { name: "design", permissionMode: "plan", tools: "read" }); expect(design.args).not.toContain("--add-dir"); + // A stage's skills join in once the setup has something of its own. + await writeAgents(repo.path, { all: { tools: ["WebFetch"] } }); + const staged = await recorded(ctx, { name: "build", permissionMode: "acceptEdits", tools: "build", cwd: ctx.worktreeDir, skills: ["tone"] }); + expect(staged.args![staged.args!.indexOf("--add-dir") + 1]).toBe(await ctx.snapshot()); await ctx.dropSnapshot(); } finally { trace.close(); repo.cleanup(); } }); From 97686d24c283a7dcd08da2e2f23cc669dec1267a Mon Sep 17 00:00:00 2001 From: John Flavan Date: Mon, 26 Oct 2026 23:08:28 -0500 Subject: [PATCH 13/64] feat(chat): agent setup for the orchestrator and the intent writer Co-Authored-By: Claude Opus 5.5 --- src/agent-files.ts | 41 ++++++++++++++++++++++++++-- src/chat/agents.ts | 27 ++++++++++++++++-- src/chat/orchestrator.ts | 1 + src/chat/writer.ts | 1 + src/phases.ts | 31 ++++----------------- tests/unit/chat-orchestrator.test.ts | 31 +++++++++++++++++++++ 6 files changed, 101 insertions(+), 31 deletions(-) diff --git a/src/agent-files.ts b/src/agent-files.ts index f5531473..bc503b5b 100644 --- a/src/agent-files.ts +++ b/src/agent-files.ts @@ -3,6 +3,7 @@ import { homedir } from "node:os"; import { join } from "node:path"; import { parse } from "yaml"; import { BUILTIN_AGENTS, inlineName, resolveSetup, type AgentSetup } from "./agents"; +import type { RunPhaseInput } from "./claude"; import type { Config } from "./config"; import { STEP_KEYS, STEPS, type StepKey } from "./models"; @@ -186,7 +187,7 @@ export async function checkAgentSetup(cfg: Config, where: Where): Promise { +async function setupAgents(setup: AgentSetup, step: string, where: Where, mainBranch: string): Promise<{ main: AgentDefinition | null; others: AgentDefinition[] } | { missing: string }> { let main: AgentDefinition | null = null; const others: AgentDefinition[] = []; for (const { name, role } of setupRoles(setup, step)) { @@ -205,7 +206,7 @@ export async function setupAgents(setup: AgentSetup, step: string, where: Where, * skill counts under its name or as `:`. A built-in agent the step runs as is not passed, so * not expected. */ -export function initProblem(setup: AgentSetup, init: { agents?: string[]; skills?: string[] }): string | null { +function initProblem(setup: AgentSetup, init: { agents?: string[]; skills?: string[] }): string | null { const missing: string[] = []; if (init.agents) { const names = [...new Set([ @@ -235,7 +236,7 @@ export function userHome(): string { * A `sources: repo` session loads only project settings, so Loopstra passes these through its own * `--settings`: a machine that signs in with them can still start sessions. */ -export function userSettingsPassthrough(home: string): { apiKeyHelper?: string; env?: Record } { +function userSettingsPassthrough(home: string): { apiKeyHelper?: string; env?: Record } { const settings = readJson(join(home, ".claude", "settings.json")); const out: { apiKeyHelper?: string; env?: Record } = {}; if (typeof settings?.apiKeyHelper === "string") out.apiKeyHelper = settings.apiKeyHelper; @@ -263,3 +264,37 @@ export function inlineAgents(main: AgentDefinition | null, others: AgentDefiniti for (const d of others) out[inlineName(d.name)] = entry(d, false); return out; } + +/** + * The run options a step's agent setup adds to its session, for loop steps and chat steps alike: + * - every agent it names is read from `whereOf()` and passed inline under its Loopstra name (see + * inlineAgents); the session runs as its agent with `--agent`. `whereOf` is asked only when the setup + * names an agent or a skill. + * - `sources: repo` loads project settings only, so `baseSettings` and the user's sign-in settings + * ride along in `--settings`. + * - a session outside `where.repo` (`cwd` differs) that uses skills also loads that folder's (`--add-dir`). + * - subagents' text reaches the trace; the init event must list every agent and skill (see initProblem). + * `models`: the model each inline agent sets, for the trace. `missing`: see setupAgents. + */ +export async function setupSession( + setup: AgentSetup, step: string, mainBranch: string, cwd: string, baseSettings: Record, whereOf: () => Promise, +): Promise<{ run: Partial; models: Record } | { missing: string }> { + const run: Partial = { checkInit: (init) => initProblem(setup, init) }; + let models: Record = {}; + if (setup.agent !== null || setup.subagents.length > 0 || setup.teams.length > 0 || setup.skills.length > 0) { + const where = await whereOf(); + const found = await setupAgents(setup, step, where, mainBranch); + if ("missing" in found) return found; + const agents = inlineAgents(found.main, found.others); + if (Object.keys(agents).length) run.agents = agents; + if (found.main) run.agent = inlineName(found.main.name); + models = Object.fromEntries(Object.entries(agents).flatMap(([name, a]) => (a.model ? [[name, a.model]] : []))); + if (setup.skills.length && cwd !== where.repo) run.addDirs = [where.repo]; + } + if (setup.sources === "repo") { + run.settingSources = "project"; + run.settings = JSON.stringify({ ...baseSettings, ...userSettingsPassthrough(userHome()) }); + } + if (setup.subagents.length || setup.teams.length) run.forwardSubagentText = true; + return { run, models }; +} diff --git a/src/chat/agents.ts b/src/chat/agents.ts index 55c4533a..01ff2936 100644 --- a/src/chat/agents.ts +++ b/src/chat/agents.ts @@ -3,6 +3,8 @@ import { runDir } from "../paths"; import type { z } from "zod"; import { CHAT_SLUG, limitOf, MIN_SESSION_USD, staleBefore, startOfToday } from "../budget"; import type { FailureReason } from "../claude"; +import { findAgent, setupSession, userHome, type Where } from "../agent-files"; +import { BUILTIN_AGENTS, resolveSetup, setupLines, type AgentSetup } from "../agents"; import { modelFor, type Config } from "../config"; import { jsonSchemaOf } from "../envelopes"; import { stepModel, type StepKey } from "../models"; @@ -94,6 +96,17 @@ export function secretEnvNames(cfg: Config, env: Record string | null> { + const found = new Map(); + for (const name of new Set([...setup.subagents, ...setup.teams.flatMap((t) => t.members)])) { + if (BUILTIN_AGENTS.has(name)) continue; + const def = await findAgent(name, where); + if (def?.description) found.set(name, def.description); + } + return (name) => found.get(name) ?? null; +} + type ChatAgentResult = | { ok: true; value: T; sessionId: string | null; costUsd: number } | { ok: false; reason: FailureReason; detail: string; sessionId: string | null; costUsd: number; @@ -132,18 +145,28 @@ export async function runChatAgent(o: ChatAgentInput): Promise where); + if ("missing" in extra) { + // No session starts: the hold made above ends, costing nothing. + o.trace.phaseEnd(CHAT_SLUG, held.seq, { status: "fail", costUsd: 0, error: `setup: ${extra.missing}` }); + return { ok: false, reason: "setup", detail: extra.missing, sessionId: null, costUsd: 0 }; + } const out = await tracedSession({ trace: o.trace, slug: CHAT_SLUG, seq: held.seq, capUsd: held.heldUsd, ref, dir: join(runDir(o.root, CHAT_SLUG), "phases", `${held.seq}-${o.name}`), run: { cwd: o.root, - prompt: o.prompt, + prompt: setupLines(setup, await describe(setup, where)) + o.prompt, schema: jsonSchemaOf(o.schema), // Its step's model (see stepModel): chat loads the config for every message, so a change applies on the next turn. model: modelFor(o.cfg, ref), permissionMode: "default", - allowedTools: CHAT_TOOLS, + allowedTools: [...CHAT_TOOLS, ...setup.tools, ...setup.mcpTools], disallowedTools: CHAT_DENIED, + ...extra.run, timeoutMs: o.cfg.claude.timeout_minutes * 60_000, maxBudgetUsd: held.heldUsd, resume: o.resume ?? undefined, diff --git a/src/chat/orchestrator.ts b/src/chat/orchestrator.ts index 15a2a9bf..377907da 100644 --- a/src/chat/orchestrator.ts +++ b/src/chat/orchestrator.ts @@ -277,6 +277,7 @@ export class Orchestrator { await say(unavailable(r.reason) ? `I cannot reach the assistant right now (${r.detail.replace(/^the assistant is unavailable: /, "")}). Please try again in a few minutes.` : r.reason === "model" ? badModelNote(orchestratorModel, modelFor(cfg, orchestratorModel), "orchestrator") + : r.reason === "setup" ? `My setup is not complete: ${r.detail}` : r.reason === "budget" ? "That answer hit its spending limit before it finished. Try asking something narrower." : r.reason === "timeout" ? "That took too long to answer. Please try again, or ask something narrower." : "Something went wrong answering that. Please try again; an engineer can find the details in the trace."); diff --git a/src/chat/writer.ts b/src/chat/writer.ts index 259a89f7..7a60ff12 100644 --- a/src/chat/writer.ts +++ b/src/chat/writer.ts @@ -132,6 +132,7 @@ export async function writeIntents(o: { const ref = stepModel(o.cfg, "write-intent"); return { ok: false, costUsd: cost, problem: badModelNote(ref, modelFor(o.cfg, ref), "write-intent"), badModel: true }; } + if (!r.ok && r.reason === "setup") return { ok: false, costUsd: cost, problem: `The writer's setup is not complete: ${r.detail}` }; if (!r.ok) return { ok: false, costUsd: cost, problem: "The writer could not finish." }; if (r.value.status === "fail") return { ok: false, costUsd: cost, problem: r.value.summary || "The writer said the brief is too thin to write up." }; const rendered = r.value.intents.map((d) => { diff --git a/src/phases.ts b/src/phases.ts index e5d8e5b7..65f7d0d7 100644 --- a/src/phases.ts +++ b/src/phases.ts @@ -5,8 +5,8 @@ import { CHAT_SLUG, limitOf, MIN_SESSION_USD, roomBeside, staleBefore, startOfTo import { runPhase, unavailable, type FailureReason, type PermissionMode, type RunPhaseInput } from "./claude"; import { modelFor, type Config, type ModelRef } from "./config"; import { badModelNote, STEP_KEYS, stepModel, type StepKey } from "./models"; -import { BUILTIN_AGENTS, inlineName, isEmptySetup, resolveSetup, setupLines, type AgentSetup } from "./agents"; -import { findAgent, initProblem, inlineAgents, setupAgents, userHome, userSettingsPassthrough, type AgentDefinition, type Where } from "./agent-files"; +import { BUILTIN_AGENTS, isEmptySetup, resolveSetup, setupLines, type AgentSetup } from "./agents"; +import { findAgent, setupSession, userHome, type AgentDefinition, type Where } from "./agent-files"; import type { Trace } from "./trace"; import { GitTimeout } from "./git"; import { CRASH_NOTE, TIMEOUT_NOTE, type StepContext } from "./context"; @@ -585,32 +585,11 @@ async function attempt(ctx: StepContext, spec: AgentPhaseSp * What a session's run gets from its step's setup when that is not empty, and the models of its * inline agents for the trace; `missing`, instead, when an agent it names is not found. * - `tools` and `mcp_tools` go on the allow list (see toolsFor). - * - Every agent it names is read from main (see setupWhere), never the change's branch, and passed - * inline under its Loopstra name (see inlineAgents); the session runs as its agent with `--agent`. - * - `sources: repo` loads project settings only, so the user's sign-in settings ride along in `--settings`. - * - A session outside the snapshot of main (a worktree) also loads main's skills (`--add-dir`). - * - Subagents' text reaches the trace; the init event must list every agent and skill (see initProblem). + * - Every agent it names is read from main (see setupWhere), never the change's branch; the rest is + * setupSession's: inline agents, `--agent`, settings sources, `--add-dir`, subagent text, the init check. */ async function setupRun(ctx: StepContext, spec: AgentPhaseSpec, setup: AgentSetup, cwd: string): Promise<{ run: Partial; models: Record } | { missing: string }> { - const run: Partial = { checkInit: (init) => initProblem(setup, init) }; - let models: Record = {}; - const usesMain = setup.agent !== null || setup.subagents.length > 0 || setup.teams.length > 0 || setup.skills.length > 0; - const where = usesMain ? await setupWhere(ctx, setup) : null; - if (where) { - const found = await setupAgents(setup, spec.name, where, ctx.cfg.main_branch); - if ("missing" in found) return found; - const agents = inlineAgents(found.main, found.others); - if (Object.keys(agents).length) run.agents = agents; - if (found.main) run.agent = inlineName(found.main.name); - models = Object.fromEntries(Object.entries(agents).flatMap(([name, a]) => (a.model ? [[name, a.model]] : []))); - if (setup.skills.length && cwd !== where.repo) run.addDirs = [where.repo]; - } - if (setup.sources === "repo") { - run.settingSources = "project"; - run.settings = JSON.stringify({ ...(JSON.parse(hookSettings()) as Record), ...userSettingsPassthrough(userHome()) }); - } - if (setup.subagents.length || setup.teams.length) run.forwardSubagentText = true; - return { run, models }; + return setupSession(setup, spec.name, ctx.cfg.main_branch, cwd, JSON.parse(hookSettings()) as Record, () => setupWhere(ctx, setup)); } /** diff --git a/tests/unit/chat-orchestrator.test.ts b/tests/unit/chat-orchestrator.test.ts index c3fc0f8f..b4c84d0d 100644 --- a/tests/unit/chat-orchestrator.test.ts +++ b/tests/unit/chat-orchestrator.test.ts @@ -532,6 +532,37 @@ describe("the orchestrator's guards (review fixes)", () => { }); }); +describe("a chat step's agent setup", () => { + test("a chat step gets only its own tools; agents.all never reaches it", async () => { + const r = await chatRepo({ config: "agents:\n all:\n tools: [WebFetch]\n mcp_tools: [mcp__x]\n steps:\n orchestrator:\n skills: [tone]\n mcp_tools: [mcp__docs]\n" }); + const seen = join(r.root, ".loopstra", "fake-args.json"); + const restore = setEnv({ LOOPSTRA_FAKE_ARGS: seen }); + try { + mkdirSync(join(r.root, ".claude", "skills", "tone"), { recursive: true }); + writeFileSync(join(r.root, ".claude", "skills", "tone", "SKILL.md"), "---\nname: tone\ndescription: how to sound\n---\nBe kind.\n"); + await r.answer("orchestrator", 1, turn("ok")); + await new Orchestrator(r.root).handle(message("hi"), sink().send); + const call = JSON.parse(readFileSync(seen, "utf8")) as { args: string[]; prompt: string }; + const flag = (name: string) => call.args[call.args.indexOf(name) + 1]!.split(","); + expect(flag("--allowedTools")).toEqual(expect.arrayContaining(["Read", "mcp__docs"])); + expect(flag("--allowedTools")).not.toContain("WebFetch"); + expect(flag("--allowedTools")).not.toContain("mcp__x"); + expect(flag("--disallowedTools")).toEqual(CHAT_DENIED); + expect(call.prompt.startsWith("Use these skills: `tone`.")).toBe(true); + } finally { restore(); r.cleanup(); } + }); + + test("a chat agent that cannot be found is answered in words", async () => { + const r = await chatRepo({ config: "agents:\n steps:\n orchestrator:\n agent: ghost\n" }); + try { + const out = sink(); + await new Orchestrator(r.root).handle(message("hi"), out.send); + expect(out.sent[0]).toStartWith("My setup is not complete:"); + expect(out.sent[0]).toContain("ghost"); + } finally { r.cleanup(); } + }); +}); + describe("the daily budget is held, not just checked (Copilot review)", () => { test("running sessions count at what they hold; a phase that would go past the day's budget does not start", async () => { const r = await chatRepo({ config: "claude:\n max_budget_usd: 2\nchat:\n max_budget_usd_per_day: 3\n" }); From 7705d36ebd6463cfa9a5d586dbb7ac3584dc5096 Mon Sep 17 00:00:00 2001 From: John Flavan Date: Mon, 26 Oct 2026 23:12:05 -0500 Subject: [PATCH 14/64] test(chat): pin that an empty agents block leaves chat's argv and prompt unchanged Co-Authored-By: Claude Opus 5.5 --- tests/unit/chat-orchestrator.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/unit/chat-orchestrator.test.ts b/tests/unit/chat-orchestrator.test.ts index b4c84d0d..4a5de230 100644 --- a/tests/unit/chat-orchestrator.test.ts +++ b/tests/unit/chat-orchestrator.test.ts @@ -552,6 +552,25 @@ describe("a chat step's agent setup", () => { } finally { restore(); r.cleanup(); } }); + test("with no agents block, or only agents.all, a chat session's argv and prompt are what they always were", async () => { + const record = async (config: string) => { + const r = await chatRepo({ config }); + const seen = join(r.root, ".loopstra", "fake-args.json"); + const restore = setEnv({ LOOPSTRA_FAKE_ARGS: seen }); + try { + await r.answer("orchestrator", 1, turn("ok")); + await new Orchestrator(r.root).handle(message("hi"), sink().send); + return JSON.parse(readFileSync(seen, "utf8")) as { args: string[]; prompt: string }; + } finally { restore(); r.cleanup(); } + }; + const none = await record("agents: {}\n"); + const all = await record("agents:\n sources: all\n all:\n skills: [x]\n tools: [WebFetch]\n steps: {}\n"); + expect(all.args).toEqual(none.args); + expect(all.prompt).toBe(none.prompt); + for (const flag of ["--agents", "--agent", "--setting-sources", "--add-dir", "--forward-subagent-text"]) expect(none.args).not.toContain(flag); + expect(none.args[none.args.indexOf("--allowedTools") + 1]).toBe(["Read", "Glob", "Grep", "Bash(loopstra status)", "Bash(loopstra status *)"].join(",")); + }); + test("a chat agent that cannot be found is answered in words", async () => { const r = await chatRepo({ config: "agents:\n steps:\n orchestrator:\n agent: ghost\n" }); try { From 72a47de2a470587511beae411f876345f95b4186 Mon Sep 17 00:00:00 2001 From: John Flavan Date: Mon, 26 Oct 2026 23:16:47 -0500 Subject: [PATCH 15/64] feat(start): refuse while a step names a missing agent Co-Authored-By: Claude Opus 5.5 --- src/scheduler.ts | 30 ++++++++++++++++++++++++++++- tests/integration/scheduler.test.ts | 9 +++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/scheduler.ts b/src/scheduler.ts index 1afc917b..207bcac2 100644 --- a/src/scheduler.ts +++ b/src/scheduler.ts @@ -5,7 +5,7 @@ import { FAKE_CLAUDE_ENV } from "./claude"; import { configPath, DEFAULT_POLL_SECONDS, loadConfig, NOT_SET_UP, loopPollSeconds, type Config } from "./config"; import { writeFileAtomic } from "./fsutil"; import { MainCheckoutMoved, OFF_MAIN_NOTE, PersonChangedStatus, StepContext, block, clearMarker, onceMarker, personChangedStatus, settleRecord, snapshotsDir, type StepResult } from "./context"; -import { Git, GIT_TIMEOUT_NOTE, GitTimeout, removeStaleLocks, removeWorktree, STALE_LOCK_MS } from "./git"; +import { addDetachedWorktree, Git, GIT_TIMEOUT_NOTE, GitTimeout, removeStaleLocks, removeWorktree, STALE_LOCK_MS } from "./git"; import { GitHub } from "./github"; import { claim, release, runsNoCommands } from "./lanes"; import { activePause, clearPause, dropLoopLock, heartbeatDayPause, heartbeatDoneWith, heartbeatWorkingOn, loopAlreadyRunning, loopHoldText, pauseAfterUnavailable, pidAlive, readLoopHold, runningLoopPid, startHeartbeat, takeLoopLock, type LoopBeat } from "./heartbeat"; @@ -32,6 +32,7 @@ import { olderSetupRefusal } from "./stamp"; import { AssistantUnavailable, installStopSignals, LoopBudgetReached, notTheStepsFault, QuestionAsked, recordNotYet, resetStop, SessionStopped, stopPromise, stopRequested, StopRequested } from "./stop"; import { Trace, type EventType } from "./trace"; import { wakeStamp } from "./wake"; +import { checkAgentSetup, userHome } from "./agent-files"; import { chatDir } from "./chat/threads"; export interface TickResult { @@ -982,6 +983,31 @@ export function log(line: string): void { console.log(`${new Date().toISOString()} ${line}`); } +/** + * What `start` refuses with while a step names an agent main does not have; null when all are there, + * when no agents are set, or when the config does not load (the loop's own handling deals with that). + * Warnings are printed, not refused. The agents are looked up in a snapshot of main, as a session finds them. + */ +async function missingAgents(root: string): Promise { + let cfg: Config; + try { cfg = await loadConfig(root); } catch { return null; } + const a = cfg.agents; + const sets = (s: Config["agents"]["all"]) => Object.values(s).some((v) => (Array.isArray(v) ? v.length > 0 : v !== undefined)); + if (!sets(a.all) && !Object.values(a.steps).some(sets) && !Object.keys(a.teams).length && a.sources === "all") return null; + const git = new Git(root); + const dir = join(snapshotsDir(root), "_start"); + await addDetachedWorktree(git, dir, cfg.main_branch); + let problems; + try { + problems = await checkAgentSetup(cfg, { repo: dir, home: userHome(), sources: a.sources }); + } finally { + await removeWorktree(git, dir, { cleanup: true }); + } + for (const p of problems) if (p.level === "warn") log(p.text); + const fails = problems.filter((p) => p.level === "fail"); + return fails.length ? "Some steps name agents that are not set up:\n" + fails.map((p) => `- ${p.text}`).join("\n") : null; +} + /** * The loop. It never dies: each tick is guarded, a problem is traced, and the next tick comes. * Ctrl-C asks for a stop (the running step is interrupted and resumes on the next start); a @@ -989,6 +1015,8 @@ export function log(line: string): void { * the refusal when another loop holds the lock, or null once the loop has run. */ export async function start(root: string, opts: { once: boolean; installSignals?: boolean }): Promise { + const missing = await missingAgents(root); + if (missing) return missing; const refused = await takeLoopLock(root, await loopPollSeconds(root)); if (refused) return refused; let beat: LoopBeat | null = null; diff --git a/tests/integration/scheduler.test.ts b/tests/integration/scheduler.test.ts index 6580c362..5eea40c6 100644 --- a/tests/integration/scheduler.test.ts +++ b/tests/integration/scheduler.test.ts @@ -331,6 +331,15 @@ describe("scheduler resilience", () => { trace.close(); repo.cleanup(); }, 30_000); + test("start refuses while a step names an agent that main does not have", async () => { + const { repo, trace } = await setupRepo("draft", { config: "agents:\n steps:\n design:\n agent: ghost\n" }); + const refused = await start(repo.path, { once: true, installSignals: false }); + expect(refused).toStartWith("Some steps name agents that are not set up:\n- `ghost`, the agent design runs as"); + expect(existsSync(join(repo.path, ".loopstra", "snapshots", "_start"))).toBe(false); + expect(existsSync(join(repo.path, ".loopstra", "loop.lock"))).toBe(false); + trace.close(); repo.cleanup(); + }); + test("a merged change's leftover worktree and branch are cleaned up on a later tick", async () => { const { repo, ctx, trace } = await setupRepo("done"); const git = new Git(repo.path); From 521c32c9a27c073500c557e0ab1b96e8d4e92bd3 Mon Sep 17 00:00:00 2001 From: John Flavan Date: Mon, 26 Oct 2026 23:26:46 -0500 Subject: [PATCH 16/64] docs: agent setup in the config template, skill, README and decisions Co-Authored-By: Claude Opus 5.5 --- README.md | 5 +++-- docs/decisions.md | 24 ++++++++++++++++++++++++ templates/config.yaml | 21 ++++++++++++++++++--- templates/skill/SKILL.md | 4 ++-- 4 files changed, 47 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 0f51fe89..f89eba86 100644 --- a/README.md +++ b/README.md @@ -298,14 +298,15 @@ The full design is in `docs/superpowers/specs/2026-09-28-loopstra-design.md`, wi - `main_branch`, `poll_seconds` - `commands`: test (required), install, lint, build, run -- `claude`: models (`cheap`, `default`, `strong`, `max`: by default the aliases `haiku`, `sonnet`, `opus` and `fable`, which Claude Code resolves to the newest model of each family; a full name such as `claude-opus-5-5` pins one), `timeout_minutes`, optional spending limits (`max_budget_usd` per session, `max_budget_usd_per_day` for the loop's sessions together; unset means no limit), and `allowed_tools` for build sessions (the configured commands are always added; anything else a build needs, like `"Bash(make *)"`, goes here), and `mcp_tools`: MCP tools every session may use, judges included (`mcp__` for all of a server's tools, `mcp____` for one). Sessions load the MCP servers, skills and CLAUDE.md files of the machine and repository they run in, but without an allow rule an MCP call is refused (a session cannot ask). A server can still be connecting when a session starts, so its tools may only appear after the first turn +- `claude`: models (`cheap`, `default`, `strong`, `max`: by default the aliases `haiku`, `sonnet`, `opus` and `fable`, which Claude Code resolves to the newest model of each family; a full name such as `claude-opus-5-5` pins one), `timeout_minutes`, optional spending limits (`max_budget_usd` per session, `max_budget_usd_per_day` for the loop's sessions together; unset means no limit), and `allowed_tools` for build sessions (the configured commands are always added; anything else a build needs, like `"Bash(make *)"`, goes here). Sessions load the MCP servers, skills and CLAUDE.md files of the machine and repository they run in, but without an allow rule an MCP call is refused (a session cannot ask). A server can still be connecting when a session starts, so its tools may only appear after the first turn +- `agents`: which agents, subagents, skills and tools each step's session gets. `sources` is `all` (the default: also the personal skills, agents, plugins and MCP servers of whoever runs `loopstra start`) or `repo` (only what the repository declares, the same on every machine; the user's `apiKeyHelper` and `env` still pass through so sessions can sign in). `teams` names groups of subagents (`members`, and `way`: `parallel` gives each the same task, `pick` hands each piece of work to the member that fits). `all` applies to every loop step and `steps.` adds to it; each takes `agent`, `subagents`, `teams`, `skills`, `tools` and `mcp_tools` (`mcp__` for all of a server's tools, `mcp____` for one; MCP tools reach every session of the step, judges included). Lists add up, each name once; `agent` is one value and the step's own wins. Agents and skills are Claude Code's own files, committed in the repository's `.claude/`; Loopstra reads agent files from main and passes them to the session inline as `loopstra-`, so a change cannot rewrite the role that reviews it. Safety does not move: read-only steps never get `Edit`, `Write`, `NotebookEdit` or a bare `Bash`, and no step's `tools` may hold git that changes history or a remote (`commit`, `push`, `merge`, `rebase`, `reset`, `checkout`, `switch`, `branch`, `tag`, `stash`, bare `git`). `agent` cannot be set on `fix`, `reconcile` or `revise`, which continue the build's session and run as the build's agent, and a built-in agent such as `Explore` can be a subagent but not a step's agent. Chat's two steps (`orchestrator`, `write-intent`) take only their own entries under `steps` (never `all`, and not the old shared MCP rule), and anyone allowed to chat can steer the tools given there. `loopstra start` refuses while a step names an agent that is missing, and only warns about a missing skill (built-in and claude.ai skills are not files on disk). If a session starts without a named agent or skill, the step fails with the reason `setup` and is not retried; the owner note says what is missing. What each step ran as shows in its trace (`agent_setup`). - `gates`: spec, plan, merge, done, each with `human` (`status` or `none`; merge also `pr`) and `agent` (independent reviewer); merge also `method` (`squash` or `merge`) - `stages`: per-stage model, `steps` (a model for single steps; see Models for each step), skills, `before`/`after` commands, and loop limits - `speed`: work that runs at the same time to save waiting; only `cheap_judges: first` changes who decides (once earned, a cheaper judge's pass stands). `overlap_judges` (on) runs the first review alongside the verifier (in a copy of the tested commit), and lessons alongside the final check; when the verifier fails, the review running beside it is stopped at once and round 1 runs after the fix. With `claude.max_budget_usd_per_day` set, they overlap only when what is left of the day covers two whole sessions (`claude.max_budget_usd`), so never without a session limit. `parallel_checks` (off) starts test, lint and build together, and stops the rest once one fails; leave it off if they write the same folder. `plan_ahead` (on) writes the first plan while the spec is being checked. The plan is stopped if the spec does not pass, and it is used only if the code on `main` it read (outside `intent/` and `loopstra/config.yaml`) is still the same commit's code when planning starts. It follows the same daily budget rule, and does not run with a person on the spec gate, without a spec judge, with plan `before` commands, or while another change is about to merge. `cheap_judges` (off) asks a cheaper model (`claude.models.default`) the same question as the spec check, the plan challenge and the review: `shadow` only records whether it agreed with the strong judge, which still decides, and `first` lets it pass something alone once it has also caught the strong judge's last 30 rejections, sending anything it rejects or is unsure of to the strong judge (`loopstra judges` shows the record). In `first`, every 5th call still runs both judges as an audit, with the strong judge deciding, so a later miss sends it back to `shadow`. Like the overlaps, `shadow` and the audits run only with room for every session at once (three when the early review runs beside the verifier, or the spec check beside a plan written ahead), so never with a daily budget and no session limit; then the strong judge decides alone, and in `first` the cheaper judge never decides alone. `think_ahead` (on) runs design and plan steps beside the changes running a step (see How it works). `max_parallel_changes` (1, up to 4) is how many changes may each run a step at the same time. Raise it only if the project's commands (test, lint, build, install) can run twice at once on this machine: no fixed ports, no shared test database, no output outside the worktree. Each command, and each session that may run commands, gets `LOOPSTRA_SLUG` and `LOOPSTRA_SLOT`: a small number (0, 1, ...) that no other change running at the same time has, so it can pick its own ports, database and Docker project names (port `3100 + LOOPSTRA_SLOT`, say). Main's health check runs alone and gets no slot. More changes at once also spend the daily budget and the subscription's usage limits faster. With `claude.max_budget_usd_per_day` set, only as many run at once as what is left of the day covers whole sessions (`claude.max_budget_usd`), so one at a time without a session limit; each overlap above also counts the sessions of the other changes running. - `signals`: how often main's health check runs - `chat`: the orchestrator's model (`model`), the model of the write-up (`steps.write-intent`), optional spending limits (`max_budget_usd_per_day`, `max_budget_usd_per_session`), and the Slack and Discord bots under `transports` (see Chat) -`loopstra setup` asks about the commands, spending limits, gates, merging, chat and models (the four names, each stage's model, and, if you say yes, a model for each of its steps); edit the file by hand for the rest (`poll_seconds`, `allowed_tools`, `mcp_tools`, stage skills and commands, speed, signals). During onboarding the `loopstra` skill lists the MCP tools its own session has, says which only read and which change things, and writes the ones you pick to `mcp_tools`. +`loopstra setup` asks about the commands, spending limits, gates, merging, chat and models (the four names, each stage's model, and, if you say yes, a model for each of its steps); edit the file by hand for the rest (`poll_seconds`, `allowed_tools`, stage skills and commands, speed, signals). During onboarding the `loopstra` skill lists the MCP tools its own session has, says which only read and which change things, and writes the ones you pick to `agents.all.mcp_tools`. **Models for each step.** The four names are `cheap`, `default`, `strong` and `max`. Each stage has a model, and so does chat; a step can have its own under `stages..steps.` (or `chat.steps.write-intent`), set to one of the four names. A step with none of its own runs on its default below. Nothing is put on `max` by a default: only a person does that. diff --git a/docs/decisions.md b/docs/decisions.md index 5a070d63..1858ff2b 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -814,3 +814,27 @@ A record (a status, an artifact, a conversation entry, a person's edits, chat's - **`claude.mcp_tools`, allowed in every tool set.** Sessions load the user's and the repository's MCP servers (no `--strict-mcp-config`), but a `-p` session cannot ask, and no allow list had an `mcp__` rule, so every MCP call was refused outside plan mode (checked on the real CLI: refused in `default` and `acceptEdits`, allowed in `plan` for a tool marked read-only). The rules are added after each set's own tools, judges' included: a judge may need the same lookup (an issue, a doc) as the build. They are not limited to read-only tools; what an MCP tool may change is the owner's call when they list it. - **Rules, not tool lists.** Each entry must start with `mcp__`, so the setting cannot widen a judge's shell or file access; `allowed_tools` stays the place for those, build sessions only. - **The skill offers the MCP tools; setup does not ask.** `loopstra setup` cannot see an MCP server's tools without connecting to it (`claude mcp list` names servers only). The operator skill runs in a Claude Code session that already has them, so it lists them by server, says which only read and which change things, and writes the engineer's picks. None is picked by default. A project's `.mcp.json` servers load in `-p` sessions without approval (checked on the real CLI), so committing the file is enough for the loop to see them. + +## Agent setup for every step (2026-10-26) + +A new `agents` block in `loopstra/config.yaml` gives each step an agent to play, subagents and teams to hand work to, skills, and extra tools and MCP tools. With no block, or an empty one, every session's flags and prompt are what they were before (pinned by a test). + +- **What the CLI does (checked on Claude Code 2.1.311, `-p`, stream-json).** `--agents ''` with `--agent ` runs the main session as an inline agent, and `--model` wins over the agent's `model`. `--disallowedTools` also binds subagents and `--agent`, so the read-only guard holds. `--add-dir ` loads that folder's `.claude/skills`; a skill of the same name in the working directory wins. `--setting-sources project` drops user skills, agents, plugins and user MCP servers while project ones load. `total_cost_usd` includes subagents. The `init` event carries `agents` and `skills`; the fake CLI's fixtures carry neither, so absent means "cannot tell". +- **`claude.mcp_tools` moved to `agents.all.mcp_tools` before release.** The setting had not shipped, so there is no alias: one place says which MCP tools every session may use. Chat steps never inherit it; they keep `CHAT_DENIED` and the scrubbed environment and take only the tools on their own entries. The operator skill writes `agents.all.mcp_tools`. +- **Agent files are read from main and passed inline as `loopstra-`.** The loop's settings are main's, so the definitions are: they are read from the step's snapshot of main (or the user's folder with `sources: all`) and handed over with `--agents`, so a same-named file in a change's worktree never wins and a change cannot rewrite the role that reviews it. `:` becomes `-` (`superpowers:code-reviewer` is `loopstra-superpowers-code-reviewer`). The snapshot is made only when the setup names an agent, subagent, team or skill. +- **`StructuredOutput` is added to a main agent's tools.** An agent file with its own `tools:` list loses the structured-output tool otherwise (checked: `structured_output` comes back null). When the file lists tools, the inline copy gets `StructuredOutput` too. Tools are not intersected with the step's allow list: permissions (allow rules and `--disallowedTools`) still gate every call, so a definition cannot widen a step; a tool it names that the step refuses is denied and shows in the trace. +- **A built-in agent cannot be a step's main agent.** `Explore`, `Plan` and the others have fixed tool lists without `StructuredOutput`, so the step could not report. They work as subagents; a team that wants one as a role defines its own. +- **`fix`, `reconcile` and `revise` take no `agent`.** They resume the build's session and run as the build's agent; the inline agents are passed again on every resume. Their other lists layer as for any step. +- **Stage skills alone keep today's behaviour.** The setup counts as empty when only `stages..skills` is set: the prompt line, with no snapshot, `--add-dir` or init check. Once the setup is non-empty for another reason, stage skills join the `--add-dir` and the init check. +- **Skills only main has reach worktree steps by `--add-dir`.** Claude Code loads skills from the working directory, which for a step in a worktree is the change's branch. The step's snapshot of main is added with `--add-dir`, so main's skills are the ones available. +- **`sources: repo` passes the user's sign-in through.** It is `--setting-sources project`, which also drops the user's `apiKeyHelper` and `env` from `~/.claude/settings.json`; Loopstra passes those two through its own `--settings` so sessions can still sign in. `.claude/settings.local.json` stays out, since it is personal. +- **The start check refuses on missing agents and warns on skills.** `loopstra start` refuses while a step names an agent, subagent or team member that is not in the repository (or, with `sources: all`, the user's folder). A missing skill is a warning: built-in and claude.ai skills are not files on disk, so the file check cannot tell them from a typo. A name found only on this machine also warns. +- **The init event is the last check.** A session whose `init` event lacks a named agent or skill fails the step with reason `setup`: not an outage, not retried, with a note naming the step and what is missing. When the event lists neither (the fake CLI), nothing is checked. +- **Chat steps take only their own entries.** `orchestrator` and `write-intent` run in the main checkout, steered by people in channels, so `agents.all` does not reach them. A missing name in a chat turn is answered in the reply and traced under `_chat`, not a refusal. +- **The trace shows it.** Each agent step writes an `agent_setup` event before the session (agent, subagents, teams, skills, tools, MCP tools, sources, models). + +### Live check (Claude Code, haiku for every model, `max_budget_usd: 1`, a toy Bun project, about $0.71 in all) + +- **Intake through an inline agent parsed.** `agents.steps.intake.agent: lead` (a committed `.claude/agents/lead.md` with `tools: Read`) ran as `--agent loopstra-lead`; the session called `StructuredOutput` and the envelope came back valid (priority, clarity and summary). This is the case that returned `null` before `StructuredOutput` was added. +- **The build's `agent_setup` event.** `{"agent":"lead","subagents":["Explore"],"teams":[],"skills":[],"tools":[],"mcp_tools":[],"sources":"all","models":{}}` for build with `lead` and `subagents: [Explore]`. With `lead` the build reported that it had only `Read` and `StructuredOutput` and could not write: the agent's narrow tools do narrow the step, as designed. Switching build to a `builder` agent with `Read, Write, Edit, Glob, Grep, Bash` built the change, and it went through to `done`. +- **`--resume` with the same `--agent` kept the session id.** On a second change a failing lint check sent the loop to fix-1, fix-2 and fix-3. All three, and the build, ran under session `1394159d-3245-4ded-a5e1-1cb973d8e60f`, each with `agent_setup` showing agent `builder` (and no subagents on the fixes). The toy blocked after three fixes only because the agent declined to create a file its plan called out of scope; that is the project's doing, not the setup's. diff --git a/templates/config.yaml b/templates/config.yaml index 84dccd86..70bf752f 100644 --- a/templates/config.yaml +++ b/templates/config.yaml @@ -18,9 +18,24 @@ claude: # What a build session may do without asking. The commands above (test, lint, build, run, install) # are always added. Git stays read-only: Loopstra makes every commit itself. The default: # allowed_tools: [Read, Edit, Write, Glob, Grep, "Bash(bun *)", "Bash(git diff *)", "Bash(git log *)", "Bash(git show *)", "Bash(git status *)"] - # MCP tools every session may use, read-only judges included (none by default; an MCP call - # without a rule here is refused). mcp__ allows all of a server's tools: - # mcp_tools: [mcp__github, "mcp__claude_ai_Claude_Docs__guide"] + # Agents, subagents, skills and MCP tools for each step: a top-level `agents:` block of its own (not + # under claude). Every line is optional; with none set, sessions run exactly as before. Agents and skills are Claude Code's own files, committed under + # .claude/agents/.md and .claude/skills//; this block only names which step uses which. + # sources: all (default) also loads the personal setup of whoever runs `loopstra start`; repo loads + # only what the repository declares (.claude/, .mcp.json, CLAUDE.md), the same on every machine. + # teams are named groups of subagents (way: parallel gives each member the same task; pick hands each + # piece of work to the member that fits). all applies to every step except chat's; steps. adds + # to it. mcp_tools are MCP tools every session may use, read-only judges included (none by default; + # an MCP call without a rule is refused). mcp__ allows all of a server's tools. + # agent is one agent a step's session plays (not on fix, reconcile or revise, which continue the build's). + # agents: + # sources: all + # teams: + # reviewers: { members: [security-reviewer, correctness-reviewer], way: parallel } + # all: + # mcp_tools: [mcp__github, "mcp__claude_ai_Claude_Docs__guide"] + # steps: + # review: { agent: lead-reviewer, teams: [reviewers], subagents: [Explore], skills: [threat-modeling] } # Spending limits, in US dollars, are off unless set (`loopstra setup budgets` sets them in minutes or dollars): # max_budget_usd: what one session may spend # max_budget_usd_per_day: what the loop's sessions may spend together in a day diff --git a/templates/skill/SKILL.md b/templates/skill/SKILL.md index 8840c0d6..e35a75a9 100644 --- a/templates/skill/SKILL.md +++ b/templates/skill/SKILL.md @@ -29,10 +29,10 @@ draft -> accepted -> designing -> spec-review -> spec-approved -> planning -> pl ## Onboard 1. Run `loopstra init` at the repository's top folder (in a folder below it, init names the top and writes nothing) and read what it printed. Files it reports as "kept" already existed and were left alone. The first steps are always in this order: setup, commit, start. If it says the repo was set up by an older Loopstra, have them run `loopstra upgrade` (see Upgrade). 2. Have them run `loopstra setup` in a terminal (`init` offers it at the end). It walks through budgets, commands, gates, GitHub, chat and models (Enter takes the suggestion; `-` leaves an optional answer empty, and a command left out that way stays out), saves `loopstra/config.yaml` keeping its comments, and checks what it can: `claude`, `commands.test` on main, the git remote and `gh`, the chat tokens. "To fix" names the `loopstra setup
` to run again. One section at a time: `loopstra setup budgets` (or `commands`, `gates`, `github`, `chat`, `models`; `loopstra setup --help` lists them); `loopstra setup --check` changes nothing and only checks. If it says "Fix these in loopstra/config.yaml first:", those problems are outside what its questions ask (an unknown key, for example): help them fix the file, then run it again. Your part is to help them decide, in their context, and tell them which section to run; do not edit `config.yaml` yourself for anything setup asks. - Build sessions may always run the configured commands; anything else they need (another tool, a script) goes in `claude.allowed_tools`, for example `"Bash(make *)"`. Setup does not ask about it, so that one is edited in the file; so is `claude.mcp_tools` (step 5). Refused commands show in the console and `loopstra tail`. + Build sessions may always run the configured commands; anything else they need (another tool, a script) goes in `claude.allowed_tools`, for example `"Bash(make *)"`. Setup does not ask about it, so that one is edited in the file; so is `agents.all.mcp_tools` (step 5). Refused commands show in the console and `loopstra tail`. 3. A person always accepts an intent (draft to accepted). The other gates (spec, plan, merge, done) are unattended by default; setup's `gates` and `github` sections change that. With a git remote every change goes through a pull request, so `gh` must be signed in. Budgets are off by default (no limit; `timeout_minutes` still ends a session); suggest limits only if they pay per use with an API key or want a ceiling. 4. Ask which skills in `.claude/skills/` each stage should load and list them under `stages..skills`. -5. Offer the MCP tools. The loop's sessions may call only the MCP tools in `claude.mcp_tools`; any other MCP call is refused. List every MCP tool your own session has, grouped by server: the ones you were given and the deferred ones (search with ToolSearch, for example the query `mcp__`). Run `claude mcp list` too, and name the servers that need sign-in or failed to connect, since you cannot see their tools. For each tool, say in a few words what it does and whether it only reads or also changes something (sends, creates, edits, deletes). Ask the engineer which ones the loop may use. Nothing is allowed unless they pick it, and none is fine. Say that every session gets the picks, read-only judges included, so any session can call a tool that sends or deletes. Write their choice under `claude:` in `loopstra/config.yaml` as `mcp_tools: [...]`, using the names as your tools show them: `mcp__` for all of a server's tools, `mcp____` for one. The loop's sessions get the MCP servers of the machine and user that run `loopstra start`, plus the repository's committed `.mcp.json`. If the loop will run on another machine or as another user, say that a server missing there is unavailable to the loop, whatever the config allows. +5. Offer the MCP tools. The loop's sessions may call only the MCP tools in `agents.all.mcp_tools`; any other MCP call is refused. List every MCP tool your own session has, grouped by server: the ones you were given and the deferred ones (search with ToolSearch, for example the query `mcp__`). Run `claude mcp list` too, and name the servers that need sign-in or failed to connect, since you cannot see their tools. For each tool, say in a few words what it does and whether it only reads or also changes something (sends, creates, edits, deletes). Ask the engineer which ones the loop may use. Nothing is allowed unless they pick it, and none is fine. Say that every session gets the picks, read-only judges included, so any session can call a tool that sends or deletes. Write their choice in `loopstra/config.yaml` as `mcp_tools: [...]` under `all:` under `agents:` (`agents.all.mcp_tools`), using the names as your tools show them: `mcp__` for all of a server's tools, `mcp____` for one. The loop's sessions get the MCP servers of the machine and user that run `loopstra start`, plus the repository's committed `.mcp.json`. If the loop will run on another machine or as another user, say that a server missing there is unavailable to the loop, whatever the config allows. 6. Read `CLAUDE.md`; make sure its Commands block matches the config. 7. Commit what `init` wrote, with the config as setup left it and the edits above, on main: the loop works in its own checkouts, which only see what is committed, and `loopstra start` refuses until the config, `loopstra/stamp.json` and any customized prompt are committed. Commit the config again whenever setup changes it. 8. Tell them to start the loop in a terminal with `loopstra start` and to watch it with `loopstra status` or `loopstra ui`. Product owners can use `loopstra chat` or the console's orchestrator (Ctrl J) instead of writing intents by hand (see Chat). From c035e181af8ce1f5157d47a87dbc58bea8c4d2b4 Mon Sep 17 00:00:00 2001 From: John Flavan Date: Mon, 26 Oct 2026 23:30:46 -0500 Subject: [PATCH 17/64] docs: plan for agent setup, part 2 (setup section) Co-Authored-By: Claude Opus 5.5 --- .../plans/2026-10-27-agent-setup-2-setup.md | 249 ++++++++++++++++++ 1 file changed, 249 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-27-agent-setup-2-setup.md diff --git a/docs/superpowers/plans/2026-10-27-agent-setup-2-setup.md b/docs/superpowers/plans/2026-10-27-agent-setup-2-setup.md new file mode 100644 index 00000000..18ae185a --- /dev/null +++ b/docs/superpowers/plans/2026-10-27-agent-setup-2-setup.md @@ -0,0 +1,249 @@ +# Agent Setup, Part 2: Setup Section and the Agent Library Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** `loopstra setup agents` lets an engineer choose where sessions get their setup from, set each step's agent, subagents, teams, skills, tools and MCP tools, and add skills and agents the repo lacks (copied from this machine or a plugin, fetched from a git repository or folder, or a new agent from a short form); `loopstra setup --check` reports missing names. + +**Architecture:** A new library module (`src/agent-library.ts`) lists what is available (repo, personal, plugins), copies a skill folder or agent file into the repo, scans a fetched folder, clones a git source shallowly into a temporary folder, and writes a new agent file. It returns files to write; it never writes the repo itself, so the console (Part 3) can reuse it through its request files. The setup runner gains pending file writes, written only when the config saves. The new `agents` section asks the questions with the existing `Prompt` and `ConfigDocument`. + +**Tech Stack:** Bun, TypeScript, Zod 4, `yaml`, bun:test; setup test helpers in `tests/setup-helpers.ts` (`scripted`, `configRepo`, `askSection`, `checkSection`). + +**Spec:** `docs/superpowers/specs/2026-10-26-agent-setup-design.md` ("Getting skills and agents", "Setup (`loopstra setup agents`)", "Checks"). Part 1 (`docs/superpowers/plans/2026-10-26-agent-setup-1-runtime.md`) is built on this branch. + +## Global Constraints + +- The `agents` section runs in a full `loopstra setup` only after a yes to `Set up agents for the loop's steps now?` (suggestion: no). Run alone (`loopstra setup agents`) it skips that question. Under `--defaults` it changes nothing. +- Setup never commits. Nothing (config or files) is written when setup stops or fails; files are written only after the config saved (or was unchanged and files were added), then it says to commit `loopstra/config.yaml` and `.claude/` on main. +- Files written only under `/.claude/agents/` and `/.claude/skills/`; every path is checked to stay inside them (no `..`, no absolute paths, no symlinks followed out of a source). +- Downloaded and copied content is data: nothing in it runs; a git source is cloned with `git clone --depth 1 -- ` into a new temporary folder that is removed afterwards; a URL starting with `-` is refused; before anything is written, the person sees the list of files to be written. +- A copied skill or agent records where it came from: a skill folder gets `SOURCE.md` (one line: `Copied from [ ()] on .`); an agent file gets a `source:` frontmatter key. `source` is added to `parseAgentFile`'s known keys (not dropped, not passed inline). +- Limits on a copy: at most 200 files and 5 MB per skill folder; larger is refused in words. +- Names: an agent or skill name is lowercase letters, digits and dashes (`/^[a-z0-9][a-z0-9-]*$/`); a plugin item keeps its `plugin:name` when used from the plugin, and gets its plain `name` when copied into the repo. +- Answers: `-` empties an optional answer (as in the other sections); lists are typed comma-separated or picked with `pickMany`. +- Copy follows the codebase: plain sentences; the other sections' question style ("Model for the design stage"). + +## Review Focus + +1. A git source with a skill whose folder holds a symlink pointing outside the clone → the link is not followed; the copy is refused naming the file. (Task 1.) +2. A config with `agents.steps.review.agent: lead` where `.claude/agents/lead.md` is only in the working tree, not committed → `--check` reads the working tree at `` (setup edits that tree, and its save message says to commit), so it passes; `loopstra start` (Part 1) checks main and refuses until it is committed. Pinned: `--check` reads ``. (Task 4.) +3. The person adds a skill, then stops setup (Ctrl-C, `SetupStopped`) → no file is written. (Task 2.) +4. Picking "none" for a step that had an agent removes `agents.steps..agent`, and a step left with no settings is removed from `agents.steps` entirely (no empty `{}` left in the file). (Task 3.) +5. A plugin skill copied into the repo whose name collides with an existing repo skill → refused unless the person confirms overwriting; the existing folder is replaced whole, not merged. (Task 1 for the function, Task 3 for the question.) + +--- + +## File structure + +- Create `src/agent-library.ts`: `LibraryItem`, `FileWrite`, `listLibrary`, `copyItem`, `scanFolder`, `cloneSource`, `newAgentFile`, `repoPath`. +- Modify `src/agent-files.ts`: `source` is a known frontmatter key. +- Modify `src/setup/types.ts`: `SetupContext.files` (pending writes) and `SetupContext.only`. +- Modify `src/setup/index.ts`: create `files`, set `only`, write files after save, say so. +- Create `src/setup/sections/agents.ts`; modify `src/setup/sections/index.ts` (last in `SECTIONS`). +- Modify `tests/setup-helpers.ts` (`askSection` writes pending files, accepts `only`). +- Tests: `tests/unit/agent-library.test.ts` (new), `tests/unit/setup-agents.test.ts` (new), `tests/unit/setup.test.ts` or the file covering `setup()` (find with `grep -l "await setup(" tests`). +- Docs: `README.md` (setup paragraph), `docs/decisions.md` (Part 2 bullets). + +--- + +### Task 1: The agent library + +**Files:** +- Create: `src/agent-library.ts` +- Modify: `src/agent-files.ts` (known keys) +- Test: `tests/unit/agent-library.test.ts` + +**Interfaces:** +- Consumes: `findAgent`, `findSkill`, `parseAgentFile`, `Where`, `userHome` (src/agent-files.ts); plugin listing reads the same `installed_plugins.json` and `enabledPlugins` as `findAgent` (reuse its private helpers by exporting one function `enabledPlugins(home): Array<{ plugin: string; installPath: string }>` from src/agent-files.ts). +- Produces: + - `export interface LibraryItem { kind: "agent" | "skill"; name: string; description: string; from: "repo" | "personal" | "plugin" | "source"; path: string }` (`path`: the agent file, or the skill folder; `name` is `plugin:name` for plugin items) + - `export interface FileWrite { path: string; content: string | null }` (`path` relative to the repo root, `/`-separated; `null` removes) + - `export function listLibrary(root: string, home: string, sources: "all" | "repo"): LibraryItem[]`: repo items, then (with `all`) personal, then enabled plugins' items; sorted by kind then name within each group. + - `export function scanFolder(dir: string): LibraryItem[]` (`from: "source"`): every `SKILL.md` (its folder is the skill; name from frontmatter `name`, else the folder name) and every `*.md` directly in a folder named `agents` (name from frontmatter, else the file name); hidden folders other than `.claude` and `node_modules` are skipped. + - `export function copyItem(item: LibraryItem, o: { root: string; source?: string; today: string }): FileWrite[] | { refused: string }`: an agent → one write `.claude/agents/.md` (frontmatter gains `source: `); a skill → every file of the folder under `.claude/skills//`, plus `SOURCE.md`. Refusals: a symlink inside the folder; over 200 files or 5 MB; a name failing the name rule. Text files are read as UTF-8; a binary file (contains a NUL byte in its first 8000 bytes) is refused for now (`content` is a string). + - `export async function cloneSource(url: string): Promise<{ dir: string; cleanup: () => void } | { refused: string }>`: `url` starting with `-`, or not `https://`, `ssh://`, `git@`, `file://` or an existing local folder path → refused. A local folder is returned as is with a no-op cleanup. + - `export function newAgentFile(o: { name: string; description: string; prompt: string; tools: string[]; model: string | null }): FileWrite` + - `export function repoPath(p: string): string | null`: the normalized repo-relative path when it lies inside `.claude/agents/` or `.claude/skills/`, else null. + +- [ ] **Step 1: Write the failing tests** + +```ts +test("listLibrary: repo, then personal, then enabled plugins", () => { + // repo: .claude/agents/lead.md, .claude/skills/house/SKILL.md; home: .claude/agents/mine.md, .claude/skills/tone/SKILL.md; + // plugin sp@m (enabled): agents/cr.md, skills/tdd/SKILL.md + expect(listLibrary(repo, home, "all").map((i) => `${i.from}:${i.kind}:${i.name}`)).toEqual([ + "repo:agent:lead", "repo:skill:house", "personal:agent:mine", "personal:skill:tone", "plugin:agent:sp:cr", "plugin:skill:sp:tdd", + ]); + expect(listLibrary(repo, home, "repo").map((i) => i.name)).toEqual(["lead", "house"]); +}); + +test("copyItem copies a skill folder whole, records its source, and refuses links and size", () => { + // personal skill tone/ with SKILL.md, scripts/run.sh, references/a.md + const w = copyItem(tone, { root: repo, source: "https://example.com/skills.git (tone)", today: "2026-10-27" }) as FileWrite[]; + expect(w.map((f) => f.path).sort()).toEqual([".claude/skills/tone/SKILL.md", ".claude/skills/tone/SOURCE.md", ".claude/skills/tone/references/a.md", ".claude/skills/tone/scripts/run.sh"]); + expect(w.find((f) => f.path.endsWith("SOURCE.md"))!.content).toBe("Copied from https://example.com/skills.git (tone) on 2026-10-27.\n"); + // symlinkSync(join(t, "outside.txt"), join(toneDir, "leak")) → { refused } naming "leak" + // 201 files → { refused } saying "at most 200 files" +}); + +test("copyItem gives a plugin agent its plain name and a source key", () => { + const w = copyItem(pluginCr, { root: repo, today: "2026-10-27" }) as FileWrite[]; + expect(w[0]!.path).toBe(".claude/agents/cr.md"); + expect(parseAgentFile("cr", w[0]!.content!, "repo", "p")).toMatchObject({ dropped: [] }); + expect(w[0]!.content).toContain("source: "); +}); + +test("scanFolder finds skills and agents in a fetched source", () => { + // dir: skills/a/SKILL.md, nested/b/SKILL.md (name: bee), agents/x.md, .git/agents/y.md (skipped), docs/agents.md (not in an agents folder) + expect(scanFolder(dir).map((i) => `${i.kind}:${i.name}`).sort()).toEqual(["agent:x", "skill:a", "skill:bee"]); +}); + +test("cloneSource refuses option-like and unknown URLs, and clones a file:// repo shallowly", async () => { + expect(await cloneSource("--upload-pack=x")).toEqual({ refused: expect.stringContaining("") }); + expect(await cloneSource("ftp://x")).toHaveProperty("refused"); + // a temp git repo with one commit: cloneSource(`file://${repo}`) → dir has its files; cleanup removes dir +}); + +test("repoPath keeps writes inside .claude/agents and .claude/skills", () => { + expect(repoPath(".claude/skills/a/SKILL.md")).toBe(".claude/skills/a/SKILL.md"); + expect(repoPath(".claude/agents/../settings.json")).toBeNull(); + expect(repoPath("/etc/passwd")).toBeNull(); + expect(repoPath(".claude\\agents\\x.md")).toBe(".claude/agents/x.md"); +}); + +test("newAgentFile writes frontmatter that parses back", () => { + const f = newAgentFile({ name: "lead", description: "Leads reviews", prompt: "You lead.", tools: ["Read", "Grep"], model: null }); + expect(f.path).toBe(".claude/agents/lead.md"); + expect(parseAgentFile("lead", f.content!, "repo", "p")).toMatchObject({ description: "Leads reviews", prompt: "You lead.", tools: ["Read", "Grep"], model: null, dropped: [] }); +}); +``` + +- [ ] **Step 2: Run to see them fail.** `bun test tests/unit/agent-library.test.ts` → FAIL (module missing). +- [ ] **Step 3: Implement.** `cloneSource` runs `git clone --depth 1 --quiet -- ` with `Bun.spawn` (no shell), a 2-minute timeout, `GIT_TERMINAL_PROMPT=0` in its env so it never waits for a password. Use `lstatSync` to detect symlinks. Frontmatter is written with `yaml`'s `stringify` so descriptions with `:` stay valid. +- [ ] **Step 4: Run tests.** → PASS; `bun run check` → PASS. +- [ ] **Step 5: Commit** `feat(agents): a library to list, copy, fetch and write skills and agents`. + +--- + +### Task 2: Pending file writes in setup + +**Files:** +- Modify: `src/setup/types.ts`, `src/setup/index.ts`, `tests/setup-helpers.ts` +- Test: the test file covering `setup()` + +**Interfaces:** +- Consumes: `FileWrite`, `repoPath` (Task 1). +- Produces: + - `SetupContext.files: FileWrite[]` (pending; a later write to the same path replaces an earlier one) and `SetupContext.only: boolean` (true when one section was named). + - `export function writeFiles(root: string, files: FileWrite[]): string[]` in `src/setup/index.ts` or a small `src/setup/files.ts`: writes each (via `writeFileAtomic`, making folders; `null` removes the file; a skill folder replaced whole is expressed by the section as removals plus writes), refuses (throws) any path `repoPath` rejects, returns the paths written. + - `runSetup`: after `doc.validate()` and `doc.save()`, when `files.length`, `writeFiles` and print `Wrote file(s) under .claude/: . Commit them with loopstra/config.yaml on the main branch.` When the config was unchanged and files were written, print that line instead of "No changes.". A section that throws, or `SetupStopped`, writes nothing. + - `askSection(section, root, answers, env, o?: { only?: boolean })` in tests/setup-helpers.ts writes pending files after `doc.save()` and returns them as `files: string[]` too. + +- [ ] **Step 1: Write the failing tests** + +```ts +test("files a section adds are written after the config saves, and not at all when setup stops", async () => { + // a fake Section whose ask() pushes { path: ".claude/skills/a/SKILL.md", content: "x" } and sets one config value + // setup(root, { sections: [fake], prompt: scripted(...).prompt, interactive: true }) → 0; the file exists; output contains "Wrote 1 file under .claude/: .claude/skills/a/SKILL.md" + // a fake Section that pushes a file then throws SetupStopped → 1; the file does not exist + // a fake Section that pushes ".claude/../x" → not saved, output names the path +}); +``` + +- [ ] **Step 2: Run to see it fail.** +- [ ] **Step 3: Implement.** +- [ ] **Step 4: Run tests.** → PASS; `bun run check`. +- [ ] **Step 5: Commit** `feat(setup): sections can add files, written only when the config saves`. + +--- + +### Task 3: The `agents` section + +**Files:** +- Create: `src/setup/sections/agents.ts` +- Modify: `src/setup/sections/index.ts` (`SECTIONS` ends with `agents`) +- Test: `tests/unit/setup-agents.test.ts` + +**Interfaces:** +- Consumes: Tasks 1-2; `STEPS`, `STEP_KEYS` (src/models.ts); `TEAM_WAYS`, `CONTINUES_BUILD`, `CHAT_STEPS`, `BUILTIN_AGENTS` (src/agents.ts); `userHome`. +- Produces: `export const agents: Section` with `name: "agents"`, `title: "Agents"`, `covers: ["agents"]` (and `"agents.*"` paths: whatever `coveredBy` needs so a bad `agents` value can be fixed by this section). + +The questions, in order (wording is the implementer's except where quoted): +1. Unless `ctx.only`: `Set up agents for the loop's steps now?` (suggestion no). No → return. +2. `ctx.ask.say` one paragraph: what an agent, subagents, teams and skills are for a step, that picks reach the loop's sessions, that fix, reconcile and revise run as the build's agent, and that chat's two steps are steered by channel members. +3. `Where sessions get skills, agents, plugins and MCP servers` pick `["all", "repo"]` with one line each said first (`all`: the setup of whoever runs `loopstra start` plus the repo's; `repo`: only what the repository declares, the same on every machine). `doc.put(["agents","sources"], v, "all")`. +4. Say the library (`listLibrary(root, userHome(), sources)`): one line per item ` (): `, or "No agents or skills found yet." +5. Loop: `Which step to set up` pick from `["done", "all steps", "teams", ...STEP_KEYS]` (suggestion `done`). For a step (or all steps → path `agents.all`): + - agent (not for `fix`/`reconcile`/`revise`, and not offered built-ins): pick from `["none", "add one", ...agent names usable as a main agent]`, suggestion the current or `none`. + - subagents: `pickMany` over agent names including built-ins, plus a separate yes/no `Add an agent first?` that runs the add flow. + - teams: `pickMany` over team names (skip the question when there are none). + - skills: `pickMany` over skill names, with the same `Add a skill first?`. + - tools: `text` comma-separated, `-` empty, `check` = each rule through `toolProblem(rule, READ_ONLY_STEPS.has(step))` (for all steps: read-only true). + - MCP tools: `text` comma-separated, `-` empty, `check` = the mcp rule regex message. + - For chat steps, `say` first that anyone allowed to chat can steer these tools. + - Writes with `doc.put`/`doc.unset`; a step left with no settings is unset whole (`agents.steps.`); an empty `agents.steps`/`agents.all`/`agents` is not left behind as `{}` (unset it). +6. "teams": pick `["done", "new team", ...team names]`; a team: members `pickMany` (at least one), way pick `TEAM_WAYS`, or `delete` (refused while a step uses it: say which steps). +7. "add one" flow: pick `["from this machine", "from a git repository or folder", "a new agent"]` (skills: the first two only): + - this machine: pick from `listLibrary` items of that kind not from the repo → `copyItem`. + - git/folder: `text` URL or path → `cloneSource` → `scanFolder` → `pickMany` of items of that kind → `copyItem` each with `source: url`. + - new agent: name (`check`: the name rule and not taken), description, role text (one line), tools (comma-separated, `-` for the session's), model (`-` for the step's). + - Before adding, `say` the files that will be written; if a target already exists in the repo, `yesNo("Replace the existing ?", false)`; replacing a skill removes the old folder's files first (`content: null` writes for each existing file). + - Push to `ctx.files`; the new item is then listed in this run's picks (keep a run-local list of added names). + - `cleanup()` the clone in a `finally`. + +`check(ctx, cfg)`: `checkAgentSetup(cfg, { repo: ctx.root, home: userHome(), sources: cfg.agents.sources })` mapped to `Check { level: p.level, text: p.text }`; when there are no problems and agents are set, one `ok` line `Every agent the steps name is in place.`; when agents are not set, `[]`. + +- [ ] **Step 1: Write the failing tests** (with `askSection` and `scripted` answers; pickers take a number or the choice text, as the other section tests do) + +```ts +test("in a full setup the section asks first and changes nothing on no", async () => { + // configRepo(minimal); askSection(agents, root, ["n"]) → config text unchanged + // askSection(agents, root, "defaults") → unchanged +}); + +test("a step's agent, subagents and skills are written; none removes them; an emptied step is removed", async () => { + // repo .claude/agents/lead.md, .claude/skills/house/SKILL.md + // answers (only: true): sources all; step review; agent lead; subagents Explore; skills house; tools -; mcp -; step done + // → agents.steps.review = { agent: lead, subagents: [Explore], skills: [house] }; sources not written (default) + // second run: step review; agent none; subagents (none); skills (none); tools -; mcp -; done → no agents.steps.review in the file, no "steps: {}" +}); + +test("tools are refused in words on a read-only step", async () => { + // step review; ...; tools "Edit" → shown contains "review is read-only" and asks again; then "-" +}); + +test("adding a skill from this machine copies it into the repo after the config saves", async () => { + // home .claude/skills/tone/SKILL.md; answers: sources all; step build; agent none; subagents: (none) / add first? n; skills: add first? y → from this machine → tone → (shown lists .claude/skills/tone/SKILL.md, SOURCE.md); then pick tone; ...; done + // → files written: .claude/skills/tone/SKILL.md and SOURCE.md; agents.steps.build.skills = [tone] +}); + +test("a team is made and used, and cannot be deleted while used", async () => {}); + +test("check: missing agents fail, machine-only names warn, nothing set is silent", async () => {}); +``` + +- [ ] **Step 2: Run to see them fail.** +- [ ] **Step 3: Implement.** +- [ ] **Step 4: Run tests.** → PASS; `bun run check` (the full-setup tests in the setup test file still pass: their scripted answers now need one more `n` at the end for the agents question, or the setup under test passes `sections` explicitly; update those tests). +- [ ] **Step 5: Commit** `feat(setup): an agents section for each step's agent, subagents, teams, skills and tools`. + +--- + +### Task 4: `--check`, docs + +**Files:** +- Modify: `README.md` (the setup paragraph lists `agents`; one sentence on adding skills and agents from this machine, a git repository or folder, or a new agent; that setup never commits), `docs/decisions.md` (Part 2 bullets: files written only after save; copies are data, clone shallow into a temporary folder, `SOURCE.md` / `source:`; binary files refused for now; the section is last and asked for) +- Modify: `templates/skill/SKILL.md` Onboard step 2's section list includes `agents` +- Test: the setup `--check` test file + +- [ ] **Step 1: Write the failing test** + +```ts +test("setup --check lists a step's missing agent under the agents section and fails", async () => { + // config agents.steps.design.agent: ghost; setup(root, { check: true, output }) → 1; output has "FAIL `ghost`, the agent design runs as" and "To fix: ... loopstra setup agents" +}); +``` + +- [ ] **Step 2: Run to see it fail** (if Task 3's `check` already makes it pass, record that and keep the test). +- [ ] **Step 3: Edit the docs.** +- [ ] **Step 4:** `bun run check` → PASS. +- [ ] **Step 5: Commit** `docs: setup agents section`. From d20f64f950ba6e2ddfc25a19a5cb950351b32b70 Mon Sep 17 00:00:00 2001 From: John Flavan Date: Mon, 26 Oct 2026 23:32:35 -0500 Subject: [PATCH 18/64] docs: plan for agent setup, part 3 (console and operator skill) Co-Authored-By: Claude Opus 5.5 --- .../plans/2026-10-27-agent-setup-3-console.md | 200 ++++++++++++++++++ 1 file changed, 200 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-27-agent-setup-3-console.md diff --git a/docs/superpowers/plans/2026-10-27-agent-setup-3-console.md b/docs/superpowers/plans/2026-10-27-agent-setup-3-console.md new file mode 100644 index 00000000..5c62e686 --- /dev/null +++ b/docs/superpowers/plans/2026-10-27-agent-setup-3-console.md @@ -0,0 +1,200 @@ +# Agent Setup, Part 3: Console Agents Screen and Operator Skill Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** The console's Settings page gets an Agents section where a person sets each step's agent, subagents, teams, skills, tools and MCP tools, manages teams, and adds, edits or removes the repo's agents and skills; the loopstra operator skill gets an onboarding step that sets agents up with the engineer. + +**Architecture:** The console never writes the repository. Config changes reuse the `settings` request (extended to `agents.*` paths and list values). File changes are a new `agent-files` request: the console server turns an operation (copy from this machine, fetch from a git repository or folder, new agent, edit agent, remove) into file writes with `src/agent-library.ts` (Part 2), and the loop commits them through `commitRecord` under `_settings`, refusing if any file changed since the page read it. A read-only `/api/agents` view feeds the page. The page's logic lives in a pure `src/ui/agent-settings.ts` (tested like `src/ui/settings.ts`); the component is `src/ui/views/AgentSettings.tsx`, rendered inside the Settings view. + +**Tech Stack:** Bun, TypeScript, Preact (`preact/hooks`), the console's server and request plumbing (`src/console/*`, `src/requests.ts`, `src/apply.ts`, `src/record-draft.ts`), bun:test, `tests/console-helpers.ts`. + +**Spec:** `docs/superpowers/specs/2026-10-26-agent-setup-design.md` ("The console: an Agents settings screen", "The operator skill (onboarding)", "Observability"). Parts 1-2 are built on this branch. + +## Global Constraints + +- The console never writes the repo or `loopstra/config.yaml` itself: every change is a request the loop applies and commits (`loopstra(_settings): …`), as the models page does. +- Config writes: `agents.sources`, `agents.all.`, `agents.steps..`, `agents.teams..members`, `agents.teams..way`, and `agents.teams.` / `agents.steps.` set to null (remove). `` is one of `agent`, `subagents`, `teams`, `skills`, `tools`, `mcp_tools`. List fields take a list of strings (sent whole); `agent` and `way` and `sources` take a string; any field may be null (removed). After a write, empty `agents.steps.`, `agents.steps`, `agents.all`, `agents.teams` and `agents` maps are removed, not left as `{}`. +- Every write is checked twice, by the console before submitting and by the loop before committing: the config must still load (`validateConfig`), and the file's version must be the one the page read. +- File writes only under `.claude/agents/` and `.claude/skills/` (`repoPath`), text only, at most 200 files and 5 MB per request; each file's base version is the sha256 of its text (`configVersion`-style) or `"absent"`; any mismatch refuses the whole request with the stale wording. +- Removing an agent or skill a step or team uses is refused, naming the users (`agents.steps.review`, `team reviewers`). +- Commit messages: `loopstra(_settings): agents.steps.review.teams → [reviewers] by from the dashboard`; files: `loopstra(_settings): add .claude/agents/lead.md by from the dashboard` (`add`, `edit`, `remove`; several files: `add skill tone (3 files) by …`). +- Writes come only from the page (`jsonWrite`: POST, same Origin, JSON). Fetching a git source happens on the server, into a temporary folder removed after the answer; only `https://`, `ssh://`, `git@`, `file://` URLs or an existing folder (Part 2's `cloneSource`). +- The page keeps the Settings page's look (the tokens in `src/ui/theme.css`, `set-row`/`set-group` classes) and works at phone width. +- Chat steps show the warning that anyone allowed to chat can steer their tools, next to their tools and MCP fields. + +## Review Focus + +1. Two tabs: tab A adds a skill while tab B, holding the old version, edits a step's skills → B's write still lands (config version unchanged by a file-only write), and A's file write lands; a second file write from a tab that read the old file is refused as stale. (Tasks 1-2.) +2. A config edited by hand while the page is open (uncommitted) → every agents write is refused with the existing "commit loopstra/config.yaml first" wording, as model writes are. (Task 1.) +3. A git source URL that needs a password → the clone fails within its timeout (GIT_TERMINAL_PROMPT=0) and the page shows the reason; no temporary folder is left. (Task 3.) +4. Removing a step's last setting from the page leaves no `steps: {}` or `review: {}` in the file. (Task 1.) +5. A list value sent as a string, or with a non-string member, is refused with a 400 in words, before any request file is written. (Task 1.) + +--- + +## File structure + +- Modify `src/settings.ts` (paths, problems, list values, pruning), `src/requests.ts` (settings `value` union; new `agent-files` kind), `src/console/writes.ts` (parse both), `src/apply.ts` (apply `agent-files`; settings trace text for lists). +- Create `src/console/agents.ts` (the view and the operations → writes); modify `src/console/server.ts` (routes), `src/console/types.ts` (`AgentsView`, `AgentFilesOp`). +- Create `src/ui/agent-settings.ts` (pure helpers), `src/ui/views/AgentSettings.tsx`; modify `src/ui/views/Settings.tsx` (render it), `src/ui/requests.ts` (`sendAgentFiles`), `src/ui/theme.css` (only if a new class is needed). +- Modify `templates/skill/SKILL.md`, `README.md`, `docs/decisions.md`. +- Tests: `tests/unit/settings.test.ts` (or the file covering `src/settings.ts`), `tests/unit/apply*.test.ts` (whichever covers settings requests), `tests/unit/console-*.test.ts` (routes), `tests/unit/ui-logic.test.ts`. + +--- + +### Task 1: Agents settings through the `settings` request + +**Files:** `src/settings.ts`, `src/requests.ts`, `src/console/writes.ts`, `src/apply.ts`; tests for each. + +**Interfaces:** +- Produces: + - `type SettingValue = string | string[] | null` (export from `src/settings.ts`); the settings request's `value` is `z.union([z.string(), z.array(z.string()).max(64), z.null()])`. + - `settingPath(dotted)` accepts the agents paths in Global Constraints (step keys from `STEP_KEYS`; team names by the team-name rule from Part 1). + - `settingProblem(path, value: SettingValue)`: list fields need a list, scalar fields a string; `null` allowed for every agents path; model paths unchanged. + - `applySetting(file, text, path, value)`: lists written in flow style (`[a, b]`); after a set or unset, prune empty maps (Global Constraints); `doc.validate()` so config problems (a read-only step's `Edit`, a built-in main agent, an unknown team) come back as `problem` in words. + - The `settings` apply trace and toast text show a list as `[a, b]`. + +- [ ] **Step 1: Write the failing tests** + +```ts +test("agents paths are writable settings; others are not", () => { + expect(settingPath("agents.steps.review.teams")).toEqual(["agents", "steps", "review", "teams"]); + expect(settingPath("agents.all.mcp_tools")).toEqual(["agents", "all", "mcp_tools"]); + expect(settingPath("agents.teams.reviewers.way")).toEqual(["agents", "teams", "reviewers", "way"]); + expect(settingPath("agents.steps.review")).toEqual(["agents", "steps", "review"]); + expect(settingPath("agents.steps.nope.agent")).toBeNull(); + expect(settingPath("agents.steps.review.model")).toBeNull(); + expect(settingPath("agents.teams.Bad.way")).toBeNull(); +}); + +test("list fields take lists, scalars take strings", () => { + expect(settingProblem(["agents", "steps", "review", "skills"], "house")).toBe("agents.steps.review.skills takes a list of names."); + expect(settingProblem(["agents", "steps", "review", "agent"], ["a"])).toBe("agents.steps.review.agent takes one name."); + expect(settingProblem(["agents", "steps", "review"], null)).toBeNull(); +}); + +test("applySetting writes flow lists and prunes empty maps", () => { + const base = "version: 1\ncommands:\n test: bun test\n"; + const a = applySetting("c.yaml", base, ["agents", "steps", "review", "skills"], ["a", "b"]); + expect(a.ok).toBe(true); + expect(parse((a as { text: string }).text).agents.steps.review.skills).toEqual(["a", "b"]); // `parse` from "yaml" + expect((a as { text: string }).text).toMatch(/skills: \[.*a.*b.*\]/); // flow style, one line + const b = applySetting("c.yaml", (a as { text: string }).text, ["agents", "steps", "review", "skills"], null); + expect(b.ok && b.text).toBe(base); + const c = applySetting("c.yaml", base, ["agents", "steps", "review", "tools"], ["Edit"]); + expect(c).toEqual({ ok: false, problem: expect.stringContaining("review is read-only") }); +}); + +test("the console refuses a malformed agents value before writing a request", async () => { + // POST /api/requests { kind: "settings", path: "agents.steps.review.skills", value: "house", version } → 400 "agents.steps.review.skills takes a list of names." + // value: ["a", 1] → 400; no file under .loopstra/requests +}); + +test("the loop commits an agents setting under _settings", async () => { + // submit { kind: settings, path: agents.steps.review.teams, value: ["reviewers"] } with agents.teams.reviewers defined → commit subject "loopstra(_settings): agents.steps.review.teams → [reviewers] by …" + // with an uncommitted config edit → refused with UNCOMMITTED_CONFIG +}); +``` + +- [ ] **Step 2: Run to see them fail.** +- [ ] **Step 3: Implement.** Pruning lives in `applySetting` (walk from the written path up while the map is empty and is under `agents`). +- [ ] **Step 4: Run tests**; `bun run check`. +- [ ] **Step 5: Commit** `feat(console): agents settings through settings requests`. + +--- + +### Task 2: The `agent-files` request + +**Files:** `src/requests.ts`, `src/console/writes.ts` (submit path from Task 3's route, not from `/api/requests`), `src/apply.ts`, `src/record-draft.ts` only if a records' `paths` need anything new; tests. + +**Interfaces:** +- Consumes: `FileWrite`, `repoPath` (Part 2). +- Produces: + - Request kind `agent-files`: `{ kind: "agent-files", what: string (≤ 200 chars, the commit's words: "add .claude/agents/lead.md"), writes: Array<{ path: string; content: string | null }> (1..200), base: Record (path → sha256 hex of the text the page/server read, or "absent") }`. + - `export function fileVersion(text: string | null): string` in `src/settings.ts`: `"absent"` for null, else the same hash as `configVersion`. + - `applyAgentFiles(root, cfg, trace, req)`: every path through `repoPath` (else refused); total content ≤ 5 MB; `commitRecord(root, main, SETTINGS_SLUG, \`${req.what} by ${byWhom(req)}\`, draft => { for each path: fileVersion(draft.read(path)) must equal base[path] (else stale); draft.write(path, content) }, { paths: writes.map(w => w.path), … })`. Outcome texts: `Saved: .` / `That change was not saved: ` / `STALE_SETTINGS`. + - A config-file write is never part of `agent-files` (refused if `loopstra/config.yaml` appears). + +- [ ] **Step 1: Write the failing tests** + +```ts +test("agent-files writes and removes files under .claude and commits them", async () => { + // repo on main with .claude/skills/old/SKILL.md; request writes .claude/agents/lead.md (base absent) and removes old/SKILL.md (base = its hash) + // → one commit "loopstra(_settings): add .claude/agents/lead.md by …" with both changes; the working tree matches +}); + +test("agent-files refuses paths outside .claude/agents and .claude/skills, and stale bases", async () => { + // writes ".claude/settings.json" → refused, nothing committed + // base for lead.md says absent but lead.md exists on main → STALE_SETTINGS, nothing committed +}); +``` + +- [ ] **Step 2-5:** fail → implement → pass (+ `bun run check`) → commit `feat(console): agent-files requests the loop commits`. + +--- + +### Task 3: The agents view and operations on the server + +**Files:** create `src/console/agents.ts`; modify `src/console/server.ts`, `src/console/types.ts`; test `tests/unit/console-agents.test.ts`. + +**Interfaces:** +- Consumes: `resolveSetup`, `READ_ONLY_STEPS`, `CHAT_STEPS`, `CONTINUES_BUILD`, `BUILTIN_AGENTS`, `TEAM_WAYS` (src/agents.ts); `checkAgentSetup`, `parseAgentFile`, `userHome` (src/agent-files.ts); `listLibrary`, `scanFolder`, `cloneSource`, `copyItem`, `newAgentFile` (src/agent-library.ts); `configVersion`, `fileVersion` (src/settings.ts); `STEPS` (src/models.ts). +- Produces (in `src/console/types.ts`; the page imports only types): + - `AgentsView { version: string | null; problem: string | null; sources: "all" | "repo"; steps: AgentStepRow[]; teams: Array<{ name: string; members: string[]; way: string; usedBy: string[] }>; library: Array; problems: Array<{ level: "fail" | "warn"; step: string; name: string; text: string }>; builtins: string[]; ways: string[] }` + - `AgentStepRow { key: string; what: string; readOnly: boolean; chat: boolean; continuesBuild: boolean; own: Partial>; fromAll: Partial>; fromStage: string[] /* stage skills */ }` plus one row with `key: "all"`. + - `LibraryItemView { kind: "agent" | "skill"; name: string; description: string; from: "repo" | "personal" | "plugin"; usedBy: string[]; base: string /* fileVersion of the agent file, or of SKILL.md */; editable: boolean /* repo agents */ ; text?: string /* a repo agent's file, for the edit form */ }` + - `AgentFilesOp = { op: "copy"; kind; name; from } | { op: "fetch"; url: string; names: string[]; kind } | { op: "new"; name; description; prompt; tools: string[]; model: string | null } | { op: "edit"; name; text: string; base: string } | { op: "remove"; kind; name }` + - Routes: `GET /api/agents` → `AgentsView`; `GET /api/agents/source?url=…` → `{ items: Array<{ kind; name; description }> }` or `{ error }` (clone, scan, cleanup); `POST /api/agents/files` (via `jsonWrite`) with an `AgentFilesOp` → builds `writes` and `base` from the working tree at `root` (the files' current text) and submits an `agent-files` request (202 `{ id, loop }`), or refuses (409/400) in words: a name in use (remove), a copy refused by `copyItem`, a target that exists for `new`/`copy`/`fetch` unless the op says `replace: true`. + +- [ ] **Step 1: Write the failing tests** (server-level, like the existing console route tests: start the console app on a temp repo, `fetch` with the page's Origin) + +```ts +test("GET /api/agents shows each step's own, inherited and stage values, the library and problems", async () => {}); +test("POST /api/agents/files copy submits an agent-files request with the skill's files and absent bases", async () => {}); +test("POST /api/agents/files remove is refused while a step uses the agent", async () => {}); +test("GET /api/agents/source lists what a file:// repository holds and leaves no temp folder", async () => {}); +test("a write without the page's Origin is refused (403)", async () => {}); +``` + +Fill each with the assertions their names state; the existing console route tests show the server setup. + +- [ ] **Step 2-5:** fail → implement → pass (+ `bun run check`) → commit `feat(console): agents view and file operations`. + +--- + +### Task 4: The Agents section on the Settings page + +**Files:** create `src/ui/agent-settings.ts`, `src/ui/views/AgentSettings.tsx`; modify `src/ui/views/Settings.tsx`, `src/ui/requests.ts`; test `tests/unit/ui-logic.test.ts`. + +**Interfaces:** +- Consumes: `AgentsView`, `AgentFilesOp` (Task 3); `sendSetting`/`cancelSetting` (src/ui/requests.ts), extended so `value` may be a list. +- Produces: + - `src/ui/agent-settings.ts` (pure): `effective(row: AgentStepRow, field: Field): { value: string | string[] | null; from: "own" | "all" | "stage" | "build" | null }` (fix/reconcile/revise agent shows `from: "build"`); `choicesFor(view, field, step): string[]` (agents for `agent` exclude built-ins; subagents include them; teams by name; skills from the library); `listText(v: string[]): string` (`"a, b"`, `"none"`); `parseList(text: string): string[]` (split on commas, trim, drop empties, keep order, each once). + - `sendAgentFiles(op: AgentFilesOp, onDone)` in `src/ui/requests.ts`, same shape as `sendSetting`. + - `AgentSettings` component: under the models, an "Agents" heading; a problems callout (fails first) when `problems` is not empty; a Sources picker; a table of rows (All steps first, then the steps in `STEPS` order): each field shows its effective value with a muted "from all steps" / "stage skill" / "as build" note, and an edit control (a `