diff --git a/CHANGELOG.md b/CHANGELOG.md index 600586f1c..64216bf37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ All notable changes to this project will be documented in this file. See [standa ### 🐛 Bug Fixes +- `teamai pull` keeps a skill, rule or agent you changed since teamai delivered it instead of overwriting it, and names it: `Kept : you changed it since teamai delivered it`, with a warning when the team version has changed since, which `teamai push` repeats for that copy, since the SessionStart pull is silent. Pull records the sha256 of what it writes at each path in the checkout's record, and the pre-push sync records its writes too, so a copy counts as changed only against that record; a skill counts as one copy, and files only you added do not count. The copies of other tools still update. `--force` keeps these copies, `--dry-run` prints `Would keep `, and a copy of an item the team removed stays when you changed it. To take the team version, delete your copy and run `teamai pull --force`. There is no record before the first full pull on this version, or in a new worktree, so that pull overwrites as before. A file you added to a skill at a path another team version of it has is no longer named on every pull, and `teamai doctor` no longer fails a rules or agents check, or points at `teamai pull --force`, for a kept copy: it lists one as `changed by you (kept by pull)` beside any other problem (for [#822](https://github.com/Tencent/teamai-cli/issues/822)). - On macOS, the git commands behind pull, push, reports and learnings publishing no longer wait on a PATH search each time they run. teamai spawned `git` by name, and on macOS that lookup costs a few milliseconds for each PATH entry ahead of git's directory: 30-67 ms per call with a typical PATH, while git itself takes about 8 ms. teamai now runs the `git` that lookup would pick by its absolute path, found once and again whenever PATH changes or that `git` is gone or no longer executable, and only once a `git --version` by that path starts. On macOS 26 an up-to-date `teamai pull` drops from 1.3 s to 0.4 s with a 42-entry PATH, and from 2.0 s to 0.4 s with the PATH an npm script gives; macOS 27 no longer shows the lookup cost. Windows, a PATH with an empty or relative entry, a first `git` on PATH that does not start (a missing interpreter, no execute permission), and the few one-off git calls outside these paths (provider clone, version and user probes) keep the bare-name spawn (for [#868](https://github.com/Tencent/teamai-cli/issues/868)). - On Node 24, `teamai codebase --extract`, and the `teamai import` and CI extract paths that run it, no longer abort with `Fatal process out of memory: Zone` on a repository with `.swift` files. V8's optimizing Wasm compiler (nodejs/node#63421) ran out of memory on the tree-sitter grammars, so the AST track now keeps them on V8's baseline tier on Node 24 and later. That also cuts the TypeScript grammar's peak memory there from about 1.4 GB to about 0.1 GB, for a parse about 1.6x slower; Node 20 and 22 are unchanged (for [#860](https://github.com/Tencent/teamai-cli/issues/860)). - `teamai remove mcp ambiguous` removes a server named `ambiguous` from `mcp/mcp.yaml`. The name matched the value `remove` used internally to mean "refused", so the command printed `Nothing was removed.`, exited 1, and gave no reason (for [#862](https://github.com/Tencent/teamai-cli/issues/862)). diff --git a/docs/designs/data-directory-layout.md b/docs/designs/data-directory-layout.md index 3a3ffe3a0..828821a1c 100644 --- a/docs/designs/data-directory-layout.md +++ b/docs/designs/data-directory-layout.md @@ -75,6 +75,8 @@ directories do not authorize writes to rules excluded by the local configuration For Copilot updates that only change `paths`, it compares the entire local file with the rendered recorded versions before refreshing `applyTo`, preserving locally edited headers rather than overwriting them on a body match alone. +Each copy it writes, in any format, is recorded in the checkout's `delivered` +(#822), so the next pull does not keep it as the member's edit. A placed agent, which push does not sync, is held when the team file has changed since any of those revisions, or since it was added if one of them predates it (#823). That sync brings the diff --git a/docs/designs/multi-project-management.md b/docs/designs/multi-project-management.md index 51511ee6b..a3f15deee 100644 --- a/docs/designs/multi-project-management.md +++ b/docs/designs/multi-project-management.md @@ -279,7 +279,9 @@ namespace) has and the new one lacks, when they match that version byte for byte, so switching versions leaves no team file behind; a file the member added or edited stays, because push never counts such extras as changes and they may never have been pushed, and one at a path another version has is named on each -pull. +pull. With a record of what teamai delivered (below), a file at such a path is +also removed when it is still what teamai wrote there, and one teamai has no +record of is the member's own and stays without a warning. An item that cannot be used replaces nothing. A skill directory without `SKILL.md` is not a skill: it is left out of the desired set, so the root skill @@ -457,10 +459,36 @@ legacy mode each repeated name. `teamai env|mcp|hooks|models list`, `teamai list --source repo` and `teamai status` show where each entry comes from. +### Local edits (#822) + +Each checkout record (`lastPullByWorkspace[]`, HOME's for the user +scope) carries `delivered`: the sha256 of the bytes teamai last wrote at each +skill, rule and agent file path. Pull and the pre-push sync update it when they +write, through the same state save. A copy is the member's edit only when it +has a record and no longer matches it. Pull keeps such a copy and names it: an +info line when the team version is unchanged, a warning when it has moved. +Push warns about such a copy as well (the SessionStart pull is silent), without +holding it, since the member may have merged the team change already. A +skill directory is one unit, and files only the member added are not recorded. +Tombstone cleanup keeps an edited copy the same way, and so does the rules +sweep of a rule no longer delivered (deleted from the team repo, or of a +namespace the member left). `--force` keeps edits; +deleting the copy and running `pull --force` takes the team version. A record +without `delivered` (the first pull on this version, a new worktree) protects +nothing, and a copy teamai never delivered to that path is overwritten as +before. A forced full sync elsewhere keeps each checkout's `delivered`. +`doctor` does not fail on a kept copy; next to another problem it lists one +as "changed by you (kept by pull)". + ### Known gaps -- No pull protects a local edit from being overwritten, override transitions - included. +- `teamai remove`'s rules refresh and local-agent installs deliver rules and + skills as before: they overwrite a changed copy and record nothing. If the + team changes a copy they wrote before the next pull, that pull keeps it as an + edit. +- Step 3b and the inactive-namespace cleanup of skills and agents still compare + with the team source, not the record, so an untouched copy delivered at an + older revision stays there with a warning. ## Backward compatibility diff --git a/docs/usage-guide.md b/docs/usage-guide.md index e7cd0c017..e93cd5245 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -645,6 +645,8 @@ teamai pull --dry-run # Dry run, no actual changes A manual `teamai pull` ends by running the `teamai doctor` checks and printing each one that failed, with its fix — including whether the skills it just reported syncing are readable on disk for every enabled tool. It prints nothing when they all pass, and the exit code is unchanged. The SessionStart hook path and `--dry-run` run no checks at all, so session startup stays as fast as before. Provider checks (`gh`/`gf` authentication) are left to `teamai doctor`: the pull just used the provider. +**Pull keeps a skill, rule or agent you changed.** For each checkout, pull records what it wrote at each skill, rule and agent path. On a full sync, a copy that no longer matches that record is kept, and pull names it, while the copies of other tools still update. A skill counts as one copy: a change to any of its team files keeps the whole skill, and files only you added do not count. If the team version has not changed, pull prints ``Kept : you changed it since teamai delivered it. Share it with `teamai push`, or delete it and run `teamai pull --force` to get the team version back.`` If it has, pull warns and asks you to merge the team change into your copy before you push it, and `teamai push` warns about that copy too, since the SessionStart pull runs silently. `--force` keeps these copies too, and `--dry-run` prints `Would keep ` for each. When the team removes an item, a copy you changed stays, and pull names it. There is no record before your first full pull with this version, so that pull overwrites as earlier versions did, and your changes are protected from then on. The same goes for a new worktree's first pull, and for a copy teamai never delivered to that path. `teamai remove` and installs from the local agent still rewrite the team rules without this check. An older CLI that saves state drops the record. + > Project scope is isolated by default. When the current working directory contains a project-scope `.teamai/config.yaml`, `pull` processes that project and skips user scope unless the local config has `inheritUserScope: true`; in that case it first refreshes the safe user-resource channel. Without a project config in the current directory, `pull` processes user scope. User `env`, MCP definitions, sources, reporting, and writes remain isolated in project mode. Hooks are the one exception: a project scope's hooks are injected into your **HOME** tool settings (`~/.claude/settings.json`, …), not ``, because the built-in hooks gate on the `cwd` handed to `hook-dispatch` and `~/.claude` always exists so the "installed tool" gate passes (see the Hooks section). In a directory with no teamai config (no project config and no user scope), the team hooks do nothing: no reminders, and no session or skill usage is recorded; only machine-level work runs (the CLI update check, the session-start pull, the local agent, and package hints a pull stashed). For the team hooks and skill usage, a project config that exists but cannot be read counts as none, never as the user scope or as a lower-priority project config (such as a legacy `.teamai/config.yaml`) behind it. `pull` follows the same rule: it syncs no scope there, prints ``Nothing was synced: : . Fix the file, or move it aside and run `teamai init` to write a new one.`` and exits 1 (with `--silent`, it prints nothing and still exits 1); a session start there runs no pull, seeds no agent directory and stashes no package hint. A hook whose `cwd` was deleted (a session that outlives its worktree) keeps the scope its session last recorded, so the session's last events and skill uses stay with the project, and its share reminder follows the project's settings, instead of the user scope's. This needs the session's earlier events in the local event log, which compaction trims to active sessions, and does not cover Copilot, whose events record no directory. Self single-repo mode keeps its hooks in the business repo so they travel on clone. With role-based skills enabled, `pull`'s skill sync source becomes the contents of `skills//`, expanded according to `primaryRole + additionalRoles` and flattened into each local AI tool's skills directory. `rules//` and `claudemd//` follow the `knowledge` namespaces, and a `docs//` follows the `docs` namespaces once one is declared (see [Docs](#docs)); `agents//` follows the role's `agents` namespaces (see [Agents Resource Type](#agents-resource-type)). `learnings/` at the root is shared with everyone, while `learnings//` subdirectories sync only for the directory's active projects (see [Multi-project](#multi-project-project-as-a-dimension-orthogonal-to-role)). @@ -753,7 +755,7 @@ Exclusion rules take effect after role and tag filtering. When running `teamai p ### Push local resources -Before scanning, `push` refreshes unedited old rule copies from the team repo. For Copilot, it compares Markdown bodies independently of the generated `applyTo` header and renders updates in `.instructions.md` format. Local body edits are preserved. This applies to project rules and user rules under `COPILOT_HOME`. +Before scanning, `push` refreshes unedited old rule copies from the team repo. For Copilot, it compares Markdown bodies independently of the generated `applyTo` header and renders updates in `.instructions.md` format. Local body edits are preserved. This applies to project rules and user rules under `COPILOT_HOME`. Each copy it refreshes is recorded as delivered, so the next `teamai pull` still updates it instead of keeping it as your change. When only the team's `paths` change, `push` also refreshes Copilot's `applyTo` if the local file still matches a recorded version's generated copy. A locally edited header is preserved in this case. @@ -790,7 +792,7 @@ Choose namespace [1-3] (default: 1 = common): - `--role`/`--project` places new resources only. An edit of a shared-root rule or agent stays at the shared root, and push says so - A placed resource stays maintainable from the machine that published it. While its PR is open, the open-PR record routes a later edit of the author's own copy back to that PR; once the file is on the default branch, `state.json` records where push put it, so the edit goes back to the same file, and an agent published into a namespace this directory has not activated is still editable rather than skipped as having no active source - `teamai remove rules ` accepts the bare name the author's copy carries as well as the published `/`; it reports which one it resolved to, and removes both the namespaced team file and the author's copy at the rules root. If the team repo cannot be refreshed first, or this machine's placement records cannot be updated and saved, `remove` stops with exit 1 and removes nothing, because either can resolve the name to the wrong files -- A local agent is an edit of the team agent it was delivered from: one in an active namespace first, then one this machine placed, then the shared-root agent either of them replaces. Only when none exists does `--role`/`--project` decide, and the agent is new in that namespace; if that namespace already holds an agent of that name, the agent is skipped rather than written over it, as a rule would be. Two active agents of one name stay ambiguous and are skipped, flag or not. The same agent name may exist in several namespaces, so a copy in an inactive one you did not name never blocks publishing yours. A placed agent that changed on the team since this checkout last synced it is held until you run `teamai pull`, because agents have no pre-push sync. In single-repo mode, a root copy under `.teamai/` that matches an older version of the file it was placed at is held too: nothing refreshes it, so it is an old copy rather than an edit +- A local agent is an edit of the team agent it was delivered from: one in an active namespace first, then one this machine placed, then the shared-root agent either of them replaces. Only when none exists does `--role`/`--project` decide, and the agent is new in that namespace; if that namespace already holds an agent of that name, the agent is skipped rather than written over it, as a rule would be. Two active agents of one name stay ambiguous and are skipped, flag or not. The same agent name may exist in several namespaces, so a copy in an inactive one you did not name never blocks publishing yours. A placed agent that changed on the team since this checkout last synced it is held, because agents have no pre-push sync. Pull keeps your changed copy, so save your edit, delete the copy, run `teamai pull --force`, reapply the edit and push again. In single-repo mode, a root copy under `.teamai/` that matches an older version of the file it was placed at is held too: nothing refreshes it, so it is an old copy rather than an edit - A new resource is never placed on top of one that is already there. If the resolved namespace already holds that name, the push stops and names the file: pull and edit the existing copy, rename yours, or pick another namespace with `--role ` - An agent whose namespace is not active here stays editable through its placement record, and `pull` delivers it for the same reason, so your copy tracks the team file. It replaces a shared-root agent of the same name, as an active namespace's agent would. An active namespace holding that name wins: that agent is the one deployed here - A resource awaiting review in an open PR keeps that PR's destination — unless this push names a namespace other than the one recorded (the shared root counts as one), in which case the flag decides, the open PR is left untouched, and the collision is reported diff --git a/docs/usage-guide.zh-CN.md b/docs/usage-guide.zh-CN.md index 71232ec19..771d86d72 100644 --- a/docs/usage-guide.zh-CN.md +++ b/docs/usage-guide.zh-CN.md @@ -574,6 +574,8 @@ teamai pull --dry-run # 试运行，不实际修改 手动执行 `teamai pull` 会在结束时运行 `teamai doctor` 的检查，并逐条打印失败项及其修复建议——包括它刚刚报告同步的 skill 是否真的落到每个启用工具的磁盘上、且可被读取。全部通过时不会有任何额外输出，退出码也不变。SessionStart hook 路径和 `--dry-run` 完全不运行检查，会话启动速度保持不变。托管平台相关的检查（`gh`/`gf` 认证）留给 `teamai doctor`：这次 pull 刚刚用过该平台。 +**pull 会保留你修改过的 skill、rule 和 agent。** pull 按检出记录它在每个 skill、rule、agent 路径写入的内容。完整同步时，与记录不一致的副本会被保留并由 pull 指出，其他工具的副本照常更新。一个 skill 算作一份副本：它的任一团队文件被改动，整个 skill 都会保留；只有你自己添加的文件不计入。团队版本没有变化时，pull 输出 ``Kept : you changed it since teamai delivered it. Share it with `teamai push`, or delete it and run `teamai pull --force` to get the team version back.``；团队版本也变了时，pull 给出警告，请你先把团队的改动合并进自己的副本，再 push；由于 SessionStart 时的 pull 不输出信息，`teamai push` 也会对该副本给出警告。`--force` 同样保留这些副本，`--dry-run` 会逐个输出 `Would keep `。团队删除某项资源时，你修改过的副本也会保留，并由 pull 指出。升级后第一次完整 pull 之前还没有记录，因此那次 pull 仍像旧版本一样覆盖，此后你的修改才受保护。新 worktree 的第一次 pull、以及 teamai 从未写入过该路径的副本，同样如此。`teamai remove` 和本地 agent 的安装仍会不经这项检查重写团队 rule。旧版 CLI 保存 state 时会丢弃这份记录。 + > Project scope 默认与 user scope 隔离。当前工作目录包含 project scope 的 `.teamai/config.yaml` 时，`pull` 会处理该项目并跳过 user scope；仅当本地配置包含 `inheritUserScope: true` 时，才会先刷新安全的 user 资源通道。当前目录没有 project 配置时，`pull` 处理 user scope。project 模式下，user 的 `env`、MCP 定义、sources、reporting 和写入行为仍保持隔离。hooks 是唯一例外：project scope 的 hooks 会注入到你的 **HOME** 工具设置（`~/.claude/settings.json` 等），而非 ``——因为内置 hooks 依据传给 `hook-dispatch` 的 `cwd` 门控，且 `~/.claude` 恒存在、能通过「已安装工具」门槛（详见 Hooks 章节）。在没有 teamai 配置的目录中（既没有 project 配置也没有 user scope），团队 hooks 不做任何事：不显示提醒，也不记录会话或 skill 使用；只运行机器级别的工作（CLI 更新检查、SessionStart 时的 pull、本地 agent，以及 pull 暂存的包提示）。对团队 hooks 和 skill 使用记录而言，存在但无法读取的 project 配置视为没有配置，而不会退回 user scope，也不会退回其后优先级更低的 project 配置（如旧的 `.teamai/config.yaml`）。`pull` 遵循同一规则：此时不同步任何 scope，输出 ``Nothing was synced: : . Fix the file, or move it aside and run `teamai init` to write a new one.`` 并以 exit 1 退出（加 `--silent` 时不输出，但仍以 exit 1 退出）；会话启动时不运行 pull，也不创建 agent 目录、不暂存包提示。`cwd` 已被删除的 hook（会话比它的 worktree 活得更久）沿用该会话最后记录的 scope，因此会话最后的事件和 skill 使用仍归属项目，分享提醒也遵循项目的设置，而不是 user scope 的。这需要本地事件日志中仍保留该会话之前的事件（压缩只保留活跃会话），且不适用于 Copilot，因为它的事件不记录目录。self 单仓模式则把 hooks 保留在业务仓库里，随 clone 传播。 启用角色化 skills 后，`pull` 的 skills 同步来源会变成 `skills//` 中的内容，按 `primaryRole + additionalRoles` 展开对应的 namespace，拍平安装到本地各 AI 工具 skills 目录。`rules//` 和 `claudemd//` 按 `knowledge` namespace 同步，`docs//` 在被声明后按 `docs` namespace 同步（见 [Docs（文档）](#docs文档)）；`agents//` 按角色的 `agents` namespace 同步（见 [Agents 资源类型](#agents-资源类型)）。`learnings/` 根目录对所有人共享，而 `learnings//` 子目录只对本目录激活的项目同步（见 [多项目](#多项目project-作为与-role-正交的维度)）。 @@ -682,7 +684,7 @@ excludedSkills: ### 推送本地资源 -扫描前，`push` 会用团队仓库的新版刷新未修改的旧规则副本。对于 Copilot，会单独比较 Markdown 正文，忽略自动生成的 `applyTo` 头，并以 `.instructions.md` 格式写入更新；本地正文编辑会保留。此行为适用于项目规则和 `COPILOT_HOME` 下的用户规则。 +扫描前，`push` 会用团队仓库的新版刷新未修改的旧规则副本。对于 Copilot，会单独比较 Markdown 正文，忽略自动生成的 `applyTo` 头，并以 `.instructions.md` 格式写入更新；本地正文编辑会保留。此行为适用于项目规则和 `COPILOT_HOME` 下的用户规则。它刷新的每份副本都会记录为 teamai 写入的内容，因此下一次 `teamai pull` 仍会更新它，而不会当作你的修改保留。 团队仅修改 `paths` 时，只要本地文件仍与某个已记录版本的生成副本一致，`push` 也会刷新 Copilot 的 `applyTo`；此时本地手动修改过的头部会保留。 @@ -719,7 +721,7 @@ Choose namespace [1-3] (default: 1 = common): - `--role`/`--project` 只放置新资源。对共享根目录 rule 或 agent 的修改仍留在共享根目录，push 会给出提示 - 已落点的资源在发布它的机器上仍可维护：PR 未合并期间，待评审 PR 记录会把作者对自己副本的修改带回该 PR；文件进入默认分支后，`state.json` 会记录 push 的落点，因此修改仍会写回同一个文件；即使 agent 落在本目录未激活的 namespace，也不会被当作“无活跃源”跳过 - `teamai remove rules ` 同时接受作者副本的简名和发布名 `/`：会打印实际解析到的名字，并同时删除带 namespace 的团队文件和作者在 rules 根目录的副本。若无法先刷新团队仓库，或本机的落点记录无法更新并保存，`remove` 会以退出码 1 停止且不删除任何内容，因为两者都可能把名字解析到错误的文件 -- 本地 agent 被视为其来源团队 agent 的编辑：优先是活跃 namespace 中的 agent，其次是本机放置的 agent，最后是被二者替换的共享根目录 agent。只有三者都不存在时，才由 `--role`/`--project` 决定，此时该 agent 在该 namespace 中是新的；若该 namespace 已有同名 agent，则跳过该 agent 而不是覆盖它，与 rule 的处理一致。两个活跃的同名 agent 无论是否指定参数都视为有歧义并跳过。同名 agent 允许存在于多个 namespace，因此你未指定的非活跃 namespace 中的同名副本不会阻止你发布。本机放置的 agent 若在当前检出上次同步后被团队修改，会暂缓推送，直到你运行 `teamai pull`，因为 agents 没有推送前同步。单仓库模式下，`.teamai/` 中的根目录副本若与其落点文件的某个旧版本相同，也会暂缓推送：没有任何操作会刷新它，因此它是旧副本而不是编辑 +- 本地 agent 被视为其来源团队 agent 的编辑：优先是活跃 namespace 中的 agent，其次是本机放置的 agent，最后是被二者替换的共享根目录 agent。只有三者都不存在时，才由 `--role`/`--project` 决定，此时该 agent 在该 namespace 中是新的；若该 namespace 已有同名 agent，则跳过该 agent 而不是覆盖它，与 rule 的处理一致。两个活跃的同名 agent 无论是否指定参数都视为有歧义并跳过。同名 agent 允许存在于多个 namespace，因此你未指定的非活跃 namespace 中的同名副本不会阻止你发布。本机放置的 agent 若在当前检出上次同步后被团队修改，会暂缓推送，因为 agents 没有推送前同步。pull 会保留你修改过的副本，因此请先另存你的修改，删除该副本，执行 `teamai pull --force`，重新应用修改后再 push。单仓库模式下，`.teamai/` 中的根目录副本若与其落点文件的某个旧版本相同，也会暂缓推送：没有任何操作会刷新它，因此它是旧副本而不是编辑 - 新资源绝不会覆盖已存在的资源：若解析出的 namespace 下已有同名文件，命令会报错并指出该文件：请先 pull 并修改已有副本、重命名自己的资源，或用 `--role ` 换一个 namespace - 本目录未激活的 namespace 下的 agent 可通过落点记录继续编辑，`pull` 也会基于同一记录下发它，使本地副本与团队文件保持同步；它会像活跃 namespace 中的 agent 一样替换共享根目录的同名 agent。若已激活的 namespace 中已有同名 agent，则以它为准 - 待评审 PR 中的资源默认沿用该 PR 的落点；但若本次 push 明确指定的 namespace 与记录的落点不同（共享根目录也算一种落点），则以命令行为准，原 PR 保持不动，并提示该冲突 diff --git a/skill-data/core/SKILL.md b/skill-data/core/SKILL.md index dd73fdaee..af0bd6f5b 100644 --- a/skill-data/core/SKILL.md +++ b/skill-data/core/SKILL.md @@ -109,6 +109,13 @@ generated reference below. Read it instead of guessing a flag. removing stale and local-only documents; an edited doc of a docs namespace you left is kept and named. Use a dedicated directory; preview with `--dry-run`. +`teamai pull` keeps a skill, rule or agent copy the user changed since teamai +delivered it, `--force` included, and names it (`Kept : ...`). To share +the change, `teamai push`; when pull or push says the team version has changed +since, merge that change into the copy first, or the push replaces it. To take the team version instead, delete +the copy and run `teamai pull --force`. The first pull after upgrading, and a +new worktree's first pull, still overwrite: nothing is recorded yet. + ## References In the files below, `{SKILL_DIR}` is the directory `teamai skill path core` prints; a reference file you open on its own writes that directory as `SKILL_DIR` in braces. diff --git a/skill-data/core/references/contribute-member.md b/skill-data/core/references/contribute-member.md index 29209f25f..40a3f8b36 100644 --- a/skill-data/core/references/contribute-member.md +++ b/skill-data/core/references/contribute-member.md @@ -123,6 +123,7 @@ edit: unedited old instructions update in native format, including under Rule pre-sync leaves tools excluded by `enabledAgents` or `disabledAgents` untouched. When only team `paths` change, `applyTo` refreshes if the local file still matches a recorded version's generated copy; locally edited headers are kept. +The copies push refreshes are recorded, so a later `teamai pull` still updates them. ## If push is denied diff --git a/src/__tests__/delivered-copies.test.ts b/src/__tests__/delivered-copies.test.ts new file mode 100644 index 000000000..4440484eb --- /dev/null +++ b/src/__tests__/delivered-copies.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from 'vitest'; +import fc from 'fast-check'; +import { classifyCopy, type DeliveredFile } from '../resources/delivered-copies.js'; + +// #822 item 5: pull keeps a copy only when the record proves teamai wrote +// other bytes there than the member has now. +describe('classifyCopy', () => { + const file = (disk: string | null, recorded: string | undefined, next: string | null): DeliveredFile => ( + { disk, recorded, next } + ); + + it('writes a copy teamai has no record of', () => { + expect(classifyCopy([file('mine', undefined, 'team')])).toEqual({ kind: 'write' }); + }); + + it('writes an untouched copy, and one already at the team version', () => { + expect(classifyCopy([file('v1', 'v1', 'v2')])).toEqual({ kind: 'write' }); + expect(classifyCopy([file('v2', 'v1', 'v2')])).toEqual({ kind: 'write' }); + }); + + it('writes a copy the member deleted, so pull brings the team version back', () => { + expect(classifyCopy([file(null, 'v1', 'v1')])).toEqual({ kind: 'write' }); + expect(classifyCopy([file(null, 'v1', 'v1'), file(null, 'x1', 'x2')])).toEqual({ kind: 'write' }); + }); + + it('keeps an edited copy and says whether the team version moved since', () => { + expect(classifyCopy([file('edit', 'v1', 'v1')])).toEqual({ kind: 'keep', teamChanged: false }); + expect(classifyCopy([file('edit', 'v1', 'v2')])).toEqual({ kind: 'keep', teamChanged: true }); + }); + + it('keeps a whole skill when one file of it was edited or deleted', () => { + expect(classifyCopy([file('s1', 's1', 's1'), file('edit', 'x1', 'x1')])).toEqual({ kind: 'keep', teamChanged: false }); + expect(classifyCopy([file('s1', 's1', 's1'), file(null, 'x1', 'x1')])).toEqual({ kind: 'keep', teamChanged: false }); + // A file the team added, or removed, since the last delivery is a team change. + expect(classifyCopy([file('edit', 's1', 's1'), file(null, undefined, 'new')])).toEqual({ kind: 'keep', teamChanged: true }); + expect(classifyCopy([file('edit', 's1', 's1'), file('old', 'old', null)])).toEqual({ kind: 'keep', teamChanged: true }); + }); + + it('keeps nothing without proof: only a recorded file whose bytes are neither the record nor the team version', () => { + const hash = fc.constantFrom('a', 'b', 'c'); + const files = fc.array(fc.record({ + disk: fc.option(hash, { nil: null }), + recorded: fc.option(hash, { nil: undefined }), + next: fc.option(hash, { nil: null }), + }), { maxLength: 4 }); + fc.assert(fc.property(files, (copy) => { + const proven = copy.some((f) => f.recorded !== undefined && f.disk !== f.recorded && f.disk !== f.next) + && copy.some((f) => f.disk !== null); + expect(classifyCopy(copy).kind).toBe(proven ? 'keep' : 'write'); + })); + }); +}); diff --git a/src/__tests__/doctor-rules-delivery.test.ts b/src/__tests__/doctor-rules-delivery.test.ts index 411f5b752..9cbc8d07e 100644 --- a/src/__tests__/doctor-rules-delivery.test.ts +++ b/src/__tests__/doctor-rules-delivery.test.ts @@ -19,9 +19,11 @@ vi.mock('../utils/logger.js', () => ({ setStderrOnly: vi.fn(), })); -import { loadLocalConfig, loadTeamConfig } from '../config.js'; +import crypto from 'node:crypto'; +import { loadLocalConfig, loadStateForScope, loadTeamConfig } from '../config.js'; import { buildChecks, resolveDoctorContext, type Check } from '../doctor.js'; -import type { LocalConfig, TeamaiConfig } from '../types.js'; +import { checkoutKey } from '../pull.js'; +import { StateSchema, type LocalConfig, type TeamaiConfig } from '../types.js'; /** * The rules half of the delivery check (#624). A rule changes both its filename @@ -201,6 +203,29 @@ describe('doctor — rules delivered on disk', () => { expect(claude.fix).toContain('delivered from an older copy: reviews'); }); + it('passes a copy the member changed since teamai delivered it, which pull keeps (#822)', async () => { + const edited = path.join(homeDir, CLAUDE_RULES, 'reviews.md'); + await fse.writeFile(edited, 'My own version\n'); + await deliverMdc('coding-style'); + await deliverMdc('reviews'); + const delivered = { [edited]: crypto.createHash('sha256').update('Body of reviews\n').digest('hex') }; + vi.mocked(loadStateForScope).mockResolvedValue(StateSchema.parse({ + lastPullByWorkspace: { [await checkoutKey(homeDir)]: { rev: 'r1', targets: [], delivered } }, + })); + try { + const withMissing = await rulesCheck('claude'); + expect(await withMissing.check()).toBe(false); + expect(withMissing.fix).toContain('not delivered: coding-style; changed by you (kept by pull): reviews.'); + expect(withMissing.fix).not.toContain('delivered from an older copy: reviews'); + + // Only the member's change is left: nothing is wrong with the delivery. + await deliverPlain(CLAUDE_RULES, 'coding-style'); + expect(await (await rulesCheck('claude')).check()).toBe(true); + } finally { + vi.mocked(loadStateForScope).mockResolvedValue(StateSchema.parse({})); + } + }); + it('treats a plain .md rule as applicable without frontmatter', async () => { await deliverPlain(CLAUDE_RULES, 'coding-style'); await deliverPlain(CLAUDE_RULES, 'reviews'); diff --git a/src/__tests__/e2e/copilot-pre-push-sync.test.ts b/src/__tests__/e2e/copilot-pre-push-sync.test.ts index abd545024..2ee4c96ec 100644 --- a/src/__tests__/e2e/copilot-pre-push-sync.test.ts +++ b/src/__tests__/e2e/copilot-pre-push-sync.test.ts @@ -109,6 +109,15 @@ it('push refreshes an unedited Copilot rule instead of pushing a rollback, and p expect(fs.readFileSync(deployed, 'utf8')).toBe(teamRuleToCopilotInstructions(pathsOnlyUpdate)); expect(git(['for-each-ref', '--format=%(refname)', 'refs/heads/teamai/push/'], remote)).toBe(''); + // Push recorded the copy it refreshed (#822), so pull updates it rather + // than keeping it as the member's edit. + const v3 = pathsOnlyUpdate.replace('Use the new endpoint.', 'Use the v3 endpoint.'); + put(path.join(teammate, 'rules/api.md'), v3); + git(['commit', '-q', '-am', 'Teammate updates rule again'], teammate); + git(['push', '-q', 'origin', 'main'], teammate); + expect(run(['pull'])).not.toContain('Kept'); + expect(fs.readFileSync(deployed, 'utf8')).toBe(teamRuleToCopilotInstructions(v3)); + const edited = `${fs.readFileSync(deployed, 'utf8')}\nMy local addition.\n`; put(deployed, edited); expect(run(['--dry-run', 'push'])).toContain('[rules] api (modified)'); diff --git a/src/__tests__/e2e/pull-keeps-edits-822.test.ts b/src/__tests__/e2e/pull-keeps-edits-822.test.ts new file mode 100644 index 000000000..41a8c77c4 --- /dev/null +++ b/src/__tests__/e2e/pull-keeps-edits-822.test.ts @@ -0,0 +1,361 @@ +/** + * E2E (#822 item 5): pull keeps a skill, rule or agent the member changed + * since teamai delivered it, instead of overwriting it. + * + * Pull records the sha256 of the bytes it writes at each destination in the + * checkout's record. A copy is edited when it has a record and no longer + * matches it; it is kept and named, per tool, while the other tools' copies + * update. A copy teamai has no record of (the first pull on this version) is + * overwritten as before, and protected from then on. + */ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { execFileSync, spawn } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { projectSlug } from '../../utils/partition.js'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const ROOT = path.resolve(__dirname, '..', '..', '..'); +const CLI = path.join(ROOT, 'dist', 'index.js'); + +const GIT_ENV = { + GIT_AUTHOR_NAME: 'TeamAI CI', + GIT_AUTHOR_EMAIL: 'ci@teamai.test', + GIT_COMMITTER_NAME: 'TeamAI CI', + GIT_COMMITTER_EMAIL: 'ci@teamai.test', +}; + +interface RunResult { + code: number | null; + output: string; +} + +function runCLI(args: string[], cwd: string, home: string): Promise { + return new Promise((resolve) => { + const child = spawn('node', [CLI, ...args], { + cwd, + env: { ...process.env, ...GIT_ENV, HOME: home, FORCE_COLOR: '0' }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + let output = ''; + child.stdout.on('data', (data: Buffer) => { output += data.toString(); }); + child.stderr.on('data', (data: Buffer) => { output += data.toString(); }); + child.on('close', (code) => resolve({ code, output })); + }); +} + +function git(args: string[], cwd: string): void { + execFileSync('git', args, { cwd, stdio: 'pipe', env: { ...process.env, ...GIT_ENV } }); +} + +const SKILL_MD = '---\nname: team-skill\ndescription: Team skill fixture\n---\n\n# Team skill\n'; +const agentYaml = (instructions: string): string => [ + 'name: team-helper', + 'description: Team helper fixture', + 'targets:', + ' - claude', + ' - cursor', + 'instructions: |', + ` ${instructions}`, + '', +].join('\n'); + +describe('pull keeps a delivered copy the member changed (#822 item 5)', () => { + let sandbox: string; + let home: string; + let projectRoot: string; + let teammate: string; + + const claudeSkill = () => path.join(projectRoot, '.claude', 'skills', 'team-skill'); + const claudeScript = () => path.join(claudeSkill(), 'scripts', 'run.sh'); + const claudeRule = () => path.join(projectRoot, '.claude', 'rules', 'team-rule.md'); + const cursorRule = () => path.join(projectRoot, '.cursor', 'rules', 'team-rule.mdc'); + const claudeAgent = () => path.join(projectRoot, '.claude', 'agents', 'team-helper.md'); + const cursorAgent = () => path.join(projectRoot, '.cursor', 'agents', 'team-helper.md'); + const cursorScript = () => path.join(projectRoot, '.cursor', 'skills', 'team-skill', 'scripts', 'run.sh'); + const statePath = () => path.join(home, '.teamai', 'projects', projectSlug(projectRoot), 'state.json'); + const read = (file: string) => fs.readFileSync(file, 'utf8'); + + beforeEach(() => { + if (!fs.existsSync(CLI)) { + throw new Error(`CLI binary not found at ${CLI}. Run "npm run build" first.`); + } + + sandbox = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-issue822-e2e-'))); + home = path.join(sandbox, 'home'); + projectRoot = path.join(sandbox, 'project'); + teammate = path.join(sandbox, 'teammate'); + const seed = path.join(sandbox, 'seed'); + const remote = path.join(sandbox, 'team-remote.git'); + const teamRepo = path.join(projectRoot, '.teamai', 'team-repo'); + + fs.mkdirSync(home, { recursive: true }); + fs.mkdirSync(path.join(seed, 'skills', 'team-skill', 'scripts'), { recursive: true }); + fs.mkdirSync(path.join(seed, 'rules'), { recursive: true }); + fs.mkdirSync(path.join(seed, 'agents'), { recursive: true }); + fs.writeFileSync(path.join(seed, 'teamai.yaml'), 'team: issue-822-e2e\nrepo: https://example.com/team.git\nprovider: tgit\n'); + fs.writeFileSync(path.join(seed, 'skills', 'team-skill', 'SKILL.md'), SKILL_MD); + fs.writeFileSync(path.join(seed, 'skills', 'team-skill', 'scripts', 'run.sh'), 'echo one\n'); + fs.writeFileSync(path.join(seed, 'rules', 'team-rule.md'), '# Team rule\n\nVersion one.\n'); + fs.writeFileSync(path.join(seed, 'agents', 'team-helper.yaml'), agentYaml('Version one.')); + git(['init', '-q', '-b', 'main'], seed); + git(['add', '-A'], seed); + git(['commit', '-q', '-m', 'seed'], seed); + git(['clone', '-q', '--bare', seed, remote], sandbox); + git(['clone', '-q', remote, teammate], sandbox); + + fs.mkdirSync(path.join(projectRoot, '.claude'), { recursive: true }); + fs.mkdirSync(path.join(projectRoot, '.cursor'), { recursive: true }); + fs.writeFileSync(path.join(projectRoot, '.claude', 'settings.json'), '{}\n'); + fs.writeFileSync(path.join(projectRoot, '.gitignore'), '.teamai/\n.cursor/\n.claude/\n'); + git(['init', '-q', '-b', 'main'], projectRoot); + git(['add', '-A'], projectRoot); + git(['commit', '-q', '-m', 'project'], projectRoot); + git(['clone', '-q', remote, teamRepo], sandbox); + fs.writeFileSync(path.join(projectRoot, '.teamai', 'config.yaml'), [ + 'repo:', + ` localPath: ${teamRepo}`, + ` remote: ${remote}`, + 'username: ci-822', + 'updatePolicy: auto', + 'scope: project', + `projectRoot: ${projectRoot}`, + 'enabledAgents: [claude, cursor]', + '', + ].join('\n')); + }); + + afterEach(() => { + if (sandbox) fs.rmSync(sandbox, { recursive: true, force: true }); + }); + + const pull = async (...flags: string[]): Promise => { + const r = await runCLI(['pull', ...flags], projectRoot, home); + expect(r.code, r.output).toBe(0); + return r.output; + }; + + /** A teammate's commit to the team repo, pushed to its remote. */ + const teamCommit = (change: (repo: string) => void): void => { + git(['pull', '-q'], teammate); + change(teammate); + git(['add', '-A'], teammate); + git(['commit', '-q', '-m', 'teammate'], teammate); + git(['push', '-q', 'origin', 'main'], teammate); + }; + const teamUpdatesAll = (repo: string): void => { + fs.writeFileSync(path.join(repo, 'skills', 'team-skill', 'scripts', 'run.sh'), 'echo two\n'); + fs.writeFileSync(path.join(repo, 'rules', 'team-rule.md'), '# Team rule\n\nVersion two.\n'); + fs.writeFileSync(path.join(repo, 'agents', 'team-helper.yaml'), agentYaml('Version two.')); + }; + const editClaudeCopies = (): void => { + fs.writeFileSync(claudeScript(), 'echo mine\n'); + fs.writeFileSync(claudeRule(), '# Team rule\n\nMy version.\n'); + fs.appendFileSync(claudeAgent(), '\nMy extra instruction.\n'); + }; + + it('updates every copy nobody edited when the team changes it', async () => { + await pull(); + teamCommit(teamUpdatesAll); + + const output = await pull(); + + expect(output).not.toContain('Kept'); + expect(read(claudeScript())).toBe('echo two\n'); + expect(read(claudeRule())).toContain('Version two.'); + expect(read(cursorRule())).toContain('Version two.'); + expect(read(claudeAgent())).toContain('Version two.'); + expect(read(cursorAgent())).toContain('Version two.'); + }); + + it('keeps an edited copy on a forced full sync of an unchanged team repo, and restores one the member deleted', async () => { + await pull(); + editClaudeCopies(); + const agentEdit = read(claudeAgent()); + + const output = await pull('--force'); + + expect(read(claudeScript())).toBe('echo mine\n'); + expect(read(claudeRule())).toContain('My version.'); + expect(read(claudeAgent())).toBe(agentEdit); + for (const kept of [claudeSkill(), claudeRule(), claudeAgent()]) { + expect(output).toContain(`Kept ${kept}: you changed it since teamai delivered it. Share it with \`teamai push\``); + } + expect(output).not.toContain('has changed since'); + + fs.rmSync(claudeSkill(), { recursive: true }); + fs.rmSync(claudeRule()); + const restored = await pull('--force'); + expect(read(claudeScript())).toBe('echo one\n'); + expect(read(claudeRule())).toContain('Version one.'); + expect(restored).not.toContain(`Kept ${claudeSkill()}`); + expect(restored).not.toContain(`Kept ${claudeRule()}`); + expect(restored).toContain(`Kept ${claudeAgent()}`); + }); + + it('keeps an edited copy the team changed since and warns, while the other tools\' copies update', async () => { + await pull(); + editClaudeCopies(); + const agentEdit = read(claudeAgent()); + teamCommit(teamUpdatesAll); + + const output = await pull(); + + expect(read(claudeScript())).toBe('echo mine\n'); + expect(read(claudeRule())).toContain('My version.'); + expect(read(claudeAgent())).toBe(agentEdit); + expect(output).toContain(`Kept ${claudeSkill()}: you changed it, and the team version (skills/team-skill) has changed since.`); + expect(output).toContain(`Kept ${claudeRule()}: you changed it, and the team version (rules/team-rule.md) has changed since.`); + expect(output).toContain(`Kept ${claudeAgent()}: you changed it, and the team version (agents/team-helper.yaml) has changed since.`); + // Per tool: the Cursor render of the same rule and agent is not the member's, so it updates. + expect(read(cursorRule())).toContain('Version two.'); + expect(read(cursorAgent())).toContain('Version two.'); + expect(read(cursorScript())).toBe('echo two\n'); + }); + + const pushDryRun = async (): Promise => { + const r = await runCLI(['push', '--dry-run'], projectRoot, home); + expect(r.code, r.output).toBe(0); + return r.output; + }; + const teamChangedWarning = (relPath: string, dest: string): string => ( + `The team changed ${relPath} since teamai delivered ${dest}; pushing replaces that change unless you merged it.` + ); + + it('warns at push about a kept copy the team changed since, after a silent pull kept it', async () => { + await pull(); + editClaudeCopies(); + teamCommit(teamUpdatesAll); + // The SessionStart pull: it keeps the copies and prints nothing. + await pull('--silent'); + + const output = await pushDryRun(); + + expect(output).toContain(teamChangedWarning('skills/team-skill', claudeSkill())); + expect(output).toContain(teamChangedWarning('rules/team-rule.md', claudeRule())); + expect(output).toContain(teamChangedWarning('agents/team-helper.yaml', claudeAgent())); + expect(output).not.toContain(`delivered ${cursorRule()}`); + }); + + it('warns at push once the team amends a pushed copy, and not while the team is unchanged', async () => { + await pull(); + fs.writeFileSync(claudeRule(), '# Team rule\n\nMy version.\n'); + expect(await pushDryRun()).not.toContain('The team changed'); + + // The review amends the member's change before it merges. + teamCommit((repo) => fs.writeFileSync(path.join(repo, 'rules', 'team-rule.md'), '# Team rule\n\nMy version, amended.\n')); + await pull(); + + expect(await pushDryRun()).toContain(teamChangedWarning('rules/team-rule.md', claudeRule())); + }); + + it('names the copies --dry-run would keep and writes nothing', async () => { + await pull(); + editClaudeCopies(); + teamCommit(teamUpdatesAll); + const stateBefore = read(statePath()); + const cursorRuleBefore = read(cursorRule()); + + const output = await pull('--dry-run'); + + for (const kept of [claudeSkill(), claudeRule(), claudeAgent()]) { + expect(output).toContain(`[dry-run] Would keep ${kept}: you changed it since teamai delivered it.`); + } + expect(output).not.toContain(`Would keep ${cursorRule()}`); + expect(read(statePath())).toBe(stateBefore); + expect(read(cursorRule())).toBe(cursorRuleBefore); + expect(read(claudeScript())).toBe('echo mine\n'); + }); + + it('keeps an edited copy of a resource the team removed, and removes the untouched copies', async () => { + await pull(); + fs.writeFileSync(claudeScript(), 'echo mine\n'); + fs.writeFileSync(claudeRule(), '# Team rule\n\nMy version.\n'); + fs.appendFileSync(claudeAgent(), '\nMy extra instruction.\n'); + teamCommit((repo) => { + fs.rmSync(path.join(repo, 'skills', 'team-skill'), { recursive: true }); + fs.rmSync(path.join(repo, 'rules', 'team-rule.md')); + fs.rmSync(path.join(repo, 'agents', 'team-helper.yaml')); + fs.writeFileSync(path.join(repo, 'skills', '.removed'), 'team-skill\n'); + fs.writeFileSync(path.join(repo, 'rules', '.removed'), 'team-rule\n'); + fs.writeFileSync(path.join(repo, 'agents', '.removed'), 'team-helper\n'); + }); + + const output = await pull(); + + expect(read(claudeScript())).toBe('echo mine\n'); + expect(read(claudeRule())).toContain('My version.'); + expect(output).toContain(`Kept ${claudeSkill()}: the team removed team-skill, but you changed this copy.`); + expect(output).toContain(`Kept ${claudeRule()}: the team removed team-rule, but you changed this copy.`); + expect(output).toContain(`Kept ${claudeAgent()}: the team removed team-helper, but you changed this copy.`); + expect(read(claudeAgent())).toContain('My extra instruction.'); + expect(fs.existsSync(cursorScript())).toBe(false); + expect(fs.existsSync(cursorRule())).toBe(false); + expect(fs.existsSync(cursorAgent())).toBe(false); + }); + + it('keeps an edited copy of a rule the team deleted without a tombstone, and removes the untouched one', async () => { + await pull(); + fs.writeFileSync(claudeRule(), '# Team rule\n\nMy version.\n'); + teamCommit((repo) => { + fs.rmSync(path.join(repo, 'rules', 'team-rule.md')); + fs.writeFileSync(path.join(repo, 'rules', 'other-rule.md'), '# Other rule\n'); + }); + + const output = await pull(); + + expect(read(claudeRule())).toContain('My version.'); + expect(output).toContain(`Kept ${claudeRule()}: teamai no longer delivers team-rule here, but you changed this copy.`); + expect(fs.existsSync(cursorRule())).toBe(false); + }); + + it('keeps another worktree\'s record of what it delivered through a forced full sync', async () => { + await pull(); + const worktree = path.join(sandbox, 'wt'); + git(['worktree', 'add', '-q', worktree, '-b', 'wt'], projectRoot); + fs.mkdirSync(path.join(worktree, '.claude'), { recursive: true }); + const inWorktree = async (): Promise => { + const r = await runCLI(['pull'], worktree, home); + expect(r.code, r.output).toBe(0); + return r.output; + }; + await inWorktree(); + const worktreeRule = path.join(worktree, '.claude', 'rules', 'team-rule.md'); + fs.writeFileSync(worktreeRule, '# Team rule\n\nMy version.\n'); + // What `roles set`, `projects set` and `skill exclude` do: clear the shared + // revision, so the next pull resets every other checkout's record. + const state = JSON.parse(read(statePath())) as { lastPullRev: string | null }; + state.lastPullRev = null; + fs.writeFileSync(statePath(), JSON.stringify(state, null, 2)); + await pull(); + teamCommit(teamUpdatesAll); + + const output = await inWorktree(); + + expect(read(worktreeRule)).toContain('My version.'); + expect(output).toContain(`Kept ${worktreeRule}: you changed it, and the team version (rules/team-rule.md) has changed since.`); + }); + + it('overwrites an edited copy teamai has no record of, as before, and protects it from then on', async () => { + await pull(); + // The state an older CLI leaves: a checkout record without `delivered`. + const state = JSON.parse(read(statePath())) as { lastPullByWorkspace: Record }; + for (const record of Object.values(state.lastPullByWorkspace)) delete record.delivered; + fs.writeFileSync(statePath(), JSON.stringify(state, null, 2)); + fs.writeFileSync(claudeRule(), '# Team rule\n\nMy version.\n'); + teamCommit(teamUpdatesAll); + + const upgraded = await pull(); + + expect(upgraded).not.toContain('Kept'); + expect(read(claudeRule())).toContain('Version two.'); + + fs.writeFileSync(claudeRule(), '# Team rule\n\nMy version again.\n'); + teamCommit((repo) => fs.writeFileSync(path.join(repo, 'rules', 'team-rule.md'), '# Team rule\n\nVersion three.\n')); + const protectedPull = await pull(); + expect(read(claudeRule())).toContain('My version again.'); + expect(protectedPull).toContain(`Kept ${claudeRule()}: you changed it, and the team version (rules/team-rule.md) has changed since.`); + }); +}); diff --git a/src/__tests__/pre-push-sync.test.ts b/src/__tests__/pre-push-sync.test.ts index 255411911..42dff39cc 100644 --- a/src/__tests__/pre-push-sync.test.ts +++ b/src/__tests__/pre-push-sync.test.ts @@ -28,6 +28,7 @@ vi.mock('../utils/git.js', () => ({ })); import { syncTeamUpdatesToLocal } from '../utils/pre-push-sync.js'; +import { fileHash } from '../utils/fs.js'; import { teamRuleToCopilotInstructions } from '../resources/copilot-instructions.js'; import type { TeamaiConfig, LocalConfig } from '../types.js'; @@ -101,6 +102,21 @@ describe('syncTeamUpdatesToLocal — rules', () => { expect(content).toBe('v2 content'); }); + it('records the rule it syncs as delivered, and leaves an edited one on record (#822)', async () => { + await fse.writeFile(path.join(repoPath, 'rules', 'synced.md'), 'v2 content'); + await fse.writeFile(path.join(repoPath, 'rules', 'edited.md'), 'v2 content'); + const synced = path.join(homeDir, '.claude/rules', 'synced.md'); + const edited = path.join(homeDir, '.claude/rules', 'edited.md'); + await fse.writeFile(synced, 'v1 content'); + await fse.writeFile(edited, 'local edit'); + mockGetFileContentAtRev.mockResolvedValue(Buffer.from('v1 content')); + const delivered: Record = { [synced]: 'hash-of-v1', [edited]: 'hash-of-v1' }; + + await syncTeamUpdatesToLocal(teamConfig, localConfig, 'abc1234', undefined, delivered); + + expect(delivered).toEqual({ [synced]: await fileHash(synced), [edited]: 'hash-of-v1' }); + }); + it('syncs a local rule at any of several bases, and keeps one at none (#812)', async () => { await fse.writeFile(path.join(repoPath, 'rules', 'at-older.md'), 'v3 content'); await fse.writeFile(path.join(repoPath, 'rules', 'edited.md'), 'v3 content'); @@ -480,6 +496,17 @@ describe('syncTeamUpdatesToLocal — rules', () => { expect(mockGetFileContentAtRev).toHaveBeenCalledWith(repoPath, 'abc1234', './rules/my-rule.md'); }); + it('records the Copilot rule it syncs as delivered (#822)', async () => { + const localFile = path.join(instructionsDir, 'my-rule.instructions.md'); + await fse.outputFile(localFile, teamRuleToCopilotInstructions(oldRule)); + await fse.writeFile(path.join(repoPath, 'rules/my-rule.md'), newRule); + const delivered: Record = { [localFile]: 'hash-of-v1' }; + + await syncTeamUpdatesToLocal(teamConfig, localConfig, 'abc1234', undefined, delivered); + + expect(delivered).toEqual({ [localFile]: await fileHash(localFile) }); + }); + it('refreshes applyTo when only the team paths change', async () => { const localFile = path.join(instructionsDir, 'my-rule.instructions.md'); const pathsOnlyUpdate = oldRule.replace('src/**/*.ts', 'lib/**/*.ts'); @@ -629,6 +656,21 @@ describe('syncTeamUpdatesToLocal — skills', () => { expect(content).toBe('v2 skill'); }); + it('records the team files of a skill it syncs as delivered, not the member\'s own (#822)', async () => { + const teamSkillDir = path.join(repoPath, 'skills', 'my-skill'); + await fse.outputFile(path.join(teamSkillDir, 'SKILL.md'), 'v2 skill'); + await fse.outputFile(path.join(teamSkillDir, 'CONTRIBUTORS'), 'alice\n'); + const localSkillDir = path.join(homeDir, '.claude/skills', 'my-skill'); + await fse.outputFile(path.join(localSkillDir, 'SKILL.md'), 'v1 skill'); + await fse.outputFile(path.join(localSkillDir, 'notes.md'), 'mine'); + mockGetFileContentAtRev.mockResolvedValue(Buffer.from('v1 skill')); + const delivered: Record = { [path.join(localSkillDir, 'SKILL.md')]: 'hash-of-v1' }; + + await syncTeamUpdatesToLocal(teamConfig, localConfig, 'abc1234', undefined, delivered); + + expect(delivered).toEqual({ [path.join(localSkillDir, 'SKILL.md')]: await fileHash(path.join(localSkillDir, 'SKILL.md')) }); + }); + it('should NOT sync skill dir when user edited any file', async () => { // Team repo: skill with SKILL.md v2 const teamSkillDir = path.join(repoPath, 'skills', 'my-skill'); diff --git a/src/__tests__/pull-namespace-override.test.ts b/src/__tests__/pull-namespace-override.test.ts index d679ca78f..6080dec68 100644 --- a/src/__tests__/pull-namespace-override.test.ts +++ b/src/__tests__/pull-namespace-override.test.ts @@ -54,10 +54,11 @@ vi.mock('../update.js', () => ({ releaseLock: vi.fn().mockResolvedValue(undefined), })); -import { pull } from '../pull.js'; +import crypto from 'node:crypto'; +import { checkoutKey, pull } from '../pull.js'; import { loadLocalConfigForScope, loadTeamConfig, detectProjectConfig, loadStateForScope } from '../config.js'; import { log } from '../utils/logger.js'; -import type { TeamaiConfig, LocalConfig } from '../types.js'; +import { StateSchema, type TeamaiConfig, type LocalConfig } from '../types.js'; const ROLES_YAML = ` version: 1 @@ -425,6 +426,41 @@ describe('pull: an active namespace item replaces the root item of the same name )); }); + describe('with a record of what teamai delivered (#822)', () => { + const frontOnly = (): string => path.join(homeDir, '.claude/skills/review/front-only.md'); + const recordDelivered = async (delivered: Record): Promise => { + vi.mocked(loadStateForScope).mockResolvedValue(StateSchema.parse({ + lastPullByWorkspace: { [await checkoutKey(homeDir)]: { rev: '', targets: [], delivered } }, + })); + }; + afterEach(() => { + vi.mocked(loadStateForScope).mockResolvedValue(StateSchema.parse({ lastPull: null })); + }); + + it('keeps a file the member added at a path another version has without naming it', async () => { + await recordDelivered({}); + as(['devops'], { subscribedTags: ['ui'] }); + await pull({}); + await fse.outputFile(frontOnly(), 'my own notes\n'); + + await pull({ force: true }); + + expect(await read('.claude/skills/review/front-only.md')).toBe('my own notes\n'); + expect(logged('warn', /front-only\.md/)).toBe(false); + }); + + it('removes a leftover teamai wrote there at an earlier delivery', async () => { + await fse.outputFile(frontOnly(), 'older front notes\n'); + await recordDelivered({ [frontOnly()]: crypto.createHash('sha256').update('older front notes\n').digest('hex') }); + as(['devops'], { subscribedTags: ['ui'] }); + + await pull({}); + + expect(await exists('.claude/skills/review/front-only.md')).toBe(false); + expect(logged('warn', /front-only\.md/)).toBe(false); + }); + }); + // A directory without SKILL.md is not a skill: it must neither replace the // root skill nor strip the installed one of its SKILL.md. it('keeps delivering the root skill while the namespace directory of its name has no SKILL.md', async () => { diff --git a/src/__tests__/pull-sync-truth.test.ts b/src/__tests__/pull-sync-truth.test.ts index 651829443..e4d119159 100644 --- a/src/__tests__/pull-sync-truth.test.ts +++ b/src/__tests__/pull-sync-truth.test.ts @@ -224,7 +224,8 @@ describe('pull reports what reached the tool directory (#585)', () => { expect(vi.mocked(log.warn).mock.calls.flat()).toContainEqual(expect.stringContaining('[project] Failed to sync docs:')); // The marker stays cleared for a retry, and the record keeps its rev. expect(state.lastPullRev).toBeNull(); - expect(state.lastPullByWorkspace?.[key]).toEqual({ rev: 'old1234', targets: [], pushBaseRevs: ['abc1234'] }); + // It also records what the pull delivered, docs failure or not (#822). + expect(state.lastPullByWorkspace?.[key]).toEqual({ rev: 'old1234', targets: [], pushBaseRevs: ['abc1234'], delivered: {} }); }); it.each(['empty', 'missing'])('prunes only stale empty directories when the team bundle is %s', async (state) => { diff --git a/src/__tests__/rules.test.ts b/src/__tests__/rules.test.ts index d27e15f91..bf671a388 100644 --- a/src/__tests__/rules.test.ts +++ b/src/__tests__/rules.test.ts @@ -37,6 +37,7 @@ vi.mock('../utils/logger.js', () => ({ import { RulesHandler } from '../resources/rules.js'; import { loadStateForScope } from '../config.js'; +import { openLedger, recordDelivered, type DeliveredHashes } from '../resources/delivered-copies.js'; import type { TeamaiConfig, LocalConfig, State } from '../types.js'; describe('RulesHandler.scanLocalForPush — modified rule detection', () => { @@ -642,6 +643,28 @@ scope: 'user', expect(await fse.readFile(personal, 'utf-8')).toBe('# Mine\n'); }); + it('drops a reclaimed copy from the delivered record and keeps the record of an edited one (#822)', async () => { + const teamRulesDir = path.join(localConfig.repo.localPath, 'rules'); + await fse.ensureDir(path.join(teamRulesDir, 'alpha')); + await fse.writeFile(path.join(teamRulesDir, 'alpha/alpha-rule.md'), '# Alpha rule\n'); + await fse.writeFile(path.join(teamRulesDir, 'alpha/edited.md'), '# Team version\n'); + const localRulesDir = path.join(homeDir, '.claude/rules'); + const reclaimed = path.join(localRulesDir, 'alpha/alpha-rule.md'); + const edited = path.join(localRulesDir, 'alpha/edited.md'); + await fse.outputFile(reclaimed, '# Alpha rule\n'); + await fse.outputFile(edited, '# Team version\n'); + const previous: DeliveredHashes = {}; + await recordDelivered(previous, reclaimed); + await recordDelivered(previous, edited); + await fse.writeFile(edited, '# Edited locally\n'); + const ledger = openLedger(previous); + + await handler.pullAllRules(teamConfig, localConfig, [], [], ledger); + + expect(await fse.pathExists(reclaimed)).toBe(false); + expect(Object.keys(ledger.hashes)).toEqual([edited]); + }); + it('removes a namespace directory it empties', async () => { const teamRulesDir = path.join(localConfig.repo.localPath, 'rules'); await fse.ensureDir(path.join(teamRulesDir, 'alpha')); diff --git a/src/doctor-delivery.ts b/src/doctor-delivery.ts index e0d7b5a83..844e04233 100644 --- a/src/doctor-delivery.ts +++ b/src/doctor-delivery.ts @@ -124,6 +124,38 @@ function describeProblems(problems: Map, labels: readonly stri .join('; '); } +/** + * The label for a copy pull keeps because the member changed it (#822). It is + * not a delivery problem, so it never fails a check, and `pull --force` would + * not replace it. + */ +const CHANGED_BY_YOU = 'changed by you (kept by pull)'; +/** The advice for CHANGED_BY_YOU, when a failing check lists it. */ +function changedByYouFix(delivery: ToolDelivery): string { + return delivery.problems.has(CHANGED_BY_YOU) + ? ' A copy changed by you is kept by pull: share it with `teamai push`, ' + + 'or delete it and run `teamai pull --force` to take the team version.' + : ''; +} + +/** Whether a tool's delivery has a problem other than copies the member changed. */ +function hasDeliveryProblem(delivery: ToolDelivery): boolean { + return [...delivery.problems.keys()].some((label) => label !== CHANGED_BY_YOU); +} + +/** + * What `pullItem` did not write at `target`: an older render, or a copy the + * member changed since teamai delivered it, which pull keeps. + */ +async function differingCopyLabel( + item: ResourceItem, target: DeliveryTarget, olderLabel: string, localConfig: LocalConfig, +): Promise { + const { deliveredHashes } = await import('./pull.js'); + const { judgeCopy } = await import('./resources/delivered-copies.js'); + const verdict = await judgeCopy(await deliveredHashes(localConfig), item, target); + return verdict.kind === 'keep' ? CHANGED_BY_YOU : olderLabel; +} + /** `a, b, c and 4 more` — a fix a human reads, not a wall of paths. */ function nameList(names: string[]): string { if (names.length <= MAX_NAMED_IN_FIX) return names.join(', '); @@ -220,22 +252,23 @@ export async function buildRulesDeliveryChecks(ctx: DoctorContext): Promise { + async (target, item) => { // readFileSafe answers both questions at once: a directory or a dangling // link on the name reads as null, the same as nothing being there. - const delivered = await readFileSafe(dest); + const delivered = await readFileSafe(target.dest); if (delivered === null) return ruleLabels[0]; - return content === undefined || delivered === content ? null : ruleLabels[1]; + if (target.content === undefined || delivered === target.content) return null; + return differingCopyLabel(item, target, ruleLabels[1], localConfig); }, )).byTool].map(([tool, delivery]) => ({ name: `Rules delivered to ${tool}`, source: 'local', - check: async () => delivery.problems.size === 0, + check: async () => !hasDeliveryProblem(delivery), // The fix names the directory rather than the tool: a rule's delivered // filename carries a per-tool extension the reader would have to derive. fix: `In ${delivery.dir}, ${describeProblems(delivery.problems, ruleLabels)}. ` @@ -243,7 +276,7 @@ export async function buildRulesDeliveryChecks(ctx: DoctorContext): Promise { + async (target, item) => { // readFileSafe answers both questions at once: a directory or a dangling // link on the name reads as null, the same as nothing being there. - const delivered = await readFileSafe(dest); + const delivered = await readFileSafe(target.dest); if (delivered === null) return agentLabels[0]; - return content === undefined || delivered === content ? null : agentLabels[1]; + if (target.content === undefined || delivered === target.content) return null; + return differingCopyLabel(item, target, agentLabels[1], localConfig); }, ); const checks: Check[] = [...byTool].map(([tool, delivery]) => ({ name: `Agents delivered to ${tool}`, source: 'local', - check: async () => delivery.problems.size === 0, + check: async () => !hasDeliveryProblem(delivery), fix: `In ${delivery.dir}, ${describeProblems(delivery.problems, agentLabels)}. ` + 'Run `teamai pull --force`: a plain pull skips a scope whose team repo has not changed, ' - + 'so it cannot restore this.', + + `so it cannot restore this.${changedByYouFix(delivery)}`, })); // Only worth reporting once a tool is there to receive agents: with none diff --git a/src/pull.ts b/src/pull.ts index 804f7ec2c..6604a59f4 100644 --- a/src/pull.ts +++ b/src/pull.ts @@ -21,6 +21,9 @@ import { isToolInstalledForConfig, ResourceHandler } from './resources/base.js'; import { skillsDirForTool } from './resources/skills.js'; import { ruleFileExtensionForTool } from './resources/rule-format.js'; import { AGENT_FILE_EXTENSIONS } from './resources/agent-format.js'; +import { + forgetDelivered, judgeCopy, openLedger, removedCopyChanged, reportKept, type DeliveredHashes, type DeliveryLedger, +} from './resources/delivered-copies.js'; import { BUILTIN_SKILL_NAMES } from './builtin-skills.js'; import type { GlobalOptions, ResourceType, ResourceItem, TeamaiConfig, LocalConfig, State } from './types.js'; import { @@ -305,6 +308,24 @@ async function getExistingLocalNames( return existing; } +/** `--dry-run`: name each copy the sync would keep because the member changed it (#822). */ +async function reportWouldKeep( + handler: ResourceHandler, + items: readonly ResourceItem[], + freshConfig: TeamaiConfig, + localConfig: LocalConfig, + ledger: DeliveryLedger, + scopeLabel: string, +): Promise { + for (const item of items) { + for (const target of await handler.deliveryTargets(freshConfig, localConfig, item)) { + if ((await judgeCopy(ledger.previous, item, target)).kind === 'keep') { + log.info(`[${scopeLabel}] [dry-run] Would keep ${target.dest}: you changed it since teamai delivered it.`); + } + } + } +} + /** * Format pull detail output showing new vs updated items. */ @@ -399,11 +420,13 @@ function tombstoneExtensions(type: ResourceType, tool: string): readonly string[ * Called from the full sync and from the "already synced" fast path: a CLI * upgrade that widens the extensions above must still reach a machine whose * team repo HEAD has not moved since it pulled the tombstone (issue #576). + * A copy the member changed since teamai delivered it is kept (#822). */ async function cleanupTombstonedResources( freshConfig: TeamaiConfig, localConfig: LocalConfig, scopeLabel: string, + ledger: DeliveryLedger, ): Promise { // Each entry maps a resource type to the field on toolPath that names the // tool-side directory; `tombstoneExtensions` supplies the filename suffixes. @@ -440,7 +463,12 @@ async function cleanupTombstonedResources( log.warn(`[${scopeLabel}] Kept tombstoned skill "${name}" (${tool}): it has local VCS metadata (.git) that may hold unpushed history. Back it up, then delete it manually.`); continue; } + if (await removedCopyChanged(ledger.previous, localPath)) { + log.warn(`[${scopeLabel}] Kept ${localPath}: the team removed ${name}, but you changed this copy. Delete it when you no longer need it.`); + continue; + } await remove(localPath); + forgetDelivered(ledger.hashes, localPath); log.debug(`[${scopeLabel}] Cleaned up tombstoned ${type} ${name} from ${dir}`); } } @@ -661,11 +689,30 @@ export async function userScopeRecord(state: State): Promise { return record; } -/** `records` after a forced full sync: see FORCED_FULL_SYNC_REV. */ +/** + * What teamai last wrote at each skill, rule and agent file of the checkout + * `localConfig`'s pulls deliver into, or undefined when nothing is recorded + * yet (#822). + */ +export async function deliveredHashes(localConfig: LocalConfig, state?: State): Promise { + const key = await checkoutRecordKey(localConfig); + if (!key) return undefined; + return (state ?? await loadStateForScope(localConfig)).lastPullByWorkspace?.[key]?.delivered; +} + +/** + * `records` after a forced full sync: see FORCED_FULL_SYNC_REV. Each keeps + * what teamai delivered into its checkout, or that checkout's next pull would + * overwrite the copies its member changed. + */ function awaitingFullSync(records: Record): Record { return Object.fromEntries(Object.entries(records).map(([key, record]) => { const pushBaseRevs = checkoutBaseRevs(record).slice(0, MAX_PUSH_BASE_REVS); - const reset: CheckoutRecord = { rev: FORCED_FULL_SYNC_REV, targets: record.targets }; + const reset: CheckoutRecord = { + rev: FORCED_FULL_SYNC_REV, + targets: record.targets, + ...(record.delivered ? { delivered: record.delivered } : {}), + }; return [key, pushBaseRevs.length === 0 ? reset : { ...reset, pushBaseRevs }]; })); } @@ -1019,7 +1066,7 @@ async function pullForScope( // Same reason: a machine that already pulled a tombstone with an older // CLI keeps the copies that CLI failed to delete, and its stored rev // never moves again. Re-run the cleanup so the upgrade reaches it (#576). - await cleanupTombstonedResources(freshConfig, localConfig, scopeLabel); + await cleanupTombstonedResources(freshConfig, localConfig, scopeLabel, openLedger(await deliveredHashes(localConfig, state))); // A repo that has not moved can still carry a malformed env.yaml, or // scope a variable this CLI version now withholds; the Step 2 env // branch below is unreachable from here. @@ -1049,6 +1096,10 @@ async function pullForScope( const excludedSkills = new Set(localConfig.excludedSkills ?? []); + // What teamai last wrote into this checkout: a copy changed since is kept, + // and this pull's writes are recorded when the state is saved (#822). + const ledger = openLedger(await deliveredHashes(localConfig)); + // Step 2: Sync each resource type let totalSynced = 0; let docsSyncFailed = false; @@ -1072,15 +1123,17 @@ async function pullForScope( if (items.length > 0) { log.info(`[${scopeLabel}] [dry-run] Would sync ${items.length} rule(s)${skippedByTags > 0 ? ` (skipped ${skippedByTags} by tags)` : ''}`); } + await reportWouldKeep(rulesHandler, items, freshConfig, localConfig, ledger, scopeLabel); } else { // Always call pullAllRules, even with an empty set: it also cleans up // stale local rule files and deactivates the OpenCode instructions glob // when the team's last rule is removed. Guarding on items.length > 0 // would leak those artifacts on the machine after upstream deletion. - await rulesHandler.pullAllRules(freshConfig, localConfig, items, replaced); + await rulesHandler.pullAllRules(freshConfig, localConfig, items, replaced, ledger); if (items.length > 0) { log.success(`[${scopeLabel}] Synced ${items.length} rule(s)${skippedByTags > 0 ? ` (skipped ${skippedByTags} by tags)` : ''}`); } + reportKept(ledger, scopeLabel); } totalSynced += items.length; continue; @@ -1186,6 +1239,7 @@ async function pullForScope( log.dim(` ${item.name}`); } } + await reportWouldKeep(handler, items, freshConfig, localConfig, ledger, scopeLabel); } else { // Skills and agents land in a tool's own directory, which a brand-new // member may not have yet. The handler skips such a tool by design and @@ -1199,7 +1253,7 @@ async function pullForScope( || (await getInstalledResourceTargets(freshConfig, localConfig, type)).length > 0; for (const item of items) { - await handler.pullItem(item, freshConfig, localConfig); + await handler.pullItem(item, freshConfig, localConfig, ledger); } if (canReceive) { @@ -1209,6 +1263,7 @@ async function pullForScope( log.success(`[${scopeLabel}] Synced ${items.length} ${type}`); } } + reportKept(ledger, scopeLabel); } totalSynced += items.length; @@ -1216,7 +1271,7 @@ async function pullForScope( // Step 3: Clean up tombstoned resources if (!options.dryRun) { - await cleanupTombstonedResources(freshConfig, localConfig, scopeLabel); + await cleanupTombstonedResources(freshConfig, localConfig, scopeLabel, ledger); if (roleContext) { if (!skillsHeld) { @@ -1391,6 +1446,7 @@ async function pullForScope( ? await userScopeRecord(state) : state.lastPullByWorkspace?.[recordKey] ?? { rev: FORCED_FULL_SYNC_REV, targets: syncedTargets }; addPushBaseRev(record, deliveredRev); + record.delivered = ledger.hashes; state.lastPullByWorkspace = { ...state.lastPullByWorkspace, [recordKey]: record }; } else if (recordKey && deliveredRev) { // A forced full sync (lastPullRev cleared) leaves every other checkout @@ -1404,7 +1460,7 @@ async function pullForScope( ? await liveCheckoutRecords(localConfig.projectRoot, state.lastPullByWorkspace) : undefined; const others = previousRev === null && live ? awaitingFullSync(live) : live; - const record: CheckoutRecord = { rev: deliveredRev, targets: syncedTargets }; + const record: CheckoutRecord = { rev: deliveredRev, targets: syncedTargets, delivered: ledger.hashes }; state.lastPullByWorkspace = { ...others, [recordKey]: keptBases.length > 0 ? { ...record, pushBaseRevs: keptBases } : record, diff --git a/src/push.ts b/src/push.ts index 25772efa8..f3e0cf48e 100644 --- a/src/push.ts +++ b/src/push.ts @@ -228,6 +228,47 @@ async function resolveNamespaceForNew( return { kind: 'namespace', namespace: candidates[selection - 1] }; } +/** + * Warn about each copy of a modified item that pull kept because the member + * changed it, when the team version has moved on since teamai delivered it: + * pushing it as it is would replace that change (#822). The pull that kept it + * may have been the silent SessionStart one, so push is where the member hears + * of it. A warning, not a hold: the member may have merged the change already. + */ +async function warnKeptCopiesTheTeamChanged( + items: readonly ResourceItem[], + teamConfig: TeamaiConfig, + localConfig: LocalConfig, +): Promise { + const { deliveredHashes } = await import('./pull.js'); + const previous = await deliveredHashes(localConfig); + if (previous === undefined) return; + const { judgeCopy } = await import('./resources/delivered-copies.js'); + for (const item of items) { + if (item.status !== 'modified' || !(item.type === 'skills' || item.type === 'rules' || item.type === 'agents')) continue; + const teamItem: ResourceItem = { + name: item.name, + type: item.type, + sourcePath: path.join(localConfig.repo.localPath, item.relativePath), + relativePath: item.relativePath, + }; + try { + for (const target of await getHandler(item.type).deliveryTargets(teamConfig, localConfig, teamItem)) { + const verdict = await judgeCopy(previous, teamItem, target); + if (verdict.kind === 'keep' && verdict.teamChanged) { + log.warn( + `[${item.type}] The team changed ${item.relativePath} since teamai delivered ${target.dest}; ` + + 'pushing replaces that change unless you merged it. Merge the team version first, ' + + 'or delete your copy and run `teamai pull --force`.', + ); + } + } + } catch (e) { + log.debug(`Could not compare ${item.relativePath} with what teamai delivered: ${e instanceof Error ? e.message : String(e)}`); + } + } +} + /** * Create a PR/MR via the configured provider with standard error handling. * Returns the PR URL on success, or null if creation failed (branch is still pushed). @@ -1119,15 +1160,19 @@ async function pushCore( // Compare with the revisions THIS checkout synced: state.json is shared by // every worktree, and a pull in another checkout moves the shared // lastPullRev past a copy this checkout still holds unedited (#812). - const { resolveCheckoutBases, addPushBaseRev, userScopeRecord } = await import('./pull.js'); + const { resolveCheckoutBases, addPushBaseRev, userScopeRecord, deliveredHashes } = await import('./pull.js'); const bases = await resolveCheckoutBases(localConfig, state); + // What the sync writes is recorded as delivered, like a pull's writes, or + // the next pull after a further team change keeps those copies (#822). + const delivered = await deliveredHashes(localConfig, state); + const deliveredBefore = JSON.stringify(delivered); unrecordedCheckout = bases.source === 'shared' && bases.unrecorded; placedRules = state.placedRules; try { // placedRules redirects a root-authored rule to the rules// file push // put it in, so a teammate's newer version syncs down instead of being // overwritten by the stale root copy the scan would otherwise call modified. - await syncTeamUpdatesToLocal(teamConfig, localConfig, bases.revs, state.placedRules); + await syncTeamUpdatesToLocal(teamConfig, localConfig, bases.revs, state.placedRules, delivered); } catch (e) { preSyncFailure = e instanceof Error ? e.message : String(e); } @@ -1142,8 +1187,8 @@ async function pushCore( const syncedRev = recordsBase && !teamRepoStale ? await getHeadCommit(localConfig.repo.localPath) : null; - if (syncedRev) { - addPushBaseRev(bases.source === 'checkout' ? bases.record : await userScopeRecord(state), syncedRev); + if (syncedRev) addPushBaseRev(bases.source === 'checkout' ? bases.record : await userScopeRecord(state), syncedRev); + if (syncedRev || JSON.stringify(delivered) !== deliveredBefore) { try { await saveStateForScope(state, localConfig); } catch (e) { @@ -1614,6 +1659,8 @@ async function pushCore( return false; }); + await warnKeptCopiesTheTeamChanged(allItems, scanTeamConfig, localConfig); + // ── Step 1: Display ALL scanned items with numbers ───────────────── console.log(''); console.log(`Found ${allItems.length} resource(s) to push:`); diff --git a/src/resources/agents.ts b/src/resources/agents.ts index 33be1bede..dd2cd3d2b 100644 --- a/src/resources/agents.ts +++ b/src/resources/agents.ts @@ -14,6 +14,7 @@ import { loadStateForScope } from '../config.js'; import { placedResourcePath } from '../push-namespaces.js'; import { itemCandidate, resolveNamespacedItems, type NamespaceResolution } from '../namespace-resolver.js'; import { getFileContentAtRev, getFileContentWhenAdded, isPastVersionOf } from '../utils/git.js'; +import { keepsEditedCopy, recordDelivered, type DeliveryLedger } from './delivered-copies.js'; import { parseAgentYaml, serializeAgentYaml, @@ -638,7 +639,7 @@ export class AgentsHandler extends ResourceHandler { * New format (.yaml): parses spec, respects spec.targets, renders per-tool native format. * Legacy format (.md): copies .md as-is to Claude-compatible tools. */ - async pullItem(item: ResourceItem, teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise { + async pullItem(item: ResourceItem, teamConfig: TeamaiConfig, localConfig: LocalConfig, ledger?: DeliveryLedger): Promise { const agentItem = item as AgentResourceItem; // Determine format: explicit flag takes precedence; fall back to extension detection @@ -661,6 +662,7 @@ export class AgentsHandler extends ResourceHandler { for (const { tool, dest, render } of await this.resolveRenders(teamConfig, localConfig, item)) { const destDir = path.dirname(dest); try { + if (ledger && await keepsEditedCopy(ledger, item, { tool, dest, content: render.content })) continue; await ensureDir(destDir); // Only a rendered spec can leave a sibling behind: its extension follows // the tool's format and changes when `targets` does. A legacy `.md` is @@ -671,6 +673,7 @@ export class AgentsHandler extends ResourceHandler { await removeStaleAgentSiblings(destDir, item.name, render.ext); } await writeFile(dest, render.content); + if (ledger) await recordDelivered(ledger.hashes, dest); log.debug(`Rendered agent ${item.name} → ${tool} (${render.ext})`); } catch (e) { log.warn(`Failed to sync agent ${item.name} to ${tool}: ${(e as Error).message}`); @@ -963,9 +966,9 @@ type TeamAgentFile = { path: string; ext: '.yaml' | '.md'; namespace?: string }; * edit made before the author's next pull would otherwise be overwritten by the * stale local copy (#649 review). The copy stays at the revision pull delivered * while push bases move on (push records the team HEAD before the scan), so a - * difference from any of those versions counts. A guard, not a merge: `pull` - * delivers the recorded agent and resets the bases, after which the edit can be - * pushed. + * difference from any of those versions counts. A guard, not a merge: pull + * keeps the edited copy (#822), so the member takes the team version by + * deleting it and pulling, which resets the bases, then reapplies the edit. */ async function recordedAgentMovedOn(repoPath: string, relPath: string, bases: readonly string[]): Promise { const current = await readFileSafe(path.join(repoPath, relPath)); @@ -984,8 +987,8 @@ async function recordedAgentMovedOn(repoPath: string, relPath: string, bases: re function staleRecordedAgentReason(stem: string, relPath: string): string { return `Agent "${stem}" (${relPath}) changed on the team since this checkout last synced it, ` - + 'so pushing your copy would overwrite that change. `teamai pull` replaces your copy with the team version, ' - + 'so first copy your edit aside, then pull, reapply it, and push again.'; + + 'so pushing your copy would overwrite that change. `teamai pull` keeps a copy you changed, ' + + 'so copy your edit aside, delete your copy, run `teamai pull --force`, reapply the edit, and push again.'; } /** diff --git a/src/resources/base.ts b/src/resources/base.ts index 4a6ed04ff..1404f7317 100644 --- a/src/resources/base.ts +++ b/src/resources/base.ts @@ -3,6 +3,7 @@ import { COPILOT_TOOL_ID, getCopilotHome, resolveToolBaseDir, toolInstallRoot } import type { ResourceType, ResourceItem, ResourceDiff, DeliveryTarget, TeamaiConfig, LocalConfig } from '../types.js'; import { readFileSafe, writeFile, ensureDir, pathExists } from '../utils/fs.js'; import { getUserHome } from '../utils/home.js'; +import type { DeliveryLedger } from './delivered-copies.js'; const TOMBSTONE_FILE = '.removed'; @@ -71,11 +72,14 @@ export abstract class ResourceHandler { /** * Pull a resource item from the team repo and inject into local AI tool directories. + * With `ledger`, a skill, rule or agent copy the member changed since teamai + * delivered it is kept, and each write is recorded (#822). */ abstract pullItem( item: ResourceItem, teamConfig: TeamaiConfig, localConfig: LocalConfig, + ledger?: DeliveryLedger, ): Promise; /** diff --git a/src/resources/delivered-copies.ts b/src/resources/delivered-copies.ts new file mode 100644 index 000000000..5023c35bc --- /dev/null +++ b/src/resources/delivered-copies.ts @@ -0,0 +1,168 @@ +import crypto from 'node:crypto'; +import path from 'node:path'; +import fse from 'fs-extra'; +import type { DeliveryTarget, ResourceItem } from '../types.js'; +import { fileHash, listFilesRecursive } from '../utils/fs.js'; +import { log } from '../utils/logger.js'; + +/** + * What teamai last wrote at each skill, rule and agent file it delivered into + * one checkout, and what pull does with a copy that no longer has those bytes + * (#822). A copy is the member's edit only when it has a record and differs + * from it: without a record, pull writes as it always has. + */ + +/** sha256 of the bytes teamai last wrote, by absolute destination file path. */ +export type DeliveredHashes = Record; + +/** + * One file of a delivered copy, as hashes: on disk (null when missing), on + * record (undefined when teamai never recorded writing it), and what pull + * writes there now (null when the team version no longer has the file). + */ +export interface DeliveredFile { + disk: string | null; + recorded: string | undefined; + next: string | null; +} + +export type CopyVerdict = + | { kind: 'write' } + | { kind: 'keep'; teamChanged: boolean }; + +/** Push does not count a skill's CONTRIBUTORS as a change, so neither does this. */ +const CONTRIBUTORS_FILE = 'CONTRIBUTORS'; + +/** + * Keep a copy only on proof that the member changed it: a recorded file whose + * bytes are neither what teamai wrote nor what it would write now. A skill is + * one copy, kept whole, as push reads it. A copy with no file left is written, + * so deleting it is how the member takes the team version back. + */ +export function classifyCopy(files: readonly DeliveredFile[]): CopyVerdict { + if (files.every((file) => file.disk === null)) return { kind: 'write' }; + const edited = files.some((file) => file.recorded !== undefined && file.disk !== file.recorded && file.disk !== file.next); + if (!edited) return { kind: 'write' }; + return { kind: 'keep', teamChanged: files.some((file) => file.next !== (file.recorded ?? null)) }; +} + +/** A pull's view of the checkout's record, and the copies it kept. */ +export interface DeliveryLedger { + /** The record the pull started from; undefined when there is none yet, which protects nothing. */ + readonly previous: DeliveredHashes | undefined; + /** What the pull leaves on record: `previous` with its writes and removals applied. */ + readonly hashes: DeliveredHashes; + readonly kept: { dest: string; teamRelPath: string; teamChanged: boolean }[]; +} + +export function openLedger(previous: DeliveredHashes | undefined): DeliveryLedger { + return { previous, hashes: { ...previous }, kept: [] }; +} + +function sha256(content: string | Buffer): string { + return crypto.createHash('sha256').update(content).digest('hex'); +} + +/** The recorded paths of the copy at `dest`: the file, or every file under the skill directory. */ +function recordedUnder(hashes: DeliveredHashes, dest: string): string[] { + return Object.keys(hashes).filter((file) => file === dest || file.startsWith(dest + path.sep)); +} + +/** + * Each file pull writes for `target`, with its hash, plus each recorded file + * the team version no longer has. A rule or an agent is the rendered file; a + * skill is every team file, with SKILL.md as pull repairs its frontmatter. + */ +async function nextHashes(previous: DeliveredHashes, item: ResourceItem, target: DeliveryTarget): Promise> { + if (item.type !== 'skills') { + return new Map([[target.dest, target.content === undefined ? null : sha256(target.content)]]); + } + const { withSkillFrontmatter } = await import('./skills.js'); + const next = new Map(); + for (const file of await listFilesRecursive(item.sourcePath)) { + if (path.basename(file) === CONTRIBUTORS_FILE) continue; + const bytes = await fse.readFile(path.join(item.sourcePath, file)); + const text = bytes.toString('utf-8'); + const written = file === 'SKILL.md' ? withSkillFrontmatter(text, item.name) : text; + next.set(path.join(target.dest, file), sha256(written === text ? bytes : written)); + } + for (const file of recordedUnder(previous, target.dest)) { + if (!next.has(file)) next.set(file, null); + } + return next; +} + +async function withDisk(previous: DeliveredHashes, next: Iterable<[string, string | null]>): Promise { + return Promise.all([...next].map(async ([file, hash]) => ({ disk: await fileHash(file), recorded: previous[file], next: hash }))); +} + +/** What pull does with `target`'s copy of `item`. Read-only. */ +export async function judgeCopy(previous: DeliveredHashes | undefined, item: ResourceItem, target: DeliveryTarget): Promise { + if (previous === undefined) return { kind: 'write' }; + return classifyCopy(await withDisk(previous, await nextHashes(previous, item, target))); +} + +/** Whether pull leaves `target`'s copy as the member changed it; a kept copy is named by reportKept. */ +export async function keepsEditedCopy(ledger: DeliveryLedger, item: ResourceItem, target: DeliveryTarget): Promise { + const verdict = await judgeCopy(ledger.previous, item, target); + if (verdict.kind === 'write') return false; + ledger.kept.push({ dest: target.dest, teamRelPath: item.relativePath, teamChanged: verdict.teamChanged }); + return true; +} + +/** + * Whether the copy at `dest` of a resource the team removed has changed since + * teamai delivered it. Without a record it has not, and is removed as before. + */ +export async function removedCopyChanged(previous: DeliveredHashes | undefined, dest: string): Promise { + if (previous === undefined) return false; + const files = await withDisk(previous, recordedUnder(previous, dest).map((file) => [file, null])); + return classifyCopy(files).kind === 'keep'; +} + +/** + * Record what teamai just wrote at `dest`: the file, or for a skill, each + * file of its team version `skillSource`. Files only the member has are not + * recorded, so they never count as an edit. + */ +export async function recordDelivered(hashes: DeliveredHashes, dest: string, skillSource?: string): Promise { + forgetDelivered(hashes, dest); + const files = skillSource === undefined + ? [dest] + : (await listFilesRecursive(skillSource)) + .filter((file) => path.basename(file) !== CONTRIBUTORS_FILE) + .map((file) => path.join(dest, file)); + for (const file of files) { + const hash = await fileHash(file); + if (hash !== null) hashes[file] = hash; + } +} + +export function forgetDelivered(hashes: DeliveredHashes, dest: string): void { + for (const file of recordedUnder(hashes, dest)) delete hashes[file]; +} + +/** + * Name each copy pull kept, with the step that shares it or takes the team + * version. The step is `pull --force`: this pull has recorded the team + * revision, so a plain pull after it would skip the sync. + */ +export function reportKept(ledger: DeliveryLedger, scopeLabel: string): void { + const named = new Set(); + for (const { dest, teamRelPath, teamChanged } of ledger.kept.splice(0)) { + if (named.has(dest)) continue; + named.add(dest); + if (teamChanged) { + log.warn( + `[${scopeLabel}] Kept ${dest}: you changed it, and the team version (${teamRelPath}) has changed since. ` + + 'Merge the team change into your copy and share it with `teamai push`, ' + + 'or delete your copy and run `teamai pull --force` to take the team version.', + ); + } else { + log.info( + `[${scopeLabel}] Kept ${dest}: you changed it since teamai delivered it. ` + + 'Share it with `teamai push`, or delete it and run `teamai pull --force` to get the team version back.', + ); + } + } +} diff --git a/src/resources/rules.ts b/src/resources/rules.ts index cf96038c9..d4acd4328 100644 --- a/src/resources/rules.ts +++ b/src/resources/rules.ts @@ -16,6 +16,7 @@ import { loadStateForScope } from '../config.js'; import { placedResourcePath } from '../push-namespaces.js'; import { deliversEveryNamespace } from '../resource-namespaces.js'; import { getFileContentAtRev, isPastVersionOf } from '../utils/git.js'; +import { forgetDelivered, keepsEditedCopy, recordDelivered, removedCopyChanged, type DeliveryLedger } from './delivered-copies.js'; import { ruleFileExtensionForTool, ruleStemFromFilename, @@ -308,8 +309,9 @@ export class RulesHandler extends ResourceHandler { /** * Pull a single rule file to all configured AI tool rules/ directories. */ - async pullItem(item: ResourceItem, teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise { - for (const { tool, dest, content, supersedes } of await this.deliveryTargets(teamConfig, localConfig, item)) { + async pullItem(item: ResourceItem, teamConfig: TeamaiConfig, localConfig: LocalConfig, ledger?: DeliveryLedger): Promise { + for (const target of await this.deliveryTargets(teamConfig, localConfig, item)) { + const { tool, dest, content, supersedes } = target; const destDir = path.dirname(dest); try { if (content === undefined) { @@ -317,7 +319,10 @@ export class RulesHandler extends ResourceHandler { throw new Error(`Cannot read rule source ${item.sourcePath}`); } await ensureDir(destDir); - await writeFile(dest, content); + if (!ledger || !await keepsEditedCopy(ledger, item, target)) { + await writeFile(dest, content); + if (ledger) await recordDelivered(ledger.hashes, dest); + } // Drop the `.md` copy left by an older layout; a tool that reads a // derived extension does not read it, and it would outlive the rule. const legacyCopy = path.join(destDir, `${path.basename(dest, path.extname(dest))}.md`); @@ -430,6 +435,7 @@ export class RulesHandler extends ResourceHandler { localConfig: LocalConfig, filteredRules?: ResourceItem[], replacedRoots: readonly ResourceItem[] = [], + ledger?: DeliveryLedger, ): Promise { const rules = filteredRules ?? await this.scanTeamForPull(teamConfig, localConfig); @@ -457,13 +463,13 @@ export class RulesHandler extends ResourceHandler { // OpenCode glob deactivation above still runs, so the (now unmanaged) rules // stop being auto-loaded. if (rules.length === 0) { - await this.reclaimUnselectedTeamRules(teamConfig, localConfig); + await this.reclaimUnselectedTeamRules(teamConfig, localConfig, ledger); return; } // 1. Distribute rule files to each tool's rules/ directory for (const rule of rules) { - await this.pullItem(rule, teamConfig, localConfig); + await this.pullItem(rule, teamConfig, localConfig, ledger); } // 1.5. Clean up stale local rule files not present in team repo @@ -557,7 +563,16 @@ export class RulesHandler extends ResourceHandler { if (EXCLUDED_RULE_NAMES.has(ruleName)) continue; if (!teamRuleNames.has(ruleName)) { const fullPath = path.join(destDir, localFile); + // A copy the member changed since teamai delivered it stays (#822); + // the tombstone cleanup names one of a rule the team removed. + if (await removedCopyChanged(ledger?.previous, fullPath)) { + if (!tombstones.has(ruleName)) { + log.warn(`Kept ${fullPath}: teamai no longer delivers ${ruleName} here, but you changed this copy. Delete it when you no longer need it.`); + } + continue; + } await remove(fullPath); + if (ledger) forgetDelivered(ledger.hashes, fullPath); log.debug(`Removed stale rule ${localFile} from ${tool}`); } } @@ -663,6 +678,7 @@ export class RulesHandler extends ResourceHandler { private async reclaimUnselectedTeamRules( teamConfig: TeamaiConfig, localConfig: LocalConfig, + ledger: DeliveryLedger | undefined, ): Promise { const teamRules = await this.scanTeamForPull(teamConfig, localConfig); if (teamRules.length === 0) return; @@ -676,6 +692,7 @@ export class RulesHandler extends ResourceHandler { if (supersedes) continue; if (!await isDeliveredRender(tool, dest, item, localConfig.repo.localPath, deliveredRevs)) continue; await remove(dest); + if (ledger) forgetDelivered(ledger.hashes, dest); touchedDirs.add(path.join(resolveToolBaseDir(tool, localConfig), scopedToolPaths(teamConfig, localConfig)[tool].rules!)); log.debug(`Removed unselected team rule ${item.name} from ${tool}`); } diff --git a/src/resources/skills.ts b/src/resources/skills.ts index 72a5761d9..e318bfeb9 100644 --- a/src/resources/skills.ts +++ b/src/resources/skills.ts @@ -3,7 +3,7 @@ import YAML from 'yaml'; import { isToolInstalledForConfig, ResourceHandler } from './base.js'; import type { ResourceItem, ResourceItemStatus, DeliveryTarget, TeamaiConfig, LocalConfig } from '../types.js'; import { getPushignorePath, isAgentExcluded, resolveToolBaseDir, scopedToolPaths, SELF_KNOWLEDGE_SCAN_KEY } from '../types.js'; -import { listDirs, listFilesRecursive, pathExists, copyDir, remove, pruneEmptyDirs, dirContentEqual, dirTeamSubsetEqual, fileContentEqual, getDirLatestMtime, readFileSafe, writeFile } from '../utils/fs.js'; +import { listDirs, listFilesRecursive, pathExists, copyDir, remove, pruneEmptyDirs, dirContentEqual, dirTeamSubsetEqual, fileContentEqual, fileHash, getDirLatestMtime, readFileSafe, writeFile } from '../utils/fs.js'; import { log } from '../utils/logger.js'; import { getFileContentWhenAdded, isPastVersionOf } from '../utils/git.js'; import { isCliOwnedSkillName } from '../builtin-skills.js'; @@ -16,6 +16,7 @@ import { loadProjectsManifest, resolveProjectResourceNamespaces } from '../proje import { assertSafeFallbackNamespaces } from '../manifest-schema.js'; import { assertWithinRoot } from '../utils/path-safety.js'; import { splitFrontmatter, stringifyFrontmatter } from '../utils/frontmatter.js'; +import { keepsEditedCopy, recordDelivered, type DeliveredHashes, type DeliveryLedger } from './delivered-copies.js'; /** File name used to track who has contributed (pushed) a skill. */ const CONTRIBUTORS_FILE = 'CONTRIBUTORS'; @@ -158,27 +159,40 @@ export async function ensureSkillFrontmatter(skillDir: string, skillName: string const content = await readFileSafe(skillMdPath); if (!content) return false; + const { raw, valid } = splitFrontmatter(content); + if (raw && !valid) { + log.warn(`Could not repair malformed frontmatter in ${skillName}/SKILL.md; leaving it unchanged`); + return false; + } + + const repaired = withSkillFrontmatter(content, skillName); + if (repaired === content) return false; // Already complete + await writeFile(skillMdPath, repaired); + log.debug(`Added missing frontmatter to ${skillName}/SKILL.md`); + return true; +} + +/** + * The SKILL.md `ensureSkillFrontmatter` leaves from `content`: the same text + * when it is empty, already has `name` and `description`, or has frontmatter + * that does not parse. + */ +export function withSkillFrontmatter(content: string, skillName: string): string { + if (!content) return content; const { data, body, raw, valid } = splitFrontmatter(content); if (!raw) { // No frontmatter at all — derive description from first heading or first non-empty line const description = extractDescriptionFromContent(body, skillName); - const newContent = stringifyFrontmatter({ name: skillName, description }, body); - await writeFile(skillMdPath, newContent); - log.debug(`Injected YAML frontmatter into ${skillName}/SKILL.md`); - return true; - } - - if (!valid) { - log.warn(`Could not repair malformed frontmatter in ${skillName}/SKILL.md; leaving it unchanged`); - return false; + return stringifyFrontmatter({ name: skillName, description }, body); } + if (!valid) return content; // Frontmatter exists — check for missing fields const hasName = typeof data['name'] === 'string' && String(data['name']).trim() !== ''; const hasDescription = typeof data['description'] === 'string' && String(data['description']).trim() !== ''; - if (hasName && hasDescription) return false; // Already complete + if (hasName && hasDescription) return content; const missingFields: Record = {}; if (!hasName) missingFields.name = skillName; @@ -186,10 +200,7 @@ export async function ensureSkillFrontmatter(skillDir: string, skillName: string // Preserve existing comments, quoting, key order, and line endings. Re-serializing // the whole block would make an unrelated metadata repair unnecessarily lossy. - const newContent = appendFrontmatterFields(raw, missingFields) + body; - await writeFile(skillMdPath, newContent); - log.debug(`Added missing frontmatter fields to ${skillName}/SKILL.md`); - return true; + return appendFrontmatterFields(raw, missingFields) + body; } /** @@ -404,8 +415,12 @@ async function otherVersionFiles(repoPath: string, item: ResourceItem): Promise< * one behind. Any other file is the member's own, and stays: push does not * count such an extra as a change, so it may never have been pushed. One at a * path another version has is named, since it may be an edited leftover. + * `delivered` tells them apart (#822): a file still as teamai recorded writing + * it is a leftover, and one it has no record of is the member's own. */ -async function removeLeftoverVersionFiles(source: string, dest: string, otherVersions: Map): Promise { +async function removeLeftoverVersionFiles( + source: string, dest: string, otherVersions: Map, delivered: DeliveredHashes | undefined, +): Promise { if (otherVersions.size === 0) return; const sourceFiles = new Set(await listFilesRecursive(source)); let removed = false; @@ -413,8 +428,11 @@ async function removeLeftoverVersionFiles(source: string, dest: string, otherVer const versions = otherVersions.get(file); if (sourceFiles.has(file) || !versions) continue; const installed = path.join(dest, file); - const leftover = (await Promise.all(versions.map((version) => fileContentEqual(installed, version)))).some(Boolean); + const recorded = delivered?.[installed]; + const leftover = (recorded !== undefined && await fileHash(installed) === recorded) + || (await Promise.all(versions.map((version) => fileContentEqual(installed, version)))).some(Boolean); if (!leftover) { + if (delivered !== undefined && recorded === undefined) continue; log.warn( `Kept ${installed}: another team version of this skill has a file at that path with different content, ` + 'so it may be yours or an edited copy. Delete it if you do not need it.', @@ -736,13 +754,16 @@ export class SkillsHandler extends ResourceHandler { /** * Pull a skill from team repo to all configured AI tool directories. */ - async pullItem(item: ResourceItem, teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise { + async pullItem(item: ResourceItem, teamConfig: TeamaiConfig, localConfig: LocalConfig, ledger?: DeliveryLedger): Promise { const otherVersions = await otherVersionFiles(localConfig.repo.localPath, item); - for (const { tool, dest } of await this.resolveTargets(teamConfig, localConfig, item, item.sourcePath)) { + for (const target of await this.resolveTargets(teamConfig, localConfig, item, item.sourcePath)) { + const { tool, dest } = target; try { + if (ledger && await keepsEditedCopy(ledger, item, target)) continue; await copyDir(item.sourcePath, dest); - await removeLeftoverVersionFiles(item.sourcePath, dest, otherVersions); + await removeLeftoverVersionFiles(item.sourcePath, dest, otherVersions, ledger?.previous); await ensureSkillFrontmatter(dest, item.name); + if (ledger) await recordDelivered(ledger.hashes, dest, item.sourcePath); log.debug(`Synced skill ${item.name} → ${tool}`); } catch (e) { log.warn(`Failed to sync skill ${item.name} to ${tool}: ${(e as Error).message}`); diff --git a/src/types.ts b/src/types.ts index b0f4f026d..2125fd489 100644 --- a/src/types.ts +++ b/src/types.ts @@ -699,11 +699,16 @@ export const StateSchema = z.object({ * (`FORCED_FULL_SYNC_REV` in pull.ts). The user scope's entry is HOME's. An * inherited pull, and a pull whose docs mirror or submodule update fails, add * the revision they delivered to these bases and keep `rev` (#823). + * `delivered` is the sha256 of the bytes teamai last wrote at each skill, + * rule and agent file path of the checkout, which pull and the pre-push sync + * update. Pull keeps a copy that no longer matches it; without it, pull + * overwrites as before (#822). An older CLI that saves state drops it. */ lastPullByWorkspace: z.record(z.string(), z.object({ rev: z.string(), targets: z.array(z.string()), pushBaseRevs: z.array(z.string()).optional(), + delivered: z.record(z.string(), z.string()).optional(), })).optional(), /** Git commit hash synchronized through the safe user-resource inheritance channel. */ lastInheritedPullRev: z.string().nullable().optional(), diff --git a/src/utils/fs.ts b/src/utils/fs.ts index 290e0803a..77ab90047 100644 --- a/src/utils/fs.ts +++ b/src/utils/fs.ts @@ -382,7 +382,7 @@ export async function getDirLatestMtime(dirPath: string): Promise { /** * Compute SHA-256 hash of a file's contents. Returns null if file does not exist. */ -async function fileHash(filePath: string): Promise { +export async function fileHash(filePath: string): Promise { try { const content = await fse.readFile(filePath); return crypto.createHash('sha256').update(content).digest('hex'); diff --git a/src/utils/pre-push-sync.ts b/src/utils/pre-push-sync.ts index c4288ac20..3ee91664c 100644 --- a/src/utils/pre-push-sync.ts +++ b/src/utils/pre-push-sync.ts @@ -23,6 +23,7 @@ import { teamRuleToCopilotInstructions, copilotInstructionsBodyEqualsTeamMd } fr import { EXCLUDED_RULE_NAMES } from '../builtin-rules.js'; import { log } from './logger.js'; import { placedResourcePath } from '../push-namespaces.js'; +import { recordDelivered, type DeliveredHashes } from '../resources/delivered-copies.js'; const CONTRIBUTORS_FILE = 'CONTRIBUTORS'; @@ -52,12 +53,17 @@ const CONTRIBUTORS_FILE = 'CONTRIBUTORS'; * without this map the three-way check below would skip it and the scanner — * which DOES follow the map — would then read the stale root copy as a local * modification and push it over a teammate's newer version. + * + * `delivered` is the checkout record's map of what teamai wrote (#822). Each + * copy the sync writes is recorded there, or the next pull after a further + * team change would read it as the member's edit and keep it. */ export async function syncTeamUpdatesToLocal( teamConfig: TeamaiConfig, localConfig: LocalConfig, baseRevs: string | readonly string[] | null, placedRules?: Record, + delivered?: DeliveredHashes, ): Promise { const bases = (typeof baseRevs === 'string' ? [baseRevs] : baseRevs ?? []).filter((rev) => rev !== ''); if (bases.length === 0) { @@ -68,8 +74,8 @@ export async function syncTeamUpdatesToLocal( const repoPath = localConfig.repo.localPath; const baseDir = resolveBaseDir(localConfig); - await syncRulesToLocal(teamConfig, localConfig, repoPath, bases, placedRules); - await syncSkillsToLocal(teamConfig, localConfig, repoPath, baseDir, bases); + await syncRulesToLocal(teamConfig, localConfig, repoPath, bases, placedRules, delivered); + await syncSkillsToLocal(teamConfig, localConfig, repoPath, baseDir, bases, delivered); } /** @@ -82,6 +88,7 @@ async function syncRulesToLocal( repoPath: string, bases: readonly string[], placedRules: Record | undefined, + delivered: DeliveredHashes | undefined, ): Promise { const teamRulesDir = path.join(repoPath, 'rules'); if (!await pathExists(teamRulesDir)) return; @@ -160,6 +167,7 @@ async function syncRulesToLocal( ? localRaw === render(old.toString('utf-8')) : bodyEquals(localRaw, old.toString('utf-8')))) { await writeFile(localFilePath, render(teamRaw)); + if (delivered) await recordDelivered(delivered, localFilePath); log.debug(`Pre-push sync: updated ${tool} rule ${name} to match team repo`); } continue; @@ -181,6 +189,7 @@ async function syncRulesToLocal( if (matchesBase) { // Local matches old team version → team updated, user didn't → sync await copyFile(teamFilePath, localFilePath); + if (delivered) await recordDelivered(delivered, localFilePath); log.debug(`Pre-push sync: updated ${tool} rule ${name} to match team repo`); } // else: local differs from old version too → user edited → leave alone @@ -198,6 +207,7 @@ async function syncSkillsToLocal( repoPath: string, baseDir: string, bases: readonly string[], + delivered: DeliveredHashes | undefined, ): Promise { const teamSkillsDir = path.join(repoPath, 'skills'); if (!await pathExists(teamSkillsDir)) return; @@ -245,6 +255,7 @@ async function syncSkillsToLocal( if (await skillAtBase(repoPath, localSkillDir, teamSkillDir, teamFiles, base)) { // All differing files match that base → team updated, user didn't → sync await replaceSkillDir(teamSkillDir, localSkillDir); + if (delivered) await recordDelivered(delivered, localSkillDir, teamSkillDir); log.debug(`Pre-push sync: updated ${tool} skill ${skillName} to match team repo`); break; }