diff --git a/CHANGELOG.md b/CHANGELOG.md index c58df672c..1777ef717 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ All notable changes to this project will be documented in this file. See [standa ### 🐛 Bug Fixes +- Team rules reach Codex, `codex-internal` and `tcodex`. Pull used to copy them to `.codex/rules/.md`, which Codex does not read, and `teamai doctor` reported them delivered. In user scope they now go into a team-rules block of the tool's own `AGENTS.md` (`~/.codex/AGENTS.md`), beside the culture, shared-instructions and recall blocks. In a project the session-start hook adds the project's rules and those blocks to each session, and to a spawned subagent through a new `SubagentStart` entry, with `additionalContextLimit: 0` so Codex keeps them whole; pull leaves the project `AGENTS.md`, which other tools read too, unchanged. Pull removes the `.md` copies earlier pulls left in `.codex/rules/`, keeping an edited one with a warning. Pull and uninstall keep a removed rule's legacy copy unless it matches its recorded delivery hash; a missing record proves nothing about local edits. A path-scoped rule is rendered without frontmatter after an `Applies to files matching: ` line, in Hermes' `SOUL.md` too. `teamai doctor` checks the user-scope block and, in a project, both hook entries' limit. The public Codex asks once to approve the changed teamai hooks (for [#938](https://github.com/Tencent/teamai-cli/issues/938)). - `teamai pull` names the skills it removes because they are no longer delivered here, in one line, instead of a `debug` line calling them excluded and a summary that says `No resources to sync`. Picking a role or project, or an admin adding `manifest/projects.yaml`, takes root skills away, since the root `skills/` is then the tag catalog; the line then says that `teamai tags subscribe ` brings one back. A namespace skill removed because its namespace is no longer active is named too. The usage guide and the multi-project design no longer say a member with no project still gets `common` or every root item (for [#911](https://github.com/Tencent/teamai-cli/issues/911)). - `teamai pull` keeps the `teamai tags subscribe ` recovery line when the skill directory it removes is byte-identical to an inactive namespace copy: that copy made the namespace cleanup phase remove the directory first, and the hint was lost, because pull inferred whether a root skill had left from which phase did the removing. The hint now follows the repo — it appears exactly when the team repo holds the removed skill at the root, the copy a tag delivers — so a namespace-only skill removed on deactivation is still named without the hint. The usage guide no longer says a member with no role gets no skills at all, in English or Chinese: root skills still arrive through a tag (review of [#917](https://github.com/Tencent/teamai-cli/pull/917)). - `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)). diff --git a/docs/usage-guide.md b/docs/usage-guide.md index 40fe539c0..1920a7cb6 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -668,7 +668,7 @@ With role-based skills enabled, `pull`'s skill sync source becomes the contents - A skill replaces the root skill of the same directory name, including a root skill you receive through a tag. The install removes the files of the version it replaces. Files that no team version of the skill has stay. - An agent replaces the root agent of the same file stem. -- A rule replaces the root rule of the same first-level file name: `rules//.md` replaces `rules/.md`, in Hermes' `SOUL.md` block too. Deeper paths such as `rules///.md` replace nothing, and neither does a namespace rule your tag subscriptions leave out. In rule directories you share with rules of your own (JoyCode, OMP, Pi, Copilot), the replaced root rule's copy is removed only while it is what teamai delivered (the current root rule, or the one of your last pull); an edited copy stays, and each pull names it, since the tool loads it beside the namespace rule. +- A rule replaces the root rule of the same first-level file name: `rules//.md` replaces `rules/.md`, in Hermes' `SOUL.md` block and the rules Codex's session-start hook adds too. Deeper paths such as `rules///.md` replace nothing, and neither does a namespace rule your tag subscriptions leave out. In rule directories you share with rules of your own (JoyCode, OMP, Pi, Copilot), the replaced root rule's copy is removed only while it is what teamai delivered (the current root rule, or the one of your last pull); an edited copy stays, and each pull names it, since the tool loads it beside the namespace rule. - A `claudemd//.md` file replaces `claudemd/.md` in the managed block. When the namespace stops being active, the next pull delivers the root item again. If two active namespaces define the same skill or agent name, they compete for one installed file, so pull reports an error that names both files, does not update that type in that run, and keeps what is installed (for skills, recall keeps the ones it had indexed too); the other resource types still sync. Two active namespaces with the same rule or shared-instructions name are both delivered, because each keeps its own place (`rules//` locally, its own section of the block); only the root one gives way. `push` writes an edit of a replaced item back to its namespace, never to the root, and recall indexes the skills and rules you receive rather than every one in the repo. A replacement that cannot be used replaces nothing: a skill directory without `SKILL.md` is not delivered and pull names it, and while an agent file does not parse the agent it would replace stays installed. `teamai doctor` lists each replacement as a note. Without roles or projects nothing changes: every namespace is delivered beside the root, and `doctor` lists each name the team repo defines more than once. @@ -960,6 +960,14 @@ teamai push > Admins can set enforced rules in `teamai.yaml` (`sharing.rules.enforced`), which members cannot delete. +Most tools get one file per rule in their rules directory. Codex, `codex-internal` and `tcodex` read no rules directory (`.codex/rules/` holds Codex's own `*.rules` command policies), so `pull` writes no rule file for them. In user scope the team rules go into a `` block of the tool's own `AGENTS.md` (`~/.codex/AGENTS.md`, `~/.codex-internal/AGENTS.md`, `~/.tcodex/AGENTS.md`; a `toolRoots` entry moves it), which only that tool reads. In a project their session-start hook adds the project's team rules to each session instead: the project `AGENTS.md` is the owners' file, and other tools with a rules format of their own read it too. Hermes gets the same text in its `SOUL.md` block. Frontmatter is dropped, so a rule with `paths:` applies everywhere there, led by an `Applies to files matching: ` line. Codex runs the hook again after a compaction or a clear, and adds nothing when it resumes a session, which already holds the rules. A subagent Codex spawns gets them through the `SubagentStart` hook. The public Codex runs a new or changed hook only after you approve it in `/hooks`, and until then it gets no project rules. + +The culture, shared-instructions and recall blocks follow the same split. In user scope they go to that same `AGENTS.md`, and your own content outside the markers is kept. In a project the session-start hook adds them with the rules, and `pull` leaves the project `AGENTS.md` unchanged. When another tool, such as Pi, already wrote a block into the project `AGENTS.md`, which Codex reads too, the hook leaves that block out. + +> A `toolPaths` in the team `teamai.yaml` replaces the built-in defaults whole. A team that sets it should give each Codex-family entry `userScope.claudemd: .codex/AGENTS.md` (`.codex-internal/…`, `.tcodex/…`) for the user-scope rules and blocks, and drop its `rules` path, since Codex never reads that directory. A top-level `claudemd` would put the blocks back in the project `AGENTS.md`, so leave it out. In a project the hook needs only the entry's `settings` path, where it is installed. + +> Upgrading from a release that copied rules to `.codex/rules/`: the next `pull` removes the `.md` copies teamai delivered there, including `teamai-recall.md`. A copy you edited is kept and named in a warning, and the `*.rules` files are never touched. A copy of a rule the team has since removed is deleted only if it matches its recorded delivery hash; without that record, it is kept and named too. The same pull adds `additionalContextLimit: 0` and a `SubagentStart` entry to the teamai hooks in `hooks.json`, so the public Codex asks you once to approve the changed hooks. + ### Env, hooks and MCP servers by namespace Env variables, team hooks and MCP servers are each a list file in the team @@ -1908,7 +1916,7 @@ teamai hooks inject # Re-inject teamai hooks remove # Remove ``` -`hooks list` prints the built-in set per tool, because the set is not universal: Copilot also gets `SessionEnd`, Claude Code, Codex, CodeBuddy and Qoder also get `SubagentStop`, OMP's extension covers four events without the `Skill` / `TodoWrite` matchers, OpenClaw maps only `SessionStart` + `UserPromptSubmit`, and Hermes only `SessionStart`. Tools the hook pipeline installs nothing for (e.g. JoyCode) are omitted, and so is Kiro — its `SessionStart` command is embedded as `hooks.agentSpawn` by the agent sync, so it exists only for the agents you actually synced. +`hooks list` prints the built-in set per tool, because the set is not universal: Copilot also gets `SessionEnd`, Claude Code, Codex, CodeBuddy and Qoder also get `SubagentStop`, the Codex family also gets `SubagentStart` (the project's team rules and instructions for a spawned subagent), OMP's extension covers four events without the `Skill` / `TodoWrite` matchers, OpenClaw maps only `SessionStart` + `UserPromptSubmit`, and Hermes only `SessionStart`. Tools the hook pipeline installs nothing for (e.g. JoyCode) are omitted, and so is Kiro — its `SessionStart` command is embedded as `hooks.agentSpawn` by the agent sync, so it exists only for the agents you actually synced. The inject and remove commands only touch tools you actually have installed (i.e. whose `~/./` root directory already exists). They never create root directories for tools listed in `toolPaths` but not installed. @@ -2233,7 +2241,7 @@ Besides the provider, clone, config and hook checks, `doctor` verifies what reac `Rules delivered to ` and `Agents delivered to ` do the same for the other two per-tool resources, and both ask the handler where an item lands rather than deriving a path: a rule's filename and content change per tool (`.md` verbatim, `.mdc` with derived `globs`/`alwaysApply`, `.instructions.md` with `applyTo`), and an agent's destination comes from its render, with `targets:` deciding which tools are owed a copy at all. A delivered rule is compared with the bytes the handler renders for that tool, not merely read for the keys its tool needs: a `.mdc` whose `globs` no longer match the team rule's `paths:` applies to the wrong files while carrying a perfectly legal `alwaysApply`, and that reads here as `delivered from an older copy` — the same label as a body that drifted, because both landed successfully and are still wrong. An agent is compared with the bytes its render produces, so a copy left behind by an older spec — a plain pull skips a scope whose team repo has not changed, so it can sit there indefinitely — is reported as `delivered from an older spec` rather than passing as present. `Every team agent reaches a tool` names an agent that renders for no installed tool — usually a spec that does not parse, or a `targets:` list naming only tools you do not have. These two are `doctor`-only: they read every rule per tool and parse every agent, which would spend the budget the checks at the end of a pull run under. -Two tools do not read a rules directory, so a per-file check cannot speak for them and each gets one of its own. `Team rules are active in opencode` checks that `opencode.json` still lists the glob the pull owns under `instructions`: OpenCode does not auto-scan `.opencode/rules`, so without it every delivered `.md` is inert while the per-file check keeps passing. `Team rules are inlined in Hermes SOUL.md` compares the teamai-managed block of `SOUL.md` with what the team rules inline to, since Hermes reads standing instructions from that one file rather than from a directory — a deleted block, or one left on an older rule set, is a tool reading the wrong rules with nothing on disk to show for it. +Three tools do not read a rules directory, so a per-file check cannot speak for them and each gets one of its own. `Team rules are active in opencode` checks that `opencode.json` still lists the glob the pull owns under `instructions`: OpenCode does not auto-scan `.opencode/rules`, so without it every delivered `.md` is inert while the per-file check keeps passing. `Team rules are inlined in Hermes SOUL.md` compares the teamai-managed block of `SOUL.md` with what the team rules inline to, since Hermes reads standing instructions from that one file rather than from a directory — a deleted block, or one left on an older rule set, is a tool reading the wrong rules with nothing on disk to show for it. In user scope, `Team rules are inlined in Codex AGENTS.md` compares the team-rules block of the tool's `AGENTS.md` with what the team rules inline to, and fails when an `AGENTS.override.md` beside it shadows the file. In a project, `Project rules and instructions reach whole through its session hooks` fails when the teamai `SessionStart` or `SubagentStart` entry in that tool's `hooks.json` is missing or does not set `additionalContextLimit: 0`, without which Codex keeps only the start and end of a large set. Codex has no `Rules delivered to ` check. `MCP servers delivered to ` compares each server the team's `mcp.yaml` resolves for that tool against the entry in the tool's own config, and names any the reconcile skipped with its reason. The comparison is the entry, not the name: reconciliation leaves an entry teamai does not own alone, so a server of your own under a team name holds the key while the team's definition never arrives, and a stale copy is just as undelivered. Both are reported as `not the team's definition`, and only `teamai pull --force` replaces an entry teamai did not write. An unresolved `${VAR}` is reported here with the variable's name, which is otherwise said once during a pull and never again. A declared secret with no value is not a failure: doctor prints it as a note (`notes` in `--json`) with the command that sets it, and the exit code stays as it would be without it; a note also says when an entry kept for it may hold an old value, and when a key is declared as a secret and also set in `env.yaml`. An `mcp.yaml` that does not parse is not a team without MCP: it is reported as `Team MCP servers can be read` with the parse error, since it injects nothing into any tool and every run after the first is silent about it. Team hooks and team model profiles that cannot be resolved (a file that does not parse, a name defined twice in one file, or one name in two active namespaces) fail `Team hooks can be resolved` and `Team model profiles can be resolved` with the reason pull logs once; `teamai status` points here when it counts them as 0. `Env variables injected in shell profile` no longer stops at finding the marker comment: it checks that `env/env.yaml` parses and declares its variables under the `variables:` key (a plain `KEY: value` mapping parses as none, while an explicit `variables: []` is a configuration with nothing to deliver and fails nothing), that each one reached `env.sh` with the value `env.yaml` declares, or your value for this team (one set with `--from-env` is not written there) — a key left over from an older value exports it to every shell and MCP server until the next pull, and the comparison reads `env.sh` back through the generator's own inverse, so a multiline value quoted across several lines is matched rather than called stale — and that this scope's injected block (the one sourcing its own `env.sh`, since a profile can also carry another scope's) would actually load it — an unquoted Windows path degrades to something a POSIX shell cannot read, so `source` never runs and nothing says so. `No stale env blocks left behind` is a separate check: which file `pull` prefers has changed over time (Windows Git Bash's login shell reads `.bash_profile`/`.bash_login`/`.profile`, never `.bashrc`), and a pull only ever adds a block, never migrates an old one away, so a dead block from an earlier install or platform change can sit in another candidate file indefinitely. It names every such file (checking `.zshrc`, `.bashrc`, `.bash_profile`, `.bash_login` and `.profile`, current and legacy spellings alike) and points at `teamai uninstall` to remove them — separately from delivery, so a working env block never reads as broken just because an old one is still lying around. @@ -2752,9 +2760,9 @@ teamai uninstall --agent claude What gets removed: - TeamAI-managed model settings are restored first when ownership is still intact - teamai hooks in AI tool settings -- The teamai rules block in CLAUDE.md (your own content is preserved) +- The teamai blocks in CLAUDE.md and AGENTS.md (your own content is preserved) - Team-synced skills, including OpenClaw workspace skills (your own skills are preserved) -- Team-synced rules +- Team-synced rules, including the copies older releases left in `.codex/rules/`, also of rules the team has since removed. A copy there you edited is kept and named in a warning. A removed rule's copy is deleted only if it matches its recorded delivery hash; without that record, it is kept and named too. Codex's `*.rules` files are kept - Team-synced custom agents and CLI built-in agents (your own agents are preserved) - The env block in your shell profile — every candidate file (`.zshrc`, `.bashrc`, `.bash_profile`, `.bash_login`, `.profile`) carrying a block that sources this scope's own `env.sh` is cleaned, not only the one file `pull` would choose today; a block sourcing a different scope's `env.sh` is left alone - The `~/.teamai/` directory @@ -2763,6 +2771,8 @@ What gets removed: `--agent ` removes only that tool's teamai resources (hooks, CLAUDE.md block, skills, rules, team-synced custom agents, and built-in agents). The tool name is a key of `toolPaths` (e.g. `claude`, `codex`, `codebuddy`) and is matched case-insensitively. An unknown tool name aborts without deleting anything, lists the available tools, and exits with a non-zero status. +An instructions file several tools map is cleaned per block: a teamai block stays while a remaining tool on that file still writes it. The project `AGENTS.md` is the common case: with Pi still enabled, `--agent workbuddy` removes the recall block and keeps the culture and shared-instructions blocks Pi writes. The Codex family writes nothing there. A file teamai created goes with its last block; an instructions file you had before stays, even an empty one. + Shared resources (the env block, docs directory, and `~/.teamai/`) are removed **only when the target itself has teamai resources AND is the last tool still using teamai** — otherwise they are kept for the remaining tools. (So targeting a tool that has no teamai resources of its own is a no-op and leaves shared resources in place, even if it happens to be the only tool.) The exclusion is durable: `uninstall --agent ` drops the tool from `enabledAgents` and records it in `disabledAgents`, so a later `pull` (or another tool's session-start hook) will not resurrect its skills, rules, agents, CLAUDE.md block, or hooks. Running `init --agent ` again clears the exclusion and re-enables sync for that tool. diff --git a/docs/usage-guide.zh-CN.md b/docs/usage-guide.zh-CN.md index 10c557148..4229aaad5 100644 --- a/docs/usage-guide.zh-CN.md +++ b/docs/usage-guide.zh-CN.md @@ -594,7 +594,7 @@ teamai pull --dry-run # 试运行，不实际修改 - skill 按目录名替换根目录的同名 skill，包括你通过标签收到的根目录 skill。安装时会删除被替换版本的文件；任何团队版本都没有的文件会保留。 - agent 按文件名（不含扩展名）替换根目录的同名 agent。 -- rule 按第一层文件名替换：`rules//.md` 替换 `rules/.md`，Hermes 的 `SOUL.md` 区块同样如此。更深的路径（如 `rules///.md`）不替换任何文件，被你的标签订阅排除的 namespace rule 也不替换。在与你自己的 rule 共用的目录中（JoyCode、OMP、Pi、Copilot），被替换的根 rule 副本只在仍是 teamai 所下发的内容（当前的根 rule，或你上次 pull 时的版本）时删除；你改过的副本会保留，且每次 pull 都会点名它，因为工具会把它与 namespace rule 一起加载。 +- rule 按第一层文件名替换：`rules//.md` 替换 `rules/.md`，Hermes 的 `SOUL.md` 区块和 Codex 的 session-start hook 添加的 rule 同样如此。更深的路径（如 `rules///.md`）不替换任何文件，被你的标签订阅排除的 namespace rule 也不替换。在与你自己的 rule 共用的目录中（JoyCode、OMP、Pi、Copilot），被替换的根 rule 副本只在仍是 teamai 所下发的内容（当前的根 rule，或你上次 pull 时的版本）时删除；你改过的副本会保留，且每次 pull 都会点名它，因为工具会把它与 namespace rule 一起加载。 - `claudemd//.md` 在托管区块中替换 `claudemd/.md`。 该 namespace 不再活跃后，下一次 pull 会重新下发根目录条目。两个活跃 namespace 定义同名 skill 或 agent 时，它们会争用同一个安装文件，因此 pull 会报错并列出两个文件，本次运行不更新该类型，已安装的内容保持不变（skills 在 recall 中已有的索引也保持不变）；其他资源类型照常同步。两个活跃 namespace 定义同名 rule 或共享指令时，两者都会下发，因为它们各有自己的位置（本地的 `rules//`、区块中各自的一段）；只有根目录的那一份会让位。`push` 会把被替换条目的修改写回其 namespace，而不会写到根目录；recall 只索引你实际收到的 skills 和 rules，而不是仓库中的全部内容。无法使用的替换项不会替换任何内容：没有 `SKILL.md` 的 skill 目录不会下发，pull 会点名提示；agent 文件无法解析时，它原本要替换的 agent 保持安装。`teamai doctor` 会以提示的形式列出每一处替换。未配置角色或项目时行为不变：所有 namespace 与根目录并列下发，`doctor` 会列出团队仓库中重复定义的每个名称。 @@ -881,6 +881,14 @@ teamai push > 管理员可在 `teamai.yaml` 中设置强制规则（`sharing.rules.enforced`），成员不可删除。 +大多数工具在自己的 rules 目录中为每条 rule 得到一个文件。Codex、`codex-internal` 和 `tcodex` 不读取 rules 目录（`.codex/rules/` 存放的是 Codex 自己的 `*.rules` 命令策略文件），因此 `pull` 不为它们写任何 rule 文件。user scope 下，团队 rule 写入该工具自己的 `AGENTS.md`（`~/.codex/AGENTS.md`、`~/.codex-internal/AGENTS.md`、`~/.tcodex/AGENTS.md`；`toolRoots` 条目可改变其位置）中的 `` 区块，只有该工具读取这个文件。在项目中，改由它们的 session-start hook 把项目的团队 rule 加入每个会话：项目 `AGENTS.md` 属于项目维护者，其他拥有自己 rules 格式的工具也会读取它。Hermes 的 `SOUL.md` 区块得到同样的内容。frontmatter 会被去掉，所以带 `paths:` 的 rule 在这里对所有文件生效，并以一行 `Applies to files matching: ` 开头。Codex 在压缩上下文或 clear 之后会再次运行该 hook；恢复会话时不添加任何内容，因为会话中已包含这些 rule。Codex 启动的子 agent 通过 `SubagentStart` hook 获得它们。公开版 Codex 只在你于 `/hooks` 中批准新增或改动的 hook 后才运行它，在此之前不会得到项目的 rule。 + +culture、共享指令和 recall 区块采用同样的划分。user scope 下它们写入同一个 `AGENTS.md`，标记之外你自己的内容保持不变。在项目中，session-start hook 把它们与 rule 一起加入会话，`pull` 不改动项目 `AGENTS.md`。如果其他工具（如 Pi）已把某个区块写入项目 `AGENTS.md`，而 Codex 也会读取该文件，hook 就不再重复添加这个区块。 + +> 团队 `teamai.yaml` 中的 `toolPaths` 会整体替换内置默认值。设置了它的团队应为每个 Codex 系条目加上 `userScope.claudemd: .codex/AGENTS.md`（`.codex-internal/…`、`.tcodex/…`），用于 user scope 的 rule 和区块，并去掉其 `rules` 路径，因为 Codex 从不读取该目录。顶层的 `claudemd` 会把区块重新写进项目 `AGENTS.md`，所以不要设置。在项目中，hook 只需要该条目的 `settings` 路径，它安装在那里。 + +> 从把 rule 复制到 `.codex/rules/` 的旧版本升级后，下一次 `pull` 会删除 teamai 投递到那里的 `.md` 副本，包括 `teamai-recall.md`。你改过的副本会保留，并在警告中点名；`*.rules` 文件从不改动。团队此后已删除的 rule，其副本只有与记录的投递哈希一致时才会删除；没有该记录时也会保留并点名。同一次 pull 会为 `hooks.json` 中的 teamai hook 加上 `additionalContextLimit: 0` 和一个 `SubagentStart` 条目，因此公开版 Codex 会请你批准一次改动后的 hook。 + ### Env、hooks 与 MCP server 按 namespace 划分 环境变量、团队 hooks 和 MCP server 各自是团队仓库根目录下的一个列表文件（对所有人共享）， @@ -1773,7 +1781,7 @@ teamai hooks inject # 重新注入 teamai hooks remove # 移除 ``` -`hooks list` 按工具分别列出内置 hooks，因为各工具的集合并不相同：Copilot 额外有 `SessionEnd`，Claude Code、Codex、CodeBuddy 和 Qoder 额外有 `SubagentStop`，OMP 扩展覆盖四个事件且没有 `Skill` / `TodoWrite` matcher，OpenClaw 只映射 `SessionStart` + `UserPromptSubmit`，Hermes 只有 `SessionStart`。hook 注入流程不会为其安装任何内置 hook 的工具（如 JoyCode）不会列出；Kiro 也不列出——它的 `SessionStart` 由 agent 同步以 `hooks.agentSpawn` 形式内嵌，只存在于你实际同步过的 agent 中。 +`hooks list` 按工具分别列出内置 hooks，因为各工具的集合并不相同：Copilot 额外有 `SessionEnd`，Claude Code、Codex、CodeBuddy 和 Qoder 额外有 `SubagentStop`，Codex 系工具还额外有 `SubagentStart`（为其启动的子 agent 提供项目的团队 rule 和指令），OMP 扩展覆盖四个事件且没有 `Skill` / `TodoWrite` matcher，OpenClaw 只映射 `SessionStart` + `UserPromptSubmit`，Hermes 只有 `SessionStart`。hook 注入流程不会为其安装任何内置 hook 的工具（如 JoyCode）不会列出；Kiro 也不列出——它的 `SessionStart` 由 agent 同步以 `hooks.agentSpawn` 形式内嵌，只存在于你实际同步过的 agent 中。 inject 和 remove 只会操作你实际已安装的工具（即 `~/./` 根目录已存在的工具）。对于 `toolPaths` 中已配置但未安装的工具，命令不会为其凭空创建根目录。 @@ -2096,7 +2104,7 @@ teamai remove rules --force # 跳过确认，用于脚本和 CI `Rules delivered to ` 与 `Agents delivered to ` 对另外两类按工具下发的资源做同样的事，并且都向 handler 询问落点，而不是自行拼路径：rule 的文件名和内容因工具而异（`.md` 原样、`.mdc` 带派生的 `globs`/`alwaysApply`、`.instructions.md` 带 `applyTo`），agent 的落点来自渲染结果，且由 `targets:` 决定哪些工具应当收到。已送达的 rule 会与 handler 为该工具渲染出的字节逐一比对，而不只是检查该工具所需的键是否存在：`globs` 与团队 rule 的 `paths:` 不再一致的 `.mdc`，即使 `alwaysApply` 取值合法，也会作用到错误的文件上；这里会报告为 `delivered from an older copy`——正文漂移的副本同样如此，因为两者都写入成功，却都是错的。agent 会与渲染结果逐字节比对：旧版 spec 留下的副本（普通 pull 会跳过团队仓库未变化的 scope，它可能一直留在那里）报告为 `delivered from an older spec`，而不是当作已送达。`Every team agent reaches a tool` 会指出在任何已安装工具上都无法渲染的 agent，通常是 spec 解析失败，或 `targets:` 只列了本机没有的工具。这两项仅在 `doctor` 中运行：它们会按工具读取每条 rule、解析每个 agent，放进 pull 结束时的检查会耗尽其时间预算。 -有两个工具并不读取 rules 目录，按文件比对的检查无法代表它们，因此各自单列一项。`Team rules are active in opencode` 检查 `opencode.json` 的 `instructions` 中是否仍列着 teamai 所拥有的那条 glob：OpenCode 不会自动扫描 `.opencode/rules`，缺了它，已送达的每个 `.md` 都不会生效，而按文件比对的检查依旧通过。`Team rules are inlined in Hermes SOUL.md` 把 `SOUL.md` 中 teamai 管理的代码块与团队 rule 内联后的内容比对——Hermes 的常驻指令来自这一个文件而非某个目录，因此代码块被删除或停留在旧版规则集上，都意味着该工具读到的是错误的规则，而磁盘上看不出任何异常。 +有三个工具并不读取 rules 目录，按文件比对的检查无法代表它们，因此各自单列一项。`Team rules are active in opencode` 检查 `opencode.json` 的 `instructions` 中是否仍列着 teamai 所拥有的那条 glob：OpenCode 不会自动扫描 `.opencode/rules`，缺了它，已送达的每个 `.md` 都不会生效，而按文件比对的检查依旧通过。`Team rules are inlined in Hermes SOUL.md` 把 `SOUL.md` 中 teamai 管理的代码块与团队 rule 内联后的内容比对——Hermes 的常驻指令来自这一个文件而非某个目录，因此代码块被删除或停留在旧版规则集上，都意味着该工具读到的是错误的规则，而磁盘上看不出任何异常。user scope 下，`Team rules are inlined in Codex AGENTS.md` 把该工具 `AGENTS.md` 中的 team-rules 区块与团队 rule 内联后的内容比对，旁边有 `AGENTS.override.md` 遮蔽该文件时也会失败。在项目中，当该工具 `hooks.json` 中的 teamai `SessionStart` 或 `SubagentStart` 条目缺失或没有设置 `additionalContextLimit: 0` 时，`Project rules and instructions reach whole through its session hooks` 会失败：没有它，Codex 对较大的内容只保留开头和结尾。Codex 没有 `Rules delivered to ` 检查。 `MCP servers delivered to ` 将团队 `mcp.yaml` 为该工具解析出的每个 server 与该工具自己配置文件中的条目逐一比对，并列出 reconcile 跳过的 server 及原因。比对的是条目内容而非名字：reconcile 不会覆盖不属于 teamai 的条目，因此你自己写的同名 server 会占住这个名字，团队的定义从未真正送达；过期的旧副本同样等于没送达。两者都报告为 `not the team's definition`，而覆盖非 teamai 写入的条目只有 `teamai pull --force` 能做到。未解析的 `${VAR}` 会在这里连同变量名一起报告——否则它只在 pull 时出现一次，之后再无提示。没有值的已声明密钥不算失败：doctor 把它作为备注打印（`--json` 中的 `notes`），并附上设置它的命令，退出码与没有它时相同；备注还会说明为它保留的条目可能含有旧值，以及某个 key 既声明为密钥、又在 `env.yaml` 中设置的情况。无法解析的 `mcp.yaml` 并不等于团队没有 MCP：它会作为 `Team MCP servers can be read` 连同解析错误一起报告，因为这种文件不会向任何工具注入内容，而且除第一次之外的每次运行都对此保持沉默。无法解析的团队 hooks 与团队模型配置（文件无法解析、同一文件内重复的名字，或两个活动 namespace 中的同名条目）会让 `Team hooks can be resolved` 与 `Team model profiles can be resolved` 失败，并给出 pull 只记录一次的原因；`teamai status` 把它们计为 0 时会指向这里。`Env variables injected in shell profile` 不再只查标记注释：它会检查 `env/env.yaml` 能否解析、以及是否在 `variables:` 键下声明了变量（写成普通的 `KEY: value` 映射等于没有声明；而显式写成 `variables: []` 属于没有内容要下发的配置，不会判为失败）、每个变量是否以 `env.yaml` 声明的值（或你为该团队设置的值；用 `--from-env` 设置的不会写入）写进了 `env.sh`（残留的旧值会一直被导出到每个 shell 和 MCP server，直到下次 pull；比对时会用生成器自身的逆运算读回 `env.sh`，因此跨多行引用的多行值能够正确匹配，而不会被误判为过期），以及本作用域注入的代码块（即 source 本作用域 `env.sh` 的那一块，因为同一个 profile 里还可能有其他作用域的代码块）是否真的能加载它——未加引号的 Windows 路径在 POSIX shell 中会被转义破坏，`source` 从不执行，而且没有任何提示。`No stale env blocks left behind` 是独立的一项检查：pull 优先选用哪个文件会随时间变化（Windows 上 Git Bash 的登录 shell 读取的是 `.bash_profile`/`.bash_login`/`.profile`，从不读取 `.bashrc`），而 pull 只会新增代码块，从不迁移旧的，因此早期安装或平台变化留下的失效代码块可能一直留在另一个候选文件里。它会列出每一个这样的文件（检查 `.zshrc`、`.bashrc`、`.bash_profile`、`.bash_login` 和 `.profile`，新旧写法都算），并指向 `teamai uninstall` 来清除它们——这与投递检查分开进行，因此不会因为还留着一个旧副本，就让一个正常工作的 env 代码块被判成故障。 @@ -2563,9 +2571,9 @@ teamai uninstall --agent claude 移除内容： - 如果 ownership 仍有效，先恢复 TeamAI 管理的模型配置 - AI 工具 settings 中的 teamai hooks -- CLAUDE.md 中的 teamai rules 块（保留用户自写内容） +- CLAUDE.md 和 AGENTS.md 中的 teamai 块（保留用户自写内容） - 团队同步的 skills，包括 OpenClaw workspace skills（保留用户自建 skills） -- 团队同步的 rules +- 团队同步的 rules，包括旧版本留在 `.codex/rules/` 中的副本，团队此后已删除的 rule 的副本也包括在内。其中你改过的副本会保留，并在警告中点名。已删除 rule 的副本只有与记录的投递哈希一致时才会删除；没有该记录时也会保留并点名。Codex 的 `*.rules` 文件保留 - 团队同步的自定义 agents 和 CLI 内置 agents（保留用户自建 agents） - Shell profile 中的 env 块——会清理每一个候选文件（`.zshrc`、`.bashrc`、`.bash_profile`、`.bash_login`、`.profile`）中、代码块指向本作用域自身 `env.sh` 的那些，而不仅仅是当前 `pull` 会选中的那一个；指向其他作用域 `env.sh` 的代码块不受影响 - `~/.teamai/` 目录 @@ -2574,6 +2582,8 @@ teamai uninstall --agent claude `--agent ` 只移除该工具的 teamai 资源（hooks、CLAUDE.md 块、skills、rules、团队同步的自定义 agents、内置 agents）。工具名即 `toolPaths` 的键（如 `claude`、`codex`、`codebuddy`），匹配大小写不敏感。传入未知工具名会直接报错并列出可用工具、不执行任何删除，并以非零状态码退出。 +多个工具共同映射的指令文件按区块清理：只要该文件上仍有剩余工具会写入某个 teamai 区块，该区块就保留。最常见的是项目 `AGENTS.md`：Pi 仍启用时，`--agent workbuddy` 会移除 recall 区块，保留 Pi 写入的 culture 和共享指令区块。Codex 系工具不会在那里写入任何内容。teamai 创建的文件随最后一个区块一起删除；你原有的指令文件（即使是空文件）会保留。 + 跨工具共享资源（shell profile env 块、docs 目录、`~/.teamai/`）**仅当该工具自身存在 teamai 资源、且它是最后一个仍在使用 teamai 的工具时**才一并移除，否则会为其余工具保留。（因此，定向卸载一个自身没有任何 teamai 资源的工具是 no-op，即便它恰好是唯一的工具，也不会删除共享资源。） 该排除是持久的：`uninstall --agent ` 会把该工具从 `enabledAgents` 移除并记入 `disabledAgents`，因此之后的 `pull`（或其他工具的 session-start hook）不会再把它的 skills、rules、agents、CLAUDE.md 块或 hooks 重新装回。重新执行 `init --agent ` 会清除该排除、恢复对该工具的同步。 diff --git a/skill-data/setup/references/uninstall.md b/skill-data/setup/references/uninstall.md index 069f637bb..2f3beaa6c 100644 --- a/skill-data/setup/references/uninstall.md +++ b/skill-data/setup/references/uninstall.md @@ -14,7 +14,11 @@ machine** (all tools)?"* - **Just this tool** → `--agent ` (use the tool this conversation runs in, e.g. `claude`). Shared resources are removed only if it is the last tool using - them. + them. An instructions file several tools read (the project `AGENTS.md` of Pi, + Hermes and WorkBuddy) is cleaned block by block: a teamai block stays while + a remaining tool on that file still writes it, so `--agent workbuddy` with Pi + still enabled removes the recall block and keeps the rest. A file teamai + created goes with its last block; one the user had before stays, even if empty. - **Whole machine** → no `--agent` flag. Reassure them (in their language): *"This only removes things from your computer. @@ -51,6 +55,10 @@ and give it your team repo URL."* ## Notes +- Uninstall keeps edited legacy Codex rule copies. For a rule the team has + removed, it deletes the copy only if its hash matches the recorded delivery. + Without that record, it keeps the copy and names it in a warning. Save any + changes you need, then delete the copy manually. - Do **not** delete the team repo on the Git platform — uninstall never touches it, and neither should you. - If the user only wants to stop auto-sync for one tool but keep TeamAI otherwise, diff --git a/src/__tests__/builtin-hooks.test.ts b/src/__tests__/builtin-hooks.test.ts index 2a84f5ca8..2b45fba19 100644 --- a/src/__tests__/builtin-hooks.test.ts +++ b/src/__tests__/builtin-hooks.test.ts @@ -27,6 +27,26 @@ describe('builtinHookDefs — unified built-in hook model', () => { for (const tool of ['cursor', 'copilot', 'zcode', 'workbuddy']) expect(subagentStop(tool)).toEqual([]); }); + it('lets the Codex family\'s session-start hook pass the team rules whole (#938)', () => { + // Codex keeps only the start and end of additionalContext past 2,500 tokens; 0 turns that off. + const sessionStart = (tool: string) => builtinHookDefs(tool).find((d) => d.event === 'SessionStart'); + for (const tool of ['codex', 'codex-internal', 'tcodex']) expect(sessionStart(tool)?.additionalContextLimit).toBe(0); + for (const tool of ['claude', 'cursor', 'copilot', 'codebuddy']) expect(sessionStart(tool)?.additionalContextLimit).toBeUndefined(); + }); + + it('adds SubagentStart, with the same limit, only for the Codex family, whose fresh subagents fire no SessionStart (#938)', () => { + const subagentStart = (tool: string) => builtinHookDefs(tool).filter((d) => d.event === 'SubagentStart'); + for (const tool of ['codex', 'codex-internal', 'tcodex']) { + expect(subagentStart(tool)).toEqual([expect.objectContaining({ + key: 'Hook dispatch subagent-start', + matcher: '*', + command: expect.stringContaining(`hook-dispatch subagent-start --tool ${tool}`), + additionalContextLimit: 0, + })]); + } + for (const tool of ['claude', 'cursor', 'copilot', 'codebuddy', 'qoder', 'zcode']) expect(subagentStart(tool)).toEqual([]); + }); + it('adds a lifecycle-complete SessionEnd hook only for Copilot', () => { const defs = builtinHookDefs('copilot'); expect(defs).toHaveLength(7); diff --git a/src/__tests__/claudemd.test.ts b/src/__tests__/claudemd.test.ts index 3a9ffc77e..a30a8cb36 100644 --- a/src/__tests__/claudemd.test.ts +++ b/src/__tests__/claudemd.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import path from 'node:path'; import os from 'node:os'; import fse from 'fs-extra'; -import { injectClaudeMdSection } from '../utils/claudemd.js'; +import { injectClaudeMdSection, removeClaudeMdSection } from '../utils/claudemd.js'; const START = ''; const END = ''; @@ -75,3 +75,54 @@ describe('injectClaudeMdSection', () => { expect(result).toContain(END); }); }); + +describe('removeClaudeMdSection', () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-claudemd-')); + }); + + afterEach(async () => { + await fse.remove(tmpDir); + }); + + it('with deleteIfEmpty, deletes a file injectClaudeMdSection created for the section', async () => { + const filePath = path.join(tmpDir, 'AGENTS.md'); + await injectClaudeMdSection(filePath, START, END, makeBlock('x')); + + expect(await removeClaudeMdSection(filePath, START, END, { deleteIfEmpty: true })).toBe(true); + + expect(await fse.pathExists(filePath)).toBe(false); + }); + + it('with deleteIfEmpty, deletes a file injectClaudeMdSection created once its sections go one by one', async () => { + const filePath = path.join(tmpDir, 'AGENTS.md'); + await injectClaudeMdSection(filePath, START, END, makeBlock('x')); + await injectClaudeMdSection(filePath, '', '', '\ny\n'); + + await removeClaudeMdSection(filePath, START, END, { deleteIfEmpty: true }); + await removeClaudeMdSection(filePath, '', '', { deleteIfEmpty: true }); + + expect(await fse.pathExists(filePath)).toBe(false); + }); + + it('with deleteIfEmpty, gives back a member\'s empty file the section was appended to', async () => { + const filePath = path.join(tmpDir, 'AGENTS.md'); + await fse.writeFile(filePath, ''); + await injectClaudeMdSection(filePath, START, END, makeBlock('x')); + + expect(await removeClaudeMdSection(filePath, START, END, { deleteIfEmpty: true })).toBe(true); + + expect(await fse.readFile(filePath, 'utf8')).toBe(''); + }); + + it('with deleteIfEmpty, leaves an empty file that has no section', async () => { + const filePath = path.join(tmpDir, 'AGENTS.md'); + await fse.writeFile(filePath, '\n'); + + expect(await removeClaudeMdSection(filePath, START, END, { deleteIfEmpty: true })).toBe(false); + + expect(await fse.readFile(filePath, 'utf8')).toBe('\n'); + }); +}); diff --git a/src/__tests__/codex-hook-rules.test.ts b/src/__tests__/codex-hook-rules.test.ts new file mode 100644 index 000000000..5d6a49a43 --- /dev/null +++ b/src/__tests__/codex-hook-rules.test.ts @@ -0,0 +1,192 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import path from 'node:path'; +import os from 'node:os'; +import fse from 'fs-extra'; + +import { buildHandlerRegistry, filterHandlersForConfig } from '../hook-handlers.js'; +import type { LocalConfig } from '../types.js'; + +const CODEX_FAMILY = ['codex', 'codex-internal', 'tcodex']; + +describe('Codex gets the project\'s rules and instruction blocks from its session-start hook (#938, #945)', () => { + let tmpDir: string; + let repoPath: string; + let localConfig: LocalConfig; + + const registration = (event = 'session-start') => + buildHandlerRegistry().find((r) => r.event === event && r.handler.name === 'team-rules'); + const context = async (stdin: Record, tool = 'codex'): Promise => { + const event = stdin.hook_event_name === 'SubagentStart' ? 'subagent-start' : 'session-start'; + const output = await registration(event)!.handler.execute(stdin, tool, localConfig); + if (output === null) return null; + const parsed = JSON.parse(output) as { hookSpecificOutput: { hookEventName: string; additionalContext: string } }; + // Codex rejects a hookEventName that is not the event it ran. + expect(parsed.hookSpecificOutput.hookEventName).toBe(stdin.hook_event_name); + return parsed.hookSpecificOutput.additionalContext; + }; + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-codex-hook-rules-')); + // No user scope unless a test writes ~/.teamai/config.yaml. + vi.stubEnv('HOME', path.join(tmpDir, 'home')); + repoPath = path.join(tmpDir, 'team-repo'); + await fse.outputFile(path.join(repoPath, 'teamai.yaml'), 'team: test\nrepo: https://example.invalid/x/team.git\n'); + await fse.outputFile(path.join(repoPath, 'rules', 'codeword.md'), 'The team codeword is PELICAN-42.\n'); + await fse.outputFile(path.join(repoPath, 'rules', 'scoped.md'), '---\npaths:\n - "src/**"\n---\nPrefer named exports.\n'); + localConfig = { + repo: { localPath: repoPath, remote: 'https://example.invalid/x/team.git' }, + username: 'u', + additionalRoles: [], + scope: 'project', + projectRoot: path.join(tmpDir, 'project'), + } as unknown as LocalConfig; + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + await fse.remove(tmpDir); + }); + + /** A user-scope teamai install whose team repo holds `rules`. */ + async function userScope(rules: Record, extra = ''): Promise { + const userRepo = path.join(tmpDir, 'user-team-repo'); + await fse.outputFile(path.join(userRepo, 'teamai.yaml'), 'team: personal\nrepo: https://example.invalid/x/personal.git\n'); + for (const [name, body] of Object.entries(rules)) await fse.outputFile(path.join(userRepo, 'rules', name), body); + await fse.outputFile( + path.join(tmpDir, 'home', '.teamai', 'config.yaml'), + `repo:\n localPath: ${userRepo}\n remote: https://example.invalid/x/personal.git\nusername: u\n${extra}`, + ); + } + + it.each(CODEX_FAMILY)('adds the team rules for %s at session start, a path-scoped rule after its globs', async (tool) => { + const text = await context({ hook_event_name: 'SessionStart', source: 'startup' }, tool); + + expect(text).toContain('The team codeword is PELICAN-42.'); + expect(text).toContain('Applies to files matching: src/**\nPrefer named exports.'); + expect(text).not.toContain('paths:'); + }); + + it('adds them again after a compaction or a clear, but not on resume, whose history already holds them', async () => { + for (const source of ['startup', 'clear', 'compact']) { + expect(await context({ hook_event_name: 'SessionStart', source })).toContain('PELICAN-42'); + } + expect(await context({ hook_event_name: 'SessionStart', source: 'resume' })).toBeNull(); + }); + + it('adds nothing from the user scope, which reaches Codex through its own AGENTS.md', async () => { + await userScope({ 'personal.md': 'The personal codeword is WREN-5.\n' }); + + const text = (await context({ hook_event_name: 'SessionStart', source: 'startup' }))!; + + expect(text).toContain('PELICAN-42'); + expect(text).not.toContain('WREN-5'); + }); + + it('adds nothing when the session starts outside a teamai project', async () => { + localConfig = { ...localConfig, scope: 'user', projectRoot: undefined } as unknown as LocalConfig; + + expect(await context({ hook_event_name: 'SessionStart', source: 'startup' })).toBeNull(); + }); + + it('adds the project\'s culture, shared instructions and recall, without their file markers', async () => { + await fse.outputFile(path.join(repoPath, 'culture.md'), '---\ncompany:\n name: Acme\n---\n\nBe kind to teammates.\n'); + await fse.outputFile(path.join(repoPath, 'claudemd', 'shared.md'), 'Shared team instructions.\n'); + localConfig = { ...localConfig, recallEnabled: true } as LocalConfig; + + const text = (await context({ hook_event_name: 'SessionStart', source: 'startup' }))!; + + expect(text).toContain('Be kind to teammates.'); + expect(text).toContain('Shared team instructions.'); + expect(text).toContain('Team Knowledge Recall (teamai)'); + expect(text).toContain('PELICAN-42'); + expect(text).not.toContain('\nBe kind to teammates.\n\n', + ); + + const text = (await context({ hook_event_name: 'SessionStart', source: 'startup' }))!; + + expect(text).not.toContain('Be kind to teammates.'); + expect(text).toContain('Shared team instructions.'); + }); + + it('adds no rules from a scope that does not enable the tool', async () => { + await userScope({ 'personal.md': 'The personal codeword is WREN-5.\n' }, 'enabledAgents: [claude]\n'); + localConfig = { ...localConfig, enabledAgents: ['claude'] } as LocalConfig; + + expect(await context({ hook_event_name: 'SessionStart', source: 'startup' })).toBeNull(); + }); + + it('adds them to a subagent Codex starts, as SubagentStart', async () => { + expect(await context({ hook_event_name: 'SubagentStart', agent_type: 'worker' })).toContain('PELICAN-42'); + expect(registration('subagent-start')).toMatchObject({ matcher: '*', requiresConfig: true }); + expect(registration('subagent-start')?.background).not.toBe(true); + }); + + it('adds nothing for a tool that reads its own rules directory', async () => { + expect(await context({ hook_event_name: 'SessionStart', source: 'startup' }, 'claude')).toBeNull(); + }); + + it('adds only the rules of the member\'s projects', async () => { + await fse.outputFile(path.join(repoPath, 'manifest', 'projects.yaml'), ` +version: 1 +projects: + - id: alpha + resources: { knowledge: [alpha] } + - id: billing + resources: { knowledge: [billing] } +`); + await fse.outputFile(path.join(repoPath, 'rules', 'alpha', 'alpha-rule.md'), 'Alpha rule.\n'); + await fse.outputFile(path.join(repoPath, 'rules', 'billing', 'billing-rule.md'), 'Billing rule.\n'); + localConfig = { ...localConfig, projects: ['alpha'] } as LocalConfig; + + const text = await context({ hook_event_name: 'SessionStart', source: 'startup' }); + + expect(text).toContain('Alpha rule.'); + expect(text).not.toContain('Billing rule.'); + }); + + it('adds nothing when no rule has a body', async () => { + await fse.remove(path.join(repoPath, 'rules')); + await fse.outputFile(path.join(repoPath, 'rules', 'empty.md'), '---\npaths:\n - "src/**"\n---\n'); + + expect(await context({ hook_event_name: 'SessionStart', source: 'startup' })).toBeNull(); + }); + + it('runs in the foreground at session start, only where teamai is set up', () => { + expect(registration()).toMatchObject({ event: 'session-start', matcher: '*', requiresConfig: true }); + expect(registration()?.background).not.toBe(true); + expect(filterHandlersForConfig(buildHandlerRegistry(), null).map((r) => r.handler.name)).not.toContain('team-rules'); + }); +}); diff --git a/src/__tests__/codex-instructions.test.ts b/src/__tests__/codex-instructions.test.ts new file mode 100644 index 000000000..d0b17c2b3 --- /dev/null +++ b/src/__tests__/codex-instructions.test.ts @@ -0,0 +1,645 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import path from 'node:path'; +import os from 'node:os'; +import fse from 'fs-extra'; + +vi.mock('../config.js', async (importOriginal) => ({ + ...(await importOriginal()), + requireInit: vi.fn(), + loadState: vi.fn().mockResolvedValue({ lastPull: null, lastPullRev: null }), + saveState: vi.fn(), + loadStateForScope: vi.fn(async () => ({})), + saveStateForScope: vi.fn(), + loadLocalConfigForScope: vi.fn(), + loadTeamConfig: vi.fn(), + detectProjectConfig: vi.fn().mockResolvedValue(null), + autoDetectInit: vi.fn(), +})); + +vi.mock('../utils/git.js', async (importOriginal) => ({ + ...(await importOriginal()), + pullRepo: vi.fn().mockResolvedValue('already up to date'), + getHeadRev: vi.fn().mockResolvedValue('abc1234'), + createGit: vi.fn(), +})); + +// pull() takes a real ~/.teamai/.sync-lock; parallel workers would race on it. +vi.mock('../update.js', () => ({ + acquireLock: vi.fn().mockResolvedValue(true), + releaseLock: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock('../utils/logger.js', () => ({ + log: { + persist: vi.fn(), + info: vi.fn(), + success: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + dim: vi.fn(), + }, + spinner: vi.fn(() => ({ + start: vi.fn().mockReturnThis(), + succeed: vi.fn().mockReturnThis(), + fail: vi.fn().mockReturnThis(), + warn: vi.fn().mockReturnThis(), + info: vi.fn().mockReturnThis(), + stop: vi.fn().mockReturnThis(), + })), +})); + +import { RulesHandler } from '../resources/rules.js'; +import { pull } from '../pull.js'; +import { log } from '../utils/logger.js'; +import { uninstall } from '../uninstall.js'; +import { autoDetectInit, detectProjectConfig, loadLocalConfigForScope, loadStateForScope, loadTeamConfig, saveStateForScope } from '../config.js'; +import { + TeamaiConfigSchema, + TEAMAI_RULES_START, + TEAMAI_RULES_END, + TEAMAI_CULTURE_START, + TEAMAI_TEAM_RULES_START, + TEAMAI_TEAM_RULES_END, + TEAMAI_CLAUDEMD_START, + TEAMAI_RECALL_RULES_START, +} from '../types.js'; +import type { LocalConfig, TeamaiConfig } from '../types.js'; + +const CODEX_FAMILY = ['codex', 'codex-internal', 'tcodex']; + +/** How many times `needle` occurs in `haystack`. */ +function count(haystack: string, needle: string): number { + return haystack.split(needle).length - 1; +} + +describe('a project-scope rules sync writes no team rules for Codex (#938)', () => { + let tmpDir: string; + let homeDir: string; + let projectRoot: string; + let repoPath: string; + let handler: RulesHandler; + // The built-in defaults: the bug lives in the default Codex tool paths. + let teamConfig: TeamaiConfig; + let localConfig: LocalConfig; + + const agentsMd = () => path.join(projectRoot, 'AGENTS.md'); + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-codex-rules-')); + homeDir = path.join(tmpDir, 'home'); + projectRoot = path.join(tmpDir, 'project'); + repoPath = path.join(tmpDir, 'team-repo'); + await fse.ensureDir(homeDir); + await fse.ensureDir(path.join(repoPath, 'rules')); + // Codex is installed for this project: opening a session creates its root. + await fse.ensureDir(path.join(projectRoot, '.codex')); + await fse.writeFile(path.join(repoPath, 'rules', 'codeword.md'), 'The team codeword is PELICAN-42.\n'); + vi.stubEnv('HOME', homeDir); + + handler = new RulesHandler(); + teamConfig = TeamaiConfigSchema.parse({ team: 'test', repo: 'https://example.invalid/x/team.git' }); + localConfig = { + repo: { localPath: repoPath, remote: 'https://example.invalid/x/team.git' }, + username: 'u', + additionalRoles: [], + scope: 'project', + projectRoot, + enabledAgents: ['codex'], + } as unknown as LocalConfig; + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + await fse.remove(tmpDir); + }); + + it.each(CODEX_FAMILY)('writes no team rules for %s into /AGENTS.md or its rules dir', async (tool) => { + // Its session-start hook adds them (codex-hook-rules.test.ts): other tools read this file too. + await fse.ensureDir(path.join(projectRoot, `.${tool}`)); + localConfig = { ...localConfig, enabledAgents: [tool] } as LocalConfig; + await fse.writeFile(agentsMd(), '# Project notes\n\nKeep this line.\n'); + + await handler.pullAllRules(teamConfig, localConfig); + + expect(await fse.readFile(agentsMd(), 'utf8')).toBe('# Project notes\n\nKeep this line.\n'); + expect(await fse.pathExists(path.join(projectRoot, `.${tool}`, 'rules'))).toBe(false); + }); + + it('keeps the rule files to the member\'s projects after `remove rules`', async () => { + await fse.outputFile(path.join(repoPath, 'manifest', 'projects.yaml'), ` +version: 1 +projects: + - id: alpha + resources: { knowledge: [alpha] } + - id: billing + resources: { knowledge: [billing] } +`); + await fse.outputFile(path.join(repoPath, 'rules', 'alpha', 'alpha-rule.md'), 'Alpha rule.\n'); + await fse.outputFile(path.join(repoPath, 'rules', 'billing', 'billing-rule.md'), 'Billing rule.\n'); + await fse.ensureDir(path.join(projectRoot, '.claude')); + localConfig = { ...localConfig, enabledAgents: ['claude'], projects: ['alpha'] } as LocalConfig; + + await handler.removeItem('codeword', teamConfig, localConfig); + + const rulesDir = path.join(projectRoot, '.claude', 'rules'); + expect(await fse.pathExists(path.join(rulesDir, 'alpha', 'alpha-rule.md'))).toBe(true); + expect(await fse.pathExists(path.join(rulesDir, 'billing'))).toBe(false); + expect(await fse.pathExists(path.join(rulesDir, 'codeword.md'))).toBe(false); + }); + + it('strips the legacy [teamai:rules] block from the AGENTS.md Codex shares with Pi, and adds no rules there', async () => { + await fse.ensureDir(path.join(projectRoot, '.pi')); + const shared = { ...localConfig, enabledAgents: ['codex', 'pi'] } as LocalConfig; + // A block an old release inlined into Pi's AGENTS.md, which pull strips. + await fse.writeFile(agentsMd(), `# Notes\n\n${TEAMAI_RULES_START}\nold rules\n${TEAMAI_RULES_END}\n`); + + await handler.pullAllRules(teamConfig, shared); + + expect(await fse.readFile(agentsMd(), 'utf8')).toBe('# Notes\n'); + }); + + it.each(CODEX_FAMILY)('creates no AGENTS.md when %s is not installed for the project', async (tool) => { + await fse.remove(path.join(projectRoot, '.codex')); + localConfig = { ...localConfig, enabledAgents: [tool] } as LocalConfig; + + await handler.pullAllRules(teamConfig, localConfig); + + expect(await fse.pathExists(agentsMd())).toBe(false); + expect(await fse.pathExists(path.join(homeDir, `.${tool}`))).toBe(false); + }); + + it.each([['empty', ''], ['whitespace-only', '\n \n']])('keeps a member\'s %s AGENTS.md in a Pi-only project, which holds no block to remove', async (_label, content) => { + await fse.ensureDir(path.join(projectRoot, '.pi')); + await fse.writeFile(agentsMd(), content); + + await handler.pullAllRules(teamConfig, { ...localConfig, enabledAgents: ['pi'] } as LocalConfig); + + expect(await fse.readFile(agentsMd(), 'utf8')).toBe(content); + }); +}); + +describe('pull on a machine without Codex (#938)', () => { + let tmpDir: string; + let homeDir: string; + let repoPath: string; + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-codex-absent-')); + homeDir = path.join(tmpDir, 'home'); + repoPath = path.join(tmpDir, 'team-repo'); + await fse.ensureDir(path.join(homeDir, '.claude', 'rules')); + await fse.ensureDir(path.join(repoPath, 'rules')); + await fse.writeFile(path.join(repoPath, 'rules', 'codeword.md'), 'The team codeword is PELICAN-42.\n'); + await fse.writeFile(path.join(repoPath, 'culture.md'), '---\ncompany:\n name: Acme\n---\n\nBe kind to teammates.\n'); + await fse.ensureDir(path.join(repoPath, 'claudemd')); + await fse.writeFile(path.join(repoPath, 'claudemd', 'shared.md'), 'Shared team instructions.\n'); + vi.stubEnv('HOME', homeDir); + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + await fse.remove(tmpDir); + }); + + it('creates no ~/.codex for culture, shared instructions or rules when enabledAgents is unset', async () => { + // A legacy config with no whitelist syncs every installed tool; Codex is not one. + vi.mocked(loadTeamConfig).mockResolvedValue( + TeamaiConfigSchema.parse({ team: 'test', repo: 'https://example.invalid/x/team.git' }), + ); + vi.mocked(loadLocalConfigForScope).mockResolvedValue({ + repo: { localPath: repoPath, remote: 'https://example.invalid/x/team.git' }, + username: 'u', + updatePolicy: 'auto', + additionalRoles: [], + scope: 'user', + } as LocalConfig); + + await pull({}); + + expect(await fse.readFile(path.join(homeDir, '.claude', 'CLAUDE.md'), 'utf8')).toContain('Shared team instructions.'); + expect(await fse.pathExists(path.join(homeDir, '.codex'))).toBe(false); + }); + + // A team entry replaces the default whole. With neither `rules` nor + // `settings`, only its `skills` path can say whether Codex is installed. + it.each([ + ['skills only', { skills: '.codex/skills' }], + ['skills and claudemd', { skills: '.codex/skills', claudemd: 'AGENTS.md' }], + ])('writes no AGENTS.md for a team codex entry with %s when the project has no .codex/', async (_label, codex) => { + const projectRoot = path.join(tmpDir, 'project'); + await fse.ensureDir(projectRoot); + vi.mocked(loadTeamConfig).mockResolvedValue( + TeamaiConfigSchema.parse({ team: 'test', repo: 'https://example.invalid/x/team.git', toolPaths: { codex } }), + ); + vi.mocked(detectProjectConfig).mockResolvedValue({ + repo: { localPath: repoPath, remote: 'https://example.invalid/x/team.git' }, + username: 'u', + updatePolicy: 'auto', + additionalRoles: [], + scope: 'project', + projectRoot, + recallEnabled: true, + } as LocalConfig); + + try { + await pull({}); + } finally { + vi.mocked(detectProjectConfig).mockResolvedValue(null); + } + + expect(await fse.pathExists(path.join(projectRoot, 'AGENTS.md'))).toBe(false); + expect(await fse.pathExists(path.join(projectRoot, '.codex'))).toBe(false); + }); +}); + +describe('a user-scope rules sync puts the team rules in Codex\'s own AGENTS.md (#938)', () => { + let tmpDir: string; + let homeDir: string; + let repoPath: string; + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-codex-user-')); + homeDir = path.join(tmpDir, 'home'); + repoPath = path.join(tmpDir, 'team-repo'); + await fse.ensureDir(path.join(repoPath, 'rules')); + await fse.writeFile(path.join(repoPath, 'rules', 'codeword.md'), 'The team codeword is PELICAN-42.\n'); + await fse.writeFile(path.join(repoPath, 'rules', 'scoped.md'), '---\npaths:\n - "src/**"\n---\nPrefer named exports.\n'); + vi.stubEnv('HOME', homeDir); + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + await fse.remove(tmpDir); + }); + + const teamConfig = () => TeamaiConfigSchema.parse({ team: 'test', repo: 'https://example.invalid/x/team.git' }); + const userConfig = (enabledAgents: string[]) => ({ + repo: { localPath: repoPath, remote: 'https://example.invalid/x/team.git' }, + username: 'u', + additionalRoles: [], + scope: 'user', + enabledAgents, + }) as unknown as LocalConfig; + const agentsMd = (tool: string) => path.join(homeDir, `.${tool}`, 'AGENTS.md'); + + it.each(CODEX_FAMILY)('writes a team-rules block for %s to ~/./AGENTS.md, beside the member\'s text, and nothing elsewhere', async (tool) => { + await fse.ensureDir(path.join(homeDir, `.${tool}`)); + await fse.writeFile(agentsMd(tool), '# My notes\n'); + + await new RulesHandler().pullAllRules(teamConfig(), userConfig([tool])); + + const content = await fse.readFile(agentsMd(tool), 'utf8'); + expect(content.startsWith('# My notes\n')).toBe(true); + expect(count(content, TEAMAI_TEAM_RULES_START)).toBe(1); + expect(content).toContain('The team codeword is PELICAN-42.'); + expect(content).toContain('Applies to files matching: src/**\nPrefer named exports.'); + expect(content).not.toContain('paths:'); + expect(await fse.pathExists(path.join(homeDir, 'AGENTS.md'))).toBe(false); + expect(await fse.pathExists(path.join(homeDir, `.${tool}`, 'rules'))).toBe(false); + }); + + it('removes the block, and a file that held only it, once the team has no rules', async () => { + await fse.ensureDir(path.join(homeDir, '.codex')); + await new RulesHandler().pullAllRules(teamConfig(), userConfig(['codex'])); + expect(await fse.pathExists(agentsMd('codex'))).toBe(true); + await fse.remove(path.join(repoPath, 'rules')); + + await new RulesHandler().pullAllRules(teamConfig(), userConfig(['codex']), []); + + expect(await fse.pathExists(agentsMd('codex'))).toBe(false); + }); + + it('removes the block once Codex is no longer enabled, and creates nothing when it is not installed', async () => { + await fse.ensureDir(path.join(homeDir, '.codex')); + await fse.writeFile(agentsMd('codex'), '# My notes\n'); + await new RulesHandler().pullAllRules(teamConfig(), userConfig(['codex'])); + + await new RulesHandler().pullAllRules(teamConfig(), userConfig(['claude'])); + + expect(await fse.readFile(agentsMd('codex'), 'utf8')).toBe('# My notes\n'); + await fse.remove(path.join(homeDir, '.codex')); + await new RulesHandler().pullAllRules(teamConfig(), userConfig(['codex'])); + expect(await fse.pathExists(path.join(homeDir, '.codex'))).toBe(false); + }); + + it('keeps the whole rule when its text mentions the block markers inline', async () => { + await fse.ensureDir(path.join(homeDir, '.codex')); + await fse.writeFile( + path.join(repoPath, 'rules', 'codeword.md'), + `Never edit ${TEAMAI_TEAM_RULES_START} or ${TEAMAI_TEAM_RULES_END} by hand.\nThe team codeword is PELICAN-42.\n`, + ); + + await new RulesHandler().pullAllRules(teamConfig(), userConfig(['codex'])); + await new RulesHandler().pullAllRules(teamConfig(), userConfig(['codex'])); + + const content = await fse.readFile(agentsMd('codex'), 'utf8'); + expect(count(content, TEAMAI_TEAM_RULES_START)).toBe(1); + expect(count(content, TEAMAI_TEAM_RULES_END)).toBe(1); + expect(content).toContain('by hand.\nThe team codeword is PELICAN-42.'); + }); +}); + +describe('instruction blocks reach Codex from its own file in user scope, and never through the project AGENTS.md (#938, #945)', () => { + let tmpDir: string; + let homeDir: string; + let projectRoot: string; + let repoPath: string; + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-codex-blocks-')); + homeDir = path.join(tmpDir, 'home'); + projectRoot = path.join(tmpDir, 'project'); + repoPath = path.join(tmpDir, 'team-repo'); + await fse.ensureDir(homeDir); + await fse.ensureDir(path.join(repoPath, 'rules')); + await fse.writeFile(path.join(repoPath, 'rules', 'codeword.md'), 'The team codeword is PELICAN-42.\n'); + await fse.writeFile(path.join(repoPath, 'culture.md'), '---\ncompany:\n name: Acme\n---\n\nBe kind to teammates.\n'); + await fse.ensureDir(path.join(repoPath, 'claudemd')); + await fse.writeFile(path.join(repoPath, 'claudemd', 'shared.md'), 'Shared team instructions.\n'); + vi.stubEnv('HOME', homeDir); + vi.mocked(loadTeamConfig).mockResolvedValue( + TeamaiConfigSchema.parse({ team: 'test', repo: 'https://example.invalid/x/team.git' }), + ); + }); + + afterEach(async () => { + vi.mocked(detectProjectConfig).mockResolvedValue(null); + vi.mocked(loadLocalConfigForScope).mockReset(); + vi.unstubAllEnvs(); + await fse.remove(tmpDir); + }); + + const config = (scope: 'project' | 'user', tool: string) => ({ + repo: { localPath: repoPath, remote: 'https://example.invalid/x/team.git' }, + username: 'u', + updatePolicy: 'auto', + additionalRoles: [], + scope, + ...(scope === 'project' ? { projectRoot } : {}), + enabledAgents: [tool], + recallEnabled: true, + }) as LocalConfig; + + it.each(CODEX_FAMILY)('a project-scope pull for %s leaves the project AGENTS.md byte for byte', async (tool) => { + await fse.ensureDir(path.join(projectRoot, `.${tool}`)); + const owners = '# Project notes\n\nKeep commits small.\n'; + await fse.writeFile(path.join(projectRoot, 'AGENTS.md'), owners); + vi.mocked(detectProjectConfig).mockResolvedValue(config('project', tool)); + + await pull({}); + + expect(await fse.readFile(path.join(projectRoot, 'AGENTS.md'), 'utf8')).toBe(owners); + }); + + it.each(CODEX_FAMILY)('a user-scope pull writes culture, shared instructions and recall into ~/.%s/AGENTS.md', async (tool) => { + await fse.ensureDir(path.join(homeDir, `.${tool}`)); + vi.mocked(loadLocalConfigForScope).mockImplementation(async (scope) => (scope === 'user' ? config('user', tool) : null) as never); + + await pull({}); + + const content = await fse.readFile(path.join(homeDir, `.${tool}`, 'AGENTS.md'), 'utf8'); + for (const marker of [TEAMAI_CULTURE_START, TEAMAI_CLAUDEMD_START, TEAMAI_RECALL_RULES_START]) { + expect(count(content, marker)).toBe(1); + } + expect(content).toContain('Be kind to teammates.'); + expect(content).toContain('Shared team instructions.'); + }); +}); + +describe('a pull at an unchanged team revision after a CLI upgrade (#938)', () => { + let tmpDir: string; + let homeDir: string; + let repoPath: string; + let saved: Record; + + const agentsMd = () => path.join(homeDir, '.codex', 'AGENTS.md'); + // What 0.22.0 delivered: the team rule verbatim, in a directory Codex never read. + const legacyCopy = () => path.join(homeDir, '.codex', 'rules', 'codeword.md'); + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-codex-upgrade-')); + homeDir = path.join(tmpDir, 'home'); + repoPath = path.join(tmpDir, 'team-repo'); + await fse.ensureDir(path.join(homeDir, '.codex')); + await fse.ensureDir(path.join(repoPath, 'rules')); + await fse.writeFile(path.join(repoPath, 'rules', 'codeword.md'), 'The team codeword is PELICAN-42.\n'); + vi.stubEnv('HOME', homeDir); + // The state persists between pulls, so the second one takes the fast path. + saved = {}; + vi.mocked(saveStateForScope).mockImplementation(async (state) => { + saved = structuredClone(state) as Record; + }); + vi.mocked(loadStateForScope).mockImplementation(async () => structuredClone(saved) as never); + vi.mocked(loadTeamConfig).mockResolvedValue( + TeamaiConfigSchema.parse({ team: 'test', repo: 'https://example.invalid/x/team.git' }), + ); + vi.mocked(loadLocalConfigForScope).mockResolvedValue({ + repo: { localPath: repoPath, remote: 'https://example.invalid/x/team.git' }, + username: 'u', + updatePolicy: 'auto', + additionalRoles: [], + scope: 'user', + enabledAgents: ['codex'], + } as LocalConfig); + await pull({}); + // What an older CLI left at this revision: no block, and the copy. + await fse.writeFile(agentsMd(), 'My own notes.\n'); + await fse.outputFile(legacyCopy(), 'The team codeword is PELICAN-42.\n'); + vi.mocked(log.success).mockClear(); + }); + + afterEach(async () => { + vi.mocked(saveStateForScope).mockReset(); + vi.mocked(loadStateForScope).mockImplementation(async () => ({}) as never); + vi.unstubAllEnvs(); + await fse.remove(tmpDir); + }); + + it('removes the old .codex/rules copy and writes the team-rules block into ~/.codex/AGENTS.md', async () => { + await pull({}); + + expect(vi.mocked(log.success).mock.calls.some(([message]) => String(message).includes('Already synced at abc1234'))).toBe(true); + const content = await fse.readFile(agentsMd(), 'utf8'); + expect(content.startsWith('My own notes.\n')).toBe(true); + expect(count(content, TEAMAI_TEAM_RULES_START)).toBe(1); + expect(content).toContain('PELICAN-42'); + expect(await fse.pathExists(legacyCopy())).toBe(false); + expect(await fse.pathExists(path.join(homeDir, '.codex', 'rules'))).toBe(false); + }); + + it('writes nothing on a dry run', async () => { + await pull({ dryRun: true }); + + expect(await fse.readFile(agentsMd(), 'utf8')).toBe('My own notes.\n'); + expect(await fse.pathExists(legacyCopy())).toBe(true); + }); +}); + +describe('a project-scope pull at an unchanged team revision after a CLI upgrade (#938)', () => { + let tmpDir: string; + let projectRoot: string; + let repoPath: string; + let saved: Record; + + const agentsMd = () => path.join(projectRoot, 'AGENTS.md'); + const legacyCopy = () => path.join(projectRoot, '.codex', 'rules', 'codeword.md'); + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-codex-upgrade-project-')); + projectRoot = path.join(tmpDir, 'project'); + repoPath = path.join(tmpDir, 'team-repo'); + await fse.ensureDir(path.join(tmpDir, 'home')); + await fse.ensureDir(path.join(projectRoot, '.codex')); + await fse.ensureDir(path.join(repoPath, 'rules')); + await fse.writeFile(path.join(repoPath, 'rules', 'codeword.md'), 'The team codeword is PELICAN-42.\n'); + vi.stubEnv('HOME', path.join(tmpDir, 'home')); + saved = {}; + vi.mocked(saveStateForScope).mockImplementation(async (state) => { + saved = structuredClone(state) as Record; + }); + vi.mocked(loadStateForScope).mockImplementation(async () => structuredClone(saved) as never); + vi.mocked(loadTeamConfig).mockResolvedValue( + TeamaiConfigSchema.parse({ team: 'test', repo: 'https://example.invalid/x/team.git' }), + ); + vi.mocked(detectProjectConfig).mockResolvedValue({ + repo: { localPath: repoPath, remote: 'https://example.invalid/x/team.git' }, + username: 'u', + updatePolicy: 'auto', + additionalRoles: [], + scope: 'project', + projectRoot, + enabledAgents: ['codex'], + } as LocalConfig); + await pull({}); + await fse.writeFile(agentsMd(), 'My own notes.\n'); + await fse.outputFile(legacyCopy(), 'The team codeword is PELICAN-42.\n'); + vi.mocked(log.success).mockClear(); + }); + + afterEach(async () => { + vi.mocked(detectProjectConfig).mockResolvedValue(null); + vi.mocked(saveStateForScope).mockReset(); + vi.mocked(loadStateForScope).mockImplementation(async () => ({}) as never); + vi.unstubAllEnvs(); + await fse.remove(tmpDir); + }); + + it('removes the old .codex/rules copy and writes no rules into /AGENTS.md', async () => { + await pull({}); + + expect(vi.mocked(log.success).mock.calls.some(([message]) => String(message).includes('Already synced at abc1234'))).toBe(true); + expect(await fse.readFile(agentsMd(), 'utf8')).toBe('My own notes.\n'); + expect(await fse.pathExists(legacyCopy())).toBe(false); + expect(await fse.pathExists(path.join(projectRoot, '.codex', 'rules'))).toBe(false); + }); +}); + +describe('Codex leaves the project AGENTS.md to the tools that write it (#938, #945)', () => { + const BLOCK_START: Record = { + culture: TEAMAI_CULTURE_START, + claudemd: TEAMAI_CLAUDEMD_START, + recall: TEAMAI_RECALL_RULES_START, + }; + const BLOCKS = Object.values(BLOCK_START); + const defaults = TeamaiConfigSchema.parse({ team: 'test', repo: 'https://example.invalid/x/team.git' }).toolPaths; + let tmpDir: string; + let projectRoot: string; + let repoPath: string; + + const agentsMd = () => path.join(projectRoot, 'AGENTS.md'); + const blocksIn = async () => { + const content = await fse.readFile(agentsMd(), 'utf8').catch(() => ''); + return BLOCKS.filter((marker) => content.includes(marker)); + }; + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-codex-agree-')); + projectRoot = path.join(tmpDir, 'project'); + repoPath = path.join(tmpDir, 'team-repo'); + await fse.ensureDir(path.join(tmpDir, 'home')); + await fse.ensureDir(path.join(projectRoot, '.codex')); + await fse.ensureDir(path.join(repoPath, 'rules')); + await fse.writeFile(path.join(repoPath, 'rules', 'codeword.md'), 'The team codeword is PELICAN-42.\n'); + await fse.writeFile(path.join(repoPath, 'culture.md'), '---\ncompany:\n name: Acme\n---\n\nBe kind to teammates.\n'); + await fse.ensureDir(path.join(repoPath, 'claudemd')); + await fse.writeFile(path.join(repoPath, 'claudemd', 'shared.md'), 'Shared team instructions.\n'); + vi.stubEnv('HOME', path.join(tmpDir, 'home')); + }); + + afterEach(async () => { + vi.mocked(detectProjectConfig).mockResolvedValue(null); + vi.unstubAllEnvs(); + await fse.remove(tmpDir); + }); + + it.each([ + ['pi', 'no agents', { skills: '.pi/skills', rules: '.pi/rules', claudemd: 'AGENTS.md' }, ['culture', 'claudemd']], + ['workbuddy', 'agents', { + skills: '.workbuddy/skills', rules: '.workbuddy/rules', settings: '.workbuddy/settings.json', + claudemd: 'AGENTS.md', agents: '.workbuddy/agents', + }, ['culture', 'claudemd', 'recall']], + ['tcodex', 'the default entry', defaults.tcodex, []], + ])('Codex adds nothing to the AGENTS.md %s (%s) writes, and uninstall --agent codex leaves it', async (tool, _label, entry, written) => { + const teamConfig = TeamaiConfigSchema.parse({ + team: 'test', repo: 'https://example.invalid/x/team.git', toolPaths: { codex: defaults.codex, [tool]: entry }, + }); + await fse.ensureDir(path.join(projectRoot, `.${tool}`)); + const localConfig = (enabledAgents: string[]) => ({ + repo: { localPath: repoPath, remote: 'https://example.invalid/x/team.git' }, + username: 'u', + updatePolicy: 'auto', + additionalRoles: [], + scope: 'project', + projectRoot, + enabledAgents, + recallEnabled: true, + }) as LocalConfig; + const pullAs = async (enabledAgents: string[]) => { + vi.mocked(loadTeamConfig).mockResolvedValue(teamConfig); + vi.mocked(detectProjectConfig).mockResolvedValue(localConfig(enabledAgents)); + await pull({}); + }; + + await pullAs([tool]); + expect(await blocksIn()).toEqual(written.map((block) => BLOCK_START[block])); + const own = await fse.readFile(agentsMd(), 'utf8').catch(() => null); + await pullAs(['codex', tool]); + expect(await fse.readFile(agentsMd(), 'utf8').catch(() => null)).toBe(own); + vi.mocked(autoDetectInit).mockResolvedValue({ localConfig: localConfig(['codex', tool]), teamConfig } as never); + + await uninstall({ force: true, agent: 'codex' }); + + expect(await fse.readFile(agentsMd(), 'utf8').catch(() => null)).toBe(own); + }); + + it.each([ + ['keeps a member\'s empty AGENTS.md as it was', ''], + ['creates no AGENTS.md', null], + ])('a Codex-only project: pull and uninstall --agent codex %s', async (_label, before) => { + const teamConfig = TeamaiConfigSchema.parse({ team: 'test', repo: 'https://example.invalid/x/team.git' }); + if (before !== null) await fse.writeFile(agentsMd(), before); + const localConfig = { + repo: { localPath: repoPath, remote: 'https://example.invalid/x/team.git' }, + username: 'u', + updatePolicy: 'auto', + additionalRoles: [], + scope: 'project', + projectRoot, + enabledAgents: ['codex'], + recallEnabled: true, + } as LocalConfig; + vi.mocked(loadTeamConfig).mockResolvedValue(teamConfig); + vi.mocked(detectProjectConfig).mockResolvedValue(localConfig); + const unchanged = async () => { + if (before === null) expect(await fse.pathExists(agentsMd())).toBe(false); + else expect(await fse.readFile(agentsMd(), 'utf8')).toBe(before); + }; + + await pull({}); + await unchanged(); + vi.mocked(autoDetectInit).mockResolvedValue({ localConfig, teamConfig } as never); + await uninstall({ force: true, agent: 'codex' }); + await unchanged(); + }); +}); diff --git a/src/__tests__/codex-legacy-rules.test.ts b/src/__tests__/codex-legacy-rules.test.ts new file mode 100644 index 000000000..4012474aa --- /dev/null +++ b/src/__tests__/codex-legacy-rules.test.ts @@ -0,0 +1,294 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import path from 'node:path'; +import os from 'node:os'; +import crypto from 'node:crypto'; +import { execFileSync } from 'node:child_process'; +import fse from 'fs-extra'; + +vi.mock('../config.js', async (importOriginal) => ({ + ...(await importOriginal()), + loadStateForScope: vi.fn(async () => ({})), + saveStateForScope: vi.fn(), +})); + +vi.mock('../utils/logger.js', () => ({ + log: { + persist: vi.fn(), + info: vi.fn(), + success: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + dim: vi.fn(), + }, +})); + +import { RulesHandler } from '../resources/rules.js'; +import { deployBuiltinRules } from '../builtin-rules.js'; +import { openLedger, type DeliveredHashes } from '../resources/delivered-copies.js'; +import { loadStateForScope } from '../config.js'; +import { log } from '../utils/logger.js'; +import { TeamaiConfigSchema } from '../types.js'; +import type { LocalConfig, TeamaiConfig } from '../types.js'; + +const CODEX_FAMILY = ['codex', 'codex-internal', 'tcodex']; + +describe('pull reclaims the .codex/rules copies earlier pulls wrote (#938)', () => { + let tmpDir: string; + let homeDir: string; + let projectRoot: string; + let repoPath: string; + let handler: RulesHandler; + let teamConfig: TeamaiConfig; + let localConfig: LocalConfig; + + const legacyDir = () => path.join(projectRoot, '.codex', 'rules'); + const legacy = (file: string) => path.join(legacyDir(), file); + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-codex-legacy-')); + homeDir = path.join(tmpDir, 'home'); + projectRoot = path.join(tmpDir, 'project'); + repoPath = path.join(tmpDir, 'team-repo'); + await fse.ensureDir(homeDir); + await fse.ensureDir(path.join(repoPath, 'rules')); + await fse.ensureDir(path.join(projectRoot, '.codex')); + await fse.writeFile(path.join(repoPath, 'rules', 'codeword.md'), 'The team codeword is PELICAN-42.\n'); + vi.stubEnv('HOME', homeDir); + vi.clearAllMocks(); + vi.mocked(loadStateForScope).mockImplementation(async () => ({}) as never); + + handler = new RulesHandler(); + teamConfig = TeamaiConfigSchema.parse({ team: 'test', repo: 'https://example.invalid/x/team.git' }); + localConfig = { + repo: { localPath: repoPath, remote: 'https://example.invalid/x/team.git' }, + username: 'u', + additionalRoles: [], + scope: 'project', + projectRoot, + enabledAgents: ['codex'], + } as unknown as LocalConfig; + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + await fse.remove(tmpDir); + }); + + it.each(CODEX_FAMILY)('removes an unchanged %s rule copy, and the directory once nothing else is in it', async (tool) => { + localConfig = { ...localConfig, enabledAgents: [tool] } as LocalConfig; + const dir = path.join(projectRoot, `.${tool}`, 'rules'); + await fse.ensureDir(dir); + await fse.writeFile(path.join(dir, 'codeword.md'), 'The team codeword is PELICAN-42.\n'); + + await handler.pullAllRules(teamConfig, localConfig); + + expect(await fse.pathExists(path.join(dir, 'codeword.md'))).toBe(false); + expect(await fse.pathExists(dir)).toBe(false); + }); + + /** The recall rule as a pull before #938 deployed it to `.codex/rules`. */ + async function deployLegacyRecallRule(): Promise { + await fse.ensureDir(legacyDir()); + const legacyConfig = { + ...teamConfig, + toolPaths: { ...teamConfig.toolPaths, codex: { ...teamConfig.toolPaths.codex, rules: '.codex/rules' } }, + }; + await deployBuiltinRules(legacyConfig, localConfig); + return fse.readFile(legacy('teamai-recall.md'), 'utf8'); + } + + it('removes the built-in teamai-recall.md as teamai deployed it', async () => { + await deployLegacyRecallRule(); + + await handler.pullAllRules(teamConfig, localConfig); + + expect(await fse.pathExists(legacy('teamai-recall.md'))).toBe(false); + expect(await fse.pathExists(legacyDir())).toBe(false); + }); + + it('keeps a teamai-recall.md the member edited after the last pull that wrote it', async () => { + const deployed = await deployLegacyRecallRule(); + const edited = `${deployed}\nAlso read docs/onboarding.md first.\n`; + await fse.writeFile(legacy('teamai-recall.md'), edited); + + await handler.pullAllRules(teamConfig, localConfig); + + expect(await fse.readFile(legacy('teamai-recall.md'), 'utf8')).toBe(edited); + const warnings = vi.mocked(log.warn).mock.calls.map(([message]) => String(message)); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain(legacy('teamai-recall.md')); + }); + + it.each(CODEX_FAMILY)('keeps an edited %s copy and names it once in an English warning with how to remove it', async (tool) => { + localConfig = { ...localConfig, enabledAgents: [tool] } as LocalConfig; + const copy = (file: string) => path.join(projectRoot, `.${tool}`, 'rules', file); + await fse.writeFile(path.join(repoPath, 'rules', 'style.md'), 'Use tabs.\n'); + await fse.ensureDir(path.dirname(copy('x'))); + await fse.writeFile(copy('codeword.md'), 'The team codeword is PELICAN-42.\nMy own note.\n'); + await fse.writeFile(copy('style.md'), 'Use spaces.\n'); + + await handler.pullAllRules(teamConfig, localConfig); + + expect(await fse.readFile(copy('codeword.md'), 'utf8')).toBe('The team codeword is PELICAN-42.\nMy own note.\n'); + expect(await fse.readFile(copy('style.md'), 'utf8')).toBe('Use spaces.\n'); + const warnings = vi.mocked(log.warn).mock.calls.map(([message]) => String(message)); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain(copy('codeword.md')); + expect(warnings[0]).toContain(copy('style.md')); + expect(warnings[0]).toContain('AGENTS.md'); + expect(warnings[0]).toMatch(/[Dd]elete/); + }); + + it('never touches Codex exec-policy files or files that are not team rules', async () => { + await fse.ensureDir(legacyDir()); + await fse.writeFile(legacy('codeword.md'), 'The team codeword is PELICAN-42.\n'); + await fse.writeFile(legacy('default.rules'), 'prefix_rule(pattern = ["git", "status"], decision = "allow")\n'); + // Same bytes as a team rule, but not a team rule's name. + await fse.writeFile(legacy('personal.md'), 'The team codeword is PELICAN-42.\n'); + + await handler.pullAllRules(teamConfig, localConfig); + + expect(await fse.pathExists(legacy('codeword.md'))).toBe(false); + expect(await fse.readFile(legacy('default.rules'), 'utf8')).toContain('prefix_rule'); + expect(await fse.pathExists(legacy('personal.md'))).toBe(true); + expect(log.warn).not.toHaveBeenCalled(); + }); + + it('removes a copy of the rule as it was at the revision this checkout last pulled', async () => { + const run = (args: string[]) => execFileSync('git', args, { cwd: repoPath, encoding: 'utf8', env: { + ...process.env, GIT_AUTHOR_NAME: 'T', GIT_AUTHOR_EMAIL: 't@t', GIT_COMMITTER_NAME: 'T', GIT_COMMITTER_EMAIL: 't@t', + } }); + run(['init', '-q', '-b', 'main']); + run(['add', '-A']); + run(['commit', '-q', '-m', 'rule as delivered']); + const deliveredRev = run(['rev-parse', 'HEAD']).trim(); + await fse.writeFile(path.join(repoPath, 'rules', 'codeword.md'), 'The team codeword is HERON-7.\n'); + run(['commit', '-q', '-am', 'rule changed since']); + vi.mocked(loadStateForScope).mockImplementation(async () => ({ lastPullRev: deliveredRev }) as never); + await fse.ensureDir(legacyDir()); + await fse.writeFile(legacy('codeword.md'), 'The team codeword is PELICAN-42.\n'); + + await handler.pullAllRules(teamConfig, localConfig); + + expect(await fse.pathExists(legacy('codeword.md'))).toBe(false); + expect(log.warn).not.toHaveBeenCalled(); + }); + + it('removes a copy the delivery ledger recorded, and forgets the record', async () => { + await fse.ensureDir(legacyDir()); + const file = legacy('codeword.md'); + // Written by a build that delivered a since-reverted edit of the rule. + await fse.writeFile(file, 'The team codeword was PELICAN-41.\n'); + const hash = crypto.createHash('sha256').update('The team codeword was PELICAN-41.\n').digest('hex'); + const ledger = openLedger({ [file]: hash }); + + await handler.pullAllRules(teamConfig, localConfig, undefined, [], ledger); + + expect(await fse.pathExists(file)).toBe(false); + expect(ledger.hashes[file]).toBeUndefined(); + }); + + it.each([undefined, {}, { unrelated: 'recorded-hash' }])('keeps a removed rule copy without a recorded delivery hash (%j)', async (previous) => { + await fse.writeFile(path.join(repoPath, 'rules', '.removed'), 'retired\n'); + await fse.ensureDir(legacyDir()); + const file = legacy('retired.md'); + const edited = 'A rule the team retired.\nMy own note.\n'; + await fse.writeFile(file, edited); + + await handler.pullAllRules(teamConfig, localConfig, undefined, [], openLedger(previous)); + + expect(await fse.readFile(file, 'utf8')).toBe(edited); + const warnings = vi.mocked(log.warn).mock.calls.map(([message]) => String(message)); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain(file); + expect(warnings[0]).toContain('could not verify'); + }); + + it('removes the recorded, unchanged copy of a rule the team has since removed', async () => { + await fse.writeFile(path.join(repoPath, 'rules', '.removed'), 'retired\n'); + await fse.ensureDir(legacyDir()); + await fse.writeFile(legacy('retired.md'), 'A rule the team retired.\n'); + await fse.writeFile(legacy('codeword.md'), 'The team codeword is PELICAN-42.\n'); + const hash = crypto.createHash('sha256').update('A rule the team retired.\n').digest('hex'); + const ledger = openLedger({ [legacy('retired.md')]: hash }); + + await handler.pullAllRules(teamConfig, localConfig, undefined, [], ledger); + + expect(await fse.pathExists(legacy('retired.md'))).toBe(false); + expect(await fse.pathExists(legacyDir())).toBe(false); + expect(log.warn).not.toHaveBeenCalled(); + }); + + it('keeps and names the copy of a removed rule the member changed since teamai delivered it (#822)', async () => { + await fse.writeFile(path.join(repoPath, 'rules', '.removed'), 'retired\n'); + await fse.ensureDir(legacyDir()); + const file = legacy('retired.md'); + await fse.writeFile(file, 'A rule the team retired.\nMy own note.\n'); + const delivered = crypto.createHash('sha256').update('A rule the team retired.\n').digest('hex'); + const ledger = openLedger({ [file]: delivered }); + + await handler.pullAllRules(teamConfig, localConfig, undefined, [], ledger); + + expect(await fse.readFile(file, 'utf8')).toBe('A rule the team retired.\nMy own note.\n'); + const warnings = vi.mocked(log.warn).mock.calls.map(([message]) => String(message)); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain(file); + }); + + it('runs when no rule reaches this directory, and covers rules filtered out by roles or tags', async () => { + await fse.writeFile(path.join(repoPath, 'rules', 'backend-only.md'), 'Backend rule.\n'); + await fse.ensureDir(legacyDir()); + await fse.writeFile(legacy('codeword.md'), 'The team codeword is PELICAN-42.\n'); + await fse.writeFile(legacy('backend-only.md'), 'Backend rule.\n'); + + await handler.pullAllRules(teamConfig, localConfig, []); + + expect(await fse.pathExists(legacy('codeword.md'))).toBe(false); + expect(await fse.pathExists(legacy('backend-only.md'))).toBe(false); + expect(await fse.pathExists(legacyDir())).toBe(false); + }); + + it('also reclaims the copies of a Codex that is excluded or not installed', async () => { + // Excluded, and no hooks.json: nothing marks Codex as installed. + localConfig = { ...localConfig, enabledAgents: ['claude'] } as LocalConfig; + await fse.ensureDir(legacyDir()); + await fse.writeFile(legacy('codeword.md'), 'The team codeword is PELICAN-42.\n'); + + await handler.pullAllRules(teamConfig, localConfig); + + expect(await fse.pathExists(legacy('codeword.md'))).toBe(false); + }); + + it('leaves a directory the team still delivers rules to through its toolPaths', async () => { + teamConfig = { + ...teamConfig, + toolPaths: { ...teamConfig.toolPaths, codex: { ...teamConfig.toolPaths.codex, rules: '.codex/rules' } }, + }; + await fse.ensureDir(legacyDir()); + await fse.writeFile(legacy('codeword.md'), 'The team codeword is PELICAN-42.\n'); + // The built-in pull deploys there next to the team rules. + await fse.writeFile(legacy('teamai-recall.md'), '# Team Knowledge Recall (teamai)\n\nRecall first.\n'); + + await handler.pullAllRules(teamConfig, localConfig); + + expect(await fse.readFile(legacy('codeword.md'), 'utf8')).toBe('The team codeword is PELICAN-42.\n'); + expect(await fse.pathExists(legacy('teamai-recall.md'))).toBe(true); + }); + + it.each(['codex', 'codex-internal', 'tcodex'])('reclaims %s copies in user scope, and a second pull finds nothing to do', async (tool) => { + // The default entries: no Codex-family id delivers to a rules dir. + localConfig = { ...localConfig, scope: 'user', projectRoot: undefined } as unknown as LocalConfig; + const dir = path.join(homeDir, `.${tool}`, 'rules'); + await fse.ensureDir(dir); + await fse.writeFile(path.join(dir, 'codeword.md'), 'The team codeword is PELICAN-42.\n'); + await fse.writeFile(path.join(dir, 'default.rules'), 'prefix_rule(pattern = ["ls"], decision = "allow")\n'); + + await handler.pullAllRules(teamConfig, localConfig); + await handler.pullAllRules(teamConfig, localConfig); + + expect(await fse.pathExists(path.join(dir, 'codeword.md'))).toBe(false); + expect(await fse.pathExists(path.join(dir, 'default.rules'))).toBe(true); + expect(log.warn).not.toHaveBeenCalled(); + }); +}); diff --git a/src/__tests__/doctor-rules-delivery.test.ts b/src/__tests__/doctor-rules-delivery.test.ts index 9cbc8d07e..5bacade63 100644 --- a/src/__tests__/doctor-rules-delivery.test.ts +++ b/src/__tests__/doctor-rules-delivery.test.ts @@ -23,7 +23,7 @@ import crypto from 'node:crypto'; import { loadLocalConfig, loadStateForScope, loadTeamConfig } from '../config.js'; import { buildChecks, resolveDoctorContext, type Check } from '../doctor.js'; import { checkoutKey } from '../pull.js'; -import { StateSchema, type LocalConfig, type TeamaiConfig } from '../types.js'; +import { StateSchema, TeamaiConfigSchema, type LocalConfig, type TeamaiConfig } from '../types.js'; /** * The rules half of the delivery check (#624). A rule changes both its filename @@ -313,12 +313,208 @@ describe('doctor — rules delivered on disk', () => { expect(await (await namedCheck('Team rules are inlined in Hermes SOUL.md'))!.check()).toBe(true); }); + it('passes when the Hermes block holds a path-scoped rule the way pull inlines it (#938)', async () => { + await writeTeamRule('reviews', '---\npaths:\n - "src/**"\n---\n'); + const hermesHome = path.join(tempDir, 'hermes'); + await fse.ensureDir(hermesHome); + vi.stubEnv('HERMES_HOME', hermesHome); + await fse.writeFile( + path.join(hermesHome, 'SOUL.md'), + '\nBody of coding-style\n\n' + + 'Applies to files matching: src/**\nBody of reviews\n\n', + ); + + expect(await (await namedCheck('Team rules are inlined in Hermes SOUL.md'))!.check()).toBe(true); + }); + it('emits no Hermes check while Hermes is not installed here', async () => { vi.stubEnv('HERMES_HOME', path.join(tempDir, 'no-hermes')); expect(await namedCheck('Team rules are inlined in Hermes SOUL.md')).toBeUndefined(); }); + describe('Codex AGENTS.md, user scope (#938)', () => { + const CODEX = 'Team rules are inlined in Codex AGENTS.md'; + // What pull writes for the two team rules, markers included. + const CURRENT_BLOCK = '\n' + + '\n\n' + + 'Body of coding-style\n\nBody of reviews\n\n' + + ''; + const CODEX_FAMILY = ['codex', 'codex-internal', 'tcodex']; + let agentsMd: string; + + /** A Codex-family tool's default-shaped entry, installed in this (user) scope in place of Codex. */ + async function installCodex(tool = 'codex'): Promise { + delete teamConfig.toolPaths.codex; + teamConfig.toolPaths[tool] = { + skills: `.${tool}/skills`, + settings: `.${tool}/hooks.json`, + agents: `.${tool}/agents`, + userScope: { claudemd: `.${tool}/AGENTS.md` }, + }; + await fse.ensureDir(path.join(homeDir, `.${tool}`)); + agentsMd = path.join(homeDir, `.${tool}`, 'AGENTS.md'); + } + + const checkNameFor = (tool: string) => (tool === 'codex' ? CODEX : `${CODEX} (${tool})`); + + beforeEach(() => installCodex()); + + it('passes when AGENTS.md holds the block pull writes, beside the member\'s own text', async () => { + await fse.writeFile(agentsMd, `My own instructions\n\n${CURRENT_BLOCK}\n`); + + const check = await namedCheck(CODEX); + expect(check).toBeDefined(); + expect(await check!.check()).toBe(true); + }); + + it.each(CODEX_FAMILY)('fails for %s when AGENTS.md carries no block, naming the file and the fix', async (tool) => { + await installCodex(tool); + await fse.writeFile(agentsMd, 'My own instructions\n'); + + const check = (await namedCheck(checkNameFor(tool)))!; + expect(await check.check()).toBe(false); + expect(check.fix).toContain(agentsMd); + expect(check.fix).toContain('carries no team-rules block'); + // An already-synced pull restores the block too (#938). + expect(check.fix).toContain('Run `teamai pull` to restore it.'); + expect(check.fix).not.toContain('--force'); + }); + + it('passes without a block when every team rule is frontmatter only, since pull writes none', async () => { + for (const name of ['coding-style', 'reviews']) { + await fse.writeFile(path.join(repoPath, 'rules', `${name}.md`), '---\npaths:\n - "src/**"\n---\n'); + } + await fse.writeFile(agentsMd, 'My own instructions\n'); + + const check = (await namedCheck(CODEX))!; + expect(await check.check()).toBe(true); + }); + + it('fails when the team has no rules left but AGENTS.md still carries a block', async () => { + await fse.remove(path.join(repoPath, 'rules')); + await fse.writeFile(agentsMd, `My own instructions\n\n${CURRENT_BLOCK}\n`); + + const check = (await namedCheck(CODEX))!; + expect(check).toBeDefined(); + expect(await check.check()).toBe(false); + expect(check.fix).toContain(agentsMd); + }); + + it('adds no Codex check when the team has no rules and AGENTS.md carries no block', async () => { + await fse.remove(path.join(repoPath, 'rules')); + await fse.writeFile(agentsMd, 'My own instructions\n'); + + expect(await namedCheck(CODEX)).toBeUndefined(); + }); + + it.each(CODEX_FAMILY)('fails for %s when the block holds a stale rule set', async (tool) => { + await installCodex(tool); + await fse.writeFile(agentsMd, CURRENT_BLOCK.replace('\n\nBody of reviews', '') + '\n'); + + const check = (await namedCheck(checkNameFor(tool)))!; + expect(await check.check()).toBe(false); + expect(check.fix).toContain(agentsMd); + expect(check.fix).toContain('not what the team rules inline to'); + expect(check.fix).toContain('Run `teamai pull` to rewrite it.'); + expect(check.fix).not.toContain('--force'); + }); + + it.each(CODEX_FAMILY)('fails for %s on a current block when AGENTS.override.md sits beside it, which Codex reads instead', async (tool) => { + await installCodex(tool); + await fse.writeFile(agentsMd, `${CURRENT_BLOCK}\n`); + const override = path.join(homeDir, `.${tool}`, 'AGENTS.override.md'); + await fse.writeFile(override, 'Local override\n'); + + const check = (await namedCheck(checkNameFor(tool)))!; + expect(await check.check()).toBe(false); + expect(check.fix).toContain(override); + expect(check.fix).toContain('instead of'); + expect(check.fix).toContain('Move its content into AGENTS.md, or delete it'); + }); + + it.each(CODEX_FAMILY)('fails when a team toolPaths entry gives an installed %s no instructions file', async (tool) => { + await installCodex(tool); + // A team teamai.yaml written against the 0.22.0 defaults. + teamConfig.toolPaths[tool] = { skills: `.${tool}/skills`, rules: `.${tool}/rules`, settings: `.${tool}/hooks.json` }; + + const check = await namedCheck(checkNameFor(tool)); + expect(check).toBeDefined(); + expect(await check!.check()).toBe(false); + expect(check!.fix).toContain('has no `claudemd` path'); + // A project-scope `claudemd` would put the blocks back in the shared AGENTS.md (#945). + expect(check!.fix).not.toContain('`claudemd: AGENTS.md`'); + expect(check!.fix).toContain(`\`userScope.claudemd: .${tool}/AGENTS.md\``); + // A teamai.yaml edit moves the team repo, so a plain pull syncs it. + expect(check!.fix).toContain('then run `teamai pull`.'); + expect(check!.fix).not.toContain('--force'); + }); + + it('no longer reports rule files delivered to codex', async () => { + await fse.writeFile(agentsMd, `${CURRENT_BLOCK}\n`); + await deliverPlain(CLAUDE_RULES, 'coding-style'); + + const names = (await checks()).map((c) => c.name); + expect(names).toContain('Rules delivered to claude'); + expect(names).toContain(CODEX); + expect(names).not.toContain('Rules delivered to codex'); + }); + + it('asks nothing of a Codex that is not installed, and creates nothing', async () => { + await fse.remove(path.join(homeDir, '.codex')); + + expect(await namedCheck(CODEX)).toBeUndefined(); + expect(await fse.pathExists(path.join(homeDir, '.codex'))).toBe(false); + }); + + it('asks nothing of a Codex the member disabled', async () => { + localConfig.disabledAgents = ['codex']; + + expect(await namedCheck(CODEX)).toBeUndefined(); + }); + + it.each(['codex', 'codex-internal', 'tcodex'])('passes for the default %s entry when its AGENTS.md holds the block', async (tool) => { + teamConfig.toolPaths = { [tool]: TeamaiConfigSchema.parse({ team: 't', repo: 'owner/repo' }).toolPaths[tool] }; + await fse.ensureDir(path.join(homeDir, `.${tool}`)); + await fse.writeFile(path.join(homeDir, `.${tool}`, 'AGENTS.md'), `${CURRENT_BLOCK}\n`); + + const codex = (await checks()).filter((c) => c.name.startsWith(CODEX)); + expect(codex.map((c) => c.name)).toEqual([tool === 'codex' ? CODEX : `${CODEX} (${tool})`]); + expect(await codex[0].check()).toBe(true); + }); + + // A team entry replaces the default whole; `{ skills }` leaves its root + // as the only sign Codex is installed. + it('asks nothing of a team codex entry with only skills on a machine without .codex/', async () => { + await fse.remove(path.join(homeDir, '.codex')); + teamConfig.toolPaths.codex = { skills: '.codex/skills' }; + + expect(await namedCheck(CODEX)).toBeUndefined(); + expect(await fse.pathExists(path.join(homeDir, '.codex'))).toBe(false); + }); + + // An entry with neither `rules` nor `claudemd` delivers no rules to Codex + // on purpose, like any tool without a rules path: nothing to check. + it.each([ + ['only skills', { skills: '.codex/skills' }], + ['only agents', { agents: '.codex/agents' }], + ])('asks nothing of a team codex entry with %s once .codex/ exists, since it delivers no rules to Codex', async (_label, entry) => { + teamConfig.toolPaths.codex = entry; + + expect(await namedCheck(CODEX)).toBeUndefined(); + }); + + it('asks nothing of the project AGENTS.md, since the session-start hook gives Codex the project\'s rules', async () => { + const projectRoot = path.join(tempDir, 'project'); + await fse.ensureDir(path.join(projectRoot, '.codex')); + Object.assign(localConfig, { scope: 'project', projectRoot }); + teamConfig.toolPaths = { ...teamConfig.toolPaths, codex: TeamaiConfigSchema.parse({ team: 't', repo: 'owner/repo' }).toolPaths.codex }; + await fse.writeFile(path.join(projectRoot, 'AGENTS.md'), 'My own instructions\n'); + + expect((await checks()).filter((c) => c.name.startsWith(CODEX))).toEqual([]); + }); + }); + it('emits no check for a tool configured without a rules path', async () => { teamConfig.toolPaths = { claude: { rules: CLAUDE_RULES }, codex: { skills: '.codex/skills' } }; await deliverPlain(CLAUDE_RULES, 'coding-style'); diff --git a/src/__tests__/doctor.test.ts b/src/__tests__/doctor.test.ts index f82b8d775..9f926ef13 100644 --- a/src/__tests__/doctor.test.ts +++ b/src/__tests__/doctor.test.ts @@ -673,6 +673,77 @@ describe('buildChecks', () => { }); }); +// #938: Codex gets the team rules from its session-start hook. Past 2,500 +// tokens Codex keeps only the start and end of a hook's context unless the +// entry sets additionalContextLimit: 0. +describe('buildChecks — the Codex team rules hook (#938)', () => { + const NAME = 'Project rules and instructions reach codex whole through its session hooks'; + const DISPATCH = 'bash -lc "teamai hook-dispatch session-start --tool codex 2>/dev/null" || true'; + const SUBAGENT = 'bash -lc "teamai hook-dispatch subagent-start --tool codex 2>/dev/null" || true'; + // An entry for SubagentStart with the limit, unless a test passes its own. + const sessionStart = (entry: Record, subagent: Record | null = { command: SUBAGENT, additionalContextLimit: 0 }) => JSON.stringify({ + hooks: { + SessionStart: [{ hooks: [{ type: 'command', ...entry }] }], + ...(subagent ? { SubagentStart: [{ hooks: [{ type: 'command', ...subagent }] }] } : {}), + }, + }); + + async function codexCheck(hooksJson: string) { + mockedLoadLocalConfig.mockResolvedValue({ ...mockLocalConfig, enabledAgents: ['claude', 'codex'] }); + mockedLoadTeamConfig.mockResolvedValue({ + ...mockTeamConfig, + toolPaths: { ...mockTeamConfig.toolPaths, codex: { settings: '.codex/hooks.json', skills: '.codex/skills' } }, + }); + const fallback = mockedReadFileSafe.getMockImplementation()!; + mockedReadFileSafe.mockImplementation(async (filePath: string) => ( + filePath.endsWith(path.join('.codex', 'hooks.json')) ? hooksJson : fallback(filePath) + )); + const ctx = await resolveDoctorContext(); + if (!ctx) throw new Error('expected a resolved doctor context'); + return (await buildChecks(ctx)).find((c) => c.name === NAME); + } + + it('passes when the teamai session-start entry sets additionalContextLimit 0', async () => { + const check = await codexCheck(sessionStart({ command: DISPATCH, additionalContextLimit: 0 })); + + expect(await check!.check()).toBe(true); + }); + + it('fails on an entry an older pull wrote without the limit, naming the file, the fix and the approval', async () => { + const check = await codexCheck(sessionStart({ command: DISPATCH })); + + expect(await check!.check()).toBe(false); + expect(check!.fix).toContain(path.join('.codex', 'hooks.json')); + expect(check!.fix).toContain('additionalContextLimit'); + expect(check!.fix).toContain('teamai pull'); + expect(check!.fix).toContain('/hooks'); + }); + + it('leaves a missing entry to the hooks check', async () => { + const check = await codexCheck(sessionStart({ command: 'my-own-hook' })); + + expect(await check!.check()).toBe(true); + }); + + it.each([ + ['missing', null], + ['without the limit', { command: SUBAGENT }], + ])('fails when the SubagentStart entry is %s, since a fresh subagent fires no SessionStart', async (_label, subagent) => { + const check = await codexCheck(sessionStart({ command: DISPATCH, additionalContextLimit: 0 }, subagent)); + + expect(await check!.check()).toBe(false); + expect(check!.fix).toContain('SubagentStart'); + expect(check!.fix).toContain('Run `teamai pull`'); + }); + + it('asks nothing of a tool that reads its own rules directory', async () => { + await codexCheck(sessionStart({ command: DISPATCH, additionalContextLimit: 0 })); + const ctx = await resolveDoctorContext(); + + expect((await buildChecks(ctx!)).map((c) => c.name).filter((n) => n.startsWith('Project rules and instructions reach'))).toEqual([NAME]); + }); +}); + // A tool listed in enabledAgents is a claim by the user that they use it. Until // #598 the registry answered that claim with silence: buildHookChecks skipped // any tool whose settings directory was missing — the same silent skip #574 diff --git a/src/__tests__/hooks.test.ts b/src/__tests__/hooks.test.ts index 4ee2d7e89..1b9817768 100644 --- a/src/__tests__/hooks.test.ts +++ b/src/__tests__/hooks.test.ts @@ -105,12 +105,20 @@ describe('hooks', () => { const result = mockFiles['/test/codex-hooks.json'] as { hooks: Record }>> }; expect(result.hooks).toBeDefined(); - expect(Object.keys(result.hooks)).toEqual(['SessionStart', 'Stop', 'PostToolUse', 'UserPromptSubmit', 'SubagentStop']); + expect(Object.keys(result.hooks)).toEqual(['SessionStart', 'Stop', 'PostToolUse', 'UserPromptSubmit', 'SubagentStop', 'SubagentStart']); expect(result.hooks.PostToolUse).toHaveLength(3); expect(result.hooks.SessionStart[0].hooks[0].command).toContain('--tool codex'); expect(result.hooks.SessionStart[0].description).toBeUndefined(); }); + it('Codex format: writes additionalContextLimit 0 on the session-start entry only (#938)', async () => { + await injectHooks('/test/codex-hooks.json', 'codex'); + + const result = mockFiles['/test/codex-hooks.json'] as { hooks: Record }>> }; + expect(result.hooks.SessionStart[0].hooks[0].additionalContextLimit).toBe(0); + expect(result.hooks.Stop[0].hooks[0]).not.toHaveProperty('additionalContextLimit'); + }); + it('Claude uses PascalCase event names', async () => { await injectHooks('/test/settings.json', 'claude'); const result = mockFiles['/test/settings.json'] as { hooks: Record }; diff --git a/src/__tests__/local-agent.test.ts b/src/__tests__/local-agent.test.ts index ec6fce25e..70da0f394 100644 --- a/src/__tests__/local-agent.test.ts +++ b/src/__tests__/local-agent.test.ts @@ -5,6 +5,7 @@ import path from 'node:path'; import os from 'node:os'; import fse from 'fs-extra'; import { agentHookDescription } from '../hooks.js'; +import { TEAMAI_CLAUDEMD_START } from '../types.js'; vi.mock('../utils/logger.js', () => ({ log: { @@ -1684,6 +1685,36 @@ describe('local-agent: cmds[] migration', () => { expect(manifest.scopes.user.rules?.['doc-a']).toBeUndefined(); }); + // Codex's default `claudemd` (#938) makes a Codex report a target of this sync. + it('handle_type=prompt from Codex writes the prompt into ~/.codex/AGENTS.md when ~/.codex exists', async () => { + await fse.ensureDir(path.join(tmpDir, '.codex')); + + const acks = await runResponse({ + cmds: [{ + id: 4, type: 'install_prompt_rule', handle_type: 'prompt', slug: 'doc-codex', + version: '1.0.0', download_url: 'http://127.0.0.1:42100/doc-codex.md', scope: 'user', + }], + }, undefined, 'codex'); + + expect(acks.find((a) => a.id === 4)?.status).toBe('success'); + const content = await fse.readFile(path.join(tmpDir, '.codex', 'AGENTS.md'), 'utf8'); + expect(content).toContain(TEAMAI_CLAUDEMD_START); + expect(content).toContain('# content'); + }); + + it('handle_type=prompt from Codex creates no ~/.codex when Codex is not installed', async () => { + const acks = await runResponse({ + cmds: [{ + id: 5, type: 'install_prompt_rule', handle_type: 'prompt', slug: 'doc-codex', + version: '1.0.0', download_url: 'http://127.0.0.1:42100/doc-codex.md', scope: 'user', + }], + }, undefined, 'codex'); + + expect(await fse.pathExists(path.join(tmpDir, '.codex'))).toBe(false); + // The sync reached no tool, which the ack reports rather than claiming success. + expect(acks.find((a) => a.id === 5)?.status).toBe('failed'); + }); + it('syncs and removes prompts under custom COPILOT_HOME without replacing user content', async () => { const copilotHome = path.join(tmpDir, 'copilot-home'); const instructionPath = path.join(copilotHome, 'copilot-instructions.md'); diff --git a/src/__tests__/recall-toggle.test.ts b/src/__tests__/recall-toggle.test.ts index 93f2c5020..8beb33e9f 100644 --- a/src/__tests__/recall-toggle.test.ts +++ b/src/__tests__/recall-toggle.test.ts @@ -281,3 +281,65 @@ describe('recall toggle honors the enabledAgents whitelist', () => { expect(await fse.readFile(claudeMd, 'utf8')).toContain(TEAMAI_RECALL_RULES_START); }); }); + +// Codex reads instructions from AGENTS.md only (#938): the recall block has to +// land there, and `recall off` has to take it out again. +describe('recall toggle reaches Codex AGENTS.md with the default tool paths', () => { + let tmpDir: string; + let homeDir: string; + let projectRoot: string; + const teamConfig = TeamaiConfigSchema.parse({ team: 'test', repo: 'https://example.invalid/x/team.git' }); + + function stub(scope: 'user' | 'project'): void { + const localConfig = { + repo: { localPath: path.join(tmpDir, 'team-repo'), remote: 'https://example.invalid/x/team.git' }, + username: 'testuser', + updatePolicy: 'auto', + additionalRoles: [], + scope, + ...(scope === 'project' ? { projectRoot } : {}), + enabledAgents: ['codex'], + } as LocalConfig; + mockAutoDetectInit.mockResolvedValue({ localConfig, teamConfig }); + } + + beforeEach(async () => { + vi.clearAllMocks(); + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-recall-codex-')); + homeDir = path.join(tmpDir, 'home'); + projectRoot = path.join(tmpDir, 'project'); + await fse.ensureDir(homeDir); + vi.stubEnv('HOME', homeDir); + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + await fse.remove(tmpDir); + }); + + it('user scope: on writes the block to ~/.codex/AGENTS.md, off removes the file it created', async () => { + await fse.ensureDir(path.join(homeDir, '.codex')); + stub('user'); + const agentsMd = path.join(homeDir, '.codex', 'AGENTS.md'); + + await recallEnable({}); + expect(await fse.readFile(agentsMd, 'utf8')).toContain(TEAMAI_RECALL_RULES_START); + expect(await fse.pathExists(path.join(homeDir, '.codex', 'rules'))).toBe(false); + + await recallDisable({}); + expect(await fse.pathExists(agentsMd)).toBe(false); + }); + + it('project scope: on and off leave /AGENTS.md as it was, since the session-start hook adds recall there', async () => { + await fse.ensureDir(path.join(projectRoot, '.codex')); + const agentsMd = path.join(projectRoot, 'AGENTS.md'); + await fse.writeFile(agentsMd, '# Project notes\n'); + stub('project'); + + await recallEnable({}); + expect(await fse.readFile(agentsMd, 'utf8')).toBe('# Project notes\n'); + + await recallDisable({}); + expect(await fse.readFile(agentsMd, 'utf8')).toBe('# Project notes\n'); + }); +}); diff --git a/src/__tests__/rules.test.ts b/src/__tests__/rules.test.ts index bf671a388..6f7991a95 100644 --- a/src/__tests__/rules.test.ts +++ b/src/__tests__/rules.test.ts @@ -35,7 +35,7 @@ vi.mock('../utils/logger.js', () => ({ })), })); -import { RulesHandler } from '../resources/rules.js'; +import { RulesHandler, inlinedRulesText } 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'; @@ -1347,3 +1347,90 @@ describe('RulesHandler — Cursor-compatible .mdc handling', () => { expect(await fse.pathExists(path.join(homeDir, '.cursor/rules/gone.mdc'))).toBe(false); }); }); + +describe('inlinedRulesText — rules inlined into one instructions file (#938)', () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-rules-inline-')); + }); + + afterEach(async () => { + await fse.remove(tmpDir); + }); + + async function rule(name: string, content: string) { + const sourcePath = path.join(tmpDir, `${name}.md`); + await fse.writeFile(sourcePath, content); + return { name, type: 'rules' as const, sourcePath, relativePath: `rules/${name}.md` }; + } + + it('drops frontmatter and names the globs a path-scoped rule applies to', async () => { + const rules = [ + await rule('plain', 'Always write tests.\n'), + await rule('scoped', '---\npaths:\n - "src/**/*.ts"\n - "test/**"\n---\n\nUse strict types.\n'), + await rule('described', '---\ndescription: Reviews\n---\nReview every PR.\n'), + await rule('listed', '---\npaths: "docs/**, *.md"\n---\nKeep docs short.\n'), + ]; + + expect(await inlinedRulesText(rules)).toBe( + 'Always write tests.\n\n' + + 'Applies to files matching: src/**/*.ts, test/**\nUse strict types.\n\n' + + 'Review every PR.\n\n' + + 'Applies to files matching: docs/**, *.md\nKeep docs short.', + ); + }); + + it('skips a rule whose body is empty once its frontmatter is gone', async () => { + const rules = [ + await rule('only-frontmatter', '---\npaths:\n - "src/**"\n---\n\n'), + await rule('kept', 'Body\n'), + ]; + + expect(await inlinedRulesText(rules)).toBe('Body'); + }); +}); + +describe('RulesHandler.pullAllRules — Hermes SOUL.md (#938)', () => { + let tmpDir: string; + let hermesHome: string; + let localConfig: LocalConfig; + let teamConfig: TeamaiConfig; + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-rules-hermes-')); + hermesHome = path.join(tmpDir, 'hermes'); + await fse.ensureDir(hermesHome); + vi.stubEnv('HOME', path.join(tmpDir, 'home')); + vi.stubEnv('HERMES_HOME', hermesHome); + const repoPath = path.join(tmpDir, 'team-repo'); + await fse.ensureDir(path.join(repoPath, 'rules')); + teamConfig = { + team: 'test', description: '', repo: 'r', provider: 'tgit' as const, reviewers: [], + sharing: { skills: {}, rules: { enforced: [] }, docs: { localDir: '' }, env: { injectShellProfile: true } }, + toolPaths: { hermes: { skills: 'skills' } }, + } as unknown as TeamaiConfig; + localConfig = { + repo: { localPath: repoPath, remote: 'r' }, + username: 'u', additionalRoles: [], scope: 'user', + } as unknown as LocalConfig; + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + await fse.remove(tmpDir); + }); + + it('inlines a path-scoped rule without its frontmatter', async () => { + await fse.writeFile( + path.join(localConfig.repo.localPath, 'rules', 'scoped.md'), + '---\npaths:\n - "src/**/*.ts"\n---\n\nUse strict types.\n', + ); + + await new RulesHandler().pullAllRules(teamConfig, localConfig); + + const soul = await fse.readFile(path.join(hermesHome, 'SOUL.md'), 'utf-8'); + expect(soul).toContain('Applies to files matching: src/**/*.ts\nUse strict types.'); + expect(soul).not.toContain('paths:'); + }); +}); diff --git a/src/__tests__/tclaude-tcodex.test.ts b/src/__tests__/tclaude-tcodex.test.ts index b82b89aa1..81531a4cd 100644 --- a/src/__tests__/tclaude-tcodex.test.ts +++ b/src/__tests__/tclaude-tcodex.test.ts @@ -22,11 +22,12 @@ describe('tclaude/tcodex adapter integration', () => { it('tcodex has codex-compatible paths including hooks.json', async () => { const { TeamaiConfigSchema } = await import('../types.js'); const config = TeamaiConfigSchema.parse({ team: 'test', repo: 'test/repo' }); + // Same shape as codex: no rules dir, and an AGENTS.md in user scope only (#938, #945). expect(config.toolPaths.tcodex).toEqual({ skills: '.tcodex/skills', - rules: '.tcodex/rules', settings: '.tcodex/hooks.json', agents: '.tcodex/agents', + userScope: { claudemd: '.tcodex/AGENTS.md' }, }); }); }); diff --git a/src/__tests__/tool-roots.test.ts b/src/__tests__/tool-roots.test.ts index 56ed05344..31ebded2f 100644 --- a/src/__tests__/tool-roots.test.ts +++ b/src/__tests__/tool-roots.test.ts @@ -68,7 +68,8 @@ describe('toolRoots — re-rooting a relocated tool', () => { // Project scope is anchored on the project root, not on the member's root. mcpProject: '.mcp.json', }); - expect(paths.codex).toEqual(teamConfig.toolPaths.codex); + // Codex and Copilot have user-scope overrides, so compare their user-scope view. + expect(paths.codex).toEqual(scopedToolPaths(teamConfig, localConfig()).codex); expect(paths.tclaude).toEqual(teamConfig.toolPaths.tclaude); expect(paths.copilot).toEqual(scopedToolPaths(teamConfig, localConfig()).copilot); }); @@ -78,15 +79,17 @@ describe('toolRoots — re-rooting a relocated tool', () => { toolRoots: { codex: path.join(home, '.codex-alt') }, })); + // Codex reads $CODEX_HOME/AGENTS.md in user scope, so its instruction file moves too (#938). expect(paths.codex).toEqual({ skills: '.codex-alt/skills', - rules: '.codex-alt/rules', settings: '.codex-alt/hooks.json', + claudemd: '.codex-alt/AGENTS.md', agents: '.codex-alt/agents', mcp: '.codex-alt/config.toml', + userScope: { claudemd: '.codex-alt/AGENTS.md' }, }); expect(paths.claude).toEqual(teamConfig.toolPaths.claude); - expect(paths['codex-internal']).toEqual(teamConfig.toolPaths['codex-internal']); + expect(paths['codex-internal']).toEqual(scopedToolPaths(teamConfig, localConfig())['codex-internal']); }); it('expands a leading ~/ in the configured root', () => { diff --git a/src/__tests__/types.test.ts b/src/__tests__/types.test.ts index 093f9aa2f..52fb04230 100644 --- a/src/__tests__/types.test.ts +++ b/src/__tests__/types.test.ts @@ -139,11 +139,12 @@ describe('TeamaiConfigSchema', () => { repo: 'https://git.woa.com/test/repo.git', }); expect(result.toolPaths).toHaveProperty('codex-internal'); + // Same shape as codex: no rules dir, and an AGENTS.md in user scope only (#938, #945). expect(result.toolPaths['codex-internal']).toEqual({ skills: '.codex-internal/skills', - rules: '.codex-internal/rules', settings: '.codex-internal/hooks.json', agents: '.codex-internal/agents', + userScope: { claudemd: '.codex-internal/AGENTS.md' }, }); }); diff --git a/src/__tests__/uninstall.test.ts b/src/__tests__/uninstall.test.ts index dfff1eea8..e10d3d5db 100644 --- a/src/__tests__/uninstall.test.ts +++ b/src/__tests__/uninstall.test.ts @@ -48,7 +48,12 @@ vi.mock('../utils/logger.js', () => ({ })); import { uninstall } from '../uninstall.js'; +import { log } from '../utils/logger.js'; import { EnvHandler } from '../resources/env.js'; +import { deployBuiltinRules } from '../builtin-rules.js'; +import { loadStateForScope, saveStateForScope } from '../config.js'; +import { checkoutKey } from '../pull.js'; +import { fileHash } from '../utils/fs.js'; import { TeamaiConfigSchema, getDataHome, managedMcpManifestKey, managedMcpManifestPath } from '../types.js'; import { ModelProfileSchema, resolveProfile } from '../models/profile.js'; import { switchModelProfile } from '../models/switch.js'; @@ -64,6 +69,8 @@ const TEAMAI_CLAUDEMD_START = ''; const TEAMAI_CLAUDEMD_END = ''; const TEAMAI_RECALL_RULES_START = ''; const TEAMAI_RECALL_RULES_END = ''; +const TEAMAI_TEAM_RULES_START = ''; +const TEAMAI_TEAM_RULES_END = ''; const TEAMAI_ENV_START = '# [teamai:env:start]'; const TEAMAI_ENV_END = '# [teamai:env:end]'; const TEAMAI_PI_HOOK = '// [teamai] hooks extension — generated by teamai'; @@ -589,7 +596,8 @@ describe('uninstall', () => { // WorkBuddy is actually installed here (its own resource dir exists), // not just present in toolPaths — see the sibling "not installed" test. await fse.ensureDir(path.join(projectRoot, '.workbuddy', 'skills')); - await fse.writeFile(sharedInstructions, `${TEAMAI_RULES_START}\nteam rules\n${TEAMAI_RULES_END}\n`); + // A block WorkBuddy's pull writes: the legacy rules block is kept for nobody. + await fse.writeFile(sharedInstructions, `${TEAMAI_CULTURE_START}\nteam culture\n${TEAMAI_CULTURE_END}\n`); await fse.writeFile(projectPiHook, TEAMAI_PI_HOOK); const teamConfig = makeTeamConfig({ @@ -624,7 +632,7 @@ describe('uninstall', () => { expect(await fse.pathExists(projectPiHook)).toBe(false); expect(await fse.pathExists(sharedInstructions)).toBe(true); - expect(await fse.readFile(sharedInstructions, 'utf8')).toContain(TEAMAI_RULES_START); + expect(await fse.readFile(sharedInstructions, 'utf8')).toContain(TEAMAI_CULTURE_START); }); it('targeted Pi uninstall removes shared AGENTS.md when the other tool sharing it was never installed', async () => { @@ -666,6 +674,218 @@ describe('uninstall', () => { expect(await fse.pathExists(sharedInstructions)).toBe(false); }); + // #938: Codex gets its instruction blocks in AGENTS.md and the team rules + // from its session-start hook. Earlier releases copied the rules into + // .codex/rules/*.md, next to Codex's own exec-policy files. + describe('Codex instruction blocks and old rule copies (#938)', () => { + const block = (start: string, end: string, body: string) => `${start}\n${body}\n${end}`; + const culture = block(TEAMAI_CULTURE_START, TEAMAI_CULTURE_END, '## Team Culture'); + const shared = block(TEAMAI_CLAUDEMD_START, TEAMAI_CLAUDEMD_END, '## Shared Instructions'); + const recall = block(TEAMAI_RECALL_RULES_START, TEAMAI_RECALL_RULES_END, '## Recall Rules'); + + const codexPaths = { + skills: '.codex/skills', + settings: '.codex/hooks.json', + agents: '.codex/agents', + claudemd: 'AGENTS.md', + userScope: { claudemd: '.codex/AGENTS.md' }, + }; + const piPaths = { skills: '.pi/skills', rules: '.pi/rules', claudemd: 'AGENTS.md' }; + const workbuddyPaths = { + skills: '.workbuddy/skills', + rules: '.workbuddy/rules', + settings: '.workbuddy/settings.json', + claudemd: 'AGENTS.md', + agents: '.workbuddy/agents', + }; + + async function projectFixture(toolPaths: TeamaiConfig['toolPaths'], enabledAgents: string[]) { + const { homeDir, repoPath, teamaiHome } = await setupFixture(tmpDir); + const projectRoot = path.join(tmpDir, 'business-repo'); + vi.stubEnv('HOME', homeDir); + vi.stubEnv('SHELL', '/bin/zsh'); + const teamConfig = makeTeamConfig({ toolPaths }); + const localConfig = makeLocalConfig(homeDir, repoPath, { + scope: 'project', + projectRoot, + enabledAgents, + repo: { localPath: repoPath, remote: '', kind: 'self', businessRepoRoot: projectRoot }, + }); + mockAutoDetectInit.mockResolvedValue({ localConfig, teamConfig }); + return { projectRoot, teamaiHome, localConfig, agentsMd: path.join(projectRoot, 'AGENTS.md') }; + } + + it.each(['codex', 'codex-internal', 'tcodex'])('a full uninstall removes the instruction blocks and the old .%s/rules copies, keeping its own files', async (tool) => { + const toolPaths = { + skills: `.${tool}/skills`, + settings: `.${tool}/hooks.json`, + agents: `.${tool}/agents`, + claudemd: 'AGENTS.md', + userScope: { claudemd: `.${tool}/AGENTS.md` }, + }; + const { projectRoot, localConfig, agentsMd } = await projectFixture({ [tool]: toolPaths }, [tool]); + await fse.ensureDir(path.join(projectRoot, `.${tool}`)); + await fse.writeJson(path.join(projectRoot, `.${tool}`, 'hooks.json'), {}); + // The file as teamai creates it for its blocks: nothing left worth a file. + await fse.writeFile(agentsMd, `${culture}\n\n${recall}\n`); + const legacyRules = path.join(projectRoot, `.${tool}`, 'rules'); + await fse.ensureDir(legacyRules); + await fse.writeFile(path.join(legacyRules, 'team-rule.md'), '# Team Rule'); + // The recall rule as a pull before #938 deployed it there. + await deployBuiltinRules(makeTeamConfig({ toolPaths: { [tool]: { ...toolPaths, rules: `.${tool}/rules` } } }), localConfig); + await fse.writeFile(path.join(legacyRules, 'default.rules'), 'prefix_rule(pattern=["ls"])'); + await fse.writeFile(path.join(legacyRules, 'my-own-rule.md'), 'mine'); + const printed = vi.spyOn(console, 'log').mockImplementation(() => {}); + + await uninstall({ force: true }); + + // The summary and the result name the file, which is not a CLAUDE.md. + const output = [...printed.mock.calls, ...vi.mocked(log.success).mock.calls].map((c) => c.join(' ')).join('\n'); + printed.mockRestore(); + expect(output).toContain(agentsMd); + expect(output).toContain(`Cleaned ${agentsMd}`); + expect(output).not.toContain('CLAUDE.md'); + expect(await fse.pathExists(agentsMd)).toBe(false); + expect(await fse.pathExists(path.join(legacyRules, 'team-rule.md'))).toBe(false); + expect(await fse.pathExists(path.join(legacyRules, 'teamai-recall.md'))).toBe(false); + expect(await fse.pathExists(path.join(legacyRules, 'default.rules'))).toBe(true); + expect(await fse.pathExists(path.join(legacyRules, 'my-own-rule.md'))).toBe(true); + }); + + it.each(['codex', 'codex-internal', 'tcodex'])('a user-scope uninstall removes the team-rules block from ~/.%s/AGENTS.md and keeps the member\'s text', async (tool) => { + const { homeDir, repoPath } = await setupFixture(tmpDir); + vi.stubEnv('HOME', homeDir); + vi.stubEnv('SHELL', '/bin/zsh'); + const defaults = TeamaiConfigSchema.parse({ team: 't', repo: 'owner/repo' }).toolPaths; + const teamConfig = makeTeamConfig({ toolPaths: { [tool]: defaults[tool] } }); + mockAutoDetectInit.mockResolvedValue({ localConfig: makeLocalConfig(homeDir, repoPath, { enabledAgents: [tool] }), teamConfig }); + const agentsMd = path.join(homeDir, `.${tool}`, 'AGENTS.md'); + await fse.outputFile(path.join(homeDir, `.${tool}`, 'hooks.json'), '{}'); + await fse.writeFile(agentsMd, `# My notes\n\n${block(TEAMAI_TEAM_RULES_START, TEAMAI_TEAM_RULES_END, '# Team Rule')}\n`); + + await uninstall({ force: true, agent: tool }); + + expect(await fse.readFile(agentsMd, 'utf8')).toBe('# My notes\n'); + }); + + it('a project-scope uninstall leaves the project AGENTS.md to its owners with the default Codex entry', async () => { + const defaults = TeamaiConfigSchema.parse({ team: 't', repo: 'owner/repo' }).toolPaths; + const { projectRoot, agentsMd } = await projectFixture({ codex: defaults.codex }, ['codex']); + await fse.ensureDir(path.join(projectRoot, '.codex')); + const owners = `# Project notes\n\n${culture}\n`; + await fse.writeFile(agentsMd, owners); + + await uninstall({ force: true }); + + expect(await fse.readFile(agentsMd, 'utf8')).toBe(owners); + }); + + async function codexLegacyFixture() { + const fixture = await projectFixture({ codex: codexPaths }, ['codex']); + await fse.ensureDir(path.join(fixture.projectRoot, '.codex', 'rules')); + await fse.writeJson(path.join(fixture.projectRoot, '.codex', 'hooks.json'), {}); + return { ...fixture, legacy: (file: string) => path.join(fixture.projectRoot, '.codex', 'rules', file) }; + } + + it('keeps a .codex/rules copy the member edited and names it in an English warning', async () => { + const { legacy } = await codexLegacyFixture(); + await fse.writeFile(legacy('team-rule.md'), '# Team Rule\nMy own note.\n'); + await fse.writeFile(legacy('teamai-recall.md'), '# Recall Rule, as I rewrote it'); + + await uninstall({ force: true }); + + expect(await fse.readFile(legacy('team-rule.md'), 'utf8')).toBe('# Team Rule\nMy own note.\n'); + expect(await fse.readFile(legacy('teamai-recall.md'), 'utf8')).toBe('# Recall Rule, as I rewrote it'); + const warnings = vi.mocked(log.warn).mock.calls.map(([message]) => String(message)); + const kept = warnings.filter((message) => message.includes(legacy('team-rule.md'))); + expect(kept).toHaveLength(1); + expect(kept[0]).toContain(legacy('teamai-recall.md')); + expect(kept[0]).toContain('Codex does not read'); + expect(kept[0]).toMatch(/[Dd]elete/); + }); + + it.each([false, true])('keeps a removed legacy rule without a delivery record, selective uninstall: %s', async (selective) => { + const { legacy } = await codexLegacyFixture(); + await fse.writeFile(path.join(tmpDir, 'team-repo', 'rules', '.removed'), 'retired\n'); + const edited = 'A rule the team retired.\nMy own note.\n'; + await fse.writeFile(legacy('retired.md'), edited); + vi.mocked(log.warn).mockClear(); + + await uninstall({ force: true, ...(selective ? { agent: 'codex' } : {}) }); + + expect(await fse.readFile(legacy('retired.md'), 'utf8')).toBe(edited); + const warnings = vi.mocked(log.warn).mock.calls.map(([message]) => String(message)); + const kept = warnings.filter((message) => message.includes(legacy('retired.md'))); + expect(kept).toHaveLength(1); + expect(kept[0]).toContain('could not verify'); + expect(kept[0]).toMatch(/[Dd]elete/); + }); + + it('removes a recorded, unchanged .codex/rules copy of a rule the team removed before this checkout pulled again', async () => { + const { legacy, localConfig, projectRoot } = await codexLegacyFixture(); + const repoPath = path.join(tmpDir, 'team-repo'); + await fse.writeFile(path.join(repoPath, 'rules', '.removed'), 'retired\n'); + await fse.writeFile(legacy('retired.md'), 'A rule the team retired.\n'); + const state = await loadStateForScope(localConfig); + state.lastPullByWorkspace = { + [await checkoutKey(projectRoot)]: { + rev: 'old', targets: ['codex'], delivered: { [legacy('retired.md')]: (await fileHash(legacy('retired.md')))! }, + }, + }; + await saveStateForScope(state, localConfig); + + await uninstall({ force: true }); + + expect(await fse.pathExists(legacy('retired.md'))).toBe(false); + }); + + it('--agent codex with Pi still active removes the recall block and keeps Pi\'s', async () => { + const { projectRoot, agentsMd } = await projectFixture({ codex: codexPaths, pi: piPaths }, ['codex', 'pi']); + await fse.ensureDir(path.join(projectRoot, '.codex')); + await fse.writeJson(path.join(projectRoot, '.codex', 'hooks.json'), {}); + await fse.ensureDir(path.join(projectRoot, '.pi', 'skills')); + await fse.writeFile(agentsMd, ['# Project notes', '', culture, '', shared, '', recall, ''].join('\n')); + + await uninstall({ force: true, agent: 'codex' }); + + const content = await fse.readFile(agentsMd, 'utf8'); + expect(content).toContain('# Project notes'); + expect(content).toContain(culture); + expect(content).toContain(shared); + // Pi has no `agents`, so it never receives the recall block. + expect(content).not.toContain(TEAMAI_RECALL_RULES_START); + }); + + it('--agent codex keeps the culture block while another Codex-family tool maps the same file', async () => { + const { projectRoot, agentsMd } = await projectFixture( + { codex: codexPaths, tcodex: { settings: '.tcodex/hooks.json', claudemd: 'AGENTS.md' } }, + ['codex', 'tcodex'], + ); + await fse.ensureDir(path.join(projectRoot, '.codex')); + await fse.writeJson(path.join(projectRoot, '.codex', 'hooks.json'), {}); + await fse.ensureDir(path.join(projectRoot, '.tcodex')); + await fse.writeJson(path.join(projectRoot, '.tcodex', 'hooks.json'), {}); + await fse.writeFile(agentsMd, `${culture}\n`); + + await uninstall({ force: true, agent: 'codex' }); + + expect(await fse.readFile(agentsMd, 'utf8')).toContain(culture); + }); + + it('--agent workbuddy with Pi remaining removes WorkBuddy\'s recall block, which Pi never writes', async () => { + const { projectRoot, agentsMd } = await projectFixture({ pi: piPaths, workbuddy: workbuddyPaths }, ['pi', 'workbuddy']); + await fse.ensureDir(path.join(projectRoot, '.pi', 'skills')); + await fse.ensureDir(path.join(projectRoot, '.workbuddy', 'skills')); + await fse.writeFile(agentsMd, [culture, '', recall, ''].join('\n')); + + await uninstall({ force: true, agent: 'workbuddy' }); + + const content = await fse.readFile(agentsMd, 'utf8'); + expect(content).toContain(culture); + expect(content).not.toContain(TEAMAI_RECALL_RULES_START); + }); + }); + // Regression: Cursor rules are `.mdc`; matching only `.md` left every team // rule on disk after uninstall, still injected into each Cursor session. it('removes cursor .mdc rules (and a legacy .md copy) on uninstall', async () => { diff --git a/src/builtin-hooks.ts b/src/builtin-hooks.ts index d78c4ef05..98b7800e5 100644 --- a/src/builtin-hooks.ts +++ b/src/builtin-hooks.ts @@ -370,6 +370,19 @@ const SUBAGENT_STOP_SPEC: BuiltinHookSpec = { timeoutSec: 15, }; +/** + * A subagent Codex spawns fires SubagentStart, not SessionStart (codex-rs + * core/src/hook_runtime.rs), so a fresh one gets the team rules only through + * this entry (#938). Codex alone: the team rules reach no other tool this way. + */ +const SUBAGENT_START_SPEC: BuiltinHookSpec = { + key: 'Hook dispatch subagent-start', + event: 'SubagentStart', + dispatchEvent: 'subagent-start', + matcher: '*', + timeoutSec: 15, +}; + /** * Tools that fire SubagentStop with the parent's session id, so the recall * reducer can credit a read a subagent made after the session's last Stop @@ -401,6 +414,10 @@ const SUBAGENT_STOP_TOOLS = new Set([ */ const WRAPPER_TOOLS = SHELL_DEPENDENT_TOOLS; +function isCodexTool(tool: string): boolean { + return (CODEX_TOOL_IDS as readonly string[]).includes(tool); +} + export function builtinHookDefs(tool: string): HookDef[] { // ZCode renders per-event timeouts from the ZCODE_TIMEOUT_MS table in its own // writer (toZcodeEntry), so def.timeout stays unset for it. @@ -414,6 +431,7 @@ export function builtinHookDefs(tool: string): HookDef[] { ...BUILTIN_HOOK_SPECS, ...(tool === 'copilot' ? [COPILOT_SESSION_END_SPEC] : []), ...(SUBAGENT_STOP_TOOLS.has(tool) ? [SUBAGENT_STOP_SPEC] : []), + ...(isCodexTool(tool) ? [SUBAGENT_START_SPEC] : []), ]; return specs.map((spec) => ({ source: 'builtin' as const, @@ -422,6 +440,9 @@ export function builtinHookDefs(tool: string): HookDef[] { matcher: spec.matcher, command: buildCommand(spec.dispatchEvent, tool, spec.matcher), timeout: withTimeout ? spec.timeoutSec : undefined, + // The team rules reach the Codex family through its start hooks (#938); + // Codex would otherwise keep only the start and end of a large rule set. + ...(isCodexTool(tool) && (spec.event === 'SessionStart' || spec.event === 'SubagentStart') ? { additionalContextLimit: 0 } : {}), description: `${TEAMAI_HOOK_DESCRIPTION_PREFIX} ${spec.key}`, })); } diff --git a/src/builtin-rules.ts b/src/builtin-rules.ts index 7f7d912b3..89048d847 100644 --- a/src/builtin-rules.ts +++ b/src/builtin-rules.ts @@ -7,6 +7,7 @@ import { teamRuleToCursorMdc } from './resources/cursor-mdc.js'; import type { TeamaiConfig, LocalConfig } from './types.js'; import { resolveToolBaseDir, isAgentExcluded, scopedToolPaths } from './types.js'; import fs from 'node:fs/promises'; +import crypto from 'node:crypto'; import { getUserHome } from './utils/home.js'; // ─── Built-in rules deployment ────────────────────────── @@ -124,7 +125,39 @@ export async function deployBuiltinRules( // ─── Rule content ────────────────────────────────────── -const TEAMAI_RECALL_RULE_CONTENT = `# Team Knowledge Recall (teamai) +const TEAMAI_RECALL_RULE_HEADING = '# Team Knowledge Recall (teamai)'; + +/** + * SHA-256 of every `teamai-recall` rule body a teamai version deployed under + * TEAMAI_RECALL_RULE_HEADING, newest first (from `git log origin/main -- + * src/builtin-rules.ts`). The built-ins are not in the delivery ledger, so + * these hashes are what proves a copy in a Codex legacy rules dir unedited. + * Add the new hash when TEAMAI_RECALL_RULE_CONTENT changes: a team `toolPaths` + * that still names such a dir keeps receiving the current body. + */ +const DEPLOYED_RECALL_RULE_HASHES = new Set([ + '9fbe3cc0b5bd39b9418ab2672a38cac831f2d4a3ab31ad539ce4de51d0bcab7f', + '564b9e8369fbc0009580f0174af119144da6b07ee4e1030f75bc5a03e9c60015', + '11698b506859049b7dc7ceb7ea149c371ae42dfc0db9a512763d9295f4af480e', + '37af125211b657cb81d5d3b11ef1c3a1daaa38d3f3323255258033c18ebd03d8', + '6ef190b7ac41d95357dc3de82791c4cea97203409972a53f6cbae21fe23b4f33', + 'c15adbb1b6d9a96ebe71eed179edbe150ea840505169068af8ae3b79429a83f6', + '0c9fc1c9c026f5625f42062e8116bc6f8985a6177ecc5ed7328a74159b2ba525', + '168278feb1a0b5d936497bc8e29cc5652dc472b69ddaba3ee9737fff90bb2c91', + '1929d8f639410e94315450548ab4a063cce0ace9415e48f0444666d8bc33c680', + 'ed52f7cd1c2baa891e83b285bd5dd1347a2e7b31b613b1b2843fdaea6ff0d140', +]); + +/** + * True when `content` is the `teamai-recall` rule exactly as some teamai + * version deployed it. A member's edit since the last pull that wrote the + * file makes it false, so the copy is kept. + */ +export function isDeployedRecallRule(content: string): boolean { + return DEPLOYED_RECALL_RULE_HASHES.has(crypto.createHash('sha256').update(content).digest('hex')); +} + +const TEAMAI_RECALL_RULE_CONTENT = `${TEAMAI_RECALL_RULE_HEADING} > **Self-exemption (must read first):** If you ARE the \`teamai-recall\` subagent yourself, this rule does NOT apply to you — do not invoke \`teamai-recall\` (or any recall) again. Proceed directly to performing the knowledge search that is your task. This prevents infinite subagent recursion in tools (e.g. Cursor) whose always-apply rules leak into subagent sessions. > diff --git a/src/doctor-delivery.ts b/src/doctor-delivery.ts index 6b9993020..400148eb8 100644 --- a/src/doctor-delivery.ts +++ b/src/doctor-delivery.ts @@ -257,7 +257,14 @@ export async function buildRulesDeliveryChecks(ctx: DoctorContext): Promise { + const { localConfig, teamConfig } = ctx; + if (!teamConfig || localConfig.scope !== 'user') return []; + + const { teamRulesBlock } = await import('./resources/rules.js'); + const { getsRulesFromSessionHook, instructionFileInstallProbe, writesInstructionBlock } = await import('./resources/rule-format.js'); + const { isToolInstalledForConfig } = await import('./resources/base.js'); + const { TEAMAI_TEAM_RULES_START, TEAMAI_TEAM_RULES_END, resolveToolBaseDir, scopedToolPaths } = await import('./types.js'); + const expected = await teamRulesBlock(items); + const checks: Check[] = []; + for (const [tool, toolPath] of Object.entries(scopedToolPaths(teamConfig, localConfig))) { + if (!getsRulesFromSessionHook(tool) || isAgentExcluded(localConfig, tool)) continue; + const probe = instructionFileInstallProbe(tool, toolPath); + if (probe !== undefined && !await isToolInstalledForConfig(tool, probe, localConfig)) continue; + const name = tool === 'codex' + ? 'Team rules are inlined in Codex AGENTS.md' + : `Team rules are inlined in Codex AGENTS.md (${tool})`; + if (!writesInstructionBlock(tool, toolPath, 'team-rules')) { + // A team `toolPaths` entry replaces the default one whole, so an entry + // written before #938 leaves this tool nowhere to read user rules from. + // One with no `rules` path delivers no rules to it on purpose; with no + // team rules it misses none. + if (items.length === 0 || !toolPath.rules) continue; + checks.push({ + name, + source: 'local', + check: async () => false, + fix: `The toolPaths entry for ${tool} has no \`claudemd\` path, so pull has no instructions ` + + `file to inline the team rules into and ${tool} reads none of them. Add ` + + `\`userScope.claudemd: .${tool}/AGENTS.md\` to that entry in the team teamai.yaml, ` + + 'then run `teamai pull`.', + }); + continue; + } + const file = path.join(resolveToolBaseDir(tool, localConfig), toolPath.claudemd); + const content = await readFileSafe(file); + const start = content?.indexOf(TEAMAI_TEAM_RULES_START) ?? -1; + const end = content?.indexOf(TEAMAI_TEAM_RULES_END) ?? -1; + const delivered = content !== null && start !== -1 && end > start + ? content.slice(start, end + TEAMAI_TEAM_RULES_END.length) + : null; + // Codex reads AGENTS.override.md instead of AGENTS.md in the same + // directory, so a current block there is never seen. An empty or + // whitespace-only override shadows it too (checked with `codex exec`). + const override = path.join(path.dirname(file), 'AGENTS.override.md'); + const problems: string[] = []; + // With no rule body to inline (`expected === null`), pull writes no block. + if (delivered === null && expected !== null) { + problems.push(`${file} carries no team-rules block, so ${tool} reads none of the team ` + + 'rules. Run `teamai pull` to restore it.'); + } else if (delivered !== expected) { + problems.push(`The team-rules block in ${file} is not what the team rules inline to: Codex reads ` + + 'standing instructions from this file rather than a rules directory, so a stale block ' + + 'is a stale rule set. Run `teamai pull` to rewrite it.'); + } + if (expected !== null && await isReadableFile(override)) { + problems.push(`${override} exists, so Codex reads it instead of ${file} and never sees the ` + + 'team rules. Move its content into AGENTS.md, or delete it.'); + } + checks.push({ name, source: 'local', check: async () => problems.length === 0, fix: problems.join(' ') }); + } return checks; } diff --git a/src/doctor.ts b/src/doctor.ts index aeb6cfe1b..85ffb0dd9 100644 --- a/src/doctor.ts +++ b/src/doctor.ts @@ -20,6 +20,7 @@ import { } from './types.js'; import { isToolInstalledForConfig } from './resources/base.js'; import { skillsDirForTool } from './resources/skills.js'; +import { getsRulesFromSessionHook } from './resources/rule-format.js'; import { TEAMAI_HOOK_SUBCOMMANDS, isCodexTrustGatedTool, codexTrustReminder } from './hooks.js'; import { buildDeliveryChecks, @@ -295,10 +296,56 @@ async function buildHookChecks( }, fix: 'Run `teamai hooks inject` to inject/update hooks', }); + if (getsRulesFromSessionHook(tool)) checks.push(sessionHookRulesCheck(tool, settingsPath)); } return checks; } +/** + * In a project the Codex family gets the team rules and instruction blocks + * from its session hooks (#938, #945): SessionStart, and SubagentStart for a + * fresh subagent, which fires no SessionStart. Past 2,500 tokens Codex keeps + * only the start and end of a hook's context unless the entry sets + * `additionalContextLimit: 0`. A missing session-start entry is the hooks + * check's to report. + */ +function sessionHookRulesCheck(tool: string, settingsPath: string): Check { + return { + name: `Project rules and instructions reach ${tool} whole through its session hooks`, + source: 'local', + check: async () => { + const sessionStart = await teamaiHookEntries(settingsPath, 'SessionStart', 'session-start'); + if (sessionStart.length === 0) return true; + const subagentStart = await teamaiHookEntries(settingsPath, 'SubagentStart', 'subagent-start'); + return [sessionStart, subagentStart].every((entries) => entries.some((entry) => entry.additionalContextLimit === 0)); + }, + fix: `The teamai SessionStart and SubagentStart entries in ${settingsPath} must both exist and set ` + + `\`additionalContextLimit: 0\`. Without them ${tool} keeps only the start and end of a large set of ` + + 'team rules and instructions, and a fresh subagent gets none. Run `teamai pull` to rewrite them' + + (isCodexTrustGatedTool(tool) ? ', then approve the changed hooks in Codex (/hooks).' : '.'), + }; +} + +/** The teamai handlers for one event in a Codex hooks.json; none when it does not parse. */ +async function teamaiHookEntries( + settingsPath: string, + event: 'SessionStart' | 'SubagentStart', + subcommand: string, +): Promise> { + let parsed: unknown; + try { + parsed = JSON.parse(await readFileSafe(settingsPath) ?? ''); + } catch { + return []; + } + const groups = (parsed as { hooks?: Record } | null)?.hooks?.[event]; + if (!Array.isArray(groups)) return []; + return groups + .flatMap((group) => (Array.isArray(group?.hooks) ? group.hooks : [])) + .filter((entry): entry is { command: string; additionalContextLimit?: unknown } => + typeof entry?.command === 'string' && entry.command.includes(`teamai hook-dispatch ${subcommand}`)); +} + /** * True if a trust-gated Codex tool (the public `codex`) already has teamai hooks diff --git a/src/hook-dispatch-cli.ts b/src/hook-dispatch-cli.ts index 868bbb25d..45f493c22 100644 --- a/src/hook-dispatch-cli.ts +++ b/src/hook-dispatch-cli.ts @@ -370,6 +370,7 @@ export function parseStdin(raw: string, event: string): Record 'session-end': 'SessionEnd', 'stop': 'Stop', 'subagent-stop': 'SubagentStop', + 'subagent-start': 'SubagentStart', 'post-tool-use': 'PostToolUse', 'prompt-submit': 'UserPromptSubmit', }; diff --git a/src/hook-handlers.ts b/src/hook-handlers.ts index f7980a31a..4358ed31c 100644 --- a/src/hook-handlers.ts +++ b/src/hook-handlers.ts @@ -715,6 +715,38 @@ const secretsHintHandler: HookHandler = { }, }; +/** + * SessionStart: a project's rules and instruction blocks (culture, shared + * instructions, recall) for a tool with no rules format and no project file of + * its own (the Codex family, #938, #945). The project AGENTS.md is the + * owners' file, and other tools read it too. User-scope content is in the + * tool's own AGENTS.md, so a session outside a project gets nothing here. + * Codex runs SessionStart again after a compaction or a clear; a resumed + * session already holds the content in its history. A subagent fires + * SubagentStart instead, which gets the same content. + */ +const teamRulesHandler: HookHandler = { + name: 'team-rules', + async execute(stdin, tool, config) { + if (!config || config.scope !== 'project' || stdin.source === 'resume') return null; + const { getsRulesFromSessionHook } = await import('./resources/rule-format.js'); + const { isAgentExcluded } = await import('./types.js'); + if (!getsRulesFromSessionHook(tool) || isAgentExcluded(config, tool)) return null; + const { loadTeamConfig } = await import('./config.js'); + const teamConfig = await loadTeamConfig(config.repo.localPath); + if (!teamConfig) return null; + const { sessionInstructionBlocks } = await import('./pull.js'); + const { teamRulesContext } = await import('./resources/rules.js'); + const parts = await sessionInstructionBlocks(teamConfig, config, tool); + const rules = await teamRulesContext(teamConfig, config); + if (rules !== null) parts.push(rules); + if (parts.length === 0) return null; + // Codex rejects output whose hookEventName is not the event it ran. + const hookEventName = stdin.hook_event_name === 'SubagentStart' ? 'SubagentStart' : 'SessionStart'; + return JSON.stringify({ hookSpecificOutput: { hookEventName, additionalContext: parts.join('\n\n') } }); + }, +}; + /** HTTP local-agent report/sync + workspace binding prompts. */ const localAgentHandler: HookHandler = { name: 'local-agent-sync', @@ -826,6 +858,7 @@ export function buildHandlerRegistry(): HandlerRegistration[] { // (PULL_TIMEOUT_MS) — the shared 15s truncated the pull itself. { event: 'session-start', matcher: '*', handler: pullHandler, timeoutMs: PULL_TIMEOUT_MS, background: true }, { event: 'session-start', matcher: '*', handler: dashboardReportHandler, timeoutMs: FOREGROUND_HOOK_TIMEOUT_MS, requiresConfig: true }, + { event: 'session-start', matcher: '*', handler: teamRulesHandler, timeoutMs: FOREGROUND_HOOK_TIMEOUT_MS, requiresConfig: true }, { event: 'session-start', matcher: '*', handler: mrHintHandler, timeoutMs: FOREGROUND_HOOK_TIMEOUT_MS, gitOnly: true, requiresConfig: true }, { event: 'session-start', matcher: '*', handler: packageHintHandler, timeoutMs: FOREGROUND_HOOK_TIMEOUT_MS, requiresConfig: true }, { event: 'session-start', matcher: '*', handler: secretsHintHandler, timeoutMs: FOREGROUND_HOOK_TIMEOUT_MS, requiresConfig: true }, @@ -859,6 +892,10 @@ export function buildHandlerRegistry(): HandlerRegistration[] { { event: 'stop', matcher: '*', handler: localAgentHandler, timeoutMs: LOCAL_AGENT_TIMEOUT_MS, background: true }, { event: 'stop', matcher: '*', handler: webhookHandler, timeoutMs: FOREGROUND_HOOK_TIMEOUT_MS, background: true, requiresConfig: true }, + // ─── SubagentStart ──────────────────────────────── + // Codex only (SUBAGENT_START_SPEC): a fresh subagent fires no SessionStart (#938). + { event: 'subagent-start', matcher: '*', handler: teamRulesHandler, timeoutMs: FOREGROUND_HOOK_TIMEOUT_MS, requiresConfig: true }, + // ─── SubagentStop ───────────────────────────────── // A subagent can finish after the session's last Stop (a background // worker): votes-sync credits what it read then, locally; the next Stop or diff --git a/src/hooks.ts b/src/hooks.ts index 2a6a32f8f..62d9a02c5 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -98,6 +98,7 @@ interface CodexHookEntry { type: string; command: string; timeout?: number; + additionalContextLimit?: number; } interface CodexHookMatcher { @@ -407,6 +408,7 @@ function toCodexEntry(def: HookDef): CodexHookMatcher { type: 'command', command: def.command, ...(def.timeout !== undefined ? { timeout: def.timeout } : {}), + ...(def.additionalContextLimit !== undefined ? { additionalContextLimit: def.additionalContextLimit } : {}), }, ], }; diff --git a/src/pull.ts b/src/pull.ts index 509a8135c..e2b513b5b 100644 --- a/src/pull.ts +++ b/src/pull.ts @@ -20,7 +20,7 @@ import { reportHeldAgents, type RedeployedCopy } from './resources/agents.js'; import { listStaleDocDirectories, resolveDesiredDocs, resolveDocsDestination } from './resources/docs.js'; import { isToolInstalledForConfig, ResourceHandler } from './resources/base.js'; import { skillsDirForTool } from './resources/skills.js'; -import { ruleFileExtensionForTool } from './resources/rule-format.js'; +import { instructionFileInstallProbe, ruleFileExtensionForTool, writesInstructionBlock } from './resources/rule-format.js'; import { AGENT_FILE_EXTENSIONS } from './resources/agent-format.js'; import { forgetDelivered, judgeCopy, openLedger, removedCopyChanged, reportKept, type DeliveredHashes, type DeliveryLedger, @@ -1224,6 +1224,19 @@ async function pullForScope( // 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, openLedger(await deliveredHashes(localConfig, state))); + // Same reason: a CLI that moves Codex's team rules into its own + // AGENTS.md (user scope) and its session-start hook (project scope) + // writes that block and reclaims the old .codex/rules copies here (#938). + if (resourceTypes.includes('rules')) { + try { + const { items } = await resolveDesiredRules(freshConfig, localConfig, roleContext); + await (getHandler('rules') as RulesHandler).syncCodexInstructionRules( + freshConfig, localConfig, items, openLedger(await deliveredHashes(localConfig, state)), + ); + } catch (error) { + log.warn(`[${scopeLabel}] Codex's team rules were not updated: ${(error as Error).message}. Run \`teamai pull --force\` to retry.`); + } + } // The repo has not moved, but an agent's model may have (#830). if (resourceTypes.includes('agents')) { if (await redeployAgentsWithChangedModels(freshConfig, localConfig, roleContext, scopeLabel, revisionField, reported) && result) { @@ -1854,8 +1867,9 @@ async function syncManagedInstructions( if (compiledCulture !== undefined) { for (const [tool, toolPath] of Object.entries(scopedToolPaths(config, localConfig))) { - if (isAgentExcluded(localConfig, tool) || !toolPath.claudemd) continue; - if (toolPath.rules && !await isToolInstalledForConfig(tool, toolPath.rules, localConfig)) continue; + if (isAgentExcluded(localConfig, tool) || !writesInstructionBlock(tool, toolPath, 'culture')) continue; + const installProbe = instructionFileInstallProbe(tool, toolPath); + if (installProbe && !await isToolInstalledForConfig(tool, installProbe, localConfig)) continue; const claudeMdPath = path.join(resolveToolBaseDir(tool, localConfig), toolPath.claudemd); try { @@ -1885,8 +1899,9 @@ async function syncManagedInstructions( const compiled = compileClaudemd(claudemdContents); for (const [tool, toolPath] of Object.entries(scopedToolPaths(config, localConfig))) { - if (isAgentExcluded(localConfig, tool) || !toolPath.claudemd) continue; - if (toolPath.rules && !await isToolInstalledForConfig(tool, toolPath.rules, localConfig)) continue; + if (isAgentExcluded(localConfig, tool) || !writesInstructionBlock(tool, toolPath, 'claudemd')) continue; + const installProbe = instructionFileInstallProbe(tool, toolPath); + if (installProbe && !await isToolInstalledForConfig(tool, installProbe, localConfig)) continue; const claudeMdPath = path.join(resolveToolBaseDir(tool, localConfig), toolPath.claudemd); try { @@ -1918,9 +1933,10 @@ async function syncManagedInstructions( * Inject (or replace) the teamai-recall block into every Tier-1 tool's CLAUDE.md. * * Only injected for Tier-1 tools that have BOTH `agents` and `claudemd` - * configured. Tools without subagent support (cursor / codex / openclaw / - * workbuddy) are skipped — for them the recall flow runs purely via the - * TodoWrite hint hook and the manual `teamai recall` command. + * configured (Codex included: its `claudemd` is AGENTS.md). Tools missing + * either (e.g. cursor / openclaw / hermes) are skipped — for them the recall + * flow runs purely via the TodoWrite hint hook and the manual `teamai recall` + * command. * * Extracted so both the full-sync path (Step 3.8) and the "Already synced" * rev fast-path can call it — otherwise a CLI upgrade that ships a new recall @@ -1938,7 +1954,7 @@ export async function injectRecallBlockIntoTools( let injected = 0; for (const [tool, toolPath] of Object.entries(scopedToolPaths(config, localConfig))) { if (isAgentExcluded(localConfig, tool)) continue; - if (!toolPath.claudemd || !toolPath.agents) continue; + if (!writesInstructionBlock(tool, toolPath, 'recall')) continue; if (!await isToolInstalledForConfig(tool, toolPath.agents, localConfig)) continue; const baseDir = resolveToolBaseDir(tool, localConfig); @@ -2028,6 +2044,46 @@ export function compileRecallRulesBlock(): string { return lines.join('\n'); } +/** The text of a managed block, without its markers and DO NOT EDIT line. */ +function managedBlockBody(block: string): string { + return block + .split('\n') + .filter((line) => !/^$/.test(line.trim())) + .join('\n') + .trim(); +} + +/** + * The culture, shared-instruction and recall blocks a tool whose project + * instructions come from its session-start hook (the Codex family) gets in a + * project, resolved as pull resolves them. In user scope they are in the + * tool's own instructions file instead (#945). A block another tool already + * wrote into the project AGENTS.md is skipped, since the tool reads that file + * too. + */ +export async function sessionInstructionBlocks( + teamConfig: TeamaiConfig, + localConfig: LocalConfig, + tool: string, +): Promise { + const projectAgents = localConfig.projectRoot + ? await readFileSafe(path.join(localConfig.projectRoot, 'AGENTS.md')) ?? '' + : ''; + const blocks: Array<[string, string | null]> = []; + const culture = await readFileSafe(path.join(localConfig.repo.localPath, 'culture.md')); + blocks.push([TEAMAI_CULTURE_START, culture === null ? null : compileCulture(culture)]); + const { contents } = await collectClaudemdFiles(localConfig.repo.localPath, await buildRolePullContext(localConfig)); + blocks.push([TEAMAI_CLAUDEMD_START, compileClaudemd(contents)]); + const toolPath = scopedToolPaths(teamConfig, localConfig)[tool]; + if (toolPath?.agents && isRecallEnabled(localConfig, teamConfig)) { + blocks.push([TEAMAI_RECALL_RULES_START, compileRecallRulesBlock()]); + } + return blocks + .filter((entry): entry is [string, string] => entry[1] !== null && !projectAgents.includes(entry[0])) + .map(([, block]) => managedBlockBody(block)) + .filter((body) => body !== ''); +} + /** * Auto-migrate hooks from old individual format to unified hook-dispatch format. * Runs at session start: if settings.json doesn't contain 'hook-dispatch' commands, diff --git a/src/recall-toggle.ts b/src/recall-toggle.ts index a9293408d..1e8788a64 100644 --- a/src/recall-toggle.ts +++ b/src/recall-toggle.ts @@ -1,14 +1,15 @@ import path from 'node:path'; import { autoDetectInit, saveLocalConfigForScope } from './config.js'; import { log } from './utils/logger.js'; -import { readFileSafe, writeFile, remove, pathExists } from './utils/fs.js'; +import { remove, pathExists } from './utils/fs.js'; +import { removeClaudeMdSection } from './utils/claudemd.js'; import { isToolInstalledForConfig } from './resources/base.js'; import { ALL_SUPPORTED_TOOLS, agentFileExtensionForTool, type ToolName, } from './resources/agent-format.js'; -import { ruleFileExtensionForTool } from './resources/rule-format.js'; +import { ruleFileExtensionForTool, writesInstructionBlock } from './resources/rule-format.js'; import { LEGACY_RECALL_SKILL_NAMES, builtinSkillsTarget, pruneLegacyBuiltinSkills } from './builtin-skills.js'; import { resolveToolBaseDir, @@ -68,21 +69,8 @@ async function removeRecallArtifacts(teamConfig: TeamaiConfig, localConfig: Loca // Remove recall block from CLAUDE.md if (toolPath.claudemd) { const claudeMdPath = path.join(baseDir, toolPath.claudemd); - const content = await readFileSafe(claudeMdPath); - if (content && content.includes(TEAMAI_RECALL_RULES_START)) { - const startIdx = content.indexOf(TEAMAI_RECALL_RULES_START); - const endIdx = content.indexOf(TEAMAI_RECALL_RULES_END); - if (startIdx !== -1 && endIdx !== -1) { - const before = content.substring(0, startIdx).replace(/\n+$/, '\n'); - const after = content.substring(endIdx + TEAMAI_RECALL_RULES_END.length).replace(/^\n+/, '\n'); - const cleaned = (before + after).trim(); - if (cleaned.length === 0) { - await remove(claudeMdPath); - } else { - await writeFile(claudeMdPath, cleaned + '\n'); - } - log.debug(`Removed recall rules block from ${tool} CLAUDE.md`); - } + if (await removeClaudeMdSection(claudeMdPath, TEAMAI_RECALL_RULES_START, TEAMAI_RECALL_RULES_END, { deleteIfEmpty: true })) { + log.debug(`Removed recall rules block from ${tool} CLAUDE.md`); } } } @@ -104,7 +92,7 @@ async function deployRecallArtifacts(teamConfig: TeamaiConfig, localConfig: Loca for (const [tool, toolPath] of Object.entries(scopedToolPaths(teamConfig, localConfig))) { if (isAgentExcluded(localConfig, tool)) continue; - if (!toolPath.claudemd || !toolPath.agents) continue; + if (!writesInstructionBlock(tool, toolPath, 'recall')) continue; if (!await isToolInstalledForConfig(tool, toolPath.agents, localConfig)) continue; const baseDir = resolveToolBaseDir(tool, localConfig); diff --git a/src/resources/copilot-instructions.ts b/src/resources/copilot-instructions.ts index f4ea73e4f..bf5202225 100644 --- a/src/resources/copilot-instructions.ts +++ b/src/resources/copilot-instructions.ts @@ -1,4 +1,5 @@ import { splitFrontmatter, stringifyFrontmatter } from '../utils/frontmatter.js'; +import { rulePaths } from './rule-format.js'; const ALL_FILES_GLOB = '**'; @@ -6,17 +7,6 @@ function normalizeBody(body: string): string { return body.replace(/^\s+/, '').replace(/\s+$/, ''); } -function rulePaths(data: Record): string[] { - const value = data.paths; - if (Array.isArray(value)) { - return value.map((entry) => String(entry).trim()).filter(Boolean); - } - if (typeof value === 'string') { - return value.split(',').map((entry) => entry.trim()).filter(Boolean); - } - return []; -} - /** Convert a tool-neutral team rule into Copilot's native instructions format. */ export function teamRuleToCopilotInstructions(rawTeamRule: string): string { const { data, body } = splitFrontmatter(rawTeamRule); diff --git a/src/resources/rule-format.ts b/src/resources/rule-format.ts index 074d347f3..4fcabda62 100644 --- a/src/resources/rule-format.ts +++ b/src/resources/rule-format.ts @@ -11,8 +11,13 @@ * new per-tool extension never has to be re-discovered call site by call site. */ +import type { TeamaiConfig } from '../types.js'; + +type ToolPath = TeamaiConfig['toolPaths'][string]; + const CURSOR_MDC_RULE_TOOLS = new Set(['cursor', 'joycode']); const COPILOT_INSTRUCTIONS_RULE_TOOLS = new Set(['copilot']); +const SESSION_HOOK_RULE_TOOLS = new Set(['codex', 'codex-internal', 'tcodex']); /** Extension teamai writes rules with for a given tool. */ export function ruleFileExtensionForTool(tool: string): '.md' | '.mdc' | '.instructions.md' { @@ -30,6 +35,75 @@ export function usesCopilotInstructions(tool: string): boolean { return COPILOT_INSTRUCTIONS_RULE_TOOLS.has(tool); } +/** + * True when the tool has no rules format of its own, so its session-start hook + * adds the team rules (the Codex family, #938). Pull writes no rule file for it. + */ +export function getsRulesFromSessionHook(tool: string): boolean { + return SESSION_HOOK_RULE_TOOLS.has(tool); +} + +/** A managed block pull writes into a tool's instructions file (`claudemd`). */ +export type InstructionBlock = 'culture' | 'claudemd' | 'recall' | 'team-rules'; + +/** + * Whether pull writes `block` into this tool's instructions file: culture and + * shared instructions for every tool that has one, recall for a tool that + * also has `agents`, the team rules for a tool with no rules format (the + * Codex family has an instructions file in user scope only). Pull's writers + * and uninstall both ask this, so what uninstall keeps for a remaining tool is + * what that tool's next pull refreshes. Whether the tool is installed is a + * separate question (`instructionFileInstallProbe`). + */ +export function writesInstructionBlock( + tool: string, toolPath: ToolPath, block: 'recall', +): toolPath is ToolPath & { claudemd: string; agents: string }; +export function writesInstructionBlock( + tool: string, toolPath: ToolPath, block: InstructionBlock, +): toolPath is ToolPath & { claudemd: string }; +export function writesInstructionBlock(tool: string, toolPath: ToolPath, block: InstructionBlock): boolean { + if (!toolPath.claudemd) return false; + if (block === 'recall') return toolPath.agents !== undefined; + if (block === 'team-rules') return getsRulesFromSessionHook(tool); + return true; +} + +/** + * The tool path whose root says a tool is installed, for the culture and + * shared-instruction writers; undefined when there is none to + * probe. Never `claudemd`: a root-level AGENTS.md exists without the tool. A + * Codex-family entry may carry neither `rules` nor `settings` (a team entry + * replaces the default whole), so its `skills` root is the last resort. + */ +export function instructionFileInstallProbe(tool: string, toolPath: ToolPath): string | undefined { + const probe = toolPath.rules ?? toolPath.settings; + return getsRulesFromSessionHook(tool) ? probe ?? toolPath.skills : probe; +} + +/** + * The rules directory each tool that now gets rules from its session-start + * hook received `.md` copies in before #938, relative to the tool's base + * dir in either scope. The tool never read them. It keeps its own `*.rules` + * exec-policy files there, so only teamai's copies may be removed from it. + */ +export const LEGACY_RULE_DIRS: Readonly> = { + codex: '.codex/rules', + 'codex-internal': '.codex-internal/rules', + tcodex: '.tcodex/rules', +}; + +/** The globs a team rule's `paths:` frontmatter scopes it to; empty when unscoped. */ +export function rulePaths(data: Record): string[] { + const value = data.paths; + if (Array.isArray(value)) { + return value.map((entry) => String(entry).trim()).filter(Boolean); + } + if (typeof value === 'string') { + return value.split(',').map((entry) => entry.trim()).filter(Boolean); + } + return []; +} + /** * Every extension a rule file may carry on disk, newest layout first. * diff --git a/src/resources/rules.ts b/src/resources/rules.ts index d4acd4328..df8aa4168 100644 --- a/src/resources/rules.ts +++ b/src/resources/rules.ts @@ -1,29 +1,35 @@ import path from 'node:path'; import { isToolInstalledForConfig, ResourceHandler } from './base.js'; import type { ResourceItem, ResourceItemStatus, DeliveryTarget, TeamaiConfig, LocalConfig } from '../types.js'; -import { listFilesRecursive, pathExists, copyFile, ensureDir, remove, fileContentEqual, getFileMtime, listDirs, readFileSafe, writeFile } from '../utils/fs.js'; +import { listFilesRecursive, pathExists, copyFile, ensureDir, remove, fileContentEqual, getFileMtime, listDirs, readFileSafe, writeFile, pruneEmptyDirs, fileHash } from '../utils/fs.js'; import { log } from '../utils/logger.js'; -import { TEAMAI_RULES_START, TEAMAI_RULES_END, resolveBaseDir, resolveToolBaseDir, isAgentExcluded, scopedToolPaths, SELF_KNOWLEDGE_SCAN_KEY } from '../types.js'; -import { EXCLUDED_RULE_NAMES } from '../builtin-rules.js'; +import { TEAMAI_RULES_START, TEAMAI_RULES_END, TEAMAI_TEAM_RULES_START, TEAMAI_TEAM_RULES_END, resolveBaseDir, resolveToolBaseDir, isAgentExcluded, scopedToolPaths, SELF_KNOWLEDGE_SCAN_KEY } from '../types.js'; +import { EXCLUDED_RULE_NAMES, isDeployedRecallRule } from '../builtin-rules.js'; import { teamRuleToCursorMdc, mergeCursorBodyIntoTeamMd, cursorMdcBodyEqualsTeamMd } from './cursor-mdc.js'; import { copilotInstructionsBodyEqualsTeamMd, mergeCopilotBodyIntoTeamMd, teamRuleToCopilotInstructions, } from './copilot-instructions.js'; +import { splitFrontmatter } from '../utils/frontmatter.js'; import { assertWithinRoot } from '../utils/path-safety.js'; 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 { forgetDelivered, keepsEditedCopy, recordDelivered, removedCopyChanged, type DeliveredHashes, type DeliveryLedger } from './delivered-copies.js'; import { ruleFileExtensionForTool, ruleStemFromFilename, usesCursorMdcRules, usesCopilotInstructions, isLegacyCursorRuleFile, + LEGACY_RULE_DIRS, + rulePaths, + writesInstructionBlock, + instructionFileInstallProbe, } from './rule-format.js'; +import { injectClaudeMdSection, removeClaudeMdSection } from '../utils/claudemd.js'; export class RulesHandler extends ResourceHandler { readonly type = 'rules' as const; @@ -414,8 +420,11 @@ export class RulesHandler extends ResourceHandler { } } - // Refresh CLAUDE.md references - await this.pullAllRules(teamConfig, localConfig); + // Refresh with the rules this member's pull delivers, so the role, project + // and tag selection still applies to the files the refresh writes. + const { buildRolePullContext, resolveDesiredRules } = await import('./desired.js'); + const { items, replaced } = await resolveDesiredRules(teamConfig, localConfig, await buildRolePullContext(localConfig)); + await this.pullAllRules(teamConfig, localConfig, items, replaced); return removed; } @@ -446,10 +455,12 @@ export class RulesHandler extends ResourceHandler { const { getHermesHome } = await import('../hermes-home.js'); if (await pathExists(getHermesHome())) { const { upsertSoulRules } = await import('../hermes-config.js'); - await upsertSoulRules(await hermesRulesText(rules)); + await upsertSoulRules(await inlinedRulesText(rules)); } } + await this.syncCodexInstructionRules(teamConfig, localConfig, rules, ledger); + // OpenCode does not auto-scan a rules directory: the .md files are inert // until referenced from `instructions` in opencode.json. Activate (or, when // there are no team rules, deactivate) that glob. Runs before the empty-set @@ -587,26 +598,144 @@ export class RulesHandler extends ResourceHandler { const baseDir = resolveToolBaseDir(tool, localConfig); const claudeMdPath = path.join(baseDir, toolPath.claudemd); try { - const content = await readFileSafe(claudeMdPath); - if (!content || !content.includes(TEAMAI_RULES_START)) continue; - const startIdx = content.indexOf(TEAMAI_RULES_START); - const endIdx = content.indexOf(TEAMAI_RULES_END); - if (startIdx === -1 || endIdx === -1) continue; - const before = content.substring(0, startIdx).replace(/\n+$/, '\n'); - const after = content.substring(endIdx + TEAMAI_RULES_END.length).replace(/^\n+/, '\n'); - const newContent = (before + after).trim(); - if (newContent.length === 0) { - await remove(claudeMdPath); - } else { - await writeFile(claudeMdPath, newContent + '\n'); + if (await removeClaudeMdSection(claudeMdPath, TEAMAI_RULES_START, TEAMAI_RULES_END, { deleteIfEmpty: true })) { + log.debug(`Removed legacy rules section from ${claudeMdPath}`); } - log.debug(`Removed legacy rules section from ${claudeMdPath}`); } catch { // Best-effort cleanup } } } + /** + * The Codex family's part of a rules sync: the team rules go into its + * user-scope AGENTS.md, which only it reads; in a project its session-start + * hook adds them (#938). The copies earlier pulls left in its rules + * directory are reclaimed. Public so the "Already synced" pull can run it + * after a CLI upgrade. + */ + async syncCodexInstructionRules( + teamConfig: TeamaiConfig, + localConfig: LocalConfig, + rules: ResourceItem[], + ledger?: DeliveryLedger, + ): Promise { + const block = await teamRulesBlock(rules); + for (const [tool, toolPath] of Object.entries(scopedToolPaths(teamConfig, localConfig))) { + if (!writesInstructionBlock(tool, toolPath, 'team-rules')) continue; + const file = path.join(resolveToolBaseDir(tool, localConfig), toolPath.claudemd); + const probe = instructionFileInstallProbe(tool, toolPath); + const active = !isAgentExcluded(localConfig, tool) + && (probe === undefined || await isToolInstalledForConfig(tool, probe, localConfig)); + try { + if (active && block !== null) { + await injectClaudeMdSection(file, TEAMAI_TEAM_RULES_START, TEAMAI_TEAM_RULES_END, block); + } else { + // A file teamai created for the block alone goes with it. + await removeClaudeMdSection(file, TEAMAI_TEAM_RULES_START, TEAMAI_TEAM_RULES_END, { deleteIfEmpty: true }); + } + } catch (e) { + log.warn(`Failed to update team rules in ${file}: ${(e as Error).message}`); + } + } + await this.reclaimLegacyRuleCopies(teamConfig, localConfig, ledger); + } + + /** + * Remove the `.md` copies earlier pulls wrote to a rules directory the + * tool never read (`LEGACY_RULE_DIRS`) that are still teamai's + * (`legacyRuleCopies`). The copies kept are named in one warning per rules + * sync, until the member deletes them. Public so the "Already synced" pull + * can run it after a CLI upgrade without a full sync (#938). + */ + async reclaimLegacyRuleCopies( + teamConfig: TeamaiConfig, + localConfig: LocalConfig, + ledger: DeliveryLedger | undefined, + ): Promise { + const kept: string[] = []; + for (const { tool, dir, owned, edited } of await this.legacyRuleCopies(teamConfig, localConfig, ledger?.previous)) { + for (const file of owned) { + await remove(file); + if (ledger) forgetDelivered(ledger.hashes, file); + log.debug(`Removed ${file}: ${tool} gets the team rules from its session-start hook`); + } + kept.push(...edited); + // Codex's own `*.rules` keep the directory; only an emptied one goes. + if (owned.length > 0) await pruneEmptyDirs(dir); + } + if (kept.length > 0) { + log.warn( + `Kept ${kept.join(', ')}: teamai could not verify that ${kept.length === 1 ? 'it matches' : 'they match'} what it delivered there, ` + + 'and Codex does not read .md files in its rules directory (team rules now reach it through its session-start hook). ' + + 'Delete what you did not edit; to keep your changes, move them into AGENTS.md outside the teamai markers, ' + + 'then delete the copy.', + ); + } + } + + /** + * The `.md` copies earlier pulls wrote to each rules directory a tool + * never read (`LEGACY_RULE_DIRS`), split into the ones still teamai's and the + * ones the member edited. A copy is teamai's while it holds what teamai + * delivered there: the team rule verbatim, now or at a revision this + * checkout pulled, or as `previous` (the delivery ledger) recorded it; the + * built-in `teamai-recall.md` as any teamai version deployed it. Every team + * rule counts, not just the ones delivered here, since a copy outlives the + * role or tag that selected it. A copy of a rule the team removed is teamai's + * only while it matches its recorded delivery hash. Without that record, + * the copy stays because it may contain the member's edits. A directory a team + * `toolPaths` still delivers rules to is not listed. + * + * Read-only and public so `uninstall` removes exactly what a pull reclaims. + */ + async legacyRuleCopies( + teamConfig: TeamaiConfig, + localConfig: LocalConfig, + previous: DeliveredHashes | undefined, + ): Promise> { + const teamRules = await this.scanTeamForPull(teamConfig, localConfig); + const tombstoned = [...await this.readTombstones(localConfig)] + .filter((name) => !teamRules.some((rule) => rule.name === name)); + let deliveredRevs: readonly string[] | undefined; + const out: Array<{ tool: string; dir: string; owned: string[]; edited: string[] }> = []; + // A team `toolPaths` that still names one of these dirs delivers there. + const deliveredDirs = new Set( + Object.entries(scopedToolPaths(teamConfig, localConfig)) + .filter(([, toolPath]) => toolPath.rules) + .map(([tool, toolPath]) => path.join(resolveToolBaseDir(tool, localConfig), toolPath.rules!)), + ); + for (const [tool, rel] of Object.entries(LEGACY_RULE_DIRS)) { + const dir = path.join(resolveToolBaseDir(tool, localConfig), rel); + if (deliveredDirs.has(dir) || !await pathExists(dir)) continue; + const owned: string[] = []; + const edited: string[] = []; + for (const rule of teamRules) { + const file = path.join(dir, `${rule.name}.md`); + if (!await pathExists(file)) continue; + deliveredRevs ??= ( + await (await import('../pull.js')).resolveCheckoutBases(localConfig, await loadStateForScope(localConfig)) + ).revs; + const recorded = previous?.[file]; + const delivered = (recorded !== undefined && recorded === await fileHash(file)) + || await isDeliveredRender(tool, file, rule, localConfig.repo.localPath, deliveredRevs); + (delivered ? owned : edited).push(file); + } + // The source is gone, so only a recorded hash proves a copy is unchanged. + for (const name of tombstoned) { + const file = path.join(dir, `${name}.md`); + if (!await pathExists(file)) continue; + const recorded = previous?.[file]; + (recorded !== undefined && recorded === await fileHash(file) ? owned : edited).push(file); + } + const recall = path.join(dir, 'teamai-recall.md'); + const recallContent = await readFileSafe(recall); + if (recallContent !== null) (isDeployedRecallRule(recallContent) ? owned : edited).push(recall); + out.push({ tool, dir, owned, edited }); + } + return out; + } + /** * Add or remove the teamai rules glob in OpenCode's opencode.json `instructions` * array, so copied rule files are actually loaded. No-op for any tool other than @@ -756,18 +885,64 @@ async function isDeliveredRender( } /** - * The text `upsertSoulRules` inlines into the teamai block of Hermes SOUL.md. + * The team rules as one text, for a tool with no rules directory: Hermes, + * whose SOUL.md block pull writes and `doctor` compares, and the Codex + * family, whose session-start hook adds it (`teamRulesContext`). * - * Hermes reads standing instructions from one file rather than a rules - * directory, so its rules are delivered as this block's contents. `doctor` - * compares what is in the block with this, the same way it compares a rule - * file with its render. + * Frontmatter is dropped. Neither can scope a rule to paths, so a path-scoped + * rule is always on, led by a line naming its globs. */ -export async function hermesRulesText(rules: ResourceItem[]): Promise { +export async function inlinedRulesText(rules: ResourceItem[]): Promise { const bodies: string[] = []; for (const rule of rules) { - const body = await readFileSafe(rule.sourcePath); - if (body && body.trim() !== '') bodies.push(body.trim()); + const content = await readFileSafe(rule.sourcePath); + if (!content) continue; + const { data, body } = splitFrontmatter(content); + if (body.trim() === '') continue; + const paths = rulePaths(data); + const scope = paths.length > 0 ? `Applies to files matching: ${paths.join(', ')}\n` : ''; + bodies.push(`${scope}${body.trim()}`); } return bodies.join('\n\n'); } + +/** + * The team-rules block for a tool's user-scope instructions file (the Codex + * family, #938), markers included. Null when no rule has a body. The body is + * the same render Hermes gets in SOUL.md. + */ +export async function teamRulesBlock(rules: ResourceItem[]): Promise { + // A marker anywhere in a rule body would cut the block short on the next + // read, which finds the markers by substring. A line that held only one goes. + const body = (await inlinedRulesText(rules)) + .split('\n') + .flatMap((line) => { + const cleaned = line.replaceAll(TEAMAI_TEAM_RULES_START, '').replaceAll(TEAMAI_TEAM_RULES_END, ''); + return cleaned !== line && cleaned.trim() === '' ? [] : [cleaned]; + }) + .join('\n') + .trim(); + if (body === '') return null; + return [ + TEAMAI_TEAM_RULES_START, + '', + '', + body, + '', + TEAMAI_TEAM_RULES_END, + ].join('\n'); +} + +/** + * The team rules a tool with no rules format gets from its session-start hook + * in a project (the Codex family, #938): the rules this member receives there, + * as pull resolves them, in the same render as Hermes' SOUL.md. Null when no + * rule has a body. User-scope rules reach it through its own instructions + * file instead. + */ +export async function teamRulesContext(teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise { + const { buildRolePullContext, resolveDesiredRules } = await import('./desired.js'); + const { items } = await resolveDesiredRules(teamConfig, localConfig, await buildRolePullContext(localConfig)); + const text = await inlinedRulesText(items); + return text === '' ? null : `Team rules (from teamai):\n\n${text}`; +} diff --git a/src/types.ts b/src/types.ts index 52afe5681..0a3096dbf 100644 --- a/src/types.ts +++ b/src/types.ts @@ -323,8 +323,28 @@ export const TeamaiConfigSchema = z.object({ // wrong guess can never create a junk config file on a user's machine. toolPaths: z.record(z.string(), ToolPathsSchema).default({ claude: { skills: '.claude/skills', rules: '.claude/rules', settings: '.claude/settings.json', claudemd: '.claude/CLAUDE.md', agents: '.claude/agents', mcp: '.claude.json', mcpProject: '.mcp.json' }, - codex: { skills: '.codex/skills', rules: '.codex/rules', settings: '.codex/hooks.json', agents: '.codex/agents', mcp: '.codex/config.toml' }, - 'codex-internal': { skills: '.codex-internal/skills', rules: '.codex-internal/rules', settings: '.codex-internal/hooks.json', agents: '.codex-internal/agents' }, + // Codex reads no rules directory: `.codex/rules` holds its exec-policy + // `*.rules` files. In user scope teamai's blocks go to ~/.codex/AGENTS.md, + // which only Codex reads. In project scope they come from the session-start + // hook, since the project AGENTS.md is the owners' file and other tools + // read it too (#938, #945); so the entry has no project `claudemd`. + codex: { + skills: '.codex/skills', + settings: '.codex/hooks.json', + agents: '.codex/agents', + mcp: '.codex/config.toml', + userScope: { claudemd: '.codex/AGENTS.md' }, + }, + // codex-internal and tcodex run the same Codex from their own home root, so + // they take the codex shape. Their user-scope AGENTS.md locations + // (~/.codex-internal/AGENTS.md, ~/.tcodex/AGENTS.md) are assumed from that, + // not verified against either build. + 'codex-internal': { + skills: '.codex-internal/skills', + settings: '.codex-internal/hooks.json', + agents: '.codex-internal/agents', + userScope: { claudemd: '.codex-internal/AGENTS.md' }, + }, 'claude-internal': { skills: '.claude-internal/skills', rules: '.claude-internal/rules', settings: '.claude-internal/settings.json', claudemd: '.claude-internal/CLAUDE.md', agents: '.claude-internal/agents' }, // tclaude ships Claude Code with `customUserDataDir: .tclaude`, which // relocates the whole user data dir — so its MCP file is @@ -332,7 +352,13 @@ export const TeamaiConfigSchema = z.object({ // for the Claude family is /.mcp.json, which the `claude` target // already writes and tclaude reads from the same location. tclaude: { skills: '.tclaude/skills', rules: '.tclaude/rules', settings: '.tclaude/settings.json', claudemd: '.tclaude/CLAUDE.md', agents: '.tclaude/agents', mcp: '.tclaude/.claude.json' }, - tcodex: { skills: '.tcodex/skills', rules: '.tcodex/rules', settings: '.tcodex/hooks.json', agents: '.tcodex/agents' }, + // Same shape as codex; see codex-internal above. + tcodex: { + skills: '.tcodex/skills', + settings: '.tcodex/hooks.json', + agents: '.tcodex/agents', + userScope: { claudemd: '.tcodex/AGENTS.md' }, + }, cursor: { skills: '.cursor/skills', rules: '.cursor/rules', settings: '.cursor/hooks.json', agents: '.cursor/agents', mcp: '.cursor/mcp.json', mcpProject: '.cursor/mcp.json' }, // GitHub Copilot CLI keeps project customizations under .github and moves // the complete user customization root when COPILOT_HOME is set. Agents use @@ -880,6 +906,11 @@ export interface HookDef { command: string; /** Per-hook timeout in seconds (tool-specific; omitted = tool default). */ timeout?: number; + /** + * Codex only: the token count past which Codex keeps just the start and end + * of the hook's additionalContext. 0 turns that off. Omitted = Codex's 2,500. + */ + additionalContextLimit?: number; /** settings.json description. builtin: "[teamai] "; team: "[teamai:hook:] ...". */ description: string; /** Team hooks only: restrict to these tools (default = all hook-capable tools). */ @@ -1093,6 +1124,13 @@ export const TEAMAI_AGENT_HOOK_PREFIX = '[teamai:agent-hook:'; export const TEAMAI_ENV_START = '# [teamai:env:start]'; export const TEAMAI_ENV_END = '# [teamai:env:end]'; +/** + * The team rules inlined into the user-scope instructions file of a tool with + * no rules format (the Codex family, #938). Not `[teamai:rules]`: pull strips + * that legacy marker from every `claudemd` file. + */ +export const TEAMAI_TEAM_RULES_START = ''; +export const TEAMAI_TEAM_RULES_END = ''; export const TEAMAI_CULTURE_START = ''; export const TEAMAI_CULTURE_END = ''; diff --git a/src/uninstall.ts b/src/uninstall.ts index 2338ae36d..301fb70ac 100644 --- a/src/uninstall.ts +++ b/src/uninstall.ts @@ -16,6 +16,8 @@ import { TEAMAI_CLAUDEMD_END, TEAMAI_RECALL_RULES_START, TEAMAI_RECALL_RULES_END, + TEAMAI_TEAM_RULES_START, + TEAMAI_TEAM_RULES_END, getDataHome, getManagedHooksPath, isAgentExcluded, @@ -32,11 +34,14 @@ import { type ManagedMcpManifest, } from './types.js'; import { BUILTIN_RULE_NAMES } from './builtin-rules.js'; -import { ruleStemFromFilename } from './resources/rule-format.js'; +import { ruleStemFromFilename, writesInstructionBlock, type InstructionBlock } from './resources/rule-format.js'; import { agentStemFromFilename } from './resources/agent-format.js'; import { resolveDocsDestination } from './resources/docs.js'; import { listTeamAgentDirs } from './resources/agents.js'; +import { RulesHandler } from './resources/rules.js'; +import { deliveredHashes } from './pull.js'; import { isToolInstalledForConfig } from './resources/base.js'; +import { removeClaudeMdSection } from './utils/claudemd.js'; import { BUILTIN_AGENT_NAMES } from './builtin-agents.js'; import { BUILTIN_SKILL_NAMES, @@ -93,8 +98,8 @@ interface RemovalPlan { dshHookFile: string | null; /** Manifest used by the primary hook injection scope. */ hookManifestPath: string; - /** CLAUDE.md files with teamai rules blocks. */ - claudeMdFiles: string[]; + /** Instruction files (CLAUDE.md, AGENTS.md, …), each with the teamai blocks to strip from it. */ + claudeMdFiles: Array<{ path: string; blocks: Array<[string, string]> }>; /** * Skill directories synced from team repo, each with the base directory its * skills root hangs off: the prune refuses a link anywhere below that base. @@ -102,6 +107,8 @@ interface RemovalPlan { skillDirs: SkillDirEntry[]; /** Rule .md files synced from team repo (plus CLI built-in rules). */ ruleFiles: string[]; + /** Copies in a tool's legacy rules directory the member edited: never removed, only named. */ + keptRuleFiles: string[]; /** Built-in agent .md files deployed by the CLI (e.g. teamai-recall). */ agentFiles: string[]; /** teamai-managed MCP servers from managed-mcp.json (`tool/server` or `tool:project/server`). */ @@ -146,6 +153,7 @@ interface ToolResources { claudeMdFiles: string[]; skillDirs: SkillDirEntry[]; ruleFiles: string[]; + keptRuleFiles: string[]; agentFiles: string[]; } @@ -171,8 +179,27 @@ const CLAUDEMD_MARKER_PAIRS: Array<[string, string]> = [ [TEAMAI_CULTURE_START, TEAMAI_CULTURE_END], [TEAMAI_CLAUDEMD_START, TEAMAI_CLAUDEMD_END], [TEAMAI_RECALL_RULES_START, TEAMAI_RECALL_RULES_END], + [TEAMAI_TEAM_RULES_START, TEAMAI_TEAM_RULES_END], ]; +const INSTRUCTION_BLOCK_STARTS: Record = { + culture: TEAMAI_CULTURE_START, + claudemd: TEAMAI_CLAUDEMD_START, + recall: TEAMAI_RECALL_RULES_START, + 'team-rules': TEAMAI_TEAM_RULES_START, +}; + +/** + * Start markers of the blocks a pull writes into a tool's `claudemd` file, + * as pull's writers decide it (`writesInstructionBlock`). Nobody writes the + * legacy `[teamai:rules]` block any more. + */ +function instructionBlocksWrittenBy(tool: string, toolPath: TeamaiConfig['toolPaths'][string]): string[] { + return (Object.keys(INSTRUCTION_BLOCK_STARTS) as InstructionBlock[]) + .filter((block) => writesInstructionBlock(tool, toolPath, block)) + .map((block) => INSTRUCTION_BLOCK_STARTS[block]); +} + /** * Collect team repo skill names, handling both flat and namespaced layouts. * A directory is a namespace if it does NOT contain SKILL.md. @@ -285,7 +312,7 @@ async function discoverToolResources( ): Promise { const res: ToolResources = { hookFiles: [], openclawHookDirs: [], opencodeHookScopes: [], ompHookFile: null, piHookFiles: [], dshHookFile: null, - claudeMdFiles: [], skillDirs: [], ruleFiles: [], agentFiles: [], + claudeMdFiles: [], skillDirs: [], ruleFiles: [], keptRuleFiles: [], agentFiles: [], }; // (a) Hooks — settings.json / hooks.json @@ -454,6 +481,7 @@ async function discoverToolResources( // (d) Rules — team-synced rules plus CLI built-in rules (teamRuleNames // now includes BUILTIN_RULE_NAMES). User-authored rules are left alone. + // A legacy rules directory is buildRemovalPlan's: its copies go on ownership. if (toolPath.rules) { const rulesDir = path.join(baseDir, toolPath.rules); if (await pathExists(rulesDir)) { @@ -548,8 +576,9 @@ async function buildRemovalPlan( // agents and CLAUDE.md stay on the config-scope paths below — those are real // project resources. const hookToolPaths = scopedToolPaths(teamConfig, { ...localConfig, scope: primaryHookScope.scope }); + const toolPaths = scopedToolPaths(teamConfig, localConfig); const perTool = new Map(); - for (const [tool, toolPath] of Object.entries(scopedToolPaths(teamConfig, localConfig))) { + for (const [tool, toolPath] of Object.entries(toolPaths)) { perTool.set( tool, await discoverToolResources( @@ -568,6 +597,19 @@ async function buildRemovalPlan( ); } + // (d) continued: the copies a release made in a tool's legacy rules + // directory, before its rules moved into its instructions file, which a pull + // may not have reclaimed yet. A name is no proof there: only the copies a + // pull would reclaim go, and the ones the member edited stay, named. + const legacyCopies = await new RulesHandler() + .legacyRuleCopies(teamConfig, localConfig, await deliveredHashes(localConfig)); + for (const { tool, owned, edited } of legacyCopies) { + const res = perTool.get(tool); + if (!res) continue; + res.ruleFiles.push(...owned); + res.keptRuleFiles.push(...edited); + } + // A tool only still "uses" a shared resource (AGENTS.md, .teamai/) if it is // actually enabled and installed. Several tools default to the same shared // path — e.g. Hermes/WorkBuddy default to the same project AGENTS.md as Pi — @@ -576,7 +618,7 @@ async function buildRemovalPlan( // tool-specific root (skills/rules/settings), never `claudemd`: that's // exactly the shared, ambiguous path this check exists to disambiguate. const activeTools = new Set(); - for (const [tool, toolPath] of Object.entries(scopedToolPaths(teamConfig, localConfig))) { + for (const [tool, toolPath] of Object.entries(toolPaths)) { if (isAgentExcluded(localConfig, tool)) continue; const probePath = toolPath.skills ?? toolPath.rules ?? toolPath.settings ?? toolPath.claudemd; if (probePath && await isToolInstalledForConfig(tool, probePath, localConfig)) { @@ -614,6 +656,7 @@ async function buildRemovalPlan( claudeMdFiles: [], skillDirs: [], ruleFiles: [], + keptRuleFiles: [], agentFiles: [], mcpServers: [], shellProfiles: [], @@ -628,14 +671,18 @@ async function buildRemovalPlan( }; // A single instruction file can be the native target for several agents - // (for example project `AGENTS.md` is shared by Pi, Hermes, and WorkBuddy). - // Keep its TeamAI blocks when another enabled, installed agent still - // references the same file; a targeted uninstall must not remove - // instructions owned by that remaining agent. - const retainedInstructionFiles = new Set(); + // (for example project `AGENTS.md` is shared by Pi, Hermes, WorkBuddy, Codex). + // Keep a TeamAI block when another enabled, installed agent that maps the + // same file would write that block; the rest go, since no remaining agent's + // pull would ever refresh or remove them. + const retainedBlocks = new Map>(); for (const [tool, resources] of perTool) { - if (!toolsToMerge.includes(tool) && activeTools.has(tool)) { - for (const file of resources.claudeMdFiles) retainedInstructionFiles.add(file); + if (toolsToMerge.includes(tool) || !activeTools.has(tool)) continue; + const written = instructionBlocksWrittenBy(tool, toolPaths[tool]); + for (const file of resources.claudeMdFiles) { + const kept = retainedBlocks.get(file) ?? new Set(); + for (const start of written) kept.add(start); + retainedBlocks.set(file, kept); } } @@ -650,12 +697,16 @@ async function buildRemovalPlan( plan.piHookFiles.push(...res.piHookFiles); if (res.dshHookFile) plan.dshHookFile = res.dshHookFile; for (const file of res.claudeMdFiles) { - if (!retainedInstructionFiles.has(file) && !plan.claudeMdFiles.includes(file)) { - plan.claudeMdFiles.push(file); - } + if (plan.claudeMdFiles.some((entry) => entry.path === file)) continue; + const content = await readFileSafe(file) ?? ''; + const kept = retainedBlocks.get(file); + const blocks = CLAUDEMD_MARKER_PAIRS + .filter(([start]) => content.includes(start) && !kept?.has(start)); + if (blocks.length > 0) plan.claudeMdFiles.push({ path: file, blocks }); } plan.skillDirs.push(...res.skillDirs); plan.ruleFiles.push(...res.ruleFiles); + plan.keptRuleFiles.push(...res.keptRuleFiles); plan.agentFiles.push(...res.agentFiles); } @@ -812,8 +863,8 @@ function printSummary(plan: RemovalPlan, agentFilter?: string): void { } if (plan.claudeMdFiles.length > 0) { - console.log(` CLAUDE.md rule blocks (${plan.claudeMdFiles.length} files):`); - for (const p of plan.claudeMdFiles) { + console.log(` Instruction-file blocks (${plan.claudeMdFiles.length} files):`); + for (const { path: p } of plan.claudeMdFiles) { console.log(` ${p}`); } console.log(''); @@ -988,30 +1039,16 @@ async function executeRemoval(plan: RemovalPlan): Promise { } // (b) Clean CLAUDE.md teamai section blocks - for (const claudeMdPath of plan.claudeMdFiles) { + for (const { path: claudeMdPath, blocks } of plan.claudeMdFiles) { try { - const raw = await readFileSafe(claudeMdPath); - if (!raw) continue; - - let content: string = raw; - for (const [startMarker, endMarker] of CLAUDEMD_MARKER_PAIRS) { - const startIdx = content.indexOf(startMarker); - const endIdx = content.indexOf(endMarker); - if (startIdx === -1 || endIdx === -1) continue; - - const before = content.substring(0, startIdx).replace(/\n+$/, '\n'); - const after = content.substring(endIdx + endMarker.length).replace(/^\n+/, '\n'); - content = (before + after).trim(); + // A file teamai created goes with its last block; a member's file, + // even an empty one, stays. + for (const [startMarker, endMarker] of blocks) { + await removeClaudeMdSection(claudeMdPath, startMarker, endMarker, { deleteIfEmpty: true }); } - - if (content.length === 0) { - await remove(claudeMdPath); - } else { - await writeFile(claudeMdPath, content + '\n'); - } - log.success(`Cleaned CLAUDE.md: ${claudeMdPath}`); + log.success(`Cleaned ${claudeMdPath}`); } catch (e) { - log.warn(`Failed to clean CLAUDE.md ${claudeMdPath}: ${(e as Error).message}`); + log.warn(`Failed to clean ${claudeMdPath}: ${(e as Error).message}`); } } @@ -1175,6 +1212,15 @@ export async function uninstall(opts: UninstallOptions): Promise { agentKey = matched; // normalize to canonical toolPaths key } const plan = await buildRemovalPlan(localConfig, teamConfig, agentKey); + // Uninstall never removes these, so they are named whatever happens next. + if (plan.keptRuleFiles.length > 0) { + const one = plan.keptRuleFiles.length === 1; + log.warn( + `Kept ${plan.keptRuleFiles.join(', ')}: teamai could not verify that ${one ? 'it matches' : 'they match'} what it delivered there. ` + + 'Codex does not read .md files in its rules directory; ' + + `delete ${one ? 'it' : 'them'} once you have saved what you need.`, + ); + } if (isPlanEmpty(plan)) { log.info('Nothing to uninstall'); diff --git a/src/utils/claudemd.ts b/src/utils/claudemd.ts index 5fab0c767..e9de56f45 100644 --- a/src/utils/claudemd.ts +++ b/src/utils/claudemd.ts @@ -1,4 +1,4 @@ -import { readFileSafe, writeFile } from './fs.js'; +import { readFileSafe, remove, writeFile } from './fs.js'; /** * Inject or replace a marker-delimited section in a CLAUDE.md file. @@ -7,7 +7,8 @@ import { readFileSafe, writeFile } from './fs.js'; * - Both markers present → replace content between them (inclusive). * - Neither marker present → append the block to the end of the file. * - Only one marker present (corrupted) → append the block (safe fallback). - * - File does not exist → create it with just the block. + * - File does not exist → create it with just the block, which is how + * `removeClaudeMdSection` tells a file teamai created from a member's. * * @param filePath Absolute path to the CLAUDE.md file. * @param startMarker Opening marker comment, e.g. ``. @@ -20,7 +21,11 @@ export async function injectClaudeMdSection( endMarker: string, block: string, ): Promise { - const existing = await readFileSafe(filePath) ?? ''; + const existing = await readFileSafe(filePath); + if (existing === null) { + await writeFile(filePath, block + '\n'); + return; + } const startIdx = existing.indexOf(startMarker); const endIdx = existing.indexOf(endMarker); @@ -37,20 +42,37 @@ export async function injectClaudeMdSection( await writeFile(filePath, updated); } -/** Remove a marker-delimited managed section while preserving user content. */ +/** + * Remove a marker-delimited managed section while preserving user content. + * Returns whether a section was removed. With `deleteIfEmpty`, a file left + * holding only whitespace is deleted when it opens with the section, as the + * file `injectClaudeMdSection` created does; a member's file that was empty + * before the section was appended is left empty instead. A file without the + * section is never touched. + */ export async function removeClaudeMdSection( filePath: string, startMarker: string, endMarker: string, -): Promise { + options: { deleteIfEmpty?: boolean } = {}, +): Promise { const existing = await readFileSafe(filePath); - if (!existing) return; + if (!existing) return false; const startIdx = existing.indexOf(startMarker); const endIdx = existing.indexOf(endMarker); - if (startIdx === -1 || endIdx === -1 || endIdx < startIdx) return; + if (startIdx === -1 || endIdx === -1 || endIdx < startIdx) return false; const before = existing.substring(0, startIdx).replace(/\n+$/, '\n'); - const after = existing.substring(endIdx + endMarker.length).replace(/^\n+/, '\n'); - await writeFile(filePath, (before + after).trimEnd() + '\n'); + // A file that opened with this section opens with whatever follows it, so + // a teamai-created file still reads as teamai's when its next section goes. + const after = existing.substring(endIdx + endMarker.length).replace(/^\n+/, startIdx === 0 ? '' : '\n'); + const rest = (before + after).trimEnd(); + if (options.deleteIfEmpty && rest.trim() === '') { + if (startIdx === 0) await remove(filePath); + else await writeFile(filePath, ''); + } else { + await writeFile(filePath, rest + '\n'); + } + return true; }