diff --git a/.rulesync/skills/rulesync-feature-research/references/takt.md b/.rulesync/skills/rulesync-feature-research/references/takt.md index 0974653c8..f58601429 100644 --- a/.rulesync/skills/rulesync-feature-research/references/takt.md +++ b/.rulesync/skills/rulesync-feature-research/references/takt.md @@ -2,25 +2,26 @@ ## Official Docs -| Feature | Official docs | Upstream surface | -| ------------- | ------------------------------------------------ | ---------------------------------------------------- | -| index | `https://github.com/nrslib/takt` | TAKT repository | -| `rules` | `https://nrslib.com/faceted-prompting/` | Faceted prompting policies | -| `ignore` | No dedicated upstream ignore surface in map | No Rulesync-supported TAKT ignore target in map | -| `mcp` | No dedicated upstream MCP surface in map | No Rulesync-supported TAKT MCP target in map | -| `commands` | `https://nrslib.com/faceted-prompting/` | Faceted prompting instructions | -| `subagents` | `https://nrslib.com/faceted-prompting/` | Faceted prompting personas | -| `skills` | `https://nrslib.com/faceted-prompting/` | Faceted prompting knowledge | -| `hooks` | No dedicated upstream hooks surface in map | No Rulesync-supported TAKT hooks target in map | -| `permissions` | No dedicated upstream permissions surface in map | No Rulesync-supported TAKT permissions target in map | +| Feature | Official docs | Upstream surface | +| ------------- | ---------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------ | +| index | `https://github.com/nrslib/takt` | TAKT repository | +| `rules` | `https://nrslib.com/faceted-prompting/` | Faceted prompting policies | +| `ignore` | No dedicated upstream ignore surface in map | No Rulesync-supported TAKT ignore target in map | +| `mcp` | `https://github.com/nrslib/takt/blob/main/docs/configuration.md` | `workflow_mcp_servers` transport allowlist in `config.yaml` (per-step `mcp_servers` not representable) | +| `commands` | `https://nrslib.com/faceted-prompting/` | Faceted prompting instructions | +| `subagents` | `https://nrslib.com/faceted-prompting/` | Faceted prompting personas | +| `skills` | `https://nrslib.com/faceted-prompting/` | Faceted prompting knowledge | +| `hooks` | No dedicated upstream hooks surface in map | No Rulesync-supported TAKT hooks target in map | +| `permissions` | No dedicated upstream permissions surface in map | No Rulesync-supported TAKT permissions target in map | ## Client Anchors Common adapter paths: `rulesync-source-map.md`. -| Surface | Anchor | -| ----------- | ------------------------------------------------------------------------------------------------------ | -| `rules` | `.takt/facets/policies`, `takt.name` stem override, and plain-Markdown output in `takt-rule.ts` | -| `commands` | `.takt/facets/instructions`, `takt.name` stem override, and frontmatter stripping in `takt-command.ts` | -| `subagents` | `.takt/facets/personas`, `takt.name` stem override, and plain-Markdown output in `takt-subagent.ts` | -| `skills` | Flat `.takt/facets/knowledge/{name}.md` output and unsupported reverse import in `takt-skill.ts` | +| Surface | Anchor | +| ----------- | ---------------------------------------------------------------------------------------------------------------------------------------------- | +| `rules` | `.takt/facets/policies`, `takt.name` stem override, and plain-Markdown output in `takt-rule.ts` | +| `commands` | `.takt/facets/instructions`, `takt.name` stem override, and frontmatter stripping in `takt-command.ts` | +| `subagents` | `.takt/facets/personas`, `takt.name` stem override, and plain-Markdown output in `takt-subagent.ts` | +| `skills` | Flat `.takt/facets/knowledge/{name}.md` output and unsupported reverse import in `takt-skill.ts` | +| `mcp` | `workflow_mcp_servers` default-deny transport allowlist merged into `.takt/config.yaml`; server definitions not representable in `takt-mcp.ts` | diff --git a/README.md b/README.md index 126b5a6fa..5f999c7b4 100644 --- a/README.md +++ b/README.md @@ -94,7 +94,7 @@ The tables below show whether each tool supports a given feature (✅ = supporte | Kilo Code | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Roo Code | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | | | Rovodev (Atlassian) | ✅ | | ✅ | | ✅ | ✅ | | ✅ | -| Takt | ✅ | | | ✅ | ✅ | ✅ | | ✅ | +| Takt | ✅ | | ✅ | ✅ | ✅ | ✅ | | ✅ | | Vibe Code | ✅ | ✅ | ✅ | | ✅ | ✅ | ✅ | ✅ | | Qwen Code | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Reasonix | | | ✅ | | | | | | diff --git a/cspell.json b/cspell.json index 0753905fb..55f2404b5 100644 --- a/cspell.json +++ b/cspell.json @@ -23,6 +23,7 @@ ], "words": [ "agentic", + "lossiness", "Blocklist", "aiassistant", "anthropics", diff --git a/docs/reference/file-formats.md b/docs/reference/file-formats.md index 16523e7b4..ad71aac40 100644 --- a/docs/reference/file-formats.md +++ b/docs/reference/file-formats.md @@ -614,6 +614,8 @@ The `type` (and the equivalent `transport`) field accepts `local`, `stdio`, `sse > **Kilo Code note:** Kilo's MCP config uses its own native shape in `kilo.jsonc` (`type: "local" | "remote"`, `environment`, `enabled`, `command` as an array). Rulesync maps `stdio`/`local` ⇄ Kilo `local` and `http`/`sse` ⇄ Kilo `remote`; on import, Kilo `remote` is normalized to the canonical `http` transport (the deprecated `sse` is no longer emitted). The Kilo-specific `timeout` (local + remote, a positive integer in milliseconds) and `oauth` (remote only — either an OAuth-config object or `false` to disable auto-detection) fields are preserved on round-trip. The `kilo.jsonc` `skills` config key (`skills.paths` for extra skill locations and `skills.urls` for remote skill manifests) is likewise preserved when Rulesync writes the file. +> **Takt note (partial / transport-allowlist only):** Takt does **not** have a project- or global-level registry of MCP server _definitions_. The concrete `mcp_servers` map (`command`/`args`/`env` or `type`/`url`/`headers`) is declared **per workflow step** inside individual workflow YAML files; there is no top-level `mcp_servers` key in `config.yaml`, and Takt's config loader hard-rejects unknown top-level keys (introduced with MCP support in [Takt v0.21.0](https://github.com/nrslib/takt/blob/main/CHANGELOG.md)). What `config.yaml` _does_ hold is the **default-deny transport allowlist** `workflow_mcp_servers: { stdio, sse, http }` — without it, workflow-defined MCP servers are refused regardless of how they are declared. So Rulesync emits **only** this allowlist into the shared `.takt/config.yaml` (project) / `~/.takt/config.yaml` (global), enabling exactly the transports your `.rulesync/mcp.json` servers use (`local`/`stdio` ⇒ `stdio`; `sse` ⇒ `sse`; `http`/`streamable-http`/`ws` ⇒ `http`). The merge is in place — every other top-level key (`provider`, `provider_profiles`, …) is preserved and the file is never deleted. **Documented lossiness:** per-server names, commands, env, URLs, and headers are not representable in `config.yaml` and are intentionally **not** written; you still declare the concrete servers in your workflow YAML steps, and Rulesync only opens the transport gate that permits them. As a corollary, **import** cannot reconstruct server definitions from a transport allowlist and yields an empty `mcpServers` map. See the [Takt configuration docs](https://github.com/nrslib/takt/blob/main/docs/configuration.md). + ### MCP Tool Config (`enabledTools` / `disabledTools`) You can control which individual tools from an MCP server are enabled or disabled using `enabledTools` and `disabledTools` arrays per server. diff --git a/docs/reference/supported-tools.md b/docs/reference/supported-tools.md index 63ded6e21..6c45c18f2 100644 --- a/docs/reference/supported-tools.md +++ b/docs/reference/supported-tools.md @@ -24,7 +24,7 @@ Rulesync supports both **generation** and **import** for All of the major AI cod | Kilo Code | kilo | ✅ 🌏 | ✅ | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | Roo Code | roo | ✅ 🌏 | ✅ | ✅ | ✅ | ✅ | ✅ 🌏 | | | | Rovodev (Atlassian) | rovodev | ✅ 🌏 | | 🌏 | | ✅ 🌏 | ✅ 🌏 | | 🌏 | -| Takt | takt | ✅ 🌏 | | | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | ✅ 🌏 | +| Takt | takt | ✅ 🌏 | | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | ✅ 🌏 | | Vibe Code | vibe | ✅ 🌏 | ✅ | ✅ 🌏 | | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | Qwen Code | qwencode | ✅ 🌏 | ✅ | ✅ 🌏 🔧 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | Reasonix | reasonix | | | ✅ 🌏 | | | | | | diff --git a/docs/tools/takt.md b/docs/tools/takt.md index 0623f1255..dee11ca04 100644 --- a/docs/tools/takt.md +++ b/docs/tools/takt.md @@ -38,9 +38,17 @@ Output files are **plain Markdown** — the source frontmatter is dropped entire .rulesync/skills/oncall/SKILL.md → .takt/facets/knowledge/oncall.md ``` +## MCP (partial — transport allowlist only) + +Takt has no project- or global-level registry of MCP server _definitions_: the concrete `mcp_servers` map (`command`/`args`/`env` or `type`/`url`/`headers`) is declared **per workflow step** inside individual workflow YAML files, and Takt's `config.yaml` loader rejects unknown top-level keys. The one MCP knob `config.yaml` does expose is the **default-deny transport allowlist** `workflow_mcp_servers: { stdio, sse, http }`; until a transport is enabled there, every workflow-defined MCP server using it is refused. + +Rulesync therefore emits **only** this allowlist into the shared `.takt/config.yaml` (project) / `~/.takt/config.yaml` (global), turning on exactly the transports the servers in `.rulesync/mcp.json` use (`local`/`stdio` → `stdio`, `sse` → `sse`, `http`/`streamable-http`/`ws` → `http`). The merge is in place, so the active provider, provider profiles, and all other config keys are preserved; the file is never deleted. + +**Lossiness:** the per-server names, commands, env, URLs, and headers are not representable in `config.yaml` and are intentionally not written — you still declare the concrete servers in your workflow YAML steps; Rulesync only opens the transport gate that permits them. Because of this, reverse import cannot reconstruct server definitions and yields an empty `mcpServers` map. + ## Scope -Both project mode (`.takt/facets/...`) and global mode (`~/.takt/facets/...`) are supported. +Both project mode (`.takt/facets/...`, `.takt/config.yaml`) and global mode (`~/.takt/facets/...`, `~/.takt/config.yaml`) are supported. ## Importing existing TAKT files into rulesync diff --git a/skills/rulesync/file-formats.md b/skills/rulesync/file-formats.md index 16523e7b4..ad71aac40 100644 --- a/skills/rulesync/file-formats.md +++ b/skills/rulesync/file-formats.md @@ -614,6 +614,8 @@ The `type` (and the equivalent `transport`) field accepts `local`, `stdio`, `sse > **Kilo Code note:** Kilo's MCP config uses its own native shape in `kilo.jsonc` (`type: "local" | "remote"`, `environment`, `enabled`, `command` as an array). Rulesync maps `stdio`/`local` ⇄ Kilo `local` and `http`/`sse` ⇄ Kilo `remote`; on import, Kilo `remote` is normalized to the canonical `http` transport (the deprecated `sse` is no longer emitted). The Kilo-specific `timeout` (local + remote, a positive integer in milliseconds) and `oauth` (remote only — either an OAuth-config object or `false` to disable auto-detection) fields are preserved on round-trip. The `kilo.jsonc` `skills` config key (`skills.paths` for extra skill locations and `skills.urls` for remote skill manifests) is likewise preserved when Rulesync writes the file. +> **Takt note (partial / transport-allowlist only):** Takt does **not** have a project- or global-level registry of MCP server _definitions_. The concrete `mcp_servers` map (`command`/`args`/`env` or `type`/`url`/`headers`) is declared **per workflow step** inside individual workflow YAML files; there is no top-level `mcp_servers` key in `config.yaml`, and Takt's config loader hard-rejects unknown top-level keys (introduced with MCP support in [Takt v0.21.0](https://github.com/nrslib/takt/blob/main/CHANGELOG.md)). What `config.yaml` _does_ hold is the **default-deny transport allowlist** `workflow_mcp_servers: { stdio, sse, http }` — without it, workflow-defined MCP servers are refused regardless of how they are declared. So Rulesync emits **only** this allowlist into the shared `.takt/config.yaml` (project) / `~/.takt/config.yaml` (global), enabling exactly the transports your `.rulesync/mcp.json` servers use (`local`/`stdio` ⇒ `stdio`; `sse` ⇒ `sse`; `http`/`streamable-http`/`ws` ⇒ `http`). The merge is in place — every other top-level key (`provider`, `provider_profiles`, …) is preserved and the file is never deleted. **Documented lossiness:** per-server names, commands, env, URLs, and headers are not representable in `config.yaml` and are intentionally **not** written; you still declare the concrete servers in your workflow YAML steps, and Rulesync only opens the transport gate that permits them. As a corollary, **import** cannot reconstruct server definitions from a transport allowlist and yields an empty `mcpServers` map. See the [Takt configuration docs](https://github.com/nrslib/takt/blob/main/docs/configuration.md). + ### MCP Tool Config (`enabledTools` / `disabledTools`) You can control which individual tools from an MCP server are enabled or disabled using `enabledTools` and `disabledTools` arrays per server. diff --git a/skills/rulesync/supported-tools.md b/skills/rulesync/supported-tools.md index 63ded6e21..6c45c18f2 100644 --- a/skills/rulesync/supported-tools.md +++ b/skills/rulesync/supported-tools.md @@ -24,7 +24,7 @@ Rulesync supports both **generation** and **import** for All of the major AI cod | Kilo Code | kilo | ✅ 🌏 | ✅ | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | Roo Code | roo | ✅ 🌏 | ✅ | ✅ | ✅ | ✅ | ✅ 🌏 | | | | Rovodev (Atlassian) | rovodev | ✅ 🌏 | | 🌏 | | ✅ 🌏 | ✅ 🌏 | | 🌏 | -| Takt | takt | ✅ 🌏 | | | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | ✅ 🌏 | +| Takt | takt | ✅ 🌏 | | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | ✅ 🌏 | | Vibe Code | vibe | ✅ 🌏 | ✅ | ✅ 🌏 | | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | Qwen Code | qwencode | ✅ 🌏 | ✅ | ✅ 🌏 🔧 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | Reasonix | reasonix | | | ✅ 🌏 | | | | | | diff --git a/src/constants/takt-paths.ts b/src/constants/takt-paths.ts index 76001a639..bba137b93 100644 --- a/src/constants/takt-paths.ts +++ b/src/constants/takt-paths.ts @@ -17,3 +17,12 @@ export const TAKT_RULE_OVERVIEW_FILE_NAME = "overview.md"; * @see https://github.com/nrslib/takt/blob/main/docs/configuration.md */ export const TAKT_CONFIG_FILE_NAME = "config.yaml"; + +/** + * Top-level key in Takt's `config.yaml` holding the workflow MCP transport + * allowlist (`stdio` / `sse` / `http` booleans). Takt is default-deny: a + * transport must be explicitly enabled here before any workflow-defined MCP + * server using it is permitted to run. + * @see https://github.com/nrslib/takt/blob/main/docs/configuration.md + */ +export const TAKT_WORKFLOW_MCP_SERVERS_KEY = "workflow_mcp_servers"; diff --git a/src/e2e/e2e-mcp.spec.ts b/src/e2e/e2e-mcp.spec.ts index 1ec0e683a..1499cb1e4 100644 --- a/src/e2e/e2e-mcp.spec.ts +++ b/src/e2e/e2e-mcp.spec.ts @@ -2,6 +2,7 @@ import { spawn } from "node:child_process"; import { join } from "node:path"; import { setTimeout } from "node:timers/promises"; +import { load } from "js-yaml"; import * as smolToml from "smol-toml"; import { describe, expect, it } from "vitest"; @@ -222,6 +223,11 @@ describe("E2E: mcp", () => { outputPath: "reasonix.toml", content: 'default_model = "deepseek"\n', }, + { + target: "takt", + outputPath: join(".takt", "config.yaml"), + content: "provider: claude\n", + }, ])( "should succeed in check mode when a $target mcp file is non-deletable", async ({ target, outputPath, content }) => { @@ -327,6 +333,41 @@ describe("E2E: mcp", () => { expect(parsed.disabled_tools).toContain("write_file"); }); + it("should generate Takt MCP transport allowlist into .takt/config.yaml", async () => { + const testDir = getTestDir(); + + // Pre-seed an unrelated key so the in-place merge can be asserted. + await writeFileContent(join(testDir, ".takt", "config.yaml"), "provider: claude\n"); + + await writeFileContent( + join(testDir, RULESYNC_MCP_RELATIVE_FILE_PATH), + JSON.stringify( + { + mcpServers: { + "test-server": { type: "stdio", command: "echo", args: ["hello"] }, + "remote-server": { type: "http", url: "https://example.com/mcp" }, + }, + }, + null, + 2, + ), + ); + + await runGenerate({ target: "takt", features: "mcp" }); + + const parsed = toTable( + load(await readFileContent(join(testDir, ".takt", "config.yaml"))) as Record, + ); + // The default-deny transport allowlist reflects the servers' transports. + expect(toTable(parsed.workflow_mcp_servers)).toEqual({ stdio: true, sse: false, http: true }); + // The unrelated key is preserved by the in-place merge. + expect(parsed.provider).toBe("claude"); + // Server definitions are NOT representable in config.yaml and are not written. + const raw = await readFileContent(join(testDir, ".takt", "config.yaml")); + expect(raw).not.toContain("test-server"); + expect(raw).not.toContain("remote-server"); + }); + it("should generate Reasonix MCP into reasonix.toml as [[plugins]] entries", async () => { const testDir = getTestDir(); @@ -566,6 +607,37 @@ describe("E2E: mcp (global mode)", () => { expect(generatedContent).toContain("test-server"); }); + it("should generate Takt MCP transport allowlist into ~/.takt/config.yaml (global)", async () => { + const projectDir = getProjectDir(); + const homeDir = getHomeDir(); + + await writeFileContent( + join(projectDir, RULESYNC_MCP_RELATIVE_FILE_PATH), + JSON.stringify( + { + root: true, + mcpServers: { + "test-server": { type: "sse", url: "https://example.com/sse" }, + }, + }, + null, + 2, + ), + ); + + await runGenerate({ + target: "takt", + features: "mcp", + global: true, + env: { HOME_DIR: homeDir }, + }); + + const parsed = toTable( + load(await readFileContent(join(homeDir, ".takt", "config.yaml"))) as Record, + ); + expect(toTable(parsed.workflow_mcp_servers)).toEqual({ stdio: false, sse: true, http: false }); + }); + it("should preserve legacy ~/.claude/.claude.json when writing to recommended path (global)", async () => { // Pins both behaviors end-to-end: (a) canonical ~/.claude.json receives // fresh mcpServers AND preserves Claude Code's own user-config keys via diff --git a/src/features/mcp/mcp-processor.ts b/src/features/mcp/mcp-processor.ts index f87a05287..578b6adda 100644 --- a/src/features/mcp/mcp-processor.ts +++ b/src/features/mcp/mcp-processor.ts @@ -33,6 +33,7 @@ import { ReasonixMcp } from "./reasonix-mcp.js"; import { RooMcp } from "./roo-mcp.js"; import { RovodevMcp } from "./rovodev-mcp.js"; import { RulesyncMcp } from "./rulesync-mcp.js"; +import { TaktMcp } from "./takt-mcp.js"; import { ToolMcp, ToolMcpForDeletionParams, @@ -437,6 +438,25 @@ export const toolMcpFactories = new Map( }, }, ], + [ + "takt", + { + // Takt has no project/global registry of MCP server *definitions* — those + // live per-step in workflow YAML. `.takt/config.yaml` (project) / + // `~/.takt/config.yaml` (global) only hold the default-deny transport + // allowlist `workflow_mcp_servers: { stdio, sse, http }`. TaktMcp emits + // that allowlist (derived from the rulesync servers' transports); the + // concrete server map is intentionally not representable here. + // https://github.com/nrslib/takt/blob/main/docs/configuration.md + class: TaktMcp, + meta: { + supportsProject: true, + supportsGlobal: true, + supportsEnabledTools: false, + supportsDisabledTools: false, + }, + }, + ], [ "vibe", { diff --git a/src/features/mcp/takt-mcp.test.ts b/src/features/mcp/takt-mcp.test.ts new file mode 100644 index 000000000..c99cb69f4 --- /dev/null +++ b/src/features/mcp/takt-mcp.test.ts @@ -0,0 +1,186 @@ +import { join } from "node:path"; + +import { load } from "js-yaml"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { setupTestDirectory } from "../../test-utils/test-directories.js"; +import { writeFileContent } from "../../utils/file.js"; +import { RulesyncMcp } from "./rulesync-mcp.js"; +import { TaktMcp } from "./takt-mcp.js"; + +const makeRulesyncMcp = (mcpServers: Record) => + new RulesyncMcp({ + relativeDirPath: ".rulesync", + relativeFilePath: "mcp.json", + fileContent: JSON.stringify({ mcpServers }), + }); + +const toRecord = (value: unknown): Record => + value && typeof value === "object" && !Array.isArray(value) + ? (value as Record) + : {}; + +const readAllowlist = (content: string): Record => + toRecord(toRecord(load(content)).workflow_mcp_servers); + +describe("TaktMcp", () => { + let testDir: string; + let cleanup: () => Promise; + + beforeEach(async () => { + ({ testDir, cleanup } = await setupTestDirectory()); + vi.spyOn(process, "cwd").mockReturnValue(testDir); + }); + + afterEach(async () => { + await cleanup(); + vi.restoreAllMocks(); + }); + + describe("getSettablePaths", () => { + it("writes to .takt/config.yaml for both scopes", () => { + expect(TaktMcp.getSettablePaths()).toEqual({ + relativeDirPath: ".takt", + relativeFilePath: "config.yaml", + }); + expect(TaktMcp.getSettablePaths({ global: true })).toEqual({ + relativeDirPath: ".takt", + relativeFilePath: "config.yaml", + }); + }); + }); + + describe("fromRulesyncMcp (generate)", () => { + it("enables only the transports present among the servers", async () => { + const mcp = await TaktMcp.fromRulesyncMcp({ + outputRoot: testDir, + rulesyncMcp: makeRulesyncMcp({ + local: { type: "stdio", command: "echo", args: ["hi"] }, + remote: { type: "http", url: "https://example.com/mcp" }, + }), + }); + + expect(readAllowlist(mcp.getFileContent())).toEqual({ + stdio: true, + sse: false, + http: true, + }); + }); + + it("treats sse type and aliases (local/streamable-http/ws) correctly", async () => { + const mcp = await TaktMcp.fromRulesyncMcp({ + outputRoot: testDir, + rulesyncMcp: makeRulesyncMcp({ + a: { type: "sse", url: "https://example.com/sse" }, + b: { type: "local", command: "node" }, + c: { type: "streamable-http", url: "https://example.com/s" }, + }), + }); + + expect(readAllowlist(mcp.getFileContent())).toEqual({ + stdio: true, + sse: true, + http: true, + }); + }); + + it("infers stdio from a bare command and http from a bare url", async () => { + const mcp = await TaktMcp.fromRulesyncMcp({ + outputRoot: testDir, + rulesyncMcp: makeRulesyncMcp({ + a: { command: "echo" }, + b: { url: "https://example.com/mcp" }, + }), + }); + + expect(readAllowlist(mcp.getFileContent())).toEqual({ + stdio: true, + sse: false, + http: true, + }); + }); + + it("emits an all-false allowlist when there are no servers", async () => { + const mcp = await TaktMcp.fromRulesyncMcp({ + outputRoot: testDir, + rulesyncMcp: makeRulesyncMcp({}), + }); + + expect(readAllowlist(mcp.getFileContent())).toEqual({ + stdio: false, + sse: false, + http: false, + }); + }); + + it("preserves unrelated existing config keys (in-place merge)", async () => { + await writeFileContent( + join(testDir, ".takt", "config.yaml"), + [ + "provider: claude", + "provider_profiles:", + " claude:", + " default_permission_mode: edit", + ].join("\n"), + ); + + const mcp = await TaktMcp.fromRulesyncMcp({ + outputRoot: testDir, + rulesyncMcp: makeRulesyncMcp({ + local: { type: "stdio", command: "echo" }, + }), + }); + + const parsed = toRecord(load(mcp.getFileContent())); + expect(parsed.provider).toBe("claude"); + expect(toRecord(toRecord(parsed.provider_profiles).claude).default_permission_mode).toBe( + "edit", + ); + expect(readAllowlist(mcp.getFileContent())).toEqual({ + stdio: true, + sse: false, + http: false, + }); + }); + + it("does not write any server name/command into config.yaml (documented lossiness)", async () => { + const mcp = await TaktMcp.fromRulesyncMcp({ + outputRoot: testDir, + rulesyncMcp: makeRulesyncMcp({ + "secret-server": { type: "stdio", command: "run-secret", env: { TOKEN: "abc" } }, + }), + }); + + const content = mcp.getFileContent(); + expect(content).not.toContain("secret-server"); + expect(content).not.toContain("run-secret"); + expect(content).not.toContain("TOKEN"); + }); + }); + + describe("toRulesyncMcp (import)", () => { + it("yields an empty mcpServers map (definitions are not recoverable)", () => { + const mcp = new TaktMcp({ + relativeDirPath: ".takt", + relativeFilePath: "config.yaml", + fileContent: "workflow_mcp_servers:\n stdio: true\n sse: false\n http: false\n", + validate: false, + }); + + const rulesync = mcp.toRulesyncMcp(); + const parsed = toRecord(JSON.parse(rulesync.getFileContent())); + expect(parsed.mcpServers).toEqual({}); + }); + }); + + describe("isDeletable", () => { + it("is never deletable (shared config)", () => { + const mcp = TaktMcp.forDeletion({ + outputRoot: testDir, + relativeDirPath: ".takt", + relativeFilePath: "config.yaml", + }); + expect(mcp.isDeletable()).toBe(false); + }); + }); +}); diff --git a/src/features/mcp/takt-mcp.ts b/src/features/mcp/takt-mcp.ts new file mode 100644 index 000000000..160836b2d --- /dev/null +++ b/src/features/mcp/takt-mcp.ts @@ -0,0 +1,224 @@ +import { join } from "node:path"; + +import { dump } from "js-yaml"; + +import { + TAKT_CONFIG_FILE_NAME, + TAKT_DIR, + TAKT_WORKFLOW_MCP_SERVERS_KEY, +} from "../../constants/takt-paths.js"; +import type { ValidationResult } from "../../types/ai-file.js"; +import type { McpServer, McpServers } from "../../types/mcp.js"; +import { readFileContentOrNull } from "../../utils/file.js"; +import { isRecord } from "../../utils/type-guards.js"; +import { parseTaktConfig } from "../shared/takt-config.js"; +import { RulesyncMcp } from "./rulesync-mcp.js"; +import { + ToolMcp, + type ToolMcpForDeletionParams, + type ToolMcpFromFileParams, + type ToolMcpFromRulesyncMcpParams, + type ToolMcpParams, + type ToolMcpSettablePaths, +} from "./tool-mcp.js"; + +/** + * The three MCP transports Takt's allowlist can gate. Takt models exactly these + * (`stdio` / `sse` / `http`); every other rulesync transport alias is folded + * onto one of them by {@link transportOf}. + */ +type TaktTransport = "stdio" | "sse" | "http"; + +/** + * MCP adapter for Takt (`.takt/config.yaml` project / `~/.takt/config.yaml` + * global). + * + * IMPORTANT — what this adapter can and cannot represent. + * + * Takt does NOT have a project- or global-level registry of MCP *server + * definitions*. The concrete `mcp_servers` map (a server's `command`/`args`/`env` + * or `type`/`url`/`headers`) is declared per-step inside individual *workflow* + * YAML files; there is no top-level `mcp_servers` key in `config.yaml`, and the + * config loader hard-rejects unknown top-level keys + * (`assertNoUnknownGlobalConfigKeys`). Writing a server map into `config.yaml` + * would therefore both be ignored and break the user's config. + * + * What `config.yaml` *does* hold is the default-deny transport allowlist + * `workflow_mcp_servers: { stdio, sse, http }`. Without it, workflow-defined MCP + * servers are refused regardless of how they are declared. So this adapter emits + * the transport allowlist derived from the transports present in + * `.rulesync/mcp.json`, enabling exactly the transports the user's servers need. + * + * Lossiness (documented, intentional): the per-server names, commands, env, URLs + * and headers are NOT representable in `config.yaml` and are intentionally not + * written. Users still declare the concrete servers in their workflow YAML + * steps; rulesync only opens the transport gate that permits them. As a + * corollary, import (`toRulesyncMcp`) cannot reconstruct server definitions from + * a transport allowlist and yields an empty `mcpServers` map. + * + * The shared `config.yaml` is merged in place: only the + * `workflow_mcp_servers` key is set; every other top-level key (provider, + * provider_profiles, etc.) is preserved. The file is never deleted. + * + * @see https://github.com/nrslib/takt/blob/main/docs/configuration.md + * @see https://github.com/nrslib/takt/blob/main/src/core/models/mcp-schemas.ts + */ +export class TaktMcp extends ToolMcp { + constructor(params: ToolMcpParams) { + super({ + ...params, + fileContent: params.fileContent ?? "", + }); + } + + override isDeletable(): boolean { + // config.yaml holds other Takt settings (provider, profiles, ...), so it + // must never be removed wholesale; changes happen via in-place merge. + return false; + } + + static getSettablePaths(_options?: { global?: boolean }): ToolMcpSettablePaths { + // Project: `.takt/config.yaml`; global: `~/.takt/config.yaml` (the home + // directory is resolved by the processor through outputRoot). Same as the + // permissions adapter so the two features co-locate in one config file. + return { + relativeDirPath: TAKT_DIR, + relativeFilePath: TAKT_CONFIG_FILE_NAME, + }; + } + + static async fromFile({ + outputRoot = process.cwd(), + validate = true, + global = false, + }: ToolMcpFromFileParams): Promise { + const paths = TaktMcp.getSettablePaths({ global }); + const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); + const fileContent = (await readFileContentOrNull(filePath)) ?? ""; + return new TaktMcp({ + outputRoot, + relativeDirPath: paths.relativeDirPath, + relativeFilePath: paths.relativeFilePath, + fileContent, + validate, + global, + }); + } + + static async fromRulesyncMcp({ + outputRoot = process.cwd(), + rulesyncMcp, + validate = true, + global = false, + }: ToolMcpFromRulesyncMcpParams): Promise { + const paths = TaktMcp.getSettablePaths({ global }); + const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); + // Read without initializing so a dry-run/check does not create the user's + // config.yaml as a side effect (mirrors the permissions adapter). + const existingContent = (await readFileContentOrNull(filePath)) ?? ""; + const config = parseTaktConfig(existingContent, paths.relativeDirPath, paths.relativeFilePath); + + const allowlist = deriveTransportAllowlist(rulesyncMcp.getMcpServers()); + + const merged: Record = { + ...config, + [TAKT_WORKFLOW_MCP_SERVERS_KEY]: allowlist, + }; + + return new TaktMcp({ + outputRoot, + relativeDirPath: paths.relativeDirPath, + relativeFilePath: paths.relativeFilePath, + fileContent: dump(merged), + validate, + global, + }); + } + + /** + * A transport allowlist cannot reconstruct the per-step server definitions, + * so import yields an empty `mcpServers` map. This keeps the round-trip honest + * rather than fabricating placeholder servers. + */ + toRulesyncMcp(): RulesyncMcp { + return this.toRulesyncMcpDefault({ + fileContent: JSON.stringify({ mcpServers: {} }, null, 2), + }); + } + + validate(): ValidationResult { + return { success: true, error: null }; + } + + static forDeletion({ + outputRoot = process.cwd(), + relativeDirPath, + relativeFilePath, + global = false, + }: ToolMcpForDeletionParams): TaktMcp { + return new TaktMcp({ + outputRoot, + relativeDirPath, + relativeFilePath, + fileContent: "", + validate: false, + global, + }); + } +} + +/** + * Map a rulesync MCP server onto the single Takt transport its allowlist gates. + * + * Takt allows only `stdio` / `sse` / `http`, so the broader rulesync alias set is + * folded: `local` ⇒ stdio; `streamable-http` / `ws` ⇒ http. A server with no + * explicit transport is treated as stdio when it carries a `command`, else as a + * remote `http` server (it must have a `url`). Returns `undefined` only when the + * shape is too ambiguous to classify. + */ +function transportOf(server: McpServer): TaktTransport | undefined { + const declared = server.type ?? server.transport; + switch (declared) { + case "stdio": + case "local": + return "stdio"; + case "sse": + return "sse"; + case "http": + case "streamable-http": + case "ws": + return "http"; + default: + break; + } + if (server.command !== undefined) { + return "stdio"; + } + if (server.url !== undefined || server.httpUrl !== undefined) { + return "http"; + } + return undefined; +} + +/** + * Derive Takt's `workflow_mcp_servers` allowlist from the transports present in + * the rulesync servers. All three keys are emitted explicitly (default-deny made + * visible): a transport is `true` only when at least one server uses it. + * + * Server entries are read defensively (record guard); prototype-pollution server + * names are irrelevant here because no user-controlled key or value is written — + * only the three fixed boolean keys are. + */ +function deriveTransportAllowlist(servers: McpServers): Record { + const allowlist: Record = { stdio: false, sse: false, http: false }; + + for (const server of Object.values(servers)) { + if (!isRecord(server)) continue; + const transport = transportOf(server as McpServer); + if (transport) { + allowlist[transport] = true; + } + } + + return allowlist; +} diff --git a/src/features/permissions/takt-permissions.ts b/src/features/permissions/takt-permissions.ts index 62c083adb..d94586eb4 100644 --- a/src/features/permissions/takt-permissions.ts +++ b/src/features/permissions/takt-permissions.ts @@ -1,13 +1,13 @@ import { join } from "node:path"; -import { dump, load } from "js-yaml"; +import { dump } from "js-yaml"; import { TAKT_CONFIG_FILE_NAME, TAKT_DIR } from "../../constants/takt-paths.js"; import type { AiFileParams, ValidationResult } from "../../types/ai-file.js"; import type { PermissionsConfig } from "../../types/permissions.js"; -import { formatError } from "../../utils/error.js"; import { readFileContentOrNull } from "../../utils/file.js"; import { isPlainObject } from "../../utils/type-guards.js"; +import { parseTaktConfig } from "../shared/takt-config.js"; import { RulesyncPermissions } from "./rulesync-permissions.js"; import { ToolPermissions, @@ -191,35 +191,6 @@ export class TaktPermissions extends ToolPermissions { } } -/** - * Parse a Takt `config.yaml` into a plain object, treating an empty file as `{}`. - */ -function parseTaktConfig( - fileContent: string, - relativeDirPath: string, - relativeFilePath: string, -): Record { - const configPath = join(relativeDirPath, relativeFilePath); - let parsed: unknown; - try { - parsed = fileContent.trim() === "" ? {} : load(fileContent); - } catch (error) { - throw new Error(`Failed to parse Takt config at ${configPath}: ${formatError(error)}`, { - cause: error, - }); - } - // An empty config.yaml parses to undefined/null; treat it as an empty object. - if (parsed === undefined || parsed === null) { - return {}; - } - // `isPlainObject` (not `isRecord`) rejects class instances for - // prototype-pollution hardening; a YAML mapping always parses to a plain object. - if (!isPlainObject(parsed)) { - throw new Error(`Failed to parse Takt config at ${configPath}: expected a YAML mapping`); - } - return parsed; -} - /** * Resolve the active Takt provider: the top-level `provider:` value, else the * sole key in `provider_profiles`, else the `claude` default. diff --git a/src/features/shared/takt-config.test.ts b/src/features/shared/takt-config.test.ts new file mode 100644 index 000000000..d060ed4ab --- /dev/null +++ b/src/features/shared/takt-config.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from "vitest"; + +import { parseTaktConfig } from "./takt-config.js"; + +describe("parseTaktConfig", () => { + it("parses a YAML mapping into a plain object", () => { + expect(parseTaktConfig("provider: claude\n", ".takt", "config.yaml")).toEqual({ + provider: "claude", + }); + }); + + it("treats an empty file as an empty object", () => { + expect(parseTaktConfig("", ".takt", "config.yaml")).toEqual({}); + expect(parseTaktConfig(" \n", ".takt", "config.yaml")).toEqual({}); + }); + + it("throws with the config path on invalid YAML", () => { + expect(() => parseTaktConfig("a: [1, 2", ".takt", "config.yaml")).toThrow( + /Failed to parse Takt config at/, + ); + }); + + it("throws when the YAML is not a mapping", () => { + expect(() => parseTaktConfig("- a\n- b\n", ".takt", "config.yaml")).toThrow( + /expected a YAML mapping/, + ); + }); +}); diff --git a/src/features/shared/takt-config.ts b/src/features/shared/takt-config.ts new file mode 100644 index 000000000..07fd2b615 --- /dev/null +++ b/src/features/shared/takt-config.ts @@ -0,0 +1,38 @@ +import { join } from "node:path"; + +import { load } from "js-yaml"; + +import { formatError } from "../../utils/error.js"; +import { isPlainObject } from "../../utils/type-guards.js"; + +/** + * Parse a Takt `config.yaml` into a plain object, treating an empty file as `{}`. + * + * Shared by the Takt adapters that read-modify-write the same `config.yaml` + * (mcp, permissions, ...). Uses `isPlainObject` (not `isRecord`) so class + * instances are rejected for prototype-pollution hardening; a YAML mapping + * always parses to a plain object. + */ +export function parseTaktConfig( + fileContent: string, + relativeDirPath: string, + relativeFilePath: string, +): Record { + const configPath = join(relativeDirPath, relativeFilePath); + let parsed: unknown; + try { + parsed = fileContent.trim() === "" ? {} : load(fileContent); + } catch (error) { + throw new Error(`Failed to parse Takt config at ${configPath}: ${formatError(error)}`, { + cause: error, + }); + } + // An empty config.yaml parses to undefined/null; treat it as an empty object. + if (parsed === undefined || parsed === null) { + return {}; + } + if (!isPlainObject(parsed)) { + throw new Error(`Failed to parse Takt config at ${configPath}: expected a YAML mapping`); + } + return parsed; +} diff --git a/src/types/tool-target-tuples.ts b/src/types/tool-target-tuples.ts index 5ed00e2a4..867c132f8 100644 --- a/src/types/tool-target-tuples.ts +++ b/src/types/tool-target-tuples.ts @@ -89,6 +89,7 @@ export const mcpProcessorToolTargetTuple = [ "reasonix", "roo", "rovodev", + "takt", "vibe", "warp", "devin",