From 537859a80be04da57565d2fad764b91490abec93 Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 1 Oct 2026 14:30:07 +0200 Subject: [PATCH] fix(rules): address Codex delivery review findings --- CHANGELOG.md | 2 +- docs/usage-guide.md | 6 +-- docs/usage-guide.zh-CN.md | 6 +-- skill-data/setup/references/uninstall.md | 4 +- src/__tests__/codex-hook-rules.test.ts | 29 +++++++++++++ src/__tests__/codex-legacy-rules.test.ts | 55 ++++++++++++++++++++++++ src/__tests__/doctor.test.ts | 13 +++++- src/__tests__/uninstall.test.ts | 42 ++++++++++++++++++ src/doctor.ts | 5 +-- src/pull.ts | 7 +-- src/resources/rules.ts | 40 ++++++++++------- 11 files changed, 177 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1777ef717..32dd805fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,7 +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)). +- 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. The hook omits blocks already present in the active project instructions file, reading `AGENTS.override.md` before `AGENTS.md`. Pull removes the `.md` copies earlier pulls left in the Codex rules directory at its recorded `toolRoots` location, including a publisher's bare local copy, 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 76c846c8e..7ab634a9e 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -962,11 +962,11 @@ teamai push 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. +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. The hook leaves out a block already present in the active project instructions file. Codex reads `AGENTS.override.md` when it exists, otherwise `AGENTS.md`, so a block in a shadowed `AGENTS.md` still reaches Codex through the hook. > 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. +> Upgrading from a release that copied rules to `.codex/rules/`: the next `pull` removes the `.md` copies teamai delivered there, including `teamai-recall.md`. Cleanup follows the recorded `toolRoots` location and checks both a publisher's bare local filename and its namespaced copy. 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 @@ -2762,7 +2762,7 @@ What gets removed: - teamai hooks in AI tool settings - 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, 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 rules, including the copies older releases left in `.codex/rules/`, also of rules the team has since removed. Cleanup follows the recorded `toolRoots` location and the publisher's local filenames. 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 diff --git a/docs/usage-guide.zh-CN.md b/docs/usage-guide.zh-CN.md index 01ac7f4e4..623eee1e5 100644 --- a/docs/usage-guide.zh-CN.md +++ b/docs/usage-guide.zh-CN.md @@ -883,11 +883,11 @@ teamai push 大多数工具在自己的 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 就不再重复添加这个区块。 +culture、共享指令和 recall 区块采用同样的划分。user scope 下它们写入同一个 `AGENTS.md`，标记之外你自己的内容保持不变。在项目中，session-start hook 把它们与 rule 一起加入会话，`pull` 不改动项目 `AGENTS.md`。hook 会省略项目当前生效的指令文件中已有的区块。Codex 在 `AGENTS.override.md` 存在时读取它，否则读取 `AGENTS.md`，因此被遮蔽的 `AGENTS.md` 中的区块仍会通过 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。 +> 从把 rule 复制到 `.codex/rules/` 的旧版本升级后，下一次 `pull` 会删除 teamai 投递到那里的 `.md` 副本，包括 `teamai-recall.md`。清理使用记录的 `toolRoots` 位置，同时检查发布者本地的无命名空间文件名及命名空间副本。你改过的副本会保留，并在警告中点名；`*.rules` 文件从不改动。团队此后已删除的 rule，其副本只有与记录的投递哈希一致时才会删除；没有该记录时也会保留并点名。同一次 pull 会为 `hooks.json` 中的 teamai hook 加上 `additionalContextLimit: 0` 和一个 `SubagentStart` 条目，因此公开版 Codex 会请你批准一次改动后的 hook。 ### Env、hooks 与 MCP server 按 namespace 划分 @@ -2573,7 +2573,7 @@ teamai uninstall --agent claude - AI 工具 settings 中的 teamai hooks - CLAUDE.md 和 AGENTS.md 中的 teamai 块（保留用户自写内容） - 团队同步的 skills，包括 OpenClaw workspace skills（保留用户自建 skills） -- 团队同步的 rules，包括旧版本留在 `.codex/rules/` 中的副本，团队此后已删除的 rule 的副本也包括在内。其中你改过的副本会保留，并在警告中点名。已删除 rule 的副本只有与记录的投递哈希一致时才会删除；没有该记录时也会保留并点名。Codex 的 `*.rules` 文件保留 +- 团队同步的 rules，包括旧版本留在 `.codex/rules/` 中的副本，团队此后已删除的 rule 的副本也包括在内。清理使用记录的 `toolRoots` 位置和发布者本地的文件名。其中你改过的副本会保留，并在警告中点名。已删除 rule 的副本只有与记录的投递哈希一致时才会删除；没有该记录时也会保留并点名。Codex 的 `*.rules` 文件保留 - 团队同步的自定义 agents 和 CLI 内置 agents（保留用户自建 agents） - Shell profile 中的 env 块——会清理每一个候选文件（`.zshrc`、`.bashrc`、`.bash_profile`、`.bash_login`、`.profile`）中、代码块指向本作用域自身 `env.sh` 的那些，而不仅仅是当前 `pull` 会选中的那一个；指向其他作用域 `env.sh` 的代码块不受影响 - `~/.teamai/` 目录 diff --git a/skill-data/setup/references/uninstall.md b/skill-data/setup/references/uninstall.md index 2f3beaa6c..0495129fd 100644 --- a/skill-data/setup/references/uninstall.md +++ b/skill-data/setup/references/uninstall.md @@ -55,7 +55,9 @@ and give it your team repo URL."* ## Notes -- Uninstall keeps edited legacy Codex rule copies. For a rule the team has +- Uninstall cleans legacy Codex rule copies at the recorded `toolRoots` + location, including publishers' bare local filenames. It keeps edited 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. diff --git a/src/__tests__/codex-hook-rules.test.ts b/src/__tests__/codex-hook-rules.test.ts index 5d6a49a43..e1e0d53c8 100644 --- a/src/__tests__/codex-hook-rules.test.ts +++ b/src/__tests__/codex-hook-rules.test.ts @@ -141,6 +141,35 @@ projects: expect(text).toContain('Shared team instructions.'); }); + it.each(['# Owners\n', ''])('adds blocks from a shadowed AGENTS.md when AGENTS.override.md contains %j', async (override) => { + 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 agents = [ + '', 'Be kind to teammates.', '', + '', 'Shared team instructions.', '', + '', 'Team Knowledge Recall (teamai)', '', + ].join('\n'); + await fse.outputFile(path.join(tmpDir, 'project', 'AGENTS.md'), agents); + await fse.writeFile(path.join(tmpDir, 'project', 'AGENTS.override.md'), override); + + 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(await fse.readFile(path.join(tmpDir, 'project', 'AGENTS.md'), 'utf8')).toBe(agents); + expect(await fse.readFile(path.join(tmpDir, 'project', 'AGENTS.override.md'), 'utf8')).toBe(override); + }); + + it('skips a block already present in the active AGENTS.override.md', 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(tmpDir, 'project', 'AGENTS.override.md'), + '\nBe kind to teammates.\n\n'); + + expect(await context({ hook_event_name: 'SessionStart', source: 'startup' })).not.toContain('Be kind to teammates.'); + }); + 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; diff --git a/src/__tests__/codex-legacy-rules.test.ts b/src/__tests__/codex-legacy-rules.test.ts index 4012474aa..bc0994b1a 100644 --- a/src/__tests__/codex-legacy-rules.test.ts +++ b/src/__tests__/codex-legacy-rules.test.ts @@ -155,6 +155,61 @@ describe('pull reclaims the .codex/rules copies earlier pulls wrote (#938)', () expect(log.warn).not.toHaveBeenCalled(); }); + it('reclaims the user-scope legacy copies under the recorded Codex root', async () => { + const root = path.join(homeDir, '.config', 'codex-work'); + localConfig = { ...localConfig, scope: 'user', projectRoot: undefined, toolRoots: { codex: root } } as LocalConfig; + await fse.outputFile(path.join(root, 'rules', 'codeword.md'), 'The team codeword is PELICAN-42.\n'); + await fse.writeFile(path.join(root, 'rules', 'default.rules'), 'prefix_rule(pattern = ["ls"], decision = "allow")\n'); + + await handler.pullAllRules(teamConfig, localConfig); + + expect(await fse.pathExists(path.join(root, 'rules', 'codeword.md'))).toBe(false); + expect(await fse.pathExists(path.join(root, 'rules', 'default.rules'))).toBe(true); + }); + + it.each([false, true])('reclaims a publisher\'s bare and namespaced legacy copies only while unchanged, edited: %s', async (edited) => { + await fse.outputFile(path.join(repoPath, 'rules', 'frontend', 'style.md'), 'Frontend rule.\n'); + vi.mocked(loadStateForScope).mockImplementation(async () => ({ placedRules: { style: 'rules/frontend/style.md' } }) as never); + const content = edited ? 'Frontend rule.\nMy own note.\n' : 'Frontend rule.\n'; + await fse.outputFile(legacy('style.md'), content); + await fse.outputFile(legacy('frontend/style.md'), content); + + await handler.pullAllRules(teamConfig, localConfig); + + for (const file of [legacy('style.md'), legacy('frontend/style.md')]) { + if (edited) expect(await fse.readFile(file, 'utf8')).toBe(content); + else expect(await fse.pathExists(file)).toBe(false); + } + }); + + it('keeps a bare same-name copy without a publisher placement record', async () => { + await fse.outputFile(path.join(repoPath, 'rules', 'frontend', 'style.md'), 'Frontend rule.\n'); + await fse.outputFile(legacy('style.md'), 'Frontend rule.\n'); + + await handler.pullAllRules(teamConfig, localConfig); + + expect(await fse.readFile(legacy('style.md'), 'utf8')).toBe('Frontend rule.\n'); + }); + + it.each([false, true])('reclaims a publisher\'s tombstoned bare copy only while its recorded hash matches, edited: %s', async (edited) => { + await fse.writeFile(path.join(repoPath, 'rules', '.removed'), 'frontend/style\n'); + vi.mocked(loadStateForScope).mockImplementation(async () => ({ placedRules: { style: 'rules/frontend/style.md' } }) as never); + const file = legacy('style.md'); + const content = edited ? 'Frontend rule.\nMy own note.\n' : 'Frontend rule.\n'; + await fse.outputFile(file, content); + const ledger = openLedger({ [file]: crypto.createHash('sha256').update('Frontend rule.\n').digest('hex') }); + + await handler.pullAllRules(teamConfig, localConfig, undefined, [], ledger); + + if (edited) { + expect(await fse.readFile(file, 'utf8')).toBe(content); + expect(log.warn).toHaveBeenCalledWith(expect.stringContaining(file)); + } else { + expect(await fse.pathExists(file)).toBe(false); + expect(ledger.hashes[file]).toBeUndefined(); + } + }); + 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', diff --git a/src/__tests__/doctor.test.ts b/src/__tests__/doctor.test.ts index 9f926ef13..3904724d6 100644 --- a/src/__tests__/doctor.test.ts +++ b/src/__tests__/doctor.test.ts @@ -719,10 +719,19 @@ describe('buildChecks — the Codex team rules hook (#938)', () => { expect(check!.fix).toContain('/hooks'); }); - it('leaves a missing entry to the hooks check', async () => { + it('fails when the teamai SessionStart entry is missing', async () => { const check = await codexCheck(sessionStart({ command: 'my-own-hook' })); - expect(await check!.check()).toBe(true); + expect(await check!.check()).toBe(false); + }); + + it('fails when only a Stop hook is installed', async () => { + const check = await codexCheck(JSON.stringify({ + hooks: { Stop: [{ hooks: [{ type: 'command', command: 'teamai hook-dispatch stop --tool codex' }] }] }, + })); + + expect(await check!.check()).toBe(false); + expect(check!.fix).toContain('SessionStart'); }); it.each([ diff --git a/src/__tests__/uninstall.test.ts b/src/__tests__/uninstall.test.ts index e10d3d5db..20d26ee83 100644 --- a/src/__tests__/uninstall.test.ts +++ b/src/__tests__/uninstall.test.ts @@ -787,6 +787,48 @@ describe('uninstall', () => { return { ...fixture, legacy: (file: string) => path.join(fixture.projectRoot, '.codex', 'rules', file) }; } + it('reclaims legacy rules under a recorded user-scope Codex root', async () => { + const { homeDir, repoPath } = await setupFixture(tmpDir); + vi.stubEnv('HOME', homeDir); + vi.stubEnv('SHELL', '/bin/zsh'); + const root = path.join(homeDir, '.codex-work'); + const localConfig = makeLocalConfig(homeDir, repoPath, { enabledAgents: ['codex'], toolRoots: { codex: root } }); + const defaults = TeamaiConfigSchema.parse({ team: 't', repo: 'owner/repo' }).toolPaths; + mockAutoDetectInit.mockResolvedValue({ localConfig, teamConfig: makeTeamConfig({ toolPaths: { codex: defaults.codex } }) }); + await fse.outputFile(path.join(root, 'rules', 'team-rule.md'), '# Team Rule'); + await fse.writeFile(path.join(root, 'rules', 'default.rules'), 'prefix_rule(pattern=["ls"])'); + + await uninstall({ force: true }); + + expect(await fse.pathExists(path.join(root, 'rules', 'team-rule.md'))).toBe(false); + expect(await fse.pathExists(path.join(root, 'rules', 'default.rules'))).toBe(true); + }); + + it.each([false, true])('reclaims a publisher\'s bare and namespaced legacy copies on uninstall, tombstoned: %s', async (tombstoned) => { + const { legacy, localConfig, projectRoot } = await codexLegacyFixture(); + const rulesDir = path.join(tmpDir, 'team-repo', 'rules'); + if (tombstoned) await fse.writeFile(path.join(rulesDir, '.removed'), 'frontend/style\n'); + else await fse.outputFile(path.join(rulesDir, 'frontend', 'style.md'), 'Frontend rule.\n'); + await fse.outputFile(legacy('style.md'), 'Frontend rule.\n'); + await fse.outputFile(legacy('frontend/style.md'), 'Frontend rule.\n'); + const state = await loadStateForScope(localConfig); + state.placedRules = { style: 'rules/frontend/style.md' }; + state.lastPullByWorkspace = { + [await checkoutKey(projectRoot)]: { + rev: 'old', targets: ['codex'], delivered: { + [legacy('style.md')]: (await fileHash(legacy('style.md')))!, + [legacy('frontend/style.md')]: (await fileHash(legacy('frontend/style.md')))!, + }, + }, + }; + await saveStateForScope(state, localConfig); + + await uninstall({ force: true }); + + expect(await fse.pathExists(legacy('style.md'))).toBe(false); + expect(await fse.pathExists(legacy('frontend/style.md'))).toBe(false); + }); + 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'); diff --git a/src/doctor.ts b/src/doctor.ts index 85ffb0dd9..5892c955c 100644 --- a/src/doctor.ts +++ b/src/doctor.ts @@ -306,8 +306,8 @@ async function buildHookChecks( * 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. + * `additionalContextLimit: 0`. Both start entries must be present: the generic + * hooks check only proves some hook-dispatch command is installed. */ function sessionHookRulesCheck(tool: string, settingsPath: string): Check { return { @@ -315,7 +315,6 @@ function sessionHookRulesCheck(tool: string, settingsPath: string): Check { 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)); }, diff --git a/src/pull.ts b/src/pull.ts index e2b513b5b..f3021a361 100644 --- a/src/pull.ts +++ b/src/pull.ts @@ -2058,8 +2058,8 @@ function managedBlockBody(block: string): string { * 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. + * wrote into the active project instructions file is skipped, since the tool + * reads it too. AGENTS.override.md takes precedence over AGENTS.md. */ export async function sessionInstructionBlocks( teamConfig: TeamaiConfig, @@ -2067,7 +2067,8 @@ export async function sessionInstructionBlocks( tool: string, ): Promise { const projectAgents = localConfig.projectRoot - ? await readFileSafe(path.join(localConfig.projectRoot, 'AGENTS.md')) ?? '' + ? await readFileSafe(path.join(localConfig.projectRoot, 'AGENTS.override.md')) + ?? 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')); diff --git a/src/resources/rules.ts b/src/resources/rules.ts index df8aa4168..8434f4729 100644 --- a/src/resources/rules.ts +++ b/src/resources/rules.ts @@ -3,7 +3,7 @@ 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, pruneEmptyDirs, fileHash } from '../utils/fs.js'; import { log } from '../utils/logger.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 { TEAMAI_RULES_START, TEAMAI_RULES_END, TEAMAI_TEAM_RULES_START, TEAMAI_TEAM_RULES_END, resolveBaseDir, resolveToolBaseDir, resolveToolRootDir, 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 { @@ -706,32 +706,40 @@ export class RulesHandler extends ResourceHandler { .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); + const dir = localConfig.scope === 'user' + ? path.join(resolveToolRootDir(tool, path.dirname(rel), localConfig.toolRoots), path.basename(rel)) + : 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); + // A publisher's copy uses its bare local name; older namespaced copies + // can remain beside it, so check both against the same delivery proof. + for (const name of new Set([rule.name, await this.localNameFor(rule.name, localConfig)])) { + const file = path.join(dir, `${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); + for (const localName of new Set([name, await this.localNameFor(name, localConfig)])) { + const file = path.join(dir, `${localName}.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 }); + out.push({ tool, dir, owned: [...new Set(owned)], edited: [...new Set(edited)].filter((file) => !owned.includes(file)) }); } return out; }