feat(data-layout): route managed-mcp.json + project resource cache to the partition (#374 P1-2C) - #417
feat(data-layout): route managed-mcp.json + project resource cache to the partition (#374 P1-2C)#417jeff-r2026 wants to merge 7 commits into
Conversation
… the partition (#374 P1-2C) Completes the P1-2 partition routing deferred from #406: the two remaining A1 items that still landed in the business workspace for a project install now follow the machine-data home. - `managed-mcp.json` (per-project MCP injection manifest) and the project-scope `local-agent/resources` cache (the 4MB the issue calls out) move to `~/.teamai/projects/<slug>/` for a partitioned install, keeping the workspace clean. (local-agent's own config/manifest are A2 — already global under ~/.teamai — and are untouched.) The trap these were deferred for: both are resolved from two sides — callers that hold a full LocalConfig (mcp-reconcile / mcp-cmd / uninstall) and the local-agent subsystem, which only carries (scope, projectRoot) and has no LocalConfig. If only one side moved, a partitioned install's writer and reader would disagree (partition vs legacy) and desync. Fix: one shared resolver. - `managedMcpManifestPath(dataHome)` now takes a resolved data home. LocalConfig holders pass `getDataHome(localConfig)`. - New `resolveDataHomeForScope(scope, projectRoot)` in config.ts reproduces detection's double-read (user → ~/.teamai; project → getDataHome of detectProjectConfig's result, i.e. partition for a new/migrated install, else legacy). The local-agent managed-mcp + resource paths route through it, so both sides always land on the identical directory. - `getResourceRepoPath` (local-agent) is now async and uses the shared resolver for its project branch; user scope is unchanged (~/.teamai/local-agent/resources). - `ensureProjectGitignore` now runs only when the cache actually lands inside the workspace (a legacy install) — a partitioned cache lives under ~/.teamai, so there is nothing in the workspace to ignore. Test plan (all green): - npx tsc --noEmit — clean; npx vitest run — 191 files / 2644 tests pass - new real git+fs integration tests (detect-subdir.test.ts) prove the desync guard: `resolveDataHomeForScope` == `getDataHome(detectProjectConfig(...))` for legacy, subdirectory, user, non-git, AND partitioned installs (config in ~/.teamai/projects/<slug>/ resolves to the partition from both sides). Note: the full http-backend local-agent install flow (install_skill/install_mcp end-to-end) is covered by the 65 existing local-agent unit tests; a live-backend e2e remains a manual TODO, as with the http provider. Refs #374 (P1-2C).
|
[P1] The new partition is keyed by I reproduced this locally with a real
The same shared ownership can also cause cross-worktree removal/orphaning when one worktree reconciles an empty desired set or uninstalls. Please keep project MCP ownership isolated by canonical |
…-2C review) The partition keys managed-mcp.json by projectAnchor, so the main checkout and every linked worktree share ONE manifest. But a project MCP file (`<workspace>/.mcp.json`, `.codex/config.toml`, …) is per-worktree, while the manifest ownership key was only `<tool>:project`. So one worktree's reconcile / uninstall claimed ownership of entries another worktree wrote into its OWN file — silently overwriting (or, on an empty desired set, removing) a sibling worktree's user-owned same-name MCP, without --force. Fix: the project-scope manifest key now carries the current workspace identity. New `managedMcpManifestKey(tool, projectScope, workspaceRoot)` in types.ts returns `<tool>:project:<sha1(workspaceRoot)[:12]>` (user scope stays `<tool>`). Both the CLI reconcile/uninstall path (mcp-reconcile: keyed off localConfig.projectRoot, which detection anchors to the current worktree) and the local-agent install/uninstall path (keyed off workspacePath) build the key through this one helper, so the two sides always agree. The manifest still lives in the shared partition; only ownership is now per-worktree. Test plan (all green): - npx tsc --noEmit — clean; npx vitest run — 191 files / 2645 tests pass - NEW real-fs regression (mcp-reconcile.test.ts), reproducing the reviewer's scenario: two worktrees of one repo sharing one partition/manifest; reconcile `shared` v1 in worktree A; worktree B has a user-owned same-name `shared` → reconcile v2 from A only. A gets team-v2, B's `https://mine.example/mcp` is left untouched, and the manifest carries a distinct `claude:project:<id>` key per worktree. - updated the local-agent-mcp install assertion to the workspace-scoped key. Refs #374 (P1-2C).
|
确认成立,已修复(e2d5c74)。这是 P1-2C 把 manifest 搬进共享分区后暴露的所有权失配:分区按 采纳你的方案,在 key 里加入当前 workspace 身份:新增 按你的要求补了真 linked-worktree 回归测试(mcp-reconcile.test.ts):两个 worktree 共享一个分区/manifest,worktree A reconcile |
|
The original cross-worktree overwrite is fixed when I replay the actual trigger from worktree B. I found one new P1 upgrade regression and one smaller missed reader: [P1] Migrate the existing bare project ownership keysEvery existing project-scope installation writes ownership under a bare key such as I reproduced this locally using the pre-fix on-disk state:
With the latest PR head, reconcile returns: The v1 entry is not updated. Calling reconcile with Please migrate or compatibly consume the old [P2]
|
… reader (#374 P1-2C review) Two follow-ups from the #417 review of the per-worktree ownership key: [P1] Legacy bare `<tool>:project` keys were neither read nor migrated. Every pre-#374 project install owns its MCP entries under a bare `claude:project` key; the new code only looked up `claude:project:<workspace-hash>`, so after upgrade reconcile reported "a server with this name already exists and is not managed by teamai" (no update), and uninstall would drop the manifest while leaving the injected server behind — permanently losing the ownership record. Fix: `resolveManagedMcpOwnership(manifest, tool, projectScope, workspaceRoot, workspaceLocalManifest)` in types.ts. It returns the workspace-scoped key's records, and — only when the manifest is workspace-local (a legacy `<projectRoot>/.teamai` data home, where a bare key is unambiguous) — adopts an old `<tool>:project` record into the workspace-scoped key and drops the bare key. In the shared partition a bare key stays untouched (ambiguous; a future layout migration that knows the source workspace can claim it). mcp-reconcile routes its ownership read through this helper. uninstall already iterates ALL manifest keys, so it collects bare + hashed records unchanged. [P2] `teamai mcp list` still read `${tool}:project`, so a project injection stored under the hashed key showed `installed: (none)`. It now uses `managedMcpManifestKey(..., localConfig.projectRoot)`. Test changes (reviewer notes both): - Rewrote the sibling-worktree regression to replay the reported trigger — reconcile FROM worktree B while B has a user-owned same-name `shared` that A manages. Asserts B's server is preserved verbatim, the change is reported `skipped` (not `updated`), and A's ownership/file are unaffected. (The prior version reconciled from A, where B was untouched even without the fix.) - New test starting from the exact pre-#374 on-disk format (bare `claude:project` + injected v1) verifies reconcile UPDATES to v2, migrates the bare key to the workspace-scoped key, and leaves no ambiguous key. Test plan: npx tsc --noEmit clean; npx vitest run — 191 files / 2646 tests pass. Refs #374 (P1-2C).
|
两条都确认成立,已修复(170c3ff);测试也按你的两点建议改了。 [P1] 旧 bare key 迁移/兼容读:老装的所有权在 [P2] 测试(采纳你两点):
2646 单测全绿。 |
|
The previous CLI legacy-key migration and [P1] Local-agent install/uninstall does not adopt legacy bare project ownership
const manifestKey = managedMcpManifestKey(tool, projectScope, workspacePath);
const owned = manifest[manifestKey] ?? [];They never call I reproduced this with:
The command ACK is {
"ack": { "type": "uninstall_mcp", "status": "success" },
"after": {
"mcpServers": {
"legacy": { "url": "https://old.example/mcp" }
}
}
}The install/update path similarly treats the old TeamAI entry as unmanaged and fails. Please route both local-agent paths through the same compatibility helper and persist the safe workspace-local migration. [P1] Project MCP reporting includes ownership from sibling worktrees
A minimal local repro placed {
"path": ".../repo-b",
"mcps": [{ "slug": "a-only", "source": "enterprise" }]
}This can make the backend believe B already has an MCP, suppress the required install, or issue an incorrect uninstall. Please make project reporting select only |
…ugh workspace-scoped ownership (#374 P1-2C review) The CLI reconcile/uninstall paths were fixed to use resolveManagedMcpOwnership + per-worktree keys, but the local-agent subsystem's three project-MCP paths were not — same class of bug the reviewer had flagged for the CLI side. [P1] local-agent install/uninstall did not adopt legacy bare ownership. installMcpServer/uninstallMcpServer read only `managedMcpManifestKey(...)`, so an MCP an older local-agent installed under a bare `<tool>:project` key lost ownership after upgrade (uninstall ack'd success but left the server; install treated the old entry as unmanaged and failed). Both now go through resolveManagedMcpOwnership with a workspace-local check (dataHome === <workspace>/.teamai), migrating the bare key only when unambiguous — identical to the CLI reconcile path. [P1] project MCP reporting leaked sibling-worktree ownership. scanMcpFromManifest iterated EVERY manifest value; since the shared partition now holds per-worktree keys, worktree B reported MCPs owned by worktree A (which could suppress a needed install or trigger a wrong uninstall on the backend). It now filters to keys that belong to this scope: project → keys carrying this workspace's id (managedMcpWorkspaceId(projectRoot)), plus a legacy bare `<tool>:project` only for a workspace-local manifest; user → bare `<tool>` keys only. Added `managedMcpWorkspaceId(workspaceRoot)` to types.ts (the shared id segment) so the key builder and the report filter derive the identity the same way. Test plan (all green): - npx tsc --noEmit clean; npx vitest run — 191 files / 2647 tests pass - NEW real git-worktree report test: a shared partition manifest holds `a-only` under worktree A's key and `b-only` under worktree B's key; buildReportPayload for worktree B contains ONLY `b-only`, never `a-only`. - the existing local-agent-mcp + mcp-reconcile suites (install/uninstall + legacy-migration + sibling-worktree) stay green. Refs #374 (P1-2C).
|
两条都确认成立,已修复(5e97ec0)。这正是我上轮改了 CLI reconcile/uninstall 却漏掉 local-agent 子系统对应三处的同类遗漏。 [P1] local-agent install/uninstall 不兼容旧 bare key: [P1] project MCP 汇报串 worktree: 新增 测试(按你的要求):新增真 git-worktree 汇报测试 —— 共享分区 manifest 里 |
|
The previous local-agent ownership/report issues are fixed. One P1 remains now that [P1] Concurrent worktrees lose manifest ownership through read-modify-writeCLI reconcile and local-agent commands can operate on the same partition manifest concurrently. Each operation reads the whole JSON object, adds or removes its workspace key, then atomically replaces the whole file. Atomic rename prevents a partial file, but it does not prevent a lost update when both readers started from the same snapshot. I reproduced this locally by running await Promise.all([
reconcileMcpForConfig(teamConfig, cfgA),
reconcileMcpForConfig(teamConfig, cfgB),
]);Both worktree MCP files were written, but the shared manifest contained only one of the two expected ownership keys: {
"keys": ["claude:project:f837ecfc1203"]
}The other worktree therefore has a TeamAI-injected MCP with no ownership record; later update/remove/uninstall treats it as user-owned or leaves it behind. This does not require a large locking framework. The simplest design may be to store a separate manifest per worktree inside the partition, for example: Then CLI reconcile, local-agent install/uninstall, and reporting read only the current workspace's file, eliminating both write contention and key filtering. User scope can keep its existing global manifest. If retaining one shared JSON file is required, a small manifest-specific lock around the complete read → modify → write transaction is sufficient; all CLI and local-agent writers must use the same helper. Please add a concurrent regression asserting that both workspace ownership records survive simultaneous reconcile operations. |
…g shared-manifest lost updates (#374 P1-2C review) The shared partition manifest still lost ownership records under concurrent read-modify-write: two worktrees reconciling at once each read the same JSON snapshot, added their key, and atomically replaced the whole file — atomic rename prevents a torn file but not a lost update, so one worktree's ownership key was dropped and its injected MCP became orphaned. Fix (reviewer's preferred option): a manifest file PER WORKTREE at `<partition>/workspaces/<workspaceId>/managed-mcp.json`. Simultaneous reconciles now touch disjoint files, so neither concurrency loss nor cross-worktree bleed is possible — and the ownership key drops back to a plain `<tool>:project` (no workspace-id segment) with no key-filtering needed on read. User scope keeps its single global `<dataHome>/managed-mcp.json`. New `src/utils/mcp-manifest.ts` owns the per-worktree path + one-time compat migration: on first read of a worktree's file, records belonging to this worktree are lifted out of any legacy shared `<dataHome>/managed-mcp.json` (a bare `<tool>:project` when the data home is workspace-local, or an interim `<tool>:project:<thisId>` key) and written into the per-worktree file, then removed from the shared file. All writers/readers route through it: - mcp-reconcile: project → loadProjectMcpManifest; user → global file. - local-agent install/uninstall + report scan: same, per worktree. - uninstall + `mcp list`: read the scope-appropriate path. `managedMcpManifestKey` simplified to `(tool, projectScope)`; the workspace-id and resolveManagedMcpOwnership helpers are gone (folded into the per-worktree file + migration). Test plan (all green): - npx tsc --noEmit clean; npx vitest run — 191 files / 2648 tests pass - NEW concurrent regression (reviewer-requested): reconcile two worktrees sharing one partition via Promise.all → BOTH ownership records survive (each in its own per-worktree file) and both workspace .mcp.json get the server. - rewrote the sibling-worktree + legacy-migration + local-agent install tests to the per-worktree file layout; the real git-worktree report test still isolates A vs B. Refs #374 (P1-2C).
|
确认成立,已按你推荐的方案 a 修复(1fad7a9)。 根因:共享分区单文件在两个 worktree 并发 read-modify-write 时丢更新 —— 原子 rename 只防半写、不防「两者读同一快照 → 后写覆盖先写的 key」,一个 worktree 的所有权 key 被丢、注入的 MCP 变孤儿。 修复:project MCP manifest 改为每 worktree 一个文件 新增 测试:新增你要求的并发回归 —— 两个 worktree 共享一个分区、 |
|
[P1] Persist the per-worktree manifest before removing migrated ownership from the shared file
I reproduced this locally using the existing real-worktree report test: call After the first call, the B key has been deleted from Please make migration durable before deleting the source record (atomically write the per-worktree destination first, then best-effort remove the shared key), or defer both migration writes to an explicit save/commit step. A regression test should run the report twice and assert that both reports contain |
…igrated records (#374 P1-2C review) loadProjectMcpManifest removed the claimed records from the legacy shared file and rewrote that file, but never persisted the per-worktree DESTINATION — it only returned the migrated manifest in memory. So whenever the caller was read-only (report) or skipped its write (reconcile with wrote === false), the source record was deleted while the destination file never appeared: the injected MCP became orphaned. Reviewer reproduced it by running the report twice — the second call returned an empty MCP list. Fix: make migration durable inside the loader, independent of the caller. On a first-read migration it now atomically writes the per-worktree destination FIRST, then best-effort removes the claimed keys from the shared file. Destination-first ordering means a crash between the two writes leaves the records readable in both files (the per-worktree file wins), never in neither. Added a `dryRun` option that suppresses both writes (preview), threaded from reconcile's options. Test plan (all green): - npx tsc --noEmit clean; npx vitest run — 191 files / 2648 tests pass - extended the real git-worktree report test to run buildReportPayload TWICE: both reports contain `b-only`, and the per-worktree manifest file exists after the first (migrating) call — the exact durability repro from the review. Refs #374 (P1-2C).
|
确认成立,是我这轮引入的真 bug,已修复(d945dcb)。 修复:把迁移做成在 loader 内部持久、与调用方无关。首次读触发迁移时,先原子写 per-worktree 目标文件,成功后再 best-effort 删共享文件里的 claimed key。目标优先的顺序意味着两次写之间崩溃时记录在两个文件都可读(per-worktree 文件优先),绝不会两边都没有。另加 测试:把真 git-worktree 汇报测试扩成跑两次 |
|
[P1] Project uninstall deletes sibling-worktree ownership while leaving the sibling MCP installed The new per-worktree manifests live below the shared project partition, but I reproduced this locally with worktrees A and B sharing one
The failing regression assertion is effectively: expect((await fse.readJson(path.join(wtB, '.mcp.json'))).mcpServers.shared).toBeDefined();
expect(await fse.pathExists(managedMcpManifestPath(dataHome, wtB))).toBe(true);
// second assertion: expected false to be trueThis leaves B's injected server permanently orphaned; later update/uninstall paths can no longer prove ownership and will treat it as user-managed. It also makes the operation internally inconsistent regardless of the intended uninstall scope. Please define and implement one complete semantic before deleting the partition:
A regression test should set up two worktree manifests, uninstall from A, and assert that B never ends in the invalid state "server present, ownership missing". |
…ting the shared partition (#374 P1-2C review) `teamai uninstall` (project scope) removed managed MCP servers only for the current worktree, then executeRemoval deleted the WHOLE shared partition — including every sibling worktree's per-worktree manifest. Each sibling was left with a teamai-injected MCP server in its own <workspace>/.mcp.json whose ownership record had just been deleted: permanently orphaned, and later update/uninstall treats it as user-managed. Fix (project-wide semantic): before executeRemoval deletes the partition, the MCP cleanup now enumerates every linked worktree (new `listWorktrees()` in utils/git, from `git worktree list --porcelain`, realpath'd) and runs a `removeAll` reconcile for each — building a per-worktree LocalConfig (its projectRoot + resolved dataHome) so each worktree's own manifest AND its .mcp.json are cleared. User scope is unchanged (single global manifest → current config suffices). Test plan (all green): - npx tsc --noEmit clean; npx vitest run — 191 files / 2651 tests pass - NEW regression (mcp-reconcile.test.ts): two worktrees sharing one partition, both with `shared` installed; run removeAll for BOTH (what uninstall now does); assert NEITHER worktree ends in "server present, ownership missing" — the server is gone from both .mcp.json and neither manifest still owns it. - NEW listWorktrees tests (anchors.test.ts): lists main + linked worktree from a subdirectory; returns [] outside a git repo. Refs #374 (P1-2C).
|
确认成立,已按 project-wide 语义修复(87f0be8)。 采纳你给的方案一(project-wide uninstall):在 executeRemoval 删分区之前,MCP 清理阶段现在枚举所有 linked worktree(新增 测试(你要求的):
2651 单测全绿。 |
Context
Completes the P1-2 partition routing deferred from #406. Two A1 items still landed
in the business workspace for a project install; they now follow the
machine-data home so a partitioned install has zero teamai residue:
managed-mcp.json— the per-project MCP injection manifest.local-agent/resources— the ~4MB skill cache the issue calls out.(local-agent's own config/manifest are A2 — already global under
~/.teamai— andare left untouched, per the issue's A1/A2 split.)
Why these were deferred (the desync trap)
Both are resolved from two sides: callers holding a full
LocalConfig(
mcp-reconcile/mcp-cmd/uninstall) and the local-agent subsystem,which only carries
(scope, projectRoot)and has noLocalConfig. Converging oneside alone would make a partitioned install's writer and reader disagree
(partition vs legacy) and desync. So both must resolve through one source.
What this PR does
managedMcpManifestPath(dataHome)now takes a resolved data home;LocalConfig holders pass
getDataHome(localConfig).resolveDataHomeForScope(scope, projectRoot)(config.ts) reproducesdetection's double-read: user →
~/.teamai; project →getDataHomeofdetectProjectConfig's result (partition for a new/migrated install, elselegacy
<projectRoot>/.teamai). The local-agent managed-mcp + resource pathsroute through it, so both sides always land on the identical directory.
getResourceRepoPath(local-agent) is now async and uses the shared resolverfor its project branch; user scope unchanged (
~/.teamai/local-agent/resources).ensureProjectGitignoreruns only when the cache actually lands inside theworkspace (a legacy install); a partitioned cache is under
~/.teamai, sothere's nothing in the workspace to ignore.
Test plan (all executed green)
npx tsc --noEmit— cleannpx vitest run— 191 files / 2644 tests passdetect-subdir.test.ts) prove thedesync guard:
resolveDataHomeForScope==getDataHome(detectProjectConfig(...))for legacy, subdirectory, user, non-git, and partitioned installs
(config in
~/.teamai/projects/<slug>/resolves to the partition fromboth sides).
Note: the full http-backend local-agent install flow (
install_skill/install_mcpend-to-end) is covered by the 65 existing local-agent unit tests; a live-backend
e2e remains a manual TODO, as with the http provider.
Refs #374 (P1-2C).