mirror of
https://github.com/Tencent/teamai-cli.git
synced 2026-10-02 03:14:40 +08:00
#882's pre-write exclusion is now the single gate for a project MCP config git would commit. #880's separate tracked-file check (gitTracks in buildDesiredMcpContext, `withheld` in desiredMcpForTarget, its warning, mcp list lines and doctor note) is removed: a tracked file is one more way the exclusion fails, reported once with `git rm --cached <file>` and rotate. A dry run (mcp list, doctor) now also names a tracked file before any pull has listed it, mcp list reports a withheld file with an entry already installed, and doctor reports withheld servers without the pull --force text. A tracked file now gets no resolved value, declared secret or not.
This commit is contained in:
Saul Moro
@@ -144,7 +144,7 @@ the member's value for this team teamai env set KEY [--from-env VAR]
|
||||
|
||||
**Still reachable.** The resolved value is written in plaintext to each tool's MCP config, as before. A config that holds a resolved `${VAR}` value is written `0600`, an existing wider one (`.mcp.json` is often `0644`) included, and a pull that changes nothing in it still tightens it to `0600` without rewriting it; one without such a value keeps its mode, and a new one is created `0600`. A command run under `env exec` gets it in its environment, and so does every process it starts: an agent that runs `teamai env exec -- env` can read it. The agent skills forbid that, but nothing enforces it. This keeps secrets out of git, not away from the member's machine or the agent running on it.
|
||||
|
||||
**Out of git.** A project-scope MCP config that holds a resolved value is listed in the clone's `.git/info/exclude` (#882), which stops `git add` but not a file git already tracks. So a declared secret's value is never written into a project config `git ls-files` tracks: that server is skipped for that tool, an entry an earlier pull wrote there stays as it is, and `pull` (a warning), `teamai mcp list` (`withheld:`) and `teamai doctor` (`MCP servers delivered to <tool>` fails) name the file and the fix: `git rm --cached <file>`, and rotate the token if it was ever committed. A variable the team does not declare as a secret is written as before. A repository git cannot answer for is not treated as tracking the file: a commit fails there too.
|
||||
**Out of git.** A resolved value lands in a project-scope MCP config only once the clone's `.git/info/exclude` lists the file (#882). An exclude rule does not stop a file git already tracks, so no resolved value, declared secret or not, is written into a project config `git ls-files` tracks: pull leaves that file as it was (an entry an earlier pull wrote stays), and `pull` (a warning), `teamai mcp list` (`withheld:`) and `teamai doctor` (`MCP servers delivered to <tool>` fails) name the file and the fix: `git rm --cached <file>`, and rotate the token if it was ever committed. An exclusion that fails for another reason (`.git/info` or the exclude file not writable, another teamai command holding it, a git error) leaves the file as it was the same way, with that reason and its fix.
|
||||
|
||||
## A missing secret keeps the MCP entry
|
||||
|
||||
|
||||
@@ -144,7 +144,7 @@ teamai env unset GITHUB_TOKEN [--global]
|
||||
|
||||
**仍可访问。** 解析后的值仍以明文写入各工具的 MCP 配置。含有已解析 `${VAR}` 值的配置以 `0600` 写入，已有的更宽权限文件（`.mcp.json` 常为 `0644`）也会收紧，即使 pull 没有改动其中任何内容，也会在不重写文件的情况下收紧为 `0600`；不含这类值的配置保持原权限，新文件以 `0600` 创建。在 `env exec` 下运行的命令会在环境变量中拿到它，它启动的每个进程也一样：agent 运行 `teamai env exec -- env` 就能读到。agent skill 禁止这样做，但没有任何机制强制。这让密钥不进入 git，而不是让它远离成员的机器或在上面运行的 agent。
|
||||
|
||||
**不进入 git。** 含有已解析值的项目级 MCP 配置会被写入本地克隆的 `.git/info/exclude`（#882），这能阻止 `git add`，但挡不住 git 已跟踪的文件。因此已声明密钥的值永远不会写入 `git ls-files` 已跟踪的项目配置：该工具跳过这个 server，之前 pull 写入的条目保持不变，`pull`（警告）、`teamai mcp list`（`withheld:`）和 `teamai doctor`（`MCP servers delivered to <tool>` 失败）会指出该文件和修复方法：`git rm --cached <file>`，如果它曾随 token 一起提交过，还要轮换 token。团队未声明为密钥的变量照旧写入。git 无法判断的仓库不视为跟踪了该文件：在那里提交同样会失败。
|
||||
**不进入 git。** 只有在本地克隆的 `.git/info/exclude` 列出某个项目级 MCP 配置之后，解析后的值才会写入该文件（#882）。exclude 规则挡不住 git 已跟踪的文件，因此无论是否为已声明密钥，解析后的值都不会写入 `git ls-files` 已跟踪的项目配置：pull 保持该文件原样（之前 pull 写入的条目保留），`pull`（警告）、`teamai mcp list`（`withheld:`）和 `teamai doctor`（`MCP servers delivered to <tool>` 失败）会指出该文件和修复方法：`git rm --cached <file>`，如果它曾随 token 一起提交过，还要轮换 token。因其他原因无法排除时（`.git/info` 或 exclude 文件不可写、另一个 teamai 命令占用它、git 出错），同样保持该文件原样，并给出对应的原因与修复方法。
|
||||
|
||||
## 缺少密钥时保留 MCP 条目
|
||||
|
||||
|
||||
+1
-1
@@ -1244,7 +1244,7 @@ Copilot uses its native `mcpServers` schema: `stdio` becomes `type: "local"`, re
|
||||
|
||||
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 holds 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). That covers a file this pull did not write: one written earlier for a tool since disabled, or dropped from `toolPaths` (at the tool's built-in location), or one still holding a server since removed from `mcp.yaml`. A path git cannot answer for is listed all the same, or teamai warns; so does a pull that finds another teamai command holding the exclude file past a short wait, which writes nothing (run `teamai pull` again). The committed `.gitignore` is left alone, a path git already ignores adds nothing, and `teamai uninstall` removes a path from the block (the block with its last path) once that file, in every worktree of its repository, 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, 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`) still present. Otherwise, or for a file it cannot check (for example one that does not parse), it keeps the path and 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 config git already tracks never gets a declared secret's value (`env/secrets.yaml`): teamai skips that server for that tool, keeps the entry an earlier pull wrote as it is, and `pull`, `teamai mcp list` and `teamai doctor` name the file and the fix (`git rm --cached <file>`, then 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. That covers a file this pull did not write: one written earlier for a tool since disabled, or dropped from `toolPaths` (at the tool's built-in location), or one still holding a server since removed from `mcp.yaml`. A path git cannot answer for is listed all the same. 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, 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; make the file writable (or `git rm --cached` the tracked file and rotate the token) and run `teamai pull` again. The committed `.gitignore` is left alone, a path git already ignores adds nothing, and `teamai uninstall` removes a path from the block (the block with its last path) once that file, in every worktree of its repository, 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, 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`) still present. Otherwise, or for a file it cannot check (for example one that does not parse), it keeps the path and 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.
|
||||
|
||||
Claude Code may show project `.mcp.json` servers as pending approval until you accept them once in an interactive session.
|
||||
|
||||
|
||||
@@ -1126,7 +1126,7 @@ Copilot 使用原生 `mcpServers` 结构：`stdio` 写成 `type: "local"`，远
|
||||
|
||||
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 共用该文件）。本次 pull 未写入的文件同样适用：之前为某个现已禁用、或已从 `toolPaths` 移除的工具写入的文件（按该工具的内置位置查找），或仍含已从 `mcp.yaml` 删除的 server 的文件。git 无法判断的路径也会照样写入，否则 teamai 会给出警告；若 pull 在短暂等待后仍发现另一个 teamai 命令占用该 exclude 文件，则不写入并给出警告（请再次运行 `teamai pull`）。不会改动已提交的 `.gitignore`，git 已忽略的路径不会重复添加，`teamai uninstall` 会从块中移除某个路径（移除最后一个路径时连同整个块），前提是该文件（在其仓库的每个 worktree 中）已不存在、不含任何 MCP server，或在 teamai 的写入记录（`managed-mcp.json`）仍在的情况下不含以下任何一项：带解析值的团队 server、清理后仍残留的 teamai 条目、仍在环境中设置的变量的值（8 个字符以上）。否则，或对无法检查的文件（例如无法解析），会保留该路径并给出警告，说明文件及原因：请先从中移除 teamai 的 server，再自行删除那一行（删到最后一行时连同块的首尾标记）。`teamai doctor` 会报告 git 仍会提交或无法判断的这类文件——例如已被跟踪的文件：请 `git rm --cached` 并轮换 token。git 已跟踪的配置永远不会写入已声明密钥（`env/secrets.yaml`）的值：teamai 对该工具跳过这个 server，保留之前 pull 写入的条目不变，`pull`、`teamai mcp list` 和 `teamai doctor` 会指出该文件和修复方法（`git rm --cached <file>`，然后轮换 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 共用该文件）。本次 pull 未写入的文件同样适用：之前为某个现已禁用、或已从 `toolPaths` 移除的工具写入的文件（按该工具的内置位置查找），或仍含已从 `mcp.yaml` 删除的 server 的文件。git 无法判断的路径也会照样写入。若无法写入——`.git/info` 或 exclude 文件不可写、另一个 teamai 命令在短暂等待后仍占用 exclude 文件、git 已跟踪该文件，或 git 出错——teamai 会保持该文件原样（之前 pull 写入的条目保留），给出原因与修复方法的警告，`teamai mcp list` 和 `teamai doctor` 也会把该 server 报告为未写入（withheld）；请让文件可写（或对已跟踪的文件执行 `git rm --cached` 并轮换 token），再运行 `teamai pull`。不会改动已提交的 `.gitignore`，git 已忽略的路径不会重复添加，`teamai uninstall` 会从块中移除某个路径（移除最后一个路径时连同整个块），前提是该文件（在其仓库的每个 worktree 中）已不存在、不含任何 MCP server，或在 teamai 的写入记录（`managed-mcp.json`）仍在的情况下不含以下任何一项：带解析值的团队 server、清理后仍残留的 teamai 条目、仍在环境中设置的变量的值（8 个字符以上）。否则，或对无法检查的文件（例如无法解析），会保留该路径并给出警告，说明文件及原因：请先从中移除 teamai 的 server，再自行删除那一行（删到最后一行时连同块的首尾标记）。`teamai doctor` 会报告 git 仍会提交或无法判断的这类文件——例如已被跟踪的文件：请 `git rm --cached` 并轮换 token。
|
||||
|
||||
Claude Code 可能把来自仓库的 `.mcp.json` 标为待批准，需在交互式会话中确认一次。
|
||||
|
||||
|
||||
@@ -82,14 +82,17 @@ reference to VAR (`--from-env`) and VAR is unset in this environment. Ask the
|
||||
user whether to set VAR in their shell or replace the reference with the
|
||||
command in the line; do not choose for them.
|
||||
|
||||
## "MCP server X not written: <file> is tracked by git"
|
||||
## "Did not write <tool>'s MCP servers to <file>" / `withheld:`
|
||||
|
||||
`pull`, `teamai mcp list` (`withheld:`) and `teamai doctor` print this when a
|
||||
project MCP config git already tracks would get a declared secret's value.
|
||||
That server is skipped for that tool, and an entry an earlier pull wrote
|
||||
stays as it is. Tell the user: `git rm --cached <file>` (the file stays on
|
||||
disk), commit that, and rotate the token if the file was ever committed with
|
||||
it; then `teamai pull`. Do not run `git rm` or commit for them.
|
||||
`pull` prints this, and `teamai mcp list` (`withheld:`) and `teamai doctor`
|
||||
report it, when a project MCP config would get a resolved `${VAR}` value that
|
||||
git would commit: the file could not be kept out of git first. It is left as
|
||||
it was, and an entry an earlier pull wrote stays. The line names the reason and the
|
||||
fix. For `git already tracks <file>`, tell the user: `git rm --cached <file>`
|
||||
(the file stays on disk), commit that, and rotate the token if the file was
|
||||
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.
|
||||
|
||||
## Permission / access denied
|
||||
|
||||
|
||||
@@ -344,22 +344,39 @@ describe('doctor — MCP servers delivered on disk', () => {
|
||||
expect(await excludeCheck()).toBeUndefined();
|
||||
});
|
||||
|
||||
it.skipIf(process.getuid?.() === 0)('says a server was withheld because its file cannot be kept out of git, and the fix', async () => {
|
||||
await fse.remove(path.join(projectRoot, '.mcp.json'));
|
||||
vi.stubEnv('JIRA_TOKEN', 'long-t0ken-value-7c1');
|
||||
const excludeFile = path.join(projectRoot, '.git', 'info', 'exclude');
|
||||
await fse.chmod(excludeFile, 0o444);
|
||||
|
||||
try {
|
||||
const check = await mcpCheck();
|
||||
expect(await check.check()).toBe(false);
|
||||
expect(check.fix).toMatch(/\.git\/info\/exclude is not writable/);
|
||||
expect(check.fix).toContain('teamai pull');
|
||||
expect(check.fix).not.toContain('again..');
|
||||
} finally {
|
||||
await fse.chmod(excludeFile, 0o644);
|
||||
}
|
||||
});
|
||||
|
||||
it('emits no check when the installed servers carry no resolved value', async () => {
|
||||
await writeTeamMcp('servers:\n - name: jira\n transport: http\n url: https://jira.example/mcp\n');
|
||||
|
||||
expect(await excludeCheck()).toBeUndefined();
|
||||
});
|
||||
|
||||
it('fails the delivery check for a declared secret withheld from a file git tracks, naming the file and the fix (#879)', async () => {
|
||||
await fse.outputFile(path.join(repoPath, 'env', 'secrets.yaml'), 'secrets:\n - key: JIRA_TOKEN\n');
|
||||
it('fails the delivery check for a server withheld from a file git tracks, naming the file and the fix once (#879)', async () => {
|
||||
vi.stubEnv('JIRA_TOKEN', 'fixture-jira-token');
|
||||
execFileSync('git', ['add', '.mcp.json'], { cwd: projectRoot });
|
||||
|
||||
const check = await mcpCheck();
|
||||
expect(await check.check()).toBe(false);
|
||||
const file = path.join(projectRoot, '.mcp.json');
|
||||
expect(check.fix).toContain(`jira not written: ${file} is tracked by git, so the value of JIRA_TOKEN would be committed.`);
|
||||
expect(check.fix).toContain(`git rm --cached ${file}`);
|
||||
expect(check.fix).toContain(`In ${file}, withheld: jira, as git would commit the file: git already tracks ${file}.`);
|
||||
expect(check.fix).toContain(`git rm --cached ${file}\` (rotate any value a commit of it holds)`);
|
||||
expect(check.fix).not.toContain('not the team\'s definition');
|
||||
expect(check.fix).not.toContain('pull --force');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -327,8 +327,8 @@ describe('a missing declared secret tells the member what to run', () => {
|
||||
await mcpList({});
|
||||
const out = spy.mock.calls.map(([line]) => String(line)).join('\n');
|
||||
const file = path.join(projectRoot, '.mcp.json');
|
||||
expect(out).toContain(`withheld: claude — ${file} is tracked by git, so the value of GITHUB_TOKEN would be committed.`);
|
||||
expect(out).toContain(`git rm --cached ${file}`);
|
||||
expect(out).toContain(`withheld: claude — git already tracks ${file}. Run \`git rm --cached ${file}\` (rotate any value a commit of it holds)`);
|
||||
expect(out.match(/withheld:/g)).toHaveLength(1);
|
||||
} finally {
|
||||
spy.mockRestore();
|
||||
}
|
||||
|
||||
@@ -11,8 +11,11 @@ vi.mock('../namespaced-entries.js', async (importOriginal) => ({
|
||||
vi.mock('../mcp-reconcile.js', () => ({
|
||||
reconcileMcpForConfig: vi.fn(),
|
||||
resolveMcpTargets: vi.fn().mockResolvedValue([]),
|
||||
buildDesiredMcpContext: vi.fn().mockResolvedValue({ vars: {} }),
|
||||
desiredMcpForTarget: vi.fn(),
|
||||
buildVarTable: vi.fn().mockResolvedValue({}),
|
||||
}));
|
||||
vi.mock('../mcp-git-exclude.js', async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import('../mcp-git-exclude.js')>()),
|
||||
ensureExcludedFromGit: vi.fn(),
|
||||
}));
|
||||
vi.mock('../utils/fs.js', () => ({
|
||||
readJson: vi.fn().mockResolvedValue(null),
|
||||
@@ -26,7 +29,8 @@ vi.mock('../utils/logger.js', () => ({
|
||||
import { autoDetectInit } from '../config.js';
|
||||
import { entryLayout, resolveEntriesFor } from '../namespaced-entries.js';
|
||||
import { mcpInject, mcpList } from '../mcp-cmd.js';
|
||||
import { reconcileMcpForConfig } from '../mcp-reconcile.js';
|
||||
import { reconcileMcpForConfig, resolveMcpTargets } from '../mcp-reconcile.js';
|
||||
import { ensureExcludedFromGit } from '../mcp-git-exclude.js';
|
||||
|
||||
const mockedAutoDetectInit = autoDetectInit as Mock;
|
||||
const mockedResolve = resolveEntriesFor as Mock;
|
||||
@@ -90,6 +94,25 @@ describe('mcpList', () => {
|
||||
expect(text.match(/roles:/g)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('says where a server needing a resolved value is withheld because git would commit the file, and the fix (#882)', async () => {
|
||||
mockedResolve.mockResolvedValue(resolved([
|
||||
[{ name: 'jira', transport: 'http', url: 'https://jira.example/mcp', headers: { Authorization: 'Bearer ${JIRA_TOKEN}' } }, 'mcp/mcp.yaml', null],
|
||||
]));
|
||||
(resolveMcpTargets as Mock).mockResolvedValueOnce([
|
||||
{ tool: 'claude', format: 'claude', file: '/work/app/.mcp.json', projectScope: true },
|
||||
]);
|
||||
(ensureExcludedFromGit as Mock).mockResolvedValueOnce({
|
||||
kind: 'failed',
|
||||
reason: '/work/app/.git/info/exclude is not writable',
|
||||
fix: 'Make it writable, then run `teamai pull` again.',
|
||||
});
|
||||
|
||||
const text = await listOutput();
|
||||
|
||||
expect(ensureExcludedFromGit).toHaveBeenCalledWith('/work/app/.mcp.json', { dryRun: true });
|
||||
expect(text).toContain('withheld: claude — /work/app/.git/info/exclude is not writable. Make it writable, then run `teamai pull` again.');
|
||||
});
|
||||
|
||||
it('reports a set that cannot be resolved instead of listing part of it', async () => {
|
||||
mockedResolve.mockResolvedValue({
|
||||
kind: 'failed',
|
||||
|
||||
@@ -21,7 +21,25 @@ vi.mock('../utils/logger.js', () => ({
|
||||
})),
|
||||
}));
|
||||
|
||||
// What .git/info/exclude held at the moment each JSON config was written (#882).
|
||||
const excludeAtWrite = vi.hoisted(() => new Map<string, string | null>());
|
||||
vi.mock('../utils/fs.js', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import('../utils/fs.js')>();
|
||||
return {
|
||||
...actual,
|
||||
writeJsonAtomic: async (...args: Parameters<typeof actual.writeJsonAtomic>) => {
|
||||
const [file] = args;
|
||||
const gitDir = path.join(path.dirname(String(file)), '.git');
|
||||
if (await fse.pathExists(gitDir)) {
|
||||
excludeAtWrite.set(String(file), await actual.readFileSafe(path.join(gitDir, 'info', 'exclude')));
|
||||
}
|
||||
return actual.writeJsonAtomic(...args);
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
import { reconcileMcpForConfig, resolveMcpTargets, spliceCodexBlock, codexServerNames, writeCodexAtomic } from '../mcp-reconcile.js';
|
||||
import { acquireLock, releaseLock } from '../update.js';
|
||||
import { log } from '../utils/logger.js';
|
||||
import { resetWarnOnce } from '../utils/warn-once.js';
|
||||
import { TeamaiConfigSchema, type TeamaiConfig, type LocalConfig } from '../types.js';
|
||||
@@ -1015,54 +1033,116 @@ servers:
|
||||
});
|
||||
});
|
||||
|
||||
describe('a config git already tracks never gets a declared secret (#879)', () => {
|
||||
const claudeFile = (): string => path.join(projectRoot, '.mcp.json');
|
||||
it('lists the config in .git/info/exclude before writing the value into it', async () => {
|
||||
await writeMcpYaml(withSecret);
|
||||
|
||||
beforeEach(async () => {
|
||||
await fse.outputFile(path.join(repoPath, 'env', 'secrets.yaml'), 'secrets:\n - key: SECRET_TOKEN\n');
|
||||
await writeMcpYaml(withSecret);
|
||||
await reconcileMcpForConfig(teamConfig, projectConfig);
|
||||
|
||||
expect(excludeAtWrite.get(path.join(projectRoot, '.mcp.json'))).toMatch(/^\/\.mcp\.json$/m);
|
||||
expect(await fse.readFile(path.join(projectRoot, '.mcp.json'), 'utf-8')).toContain('super-secret-value');
|
||||
});
|
||||
|
||||
describe('when the config cannot be kept out of git first', () => {
|
||||
const mcpJson = (): string => path.join(projectRoot, '.mcp.json');
|
||||
const infoDir = (): string => path.join(projectRoot, '.git', 'info');
|
||||
const claudeOnly = (): LocalConfig => ({ ...projectConfig, disabledAgents: ['cursor'] } as LocalConfig);
|
||||
|
||||
beforeEach(() => {
|
||||
vi.mocked(log.warn).mockClear();
|
||||
});
|
||||
|
||||
it('skips the server there, names the file and the fix, and still writes and excludes an untracked config', async () => {
|
||||
await fse.writeJson(claudeFile(), { mcpServers: {} });
|
||||
afterEach(async () => {
|
||||
await fse.chmod(infoDir(), 0o755);
|
||||
await fse.chmod(path.join(infoDir(), 'exclude'), 0o644);
|
||||
});
|
||||
|
||||
it.skipIf(process.getuid?.() === 0).each([
|
||||
['.git/info/exclude is read-only', () => fse.chmod(path.join(infoDir(), 'exclude'), 0o444)],
|
||||
['.git/info is read-only', () => fse.chmod(infoDir(), 0o555)],
|
||||
])('writes no value when %s, and warns with the fix', async (_label, lockDown) => {
|
||||
await writeMcpYaml(withSecret);
|
||||
await lockDown();
|
||||
|
||||
const { changes } = await reconcileMcpForConfig(teamConfig, claudeOnly());
|
||||
|
||||
expect(await fse.pathExists(mcpJson())).toBe(false);
|
||||
expect(changes).toContainEqual(expect.objectContaining({ tool: 'claude', server: 'with-secret', action: 'skipped' }));
|
||||
expect(log.warn).toHaveBeenCalledWith(expect.stringContaining(mcpJson()));
|
||||
expect(log.warn).toHaveBeenCalledWith(expect.stringMatching(/not writable[\s\S]*teamai pull/));
|
||||
});
|
||||
|
||||
it.skipIf(process.getuid?.() === 0)('keeps an earlier entry as it was', async () => {
|
||||
await writeMcpYaml(withSecret);
|
||||
await reconcileMcpForConfig(teamConfig, claudeOnly());
|
||||
await fse.writeFile(path.join(infoDir(), 'exclude'), '');
|
||||
const before = await fse.readFile(mcpJson(), 'utf-8');
|
||||
await writeMcpYaml(withSecret.replace('https://example.com/mcp', 'https://example.com/v2'));
|
||||
vi.stubEnv('SECRET_TOKEN', 'rotated-secret-value');
|
||||
await fse.chmod(infoDir(), 0o555);
|
||||
|
||||
await reconcileMcpForConfig(teamConfig, claudeOnly());
|
||||
|
||||
expect(await fse.readFile(mcpJson(), 'utf-8')).toBe(before);
|
||||
});
|
||||
|
||||
it('writes no value while another command holds the exclude file\'s lock', async () => {
|
||||
const lock = path.join(infoDir(), 'exclude.teamai-lock');
|
||||
expect(await acquireLock(lock)).toBe(true);
|
||||
await writeMcpYaml(withSecret);
|
||||
|
||||
try {
|
||||
await reconcileMcpForConfig(teamConfig, claudeOnly());
|
||||
} finally {
|
||||
await releaseLock(lock);
|
||||
}
|
||||
|
||||
expect(await fse.pathExists(mcpJson())).toBe(false);
|
||||
expect(log.warn).toHaveBeenCalledWith(expect.stringContaining(mcpJson()));
|
||||
expect(log.warn).toHaveBeenCalledWith(expect.stringContaining('teamai pull'));
|
||||
});
|
||||
|
||||
it('writes no value into a file git already tracks, names the fix once, and still writes an untracked config', async () => {
|
||||
await fse.writeJson(mcpJson(), { mcpServers: {} });
|
||||
git(projectRoot, 'add', '.mcp.json');
|
||||
await writeMcpYaml(withSecret);
|
||||
|
||||
const { changes } = await reconcileMcpForConfig(teamConfig, projectConfig);
|
||||
|
||||
expect(await fse.readFile(claudeFile(), 'utf-8')).not.toContain('super-secret-value');
|
||||
expect(await fse.readFile(mcpJson(), 'utf-8')).not.toContain('super-secret-value');
|
||||
expect(changes).toContainEqual(expect.objectContaining({
|
||||
tool: 'claude', server: 'with-secret', action: 'skipped', reason: expect.stringContaining(`git rm --cached ${claudeFile()}`),
|
||||
tool: 'claude', server: 'with-secret', action: 'skipped', reason: expect.stringContaining(`git already tracks ${mcpJson()}`),
|
||||
}));
|
||||
expect(log.warn).toHaveBeenCalledWith(expect.stringMatching(/tracked by git.*git rm --cached .*rotate/));
|
||||
const warnings = vi.mocked(log.warn).mock.calls.map(([line]) => String(line)).filter((line) => line.includes(mcpJson()));
|
||||
expect(warnings).toHaveLength(1);
|
||||
expect(warnings[0]).toMatch(new RegExp(`git already tracks[\\s\\S]*git rm --cached ${mcpJson()}[\\s\\S]*rotate`));
|
||||
expect((await fse.readJson(path.join(projectRoot, '.cursor', 'mcp.json'))).mcpServers['with-secret'].headers.Authorization)
|
||||
.toBe('Bearer super-secret-value');
|
||||
expect(await excludeOf(projectRoot)).toMatch(/^\/\.cursor\/mcp\.json$/m);
|
||||
});
|
||||
|
||||
it('keeps the entry an earlier pull wrote as it is, and updates it once the file is untracked', async () => {
|
||||
await reconcileMcpForConfig(teamConfig, projectConfig);
|
||||
it('keeps the entry an earlier pull wrote to a file git now tracks, and updates it once the file is untracked (#879)', async () => {
|
||||
await writeMcpYaml(withSecret);
|
||||
await reconcileMcpForConfig(teamConfig, claudeOnly());
|
||||
git(projectRoot, 'add', '-f', '.mcp.json');
|
||||
const before = await fse.readFile(claudeFile(), 'utf-8');
|
||||
const before = await fse.readFile(mcpJson(), 'utf-8');
|
||||
vi.stubEnv('SECRET_TOKEN', 'rotated-secret-value');
|
||||
|
||||
await reconcileMcpForConfig(teamConfig, projectConfig);
|
||||
expect(await fse.readFile(claudeFile(), 'utf-8')).toBe(before);
|
||||
await reconcileMcpForConfig(teamConfig, claudeOnly());
|
||||
expect(await fse.readFile(mcpJson(), 'utf-8')).toBe(before);
|
||||
|
||||
git(projectRoot, 'rm', '-q', '--cached', '.mcp.json');
|
||||
const { changes } = await reconcileMcpForConfig(teamConfig, projectConfig);
|
||||
const { changes } = await reconcileMcpForConfig(teamConfig, claudeOnly());
|
||||
expect(changes).toContainEqual({ tool: 'claude', server: 'with-secret', action: 'updated' });
|
||||
expect(await fse.readFile(claudeFile(), 'utf-8')).toContain('rotated-secret-value');
|
||||
expect(await fse.readFile(mcpJson(), 'utf-8')).toContain('rotated-secret-value');
|
||||
});
|
||||
|
||||
it('writes a variable the team does not declare as a secret, as before', async () => {
|
||||
await fse.remove(path.join(repoPath, 'env', 'secrets.yaml'));
|
||||
await fse.writeJson(claudeFile(), { mcpServers: {} });
|
||||
git(projectRoot, 'add', '.mcp.json');
|
||||
it('still writes a config that carries no resolved value', async () => {
|
||||
await fse.chmod(infoDir(), 0o555);
|
||||
await writeMcpYaml('servers:\n - name: open\n transport: http\n url: https://example.com/open\n');
|
||||
|
||||
await reconcileMcpForConfig(teamConfig, projectConfig);
|
||||
await reconcileMcpForConfig(teamConfig, claudeOnly());
|
||||
|
||||
expect(await fse.readFile(claudeFile(), 'utf-8')).toContain('super-secret-value');
|
||||
expect(await fse.readFile(mcpJson(), 'utf-8')).toContain('https://example.com/open');
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
+20
-10
@@ -430,6 +430,7 @@ export async function buildMcpDeliveryChecks(ctx: DoctorContext): Promise<Check[
|
||||
resolveMcpTargets, buildDesiredMcpContext, desiredMcpForTarget,
|
||||
mcpTargetExcluded, installedMcpEntries,
|
||||
} = await import('./mcp-reconcile.js');
|
||||
const { carriesResolvedValue, ensureExcludedFromGit } = await import('./mcp-git-exclude.js');
|
||||
const { mcpEntryReader, teamMcpToDef } = await import('./resources/mcp.js');
|
||||
const { describeEntryFailure, resolveEntriesFor } = await import('./namespaced-entries.js');
|
||||
|
||||
@@ -451,23 +452,23 @@ export async function buildMcpDeliveryChecks(ctx: DoctorContext): Promise<Check[
|
||||
if (teamDefs.length === 0) return [];
|
||||
|
||||
const targets = await resolveMcpTargets(teamConfig, localConfig);
|
||||
const desiredContext = await buildDesiredMcpContext(teamConfig, localConfig, { teamEnv: ctx.teamEnv }, targets);
|
||||
const desiredContext = await buildDesiredMcpContext(teamConfig, localConfig, { teamEnv: ctx.teamEnv });
|
||||
const excludedByUser = new Set(localConfig.excludedSkills ?? []);
|
||||
|
||||
const checks: Check[] = [];
|
||||
for (const target of targets) {
|
||||
if (mcpTargetExcluded(localConfig, target)) continue;
|
||||
|
||||
const { desired, skipped, kept, withheld } = desiredMcpForTarget(target, teamDefs, desiredContext);
|
||||
const { desired, skipped, kept } = desiredMcpForTarget(target, teamDefs, desiredContext);
|
||||
// A server skipped only for a missing declared secret (#875) is a note
|
||||
// doctor prints with the command that fixes it, not a failed delivery.
|
||||
const blocked = skipped
|
||||
.filter((change) => !excludedByUser.has(change.server) && !kept.has(change.server) && !withheld.has(change.server))
|
||||
.filter((change) => !excludedByUser.has(change.server) && !kept.has(change.server))
|
||||
.map((change) => `${change.server} (${change.reason ?? 'skipped'})`);
|
||||
// Its reason says what fixes it (#879).
|
||||
const withheldNotes = skipped.filter((change) => withheld.has(change.server)).map((change) => `${change.server} not written: ${change.reason}.`);
|
||||
|
||||
const problems: string[] = [];
|
||||
// Its fix is the exclusion's own, not another pull (#882).
|
||||
let withheld: string | undefined;
|
||||
const installed = await installedMcpEntries(target);
|
||||
if (installed === null) {
|
||||
problems.push(`${target.file} could not be parsed, so no server was injected`);
|
||||
@@ -481,12 +482,21 @@ export async function buildMcpDeliveryChecks(ctx: DoctorContext): Promise<Check[
|
||||
// held by something else entirely, and a stale copy is equally undelivered.
|
||||
else if (!isDeepStrictEqual(installed.get(name), entry)) foreign.push(name);
|
||||
}
|
||||
if (absent.length > 0) problems.push(`not injected: ${nameList(absent)}`);
|
||||
if (foreign.length > 0) problems.push(`not the team's definition: ${nameList(foreign)}`);
|
||||
// Pull writes a resolved value only into a file git leaves out of a
|
||||
// commit (#882), and otherwise leaves the whole file as it was.
|
||||
const exclusion = carriesResolvedValue(target, teamDefs, [...absent, ...foreign])
|
||||
? await ensureExcludedFromGit(target.file, { dryRun: true })
|
||||
: undefined;
|
||||
if (exclusion?.kind === 'failed') {
|
||||
withheld = `In ${target.file}, withheld: ${nameList([...absent, ...foreign])}, as git would commit the file: ${exclusion.reason}. ${exclusion.fix}`;
|
||||
} else {
|
||||
if (absent.length > 0) problems.push(`not injected: ${nameList(absent)}`);
|
||||
if (foreign.length > 0) problems.push(`not the team's definition: ${nameList(foreign)}`);
|
||||
}
|
||||
}
|
||||
if (blocked.length > 0) problems.push(`skipped: ${nameList(blocked)}`);
|
||||
|
||||
if (problems.length === 0 && withheldNotes.length === 0 && desired.size === 0) continue;
|
||||
if (problems.length === 0 && !withheld && desired.size === 0) continue;
|
||||
|
||||
const delivery = problems.length === 0 ? [] : [`In ${target.file}, ${problems.join('; ')}. A server needing a variable reads it from `
|
||||
+ '`env/env.yaml` or an active `env/<ns>/env.yaml`, whose top-level key is `variables:` — a plain `KEY: value` mapping '
|
||||
@@ -496,8 +506,8 @@ export async function buildMcpDeliveryChecks(ctx: DoctorContext): Promise<Check[
|
||||
checks.push({
|
||||
name: `MCP servers delivered to ${target.tool}`,
|
||||
source: 'local',
|
||||
check: async () => problems.length === 0 && withheldNotes.length === 0,
|
||||
fix: [...withheldNotes, ...delivery].join(' '),
|
||||
check: async () => problems.length === 0 && !withheld,
|
||||
fix: [...withheld ? [withheld] : [], ...delivery].join(' '),
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
+14
-16
@@ -5,13 +5,14 @@ import { describeEntryFailure, describeOrigin, resolveEntriesFor } from './names
|
||||
import {
|
||||
reconcileMcpForConfig,
|
||||
resolveMcpTargets,
|
||||
buildDesiredMcpContext,
|
||||
desiredMcpForTarget,
|
||||
buildVarTable,
|
||||
type McpChange,
|
||||
type McpTarget,
|
||||
} from './mcp-reconcile.js';
|
||||
import { referencedVars } from './resources/mcp-format.js';
|
||||
import { reportMissingSecrets } from './env-advisories.js';
|
||||
import { resolveTeamEnv } from './env-resolution.js';
|
||||
import { carriesResolvedValue, ensureExcludedFromGit } from './mcp-git-exclude.js';
|
||||
import { log } from './utils/logger.js';
|
||||
import type { GlobalOptions } from './types.js';
|
||||
import { managedMcpManifestPath, managedMcpManifestKey, getDataHome } from './types.js';
|
||||
@@ -53,16 +54,7 @@ export async function mcpList(_options: GlobalOptions): Promise<void> {
|
||||
}
|
||||
|
||||
const targets = await resolveMcpTargets(teamConfig, localConfig);
|
||||
const context = await buildDesiredMcpContext(teamConfig, localConfig, { teamEnv }, targets);
|
||||
const { vars } = context;
|
||||
// Why each server is not written to a tool's config git tracks (#879), by server.
|
||||
const withheld = new Map<string, string[]>();
|
||||
for (const target of targets) {
|
||||
const { skipped, withheld: names } = desiredMcpForTarget(target, servers.map((s) => teamMcpToDef(s.entry)), context);
|
||||
for (const change of skipped) {
|
||||
if (names.has(change.server)) withheld.set(change.server, [...withheld.get(change.server) ?? [], `${target.tool} — ${change.reason}`]);
|
||||
}
|
||||
}
|
||||
const vars = await buildVarTable(localConfig, teamEnv);
|
||||
// Project scope reads THIS worktree's own per-worktree manifest; user the global file.
|
||||
const manifest = (await readJson<ManagedMcpManifest>(
|
||||
managedMcpManifestPath(
|
||||
@@ -90,11 +82,17 @@ export async function mcpList(_options: GlobalOptions): Promise<void> {
|
||||
console.log(` secrets: ${needed.join(', ')} (${state})`);
|
||||
}
|
||||
|
||||
const installedIn = targets
|
||||
.filter((t) => (manifest[managedMcpManifestKey(t.tool, t.projectScope)] ?? []).some((r) => r.name === s.name))
|
||||
.map((t) => t.tool);
|
||||
const installed = (t: McpTarget): boolean =>
|
||||
(manifest[managedMcpManifestKey(t.tool, t.projectScope)] ?? []).some((r) => r.name === s.name);
|
||||
const installedIn = targets.filter(installed).map((t) => t.tool);
|
||||
console.log(` installed: ${installedIn.length > 0 ? installedIn.join(', ') : '(none)'}`);
|
||||
for (const reason of withheld.get(s.name) ?? []) console.log(` withheld: ${reason}`);
|
||||
// Pull writes a resolved value only into a file git leaves out of a commit
|
||||
// (#882); an entry an earlier pull wrote there stays as it was.
|
||||
for (const t of targets) {
|
||||
if (!carriesResolvedValue(t, [s], [s.name])) continue;
|
||||
const exclusion = await ensureExcludedFromGit(t.file, { dryRun: true });
|
||||
if (exclusion.kind === 'failed') console.log(` withheld: ${t.tool} — ${exclusion.reason}. ${exclusion.fix}`);
|
||||
}
|
||||
console.log('');
|
||||
}
|
||||
|
||||
|
||||
+94
-41
@@ -75,6 +75,13 @@ async function gitExcludeFile(dir: string): Promise<{ excludeFile: string; root:
|
||||
return { excludeFile: path.resolve(base, gitPath), root, prefix };
|
||||
}
|
||||
|
||||
/** The closest directory above `file` that exists. */
|
||||
async function existingAncestor(file: string): Promise<string> {
|
||||
let dir = path.dirname(path.resolve(file));
|
||||
while (!await pathExists(dir) && path.dirname(dir) !== dir) dir = path.dirname(dir);
|
||||
return dir;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether git would put a file in a commit. `unknown` is a repository git could
|
||||
* not answer for (unsafe ownership, a bad config): never read it as safe.
|
||||
@@ -87,8 +94,8 @@ export type GitTracking =
|
||||
|
||||
/** Whether git would put `file` in a commit: tracked, or untracked without an ignore rule. Read-only. */
|
||||
export async function gitTracking(file: string): Promise<GitTracking> {
|
||||
const dir = path.dirname(file);
|
||||
const result = await execCommand('git', ['check-ignore', '-q', '--', path.basename(file)], { cwd: dir, timeoutMs: 10_000 })
|
||||
const dir = await existingAncestor(file);
|
||||
const result = await execCommand('git', ['check-ignore', '-q', '--', path.relative(dir, file)], { cwd: dir, timeoutMs: 10_000 })
|
||||
.catch((e: unknown) => ({ code: -1, stdout: '', stderr: e instanceof Error ? e.message : String(e) }));
|
||||
if (result.code === 0) return { kind: 'ignored' };
|
||||
if (result.code === 1) return { kind: 'would-commit' };
|
||||
@@ -105,10 +112,9 @@ export async function gitTracking(file: string): Promise<GitTracking> {
|
||||
* is not tracked; nor is one in a repository git cannot answer for, where a
|
||||
* commit fails too.
|
||||
*/
|
||||
export async function gitTracks(file: string): Promise<boolean> {
|
||||
async function gitTracks(file: string): Promise<boolean> {
|
||||
// The file, or even its directory, may be gone from disk and still be in the index.
|
||||
let dir = path.dirname(file);
|
||||
while (!await pathExists(dir) && path.dirname(dir) !== dir) dir = path.dirname(dir);
|
||||
const dir = await existingAncestor(file);
|
||||
const result = await execCommand('git', ['--literal-pathspecs', 'ls-files', '--error-unmatch', '--', path.relative(dir, file)], { cwd: dir, timeoutMs: 10_000 })
|
||||
.catch(() => null);
|
||||
return result?.code === 0;
|
||||
@@ -129,49 +135,96 @@ function splitBlock(content: string): { before: string; patterns: string[]; afte
|
||||
return { before: content.slice(0, start), patterns, after };
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether `file` is kept out of git, or why teamai could not keep it out and
|
||||
* what the member does about it. `pending`: a dry run found nothing in the way
|
||||
* of listing it.
|
||||
*/
|
||||
export type GitExclusion =
|
||||
| { kind: 'excluded' }
|
||||
| { kind: 'pending' }
|
||||
| { kind: 'failed'; reason: string; fix: string };
|
||||
|
||||
/**
|
||||
* Add `file` to its repository's `.git/info/exclude` unless git ignores it
|
||||
* already. Idempotent; a path already ignored, or outside any repository, adds
|
||||
* nothing, and one git cannot answer for is added all the same. A failure warns
|
||||
* already, and whether git now leaves it out of a commit. Idempotent; a path
|
||||
* already ignored, or outside any repository, adds nothing, and one git cannot
|
||||
* answer for is added all the same. `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.
|
||||
*/
|
||||
export async function ensureExcludedFromGit(file: string, options: { dryRun?: boolean } = {}): Promise<GitExclusion> {
|
||||
const tracking = await gitTracking(file);
|
||||
if (tracking.kind === 'ignored' || tracking.kind === 'outside-repo') return { kind: 'excluded' };
|
||||
// `file` and its directory need not exist yet: git is asked from the nearest one that does.
|
||||
const dir = await existingAncestor(file);
|
||||
const location = await gitExcludeFile(dir);
|
||||
if (!location) {
|
||||
return {
|
||||
kind: 'failed',
|
||||
reason: tracking.kind === 'unknown' ? tracking.error : 'git could not locate .git/info/exclude',
|
||||
fix: 'Fix the repository, or add the file to its .git/info/exclude yourself, then run `teamai pull` again.',
|
||||
};
|
||||
}
|
||||
const { excludeFile } = location;
|
||||
// Anchored at the working tree root, glob characters escaped.
|
||||
const rel = path.relative(dir, file).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.`;
|
||||
// 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);
|
||||
if (denied) return { kind: 'failed', reason: `${writable} is not writable`, fix: retry };
|
||||
}
|
||||
const add = (content: string): string | null => {
|
||||
const block = splitBlock(content);
|
||||
if (block?.patterns.includes(pattern)) return null;
|
||||
const head = block ? block.before : content;
|
||||
const patterns = [...(block?.patterns ?? []), pattern];
|
||||
const body = [MCP_EXCLUDE_START, ...patterns, MCP_EXCLUDE_END].join('\n');
|
||||
const sep = head === '' || head.endsWith('\n') ? '' : '\n';
|
||||
return `${head}${sep}${body}\n${block?.after ?? ''}`;
|
||||
};
|
||||
// An exclude rule does not apply to a file git tracks already.
|
||||
const tracked: GitExclusion = {
|
||||
kind: 'failed',
|
||||
reason: `git already tracks ${file}`,
|
||||
fix: `Run \`git rm --cached ${file}\` (rotate any value a commit of it holds), then \`teamai pull\` again.`,
|
||||
};
|
||||
let result: ExcludeUpdate;
|
||||
try {
|
||||
if (options.dryRun) {
|
||||
// Nothing listed yet: only a tracked file would still stop the write.
|
||||
if (add((await readFileSafe(excludeFile)) ?? '') !== null) return await gitTracks(file) ? tracked : { kind: 'pending' };
|
||||
result = 'unchanged';
|
||||
} else {
|
||||
result = await updateExclude(excludeFile, add);
|
||||
}
|
||||
} catch (e) {
|
||||
return { kind: 'failed', reason: `adding it to ${excludeFile} failed: ${e instanceof Error ? e.message : String(e)}`, fix: retry };
|
||||
}
|
||||
if (result === 'locked') {
|
||||
return {
|
||||
kind: 'failed',
|
||||
reason: `another teamai command held ${excludeFile} past the wait`,
|
||||
fix: 'Run `teamai pull` again.',
|
||||
};
|
||||
}
|
||||
if (result === 'written') log.debug(`Added ${pattern} to ${excludeFile}`);
|
||||
return (await gitTracking(file)).kind === 'would-commit' ? tracked : { kind: 'excluded' };
|
||||
}
|
||||
|
||||
/**
|
||||
* `ensureExcludedFromGit` for a file already on disk, warning when it fails
|
||||
* rather than failing the sync that wrote the file.
|
||||
*/
|
||||
export async function excludeFromGit(file: string): Promise<void> {
|
||||
if (!await pathExists(file)) return;
|
||||
const tracking = await gitTracking(file);
|
||||
if (tracking.kind === 'ignored' || tracking.kind === 'outside-repo') return;
|
||||
const location = await gitExcludeFile(path.dirname(file));
|
||||
if (!location) {
|
||||
const reason = tracking.kind === 'unknown' ? tracking.error : 'git could not locate .git/info/exclude';
|
||||
const exclusion = await ensureExcludedFromGit(file);
|
||||
if (exclusion.kind === 'failed') {
|
||||
log.warn(
|
||||
`${file} holds a resolved MCP variable, and teamai could not keep it out of git: ${reason}. `
|
||||
+ 'Fix the repository, or add the file to its .git/info/exclude yourself, so git does not commit the value.',
|
||||
);
|
||||
return;
|
||||
}
|
||||
const { excludeFile } = location;
|
||||
// Anchored at the working tree root, glob characters escaped.
|
||||
const pattern = `/${location.prefix}${path.basename(file)}`.replace(/[\\*?[\]!#]/g, '\\$&');
|
||||
try {
|
||||
const result = await updateExclude(excludeFile, (content) => {
|
||||
const block = splitBlock(content);
|
||||
if (block?.patterns.includes(pattern)) return null;
|
||||
const head = block ? block.before : content;
|
||||
const patterns = [...(block?.patterns ?? []), pattern];
|
||||
const body = [MCP_EXCLUDE_START, ...patterns, MCP_EXCLUDE_END].join('\n');
|
||||
const sep = head === '' || head.endsWith('\n') ? '' : '\n';
|
||||
return `${head}${sep}${body}\n${block?.after ?? ''}`;
|
||||
});
|
||||
if (result === 'written') log.debug(`Added ${pattern} to ${excludeFile}`);
|
||||
if (result === 'locked') {
|
||||
log.warn(
|
||||
`${file} holds a resolved MCP variable and is not excluded from git yet: another teamai command held ${excludeFile} past the wait. `
|
||||
+ 'Run `teamai pull` again, and do not commit the file meanwhile.',
|
||||
);
|
||||
}
|
||||
} catch (e) {
|
||||
log.warn(
|
||||
`${file} holds a resolved MCP variable, and adding it to ${excludeFile} failed: ${e instanceof Error ? e.message : String(e)}. `
|
||||
+ `Add \`${pattern}\` to that file yourself so git does not commit the value.`,
|
||||
`${file} holds a resolved MCP variable, and teamai could not keep it out of git: ${exclusion.reason}. `
|
||||
+ `${exclusion.fix} Do not commit the file meanwhile.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
+43
-43
@@ -49,7 +49,7 @@ import { log } from './utils/logger.js';
|
||||
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 { carriesResolvedValue, excludeFromGit, gitTracks, resolvedVariableIn } from './mcp-git-exclude.js';
|
||||
import { carriesResolvedValue, ensureExcludedFromGit, excludeFromGit, resolvedVariableIn, type GitExclusion } from './mcp-git-exclude.js';
|
||||
|
||||
// ─── Reconcile engine ────────────────────────────────────────
|
||||
//
|
||||
@@ -418,38 +418,21 @@ export interface DesiredMcpContext {
|
||||
vars: Record<string, string>;
|
||||
/** Which `${VAR}` names are declared secrets, whose missing value keeps an entry (#875). */
|
||||
secrets: SecretDeclarations;
|
||||
/** The project MCP configs git tracks, which never get a declared secret's value (#879). */
|
||||
tracked: ReadonlySet<string>;
|
||||
lookPath?: McpReconcileOptions['lookPath'];
|
||||
}
|
||||
|
||||
/** Why a server is not written to `file`, a config git tracks, and what fixes it (#879). */
|
||||
export function trackedConfigReason(file: string, secrets: readonly string[]): string {
|
||||
return `${file} is tracked by git, so the value of ${secrets.join(', ')} would be committed. `
|
||||
+ `Run \`git rm --cached ${file}\` and rotate the token, then \`teamai pull\``;
|
||||
}
|
||||
|
||||
/** `targets` are the ones the caller will render for: which of them git tracks is asked here, once. */
|
||||
export async function buildDesiredMcpContext(
|
||||
teamConfig: TeamaiConfig,
|
||||
localConfig: LocalConfig,
|
||||
options: McpReconcileOptions = {},
|
||||
targets: readonly McpTarget[] = [],
|
||||
): Promise<DesiredMcpContext> {
|
||||
// HTTP mode has no repo tree to declare secrets in.
|
||||
const teamEnv = localConfig.repo.kind === 'http' ? undefined : options.teamEnv ?? await resolveTeamEnv(localConfig);
|
||||
const secrets: SecretDeclarations = teamEnv?.declarations ?? { kind: 'absent' };
|
||||
// Declarations that failed may name any variable, so they are asked about too.
|
||||
const mayHoldSecret = declaredSecretKeys(secrets)?.size !== 0;
|
||||
const tracked = mayHoldSecret
|
||||
? await Promise.all(targets.filter((t) => t.projectScope).map(async (t) => (await gitTracks(t.file) ? [t.file] : [])))
|
||||
: [];
|
||||
return {
|
||||
sharing: getMcpSharing(teamConfig),
|
||||
excluded: new Set(localConfig.excludedSkills ?? []),
|
||||
vars: await buildVarTable(localConfig, teamEnv),
|
||||
secrets,
|
||||
tracked: new Set(tracked.flat()),
|
||||
secrets: teamEnv?.declarations ?? { kind: 'absent' },
|
||||
lookPath: options.lookPath,
|
||||
};
|
||||
}
|
||||
@@ -468,20 +451,15 @@ export async function buildDesiredMcpContext(
|
||||
* a secret that lives in the member's shell is there for one pull and gone for
|
||||
* the next, and an entry an earlier pull wrote stays as it is. With
|
||||
* declarations that failed, every skipped server is kept.
|
||||
*
|
||||
* `withheld` names the servers skipped because `target` is a project config
|
||||
* git tracks and the entry would hold a declared secret's value (#879): git
|
||||
* would commit it. An entry an earlier pull wrote there stays as it is.
|
||||
*/
|
||||
export function desiredMcpForTarget(
|
||||
target: McpTarget,
|
||||
teamDefs: McpServerDef[],
|
||||
ctx: DesiredMcpContext,
|
||||
): { desired: Map<string, DesiredMcpEntry>; skipped: McpChange[]; kept: Set<string>; withheld: Set<string> } {
|
||||
): { desired: Map<string, DesiredMcpEntry>; skipped: McpChange[]; kept: Set<string> } {
|
||||
const desired = new Map<string, DesiredMcpEntry>();
|
||||
const skipped: McpChange[] = [];
|
||||
const kept = new Set<string>();
|
||||
const withheld = new Set<string>();
|
||||
// Declarations that failed can't say which variables are secrets, so every
|
||||
// missing one may be: pull keeps every installed entry then.
|
||||
const declared = declaredSecretKeys(ctx.secrets);
|
||||
@@ -515,8 +493,7 @@ export function desiredMcpForTarget(
|
||||
// Pass ${VAR} through where the tool expands it itself, so the secret
|
||||
// never lands on disk; otherwise resolve and require every var to exist.
|
||||
// A resolved value is written verbatim into the target file; a project
|
||||
// file holding one is kept out of git (#882), and one git already tracks
|
||||
// never gets a declared secret's value.
|
||||
// file gets one only once it is kept out of git (#882, reconcileTargets).
|
||||
const passthrough = supportsEnvExpansion(target.format, target.projectScope, raw);
|
||||
let def = raw;
|
||||
if (!passthrough) {
|
||||
@@ -531,12 +508,6 @@ export function desiredMcpForTarget(
|
||||
if (missing.every((key) => declared?.has(key) ?? true)) kept.add(raw.name);
|
||||
continue;
|
||||
}
|
||||
const secrets = referencedVars(raw).filter((key) => declared?.has(key) ?? true);
|
||||
if (secrets.length > 0 && ctx.tracked.has(target.file)) {
|
||||
skipped.push({ tool: target.tool, server: raw.name, action: 'skipped', reason: trackedConfigReason(target.file, secrets) });
|
||||
withheld.add(raw.name);
|
||||
continue;
|
||||
}
|
||||
def = resolved;
|
||||
} else if (referencedVars(raw).length > 0) {
|
||||
log.debug(`${raw.name}: passing ${referencedVars(raw).join(', ')} through to ${target.tool}`);
|
||||
@@ -552,7 +523,7 @@ export function desiredMcpForTarget(
|
||||
}
|
||||
}
|
||||
|
||||
return { desired, skipped, kept, withheld };
|
||||
return { desired, skipped, kept };
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -676,11 +647,13 @@ export async function reconcileMcpForConfig(
|
||||
localConfig: LocalConfig,
|
||||
options: McpReconcileOptions = {},
|
||||
): Promise<McpReconcileResult> {
|
||||
// Each project config's exclusion from git, established before a resolved value is written into it.
|
||||
const exclusions = new Map<string, GitExclusion>();
|
||||
try {
|
||||
return await reconcileTargets(teamConfig, localConfig, options);
|
||||
return await reconcileTargets(teamConfig, localConfig, options, exclusions);
|
||||
} finally {
|
||||
// Also after a failed write: what earlier pulls wrote is on disk either way.
|
||||
if (!options.removeAll && !options.dryRun) await protectResolvedMcpConfigs(teamConfig, localConfig);
|
||||
if (!options.removeAll && !options.dryRun) await protectResolvedMcpConfigs(teamConfig, localConfig, exclusions);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -690,11 +663,15 @@ export async function reconcileMcpForConfig(
|
||||
* 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.
|
||||
*/
|
||||
async function protectResolvedMcpConfigs(teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise<void> {
|
||||
async function protectResolvedMcpConfigs(
|
||||
teamConfig: TeamaiConfig,
|
||||
localConfig: LocalConfig,
|
||||
exclusions: Map<string, GitExclusion>,
|
||||
): Promise<void> {
|
||||
const { projectRoot } = localConfig;
|
||||
if (localConfig.scope !== 'project' || !projectRoot || localConfig.repo.kind === 'http') return;
|
||||
try {
|
||||
await protectProjectMcpConfigs(teamConfig, localConfig, projectRoot);
|
||||
await protectProjectMcpConfigs(teamConfig, localConfig, projectRoot, exclusions);
|
||||
} 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)}. `
|
||||
@@ -703,12 +680,19 @@ async function protectResolvedMcpConfigs(teamConfig: TeamaiConfig, localConfig:
|
||||
}
|
||||
}
|
||||
|
||||
async function protectProjectMcpConfigs(teamConfig: TeamaiConfig, localConfig: LocalConfig, projectRoot: string): Promise<void> {
|
||||
async function protectProjectMcpConfigs(
|
||||
teamConfig: TeamaiConfig,
|
||||
localConfig: LocalConfig,
|
||||
projectRoot: string,
|
||||
exclusions: Map<string, GitExclusion>,
|
||||
): Promise<void> {
|
||||
const resolution = await resolveEntriesFor(mcpEntryReader, localConfig);
|
||||
const teamDefs = resolution.kind === 'failed' ? null : resolution.entries.map((entry) => teamMcpToDef(entry.entry));
|
||||
const { manifest } = await loadProjectMcpManifest(getDataHome(localConfig), projectRoot, { dryRun: true });
|
||||
const vars = await buildVarTable(localConfig);
|
||||
for (const target of await resolveMcpTargets(teamConfig, localConfig, { includeUndetected: true })) {
|
||||
// Tried before its write this run, and reported there when it failed.
|
||||
if (exclusions.has(target.file)) continue;
|
||||
const owned = (manifest[managedMcpManifestKey(target.tool, true)] ?? []).map((record) => record.name);
|
||||
if (await resolvedValueEvidence(target, teamDefs, owned, vars)) await excludeFromGit(target.file);
|
||||
}
|
||||
@@ -718,6 +702,7 @@ async function reconcileTargets(
|
||||
teamConfig: TeamaiConfig,
|
||||
localConfig: LocalConfig,
|
||||
options: McpReconcileOptions,
|
||||
exclusions: Map<string, GitExclusion>,
|
||||
): Promise<McpReconcileResult> {
|
||||
const changes: McpChange[] = [];
|
||||
let wrote = false;
|
||||
@@ -758,7 +743,7 @@ async function reconcileTargets(
|
||||
const nothingOwned = Object.values(manifest).every((r) => r.length === 0);
|
||||
if (teamDefs.length === 0 && nothingOwned) return { changes, wrote };
|
||||
|
||||
const desiredContext = await buildDesiredMcpContext(teamConfig, localConfig, options, targets);
|
||||
const desiredContext = await buildDesiredMcpContext(teamConfig, localConfig, options);
|
||||
// A failed declaration is not "no secrets": read as none, every server whose
|
||||
// secret the member left in their shell would be removed. Keep what is
|
||||
// installed rather than guess which variables are secrets. `removeAll`
|
||||
@@ -779,11 +764,26 @@ async function reconcileTargets(
|
||||
const nextRecords: ManagedMcpRecord[] = [];
|
||||
|
||||
// Which of this team's servers apply to this tool, and in what rendered form.
|
||||
const { desired, skipped, kept, withheld } = desiredMcpForTarget(target, teamDefs, desiredContext);
|
||||
const { desired, skipped, kept } = desiredMcpForTarget(target, teamDefs, desiredContext);
|
||||
changes.push(...skipped);
|
||||
for (const change of skipped) if (withheld.has(change.server)) log.warn(`MCP server ${change.server} not written: ${change.reason}.`);
|
||||
// Their old records, so a manifest this run writes still claims them.
|
||||
const keep = new Map(owned.filter((r) => kept.has(r.name) || withheld.has(r.name)).map((r) => [r.name, r]));
|
||||
const keep = new Map(owned.filter((r) => kept.has(r.name)).map((r) => [r.name, r]));
|
||||
|
||||
// A resolved value lands only in a file git leaves out of a commit (#882).
|
||||
// Otherwise the file stays as it was, its manifest entry with it.
|
||||
if (carriesResolvedValue(target, teamDefs, desired.keys())) {
|
||||
const exclusion = exclusions.get(target.file) ?? await ensureExcludedFromGit(target.file, { dryRun: options.dryRun });
|
||||
exclusions.set(target.file, exclusion);
|
||||
if (exclusion.kind === 'failed') {
|
||||
const reason = `${target.file} is not kept out of git: ${exclusion.reason}`;
|
||||
for (const server of desired.keys()) changes.push({ tool: target.tool, server, action: 'skipped', reason });
|
||||
log.warn(
|
||||
`Did not write ${target.tool}'s MCP servers to ${target.file}: it would hold resolved values, and teamai could not `
|
||||
+ `keep it out of git first: ${exclusion.reason}. The file is left as it was. ${exclusion.fix}`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
if (target.format === 'codex') {
|
||||
wrote = await applyCodex(target, desired, keep, ownedNames, nextRecords, changes, options) || wrote;
|
||||
|
||||
Reference in New Issue
Block a user