Conversation
…encent#882) A project-scope MCP config that carries a resolved ${VAR} sat untracked and unignored in the business repo, one `git add -A` from committing the token. After the reconcile writes such a file and git would track it, teamai lists its path in the clone's .git/info/exclude inside a marked block (resolved via `git rev-parse --git-path`, so linked worktrees and submodules work). The committed .gitignore is never touched; an ignored path or a config with no resolved value adds nothing; dry runs write nothing. Project-scope uninstall removes only teamai's block, and doctor reports such a file git would still commit. The hook sits after the appliers in reconcileMcpForConfig, outside desiredMcpForTarget/applyJson/applyCodex, so it merges cleanly with Tencent#880.
…Tencent#882) The plan now records whether the project's .git/info/exclude holds teamai's MCP config block (gitExcludeBlock). It counts toward isPlanEmpty, is listed in the summary and dry run, and gates the removal, so a plan whose only teamai leftover is the block removes it instead of reporting "Nothing to uninstall".
|
Findings
The PR description includes sufficient unit, e2e, and representative real-CLI testing. |
…cent#882) - uninstall keeps a repository's .git/info/exclude block while a config in it could not be parsed and still holds teamai servers, and warns - uninstall finds and removes the block in nested repositories holding an MCP config, across every worktree - the block opens at the last start marker, so an orphaned start never pairs with a later block's end and takes the member's lines - doctor counts only servers the ownership manifest records, not a member's own server under a team name
|
Findings
The four earlier findings are resolved in the current diff. The PR description includes sufficient unit, e2e, and representative real-CLI testing. |
…en clean (Tencent#882) A missing or unreadable managed-mcp.json made the MCP cleanup return early without reporting anything, so uninstall removed the block while .mcp.json still held the resolved token. Uninstall now inspects every path the block protects after the cleanup. The block goes only when each one is missing, or parses and holds none of the team's servers that need a resolved ${VAR}. Anything it cannot check keeps the block, with a warning naming the file. This replaces the leftInPlace report from the reconcile, which the check subsumes.
|
Findings
The four earlier findings are resolved in the current diff. The PR description includes sufficient unit, e2e, and representative real-CLI testing. |
…encent#882) Pull, doctor and uninstall each skipped a case they had not inspected and treated it as safe. Now: - pull lists a config in .git/info/exclude whether or not it delivered to it this run: a disabled or undetected tool's file, a team with automatic delivery off, an unreadable mcp.yaml (any teamai entry counts), a failed write to another tool's config, and a lost ownership manifest (the resolved value found in the file) - doctor checks the same files, including one that does not parse, and counts a git error as a failure - git check-ignore failing inside a repository is no longer read as "not tracked": the path is excluded anyway, or teamai warns with git's error - uninstall also keeps the block while a file contains the value (8+ characters, not a path or the login name) of a variable still set in the environment, which finds a server since dropped from mcp.yaml
|
Findings
The previously reported findings are resolved in the current diff. The PR description documents sufficient unit, e2e, and representative real-CLI testing. |
…encent#882) Pull, doctor and uninstall still decided "clean" from the current team config in places. Now one function, resolvedValueEvidence, decides for all three: - a teamai-owned entry still in the file counts when its server has left mcp.yaml, as well as when it needs a resolved ${VAR} or mcp.yaml cannot be read (doctor no longer skips that case) - targets include the built-in location of a tool the team dropped from toolPaths or moved - doctor names a file two tools share once - exclude updates take the existing acquireLock helper, re-read the file and write it atomically, so concurrent commands keep each other's paths - uninstall inspects every worktree of each repository owning a block, including a nested repository's linked worktrees, and applies the manifest rule per worktree - the kept-block warning names each file and why, such as the variable whose value matched
|
Findings
The previously reported findings are resolved in the current diff. The PR description includes sufficient unit, e2e, and representative real-CLI testing. |
9 tasks
SaulMoro
added a commit
to SaulMoro/teamai-cli
that referenced
this pull request
Sep 28, 2026
…esolved tokens out of git # Conflicts: # src/doctor-delivery.ts # src/mcp-reconcile.ts
…s its lock (Tencent#882) After the 2.5 s wait for the exclude file's lock, updateExclude wrote without it, so two writers could drop each other's pattern and leave a plaintext MCP config committable. It now writes nothing and reports 'locked': pull warns that the file is not excluded yet and to run `teamai pull` again (doctor's exclude check keeps reporting it meanwhile), and uninstall keeps the block and warns.
… free of teamai's servers (Tencent#882) Uninstall judged a protected file clean from the current mcp.yaml, manifest and resolvable values, so with the manifest lost, the server gone from mcp.yaml and its value unset, a plaintext token looked like the member's own server and the exclusion went. It now fails closed and works per entry: a pattern goes only when its file is gone, holds no server, or holds none of teamai's servers with managed-mcp.json still there to say what teamai wrote. A kept entry is named with its file, why, and how to clean it by hand, since a rerun of uninstall finds no config after a full uninstall.
|
Findings
The other earlier findings are resolved. The PR description documents sufficient unit, e2e, and representative real-CLI testing. |
SaulMoro
added a commit
to SaulMoro/teamai-cli
that referenced
this pull request
Sep 29, 2026
…proof, skip unlocked writes # Conflicts: # docs/usage-guide.md # docs/usage-guide.zh-CN.md
…lved value into it (Tencent#882) Pull listed the file in .git/info/exclude only after writing the plaintext, and a failed exclusion only warned, so the secret-bearing file stayed eligible for git add -A. The exclusion now comes first; when it cannot be established (exclude file or .git/info not writable, lock held past the wait, file already tracked, git error) the file is left as it was and the warning names the reason and the fix.
…p list and doctor (Tencent#882)
|
Findings
The earlier nested-repository linked-worktree issue and the other previously resolved findings remain fixed. The PR description documents sufficient unit, e2e, and representative real-CLI testing. |
SaulMoro
added a commit
to SaulMoro/teamai-cli
that referenced
this pull request
Sep 29, 2026
…ed value Tencent#882's pre-write exclusion is now the single gate for a project MCP config git would commit. Tencent#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.
…ready installed (Tencent#882) Backports Tencent#880's merge 0d9f7fa: a dry run (doctor, mcp list) names a tracked file before any pull has listed it, mcp list reports withheld for a server an earlier pull installed, and doctor's withheld note carries the exclusion's own fix instead of the pull --force advice.
…clude (Tencent#882) A tracked file needs `git rm --cached` whatever else is wrong, so ensureExcludedFromGit checks gitTracks before the writability check, on a pull and a dry run alike, and lists nothing for it.
…olds no resolved value (Tencent#882) A pull that lists a config in .git/info/exclude and then writes no value into it (it does not parse, a member's server holds the team's name, the write fails) removes the line it added. After a pull or `teamai mcp remove`, a line whose configs are proven clean in every worktree, by the proof uninstall uses (moved to mcp-reconcile.ts), is removed under the lock; one not proven clean stays. A config listed before its write is listed again after it, so a concurrent uninstall that dropped the line between the check and the write does not leave the value unprotected.
…re the pull rewrote it (Tencent#882) A pull whose manifest was lost before it ran recreates managed-mcp.json while reconciling, so the clean-file proof read the new record and took the exclude line out of a file still holding a teamai server that left mcp.yaml with its variable unset. The proof now uses this worktree's manifest as read before the reconcile.
A line this pull added and took back out, because it wrote no resolved value into the file, was reported as removed although the member never saw it added. Only removing a line an earlier run added stays at info.
|
Findings
The other previously fixed findings remain resolved. The PR description documents sufficient unit, e2e, and representative real-CLI testing. |
SaulMoro
added a commit
to SaulMoro/teamai-cli
that referenced
this pull request
Sep 29, 2026
…rst, backport
…holds a server (Tencent#882) A pull or `teamai mcp remove` judged every linked worktree's MCP config with today's definitions and values. Once a server's ${VAR} became a literal, and the value was no longer set, a pull in worktree A took worktree B's stale token-bearing entry for clean and removed the shared /.mcp.json line, so `git add -A` in B staged the token. These commands now release a line only when the current worktree's file passes the full proof and every other worktree's file is missing or holds no MCP server. `teamai uninstall` keeps its full proof in each worktree.
…onfig, and pin an uninstalled tool's leftover config (Tencent#882)
|
Findings
The other previously reported findings appear resolved. The PR description documents sufficient unit, e2e, and representative real-CLI testing. |
…d tool maps its file, and name a re-including .gitignore rule on a dry run (Tencent#882)
|
Findings
The previously reported findings appear resolved. The PR description includes sufficient unit, e2e, and representative real-CLI testing. |
…e key, and settle its notes on every format's view (Tencent#882)
|
Findings
The previously reported findings appear resolved. The PR description documents sufficient unit, e2e, and representative real-CLI testing. |
…ts as a writer, though an installed tool maps the file (Tencent#882)
|
Findings
The previously reported findings appear resolved. The PR description documents sufficient unit, e2e, and representative real-CLI testing. |
…ng the same key today (Tencent#882)
|
Findings
The previously reported findings appear resolved. The PR description documents sufficient unit, e2e, and representative real-CLI testing. |
…Servers another tool added, and remove teamai's there (Tencent#882)
…v value to out of git, and have doctor check it for HTTP teams (Tencent#882)
…lot's bare servers beside mcpServers (Tencent#882)
…server again, not to pull (Tencent#882)
|
Findings
The previously reported findings appear resolved. The PR description documents sufficient unit, e2e, and representative real-CLI testing. |
… agent writes it again under mcpServers (Tencent#882)
|
Findings
The previously reported Copilot bare-entry finding is resolved. The PR description documents sufficient unit, e2e, and representative real-CLI testing. |
…and command line as credentials, and scope the rebuild's claims by key (Tencent#882)
|
Findings
The previously reported findings appear resolved. The PR description documents sufficient unit, e2e, and representative real-CLI testing. |
… have doctor judge a recorded HTTP-team file with no record (Tencent#882)
|
Findings
The other previously reported findings appear resolved. The PR description includes sufficient unit, e2e, and representative real-CLI testing. |
…e Copilot copy, hold a shadowed one, and judge a partly lost HTTP record by its entries (Tencent#882)
|
Findings
The previously reported findings appear resolved. The PR description documents sufficient unit, e2e, and representative real-CLI testing. |
…al into on the next sync or pull of an HTTP-backed team (Tencent#882)
… install's credential file (Tencent#882)
…nd call the file's content a credential (Tencent#882)
|
Findings
The previously reported findings appear resolved. The PR description documents sufficient unit, e2e, and representative real-CLI testing. |
…amai too: a failed or partial one leaves what to keep out of git (Tencent#882)
|
Review Result
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A project MCP config that holds a resolved token is kept out of git through the clone's own
.git/info/exclude. Nothing committed changes.The block is the span from the last start marker to the end marker after it, so a start marker that lost its end is left behind rather than paired with a later block's end.
Type of Change
Test Plan
npx tsc --noEmitpassesnpm run lintpassesnpx vitest runpasses (344 files, 5786 passed, 1 skipped,origin/mainmerged including fix(env,hooks,mcp,status): name the entries that are not delivered (#822) #851, fix(pull): keep a skill, rule or agent you edited instead of overwriting it (#822) #865, fix(init): seed a custom agent's configured root dir on regular init, not just self-mode (#867) #873, fix(models): key team values by repo identity, migrating legacy slug names (#894) #895, fix(dry-run): thread { dryRun } into the queue lock, so a preview stops creating <home>/.teamai/locks/ #896, fix(test): keep CI validate from timing out on the usage lock and an unborn HEAD #897, fix(dry-run): stop dry-run and read-only commands persisting config migrations (#893) #901 and fix(dry-run): let --dry-run reach recall maintenance and recall promote (#900) #903; run withCLAUDE_CONFIG_DIRunset, see fix(tests): prevent model tests from overwriting Claude config (P1) #890)doctor.test.ts'sutils/fsmock gainsreadJson(returns null), since doctor now reads the manifest whenmcp.yamlis unreadablenpm run test:e2e: 380 passed, 26 skipped.Tests added for the review findings (each red before its fix):
Real CLI, project scope,
Bearer ${GITHUB_TOKEN}:Real CLI, a tool disabled after a pull (temporary
HOME,CLAUDE_CONFIG_DIRunset, team repo withBearer ${JIRA_TOKEN}, Claude and Cursor):Real CLI, a server dropped from
mcp.yamland its tool disabled after a pull (temporaryHOME,CLAUDE_CONFIG_DIRunset, Claude and Cursor):Real CLI, uninstall with a config it cannot clean (temporary
HOME,CLAUDE_CONFIG_DIRunset).mcp injectwithBearer ${JIRA_TOKEN}first, thenmanaged-mcp.jsondeleted:Same setup,
.mcp.jsonhand-broken into invalid JSON instead:Tests that close the former known limits (each red with only that step's source reverted):
Real CLI, before (R1-R4
34bb7c11, R5-R672799f5a, R816007ae2, R5bd19a6d6d, R9-R10bf28224c, R1116c1938f) and after, temporaryHOME, Claude + Cursor,Bearer ${JIRA_TOKEN}:Related Issues
Closes #882. Found while implementing #879 / #880.
Notes for Reviewers
Door: two-way. Only writes a marked block in a local, uncommitted file; uninstall removes it.
Blast Radius: project scope. Clones whose project MCP config holds a resolved
${VAR}; user-scope configs untouched.teamai mcp removeleaves a file provably free of resolved values, its line is removed under the lock (the block with its last line); the proof uses the manifest as it stood before the command ran, so a pull that recreates a lost manifest can't open the block. Pull andmcp removejudge only the current worktree: while another worktree's copy of the file holds any server, the shared line stays (only a pull there, or uninstall, can judge it). A file listed before its write is listed again after it, so an exclusion a concurrent uninstall dropped is restored. A tracked file is named before an unwritable exclude file..git/infoor exclude file, lock held past the wait, a git error, or a file git already tracks), teamai doesn't write that file: the earlier content and manifest entry stay, pull andmcp injectwarn with the reason and fix, andmcp listand doctor show the server as withheld. A tracked file's fix isgit rm --cached <file>and rotating the token.${VAR}whilemanaged-mcp.jsonstill records what teamai wrote there (a lost manifest keeps the line, with a warning naming the file); the block goes with its last line. The check per file iscarriesResolvedValueover the entries actually in the file, the predicate pull uses to add the path, applied to every entry rather than only the owned ones. The ownership manifest is not consulted, so a missing or unreadablemanaged-mcp.json, or ownership records dropped after a parse failure, cannot open the block. A file no detected tool reads, one that does not parse, or a teammcp.yamlthat cannot be read keeps the block, with a warning naming the file. The member then removes the team's servers and the block by hand.disabledAgents/enabledAgents),autoApply: false, tool detection and a failed write stop delivery only. The protection pass runs in afinallyafter every non-dry reconcile and visits every tool's project MCP file. A failure inside it warns and points atteamai doctor, rather than going into pull's debug-level reconcile log.check-ignoreerrors, tracking is decided bygit ls-files --error-unmatch; a tracked file is withheld, and ifls-filescan't answer either, nothing is written and git's error is the reason. A line this run added is rolled back only for a file this run did not write, so a later failure (the manifest write) keeps the line.mcp listreportswithheldonly for targets delivery would write the server to.check-ignoreexit 0 means ignored and exit 1 means would commit. Anything else is "outside a repository" only when no.gitexists above the file. Otherwise the path is excluded all the same, or teamai warns with git's error, and doctor fails the check.resolvedValueEvidence, decides for pull, doctor and uninstall. A teamai-owned entry still in the file counts when its server needs a resolved${VAR}, when its server has leftmcp.yaml(the definition that would prove it clean is gone), or whenmcp.yamlcannot be read. Targets include the built-in location of a tool the team dropped fromtoolPathsor moved. Uninstall inspects every worktree of each repository owning a block, including a nested repository's linked worktrees. A file there that no tool reads keeps the block.managed-mcp-files.json(next tomanaged-mcp.json) lists each config a pull writes a resolved value to, recorded after its exclusion and before the write, so a file left behind by a changedtoolPathsmapping is still visited. When a lostmanaged-mcp.jsonis rebuilt, the servers in the file that the new record doesn't claim are noted there and keep the line until they leave the file or teamai owns them again. A file two tools share counts as recorded only while every tool the sidecar says wrote a resolved value there still has its record; with no sidecar entry, every tool the team maps there must have one. A tool's built-in location, once the team moves or drops the tool, is judged like an earlier-mapped file; when another tool maps it today, it holds while it contains a server that tool's records don't own. While a worktree has nomanaged-mcp.json, a config holding a server no record claims is kept out and that server noted. Records now carryresolved, so an entry unchanged since a pull wrote it keeps its line even after its definition turns literal. A config an older teamai wrote before this record existed is found once per worktree by reading everymcpProjectin the team repo's history ofteamai.yaml, plus the built-in defaults teamai has since changed; such a file is judged like a recorded one (it holds while it doesn't parse or holds any server), since today's records describe the new path, and doctor reads the same history until a pull has. One git tracks is recorded as tracked and judged once it isn't. A file a moved tool wrote keeps being judged for that tool while another tool maps it. A rebuilt record whose note fails to land is markedunnotedin the same manifest write, and its file keeps the line until a pull notes it. Missing or unreadable, the file reads as empty and the earlier rules apply.realFilePath(file): the real path of its closest existing directory, with the rest appended..cursor/linked toconfig/gets/config/mcp.jsonin the exclude, and a tracked file there is named by the path thatgit rm --cachedaccepts. Reads keep the logical path.info/exclude. Each change takes the existingacquireLockhelper fromupdate.ts(retrying for about 2.5 s), re-reads the file, and writes it atomically. If the lock is still held after the wait, nothing is written: pull warns that the file isn't excluded yet and to runteamai pullagain, uninstall keeps the block, and doctor's check fails until a pull succeeds.mcp.yaml. The kept-block warning names the file and why, such as the variable whose value matched, so a member can judge an ordinary value (NODE_ENV, a base URL). The scan stays fail-closed.Known limits
managed-mcp.jsonthat an older teamai rebuilt noted no servers: a stale entry for a server since dropped frommcp.yaml, whose value is no longer set, then looks like the member's own, and its line can go. The same holds ifmanaged-mcp-files.jsonis deleted after the rebuild.teamai pull --forcereclaims it; an older record counts as resolved once today's definition no longer produces the entry; a file under a changed mapping (recorded, or found in the history ofteamai.yaml) keeps it while it holds any server, the member's own included; so does a file another tool now maps, written for a tool the team moved, while it holds a server the tools mapping it didn't write; and an older record withoutresolvedfor a server with atools:filter, in a file two tools map, reads as no longer produced until a pull rewrites it. So does a tool's built-in location once the team moves or drops the tool (a tool a customtoolPathsnever listed counts as dropped), while it holds any server; and a shared file no pull on this version recorded, until both tools have a record (a Claude-only member with CodeBuddy on the same.mcp.jsonkeeps the line until a pull records the file). CodeBuddy's.mcp.json, once the team moves or drops CodeBuddy, keeps it while it holds a server Claude's records don't own, one of the member's own included. And whilemanaged-mcp.jsonhas no record for a tool (the whole file lost, that tool's record lost, a new worktree's first pull, or teamai's first delivery to that tool), each pull (and doctor) treats its untracked config as unrecorded, and the pull notes each server no record claims; a tool this machine doesn't have counts when no installed tool maps its file ormanaged-mcp-files.jsonlists it as a writer there (CodeBuddy never installed beside Claude's.mcp.jsondoesn't); the config keeps its line until those servers leave the file orteamai pull --forcereclaims them, a member's own server already there included. A note that cannot land (another command holdsmanaged-mcp-files.json, an I/O error) keeps the line until a later pull writes it; if a pull empties a tool's record meanwhile, the record is dropped instead of kept empty, and the file keeps its line while it holds any server until the team delivers to that tool again.install_mcplists a project config before writing a server with any header, env value, argument or URL, or a command line with arguments: only a bare stdio command carries no credential (its payload carries literal values, so any counts). A file an older local agent wrote a credential into is listed on the local agent's next sync in that workspace, and on a pull there (the current workspace only; another is protected on its next session there). Onlyteamai uninstalltakes such a line out: no pull orteamai mcp removereleases one for an HTTP team.mcpServers, every other top-level object reads as a bare server; an object-valued setting there reads as a server no record claims, which keeps the line.teamai.yaml) to enumerate targets. Without it, this check is skipped.~/.claude.jsonand similar) are out of scope. If$HOMEis a git repository (dotfiles), such a file is not protected.teamai mcp removeleaves the block: all worktrees share one exclude file.