From 9f82ffd6bfe59270c6cdec0e2f3314038f4a7faf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Tue, 1 Sep 2026 10:03:08 +0100 Subject: [PATCH] fix: MUST graph tools on structural reads in generated agent block (#3125) * fix: add a read-path MUST so generated agent blocks invoke GitNexus on structural questions The managed Always-Do list gated every MUST on edit/commit/rename, so read-only sessions had no reason to call query, context, or impact. Replace the advisory Explore/Use bullets and keep the #2059 call shapes. Fixes #3076 Co-authored-by: Cursor * fix(review): assert pdg_query, Spring Actuator, and Explore/Use absence Co-authored-by: Cursor * style: prettier-wrap read-path MUST unit assertions CI quality/format failed on the two test files that grew beyond printWidth. Co-authored-by: Cursor * Address PR review feedback (#3125) - Assert the read-path MUST bullet is immediately followed by the Spring Actuator Always-Do line, not merely that both substrings exist. Co-authored-by: Cursor * test: pin the read-path MUST to Always-Do so CI cannot miss a move The previous floor and whole-block toContain still passed if the MUST left Always-Do while pdg_query kept the count. Own-line and ungated-length asserts close that hole. Co-authored-by: Cursor * fix: pick graph tools by question type and require graph-first reads Co-authored-by: Cursor --------- Co-authored-by: Gergo Magyar Co-authored-by: Cursor --- AGENTS.md | 3 +- CLAUDE.md | 3 +- gitnexus/src/cli/ai-context.ts | 3 +- .../unit/ai-context-read-path-must.test.ts | 48 +++++++++++++++++++ .../test/unit/shipped-skills-sync.test.ts | 33 ++++++++++--- 5 files changed, 77 insertions(+), 13 deletions(-) create mode 100644 gitnexus/test/unit/ai-context-read-path-must.test.ts diff --git a/AGENTS.md b/AGENTS.md index 05be52af2..251f410c1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -121,8 +121,7 @@ This project is indexed by GitNexus as **GitNexus** (248612 symbols, 565510 rela - **MUST analyze graph changes before committing.** Use `detect_changes({scope: "all"})` (MCP) or `node .gitnexus/run.cjs detect-changes --scope all --repo .` (CLI fallback). `partial: true` or `truncated: true` is not a clean check — a zero means unseen, not unaffected; re-run it. For regression review: `detect_changes({scope: "compare", base_ref: "main"})` or `node .gitnexus/run.cjs detect-changes --scope compare --base-ref "main" --repo .`. - MUST warn on HIGH/CRITICAL `risk` pre-edit; never use `riskSharedAxes` to waive a HIGH/CRITICAL `risk` warning. Compare File/symbol: MCP File omits axes; Graph-RAG expands File. - **MUST treat `risk: UNKNOWN` as unresolved, not as low.** An empty caller set is not evidence the symbol is unused — it can also mean the callers are not resolvable by the index (plain-object property access, dynamic dispatch, cross-language calls). `impact` pairs `UNKNOWN` with a `riskNote` saying so. Confirm with a text search before treating the symbol as safe to change or delete; do not proceed on the strength of a zero. -- Explore with `query({search_query: "concept"})` for process-grouped flows. -- Use `context({name: "symbolName"})` for callers, callees, and flows. +- **MUST use `query({search_query: "concept"})` for concepts/flows, `context({name: "symbolName"})` for a named symbol, or `impact` for blast radius, on read-only callers, dependencies, imports, or execution flow.** Graph first; text search only for empty/`UNKNOWN`/literals. - For security review, `explain({target: "fileOrSymbol"})` lists taint findings (source→sink flows; needs `analyze --pdg`). - For control/data dependence, `pdg_query({mode: "controls", target: "fileOrSymbol"})` answers "under what condition does X run?" (CDG, incl. guard clauses) and `pdg_query({mode: "flows", target, variable})` traces "where does variable Y flow?" (REACHING_DEF). `--pdg` layer. diff --git a/CLAUDE.md b/CLAUDE.md index 10681d819..069163232 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -72,8 +72,7 @@ This project is indexed by GitNexus as **GitNexus** (248612 symbols, 565510 rela - **MUST analyze graph changes before committing.** Use `detect_changes({scope: "all"})` (MCP) or `node .gitnexus/run.cjs detect-changes --scope all --repo .` (CLI fallback). `partial: true` or `truncated: true` is not a clean check — a zero means unseen, not unaffected; re-run it. For regression review: `detect_changes({scope: "compare", base_ref: "main"})` or `node .gitnexus/run.cjs detect-changes --scope compare --base-ref "main" --repo .`. - MUST warn on HIGH/CRITICAL `risk` pre-edit; never use `riskSharedAxes` to waive a HIGH/CRITICAL `risk` warning. Compare File/symbol: MCP File omits axes; Graph-RAG expands File. - **MUST treat `risk: UNKNOWN` as unresolved, not as low.** An empty caller set is not evidence the symbol is unused — it can also mean the callers are not resolvable by the index (plain-object property access, dynamic dispatch, cross-language calls). `impact` pairs `UNKNOWN` with a `riskNote` saying so. Confirm with a text search before treating the symbol as safe to change or delete; do not proceed on the strength of a zero. -- Explore with `query({search_query: "concept"})` for process-grouped flows. -- Use `context({name: "symbolName"})` for callers, callees, and flows. +- **MUST use `query({search_query: "concept"})` for concepts/flows, `context({name: "symbolName"})` for a named symbol, or `impact` for blast radius, on read-only callers, dependencies, imports, or execution flow.** Graph first; text search only for empty/`UNKNOWN`/literals. - For security review, `explain({target: "fileOrSymbol"})` lists taint findings (source→sink flows; needs `analyze --pdg`). - For control/data dependence, `pdg_query({mode: "controls", target: "fileOrSymbol"})` answers "under what condition does X run?" (CDG, incl. guard clauses) and `pdg_query({mode: "flows", target, variable})` traces "where does variable Y flow?" (REACHING_DEF). `--pdg` layer. diff --git a/gitnexus/src/cli/ai-context.ts b/gitnexus/src/cli/ai-context.ts index 5d2179c05..ca02f2b08 100644 --- a/gitnexus/src/cli/ai-context.ts +++ b/gitnexus/src/cli/ai-context.ts @@ -231,8 +231,7 @@ This project is indexed by GitNexus as **${projectName}**${noStats ? '' : ` (${s - **MUST analyze graph changes before committing.** Use \`detect_changes({scope: "all"})\` (MCP) or \`${runner} detect-changes --scope all --repo .\` (CLI fallback). \`partial: true\` or \`truncated: true\` is not a clean check — a zero means unseen, not unaffected; re-run it. For regression review: \`detect_changes({scope: "compare", base_ref: ${JSON.stringify(markdownSafeBranch(defaultBranch))}})\` or \`${runner} detect-changes --scope compare --base-ref ${JSON.stringify(markdownSafeBranch(defaultBranch))} --repo .\`. - MUST warn on HIGH/CRITICAL \`risk\` pre-edit; never use \`riskSharedAxes\` to waive a HIGH/CRITICAL \`risk\` warning. Compare File/symbol: MCP File omits axes; Graph-RAG expands File. - **MUST treat \`risk: UNKNOWN\` as unresolved, not as low.** An empty caller set is not evidence the symbol is unused — it can also mean the callers are not resolvable by the index (plain-object property access, dynamic dispatch, cross-language calls). \`impact\` pairs \`UNKNOWN\` with a \`riskNote\` saying so. Confirm with a text search before treating the symbol as safe to change or delete; do not proceed on the strength of a zero. -- Explore with \`query({search_query: "concept"})\` for process-grouped flows. -- Use \`context({name: "symbolName"})\` for callers, callees, and flows.${ +- **MUST use \`query({search_query: "concept"})\` for concepts/flows, \`context({name: "symbolName"})\` for a named symbol, or \`impact\` for blast radius, on read-only callers, dependencies, imports, or execution flow.** Graph first; text search only for empty/\`UNKNOWN\`/literals.${ hasSpringActuator ? '\n- Spring Actuator runtime evidence is enabled. A Route is authoritative only when `runtimeConfirmed === true`; `runtimeSource` is provenance and may also describe conflicts. Snapshot values are never persisted.' : '' diff --git a/gitnexus/test/unit/ai-context-read-path-must.test.ts b/gitnexus/test/unit/ai-context-read-path-must.test.ts new file mode 100644 index 000000000..20c39d145 --- /dev/null +++ b/gitnexus/test/unit/ai-context-read-path-must.test.ts @@ -0,0 +1,48 @@ +import { describe, it, expect } from 'vitest'; +import { generateGitNexusContent } from '../../src/cli/ai-context.js'; + +// Regression guard for #3076. The Explore/Use Always-Do lines were advisory, so +// read-only sessions had no MUST to call query/context/impact. The replacement +// bullet is not hasPdg-gated (unlike pdg_query) and is not nested in an +// edit/commit/rename-only sentence. +describe('generateGitNexusContent emits a read-path MUST (#3076)', () => { + const stats = { nodes: 50, edges: 100, processes: 5 }; + const mustBullet = + '- **MUST use `query({search_query: "concept"})` for concepts/flows, `context({name: "symbolName"})` for a named symbol, or `impact` for blast radius, on read-only callers, dependencies, imports, or execution flow.** Graph first; text search only for empty/`UNKNOWN`/literals.'; + + function alwaysDoSection(content: string): string { + return content.slice(content.indexOf('## Always Do'), content.indexOf('## Never Do')); + } + + function assertNoAdvisoryExploreUse(content: string): void { + expect(content).not.toMatch(/Explore\s+with/); + expect(content).not.toMatch(/Use\s+`context\(\{name:/); + expect(alwaysDoSection(content)).not.toMatch(/^- [^\n]*Explore/m); + } + + it.each([true, false])( + 'renders the MUST and drops Explore/Use bullets when hasPdg=%s', + (hasPdg) => { + const content = generateGitNexusContent('ReadPathProject', stats, { hasPdg }); + expect(alwaysDoSection(content)).toContain(`\n${mustBullet}\n`); + assertNoAdvisoryExploreUse(content); + if (hasPdg) { + expect(content).toContain('pdg_query'); + } + }, + ); + + it('keeps the MUST beside the Spring Actuator Always-Do line', () => { + const content = generateGitNexusContent('SpringProject', stats, { hasSpringActuator: true }); + expect(alwaysDoSection(content)).toContain( + `${mustBullet}\n- Spring Actuator runtime evidence is enabled`, + ); + assertNoAdvisoryExploreUse(content); + }); + + it('keeps pdg_query gated on hasPdg while the read-path MUST stays always-emitted', () => { + const withoutPdg = generateGitNexusContent('PlainProject', stats); + expect(alwaysDoSection(withoutPdg)).toContain(`\n${mustBullet}\n`); + expect(withoutPdg).not.toContain('pdg_query'); + }); +}); diff --git a/gitnexus/test/unit/shipped-skills-sync.test.ts b/gitnexus/test/unit/shipped-skills-sync.test.ts index 984037e4c..936f4c2af 100644 --- a/gitnexus/test/unit/shipped-skills-sync.test.ts +++ b/gitnexus/test/unit/shipped-skills-sync.test.ts @@ -330,6 +330,10 @@ function extractManagedBlock(file: string): string { return match![1]; } +function alwaysDoSection(block: string): string { + return block.slice(block.indexOf('## Always Do'), block.indexOf('## Never Do')); +} + // The `risk: UNKNOWN` Always-Do bullet and its Never-Do clause were hand-added // INSIDE the machine-managed region instead of living in the template, so a // real analyze run silently deleted them on regeneration — twice (#2856's @@ -357,18 +361,33 @@ describe('root AGENTS.md / CLAUDE.md managed block keeps the risk: UNKNOWN polic for (const fragment of REQUIRED_FRAGMENTS) expect(block).toContain(fragment); }); + it.each(['AGENTS.md', 'CLAUDE.md'])( + '%s Always-Do pins the read-path MUST as its own bullet (#3076)', + (file) => { + const alwaysDo = alwaysDoSection(extractManagedBlock(file)); + expect(alwaysDo).toMatch(/^- \*\*MUST use `query\(\{search_query: "concept"\}\)`/m); + expect(alwaysDo).toContain('Graph first'); + expect(alwaysDo).toContain('text search only for empty/'); + expect(alwaysDo).not.toMatch(/Explore\s+with/); + expect(alwaysDo).not.toMatch(/Use\s+`context\(\{name:/); + expect(alwaysDo).not.toMatch(/^- [^\n]*Explore/m); + }, + ); + it.each(['AGENTS.md', 'CLAUDE.md'])( "%s managed block's Always Do / Never Do bullet counts do not drop below the known floor", (file) => { const block = extractManagedBlock(file); - const alwaysDoSection = block.slice( - block.indexOf('## Always Do'), - block.indexOf('## Never Do'), - ); + const alwaysDo = alwaysDoSection(block); const neverDoSection = block.slice(block.indexOf('## Never Do')); - // 7 Always-Do bullets are unconditional; an 8th (pdg_query) only - // appears when the index was built with --pdg, so the floor is 7, not 8. - expect((alwaysDoSection.match(/^- /gm) || []).length).toBeGreaterThanOrEqual(7); + const ungated = (alwaysDo.match(/^- .+/gm) ?? []).filter( + (line) => !line.includes('pdg_query'), + ); + // Six Always-Do bullets are not hasPdg-gated after #3076. pdg_query is + // extra when the committed block was generated with --pdg. Counting + // ungated bullets (not total >= 6) fails if the read-path MUST leaves + // Always-Do while pdg_query keeps the old slack. + expect(ungated).toHaveLength(6); // Never Do never varies with hasPdg — exactly 4 today, so 4 is the floor. expect((neverDoSection.match(/^- NEVER /gm) || []).length).toBeGreaterThanOrEqual(4); },