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 <cursoragent@cursor.com>

* fix(review): assert pdg_query, Spring Actuator, and Explore/Use absence

Co-authored-by: Cursor <cursoragent@cursor.com>

* 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 <cursoragent@cursor.com>

* 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 <cursoragent@cursor.com>

* 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 <cursoragent@cursor.com>

* fix: pick graph tools by question type and require graph-first reads

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Gergő Magyar 2026-09-01 10:03:08 +01:00 • committed by GitHub
parent 60ffffeb83
commit 9f82ffd6bf
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 77 additions and 13 deletions

View file

@ -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.

View file

@ -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.

View file

@ -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.'
: ''

View file

@ -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');
});
});

View file

@ -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);
},