diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 97971b5..2557a37 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,14 +1,21 @@ name: Release -# Publishes loopable-cli to npm: the version main's package.json says, once -# that commit is live. The version is bumped in a pull request like any other +# Publishes loopable-cli to npm: the version package.json says, once that +# commit is live. The version is bumped in a pull request like any other # change (pnpm bump minor), so this never pushes to main; it adds the tag. -# Run by hand from the Actions tab. +# +# Runs by itself after Deploy finishes a merge to main. A commit whose version +# is already on npm is a service-only change, and is left alone. Run it by hand +# from the Actions tab to publish main again after a failed release. # # npm trusts this workflow directly (trusted publishing), so there is no npm # token anywhere. Set up once on npmjs.com: loopable-cli → Settings → Trusted # publishing → GitHub Actions, repository imsobear/loopable, workflow release.yml. +# Keep the file name: npm trusts this file, not whatever runs under its name. on: + workflow_run: + workflows: [Deploy] + types: [completed] workflow_dispatch: concurrency: @@ -21,34 +28,50 @@ permissions: id-token: write jobs: - release: + check: + # After Deploy only when it deployed a merge. A Deploy run by hand may be a + # rollback to an older commit, and that is not a new version. + if: >- + github.event_name == 'workflow_dispatch' || + (github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.event == 'workflow_run') runs-on: ubuntu-latest - timeout-minutes: 20 + timeout-minutes: 5 + outputs: + sha: ${{ steps.commit.outputs.sha }} + tag: ${{ steps.version.outputs.tag }} steps: - uses: actions/checkout@v4 with: - ref: main + ref: ${{ github.event.workflow_run.head_sha || 'main' }} fetch-depth: 0 - - name: Check main is what is deployed + # After Deploy, the run that triggered this is the proof. By hand, main + # has to be what the last deploy put out. + - name: Check the commit is what is deployed + id: commit env: GH_TOKEN: ${{ github.token }} run: | head="$(git rev-parse HEAD)" live="$(gh run list --workflow deploy.yml --status success --limit 1 --json headSha -q '.[0].headSha')" - if [ "$head" != "$live" ]; then - echo "main is at $head but the last successful deploy was $live." >&2 + if [ "${{ github.event_name }}" = workflow_dispatch ] && [ "$head" != "$live" ]; then + echo "This is $head but the last successful deploy was $live." >&2 echo "Wait for the deploy of main to finish, or run Deploy, then release." >&2 exit 1 fi + echo "sha=$head" >> "$GITHUB_OUTPUT" - name: Check the version is new id: version run: | version="$(node -p 'require("./package.json").version')" if npm view "loopable-cli@$version" version >/dev/null 2>&1; then - echo "loopable-cli@$version is already on npm. Bump it in a pull request first: pnpm bump minor" >&2 - exit 1 + if [ "${{ github.event_name }}" = workflow_dispatch ]; then + echo "loopable-cli@$version is already on npm. Bump it in a pull request first: pnpm bump minor" >&2 + exit 1 + fi + echo "loopable-cli@$version is already on npm, so there is nothing to release." + exit 0 fi if git rev-parse "v$version" >/dev/null 2>&1; then echo "The tag v$version already exists." >&2 @@ -56,6 +79,16 @@ jobs: fi echo "tag=v$version" >> "$GITHUB_OUTPUT" + release: + needs: check + if: needs.check.outputs.tag != '' + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ needs.check.outputs.sha }} + - uses: pnpm/action-setup@v4 - uses: actions/setup-node@v4 with: @@ -78,10 +111,10 @@ jobs: run: | git config user.name "github-actions[bot]" git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git tag -a "${{ steps.version.outputs.tag }}" -m "Release ${{ steps.version.outputs.tag }}." - git push origin "${{ steps.version.outputs.tag }}" + git tag -a "${{ needs.check.outputs.tag }}" -m "Release ${{ needs.check.outputs.tag }}." + git push origin "${{ needs.check.outputs.tag }}" - name: Release notes env: GH_TOKEN: ${{ github.token }} - run: gh release create "${{ steps.version.outputs.tag }}" --generate-notes + run: gh release create "${{ needs.check.outputs.tag }}" --generate-notes diff --git a/docs/concepts.md b/docs/concepts.md index 54560e2..d631a59 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -46,6 +46,8 @@ The first look never acts. Whatever is already waiting when a loop is created is **Folder.** Loops that read your code (chat bots and schedules) work in a folder, given relative to the runner's home, such as `code/web`. Each runner looks for it under its own home, so any runner with that checkout can take the job. +**MCP servers.** The MCP servers a loop's agent may call, named as the runner's agent knows them, such as `sentry`. An agent run by a loop has nobody to ask, so a call to any server the loop does not name is refused, in either permission mode. The server itself, and its login, live on the runner like everything else the agent uses. + ## What does the work **Agent.** A coding CLI Loopable knows how to invoke: Codex, Claude Code, Cursor Agent. The list is a catalog in the repo, not discovered from the network. Loopable stores only the choices a person makes: which agent is the default, permission mode, model, timeout. Whether one is installed lives on each runner’s inventory. diff --git a/docs/release.md b/docs/release.md index c583f7e..74d2229 100644 --- a/docs/release.md +++ b/docs/release.md @@ -4,7 +4,7 @@ How a change gets from a branch to loop.goodone.si and to npm. The workflows are ```text branch → pull request → CI → merge to main → CI → Deploy (migrate, then deploy) - → Release (by hand, CLI only, after pnpm bump in a pull request) + → Release (CLI to npm, only when pnpm bump changed the version) ``` ## Changing code @@ -33,9 +33,12 @@ The GitHub **production** environment lists every deploy and its commit, so what Release a new CLI when the runner or the `loopable` command changed. A service-only change needs no release. -1. On a branch, `pnpm bump minor` (or `patch`, `major`, an exact version). It commits the new version. -2. Merge that pull request, and wait for its deploy to finish. -3. Run **Release** from the Actions tab. It refuses unless `main` is the commit that is deployed, so the service always knows about a new runner before any runner is out. It checks, builds, publishes the version in `package.json` to npm through trusted publishing, tags `vX.Y.Z`, and writes the release notes. It never pushes to `main`. +1. In the pull request, `pnpm bump minor` (or `patch`, `major`, an exact version). It commits the new version. +2. Merge it. Once its deploy has finished, **Release** runs by itself: it checks, builds, publishes the version in `package.json` to npm through trusted publishing, tags `vX.Y.Z`, and writes the release notes. It never pushes to `main`. + +Release waits for the deploy, so the service always knows about a new runner before any runner is out. A merge that did not change the version publishes nothing. A Deploy run by hand, which may be a rollback, never releases. + +If a release fails, fix it in a pull request, or run **Release** from the Actions tab once the cause is gone. By hand it refuses unless `main` is the commit that is deployed and its version is not on npm yet. Runners on 0.7.0 or later can then be updated from the Runners page. diff --git a/drizzle/0035_loop_mcp_servers.sql b/drizzle/0035_loop_mcp_servers.sql new file mode 100644 index 0000000..48b1369 --- /dev/null +++ b/drizzle/0035_loop_mcp_servers.sql @@ -0,0 +1,2 @@ +-- MCP servers a loop's agent may call without asking. +ALTER TABLE `loops` ADD `mcp_servers` text DEFAULT '[]' NOT NULL; diff --git a/drizzle/meta/_journal.json b/drizzle/meta/_journal.json index 1f5efbb..049cf26 100644 --- a/drizzle/meta/_journal.json +++ b/drizzle/meta/_journal.json @@ -246,6 +246,13 @@ "when": 1789100000019, "tag": "0034_runner_controls", "breakpoints": true + }, + { + "idx": 35, + "version": "6", + "when": 1789100000020, + "tag": "0035_loop_mcp_servers", + "breakpoints": true } ] } diff --git a/package.json b/package.json index fb61ef4..36ca85b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "loopable-cli", - "version": "0.7.0", + "version": "0.8.0", "description": "Connect your machine to Loopable: a runner that does your workspace's jobs with your own coding agent.", "license": "MIT", "type": "module", @@ -24,7 +24,14 @@ "url": "https://github.com/imsobear/loopable/issues" }, "homepage": "https://loop.goodone.si", - "keywords": ["coding-agent", "codex", "claude-code", "agent-workflows", "runner", "loopable"], + "keywords": [ + "coding-agent", + "codex", + "claude-code", + "agent-workflows", + "runner", + "loopable" + ], "publishConfig": { "access": "public" }, diff --git a/scripts/bump.mjs b/scripts/bump.mjs index ee04197..9465179 100644 --- a/scripts/bump.mjs +++ b/scripts/bump.mjs @@ -1,5 +1,6 @@ // Bumps loopable-cli's version on the current branch, as its own commit, for -// the pull request that releases it. Publishing is the Release workflow's. +// the pull request that releases it. Publishing is the Release workflow's, +// which runs by itself once that pull request is merged and deployed. // // pnpm bump patch|minor|major| import { execFileSync } from "node:child_process"; @@ -37,4 +38,4 @@ const version = next(pkg.version); pkg.version = version; writeFileSync(path, `${JSON.stringify(pkg, null, 2)}\n`); git("commit", "--quiet", "-m", `Bump loopable-cli to v${version}.`, "--", "package.json"); -console.log(`✓ v${version}, committed. Merge the pull request, wait for the deploy, then run Release.`); +console.log(`✓ v${version}, committed. Merge the pull request; it is published once it is deployed.`); diff --git a/src/agents/claude/runtime.ts b/src/agents/claude/runtime.ts index 062aa65..f2102a0 100644 --- a/src/agents/claude/runtime.ts +++ b/src/agents/claude/runtime.ts @@ -32,6 +32,9 @@ function args(input: AgentRunInput): string[] { // which is how the container runs. list.push("--permission-mode", "acceptEdits", "--allowedTools", "Bash"); } + // Neither mode lets an MCP tool through on its own. A rule naming just the + // server allows every tool it has. + for (const server of input.mcpServers ?? []) list.push("--allowedTools", `mcp__${server}`); if (input.settings.model) list.push("--model", input.settings.model); // The prompt follows a variadic option, so it has to be marked as the // positional argument or --allowedTools would swallow it. diff --git a/src/agents/codex/runtime.ts b/src/agents/codex/runtime.ts index 982137f..54fa227 100644 --- a/src/agents/codex/runtime.ts +++ b/src/agents/codex/runtime.ts @@ -1,4 +1,6 @@ import { readFileSync, rmSync } from "node:fs"; +import { homedir } from "node:os"; +import { join } from "node:path"; import { defineAgentRuntime } from "../define.ts"; import { firstLine, probe, resolveBinary } from "../discover.ts"; import { runProcess } from "../process.ts"; @@ -11,7 +13,25 @@ async function binary(): Promise { return found; } -function args(input: AgentRunInput): string[] { +/** + * The allowed servers this host's codex actually has. Naming one it does not + * have is worse than leaving it out: codex reads the override as half a + * server and refuses to start at all. + */ +function configuredServers(names: string[]): string[] { + if (names.length === 0) return []; + let config = ""; + try { + config = readFileSync(join(process.env.CODEX_HOME || join(homedir(), ".codex"), "config.toml"), "utf8"); + } catch { + return []; + } + return names.filter((name) => + new RegExp(`^\\s*\\[\\s*mcp_servers\\.("?)${name}\\1\\s*[.\\]]`, "m").test(config), + ); +} + +function args(input: AgentRunInput, mcpServers = input.mcpServers ?? []): string[] { const list = [ "exec", "--sandbox", @@ -26,6 +46,11 @@ function args(input: AgentRunInput): string[] { input.cwd, ]; if (input.settings.model) list.push("-m", input.settings.model); + // exec never asks, so an MCP call that needs approval is refused. These + // servers' tools are approved up front instead. + for (const server of mcpServers) { + list.push("-c", `mcp_servers.${server}.default_tools_approval_mode="approve"`); + } if (input.outputFile) list.push("-o", input.outputFile); list.push(input.prompt); return list; @@ -55,7 +80,7 @@ export const codexRuntime = defineAgentRuntime({ async run(input) { const bin = await binary(); - const argv = args(input); + const argv = args(input, configuredServers(input.mcpServers ?? [])); const command = `${bin} ${argv.join(" ")}`; const outcome = await runProcess({ bin, diff --git a/src/agents/cursor-agent/runtime.ts b/src/agents/cursor-agent/runtime.ts index ffa7c41..385587f 100644 --- a/src/agents/cursor-agent/runtime.ts +++ b/src/agents/cursor-agent/runtime.ts @@ -1,3 +1,6 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { defineAgentRuntime } from "../define.ts"; import { firstLine, probe, resolveBinary } from "../discover.ts"; import { runProcess } from "../process.ts"; @@ -11,6 +14,22 @@ async function binary(): Promise { return found; } +/** + * Where to start the agent from, when a read-only run has MCP servers to + * allow. There is no flag for that, but the agent reads permissions from a + * .cursor/cli.json between the git root and where it was started, on top of + * the person's own. Started from a directory of its own, the rule lasts one run + * and never touches the checkout. --force already allows everything else. + */ +function permissionsDir(input: AgentRunInput): string | null { + if (input.settings.permissionMode !== "read_only" || !input.mcpServers?.length) return null; + const dir = mkdtempSync(join(tmpdir(), "loopable-cursor-")); + mkdirSync(join(dir, ".cursor")); + const allow = input.mcpServers.map((server) => `Mcp(${server}:*)`); + writeFileSync(join(dir, ".cursor", "cli.json"), JSON.stringify({ permissions: { allow, deny: [] } })); + return dir; +} + function args(input: AgentRunInput): string[] { const list = [ "-p", @@ -32,6 +51,8 @@ function args(input: AgentRunInput): string[] { } else { list.push("--force"); } + // Otherwise a server nobody has approved on this machine is not started. + if (input.mcpServers?.length) list.push("--approve-mcps"); if (input.settings.model) list.push("--model", input.settings.model); list.push(input.prompt); return list; @@ -63,14 +84,18 @@ export const cursorAgentRuntime = defineAgentRuntime({ async run(input) { const bin = await binary(); const argv = args(input); + const permissions = permissionsDir(input); const outcome = await runProcess({ bin, args: argv, - cwd: input.cwd, + // --workspace says where to work, so starting elsewhere changes nothing else. + cwd: permissions ?? input.cwd, timeoutMs: input.settings.timeoutMs, signal: input.signal, logPath: input.logFile, logLine: (line) => describeEvent(line, input.cwd), + }).finally(() => { + if (permissions) rmSync(permissions, { recursive: true, force: true }); }); const command = `${bin} ${argv.join(" ")}`; const answer = answerFrom(outcome.stdout); diff --git a/src/agents/runtimes.test.ts b/src/agents/runtimes.test.ts new file mode 100644 index 0000000..735e5d9 --- /dev/null +++ b/src/agents/runtimes.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from "vitest"; +import { agentRuntime } from "./runtimes.ts"; +import type { AgentRunInput, PermissionMode } from "./types.ts"; + +function input(permissionMode: PermissionMode, mcpServers?: string[]): AgentRunInput { + return { + prompt: "Check it.", + cwd: "/tmp/work", + settings: { permissionMode, model: null, timeoutMs: 60_000 }, + ...(mcpServers ? { mcpServers } : {}), + }; +} + +const argsOf = (agent: string, given: AgentRunInput) => agentRuntime(agent).invocation(given).args; + +describe("allowed MCP servers", () => { + it("reach claude as rules naming each server, in either mode", () => { + for (const mode of ["read_only", "workspace_write"] as const) { + const args = argsOf("claude", input(mode, ["sentry", "sub2tok-admin"])); + expect(args.join(" ")).toContain("--allowedTools mcp__sentry --allowedTools mcp__sub2tok-admin"); + // Still the last thing, so no variadic option swallows the prompt. + expect(args.slice(-2)).toEqual(["--", "Check it."]); + } + }); + + it("reach codex as servers whose tools are approved up front", () => { + expect(argsOf("codex", input("read_only", ["sentry"]))).toContain( + 'mcp_servers.sentry.default_tools_approval_mode="approve"', + ); + }); + + it("have cursor start the servers nobody approved on this machine", () => { + expect(argsOf("cursor-agent", input("read_only", ["sentry"]))).toContain("--approve-mcps"); + }); + + it("change nothing when a loop allows none", () => { + expect(argsOf("claude", input("read_only")).join(" ")).not.toContain("mcp__"); + expect(argsOf("codex", input("read_only")).join(" ")).not.toContain("mcp_servers"); + expect(argsOf("cursor-agent", input("read_only"))).not.toContain("--approve-mcps"); + }); +}); diff --git a/src/agents/types.ts b/src/agents/types.ts index 79fc486..94a54c2 100644 --- a/src/agents/types.ts +++ b/src/agents/types.ts @@ -60,6 +60,11 @@ export type AgentRunInput = { signal?: AbortSignal; /** Where to write everything the agent prints, while it prints it. */ logFile?: string; + /** + * MCP servers the agent may call without asking, in either permission mode. + * Print mode has nobody to ask, so every other server's tools are refused. + */ + mcpServers?: string[]; }; export type AgentInvocation = { bin: string; args: string[] }; diff --git a/src/components/loop-form.tsx b/src/components/loop-form.tsx index fe514a6..8b4b475 100644 --- a/src/components/loop-form.tsx +++ b/src/components/loop-form.tsx @@ -104,6 +104,7 @@ export function LoopForm({ loop, trigger }: { loop: LoopEdit; trigger?: React.Re return saved.startsWith("~/") ? saved.slice(2) : saved; }); const [agentId, setAgentId] = useState(loop.agentId ?? DEFAULT_AGENT); + const [mcpServers, setMcpServers] = useState(loop.mcpServers.join(", ")); const [settings, setSettings] = useState(() => initialFieldValues(workflow?.settings ?? [], loop.settings), ); @@ -153,6 +154,7 @@ export function LoopForm({ loop, trigger }: { loop: LoopEdit; trigger?: React.Re prompt, guidance: null, agentId: agentId === DEFAULT_AGENT ? null : agentId, + mcpServers: mcpServers.split(/[\s,]+/).filter(Boolean), settings: workflow.runsIn === "folder" ? { ...settings, folder } : settings, actionConnectorId, actionId, @@ -238,6 +240,21 @@ export function LoopForm({ loop, trigger }: { loop: LoopEdit; trigger?: React.Re + + setMcpServers(event.target.value)} + className="font-mono" + /> + + {workflow.runsIn === "folder" ? ( = {}): LoopView { prompt: "Review it.", guidance: null, agentId: null, + mcpServers: [], actionConnectorId: "github", actionId: "github.submit_review", actionTarget: {}, diff --git a/src/lib/loop-draft.ts b/src/lib/loop-draft.ts index 0a6a297..cee7d6c 100644 --- a/src/lib/loop-draft.ts +++ b/src/lib/loop-draft.ts @@ -25,6 +25,7 @@ export function draftForWorkflow(connectorId: string, workflowId: string): LoopE prompt: workflow.prompt, guidance: null, agentId: null, + mcpServers: [], settings: {}, // Where it reads, unless the workflow says otherwise. A connector that // only reads has no action of its own to fall back to. diff --git a/src/lib/version.ts b/src/lib/version.ts index 5f21aca..12821bd 100644 --- a/src/lib/version.ts +++ b/src/lib/version.ts @@ -15,8 +15,9 @@ export const VERSION: string = pkg.version; * 1. Up to 0.2.x, which did not report a protocol at all. * 2. A folder is relative to the runner's home, and the prompt names context * files by a placeholder the runner fills in with its own workspace. + * 3. A job can name MCP servers the agent may call without asking. */ -export const RUNNER_PROTOCOL = 2; +export const RUNNER_PROTOCOL = 3; /** What a runner that says nothing is assumed to speak. */ export const LEGACY_RUNNER_PROTOCOL = 1; @@ -24,6 +25,9 @@ export const LEGACY_RUNNER_PROTOCOL = 1; /** The lowest protocol a folder job can go to. */ export const FOLDER_JOB_PROTOCOL = 2; +/** The lowest protocol a job that allows MCP servers can go to. */ +export const MCP_JOB_PROTOCOL = 3; + /** Whether `a` is an older x.y.z than `b`. Anything unreadable counts as older. */ export function olderThan(a: string | null, b: string): boolean { if (!a) return true; diff --git a/src/server/agent-job.test.ts b/src/server/agent-job.test.ts index 6e2b795..fc7069b 100644 --- a/src/server/agent-job.test.ts +++ b/src/server/agent-job.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { filesFromWorkItem, isAgentNothing, + jobMinProtocol, jobRequiresHost, normalizeFolder, resolveFolder, @@ -15,6 +16,15 @@ describe("jobRequiresHost", () => { }); }); +describe("jobMinProtocol", () => { + it("asks for a runner that passes allowed MCP servers on", () => { + expect(jobMinProtocol({})).toBe(0); + expect(jobMinProtocol({ cwd: "code/web" })).toBe(2); + expect(jobMinProtocol({ mcpServers: [] })).toBe(0); + expect(jobMinProtocol({ cwd: "code/web", mcpServers: ["sentry"] })).toBe(3); + }); +}); + describe("folders", () => { it("are kept relative to home, with or without ~/", () => { expect(normalizeFolder(" ~/code/web ")).toBe("code/web"); diff --git a/src/server/agent-job.ts b/src/server/agent-job.ts index 8227abf..3203699 100644 --- a/src/server/agent-job.ts +++ b/src/server/agent-job.ts @@ -1,5 +1,5 @@ import { isAbsolute, join } from "node:path"; -import { FOLDER_JOB_PROTOCOL } from "#/lib/version.ts"; +import { FOLDER_JOB_PROTOCOL, MCP_JOB_PROTOCOL } from "#/lib/version.ts"; import type { AgentSettingsValues } from "#/agents/types.ts"; export type AgentJobFile = { name: string; body: string }; @@ -16,6 +16,8 @@ export type AgentJob = { * runner works in a throwaway directory that holds the files. */ cwd?: string; + /** MCP servers the agent may call without asking. Absent means none. */ + mcpServers?: string[]; }; export type AgentJobResult = { @@ -74,9 +76,12 @@ export function jobRequiresHost(job: Pick): boolean { /** * The oldest runner protocol that can do this job. A relative folder and the * workspace placeholder are both protocol 2; an older runner would run the - * agent in the wrong place and hand it a path that does not exist. + * agent in the wrong place and hand it a path that does not exist. Allowed MCP + * servers are protocol 3; an older runner would drop them, and the agent would + * be refused every call the loop was set up to make. */ -export function jobMinProtocol(job: Pick): number { +export function jobMinProtocol(job: Pick): number { + if (job.mcpServers?.length) return MCP_JOB_PROTOCOL; return job.cwd && !folderIsLegacy(job.cwd) ? FOLDER_JOB_PROTOCOL : 0; } diff --git a/src/server/agent-runner.ts b/src/server/agent-runner.ts index 6609e44..b8e7800 100644 --- a/src/server/agent-runner.ts +++ b/src/server/agent-runner.ts @@ -15,6 +15,7 @@ export type RunAgentJobOptions = { outputFile: string; logFile: string; signal?: AbortSignal; + mcpServers?: string[]; }) => Promise; signal?: AbortSignal; /** Joined runners work in a throwaway directory; the host uses the run dir. */ @@ -58,6 +59,7 @@ export async function runAgentJob( outputFile, logFile, signal: options.signal, + ...(job.mcpServers?.length ? { mcpServers: job.mcpServers } : {}), }); return { ok: result.ok, diff --git a/src/server/db/schema.ts b/src/server/db/schema.ts index 86328d3..e0f79d3 100644 --- a/src/server/db/schema.ts +++ b/src/server/db/schema.ts @@ -196,6 +196,12 @@ export const loops = sqliteTable( prompt: text("prompt").notNull(), /** Added after the prompt, for a note that is not worth editing it over. */ guidance: text("guidance"), + /** + * MCP servers the agent may call without asking, by the name the runner's + * agent knows them by. Empty is the norm: an agent in print mode has nobody + * to ask, so a server that is not named here is refused. + */ + mcpServers: text("mcp_servers", { mode: "json" }).$type().notNull().default([]), /** Null means whichever agent is currently the default. */ agentId: text("agent_id"), /** diff --git a/src/server/loops.test.ts b/src/server/loops.test.ts index 2b6050a..69753fd 100644 --- a/src/server/loops.test.ts +++ b/src/server/loops.test.ts @@ -32,6 +32,7 @@ async function edited(id: string, changes: Record) { prompt: loop.prompt, guidance: loop.guidance, agentId: loop.agentId, + mcpServers: loop.mcpServers, settings: loop.settings, actionConnectorId: loop.actionConnectorId, actionId: loop.actionId, @@ -163,6 +164,30 @@ describe("a loop that has to work somewhere", () => { }); }); +describe("MCP servers a loop allows", () => { + it("are none unless somebody names one", async () => { + expect((await added("github", "github.review_requested")).mcpServers).toEqual([]); + }); + + it("are kept trimmed and once each", async () => { + const loop = await added("github", "github.review_requested", { + mcpServers: [" sentry ", "sub2tok-admin", "sentry", ""], + }); + + expect(loop.mcpServers).toEqual(["sentry", "sub2tok-admin"]); + expect((await edited(loop.id, { mcpServers: [] })).mcpServers).toEqual([]); + }); + + it("refuse a name that would mean something else to the agent", async () => { + // Each agent puts the name into a rule or a config key of its own. + for (const name of ["a:b", "x)", 'say"hi', "two words"]) { + await expect(added("github", "github.review_requested", { mcpServers: [name] })).rejects.toThrow( + /not an MCP server name/, + ); + } + }); +}); + describe("choosing where a loop writes", () => { it("accepts an action on a connector it does not watch", async () => { const { id } = await added("github", "github.review_requested"); diff --git a/src/server/loops.ts b/src/server/loops.ts index 81518ce..24b9cc1 100644 --- a/src/server/loops.ts +++ b/src/server/loops.ts @@ -25,6 +25,30 @@ function byId(id: string) { const NAME_LIMIT = 80; const PROMPT_LIMIT = 8000; const GUIDANCE_LIMIT = 4000; +const MCP_SERVER_LIMIT = 20; +/** + * What all three agents accept as a server name. Each puts the name into + * something of its own (a permission rule, a tool name, a config key), so + * anything else could change what that something means. + */ +const MCP_SERVER_NAME = /^[A-Za-z0-9_-]{1,64}$/; + +/** Trimmed, unique, and only names an agent can be told about. */ +function cleanMcpServers(given: unknown): string[] { + const list = Array.isArray(given) ? given.filter((name) => typeof name === "string") : []; + const names = [...new Set(list.map((name) => name.trim()).filter(Boolean))]; + for (const name of names) { + if (!MCP_SERVER_NAME.test(name)) { + throw new Error( + `"${name}" is not an MCP server name. Use letters, numbers, - and _ as the agent shows it.`, + ); + } + } + if (names.length > MCP_SERVER_LIMIT) { + throw new Error(`Allow at most ${MCP_SERVER_LIMIT} MCP servers.`); + } + return names; +} function toView(row: Loop): LoopView { return { @@ -38,6 +62,7 @@ function toView(row: Loop): LoopView { prompt: row.prompt, guidance: row.guidance, agentId: row.agentId, + mcpServers: row.mcpServers, actionConnectorId: row.actionConnectorId, actionId: row.actionId, actionTarget: row.actionTarget, @@ -136,6 +161,7 @@ function validate(draft: LoopDraft): LoopDraft { name, prompt, guidance, + mcpServers: cleanMcpServers(draft.mcpServers), settings: { ...cleanConditions(workflow.settings, draft.settings), ...(folder ? { folder } : {}), @@ -179,6 +205,7 @@ export async function createLoop(draft: LoopDraft): Promise { prompt: checked.prompt, guidance: checked.guidance, agentId: checked.agentId, + mcpServers: checked.mcpServers, actionConnectorId: checked.actionConnectorId, actionId: checked.actionId, actionTarget: checked.actionTarget, @@ -202,6 +229,7 @@ export async function updateLoop(id: string, draft: LoopDraft): Promise { await db() .insert(loops) @@ -225,6 +226,18 @@ describe("what the agent is asked", () => { expect(job.settings.permissionMode).toBe("workspace_write"); }); + it("carries the MCP servers the loop allows, and nothing when it allows none", async () => { + await givenLoop({ actionConnectorId: "github", actionId: "github.submit_review" }); + const plain = await enqueueTask({ loopId: LOOP_ID, url: "https://github.com/acme/web/pull/7", dryRun: true }); + await runTask(plain.id); + expect("mcpServers" in (await jobForTask(plain.id))).toBe(false); + + await db().update(loops).set({ mcpServers: ["sentry"] }).where(eq(loops.id, LOOP_ID)).run(); + const allowed = await enqueueTask({ loopId: LOOP_ID, url: "https://github.com/acme/web/pull/8", dryRun: true }); + await runTask(allowed.id); + expect((await jobForTask(allowed.id)).mcpServers).toEqual(["sentry"]); + }); + it("names the folder relative to home, and context files by the runner's workspace", async () => { const folder = "code/web"; await db() diff --git a/src/server/tasks.ts b/src/server/tasks.ts index 05a97cc..38b0582 100644 --- a/src/server/tasks.ts +++ b/src/server/tasks.ts @@ -21,10 +21,10 @@ import { WORKSPACE, folderIsLegacy, isAgentNothing, + jobMinProtocol, normalizeFolder, type AgentJob, } from "./agent-job.ts"; -import { FOLDER_JOB_PROTOCOL } from "#/lib/version.ts"; import { listAgents, settingsFor } from "./agents.ts"; import { branchFor } from "./checkout.ts"; import { credentialForConnector } from "./connections.ts"; @@ -586,8 +586,9 @@ async function prepareTask(id: string, signal?: AbortSignal): Promise const runnerId = await pickRunner({ agentId, requiresHost: folder !== null && folderIsLegacy(folder), - // A relative folder needs a runner that resolves it under its own home. - minProtocol: folder !== null && !folderIsLegacy(folder) ? FOLDER_JOB_PROTOCOL : 0, + // A relative folder needs a runner that resolves it under its own home, + // and allowed MCP servers one that passes them on to the agent. + minProtocol: jobMinProtocol({ cwd: folder ?? undefined, mcpServers: loop.mcpServers }), }); await updateTask(id, { state: "awaiting_agent", @@ -641,6 +642,7 @@ export async function jobForTask(id: string): Promise { settings, files: item.context.map((file) => ({ name: file.name, body: file.body })), ...(cwd ? { cwd } : {}), + ...(loop.mcpServers.length ? { mcpServers: loop.mcpServers } : {}), }; }