fix: address PR #1508 review findings (F1-F5)

Refactor the keep-marker stats-update path and close the test-coverage gaps surfaced by the production-readiness review. ## Findings 2 + 3 (high) — fragile extraction → silent corruption Stop re-extracting `newName` (first `**bold**`) and `newStats` (first `(...)`, with fallback) from generated content. Both are structurally fragile: - F2: newName silently picks the wrong value if the template ever emits bold text before the project-name line (no current bug; an unstated contract with no enforcement) - F3: newStats fallback `$([^)]+)$` matches `({target: "symbolName", direction: "upstream"})` from the Always-Do bullet when `noStats: true` suppresses the canonical stats line, silently corrupting the stats output Fix: pass `projectName: string` and `stats: RepoStats` directly into `upsertGitNexusSection`. Build the stats line from those values. Both callers in `generateAIContextFiles` already have them in scope. ## Finding 1 (high) — misleading return value When a keep marker is present but no stats line matches the pattern, the function previously returned `'updated'` without writing, producing `CLAUDE.md (updated)` in CLI output for a file that was not touched. Add a distinct `'preserved'` return variant; CLI now reports `CLAUDE.md (preserved)` honestly. ## Finding 4 (medium) — unanchored stats regex `/(?:Indexed as|...) \*\*[^*]+\*\* $[^)]+$/` could match prose embedded mid-paragraph in user content (e.g. "you'll see it Indexed as **Foo** (note: ...)"). Anchor with `^...$` plus the `m` flag so only standalone stats lines match. ## Finding 5 — test coverage gaps Seven new tests, each cross-referenced to the review finding: - keep marker OUTSIDE the GitNexus section has no effect - AGENTS.md keep path preserves custom layout (parity with CLAUDE.md) - idempotent: second run produces byte-identical output - CRLF file with keep marker: stats line updates correctly - noStats + keep marker: not corrupted by Always-Do tuple text (F3 regression guard) - returns 'preserved' (not 'updated') when no stats line matches (F1 regression guard) - project name with markdown punctuation (hyphens/slash/dot) lands intact All 23 ai-context tests pass; typecheck, prettier, eslint clean.
2026-09-12 23:02:45 +00:00 · 2026-05-13 15:42:20 -07:00 · 2026-05-13 15:42:20 -07:00 · 545ca50e56
commit 545ca50e56
parent d61338294b
2 changed files with 262 additions and 15 deletions
--- a/gitnexus/src/cli/ai-context.ts
+++ b/gitnexus/src/cli/ai-context.ts
@ -199,7 +199,9 @@ async function fileExists(filePath: string): Promise<boolean> {
 async function upsertGitNexusSection(
  filePath: string,
  content: string,
-): Promise<'created' | 'updated' | 'appended'> {
+  projectName: string,
+  stats: RepoStats,
+): Promise<'created' | 'updated' | 'appended' | 'preserved'> {
  const exists = await fileExists(filePath);

  if (!exists) {
@ -232,18 +234,27 @@ async function upsertGitNexusSection(
    // custom layout and only update the stats line (node/edge/flow counts).
    // This lets teams trim the verbose default template to a lean format without
    // having it overwritten on every `gitnexus analyze`.
+    //
+    // Note: the keep-marker check operates on `existingSection` (the substring
+    // between valid section markers identified by findSectionMarkerIndex), so
+    // a keep marker in user prose OUTSIDE the GitNexus block has no effect.
    if (existingSection.includes('<!-- gitnexus:keep -->')) {
-      // Match both formats:
-      //   "Indexed as **name** (N symbols, M relationships, P execution flows)"
-      //   "This project is indexed by GitNexus as **name** (N symbols, ...)"
-      const statsPattern = /(?:Indexed as|indexed by GitNexus as) \*\*[^*]+\*\* \([^)]+\)/;
+      // Build the new stats line from the caller-provided values directly.
+      // We do NOT re-extract from `content` because:
+      //   (a) first-bold extraction is fragile if the template evolves
+      //   (b) the parenthesized-text fallback can match unrelated tuples
+      //       like `({target: "symbolName", direction: "upstream"})`
+      //       when noStats is set
+      // Passing projectName + stats explicitly makes the contract obvious.
+      const newStatsInner = `${stats.nodes || 0} symbols, ${stats.edges || 0} relationships, ${stats.processes || 0} execution flows`;
+      const statsLine = `Indexed as **${projectName}** (${newStatsInner})`;

-      // Extract fresh stats from the newly generated content
-      const newName = (content.match(/\*\*([^*]+)\*\*/) || [])[1] || 'unknown';
-      const newStats =
-        (content.match(/\((\d[\d,]* symbols[^)]+)\)/) || content.match(/\(([^)]+)\)/) || [])[1] ||
-        '0 nodes';
-      const statsLine = `Indexed as **${newName}** (${newStats})`;
+      // Match either canonical phrasing, anchored to line boundaries (`^`/`$`
+      // with `m` flag) so we cannot replace prose embedded mid-paragraph like
+      // "you'll see it Indexed as **Foo** (note: ...)". The trailing period
+      // / sentence text the generator emits is preserved by sitting outside
+      // the matched pattern.
+      const statsPattern = /^(?:Indexed as|indexed by GitNexus as) \*\*[^*]+\*\* \([^)]+\)/m;

      if (statsPattern.test(existingSection)) {
        const updatedSection = existingSection.replace(statsPattern, statsLine);
@ -252,8 +263,10 @@ async function upsertGitNexusSection(
        await fs.writeFile(filePath, (before + updatedSection + after).trim() + '\n', 'utf-8');
        return 'updated';
      }
-      // Keep marker present but no stats line found — preserve section as-is
-      return 'updated';
+      // Keep marker present but no stats line matched. Section is preserved
+      // unchanged on disk; return a distinct status so callers/CLI output
+      // don't mis-report this as 'updated' (which would imply a write).
+      return 'preserved';
    }

    // No keep marker — replace existing section with full verbose content
@ -377,12 +390,12 @@ export async function generateAIContextFiles(
  if (!options?.skipAgentsMd) {
    // Create AGENTS.md (standard for Cursor, Windsurf, OpenCode, Cline, etc.)
    const agentsPath = path.join(repoPath, 'AGENTS.md');
-    const agentsResult = await upsertGitNexusSection(agentsPath, content);
+    const agentsResult = await upsertGitNexusSection(agentsPath, content, projectName, stats);
    createdFiles.push(`AGENTS.md (${agentsResult})`);

    // Create CLAUDE.md (for Claude Code)
    const claudePath = path.join(repoPath, 'CLAUDE.md');
-    const claudeResult = await upsertGitNexusSection(claudePath, content);
+    const claudeResult = await upsertGitNexusSection(claudePath, content, projectName, stats);
    createdFiles.push(`CLAUDE.md (${claudeResult})`);
  } else {
    createdFiles.push('AGENTS.md (skipped via --skip-agents-md)');
--- a/gitnexus/test/unit/ai-context.test.ts
+++ b/gitnexus/test/unit/ai-context.test.ts
@ -442,4 +442,238 @@ Old content here.
      await fs.rm(crlfDir, { recursive: true, force: true });
    }
  });
+
+  // ──────────────────────────────────────────────────────────────────
+  // Keep-marker edge cases (added to address PR #1508 review findings)
+  // ──────────────────────────────────────────────────────────────────
+
+  it('keep marker OUTSIDE the GitNexus section has no effect (#1508 review F5)', async () => {
+    const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-keep-scope-'));
+    try {
+      const claudePath = path.join(dir, 'CLAUDE.md');
+      // Keep marker appears in user prose BEFORE the GitNexus section.
+      // The keep-path must NOT be triggered — full template replacement
+      // is the correct behavior here, because the marker is not inside
+      // the generated block.
+      const fileWithOutOfBandMarker = `# My Project
+
+A note about <!-- gitnexus:keep --> markers: they only apply inside the
+GitNexus block below, not in prose like this.
+
+<!-- gitnexus:start -->
+Old verbose stub here.
+<!-- gitnexus:end -->
+`;
+      await fs.writeFile(claudePath, fileWithOutOfBandMarker, 'utf-8');
+
+      const stats = { nodes: 50, edges: 100, processes: 5 };
+      await generateAIContextFiles(dir, path.join(dir, '.gitnexus'), 'TestProject', stats);
+
+      const result = await fs.readFile(claudePath, 'utf-8');
+      // Section MUST have been fully replaced — keep marker outside section ignored
+      expect(result).toContain('## Always Do');
+      expect(result).not.toContain('Old verbose stub here.');
+      // User's prose with the marker reference is preserved untouched
+      expect(result).toContain('A note about <!-- gitnexus:keep --> markers');
+    } finally {
+      await fs.rm(dir, { recursive: true, force: true });
+    }
+  });
+
+  it('AGENTS.md keep path preserves custom layout (#1508 review F5)', async () => {
+    const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-keep-agents-'));
+    try {
+      const agentsPath = path.join(dir, 'AGENTS.md');
+      const customAgents = `# AGENTS instructions
+
+Project-specific agent guidance.
+
+<!-- gitnexus:start -->
+<!-- gitnexus:keep -->
+# GitNexus context for AGENTS
+
+Indexed as **AgentsTest** (10 symbols, 20 relationships, 1 execution flows).
+
+Use 'query' for finding flows, 'context' for symbol details.
+<!-- gitnexus:end -->
+`;
+      await fs.writeFile(agentsPath, customAgents, 'utf-8');
+
+      const stats = { nodes: 777, edges: 888, processes: 9 };
+      await generateAIContextFiles(dir, path.join(dir, '.gitnexus'), 'AgentsTest', stats);
+
+      const result = await fs.readFile(agentsPath, 'utf-8');
+      // Stats updated
+      expect(result).toContain('777 symbols');
+      expect(result).toContain('888 relationships');
+      expect(result).toContain('9 execution flows');
+      // Custom layout preserved
+      expect(result).toContain('# GitNexus context for AGENTS');
+      expect(result).toContain("Use 'query' for finding flows");
+      // Verbose template NOT injected
+      expect(result).not.toContain('## Always Do');
+      // Non-GitNexus content preserved
+      expect(result).toContain('# AGENTS instructions');
+      expect(result).toContain('Project-specific agent guidance.');
+    } finally {
+      await fs.rm(dir, { recursive: true, force: true });
+    }
+  });
+
+  it('idempotent: second run with keep marker produces byte-identical output (#1508 review F5)', async () => {
+    const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-keep-idem-'));
+    try {
+      const claudePath = path.join(dir, 'CLAUDE.md');
+      const seed = `# Project
+
+<!-- gitnexus:start -->
+<!-- gitnexus:keep -->
+Indexed as **Idem** (1 symbols, 2 relationships, 3 execution flows). Custom.
+<!-- gitnexus:end -->
+`;
+      await fs.writeFile(claudePath, seed, 'utf-8');
+
+      const stats = { nodes: 99, edges: 100, processes: 7 };
+      await generateAIContextFiles(dir, path.join(dir, '.gitnexus'), 'Idem', stats);
+      const afterFirst = await fs.readFile(claudePath, 'utf-8');
+
+      await generateAIContextFiles(dir, path.join(dir, '.gitnexus'), 'Idem', stats);
+      const afterSecond = await fs.readFile(claudePath, 'utf-8');
+
+      expect(afterSecond).toBe(afterFirst);
+    } finally {
+      await fs.rm(dir, { recursive: true, force: true });
+    }
+  });
+
+  it('CRLF file with keep marker: stats line updates without corrupting content (#1508 review F5)', async () => {
+    const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-keep-crlf-'));
+    try {
+      const claudePath = path.join(dir, 'CLAUDE.md');
+      const crlfContent =
+        '# Project\r\n' +
+        '\r\n' +
+        '<!-- gitnexus:start -->\r\n' +
+        '<!-- gitnexus:keep -->\r\n' +
+        'Indexed as **CRLFTest** (5 symbols, 6 relationships, 7 execution flows). Custom CRLF.\r\n' +
+        '<!-- gitnexus:end -->\r\n';
+      await fs.writeFile(claudePath, crlfContent, 'utf-8');
+
+      const stats = { nodes: 50, edges: 60, processes: 7 };
+      await generateAIContextFiles(dir, path.join(dir, '.gitnexus'), 'CRLFTest', stats);
+
+      const result = await fs.readFile(claudePath, 'utf-8');
+      // Stats updated correctly
+      expect(result).toContain('50 symbols');
+      expect(result).toContain('60 relationships');
+      // Custom prose preserved
+      expect(result).toContain('Custom CRLF');
+      // No verbose template injected
+      expect(result).not.toContain('## Always Do');
+    } finally {
+      await fs.rm(dir, { recursive: true, force: true });
+    }
+  });
+
+  it('noStats + keep marker: stats line update is NOT corrupted by Always-Do tuple text (#1508 review F3)', async () => {
+    // Regression guard: with the old fallback regex `\(([^)]+)\)`, when
+    // noStats=true suppressed the canonical stats line from generated
+    // content, the fallback matched the FIRST parenthesized text in the
+    // template, which was `({target: "symbolName", direction: "upstream"})`
+    // from the Always Do bullet — silently writing that as the stats line.
+    const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-keep-nostats-'));
+    try {
+      const claudePath = path.join(dir, 'CLAUDE.md');
+      const seed = `<!-- gitnexus:start -->
+<!-- gitnexus:keep -->
+Indexed as **NoStatsTest** (1 symbols, 1 relationships, 1 execution flows). Custom.
+<!-- gitnexus:end -->
+`;
+      await fs.writeFile(claudePath, seed, 'utf-8');
+
+      const stats = { nodes: 42, edges: 84, processes: 3 };
+      await generateAIContextFiles(
+        dir,
+        path.join(dir, '.gitnexus'),
+        'NoStatsTest',
+        stats,
+        undefined,
+        { noStats: true },
+      );
+
+      const result = await fs.readFile(claudePath, 'utf-8');
+      // Stats line MUST NOT have been corrupted with the Always-Do tuple text
+      expect(result).not.toMatch(/\(\{target:/);
+      expect(result).not.toMatch(/direction:\s*"upstream"/);
+      // Stats line should reflect a sensible numeric update (passed stats)
+      expect(result).toContain('42 symbols');
+      // Custom prose still preserved
+      expect(result).toContain('Custom.');
+    } finally {
+      await fs.rm(dir, { recursive: true, force: true });
+    }
+  });
+
+  it("returns 'preserved' (not 'updated') when keep marker is present but no stats line matches (#1508 review F1)", async () => {
+    // Regression guard for the misleading-return-value bug: previously the
+    // function returned 'updated' without writing when the keep-section had
+    // no recognizable stats line, causing CLI output to claim files were
+    // updated when they were not.
+    const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-keep-noline-'));
+    try {
+      const claudePath = path.join(dir, 'CLAUDE.md');
+      // Custom keep-section with NO "Indexed as ..." or "indexed by GitNexus as ..." line
+      const seed = `# Project
+
+<!-- gitnexus:start -->
+<!-- gitnexus:keep -->
+# GitNexus block (custom, no stats line)
+
+This block intentionally omits the standard stats line.
+<!-- gitnexus:end -->
+`;
+      await fs.writeFile(claudePath, seed, 'utf-8');
+
+      const stats = { nodes: 100, edges: 200, processes: 10 };
+      const result = await generateAIContextFiles(
+        dir,
+        path.join(dir, '.gitnexus'),
+        'NoLineTest',
+        stats,
+      );
+
+      // The result manifest should reflect 'preserved', not 'updated'
+      expect(result.files).toContain('CLAUDE.md (preserved)');
+      // File on disk is unchanged
+      const onDisk = await fs.readFile(claudePath, 'utf-8');
+      expect(onDisk).toBe(seed);
+    } finally {
+      await fs.rm(dir, { recursive: true, force: true });
+    }
+  });
+
+  it('project name with markdown-sensitive punctuation lands intact in stats line (#1508 review F5)', async () => {
+    const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-keep-punct-'));
+    try {
+      const claudePath = path.join(dir, 'CLAUDE.md');
+      const seed = `<!-- gitnexus:start -->
+<!-- gitnexus:keep -->
+Indexed as **placeholder** (1 symbols, 1 relationships, 1 execution flows). Custom.
+<!-- gitnexus:end -->
+`;
+      await fs.writeFile(claudePath, seed, 'utf-8');
+
+      // Name with hyphens, dot, and slash — exactly what dp-web4/some-repo
+      // style names look like
+      const trickyName = 'dp-web4/some-repo.v2';
+      const stats = { nodes: 5, edges: 10, processes: 1 };
+      await generateAIContextFiles(dir, path.join(dir, '.gitnexus'), trickyName, stats);
+
+      const result = await fs.readFile(claudePath, 'utf-8');
+      // The full name appears in the bold of the stats line, intact
+      expect(result).toContain(`Indexed as **${trickyName}** (5 symbols`);
+    } finally {
+      await fs.rm(dir, { recursive: true, force: true });
+    }
+  });
 });