diff --git a/.github/code-review-graph.instruction.md b/.github/code-review-graph.instruction.md deleted file mode 100644 index 0cdcabb2e..000000000 --- a/.github/code-review-graph.instruction.md +++ /dev/null @@ -1,43 +0,0 @@ ---- -applyTo: '**' -description: Use code-review-graph MCP tools for token-efficient codebase exploration and code review instead of built-in file/search tools. ---- - - -## MCP Tools: code-review-graph - -**IMPORTANT: This project has a knowledge graph. ALWAYS use the -code-review-graph MCP tools BEFORE using #tool:read/readFile #tool:search/fileSearch #tool:search/textSearch to explore -the codebase.** The graph is faster, cheaper (fewer tokens), and gives -you structural context (callers, dependents, test coverage) that file -scanning cannot. - -### When to use graph tools FIRST - -- **Exploring code**: `semantic_search_nodes_tool` or `query_graph_tool` instead of Grep -- **Understanding impact**: `get_impact_radius_tool` instead of manually tracing imports -- **Code review**: `detect_changes_tool` + `get_review_context_tool` instead of reading entire files -- **Finding relationships**: `query_graph_tool` with callers_of/callees_of/imports_of/tests_for -- **Architecture questions**: `get_architecture_overview_tool` + `list_communities_tool` - -Fall back to Grep/Glob/Read **only** when the graph doesn't cover what you need. - -### Key Tools - -| Tool | Use when | -| ------ | ---------- | -| `detect_changes_tool` | Reviewing code changes — gives risk-scored analysis | -| `get_review_context_tool` | Need source snippets for review — token-efficient | -| `get_impact_radius_tool` | Understanding blast radius of a change | -| `get_affected_flows_tool` | Finding which execution paths are impacted | -| `query_graph_tool` | Tracing callers, callees, imports, tests, dependencies | -| `semantic_search_nodes_tool` | Finding functions/classes by name or keyword | -| `get_architecture_overview_tool` | Understanding high-level codebase structure | -| `refactor_tool` | Planning renames, finding dead code | - -### Workflow - -1. The graph auto-updates on file changes (via hooks). -2. Use `detect_changes_tool` for code review. -3. Use `get_affected_flows_tool` to understand impact. -4. Use `query_graph_tool` pattern="tests_for" to check coverage. diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 0a9468fc6..b8f24fb6c 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1,11 +1,9 @@ ## MCP Tools: code-review-graph -**IMPORTANT: This project has a knowledge graph. ALWAYS use the -code-review-graph MCP tools BEFORE using Grep/Glob/Read to explore -the codebase.** The graph is faster, cheaper (fewer tokens), and gives -you structural context (callers, dependents, test coverage) that file -scanning cannot. +**This project has a knowledge graph. Start with the code-review-graph +MCP tools to narrow scope, then read the source.** The graph is cheaper than scanning files and +gives you structural context (callers, dependents, test coverage) that file search cannot. ### When to use graph tools FIRST @@ -15,7 +13,15 @@ scanning cannot. - **Finding relationships**: `query_graph_tool` with callers_of/callees_of/imports_of/tests_for - **Architecture questions**: `get_architecture_overview_tool` + `list_communities_tool` -Fall back to Grep/Glob/Read **only** when the graph doesn't cover what you need. +### Verify in the source + +- Narrow scope with the graph, then read the source. Do not change code from graph output alone. +- For any non-trivial change, read the implementation and the relevant tests before concluding. +- Verify the exact source when touching behavior, database logic, migrations, retries, fallbacks, + recovery, or compatibility code. +- When the graph and the source disagree, the source wins. The graph may be stale or may not + model that relationship. +- An empty graph result can mean "not indexed" or "not statically visible", not "does not exist". ### Key Tools @@ -36,3 +42,4 @@ Fall back to Grep/Glob/Read **only** when the graph doesn't cover what you need. 2. Use `detect_changes_tool` for code review. 3. Use `get_affected_flows_tool` to understand impact. 4. Use `query_graph_tool` pattern="tests_for" to check coverage. + diff --git a/AGENTS.md b/AGENTS.md index 418ea2c2c..96d937173 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -86,11 +86,9 @@ bd close # Complete work ## MCP Tools: code-review-graph -**IMPORTANT: This project has a knowledge graph. ALWAYS use the -code-review-graph MCP tools BEFORE using Grep/Glob/Read to explore -the codebase.** The graph is faster, cheaper (fewer tokens), and gives -you structural context (callers, dependents, test coverage) that file -scanning cannot. +**This project has a knowledge graph. Start with the code-review-graph +MCP tools to narrow scope, then read the source.** The graph is cheaper than scanning files and +gives you structural context (callers, dependents, test coverage) that file search cannot. ### When to use graph tools FIRST @@ -100,12 +98,20 @@ scanning cannot. - **Finding relationships**: `query_graph_tool` with callers_of/callees_of/imports_of/tests_for - **Architecture questions**: `get_architecture_overview_tool` + `list_communities_tool` -Fall back to Grep/Glob/Read **only** when the graph doesn't cover what you need. +### Verify in the source + +- Narrow scope with the graph, then read the source. Do not change code from graph output alone. +- For any non-trivial change, read the implementation and the relevant tests before concluding. +- Verify the exact source when touching behavior, database logic, migrations, retries, fallbacks, + recovery, or compatibility code. +- When the graph and the source disagree, the source wins. The graph may be stale or may not + model that relationship. +- An empty graph result can mean "not indexed" or "not statically visible", not "does not exist". ### Key Tools | Tool | Use when | -|------|----------| +| ------ | ---------- | | `detect_changes_tool` | Reviewing code changes — gives risk-scored analysis | | `get_review_context_tool` | Need source snippets for review — token-efficient | | `get_impact_radius_tool` | Understanding blast radius of a change | @@ -121,3 +127,4 @@ Fall back to Grep/Glob/Read **only** when the graph doesn't cover what you need. 2. Use `detect_changes_tool` for code review. 3. Use `get_affected_flows_tool` to understand impact. 4. Use `query_graph_tool` pattern="tests_for" to check coverage. + diff --git a/CHANGELOG.md b/CHANGELOG.md index 771c738ac..3611a04c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,29 @@ ### Fixed +- The generated instruction sections no longer tell agents to always use the + graph before reading source and to fall back to file search only when the + graph misses. Every platform instruction file now carries the same short + guardrails: narrow scope with the graph, read the implementation and its + tests before a non-trivial change, prefer the source when the two disagree, + and treat an empty graph result as possibly unindexed rather than absent. + CONTRIBUTING.md also states what a new platform target must include before + it will be reviewed (#314). +- Reinstalling now upgrades an instruction section written by an older release + instead of skipping the file because the opening marker was already there, + which is what made every wording fix invisible to existing users. Generated + sections carry a closing marker so the block has real boundaries; blocks from + before that marker existed are matched by their exact recorded text, so + anything written around them survives byte for byte. A section someone edited + by hand is left alone and named in the install output instead of being + overwritten, and `install` now reports created, updated and left-alone files + separately (#314, #558). +- `uninstall` now removes an instruction section written by any past release, + not only one written by the running version, so asking for the instructions + to go no longer leaves orphaned text behind. It matches the same recorded + block texts install uses, clears duplicate blocks, keeps the text on both + sides of the block, closes the gap without leaving a run of blank lines, and + still refuses to touch a section that was edited by hand (#314). - C# receiver calls (`Service.StaticCall()`, `obj.Method()`, `obj?.Method()`) now resolve to canonical method nodes using receiver-type and namespace evidence recorded at parse time, so `callers_of`, `get_impact_radius`, and diff --git a/CLAUDE.md b/CLAUDE.md index 47727eb1a..df2527a9a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -169,11 +169,9 @@ bd close # Complete work ## MCP Tools: code-review-graph -**IMPORTANT: This project has a knowledge graph. ALWAYS use the -code-review-graph MCP tools BEFORE using Grep/Glob/Read to explore -the codebase.** The graph is faster, cheaper (fewer tokens), and gives -you structural context (callers, dependents, test coverage) that file -scanning cannot. +**This project has a knowledge graph. Start with the code-review-graph +MCP tools to narrow scope, then read the source.** The graph is cheaper than scanning files and +gives you structural context (callers, dependents, test coverage) that file search cannot. ### When to use graph tools FIRST @@ -183,12 +181,20 @@ scanning cannot. - **Finding relationships**: `query_graph_tool` with callers_of/callees_of/imports_of/tests_for - **Architecture questions**: `get_architecture_overview_tool` + `list_communities_tool` -Fall back to Grep/Glob/Read **only** when the graph doesn't cover what you need. +### Verify in the source + +- Narrow scope with the graph, then read the source. Do not change code from graph output alone. +- For any non-trivial change, read the implementation and the relevant tests before concluding. +- Verify the exact source when touching behavior, database logic, migrations, retries, fallbacks, + recovery, or compatibility code. +- When the graph and the source disagree, the source wins. The graph may be stale or may not + model that relationship. +- An empty graph result can mean "not indexed" or "not statically visible", not "does not exist". ### Key Tools | Tool | Use when | -|------|----------| +| ------ | ---------- | | `detect_changes_tool` | Reviewing code changes — gives risk-scored analysis | | `get_review_context_tool` | Need source snippets for review — token-efficient | | `get_impact_radius_tool` | Understanding blast radius of a change | @@ -204,3 +210,4 @@ Fall back to Grep/Glob/Read **only** when the graph doesn't cover what you need. 2. Use `detect_changes_tool` for code review. 3. Use `get_affected_flows_tool` to understand impact. 4. Use `query_graph_tool` pattern="tests_for" to check coverage. + diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d4b006422..14d235d34 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -80,6 +80,47 @@ If you just need a language for your own repo, you may not need to contribute at 3. Add a sample fixture file in `tests/fixtures/` 4. Add parsing tests in `tests/test_multilang.py` +## Adding a Platform Target + +Every supported AI tool is permanent maintenance surface. Its config path, schema, install merge, +uninstall, and tests all have to keep working on every release. Some existing targets were merged +without any evidence that the integration worked in a released client, and those are the ones that +break. New targets are held to the bar below. + +Start with a platform request issue (https://github.com/tirth8205/code-review-graph/issues/new/choose) +so the client can be discussed before anyone writes code. A pull request that adds a platform will +not be reviewed until it includes all of the following. + +1. A link to the platform's official MCP configuration documentation. Blog posts, forum replies, + and screenshots of a settings dialog are not enough. +2. The exact config file path and the exact schema of a server entry, including which top-level key + holds the servers, whether that value is an object or an array, and whether a `type` field is + required. +3. The entry added through the existing `PLATFORMS` table in `code_review_graph/skills.py`, plus + `_PLATFORM_CHOICES` in `code_review_graph/cli.py`. Use the fields already there: `name`, + `config_path`, `key`, `detect`, `format`, `needs_type`, and where needed `legacy_keys`, + `server_type`, `entry_fields`. If the client needs something the table cannot express, say so in + the pull request and explain why, rather than adding a bespoke code path beside it. +4. Preservation of unrelated user settings. Install must merge only the `code-review-graph` server + entry and leave every other server, key, and top-level setting intact. If the file cannot be + parsed, install must skip it rather than rewrite it. +5. A byte-idempotent reinstall. Running install twice must leave the config file and any generated + instruction file byte for byte identical. +6. A working uninstall in `code_review_graph/uninstall.py` that removes only what install added, + including any legacy keys, and leaves the rest of the file untouched. +7. Lifecycle tests matching the existing ones: an install, reinstall, and uninstall test in + `tests/test_cli_install.py` shaped like `test_copilot_cli_install_reinstall_uninstall_lifecycle`, + and a passing run of the all-platforms sweep in `tests/test_uninstall.py` + (`test_uninstall_removes_mcp_entry_for_every_current_platform_spec`), which every new entry is + automatically subject to. +8. Evidence from a real released client: a screenshot or transcript of an actual session in that + client where a code-review-graph tool is invoked and returns a result. A rendered image of text, + a mockup, or a description of what should happen is not evidence. + +If no maintainer can install and run the client, the request may be declined or left open until +someone who uses it is willing to own it and respond when it breaks. An existing target may also be +removed if it breaks and nobody steps up to fix it. + ## Reporting Issues - Open an issue via the issue forms: https://github.com/tirth8205/code-review-graph/issues/new/choose (bug report, feature request, or platform request — blank issues are disabled) diff --git a/GEMINI.md b/GEMINI.md index 3d7c6cfa1..b8f24fb6c 100644 --- a/GEMINI.md +++ b/GEMINI.md @@ -1,11 +1,9 @@ ## MCP Tools: code-review-graph -**IMPORTANT: This project has a knowledge graph. ALWAYS use the -code-review-graph MCP tools BEFORE using Grep/Glob/Read to explore -the codebase.** The graph is faster, cheaper (fewer tokens), and gives -you structural context (callers, dependents, test coverage) that file -scanning cannot. +**This project has a knowledge graph. Start with the code-review-graph +MCP tools to narrow scope, then read the source.** The graph is cheaper than scanning files and +gives you structural context (callers, dependents, test coverage) that file search cannot. ### When to use graph tools FIRST @@ -15,12 +13,20 @@ scanning cannot. - **Finding relationships**: `query_graph_tool` with callers_of/callees_of/imports_of/tests_for - **Architecture questions**: `get_architecture_overview_tool` + `list_communities_tool` -Fall back to Grep/Glob/Read **only** when the graph doesn't cover what you need. +### Verify in the source + +- Narrow scope with the graph, then read the source. Do not change code from graph output alone. +- For any non-trivial change, read the implementation and the relevant tests before concluding. +- Verify the exact source when touching behavior, database logic, migrations, retries, fallbacks, + recovery, or compatibility code. +- When the graph and the source disagree, the source wins. The graph may be stale or may not + model that relationship. +- An empty graph result can mean "not indexed" or "not statically visible", not "does not exist". ### Key Tools | Tool | Use when | -|------|----------| +| ------ | ---------- | | `detect_changes_tool` | Reviewing code changes — gives risk-scored analysis | | `get_review_context_tool` | Need source snippets for review — token-efficient | | `get_impact_radius_tool` | Understanding blast radius of a change | @@ -36,3 +42,4 @@ Fall back to Grep/Glob/Read **only** when the graph doesn't cover what you need. 2. Use `detect_changes_tool` for code review. 3. Use `get_affected_flows_tool` to understand impact. 4. Use `query_graph_tool` pattern="tests_for" to check coverage. + diff --git a/code_review_graph/_legacy_instructions.py b/code_review_graph/_legacy_instructions.py new file mode 100644 index 000000000..713220604 --- /dev/null +++ b/code_review_graph/_legacy_instructions.py @@ -0,0 +1,455 @@ +"""Verbatim copies of instruction sections shipped by earlier releases. + +``skills._inject_instructions`` upgrades a managed block in place, and it only +rewrites text that exactly equals a block this project generated. Blocks written +before the end marker existed have no closing boundary, so matching their full +text is the only way to find them without guessing where they stop. That is what +this module stores. + +The list is append-only. Dropping an entry means anyone still carrying that block +stops being upgraded and gets reported as hand-edited instead. Entries were +recovered by walking git history twice: an AST evaluation of every past revision +of ``code_review_graph/skills.py``, and a scan of the instruction files this repo +ships, which caught blocks a markdown formatter rewrote after generation. +""" + +from __future__ import annotations + +# shipped in .github/code-review-graph.instruction.md@997dcf49 (2062 chars) +_SECTION_01 = """--- +applyTo: '**' +description: Use code-review-graph MCP tools for token-efficient codebase exploration and code review instead of built-in file/search tools. +--- + + +## MCP Tools: code-review-graph + +**IMPORTANT: This project has a knowledge graph. ALWAYS use the +code-review-graph MCP tools BEFORE using #tool:read/readFile #tool:search/fileSearch #tool:search/textSearch to explore +the codebase.** The graph is faster, cheaper (fewer tokens), and gives +you structural context (callers, dependents, test coverage) that file +scanning cannot. + +### When to use graph tools FIRST + +- **Exploring code**: `semantic_search_nodes_tool` or `query_graph_tool` instead of Grep +- **Understanding impact**: `get_impact_radius_tool` instead of manually tracing imports +- **Code review**: `detect_changes_tool` + `get_review_context_tool` instead of reading entire files +- **Finding relationships**: `query_graph_tool` with callers_of/callees_of/imports_of/tests_for +- **Architecture questions**: `get_architecture_overview_tool` + `list_communities_tool` + +Fall back to Grep/Glob/Read **only** when the graph doesn't cover what you need. + +### Key Tools + +| Tool | Use when | +| ------ | ---------- | +| `detect_changes_tool` | Reviewing code changes — gives risk-scored analysis | +| `get_review_context_tool` | Need source snippets for review — token-efficient | +| `get_impact_radius_tool` | Understanding blast radius of a change | +| `get_affected_flows_tool` | Finding which execution paths are impacted | +| `query_graph_tool` | Tracing callers, callees, imports, tests, dependencies | +| `semantic_search_nodes_tool` | Finding functions/classes by name or keyword | +| `get_architecture_overview_tool` | Understanding high-level codebase structure | +| `refactor_tool` | Planning renames, finding dead code | + +### Workflow + +1. The graph auto-updates on file changes (via hooks). +2. Use `detect_changes_tool` for code review. +3. Use `get_affected_flows_tool` to understand impact. +4. Use `query_graph_tool` pattern="tests_for" to check coverage. +""" + +# skills.py _COPILOT_SECTION as of 9f029e01 (2049 chars) +_SECTION_02 = """--- +applyTo: '**' +description: Use code-review-graph MCP tools for token-efficient codebase exploration and code review instead of built-in file/search tools. +--- + + +## MCP Tools: code-review-graph + +**IMPORTANT: This project has a knowledge graph. ALWAYS use the +code-review-graph MCP tools BEFORE using #tool:read/readFile #tool:search/fileSearch #tool:search/textSearch to explore +the codebase.** The graph is faster, cheaper (fewer tokens), and gives +you structural context (callers, dependents, test coverage) that file +scanning cannot. + +### When to use graph tools FIRST + +- **Exploring code**: `semantic_search_nodes` or `query_graph` instead of #tool:search/fileSearch +- **Understanding impact**: `get_impact_radius` instead of manually tracing imports +- **Code review**: `detect_changes` + `get_review_context` instead of reading entire files +- **Finding relationships**: `query_graph` with callers_of/callees_of/imports_of/tests_for +- **Architecture questions**: `get_architecture_overview` + `list_communities` + +Fall back to #tool:read/readFile, #tool:search/fileSearch, or #tool:search/textSearch **only** when the graph doesn't cover what you need. + +### Key Tools + +| Tool | Use when | +| ------ | ---------- | +| `detect_changes` | Reviewing code changes — gives risk-scored analysis | +| `get_review_context` | Need source snippets for review — token-efficient | +| `get_impact_radius` | Understanding blast radius of a change | +| `get_affected_flows` | Finding which execution paths are impacted | +| `query_graph` | Tracing callers, callees, imports, tests, dependencies | +| `semantic_search_nodes` | Finding functions/classes by name or keyword | +| `get_architecture_overview` | Understanding high-level codebase structure | +| `refactor_tool` | Planning renames, finding dead code | + +### Workflow + +1. The graph auto-updates on file changes (via hooks). +2. Use `detect_changes` for code review. +3. Use `get_affected_flows` to understand impact. +4. Use `query_graph` pattern="tests_for" to check coverage. +""" + +# skills.py _CLAUDE_MD_SECTION as of 6e454683 (1845 chars) +_SECTION_03 = """ +## MCP Tools: code-review-graph + +**IMPORTANT: This project has a knowledge graph. ALWAYS use the +code-review-graph MCP tools BEFORE using Grep/Glob/Read to explore +the codebase.** The graph is faster, cheaper (fewer tokens), and gives +you structural context (callers, dependents, test coverage) that file +scanning cannot. + +### When to use graph tools FIRST + +- **Exploring code**: `semantic_search_nodes_tool` or `query_graph_tool` instead of Grep +- **Understanding impact**: `get_impact_radius_tool` instead of manually tracing imports +- **Code review**: `detect_changes_tool` + `get_review_context_tool` instead of reading entire files +- **Finding relationships**: `query_graph_tool` with callers_of/callees_of/imports_of/tests_for +- **Architecture questions**: `get_architecture_overview_tool` + `list_communities_tool` + +Fall back to Grep/Glob/Read **only** when the graph doesn't cover what you need. + +### Key Tools + +| Tool | Use when | +| ------ | ---------- | +| `detect_changes_tool` | Reviewing code changes — gives risk-scored analysis | +| `get_review_context_tool` | Need source snippets for review — token-efficient | +| `get_impact_radius_tool` | Understanding blast radius of a change | +| `get_affected_flows_tool` | Finding which execution paths are impacted | +| `query_graph_tool` | Tracing callers, callees, imports, tests, dependencies | +| `semantic_search_nodes_tool` | Finding functions/classes by name or keyword | +| `get_architecture_overview_tool` | Understanding high-level codebase structure | +| `refactor_tool` | Planning renames, finding dead code | + +### Workflow + +1. The graph auto-updates on file changes (via hooks). +2. Use `detect_changes_tool` for code review. +3. Use `get_affected_flows_tool` to understand impact. +4. Use `query_graph_tool` pattern="tests_for" to check coverage. +""" + +# shipped in CLAUDE.md@039f231a (1841 chars) +_SECTION_04 = """ +## MCP Tools: code-review-graph + +**IMPORTANT: This project has a knowledge graph. ALWAYS use the +code-review-graph MCP tools BEFORE using Grep/Glob/Read to explore +the codebase.** The graph is faster, cheaper (fewer tokens), and gives +you structural context (callers, dependents, test coverage) that file +scanning cannot. + +### When to use graph tools FIRST + +- **Exploring code**: `semantic_search_nodes_tool` or `query_graph_tool` instead of Grep +- **Understanding impact**: `get_impact_radius_tool` instead of manually tracing imports +- **Code review**: `detect_changes_tool` + `get_review_context_tool` instead of reading entire files +- **Finding relationships**: `query_graph_tool` with callers_of/callees_of/imports_of/tests_for +- **Architecture questions**: `get_architecture_overview_tool` + `list_communities_tool` + +Fall back to Grep/Glob/Read **only** when the graph doesn't cover what you need. + +### Key Tools + +| Tool | Use when | +|------|----------| +| `detect_changes_tool` | Reviewing code changes — gives risk-scored analysis | +| `get_review_context_tool` | Need source snippets for review — token-efficient | +| `get_impact_radius_tool` | Understanding blast radius of a change | +| `get_affected_flows_tool` | Finding which execution paths are impacted | +| `query_graph_tool` | Tracing callers, callees, imports, tests, dependencies | +| `semantic_search_nodes_tool` | Finding functions/classes by name or keyword | +| `get_architecture_overview_tool` | Understanding high-level codebase structure | +| `refactor_tool` | Planning renames, finding dead code | + +### Workflow + +1. The graph auto-updates on file changes (via hooks). +2. Use `detect_changes_tool` for code review. +3. Use `get_affected_flows_tool` to understand impact. +4. Use `query_graph_tool` pattern="tests_for" to check coverage. +""" + +# skills.py _CLAUDE_MD_SECTION as of 7990fc8f (1755 chars) +_SECTION_05 = """ +## MCP Tools: code-review-graph + +**IMPORTANT: This project has a knowledge graph. ALWAYS use the +code-review-graph MCP tools BEFORE using Grep/Glob/Read to explore +the codebase.** The graph is faster, cheaper (fewer tokens), and gives +you structural context (callers, dependents, test coverage) that file +scanning cannot. + +### When to use graph tools FIRST + +- **Exploring code**: `semantic_search_nodes` or `query_graph` instead of Grep +- **Understanding impact**: `get_impact_radius` instead of manually tracing imports +- **Code review**: `detect_changes` + `get_review_context` instead of reading entire files +- **Finding relationships**: `query_graph` with callers_of/callees_of/imports_of/tests_for +- **Architecture questions**: `get_architecture_overview` + `list_communities` + +Fall back to Grep/Glob/Read **only** when the graph doesn't cover what you need. + +### Key Tools + +| Tool | Use when | +| ------ | ---------- | +| `detect_changes` | Reviewing code changes — gives risk-scored analysis | +| `get_review_context` | Need source snippets for review — token-efficient | +| `get_impact_radius` | Understanding blast radius of a change | +| `get_affected_flows` | Finding which execution paths are impacted | +| `query_graph` | Tracing callers, callees, imports, tests, dependencies | +| `semantic_search_nodes` | Finding functions/classes by name or keyword | +| `get_architecture_overview` | Understanding high-level codebase structure | +| `refactor_tool` | Planning renames, finding dead code | + +### Workflow + +1. The graph auto-updates on file changes (via hooks). +2. Use `detect_changes` for code review. +3. Use `get_affected_flows` to understand impact. +4. Use `query_graph` pattern="tests_for" to check coverage. +""" + +# skills.py _CLAUDE_MD_SECTION as of 779b3761 (1751 chars) +_SECTION_06 = """ +## MCP Tools: code-review-graph + +**IMPORTANT: This project has a knowledge graph. ALWAYS use the +code-review-graph MCP tools BEFORE using Grep/Glob/Read to explore +the codebase.** The graph is faster, cheaper (fewer tokens), and gives +you structural context (callers, dependents, test coverage) that file +scanning cannot. + +### When to use graph tools FIRST + +- **Exploring code**: `semantic_search_nodes` or `query_graph` instead of Grep +- **Understanding impact**: `get_impact_radius` instead of manually tracing imports +- **Code review**: `detect_changes` + `get_review_context` instead of reading entire files +- **Finding relationships**: `query_graph` with callers_of/callees_of/imports_of/tests_for +- **Architecture questions**: `get_architecture_overview` + `list_communities` + +Fall back to Grep/Glob/Read **only** when the graph doesn't cover what you need. + +### Key Tools + +| Tool | Use when | +|------|----------| +| `detect_changes` | Reviewing code changes — gives risk-scored analysis | +| `get_review_context` | Need source snippets for review — token-efficient | +| `get_impact_radius` | Understanding blast radius of a change | +| `get_affected_flows` | Finding which execution paths are impacted | +| `query_graph` | Tracing callers, callees, imports, tests, dependencies | +| `semantic_search_nodes` | Finding functions/classes by name or keyword | +| `get_architecture_overview` | Understanding high-level codebase structure | +| `refactor_tool` | Planning renames, finding dead code | + +### Workflow + +1. The graph auto-updates on file changes (via hooks). +2. Use `detect_changes` for code review. +3. Use `get_affected_flows` to understand impact. +4. Use `query_graph` pattern="tests_for" to check coverage. +""" + +# skills.py _CLAUDE_MD_SECTION as of df5f3761 (1751 chars) +_SECTION_07 = """ +## MCP Tools: code-review-graph + +**IMPORTANT: This project has a knowledge graph. ALWAYS use the code-review-graph MCP tools BEFORE using Grep/Glob/Read to explore the codebase.** The graph is faster, cheaper (fewer tokens), and gives you structural context (callers, dependents, test coverage) that file scanning cannot. + +### When to use graph tools FIRST + +- **Exploring code**: `semantic_search_nodes` or `query_graph` instead of Grep +- **Understanding impact**: `get_impact_radius` instead of manually tracing imports +- **Code review**: `detect_changes` + `get_review_context` instead of reading entire files +- **Finding relationships**: `query_graph` with callers_of/callees_of/imports_of/tests_for +- **Architecture questions**: `get_architecture_overview` + `list_communities` + +Fall back to Grep/Glob/Read **only** when the graph doesn't cover what you need. + +### Key Tools + +| Tool | Use when | +|------|----------| +| `detect_changes` | Reviewing code changes — gives risk-scored analysis | +| `get_review_context` | Need source snippets for review — token-efficient | +| `get_impact_radius` | Understanding blast radius of a change | +| `get_affected_flows` | Finding which execution paths are impacted | +| `query_graph` | Tracing callers, callees, imports, tests, dependencies | +| `semantic_search_nodes` | Finding functions/classes by name or keyword | +| `get_architecture_overview` | Understanding high-level codebase structure | +| `refactor_tool` | Planning renames, finding dead code | + +### Workflow + +1. The graph auto-updates on file changes (via hooks). +2. Use `detect_changes` for code review. +3. Use `get_affected_flows` to understand impact. +4. Use `query_graph` pattern="tests_for" to check coverage. +""" + +# skills.py _COPILOT_SECTION as of 6e454683 (1715 chars) +_SECTION_08 = """--- +applyTo: '**' +description: >- + Use code-review-graph MCP tools for token-efficient + codebase exploration and code review. +--- + + +## MCP Tools: code-review-graph + +**IMPORTANT: This project has a knowledge graph. ALWAYS use the +code-review-graph MCP tools BEFORE using file/search tools to +explore the codebase.** The graph is faster, cheaper (fewer +tokens), and gives you structural context (callers, dependents, +test coverage) that file scanning cannot. + +### When to use graph tools FIRST + +- **Exploring code**: `semantic_search_nodes_tool` or `query_graph_tool` +- **Understanding impact**: `get_impact_radius_tool` +- **Code review**: `detect_changes_tool` + `get_review_context_tool` +- **Finding relationships**: `query_graph_tool` callers_of/callees_of +- **Architecture questions**: `get_architecture_overview_tool` + +Fall back to file/search tools **only** when the graph doesn't +cover what you need. + +### Key Tools + +| Tool | Use when | +| ------ | ---------- | +| `detect_changes_tool` | Risk-scored change analysis | +| `get_review_context_tool` | Token-efficient source snippets | +| `get_impact_radius_tool` | Blast radius of a change | +| `get_affected_flows_tool` | Impacted execution paths | +| `query_graph_tool` | Trace callers, callees, imports, tests | +| `semantic_search_nodes_tool` | Find functions/classes by keyword | +| `get_architecture_overview_tool` | High-level structure | +| `refactor_tool` | Rename planning, dead code | + +### Workflow + +1. The graph auto-updates on file changes (via hooks). +2. Use `detect_changes_tool` for code review. +3. Use `get_affected_flows_tool` to understand impact. +4. Use `query_graph_tool` pattern="tests_for" to check coverage. +""" + +# skills.py _COPILOT_SECTION as of 7990fc8f (1630 chars) +_SECTION_09 = """--- +applyTo: '**' +description: >- + Use code-review-graph MCP tools for token-efficient + codebase exploration and code review. +--- + + +## MCP Tools: code-review-graph + +**IMPORTANT: This project has a knowledge graph. ALWAYS use the +code-review-graph MCP tools BEFORE using file/search tools to +explore the codebase.** The graph is faster, cheaper (fewer +tokens), and gives you structural context (callers, dependents, +test coverage) that file scanning cannot. + +### When to use graph tools FIRST + +- **Exploring code**: `semantic_search_nodes` or `query_graph` +- **Understanding impact**: `get_impact_radius` +- **Code review**: `detect_changes` + `get_review_context` +- **Finding relationships**: `query_graph` callers_of/callees_of +- **Architecture questions**: `get_architecture_overview` + +Fall back to file/search tools **only** when the graph doesn't +cover what you need. + +### Key Tools + +| Tool | Use when | +| ------ | ---------- | +| `detect_changes` | Risk-scored change analysis | +| `get_review_context` | Token-efficient source snippets | +| `get_impact_radius` | Blast radius of a change | +| `get_affected_flows` | Impacted execution paths | +| `query_graph` | Trace callers, callees, imports, tests | +| `semantic_search_nodes` | Find functions/classes by keyword | +| `get_architecture_overview` | High-level structure | +| `refactor_tool` | Rename planning, dead code | + +### Workflow + +1. The graph auto-updates on file changes (via hooks). +2. Use `detect_changes` for code review. +3. Use `get_affected_flows` to understand impact. +4. Use `query_graph` pattern="tests_for" to check coverage. +""" + +# skills.py _CLAUDE_MD_SECTION as of a14e6029 (1377 chars) +_SECTION_10 = """ +## MCP Tools: code-review-graph + +This project uses **code-review-graph** for structural code analysis via MCP. + +### Available Tools + +| Tool | Description | +|------|-------------| +| `build_or_update_graph` | Build or incrementally update the knowledge graph | +| `detect_changes` | Risk-scored change impact analysis for code review | +| `get_impact_radius` | Blast radius from changed files | +| `get_review_context` | Focused review context with source snippets | +| `get_affected_flows` | Find execution flows affected by changes | +| `query_graph` | Predefined graph queries (callers, callees, imports, tests) | +| `semantic_search_nodes` | Search by name or semantic similarity | +| `list_flows` / `get_flow` | Explore execution flows | +| `list_communities` / `get_community` | Explore code communities | +| `get_architecture_overview` | High-level architecture from communities | +| `find_large_functions` | Find oversized functions/classes | +| `refactor_tool` / `apply_refactor_tool` | Graph-powered refactoring | +| `list_graph_stats` | Codebase metrics | +| `embed_graph` | Compute vector embeddings for semantic search | + +### Workflow + +1. The graph auto-updates on file changes (via hooks). +2. Use `detect_changes` for code review. +3. Use `get_affected_flows` to understand impact. +4. Use `query_graph` pattern="tests_for" to check coverage. +""" + +# Longest first: a shorter variant must never shadow a longer one that +# contains it. ``skills`` relies on this ordering when it picks a match. +LEGACY_INSTRUCTION_SECTIONS: tuple[str, ...] = ( + _SECTION_01, + _SECTION_02, + _SECTION_03, + _SECTION_04, + _SECTION_05, + _SECTION_06, + _SECTION_07, + _SECTION_08, + _SECTION_09, + _SECTION_10, +) diff --git a/code_review_graph/cli.py b/code_review_graph/cli.py index aecf0013d..4cff13f7c 100644 --- a/code_review_graph/cli.py +++ b/code_review_graph/cli.py @@ -162,30 +162,43 @@ def _instruction_files_to_modify( """Return the list of instruction files that ``install`` would write or modify, given the current state of the repo and the selected platform target. Used for the dry-run / confirm preview (#173). + + A file holding a section from an older release is listed as ``(update)``: + install replaces that block in place rather than leaving it stale (#314). """ - from .skills import _CLAUDE_MD_SECTION_MARKER, _PLATFORM_INSTRUCTION_FILES + from .skills import ( + _CLAUDE_MD_SECTION, + _CLAUDE_MD_SECTION_MARKER, + _COPILOT_SECTION, + _PLATFORM_INSTRUCTION_CUSTOM_SECTIONS, + _PLATFORM_INSTRUCTION_FILES, + _upgrade_managed_block, + ) targets: list[str] = [] + def _describe(filename: str, path: Path, section: str) -> None: + if not path.exists(): + targets.append(f"{filename} (new)") + return + content = path.read_text(encoding="utf-8", errors="replace") + if _CLAUDE_MD_SECTION_MARKER not in content: + targets.append(f"{filename} (append)") + elif _upgrade_managed_block(content, section) is not None: + targets.append(f"{filename} (update)") + if target in ("claude", "all"): - claude_md = repo_root / "CLAUDE.md" - if claude_md.exists(): - content = claude_md.read_text(encoding="utf-8") - if _CLAUDE_MD_SECTION_MARKER not in content: - targets.append("CLAUDE.md (append)") - else: - targets.append("CLAUDE.md (new)") + _describe("CLAUDE.md", repo_root / "CLAUDE.md", _CLAUDE_MD_SECTION) for filename, owners in _PLATFORM_INSTRUCTION_FILES.items(): if target != "all" and target not in owners: continue - path = repo_root / filename - if path.exists(): - content = path.read_text(encoding="utf-8") - if _CLAUDE_MD_SECTION_MARKER not in content: - targets.append(f"{filename} (append)") - else: - targets.append(f"{filename} (new)") + section = ( + _COPILOT_SECTION + if filename in _PLATFORM_INSTRUCTION_CUSTOM_SECTIONS + else _CLAUDE_MD_SECTION + ) + _describe(filename, repo_root / filename, section) return targets @@ -321,8 +334,7 @@ def _handle_init(args: argparse.Namespace) -> None: from .skills import ( PLATFORMS, generate_skills, - inject_claude_md, - inject_platform_instructions, + inject_instruction_files, install_codebuddy_hooks, install_codebuddy_skills, install_codex_hooks, @@ -364,14 +376,24 @@ def _handle_init(args: argparse.Namespace) -> None: "Inject graph instructions into the files above?", default_yes=True, ): - if target in ("claude", "all"): - inject_claude_md(repo_root) - inject_platform_instructions(repo_root, target=target) - # Use the precomputed instr_targets list for the confirmation - # message; we don't need the fresh return value from - # inject_platform_instructions here. - names = [t.split(" ")[0] for t in instr_targets] - print(f"Injected graph instructions into: {', '.join(names)}") + outcomes = inject_instruction_files(repo_root, target=target) + for label, wording in ( + ("created", "Injected graph instructions into"), + ("updated", "Updated graph instructions in"), + ): + names = [f for f, o in outcomes.items() if o == label] + if names: + print(f"{wording}: {', '.join(names)}") + # A hand-edited block is never overwritten, so say which file it is + # rather than reporting success the user did not get (#314). + stale = [f for f, o in outcomes.items() if o == "conflict"] + if stale: + print( + "Left edited graph instructions alone in: " + f"{', '.join(stale)}. Delete the section between " + " and its closing marker " + "and reinstall to pick up the current text." + ) else: print("Skipped instruction injection (user declined).") elif skip_instructions: diff --git a/code_review_graph/skills.py b/code_review_graph/skills.py index c529384c9..0178f12e1 100644 --- a/code_review_graph/skills.py +++ b/code_review_graph/skills.py @@ -19,6 +19,8 @@ from pathlib import Path from typing import Any +from ._legacy_instructions import LEGACY_INSTRUCTION_SECTIONS + logger = logging.getLogger(__name__) @@ -891,12 +893,14 @@ def _record_configured(key: str, plat: dict[str, Any]) -> None: "- Use `children_of` on a file to see all its functions and classes.\n" "- Use `find_large_functions_tool` to identify complex code.\n\n" "## Token Efficiency Rules\n" - '- ALWAYS start with `get_minimal_context_tool(task="")` ' - "before any other graph tool.\n" + '- Start with `get_minimal_context_tool(task="")` ' + "before other graph tools.\n" '- Use `detail_level="minimal"` on all calls. Only escalate to ' '"standard" when minimal is insufficient.\n' "- Target: complete any review/debug/refactor task in ≤5 tool calls " - "and ≤800 total output tokens." + "and ≤800 total output tokens.\n" + "- Read the implementation and its tests before changing code. The graph " + "narrows scope; it does not replace the source." ), }, "review-changes.md": { @@ -919,12 +923,14 @@ def _record_configured(key: str, plat: dict[str, Any]) -> None: "- Suggested improvements\n" "- Overall merge recommendation\n\n" "## Token Efficiency Rules\n" - '- ALWAYS start with `get_minimal_context_tool(task="")` ' - "before any other graph tool.\n" + '- Start with `get_minimal_context_tool(task="")` ' + "before other graph tools.\n" '- Use `detail_level="minimal"` on all calls. Only escalate to ' '"standard" when minimal is insufficient.\n' "- Target: complete any review/debug/refactor task in ≤5 tool calls " - "and ≤800 total output tokens." + "and ≤800 total output tokens.\n" + "- Read the implementation and its tests before changing code. The graph " + "narrows scope; it does not replace the source." ), }, "debug-issue.md": { @@ -945,12 +951,14 @@ def _record_configured(key: str, plat: dict[str, Any]) -> None: "- Look at affected flows to find the entry point that triggers the bug.\n" "- Recent changes are the most common source of new issues.\n\n" "## Token Efficiency Rules\n" - '- ALWAYS start with `get_minimal_context_tool(task="")` ' - "before any other graph tool.\n" + '- Start with `get_minimal_context_tool(task="")` ' + "before other graph tools.\n" '- Use `detail_level="minimal"` on all calls. Only escalate to ' '"standard" when minimal is insufficient.\n' "- Target: complete any review/debug/refactor task in ≤5 tool calls " - "and ≤800 total output tokens." + "and ≤800 total output tokens.\n" + "- Read the implementation and its tests before changing code. The graph " + "narrows scope; it does not replace the source." ), }, "refactor-safely.md": { @@ -973,12 +981,14 @@ def _record_configured(key: str, plat: dict[str, Any]) -> None: "- Use `get_affected_flows_tool` to ensure no critical paths are broken.\n" "- Run `find_large_functions_tool` to identify decomposition targets.\n\n" "## Token Efficiency Rules\n" - '- ALWAYS start with `get_minimal_context_tool(task="")` ' - "before any other graph tool.\n" + '- Start with `get_minimal_context_tool(task="")` ' + "before other graph tools.\n" '- Use `detail_level="minimal"` on all calls. Only escalate to ' '"standard" when minimal is insufficient.\n' "- Target: complete any review/debug/refactor task in ≤5 tool calls " - "and ≤800 total output tokens." + "and ≤800 total output tokens.\n" + "- Read the implementation and its tests before changing code. The graph " + "narrows scope; it does not replace the source." ), }, } @@ -1332,14 +1342,30 @@ def install_codex_hooks(repo_root: Path) -> Path: _CLAUDE_MD_SECTION_MARKER = "" +# Closes the managed block so reinstall can replace it without guessing where it +# ends. Releases before this shipped only the opening marker; those blocks are +# matched by their full text instead, see _legacy_instructions. +_CLAUDE_MD_SECTION_END_MARKER = "" + +# Shared across every platform instruction file so the wording stays identical. +_INSTRUCTION_INTRO = """**This project has a knowledge graph. Start with the code-review-graph +MCP tools to narrow scope, then read the source.** The graph is cheaper than scanning files and +gives you structural context (callers, dependents, test coverage) that file search cannot.""" + +_INSTRUCTION_GUARDRAILS = """### Verify in the source + +- Narrow scope with the graph, then read the source. Do not change code from graph output alone. +- For any non-trivial change, read the implementation and the relevant tests before concluding. +- Verify the exact source when touching behavior, database logic, migrations, retries, fallbacks, + recovery, or compatibility code. +- When the graph and the source disagree, the source wins. The graph may be stale or may not + model that relationship. +- An empty graph result can mean "not indexed" or "not statically visible", not "does not exist".""" + _CLAUDE_MD_SECTION = f"""{_CLAUDE_MD_SECTION_MARKER} ## MCP Tools: code-review-graph -**IMPORTANT: This project has a knowledge graph. ALWAYS use the -code-review-graph MCP tools BEFORE using Grep/Glob/Read to explore -the codebase.** The graph is faster, cheaper (fewer tokens), and gives -you structural context (callers, dependents, test coverage) that file -scanning cannot. +{_INSTRUCTION_INTRO} ### When to use graph tools FIRST @@ -1349,7 +1375,7 @@ def install_codex_hooks(repo_root: Path) -> Path: - **Finding relationships**: `query_graph_tool` with callers_of/callees_of/imports_of/tests_for - **Architecture questions**: `get_architecture_overview_tool` + `list_communities_tool` -Fall back to Grep/Glob/Read **only** when the graph doesn't cover what you need. +{_INSTRUCTION_GUARDRAILS} ### Key Tools @@ -1370,6 +1396,7 @@ def install_codex_hooks(repo_root: Path) -> Path: 2. Use `detect_changes_tool` for code review. 3. Use `get_affected_flows_tool` to understand impact. 4. Use `query_graph_tool` pattern=\"tests_for\" to check coverage. +{_CLAUDE_MD_SECTION_END_MARKER} """ # Copilot-specific instruction file content: uses VS Code tool references and @@ -1384,11 +1411,7 @@ def install_codex_hooks(repo_root: Path) -> Path: {_CLAUDE_MD_SECTION_MARKER} ## MCP Tools: code-review-graph -**IMPORTANT: This project has a knowledge graph. ALWAYS use the -code-review-graph MCP tools BEFORE using file/search tools to -explore the codebase.** The graph is faster, cheaper (fewer -tokens), and gives you structural context (callers, dependents, -test coverage) that file scanning cannot. +{_INSTRUCTION_INTRO} ### When to use graph tools FIRST @@ -1398,8 +1421,7 @@ def install_codex_hooks(repo_root: Path) -> Path: - **Finding relationships**: `query_graph_tool` callers_of/callees_of - **Architecture questions**: `get_architecture_overview_tool` -Fall back to file/search tools **only** when the graph doesn't -cover what you need. +{_INSTRUCTION_GUARDRAILS} ### Key Tools @@ -1420,6 +1442,7 @@ def install_codex_hooks(repo_root: Path) -> Path: 2. Use `detect_changes_tool` for code review. 3. Use `get_affected_flows_tool` to understand impact. 4. Use `query_graph_tool` pattern=\"tests_for\" to check coverage. +{_CLAUDE_MD_SECTION_END_MARKER} """ # Maps instruction file path → (marker, section) for files that need content @@ -1434,33 +1457,96 @@ def install_codex_hooks(repo_root: Path) -> Path: } -def _inject_instructions(file_path: Path, marker: str, section: str) -> bool: - """Append an instruction section to a file if not already present. +def _known_instruction_sections() -> tuple[str, ...]: + """Every block text this project has ever generated, longest first. - Idempotent: checks if the marker is already present before appending. - Creates the file if it doesn't exist. + Longest first matters: a shorter variant that happens to be contained in a + longer one must never win the match and leave the tail behind. + """ + current = (_CLAUDE_MD_SECTION, _COPILOT_SECTION) + return tuple(sorted({*current, *LEGACY_INSTRUCTION_SECTIONS}, key=len, reverse=True)) - Returns True if the file was modified. + +def _upgrade_managed_block(existing: str, section: str) -> str | None: + """Replace a previously generated block with ``section``. + + Only text that exactly equals a known generated block is ever rewritten, so + anything the user wrote around it survives byte for byte. Blocks predating + the end marker have no closing boundary, which is why nothing here searches + for one; guessing where such a block stops would eat user content. + + Returns the new file content, or None when the marker is present but no + known block is, meaning someone edited the block by hand. + """ + stale = [ + block + for block in _known_instruction_sections() + if block != section and block in existing + ] + if not stale: + return None + # Anchor on the longest match, then drop any duplicate blocks an older + # release left behind. Splitting around the anchor keeps the cleanup away + # from the text being written in, which a plain str.replace would not. + head = stale[0] + index = existing.index(head) + before, after = existing[:index], existing[index + len(head) :] + for block in stale[1:]: + before = before.replace(block, "") + after = after.replace(block, "") + if section in before or section in after: + # The current block is already there; the stale ones were duplicates. + return before + after + return before + section + after + + +def _inject_instructions(file_path: Path, marker: str, section: str) -> str: + """Create, or upgrade in place, the managed instruction block in a file. + + Returns one of: + + - ``"created"``: the block was written for the first time, creating the + file or appending to one that had no block. + - ``"updated"``: an older generated block was replaced with the current one. + - ``"unchanged"``: the file already holds the current block, byte for byte. + Nothing is written, so repeated installs do not touch the file. + - ``"conflict"``: the marker is present but the block matches nothing this + project generated, so it was hand-edited. The file is left alone and the + caller is expected to tell the user about it. """ existing = "" if file_path.exists(): existing = file_path.read_text(encoding="utf-8", errors="replace") if marker in existing: - logger.info("%s already contains instructions, skipping.", file_path.name) - return False + upgraded = _upgrade_managed_block(existing, section) + if upgraded is None: + if section in existing: + logger.info("%s already holds the current instructions.", file_path.name) + return "unchanged" + logger.warning( + "%s has a hand-edited code-review-graph section; leaving it alone.", + file_path, + ) + return "conflict" + file_path.write_text(upgraded, encoding="utf-8") + logger.info("Updated the MCP tools section in %s", file_path) + return "updated" separator = "\n" if existing and not existing.endswith("\n") else "" extra_newline = "\n" if existing else "" file_path.parent.mkdir(parents=True, exist_ok=True) file_path.write_text(existing + separator + extra_newline + section, encoding="utf-8") logger.info("Appended MCP tools section to %s", file_path) - return True + return "created" + +def inject_claude_md(repo_root: Path) -> str: + """Create or upgrade the MCP tools section in CLAUDE.md. -def inject_claude_md(repo_root: Path) -> None: - """Append MCP tools section to CLAUDE.md.""" - _inject_instructions( + Returns the outcome string documented on ``_inject_instructions``. + """ + return _inject_instructions( repo_root / "CLAUDE.md", _CLAUDE_MD_SECTION_MARKER, _CLAUDE_MD_SECTION, @@ -1495,7 +1581,9 @@ def _remove_legacy_instruction_file(path: Path) -> None: content = path.read_text(encoding="utf-8", errors="replace") if _CLAUDE_MD_SECTION_MARKER not in content: return - for section in (_COPILOT_SECTION, _CLAUDE_MD_SECTION): + # Longest first, so removing a long block cannot leave the tail of a shorter + # variant it contains. Anything not generated by this project is left alone. + for section in _known_instruction_sections(): content = content.replace(section, "") if _CLAUDE_MD_SECTION_MARKER in content: return @@ -1702,9 +1790,30 @@ def inject_platform_instructions(repo_root: Path, target: str = "all") -> list[s - any other platform key (``cursor``, ``windsurf``, ``antigravity``, ``opencode``, ``codex``): writes only the files associated with that platform. - Returns list of filenames that were created or updated. + Returns list of filenames that were created or updated. Use + ``inject_instruction_files`` when the caller also needs to know which files + were left alone because someone edited the block by hand. + """ + outcomes = inject_instruction_files(repo_root, target=target, include_claude_md=False) + return [name for name, outcome in outcomes.items() if outcome in ("created", "updated")] + + +def inject_instruction_files( + repo_root: Path, + target: str = "all", + *, + include_claude_md: bool = True, +) -> dict[str, str]: + """Write every instruction file for ``target`` and report what happened. + + Maps each filename to ``"created"``, ``"updated"``, ``"unchanged"`` or + ``"conflict"``, as documented on ``_inject_instructions``. This is the entry + point the install command uses so it can tell the user which files it + upgraded and which ones need manual attention. """ - updated: list[str] = [] + outcomes: dict[str, str] = {} + if include_claude_md and target in ("claude", "all"): + outcomes["CLAUDE.md"] = inject_claude_md(repo_root) for filename, owners in _PLATFORM_INSTRUCTION_FILES.items(): if target != "all" and target not in owners: continue @@ -1713,13 +1822,12 @@ def inject_platform_instructions(repo_root: Path, target: str = "all") -> list[s marker, section = _PLATFORM_INSTRUCTION_CUSTOM_SECTIONS[filename] else: marker, section = _CLAUDE_MD_SECTION_MARKER, _CLAUDE_MD_SECTION - if _inject_instructions(path, marker, section): - updated.append(filename) + outcomes[filename] = _inject_instructions(path, marker, section) for filename, owners in _LEGACY_PLATFORM_INSTRUCTION_FILES.items(): if target != "all" and target not in owners: continue _remove_legacy_instruction_file(repo_root / filename) - return updated + return outcomes # --- Cursor hooks --- diff --git a/code_review_graph/uninstall.py b/code_review_graph/uninstall.py index ea12b31e4..6aabdd4f2 100644 --- a/code_review_graph/uninstall.py +++ b/code_review_graph/uninstall.py @@ -839,34 +839,69 @@ def _remove_skill_file( _prune_empty_directory(path.parent, boundary) +def _join_without_instruction(before: str, after: str) -> str: + """Close the gap a removed instruction block leaves behind. + + The text on either side is kept. Only the whitespace at the seam is + normalised, so removing a block from the middle of a file does not leave a + pile of blank lines where it used to be. + """ + if not after.strip(): + # The block ran to the end, which is where install appends it. + return before.rstrip() + ("\n" if before.strip() else "") + if not before.strip(): + return after.lstrip("\n") + return before.rstrip("\n") + "\n\n" + after.lstrip("\n") + + def _remove_instruction( path: Path, - section: str, boundary: Path, report: UninstallReport, *, dry_run: bool, ) -> None: + """Strip every generated instruction block from a file. + + Only text that exactly equals a block this project generated is removed, so + a section someone edited by hand survives and is reported instead. Blocks + written before the closing marker existed have no end boundary, which is why + nothing here searches for one: their full recorded text is the boundary. + Matching every known variant means a block from any past release comes out, + not just one written by the running version (#314). + """ if not path.exists() or not _safe_path(path, boundary, report): return raw = _read_text(path, report) if raw is None: return - exact_index = raw.find(section) - if exact_index >= 0: - start = exact_index - end = exact_index + len(section) - rewritten = raw[:start] + raw[end:] - else: - marker_index = raw.find(skills._CLAUDE_MD_SECTION_MARKER) - if marker_index < 0: - return + + rewritten = raw + # Longest first, so removing a short variant cannot strand the tail of a + # longer one that contains it. The loop also clears duplicate blocks that + # older releases stacked up (#558). + for known in skills._known_instruction_sections(): + while (index := rewritten.find(known)) >= 0: + rewritten = _join_without_instruction( + rewritten[:index], rewritten[index + len(known) :] + ) + + if rewritten == raw: + if skills._CLAUDE_MD_SECTION_MARKER in raw: + report.skipped_paths.append( + f"{path} (marked instruction section differs from a known installed section; " + "left unchanged)" + ) + return + + if skills._CLAUDE_MD_SECTION_MARKER in rewritten: + # One block was generated and another was edited. Remove what this + # project owns and name the file so the user can deal with the rest. report.skipped_paths.append( - f"{path} (marked instruction section differs from a known installed section; " - "left unchanged)" + f"{path} (a further marked instruction section differs from a known " + "installed section; left unchanged)" ) - return - rewritten = rewritten.rstrip() + ("\n" if rewritten.strip() else "") + if rewritten: _write_text( path, @@ -1211,23 +1246,18 @@ def _process_repo( dry_run=dry_run, ) - instruction_sections = { - "CLAUDE.md": skills._CLAUDE_MD_SECTION, - **{ - relative: skills._PLATFORM_INSTRUCTION_CUSTOM_SECTIONS.get( - relative, - (skills._CLAUDE_MD_SECTION_MARKER, skills._CLAUDE_MD_SECTION), - )[1] - for relative in ( - *skills._PLATFORM_INSTRUCTION_FILES, - *skills._LEGACY_PLATFORM_INSTRUCTION_FILES, - ) - }, - } - for relative, section in instruction_sections.items(): + # Every variant is matched per file, so which section a given path was + # written with no longer has to be worked out here. + instruction_files = dict.fromkeys( + ( + "CLAUDE.md", + *skills._PLATFORM_INSTRUCTION_FILES, + *skills._LEGACY_PLATFORM_INSTRUCTION_FILES, + ) + ) + for relative in instruction_files: _remove_instruction( repo_root / relative, - section, repo_root, report, dry_run=dry_run, diff --git a/pyproject.toml b/pyproject.toml index 87995c9d6..f2bb21c8c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -114,6 +114,7 @@ select = ["E", "F", "I", "N", "W"] [tool.ruff.lint.per-file-ignores] "code_review_graph/visualization.py" = ["E501"] # embedded HTML/JS template +"code_review_graph/_legacy_instructions.py" = ["E501"] # verbatim past output, must not be reflowed "tests/fixtures/sample_databricks_export.py" = ["F841", "W292"] # intentional fixture patterns "tests/fixtures/sample_notebook.ipynb" = ["F401", "I001"] # fixture imports: intentionally unused, split across cells "tests/test_multilang.py" = ["E501"] # long assertions with explanatory comments diff --git a/skills/debug-issue/SKILL.md b/skills/debug-issue/SKILL.md index a0fc76935..124844cf1 100644 --- a/skills/debug-issue/SKILL.md +++ b/skills/debug-issue/SKILL.md @@ -22,6 +22,7 @@ Use the knowledge graph to systematically trace and debug issues. - Recent changes are the most common source of new issues. ## Token Efficiency Rules -- ALWAYS start with `get_minimal_context_tool(task="")` before any other graph tool. +- Start with `get_minimal_context_tool(task="")` before other graph tools. - Use `detail_level="minimal"` on all calls. Only escalate to "standard" when minimal is insufficient. - Target: complete any review/debug/refactor task in ≤5 tool calls and ≤800 total output tokens. +- Read the implementation and its tests before changing code. The graph narrows scope; it does not replace the source. diff --git a/skills/explore-codebase/SKILL.md b/skills/explore-codebase/SKILL.md index 74dec19b2..67eca1ac1 100644 --- a/skills/explore-codebase/SKILL.md +++ b/skills/explore-codebase/SKILL.md @@ -23,6 +23,7 @@ Use the code-review-graph MCP tools to explore and understand the codebase. - Use `find_large_functions_tool` to identify complex code. ## Token Efficiency Rules -- ALWAYS start with `get_minimal_context_tool(task="")` before any other graph tool. +- Start with `get_minimal_context_tool(task="")` before other graph tools. - Use `detail_level="minimal"` on all calls. Only escalate to "standard" when minimal is insufficient. - Target: complete any review/debug/refactor task in ≤5 tool calls and ≤800 total output tokens. +- Read the implementation and its tests before changing code. The graph narrows scope; it does not replace the source. diff --git a/skills/refactor-safely/SKILL.md b/skills/refactor-safely/SKILL.md index 410b5c397..4803ba04e 100644 --- a/skills/refactor-safely/SKILL.md +++ b/skills/refactor-safely/SKILL.md @@ -23,6 +23,7 @@ Use the knowledge graph to plan and execute refactoring with confidence. - Run `find_large_functions_tool` to identify decomposition targets. ## Token Efficiency Rules -- ALWAYS start with `get_minimal_context_tool(task="")` before any other graph tool. +- Start with `get_minimal_context_tool(task="")` before other graph tools. - Use `detail_level="minimal"` on all calls. Only escalate to "standard" when minimal is insufficient. - Target: complete any review/debug/refactor task in ≤5 tool calls and ≤800 total output tokens. +- Read the implementation and its tests before changing code. The graph narrows scope; it does not replace the source. diff --git a/skills/review-changes/SKILL.md b/skills/review-changes/SKILL.md index c62810d2b..0c2234bc6 100644 --- a/skills/review-changes/SKILL.md +++ b/skills/review-changes/SKILL.md @@ -24,6 +24,7 @@ Provide findings grouped by risk level (high/medium/low) with: - Overall merge recommendation ## Token Efficiency Rules -- ALWAYS start with `get_minimal_context_tool(task="")` before any other graph tool. +- Start with `get_minimal_context_tool(task="")` before other graph tools. - Use `detail_level="minimal"` on all calls. Only escalate to "standard" when minimal is insufficient. - Target: complete any review/debug/refactor task in ≤5 tool calls and ≤800 total output tokens. +- Read the implementation and its tests before changing code. The graph narrows scope; it does not replace the source. diff --git a/tests/test_cli_install.py b/tests/test_cli_install.py index 9ac3b5b2d..0e4fec04a 100644 --- a/tests/test_cli_install.py +++ b/tests/test_cli_install.py @@ -217,9 +217,9 @@ def _install_codebuddy_hooks(repo_root): called["codebuddy_hooks"] = True return repo_root / ".codebuddy" / "settings.json" - def _inject_platform_instructions(repo_root, target="all"): + def _inject_instruction_files(repo_root, target="all", *, include_claude_md=True): called["codebuddy_instructions"] = target == "codebuddy" - return ["CODEBUDDY.md"] + return {"CODEBUDDY.md": "created"} monkeypatch.setattr(skills_module, "generate_skills", _generate_skills) monkeypatch.setattr( @@ -236,8 +236,8 @@ def _inject_platform_instructions(repo_root, target="all"): ) monkeypatch.setattr( skills_module, - "inject_platform_instructions", - _inject_platform_instructions, + "inject_instruction_files", + _inject_instruction_files, ) args = _args(tmp_path, "codebuddy") diff --git a/tests/test_skills.py b/tests/test_skills.py index 5aac357d7..de0d70b6a 100644 --- a/tests/test_skills.py +++ b/tests/test_skills.py @@ -709,6 +709,158 @@ def test_idempotent_with_existing_content(self, tmp_path): assert second_content.count(_CLAUDE_MD_SECTION_MARKER) == 1 +def _legacy_sections(*, copilot: bool) -> list[str]: + """Recorded past blocks, longest first. Copilot ones carry YAML front matter.""" + return [ + block + for block in skills_module.LEGACY_INSTRUCTION_SECTIONS + if block.startswith("---\n") is copilot + ] + + +class TestManagedBlockUpgrade: + """Reinstall must replace an older generated block, not silently skip it. + + Regression test for #314: the injector only checked whether the opening + marker was present, so anyone who installed before the guardrails landed + kept the old text forever and reinstalling was a no-op. + """ + + OLDER = _legacy_sections(copilot=False)[0] + + def test_reinstall_upgrades_an_older_generated_section(self, tmp_path): + claude_md = tmp_path / "CLAUDE.md" + claude_md.write_text(self.OLDER, encoding="utf-8") + + assert inject_claude_md(tmp_path) == "updated" + + content = claude_md.read_text(encoding="utf-8") + assert content == skills_module._CLAUDE_MD_SECTION + assert self.OLDER not in content + assert "### Verify in the source" in content + assert content.count(_CLAUDE_MD_SECTION_MARKER) == 1 + + def test_reinstall_over_current_section_is_byte_idempotent(self, tmp_path): + claude_md = tmp_path / "CLAUDE.md" + assert inject_claude_md(tmp_path) == "created" + first = claude_md.read_bytes() + stat_before = claude_md.stat().st_mtime_ns + + assert inject_claude_md(tmp_path) == "unchanged" + + assert claude_md.read_bytes() == first + # "unchanged" must not rewrite the file at all. + assert claude_md.stat().st_mtime_ns == stat_before + + def test_hand_edited_block_is_preserved_and_reported(self, tmp_path): + edited = self.OLDER.replace("### Key Tools", "### Key Tools (our notes)") + claude_md = tmp_path / "CLAUDE.md" + claude_md.write_text(edited, encoding="utf-8") + + assert inject_claude_md(tmp_path) == "conflict" + + assert claude_md.read_text(encoding="utf-8") == edited + + def test_user_content_around_the_block_survives_an_upgrade(self, tmp_path): + head = "# House rules\n\nNever force push.\n\n" + tail = "\n## Deploy notes\n\nRun the migration first.\n" + claude_md = tmp_path / "CLAUDE.md" + claude_md.write_text(head + self.OLDER + tail, encoding="utf-8") + + assert inject_claude_md(tmp_path) == "updated" + + content = claude_md.read_text(encoding="utf-8") + assert content == head + skills_module._CLAUDE_MD_SECTION + tail + assert content.startswith(head) + assert content.endswith(tail) + + def test_duplicate_stale_blocks_collapse_to_one(self, tmp_path): + """#558 left repeat installs stacking blocks; upgrade must not keep both.""" + older_two = _legacy_sections(copilot=False)[1] + claude_md = tmp_path / "CLAUDE.md" + claude_md.write_text(self.OLDER + "\n" + older_two, encoding="utf-8") + + assert inject_claude_md(tmp_path) == "updated" + + content = claude_md.read_text(encoding="utf-8") + assert content.count(_CLAUDE_MD_SECTION_MARKER) == 1 + assert skills_module._CLAUDE_MD_SECTION in content + + def test_missing_file_is_still_created(self, tmp_path): + assert not (tmp_path / "CLAUDE.md").exists() + + assert inject_claude_md(tmp_path) == "created" + + assert (tmp_path / "CLAUDE.md").read_text(encoding="utf-8") == ( + skills_module._CLAUDE_MD_SECTION + ) + + def test_new_sections_carry_an_end_marker(self, tmp_path): + end = skills_module._CLAUDE_MD_SECTION_END_MARKER + inject_claude_md(tmp_path) + skills_module.inject_platform_instructions(tmp_path, target="all") + + names = ["CLAUDE.md", *skills_module._PLATFORM_INSTRUCTION_FILES] + for name in names: + content = (tmp_path / name).read_text(encoding="utf-8") + assert content.count(end) == 1, name + assert content.index(_CLAUDE_MD_SECTION_MARKER) < content.index(end), name + + def test_every_platform_file_upgrades_from_its_older_section(self, tmp_path): + older_copilot = _legacy_sections(copilot=True)[0] + for name in skills_module._PLATFORM_INSTRUCTION_FILES: + path = tmp_path / name + path.parent.mkdir(parents=True, exist_ok=True) + custom = name in skills_module._PLATFORM_INSTRUCTION_CUSTOM_SECTIONS + path.write_text(older_copilot if custom else self.OLDER, encoding="utf-8") + + outcomes = skills_module.inject_instruction_files(tmp_path, target="all") + + for name in skills_module._PLATFORM_INSTRUCTION_FILES: + assert outcomes[name] == "updated", name + content = (tmp_path / name).read_text(encoding="utf-8") + assert "### Verify in the source" in content, name + assert "ALWAYS use the" not in content, name + + def test_legacy_sections_are_exact_and_ordered_longest_first(self): + legacy = skills_module.LEGACY_INSTRUCTION_SECTIONS + assert len(set(legacy)) == len(legacy) + assert all(_CLAUDE_MD_SECTION_MARKER in block for block in legacy) + known = skills_module._known_instruction_sections() + assert list(known) == sorted(known, key=len, reverse=True) + assert skills_module._CLAUDE_MD_SECTION in known + assert skills_module._COPILOT_SECTION in known + + +class TestInjectInstructionFilesOutcomes: + def test_reports_created_then_unchanged(self, tmp_path): + first = skills_module.inject_instruction_files(tmp_path, target="all") + assert set(first) == {"CLAUDE.md", *skills_module._PLATFORM_INSTRUCTION_FILES} + assert set(first.values()) == {"created"} + + second = skills_module.inject_instruction_files(tmp_path, target="all") + assert set(second.values()) == {"unchanged"} + + def test_reports_conflict_without_touching_the_file(self, tmp_path): + edited = _CLAUDE_MD_SECTION_MARKER + "\n## Our own rules\n" + (tmp_path / "CLAUDE.md").write_text(edited, encoding="utf-8") + + outcomes = skills_module.inject_instruction_files(tmp_path, target="claude") + + assert outcomes == {"CLAUDE.md": "conflict"} + assert (tmp_path / "CLAUDE.md").read_text(encoding="utf-8") == edited + + def test_platform_wrapper_still_returns_written_filenames(self, tmp_path): + first = inject_platform_instructions(tmp_path, target="windsurf") + assert first == [".windsurfrules"] + assert inject_platform_instructions(tmp_path, target="windsurf") == [] + + (tmp_path / ".windsurfrules").write_text( + _legacy_sections(copilot=False)[0], encoding="utf-8" + ) + assert inject_platform_instructions(tmp_path, target="windsurf") == [".windsurfrules"] + + class TestInjectPlatformInstructionsFiltering: def test_all_writes_every_file(self, tmp_path): updated = inject_platform_instructions(tmp_path, target="all") @@ -801,6 +953,64 @@ def test_codebuddy_writes_only_codebuddy_md_and_is_idempotent(self, tmp_path): assert not (tmp_path / "AGENTS.md").exists() +class TestInstructionGuardrails: + """Every generated instruction file must carry the source-verification guardrails. + + Regression test for #314: the generated text used to tell agents to ALWAYS use + the graph before reading source and to fall back to file search ONLY when the + graph did not cover the need, which made models act on graph summaries alone. + """ + + # Each guardrail is asserted by a fragment short enough to survive rewrapping. + GUARDRAIL_FRAGMENTS = ( + "Do not change code from graph output alone", + "read the implementation and the relevant tests before concluding", + "migrations, retries, fallbacks", + "the source wins", + 'can mean "not indexed" or "not statically visible"', + ) + + @staticmethod + def _instruction_files(tmp_path: Path) -> dict[str, str]: + inject_claude_md(tmp_path) + inject_platform_instructions(tmp_path, target="all") + names = ["CLAUDE.md", *skills_module._PLATFORM_INSTRUCTION_FILES] + return { + name: (tmp_path / name).read_text(encoding="utf-8") for name in names + } + + def test_every_instruction_file_has_all_guardrails(self, tmp_path): + written = self._instruction_files(tmp_path) + assert len(written) == 9 + for name, content in written.items(): + assert "### Verify in the source" in content, name + for fragment in self.GUARDRAIL_FRAGMENTS: + assert fragment in content, f"{name} is missing: {fragment}" + + def test_no_instruction_file_claims_the_graph_replaces_source(self, tmp_path): + for name, content in self._instruction_files(tmp_path).items(): + assert "ALWAYS use the" not in content, name + assert "**only** when the graph" not in content, name + + def test_shared_guardrail_text_is_identical_across_platforms(self, tmp_path): + written = self._instruction_files(tmp_path) + for name, content in written.items(): + assert skills_module._INSTRUCTION_GUARDRAILS in content, name + assert skills_module._INSTRUCTION_INTRO in content, name + + def test_guardrails_stay_small(self): + """The section ships in every user's context, so cap its growth.""" + assert skills_module._INSTRUCTION_GUARDRAILS.count("\n") + 1 <= 9 + assert skills_module._CLAUDE_MD_SECTION.count("\n") <= 46 + assert skills_module._COPILOT_SECTION.count("\n") <= 53 + + def test_skill_templates_do_not_demand_graph_only_work(self): + for filename, skill in skills_module._SKILLS.items(): + body = skill["body"] + assert "ALWAYS start with" not in body, filename + assert "Read the implementation and its tests before changing code." in body, filename + + class TestCodeBuddyPlatform: def test_platform_uses_official_project_mcp_contract(self): assert "codebuddy" in PLATFORMS diff --git a/tests/test_uninstall.py b/tests/test_uninstall.py index 86c460211..f02b8e399 100644 --- a/tests/test_uninstall.py +++ b/tests/test_uninstall.py @@ -457,6 +457,139 @@ def test_uninstall_cleans_current_and_legacy_copilot_instruction_paths( assert path.read_text(encoding="utf-8") == "# User notes\n" +def _legacy_claude_section() -> str: + """The longest recorded pre-guardrails CLAUDE.md-shaped block.""" + return next( + block + for block in skills.LEGACY_INSTRUCTION_SECTIONS + if not block.startswith("---\n") + ) + + +def test_uninstall_removes_a_section_written_by_an_older_release( + fake_repo: Path, + fake_home: Path, +) -> None: + """Removal used to match only the running version's text, so a block from + any earlier release survived an explicit uninstall (#314).""" + path = fake_repo / "CLAUDE.md" + _write(path, "user instructions\n\n" + _legacy_claude_section()) + + report = uninstall.run(repo=fake_repo, keep_data=True) + + assert report.errors == [] + assert path.read_text(encoding="utf-8") == "user instructions\n" + assert report.skipped_paths == [] + + +def test_uninstall_removes_the_current_section_including_its_end_marker( + fake_repo: Path, + fake_home: Path, +) -> None: + path = fake_repo / "CLAUDE.md" + _write(path, "user instructions\n\n" + skills._CLAUDE_MD_SECTION) + + uninstall.run(repo=fake_repo, keep_data=True) + + remaining = path.read_text(encoding="utf-8") + assert remaining == "user instructions\n" + assert skills._CLAUDE_MD_SECTION_END_MARKER not in remaining + + +def test_uninstall_leaves_a_hand_edited_section_alone_and_reports_it( + fake_repo: Path, + fake_home: Path, +) -> None: + path = fake_repo / "CLAUDE.md" + content = "user prefix\n\n" + _legacy_claude_section().replace( + "### Key Tools", "### Key Tools (our notes)" + ) + _write(path, content) + + report = uninstall.run(repo=fake_repo, keep_data=True) + + assert path.read_text(encoding="utf-8") == content + assert any(str(path) in item and "left unchanged" in item for item in report.skipped_paths) + + +def test_uninstall_keeps_user_content_on_both_sides_of_the_block( + fake_repo: Path, + fake_home: Path, +) -> None: + head = "# House rules\n\nNever force push.\n\n" + tail = "\n## Deploy notes\n\nRun the migration first.\n" + path = fake_repo / "CLAUDE.md" + _write(path, head + _legacy_claude_section() + tail) + + uninstall.run(repo=fake_repo, keep_data=True) + + remaining = path.read_text(encoding="utf-8") + assert remaining == ( + "# House rules\n\nNever force push.\n\n## Deploy notes\n\nRun the migration first.\n" + ) + assert skills._CLAUDE_MD_SECTION_MARKER not in remaining + # One blank line where the block was, not a pileup left by the removal. + assert "\n\n\n" not in remaining + + +def test_uninstall_clears_every_duplicate_stale_block( + fake_repo: Path, + fake_home: Path, +) -> None: + """Repeat installs used to stack blocks (#558); removal must clear them all.""" + older = [ + block + for block in skills.LEGACY_INSTRUCTION_SECTIONS + if not block.startswith("---\n") + ] + path = fake_repo / "CLAUDE.md" + _write(path, "user instructions\n\n" + older[0] + "\n" + older[1] + "\n" + older[0]) + + report = uninstall.run(repo=fake_repo, keep_data=True) + + assert report.errors == [] + assert path.read_text(encoding="utf-8") == "user instructions\n" + + +def test_install_then_uninstall_restores_the_file_byte_for_byte( + fake_repo: Path, + fake_home: Path, +) -> None: + original = "# House rules\n\nNever force push.\n" + path = fake_repo / "CLAUDE.md" + _write(path, original) + + skills.inject_claude_md(fake_repo) + assert path.read_text(encoding="utf-8") != original + + uninstall.run(repo=fake_repo, keep_data=True) + + assert path.read_text(encoding="utf-8") == original + + +def test_uninstall_removes_older_sections_from_every_instruction_path( + fake_repo: Path, + fake_home: Path, +) -> None: + older_claude = _legacy_claude_section() + older_copilot = next( + block for block in skills.LEGACY_INSTRUCTION_SECTIONS if block.startswith("---\n") + ) + relatives = ["CLAUDE.md", *skills._PLATFORM_INSTRUCTION_FILES] + for relative in relatives: + custom = relative in skills._PLATFORM_INSTRUCTION_CUSTOM_SECTIONS + _write( + fake_repo / relative, + "user instructions\n\n" + (older_copilot if custom else older_claude), + ) + + report = uninstall.run(repo=fake_repo, keep_data=True) + + assert report.errors == [] + for relative in relatives: + assert (fake_repo / relative).read_text(encoding="utf-8") == "user instructions\n" + + def test_modified_instruction_section_is_not_guessed_or_truncated( fake_repo: Path, fake_home: Path,