diff --git a/docs/designs/data-directory-layout.md b/docs/designs/data-directory-layout.md index c09e4cedc..093ece986 100644 --- a/docs/designs/data-directory-layout.md +++ b/docs/designs/data-directory-layout.md @@ -439,13 +439,23 @@ every checkout, so that is where they live now: ├── reports-wt/ (the side-branch locks sit beside them) ├── pending-learnings/ pendingLearningsDir → /pending-learnings └── workspaces// - ├── managed-mcp.json managedMcpManifestPath, one per checkout + ├── managed-mcp.json managedMcpManifestPath, one per checkout; Copilot placement is true for bare, false for keyed, absent when unproven ├── managed-mcp-files.json resolvedMcpFilesPath: project MCP configs teamai may have written a resolved ${VAR} to, and whether - │ the paths earlier teamai.yaml revisions mapped were read; one of those git tracks is marked tracked (#882) + │ the paths earlier teamai.yaml revisions mapped were read; one of those git tracks is marked tracked (#882); + │ for an HTTP team, the configs the local agent wrote a credential to └── search-index.json getProjectSearchIndexPath, one per checkout /.teamai/ one per checkout: committed knowledge, knowledge-wt/ ``` +MCP configs and ownership must describe the same completed writes. Existing +JSON local-agent installs keep the old record until the config write succeeds; +uninstall keeps it until the entry is removed. A later manifest-write failure +restores the previous config. Reconcile keeps one snapshot per config before +any tool writes it and restores those snapshots if saving ownership or a later +config write fails. File records added by that failed run are cleaned up before +Git protection is checked against the restored configs. If restoration also +fails, the command reports both failures and keeps credential files excluded. + `git worktree add` takes a path outside the repo, and the owning repo is still the business repo, whose refs every checkout shares. The search index is keyed per checkout, like managed MCP, because each checkout indexes its own branch's diff --git a/docs/usage-guide.md b/docs/usage-guide.md index 09b632016..d2a3ad433 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -1245,13 +1245,17 @@ precedence. For an existing team that pins run servers in either file; TeamAI does not migrate or delete the old file. Claude Code also reads the root `.mcp.json`, so this file is shared by both tools. +TeamAI removes a bare Copilot entry beside `mcpServers` only when its ownership record proves a completed bare write and the entry still matches that write. Older records without placement evidence leave the bare entry alone, even if it matches the team definition. A bare ownership record does not authorize changes to a same-named member entry under `mcpServers`; update skips that collision and removal cleans only the owned bare copy. An unmarked record can claim a keyed entry only when its hash matches that entry and does not also match the bare entry. Completed keyed writes record `bare: false`; a failed placement-record write leaves ownership unproven. New HTTP local-agent installs check Git protection without changing it, persist provisional ownership, then add the exclusion and file record before writing a credential. A failed initial ownership write changes neither Git exclusions nor the MCP config. + +An HTTP local-agent update keeps the existing JSON MCP ownership record until the config write succeeds. If saving the new record then fails, it restores the previous config. `uninstall_mcp` removes the entry before dropping its ownership record; a failed config write or an unreadable config keeps that record for a retry, and a failed manifest write restores the entry. A failed MCP reconcile restores each config it wrote before saving ownership, including a file shared by multiple tools. A restoration failure reports both errors and the affected files: repair the config and ownership record before retrying. Git protection remains while a credential is still present. + Copilot uses its native `mcpServers` schema: `stdio` becomes `type: "local"`, remote transports keep `http` or `sse`, and every managed entry gets the required `tools: ["*"]` allowlist. TeamAI honors `COPILOT_HOME`; project configuration uses Copilot CLI's documented `.github/mcp.json` repository location. See [Adding MCP servers for GitHub Copilot CLI](https://docs.github.com/en/copilot/how-tos/copilot-cli/customize-copilot/add-mcp-servers). Codex supports `stdio` and `http`; `sse` is skipped. Qoder supports the Claude-compatible `mcpServers` format in its scope-specific `.qoder/settings.json`. Kiro supports the same `mcpServers` format in its dedicated, mcpServers-only `.kiro/settings/mcp.json` (see [Kiro's MCP configuration docs](https://kiro.dev/docs/mcp/configuration/)). OpenCode supports `stdio` (written as its `type:"local"` shape) and `http` (`type:"remote"`); `sse` is skipped, and its servers live under the `mcp` key of the shared `opencode.json`. Ownership is tracked in `~/.teamai/managed-mcp.json` — hand-added servers are left alone; name collisions skip unless `--force`. **Secrets.** Write `${VAR}`, never a literal, in `mcp.yaml`. A key the team declares in `env/secrets.yaml` resolves from your value for this team (`teamai env set`), then your value for the machine (`teamai env set --global`), then your own environment, which leaves out values a teamai `env.sh` exported (see [Team secrets](designs/team-secrets.md#resolution)). Any other variable resolves from your value for this team (`teamai env set KEY`), then from the team env variables this directory receives (`env/env.yaml` and the active `env//env.yaml`); the environment fills only a key the team sets nothing for, and no longer overrides a team variable (see [Team secrets](designs/team-secrets.md#variables)). An interactive `pull` and `teamai doctor` say when your export differs from the team's value and is ignored. Unresolved variables skip the server with a hint. A declared secret is different: when a pull can't find it, the entry an earlier pull wrote stays as it is, so it may hold a value that was since rotated, until a pull finds the new one (see [Team secrets](designs/team-secrets.md#a-missing-secret-keeps-the-mcp-entry)). An interactive `pull`, `teamai mcp list`, `teamai env list`, `teamai doctor` and `teamai env exec` name a declared secret with no value, the servers that use it and the command that sets it: `` github: GITHUB_TOKEN is not set. Run `teamai env set GITHUB_TOKEN` (). `` teamai **resolves every `${VAR}` to its value and writes it verbatim** into each tool's config, which is then written `0600`, an existing `0644` one included (a config without a resolved value keeps its mode; new files are created `0600`). It does not rely on any tool's own env-var expansion: that expansion is fragile — most decisively, IDEs launched from the GUI (Dock/Launchpad) never inherit your shell's exported variables, so a `${VAR}` placeholder expands to empty and the server 401s. Resolving to plaintext makes the token present no matter how the tool is started. -> ⚠️ **The resolved token lands on disk.** Project-scope MCP configs (`.mcp.json`, `.github/mcp.json`, `.cursor/mcp.json`, `.codex/config.toml`, `opencode.json`) then contain the literal secret. Whenever such a file would hold a value teamai resolved and git would track it, teamai lists the path in the clone's `.git/info/exclude`, inside a `# [teamai:mcp-exclude:start]` block (the worktrees of a repo share it), before it writes the value. A config reached through a symlinked directory (say `.cursor/` linking to `config/`) is judged where the write lands: that path (`/config/mcp.json`) is the one listed, checked and reported, and a tracked one is named with both paths. A symlink at the file itself is replaced by the write, so there the file's own path counts. That covers a file this pull did not write: one written earlier for a tool since disabled, one at the built-in location of a tool the team has dropped from `toolPaths` or moved elsewhere (it counts while it holds any MCP server, since teamai's record for that tool describes another file or none; one another tool maps today, such as CodeBuddy's `.mcp.json`, which Claude maps, while it holds a server that tool did not write, as below), one written under a `toolPaths` mapping the team has since changed (each worktree records the files it wrote a resolved value to in `managed-mcp-files.json`, beside its `managed-mcp.json`; for one an older teamai wrote before it kept that record, the first pull reads each `mcpProject` path in the team repo's history of `teamai.yaml`, and the built-in ones teamai has since changed (CodeBuddy's `.codebuddy/mcp.json`), once, as far as the clone has it, inside the project only, skipping a path the same tool maps today; such a file counts while it holds any MCP server, since teamai's record for the tool describes only today's path (one another tool maps today, while it holds a server that tool did not write, as below), and `teamai doctor` checks the same files until that pull; one git tracks is not listed, since a line does nothing for it, but is recorded as tracked whatever it holds, judged as the others once git no longer tracks it (`git rm --cached`), and forgotten once it is gone from both the disk and git), or one still holding a server since removed from `mcp.yaml`. An entry a pull wrote with a resolved value counts while it is unchanged, even after the team makes its `${VAR}` a literal. While the worktree has no `managed-mcp.json` at all (lost, or before its first pull), a config git does not track counts while it holds a server no record claims, one of your own included: the pull notes those servers in `managed-mcp-files.json`, as when it rebuilds a lost record, and they keep its path until they leave the file; `teamai doctor` checks the same way. So does a config a pull writes a tool's first record for while `managed-mcp.json` holds none for that tool (lost, or teamai's first delivery to it). A path git cannot say it ignores is listed all the same once `git ls-files` shows the file untracked; when git cannot say that either, it counts as git failing. When it cannot — `.git/info` or the exclude file is not writable, another teamai command holds the exclude file past a short wait, git already tracks the file, a rule in your own git ignore files re-includes it (say `!/.mcp.json`; the warning names it), or git fails — it leaves that file as it was (an entry an earlier pull wrote stays), warns with the reason and the fix, and `teamai mcp list` and `teamai doctor` report the server as withheld from each tool a pull would write it to; make the file writable (or `git rm --cached` the tracked file, or remove the rule that re-includes it) and run `teamai pull` again. A tracked file is reported first, and listed nowhere. The committed `.gitignore` is left alone, a path git already ignores adds nothing, and a pull, `teamai mcp remove` and `teamai uninstall` remove a path from the block (the block with its last path) once that file is gone, holds no MCP server, or holds none of: a team server with a resolved value, an entry of teamai's that cleanup left, a server that was in the file when teamai rebuilt a lost `managed-mcp.json`, or the value (8+ characters) of a variable still set in the environment, with teamai's record of what it wrote there (`managed-mcp.json`) present before the command ran, readable, and holding an entry for that file's tool (for a file two tools map, such as Claude and CodeBuddy on `.mcp.json`: for each tool `managed-mcp-files.json` says wrote a resolved value there, or for each tool mapping it when it names none; an empty, unreadable or truncated record proves nothing, and neither does one written by a pull that rebuilt it or found no record for its tool in `managed-mcp.json`, while that pull could not note the file's other servers in `managed-mcp-files.json`, until a later pull notes them). A file written under a mapping since changed, one at the built-in location of a tool the team dropped or moved (unless another tool maps it today), or one in a linked worktree of a nested repository, needs to be gone or hold no MCP server. One written for a tool the team has since moved elsewhere (recorded, found in that history, or at the tool's built-in location), that another tool's mapping still reaches, also keeps its path while it holds a server the tools now mapping it did not write (by their `managed-mcp.json` record); as in any file under a changed mapping, a server of your own there keeps it too. `teamai uninstall` applies that to the file in every worktree of the repository; a pull and `teamai mcp remove` apply it only to the current worktree's file, and keep the path while the file in any other worktree still holds an MCP server: an entry that worktree's last pull wrote (say, a `${VAR}` the team has since made a literal) is judged only by a pull there. A path a pull listed and then wrote no value into (the file does not parse, or holds a server of your own under the team's name) comes out again at the end of that pull, and so does its record in `managed-mcp-files.json`. Otherwise, or for a file it cannot check (for example one that does not parse), the path stays, and `teamai uninstall` warns, naming the file and why: remove teamai's servers from it, then delete that line yourself (with its last line, the block's markers). `teamai doctor` reports such a file git would still commit or cannot answer for — for example one already tracked: `git rm --cached` it and rotate the token. +> ⚠️ **The resolved token lands on disk.** Project-scope MCP configs (`.mcp.json`, `.github/mcp.json`, `.cursor/mcp.json`, `.codex/config.toml`, `opencode.json`) then contain the literal secret. Whenever such a file would hold a value teamai resolved and git would track it, teamai lists the path in the clone's `.git/info/exclude`, inside a `# [teamai:mcp-exclude:start]` block (the worktrees of a repo share it), before it writes the value. A config reached through a symlinked directory (say `.cursor/` linking to `config/`) is judged where the write lands: that path (`/config/mcp.json`) is the one listed, checked and reported, and a tracked one is named with both paths. A symlink at the file itself is replaced by the write, so there the file's own path counts. That covers a file this pull did not write: one written earlier for a tool since disabled, one at the built-in location of a tool the team has dropped from `toolPaths` or moved elsewhere (it counts while it holds any MCP server, since teamai's record for that tool describes another file or none; one another tool maps today, such as CodeBuddy's `.mcp.json`, which Claude maps, while it holds a server that tool did not write, as below), one written under a `toolPaths` mapping the team has since changed (each worktree records the files it wrote a resolved value to in `managed-mcp-files.json`, beside its `managed-mcp.json`; for one an older teamai wrote before it kept that record, the first pull reads each `mcpProject` path in the team repo's history of `teamai.yaml`, and the built-in ones teamai has since changed (CodeBuddy's `.codebuddy/mcp.json`), once, as far as the clone has it, inside the project only, skipping a path the same tool maps today; such a file counts while it holds any MCP server, since teamai's record for the tool describes only today's path (one another tool maps today, while it holds a server that tool did not write, as below), and `teamai doctor` checks the same files until that pull; one git tracks is not listed, since a line does nothing for it, but is recorded as tracked whatever it holds, judged as the others once git no longer tracks it (`git rm --cached`), and forgotten once it is gone from both the disk and git), or one still holding a server since removed from `mcp.yaml`. An entry a pull wrote with a resolved value counts while it is unchanged, even after the team makes its `${VAR}` a literal. While the worktree has no `managed-mcp.json` at all (lost, or before its first pull), a config git does not track counts while it holds a server no record claims, one of your own included: the pull notes those servers in `managed-mcp-files.json`, as when it rebuilds a lost record, and they keep its path until they leave the file; `teamai doctor` checks the same way. So does a config a pull writes a tool's first record for while `managed-mcp.json` holds none for that tool (lost, or teamai's first delivery to it). A path git cannot say it ignores is listed all the same once `git ls-files` shows the file untracked; when git cannot say that either, it counts as git failing. When it cannot — `.git/info` or the exclude file is not writable, another teamai command holds the exclude file past a short wait, git already tracks the file, a rule in your own git ignore files re-includes it (say `!/.mcp.json`; the warning names it), or git fails — it leaves that file as it was (an entry an earlier pull wrote stays), warns with the reason and the fix, and `teamai mcp list` and `teamai doctor` report the server as withheld from each tool a pull would write it to; make the file writable (or `git rm --cached` the tracked file, or remove the rule that re-includes it) and run `teamai pull` again. A tracked file is reported first, and listed nowhere. The committed `.gitignore` is left alone, a path git already ignores adds nothing, and a pull, `teamai mcp remove` and `teamai uninstall` remove a path from the block (the block with its last path) once that file is gone, holds no MCP server, or holds none of: a team server with a resolved value, an entry of teamai's that cleanup left, a server that was in the file when teamai rebuilt a lost `managed-mcp.json`, or the value (8+ characters) of a variable still set in the environment, with teamai's record of what it wrote there (`managed-mcp.json`) present before the command ran, readable, and holding an entry for that file's tool (for a file two tools map, such as Claude and CodeBuddy on `.mcp.json`: for each tool `managed-mcp-files.json` says wrote a resolved value there, or for each tool mapping it when it names none; an empty, unreadable or truncated record proves nothing, and neither does one written by a pull that rebuilt it or found no record for its tool in `managed-mcp.json`, while that pull could not note the file's other servers in `managed-mcp-files.json`, until a later pull notes them). A file written under a mapping since changed, one at the built-in location of a tool the team dropped or moved (unless another tool maps it today), or one in a linked worktree of a nested repository, needs to be gone or hold no MCP server. One written for a tool the team has since moved elsewhere (recorded, found in that history, or at the tool's built-in location), that another tool's mapping still reaches, also keeps its path while it holds a server the tools now mapping it did not write (by their `managed-mcp.json` record); as in any file under a changed mapping, a server of your own there keeps it too. `teamai uninstall` applies that to the file in every worktree of the repository; a pull and `teamai mcp remove` apply it only to the current worktree's file, and keep the path while the file in any other worktree still holds an MCP server: an entry that worktree's last pull wrote (say, a `${VAR}` the team has since made a literal) is judged only by a pull there. A path a pull listed and then wrote no value into (the file does not parse, or holds a server of your own under the team's name) comes out again at the end of that pull, and so does its record in `managed-mcp-files.json`. Otherwise, or for a file it cannot check (for example one that does not parse), the path stays, and `teamai uninstall` warns, naming the file and why: remove teamai's servers from it, then delete that line yourself (with its last line, the block's markers). `teamai doctor` reports such a file git would still commit or cannot answer for — for example one already tracked: `git rm --cached` it and rotate the token. A Copilot project config whose servers sit bare at the top level has those counted, and teamai's among them removed once the team drops them, after another tool writes `mcpServers` into the same file too. For an HTTP-backed team (`teamai init --http`) no pull writes a server: the local agent's `install_mcp` does, with the values themselves rather than `${VAR}` references, so a project-scope server carrying any header, env value or argument, a URL (a token can sit in its path), or a command line with arguments counts as holding a credential; only a bare stdio command doesn't. Its install lists the file first and records it in `managed-mcp-files.json`, and when it cannot (the same causes as above), writes nothing and reports the install as failed with the reason. A file an older local agent wrote a credential into without listing it is listed and recorded, judged as `teamai doctor` judges it below, by the local agent's next sync in that workspace (its hooks run one in each session) and by a `teamai pull` there; a dry run writes nothing. No command but `teamai uninstall` takes such a line out. `teamai doctor` checks those files by the local agent's records: one it noted as carrying a credential, or an older install's entry carrying one, and, with no record of the tool, a file `managed-mcp-files.json` lists while it holds any server; add a file it names to `.git/info/exclude` yourself, or `git rm --cached` it and rotate the token. Claude Code may show project `.mcp.json` servers as pending approval until you accept them once in an interactive session. diff --git a/docs/usage-guide.zh-CN.md b/docs/usage-guide.zh-CN.md index d75612246..1307e1408 100644 --- a/docs/usage-guide.zh-CN.md +++ b/docs/usage-guide.zh-CN.md @@ -1126,13 +1126,17 @@ CodeBuddy Code 的 [MCP 文档](https://www.codebuddy.cn/docs/cli/mcp) TeamAI 不会迁移或删除旧文件。Claude Code 也读取根目录的 `.mcp.json`， 因此两个工具共享该文件。 +TeamAI 仅在所有权记录证明已完成顶层写入且内容仍匹配时，才删除 `mcpServers` 旁的 Copilot 顶层条目。旧记录缺少位置证据时，即使内容与团队定义相同，也保留顶层条目。顶层所有权记录不授权修改 `mcpServers` 下的同名成员条目；更新跳过该冲突，移除时只清理受管理的顶层副本。缺少位置标记的记录只有在哈希匹配嵌套条目且不同时匹配顶层条目时，才能认领嵌套条目。完成的嵌套写入记录 `bare: false`；位置记录写入失败时，所有权仍未得到证明。HTTP 本地代理首次安装先只读检查 Git 保护，再保存临时所有权记录，随后添加排除规则和文件记录，最后写入凭据。初始所有权记录写入失败不会改变 Git 排除规则或 MCP 配置。 + +HTTP local-agent 更新 JSON MCP 配置时，先保留原有 ownership 记录，配置写入成功后才更新记录；如果随后保存记录失败，会恢复原配置。`uninstall_mcp` 先删除配置中的条目，再移除 ownership 记录：配置写入失败或无法读取时保留记录以便重试，manifest 写入失败时恢复条目。MCP reconcile 在保存 ownership 前失败时，会恢复本次已写入的所有配置，包括多个工具共用的文件。恢复本身也失败时，错误会同时说明两次失败及受影响的文件；修复配置与 ownership 记录后再重试。只要凭据仍在文件中，就继续保留 Git 排除保护。 + Copilot 使用原生 `mcpServers` 结构：`stdio` 写成 `type: "local"`，远程传输保留 `http` 或 `sse`，每个 TeamAI 管理的条目都会带上必需的 `tools: ["*"]` 允许列表。TeamAI 遵循 `COPILOT_HOME`，项目配置使用 Copilot CLI 官方文档指定的 `.github/mcp.json` 仓库路径。详见 [GitHub Copilot CLI 添加 MCP Server](https://docs.github.com/zh/copilot/how-tos/copilot-cli/customize-copilot/add-mcp-servers)。Codex 支持 `stdio` 与 `http`，`sse` 会被跳过。Qoder 使用对应作用域 `.qoder/settings.json` 中与 Claude 兼容的 `mcpServers` 格式。Kiro 在专用的、只含 `mcpServers` 的 `.kiro/settings/mcp.json` 中使用同一格式（见 [Kiro MCP 配置文档](https://kiro.dev/docs/mcp/configuration/)）。OpenCode 支持 `stdio`（写成其 `type:"local"` 形态）与 `http`（`type:"remote"`），`sse` 会被跳过，其 server 位于共享 `opencode.json` 的 `mcp` 键下。归属记录在 `~/.teamai/managed-mcp.json`——手动添加的 server 不动；与手写同名则跳过，除非 `--force`。 **密钥**：在 `mcp.yaml` 里写 `${VAR}`，不要写明文。团队在 `env/secrets.yaml` 中声明的 key 优先取你为该团队设置的值（`teamai env set`），其次取你为本机设置的值（`teamai env set --global`），再次取你自己的环境，不包括 teamai `env.sh` 导出的值（见[团队密钥](designs/team-secrets.zh-CN.md#解析顺序)）。其他变量优先取你为该团队设置的值（`teamai env set KEY`），其次是该目录收到的团队环境变量（`env/env.yaml` 与活动的 `env//env.yaml`）；环境只补充团队没有设置的 key，不再覆盖团队变量（见[团队密钥](designs/team-secrets.zh-CN.md#变量)）。你导出的值与团队的值不同而被忽略时，交互式 `pull` 和 `teamai doctor` 会指出。变量无法解析则跳过并提示。已声明的密钥不同：pull 找不到它时，之前某次 pull 写入的条目原样保留，因此里面可能是已经轮换掉的旧值，直到某次 pull 找到新值（见[团队密钥](designs/team-secrets.zh-CN.md#缺少密钥时保留-mcp-条目)）。交互式 `pull`、`teamai mcp list`、`teamai env list`、`teamai doctor` 和 `teamai env exec` 会指出没有值的已声明密钥、用到它的 server 以及设置它的命令：`` github: GITHUB_TOKEN is not set. Run `teamai env set GITHUB_TOKEN` (). `` teamai 会**把每个 `${VAR}` 解析成取值后原样写入**各工具的配置文件，并以 `0600` 写入该文件，已有的 `0644` 文件也会收紧（不含已解析值的配置保持原权限；新建文件权限为 `0600`）。它不依赖任何工具自身的环境变量展开——因为那种展开很脆弱：最典型的是，以 GUI 方式（Dock/Launchpad）启动的 IDE 不会继承你 shell 中 `export` 的变量，`${VAR}` 占位符会展开为空、导致服务端 401。解析成明文可以保证无论工具如何启动，token 都在。 -> ⚠️ **解析后的 token 会落盘。** 项目级 MCP 配置（`.mcp.json`、`.github/mcp.json`、`.cursor/mcp.json`、`.codex/config.toml`、`opencode.json`）因此含有明文密钥。只要这类文件将含有 teamai 解析出的值且 git 会跟踪它，teamai 就会在写入该值之前把路径写入本地克隆的 `.git/info/exclude`，放在 `# [teamai:mcp-exclude:start]` 块中（同一仓库的各 worktree 共用该文件）。经由符号链接目录访问的配置（例如 `.cursor/` 指向 `config/`）按写入实际落到的位置判断：写入 exclude、检查和报告的都是该路径（`/config/mcp.json`），已被跟踪时会同时给出两个路径。文件本身是符号链接时，写入会替换该链接，因此以文件自身的路径为准。本次 pull 未写入的文件同样适用：之前为某个现已禁用的工具写入的文件，团队已从 `toolPaths` 移除或改到别处的工具的内置位置上的文件（只要含有任何 MCP server 就算数，因为 teamai 对该工具的记录描述的是另一个文件或没有文件；当前由另一个工具映射的文件，例如 Claude 映射的 CodeBuddy 的 `.mcp.json`，则在含有该工具未写入的 server 时算数，见下文），在团队此后改动的 `toolPaths` 映射下写入的文件（每个 worktree 会把写入过解析值的文件记录在其 `managed-mcp.json` 旁的 `managed-mcp-files.json` 中；对于旧版 teamai 在有这份记录之前写入的文件，第一次 pull 会读取一次团队仓库中 `teamai.yaml` 历史里的每个 `mcpProject` 路径，以及 teamai 此后改掉的内置路径（CodeBuddy 的 `.codebuddy/mcp.json`），以克隆中现有的历史为限，且只看项目内的文件，跳过同一工具当前仍映射的路径；这类文件只要含有任何 MCP server 就算数，因为 teamai 对该工具的记录只描述当前路径（当前由另一个工具映射的文件，则在含有该工具未写入的 server 时算数，见下文），在那次 pull 之前 `teamai doctor` 也会检查这些文件；被 git 跟踪的文件不会写入 exclude（写入也不起作用），但无论其内容如何都会记为已跟踪，待 git 不再跟踪它（`git rm --cached`）后按其他此类文件的规则判断，直到它从磁盘和 git 中都消失才会被遗忘），或仍含已从 `mcp.yaml` 删除的 server 的文件。pull 写入的带解析值的条目只要未被改动就一直算数，即使团队后来把其中的 `${VAR}` 改成了字面值。worktree 中完全没有 `managed-mcp.json` 时（记录丢失，或在其第一次 pull 之前），未被 git 跟踪的配置只要含有任何记录都未认领的 server 就算数，你自己的 server 也包括在内：pull 会像重建丢失的记录时那样把这些 server 记入 `managed-mcp-files.json`，在它们离开该文件之前该路径一直保留；`teamai doctor` 也按同样方式检查。`managed-mcp.json` 中没有某个工具的记录时（记录丢失，或这是 teamai 对该工具的第一次投递），pull 为该工具写入第一份记录的配置也按此处理。git 无法判断是否忽略的路径，只要 `git ls-files` 显示该文件未被跟踪，也会照样写入；若连这一点也无法判断，则按 git 出错处理。若无法写入——`.git/info` 或 exclude 文件不可写、另一个 teamai 命令在短暂等待后仍占用 exclude 文件、git 已跟踪该文件、你自己的 git 忽略文件中有规则重新包含了它（例如 `!/.mcp.json`；警告会指出该规则），或 git 出错——teamai 会保持该文件原样（之前 pull 写入的条目保留），给出原因与修复方法的警告，`teamai mcp list` 和 `teamai doctor` 也会针对 pull 会写入它的每个工具，把该 server 报告为未写入（withheld）；请让文件可写（或对已跟踪的文件执行 `git rm --cached`，或删除重新包含它的规则），再运行 `teamai pull`。已被跟踪的文件会优先报告，且不会写入任何路径。不会改动已提交的 `.gitignore`，git 已忽略的路径不会重复添加，pull、`teamai mcp remove` 和 `teamai uninstall` 会从块中移除某个路径（移除最后一个路径时连同整个块），前提是该文件已不存在、不含任何 MCP server，或在命令运行前 teamai 的写入记录（`managed-mcp.json`）就已存在、可以解析且记有该文件所属工具的条目的情况下（对于两个工具共用的文件，例如 Claude 和 CodeBuddy 共用的 `.mcp.json`：需记有 `managed-mcp-files.json` 中写入过解析值的每个工具的条目；若其中没有列出任何工具，则需记有映射到它的每个工具的条目；空的、无法读取或被截断的记录不能作为依据；pull 重建记录时、或在 `managed-mcp.json` 中没有该工具的记录时写入记录时，若无法把文件中的其他 server 记入 `managed-mcp-files.json`，该记录在之后某次 pull 记下它们之前也不能作为依据）不含以下任何一项：带解析值的团队 server、清理后仍残留的 teamai 条目、teamai 重建丢失的 `managed-mcp.json` 时文件中已有的 server、仍在环境中设置的变量的值（8 个字符以上）。在已改动的映射下写入的文件、团队已移除或改到别处的工具的内置位置上的文件（当前有另一个工具映射到它的除外），或位于嵌套仓库某个关联 worktree 中的文件，须已不存在或不含任何 MCP server。为团队此后改到别处的工具写入（有记录、在上述历史中找到，或位于该工具的内置位置）、但仍被另一个工具的映射指向的文件，只要含有当前映射到它的工具未写入的 server（以它们的 `managed-mcp.json` 记录为准），也会保留该路径；与其他在已改动映射下写入的文件一样，你自己的 server 也会让它保留。`teamai uninstall` 对仓库每个 worktree 中的该文件都按此判断；pull 和 `teamai mcp remove` 只对当前 worktree 的文件按此判断，只要其他任一 worktree 中的该文件仍含 MCP server，就保留该路径：那个 worktree 上次 pull 写入的条目（例如团队后来改成字面值的 `${VAR}`）只能由在那里运行的 pull 判断。某次 pull 写入了路径、随后却没有把值写进该文件（文件无法解析，或其中有你自己的同名 server）时，该路径会在这次 pull 结束时移除，它在 `managed-mcp-files.json` 中的记录也会一并移除。否则，或对无法检查的文件（例如无法解析），会保留该路径，`teamai uninstall` 会给出警告，说明文件及原因：请先从中移除 teamai 的 server，再自行删除那一行（删到最后一行时连同块的首尾标记）。`teamai doctor` 会报告 git 仍会提交或无法判断的这类文件——例如已被跟踪的文件：请 `git rm --cached` 并轮换 token。 +> ⚠️ **解析后的 token 会落盘。** 项目级 MCP 配置（`.mcp.json`、`.github/mcp.json`、`.cursor/mcp.json`、`.codex/config.toml`、`opencode.json`）因此含有明文密钥。只要这类文件将含有 teamai 解析出的值且 git 会跟踪它，teamai 就会在写入该值之前把路径写入本地克隆的 `.git/info/exclude`，放在 `# [teamai:mcp-exclude:start]` 块中（同一仓库的各 worktree 共用该文件）。经由符号链接目录访问的配置（例如 `.cursor/` 指向 `config/`）按写入实际落到的位置判断：写入 exclude、检查和报告的都是该路径（`/config/mcp.json`），已被跟踪时会同时给出两个路径。文件本身是符号链接时，写入会替换该链接，因此以文件自身的路径为准。本次 pull 未写入的文件同样适用：之前为某个现已禁用的工具写入的文件，团队已从 `toolPaths` 移除或改到别处的工具的内置位置上的文件（只要含有任何 MCP server 就算数，因为 teamai 对该工具的记录描述的是另一个文件或没有文件；当前由另一个工具映射的文件，例如 Claude 映射的 CodeBuddy 的 `.mcp.json`，则在含有该工具未写入的 server 时算数，见下文），在团队此后改动的 `toolPaths` 映射下写入的文件（每个 worktree 会把写入过解析值的文件记录在其 `managed-mcp.json` 旁的 `managed-mcp-files.json` 中；对于旧版 teamai 在有这份记录之前写入的文件，第一次 pull 会读取一次团队仓库中 `teamai.yaml` 历史里的每个 `mcpProject` 路径，以及 teamai 此后改掉的内置路径（CodeBuddy 的 `.codebuddy/mcp.json`），以克隆中现有的历史为限，且只看项目内的文件，跳过同一工具当前仍映射的路径；这类文件只要含有任何 MCP server 就算数，因为 teamai 对该工具的记录只描述当前路径（当前由另一个工具映射的文件，则在含有该工具未写入的 server 时算数，见下文），在那次 pull 之前 `teamai doctor` 也会检查这些文件；被 git 跟踪的文件不会写入 exclude（写入也不起作用），但无论其内容如何都会记为已跟踪，待 git 不再跟踪它（`git rm --cached`）后按其他此类文件的规则判断，直到它从磁盘和 git 中都消失才会被遗忘），或仍含已从 `mcp.yaml` 删除的 server 的文件。pull 写入的带解析值的条目只要未被改动就一直算数，即使团队后来把其中的 `${VAR}` 改成了字面值。worktree 中完全没有 `managed-mcp.json` 时（记录丢失，或在其第一次 pull 之前），未被 git 跟踪的配置只要含有任何记录都未认领的 server 就算数，你自己的 server 也包括在内：pull 会像重建丢失的记录时那样把这些 server 记入 `managed-mcp-files.json`，在它们离开该文件之前该路径一直保留；`teamai doctor` 也按同样方式检查。`managed-mcp.json` 中没有某个工具的记录时（记录丢失，或这是 teamai 对该工具的第一次投递），pull 为该工具写入第一份记录的配置也按此处理。git 无法判断是否忽略的路径，只要 `git ls-files` 显示该文件未被跟踪，也会照样写入；若连这一点也无法判断，则按 git 出错处理。若无法写入——`.git/info` 或 exclude 文件不可写、另一个 teamai 命令在短暂等待后仍占用 exclude 文件、git 已跟踪该文件、你自己的 git 忽略文件中有规则重新包含了它（例如 `!/.mcp.json`；警告会指出该规则），或 git 出错——teamai 会保持该文件原样（之前 pull 写入的条目保留），给出原因与修复方法的警告，`teamai mcp list` 和 `teamai doctor` 也会针对 pull 会写入它的每个工具，把该 server 报告为未写入（withheld）；请让文件可写（或对已跟踪的文件执行 `git rm --cached`，或删除重新包含它的规则），再运行 `teamai pull`。已被跟踪的文件会优先报告，且不会写入任何路径。不会改动已提交的 `.gitignore`，git 已忽略的路径不会重复添加，pull、`teamai mcp remove` 和 `teamai uninstall` 会从块中移除某个路径（移除最后一个路径时连同整个块），前提是该文件已不存在、不含任何 MCP server，或在命令运行前 teamai 的写入记录（`managed-mcp.json`）就已存在、可以解析且记有该文件所属工具的条目的情况下（对于两个工具共用的文件，例如 Claude 和 CodeBuddy 共用的 `.mcp.json`：需记有 `managed-mcp-files.json` 中写入过解析值的每个工具的条目；若其中没有列出任何工具，则需记有映射到它的每个工具的条目；空的、无法读取或被截断的记录不能作为依据；pull 重建记录时、或在 `managed-mcp.json` 中没有该工具的记录时写入记录时，若无法把文件中的其他 server 记入 `managed-mcp-files.json`，该记录在之后某次 pull 记下它们之前也不能作为依据）不含以下任何一项：带解析值的团队 server、清理后仍残留的 teamai 条目、teamai 重建丢失的 `managed-mcp.json` 时文件中已有的 server、仍在环境中设置的变量的值（8 个字符以上）。在已改动的映射下写入的文件、团队已移除或改到别处的工具的内置位置上的文件（当前有另一个工具映射到它的除外），或位于嵌套仓库某个关联 worktree 中的文件，须已不存在或不含任何 MCP server。为团队此后改到别处的工具写入（有记录、在上述历史中找到，或位于该工具的内置位置）、但仍被另一个工具的映射指向的文件，只要含有当前映射到它的工具未写入的 server（以它们的 `managed-mcp.json` 记录为准），也会保留该路径；与其他在已改动映射下写入的文件一样，你自己的 server 也会让它保留。`teamai uninstall` 对仓库每个 worktree 中的该文件都按此判断；pull 和 `teamai mcp remove` 只对当前 worktree 的文件按此判断，只要其他任一 worktree 中的该文件仍含 MCP server，就保留该路径：那个 worktree 上次 pull 写入的条目（例如团队后来改成字面值的 `${VAR}`）只能由在那里运行的 pull 判断。某次 pull 写入了路径、随后却没有把值写进该文件（文件无法解析，或其中有你自己的同名 server）时，该路径会在这次 pull 结束时移除，它在 `managed-mcp-files.json` 中的记录也会一并移除。否则，或对无法检查的文件（例如无法解析），会保留该路径，`teamai uninstall` 会给出警告，说明文件及原因：请先从中移除 teamai 的 server，再自行删除那一行（删到最后一行时连同块的首尾标记）。`teamai doctor` 会报告 git 仍会提交或无法判断的这类文件——例如已被跟踪的文件：请 `git rm --cached` 并轮换 token。Copilot 项目级配置中直接写在顶层（bare）的 server，在另一个工具也向同一文件写入 `mcpServers` 之后同样算数；其中属于 teamai 的，在团队删除它们后会被移除。对于 HTTP 模式的团队（`teamai init --http`），server 不由 pull 写入，而由本地 agent 的 `install_mcp` 写入，写入的是值本身而非 `${VAR}` 引用，因此项目级 server 只要带有任何 header、env 值或参数、URL（token 可能就在路径里），或带参数的命令行，就视为含有凭据；只有不带参数的 stdio 命令不算。安装时会先把该文件写入 exclude 并记入 `managed-mcp-files.json`；若无法写入（原因同上），则不写入任何内容，并把本次安装报告为失败，附上原因。旧版本地 agent 写入了凭据却未写入 exclude 的文件，会由本地 agent 在该工作区的下一次同步（其 hook 在每个会话中都会运行一次）以及在那里运行的 `teamai pull` 写入 exclude 并记入 `managed-mcp-files.json`，判断方式与下文 `teamai doctor` 相同；dry run 不写入任何内容。除 `teamai uninstall` 外，没有命令会移除这样的路径。`teamai doctor` 按本地 agent 的记录检查这些文件：记录为带有凭据的 server，或旧版安装写入的带凭据的条目；没有该工具的记录时，`managed-mcp-files.json` 列出的文件只要还有 server 也算；对它指出的文件，请自行把路径加入 `.git/info/exclude`，或 `git rm --cached` 并轮换 token。 Claude Code 可能把来自仓库的 `.mcp.json` 标为待批准，需在交互式会话中确认一次。 diff --git a/skill-data/core/references/troubleshooting.md b/skill-data/core/references/troubleshooting.md index 6fb169cc1..790b52f1e 100644 --- a/skill-data/core/references/troubleshooting.md +++ b/skill-data/core/references/troubleshooting.md @@ -94,6 +94,8 @@ ever committed with it; then `teamai pull`. Do not run `git rm` or commit for them. For an exclude file that is not writable, one another teamai command held, or a git error, relay the fix the line gives. +For new HTTP local-agent MCP installs, a failed initial ownership-manifest write leaves the MCP config and Git exclusions unchanged. Retry the install after fixing the manifest write error. A bare Copilot entry beside `mcpServers` is removed only with a matching ownership record proving a completed bare write. Older records without that evidence preserve the bare entry. A bare ownership record cannot claim a same-named member entry under `mcpServers`: updates skip the collision, and removal leaves that keyed entry alone. An unmarked Copilot record needs a matching keyed hash that does not also match the bare entry. Completed writes record `bare: true` or `bare: false`; missing placement remains unproven, including after a failed placement-record write. Existing JSON MCP updates keep the old ownership until the config write completes; a later manifest failure restores the config. `uninstall_mcp` keeps ownership if reading or writing the config fails, and restores the entry if removing its manifest record fails. MCP reconcile also restores all configs written before an ownership-save failure. If restoration fails too, repair the named configs and ownership records before retrying; the error reports both failures, and configs still carrying credentials stay excluded from Git. + ## Permission / access denied `init`, `pull`, or `push` failing with a permission error usually means the user diff --git a/skill-data/setup/references/manage-admin.md b/skill-data/setup/references/manage-admin.md index 50c04a8f9..dd3490816 100644 --- a/skill-data/setup/references/manage-admin.md +++ b/skill-data/setup/references/manage-admin.md @@ -71,7 +71,12 @@ and that server noted: it keeps the line until it leaves the file. So is the file of a tool `managed-mcp.json` has no record for, when a pull writes that tool's first record (its record lost, or teamai's first delivery to it). While that note cannot be written (another teamai command holds the record), the line stays -until a later pull writes it. +until a later pull writes it. A Copilot project config's bare top-level servers +still count once another tool writes `mcpServers` into the file. On an HTTP-backed +team the local agent's `install_mcp` lists a project config before writing a +server with any header, env value, argument or URL (only a bare stdio command is not), fails the install when it cannot, and only +`teamai uninstall` takes that line out. The next sync or `teamai pull` in the +workspace also lists a file an older local agent wrote a credential into; `teamai doctor` checks those files too. ## Invite a member diff --git a/src/__tests__/doctor-mcp-delivery.test.ts b/src/__tests__/doctor-mcp-delivery.test.ts index 9cc9da8c4..12c827b6b 100644 --- a/src/__tests__/doctor-mcp-delivery.test.ts +++ b/src/__tests__/doctor-mcp-delivery.test.ts @@ -348,6 +348,135 @@ describe('doctor — MCP servers delivered on disk', () => { expect(check.fix).not.toContain(path.join(projectRoot, '.mcp.json')); }); + it('fails for a moved tool\'s file while the tool mapping it today owns that server name only under another key', async () => { + const { trackResolvedMcpFiles } = await import('../mcp-resolved-files.js'); + teamConfig.toolPaths = { ...teamConfig.toolPaths, opencode: { skills: '.opencode/skills', mcp: '.config/opencode/opencode.json', mcpProject: 'shared/mcp.json' } }; + const shared = path.join(projectRoot, 'shared', 'mcp.json'); + await fse.outputJson(shared, { + mcpServers: { x: { type: 'http', url: 'https://x.example/mcp', headers: { Authorization: 'Bearer t0ken-of-cursor' } } }, + mcp: { x: { type: 'remote', url: 'https://x.example/mcp' } }, + }); + expect(await trackResolvedMcpFiles(localConfig, [{ tool: 'cursor', file: shared }])).toBe('written'); + await fse.outputJson(managedMcpManifestPath(getDataHome(localConfig), projectRoot), { + [managedMcpManifestKey('opencode', true)]: [{ name: 'x', hash: 'fixture-hash', resolved: false }], + }); + await fse.appendFile(path.join(projectRoot, '.git', 'info', 'exclude'), '/.mcp.json\n'); + + const check = await excludeCheck(); + if (!check) throw new Error('no git exclude check'); + expect(await check.check()).toBe(false); + expect(check.fix ?? '').toContain(shared); + }); + + it('fails for a moved Copilot\'s file while the tool mapping it today owns that server name only under mcpServers', async () => { + const { trackResolvedMcpFiles } = await import('../mcp-resolved-files.js'); + teamConfig.toolPaths = { + ...teamConfig.toolPaths, + cursor: { skills: '.cursor/skills', mcp: '.cursor/mcp.json', mcpProject: 'shared/mcp.json' }, + copilot: { skills: '.github/skills', mcp: '.copilot/mcp-config.json', mcpProject: '.github/mcp.json' }, + }; + const shared = path.join(projectRoot, 'shared', 'mcp.json'); + await fse.outputJson(shared, { + x: { type: 'http', url: 'https://x.example/mcp', headers: { Authorization: 'Bearer t0ken-of-copilot' } }, + mcpServers: { x: { type: 'http', url: 'https://x.example/mcp' } }, + }); + expect(await trackResolvedMcpFiles(localConfig, [{ tool: 'copilot', file: shared }])).toBe('written'); + await fse.outputJson(managedMcpManifestPath(getDataHome(localConfig), projectRoot), { + [managedMcpManifestKey('cursor', true)]: [{ name: 'x', hash: 'fixture-hash', resolved: false }], + // Copilot's record describes the file its mapping reaches today, not this one. + [managedMcpManifestKey('copilot', true)]: [], + }); + // Cursor's x is still the team's, now a literal: its own rules find nothing to keep. + await writeTeamMcp('servers:\n - name: x\n transport: http\n url: https://x.example/mcp\n'); + await fse.appendFile(path.join(projectRoot, '.git', 'info', 'exclude'), '/.mcp.json\n'); + + const check = await excludeCheck(); + if (!check) throw new Error('no git exclude check'); + expect(await check.check()).toBe(false); + expect(check.fix ?? '').toContain(shared); + }); + + describe('for an HTTP-backed team, judged by the records the local agent wrote', () => { + const writeRecord = (record: Record): Promise => + fse.outputJson(managedMcpManifestPath(getDataHome(localConfig), projectRoot), { [managedMcpManifestKey('claude', true)]: [record] }); + + beforeEach(async () => { + Object.assign(localConfig, { repo: { localPath: repoPath, remote: 'https://teamai.example', kind: 'http', url: 'https://teamai.example' } }); + // An HTTP team has no mcp.yaml: its servers arrive through install_mcp. + await fse.remove(path.join(repoPath, 'mcp')); + }); + + it.each([ + ['notes it carried a header or env value', { name: 'jira', hash: 'h', resolved: true }], + ['is an older local agent\'s, without that note, and its entry holds a header', { name: 'jira', hash: 'h' }], + ])('fails while git would track the file, and names it, when the record %s', async (_label, record) => { + await writeRecord(record); + + const check = await excludeCheck(); + if (!check) throw new Error('no git exclude check'); + expect(await check.check()).toBe(false); + expect(check.fix).toContain(path.join(projectRoot, '.mcp.json')); + // No pull writes an HTTP team's servers, so none lists the file. + expect(check.fix).not.toContain('teamai pull'); + }); + + it('passes once git ignores the file', async () => { + await writeRecord({ name: 'jira', hash: 'h', resolved: true }); + await fse.appendFile(path.join(projectRoot, '.git', 'info', 'exclude'), '/.mcp.json\n'); + + const check = await excludeCheck(); + if (!check) throw new Error('no git exclude check'); + expect(await check.check()).toBe(true); + }); + + it('still fails for a file managed-mcp-files.json lists, holding a server, while the manifest has no record for it', async () => { + const { trackResolvedMcpFiles } = await import('../mcp-resolved-files.js'); + await fse.remove(managedMcpManifestPath(getDataHome(localConfig), projectRoot)); + expect(await trackResolvedMcpFiles(localConfig, [{ tool: 'claude', file: path.join(projectRoot, '.mcp.json') }])).toBe('written'); + + const check = await excludeCheck(); + if (!check) throw new Error('no git exclude check'); + expect(await check.check()).toBe(false); + expect(check.fix).toContain(path.join(projectRoot, '.mcp.json')); + }); + + it('still fails while a server carrying a credential has lost its record, though another server\'s remains', async () => { + await fse.writeJson(path.join(projectRoot, '.mcp.json'), { + mcpServers: { + jira: { type: 'http', url: 'https://jira.example/mcp', headers: { Authorization: 'Bearer t0ken' } }, + local: { command: 'local-mcp' }, + }, + }); + await writeRecord({ name: 'local', hash: 'h', resolved: false }); + + const check = await excludeCheck(); + if (!check) throw new Error('no git exclude check'); + expect(await check.check()).toBe(false); + }); + + it('fails for a credential a local agent from before 57636a27 wrote at CodeBuddy\'s former .codebuddy/mcp.json', async () => { + const old = path.join(projectRoot, '.codebuddy', 'mcp.json'); + await fse.outputJson(old, { mcpServers: { clawpro: { type: 'http', url: 'https://clawpro.example/mcp', headers: { Authorization: 'Bearer t0ken' } } } }); + await fse.outputJson(managedMcpManifestPath(getDataHome(localConfig), projectRoot), { + [managedMcpManifestKey('codebuddy', true)]: [{ name: 'clawpro', hash: 'h' }], + }); + + const check = await excludeCheck(); + if (!check) throw new Error('no git exclude check'); + expect(await check.check()).toBe(false); + expect(check.fix ?? '').toContain(old); + }); + + it('has nothing to say of a file whose recorded server carries neither header nor env value', async () => { + const jira = { type: 'http', url: 'https://jira.example/mcp' }; + await fse.writeJson(path.join(projectRoot, '.mcp.json'), { mcpServers: { jira } }); + const { entryHash } = await import('../resources/mcp-format.js'); + await writeRecord({ name: 'jira', hash: entryHash(jira), resolved: false }); + + expect(await excludeCheck()).toBeUndefined(); + }); + }); + describe('a config an older teamai wrote under a mapping an earlier teamai.yaml made, before a pull on this version', () => { const old = (): string => path.join(projectRoot, '.cursor', 'team-mcp.json'); const commitTeamYaml = (toolPaths: object): void => { diff --git a/src/__tests__/local-agent-mcp.test.ts b/src/__tests__/local-agent-mcp.test.ts index 39a797b65..86a910699 100644 --- a/src/__tests__/local-agent-mcp.test.ts +++ b/src/__tests__/local-agent-mcp.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import path from 'node:path'; import os from 'node:os'; import fse from 'fs-extra'; +import { execFileSync } from 'node:child_process'; vi.mock('../utils/logger.js', () => ({ log: { @@ -49,6 +50,7 @@ async function setupConfig(bindings: Record = {}): Promise, tool: string = 'codebuddy', + cwd: string = tmpDir, ): Promise>> { // 确保 tool 目录存在，使 isToolInstalled 检查通过 await fse.ensureDir(path.join(tmpDir, `.${tool}`, 'skills')); @@ -66,7 +68,7 @@ async function runResponse( }); vi.stubGlobal('fetch', fetchMock); const { reportAndSyncLocalAgent } = await import('../local-agent.js'); - await reportAndSyncLocalAgent({ cwd: tmpDir, tool, status: 'running' }); + await reportAndSyncLocalAgent({ cwd, tool, status: 'running' }); return acks; } @@ -159,6 +161,371 @@ describe('local-agent: MCP install/uninstall commands', () => { expect(document).toEqual({ 'user-server': userServer }); }); + it('uninstalls a bare Copilot project entry once another tool has written mcpServers into the file', async () => { + const workspacePath = path.join(tmpDir, 'copilot-shared-project'); + const configFile = path.join(workspacePath, '.github', 'mcp.json'); + const userServer = { type: 'http', url: 'https://user.example.com/mcp' }; + await fse.ensureDir(path.dirname(configFile)); + await fse.writeJson(configFile, { 'user-server': userServer }); + const command = { scope: 'workspace', workspace_path: workspacePath, slug: COPILOT_SERVER, version: '1.0.0' }; + let acks = await runResponse({ + cmds: [{ id: 8995, type: 'install_mcp', ...command, mcp_config: { transport: 'http', url: 'https://copilot.example.com/mcp' } }], + }, 'copilot'); + expect(acks[0].status).toBe('success'); + const other = { mcpServers: { claude: { type: 'http', url: 'https://claude.example.com/mcp' } } }; + await fse.writeJson(configFile, { ...await fse.readJson(configFile), ...other }); + + acks = await runResponse({ cmds: [{ id: 8996, type: 'uninstall_mcp', ...command }] }, 'copilot'); + + expect(acks[0].status).toBe('success'); + expect(await fse.readJson(configFile)).toEqual({ 'user-server': userServer, ...other }); + }); + + it('replaces a bare Copilot project entry it installs again once another tool has written mcpServers into the file', async () => { + const workspacePath = path.join(tmpDir, 'copilot-reinstall-project'); + const configFile = path.join(workspacePath, '.github', 'mcp.json'); + await fse.ensureDir(path.dirname(configFile)); + await fse.writeJson(configFile, {}); + const command = { scope: 'workspace', workspace_path: workspacePath, slug: COPILOT_SERVER }; + let acks = await runResponse({ + cmds: [{ id: 8997, type: 'install_mcp', ...command, version: '1.0.0', mcp_config: { transport: 'http', url: 'https://old.example.com/mcp' } }], + }, 'copilot'); + expect(acks[0].status).toBe('success'); + const other = { mcpServers: { claude: { type: 'http', url: 'https://claude.example.com/mcp' } } }; + await fse.writeJson(configFile, { ...await fse.readJson(configFile), ...other }); + + acks = await runResponse({ + cmds: [{ id: 8998, type: 'install_mcp', ...command, version: '1.0.1', mcp_config: { transport: 'http', url: 'https://new.example.com/mcp' } }], + }, 'copilot'); + + expect(acks[0].status).toBe('success'); + const doc = await fse.readJson(configFile) as Record; + expect(doc[COPILOT_SERVER]).toBeUndefined(); + expect(JSON.stringify(doc)).not.toContain('old.example.com'); + expect((doc.mcpServers as Record)[COPILOT_SERVER]).toEqual(expect.objectContaining({ url: 'https://new.example.com/mcp' })); + }); + + it('preserves an identical member-owned bare Copilot entry through install, update and uninstall', async () => { + const workspacePath = path.join(tmpDir, 'copilot-identical-member'); + const configFile = path.join(workspacePath, '.github', 'mcp.json'); + const mine = { type: 'http', tools: ['*'], url: 'https://copilot.example.com/mcp' }; + await fse.ensureDir(path.dirname(configFile)); + await fse.writeJson(configFile, { [COPILOT_SERVER]: mine, mcpServers: {} }); + for (const [id, type] of [[8995, 'install_mcp'], [8996, 'install_mcp'], [8997, 'uninstall_mcp']] as const) { + const acks = await runResponse({ cmds: [{ + id, type, scope: 'workspace', workspace_path: workspacePath, + slug: COPILOT_SERVER, version: '1.0.0', + mcp_config: { transport: 'http', url: mine.url }, + }] }, 'copilot'); + expect(acks[0].status).toBe('success'); + expect((await fse.readJson(configFile))[COPILOT_SERVER]).toEqual(mine); + } + expect((await fse.readJson(configFile)).mcpServers[COPILOT_SERVER]).toBeUndefined(); + }); + + it.each(['install_mcp', 'uninstall_mcp'] as const)('preserves a member-owned keyed Copilot entry after a bare install during %s', async (type) => { + const workspacePath = path.join(tmpDir, 'copilot-keyed-member'); + const configFile = path.join(workspacePath, '.github', 'mcp.json'); + await fse.ensureDir(path.dirname(configFile)); + await fse.writeFile(configFile, ''); + const command = { + id: 9010, type: 'install_mcp', scope: 'workspace', workspace_path: workspacePath, + slug: COPILOT_SERVER, version: '1.0.0', + mcp_config: { transport: 'http', url: 'https://team.example/mcp' }, + }; + expect((await runResponse({ cmds: [command] }, 'copilot'))[0].status).toBe('success'); + const doc = await fse.readJson(configFile); + const mine = { type: 'http', tools: ['*'], url: 'https://member.example/mcp' }; + await fse.writeJson(configFile, { ...doc, mcpServers: { [COPILOT_SERVER]: mine } }); + + const acks = await runResponse({ cmds: [{ ...command, id: 9011, type }] }, 'copilot'); + + expect((await fse.readJson(configFile)).mcpServers[COPILOT_SERVER]).toEqual(mine); + if (type === 'install_mcp') { + expect(acks[0].status).toBe('failed'); + expect(acks[0].error).toContain('not managed by teamai'); + expect((await fse.readJson(configFile))[COPILOT_SERVER]).toEqual(doc[COPILOT_SERVER]); + } else { + expect(acks[0].status).toBe('success'); + expect((await fse.readJson(configFile))[COPILOT_SERVER]).toBeUndefined(); + } + }); + + it.each([ + ['legacy', 'install_mcp', false], ['legacy', 'uninstall_mcp', false], + ['failed placement write', 'install_mcp', false], ['failed placement write', 'uninstall_mcp', false], + ['legacy', 'install_mcp', true], ['legacy', 'uninstall_mcp', true], + ['failed placement write', 'install_mcp', true], ['failed placement write', 'uninstall_mcp', true], + ] as const)('preserves a keyed member entry after an unmarked bare install from %s during %s, identical=%s', async (source, type, identical) => { + const workspacePath = path.join(tmpDir, 'copilot-unmarked-member'); + const configFile = path.join(workspacePath, '.github', 'mcp.json'); + await fse.ensureDir(path.dirname(configFile)); + await fse.writeFile(configFile, ''); + const command = { + id: 9020, type: 'install_mcp', scope: 'workspace', workspace_path: workspacePath, + slug: COPILOT_SERVER, version: '1.0.0', + mcp_config: { transport: 'http', url: 'https://team.example/mcp' }, + }; + const fs = await import('../utils/fs.js'); + const write = fs.writeJsonAtomic; + let writes = 0; + const spy = vi.spyOn(fs, 'writeJsonAtomic').mockImplementation(async (file, ...args) => { + if (source === 'failed placement write' && file.endsWith('/managed-mcp.json') && ++writes === 2) { + throw new Error('simulated placement write failure'); + } + return write(file, ...args); + }); + const installed = await runResponse({ cmds: [command] }, 'copilot'); + spy.mockRestore(); + expect(installed[0].status).toBe(source === 'legacy' ? 'success' : 'failed'); + if (source === 'failed placement write') expect(installed[0].error).toContain('simulated placement write failure'); + const wsDir = path.join(workspacePath, '.teamai', 'workspaces'); + const [id] = await fse.readdir(wsDir); + const manifestFile = path.join(wsDir, id, 'managed-mcp.json'); + const manifest = await fse.readJson(manifestFile); + if (source === 'legacy') { + delete manifest['copilot:project'][0].bare; + await fse.writeJson(manifestFile, manifest); + } + expect((await fse.readJson(manifestFile))['copilot:project'][0].bare).toBeUndefined(); + const doc = await fse.readJson(configFile); + const mine = identical ? doc[COPILOT_SERVER] : { type: 'http', tools: ['*'], url: 'https://member.example/mcp' }; + await fse.writeJson(configFile, { ...doc, mcpServers: { [COPILOT_SERVER]: mine } }); + + const acks = await runResponse({ cmds: [{ ...command, id: 9021, type }] }, 'copilot'); + + expect((await fse.readJson(configFile)).mcpServers[COPILOT_SERVER]).toEqual(mine); + expect((await fse.readJson(configFile))[COPILOT_SERVER]).toEqual(doc[COPILOT_SERVER]); + expect(acks[0].status).toBe(type === 'install_mcp' ? 'failed' : 'success'); + if (type === 'install_mcp') expect(acks[0].error).toContain('not managed by teamai'); + }); + + it('updates a legacy keyed Copilot entry with matching content and records keyed placement', async () => { + const workspacePath = path.join(tmpDir, 'copilot-legacy-keyed'); + const configFile = path.join(workspacePath, '.github', 'mcp.json'); + await fse.ensureDir(path.dirname(configFile)); + await fse.writeJson(configFile, { mcpServers: {} }); + const command = { + id: 9030, type: 'install_mcp', scope: 'workspace', workspace_path: workspacePath, + slug: COPILOT_SERVER, version: '1.0.0', + mcp_config: { transport: 'http', url: 'https://team.example/mcp' }, + }; + expect((await runResponse({ cmds: [command] }, 'copilot'))[0].status).toBe('success'); + const wsDir = path.join(workspacePath, '.teamai', 'workspaces'); + const [id] = await fse.readdir(wsDir); + const manifestFile = path.join(wsDir, id, 'managed-mcp.json'); + const manifest = await fse.readJson(manifestFile); + expect(manifest['copilot:project'][0].bare).toBe(false); + delete manifest['copilot:project'][0].bare; + await fse.writeJson(manifestFile, manifest); + + const updated = await runResponse({ cmds: [{ ...command, id: 9031, mcp_config: { transport: 'http', url: 'https://team.example/updated' } }] }, 'copilot'); + + expect(updated[0].status).toBe('success'); + expect((await fse.readJson(configFile)).mcpServers[COPILOT_SERVER].url).toBe('https://team.example/updated'); + expect((await fse.readJson(manifestFile))['copilot:project'][0].bare).toBe(false); + expect((await runResponse({ cmds: [{ ...command, id: 9032, type: 'uninstall_mcp' }] }, 'copilot'))[0].status).toBe('success'); + expect((await fse.readJson(configFile)).mcpServers[COPILOT_SERVER]).toBeUndefined(); + }); + + it.each((['proven bare', 'proven bare migration', 'legacy bare', 'proven keyed', 'legacy keyed'] as const) + .flatMap((source) => (['config', 'manifest'] as const) + .flatMap((failure) => (['retry', 'uninstall'] as const).map((next) => [source, failure, next] as const))))( + 'preserves ownership for %s after a %s write fails, then allows %s', async (source, failure, next) => { + const workspacePath = path.join(tmpDir, 'copilot-failed-update'); + const configFile = path.join(workspacePath, '.github', 'mcp.json'); + await fse.ensureDir(path.dirname(configFile)); + execFileSync('git', ['init', '-q'], { cwd: workspacePath }); + await fse.writeFile(configFile, source.includes('bare') ? '' : '{"mcpServers":{}}'); + const command = { + id: 9040, type: 'install_mcp', scope: 'workspace', workspace_path: workspacePath, + slug: COPILOT_SERVER, version: '1.0.0', + mcp_config: { transport: 'http', url: 'https://team.example/mcp', headers: { Authorization: 'Bearer fixture-old' } }, + }; + expect((await runResponse({ cmds: [command] }, 'copilot'))[0].status).toBe('success'); + const wsDir = path.join(workspacePath, '.teamai', 'workspaces'); + const [id] = await fse.readdir(wsDir); + const manifestFile = path.join(wsDir, id, 'managed-mcp.json'); + const before = await fse.readJson(manifestFile); + if (source.startsWith('legacy')) { + delete before['copilot:project'][0].bare; + await fse.writeJson(manifestFile, before); + } + if (source === 'proven bare migration') { + await fse.writeJson(configFile, { ...(await fse.readJson(configFile)), mcpServers: { mine: { command: 'member-server' } } }); + } + const original = await fse.readJson(configFile); + const replacement = { ...command, id: 9041, mcp_config: { transport: 'stdio', command: 'replacement-server' } }; + const fs = await import('../utils/fs.js'); + const write = fs.writeJsonAtomic; + let configWritten = false; + const spy = vi.spyOn(fs, 'writeJsonAtomic').mockImplementation(async (file, ...args) => { + if (failure === 'config' && file.endsWith('/.github/mcp.json')) throw new Error('simulated MCP config write failure'); + if (failure === 'manifest' && file.endsWith('/managed-mcp.json') && configWritten) throw new Error('simulated manifest write failure'); + await write(file, ...args); + if (file.endsWith('/.github/mcp.json')) configWritten = true; + }); + + const failed = await runResponse({ cmds: [replacement] }, 'copilot'); + spy.mockRestore(); + const afterFailure = await fse.readJson(manifestFile); + expect(failed[0].status).toBe('failed'); + expect(failed[0].error).toContain(failure === 'config' ? 'simulated MCP config write failure' : 'simulated manifest write failure'); + expect(await fse.readJson(configFile)).toEqual(original); + if (source === 'proven bare') { + await fse.writeJson(configFile, { ...original, mcpServers: { mine: { command: 'member-server' } } }); + } + + const resumed = await runResponse({ cmds: [{ ...replacement, id: 9042, type: next === 'retry' ? 'install_mcp' : 'uninstall_mcp' }] }, 'copilot'); + + expect(resumed[0].status).toBe('success'); + const after = await fse.readJson(configFile); + if (next === 'uninstall') { + expect(after[COPILOT_SERVER]).toBeUndefined(); + expect(after.mcpServers?.[COPILOT_SERVER]).toBeUndefined(); + } else { + expect(JSON.stringify(after)).not.toContain('fixture-old'); + expect(JSON.stringify(after)).toContain('replacement-server'); + if (source.startsWith('proven bare')) expect(after[COPILOT_SERVER]).toBeUndefined(); + } + if (source.startsWith('proven bare')) expect(after.mcpServers.mine).toEqual({ command: 'member-server' }); + expect(afterFailure).toEqual(before); + }); + + it.each((['copilot bare', 'copilot keyed', 'codebuddy keyed', 'codex user'] as const) + .flatMap((source) => (['config', 'manifest'] as const).map((failure) => [source, failure] as const)))( + 'allows retrying uninstall of %s after a %s write fails', async (source, failure) => { + const tool = source.split(' ')[0]; + const workspacePath = path.join(tmpDir, 'failed-uninstall'); + const configFile = source === 'codex user' ? path.join(tmpDir, '.codex', 'config.toml') + : path.join(workspacePath, tool === 'copilot' ? '.github/mcp.json' : '.mcp.json'); + await fse.ensureDir(path.dirname(configFile)); + if (tool !== 'codex') await fse.writeFile(configFile, source === 'copilot bare' ? '' : '{"mcpServers":{}}'); + const command = { + id: 9050, type: 'install_mcp', scope: tool === 'codex' ? 'user' : 'workspace', workspace_path: workspacePath, + slug: COPILOT_SERVER, version: '1.0.0', mcp_config: { transport: 'stdio', command: 'team-server' }, + }; + expect((await runResponse({ cmds: [command] }, tool))[0].status).toBe('success'); + const wsDir = path.join(workspacePath, '.teamai', 'workspaces'); + const manifestFile = tool === 'codex' ? path.join(tmpDir, '.teamai', 'managed-mcp.json') + : path.join(wsDir, (await fse.readdir(wsDir))[0], 'managed-mcp.json'); + const before = await fse.readJson(manifestFile); + const original = await fse.readFile(configFile, 'utf-8'); + const fs = await import('../utils/fs.js'); + const write = fs.writeJsonAtomic; + const spy = vi.spyOn(fs, 'writeJsonAtomic').mockImplementation(async (file, ...args) => { + if ((failure === 'config' && file.endsWith(tool === 'copilot' ? '/.github/mcp.json' : '/.mcp.json')) || (failure === 'manifest' && file.endsWith('/managed-mcp.json'))) { + throw new Error(`simulated ${failure} write failure`); + } + return write(file, ...args); + }); + const reconcile = await import('../mcp-reconcile.js'); + const codexWrite = reconcile.writeCodexAtomic; + const codexSpy = vi.spyOn(reconcile, 'writeCodexAtomic').mockImplementation(async (...args) => { + if (failure === 'config') throw new Error('simulated config write failure'); + return codexWrite(...args); + }); + const removal = { ...command, id: 9051, type: 'uninstall_mcp' }; + + const failed = await runResponse({ cmds: [removal] }, tool); + spy.mockRestore(); + codexSpy.mockRestore(); + + expect(failed[0].status).toBe('failed'); + expect(failed[0].error).toContain(`simulated ${failure} write failure`); + expect(await fse.readJson(manifestFile)).toEqual(before); + expect(await fse.readFile(configFile, 'utf-8')).toBe(original); + expect((await runResponse({ cmds: [{ ...removal, id: 9052 }] }, tool))[0].status).toBe('success'); + expect(await fse.readFile(configFile, 'utf-8')).not.toContain('team-server'); + }); + + it('keeps ownership when uninstall cannot parse the config, then removes the repaired entry', async () => { + const workspacePath = path.join(tmpDir, 'malformed-uninstall'); + const configFile = path.join(workspacePath, '.github', 'mcp.json'); + await fse.ensureDir(workspacePath); + const command = { + id: 9060, type: 'install_mcp', scope: 'workspace', workspace_path: workspacePath, + slug: COPILOT_SERVER, version: '1.0.0', mcp_config: { transport: 'stdio', command: 'team-server' }, + }; + expect((await runResponse({ cmds: [command] }, 'copilot'))[0].status).toBe('success'); + const wsDir = path.join(workspacePath, '.teamai', 'workspaces'); + const manifestFile = path.join(wsDir, (await fse.readdir(wsDir))[0], 'managed-mcp.json'); + const before = await fse.readJson(manifestFile); + const original = await fse.readFile(configFile, 'utf-8'); + await fse.writeFile(configFile, '{invalid config'); + const removal = { ...command, id: 9061, type: 'uninstall_mcp' }; + + const failed = await runResponse({ cmds: [removal] }, 'copilot'); + + expect(failed[0].status).toBe('failed'); + expect(failed[0].error).toContain('cannot parse'); + expect(await fse.readJson(manifestFile)).toEqual(before); + expect(await fse.readFile(configFile, 'utf-8')).toBe('{invalid config'); + await fse.writeFile(configFile, original); + expect((await runResponse({ cmds: [{ ...removal, id: 9062 }] }, 'copilot'))[0].status).toBe('success'); + expect(await fse.readFile(configFile, 'utf-8')).not.toContain('team-server'); + }); + + it.each(['install_mcp', 'uninstall_mcp'] as const)( + 'keeps Codex config and ownership when %s cannot read the config', async (type) => { + const configFile = path.join(tmpDir, '.codex', 'config.toml'); + const command = { + id: 9070, type: 'install_mcp', scope: 'user', slug: COPILOT_SERVER, version: '1.0.0', + mcp_config: { transport: 'stdio', command: 'team-server' }, + }; + expect((await runResponse({ cmds: [command] }, 'codex'))[0].status).toBe('success'); + const manifestFile = path.join(tmpDir, '.teamai', 'managed-mcp.json'); + const before = await fse.readJson(manifestFile); + const original = await fse.readFile(configFile, 'utf-8'); + const read = fse.readFile; + const spy = vi.spyOn(fse, 'readFile').mockImplementation((file, ...args) => { + if (String(file).endsWith('/.codex/config.toml')) return Promise.reject(new Error('simulated config read failure')); + return read(file, ...args); + }); + + const failed = await runResponse({ cmds: [{ ...command, id: 9071, type }] }, 'codex'); + spy.mockRestore(); + + expect(failed[0].status).toBe('failed'); + expect(failed[0].error).toContain('simulated config read failure'); + expect(await fse.readJson(manifestFile)).toEqual(before); + expect(await fse.readFile(configFile, 'utf-8')).toBe(original); + expect((await runResponse({ cmds: [{ ...command, id: 9072, type }] }, 'codex'))[0].status).toBe('success'); + }); + + it.each(['install_mcp', 'uninstall_mcp'] as const)( + 'reports both failures when %s cannot restore a config after its manifest write fails', async (type) => { + const workspacePath = path.join(tmpDir, 'failed-restoration'); + const configFile = path.join(workspacePath, '.github', 'mcp.json'); + await fse.ensureDir(workspacePath); + const command = { + id: 9080, type: 'install_mcp', scope: 'workspace', workspace_path: workspacePath, + slug: COPILOT_SERVER, version: '1.0.0', mcp_config: { transport: 'stdio', command: 'team-server' }, + }; + expect((await runResponse({ cmds: [command] }, 'copilot'))[0].status).toBe('success'); + const original = await fse.readJson(configFile); + const fs = await import('../utils/fs.js'); + const write = fs.writeJsonAtomic; + let configWritten = false; + const spy = vi.spyOn(fs, 'writeJsonAtomic').mockImplementation(async (file, ...args) => { + if (file.endsWith('/managed-mcp.json')) throw new Error('simulated manifest write failure'); + if (file.endsWith('/.github/mcp.json') && configWritten) throw new Error('simulated restoration failure'); + await write(file, ...args); + if (file.endsWith('/.github/mcp.json')) configWritten = true; + }); + const retry = { ...command, id: 9081, type, mcp_config: { transport: 'stdio', command: 'replacement-server' } }; + + const failed = await runResponse({ cmds: [retry] }, 'copilot'); + spy.mockRestore(); + + expect(failed[0].status).toBe('failed'); + expect(failed[0].error).toContain('simulated manifest write failure'); + expect(failed[0].error).toContain('simulated restoration failure'); + expect(failed[0].error).toContain('The config may not match'); + await fse.writeJson(configFile, original); + expect((await runResponse({ cmds: [{ ...retry, id: 9082 }] }, 'copilot'))[0].status).toBe('success'); + }); + it('rejects an unmanaged collision in a bare Copilot project map', async () => { const workspacePath = path.join(tmpDir, 'copilot-collision-project'); const configFile = path.join(workspacePath, '.github', 'mcp.json'); @@ -511,6 +878,215 @@ describe('local-agent: MCP install/uninstall commands', () => { ); }); + // A project-scope install carrying a credential lands only in a file git leaves out of a commit (#882). + describe('a workspace install carrying a header or env value, in a git checkout (#882)', () => { + let wsPath: string; + const git = (...args: string[]): string => execFileSync('git', args, { cwd: wsPath, encoding: 'utf-8' }); + const install = (id: number, mcpConfig: Record) => runResponse({ + cmds: [{ + id, type: 'install_mcp', scope: 'workspace', workspace_path: wsPath, slug: 'clawpro', version: '1.0.0', mcp_config: mcpConfig, + }], + }); + const bearer = { transport: 'http', url: 'https://clawpro.example.com/mcp', headers: { Authorization: 'Bearer bmcp-test-token' } }; + const workspaceFile = async (name: string): Promise => { + const wsDir = path.join(wsPath, '.teamai', 'workspaces'); + const ids = await fse.readdir(wsDir); + expect(ids).toHaveLength(1); + return path.join(wsDir, ids[0], name); + }; + + beforeEach(async () => { + wsPath = path.join(tmpDir, 'projects', 'repo-git'); + await fse.ensureDir(path.join(wsPath, '.codebuddy', 'skills')); + git('init', '-q'); + }); + + it.each([ + ['an Authorization header', bearer], + ['a stdio env value', { transport: 'stdio', command: 'clawpro-mcp', env: { CLAWPRO_TOKEN: 'bmcp-test-token' } }], + ])('lists the config in .git/info/exclude before writing %s, and records it in managed-mcp-files.json', async (_label, mcpConfig) => { + const acks = await install(9101, mcpConfig); + + expect(acks[0].status).toBe('success'); + expect(await fse.readFile(path.join(wsPath, '.mcp.json'), 'utf-8')).toContain('bmcp-test-token'); + expect(await fse.readFile(path.join(wsPath, '.git', 'info', 'exclude'), 'utf-8')).toMatch(/^\/\.mcp\.json$/m); + expect(git('status', '--porcelain', '--untracked-files=all', '--', '.mcp.json')).toBe(''); + const sidecar = await fse.readJson(await workspaceFile('managed-mcp-files.json')) as { files: Record }; + expect(Object.entries(sidecar.files)).toEqual([[expect.stringMatching(/\.mcp\.json$/), { tools: ['codebuddy'] }]]); + const manifest = await fse.readJson(await workspaceFile('managed-mcp.json')); + expect(manifest['codebuddy:project']).toEqual([expect.objectContaining({ name: 'clawpro', resolved: true })]); + }); + + it('leaves a user config visible to git when the ownership manifest cannot be written', async () => { + const original = { mcpServers: { mine: { command: 'my-server' } } }; + await fse.writeJson(path.join(wsPath, '.mcp.json'), original); + const excludeBefore = await fse.readFile(path.join(wsPath, '.git', 'info', 'exclude'), 'utf-8'); + const fs = await import('../utils/fs.js'); + const write = fs.writeJsonAtomic; + vi.spyOn(fs, 'writeJsonAtomic').mockImplementation(async (file, ...args) => { + if (file.endsWith('/managed-mcp.json')) throw new Error('simulated manifest write failure'); + return write(file, ...args); + }); + + const acks = await install(9110, bearer); + + expect(acks[0].status).toBe('failed'); + expect(acks[0].error).toContain('simulated manifest write failure'); + expect(await fse.readJson(path.join(wsPath, '.mcp.json'))).toEqual(original); + expect(await fse.readFile(path.join(wsPath, '.git', 'info', 'exclude'), 'utf-8')).toBe(excludeBefore); + expect(git('status', '--porcelain', '--untracked-files=all', '--', '.mcp.json')).toContain('?? .mcp.json'); + const wsDir = path.join(wsPath, '.teamai', 'workspaces'); + const ids = await fse.pathExists(wsDir) ? await fse.readdir(wsDir) : []; + for (const id of ids) expect(await fse.pathExists(path.join(wsDir, id, 'managed-mcp-files.json'))).toBe(false); + }); + + it('withholds it from a config git tracks, naming why, and leaves the file and its records as they were', async () => { + const original = { mcpServers: { mine: { type: 'http', url: 'https://mine.example.com/mcp' } } }; + await fse.writeJson(path.join(wsPath, '.mcp.json'), original); + git('add', '.mcp.json'); + + const acks = await install(9102, bearer); + + expect(acks[0].status).toBe('failed'); + expect(acks[0].error).toContain('git already tracks'); + // Commands come from the server: no pull replays one. + expect(acks[0].error).toContain('then install the MCP server again.'); + expect(acks[0].error).not.toContain('teamai pull'); + expect(await fse.readJson(path.join(wsPath, '.mcp.json'))).toEqual(original); + const manifestFile = path.join(wsPath, '.teamai', 'workspaces'); + const manifests = await fse.pathExists(manifestFile) ? await fse.readdir(manifestFile) : []; + for (const id of manifests) { + const manifest = await fse.readJson(path.join(manifestFile, id, 'managed-mcp.json')).catch(() => ({})); + expect(manifest['codebuddy:project']).toBeUndefined(); + } + }); + + // Any URL counts (a token can sit in its path), so only a bare stdio command carries none. + it('adds no line for a server that carries no credential: a bare stdio command', async () => { + const acks = await install(9103, { transport: 'stdio', command: 'clawpro-mcp' }); + + expect(acks[0].status).toBe('success'); + expect(await fse.readFile(path.join(wsPath, '.git', 'info', 'exclude'), 'utf-8')).not.toMatch(/\.mcp\.json/); + }); + + it('still lists that config when the sync also carried an uninstall_teamai that failed', async () => { + await install(9106, bearer); + await fse.writeFile(path.join(wsPath, '.git', 'info', 'exclude'), ''); + await fse.remove(await workspaceFile('managed-mcp-files.json')); + vi.stubEnv('TEAMAI_DISABLE_REMOTE_CMD', '1'); + + await runResponse({ cmds: [{ id: 9107, type: 'uninstall_teamai', cmd: 'teamai uninstall --force --agent codebuddy' }] }, 'codebuddy', wsPath); + + expect(await fse.readFile(path.join(wsPath, '.git', 'info', 'exclude'), 'utf-8')).toMatch(/^\/\.mcp\.json$/m); + }); + + it('says the next session tries again when that sync cannot list the file, and calls it a credential', async () => { + await install(9105, bearer); + const excludeFile = path.join(wsPath, '.git', 'info', 'exclude'); + await fse.writeFile(excludeFile, ''); + await fse.remove(await workspaceFile('managed-mcp-files.json')); + await fse.chmod(excludeFile, 0o444); + const { log } = await import('../utils/logger.js'); + vi.mocked(log.warn).mockClear(); + try { + await runResponse({ cmds: [] }, 'codebuddy', wsPath); + } finally { + await fse.chmod(excludeFile, 0o644); + } + + const warned = vi.mocked(log.warn).mock.calls.map(([line]) => String(line)).join('\n'); + expect(warned).toContain('may hold a credential'); + expect(warned).toContain('start a new session'); + expect(warned).not.toContain('teamai pull'); + }); + + it('lists a config an older install wrote a credential into on the next sync in the workspace, with no command to run', async () => { + await install(9104, bearer); + // As an older local agent left it: no line, no managed-mcp-files.json, no note on the record. + await fse.writeFile(path.join(wsPath, '.git', 'info', 'exclude'), ''); + await fse.remove(await workspaceFile('managed-mcp-files.json')); + const manifestFile = await workspaceFile('managed-mcp.json'); + const manifest = await fse.readJson(manifestFile) as Record>; + manifest['codebuddy:project'] = manifest['codebuddy:project'].map(({ name, hash }) => ({ name, hash })); + await fse.writeJson(manifestFile, manifest); + expect(git('status', '--porcelain', '--untracked-files=all', '--', '.mcp.json')).not.toBe(''); + + await runResponse({ cmds: [] }, 'codebuddy', wsPath); + + expect(await fse.readFile(path.join(wsPath, '.git', 'info', 'exclude'), 'utf-8')).toMatch(/^\/\.mcp\.json$/m); + expect(git('status', '--porcelain', '--untracked-files=all', '--', '.mcp.json')).toBe(''); + const sidecar = await fse.readJson(await workspaceFile('managed-mcp-files.json')) as { files: Record }; + expect(Object.entries(sidecar.files)).toEqual([[expect.stringMatching(/\.mcp\.json$/), { tools: ['codebuddy'] }]]); + expect(await fse.readFile(path.join(wsPath, '.mcp.json'), 'utf-8')).toContain('bmcp-test-token'); + }); + + // As an older local agent left it: no line, no managed-mcp-files.json, no note on the record. + const asAnOlderAgentLeftIt = async (tool: string): Promise => { + await fse.writeFile(path.join(wsPath, '.git', 'info', 'exclude'), ''); + await fse.remove(await workspaceFile('managed-mcp-files.json')); + const manifestFile = await workspaceFile('managed-mcp.json'); + const manifest = await fse.readJson(manifestFile) as Record>; + manifest[`${tool}:project`] = manifest[`${tool}:project`].map(({ name, hash }) => ({ name, hash })); + await fse.writeJson(manifestFile, manifest); + }; + + it('lists the config an agent from before 57636a27 wrote at CodeBuddy\'s former .codebuddy/mcp.json, on the next sync', async () => { + await install(9107, bearer); + await asAnOlderAgentLeftIt('codebuddy'); + await fse.move(path.join(wsPath, '.mcp.json'), path.join(wsPath, '.codebuddy', 'mcp.json')); + expect(git('status', '--porcelain', '--untracked-files=all', '--', '.codebuddy/mcp.json')).not.toBe(''); + + await runResponse({ cmds: [] }, 'codebuddy', wsPath); + + expect(await fse.readFile(path.join(wsPath, '.git', 'info', 'exclude'), 'utf-8')).toMatch(/^\/\.codebuddy\/mcp\.json$/m); + expect(git('status', '--porcelain', '--untracked-files=all', '--', '.codebuddy/mcp.json')).toBe(''); + }); + + it('lists a Copilot config whose bare entry holds a credential beside a credential-free one of its name under mcpServers', async () => { + const configFile = path.join(wsPath, '.github', 'mcp.json'); + await fse.outputJson(configFile, {}); + const acks = await runResponse({ + cmds: [{ id: 9108, type: 'install_mcp', scope: 'workspace', workspace_path: wsPath, slug: 'clawpro', version: '1.0.0', mcp_config: bearer }], + }, 'copilot'); + expect(acks[0].status).toBe('success'); + await asAnOlderAgentLeftIt('copilot'); + const doc = await fse.readJson(configFile) as Record; + expect(JSON.stringify(doc.clawpro)).toContain('bmcp-test-token'); + await fse.writeJson(configFile, { ...doc, mcpServers: { clawpro: { command: 'clawpro-mcp' } } }); + + await runResponse({ cmds: [] }, 'copilot', wsPath); + + expect(await fse.readFile(path.join(wsPath, '.git', 'info', 'exclude'), 'utf-8')).toMatch(/^\/\.github\/mcp\.json$/m); + expect(git('status', '--porcelain', '--untracked-files=all', '--', '.github/mcp.json')).toBe(''); + }); + + it('still lists that config when an install replacing its entry with a bare command cannot write the file', async () => { + await install(9105, bearer); + await fse.writeFile(path.join(wsPath, '.git', 'info', 'exclude'), ''); + await fse.remove(await workspaceFile('managed-mcp-files.json')); + const manifestFile = await workspaceFile('managed-mcp.json'); + const manifest = await fse.readJson(manifestFile) as Record>; + manifest['codebuddy:project'] = manifest['codebuddy:project'].map(({ name, hash }) => ({ name, hash })); + await fse.writeJson(manifestFile, manifest); + // The config's directory refuses the write: the old entry, and its token, stay. + await fse.chmod(wsPath, 0o555); + let acks; + try { + acks = await install(9106, { transport: 'stdio', command: 'clawpro-mcp' }); + } finally { + await fse.chmod(wsPath, 0o755); + } + + expect(acks[0].status).toBe('failed'); + expect(await fse.readFile(path.join(wsPath, '.mcp.json'), 'utf-8')).toContain('bmcp-test-token'); + + await runResponse({ cmds: [] }, 'codebuddy', wsPath); + + expect(await fse.readFile(path.join(wsPath, '.git', 'info', 'exclude'), 'utf-8')).toMatch(/^\/\.mcp\.json$/m); + expect(git('status', '--porcelain', '--untracked-files=all', '--', '.mcp.json')).toBe(''); + }); + }); + // ─── install_mcp: 缺少 mcp_config 时失败 ────────────────────────── it('install_mcp fails when mcp_config is missing', async () => { const acks = await runResponse({ diff --git a/src/__tests__/mcp-git-exclude.test.ts b/src/__tests__/mcp-git-exclude.test.ts index e17bb895b..46bdcc387 100644 --- a/src/__tests__/mcp-git-exclude.test.ts +++ b/src/__tests__/mcp-git-exclude.test.ts @@ -39,7 +39,7 @@ vi.mock('../utils/fs.js', async (importOriginal) => { }; }); -import { MCP_EXCLUDE_END, MCP_EXCLUDE_START, ensureExcludedFromGit, excludeFromGit, removeMcpGitExclude } from '../mcp-git-exclude.js'; +import { MCP_EXCLUDE_END, MCP_EXCLUDE_START, carriesLocalAgentCredential, ensureExcludedFromGit, excludeFromGit, removeMcpGitExclude } from '../mcp-git-exclude.js'; import { acquireLock, releaseLock } from '../update.js'; import { log } from '../utils/logger.js'; @@ -179,6 +179,14 @@ describe('teamai block in .git/info/exclude (#882)', () => { expect(await fse.pathExists(excludeFile) ? await fse.readFile(excludeFile, 'utf8') : '').not.toContain('teamai'); }); + it('names the caller\'s way to try again in its fix, when given one', async () => { + const file = path.join(repo, '.mcp.json'); + + expect(await ensureExcludedFromGit(file, { dryRun: true, rerun: 'install the MCP server again' })).toMatchObject({ + fix: `Run \`git rm --cached ${file}\` (rotate any value a commit of it holds), then install the MCP server again.`, + }); + }); + it.skipIf(process.getuid?.() === 0).each([ ['a pull', {}], ['a dry run', { dryRun: true }], @@ -349,3 +357,27 @@ describe('teamai block in .git/info/exclude (#882)', () => { }); }); }); + +describe('a local agent install that carries a credential (#882)', () => { + it.each([ + ['a header', { type: 'http', url: 'https://x.example/mcp', headers: { Authorization: 'Bearer t' } }], + ['an env value', { command: 'npx', env: { TOKEN: 't' } }], + ['an argument', { command: 'npx', args: ['-y', 'server', '--token', 't'] }], + ['a URL with a user', { type: 'http', url: 'https://user:t@x.example/mcp' }], + ['a URL with a query', { type: 'http', url: 'https://x.example/mcp?key=t' }], + ['a URL with a token in its path', { type: 'http', url: 'https://x.example/mcp/bmcp-t0ken' }], + ['any URL: nothing tells a token in its path from a plain one', { type: 'http', url: 'https://x.example/mcp' }], + ['a whole command line', { command: 'server --token t' }], + ['a whole command line in OpenCode\'s one-element array', { type: 'local', command: ['server --token t'] }], + ['a command array with arguments', { type: 'local', command: ['server', '--token', 't'] }], + ])('counts %s', (_, entry) => { + expect(carriesLocalAgentCredential(entry)).toBe(true); + }); + + it.each([ + ['a bare command', { command: 'npx' }], + ['a bare command in a one-element array', { type: 'local', command: ['npx'] }], + ])('does not count %s', (_, entry) => { + expect(carriesLocalAgentCredential(entry)).toBe(false); + }); +}); diff --git a/src/__tests__/mcp-reconcile.test.ts b/src/__tests__/mcp-reconcile.test.ts index 38964d1ee..19145eb61 100644 --- a/src/__tests__/mcp-reconcile.test.ts +++ b/src/__tests__/mcp-reconcile.test.ts @@ -990,6 +990,49 @@ servers: expect(await excludeOf(projectRoot)).toMatch(/^\/\.mcp\.json$/m); }); + it('never lets a tool that maps a moved tool\'s file today claim its stale server under another key', async () => { + const opencode = { skills: '.opencode/skills', mcp: '.config/opencode/opencode.json', mcpProject: '.mcp.json' }; + const before = { ...teamConfig, toolPaths: { ...TOOL_PATHS, cursor: { ...TOOL_PATHS.cursor, mcpProject: '.mcp.json' }, opencode } } as TeamaiConfig; + const after = { ...teamConfig, toolPaths: { ...TOOL_PATHS, opencode } } as TeamaiConfig; + await fse.ensureDir(path.join(projectRoot, '.opencode', 'skills')); + const open = ' - name: open\n transport: http\n url: https://example.com/open\n tools: [claude]\n'; + await writeMcpYaml(`servers:\n - name: x\n transport: http\n url: https://example.com/x\n headers:\n Authorization: Bearer \${SECRET_TOKEN}\n tools: [cursor]\n${open}`); + await reconcileMcpForConfig(before, projectConfig); + expect(await fse.readFile(path.join(projectRoot, '.mcp.json'), 'utf-8')).toContain('super-secret-value'); + // Cursor moves back to .cursor/mcp.json; OpenCode, on .mcp.json, now owns a literal x under `mcp`. + await writeMcpYaml(`servers:\n - name: x\n transport: http\n url: https://example.com/x\n tools: [opencode]\n${open}`); + vi.stubEnv('SECRET_TOKEN', ''); + await fse.writeFile(path.join(projectRoot, '.git', 'info', 'exclude'), ''); + + await reconcileMcpForConfig(after, projectConfig); + + expect(await fse.readFile(path.join(projectRoot, '.mcp.json'), 'utf-8')).toContain('super-secret-value'); + expect(await excludeOf(projectRoot)).toMatch(/^\/\.mcp\.json$/m); + }); + + it('never lets a tool that maps a moved Copilot\'s file today claim its stale bare server by the name it owns under mcpServers', async () => { + const copilot = { skills: '.github/skills', mcp: '.copilot/mcp-config.json' }; + const before = { ...teamConfig, toolPaths: { ...TOOL_PATHS, copilot: { ...copilot, mcpProject: '.mcp.json' } } } as TeamaiConfig; + const after = { ...teamConfig, toolPaths: { ...TOOL_PATHS, copilot: { ...copilot, mcpProject: '.github/mcp.json' } } } as TeamaiConfig; + await fse.ensureDir(path.join(projectRoot, '.github', 'skills')); + // An empty file reads as Copilot's bare map: its write stays bare. + await fse.writeFile(path.join(projectRoot, '.mcp.json'), ''); + await writeMcpYaml(`${withSecret} tools: [copilot]\n`); + await reconcileMcpForConfig(before, projectConfig); + expect((await fse.readJson(path.join(projectRoot, '.mcp.json')) as Record)['with-secret']).toBeDefined(); + // Copilot moves to .github/mcp.json; Claude, on .mcp.json, now owns a literal with-secret under mcpServers. + await writeMcpYaml(`${withSecret.replace('${SECRET_TOKEN}', 'published-literal')} tools: [claude]\n`); + vi.stubEnv('SECRET_TOKEN', ''); + await fse.writeFile(path.join(projectRoot, '.git', 'info', 'exclude'), ''); + + await reconcileMcpForConfig(after, projectConfig); + + const doc = await fse.readJson(path.join(projectRoot, '.mcp.json')) as Record; + expect((doc.mcpServers as Record)['with-secret']).toBeDefined(); + expect(JSON.stringify(doc['with-secret'])).toContain('super-secret-value'); + expect(await excludeOf(projectRoot)).toMatch(/^\/\.mcp\.json$/m); + }); + it('never lets one format\'s record claim a server of the same name under another format\'s key', async () => { const toolPaths = { ...UNMOVED_TOOL_PATHS, @@ -1053,6 +1096,241 @@ servers: expect(await excludeOf(projectRoot)).toMatch(/^\/\.mcp\.json$/m); }); + // No pull writes an HTTP team's servers, but one lists what an older local agent's install_mcp left unlisted. + describe('for an HTTP-backed team, a config an older local agent wrote a credential into', () => { + let httpConfig: LocalConfig; + const file = (): string => path.join(projectRoot, '.mcp.json'); + const written = { mcpServers: { clawpro: { type: 'http', url: 'https://clawpro.example.com/mcp', headers: { Authorization: 'Bearer bmcp-old-token' } } } }; + + beforeEach(async () => { + httpConfig = { ...projectConfig, repo: { ...projectConfig.repo, kind: 'http', url: 'https://teamai.example' } } as LocalConfig; + await fse.writeJson(file(), written); + const { getDataHome, managedMcpManifestPath } = await import('../types.js'); + // Without the `resolved` note an install on this version adds. + await fse.outputJson(managedMcpManifestPath(getDataHome(httpConfig), projectRoot), { 'claude:project': [{ name: 'clawpro', hash: 'h' }] }); + }); + + it('lists it in .git/info/exclude on a pull, and records it in managed-mcp-files.json', async () => { + await reconcileMcpForConfig(teamConfig, httpConfig); + + expect(await excludeOf(projectRoot)).toMatch(/^\/\.mcp\.json$/m); + expect(git(projectRoot, 'status', '--porcelain', '--untracked-files=all', '--', '.mcp.json')).toBe(''); + const { readResolvedMcpFiles } = await import('../mcp-resolved-files.js'); + expect((await readResolvedMcpFiles(httpConfig)).files).toEqual({ [file()]: { tools: ['claude'] } }); + expect(await fse.readJson(file())).toEqual(written); + }); + + it('writes nothing on a dry run', async () => { + await reconcileMcpForConfig(teamConfig, httpConfig, { dryRun: true }); + + expect(await excludeOf(projectRoot)).not.toMatch(/\.mcp\.json/); + const { resolvedMcpFilesPath } = await import('../mcp-resolved-files.js'); + expect(await fse.pathExists(resolvedMcpFilesPath(httpConfig) ?? '')).toBe(false); + }); + }); + + describe('a bare Copilot config another tool then writes mcpServers into (Copilot and Claude on .mcp.json)', () => { + const shared = (): TeamaiConfig => ({ + ...teamConfig, + toolPaths: { ...TOOL_PATHS, copilot: { skills: '.github/skills', mcp: '.copilot/mcp-config.json', mcpProject: '.mcp.json' } }, + } as TeamaiConfig); + const open = 'servers:\n - name: open\n transport: http\n url: https://example.com/open\n tools: [claude]\n'; + + beforeEach(async () => { + await fse.ensureDir(path.join(projectRoot, '.github', 'skills')); + // An empty file reads as Copilot's bare map: its first write stays bare. + await fse.writeFile(path.join(projectRoot, '.mcp.json'), ''); + await writeMcpYaml(`${withSecret} tools: [copilot]\n`); + await reconcileMcpForConfig(shared(), projectConfig); + const first = await fse.readJson(path.join(projectRoot, '.mcp.json')) as Record; + expect(first['with-secret']).toEqual(expect.objectContaining({ headers: { Authorization: 'Bearer super-secret-value' } })); + expect(first.mcpServers).toBeUndefined(); + }); + + it('does not infer bare ownership from an unchanged entry with a legacy record', async () => { + const { getDataHome, managedMcpManifestPath } = await import('../types.js'); + const manifestFile = managedMcpManifestPath(getDataHome(projectConfig), projectRoot); + const manifest = await fse.readJson(manifestFile); + delete manifest['copilot:project'][0].bare; + await fse.writeJson(manifestFile, manifest); + const bare = (await fse.readJson(path.join(projectRoot, '.mcp.json')))['with-secret']; + + await reconcileMcpForConfig(shared(), projectConfig); + expect((await fse.readJson(manifestFile))['copilot:project'][0].bare).toBeUndefined(); + await writeMcpYaml(open); + await reconcileMcpForConfig(shared(), { ...projectConfig, disabledAgents: ['copilot'] } as LocalConfig); + await reconcileMcpForConfig(shared(), projectConfig); + + expect((await fse.readJson(path.join(projectRoot, '.mcp.json')))['with-secret']).toEqual(bare); + }); + + it.each(['update', 'drop', 'remove', 'missing-secret'] as const)('preserves a member-owned keyed Copilot entry after a bare write during %s', async (action) => { + const file = path.join(projectRoot, '.mcp.json'); + const mine = { type: 'http', tools: ['*'], url: 'https://member.example/mcp' }; + const doc = await fse.readJson(file); + await fse.writeJson(file, { ...doc, mcpServers: { 'with-secret': mine } }); + if (action === 'update') await writeMcpYaml(`${withSecret.replace('${SECRET_TOKEN}', 'new-team-value')} tools: [copilot]\n`); + if (action === 'drop') await writeMcpYaml('servers: []\n'); + if (action === 'missing-secret') { + await fse.outputFile(path.join(repoPath, 'env', 'secrets.yaml'), 'secrets:\n - key: SECRET_TOKEN\n'); + vi.stubEnv('SECRET_TOKEN', ''); + } + + const result = await reconcileMcpForConfig(shared(), projectConfig, action === 'remove' ? { removeAll: true } : {}); + if (action === 'update') await reconcileMcpForConfig(shared(), projectConfig); + + expect((await fse.readJson(file)).mcpServers['with-secret']).toEqual(mine); + if (action === 'update') { + expect(result.changes).toContainEqual(expect.objectContaining({ tool: 'copilot', server: 'with-secret', action: 'skipped' })); + expect((await fse.readJson(file))['with-secret']).toEqual(doc['with-secret']); + } + if (action === 'drop' || action === 'remove') expect((await fse.readJson(file))['with-secret']).toBeUndefined(); + }); + + it.each([['update', false], ['remove', false], ['update', true], ['remove', true]] as const)('preserves a keyed member entry with an unmarked bare record during %s, identical=%s', async (action, identical) => { + const { getDataHome, managedMcpManifestPath } = await import('../types.js'); + const manifestFile = managedMcpManifestPath(getDataHome(projectConfig), projectRoot); + const manifest = await fse.readJson(manifestFile); + delete manifest['copilot:project'][0].bare; + await fse.writeJson(manifestFile, manifest); + const file = path.join(projectRoot, '.mcp.json'); + const doc = await fse.readJson(file); + const mine = identical ? doc['with-secret'] : { type: 'http', tools: ['*'], url: 'https://member.example/mcp' }; + await fse.writeJson(file, { ...doc, mcpServers: { 'with-secret': mine } }); + if (action === 'update') await writeMcpYaml(`${withSecret.replace('${SECRET_TOKEN}', 'new-team-value')} tools: [copilot]\n`); + + const result = await reconcileMcpForConfig(shared(), projectConfig, action === 'remove' ? { removeAll: true } : {}); + + expect((await fse.readJson(file)).mcpServers['with-secret']).toEqual(mine); + expect((await fse.readJson(file))['with-secret']).toEqual(doc['with-secret']); + if (action === 'update') expect(result.changes).toContainEqual(expect.objectContaining({ tool: 'copilot', server: 'with-secret', action: 'skipped' })); + }); + + it.each(['legacy bare', 'legacy keyed', 'proven keyed', 'bare migration'] as const)( + 'restores %s after a manifest write fails so update and removal can be retried', async (source) => { + const { getDataHome, managedMcpManifestPath } = await import('../types.js'); + const manifestFile = managedMcpManifestPath(getDataHome(projectConfig), projectRoot); + const manifest = await fse.readJson(manifestFile); + const file = path.join(projectRoot, '.mcp.json'); + const doc = await fse.readJson(file); + if (source.endsWith('keyed')) { + await fse.writeJson(file, { mcpServers: doc }); + manifest['copilot:project'][0].bare = false; + } + if (source.startsWith('legacy')) delete manifest['copilot:project'][0].bare; + if (source === 'bare migration') await fse.writeJson(file, { ...doc, mcpServers: { mine: { command: 'member-server' } } }); + await fse.writeJson(manifestFile, manifest); + const original = await fse.readJson(file); + await writeMcpYaml(`${withSecret.replace('${SECRET_TOKEN}', 'replacement')} tools: [copilot]\n`); + beforeJsonWrite.run = async (target) => { + if (target.endsWith('/managed-mcp.json')) throw new Error('simulated manifest write failure'); + }; + + await expect(reconcileMcpForConfig(shared(), projectConfig)).rejects.toThrow('simulated manifest write failure'); + beforeJsonWrite.run = null; + + expect(await fse.readJson(manifestFile)).toEqual(manifest); + expect(await fse.readJson(file)).toEqual(original); + expect(git(projectRoot, 'status', '--porcelain', '--untracked-files=all', '--', '.mcp.json')).toBe(''); + await reconcileMcpForConfig(shared(), projectConfig); + expect(await fse.readFile(file, 'utf-8')).not.toContain('super-secret-value'); + await reconcileMcpForConfig(shared(), projectConfig, { removeAll: true }); + const removed = await fse.readJson(file); + expect(removed['with-secret']).toBeUndefined(); + expect(removed.mcpServers?.['with-secret']).toBeUndefined(); + if (source === 'bare migration') expect(removed.mcpServers.mine).toEqual({ command: 'member-server' }); + }); + + it('restores the first write when a second tool fails on the same config', async () => { + const { getDataHome, managedMcpManifestPath } = await import('../types.js'); + const manifestFile = managedMcpManifestPath(getDataHome(projectConfig), projectRoot); + const file = path.join(projectRoot, '.mcp.json'); + const original = await fse.readJson(file); + const manifest = await fse.readJson(manifestFile); + await writeMcpYaml(`${withSecret.replace('${SECRET_TOKEN}', 'replacement')} tools: [copilot]\n${open.replace('servers:\n', '')}`); + let writes = 0; + beforeJsonWrite.run = async (target) => { + if (target === file && ++writes === 2) throw new Error('simulated second config write failure'); + }; + + await expect(reconcileMcpForConfig(shared(), projectConfig)).rejects.toThrow('simulated second config write failure'); + beforeJsonWrite.run = null; + + expect(await fse.readJson(file)).toEqual(original); + expect(await fse.readJson(manifestFile)).toEqual(manifest); + await reconcileMcpForConfig(shared(), projectConfig); + expect(await fse.readFile(file, 'utf-8')).not.toContain('super-secret-value'); + }); + + it('keeps its line, pull after pull, while Copilot\'s bare entry holds the value beside the mcpServers Claude wrote', async () => { + await writeMcpYaml(`${withSecret} tools: [copilot]\n${open.replace('servers:\n', '')}`); + // No longer set: only the entry, not a scan for the value, says what the file holds. + vi.stubEnv('SECRET_TOKEN', ''); + + await reconcileMcpForConfig(shared(), { ...projectConfig, disabledAgents: ['copilot'] } as LocalConfig); + await reconcileMcpForConfig(shared(), { ...projectConfig, disabledAgents: ['copilot'] } as LocalConfig); + + const after = await fse.readJson(path.join(projectRoot, '.mcp.json')) as Record; + expect(after.mcpServers).toEqual({ open: expect.objectContaining({ url: 'https://example.com/open' }) }); + expect(JSON.stringify(after)).toContain('super-secret-value'); + expect(await excludeOf(projectRoot)).toMatch(/^\/\.mcp\.json$/m); + }); + + it('replaces Copilot\'s bare entry when it writes that server again under the mcpServers Claude added', async () => { + await writeMcpYaml(`${withSecret} tools: [copilot]\n${open.replace('servers:\n', '')}`); + await reconcileMcpForConfig(shared(), projectConfig); + expect((await fse.readJson(path.join(projectRoot, '.mcp.json')) as Record).mcpServers).toBeDefined(); + await writeMcpYaml(`${withSecret.replace('${SECRET_TOKEN}', 'published-literal')} tools: [copilot]\n${open.replace('servers:\n', '')}`); + vi.stubEnv('SECRET_TOKEN', ''); + + await reconcileMcpForConfig(shared(), projectConfig); + + const after = await fse.readJson(path.join(projectRoot, '.mcp.json')) as Record; + expect(JSON.stringify(after)).not.toContain('super-secret-value'); + expect(after['with-secret']).toBeUndefined(); + expect((after.mcpServers as Record)['with-secret']).toEqual(expect.objectContaining({ headers: { Authorization: 'Bearer published-literal' } })); + }); + + it('never removes a bare server of the member\'s own that shares a name with one teamai writes under mcpServers', async () => { + const mine = { type: 'http', tools: ['*'], url: 'https://jira.example/mcp' }; + const doc = await fse.readJson(path.join(projectRoot, '.mcp.json')) as Record; + await fse.writeJson(path.join(projectRoot, '.mcp.json'), { ...doc, jira: mine }); + const jira = ' - name: jira\n transport: http\n url: https://jira.example/mcp\n tools: [copilot]\n'; + await writeMcpYaml(`${withSecret} tools: [copilot]\n${open.replace('servers:\n', '')}${jira}`); + await reconcileMcpForConfig(shared(), projectConfig); + await reconcileMcpForConfig(shared(), projectConfig); + await writeMcpYaml(`${withSecret} tools: [copilot]\n${open.replace('servers:\n', '')}`); + await reconcileMcpForConfig(shared(), projectConfig); + + expect((await fse.readJson(path.join(projectRoot, '.mcp.json')) as Record).jira).toEqual(mine); + }); + + it('keeps its line while a stale bare copy differs from the entry of that name under mcpServers', async () => { + const stale = (await fse.readJson(path.join(projectRoot, '.mcp.json')) as Record)['with-secret']; + await fse.writeJson(path.join(projectRoot, '.mcp.json'), { + 'with-secret': stale, + mcpServers: { 'with-secret': { type: 'http', url: 'https://example.com/mcp', headers: { Authorization: 'Bearer published-literal' } } }, + }); + await fse.writeFile(path.join(projectRoot, '.git', 'info', 'exclude'), ''); + vi.stubEnv('SECRET_TOKEN', ''); + + await reconcileMcpForConfig(shared(), { ...projectConfig, disabledAgents: ['copilot'] } as LocalConfig); + + expect(JSON.stringify(await fse.readJson(path.join(projectRoot, '.mcp.json')))).toContain('super-secret-value'); + expect(await excludeOf(projectRoot)).toMatch(/^\/\.mcp\.json$/m); + }); + + it('removes Copilot\'s bare entry once the team drops it, leaving the mcpServers Claude wrote', async () => { + await writeMcpYaml(open); + vi.stubEnv('SECRET_TOKEN', ''); + + await reconcileMcpForConfig(shared(), projectConfig); + + const after = await fse.readJson(path.join(projectRoot, '.mcp.json')) as Record; + expect(after).toEqual({ mcpServers: { open: expect.objectContaining({ url: 'https://example.com/open' }) } }); + }); + }); + describe('a config two tools share (Claude and CodeBuddy on .mcp.json)', () => { const shared = { ...teamConfig, toolPaths: { ...TOOL_PATHS, codebuddy: { ...TOOL_PATHS.codebuddy, mcpProject: '.mcp.json' } } } as TeamaiConfig; const open = ' - name: open\n transport: http\n url: https://example.com/open\n'; @@ -1982,7 +2260,7 @@ servers: expect(vi.mocked(log.debug).mock.calls.flat().join('\n')).toMatch(/\/\.mcp\.json/); }); - it('but one this pull listed stays when it wrote the value and then failed to record it', async () => { + it.each([false, true])('handles a failed ownership write after adding a credential, restoration fails=%s', async (restoreFails) => { // Shorter than eight characters: no scan of the file can find it again. vi.stubEnv('SECRET_TOKEN', 'short'); await writeMcpYaml(withSecret); @@ -1990,10 +2268,23 @@ servers: if (path.basename(file) === 'managed-mcp.json') throw new Error('disk full'); }; - await expect(reconcileMcpForConfig(teamConfig, claudeOnly())).rejects.toThrow('disk full'); + const rm = fs.promises.rm; + const spy = vi.spyOn(fs.promises, 'rm').mockImplementation(async (file, ...args) => { + if (restoreFails && String(file) === mcpJson()) throw new Error('simulated restoration failure'); + return rm(file, ...args); + }); + await expect(reconcileMcpForConfig(teamConfig, claudeOnly())).rejects.toThrow(restoreFails ? /restoring configs failed.*simulated restoration failure/ : 'disk full'); + spy.mockRestore(); - expect(await fse.readFile(mcpJson(), 'utf-8')).toContain('Bearer short'); - expect(await excludeOf(projectRoot)).toMatch(/^\/\.mcp\.json$/m); + if (restoreFails) { + expect(await fse.readFile(mcpJson(), 'utf-8')).toContain('Bearer short'); + expect(await excludeOf(projectRoot)).toMatch(/^\/\.mcp\.json$/m); + } else { + expect(await fse.pathExists(mcpJson())).toBe(false); + expect(await excludeOf(projectRoot)).not.toMatch(/^\/\.mcp\.json$/m); + const { readResolvedMcpFiles } = await import('../mcp-resolved-files.js'); + expect((await readResolvedMcpFiles(claudeOnly())).files[mcpJson()]).toBeUndefined(); + } }); it('but one an earlier pull listed stays while the config cannot be proven clean', async () => { diff --git a/src/doctor-delivery.ts b/src/doctor-delivery.ts index 2a330c207..9e5074de1 100644 --- a/src/doctor-delivery.ts +++ b/src/doctor-delivery.ts @@ -504,7 +504,7 @@ export async function buildMcpDeliveryChecks(ctx: DoctorContext): Promise { const { localConfig, teamConfig } = ctx; const { projectRoot } = localConfig; - if (!teamConfig || localConfig.scope !== 'project' || !projectRoot || localConfig.repo.kind === 'http') return []; + if (!teamConfig || localConfig.scope !== 'project' || !projectRoot) return []; const { - resolveMcpTargets, resolvedValueEvidence, buildVarTable, buildDesiredMcpContext, recordedMcpTargets, recordedMcpFileEvidence, - earlierMappedMcpTargets, earlierMappedMcpFileEvidence, unrecordedMcpTool, unmappedMcpDefaults, unrecordedUnmappedMcpDefaults, unclaimedMcpServers, + resolveMcpTargets, resolvedValueEvidence, buildVarTable, buildDesiredMcpContext, recordedMcpTargets, recordedMcpFileEvidence, localAgentCredentialFiles, + earlierMappedMcpTargets, earlierMappedMcpFileEvidence, ownedByMappers, unrecordedMcpTool, unmappedMcpDefaults, unrecordedUnmappedMcpDefaults, unclaimedMcpServers, } = await import('./mcp-reconcile.js'); const { readResolvedMcpFiles } = await import('./mcp-resolved-files.js'); const { gitPathOf, gitTracking, gitTracks } = await import('./mcp-git-exclude.js'); @@ -593,6 +595,20 @@ export async function buildMcpGitExcludeCheck(ctx: DoctorContext): Promise !unmapped.has(target)); + const report = (held: string, next: string): Check[] => holding.size === 0 ? [] : [{ + name: 'Project MCP configs with resolved values are kept out of git', + source: 'local', + check: async () => tracked.length === 0, + fix: `${tracked.join(', ')} may hold ${held}, and git would commit them or cannot say. ${next}`, + }]; + if (localConfig.repo.kind === 'http') { + // No mcp.yaml to judge by: the files that may hold a credential the local agent wrote. + for (const { file } of await localAgentCredentialFiles(localConfig, targets)) { + if (!holding.has(file)) await hold(file); + } + return report('MCP credentials in plaintext', 'Fix any git error shown, then add each to .git/info/exclude. If git already tracks one, run ' + + '`git rm --cached ` and rotate the values it held.'); + } for (const target of targets) { if (holding.has(target.file) || !await pathExists(target.file)) continue; manifest ??= (await loadProjectMcpManifest(getDataHome(localConfig), projectRoot, { dryRun: true })).manifest; @@ -613,9 +629,7 @@ export async function buildMcpGitExcludeCheck(ctx: DoctorContext): Promise manifest?.[managedMcpManifestKey(tool, true)] ?? []).map((record) => record.name); - if (await recordedMcpFileEvidence(group, owned)) await hold(file); + if (await recordedMcpFileEvidence(group, ownedByMappers(mappedBy, manifest))) await hold(file); } // And, until a pull on this version reads them, those an older teamai wrote under a mapping an earlier // teamai.yaml made. Read-only: the record of that read is pull's. Unreadable history skips them. @@ -626,20 +640,10 @@ export async function buildMcpGitExcludeCheck(ctx: DoctorContext): Promise manifest?.[managedMcpManifestKey(tool, true)] ?? []).map((record) => record.name); - if (await earlierMappedMcpFileEvidence(target, teamDefs, vars, desired, owned)) await hold(target.file); + if (await earlierMappedMcpFileEvidence(target, teamDefs, vars, desired, ownedByMappers(mappedBy, manifest))) await hold(target.file); } - if (holding.size === 0) return []; - - return [{ - name: 'Project MCP configs with resolved values are kept out of git', - source: 'local', - check: async () => tracked.length === 0, - fix: `${tracked.join(', ')} may hold MCP variables resolved to plaintext, and git would commit them or cannot say. ` - + 'Fix any git error shown, then run `teamai pull` to list them in .git/info/exclude. If git already tracks one, run ' - + '`git rm --cached ` and rotate the values it held.', - }]; + return report('MCP variables resolved to plaintext', 'Fix any git error shown, then run `teamai pull` to list them in .git/info/exclude. If git already tracks one, run ' + + '`git rm --cached ` and rotate the values it held.'); } /** diff --git a/src/local-agent.ts b/src/local-agent.ts index 9cd35fe2d..5729273af 100644 --- a/src/local-agent.ts +++ b/src/local-agent.ts @@ -13,6 +13,7 @@ import { listFilesRecursive, pathExists, readFileSafe, + readFileIfExists, readJson, remove, writeFile, @@ -42,6 +43,8 @@ import { } from './resources/mcp-format.js'; import { readJsonDoc, + isTeamaiBareCopy, + ownsJsonMcpEntry, writeJsonDoc, writeCodexAtomic, spliceCodexBlock, @@ -2845,13 +2848,17 @@ function updateManifestRecord( key: string, name: string, hash: string, + /** Project scope: whether the entry carries a credential, as `resolved` notes for a pull's (#882). */ + resolved?: boolean, + bare?: boolean, ): void { const records = manifest[key] ?? []; const idx = records.findIndex((r: ManagedMcpRecord) => r.name === name); + const record: ManagedMcpRecord = { name, hash, ...resolved === undefined ? {} : { resolved }, ...bare === undefined ? {} : { bare } }; if (idx >= 0) { - records[idx] = { name, hash }; + records[idx] = record; } else { - records.push({ name, hash }); + records.push(record); } manifest[key] = records; } @@ -2917,7 +2924,7 @@ async function installMcpServer( if (format === 'codex') { const block = renderCodexBlock(def); const hash = entryHash(block); - let source = (await readFileSafe(targetFile)) ?? ''; + let source = (await readFileIfExists(targetFile)) ?? ''; const present = new Set(codexServerNames(source)); if (present.has(slug) && !ownedNames.has(slug)) { throw new Error(`install_mcp: server "${slug}" exists in ${tool} config and is not managed by teamai`); @@ -2935,18 +2942,86 @@ async function installMcpServer( if (!doc) { throw new Error(`install_mcp: cannot parse ${targetFile}`); } - if (doc.servers[slug] !== undefined && !ownedNames.has(slug)) { + if (doc.servers[slug] !== undefined && !ownsJsonMcpEntry(doc, slug, owned, allowBare)) { throw new Error(`install_mcp: server "${slug}" exists in ${tool} config and is not managed by teamai`); } - updateManifestRecord(manifest, manifestKey, slug, hash); - await writeJsonAtomic(manifestPath, manifest); + // The copy a bare install left before another tool added the key would keep the old value beside this one (#882). + // Judged by the record as it was before this install updates it. + const bareCopy = isTeamaiBareCopy(doc, slug, owned); + // Check Git without changing it until ownership is persisted. Recheck protection before writing the credential (#882). + const credential = projectScope && await keepCredentialOutOfGit({ ...localConfig, dataHome }, tool, slug, targetFile, entry, true); + const previousRecord = owned.find((record) => record.name === slug); + const previousData = previousRecord ? structuredClone(doc.data) : undefined; + // Existing ownership stays valid until the config write completes. New installs + // still persist a provisional record before adding a Git exclusion (#882). + if (!previousRecord) { + updateManifestRecord(manifest, manifestKey, slug, hash, projectScope ? credential : undefined); + await writeJsonAtomic(manifestPath, manifest); + } + if (credential) await keepCredentialOutOfGit({ ...localConfig, dataHome }, tool, slug, targetFile, entry); + if (bareCopy) delete doc.data[slug]; doc.servers[slug] = entry; await writeJsonDoc(targetFile, serverKey, doc); + if (allowBare || previousRecord) { + // Placement is evidence of a completed write, not just an attempted install. + updateManifestRecord(manifest, manifestKey, slug, hash, projectScope ? credential : undefined, allowBare ? doc.bare : undefined); + try { + await writeJsonAtomic(manifestPath, manifest); + } catch (error) { + if (previousData) { + try { + await writeJsonAtomic(targetFile, previousData); + } catch (restoreError) { + throw new Error( + `install_mcp: ownership write failed (${error instanceof Error ? error.message : String(error)}), and restoring ${targetFile} failed ` + + `(${restoreError instanceof Error ? restoreError.message : String(restoreError)}). The config may not match ${manifestPath}. Repair the config and ownership record after fixing both write errors, then install the server again.`, + { cause: error }, + ); + } + } + throw error; + } + } } log.debug(`local-agent: installed MCP server "${slug}" for ${tool} (scope=${scope})`); return command.version; } +/** + * For a project-scope install: whether `entry` carries a credential and, if + * so, list `file` in `.git/info/exclude` and record it in + * managed-mcp-files.json, as a pull does before writing a resolved value + * (#882). With dryRun, checks protection without adding an exclusion or file record. + * Throws when git protection fails; the MCP config is left unchanged. + */ +async function keepCredentialOutOfGit( + localConfig: LocalConfig, + tool: string, + slug: string, + file: string, + entry: unknown, + dryRun = false, +): Promise { + const { carriesLocalAgentCredential, ensureExcludedFromGit } = await import('./mcp-git-exclude.js'); + if (!carriesLocalAgentCredential(entry)) return false; + // Commands come from the server: no pull replays one. + const exclusion = await ensureExcludedFromGit(file, { dryRun, rerun: 'install the MCP server again' }); + if (exclusion.kind === 'failed') { + throw new Error( + `install_mcp: withheld "${slug}" from ${file}: it may carry a credential (a header, env value, argument or URL), and teamai could not keep the file ` + + `out of git: ${exclusion.reason}. The file is left as it was. ${exclusion.fix}`, + ); + } + if (dryRun) return true; + const { trackResolvedMcpFiles } = await import('./mcp-resolved-files.js'); + // A failure does not stop the write: the exclusion protects the file. + const result = await trackResolvedMcpFiles(localConfig, [{ tool, file }]).catch((e: unknown) => e instanceof Error ? e.message : String(e)); + if (result !== 'written' && result !== 'unchanged') { + log.debug(`Did not record ${file} in managed-mcp-files.json: ${result === 'locked' ? 'another teamai command held it past the wait' : result}.`); + } + return true; +} + async function uninstallMcpServer( config: LocalAgentConfig, tool: string, @@ -2990,22 +3065,47 @@ async function uninstallMcpServer( if (!ownedNames.has(slug)) return; - manifest[manifestKey] = owned.filter((r: ManagedMcpRecord) => r.name !== slug); - if ((manifest[manifestKey] as ManagedMcpRecord[]).length === 0) delete manifest[manifestKey]; - await writeJsonAtomic(manifestPath, manifest); - + let restoreConfig: (() => Promise) | undefined; if (format === 'codex') { - let source = (await readFileSafe(targetFile)) ?? ''; - source = spliceCodexBlock(source, slug, null); - await writeCodexAtomic(targetFile, source); + const source = (await readFileIfExists(targetFile)) ?? ''; + const next = spliceCodexBlock(source, slug, null); + if (next !== source) { + await writeCodexAtomic(targetFile, next); + restoreConfig = () => writeCodexAtomic(targetFile, source); + } } else { const serverKey = MCP_SERVER_KEY[format]; const allowBare = format === 'copilot' && projectScope; const doc = await readJsonDoc(targetFile, serverKey, allowBare); - if (doc && doc.servers[slug] !== undefined) { - delete doc.servers[slug]; + if (!doc) throw new Error(`uninstall_mcp: cannot parse ${targetFile}. Ownership was kept; repair the config and uninstall the server again.`); + // Also a bare entry another tool's mcpServers now sits beside (#882). + const bareCopy = isTeamaiBareCopy(doc, slug, owned); + const ownsEntry = ownsJsonMcpEntry(doc, slug, owned, allowBare); + if ((ownsEntry && doc.servers[slug] !== undefined) || bareCopy) { + const previousData = structuredClone(doc.data); + if (ownsEntry) delete doc.servers[slug]; + if (bareCopy) delete doc.data[slug]; await writeJsonDoc(targetFile, serverKey, doc); + restoreConfig = () => writeJsonAtomic(targetFile, previousData); + } + } + manifest[manifestKey] = owned.filter((r: ManagedMcpRecord) => r.name !== slug); + if (manifest[manifestKey].length === 0) delete manifest[manifestKey]; + try { + await writeJsonAtomic(manifestPath, manifest); + } catch (error) { + if (restoreConfig) { + try { + await restoreConfig(); + } catch (restoreError) { + throw new Error( + `uninstall_mcp: ownership write failed (${error instanceof Error ? error.message : String(error)}), and restoring ${targetFile} failed ` + + `(${restoreError instanceof Error ? restoreError.message : String(restoreError)}). The config may not match ${manifestPath}. Repair the config and ownership record after fixing both write errors, then uninstall the server again.`, + { cause: error }, + ); + } } + throw error; } log.debug(`local-agent: uninstalled MCP server "${slug}" from ${tool} (scope=${scope})`); } @@ -3241,10 +3341,39 @@ export async function reportAndSyncLocalAgent(context: LocalAgentContext): Promi log.error(`${tag} sync FAILED: ${error}`); await appendErrorLog({ error, context }); } + // Also when the sync failed: what an install wrote is on disk either way. Also after an uninstall_teamai: + // one that removed teamai's servers and records leaves nothing to list, and one that failed or kept the + // shared files (another agent remains) leaves what still needs keeping out of git. + await protectWorkspaceMcpConfigs(config, context.cwd); return true; } +/** + * List in `.git/info/exclude` each MCP config of the current workspace that + * may hold a credential an install wrote (#882). An older local agent wrote + * one without listing it, and the server sends no install again for a server + * already in place. The workspace and its files resolve as `install_mcp` + * resolves them. + */ +async function protectWorkspaceMcpConfigs(config: LocalAgentConfig, cwd?: string): Promise { + const workspacePath = await resolveWorkspacePath(cwd); + if (!workspacePath) return; + try { + const { resolveDataHomeForScope } = await import('./config.js'); + const dataHome = await resolveDataHomeForScope('project', workspacePath); + const localConfig = await createResourceLocalConfig(config, 'project', getUserHome(), workspacePath); + const { protectLocalAgentMcpConfigs } = await import('./mcp-reconcile.js'); + // The sync at the next session start checks again, not a pull. + await protectLocalAgentMcpConfigs(createLocalAgentTeamConfig(config.endpoint), { ...localConfig, dataHome }, { rerun: 'start a new session' }); + } catch (e) { + log.warn( + `Could not check ${workspacePath}'s MCP configs for a credential to keep out of git: ${e instanceof Error ? e.message : String(e)}. ` + + 'The next session checks again; do not commit them meanwhile.', + ); + } +} + function statusFromEvent(event?: DashboardEvent): string { if (!event) return 'running'; if (event.type === 'stop' || event.type === 'process_exit') return 'stopped'; diff --git a/src/mcp-git-exclude.ts b/src/mcp-git-exclude.ts index fb94efaf6..5b469be4e 100644 --- a/src/mcp-git-exclude.ts +++ b/src/mcp-git-exclude.ts @@ -37,6 +37,27 @@ export function carriesResolvedValue( && !supportsEnvExpansion(target.format, target.projectScope, def)); } +/** + * Whether a JSON MCP entry the local agent installs for an HTTP-backed team + * carries a credential (#882): a header, env value or argument of any kind, a + * URL (a token can sit in its path, as well as in a user or a query), or a + * command line with arguments in it. Its payload holds the values themselves, + * not `${VAR}` references teamai resolves, so nothing tells a token from a + * plain setting: every one counts. Only a bare stdio command does not. + */ +export function carriesLocalAgentCredential(entry: unknown): boolean { + if (typeof entry !== 'object' || entry === null) return false; + const fields = entry as Record; + const nonEmpty = (value: unknown): boolean => + Array.isArray(value) ? value.length > 0 : typeof value === 'object' && value !== null && Object.keys(value).length > 0; + // OpenCode keeps env under `environment`, and a stdio command with its arguments under `command`. + if (['headers', 'env', 'environment', 'args'].some((key) => nonEmpty(fields[key]))) return true; + // A command line in one string, or OpenCode's one-element array holding it, carries its arguments too. + const commandParts: unknown[] = Array.isArray(fields.command) ? fields.command : [fields.command]; + if (commandParts.length > 1 || commandParts.some((part) => typeof part === 'string' && /\s/.test(part.trim()))) return true; + return ['url', 'serverUrl', 'httpUrl'].some((key) => typeof fields[key] === 'string' && fields[key].trim() !== ''); +} + /** * The variable whose value, resolved by teamai into `target`, `raw` (a project * file's text) holds, or null: one `teamDefs` references that the tool does not @@ -188,18 +209,24 @@ export type GitExclusion = * does not track: an exclude rule does not apply to a tracked file, and a git * error is never read as safe. `file` need not exist yet: pull calls this * before writing a resolved value into it. `dryRun` writes nothing and reports - * what would stop the write. + * what would stop the write. `rerun` ends each fix: how the caller's write is + * tried again. */ -export async function ensureExcludedFromGit(file: string, options: { dryRun?: boolean } = {}): Promise { +export async function ensureExcludedFromGit( + file: string, + options: { dryRun?: boolean; rerun?: string } = {}, +): Promise { + const { rerun = 'run `teamai pull` again' } = options; const tracking = await gitTracking(file); if (tracking.kind === 'ignored' || tracking.kind === 'outside-repo') return { kind: 'excluded', added: false }; - const repair = 'Fix the repository, or add the file to its .git/info/exclude yourself, then run `teamai pull` again.'; + const repair = `Fix the repository, or add the file to its .git/info/exclude yourself, then ${rerun}.`; const tracked = async (): Promise => { const named = await gitPathOf(file); return { kind: 'failed', reason: `git already tracks ${named.label}`, - fix: `Run \`git rm --cached ${named.path}\` (rotate any value a commit of it holds), then \`teamai pull\` again.`, + // After "Run `git rm …`", a second "run" is dropped: "then `teamai pull` again". + fix: `Run \`git rm --cached ${named.path}\` (rotate any value a commit of it holds), then ${rerun.replace(/^run /, '')}.`, }; }; const inIndex = await gitTracks(file); @@ -220,7 +247,7 @@ export async function ensureExcludedFromGit(file: string, options: { dryRun?: bo // Anchored at the working tree root, glob characters escaped. const rel = path.relative(dir, landed).split(path.sep).join('/'); const pattern = `/${location.prefix}${rel}`.replace(/[\\*?[\]!#]/g, '\\$&'); - const retry = `Make it writable, or add \`${pattern}\` to it yourself, then run \`teamai pull\` again.`; + const retry = `Make it writable, or add \`${pattern}\` to it yourself, then ${rerun}.`; // A read-only exclude file is the member's choice; the atomic write would replace it all the same. for (const writable of [path.dirname(excludeFile), ...(await pathExists(excludeFile) ? [excludeFile] : [])]) { const denied = await fse.access(writable, fse.constants.W_OK).then(() => false, () => true); @@ -241,7 +268,7 @@ export async function ensureExcludedFromGit(file: string, options: { dryRun?: bo if (add((await readFileSafe(excludeFile)) ?? '') !== null) { // A negated rule in a .gitignore outranks .git/info/exclude: the line would change nothing. const rule = await reincludingRule(landed); - return rule && path.basename(rule.source) === '.gitignore' ? reincluded(await gitPathOf(file), rule) : { kind: 'pending' }; + return rule && path.basename(rule.source) === '.gitignore' ? reincluded(await gitPathOf(file), rule, rerun) : { kind: 'pending' }; } result = 'unchanged'; } else { @@ -254,27 +281,27 @@ export async function ensureExcludedFromGit(file: string, options: { dryRun?: bo return { kind: 'failed', reason: `another teamai command held ${excludeFile} past the wait`, - fix: 'Run `teamai pull` again.', + fix: `${rerun.charAt(0).toUpperCase()}${rerun.slice(1)}.`, }; } if (result === 'written') log.debug(`Added ${pattern} to ${excludeFile}`); if ((await gitTracking(file)).kind !== 'would-commit') return { kind: 'excluded', added: result === 'written' }; // Untracked, as checked above: a rule git reads after teamai's line, or before it, re-includes the file. - return reincluded(await gitPathOf(file), await reincludingRule(landed)); + return reincluded(await gitPathOf(file), await reincludingRule(landed), rerun); } /** The failure for a file a rule of the member's re-includes, naming `rule` when git could. */ -function reincluded(named: { label: string }, rule: { source: string; line: string; pattern: string } | null): GitExclusion { +function reincluded(named: { label: string }, rule: { source: string; line: string; pattern: string } | null, rerun: string): GitExclusion { return rule ? { kind: 'failed', reason: `a rule in your git ignore files re-includes ${named.label}: \`${rule.pattern}\` (${rule.source}:${rule.line})`, - fix: `Remove \`${rule.pattern}\` from ${rule.source}, then run \`teamai pull\` again.`, + fix: `Remove \`${rule.pattern}\` from ${rule.source}, then ${rerun}.`, } : { kind: 'failed', reason: `a rule in your git ignore files re-includes ${named.label}`, - fix: 'Remove the rule in .gitignore, .git/info/exclude or core.excludesFile that re-includes it (`git check-ignore -v` names it), then run `teamai pull` again.', + fix: `Remove the rule in .gitignore, .git/info/exclude or core.excludesFile that re-includes it (\`git check-ignore -v\` names it), then ${rerun}.`, }; } @@ -292,12 +319,12 @@ async function reincludingRule(file: string): Promise<{ source: string; line: st * `ensureExcludedFromGit` for a file already on disk that may hold a resolved * value, warning when it fails rather than failing the sync that wrote the file. */ -export async function excludeFromGit(file: string): Promise { +export async function excludeFromGit(file: string, options: { rerun?: string; holds?: string } = {}): Promise { if (!await pathExists(file)) return; - const exclusion = await ensureExcludedFromGit(file); + const exclusion = await ensureExcludedFromGit(file, { rerun: options.rerun }); if (exclusion.kind === 'failed') { log.warn( - `${file} may hold a resolved MCP variable, and teamai could not keep it out of git: ${exclusion.reason}. ` + `${file} may hold ${options.holds ?? 'a resolved MCP variable'}, and teamai could not keep it out of git: ${exclusion.reason}. ` + `${exclusion.fix} Do not commit the file meanwhile.`, ); } diff --git a/src/mcp-reconcile.ts b/src/mcp-reconcile.ts index 7c12c7b71..507a035f1 100644 --- a/src/mcp-reconcile.ts +++ b/src/mcp-reconcile.ts @@ -44,6 +44,7 @@ import { readJson, writeJsonAtomic, readFileSafe, + readFileIfExists, pathExists, expandHome, } from './utils/fs.js'; @@ -52,6 +53,7 @@ import { warnOnce } from './utils/warn-once.js'; import { loadProjectMcpManifest } from './utils/mcp-manifest.js'; import { isOnPath, SAFE_BIN_RE, type LookPathOptions } from './utils/lookpath.js'; import { + carriesLocalAgentCredential, carriesResolvedValue, ensureExcludedFromGit, excludeFromGit, @@ -384,8 +386,15 @@ export interface JsonDoc { servers: Record; /** The existing document stores server names directly at the top level. */ bare: boolean; + /** + * A Copilot project file holding `serverKey` as well: the servers at its top level beside it. + * These may belong to the member or come from a previous bare write (#882). + */ + beside?: Record; } +const SERVER_KEYS = new Set(Object.values(MCP_SERVER_KEY)); + /** * Read a JSON MCP config. Returns null when the file exists but cannot be * parsed — we abandon the injection rather than risk clobbering a file we do @@ -407,7 +416,9 @@ export async function readJsonDoc( const bare = allowBare && !(serverKey in data); const servers = bare ? data : (data[serverKey] as Record) ?? {}; if (typeof servers !== 'object' || servers === null || Array.isArray(servers)) return null; - return { data, servers: { ...servers }, bare }; + const beside = allowBare && !bare ? Object.fromEntries(Object.entries(data).filter(([key, value]) => + !SERVER_KEYS.has(key) && typeof value === 'object' && value !== null && !Array.isArray(value))) : {}; + return { data, servers: { ...servers }, bare, ...Object.keys(beside).length > 0 ? { beside } : {} }; } catch { return null; } @@ -624,11 +635,17 @@ export function desiredMcpForTarget( * team's server arrived: the appliers refuse to overwrite an entry teamai does * not own, so an unrelated server of the same name leaves the key there and the * team's definition undelivered. Only the value tells those two apart. + * A Copilot project file's bare servers beside `mcpServers` count as well + * (#882): what the file holds, not only what the tool reads. * * Read-only. An MCP server is an entry inside a tool's config rather than a * file of its own, so this, not a destination path, is what "delivered" means. */ -export async function installedMcpEntries(target: McpTarget): Promise | null> { +export async function installedMcpEntries( + target: McpTarget, + /** Only the servers under the format's key, as the tool reads them: not a Copilot file's bare ones beside it. */ + options: { underKeyOnly?: boolean } = {}, +): Promise | null> { if (target.format === 'codex') { const raw = await readFileSafe(target.file); if (raw === null) return new Map(); @@ -637,7 +654,17 @@ export async function installedMcpEntries(target: McpTarget): Promise]; const allowBare = target.format === 'copilot' && target.projectScope; const doc = await readJsonDoc(target.file, serverKey, allowBare); - return doc === null ? null : new Map(Object.entries(doc.servers)); + if (doc === null) return null; + return new Map([...options.underKeyOnly ? [] : Object.entries(doc.beside ?? {}), ...Object.entries(doc.servers)]); +} + +/** In a Copilot project file that also holds `mcpServers`, a bare server whose value differs from the one of its name there. */ +async function shadowedBareCopilotServer(target: McpTarget): Promise { + if (target.format !== 'copilot' || !target.projectScope) return undefined; + const doc = await readJsonDoc(target.file, MCP_SERVER_KEY.copilot, true).catch(() => null); + if (!doc?.beside) return undefined; + return Object.keys(doc.beside).find((name) => doc.servers[name] !== undefined + && JSON.stringify(doc.servers[name]) !== JSON.stringify(doc.beside?.[name])); } /** @@ -716,6 +743,10 @@ export async function resolvedValueEvidence( const present = records.map((record) => record.name); const unverified = (ledger.unverified ?? []).find((name) => !installed || installed.has(name)); if (unverified) return `${unverified}, which was in the file when teamai rebuilt its lost record, so teamai cannot tell whether a pull wrote it`; + // A Copilot file's bare server beside a different one of its name under mcpServers: the merged view reads the + // latter, and the bare copy may be one an earlier pull wrote with a value since resolved away (#882). + const shadowed = await shadowedBareCopilotServer(target); + if (shadowed) return `a bare ${shadowed} beside a different ${shadowed} under mcpServers, which may be an earlier pull's`; if (!teamDefs) return present.length > 0 ? `teamai's ${present.join(', ')}, and the team's MCP servers cannot be read` : null; const dropped = present.find((name) => !teamDefs.some((def) => def.name === name)); if (dropped) return `teamai's ${dropped}, which has left the team's MCP servers`; @@ -809,16 +840,20 @@ const EARLIER_BUILTIN_MCP_PROJECT = { export async function earlierMappedMcpTargets( cfg: LocalConfig, known: McpTarget[], + /** `history: false`: only the built-in defaults, for a team with no teamai.yaml (HTTP-backed). */ + options: { history?: boolean } = {}, ): Promise | null> { const { projectRoot } = cfg; if (!projectRoot) return []; const repoPath = cfg.repo.localPath; - let revisions: string[]; - try { - revisions = (await createGit(repoPath).raw(['log', '--format=%H', 'HEAD', '--', 'teamai.yaml'])).split('\n').filter(Boolean); - } catch (e) { - log.debug(`Could not read the history of teamai.yaml in ${repoPath}: ${e instanceof Error ? e.message : String(e)}. The next pull tries again.`); - return null; + let revisions: string[] = []; + if (options.history !== false) { + try { + revisions = (await createGit(repoPath).raw(['log', '--format=%H', 'HEAD', '--', 'teamai.yaml'])).split('\n').filter(Boolean); + } catch (e) { + log.debug(`Could not read the history of teamai.yaml in ${repoPath}: ${e instanceof Error ? e.message : String(e)}. The next pull tries again.`); + return null; + } } const root = await realFilePath(projectRoot); // Each path, by real path, with the tools today's targets or the record reach it for. @@ -875,7 +910,7 @@ async function mcpFileState(targets: McpTarget[]): Promise { +export async function recordedMcpFileEvidence(targets: McpTarget[], owned?: McpOwnedFor): Promise { const state = await mcpFileState(targets); if (state.kind === 'unparsable') return 'it does not parse'; if (state.kind !== 'parsed') return null; @@ -884,10 +919,53 @@ export async function recordedMcpFileEvidence(targets: McpTarget[], owned?: read ? 'teamai may have written a resolved value to it under an earlier toolPaths mapping, and it still holds MCP servers' : null; } - const other = state.servers.find((name) => !owned.includes(name)); - return other === undefined ? null - : `teamai may have written a resolved value to it for ${targets.map((t) => t.tool).join(', ')} under an earlier toolPaths mapping, ` - + `and it holds ${other}, which no tool that maps it now owns`; + // Each target's key read alone: another key's owner proves nothing of it (OpenCode's `mcp` beside `mcpServers`). + for (const target of targets) { + const placed = await mcpEntriesByPlacement(target); + const other = [...placed?.keyed.keys() ?? []].find((name) => !owned(target).includes(name)) + ?? [...placed?.bare.keys() ?? []].find((name) => !owned(target, { bare: true }).includes(name)); + if (other !== undefined) { + return `teamai may have written a resolved value to it for ${targets.map((t) => t.tool).join(', ')} under an earlier toolPaths mapping, ` + + `and it holds ${other}, which no tool that maps it now owns`; + } + } + return null; +} + +/** + * `target`'s servers under its format's key, and apart, a Copilot project file's bare ones, or null when + * the file does not parse (#882). `installedMcpEntries` merges the two by name, the keyed one winning: a + * bare server beside one of its name under `mcpServers` is judged on its own here. + */ +async function mcpEntriesByPlacement(target: McpTarget): Promise<{ keyed: Map; bare: Map } | null> { + if (target.format !== 'copilot' || !target.projectScope) { + const keyed = await installedMcpEntries(target); + return keyed && { keyed, bare: new Map() }; + } + const doc = await readJsonDoc(target.file, MCP_SERVER_KEY.copilot, true); + if (!doc) return null; + const entries = (servers: Record | undefined): Map => new Map(Object.entries(servers ?? {})); + return doc.bare ? { keyed: new Map(), bare: entries(doc.servers) } : { keyed: entries(doc.servers), bare: entries(doc.beside) }; +} + +/** + * For a target of a file other tools map today, the servers their records own under its key, or, with + * `bare`, at a Copilot project file's top level, where only Copilot writes. + */ +export type McpOwnedFor = (target: McpTarget, options?: { bare?: boolean }) => readonly string[]; + +/** + * `McpOwnedFor` from `mappedBy`, the tools a file's mapping reaches today: only the records of those that + * keep their servers under the judged target's key count (#882). Undefined when no tool maps it today. + */ +export function ownedByMappers(mappedBy: readonly string[], manifest: ManagedMcpManifest | undefined): McpOwnedFor | undefined { + if (mappedBy.length === 0) return undefined; + return (target, options = {}) => mappedBy + .filter((tool) => { + const format = detectMcpFormat(tool); + return format !== null && (options.bare ? format === 'copilot' : sameServerKey(format, target.format)); + }) + .flatMap((tool) => manifest?.[managedMcpManifestKey(tool, true)] ?? []).map((record) => record.name); } /** @@ -902,7 +980,7 @@ export async function earlierMappedMcpFileEvidence( teamDefs: McpServerDef[] | null, vars: Record, ctx: () => Promise, - owned?: readonly string[], + owned?: McpOwnedFor, ): Promise { return await recordedMcpFileEvidence([target], owned) ?? await resolvedValueEvidence(target, teamDefs, { owned: [] }, vars, ctx); } @@ -928,12 +1006,11 @@ async function observeMcpConfigs( for (const [file, { targets: group, mappedBy, tracked }] of await recordedMcpTargets(localConfig, targets)) { const state = await mcpFileState(group); const stillTracked = tracked && (await gitTracks(file)).kind === 'tracked'; - const owned = mappedBy.length === 0 ? undefined - : mappedBy.flatMap((tool) => manifest[managedMcpManifestKey(tool, true)] ?? []).map((record) => record.name); + const owned = ownedByMappers(mappedBy, manifest); const holding = !stillTracked && await recordedMcpFileEvidence(group, owned) !== null; - for (const { tool } of group) { + for (const target of group) { observations.push({ - file, tool, state, holding, owned: owned ?? [], + file, tool: target.tool, state, holding, owned: owned ? [...owned(target)] : [], ...tracked ? { tracked: stillTracked } : {}, ...owned && !stillTracked ? { remapped: true as const } : {}, }); @@ -1016,6 +1093,8 @@ export async function mcpConfigsNotProvenClean( mappers: Set; mapsToday: Set; proven: Set; writers: Set; /** Every tool's target on this file: tools of different formats read different keys of it. */ all: McpTarget[]; + /** What each of those tools' records own there, by format. */ + ownedByFormat: Array<{ format: McpFormat; names: string[] }>; }>(); const realRoot = (root: string | undefined): Promise => root ? fs.promises.realpath(root).catch(() => root) : Promise.resolve(undefined); @@ -1065,6 +1144,7 @@ export async function mcpConfigsNotProvenClean( proven, writers, all: [...seen?.all ?? [], target], + ownedByFormat: [...seen?.ownedByFormat ?? [], { format: target.format, names: owned.map((record) => record.name) }], foreign: foreign || seen?.foreign === true, }); } @@ -1104,7 +1184,9 @@ export async function mcpConfigsNotProvenClean( continue; } const moved = remapped.get(file); - const movedWhy = moved && await recordedMcpFileEvidence(moved, targets.get(file)?.owned.map((record) => record.name) ?? []); + const mappedHereNow = targets.get(file); + const movedWhy = moved && await recordedMcpFileEvidence(moved, (target) => (mappedHereNow?.ownedByFormat ?? []) + .filter((o) => sameServerKey(o.format, target.format)).flatMap((o) => o.names)); if (movedWhy) { held.set(file, movedWhy); continue; @@ -1167,11 +1249,31 @@ export async function reconcileMcpForConfig( // The (file, tool) pairs managed-mcp-files.json first recorded this run, before their write, until that // tool's records hold a resolved value there: another tool's write to the same file proves nothing of it. const recorded: McpTarget[] = []; + // One snapshot per file, before any tool writes it, until ownership is saved. + const restoreConfigs = new Map Promise>(); const protect = !options.removeAll && !options.dryRun; // Read before the reconcile records what it writes: a manifest it recreates says nothing of what came before. const before = protect && localConfig.projectRoot ? await readProjectMcpManifest(localConfig, localConfig.projectRoot) : undefined; try { - return await reconcileTargets(teamConfig, localConfig, options, exclusions, written, recorded); + return await reconcileTargets(teamConfig, localConfig, options, exclusions, written, recorded, restoreConfigs); + } catch (error) { + const failures: string[] = []; + for (const [file, restore] of restoreConfigs) { + try { + await restore(); + written.delete(file); + } catch (restoreError) { + failures.push(`${file}: ${restoreError instanceof Error ? restoreError.message : String(restoreError)}`); + } + } + if (failures.length > 0) { + throw new Error( + `MCP sync failed (${error instanceof Error ? error.message : String(error)}), and restoring configs failed (${failures.join('; ')}). ` + + 'Their ownership records may not match. Repair the configs and ownership records before retrying the command.', + { cause: error }, + ); + } + throw error; } finally { // A record this run added for a tool that then wrote no value goes, as its exclude line does. The settle // below records the file again if it holds a resolved value all the same (an earlier pull wrote it). @@ -1186,7 +1288,9 @@ export async function reconcileMcpForConfig( * `.git/info/exclude` (#882), and take out the line of one proven clean. It * covers what is on disk, whether or not this run delivered to it: the file of * a disabled or undetected tool, or one written before the team turned - * delivery off, still holds what a pull wrote. + * delivery off, still holds what a pull wrote. For an HTTP-backed team, whose + * servers no pull writes, each config that may hold a credential its local + * agent wrote (`protectLocalAgentMcpConfigs`). */ async function protectResolvedMcpConfigs( teamConfig: TeamaiConfig, @@ -1196,9 +1300,11 @@ async function protectResolvedMcpConfigs( before: ManagedMcpManifest | undefined, ): Promise { const { projectRoot } = localConfig; - if (localConfig.scope !== 'project' || !projectRoot || localConfig.repo.kind === 'http') return; + if (localConfig.scope !== 'project' || !projectRoot) return; try { - await protectProjectMcpConfigs(teamConfig, localConfig, projectRoot, exclusions, written, before); + await (localConfig.repo.kind === 'http' + ? protectLocalAgentMcpConfigs(teamConfig, localConfig) + : protectProjectMcpConfigs(teamConfig, localConfig, projectRoot, exclusions, written, before)); } catch (e) { log.warn( `Could not check this project's MCP configs for resolved values to keep out of git: ${e instanceof Error ? e.message : String(e)}. ` @@ -1207,6 +1313,77 @@ async function protectResolvedMcpConfigs( } } +/** + * The targets among `targets`, those managed-mcp-files.json recorded under + * a mapping another teamai.yaml made, and the built-in defaults teamai has + * since changed, whose project MCP config may hold a credential an HTTP-backed + * team's local agent wrote (#882). No mcp.yaml to judge by: a server its + * install recorded as carrying a credential, or an older install's entry + * carrying one (a header, env value, argument or URL), or one whose record was + * lost while another server's remains. Each entry is judged on its own, a + * Copilot file's bare one apart from the one of its name under mcpServers. With no record + * of the tool at all, a file managed-mcp-files.json lists holds while it holds + * any server, or doesn't parse: nothing says which of them the local agent + * wrote. A file two tools map may appear once for each. Read-only. + */ +export async function localAgentCredentialFiles(localConfig: LocalConfig, targets: McpTarget[]): Promise { + const { projectRoot } = localConfig; + if (localConfig.scope !== 'project' || !projectRoot) return []; + const { manifest } = await loadProjectMcpManifest(getDataHome(localConfig), projectRoot, { dryRun: true }); + const ledger = (await readResolvedMcpFiles(localConfig)).files; + const recorded = [...(await recordedMcpTargets(localConfig, targets)).values()].flatMap((file) => file.targets); + // An older agent wrote to a built-in default teamai has since changed; an HTTP team has no teamai.yaml history. + const earlier = await earlierMappedMcpTargets(localConfig, targets, { history: false }) ?? []; + const held: McpTarget[] = []; + for (const target of [...targets, ...recorded, ...earlier]) { + if (!await pathExists(target.file)) continue; + const placed = await mcpEntriesByPlacement(target); + const entries = placed && [...placed.keyed, ...placed.bare]; + const records = manifest[managedMcpManifestKey(target.tool, true)]; + if (records === undefined) { + if (ledger[target.file] !== undefined && (entries === null || entries.length > 0)) held.push(target); + continue; + } + const byName = new Map(records.map((record) => [record.name, record])); + const credential = entries === null + ? records.some((record) => record.resolved !== false) + : entries.some(([name, entry]) => { + const record = byName.get(name); + // `resolved: false` speaks for the entry its install wrote: an older one a failed write left is judged by what it holds. + const noted = record && (record.resolved === true || entryHash(entry) === record.hash) ? record.resolved : undefined; + return noted ?? carriesLocalAgentCredential(entry); + }); + if (credential) held.push(target); + } + return held; +} + +/** + * For an HTTP-backed team: list in `.git/info/exclude` each project MCP + * config that may hold a credential its local agent wrote + * (`localAgentCredentialFiles`), and record it in managed-mcp-files.json + * (#882). An older local agent wrote one without listing it, and no install + * runs again for a server already in place. Only `teamai uninstall` takes + * such a line out. The caller skips a dry run. + */ +export async function protectLocalAgentMcpConfigs( + teamConfig: TeamaiConfig, + localConfig: LocalConfig, + options: { rerun?: string } = {}, +): Promise { + const mapped = await resolveMcpTargets(teamConfig, localConfig, { includeUndetected: true }); + const unmapped = await unmappedMcpDefaults(mapped); + const held = await localAgentCredentialFiles(localConfig, mapped.filter((target) => !unmapped.has(target))); + if (held.length === 0) return; + for (const file of new Set(held.map((target) => target.file))) await excludeFromGit(file, { rerun: options.rerun, holds: 'a credential' }); + // A failure does not undo the line: the exclusion protects the file. + const result = await trackResolvedMcpFiles(localConfig, held.map(({ tool, file }) => ({ tool, file }))) + .catch((e: unknown) => e instanceof Error ? e.message : String(e)); + if (result !== 'written' && result !== 'unchanged') { + log.debug(`Did not record ${held.map((target) => target.file).join(', ')} in managed-mcp-files.json: ${result === 'locked' ? 'another teamai command held it past the wait' : result}.`); + } +} + async function protectProjectMcpConfigs( teamConfig: TeamaiConfig, localConfig: LocalConfig, @@ -1265,11 +1442,10 @@ async function protectProjectMcpConfigs( continue; } // In a file other tools map today, their records tell their own servers. - const owned = mappedBy.length === 0 ? undefined - : mappedBy.flatMap((tool) => manifest[managedMcpManifestKey(tool, true)] ?? []).map((record) => record.name); + const owned = ownedByMappers(mappedBy, manifest); const holding = await earlierMappedMcpFileEvidence(target, teamDefs, vars, ctx, owned) !== null; if (holding) found.push(target.file); - observations.push({ file: target.file, tool: target.tool, state, holding, owned: owned ?? [], ...owned ? { remapped: true as const } : {} }); + observations.push({ file: target.file, tool: target.tool, state, holding, owned: owned ? [...owned(target)] : [], ...owned ? { remapped: true as const } : {} }); } const holding = new Set(observations.filter((o) => o.holding).map((o) => o.file)); const unproven = new Set(observations.filter((o) => !o.holding).map((o) => o.file)); @@ -1417,6 +1593,7 @@ async function reconcileTargets( exclusions: Map, written: Set, recorded: McpTarget[], + restoreConfigs: Map Promise>, ): Promise { const changes: McpChange[] = []; let wrote = false; @@ -1512,8 +1689,8 @@ async function reconcileTargets( } const wroteTarget = target.format === 'codex' - ? await applyCodex(target, desired, keep, ownedNames, nextRecords, changes, options) - : await applyJson(target, desired, keep, owned, ownedNames, nextRecords, changes, options); + ? await applyCodex(target, desired, keep, ownedNames, nextRecords, changes, options, restoreConfigs) + : await applyJson(target, desired, keep, owned, ownedNames, nextRecords, changes, options, restoreConfigs); if (wroteTarget) written.add(target.file); wrote = wroteTarget || wrote; // Not read: its record stays as it was, or absent. An empty one would say teamai owns nothing there (#882). @@ -1529,8 +1706,6 @@ async function reconcileTargets( if (marked) record.unnoted = true; else delete record.unnoted; } - const at = recorded.indexOf(target); - if (at >= 0 && nextRecords.some((record) => record.resolved === true)) recorded.splice(at, 1); } // Rebuilt this run, or by one that could not note what else was in the file. const unnoted = manifest[manifestKey] === undefined || manifest[manifestKey].some((record) => record.unnoted); @@ -1546,7 +1721,8 @@ async function reconcileTargets( // unnoted until protectProjectMcpConfigs notes that server, after its settle records the file. for (const target of targets.filter((t) => unrecorded.has(t.tool))) { const records = manifest[managedMcpManifestKey(target.tool, true)] ?? []; - const claimed = targets.filter((t) => t.file === target.file) + // Only tools reading the same key claim: another key's owner proves nothing of this one's (#882). + const claimed = targets.filter((t) => t.file === target.file && sameServerKey(t.format, target.format)) .flatMap((t) => manifest[managedMcpManifestKey(t.tool, true)] ?? []).map((record) => record.name); if (records.length > 0 && (await unclaimedMcpServers(target, claimed)).length > 0) { for (const record of records) record.unnoted = true; @@ -1563,6 +1739,12 @@ async function reconcileTargets( } await writeJsonAtomic(manifestPath, manifest); } + // Only committed ownership retains a file record added by this run. On failure, + // the outer cleanup removes it before inspecting the restored configs. + for (let index = recorded.length - 1; index >= 0; index--) { + const target = recorded[index]; + if (manifest[managedMcpManifestKey(target.tool, target.projectScope)]?.some((record) => record.resolved === true)) recorded.splice(index, 1); + } return { changes, wrote }; } @@ -1619,6 +1801,34 @@ async function forgetUnwrittenMcpConfigs(localConfig: LocalConfig, targets: McpT } } +/** + * Whether the bare Copilot server `name` beside `mcpServers` is the copy a teamai write left before another tool + * added the key (#882): a completed bare write in `owned`, with matching content. A member's own server of that name, + * or one edited since, is left alone. + */ +export function isTeamaiBareCopy(doc: { beside?: Record }, name: string, owned: readonly ManagedMcpRecord[]): boolean { + const bare = doc.beside?.[name]; + return bare !== undefined && owned.some((record) => record.name === name && record.bare === true && record.hash === entryHash(bare)); +} + +/** Copilot project ownership without placement needs a matching, unambiguous entry. */ +export function ownsJsonMcpEntry( + doc: Pick, + name: string, + owned: readonly ManagedMcpRecord[], + allowBare: boolean, +): boolean { + return owned.some((record) => { + if (record.name !== name) return false; + if (!allowBare) return record.bare !== true; + if (record.bare !== undefined) return record.bare === doc.bare; + const entry = doc.servers[name]; + const beside = doc.beside?.[name]; + return entry !== undefined && record.hash === entryHash(entry) + && (beside === undefined || record.hash !== entryHash(beside)); + }); +} + // ─── Appliers ──────────────────────────────────────────────── /** Whether it wrote `target`'s file; null when the file does not parse, and so was not read. */ @@ -1631,6 +1841,7 @@ async function applyJson( nextRecords: ManagedMcpRecord[], changes: McpChange[], options: McpReconcileOptions, + restoreConfigs: Map Promise>, ): Promise { const serverKey = MCP_SERVER_KEY[target.format as Exclude]; const allowBare = target.format === 'copilot' && target.projectScope; @@ -1639,27 +1850,41 @@ async function applyJson( log.warn(`Could not parse ${target.file} — skipping MCP injection for ${target.tool}`); return null; } + const existed = await pathExists(target.file); + const previousData = structuredClone(doc.data); - const ownedHash = new Map(owned.map((r) => [r.name, r.hash])); + const ownedHere = owned.filter((record) => ownsJsonMcpEntry(doc, record.name, [record], allowBare)); + const ownedHash = new Map(ownedHere.map((r) => [r.name, r.hash])); let dirty = false; // A kept entry holds the value an earlier pull resolved (desiredMcpForTarget). let holdsResolvedValue = false; for (const [name, { entry, hash, resolvedValue }] of desired) { const existing = doc.servers[name]; - if (existing !== undefined && !ownedNames.has(name) && !options.force) { + if (existing !== undefined && !ownsJsonMcpEntry(doc, name, owned, allowBare) && !options.force) { changes.push({ tool: target.tool, server: name, action: 'skipped', reason: 'a server with this name already exists and is not managed by teamai', }); + const previous = owned.find((record) => record.name === name); + if (previous) nextRecords.push(previous); continue; } - nextRecords.push({ name, hash }); + const record: ManagedMcpRecord = { name, hash }; + if (allowBare && !doc.bare) record.bare = false; + if (doc.bare && owned.some((r) => r.name === name && r.bare === true)) record.bare = true; + nextRecords.push(record); holdsResolvedValue ||= resolvedValue; + // The copy a bare write left before another tool added the key would keep the old value beside this one (#882). + if (isTeamaiBareCopy(doc, name, owned)) { + delete doc.data[name]; + dirty = true; + } if (existing !== undefined && ownedHash.get(name) === hash) continue; doc.servers[name] = entry; + if (doc.bare) record.bare = true; dirty = true; changes.push({ tool: target.tool, server: name, action: existing === undefined ? 'added' : 'updated' }); } @@ -1667,15 +1892,20 @@ async function applyJson( for (const name of ownedNames) { if (desired.has(name)) continue; const kept = keep.get(name); - if (kept && doc.servers[name] !== undefined) { + if (kept && ((ownsJsonMcpEntry(doc, name, owned, allowBare) && doc.servers[name] !== undefined) || isTeamaiBareCopy(doc, name, owned))) { nextRecords.push(kept); holdsResolvedValue = true; continue; } - if (doc.servers[name] !== undefined) { + if (ownsJsonMcpEntry(doc, name, owned, allowBare) && doc.servers[name] !== undefined) { delete doc.servers[name]; dirty = true; } + // One a bare write left before another tool added the key goes too (#882). + if (isTeamaiBareCopy(doc, name, owned)) { + delete doc.data[name]; + dirty = true; + } changes.push({ tool: target.tool, server: name, action: 'removed' }); } @@ -1691,6 +1921,11 @@ async function applyJson( // empty `mcpServers` in a file the tool never reads under that name. // A file that holds a resolved value is the member's alone, an existing one tightened. await writeJsonDoc(target.file, serverKey, doc, holdsResolvedValue ? { mode: 0o600 } : undefined); + if (!restoreConfigs.has(target.file)) { + restoreConfigs.set(target.file, existed + ? () => writeJsonAtomic(target.file, previousData) + : () => fs.promises.rm(target.file, { force: true })); + } return true; } @@ -1702,8 +1937,10 @@ async function applyCodex( nextRecords: ManagedMcpRecord[], changes: McpChange[], options: McpReconcileOptions, + restoreConfigs: Map Promise>, ): Promise { - let source = (await readFileSafe(target.file)) ?? ''; + const previous = await readFileIfExists(target.file); + let source = previous ?? ''; const present = new Set(codexServerNames(source)); let dirty = false; let holdsResolvedValue = false; @@ -1750,6 +1987,11 @@ async function applyCodex( } await writeCodexAtomic(target.file, source); + if (!restoreConfigs.has(target.file)) { + restoreConfigs.set(target.file, previous === null + ? () => fs.promises.rm(target.file, { force: true }) + : () => writeCodexAtomic(target.file, previous)); + } return true; } diff --git a/src/types.ts b/src/types.ts index b6a78f911..8d5f49f3f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -904,6 +904,8 @@ export interface ManagedMcpRecord { * (#882). Absent in records an older teamai wrote. */ resolved?: boolean; + /** Completed Copilot project write: true for bare, false for keyed; absent means unproven. */ + bare?: boolean; /** * Project scope: this record was rebuilt after it was lost, or written by a * pull that found no managed-mcp.json, and the other servers in its file